| Message ID | 20210912060812.222996-1-jernej.skrabec@gmail.com |
|---|---|
| State | Accepted |
| Commit | 5db127a534e1aa381c3ba60f51455863f37ea96b |
| Headers | show |
| Series | media: cedrus: Don't kernel map most buffers | expand |
Hi Jernej, Some comments below: On 12/09/2021 08:08, Jernej Skrabec wrote: > Except VP8 probability coefficients buffer, all other buffers are never Except -> Except for > accessed by CPU. That allows us to mark them with DMA_ATTR_NO_KERNEL_MAPPING > flag. This helps with decoding big (like 4k) videos on 32-bit ARM > platforms where default vmalloc size is relatively small - 240 MiB. > Since auxiliary buffer are not yet efficiently allocated, this can be > easily exceeded. Even if allocation is optimized, 4k videos will still > often exceed this limit. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > --- > .../staging/media/sunxi/cedrus/cedrus_h264.c | 102 ++++++++++-------- > .../staging/media/sunxi/cedrus/cedrus_h265.c | 28 ++--- > .../staging/media/sunxi/cedrus/cedrus_video.c | 2 + > 3 files changed, 73 insertions(+), 59 deletions(-) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > index de7442d4834d..6e38b37d9fe1 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > @@ -538,23 +538,23 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > > ctx->codec.h264.pic_info_buf_size = pic_info_size; > ctx->codec.h264.pic_info_buf = > - dma_alloc_coherent(dev->dev, ctx->codec.h264.pic_info_buf_size, > - &ctx->codec.h264.pic_info_buf_dma, > - GFP_KERNEL); > + dma_alloc_attrs(dev->dev, ctx->codec.h264.pic_info_buf_size, > + &ctx->codec.h264.pic_info_buf_dma, > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > if (!ctx->codec.h264.pic_info_buf) > return -ENOMEM; > > /* > * That buffer is supposed to be 16kiB in size, and be aligned > - * on 16kiB as well. However, dma_alloc_coherent provides the > + * on 16kiB as well. However, dma_alloc_attrs provides the > * guarantee that we'll have a CPU and DMA address aligned on Does the 'CPU' part of this sentence still make sense since the CPU won't access the buffer? > * the smallest page order that is greater to the requested > * size, so we don't have to overallocate. > */ > ctx->codec.h264.neighbor_info_buf = > - dma_alloc_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > - &ctx->codec.h264.neighbor_info_buf_dma, > - GFP_KERNEL); > + dma_alloc_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > + &ctx->codec.h264.neighbor_info_buf_dma, > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); I think it would be good to have a comment for all these dma_alloc_attrs calls where you note that these buffers are used by the HW only, and never by the CPU, hence the use of DMA_ATTR_NO_KERNEL_MAPPING. > if (!ctx->codec.h264.neighbor_info_buf) { > ret = -ENOMEM; > goto err_pic_buf; > @@ -582,10 +582,11 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > > mv_col_size = field_size * 2 * CEDRUS_H264_FRAME_NUM; > ctx->codec.h264.mv_col_buf_size = mv_col_size; > - ctx->codec.h264.mv_col_buf = dma_alloc_coherent(dev->dev, > - ctx->codec.h264.mv_col_buf_size, > - &ctx->codec.h264.mv_col_buf_dma, > - GFP_KERNEL); > + ctx->codec.h264.mv_col_buf = > + dma_alloc_attrs(dev->dev, > + ctx->codec.h264.mv_col_buf_size, > + &ctx->codec.h264.mv_col_buf_dma, > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > if (!ctx->codec.h264.mv_col_buf) { > ret = -ENOMEM; > goto err_neighbor_buf; > @@ -600,10 +601,10 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > ctx->codec.h264.deblk_buf_size = > ALIGN(ctx->src_fmt.width, 32) * 12; > ctx->codec.h264.deblk_buf = > - dma_alloc_coherent(dev->dev, > - ctx->codec.h264.deblk_buf_size, > - &ctx->codec.h264.deblk_buf_dma, > - GFP_KERNEL); > + dma_alloc_attrs(dev->dev, > + ctx->codec.h264.deblk_buf_size, > + &ctx->codec.h264.deblk_buf_dma, > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > if (!ctx->codec.h264.deblk_buf) { > ret = -ENOMEM; > goto err_mv_col_buf; > @@ -616,10 +617,10 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > ctx->codec.h264.intra_pred_buf_size = > ALIGN(ctx->src_fmt.width, 64) * 5 * 2; > ctx->codec.h264.intra_pred_buf = > - dma_alloc_coherent(dev->dev, > - ctx->codec.h264.intra_pred_buf_size, > - &ctx->codec.h264.intra_pred_buf_dma, > - GFP_KERNEL); > + dma_alloc_attrs(dev->dev, > + ctx->codec.h264.intra_pred_buf_size, > + &ctx->codec.h264.intra_pred_buf_dma, > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > if (!ctx->codec.h264.intra_pred_buf) { > ret = -ENOMEM; > goto err_deblk_buf; > @@ -629,24 +630,28 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > return 0; > > err_deblk_buf: > - dma_free_coherent(dev->dev, ctx->codec.h264.deblk_buf_size, > - ctx->codec.h264.deblk_buf, > - ctx->codec.h264.deblk_buf_dma); > + dma_free_attrs(dev->dev, ctx->codec.h264.deblk_buf_size, > + ctx->codec.h264.deblk_buf, > + ctx->codec.h264.deblk_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > > err_mv_col_buf: > - dma_free_coherent(dev->dev, ctx->codec.h264.mv_col_buf_size, > - ctx->codec.h264.mv_col_buf, > - ctx->codec.h264.mv_col_buf_dma); > + dma_free_attrs(dev->dev, ctx->codec.h264.mv_col_buf_size, > + ctx->codec.h264.mv_col_buf, > + ctx->codec.h264.mv_col_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > > err_neighbor_buf: > - dma_free_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > - ctx->codec.h264.neighbor_info_buf, > - ctx->codec.h264.neighbor_info_buf_dma); > + dma_free_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > + ctx->codec.h264.neighbor_info_buf, > + ctx->codec.h264.neighbor_info_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > > err_pic_buf: > - dma_free_coherent(dev->dev, ctx->codec.h264.pic_info_buf_size, > - ctx->codec.h264.pic_info_buf, > - ctx->codec.h264.pic_info_buf_dma); > + dma_free_attrs(dev->dev, ctx->codec.h264.pic_info_buf_size, > + ctx->codec.h264.pic_info_buf, > + ctx->codec.h264.pic_info_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > return ret; > } > > @@ -654,23 +659,28 @@ static void cedrus_h264_stop(struct cedrus_ctx *ctx) > { > struct cedrus_dev *dev = ctx->dev; > > - dma_free_coherent(dev->dev, ctx->codec.h264.mv_col_buf_size, > - ctx->codec.h264.mv_col_buf, > - ctx->codec.h264.mv_col_buf_dma); > - dma_free_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > - ctx->codec.h264.neighbor_info_buf, > - ctx->codec.h264.neighbor_info_buf_dma); > - dma_free_coherent(dev->dev, ctx->codec.h264.pic_info_buf_size, > - ctx->codec.h264.pic_info_buf, > - ctx->codec.h264.pic_info_buf_dma); > + dma_free_attrs(dev->dev, ctx->codec.h264.mv_col_buf_size, > + ctx->codec.h264.mv_col_buf, > + ctx->codec.h264.mv_col_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > + dma_free_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > + ctx->codec.h264.neighbor_info_buf, > + ctx->codec.h264.neighbor_info_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > + dma_free_attrs(dev->dev, ctx->codec.h264.pic_info_buf_size, > + ctx->codec.h264.pic_info_buf, > + ctx->codec.h264.pic_info_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > if (ctx->codec.h264.deblk_buf_size) > - dma_free_coherent(dev->dev, ctx->codec.h264.deblk_buf_size, > - ctx->codec.h264.deblk_buf, > - ctx->codec.h264.deblk_buf_dma); > + dma_free_attrs(dev->dev, ctx->codec.h264.deblk_buf_size, > + ctx->codec.h264.deblk_buf, > + ctx->codec.h264.deblk_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > if (ctx->codec.h264.intra_pred_buf_size) > - dma_free_coherent(dev->dev, ctx->codec.h264.intra_pred_buf_size, > - ctx->codec.h264.intra_pred_buf, > - ctx->codec.h264.intra_pred_buf_dma); > + dma_free_attrs(dev->dev, ctx->codec.h264.intra_pred_buf_size, > + ctx->codec.h264.intra_pred_buf, > + ctx->codec.h264.intra_pred_buf_dma, > + DMA_ATTR_NO_KERNEL_MAPPING); > } > > static void cedrus_h264_trigger(struct cedrus_ctx *ctx) > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > index 3d9561d4aadb..bb7eb56106c5 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > @@ -351,10 +351,10 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx, > ctx->codec.h265.mv_col_buf_unit_size; > > ctx->codec.h265.mv_col_buf = > - dma_alloc_coherent(dev->dev, > - ctx->codec.h265.mv_col_buf_size, > - &ctx->codec.h265.mv_col_buf_addr, > - GFP_KERNEL); > + dma_alloc_attrs(dev->dev, > + ctx->codec.h265.mv_col_buf_size, > + &ctx->codec.h265.mv_col_buf_addr, > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > if (!ctx->codec.h265.mv_col_buf) { > ctx->codec.h265.mv_col_buf_size = 0; > // TODO: Abort the process here. > @@ -668,9 +668,9 @@ static int cedrus_h265_start(struct cedrus_ctx *ctx) > ctx->codec.h265.mv_col_buf_size = 0; > > ctx->codec.h265.neighbor_info_buf = > - dma_alloc_coherent(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, > - &ctx->codec.h265.neighbor_info_buf_addr, > - GFP_KERNEL); > + dma_alloc_attrs(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, > + &ctx->codec.h265.neighbor_info_buf_addr, > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > if (!ctx->codec.h265.neighbor_info_buf) > return -ENOMEM; > > @@ -682,16 +682,18 @@ static void cedrus_h265_stop(struct cedrus_ctx *ctx) > struct cedrus_dev *dev = ctx->dev; > > if (ctx->codec.h265.mv_col_buf_size > 0) { > - dma_free_coherent(dev->dev, ctx->codec.h265.mv_col_buf_size, > - ctx->codec.h265.mv_col_buf, > - ctx->codec.h265.mv_col_buf_addr); > + dma_free_attrs(dev->dev, ctx->codec.h265.mv_col_buf_size, > + ctx->codec.h265.mv_col_buf, > + ctx->codec.h265.mv_col_buf_addr, > + DMA_ATTR_NO_KERNEL_MAPPING); > > ctx->codec.h265.mv_col_buf_size = 0; > } > > - dma_free_coherent(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, > - ctx->codec.h265.neighbor_info_buf, > - ctx->codec.h265.neighbor_info_buf_addr); > + dma_free_attrs(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, > + ctx->codec.h265.neighbor_info_buf, > + ctx->codec.h265.neighbor_info_buf_addr, > + DMA_ATTR_NO_KERNEL_MAPPING); > } > > static void cedrus_h265_trigger(struct cedrus_ctx *ctx) > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > index 66714609b577..800ffa5382de 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > @@ -568,6 +568,7 @@ int cedrus_queue_init(void *priv, struct vb2_queue *src_vq, > > src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT; > src_vq->io_modes = VB2_MMAP | VB2_DMABUF; > + src_vq->dma_attrs = DMA_ATTR_NO_KERNEL_MAPPING; > src_vq->drv_priv = ctx; > src_vq->buf_struct_size = sizeof(struct cedrus_buffer); > src_vq->ops = &cedrus_qops; > @@ -584,6 +585,7 @@ int cedrus_queue_init(void *priv, struct vb2_queue *src_vq, > > dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; > dst_vq->io_modes = VB2_MMAP | VB2_DMABUF; > + src_vq->dma_attrs = DMA_ATTR_NO_KERNEL_MAPPING; This should be dst_vq! I'm also not quite sure that it is right to use this for the destination buffer. Could this cause problems if this buffer is exported to a dmabuf and another driver will be using it and requiring a kernel mapping. Regards, Hans > dst_vq->drv_priv = ctx; > dst_vq->buf_struct_size = sizeof(struct cedrus_buffer); > dst_vq->ops = &cedrus_qops; >
Hi! Dne ponedeljek, 04. oktober 2021 ob 10:10:55 CEST je Hans Verkuil napisal(a): > Hi Jernej, > > Some comments below: > > On 12/09/2021 08:08, Jernej Skrabec wrote: > > Except VP8 probability coefficients buffer, all other buffers are never > > Except -> Except for > Ok. > > accessed by CPU. That allows us to mark them with DMA_ATTR_NO_KERNEL_MAPPING > > flag. This helps with decoding big (like 4k) videos on 32-bit ARM > > platforms where default vmalloc size is relatively small - 240 MiB. > > Since auxiliary buffer are not yet efficiently allocated, this can be > > easily exceeded. Even if allocation is optimized, 4k videos will still > > often exceed this limit. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > > --- > > .../staging/media/sunxi/cedrus/cedrus_h264.c | 102 ++++++++++-------- > > .../staging/media/sunxi/cedrus/cedrus_h265.c | 28 ++--- > > .../staging/media/sunxi/cedrus/cedrus_video.c | 2 + > > 3 files changed, 73 insertions(+), 59 deletions(-) > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/ staging/media/sunxi/cedrus/cedrus_h264.c > > index de7442d4834d..6e38b37d9fe1 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > @@ -538,23 +538,23 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > > > > ctx->codec.h264.pic_info_buf_size = pic_info_size; > > ctx->codec.h264.pic_info_buf = > > - dma_alloc_coherent(dev->dev, ctx- >codec.h264.pic_info_buf_size, > > - &ctx- >codec.h264.pic_info_buf_dma, > > - GFP_KERNEL); > > + dma_alloc_attrs(dev->dev, ctx- >codec.h264.pic_info_buf_size, > > + &ctx- >codec.h264.pic_info_buf_dma, > > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > > if (!ctx->codec.h264.pic_info_buf) > > return -ENOMEM; > > > > /* > > * That buffer is supposed to be 16kiB in size, and be aligned > > - * on 16kiB as well. However, dma_alloc_coherent provides the > > + * on 16kiB as well. However, dma_alloc_attrs provides the > > * guarantee that we'll have a CPU and DMA address aligned on > > Does the 'CPU' part of this sentence still make sense since the CPU > won't access the buffer? > Good catch! I'll remove it. > > * the smallest page order that is greater to the requested > > * size, so we don't have to overallocate. > > */ > > ctx->codec.h264.neighbor_info_buf = > > - dma_alloc_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > > - &ctx- >codec.h264.neighbor_info_buf_dma, > > - GFP_KERNEL); > > + dma_alloc_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > > + &ctx- >codec.h264.neighbor_info_buf_dma, > > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > > I think it would be good to have a comment for all these dma_alloc_attrs > calls where you note that these buffers are used by the HW only, and > never by the CPU, hence the use of DMA_ATTR_NO_KERNEL_MAPPING. > Ok. > > if (!ctx->codec.h264.neighbor_info_buf) { > > ret = -ENOMEM; > > goto err_pic_buf; > > @@ -582,10 +582,11 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > > > > mv_col_size = field_size * 2 * CEDRUS_H264_FRAME_NUM; > > ctx->codec.h264.mv_col_buf_size = mv_col_size; > > - ctx->codec.h264.mv_col_buf = dma_alloc_coherent(dev->dev, > > - ctx->codec.h264.mv_col_buf_size, > > - &ctx->codec.h264.mv_col_buf_dma, > > - GFP_KERNEL); > > + ctx->codec.h264.mv_col_buf = > > + dma_alloc_attrs(dev->dev, > > + ctx->codec.h264.mv_col_buf_size, > > + &ctx->codec.h264.mv_col_buf_dma, > > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > > if (!ctx->codec.h264.mv_col_buf) { > > ret = -ENOMEM; > > goto err_neighbor_buf; > > @@ -600,10 +601,10 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > > ctx->codec.h264.deblk_buf_size = > > ALIGN(ctx->src_fmt.width, 32) * 12; > > ctx->codec.h264.deblk_buf = > > - dma_alloc_coherent(dev->dev, > > - ctx- >codec.h264.deblk_buf_size, > > - &ctx- >codec.h264.deblk_buf_dma, > > - GFP_KERNEL); > > + dma_alloc_attrs(dev->dev, > > + ctx- >codec.h264.deblk_buf_size, > > + &ctx- >codec.h264.deblk_buf_dma, > > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > > if (!ctx->codec.h264.deblk_buf) { > > ret = -ENOMEM; > > goto err_mv_col_buf; > > @@ -616,10 +617,10 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > > ctx->codec.h264.intra_pred_buf_size = > > ALIGN(ctx->src_fmt.width, 64) * 5 * 2; > > ctx->codec.h264.intra_pred_buf = > > - dma_alloc_coherent(dev->dev, > > - ctx- >codec.h264.intra_pred_buf_size, > > - &ctx- >codec.h264.intra_pred_buf_dma, > > - GFP_KERNEL); > > + dma_alloc_attrs(dev->dev, > > + ctx- >codec.h264.intra_pred_buf_size, > > + &ctx- >codec.h264.intra_pred_buf_dma, > > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > > if (!ctx->codec.h264.intra_pred_buf) { > > ret = -ENOMEM; > > goto err_deblk_buf; > > @@ -629,24 +630,28 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) > > return 0; > > > > err_deblk_buf: > > - dma_free_coherent(dev->dev, ctx->codec.h264.deblk_buf_size, > > - ctx->codec.h264.deblk_buf, > > - ctx->codec.h264.deblk_buf_dma); > > + dma_free_attrs(dev->dev, ctx->codec.h264.deblk_buf_size, > > + ctx->codec.h264.deblk_buf, > > + ctx->codec.h264.deblk_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > > > err_mv_col_buf: > > - dma_free_coherent(dev->dev, ctx->codec.h264.mv_col_buf_size, > > - ctx->codec.h264.mv_col_buf, > > - ctx->codec.h264.mv_col_buf_dma); > > + dma_free_attrs(dev->dev, ctx->codec.h264.mv_col_buf_size, > > + ctx->codec.h264.mv_col_buf, > > + ctx->codec.h264.mv_col_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > > > err_neighbor_buf: > > - dma_free_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > > - ctx->codec.h264.neighbor_info_buf, > > - ctx->codec.h264.neighbor_info_buf_dma); > > + dma_free_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > > + ctx->codec.h264.neighbor_info_buf, > > + ctx->codec.h264.neighbor_info_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > > > err_pic_buf: > > - dma_free_coherent(dev->dev, ctx->codec.h264.pic_info_buf_size, > > - ctx->codec.h264.pic_info_buf, > > - ctx->codec.h264.pic_info_buf_dma); > > + dma_free_attrs(dev->dev, ctx->codec.h264.pic_info_buf_size, > > + ctx->codec.h264.pic_info_buf, > > + ctx->codec.h264.pic_info_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > return ret; > > } > > > > @@ -654,23 +659,28 @@ static void cedrus_h264_stop(struct cedrus_ctx *ctx) > > { > > struct cedrus_dev *dev = ctx->dev; > > > > - dma_free_coherent(dev->dev, ctx->codec.h264.mv_col_buf_size, > > - ctx->codec.h264.mv_col_buf, > > - ctx->codec.h264.mv_col_buf_dma); > > - dma_free_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > > - ctx->codec.h264.neighbor_info_buf, > > - ctx->codec.h264.neighbor_info_buf_dma); > > - dma_free_coherent(dev->dev, ctx->codec.h264.pic_info_buf_size, > > - ctx->codec.h264.pic_info_buf, > > - ctx->codec.h264.pic_info_buf_dma); > > + dma_free_attrs(dev->dev, ctx->codec.h264.mv_col_buf_size, > > + ctx->codec.h264.mv_col_buf, > > + ctx->codec.h264.mv_col_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > + dma_free_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, > > + ctx->codec.h264.neighbor_info_buf, > > + ctx->codec.h264.neighbor_info_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > + dma_free_attrs(dev->dev, ctx->codec.h264.pic_info_buf_size, > > + ctx->codec.h264.pic_info_buf, > > + ctx->codec.h264.pic_info_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > if (ctx->codec.h264.deblk_buf_size) > > - dma_free_coherent(dev->dev, ctx- >codec.h264.deblk_buf_size, > > - ctx->codec.h264.deblk_buf, > > - ctx->codec.h264.deblk_buf_dma); > > + dma_free_attrs(dev->dev, ctx- >codec.h264.deblk_buf_size, > > + ctx->codec.h264.deblk_buf, > > + ctx->codec.h264.deblk_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > if (ctx->codec.h264.intra_pred_buf_size) > > - dma_free_coherent(dev->dev, ctx- >codec.h264.intra_pred_buf_size, > > - ctx->codec.h264.intra_pred_buf, > > - ctx- >codec.h264.intra_pred_buf_dma); > > + dma_free_attrs(dev->dev, ctx- >codec.h264.intra_pred_buf_size, > > + ctx->codec.h264.intra_pred_buf, > > + ctx->codec.h264.intra_pred_buf_dma, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > } > > > > static void cedrus_h264_trigger(struct cedrus_ctx *ctx) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/ staging/media/sunxi/cedrus/cedrus_h265.c > > index 3d9561d4aadb..bb7eb56106c5 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > @@ -351,10 +351,10 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx, > > ctx->codec.h265.mv_col_buf_unit_size; > > > > ctx->codec.h265.mv_col_buf = > > - dma_alloc_coherent(dev->dev, > > - ctx- >codec.h265.mv_col_buf_size, > > - &ctx- >codec.h265.mv_col_buf_addr, > > - GFP_KERNEL); > > + dma_alloc_attrs(dev->dev, > > + ctx- >codec.h265.mv_col_buf_size, > > + &ctx- >codec.h265.mv_col_buf_addr, > > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > > if (!ctx->codec.h265.mv_col_buf) { > > ctx->codec.h265.mv_col_buf_size = 0; > > // TODO: Abort the process here. > > @@ -668,9 +668,9 @@ static int cedrus_h265_start(struct cedrus_ctx *ctx) > > ctx->codec.h265.mv_col_buf_size = 0; > > > > ctx->codec.h265.neighbor_info_buf = > > - dma_alloc_coherent(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, > > - &ctx- >codec.h265.neighbor_info_buf_addr, > > - GFP_KERNEL); > > + dma_alloc_attrs(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, > > + &ctx- >codec.h265.neighbor_info_buf_addr, > > + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); > > if (!ctx->codec.h265.neighbor_info_buf) > > return -ENOMEM; > > > > @@ -682,16 +682,18 @@ static void cedrus_h265_stop(struct cedrus_ctx *ctx) > > struct cedrus_dev *dev = ctx->dev; > > > > if (ctx->codec.h265.mv_col_buf_size > 0) { > > - dma_free_coherent(dev->dev, ctx- >codec.h265.mv_col_buf_size, > > - ctx->codec.h265.mv_col_buf, > > - ctx- >codec.h265.mv_col_buf_addr); > > + dma_free_attrs(dev->dev, ctx- >codec.h265.mv_col_buf_size, > > + ctx->codec.h265.mv_col_buf, > > + ctx->codec.h265.mv_col_buf_addr, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > > > ctx->codec.h265.mv_col_buf_size = 0; > > } > > > > - dma_free_coherent(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, > > - ctx->codec.h265.neighbor_info_buf, > > - ctx->codec.h265.neighbor_info_buf_addr); > > + dma_free_attrs(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, > > + ctx->codec.h265.neighbor_info_buf, > > + ctx->codec.h265.neighbor_info_buf_addr, > > + DMA_ATTR_NO_KERNEL_MAPPING); > > } > > > > static void cedrus_h265_trigger(struct cedrus_ctx *ctx) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/ staging/media/sunxi/cedrus/cedrus_video.c > > index 66714609b577..800ffa5382de 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > @@ -568,6 +568,7 @@ int cedrus_queue_init(void *priv, struct vb2_queue *src_vq, > > > > src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT; > > src_vq->io_modes = VB2_MMAP | VB2_DMABUF; > > + src_vq->dma_attrs = DMA_ATTR_NO_KERNEL_MAPPING; > > src_vq->drv_priv = ctx; > > src_vq->buf_struct_size = sizeof(struct cedrus_buffer); > > src_vq->ops = &cedrus_qops; > > @@ -584,6 +585,7 @@ int cedrus_queue_init(void *priv, struct vb2_queue *src_vq, > > > > dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; > > dst_vq->io_modes = VB2_MMAP | VB2_DMABUF; > > + src_vq->dma_attrs = DMA_ATTR_NO_KERNEL_MAPPING; > > This should be dst_vq! > > I'm also not quite sure that it is right to use this for the destination > buffer. Could this cause problems if this buffer is exported to a dmabuf > and another driver will be using it and requiring a kernel mapping. Hm, if I fix this, it actually breaks DRM mapping. For some reason it expects vmap to be valid. I'll drop this for now. Thanks for review! Best regards, Jernej > > Regards, > > Hans > > > dst_vq->drv_priv = ctx; > > dst_vq->buf_struct_size = sizeof(struct cedrus_buffer); > > dst_vq->ops = &cedrus_qops; > > > >
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index de7442d4834d..6e38b37d9fe1 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c @@ -538,23 +538,23 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) ctx->codec.h264.pic_info_buf_size = pic_info_size; ctx->codec.h264.pic_info_buf = - dma_alloc_coherent(dev->dev, ctx->codec.h264.pic_info_buf_size, - &ctx->codec.h264.pic_info_buf_dma, - GFP_KERNEL); + dma_alloc_attrs(dev->dev, ctx->codec.h264.pic_info_buf_size, + &ctx->codec.h264.pic_info_buf_dma, + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); if (!ctx->codec.h264.pic_info_buf) return -ENOMEM; /* * That buffer is supposed to be 16kiB in size, and be aligned - * on 16kiB as well. However, dma_alloc_coherent provides the + * on 16kiB as well. However, dma_alloc_attrs provides the * guarantee that we'll have a CPU and DMA address aligned on * the smallest page order that is greater to the requested * size, so we don't have to overallocate. */ ctx->codec.h264.neighbor_info_buf = - dma_alloc_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, - &ctx->codec.h264.neighbor_info_buf_dma, - GFP_KERNEL); + dma_alloc_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, + &ctx->codec.h264.neighbor_info_buf_dma, + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); if (!ctx->codec.h264.neighbor_info_buf) { ret = -ENOMEM; goto err_pic_buf; @@ -582,10 +582,11 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) mv_col_size = field_size * 2 * CEDRUS_H264_FRAME_NUM; ctx->codec.h264.mv_col_buf_size = mv_col_size; - ctx->codec.h264.mv_col_buf = dma_alloc_coherent(dev->dev, - ctx->codec.h264.mv_col_buf_size, - &ctx->codec.h264.mv_col_buf_dma, - GFP_KERNEL); + ctx->codec.h264.mv_col_buf = + dma_alloc_attrs(dev->dev, + ctx->codec.h264.mv_col_buf_size, + &ctx->codec.h264.mv_col_buf_dma, + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); if (!ctx->codec.h264.mv_col_buf) { ret = -ENOMEM; goto err_neighbor_buf; @@ -600,10 +601,10 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) ctx->codec.h264.deblk_buf_size = ALIGN(ctx->src_fmt.width, 32) * 12; ctx->codec.h264.deblk_buf = - dma_alloc_coherent(dev->dev, - ctx->codec.h264.deblk_buf_size, - &ctx->codec.h264.deblk_buf_dma, - GFP_KERNEL); + dma_alloc_attrs(dev->dev, + ctx->codec.h264.deblk_buf_size, + &ctx->codec.h264.deblk_buf_dma, + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); if (!ctx->codec.h264.deblk_buf) { ret = -ENOMEM; goto err_mv_col_buf; @@ -616,10 +617,10 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) ctx->codec.h264.intra_pred_buf_size = ALIGN(ctx->src_fmt.width, 64) * 5 * 2; ctx->codec.h264.intra_pred_buf = - dma_alloc_coherent(dev->dev, - ctx->codec.h264.intra_pred_buf_size, - &ctx->codec.h264.intra_pred_buf_dma, - GFP_KERNEL); + dma_alloc_attrs(dev->dev, + ctx->codec.h264.intra_pred_buf_size, + &ctx->codec.h264.intra_pred_buf_dma, + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); if (!ctx->codec.h264.intra_pred_buf) { ret = -ENOMEM; goto err_deblk_buf; @@ -629,24 +630,28 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx) return 0; err_deblk_buf: - dma_free_coherent(dev->dev, ctx->codec.h264.deblk_buf_size, - ctx->codec.h264.deblk_buf, - ctx->codec.h264.deblk_buf_dma); + dma_free_attrs(dev->dev, ctx->codec.h264.deblk_buf_size, + ctx->codec.h264.deblk_buf, + ctx->codec.h264.deblk_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); err_mv_col_buf: - dma_free_coherent(dev->dev, ctx->codec.h264.mv_col_buf_size, - ctx->codec.h264.mv_col_buf, - ctx->codec.h264.mv_col_buf_dma); + dma_free_attrs(dev->dev, ctx->codec.h264.mv_col_buf_size, + ctx->codec.h264.mv_col_buf, + ctx->codec.h264.mv_col_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); err_neighbor_buf: - dma_free_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, - ctx->codec.h264.neighbor_info_buf, - ctx->codec.h264.neighbor_info_buf_dma); + dma_free_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, + ctx->codec.h264.neighbor_info_buf, + ctx->codec.h264.neighbor_info_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); err_pic_buf: - dma_free_coherent(dev->dev, ctx->codec.h264.pic_info_buf_size, - ctx->codec.h264.pic_info_buf, - ctx->codec.h264.pic_info_buf_dma); + dma_free_attrs(dev->dev, ctx->codec.h264.pic_info_buf_size, + ctx->codec.h264.pic_info_buf, + ctx->codec.h264.pic_info_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); return ret; } @@ -654,23 +659,28 @@ static void cedrus_h264_stop(struct cedrus_ctx *ctx) { struct cedrus_dev *dev = ctx->dev; - dma_free_coherent(dev->dev, ctx->codec.h264.mv_col_buf_size, - ctx->codec.h264.mv_col_buf, - ctx->codec.h264.mv_col_buf_dma); - dma_free_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, - ctx->codec.h264.neighbor_info_buf, - ctx->codec.h264.neighbor_info_buf_dma); - dma_free_coherent(dev->dev, ctx->codec.h264.pic_info_buf_size, - ctx->codec.h264.pic_info_buf, - ctx->codec.h264.pic_info_buf_dma); + dma_free_attrs(dev->dev, ctx->codec.h264.mv_col_buf_size, + ctx->codec.h264.mv_col_buf, + ctx->codec.h264.mv_col_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); + dma_free_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE, + ctx->codec.h264.neighbor_info_buf, + ctx->codec.h264.neighbor_info_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); + dma_free_attrs(dev->dev, ctx->codec.h264.pic_info_buf_size, + ctx->codec.h264.pic_info_buf, + ctx->codec.h264.pic_info_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); if (ctx->codec.h264.deblk_buf_size) - dma_free_coherent(dev->dev, ctx->codec.h264.deblk_buf_size, - ctx->codec.h264.deblk_buf, - ctx->codec.h264.deblk_buf_dma); + dma_free_attrs(dev->dev, ctx->codec.h264.deblk_buf_size, + ctx->codec.h264.deblk_buf, + ctx->codec.h264.deblk_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); if (ctx->codec.h264.intra_pred_buf_size) - dma_free_coherent(dev->dev, ctx->codec.h264.intra_pred_buf_size, - ctx->codec.h264.intra_pred_buf, - ctx->codec.h264.intra_pred_buf_dma); + dma_free_attrs(dev->dev, ctx->codec.h264.intra_pred_buf_size, + ctx->codec.h264.intra_pred_buf, + ctx->codec.h264.intra_pred_buf_dma, + DMA_ATTR_NO_KERNEL_MAPPING); } static void cedrus_h264_trigger(struct cedrus_ctx *ctx) diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c index 3d9561d4aadb..bb7eb56106c5 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c @@ -351,10 +351,10 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx, ctx->codec.h265.mv_col_buf_unit_size; ctx->codec.h265.mv_col_buf = - dma_alloc_coherent(dev->dev, - ctx->codec.h265.mv_col_buf_size, - &ctx->codec.h265.mv_col_buf_addr, - GFP_KERNEL); + dma_alloc_attrs(dev->dev, + ctx->codec.h265.mv_col_buf_size, + &ctx->codec.h265.mv_col_buf_addr, + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); if (!ctx->codec.h265.mv_col_buf) { ctx->codec.h265.mv_col_buf_size = 0; // TODO: Abort the process here. @@ -668,9 +668,9 @@ static int cedrus_h265_start(struct cedrus_ctx *ctx) ctx->codec.h265.mv_col_buf_size = 0; ctx->codec.h265.neighbor_info_buf = - dma_alloc_coherent(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, - &ctx->codec.h265.neighbor_info_buf_addr, - GFP_KERNEL); + dma_alloc_attrs(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, + &ctx->codec.h265.neighbor_info_buf_addr, + GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING); if (!ctx->codec.h265.neighbor_info_buf) return -ENOMEM; @@ -682,16 +682,18 @@ static void cedrus_h265_stop(struct cedrus_ctx *ctx) struct cedrus_dev *dev = ctx->dev; if (ctx->codec.h265.mv_col_buf_size > 0) { - dma_free_coherent(dev->dev, ctx->codec.h265.mv_col_buf_size, - ctx->codec.h265.mv_col_buf, - ctx->codec.h265.mv_col_buf_addr); + dma_free_attrs(dev->dev, ctx->codec.h265.mv_col_buf_size, + ctx->codec.h265.mv_col_buf, + ctx->codec.h265.mv_col_buf_addr, + DMA_ATTR_NO_KERNEL_MAPPING); ctx->codec.h265.mv_col_buf_size = 0; } - dma_free_coherent(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, - ctx->codec.h265.neighbor_info_buf, - ctx->codec.h265.neighbor_info_buf_addr); + dma_free_attrs(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE, + ctx->codec.h265.neighbor_info_buf, + ctx->codec.h265.neighbor_info_buf_addr, + DMA_ATTR_NO_KERNEL_MAPPING); } static void cedrus_h265_trigger(struct cedrus_ctx *ctx) diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c index 66714609b577..800ffa5382de 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c @@ -568,6 +568,7 @@ int cedrus_queue_init(void *priv, struct vb2_queue *src_vq, src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT; src_vq->io_modes = VB2_MMAP | VB2_DMABUF; + src_vq->dma_attrs = DMA_ATTR_NO_KERNEL_MAPPING; src_vq->drv_priv = ctx; src_vq->buf_struct_size = sizeof(struct cedrus_buffer); src_vq->ops = &cedrus_qops; @@ -584,6 +585,7 @@ int cedrus_queue_init(void *priv, struct vb2_queue *src_vq, dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; dst_vq->io_modes = VB2_MMAP | VB2_DMABUF; + src_vq->dma_attrs = DMA_ATTR_NO_KERNEL_MAPPING; dst_vq->drv_priv = ctx; dst_vq->buf_struct_size = sizeof(struct cedrus_buffer); dst_vq->ops = &cedrus_qops;
Except VP8 probability coefficients buffer, all other buffers are never accessed by CPU. That allows us to mark them with DMA_ATTR_NO_KERNEL_MAPPING flag. This helps with decoding big (like 4k) videos on 32-bit ARM platforms where default vmalloc size is relatively small - 240 MiB. Since auxiliary buffer are not yet efficiently allocated, this can be easily exceeded. Even if allocation is optimized, 4k videos will still often exceed this limit. Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> --- .../staging/media/sunxi/cedrus/cedrus_h264.c | 102 ++++++++++-------- .../staging/media/sunxi/cedrus/cedrus_h265.c | 28 ++--- .../staging/media/sunxi/cedrus/cedrus_video.c | 2 + 3 files changed, 73 insertions(+), 59 deletions(-)