diff mbox series

[2/2] drm/i915/tv: Use the scanline counter for timestamps on i965gm TV output

Message ID 20190125181931.19482-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/tv: Fix return value for intel_tv_compute_config() | expand

Commit Message

Ville Syrjala Jan. 25, 2019, 6:19 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just like the frame counter, the pixel counter also reads zero
all the time when the TV encoder is used. Fortunately the
scanline counter still works sufficiently well so let's use that
to correct the vblank timestamps. Otherwise the timestamps may
en up out of whack, and since we use them to guesstimate the
vblank counter value that may end up incorrect as well.

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c  |  7 +++++--
 drivers/gpu/drm/i915/intel_drv.h |  4 +++-
 drivers/gpu/drm/i915/intel_tv.c  | 10 ++++++++++
 3 files changed, 18 insertions(+), 3 deletions(-)

Comments

Imre Deak Jan. 25, 2019, 8:17 p.m. UTC | #1 
On Fri, Jan 25, 2019 at 08:19:31PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Just like the frame counter, the pixel counter also reads zero
> all the time when the TV encoder is used. Fortunately the
> scanline counter still works sufficiently well so let's use that
> to correct the vblank timestamps. Otherwise the timestamps may
> en up out of whack, and since we use them to guesstimate the
> vblank counter value that may end up incorrect as well.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_irq.c  |  7 +++++--
>  drivers/gpu/drm/i915/intel_drv.h |  4 +++-
>  drivers/gpu/drm/i915/intel_tv.c  | 10 ++++++++++
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index fe097725c27a..caade521c174 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1014,6 +1014,9 @@ static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
>  	int position;
>  	int vbl_start, vbl_end, hsync_start, htotal, vtotal;
>  	unsigned long irqflags;
> +	bool use_scanline_counter = INTEL_GEN(dev_priv) >= 5 ||
> +		IS_G4X(dev_priv) || IS_GEN(dev_priv, 2) ||
> +		mode->private_flags & I915_MODE_FLAG_USE_SCANLINE_COUNTER;
>  
>  	if (WARN_ON(!mode->crtc_clock)) {
>  		DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled "
> @@ -1046,7 +1049,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
>  	if (stime)
>  		*stime = ktime_get();
>  
> -	if (IS_GEN(dev_priv, 2) || IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) {
> +	if (use_scanline_counter) {
>  		/* No obvious pixelcount register. Only query vertical
>  		 * scanout position from Display scan line register.
>  		 */
> @@ -1106,7 +1109,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
>  	else
>  		position += vtotal - vbl_end;
>  
> -	if (IS_GEN(dev_priv, 2) || IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) {
> +	if (use_scanline_counter) {
>  		*vpos = position;
>  		*hpos = 0;
>  	} else {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 85b913ea6e80..90ba5436370e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -630,9 +630,11 @@ struct intel_crtc_scaler_state {
>  };
>  
>  /* drm_mode->private_flags */
> -#define I915_MODE_FLAG_INHERITED 1
> +#define I915_MODE_FLAG_INHERITED (1<<0)
>  /* Flag to get scanline using frame time stamps */
>  #define I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1)
> +/* Flag to use the scanline counter instead of the pixel counter */
> +#define I915_MODE_FLAG_USE_SCANLINE_COUNTER (1<<2)
>  
>  struct intel_pipe_wm {
>  	struct intel_wm_level wm[5];
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 78be08e2971b..751b88dde18e 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1150,6 +1150,11 @@ intel_tv_get_config(struct intel_encoder *encoder,
>  				 ypos, mode.vdisplay - ysize - ypos);
>  
>  	adjusted_mode->crtc_clock = mode.clock;
> +
> +	/* pixel counter doesn't work on i965gm TV output */
> +	if (IS_I965GM(dev_priv))
> +		adjusted_mode->private_flags |=
> +			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
>  }
>  
>  static int
> @@ -1295,6 +1300,11 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>  	drm_mode_set_crtcinfo(adjusted_mode, 0);
>  	adjusted_mode->name[0] = '\0';
>  
> +	/* pixel counter doesn't work on i965gm TV output */
> +	if (IS_I965GM(dev_priv))
> +		adjusted_mode->private_flags |=
> +			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
> +
>  	return 0;
>  }
>  
> -- 
> 2.19.2
>
Ville Syrjala Jan. 28, 2019, 2:01 p.m. UTC | #2 
On Fri, Jan 25, 2019 at 10:17:04PM +0200, Imre Deak wrote:
> On Fri, Jan 25, 2019 at 08:19:31PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Just like the frame counter, the pixel counter also reads zero
> > all the time when the TV encoder is used. Fortunately the
> > scanline counter still works sufficiently well so let's use that
> > to correct the vblank timestamps. Otherwise the timestamps may
> > en up out of whack, and since we use them to guesstimate the
> > vblank counter value that may end up incorrect as well.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks. Series pushed to dinq.

> 
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c  |  7 +++++--
> >  drivers/gpu/drm/i915/intel_drv.h |  4 +++-
> >  drivers/gpu/drm/i915/intel_tv.c  | 10 ++++++++++
> >  3 files changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index fe097725c27a..caade521c174 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1014,6 +1014,9 @@ static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
> >  	int position;
> >  	int vbl_start, vbl_end, hsync_start, htotal, vtotal;
> >  	unsigned long irqflags;
> > +	bool use_scanline_counter = INTEL_GEN(dev_priv) >= 5 ||
> > +		IS_G4X(dev_priv) || IS_GEN(dev_priv, 2) ||
> > +		mode->private_flags & I915_MODE_FLAG_USE_SCANLINE_COUNTER;
> >  
> >  	if (WARN_ON(!mode->crtc_clock)) {
> >  		DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled "
> > @@ -1046,7 +1049,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
> >  	if (stime)
> >  		*stime = ktime_get();
> >  
> > -	if (IS_GEN(dev_priv, 2) || IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) {
> > +	if (use_scanline_counter) {
> >  		/* No obvious pixelcount register. Only query vertical
> >  		 * scanout position from Display scan line register.
> >  		 */
> > @@ -1106,7 +1109,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
> >  	else
> >  		position += vtotal - vbl_end;
> >  
> > -	if (IS_GEN(dev_priv, 2) || IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) {
> > +	if (use_scanline_counter) {
> >  		*vpos = position;
> >  		*hpos = 0;
> >  	} else {
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 85b913ea6e80..90ba5436370e 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -630,9 +630,11 @@ struct intel_crtc_scaler_state {
> >  };
> >  
> >  /* drm_mode->private_flags */
> > -#define I915_MODE_FLAG_INHERITED 1
> > +#define I915_MODE_FLAG_INHERITED (1<<0)
> >  /* Flag to get scanline using frame time stamps */
> >  #define I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1)
> > +/* Flag to use the scanline counter instead of the pixel counter */
> > +#define I915_MODE_FLAG_USE_SCANLINE_COUNTER (1<<2)
> >  
> >  struct intel_pipe_wm {
> >  	struct intel_wm_level wm[5];
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> > index 78be08e2971b..751b88dde18e 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -1150,6 +1150,11 @@ intel_tv_get_config(struct intel_encoder *encoder,
> >  				 ypos, mode.vdisplay - ysize - ypos);
> >  
> >  	adjusted_mode->crtc_clock = mode.clock;
> > +
> > +	/* pixel counter doesn't work on i965gm TV output */
> > +	if (IS_I965GM(dev_priv))
> > +		adjusted_mode->private_flags |=
> > +			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
> >  }
> >  
> >  static int
> > @@ -1295,6 +1300,11 @@ intel_tv_compute_config(struct intel_encoder *encoder,
> >  	drm_mode_set_crtcinfo(adjusted_mode, 0);
> >  	adjusted_mode->name[0] = '\0';
> >  
> > +	/* pixel counter doesn't work on i965gm TV output */
> > +	if (IS_I965GM(dev_priv))
> > +		adjusted_mode->private_flags |=
> > +			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.19.2
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index fe097725c27a..caade521c174 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1014,6 +1014,9 @@  static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
 	int position;
 	int vbl_start, vbl_end, hsync_start, htotal, vtotal;
 	unsigned long irqflags;
+	bool use_scanline_counter = INTEL_GEN(dev_priv) >= 5 ||
+		IS_G4X(dev_priv) || IS_GEN(dev_priv, 2) ||
+		mode->private_flags & I915_MODE_FLAG_USE_SCANLINE_COUNTER;
 
 	if (WARN_ON(!mode->crtc_clock)) {
 		DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled "
@@ -1046,7 +1049,7 @@  static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
 	if (stime)
 		*stime = ktime_get();
 
-	if (IS_GEN(dev_priv, 2) || IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) {
+	if (use_scanline_counter) {
 		/* No obvious pixelcount register. Only query vertical
 		 * scanout position from Display scan line register.
 		 */
@@ -1106,7 +1109,7 @@  static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
 	else
 		position += vtotal - vbl_end;
 
-	if (IS_GEN(dev_priv, 2) || IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) {
+	if (use_scanline_counter) {
 		*vpos = position;
 		*hpos = 0;
 	} else {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 85b913ea6e80..90ba5436370e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -630,9 +630,11 @@  struct intel_crtc_scaler_state {
 };
 
 /* drm_mode->private_flags */
-#define I915_MODE_FLAG_INHERITED 1
+#define I915_MODE_FLAG_INHERITED (1<<0)
 /* Flag to get scanline using frame time stamps */
 #define I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1)
+/* Flag to use the scanline counter instead of the pixel counter */
+#define I915_MODE_FLAG_USE_SCANLINE_COUNTER (1<<2)
 
 struct intel_pipe_wm {
 	struct intel_wm_level wm[5];
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 78be08e2971b..751b88dde18e 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1150,6 +1150,11 @@  intel_tv_get_config(struct intel_encoder *encoder,
 				 ypos, mode.vdisplay - ysize - ypos);
 
 	adjusted_mode->crtc_clock = mode.clock;
+
+	/* pixel counter doesn't work on i965gm TV output */
+	if (IS_I965GM(dev_priv))
+		adjusted_mode->private_flags |=
+			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
 }
 
 static int
@@ -1295,6 +1300,11 @@  intel_tv_compute_config(struct intel_encoder *encoder,
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 	adjusted_mode->name[0] = '\0';
 
+	/* pixel counter doesn't work on i965gm TV output */
+	if (IS_I965GM(dev_priv))
+		adjusted_mode->private_flags |=
+			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
+
 	return 0;
 }