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);
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;}@@ -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;}
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;}@@ -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;}
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;}@@ -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;}
dri-devel@lists.freedesktop.org
-
Christian König -
guangming.cao＠mediatek.com