| Message ID | 20221118183354.1047829-1-badal.nilawar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/i915/mtl: Enable Idle Messaging for GSC CS | expand |
On Sat, 2022-11-19 at 00:03 +0530, Badal Nilawar wrote: > From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > By defaut idle messaging is disabled for GSC CS so to unblock RC6 > entry on media tile idle messaging need to be enabled. > > v2: > - Fix review comments (Vinay) > - Set GSC idle hysteresis as per spec (Badal) > v3: > - Fix review comments (Rodrigo) > > Bspec: 71496 > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> > Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> He is the author of the patch, no?! or you can remove this or change the author to be you and keep his reviewed-by... or I can just remove his rv-b while merging.. just let me know.. > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++ > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 ++++ > 2 files changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > index b0a4a2dbe3ee..e971b153fda9 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > @@ -15,6 +15,22 @@ > #include "intel_rc6.h" > #include "intel_ring.h" > #include "shmem_utils.h" > +#include "intel_gt_regs.h" > + > +static void intel_gsc_idle_msg_enable(struct intel_engine_cs > *engine) > +{ > + struct drm_i915_private *i915 = engine->i915; > + > + if (IS_METEORLAKE(i915) && engine->id == GSC0) { > + intel_uncore_write(engine->gt->uncore, > + RC_PSMI_CTRL_GSCCS, > + > _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE)); > + /* hysteresis 0xA=5us as recommended in spec*/ > + intel_uncore_write(engine->gt->uncore, > + PWRCTX_MAXCNT_GSCCS, > + 0xA); > + } > +} > > static void dbg_poison_ce(struct intel_context *ce) > { > @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs > *engine) > > intel_wakeref_init(&engine->wakeref, rpm, &wf_ops); > intel_engine_init_heartbeat(engine); > + > + intel_gsc_idle_msg_enable(engine); > } > > /** > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > index c3cd92691795..80a979e6f6be 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > @@ -917,6 +917,10 @@ > #define MSG_IDLE_FW_MASK REG_GENMASK(13, 9) > #define MSG_IDLE_FW_SHIFT 9 > > +#define RC_PSMI_CTRL_GSCCS _MMIO(0x11a050) > +#define IDLE_MSG_DISABLE REG_BIT(0) > +#define PWRCTX_MAXCNT_GSCCS _MMIO(0x11a054) > + > #define FORCEWAKE_MEDIA_GEN9 _MMIO(0xa270) > #define FORCEWAKE_RENDER_GEN9 _MMIO(0xa278) >
On Fri, 18 Nov 2022 10:37:37 -0800, Vivi, Rodrigo wrote: > > On Sat, 2022-11-19 at 00:03 +0530, Badal Nilawar wrote: > > From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > > > By defaut idle messaging is disabled for GSC CS so to unblock RC6 > > entry on media tile idle messaging need to be enabled. > > > > v2: > > - Fix review comments (Vinay) > > - Set GSC idle hysteresis as per spec (Badal) > > v3: > > - Fix review comments (Rodrigo) > > > > Bspec: 71496 > > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> > > Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > He is the author of the patch, no?! > or you can remove this or change the author to be you and keep his > reviewed-by... > > or I can just remove his rv-b while merging.. just let me know.. Not sure if that is the case here, but when multiple people contribute to a patch, the original author can review changes by others and add his Reviewed-by, no? Or are we saying it is redundant for the author to add his R-b? Similarly, are S-o-b and R-b by the same person ok? I add changes to someone's patch so add my S-o-b but also review other's changes so add my R-b? Sometimes finding a 3rd person to add a R-b is hard. But two poeple can contribute to a patch and review each other's changes so add both their S-o-b's and R-b's or no? :) Ashutosh
On 19-11-2022 00:07, Vivi, Rodrigo wrote: > On Sat, 2022-11-19 at 00:03 +0530, Badal Nilawar wrote: >> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> >> >> By defaut idle messaging is disabled for GSC CS so to unblock RC6 >> entry on media tile idle messaging need to be enabled. >> >> v2: >> - Fix review comments (Vinay) >> - Set GSC idle hysteresis as per spec (Badal) >> v3: >> - Fix review comments (Rodrigo) >> >> Bspec: 71496 >> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> >> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> >> Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > He is the author of the patch, no?! > or you can remove this or change the author to be you and keep his > reviewed-by... > > or I can just remove his rv-b while merging.. just let me know.. As he is original author I will prefer not to change it. You can remove his rv-b while merging. Regards, Badal > >> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++ >> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 ++++ >> 2 files changed, 22 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c >> b/drivers/gpu/drm/i915/gt/intel_engine_pm.c >> index b0a4a2dbe3ee..e971b153fda9 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c >> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c >> @@ -15,6 +15,22 @@ >> #include "intel_rc6.h" >> #include "intel_ring.h" >> #include "shmem_utils.h" >> +#include "intel_gt_regs.h" >> + >> +static void intel_gsc_idle_msg_enable(struct intel_engine_cs >> *engine) >> +{ >> + struct drm_i915_private *i915 = engine->i915; >> + >> + if (IS_METEORLAKE(i915) && engine->id == GSC0) { >> + intel_uncore_write(engine->gt->uncore, >> + RC_PSMI_CTRL_GSCCS, >> + >> _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE)); >> + /* hysteresis 0xA=5us as recommended in spec*/ >> + intel_uncore_write(engine->gt->uncore, >> + PWRCTX_MAXCNT_GSCCS, >> + 0xA); >> + } >> +} >> >> static void dbg_poison_ce(struct intel_context *ce) >> { >> @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs >> *engine) >> >> intel_wakeref_init(&engine->wakeref, rpm, &wf_ops); >> intel_engine_init_heartbeat(engine); >> + >> + intel_gsc_idle_msg_enable(engine); >> } >> >> /** >> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h >> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h >> index c3cd92691795..80a979e6f6be 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h >> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h >> @@ -917,6 +917,10 @@ >> #define MSG_IDLE_FW_MASK REG_GENMASK(13, 9) >> #define MSG_IDLE_FW_SHIFT 9 >> >> +#define RC_PSMI_CTRL_GSCCS _MMIO(0x11a050) >> +#define IDLE_MSG_DISABLE REG_BIT(0) >> +#define PWRCTX_MAXCNT_GSCCS _MMIO(0x11a054) >> + >> #define FORCEWAKE_MEDIA_GEN9 _MMIO(0xa270) >> #define FORCEWAKE_RENDER_GEN9 _MMIO(0xa278) >> >
On Fri, 2022-11-18 at 13:37 -0800, Dixit, Ashutosh wrote: > On Fri, 18 Nov 2022 10:37:37 -0800, Vivi, Rodrigo wrote: > > > > On Sat, 2022-11-19 at 00:03 +0530, Badal Nilawar wrote: > > > From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > > > > > By defaut idle messaging is disabled for GSC CS so to unblock RC6 > > > entry on media tile idle messaging need to be enabled. > > > > > > v2: > > > - Fix review comments (Vinay) > > > - Set GSC idle hysteresis as per spec (Badal) > > > v3: > > > - Fix review comments (Rodrigo) > > > > > > Bspec: 71496 > > > > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> > > > Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > > > He is the author of the patch, no?! > > or you can remove this or change the author to be you and keep his > > reviewed-by... > > > > or I can just remove his rv-b while merging.. just let me know.. > > Not sure if that is the case here, yeap, this is too small patch for that to happen. > but when multiple people contribute to a > patch, the original author can review changes by others and add his > Reviewed-by, no? Or are we saying it is redundant for the author to > add his > R-b? > > Similarly, are S-o-b and R-b by the same person ok? I add changes to > someone's patch so add my S-o-b but also review other's changes so > add my > R-b? Sometimes finding a 3rd person to add a R-b is hard. But two > poeple > can contribute to a patch and review each other's changes so add both > their > S-o-b's and R-b's or no? Definitely a case by case thing. If that happens it is good to have it clearly written in the commit message to know who did what, who reviewed what. Although I'd say that I'd still prefer the co-authored-by and having a third one doing the full review. > > :) > > Ashutosh >
On Sat, 2022-11-19 at 09:02 +0530, Nilawar, Badal wrote: > > > On 19-11-2022 00:07, Vivi, Rodrigo wrote: > > On Sat, 2022-11-19 at 00:03 +0530, Badal Nilawar wrote: > > > From: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > > > > > By defaut idle messaging is disabled for GSC CS so to unblock RC6 > > > entry on media tile idle messaging need to be enabled. > > > > > > v2: > > > - Fix review comments (Vinay) > > > - Set GSC idle hysteresis as per spec (Badal) > > > v3: > > > - Fix review comments (Rodrigo) > > > > > > Bspec: 71496 > > > > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> > > > Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > > > He is the author of the patch, no?! > > or you can remove this or change the author to be you and keep his > > reviewed-by... > > > > or I can just remove his rv-b while merging.. just let me know.. > As he is original author I will prefer not to change it. You can > remove > his rv-b while merging. Thanks and pushed. > > Regards, > Badal > > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > --- > > > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 > > > ++++++++++++++++++ > > > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 ++++ > > > 2 files changed, 22 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > > b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > > index b0a4a2dbe3ee..e971b153fda9 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > > @@ -15,6 +15,22 @@ > > > #include "intel_rc6.h" > > > #include "intel_ring.h" > > > #include "shmem_utils.h" > > > +#include "intel_gt_regs.h" > > > + > > > +static void intel_gsc_idle_msg_enable(struct intel_engine_cs > > > *engine) > > > +{ > > > + struct drm_i915_private *i915 = engine->i915; > > > + > > > + if (IS_METEORLAKE(i915) && engine->id == GSC0) { > > > + intel_uncore_write(engine->gt->uncore, > > > + RC_PSMI_CTRL_GSCCS, > > > + > > > _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE)); > > > + /* hysteresis 0xA=5us as recommended in spec*/ > > > + intel_uncore_write(engine->gt->uncore, > > > + PWRCTX_MAXCNT_GSCCS, > > > + 0xA); > > > + } > > > +} > > > > > > static void dbg_poison_ce(struct intel_context *ce) > > > { > > > @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct > > > intel_engine_cs > > > *engine) > > > > > > intel_wakeref_init(&engine->wakeref, rpm, &wf_ops); > > > intel_engine_init_heartbeat(engine); > > > + > > > + intel_gsc_idle_msg_enable(engine); > > > } > > > > > > /** > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > > b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > > index c3cd92691795..80a979e6f6be 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > > @@ -917,6 +917,10 @@ > > > #define MSG_IDLE_FW_MASK REG_GENMASK(13, 9) > > > #define MSG_IDLE_FW_SHIFT 9 > > > > > > +#define RC_PSMI_CTRL_GSCCS _MMIO(0x11a050) > > > +#define IDLE_MSG_DISABLE REG_BIT(0) > > > +#define PWRCTX_MAXCNT_GSCCS _MMIO(0x11a054) > > > + > > > #define FORCEWAKE_MEDIA_GEN9 _MMIO(0xa270) > > > #define FORCEWAKE_RENDER_GEN9 _MMIO(0xa278) > > > > >
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index b0a4a2dbe3ee..e971b153fda9 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -15,6 +15,22 @@ #include "intel_rc6.h" #include "intel_ring.h" #include "shmem_utils.h" +#include "intel_gt_regs.h" + +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine) +{ + struct drm_i915_private *i915 = engine->i915; + + if (IS_METEORLAKE(i915) && engine->id == GSC0) { + intel_uncore_write(engine->gt->uncore, + RC_PSMI_CTRL_GSCCS, + _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE)); + /* hysteresis 0xA=5us as recommended in spec*/ + intel_uncore_write(engine->gt->uncore, + PWRCTX_MAXCNT_GSCCS, + 0xA); + } +} static void dbg_poison_ce(struct intel_context *ce) { @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine) intel_wakeref_init(&engine->wakeref, rpm, &wf_ops); intel_engine_init_heartbeat(engine); + + intel_gsc_idle_msg_enable(engine); } /** diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index c3cd92691795..80a979e6f6be 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -917,6 +917,10 @@ #define MSG_IDLE_FW_MASK REG_GENMASK(13, 9) #define MSG_IDLE_FW_SHIFT 9 +#define RC_PSMI_CTRL_GSCCS _MMIO(0x11a050) +#define IDLE_MSG_DISABLE REG_BIT(0) +#define PWRCTX_MAXCNT_GSCCS _MMIO(0x11a054) + #define FORCEWAKE_MEDIA_GEN9 _MMIO(0xa270) #define FORCEWAKE_RENDER_GEN9 _MMIO(0xa278)