[Intel-gfx] [PATCH v8 2/8] drm/i915: Update GEM suspend/resume flows considering GuC and GEM fences

[Chris Wilson]

Quoting Sagar Arun Kamble (2017-09-26 15:24:15)
> 
> 
> On 9/26/2017 7:12 PM, Chris Wilson wrote:
> > Quoting Sagar Arun Kamble (2017-09-26 14:24:39)
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >> index dbe181b..5dcd8c0 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> @@ -2022,11 +2022,22 @@ int i915_gem_fault(struct vm_fault *vmf)
> >>          intel_runtime_pm_put(i915);
> >>   }
> >>   
> >> +/**
> >> + * i915_gem_runtime_suspend() - Finish GEM suspend
> >> + * @dev_priv: i915 device private
> >> + *
> >> + * This function suspends GuC, removes userspace mappings for all GEM obejcts
> >> + * currently on userfault list and marks fences if any being used as lost.
> >> + *
> >> + * Return:     non-zero code on error
> >> + */
> >> +/**
> >> + * i915_gem_runtime_resume() - Restore GEM state
> >> + * @dev_priv: i915 device private
> >> + *
> >> + * This function inits swizzling, restores fences and resumes GuC.
> >> + *
> >> + * Return:     non-zero code on error
> >> + */
> >> +/**
> >> + * i915_gem_suspend() - Suspend all GT activity.
> >> + * @dev_priv: i915 device private
> >> + *
> >> + * This function disables RPS, flushes all executing context ensuring
> >> + * GEM/GT/Engines idleness, cancels all work that needs GT access and suspends
> >> + * GuC. In the end currently, it also reset the GEM state and GPU HW.
> >> + *
> >> + * Return:     non-zero code on error
> >> + */
> >> +/**
> >> + * i915_gem_resume() - Resume GT activity.
> >> + * @dev_priv: i915 device private
> >> + *
> >> + * This function restores GTT mappings, restores fences and resets the
> >> + * context images and resumes GuC.
> >> + *
> >> + * Return:     non-zero code on error
> >> + */
> > I can very much read what the functions do. These comments blocks are
> > for telling me when to call them, under what pre/post-conditions and the
> > overall intent of the function.
> >
> > Who are these blocks for? This isn't a library interface we expect to be
> > used, these are hooks.
> > -Chris
> Since these functions are exported from i915_gem.c and are being used 
> from respective drm functions, to make
> the semantics available in kernel docs I have added these comments.

My point is that this isn't functionality that was exported for somebody
to call, this was functionality that was pulled from i915_drv.c for a
singular purpose. It is driven by that context, and not for general
purpose. So who is the consumer?
-Chris