On Wed, Mar 19, 2014 at 11:15:36AM -0700, Matt Roper wrote: > On Wed, Mar 19, 2014 at 12:28:43PM +0100, Daniel Vetter wrote: > > On Tue, Mar 18, 2014 at 05:22:48PM -0700, Matt Roper wrote: > ... > > > + > > > + /* > > > + * set_config() adjusts crtc->primary->fb; however the DRM setplane > > > + * code that called us expects to handle the framebuffer update and > > > + * reference counting; save and restore the current fb before > > > + * calling it. > > > + */ > > > + tmpfb = plane->fb; > > > + ret = crtc->funcs->set_config(&set); > > > > I wonder whether we should have an oppportunistic path using the page_flip > > interface here. Otoh we must have a fallback to ->set_config anyway since > > the drivers currently only allow pageflips on identical buffers. E.g. i915 > > rejects tiling changes and stride changes and position changes. So I think > > having a pageflip fastpath isn't worth the trouble. > > > > Also I think you need to use set_config_internal here to get the > > refcounting right. > > Hmm. I specifically didn't use set_config_internal here because I > thought drm_mode_setplane() (and presumably the future atomic ioctl > which may also call into this) will already handle the refcounting for > us. Am I overlooking something?
Hm, I need to take a closer look again, but set_config has some really funny rules about who updates which pointer and who grabs references for which fb. Mostly since this has all grown rather add-hoc. But you're right and the update_plane code already has some refcounting and frobbing of its own, so I'm not sure what we really need to do here. I think it at least needs a really big comment explaining what's going on. Note that iirc the crtc helper code can still disable connectors in _any_ setcrtc call, so you might accidentally end up in a full modeset, with all the fun this entails. Definitely need to think more about this here. I hope we don't need to rework the semantics of all drivers, since that would be major pain. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch