Cursors uAPI vs async update - questions

[Daniel Vetter]

On Mon, Feb 18, 2019 at 01:47:57PM -0300, Helen Koike wrote:
> Hello,
> 
> After some discussions I had with some people I would like to discuss
> some design choices regarding uAPI to expose async updates.
> 
> The plan is to allow userspace to update the cursor plane through the
> atomic API instead of the legacy cursor API.
> 
> The steps I was following were:
>     1) Make the legacy cursor implementation use async updates
> path/helpers; and this should work as before
>     2) Expose a way for userspace to trigger an async updates for any
> plane (adding proper tests to igt)
> 
> Assuming that async here means: update the hw immediately.
> 
> But for this to happen, the idea of legacy cursor API and async update
> should be similar, and my question is: are they?
> 
> The question that was raised was:
> 
> When we perform 100 legacy cursor updates to the kernel between 2
> v-blanks, how many times does the screen update cursor?
> (a) 1 - at vblank
> (b) 100
> (c) depends on the hw and driver
> 
> I would say (b), is that correct?

b)

We even have an igt testcase (kms_cursor_legacy) to make sure that's the
case. Might need some tweaking, but should run on any kms driver.

> If it's (b), then async updates should allow the same.
> (NOTE: Actually not really with the current implementation, if there is
> a sync pending commit, then async update blocks,
> which makes the name confusing).
> 
> Also, if async is not possible, in the current implementation we fall
> back to a sync update. But this is not that interesting for userspace,
> for example,
> if drmModeSetCursor succeeds, xorg will call that every single time it
> gets an input event, but if it fails, there are no expectations
> that cursor moves are async and xorg handles cursor updates it self
> (using the primary plane for instance). So another question here
> is: should we have another flag to say that async update should fail
> instead of falling back to a sync update ?

Maybe? I have no idea what userspace wants to do with this. Definitely
need to understand why userspace wants/needs async exposed through atomic,
that should help with answering. I think a flag like ALLOW_MODESET for
async (e.g. FORCE_ASYNC or fail if not possible) could be useful. But we
don't add new uapi without justification in form of real userspace.

> Another point is, if we perform an async update while there is still a
> pending commit, It seems to me that instead of falling back to a
> sync update, it would be better for the userspace if we could amend the
> pending commit, what do you think?

>From a software pov the current async logic essentialy does exactly that -
it amends the currently pending commit. We can extend that, but it gets
really tricky, really fast. Again, we need to know the exact user of this,
lots of things are possible.

> And I just want to clarify (just because this wans't clear to me before
> and might not be clear to others) that async update (programming
> the hw immediately) can cause tearing if the hw reads the configuration
> dynamically during a scanout.

Yup. There's also hw where async cursor just keeps updating, and only the
last update before vblank will be the one that's shown, so no tearing. But
we need to allow both I think. If userspace isn't happy with that (because
it e.g. wants to use async for video updates or something like that), then
we might need another flag.
-Daniel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch