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

[John Harrison]

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.