On Fri, Sep 27, 2024, at 7:06 PM, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limoncie...@amd.com>
>
> Some manufacturers have intentionally put an EDID that differs from
> the EDID on the internal panel on laptops.
>
> Attempt to fetch this EDID if it exists and prefer it over the EDID
> that is provided by the panel. If a user prefers to use the EDID from
> the panel, offer a DC debugging parameter that would disable this.
>
> Reviewed-by: Alex Hung <alex.h...@amd.com>
> Signed-off-by: Mario Limonciello <mario.limoncie...@amd.com>
> ---
> v3:
>  * Change message to INFO when using ACPI EDID
>  * rebase
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 65 ++++++++++++++++++-
>  drivers/gpu/drm/amd/include/amd_shared.h      |  5 ++
>  2 files changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> index b8004ccdcc33..7534e1624e4f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> @@ -23,6 +23,8 @@
>   *
>   */
> 
> +#include <acpi/video.h>
> +
>  #include <linux/string.h>
>  #include <linux/acpi.h>
>  #include <linux/i2c.h>
> @@ -887,6 +889,60 @@ bool dm_helpers_is_dp_sink_present(struct dc_link *link)
>       return dp_sink_present;
>  }
> 
> +static int
> +dm_helpers_probe_acpi_edid(void *data, u8 *buf, unsigned int block, 
> size_t len)
> +{
> +     struct drm_connector *connector = data;
> +     struct acpi_device *acpidev = ACPI_COMPANION(connector->dev->dev);
> +     unsigned char start = block * EDID_LENGTH;
> +     void *edid;
> +     int r;
> +
> +     if (!acpidev)
> +             return -ENODEV;
> +
> +     /* fetch the entire edid from BIOS */
> +     r = acpi_video_get_edid(acpidev, ACPI_VIDEO_DISPLAY_LCD, -1, &edid);
> +     if (r < 0) {
> +             drm_dbg(connector->dev, "Failed to get EDID from ACPI: %d\n", 
> r);
> +             return r;
> +     }
> +     if (len > r || start > r || start + len > r) {
> +             r = -EINVAL;
> +             goto cleanup;
> +     }
> +
> +     memcpy(buf, edid + start, len);
> +     r = 0;
> +
> +cleanup:
> +     kfree(edid);
> +
> +     return r;
> +}
> +
> +static const struct drm_edid *
> +dm_helpers_read_acpi_edid(struct amdgpu_dm_connector *aconnector)
> +{
> +     struct drm_connector *connector = &aconnector->base;
> +
> +     if (amdgpu_dc_debug_mask & DC_DISABLE_ACPI_EDID)
> +             return NULL;
> +
> +     switch (connector->connector_type) {
> +     case DRM_MODE_CONNECTOR_LVDS:
> +     case DRM_MODE_CONNECTOR_eDP:
> +             break;
> +     default:
> +             return NULL;
> +     }
> +
> +     if (connector->force == DRM_FORCE_OFF)
> +             return NULL;
> +
> +     return drm_edid_read_custom(connector, dm_helpers_probe_acpi_edid, 
> connector);
> +}
> +
>  enum dc_edid_status dm_helpers_read_local_edid(
>               struct dc_context *ctx,
>               struct dc_link *link,
> @@ -909,8 +965,11 @@ enum dc_edid_status dm_helpers_read_local_edid(
>        * do check sum and retry to make sure read correct edid.
>        */
>       do {
> -
> -             drm_edid = drm_edid_read_ddc(connector, ddc);
> +             drm_edid = dm_helpers_read_acpi_edid(aconnector);
> +             if (drm_edid)
> +                     drm_info(connector->dev, "Using ACPI provided EDID for 
> %s\n", 
> connector->name);
> +             else
> +                     drm_edid = drm_edid_read_ddc(connector, ddc);
>               drm_edid_connector_update(connector, drm_edid);
> 
>               /* DP Compliance Test 4.2.2.6 */
> @@ -1300,4 +1359,4 @@ bool dm_helpers_is_hdr_on(struct dc_context *ctx, 
> struct dc_stream_state *stream
>  {
>       // TODO
>       return false;
> -}
> \ No newline at end of file
> +}
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
> b/drivers/gpu/drm/amd/include/amd_shared.h
> index 3f91926a50e9..1ec7c5e5249e 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -337,6 +337,11 @@ enum DC_DEBUG_MASK {
>        * @DC_FORCE_IPS_ENABLE: If set, force enable all IPS, all the time.
>        */
>       DC_FORCE_IPS_ENABLE = 0x4000,
> +     /**
> +      * @DC_DISABLE_ACPI_EDID: If set, don't attempt to fetch EDID for
> +      * eDP display from ACPI _DDC method.
> +      */
> +     DC_DISABLE_ACPI_EDID = 0x8000,
>  };
> 
>  enum amd_dpm_forced_level;
> -- 
> 2.43.0

For the series, we tested at Lenovo and it fixed a couple of different issues 
we had seen and reported on different HW models.
 - issue with setting 1600 x 1200 on Z16 G2
 - issue with frequency setting being incorrect on T14 G4 AMD with OLED panels
I didn't do the testing myself (I don't have the configurations on hand that 
reproduce the problem) but my colleagues did.
Can I do a:
Tested-by: Lenovo Linux team <mpearson-len...@squebb.ca>
Or is there some other protocol for this?

Thanks
Mark

Reply via email to