| Message ID | 1392725061-30144-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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); >
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(). > 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); > > >
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); > > > > > > > >
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); } 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);