[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250902210445.GX13448@pendragon.ideasonboard.com>
Date: Tue, 2 Sep 2025 23:04:45 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Maxime Ripard <mripard@...nel.org>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>,
Robert Foss <rfoss@...nel.org>, Jonas Karlman <jonas@...boo.se>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Jyri Sarha <jyri.sarha@....fi>,
Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
Devarsh Thakkar <devarsht@...com>, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 09/29] drm/modeset: Create atomic_reset hook
On Tue, Sep 02, 2025 at 10:32:37AM +0200, Maxime Ripard wrote:
> Since we're about to integrate some infrastructure to implement hardware
> state readout, we need a way to differentiate between drivers wanting to
> start from a pristine state, with the classic reset sequence, and
> drivers that want to pickup their initial state from reading out the
> hardware state.
>
> To do so we can create an optional reset hook in
> drm_mode_config_helper_funcs that will default to the classic reset
> implementation, and can be setup to a helper we will provide in a later
> patch to read the hardware state.
I'm a bit puzzled by this. Isn't the whole point of the existing reset
operations to allow drivers to read out the hardware state if they wish
? Why do we need something new ?
> Signed-off-by: Maxime Ripard <mripard@...nel.org>
> ---
> drivers/gpu/drm/drm_mode_config.c | 32 +++++++++++++++++++++++---------
> include/drm/drm_modeset_helper_vtables.h | 13 +++++++++++++
> 2 files changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 25f376869b3a41d47bbe72b0df3e35cad142f3e6..82180760032d3490d63fe83136465d2c26551d08 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -27,10 +27,11 @@
> #include <drm/drm_encoder.h>
> #include <drm/drm_file.h>
> #include <drm/drm_framebuffer.h>
> #include <drm/drm_managed.h>
> #include <drm/drm_mode_config.h>
> +#include <drm/drm_modeset_helper_vtables.h>
> #include <drm/drm_print.h>
> #include <linux/dma-resv.h>
>
> #include "drm_crtc_internal.h"
> #include "drm_internal.h"
> @@ -179,19 +180,11 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
> drm_connector_list_iter_end(&conn_iter);
>
> return ret;
> }
>
> -/**
> - * drm_mode_config_reset - call ->reset callbacks
> - * @dev: drm device
> - *
> - * This functions calls all the crtc's, encoder's and connector's ->reset
> - * callback. Drivers can use this in e.g. their driver load or resume code to
> - * reset hardware and software state.
> - */
> -void drm_mode_config_reset(struct drm_device *dev)
> +static void drm_mode_config_reset_pristine(struct drm_device *dev)
> {
> struct drm_crtc *crtc;
> struct drm_plane *plane;
> struct drm_encoder *encoder;
> struct drm_connector *connector;
> @@ -213,10 +206,31 @@ void drm_mode_config_reset(struct drm_device *dev)
> drm_for_each_connector_iter(connector, &conn_iter)
> if (connector->funcs->reset)
> connector->funcs->reset(connector);
> drm_connector_list_iter_end(&conn_iter);
> }
> +
> +/**
> + * drm_mode_config_reset - call ->reset callbacks
> + * @dev: drm device
> + *
> + * This functions calls all the crtc's, encoder's and connector's ->reset
> + * callback. Drivers can use this in e.g. their driver load or resume code to
> + * reset hardware and software state.
> + */
> +void drm_mode_config_reset(struct drm_device *dev)
> +{
> + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> + const struct drm_mode_config_helper_funcs *funcs =
> + dev->mode_config.helper_private;
> +
> + if (funcs && funcs->atomic_reset)
> + return funcs->atomic_reset(dev);
> + }
> +
> + return drm_mode_config_reset_pristine(dev);
> +}
> EXPORT_SYMBOL(drm_mode_config_reset);
>
> /*
> * Global properties
> */
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index ce7c7aeac887bb8438d73710f16071c97a851839..6d22a7676d6bf49fb78af4d0706bd91005cef186 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -1561,8 +1561,21 @@ struct drm_mode_config_helper_funcs {
> * how one should implement this.
> *
> * This hook is optional.
> */
> int (*atomic_commit_setup)(struct drm_atomic_state *state);
> +
> + /**
> + * @atomic_reset:
> + *
> + * This hook is used to create the initial @drm_atomic_state.
> + * It's used by drm_mode_config_reset().
> + *
> + * The default implementation will create an empty one, but
> + * drivers can provide an alternative implementation to, for
> + * example, read the initial state from hardware to implement
> + * flicker-free and / or faster boot.
> + */
> + void (*atomic_reset)(struct drm_device *dev);
> };
>
> #endif
>
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists