[Intel-gfx] [PATCH 5/8] drm/i915: Don't access to crtc->new_config from intel_mode_max_pixclk()

[Daniel Vetter]

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