2012/10/17 Damien Lespiau <damien.lespiau at gmail.com>: > From: Damien Lespiau <damien.lespiau at 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 at 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 at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni