[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e6444ca3-53c9-15c3-ff49-cb0b3e291fc2@xs4all.nl>
Date: Wed, 20 Jul 2016 16:12:30 +0200
From: Hans Verkuil <hverkuil@...all.nl>
To: Javier Martinez Canillas <javier@....samsung.com>,
Sakari Ailus <sakari.ailus@....fi>
Cc: linux-kernel@...r.kernel.org,
Marek Szyprowski <m.szyprowski@...sung.com>,
Mauro Carvalho Chehab <mchehab@...pensource.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
Pawel Osciak <pawel@...iak.com>, linux-media@...r.kernel.org,
Shuah Khan <shuahkh@....samsung.com>,
Nicolas Dufresne <nicolas.dufresne@...labora.com>,
Luis de Bethencourt <luisbg@....samsung.com>
Subject: Re: [PATCH] [media] vb2: map dmabuf for planes on driver queue
instead of vidioc_qbuf
On 07/20/2016 04:06 PM, Javier Martinez Canillas wrote:
> Hello Sakari,
>
> On 07/20/2016 09:20 AM, Sakari Ailus wrote:
>> Hi Javier,
>>
>> On Fri, Jul 15, 2016 at 12:26:06PM -0400, Javier Martinez Canillas wrote:
>>> The buffer planes' dma-buf are currently mapped when buffers are queued
>>> from userspace but it's more appropriate to do the mapping when buffers
>>> are queued in the driver since that's when the actual DMA operation are
>>> going to happen.
>>>
>>> Suggested-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
>>> Signed-off-by: Javier Martinez Canillas <javier@....samsung.com>
>>>
>>> ---
>>>
>>> Hello,
>>>
>>> A side effect of this change is that if the dmabuf map fails for some
>>> reasons (i.e: a driver using the DMA contig memory allocator but CMA
>>> not being enabled), the fail will no longer happen on VIDIOC_QBUF but
>>> later (i.e: in VIDIOC_STREAMON).
>>>
>>> I don't know if that's an issue though but I think is worth mentioning.
>>
>> I have the same question has Hans --- why?
>>
>
> Yes, sorry for missing this information. Nicolas already explained a little
> bit but the context is that I want to add dma-buf fence support to videobuf2,
> and currently the dma-buf is unmapped in VIDIOC_DQBUF.
>
> But with dma-buf fence, the idea is to be able to dequeue a buffer even when
> the driver has not yet finished processing the buffer. So the dma-buf needs to
> be mapped until vb2_buffer_done() when the driver is done processing the vb2,
> and is able to signal the pending fence.
>
> Since the unmapping was going to be delayed to vb2_buffer_done(), I thought
> it would make sense to also move the mapping closer to when is really going
> to be used and that's why I moved it to __enqueue_in_driver() in this patch.
>
> But I didn't know that user-space was using the dma-buf map as a way to know
> if the dma-buf will be compatible and fallback to a different streaming I/O
> method if that's not the case. So $SUBJECT is wrong if it prevents user-space
> to recover gracefully from a dma-buf mapping failure.
>
> In any case, only delaying the unmapping is needed to support fence and doing
> the map early in VIDIOC_QBUF is not an issue.
OK. I've rejected this patch. I understand the DQBUF part and I happily accept
a patch for that. But the other side should be left as-is. The TODO comment
should probably be dropped, now that I think about it.
Regards,
Hans
>> I rather think we should keep the buffers mapped all the time. That'd
>> require a bit of extra from the DMA-BUF framework I suppose, to support
>> streaming mappings.
>>
>
> Interesting, I can take a look to this possibility after adding the dma-buf
> fence support.
>
> Best regards,
>
Powered by blists - more mailing lists