Le jeudi 18 juillet 2024 à 16:43 +0200, Benjamin Gaignard a écrit : > Le 18/07/2024 à 16:02, Nicolas Dufresne a écrit : > > Le jeudi 18 juillet 2024 à 09:56 -0400, Nicolas Dufresne a écrit : > > > Hi, > > > > > > Le jeudi 18 juillet 2024 à 09:04 +0200, Benjamin Gaignard a écrit : > > > > Le 17/07/2024 à 19:50, Nicolas Dufresne a écrit : > > > [...] > > > > > > > > > > > > @@ -1569,6 +1569,7 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, > > > > > > > > > int ret = check_fmt(file, p->type); > > > > > > > > > u32 mbus_code; > > > > > > > > > u32 cap_mask; > > > > > > > > > + u32 flags; > > > > > > > > > > > > > > > > > > if (ret) > > > > > > > > > return ret; > > > > > > > > > @@ -1578,8 +1579,10 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, > > > > > > > > > p->mbus_code = 0; > > > > > > > > > > > > > > > > > > mbus_code = p->mbus_code; > > > > > > > > > + flags = p->flags & V4L2_FMT_FLAG_ENUM_ALL_FORMATS; > > > > > > > > > memset_after(p, 0, type); > > > > > > > > > p->mbus_code = mbus_code; > > > > > > > > > + p->flags = flags; > > > > > > > > Won't this set V4L2_FMT_FLAG_ENUM_ALL_FORMATS (if present) in the > > > > > > > > flags returned to userspace ? Shouldn't be drivers to set > > > > > > > > V4L2_FMT_FLAG_ALL_FORMATS instead ? > > > > > > > memset_after zeroed flags field so we need this to send V4L2_FMT_FLAG_ENUM_ALL_FORMATS > > > > > > > flag to drivers. Return it to userspace is a side effect but I don't that is problem > > > > > > > since it set it anyway. > > > > > > > > > > > > > Ok, if the expectation is that the flag is preserved through the ioctl > > > > > > call, this is fine with me > > > > > I might be missing something other similar features are explicitly advertised by > > > > > drivers. This way, the generic layer can keep or clear the flag based of if its > > > > > supported. The fact the flag persist the ioctl() or not endup having a useful > > > > > semantic. > > > > > > > > > > Could we do the same? > > > > It is the only flag set by userspace when calling the ioctl(), all others > > > > are set by the drivers. > > > > I can clean it from the ioctl() structure after driver call but that won't change anything. > > > This does not answer my question. In other similar feature, we have an > > > **internal** flag set by drivers to tell the framework that such feature is > > > abled. Using that, the framework can keep or remove that flag based on if its > > > supported or not. This way, userspace have a clue if the driver do have this > > > support or if the returned result (in that case) is just a subset matching the > > > configuration. We don't seem to have done the same level of effort here. > > For the reference, you actually use that semantic in GStreamer implementation, > > but the kernel code seems broken. > > > > https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7078/diffs#eb90d5495df2f085f163996014c748a36f143f76_516_527 > > device_caps u32 field is already almost fully used, only one 1 bit is free. > I could use it but, for me, the capability to enumerate all the formats > doesn't fit well in the existing list. Sorry, but I will re-iterate that I don't mean to add a uAPI but an **internal** flag between driver and framework. This way the framework knows at least. > > Benjamin > > > > > > Nicolas > > > > > > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > > > > > > > > > > > > > > > switch (p->type) { > > > > > > > > > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > > > > > > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > > > > > > > index fe6b67e83751..b6a5da79ba21 100644 > > > > > > > > > --- a/include/uapi/linux/videodev2.h > > > > > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > > > > > @@ -886,6 +886,8 @@ struct v4l2_fmtdesc { > > > > > > > > > #define V4L2_FMT_FLAG_CSC_HSV_ENC V4L2_FMT_FLAG_CSC_YCBCR_ENC > > > > > > > > > #define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0100 > > > > > > > > > #define V4L2_FMT_FLAG_META_LINE_BASED 0x0200 > > > > > > > > > +#define V4L2_FMT_FLAG_ENUM_ALL_FORMATS 0x0400 > > > > > > > > > +#define V4L2_FMT_FLAG_ALL_FORMATS 0x0800 > > > > > > > > > > > > > > > > > > /* Frame Size and frame rate enumeration */ > > > > > > > > > /* > > > > > > > > > -- > > > > > > > > > 2.43.0 > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > Kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxx > > > > > > > > To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxx > > > > > > > > This list is managed by https://mailman.collabora.com > > > > _______________________________________________ > > > > Kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxx > > > > To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxx > > > > This list is managed by https://mailman.collabora.com > > > _______________________________________________ > > > Kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxx > > > To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxx > > > This list is managed by https://mailman.collabora.com
