2020-04-24 18:23:38

by Adrian Pop

[permalink] [raw]
Subject: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support

STM32f769-disco features a 4" MIPI DSI display: add support for it.

Signed-off-by: Adrian Pop <[email protected]>
---
arch/arm/boot/dts/stm32f746.dtsi | 34 ++++++++++++++++++
arch/arm/boot/dts/stm32f769-disco.dts | 50 +++++++++++++++++++++++++++
2 files changed, 84 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index 93c063796780..202bb6edc9f1 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -48,6 +48,19 @@ / {
#address-cells = <1>;
#size-cells = <1>;

+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ linux,dma {
+ compatible = "shared-dma-pool";
+ linux,dma-default;
+ no-map;
+ size = <0x10F000>;
+ };
+ };
+
clocks {
clk_hse: clk-hse {
#clock-cells = <0>;
@@ -75,6 +88,27 @@ clk_i2s_ckin: clk-i2s-ckin {
};

soc {
+ ltdc: display-controller@40016800 {
+ compatible = "st,stm32-ltdc";
+ reg = <0x40016800 0x200>;
+ interrupts = <88>, <89>;
+ resets = <&rcc STM32F7_APB2_RESET(LTDC)>;
+ clocks = <&rcc 1 CLK_LCD>;
+ clock-names = "lcd";
+ status = "disabled";
+ };
+
+ dsi: dsi@40016c00 {
+ compatible = "st,stm32-dsi";
+ reg = <0x40016c00 0x800>;
+ interrupts = <98>;
+ clocks = <&rcc 1 CLK_F769_DSI>, <&clk_hse>;
+ clock-names = "pclk", "ref";
+ resets = <&rcc STM32F7_APB2_RESET(DSI)>;
+ reset-names = "apb";
+ status = "disabled";
+ };
+
timer2: timer@40000000 {
compatible = "st,stm32-timer";
reg = <0x40000000 0x400>;
diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
index 1626e00bb2cb..30ebbc193e82 100644
--- a/arch/arm/boot/dts/stm32f769-disco.dts
+++ b/arch/arm/boot/dts/stm32f769-disco.dts
@@ -153,3 +153,53 @@ &usbotg_hs {
pinctrl-names = "default";
status = "okay";
};
+
+&dsi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dsi_in: endpoint {
+ remote-endpoint = <&ltdc_out_dsi>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dsi_out: endpoint {
+ remote-endpoint = <&dsi_in_panel>;
+ };
+ };
+
+ };
+
+ panel: panel {
+ compatible = "orisetech,otm8009a";
+ reg = <0>; /* dsi virtual channel (0..3) */
+ reset-gpios = <&gpioj 15 GPIO_ACTIVE_LOW>;
+ status = "okay";
+
+ port {
+ dsi_in_panel: endpoint {
+ remote-endpoint = <&dsi_out>;
+ };
+ };
+ };
+};
+
+&ltdc {
+ dma-ranges;
+ status = "okay";
+
+ port {
+ ltdc_out_dsi: endpoint {
+ remote-endpoint = <&dsi_in>;
+ };
+ };
+};
--
2.26.2


2020-04-27 08:31:42

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support

Hi Adrian

On 4/24/20 8:21 PM, Adrian Pop wrote:
> STM32f769-disco features a 4" MIPI DSI display: add support for it.
>
> Signed-off-by: Adrian Pop <[email protected]>
> ---

Commit title should be ARM: dts: stm32: ...

Can you explain a bit more in your commit message why do you use a
reserved memory pool for DMA and where this pool is located. (I assume
it's linked to a story of DMA and cache memory attribute on cortexM7...)

Did you try this configuration with XIP boot ?

regards
alex

> arch/arm/boot/dts/stm32f746.dtsi | 34 ++++++++++++++++++
> arch/arm/boot/dts/stm32f769-disco.dts | 50 +++++++++++++++++++++++++++
> 2 files changed, 84 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
> index 93c063796780..202bb6edc9f1 100644
> --- a/arch/arm/boot/dts/stm32f746.dtsi
> +++ b/arch/arm/boot/dts/stm32f746.dtsi
> @@ -48,6 +48,19 @@ / {
> #address-cells = <1>;
> #size-cells = <1>;
>
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + linux,dma {
> + compatible = "shared-dma-pool";
> + linux,dma-default;
> + no-map;
> + size = <0x10F000>;
> + };
> + };
> +
> clocks {
> clk_hse: clk-hse {
> #clock-cells = <0>;
> @@ -75,6 +88,27 @@ clk_i2s_ckin: clk-i2s-ckin {
> };
>
> soc {
> + ltdc: display-controller@40016800 {
> + compatible = "st,stm32-ltdc";
> + reg = <0x40016800 0x200>;
> + interrupts = <88>, <89>;
> + resets = <&rcc STM32F7_APB2_RESET(LTDC)>;
> + clocks = <&rcc 1 CLK_LCD>;
> + clock-names = "lcd";
> + status = "disabled";
> + };
> +
> + dsi: dsi@40016c00 {
> + compatible = "st,stm32-dsi";
> + reg = <0x40016c00 0x800>;
> + interrupts = <98>;
> + clocks = <&rcc 1 CLK_F769_DSI>, <&clk_hse>;
> + clock-names = "pclk", "ref";
> + resets = <&rcc STM32F7_APB2_RESET(DSI)>;
> + reset-names = "apb";
> + status = "disabled";
> + };
> +
> timer2: timer@40000000 {
> compatible = "st,stm32-timer";
> reg = <0x40000000 0x400>;
> diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
> index 1626e00bb2cb..30ebbc193e82 100644
> --- a/arch/arm/boot/dts/stm32f769-disco.dts
> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
> @@ -153,3 +153,53 @@ &usbotg_hs {
> pinctrl-names = "default";
> status = "okay";
> };
> +
> +&dsi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + dsi_in: endpoint {
> + remote-endpoint = <&ltdc_out_dsi>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + dsi_out: endpoint {
> + remote-endpoint = <&dsi_in_panel>;
> + };
> + };
> +
> + };
> +
> + panel: panel {
> + compatible = "orisetech,otm8009a";
> + reg = <0>; /* dsi virtual channel (0..3) */
> + reset-gpios = <&gpioj 15 GPIO_ACTIVE_LOW>;
> + status = "okay";
> +
> + port {
> + dsi_in_panel: endpoint {
> + remote-endpoint = <&dsi_out>;
> + };
> + };
> + };
> +};
> +
> +&ltdc {
> + dma-ranges;
> + status = "okay";
> +
> + port {
> + ltdc_out_dsi: endpoint {
> + remote-endpoint = <&dsi_in>;
> + };
> + };
> +};
>

2020-04-27 20:10:15

by Adrian Pop

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support

Added [email protected].

First, thank you all for taking a look at my changes!

Hello Alex,

On Mon, Apr 27, 2020 at 11:28 AM Alexandre Torgue
<[email protected]> wrote:
>
> Hi Adrian
>
> On 4/24/20 8:21 PM, Adrian Pop wrote:
> > STM32f769-disco features a 4" MIPI DSI display: add support for it.
> >
> > Signed-off-by: Adrian Pop <[email protected]>
> > ---
>
> Commit title should be ARM: dts: stm32: ...

Will fix in next version if that's ok.

>
> Can you explain a bit more in your commit message why do you use a
> reserved memory pool for DMA and where this pool is located. (I assume
> it's linked to a story of DMA and cache memory attribute on cortexM7...)

Need to look more into this, but if I remove it, /dev/fb0 is not
available anymore and I get a warning stating:
...
[drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[drm] Initialized stm 1.0.0 20170330 for 40016800.display-controller on minor 0
------------[ cut here ]------------
WARNING: CPU: 0 PID: 13 at arch/arm/mm/dma-mapping-nommu.c:50 0xc000b8ed
CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 5.6.0-next-20200412 #23
Hardware name: STM32 (Device Tree Support)
Workqueue: events 0xc014fa35
Function entered at [<c000b325>] from [<c000a487>]
...

When I looked in arch/arm/mm/dma-mapping-nommu.c:50, there is a comment stating:

/*
* dma_alloc_from_global_coherent() may fail because:
*
* - no consistent DMA region has been defined, so we can't
* continue.
* - there is no space left in consistent DMA region, so we
* only can fallback to generic allocator if we are
* advertised that consistency is not required.
*/

This is the reason I added the reserved-memory.

About the location, does it need to be hardcoded? On my board
(STM32F769I-Disco, tftp boot) in boot log I get:
...
Reserved memory: created DMA memory pool at 0xc0ef1000, size 1 MiB
OF: reserved mem: initialized node linux,dma, compatible id shared-dma-pool
...

>
> Did you try this configuration with XIP boot ?

I did not try with XIP. Currently loading zImage from tftp to memory.
Will try with XIP as well, and get back with feedback.

>
> regards
> alex
>
> > arch/arm/boot/dts/stm32f746.dtsi | 34 ++++++++++++++++++
> > arch/arm/boot/dts/stm32f769-disco.dts | 50 +++++++++++++++++++++++++++
> > 2 files changed, 84 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
> > index 93c063796780..202bb6edc9f1 100644
> > --- a/arch/arm/boot/dts/stm32f746.dtsi
> > +++ b/arch/arm/boot/dts/stm32f746.dtsi
> > @@ -48,6 +48,19 @@ / {
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > + reserved-memory {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges;
> > +
> > + linux,dma {
> > + compatible = "shared-dma-pool";
> > + linux,dma-default;
> > + no-map;
> > + size = <0x10F000>;
> > + };
> > + };
> > +
> > clocks {
> > clk_hse: clk-hse {
> > #clock-cells = <0>;
> > @@ -75,6 +88,27 @@ clk_i2s_ckin: clk-i2s-ckin {
> > };
> >
> > soc {
> > + ltdc: display-controller@40016800 {
> > + compatible = "st,stm32-ltdc";
> > + reg = <0x40016800 0x200>;
> > + interrupts = <88>, <89>;
> > + resets = <&rcc STM32F7_APB2_RESET(LTDC)>;
> > + clocks = <&rcc 1 CLK_LCD>;
> > + clock-names = "lcd";
> > + status = "disabled";
> > + };
> > +
> > + dsi: dsi@40016c00 {
> > + compatible = "st,stm32-dsi";
> > + reg = <0x40016c00 0x800>;
> > + interrupts = <98>;
> > + clocks = <&rcc 1 CLK_F769_DSI>, <&clk_hse>;
> > + clock-names = "pclk", "ref";
> > + resets = <&rcc STM32F7_APB2_RESET(DSI)>;
> > + reset-names = "apb";
> > + status = "disabled";
> > + };
> > +
> > timer2: timer@40000000 {
> > compatible = "st,stm32-timer";
> > reg = <0x40000000 0x400>;
> > diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
> > index 1626e00bb2cb..30ebbc193e82 100644
> > --- a/arch/arm/boot/dts/stm32f769-disco.dts
> > +++ b/arch/arm/boot/dts/stm32f769-disco.dts
> > @@ -153,3 +153,53 @@ &usbotg_hs {
> > pinctrl-names = "default";
> > status = "okay";
> > };
> > +
> > +&dsi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + status = "okay";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + dsi_in: endpoint {
> > + remote-endpoint = <&ltdc_out_dsi>;
> > + };
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > + dsi_out: endpoint {
> > + remote-endpoint = <&dsi_in_panel>;
> > + };
> > + };
> > +
> > + };
> > +
> > + panel: panel {
> > + compatible = "orisetech,otm8009a";
> > + reg = <0>; /* dsi virtual channel (0..3) */
> > + reset-gpios = <&gpioj 15 GPIO_ACTIVE_LOW>;
> > + status = "okay";
> > +
> > + port {
> > + dsi_in_panel: endpoint {
> > + remote-endpoint = <&dsi_out>;
> > + };
> > + };
> > + };
> > +};
> > +
> > +&ltdc {
> > + dma-ranges;

Need to remove this, not needed and causes a warning.

> > + status = "okay";
> > +
> > + port {
> > + ltdc_out_dsi: endpoint {
> > + remote-endpoint = <&dsi_in>;
> > + };
> > + };
> > +};
> >

Regards,
Adrian

2020-04-28 08:44:00

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support

Hi Adrian

On 4/27/20 10:05 PM, Adrian Pop wrote:
> Added [email protected].
>
> First, thank you all for taking a look at my changes!

no pb.

>
> Hello Alex,
>
> On Mon, Apr 27, 2020 at 11:28 AM Alexandre Torgue
> <[email protected]> wrote:
>>
>> Hi Adrian
>>
>> On 4/24/20 8:21 PM, Adrian Pop wrote:
>>> STM32f769-disco features a 4" MIPI DSI display: add support for it.
>>>
>>> Signed-off-by: Adrian Pop <[email protected]>
>>> ---
>>
>> Commit title should be ARM: dts: stm32: ...
>
> Will fix in next version if that's ok.
>
>>
>> Can you explain a bit more in your commit message why do you use a
>> reserved memory pool for DMA and where this pool is located. (I assume
>> it's linked to a story of DMA and cache memory attribute on cortexM7...)
>
> Need to look more into this, but if I remove it, /dev/fb0 is not
> available anymore and I get a warning stating:
> ...
> [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [drm] Initialized stm 1.0.0 20170330 for 40016800.display-controller on minor 0
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 13 at arch/arm/mm/dma-mapping-nommu.c:50 0xc000b8ed
> CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 5.6.0-next-20200412 #23
> Hardware name: STM32 (Device Tree Support)
> Workqueue: events 0xc014fa35
> Function entered at [<c000b325>] from [<c000a487>]
> ...
>
> When I looked in arch/arm/mm/dma-mapping-nommu.c:50, there is a comment stating:
>
> /*
> * dma_alloc_from_global_coherent() may fail because:
> *
> * - no consistent DMA region has been defined, so we can't
> * continue.
> * - there is no space left in consistent DMA region, so we
> * only can fallback to generic allocator if we are
> * advertised that consistency is not required.
> */
>
> This is the reason I added the reserved-memory.

Note that on cortexM7 DMA can't use cached memory. For this reason you
have to declare a dedicated memory area for DMA with no-cache attribute.
It is done thanks to a "linux,dma" node plus a kernel config:
CONFIG_ARM_MPU. I planed to declare this dedicated memeory region in
sram. Can you check if add it for the same reason I explain and check if
it works using sram ?



>
> About the location, does it need to be hardcoded? On my board
> (STM32F769I-Disco, tftp boot) in boot log I get:
> ...
> Reserved memory: created DMA memory pool at 0xc0ef1000, size 1 MiB
> OF: reserved mem: initialized node linux,dma, compatible id shared-dma-pool
> ...
>
>>
>> Did you try this configuration with XIP boot ?
>
> I did not try with XIP. Currently loading zImage from tftp to memory.
> Will try with XIP as well, and get back with feedback.

Ok thanks.

>
>>
>> regards
>> alex
>>
>>> arch/arm/boot/dts/stm32f746.dtsi | 34 ++++++++++++++++++
>>> arch/arm/boot/dts/stm32f769-disco.dts | 50 +++++++++++++++++++++++++++
>>> 2 files changed, 84 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
>>> index 93c063796780..202bb6edc9f1 100644
>>> --- a/arch/arm/boot/dts/stm32f746.dtsi
>>> +++ b/arch/arm/boot/dts/stm32f746.dtsi
>>> @@ -48,6 +48,19 @@ / {
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>>
>>> + reserved-memory {
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> + ranges;
>>> +
>>> + linux,dma {
>>> + compatible = "shared-dma-pool";
>>> + linux,dma-default;
>>> + no-map;
>>> + size = <0x10F000>;
>>> + };
>>> + };
>>> +
>>> clocks {
>>> clk_hse: clk-hse {
>>> #clock-cells = <0>;
>>> @@ -75,6 +88,27 @@ clk_i2s_ckin: clk-i2s-ckin {
>>> };
>>>
>>> soc {
>>> + ltdc: display-controller@40016800 {
>>> + compatible = "st,stm32-ltdc";
>>> + reg = <0x40016800 0x200>;
>>> + interrupts = <88>, <89>;
>>> + resets = <&rcc STM32F7_APB2_RESET(LTDC)>;
>>> + clocks = <&rcc 1 CLK_LCD>;
>>> + clock-names = "lcd";
>>> + status = "disabled";
>>> + };
>>> +
>>> + dsi: dsi@40016c00 {
>>> + compatible = "st,stm32-dsi";
>>> + reg = <0x40016c00 0x800>;
>>> + interrupts = <98>;
>>> + clocks = <&rcc 1 CLK_F769_DSI>, <&clk_hse>;
>>> + clock-names = "pclk", "ref";
>>> + resets = <&rcc STM32F7_APB2_RESET(DSI)>;
>>> + reset-names = "apb";
>>> + status = "disabled";
>>> + };
>>> +
>>> timer2: timer@40000000 {
>>> compatible = "st,stm32-timer";
>>> reg = <0x40000000 0x400>;
>>> diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
>>> index 1626e00bb2cb..30ebbc193e82 100644
>>> --- a/arch/arm/boot/dts/stm32f769-disco.dts
>>> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
>>> @@ -153,3 +153,53 @@ &usbotg_hs {
>>> pinctrl-names = "default";
>>> status = "okay";
>>> };
>>> +
>>> +&dsi {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + status = "okay";
>>> +
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + dsi_in: endpoint {
>>> + remote-endpoint = <&ltdc_out_dsi>;
>>> + };
>>> + };
>>> +
>>> + port@1 {
>>> + reg = <1>;
>>> + dsi_out: endpoint {
>>> + remote-endpoint = <&dsi_in_panel>;
>>> + };
>>> + };
>>> +
>>> + };
>>> +
>>> + panel: panel {
>>> + compatible = "orisetech,otm8009a";
>>> + reg = <0>; /* dsi virtual channel (0..3) */
>>> + reset-gpios = <&gpioj 15 GPIO_ACTIVE_LOW>;
>>> + status = "okay";
>>> +
>>> + port {
>>> + dsi_in_panel: endpoint {
>>> + remote-endpoint = <&dsi_out>;
>>> + };
>>> + };
>>> + };
>>> +};
>>> +
>>> +&ltdc {
>>> + dma-ranges;
>
> Need to remove this, not needed and causes a warning.
>
>>> + status = "okay";
>>> +
>>> + port {
>>> + ltdc_out_dsi: endpoint {
>>> + remote-endpoint = <&dsi_in>;
>>> + };
>>> + };
>>> +};
>>>
>
> Regards,
> Adrian
>

2020-04-28 09:07:34

by Vladimir Murzin

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support

Hi Adrian,

On 4/27/20 9:05 PM, Adrian Pop wrote:
> Added [email protected].
>
> First, thank you all for taking a look at my changes!
>
> Hello Alex,
>
> On Mon, Apr 27, 2020 at 11:28 AM Alexandre Torgue
> <[email protected]> wrote:
>>
>> Hi Adrian
>>
>> On 4/24/20 8:21 PM, Adrian Pop wrote:
>>> STM32f769-disco features a 4" MIPI DSI display: add support for it.
>>>
>>> Signed-off-by: Adrian Pop <[email protected]>
>>> ---
>>
>> Commit title should be ARM: dts: stm32: ...
>
> Will fix in next version if that's ok.
>
>>
>> Can you explain a bit more in your commit message why do you use a
>> reserved memory pool for DMA and where this pool is located. (I assume
>> it's linked to a story of DMA and cache memory attribute on cortexM7...)
>
> Need to look more into this, but if I remove it, /dev/fb0 is not
> available anymore and I get a warning stating:
> ...
> [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [drm] Initialized stm 1.0.0 20170330 for 40016800.display-controller on minor 0
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 13 at arch/arm/mm/dma-mapping-nommu.c:50 0xc000b8ed
> CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 5.6.0-next-20200412 #23
> Hardware name: STM32 (Device Tree Support)
> Workqueue: events 0xc014fa35
> Function entered at [<c000b325>] from [<c000a487>]
> ...
>
> When I looked in arch/arm/mm/dma-mapping-nommu.c:50, there is a comment stating:
>
> /*
> * dma_alloc_from_global_coherent() may fail because:
> *
> * - no consistent DMA region has been defined, so we can't
> * continue.
> * - there is no space left in consistent DMA region, so we
> * only can fallback to generic allocator if we are
> * advertised that consistency is not required.
> */
>
> This is the reason I added the reserved-memory.
>
> About the location, does it need to be hardcoded? On my board
> (STM32F769I-Disco, tftp boot) in boot log I get:
> ...
> Reserved memory: created DMA memory pool at 0xc0ef1000, size 1 MiB
> OF: reserved mem: initialized node linux,dma, compatible id shared-dma-pool
> ...
>

I'd recommend to place it at specific address, otherwise it will play badly with
CONFIG_MPU=y. MPU covers only single contiguous memblock (due to limitations
in number of available MPU regions), so placing DMA pool anywhere may result
in split of such contiguous memblock, as effect you may see that some memory
is not used. Usually, folks place DMA pool at the end of RAM.

>>
>> Did you try this configuration with XIP boot ?
>
> I did not try with XIP. Currently loading zImage from tftp to memory.
> Will try with XIP as well, and get back with feedback.
>

Bear in mind that with CONFIG_MPU=y XIP start address need to be aligned to 1M.

Cheers
Vladimir

2020-05-16 08:40:44

by Adrian Pop

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support

Hello all,

a bit of a delayed response here, but:

On Tue, Apr 28, 2020 at 11:39 AM Alexandre Torgue
<[email protected]> wrote:
>
> Hi Adrian
>
> On 4/27/20 10:05 PM, Adrian Pop wrote:
> > Added [email protected].
> >
> > First, thank you all for taking a look at my changes!
>
> no pb.
>
> >
> > Hello Alex,
> >
> > On Mon, Apr 27, 2020 at 11:28 AM Alexandre Torgue
> > <[email protected]> wrote:
> >>
> >> Hi Adrian
> >>
> >> On 4/24/20 8:21 PM, Adrian Pop wrote:
> >>> STM32f769-disco features a 4" MIPI DSI display: add support for it.
> >>>
> >>> Signed-off-by: Adrian Pop <[email protected]>
> >>> ---
> >>
> >> Commit title should be ARM: dts: stm32: ...
> >
> > Will fix in next version if that's ok.
> >
> >>
> >> Can you explain a bit more in your commit message why do you use a
> >> reserved memory pool for DMA and where this pool is located. (I assume
> >> it's linked to a story of DMA and cache memory attribute on cortexM7...)
> >
> > Need to look more into this, but if I remove it, /dev/fb0 is not
> > available anymore and I get a warning stating:
> > ...
> > [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> > [drm] Initialized stm 1.0.0 20170330 for 40016800.display-controller on minor 0
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 13 at arch/arm/mm/dma-mapping-nommu.c:50 0xc000b8ed
> > CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 5.6.0-next-20200412 #23
> > Hardware name: STM32 (Device Tree Support)
> > Workqueue: events 0xc014fa35
> > Function entered at [<c000b325>] from [<c000a487>]
> > ...
> >
> > When I looked in arch/arm/mm/dma-mapping-nommu.c:50, there is a comment stating:
> >
> > /*
> > * dma_alloc_from_global_coherent() may fail because:
> > *
> > * - no consistent DMA region has been defined, so we can't
> > * continue.
> > * - there is no space left in consistent DMA region, so we
> > * only can fallback to generic allocator if we are
> > * advertised that consistency is not required.
> > */
> >
> > This is the reason I added the reserved-memory.
>
> Note that on cortexM7 DMA can't use cached memory. For this reason you
> have to declare a dedicated memory area for DMA with no-cache attribute.
> It is done thanks to a "linux,dma" node plus a kernel config:
> CONFIG_ARM_MPU. I planed to declare this dedicated memeory region in
> sram. Can you check if add it for the same reason I explain and check if
> it works using sram ?
>

I did not have CONFIG_ARM_MPU enabled, enabled it now.

Just tried with SRAM:
reg = <0x20020000 0x60000>; /* SRAM1 368KB + SRAM2 16KB*/

but `arm_nommu_dma_alloc()` size parameter is 819200 (which is bigger
than the SRAM reserved memory), so the
`dma_alloc_from_global_coherent()` fails, so again I get the warning
stated above.

>
>
> >
> > About the location, does it need to be hardcoded? On my board
> > (STM32F769I-Disco, tftp boot) in boot log I get:
> > ...
> > Reserved memory: created DMA memory pool at 0xc0ef1000, size 1 MiB
> > OF: reserved mem: initialized node linux,dma, compatible id shared-dma-pool
> > ...
> >
> >>
> >> Did you try this configuration with XIP boot ?
> >
> > I did not try with XIP. Currently loading zImage from tftp to memory.
> > Will try with XIP as well, and get back with feedback.

Still trying to figure how to XIP :).

>
> Ok thanks.
>
> >
> >>
> >> regards
> >> alex
> >>
> >>> arch/arm/boot/dts/stm32f746.dtsi | 34 ++++++++++++++++++
> >>> arch/arm/boot/dts/stm32f769-disco.dts | 50 +++++++++++++++++++++++++++
> >>> 2 files changed, 84 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
> >>> index 93c063796780..202bb6edc9f1 100644
> >>> --- a/arch/arm/boot/dts/stm32f746.dtsi
> >>> +++ b/arch/arm/boot/dts/stm32f746.dtsi
> >>> @@ -48,6 +48,19 @@ / {
> >>> #address-cells = <1>;
> >>> #size-cells = <1>;
> >>>
> >>> + reserved-memory {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <1>;
> >>> + ranges;
> >>> +
> >>> + linux,dma {
> >>> + compatible = "shared-dma-pool";
> >>> + linux,dma-default;
> >>> + no-map;
> >>> + size = <0x10F000>;
> >>> + };
> >>> + };
> >>> +
> >>> clocks {
> >>> clk_hse: clk-hse {
> >>> #clock-cells = <0>;
> >>> @@ -75,6 +88,27 @@ clk_i2s_ckin: clk-i2s-ckin {
> >>> };
> >>>
> >>> soc {
> >>> + ltdc: display-controller@40016800 {
> >>> + compatible = "st,stm32-ltdc";
> >>> + reg = <0x40016800 0x200>;
> >>> + interrupts = <88>, <89>;
> >>> + resets = <&rcc STM32F7_APB2_RESET(LTDC)>;
> >>> + clocks = <&rcc 1 CLK_LCD>;
> >>> + clock-names = "lcd";
> >>> + status = "disabled";
> >>> + };
> >>> +
> >>> + dsi: dsi@40016c00 {
> >>> + compatible = "st,stm32-dsi";
> >>> + reg = <0x40016c00 0x800>;
> >>> + interrupts = <98>;
> >>> + clocks = <&rcc 1 CLK_F769_DSI>, <&clk_hse>;
> >>> + clock-names = "pclk", "ref";
> >>> + resets = <&rcc STM32F7_APB2_RESET(DSI)>;
> >>> + reset-names = "apb";
> >>> + status = "disabled";
> >>> + };
> >>> +
> >>> timer2: timer@40000000 {
> >>> compatible = "st,stm32-timer";
> >>> reg = <0x40000000 0x400>;
> >>> diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> index 1626e00bb2cb..30ebbc193e82 100644
> >>> --- a/arch/arm/boot/dts/stm32f769-disco.dts
> >>> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> @@ -153,3 +153,53 @@ &usbotg_hs {
> >>> pinctrl-names = "default";
> >>> status = "okay";
> >>> };
> >>> +
> >>> +&dsi {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> + status = "okay";
> >>> +
> >>> + ports {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + port@0 {
> >>> + reg = <0>;
> >>> + dsi_in: endpoint {
> >>> + remote-endpoint = <&ltdc_out_dsi>;
> >>> + };
> >>> + };
> >>> +
> >>> + port@1 {
> >>> + reg = <1>;
> >>> + dsi_out: endpoint {
> >>> + remote-endpoint = <&dsi_in_panel>;
> >>> + };
> >>> + };
> >>> +
> >>> + };
> >>> +
> >>> + panel: panel {
> >>> + compatible = "orisetech,otm8009a";
> >>> + reg = <0>; /* dsi virtual channel (0..3) */
> >>> + reset-gpios = <&gpioj 15 GPIO_ACTIVE_LOW>;
> >>> + status = "okay";
> >>> +
> >>> + port {
> >>> + dsi_in_panel: endpoint {
> >>> + remote-endpoint = <&dsi_out>;
> >>> + };
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> +&ltdc {
> >>> + dma-ranges;
> >
> > Need to remove this, not needed and causes a warning.
> >
> >>> + status = "okay";
> >>> +
> >>> + port {
> >>> + ltdc_out_dsi: endpoint {
> >>> + remote-endpoint = <&dsi_in>;
> >>> + };
> >>> + };
> >>> +};
> >>>
> >
> > Regards,
> > Adrian
> >

Regards,
Adrian