Hi Ricardo,
On Mon, May 01, 2023 at 03:36:22PM +0200, Ricardo Ribalda wrote:
> On Thu, 20 Apr 2023 at 12:09, Laurent Pinchart wrote:
> >
> > The uvc_streaming 'format' field points to an array of formats. Rename
> > it to 'formats' to make this clearer.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
>
> > ---
> > drivers/media/usb/uvc/uvc_driver.c | 4 ++--
> > drivers/media/usb/uvc/uvc_v4l2.c | 16 ++++++++--------
> > drivers/media/usb/uvc/uvc_video.c | 6 +++---
> > drivers/media/usb/uvc/uvcvideo.h | 2 +-
> > 4 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 25b8199f4e82..77d4403b0b4f 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -184,7 +184,7 @@ static void uvc_stream_delete(struct uvc_streaming *stream)
> >
> > usb_put_intf(stream->intf);
> >
> > - kfree(stream->format);
> > + kfree(stream->formats);
> > kfree(stream->header.bmaControls);
> > kfree(stream);
> > }
> > @@ -677,7 +677,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
> > frame = (struct uvc_frame *)&format[nformats];
> > interval = (u32 *)&frame[nframes];
> >
> > - streaming->format = format;
> > + streaming->formats = format;
> > streaming->nformats = nformats;
>
> Unrelated question:
> For:
> size = nformats * sizeof(*format) + nframes * sizeof(*frame)
> + nintervals * sizeof(*interval);
> frame = (struct uvc_frame *)&format[nformats];
> interval = (u32 *)&frame[nframes];
>
> streaming->format = format;
> streaming->nformats = nformats;
>
> We are very lucky that (*format) (*frame) and (*interval) are cache
> aligned right?
>
> Maybe we should make 3 allocations?
If you measure a noticeable positive impact, I'm fine with that change
:-)
> > /* Parse the format descriptors. */
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> > index 35453f81c1d9..6960d7ebd904 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -235,7 +235,7 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
> > * format otherwise.
> > */
> > for (i = 0; i < stream->nformats; ++i) {
> > - format = &stream->format[i];
> > + format = &stream->formats[i];
> > if (format->fcc == fmt->fmt.pix.pixelformat)
> > break;
> > }
> > @@ -319,8 +319,8 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
> > * accepted the requested format as-is.
> > */
> > for (i = 0; i < stream->nformats; ++i) {
> > - if (probe->bFormatIndex == stream->format[i].index) {
> > - format = &stream->format[i];
> > + if (probe->bFormatIndex == stream->formats[i].index) {
> > + format = &stream->formats[i];
> > break;
> > }
> > }
> > @@ -708,7 +708,7 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream,
> > fmt->index = index;
> > fmt->type = type;
> >
> > - format = &stream->format[fmt->index];
> > + format = &stream->formats[fmt->index];
> > fmt->flags = 0;
> > if (format->flags & UVC_FMT_FLAG_COMPRESSED)
> > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED;
> > @@ -1256,8 +1256,8 @@ static int uvc_ioctl_enum_framesizes(struct file *file, void *fh,
> >
> > /* Look for the given pixel format */
> > for (i = 0; i < stream->nformats; i++) {
> > - if (stream->format[i].fcc == fsize->pixel_format) {
> > - format = &stream->format[i];
> > + if (stream->formats[i].fcc == fsize->pixel_format) {
> > + format = &stream->formats[i];
> > break;
> > }
> > }
> > @@ -1297,8 +1297,8 @@ static int uvc_ioctl_enum_frameintervals(struct file *file, void *fh,
> >
> > /* Look for the given pixel format and frame size */
> > for (i = 0; i < stream->nformats; i++) {
> > - if (stream->format[i].fcc == fival->pixel_format) {
> > - format = &stream->format[i];
> > + if (stream->formats[i].fcc == fival->pixel_format) {
> > + format = &stream->formats[i];
> > break;
> > }
> > }
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index d4b023d4de7c..af540f435099 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -166,8 +166,8 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream,
> > }
> >
> > for (i = 0; i < stream->nformats; ++i) {
> > - if (stream->format[i].index == ctrl->bFormatIndex) {
> > - format = &stream->format[i];
> > + if (stream->formats[i].index == ctrl->bFormatIndex) {
> > + format = &stream->formats[i];
> > break;
> > }
> > }
> > @@ -2161,7 +2161,7 @@ int uvc_video_init(struct uvc_streaming *stream)
> > * available format otherwise.
> > */
> > for (i = stream->nformats; i > 0; --i) {
> > - format = &stream->format[i-1];
> > + format = &stream->formats[i-1];
> > if (format->index == probe->bFormatIndex)
> > break;
> > }
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index 50f171e7381b..9c8bea8d405c 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -438,7 +438,7 @@ struct uvc_streaming {
> > enum v4l2_buf_type type;
> >
> > unsigned int nformats;
> > - struct uvc_format *format;
> > + struct uvc_format *formats;
> >
> > struct uvc_streaming_control ctrl;
> > struct uvc_format *def_format;
--
Regards,
Laurent Pinchart
