| Message ID | 61c3df83ab73aba0bc7a941a443cd7faf4cf7fb0.1743195250.git.soyer@irl.hu (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | [v2] ACPI: video: Handle fetching EDID as ACPI_TYPE_PACKAGE | expand |
On Fri, Mar 28, 2025 at 10:09 PM Gergo Koteles <soyer@irl.hu> wrote: > > The _DDC method should return a buffer, or an integer in case of an error. > But some Lenovo laptops incorrectly return EDID as buffer in ACPI package. > > Calling _DDC generates this ACPI Warning: > ACPI Warning: \_SB.PCI0.GP17.VGA.LCD._DDC: Return type mismatch - \ > found Package, expected Integer/Buffer (20240827/nspredef-254) > > Use the first element of the package to get the EDID buffer. > > The DSDT: > > Name (AUOP, Package (0x01) > { > Buffer (0x80) > { > ... > } > }) > > ... > > Method (_DDC, 1, NotSerialized) // _DDC: Display Data Current > { > If ((PAID == AUID)) > { > Return (AUOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.AUOP */ > } > ElseIf ((PAID == IVID)) > { > Return (IVOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.IVOP */ > } > ElseIf ((PAID == BOID)) > { > Return (BOEP) /* \_SB_.PCI0.GP17.VGA_.LCD_.BOEP */ > } > ElseIf ((PAID == SAID)) > { > Return (SUNG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUNG */ > } > > Return (Zero) > } > > Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device > Cc: stable@vger.kernel.org > Fixes: c6a837088bed ("drm/amd/display: Fetch the EDID from _DDC if available for eDP") > Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4085 > Signed-off-by: Gergo Koteles <soyer@irl.hu> > --- > Changes in v2: > - Added comment > - Improved commit message > - Link to v1: https://lore.kernel.org/all/4cef341fdf7a0e877c50b502fc95ee8be28aa811.1743129387.git.soyer@irl.hu/ Hans, any concerns here? > drivers/acpi/acpi_video.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c > index efdadc74e3f4..103f29661576 100644 > --- a/drivers/acpi/acpi_video.c > +++ b/drivers/acpi/acpi_video.c > @@ -649,6 +649,13 @@ acpi_video_device_EDID(struct acpi_video_device *device, void **edid, int length > > obj = buffer.pointer; > > + /* > + * Some buggy implementations incorrectly return the EDID buffer in an ACPI package. > + * In this case, extract the buffer from the package. > + */ > + if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 1) > + obj = &obj->package.elements[0]; > + > if (obj && obj->type == ACPI_TYPE_BUFFER) { > *edid = kmemdup(obj->buffer.pointer, obj->buffer.length, GFP_KERNEL); > ret = *edid ? obj->buffer.length : -ENOMEM; > @@ -658,7 +665,7 @@ acpi_video_device_EDID(struct acpi_video_device *device, void **edid, int length > ret = -EFAULT; > } > > - kfree(obj); > + kfree(buffer.pointer); > return ret; > } > > -- > 2.49.0 >
Hi, On 31-Mar-25 1:46 PM, Rafael J. Wysocki wrote: > On Fri, Mar 28, 2025 at 10:09 PM Gergo Koteles <soyer@irl.hu> wrote: >> >> The _DDC method should return a buffer, or an integer in case of an error. >> But some Lenovo laptops incorrectly return EDID as buffer in ACPI package. >> >> Calling _DDC generates this ACPI Warning: >> ACPI Warning: \_SB.PCI0.GP17.VGA.LCD._DDC: Return type mismatch - \ >> found Package, expected Integer/Buffer (20240827/nspredef-254) >> >> Use the first element of the package to get the EDID buffer. >> >> The DSDT: >> >> Name (AUOP, Package (0x01) >> { >> Buffer (0x80) >> { >> ... >> } >> }) >> >> ... >> >> Method (_DDC, 1, NotSerialized) // _DDC: Display Data Current >> { >> If ((PAID == AUID)) >> { >> Return (AUOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.AUOP */ >> } >> ElseIf ((PAID == IVID)) >> { >> Return (IVOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.IVOP */ >> } >> ElseIf ((PAID == BOID)) >> { >> Return (BOEP) /* \_SB_.PCI0.GP17.VGA_.LCD_.BOEP */ >> } >> ElseIf ((PAID == SAID)) >> { >> Return (SUNG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUNG */ >> } >> >> Return (Zero) >> } >> >> Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device >> Cc: stable@vger.kernel.org >> Fixes: c6a837088bed ("drm/amd/display: Fetch the EDID from _DDC if available for eDP") >> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4085 >> Signed-off-by: Gergo Koteles <soyer@irl.hu> >> --- >> Changes in v2: >> - Added comment >> - Improved commit message >> - Link to v1: https://lore.kernel.org/all/4cef341fdf7a0e877c50b502fc95ee8be28aa811.1743129387.git.soyer@irl.hu/ > > Hans, any concerns here? No the patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > >> drivers/acpi/acpi_video.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c >> index efdadc74e3f4..103f29661576 100644 >> --- a/drivers/acpi/acpi_video.c >> +++ b/drivers/acpi/acpi_video.c >> @@ -649,6 +649,13 @@ acpi_video_device_EDID(struct acpi_video_device *device, void **edid, int length >> >> obj = buffer.pointer; >> >> + /* >> + * Some buggy implementations incorrectly return the EDID buffer in an ACPI package. >> + * In this case, extract the buffer from the package. >> + */ >> + if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 1) >> + obj = &obj->package.elements[0]; >> + >> if (obj && obj->type == ACPI_TYPE_BUFFER) { >> *edid = kmemdup(obj->buffer.pointer, obj->buffer.length, GFP_KERNEL); >> ret = *edid ? obj->buffer.length : -ENOMEM; >> @@ -658,7 +665,7 @@ acpi_video_device_EDID(struct acpi_video_device *device, void **edid, int length >> ret = -EFAULT; >> } >> >> - kfree(obj); >> + kfree(buffer.pointer); >> return ret; >> } >> >> -- >> 2.49.0 >> >
On Mon, Mar 31, 2025 at 3:04 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 31-Mar-25 1:46 PM, Rafael J. Wysocki wrote: > > On Fri, Mar 28, 2025 at 10:09 PM Gergo Koteles <soyer@irl.hu> wrote: > >> > >> The _DDC method should return a buffer, or an integer in case of an error. > >> But some Lenovo laptops incorrectly return EDID as buffer in ACPI package. > >> > >> Calling _DDC generates this ACPI Warning: > >> ACPI Warning: \_SB.PCI0.GP17.VGA.LCD._DDC: Return type mismatch - \ > >> found Package, expected Integer/Buffer (20240827/nspredef-254) > >> > >> Use the first element of the package to get the EDID buffer. > >> > >> The DSDT: > >> > >> Name (AUOP, Package (0x01) > >> { > >> Buffer (0x80) > >> { > >> ... > >> } > >> }) > >> > >> ... > >> > >> Method (_DDC, 1, NotSerialized) // _DDC: Display Data Current > >> { > >> If ((PAID == AUID)) > >> { > >> Return (AUOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.AUOP */ > >> } > >> ElseIf ((PAID == IVID)) > >> { > >> Return (IVOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.IVOP */ > >> } > >> ElseIf ((PAID == BOID)) > >> { > >> Return (BOEP) /* \_SB_.PCI0.GP17.VGA_.LCD_.BOEP */ > >> } > >> ElseIf ((PAID == SAID)) > >> { > >> Return (SUNG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUNG */ > >> } > >> > >> Return (Zero) > >> } > >> > >> Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device > >> Cc: stable@vger.kernel.org > >> Fixes: c6a837088bed ("drm/amd/display: Fetch the EDID from _DDC if available for eDP") > >> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4085 > >> Signed-off-by: Gergo Koteles <soyer@irl.hu> > >> --- > >> Changes in v2: > >> - Added comment > >> - Improved commit message > >> - Link to v1: https://lore.kernel.org/all/4cef341fdf7a0e877c50b502fc95ee8be28aa811.1743129387.git.soyer@irl.hu/ > > > > Hans, any concerns here? > > No the patch looks good to me: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> OK, applied as 6.15-rc material, thanks!
On 3/31/2025 08:23, Rafael J. Wysocki wrote: > On Mon, Mar 31, 2025 at 3:04 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi, >> >> On 31-Mar-25 1:46 PM, Rafael J. Wysocki wrote: >>> On Fri, Mar 28, 2025 at 10:09 PM Gergo Koteles <soyer@irl.hu> wrote: >>>> >>>> The _DDC method should return a buffer, or an integer in case of an error. >>>> But some Lenovo laptops incorrectly return EDID as buffer in ACPI package. >>>> >>>> Calling _DDC generates this ACPI Warning: >>>> ACPI Warning: \_SB.PCI0.GP17.VGA.LCD._DDC: Return type mismatch - \ >>>> found Package, expected Integer/Buffer (20240827/nspredef-254) >>>> >>>> Use the first element of the package to get the EDID buffer. >>>> >>>> The DSDT: >>>> >>>> Name (AUOP, Package (0x01) >>>> { >>>> Buffer (0x80) >>>> { >>>> ... >>>> } >>>> }) >>>> >>>> ... >>>> >>>> Method (_DDC, 1, NotSerialized) // _DDC: Display Data Current >>>> { >>>> If ((PAID == AUID)) >>>> { >>>> Return (AUOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.AUOP */ >>>> } >>>> ElseIf ((PAID == IVID)) >>>> { >>>> Return (IVOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.IVOP */ >>>> } >>>> ElseIf ((PAID == BOID)) >>>> { >>>> Return (BOEP) /* \_SB_.PCI0.GP17.VGA_.LCD_.BOEP */ >>>> } >>>> ElseIf ((PAID == SAID)) >>>> { >>>> Return (SUNG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUNG */ >>>> } >>>> >>>> Return (Zero) >>>> } >>>> >>>> Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device >>>> Cc: stable@vger.kernel.org >>>> Fixes: c6a837088bed ("drm/amd/display: Fetch the EDID from _DDC if available for eDP") >>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4085 >>>> Signed-off-by: Gergo Koteles <soyer@irl.hu> >>>> --- >>>> Changes in v2: >>>> - Added comment >>>> - Improved commit message >>>> - Link to v1: https://lore.kernel.org/all/4cef341fdf7a0e877c50b502fc95ee8be28aa811.1743129387.git.soyer@irl.hu/ >>> >>> Hans, any concerns here? >> >> No the patch looks good to me: >> >> Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > OK, applied as 6.15-rc material, thanks! Sorry I'm a bit late to the party; but this looks good to me as well. Thanks Gergo for the solution! Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c index efdadc74e3f4..103f29661576 100644 --- a/drivers/acpi/acpi_video.c +++ b/drivers/acpi/acpi_video.c @@ -649,6 +649,13 @@ acpi_video_device_EDID(struct acpi_video_device *device, void **edid, int length obj = buffer.pointer; + /* + * Some buggy implementations incorrectly return the EDID buffer in an ACPI package. + * In this case, extract the buffer from the package. + */ + if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 1) + obj = &obj->package.elements[0]; + if (obj && obj->type == ACPI_TYPE_BUFFER) { *edid = kmemdup(obj->buffer.pointer, obj->buffer.length, GFP_KERNEL); ret = *edid ? obj->buffer.length : -ENOMEM; @@ -658,7 +665,7 @@ acpi_video_device_EDID(struct acpi_video_device *device, void **edid, int length ret = -EFAULT; } - kfree(obj); + kfree(buffer.pointer); return ret; }
The _DDC method should return a buffer, or an integer in case of an error. But some Lenovo laptops incorrectly return EDID as buffer in ACPI package. Calling _DDC generates this ACPI Warning: ACPI Warning: \_SB.PCI0.GP17.VGA.LCD._DDC: Return type mismatch - \ found Package, expected Integer/Buffer (20240827/nspredef-254) Use the first element of the package to get the EDID buffer. The DSDT: Name (AUOP, Package (0x01) { Buffer (0x80) { ... } }) ... Method (_DDC, 1, NotSerialized) // _DDC: Display Data Current { If ((PAID == AUID)) { Return (AUOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.AUOP */ } ElseIf ((PAID == IVID)) { Return (IVOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.IVOP */ } ElseIf ((PAID == BOID)) { Return (BOEP) /* \_SB_.PCI0.GP17.VGA_.LCD_.BOEP */ } ElseIf ((PAID == SAID)) { Return (SUNG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUNG */ } Return (Zero) } Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device Cc: stable@vger.kernel.org Fixes: c6a837088bed ("drm/amd/display: Fetch the EDID from _DDC if available for eDP") Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4085 Signed-off-by: Gergo Koteles <soyer@irl.hu> --- Changes in v2: - Added comment - Improved commit message - Link to v1: https://lore.kernel.org/all/4cef341fdf7a0e877c50b502fc95ee8be28aa811.1743129387.git.soyer@irl.hu/ drivers/acpi/acpi_video.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)