Re: [PATCH v3 02/11] drm/atomic: Create function to insert CRTC state into a commit

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 08.09.26 um 16:35 schrieb Maxime Ripard:
> drm_atomic_get_crtc_state() allocates a new CRTC state by duplicating
> the current one and inserts it into the atomic commit as a single
> operation.
>
> However, a later change will need to insert a CRTC state into a
> commit without going through the full allocation and duplication path
> in drm_atomic_get_crtc_state().
>
> Extract the state insertion logic into a new static
> drm_atomic_commit_set_crtc_state() helper, and convert
> drm_atomic_get_crtc_state() to use it.

Another change that makes sense on its own.

>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/drm_atomic.c | 29 +++++++++++++++++++++++------
>   1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5c0f7b7024d1..153d959bd4ff 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -421,10 +421,27 @@ void __drm_atomic_commit_free(struct kref *ref)
>   
>   	drm_dev_put(dev);
>   }
>   EXPORT_SYMBOL(__drm_atomic_commit_free);
>   
> +static int drm_atomic_commit_set_crtc_state(struct drm_atomic_commit *commit,
> +					    struct drm_crtc *crtc,
> +					    struct drm_crtc_state *crtc_state)
> +{
> +	int index = drm_crtc_index(crtc);
> +
> +	drm_modeset_lock_assert_held(&crtc->mutex);
> +
> +	commit->crtcs[index].state_to_destroy = crtc_state;
> +	commit->crtcs[index].old_state = crtc->state;
> +	commit->crtcs[index].new_state = crtc_state;
> +	commit->crtcs[index].ptr = crtc;
> +	crtc_state->state = commit;
> +
> +	return 0;
> +}
> +
>   /**
>    * drm_atomic_get_crtc_state - get CRTC state
>    * @state: global atomic state object
>    * @crtc: CRTC to get state object for
>    *
> @@ -443,11 +460,11 @@ EXPORT_SYMBOL(__drm_atomic_commit_free);
>    */
>   struct drm_crtc_state *
>   drm_atomic_get_crtc_state(struct drm_atomic_commit *state,
>   			  struct drm_crtc *crtc)
>   {
> -	int ret, index = drm_crtc_index(crtc);
> +	int ret;
>   	struct drm_crtc_state *crtc_state;
>   
>   	WARN_ON(!state->acquire_ctx);
>   	drm_WARN_ON(state->dev, state->checked);
>   
> @@ -461,15 +478,15 @@ drm_atomic_get_crtc_state(struct drm_atomic_commit *state,
>   
>   	crtc_state = crtc->funcs->atomic_duplicate_state(crtc);
>   	if (!crtc_state)
>   		return ERR_PTR(-ENOMEM);
>   
> -	state->crtcs[index].state_to_destroy = crtc_state;
> -	state->crtcs[index].old_state = crtc->state;
> -	state->crtcs[index].new_state = crtc_state;
> -	state->crtcs[index].ptr = crtc;
> -	crtc_state->state = state;
> +	ret = drm_atomic_commit_set_crtc_state(state, crtc, crtc_state);
> +	if (ret) {
> +		crtc->funcs->atomic_destroy_state(crtc, crtc_state);
> +		return ERR_PTR(ret);
> +	}
>   
>   	drm_dbg_atomic(state->dev, "Added [CRTC:%d:%s] %p state to %p\n",
>   		       crtc->base.id, crtc->name, crtc_state, state);
>   
>   	return crtc_state;
>

-- 
--
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)