Re: [PATCH v6 06/10] soc: qcom: rpmhpd: Add RPMh power domain driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/12/2018 3:31 AM, Stephen Boyd wrote:
Quoting Rajendra Nayak (2018-12-11 01:49:34)
The RPMh power domain driver aggregates the corner votes from various
consumers for the ARC resources and communicates it to RPMh.

With RPMh we use 2 different numbering space for corners, one used
by the clients to express their performance needs, and another used
to communicate to RPMh hardware.

The clients express their performance requirements using a sparse
numbering space which are mapped to meaningful levels like RET, SVS,
NOMINAL, TURBO etc which then get mapped to another number space
between 0 and 15 which is communicated to RPMh. The sparse number space,
also referred to as vlvl is mapped to the continuous number space of 0
to 15, also referred to as hlvl, using command DB.

Some power domain clients could request a performance state only while
the CPU is active, while some others could request for a certain
performance state all the time regardless of the state of the CPU.
We handle this by internally aggregating the votes from both type of
clients and then send the aggregated votes to RPMh.

There are also 3 different types of Votes that are comunicated to RPMh

Why capitalize vote?

will fix,


for every resource.
1. ACTIVE_ONLY: This specifies the requirement for the resource when the
CPU is active
2. SLEEP: This specifies the requirement for the resource when the CPU
is going to sleep
3. WAKE_ONLY: This specifies the requirement for the resource when the
CPU is coming out of sleep to active state

Can you tab these in?

sure, will do



We add data for all power domains on sdm845 SoC as part of the patch.
The driver can be extended to support other SoCs which support RPMh

Signed-off-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx>
Reviewed-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>

Just minor nitpicks ahead and a warning.

diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index f1b25fdcf2ad..dd6ca92985ee 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_QCOM_APR) += apr.o
  obj-$(CONFIG_QCOM_LLCC) += llcc-slice.o
  obj-$(CONFIG_QCOM_SDM845_LLCC) += llcc-sdm845.o
  obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
+obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o

Put this before RPMPD? At least it would be semi-sorted then.

okay, will do


diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
new file mode 100644
index 000000000000..f993a86be48c
--- /dev/null
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -0,0 +1,417 @@
[..]
+
+static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
+{
+       u8 buf[RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE];
+       int i, j, len, ret;
+
+       len = cmd_db_read_aux_data_len(rpmhpd->res_name);
+       if (len <= 0)
+               return len;
+       else if (len > RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE)
+               return -EINVAL;
+
+       ret = cmd_db_read_aux_data(rpmhpd->res_name, buf, len);
+       if (ret < 0)
+               return ret;

I've changed cmd_db_read_aux_data() and that change is winding through
the arm-soc tree. This will break in the compilation phase in
linux-next.

will resend these based on Andy's for-next


+
+       rpmhpd->level_count = len / RPMH_ARC_LEVEL_SIZE;
+
+       for (i = 0; i < rpmhpd->level_count; i++) {
+               rpmhpd->level[i] = 0;
+               for (j = 0; j < RPMH_ARC_LEVEL_SIZE; j++)
+                       rpmhpd->level[i] |=
+                               buf[i * RPMH_ARC_LEVEL_SIZE + j] << (8 * j);
+
+               /*
+                * The AUX data may be zero padded.  These 0 valued entries at
+                * the end of the map must be ignored.
+                */
+               if (i > 0 && rpmhpd->level[i] == 0) {
+                       rpmhpd->level_count = i;
+                       break;
+               }
+               pr_debug("%s: ARC hlvl=%2d --> vlvl=%4u\n", rpmhpd->res_name, i,
+                        rpmhpd->level[i]);
+       }
+
+       return 0;
+}
+
+static int rpmhpd_probe(struct platform_device *pdev)
+{
+       int i, ret;
+       size_t num_pds;
+       struct device *dev = &pdev->dev;
+       struct genpd_onecell_data *data;
+       struct rpmhpd **rpmhpds;
+       const struct rpmhpd_desc *desc;
+
+       desc = of_device_get_match_data(dev);
+       if (!desc)
+               return -EINVAL;
+
+       rpmhpds = desc->rpmhpds;
+       num_pds = desc->num_pds;
+
+       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+       if (!data)
+               return -ENOMEM;
+
+       data->domains = devm_kcalloc(dev, num_pds, sizeof(*data->domains),
+                                    GFP_KERNEL);
+       if (!data->domains)
+               return -ENOMEM;
+
+       data->num_domains = num_pds;
+
+       ret = cmd_db_ready();

Does this matter? I thought we forced cmd_db_ready() in the rpmh driver
probe, and that was the parent of all rpmh devices so we should be fine
to not need to check it again here?

sure, will remove.

thanks for the review.


+       if (ret) {
+               if (ret != -EPROBE_DEFER)
+                       dev_err(dev, "Command DB unavailable, ret=%d\n", ret);
+               return ret;
+       }
+



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux