[Intel-gfx] [PATCH] drm/atomic: pass old crtc state to atomic_begin/flush.
ville.syrjala at linux.intel.com
Tue Jun 16 01:27:55 PDT 2015
On Mon, Jun 15, 2015 at 03:53:43PM +0200, Daniel Vetter wrote:
> On Mon, Jun 15, 2015 at 11:18:46AM +0200, Maarten Lankhorst wrote:
> > Op 15-06-15 om 11:13 schreef Daniel Vetter:
> > > 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 at 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.
> > Agreed. Right now get_initial_plane_config takes an initial_plane_state, could we make this atomic too?
> The initial fb takeover code is a bit tricky since we need to temporarily
> store a few things while not everything is set up yet fully. We could try
> to move that information into the plane state, but it would duplicate
> existing information stored in state->fb->i915_gem_object. Not sure
> whether it's worth it to have something fully atomic for plane state
> The other option would be to allow enabled planes without a full-blown fb
> object, but experience says this leads to piles of drama in the watermark
We could create some kind of fake fb without an actual gem object. The
wm code just needs the metadata.
More information about the dri-devel