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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[AMD Official Use Only - General]

+ Charlie

-----Original Message-----
From: Jani Nikula <jani.nikula@xxxxxxxxx>
Sent: Tuesday, August 29, 2023 6:49 AM
To: Hung, Alex <Alex.Hung@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Li, Sun peng (Leo) <Sunpeng.Li@xxxxxxx>; David Airlie <airlied@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Siqueira, Rodrigo <Rodrigo.Siqueira@xxxxxxx>; Wheeler, Daniel <Daniel.Wheeler@xxxxxxx>; Wu, Hersen <hersenxs.wu@xxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; Chien, WenChieh (Jay) <WenChieh.Chien@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Wentland, Harry <Harry.Wentland@xxxxxxx>
Subject: Re: [Intel-gfx] [PATCH 0/4] drm/amd/display: stop using drm_edid_override_connector_update()

On Wed, 23 Aug 2023, Jani Nikula <jani.nikula@xxxxxxxxx> wrote:
> On Tue, 22 Aug 2023, Alex Hung <alex.hung@xxxxxxx> 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.

Any updates on fixing this, or shall we proceed with the reverts?

BR,
Jani.



>
> BR,
> Jani.
>
>
>>
>> Cheers,
>> Alex
>>
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
>>> Cc: Alex Hung <alex.hung@xxxxxxx>
>>> Cc: Chao-kai Wang <Stylon.Wang@xxxxxxx>
>>> Cc: Daniel Wheeler <daniel.wheeler@xxxxxxx>
>>> Cc: Harry Wentland <harry.wentland@xxxxxxx>
>>> Cc: Hersen Wu <hersenxs.wu@xxxxxxx>
>>> Cc: Leo Li <sunpeng.li@xxxxxxx>
>>> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>
>>> Cc: Wenchieh Chien <wenchieh.chien@xxxxxxx>
>>> Cc: David Airlie <airlied@xxxxxxxxx>
>>> Cc: Daniel Vetter <daniel@xxxxxxxx>
>>>
>>> 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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux