Re: [PATCH v2 02/20] drm: Don't update plane properties for atomic planes if it stays the same

Daniel Vetter
2015-07-07T16:41:06Z

On Tue, Jul 07, 2015 at 05:08:34PM +0200, Maarten Lankhorst wrote:
> Op 07-07-15 om 14:10 schreef Daniel Vetter:
> > On Tue, Jul 07, 2015 at 12:20:10PM +0200, Maarten Lankhorst wrote:
> >> Op 07-07-15 om 11:18 schreef Daniel Vetter:
> >>> On Tue, Jul 07, 2015 at 09:08:13AM +0200, Maarten Lankhorst wrote:
> >>>> This allows the first atomic call during hw init to be a real modeset,
> >>>> which is useful for forcing a recalculation.
> >>> fbcon is optional, you can't rely on anything being done in any specific
> >>> way. What exactly do you need this for, what's the implications?
> >> In the hw readout I noticed some warnings when I wasn't setting any mode property in the readout.
> >> I want the first function to be the modeset, so we have a sane base to commit changes on.
> >> Ideally this whole function would have a atomic counterpart which does it in one go. :)
> > Yeah. Otoh as soon as we have atomic modeset working we can replace all
> > the legacy entry points with atomic helpers, and then even plane_disable
> > will be a full atomic modeset.
> >
> > What did fall apart with just touching properties/planes now?
> Also when i915 is fully atomic it calculates in intel_modeset_compute_config
> if a modeset is needed after the first atomic call. Right now because
> intel_modeset_compute_config is only called in set_config so this works as expected.
> Otherwise drm_plane_force_disable or rotate_0 will force a modeset,
> and if the final mode is different this will introduce a double modeset.

For expensive properties (i.e. a no-op changes causes something that takes
time like modeset or vblank wait) we need to make sure we filter them out
in atomic_check. Yeah not quite there yet with pure atomic, but meanwhile
the existing legacy set_prop functions should all filter out no-op changes
themselves. If we don't do that for rotation then that's a bug.

Same for disabling planes harder, that shouldn't take time. Especially
since fbcon only force-disable non-primary plane, and for driver load
that's the exact thing we already do in the driver anyway.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation