Conflict between video-lut and pmu on meson-g12

Conflict between video-lut and pmu on meson-g12

[Marc Gonzalez]

Hello everyone,

Running 6.2.0-rc8 on a S905X2 board, I note the following error
in the kernel log:

[    1.059175] meson-g12-ddr-pmu ff638000.pmu: can't request region for resource [mem 0xff638000-0xff6380ff]
[    1.068647] meson-g12-ddr-pmu: probe of ff638000.pmu failed with error -16

Relevant DT node from decompiled DTB:
(arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi + arch/arm64/boot/dts/amlogic/meson-g12a.dtsi)

	pmu@ff638000 {
		reg = <0x0 0xff638000 0x0 0x100 0x0 0xff638c00 0x0 0x100>;
		interrupts = <0x0 0x34 0x1>;
		compatible = "amlogic,g12a-ddr-pmu";
	};

However, according to /proc/iomem
ff638048-ff63805b : ff638048.video-lut video-lut@48

Corresponding DT node:
(arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi)

	video-lut@48 {
		compatible = "amlogic,canvas";
		reg = <0x0 0x48 0x0 0x14>;
		phandle = <0x22>;
	};

(with a base of 0xff600000 + 0x38000 = 0xff638000)

Unless I'm mistaken, ff638000-0xff6380ff and ff638048-ff63805b
cannot co-exist?

A simple solution might be to specify the "actual" base of
the register set, and count from 0 in the driver?


diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 534178eaaf373..cf37eecfba5b2 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -1712,7 +1712,7 @@ internal_ephy: ethernet-phy@8 {
 		};
 
 		pmu: pmu@ff638000 {
-			reg = <0x0 0xff638000 0x0 0x100>,
+			reg = <0x0 0xff638080 0x0 0x40>,
 			      <0x0 0xff638c00 0x0 0x100>;
 			interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
 		};
diff --git a/drivers/perf/amlogic/meson_g12_ddr_pmu.c b/drivers/perf/amlogic/meson_g12_ddr_pmu.c
index a78fdb15e26c2..8b643888d5036 100644
--- a/drivers/perf/amlogic/meson_g12_ddr_pmu.c
+++ b/drivers/perf/amlogic/meson_g12_ddr_pmu.c
@@ -21,23 +21,23 @@
 #define DMC_QOS_IRQ		BIT(30)
 
 /* DMC bandwidth monitor register address offset */
-#define DMC_MON_G12_CTRL0		(0x20  << 2)
-#define DMC_MON_G12_CTRL1		(0x21  << 2)
-#define DMC_MON_G12_CTRL2		(0x22  << 2)
-#define DMC_MON_G12_CTRL3		(0x23  << 2)
-#define DMC_MON_G12_CTRL4		(0x24  << 2)
-#define DMC_MON_G12_CTRL5		(0x25  << 2)
-#define DMC_MON_G12_CTRL6		(0x26  << 2)
-#define DMC_MON_G12_CTRL7		(0x27  << 2)
-#define DMC_MON_G12_CTRL8		(0x28  << 2)
-
-#define DMC_MON_G12_ALL_REQ_CNT		(0x29  << 2)
-#define DMC_MON_G12_ALL_GRANT_CNT	(0x2a  << 2)
-#define DMC_MON_G12_ONE_GRANT_CNT	(0x2b  << 2)
-#define DMC_MON_G12_SEC_GRANT_CNT	(0x2c  << 2)
-#define DMC_MON_G12_THD_GRANT_CNT	(0x2d  << 2)
-#define DMC_MON_G12_FOR_GRANT_CNT	(0x2e  << 2)
-#define DMC_MON_G12_TIMER		(0x2f  << 2)
+#define DMC_MON_G12_CTRL0		(0x0  << 2)
+#define DMC_MON_G12_CTRL1		(0x1  << 2)
+#define DMC_MON_G12_CTRL2		(0x2  << 2)
+#define DMC_MON_G12_CTRL3		(0x3  << 2)
+#define DMC_MON_G12_CTRL4		(0x4  << 2)
+#define DMC_MON_G12_CTRL5		(0x5  << 2)
+#define DMC_MON_G12_CTRL6		(0x6  << 2)
+#define DMC_MON_G12_CTRL7		(0x7  << 2)
+#define DMC_MON_G12_CTRL8		(0x8  << 2)
+
+#define DMC_MON_G12_ALL_REQ_CNT		(0x9  << 2)
+#define DMC_MON_G12_ALL_GRANT_CNT	(0xa  << 2)
+#define DMC_MON_G12_ONE_GRANT_CNT	(0xb  << 2)
+#define DMC_MON_G12_SEC_GRANT_CNT	(0xc  << 2)
+#define DMC_MON_G12_THD_GRANT_CNT	(0xd  << 2)
+#define DMC_MON_G12_FOR_GRANT_CNT	(0xe  << 2)
+#define DMC_MON_G12_TIMER		(0xf  << 2)
 
 /* Each bit represent a axi line */
 PMU_FORMAT_ATTR(event, "config:0-7");


With the above patch, /proc/iomem becomes:

ff638048-ff63805b : ff638048.video-lut video-lut@48
ff638080-ff6380bf : ff638080.pmu pmu@ff638000
ff638c00-ff638cff : ff638080.pmu pmu@ff638000

(I didn't test that the driver actually works.
I suppose one needs perf for that?)

Regards.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Conflict between video-lut and pmu on meson-g12

[Martin Blumenstingl]

Hello Marc,

On Tue, Feb 28, 2023 at 5:48 PM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
>
> Hello everyone,
>
> Running 6.2.0-rc8 on a S905X2 board, I note the following error
> in the kernel log:
>
> [    1.059175] meson-g12-ddr-pmu ff638000.pmu: can't request region for resource [mem 0xff638000-0xff6380ff]
> [    1.068647] meson-g12-ddr-pmu: probe of ff638000.pmu failed with error -16
[...]
ouch, it seems we missed that during the driver/bindings review

> A simple solution might be to specify the "actual" base of
> the register set, and count from 0 in the driver?
I think your fix is correct (formally it would need to be split into a
driver and a .dtsi patch - but let's do things step by step).
It would be great to get some confirmation from Jiucheng Xu on this as
I think we should be quick with fixing before this bug makes it
elsewhere (.dtsis can be shared with u-boot and other systems for
example).

Apart from the driver and .dtsi changes we should also update
Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
But again: let's do this one step at a time.


Thanks for the analysis and your work on this!
Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Conflict between video-lut and pmu on meson-g12

[Martin Blumenstingl]

Hello Jiucheng Xu,

On Tue, Feb 28, 2023 at 10:04 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
[...]
> > A simple solution might be to specify the "actual" base of
> > the register set, and count from 0 in the driver?
> I think your fix is correct (formally it would need to be split into a
> driver and a .dtsi patch - but let's do things step by step).
While thinking more about this - I think the whole .dtsi code should
be improved. Both of the PMU IO regions are part of the &dmc region.
So I think &pmu should be moved inside &dmc (with the offsets adjusted
accordingly of course).

Also I think the dt-bindings are incomplete: according to the driver
code we're using XTAL as input clock.
But this is not described anywhere in the dt-bindings.
dt-bindings should always describe the hardware. The driver can decide
not to use it but the bindings must always be complete.
And with this comes the question: is the DMC PLL specific to the PMU
or is it shared with something else (e.g. the actual memory
controller)? On the 32-bit SoCs (Meson8b/S805 for example) there's a
whole DDR clock controller (used by the DDR memory controller), so I'm
wondering if these newer SoCs are still following that approach.


Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Conflict between video-lut and pmu on meson-g12

[Jiucheng Xu]

+ jiuchengxu@163.com

Amlogic mail sending server has problem. Add my personal email.
Comments inline.

On 2023/3/1 5:49, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
>
> Hello Jiucheng Xu,
>
> On Tue, Feb 28, 2023 at 10:04 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> [...]
>>> A simple solution might be to specify the "actual" base of
>>> the register set, and count from 0 in the driver?
>> I think your fix is correct (formally it would need to be split into a
>> driver and a .dtsi patch - but let's do things step by step).
> While thinking more about this - I think the whole .dtsi code should
> be improved. Both of the PMU IO regions are part of the &dmc region.
> So I think &pmu should be moved inside &dmc (with the offsets adjusted
> accordingly of course).
Okay I agree with you.I will put &pmu node into &dmc node.
>
> Also I think the dt-bindings are incomplete: according to the driver
> code we're using XTAL as input clock.
> But this is not described anywhere in the dt-bindings.
> dt-bindings should always describe the hardware. The driver can decide
> not to use it but the bindings must always be complete.
Sorry, I couldn't get your point. I didn't know the relationship between
XTAL and DMC. Would you please share more the information?
> And with this comes the question: is the DMC PLL specific to the PMU
> or is it shared with something else (e.g. the actual memory
> controller)? On the 32-bit SoCs (Meson8b/S805 for example) there's a
> whole DDR clock controller (used by the DDR memory controller), so I'm
> wondering if these newer SoCs are still following that approach.
Yes, the PLL is the base input clock of DMC and PMU inside. I have never
seen the meson8b board. I think G12 is a newer generation which has some 
improvement than old
SoCs.

My English is not well. Maybe I doesn't answer your point since I got 
miss-understanding.

Thanks & Best regards
Jiucheng
>
> Best regards,
> Martin
>


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Conflict between video-lut and pmu on meson-g12

[Marc Gonzalez]

On 28/02/2023 22:49, Martin Blumenstingl wrote:

> While thinking more about this - I think the whole .dtsi code should
> be improved. Both of the PMU IO regions are part of the &dmc region.
> So I think &pmu should be moved inside &dmc (with the offsets adjusted
> accordingly of course).
> 
> Also I think the dt-bindings are incomplete: according to the driver
> code we're using XTAL as input clock.
> But this is not described anywhere in the dt-bindings.
> dt-bindings should always describe the hardware. The driver can decide
> not to use it but the bindings must always be complete.
> And with this comes the question: is the DMC PLL specific to the PMU
> or is it shared with something else (e.g. the actual memory
> controller)? On the 32-bit SoCs (Meson8b/S805 for example) there's a
> whole DDR clock controller (used by the DDR memory controller), so I'm
> wondering if these newer SoCs are still following that approach.

FWIW, the vendor device-tree specifies the following nodes:
https://android.googlesource.com/kernel/arm64/+/f5269100977385d1fd4a5ef68e49631892cf4fe4/arch/arm64/boot/dts/amlogic/g12a_s905x2_u215.dts

	canvas {
		compatible = "amlogic, meson, canvas";
		dev_name = "amlogic-canvas";
		status = "okay";
		reg = <0x0 0xff638000 0x0 0x2000>;
		phandle = <0x111>;
	};

	codec_io {
		compatible = "amlogic, codec_io";
		status = "okay";
		#address-cells = <0x2>;
		#size-cells = <0x2>;
		ranges;
		phandle = <0x112>;
		[...]
		io_dmc_base {
			reg = <0x0 0xff638000 0x0 0x2000>;
		};
	};

	ddr_bandwidth {
		compatible = "amlogic, ddr-bandwidth";
		status = "okay";
		reg = <0x0 0xff638000 0x0 0x100 0x0 0xff638c00 0x0 0x100>;
		sec_base = <0xff639000>;
		interrupts = <0x0 0x34 0x1>;
		interrupt-names = "ddr_bandwidth";
	};


I don't understand how it's possible to have 3 overlapping ranges?
Unless the respective drivers know to map only specific ranges?


Regarding your DMC (DDR memory controller) clock question,
clock tree seems to be:
24 MHz XTAL feeds DDR_PLL block,
which outputs DDR_CLK pulse for the DMC.


Something I do not understand is that the datasheet states:
DMC unsecure register. Base address 0xFF638000.
Offset 0 = AM_DDR_PLL_CNTL0
Offset 4 = AM_DDR_PLL_CNTL1
...

And then also states:
The following registers' base address is 0xff638000.
Offset 0 = DMC_REQ_CTRL
Offset 4 = DMC_SOFT_RST
...

And these two register sets have nothing in common
(except the SAME base address...)

https://android.googlesource.com/kernel/arm64/+/f5269100977385d1fd4a5ef68e49631892cf4fe4/include/linux/amlogic/media/registers/regs/dmc_regs.h

Is there perhaps a typo in one of the base addresses?


Regards.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Conflict between video-lut and pmu on meson-g12

[Marc Gonzalez]

On 01/03/2023 14:28, Marc Gonzalez wrote:

> On 28/02/2023 22:49, Martin Blumenstingl wrote:
> 
>> While thinking more about this - I think the whole .dtsi code should
>> be improved. Both of the PMU IO regions are part of the &dmc region.
>> So I think &pmu should be moved inside &dmc (with the offsets adjusted
>> accordingly of course).
>>
>> Also I think the dt-bindings are incomplete: according to the driver
>> code we're using XTAL as input clock.
>> But this is not described anywhere in the dt-bindings.
>> dt-bindings should always describe the hardware. The driver can decide
>> not to use it but the bindings must always be complete.
>> And with this comes the question: is the DMC PLL specific to the PMU
>> or is it shared with something else (e.g. the actual memory
>> controller)? On the 32-bit SoCs (Meson8b/S805 for example) there's a
>> whole DDR clock controller (used by the DDR memory controller), so I'm
>> wondering if these newer SoCs are still following that approach.
> 
> FWIW, the vendor device-tree specifies the following nodes:
> https://android.googlesource.com/kernel/arm64/+/f5269100977385d1fd4a5ef68e49631892cf4fe4/arch/arm64/boot/dts/amlogic/g12a_s905x2_u215.dts
> 
> 	canvas {
> 		compatible = "amlogic, meson, canvas";
> 		dev_name = "amlogic-canvas";
> 		status = "okay";
> 		reg = <0x0 0xff638000 0x0 0x2000>;
> 		phandle = <0x111>;
> 	};
> 
> 	codec_io {
> 		compatible = "amlogic, codec_io";
> 		status = "okay";
> 		#address-cells = <0x2>;
> 		#size-cells = <0x2>;
> 		ranges;
> 		phandle = <0x112>;
> 		[...]
> 		io_dmc_base {
> 			reg = <0x0 0xff638000 0x0 0x2000>;
> 		};
> 	};
> 
> 	ddr_bandwidth {
> 		compatible = "amlogic, ddr-bandwidth";
> 		status = "okay";
> 		reg = <0x0 0xff638000 0x0 0x100 0x0 0xff638c00 0x0 0x100>;
> 		sec_base = <0xff639000>;
> 		interrupts = <0x0 0x34 0x1>;
> 		interrupt-names = "ddr_bandwidth";
> 	};
> 
> 
> I don't understand how it's possible to have 3 overlapping ranges?
> Unless the respective drivers know to map only specific ranges?
> 
> 
> Regarding your DMC (DDR memory controller) clock question,
> clock tree seems to be:
> 24 MHz XTAL feeds DDR_PLL block,
> which outputs DDR_CLK pulse for the DMC.
> 
> 
> Something I do not understand is that the datasheet states:
> DMC unsecure register. Base address 0xFF638000.
> Offset 0 = AM_DDR_PLL_CNTL0
> Offset 4 = AM_DDR_PLL_CNTL1
> ...
> 
> And then also states:
> The following registers' base address is 0xff638000.
> Offset 0 = DMC_REQ_CTRL
> Offset 4 = DMC_SOFT_RST
> ...
> 
> And these two register sets have nothing in common
> (except the SAME base address...)
> 
> https://android.googlesource.com/kernel/arm64/+/f5269100977385d1fd4a5ef68e49631892cf4fe4/include/linux/amlogic/media/registers/regs/dmc_regs.h
> 
> Is there perhaps a typo in one of the base addresses?


Any ideas/suggestions?
How do we proceed?

Regards


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Conflict between video-lut and pmu on meson-g12

[Martin Blumenstingl]

Hi Marc,

On Thu, Mar 9, 2023 at 10:48 AM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
[...]
> Any ideas/suggestions?
> How do we proceed?
I suggest you go ahead and send your original diff as formal patches.
It would be best to have two/three patches (that can go through
different maintainers repositories if needed):
- move &pmu into &dmc, update the register size (as you suggested) and
also update the offset (since they will have to be calculated based on
&dmc)
- fix up the #defines in the driver as you suggested
- nice to have: update the example in
Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml

Before we work on the XTAL clock input I think it's best if we know
about the actual hardware setup.
amlogic,g12-ddr-pmu.yaml mentions that the second register set is for
the "DMC PLL register space".
I suspect that the DMC PLL has two purposes:
- it's used as timer/counter for the PMU
- it is also used for clocking the DDR memory

If this assumption is correct then I think that the DMC PLL should get
a separate node (with XTAL as clock input). Then the DMC PLL output
should be used as clock input for the &pmu node.
The reason for this is that the device tree should describe the
hardware, not drivers. Also device tree is not Linux specific, it can
be used by u-boot, etc.
So if someone would implement the DDR setup in u-boot (or another
bootloader - which may even share the device tree with Linux) then it
has to be described correctly.
I'm hoping that Jiucheng can provide his input on this topic.

By the way: we already have a DDR (PLL) driver in
drivers/clk/meson/meson8-ddr.c. At this point in time it only has
Meson8/Meson8b/Meson8m2 support. I suspect it will be easy to extend
the existing driver or just write a new one.


Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH] arm64: dts: meson-g12-common: specify full DMC range

[Marc Gonzalez]

According to S905X2 Datasheet - Revision 07:
DRAM Memory Controller (DMC) register area spans ff638000-ff63a000.

According to DeviceTree Specification - Release v0.4-rc1:
simple-bus nodes do not require reg property.

Fixes: 1499218c80c9 ("arm64: dts: move common G12A & G12B modes to meson-g12-common.dtsi")
Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
---
 arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 534178eaaf373..a5653ab1f0b43 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -1577,10 +1577,9 @@ usb2_phy0: phy@36000 {
 
 			dmc: bus@38000 {
 				compatible = "simple-bus";
-				reg = <0x0 0x38000 0x0 0x400>;
 				#address-cells = <2>;
 				#size-cells = <2>;
-				ranges = <0x0 0x0 0x0 0x38000 0x0 0x400>;
+				ranges = <0x0 0x0 0x0 0x38000 0x0 0x2000>;
 
 				canvas: video-lut@48 {
 					compatible = "amlogic,canvas";
-- 
2.25.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH] perf/amlogic: resolve conflict between canvas & pmu

[Marc Gonzalez]

According to S905X2 Datasheet - Revision 07:

DMC_MON area spans 0xff638080-0xff6380c0
DDR_PLL area spans 0xff638c00-0xff638c34

Round DDR_PLL area size up to 0x40

Fixes: 90cf8e21016f ("arm64: dts: meson: Add DDR PMU node")
Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
---
 arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 12 ++++++------
 drivers/perf/amlogic/meson_g12_ddr_pmu.c          | 34 +++++++++++++++++-----------------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index a5653ab1f0b43..1aab65bb5f578 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -1585,6 +1585,12 @@ canvas: video-lut@48 {
 					compatible = "amlogic,canvas";
 					reg = <0x0 0x48 0x0 0x14>;
 				};
+
+				pmu: pmu@80 {
+					reg = <0x0 0x80 0x0 0x40>,
+					      <0x0 0xc00 0x0 0x40>;
+					interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
+				};
 			};
 
 			usb2_phy1: phy@3a000 {
@@ -1710,12 +1716,6 @@ internal_ephy: ethernet-phy@8 {
 			};
 		};
 
-		pmu: pmu@ff638000 {
-			reg = <0x0 0xff638000 0x0 0x100>,
-			      <0x0 0xff638c00 0x0 0x100>;
-			interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
-		};
-
 		aobus: bus@ff800000 {
 			compatible = "simple-bus";
 			reg = <0x0 0xff800000 0x0 0x100000>;
diff --git a/drivers/perf/amlogic/meson_g12_ddr_pmu.c b/drivers/perf/amlogic/meson_g12_ddr_pmu.c
index a78fdb15e26c2..8b643888d5036 100644
--- a/drivers/perf/amlogic/meson_g12_ddr_pmu.c
+++ b/drivers/perf/amlogic/meson_g12_ddr_pmu.c
@@ -21,23 +21,23 @@
 #define DMC_QOS_IRQ		BIT(30)
 
 /* DMC bandwidth monitor register address offset */
-#define DMC_MON_G12_CTRL0		(0x20  << 2)
-#define DMC_MON_G12_CTRL1		(0x21  << 2)
-#define DMC_MON_G12_CTRL2		(0x22  << 2)
-#define DMC_MON_G12_CTRL3		(0x23  << 2)
-#define DMC_MON_G12_CTRL4		(0x24  << 2)
-#define DMC_MON_G12_CTRL5		(0x25  << 2)
-#define DMC_MON_G12_CTRL6		(0x26  << 2)
-#define DMC_MON_G12_CTRL7		(0x27  << 2)
-#define DMC_MON_G12_CTRL8		(0x28  << 2)
-
-#define DMC_MON_G12_ALL_REQ_CNT		(0x29  << 2)
-#define DMC_MON_G12_ALL_GRANT_CNT	(0x2a  << 2)
-#define DMC_MON_G12_ONE_GRANT_CNT	(0x2b  << 2)
-#define DMC_MON_G12_SEC_GRANT_CNT	(0x2c  << 2)
-#define DMC_MON_G12_THD_GRANT_CNT	(0x2d  << 2)
-#define DMC_MON_G12_FOR_GRANT_CNT	(0x2e  << 2)
-#define DMC_MON_G12_TIMER		(0x2f  << 2)
+#define DMC_MON_G12_CTRL0		(0x0  << 2)
+#define DMC_MON_G12_CTRL1		(0x1  << 2)
+#define DMC_MON_G12_CTRL2		(0x2  << 2)
+#define DMC_MON_G12_CTRL3		(0x3  << 2)
+#define DMC_MON_G12_CTRL4		(0x4  << 2)
+#define DMC_MON_G12_CTRL5		(0x5  << 2)
+#define DMC_MON_G12_CTRL6		(0x6  << 2)
+#define DMC_MON_G12_CTRL7		(0x7  << 2)
+#define DMC_MON_G12_CTRL8		(0x8  << 2)
+
+#define DMC_MON_G12_ALL_REQ_CNT		(0x9  << 2)
+#define DMC_MON_G12_ALL_GRANT_CNT	(0xa  << 2)
+#define DMC_MON_G12_ONE_GRANT_CNT	(0xb  << 2)
+#define DMC_MON_G12_SEC_GRANT_CNT	(0xc  << 2)
+#define DMC_MON_G12_THD_GRANT_CNT	(0xd  << 2)
+#define DMC_MON_G12_FOR_GRANT_CNT	(0xe  << 2)
+#define DMC_MON_G12_TIMER		(0xf  << 2)
 
 /* Each bit represent a axi line */
 PMU_FORMAT_ATTR(event, "config:0-7");
-- 
2.25.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] arm64: dts: meson-g12-common: specify full DMC range

[Martin Blumenstingl]

On Thu, Mar 23, 2023 at 5:12 PM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
>
> According to S905X2 Datasheet - Revision 07:
> DRAM Memory Controller (DMC) register area spans ff638000-ff63a000.
>
> According to DeviceTree Specification - Release v0.4-rc1:
> simple-bus nodes do not require reg property.
Up until this point I thought that the reg property is mandatory even
for a simple-bus. Turns out this is not the case so we probably have
to clean up more meson.dtsi - but that's out of scope of this patch of
course.

> Fixes: 1499218c80c9 ("arm64: dts: move common G12A & G12B modes to meson-g12-common.dtsi")
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] perf/amlogic: resolve conflict between canvas & pmu

[Martin Blumenstingl]

Hi Marc,

thanks for working on this!

On Thu, Mar 23, 2023 at 5:24 PM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
>
> According to S905X2 Datasheet - Revision 07:
>
> DMC_MON area spans 0xff638080-0xff6380c0
> DDR_PLL area spans 0xff638c00-0xff638c34
>
> Round DDR_PLL area size up to 0x40
>
> Fixes: 90cf8e21016f ("arm64: dts: meson: Add DDR PMU node")
This is only partially correct. We also need:
Fixes: 2016e2113d35 ("perf/amlogic: Add support for Amlogic meson G12
SoC DDR PMU driver")

> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
>  arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 12 ++++++------
>  drivers/perf/amlogic/meson_g12_ddr_pmu.c          | 34 +++++++++++++++++-----------------
I think it makes sense to split this patch into two separate patches,
each of them with the proper Fixes tag attached (and subject prefix).
That way the maintainers of the according files can pick them up (.dts
and driver changes usually go through different trees, I expect this
to be the case here as well).

My understanding is that the driver changes will go through a tree
other than the Amlogic one (I suspect it's Will Deacon's tree).
Please Cc the other maintainers and mailing lists as shown by
scripts/get_maintainer.pl:
  Will Deacon <will@kernel.org> (maintainer:ARM PMU PROFILING AND DEBUGGING)
  Mark Rutland <mark.rutland@arm.com> (maintainer:ARM PMU PROFILING
AND DEBUGGING)
  linux-arm-kernel@lists.infradead.org (moderated list:ARM/Amlogic
Meson SoC support)

The contents of the patch look good to me - so this is only about
organizing things.


Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] perf/amlogic: resolve conflict between canvas & pmu

[Neil Armstrong]

On 25/03/2023 21:54, Martin Blumenstingl wrote:
> Hi Marc,
> 
> thanks for working on this!
> 
> On Thu, Mar 23, 2023 at 5:24 PM Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
>>
>> According to S905X2 Datasheet - Revision 07:
>>
>> DMC_MON area spans 0xff638080-0xff6380c0
>> DDR_PLL area spans 0xff638c00-0xff638c34
>>
>> Round DDR_PLL area size up to 0x40
>>
>> Fixes: 90cf8e21016f ("arm64: dts: meson: Add DDR PMU node")
> This is only partially correct. We also need:
> Fixes: 2016e2113d35 ("perf/amlogic: Add support for Amlogic meson G12
> SoC DDR PMU driver")
> 
>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>> ---
>>   arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 12 ++++++------
>>   drivers/perf/amlogic/meson_g12_ddr_pmu.c          | 34 +++++++++++++++++-----------------
> I think it makes sense to split this patch into two separate patches,
> each of them with the proper Fixes tag attached (and subject prefix).
> That way the maintainers of the according files can pick them up (.dts
> and driver changes usually go through different trees, I expect this
> to be the case here as well).
> 
> My understanding is that the driver changes will go through a tree
> other than the Amlogic one (I suspect it's Will Deacon's tree).
> Please Cc the other maintainers and mailing lists as shown by
> scripts/get_maintainer.pl:
>    Will Deacon <will@kernel.org> (maintainer:ARM PMU PROFILING AND DEBUGGING)
>    Mark Rutland <mark.rutland@arm.com> (maintainer:ARM PMU PROFILING
> AND DEBUGGING)
>    linux-arm-kernel@lists.infradead.org (moderated list:ARM/Amlogic
> Meson SoC support)
> 
> The contents of the patch look good to me - so this is only about
> organizing things.

The content is good, but can you send the 3 patches (DMC range, DT fix, Driver fix)
in a separate patchset ?

And avoid sending patches in reply to other patchsets/discussions, this messes things out.

Thanks !
Neil

> 
> 
> Best regards,
> Martin


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic