Date: Tue, 11 Jun 2024 09:56:13 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Douglas Anderson <dianders@...omium.org>,
 dri-devel@...ts.freedesktop.org, Maxime Ripard <mripard@...nel.org>
Cc: Linus Walleij <linus.walleij@...aro.org>,
 Chris Morgan <macromorgan@...mail.com>,
 Yuran Pereira <yuran.pereira@...mail.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...il.com>,
 Jerry Han <hanxu5@...qin.corp-partner.google.com>,
 Jessica Zhang <quic_jesszhan@...cinc.com>, Jonathan Corbet <corbet@....net>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Matthias Brugger <matthias.bgg@...il.com>,
 Robert Chiras <robert.chiras@....com>, Stefan Mavrodiev <stefan@...mex.com>,
 Thomas Zimmermann <tzimmermann@...e.de>,
 linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v3 00/24] drm/panel: Remove most store/double-check of
 prepared/enabled state

Hi,

On 05/06/2024 02:22, 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.
> 
> 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.
> 
> Many of the patches in the V2 series [5] landed, so this V3 series is
> the patches that are left plus one new bonus patch. At this point, we
> may want to just land the patches that are left since it seems like
> nobody is going to test/review them and they've all been Acked by
> Linus and Maxime.
> 
> 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
> [5] https://lore.kernel.org/r/20240503213441.177109-1-dianders@chromium.org/
> 
> Changes in v3:
> - drm/panel: Avoid warnings w/ panel-simple/panel-edp at shutdown
> 
> Changes in v2:
> - Only handle 1 panel per patch.
> - Split removal of prepared/enabled from handling of remove/shutdown.
> 
> Douglas Anderson (24):
>    drm/panel: boe-himax8279d: Stop tracking prepared/enabled
>    drm/panel: boe-himax8279d: 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: 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: tdo-tl070wsh30: Stop tracking prepared
>    drm/panel: tdo-tl070wsh30: 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: raydium-rm67191: Stop tracking enabled
>    drm/panel: raydium-rm67191: Don't call unprepare+disable at shutdown
>    drm/panel: Update TODO list item for cleaning up prepared/enabled
>      tracking
>    drm/panel: Avoid warnings w/ panel-simple/panel-edp at shutdown
> 
>   Documentation/gpu/todo.rst                    |  47 +++---
>   drivers/gpu/drm/drm_panel.c                   |  12 ++
>   drivers/gpu/drm/panel/panel-boe-himax8279d.c  |  40 -----
>   .../gpu/drm/panel/panel-drm-shutdown-check.h  | 151 ++++++++++++++++++
>   drivers/gpu/drm/panel/panel-edp.c             |  19 +--
>   .../gpu/drm/panel/panel-jdi-lt070me05000.c    |  35 ----
>   drivers/gpu/drm/panel/panel-khadas-ts050.c    |  39 -----
>   .../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-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          |  19 +--
>   drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c  |  23 ---
>   16 files changed, 199 insertions(+), 480 deletions(-)
>   create mode 100644 drivers/gpu/drm/panel/panel-drm-shutdown-check.h
> 


Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>

for all patches, let me apply all but the last one and let us a more
week to review it. Could you resend it as standalone patch?

Thanks,
Neil

Powered by blists - more mailing lists