[PATCH v2 1/4] arm64: dts: qcom: sdm845: Reduce reg size for aoss_qmp

[PATCH v2 1/4] arm64: dts: qcom: sdm845: Reduce reg size for aoss_qmp

[Abel Vesa]

Like on the other platforms that provide RPMh stats, on SDM845, the
aoss_qmp reg size needs to be reduced to its actual size of 0x400,
otherwise it will overlap with the RPMh stats reg base, node that will
be added later on.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---

No changes since v1.

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index f0e286715d1b..5bea96a9ce06 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -4836,7 +4836,7 @@ aoss_reset: reset-controller@c2a0000 {
 
 		aoss_qmp: power-controller@c300000 {
 			compatible = "qcom,sdm845-aoss-qmp", "qcom,aoss-qmp";
-			reg = <0 0x0c300000 0 0x100000>;
+			reg = <0 0x0c300000 0 0x400>;
 			interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
 			mboxes = <&apss_shared 0>;
 
-- 
2.34.1

[PATCH v2 2/4] soc: qcom: stats: Add SDM845 stats config and compatible

[Abel Vesa]

SDM845 is a special case compared to the other platforms that use RPMh
stats, since it only has 2 stats (aosd and cxsd), while the others have
a 3rd one (ddr).

So lets add dedicated stats config and compatible for SDM845 to make the
driver aware of this num_records difference.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---

Changed qcom,rpmh-stats-sdm845 to qcom,sdm845-rpmh-stats, as suggested
by Krzysztof.

 drivers/soc/qcom/qcom_stats.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
index d6bfd1bbdc2a..121ea409fafc 100644
--- a/drivers/soc/qcom/qcom_stats.c
+++ b/drivers/soc/qcom/qcom_stats.c
@@ -246,6 +246,14 @@ static const struct stats_config rpm_data_dba0 = {
 	.subsystem_stats_in_smem = false,
 };
 
+static const struct stats_config rpmh_data_sdm845 = {
+	.stats_offset = 0x48,
+	.num_records = 2,
+	.appended_stats_avail = false,
+	.dynamic_offset = false,
+	.subsystem_stats_in_smem = true,
+};
+
 static const struct stats_config rpmh_data = {
 	.stats_offset = 0x48,
 	.num_records = 3,
@@ -261,6 +269,7 @@ static const struct of_device_id qcom_stats_table[] = {
 	{ .compatible = "qcom,msm8974-rpm-stats", .data = &rpm_data_dba0 },
 	{ .compatible = "qcom,rpm-stats", .data = &rpm_data },
 	{ .compatible = "qcom,rpmh-stats", .data = &rpmh_data },
+	{ .compatible = "qcom,sdm845-rpmh-stats", .data = &rpmh_data_sdm845 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_stats_table);
-- 
2.34.1

[PATCH v2 3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node

[Abel Vesa]

SDM845 is a special case compared to the other platforms that use RPMh
stats, since it only has 2 stats (aosd and cxsd), while the others have
a 3rd one (ddr).

So lets add the node but with a SDM845 dedicated compatible to make
the driver aware of the different stats config.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---

Changed qcom,rpmh-stats-sdm845 to qcom,sdm845-rpmh-stats, as suggested
by Krzysztof.

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 5bea96a9ce06..67fe08b837be 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -4851,6 +4851,11 @@ ebi_cdev: ebi {
 			};
 		};
 
+		sram@c3f0000 {
+			compatible = "qcom,sdm845-rpmh-stats";
+			reg = <0 0x0c3f0000 0 0x400>;
+		};
+
 		spmi_bus: spmi@c440000 {
 			compatible = "qcom,spmi-pmic-arb";
 			reg = <0 0x0c440000 0 0x1100>,
-- 
2.34.1

[PATCH v2 4/4] dt-bindings: soc: qcom: stats: Document SDM845 compatible

[Abel Vesa]

SDM845 is a special case compared to the other platforms that use RPMh
stats, since it only has 2 stats (aosd and cxsd), while the others have
a 3rd one (ddr).

So in order for the driver to use the dedicated stats config, we added
the SDM845 dedicated compatible, which we document here.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---

Changed qcom,rpmh-stats-sdm845 to qcom,sdm845-rpmh-stats, as suggested
by Krzysztof.

 Documentation/devicetree/bindings/soc/qcom/qcom-stats.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom-stats.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom-stats.yaml
index 473adca4e973..48eda4d0d391 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom-stats.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom-stats.yaml
@@ -20,6 +20,7 @@ properties:
   compatible:
     enum:
       - qcom,rpmh-stats
+      - qcom,sdm845-rpmh-stats
       - qcom,rpm-stats
       # For older RPM firmware versions with fixed offset for the sleep stats
       - qcom,apq8084-rpm-stats
-- 
2.34.1

Re: [PATCH v2 4/4] dt-bindings: soc: qcom: stats: Document SDM845 compatible

[Krzysztof Kozlowski]

On 12/08/2022 13:12, Abel Vesa wrote:
> SDM845 is a special case compared to the other platforms that use RPMh
> stats, since it only has 2 stats (aosd and cxsd), while the others have
> a 3rd one (ddr).
> 
> So in order for the driver to use the dedicated stats config, we added
> the SDM845 dedicated compatible, which we document here.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v2 3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node

[Krzysztof Kozlowski]

On 12/08/2022 13:12, Abel Vesa wrote:
> SDM845 is a special case compared to the other platforms that use RPMh
> stats, since it only has 2 stats (aosd and cxsd), while the others have
> a 3rd one (ddr).
> 
> So lets add the node but with a SDM845 dedicated compatible to make
> the driver aware of the different stats config.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v2 1/4] arm64: dts: qcom: sdm845: Reduce reg size for aoss_qmp

[Krzysztof Kozlowski]

On 12/08/2022 13:12, Abel Vesa wrote:
> Like on the other platforms that provide RPMh stats, on SDM845, the
> aoss_qmp reg size needs to be reduced to its actual size of 0x400,
> otherwise it will overlap with the RPMh stats reg base, node that will
> be added later on.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> 


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v2 2/4] soc: qcom: stats: Add SDM845 stats config and compatible

[Krzysztof Kozlowski]

On 12/08/2022 13:12, Abel Vesa wrote:
> SDM845 is a special case compared to the other platforms that use RPMh
> stats, since it only has 2 stats (aosd and cxsd), while the others have
> a 3rd one (ddr).
> 
> So lets add dedicated stats config and compatible for SDM845 to make the
> driver aware of this num_records difference.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v2 3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node

[Caleb Connolly]

On 12/08/2022 11:12, Abel Vesa wrote:
> SDM845 is a special case compared to the other platforms that use RPMh
> stats, since it only has 2 stats (aosd and cxsd), while the others have
> a 3rd one (ddr).
> 
> So lets add the node but with a SDM845 dedicated compatible to make
> the driver aware of the different stats config.
Hi,

I gave this a go on the OnePlus 6, I noticed the driver is also meant to read 
the stats for remote procs via smem, however this seems to fail for me - it 
can't find any of the SMEM items even if I probe the driver manually after 
ensuring remoteprocs have booted. Is this an unsupported feature on SDM845?
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> 
> Changed qcom,rpmh-stats-sdm845 to qcom,sdm845-rpmh-stats, as suggested
> by Krzysztof.
> 
>   arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 5bea96a9ce06..67fe08b837be 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -4851,6 +4851,11 @@ ebi_cdev: ebi {
>   			};
>   		};
> 
> +		sram@c3f0000 {
> +			compatible = "qcom,sdm845-rpmh-stats";
> +			reg = <0 0x0c3f0000 0 0x400>;
> +		};
> +
>   		spmi_bus: spmi@c440000 {
>   			compatible = "qcom,spmi-pmic-arb";
>   			reg = <0 0x0c440000 0 0x1100>,
> --
> 2.34.1
> 

-- 
Kind Regards,
Caleb (they/he)

Re: [PATCH v2 3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node

[Abel Vesa]

On 22-08-15 21:34:07, Caleb Connolly wrote:
> 
> 
> On 12/08/2022 11:12, Abel Vesa wrote:
> > SDM845 is a special case compared to the other platforms that use RPMh
> > stats, since it only has 2 stats (aosd and cxsd), while the others have
> > a 3rd one (ddr).
> > 
> > So lets add the node but with a SDM845 dedicated compatible to make
> > the driver aware of the different stats config.
> Hi,
> 
> I gave this a go on the OnePlus 6, I noticed the driver is also meant to
> read the stats for remote procs via smem, however this seems to fail for me
> - it can't find any of the SMEM items even if I probe the driver manually
> after ensuring remoteprocs have booted. Is this an unsupported feature on
> SDM845?

Thanks for giving it a test.

Actually, you need to probe the qcom_stats after the remoteprocs have
booted.

Doing so, you'll end up having the following:
adsp  aosd  cdsp  cxsd

> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> > 
> > Changed qcom,rpmh-stats-sdm845 to qcom,sdm845-rpmh-stats, as suggested
> > by Krzysztof.
> > 
> >   arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 5bea96a9ce06..67fe08b837be 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -4851,6 +4851,11 @@ ebi_cdev: ebi {
> >   			};
> >   		};
> > 
> > +		sram@c3f0000 {
> > +			compatible = "qcom,sdm845-rpmh-stats";
> > +			reg = <0 0x0c3f0000 0 0x400>;
> > +		};
> > +
> >   		spmi_bus: spmi@c440000 {
> >   			compatible = "qcom,spmi-pmic-arb";
> >   			reg = <0 0x0c440000 0 0x1100>,
> > --
> > 2.34.1
> > 
> 
> -- 
> Kind Regards,
> Caleb (they/he)

Re: [PATCH v2 3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node

[Caleb Connolly]

On 16/08/2022 11:30, Abel Vesa wrote:
> On 22-08-15 21:34:07, Caleb Connolly wrote:
>>
>>
>> On 12/08/2022 11:12, Abel Vesa wrote:
>>> SDM845 is a special case compared to the other platforms that use RPMh
>>> stats, since it only has 2 stats (aosd and cxsd), while the others have
>>> a 3rd one (ddr).
>>>
>>> So lets add the node but with a SDM845 dedicated compatible to make
>>> the driver aware of the different stats config.
>> Hi,
>>
>> I gave this a go on the OnePlus 6, I noticed the driver is also meant to
>> read the stats for remote procs via smem, however this seems to fail for me
>> - it can't find any of the SMEM items even if I probe the driver manually
>> after ensuring remoteprocs have booted. Is this an unsupported feature on
>> SDM845?
> 
> Thanks for giving it a test.
> 
> Actually, you need to probe the qcom_stats after the remoteprocs have
> booted.

Hi, thanks for getting back to me. I did try this as mentioned above but I think I must have been 
doing something wrong as I get different behaviour now:


enchilada:/ # cat /sys/class/remoteproc/remoteproc*/state

running

running

running

running

enchilada:/ # ls /d/qcom_stats/
aosd  cxsd
enchilada:/ # rmmod qcom_stats
enchilada:/ # insmod /data/qcom_stats.ko
enchilada:/ # ls /d/qcom_stats/
adsp  aosd  cdsp  cxsd  modem  slpi



Weirdly, despite it succeeding it prints the following in dmesg with logging added to 
qcom_create_subsystem_stat_files() [1]:

[  156.540307] Couldn't get smem object 'wpss' (item: 605, pid: 13): -2
[  156.546899] Couldn't get smem object 'gpu' (item: 609, pid: 0): -2
[  156.553260] Couldn't get smem object 'display' (item: 610, pid: 0): -2
[  156.559957] Couldn't get smem object 'adsp_island' (item: 613, pid: 2): -2
[  156.567007] Couldn't get smem object 'slpi_island' (item: 613, pid: 3): -2
> 
> Doing so, you'll end up having the following:
> adsp  aosd  cdsp  cxsd

I seem to get a few more, I have some out of tree patches enabling the SLPI, and iirc the db845c 
doesn't have a full modem firmware. If these look good to you I'd appreciate it if you add my Tested-by.

enchilada:/ # for x in /d/qcom_stats/*; do echo $x; cat $x; done
/d/qcom_stats/adsp
Count: 48
Last Entered At: 1199663157
Last Exited At: 1524359015
Accumulated Duration: 793060082
/d/qcom_stats/aosd
Count: 0
Last Entered At: 0
Last Exited At: 0
Accumulated Duration: 0
/d/qcom_stats/cdsp
Count: 35
Last Entered At: 1194818037
Last Exited At: 1194769648
Accumulated Duration: 3223580811
/d/qcom_stats/cxsd
Count: 0
Last Entered At: 0
Last Exited At: 0
Accumulated Duration: 0
/d/qcom_stats/modem
Count: 49
Last Entered At: 3687081003
Last Exited At: 3686727026
Accumulated Duration: 2915592136
/d/qcom_stats/slpi
Count: 53
Last Entered At: 3120905905
Last Exited At: 3120894535
Accumulated Duration: 3218969498

Am I right in thinking the aosd and cxsd being all 0 is probably a similar issue to the one reported 
by Stephen in [2]?


[1]:

diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
index 121ea409fafc..56cfb20d3683 100644
--- a/drivers/soc/qcom/qcom_stats.c
+++ b/drivers/soc/qcom/qcom_stats.c
@@ -178,8 +178,12 @@ static void qcom_create_subsystem_stat_files(struct dentry *root,

         for (i = 0; i < ARRAY_SIZE(subsystems); i++) {
                 stat = qcom_smem_get(subsystems[i].pid, subsystems[i].smem_item, NULL);
-               if (IS_ERR(stat))
+               if (IS_ERR(stat)) {
+                       pr_info("Couldn't get smem object '%s' (item: %d, pid: %d): %ld\n",
+                               subsystems[i].name, subsystems[i].smem_item, subsystems[i].pid,
+                               PTR_ERR(stat));
                         continue;
+               }

                 debugfs_create_file(subsystems[i].name, 0400, root, (void *)&subsystems[i],
                                     &qcom_subsystem_sleep_stats_fops);



[2]: https://lore.kernel.org/linux-arm-msm/20220628201340.3981860-1-swboyd@chromium.org/
> 
>>>
>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>

Tested-by: Caleb Connolly <caleb.connolly@linaro.org>
>>> ---
>>>
>>> Changed qcom,rpmh-stats-sdm845 to qcom,sdm845-rpmh-stats, as suggested
>>> by Krzysztof.
>>>
>>>    arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>>> index 5bea96a9ce06..67fe08b837be 100644
>>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>>> @@ -4851,6 +4851,11 @@ ebi_cdev: ebi {
>>>    			};
>>>    		};
>>>
>>> +		sram@c3f0000 {
>>> +			compatible = "qcom,sdm845-rpmh-stats";
>>> +			reg = <0 0x0c3f0000 0 0x400>;
>>> +		};
>>> +
>>>    		spmi_bus: spmi@c440000 {
>>>    			compatible = "qcom,spmi-pmic-arb";
>>>    			reg = <0 0x0c440000 0 0x1100>,
>>> --
>>> 2.34.1
>>>
>>
>> -- 
>> Kind Regards,
>> Caleb (they/he)

-- 
Kind Regards,
Caleb (they/he)

Re: [PATCH v2 3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node

[Abel Vesa]

On 22-08-16 14:50:50, Caleb Connolly wrote:
> 
> 
> On 16/08/2022 11:30, Abel Vesa wrote:
> > On 22-08-15 21:34:07, Caleb Connolly wrote:
> > > 
> > > 
> > > On 12/08/2022 11:12, Abel Vesa wrote:
> > > > SDM845 is a special case compared to the other platforms that use RPMh
> > > > stats, since it only has 2 stats (aosd and cxsd), while the others have
> > > > a 3rd one (ddr).
> > > > 
> > > > So lets add the node but with a SDM845 dedicated compatible to make
> > > > the driver aware of the different stats config.
> > > Hi,
> > > 
> > > I gave this a go on the OnePlus 6, I noticed the driver is also meant to
> > > read the stats for remote procs via smem, however this seems to fail for me
> > > - it can't find any of the SMEM items even if I probe the driver manually
> > > after ensuring remoteprocs have booted. Is this an unsupported feature on
> > > SDM845?
> > 
> > Thanks for giving it a test.
> > 
> > Actually, you need to probe the qcom_stats after the remoteprocs have
> > booted.
> 
> Hi, thanks for getting back to me. I did try this as mentioned above but I
> think I must have been doing something wrong as I get different behaviour
> now:
> 
> 
> enchilada:/ # cat /sys/class/remoteproc/remoteproc*/state
> 
> running
> 
> running
> 
> running
> 
> running
> 
> enchilada:/ # ls /d/qcom_stats/
> aosd  cxsd
> enchilada:/ # rmmod qcom_stats
> enchilada:/ # insmod /data/qcom_stats.ko
> enchilada:/ # ls /d/qcom_stats/
> adsp  aosd  cdsp  cxsd  modem  slpi

Well, I run on upstream MTP, which has less enabled in devicetree.

> 
> 
> 
> Weirdly, despite it succeeding it prints the following in dmesg with logging
> added to qcom_create_subsystem_stat_files() [1]:
> 
> [  156.540307] Couldn't get smem object 'wpss' (item: 605, pid: 13): -2
> [  156.546899] Couldn't get smem object 'gpu' (item: 609, pid: 0): -2
> [  156.553260] Couldn't get smem object 'display' (item: 610, pid: 0): -2
> [  156.559957] Couldn't get smem object 'adsp_island' (item: 613, pid: 2): -2
> [  156.567007] Couldn't get smem object 'slpi_island' (item: 613, pid: 3): -2

See my comment below your related changes.

> > 
> > Doing so, you'll end up having the following:
> > adsp  aosd  cdsp  cxsd
> 
> I seem to get a few more, I have some out of tree patches enabling the SLPI,
> and iirc the db845c doesn't have a full modem firmware. If these look good
> to you I'd appreciate it if you add my Tested-by.

Looks OK to me.

> 
> enchilada:/ # for x in /d/qcom_stats/*; do echo $x; cat $x; done
> /d/qcom_stats/adsp
> Count: 48
> Last Entered At: 1199663157
> Last Exited At: 1524359015
> Accumulated Duration: 793060082
> /d/qcom_stats/aosd
> Count: 0
> Last Entered At: 0
> Last Exited At: 0
> Accumulated Duration: 0
> /d/qcom_stats/cdsp
> Count: 35
> Last Entered At: 1194818037
> Last Exited At: 1194769648
> Accumulated Duration: 3223580811
> /d/qcom_stats/cxsd
> Count: 0
> Last Entered At: 0
> Last Exited At: 0
> Accumulated Duration: 0
> /d/qcom_stats/modem
> Count: 49
> Last Entered At: 3687081003
> Last Exited At: 3686727026
> Accumulated Duration: 2915592136
> /d/qcom_stats/slpi
> Count: 53
> Last Entered At: 3120905905
> Last Exited At: 3120894535
> Accumulated Duration: 3218969498
> 
> Am I right in thinking the aosd and cxsd being all 0 is probably a similar
> issue to the one reported by Stephen in [2]?

Might be, I'm not sure. I'll have closer a look.

> 
> 
> [1]:
> 
> diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
> index 121ea409fafc..56cfb20d3683 100644
> --- a/drivers/soc/qcom/qcom_stats.c
> +++ b/drivers/soc/qcom/qcom_stats.c
> @@ -178,8 +178,12 @@ static void qcom_create_subsystem_stat_files(struct dentry *root,
> 
>         for (i = 0; i < ARRAY_SIZE(subsystems); i++) {
>                 stat = qcom_smem_get(subsystems[i].pid, subsystems[i].smem_item, NULL);
> -               if (IS_ERR(stat))

Basically, the error here means the subsystem is not available right at
this moment. We could probably return EPROBE_DEFER here, but it really
depends on each platform's devicetree what remoteprocs they have
enabled or not. So I guess the safest thing to to is to just skip
quietly the ones that haven't probed yet.

> +               if (IS_ERR(stat)) {
> +                       pr_info("Couldn't get smem object '%s' (item: %d, pid: %d): %ld\n",
> +                               subsystems[i].name, subsystems[i].smem_item, subsystems[i].pid,
> +                               PTR_ERR(stat));
>                         continue;
> +               }
> 
>                 debugfs_create_file(subsystems[i].name, 0400, root, (void *)&subsystems[i],
>                                     &qcom_subsystem_sleep_stats_fops);
> 
> 
> 
> [2]: https://lore.kernel.org/linux-arm-msm/20220628201340.3981860-1-swboyd@chromium.org/
> > 
> > > > 
> > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> 
> Tested-by: Caleb Connolly <caleb.connolly@linaro.org>
> > > > ---
> > > > 
> > > > Changed qcom,rpmh-stats-sdm845 to qcom,sdm845-rpmh-stats, as suggested
> > > > by Krzysztof.
> > > > 
> > > >    arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++++
> > > >    1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > index 5bea96a9ce06..67fe08b837be 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > @@ -4851,6 +4851,11 @@ ebi_cdev: ebi {
> > > >    			};
> > > >    		};
> > > > 
> > > > +		sram@c3f0000 {
> > > > +			compatible = "qcom,sdm845-rpmh-stats";
> > > > +			reg = <0 0x0c3f0000 0 0x400>;
> > > > +		};
> > > > +
> > > >    		spmi_bus: spmi@c440000 {
> > > >    			compatible = "qcom,spmi-pmic-arb";
> > > >    			reg = <0 0x0c440000 0 0x1100>,
> > > > --
> > > > 2.34.1
> > > > 
> > > 
> > > -- 
> > > Kind Regards,
> > > Caleb (they/he)
> 
> -- 
> Kind Regards,
> Caleb (they/he)

Re: [PATCH v2 3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node

[Bjorn Andersson]

On Tue 16 Aug 10:32 CDT 2022, Abel Vesa wrote:

> On 22-08-16 14:50:50, Caleb Connolly wrote:
> > 
> > 
> > On 16/08/2022 11:30, Abel Vesa wrote:
> > > On 22-08-15 21:34:07, Caleb Connolly wrote:
> > > > 
> > > > 
> > > > On 12/08/2022 11:12, Abel Vesa wrote:
> > > > > SDM845 is a special case compared to the other platforms that use RPMh
> > > > > stats, since it only has 2 stats (aosd and cxsd), while the others have
> > > > > a 3rd one (ddr).
> > > > > 
> > > > > So lets add the node but with a SDM845 dedicated compatible to make
> > > > > the driver aware of the different stats config.
> > > > Hi,
> > > > 
> > > > I gave this a go on the OnePlus 6, I noticed the driver is also meant to
> > > > read the stats for remote procs via smem, however this seems to fail for me
> > > > - it can't find any of the SMEM items even if I probe the driver manually
> > > > after ensuring remoteprocs have booted. Is this an unsupported feature on
> > > > SDM845?
> > > 
> > > Thanks for giving it a test.
> > > 
> > > Actually, you need to probe the qcom_stats after the remoteprocs have
> > > booted.
> > 
> > Hi, thanks for getting back to me. I did try this as mentioned above but I
> > think I must have been doing something wrong as I get different behaviour
> > now:
> > 
> > 
> > enchilada:/ # cat /sys/class/remoteproc/remoteproc*/state
> > 
> > running
> > 
> > running
> > 
> > running
> > 
> > running
> > 
> > enchilada:/ # ls /d/qcom_stats/
> > aosd  cxsd
> > enchilada:/ # rmmod qcom_stats
> > enchilada:/ # insmod /data/qcom_stats.ko
> > enchilada:/ # ls /d/qcom_stats/
> > adsp  aosd  cdsp  cxsd  modem  slpi
> 
> Well, I run on upstream MTP, which has less enabled in devicetree.
> 
> > 
> > 
> > 
> > Weirdly, despite it succeeding it prints the following in dmesg with logging
> > added to qcom_create_subsystem_stat_files() [1]:
> > 
> > [  156.540307] Couldn't get smem object 'wpss' (item: 605, pid: 13): -2
> > [  156.546899] Couldn't get smem object 'gpu' (item: 609, pid: 0): -2
> > [  156.553260] Couldn't get smem object 'display' (item: 610, pid: 0): -2
> > [  156.559957] Couldn't get smem object 'adsp_island' (item: 613, pid: 2): -2
> > [  156.567007] Couldn't get smem object 'slpi_island' (item: 613, pid: 3): -2
> 
> See my comment below your related changes.
> 
> > > 
> > > Doing so, you'll end up having the following:
> > > adsp  aosd  cdsp  cxsd
> > 
> > I seem to get a few more, I have some out of tree patches enabling the SLPI,
> > and iirc the db845c doesn't have a full modem firmware. If these look good
> > to you I'd appreciate it if you add my Tested-by.
> 
> Looks OK to me.
> 
> > 
> > enchilada:/ # for x in /d/qcom_stats/*; do echo $x; cat $x; done
> > /d/qcom_stats/adsp
> > Count: 48
> > Last Entered At: 1199663157
> > Last Exited At: 1524359015
> > Accumulated Duration: 793060082
> > /d/qcom_stats/aosd
> > Count: 0
> > Last Entered At: 0
> > Last Exited At: 0
> > Accumulated Duration: 0
> > /d/qcom_stats/cdsp
> > Count: 35
> > Last Entered At: 1194818037
> > Last Exited At: 1194769648
> > Accumulated Duration: 3223580811
> > /d/qcom_stats/cxsd
> > Count: 0
> > Last Entered At: 0
> > Last Exited At: 0
> > Accumulated Duration: 0
> > /d/qcom_stats/modem
> > Count: 49
> > Last Entered At: 3687081003
> > Last Exited At: 3686727026
> > Accumulated Duration: 2915592136
> > /d/qcom_stats/slpi
> > Count: 53
> > Last Entered At: 3120905905
> > Last Exited At: 3120894535
> > Accumulated Duration: 3218969498
> > 
> > Am I right in thinking the aosd and cxsd being all 0 is probably a similar
> > issue to the one reported by Stephen in [2]?
> 
> Might be, I'm not sure. I'll have closer a look.
> 
> > 
> > 
> > [1]:
> > 
> > diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
> > index 121ea409fafc..56cfb20d3683 100644
> > --- a/drivers/soc/qcom/qcom_stats.c
> > +++ b/drivers/soc/qcom/qcom_stats.c
> > @@ -178,8 +178,12 @@ static void qcom_create_subsystem_stat_files(struct dentry *root,
> > 
> >         for (i = 0; i < ARRAY_SIZE(subsystems); i++) {
> >                 stat = qcom_smem_get(subsystems[i].pid, subsystems[i].smem_item, NULL);
> > -               if (IS_ERR(stat))
> 
> Basically, the error here means the subsystem is not available right at
> this moment. We could probably return EPROBE_DEFER here, but it really
> depends on each platform's devicetree what remoteprocs they have
> enabled or not. So I guess the safest thing to to is to just skip
> quietly the ones that haven't probed yet.
> 

Not only does the/each remoteproc driver need to probe, we actually need
to wait for them to boot in order to register their parts.

Perhaps we can use qcom_register_ssr_notifier() to dynamically register
these at some later time. This does however seem like a reasonable thing
to improve upon in a follow up series.

Regards,
Bjorn

Re: [PATCH v2 3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node

[Abel Vesa]

On 22-08-17 20:47:23, Bjorn Andersson wrote:
> On Tue 16 Aug 10:32 CDT 2022, Abel Vesa wrote:
> 
> > On 22-08-16 14:50:50, Caleb Connolly wrote:
> > > 
> > > 
> > > On 16/08/2022 11:30, Abel Vesa wrote:
> > > > On 22-08-15 21:34:07, Caleb Connolly wrote:
> > > > > 
> > > > > 
> > > > > On 12/08/2022 11:12, Abel Vesa wrote:
> > > > > > SDM845 is a special case compared to the other platforms that use RPMh
> > > > > > stats, since it only has 2 stats (aosd and cxsd), while the others have
> > > > > > a 3rd one (ddr).
> > > > > > 
> > > > > > So lets add the node but with a SDM845 dedicated compatible to make
> > > > > > the driver aware of the different stats config.
> > > > > Hi,
> > > > > 
> > > > > I gave this a go on the OnePlus 6, I noticed the driver is also meant to
> > > > > read the stats for remote procs via smem, however this seems to fail for me
> > > > > - it can't find any of the SMEM items even if I probe the driver manually
> > > > > after ensuring remoteprocs have booted. Is this an unsupported feature on
> > > > > SDM845?
> > > > 
> > > > Thanks for giving it a test.
> > > > 
> > > > Actually, you need to probe the qcom_stats after the remoteprocs have
> > > > booted.
> > > 
> > > Hi, thanks for getting back to me. I did try this as mentioned above but I
> > > think I must have been doing something wrong as I get different behaviour
> > > now:
> > > 
> > > 
> > > enchilada:/ # cat /sys/class/remoteproc/remoteproc*/state
> > > 
> > > running
> > > 
> > > running
> > > 
> > > running
> > > 
> > > running
> > > 
> > > enchilada:/ # ls /d/qcom_stats/
> > > aosd  cxsd
> > > enchilada:/ # rmmod qcom_stats
> > > enchilada:/ # insmod /data/qcom_stats.ko
> > > enchilada:/ # ls /d/qcom_stats/
> > > adsp  aosd  cdsp  cxsd  modem  slpi
> > 
> > Well, I run on upstream MTP, which has less enabled in devicetree.
> > 
> > > 
> > > 
> > > 
> > > Weirdly, despite it succeeding it prints the following in dmesg with logging
> > > added to qcom_create_subsystem_stat_files() [1]:
> > > 
> > > [  156.540307] Couldn't get smem object 'wpss' (item: 605, pid: 13): -2
> > > [  156.546899] Couldn't get smem object 'gpu' (item: 609, pid: 0): -2
> > > [  156.553260] Couldn't get smem object 'display' (item: 610, pid: 0): -2
> > > [  156.559957] Couldn't get smem object 'adsp_island' (item: 613, pid: 2): -2
> > > [  156.567007] Couldn't get smem object 'slpi_island' (item: 613, pid: 3): -2
> > 
> > See my comment below your related changes.
> > 
> > > > 
> > > > Doing so, you'll end up having the following:
> > > > adsp  aosd  cdsp  cxsd
> > > 
> > > I seem to get a few more, I have some out of tree patches enabling the SLPI,
> > > and iirc the db845c doesn't have a full modem firmware. If these look good
> > > to you I'd appreciate it if you add my Tested-by.
> > 
> > Looks OK to me.
> > 
> > > 
> > > enchilada:/ # for x in /d/qcom_stats/*; do echo $x; cat $x; done
> > > /d/qcom_stats/adsp
> > > Count: 48
> > > Last Entered At: 1199663157
> > > Last Exited At: 1524359015
> > > Accumulated Duration: 793060082
> > > /d/qcom_stats/aosd
> > > Count: 0
> > > Last Entered At: 0
> > > Last Exited At: 0
> > > Accumulated Duration: 0
> > > /d/qcom_stats/cdsp
> > > Count: 35
> > > Last Entered At: 1194818037
> > > Last Exited At: 1194769648
> > > Accumulated Duration: 3223580811
> > > /d/qcom_stats/cxsd
> > > Count: 0
> > > Last Entered At: 0
> > > Last Exited At: 0
> > > Accumulated Duration: 0
> > > /d/qcom_stats/modem
> > > Count: 49
> > > Last Entered At: 3687081003
> > > Last Exited At: 3686727026
> > > Accumulated Duration: 2915592136
> > > /d/qcom_stats/slpi
> > > Count: 53
> > > Last Entered At: 3120905905
> > > Last Exited At: 3120894535
> > > Accumulated Duration: 3218969498
> > > 
> > > Am I right in thinking the aosd and cxsd being all 0 is probably a similar
> > > issue to the one reported by Stephen in [2]?
> > 
> > Might be, I'm not sure. I'll have closer a look.
> > 
> > > 
> > > 
> > > [1]:
> > > 
> > > diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
> > > index 121ea409fafc..56cfb20d3683 100644
> > > --- a/drivers/soc/qcom/qcom_stats.c
> > > +++ b/drivers/soc/qcom/qcom_stats.c
> > > @@ -178,8 +178,12 @@ static void qcom_create_subsystem_stat_files(struct dentry *root,
> > > 
> > >         for (i = 0; i < ARRAY_SIZE(subsystems); i++) {
> > >                 stat = qcom_smem_get(subsystems[i].pid, subsystems[i].smem_item, NULL);
> > > -               if (IS_ERR(stat))
> > 
> > Basically, the error here means the subsystem is not available right at
> > this moment. We could probably return EPROBE_DEFER here, but it really
> > depends on each platform's devicetree what remoteprocs they have
> > enabled or not. So I guess the safest thing to to is to just skip
> > quietly the ones that haven't probed yet.
> > 
> 
> Not only does the/each remoteproc driver need to probe, we actually need
> to wait for them to boot in order to register their parts.
> 
> Perhaps we can use qcom_register_ssr_notifier() to dynamically register
> these at some later time. This does however seem like a reasonable thing
> to improve upon in a follow up series.

Sounds like a good idea. Will give it a try and sent a patch a soon as I
have something working.

Thanks,
Abel

> 
> Regards,
> Bjorn

Re: (subset) [PATCH v2 1/4] arm64: dts: qcom: sdm845: Reduce reg size for aoss_qmp

[Bjorn Andersson]

On Fri, 12 Aug 2022 13:12:37 +0300, Abel Vesa wrote:
> Like on the other platforms that provide RPMh stats, on SDM845, the
> aoss_qmp reg size needs to be reduced to its actual size of 0x400,
> otherwise it will overlap with the RPMh stats reg base, node that will
> be added later on.
> 
> 

Applied, thanks!

[1/4] arm64: dts: qcom: sdm845: Reduce reg size for aoss_qmp
      commit: b0f8e8a38a010999f7b07617d874e3eb594a0a3e
[3/4] arm64: dts: qcom: sdm845: Add the RPMh stats node
      commit: 528dc60f9e5eadcfde651b1458da2b8d008a2cf0

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>