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

[Melissa Wen]

On 06/13, Mario Limonciello wrote:
> On 6/13/2025 7:58 AM, 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()
> > 
> > v3:
> > - remove uneccessary include (jani)
> > 
> > Signed-off-by: Melissa Wen <mwen at igalia.com>
> > ---
> >   drivers/gpu/drm/amd/display/amdgpu_dm/dc_edid.c | 6 ++++++
> >   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             | 1 +
> >   include/drm/drm_edid.h                          | 4 ++--
> >   5 files changed, 13 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 a90545b176cc..9e86dc15557b 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,8 @@ void dc_edid_copy_edid_to_dc(struct dc_sink *dc_sink,
> >   	memmove(dc_sink->dc_edid.raw_edid, 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 86feef038de6..cf56a0405a4f 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dc.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> > @@ -2466,6 +2466,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*/
> 
> Don't you need a forward declaration for 'struct drm_edid' in dc.h to be
> able to do this?

I understand that, as it's just a pointer (the compiler knows the size)
and there is no circular dependencies between dc_sink and drm_edid, we
don't need a forward declaration. So I think we are fine also because
dc_sink->drm_edid dereference only happens in dc_edid.h that already
needs to include drm_edid.h for drm_edid helpers... but let me know if
I'm missing something.

> 
> Also you're missing a space at the end of the comment before the '*/'.

ack. I'll wait for more comments to send it fixed.

Thanks for reviewing.

Melissa

> 
> >   	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/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index e7a9a4928b97..8617d2285f38 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -469,8 +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_eq(const struct drm_edid *drm_edid_first,
> > -			 const struct drm_edid *drm_edid_second);
> > +bool drm_edid_eq(const struct drm_edid *drm_edid_1,
> > +		 const struct drm_edid *drm_edid_2);
> >   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,
>