On Sat, May 06, 2017 at 12:31:50AM +0300, Imre Deak wrote:
> On Mon, Mar 27, 2017 at 09:55:46PM +0300, [email protected] wrote:
> > From: Ville Syrjälä <[email protected]>
> >
> > 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ä <[email protected]>
>
> Reviewed-by: Imre Deak <[email protected]>
Thanks for the reviews. Entire series pushed to dinq.
>
> > ---
> > 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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
