On Thu, Apr 23, 2026 at 12:18:24PM +0200, Maxime Ripard wrote:
> The hardware state readout is useful, but might need to be disabled
> in case of bugs, or its checks relaxed during development when not
> all hooks are implemented yet.
>
> Add a module parameter to control the readout behavior: it can be
> disabled entirely, or the checks for missing compare or readout hooks
> can be skipped independently.
>
> Suggested-by: Simona Vetter <[email protected]>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
> drivers/gpu/drm/drm_atomic_sro.c | 36 ++++++++++++++++++++++++++++++++++++
> include/drm/drm_atomic_sro.h | 2 ++
> 2 files changed, 38 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_sro.c
> b/drivers/gpu/drm/drm_atomic_sro.c
> index 177b97d451f5..a46f06e75c4e 100644
> --- a/drivers/gpu/drm/drm_atomic_sro.c
> +++ b/drivers/gpu/drm/drm_atomic_sro.c
> @@ -11,10 +11,46 @@
> #include <linux/module.h>
>
> #include "drm_internal.h"
> #include "drm_crtc_internal.h"
>
> +enum drm_atomic_readout_status {
> + DRM_ATOMIC_READOUT_DISABLED = 0,
> + DRM_ATOMIC_READOUT_ENABLED,
> + DRM_ATOMIC_READOUT_SKIP_MISSING_COMPARE,
> + DRM_ATOMIC_READOUT_SKIP_MISSING_READOUT,
> +};
> +
> +static unsigned int atomic_readout = DRM_ATOMIC_READOUT_ENABLED;
> +module_param_unsafe(atomic_readout, uint, 0);
Default is actually 0 here. I agree with the docs that it should be 1,
since drivers have an explicit opt-in through setting the main entry point
in drm_mode_config_funcs.
I was also pondering whether we should have a compare-only mode, but the
issue is that once you build a driver on readout being a thing, that could
blow up. So I think that should be left as a per-driver tunable.
That's also why this must be a unsafe debug option, it might actually
break the driver.
> +MODULE_PARM_DESC(atomic_readout,
> + "Enable Hardware State Readout (0 = disabled, 1 = enabled, 2 =
> ignore missing compares, 3 = ignore missing readouts and compares, default =
> 1)");
> +
> +/**
> + * drm_atomic_sro_device_can_readout - check if a device supports hardware
> state readout
> + * @dev: DRM device to check
> + *
> + * Verifies that the device is an atomic driver, that readout is
> + * enabled, and that all KMS objects implement the relevant hooks.
> + *
> + * RETURNS:
> + *
> + * True if the device supports full hardware state readout, false
> + * otherwise.
> + */
> +bool drm_atomic_sro_device_can_readout(struct drm_device *dev)
> +{
> + if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
I think this should be drm_drv_uses_atomic_modeset() since it's an
internal check, not an uapi check.
With the two issues addressed:
Reviewed-by: Simona Vetter <[email protected]>
> + return false;
> +
> + if (atomic_readout == DRM_ATOMIC_READOUT_DISABLED)
> + return false;
> +
> + return true;
> +}
> +EXPORT_SYMBOL(drm_atomic_sro_device_can_readout);
> +
> struct __drm_atomic_sro_plane {
> struct drm_plane *ptr;
> struct drm_plane_state *state;
> };
>
> diff --git a/include/drm/drm_atomic_sro.h b/include/drm/drm_atomic_sro.h
> index 5a9333a05796..6e5262384c71 100644
> --- a/include/drm/drm_atomic_sro.h
> +++ b/include/drm/drm_atomic_sro.h
> @@ -13,10 +13,12 @@ struct drm_plane;
> struct drm_plane_state;
> struct drm_printer;
> struct drm_private_obj;
> struct drm_private_state;
>
> +bool drm_atomic_sro_device_can_readout(struct drm_device *dev);
> +
> struct drm_atomic_sro_state *drm_atomic_sro_state_alloc(struct drm_device
> *dev);
> void drm_atomic_sro_state_free(struct drm_atomic_sro_state *state);
> void drm_atomic_sro_state_print(const struct drm_atomic_sro_state *state,
> struct drm_printer *p);
>
>
> --
> 2.53.0
>
--
Simona Vetter
Software Engineer
http://blog.ffwll.ch
