[Intel-gfx] [PATCH] drm/i915/dsb: fix cmd_buf being wrongly set

[Animesh Manna]

On 11/28/2019 3:41 AM, Lucas De Marchi wrote:
> The "err" label is not really "err", but rather "out" since the return
> path is shared between error condition and normal path. This broke when
> commit 03cea61076f0 ("drm/i915/dsb: fix extra warning on error path
> handling") added a "dsb->cmd_buf = NULL;" there, making DSB to stop
> working since now all writes would pass-through via mmio.
>
> Remove the set to NULL since it's actually not needed: we only set it if
> all steps are succesful. While at it, rename the label so this confusion
Typo above, otherwise looks good.
Reviewed-by: Animesh Manna <animesh.manna at intel.com>

Quick merge will help in validation.

Regards,
Animesh
> doesn't happen again.
>
> Fixes: 03cea61076f0 ("drm/i915/dsb: fix extra warning on error path handling"
> Resolves: https://gitlab.freedesktop.org/drm/intel/issues/8
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>
> Right now I don't have access to the hw to reproduce it, so this is
> build-tested only.
>
>   drivers/gpu/drm/i915/display/intel_dsb.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 5bf67bdc8182..ada006a690df 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -116,34 +116,34 @@ intel_dsb_get(struct intel_crtc *crtc)
>   	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
>   	if (IS_ERR(obj)) {
>   		DRM_ERROR("Gem object creation failed\n");
> -		goto err;
> +		goto out;
>   	}
>   
>   	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
>   	if (IS_ERR(vma)) {
>   		DRM_ERROR("Vma creation failed\n");
>   		i915_gem_object_put(obj);
> -		goto err;
> +		goto out;
>   	}
>   
>   	buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
>   	if (IS_ERR(buf)) {
>   		DRM_ERROR("Command buffer creation failed\n");
> -		goto err;
> +		goto out;
>   	}
>   
>   	dsb->id = DSB1;
>   	dsb->vma = vma;
>   	dsb->cmd_buf = buf;
>   
> -err:
> +out:
>   	/*
> -	 * Set cmd_buf to NULL so the writes pass-through, but leave the
> -	 * dangling refcount to be removed later by the corresponding
> -	 * intel_dsb_put(): the important error message will already be
> -	 * logged above.
> +	 * On error dsb->cmd_buf will continue to be NULL, making the writes
> +	 * pass-through. Leave the dangling ref to be removed later by the
> +	 * corresponding intel_dsb_put(): the important error message will
> +	 * already be logged above.
>   	 */
> -	dsb->cmd_buf = NULL;
> +
>   	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>   
>   	return dsb;