Re: [PATCH v3 09/11] drm/atomic-uapi: Add DRM_MODE_ATOMIC_RESET flag

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 08.09.26 um 16:35 schrieb Maxime Ripard:
[...]
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 0dbf04d4aa9e..f13bb7c490c4 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -303,10 +303,13 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>   		break;
>   	case DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP:
>   		req->value = drm_core_check_feature(dev, DRIVER_ATOMIC) &&
>   			     dev->mode_config.async_page_flip;
>   		break;
> +	case DRM_CAP_ATOMIC_RESET:
> +		req->value = drm_atomic_can_create_state(dev);
> +		break;

Looking at this and the other places where _can_create_state is being 
used, I'd like to present a different design.

Scratch the helper entirely and introduce a dedicated callback in 
drm_mode_config_funcs that sets up the default state.  Your current 
helper drm_atomic_commit_fill_with_defaults would be the common 
implementation. The DRM core could test for the existence of this 
callback to see if default-reset is available. Sure, we'd have to modify 
all drivers, but it would be architecturally cleaner IMHO and give full 
control to the drivers. Best regards Thomas
>   	default:
>   		return -EINVAL;
>   	}
>   	return 0;
>   }
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index bc7ef7684099..b6e2f2edd122 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -793,10 +793,20 @@ struct drm_gem_change_handle {
>    * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for atomic
>    * commits.
>    */
>   #define DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP	0x15
>   
> +/**
> + * DRM_CAP_ATOMIC_RESET
> + *
> + * If set to 1, the driver supports the &DRM_MODE_ATOMIC_RESET flag in
> + * &DRM_IOCTL_MODE_ATOMIC commits. When supported, userspace can pass that
> + * flag to reset all KMS object states to their defaults before applying
> + * property changes.
> + */
> +#define DRM_CAP_ATOMIC_RESET	0x16
> +
>   /* DRM_IOCTL_GET_CAP ioctl argument type */
>   struct drm_get_cap {
>   	__u64 capability;
>   	__u64 value;
>   };
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index ed1a660a3dfd..32eb3f650d7a 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -1326,10 +1326,21 @@ struct drm_mode_destroy_dumb {
>    * To the best of the driver's knowledge, visual artifacts are guaranteed to
>    * not appear when this flag is not set. Some sinks might display visual
>    * artifacts outside of the driver's control.
>    */
>   #define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
> +/**
> + * DRM_MODE_ATOMIC_RESET
> + *
> + * Reset all KMS object states (CRTCs, planes, connectors, color operations)
> + * to their default values before applying the properties in this commit.
> + * Properties not explicitly included in the commit will remain at their
> + * defaults (CRTCs inactive, planes disabled, connectors unbound, etc.).
> + *
> + * This flag cannot be combined with &DRM_MODE_PAGE_FLIP_ASYNC.
> + */
> +#define DRM_MODE_ATOMIC_RESET 0x0800
>   
>   /**
>    * DRM_MODE_ATOMIC_FLAGS
>    *
>    * Bitfield of flags accepted by the &DRM_IOCTL_MODE_ATOMIC IOCTL in
> @@ -1338,11 +1349,12 @@ struct drm_mode_destroy_dumb {
>   #define DRM_MODE_ATOMIC_FLAGS (\
>   		DRM_MODE_PAGE_FLIP_EVENT |\
>   		DRM_MODE_PAGE_FLIP_ASYNC |\
>   		DRM_MODE_ATOMIC_TEST_ONLY |\
>   		DRM_MODE_ATOMIC_NONBLOCK |\
> -		DRM_MODE_ATOMIC_ALLOW_MODESET)
> +		DRM_MODE_ATOMIC_ALLOW_MODESET |\
> +		DRM_MODE_ATOMIC_RESET)
>   
>   struct drm_mode_atomic {
>   	__u32 flags;
>   	__u32 count_objs;
>   	__u64 objs_ptr;
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)