Re: [PATCH v3] drm/i915/gt: Schedule request retirement when timeline idles

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Apr 13, 2020 at 08:29:30AM -0700, Sultan Alsawaf wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> commit 4f88f8747fa43c97c3b3712d8d87295ea757cc51 upstream.
> 
> The major drawback of commit 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX
> corruption WA") is that it disables RC6 while Skylake (and friends) is
> active, and we do not consider the GPU idle until all outstanding
> requests have been retired and the engine switched over to the kernel
> context. If userspace is idle, this task falls onto our background idle
> worker, which only runs roughly once a second, meaning that userspace has
> to have been idle for a couple of seconds before we enable RC6 again.
> Naturally, this causes us to consume considerably more energy than
> before as powersaving is effectively disabled while a display server
> (here's looking at you Xorg) is running.
> 
> As execlists will get a completion event as each context is completed,
> we can use this interrupt to queue a retire worker bound to this engine
> to cleanup idle timelines. We will then immediately notice the idle
> engine (without userspace intervention or the aid of the background
> retire worker) and start parking the GPU. Thus during light workloads,
> we will do much more work to idle the GPU faster...  Hopefully with
> commensurate power saving!
> 
> v2: Watch context completions and only look at those local to the engine
> when retiring to reduce the amount of excess work we perform.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112315
> References: 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX corruption WA")
> References: 2248a28384fe ("drm/i915/gen8+: Add RC6 CTX corruption WA")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20191125105858.1718307-3-chris@chris-wilson.co.uk
> [Sultan: for the backport to 5.4, struct_mutex needs to be held while
>  retiring so that retirement doesn't race with vma destruction.]
> Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  2 +
>  drivers/gpu/drm/i915/gt/intel_engine_types.h  |  8 ++
>  drivers/gpu/drm/i915/gt/intel_lrc.c           |  8 ++
>  drivers/gpu/drm/i915/gt/intel_timeline.c      |  1 +
>  .../gpu/drm/i915/gt/intel_timeline_types.h    |  3 +
>  drivers/gpu/drm/i915/i915_request.c           | 75 +++++++++++++++++++
>  drivers/gpu/drm/i915/i915_request.h           |  4 +
>  7 files changed, 101 insertions(+)

Why are you not cc:ing all of the relevant people on these patches?
That's not ok, I'm just going to drop all of these requests until that
happens, and I get an ack from the developers/maintainers involved.

thanks,

greg k-h