RE: [PATCH] OMAP PM: MPU/DMA Latency constraints

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

 



Kevin, Paul,

Do you have any comments on this patch?

Regards
Vishwa

> -----Original Message-----
> From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Sripathy, Vishwanath
> Sent: Tuesday, August 10, 2010 10:43 AM
> To: Kevin Hilman
> Cc: linux-omap@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH] OMAP PM: MPU/DMA Latency constraints
>
> Kevin,
>
> > -----Original Message-----
> > From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx]
> > Sent: Monday, August 09, 2010 11:59 PM
> > To: Sripathy, Vishwanath
> > Cc: linux-omap@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH] OMAP PM: MPU/DMA Latency constraints
> >
> > Vishwanath Sripathy <vishwanath.bs@xxxxxx> 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.bs@xxxxxx>
> > > ---
> > >  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 majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux