[Intel-gfx] [PATCH 02/22] Xv i830_display_video splitup: extract i830_calc_src_regs

[Daniel Vetter]

> > Also introduce an is_planar_fourcc helper. I'll use that one later.
...
> > +    if (planar) {
> > +	*swidth_out = width | ((width/2 & 0x7ff) << 16);
> > +	swidthy  = i830_swidth (pI830, offsety, width, mask, shift);
> > +	swidthuv = i830_swidth (pI830, offsetu, width/2, mask, shift);
> > +	*swidthsw_out = (swidthy) | (swidthuv << 16);
> > +	*sheigth_out = height | ((height / 2) << 16);
> > +    } else {
> > +	*swidth_out = width;
> > +	swidth = i830_swidth (pI830, offsety, width << 1, mask, shift);
> > +	*swidthsw_out = swidth;
> > +	*sheigth_out = height;
> > +    }
> 
> This will behave differently than the previous code for the case where id is
> FOURCC_XVMC.  In the original code this would have been handled by the default
> case of the switch-statement, which has become the else part of this
> if-statement.  However, this will give planar=true, and it will be handled by
> the if part of the if-statement.
> 
> So, the question is whether or not id can be FOURCC_XVMC at that part of
> i830_display_video.  If it can be, was the original code correct?  If it
> cannot be, when that case is encountered the code should give an error, or
> assert, or something.

Uups, good catch. I've forgotten that I've changed something there. XvMC
is everywhere else handled as a planar format (e.g. in the register
programming a few lines down) and that default didn't make sense there. So
I've just changed it. Furthermore the id variable gets mapped to
FOURCC_YV12 if IS_I915(pI830) is true in I830PutImage. There's a second
caller in the offscreen overlay support code.  But I think that code is
bitrotten and not reliable as an information source.

So we have a different behaviour only for id=FOURCC_XVMC and i965 class hw
(i830 class doesn't have xvmc). I've crawled through various sources/intel
documentations. Finally in the textured video implemention for i965 class
hw (src/i965_video.c) I've found a switch statement that puts XVMC into
the same case as I420 and YV12. So also in i965 class hw xvmc uses a
planar format.

In conclusion I claim that this code was bogus and XvMC on i965 class hw
over Xv overlay was most likely broken.

If you agree with this and merge the patch I'd say it's best to add my
comments here in the commit message.

-Daniel
-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: 079 365 57 48