[Intel-gfx] [PATCH 2/2] drm/i915/dsb: fix extra warning on error path handling

[Matt Roper]

On Mon, Nov 11, 2019 at 12:50:25PM -0800, Lucas De Marchi wrote:
> When we call intel_dsb_get(), the dsb initialization may fail for
> various reasons. We already log the error message in that path, making
> it unnecessary to trigger a warning that refcount == 0 when calling
> intel_dsb_put().
> 
> So here we simplify the logic and do lazy shutdown: leaving the extra
> refcount alive so when we call intel_dsb_put() we end up calling
> i915_vma_unpin_and_release().
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>

Due to the lack of any actual concurrency, it seems like we could
eventually get rid of the whole get/put design and just allocate the
buffer once (and pin it during the prepare step).  But this seems good
enough for now.

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 4feebbeb0b0c..858af6be9c36 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -102,6 +102,7 @@ intel_dsb_get(struct intel_crtc *crtc)
>  	struct intel_dsb *dsb = &crtc->dsb;
>  	struct drm_i915_gem_object *obj;
>  	struct i915_vma *vma;
> +	u32 *buf;
>  	intel_wakeref_t wakeref;
>  
>  	if (!HAS_DSB(i915))
> @@ -110,7 +111,6 @@ intel_dsb_get(struct intel_crtc *crtc)
>  	if (++dsb->refcount != 1)
>  		return dsb;
>  
> -	dsb->id = DSB1;
>  	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>  
>  	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
> @@ -123,22 +123,29 @@ intel_dsb_get(struct intel_crtc *crtc)
>  	if (IS_ERR(vma)) {
>  		DRM_ERROR("Vma creation failed\n");
>  		i915_gem_object_put(obj);
> -		dsb->refcount--;
>  		goto err;
>  	}
>  
> -	dsb->cmd_buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
> -	if (IS_ERR(dsb->cmd_buf)) {
> +	buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
> +	if (IS_ERR(buf)) {
>  		DRM_ERROR("Command buffer creation failed\n");
> -		i915_vma_unpin_and_release(&vma, 0);
> -		dsb->cmd_buf = NULL;
> -		dsb->refcount--;
>  		goto err;
>  	}
> +
> +	dsb->id = DSB1;
>  	dsb->vma = vma;
> +	dsb->cmd_buf = buf;
>  
>  err:
> +	/*
> +	 * 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.
> +	 */
> +	dsb->cmd_buf = NULL;
>  	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> +
>  	return dsb;
>  }
>  
> -- 
> 2.24.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795