[PATCH 00/13] drm/amd/display: more drm_edid to AMD display driver

[PATCH 00/13] drm/amd/display: more drm_edid to AMD display driver

[Melissa Wen]

Hi,

Siqueira and I worked on a solution to reduce the usage of drm_edid_raw
in the AMD display driver, since the current guideline in the DRM
subsystem is to stop handling raw edid data in driver-specific
implementation and use opaque `drm_edid` object with common-code
helpers.

In short, this series approaches the issue of maintaining DC as an
OS-agnostic component by creating a mid layer to isolate `drm_edid`
helpers that need to be called in the DC code, while allowing other OSes
to implement their specific implementation. This is an extension of [1].

- Patch 1 addresses a possible leak added by previous migration to
  drm_edid.
- Patches 2-6 use common-code, drm_edid helpers to parse edid
  capabilities instead of driver-specific solutions.
- Patches 7-8 are groundwork to reduce the noise of Linux/DRM specific
  code in the DC shared code
- Patch 9 creates a mid layer to make DC embraces different ways of
  handling EDID by platforms.
- Patch 10-11 move open-coded management of raw EDID data to the mid
  layer created before.
- Patch 12 adds drm_edid to dc_sink struct and a mid-layer helper to
  free `drm_edid`.
- Patch 13 switch dc_edid to drm_edid across the driver in a way that
  the DC shared code is little affected by Linux specific stuff.

There are three specific points where we still use drm_edid_raw() in the
driver:
1. raw edid data for write EDID checksum in DP_TEST_EDID_CHECKSUM via
   drm_dp_dpcd_write(), that AFAIK there is no common code solution yet;
2. open-coded connectivity log for dc link detection, that maybe can be
   moved to drm (?);
3. open-coded parser that I suspect is a lot of duplicated code, but
   needs careful examining.

I suggest to address those points in a next phase for regression control.

[1] https://lore.kernel.org/amd-gfx/20250308142650.35920-1-mwen@igalia.com/

Let me know yours thoughts!

Melissa

Melissa Wen (11):
  drm/amd/display: make sure drm_edid stored in aconnector doesn't leak
  drm/amd/display: use drm_edid_product_id for parsing EDID product info
  drm/amd/display: parse display name from drm_eld
  drm/amd/display: get panel id with drm_edid helper
  drm/amd/display: get SAD from drm_eld when parsing EDID caps
  drm/amd/display: get SADB from drm_eld when parsing EDID caps
  drm/amd/display: simplify dm_helpers_parse_edid_caps signature
  drm/amd/display: change DC functions to accept private types for edid
  drm/edid: introduce a helper that compares edid data from two drm_edid
  drm/amd/display: add drm_edid to dc_sink
  drm/amd/display: move dc_sink from dc_edid to drm_edid

Rodrigo Siqueira (2):
  drm/amd/display: add a mid-layer file to handle EDID in DC
  drm/amd/display: create a function to fill dc_sink with edid data

 .../gpu/drm/amd/display/amdgpu_dm/Makefile    |   1 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  33 +++---
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 104 ++++++++----------
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  21 ++--
 .../gpu/drm/amd/display/amdgpu_dm/dc_edid.c   |  34 ++++++
 .../gpu/drm/amd/display/amdgpu_dm/dc_edid.h   |  15 +++
 .../drm/amd/display/dc/core/dc_link_exports.c |   9 +-
 drivers/gpu/drm/amd/display/dc/core/dc_sink.c |   3 +
 drivers/gpu/drm/amd/display/dc/dc.h           |  12 +-
 drivers/gpu/drm/amd/display/dc/dm_helpers.h   |   7 +-
 drivers/gpu/drm/amd/display/dc/inc/link.h     |   9 +-
 .../drm/amd/display/dc/link/link_detection.c  |  30 ++---
 .../drm/amd/display/dc/link/link_detection.h  |   9 +-
 drivers/gpu/drm/drm_edid.c                    |  18 +++
 include/drm/drm_edid.h                        |   2 +
 15 files changed, 165 insertions(+), 142 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h

-- 
2.47.2

[PATCH 01/13] drm/amd/display: make sure drm_edid stored in aconnector doesn't leak

[Melissa Wen]

Make sure the drm_edid container stored in aconnector is freed when
detroying the aconnector.

Fixes: 48edb2a4 ("drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid")
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 8665bd3cb75a..960bb8c62ffe 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7397,6 +7397,8 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
 		dc_sink_release(aconnector->dc_sink);
 	aconnector->dc_sink = NULL;
 
+	drm_edid_free(aconnector->drm_edid);
+
 	drm_dp_cec_unregister_connector(&aconnector->dm_dp_aux.aux);
 	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
-- 
2.47.2

Re: [PATCH 01/13] drm/amd/display: make sure drm_edid stored in aconnector doesn't leak

[Mario Limonciello]

On 4/11/2025 3:08 PM, Melissa Wen wrote:
> Make sure the drm_edid container stored in aconnector is freed when
> detroying the aconnector.

destroying

> 
> Fixes: 48edb2a4 ("drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid")
> Signed-off-by: Melissa Wen <mwen@igalia.com>

Minor nit above.  Add to next version:

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8665bd3cb75a..960bb8c62ffe 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7397,6 +7397,8 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
>   		dc_sink_release(aconnector->dc_sink);
>   	aconnector->dc_sink = NULL;
>   
> +	drm_edid_free(aconnector->drm_edid);
> +
>   	drm_dp_cec_unregister_connector(&aconnector->dm_dp_aux.aux);
>   	drm_connector_unregister(connector);
>   	drm_connector_cleanup(connector);

[PATCH 02/13] drm/amd/display: use drm_edid_product_id for parsing EDID product info

[Melissa Wen]

Since [1], we can use drm_edid_product_id to get debug info from
drm_edid instead of directly parsing EDID.

Link: https://lore.kernel.org/dri-devel/cover.1712655867.git.jani.nikula@intel.com/ [1]
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c    | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 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 62954b351ebd..e93adb7e48a5 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
@@ -108,6 +108,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	struct drm_connector *connector = &aconnector->base;
 	struct drm_device *dev = connector->dev;
 	struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
+	struct drm_edid *drm_edid;
+	struct drm_edid_product_id product_id;
 	struct cea_sad *sads;
 	int sad_count = -1;
 	int sadb_count = -1;
@@ -122,13 +124,13 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	if (!drm_edid_is_valid(edid_buf))
 		result = EDID_BAD_CHECKSUM;
 
-	edid_caps->manufacturer_id = (uint16_t) edid_buf->mfg_id[0] |
-					((uint16_t) edid_buf->mfg_id[1])<<8;
-	edid_caps->product_id = (uint16_t) edid_buf->prod_code[0] |
-					((uint16_t) edid_buf->prod_code[1])<<8;
-	edid_caps->serial_number = edid_buf->serial;
-	edid_caps->manufacture_week = edid_buf->mfg_week;
-	edid_caps->manufacture_year = edid_buf->mfg_year;
+	drm_edid_get_product_id(drm_edid, &product_id);
+
+	edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name);
+	edid_caps->product_id = le16_to_cpu(product_id.product_code);
+	edid_caps->serial_number = le32_to_cpu(product_id.serial_number);
+	edid_caps->manufacture_week = product_id.week_of_manufacture;
+	edid_caps->manufacture_year = product_id.year_of_manufacture;
 
 	drm_edid_get_monitor_name(edid_buf,
 				  edid_caps->display_name,
-- 
2.47.2

Re: [PATCH 02/13] drm/amd/display: use drm_edid_product_id for parsing EDID product info

[Michel Dänzer]

On 2025-04-11 22:08, Melissa Wen wrote:
> Since [1], we can use drm_edid_product_id to get debug info from
> drm_edid instead of directly parsing EDID.
> 
> Link: https://lore.kernel.org/dri-devel/cover.1712655867.git.jani.nikula@intel.com/ [1]
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c    | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 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 62954b351ebd..e93adb7e48a5 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
> [...]
> @@ -122,13 +124,13 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>  	if (!drm_edid_is_valid(edid_buf))
>  		result = EDID_BAD_CHECKSUM;
>  
> -	edid_caps->manufacturer_id = (uint16_t) edid_buf->mfg_id[0] |
> -					((uint16_t) edid_buf->mfg_id[1])<<8;
> -	edid_caps->product_id = (uint16_t) edid_buf->prod_code[0] |
> -					((uint16_t) edid_buf->prod_code[1])<<8;
> -	edid_caps->serial_number = edid_buf->serial;
> -	edid_caps->manufacture_week = edid_buf->mfg_week;
> -	edid_caps->manufacture_year = edid_buf->mfg_year;
> +	drm_edid_get_product_id(drm_edid, &product_id);
> +
> +	edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name);

struct drm_edid_product_id has

	__be16 manufacturer_name;

so shouldn't this use be16_to_cpu? (Though I see that would be a change in behaviour from the existing code...)


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast

Re: [PATCH 02/13] drm/amd/display: use drm_edid_product_id for parsing EDID product info

[Melissa Wen]

On 15/04/2025 06:32, Michel Dänzer wrote:
> On 2025-04-11 22:08, Melissa Wen wrote:
>> Since [1], we can use drm_edid_product_id to get debug info from
>> drm_edid instead of directly parsing EDID.
>>
>> Link: https://lore.kernel.org/dri-devel/cover.1712655867.git.jani.nikula@intel.com/ [1]
>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>> ---
>>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c    | 16 +++++++++-------
>>   1 file changed, 9 insertions(+), 7 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 62954b351ebd..e93adb7e48a5 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
>> [...]
>> @@ -122,13 +124,13 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>>   	if (!drm_edid_is_valid(edid_buf))
>>   		result = EDID_BAD_CHECKSUM;
>>   
>> -	edid_caps->manufacturer_id = (uint16_t) edid_buf->mfg_id[0] |
>> -					((uint16_t) edid_buf->mfg_id[1])<<8;
>> -	edid_caps->product_id = (uint16_t) edid_buf->prod_code[0] |
>> -					((uint16_t) edid_buf->prod_code[1])<<8;
>> -	edid_caps->serial_number = edid_buf->serial;
>> -	edid_caps->manufacture_week = edid_buf->mfg_week;
>> -	edid_caps->manufacture_year = edid_buf->mfg_year;
>> +	drm_edid_get_product_id(drm_edid, &product_id);
>> +
>> +	edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name);
> struct drm_edid_product_id has
>
> 	__be16 manufacturer_name;
>
> so shouldn't this use be16_to_cpu? (Though I see that would be a change in behaviour from the existing code...)
Hi Michel, thanks for reviewing this patch.

It changes the behaviour, yes. But as you pointed it out I realized I 
can just assign product_id.manufacturer_name directly.
It also noticed that I screwed up on rebasing and there is a patch 
missing here [1], let me fix all these things in the next version.

[1] https://lore.kernel.org/amd-gfx/20250308142650.35920-3-mwen@igalia.com

Melissa

>
>

Re: [PATCH 02/13] drm/amd/display: use drm_edid_product_id for parsing EDID product info

[Michel Dänzer]

On 2025-04-17 15:27, Melissa Wen wrote:
> On 15/04/2025 06:32, Michel Dänzer wrote:
>> On 2025-04-11 22:08, Melissa Wen wrote:
>>> Since [1], we can use drm_edid_product_id to get debug info from
>>> drm_edid instead of directly parsing EDID.
>>>
>>> Link: https://lore.kernel.org/dri-devel/cover.1712655867.git.jani.nikula@intel.com/ [1]
>>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>>> ---
>>>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c    | 16 +++++++++-------
>>>   1 file changed, 9 insertions(+), 7 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 62954b351ebd..e93adb7e48a5 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
>>> [...]
>>> @@ -122,13 +124,13 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>>>       if (!drm_edid_is_valid(edid_buf))
>>>           result = EDID_BAD_CHECKSUM;
>>>   -    edid_caps->manufacturer_id = (uint16_t) edid_buf->mfg_id[0] |
>>> -                    ((uint16_t) edid_buf->mfg_id[1])<<8;
>>> -    edid_caps->product_id = (uint16_t) edid_buf->prod_code[0] |
>>> -                    ((uint16_t) edid_buf->prod_code[1])<<8;
>>> -    edid_caps->serial_number = edid_buf->serial;
>>> -    edid_caps->manufacture_week = edid_buf->mfg_week;
>>> -    edid_caps->manufacture_year = edid_buf->mfg_year;
>>> +    drm_edid_get_product_id(drm_edid, &product_id);
>>> +
>>> +    edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name);
>> struct drm_edid_product_id has
>>
>>     __be16 manufacturer_name;
>>
>> so shouldn't this use be16_to_cpu? (Though I see that would be a change in behaviour from the existing code...)
> Hi Michel, thanks for reviewing this patch.
> 
> It changes the behaviour, yes. But as you pointed it out I realized I can just assign product_id.manufacturer_name directly.

That's a third option, with its own issues:

struct dc_edid_caps::manufacturer_id doesn't have any endianness annotation and is otherwise accessed directly, not via [bl]e16_to_cpu.

It's also assigned directly to struct audio_info::manufacture_id, which is programmed to the AZALIA_F0_CODEC_PIN_CONTROL_SINK_INFO0 register.

This means different behaviour (and specifically a different value being written to the register) on little vs big endian hosts. That can't be right.


P.S. Independently from the above, AFAIK sparse will complain if a field marked with __be16 isn't accessed via be16_to_cpu / cpu_to_be16.

-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast

[PATCH 03/13] drm/amd/display: parse display name from drm_eld

[Melissa Wen]

We don't need to parse dc_edid to get the display name since it's
already set in drm_eld which in turn had it values updated when updating
connector with the opaque drm_edid.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 10 ++++++----
 1 file changed, 6 insertions(+), 4 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 e93adb7e48a5..faea6b7fb3f3 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
@@ -34,7 +34,7 @@
 #include <drm/amdgpu_drm.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_fixed.h>
-
+#include <drm/drm_eld.h>
 #include "dm_services.h"
 #include "amdgpu.h"
 #include "dc.h"
@@ -90,6 +90,7 @@ static void apply_edid_quirks(struct drm_device *dev, struct edid *edid, struct
 	}
 }
 
+#define AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS 13
 /**
  * dm_helpers_parse_edid_caps() - Parse edid caps
  *
@@ -132,9 +133,10 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	edid_caps->manufacture_week = product_id.week_of_manufacture;
 	edid_caps->manufacture_year = product_id.year_of_manufacture;
 
-	drm_edid_get_monitor_name(edid_buf,
-				  edid_caps->display_name,
-				  AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS);
+	memset(edid_caps->display_name, 0, AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS);
+	memcpy(edid_caps->display_name,
+	       &connector->eld[DRM_ELD_MONITOR_NAME_STRING],
+	       AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS);
 
 	edid_caps->edid_hdmi = connector->display_info.is_hdmi;
 
-- 
2.47.2

Re: [PATCH 03/13] drm/amd/display: parse display name from drm_eld

[Jani Nikula]

On Fri, 11 Apr 2025, Melissa Wen <mwen@igalia.com> wrote:
> We don't need to parse dc_edid to get the display name since it's
> already set in drm_eld which in turn had it values updated when updating
> connector with the opaque drm_edid.
>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 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 e93adb7e48a5..faea6b7fb3f3 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
> @@ -34,7 +34,7 @@
>  #include <drm/amdgpu_drm.h>
>  #include <drm/drm_edid.h>
>  #include <drm/drm_fixed.h>
> -
> +#include <drm/drm_eld.h>
>  #include "dm_services.h"
>  #include "amdgpu.h"
>  #include "dc.h"
> @@ -90,6 +90,7 @@ static void apply_edid_quirks(struct drm_device *dev, struct edid *edid, struct
>  	}
>  }
>  
> +#define AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS 13
>  /**
>   * dm_helpers_parse_edid_caps() - Parse edid caps
>   *
> @@ -132,9 +133,10 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>  	edid_caps->manufacture_week = product_id.week_of_manufacture;
>  	edid_caps->manufacture_year = product_id.year_of_manufacture;
>  
> -	drm_edid_get_monitor_name(edid_buf,
> -				  edid_caps->display_name,
> -				  AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS);
> +	memset(edid_caps->display_name, 0, AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS);
> +	memcpy(edid_caps->display_name,
> +	       &connector->eld[DRM_ELD_MONITOR_NAME_STRING],
> +	       AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS);

It's not that simple. The monitor name in ELD is not fixed length (see
drm_eld_mnl()) and neither is it guaranteed to be NUL terminated.

BR,
Jani.


>  
>  	edid_caps->edid_hdmi = connector->display_info.is_hdmi;

-- 
Jani Nikula, Intel

Re: [PATCH 03/13] drm/amd/display: parse display name from drm_eld

[Melissa Wen]

On 14/04/2025 07:10, Jani Nikula wrote:
> On Fri, 11 Apr 2025, Melissa Wen <mwen@igalia.com> wrote:
>> We don't need to parse dc_edid to get the display name since it's
>> already set in drm_eld which in turn had it values updated when updating
>> connector with the opaque drm_edid.
>>
>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>> ---
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 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 e93adb7e48a5..faea6b7fb3f3 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
>> @@ -34,7 +34,7 @@
>>   #include <drm/amdgpu_drm.h>
>>   #include <drm/drm_edid.h>
>>   #include <drm/drm_fixed.h>
>> -
>> +#include <drm/drm_eld.h>
>>   #include "dm_services.h"
>>   #include "amdgpu.h"
>>   #include "dc.h"
>> @@ -90,6 +90,7 @@ static void apply_edid_quirks(struct drm_device *dev, struct edid *edid, struct
>>   	}
>>   }
>>   
>> +#define AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS 13
>>   /**
>>    * dm_helpers_parse_edid_caps() - Parse edid caps
>>    *
>> @@ -132,9 +133,10 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>>   	edid_caps->manufacture_week = product_id.week_of_manufacture;
>>   	edid_caps->manufacture_year = product_id.year_of_manufacture;
>>   
>> -	drm_edid_get_monitor_name(edid_buf,
>> -				  edid_caps->display_name,
>> -				  AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS);
>> +	memset(edid_caps->display_name, 0, AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS);
>> +	memcpy(edid_caps->display_name,
>> +	       &connector->eld[DRM_ELD_MONITOR_NAME_STRING],
>> +	       AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS);
> It's not that simple. The monitor name in ELD is not fixed length (see
> drm_eld_mnl()) and neither is it guaranteed to be NUL terminated.
I see. Thanks for raising these points.
I need to looking for a better solution then.

Melissa
>
> BR,
> Jani.
>
>
>>   
>>   	edid_caps->edid_hdmi = connector->display_info.is_hdmi;

[PATCH 04/13] drm/amd/display: get panel id with drm_edid helper

[Melissa Wen]

Instead of using driver-specific code, use DRM helpers.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 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 faea6b7fb3f3..620be159b8ef 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
@@ -48,16 +48,11 @@
 #include "ddc_service_types.h"
 #include "clk_mgr.h"
 
-static u32 edid_extract_panel_id(struct edid *edid)
+static void apply_edid_quirks(struct drm_device *dev,
+			      const struct drm_edid *drm_edid,
+			      struct dc_edid_caps *edid_caps)
 {
-	return (u32)edid->mfg_id[0] << 24   |
-	       (u32)edid->mfg_id[1] << 16   |
-	       (u32)EDID_PRODUCT_ID(edid);
-}
-
-static void apply_edid_quirks(struct drm_device *dev, struct edid *edid, struct dc_edid_caps *edid_caps)
-{
-	uint32_t panel_id = edid_extract_panel_id(edid);
+	uint32_t panel_id = drm_edid_get_panel_id(drm_edid);
 
 	switch (panel_id) {
 	/* Workaround for monitors that need a delay after detecting the link */
@@ -140,7 +135,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 
 	edid_caps->edid_hdmi = connector->display_info.is_hdmi;
 
-	apply_edid_quirks(dev, edid_buf, edid_caps);
+	apply_edid_quirks(dev, drm_edid, edid_caps);
 
 	sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
 	if (sad_count <= 0)
-- 
2.47.2

[PATCH 05/13] drm/amd/display: get SAD from drm_eld when parsing EDID caps

[Melissa Wen]

drm_edid_connector_update() updates display info, filling ELD with audio
info from Short-Audio Descriptors in the last step of
update_dislay_info(). Our goal is stopping using raw edid, so we can
extract SAD from drm_eld instead of access raw edid to get audio caps.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 32 +++++++++++--------
 1 file changed, 19 insertions(+), 13 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 620be159b8ef..ca9fe423e18e 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
@@ -104,11 +104,9 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	struct drm_connector *connector = &aconnector->base;
 	struct drm_device *dev = connector->dev;
 	struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
-	struct drm_edid *drm_edid;
+	const struct drm_edid *drm_edid;
 	struct drm_edid_product_id product_id;
-	struct cea_sad *sads;
-	int sad_count = -1;
-	int sadb_count = -1;
+	int sad_count, sadb_count;
 	int i = 0;
 	uint8_t *sadb = NULL;
 
@@ -117,9 +115,12 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	if (!edid_caps || !edid)
 		return EDID_BAD_INPUT;
 
-	if (!drm_edid_is_valid(edid_buf))
+	drm_edid = drm_edid_alloc(edid_buf, EDID_LENGTH * (edid_buf->extensions + 1));
+
+	if (!drm_edid_valid(drm_edid))
 		result = EDID_BAD_CHECKSUM;
 
+	drm_edid_connector_update(connector, drm_edid);
 	drm_edid_get_product_id(drm_edid, &product_id);
 
 	edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name);
@@ -137,18 +138,23 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 
 	apply_edid_quirks(dev, drm_edid, edid_caps);
 
-	sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
-	if (sad_count <= 0)
+	sad_count = drm_eld_sad_count(connector->eld);
+	if (sad_count <= 0) {
+		drm_edid_free(drm_edid);
 		return result;
+	}
 
 	edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
 	for (i = 0; i < edid_caps->audio_mode_count; ++i) {
-		struct cea_sad *sad = &sads[i];
+		struct cea_sad sad;
 
-		edid_caps->audio_modes[i].format_code = sad->format;
-		edid_caps->audio_modes[i].channel_count = sad->channels + 1;
-		edid_caps->audio_modes[i].sample_rate = sad->freq;
-		edid_caps->audio_modes[i].sample_size = sad->byte2;
+		if (drm_eld_sad_get(connector->eld, i, &sad) < 0)
+			continue;
+
+		edid_caps->audio_modes[i].format_code = sad.format;
+		edid_caps->audio_modes[i].channel_count = sad.channels + 1;
+		edid_caps->audio_modes[i].sample_rate = sad.freq;
+		edid_caps->audio_modes[i].sample_size = sad.byte2;
 	}
 
 	sadb_count = drm_edid_to_speaker_allocation((struct edid *) edid->raw_edid, &sadb);
@@ -163,8 +169,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	else
 		edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
 
-	kfree(sads);
 	kfree(sadb);
+	drm_edid_free(drm_edid);
 
 	return result;
 }
-- 
2.47.2

[PATCH 06/13] drm/amd/display: get SADB from drm_eld when parsing EDID caps

[Melissa Wen]

drm_edid_connector_update() updates display info, filling ELD with
speaker allocation data in the last step of update_dislay_info(). Our
goal is stopping using raw edid, so we can extract SADB from drm_eld
instead of access raw edid to get audio caps.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c    | 16 +++-------------
 1 file changed, 3 insertions(+), 13 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 ca9fe423e18e..a96f527a59df 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
@@ -106,10 +106,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
 	const struct drm_edid *drm_edid;
 	struct drm_edid_product_id product_id;
-	int sad_count, sadb_count;
+	int sad_count;
 	int i = 0;
-	uint8_t *sadb = NULL;
-
 	enum dc_edid_status result = EDID_OK;
 
 	if (!edid_caps || !edid)
@@ -157,19 +155,11 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 		edid_caps->audio_modes[i].sample_size = sad.byte2;
 	}
 
-	sadb_count = drm_edid_to_speaker_allocation((struct edid *) edid->raw_edid, &sadb);
-
-	if (sadb_count < 0) {
-		DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sadb_count);
-		sadb_count = 0;
-	}
-
-	if (sadb_count)
-		edid_caps->speaker_flags = sadb[0];
+	if (connector->eld[DRM_ELD_SPEAKER])
+		edid_caps->speaker_flags = connector->eld[DRM_ELD_SPEAKER];
 	else
 		edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
 
-	kfree(sadb);
 	drm_edid_free(drm_edid);
 
 	return result;
-- 
2.47.2

[PATCH 07/13] drm/amd/display: simplify dm_helpers_parse_edid_caps signature

[Melissa Wen]

Pass dc_sink to dm_helpers_parse_edid_caps(), since it already contains
edid info. It's a groundwork to get rid of raw edid stored as dc_edid.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  5 +----
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c   | 17 +++++++----------
 drivers/gpu/drm/amd/display/dc/dm_helpers.h     |  7 ++-----
 .../drm/amd/display/dc/link/link_detection.c    |  5 +----
 4 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 960bb8c62ffe..3cad6d9153f7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7529,10 +7529,7 @@ static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
 		memset(&dc_em_sink->edid_caps, 0, sizeof(struct dc_edid_caps));
 		memmove(dc_em_sink->dc_edid.raw_edid, edid,
 			(edid->extensions + 1) * EDID_LENGTH);
-		dm_helpers_parse_edid_caps(
-			dc_link,
-			&dc_em_sink->dc_edid,
-			&dc_em_sink->edid_caps);
+		dm_helpers_parse_edid_caps(dc_link, dc_em_sink);
 	}
 }
 
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 a96f527a59df..3082582c1579 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
@@ -95,22 +95,22 @@ static void apply_edid_quirks(struct drm_device *dev,
  *
  * Return: void
  */
-enum dc_edid_status dm_helpers_parse_edid_caps(
-		struct dc_link *link,
-		const struct dc_edid *edid,
-		struct dc_edid_caps *edid_caps)
+enum dc_edid_status dm_helpers_parse_edid_caps(struct dc_link *link,
+					       struct dc_sink *sink)
 {
 	struct amdgpu_dm_connector *aconnector = link->priv;
 	struct drm_connector *connector = &aconnector->base;
 	struct drm_device *dev = connector->dev;
-	struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
+	struct edid *edid_buf;
 	const struct drm_edid *drm_edid;
 	struct drm_edid_product_id product_id;
+	struct dc_edid_caps *edid_caps = &sink->edid_caps;
 	int sad_count;
 	int i = 0;
 	enum dc_edid_status result = EDID_OK;
 
-	if (!edid_caps || !edid)
+	edid_buf = (struct edid *) &sink->dc_edid.raw_edid;
+	if (!edid_caps || !edid_buf)
 		return EDID_BAD_INPUT;
 
 	drm_edid = drm_edid_alloc(edid_buf, EDID_LENGTH * (edid_buf->extensions + 1));
@@ -1030,10 +1030,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
 		/* We don't need the original edid anymore */
 		drm_edid_free(drm_edid);
 
-		edid_status = dm_helpers_parse_edid_caps(
-						link,
-						&sink->dc_edid,
-						&sink->edid_caps);
+		edid_status = dm_helpers_parse_edid_caps(link, sink);
 
 	} while (edid_status == EDID_BAD_CHECKSUM && --retry > 0);
 
diff --git a/drivers/gpu/drm/amd/display/dc/dm_helpers.h b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
index 9d160b39e8c5..ce6a70368bd0 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
@@ -59,11 +59,8 @@ void dm_helpers_free_gpu_mem(
 		enum dc_gpu_mem_alloc_type type,
 		void *pvMem);
 
-enum dc_edid_status dm_helpers_parse_edid_caps(
-	struct dc_link *link,
-	const struct dc_edid *edid,
-	struct dc_edid_caps *edid_caps);
-
+enum dc_edid_status dm_helpers_parse_edid_caps(struct dc_link *link,
+					       struct dc_sink *sink);
 
 /*
  * Update DP branch info
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index cc9191a5c9e6..8c7a00c1ad2b 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -1429,10 +1429,7 @@ struct dc_sink *link_add_remote_sink(
 			dc_sink))
 		goto fail_add_sink;
 
-	edid_status = dm_helpers_parse_edid_caps(
-			link,
-			&dc_sink->dc_edid,
-			&dc_sink->edid_caps);
+	edid_status = dm_helpers_parse_edid_caps(link, dc_sink);
 
 	/*
 	 * Treat device as no EDID device if EDID
-- 
2.47.2

Re: [PATCH 07/13] drm/amd/display: simplify dm_helpers_parse_edid_caps signature

[kernel test robot]

Hi Melissa,

kernel test robot noticed the following build warnings:

[auto build test WARNING on amd-pstate/linux-next]
[also build test WARNING on amd-pstate/bleeding-edge linus/master v6.15-rc2 next-20250415]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Melissa-Wen/drm-amd-display-make-sure-drm_edid-stored-in-aconnector-doesn-t-leak/20250414-132618
base:   https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git linux-next
patch link:    https://lore.kernel.org/r/20250411201333.151335-8-mwen%40igalia.com
patch subject: [PATCH 07/13] drm/amd/display: simplify dm_helpers_parse_edid_caps signature
config: powerpc-randconfig-003-20250415 (https://download.01.org/0day-ci/archive/20250415/202504151833.9WxWkFzM-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250415/202504151833.9WxWkFzM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504151833.9WxWkFzM-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:100: warning: Function parameter or struct member 'sink' not described in 'dm_helpers_parse_edid_caps'
>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:100: warning: Excess function parameter 'edid' description in 'dm_helpers_parse_edid_caps'
>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:100: warning: Excess function parameter 'edid_caps' description in 'dm_helpers_parse_edid_caps'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for HOTPLUG_CPU
   Depends on [n]: SMP [=y] && (PPC_PSERIES [=n] || PPC_PMAC [=n] || PPC_POWERNV [=n] || FSL_SOC_BOOKE [=n])
   Selected by [y]:
   - PM_SLEEP_SMP [=y] && SMP [=y] && (ARCH_SUSPEND_POSSIBLE [=y] || ARCH_HIBERNATION_POSSIBLE [=y]) && PM_SLEEP [=y]


vim +100 drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c

613a7956deb3b1 Aurabindo Pillai     2023-06-12   87  
027ec1819a19f7 Melissa Wen          2025-04-11   88  #define AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS 13
f0b60e6e9b2ba3 Srinivasan Shanmugam 2023-07-13   89  /**
f0b60e6e9b2ba3 Srinivasan Shanmugam 2023-07-13   90   * dm_helpers_parse_edid_caps() - Parse edid caps
4562236b3bc0a2 Harry Wentland       2017-09-12   91   *
f0b60e6e9b2ba3 Srinivasan Shanmugam 2023-07-13   92   * @link: current detected link
4562236b3bc0a2 Harry Wentland       2017-09-12   93   * @edid:	[in] pointer to edid
f0b60e6e9b2ba3 Srinivasan Shanmugam 2023-07-13   94   * @edid_caps:	[in] pointer to edid caps
f0b60e6e9b2ba3 Srinivasan Shanmugam 2023-07-13   95   *
f0b60e6e9b2ba3 Srinivasan Shanmugam 2023-07-13   96   * Return: void
f0b60e6e9b2ba3 Srinivasan Shanmugam 2023-07-13   97   */
cce130cc696979 Melissa Wen          2025-04-11   98  enum dc_edid_status dm_helpers_parse_edid_caps(struct dc_link *link,
cce130cc696979 Melissa Wen          2025-04-11   99  					       struct dc_sink *sink)
4562236b3bc0a2 Harry Wentland       2017-09-12 @100  {
3c021931023a30 Claudio Suarez       2021-10-17  101  	struct amdgpu_dm_connector *aconnector = link->priv;
3c021931023a30 Claudio Suarez       2021-10-17  102  	struct drm_connector *connector = &aconnector->base;
41b830476009f5 Aurabindo Pillai     2025-02-24  103  	struct drm_device *dev = connector->dev;
cce130cc696979 Melissa Wen          2025-04-11  104  	struct edid *edid_buf;
66c33284fbf286 Melissa Wen          2025-04-11  105  	const struct drm_edid *drm_edid;
23abb3407f98d7 Melissa Wen          2025-04-11  106  	struct drm_edid_product_id product_id;
cce130cc696979 Melissa Wen          2025-04-11  107  	struct dc_edid_caps *edid_caps = &sink->edid_caps;
8e9dfe0e0f1915 Melissa Wen          2025-04-11  108  	int sad_count;
4562236b3bc0a2 Harry Wentland       2017-09-12  109  	int i = 0;
4562236b3bc0a2 Harry Wentland       2017-09-12  110  	enum dc_edid_status result = EDID_OK;
4562236b3bc0a2 Harry Wentland       2017-09-12  111  
cce130cc696979 Melissa Wen          2025-04-11  112  	edid_buf = (struct edid *) &sink->dc_edid.raw_edid;
cce130cc696979 Melissa Wen          2025-04-11  113  	if (!edid_caps || !edid_buf)
4562236b3bc0a2 Harry Wentland       2017-09-12  114  		return EDID_BAD_INPUT;
4562236b3bc0a2 Harry Wentland       2017-09-12  115  
66c33284fbf286 Melissa Wen          2025-04-11  116  	drm_edid = drm_edid_alloc(edid_buf, EDID_LENGTH * (edid_buf->extensions + 1));
66c33284fbf286 Melissa Wen          2025-04-11  117  
66c33284fbf286 Melissa Wen          2025-04-11  118  	if (!drm_edid_valid(drm_edid))
4562236b3bc0a2 Harry Wentland       2017-09-12  119  		result = EDID_BAD_CHECKSUM;
4562236b3bc0a2 Harry Wentland       2017-09-12  120  
66c33284fbf286 Melissa Wen          2025-04-11  121  	drm_edid_connector_update(connector, drm_edid);
23abb3407f98d7 Melissa Wen          2025-04-11  122  	drm_edid_get_product_id(drm_edid, &product_id);
23abb3407f98d7 Melissa Wen          2025-04-11  123  
23abb3407f98d7 Melissa Wen          2025-04-11  124  	edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name);
23abb3407f98d7 Melissa Wen          2025-04-11  125  	edid_caps->product_id = le16_to_cpu(product_id.product_code);
23abb3407f98d7 Melissa Wen          2025-04-11  126  	edid_caps->serial_number = le32_to_cpu(product_id.serial_number);
23abb3407f98d7 Melissa Wen          2025-04-11  127  	edid_caps->manufacture_week = product_id.week_of_manufacture;
23abb3407f98d7 Melissa Wen          2025-04-11  128  	edid_caps->manufacture_year = product_id.year_of_manufacture;
4562236b3bc0a2 Harry Wentland       2017-09-12  129  
027ec1819a19f7 Melissa Wen          2025-04-11  130  	memset(edid_caps->display_name, 0, AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS);
027ec1819a19f7 Melissa Wen          2025-04-11  131  	memcpy(edid_caps->display_name,
027ec1819a19f7 Melissa Wen          2025-04-11  132  	       &connector->eld[DRM_ELD_MONITOR_NAME_STRING],
027ec1819a19f7 Melissa Wen          2025-04-11  133  	       AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS);
4562236b3bc0a2 Harry Wentland       2017-09-12  134  
3c021931023a30 Claudio Suarez       2021-10-17  135  	edid_caps->edid_hdmi = connector->display_info.is_hdmi;
4562236b3bc0a2 Harry Wentland       2017-09-12  136  
24baf0ebdb9d17 Melissa Wen          2025-04-11  137  	apply_edid_quirks(dev, drm_edid, edid_caps);
b7cdccc6a84956 Ryan Lin             2024-02-28  138  
66c33284fbf286 Melissa Wen          2025-04-11  139  	sad_count = drm_eld_sad_count(connector->eld);
66c33284fbf286 Melissa Wen          2025-04-11  140  	if (sad_count <= 0) {
66c33284fbf286 Melissa Wen          2025-04-11  141  		drm_edid_free(drm_edid);
4562236b3bc0a2 Harry Wentland       2017-09-12  142  		return result;
66c33284fbf286 Melissa Wen          2025-04-11  143  	}
4562236b3bc0a2 Harry Wentland       2017-09-12  144  
1347b15d5e8e16 Srinivasan Shanmugam 2023-08-13  145  	edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
4562236b3bc0a2 Harry Wentland       2017-09-12  146  	for (i = 0; i < edid_caps->audio_mode_count; ++i) {
66c33284fbf286 Melissa Wen          2025-04-11  147  		struct cea_sad sad;
66c33284fbf286 Melissa Wen          2025-04-11  148  
66c33284fbf286 Melissa Wen          2025-04-11  149  		if (drm_eld_sad_get(connector->eld, i, &sad) < 0)
66c33284fbf286 Melissa Wen          2025-04-11  150  			continue;
4562236b3bc0a2 Harry Wentland       2017-09-12  151  
66c33284fbf286 Melissa Wen          2025-04-11  152  		edid_caps->audio_modes[i].format_code = sad.format;
66c33284fbf286 Melissa Wen          2025-04-11  153  		edid_caps->audio_modes[i].channel_count = sad.channels + 1;
66c33284fbf286 Melissa Wen          2025-04-11  154  		edid_caps->audio_modes[i].sample_rate = sad.freq;
66c33284fbf286 Melissa Wen          2025-04-11  155  		edid_caps->audio_modes[i].sample_size = sad.byte2;
4562236b3bc0a2 Harry Wentland       2017-09-12  156  	}
4562236b3bc0a2 Harry Wentland       2017-09-12  157  
8e9dfe0e0f1915 Melissa Wen          2025-04-11  158  	if (connector->eld[DRM_ELD_SPEAKER])
8e9dfe0e0f1915 Melissa Wen          2025-04-11  159  		edid_caps->speaker_flags = connector->eld[DRM_ELD_SPEAKER];
4562236b3bc0a2 Harry Wentland       2017-09-12  160  	else
4562236b3bc0a2 Harry Wentland       2017-09-12  161  		edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
4562236b3bc0a2 Harry Wentland       2017-09-12  162  
66c33284fbf286 Melissa Wen          2025-04-11  163  	drm_edid_free(drm_edid);
4562236b3bc0a2 Harry Wentland       2017-09-12  164  
4562236b3bc0a2 Harry Wentland       2017-09-12  165  	return result;
4562236b3bc0a2 Harry Wentland       2017-09-12  166  }
4562236b3bc0a2 Harry Wentland       2017-09-12  167  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 08/13] drm/amd/display: change DC functions to accept private types for edid

[Melissa Wen]

There is an opaque obj in Linux/DRM to encapsulate edid data as
`drm_edid`. This obj isn't present in other platforms but we need to
pass it through DC when adding sink. To pass this data without
compromise the independence of DC code, make some DC functions accept
edid data as private options.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c | 9 ++++-----
 drivers/gpu/drm/amd/display/dc/dc.h                   | 9 ++++-----
 drivers/gpu/drm/amd/display/dc/inc/link.h             | 9 ++++-----
 drivers/gpu/drm/amd/display/dc/link/link_detection.c  | 4 ++--
 drivers/gpu/drm/amd/display/dc/link/link_detection.h  | 9 ++++-----
 5 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
index 71e15da4bb69..b6f03ac16cad 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
@@ -278,11 +278,10 @@ unsigned int dc_dp_trace_get_link_loss_count(struct dc_link *link)
 	return link->dc->link_srv->dp_trace_get_link_loss_count(link);
 }
 
-struct dc_sink *dc_link_add_remote_sink(
-		struct dc_link *link,
-		const uint8_t *edid,
-		int len,
-		struct dc_sink_init_data *init_data)
+struct dc_sink *dc_link_add_remote_sink(struct dc_link *link,
+					const void *edid,
+					int len,
+					struct dc_sink_init_data *init_data)
 {
 	return link->dc->link_srv->add_remote_sink(link, edid, len, init_data);
 }
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 7624b909497e..d60cd9d23848 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1875,11 +1875,10 @@ struct dc_sink_init_data;
  * @len - size of the edid in byte
  * @init_data -
  */
-struct dc_sink *dc_link_add_remote_sink(
-		struct dc_link *dc_link,
-		const uint8_t *edid,
-		int len,
-		struct dc_sink_init_data *init_data);
+struct dc_sink *dc_link_add_remote_sink(struct dc_link *dc_link,
+					const void *edid,
+					int len,
+					struct dc_sink_init_data *init_data);
 
 /* Remove remote sink from a link with dc_connection_mst_branch connection type.
  * @link - link the sink should be removed from
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link.h b/drivers/gpu/drm/amd/display/dc/inc/link.h
index 2948a696ee12..ab69af34ec82 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/link.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/link.h
@@ -107,11 +107,10 @@ struct link_service {
 	bool (*detect_link)(struct dc_link *link, enum dc_detect_reason reason);
 	bool (*detect_connection_type)(struct dc_link *link,
 			enum dc_connection_type *type);
-	struct dc_sink *(*add_remote_sink)(
-			struct dc_link *link,
-			const uint8_t *edid,
-			int len,
-			struct dc_sink_init_data *init_data);
+	struct dc_sink *(*add_remote_sink)(struct dc_link *link,
+					   const void *edid,
+					   int len,
+					   struct dc_sink_init_data *init_data);
 	void (*remove_remote_sink)(struct dc_link *link, struct dc_sink *sink);
 	bool (*get_hpd_state)(struct dc_link *link);
 	struct gpio *(*get_hpd_gpio)(struct dc_bios *dcb,
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index 8c7a00c1ad2b..6d05ddb194c9 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -1394,7 +1394,7 @@ static bool link_add_remote_sink_helper(struct dc_link *dc_link, struct dc_sink
 
 struct dc_sink *link_add_remote_sink(
 		struct dc_link *link,
-		const uint8_t *edid,
+		const void *edid,
 		int len,
 		struct dc_sink_init_data *init_data)
 {
@@ -1421,7 +1421,7 @@ struct dc_sink *link_add_remote_sink(
 	if (!dc_sink)
 		return NULL;
 
-	memmove(dc_sink->dc_edid.raw_edid, edid, len);
+	memmove(dc_sink->dc_edid.raw_edid, (const uint8_t *) edid, len);
 	dc_sink->dc_edid.length = len;
 
 	if (!link_add_remote_sink_helper(
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.h b/drivers/gpu/drm/amd/display/dc/link/link_detection.h
index 7da05078721e..9cd3aa36c7d8 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.h
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.h
@@ -29,11 +29,10 @@
 bool link_detect(struct dc_link *link, enum dc_detect_reason reason);
 bool link_detect_connection_type(struct dc_link *link,
 		enum dc_connection_type *type);
-struct dc_sink *link_add_remote_sink(
-		struct dc_link *link,
-		const uint8_t *edid,
-		int len,
-		struct dc_sink_init_data *init_data);
+struct dc_sink *link_add_remote_sink(struct dc_link *link,
+				     const void *edid,
+				     int len,
+				     struct dc_sink_init_data *init_data);
 void link_remove_remote_sink(struct dc_link *link, struct dc_sink *sink);
 bool link_reset_cur_dp_mst_topology(struct dc_link *link);
 const struct dc_link_status *link_get_status(const struct dc_link *link);
-- 
2.47.2

[PATCH 09/13] drm/amd/display: add a mid-layer file to handle EDID in DC

[Melissa Wen]

From: Rodrigo Siqueira <siqueira@igalia.com>

Since DC is a shared code, this commit introduces a new file to work as
a mid-layer in DC for the edid manipulation.

Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
Co-developed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/Makefile    |  1 +
 .../gpu/drm/amd/display/amdgpu_dm/dc_edid.c   | 19 +++++++++++++++++++
 .../gpu/drm/amd/display/amdgpu_dm/dc_edid.h   | 11 +++++++++++
 .../drm/amd/display/dc/link/link_detection.c  | 17 +++--------------
 4 files changed, 34 insertions(+), 14 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
index ab2a97e354da..30188bf75724 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
@@ -38,6 +38,7 @@ AMDGPUDM = \
 	amdgpu_dm_pp_smu.o \
 	amdgpu_dm_psr.o \
 	amdgpu_dm_replay.o \
+	dc_edid.o \
 	amdgpu_dm_wb.o
 
 ifdef CONFIG_DRM_AMD_DC_FP
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
new file mode 100644
index 000000000000..fab873b091f5
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+#include "amdgpu_dm/dc_edid.h"
+#include "dc.h"
+
+bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
+			  struct dc_sink *current_sink)
+{
+	struct dc_edid *old_edid = &prev_sink->dc_edid;
+	struct dc_edid *new_edid = &current_sink->dc_edid;
+
+       if (old_edid->length != new_edid->length)
+               return false;
+
+       if (new_edid->length == 0)
+               return false;
+
+       return (memcmp(old_edid->raw_edid,
+                      new_edid->raw_edid, new_edid->length) == 0);
+}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
new file mode 100644
index 000000000000..7e3b1177bc8a
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef __DC_EDID_H__
+#define __DC_EDID_H__
+
+#include "dc.h"
+
+bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
+			  struct dc_sink *current_sink);
+
+#endif /* __DC_EDID_H__ */
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index 6d05ddb194c9..e748721f31e4 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -48,6 +48,8 @@
 #include "dm_helpers.h"
 #include "clk_mgr.h"
 
+#include "dc_edid.h"
+
  // Offset DPCD 050Eh == 0x5A
 #define MST_HUB_ID_0x5A  0x5A
 
@@ -616,18 +618,6 @@ static bool detect_dp(struct dc_link *link,
 	return true;
 }
 
-static bool is_same_edid(struct dc_edid *old_edid, struct dc_edid *new_edid)
-{
-	if (old_edid->length != new_edid->length)
-		return false;
-
-	if (new_edid->length == 0)
-		return false;
-
-	return (memcmp(old_edid->raw_edid,
-		       new_edid->raw_edid, new_edid->length) == 0);
-}
-
 static bool wait_for_entering_dp_alt_mode(struct dc_link *link)
 {
 
@@ -1114,8 +1104,7 @@ static bool detect_link_and_local_sink(struct dc_link *link,
 		// Check if edid is the same
 		if ((prev_sink) &&
 		    (edid_status == EDID_THE_SAME || edid_status == EDID_OK))
-			same_edid = is_same_edid(&prev_sink->dc_edid,
-						 &sink->dc_edid);
+			same_edid = dc_edid_is_same_edid(prev_sink, sink);
 
 		if (sink->edid_caps.panel_patch.skip_scdc_overwrite)
 			link->ctx->dc->debug.hdmi20_disable = true;
-- 
2.47.2

[PATCH 10/13] drm/amd/display: create a function to fill dc_sink with edid data

[Melissa Wen]

From: Rodrigo Siqueira <siqueira@igalia.com>

As part of the effort of stopping using raw edid, this commit move the
copy of the edid in DC to a dedicated function that will allow the usage
of drm_edid in the next steps.

Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
Co-developer--by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c      | 8 ++++++++
 drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h      | 2 ++
 drivers/gpu/drm/amd/display/dc/link/link_detection.c | 3 +--
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
index fab873b091f5..39fcaa16a14a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
@@ -17,3 +17,11 @@ bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
        return (memcmp(old_edid->raw_edid,
                       new_edid->raw_edid, new_edid->length) == 0);
 }
+
+void dc_edid_copy_edid_to_dc(struct dc_sink *dc_sink,
+			     const void *edid,
+			     int len)
+{
+	memmove(dc_sink->dc_edid.raw_edid, (const uint8_t *) edid, len);
+	dc_sink->dc_edid.length = len;
+}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
index 7e3b1177bc8a..f42cd5bbc730 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
@@ -7,5 +7,7 @@
 
 bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
 			  struct dc_sink *current_sink);
+void dc_edid_copy_edid_to_dc(struct dc_sink *dc_sink,
+			     const void *edid, int len);
 
 #endif /* __DC_EDID_H__ */
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index e748721f31e4..978d2b4a4d29 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -1410,8 +1410,7 @@ struct dc_sink *link_add_remote_sink(
 	if (!dc_sink)
 		return NULL;
 
-	memmove(dc_sink->dc_edid.raw_edid, (const uint8_t *) edid, len);
-	dc_sink->dc_edid.length = len;
+	dc_edid_copy_edid_to_dc(dc_sink, edid, len);
 
 	if (!link_add_remote_sink_helper(
 			link,
-- 
2.47.2

[PATCH 11/13] drm/edid: introduce a helper that compares edid data from two drm_edid

[Melissa Wen]

AMD driver has a function used to compare if two edid are the same; this
is useful to some of the link detection algorithms implemented by
amdgpu. Since the amdgpu function can be helpful for other drivers, this
commit abstracts the AMD function to make it available at the DRM level
by wrapping existent drm_edid_eq().

Co-developed-by: Rodrigo Siqueira <siqueira@igalia.com>
Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++
 include/drm/drm_edid.h     |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 855beafb76ff..328a25d198e5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -7502,3 +7502,21 @@ bool drm_edid_is_digital(const struct drm_edid *drm_edid)
 		drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
 }
 EXPORT_SYMBOL(drm_edid_is_digital);
+
+/**
+ * drm_edid_is_edid_eq - Check if it the EDID is equal
+ *
+ * @drm_edid_old: old drm_edid to compare edid
+ * @drm_edid_new: new drm_edid to compare edid
+ *
+ * Return true if the EDID is equal
+ */
+bool drm_edid_is_edid_eq(const struct drm_edid *drm_edid_old,
+			 const struct drm_edid *drm_edid_new)
+{
+	const void *old_edid = drm_edid_old->edid;
+	size_t old_edid_size = drm_edid_old->size;
+
+	return drm_edid_eq(drm_edid_new, old_edid, old_edid_size);
+}
+EXPORT_SYMBOL(drm_edid_is_edid_eq);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index eaac5e665892..0e062761296c 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -469,6 +469,8 @@ int drm_edid_connector_update(struct drm_connector *connector,
 			      const struct drm_edid *edid);
 int drm_edid_connector_add_modes(struct drm_connector *connector);
 bool drm_edid_is_digital(const struct drm_edid *drm_edid);
+bool drm_edid_is_edid_eq(const struct drm_edid *drm_edid_first,
+			 const struct drm_edid *drm_edid_second);
 void drm_edid_get_product_id(const struct drm_edid *drm_edid,
 			     struct drm_edid_product_id *id);
 void drm_edid_print_product_id(struct drm_printer *p,
-- 
2.47.2

Re: [PATCH 11/13] drm/edid: introduce a helper that compares edid data from two drm_edid

[Jani Nikula]

On Fri, 11 Apr 2025, Melissa Wen <mwen@igalia.com> wrote:
> AMD driver has a function used to compare if two edid are the same; this
> is useful to some of the link detection algorithms implemented by
> amdgpu. Since the amdgpu function can be helpful for other drivers, this
> commit abstracts the AMD function to make it available at the DRM level
> by wrapping existent drm_edid_eq().
>
> Co-developed-by: Rodrigo Siqueira <siqueira@igalia.com>
> Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++
>  include/drm/drm_edid.h     |  2 ++
>  2 files changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 855beafb76ff..328a25d198e5 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -7502,3 +7502,21 @@ bool drm_edid_is_digital(const struct drm_edid *drm_edid)
>  		drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
>  }
>  EXPORT_SYMBOL(drm_edid_is_digital);
> +
> +/**
> + * drm_edid_is_edid_eq - Check if it the EDID is equal

I think drm_edid_eq() is the better name. Please rename the static one
to make room. Maybe make it drm_edid_eq_buf() or something, because
that's what it really does.

"Check if the EDIDs are equal"

> + *
> + * @drm_edid_old: old drm_edid to compare edid
> + * @drm_edid_new: new drm_edid to compare edid

Old and new are meaningless here. It's supposed to be a generic
function. Just a/b or edid1/edid2 or something.

> + *
> + * Return true if the EDID is equal

"Return true if the EDIDs are equal."

> + */
> +bool drm_edid_is_edid_eq(const struct drm_edid *drm_edid_old,
> +			 const struct drm_edid *drm_edid_new)
> +{
> +	const void *old_edid = drm_edid_old->edid;
> +	size_t old_edid_size = drm_edid_old->size;

The existing drm_edid_eq() function supports the use case of either or
both EDIDs being NULL, and returning true for two NULL EDIDs. This one
oopses if the "old" EDID is NULL.

I'm not sure you can trivially replicate that behaviour by reusing the
existing function, though.

> +
> +	return drm_edid_eq(drm_edid_new, old_edid, old_edid_size);
> +}
> +EXPORT_SYMBOL(drm_edid_is_edid_eq);
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index eaac5e665892..0e062761296c 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -469,6 +469,8 @@ int drm_edid_connector_update(struct drm_connector *connector,
>  			      const struct drm_edid *edid);
>  int drm_edid_connector_add_modes(struct drm_connector *connector);
>  bool drm_edid_is_digital(const struct drm_edid *drm_edid);
> +bool drm_edid_is_edid_eq(const struct drm_edid *drm_edid_first,
> +			 const struct drm_edid *drm_edid_second);
>  void drm_edid_get_product_id(const struct drm_edid *drm_edid,
>  			     struct drm_edid_product_id *id);
>  void drm_edid_print_product_id(struct drm_printer *p,

-- 
Jani Nikula, Intel

Re: [PATCH 11/13] drm/edid: introduce a helper that compares edid data from two drm_edid

[Melissa Wen]

On 14/04/2025 07:06, Jani Nikula wrote:
> On Fri, 11 Apr 2025, Melissa Wen <mwen@igalia.com> wrote:
>> AMD driver has a function used to compare if two edid are the same; this
>> is useful to some of the link detection algorithms implemented by
>> amdgpu. Since the amdgpu function can be helpful for other drivers, this
>> commit abstracts the AMD function to make it available at the DRM level
>> by wrapping existent drm_edid_eq().
>>
>> Co-developed-by: Rodrigo Siqueira <siqueira@igalia.com>
>> Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++
>>   include/drm/drm_edid.h     |  2 ++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 855beafb76ff..328a25d198e5 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -7502,3 +7502,21 @@ bool drm_edid_is_digital(const struct drm_edid *drm_edid)
>>   		drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
>>   }
>>   EXPORT_SYMBOL(drm_edid_is_digital);
>> +
>> +/**
>> + * drm_edid_is_edid_eq - Check if it the EDID is equal
Hi Jani,
> I think drm_edid_eq() is the better name. Please rename the static one
> to make room. Maybe make it drm_edid_eq_buf() or something, because
> that's what it really does.
right
> "Check if the EDIDs are equal"
>
>> + *
>> + * @drm_edid_old: old drm_edid to compare edid
>> + * @drm_edid_new: new drm_edid to compare edid
> Old and new are meaningless here. It's supposed to be a generic
> function. Just a/b or edid1/edid2 or something.
ack
>> + *
>> + * Return true if the EDID is equal
> "Return true if the EDIDs are equal."
ack
>
>> + */
>> +bool drm_edid_is_edid_eq(const struct drm_edid *drm_edid_old,
>> +			 const struct drm_edid *drm_edid_new)
>> +{
>> +	const void *old_edid = drm_edid_old->edid;
>> +	size_t old_edid_size = drm_edid_old->size;
> The existing drm_edid_eq() function supports the use case of either or
> both EDIDs being NULL, and returning true for two NULL EDIDs. This one
> oopses if the "old" EDID is NULL.
>
> I'm not sure you can trivially replicate that behaviour by reusing the
> existing function, though.
Right.

I think I can replicate this behavior by doing:

         const void *old_edid = drm_edid ? drm_edid_old->edid : NULL;
         size_t old_edid_size = drm_edid ? drm_edid_old->size : 0;

         return drm_edid_eq(drm_edid_new, old_edid, old_edid_size);

WDTY?

Thanks for reviewing.

Melissa

>
>> +
>> +	return drm_edid_eq(drm_edid_new, old_edid, old_edid_size);
>> +}
>> +EXPORT_SYMBOL(drm_edid_is_edid_eq);
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index eaac5e665892..0e062761296c 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -469,6 +469,8 @@ int drm_edid_connector_update(struct drm_connector *connector,
>>   			      const struct drm_edid *edid);
>>   int drm_edid_connector_add_modes(struct drm_connector *connector);
>>   bool drm_edid_is_digital(const struct drm_edid *drm_edid);
>> +bool drm_edid_is_edid_eq(const struct drm_edid *drm_edid_first,
>> +			 const struct drm_edid *drm_edid_second);
>>   void drm_edid_get_product_id(const struct drm_edid *drm_edid,
>>   			     struct drm_edid_product_id *id);
>>   void drm_edid_print_product_id(struct drm_printer *p,

[PATCH 12/13] drm/amd/display: add drm_edid to dc_sink

[Melissa Wen]

Add Linux opaque object to dc_sink for storing edid data cross driver,
drm_edid. Also include the Linux call to free this object, the
drm_edid_free()

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c | 7 +++++++
 drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h | 1 +
 drivers/gpu/drm/amd/display/dc/core/dc_sink.c   | 3 +++
 drivers/gpu/drm/amd/display/dc/dc.h             | 3 +++
 4 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
index 39fcaa16a14a..fa0f0e61f05d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 #include "amdgpu_dm/dc_edid.h"
 #include "dc.h"
+#include <drm/drm_edid.h>
 
 bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
 			  struct dc_sink *current_sink)
@@ -25,3 +26,9 @@ void dc_edid_copy_edid_to_dc(struct dc_sink *dc_sink,
 	memmove(dc_sink->dc_edid.raw_edid, (const uint8_t *) edid, len);
 	dc_sink->dc_edid.length = len;
 }
+
+
+void dc_edid_sink_edid_free(struct dc_sink *sink)
+{
+	drm_edid_free(sink->drm_edid);
+}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
index f42cd5bbc730..2c76768be459 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
@@ -9,5 +9,6 @@ bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
 			  struct dc_sink *current_sink);
 void dc_edid_copy_edid_to_dc(struct dc_sink *dc_sink,
 			     const void *edid, int len);
+void dc_edid_sink_edid_free(struct dc_sink *sink);
 
 #endif /* __DC_EDID_H__ */
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_sink.c b/drivers/gpu/drm/amd/display/dc/core/dc_sink.c
index 455fa5dd1420..3774a3245506 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_sink.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_sink.c
@@ -26,6 +26,7 @@
 #include "dm_services.h"
 #include "dm_helpers.h"
 #include "core_types.h"
+#include "dc_edid.h"
 
 /*******************************************************************************
  * Private functions
@@ -65,6 +66,8 @@ void dc_sink_retain(struct dc_sink *sink)
 static void dc_sink_free(struct kref *kref)
 {
 	struct dc_sink *sink = container_of(kref, struct dc_sink, refcount);
+
+	dc_edid_sink_edid_free(sink);
 	kfree(sink->dc_container_id);
 	kfree(sink);
 }
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index d60cd9d23848..0d4e6ea1821b 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -46,6 +46,8 @@
 
 #include "dmub/inc/dmub_cmd.h"
 
+#include <drm/drm_edid.h>
+
 struct abm_save_restore;
 
 /* forward declaration */
@@ -2430,6 +2432,7 @@ struct scdc_caps {
 struct dc_sink {
 	enum signal_type sink_signal;
 	struct dc_edid dc_edid; /* raw edid */
+	const struct drm_edid *drm_edid; /* Linux DRM edid*/
 	struct dc_edid_caps edid_caps; /* parse display caps */
 	struct dc_container_id *dc_container_id;
 	uint32_t dongle_max_pix_clk;
-- 
2.47.2

[PATCH 13/13] drm/amd/display: move dc_sink from dc_edid to drm_edid

[Melissa Wen]

Reduce direct handling of edid data by resorting to drm helpers that
deal with this info inside drm_edid infrastructure.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 26 +++++++------------
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 24 +++++------------
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 21 +++++----------
 .../gpu/drm/amd/display/amdgpu_dm/dc_edid.c   | 26 +++++++++----------
 .../gpu/drm/amd/display/amdgpu_dm/dc_edid.h   |  1 +
 .../drm/amd/display/dc/link/link_detection.c  |  3 ++-
 6 files changed, 40 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 3cad6d9153f7..3598f0091551 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -68,6 +68,7 @@
 #endif
 #include "amdgpu_dm_psr.h"
 #include "amdgpu_dm_replay.h"
+#include "dc_edid.h"
 
 #include "ivsrcid/ivsrcid_vislands30.h"
 
@@ -3862,6 +3863,8 @@ void amdgpu_dm_update_connector_after_detect(
 	 * 2. Send an event and let userspace tell us what to do
 	 */
 	if (sink) {
+		const struct drm_edid *drm_edid = sink->drm_edid;
+
 		/*
 		 * TODO: check if we still need the S3 mode update workaround.
 		 * If yes, put it here.
@@ -3873,16 +3876,15 @@ void amdgpu_dm_update_connector_after_detect(
 
 		aconnector->dc_sink = sink;
 		dc_sink_retain(aconnector->dc_sink);
-		if (sink->dc_edid.length == 0) {
+
+		if (!drm_edid_valid(drm_edid)) {
 			aconnector->drm_edid = NULL;
 			hdmi_cec_unset_edid(aconnector);
 			if (aconnector->dc_link->aux_mode) {
 				drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux);
 			}
 		} else {
-			const struct edid *edid = (const struct edid *)sink->dc_edid.raw_edid;
-
-			aconnector->drm_edid = drm_edid_alloc(edid, sink->dc_edid.length);
+			aconnector->drm_edid = drm_edid_dup(sink->drm_edid);
 			drm_edid_connector_update(connector, aconnector->drm_edid);
 
 			hdmi_cec_set_edid(aconnector);
@@ -7523,12 +7525,8 @@ static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
 	aconnector->drm_edid = drm_edid;
 	/* Update emulated (virtual) sink's EDID */
 	if (dc_em_sink && dc_link) {
-		// FIXME: Get rid of drm_edid_raw()
-		const struct edid *edid = drm_edid_raw(drm_edid);
-
 		memset(&dc_em_sink->edid_caps, 0, sizeof(struct dc_edid_caps));
-		memmove(dc_em_sink->dc_edid.raw_edid, edid,
-			(edid->extensions + 1) * EDID_LENGTH);
+		dc_edid_copy_edid_to_dc(dc_em_sink, drm_edid, 0);
 		dm_helpers_parse_edid_caps(dc_link, dc_em_sink);
 	}
 }
@@ -7561,7 +7559,6 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
 			.sink_signal = SIGNAL_TYPE_VIRTUAL
 	};
 	const struct drm_edid *drm_edid;
-	const struct edid *edid;
 	struct i2c_adapter *ddc;
 
 	if (dc_link && dc_link->aux_mode)
@@ -7581,12 +7578,9 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
 
 	aconnector->drm_edid = drm_edid;
 
-	edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
-	aconnector->dc_em_sink = dc_link_add_remote_sink(
-		aconnector->dc_link,
-		(uint8_t *)edid,
-		(edid->extensions + 1) * EDID_LENGTH,
-		&init_params);
+	aconnector->dc_em_sink = dc_link_add_remote_sink(aconnector->dc_link,
+							 drm_edid, 0,
+							 &init_params);
 
 	if (aconnector->base.force == DRM_FORCE_ON) {
 		aconnector->dc_sink = aconnector->dc_link->local_sink ?
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 3082582c1579..c56c1e36f539 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
@@ -47,6 +47,7 @@
 #include "dm_helpers.h"
 #include "ddc_service_types.h"
 #include "clk_mgr.h"
+#include "dc_edid.h"
 
 static void apply_edid_quirks(struct drm_device *dev,
 			      const struct drm_edid *drm_edid,
@@ -101,20 +102,16 @@ enum dc_edid_status dm_helpers_parse_edid_caps(struct dc_link *link,
 	struct amdgpu_dm_connector *aconnector = link->priv;
 	struct drm_connector *connector = &aconnector->base;
 	struct drm_device *dev = connector->dev;
-	struct edid *edid_buf;
-	const struct drm_edid *drm_edid;
+	const struct drm_edid *drm_edid = sink->drm_edid;
 	struct drm_edid_product_id product_id;
 	struct dc_edid_caps *edid_caps = &sink->edid_caps;
 	int sad_count;
 	int i = 0;
 	enum dc_edid_status result = EDID_OK;
 
-	edid_buf = (struct edid *) &sink->dc_edid.raw_edid;
-	if (!edid_caps || !edid_buf)
+	if (!edid_caps || !drm_edid)
 		return EDID_BAD_INPUT;
 
-	drm_edid = drm_edid_alloc(edid_buf, EDID_LENGTH * (edid_buf->extensions + 1));
-
 	if (!drm_edid_valid(drm_edid))
 		result = EDID_BAD_CHECKSUM;
 
@@ -137,10 +134,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps(struct dc_link *link,
 	apply_edid_quirks(dev, drm_edid, edid_caps);
 
 	sad_count = drm_eld_sad_count(connector->eld);
-	if (sad_count <= 0) {
-		drm_edid_free(drm_edid);
+	if (sad_count <= 0)
 		return result;
-	}
 
 	edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
 	for (i = 0; i < edid_caps->audio_mode_count; ++i) {
@@ -160,8 +155,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(struct dc_link *link,
 	else
 		edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
 
-	drm_edid_free(drm_edid);
-
 	return result;
 }
 
@@ -993,7 +986,6 @@ enum dc_edid_status dm_helpers_read_local_edid(
 	int retry = 3;
 	enum dc_edid_status edid_status;
 	const struct drm_edid *drm_edid;
-	const struct edid *edid;
 
 	if (link->aux_mode)
 		ddc = &aconnector->dm_dp_aux.aux.ddc;
@@ -1023,11 +1015,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
 		if (!drm_edid)
 			return EDID_NO_RESPONSE;
 
-		edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
-		sink->dc_edid.length = EDID_LENGTH * (edid->extensions + 1);
-		memmove(sink->dc_edid.raw_edid, (uint8_t *)edid, sink->dc_edid.length);
-
-		/* We don't need the original edid anymore */
+		sink->drm_edid = drm_edid_dup(drm_edid);
 		drm_edid_free(drm_edid);
 
 		edid_status = dm_helpers_parse_edid_caps(link, sink);
@@ -1053,6 +1041,8 @@ enum dc_edid_status dm_helpers_read_local_edid(
 
 		test_response.bits.EDID_CHECKSUM_WRITE = 1;
 
+		// TODO: drm_edid doesn't have a helper for dp_write_dpcd yet
+		dc_edid_copy_edid_to_sink(sink);
 		dm_helpers_dp_write_dpcd(ctx,
 					link,
 					DP_TEST_EDID_CHECKSUM,
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 075e8a5be47c..e3de1526397d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -333,12 +333,10 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
 					.link = aconnector->dc_link,
 					.sink_signal = SIGNAL_TYPE_DISPLAY_PORT_MST };
 
-				dc_sink = dc_link_add_remote_sink(
-					aconnector->dc_link,
-					NULL,
-					0,
-					&init_params);
-
+				dc_sink = dc_link_add_remote_sink(aconnector->dc_link,
+								  NULL,
+								  0,
+								  &init_params);
 				if (!dc_sink) {
 					DRM_ERROR("Unable to add a remote sink\n");
 					return 0;
@@ -371,15 +369,10 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
 		struct dc_sink_init_data init_params = {
 				.link = aconnector->dc_link,
 				.sink_signal = SIGNAL_TYPE_DISPLAY_PORT_MST };
-		const struct edid *edid;
-
-		edid = drm_edid_raw(aconnector->drm_edid); // FIXME: Get rid of drm_edid_raw()
-		dc_sink = dc_link_add_remote_sink(
-			aconnector->dc_link,
-			(uint8_t *)edid,
-			(edid->extensions + 1) * EDID_LENGTH,
-			&init_params);
 
+		dc_sink = dc_link_add_remote_sink(aconnector->dc_link,
+						  aconnector->drm_edid, 0,
+						  &init_params);
 		if (!dc_sink) {
 			DRM_ERROR("Unable to add a remote sink\n");
 			return 0;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
index fa0f0e61f05d..b398c9c5e04f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
@@ -6,25 +6,25 @@
 bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
 			  struct dc_sink *current_sink)
 {
-	struct dc_edid *old_edid = &prev_sink->dc_edid;
-	struct dc_edid *new_edid = &current_sink->dc_edid;
-
-       if (old_edid->length != new_edid->length)
-               return false;
-
-       if (new_edid->length == 0)
-               return false;
-
-       return (memcmp(old_edid->raw_edid,
-                      new_edid->raw_edid, new_edid->length) == 0);
+	return drm_edid_is_edid_eq(prev_sink->drm_edid, current_sink->drm_edid);
 }
 
 void dc_edid_copy_edid_to_dc(struct dc_sink *dc_sink,
 			     const void *edid,
 			     int len)
 {
-	memmove(dc_sink->dc_edid.raw_edid, (const uint8_t *) edid, len);
-	dc_sink->dc_edid.length = len;
+	dc_sink->drm_edid = drm_edid_dup((const struct drm_edid *) edid);
+}
+
+void dc_edid_copy_edid_to_sink(struct dc_sink *sink)
+{
+	const struct edid *edid;
+	uint32_t edid_length;
+
+	edid = drm_edid_raw(sink->drm_edid); // FIXME: Get rid of drm_edid_raw()
+	edid_length = EDID_LENGTH * (edid->extensions + 1);
+	memcpy(sink->dc_edid.raw_edid, (uint8_t *) edid, edid_length);
+	sink->dc_edid.length = edid_length;
 }
 
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
index 2c76768be459..a95cc6ccc743 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.h
@@ -9,6 +9,7 @@ bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
 			  struct dc_sink *current_sink);
 void dc_edid_copy_edid_to_dc(struct dc_sink *dc_sink,
 			     const void *edid, int len);
+void dc_edid_copy_edid_to_sink(struct dc_sink *sink);
 void dc_edid_sink_edid_free(struct dc_sink *sink);
 
 #endif /* __DC_EDID_H__ */
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index 978d2b4a4d29..40cf1f0aa7cf 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -1142,6 +1142,7 @@ static bool detect_link_and_local_sink(struct dc_link *link,
 			dp_trace_init(link);
 
 		/* Connectivity log: detection */
+		dc_edid_copy_edid_to_sink(sink);
 		for (i = 0; i < sink->dc_edid.length / DC_EDID_BLOCK_SIZE; i++) {
 			CONN_DATA_DETECT(link,
 					 &sink->dc_edid.raw_edid[i * DC_EDID_BLOCK_SIZE],
@@ -1424,7 +1425,7 @@ struct dc_sink *link_add_remote_sink(
 	 * parsing fails
 	 */
 	if (edid_status != EDID_OK && edid_status != EDID_PARTIAL_VALID) {
-		dc_sink->dc_edid.length = 0;
+		drm_edid_free(dc_sink->drm_edid);
 		dm_error("Bad EDID, status%d!\n", edid_status);
 	}
 
-- 
2.47.2

Re: [PATCH 13/13] drm/amd/display: move dc_sink from dc_edid to drm_edid

[Jani Nikula]

On Fri, 11 Apr 2025, Melissa Wen <mwen@igalia.com> wrote:
> +void dc_edid_copy_edid_to_sink(struct dc_sink *sink)
> +{
> +	const struct edid *edid;
> +	uint32_t edid_length;
> +
> +	edid = drm_edid_raw(sink->drm_edid); // FIXME: Get rid of drm_edid_raw()
> +	edid_length = EDID_LENGTH * (edid->extensions + 1);

I guess none of my concern, really, but here's a hint:

The underlying reason for me to create struct drm_edid to begin with,
make it opaque, and to go through all the trouble of converting tons of
code from struct edid to struct drm_edid was exactly the above statement
being replicated all over the place. It's fundamentally incompatible
with the HF-EEODB EDID extension block.

BR,
Jani.



> +	memcpy(sink->dc_edid.raw_edid, (uint8_t *) edid, edid_length);
> +	sink->dc_edid.length = edid_length;
>  }
>  
>  

-- 
Jani Nikula, Intel