[Intel-gfx] [PATCH 2/3] drm/i915: add the FBC mutex

[Paulo Zanoni]

2015-06-17 4:52 GMT-03:00 Chris Wilson <chris at chris-wilson.co.uk>:
> On Tue, Jan 02, 2001 at 04:58:58AM -0200, Paulo Zanoni wrote:
>>  void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
>>                         unsigned int frontbuffer_bits,
>>                         enum fb_op_origin origin)
>> @@ -691,6 +725,8 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
>>       if (origin == ORIGIN_GTT)
>>               return;
>>
>> +     mutex_lock(&dev_priv->fbc.lock);
>> +
>>       if (dev_priv->fbc.enabled)
>>               fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
>>       else if (dev_priv->fbc.fbc_work)
>> @@ -702,7 +738,9 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
>>       dev_priv->fbc.busy_bits |= (fbc_bits & frontbuffer_bits);
>>
>>       if (dev_priv->fbc.busy_bits)
>> -             intel_fbc_disable(dev);
>> +             __intel_fbc_disable(dev);
>> +
>> +     mutex_unlock(&dev_priv->fbc.lock);
>>  }
>>
>>  void intel_fbc_flush(struct drm_i915_private *dev_priv,
>> @@ -710,13 +748,18 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>>  {
>>       struct drm_device *dev = dev_priv->dev;
>>
>> +     mutex_lock(&dev_priv->fbc.lock);
>> +
>>       if (!dev_priv->fbc.busy_bits)
>> -             return;
>> +             goto out;
>>
>>       dev_priv->fbc.busy_bits &= ~frontbuffer_bits;
>>
>>       if (!dev_priv->fbc.busy_bits)
>> -             intel_fbc_update(dev);
>> +             __intel_fbc_update(dev);
>> +
>> +out:
>> +     mutex_unlock(&dev_priv->fbc.lock);
>>  }
>
> These busy bits are locked higher up. In fact I want to migrate that
> lock to a spinlock, which has implications here. I didn't see anything
> that mandated using a mutex for fbc, right?

I didn't understand your idea. You want to replace the whole FBC mutex
for a spinlock? Why?

Please notice that we have dev_priv->fbc.busy_bits and also
dev_priv->fb_tracking.busy_bits. The FBC busy bits are only handled in
the intel_fbc.c functions. So maybe you want the spilock around the
fb_tracking ones? That wouldn't require changing the FBC mutex to a
spinlock, and it could be done today.

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni