Re: [PATCH v3 3/4] drm/i915: Skip CURBASE write when nothing changed

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sat, Sep 13, 2014 at 05:20:10PM +0100, Chris Wilson wrote:
> On Fri, Sep 12, 2014 at 08:53:34PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Only write CURBASE when something about the cursor changed. Also
> > eliminate the unnecessary posting read after writing CURCNTR.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 82c0ad1..60c1aa4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8322,16 +8322,18 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> >  			cntl |= CURSOR_PIPE_CSC_ENABLE;
> >  	}
> >  
> > -	if (intel_crtc->cursor_cntl != cntl) {
> > +	if (intel_crtc->cursor_cntl != cntl)
> >  		I915_WRITE(CURCNTR(pipe), cntl);
> > -		POSTING_READ(CURCNTR(pipe));
> > -		intel_crtc->cursor_cntl = cntl;
> > -	}
> > +
> > +	if (intel_crtc->cursor_cntl == cntl &&
> > +	    intel_crtc->cursor_base == base)
> > +		return;
> 
> I'd vote for doing this first and then
> 
> I915_WRITE(CURCNTR(pipe), cntl);
> 
> unconditionally along with the CURBASE flush.

So you want to write all the cursor registers unconditionally when
anything changes? Would work for CURCNTR, but it would also make
CUR_FBC_CTL stand out like a sore thumb since it can't be written
uncoditionally.

> 
> >  	/* and commit changes on next vblank */
> >  	I915_WRITE(CURBASE(pipe), base);
> >  	POSTING_READ(CURBASE(pipe));
> >  
> > +	intel_crtc->cursor_cntl = cntl;
> >  	intel_crtc->cursor_base = base;
> >  }
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC