* [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support
@ 2020-04-24 18:21 Adrian Pop
2020-04-27 8:28 ` Alexandre Torgue
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Pop @ 2020-04-24 18:21 UTC (permalink / raw)
To: Maxime Coquelin, Alexandre Torgue, Rob Herring
Cc: linux-stm32, linux-arm-kernel, devicetree, linux-kernel,
Adrian Pop
STM32f769-disco features a 4" MIPI DSI display: add support for it.
Signed-off-by: Adrian Pop <pop.adrian61@gmail.com>
---
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 = <<dc_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>;
+ };
+ };
+ };
+};
+
+<dc {
+ dma-ranges;
+ status = "okay";
+
+ port {
+ ltdc_out_dsi: endpoint {
+ remote-endpoint = <&dsi_in>;
+ };
+ };
+};
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support
2020-04-24 18:21 [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support Adrian Pop
@ 2020-04-27 8:28 ` Alexandre Torgue
2020-04-27 20:05 ` Adrian Pop
0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Torgue @ 2020-04-27 8:28 UTC (permalink / raw)
To: Adrian Pop, Maxime Coquelin, Rob Herring
Cc: linux-stm32, linux-arm-kernel, devicetree, linux-kernel
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 <pop.adrian61@gmail.com>
> ---
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 = <<dc_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>;
> + };
> + };
> + };
> +};
> +
> +<dc {
> + dma-ranges;
> + status = "okay";
> +
> + port {
> + ltdc_out_dsi: endpoint {
> + remote-endpoint = <&dsi_in>;
> + };
> + };
> +};
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support
2020-04-27 8:28 ` Alexandre Torgue
@ 2020-04-27 20:05 ` Adrian Pop
2020-04-28 8:39 ` Alexandre Torgue
2020-04-28 9:05 ` Vladimir Murzin
0 siblings, 2 replies; 6+ messages in thread
From: Adrian Pop @ 2020-04-27 20:05 UTC (permalink / raw)
To: Alexandre Torgue, Lee Jones
Cc: Maxime Coquelin, Rob Herring, linux-stm32, linux-arm-kernel,
devicetree, linux-kernel
Added lee.jones@linaro.org.
First, thank you all for taking a look at my changes!
Hello Alex,
On Mon, Apr 27, 2020 at 11:28 AM Alexandre Torgue
<alexandre.torgue@st.com> 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 <pop.adrian61@gmail.com>
> > ---
>
> 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 = <<dc_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>;
> > + };
> > + };
> > + };
> > +};
> > +
> > +<dc {
> > + 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support
2020-04-27 20:05 ` Adrian Pop
@ 2020-04-28 8:39 ` Alexandre Torgue
2020-05-16 8:39 ` Adrian Pop
2020-04-28 9:05 ` Vladimir Murzin
1 sibling, 1 reply; 6+ messages in thread
From: Alexandre Torgue @ 2020-04-28 8:39 UTC (permalink / raw)
To: Adrian Pop, Lee Jones
Cc: Maxime Coquelin, Rob Herring, linux-stm32, linux-arm-kernel,
devicetree, linux-kernel
Hi Adrian
On 4/27/20 10:05 PM, Adrian Pop wrote:
> Added lee.jones@linaro.org.
>
> 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
> <alexandre.torgue@st.com> 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 <pop.adrian61@gmail.com>
>>> ---
>>
>> 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 = <<dc_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>;
>>> + };
>>> + };
>>> + };
>>> +};
>>> +
>>> +<dc {
>>> + 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support
2020-04-27 20:05 ` Adrian Pop
2020-04-28 8:39 ` Alexandre Torgue
@ 2020-04-28 9:05 ` Vladimir Murzin
1 sibling, 0 replies; 6+ messages in thread
From: Vladimir Murzin @ 2020-04-28 9:05 UTC (permalink / raw)
To: Adrian Pop, Alexandre Torgue, Lee Jones
Cc: devicetree, linux-kernel, Rob Herring, Maxime Coquelin,
linux-stm32, linux-arm-kernel
Hi Adrian,
On 4/27/20 9:05 PM, Adrian Pop wrote:
> Added lee.jones@linaro.org.
>
> First, thank you all for taking a look at my changes!
>
> Hello Alex,
>
> On Mon, Apr 27, 2020 at 11:28 AM Alexandre Torgue
> <alexandre.torgue@st.com> 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 <pop.adrian61@gmail.com>
>>> ---
>>
>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support
2020-04-28 8:39 ` Alexandre Torgue
@ 2020-05-16 8:39 ` Adrian Pop
0 siblings, 0 replies; 6+ messages in thread
From: Adrian Pop @ 2020-05-16 8:39 UTC (permalink / raw)
To: Alexandre Torgue
Cc: Lee Jones, Maxime Coquelin, Rob Herring, linux-stm32,
linux-arm-kernel, devicetree, linux-kernel
Hello all,
a bit of a delayed response here, but:
On Tue, Apr 28, 2020 at 11:39 AM Alexandre Torgue
<alexandre.torgue@st.com> wrote:
>
> Hi Adrian
>
> On 4/27/20 10:05 PM, Adrian Pop wrote:
> > Added lee.jones@linaro.org.
> >
> > 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
> > <alexandre.torgue@st.com> 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 <pop.adrian61@gmail.com>
> >>> ---
> >>
> >> 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 = <<dc_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>;
> >>> + };
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> +<dc {
> >>> + 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-16 8:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-24 18:21 [PATCH 2/2] arm: dts: stm32f769-disco: Enable MIPI DSI display support Adrian Pop
2020-04-27 8:28 ` Alexandre Torgue
2020-04-27 20:05 ` Adrian Pop
2020-04-28 8:39 ` Alexandre Torgue
2020-05-16 8:39 ` Adrian Pop
2020-04-28 9:05 ` Vladimir Murzin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).