On Wed, Aug 28, 2013 at 11:51:59PM -0400, Rob Clark wrote:
> On Wed, Aug 28, 2013 at 9:51 PM, Greg Hackmann <ghackmann@xxxxxxxxxx> wrote:
> > Hi,
> >
> > ADF is an experimental display framework that I designed after experimenting with a KMS-based hardware composer for Android. ADF started as an proof-of-concept implemented from scratch, so right now it's a separate framework rather than a patchstack on top of KMS. If there's community interest, moving forward I'd like to merge its functionality into KMS rather than keep it as a separate thing.
> >
> > I'm going to talk about ADF at the Android and Graphics session at Linux Plumbers. The documentation's not done but I wanted to post these patches to people a heads-up about ADF. If there's interest I can write up some more formal documentation ahead of Plumbers.
> >
> > I designed ADF to deal with some serious issues I had working with KMS:
> >
> > 1. The API is geared toward updating one object at a time. Android's graphics stack needs the entire screen updated atomically to avoid tearing, and on some SoCs to avoid wedging the display hardware. Rob Clark's atomic modeset patchset worked, but copy/update/commit design meant the driver had to keep a lot more internal state.
> >
>
> I'm not entirely sure how to avoid that, because at least some hw we
> need to have the entire new-state in order to validate if it is
> possible.
I guess the only reason adf is a bit different is that there can only be
one custom (driver specific!) blob in the ioctl, so the driver is just
free to dump that directly into whatever internal structure it uses to
store the full state. So it just frees you from the per-prop state
buildup process.
But if the idea would to be totally driver specific anyway, I wouldn't
even bother with any of this fancy framework stuff. Just specify some
massive driver specific structure with a custom ioctl and call it a
day.
> I'm open to suggestions, of course, but the approach I was
> going for was to at least do most of the boring work for this in
> drm/kms core (ie. w/ the drm_{plane,crtc,etc}_state stuff)
I'm of two minds about this. On the other hand it would be great to have
the core take care of the boring stuff as you say, but on the other hand
the driver most likely will duplicate a bunch of that stuff to some
internal structures. So there's a slight danger of getting those two out
of sync somehow.
But I guess relieving the drivers from the boring stuff wins, and we can
live with the duplication. Or maybe if we get the core stuff right, we
can avoid most duplication on the driver side.
> > 2. Some SoCs don't map well to KMS's CRTC + planes + encoders + connector model. At the time I was dealing with hardware where the blending engines didn't have their own framebuffer (they could only scan out constant colors or mix the output of other blocks), and you needed to gang several mixers together to drive high-res displays.
> >
>
> currently you can connect a crtc to multiple encoders (at least for
> some hw).. I suppose w/ the small addition of having a way to specify
> an encoder's x/y offset, we could cover this sort of case?
Hmm. Yeah simply killing the fb->crtc link (which is definitely on my
TODO list for the atomic API) isn't enough to cover this case. At first
glance the x/y offset in the encoder seems like a reasonable approach
to me.
But there is one other issue here, and that is how to represent the
connector level. Do we have multiple active connectors for such
displays, or just one? If we would go with one connector, we'd need to
link multiple encoders to the same connector, which isn't possibly
currently.
The other option would be to abstract the hardware a bit, and just
expose one crtc to userspace, but internally build it up from multiple
mixers.
> > 3. KMS doesn't support custom pixel formats, which a lot of newer SoCs use internally to cut down on bandwidth between hardware blocks.
>
> for custom formats, use a custom fourcc value, this should work.
>
> err, hmm, looks like some of the error checking that was added is a
> bit too aggressive. What I'd propose is:
>
> #define DRM_FORMAT_CUSTOM 0x80000000
>
> and then 'if (format & DRM_FORMAT_CUSTOM) ... pass through to driver ..'
Bit 31 is already taken :) It has to be one of the other three high bits.
The aggresive checking was added precisely to avoid people adding
formats at a moments whim. We kind of blew that with the exynos NV12MT
format, but then again that thing still hasn't been added to
format_check() so I guess it's not _that_ important. Maybe we could
"rewrite" history a bit and move that to live under the new
DRM_FORMAT_CUSTOM bit as well (assuming we add such a thing).
>
> > 4. KMS doesn't have a way to exchange sync fences. As a hack I managed to pass sync fences into the kernel as properties of the atomic pageflip, but there was no good way to get them back out of the kernel without a side channel.
> >
>
> I was going to recommend property to pass in. Or, we could possibly
> even just add a generic fence parameter to the pageflip/modeset ioctl,
> and just pass it through opaquely to the driver. I guess it should
> not be any harm to the upstream gpu drivers, they'd just ignore it and
> do same 'ol implicit synchronization.
>
> for returning to userspace, just put the new fence value (if there is
> just one?) in the ioctl as an out param.
Or just add an array of them if we need many. My design for the atomic ioctl
is already variable size, so adding one more special purpose list of things
isn't that far fetched, assuming there's a good reason for it. Of course
the caller has to specify the max number of things it's expecting back
since it has to reserve enough space for them.
Another "multiple return values" issue I was thinking is the error
reporting. It might be semi-useful to hand back per object errors. Then the
user might be able to just drop those objects from the list and try again.
But there are probably too many ways that things can still fail, and
it's just not worth trying to iterate the config too much as that would
take time, possibly leading to missed deadline.
>
> BR,
> -R
>
> > ADF represents display devices as collections of overlay engines and interfaces. Overlay engines (struct adf_overlay_engine) scan out images and interfaces (struct adf_interface) display those images. Overlay engines and interfaces can be connected in any n-to-n configuration that the hardware supports.
> >
> > Clients issue atomic updates to the screen by passing in a list of buffers (struct adf_buffer) consisting of dma-buf handles, sync fences, and basic metadata like format and size. If this involves composing multiple buffers, clients include a block of custom data describing the actual composition (scaling, z-order, blending, etc.) in a driver-specific format.
> >
> > Drivers provide hooks to validate these custom data blocks and commit the new configuration to hardware. ADF handles importing the dma-bufs and fences, waiting on incoming sync fences before committing, advancing the display's sync timeline, and releasing dma-bufs once they're removed from the screen.
> >
> > ADF represents pixel formats using DRM-style fourccs, and automatically sanity-checks buffer sizes when using one of the formats listed in drm_fourcc.h. Drivers can support custom fourccs if they provide hooks to validate buffers that use them.
> >
> > ADF also provides driver hooks for modesetting, managing and reporting hardware events like vsync, and changing DPMS state. These are documented in struct adf_{obj,overlay_engine,interface,device}_ops, and are similar to the equivalent DRM ops.
> >
> > Greg Hackmann (3):
> > video: add atomic display framework
> > video: adf: add display core helper
> > video: adf: add memblock helper
> >
> > Laurent Pinchart (1):
> > video: Add generic display entity core
> >
> > drivers/video/Kconfig | 2 +
> > drivers/video/Makefile | 2 +
> > drivers/video/adf/Kconfig | 15 +
> > drivers/video/adf/Makefile | 14 +
> > drivers/video/adf/adf.c | 1013 ++++++++++++++++++++++++++++++++++
> > drivers/video/adf/adf.h | 49 ++
> > drivers/video/adf/adf_client.c | 853 ++++++++++++++++++++++++++++
> > drivers/video/adf/adf_display.c | 123 +++++
> > drivers/video/adf/adf_fops.c | 982 ++++++++++++++++++++++++++++++++
> > drivers/video/adf/adf_fops.h | 37 ++
> > drivers/video/adf/adf_fops32.c | 217 ++++++++
> > drivers/video/adf/adf_fops32.h | 78 +++
> > drivers/video/adf/adf_memblock.c | 150 +++++
> > drivers/video/adf/adf_sysfs.c | 291 ++++++++++
> > drivers/video/adf/adf_sysfs.h | 33 ++
> > drivers/video/adf/adf_trace.h | 93 ++++
> > drivers/video/display/Kconfig | 4 +
> > drivers/video/display/Makefile | 1 +
> > drivers/video/display/display-core.c | 362 ++++++++++++
> > include/video/adf.h | 743 +++++++++++++++++++++++++
> > include/video/adf_client.h | 61 ++
> > include/video/adf_display.h | 31 ++
> > include/video/adf_format.h | 282 ++++++++++
> > include/video/adf_memblock.h | 20 +
> > include/video/display.h | 150 +++++
> > 25 files changed, 5606 insertions(+)
> > create mode 100644 drivers/video/adf/Kconfig
> > create mode 100644 drivers/video/adf/Makefile
> > create mode 100644 drivers/video/adf/adf.c
> > create mode 100644 drivers/video/adf/adf.h
> > create mode 100644 drivers/video/adf/adf_client.c
> > create mode 100644 drivers/video/adf/adf_display.c
> > create mode 100644 drivers/video/adf/adf_fops.c
> > create mode 100644 drivers/video/adf/adf_fops.h
> > create mode 100644 drivers/video/adf/adf_fops32.c
> > create mode 100644 drivers/video/adf/adf_fops32.h
> > create mode 100644 drivers/video/adf/adf_memblock.c
> > create mode 100644 drivers/video/adf/adf_sysfs.c
> > create mode 100644 drivers/video/adf/adf_sysfs.h
> > create mode 100644 drivers/video/adf/adf_trace.h
> > create mode 100644 drivers/video/display/Kconfig
> > create mode 100644 drivers/video/display/Makefile
> > create mode 100644 drivers/video/display/display-core.c
> > create mode 100644 include/video/adf.h
> > create mode 100644 include/video/adf_client.h
> > create mode 100644 include/video/adf_display.h
> > create mode 100644 include/video/adf_format.h
> > create mode 100644 include/video/adf_memblock.h
> > create mode 100644 include/video/display.h
> >
> > --
> > 1.8.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
