Kevin,
> -----Original Message-----
> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
> Sent: Monday, August 09, 2010 11:59 PM
> To: Sripathy, Vishwanath
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH] OMAP PM: MPU/DMA Latency constraints
>
> Vishwanath Sripathy <vishwanath...@ti.com> writes:
>
> > This patch has implementation for OMAP MPU/DMA Latency constraints using PM
> QOS.
>
> Changelog is missing description of the problem being solved and the
> motivation for the solution.
>
> In particular, a system-wide API is being changed here with no
> description of the problem or the reason for this particular solution.
>
> On first glance, the idea here seems wrong. In getting rid of the device
> pointer, how do you plan to handle per-device constraints?
Sorry for not being a very descriptive.
The motivation here is to remove the dependency on SRF for implementing mpu/dma
latency constraints. Instead they have been implemented using pm_qos
infrastructure.
Latest pm_qos apis take pm_qos handle to distinguish different pm_qos requests
(or use counting mechanism). Hence drivers need to pass pm_qos handle instead
of device pointer. Drivers just to define a handle and this handle gets
initialized when they make the first request. So from driver's point of view,
it's an opaque handle. That's why you see the change in signature of the api.
Regards
Vishwa
>
> Kevin
>
> > Signed-off-by: Vishwanath Sripathy <vishwanath...@ti.com>
> > ---
> > arch/arm/plat-omap/Kconfig | 3 +
> > arch/arm/plat-omap/Makefile | 1 +
> > arch/arm/plat-omap/i2c.c | 3 +-
> > arch/arm/plat-omap/include/plat/omap-pm.h | 9 +-
> > arch/arm/plat-omap/omap-pm-noop.c | 20 +--
> > arch/arm/plat-omap/omap-pm.c | 309
> +++++++++++++++++++++++++++++
> > 6 files changed, 328 insertions(+), 17 deletions(-)
> > create mode 100755 arch/arm/plat-omap/omap-pm.c
> >
> > diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
> > index 286b606..ce544b0 100644
> > --- a/arch/arm/plat-omap/Kconfig
> > +++ b/arch/arm/plat-omap/Kconfig
> > @@ -194,6 +194,9 @@ config OMAP_PM_NONE
> > config OMAP_PM_NOOP
> > bool "No-op/debug PM layer"
> >
> > +config OMAP_PM
> > + depends on PM
> > + bool "OMAP PM layer implementation"
> > endchoice
> >
> > endmenu
> > diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> > index 2a15191..fad2475 100644
> > --- a/arch/arm/plat-omap/Makefile
> > +++ b/arch/arm/plat-omap/Makefile
> > @@ -32,3 +32,4 @@ obj-y += $(i2c-omap-m) $(i2c-omap-y)
> > obj-$(CONFIG_OMAP_MBOX_FWK) += mailbox.o
> >
> > obj-$(CONFIG_OMAP_PM_NOOP) += omap-pm-noop.o
> > +obj-$(CONFIG_OMAP_PM) += omap-pm.o
> > diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c
> > index a5ce4f0..29dc45a
> > --- a/arch/arm/plat-omap/i2c.c
> > +++ b/arch/arm/plat-omap/i2c.c
> > @@ -145,7 +145,8 @@ static inline int omap1_i2c_add_bus(struct
> > platform_device
> *pdev, int bus_id)
> > */
> > static void omap_pm_set_max_mpu_wakeup_lat_compat(struct device *dev, long
> t)
> > {
> > - omap_pm_set_max_mpu_wakeup_lat(dev, t);
> > + struct pm_qos_request_list *qos_handle = NULL;
> > + omap_pm_set_max_mpu_wakeup_lat(&qos_handle, t);
> > }
> >
> > static inline int omap2_i2c_add_bus(struct platform_device *pdev, int
> > bus_id)
> > diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-
> omap/include/plat/omap-pm.h
> > index 728fbb9..47ba9e6 100644
> > --- a/arch/arm/plat-omap/include/plat/omap-pm.h
> > +++ b/arch/arm/plat-omap/include/plat/omap-pm.h
> > @@ -19,6 +19,7 @@
> > #include <linux/clk.h>
> >
> > #include "powerdomain.h"
> > +#include <linux/pm_qos_params.h>
> >
> > /**
> > * struct omap_opp - clock frequency-to-OPP ID table for DSP, MPU
> > @@ -86,7 +87,7 @@ void omap_pm_if_exit(void);
> >
> > /**
> > * omap_pm_set_max_mpu_wakeup_lat - set the maximum MPU wakeup latency
> > - * @dev: struct device * requesting the constraint
> > + * @qos_request: handle for the constraint. The pointer should be
> > initialized to
> NULL
> > * @t: maximum MPU wakeup latency in microseconds
> > *
> > * Request that the maximum interrupt latency for the MPU to be no
> > @@ -118,7 +119,7 @@ void omap_pm_if_exit(void);
> > * Returns -EINVAL for an invalid argument, -ERANGE if the constraint
> > * is not satisfiable, or 0 upon success.
> > */
> > -int omap_pm_set_max_mpu_wakeup_lat(struct device *dev, long t);
> > +int omap_pm_set_max_mpu_wakeup_lat(struct pm_qos_request_list
> **qos_request, long t);
> >
> >
> > /**
> > @@ -185,7 +186,7 @@ int omap_pm_set_max_dev_wakeup_lat(struct device
> *req_dev, struct device *dev,
> >
> > /**
> > * omap_pm_set_max_sdma_lat - set the maximum system DMA transfer start
> latency
> > - * @dev: struct device *
> > + * @qos_request: handle for the constraint. The pointer should be
> > initialized to
> NULL
> > * @t: maximum DMA transfer start latency in microseconds
> > *
> > * Request that the maximum system DMA transfer start latency for this
> > @@ -210,7 +211,7 @@ int omap_pm_set_max_dev_wakeup_lat(struct device
> *req_dev, struct device *dev,
> > * Returns -EINVAL for an invalid argument, -ERANGE if the constraint
> > * is not satisfiable, or 0 upon success.
> > */
> > -int omap_pm_set_max_sdma_lat(struct device *dev, long t);
> > +int omap_pm_set_max_sdma_lat(struct pm_qos_request_list **qos_request, long
> t);
> >
> >
> > /**
> > diff --git a/arch/arm/plat-omap/omap-pm-noop.c b/arch/arm/plat-omap/omap-pm-
> noop.c
> > index e129ce8..af2fe43 100644
> > --- a/arch/arm/plat-omap/omap-pm-noop.c
> > +++ b/arch/arm/plat-omap/omap-pm-noop.c
> > @@ -34,19 +34,17 @@ struct omap_opp *l3_opps;
> > * Device-driver-originated constraints (via board-*.c files)
> > */
> >
> > -int omap_pm_set_max_mpu_wakeup_lat(struct device *dev, long t)
> > +int omap_pm_set_max_mpu_wakeup_lat(struct pm_qos_request_list
> **pmqos_req, long t)
> > {
> > - if (!dev || t < -1) {
> > + if (!pmqos_req || t < -1) {
> > WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
> > return -EINVAL;
> > };
> >
> > if (t == -1)
> > - pr_debug("OMAP PM: remove max MPU wakeup latency constraint: "
> > - "dev %s\n", dev_name(dev));
> > + pr_debug("OMAP PM: remove max MPU wakeup latency constraint\n");
> > else
> > - pr_debug("OMAP PM: add max MPU wakeup latency constraint: "
> > - "dev %s, t = %ld usec\n", dev_name(dev), t);
> > + pr_debug("OMAP PM: add max MPU wakeup latency constraint: t =
> > %ld
> usec\n", t);
> >
> > /*
> > * For current Linux, this needs to map the MPU to a
> > @@ -120,19 +118,17 @@ int omap_pm_set_max_dev_wakeup_lat(struct device
> *req_dev, struct device *dev,
> > return 0;
> > }
> >
> > -int omap_pm_set_max_sdma_lat(struct device *dev, long t)
> > +int omap_pm_set_max_sdma_lat(struct pm_qos_request_list **qos_request, long
> t)
> > {
> > - if (!dev || t < -1) {
> > + if (!qos_request || t < -1) {
> > WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
> > return -EINVAL;
> > };
> >
> > if (t == -1)
> > - pr_debug("OMAP PM: remove max DMA latency constraint: "
> > - "dev %s\n", dev_name(dev));
> > + pr_debug("OMAP PM: remove max DMA latency constraint:\n");
> > else
> > - pr_debug("OMAP PM: add max DMA latency constraint: "
> > - "dev %s, t = %ld usec\n", dev_name(dev), t);
> > + pr_debug("OMAP PM: add max DMA latency constraint: t = %ld
> usec\n", t);
> >
> > /*
> > * For current Linux PM QOS params, this code should scan the
> > diff --git a/arch/arm/plat-omap/omap-pm.c b/arch/arm/plat-omap/omap-pm.c
> > new file mode 100755
> > index 0000000..937196a
> > --- /dev/null
> > +++ b/arch/arm/plat-omap/omap-pm.c
> > @@ -0,0 +1,309 @@
> > +/*
> > + * omap-pm.c - OMAP power management interface
> > + *
> > + * Copyright (C) 2008-2010 Texas Instruments, Inc.
> > + * Copyright (C) 2008-2009 Nokia Corporation
> > + * Vishwanath BS
> > + *
> > + * This code is based on plat-omap/omap-pm-noop.c.
> > + *
> > + * Interface developed by (in alphabetical order):
> > + * Karthik Dasu, Tony Lindgren, Rajendra Nayak, Sakari Poussa,
> Veeramanikandan
> > + * Raju, Anand Sawant, Igor Stoppa, Paul Walmsley, Richard Woodruff
> > + */
> > +
> > +#undef DEBUG
> > +
> > +#include <linux/init.h>
> > +#include <linux/cpufreq.h>
> > +#include <linux/device.h>
> > +
> > +/* Interface documentation is in mach/omap-pm.h */
> > +#include <plat/omap-pm.h>
> > +
> > +#include <plat/powerdomain.h>
> > +
> > +struct omap_opp *dsp_opps;
> > +struct omap_opp *mpu_opps;
> > +struct omap_opp *l3_opps;
> > +
> > +/*
> > + * Device-driver-originated constraints (via board-*.c files)
> > + */
> > +
> > +int omap_pm_set_max_mpu_wakeup_lat(struct pm_qos_request_list
> **qos_request, long t)
> > +{
> > + if (!qos_request || t < -1) {
> > + pr_warning("Warning: invalid params to
> omap_pm_set_max_mpu_wakeup_lat \n:");
> > + return -EINVAL;
> > + };
> > +
> > + if (t == -1) {
> > + pm_qos_remove_request(*qos_request);
> > + *qos_request = NULL;
> > + } else if (*qos_request == NULL)
> > + *qos_request = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> t);
> > + else
> > + pm_qos_update_request(*qos_request, t);
> > +
> > + return 0;
> > +}
> > +
> > +
> > +int omap_pm_set_min_bus_tput(struct device *dev, u8 agent_id, unsigned long
> r)
> > +{
> > + if (!dev || (agent_id != OCP_INITIATOR_AGENT &&
> > + agent_id != OCP_TARGET_AGENT)) {
> > + WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
> > + return -EINVAL;
> > + };
> > +
> > + if (r == 0)
> > + pr_debug("OMAP PM: remove min bus tput constraint: "
> > + "dev %s for agent_id %d\n", dev_name(dev), agent_id);
> > + else
> > + pr_debug("OMAP PM: add min bus tput constraint: "
> > + "dev %s for agent_id %d: rate %ld KiB\n",
> > + dev_name(dev), agent_id, r);
> > +
> > + /*
> > + * This code should model the interconnect and compute the
> > + * required clock frequency, convert that to a VDD2 OPP ID, then
> > + * set the VDD2 OPP appropriately.
> > + *
> > + * TI CDP code can call constraint_set here on the VDD2 OPP.
> > + */
> > +
> > + return 0;
> > +}
> > +
> > +int omap_pm_set_max_dev_wakeup_lat(struct device *req_dev, struct device
> *dev,
> > + long t)
> > +{
> > + if (!req_dev || !dev || t < -1) {
> > + WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
> > + return -EINVAL;
> > + };
> > +
> > + if (t == -1)
> > + pr_debug("OMAP PM: remove max device latency constraint: "
> > + "dev %s\n", dev_name(dev));
> > + else
> > + pr_debug("OMAP PM: add max device latency constraint: "
> > + "dev %s, t = %ld usec\n", dev_name(dev), t);
> > +
> > + /*
> > + * For current Linux, this needs to map the device to a
> > + * powerdomain, then go through the list of current max lat
> > + * constraints on that powerdomain and find the smallest. If
> > + * the latency constraint has changed, the code should
> > + * recompute the state to enter for the next powerdomain
> > + * state. Conceivably, this code should also determine
> > + * whether to actually disable the device clocks or not,
> > + * depending on how long it takes to re-enable the clocks.
> > + *
> > + * TI CDP code can call constraint_set here.
> > + */
> > +
> > + return 0;
> > +}
> > +
> > +int omap_pm_set_max_sdma_lat(struct pm_qos_request_list **qos_request, long
> t)
> > +{
> > + if (!qos_request || t < -1) {
> > + pr_warning("Warning: invalid params to
> omap_pm_set_max_sdma_lat\n:");
> > + return -EINVAL;
> > + };
> > +
> > + if (t == -1) {
> > + pm_qos_remove_request(*qos_request);
> > + *qos_request = NULL;
> > + } else if (*qos_request == NULL)
> > + *qos_request = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> t);
> > + else
> > + pm_qos_update_request(*qos_request, t);
> > +
> > + return 0;
> > +}
> > +
> > +int omap_pm_set_min_clk_rate(struct device *dev, struct clk *c, long r)
> > +{
> > + if (!dev || !c || r < 0) {
> > + WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
> > + return -EINVAL;
> > + }
> > +
> > + if (r == 0)
> > + pr_debug("OMAP PM: remove min clk rate constraint: "
> > + "dev %s\n", dev_name(dev));
> > + else
> > + pr_debug("OMAP PM: add min clk rate constraint: "
> > + "dev %s, rate = %ld Hz\n", dev_name(dev), r);
> > +
> > + /*
> > + * Code in a real implementation should keep track of these
> > + * constraints on the clock, and determine the highest minimum
> > + * clock rate. It should iterate over each OPP and determine
> > + * whether the OPP will result in a clock rate that would
> > + * satisfy this constraint (and any other PM constraint in effect
> > + * at that time). Once it finds the lowest-voltage OPP that
> > + * meets those conditions, it should switch to it, or return
> > + * an error if the code is not capable of doing so.
> > + */
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * DSP Bridge-specific constraints
> > + */
> > +
> > +const struct omap_opp *omap_pm_dsp_get_opp_table(void)
> > +{
> > + pr_debug("OMAP PM: DSP request for OPP table\n");
> > +
> > + /*
> > + * Return DSP frequency table here: The final item in the
> > + * array should have .rate = .opp_id = 0.
> > + */
> > +
> > + return NULL;
> > +}
> > +
> > +void omap_pm_dsp_set_min_opp(u8 opp_id)
> > +{
> > + if (opp_id == 0) {
> > + WARN_ON(1);
> > + return;
> > + }
> > +
> > + pr_debug("OMAP PM: DSP requests minimum VDD1 OPP to be %d\n", opp_id);
> > +
> > + /*
> > + *
> > + * For l-o dev tree, our VDD1 clk is keyed on OPP ID, so we
> > + * can just test to see which is higher, the CPU's desired OPP
> > + * ID or the DSP's desired OPP ID, and use whichever is
> > + * highest.
> > + *
> > + * In CDP12.14+, the VDD1 OPP custom clock that controls the DSP
> > + * rate is keyed on MPU speed, not the OPP ID. So we need to
> > + * map the OPP ID to the MPU speed for use with clk_set_rate()
> > + * if it is higher than the current OPP clock rate.
> > + *
> > + */
> > +}
> > +
> > +
> > +u8 omap_pm_dsp_get_opp(void)
> > +{
> > + pr_debug("OMAP PM: DSP requests current DSP OPP ID\n");
> > +
> > + /*
> > + * For l-o dev tree, call clk_get_rate() on VDD1 OPP clock
> > + *
> > + * CDP12.14+:
> > + * Call clk_get_rate() on the OPP custom clock, map that to an
> > + * OPP ID using the tables defined in board-*.c/chip-*.c files.
> > + */
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * CPUFreq-originated constraint
> > + *
> > + * In the future, this should be handled by custom OPP clocktype
> > + * functions.
> > + */
> > +
> > +struct cpufreq_frequency_table **omap_pm_cpu_get_freq_table(void)
> > +{
> > + pr_debug("OMAP PM: CPUFreq request for frequency table\n");
> > +
> > + /*
> > + * Return CPUFreq frequency table here: loop over
> > + * all VDD1 clkrates, pull out the mpu_ck frequencies, build
> > + * table
> > + */
> > +
> > + return NULL;
> > +}
> > +
> > +void omap_pm_cpu_set_freq(unsigned long f)
> > +{
> > + if (f == 0) {
> > + WARN_ON(1);
> > + return;
> > + }
> > +
> > + pr_debug("OMAP PM: CPUFreq requests CPU frequency to be set to %lu\n",
> > + f);
> > +
> > + /*
> > + * For l-o dev tree, determine whether MPU freq or DSP OPP id
> > + * freq is higher. Find the OPP ID corresponding to the
> > + * higher frequency. Call clk_round_rate() and clk_set_rate()
> > + * on the OPP custom clock.
> > + *
> > + * CDP should just be able to set the VDD1 OPP clock rate here.
> > + */
> > +}
> > +
> > +unsigned long omap_pm_cpu_get_freq(void)
> > +{
> > + pr_debug("OMAP PM: CPUFreq requests current CPU frequency\n");
> > +
> > + /*
> > + * Call clk_get_rate() on the mpu_ck.
> > + */
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Device context loss tracking
> > + */
> > +
> > +int omap_pm_get_dev_context_loss_count(struct device *dev)
> > +{
> > + if (!dev) {
> > + WARN_ON(1);
> > + return -EINVAL;
> > + };
> > +
> > + pr_debug("OMAP PM: returning context loss count for dev %s\n",
> > + dev_name(dev));
> > +
> > + /*
> > + * Map the device to the powerdomain. Return the powerdomain
> > + * off counter.
> > + */
> > +
> > + return 0;
> > +}
> > +
> > +
> > +/* Should be called before clk framework init */
> > +int __init omap_pm_if_early_init(struct omap_opp *mpu_opp_table,
> > + struct omap_opp *dsp_opp_table,
> > + struct omap_opp *l3_opp_table)
> > +{
> > + mpu_opps = mpu_opp_table;
> > + dsp_opps = dsp_opp_table;
> > + l3_opps = l3_opp_table;
> > + return 0;
> > +}
> > +
> > +/* Must be called after clock framework is initialized */
> > +int __init omap_pm_if_init(void)
> > +{
> > + return 0;
> > +}
> > +
> > +void omap_pm_if_exit(void)
> > +{
> > + /* Deallocate CPUFreq frequency table here */
> > +}
> > +
> > +
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
