Re: [PATCH 09/29] drm/modeset: Create atomic_reset hook

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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@kernel.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