Hi Hans,
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;
+}
+
/**
* __vb2_buf_mem_alloc() - allocate video memory for the given buffer
*/
static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
{
struct vb2_queue *q = vb->vb2_queue;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
void *mem_priv;
int plane;
int ret = -ENOMEM;
@@ -978,8 +985,7 @@ static int __prepare_userptr(struct vb2_buffer *vb,
const void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1096,8 +1102,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb,
const void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
--
regards,
Stan
