Re: [PATCH] drm/i915/guc: Add host2guc notification for suspend and resume

[Yu Dai <yu.dai@intel.com>]

On 09/30/2015 03:46 AM, Kamble, Sagar A wrote:
> Thanks for the updated patch. Minor comment below.
>
> Thanks
> Sagar
>
> On 9/26/2015 12:16 AM, yu.dai@intel.com wrote:
> > From: Alex Dai <yu.dai@intel.com>
> >
> > Add host2guc interfaces to nofigy GuC power state changes when
> > enter or resume from power saving state.
> >
> > v2: Add GuC suspend/resume to runtime suspend/resume too
> >
> > v1: Change to a more flexible way when fill host to GuC scratch
> > data in order to remove hard coding.
> >
> > Signed-off-by: Alex Dai <yu.dai@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_drv.c            |  5 +++
> >   drivers/gpu/drm/i915/i915_gem.c            |  2 ++
> >   drivers/gpu/drm/i915/i915_guc_submission.c | 50 ++++++++++++++++++++++++++++++
> >   drivers/gpu/drm/i915/intel_guc.h           |  2 ++
> >   drivers/gpu/drm/i915/intel_guc_fwif.h      |  8 +++++
> >   drivers/gpu/drm/i915/intel_guc_loader.c    |  4 ++-
> >   6 files changed, 70 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index e6d7a69..842eb13 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -737,6 +737,8 @@ static int i915_drm_resume(struct drm_device *dev)
> >   	}
> >   	mutex_unlock(&dev->struct_mutex);
> >
> > +	intel_guc_resume(dev);
> > +
> >   	intel_modeset_init_hw(dev);
> >
> >   	spin_lock_irq(&dev_priv->irq_lock);
> > @@ -1476,6 +1478,7 @@ static int intel_runtime_suspend(struct device *device)
> >   	i915_gem_release_all_mmaps(dev_priv);
> >   	mutex_unlock(&dev->struct_mutex);
> >
> > +	intel_guc_suspend(dev);
> >   	intel_suspend_gt_powersave(dev);
> >   	intel_runtime_pm_disable_interrupts(dev_priv);
> >
> > @@ -1535,6 +1538,8 @@ static int intel_runtime_resume(struct device *device)
> >   	intel_opregion_notify_adapter(dev, PCI_D0);
> >   	dev_priv->pm.suspended = false;
> >
> > +	intel_guc_resume(dev);
> > +
> >   	if (IS_GEN6(dev_priv))
> >   		intel_init_pch_refclk(dev);
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index bf5ef7a..679ed55 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4460,6 +4460,8 @@ i915_gem_suspend(struct drm_device *dev)
> >   	i915_gem_stop_ringbuffers(dev);
> >   	mutex_unlock(&dev->struct_mutex);
> >
> > +	intel_guc_suspend(dev);
> > +
> Should this be called as part of i915_drm_suspend for consistency
> instead of i915_gem_suspend?
>

Yes, I will change it accordingly. i915_gem_suspend is called during 
unload, where we won't need to suspend guc - it will be unloaded anyway.

Thanks for the review,
Alex
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx