summaryrefslogtreecommitdiff
path: root/Documentation/gpu
diff options
context:
space:
mode:
authorDouglas Anderson <dianders@chromium.org>2023-07-27 10:16:29 -0700
committerDouglas Anderson <dianders@chromium.org>2023-08-01 07:35:36 -0700
commitd2aacaf07395bd798373cbec6af05fff4147aff3 (patch)
treea70aa3b04c5c53ab75e3d6d118059e0f7fa35b98 /Documentation/gpu
parent2ca376ef18f6bc36a1bc9d53a19026e5fc16ab76 (diff)
drm/panel: Check for already prepared/enabled in drm_panel
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. It's silly to have this code duplicated everywhere. Add it to the core instead so that we can eventually delete it from all the drivers. Note: 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 NOTE: arguably, the right thing to do here is actually to skip this patch and simply remove all the extra checks from the individual drivers. Perhaps the checks were needed at some point in time in the past but maybe they no longer are? Certainly as we continue transitioning over to "panel_bridge" then we expect there to be much less variety in how these calls are made. When we're called as part of the bridge chain, things should be pretty simple. In fact, there was some discussion in the past about these checks [1], including a discussion about whether the checks were needed and whether the calls ought to be refcounted. At the time, I decided not to mess with it because it felt too risky. Looking closer at it now, I'm fairly certain that nothing in the existing codebase is expecting these calls to be refcounted. The only real question is whether someone is already doing something to ensure prepare()/unprepare() match and enabled()/disable() match. I would say that, even if there is something else ensuring that things match, there's enough complexity that adding an extra bool and an extra double-check here is a good idea. Let's add a drm_warn() to let people know that it's considered a minor error to take advantage of drm_panel's double-checking but we'll still make things work fine. We'll also add an entry to the official DRM todo list to remove the now pointless check from the panels after this patch lands and, eventually, fixup anyone who is triggering the new warning. [1] https://lore.kernel.org/r/20210416153909.v4.27.I502f2a92ddd36c3d28d014dd75e170c2d405a0a5@changeid Acked-by: Neil Armstrong <neil.armstrong@linaro.org> Reviewed-by: Maxime Ripard <mripard@kernel.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Link: https://patchwork.freedesktop.org/patch/msgid/20230727101636.v4.2.I59b417d4c29151cc2eff053369ec4822b606f375@changeid
Diffstat (limited to 'Documentation/gpu')
-rw-r--r--Documentation/gpu/todo.rst24
1 files changed, 24 insertions, 0 deletions
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 02de2ff6b84f..aa0052f9b93b 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -460,6 +460,30 @@ 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.
+
+Contact: Douglas Anderson <dianders@chromium.org>
+
+Level: Starter/Intermediate
+
Core refactorings
=================