| Message ID | 1524256446-28490-3-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 20, 2018 at 01:33:46PM -0700, Oscar Mateo wrote: > Sampler Dynamic Frequency Rebalancing (DFR) aims to reduce Sampler > power by dynamically changing its clock frequency in low-throughput > conditions. This patches enables it by default on Gen11. > > v2: Wrong operation to clear the bit (Praveen) > v3: Rebased on top of the WA refactoring > > Cc: Praveen Paneri <praveen.paneri@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++ > 2 files changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index f2ee225..4b7e6bc 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8218,6 +8218,9 @@ enum { > #define GEN8_GARBCNTL _MMIO(0xB004) > #define GEN9_GAPS_TSV_CREDIT_DISABLE (1<<7) > > +#define GEN10_DFR_RATIO_EN_AND_CHICKEN _MMIO(0x9550) > +#define DFR_DISABLE (1 << 9) > + > /* IVYBRIDGE DPF */ > #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) /* L3CD Error Status 1 */ > #define GEN7_L3CDERRST1_ROW_MASK (0x7ff<<14) > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > index 3f00623..60a5b1d 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -692,6 +692,10 @@ static void icl_gt_workarounds_apply(struct drm_i915_private *dev_priv) > I915_WRITE(_3D_CHICKEN3, > _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE)); > > + /* This is not an Wa. Enable to reduce Sampler power */ First of all it is strange that a feature is under a chicken bit, but if it is not an wa, but a PM feature, shouldn't we add it to some sort of init clock gating function, or a new specific function called when it makes sense? > + I915_WRITE(GEN10_DFR_RATIO_EN_AND_CHICKEN, > + (I915_READ(GEN10_DFR_RATIO_EN_AND_CHICKEN) & ~DFR_DISABLE)); > + > /* WaInPlaceDecompressionHang:icl */ > I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA, (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) | > GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS)); > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 04/20/2018 01:48 PM, Rodrigo Vivi wrote: > On Fri, Apr 20, 2018 at 01:33:46PM -0700, Oscar Mateo wrote: >> Sampler Dynamic Frequency Rebalancing (DFR) aims to reduce Sampler >> power by dynamically changing its clock frequency in low-throughput >> conditions. This patches enables it by default on Gen11. >> >> v2: Wrong operation to clear the bit (Praveen) >> v3: Rebased on top of the WA refactoring >> >> Cc: Praveen Paneri <praveen.paneri@intel.com> >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 3 +++ >> drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++ >> 2 files changed, 7 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index f2ee225..4b7e6bc 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -8218,6 +8218,9 @@ enum { >> #define GEN8_GARBCNTL _MMIO(0xB004) >> #define GEN9_GAPS_TSV_CREDIT_DISABLE (1<<7) >> >> +#define GEN10_DFR_RATIO_EN_AND_CHICKEN _MMIO(0x9550) >> +#define DFR_DISABLE (1 << 9) >> + >> /* IVYBRIDGE DPF */ >> #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) /* L3CD Error Status 1 */ >> #define GEN7_L3CDERRST1_ROW_MASK (0x7ff<<14) >> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c >> index 3f00623..60a5b1d 100644 >> --- a/drivers/gpu/drm/i915/intel_workarounds.c >> +++ b/drivers/gpu/drm/i915/intel_workarounds.c >> @@ -692,6 +692,10 @@ static void icl_gt_workarounds_apply(struct drm_i915_private *dev_priv) >> I915_WRITE(_3D_CHICKEN3, >> _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE)); >> >> + /* This is not an Wa. Enable to reduce Sampler power */ > First of all it is strange that a feature is under a chicken bit, There is a precedent: _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE > but if it is not an wa, but a PM feature, shouldn't we add it to some sort of > init clock gating function, or a new specific function called when it makes sense? I'm open to suggestions >> + I915_WRITE(GEN10_DFR_RATIO_EN_AND_CHICKEN, >> + (I915_READ(GEN10_DFR_RATIO_EN_AND_CHICKEN) & ~DFR_DISABLE)); >> + >> /* WaInPlaceDecompressionHang:icl */ >> I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA, (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) | >> GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS)); >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Apr 20, 2018 at 01:52:24PM -0700, Oscar Mateo wrote: > > > On 04/20/2018 01:48 PM, Rodrigo Vivi wrote: > > On Fri, Apr 20, 2018 at 01:33:46PM -0700, Oscar Mateo wrote: > > > Sampler Dynamic Frequency Rebalancing (DFR) aims to reduce Sampler > > > power by dynamically changing its clock frequency in low-throughput > > > conditions. This patches enables it by default on Gen11. > > > > > > v2: Wrong operation to clear the bit (Praveen) > > > v3: Rebased on top of the WA refactoring > > > > > > Cc: Praveen Paneri <praveen.paneri@intel.com> > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > > Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++ > > > 2 files changed, 7 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index f2ee225..4b7e6bc 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -8218,6 +8218,9 @@ enum { > > > #define GEN8_GARBCNTL _MMIO(0xB004) > > > #define GEN9_GAPS_TSV_CREDIT_DISABLE (1<<7) > > > +#define GEN10_DFR_RATIO_EN_AND_CHICKEN _MMIO(0x9550) > > > +#define DFR_DISABLE (1 << 9) > > > + > > > /* IVYBRIDGE DPF */ > > > #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) /* L3CD Error Status 1 */ > > > #define GEN7_L3CDERRST1_ROW_MASK (0x7ff<<14) > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > > > index 3f00623..60a5b1d 100644 > > > --- a/drivers/gpu/drm/i915/intel_workarounds.c > > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > > > @@ -692,6 +692,10 @@ static void icl_gt_workarounds_apply(struct drm_i915_private *dev_priv) > > > I915_WRITE(_3D_CHICKEN3, > > > _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE)); > > > + /* This is not an Wa. Enable to reduce Sampler power */ > > First of all it is strange that a feature is under a chicken bit, > > There is a precedent: _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE Oh, true! Actually that never got same question from me because that function is called init clock gating, although this function nowadays is basically workarounds related to clock gatings :/) > > > but if it is not an wa, but a PM feature, shouldn't we add it to some sort of > > init clock gating function, or a new specific function called when it makes sense? > > I'm open to suggestions moving to clock gating is an option? or needs to be called from context? > > > > + I915_WRITE(GEN10_DFR_RATIO_EN_AND_CHICKEN, > > > + (I915_READ(GEN10_DFR_RATIO_EN_AND_CHICKEN) & ~DFR_DISABLE)); > > > + > > > /* WaInPlaceDecompressionHang:icl */ > > > I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA, (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) | > > > GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS)); > > > -- > > > 1.9.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 04/20/2018 02:26 PM, Rodrigo Vivi wrote: > On Fri, Apr 20, 2018 at 01:52:24PM -0700, Oscar Mateo wrote: >> >> On 04/20/2018 01:48 PM, Rodrigo Vivi wrote: >>> On Fri, Apr 20, 2018 at 01:33:46PM -0700, Oscar Mateo wrote: >>>> Sampler Dynamic Frequency Rebalancing (DFR) aims to reduce Sampler >>>> power by dynamically changing its clock frequency in low-throughput >>>> conditions. This patches enables it by default on Gen11. >>>> >>>> v2: Wrong operation to clear the bit (Praveen) >>>> v3: Rebased on top of the WA refactoring >>>> >>>> Cc: Praveen Paneri <praveen.paneri@intel.com> >>>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >>>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >>>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >>>> --- >>>> drivers/gpu/drm/i915/i915_reg.h | 3 +++ >>>> drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++ >>>> 2 files changed, 7 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >>>> index f2ee225..4b7e6bc 100644 >>>> --- a/drivers/gpu/drm/i915/i915_reg.h >>>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>>> @@ -8218,6 +8218,9 @@ enum { >>>> #define GEN8_GARBCNTL _MMIO(0xB004) >>>> #define GEN9_GAPS_TSV_CREDIT_DISABLE (1<<7) >>>> +#define GEN10_DFR_RATIO_EN_AND_CHICKEN _MMIO(0x9550) >>>> +#define DFR_DISABLE (1 << 9) >>>> + >>>> /* IVYBRIDGE DPF */ >>>> #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) /* L3CD Error Status 1 */ >>>> #define GEN7_L3CDERRST1_ROW_MASK (0x7ff<<14) >>>> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c >>>> index 3f00623..60a5b1d 100644 >>>> --- a/drivers/gpu/drm/i915/intel_workarounds.c >>>> +++ b/drivers/gpu/drm/i915/intel_workarounds.c >>>> @@ -692,6 +692,10 @@ static void icl_gt_workarounds_apply(struct drm_i915_private *dev_priv) >>>> I915_WRITE(_3D_CHICKEN3, >>>> _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE)); >>>> + /* This is not an Wa. Enable to reduce Sampler power */ >>> First of all it is strange that a feature is under a chicken bit, >> There is a precedent: _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE > Oh, true! > > Actually that never got same question from me because that > function is called init clock gating, although this function nowadays > is basically workarounds related to clock gatings :/) > >>> but if it is not an wa, but a PM feature, shouldn't we add it to some sort of >>> init clock gating function, or a new specific function called when it makes sense? >> I'm open to suggestions > moving to clock gating is an option? > or needs to be called from context? No, moving it to init_clock_gating is definitely an option. It is not truly related to clock gating, but at least it is related to Power Management... (the _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE thing is more difficult to swallow...) >>>> + I915_WRITE(GEN10_DFR_RATIO_EN_AND_CHICKEN, >>>> + (I915_READ(GEN10_DFR_RATIO_EN_AND_CHICKEN) & ~DFR_DISABLE)); >>>> + >>>> /* WaInPlaceDecompressionHang:icl */ >>>> I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA, (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) | >>>> GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS)); >>>> -- >>>> 1.9.1 >>>> >>>> _______________________________________________ >>>> Intel-gfx mailing list >>>> Intel-gfx@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Apr 20, 2018 at 02:29:27PM -0700, Oscar Mateo wrote: > > > On 04/20/2018 02:26 PM, Rodrigo Vivi wrote: > > On Fri, Apr 20, 2018 at 01:52:24PM -0700, Oscar Mateo wrote: > > > > > > On 04/20/2018 01:48 PM, Rodrigo Vivi wrote: > > > > On Fri, Apr 20, 2018 at 01:33:46PM -0700, Oscar Mateo wrote: > > > > > Sampler Dynamic Frequency Rebalancing (DFR) aims to reduce Sampler > > > > > power by dynamically changing its clock frequency in low-throughput > > > > > conditions. This patches enables it by default on Gen11. > > > > > > > > > > v2: Wrong operation to clear the bit (Praveen) > > > > > v3: Rebased on top of the WA refactoring > > > > > > > > > > Cc: Praveen Paneri <praveen.paneri@intel.com> > > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > > > > Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > > > drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++ > > > > > 2 files changed, 7 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > > > index f2ee225..4b7e6bc 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > @@ -8218,6 +8218,9 @@ enum { > > > > > #define GEN8_GARBCNTL _MMIO(0xB004) > > > > > #define GEN9_GAPS_TSV_CREDIT_DISABLE (1<<7) > > > > > +#define GEN10_DFR_RATIO_EN_AND_CHICKEN _MMIO(0x9550) > > > > > +#define DFR_DISABLE (1 << 9) > > > > > + > > > > > /* IVYBRIDGE DPF */ > > > > > #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) /* L3CD Error Status 1 */ > > > > > #define GEN7_L3CDERRST1_ROW_MASK (0x7ff<<14) > > > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > > > > > index 3f00623..60a5b1d 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_workarounds.c > > > > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > > > > > @@ -692,6 +692,10 @@ static void icl_gt_workarounds_apply(struct drm_i915_private *dev_priv) > > > > > I915_WRITE(_3D_CHICKEN3, > > > > > _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE)); > > > > > + /* This is not an Wa. Enable to reduce Sampler power */ > > > > First of all it is strange that a feature is under a chicken bit, > > > There is a precedent: _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE > > Oh, true! > > > > Actually that never got same question from me because that > > function is called init clock gating, although this function nowadays > > is basically workarounds related to clock gatings :/) > > > > > > but if it is not an wa, but a PM feature, shouldn't we add it to some sort of > > > > init clock gating function, or a new specific function called when it makes sense? > > > I'm open to suggestions > > moving to clock gating is an option? > > or needs to be called from context? > > No, moving it to init_clock_gating is definitely an option. It is not truly > related to clock gating, but at least it is related to Power Management... > (the _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE thing is more difficult to > swallow...) I agree hehe... my bad... :$ > > > > > > + I915_WRITE(GEN10_DFR_RATIO_EN_AND_CHICKEN, > > > > > + (I915_READ(GEN10_DFR_RATIO_EN_AND_CHICKEN) & ~DFR_DISABLE)); > > > > > + > > > > > /* WaInPlaceDecompressionHang:icl */ > > > > > I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA, (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) | > > > > > GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS)); > > > > > -- > > > > > 1.9.1 > > > > > > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f2ee225..4b7e6bc 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8218,6 +8218,9 @@ enum { #define GEN8_GARBCNTL _MMIO(0xB004) #define GEN9_GAPS_TSV_CREDIT_DISABLE (1<<7) +#define GEN10_DFR_RATIO_EN_AND_CHICKEN _MMIO(0x9550) +#define DFR_DISABLE (1 << 9) + /* IVYBRIDGE DPF */ #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) /* L3CD Error Status 1 */ #define GEN7_L3CDERRST1_ROW_MASK (0x7ff<<14) diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c index 3f00623..60a5b1d 100644 --- a/drivers/gpu/drm/i915/intel_workarounds.c +++ b/drivers/gpu/drm/i915/intel_workarounds.c @@ -692,6 +692,10 @@ static void icl_gt_workarounds_apply(struct drm_i915_private *dev_priv) I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE)); + /* This is not an Wa. Enable to reduce Sampler power */ + I915_WRITE(GEN10_DFR_RATIO_EN_AND_CHICKEN, + (I915_READ(GEN10_DFR_RATIO_EN_AND_CHICKEN) & ~DFR_DISABLE)); + /* WaInPlaceDecompressionHang:icl */ I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA, (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) | GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS));