[RFC PATCH] drm: Add plane event

[Kristian Høgsberg]

On Wed, Apr 18, 2012 at 10:36 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Wed, Apr 18, 2012 at 05:26:07PM +0300, Ville Syrjälä wrote:
>> On Wed, Apr 18, 2012 at 04:04:56PM +0200, Marcus Lorentzon wrote:
>> > On 04/18/2012 02:25 PM, Rob Clark wrote:
>> > > On Wed, Apr 18, 2012 at 5:11 AM, Joonyoung Shim<jy0922.shim at samsung.com>  wrote:
>> > >> On 04/18/2012 05:46 PM, Daniel Vetter wrote:
>> > >>> On Wed, Apr 18, 2012 at 01:31:59PM +0900, Joonyoung Shim wrote:
>> > >>>> DRM_MODE_PLANE_EVENT is similar to DRM_MODE_PAGE_FLIP_EVENT but it is
>> > >>>> for a plane. The setplane ioctl (DRM_IOCTL_MODE_SETPLANE) needs to
>> > >>>> provide the event such as DRM_MODE_PAGE_FLIP_EVENT. The setplane ioctl
>> > >>>> can change the framebuffer of plane but user can't know completion of
>> > >>>> changing the framebuffer of plane via event. If DRM_MODE_PLANE_EVENT is
>> > >>>> added, we can also do pageflip of a plane.
>> > >>>>
>> > >>>> Signed-off-by: Joonyoung Shim<jy0922.shim at samsung.com>
>> > >>>> Signed-off-by: Kyungmin Park<kyungmin.park at samsung.com>
>> > >>> If I understand the current kms api correctly, set_plane is akin to
>> > >>> set_base and should not generate an asynchronous flip completion event.
>> > >>> To
>> > >>> do that we need a new pageflip ioctl which changes a complete set of fb +
>> > >>> planes + any crtc attributes that might be in an atomic fashion. At which
>> > >>> point we can just reuse the existing page flip event mechanism.
>> > >>
>> > >> It seems better way to add new pageflip ioctl for plane. I will try it.
>> > > fwiw, an atomic mode set which can update crtc and zero or more plane
>> > > layers is, I think, the way to go.  Jesse Barnes had an RFC for this,
>> > > although IIRC it was only the API and not the implementation.  And
>> > > combination w/ the plane/crtc properties patchset to allow setting
>> > > properties as part of the update might not be a bad thing either.
>> > >
>> > > BR,
>> > > -R
>> >
>> > Before planes and rotation properties modeset was atomic (single ioctl).
>> > Would it not be possible to define atomic modeset for planes and
>> > properties like this?
>> >
>> > SETPROPERTY and SETPLANE (maybe more) should not be commited when set,
>> > only on SETCRTC ioctl like before planes. All properties and plane
>> > changes before SETCRTC should be considered staged settings for the next
>> > SETCRTC.
>> > If this would break API backwards compatibility, maybe a new "standard"
>> > boolean property called EnableAtomicMode could be used to trigger this
>> > mode in the drivers. If a driver doesn't support it, things will just
>> > work as currently with modeset not being atomic across planes.
>> > Maybe this could be implemented in the generic parts of KMS, but it
>> > could be tried out first inside a driver.
>>
>> Multi ioctl solution can have some issues since you can't hold any
>> locks around the whole operation. Also there could be issues if the
>> process performing the operation dies or hangs in mid operation.
>> Error handling can also be difficult since the intermediate steps
>> may violate some constraints in the system.
>>
>> Also SETCRTC itself is a per-CRTC operation, but we actually want
>> per-device atomic operations instead.
>>
>> So I think a single ioctl solution is a better idea. I'm currently
>> pondering on what the API would look like. Ideally I would like to
>> go with the "everything is a property" approach, and I'd like to get
>> rid of some of the other limitations in the current API at the same
>> time.
>
> Last time around I've discussed with people we've ended up with 2 new
> ioctls:
>
> - atomic modeset, to configure the output state on more than one crtc at
>  the same time. This is useful to get pll assignement, memory bandwidth
>  constrains and similar stuff right. This ioctl is synchronous. A
>  testmode can be used to inquire the driver whether the proposed mode
>  actually works. This could be used for gui interfaces to automatically
>  grey out unsupportable configurations, e.g. if you have 3 crtc but on 2
>  pll and 2 modes need to have the same pixelclocks to drive all 3 crtcs.
>
> - an atomic pageflip. This one would be like the current pageflip ioclt,
>  i.e. run asynchronously and deliver a drm event upont completion. The
>  idea is that compositors can use this to make flicker-free compositition
>  with drm planes possible. I think we want drivers to be able to indicate
>  which crtc properties they can switch with this ioctl, e.g. I expect
>  some yuv->rbg color space properties might not work. All the changes
>  should be applied on the same vblank, obviously.
>
> But Jesse and Kristian have been more active in this discussion, cc'ed
> them.

Yeah, that sums it up pretty well.  The atomic modeset allows KMS to
allocate resources (bandwidth, connectors, encoders etc) across all
crtcs may take a while to complete.  The nuclear pageflip only affects
one crtc at a time, but ideally should let us change all properties
that don't affect bandwidth or timing parameters: fb/overlay/sprite/hw
cursor base addresses, overlay/sprite/hw cursor position, scaling
properties.

One thing that's not clear to me is how we would enable a sprite
without going through the atomic modeset again.  If the atomic modeset
is all about calculating bandwidth requirements etc, then enabling a
sprite will affect that and it may or may not be possible based on the
current configuration.  However, enabling a sprite from one frame to
another is something that we would want to do as part of the nuclear
pageflip.  So I'm not sure how this would work... maybe we could have
a prepare_sprite_enable type ioctl that verifies that it's possible to
add the sprite plane to the current configuration.  Then if that
succeeds, we can use the nuclear pageflip to enable the sprite plane.

Kristian