[PATCH 1/4] drm/atomic: Print the state every commit

[Daniel Vetter]

On Thu, Mar 24, 2022 at 03:47:21PM +0100, Maxime Ripard wrote:
> The DRM_UT_STATE controls whether we're calling
> drm_atomic_print_new_state() whenever a new state is committed. However,
> that call is made in the drm_mode_atomic_ioctl(), whereas we have
> multiple users of the drm_atomic_commit() function in the kernel
> (framebuffer emulation, drm_atomic_helper_dirtyfb, etc.). Similarly,
> it's only called for a blocking atomic commit.
> 
> This leads to multiple states being committed but never actually
> displayed even though we asked to have verbose atomic state debugging.
> 
> Let's move the call to drm_atomic_print_new_state() to
> drm_atomic_commit() and drm_atomic_nonblocking_commit() to make sure we
> don't miss any.
> 
> Signed-off-by: Maxime Ripard <maxime at cerno.tech>

Iirc the idea was that if you get a printing for every nonblocking then
the logs get completely flooded. git blame + mailing lists should bring up
the people who's ack you need (and maybe cc on v2 of this).

I think we might need a new DRM_UT_NONBLOCKING_STATE or something like
that to handle this.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c      | 8 ++++++++
>  drivers/gpu/drm/drm_atomic_uapi.c | 3 ---
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 88cd992df356..ee2496ff3dcc 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1423,6 +1423,7 @@ EXPORT_SYMBOL(drm_atomic_check_only);
>  int drm_atomic_commit(struct drm_atomic_state *state)
>  {
>  	struct drm_mode_config *config = &state->dev->mode_config;
> +	struct drm_printer p = drm_info_printer(state->dev->dev);
>  	int ret;
>  
>  	ret = drm_atomic_check_only(state);
> @@ -1431,6 +1432,9 @@ int drm_atomic_commit(struct drm_atomic_state *state)
>  
>  	drm_dbg_atomic(state->dev, "committing %p\n", state);
>  
> +	if (drm_debug_enabled(DRM_UT_STATE))
> +		drm_atomic_print_new_state(state, &p);
> +
>  	return config->funcs->atomic_commit(state->dev, state, false);
>  }
>  EXPORT_SYMBOL(drm_atomic_commit);
> @@ -1452,6 +1456,7 @@ EXPORT_SYMBOL(drm_atomic_commit);
>  int drm_atomic_nonblocking_commit(struct drm_atomic_state *state)
>  {
>  	struct drm_mode_config *config = &state->dev->mode_config;
> +	struct drm_printer p = drm_info_printer(state->dev->dev);
>  	int ret;
>  
>  	ret = drm_atomic_check_only(state);
> @@ -1460,6 +1465,9 @@ int drm_atomic_nonblocking_commit(struct drm_atomic_state *state)
>  
>  	drm_dbg_atomic(state->dev, "committing %p nonblocking\n", state);
>  
> +	if (drm_debug_enabled(DRM_UT_STATE))
> +		drm_atomic_print_new_state(state, &p);
> +
>  	return config->funcs->atomic_commit(state->dev, state, true);
>  }
>  EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 9781722519c3..e9bb136c7a7c 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1458,9 +1458,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  	} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
>  		ret = drm_atomic_nonblocking_commit(state);
>  	} else {
> -		if (drm_debug_enabled(DRM_UT_STATE))
> -			drm_atomic_print_new_state(state, &p);
> -
>  		ret = drm_atomic_commit(state);
>  	}
>  
> -- 
> 2.35.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch