Re: [PATCH 2/4] arm64: dts: qcom: sc7280: Add dma-coherent property into venus node

[Vishnu Reddy <busanna.reddy@oss.qualcomm.com>]

On 8/14/2026 3:56 PM, Krzysztof Kozlowski wrote:
> On 14/08/2026 12:24, Vishnu Reddy wrote:
>> On 8/14/2026 3:26 PM, Krzysztof Kozlowski wrote:
>>> On 14/08/2026 11:49, Vishnu Reddy wrote:
>>>> On 8/14/2026 12:01 PM, Krzysztof Kozlowski wrote:
>>>>> On 14/08/2026 07:29, Vishnu Reddy wrote:
>>>>>> On 8/13/2026 2:49 PM, Krzysztof Kozlowski wrote:
>>>>>>> On Sat, Aug 01, 2026 at 01:07:28PM +0530, Vishnu Reddy wrote:
>>>>>>>> While testing with some higher resolution clips, the venus hardware
>>>>>>>> triggers a fault due to wrong input data being received. Corruption
>>>>>>>> was also observed in the captured output when the client dumped it
>>>>>>>> to a file.
>>>>>>>>
>>>>>>>> On debugging, this was traced to the venus node not declaring
>>>>>>>> dma-coherent. As a result, DMA buffers shared between the CPU and the
>>>>>>>> venus video hardware/controller are not guaranteed to be I/O coherent:
>>>>>>>> CPU writes to an input buffer can remain in CPU caches without being
>>>>>>>> visible to the video hardware when it reads the same buffer, so the
>>>>>>>> hardware receives input data that does not match what the CPU wrote.
>>>>>>>> Likewise, on the capture path, data written by the video hardware to
>>>>>>>> the output buffer may not be visible to the CPU, so the client reads
>>>>>>>> stale or partial data, resulting in corruption.
>>>>>>>>
>>>>>>>> Add the dma-coherent property to the venus node so that DMA buffers
>>>>>>>> shared between the CPU and the video hardware and controller remain
>>>>>>>> coherent.
>>>>>>>>
>>>>>>>> Fixes: 37613aee2179 ("arm64: dts: qcom: sc7280: Add venus DT node")
>>>>>>> Really does not look like a fix. How skipping a cache sync can fix
>>>>>>> anything exactly?
>>>>>> dma-coherent tells the kernel that this device DMA path is hardware-coherent,
>>>>>> so the SMMU maps its buffers as cacheable. That attribute is what causes the
>>>>>> coherent interconnect to snoop CPU cache lines on every VPU transaction, so
>>>>>> hardware keeps CPU caches and VPU-visible memory in sync — which is why the
>>>>>> dma-coherent property lets us skip the unnecessary cache clean/invalidate.
>>>>> You just repeated what I wrote. What for?
>>>>>
>>>>>> If a device doesn't support I/O coherence, its buffers would instead be mapped
>>>>>> non-cacheable, no snooping would occur, and the driver would need explicit
>>>>>> dma_sync_*() calls at each transition. Venus on SC7280 does support I/O
>>>>>> coherence, so dma-coherent describes that existing hardware capability to
>>>>>> the DMA subsystem.
>>>>> How does this anyhow explain your commit msg? Are you saying driver is
>>>>> buggy that it misses sync calls?
>>>> No, driver is not buggy here. dma-coherent flag is missed in the dt node.
>>> Again, no... or explain why. Missing dma-coherent is not a fix, but
>>> rather optimisation, but you claimed it is a fix. Explain why.
>> Is the dma-coherent flag only for skipping cache sync?
>> I might be repeating here — it enables the hardware level snoop between CPU
>> caches and DMA memory. Without dma-coherent, the CPU cache and the memory
>> which VPU accesses aren't in sync at the hardware level, which is what
> They are in sync, because driver explicitly MUST sync it.

No, currently driver is not calling the dma_sync*() explicitly.
It won't be in sync. That's why dma-coherent flag added to keep the
CPU cache and DMA memory in sync.
Will you suggest to sync from the driver explicitly?

If we call dma_sync*() in driver, will you recommend to add the
dma-coherent flag or not in the dt node?

> You cannot have uncoherent accesses without proper DMA cache syncing
> operations.
>
>> caused the corruption. So dma-coherent is the proper fix here, and that's
>> why I've mentioned it as a fix. Do you have any opinion on how you'd prefer
>> to handle this? Please share.
> As I told you, it is not a fix and provide arguments why.
>
>
> Best regards,
> Krzysztof
>