Re: [PATCH] drm/atomic: pass old crtc state to atomic_begin/flush.

From: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/atomic: pass old crtc state to atomic_begin/flush.
Date: Mon, 15 Jun 2015 11:13:28 +0200	[thread overview]
Message-ID: <20150615091328.GI8341@phenom.ffwll.local> (raw)
In-Reply-To: <557E7F0B.9050904@linux.intel.com>

On Mon, Jun 15, 2015 at 09:30:19AM +0200, Maarten Lankhorst wrote:
> Op 15-06-15 om 09:10 schreef Daniel Vetter:
> > On Fri, Jun 12, 2015 at 11:18:22AM +0200, Maarten Lankhorst wrote:
> >> In intel it's useful to keep track of some state changes with old
> >> crtc state vs new state, for example to disable initial planes or
> >> when a modeset's prevented during fastboot.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Hm, thus far the approach has been that the various ->check callbacks diff
> > the state and set appropriate stuff like needs_modeset or planes_changed.
> > And with intel_crtc->atomic we've kinda started to build up similar
> > things for i915. What do you plan to use this for?
> > -Daniel
> On a modeset I want to disable all old planes by calling plane->disable_plane, which is old_crtc_state->plane_mask.
> This is for initial hw readout, where a plane might be active without a fb set. I want to run it during vblank evasion if possible, which
> means in atomic_begin or flush.
> 
> commit_plane is not called if the old and new state both have a NULL fb, so the initial plane would stay active in this case.

Hm, so this is for the i915 state readout code. Imo we shouldn't ever leak
this out of the state readout code but instead sanitize the plane state to
make sense. Roughly this would be:
- read out crtc state
- try to reconstruct initial fb for primary plane, if this succeeds then
  fully link up the plane with the crtc in the plane_state.
- then walk all planes for the crtc, and if any plane is enabled in the hw
  state but doesn't have fb/crtc set in the plane_state force-disable it.

We already do something similar with the vga plane, which we don't
represent at all in kms.

Then none of that initial modeset stuff would ever leak into an atomic
modeset since it would be all contained in sanitize_crtc.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx