[Intel-gfx] [PATCH] DRM planes

[Jesse Barnes]

On Thu, 3 Nov 2011 15:11:55 +0100
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Wed, Nov 02, 2011 at 01:03:18PM -0700, Jesse Barnes wrote:
> > In response to feedback, I've adjusted the new addfb2 ioctl to take per
> > component pitch and offset args.  Generally, the offset[0] field will be
> > 0, but it's conceivable that some metadata could be stored at the start
> > of a given buffer, and an offset[0] allows the client to skip past that.
> > Similarly, pitch[0] will typically describe the whole buffer, but it's
> > possible to simply string together several planes into a single object
> > where individual pitch components matter.
> > 
> > Userland patches are available in the drm-overlays branches of my
> > personal libdrm and xf86-video-intel trees at freedesktop.org.  The
> > xf86-video-intel side works well enough to handle clipping (using a new
> > i915 specific ioctl for setting a destination color key) and play
> > videos, albeit without nice flipping.
> > 
> > Assuming no major objections, I think this is finally ready for
> > drm-next.
> 
> Well, the fool I am I've attempted to write a nice little drm plane
> wrapper for the legacy intel overlay code. Code is totally untested, but I
> like what it looks like - the drm plane interfaces are a quite natural fit
> to the existing code. There's one thing though which is imo a bloody mess
> (and I've given up writing the code for it), namely planar yuv handling
> with fourcc pixelformats. The current implementation for snb+ doesn't
> stumble over that rock because it only supports packed yuv.
> 
> So here goes the rant: The legacy overlay (and my ioctl) accept a set off
> offset for Y, U and V planes (i.e. it could even accept different bos, but
> the current ioctl designed for Xv doesn't). The hw has the restriction
> that the strides for the U and V planes need to be identical, but that's
> it. Pixelformat is just an enumeration of YUV422,420,411,410 to specify
> the subsampling ratio of the U/V planes.
> 
> Now fourcc specifies all these offset implicitly in the pixelformat (at
> least that's what I've figured out, I might be wrong though):
> - Some formats have the YUV planes in a special, fixed arrangement, no
>   additional offset needs to be passed in and strides are implicit from
>   the width.
> - Some have a separate offset somewhere for UV planes, but the UV planes
>   are again in a fixed layout (either interleaved or one after another).
> - Some fourcc have all three planes independant, i.e. you need an offset
>   for the U and the V plane. No clue what that implies for the stride.
> 
> In short decoding these fourcc values with all their implicit assumptions
> about offset, strides and whatnotelse will be one giant switch mess. Not
> my idea of a nice kernel interface. Also practically guaranteed to result
> in slightly different behaviour in each driver.

There may be some v4l code we can share here, or at least refactor, for
use by drivers that want to handle planar formats like the old
overlay.  Did you look for any?

> 
> So here's the new radical proposal (and yep, I expect heat for this):
> - Ditch fourcc. Afaics this wheel is square and no amount of sharpening
>   the corners will make it round.
> - Ditch addfb support for planar yuv formats, at least for the moment.
>   Until we have more than one kms driver for this I don't think we can
>   come up with any sane interface.
> 
> In conclusion: Ditch the addfb2ioctl and just add a few more kms pixel
> formats for the new packed yuv layouts that the snb+ sprite code needs.

We'll still need the new ioctl though.  We don't have enough info today
(just bpp and depth) in the existing addfb ioctl to figure out what the
actual plane layout is.

We can move away from fourcc, but it still seems like we'd end up
duplicating it at least a little, as our full set of supported formats
would be the superset of what each driver supports.

I'm not wedded to fourcc, but I guess we'll have to see what the TI and
Samsung guys say to make a decision.

-- 
Jesse Barnes, Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20111103/4ec067ed/attachment.sig>