[RFCv3 03/14] drm: Add primary plane helpers

[Daniel Vetter]

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