[RFC][PATCH 08/10] WIP: drm: Atomic modeset ioctl

[Rob Clark]

On Sat, Sep 1, 2012 at 11:56 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Sat, Sep 1, 2012 at 5:58 PM, Rob Clark <robdclark at gmail.com> wrote:
>> yeah, nuclear-pageflip would be associated only w/ a single crtc.
>> Actually I was kinda assuming atomic-modeset was too..  ie. moving a
>> plane from one crtc to another would be two ioctls, one to remove it
>> from first crtc, one to add to the other.  Although maybe one big
>> ioctl is better.
>>
>> I do think there is a bit of a grey area between the two.  Ie.
>> enabling/disabling a crtc, or changing what encoder->connector it is
>> hooked to might be taking several vblank cycles, and is a relatively
>> infrequent operation so ok to block.  Enabling/disabling a plane could
>> be much more frequent and should not block.  Returning EINVAL or EBUSY
>> in the transient case where it isn't ready quite yet seems like a good
>> approach.. EBUSY might be better for "it might succeed if you try
>> again in next vblank" and EINVAL for "it won't ever work with current
>> setup, don't bother trying again".
>>
>> Infrequent, can be slow:
>>  + enabling/disabling a new output
>>
>> Semi-frequent, should not block but -EBUSY ok:
>>  + enabling/disabling a plane
>>  + resizing a plane
>>
>> More frequent, should not block, should not need extra "test" ioctl
>> step, but should just work
>>  + page flip, update plane position
>>
>> So for that middle group, I'm a bit fuzzy on where those operations
>> should fit in..
>
> My idea is that the nuclear pageflip should support those since they
> only affect a single crtc. The compositor would simply assume it will
> work out, and the kernel can then succeed, or return either the
> transient or definite -EINVAL if it doesn't work out. In the later
> case the compositor can retry in the next frame (and fall back to gpu
> compositing for the current one).

hmm, but this would imply that the nuclear pageflip must also have a
'test' step.. because the compositor needs to know whether or not it
will work, *then* kick rendering, and *then* issue the actual
pageflip.

But OTOH, some simple things like just flipping or perhaps
flipping+moving should not need a test step.  I guess userspace could
just know that certain operations don't need a test step.  Seems a bit
weird, though.

> [snip]
>
>>> - Also a rather practical one: The lack of nuclear pageflip is the reason
>>>   that Wayland/Weston can't use sprites atm (since set_plane is can be
>>>   synchronous and can't be synced with a pageflip). Kristian therefore
>>>   wants the atomic modeset (which I think is much more invasive) not to
>>>   stall the nuclear pageflip support. And the implementation effort is imo
>>>   really a big difference: For i915.ko I expect the nuclear pageflip to
>>>   mostly boil down to wiring up async plane/cursor updates through the
>>>   latch register (with keeping all the other setup code around since we
>>>   only allow at most one outstanding pageflip per crtc currently anyway)
>>>   and wiring that up with the existing crtc pageflip handler. atomic
>>>   modeset otoh requires an invasive rewrite of the driver code to properly
>>>   handle these shared resources (and support the test mode). The beginning
>>>   of that is happening with the modeset rework, but that's just the first
>>>   step.
>>
>> Hmm, I was thinking Ville's atomic fxns would be useful, but maybe I
>> can still bits from that and try and put nuclear-pageflip beneath
>> atomic-modeset in the stack.
>
> At least in i915 I expect that the atomic modeset and the nuclear
> pageflip would take rather different paths. I'm not even sure whether
> we should support enabling planes in the amotic modeset (userspace can
> always composite the first frame after a modeset with the gpu) and
> handle enabling planes with only the nuclear pageflip.

hmm, ok, yeah.. I guess we could just make nuclear_page_flip a fxnptr
on the crtc funcs.

I suppose I don't mind one way or another about the enabling/disabling
planes.  It seems ok to render first frame w/ gpu, so if it simplifies
things I guess it is ok.

> I guess we /could/ add a few things to the crtc helper code to
> facilitate drivers using them to support atomic modeset. I'm thinking
> of a simple ->check callback at the beginning, which checks whether
> the global configuration can be supported. This checking would be in
> addition to the ->mode_fixup checking we already have in place at the
> encoder/crtc layer.
>
> Then we could add a simple atomic_modeset implementation for using
> these interfaces.
>
> For the nuclear pageflip I guess we'll just add a new crtc interface
> (not in the helper func pointer) which gets all the new parameters and
> let drivers sort things out. Since the flip needs to happen synced to
> the same vblank for everything, I don't see how we could compose this
> without some in-depth hw knowledge (which only the driver has) in some
> generic framework. And we do not yet support pageflip on planes (at
> least not in full glory with drm events and timestamps) anyway.

Right, I was thinking to start without helper..  it is easy enough to
add helpers later if there are some common patterns emerging between
the different drivers.

BR,
-R

> Cheers, Daniel
> --
> Daniel Vetter
> daniel.vetter at ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch