From: Pekka Paalanen <ppaalanen@gmail.com>
To: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v3] drm: document drm_mode_get_connector
Date: Fri, 20 Nov 2020 13:37:46 +0200 [thread overview]
Message-ID: <20201120133746.5fa5f146@eldfell> (raw)
In-Reply-To: <4NxrTtynzPiPX4SOCzxmA1sRB8fVLfeiabVpi5j3Y@cp7-web-041.plabs.ch>
[-- Attachment #1.1: Type: text/plain, Size: 5395 bytes --]
On Fri, 20 Nov 2020 08:57:33 +0000
Simon Ser <contact@emersion.fr> wrote:
> Document how to perform a GETCONNECTOR ioctl. Document the various
> struct fields. Also document how to perform a forced probe, and when
> should user-space do it.
>
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> ---
> include/uapi/drm/drm_mode.h | 78 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index f29c1d37be67..3979389fcc4f 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -368,27 +368,95 @@ enum drm_mode_subconnector {
> #define DRM_MODE_CONNECTOR_WRITEBACK 18
> #define DRM_MODE_CONNECTOR_SPI 19
>
> +/**
> + * struct drm_mode_get_connector - Get connector metadata.
> + *
> + * User-space can perform a GETCONNECTOR ioctl to retrieve information about a
> + * connector. User-space is expected to retrieve encoders, modes and properties
> + * by performing this ioctl at least twice: the first time to retrieve the
> + * number of elements, the second time to retrieve the elements themselves.
> + *
> + * To retrieve the number of elements, set @count_props and @count_encoders to
> + * zero, set @count_modes to 1, and set @modes_ptr to a temporary struct
> + * drm_mode_modeinfo element.
How are the counts actually returned?
> + *
> + * To retrieve the elements, allocate arrays for @encoders_ptr, @modes_ptr,
> + * @props_ptr and @prop_values_ptr, then set @count_modes, @count_props and
> + * @count_encoders to their capacity.
> + *
> + * Performing the ioctl only twice may be racy: the number of elements may have
> + * changed with a hotplug event in-between the two ioctls. User-space is
> + * expected to retry the last ioctl until the number of elements stabilizes.
> + * The kernel won't fill any array which doesn't have the expected length.
How does userspace realize the kernel didn't fill in the arrays?
> + *
> + * **Force-probing a connector**
> + *
> + * If the @count_modes field is set to zero, the kernel will perform a forced
> + * probe on the connector to refresh the connector status, modes and EDID.
> + * A forced-probe can be slow and the ioctl will block. A force-probe can cause
> + * flickering and temporary freezes, so it should not be performed
> + * automatically.
> + *
> + * User-space shouldn't need to force-probe connectors in general: the kernel
> + * will automatically take care of probing connectors that don't support
> + * hot-plug detection when appropriate. However, user-space may force-probe
> + * connectors on user request (e.g. clicking a "Scan connectors" button, or
> + * opening a UI to manage screens).
This is well written.
> + */
> struct drm_mode_get_connector {
> -
> + /** @encoders_ptr: Pointer to ``__u32`` array of object IDs. */
> __u64 encoders_ptr;
> + /** @modes_ptr: Pointer to struct drm_mode_modeinfo array. */
> __u64 modes_ptr;
> + /** @props_ptr: Pointer to ``__u32`` array of property IDs. */
> __u64 props_ptr;
> + /** @prop_values_ptr: Pointer to ``__u64`` array of property values. */
> __u64 prop_values_ptr;
>
> + /** @count_modes: Number of modes. */
> __u32 count_modes;
> + /** @count_props: Number of properties. */
> __u32 count_props;
> + /** @count_encoders: Number of encoders. */
> __u32 count_encoders;
>
> - __u32 encoder_id; /**< Current Encoder */
> - __u32 connector_id; /**< Id */
> + /** @encoder_id: Object ID of the current encoder. */
> + __u32 encoder_id;
This is an out value, not an in value, right?
It's not immediately obvious whether any members here are in, out or
in/out values.
> + /** @connector_id: Object ID of the connector. */
> + __u32 connector_id;
> + /**
> + * @connector_type: Type of the connector.
> + *
> + * See DRM_MODE_CONNECTOR_* defines.
> + */
> __u32 connector_type;
> + /**
> + * @connector_type_id: Type-specific connector number.
> + *
> + * This is not an object ID. This is a per-type connector number. Each
> + * (type, type_id) combination is unique across all connectors of a DRM
> + * device.
> + */
> __u32 connector_type_id;
Naming facepalm, oh well...
>
> + /**
> + * @connection: Status of the connector.
> + *
> + * See enum drm_connector_status.
> + */
> __u32 connection;
> - __u32 mm_width; /**< width in millimeters */
> - __u32 mm_height; /**< height in millimeters */
> + /** @mm_width: Width of the connected sink in millimeters. */
> + __u32 mm_width;
> + /** @mm_height: Height of the connected sink in millimeters. */
> + __u32 mm_height;
These are actually more complicated than this, aren't they?
They could be zero for unknown, or both smaller than 20 (???) to
signify only aspect ratio? I've no idea, I just remember something
funny like that from EDID, do these have the same oddities as EDID?
> + /**
> + * @subpixel: Subpixel order of the connected sink.
> + *
> + * See enum subpixel_order.
> + */
> __u32 subpixel;
>
> + /** @pad: Padding, must be zero. */
> __u32 pad;
> };
>
Even with these questions open, this is already a huge improvement!
Thanks,
pq
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
prev parent reply other threads:[~2020-11-20 11:38 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-20 8:57 [PATCH v3] drm: document drm_mode_get_connector Simon Ser
2020-11-20 11:37 ` Pekka Paalanen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201120133746.5fa5f146@eldfell \
--to=ppaalanen@gmail.com \
--cc=contact@emersion.fr \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).