Quoting John Harrison (2019-01-10 01:10:09) > 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@xxxxxxxxxxxxxxxxxx> > >> Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > >> --- > >> +#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. Using C99 would be a nice improvement for a lot of our macros, and I hope it comes to pass. Yes, the whole reason we return -1 on tracking-failure-but-rpm-success is so that we keep 0 as meaning rpm-failure so that the different cases are identifiable required for the markup and cookie tracking. So using -1 here just falls out of the general case. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
