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

[Sebastian Wick]

On Thu, Sep 29, 2022 at 8:11 PM Simon Ser <contact at emersion.fr> wrote:
>
> 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.)
>

Honestly the right thing to do here is give up on the "no visual
artifacts" phrashing. This is really about not having to interrupt the
stream which definitely results in visual artifacts but avoiding that
doesn't guarantee no artifacts.