Re: [RFC 1/3] drm/i915: Roll intel_crtc->atomic into intel_crtc_state

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 29-08-15 om 01:57 schreef Matt Roper:
> Way back at the beginning of i915's atomic conversion I added
> intel_crtc->atomic as a temporary dumping ground for "stuff to do
> outside vblank evasion" flags since CRTC states weren't properly wired
> up and tracked at that time.  We've had proper CRTC state tracking for a
> while now, so there's really no reason for this hack to continue to
> exist.  Moving forward we want to store intermediate crtc/plane state
> data for modesets in addition to the final state, so moving these fields
> into the proper state object allows us to properly compute them for both
> the intermediate and final state.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
Can I shoot this patch down? It's better to add a field 'wm_changed' to the crtc_state,
which gets reset to false for each crtc_state duplication. It's needed for checking if a cs pageflip
can be done for atomic. It would remove the duplication of some checks there.

The other atomic state members will die soon. I already have some patches to achieve that. :)

I'm not sure if an intermediate state is a good idea. Any code that disables a crtc should only be
looking at the old state. pre_plane_update runs all stuff in preparation for disabling planes,
while post_plane_update runs everything needed for enabling planes. So no need to split it up
I think, maybe put in some intermediate watermarks in intel_atomic_state, but no need for a full
crtc_state.

After a modeset disable you should be able to put in any wm value in .crtc_enable because no plane
will be active.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx