Re: [RFCv6 PATCH 04/16] vb2-dma-sg: add allocation context to dma-sg

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 10, 2014 at 8:49 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> Require that dma-sg also uses an allocation context. This is in preparation
> for adding prepare/finish memops to sync the memory between DMA and CPU.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---

[...]

> @@ -166,6 +177,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
>                                     unsigned long size,
>                                     enum dma_data_direction dma_dir)
>  {
> +       struct vb2_dma_sg_conf *conf = alloc_ctx;
>         struct vb2_dma_sg_buf *buf;
>         unsigned long first, last;
>         int num_pages_from_user;
> @@ -235,6 +247,8 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
>                         buf->num_pages, buf->offset, size, 0))
>                 goto userptr_fail_alloc_table_from_pages;
>
> +       /* Prevent the device from being released while the buffer is used */
> +       buf->dev = get_device(conf->dev);

I'm not sure if we should be managing this... As far as I understand
the logic behind taking a ref in alloc, if we are the exporter, we may
have to keep it in case we need to free the buffers after our device
goes away. But for userptr, we only need this for syncs, and in that
case it's triggered by our driver, so I think we don't have to worry
about that. If we do though, then dma-contig should be doing this as
well.

>         return buf;
>
>  userptr_fail_alloc_table_from_pages:
> @@ -274,6 +288,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>         }
>         kfree(buf->pages);
>         vb2_put_vma(buf->vma);
> +       put_device(buf->dev);
>         kfree(buf);
>  }
>
> @@ -356,6 +371,27 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>  };
>  EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
>
> +void *vb2_dma_sg_init_ctx(struct device *dev)
> +{
> +       struct vb2_dma_sg_conf *conf;
> +
> +       conf = kzalloc(sizeof(*conf), GFP_KERNEL);
> +       if (!conf)
> +               return ERR_PTR(-ENOMEM);
> +
> +       conf->dev = dev;
> +
> +       return conf;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_sg_init_ctx);
> +
> +void vb2_dma_sg_cleanup_ctx(void *alloc_ctx)
> +{
> +       if (!IS_ERR_OR_NULL(alloc_ctx))

I would prefer not doing this, it's very weird and would really just
be a programming bug.


-- 
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux