Re: [PATCH 6/9] drm/i915: Add a small adjustment to the pixel counter on interlaced modes

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

On Thu, May 15, 2014 at 08:23:47PM +0530, akash goel wrote:
> Reviewed the patch & it looks good.
> Just to confirm, this patch tries to address the case of a tiny window of
> transition, i.e. from the 1st field (last half line) to 2nd field (first
> half line).

The hardware counts things so that one field ends up being one line
taller than the other, ie. both half lines get accounted for the same
field (as far as the pixel counter is concerned at least).

So the total number of pixels in the fields is like this:
field A = htotal * floor(vtotal/2)
field B = htotal *  ceil(vtotal/2)

But when we compute the scanout position we assume that the total
number of pixels is always 'htotal * floor(vtotal/2)'. So if we start
with a number that is greater than that, the value wraps back to zero
already before we reach the real 0. And then when we do reach the
real zero, the scanout position would appear to jump backwards. So this
patch eliminates that problem by making it appear that the scanout
position stops moving when we're on that last line of the taller field.
Occasionally non-moving (but still non-decreasing) scanout position
seems like a lesser evil than one that jumps back and forth at times.

And as stated, this more or less matches the scanline counter based
scanout position since the scanline counter doesn't include the half
lines.

> 
> Reviewed-by: "Akash Goel <akash.goels@gmail.com>"
> 
> 
> On Tue, Apr 29, 2014 at 4:05 PM, <ville.syrjala@linux.intel.com> wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > In interlaced modes, the pixel counter counts all pixels,
> > so one field will have htotal more pixels. In order to avoid
> > the reported position from jumping backwards when the pixel
> > counter is beyond the length of the shorter field, just
> > clamp the position the length of the shorter field. This
> > matches how the scanline counter based position works since
> > the scanline counter doesn't count the two half lines.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 7e0d577..64cd888 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -844,6 +844,18 @@ static int i915_get_crtc_scanoutpos(struct drm_device
> > *dev, int pipe,
> >                 vtotal *= htotal;
> >
> >                 /*
> > +                * In interlaced modes, the pixel counter counts all
> > pixels,
> > +                * so one field will have htotal more pixels. In order to
> > avoid
> > +                * the reported position from jumping backwards when the
> > pixel
> > +                * counter is beyond the length of the shorter field, just
> > +                * clamp the position the length of the shorter field. This
> > +                * matches how the scanline counter based position works
> > since
> > +                * the scanline counter doesn't count the two half lines.
> > +                */
> > +               if (position >= vtotal)
> > +                       position = vtotal - 1;
> > +
> > +               /*
> >                  * Start of vblank interrupt is triggered at start of
> > hsync,
> >                  * just prior to the first active line of vblank. However
> > we
> >                  * consider lines to start at the leading edge of
> > horizontal
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >

-- 
Ville Syrjälä
Intel OTC