diff mbox series

[v1,03/18] arm64: dts: imx8mq-evk: add reserve memory node for CMA region

Message ID 20210217080306.157876-4-benjamin.gaignard@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add HANTRO G2/HEVC decoder support for IMX8MQ | expand

Commit Message

Benjamin Gaignard Feb. 17, 2021, 8:02 a.m. UTC
Define allocation range for the default CMA region.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Ezequiel Garcia Feb. 17, 2021, 7:39 p.m. UTC | #1
Hi Benjamin,

On Wed, 2021-02-17 at 09:02 +0100, Benjamin Gaignard wrote:
> Define allocation range for the default CMA region.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>

Despite it seems like I signed-off this one...

> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> index 85b045253a0e..047dfd4a1ffd 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> @@ -21,6 +21,21 @@ memory@40000000 {
>                 reg = <0x00000000 0x40000000 0 0xc0000000>;
>         };
> 
>  
> +       resmem: reserved-memory {
> +               #address-cells = <2>;
> +               #size-cells = <2>;
> +               ranges;
> +
> +               /* global autoconfigured region for contiguous allocations */
> +               linux,cma {
> +                       compatible = "shared-dma-pool";
> +                       reusable;
> +                       size = <0 0x3c000000>;
> +                       alloc-ranges = <0 0x40000000 0 0x40000000>;
> +                       linux,cma-default;
> +               };

... I'm not a fan of the change :)

Hopefully someone from NXP can provide some insight here?

If it's absolutely needed for the VPU, then I guess it should
be 1) very well documented and 2) moved to the top-lovel dtsi.

But if we can drop it, that'd be nicer.

Thanks,
Ezequiel
Lucas Stach Feb. 18, 2021, 10:15 a.m. UTC | #2
Am Mittwoch, dem 17.02.2021 um 16:39 -0300 schrieb Ezequiel Garcia:
> Hi Benjamin,
> 
> On Wed, 2021-02-17 at 09:02 +0100, Benjamin Gaignard wrote:
> > Define allocation range for the default CMA region.
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> 
> Despite it seems like I signed-off this one...
> 
> > Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> > index 85b045253a0e..047dfd4a1ffd 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> > @@ -21,6 +21,21 @@ memory@40000000 {
> >                 reg = <0x00000000 0x40000000 0 0xc0000000>;
> >         };
> > 
> >  
> > +       resmem: reserved-memory {
> > +               #address-cells = <2>;
> > +               #size-cells = <2>;
> > +               ranges;
> > +
> > +               /* global autoconfigured region for contiguous allocations */
> > +               linux,cma {
> > +                       compatible = "shared-dma-pool";
> > +                       reusable;
> > +                       size = <0 0x3c000000>;
> > +                       alloc-ranges = <0 0x40000000 0 0x40000000>;
> > +                       linux,cma-default;
> > +               };
> 
> ... I'm not a fan of the change :)
> 
> Hopefully someone from NXP can provide some insight here?
> 
> If it's absolutely needed for the VPU, then I guess it should
> be 1) very well documented and 2) moved to the top-lovel dtsi.
> 
> But if we can drop it, that'd be nicer.

What's the justification for this CMA area?

I could only imagine the DMA addressing restrictions on the platform.
DMA masters on the i.MX8MQ can not access memory beyond the 4GB mark
and 1GB of address space is reserved for MMIO, so if you have 4GB
installed the upper 1GB of DRAM is only accessible to the CPU. But this
restriction is already properly communicated to the Linux DMA framework
by the dma-ranges in the top level SoC bus node in the DT, so I don't
think this CMA setup is necessary.

Regards,
Lucas
Dan Carpenter Feb. 18, 2021, 10:45 a.m. UTC | #3
On Wed, Feb 17, 2021 at 04:39:49PM -0300, Ezequiel Garcia wrote:
> Hi Benjamin,
> 
> On Wed, 2021-02-17 at 09:02 +0100, Benjamin Gaignard wrote:
> > Define allocation range for the default CMA region.
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> 
> Despite it seems like I signed-off this one...
> 

I've been puzzled by this as well.  :P

Signed-off-by means you either wrote the patch or you handled it in some
way.  And it is intended as a legally binding document that you didn't
sneak in any copyrighted code from SCO UNIXWARE (etc).  So like maybe
the authors snuck some in or maybe a maintainer took the patch and
sneaked some unixware code in.

Obviously if you sign the code, that counts as an Ack and Review as well
because maintainers are going to only merge stuff if they've looked it
over a bit.  But the main thing is that it means you didn't didn't
violate any copyrights.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index 85b045253a0e..047dfd4a1ffd 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -21,6 +21,21 @@  memory@40000000 {
 		reg = <0x00000000 0x40000000 0 0xc0000000>;
 	};
 
+	resmem: reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* global autoconfigured region for contiguous allocations */
+		linux,cma {
+			compatible = "shared-dma-pool";
+			reusable;
+			size = <0 0x3c000000>;
+			alloc-ranges = <0 0x40000000 0 0x40000000>;
+			linux,cma-default;
+		};
+	};
+
 	pcie0_refclk: pcie0-refclk {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;