On Mon, Dec 15, 2014 at 09:30:44PM +0200, Ville Syrjälä wrote: > On Mon, Dec 15, 2014 at 11:17:45AM -0800, Matt Roper wrote: > > Not really related to this patch specifically, but I'm a little fuzzy on > > the distinction between drm_crtc->enabled and drm_crtc_state->enable > > (and then by extension intel_crtc->new_enabled). It's not immediately > > clear to me why we can't just have a single 'enabled' in the base state > > structure and then when we have a new config that we're passing around, > > the enabled flag in that structure would serve the purpose that > > intel_crtc->new_enabled does for i915 today. I'm probably > > misunderstanding something. > > Yeah that sounds like where we want to go. No more random bits of > state sprinkled all over the place, and instead have it all neatly > collected in the state struct. Yeah, that's the plane. I don't think we can get rid of ->new_config and ->config though since conceptually for async updates we need to commit the sw side synchronously (so that the next atomic update is relative to the currently scheduled state update). But internally in the driver backend we need to still have the staging and only update once all the things from the old state have been shut down. But everything else should just go in there. The other problem is other drivers not yet converted to atomic. Until we have those we can't get rid of duplicated state variables in core drm structs. But as soon as i915 is fully converted we can forget about them, since then helpers will update those for us. Or at least should - we might need to move§ some code from the atomic helpers used only by drivers with crtc helper hooks to make that work correctly and put it into core code. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
