On Wed, 28 Sep 2022 09:41:57 +0000
Simon Ser <contact@xxxxxxxxxxx> wrote:
> Document flags accepted by the page-flip and atomic IOCTLs.
>
> v2 (Pekka):
> - Mention DRM_EVENT_FLIP_COMPLETE in DRM_MODE_PAGE_FLIP_EVENT docs.
> - Expand DRM_MODE_ATOMIC_NONBLOCK and DRM_MODE_ATOMIC_ALLOW_MODESET
> description.
> v3:
> - Fix struct field ref syntax (Daniel)
> - Clarify when artifacts are no longer displayed (Daniel)
> - Add note about sinks deciding to show artifacts on their own (Pekka, Daniel)
>
Reviewed-by: Pekka Paalanen <pekka.paalanen@xxxxxxxxxxxxx>
Only nitpicks below, you can ignore them.
> Signed-off-by: Simon Ser <contact@xxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Pekka Paalanen <ppaalanen@xxxxxxxxx>
> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> include/uapi/drm/drm_mode.h | 63 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index fa953309d9ce..9b10327b9d21 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -935,12 +935,31 @@ struct hdr_output_metadata {
> };
> };
>
> +/**
> + * DRM_MODE_PAGE_FLIP_EVENT
> + *
> + * Request that the kernel sends back a vblank event (see
> + * struct drm_event_vblank) with the &DRM_EVENT_FLIP_COMPLETE type when the
> + * page-flip is done.
> + */
> #define DRM_MODE_PAGE_FLIP_EVENT 0x01
> +/**
> + * DRM_MODE_PAGE_FLIP_ASYNC
> + *
> + * Request that the page-flip is performed as soon as possible, ie. with no
> + * delay due to waiting for vblank. This may cause tearing to be visible on
> + * the screen.
> + */
> #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
> #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> #define DRM_MODE_PAGE_FLIP_TARGET_RELATIVE 0x8
> #define DRM_MODE_PAGE_FLIP_TARGET (DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE | \
> DRM_MODE_PAGE_FLIP_TARGET_RELATIVE)
> +/**
> + * 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.
> + */
> #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT | \
> DRM_MODE_PAGE_FLIP_ASYNC | \
> DRM_MODE_PAGE_FLIP_TARGET)
> @@ -1034,11 +1053,53 @@ struct drm_mode_destroy_dumb {
> __u32 handle;
> };
>
> -/* page-flip flags are valid, plus: */
> +/**
> + * DRM_MODE_ATOMIC_TEST_ONLY
> + *
> + * Do not apply the atomic commit, instead check whether the hardware supports
> + * this configuration.
> + *
> + * See &drm_mode_config_funcs.atomic_check for more details on test-only
> + * commits.
> + */
> #define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> +/**
> + * DRM_MODE_ATOMIC_NONBLOCK
> + *
> + * Do not block while applying the atomic commit. The &DRM_IOCTL_MODE_ATOMIC
> + * IOCTL returns immediately instead of waiting for the changes to be applied
> + * in hardware. Note, the driver will still check that the update can be
> + * applied before retuning.
> + */
> #define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> +/**
> + * 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
> + * (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?
> + */
> #define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
>
> +/**
> + * DRM_MODE_ATOMIC_FLAGS
> + *
> + * Bitfield of flags accepted by the &DRM_IOCTL_MODE_ATOMIC IOCTL in
> + * &drm_mode_atomic.flags.
> + */
> #define DRM_MODE_ATOMIC_FLAGS (\
> DRM_MODE_PAGE_FLIP_EVENT |\
> DRM_MODE_PAGE_FLIP_ASYNC |\
Thanks,
pq
Attachment:
pgp3gsTk5JkH9.pgp
Description: OpenPGP digital signature
