Re: [PATCH v2 10/20] drm/i915: Convert suspend/resume to atomic.

Daniel Vetter
2015-07-07T13:17:31Z

On Tue, Jul 07, 2015 at 03:14:57PM +0200, Daniel Vetter wrote:
> On Tue, Jul 07, 2015 at 12:33:25PM +0200, Maarten Lankhorst wrote:
> > Op 07-07-15 om 11:57 schreef Daniel Vetter:
> > > On Tue, Jul 07, 2015 at 09:08:21AM +0200, Maarten Lankhorst wrote:
> > > Since we've only badly bruised escape this trap I think this deserves a
> > > comment:
> > >
> > > /*
> > > * WARNING: We can't do a full atomic modeset including
> > > * compute/check phase here since especially encoder
> > > * compute_config functions depend upon output detection state.
> > > * And that's just not yet available at driver load. Therefore we
> > > * must read out the entire relevant hw state (including any
> > > * driver internal state) faithfully here and only apply the
> > > * commit side.
> > > */
> > >
> > > Hm, makes me think ... should we end up calling just dev->atomic_commit(state) here
> > > once atomic modeset is fully working?
> > Not for initial hw readout unless you want to call detect in this function for all encoders.. resume's fine probably.
>
> I meant calling dev->mode_config.funcs->atomic_commit(state) directly,
> without calling ->atomic_check at all. That should avoid any state
> recomputation (otherwise our check/commit split is botched) and hence be
> exactly what we need here. I didn't check how close intel_set_mode is
> compared our ->atomic_commit implementation after this series (didn't
> apply them all). But I think from a semantic pov those two should match.

Ah I mixed up intel_set_mode with intel_crtc_set_config, which does a more
elaborate compute_config. I guess this is something we still need to
untangle when we replace all the existing legacy entry points with the
legacy2atomic helpers. Probably needs some shuffling of responsibilities
betwen atomic_check and atomic_commit.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation