dmaengine: axi-dmac: check cache coherency register

[tausen]

Marking the DMA as cache coherent (dma-coherent in devicetree) is only safe with versions of axi_dmac that have this feature enabled.

Related to https://github.com/analogdevicesinc/hdl/pull/925.

Example devicetree using the dma-coherent property:

mydma: dma@82510000 {
	compatible = "adi,axi-dmac-1.00.a";
	reg = <0x0 0x0082510000 0x0 0x1000>;
	interrupt-parent = <&gic>;
	interrupts = <0 111 4>;
	clock-names = "s_axi_aclk";
	clocks = <&zynqmp_clk 71>;
	#dma-cells = <1>;
	dma-coherent;

	adi,channels {
		#size-cells = <0>;
		#address-cells = <1>;

		dma-channel@0 {
			reg = <0>;
			adi,source-bus-width = <64>;
			adi,source-bus-type = <AXI_DMAC_TYPE_AXI_STREAM>;
			adi,destination-bus-width = <128>;
			adi,destination-bus-type = <AXI_DMAC_TYPE_AXI_MM>;
		};
	};
};

Data can be corrupt if used with a version of axi-dmac that does not support this feature.

Thoughts? Anything I've missed?

[tausen]

Thanks for the input!

Did you had to change your boot image to enable broadcasting memory transactions in the inner shareable domain (as stated in xilinx confluence page)? I would expect this to be needed.

No, I made no changes to my boot image. I did see the comment on Xilinx' wiki page, but since coherency seems to work without any other changes, I didn't pursue it further. There is another mention here:

https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/1027702787/Linux+DMA+From+User+Space+2.0#CCI-Enablement

where it seems that some default settings have changed since Vivado 2020.1. Perhaps changes are only required when generating the boot image with an earlier version of Vivado (I'm using Vivado 2021.1)?

this looks like a change that should hit upstream kernel (this driver is supported upstream). How comfortable are you in trying to send a patch to mainline?

Sure, I can give it a go. Any pointers on where to begin are welcome.

[nunojsa]

No, I made no changes to my boot image. I did see the comment on Xilinx' wiki page, but since coherency seems to work without any other changes, I didn't pursue it further. There is another mention here:

https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/1027702787/Linux+DMA+From+User+Space+2.0#CCI-Enablement

where it seems that some default settings have changed since Vivado 2020.1. Perhaps changes are only required when generating the boot image with an earlier version of Vivado (I'm using Vivado 2021.1)?

Hmm I see. Then it makes sense no other change is needed :)

Sure, I can give it a go. Any pointers on where to begin are welcome.

https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html

Nice, someone might have some remarks that I'm failing to see....

[tausen]

Perhaps changes are only required when generating the boot image with an earlier version of Vivado (I'm using Vivado 2021.1)?

I did some further digging and it does seem brdc_inner of lpd_apu (LPD_SLCR) (ref) is 0 unless I use a boot image built with AFIx coherency enabled through the Vivado GUI (Advanced Configuration -> CCI Enablement -> HPCx). I've been testing without it set until now, but perhaps my testcase just happens to always succeed. I'm not sure what to expect if broadcasting inner is disabled and I assume cache coherency anyway...

https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html

Thanks!

[nunojsa]

I'm not sure what to expect if broadcasting inner is disabled and I assume cache coherency anyway...

I'm also not sure but I would assume that the hw won't be coherent after all so that data corruption is possible (since the linux DMA API is doing things based on wrong assumptions)....

[nunojsa]

@tausen did you end up upstreaming this? If you're not comfortable with it, I can take care of it...

[tausen]

@nunojsa not yet, no - I got sidetracked and will be at least for another week or two. If you have the time, don't hold yourself back on my account :) Otherwise I'll come back to it soon™.

[nunojsa]

Otherwise I'll come back to it soon™

Alright, I'll hold a bit more. My only rush here is to merge/close this PR :)

[tausen]

Has been merged upstream now: https://github.com/torvalds/linux/commit/9327c7e7539371c6f202303d5539afbae006ec72

I guess we can safely close this PR?

[nunojsa]

Yeah, either closing and waiting to get this when upgrading or merging it now... I will probably merge it as this is fairly simple.