[Intel-gfx] [PATCH 17/46] drm/i915: Syntatic sugar for using intel_runtime_pm

[John Harrison]

On 1/9/2019 16:24, John Harrison wrote:
> On 1/7/2019 03:54, Chris Wilson wrote:
>> Frequently, we use intel_runtime_pm_get/_put around a small block.
>> Formalise that usage by providing a macro to define such a block with an
>> automatic closure to scope the intel_runtime_pm wakeref to that block,
>> i.e. macro abuse smelling of python.
>>
>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> ---
>>   +#define with_intel_runtime_pm(i915, wf) \
>> +    for (wf = intel_runtime_pm_get(i915); wf; \
>> +         intel_runtime_pm_put(i915, wf), wf = 0)
>> +
>> +#define with_intel_runtime_pm_if_in_use(i915, wf) \
>> +    for (wf = intel_runtime_pm_get_if_in_use(i915); wf; \
>> +         intel_runtime_pm_put(i915, wf), wf = 0)
>> +
> This is a potential change in behaviour. Previously the simple 'get' 
> version would unconditionally execute the wrapped code. Whereas now, 
> if the get function fails for some reason and returns zero, the 
> wrapped code will be skipped. Currently, the get() function can't 
> return zero - it returns -1 in the case of the tracking code failing 
> to allocate or similar. But is that guaranteed to be the case 
> forevermore? It would be a better match for the original behaviour if 
> the 'for' loop of the 'get' version was unconditional and only the 
> 'get_if_in_use' version could skip. E.g. something like:
>    for (intel_wakeref_t loop = -1, wf = intel_runtime_pm_get(i915) ; 
> loop; intel_runtime_pm_put(i915, wf), wf = loop = 0)
>
> Although that does mean the wf becomes local to the loop. On the other 
> hand, I'm also not sure why it needs to be external anyway? If it is 
> guaranteed to be zero on exit and any value on entry is overwritten, 
> then why have it external at all? Would it not be neater/smaller 
> source to get rid of all the local instantiations?
>
> John.
>
Doh. Not sure why I was thinking C99 extensions were valid in the 
kernel. I can't think of an alternative way to fix the above issues 
without making the macro truly hideous. So maybe it's not enough of a 
worry to worry about.

John.