| Message ID | 20210726190800.26762-12-vinay.belgaumkar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/guc/slpc: Enable GuC based power management features | expand |
On Mon, Jul 26, 2021 at 12:07:56PM -0700, Vinay Belgaumkar wrote: > This interrupt is enabled during RPS initialization, and > now needs to be done by SLPC code. It allows ARAT timer > expiry interrupts to get forwarded to GuC. > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 16 ++++++++++++++++ > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 2 ++ > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 8 ++++++++ > 3 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > index 995d3d4807a3..c79dba60b2e6 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > @@ -392,6 +392,20 @@ int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val) > return ret; > } > > +void intel_guc_pm_intrmsk_enable(struct intel_gt *gt) > +{ > + u32 pm_intrmsk_mbz = 0; > + > + /* Allow GuC to receive ARAT timer expiry event. I've been berated for using comments like this this by other engineers. I personally don't care at all (nor does checkpatch) but if you want to avoid the wrath of others I'd change this to what I have below: /* * Allow GuC to receive ARAT timer expiry event. * This interrupt register is setup by RPS code * when host based Turbo is enabled. */ Same goes for comment below of same style. Either way, patch looks good to me. With that: Reviewed-by: Matthew Brost <matthew.brost@intel.com> > + * This interrupt register is setup by RPS code > + * when host based Turbo is enabled. > + */ > + pm_intrmsk_mbz |= ARAT_EXPIRED_INTRMSK; > + > + intel_uncore_rmw(gt->uncore, > + GEN6_PMINTRMSK, pm_intrmsk_mbz, 0); > +} > + > /* > * intel_guc_slpc_enable() - Start SLPC > * @slpc: pointer to intel_guc_slpc. > @@ -439,6 +453,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) > > slpc_query_task_state(slpc); > > + intel_guc_pm_intrmsk_enable(&i915->gt); > + > /* min and max frequency limits being used by SLPC */ > drm_info(&i915->drm, "SLPC min freq: %u Mhz, max is %u Mhz\n", > slpc_decode_min_freq(slpc), > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h > index d133c8020c16..f128143cc1d8 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h > @@ -9,6 +9,7 @@ > #include "intel_guc_submission.h" > #include "intel_guc_slpc_types.h" > > +struct intel_gt; > struct drm_printer; > > static inline bool intel_guc_slpc_is_supported(struct intel_guc *guc) > @@ -35,5 +36,6 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val); > int intel_guc_slpc_get_max_freq(struct intel_guc_slpc *slpc, u32 *val); > int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val); > int intel_guc_slpc_info(struct intel_guc_slpc *slpc, struct drm_printer *p); > +void intel_guc_pm_intrmsk_enable(struct intel_gt *gt); > > #endif > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > index b98c14f8c229..9238bc076605 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > @@ -652,6 +652,7 @@ void intel_uc_suspend(struct intel_uc *uc) > static int __uc_resume(struct intel_uc *uc, bool enable_communication) > { > struct intel_guc *guc = &uc->guc; > + struct intel_gt *gt = guc_to_gt(guc); > int err; > > if (!intel_guc_is_fw_running(guc)) > @@ -663,6 +664,13 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication) > if (enable_communication) > guc_enable_communication(guc); > > + /* If we are only resuming GuC communication but not reloading > + * GuC, we need to ensure the ARAT timer interrupt is enabled > + * again. In case of GuC reload, it is enabled during SLPC enable. > + */ > + if (enable_communication && intel_uc_uses_guc_slpc(uc)) > + intel_guc_pm_intrmsk_enable(gt); > + > err = intel_guc_resume(guc); > if (err) { > DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err); > -- > 2.25.0 >
On 7/27/2021 8:40 AM, Matthew Brost wrote: > On Mon, Jul 26, 2021 at 12:07:56PM -0700, Vinay Belgaumkar wrote: >> This interrupt is enabled during RPS initialization, and >> now needs to be done by SLPC code. It allows ARAT timer >> expiry interrupts to get forwarded to GuC. >> >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> >> --- >> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 16 ++++++++++++++++ >> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 2 ++ >> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 8 ++++++++ >> 3 files changed, 26 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c >> index 995d3d4807a3..c79dba60b2e6 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c >> @@ -392,6 +392,20 @@ int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val) >> return ret; >> } >> >> +void intel_guc_pm_intrmsk_enable(struct intel_gt *gt) >> +{ >> + u32 pm_intrmsk_mbz = 0; >> + >> + /* Allow GuC to receive ARAT timer expiry event. > > I've been berated for using comments like this this by other engineers. > I personally don't care at all (nor does checkpatch) but if you want to > avoid the wrath of others I'd change this to what I have below: > > /* > * Allow GuC to receive ARAT timer expiry event. > * This interrupt register is setup by RPS code > * when host based Turbo is enabled. > */ > > Same goes for comment below of same style. > > Either way, patch looks good to me. With that: > Reviewed-by: Matthew Brost <matthew.brost@intel.com> Fixed. Thanks, Vinay. > >> + * This interrupt register is setup by RPS code >> + * when host based Turbo is enabled. >> + */ >> + pm_intrmsk_mbz |= ARAT_EXPIRED_INTRMSK; >> + >> + intel_uncore_rmw(gt->uncore, >> + GEN6_PMINTRMSK, pm_intrmsk_mbz, 0); >> +} >> + >> /* >> * intel_guc_slpc_enable() - Start SLPC >> * @slpc: pointer to intel_guc_slpc. >> @@ -439,6 +453,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) >> >> slpc_query_task_state(slpc); >> >> + intel_guc_pm_intrmsk_enable(&i915->gt); >> + >> /* min and max frequency limits being used by SLPC */ >> drm_info(&i915->drm, "SLPC min freq: %u Mhz, max is %u Mhz\n", >> slpc_decode_min_freq(slpc), >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h >> index d133c8020c16..f128143cc1d8 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h >> @@ -9,6 +9,7 @@ >> #include "intel_guc_submission.h" >> #include "intel_guc_slpc_types.h" >> >> +struct intel_gt; >> struct drm_printer; >> >> static inline bool intel_guc_slpc_is_supported(struct intel_guc *guc) >> @@ -35,5 +36,6 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val); >> int intel_guc_slpc_get_max_freq(struct intel_guc_slpc *slpc, u32 *val); >> int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val); >> int intel_guc_slpc_info(struct intel_guc_slpc *slpc, struct drm_printer *p); >> +void intel_guc_pm_intrmsk_enable(struct intel_gt *gt); >> >> #endif >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c >> index b98c14f8c229..9238bc076605 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c >> @@ -652,6 +652,7 @@ void intel_uc_suspend(struct intel_uc *uc) >> static int __uc_resume(struct intel_uc *uc, bool enable_communication) >> { >> struct intel_guc *guc = &uc->guc; >> + struct intel_gt *gt = guc_to_gt(guc); >> int err; >> >> if (!intel_guc_is_fw_running(guc)) >> @@ -663,6 +664,13 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication) >> if (enable_communication) >> guc_enable_communication(guc); >> >> + /* If we are only resuming GuC communication but not reloading >> + * GuC, we need to ensure the ARAT timer interrupt is enabled >> + * again. In case of GuC reload, it is enabled during SLPC enable. >> + */ >> + if (enable_communication && intel_uc_uses_guc_slpc(uc)) >> + intel_guc_pm_intrmsk_enable(gt); >> + >> err = intel_guc_resume(guc); >> if (err) { >> DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err); >> -- >> 2.25.0 >>
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c index 995d3d4807a3..c79dba60b2e6 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c @@ -392,6 +392,20 @@ int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val) return ret; } +void intel_guc_pm_intrmsk_enable(struct intel_gt *gt) +{ + u32 pm_intrmsk_mbz = 0; + + /* Allow GuC to receive ARAT timer expiry event. + * This interrupt register is setup by RPS code + * when host based Turbo is enabled. + */ + pm_intrmsk_mbz |= ARAT_EXPIRED_INTRMSK; + + intel_uncore_rmw(gt->uncore, + GEN6_PMINTRMSK, pm_intrmsk_mbz, 0); +} + /* * intel_guc_slpc_enable() - Start SLPC * @slpc: pointer to intel_guc_slpc. @@ -439,6 +453,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) slpc_query_task_state(slpc); + intel_guc_pm_intrmsk_enable(&i915->gt); + /* min and max frequency limits being used by SLPC */ drm_info(&i915->drm, "SLPC min freq: %u Mhz, max is %u Mhz\n", slpc_decode_min_freq(slpc), diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h index d133c8020c16..f128143cc1d8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h @@ -9,6 +9,7 @@ #include "intel_guc_submission.h" #include "intel_guc_slpc_types.h" +struct intel_gt; struct drm_printer; static inline bool intel_guc_slpc_is_supported(struct intel_guc *guc) @@ -35,5 +36,6 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val); int intel_guc_slpc_get_max_freq(struct intel_guc_slpc *slpc, u32 *val); int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val); int intel_guc_slpc_info(struct intel_guc_slpc *slpc, struct drm_printer *p); +void intel_guc_pm_intrmsk_enable(struct intel_gt *gt); #endif diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c index b98c14f8c229..9238bc076605 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c @@ -652,6 +652,7 @@ void intel_uc_suspend(struct intel_uc *uc) static int __uc_resume(struct intel_uc *uc, bool enable_communication) { struct intel_guc *guc = &uc->guc; + struct intel_gt *gt = guc_to_gt(guc); int err; if (!intel_guc_is_fw_running(guc)) @@ -663,6 +664,13 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication) if (enable_communication) guc_enable_communication(guc); + /* If we are only resuming GuC communication but not reloading + * GuC, we need to ensure the ARAT timer interrupt is enabled + * again. In case of GuC reload, it is enabled during SLPC enable. + */ + if (enable_communication && intel_uc_uses_guc_slpc(uc)) + intel_guc_pm_intrmsk_enable(gt); + err = intel_guc_resume(guc); if (err) { DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
This interrupt is enabled during RPS initialization, and now needs to be done by SLPC code. It allows ARAT timer expiry interrupts to get forwarded to GuC. Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> --- drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 16 ++++++++++++++++ drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 2 ++ drivers/gpu/drm/i915/gt/uc/intel_uc.c | 8 ++++++++ 3 files changed, 26 insertions(+)