The series look fine to me, except one small error in patch 2. I can
send this series to promotion tests once the error is addressed. Let me
also check others for comments.
Hi Harry and Leo,
Do you have other concerns before I sent this series to promotion tests?
On 3/8/25 07:26, Melissa Wen wrote:
Hi,
I've been working on a new version of [1] that ports the AMD display
driver to use the common `drm_edid` code instead of open and raw edid
handling.
The part of the previous series that didn't make the cut was to replace
the open coded edid parser with `drm_edid_product_id` and `eld` data.
However, when addressing feedback I ran into a design issue in the
latest version because I was trying to not add any Linux-specific code
to the DC code, specifically, DC link detection. In short, we have a few
paths in the DM code that allocate a temporary `drm_edid`, go to the DC,
and back to the DM to handle `drm_edid` data. In the last version, I was
storing this temporary `drm_edid` in the aconnector, but it was
erroneously overriding a still in use `drm_edid`.
In this new version I allocate a temporary `drm_edid` in the DM parser
from raw edid data stored in `dc_edid`, which was actually extracted
from another `drm_edid` in the previous DM caller. This is a workaround
to bypass DC boundaries and parse edid capabilities, but I think we can
do better if we can find a clean way to pass the `drm_edid` through this
kind of DM -> DC -> DM paths.
I checked working on top of Thomas' work[2] that replaces `dc_edid` by
`drm_edid` and adds this DRM struct and its helpers inside DC. The
resulted changes work stable and as expected[3], but I believe that
adding linux-specific code to DC is not desirable.
Siqueira and I have discussed other alternatives, such as updating the
DC code to match `drm_edid` structs or checking how well the change in
this series could work with `drm_edid` as a private obj[4], however we
would like to know AMD team's opinion before making this big effort (and
probably noisy change). The main goal here is removing `drm_edid_raw`
calls and duplicated code to handle edid in DC/link_detection that can
take advantage of DRM common-code instead.
Please, let me know your thoughts.
Melissa
[1]
https://lore.kernel.org/amd-gfx/20240918213845.158293-1-mario.limoncie...@amd.com/
[2]
https://lore.kernel.org/amd-gfx/20241112-amdgpu-drm_edid-v2-0-1399dc0f0...@weissschuh.net/
[3]
https://gitlab.freedesktop.org/mwen/linux-amd/-/commits/drm_edid_product_id_v4
[4] https://gitlab.freedesktop.org/mwen/linux-amd/-/commits/drm_edid_priv
Melissa Wen (7):
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/amd/display: parse display name from drm_eld
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
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 85 +++++++++----------
2 files changed, 42 insertions(+), 45 deletions(-)
