[Intel-gfx] [PATCH v5 08/11] drm/i915/dsb: added dsb refcount to synchronize between get/put.

[Sharma, Shashank]

On 9/7/2019 4:37 PM, Animesh Manna wrote:
> The lifetime of command buffer can be controlled by the dsb user
> throuh refcount. Added refcount mechanism is dsb get/put call
> which create/destroy dsb context.
>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Cc: Shashank Sharma <shashank.sharma at intel.com>
> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 22 ++++++++++++++++------
>   drivers/gpu/drm/i915/display/intel_dsb.h |  1 +
>   2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 853685751540..b951a6b5264a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -78,7 +78,12 @@ intel_dsb_get(struct intel_crtc *crtc)
>   	struct intel_dsb *dsb = &crtc->dsb;
>   	intel_wakeref_t wakeref;
>   
> -	if ((!HAS_DSB(i915)) || dsb->cmd_buf)
> +	if (!HAS_DSB(i915))
> +		return dsb;
> +
> +	atomic_inc(&dsb->refcount);
> +

As discussed we are not solving any problem with reference counting, 
rather, we are adding a complexity here. It may be useful, when we are 
extending single instance of DSB to DSB pool but not right now.

I would say we drop this patch all together, and just have the simple 
implementation now.

- Shashank

> +	if (dsb->cmd_buf)
>   		return dsb;
>   
>   	dsb->id = DSB1;
> @@ -94,6 +99,7 @@ intel_dsb_get(struct intel_crtc *crtc)
>   	if (IS_ERR(vma)) {
>   		DRM_ERROR("Vma creation failed.\n");
>   		i915_gem_object_put(obj);
> +		atomic_dec(&dsb->refcount);
>   		goto err;
>   	}
>   
> @@ -102,6 +108,7 @@ intel_dsb_get(struct intel_crtc *crtc)
>   		DRM_ERROR("Command buffer creation failed.\n");
>   		i915_vma_unpin_and_release(&vma, 0);
>   		dsb->cmd_buf = NULL;
> +		atomic_dec(&dsb->refcount);
>   		goto err;
>   	}
>   	dsb->vma = vma;
> @@ -121,11 +128,14 @@ void intel_dsb_put(struct intel_dsb *dsb)
>   		return;
>   
>   	if (dsb->cmd_buf) {
> -		mutex_lock(&i915->drm.struct_mutex);
> -		i915_gem_object_unpin_map(dsb->vma->obj);
> -		i915_vma_unpin_and_release(&dsb->vma, 0);
> -		dsb->cmd_buf = NULL;
> -		mutex_unlock(&i915->drm.struct_mutex);
> +		atomic_dec(&dsb->refcount);
> +		if (!atomic_read(&dsb->refcount)) {
> +			mutex_lock(&i915->drm.struct_mutex);
> +			i915_gem_object_unpin_map(dsb->vma->obj);
> +			i915_vma_unpin_and_release(&dsb->vma, 0);
> +			dsb->cmd_buf = NULL;
> +			mutex_unlock(&i915->drm.struct_mutex);
> +		}
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
> index 7389c8c5b665..dca4e632dd3c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -20,6 +20,7 @@ enum dsb_id {
>   };
>   
>   struct intel_dsb {
> +	atomic_t refcount;
>   	enum dsb_id id;
>   	u32 *cmd_buf;
>   	struct i915_vma *vma;