* [PATCH 1/2] media: venus: fix wrong size on dma_free
@ 2017-10-09 12:24 Stanimir Varbanov
2017-10-09 12:24 ` [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field Stanimir Varbanov
0 siblings, 1 reply; 6+ messages in thread
From: Stanimir Varbanov @ 2017-10-09 12:24 UTC (permalink / raw)
To: Hans Verkuil
Cc: Nicolas Dufresne, linux-media, linux-kernel, linux-arm-msm,
Stanimir Varbanov
This change will fix an issue with dma_free size found with
DMA API debug enabled.
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
drivers/media/platform/qcom/venus/hfi_venus.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index 1caae8feaa36..734ce11b0ed0 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -344,7 +344,7 @@ static int venus_alloc(struct venus_hfi_device *hdev, struct mem_desc *desc,
desc->attrs = DMA_ATTR_WRITE_COMBINE;
desc->size = ALIGN(size, SZ_4K);
- desc->kva = dma_alloc_attrs(dev, size, &desc->da, GFP_KERNEL,
+ desc->kva = dma_alloc_attrs(dev, desc->size, &desc->da, GFP_KERNEL,
desc->attrs);
if (!desc->kva)
return -ENOMEM;
@@ -710,10 +710,8 @@ static int venus_interface_queues_init(struct venus_hfi_device *hdev)
if (ret)
return ret;
- hdev->ifaceq_table.kva = desc.kva;
- hdev->ifaceq_table.da = desc.da;
- hdev->ifaceq_table.size = IFACEQ_TABLE_SIZE;
- offset = hdev->ifaceq_table.size;
+ hdev->ifaceq_table = desc;
+ offset = IFACEQ_TABLE_SIZE;
for (i = 0; i < IFACEQ_NUM; i++) {
queue = &hdev->queues[i];
@@ -755,9 +753,7 @@ static int venus_interface_queues_init(struct venus_hfi_device *hdev)
if (ret) {
hdev->sfr.da = 0;
} else {
- hdev->sfr.da = desc.da;
- hdev->sfr.kva = desc.kva;
- hdev->sfr.size = ALIGNED_SFR_SIZE;
+ hdev->sfr = desc;
sfr = hdev->sfr.kva;
sfr->buf_size = ALIGNED_SFR_SIZE;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field
2017-10-09 12:24 [PATCH 1/2] media: venus: fix wrong size on dma_free Stanimir Varbanov
@ 2017-10-09 12:24 ` Stanimir Varbanov
2017-10-09 12:31 ` Hans Verkuil
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stanimir Varbanov @ 2017-10-09 12:24 UTC (permalink / raw)
To: Hans Verkuil
Cc: Nicolas Dufresne, linux-media, linux-kernel, linux-arm-msm,
Stanimir Varbanov
This fixes wrongly filled bytesused field of v4l2_plane structure
by include data_offset in the plane, Also fill data_offset and
bytesused for capture type of buffers only.
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
drivers/media/platform/qcom/venus/venc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 6f123a387cf9..9445ad492966 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -963,15 +963,16 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
if (!vbuf)
return;
- vb = &vbuf->vb2_buf;
- vb->planes[0].bytesused = bytesused;
- vb->planes[0].data_offset = data_offset;
-
vbuf->flags = flags;
if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+ vb = &vbuf->vb2_buf;
+ vb2_set_plane_payload(vb, 0, bytesused + data_offset);
+ vb->planes[0].data_offset = data_offset;
vb->timestamp = timestamp_us * NSEC_PER_USEC;
vbuf->sequence = inst->sequence_cap++;
+
+ WARN_ON(vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0));
} else {
vbuf->sequence = inst->sequence_out++;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field
2017-10-09 12:24 ` [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field Stanimir Varbanov
@ 2017-10-09 12:31 ` Hans Verkuil
2017-10-09 22:17 ` Stanimir Varbanov
2017-10-09 23:41 ` Nicolas Dufresne
2017-10-10 7:52 ` [PATCH v2] " Stanimir Varbanov
2 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2017-10-09 12:31 UTC (permalink / raw)
To: Stanimir Varbanov
Cc: Nicolas Dufresne, linux-media, linux-kernel, linux-arm-msm
On 09/10/17 14:24, Stanimir Varbanov wrote:
> This fixes wrongly filled bytesused field of v4l2_plane structure
> by include data_offset in the plane, Also fill data_offset and
> bytesused for capture type of buffers only.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
> drivers/media/platform/qcom/venus/venc.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 6f123a387cf9..9445ad492966 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -963,15 +963,16 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
> if (!vbuf)
> return;
>
> - vb = &vbuf->vb2_buf;
> - vb->planes[0].bytesused = bytesused;
> - vb->planes[0].data_offset = data_offset;
> -
> vbuf->flags = flags;
>
> if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> + vb = &vbuf->vb2_buf;
> + vb2_set_plane_payload(vb, 0, bytesused + data_offset);
> + vb->planes[0].data_offset = data_offset;
> vb->timestamp = timestamp_us * NSEC_PER_USEC;
> vbuf->sequence = inst->sequence_cap++;
> +
> + WARN_ON(vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0));
It's good to have this, but this really should never happen. Because if it is,
then you'll have a memory overwrite. I hope the DMA engine will prevent this?
Just wondering how this works.
The patch looks good otherwise, but that WARN_ON is a bit scary.
Regards,
Hans
> } else {
> vbuf->sequence = inst->sequence_out++;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field
2017-10-09 12:31 ` Hans Verkuil
@ 2017-10-09 22:17 ` Stanimir Varbanov
0 siblings, 0 replies; 6+ messages in thread
From: Stanimir Varbanov @ 2017-10-09 22:17 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Nicolas Dufresne, linux-media, linux-kernel, linux-arm-msm
Hans,
On 9.10.2017 15:31, Hans Verkuil wrote:
> On 09/10/17 14:24, Stanimir Varbanov wrote:
>> This fixes wrongly filled bytesused field of v4l2_plane structure
>> by include data_offset in the plane, Also fill data_offset and
>> bytesused for capture type of buffers only.
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>> drivers/media/platform/qcom/venus/venc.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
>> index 6f123a387cf9..9445ad492966 100644
>> --- a/drivers/media/platform/qcom/venus/venc.c
>> +++ b/drivers/media/platform/qcom/venus/venc.c
>> @@ -963,15 +963,16 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
>> if (!vbuf)
>> return;
>>
>> - vb = &vbuf->vb2_buf;
>> - vb->planes[0].bytesused = bytesused;
>> - vb->planes[0].data_offset = data_offset;
>> -
>> vbuf->flags = flags;
>>
>> if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>> + vb = &vbuf->vb2_buf;
>> + vb2_set_plane_payload(vb, 0, bytesused + data_offset);
>> + vb->planes[0].data_offset = data_offset;
>> vb->timestamp = timestamp_us * NSEC_PER_USEC;
>> vbuf->sequence = inst->sequence_cap++;
>> +
>> + WARN_ON(vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0));
>
> It's good to have this, but this really should never happen. Because if it is,
> then you'll have a memory overwrite. I hope the DMA engine will prevent this? >
> Just wondering how this works.
>
> The patch looks good otherwise, but that WARN_ON is a bit scary.
Infact it is not so scary as it looks like, the IOMMU will catch this
and generate a fault. So most probably we will never come to the WARN,
thus the warning is pointless so will delete it.
regards,
Stan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field
2017-10-09 12:24 ` [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field Stanimir Varbanov
2017-10-09 12:31 ` Hans Verkuil
@ 2017-10-09 23:41 ` Nicolas Dufresne
2017-10-10 7:52 ` [PATCH v2] " Stanimir Varbanov
2 siblings, 0 replies; 6+ messages in thread
From: Nicolas Dufresne @ 2017-10-09 23:41 UTC (permalink / raw)
To: Stanimir Varbanov, Hans Verkuil; +Cc: linux-media, linux-kernel, linux-arm-msm
I confirm this works properly now. This was tested with GStreamer with
the following command:
gst-launch-1.0 videotestsrc ! v4l2vp8enc ! v4l2vp8dec ! kmssink
And the following patch, which is work in progress to implement
data_offset.
https://gitlab.collabora.com/nicolas/gst-plugins-good/commit/aaedee9a37e396657568a70fc0110375e14fb4fa
Le lundi 09 octobre 2017 à 15:24 +0300, Stanimir Varbanov a écrit :
> This fixes wrongly filled bytesused field of v4l2_plane structure
> by include data_offset in the plane, Also fill data_offset and
> bytesused for capture type of buffers only.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
> drivers/media/platform/qcom/venus/venc.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 6f123a387cf9..9445ad492966 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -963,15 +963,16 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
> if (!vbuf)
> return;
>
> - vb = &vbuf->vb2_buf;
> - vb->planes[0].bytesused = bytesused;
> - vb->planes[0].data_offset = data_offset;
> -
> vbuf->flags = flags;
>
> if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> + vb = &vbuf->vb2_buf;
> + vb2_set_plane_payload(vb, 0, bytesused + data_offset);
> + vb->planes[0].data_offset = data_offset;
> vb->timestamp = timestamp_us * NSEC_PER_USEC;
> vbuf->sequence = inst->sequence_cap++;
> +
> + WARN_ON(vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0));
> } else {
> vbuf->sequence = inst->sequence_out++;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] media: venus: venc: fix bytesused v4l2_plane field
2017-10-09 12:24 ` [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field Stanimir Varbanov
2017-10-09 12:31 ` Hans Verkuil
2017-10-09 23:41 ` Nicolas Dufresne
@ 2017-10-10 7:52 ` Stanimir Varbanov
2 siblings, 0 replies; 6+ messages in thread
From: Stanimir Varbanov @ 2017-10-10 7:52 UTC (permalink / raw)
To: Hans Verkuil
Cc: Nicolas Dufresne, linux-media, linux-kernel, linux-arm-msm,
Stanimir Varbanov
This fixes wrongly filled bytesused field of v4l2_plane structure
by include data_offset in the plane, Also fill data_offset and
bytesused for capture type of buffers only.
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
v2: Just delete pointless WARN_ON.
drivers/media/platform/qcom/venus/venc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 6f123a387cf9..3fcf0e9b7b29 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -963,13 +963,12 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
if (!vbuf)
return;
- vb = &vbuf->vb2_buf;
- vb->planes[0].bytesused = bytesused;
- vb->planes[0].data_offset = data_offset;
-
vbuf->flags = flags;
if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+ vb = &vbuf->vb2_buf;
+ vb2_set_plane_payload(vb, 0, bytesused + data_offset);
+ vb->planes[0].data_offset = data_offset;
vb->timestamp = timestamp_us * NSEC_PER_USEC;
vbuf->sequence = inst->sequence_cap++;
} else {
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-10 7:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-09 12:24 [PATCH 1/2] media: venus: fix wrong size on dma_free Stanimir Varbanov
2017-10-09 12:24 ` [PATCH 2/2] media: venus: venc: fix bytesused v4l2_plane field Stanimir Varbanov
2017-10-09 12:31 ` Hans Verkuil
2017-10-09 22:17 ` Stanimir Varbanov
2017-10-09 23:41 ` Nicolas Dufresne
2017-10-10 7:52 ` [PATCH v2] " Stanimir Varbanov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).