[2/2,v2] ARC: [axs10x] Specify reserved memory for frame buffer
diff mbox

Message ID 1461853196-15599-3-git-send-email-abrodkin@synopsys.com
State New
Headers show

Commit Message

Alexey Brodkin April 28, 2016, 2:19 p.m. UTC
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(-)

Comments

Vineet Gupta April 29, 2016, 10:49 a.m. UTC | #1
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>
Vineet Gupta June 2, 2016, 7:35 a.m. UTC | #2
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

Patch
diff mbox

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>;