From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFC 0/4] Atomic Display Framework Date: Thu, 29 Aug 2013 10:36:30 +0300 Message-ID: <20130829073630.GD11428@intel.com> References: <1377741081-30189-1-git-send-email-ghackmann@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 2FD6DE60FA for ; Thu, 29 Aug 2013 00:36:35 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Rob Clark Cc: "linaro-mm-sig@lists.linaro.org" , "dri-devel@lists.freedesktop.org" , konkers@google.com List-Id: dri-devel@lists.freedesktop.org On Wed, Aug 28, 2013 at 11:51:59PM -0400, Rob Clark wrote: > On Wed, Aug 28, 2013 at 9:51 PM, Greg Hackmann wro= te: > > Hi, > > > > ADF is an experimental display framework that I designed after experime= nting with a KMS-based hardware composer for Android. ADF started as an pr= oof-of-concept implemented from scratch, so right now it's a separate frame= work 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 k= eep it as a separate thing. > > > > I'm going to talk about ADF at the Android and Graphics session at Linu= x Plumbers. The documentation's not done but I wanted to post these patche= s 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 atomi= c modeset patchset worked, but copy/update/commit design meant the driver h= ad 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 + connec= tor model. At the time I was dealing with hardware where the blending engi= nes didn't have their own framebuffer (they could only scan out constant co= lors or mix the output of other blocks), and you needed to gang several mix= ers 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 manage= d 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 sid= e 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 in= terfaces. Overlay engines (struct adf_overlay_engine) scan out images and = interfaces (struct adf_interface) display those images. Overlay engines an= d 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 buff= ers (struct adf_buffer) consisting of dma-buf handles, sync fences, and bas= ic metadata like format and size. If this involves composing multiple buff= ers, clients include a block of custom data describing the actual compositi= on (scaling, z-order, blending, etc.) in a driver-specific format. > > > > Drivers provide hooks to validate these custom data blocks and commit t= he new configuration to hardware. ADF handles importing the dma-bufs and f= ences, waiting on incoming sync fences before committing, advancing the dis= play'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_fou= rcc.h. Drivers can support custom fourccs if they provide hooks to validat= e 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@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- = Ville Syrj=E4l=E4 Intel OTC