diff mbox

[2/3] drm/i915: Program DSPCLK_GATE_D only once on Ironlake

Message ID 1350479190-3023-2-git-send-email-damien.lespiau@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Damien Lespiau Oct. 17, 2012, 1:06 p.m. UTC
From: Damien Lespiau <damien.lespiau@intel.com>

With the consolidated registers, it appears that we're setting the same
bis several times. Let's just collect the bit we want to set and program
it once.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

Comments

Paulo Zanoni Oct. 18, 2012, 2:48 p.m. UTC | #1
2012/10/17 Damien Lespiau <damien.lespiau@gmail.com>:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> With the consolidated registers, it appears that we're setting the same
> bis several times. Let's just collect the bit we want to set and program

s/bis/bits/

> it once.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |   15 ++++-----------
>  1 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 76e10c0..be42b2c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3316,8 +3316,6 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
>         I915_WRITE(PCH_3DCGDIS1,
>                    VFMUNIT_CLOCK_GATE_DISABLE);
>
> -       I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate);
> -
>         /*
>          * According to the spec the following bits should be set in
>          * order to enable memory self-refresh
> @@ -3328,9 +3326,7 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
>         I915_WRITE(ILK_DISPLAY_CHICKEN2,
>                    (I915_READ(ILK_DISPLAY_CHICKEN2) |
>                     ILK_DPARB_GATE | ILK_VSDPFD_FULL));
> -       I915_WRITE(ILK_DSPCLK_GATE_D,
> -                  (I915_READ(ILK_DSPCLK_GATE_D) |
> -                   ILK_DPARBUNIT_CLOCK_GATE_DISABLE));
> +       dspclk_gate |= ILK_DPARBUNIT_CLOCK_GATE_DISABLE;

We are still doing "dspclk_gate |= ILK_DPARBUNIT_CLOCK_GATE_DISABLE;"
twice: here and also at the beginning of the function.

If you look at the function after this patch it looks like this:

ironlake_init_clock_gating {

/* Required for FBC */
dspclk_gate |= XXX;

/* Required for CxSR */
dspclk_gate |= YYY;

/* According to the spec, required by CxSR, big comment */
dspclk_gate |= YYY;

/* Based on document, required by FBC, big comment */
(no "dpslclk_gate |= XXX" setting here since it is already above)
}

I was told that CxSR is the same as memory self-refresh, so I guess we
could clean up the function by de-duplicating even the comments since
they're talking about the same things. My suggestion would be to
eliminate the first comments/code, leaving just the multi-line
comments and setting dspclk_gate after each of these (also taking care
to set the FBC bits not only for mobile).


>         I915_WRITE(DISP_ARB_CTL,
>                    (I915_READ(DISP_ARB_CTL) |
>                     DISP_FBC_WM_DIS));
> @@ -3343,7 +3339,7 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
>          * should be set unconditionally in order to enable FBC.
>          * The bit 22 of 0x42000
>          * The bit 22 of 0x42004
> -        * The bit 7,8,9 of 0x42020.
> +        * The bit 7,8,9 of 0x42020 (dspclk_gate is set above)
>          */
>         if (IS_IRONLAKE_M(dev)) {
>                 I915_WRITE(ILK_DISPLAY_CHICKEN1,
> @@ -3352,13 +3348,10 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
>                 I915_WRITE(ILK_DISPLAY_CHICKEN2,
>                            I915_READ(ILK_DISPLAY_CHICKEN2) |
>                            ILK_DPARB_GATE);
> -               I915_WRITE(ILK_DSPCLK_GATE_D,
> -                          I915_READ(ILK_DSPCLK_GATE_D) |
> -                          ILK_DPFCRUNIT_CLOCK_GATE_DISABLE |
> -                          ILK_DPFCUNIT_CLOCK_GATE_DISABLE |
> -                          ILK_DPFDUNIT_CLOCK_GATE_DISABLE);
>         }
>
> +       I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate);
> +
>         I915_WRITE(ILK_DISPLAY_CHICKEN2,
>                    I915_READ(ILK_DISPLAY_CHICKEN2) |
>                    ILK_ELPIN_409_SELECT);
> --
> 1.7.7.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 76e10c0..be42b2c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3316,8 +3316,6 @@  static void ironlake_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(PCH_3DCGDIS1,
 		   VFMUNIT_CLOCK_GATE_DISABLE);
 
-	I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate);
-
 	/*
 	 * According to the spec the following bits should be set in
 	 * order to enable memory self-refresh
@@ -3328,9 +3326,7 @@  static void ironlake_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(ILK_DISPLAY_CHICKEN2,
 		   (I915_READ(ILK_DISPLAY_CHICKEN2) |
 		    ILK_DPARB_GATE | ILK_VSDPFD_FULL));
-	I915_WRITE(ILK_DSPCLK_GATE_D,
-		   (I915_READ(ILK_DSPCLK_GATE_D) |
-		    ILK_DPARBUNIT_CLOCK_GATE_DISABLE));
+	dspclk_gate |= ILK_DPARBUNIT_CLOCK_GATE_DISABLE;
 	I915_WRITE(DISP_ARB_CTL,
 		   (I915_READ(DISP_ARB_CTL) |
 		    DISP_FBC_WM_DIS));
@@ -3343,7 +3339,7 @@  static void ironlake_init_clock_gating(struct drm_device *dev)
 	 * should be set unconditionally in order to enable FBC.
 	 * The bit 22 of 0x42000
 	 * The bit 22 of 0x42004
-	 * The bit 7,8,9 of 0x42020.
+	 * The bit 7,8,9 of 0x42020 (dspclk_gate is set above)
 	 */
 	if (IS_IRONLAKE_M(dev)) {
 		I915_WRITE(ILK_DISPLAY_CHICKEN1,
@@ -3352,13 +3348,10 @@  static void ironlake_init_clock_gating(struct drm_device *dev)
 		I915_WRITE(ILK_DISPLAY_CHICKEN2,
 			   I915_READ(ILK_DISPLAY_CHICKEN2) |
 			   ILK_DPARB_GATE);
-		I915_WRITE(ILK_DSPCLK_GATE_D,
-			   I915_READ(ILK_DSPCLK_GATE_D) |
-			   ILK_DPFCRUNIT_CLOCK_GATE_DISABLE |
-			   ILK_DPFCUNIT_CLOCK_GATE_DISABLE |
-			   ILK_DPFDUNIT_CLOCK_GATE_DISABLE);
 	}
 
+	I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate);
+
 	I915_WRITE(ILK_DISPLAY_CHICKEN2,
 		   I915_READ(ILK_DISPLAY_CHICKEN2) |
 		   ILK_ELPIN_409_SELECT);