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

[Paul Menzel]

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>

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