[Intel-gfx] Proposal of the z-order patch

[Yu Dai]

On 03/21/2014 04:03 PM, Matt Roper wrote:
> On Thu, Mar 20, 2014 at 11:53:07AM -0700, Yu Dai wrote:
>> All,
>>
>> Thanks for the review of z-order patch. Based on that, we had some
>> internal discussion within our Display driver team. Here is the
>> summary.
>>
>> 1. Re: define plane z-order combinations as an enum property
>>
>> User mode needs extra work to understand the definition. The number
>> of combinations expands dramatically with the increasing of planes.
>> Says for 4 planes, the total combo is 24.
>>
>> 2. Re: per-plane z-order property
>>
>> We prefer the atomic setup of z-order for all planes. So driver can
>> validate the property value in the SetProperty call. Otherwise, if
>> for each plane the separated SetProperty call is made to update the
>> z-order, it could be difficult to handle and validate. For example,
>> when set a plane to zorder 2, there might be another plane with
>> zorder 2 already. In the end, we still need another check point to
>> validate this.
>>
>> 3. We also discussed the possibility of using blob property
>>
>> Some generic blob property calls are needed to handle a property
>> with a list of plane id and zorder etc. But currently it is lack of
>> support in libdrm to pass down user data via blob property. The
>> current drm_mode_obj_set_property really doesn't handle blob. It
>> might be useful to implement a property that can handle complicated
>> data structure from user. But this is long term goal.
>>
>> The new proposal we have is still based on previous solution (64bits
>> property) but it tries to make the code platform agnostic and
>> eliminate the magic number like PASASBCA.
>>
>> Define the supported plane name tag in i915_drm.h. For i915, it might be
>> #define DRM_I915_PLANE_PRIMARY    0
>> #define DRM_I915_PLANE_SPRITE_A   1
>> #define DRM_I915_PLANE_SPRITE_B   2
>> #define DRM_I915_PLANE_CURSOR     3
>>
>> Limits the above setting within 0~15. So we need 4 bits to save it
>> into z-order property. The 64bits of z-order will indicate the order
>> of planes from bottom (lower bits) to top (higher bits) by every 4
>> bits. Bit 3:0 is the plane at bottom; Bit 7:4 is the one right on
>> top of it and so on. In this way, we can support up to 16 planes
>> total. And it is very straightforward for user mode to use. The
>> SetProperty call will reject any invalid setting. The HW specific
>> code will be wrapped up in kernel SetProperty callback. Be note on
>> Intel platform, the PLANE_SPRITE_A/B will be mapped to HW Sprite C/D
>> for pipe 2. And this is crtc property - we don't have to pass around
>> pipe id.
>>
>> Please let us know your opinion on this.
>>
>> Thanks,
>>
>> Alex
>> GCM Core uArch, Intel VGP
> Hi Alex.
>
> I think the first question here is whether you're proposing a solution
> for a pre-atomic or post-atomic driver stack?  Based on your description
> of option #2, it sounds like you're focusing on a pre-atomic environment
> since you're using SetProperty and drm_mode_obj_set_property.  With the
> atomic framework, you can use drmModePropertySetAdd() to add a bunch of
> desired property values to a single userspace property set object, then
> dispatch that whole set to the kernel with drmModePropertySetCommit()
> which calls the DRM_IOCTL_MODE_ATOMIC ioctl.  The kernel can sanity
> check the whole batch of property values and then either accept or
> reject the whole set together in a transactional manner (so you don't
> have to worry about winding up in some strange limbo where some of your
> changes were good and took effect but others weren't).
>
> Not to say your proposed solution directly conflicts with the atomic
> programming approach in any way (programming a single CRTC property
> works the same way as programming a bunch of separate plane properties
> in that case); I just want to make sure you understand the direction
> that we're expecting the API's to go in the near (hopefully!) future so
> that we can make sure we're settling on the best possible approach for
> z-order handling for the long term.
Agree.
> For the specific approach you propose above, can you clarify what the
> logic looks like from the point of view of a generic (non hw-specific)
> userspace compositor (e.g., Weston)?  The compositor, which starts out
> knowing nothing at all about the underlying hardware, would call
> drmModeGetPlaneResources() and get a list of all the planes that are
> present.  How does it know which of the HW-specific plane ID's you
> #define above correspond to which planes in the plane list it received?
> Are these ID's going to show up as immutable plane properties or
> something that userspace can read?  Is it going to assume that those
> values correspond to the order that planes show up in the plane list?
> One of the advantages of using a single value to program z-order as
> you're doing here is that presumably there can be a mechanism to
> advertise what the valid plane orderings are so that the compositor
> doesn't have to spend some time at startup testing the various
> combinations itself...do you have plans on how you'd enumerate the valid
> configs for userspace?
>
I submitted the new patch for review. It is still based on the 
assumption that user knows how those surfaces are mapped to HW planes. 
Such as, which one is primary and which one is sprite A etc. If not such 
case, it won't work.

Alex