On Mon, May 06, 2024 at 08:52:39AM GMT, Linus Walleij wrote:
> On Fri, May 3, 2024 at 11:36 PM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> > As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> > prepared/enabled in drm_panel"), we want to remove needless code from
> > panel drivers that was storing and double-checking the
> > prepared/enabled state. Even if someone was relying on the
> > double-check before, that double-check is now in the core and not
> > needed in individual drivers.
> >
> > This series attempts to do just that. While the original grep, AKA:
> > git grep 'if.*>prepared' -- drivers/gpu/drm/panel
> > git grep 'if.*>enabled' -- drivers/gpu/drm/panel
> > ...still produces a few hits after my series, they are _mostly_ all
> > gone. The ones that are left are less trivial to fix.
> >
> > One of the main reasons that many panels probably needed to store and
> > double-check their prepared/enabled appears to have been to handle
> > shutdown and/or remove. Panels drivers often wanted to force the power
> > off for panels in these cases and this was a good reason for the
> > double-check.
> >
> > In response to my V1 series [1] we had much discussion of what to
> > do. The conclusion was that as long as DRM modeset drivers properly
> > called drm_atomic_helper_shutdown() that we should be able to remove
> > the explicit shutdown/remove handling in the panel drivers. Most of
> > the patches to improve DRM modeset drivers [2] [3] [4] have now
> > landed.
> >
> > In contrast to my V1 series, I broke the V2 series up a lot
> > more. Since a few of the panel drivers in V1 already landed, we had
> > fewer total drivers and so we could devote a patch to each panel.
> > Also, since we were now relying on DRM modeset drivers I felt like we
> > should split the patches for each panel into two: one that's
> > definitely safe and one that could be reverted if we found a
> > problematic DRM modeset driver that we couldn't fix.
> >
> > Sorry for the large number of patches. I've set things to mostly just
> > CC people on the cover letter and the patches that are relevant to
> > them. I've tried to CC people on the whole series that have shown
> > interest in this TODO item.
> >
> > As patches in this series are reviewed and/or tested they could be
> > landed. There's really no ordering requirement for the series unless
> > patches touch the same driver.
> >
> > NOTE: this touches _a lot_ of drivers, is repetitive, and is not
> > really possible to generate automatically. That means it's entirely
> > possible that my eyes glazed over and I did something wrong. Please
> > double-check me and don't assume that I got everything perfect, though
> > I did my best. I have at least confirmed that "allmodconfig" for arm64
> > doesn't fall on its face with this series. I haven't done a ton of
> > other testing.
> >
> > [1] https://lore.kernel.org/r/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid
> > [2] https://lore.kernel.org/r/20230901234015.566018-1-dianders@xxxxxxxxxxxx
> > [3] https://lore.kernel.org/r/20230901234202.566951-1-dianders@xxxxxxxxxxxx
> > [4] https://lore.kernel.org/r/20230921192749.1542462-1-dianders@xxxxxxxxxxxx
>
> This is the right thing to do, thanks for looking into this!
>
> As for the behaviour of .remove() I doubt whether in many cases
> the original driver authors have even tested this themselves.
> I would say we should just apply the series as soon as it's non-RFC
> after the next merge window and see what happens. I doubt it
> will cause much trouble.
In the case of st7703 driver, yes tested, and proper shutdown of the panel is
necessary, because lack of it can lead to otherwise inexplainable blinking of
the entire screen, when the panel is quickly powered up and re-initialized again
(eg. as happens when bootloader has display support). Blinking then only ever
stops if the panel is left completely powered off for several minutes.
There's a note about this in the controller datasheet, that proper power off
is needed to enable dicharge of the panel.
Kind regards,
o.
> The series:
> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
> Yours,
> Linus Walleij
