Re: [EXT] Re: SCMI & Devfreq

Re: [EXT] Re: SCMI & Devfreq

[Sudeep Holla]

(Adding linux-pm list)

On Wed, Sep 18, 2019 at 10:53:07PM +0000, Sujeet Kumar Baranwal wrote:
> Sudeep, One trivial question wrt SCMI-CPUFREQ framework.
> 
> The SCMI perf protocol would tell what are different frequencies the
> platform support in the beginning.
>
> For example, the command :
> cat  /sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies
> shows:
> 280000 560000 840000 1120000 1400000 1820000 1960000 2240000 2520000 2800000
>
> /* Attempt to change the frequency */
> ~ # echo 2240000 > /sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed
> It works.
>
> But in a scenario, where user tries a number which is not listed, SCP
> applies its own logic to get a nearby value frequency for CPU and returns.
>
> My question is that could we add some print message in kernel that user wish
> is not exactly fulfilled, an approximation has been done so the user
> explicitly knows his command has been partially met.  If this to happen, a
> patch might be needed in kernel. What is your opinion?
>

May be, you need to check with the maintainers ? :)

The path of execution is:
cpufreq_set(policy, freq) [cpufreq_userspace.c]
__cpufreq_driver_target(policy, freq, CPUFREQ_RELATION_L) [cpufreq_userspace.c]
index = cpufreq_frequency_table_target(policy, target_freq, relation) [cpufreq.c]
__target_index(policy, index) [cpufreq.c]

So if you need logs, it needs to be in core file rather than individual
drivers.

--
Regards,
Sudeep

RE: [EXT] Re: SCMI & Devfreq

[Sujeet Kumar Baranwal]

Hi Sudeep,

Per SCMI perf protocol, the MAX_OPPS macro which is 16, means that at max there could be only 16 OPPs.
In my platform implementation, I tried with 16 OPPs but  when OPPs info came linux perf.c file from SCP, it only showed 12 OPPs only.

Suspecting the rx buffer size, I increased the size to 256 and now the message for all 16 OPPs were reliably received. 

*****************
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 449f713..737d675 100644 (file)
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -612,7 +612,7 @@ int scmi_handle_put(const struct scmi_handle *handle)
 static const struct scmi_desc scmi_generic_desc = {
        .max_rx_timeout_ms = 30,        /* We may increase this if required */
        .max_msg = 20,          /* Limited by MBOX_TX_QUEUE_LEN */
-       .max_msg_size = 128,
+       .max_msg_size = 256,
 };
 
 /* Each compatible listed below must have descriptor associated with it */
*****************
Since this is a legit case of having 16 or more OPPs, we need to have a bugger buffer for rx. Let me know what you think.

Regards
Sujeet
-----Original Message-----
From: Sudeep Holla <sudeep.holla@arm.com> 
Sent: Thursday, September 19, 2019 8:23 AM
To: Sujeet Kumar Baranwal <sbaranwal@marvell.com>
Cc: linux-arm-kernel@lists.infradead.org; Sudeep Holla <sudeep.holla@arm.com>; linux-pm@vger.kernel.org
Subject: Re: [EXT] Re: SCMI & Devfreq

(Adding linux-pm list)

On Wed, Sep 18, 2019 at 10:53:07PM +0000, Sujeet Kumar Baranwal wrote:
> Sudeep, One trivial question wrt SCMI-CPUFREQ framework.
> 
> The SCMI perf protocol would tell what are different frequencies the 
> platform support in the beginning.
>
> For example, the command :
> cat  
> /sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies
> shows:
> 280000 560000 840000 1120000 1400000 1820000 1960000 2240000 2520000 
> 2800000
>
> /* Attempt to change the frequency */
> ~ # echo 2240000 > 
> /sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed
> It works.
>
> But in a scenario, where user tries a number which is not listed, SCP 
> applies its own logic to get a nearby value frequency for CPU and returns.
>
> My question is that could we add some print message in kernel that 
> user wish is not exactly fulfilled, an approximation has been done so 
> the user explicitly knows his command has been partially met.  If this 
> to happen, a patch might be needed in kernel. What is your opinion?
>

May be, you need to check with the maintainers ? :)

The path of execution is:
cpufreq_set(policy, freq) [cpufreq_userspace.c] __cpufreq_driver_target(policy, freq, CPUFREQ_RELATION_L) [cpufreq_userspace.c] index = cpufreq_frequency_table_target(policy, target_freq, relation) [cpufreq.c] __target_index(policy, index) [cpufreq.c]

So if you need logs, it needs to be in core file rather than individual drivers.

--
Regards,
Sudeep

Re: [EXT] Re: SCMI & Devfreq

[Sudeep Holla]

On Mon, Oct 07, 2019 at 06:20:40PM +0000, Sujeet Kumar Baranwal wrote:
> Hi Sudeep,
>
> Per SCMI perf protocol, the MAX_OPPS macro which is 16, means that at max
> there could be only 16 OPPs. In my platform implementation, I tried with 16
> OPPs but  when OPPs info came linux perf.c file from SCP, it only showed 12
> OPPs only.
>
> Suspecting the rx buffer size, I increased the size to 256 and now the
> message for all 16 OPPs were reliably received.
>

OK, but is there any reason why firmware can't use num_levels[31:16]
i.e Number of remaining performance levels and [11:00] i.e.Number of
performance levels that are returned by this call to break and send in
2 calls ? The interface was designed to work with minimum shmem size.


> *****************
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 449f713..737d675 100644 (file)
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -612,7 +612,7 @@ int scmi_handle_put(const struct scmi_handle *handle)
>  static const struct scmi_desc scmi_generic_desc = {
>         .max_rx_timeout_ms = 30,        /* We may increase this if required */
>         .max_msg = 20,          /* Limited by MBOX_TX_QUEUE_LEN */
> -       .max_msg_size = 128,
> +       .max_msg_size = 256,

If you need this, I prefer to introduce new compatible for the platform
scmi and add it as platform specific scmi_desc to start with.

--
Regards,
Sudeep