On Mon, Mar 27, 2017 at 09:55:46PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> It looks like simply writing all the cursor register every single
> time might be slightly faster than checking to see of each of
> them need to be written. So if any other register apart from
> CURPOS needs to be written let's just write all the registers.
>
> CURPOS is left as a special case mainly for 845/865 where we have to
> disable the cursor to change many of the cursor parameters. This
> introduces a slight chance of the cursor flickering when things get
> updated (since we're not currently doing the vblank evade for cursor
> updates). If we write CURPOS alone then that obviously can't happen.
> And let's follow the same pattern in the i9xx code just for symmetry.
> I wasn't able to see a singificant performance difference between
> this and just writing all the registers unconditionally.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 69 ++++++++++++++++++------------------
> 1 file changed, 34 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d78ab0d35274..40ac0f938a4e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9323,36 +9323,28 @@ static void i845_update_cursor(struct intel_plane *plane,
>
> spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>
> - if (plane->cursor.cntl != 0 &&
> - (plane->cursor.base != base ||
> - plane->cursor.size != size ||
> - plane->cursor.cntl != cntl)) {
> - /* On these chipsets we can only modify the base/size/stride
> - * whilst the cursor is disabled.
> - */
> + /* On these chipsets we can only modify the base/size/stride
> + * whilst the cursor is disabled.
> + */
> + if (plane->cursor.base != base ||
> + plane->cursor.size != size ||
> + plane->cursor.cntl != cntl) {
> I915_WRITE_FW(CURCNTR(PIPE_A), 0);
> - plane->cursor.cntl = 0;
> - }
> -
> - if (plane->cursor.base != base)
> I915_WRITE_FW(CURBASE(PIPE_A), base);
> -
> - if (plane->cursor.size != size)
> I915_WRITE_FW(CURSIZE, size);
> -
> - if (cntl)
> I915_WRITE_FW(CURPOS(PIPE_A), pos);
> -
> - if (plane->cursor.cntl != cntl)
> I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
>
> + plane->cursor.base = base;
> + plane->cursor.size = size;
> + plane->cursor.cntl = cntl;
> + } else {
> + I915_WRITE_FW(CURPOS(PIPE_A), pos);
> + }
> +
> POSTING_READ_FW(CURCNTR(PIPE_A));
>
> spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> -
> - plane->cursor.cntl = cntl;
> - plane->cursor.base = base;
> - plane->cursor.size = size;
> }
>
> static void i845_disable_cursor(struct intel_plane *plane,
> @@ -9508,27 +9500,34 @@ static void i9xx_update_cursor(struct intel_plane *plane,
>
> spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>
> - if (plane->cursor.cntl != cntl)
> + /*
> + * 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.
> + *
> + * CURCNTR and CUR_FBC_CTL are always
> + * armed by the CURBASE write only.
> + */
> + if (plane->cursor.base != base ||
> + plane->cursor.size != fbc_ctl ||
> + plane->cursor.cntl != cntl) {
> I915_WRITE_FW(CURCNTR(pipe), cntl);
> -
> - if (plane->cursor.size != fbc_ctl)
> - I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
> -
> - if (cntl)
> + if (HAS_CUR_FBC(dev_priv))
> + I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
> I915_WRITE_FW(CURPOS(pipe), pos);
> -
> - if (plane->cursor.cntl != cntl ||
> - plane->cursor.size != fbc_ctl ||
> - plane->cursor.base != base)
> I915_WRITE_FW(CURBASE(pipe), base);
>
> + plane->cursor.base = base;
> + plane->cursor.size = fbc_ctl;
> + plane->cursor.cntl = cntl;
> + } else {
> + I915_WRITE_FW(CURPOS(pipe), pos);
> + }
> +
> POSTING_READ_FW(CURBASE(pipe));
>
> spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> -
> - plane->cursor.cntl = cntl;
> - plane->cursor.base = base;
> - plane->cursor.size = fbc_ctl;
> }
>
> static void i9xx_disable_cursor(struct intel_plane *plane,
> --
> 2.10.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
