Re: [PATCH 61/70] drm/i915: Make fb_tracking.lock a spinlock

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 04/07/2015 05:28 PM, Chris Wilson wrote:
> We only need a very lightweight mechanism here as the locking is only
> used for co-ordinating a bitfield.
>
> Also double check that the object is still pinned to the display plane
> before processing the state change.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/i915_drv.h          |  2 +-
>   drivers/gpu/drm/i915/i915_gem.c          |  2 +-
>   drivers/gpu/drm/i915/intel_frontbuffer.c | 40 +++++++++++++++++---------------
>   3 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 97372869097f..eeffefa10612 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1545,7 +1545,7 @@ struct intel_pipe_crc {
>   };
>
>   struct i915_frontbuffer_tracking {
> -	struct mutex lock;
> +	spinlock_t lock;
>
>   	/*
>   	 * Tracking bits for delayed frontbuffer flushing du to gpu activity or
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e9f2d2b102de..43baac2c1e20 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5260,7 +5260,7 @@ i915_gem_load(struct drm_device *dev)
>
>   	i915_gem_shrinker_init(dev_priv);
>
> -	mutex_init(&dev_priv->fb_tracking.lock);
> +	spin_lock_init(&dev_priv->fb_tracking.lock);
>   }
>
>   void i915_gem_release(struct drm_device *dev, struct drm_file *file)
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index a20cffb78c0f..28ce2ab94189 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -139,16 +139,14 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>
>   	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> -	if (!obj->frontbuffer_bits)
> +	if (!obj->frontbuffer_bits || !obj->pin_display)
>   		return;
>
>   	if (ring) {
> -		mutex_lock(&dev_priv->fb_tracking.lock);
> -		dev_priv->fb_tracking.busy_bits
> -			|= obj->frontbuffer_bits;
> -		dev_priv->fb_tracking.flip_bits
> -			&= ~obj->frontbuffer_bits;
> -		mutex_unlock(&dev_priv->fb_tracking.lock);
> +		spin_lock(&dev_priv->fb_tracking.lock);
> +		dev_priv->fb_tracking.busy_bits |= obj->frontbuffer_bits;
> +		dev_priv->fb_tracking.flip_bits &= ~obj->frontbuffer_bits;
> +		spin_unlock(&dev_priv->fb_tracking.lock);
>   	}
>
>   	intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
> @@ -175,9 +173,12 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>
>   	/* Delay flushing when rings are still busy.*/
> -	mutex_lock(&dev_priv->fb_tracking.lock);
> +	spin_lock(&dev_priv->fb_tracking.lock);
>   	frontbuffer_bits &= ~dev_priv->fb_tracking.busy_bits;
> -	mutex_unlock(&dev_priv->fb_tracking.lock);
> +	spin_unlock(&dev_priv->fb_tracking.lock);

Looks like you could just remove the lock here in process.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx