Re: [PATCH v4 13/45] media: sun6i-csi: Introduce and use video helper functions

Jernej Škrabec
2022-04-27T19:01:57Z

Dne petek, 15. april 2022 ob 17:27:39 CEST je Paul Kocialkowski napisal(a):
> Introduce some helpers for buffer and general video configuration.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> .../platform/sunxi/sun6i-csi/sun6i_video.c | 46 +++++++++++--------
> 1 file changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c index
> e6c85fcc65bb..e47eeb27dc4e 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> @@ -92,6 +92,29 @@ static bool sun6i_video_format_check(u32 format)
> return false;
> }
>
> +/* Video */
> +
> +static void sun6i_video_buffer_configure(struct sun6i_csi_device *csi_dev,
> + struct sun6i_csi_buffer
*csi_buffer)
> +{
> + csi_buffer->queued_to_csi = true;
> + sun6i_csi_update_buf_addr(csi_dev, csi_buffer->dma_addr);
> +}
> +
> +static void sun6i_video_configure(struct sun6i_csi_device *csi_dev)
> +{
> + struct sun6i_video *video = &csi_dev->video;
> + struct sun6i_csi_config config = { 0 };
> +
> + config.pixelformat = video->format.fmt.pix.pixelformat;
> + config.code = video->mbus_code;
> + config.field = video->format.fmt.pix.field;
> + config.width = video->format.fmt.pix.width;
> + config.height = video->format.fmt.pix.height;
> +
> + sun6i_csi_update_config(csi_dev, &config);
> +}
> +
> /* Queue */
>
> static int sun6i_video_queue_setup(struct vb2_queue *queue,
> @@ -160,7 +183,6 @@ static int sun6i_video_start_streaming(struct vb2_queue
> *queue, struct video_device *video_dev = &video->video_dev;
> struct sun6i_csi_buffer *buf;
> struct sun6i_csi_buffer *next_buf;
> - struct sun6i_csi_config config;
> struct v4l2_subdev *subdev;
> unsigned long flags;
> int ret;
> @@ -182,22 +204,13 @@ static int sun6i_video_start_streaming(struct
> vb2_queue *queue, goto error_media_pipeline;
> }
>
> - config.pixelformat = video->format.fmt.pix.pixelformat;
> - config.code = video->mbus_code;
> - config.field = video->format.fmt.pix.field;
> - config.width = video->format.fmt.pix.width;
> - config.height = video->format.fmt.pix.height;
> -
> - ret = sun6i_csi_update_config(csi_dev, &config);
> - if (ret < 0)
> - goto error_media_pipeline;
> + sun6i_video_configure(csi_dev);

What happened to that error handling? New helper function ignores return value
of sun6i_csi_update_config(). Why?

Best regards,
Jernej

>
> spin_lock_irqsave(&video->dma_queue_lock, flags);
>
> buf = list_first_entry(&video->dma_queue,
> struct sun6i_csi_buffer, list);
> - buf->queued_to_csi = true;
> - sun6i_csi_update_buf_addr(csi_dev, buf->dma_addr);
> + sun6i_video_buffer_configure(csi_dev, buf);
>
> sun6i_csi_set_stream(csi_dev, true);
>
> @@ -219,8 +232,7 @@ static int sun6i_video_start_streaming(struct vb2_queue
> *queue, * would also drop frame when lacking of queued buffer.
> */
> next_buf = list_next_entry(buf, list);
> - next_buf->queued_to_csi = true;
> - sun6i_csi_update_buf_addr(csi_dev, next_buf->dma_addr);
> + sun6i_video_buffer_configure(csi_dev, next_buf);
>
> spin_unlock_irqrestore(&video->dma_queue_lock, flags);
>
> @@ -294,8 +306,7 @@ void sun6i_video_frame_done(struct sun6i_csi_device
> *csi_dev) * for next ISR call.
> */
> if (!next_buf->queued_to_csi) {
> - next_buf->queued_to_csi = true;
> - sun6i_csi_update_buf_addr(csi_dev, next_buf->dma_addr);
> + sun6i_video_buffer_configure(csi_dev, next_buf);
> dev_dbg(csi_dev->dev, "Frame dropped!\n");
> goto complete;
> }
> @@ -309,8 +320,7 @@ void sun6i_video_frame_done(struct sun6i_csi_device
> *csi_dev) /* Prepare buffer for next frame but one. */
> if (!list_is_last(&next_buf->list, &video->dma_queue)) {
> next_buf = list_next_entry(next_buf, list);
> - next_buf->queued_to_csi = true;
> - sun6i_csi_update_buf_addr(csi_dev, next_buf->dma_addr);
> + sun6i_video_buffer_configure(csi_dev, next_buf);
> } else {
> dev_dbg(csi_dev->dev, "Next frame will be dropped!\n");
> }