On Tue, 29 Aug 2023, Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > On Tue, 29 Aug 2023, Alex Deucher <alexdeucher@xxxxxxxxx> wrote: >> On Tue, Aug 29, 2023 at 6:48 AM Jani Nikula <jani.nikula@xxxxxxxxx> wrote: >>> >>> 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? >> >> 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. > > The commits are in v6.5-rc1, but not yet in a release. No user depends > on them yet. I'd strongly prefer them not reaching v6.5 final and users. Sorry for confusion here, that's obviously come and gone already. :( > The firmware EDID, override EDID, connector forcing, the EDID property, > etc. have been and somewhat still are a hairy mess that we must keep > untangling, and this isn't helping. > > I've put in crazy amounts of work on this, and I've added kernel-doc > comments about stuff that should and should not be done, but they go > unread and ignored. > > I really don't want to end up having to clean this up myself before I > can embark on further cleanups and refactoring. > > And again, if the functionality in the driver depends on conflating two > things that should be separate, it's probably not such a hot idea to let > it reach users either. Even if it's just debugfs. > > > BR, > Jani. -- Jani Nikula, Intel Open Source Graphics Center
