On Mon, Mar 27, 2017 at 09:55:46PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> 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.syrj...@linux.intel.com>

Reviewed-by: Imre Deak <imre.d...@intel.com>

> ---
>  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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to