[PATCH v7 10/10] drm/amd/display: Fetch the EDID from _DDC if available for eDP

[Mario Limonciello]

On 9/19/2024 11:03, Alex Hung wrote:
> A minor comment (see inline below).
> 
> Otherwise
> 
> Reviewed-by: Alex Hung <alex.hung at amd.com>
> 
> On 2024-09-18 15:38, Mario Limonciello wrote:
>> 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.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
>> ---
>>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 62 ++++++++++++++++++-
>>   drivers/gpu/drm/amd/include/amd_shared.h      |  5 ++
>>   2 files changed, 66 insertions(+), 1 deletion(-)
>>
>> 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 8f4be7a1ec45..05d3e00ecce0 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>
>> @@ -874,6 +876,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_DEBUG_KMS("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,
>> @@ -896,7 +952,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_DEBUG_KMS("Using ACPI provided EDID for %s\n", 
>> connector->name);
> 
> It is better to always output a message when ACPI's EDID is used without 
> enabling any debug options. How about DRM_INFO?

Thanks, DRM_INFO makes sense for discoverability and will adjust it.

> 
>> +        else
>> +            drm_edid = drm_edid_read_ddc(connector, ddc);
>>           drm_edid_connector_update(connector, drm_edid);
>>           aconnector->drm_edid = drm_edid;
>> 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;