Re: [RFC PATCH 0/2] drm/amd/display: switch amdgpu_dm_connector to

[Mario Limonciello <mario.limonciello@amd.com>]

On 1/26/2024 10:28, Melissa Wen wrote:
> Hi,
> 
> I'm debugging a null-pointer dereference when running
> igt@kms_connector_force_edid and the way I found to solve the bug is to
> stop using raw edid handler in amdgpu_connector_funcs_force and
> create_eml_sink in favor of managing resouces via sruct drm_edid helpers
> (Patch 1). The proper solution seems to be switch amdgpu_dm_connector
> from struct edid to struct drm_edid and avoid the usage of different
> approaches in the driver (Patch 2). However, doing it implies a good
> amount of work and validation, therefore I decided to send this RFC
> first to collect opinions and check if there is any parallel work on
> this side. It's a working in progress.
> 
> The null-pointer error trigger by the igt@kms_connector_force_edid test
> was introduced by:
> - e54ed41620f ("drm/amd/display: Remove unwanted drm edid references")
> 
> You can check the error trace in the first patch.
> 
> This series was tested with kms_hdmi_inject and kms_force_connector. No
> null-pointer error, kms_hdmi_inject is successul and kms_force_connector
> is sucessful after the second execution - the force-edid subtest
> still fails in the first run (I'm still investigating).
> 
> There is also a couple of cast warnings to be addressed - I'm looking
> for the best replacement.
> 
> I appreciate any feedback and testing.

So I'm actually a little bit worried by hardcoding EDID_LENGTH in this 
series.

I have some other patches that I'm posting later on that let you get the 
EDID from _DDC BIOS method too.  My observation was that the EDID can be 
anywhere up to 512 bytes according to the ACPI spec.

An earlier version of my patch was using EDID_LENGTH when fetching it 
and the EDID checksum failed.

I'll CC you on the post, we probably want to get your changes and mine 
merged together.

> 
> Melissa
> 
> Melissa Wen (2):
>    drm/amd/display: fix null-pointer dereference on edid reading
>    drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
> 
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 78 ++++++++++---------
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  4 +-
>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  9 ++-
>   .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 23 +++---
>   4 files changed, 60 insertions(+), 54 deletions(-)
>