Re: [Intel-gfx] [PATCH 0/4] drm/amd/display: stop using drm_edid_override_connector_update()

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 22 Aug 2023, Alex Hung <alex.hung@amd.com> wrote:
> On 2023-08-22 06:01, Jani Nikula wrote:
>> Over the past years I've been trying to unify the override and firmware
>> EDID handling as well as EDID property updates. It won't work if drivers
>> do their own random things.
> Let's check how to replace these references by appropriate ones or fork 
> the function as reverting these patches causes regressions.

I think the fundamental problem you have is conflating connector forcing
with EDID override. They're orthogonal. The .force callback has no
business basing the decisions on connector->edid_override. Force is
force, override is override.

The driver isn't even supposed to know or care if the EDID originates
from the firmware loader or override EDID debugfs. drm_get_edid() will
handle that for you transparently. It'll return the EDID, and you
shouldn't look at connector->edid_blob_ptr either. Using that will make
future work in drm_edid.c harder.

You can't fix that with minor tweaks. I think you'll be better off
starting from scratch.

Also, connector->edid_override is debugfs. You actually can change the
behaviour. If your userspace, whatever it is, has been written to assume
connector forcing if EDID override is set, you *do* have to fix that,
and set both.

BR,
Jani.


>
> Cheers,
> Alex
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Alex Hung <alex.hung@amd.com>
>> Cc: Chao-kai Wang <Stylon.Wang@amd.com>
>> Cc: Daniel Wheeler <daniel.wheeler@amd.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Hersen Wu <hersenxs.wu@amd.com>
>> Cc: Leo Li <sunpeng.li@amd.com>
>> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
>> Cc: Wenchieh Chien <wenchieh.chien@amd.com>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> 
>> Jani Nikula (4):
>>    Revert "drm/amd/display: drop unused count variable in
>>      create_eml_sink()"
>>    Revert "drm/amd/display: assign edid_blob_ptr with edid from debugfs"
>>    Revert "drm/amd/display: mark amdgpu_dm_connector_funcs_force static"
>>    Revert "drm/amd/display: implement force function in
>>      amdgpu_dm_connector_funcs"
>> 
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 44 +++----------------
>>   1 file changed, 5 insertions(+), 39 deletions(-)
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center