[PATCH] drm/uapi: document kernel capabilities

[Simon Ser]

On Friday, March 5th, 2021 at 9:28 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> > +/**
> > + * DRM_CAP_DUMB_PREFERRED_DEPTH
> > + *
> > + * The preferred depth (in bits) for dumb buffers.
>
> this is literally depth, not bits per pixel, right?

"Depth" is pretty ambiguous [1]. Maybe we should be more explicit here and say
that it's the number of bits used to indicate the color of a single pixel? And
maybe add a note that it's different from bits per pixel?

[1]: https://en.wikipedia.org/wiki/Color_depth

> > + */
> >  #define DRM_CAP_DUMB_PREFERRED_DEPTH	0x3
> > +/**
> > + * DRM_CAP_DUMB_PREFER_SHADOW
> > + *
> > + * If set to 1, the driver prefers userspace to render to a shadow buffer
> > + * instead of directly rendering to a dumb buffer.
>
> Maybe add something like:
>
> For best speed, userspace should do streaming ordered memory copies
> into the dumb buffer and never read from it.
>
> Isn't that correct?

Good call, will add.

> > + */
> >  #define DRM_CAP_DUMB_PREFER_SHADOW	0x4
> > +/**
> > + * DRM_CAP_PRIME
> > + *
> > + * Bitfield of supported PRIME sharing capabilities. See &DRM_PRIME_CAP_IMPORT
> > + * and &DRM_PRIME_CAP_EXPORT.
> > + */
> >  #define DRM_CAP_PRIME			0x5
> > +/**
> > + * DRM_PRIME_CAP_IMPORT
> > + *
> > + * If this bit is set in &DRM_CAP_PRIME, the driver supports importing PRIME
> > + * buffers.
>
> What are PRIME buffers?

Will add something like:

    PRIME buffers are exposed as dma-buf file descriptors. See
    Documentation/gpu/drm-mm.rst, section "PRIME Buffer Sharing".

> > + */
> >  #define  DRM_PRIME_CAP_IMPORT		0x1
> > +/**
> > + * DRM_PRIME_CAP_EXPORT
> > + *
> > + * If this bit is set in &DRM_CAP_PRIME, the driver supports exporting PRIME
> > + * buffers.
>
> What's the export API? HandleToFD()?

Yes. Will add a note about it. Same for import.

> > + */
> >  #define  DRM_PRIME_CAP_EXPORT		0x2
> > +/**
> > + * DRM_CAP_TIMESTAMP_MONOTONIC
> > + *
> > + * If set to 0, the kernel will report timestamps with the realtime clock in
> > + * struct drm_event_vblank. If set to 1, the kernel will report timestamps with
> > + * the monotonic clock.
>
> I think it would be more explicit to say CLOCK_REALTIME and
> CLOCK_MONOTONIC, because there are things like CLOCK_MONOTONIC_RAW
> which is different. Mention clock_gettime()?

Ack, better be explicit here.

> > + */
> >  #define DRM_CAP_TIMESTAMP_MONOTONIC	0x6
> > +/**
> > + * DRM_CAP_ASYNC_PAGE_FLIP
> > + *
> > + * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC.
>
> Does this apply equally to both legacy and atomic KMS API?

Yes (it's included in DRM_MODE_ATOMIC_FLAGS), but I heard that some drivers
get it wrong.

> > + */
> >  #define DRM_CAP_ASYNC_PAGE_FLIP		0x7
> > -/*
> > - * The CURSOR_WIDTH and CURSOR_HEIGHT capabilities return a valid widthxheight
> > - * combination for the hardware cursor. The intention is that a hardware
> > - * agnostic userspace can query a cursor plane size to use.
> > +/**
> > + * DRM_CAP_CURSOR_WIDTH
> > + *
> > + * The ``CURSOR_WIDTH`` and ``CURSOR_HEIGHT`` capabilities return a valid
> > + * width x height combination for the hardware cursor. The intention is that a
> > + * hardware agnostic userspace can query a cursor plane size to use.
> >   *
> >   * Note that the cross-driver contract is to merely return a valid size;
> >   * drivers are free to attach another meaning on top, eg. i915 returns the
> >   * maximum plane size.
> >   */
> >  #define DRM_CAP_CURSOR_WIDTH		0x8
> > +/**
> > + * DRM_CAP_CURSOR_HEIGHT
> > + *
> > + * See &DRM_CAP_CURSOR_WIDTH.
> > + */
> >  #define DRM_CAP_CURSOR_HEIGHT		0x9
> > +/**
> > + * DRM_CAP_ADDFB2_MODIFIERS
> > + *
> > + * If set to 1, the driver supports supplying modifiers in the
> > + * &DRM_IOCTL_MODE_ADDFB2 ioctl.
> > + */
> >  #define DRM_CAP_ADDFB2_MODIFIERS	0x10
> > +/**
> > + * DRM_CAP_PAGE_FLIP_TARGET
> > + *
> > + * If set to 1, the driver supports the &DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE and
> > + * &DRM_MODE_PAGE_FLIP_TARGET_RELATIVE flags in
> > + * &drm_mode_crtc_page_flip_target.flags for the &DRM_IOCTL_MODE_PAGE_FLIP
> > + * ioctl.
> > + */
> >  #define DRM_CAP_PAGE_FLIP_TARGET	0x11
> > +/**
> > + * DRM_CAP_CRTC_IN_VBLANK_EVENT
> > + *
> > + * If set to 1, the kernel supports reporting the CRTC ID in
> > + * &drm_event_vblank.crtc_id.
>
> Does this not apply also to the pageflip / atomic completion event?

Both DRM_EVENT_VBLANK and DRM_EVENT_FLIP_COMPLETE use the struct
drm_event_vblank, so yes. I'll mention these two events explicitly.

> > + */
> >  #define DRM_CAP_CRTC_IN_VBLANK_EVENT	0x12
> > +/**
> > + * DRM_CAP_SYNCOBJ
> > + *
> > + * If set to 1, the driver supports sync objects. See
> > + * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects".
> > + */
> >  #define DRM_CAP_SYNCOBJ		0x13
> > +/**
> > + * DRM_CAP_SYNCOBJ_TIMELINE
> > + *
> > + * If set to 1, the driver supports timeline operations on sync objects. See
> > + * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects".
> > + */
> >  #define DRM_CAP_SYNCOBJ_TIMELINE	0x14
> >
> >  /* DRM_IOCTL_GET_CAP ioctl argument type */
>
> I'm so happy seeing this doc appear! :-)
> Sorry for trolling you into it. ;-)

Np :)