* [PATCH] dma-buf: heaps: remove duplicated cache sync
@ 2021-08-30 10:01 guangming.cao
2021-08-30 10:16 ` Christian König
0 siblings, 1 reply; 4+ messages in thread
From: guangming.cao @ 2021-08-30 10:01 UTC (permalink / raw)
To: Sumit Semwal, Benjamin Gaignard, Liam Mark, Laura Abbott,
Brian Starkey, John Stultz, Christian König,
Matthias Brugger, open list:DMA-BUF HEAPS FRAMEWORK,
open list:DMA-BUF HEAPS FRAMEWORK,
moderated list:DMA-BUF HEAPS FRAMEWORK, open list,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Cc: wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
Current flow, one dmabuf maybe call cache sync many times if
it has beed mapped more than one time.
Is there any case that attachments of one dmabuf will points to
different memory? If not, seems do sync only one time is more better.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/heaps/system_heap.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..909ef652a8c8 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -162,9 +162,10 @@ static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
list_for_each_entry(a, &buffer->attachments, list) {
- if (!a->mapped)
- continue;
- dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
+ if (a->mapped) {
+ dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
+ break;
+ }
}
mutex_unlock(&buffer->lock);
@@ -183,9 +184,10 @@ static int system_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
flush_kernel_vmap_range(buffer->vaddr, buffer->len);
list_for_each_entry(a, &buffer->attachments, list) {
- if (!a->mapped)
- continue;
- dma_sync_sgtable_for_device(a->dev, a->table, direction);
+ if (!a->mapped) {
+ dma_sync_sgtable_for_device(a->dev, a->table, direction);
+ break;
+ }
}
mutex_unlock(&buffer->lock);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] dma-buf: heaps: remove duplicated cache sync
2021-08-30 10:01 [PATCH] dma-buf: heaps: remove duplicated cache sync guangming.cao
@ 2021-08-30 10:16 ` Christian König
2021-08-31 3:44 ` guangming.cao
0 siblings, 1 reply; 4+ messages in thread
From: Christian König @ 2021-08-30 10:16 UTC (permalink / raw)
To: guangming.cao, Sumit Semwal, Benjamin Gaignard, Liam Mark,
Laura Abbott, Brian Starkey, John Stultz, Matthias Brugger,
open list:DMA-BUF HEAPS FRAMEWORK,
open list:DMA-BUF HEAPS FRAMEWORK,
moderated list:DMA-BUF HEAPS FRAMEWORK, open list,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Cc: wsd_upstream
Am 30.08.21 um 12:01 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> Current flow, one dmabuf maybe call cache sync many times if
> it has beed mapped more than one time.
Well I'm not an expert on DMA heaps, but this will most likely not work
correctly.
> Is there any case that attachments of one dmabuf will points to
> different memory? If not, seems do sync only one time is more better.
I think that this can happen, yes.
Christian.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/heaps/system_heap.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..909ef652a8c8 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -162,9 +162,10 @@ static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
>
> list_for_each_entry(a, &buffer->attachments, list) {
> - if (!a->mapped)
> - continue;
> - dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
> + if (a->mapped) {
> + dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
> + break;
> + }
> }
> mutex_unlock(&buffer->lock);
>
> @@ -183,9 +184,10 @@ static int system_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
> flush_kernel_vmap_range(buffer->vaddr, buffer->len);
>
> list_for_each_entry(a, &buffer->attachments, list) {
> - if (!a->mapped)
> - continue;
> - dma_sync_sgtable_for_device(a->dev, a->table, direction);
> + if (!a->mapped) {
> + dma_sync_sgtable_for_device(a->dev, a->table, direction);
> + break;
> + }
> }
> mutex_unlock(&buffer->lock);
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dma-buf: heaps: remove duplicated cache sync
2021-08-30 10:16 ` Christian König
@ 2021-08-31 3:44 ` guangming.cao
2021-08-31 6:31 ` Christian König
0 siblings, 1 reply; 4+ messages in thread
From: guangming.cao @ 2021-08-31 3:44 UTC (permalink / raw)
To: christian.koenig
Cc: Brian.Starkey, benjamin.gaignard, dri-devel, guangming.cao,
john.stultz, labbott, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, lmark, matthias.bgg,
sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
> Am 30.08.21 um 12:01 schrieb guangming.cao@mediatek.com:
> > From: Guangming Cao <Guangming.Cao@mediatek.com>
> >
> > Current flow, one dmabuf maybe call cache sync many times if
> > it has beed mapped more than one time.
>
> Well I'm not an expert on DMA heaps, but this will most likely not work
> correctly.
>
All attachments of one dmabuf will add into a list, I think it means dmabuf
supports map more than one time. Could you tell me more about it?
> > Is there any case that attachments of one dmabuf will points to
> > different memory? If not, seems do sync only one time is more better.
>
> I think that this can happen, yes.
>
> Christian.
>
Seems it's a very special case on Android, if you don't mind, could you
tell me more about it?
>
> >
> > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> > ---
> > drivers/dma-buf/heaps/system_heap.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> > index 23a7e74ef966..909ef652a8c8 100644
> > --- a/drivers/dma-buf/heaps/system_heap.c
> > +++ b/drivers/dma-buf/heaps/system_heap.c
> > @@ -162,9 +162,10 @@ static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
> >
> > list_for_each_entry(a, &buffer->attachments, list) {
> > - if (!a->mapped)
> > - continue;
> > - dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
> > + if (a->mapped) {
> > + dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
> > + break;
> > + }
> > }
> > mutex_unlock(&buffer->lock);
> >
> > @@ -183,9 +184,10 @@ static int system_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
> > flush_kernel_vmap_range(buffer->vaddr, buffer->len);
> >
> > list_for_each_entry(a, &buffer->attachments, list) {
> > - if (!a->mapped)
> > - continue;
> > - dma_sync_sgtable_for_device(a->dev, a->table, direction);
> > + if (!a->mapped) {
> > + dma_sync_sgtable_for_device(a->dev, a->table, direction);
> > + break;
> > + }
> > }
> > mutex_unlock(&buffer->lock);
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dma-buf: heaps: remove duplicated cache sync
2021-08-31 3:44 ` guangming.cao
@ 2021-08-31 6:31 ` Christian König
0 siblings, 0 replies; 4+ messages in thread
From: Christian König @ 2021-08-31 6:31 UTC (permalink / raw)
To: guangming.cao
Cc: Brian.Starkey, benjamin.gaignard, dri-devel, john.stultz, labbott,
linaro-mm-sig, linux-arm-kernel, linux-kernel, linux-media,
linux-mediatek, lmark, matthias.bgg, sumit.semwal, wsd_upstream
Am 31.08.21 um 05:44 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
>> Am 30.08.21 um 12:01 schrieb guangming.cao@mediatek.com:
>>> From: Guangming Cao <Guangming.Cao@mediatek.com>
>>>
>>> Current flow, one dmabuf maybe call cache sync many times if
>>> it has beed mapped more than one time.
>> Well I'm not an expert on DMA heaps, but this will most likely not work
>> correctly.
>>
> All attachments of one dmabuf will add into a list, I think it means dmabuf
> supports map more than one time. Could you tell me more about it?
Yes, that's correct and all of those needs to be synced as far as I know.
See the dma_sync_sgtable_for_cpu() is intentionally for each SG table
given out.
>>> Is there any case that attachments of one dmabuf will points to
>>> different memory? If not, seems do sync only one time is more better.
>> I think that this can happen, yes.
>>
>> Christian.
>>
> Seems it's a very special case on Android, if you don't mind, could you
> tell me more about it?
That might be the case, nevertheless this change here is illegal from
the DMA API point of view as far as I can see.
Regards,
Christian.
>
>>> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
>>> ---
>>> drivers/dma-buf/heaps/system_heap.c | 14 ++++++++------
>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
>>> index 23a7e74ef966..909ef652a8c8 100644
>>> --- a/drivers/dma-buf/heaps/system_heap.c
>>> +++ b/drivers/dma-buf/heaps/system_heap.c
>>> @@ -162,9 +162,10 @@ static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>> invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
>>>
>>> list_for_each_entry(a, &buffer->attachments, list) {
>>> - if (!a->mapped)
>>> - continue;
>>> - dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
>>> + if (a->mapped) {
>>> + dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
>>> + break;
>>> + }
>>> }
>>> mutex_unlock(&buffer->lock);
>>>
>>> @@ -183,9 +184,10 @@ static int system_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>>> flush_kernel_vmap_range(buffer->vaddr, buffer->len);
>>>
>>> list_for_each_entry(a, &buffer->attachments, list) {
>>> - if (!a->mapped)
>>> - continue;
>>> - dma_sync_sgtable_for_device(a->dev, a->table, direction);
>>> + if (!a->mapped) {
>>> + dma_sync_sgtable_for_device(a->dev, a->table, direction);
>>> + break;
>>> + }
>>> }
>>> mutex_unlock(&buffer->lock);
>>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-31 6:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-30 10:01 [PATCH] dma-buf: heaps: remove duplicated cache sync guangming.cao
2021-08-30 10:16 ` Christian König
2021-08-31 3:44 ` guangming.cao
2021-08-31 6:31 ` Christian König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox