[PATCH 0/2] drm/i915: Gen2 vblank stuff

[PATCH 0/2] drm/i915: Gen2 vblank stuff

[ville.syrjala]

Continuing a bit with improving the vblank timestamp stuff, I decided that
it's also time to fix gen2 properly.

All this time we've been pretending it has the same pixel+frame counter that
gen3 and gen4 have, when in fact it doesn't. Luckily the same offset has
nothing else on gen2, so it has just read out as 0 and everything more or
less worked.

Sadly there doesn't seem to be any frame counter register on gen2. For the
scanout position, DSL + additional tricks works equally well for gen2 as
it does for ctg+.

This series should be applied on top of my earlier vblank/scanout position
improvements.

[PATCH 1/2] drm/i915: Fix gen2 scanout position readout

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Gen2 doesn't have the pixelcount register that gen3 and gen4 have.
Instead we must use the scanline counter like we do for ctg+.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 63a9642..3c31a2f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -592,7 +592,7 @@ static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)
 	return I915_READ(reg);
 }
 
-static bool g4x_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
+static bool intel_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t status;
@@ -603,7 +603,13 @@ static bool g4x_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
 			I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
 
 		return I915_READ(VLV_ISR) & status;
-	} else if (IS_G4X(dev)) {
+	} else if (IS_GEN2(dev)) {
+		status = pipe == PIPE_A ?
+			I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
+			I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
+
+		return I915_READ16(ISR) & status;
+	} else if (INTEL_INFO(dev)->gen < 5) {
 		status = pipe == PIPE_A ?
 			I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
 			I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
@@ -658,11 +664,14 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
 
 	ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE;
 
-	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
+	if (IS_GEN2(dev) || IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
 		/* No obvious pixelcount register. Only query vertical
 		 * scanout position from Display scan line register.
 		 */
-		position = I915_READ(PIPEDSL(pipe)) & 0x1fff;
+		if (IS_GEN2(dev))
+			position = I915_READ(PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
+		else
+			position = I915_READ(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
 
 		/*
 		 * The scanline counter increments at the leading edge
@@ -671,7 +680,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
 		 * to get a more accurate picture whether we're in vblank
 		 * or not.
 		 */
-		in_vbl = g4x_pipe_in_vblank(dev, pipe);
+		in_vbl = intel_pipe_in_vblank(dev, pipe);
 		if ((in_vbl && position == vbl_start - 1) ||
 		    (!in_vbl && position == vbl_end - 1))
 			position = (position + 1) % vtotal;
@@ -701,7 +710,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
 	else
 		position += vtotal - vbl_end;
 
-	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
+	if (IS_GEN2(dev) || IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
 		*vpos = position;
 		*hpos = 0;
 	} else {
-- 
1.8.1.5

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

Re: [PATCH 1/2] drm/i915: Fix gen2 scanout position readout

[Rodrigo Vivi]

I was going to get this for -collector but got a big conflict.


On Fri, Oct 11, 2013 at 3:52 PM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Gen2 doesn't have the pixelcount register that gen3 and gen4 have.
> Instead we must use the scanline counter like we do for ctg+.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 63a9642..3c31a2f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -592,7 +592,7 @@ static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)
>         return I915_READ(reg);
>  }
>
> -static bool g4x_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
> +static bool intel_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         uint32_t status;
> @@ -603,7 +603,13 @@ static bool g4x_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
>                         I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
>
>                 return I915_READ(VLV_ISR) & status;
> -       } else if (IS_G4X(dev)) {
> +       } else if (IS_GEN2(dev)) {
> +               status = pipe == PIPE_A ?
> +                       I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
> +                       I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
> +
> +               return I915_READ16(ISR) & status;
> +       } else if (INTEL_INFO(dev)->gen < 5) {
>                 status = pipe == PIPE_A ?
>                         I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
>                         I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
> @@ -658,11 +664,14 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
>
>         ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE;
>
> -       if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> +       if (IS_GEN2(dev) || IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
>                 /* No obvious pixelcount register. Only query vertical
>                  * scanout position from Display scan line register.
>                  */
> -               position = I915_READ(PIPEDSL(pipe)) & 0x1fff;
> +               if (IS_GEN2(dev))
> +                       position = I915_READ(PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
> +               else
> +                       position = I915_READ(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
>
>                 /*
>                  * The scanline counter increments at the leading edge
> @@ -671,7 +680,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
>                  * to get a more accurate picture whether we're in vblank
>                  * or not.
>                  */
> -               in_vbl = g4x_pipe_in_vblank(dev, pipe);
> +               in_vbl = intel_pipe_in_vblank(dev, pipe);
>                 if ((in_vbl && position == vbl_start - 1) ||
>                     (!in_vbl && position == vbl_end - 1))
>                         position = (position + 1) % vtotal;
> @@ -701,7 +710,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
>         else
>                 position += vtotal - vbl_end;
>
> -       if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> +       if (IS_GEN2(dev) || IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
>                 *vpos = position;
>                 *hpos = 0;
>         } else {
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 1/2] drm/i915: Fix gen2 scanout position readout

[Ville Syrjälä]

On Wed, Nov 06, 2013 at 10:38:42AM -0200, Rodrigo Vivi wrote:
> I was going to get this for -collector but got a big conflict.

It should be already in. Although as it turns out it's partially
crap. Apparently I hadn't tested the ISR vblank bit behaviour
properly on non-PCH platforms. It doesn't actually work the way
we want, so there are patches in my latest vblank timestamp series
that kill the ISR parts and add a kludge to work around the scanline
counter's silly behaviour.
> 
> 
> On Fri, Oct 11, 2013 at 3:52 PM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Gen2 doesn't have the pixelcount register that gen3 and gen4 have.
> > Instead we must use the scanline counter like we do for ctg+.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 63a9642..3c31a2f 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -592,7 +592,7 @@ static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)
> >         return I915_READ(reg);
> >  }
> >
> > -static bool g4x_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
> > +static bool intel_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         uint32_t status;
> > @@ -603,7 +603,13 @@ static bool g4x_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
> >                         I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
> >
> >                 return I915_READ(VLV_ISR) & status;
> > -       } else if (IS_G4X(dev)) {
> > +       } else if (IS_GEN2(dev)) {
> > +               status = pipe == PIPE_A ?
> > +                       I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
> > +                       I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
> > +
> > +               return I915_READ16(ISR) & status;
> > +       } else if (INTEL_INFO(dev)->gen < 5) {
> >                 status = pipe == PIPE_A ?
> >                         I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
> >                         I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
> > @@ -658,11 +664,14 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
> >
> >         ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE;
> >
> > -       if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> > +       if (IS_GEN2(dev) || IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> >                 /* No obvious pixelcount register. Only query vertical
> >                  * scanout position from Display scan line register.
> >                  */
> > -               position = I915_READ(PIPEDSL(pipe)) & 0x1fff;
> > +               if (IS_GEN2(dev))
> > +                       position = I915_READ(PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
> > +               else
> > +                       position = I915_READ(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
> >
> >                 /*
> >                  * The scanline counter increments at the leading edge
> > @@ -671,7 +680,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
> >                  * to get a more accurate picture whether we're in vblank
> >                  * or not.
> >                  */
> > -               in_vbl = g4x_pipe_in_vblank(dev, pipe);
> > +               in_vbl = intel_pipe_in_vblank(dev, pipe);
> >                 if ((in_vbl && position == vbl_start - 1) ||
> >                     (!in_vbl && position == vbl_end - 1))
> >                         position = (position + 1) % vtotal;
> > @@ -701,7 +710,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
> >         else
> >                 position += vtotal - vbl_end;
> >
> > -       if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> > +       if (IS_GEN2(dev) || IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> >                 *vpos = position;
> >                 *hpos = 0;
> >         } else {
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
Ville Syrjälä
Intel OTC

[PATCH 2/2] drm/i915: Don't pretend that gen2 has a hardware frame counter

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Gen2 doesn't have a hardware frame counter that can be read out. Just
provide a stub .get_vblank_counter() that always returns 0 instead of
trying to read non-existing registers.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 3c31a2f..73781a0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -518,6 +518,12 @@ i915_pipe_enabled(struct drm_device *dev, int pipe)
 	}
 }
 
+static u32 i8xx_get_vblank_counter(struct drm_device *dev, int pipe)
+{
+	/* Gen2 doesn't have a hardware frame counter */
+	return 0;
+}
+
 /* Called from drm generic code, passed a 'crtc', which
  * we use as a pipe index
  */
@@ -3231,7 +3237,10 @@ void intel_irq_init(struct drm_device *dev)
 
 	pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
 
-	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
+	if (IS_GEN2(dev)) {
+		dev->max_vblank_count = 0;
+		dev->driver->get_vblank_counter = i8xx_get_vblank_counter;
+	} else if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
 		dev->max_vblank_count = 0xffffffff; /* full 32 bit counter */
 		dev->driver->get_vblank_counter = gm45_get_vblank_counter;
 	} else {
-- 
1.8.1.5

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

Re: [PATCH 0/2] drm/i915: Gen2 vblank stuff

[Chris Wilson]

On Fri, Oct 11, 2013 at 09:52:42PM +0300, ville.syrjala@linux.intel.com wrote:
> Continuing a bit with improving the vblank timestamp stuff, I decided that
> it's also time to fix gen2 properly.
> 
> All this time we've been pretending it has the same pixel+frame counter that
> gen3 and gen4 have, when in fact it doesn't. Luckily the same offset has
> nothing else on gen2, so it has just read out as 0 and everything more or
> less worked.
> 
> Sadly there doesn't seem to be any frame counter register on gen2. For the
> scanout position, DSL + additional tricks works equally well for gen2 as
> it does for ctg+.

Confirmed the absence of said registers, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/2] drm/i915: Gen2 vblank stuff

[Daniel Vetter]

On Mon, Oct 14, 2013 at 11:01:11AM +0100, Chris Wilson wrote:
> On Fri, Oct 11, 2013 at 09:52:42PM +0300, ville.syrjala@linux.intel.com wrote:
> > Continuing a bit with improving the vblank timestamp stuff, I decided that
> > it's also time to fix gen2 properly.
> > 
> > All this time we've been pretending it has the same pixel+frame counter that
> > gen3 and gen4 have, when in fact it doesn't. Luckily the same offset has
> > nothing else on gen2, so it has just read out as 0 and everything more or
> > less worked.
> > 
> > Sadly there doesn't seem to be any frame counter register on gen2. For the
> > scanout position, DSL + additional tricks works equally well for gen2 as
> > it does for ctg+.
> 
> Confirmed the absence of said registers, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Both merged, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch