| Message ID | 1461853196-15599-3-git-send-email-abrodkin@synopsys.com |
|---|---|
| State | New |
| Headers | show |
On Thursday 28 April 2016 07:49 PM, Alexey Brodkin wrote: > Even though for AXS101 (which sorts ARC770 CPU) IOC is not > an option for a sake of keeping one DT description for the > base-board (axs10x_mb.dtsi) we're still defining reserved > memory location in the very end of DDR. > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Acked-by: Vineet Gupta <vgupta@synopsys.com>
On Tuesday 31 May 2016 04:59 PM, Alexey Brodkin wrote: > Hi Vineet! > > On Thu, 2016-05-26 at 09:29 +0000, Vineet Gupta wrote: >> On Thursday 26 May 2016 02:38 PM, Alexey Brodkin wrote: >>> 1) IOC aperture is set to cover 0x8000_0000-0xA000_0000 >>> 2) FB area is 0xBE00_0000 - 0xC000_0000 >> So if someone rebuilds AXS103 with 1GB of memory in DT then they are screwed >> because 0xa000_0000 to 0xBDFF_FFFF will not be io-coherent. A DMA buffer in that >> range will be corrupt. >> >> The point is this IOC window programming must be tied to amount of memory in DT >> and not assume 512MB >> This is needed for supporting other customer platforms ! > Ok so actually what I think worth doing here is reverse our current logic. > As of today we treat all memory except that special "reserved" region as covered > by IOC. But if we want to have more robust IOC usage IMHO it would be better to > specify dedicated "reserved area" for IOC and everything else let go outside of IOC > aperture. Not really - we want to utilize hardware capabilities to maximum - not minimum - as in exception should be when IOC can't be used and not the other way around. > This approach will allow: > [1] Having 1 known "reserved area" in DT parse its start and length and according to > it set IOC. Moreover here we'll do checks on proper start and size settings. > Remember IOc aperture should be 4kB aligned and size is set as > 2^(SIZE+2)kB (which is 4*2^SIZE kB). > [2] Remove possible side-effects (if there're any - and that's quite interesting to > check) that could be caused by memory operations not related to peripherals but > still falling in IOC aperture and thus somehow additionally loading IOC snooper > logic. I'm not sure what you imply by point #2 above. > Only downside (which might not be a downside really) we'll need to specify IOC-covered > memory for each peripheral that we want to use DMA. Well and probably not all drivers > support memory allocation from reserved memory blocks, remember my commit that adds > this feature to ARC PGU. As of today only ARM HDLCD and ARC PGU utilize > of_reserved_mem_device_init(). Thats exactly my point above. We need to add for exception and not common case. > Now thinking a bit more about this implementation I understood that we'll need to > improve our DMA cache ops so different ops could be used for different memory areas. > As of today if IOC is enabled we don't do any cache ops for DMAed data and fortunately > PGU's frame-buffer is used as uncached so no cache ops are required. But if for some > reason another peripheral is put outside IOC aperture we'll definitely need to do > explicit cache flush/invalidation on its data buffers. I really like this idea - this will certainly be needed for silicon implementations which will likely have per peripheral IOC setting (such as routing to IOC port or not) in addition to the blanket Core setting of IOC aperture. Thus we can use that per device dma op hooks to do this additional ioc foo. In the mean time, the pressing issue is potentially ticking time bomb of AXS103 broken (IOC + PGU + 1GB DDR) - let us add a boot time check in platform code to panic if that is the case. -Vineet
diff --git a/arch/arc/boot/dts/axc001.dtsi b/arch/arc/boot/dts/axc001.dtsi index 420dcfd..262496a 100644 --- a/arch/arc/boot/dts/axc001.dtsi +++ b/arch/arc/boot/dts/axc001.dtsi @@ -93,8 +93,26 @@ memory { #address-cells = <1>; #size-cells = <1>; - ranges = <0x00000000 0x80000000 0x40000000>; + ranges = <0x00000000 0x80000000 0x20000000>; device_type = "memory"; - reg = <0x80000000 0x20000000>; /* 512MiB */ + reg = <0x80000000 0x1b000000>; /* (512 - 32) MiB */ + }; + + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + /* + * We just move frame buffer area to the very end of + * available DDR. And even though in case of ARC770 there's + * no strict requirement for a frame-buffer to be in any + * particular location it allows us to use the same + * base board's DT node for ARC PGU as for ARc HS38. + */ + frame_buffer: frame_buffer@9e000000 { + compatible = "shared-dma-pool"; + reg = <0x9e000000 0x2000000>; + no-map; + }; }; }; diff --git a/arch/arc/boot/dts/axc003.dtsi b/arch/arc/boot/dts/axc003.dtsi index f90fadf..35ece04 100644 --- a/arch/arc/boot/dts/axc003.dtsi +++ b/arch/arc/boot/dts/axc003.dtsi @@ -100,4 +100,18 @@ device_type = "memory"; reg = <0x80000000 0x20000000>; /* 512MiB */ }; + + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + /* + * Move frame buffer out of IOC aperture (0x8z-0xAz). + */ + frame_buffer: frame_buffer@be000000 { + compatible = "shared-dma-pool"; + reg = <0xbe000000 0x2000000>; + no-map; + }; + }; }; diff --git a/arch/arc/boot/dts/axc003_idu.dtsi b/arch/arc/boot/dts/axc003_idu.dtsi index 06a9f29..df9ddb6 100644 --- a/arch/arc/boot/dts/axc003_idu.dtsi +++ b/arch/arc/boot/dts/axc003_idu.dtsi @@ -123,4 +123,18 @@ device_type = "memory"; reg = <0x80000000 0x20000000>; /* 512MiB */ }; + + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + /* + * Move frame buffer out of IOC aperture (0x8z-0xAz). + */ + frame_buffer: frame_buffer@be000000 { + compatible = "shared-dma-pool"; + reg = <0xbe000000 0x2000000>; + no-map; + }; + }; }; diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi index 823f15c..64b063d 100644 --- a/arch/arc/boot/dts/axs10x_mb.dtsi +++ b/arch/arc/boot/dts/axs10x_mb.dtsi @@ -283,7 +283,7 @@ encoder-slave = <&adv7511>; clocks = <&pguclk>; clock-names = "pxlclk"; - + memory-region = <&frame_buffer>; port { pgu_output: endpoint { remote-endpoint = <&adv7511_input>;
Allocation of a frame buffer memory in a special memory region allows bypassing of so-called IO Coherency aperture which is typically set as a range 0x8z-0xAz. I.e. all data traffic to PGU bypasses IO Coherency block and saves its bandwidth for other peripherals. Even though for AXS101 (which sorts ARC770 CPU) IOC is not an option for a sake of keeping one DT description for the base-board (axs10x_mb.dtsi) we're still defining reserved memory location in the very end of DDR. Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: Vineet Gupta <vgupta@synopsys.com> Cc: devicetree@vger.kernel.org --- Changes v1 -> v2: * Reserved memory size bumped from 16Mb to 32Mb. Given the corner case 1920x1080x24bpp and tripl-buffering we'll need ~18Mb in the future so let's reserve 32Mb today and don't think about that any more. * Reserved area in AXS103 boards moved to the very end of the first Gb. Even though we use only 512Mb for kernel on AXS103 board we have 1Gb of DDR really. So let's move reserved area in the very end of available mamory. This way we'll be able to keep this mapping when we'll want to use > 512Mb in the kernel. * And while at it correct "ranges" value for AXS101 board: we do have only 512 Mb of DDR on the board so let's not temp users to try to use more. arch/arc/boot/dts/axc001.dtsi | 22 ++++++++++++++++++++-- arch/arc/boot/dts/axc003.dtsi | 14 ++++++++++++++ arch/arc/boot/dts/axc003_idu.dtsi | 14 ++++++++++++++ arch/arc/boot/dts/axs10x_mb.dtsi | 2 +- 4 files changed, 49 insertions(+), 3 deletions(-)