[Intel-gfx] [PATCH 4/7] drm/i915: add the FBC mutex

[Paulo Zanoni]

2015-07-01 17:49 GMT-03:00 Chris Wilson <chris at chris-wilson.co.uk>:
> On Wed, Jul 01, 2015 at 05:15:23PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> Make sure we're not going to have weird races in really weird cases
>> where a lot of different CRTCs are doing rendering and modesets at the
>> same time.
>>
>> With this change and the stolen_lock from the previous patch, we can start
>> removing the struct_mutex locking we have around FBC in the next patches.
>>
>> v2:
>>  - Rebase (6 months later)
>>  - Also lock debugfs and stolen.
>> v3:
>>  - Don't lock a single value read (Chris).
>>  - Replace lockdep assertions with WARNs (Daniel).
>>  - Improve commit message.
>>  - Don't forget intel_pre_plane_update() locking.
>> v4:
>>  - Don't remove struct_mutex at intel_pre_plane_update() (Chris).
>>  - Add comment regarding locking dependencies (Chris).
>>  - Rebase after the stolen code rework.
>>  - Rebase again after drm-intel-nightly changes.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
>
> See below.
>
>> @@ -683,6 +721,8 @@ void intel_fbc_update(struct drm_device *dev)
>>       const struct drm_display_mode *adjusted_mode;
>>       unsigned int max_width, max_height;
>>
>> +     WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
>> +
>>       if (!HAS_FBC(dev))
>>               return;
>
> This is now __intel_fbc_update() inside the fbc.lock. One would think
> that the internal functions would only be called when FBC is desired.
>
> That looks to be true, except for the new intel_fbc_update(). You can
> upgrade this to if (WARN_ON(!HAS_FBC(dev_priv))) return; after adding
> the proper guard to intel_fbc_update().

__intel_fbc_update() is also called by intel_fbc_flush(). See "[PATCH
8/8] drm/i915: protect FBC functions with HAS_FBC checks" on the new
series. I still have another series just with minor changes like this
which I intend to send after the locking series.

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



-- 
Paulo Zanoni