[PATCH v2 0/2] hantro: postproc related fixes

[PATCH v2 0/2] hantro: postproc related fixes

[Ezequiel Garcia]

Doing some more extended testing, we found a few
issues when the post-processor is enabled (currently
for color conversion).

The two patches below should fix these issues.

Changes from v1:

* Undo the change to the comment about the
  MV and MC layout. As pointed out by Jonas,
  the layout is not generic, and depends on the
  version of the decoder core. For now, let's
  leave it as is.

Ezequiel Garcia (2):
  hantro: h264: Get the correct fallback reference buffer
  hantro: postproc: Fix motion vector space allocation

 drivers/staging/media/hantro/hantro_h264.c     | 2 +-
 drivers/staging/media/hantro/hantro_postproc.c | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.27.0

[PATCH v2 1/2] hantro: h264: Get the correct fallback reference buffer

[Ezequiel Garcia]

If the bitstream and the application are incorrectly configuring
the reference pictures, the hardware will need to fallback
to using some other reference picture.

When the post-processor is enabled, the fallback buffer
should be a shadow buffer (postproc.dec_q), and not a
CAPTURE queue buffer, since the latter is post-processed
and not really the output of the decoder core.

Fixes: 8c2d66b036c77 ("media: hantro: Support color conversion via post-processing")
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro_h264.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/hantro/hantro_h264.c b/drivers/staging/media/hantro/hantro_h264.c
index 194d05848077..6dcd47bd9ed3 100644
--- a/drivers/staging/media/hantro/hantro_h264.c
+++ b/drivers/staging/media/hantro/hantro_h264.c
@@ -325,7 +325,7 @@ dma_addr_t hantro_h264_get_ref_buf(struct hantro_ctx *ctx,
 		 */
 		dst_buf = hantro_get_dst_buf(ctx);
 		buf = &dst_buf->vb2_buf;
-		dma_addr = vb2_dma_contig_plane_dma_addr(buf, 0);
+		dma_addr = hantro_get_dec_buf_addr(ctx, buf);
 	}
 
 	return dma_addr;
-- 
2.27.0

[PATCH v2 2/2] hantro: postproc: Fix motion vector space allocation

[Ezequiel Garcia]

When the post-processor is enabled, the driver allocates
"shadow buffers" which are used for the decoder core,
and exposes the post-processed buffers to userspace.

For this reason, extra motion vector space has to
be allocated on the shadow buffers, which the driver
wasn't doing. Fix it.

This fix should address artifacts on high profile bitstreams.

Fixes: 8c2d66b036c77 ("media: hantro: Support color conversion via post-processing")
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro_postproc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
index 44062ffceaea..6d2a8f2a8f0b 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -118,7 +118,9 @@ int hantro_postproc_alloc(struct hantro_ctx *ctx)
 	unsigned int num_buffers = cap_queue->num_buffers;
 	unsigned int i, buf_size;
 
-	buf_size = ctx->dst_fmt.plane_fmt[0].sizeimage;
+	buf_size = ctx->dst_fmt.plane_fmt[0].sizeimage +
+		   hantro_h264_mv_size(ctx->dst_fmt.width,
+				       ctx->dst_fmt.height);
 
 	for (i = 0; i < num_buffers; ++i) {
 		struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];
-- 
2.27.0

Re: [PATCH v2 1/2] hantro: h264: Get the correct fallback reference buffer

[Philipp Zabel]

On Mon, 2020-07-27 at 14:05 -0300, Ezequiel Garcia wrote:
> If the bitstream and the application are incorrectly configuring
> the reference pictures, the hardware will need to fallback
> to using some other reference picture.
> 
> When the post-processor is enabled, the fallback buffer
> should be a shadow buffer (postproc.dec_q), and not a
> CAPTURE queue buffer, since the latter is post-processed
> and not really the output of the decoder core.
> 
> Fixes: 8c2d66b036c77 ("media: hantro: Support color conversion via post-processing")
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/staging/media/hantro/hantro_h264.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_h264.c b/drivers/staging/media/hantro/hantro_h264.c
> index 194d05848077..6dcd47bd9ed3 100644
> --- a/drivers/staging/media/hantro/hantro_h264.c
> +++ b/drivers/staging/media/hantro/hantro_h264.c
> @@ -325,7 +325,7 @@ dma_addr_t hantro_h264_get_ref_buf(struct hantro_ctx *ctx,
>  		 */
>  		dst_buf = hantro_get_dst_buf(ctx);
>  		buf = &dst_buf->vb2_buf;
> -		dma_addr = vb2_dma_contig_plane_dma_addr(buf, 0);
> +		dma_addr = hantro_get_dec_buf_addr(ctx, buf);
>  	}
>  
>  	return dma_addr;

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [PATCH v2 2/2] hantro: postproc: Fix motion vector space allocation

[Philipp Zabel]

On Mon, 2020-07-27 at 14:05 -0300, Ezequiel Garcia wrote:
> When the post-processor is enabled, the driver allocates
> "shadow buffers" which are used for the decoder core,
> and exposes the post-processed buffers to userspace.
> 
> For this reason, extra motion vector space has to
> be allocated on the shadow buffers, which the driver
> wasn't doing. Fix it.
> 
> This fix should address artifacts on high profile bitstreams.
> 
> Fixes: 8c2d66b036c77 ("media: hantro: Support color conversion via post-processing")
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/staging/media/hantro/hantro_postproc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
> index 44062ffceaea..6d2a8f2a8f0b 100644
> --- a/drivers/staging/media/hantro/hantro_postproc.c
> +++ b/drivers/staging/media/hantro/hantro_postproc.c
> @@ -118,7 +118,9 @@ int hantro_postproc_alloc(struct hantro_ctx *ctx)
>  	unsigned int num_buffers = cap_queue->num_buffers;
>  	unsigned int i, buf_size;
>  
> -	buf_size = ctx->dst_fmt.plane_fmt[0].sizeimage;
> +	buf_size = ctx->dst_fmt.plane_fmt[0].sizeimage +
> +		   hantro_h264_mv_size(ctx->dst_fmt.width,
> +				       ctx->dst_fmt.height);
>  
>  	for (i = 0; i < num_buffers; ++i) {
>  		struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp