[PATCH v4 10/14] drm/amd/display: add a mid-layer file to handle EDID in DC

[Limonciello, Mario]

On 6/17/25 1:56 PM, Rodrigo Siqueira wrote:
> On 06/13, Mario Limonciello wrote:
>> On 6/13/2025 9:58 AM, Melissa Wen wrote:
>>> From: Rodrigo Siqueira <siqueira at 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.
>>>
>>> v3:
>>> - rebase on top of asdn
>> Can you put changelog below cutlist (---)?
>>>
>>> Signed-off-by: Rodrigo Siqueira <siqueira at igalia.com>
>>> Co-developed-by: Melissa Wen <mwen at igalia.com>
>>> Signed-off-by: Melissa Wen <mwen at 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 7329b8cc2576..09cb94d8e0e4 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
>>> @@ -39,6 +39,7 @@ AMDGPUDM = \
>>>    	amdgpu_dm_psr.o \
>>>    	amdgpu_dm_replay.o \
>>>    	amdgpu_dm_quirks.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 863c24fe1117..344356e26f8b 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"
>>
>> There's a problem with the header location.  If you're naming it dc_edid.h
>> but putting the header in amdgpu_dm/ directory then it's only going to
>> compile for amdgpu.
>>
>> I think dc_edid.h needs to go into dc/ directory.
> 
> Hey Mario,
> 
> DC is shared with other OSes, and sometimes, we need to find ways to
> maintain this synergy with some tricks. One of them is to implement a
> set of files with standard functions, but these files get different
> implementations per OSes. One good example of this approach was the
> introduction of the `dc_fpu.[c|h]` file:
> 
>   https://gitlab.freedesktop.org/agd5f/linux/-/commit/96ee63730fa30
> 
> The idea is that every other OS can implement the dc_fpu.c file as they
> want, and we maintain the same interface. The same idea applies to this
> patch. We want to keep all the drm-specific things isolated in the
> amdgpu_dm, call the generic functions in DC, but other OSes will do the
> same with their specific implementation. Additionally, I think that
> keeping the dc_ name in the amdgpu folder is also a good indication of
> this approach.
> 
> Thanks
> Siqueira

Siqueira,

Thanks for the comment and clarifying the situation.  You know this area 
better than most people, and thanks for explaining it to me.

THanks,

> 
>>
>>> +
>>>     // Offset DPCD 050Eh == 0x5A
>>>    #define MST_HUB_ID_0x5A  0x5A
>>> @@ -617,18 +619,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)
>>>    {
>>> @@ -1105,8 +1095,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;
>>
>