| Message ID | 1354638799-3987-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi 2012/12/4 Chris Wilson <chris@chris-wilson.co.uk>: > In a couple of places we attempt to adjust the existing watermark > registers to update them for the new cursor watermarks. This goes > horribly wrong as instead of clearing the cursor bits prior to or'ing in > the new values, we clear the rest of the register with the result that > the watermark registers contain bogus values. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=47034 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Nice catch... > --- > drivers/gpu/drm/i915/intel_pm.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 20daf58..f6b4697 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -1334,10 +1334,11 @@ static void valleyview_update_wm(struct drm_device *dev) > (planeb_wm << DSPFW_PLANEB_SHIFT) | > planea_wm); > I915_WRITE(DSPFW2, > - (I915_READ(DSPFW2) & DSPFW_CURSORA_MASK) | > + (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) | > (cursora_wm << DSPFW_CURSORA_SHIFT)); > I915_WRITE(DSPFW3, > - (I915_READ(DSPFW3) | (cursor_sr << DSPFW_CURSOR_SR_SHIFT))); > + (I915_READ(DSPFW3) & ~DSPFW_CURSOR_SR_MASK) | > + (cursor_sr << DSPFW_CURSOR_SR_SHIFT)); > } > > static void g4x_update_wm(struct drm_device *dev) > @@ -1383,11 +1384,11 @@ static void g4x_update_wm(struct drm_device *dev) > (planeb_wm << DSPFW_PLANEB_SHIFT) | > planea_wm); > I915_WRITE(DSPFW2, > - (I915_READ(DSPFW2) & DSPFW_CURSORA_MASK) | > + (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) | > (cursora_wm << DSPFW_CURSORA_SHIFT)); > /* HPLL off in SR has some issues on G4x... disable it */ > I915_WRITE(DSPFW3, > - (I915_READ(DSPFW3) & ~DSPFW_HPLL_SR_EN) | > + (I915_READ(DSPFW3) & ~(DSPFW_HPLL_SR_EN | DSPFW_CURSOR_SR_MASK)) | > (cursor_sr << DSPFW_CURSOR_SR_SHIFT)); > } > > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Dec 04, 2012 at 05:48:53PM -0200, Paulo Zanoni wrote: > Hi > > 2012/12/4 Chris Wilson <chris@chris-wilson.co.uk>: > > In a couple of places we attempt to adjust the existing watermark > > registers to update them for the new cursor watermarks. This goes > > horribly wrong as instead of clearing the cursor bits prior to or'ing in > > the new values, we clear the rest of the register with the result that > > the watermark registers contain bogus values. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=47034 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Nice catch... Indeed. Picked up for -fixes, thanks for the patch. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 20daf58..f6b4697 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1334,10 +1334,11 @@ static void valleyview_update_wm(struct drm_device *dev) (planeb_wm << DSPFW_PLANEB_SHIFT) | planea_wm); I915_WRITE(DSPFW2, - (I915_READ(DSPFW2) & DSPFW_CURSORA_MASK) | + (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) | (cursora_wm << DSPFW_CURSORA_SHIFT)); I915_WRITE(DSPFW3, - (I915_READ(DSPFW3) | (cursor_sr << DSPFW_CURSOR_SR_SHIFT))); + (I915_READ(DSPFW3) & ~DSPFW_CURSOR_SR_MASK) | + (cursor_sr << DSPFW_CURSOR_SR_SHIFT)); } static void g4x_update_wm(struct drm_device *dev) @@ -1383,11 +1384,11 @@ static void g4x_update_wm(struct drm_device *dev) (planeb_wm << DSPFW_PLANEB_SHIFT) | planea_wm); I915_WRITE(DSPFW2, - (I915_READ(DSPFW2) & DSPFW_CURSORA_MASK) | + (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) | (cursora_wm << DSPFW_CURSORA_SHIFT)); /* HPLL off in SR has some issues on G4x... disable it */ I915_WRITE(DSPFW3, - (I915_READ(DSPFW3) & ~DSPFW_HPLL_SR_EN) | + (I915_READ(DSPFW3) & ~(DSPFW_HPLL_SR_EN | DSPFW_CURSOR_SR_MASK)) | (cursor_sr << DSPFW_CURSOR_SR_SHIFT)); }
In a couple of places we attempt to adjust the existing watermark registers to update them for the new cursor watermarks. This goes horribly wrong as instead of clearing the cursor bits prior to or'ing in the new values, we clear the rest of the register with the result that the watermark registers contain bogus values. References: https://bugs.freedesktop.org/show_bug.cgi?id=47034 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_pm.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)