Re: [PATCH] drm/i915: add support for Z-order of planes.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14-02-28 11:28 AM, Matt Roper wrote:

On Fri, Feb 28, 2014 at 06:03:11PM +0200, Ville Syrjälä wrote:
On Thu, Feb 27, 2014 at 03:44:04PM -0800, Matt Roper wrote:
On Thu, Feb 27, 2014 at 02:36:06PM -0800, Yu Dai wrote:
On 14-02-25 04:19 PM, Matt Roper wrote:

On Thu, Feb 20, 2014 at 04:11:14PM -0800, yu.dai@xxxxxxxxx wrote:
From: "Yu(Alex) Dai" <yu.dai@xxxxxxxxx>

Add "zorder" property to crtc to control Z-order of sprite and
primary planes. The alpha channel of the planes can be enabled
or disabled during Z-order change.

Signed-off-by: Yu(Alex) Dai <yu.dai@xxxxxxxxx>
It seems like this is pretty VLV-specific at the moment.  Aren't you
going to be writing bogus register values if you try to set this
property on a non-VLV platform?

I'm not sure if any of the other non-VLV platforms supported by i915
today can change plane z-orders or not, but this is likely to be
possible and useful on future platforms (or non-Intel platforms) as
well, which may have a different number of overall planes and different
ways of programming them.
Yes, this is for VLV. New patch-set V3 was submitted.

Would it make more sense to move the z-order value to a plane property
so that the interface scales to any number of planes?  If you just set
an integer value as a per-plane zorder-value, the code that actually
programs the hardware can go collect the values for each plane, sort
them to determine an order, and figure out what that translates to in
terms of platform-specific register programming.  This would lend itself
nicely to the "check/calculate" step of the atomic/nuclear operation
once those interfaces land.
The z-order of a plane really has no meaning until the actual composition is
triggered. In Android, the order is determined by HWC when there are changes
in layers. Breaking up this into plane property will evoke more drm IOCTL
calls from user. The current "one-shot" call limits the flexibility but makes
it simple.
That's true today where we only really have the ability to set
properties one by one.  But I believe the end goal is to handle this
kind of functionality via the atomic/nuclear API's, where you wind up
shoving all the various things you want to change/program into a
property set (which happens in libdrm userspace) and then issue a single
ioctl which hands that property set to the kernel to be processed in a
sort of transactional model.

Without the atomic/nuclear operations, I'm not sure how well z-order
property will really work in practice.  If your compositor decides that
the next frame of content uses multiple hardware planes to display
various buffers, you really need to make sure that the z-order register
programming happens within the same vblank that the flips on each of
those planes take effect, otherwise things aren't going to look good to
the end user.  With the non-atomic API's, I don't think there's really
any way to guarantee this.
Yeah w/o the atomic API probably the only semi useable approach is to
drop out from the sprite path when stuff actually moves around, and
get back on it after the scene is again steady.

As far as the z-order property, I did have a sort of similar idea
originally where we'd define a bunch of different plane z-order
combinations as an enum property on the crtc, and then the user has
to choose one. That would make it easier for the user to figure out
which way the planes can be stacked. This would be especially nice
with some old Intel hardware where the z-order for the planes was
specified using a couple of magic bits which severeily limited the
ways you could stack the half a dozen or so planes.

But then you'd either need to parse the string enum name to make any
sense of it, or we'd need to encode the order in the value itself. But
to be hardware agnostic, we'd need to use plane IDs in there, and those
can in theory be up to 31bits, so we couldn't guarantee that the property
value could hold more than two planes. Although I suppose we could use
the plane index instead, which would then put the upper limit at 16 planes.
Well, possibly more in case not all planes can be assigned to the same
crtc.

The simple approach is of course to have a range property per plane
which defines the z-order. But then we need to worry about conflicts
between z-order assignment, and it's harder for the user to figure out
which combinations are legal. But the benefit is that this is the most
obvious way to present this information. IIRC some people were
suggesting we pick this scheme despite the limitations.

--
Ville Syrjälä
Intel OTC
Conflicts between z-order assignments don't seem too worrisome to me;
I figure most display servers would probably (re-)program z-order values
for all planes when they want to make any kind of plane ordering change
at all, so you generally wouldn't accidentally hit the case where, for
example, you set a plane to level "1" while another plane was already
sitting at level "1."

Figuring out which orderings are legal on the given hardware is somewhat
more of a problem.  It seems like the natural solution is to expect the
compositor or DDX run through a few plane ordering possibilities with
the atomic "test only" flag set when it starts up; it can then adjust
its runtime plane ordering logic according to what it determines is
possible.


Matt

Thanks for the review. I submitted patch-set 4 for this. Message-Id: <1393884748-8508-1-git-send-email-yu.dai@xxxxxxxxx>

Since this is for VLV only for now, I moved the majority code to vlv_set_plane_order. And it will be called by i915_set_plane_order. For the zorder property itself, it's now 64 bits. On current implementation for VLV, only 5 bits are needed.Be note that the planes information is setup by drmModeSetPlane.So, no need to pack things like plane id into this property. For other platform, hopefully the order can be packed into the 64 bits integer.

Alex
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux