Hi Hans,
Thanks for the set.
On Tue, Aug 07, 2018 at 12:28:45PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> Add a new topology struct that includes properties.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> include/uapi/linux/media.h | 40 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 36f76e777ef9..1910c091601e 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -342,6 +342,40 @@ struct media_v2_link {
> __u32 reserved[6];
> } __attribute__ ((packed));
>
> +#define MEDIA_PROP_TYPE_U64 1
> +#define MEDIA_PROP_TYPE_S64 2
> +#define MEDIA_PROP_TYPE_STRING 3
> +
> +/**
> + * struct media_v2_prop - A media property
> + *
> + * @id: The unique non-zero ID of this property
> + * @owner_id: The ID of the object this property belongs to
This assumes everything has a graph object ID. Speaking of "everything",
one of the use cases for this could be telling the user whether the media
device registration is finished.
One approach could be to create a special graph object with a constant ID
for that purpose. As the type is a part of the ID field, we could simply
create a new type of constant objects.
On a sidenote, the proposed API effectively prohibits conveying structured
data. One use case for that would be to tell about camera modules. We don't
have that right now as part of the uAPI as there's nothing the kernel
currently knows about camera modules --- because there's nothing to control
there. The user space would still be interested in knowing quite a few
parameters of these devices which means the concept of the camera module
would be good to have in precisely this kind of an interface. What should
be there could in practice be what does the module contain (some of the
components are sub-devices whereas some would not be, such as an IR filter
or the lens) and their parameters (e.g. focal length).
I'm not demanding this must be a part of the API in the beginning, however
this was one of the reasons why I originally proposed using JSON:
<URL:https://www.spinics.net/lists/linux-media/msg90160.html>
The downside with JSON would be that it does not fit very well with the
rest of the MC API.
> + * @type: Property type
> + * @flags: Property flags
> + * @payload_size: Property payload size, 0 for U64/S64
> + * @payload_offset: Property payload starts at this offset from &prop.id.
> + * This is 0 for U64/S64.
> + * @reserved: Property reserved field, will be zeroed.
> + * @name: Property name
> + * @uval: Property value (unsigned)
> + * @sval: Property value (signed)
> + */
> +struct media_v2_prop {
> + __u32 id;
> + __u32 owner_id;
> + __u32 type;
> + __u32 flags;
> + __u32 payload_size;
> + __u32 payload_offset;
> + __u32 reserved[18];
That's plenty. What is the typical size of the properties array for a media
device with, say, a few hundred entities with some four pads each, linked
with a single link on average?
> + char name[32];
Is 32 enough? For controls and formats it's been sometimes a bit limiting.
Another approach could be to make this variable size, but that would make
parsing a little bit more complicated. As you already have separated the
property descriptor and the payload, that wouldn't be a major change
anymore.
> + union {
> + __u64 uval;
> + __s64 sval;
> + };
> +} __attribute__ ((packed));
> +
> struct media_v2_topology {
> __u64 topology_version;
>
> @@ -360,6 +394,10 @@ struct media_v2_topology {
> __u32 num_links;
> __u32 reserved4;
> __u64 ptr_links;
> +
> + __u32 num_props;
> + __u32 props_payload_size;
> + __u64 ptr_props;
> } __attribute__ ((packed));
>
> /* ioctls */
> @@ -368,6 +406,8 @@ struct media_v2_topology {
> #define MEDIA_IOC_ENUM_ENTITIES _IOWR('|', 0x01, struct media_entity_desc)
> #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum)
> #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
> +/* Old MEDIA_IOC_G_TOPOLOGY ioctl without props support */
> +#define MEDIA_IOC_G_TOPOLOGY_OLD 0xc0487c04
> #define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
>
> #ifndef __KERNEL__
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx
