drm: document uAPI page-flip flags

[Pekka Paalanen]

On Fri, 26 Aug 2022 13:17:09 +0000
Simon Ser <contact at emersion.fr> wrote:

> On Friday, August 26th, 2022 at 15:10, Sebastian Wick <sebastian.wick at redhat.com> wrote:
> 
> > On Fri, Aug 26, 2022 at 2:17 PM Simon Ser contact at emersion.fr wrote:
> >   
> > > On Friday, August 26th, 2022 at 11:49, Sebastian Wick sebastian.wick at redhat.com wrote:
> > >   
> > > > > > +/*
> > > > > > + * DRM_MODE_ATOMIC_ALLOW_MODESET
> > > > > > + *
> > > > > > + * Allow the update to result in visible artifacts such as a black screen.  
> > > > > 
> > > > > Maybe add:
> > > > > 
> > > > > ...temporary or transient visible artifacts while the update is
> > > > > being applied. Applying the update may also take significantly
> > > > > more time than a page flip. The visual artifacts will not
> > > > > appear after the update is completed.
> > > > > 
> > > > > This flag must be set when the KMS update might cause visible
> > > > > artifacts. Without this flag such KMS update will return EINVAL
> > > > > error. What kind of updates may cause visible artifacts depends
> > > > > on the driver and the hardware. Userspace that needs to know
> > > > > beforehand if an update might cause visible artifacts can use
> > > > > DRM_MODE_ATOMIC_TEST_ONLY without DRM_MODE_ATOMIC_ALLOW_MODESET
> > > > > to see if it fails.
> > > > > 
> > > > > Visual artifacts are guaranteed to not appear when this flag is
> > > > > not set.  
> > > > 
> > > > That doesn't seem to be true, though. For example setting
> > > > HDR_OUTPUT_METADATA for example does result in visual artifacts on my
> > > > display no matter if the flag is specified or not because the
> > > > artifacts are not the result of a mode set but of the display reacting
> > > > to some AVI InfoFrame.  
> > > 
> > > One would need to read the HDMI spec to see if there's anything in
> > > there about artifacts on AVI InfoFrame change, then figure out whether
> > > this is a bug in the physical screen itself or whether the kernel
> > > driver should require ALLOW_MODESET when updating the HDR metadata.  
> > 
> > I'm not even sure if that's the right thing to do. ALLOW_MODESET isn't
> > really about if a commit can lead to visual artifacts but about
> > keeping the existing links alive (someone with more knowledge on this
> > subject probably has a better description for this). An async commit
> > can also introduce visual artifacts for example and there are probably
> > more cases.  
> 
> That's certainly not the intent of ALLOW_MODESET.

One could look at this from the perspective of who is allowed to break.
Maybe the monitor is allowed to break any time (because it is out of
our reach anyway, we can only control the electrical signals going into
the cable), but the source hardware (the display block, everything
before "the cable") is allowed to break only with ALLOW_MODESET.

IOW, if the electrical signal is kept "glitch free" in the sense that
the sink (or a human looking at a sink) cannot see artifacts, then
ALLOW_MODESET is not required. One could even change the video mode
without ALLOW_MODESET if it does not result in artifacts, like
switching from one constant refresh rate to another on VRR capable
hardware - or what I heard Intel does with some transparent power
saving by halving the refresh rate when nothing is changing.

So, at the minimum we must require that the electrical signal cannot
"break". Do we include what a monitor does in ALLOW_MODESET or not is
something I'm not sure, since different sinks presumably break on
different changes.

> See the kernel docs for atomic_check:
> 
> > This callback also needs to correctly fill out the drm_crtc_state in
> > this update to make sure that drm_atomic_crtc_needs_modeset() reflects
> > the nature of the possible update and returns true if and only if the
> > update cannot be applied without tearing within one vblank on that
> > CRTC. The core uses that information to reject updates which require
> > a full modeset (i.e. blanking the screen, or at least pausing updates
> > for a substantial amount of time) if userspace has disallowed that in
> > its request.  
> 
> I'm sure Daniel Vetter can confirm this as well.

I guess the new ASYNC flag patches will fix this wording to allow
tearing in that case?


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220826/c61d58c8/attachment.sig>