[PATCH v6 00/14] drm/amd/display: more drm_edid to AMD display driver
Melissa Wen
mwen at igalia.com
Mon Aug 11 21:47:54 UTC 2025
On 11/08/2025 18:26, Harry Wentland wrote:
>
> On 2025-08-11 17:09, Limonciello, Mario wrote:
>> On 8/11/25 4:08 PM, Hung, Alex wrote:
>>> Melissa,
>>>
>>> The patchset passed promotion and CI.
>>>
>>> However, since our DC code is shared with the other OS, calling drm_*
>>> functions in DC won't work for us. For example, calling
>>> dc_edid_copy_edid_to_sink from dc/link/link_detection.c in patch 14.
>>>
>>> I don't have a good way to handle it. Does it make sense not to touch DC
>>> code for now?
>> What about if we have a set of compatibility macros in DC code?
>>
>> Something like this:
>>
>> #ifndef drm_dbg
>> #define drm_dbg ....
>> #endif
> DC should stay OS-agnostic. No DRM concepts in DC please.
Yes, and that is exactly the reason for "[PATCH v6 10/14]
drm/amd/display: add a mid-layer file to handle EDID in DC"
(https://lore.kernel.org/amd-gfx/20250726003816.435227-11-mwen@igalia.com/).
DC code still handles raw EDID data and drivers handling raw EDID is
exactly what we are trying to remove from DRM. But with the current
implementation of the AMD driver, we cannot remove it without touching
DC code. The thing is, if we stop handle raw EDID in the DM layer, we
cannot pass this data to DC and vice-versa.
So the proposal with patch 10 (and follow-up patches) is creating
different dc_edid files, one for linux and another for different platforms.
>
> Why the need to change dc_edid_is_same_edid?
The linux file is done in this series by reimplementing the DC functions
that handles raw EDID, like dc_edid_is_same_edid, with drm_edid helpers.
The file for other platforms can be the original functions without these
changes.
In fact, there is a step that should be done by AMD people because we
don't handle other platforms. It should be somewhat similar to the DC
FPU isolation code.
If it's not possible for any reasons, could you share with us some
suggestions on how to address this issue?
Thanks in advance,
Melissa
>
> I'll comment directly on the patch.
>
> Harry
>
>>> On 8/11/25 13:40, Melissa Wen wrote:
>>>>
>>>> On 28/07/2025 20:29, Alex Hung wrote:
>>>>> Thanks. I will send v6 to promotion test.
>>>> Hi Alex,
>>>>
>>>> Any news about this round of tests?
>>>>
>>>> BR,
>>>>
>>>> Melissa
>>>>
>>>>> On 7/25/25 18:33, Melissa Wen wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Siqueira and I have been working on a solution to reduce the usage of
>>>>>> drm_edid_raw in the AMD display driver, since the current guideline in
>>>>>> the DRM subsystem is to stop handling raw edid data in driver-specific
>>>>>> implementation and use opaque `drm_edid` object with common-code
>>>>>> helpers.
>>>>>>
>>>>>> To keep DC as an OS-agnostic component, we create a mid layer that
>>>>>> isolates `drm_edid` helpers called in the DC code, while allowing other
>>>>>> OSes to implement their specific implementation.
>>>>>>
>>>>>> This work is an extension of [1].
>>>>>>
>>>>>> - Patch 1 addresses a possible leak added by previous migration to
>>>>>> drm_edid.
>>>>>> - Patch 2 allocates a temporary drm_edid from raw edid for parsing.
>>>>>> - Patches 3-7 use common-code, drm_edid helpers to parse edid
>>>>>> capabilities instead of driver-specific solutions. For this, patch 4
>>>>>> introduces a new helper that gets monitor name from drm_edid.
>>>>>> - Patches 8-9 are groundwork to reduce the noise of Linux/DRM specific
>>>>>> code in the DC shared code
>>>>>> - Patch 10 creates a mid layer to make DC embraces different ways of
>>>>>> handling EDID by platforms.
>>>>>> - Patch 11 move open-coded management of raw EDID data to the mid
>>>>>> layer created before.
>>>>>> - Patch 12 introduces a helper that compares EDIDs from two drm_edids.
>>>>>> - Patch 13 adds drm_edid to dc_sink struct and a mid-layer helper to
>>>>>> free `drm_edid`.
>>>>>> - Patch 14 switch dc_edid to drm_edid across the driver in a way that
>>>>>> the DC shared code is little affected by Linux specific stuff.
>>>>>>
>>>>>> [v1] https://lore.kernel.org/dri-devel/20250411201333.151335-1-
>>>>>> mwen at igalia.com/
>>>>>> Changes:
>>>>>> - fix broken approach to get monitor name from eld (Jani)
>>>>>> - I introduced a new helper that gets monitor name from drm_edid
>>>>>> - rename drm_edid_eq to drm_edid_eq_buf and doc fixes (Jani)
>>>>>> - add NULL edid checks (Jani)
>>>>>> - fix mishandling of product_id.manufacturer_name (Michel)
>>>>>> - I directly set it to manufacturer_id since sparse didn't complain.
>>>>>> - add Mario's r-b in the first fix patch and fix commit msg typo.
>>>>>>
>>>>>> [v2] https://lore.kernel.org/dri-devel/20250507001712.120215-1-
>>>>>> mwen at igalia.com/
>>>>>> Changes:
>>>>>> - kernel-doc and commit msg fixes (Jani)
>>>>>> - use drm_edid_legacy_init instead of open coded (Jani)
>>>>>> - place drm_edid new func into the right section (Jani)
>>>>>> - paramenter names fix (Jani)
>>>>>> - add Jani's r-b to the patch 12
>>>>>> - remove unnecessary include (Jani)
>>>>>> - call dc_edid_sink_edid_free in link_detection, instead of
>>>>>> drm_edid_free
>>>>>> - rebase on top of asdn
>>>>>>
>>>>>> [v3] https://lore.kernel.org/dri-devel/20250514202130.291324-1-
>>>>>> mwen at igalia.com/
>>>>>> Changes:
>>>>>> - rebase to asdn
>>>>>> - some kernel-doc fixes
>>>>>> - move some changes to the right commit
>>>>>>
>>>>>> [v4] https://lore.kernel.org/amd-gfx/20250613150015.245917-1-
>>>>>> mwen at igalia.com/
>>>>>> Changes:
>>>>>> - fix comments and commit messages (Mario)
>>>>>> - remove unnecessary drm_edid dup and fix mem leak (Mario)
>>>>>> - add Mario's rb to patches 5-7
>>>>>>
>>>>>> [v5] https://lore.kernel.org/amd-gfx/20250618152216.948406-1-
>>>>>> mwen at igalia.com/
>>>>>> Changes:
>>>>>> - fix NULL pointer dereference (Alex H.) with the same approach
>>>>>> proposed
>>>>>> by 7c3be3ce3dfae
>>>>>>
>>>>>> --->
>>>>>> There are three specific points where we still use drm_edid_raw() in
>>>>>> the
>>>>>> driver:
>>>>>> 1. raw edid data for write EDID checksum in DP_TEST_EDID_CHECKSUM via
>>>>>> drm_dp_dpcd_write(), that AFAIK there is no common code solution
>>>>>> yet;
>>>>>> 2. open-coded connectivity log for dc link detection, that maybe can be
>>>>>> moved to drm (?);
>>>>>> 3. open-coded parser that I suspect is a lot of duplicated code, but
>>>>>> needs careful examining.
>>>>>>
>>>>>> I suggest to address those points in a next phase for regression
>>>>>> control.
>>>>>>
>>>>>> [1] https://lore.kernel.org/amd-gfx/20250308142650.35920-1-
>>>>>> mwen at igalia.com/
>>>>>>
>>>>>> Let me know yours thoughts!
>>>>>>
>>>>>> Melissa
>>>>>>
>>>>>> Melissa Wen (12):
>>>>>> drm/amd/display: make sure drm_edid stored in aconnector doesn't
>>>>>> leak
>>>>>> drm/amd/display: start using drm_edid helpers to parse EDID caps
>>>>>> drm/amd/display: use drm_edid_product_id for parsing EDID product
>>>>>> info
>>>>>> drm/edid: introduce a helper that gets monitor name from drm_edid
>>>>>> drm/amd/display: get panel id with drm_edid helper
>>>>>> drm/amd/display: get SAD from drm_eld when parsing EDID caps
>>>>>> drm/amd/display: get SADB from drm_eld when parsing EDID caps
>>>>>> drm/amd/display: simplify dm_helpers_parse_edid_caps signature
>>>>>> drm/amd/display: change DC functions to accept private types for
>>>>>> edid
>>>>>> drm/edid: introduce a helper that compares edid data from two
>>>>>> drm_edid
>>>>>> drm/amd/display: add drm_edid to dc_sink
>>>>>> drm/amd/display: move dc_sink from dc_edid to drm_edid
>>>>>>
>>>>>> Rodrigo Siqueira (2):
>>>>>> drm/amd/display: add a mid-layer file to handle EDID in DC
>>>>>> drm/amd/display: create a function to fill dc_sink with edid data
>>>>>>
>>>>>> .../gpu/drm/amd/display/amdgpu_dm/Makefile | 1 +
>>>>>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 33 +++---
>>>>>> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 109 ++++++
>>>>>> +-----------
>>>>>> .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 21 ++--
>>>>>> .../gpu/drm/amd/display/amdgpu_dm/dc_edid.c | 39 +++++++
>>>>>> .../gpu/drm/amd/display/amdgpu_dm/dc_edid.h | 15 +++
>>>>>> .../drm/amd/display/dc/core/dc_link_exports.c | 9 +-
>>>>>> drivers/gpu/drm/amd/display/dc/core/dc_sink.c | 3 +
>>>>>> drivers/gpu/drm/amd/display/dc/dc.h | 10 +-
>>>>>> drivers/gpu/drm/amd/display/dc/dm_helpers.h | 7 +-
>>>>>> drivers/gpu/drm/amd/display/dc/inc/link.h | 9 +-
>>>>>> .../drm/amd/display/dc/link/link_detection.c | 30 ++---
>>>>>> .../drm/amd/display/dc/link/link_detection.h | 9 +-
>>>>>> drivers/gpu/drm/bridge/sil-sii8620.c | 2 +-
>>>>>> drivers/gpu/drm/display/drm_dp_mst_topology.c | 2 +-
>>>>>> drivers/gpu/drm/drm_edid.c | 54 +++++++--
>>>>>> include/drm/drm_edid.h | 10 +-
>>>>>> 17 files changed, 199 insertions(+), 164 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
>>>>>>
More information about the dri-devel
mailing list