On Wed, 11 Oct 2023 11:20:51 +0200 Daniel Vetter <[email protected]> wrote:
> msm is automagically upgrading normal commits to full modesets, and > that's a big no-no: > > - for one this results in full on->off->on transitions on all these > crtc, at least if you're using the usual helpers. Which seems to be > the case, and is breaking uapi > > - further even if the ctm change itself would not result in flicker, > this can hide modesets for other reasons. Which again breaks the > uapi > > v2: I forgot the case of adding unrelated crtc state. Add that case > and link to the existing kerneldoc explainers. This has come up in an > irc discussion with Manasi and Ville about intel's bigjoiner mode. > Also cc everyone involved in the msm irc discussion, more people > joined after I sent out v1. > > Signed-off-by: Daniel Vetter <[email protected]> > Cc: Maarten Lankhorst <[email protected]> > Cc: Maxime Ripard <[email protected]> > Cc: Thomas Zimmermann <[email protected]> > Cc: David Airlie <[email protected]> > Cc: Daniel Vetter <[email protected]> > Cc: Pekka Paalanen <[email protected]> > Cc: Rob Clark <[email protected]> > Cc: Simon Ser <[email protected]> > Cc: Manasi Navare <[email protected]> > Cc: Ville Syrjälä <[email protected]> > Cc: Abhinav Kumar <[email protected]> > Cc: Dmitry Baryshkov <[email protected]> > Signed-off-by: Daniel Vetter <[email protected]> > --- > include/drm/drm_atomic.h | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index cf8e1220a4ac..545c48545402 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -372,8 +372,27 @@ struct drm_atomic_state { > * > * Allow full modeset. This is used by the ATOMIC IOCTL handler to > * implement the DRM_MODE_ATOMIC_ALLOW_MODESET flag. Drivers should > - * never consult this flag, instead looking at the output of > - * drm_atomic_crtc_needs_modeset(). > + * not consult this flag, instead looking at the output of > + * drm_atomic_crtc_needs_modeset(). The detailed rules are: > + * > + * - Drivers must not consult @allow_modeset in the atomic commit path, > + * and instead use drm_atomic_crtc_needs_modeset(). Change to Drivers must not consult @allow_modeset in the atomic commit path. Use drm_atomic_crtc_needs_modeset() instead. maybe? It's hard for me to see the difference between "instead use X" and "instead of X". "Use Y instead (of X)." helps me at least. > + * > + * - Drivers may consult @allow_modeset in the atomic check path, if > + * they have the choice between an optimal hardware configuration > + * which requires a modeset, and a less optimal configuration which > + * can be committed without a modeset. An example would be suboptimal > + * scanout FIFO allocation resulting in increased idle power > + * consumption. This allows userspace to avoid flickering and delays > + * for the normal composition loop at reasonable cost. > + * > + * - Drivers must consult @allow_modeset before adding unrelated struct > + * drm_crtc_state to this commit by calling > + * drm_atomic_get_crtc_state(). See also the warning in the > + * documentation for that function. > + * > + * - Drivers must never change this flag, it is only under the control > + * of userspace. *only userspace may set it. ? > */ > bool allow_modeset : 1; > /** Wording bikeshed aside, Acked-by: Pekka Paalanen <[email protected]> Thanks, pq
pgp1vzhbi_WZP.pgp
Description: OpenPGP digital signature
