| Message ID | 1382633954-7375-3-git-send-email-benjamin.widawsky@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2013/10/24 Ben Widawsky <benjamin.widawsky@intel.com>: > We were turning this on for ILK regardless of whether or not we use FBC. > We can save the slightest amount of power if we don't disable it when > not using FBC. Finally someone did what I requested months ago: http://lists.freedesktop.org/archives/intel-gfx/2013-June/028906.html :) > > The workaround should be bit 8 for ILK. Notice it is 1 bit difference > from SNB. This is actually DPFCR unit as we've defined it. Ok, so we have bits 8 and 9. Judging by the register names, I would say bit 9 is WaFbcDisableDpfcClockGating (without 'r') and bit 8 is the ironlake-only WaFbcDisableDpfcrClockGating. Is that right? > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 686699c..bbcf100 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -238,6 +238,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval) > SNB_CPU_FENCE_ENABLE | obj->fence_reg); > I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y); > sandybridge_blit_fbc_update(dev); > + } else { > + /* WaFbcDisableDpfcClockGating:ilk */ If you agree with me on the question above, then the WA name here is missing an 'r' char. > + I915_WRITE(ILK_DSPCLK_GATE_D, > + I915_READ(ILK_DSPCLK_GATE_D) | > + ILK_DPFCRUNIT_CLOCK_GATE_DISABLE); > } > > DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane)); > @@ -254,6 +259,12 @@ static void ironlake_disable_fbc(struct drm_device *dev) > dpfc_ctl &= ~DPFC_CTL_EN; > I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl); > > + if (IS_GEN5(dev)) > + /* WaFbcDisableDpfcClockGating:ilk */ Same here. > + I915_WRITE(ILK_DSPCLK_GATE_D, > + I915_READ(ILK_DSPCLK_GATE_D) & > + ~ILK_DPFCRUNIT_CLOCK_GATE_DISABLE); > + > DRM_DEBUG_KMS("disabled FBC\n"); > } > } > @@ -4932,9 +4943,9 @@ static void ironlake_init_clock_gating(struct drm_device *dev) > > /* > * Required for FBC > - * WaFbcDisableDpfcClockGating:ilk > + * WaFbcDisableDpfcClockGating:snb The ":snb" part is certainly wrong since this function doesn't run on SNB. The actual code (excluding comments) looks correct. > */ > - dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE | > + dspclk_gate |= > ILK_DPFCUNIT_CLOCK_GATE_DISABLE | > ILK_DPFDUNIT_CLOCK_GATE_ENABLE; > > -- > 1.8.4.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Oct 25, 2013 at 03:14:50PM -0200, Paulo Zanoni wrote: > 2013/10/24 Ben Widawsky <benjamin.widawsky@intel.com>: > > We were turning this on for ILK regardless of whether or not we use FBC. > > We can save the slightest amount of power if we don't disable it when > > not using FBC. > > Finally someone did what I requested months ago: > http://lists.freedesktop.org/archives/intel-gfx/2013-June/028906.html > :) > > > > > > The workaround should be bit 8 for ILK. Notice it is 1 bit difference > > from SNB. This is actually DPFCR unit as we've defined it. > > Ok, so we have bits 8 and 9. Judging by the register names, I would > say bit 9 is WaFbcDisableDpfcClockGating (without 'r') and bit 8 is > the ironlake-only WaFbcDisableDpfcrClockGating. Is that right? Actually from what I remember in the database (not looking currently), the workaround names are the same, but maybe my eyes missed the 'R'. I differentiated it with the 'R' so that anyone who was reviewing it didn't go insane. > > > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 686699c..bbcf100 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -238,6 +238,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval) > > SNB_CPU_FENCE_ENABLE | obj->fence_reg); > > I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y); > > sandybridge_blit_fbc_update(dev); > > + } else { > > + /* WaFbcDisableDpfcClockGating:ilk */ > > If you agree with me on the question above, then the WA name here is > missing an 'r' char. I guess I have to go back to the database now to check. I don't have the bookmark anywhere on my machines here. > > > > + I915_WRITE(ILK_DSPCLK_GATE_D, > > + I915_READ(ILK_DSPCLK_GATE_D) | > > + ILK_DPFCRUNIT_CLOCK_GATE_DISABLE); > > } > > > > DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane)); > > @@ -254,6 +259,12 @@ static void ironlake_disable_fbc(struct drm_device *dev) > > dpfc_ctl &= ~DPFC_CTL_EN; > > I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl); > > > > + if (IS_GEN5(dev)) > > + /* WaFbcDisableDpfcClockGating:ilk */ > > Same here. > > > > + I915_WRITE(ILK_DSPCLK_GATE_D, > > + I915_READ(ILK_DSPCLK_GATE_D) & > > + ~ILK_DPFCRUNIT_CLOCK_GATE_DISABLE); > > + > > DRM_DEBUG_KMS("disabled FBC\n"); > > } > > } > > @@ -4932,9 +4943,9 @@ static void ironlake_init_clock_gating(struct drm_device *dev) > > > > /* > > * Required for FBC > > - * WaFbcDisableDpfcClockGating:ilk > > + * WaFbcDisableDpfcClockGating:snb > > The ":snb" part is certainly wrong since this function doesn't run on SNB. > > The actual code (excluding comments) looks correct. > Oops, thanks. The diff should just remove the line completely. > > > */ > > - dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE | > > + dspclk_gate |= > > ILK_DPFCUNIT_CLOCK_GATE_DISABLE | > > ILK_DPFDUNIT_CLOCK_GATE_ENABLE; > > > > -- > > 1.8.4.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 686699c..bbcf100 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -238,6 +238,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval) SNB_CPU_FENCE_ENABLE | obj->fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y); sandybridge_blit_fbc_update(dev); + } else { + /* WaFbcDisableDpfcClockGating:ilk */ + I915_WRITE(ILK_DSPCLK_GATE_D, + I915_READ(ILK_DSPCLK_GATE_D) | + ILK_DPFCRUNIT_CLOCK_GATE_DISABLE); } DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane)); @@ -254,6 +259,12 @@ static void ironlake_disable_fbc(struct drm_device *dev) dpfc_ctl &= ~DPFC_CTL_EN; I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl); + if (IS_GEN5(dev)) + /* WaFbcDisableDpfcClockGating:ilk */ + I915_WRITE(ILK_DSPCLK_GATE_D, + I915_READ(ILK_DSPCLK_GATE_D) & + ~ILK_DPFCRUNIT_CLOCK_GATE_DISABLE); + DRM_DEBUG_KMS("disabled FBC\n"); } } @@ -4932,9 +4943,9 @@ static void ironlake_init_clock_gating(struct drm_device *dev) /* * Required for FBC - * WaFbcDisableDpfcClockGating:ilk + * WaFbcDisableDpfcClockGating:snb */ - dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE | + dspclk_gate |= ILK_DPFCUNIT_CLOCK_GATE_DISABLE | ILK_DPFDUNIT_CLOCK_GATE_ENABLE;
We were turning this on for ILK regardless of whether or not we use FBC. We can save the slightest amount of power if we don't disable it when not using FBC. The workaround should be bit 8 for ILK. Notice it is 1 bit difference from SNB. This is actually DPFCR unit as we've defined it. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)