[PATCH 12/17] drm: convert crtc to properties/state

[Daniel Vetter]

On Mon, May 26, 2014 at 06:46:30PM +0300, Ville Syrjälä wrote:
> On Mon, May 26, 2014 at 05:37:57PM +0200, Daniel Vetter wrote:
> > On Mon, May 26, 2014 at 06:23:05PM +0300, Ville Syrjälä wrote:
> > > On Mon, May 26, 2014 at 11:31:10AM +0200, Daniel Vetter wrote:
> > > > On Sat, May 24, 2014 at 02:30:21PM -0400, Rob Clark wrote:
> > > > > @@ -92,7 +100,18 @@ int drm_atomic_set_event(struct drm_device *dev,
> > > > >  		struct drm_atomic_state *state, struct drm_mode_object *obj,
> > > > >  		struct drm_pending_vblank_event *event)
> > > > >  {
> > > > > -	return -EINVAL;  /* for now */
> > > > > +	switch (obj->type) {
> > > > > +	case DRM_MODE_OBJECT_CRTC: {
> > > > > +		struct drm_crtc_state *cstate =
> > > > > +			drm_atomic_get_crtc_state(obj_to_crtc(obj), state);
> > > > > +		if (IS_ERR(cstate))
> > > > > +			return PTR_ERR(cstate);
> > > > > +		cstate->event = event;
> > > > > +		return 0;
> > > > > +	}
> > > > > +	default:
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > 
> > > > Hm, I think if we only want completion events on crtcs (which I agree on)
> > > 
> > > I don't. Unless you have a nice way of passing some kind of "fbs now
> > > available for rendering" list back to userland in the single crtc
> > > event. Last time I looked making the drm event stuff deal with
> > > variable length events looked more painful than just adding per
> > > plane events. But I must admit that I didn't really try to do it.
> > 
> > Hm, why can't userspace keep a list of fb ids involved in a given crtc
> > pageflip around and index those with the cookie we pass around?
> > 
> > I really don't see why the kernel has to implement a half-baked event
> > multiplexer (which just copies the same thing n times), while userspace is
> > perfectly capable of doing that itself?
> > 
> > Now if the timestamps would be genuinely different then I'd agree, but
> > then that's not an atomic update.
> 
> User space can't do it if flips get issued faster than the vrefresh
> rate. I still want my "moar fps" triple buffering mode.

Hm right, I forget about this every time it comes up. Imo we should
postpone this to when it actually gets implemented - usualy preemptive
generalism just ends up hurting everyone for now real good. For the
current logic we could ditch all the indirection and callbacks really and
simply require that the ->commit hook sends out the crtc even if it's
there.

And even if we add per-plane events we don't need new hooks imo. Just
adding an event pointer to the plane state structure and firing off those
events together with the crtc one in drivers should be good enough.

The proposed set_event callback plus implementation otoh are serious
overkill.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch