[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
