linux-kernel - Re: [RFC PATCH] media: vb2: add bidirectional flag in vb2

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFLEztTRzyEJ-6iQLKaMe-HP-vfZ_M=zE-vR5GyV0hQcFHmZ5A@mail.gmail.com>
Date:   Thu, 17 Aug 2017 21:16:51 +0800
From:   Jacob Chen <jacobchen110@...il.com>
To:     Stanimir Varbanov <stanimir.varbanov@...aro.org>
Cc:     Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Hans Verkuil <hverkuil@...all.nl>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Pawel Osciak <pawel@...iak.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Sakari Ailus <sakari.ailus@....fi>
Subject: Re: [RFC PATCH] media: vb2: add bidirectional flag in vb2_queue

Hi,

2017-08-17 20:11 GMT+08:00 Stanimir Varbanov <stanimir.varbanov@...aro.org>:
> 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.
>>>>>

Just butt in.....
some codec hardware also need it, which will use
capture buffers as reference to decode other buffers.

>>>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@...aro.org>
>>>>> ---
>>>>>
>>>>>  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