* usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-03-21 9:45 Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2018-03-21 9:45 UTC (permalink / raw)
To: Joel Pepper; +Cc: Felipe Balbi, Greg Kroah-Hartman, linux-usb, Paul Elder
Hi Joel,
Thank you for the patch.
On Tuesday, 20 March 2018 22:28:53 EET Joel Pepper wrote:
> - Add bFrameIndex as a UVCG_FRAME_ATTR_RO for each frame size.
> - Automatically assign ascending bFrameIndex to each frame in a format.
>
> Before all "bFrameindex" attributes were set to "1" with no way to
> configure the gadget otherwise. This resulted in the host always
> negotiating for bFrameIndex 1 (i.e. the first framesize of the gadget).
> After the negotiation the host driver will set the user or application
> selected framesize, while the gadget is actually set to the first
> framesize.
>
> Now, when the containing format is linked into the streaming header,
> iterate over all child frame descriptors and assign ascending indices.
> The automatically assigned indices can be read from the new read only
> bFrameIndex configsfs attribute in each frame descriptor item.
>
> v2: Add the new attribute to both MJPEG and uncompressed frame descriptors
> in Documentation/ABI, with note that it was added only in a later
> kernel version
>
> v3: Changed from simply allowing user to set the value for bFrameIndex to
> automatically assigning correct distinct frame indexes. Changed
> bFrameIndex from RW to RO
I like the new commit message, but it would be nice if you could also update
the patch contents ;-)
(By the way the subject line should have mentioned "[PATCH v3]")
> Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
> ---
> Documentation/ABI/testing/configfs-usb-gadget-uvc | 17 +++++++++++++++++
> drivers/usb/gadget/function/uvc_configfs.c | 3 +++
> 2 files changed, 20 insertions(+)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> b/Documentation/ABI/testing/configfs-usb-gadget-uvc index 1ba0d0f..d435cf7
> 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> @@ -194,6 +194,14 @@ Description: Specific MJPEG frame descriptors
> bmCapabilities - still image support, fixed frame-rate
> support
>
> +Date: Mar 2018
> +KernelVersion: 4.16
> +
> + bFrameIndex - unique id for this framedescriptor;
> + if using multiple framedescriptors for
> + same format, user needs to set distinct
> + value for each frame descriptor
> +
> What: /config/usb-gadget/gadget/functions/uvc.name/streaming/
uncompressed
> Date: Dec 2014
> KernelVersion: 4.0
> @@ -241,6 +249,15 @@ Description: Specific uncompressed frame descriptors
> bmCapabilities - still image support, fixed frame-rate
> support
>
> +Date: Mar 2018
> +KernelVersion: 4.16
> +
> + bFrameIndex - unique id for this
> framedescriptor; + if using multiple
> framedescriptors for + same format,
> user needs to set distinct + value
> for each frame descriptor +
> +
> What: /config/usb-gadget/gadget/functions/uvc.name/streaming/header
> Date: Dec 2014
> KernelVersion: 4.0
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c
> b/drivers/usb/gadget/function/uvc_configfs.c index c9b8cc4a..5966d65 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -992,6 +992,8 @@ UVC_ATTR(uvcg_frame_, cname, aname);
>
> UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
> noop_conversion, 8);
> +UVCG_FRAME_ATTR(b_frame_index, bFrameIndex, noop_conversion,
> + noop_conversion, 8);
> UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
> UVCG_FRAME_ATTR(w_height, wHeight, le16_to_cpu, cpu_to_le16, 16);
> UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, le32_to_cpu, cpu_to_le32,
> 32); @@ -1137,6 +1139,7 @@ UVC_ATTR(uvcg_frame_, dw_frame_interval,
> dwFrameInterval);
>
> static struct configfs_attribute *uvcg_frame_attrs[] = {
> &uvcg_frame_attr_bm_capabilities,
> + &uvcg_frame_attr_b_frame_index,
> &uvcg_frame_attr_w_width,
> &uvcg_frame_attr_w_height,
> &uvcg_frame_attr_dw_min_bit_rate,
^ permalink raw reply [flat|nested] 5+ messages in thread
* usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-03-21 10:01 Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2018-03-21 10:01 UTC (permalink / raw)
To: Joel Pepper; +Cc: Felipe Balbi, Greg Kroah-Hartman, linux-usb, Paul Elder
Hi Joel,
On Wednesday, 21 March 2018 11:53:45 EET Joel Pepper wrote:
> Hi Laurent,
>
> Thanks for the heads up. I noticed the goof with Patch v3 right after
> sending, but apparently I screwed up the commit amend too. I'm still
> getting accustomed to kernel development. Sorry for wasting your time, I
> will have v4 along shortly
If the result is that you become more familiar with kernel development and
keep contributing I'll consider this as time well invested, not wasted.
^ permalink raw reply [flat|nested] 5+ messages in thread
* usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-03-20 20:28 Joel Pepper
0 siblings, 0 replies; 5+ messages in thread
From: Joel Pepper @ 2018-03-20 20:28 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Felipe Balbi, Greg Kroah-Hartman, linux-usb, Paul Elder, Joel Pepper
- Add bFrameIndex as a UVCG_FRAME_ATTR_RO for each frame size.
- Automatically assign ascending bFrameIndex to each frame in a format.
Before all "bFrameindex" attributes were set to "1" with no way to
configure the gadget otherwise. This resulted in the host always
negotiating for bFrameIndex 1 (i.e. the first framesize of the gadget).
After the negotiation the host driver will set the user or application
selected framesize, while the gadget is actually set to the first
framesize.
Now, when the containing format is linked into the streaming header,
iterate over all child frame descriptors and assign ascending indices.
The automatically assigned indices can be read from the new read only
bFrameIndex configsfs attribute in each frame descriptor item.
v2: Add the new attribute to both MJPEG and uncompressed frame descriptors
in Documentation/ABI, with note that it was added only in a later
kernel version
v3: Changed from simply allowing user to set the value for bFrameIndex to
automatically assigning correct distinct frame indexes. Changed
bFrameIndex from RW to RO
Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
---
Documentation/ABI/testing/configfs-usb-gadget-uvc | 17 +++++++++++++++++
drivers/usb/gadget/function/uvc_configfs.c | 3 +++
2 files changed, 20 insertions(+)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 1ba0d0f..d435cf7 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -194,6 +194,14 @@ Description: Specific MJPEG frame descriptors
bmCapabilities - still image support, fixed frame-rate
support
+Date: Mar 2018
+KernelVersion: 4.16
+
+ bFrameIndex - unique id for this framedescriptor;
+ if using multiple framedescriptors for
+ same format, user needs to set distinct
+ value for each frame descriptor
+
What: /config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed
Date: Dec 2014
KernelVersion: 4.0
@@ -241,6 +249,15 @@ Description: Specific uncompressed frame descriptors
bmCapabilities - still image support, fixed frame-rate
support
+Date: Mar 2018
+KernelVersion: 4.16
+
+ bFrameIndex - unique id for this framedescriptor;
+ if using multiple framedescriptors for
+ same format, user needs to set distinct
+ value for each frame descriptor
+
+
What: /config/usb-gadget/gadget/functions/uvc.name/streaming/header
Date: Dec 2014
KernelVersion: 4.0
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index c9b8cc4a..5966d65 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -992,6 +992,8 @@ UVC_ATTR(uvcg_frame_, cname, aname);
UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
noop_conversion, 8);
+UVCG_FRAME_ATTR(b_frame_index, bFrameIndex, noop_conversion,
+ noop_conversion, 8);
UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
UVCG_FRAME_ATTR(w_height, wHeight, le16_to_cpu, cpu_to_le16, 16);
UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, le32_to_cpu, cpu_to_le32, 32);
@@ -1137,6 +1139,7 @@ UVC_ATTR(uvcg_frame_, dw_frame_interval, dwFrameInterval);
static struct configfs_attribute *uvcg_frame_attrs[] = {
&uvcg_frame_attr_bm_capabilities,
+ &uvcg_frame_attr_b_frame_index,
&uvcg_frame_attr_w_width,
&uvcg_frame_attr_w_height,
&uvcg_frame_attr_dw_min_bit_rate,
^ permalink raw reply related [flat|nested] 5+ messages in thread
* usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-03-19 15:22 Greg Kroah-Hartman
0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-19 15:22 UTC (permalink / raw)
To: Joel Pepper; +Cc: linux-usb, Laurent Pinchart, Felipe Balbi
On Mon, Mar 19, 2018 at 11:55:02AM +0100, Joel Pepper wrote:
> This adds bFrameIndex as a UVCG_FRAME_ATTR for each frame size.
> Beforehand all bFrameindex were set to "1" with no way to configure the
> gadget otherwise.
>
> This resulted in the host always negotiating for bFrameIndex 1 (i.e. the
> first framesize of the gadget).
> After the negotiation the host driver will set the user- or application-
> selected framesize while the gadget is actually set to the first
> framesize.
>
> Note that this still requires the gadget to be configured with unique
> 'bFrameindex's for each frameSize of each format through configfs. An
> alternative might be to automatically assign ascending indices when the
> format is linked into the streaming header, but the user space gadget
> application would need a way to check or predict the indices so that it
> can properly interpret PROBE/COMMIT CONTROL requests.
>
> Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
> ---
> drivers/usb/gadget/function/uvc_configfs.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index c9b8cc4a..5966d65 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -992,6 +992,8 @@ UVC_ATTR(uvcg_frame_, cname, aname);
>
> UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
> noop_conversion, 8);
> +UVCG_FRAME_ATTR(b_frame_index, bFrameIndex, noop_conversion,
> + noop_conversion, 8);
> UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
> UVCG_FRAME_ATTR(w_height, wHeight, le16_to_cpu, cpu_to_le16, 16);
> UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, le32_to_cpu, cpu_to_le32, 32);
> @@ -1137,6 +1139,7 @@ UVC_ATTR(uvcg_frame_, dw_frame_interval, dwFrameInterval);
>
> static struct configfs_attribute *uvcg_frame_attrs[] = {
> &uvcg_frame_attr_bm_capabilities,
> + &uvcg_frame_attr_b_frame_index,
Isn't there also a Documentation/ABI/ update you need to do for this new
file?
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-03-19 10:55 Joel Pepper
0 siblings, 0 replies; 5+ messages in thread
From: Joel Pepper @ 2018-03-19 10:55 UTC (permalink / raw)
To: linux-usb; +Cc: Laurent Pinchart, Felipe Balbi, Joel Pepper
This adds bFrameIndex as a UVCG_FRAME_ATTR for each frame size.
Beforehand all bFrameindex were set to "1" with no way to configure the
gadget otherwise.
This resulted in the host always negotiating for bFrameIndex 1 (i.e. the
first framesize of the gadget).
After the negotiation the host driver will set the user- or application-
selected framesize while the gadget is actually set to the first
framesize.
Note that this still requires the gadget to be configured with unique
'bFrameindex's for each frameSize of each format through configfs. An
alternative might be to automatically assign ascending indices when the
format is linked into the streaming header, but the user space gadget
application would need a way to check or predict the indices so that it
can properly interpret PROBE/COMMIT CONTROL requests.
Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
---
drivers/usb/gadget/function/uvc_configfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index c9b8cc4a..5966d65 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -992,6 +992,8 @@ UVC_ATTR(uvcg_frame_, cname, aname);
UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
noop_conversion, 8);
+UVCG_FRAME_ATTR(b_frame_index, bFrameIndex, noop_conversion,
+ noop_conversion, 8);
UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
UVCG_FRAME_ATTR(w_height, wHeight, le16_to_cpu, cpu_to_le16, 16);
UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, le32_to_cpu, cpu_to_le32, 32);
@@ -1137,6 +1139,7 @@ UVC_ATTR(uvcg_frame_, dw_frame_interval, dwFrameInterval);
static struct configfs_attribute *uvcg_frame_attrs[] = {
&uvcg_frame_attr_bm_capabilities,
+ &uvcg_frame_attr_b_frame_index,
&uvcg_frame_attr_w_width,
&uvcg_frame_attr_w_height,
&uvcg_frame_attr_dw_min_bit_rate,
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-21 10:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21 9:45 usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first Laurent Pinchart
-- strict thread matches above, loose matches on Subject: below --
2018-03-21 10:01 Laurent Pinchart
2018-03-20 20:28 Joel Pepper
2018-03-19 15:22 Greg Kroah-Hartman
2018-03-19 10:55 Joel Pepper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.