drm: docs: Remove item from TODO list

Commit Message

Yuran Pereira Oct. 23, 2023, 4:30 p.m. UTC

Since "Clean up checks for already prepared/enabled in panels" has
already been done and merged [1], I think there is no longer a need
for this item to be in the gpu TODO.

[1] https://patchwork.freedesktop.org/patch/551421/

Signed-off-by: Yuran Pereira <[e-mail]>
---
 Documentation/gpu/todo.rst | 25 -------------------------
 1 file changed, 25 deletions(-)

Comments

Doug Anderson Oct. 23, 2023, 5:25 p.m. UTC | #1

Hi,

On Mon, Oct 23, 2023 at 9:31 AM Yuran Pereira <[e-mail]> wrote:
>
> Since "Clean up checks for already prepared/enabled in panels" has
> already been done and merged [1], I think there is no longer a need
> for this item to be in the gpu TODO.
>
> [1] https://patchwork.freedesktop.org/patch/551421/
>
> Signed-off-by: Yuran Pereira <[e-mail]>
> ---
>  Documentation/gpu/todo.rst | 25 -------------------------
>  1 file changed, 25 deletions(-)

It's not actually all done. It's in a bit of a limbo state right now,
unfortunately. I landed all of the "simple" cases where panels were
needlessly tracking prepare/enable, but the less simple cases are
still outstanding.

Specifically the issue is that many panels have code to properly power
cycle themselves off at shutdown time and in order to do that they
need to keep track of the prepare/enable state. After a big, long
discussion [1] it was decided that we could get rid of all the panel
code handling shutdown if only all relevant DRM KMS drivers would
properly call drm_atomic_helper_shutdown().

I made an attempt to get DRM KMS drivers to call
drm_atomic_helper_shutdown() [2] [3] [4]. I was able to land the
patches that went through drm-misc, but currently many of the
non-drm-misc ones are blocked waiting for attention.

...so things that could be done to help out:

a) Could review patches that haven't landed in [4]. Maybe adding a
Reviewed-by tag would help wake up maintainers?

b) Could see if you can identify panels that are exclusively used w/
DRM drivers that have already been converted and then we could post
patches for just those panels. I have no idea how easy this task would
be. Is it enough to look at upstream dts files by "compatible" string?

I've gotten side-tracked at the moment so I haven't been driving
progress on this very quickly. I still plan to poke some of the
drm_atomic_helper_shutdown() patches eventually...


[1] https://lore.kernel.org/r/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid
[2] https://lore.kernel.org/r/[e-mail]
[3] https://lore.kernel.org/r/[e-mail]
[4] https://lore.kernel.org/r/[e-mail]

Maxime Ripard Oct. 24, 2023, 2:24 p.m. UTC | #2

Hi,

On Mon, Oct 23, 2023 at 10:25:50AM -0700, Doug Anderson wrote:
> On Mon, Oct 23, 2023 at 9:31 AM Yuran Pereira <[e-mail]> wrote:
> >
> > Since "Clean up checks for already prepared/enabled in panels" has
> > already been done and merged [1], I think there is no longer a need
> > for this item to be in the gpu TODO.
> >
> > [1] https://patchwork.freedesktop.org/patch/551421/
> >
> > Signed-off-by: Yuran Pereira <[e-mail]>
> > ---
> >  Documentation/gpu/todo.rst | 25 -------------------------
> >  1 file changed, 25 deletions(-)
> 
> It's not actually all done. It's in a bit of a limbo state right now,
> unfortunately. I landed all of the "simple" cases where panels were
> needlessly tracking prepare/enable, but the less simple cases are
> still outstanding.
> 
> Specifically the issue is that many panels have code to properly power
> cycle themselves off at shutdown time and in order to do that they
> need to keep track of the prepare/enable state. After a big, long
> discussion [1] it was decided that we could get rid of all the panel
> code handling shutdown if only all relevant DRM KMS drivers would
> properly call drm_atomic_helper_shutdown().
> 
> I made an attempt to get DRM KMS drivers to call
> drm_atomic_helper_shutdown() [2] [3] [4]. I was able to land the
> patches that went through drm-misc, but currently many of the
> non-drm-misc ones are blocked waiting for attention.
> 
> ...so things that could be done to help out:
> 
> a) Could review patches that haven't landed in [4]. Maybe adding a
> Reviewed-by tag would help wake up maintainers?
> 
> b) Could see if you can identify panels that are exclusively used w/
> DRM drivers that have already been converted and then we could post
> patches for just those panels. I have no idea how easy this task would
> be. Is it enough to look at upstream dts files by "compatible" string?

I think it is, yes.

Maxime

Patch

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 03fe5d1247be..280020b0ad4d 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -460,31 +460,6 @@  Contact: Thomas Zimmermann <[e-mail]>
 
 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.
-
-Contact: Douglas Anderson <[e-mail]>
-
-Level: Starter/Intermediate
-
 
 Core refactorings
 =================