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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
