Re: [RFC PATCH 00/10] drm/panel: Remove most store/double-check of prepared/enabled state
Hi, On Thu, Aug 10, 2023 at 1:23 AM Linus Walleij wrote: > > On Fri, Aug 4, 2023 at 11:07 PM Douglas Anderson > 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 > > 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
Re: [RFC PATCH 00/10] drm/panel: Remove most store/double-check of prepared/enabled state
On Fri, Aug 4, 2023 at 11:07 PM Douglas Anderson 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 Please send out a non-RFC version, this is clearly the right thing to do. Yours, Linus Walleij
[RFC PATCH 00/10] drm/panel: Remove most store/double-check of prepared/enabled state
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 +--