On 06/13, Mario Limonciello wrote:
> On 6/13/2025 9:58 AM, Melissa Wen wrote:
> > From: Rodrigo Siqueira <sique...@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 <sique...@igalia.com>
> > Co-developed-by: Melissa Wen <m...@igalia.com>
> > Signed-off-by: Melissa Wen <m...@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 = ¤t_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
>
> > +
> > // 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;
>
--
Rodrigo Siqueira
