linux-kernel - Re: [PATCH] drm/panel: Avoid warnings w/ panel-simple/panel-edp at shutdown

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 18 Jun 2024 16:49:22 -0700
From: Doug Anderson <dianders@...omium.org>
To: Doug Anderson <dianders@...omium.org>, dri-devel@...ts.freedesktop.org, 
	Neil Armstrong <neil.armstrong@...aro.org>, Maxime Ripard <mripard@...nel.org>, 
	Linus Walleij <linus.walleij@...aro.org>, Yuran Pereira <yuran.pereira@...mail.com>, 
	Chris Morgan <macromorgan@...mail.com>, David Airlie <airlied@...il.com>, 
	Jessica Zhang <quic_jesszhan@...cinc.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	linux-kernel@...r.kernel.org
Cc: Daniel Vetter <daniel@...ll.ch>
Subject: Re: [PATCH] drm/panel: Avoid warnings w/ panel-simple/panel-edp at shutdown

Hi,

On Mon, Jun 17, 2024 at 7:17 AM Daniel Vetter <daniel@...ll.ch> 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