Re: [PATCH 7/5] drm/i915: Improve gen3/4 frame counter

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3591 bytes --]

On Tue, 2014-02-18 at 16:41 +0200, Ville Syrjälä wrote:
> On Tue, Feb 18, 2014 at 04:16:00PM +0200, Imre Deak wrote:
> > On Tue, 2014-02-18 at 14:04 +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Currently the logic to fix up the frame counter on gen3/4 assumes that
> > > start of vblank occurs at vblank_start*htotal pixels, when in fact
> > > it occurs htotal-hsync_start pixels earlier. Apply the appropriate
> > > adjustment to make the frame counter more accurate.
> > > 
> > > Also fix the vblank start position for interlaced display modes.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_irq.c | 21 ++++++++++++++++-----
> > >  1 file changed, 16 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index 9f1c449..fc49fb6 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -639,7 +639,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
> > >  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> > >  	unsigned long high_frame;
> > >  	unsigned long low_frame;
> > > -	u32 high1, high2, low, pixel, vbl_start;
> > > +	u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
> > >  
> > >  	if (!i915_pipe_enabled(dev, pipe)) {
> > >  		DRM_DEBUG_DRIVER("trying to get vblank count for disabled "
> > > @@ -653,17 +653,28 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
> > >  		const struct drm_display_mode *mode =
> > >  			&intel_crtc->config.adjusted_mode;
> > >  
> > > -		vbl_start = mode->crtc_vblank_start * mode->crtc_htotal;
> > > +		htotal = mode->crtc_htotal;
> > > +		hsync_start = mode->crtc_hsync_start;
> > > +		vbl_start = mode->crtc_vblank_start;
> > > +		if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> > > +			vbl_start = DIV_ROUND_UP(vbl_start, 2);
> > 
> > The adjustment for interlace mode is already done in drm_mode_setcrtc,
> > so I think we don't need it here.
> 
> We throw away the values filled in by drm_mode_setcrtc(). Which is a
> good thing since it rounds the result the wrong way (for our hardware
> at least). The values we see here are filled in by
> intel_modeset_pipe_config() which doesn't pass the
> CRTC_INTERLACE_HALVE_V flag to drm_mode_set_crtcinfo().

Ah, true, I see now. This looks also ok then.

--Imre

> > Otherwise this patch makes sense to me
> > based on the signal chart you drew on this, so:
> > 
> > Reviewed-by: Imre Deak <imre.deak@intel.com>
> > 
> > >  	} else {
> > >  		enum transcoder cpu_transcoder = (enum transcoder) pipe;
> > > -		u32 htotal;
> > >  
> > >  		htotal = ((I915_READ(HTOTAL(cpu_transcoder)) >> 16) & 0x1fff) + 1;
> > > +		hsync_start = (I915_READ(HSYNC(cpu_transcoder))  & 0x1fff) + 1;
> > >  		vbl_start = (I915_READ(VBLANK(cpu_transcoder)) & 0x1fff) + 1;
> > > -
> > > -		vbl_start *= htotal;
> > > +		if ((I915_READ(PIPECONF(cpu_transcoder)) &
> > > +		     PIPECONF_INTERLACE_MASK) != PIPECONF_PROGRESSIVE)
> > > +			vbl_start = DIV_ROUND_UP(vbl_start, 2);
> > >  	}
> > >  
> > > +	/* Convert to pixel count */
> > > +	vbl_start *= htotal;
> > > +
> > > +	/* Start of vblank event occurs at start of hsync */
> > > +	vbl_start -= htotal - hsync_start;
> > > +
> > >  	high_frame = PIPEFRAME(pipe);
> > >  	low_frame = PIPEFRAMEPIXEL(pipe);
> > >  
> > 
> 
> 
> 


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx