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 <[email protected]> > > --- > > 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, >
