* [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state
@ 2024-05-03 21:32 Douglas Anderson
2024-05-03 21:33 ` [RFT PATCH v2 48/48] drm/panel: Update TODO list item for cleaning up prepared/enabled tracking Douglas Anderson
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Douglas Anderson @ 2024-05-03 21:32 UTC (permalink / raw)
To: dri-devel, Maxime Ripard
Cc: Linus Walleij, Chris Morgan, Yuran Pereira, Neil Armstrong,
Douglas Anderson, AngeloGioacchino Del Regno, Daniel Vetter,
David Airlie, Guido Günther, Jerry Han, Jessica Zhang,
Jonathan Corbet, Maarten Lankhorst, Matthias Brugger,
Ondrej Jirman, Purism Kernel Team, Robert Chiras, Sam Ravnborg,
Stefan Mavrodiev, Sumit Semwal, Thomas Zimmermann,
linux-arm-kernel, linux-doc, linux-kernel, linux-mediatek
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@chromium.org
[3] https://lore.kernel.org/r/20230901234202.566951-1-dianders@chromium.org
[4] https://lore.kernel.org/r/20230921192749.1542462-1-dianders@chromium.org
Changes in v2:
- ("drm/panel: raydium-rm692e5: Stop tracking prepared") new for v2.
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
- panel-edp and panel-simple just get a comment now.
Douglas Anderson (48):
drm/panel: raydium-rm692e5: Stop tracking prepared
drm/panel: boe-himax8279d: Stop tracking prepared/enabled
drm/panel: boe-himax8279d: Don't call unprepare+disable at
shutdown/remove
drm/panel: boe-tv101wum-nl6: Stop tracking prepared
drm/panel: boe-tv101wum-nl6: Don't call unprepare+disable at
shutdown/remove
drm/panel: edp: Stop tracking prepared/enabled
drm/panel: edp: Add a comment about unprepare+disable at
shutdown/remove
drm/panel: innolux-p079zca: Stop tracking prepared/enabled
drm/panel: innolux-p079zca: Don't call unprepare+disable at
shutdown/remove
drm/panel: khadas-ts050: Stop tracking prepared/enabled
drm/panel: khadas-ts050: Don't call unprepare+disable at
shutdown/remove
drm/panel: kingdisplay-kd097d04: Stop tracking prepared/enabled
drm/panel: kingdisplay-kd097d04: Don't call unprepare+disable at
shutdown/remove
drm/panel: ltk050h3146w: Stop tracking prepared
drm/panel: ltk050h3146w: Don't call unprepare+disable at
shutdown/remove
drm/panel: ltk500hd1829: Stop tracking prepared
drm/panel: ltk500hd1829: Don't call unprepare+disable at
shutdown/remove
drm/panel: novatek-nt36672a: Stop tracking prepared
drm/panel: novatek-nt36672a: Don't call unprepare+disable at
shutdown/remove
drm/panel: olimex-lcd-olinuxino: Stop tracking prepared/enabled
drm/panel: olimex-lcd-olinuxino: Don't call unprepare+disable at
remove
drm/panel: osd-osd101t2587-53ts: Stop tracking prepared/enabled
drm/panel: osd-osd101t2587-53ts: Don't call unprepare+disable at
shutdown/remove
drm/panel: samsung-atna33xc20: Stop tracking prepared/enabled
drm/panel: samsung-atna33xc20: Don't call unprepare+disable at
shutdown/remove
drm/panel: simple: Stop tracking prepared/enabled
drm/panel: simple: Add a comment about unprepare+disable at
shutdown/remove
drm/panel: tdo-tl070wsh30: Stop tracking prepared
drm/panel: tdo-tl070wsh30: Don't call unprepare+disable at
shutdown/remove
drm/panel: xinpeng-xpp055c272: Stop tracking prepared
drm/panel: xinpeng-xpp055c272: Don't call unprepare+disable at
shutdown/remove
drm/panel: jdi-lt070me05000: Stop tracking prepared/enabled
drm/panel: jdi-lt070me05000: Don't call disable at shutdown/remove
drm/panel: panasonic-vvx10f034n00: Stop tracking prepared/enabled
drm/panel: panasonic-vvx10f034n00: Don't call disable at
shutdown/remove
drm/panel: seiko-43wvf1g: Stop tracking prepared/enabled
drm/panel: seiko-43wvf1g: Don't call disable at shutdown/remove
drm/panel: sharp-lq101r1sx01: Stop tracking prepared/enabled
drm/panel: sharp-lq101r1sx01: Don't call disable at shutdown/remove
drm/panel: sharp-ls043t1le01: Stop tracking prepared
drm/panel: sharp-ls043t1le01: Don't call disable at shutdown/remove
drm/panel: sitronix-st7703: Stop tracking prepared
drm/panel: sitronix-st7703: Don't call disable at shutdown/remove
drm/panel: raydium-rm67191: Stop tracking enabled
drm/panel: raydium-rm67191: Don't call unprepare+disable at shutdown
drm/panel: sony-acx565akm: Don't double-check enabled state in disable
drm/panel: sony-acx565akm: Don't call disable at remove
drm/panel: Update TODO list item for cleaning up prepared/enabled
tracking
Documentation/gpu/todo.rst | 47 +++++++-------
drivers/gpu/drm/panel/panel-boe-himax8279d.c | 40 ------------
.../gpu/drm/panel/panel-boe-tv101wum-nl6.c | 23 -------
drivers/gpu/drm/panel/panel-edp.c | 60 +++++++-----------
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 55 ----------------
.../gpu/drm/panel/panel-jdi-lt070me05000.c | 35 -----------
drivers/gpu/drm/panel/panel-khadas-ts050.c | 39 ------------
.../drm/panel/panel-kingdisplay-kd097d04.c | 48 --------------
.../drm/panel/panel-leadtek-ltk050h3146w.c | 28 ---------
.../drm/panel/panel-leadtek-ltk500hd1829.c | 28 ---------
.../gpu/drm/panel/panel-novatek-nt36672a.c | 29 ---------
.../drm/panel/panel-olimex-lcd-olinuxino.c | 44 -------------
.../drm/panel/panel-osd-osd101t2587-53ts.c | 41 +-----------
.../drm/panel/panel-panasonic-vvx10f034n00.c | 47 +-------------
drivers/gpu/drm/panel/panel-raydium-rm67191.c | 26 --------
drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 10 ---
.../gpu/drm/panel/panel-samsung-atna33xc20.c | 36 -----------
drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 49 ---------------
.../gpu/drm/panel/panel-sharp-lq101r1sx01.c | 63 +------------------
.../gpu/drm/panel/panel-sharp-ls043t1le01.c | 24 -------
drivers/gpu/drm/panel/panel-simple.c | 60 +++++++-----------
drivers/gpu/drm/panel/panel-sitronix-st7703.c | 35 +++--------
drivers/gpu/drm/panel/panel-sony-acx565akm.c | 6 --
drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 23 -------
.../gpu/drm/panel/panel-xinpeng-xpp055c272.c | 28 ---------
25 files changed, 83 insertions(+), 841 deletions(-)
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFT PATCH v2 48/48] drm/panel: Update TODO list item for cleaning up prepared/enabled tracking
2024-05-03 21:32 [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Douglas Anderson
@ 2024-05-03 21:33 ` Douglas Anderson
2024-05-06 6:52 ` [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Linus Walleij
2024-05-06 7:27 ` Maxime Ripard
2 siblings, 0 replies; 7+ messages in thread
From: Douglas Anderson @ 2024-05-03 21:33 UTC (permalink / raw)
To: dri-devel, Maxime Ripard
Cc: Linus Walleij, Chris Morgan, Yuran Pereira, Neil Armstrong,
Douglas Anderson, Daniel Vetter, David Airlie, Jonathan Corbet,
Maarten Lankhorst, Thomas Zimmermann, linux-doc, linux-kernel
Now that most panels have been updated not to track/double-check their
prepared/enabled state update the TODO with next steps.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v1)
Documentation/gpu/todo.rst | 47 +++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index e2a0585915b3..4063bc45bbd3 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -469,30 +469,35 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
Level: Starter
-Clean up checks for already prepared/enabled in panels
-------------------------------------------------------
-
-In a whole pile of panel drivers, we have code to make the
-prepare/unprepare/enable/disable callbacks behave as no-ops if they've already
-been called. To get some idea of the duplicated code, try::
-
- git grep 'if.*>prepared' -- drivers/gpu/drm/panel
- git grep 'if.*>enabled' -- drivers/gpu/drm/panel
-
-In the patch ("drm/panel: Check for already prepared/enabled in drm_panel")
-we've moved this check to the core. Now we can most definitely remove the
-check from the individual panels and save a pile of code.
-
-In adition to removing the check from the individual panels, it is believed
-that even the core shouldn't need this check and that should be considered
-an error if other code ever relies on this check. The check in the core
-currently prints a warning whenever something is relying on this check with
-dev_warn(). After a little while, we likely want to promote this to a
-WARN(1) to help encourage folks not to rely on this behavior.
+Remove disable/unprepare in remove/shutdown in panel-simple and panel-edp
+-------------------------------------------------------------------------
+
+As of commit d2aacaf07395 ("drm/panel: Check for already prepared/enabled in
+drm_panel"), we have a check in the drm_panel core to make sure nobody
+double-calls prepare/enable/disable/unprepare. Eventually that should probably
+be turned into a WARN_ON() or somehow made louder, but right now we actually
+expect it to trigger and so we don't want it to be too loud.
+
+Specifically, that warning will trigger for panel-edp and panel-simple at
+shutdown time because those panels hardcode a call to drm_panel_disable()
+and drm_panel_unprepare() at shutdown and remove time that they call regardless
+of panel state. On systems with a properly coded DRM modeset driver that
+calls drm_atomic_helper_shutdown() this is pretty much guaranteed to cause
+the warning to fire.
+
+Unfortunately we can't safely remove the calls in panel-edp and panel-simple
+until we're sure that all DRM modeset drivers that are used with those panels
+properly call drm_atomic_helper_shutdown(). This TODO item is to validate
+that all DRM modeset drivers used with panel-edp and panel-simple properly
+call drm_atomic_helper_shutdown() and then remove the calls to
+disable/unprepare from those panels. Alternatively, this TODO item could be
+removed by convincing stakeholders that those calls are fine and downgrading
+the error message in drm_panel_disable() / drm_panel_unprepare() to a
+debug-level message.
Contact: Douglas Anderson <dianders@chromium.org>
-Level: Starter/Intermediate
+Level: Intermediate
Core refactorings
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state
2024-05-03 21:32 [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Douglas Anderson
2024-05-03 21:33 ` [RFT PATCH v2 48/48] drm/panel: Update TODO list item for cleaning up prepared/enabled tracking Douglas Anderson
@ 2024-05-06 6:52 ` Linus Walleij
2024-05-06 7:17 ` Ondřej Jirman
2024-05-08 21:14 ` Doug Anderson
2024-05-06 7:27 ` Maxime Ripard
2 siblings, 2 replies; 7+ messages in thread
From: Linus Walleij @ 2024-05-06 6:52 UTC (permalink / raw)
To: Douglas Anderson
Cc: dri-devel, Maxime Ripard, Chris Morgan, Yuran Pereira,
Neil Armstrong, AngeloGioacchino Del Regno, Daniel Vetter,
David Airlie, Guido Günther, Jerry Han, Jessica Zhang,
Jonathan Corbet, Maarten Lankhorst, Matthias Brugger,
Ondrej Jirman, Purism Kernel Team, Robert Chiras, Sam Ravnborg,
Stefan Mavrodiev, Sumit Semwal, Thomas Zimmermann,
linux-arm-kernel, linux-doc, linux-kernel, linux-mediatek
On Fri, May 3, 2024 at 11:36 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.
>
> 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@chromium.org
> [3] https://lore.kernel.org/r/20230901234202.566951-1-dianders@chromium.org
> [4] https://lore.kernel.org/r/20230921192749.1542462-1-dianders@chromium.org
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.
The series:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state
2024-05-06 6:52 ` [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Linus Walleij
@ 2024-05-06 7:17 ` Ondřej Jirman
2024-05-08 21:14 ` Doug Anderson
1 sibling, 0 replies; 7+ messages in thread
From: Ondřej Jirman @ 2024-05-06 7:17 UTC (permalink / raw)
To: Linus Walleij
Cc: Douglas Anderson, dri-devel, Maxime Ripard, Chris Morgan,
Yuran Pereira, Neil Armstrong, AngeloGioacchino Del Regno,
Daniel Vetter, David Airlie, Guido Günther, Jerry Han,
Jessica Zhang, Jonathan Corbet, Maarten Lankhorst,
Matthias Brugger, Purism Kernel Team, Robert Chiras, Sam Ravnborg,
Stefan Mavrodiev, Sumit Semwal, Thomas Zimmermann,
linux-arm-kernel, linux-doc, linux-kernel, linux-mediatek
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@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.
> >
> > 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@chromium.org
> > [3] https://lore.kernel.org/r/20230901234202.566951-1-dianders@chromium.org
> > [4] https://lore.kernel.org/r/20230921192749.1542462-1-dianders@chromium.org
>
> 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@linaro.org>
>
> Yours,
> Linus Walleij
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state
2024-05-03 21:32 [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Douglas Anderson
2024-05-03 21:33 ` [RFT PATCH v2 48/48] drm/panel: Update TODO list item for cleaning up prepared/enabled tracking Douglas Anderson
2024-05-06 6:52 ` [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Linus Walleij
@ 2024-05-06 7:27 ` Maxime Ripard
2 siblings, 0 replies; 7+ messages in thread
From: Maxime Ripard @ 2024-05-06 7:27 UTC (permalink / raw)
To: Douglas Anderson
Cc: dri-devel, linux-arm-kernel, linux-doc, linux-kernel,
linux-mediatek, AngeloGioacchino Del Regno, Chris Morgan,
Daniel Vetter, David Airlie, Guido Günther, Jerry Han,
Jessica Zhang, Jonathan Corbet, Linus Walleij, Maarten Lankhorst,
Matthias Brugger, Maxime Ripard, Neil Armstrong, Ondrej Jirman,
Purism Kernel Team, Robert Chiras, Sam Ravnborg, Stefan Mavrodiev,
Sumit Semwal, Thomas Zimmermann, Yuran Pereira
On Fri, 3 May 2024 14:32:41 -0700, 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
>
> [ ... ]
Acked-by: Maxime Ripard <mripard@kernel.org>
Thanks!
Maxime
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state
2024-05-06 6:52 ` [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Linus Walleij
2024-05-06 7:17 ` Ondřej Jirman
@ 2024-05-08 21:14 ` Doug Anderson
2024-05-28 20:14 ` Doug Anderson
1 sibling, 1 reply; 7+ messages in thread
From: Doug Anderson @ 2024-05-08 21:14 UTC (permalink / raw)
To: Linus Walleij
Cc: dri-devel, Maxime Ripard, Chris Morgan, Yuran Pereira,
Neil Armstrong, AngeloGioacchino Del Regno, Daniel Vetter,
David Airlie, Guido Günther, Jerry Han, Jessica Zhang,
Jonathan Corbet, Maarten Lankhorst, Matthias Brugger,
Ondrej Jirman, Purism Kernel Team, Robert Chiras, Sam Ravnborg,
Stefan Mavrodiev, Sumit Semwal, Thomas Zimmermann,
linux-arm-kernel, linux-doc, linux-kernel, linux-mediatek
Hi,
On Sun, May 5, 2024 at 11:52 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, May 3, 2024 at 11:36 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.
> >
> > 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@chromium.org
> > [3] https://lore.kernel.org/r/20230901234202.566951-1-dianders@chromium.org
> > [4] https://lore.kernel.org/r/20230921192749.1542462-1-dianders@chromium.org
>
> 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.
Yeah, I'd tend to agree.
> I would say we should just apply the series as soon as it's non-RFC
It's not actually RFC now, but "RFT" (request for testing). I don't
_think_ there's any need to send a version without the RFT tag before
landing unless someone really feels strongly about it.
> after the next merge window
With drm-misc there's not really any specific reason to wait for the
merge window to open/close as we can land in drm-misc-next at any time
regardless of the merge window. drm-misc-next will simply stop feeding
linuxnext for a while.
That all being said, I'm happy to delay landing this until after the
next -rc1 comes out if people would prefer that. If I don't hear
anything, I guess I'll just wait until -rc1 before landing any of
these.
> and see what happens. I doubt it
> will cause much trouble.
I can land the whole series if that's what everyone agrees on. As I
mentioned above, I'm at least slightly worried that I did something
stupid _somewhere_ in this series since no automation was possible and
with repetitive tasks like this it's super easy to flub something up.
It's _probably_ fine, but I guess I still have the worry in the back
of my mind.
If folks think I should just apply the whole series then I'm happy to
do that. If folks think I should just land parts of the series as they
are reviewed/tested I can do that as well. Let me know. If I don't
hear anything I'd tend to just land patches that are reviewed/tested.
Then after a month or so (hopefully) I'd send out a v2 with anything
left.
> The series:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Thanks!
-Doug
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state
2024-05-08 21:14 ` Doug Anderson
@ 2024-05-28 20:14 ` Doug Anderson
0 siblings, 0 replies; 7+ messages in thread
From: Doug Anderson @ 2024-05-28 20:14 UTC (permalink / raw)
To: Linus Walleij
Cc: dri-devel, Maxime Ripard, Chris Morgan, Yuran Pereira,
Neil Armstrong, AngeloGioacchino Del Regno, Daniel Vetter,
David Airlie, Guido Günther, Jessica Zhang, Jonathan Corbet,
Maarten Lankhorst, Matthias Brugger, Ondrej Jirman,
Purism Kernel Team, Robert Chiras, Sam Ravnborg, Sumit Semwal,
Thomas Zimmermann, linux-arm-kernel, linux-doc, linux-kernel,
linux-mediatek
Hi,
On Wed, May 8, 2024 at 2:14 PM Doug Anderson <dianders@chromium.org> wrote:
>
> > 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.
>
> Yeah, I'd tend to agree.
>
>
> > I would say we should just apply the series as soon as it's non-RFC
>
> It's not actually RFC now, but "RFT" (request for testing). I don't
> _think_ there's any need to send a version without the RFT tag before
> landing unless someone really feels strongly about it.
>
>
> > after the next merge window
>
> With drm-misc there's not really any specific reason to wait for the
> merge window to open/close as we can land in drm-misc-next at any time
> regardless of the merge window. drm-misc-next will simply stop feeding
> linuxnext for a while.
>
> That all being said, I'm happy to delay landing this until after the
> next -rc1 comes out if people would prefer that. If I don't hear
> anything, I guess I'll just wait until -rc1 before landing any of
> these.
>
>
> > and see what happens. I doubt it
> > will cause much trouble.
>
> I can land the whole series if that's what everyone agrees on. As I
> mentioned above, I'm at least slightly worried that I did something
> stupid _somewhere_ in this series since no automation was possible and
> with repetitive tasks like this it's super easy to flub something up.
> It's _probably_ fine, but I guess I still have the worry in the back
> of my mind.
>
> If folks think I should just apply the whole series then I'm happy to
> do that. If folks think I should just land parts of the series as they
> are reviewed/tested I can do that as well. Let me know. If I don't
> hear anything I'd tend to just land patches that are reviewed/tested.
> Then after a month or so (hopefully) I'd send out a v2 with anything
> left.
Nobody said anything, so I did what I indicated above:
1. I've applied all patches that someone responded to with Linus +
Maxime's Acks + any given tags. This includes the st7703 panels which
Ondřej replied to the cover letter about but didn't officially get any
tags.
2. I also applied patches for panels that I was personally involved
with. This includes panel-edp, panel-simple, samsung-atna33xc20,
boe-tv101wum-nl6.
Anything totally unresponded to I've left unapplied. I'll wait a
little while (at least a week) and then plan to send a v2 with
anything still outstanding. If someone sends Tested-by/Reviewed-by for
some panels in the meantime I'll apply them.
Here are the 25 patches applied to drm-misc-next:
[01/48] drm/panel: raydium-rm692e5: Stop tracking prepared
commit: 598dc42f25cc3060fd350db0f52af1075af3f500
[04/48] drm/panel: boe-tv101wum-nl6: Stop tracking prepared
commit: 3c24e31c908eb12e99420ff33b74c01f045253fe
[05/48] drm/panel: boe-tv101wum-nl6: Don't call unprepare+disable at
shutdown/remove
commit: 1985e3512b5a3777f6a18c36e40f3926037120bb
[06/48] drm/panel: edp: Stop tracking prepared/enabled
commit: 3904f317fd977533f6d7d3c4bfd75e0ac6169bb7
[07/48] drm/panel: edp: Add a comment about unprepare+disable at shutdown/remove
commit: ec7629859331fb67dbfb6bcd47f887a402e390ff
[08/48] drm/panel: innolux-p079zca: Stop tracking prepared/enabled
commit: f9055051292442d52092f17e191cf0a58d23d4ed
[09/48] drm/panel: innolux-p079zca: Don't call unprepare+disable at
shutdown/remove
commit: eeb133ff78476eb1e6e88154dfb75a741e8a034a
[12/48] drm/panel: kingdisplay-kd097d04: Stop tracking prepared/enabled
commit: 157c1381780a453e06430f8b35bb8c5d439eb8c6
[13/48] drm/panel: kingdisplay-kd097d04: Don't call unprepare+disable
at shutdown/remove
commit: 68c205ef3c39edce4a3346b8a53fd2b700394a0c
[14/48] drm/panel: ltk050h3146w: Stop tracking prepared
commit: f124478dd18c519544489caddce78e7c5796a758
[15/48] drm/panel: ltk050h3146w: Don't call unprepare+disable at shutdown/remove
commit: b7ca446ecb53205944968617b158f073bcacaedc
[16/48] drm/panel: ltk500hd1829: Stop tracking prepared
commit: 2b8c19b9d7bc9d03e8c44bd391d21e95c07a2c83
[17/48] drm/panel: ltk500hd1829: Don't call unprepare+disable at shutdown/remove
commit: 3357f6f465e62c0bc5e906365063734740c9f6d4
[18/48] drm/panel: novatek-nt36672a: Stop tracking prepared
commit: b605f257f386b7f4b6fc9c0f82b86b75d0579287
[19/48] drm/panel: novatek-nt36672a: Don't call unprepare+disable at
shutdown/remove
commit: 2a9487b5aa55753993fde80e4841128c8da4df71
[24/48] drm/panel: samsung-atna33xc20: Stop tracking prepared/enabled
commit: 5a847750aac8454a1604070ab99d689c0a6e4290
[25/48] drm/panel: samsung-atna33xc20: Don't call unprepare+disable at
shutdown/remove
commit: 49869668ff0e3f380858b4c20b8d0cb02b933f48
[26/48] drm/panel: simple: Stop tracking prepared/enabled
commit: 2a1c99d7159b798288bfb20a76c1e665e2344126
[27/48] drm/panel: simple: Add a comment about unprepare+disable at
shutdown/remove
commit: bc62654df3c888dec735343f5db9907ac93aea60
[30/48] drm/panel: xinpeng-xpp055c272: Stop tracking prepared
commit: 4e5e6fa77a9d40cdf85ade7f86d07dc8929941c9
[31/48] drm/panel: xinpeng-xpp055c272: Don't call unprepare+disable at
shutdown/remove
commit: ac9e1786271f771ff1f774742602330be2d57a12
[42/48] drm/panel: sitronix-st7703: Stop tracking prepared
commit: 3004d2e9cca5d59d25dff670a03a005d40601ded
[43/48] drm/panel: sitronix-st7703: Don't call disable at shutdown/remove
commit: 718bd8a1a5ee873778a72523c06da054a89108b4
[46/48] drm/panel: sony-acx565akm: Don't double-check enabled state in disable
commit: e28df86aeeff0b84c13e676f641ea879abbdb809
[47/48] drm/panel: sony-acx565akm: Don't call disable at remove
commit: 6afebd850d1ab5518c273b32532f0b2086cc633a
-Doug
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-05-28 20:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-03 21:32 [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Douglas Anderson
2024-05-03 21:33 ` [RFT PATCH v2 48/48] drm/panel: Update TODO list item for cleaning up prepared/enabled tracking Douglas Anderson
2024-05-06 6:52 ` [RFT PATCH v2 00/48] drm/panel: Remove most store/double-check of prepared/enabled state Linus Walleij
2024-05-06 7:17 ` Ondřej Jirman
2024-05-08 21:14 ` Doug Anderson
2024-05-28 20:14 ` Doug Anderson
2024-05-06 7:27 ` Maxime Ripard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).