mbox series

[RFC,00/10] drm/panel: Remove most store/double-check of prepared/enabled state

Message ID 20230804210644.1862287-1-dianders@chromium.org (mailing list archive)
Headers show
Series drm/panel: Remove most store/double-check of prepared/enabled state | expand

Message

Doug Anderson Aug. 4, 2023, 9:06 p.m. UTC
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. As part of this series a new helper is added that uses
the state tracking that the drm_panel core is doing so each individual
panel driver doesn't need to do it.

This series changes a lot of drivers and obviously the author can't
test on all of them. The changes here are also not completely trivial
in all cases. Please double-check your drivers carefully to make sure
something wasn't missed. After looking at over 40 drivers I'll admit
that my eyes glazed over a little.

I've attempted to organize these patches like to group together panels
that needed similar handling. Panels that had code that didn't seem to
match anyone else got their own patch. I made judgement calls on what
I considered "similar".

As noted in individual patches, there are some cases here where I
expect behavior to change a little bit. I'm hoping these changes are
for the better and don't cause any problems. Fingers crossed.

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.


Douglas Anderson (10):
  drm/panel: Don't store+check prepared/enabled for simple cases
  drm/panel: s6e63m0: Don't store+check prepared/enabled
  drm/panel: otm8009a: Don't double check prepared/enabled
  drm/panel_helper: Introduce drm_panel_helper
  drm/panel: Don't store+check prepared/enabled for panels needing
    shutdown
  drm/panel: Don't store+check prepared/enabled for panels disabled at
    shutdown
  drm/panel: st7703: Don't store+check prepared
  drm/panel: rm67191: Don't store+check enabled
  drm/panel: sony-acx565akm: Don't double-check enabled state in disable
  drm/panel: Update TODO list item for cleaning up prepared/enabled
    tracking

 Documentation/gpu/todo.rst                    | 33 +++++-------
 drivers/gpu/drm/Makefile                      |  1 +
 drivers/gpu/drm/drm_panel_helper.c            | 37 ++++++++++++++
 .../drm/panel/panel-asus-z00t-tm5p5-n35596.c  |  9 ----
 .../gpu/drm/panel/panel-boe-bf060y8m-aj0.c    |  9 ----
 drivers/gpu/drm/panel/panel-boe-himax8279d.c  | 36 ++-----------
 .../gpu/drm/panel/panel-boe-tv101wum-nl6.c    | 16 +-----
 drivers/gpu/drm/panel/panel-edp.c             | 34 ++-----------
 drivers/gpu/drm/panel/panel-elida-kd35t133.c  | 21 +-------
 drivers/gpu/drm/panel/panel-himax-hx8394.c    | 21 +-------
 drivers/gpu/drm/panel/panel-innolux-p079zca.c | 51 ++-----------------
 drivers/gpu/drm/panel/panel-jdi-fhd-r63452.c  |  9 ----
 .../gpu/drm/panel/panel-jdi-lt070me05000.c    | 30 ++---------
 drivers/gpu/drm/panel/panel-khadas-ts050.c    | 35 ++-----------
 .../drm/panel/panel-kingdisplay-kd097d04.c    | 43 ++--------------
 .../drm/panel/panel-leadtek-ltk050h3146w.c    | 21 +-------
 .../drm/panel/panel-leadtek-ltk500hd1829.c    | 21 +-------
 drivers/gpu/drm/panel/panel-novatek-nt35950.c |  9 ----
 drivers/gpu/drm/panel/panel-novatek-nt36523.c | 12 -----
 .../gpu/drm/panel/panel-novatek-nt36672a.c    | 24 ++-------
 .../drm/panel/panel-olimex-lcd-olinuxino.c    | 45 +---------------
 .../gpu/drm/panel/panel-orisetech-otm8009a.c  | 17 -------
 .../drm/panel/panel-osd-osd101t2587-53ts.c    | 37 ++------------
 .../drm/panel/panel-panasonic-vvx10f034n00.c  | 42 ++-------------
 drivers/gpu/drm/panel/panel-raydium-rm67191.c | 21 +-------
 drivers/gpu/drm/panel/panel-raydium-rm68200.c | 38 --------------
 .../gpu/drm/panel/panel-samsung-atna33xc20.c  | 31 ++---------
 drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 25 ---------
 .../panel/panel-samsung-s6e88a0-ams452ef01.c  | 10 ----
 drivers/gpu/drm/panel/panel-samsung-sofef00.c |  9 ----
 drivers/gpu/drm/panel/panel-seiko-43wvf1g.c   | 45 ++--------------
 .../gpu/drm/panel/panel-sharp-lq101r1sx01.c   | 46 ++---------------
 .../gpu/drm/panel/panel-sharp-ls043t1le01.c   | 19 ++-----
 .../gpu/drm/panel/panel-sharp-ls060t1sx01.c   | 10 ----
 drivers/gpu/drm/panel/panel-simple.c          | 34 ++-----------
 drivers/gpu/drm/panel/panel-sitronix-st7703.c | 20 +-------
 drivers/gpu/drm/panel/panel-sony-acx565akm.c  |  7 +--
 drivers/gpu/drm/panel/panel-sony-td4353-jdi.c |  9 ----
 .../panel/panel-sony-tulip-truly-nt35521.c    | 18 -------
 .../drm/panel/panel-startek-kd070fhfid015.c   | 11 ----
 drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c  | 19 ++-----
 drivers/gpu/drm/panel/panel-truly-nt35597.c   | 20 --------
 drivers/gpu/drm/panel/panel-visionox-r66451.c | 16 ------
 .../gpu/drm/panel/panel-visionox-rm69299.c    |  8 ---
 .../gpu/drm/panel/panel-visionox-vtdr6130.c   |  9 ----
 .../gpu/drm/panel/panel-xinpeng-xpp055c272.c  | 21 +-------
 include/drm/drm_panel_helper.h                | 13 +++++
 47 files changed, 131 insertions(+), 941 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_panel_helper.c
 create mode 100644 include/drm/drm_panel_helper.h

Comments

Linus Walleij Aug. 10, 2023, 8:23 a.m. UTC | #1
On Fri, Aug 4, 2023 at 11:07 PM Douglas Anderson <dianders@chromium.org> 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. As part of this series a new helper is added that uses
> the state tracking that the drm_panel core is doing so each individual
> panel driver doesn't need to do it.
>
> This series changes a lot of drivers and obviously the author can't
> test on all of them. The changes here are also not completely trivial
> in all cases. Please double-check your drivers carefully to make sure
> something wasn't missed. After looking at over 40 drivers I'll admit
> that my eyes glazed over a little.
>
> I've attempted to organize these patches like to group together panels
> that needed similar handling. Panels that had code that didn't seem to
> match anyone else got their own patch. I made judgement calls on what
> I considered "similar".
>
> As noted in individual patches, there are some cases here where I
> expect behavior to change a little bit. I'm hoping these changes are
> for the better and don't cause any problems. Fingers crossed.
>
> 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.

The series:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Please send out a non-RFC version, this is clearly the right thing to
do.

Yours,
Linus Walleij
Doug Anderson Sept. 5, 2023, 7:15 p.m. UTC | #2
Hi,

On Thu, Aug 10, 2023 at 1:23 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Aug 4, 2023 at 11:07 PM Douglas Anderson <dianders@chromium.org> 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. As part of this series a new helper is added that uses
> > the state tracking that the drm_panel core is doing so each individual
> > panel driver doesn't need to do it.
> >
> > This series changes a lot of drivers and obviously the author can't
> > test on all of them. The changes here are also not completely trivial
> > in all cases. Please double-check your drivers carefully to make sure
> > something wasn't missed. After looking at over 40 drivers I'll admit
> > that my eyes glazed over a little.
> >
> > I've attempted to organize these patches like to group together panels
> > that needed similar handling. Panels that had code that didn't seem to
> > match anyone else got their own patch. I made judgement calls on what
> > I considered "similar".
> >
> > As noted in individual patches, there are some cases here where I
> > expect behavior to change a little bit. I'm hoping these changes are
> > for the better and don't cause any problems. Fingers crossed.
> >
> > 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.
>
> The series:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Please send out a non-RFC version, this is clearly the right thing to
> do.

As per the long discussion in response to patch #4, I think there are
still open questions about the later patches in this series. However,
I could land patches #1 - #3 if there are no concerns. Would anyone
object if I just landed them straight from this series with Linus's
review, or would I need to repost just patches #1 - #3 without the
"RFC" tag?

Thanks!

-Doug