[Intel-gfx] [RFC 1/4] drm/i915: split out uncore_mmio_debug

[Daniele Ceraolo Spurio]

On 6/26/19 3:02 AM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2019-06-24 21:31:49)
>> @@ -605,18 +614,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
>>   void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>>   {
>>          spin_lock_irq(&uncore->lock);
>> +       spin_lock(&uncore->debug->lock);
>>          if (!uncore->user_forcewake.count++) {
> 
> Afaict, uncore->user_forcewake.count is only guarded by uncore->lock
> and we only need to take debug->lock for the debug->unclaimed_mmio_check
> manipulation. But there needs to be a shared usage counter around the
> debug as it is shared state.
> 
>>                  intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
>>   
>>                  /* Save and disable mmio debugging for the user bypass */
>>                  uncore->user_forcewake.saved_mmio_check =
>> -                       uncore->unclaimed_mmio_check;
>> +                       uncore->debug->unclaimed_mmio_check;
>>                  uncore->user_forcewake.saved_mmio_debug =
>>                          i915_modparams.mmio_debug;
> 
> Something more like
> 
> spin_lock_irq(&uncore->lock);
> if (!uncore->user_forcewake_count++) {
> 	spin_lock(&uncore->debug->lock);
> 	if (!uncore->debug->usage_count++) {
> 		...
> 	}
> 	spin_unlock(&uncore->debug->lock);
> }
> ...
> spin_unlock_irq(&uncore->lock);
> ?
> -Chris
> 

I do something similar in the next patch in the series (with the lock 
still on the outside of the if). I've split that out because I've added 
some functional changes to the flow. I can squash the 2 patches if you 
thing it is better that way.

Daniele