Hi Laurent,
On 08/16/2017 03:28 PM, Laurent Pinchart wrote:
> Hi Stan,
>
> On Wednesday 16 Aug 2017 14:46:50 Stanimir Varbanov wrote:
>> On 08/15/2017 01:04 PM, Hans Verkuil wrote:
>>> On 08/14/17 10:41, Stanimir Varbanov wrote:
>>>> Hi,
>>>>
>>>> This RFC patch is intended to give to the drivers a choice to change
>>>> the default behavior of the v4l2-core DMA mapping direction from
>>>> DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or OUTPUT)
>>>> to DMA_BIDIRECTIONAL during queue_init time.
>>>>
>>>> Initially the issue with DMA mapping direction has been found in
>>>> Venus encoder driver where the firmware side of the driver adds few
>>>> lines padding on bottom of the image buffer, and the consequence was
>>>> triggering of IOMMU protection faults.
>>>>
>>>> Probably other drivers could also has a benefit of this feature (hint)
>>>> in the future.
>>>>
>>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx>
>>>> ---
>>>>
>>>> drivers/media/v4l2-core/videobuf2-core.c | 3 +++
>>>> include/media/videobuf2-core.h | 11 +++++++++++
>>>> 2 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>>>> b/drivers/media/v4l2-core/videobuf2-core.c index
>>>> 14f83cecfa92..17d07fda4cdc 100644
>>>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>>>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>>>> @@ -200,6 +200,9 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>>>>
>>>> int plane;
>>>> int ret = -ENOMEM;
>>>>
>>>> + if (q->bidirectional)
>>>> + dma_dir = DMA_BIDIRECTIONAL;
>>>> +
>>>
>>> Does this only have to be used in mem_alloc? In the __prepare_*() it is
>>> still using DMA_TO/FROM_DEVICE.
>>
>> Yes, it looks like the DMA direction should be covered in the
>> __prepare_* too. Thus the patch should look like below:
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 14f83cecfa92..0089e7dac7dd 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -188,14 +188,21 @@ module_param(debug, int, 0644);
>> static void __vb2_queue_cancel(struct vb2_queue *q);
>> static void __enqueue_in_driver(struct vb2_buffer *vb);
>>
>> +static enum dma_data_direction __get_dma_dir(struct vb2_queue *q)
>> +{
>> + if (q->bidirectional)
>> + return DMA_BIDIRECTIONAL;
>> +
>> + return q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>
> We could also compute the DMA direction once only and store it in the queue. I
> have no big preference at the moment.
Yes, I like the idea. I'll cook a regular patch where the DMA direction
will be computed in vb2_core_queue_init().
--
regards,
Stan
