Atomic Display Framework

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.

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.

3. KMS doesn't support custom pixel formats, which a lot of newer SoCs use internally to cut down on bandwidth between hardware blocks.

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.

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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Quoted 2 lines

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'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)

Quoted 2 lines

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?

Quoted 1 line

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 ..'

Quoted 8 lines · Rob Clark

Quoted 2 lines

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'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)

Yeah, I think Rob's design that trickle-feeds the entire state to the driver is ok (preferrably using a driver-allocated state tracking object or so). And then we'll let some helper code make this sane for most drivers.

I guess especially for pageflips a mode where we just pass in new buffers (and leave all the blending as-is) would be useful, maybe we could add a flag that says "keep all other properties as-is".

A second
flag for "reset properties to defaults" would also be good, where default = the primary plane can display unobstructed. So mostly just for boot splash screens, fbcon, ...

Quoted 6 lines

Quoted 2 lines

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?

Hm, I've thought that if we convert the primary plane to a real plane we'd already have that. Then you can just place planes at x/y offset to construct the entire scanout area. Planes also have a crtc mask, as do connectors (through the encoders) so the n-to-m mapping is also possible on the output side afaics.

Quoted 8 lines

Quoted 2 lines

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:

Quoted 3 lines

  #define DRM_FORMAT_CUSTOM 0x80000000

and then 'if (format & DRM_FORMAT_CUSTOM) ... pass through to driver ..'

Imo we should just convert primary planes to real planes so that we can do the same table-based checking for them as we do for secondary planes right now.

And then we can just ditch format_check

and allow
drivers to use whatever pleases them. Of course reserving a driver specific range would be good, otoh we could just add more stuff to the drm_fourcc.h header.

Cheers, Daniel

Signature · 7 lines

Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

After that we need to figure out how we can expose them to userspace w/o risking breaking stuff too much. Maybe a new ioctl to enumerate private planes only? And the maybe only accept direct use of private planes via the atomic API, not through the old setplane API. Or if people want to use setplane we could add a flag there indicating that the user is aware of private planes and wants to use them.

Quoted 3 lines

Imo we should just convert primary planes to real planes so that we can do the same table-based checking for them as we do for secondary planes right now.

My plan for i915 is to convert them to drm_planes but first keep them hidden via Rob's private plane mechanism.

After that we need to figure out how we can expose them to userspace w/o risking breaking stuff too much. Maybe a new ioctl to enumerate private planes only? And the maybe only accept direct use of private planes via the atomic API, not through the old setplane API. Or if people want to use setplane we could add a flag there indicating that the user is aware of private planes and wants to use them.

Quoted 8 lines · Rob Clark

Quoted 2 lines

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'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)

Yeah, I think Rob's design that trickle-feeds the entire state to the driver is ok (preferrably using a driver-allocated state tracking object or so). And then we'll let some helper code make this sane for most drivers.

I guess especially for pageflips a mode where we just pass in new buffers (and leave all the blending as-is) would be useful, maybe we could add a flag that says "keep all other properties as-is". A second flag for "reset properties to defaults" would also be good, where default = the primary plane can display unobstructed. So mostly just for boot splash screens, fbcon, ...

Quoted 6 lines

Quoted 2 lines

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?

Hm, I've thought that if we convert the primary plane to a real plane we'd already have that. Then you can just place planes at x/y offset to construct the entire scanout area. Planes also have a crtc mask, as do connectors (through the encoders) so the n-to-m mapping is also possible on the output side afaics.

Quoted 9 lines

Quoted 1 line

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

Quoted 1 line

and then 'if (format & DRM_FORMAT_CUSTOM) ... pass through to driver ..'

Imo we should just convert primary planes to real planes so that we can do the same table-based checking for them as we do for secondary planes right now. And then we can just ditch format_check and allow drivers to use whatever pleases them. Of course reserving a driver specific range would be good, otoh we could just add more stuff to the drm_fourcc.h header.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Quoted 6 lines · Greg Hackmann

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.

Quoted 4 lines

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'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)

Quoted 2 lines

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?

Quoted 1 line

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 ..'

Quoted 3 lines

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.

BR,
-R

Quoted 68 lines

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

Quoted 7 lines

--
1.8.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Quoted 7 lines · Rob Clark

Quoted 6 lines · Greg Hackmann

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.

Quoted 8 lines

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 6 lines

Quoted 2 lines

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.

Quoted 4 lines

Quoted 1 line

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.

Quoted 6 lines

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).

Quoted 10 lines

Quoted 3 lines

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.

Quoted 2 lines

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.

Quoted 75 lines

BR,
-R

Quoted 71 lines

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

Quoted 8 lines

Quoted 4 lines

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Ville Syrjälä
Intel OTC

I guess if you have multiple encoders + multiple connectors for the "ganging" case, then it probably just looks like 2x displays, and nothing more really needed?

I'm a bit fuzzy on what the hw looks like in this "ganging" scenario, so I'm not completely sure what the best approach is. But if we do have hw like this, then it makes sense to support it *somehow* in KMS.

Quoted 8 lines · Rob Clark

I guess if you have multiple encoders + multiple connectors for the "ganging" case, then it probably just looks like 2x displays, and nothing more really needed?

I'm a bit fuzzy on what the hw looks like in this "ganging" scenario, so I'm not completely sure what the best approach is. But if we do have hw like this, then it makes sense to support it *somehow* in KMS.

I don't have the hardware anymore so this is all working from memory, it didn't look like two independent displays. You had to explicitly set up connections between the two mixers to deal with things like scaled overlays that spanned both mixers (there was some in-hardware magic to make sure there wasn't a visible seam where the two mixers met).

Quoted 11 lines · Greg Hackmann

Quoted 8 lines · Rob Clark

I guess if you have multiple encoders + multiple connectors for the "ganging" case, then it probably just looks like 2x displays, and nothing more really needed?

I'm a bit fuzzy on what the hw looks like in this "ganging" scenario, so I'm not completely sure what the best approach is. But if we do have hw like this, then it makes sense to support it *somehow* in KMS.

Quoted 5 lines

I don't have the hardware anymore so this is all working from memory, it didn't look like two independent displays. You had to explicitly set up connections between the two mixers to deal with things like scaled overlays that spanned both mixers (there was some in-hardware magic to make sure there wasn't a visible seam where the two mixers met).

Ok, sounds like mixer == crtc (ie. the thing the combines one or more planes)? So it sounds maybe a bit like:

 plane0_0 -\
    ...    +---> CRTC -\
 plane0_n -/           |
                       +--> encoder -> connector
 plane1_0 -\           |
    ...    +---> CRTC -/

plane1_n -/

Quoted 11 lines · Greg Hackmann

Quoted 8 lines · Rob Clark

I guess if you have multiple encoders + multiple connectors for the "ganging" case, then it probably just looks like 2x displays, and nothing more really needed?

I'm a bit fuzzy on what the hw looks like in this "ganging" scenario, so I'm not completely sure what the best approach is. But if we do have hw like this, then it makes sense to support it *somehow* in KMS.

Quoted 5 lines

I don't have the hardware anymore so this is all working from memory, it didn't look like two independent displays. You had to explicitly set up connections between the two mixers to deal with things like scaled overlays that spanned both mixers (there was some in-hardware magic to make sure there wasn't a visible seam where the two mixers met).

Ok, sounds like mixer == crtc (ie. the thing the combines one or more planes)? So it sounds maybe a bit like:

 plane0_0 -\
    ...    +---> CRTC -\
 plane0_n -/           |
                       +--> encoder -> connector
 plane1_0 -\           |
    ...    +---> CRTC -/

plane1_n -/

Quoted 21 lines · Rob Clark

Quoted 11 lines · Greg Hackmann

Quoted 8 lines · Rob Clark

I guess if you have multiple encoders + multiple connectors for the "ganging" case, then it probably just looks like 2x displays, and nothing more really needed?

I'm a bit fuzzy on what the hw looks like in this "ganging" scenario, so I'm not completely sure what the best approach is. But if we do have hw like this, then it makes sense to support it *somehow* in KMS.

Quoted 5 lines

I don't have the hardware anymore so this is all working from memory, it didn't look like two independent displays. You had to explicitly set up connections between the two mixers to deal with things like scaled overlays that spanned both mixers (there was some in-hardware magic to make sure there wasn't a visible seam where the two mixers met).

Ok, sounds like mixer == crtc (ie. the thing the combines one or more planes)? So it sounds maybe a bit like:

Quoted 7 lines

 plane0_0 -\
    ...    +---> CRTC -\
 plane0_n -/           |
                       +--> encoder -> connector
 plane1_0 -\           |
    ...    +---> CRTC -/

plane1_n -/

More than one crtc to the same encoder feels funny. Can't we just keep this mixer thing internal to the kms driver by making the failure conditions a bit more obtuse to userspace?

Either way we need highly
special userspace to get this thing going, since a generic modesetting driver probably can't figure out that it needs to split up the logical framebuffer into smaller planes to be able to actually shovel all the pixels to the screen. Thus far the assumption we've backed into all dumb kms drivers is that the crtc/plane limit is also the limit for the maximum output resolution ...

Could we have a notch more details on how this is exactly wired up?

Another approach would be a set of encoders for each part of the display and some metadata (like left/right-of, ...) so that userspace understands how the aggregate display is stitched togeter.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Quoted 30 lines · Daniel Vetter

Quoted 21 lines · Rob Clark

Quoted 11 lines · Greg Hackmann

Quoted 8 lines · Rob Clark

I guess if you have multiple encoders + multiple connectors for the "ganging" case, then it probably just looks like 2x displays, and nothing more really needed?

I'm a bit fuzzy on what the hw looks like in this "ganging" scenario, so I'm not completely sure what the best approach is. But if we do have hw like this, then it makes sense to support it *somehow* in KMS.

Quoted 5 lines

I don't have the hardware anymore so this is all working from memory, it didn't look like two independent displays. You had to explicitly set up connections between the two mixers to deal with things like scaled overlays that spanned both mixers (there was some in-hardware magic to make sure there wasn't a visible seam where the two mixers met).

Ok, sounds like mixer == crtc (ie. the thing the combines one or more planes)? So it sounds maybe a bit like:

Quoted 7 lines

 plane0_0 -\
    ...    +---> CRTC -\
 plane0_n -/           |
                       +--> encoder -> connector
 plane1_0 -\           |
    ...    +---> CRTC -/

plane1_n -/

Quoted 3 lines

More than one crtc to the same encoder feels funny. Can't we just keep this mixer thing internal to the kms driver by making the failure conditions a bit more obtuse to userspace?

If there is not also a case where you'd want userspace to be able to use the two CRTC's independently, then I guess we can hide it in kernel. Otherwise, it seems that would get a bit awkward.

Quoted 7 lines

Either way we need highly
special userspace to get this thing going, since a generic modesetting driver probably can't figure out that it needs to split up the logical framebuffer into smaller planes to be able to actually shovel all the pixels to the screen. Thus far the assumption we've backed into all dumb kms drivers is that the crtc/plane limit is also the limit for the maximum output resolution ...

Yeah, that is the case today. But seems like we should be able to expose crtc/plane limits so that userspace can figure it out in a generic way.

Note that nothing actually has to split up fb's, but just setup planes to scanout a single fb at the appropriate x/y offsets.

Quoted 5 lines

Could we have a notch more details on how this is exactly wired up?

Another approach would be a set of encoders for each part of the display and some metadata (like left/right-of, ...) so that userspace understands how the aggregate display is stitched togeter.

yeah, I think understanding the hw better should help understand whether N CRTCs to one encoder, or N encoders to one connector, or ??

BR,
-R

Quoted 5 lines

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Quoted 1 line · Rob Clark

On Wed, Aug 28, 2013 at 9:51 PM, Greg Hackmann <[email protected]>

Quoted 1 line · wrote:

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.

Quoted 5 lines

Quoted 1 line

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.

Quoted 16 lines · Ville Syrjälä

Quoted 10 lines · Rob Clark

Quoted 6 lines · Greg Hackmann

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.

Quoted 3 lines

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.

Quoted 5 lines

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.

Right, the difference is that clients send the complete state they want rather than deltas against the current state. This means the driver doesn't have to track its current state and duplicate it at the beginning of the flip operation, which is a minor pain on hardware with a ton of knobs to twist across different hardware blocks.

Maybe the important question is whether incremental atomic updates is a use-case that clients need in practice. SurfaceFlinger tells the HW composer each frame "here's a complete description of what I want onscreen, make it so" and IIRC Weston works the same way.

I used a blob rather than property/value pairs because the composition is going to be inherently device specific anyway. Display controllers have such different features and constraints that you'd end up with each driver exposing a bunch of hardware-specific properties, and I'm not convinced that's any better than just letting the driver dictate how the requests are structured (modulo a handful of hardware-agnostic properties). I'm not strongly tied to blobs over properties but I think the former's easier on driver developers.

Quoted 5 lines

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 disagree -- this is basically what vendors do today to support Android, and there's a lot of common scaffolding that could go into a framework. The custom ioctl handlers all look reasonably close to this:

1) import dma-bufs and fences from their respective fds
2) map the buffers into the display device
3) validate the buffer sizes against their formats and width/stride/height
4) validate the requested layout doesn't violate hardware constraints
5) hand everything off to a worker that waits for the buffers' sync fences to fire
6) commit the requested layout to hardware
7) unmap and release all the buffers that just left the screen
8) advance the sync timeline

with some leeway on the ordering of (2)-(4) and (7)-(8). ADF handles all of this except for (4) and (6), which are inherently hardware-specific and delegated to driver ops.

Quoted 15 lines · Ville Syrjälä

Quoted 1 line · Rob Clark

On Wed, Aug 28, 2013 at 9:51 PM, Greg Hackmann <[email protected]>

Quoted 1 line · wrote:

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.

Quoted 5 lines

Quoted 1 line

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.

Quoted 6 lines

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.

Right, the difference is that clients send the complete state they want rather than deltas against the current state. This means the driver doesn't have to track its current state and duplicate it at the beginning of the flip operation, which is a minor pain on hardware with a ton of knobs to twist across different hardware blocks.

Maybe the important question is whether incremental atomic updates is a use-case that clients need in practice. SurfaceFlinger tells the HW composer each frame "here's a complete description of what I want onscreen, make it so" and IIRC Weston works the same way.

I used a blob rather than property/value pairs because the composition is going to be inherently device specific anyway. Display controllers have such different features and constraints that you'd end up with each driver exposing a bunch of hardware-specific properties, and I'm not convinced that's any better than just letting the driver dictate how the requests are structured (modulo a handful of hardware-agnostic properties). I'm not strongly tied to blobs over properties but I think the former's easier on driver developers.