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

[Wu, Hersen]

[AMD Official Use Only - General]

+ Charlie Wang

-----Original Message-----
From: Alex Deucher <alexdeucher at gmail.com>
Sent: Tuesday, August 29, 2023 11:44 AM
To: Jani Nikula <jani.nikula at intel.com>
Cc: Hung, Alex <Alex.Hung at amd.com>; dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org; Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; intel-gfx at lists.freedesktop.org; Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Wheeler, Daniel <Daniel.Wheeler at amd.com>; Wu, Hersen <hersenxs.wu at amd.com>; Chien, WenChieh (Jay) <WenChieh.Chien at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>
Subject: Re: [Intel-gfx] [PATCH 0/4] drm/amd/display: stop using drm_edid_override_connector_update()

On Tue, Aug 29, 2023 at 6:48 AM Jani Nikula <jani.nikula at intel.com> wrote:
>
> On Wed, 23 Aug 2023, Jani Nikula <jani.nikula at intel.com> wrote:
> > On Tue, 22 Aug 2023, Alex Hung <alex.hung at 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.
>
> Any updates on fixing this, or shall we proceed with the reverts?

What is the goal of the reverts?  I don't disagree that we may be using the interfaces wrong, but reverting them will regess functionality in the driver.

Alex