[Intel-gfx] [PATCH 3/4] drm/i915: WaFbcDisableDpfcrClockGating only with fbc

[Paulo Zanoni]

2013/10/24 Ben Widawsky <benjamin.widawsky at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni