| Message ID | 20220227144926.3006585-5-jernej.skrabec@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: hantro: Add 10-bit support | expand |
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 <jernej.skrabec@gmail.com> > --- > .../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 <ezequiel@vanguardiasur.com.ar> Thanks, Ezequiel
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_ */
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 <jernej.skrabec@gmail.com> --- .../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(-)