From 755d20ebc44759315f1602813e0ff6caade92168 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Wed, 19 Jul 2023 17:23:34 +0200 Subject: drm/todo: Add atomic modesetting references The section about converting existing KMS drivers to atomic modesetting mentions the existence of a conversion guide, but does not reference it. While the guide is old and rusty, it still contains useful information, so add a link to it. Also link to the LWN.net articles that give an overview about the atomic mode setting design. While at it, remove the reference to unconverted virtual HW drivers, as they've been converted. Signed-off-by: Geert Uytterhoeven Reviewed-by: Javier Martinez Canillas Reviewed-by: Laurent Pinchart Signed-off-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/6809d0fda0716892cbccf0ee272481032251026d.1689779916.git.geert+renesas@glider.be --- Documentation/gpu/todo.rst | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'Documentation/gpu') diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index d0f9f87ea515..60942f00768b 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -49,14 +49,18 @@ converted over. Modern compositors like Wayland or Surfaceflinger on Android really want an atomic modeset interface, so this is all about the bright future. -There is a conversion guide for atomic and all you need is a GPU for a -non-converted driver (again virtual HW drivers for KVM are still all -suitable). +There is a conversion guide for atomic [1]_ and all you need is a GPU for a +non-converted driver. The "Atomic mode setting design overview" series [2]_ +[3]_ at LWN.net can also be helpful. As part of this drivers also need to convert to universal plane (which means exposing primary & cursor as proper plane objects). But that's much easier to do by directly using the new atomic helper driver callbacks. + .. [1] https://blog.ffwll.ch/2014/11/atomic-modeset-support-for-kms-drivers.html + .. [2] https://lwn.net/Articles/653071/ + .. [3] https://lwn.net/Articles/653466/ + Contact: Daniel Vetter, respective driver maintainers Level: Advanced -- cgit v1.2.3 From 66f9f216460d8442356e0fec05421bc53c850362 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Wed, 19 Jul 2023 17:23:35 +0200 Subject: drm/todo: Convert list of fbconv links to footnotes Convert the references to fbconv links to footnotes, so they can be navigated. Signed-off-by: Geert Uytterhoeven Acked-by: Javier Martinez Canillas Signed-off-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/0761f98d3b6f8df9eea977eae063e35b450fda9e.1689779916.git.geert+renesas@glider.be --- Documentation/gpu/todo.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'Documentation/gpu') diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 60942f00768b..02de2ff6b84f 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -757,16 +757,16 @@ existing hardware. The new driver's call-back functions are filled from existing fbdev code. More complex fbdev drivers can be refactored step-by-step into a DRM -driver with the help of the DRM fbconv helpers. [1] These helpers provide +driver with the help of the DRM fbconv helpers [4]_. These helpers provide the transition layer between the DRM core infrastructure and the fbdev driver interface. Create a new DRM driver on top of the fbconv helpers, copy over the fbdev driver, and hook it up to the DRM code. Examples for -several fbdev drivers are available at [1] and a tutorial of this process -available at [2]. The result is a primitive DRM driver that can run X11 -and Weston. +several fbdev drivers are available in Thomas Zimmermann's fbconv tree +[4]_, as well as a tutorial of this process [5]_. The result is a primitive +DRM driver that can run X11 and Weston. - - [1] https://gitlab.freedesktop.org/tzimmermann/linux/tree/fbconv - - [2] https://gitlab.freedesktop.org/tzimmermann/linux/blob/fbconv/drivers/gpu/drm/drm_fbconv_helper.c + .. [4] https://gitlab.freedesktop.org/tzimmermann/linux/tree/fbconv + .. [5] https://gitlab.freedesktop.org/tzimmermann/linux/blob/fbconv/drivers/gpu/drm/drm_fbconv_helper.c Contact: Thomas Zimmermann -- cgit v1.2.3 From d2aacaf07395bd798373cbec6af05fff4147aff3 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Thu, 27 Jul 2023 10:16:29 -0700 Subject: 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 Reviewed-by: Maxime Ripard Signed-off-by: Douglas Anderson Link: https://patchwork.freedesktop.org/patch/msgid/20230727101636.v4.2.I59b417d4c29151cc2eff053369ec4822b606f375@changeid --- Documentation/gpu/todo.rst | 24 ++++++++++++++++++++++ drivers/gpu/drm/drm_panel.c | 49 +++++++++++++++++++++++++++++++++++++++------ include/drm/drm_panel.h | 14 +++++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-) (limited to 'Documentation/gpu') 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 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 + +Level: Starter/Intermediate + Core refactorings ================= diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index f634371c717a..4e1c4e42575b 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -105,11 +105,22 @@ EXPORT_SYMBOL(drm_panel_remove); */ int drm_panel_prepare(struct drm_panel *panel) { + int ret; + if (!panel) return -EINVAL; - if (panel->funcs && panel->funcs->prepare) - return panel->funcs->prepare(panel); + if (panel->prepared) { + dev_warn(panel->dev, "Skipping prepare of already prepared panel\n"); + return 0; + } + + if (panel->funcs && panel->funcs->prepare) { + ret = panel->funcs->prepare(panel); + if (ret < 0) + return ret; + } + panel->prepared = true; return 0; } @@ -128,11 +139,22 @@ EXPORT_SYMBOL(drm_panel_prepare); */ int drm_panel_unprepare(struct drm_panel *panel) { + int ret; + if (!panel) return -EINVAL; - if (panel->funcs && panel->funcs->unprepare) - return panel->funcs->unprepare(panel); + if (!panel->prepared) { + dev_warn(panel->dev, "Skipping unprepare of already unprepared panel\n"); + return 0; + } + + if (panel->funcs && panel->funcs->unprepare) { + ret = panel->funcs->unprepare(panel); + if (ret < 0) + return ret; + } + panel->prepared = false; return 0; } @@ -155,11 +177,17 @@ int drm_panel_enable(struct drm_panel *panel) if (!panel) return -EINVAL; + if (panel->enabled) { + dev_warn(panel->dev, "Skipping enable of already enabled panel\n"); + return 0; + } + if (panel->funcs && panel->funcs->enable) { ret = panel->funcs->enable(panel); if (ret < 0) return ret; } + panel->enabled = true; ret = backlight_enable(panel->backlight); if (ret < 0) @@ -187,13 +215,22 @@ int drm_panel_disable(struct drm_panel *panel) if (!panel) return -EINVAL; + if (!panel->enabled) { + dev_warn(panel->dev, "Skipping disable of already disabled panel\n"); + return 0; + } + ret = backlight_disable(panel->backlight); if (ret < 0) DRM_DEV_INFO(panel->dev, "failed to disable backlight: %d\n", ret); - if (panel->funcs && panel->funcs->disable) - return panel->funcs->disable(panel); + if (panel->funcs && panel->funcs->disable) { + ret = panel->funcs->disable(panel); + if (ret < 0) + return ret; + } + panel->enabled = false; return 0; } diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 432fab2347eb..c6cf75909389 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -198,6 +198,20 @@ struct drm_panel { * the panel is powered up. */ bool prepare_prev_first; + + /** + * @prepared: + * + * If true then the panel has been prepared. + */ + bool prepared; + + /** + * @enabled: + * + * If true then the panel has been enabled. + */ + bool enabled; }; void drm_panel_init(struct drm_panel *panel, struct device *dev, -- cgit v1.2.3 From 1ab2ddc4afdd84632c24b23dbe67eb4ca423dcc5 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Wed, 2 Aug 2023 07:47:28 -0700 Subject: drm/panel: Fix todo indentation for panel prepared/enabled cleanup In commit d2aacaf07395 ("drm/panel: Check for already prepared/enabled in drm_panel") the formatting for a code block was not quite right. This caused an error when building htmldocs: Documentation/gpu/todo.rst:469: ERROR: Unexpected indentation. Fix the error by using the proper syntax for a code block. Fixes: d2aacaf07395 ("drm/panel: Check for already prepared/enabled in drm_panel") Reported-by: Stephen Rothwell Closes: https://lore.kernel.org/r/20230802141724.0edce253@canb.auug.org.au Signed-off-by: Douglas Anderson Reviewed-by: Neil Armstrong Signed-off-by: Neil Armstrong Link: https://patchwork.freedesktop.org/patch/msgid/20230802074727.2.Iaeb7b0f7951aee6b8c090364bbc87b1ae198a857@changeid --- Documentation/gpu/todo.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'Documentation/gpu') diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index aa0052f9b93b..139980487ccf 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -465,7 +465,8 @@ 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: +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 -- cgit v1.2.3 From 9a2eabf48ade4fbadb54b95dc1ece429b1cce400 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Thu, 3 Aug 2023 09:57:39 +0000 Subject: drm/doc: use proper cross-references for sections When I originally wrote these docs, I couldn't manage to insert a cross-reference to a section. Here's how it can be done. Signed-off-by: Simon Ser Reviewed-by: Jani Nikula Acked-by: Daniel Vetter Cc: Pekka Paalanen Link: https://patchwork.freedesktop.org/patch/msgid/20230803095734.386761-1-contact@emersion.fr --- Documentation/gpu/drm-mm.rst | 2 ++ include/uapi/drm/drm.h | 9 ++++----- 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'Documentation/gpu') diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index 3d5dc9dc1bfe..513197359aba 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -517,6 +517,8 @@ DRM Cache Handling and Fast WC memcpy() .. kernel-doc:: drivers/gpu/drm/drm_cache.c :export: +.. _drm_sync_objects: + DRM Sync Objects =========================== diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 863e47200911..75ec985d95e5 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -673,8 +673,8 @@ struct drm_gem_open { * Bitfield of supported PRIME sharing capabilities. See &DRM_PRIME_CAP_IMPORT * and &DRM_PRIME_CAP_EXPORT. * - * PRIME buffers are exposed as dma-buf file descriptors. See - * Documentation/gpu/drm-mm.rst, section "PRIME Buffer Sharing". + * PRIME buffers are exposed as dma-buf file descriptors. + * See :ref:`prime_buffer_sharing`. */ #define DRM_CAP_PRIME 0x5 /** @@ -756,15 +756,14 @@ struct drm_gem_open { /** * DRM_CAP_SYNCOBJ * - * If set to 1, the driver supports sync objects. See - * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects". + * If set to 1, the driver supports sync objects. See :ref:`drm_sync_objects`. */ #define DRM_CAP_SYNCOBJ 0x13 /** * DRM_CAP_SYNCOBJ_TIMELINE * * If set to 1, the driver supports timeline operations on sync objects. See - * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects". + * :ref:`drm_sync_objects`. */ #define DRM_CAP_SYNCOBJ_TIMELINE 0x14 -- cgit v1.2.3