| Message ID | 1443206792-7995-1-git-send-email-yu.dai@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? > cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work); > cancel_delayed_work_sync(&dev_priv->mm.retire_work); > flush_delayed_work(&dev_priv->mm.idle_work); > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index 792d0b9..38b6ef4 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -914,3 +914,53 @@ void i915_guc_submission_fini(struct drm_device *dev) > gem_release_guc_obj(guc->ctx_pool_obj); > guc->ctx_pool_obj = NULL; > } > + > +/** > + * intel_guc_suspend() - notify GuC entering suspend state > + * @dev: drm device > + */ > +int intel_guc_suspend(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_context *ctx; > + u32 data[3]; > + > + if (!i915.enable_guc_submission) > + return 0; > + > + ctx = dev_priv->ring[RCS].default_context; > + > + data[0] = HOST2GUC_ACTION_ENTER_S_STATE; > + /* any value greater than GUC_POWER_D0 */ > + data[1] = GUC_POWER_D1; > + /* first page is shared data with GuC */ > + data[2] = i915_gem_obj_ggtt_offset(ctx->engine[RCS].state); > + > + return host2guc_action(guc, data, ARRAY_SIZE(data)); > +} > + > + > +/** > + * intel_guc_resume() - notify GuC resuming from suspend state > + * @dev: drm device > + */ > +int intel_guc_resume(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_context *ctx; > + u32 data[3]; > + > + if (!i915.enable_guc_submission) > + return 0; > + > + ctx = dev_priv->ring[RCS].default_context; > + > + data[0] = HOST2GUC_ACTION_EXIT_S_STATE; > + data[1] = GUC_POWER_D0; > + /* first page is shared data with GuC */ > + data[2] = i915_gem_obj_ggtt_offset(ctx->engine[RCS].state); > + > + return host2guc_action(guc, data, ARRAY_SIZE(data)); > +} > diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h > index 55c9bf8..39cf460 100644 > --- a/drivers/gpu/drm/i915/intel_guc.h > +++ b/drivers/gpu/drm/i915/intel_guc.h > @@ -116,6 +116,8 @@ extern void intel_guc_ucode_init(struct drm_device *dev); > extern int intel_guc_ucode_load(struct drm_device *dev); > extern void intel_guc_ucode_fini(struct drm_device *dev); > extern const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status); > +extern int intel_guc_suspend(struct drm_device *dev); > +extern int intel_guc_resume(struct drm_device *dev); > > /* i915_guc_submission.c */ > int i915_guc_submission_init(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h > index cd94075..4029478 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -290,12 +290,20 @@ struct guc_context_desc { > u64 desc_private; > } __packed; > > +#define GUC_POWER_UNSPECIFIED 0 > +#define GUC_POWER_D0 1 > +#define GUC_POWER_D1 2 > +#define GUC_POWER_D2 3 > +#define GUC_POWER_D3 4 > + > /* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */ > enum host2guc_action { > HOST2GUC_ACTION_DEFAULT = 0x0, > HOST2GUC_ACTION_SAMPLE_FORCEWAKE = 0x6, > HOST2GUC_ACTION_ALLOCATE_DOORBELL = 0x10, > HOST2GUC_ACTION_DEALLOCATE_DOORBELL = 0x20, > + HOST2GUC_ACTION_ENTER_S_STATE = 0x501, > + HOST2GUC_ACTION_EXIT_S_STATE = 0x502, > HOST2GUC_ACTION_SLPC_REQUEST = 0x3003, > HOST2GUC_ACTION_LIMIT > }; > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index 724b01d..95fe488 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -379,7 +379,6 @@ int intel_guc_ucode_load(struct drm_device *dev) > intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); > > direct_interrupts_to_host(dev_priv); > - i915_guc_submission_disable(dev); > > if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_NONE) > return 0; > @@ -429,6 +428,9 @@ int intel_guc_ucode_load(struct drm_device *dev) > intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); > > if (i915.enable_guc_submission) { > + /* The execbuf_client will be recreated. Release it first. */ > + i915_guc_submission_disable(dev); > + > err = i915_guc_submission_enable(dev); > if (err) > goto fail;
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
On Wed, Sep 30, 2015 at 08:59:21AM -0700, Yu Dai wrote: > > > 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 [TOR:] Is "nofigy" a typo? Thanks, Tom > >> 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
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); + cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work); cancel_delayed_work_sync(&dev_priv->mm.retire_work); flush_delayed_work(&dev_priv->mm.idle_work); diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 792d0b9..38b6ef4 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -914,3 +914,53 @@ void i915_guc_submission_fini(struct drm_device *dev) gem_release_guc_obj(guc->ctx_pool_obj); guc->ctx_pool_obj = NULL; } + +/** + * intel_guc_suspend() - notify GuC entering suspend state + * @dev: drm device + */ +int intel_guc_suspend(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_guc *guc = &dev_priv->guc; + struct intel_context *ctx; + u32 data[3]; + + if (!i915.enable_guc_submission) + return 0; + + ctx = dev_priv->ring[RCS].default_context; + + data[0] = HOST2GUC_ACTION_ENTER_S_STATE; + /* any value greater than GUC_POWER_D0 */ + data[1] = GUC_POWER_D1; + /* first page is shared data with GuC */ + data[2] = i915_gem_obj_ggtt_offset(ctx->engine[RCS].state); + + return host2guc_action(guc, data, ARRAY_SIZE(data)); +} + + +/** + * intel_guc_resume() - notify GuC resuming from suspend state + * @dev: drm device + */ +int intel_guc_resume(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_guc *guc = &dev_priv->guc; + struct intel_context *ctx; + u32 data[3]; + + if (!i915.enable_guc_submission) + return 0; + + ctx = dev_priv->ring[RCS].default_context; + + data[0] = HOST2GUC_ACTION_EXIT_S_STATE; + data[1] = GUC_POWER_D0; + /* first page is shared data with GuC */ + data[2] = i915_gem_obj_ggtt_offset(ctx->engine[RCS].state); + + return host2guc_action(guc, data, ARRAY_SIZE(data)); +} diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index 55c9bf8..39cf460 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -116,6 +116,8 @@ extern void intel_guc_ucode_init(struct drm_device *dev); extern int intel_guc_ucode_load(struct drm_device *dev); extern void intel_guc_ucode_fini(struct drm_device *dev); extern const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status); +extern int intel_guc_suspend(struct drm_device *dev); +extern int intel_guc_resume(struct drm_device *dev); /* i915_guc_submission.c */ int i915_guc_submission_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h index cd94075..4029478 100644 --- a/drivers/gpu/drm/i915/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h @@ -290,12 +290,20 @@ struct guc_context_desc { u64 desc_private; } __packed; +#define GUC_POWER_UNSPECIFIED 0 +#define GUC_POWER_D0 1 +#define GUC_POWER_D1 2 +#define GUC_POWER_D2 3 +#define GUC_POWER_D3 4 + /* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */ enum host2guc_action { HOST2GUC_ACTION_DEFAULT = 0x0, HOST2GUC_ACTION_SAMPLE_FORCEWAKE = 0x6, HOST2GUC_ACTION_ALLOCATE_DOORBELL = 0x10, HOST2GUC_ACTION_DEALLOCATE_DOORBELL = 0x20, + HOST2GUC_ACTION_ENTER_S_STATE = 0x501, + HOST2GUC_ACTION_EXIT_S_STATE = 0x502, HOST2GUC_ACTION_SLPC_REQUEST = 0x3003, HOST2GUC_ACTION_LIMIT }; diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 724b01d..95fe488 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -379,7 +379,6 @@ int intel_guc_ucode_load(struct drm_device *dev) intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); direct_interrupts_to_host(dev_priv); - i915_guc_submission_disable(dev); if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_NONE) return 0; @@ -429,6 +428,9 @@ int intel_guc_ucode_load(struct drm_device *dev) intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); if (i915.enable_guc_submission) { + /* The execbuf_client will be recreated. Release it first. */ + i915_guc_submission_disable(dev); + err = i915_guc_submission_enable(dev); if (err) goto fail;