Re: [PATCH 07/12] drm/msm: Handle drm_atomic_helper_swap_state failure

[Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 07/19/2017 01:10 PM, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state() will be changed to interruptible waiting
> in the next few commits, so all drivers have to be changed to handling
> failure.
> 
> MSM has its own busy tracking, which means the swap_state call can be
> done with stall = false, in which case it should never return an error.
> Handle failure with BUG_ON for this reason.
> 

Applied #2/12 and tried this out.

Tested-by: Archit Taneja <architt@codeaurora.org>

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-8-maarten.lankhorst@linux.intel.com
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ---
>   drivers/gpu/drm/msm/msm_atomic.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index 9633a68b14d7..badfa8717317 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -232,20 +232,18 @@ int msm_atomic_commit(struct drm_device *dev,
>   	 * mark our set of crtc's as busy:
>   	 */
>   	ret = start_atomic(dev->dev_private, c->crtc_mask);
> -	if (ret) {
> -		kfree(c);
> -		goto error;
> -	}
> +	if (ret)
> +		goto err_free;
> +
> +	BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
>   
>   	/*
>   	 * This is the point of no return - everything below never fails except
>   	 * when the hw goes bonghits. Which means we can commit the new state on
>   	 * the software side now.
> +	 *
> +	 * swap driver private state while still holding state_lock
>   	 */
> -
> -	drm_atomic_helper_swap_state(state, true);
> -
> -	/* swap driver private state while still holding state_lock */
>   	if (to_kms_state(state)->state)
>   		priv->kms->funcs->swap_state(priv->kms, state);
>   
> @@ -275,6 +273,8 @@ int msm_atomic_commit(struct drm_device *dev,
>   
>   	return 0;
>   
> +err_free:
> +	kfree(c);
>   error:
>   	drm_atomic_helper_cleanup_planes(dev, state);
>   	return ret;
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno