diff mbox series

[13/16] media: vimc: sen: Add support for multiplanar formats

Message ID 20190315164359.626-14-andrealmeid@collabora.com (mailing list archive)
State New, archived
Headers show
Series media: vimc: Add support for multiplanar formats | expand

Commit Message

André Almeida March 15, 2019, 4:43 p.m. UTC 
This commit adapts vimc-sensor to handle multiplanar pixel formats,
adapting the memory allocation and TPG configuration.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/vimc-sensor.c | 48 +++++++++++++----------
 1 file changed, 28 insertions(+), 20 deletions(-)

Comments

Helen Mae Koike Fornazier March 15, 2019, 7:32 p.m. UTC | #1 
On 3/15/19 1:43 PM, André Almeida wrote:
> This commit adapts vimc-sensor to handle multiplanar pixel formats,
> adapting the memory allocation and TPG configuration.
> 
> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> ---
>  drivers/media/platform/vimc/vimc-sensor.c | 48 +++++++++++++----------
>  1 file changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index 020651320ac9..33cbe2cd42ee 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -97,16 +97,16 @@ static int vimc_sen_get_fmt(struct v4l2_subdev *sd,
>  static void vimc_sen_tpg_s_format(struct vimc_sen_device *vsen)
>  {
>  	u32 pixelformat = vsen->ved.stream->producer_pixfmt;
> -	const struct v4l2_format_info *pix_info;
> -
> -	pix_info = v4l2_format_info(pixelformat);
> +	unsigned int i;
>  
> +	tpg_s_fourcc(&vsen->tpg, pixelformat);
>  	tpg_reset_source(&vsen->tpg, vsen->mbus_format.width,
>  			 vsen->mbus_format.height, vsen->mbus_format.field);
> -	tpg_s_bytesperline(&vsen->tpg, 0,
> -			   vsen->mbus_format.width * pix_info->bpp[0]);
>  	tpg_s_buf_height(&vsen->tpg, vsen->mbus_format.height);
> -	tpg_s_fourcc(&vsen->tpg, pixelformat);

You don't need to move this line to the top.

> +
> +	for (i = 0; i < tpg_g_planes(&vsen->tpg); i++)
> +		tpg_s_bytesperline(&vsen->tpg, i, vsen->frame.bytesperline[i]);
> +
>  	/* TODO: add support for V4L2_FIELD_ALTERNATE */
>  	tpg_s_field(&vsen->tpg, vsen->mbus_format.field, false);
>  	tpg_s_colorspace(&vsen->tpg, vsen->mbus_format.colorspace);
> @@ -182,8 +182,12 @@ static struct vimc_frame *vimc_sen_process_frame(struct vimc_ent_device *ved,
>  {
>  	struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device,
>  						    ved);
> +	unsigned int i;
> +
> +	for (i = 0; i < tpg_g_planes(&vsen->tpg); i++)
> +		tpg_fill_plane_buffer(&vsen->tpg, 0, i,
> +					vsen->frame.plane_addr[i]);

alignment

>  
> -	tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame.plane_addr[0]);
>  	return &vsen->frame;
>  }
>  
> @@ -191,32 +195,36 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
>  {
>  	struct vimc_sen_device *vsen =
>  				container_of(sd, struct vimc_sen_device, sd);
> +	unsigned int i;
>  
>  	if (enable) {
> -		u32 pixelformat = vsen->ved.stream->producer_pixfmt;
> -		const struct v4l2_format_info *pix_info;
> -		unsigned int frame_size;
> -
>  		/* Calculate the frame size */
> -		pix_info = v4l2_format_info(pixelformat);
> -		frame_size = vsen->mbus_format.width * pix_info->bpp[0] *
> -			     vsen->mbus_format.height;
> -
> +		vimc_fill_frame(&vsen->frame, vsen->ved.stream->producer_pixfmt,
> +				vsen->mbus_format.width,
> +				vsen->mbus_format.height);
>  		/*
>  		 * Allocate the frame buffer. Use vmalloc to be able to
>  		 * allocate a large amount of memory
>  		 */
> -		vsen->frame.plane_addr[0] = vmalloc(frame_size);
> -		if (!vsen->frame.plane_addr[0])
> -			return -ENOMEM;
> +		for (i = 0; i < vsen->frame.num_planes; i++) {
> +			vsen->frame.plane_addr[i] =
> +				vmalloc(vsen->frame.sizeimage[i]);
> +			if (!vsen->frame.plane_addr[i]) {
> +				for (i -= 1; i >= 0; i--)
> +					vfree(vsen->frame.plane_addr[i]);
> +				return -ENOMEM;

This code is really simiar to the else block below.
I think you can use a goto here, e.g.:

			if (!vsen->frame.plane_addr[i]) {
				ret = -ENOMEM
				goto free_planes
			}
  		/* configure the test pattern generator */
  		vimc_sen_tpg_s_format(vsen);
		return 0;
	}

	i = vsen->frame.num_planes; // this can be initialized when i is
declared too

free_planes:
	for (i = 0; i < vsen->frame.num_planes; i++) {
		vfree(vsen->frame.plane_addr[i]);
		vsen->frame.plane_addr[i] = NULL;
	}
	return ret;
}


what do you think?
this also applies to the scaler.

Regards,
Helen

> +			}
> +		}
>  
>  		/* configure the test pattern generator */
>  		vimc_sen_tpg_s_format(vsen);
>  
>  	} else {
> +		for (i = 0; i < vsen->frame.num_planes; i++) {
> +			vfree(vsen->frame.plane_addr[i]);
> +			vsen->frame.plane_addr[i] = NULL;
> +		}
>  
> -		vfree(vsen->frame.plane_addr[0]);
> -		vsen->frame.plane_addr[0] = NULL;
>  		return 0;
>  	}
>  
>
diff mbox series

Patch

diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 020651320ac9..33cbe2cd42ee 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -97,16 +97,16 @@  static int vimc_sen_get_fmt(struct v4l2_subdev *sd,
 static void vimc_sen_tpg_s_format(struct vimc_sen_device *vsen)
 {
 	u32 pixelformat = vsen->ved.stream->producer_pixfmt;
-	const struct v4l2_format_info *pix_info;
-
-	pix_info = v4l2_format_info(pixelformat);
+	unsigned int i;
 
+	tpg_s_fourcc(&vsen->tpg, pixelformat);
 	tpg_reset_source(&vsen->tpg, vsen->mbus_format.width,
 			 vsen->mbus_format.height, vsen->mbus_format.field);
-	tpg_s_bytesperline(&vsen->tpg, 0,
-			   vsen->mbus_format.width * pix_info->bpp[0]);
 	tpg_s_buf_height(&vsen->tpg, vsen->mbus_format.height);
-	tpg_s_fourcc(&vsen->tpg, pixelformat);
+
+	for (i = 0; i < tpg_g_planes(&vsen->tpg); i++)
+		tpg_s_bytesperline(&vsen->tpg, i, vsen->frame.bytesperline[i]);
+
 	/* TODO: add support for V4L2_FIELD_ALTERNATE */
 	tpg_s_field(&vsen->tpg, vsen->mbus_format.field, false);
 	tpg_s_colorspace(&vsen->tpg, vsen->mbus_format.colorspace);
@@ -182,8 +182,12 @@  static struct vimc_frame *vimc_sen_process_frame(struct vimc_ent_device *ved,
 {
 	struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device,
 						    ved);
+	unsigned int i;
+
+	for (i = 0; i < tpg_g_planes(&vsen->tpg); i++)
+		tpg_fill_plane_buffer(&vsen->tpg, 0, i,
+					vsen->frame.plane_addr[i]);
 
-	tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame.plane_addr[0]);
 	return &vsen->frame;
 }
 
@@ -191,32 +195,36 @@  static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct vimc_sen_device *vsen =
 				container_of(sd, struct vimc_sen_device, sd);
+	unsigned int i;
 
 	if (enable) {
-		u32 pixelformat = vsen->ved.stream->producer_pixfmt;
-		const struct v4l2_format_info *pix_info;
-		unsigned int frame_size;
-
 		/* Calculate the frame size */
-		pix_info = v4l2_format_info(pixelformat);
-		frame_size = vsen->mbus_format.width * pix_info->bpp[0] *
-			     vsen->mbus_format.height;
-
+		vimc_fill_frame(&vsen->frame, vsen->ved.stream->producer_pixfmt,
+				vsen->mbus_format.width,
+				vsen->mbus_format.height);
 		/*
 		 * Allocate the frame buffer. Use vmalloc to be able to
 		 * allocate a large amount of memory
 		 */
-		vsen->frame.plane_addr[0] = vmalloc(frame_size);
-		if (!vsen->frame.plane_addr[0])
-			return -ENOMEM;
+		for (i = 0; i < vsen->frame.num_planes; i++) {
+			vsen->frame.plane_addr[i] =
+				vmalloc(vsen->frame.sizeimage[i]);
+			if (!vsen->frame.plane_addr[i]) {
+				for (i -= 1; i >= 0; i--)
+					vfree(vsen->frame.plane_addr[i]);
+				return -ENOMEM;
+			}
+		}
 
 		/* configure the test pattern generator */
 		vimc_sen_tpg_s_format(vsen);
 
 	} else {
+		for (i = 0; i < vsen->frame.num_planes; i++) {
+			vfree(vsen->frame.plane_addr[i]);
+			vsen->frame.plane_addr[i] = NULL;
+		}
 
-		vfree(vsen->frame.plane_addr[0]);
-		vsen->frame.plane_addr[0] = NULL;
 		return 0;
 	}