Skip to contentSkip to conversation

drm/i915/opregion: work around buggy firmware that provides 8+ output devices

PATCH In review Aaron Lu 6 participants 14 messages Feb 12, 2014 – Dec 9, 2014

Subsystems (3)
DRM drivers Intel i915 ACPI
lore
Share & tools
lore

Aaron Lu

author
#0 lore raw

PATCH 1/1 drm/i915/opregion: work around buggy firmware that provides 8+ output devices

The ACPI table on ASUS UX302LA has more than 8 output devices under the graphics controller device node. The problem is, the real active output device, the LCD panel, is listed the last. The result is, the LCD's device id doesn't get recorded in the active device list CADL array and when the _DCS control method for the LCD device is executed, it returns 0x1d, meaning it is not active. This affects the hotkey delivery ASL code that will not deliver a notification if the output device is not active on backlight hotkey press.

I don't see a clean way to solve this problem since the operation region spec doesn't allow more than 8 output devices so we have no way of storing all these output devices. The fact that output devices that have _BCM control method usually means they have a higher possibility of being used than those who don't made me choose a simple way to work around the buggy firmware by replacing the last entry in CADL array with the one that has _BCM control method. There is no specific reason why the last entry is picked instead of others.

Buglink https://bugzilla.kernel.org/show_bug.cgi?id=70241
Signed-off-by Aaron Lu <[email protected]>
Reported-and-tested-by Oleksij Rempel <[email protected]>
---
 drivers/gpu/drm/i915/intel_opregion.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_opregion.c +15 -2
index 4e960ec7419f..fc4348284f41 100644
@@ -613,6 +613,7 @@ static void intel_didl_outputs(struct drm_device *dev)
 	acpi_status status;
 	u32 temp;
 	int i = 0;
+	bool done;
 
 	handle = ACPI_HANDLE(&dev->pdev->dev);
 	if (!handle || acpi_bus_get_device(handle, &acpi_dev))

@@ -634,11 +635,20 @@ static void intel_didl_outputs(struct drm_device *dev)
 		return;
 	}
 
+	done = false;
 	list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) {
 		if (i >= 8) {
 			dev_dbg(&dev->pdev->dev,
-				"More than 8 outputs detected via ACPI\n");
-			return;
+				"More than 8 outputs detected via ACPI, %s\n",
+				acpi_device_bid(acpi_cdev));
+			if (acpi_has_method(acpi_cdev->handle, "_BCM")) {
+				dev_dbg(&dev->pdev->dev,
+					"%s has _BCM, replacing 8th entry\n",
+					acpi_device_bid(acpi_cdev));
+				i = 7;
+				done = true;
+			} else
+				continue;
 		}
 		status =
 			acpi_evaluate_integer(acpi_cdev->handle, "_ADR",

@@ -650,6 +660,9 @@ static void intel_didl_outputs(struct drm_device *dev)
 				  &opregion->acpi->didl[i]);
 			i++;
 		}
+
+		if (done)
+			return;
 	}
 
 end:
Signature · 1 line
1.8.5.3
#1 lore raw
Quoted 9 lines · Aaron Lu

The ACPI table on ASUS UX302LA has more than 8 output devices under the graphics controller device node. The problem is, the real active output device, the LCD panel, is listed the last. The result is, the LCD's device id doesn't get recorded in the active device list CADL array and when the _DCS control method for the LCD device is executed, it returns 0x1d, meaning it is not active. This affects the hotkey delivery ASL code that will not deliver a notification if the output device is not active on backlight hotkey press.

Quoted 8 lines

I don't see a clean way to solve this problem since the operation region spec doesn't allow more than 8 output devices so we have no way of storing all these output devices. The fact that output devices that have _BCM control method usually means they have a higher possibility of being used than those who don't made me choose a simple way to work around the buggy firmware by replacing the last entry in CADL array with the one that has _BCM control method. There is no specific reason why the last entry is picked instead of others.

Another possibility is that the connector list is in rough priority order so might be useful for sorting the CADL array.

Since the CADL should only be a list of currently active devices, we could just bite the bullet and repopulate it correctly after every setcrtc.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
maintainer
#2 lore raw
Quoted 19 lines · Chris Wilson
Quoted 9 lines · Aaron Lu

The ACPI table on ASUS UX302LA has more than 8 output devices under the graphics controller device node. The problem is, the real active output device, the LCD panel, is listed the last. The result is, the LCD's device id doesn't get recorded in the active device list CADL array and when the _DCS control method for the LCD device is executed, it returns 0x1d, meaning it is not active. This affects the hotkey delivery ASL code that will not deliver a notification if the output device is not active on backlight hotkey press.

Quoted 8 lines

I don't see a clean way to solve this problem since the operation region spec doesn't allow more than 8 output devices so we have no way of storing all these output devices. The fact that output devices that have _BCM control method usually means they have a higher possibility of being used than those who don't made me choose a simple way to work around the buggy firmware by replacing the last entry in CADL array with the one that has _BCM control method. There is no specific reason why the last entry is picked instead of others.

Quoted 6 lines

Another possibility is that the connector list is in rough priority order so might be useful for sorting the CADL array.

Since the CADL should only be a list of currently active devices, we could just bite the bullet and repopulate it correctly after every setcrtc.

Agreed. Per spec,

DIDL: Writes - Graphics driver writes to this field once during its initialization after determining platform supported connectors.

CPDL: Writes - Graphics driver writes to this field on every monitor detection process.

CADL: Writes - Graphics driver writes to this field on every mode set process and during boot.

And for all of the above: Writes - System BIOS POST or ASL code should not write to these fields.

BR,
Jani.

Signature · 1 line
Jani Nikula, Intel Open Source Technology Center
author
#3 lore raw
In reply to Chris Wilson
Quoted 19 lines · Chris Wilson
Quoted 9 lines · Aaron Lu

The ACPI table on ASUS UX302LA has more than 8 output devices under the graphics controller device node. The problem is, the real active output device, the LCD panel, is listed the last. The result is, the LCD's device id doesn't get recorded in the active device list CADL array and when the _DCS control method for the LCD device is executed, it returns 0x1d, meaning it is not active. This affects the hotkey delivery ASL code that will not deliver a notification if the output device is not active on backlight hotkey press.

Quoted 8 lines

I don't see a clean way to solve this problem since the operation region spec doesn't allow more than 8 output devices so we have no way of storing all these output devices. The fact that output devices that have _BCM control method usually means they have a higher possibility of being used than those who don't made me choose a simple way to work around the buggy firmware by replacing the last entry in CADL array with the one that has _BCM control method. There is no specific reason why the last entry is picked instead of others.

Quoted 6 lines

Another possibility is that the connector list is in rough priority order so might be useful for sorting the CADL array.

Since the CADL should only be a list of currently active devices, we could just bite the bullet and repopulate it correctly after every setcrtc.

Thanks for the suggestion. As a first step, does the following un-tested patch look OK?

drivers/gpu/drm/i915/intel_opregion.c +43 -16
index acde2945eb8a..afdd7d84fb32 100644
@@ -220,11 +220,11 @@ struct opregion_asle {
 #define SWSCI_SBCB_POST_VBE_PM		SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
 #define SWSCI_SBCB_ENABLE_DISABLE_AUDIO	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 21)
 
-#define ACPI_OTHER_OUTPUT (0<<8)
-#define ACPI_VGA_OUTPUT (1<<8)
-#define ACPI_TV_OUTPUT (2<<8)
-#define ACPI_DIGITAL_OUTPUT (3<<8)
-#define ACPI_LVDS_OUTPUT (4<<8)
+#define ACPI_OTHER_OUTPUT 0
+#define ACPI_VGA_OUTPUT 1
+#define ACPI_TV_OUTPUT 2
+#define ACPI_DIGITAL_OUTPUT 3
+#define ACPI_LVDS_OUTPUT 4
 
 #define MAX_DSLP	1500
 

@@ -616,6 +616,7 @@ static void intel_didl_outputs(struct drm_device *dev)
 	acpi_status status;
 	u32 temp;
 	int i = 0;
+	int index[5] = {0};
 
 	handle = ACPI_HANDLE(&dev->pdev->dev);
 	if (!handle || acpi_bus_get_device(handle, &acpi_dev))

@@ -693,8 +694,8 @@ blind_set:
 			break;
 		}
 		temp = ioread32(&opregion->acpi->didl[i]);
-		iowrite32(temp | (1<<31) | output_type | i,
-			  &opregion->acpi->didl[i]);
+		iowrite32(temp | (1<<31) | (output_type << 8) |
+			  index[output_type]++, &opregion->acpi->didl[i]);
 		i++;
 	}
 	goto end;

@@ -705,18 +706,44 @@ static void intel_setup_cadls(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_opregion *opregion = &dev_priv->opregion;
 	int i = 0;
+	int index[5] = {0};
 	u32 disp_id;
+	struct drm_connector *connector;
+
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+		int output_type = ACPI_OTHER_OUTPUT;
 
-	/* Initialize the CADL field by duplicating the DIDL values.
-	 * Technically, this is not always correct as display outputs may exist,
-	 * but not active. This initialization is necessary for some Clevo
-	 * laptops that check this field before processing the brightness and
-	 * display switching hotkeys. Just like DIDL, CADL is NULL-terminated if
-	 * there are less than eight devices. */
-	do {
-		disp_id = ioread32(&opregion->acpi->didl[i]);
+		switch (connector->connector_type) {
+		case DRM_MODE_CONNECTOR_VGA:
+		case DRM_MODE_CONNECTOR_DVIA:
+			output_type = ACPI_VGA_OUTPUT;
+			break;
+		case DRM_MODE_CONNECTOR_Composite:
+		case DRM_MODE_CONNECTOR_SVIDEO:
+		case DRM_MODE_CONNECTOR_Component:
+		case DRM_MODE_CONNECTOR_9PinDIN:
+			output_type = ACPI_TV_OUTPUT;
+			break;
+		case DRM_MODE_CONNECTOR_DVII:
+		case DRM_MODE_CONNECTOR_DVID:
+		case DRM_MODE_CONNECTOR_DisplayPort:
+		case DRM_MODE_CONNECTOR_HDMIA:
+		case DRM_MODE_CONNECTOR_HDMIB:
+			output_type = ACPI_DIGITAL_OUTPUT;
+			break;
+		case DRM_MODE_CONNECTOR_eDP:
+		case DRM_MODE_CONNECTOR_LVDS:
+			output_type = ACPI_LVDS_OUTPUT;
+			break;
+		}
+		disp_id = (opregion->acpi->didl[0] & (1 << 31)) |
+			  (output_type << 8) | index[output_type]++;
 		iowrite32(disp_id, &opregion->acpi->cadl[i]);
-	} while (++i < 8 && disp_id != 0);
+		i++;
+	}
+
+	if (i < 8)
+		iowrite32(0, &opregion->acpi->cadl[i]);
 }
 
 void intel_opregion_init(struct drm_device *dev)

Thanks,
Aaron

#4 lore raw
Quoted 23 lines · Aaron Lu
Quoted 22 lines · Chris Wilson
Quoted 9 lines · Aaron Lu

The ACPI table on ASUS UX302LA has more than 8 output devices under the graphics controller device node. The problem is, the real active output device, the LCD panel, is listed the last. The result is, the LCD's device id doesn't get recorded in the active device list CADL array and when the _DCS control method for the LCD device is executed, it returns 0x1d, meaning it is not active. This affects the hotkey delivery ASL code that will not deliver a notification if the output device is not active on backlight hotkey press.

Quoted 8 lines

I don't see a clean way to solve this problem since the operation region spec doesn't allow more than 8 output devices so we have no way of storing all these output devices. The fact that output devices that have _BCM control method usually means they have a higher possibility of being used than those who don't made me choose a simple way to work around the buggy firmware by replacing the last entry in CADL array with the one that has _BCM control method. There is no specific reason why the last entry is picked instead of others.

Another possibility is that the connector list is in rough priority order so might be useful for sorting the CADL array.

Quoted 6 lines
Quoted 3 lines

Since the CADL should only be a list of currently active devices, we could just bite the bullet and repopulate it correctly after every setcrtc.

Thanks for the suggestion. As a first step, does the following un-tested patch look OK?

Yes. Maybe worth putting together the similar routines for blind setting the didl and the cadl, or at least for computing the value from the connector. For instance, the didl logic disagrees with the value of index - is that relevant? I have a suspicion that the CADL entry should match the DIDL entry for the connector, but that is not actually mentioned in the opregion spec afaict.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#5 lore raw
Quoted 31 lines · Chris Wilson
Quoted 23 lines · Aaron Lu
Quoted 22 lines · Chris Wilson
Quoted 9 lines · Aaron Lu

The ACPI table on ASUS UX302LA has more than 8 output devices under the graphics controller device node. The problem is, the real active output device, the LCD panel, is listed the last. The result is, the LCD's device id doesn't get recorded in the active device list CADL array and when the _DCS control method for the LCD device is executed, it returns 0x1d, meaning it is not active. This affects the hotkey delivery ASL code that will not deliver a notification if the output device is not active on backlight hotkey press.

Quoted 8 lines

I don't see a clean way to solve this problem since the operation region spec doesn't allow more than 8 output devices so we have no way of storing all these output devices. The fact that output devices that have _BCM control method usually means they have a higher possibility of being used than those who don't made me choose a simple way to work around the buggy firmware by replacing the last entry in CADL array with the one that has _BCM control method. There is no specific reason why the last entry is picked instead of others.

Another possibility is that the connector list is in rough priority order so might be useful for sorting the CADL array.

Quoted 6 lines
Quoted 3 lines

Since the CADL should only be a list of currently active devices, we could just bite the bullet and repopulate it correctly after every setcrtc.

Thanks for the suggestion. As a first step, does the following un-tested patch look OK?

Quoted 6 lines

Yes. Maybe worth putting together the similar routines for blind setting the didl and the cadl, or at least for computing the value from the connector. For instance, the didl logic disagrees with the value of index - is that relevant? I have a suspicion that the CADL entry should match the DIDL entry for the connector, but that is not actually mentioned in the opregion spec afaict.

I think a problem is that often we have more than one output for a given type. So we need to somehow match them up to make sure we put the right ones intot didl/cadl lists. The issue here is that our connectors don't match up perfectly with the acpi output entries (since we have separate dp/hdmi outputs). But I think it would be worthwhile trying to match them up and store a link from struct intel_connector to the right acpi node acpi node.

Then we could generate the didl/cadl lists by walking all connectors (only looking at the enabled ones for cadl) and evaluating the _ADR node of the linked apci node. As long as we ensure that we don't have duplicated entries we should be fine.

This is a bit more work though ... And I have no idea really how firmware uses these lists (besides for backlight purposes apparently). -Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
author
#6 lore raw
Quoted 39 lines · Daniel Vetter
Quoted 31 lines · Chris Wilson
Quoted 23 lines · Aaron Lu
Quoted 22 lines · Chris Wilson
Quoted 9 lines · Aaron Lu

The ACPI table on ASUS UX302LA has more than 8 output devices under the graphics controller device node. The problem is, the real active output device, the LCD panel, is listed the last. The result is, the LCD's device id doesn't get recorded in the active device list CADL array and when the _DCS control method for the LCD device is executed, it returns 0x1d, meaning it is not active. This affects the hotkey delivery ASL code that will not deliver a notification if the output device is not active on backlight hotkey press.

Quoted 8 lines

I don't see a clean way to solve this problem since the operation region spec doesn't allow more than 8 output devices so we have no way of storing all these output devices. The fact that output devices that have _BCM control method usually means they have a higher possibility of being used than those who don't made me choose a simple way to work around the buggy firmware by replacing the last entry in CADL array with the one that has _BCM control method. There is no specific reason why the last entry is picked instead of others.

Another possibility is that the connector list is in rough priority order so might be useful for sorting the CADL array.

Quoted 6 lines
Quoted 3 lines

Since the CADL should only be a list of currently active devices, we could just bite the bullet and repopulate it correctly after every setcrtc.

Thanks for the suggestion. As a first step, does the following un-tested patch look OK?

Quoted 6 lines

Yes. Maybe worth putting together the similar routines for blind setting the didl and the cadl, or at least for computing the value from the connector. For instance, the didl logic disagrees with the value of index - is that relevant? I have a suspicion that the CADL entry should match the DIDL entry for the connector, but that is not actually mentioned in the opregion spec afaict.

Quoted 7 lines

I think a problem is that often we have more than one output for a given type. So we need to somehow match them up to make sure we put the right ones intot didl/cadl lists. The issue here is that our connectors don't match up perfectly with the acpi output entries (since we have separate dp/hdmi outputs). But I think it would be worthwhile trying to match them up and store a link from struct intel_connector to the right acpi node acpi node.

Is this possible? I don't see a way to match them up...
The _ADR control method uses a field that seems to be assigned by BIOS like this:

                Device (LCDD)
                {
                  Method (_ADR, 0, Serialized)  // _ADR: Address
                    {
                        Return (And (0xFFFF, DID2))
                    }
		}

DID2 is in system memory region and has some assigned value like 0x400 when we read it. For this case it is easy since there is only one output device that is of type LVDS so we can match it to connector of type eDP or LVDS, suppose there is only one such connector. But for output devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea how to match them up to the connectors of that type as we can't be sure the probe order we have used in i915 driver is the same as BIOS'.

In the mean time, it seems we can just use driver's information to populate both DIDL and CADL and forget the _ADR of those devices like the following patch does:

drivers/gpu/drm/i915/intel_drv.h +3 -0
index a4ffc021c317..55956a517a77 100644
@@ -204,6 +204,9 @@ struct intel_connector {
 	/* since POLL and HPD connectors may use the same HPD line keep the native
 	   state of connector->polled in case hotplug storm detection changes it */
 	u8 polled;
+
+	/* device id with type and index information */
+	u32 disp_id;
 };
 
 typedef struct dpll {
drivers/gpu/drm/i915/intel_opregion.c +35 -72

145 lines not on this page · raw message

I've tested the patch locally on some laptops but I've no idea of what impact it has on others.

Thanks,
Aaron

#7 lore raw
Quoted 7 lines · Aaron Lu

DID2 is in system memory region and has some assigned value like 0x400 when we read it. For this case it is easy since there is only one output device that is of type LVDS so we can match it to connector of type eDP or LVDS, suppose there is only one such connector. But for output devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea how to match them up to the connectors of that type as we can't be sure the probe order we have used in i915 driver is the same as BIOS'.

Non-standard _ADR values are assigend by the GPU vendor, so Intel should be able to provide you with the correct interpretations.

Signature · 1 line
Matthew Garrett | [email protected]
author
#8 lore raw
Quoted 10 lines · Matthew Garrett
Quoted 7 lines · Aaron Lu

DID2 is in system memory region and has some assigned value like 0x400 when we read it. For this case it is easy since there is only one output device that is of type LVDS so we can match it to connector of type eDP or LVDS, suppose there is only one such connector. But for output devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea how to match them up to the connectors of that type as we can't be sure the probe order we have used in i915 driver is the same as BIOS'.

Quoted 2 lines

Non-standard _ADR values are assigend by the GPU vendor, so Intel should be able to provide you with the correct interpretations.

It doesn't seem the _ADR value has to be the format defined by _DOD, as the example of the ACPI spec gives:

Method (_ADR, 0) {
    return(0x0100)
}

So that is not the problem here.

The problem is, we don't have any way of matching an ACPI output device node to a drm connector of the same type when there are more than 1 of those with the same type, i.e. we don't know how the index value are assigned by BIOS.

#9 lore raw
Quoted 19 lines · Aaron Lu
Quoted 10 lines · Matthew Garrett
Quoted 7 lines · Aaron Lu

DID2 is in system memory region and has some assigned value like 0x400 when we read it. For this case it is easy since there is only one output device that is of type LVDS so we can match it to connector of type eDP or LVDS, suppose there is only one such connector. But for output devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea how to match them up to the connectors of that type as we can't be sure the probe order we have used in i915 driver is the same as BIOS'.

Quoted 2 lines

Non-standard _ADR values are assigend by the GPU vendor, so Intel should be able to provide you with the correct interpretations.

It doesn't seem the _ADR value has to be the format defined by _DOD, as the example of the ACPI spec gives:

Method (_ADR, 0) {
    return(0x0100)
}
Quoted 6 lines

So that is not the problem here.

The problem is, we don't have any way of matching an ACPI output device node to a drm connector of the same type when there are more than 1 of those with the same type, i.e. we don't know how the index value are assigned by BIOS.

I've thought the OpRegion spec has some additional fields in there indicating the port number, which we could match up at least on modern platforms (where there's only ever port A-E). But that's very hazy recollection from a really old OpRegion spec, i.e. I have no bloody clue at all ;-)

If I misremember this then we need to start on a begging tour again and ask the windows guys how this is all supposed to work ... -Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
author
#10 lore raw

We have a new bug report that has the same problem:
https://bugzilla.kernel.org/show_bug.cgi?id=88941

The posted patch solves the problem. I know it's not perfect, but it doesn't seem it would do any harm to existing systems so should be safe.

Better, if someone can shed some light on how this should be properly handled, that would be great.

Thanks,
Aaron

Quoted 33 lines · Daniel Vetter
Quoted 19 lines · Aaron Lu
Quoted 10 lines · Matthew Garrett
Quoted 7 lines · Aaron Lu

DID2 is in system memory region and has some assigned value like 0x400 when we read it. For this case it is easy since there is only one output device that is of type LVDS so we can match it to connector of type eDP or LVDS, suppose there is only one such connector. But for output devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea how to match them up to the connectors of that type as we can't be sure the probe order we have used in i915 driver is the same as BIOS'.

Quoted 2 lines

Non-standard _ADR values are assigend by the GPU vendor, so Intel should be able to provide you with the correct interpretations.

It doesn't seem the _ADR value has to be the format defined by _DOD, as the example of the ACPI spec gives:

Method (_ADR, 0) {
    return(0x0100)
}
Quoted 6 lines

So that is not the problem here.

The problem is, we don't have any way of matching an ACPI output device node to a drm connector of the same type when there are more than 1 of those with the same type, i.e. we don't know how the index value are assigned by BIOS.

I've thought the OpRegion spec has some additional fields in there indicating the port number, which we could match up at least on modern platforms (where there's only ever port A-E). But that's very hazy recollection from a really old OpRegion spec, i.e. I have no bloody clue at all ;-)

Quoted 4 lines

If I misremember this then we need to start on a begging tour again and ask the windows guys how this is all supposed to work ... -Daniel

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

maintainer
#11 lore raw
Quoted 8 lines · Aaron Lu

We have a new bug report that has the same problem:
https://bugzilla.kernel.org/show_bug.cgi?id=88941

The posted patch solves the problem. I know it's not perfect, but it doesn't seem it would do any harm to existing systems so should be safe.

Better, if someone can shed some light on how this should be properly handled, that would be great.

There was a bug report that I can't find right now that had a similar problem. I wrote a few patches, even somewhat polished ones (that I now pushed to [1] for reference) to handle extended DIDL. Unfortunately this didn't help the bug reporter because the right one was beyond the extended DIDL too, so I don't think I even sent these to the list.

Anyway, just one more data point. This might help your reporter, so worth a try. But it doesn't solve everything.

BR,
Jani.

Quoted 43 lines

Thanks,
Aaron

Quoted 37 lines · Daniel Vetter
Quoted 19 lines · Aaron Lu
Quoted 10 lines · Matthew Garrett
Quoted 7 lines · Aaron Lu

DID2 is in system memory region and has some assigned value like 0x400 when we read it. For this case it is easy since there is only one output device that is of type LVDS so we can match it to connector of type eDP or LVDS, suppose there is only one such connector. But for output devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea how to match them up to the connectors of that type as we can't be sure the probe order we have used in i915 driver is the same as BIOS'.

Quoted 2 lines

Non-standard _ADR values are assigend by the GPU vendor, so Intel should be able to provide you with the correct interpretations.

It doesn't seem the _ADR value has to be the format defined by _DOD, as the example of the ACPI spec gives:

Method (_ADR, 0) {
    return(0x0100)
}
Quoted 6 lines

So that is not the problem here.

The problem is, we don't have any way of matching an ACPI output device node to a drm connector of the same type when there are more than 1 of those with the same type, i.e. we don't know how the index value are assigned by BIOS.

I've thought the OpRegion spec has some additional fields in there indicating the port number, which we could match up at least on modern platforms (where there's only ever port A-E). But that's very hazy recollection from a really old OpRegion spec, i.e. I have no bloody clue at all ;-)

If I misremember this then we need to start on a begging tour again and ask the windows guys how this is all supposed to work ... -Daniel

Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
maintainer
#12 lore raw
Quoted 10 lines · Jani Nikula
Quoted 8 lines · Aaron Lu

We have a new bug report that has the same problem:
https://bugzilla.kernel.org/show_bug.cgi?id=88941

The posted patch solves the problem. I know it's not perfect, but it doesn't seem it would do any harm to existing systems so should be safe.

Better, if someone can shed some light on how this should be properly handled, that would be great.

Quoted 8 lines

There was a bug report that I can't find right now that had a similar problem. I wrote a few patches, even somewhat polished ones (that I now pushed to [1] for reference) to handle extended DIDL. Unfortunately this didn't help the bug reporter because the right one was beyond the extended DIDL too, so I don't think I even sent these to the list.

Anyway, just one more data point. This might help your reporter, so worth a try. But it doesn't solve everything.

[1] http://cgit.freedesktop.org/~jani/drm/log/?h=didl

Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#13 lore raw
Quoted 17 lines · Jani Nikula
Quoted 16 lines · Jani Nikula
Quoted 8 lines · Aaron Lu

We have a new bug report that has the same problem:
https://bugzilla.kernel.org/show_bug.cgi?id=88941

The posted patch solves the problem. I know it's not perfect, but it doesn't seem it would do any harm to existing systems so should be safe.

Better, if someone can shed some light on how this should be properly handled, that would be great.

There was a bug report that I can't find right now that had a similar problem. I wrote a few patches, even somewhat polished ones (that I now pushed to [1] for reference) to handle extended DIDL. Unfortunately this didn't help the bug reporter because the right one was beyond the extended DIDL too, so I don't think I even sent these to the list.

Quoted 4 lines
Quoted 2 lines

Anyway, just one more data point. This might help your reporter, so worth a try. But it doesn't solve everything.

[1] http://cgit.freedesktop.org/~jani/drm/log/?h=didl

Thanks for the info, I've asked Dmitry to give it a try.

Regards,
Aaron

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx