On 2025-06-18 11:19, Melissa Wen wrote:
> From: Rodrigo Siqueira <[email protected]>
> 
> 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 <[email protected]>
> Co-developed-by: Melissa Wen <[email protected]>
> Signed-off-by: Melissa Wen <[email protected]>
> 
> ---
> 
> v3:
> - rebase on top of asdn
> ---
>  .../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

It'd be better to name this amdgpu_dm_edid.h/c. Otherwise reviewers
like me will think we're looking at DC code.

It looks like dc_fpu.h/c set a precedent. Maybe we should rename that
as well.

> 
> 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..3f8295a68a72
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: MIT
> +#include "dc.h"
> +#include "dc_edid.h"
> +
> +bool dc_edid_is_same_edid(struct dc_sink *prev_sink,
> +                       struct dc_sink *current_sink)

Please avoid dc_ prefix in amdgpu_dm. Use dm_ prefix instead,
or dm_helpers_.

Harry

> +{
> +     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..957d74ff1294 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 "amdgpu_dm/dc_edid.h"

DC should be stand-alone from amdgpu_dm. Please put the function
definition into dm_helpers.h. That way each OS can implement their
own version.

Harry

> +
>   // 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;

Reply via email to