Re: [RFC PATCH 00/11] Introduce writeback connectors

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 11, 2016 at 10:24:23PM +0100, Brian Starkey wrote:
> On Tue, Oct 11, 2016 at 10:02:43PM +0200, Daniel Vetter wrote:
> > The problem with just that is that there's lots of different things
> > that can feed into the overall needs_modeset variable. That's why we
> > split it up into multiple booleans.
> > 
> > So yes you're supposed to clear connectors_changed if there is some
> > change that you can handle without a full modeset. If you want, think
> > of connectors_changed as
> > needs_modeset_due_to_change_in_connnector_state, but that's cumbersome
> > to type and too long ;-)
> > 
> 
> All right, got it :-). This intention wasn't clear to me from the
> comments in the code.

A patch to update the kernel-doc to make it clearer (there's mode_changed,
connectors_changed and active_changed, plus drm_crtc_needs_modeset) would
be awesome. I'm trying to write useful docs, but since I designed this all
I sometimes forget to make the non-obvious assumptions clear enough.

Volunteered?

> > > > tbh I don't like that, I think it'd be better to make this truly
> > > > one-shot. Otherwise we'll have real fun problems with hw where the
> > > > writeback can take longer than a vblank (it happens ...). So one-shot,
> > > > with auto-clearing to NULL/0 is imo the right approach.
> > > 
> > > That's an interesting point about hardware which won't finish within
> > > one frame; but I don't see how "true one-shot" helps.
> > > 
> > > What's the expected behaviour if userspace makes a new atomic commit
> > > with a writeback framebuffer whilst a previous writeback is ongoing?
> > > 
> > > In both cases, you either need to block or fail the commit - whether
> > > the framebuffer gets removed when it's done is immaterial.
> > 
> > See Eric's question. We need to define that, and I think the simplest
> > approach is a completion fence/sync_file. It's destaged now in 4.9, we
> > can use them. I think the simplest uabi would be a pointer property
> > (u64) where we write the fd of the fence we'll signal when write-out
> > completes.
> > 
> 
> That tells userspace that the previous writeback is finished, I agree that's
> needed. It doesn't define any behaviour in case userspace asks for another
> writeback before that fence fires though.

Hm, good point. I guess we could just state that if userspace does a
writeback, and issues a new writeback before both a) the atomic flip and
b) the write back complete fence signalled will lead to undefined
behaviour. Undefined as in: data corruption, rejected atomic commit or
anything else than a kernel crash is allowed. This is similar to doing a
page flip and starting to render to the old buffers before the flip event
signalled completion: Userspace gets the mess it asked for ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch