diff mbox series

[v1,1/4] media: uvcvideo: Rename uvc_streaming 'format' field to 'formats'

Message ID 20230420100958.10631-2-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series media: uvcvideo: Misc cleanups | expand

Commit Message

Laurent Pinchart April 20, 2023, 10:09 a.m. UTC
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@ideasonboard.com>
---
 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(-)

Comments

Ricardo Ribalda May 1, 2023, 1:36 p.m. UTC | #1
Hi Laurent

On Thu, 20 Apr 2023 at 12:09, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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@ideasonboard.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  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?


>
>         /* 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
>
Laurent Pinchart May 1, 2023, 1:51 p.m. UTC | #2
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@ideasonboard.com>
>
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
>
> > ---
> >  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;
diff mbox series

Patch

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;
 
 	/* 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;