Re: [PATCH v7 14/15] drm/amd/display: add drm_edid to dc_sink

["Timur Kristóf" <timur.kristof@gmail.com>]

On Thu, 2025-11-06 at 13:49 -0300, Melissa Wen wrote:
> 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()
> 
> v7:
> - put new edid-related helpers in dm_helpers (Harry)
> 
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 5 +++++
>  drivers/gpu/drm/amd/display/dc/core/dc_sink.c             | 2 ++
>  drivers/gpu/drm/amd/display/dc/dc.h                       | 1 +
>  drivers/gpu/drm/amd/display/dc/dm_helpers.h               | 1 +
>  4 files changed, 9 insertions(+)
> 
> 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 a7ad93c2eb5f..419852dfc237 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
> @@ -1007,6 +1007,11 @@ void dm_helpers_copy_edid_to_dc(struct dc_sink
> *dc_sink,
>  	dc_sink->dc_edid.length = len;
>  }
>  
> +void dm_helpers_sink_edid_free(struct dc_sink *sink)
> +{
> +	drm_edid_free(sink->drm_edid);
> +}
> +
>  enum dc_edid_status dm_helpers_read_local_edid(
>  		struct dc_context *ctx,
>  		struct dc_link *link,
> 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..a5b9081879e3 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_sink.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_sink.c
> @@ -65,6 +65,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);
> +
> +	dm_helpers_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 2efb9add13ff..3cf7507d11fa 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -2576,6 +2576,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 */

If we want DC to be platform-agnostic, I don't think we should use
pointers to DRM structures here. I am not an expert, but I assume that
DC needs an EDID on other platforms as well.
How about something like this?

const void *platform_edid; /* Pointer to platform specific EDID struct */

Then, on Linux this can be the drm_edid and on other platforms
it can be an equivalent struct.

>  	struct dc_edid_caps edid_caps; /* parse display caps */
>  	struct dc_container_id *dc_container_id;
>  	uint32_t dongle_max_pix_clk;
> diff --git a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> index 0415cb50fe32..e23204fdd3f5 100644
> --- a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> +++ b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> @@ -68,6 +68,7 @@ bool dm_helpers_is_same_edid(struct dc_sink
> *prev_sink,
>  void dm_helpers_copy_edid_to_dc(struct dc_sink *dc_sink,
>  				const void *edid, int len);
>  
> +void dm_helpers_sink_edid_free(struct dc_sink *sink);
>  
>  /*
>   * Update DP branch info