[PATCH v3] drm: document uAPI page-flip flags

[Simon Ser]

On Wednesday, September 28th, 2022 at 12:06, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> > +/**
> > + * DRM_MODE_PAGE_FLIP_FLAGS
> > + *
> > + * Bitmask of flags suitable for &drm_mode_crtc_page_flip_target.flags.
> 
> Should this mention also drm_mode_crtc_page_flip.flags?
> 
> UAPI header defines both structs.

drm_mode_crtc_page_flip is "v1", drm_mode_crtc_page_flip_target is "v2". The
latter just replaces a reserved field with a new one. So I figured "v1" is
mostly kept around for backwards compat and everybody should use "v2" for
simplicity's sake.

> > +/**
> > + * DRM_MODE_ATOMIC_ALLOW_MODESET
> > + *
> > + * Allow the update to result in temporary or transient visible artifacts while
> > + * the update is being applied. Applying the update may also take significantly
> > + * more time than a page flip. All visual artifacts will disappear by the time
> > + * the update is completed, as signalled throught the vblank event's timestamp
> 
> typo: throught

Indeed!

> > + * (see struct drm_event_vblank).
> > + *
> > + * This flag must be set when the KMS update might cause visible artifacts.
> > + * Without this flag such KMS update will return a EINVAL error. What kind of
> > + * update may cause visible artifacts depends on the driver and the hardware.
> > + * User-space 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.
> > + *
> > + * To the best of the driver's knowledge, visual artifacts are guaranteed to
> > + * not appear when this flag is not set. Some sinks might display visual
> > + * artifacts outside of the driver's control.
> 
> Ok, so we kept the "visual artifacts" semantics and allow monitors to
> do otherwise.
> 
> I'm still not sure what this means for things like infoframe data where
> changing a certain field (e.g. HDR_OUTPUT_METADATA structure's EOTF
> field) has a high risk of causing a visual glitch. I cannot imagine why
> a monitor manufacturer would not be able to avoid the glitch if they
> wanted to. The glitch might or might not happen, and we cannot know in
> advance or afterwards whether it did happen, but it is probable that
> many monitors will glitch.
> 
> I think "To the best of driver's knowledge" means that if someone
> reports a monitor to glitch, the driver/kernel would need to add that
> field to the "needs modeset" set. But doing so can regress other
> monitors that didn't glitch, so it needs to be a monitor quirk.
> 
> This is not something for this patch, but would it be possible to agree
> on the kernel development policy here? Should glitches be reported and
> added to monitor quirks list? Or should drivers be defensive from the
> start and require modeset if the field is known to cause glitches on
> some monitors?

Good question. I am not sure there is even a desire to have a quirks table for
this among driver developers.

This reminds me of some VRR displays showing visual artifacts when user-space
changes its page-flip rate. If we took this definition by the letter, the
kernel should require ALLOW_MODESET at each page-flip... (The better solution
would be to just to have a slew rate implemented in the kernel. I think it's
implemented to some extent in amdgpu/i915 but still some monitors are having
issues.)