Hi, On Mon, Jun 17, 2024 at 7:17 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > > > That all being said, I'm also totally OK with any of the following: > > > > 1. Dropping my patch and just accepting that we will have warnings > > printed out for all DRM drivers that do things correctly and have no > > warnings for broken DRM drivers. > > Can't we just flip the warnings around? Like make the hacky cleanup > conditional on the panel not yet being disabled/cleaned up, and complain > in that case only. With that: > - drivers which call shutdown shouldn't get a warning anymore, and also > not a surplus call to drm_panel_disable/unprepare > - drivers which are broken still get the cleanup calls > - downside: we can't enforce this, because it's not yet enforced through > device_link ordering I feel like something is getting lost in the discussion here. I'm just not sure where to put the hacky cleanup without either having a list like I have or fixing the device link problem so that the DRM device shutdown gets called before the panel. Specifically, right now I think it's possible for the panel's shutdown() callback to happen before the DRM Device's shutdown(). Thus, we have: 1. Panel shutdown() checks and says "it's not shutdown yet so do my hacky cleanup." 2. DRM device shutdown() gets called and tries to cleanup again. ...and thus in step #1 we can't detect a broken DRM device. What am I missing? > > 2. Someone else posting / landing a patch to remove the hacky "disable > > / unprepare" for panel-simple and panel-edp and asserting that they > > don't care if they break any DRM drivers that are still broken. I > > don't want to be involved in authoring or landing this patch, but I > > won't scream loudly if others want to do it. > > > > 3. Someone else taking over trying to solve this problem. > > > > ...mostly this work is janitorial and I'm trying to help move the DRM > > framework forward and get rid of cruft, so if it's going to cause too > > much conflict I'm fine just stepping back. > > My issue is that you're trading an ugly warning that hurts maintenance > with an explicit list of working drivers, which also hurts maintenance. > Does seem like forward progress to me, just pushing the issue around. IMO it at least moves things forward. If we make the warning obvious enough then I think we could assert that, within a few kernel versions, everyone who hit the warning would have addressed it. Once that happens we can fully get rid of the ugly list and just make the assumption that things are good. That feels like a clear path to me... -Doug
