Re: [PATCH] drm/i915: get runtime PM reference around GEM set_caching IOCTL

Chris Wilson
2015-11-09T13:25:56Z

On Mon, Nov 09, 2015 at 03:09:18PM +0200, Imre Deak wrote:
> Looked through it, it seems only i915_gem_set_tiling() could release
> the PTE's without waking up the hardware (if no need to unbind the
> object). Otherwise it's true that all callers hold (or should hold)
> already an RPM ref. To fix the set tiling case to work after your
> optimization we could wake up the HW unconditionally there, use a
> no_resume RPM ref+and RPM barrier or a separate new lock for the fault
> list.

I was suggesting we move to the model where writes through gsm took the
rpm reference itself.

> > From the rpm point of view, this should improve the success of
> > runtime suspend, and reduce wakelocks.
>
> Yes, seems like a worthy optimization, since I assume struct_mutex can
> be held for a long time without the need to wake up the hardware.

Admittedly most of the time we hold struct_mutex, the hw will be awake
for other reasons. But there are many times where we do take the
struct_mutex for 10s (if not 100s!) of milliseconds where the hw is
completely idle, and so every chance to reduce usage/contention on
struct_mutex is a little victory.

> Are you ok to first have the fix I posted and a similar one for
> i915_gem_set_tiling()? And then to follow-up with your plan.

Yes, adding the extra reference to that ioctl, juggling with the
struct_mutex and then moving the rpm reference to where it is required
lgtm.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre