Currently, the frequency that devfreq provides the
driver to set always leads the clocks to be scaled up.
Hence, round the clock-rate to the nearest frequency
before deciding to scale.
Also update the devfreq statistics of current frequency.
Signed-off-by: Asutosh Das <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2a2a63b..4607bc6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1187,6 +1187,9 @@ static int ufshcd_devfreq_target(struct device *dev,
if (!ufshcd_is_clkscaling_supported(hba))
return -EINVAL;
+ clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list);
+ /* Override with the closest supported frequency */
+ *freq = (unsigned long) clk_round_rate(clki->clk, *freq);
spin_lock_irqsave(hba->host->host_lock, irq_flags);
if (ufshcd_eh_in_progress(hba)) {
spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
@@ -1201,8 +1204,13 @@ static int ufshcd_devfreq_target(struct device *dev,
goto out;
}
- clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list);
+ /* Decide based on the rounded-off frequency and update */
scale_up = (*freq == clki->max_freq) ? true : false;
+ if (scale_up)
+ *freq = clki->max_freq;
+ else
+ *freq = clki->min_freq;
+ /* Update the frequency */
if (!ufshcd_is_devfreq_scaling_required(hba, scale_up)) {
spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
ret = 0;
@@ -1250,6 +1258,8 @@ static int ufshcd_devfreq_get_dev_status(struct device *dev,
struct ufs_hba *hba = dev_get_drvdata(dev);
struct ufs_clk_scaling *scaling = &hba->clk_scaling;
unsigned long flags;
+ struct list_head *clk_list = &hba->clk_list_head;
+ struct ufs_clk_info *clki;
if (!ufshcd_is_clkscaling_supported(hba))
return -EINVAL;
@@ -1260,6 +1270,8 @@ static int ufshcd_devfreq_get_dev_status(struct device *dev,
if (!scaling->window_start_t)
goto start_window;
+ clki = list_first_entry(clk_list, struct ufs_clk_info, list);
+ stat->current_frequency = clki->curr_freq;
if (scaling->is_busy_started)
scaling->tot_busy_t += ktime_to_us(ktime_sub(ktime_get(),
scaling->busy_start_t));
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Override devfreq parameters for power-performance
trade-off.
Signed-off-by: Asutosh Das <[email protected]>
---
drivers/scsi/ufs/ufs-qcom.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 6115ac6..0fb17e2 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -10,6 +10,7 @@
#include <linux/phy/phy.h>
#include <linux/gpio/consumer.h>
#include <linux/reset-controller.h>
+#include <linux/devfreq.h>
#include "ufshcd.h"
#include "ufshcd-pltfrm.h"
@@ -1689,6 +1690,21 @@ static void ufs_qcom_device_reset(struct ufs_hba *hba)
usleep_range(10, 15);
}
+static void ufs_qcom_config_scaling_param(struct ufs_hba *hba,
+ struct devfreq_dev_profile *p,
+ void *data)
+{
+ static struct devfreq_simple_ondemand_data *d;
+
+ if (!data)
+ return;
+
+ d = (struct devfreq_simple_ondemand_data *)data;
+ p->polling_ms = 60;
+ d->upthreshold = 70;
+ d->downdifferential = 65;
+}
+
/**
* struct ufs_hba_qcom_vops - UFS QCOM specific variant operations
*
@@ -1710,6 +1726,7 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
.resume = ufs_qcom_resume,
.dbg_register_dump = ufs_qcom_dump_dbg_regs,
.device_reset = ufs_qcom_device_reset,
+ .config_scaling_param = ufs_qcom_config_scaling_param,
};
/**
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Vendor drivers may have a need to update the polling interval
and thresholds.
Provide a vops for vendor drivers to use.
Signed-off-by: Asutosh Das <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 11 ++++++++++-
drivers/scsi/ufs/ufshcd.h | 12 ++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4607bc6..7643ef5 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1300,6 +1300,13 @@ static struct devfreq_dev_profile ufs_devfreq_profile = {
.get_dev_status = ufshcd_devfreq_get_dev_status,
};
+#if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
+static struct devfreq_simple_ondemand_data ufs_ondemand_data;
+static void *gov_data = &ufs_ondemand_data;
+#else
+static void *gov_data = NULL;
+#endif
+
static int ufshcd_devfreq_init(struct ufs_hba *hba)
{
struct list_head *clk_list = &hba->clk_list_head;
@@ -1315,10 +1322,12 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
dev_pm_opp_add(hba->dev, clki->min_freq, 0);
dev_pm_opp_add(hba->dev, clki->max_freq, 0);
+ ufshcd_vops_config_scaling_param(hba, &ufs_devfreq_profile,
+ gov_data);
devfreq = devfreq_add_device(hba->dev,
&ufs_devfreq_profile,
DEVFREQ_GOV_SIMPLE_ONDEMAND,
- NULL);
+ gov_data);
if (IS_ERR(devfreq)) {
ret = PTR_ERR(devfreq);
dev_err(hba->dev, "Unable to register with devfreq %d\n", ret);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index d45a044..e8e8ee2 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -56,6 +56,7 @@
#include <linux/completion.h>
#include <linux/regulator/consumer.h>
#include <linux/bitfield.h>
+#include <linux/devfreq.h>
#include "unipro.h"
#include <asm/irq.h>
@@ -327,6 +328,9 @@ struct ufs_hba_variant_ops {
void (*dbg_register_dump)(struct ufs_hba *hba);
int (*phy_initialization)(struct ufs_hba *);
void (*device_reset)(struct ufs_hba *hba);
+ void (*config_scaling_param)(struct ufs_hba *hba,
+ struct devfreq_dev_profile *profile,
+ void *data);
};
/* clock gating state */
@@ -1083,6 +1087,14 @@ static inline void ufshcd_vops_device_reset(struct ufs_hba *hba)
}
}
+static inline void ufshcd_vops_config_scaling_param(struct ufs_hba *hba,
+ struct devfreq_dev_profile
+ *profile, void *data)
+{
+ if (hba->vops && hba->vops->config_scaling_param)
+ hba->vops->config_scaling_param(hba, profile, data);
+}
+
extern struct ufs_pm_lvl_states ufs_pm_lvl_states[];
/*
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>
> Currently, the frequency that devfreq provides the
> driver to set always leads the clocks to be scaled up.
> Hence, round the clock-rate to the nearest frequency
> before deciding to scale.
>
> Also update the devfreq statistics of current frequency.
>
> Signed-off-by: Asutosh Das <[email protected]>
> ---
> drivers/scsi/ufs/ufshcd.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 2a2a63b..4607bc6 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1187,6 +1187,9 @@ static int ufshcd_devfreq_target(struct device
> *dev,
> if (!ufshcd_is_clkscaling_supported(hba))
> return -EINVAL;
>
> + clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list);
> + /* Override with the closest supported frequency */
> + *freq = (unsigned long) clk_round_rate(clki->clk, *freq);
> spin_lock_irqsave(hba->host->host_lock, irq_flags);
Please remind me what the spin lock is protecting here?
> if (ufshcd_eh_in_progress(hba)) {
> spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
> @@ -1201,8 +1204,13 @@ static int ufshcd_devfreq_target(struct device
> *dev,
> goto out;
> }
>
> - clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list);
> + /* Decide based on the rounded-off frequency and update */
> scale_up = (*freq == clki->max_freq) ? true : false;
> + if (scale_up)
> + *freq = clki->max_freq;
This was already established 2 lines above ?
> + else
> + *freq = clki->min_freq;
> + /* Update the frequency */
> if (!ufshcd_is_devfreq_scaling_required(hba, scale_up)) {
> spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
> ret = 0;
> @@ -1250,6 +1258,8 @@ static int ufshcd_devfreq_get_dev_status(struct
> device *dev,
> struct ufs_hba *hba = dev_get_drvdata(dev);
> struct ufs_clk_scaling *scaling = &hba->clk_scaling;
> unsigned long flags;
> + struct list_head *clk_list = &hba->clk_list_head;
> + struct ufs_clk_info *clki;
>
> if (!ufshcd_is_clkscaling_supported(hba))
> return -EINVAL;
> @@ -1260,6 +1270,8 @@ static int ufshcd_devfreq_get_dev_status(struct
> device *dev,
> if (!scaling->window_start_t)
> goto start_window;
>
> + clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> + stat->current_frequency = clki->curr_freq;
Is this a bug fix?
devfreq_simple_ondemand_func is trying to establish the busy period,
but also uses the frequency in its calculation - which I wasn't able to understand how.
Can you add a short comment why updating current_frequency is needed?
Thanks,
Avri
>
>
> Vendor drivers may have a need to update the polling interval
> and thresholds.
> Provide a vops for vendor drivers to use.
>
> Signed-off-by: Asutosh Das <[email protected]>
> ---
> drivers/scsi/ufs/ufshcd.c | 11 ++++++++++-
> drivers/scsi/ufs/ufshcd.h | 12 ++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 4607bc6..7643ef5 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1300,6 +1300,13 @@ static struct devfreq_dev_profile
> ufs_devfreq_profile = {
> .get_dev_status = ufshcd_devfreq_get_dev_status,
> };
>
> +#if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
> +static struct devfreq_simple_ondemand_data ufs_ondemand_data;
Might want to set default values here?
> #include <asm/irq.h>
> @@ -327,6 +328,9 @@ struct ufs_hba_variant_ops {
> void (*dbg_register_dump)(struct ufs_hba *hba);
> int (*phy_initialization)(struct ufs_hba *);
> void (*device_reset)(struct ufs_hba *hba);
> + void (*config_scaling_param)(struct ufs_hba *hba,
> + struct devfreq_dev_profile *profile,
This is an excellent idea, because not only it allows you to edit the polling interval,
But you might want to replace the target and status handlers one day.
> + void *data);
> };
Thanks,
Avri
>
> Override devfreq parameters for power-performance
> trade-off.
>
> Signed-off-by: Asutosh Das <[email protected]>
Reviewed-by: Avri Altman <[email protected]>
On 3/25/2020 6:11 AM, Avri Altman wrote:
>>
>> Currently, the frequency that devfreq provides the
>> driver to set always leads the clocks to be scaled up.
>> Hence, round the clock-rate to the nearest frequency
>> before deciding to scale.
>>
>> Also update the devfreq statistics of current frequency.
>>
>> Signed-off-by: Asutosh Das <[email protected]>
>> ---
>> drivers/scsi/ufs/ufshcd.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 2a2a63b..4607bc6 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -1187,6 +1187,9 @@ static int ufshcd_devfreq_target(struct device
>> *dev,
>> if (!ufshcd_is_clkscaling_supported(hba))
>> return -EINVAL;
>>
>> + clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list);
>> + /* Override with the closest supported frequency */
>> + *freq = (unsigned long) clk_round_rate(clki->clk, *freq);
>> spin_lock_irqsave(hba->host->host_lock, irq_flags);
> Please remind me what the spin lock is protecting here?
Hmmm ... Nothing comes to my mind. I blamed it but it's a part of a
bigger change.
>
>> if (ufshcd_eh_in_progress(hba)) {
>> spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
>> @@ -1201,8 +1204,13 @@ static int ufshcd_devfreq_target(struct device
>> *dev,
>> goto out;
>> }
>>
>> - clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list);
>> + /* Decide based on the rounded-off frequency and update */
>> scale_up = (*freq == clki->max_freq) ? true : false;
>> + if (scale_up)
>> + *freq = clki->max_freq;
> This was already established 2 lines above ?
Good point - I'll change it.
>
>> + else
>> + *freq = clki->min_freq;
>> + /* Update the frequency */
>> if (!ufshcd_is_devfreq_scaling_required(hba, scale_up)) {
>> spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
>> ret = 0;
>> @@ -1250,6 +1258,8 @@ static int ufshcd_devfreq_get_dev_status(struct
>> device *dev,
>> struct ufs_hba *hba = dev_get_drvdata(dev);
>> struct ufs_clk_scaling *scaling = &hba->clk_scaling;
>> unsigned long flags;
>> + struct list_head *clk_list = &hba->clk_list_head;
>> + struct ufs_clk_info *clki;
>>
>> if (!ufshcd_is_clkscaling_supported(hba))
>> return -EINVAL;
>> @@ -1260,6 +1270,8 @@ static int ufshcd_devfreq_get_dev_status(struct
>> device *dev,
>> if (!scaling->window_start_t)
>> goto start_window;
>>
>> + clki = list_first_entry(clk_list, struct ufs_clk_info, list);
>> + stat->current_frequency = clki->curr_freq;
> Is this a bug fix? > devfreq_simple_ondemand_func is trying to establish the busy period,
> but also uses the frequency in its calculation - which I wasn't able to understand how.
> Can you add a short comment why updating current_frequency is needed?
>
Sure - I'll add a comment. If stat->current_frequency is not updated,
the governor would always ask to set the max freq because the initial
frequency was unknown to it. Reference - devfreq_simple_ondemand_func(...)
>
> Thanks,
> Avri
>
Thanks,
-asd
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project