Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

[Greg Hackmann]

On 03/22/2017 02:48 PM, Rob Clark wrote:

On Wed, Mar 22, 2017 at 4:10 PM, Dylan Baker  wrote:

Quoting Rob Clark (2017-03-22 10:07:54)

I guess an interesting question (from someone who doesn't know meson
yet) would be whether meson could slurp in the Makefile.sources type
stuff that we have, which are shared so far between
android/scons/autotools (and for the most part, kept developers from
having to care *too* much about the different build systems)


Jason and I have talked about that too. I'd suggested that we could write a
module for meson to read makefile.sources (since we're surely not the only
project that would benefit from such a module), except that android is moving to
blueprint[1] instead of android.mk files. As far as I can tell blueprint doesn't
support using makefile.sources, so it seems somewhat moot in a world of
blueprint for android, meson for *.


I guess even if it is only a temporary thing, something that could
slurp in Makefile.sources seems like it would be useful for a
transition period.

I'm not totally up to speed on android/blueprint stuff.. but even some
simplified or different "here-are-my-sources" type file that could be
shared across build systems would be useful.  Meson sounds a bit more
extensible so maybe there is some potential to adapt to whatever
android forces on us ;-)


+ccross from the Android build team can hopefully provide some input here.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC v2 5/8] drm/fence: add in-fences support

[Greg Hackmann]

On 04/26/2016 11:39 PM, Daniel Vetter wrote:
>> A (per-CRTC?) array of fences would be more flexible.  And even in the cases
>> where you could make a 1-to-1 mapping between planes and fences, it's not
>> that much more work for userspace to assemble those fences into an array
>> anyway.
>
> I'm ok with an array too if that's what you folks prefer (it's meant to be
> used by you after all). I just don't want just 1 fence for the entire op,
> forcing userspace to first merge them all together. That seems silly.
>
> One side-effect of that is that we'd also have to rework all the internal
> bits and move fences around in atomic. Which means change a pile of
> drivers. Not sure that's worth it, but I'd be ok either way really.
> -Daniel
>

It's not a strong preference on my end.  The 1:1 plane-to-layer mapping 
breaks down somewhat on hardware where you need to split large 
hwcomposer layers across multiple DRM planes.

That said, you can force that case to fit by just dup()ing the fence a 
bunch of times or arbitrarily picking one of the planes to assign the 
fence to.  Either is kludgey, but I can't argue it's kludgey enough to 
justify refactoring a bunch of existing driver code.

[RFC v2 5/8] drm/fence: add in-fences support

[Greg Hackmann]

On 04/26/2016 01:05 PM, Daniel Vetter wrote:
> On Tue, Apr 26, 2016 at 09:55:06PM +0300, Ville Syrjälä wrote:
>> On Tue, Apr 26, 2016 at 08:23:46PM +0200, Daniel Vetter wrote:
>>> On Tue, Apr 26, 2016 at 08:40:45PM +0300, Ville Syrjälä wrote:
>>> But really the reason for per-plane is hw composer from
>>> Android. I don't see any point in designing an api that's needlessly
>>> different from what the main user expects (even if it may be silly).
>>
>> What are they doing that can't stuff the fences into an array
>> instead of props?
>
> The hw composer interface is one in-fence per plane. That's really the
> major reason why the kernel interface is built to match. And I really
> don't think we should diverge just because we have a slight different
> color preference ;-)
>
> As long as you end up with a pile of fences somehow it'll work.
> -Daniel
>

The relationship between layers and fences is only fuzzy and indirect 
though.  The relationship is really between the buffer you're displaying 
on that layer, and the fence representing the work done to render into 
that buffer.  SurfaceFlinger just happens to bundle them together inside 
the same struct hwc_layer_1 as an API convenience.

Which is kind of splitting hairs as long as you have a 1-to-1 
relationship between layers and DRM planes.  But that's not always the case.

A (per-CRTC?) array of fences would be more flexible.  And even in the 
cases where you could make a 1-to-1 mapping between planes and fences, 
it's not that much more work for userspace to assemble those fences into 
an array anyway.

[PATCH] staging/android: change IOCTLs opcode after ABI change

[Greg Hackmann]

On 03/03/2016 02:42 PM, Gustavo Padovan wrote:
> From: Gustavo Padovan 
>
> Burn the old opcode to avoid any potential old userspace running the old
> API to get weird errors. Changing the opcodes will make them fail right
> away.
>
> This is just a precaution, there no upstream users of these interfaces
> yet and the only user is Android, but we don't expect anyone trying to
> run android userspace and all it dependencies on top of upstream kernels.
>
> Moreover Android should be converted to use upstream sync_files.
>
> Suggested-by: Rob Clark 
> Signed-off-by: Gustavo Padovan 

Acked-by: Greg Hackmann 

[PATCH v7 1/5] staging/android: add num_fences field to struct sync_file_info

[Greg Hackmann]

On 03/03/2016 11:40 AM, Gustavo Padovan wrote:
> From: Gustavo Padovan 
>
> Inform userspace how many fences are in the sync_fence_info field.
>
> Signed-off-by: Gustavo Padovan 
> Reviewed-by: Maarten Lankhorst 
> ---
>   drivers/staging/android/sync.c  | 2 ++
>   drivers/staging/android/uapi/sync.h | 2 ++
>   2 files changed, 4 insertions(+)
>
> diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
> index 3a8f210..31aa462 100644
> --- a/drivers/staging/android/sync.c
> +++ b/drivers/staging/android/sync.c
> @@ -525,6 +525,8 @@ static long sync_file_ioctl_fence_info(struct sync_file 
> *sync_file,
>   if (info->status >= 0)
>   info->status = !info->status;
>
> + info->num_fences = sync_file->num_fences;
> +
>   len = sizeof(struct sync_file_info);
>
>   for (i = 0; i < sync_file->num_fences; ++i) {
> diff --git a/drivers/staging/android/uapi/sync.h 
> b/drivers/staging/android/uapi/sync.h
> index a0cf357..4ffb7cc 100644
> --- a/drivers/staging/android/uapi/sync.h
> +++ b/drivers/staging/android/uapi/sync.h
> @@ -47,12 +47,14 @@ struct sync_fence_info {
>*  userspace including pt_info.
>* @name:   name of fence
>* @status: status of fence. 1: signaled 0:active <0:error
> + * @num_fences   number of fences in the sync_file
>* @sync_fence_info: array of sync_fence_info for every fence in the 
> sync_file
>*/
>   struct sync_file_info {
>   __u32   len;
>   charname[32];
>   __s32   status;
> + __u32   num_fences;
>
>   __u8sync_fence_info[0];
>   };
>

Greg, this is ultimately your call.  But to echo what I said before, I'm 
OK with the ABI break here if that's what's needed to get sync moved out 
of staging.

Pragmatically speaking, this won't break ordinary Android apps, because 
they don't get direct access to sync fence fds to begin with.  Some 
privileged system processes do, but they go through libsync, which can 
be changed to deal with the ABI differences.

Anyway, for the whole series:

Acked-by: Greg Hackmann 

[PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file

[Greg Hackmann]

On 01/28/16 01:23, Daniel Vetter wrote:
> And I think driver_data really shouldn't be there, it makes things
> complicated with the array of variable-sized objects, and generic
> userspace can't really use it - for debug output we already have
> obj/driver_name per fence point, which I think is good enough.

I looked at our device kernels, and some vendors actually are filling in 
driver_data.  I'm just not seeing any accesses to them in our 
*userspace* tree.  And in a lot of cases it looks like they're just 
filling in debugging information that they could get elsewhere.

I'm checking with our vendor contacts to see what they're actually using 
this for (if anything).

> Would that be ok for you from the Android side if Gustavo also provides a
> patch to update libsync? I don't think the ABI is fundamentally broken,
> but this light cleanup would be nice.

No objections here.  Just upload the changes to AOSP and add me as a 
reviewer.

> Wrt keeping SYNC_WAIT: I think that's totally fine. Redundant since
> polling is supported, but not really an issue imo either. If we're totally
> lazy we could implement SYNC_WAIT internally using poll and shave off a
> few lines of the implementation.

Honestly this is the change I'm least worried about, since poll() will 
work with existing kernels too.  The only difference would be that the 
SYNC_WAIT ioctl fails when given something that's not specifically a 
sync fence; but I'm skeptical that anything actually depends on that 
behavior.

[PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file

[Greg Hackmann]

On 01/27/2016 12:25 PM, Gustavo Padovan wrote:
 Is there a value in keeping the abi unchanged?
 If not, then Documentation/ioctl/botching-up-ioctls.txt is worth a read.
>>>
>>> None from me. I'll look where we can improve the ABI.

Android has existing clients of the current ABI.  Thankfully they're all 
contained in system services like SurfaceFlinger, since end-user apps 
don't get direct access to fence fds.

As long the ABI breaks don't remove functionality we depend on, we can 
wrap around them in our userspace libsync.  I'd rather not have to do 
that, but it's a price I'm willing to pay to get this moved out of staging.

>>   - struct sync_file_info_data::fence_info is of type __u8 yet it is "a
>> fence_info struct for every fence in the sync_file". Thus shouldn't
>> one use "struct fence_info" as the type ?
>
> Agreed. But I'm currently thinking if we really should keep this ioctl.
>
>   Gustavo
>

I'm not seeing any consumers of driver_data in our tree.  OTOH 
completely getting rid of the ioctl would be a problem, since 
SurfaceFlinger depends on the timestamp information for its own bookkeeping.

[RFC 11/29] dma-buf/fence: move sync_timeline to fence_timeline

[Greg Hackmann]

On 01/15/2016 06:55 AM, Gustavo Padovan wrote:
>   /**
> + * fence_timeline_create - create a new fence_timeline
> + * @num: [in]amount of contexts to allocate
[...]
> + */
> +struct fence_timeline *fence_timeline_create(unsigned num,
> +  struct fence_timeline_ops *ops,
> +  int size, const char *name)
> +{
[...]
> + timeline->context = fence_context_alloc(1);

fence_context_alloc(num)

[RFC 26/29] dma-buf/fence: remove pointless fence_timeline_signal at destroy phase

[Greg Hackmann]

On 01/15/2016 10:02 AM, Gustavo Padovan wrote:
> Patches 27 and 28 are attempt to fix that. I assumed that if some code is
> calling fence_timeline_destroy() it wants to stop everything so I
> worked on a solution that stops any waiter and allows the timeline to be
> destroyed.
>
> No one is using fence_timeline_destroy() in mainline now, so it is
> definately a behaviour we can discuss.
>
>   Gustavo
>

+Tom Cherry and Dmitry Torokhov recently discovered that this was broken 
by the refactoring of Android sync on top of dma-buf fences.

Tom and Dmitry, did you send the proposed fix upstream?

DRM support in Android

[Greg Hackmann]

On Tue, Jan 5, 2016 at 12:06 AM, vijay kumar  wrote:

> Hii all,
>   I want to know some information regarding DRI and DRM
> support in android.Does Android uses DRI drivers in MESA for Direct
> rendering.
>

The vendor needs to supply an EGL implementation at a well-known path (
https://android.googlesource.com/platform/frameworks/native/+/android-6.0.1_r5/opengl/libs/EGL/Loader.cpp#41).
This can be Mesa or a proprietary library.
-- next part --
An HTML attachment was scrubbed...
URL: 

dumb BOs and prime

[Greg Hackmann]

On 12/04/2015 11:23 AM, Rob Herring wrote:
> On Fri, Dec 4, 2015 at 12:40 PM, Benjamin Gaignard
>  wrote:
>> Hi Rob,
>>
>> I got the same problem today with sti drm/kms driver and dumb Bo.
>> The issue comes become hwcomposer because is the master and authenticated on
>> /dev/dri/cardX
>> Dumb allocation is done by gralloc which does a new open (so it is not
>> authenticated) on drm node the consequence is that we can't use prime
>> functions...
>> If you use render node you won't be able to call dumb functions.
>>
>> To get out of this I think I will implement additional helpers in gem_cma to
>> have ioctl like DRM_GEM_CMA_CREATE and DRM_GEM_CMA_MMAP
>> and call them instead of dumb so be able to use render node.
>> Of course it is only for drivers which already use gem_cma helpers (like
>> sti)
>
> That's only marginally better than per driver BO calls which is the
> whole thing I'm trying to avoid. There should be some way to pass the
> auth token to gralloc?
>
> Rob

Frankly, you probably want to approach this by allocating dma-bufs using 
something external to DRM (probably ion) and then have your hwcomposer 
import them into DRM when they're passed to the display.

Backing gralloc allocations with dumb BOs doesn't really fit the way 
gralloc is designed: like dma-buf, it's built around passing buffers 
between different hardware blocks, and some of those buffers may never 
even touch the display hardware.  You'll also run up against other 
(deliberate) limitations of dumb BOs like not being able to allocate YUV 
buffers.

Unfortunately this currently means adding some shim driver code to 
create the ion device.  (Device-Tree bindings for ion are on my long, 
long backlog of things to do.) It's not a lot of code, especially if all 
you need is a CMA heap, but it's still non-zero.

Note that supporting dumb BOs in your KMS driver is still useful, since 
the recovery console in AOSP has KMS support now.  In that case it's a 
single privileged process software-rendering everything, so it bypasses 
gralloc/hwcomposer and calls directly into libdrm.

[PATCH] staging/android: add TODO to de-stage android sync framework

[Greg Hackmann]

On 11/24/2015 12:53 AM, Daniel Vetter wrote:
> With all the effort going on around kselftest it'd be good to integrate
> the existing testsuite google has into upstream too. Should probably be
> listed here too.
> -Daniel

The test code's available in AOSP:

https://android.googlesource.com/platform/system/core/+/master/libsync/tests/

Be warned that it sits on top of a small helper library, uses C++ 
heavily, and depends on googletest.  So it's going to need reworking 
before it's suitable for the kernel tree.  But you can at least see the 
kinds of things it's testing (and where the SW_SYNC_USER parts fit into 
the picture).

[PATCH] staging/android: add TODO to de-stage android sync framework

[Greg Hackmann]

On 11/23/15 11:41 AM, Gustavo Padovan wrote:
> + - remove sw_sync, it is used only for testing/debugging and should not be
> +upstreamed.
> + - port sw_sync testcases to use debugfs somehow

A quick but important nitpick:

sw_sync itself is just an in-kernel helper for creating fences, when you 
don't have something like sync timeline primitives baked into your hardware.

CONFIG_SW_SYNC_USER adds the interface for creating and signaling 
sw_sync objects from userspace.  This is the part that's dangerous and 
only intended for testing, etc.

AFAIK CONFIG_SW_SYNC_USER is the only part people have been objecting 
to.  I'm fine with removing it.  Removing the kernel-facing side of 
sw_sync would be a problem for us, since many drivers use it to create 
their fences.

atomic explicit fencing vs android

[Greg Hackmann]

On Fri, Nov 13, 2015 at 1:47 PM, Rob Clark  wrote:
>
> I suppose the philosophy here is
> that on android, surfaceflinger (userspace) is the trusted one (which
> may well be correct on some vendor kernel branches), whereas upstream
> the kernel is the trusted one.
>

To clarify, this wasn't the philosophy behind the Android fence semantics.
It was more that someone internally had to make a judgement call about the
semantics to go with, and that decision's been baked into the
SurfaceFlinger codebase since then.

After LPC I spoke with our graphics team about your and Daniel Vetter's
concerns.  They have plans to change SurfaceFlinger so hwcomposer HALs can
optionally use DRM's semantics for retire fences.

One possible option, perhaps... as I understand it, w/ android it is
> possible to create and signal fences from userspace.  So userspace
> could create and maintain it's own queue of fences, which it returns
> from hwc->set(), and are signalled (timestamp copied, etc) when the
> kernel fences from the next atomic ioctl are signalled.
>

CONFIG_SW_SYNC_USER exposes a userspace API for creating and signaling
fences from userspace.  But it's only intended for testing, and we *really*
don't want vendors shipping it in production code -- once userspace can
create the fences it sends to the kernel, you have a whole new set of
deadlock scenarios to worry about.  For example if userspace creates a
fence, sends it to the kernel, and crashes before signalling it, you're
stuck with a deadlocked display pipeline waiting on a fence that can never
fire.
-- next part --
An HTML attachment was scrubbed...
URL: 

[PATCH libdrm] Add missing includes

[Greg Hackmann]

On Tue, Apr 21, 2015 at 11:31 AM, Emil Velikov  
wrote:
>
> I'm not sure why I haven't hit this while building with Android-x86's
> bionic, although it's the right thing to do.

Maybe a difference in bionic versions?  I know the bionic team made
some recent (post-5.1) changes that unintentionally exposed a handful
of other missing #includes in our tree.

> From a quick look I cannot see libdrm in the AOSP build. Are you
> experimenting with a libdrm based hwcompositor again :-)

Yeah.  That's where I want to end up in the long term, anyway.  I've
said publicly that we'd eventually like to converge with upstream on
this, and I still mean it.  :)

[PATCH libdrm] Add missing includes

[Greg Hackmann]

A couple of files use ffs() without explicitly including strings.h.
Some systems will pull in ffs()'s declaration through another header
anyway, but not when compiling against bionic in AOSP master.

Signed-off-by: Greg Hackmann 
---
 nouveau/nouveau.c | 1 +
 tests/modetest/modetest.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c
index 2d95b74..22e0f80 100644
--- a/nouveau/nouveau.c
+++ b/nouveau/nouveau.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 53dfe05..63d2059 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 00/16] Atomic/nuclear modeset/pageflip

[Greg Hackmann]

On Wed, Mar 26, 2014 at 3:08 AM, Daniel Vetter  wrote:
>
> - You have an explicit callback for vblank events (well, just the
>   interrupt, afaics no support to get a vblank event for a specific frame
>   in the future). Any reason not to use android syncpoint fences for this
>   like everywhere else? Or is the idea behind all the fences hwc throws
>   around just to synchronize read/write access from
>   cpu/render/codec/whatever and not so much timing?
>

Vsync events are used to kick off Android's rendering loop.  The upshot is
that when SurfaceFlinger requests vsync events from the HWC HAL, it needs
to deliver them at every vsync even if the display hasn't changed.

Fences tell a consumer when the producer's done with the buffer, and aren't
really meant for timing.  (Or probably even useful for timing, since they
don't pass along any timestamping information about when the event
happened.)

- If I read this correct you mandate that the fences fields gets filled
>   out in the prepare hook, i.e. before we commit the new state to
>   hardware. You also mandate that the fences get eventually signalled even
>   when surfaceflinger decides to not commit the new state.
>

The HWC HAL needs to fill in the release fences during set().  The mandate
w.r.t. fences is that if SurfaceFlinger calls

prepare(A)
set(A)
prepare(B)
set(B)

and something inside set(B) fails for some reason, the HWC HAL must ensure
that the fences returned by set(A) eventually fire anyway.  That situation
really shouldn't happen unless something goes horribly wrong, since the
expectation is that prepare(B) will pick a composition that set(B) can
display.
-- next part --
An HTML attachment was scrubbed...
URL: 

[PATCH 00/16] Atomic/nuclear modeset/pageflip

[Greg Hackmann]

On Thu, Mar 20, 2014 at 5:28 PM, Rob Clark  wrote:

> oh, that's right.. I'm glad you reminded me, I do remember now talk of
> a field which drivers could use to return some opaque (to drm core)
> handle/value/token/whatever..
>
> hmm, for the non-vsync'd multi-display case, does it matter much one
> ioctl per display vs one globally?
>
> If no, I'd vote for just one output field in the ioctl struct which
> can be whatever the driver wants it to.. that would be pretty simple.


If you're planning to support generic clients, you'd at least need some way
to indicate the output contains an fd that the client's responsible for
closing.  Otherwise it would leak a fence (or whatever fd-backed object
that's being returned) each time it updates the display.


> > It's up to the vendor whether their HWC HAL does fine-grained error
> handling
> > and renegotiation like you're describing.  Typically they don't, they
> just
> > push that complexity into userspace instead.  e.g. if the display
> controller
> > needs to gang together two planes for a given configuration, the HWC
> HAL's
> > prepare() op will already know to set aside that extra plane and plan
> > accordingly.
>
> Well, I suspect that this is in the HWC HAL at all is a strong
> indication that we should think about including this somehow in the
> atomic ioctl.. considering that the trend for wayland compositors is a
> fairly generic userspace just using kms+gbm directly.  I suspect the
> alternative will only be an atomic2 ioctl at some point ;-)
>

These policies are complicated enough that I'm skeptical you could push
them into the kernel without making the drivers really large and
complicated.  A typical HWC HAL will have thousands of lines of code plus
pull in other helper libraries, and the bulk of that will be for planning
the composition in the prepare() op.  Maybe you could break some of that
out into common helper code, but IME and IMO the hardware constraints are
too different from SoC to SoC for that to scale.

That said, I'd be happy to be proven wrong -- if the hypothetical atomic2
ioctl has a kernel counterpart that I'm comfortable recommending to
vendors, being able to write a single generic HWC HAL would be good for
Android too.
-- next part --
An HTML attachment was scrubbed...
URL: 

[PATCH 00/16] Atomic/nuclear modeset/pageflip

[Greg Hackmann]

On Wed, Mar 19, 2014 at 5:23 AM, Rob Clark  wrote:

> > Hm, do you have some pointers to read up on this? I still think a more
> > elaborate fail scheme is overkill, but maybe reading a bit of android
> code
> > convinces me differently ;-)
>
> sadly no pointers to anything to read (but ofc would be interested if
> anyone else does)..
>

The hardware composer HAL is probably the best reference for what Android
needs from display.  It's defined and documented here (see struct
hwc_composer_device_1):

*https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/hwcomposer.h
*

(You can disregard any HWC 1.0 specific behavior in that documentation,
that's only around for legacy reasons and we strongly encourage vendors not
to implement HWC 1.0 anymore.)

Vendors need to ship a compliant HWC HAL implementation on their devices.
 We don't mandate anything about their userspace/kernel interface beyond
that.


>  * I kinda would still like to add to the atomic ioctl some way to indicate
>ok/error on a bit finer granularity than per-ioctl.  Ie. perhaps an
>array where userspace can ask for results per KMS object (crtc/plane)?
>Something along these lines would give the kernel a bit more flexibility
>to deal with some of the edge cases which come up when you gang crtcs
>for ultra high resolution displays.  In short, it would let the driver
>"steal" some of the planes if needed from userspace.
>
>This would also, conveniently, be pretty similar to how (AFAIU) hw
>compositor and ADF work on android.  Which seems like it would be useful
>to eventually enable android devices to use an upstream display driver.
>
>For this we could pretty easily just throw in a placeholder that we
>could replace later with an optional ptr to __user array.  I think
>that would be fine for an initial version, but I just wanted to throw
>this idea out there, because I think it will become important.
>

SurfaceFlinger needs the HWC HAL to return a retire fence fd on each flip,
for each display.  Per-interface return data would be a good fit for this,
but per-CRTC or per-plane would work too (userspace could assemble a
per-interface fence by merging finer-grained fences).

It's up to the vendor whether their HWC HAL does fine-grained error
handling and renegotiation like you're describing.  Typically they don't,
they just push that complexity into userspace instead.  e.g. if the display
controller needs to gang together two planes for a given configuration, the
HWC HAL's prepare() op will already know to set aside that extra plane and
plan accordingly.
-- next part --
An HTML attachment was scrubbed...
URL: 

[RFC 0/4] Atomic Display Framework

[Greg Hackmann]

On Thu, Aug 29, 2013 at 5:54 AM, Rob Clark  wrote:

> 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).
-- next part --
An HTML attachment was scrubbed...
URL: 

[RFC 0/4] Atomic Display Framework

[Greg Hackmann]

On Thu, Aug 29, 2013 at 12:36 AM, Ville Syrj?l? <
ville.syrjala at linux.intel.com> wrote:

> On Wed, Aug 28, 2013 at 11:51:59PM -0400, Rob Clark wrote:
> > On Wed, Aug 28, 2013 at 9:51 PM, Greg Hackmann 
> 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.
> > >
> >
> > 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.
>

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.


> 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.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20130829/40a64085/attachment.html>

Re: [RFC 0/4] Atomic Display Framework

[Greg Hackmann]

On Thu, Aug 29, 2013 at 12:36 AM, Ville Syrjälä 
ville.syrj...@linux.intel.com wrote:

 On Wed, Aug 28, 2013 at 11:51:59PM -0400, Rob Clark wrote:
  On Wed, Aug 28, 2013 at 9:51 PM, Greg Hackmann ghackm...@google.com
 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.
  
 
  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.


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.


 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.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/4] Atomic Display Framework

[Greg Hackmann]

On Thu, Aug 29, 2013 at 5:54 AM, Rob Clark robdcl...@gmail.com wrote:

 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).
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 4/4] video: adf: add memblock helper

[Greg Hackmann]

Provides a dma-buf exporter for memblocks, mainly useful for ADF devices
to wrap their bootloader logos

Signed-off-by: Greg Hackmann 
---
 drivers/video/adf/Kconfig|   5 ++
 drivers/video/adf/Makefile   |   2 +
 drivers/video/adf/adf_memblock.c | 150 +++
 include/video/adf_memblock.h |  20 ++
 4 files changed, 177 insertions(+)
 create mode 100644 drivers/video/adf/adf_memblock.c
 create mode 100644 include/video/adf_memblock.h

diff --git a/drivers/video/adf/Kconfig b/drivers/video/adf/Kconfig
index 30b0611..ad0c0eb 100644
--- a/drivers/video/adf/Kconfig
+++ b/drivers/video/adf/Kconfig
@@ -8,3 +8,8 @@ menuconfig ADF_DISPLAY_CORE
depends on ADF
depends on DISPLAY_CORE
tristate "Helper for implementing ADF interface ops with Display Core 
devices"
+
+menuconfig ADF_MEMBLOCK
+   depends on ADF
+   depends on HAVE_MEMBLOCK
+   tristate "Helper for using memblocks as buffers in ADF drivers"
diff --git a/drivers/video/adf/Makefile b/drivers/video/adf/Makefile
index 30164ee..97f9c98 100644
--- a/drivers/video/adf/Makefile
+++ b/drivers/video/adf/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_ADF) += adf.o \
 obj-$(CONFIG_COMPAT) += adf_fops32.o

 obj-$(CONFIG_ADF_DISPLAY_CORE) += adf_display.o
+
+obj-$(CONFIG_ADF_MEMBLOCK) += adf_memblock.o
diff --git a/drivers/video/adf/adf_memblock.c b/drivers/video/adf/adf_memblock.c
new file mode 100644
index 000..a1b7ec6
--- /dev/null
+++ b/drivers/video/adf/adf_memblock.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright (C) 2013 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+struct adf_memblock_pdata {
+   phys_addr_t base;
+};
+
+struct sg_table *adf_memblock_map(struct dma_buf_attachment *attach,
+   enum dma_data_direction direction)
+{
+   struct adf_memblock_pdata *pdata = attach->dmabuf->priv;
+   unsigned long pfn = (pdata->base >> PAGE_SHIFT);
+   struct page *page = pfn_to_page(pfn);
+   struct sg_table *table;
+   int ret;
+
+   table = kzalloc(sizeof(*table), GFP_KERNEL);
+   if (!table)
+   return ERR_PTR(-ENOMEM);
+
+   ret = sg_alloc_table(table, 1, GFP_KERNEL);
+   if (ret < 0)
+   goto err;
+
+   sg_set_page(table->sgl, page, attach->dmabuf->size, 0);
+   return table;
+
+err:
+   kfree(table);
+   return ERR_PTR(ret);
+}
+
+void adf_memblock_unmap(struct dma_buf_attachment *attach,
+   struct sg_table *table, enum dma_data_direction direction)
+{
+   sg_free_table(table);
+}
+
+static void __init_memblock adf_memblock_release(struct dma_buf *buf)
+{
+   struct adf_memblock_pdata *pdata = buf->priv;
+   int err = memblock_free(pdata->base, buf->size);
+
+   if (err < 0)
+   pr_warn("%s: freeing memblock failed: %d\n", __func__, err);
+   kfree(pdata);
+}
+
+static void *adf_memblock_do_kmap(struct dma_buf *buf, unsigned long pgoffset,
+   bool atomic)
+{
+   struct adf_memblock_pdata *pdata = buf->priv;
+   unsigned long pfn = (pdata->base >> PAGE_SHIFT) + pgoffset;
+   struct page *page = pfn_to_page(pfn);
+
+   if (atomic)
+   return kmap_atomic(page);
+   else
+   return kmap(page);
+}
+
+static void *adf_memblock_kmap_atomic(struct dma_buf *buf,
+   unsigned long pgoffset)
+{
+   return adf_memblock_do_kmap(buf, pgoffset, true);
+}
+
+static void adf_memblock_kunmap_atomic(struct dma_buf *buf,
+   unsigned long pgoffset, void *vaddr)
+{
+   kunmap_atomic(vaddr);
+}
+
+static void *adf_memblock_kmap(struct dma_buf *buf, unsigned long pgoffset)
+{
+   return adf_memblock_do_kmap(buf, pgoffset, false);
+}
+
+static void adf_memblock_kunmap(struct dma_buf *buf, unsigned long pgoffset,
+   void *vaddr)
+{
+   kunmap(vaddr);
+}
+
+static int adf_memblock_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
+{
+   struct adf_memblock_pdata *pdata = buf->priv;
+   unsigned long pfn = pdata->base >> PAGE_SHIFT;
+
+   return remap_pfn_range(vma, vma->vm_start, pfn,
+   vma->vm_end - vma->vm_start, vma->vm_page_prot);
+}
+
+struct dma_buf_ops adf_memblock_ops = {
+   .map_dma_buf = adf_memblock_map,
+   .unmap_dma_buf = adf_memblock_unmap,
+   .release = adf_memblock_release,
+   .kmap_atomic = adf_mem

[RFC 3/4] video: adf: add display core helper

[Greg Hackmann]

Optional helper which implements some ADF interface ops for displays
using the Display Core framework

Signed-off-by: Greg Hackmann 
---
 drivers/video/adf/Kconfig   |   5 ++
 drivers/video/adf/Makefile  |   2 +
 drivers/video/adf/adf.c |  28 -
 drivers/video/adf/adf.h |   1 +
 drivers/video/adf/adf_display.c | 123 
 include/video/adf_display.h |  31 ++
 6 files changed, 189 insertions(+), 1 deletion(-)
 create mode 100644 drivers/video/adf/adf_display.c
 create mode 100644 include/video/adf_display.h

diff --git a/drivers/video/adf/Kconfig b/drivers/video/adf/Kconfig
index 0b64408..30b0611 100644
--- a/drivers/video/adf/Kconfig
+++ b/drivers/video/adf/Kconfig
@@ -3,3 +3,8 @@ menuconfig ADF
depends on SW_SYNC
depends on DMA_SHARED_BUFFER
tristate "Atomic Display Framework"
+
+menuconfig ADF_DISPLAY_CORE
+   depends on ADF
+   depends on DISPLAY_CORE
+   tristate "Helper for implementing ADF interface ops with Display Core 
devices"
diff --git a/drivers/video/adf/Makefile b/drivers/video/adf/Makefile
index 2af5f79..30164ee 100644
--- a/drivers/video/adf/Makefile
+++ b/drivers/video/adf/Makefile
@@ -8,3 +8,5 @@ obj-$(CONFIG_ADF) += adf.o \
adf_sysfs.o

 obj-$(CONFIG_COMPAT) += adf_fops32.o
+
+obj-$(CONFIG_ADF_DISPLAY_CORE) += adf_display.o
diff --git a/drivers/video/adf/adf.c b/drivers/video/adf/adf.c
index 5dc04af..b3b57dd 100644
--- a/drivers/video/adf/adf.c
+++ b/drivers/video/adf/adf.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2013 Google, Inc.
- * adf_modeinfo_set_name modified from drm_mode_set_name in
+ * adf_modeinfo_{set_name,set_vrefresh} modified from
  * drivers/gpu/drm/drm_modes.c
  *
  * This software is licensed under the terms of the GNU General Public
@@ -966,6 +966,32 @@ void adf_modeinfo_set_name(struct drm_mode_modeinfo *mode)
 interlaced ? "i" : "");
 }

+void adf_modeinfo_set_vrefresh(struct drm_mode_modeinfo *mode)
+{
+   int refresh = 0;
+   unsigned int calc_val;
+
+   if (mode->vrefresh > 0)
+   return;
+   else if (mode->htotal > 0 && mode->vtotal > 0) {
+   int vtotal;
+   vtotal = mode->vtotal;
+   /* work out vrefresh the value will be x1000 */
+   calc_val = (mode->clock * 1000);
+   calc_val /= mode->htotal;
+   refresh = (calc_val + vtotal / 2) / vtotal;
+
+   if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+   refresh *= 2;
+   if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
+   refresh /= 2;
+   if (mode->vscan > 1)
+   refresh /= mode->vscan;
+
+   mode->vrefresh = refresh;
+   }
+}
+
 static void __exit adf_exit(void);
 static int __init adf_init(void)
 {
diff --git a/drivers/video/adf/adf.h b/drivers/video/adf/adf.h
index acad631..5f7260d 100644
--- a/drivers/video/adf/adf.h
+++ b/drivers/video/adf/adf.h
@@ -44,5 +44,6 @@ struct adf_event_refcount *adf_obj_find_refcount(struct 
adf_obj *obj,
enum adf_event_type type);

 void adf_modeinfo_set_name(struct drm_mode_modeinfo *mode);
+void adf_modeinfo_set_vrefresh(struct drm_mode_modeinfo *mode);

 #endif /* __ADF_H */
diff --git a/drivers/video/adf/adf_display.c b/drivers/video/adf/adf_display.c
new file mode 100644
index 000..c87f6a5
--- /dev/null
+++ b/drivers/video/adf/adf_display.c
@@ -0,0 +1,123 @@
+/*
+ * Copyright (C) 2013 Google, Inc.
+ * adf_modeinfo_from_videomode modified from drm_display_mode_from_videomode in
+ * drivers/gpu/drm/drm_modes.c
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+
+#include "adf.h"
+
+/**
+ * adf_display_entity_screen_size - handle the screen_size interface op
+ * by querying a display core entity
+ */
+int adf_display_entity_screen_size(struct display_entity *display,
+   u16 *width_mm, u16 *height_mm)
+{
+   unsigned int cdf_width, cdf_height;
+   int ret;
+
+   ret = display_entity_get_size(display, _width, _height);
+   if (!ret) {
+   *width_mm = cdf_width;
+   *height_mm = cdf_height;
+   }
+   return ret;
+}
+EXPORT_SYMBOL(adf_display_entity_screen_size);
+
+/**
+ * adf_display_entity_notify_connected - notify ADF of a display core entity
+ * being connected to an interface
+ *
+ * @intf: the inter

[RFC 2/4] video: add atomic display framework

[Greg Hackmann]

Signed-off-by: Greg Hackmann 
---
 drivers/video/Kconfig  |   1 +
 drivers/video/Makefile |   1 +
 drivers/video/adf/Kconfig  |   5 +
 drivers/video/adf/Makefile |  10 +
 drivers/video/adf/adf.c| 987 +
 drivers/video/adf/adf.h|  48 ++
 drivers/video/adf/adf_client.c | 853 +++
 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_sysfs.c  | 291 
 drivers/video/adf/adf_sysfs.h  |  33 ++
 drivers/video/adf/adf_trace.h  |  93 
 include/video/adf.h| 743 +++
 include/video/adf_client.h |  61 +++
 include/video/adf_format.h | 282 
 17 files changed, 4722 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_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_sysfs.c
 create mode 100644 drivers/video/adf/adf_sysfs.h
 create mode 100644 drivers/video/adf/adf_trace.h
 create mode 100644 include/video/adf.h
 create mode 100644 include/video/adf_client.h
 create mode 100644 include/video/adf_format.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 6d9788d..a77df10 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2476,6 +2476,7 @@ source "drivers/video/exynos/Kconfig"
 source "drivers/video/mmp/Kconfig"
 source "drivers/video/backlight/Kconfig"
 source "drivers/video/display/Kconfig"
+source "drivers/video/adf/Kconfig"

 if VT
source "drivers/video/console/Kconfig"
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index d7fd4a2..aa6a247 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -12,6 +12,7 @@ fb-y  := fbmem.o fbmon.o fbcmap.o 
fbsysfs.o \
  modedb.o fbcvt.o
 fb-objs   := $(fb-y)

+obj-$(CONFIG_ADF)+= adf/
 obj-$(CONFIG_VT) += console/
 obj-$(CONFIG_LOGO)   += logo/
 obj-y+= backlight/
diff --git a/drivers/video/adf/Kconfig b/drivers/video/adf/Kconfig
new file mode 100644
index 000..0b64408
--- /dev/null
+++ b/drivers/video/adf/Kconfig
@@ -0,0 +1,5 @@
+menuconfig ADF
+   depends on SYNC
+   depends on SW_SYNC
+   depends on DMA_SHARED_BUFFER
+   tristate "Atomic Display Framework"
diff --git a/drivers/video/adf/Makefile b/drivers/video/adf/Makefile
new file mode 100644
index 000..2af5f79
--- /dev/null
+++ b/drivers/video/adf/Makefile
@@ -0,0 +1,10 @@
+ccflags-y := -Idrivers/staging/android -Idrivers/staging/android/sync
+
+CFLAGS_adf.o := -I$(src)
+
+obj-$(CONFIG_ADF) += adf.o \
+   adf_client.o \
+   adf_fops.o \
+   adf_sysfs.o
+
+obj-$(CONFIG_COMPAT) += adf_fops32.o
diff --git a/drivers/video/adf/adf.c b/drivers/video/adf/adf.c
new file mode 100644
index 000..5dc04af
--- /dev/null
+++ b/drivers/video/adf/adf.c
@@ -0,0 +1,987 @@
+/*
+ * Copyright (C) 2013 Google, Inc.
+ * adf_modeinfo_set_name modified from drm_mode_set_name in
+ * drivers/gpu/drm/drm_modes.c
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "sw_sync.h"
+#include "sync.h"
+
+#include "adf.h"
+#include "adf_fops.h"
+#include "adf_sysfs.h"
+
+#define CREATE_TRACE_POINTS
+#include "adf_trace.h"
+
+#define ADF_SHORT_FENCE_TIMEOUT (1 * MSEC_PER_SEC)
+#define ADF_LONG_FENCE_TIMEOUT (10 * MSEC_PER_SEC)
+
+static void adf_fence_wait(struct adf_device *dev, struct sync_fence *fence)
+{
+   /* sync_fence_wait() dumps debug information on timeout.  Experience
+  has shown that if the pipeline gets stuck, a short timeout followed
+  by a longer one provides useful information for debugging. */
+   int err = sync_fence_wait(fence, ADF_SHORT_FENCE_TIMEOUT);
+   if (err >= 0)
+  

[RFC 1/4] video: Add generic display entity core

[Greg Hackmann]

From: Laurent Pinchart 

Signed-off-by: Laurent Pinchart 
---
 drivers/video/Kconfig|   1 +
 drivers/video/Makefile   |   1 +
 drivers/video/display/Kconfig|   4 +
 drivers/video/display/Makefile   |   1 +
 drivers/video/display/display-core.c | 362 +++
 include/video/display.h  | 150 +++
 6 files changed, 519 insertions(+)
 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/display.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2e937bd..6d9788d 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2475,6 +2475,7 @@ source "drivers/video/omap2/Kconfig"
 source "drivers/video/exynos/Kconfig"
 source "drivers/video/mmp/Kconfig"
 source "drivers/video/backlight/Kconfig"
+source "drivers/video/display/Kconfig"

 if VT
source "drivers/video/console/Kconfig"
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index e8bae8d..d7fd4a2 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -15,6 +15,7 @@ fb-objs   := $(fb-y)
 obj-$(CONFIG_VT) += console/
 obj-$(CONFIG_LOGO)   += logo/
 obj-y+= backlight/
+obj-y+= display/

 obj-$(CONFIG_EXYNOS_VIDEO) += exynos/

diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
new file mode 100644
index 000..1d533e7
--- /dev/null
+++ b/drivers/video/display/Kconfig
@@ -0,0 +1,4 @@
+menuconfig DISPLAY_CORE
+   tristate "Display Core"
+   ---help---
+ Support common display framework for graphics devices.
diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
new file mode 100644
index 000..bd93496
--- /dev/null
+++ b/drivers/video/display/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_DISPLAY_CORE) += display-core.o
diff --git a/drivers/video/display/display-core.c 
b/drivers/video/display/display-core.c
new file mode 100644
index 000..d2daa15
--- /dev/null
+++ b/drivers/video/display/display-core.c
@@ -0,0 +1,362 @@
+/*
+ * Display Core
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ *
+ * Contacts: Laurent Pinchart 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+static LIST_HEAD(display_entity_list);
+static LIST_HEAD(display_entity_notifiers);
+static DEFINE_MUTEX(display_entity_mutex);
+
+/* 
-
+ * Control operations
+ */
+
+/**
+ * display_entity_set_state - Set the display entity operation state
+ * @entity: The display entity
+ * @state: Display entity operation state
+ *
+ * See  display_entity_state for information regarding the entity states.
+ *
+ * Return 0 on success or a negative error code otherwise.
+ */
+int display_entity_set_state(struct display_entity *entity,
+enum display_entity_state state)
+{
+   int ret;
+
+   if (entity->state == state)
+   return 0;
+
+   if (!entity->ops.ctrl || !entity->ops.ctrl->set_state)
+   return 0;
+
+   ret = entity->ops.ctrl->set_state(entity, state);
+   if (ret < 0)
+   return ret;
+
+   entity->state = state;
+   return 0;
+}
+EXPORT_SYMBOL_GPL(display_entity_set_state);
+
+/**
+ * display_entity_update - Update the display
+ * @entity: The display entity
+ *
+ * Make the display entity ready to receive pixel data and start frame 
transfer.
+ * This operation can only be called if the display entity is in STANDBY or ON
+ * state.
+ *
+ * The display entity will call the upstream entity in the video chain to start
+ * the video stream.
+ *
+ * Return 0 on success or a negative error code otherwise.
+ */
+int display_entity_update(struct display_entity *entity)
+{
+   if (!entity->ops.ctrl || !entity->ops.ctrl->update)
+   return 0;
+
+   return entity->ops.ctrl->update(entity);
+}
+EXPORT_SYMBOL_GPL(display_entity_update);
+
+/**
+ * display_entity_get_modes - Get video modes supported by the display entity
+ * @entity The display entity
+ * @modes: Pointer to an array of modes
+ *
+ * Fill the modes argument with a pointer to an array of video modes. The array
+ * is owned by the display entity.
+ *
+ * Return the number of supported modes on success (including 0 if no mode is
+ * supported) or a negative error code otherwise.
+ */
+int display_entity_get_modes(struct display_entity *entity,
+const struct videomode **modes)
+{
+   if (!entity->ops.ctrl || 

[RFC 0/4] Atomic Display Framework

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

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

[RFC 4/4] video: adf: add memblock helper

[Greg Hackmann]

Provides a dma-buf exporter for memblocks, mainly useful for ADF devices
to wrap their bootloader logos

Signed-off-by: Greg Hackmann ghackm...@google.com
---
 drivers/video/adf/Kconfig|   5 ++
 drivers/video/adf/Makefile   |   2 +
 drivers/video/adf/adf_memblock.c | 150 +++
 include/video/adf_memblock.h |  20 ++
 4 files changed, 177 insertions(+)
 create mode 100644 drivers/video/adf/adf_memblock.c
 create mode 100644 include/video/adf_memblock.h

diff --git a/drivers/video/adf/Kconfig b/drivers/video/adf/Kconfig
index 30b0611..ad0c0eb 100644
--- a/drivers/video/adf/Kconfig
+++ b/drivers/video/adf/Kconfig
@@ -8,3 +8,8 @@ menuconfig ADF_DISPLAY_CORE
depends on ADF
depends on DISPLAY_CORE
tristate Helper for implementing ADF interface ops with Display Core 
devices
+
+menuconfig ADF_MEMBLOCK
+   depends on ADF
+   depends on HAVE_MEMBLOCK
+   tristate Helper for using memblocks as buffers in ADF drivers
diff --git a/drivers/video/adf/Makefile b/drivers/video/adf/Makefile
index 30164ee..97f9c98 100644
--- a/drivers/video/adf/Makefile
+++ b/drivers/video/adf/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_ADF) += adf.o \
 obj-$(CONFIG_COMPAT) += adf_fops32.o
 
 obj-$(CONFIG_ADF_DISPLAY_CORE) += adf_display.o
+
+obj-$(CONFIG_ADF_MEMBLOCK) += adf_memblock.o
diff --git a/drivers/video/adf/adf_memblock.c b/drivers/video/adf/adf_memblock.c
new file mode 100644
index 000..a1b7ec6
--- /dev/null
+++ b/drivers/video/adf/adf_memblock.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright (C) 2013 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include linux/dma-buf.h
+#include linux/highmem.h
+#include linux/memblock.h
+#include linux/slab.h
+
+struct adf_memblock_pdata {
+   phys_addr_t base;
+};
+
+struct sg_table *adf_memblock_map(struct dma_buf_attachment *attach,
+   enum dma_data_direction direction)
+{
+   struct adf_memblock_pdata *pdata = attach-dmabuf-priv;
+   unsigned long pfn = (pdata-base  PAGE_SHIFT);
+   struct page *page = pfn_to_page(pfn);
+   struct sg_table *table;
+   int ret;
+
+   table = kzalloc(sizeof(*table), GFP_KERNEL);
+   if (!table)
+   return ERR_PTR(-ENOMEM);
+
+   ret = sg_alloc_table(table, 1, GFP_KERNEL);
+   if (ret  0)
+   goto err;
+
+   sg_set_page(table-sgl, page, attach-dmabuf-size, 0);
+   return table;
+
+err:
+   kfree(table);
+   return ERR_PTR(ret);
+}
+
+void adf_memblock_unmap(struct dma_buf_attachment *attach,
+   struct sg_table *table, enum dma_data_direction direction)
+{
+   sg_free_table(table);
+}
+
+static void __init_memblock adf_memblock_release(struct dma_buf *buf)
+{
+   struct adf_memblock_pdata *pdata = buf-priv;
+   int err = memblock_free(pdata-base, buf-size);
+
+   if (err  0)
+   pr_warn(%s: freeing memblock failed: %d\n, __func__, err);
+   kfree(pdata);
+}
+
+static void *adf_memblock_do_kmap(struct dma_buf *buf, unsigned long pgoffset,
+   bool atomic)
+{
+   struct adf_memblock_pdata *pdata = buf-priv;
+   unsigned long pfn = (pdata-base  PAGE_SHIFT) + pgoffset;
+   struct page *page = pfn_to_page(pfn);
+
+   if (atomic)
+   return kmap_atomic(page);
+   else
+   return kmap(page);
+}
+
+static void *adf_memblock_kmap_atomic(struct dma_buf *buf,
+   unsigned long pgoffset)
+{
+   return adf_memblock_do_kmap(buf, pgoffset, true);
+}
+
+static void adf_memblock_kunmap_atomic(struct dma_buf *buf,
+   unsigned long pgoffset, void *vaddr)
+{
+   kunmap_atomic(vaddr);
+}
+
+static void *adf_memblock_kmap(struct dma_buf *buf, unsigned long pgoffset)
+{
+   return adf_memblock_do_kmap(buf, pgoffset, false);
+}
+
+static void adf_memblock_kunmap(struct dma_buf *buf, unsigned long pgoffset,
+   void *vaddr)
+{
+   kunmap(vaddr);
+}
+
+static int adf_memblock_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
+{
+   struct adf_memblock_pdata *pdata = buf-priv;
+   unsigned long pfn = pdata-base  PAGE_SHIFT;
+
+   return remap_pfn_range(vma, vma-vm_start, pfn,
+   vma-vm_end - vma-vm_start, vma-vm_page_prot);
+}
+
+struct dma_buf_ops adf_memblock_ops = {
+   .map_dma_buf = adf_memblock_map,
+   .unmap_dma_buf = adf_memblock_unmap,
+   .release = adf_memblock_release,
+   .kmap_atomic = adf_memblock_kmap_atomic,
+   .kunmap_atomic = adf_memblock_kunmap_atomic