Re: [PATCH] DRM planes

[Daniel Vetter <daniel@ffwll.ch>]

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.

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.

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48