[Intel-gfx] [PATCH] drm/i915: Fix cursor updates on some platforms

[Daniel Vetter]

On Mon, Jul 17, 2017 at 12:42:18PM +0200, Paul Menzel wrote:
> Dear Ville,
> 
> 
> On 07/14/17 17:52, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Turns out that just writing CURPOS isn't sufficient to move the cursor
> > on some platforms. My 830 works just fine, but eg. 945 and PNV don't.
> > On those platforms we need to arm even the CURPOS update with a
> > CURBASE write.
> > 
> > Even worse, a write to any of the cursor register apart from CURBASE
> > will cancel an already pending cursor update. So if we have armed a
> > CURCNTR/CURBASE update, a subsequent CURPOS write prior to vblank
> > would cancel that armed update. Thus we're left with a cursor that
> > doesn't appear to move, or even change shape.
> > 
> > Fix the problem by always performing the CURBASE write after a
> > CURPOS write. Bspec is somewhat unclear which platforms actually
> > require this CURBASE write and which don't. So to keep it simple
> > and to make sure we really fix the problem across all supported
> > devices, let's just perform the CURBASE write unconditionally.
> > 
> > Cc: Paul Menzel <pmenzel at molgen.mpg.de>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101790
> > Fixes: 75343a44c901 ("drm/i915: Drop useless posting reads from cursor commit")
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 2144adc5b1d5..460bd942fcb7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9555,7 +9555,16 @@ static void i9xx_update_cursor(struct intel_plane *plane,
> >   	 * On some platforms writing CURCNTR first will also
> >   	 * cause CURPOS to be armed by the CURBASE write.
> >   	 * Without the CURCNTR write the CURPOS write would
> > -	 * arm itself.
> > +	 * arm itself. Thus we always start the full update
> > +	 * with a CURCNTR write.
> > +	 *
> > +	 * On other platforms CURPOS always requires the
> > +	 * CURBASE write to arm the update. Additonally
> > +	 * a write to any of the cursor register will cancel
> > +	 * an already armed cursor update. Thus leaving out
> > +	 * the CURBASE write after CURPOS could lead to a
> > +	 * cursor that doesn't appear to move, or even change
> > +	 * shape. Thus we always write CURBASE.
> >   	 *
> >   	 * CURCNTR and CUR_FBC_CTL are always
> >   	 * armed by the CURBASE write only.
> > @@ -9574,6 +9583,7 @@ static void i9xx_update_cursor(struct intel_plane *plane,
> >   		plane->cursor.cntl = cntl;
> >   	} else {
> >   		I915_WRITE_FW(CURPOS(pipe), pos);
> > +		I915_WRITE_FW(CURBASE(pipe), base);
> >   	}
> >   	POSTING_READ_FW(CURBASE(pipe));
> 
> Tested-by: Paul Menzel <paulepanter at users.sourceforge.net>

Since Ville is on vacation I applied this.
-Daniel

> 
> I created two more bugs for an issue with pasting text with the middle mouse
> button [1], and failing *kms_cursor_crc* tests [2].
> 
> 
> Kind regards,
> 
> Paul Menzel
> 
> 
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=101819
> [2] https://bugs.freedesktop.org/show_bug.cgi?id=101817
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch