[PATCH 13/13] RFC: drm: Atomic modeset ioctl

[Michel Dänzer]

On 17.12.2014 16:20, Pekka Paalanen wrote:
> On Wed, 17 Dec 2014 11:48:51 +0900
> Michel Dänzer <michel at daenzer.net> wrote:
> 
>> On 17.12.2014 08:05, Rob Clark wrote:
>>> The atomic modeset ioctl can be used to push any number of new values
>>> for object properties. The driver can then check the full device
>>> configuration as single unit, and try to apply the changes atomically.
>>>
>>> The ioctl simply takes a list of object IDs and property IDs and their
>>> values.
>>
>> [...]
>>
>>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>>> index 86574b0..3459778 100644
>>> --- a/include/uapi/drm/drm_mode.h
>>> +++ b/include/uapi/drm/drm_mode.h
>>> @@ -519,4 +519,25 @@ struct drm_mode_destroy_dumb {
>>>  	uint32_t handle;
>>>  };
>>>  
>>> +/* page-flip flags are valid, plus: */
>>> +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
>>> +#define DRM_MODE_ATOMIC_NONBLOCK  0x0200
>>> +
>>> +#define DRM_MODE_ATOMIC_FLAGS (\
>>> +		DRM_MODE_PAGE_FLIP_EVENT |\
>>> +		DRM_MODE_PAGE_FLIP_ASYNC |\
>>> +		DRM_MODE_ATOMIC_TEST_ONLY |\
>>> +		DRM_MODE_ATOMIC_NONBLOCK)
>>> +
>>> +struct drm_mode_atomic {
>>> +	__u32 flags;
>>> +	__u32 count_objs;
>>> +	__u64 objs_ptr;
>>> +	__u64 count_props_ptr;
>>> +	__u64 props_ptr;
>>> +	__u64 prop_values_ptr;
>>> +	__u64 blob_values_ptr;  /* remove? */
>>> +	__u64 user_data;
>>> +};
>>> +
>>>  #endif
>>>
>>
>> The new ioctl(s) should take an explicit parameter specifying when the
>> changes should take effect. And since variable refresh rate displays are
>> becoming mainstream, that parameter should probably be a timestamp
>> rather than a frame counter.
> 
> That sounds cool to me, but also a rabbit hole. While having worked on
> the Wayland Presentation queueing extension, I'd like to ask the
> following questions:
> 
> - If you set the atomic kick to happen in the future, is there any way
>   to cancel it? I'd be ok with not being able to cancel initially, but
>   if one wants to add that later, we should already know how to
>   reference this atomic submission in the cancel request. What if user
>   space has a bug and schedules an update at one hour or three days from
>   now, how would we abort that?
> 
> - Can you VT-switch or drop DRM master if you have a pending atomic
>   update?
> 
> - Should one be able to set multiple pending atomic updates?
> 
> - If I schedule an atomic update for one CTRC, can I schedule another
>   update for another CRTC before the first one completes? Or am I
>   forced to gather all updates over all outputs in the same atomic
>   submission even if I don't care about or want inter-output sync and
>   the outputs might even be running at different refresh rates?
>   (Actually, this seems to be a valid question even without any target
>   time parameter.)
> 
> - If there can be multiple pending atomic updates on the same DRM
>   device, is there any way to guarantee that the
>   DRM_MODE_ATOMIC_TEST_ONLY results will be accurate also when the
>   atomic update actually kicks in? Another update may have changed the
>   configuration before this update kicks in, which means the overall
>   state isn't the same that was tested.
> 
> - Does a pending atomic update prevent immediate (old style) KMS
>   changes?
> 
> - Assuming hardware cannot do arbitrary time updates, how do you round
>   the given timestamp? Strictly "not before" given time? Round to
>   nearest possible time? The effect of required vs. unwanted sync to
>   vblank?
> 
> - How would user space match the page flip event to the atomic
>   submission it did?
> 
> I wonder if there is a way to postpone these hard(?) questions, so that
> we could have atomic sooner and add scheduling later? I would imagine
> solving everything above is quite some work.

I agree. The main reason I brought it up is because I'd like to avoid
getting into the same situation as with the current
DRM_IOCTL_MODE_PAGE_FLIP ioctl, which doesn't explicitly communicate
between userspace and kernel when the flip is supposed/expected to
occur. We recently had to jump through some hoops in the radeon kernel
driver to prevent flips from occurring sooner than expected by userspace.


But I also think it would be a shame at this point to add new ioctls
which aren't designed with variable refresh rate displays in mind.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer