| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <929e9bf7-9b9e-4b7f-889d-b2bc144fd39a@xs4all.nl>
Date: Tue, 19 Sep 2023 12:57:44 +0200
From: Hans Verkuil <hverkuil-cisco@...all.nl>
To: Benjamin Gaignard <benjamin.gaignard@...labora.com>,
mchehab@...nel.org, tfiga@...omium.org, m.szyprowski@...sung.com,
ming.qian@....com, ezequiel@...guardiasur.com.ar,
p.zabel@...gutronix.de, gregkh@...uxfoundation.org,
nicolas.dufresne@...labora.com
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
linux-rockchip@...ts.infradead.org, linux-staging@...ts.linux.dev,
kernel@...labora.com
Subject: Re: [PATCH v7 13/49] media: verisilicon: Refactor postprocessor to
store more buffers
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@...labora.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;
> }
Powered by blists - more mailing lists