2023-10-10 15:50:23

by Maramaina Naresh

[permalink] [raw]
Subject: [PATCH V1 0/4] Add per-cpu PM QoS support for QCOM UFS

Add per-cpu PM QoS support for ufs. This improves random io performance
by 20% for ufs.

tiotest benchmark tool io performance results on sm8550 platform:

1. Without PM QoS support
Type (Speed in) | Average of 6 iterations
Random Write(IPOS) | 32201
Random Read(IPOS) | 32201

2. With PM QoS support
Type (Speed in) | Average of 6 iterations
Random Write(IPOS) | 40833.5
Random Read(IPOS) | 40833.5
(Improvement % with PM QoS = ~20%).

Maramaina Naresh (4):
dt-bindings: ufs: qcom: Add qos property
ufs: ufs-qcom: Add per-cpu PM QoS support for ufs
ufs: ufs-qcom: Add per-cpu PM QoS vote support for ufs
arm64: dts: qcom: sm8550: Add per-cpu PM QoS support for ufs

.../devicetree/bindings/ufs/qcom,ufs.yaml | 16 +
arch/arm64/boot/dts/qcom/sm8550.dtsi | 9 +
drivers/ufs/host/ufs-qcom.c | 340 ++++++++++++++++++++-
drivers/ufs/host/ufs-qcom.h | 37 +++
4 files changed, 401 insertions(+), 1 deletion(-)

--
2.7.4


2023-10-10 15:50:44

by Maramaina Naresh

[permalink] [raw]
Subject: [PATCH V1 2/4] ufs: ufs-qcom: Add per-cpu PM QoS support for ufs

Qcom SoCs requires registering ufs driver to PM QoS for random io
performance improvement.

PM QoS initialization will parse all QoS CPU group device tree
entries, allocate memory for each individual PM QoS groups and
initiates DEV_PM_QOS_RESUME_LATENCY type of PM QoS request for
new dev_pm_qos_request handle for each CPU. Voting for maximum
latency S32_MAX during driver init.

tiotest benchmark tool io performance results on sm8550 platform:

1. Without PM QoS support
Type (Speed in) | Average of 6 iterations
Random Write(IPOS) | 32201
Random Read(IPOS) | 32201

2. With PM QoS support
Type (Speed in) | Average of 6 iterations
Random Write(IPOS) | 40833.5
Random Read(IPOS) | 40833.5
(Improvement % with PM QoS = ~20%).

Signed-off-by: Asutosh Das <[email protected]>
Signed-off-by: Nitin Rawat <[email protected]>
Signed-off-by: Naveen Kumar Goud Arepalli <[email protected]>
Signed-off-by: Maramaina Naresh <[email protected]>
---
drivers/ufs/host/ufs-qcom.c | 152 ++++++++++++++++++++++++++++++++++++++++++++
drivers/ufs/host/ufs-qcom.h | 29 +++++++++
2 files changed, 181 insertions(+)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 2128db0..f3e1e81 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2013-2016, Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
*/

#include <linux/acpi.h>
@@ -15,6 +16,7 @@
#include <linux/gpio/consumer.h>
#include <linux/reset-controller.h>
#include <linux/devfreq.h>
+#include <linux/cpu.h>

#include <soc/qcom/ice.h>

@@ -736,6 +738,53 @@ static void ufs_qcom_device_reset_ctrl(struct ufs_hba *hba, bool asserted)
gpiod_set_value_cansleep(host->device_reset, asserted);
}

+/**
+ * ufs_qcom_qos_add_cpu_group - inserts QoS CPU group requests into the QoS list
+ * @qcg: pointer to QoS CPU group
+ * @type: defines the qos request
+ *
+ * Returns 0 for success and non-zero for failure.
+ */
+static int ufs_qcom_qos_add_cpu_group(struct ufs_qcom_qcg *qcg,
+ enum constraint type)
+{
+ struct dev_pm_qos_request *qos_req = qcg->qos_req;
+ struct device *dev = qcg->host->hba->dev;
+ int cpu;
+ int ret;
+
+ for_each_cpu(cpu, &qcg->mask) {
+ dev_dbg(dev, "cpu: %d | assoc-qos-req: 0x%p\n",
+ cpu, qos_req);
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ qos_req,
+ DEV_PM_QOS_RESUME_LATENCY,
+ type);
+ if (ret < 0) {
+ dev_err(dev, "Add qos request has failed %d\n",
+ ret);
+ return ret;
+ }
+
+ qos_req++;
+ }
+
+ return 0;
+}
+
+/**
+ * ufs_qcom_qos_remove_cpu_group - removes QoS CPU group requests in QoS list
+ * @qcg: pointer to QoS CPU group
+ */
+static void ufs_qcom_qos_remove_cpu_group(struct ufs_qcom_qcg *qcg)
+{
+ struct dev_pm_qos_request *qos_req = qcg->qos_req;
+ int cpu;
+
+ for_each_cpu(cpu, &qcg->mask)
+ dev_pm_qos_remove_request(qos_req++);
+}
+
static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
enum ufs_notify_change_status status)
{
@@ -1175,6 +1224,107 @@ static int ufs_qcom_icc_init(struct ufs_qcom_host *host)
}

/**
+ * ufs_qcom_qos_exit - de-allocate QoS instances
+ * @hba: per adapter instance
+ */
+static void ufs_qcom_qos_exit(struct ufs_hba *hba)
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+ struct list_head *head = &host->qos_list_head;
+ struct ufs_qcom_qcg *qcg;
+ struct ufs_qcom_qcg *tmp;
+
+ if (list_empty(head))
+ return;
+
+ list_for_each_entry_safe(qcg, tmp, head, list) {
+ if (qcg->qos_req) {
+ ufs_qcom_qos_remove_cpu_group(qcg);
+ kfree(qcg->qos_req);
+ }
+
+ list_del(&qcg->list);
+ kfree(qcg);
+ }
+}
+
+/**
+ * ufs_qcom_qos_init - initialize QoS instances
+ * @hba: host controller instance
+ *
+ * This function parses all QoS CPU group dt entries, allocates memory for
+ * each individual groups and initiates a qos request for each bit of CPU
+ * mask. As part of init all qos requests are initialized with the maximum
+ * delay aggregated constraint value.
+ */
+static void ufs_qcom_qos_init(struct ufs_hba *hba)
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+ struct device *dev = hba->dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *group_node;
+ struct ufs_qcom_qcg *qcg;
+ cpumask_t mask;
+ char buf[10];
+ u32 cpumask = 0;
+ u32 vote = 0;
+ int ret;
+
+ INIT_LIST_HEAD(&host->qos_list_head);
+
+ for_each_available_child_of_node(np, group_node) {
+ if (!strstr(group_node->name, "qos"))
+ continue;
+
+ of_property_read_u32(group_node, "cpumask", &cpumask);
+ sprintf(buf, "%x", cpumask);
+ cpumask_parse(buf, &mask);
+ if (!cpumask || !cpumask_subset(&mask, cpu_possible_mask)) {
+ dev_err(dev, "Invalid group mask\n");
+ goto out_err;
+ }
+
+ of_property_read_u32(group_node, "vote", &vote);
+ if (!vote) {
+ dev_err(dev, "1 vote is needed, bailing out: %u\n",
+ vote);
+ goto out_err;
+ }
+
+ qcg = kzalloc(sizeof(*qcg), GFP_KERNEL);
+ if (!qcg)
+ goto out_err;
+
+ list_add_tail(&qcg->list, &host->qos_list_head);
+ cpumask_copy(&qcg->mask, &mask);
+ qcg->vote = vote;
+ qcg->host = host;
+ qcg->qos_req = kcalloc(cpumask_weight(&qcg->mask),
+ sizeof(struct dev_pm_qos_request),
+ GFP_KERNEL);
+ if (!qcg->qos_req)
+ goto out_err;
+
+ dev_dbg(dev, "qcg: 0x%p | mask-wt: %u | qos_req: 0x%p | vote: %u\n",
+ qcg, cpumask_weight(&qcg->mask),
+ qcg->qos_req, qcg->vote);
+
+ ret = ufs_qcom_qos_add_cpu_group(qcg, S32_MAX);
+ if (ret < 0)
+ goto out_err;
+ }
+
+ if (list_empty(&host->qos_list_head)) {
+ dev_info(dev, "QoS groups undefined\n");
+ return;
+ }
+ return;
+
+out_err:
+ ufs_qcom_qos_exit(hba);
+}
+
+/**
* ufs_qcom_init - bind phy with controller
* @hba: host controller instance
*
@@ -1298,6 +1448,7 @@ static int ufs_qcom_init(struct ufs_hba *hba)
*/
host->phy_gear = UFS_HS_G2;

+ ufs_qcom_qos_init(hba);
return 0;

out_variant_clear:
@@ -2036,6 +2187,7 @@ static void ufs_qcom_remove(struct platform_device *pdev)
struct ufs_hba *hba = platform_get_drvdata(pdev);

pm_runtime_get_sync(&(pdev)->dev);
+ ufs_qcom_qos_exit(hba);
ufshcd_remove(hba);
platform_msi_domain_free_irqs(hba->dev);
}
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index 9950a00..e975a5e 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/* Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
*/

#ifndef UFS_QCOM_H_
@@ -7,6 +8,7 @@

#include <linux/reset-controller.h>
#include <linux/reset.h>
+#include <linux/pm_qos.h>
#include <soc/qcom/ice.h>
#include <ufs/ufshcd.h>

@@ -135,6 +137,31 @@ enum {
#define PA_VS_CORE_CLK_40NS_CYCLES 0x9007
#define PA_VS_CORE_CLK_40NS_CYCLES_MASK GENMASK(6, 0)

+/**
+ * struct ufs_qcom_qcg - context of QoS cpu group
+ * @qos_req: pointer to per cpu pm qos request
+ * @host: pointer to QCOM host controller instance
+ * @list: helper for linked list
+ * @mask: parsed cpumask value from device tree
+ * @vote: parsed vote value from device tree
+ */
+struct ufs_qcom_qcg {
+ struct dev_pm_qos_request *qos_req;
+ struct ufs_qcom_host *host;
+ struct list_head list;
+ cpumask_t mask;
+ u32 vote;
+};
+
+/**
+ * enum constraint - defines QoS constraint type
+ * @QOS_PERF: QoS performance mode
+ * @QOS_POWER: QoS power save mode
+ */
+enum constraint {
+ QOS_PERF,
+ QOS_POWER,
+};

/* QCOM UFS host controller core clk frequencies */
#define UNIPRO_CORE_CLK_FREQ_37_5_MHZ 38
@@ -239,6 +266,8 @@ struct ufs_qcom_host {
struct reset_controller_dev rcdev;

struct gpio_desc *device_reset;
+ /* QoS list head */
+ struct list_head qos_list_head;

u32 phy_gear;

--
2.7.4

2023-10-10 15:51:03

by Maramaina Naresh

[permalink] [raw]
Subject: [PATCH V1 4/4] arm64: dts: qcom: sm8550: Add per-cpu PM QoS support for ufs

Add per-cpu PM QoS support for ufs.

Signed-off-by: Maramaina Naresh <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8550.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 7b9ddde..84909b0 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -1956,6 +1956,15 @@
qcom,ice = <&ice>;

status = "disabled";
+
+ qos0 {
+ cpumask = <0x0f>;
+ vote = <44>;
+ };
+ qos1 {
+ cpumask = <0xf0>;
+ vote = <44>;
+ };
};

ice: crypto@1d88000 {
--
2.7.4

2023-10-10 15:51:21

by Maramaina Naresh

[permalink] [raw]
Subject: [PATCH V1 3/4] ufs: ufs-qcom: Add per-cpu PM QoS vote support for ufs

PM QoS per-cpu framework provides performance requirements for each cpu.

The per-cpu PM QoS framework will provide the corresponding interface to
collect the resume_latency request of the specified device and provide
it to the runtime PM. When suspending the device, it will consider this
requirement and decide whether to suspend the device.

Voting will follow below sequence.
1. Vote for maximum latency S32_MAX in driver init.
2. Schedule a vote of PERF when a transfer request is received.
3. Update the vote to S32_MAX during clock gating.

Signed-off-by: Asutosh Das <[email protected]>
Signed-off-by: Nitin Rawat <[email protected]>
Signed-off-by: Naveen Kumar Goud Arepalli <[email protected]>
Signed-off-by: Maramaina Naresh <[email protected]>
---
drivers/ufs/host/ufs-qcom.c | 188 +++++++++++++++++++++++++++++++++++++++++++-
drivers/ufs/host/ufs-qcom.h | 8 ++
2 files changed, 195 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index f3e1e81..2123e8e 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -17,6 +17,7 @@
#include <linux/reset-controller.h>
#include <linux/devfreq.h>
#include <linux/cpu.h>
+#include <linux/blk-mq.h>

#include <soc/qcom/ice.h>

@@ -27,6 +28,8 @@
#include <ufs/ufshci.h>
#include <ufs/ufs_quirks.h>

+#include <scsi/scsi_cmnd.h>
+
#define MCQ_QCFGPTR_MASK GENMASK(7, 0)
#define MCQ_QCFGPTR_UNIT 0x200
#define MCQ_SQATTR_OFFSET(c) \
@@ -96,6 +99,9 @@ static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS];

static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host);
static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, bool is_scale_up);
+static int ufs_qcom_update_qos_constraints(struct ufs_qcom_qcg *qcg,
+ enum constraint type);
+static int ufs_qcom_qos_unvote_all(struct ufs_qcom_host *host);

static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd)
{
@@ -790,6 +796,7 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
{
struct ufs_qcom_host *host = ufshcd_get_variant(hba);
struct phy *phy = host->generic_phy;
+ int ret;

if (status == PRE_CHANGE)
return 0;
@@ -810,7 +817,11 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
ufs_qcom_disable_lane_clks(host);
}

- return ufs_qcom_ice_suspend(host);
+ ret = ufs_qcom_ice_suspend(host);
+ if (ret)
+ return ret;
+
+ return ufs_qcom_qos_unvote_all(host);
}

static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
@@ -840,6 +851,33 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
return ufs_qcom_ice_resume(host);
}

+/**
+ * ufs_qcom_qos_unvote_all - unvote QoS for all cpu groups
+ * @host: qcom host controller instance
+ */
+static int ufs_qcom_qos_unvote_all(struct ufs_qcom_host *host)
+{
+ struct list_head *head = &host->qos_list_head;
+ struct ufs_qcom_qcg *qcg;
+ int ret = 0;
+
+ if (list_empty(head))
+ return ret;
+
+ list_for_each_entry(qcg, head, list) {
+ flush_work(&qcg->vwork);
+ if (!qcg->voted)
+ continue;
+
+ ret = ufs_qcom_update_qos_constraints(qcg, QOS_POWER);
+ if (ret)
+ dev_err(host->hba->dev, "Failed to update qos constraints, %d\n",
+ ret);
+ }
+
+ return ret;
+}
+
static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)
{
if (host->dev_ref_clk_ctrl_mmio &&
@@ -1157,6 +1195,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
} else {
ufs_qcom_icc_set_bw(host, ufs_qcom_bw_table[MODE_MIN][0][0].mem_bw,
ufs_qcom_bw_table[MODE_MIN][0][0].cfg_bw);
+ ufs_qcom_qos_unvote_all(host);
}
break;
}
@@ -1224,6 +1263,144 @@ static int ufs_qcom_icc_init(struct ufs_qcom_host *host)
}

/**
+ * ufs_qcom_tag_to_cpu - get CPU number for given request tag
+ * @hba: host controller instance
+ * @tag: defines block request id
+ *
+ * Returns 0 or posstive value for success and negative value
+ * for failure.
+ */
+static int ufs_qcom_tag_to_cpu(struct ufs_hba *hba, unsigned int tag)
+{
+ struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+
+ if (lrbp && lrbp->cmd && scsi_cmd_to_rq(lrbp->cmd))
+ return blk_mq_rq_cpu(scsi_cmd_to_rq(lrbp->cmd));
+
+ return -EINVAL;
+}
+
+/**
+ * ufs_qcom_cpu_to_qos_group - returns QoS group address for given CPU number
+ * @host: qcom host controller instance
+ * @cpu: stores CPU number
+ *
+ * Returns ufs_qcom_qcg address for success and NULL for failure.
+ */
+static struct ufs_qcom_qcg *ufs_qcom_cpu_to_qos_group(struct ufs_qcom_host *host,
+ unsigned int cpu)
+{
+ struct list_head *head = &host->qos_list_head;
+ struct ufs_qcom_qcg *qcg;
+
+ if (cpu > num_possible_cpus())
+ return NULL;
+
+ list_for_each_entry(qcg, head, list) {
+ if (cpumask_test_cpu(cpu, &qcg->mask))
+ return qcg;
+ }
+
+ return NULL;
+}
+
+/**
+ * ufs_qcom_update_qos_constraints - update constraints for QoS group
+ * @qcg: pointer to QoS CPU group
+ * @type: defines the qos request
+ *
+ * Returns 0 for success and non-zero for failure.
+ */
+static int ufs_qcom_update_qos_constraints(struct ufs_qcom_qcg *qcg,
+ enum constraint type)
+{
+ struct dev_pm_qos_request *qos_req = qcg->qos_req;
+ struct device *dev = qcg->host->hba->dev;
+ unsigned int vote;
+ int cpu;
+ int ret;
+
+ if (type == QOS_POWER)
+ vote = S32_MAX;
+ else
+ vote = qcg->vote;
+
+ if (qcg->curr_vote == vote)
+ return 0;
+
+ for_each_cpu(cpu, &qcg->mask) {
+ dev_dbg(dev, "%s: vote: %d | cpu: %d | qos_req: 0x%p\n",
+ __func__, vote, cpu, qos_req);
+ ret = dev_pm_qos_update_request(qos_req, vote);
+ if (ret < 0)
+ return ret;
+
+ ++qos_req;
+ }
+
+ if (type == QOS_POWER)
+ qcg->voted = false;
+ else
+ qcg->voted = true;
+ qcg->curr_vote = vote;
+
+ return 0;
+}
+
+/**
+ * ufs_qcom_setup_xfer_req - setup QoS before transfer request is issued
+ * and initiates QoS vote process for given tag
+ * @hba: host controller instance
+ * @tag: defines block request id
+ * @is_scsi_cmd: tells scsi cmd or not
+ *
+ * Returns 0 for success and non-zero for failure.
+ */
+static void ufs_qcom_setup_xfer_req(struct ufs_hba *hba, int tag, bool is_scsi_cmd)
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+ struct list_head *head = &host->qos_list_head;
+ struct ufs_qcom_qcg *qcg;
+ int cpu;
+
+ if (list_empty(head))
+ return;
+
+ cpu = ufs_qcom_tag_to_cpu(hba, tag);
+ if (cpu < 0)
+ return;
+
+ qcg = ufs_qcom_cpu_to_qos_group(host, cpu);
+ if (!qcg)
+ return;
+
+ if (qcg->voted) {
+ dev_dbg(hba->dev, "%s: qcg: 0x%p | cpu: %d\n",
+ __func__, qcg, cpu);
+ return;
+ }
+
+ queue_work(host->qos_workq, &qcg->vwork);
+ dev_dbg(hba->dev, "Queued QoS work- cpu: %d\n", cpu);
+}
+
+/**
+ * ufs_qcom_qos_vote_work - starts QoS perf mode vote work
+ * @work: pointer to QoS work
+ */
+static void ufs_qcom_qos_vote_work(struct work_struct *work)
+{
+ struct ufs_qcom_qcg *qcg = container_of(work, struct ufs_qcom_qcg,
+ vwork);
+ int ret;
+
+ ret = ufs_qcom_update_qos_constraints(qcg, QOS_PERF);
+ if (ret)
+ dev_err(qcg->host->hba->dev, "%s: update qos - failed: %d\n",
+ __func__, ret);
+}
+
+/**
* ufs_qcom_qos_exit - de-allocate QoS instances
* @hba: per adapter instance
*/
@@ -1312,12 +1489,20 @@ static void ufs_qcom_qos_init(struct ufs_hba *hba)
ret = ufs_qcom_qos_add_cpu_group(qcg, S32_MAX);
if (ret < 0)
goto out_err;
+
+ INIT_WORK(&qcg->vwork, ufs_qcom_qos_vote_work);
}

if (list_empty(&host->qos_list_head)) {
dev_info(dev, "QoS groups undefined\n");
return;
}
+
+ host->qos_workq = create_singlethread_workqueue("qc_ufs_qos_swq");
+ if (!host->qos_workq) {
+ dev_err(dev, "Failed to create qos workqueue\n");
+ goto out_err;
+ }
return;

out_err:
@@ -2148,6 +2333,7 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
.dbg_register_dump = ufs_qcom_dump_dbg_regs,
.device_reset = ufs_qcom_device_reset,
.config_scaling_param = ufs_qcom_config_scaling_param,
+ .setup_xfer_req = ufs_qcom_setup_xfer_req,
.program_key = ufs_qcom_ice_program_key,
.reinit_notify = ufs_qcom_reinit_notify,
.mcq_config_resource = ufs_qcom_mcq_config_resource,
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index e975a5e..4ad2b3e 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -141,16 +141,22 @@ enum {
* struct ufs_qcom_qcg - context of QoS cpu group
* @qos_req: pointer to per cpu pm qos request
* @host: pointer to QCOM host controller instance
+ * @vwork: qos vote work handle
* @list: helper for linked list
* @mask: parsed cpumask value from device tree
+ * @curr_vote: current vote value for group
* @vote: parsed vote value from device tree
+ * @voted: vote status
*/
struct ufs_qcom_qcg {
struct dev_pm_qos_request *qos_req;
struct ufs_qcom_host *host;
+ struct work_struct vwork;
struct list_head list;
cpumask_t mask;
+ u32 curr_vote;
u32 vote;
+ bool voted;
};

/**
@@ -268,6 +274,8 @@ struct ufs_qcom_host {
struct gpio_desc *device_reset;
/* QoS list head */
struct list_head qos_list_head;
+ /* QoS workqueue */
+ struct workqueue_struct *qos_workq;

u32 phy_gear;

--
2.7.4

2023-10-10 15:54:07

by Maramaina Naresh

[permalink] [raw]
Subject: [PATCH V1 1/4] dt-bindings: ufs: qcom: Add qos property

Add bindings for per-cpu QoS for QCOM UFS. This improves random io
performance by 20% for QCOM UFS.

Signed-off-by: Maramaina Naresh <[email protected]>
---
Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index 462ead5..d8b807e 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -76,6 +76,14 @@ properties:
$ref: /schemas/types.yaml#/definitions/phandle
description: phandle to the Inline Crypto Engine node

+ qos:
+ minItems: 2
+ maxItems: 2
+ cpumask:
+ description: list of CPUs under CPU group
+ vote:
+ description: vote value for QoS CPU group
+
reg:
minItems: 1
maxItems: 2
@@ -318,5 +326,13 @@ examples:
<0 0>,
<0 0>;
qcom,ice = <&ice>;
+ qos0 {
+ cpumask = <0x0f>;
+ vote = <44>;
+ };
+ qos1 {
+ cpumask = <0xf0>;
+ vote = <44>;
+ };
};
};
--
2.7.4

2023-10-10 16:28:05

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V1 1/4] dt-bindings: ufs: qcom: Add qos property


On Tue, 10 Oct 2023 21:19:04 +0530, Maramaina Naresh wrote:
> Add bindings for per-cpu QoS for QCOM UFS. This improves random io
> performance by 20% for QCOM UFS.
>
> Signed-off-by: Maramaina Naresh <[email protected]>
> ---
> Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/ufs/qcom,ufs.yaml:83:6: [warning] wrong indentation: expected 6 but found 5 (indentation)
./Documentation/devicetree/bindings/ufs/qcom,ufs.yaml:85:6: [warning] wrong indentation: expected 6 but found 5 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: properties:qos: 'anyOf' conditional failed, one must be fixed:
'cpumask' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
'type' was expected
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: properties:qos: 'anyOf' conditional failed, one must be fixed:
'vote' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
'type' was expected
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: qos: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/qcom,ufs.example.dtb: ufs@1d84000: Unevaluated properties are not allowed ('qos0', 'qos1' were unexpected)
from schema $id: http://devicetree.org/schemas/ufs/qcom,ufs.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

2023-10-10 16:37:31

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH V1 1/4] dt-bindings: ufs: qcom: Add qos property



On 10/10/23 17:49, Maramaina Naresh wrote:
> Add bindings for per-cpu QoS for QCOM UFS. This improves random io
> performance by 20% for QCOM UFS.
The bindings addition improves performance? Cool :P

Please explain your changes and provide the "why".

[...]

> @@ -318,5 +326,13 @@ examples:
> <0 0>,
> <0 0>;
> qcom,ice = <&ice>;
> + qos0 {
Looks like the indentation is off, also missing newline before subnodes.

> + cpumask = <0x0f>;This should be a CPU phandle array instead. Besides, can we not
determine this dynamically?

> + vote = <44>;
> + };
> + qos1 {
Missing newline between subnodes.

Konrad

2023-10-10 16:41:54

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH V1 3/4] ufs: ufs-qcom: Add per-cpu PM QoS vote support for ufs



On 10/10/23 17:49, Maramaina Naresh wrote:
> PM QoS per-cpu framework provides performance requirements for each cpu.
>
> The per-cpu PM QoS framework will provide the corresponding interface to
> collect the resume_latency request of the specified device and provide
> it to the runtime PM. When suspending the device, it will consider this
> requirement and decide whether to suspend the device.
>
> Voting will follow below sequence.
> 1. Vote for maximum latency S32_MAX in driver init.
> 2. Schedule a vote of PERF when a transfer request is received.
> 3. Update the vote to S32_MAX during clock gating.
>
> Signed-off-by: Asutosh Das <[email protected]>
> Signed-off-by: Nitin Rawat <[email protected]>
> Signed-off-by: Naveen Kumar Goud Arepalli <[email protected]>
> Signed-off-by: Maramaina Naresh <[email protected]>
> ---
Is this the same patch as 2/4?

Konrad

2023-10-11 07:38:08

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH V1 1/4] dt-bindings: ufs: qcom: Add qos property

Hi Maramaina,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next robh/for-next linus/master v6.6-rc5 next-20231010]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Maramaina-Naresh/dt-bindings-ufs-qcom-Add-qos-property/20231010-235602
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link: https://lore.kernel.org/r/1696952947-18062-2-git-send-email-quic_mnaresh%40quicinc.com
patch subject: [PATCH V1 1/4] dt-bindings: ufs: qcom: Add qos property
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231011/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/ufs/qcom,ufs.yaml:83:6: [warning] wrong indentation: expected 6 but found 5 (indentation)
Documentation/devicetree/bindings/ufs/qcom,ufs.yaml:85:6: [warning] wrong indentation: expected 6 but found 5 (indentation)
--
>> Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: properties:qos: 'anyOf' conditional failed, one must be fixed:
'cpumask' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
'type' was expected
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
>> Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: properties:qos: 'anyOf' conditional failed, one must be fixed:
'vote' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
'type' was expected
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

vim +83 Documentation/devicetree/bindings/ufs/qcom,ufs.yaml

8
9 maintainers:
10 - Bjorn Andersson <[email protected]>
11 - Andy Gross <[email protected]>
12
13 # Select only our matches, not all jedec,ufs-2.0
14 select:
15 properties:
16 compatible:
17 contains:
18 const: qcom,ufshc
19 required:
20 - compatible
21
22 properties:
23 compatible:
24 items:
25 - enum:
26 - qcom,msm8994-ufshc
27 - qcom,msm8996-ufshc
28 - qcom,msm8998-ufshc
29 - qcom,sa8775p-ufshc
30 - qcom,sc8280xp-ufshc
31 - qcom,sdm845-ufshc
32 - qcom,sm6115-ufshc
33 - qcom,sm6350-ufshc
34 - qcom,sm8150-ufshc
35 - qcom,sm8250-ufshc
36 - qcom,sm8350-ufshc
37 - qcom,sm8450-ufshc
38 - qcom,sm8550-ufshc
39 - const: qcom,ufshc
40 - const: jedec,ufs-2.0
41
42 clocks:
43 minItems: 8
44 maxItems: 11
45
46 clock-names:
47 minItems: 8
48 maxItems: 11
49
50 dma-coherent: true
51
52 interconnects:
53 minItems: 2
54 maxItems: 2
55
56 interconnect-names:
57 items:
58 - const: ufs-ddr
59 - const: cpu-ufs
60
61 iommus:
62 minItems: 1
63 maxItems: 2
64
65 phys:
66 maxItems: 1
67
68 phy-names:
69 items:
70 - const: ufsphy
71
72 power-domains:
73 maxItems: 1
74
75 qcom,ice:
76 $ref: /schemas/types.yaml#/definitions/phandle
77 description: phandle to the Inline Crypto Engine node
78
79 qos:
80 minItems: 2
81 maxItems: 2
82 cpumask:
> 83 description: list of CPUs under CPU group
84 vote:
85 description: vote value for QoS CPU group
86
87 reg:
88 minItems: 1
89 maxItems: 2
90
91 reg-names:
92 items:
93 - const: std
94 - const: ice
95
96 required-opps:
97 maxItems: 1
98
99 resets:
100 maxItems: 1
101
102 '#reset-cells':
103 const: 1
104
105 reset-names:
106 items:
107 - const: rst
108
109 reset-gpios:
110 maxItems: 1
111 description:
112 GPIO connected to the RESET pin of the UFS memory device.
113
114 required:
115 - compatible
116 - reg
117
118 allOf:
119 - $ref: ufs-common.yaml
120
121 - if:
122 properties:
123 compatible:
124 contains:
125 enum:
126 - qcom,msm8998-ufshc
127 - qcom,sa8775p-ufshc
128 - qcom,sc8280xp-ufshc
129 - qcom,sm8250-ufshc
130 - qcom,sm8350-ufshc
131 - qcom,sm8450-ufshc
132 - qcom,sm8550-ufshc
133 then:
134 properties:
135 clocks:
136 minItems: 8
137 maxItems: 8
138 clock-names:
139 items:
140 - const: core_clk
141 - const: bus_aggr_clk
142 - const: iface_clk
143 - const: core_clk_unipro
144 - const: ref_clk
145 - const: tx_lane0_sync_clk
146 - const: rx_lane0_sync_clk
147 - const: rx_lane1_sync_clk
148 reg:
149 minItems: 1
150 maxItems: 1
151 reg-names:
152 maxItems: 1
153
154 - if:
155 properties:
156 compatible:
157 contains:
158 enum:
159 - qcom,sdm845-ufshc
160 - qcom,sm6350-ufshc
161 - qcom,sm8150-ufshc
162 then:
163 properties:
164 clocks:
165 minItems: 9
166 maxItems: 9
167 clock-names:
168 items:
169 - const: core_clk
170 - const: bus_aggr_clk
171 - const: iface_clk
172 - const: core_clk_unipro
173 - const: ref_clk
174 - const: tx_lane0_sync_clk
175 - const: rx_lane0_sync_clk
176 - const: rx_lane1_sync_clk
177 - const: ice_core_clk
178 reg:
179 minItems: 2
180 maxItems: 2
181 reg-names:
182 minItems: 2
183 required:
184 - reg-names
185
186 - if:
187 properties:
188 compatible:
189 contains:
190 enum:
191 - qcom,msm8996-ufshc
192 then:
193 properties:
194 clocks:
195 minItems: 11
196 maxItems: 11
197 clock-names:
198 items:
199 - const: core_clk_src
200 - const: core_clk
201 - const: bus_clk
202 - const: bus_aggr_clk
203 - const: iface_clk
204 - const: core_clk_unipro_src
205 - const: core_clk_unipro
206 - const: core_clk_ice
207 - const: ref_clk
208 - const: tx_lane0_sync_clk
209 - const: rx_lane0_sync_clk
210 reg:
211 minItems: 1
212 maxItems: 1
213 reg-names:
214 maxItems: 1
215
216 - if:
217 properties:
218 compatible:
219 contains:
220 enum:
221 - qcom,sm6115-ufshc
222 then:
223 properties:
224 clocks:
225 minItems: 8
226 maxItems: 8
227 clock-names:
228 items:
229 - const: core_clk
230 - const: bus_aggr_clk
231 - const: iface_clk
232 - const: core_clk_unipro
233 - const: ref_clk
234 - const: tx_lane0_sync_clk
235 - const: rx_lane0_sync_clk
236 - const: ice_core_clk
237 reg:
238 minItems: 2
239 maxItems: 2
240 reg-names:
241 minItems: 2
242 required:
243 - reg-names
244
245 # TODO: define clock bindings for qcom,msm8994-ufshc
246
247 - if:
248 required:
249 - qcom,ice
250 then:
251 properties:
252 reg:
253 maxItems: 1
254 clocks:
255 minItems: 8
256 maxItems: 8
257 else:
258 properties:
259 reg:
260 minItems: 1
261 maxItems: 2
262 clocks:
263 minItems: 8
264 maxItems: 11
265

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-10-11 20:57:02

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH V1 0/4] Add per-cpu PM QoS support for QCOM UFS

On 10/10/23 08:49, Maramaina Naresh wrote:
> Add per-cpu PM QoS support for ufs. This improves random io performance
> by 20% for ufs.

What in this patch series (other than the DT-bindings) is specific to
the Qualcomm driver? If the answer is not much: please move this
functionality into the UFS driver core.

Thanks,

Bart.

2023-10-12 12:54:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH V1 1/4] dt-bindings: ufs: qcom: Add qos property

On 10/10/2023 17:49, Maramaina Naresh wrote:
> Add bindings for per-cpu QoS for QCOM UFS. This improves random io
> performance by 20% for QCOM UFS.
>
> Signed-off-by: Maramaina Naresh <[email protected]>
> ---
> Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> index 462ead5..d8b807e 100644
> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> @@ -76,6 +76,14 @@ properties:
> $ref: /schemas/types.yaml#/definitions/phandle
> description: phandle to the Inline Crypto Engine node
>
> + qos:
> + minItems: 2
> + maxItems: 2

This was absolutely never tested.

Best regards,
Krzysztof

2023-10-20 06:47:16

by Maramaina Naresh

[permalink] [raw]
Subject: Re: [PATCH V1 0/4] Add per-cpu PM QoS support for QCOM UFS

Hi Bart,

Thank you for reviewing the patch.  This is not specific to the Qualcomm
driver.

We will move this feature to core UFS.

Thanks,

Naresh.

On 10/12/2023 2:26 AM, Bart Van Assche wrote:
> On 10/10/23 08:49, Maramaina Naresh wrote:
>> Add per-cpu PM QoS support for ufs. This improves random io performance
>> by 20% for ufs.
>
> What in this patch series (other than the DT-bindings) is specific to
> the Qualcomm driver? If the answer is not much: please move this
> functionality into the UFS driver core.
>
> Thanks,
>
> Bart.
>

2023-10-21 17:23:26

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH V1 1/4] dt-bindings: ufs: qcom: Add qos property



On 10/20/23 11:04, Maramaina Naresh wrote:
> Hi All,
>
> Thanks for reviewing the patch.
>
> We are moving this feature support to core UFS driver and planning to control through UFS capability flag [ As per upstream comments ].
>
> We will take care of comments and push new changes in next patch series.
Sounds good.

Please also fix your email client to avoid HTML. The lists will
only accept plaintext messages.

Konrad