diff mbox series

[v7,13/49] media: verisilicon: Refactor postprocessor to store more buffers

Message ID 20230914133323.198857-14-benjamin.gaignard@collabora.com
State Superseded
Headers show
Series [v7,01/49] media: videobuf2: Rework offset 'cookie' encoding pattern | expand

Commit Message

Benjamin Gaignard Sept. 14, 2023, 1:32 p.m. UTC
Since vb2 queue can store than VB2_MAX_FRAME buffers postprocessor
buffer storage must be capable to store more buffers too.
Change static dec_q array to allocated array to be capable to store
up to queue 'max_allowed_buffers'.
Keep allocating queue 'num_buffers' at queue setup time but also allows
to allocate postprocessors buffers on the fly.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 drivers/media/platform/verisilicon/hantro.h   |  7 +-
 .../media/platform/verisilicon/hantro_drv.c   |  4 +-
 .../media/platform/verisilicon/hantro_hw.h    |  4 +-
 .../platform/verisilicon/hantro_postproc.c    | 93 +++++++++++++++----
 .../media/platform/verisilicon/hantro_v4l2.c  |  2 +-
 5 files changed, 85 insertions(+), 25 deletions(-)

Comments

Hans Verkuil Sept. 19, 2023, 10:57 a.m. UTC | #1
On 14/09/2023 15:32, Benjamin Gaignard wrote:
> Since vb2 queue can store than VB2_MAX_FRAME buffers postprocessor

Change to:

Since vb2 queue can store more than VB2_MAX_FRAME buffers, the postprocessor

Regards,

	Hans

> buffer storage must be capable to store more buffers too.
> Change static dec_q array to allocated array to be capable to store
> up to queue 'max_allowed_buffers'.
> Keep allocating queue 'num_buffers' at queue setup time but also allows
> to allocate postprocessors buffers on the fly.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
>  drivers/media/platform/verisilicon/hantro.h   |  7 +-
>  .../media/platform/verisilicon/hantro_drv.c   |  4 +-
>  .../media/platform/verisilicon/hantro_hw.h    |  4 +-
>  .../platform/verisilicon/hantro_postproc.c    | 93 +++++++++++++++----
>  .../media/platform/verisilicon/hantro_v4l2.c  |  2 +-
>  5 files changed, 85 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> index 77aee9489516..0948b04a9f8d 100644
> --- a/drivers/media/platform/verisilicon/hantro.h
> +++ b/drivers/media/platform/verisilicon/hantro.h
> @@ -469,11 +469,14 @@ hantro_get_dst_buf(struct hantro_ctx *ctx)
>  bool hantro_needs_postproc(const struct hantro_ctx *ctx,
>  			   const struct hantro_fmt *fmt);
>  
> +dma_addr_t
> +hantro_postproc_get_dec_buf_addr(struct hantro_ctx *ctx, int index);
> +
>  static inline dma_addr_t
>  hantro_get_dec_buf_addr(struct hantro_ctx *ctx, struct vb2_buffer *vb)
>  {
>  	if (hantro_needs_postproc(ctx, ctx->vpu_dst_fmt))
> -		return ctx->postproc.dec_q[vb->index].dma;
> +		return hantro_postproc_get_dec_buf_addr(ctx, vb->index);
>  	return vb2_dma_contig_plane_dma_addr(vb, 0);
>  }
>  
> @@ -485,8 +488,8 @@ vb2_to_hantro_decoded_buf(struct vb2_buffer *buf)
>  
>  void hantro_postproc_disable(struct hantro_ctx *ctx);
>  void hantro_postproc_enable(struct hantro_ctx *ctx);
> +int hantro_postproc_init(struct hantro_ctx *ctx);
>  void hantro_postproc_free(struct hantro_ctx *ctx);
> -int hantro_postproc_alloc(struct hantro_ctx *ctx);
>  int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
>  				   struct v4l2_frmsizeenum *fsize);
>  
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 423fc85d79ee..18f56edee3fc 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -234,8 +234,10 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>  	 * The Kernel needs access to the JPEG destination buffer for the
>  	 * JPEG encoder to fill in the JPEG headers.
>  	 */
> -	if (!ctx->is_encoder)
> +	if (!ctx->is_encoder) {
>  		dst_vq->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> +		dst_vq->max_allowed_buffers = MAX_POSTPROC_BUFFERS;
> +	}
>  
>  	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>  	dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
> index 7f33f7b07ce4..292a76ef643e 100644
> --- a/drivers/media/platform/verisilicon/hantro_hw.h
> +++ b/drivers/media/platform/verisilicon/hantro_hw.h
> @@ -40,6 +40,8 @@
>  
>  #define AV1_MAX_FRAME_BUF_COUNT	(V4L2_AV1_TOTAL_REFS_PER_FRAME + 1)
>  
> +#define MAX_POSTPROC_BUFFERS	64
> +
>  struct hantro_dev;
>  struct hantro_ctx;
>  struct hantro_buf;
> @@ -336,7 +338,7 @@ struct hantro_av1_dec_hw_ctx {
>   * @dec_q:		References buffers, in decoder format.
>   */
>  struct hantro_postproc_ctx {
> -	struct hantro_aux_buf dec_q[VB2_MAX_FRAME];
> +	struct hantro_aux_buf dec_q[MAX_POSTPROC_BUFFERS];
>  };
>  
>  /**
> diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
> index 0224ff68ab3f..e624cd98f41b 100644
> --- a/drivers/media/platform/verisilicon/hantro_postproc.c
> +++ b/drivers/media/platform/verisilicon/hantro_postproc.c
> @@ -177,9 +177,11 @@ static int hantro_postproc_g2_enum_framesizes(struct hantro_ctx *ctx,
>  void hantro_postproc_free(struct hantro_ctx *ctx)
>  {
>  	struct hantro_dev *vpu = ctx->dev;
> +	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> +	struct vb2_queue *queue = &m2m_ctx->cap_q_ctx.q;
>  	unsigned int i;
>  
> -	for (i = 0; i < VB2_MAX_FRAME; ++i) {
> +	for (i = 0; i < queue->max_allowed_buffers; ++i) {
>  		struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];
>  
>  		if (priv->cpu) {
> @@ -190,20 +192,17 @@ void hantro_postproc_free(struct hantro_ctx *ctx)
>  	}
>  }
>  
> -int hantro_postproc_alloc(struct hantro_ctx *ctx)
> +static unsigned int hantro_postproc_buffer_size(struct hantro_ctx *ctx)
>  {
> -	struct hantro_dev *vpu = ctx->dev;
> -	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> -	struct vb2_queue *cap_queue = &m2m_ctx->cap_q_ctx.q;
> -	unsigned int num_buffers = cap_queue->num_buffers;
>  	struct v4l2_pix_format_mplane pix_mp;
>  	const struct hantro_fmt *fmt;
> -	unsigned int i, buf_size;
> +	unsigned int buf_size;
>  
>  	/* this should always pick native format */
>  	fmt = hantro_get_default_fmt(ctx, false, ctx->bit_depth, HANTRO_AUTO_POSTPROC);
>  	if (!fmt)
> -		return -EINVAL;
> +		return 0;
> +
>  	v4l2_fill_pixfmt_mp(&pix_mp, fmt->fourcc, ctx->src_fmt.width,
>  			    ctx->src_fmt.height);
>  
> @@ -221,23 +220,77 @@ int hantro_postproc_alloc(struct hantro_ctx *ctx)
>  		buf_size += hantro_av1_mv_size(pix_mp.width,
>  					       pix_mp.height);
>  
> -	for (i = 0; i < num_buffers; ++i) {
> -		struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];
> +	return buf_size;
> +}
> +
> +static int hantro_postproc_alloc(struct hantro_ctx *ctx, int index)
> +{
> +	struct hantro_dev *vpu = ctx->dev;
> +	struct hantro_aux_buf *priv = &ctx->postproc.dec_q[index];
> +	unsigned int buf_size = hantro_postproc_buffer_size(ctx);
> +
> +	if (!buf_size)
> +		return -EINVAL;
> +
> +	/*
> +	 * The buffers on this queue are meant as intermediate
> +	 * buffers for the decoder, so no mapping is needed.
> +	 */
> +	priv->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
> +	priv->cpu = dma_alloc_attrs(vpu->dev, buf_size, &priv->dma,
> +				    GFP_KERNEL, priv->attrs);
> +	if (!priv->cpu)
> +		return -ENOMEM;
> +	priv->size = buf_size;
> +
> +	return 0;
> +}
>  
> -		/*
> -		 * The buffers on this queue are meant as intermediate
> -		 * buffers for the decoder, so no mapping is needed.
> -		 */
> -		priv->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
> -		priv->cpu = dma_alloc_attrs(vpu->dev, buf_size, &priv->dma,
> -					    GFP_KERNEL, priv->attrs);
> -		if (!priv->cpu)
> -			return -ENOMEM;
> -		priv->size = buf_size;
> +int hantro_postproc_init(struct hantro_ctx *ctx)
> +{
> +	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> +	struct vb2_queue *cap_queue = &m2m_ctx->cap_q_ctx.q;
> +	unsigned int num_buffers = cap_queue->num_buffers;
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < num_buffers; i++) {
> +		ret = hantro_postproc_alloc(ctx, i);
> +		if (ret)
> +			return ret;
>  	}
> +
>  	return 0;
>  }
>  
> +dma_addr_t
> +hantro_postproc_get_dec_buf_addr(struct hantro_ctx *ctx, int index)
> +{
> +	struct hantro_aux_buf *priv = &ctx->postproc.dec_q[index];
> +	unsigned int buf_size = hantro_postproc_buffer_size(ctx);
> +	struct hantro_dev *vpu = ctx->dev;
> +	int ret;
> +
> +	if (priv->size < buf_size && priv->cpu) {
> +		/* buffer is too small, release it */
> +		dma_free_attrs(vpu->dev, priv->size, priv->cpu,
> +			       priv->dma, priv->attrs);
> +		priv->cpu = NULL;
> +	}
> +
> +	if (!priv->cpu) {
> +		/* buffer not already allocated, try getting a new one */
> +		ret = hantro_postproc_alloc(ctx, index);
> +		if (ret)
> +			return 0;
> +	}
> +
> +	if (!priv->cpu)
> +		return 0;
> +
> +	return priv->dma;
> +}
> +
>  static void hantro_postproc_g1_disable(struct hantro_ctx *ctx)
>  {
>  	struct hantro_dev *vpu = ctx->dev;
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index b3ae037a50f6..f0d8b165abcd 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -933,7 +933,7 @@ static int hantro_start_streaming(struct vb2_queue *q, unsigned int count)
>  		}
>  
>  		if (hantro_needs_postproc(ctx, ctx->vpu_dst_fmt)) {
> -			ret = hantro_postproc_alloc(ctx);
> +			ret = hantro_postproc_init(ctx);
>  			if (ret)
>  				goto err_codec_exit;
>  		}
diff mbox series

Patch

diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
index 77aee9489516..0948b04a9f8d 100644
--- a/drivers/media/platform/verisilicon/hantro.h
+++ b/drivers/media/platform/verisilicon/hantro.h
@@ -469,11 +469,14 @@  hantro_get_dst_buf(struct hantro_ctx *ctx)
 bool hantro_needs_postproc(const struct hantro_ctx *ctx,
 			   const struct hantro_fmt *fmt);
 
+dma_addr_t
+hantro_postproc_get_dec_buf_addr(struct hantro_ctx *ctx, int index);
+
 static inline dma_addr_t
 hantro_get_dec_buf_addr(struct hantro_ctx *ctx, struct vb2_buffer *vb)
 {
 	if (hantro_needs_postproc(ctx, ctx->vpu_dst_fmt))
-		return ctx->postproc.dec_q[vb->index].dma;
+		return hantro_postproc_get_dec_buf_addr(ctx, vb->index);
 	return vb2_dma_contig_plane_dma_addr(vb, 0);
 }
 
@@ -485,8 +488,8 @@  vb2_to_hantro_decoded_buf(struct vb2_buffer *buf)
 
 void hantro_postproc_disable(struct hantro_ctx *ctx);
 void hantro_postproc_enable(struct hantro_ctx *ctx);
+int hantro_postproc_init(struct hantro_ctx *ctx);
 void hantro_postproc_free(struct hantro_ctx *ctx);
-int hantro_postproc_alloc(struct hantro_ctx *ctx);
 int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
 				   struct v4l2_frmsizeenum *fsize);
 
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index 423fc85d79ee..18f56edee3fc 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -234,8 +234,10 @@  queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 	 * The Kernel needs access to the JPEG destination buffer for the
 	 * JPEG encoder to fill in the JPEG headers.
 	 */
-	if (!ctx->is_encoder)
+	if (!ctx->is_encoder) {
 		dst_vq->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
+		dst_vq->max_allowed_buffers = MAX_POSTPROC_BUFFERS;
+	}
 
 	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
 	dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
index 7f33f7b07ce4..292a76ef643e 100644
--- a/drivers/media/platform/verisilicon/hantro_hw.h
+++ b/drivers/media/platform/verisilicon/hantro_hw.h
@@ -40,6 +40,8 @@ 
 
 #define AV1_MAX_FRAME_BUF_COUNT	(V4L2_AV1_TOTAL_REFS_PER_FRAME + 1)
 
+#define MAX_POSTPROC_BUFFERS	64
+
 struct hantro_dev;
 struct hantro_ctx;
 struct hantro_buf;
@@ -336,7 +338,7 @@  struct hantro_av1_dec_hw_ctx {
  * @dec_q:		References buffers, in decoder format.
  */
 struct hantro_postproc_ctx {
-	struct hantro_aux_buf dec_q[VB2_MAX_FRAME];
+	struct hantro_aux_buf dec_q[MAX_POSTPROC_BUFFERS];
 };
 
 /**
diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
index 0224ff68ab3f..e624cd98f41b 100644
--- a/drivers/media/platform/verisilicon/hantro_postproc.c
+++ b/drivers/media/platform/verisilicon/hantro_postproc.c
@@ -177,9 +177,11 @@  static int hantro_postproc_g2_enum_framesizes(struct hantro_ctx *ctx,
 void hantro_postproc_free(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
+	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
+	struct vb2_queue *queue = &m2m_ctx->cap_q_ctx.q;
 	unsigned int i;
 
-	for (i = 0; i < VB2_MAX_FRAME; ++i) {
+	for (i = 0; i < queue->max_allowed_buffers; ++i) {
 		struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];
 
 		if (priv->cpu) {
@@ -190,20 +192,17 @@  void hantro_postproc_free(struct hantro_ctx *ctx)
 	}
 }
 
-int hantro_postproc_alloc(struct hantro_ctx *ctx)
+static unsigned int hantro_postproc_buffer_size(struct hantro_ctx *ctx)
 {
-	struct hantro_dev *vpu = ctx->dev;
-	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
-	struct vb2_queue *cap_queue = &m2m_ctx->cap_q_ctx.q;
-	unsigned int num_buffers = cap_queue->num_buffers;
 	struct v4l2_pix_format_mplane pix_mp;
 	const struct hantro_fmt *fmt;
-	unsigned int i, buf_size;
+	unsigned int buf_size;
 
 	/* this should always pick native format */
 	fmt = hantro_get_default_fmt(ctx, false, ctx->bit_depth, HANTRO_AUTO_POSTPROC);
 	if (!fmt)
-		return -EINVAL;
+		return 0;
+
 	v4l2_fill_pixfmt_mp(&pix_mp, fmt->fourcc, ctx->src_fmt.width,
 			    ctx->src_fmt.height);
 
@@ -221,23 +220,77 @@  int hantro_postproc_alloc(struct hantro_ctx *ctx)
 		buf_size += hantro_av1_mv_size(pix_mp.width,
 					       pix_mp.height);
 
-	for (i = 0; i < num_buffers; ++i) {
-		struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];
+	return buf_size;
+}
+
+static int hantro_postproc_alloc(struct hantro_ctx *ctx, int index)
+{
+	struct hantro_dev *vpu = ctx->dev;
+	struct hantro_aux_buf *priv = &ctx->postproc.dec_q[index];
+	unsigned int buf_size = hantro_postproc_buffer_size(ctx);
+
+	if (!buf_size)
+		return -EINVAL;
+
+	/*
+	 * The buffers on this queue are meant as intermediate
+	 * buffers for the decoder, so no mapping is needed.
+	 */
+	priv->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
+	priv->cpu = dma_alloc_attrs(vpu->dev, buf_size, &priv->dma,
+				    GFP_KERNEL, priv->attrs);
+	if (!priv->cpu)
+		return -ENOMEM;
+	priv->size = buf_size;
+
+	return 0;
+}
 
-		/*
-		 * The buffers on this queue are meant as intermediate
-		 * buffers for the decoder, so no mapping is needed.
-		 */
-		priv->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
-		priv->cpu = dma_alloc_attrs(vpu->dev, buf_size, &priv->dma,
-					    GFP_KERNEL, priv->attrs);
-		if (!priv->cpu)
-			return -ENOMEM;
-		priv->size = buf_size;
+int hantro_postproc_init(struct hantro_ctx *ctx)
+{
+	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
+	struct vb2_queue *cap_queue = &m2m_ctx->cap_q_ctx.q;
+	unsigned int num_buffers = cap_queue->num_buffers;
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_buffers; i++) {
+		ret = hantro_postproc_alloc(ctx, i);
+		if (ret)
+			return ret;
 	}
+
 	return 0;
 }
 
+dma_addr_t
+hantro_postproc_get_dec_buf_addr(struct hantro_ctx *ctx, int index)
+{
+	struct hantro_aux_buf *priv = &ctx->postproc.dec_q[index];
+	unsigned int buf_size = hantro_postproc_buffer_size(ctx);
+	struct hantro_dev *vpu = ctx->dev;
+	int ret;
+
+	if (priv->size < buf_size && priv->cpu) {
+		/* buffer is too small, release it */
+		dma_free_attrs(vpu->dev, priv->size, priv->cpu,
+			       priv->dma, priv->attrs);
+		priv->cpu = NULL;
+	}
+
+	if (!priv->cpu) {
+		/* buffer not already allocated, try getting a new one */
+		ret = hantro_postproc_alloc(ctx, index);
+		if (ret)
+			return 0;
+	}
+
+	if (!priv->cpu)
+		return 0;
+
+	return priv->dma;
+}
+
 static void hantro_postproc_g1_disable(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index b3ae037a50f6..f0d8b165abcd 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -933,7 +933,7 @@  static int hantro_start_streaming(struct vb2_queue *q, unsigned int count)
 		}
 
 		if (hantro_needs_postproc(ctx, ctx->vpu_dst_fmt)) {
-			ret = hantro_postproc_alloc(ctx);
+			ret = hantro_postproc_init(ctx);
 			if (ret)
 				goto err_codec_exit;
 		}