When allocating aux buffers for postprocessing, it's assumed that base
buffer size is the same as that of output. Coincidentally, that's true
most of the time, but not always. 10-bit source also needs aux buffer
size which is appropriate for 10-bit native format, even if the output
format is 8-bit. Similarly, mv sizes and other extra buffer size also
depends on source width/height, not destination.
Signed-off-by: Jernej Skrabec <[email protected]>
---
.../staging/media/hantro/hantro_postproc.c | 24 +++++++++++++------
drivers/staging/media/hantro/hantro_v4l2.c | 2 +-
drivers/staging/media/hantro/hantro_v4l2.h | 2 ++
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
index 248abe5423f0..1a76628d5754 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -12,6 +12,7 @@
#include "hantro_hw.h"
#include "hantro_g1_regs.h"
#include "hantro_g2_regs.h"
+#include "hantro_v4l2.h"
#define HANTRO_PP_REG_WRITE(vpu, reg_name, val) \
{ \
@@ -137,18 +138,27 @@ int hantro_postproc_alloc(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;
+ struct v4l2_pix_format_mplane pix_mp;
+ const struct hantro_fmt *fmt;
unsigned int i, buf_size;
- buf_size = ctx->dst_fmt.plane_fmt[0].sizeimage;
+ /* this should always pick native format */
+ fmt = hantro_get_default_fmt(ctx, false);
+ if (!fmt)
+ return -EINVAL;
+ v4l2_fill_pixfmt_mp(&pix_mp, fmt->fourcc, ctx->src_fmt.width,
+ ctx->src_fmt.height);
+
+ buf_size = pix_mp.plane_fmt[0].sizeimage;
if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE)
- buf_size += hantro_h264_mv_size(ctx->dst_fmt.width,
- ctx->dst_fmt.height);
+ buf_size += hantro_h264_mv_size(pix_mp.width,
+ pix_mp.height);
else if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_VP9_FRAME)
- buf_size += hantro_vp9_mv_size(ctx->dst_fmt.width,
- ctx->dst_fmt.height);
+ buf_size += hantro_vp9_mv_size(pix_mp.width,
+ pix_mp.height);
else if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_HEVC_SLICE)
- buf_size += hantro_hevc_mv_size(ctx->dst_fmt.width,
- ctx->dst_fmt.height);
+ buf_size += hantro_hevc_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];
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 1214fa2f64ae..69d2a108e1e6 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -118,7 +118,7 @@ hantro_find_format(const struct hantro_ctx *ctx, u32 fourcc)
return NULL;
}
-static const struct hantro_fmt *
+const struct hantro_fmt *
hantro_get_default_fmt(const struct hantro_ctx *ctx, bool bitstream)
{
const struct hantro_fmt *formats;
diff --git a/drivers/staging/media/hantro/hantro_v4l2.h b/drivers/staging/media/hantro/hantro_v4l2.h
index f4a5905ed518..cc9a645be886 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.h
+++ b/drivers/staging/media/hantro/hantro_v4l2.h
@@ -23,5 +23,7 @@ extern const struct vb2_ops hantro_queue_ops;
void hantro_reset_fmts(struct hantro_ctx *ctx);
int hantro_get_formath_depth(u32 fourcc);
+const struct hantro_fmt *
+hantro_get_default_fmt(const struct hantro_ctx *ctx, bool bitstream);
#endif /* HANTRO_V4L2_H_ */
--
2.35.1
Hi Jernej,
On Sun, Feb 27, 2022 at 03:49:22PM +0100, Jernej Skrabec wrote:
> When allocating aux buffers for postprocessing, it's assumed that base
> buffer size is the same as that of output. Coincidentally, that's true
> most of the time, but not always. 10-bit source also needs aux buffer
> size which is appropriate for 10-bit native format, even if the output
> format is 8-bit. Similarly, mv sizes and other extra buffer size also
> depends on source width/height, not destination.
>
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> .../staging/media/hantro/hantro_postproc.c | 24 +++++++++++++------
> drivers/staging/media/hantro/hantro_v4l2.c | 2 +-
> drivers/staging/media/hantro/hantro_v4l2.h | 2 ++
> 3 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
> index 248abe5423f0..1a76628d5754 100644
> --- a/drivers/staging/media/hantro/hantro_postproc.c
> +++ b/drivers/staging/media/hantro/hantro_postproc.c
> @@ -12,6 +12,7 @@
> #include "hantro_hw.h"
> #include "hantro_g1_regs.h"
> #include "hantro_g2_regs.h"
> +#include "hantro_v4l2.h"
>
> #define HANTRO_PP_REG_WRITE(vpu, reg_name, val) \
> { \
> @@ -137,18 +138,27 @@ int hantro_postproc_alloc(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;
> + struct v4l2_pix_format_mplane pix_mp;
> + const struct hantro_fmt *fmt;
> unsigned int i, buf_size;
>
> - buf_size = ctx->dst_fmt.plane_fmt[0].sizeimage;
> + /* this should always pick native format */
> + fmt = hantro_get_default_fmt(ctx, false);
> + if (!fmt)
> + return -EINVAL;
> + v4l2_fill_pixfmt_mp(&pix_mp, fmt->fourcc, ctx->src_fmt.width,
> + ctx->src_fmt.height);
> +
> + buf_size = pix_mp.plane_fmt[0].sizeimage;
Took me a while to see that the main change is taking buf_size
from pix_mp, which now takes into account the bit-depth :)
To me this makes sense.
Reviewed-by: Ezequiel Garcia <[email protected]>
Thanks,
Ezequiel