| Message ID | 20191207002424.201796-4-saravanak@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | Introduce Bandwidth OPPs for interconnects | expand |
On 06-12-19, 16:24, Saravana Kannan wrote: > The frequency OPP tables have helper functions to search for entries in the > table based on frequency and get the frequency values for a given (or > suspend) OPP entry. > > Add similar helper functions for bandwidth OPP tables to search for entries > in the table based on peak bandwidth and to get the peak and average > bandwidth for a given (or suspend) OPP entry. > > Signed-off-by: Saravana Kannan <saravanak@google.com> > --- > drivers/opp/core.c | 301 +++++++++++++++++++++++++++++++++++------ > include/linux/pm_opp.h | 43 ++++++ > 2 files changed, 305 insertions(+), 39 deletions(-) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index c79bbfac7289..3ff33a08198e 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -127,6 +127,29 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp) > } > EXPORT_SYMBOL_GPL(dev_pm_opp_get_freq); > > +/** > + * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an available opp > + * @opp: opp for which peak bandwidth has to be returned for s/peak // > + * @avg_bw: Pointer where the corresponding average bandwidth is stored. > + * Can be NULL. > + * > + * Return: Peak bandwidth in kBps corresponding to the opp, else > + * return 0 > + */ > +unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, unsigned long *avg_bw) > +{ > + if (IS_ERR_OR_NULL(opp) || !opp->available) { > + pr_err("%s: Invalid parameters\n", __func__); > + return 0; > + } > + > + if (avg_bw) Do you see this being NULL in practice ? If no, then we can make it mandatory for now ? > + *avg_bw = opp->avg_bw; > + > + return opp->peak_bw; > +} > +EXPORT_SYMBOL_GPL(dev_pm_opp_get_bw); > + > /** > * dev_pm_opp_get_level() - Gets the level corresponding to an available opp > * @opp: opp for which level value has to be returned for > @@ -299,6 +322,34 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) > } > EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq); > > +/** > + * dev_pm_opp_get_suspend_opp_bw() - Get peak bandwidth of suspend opp in kBps Hmm, I wasn't expecting this. So the interconnects will also have a suspend OPP ? > + * @dev: device for which we do this operation > + * @avg_bw: Pointer where the corresponding average bandwidth is stored. > + * Can be NULL. > + * > + * Return: This function returns the peak bandwidth of the OPP marked as > + * suspend_opp if one is available, else returns 0; > + */ > +unsigned long dev_pm_opp_get_suspend_opp_bw(struct device *dev, > + unsigned long *avg_bw) > +{ > + struct opp_table *opp_table; > + unsigned long peak_bw = 0; > + > + opp_table = _find_opp_table(dev); > + if (IS_ERR(opp_table)) > + return 0; > + > + if (opp_table->suspend_opp && opp_table->suspend_opp->available) > + peak_bw = dev_pm_opp_get_bw(opp_table->suspend_opp, avg_bw); > + > + dev_pm_opp_put_opp_table(opp_table); > + > + return peak_bw; > +} > +EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_bw); > + > int _get_opp_count(struct opp_table *opp_table) > { > struct dev_pm_opp *opp; > @@ -343,6 +394,40 @@ int dev_pm_opp_get_opp_count(struct device *dev) > } > EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count); > I think we should add function header here instead of the helpers which get exact match for freq, bw or level. And then pass a enum value to it, which tells what we are looking to compare. After that rest of the routines will be just one liners, make them macros in header file itself. > +struct dev_pm_opp *dev_pm_opp_find_opp_exact(struct device *dev, > + struct dev_pm_opp *opp_key, > + bool available)
On Wed, Jan 8, 2020 at 3:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 06-12-19, 16:24, Saravana Kannan wrote: > > The frequency OPP tables have helper functions to search for entries in the > > table based on frequency and get the frequency values for a given (or > > suspend) OPP entry. > > > > Add similar helper functions for bandwidth OPP tables to search for entries > > in the table based on peak bandwidth and to get the peak and average > > bandwidth for a given (or suspend) OPP entry. > > > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > --- > > drivers/opp/core.c | 301 +++++++++++++++++++++++++++++++++++------ > > include/linux/pm_opp.h | 43 ++++++ > > 2 files changed, 305 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > index c79bbfac7289..3ff33a08198e 100644 > > --- a/drivers/opp/core.c > > +++ b/drivers/opp/core.c > > @@ -127,6 +127,29 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp) > > } > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_freq); > > > > +/** > > + * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an available opp > > + * @opp: opp for which peak bandwidth has to be returned for > > s/peak // Ack > > > + * @avg_bw: Pointer where the corresponding average bandwidth is stored. > > + * Can be NULL. > > + * > > + * Return: Peak bandwidth in kBps corresponding to the opp, else > > + * return 0 > > + */ > > +unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, unsigned long *avg_bw) > > +{ > > + if (IS_ERR_OR_NULL(opp) || !opp->available) { > > + pr_err("%s: Invalid parameters\n", __func__); > > + return 0; > > + } > > + > > + if (avg_bw) > > Do you see this being NULL in practice ? If no, then we can make it > mandatory for now ? Yes, very likely. A lot of OPP tables might not have avg bandwidth listed. > > + *avg_bw = opp->avg_bw; > > + > > + return opp->peak_bw; > > +} > > +EXPORT_SYMBOL_GPL(dev_pm_opp_get_bw); > > + > > /** > > * dev_pm_opp_get_level() - Gets the level corresponding to an available opp > > * @opp: opp for which level value has to be returned for > > @@ -299,6 +322,34 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) > > } > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq); > > > > +/** > > + * dev_pm_opp_get_suspend_opp_bw() - Get peak bandwidth of suspend opp in kBps > > Hmm, I wasn't expecting this. So the interconnects will also have a > suspend OPP ? Yes, device voting for interconnect paths might want to lower the bandwidth to a suspend bandwidth when they suspend. > > + * @dev: device for which we do this operation > > + * @avg_bw: Pointer where the corresponding average bandwidth is stored. > > + * Can be NULL. > > + * > > + * Return: This function returns the peak bandwidth of the OPP marked as > > + * suspend_opp if one is available, else returns 0; > > + */ > > +unsigned long dev_pm_opp_get_suspend_opp_bw(struct device *dev, > > + unsigned long *avg_bw) > > +{ > > + struct opp_table *opp_table; > > + unsigned long peak_bw = 0; > > + > > + opp_table = _find_opp_table(dev); > > + if (IS_ERR(opp_table)) > > + return 0; > > + > > + if (opp_table->suspend_opp && opp_table->suspend_opp->available) > > + peak_bw = dev_pm_opp_get_bw(opp_table->suspend_opp, avg_bw); > > + > > + dev_pm_opp_put_opp_table(opp_table); > > + > > + return peak_bw; > > +} > > +EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_bw); > > + > > int _get_opp_count(struct opp_table *opp_table) > > { > > struct dev_pm_opp *opp; > > @@ -343,6 +394,40 @@ int dev_pm_opp_get_opp_count(struct device *dev) > > } > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count); > > > > I think we should add function header here instead of the helpers > which get exact match for freq, bw or level. And then pass a enum > value to it, which tells what we are looking to compare. After that > rest of the routines will be just one liners, make them macros in > header file itself. Not sure I understand what you are saying here. > > +struct dev_pm_opp *dev_pm_opp_find_opp_exact(struct device *dev, > > + struct dev_pm_opp *opp_key, > > + bool available) > -Saravana
On Wed, Jan 8, 2020 at 4:58 PM Saravana Kannan <saravanak@google.com> wrote: > > On Wed, Jan 8, 2020 at 3:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 06-12-19, 16:24, Saravana Kannan wrote: > > > The frequency OPP tables have helper functions to search for entries in the > > > table based on frequency and get the frequency values for a given (or > > > suspend) OPP entry. > > > > > > Add similar helper functions for bandwidth OPP tables to search for entries > > > in the table based on peak bandwidth and to get the peak and average > > > bandwidth for a given (or suspend) OPP entry. > > > > > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > > --- > > > drivers/opp/core.c | 301 +++++++++++++++++++++++++++++++++++------ > > > include/linux/pm_opp.h | 43 ++++++ > > > 2 files changed, 305 insertions(+), 39 deletions(-) > > > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > > index c79bbfac7289..3ff33a08198e 100644 > > > --- a/drivers/opp/core.c > > > +++ b/drivers/opp/core.c > > > @@ -127,6 +127,29 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp) > > > } > > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_freq); > > > > > > +/** > > > + * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an available opp > > > + * @opp: opp for which peak bandwidth has to be returned for > > > > s/peak // > > Ack Actually, isn't this correct as is? peak bandwidth is "returned". Average bandwidth is updated through the pointer. -Saravana
On 08-01-20, 16:58, Saravana Kannan wrote: > On Wed, Jan 8, 2020 at 3:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > /** > > > * dev_pm_opp_get_level() - Gets the level corresponding to an available opp > > > * @opp: opp for which level value has to be returned for > > > @@ -299,6 +322,34 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) > > > } > > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq); > > > > > > +/** > > > + * dev_pm_opp_get_suspend_opp_bw() - Get peak bandwidth of suspend opp in kBps > > > > Hmm, I wasn't expecting this. So the interconnects will also have a > > suspend OPP ? > > Yes, device voting for interconnect paths might want to lower the > bandwidth to a suspend bandwidth when they suspend. That's exactly what I was saying, the request for a change during suspend should come from the device and you can't do it here, i.e. they should lower their frequency requirement, which should lead to a low bandwidth automatically. > > > + * @dev: device for which we do this operation > > > + * @avg_bw: Pointer where the corresponding average bandwidth is stored. > > > + * Can be NULL. > > > + * > > > + * Return: This function returns the peak bandwidth of the OPP marked as > > > + * suspend_opp if one is available, else returns 0; > > > + */ > > > +unsigned long dev_pm_opp_get_suspend_opp_bw(struct device *dev, > > > + unsigned long *avg_bw) > > > +{ > > > + struct opp_table *opp_table; > > > + unsigned long peak_bw = 0; > > > + > > > + opp_table = _find_opp_table(dev); > > > + if (IS_ERR(opp_table)) > > > + return 0; > > > + > > > + if (opp_table->suspend_opp && opp_table->suspend_opp->available) > > > + peak_bw = dev_pm_opp_get_bw(opp_table->suspend_opp, avg_bw); > > > + > > > + dev_pm_opp_put_opp_table(opp_table); > > > + > > > + return peak_bw; > > > +} > > > +EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_bw); > > > + > > > int _get_opp_count(struct opp_table *opp_table) > > > { > > > struct dev_pm_opp *opp; > > > @@ -343,6 +394,40 @@ int dev_pm_opp_get_opp_count(struct device *dev) > > > } > > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count); > > > > > > > I think we should add function header here instead of the helpers > > which get exact match for freq, bw or level. And then pass a enum > > value to it, which tells what we are looking to compare. After that > > rest of the routines will be just one liners, make them macros in > > header file itself. > > Not sure I understand what you are saying here. Okay, lemme try again with proper example. enum opp_key_type { OPP_KEY_FREQ = 0x1, OPP_KEY_LEVEL= 0x2, OPP_KEY_BW = 0x4, OPP_KEY_ALL = 0x7, } /** * Add function header here.. */ struct dev_pm_opp *dev_pm_opp_find_opp_exact(struct device *dev, enum opp_key_type key, unsigned long key_value, bool available) { struct opp_table *opp_table; struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { int r = PTR_ERR(opp_table); dev_err(dev, "%s: OPP table not found (%d)\n", __func__, r); return ERR_PTR(r); } mutex_lock(&opp_table->lock); list_for_each_entry(temp_opp, &opp_table->opp_list, node) { if (temp_opp->available == available && !opp_compare_key(temp_opp, key, key_value)) { opp = temp_opp; /* Increment the reference count of OPP */ dev_pm_opp_get(opp); break; } } mutex_unlock(&opp_table->lock); dev_pm_opp_put_opp_table(opp_table); return opp; } //Now in header file #define dev_pm_opp_find_freq_exact(dev, freq, available) dev_pm_opp_find_opp_exact(dev, OPP_KEY_FREQ, freq, available); #define dev_pm_opp_find_level_exact(dev, level, available) dev_pm_opp_find_opp_exact(dev, OPP_KEY_LEVEL, level, available); #define dev_pm_opp_find_bw_exact(dev, bw, available) dev_pm_opp_find_opp_exact(dev, OPP_KEY_BW, bw, available);
On 08-01-20, 19:36, Saravana Kannan wrote: > On Wed, Jan 8, 2020 at 4:58 PM Saravana Kannan <saravanak@google.com> wrote: > > > > On Wed, Jan 8, 2020 at 3:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > On 06-12-19, 16:24, Saravana Kannan wrote: > > > > The frequency OPP tables have helper functions to search for entries in the > > > > table based on frequency and get the frequency values for a given (or > > > > suspend) OPP entry. > > > > > > > > Add similar helper functions for bandwidth OPP tables to search for entries > > > > in the table based on peak bandwidth and to get the peak and average > > > > bandwidth for a given (or suspend) OPP entry. > > > > > > > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > > > --- > > > > drivers/opp/core.c | 301 +++++++++++++++++++++++++++++++++++------ > > > > include/linux/pm_opp.h | 43 ++++++ > > > > 2 files changed, 305 insertions(+), 39 deletions(-) > > > > > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > > > index c79bbfac7289..3ff33a08198e 100644 > > > > --- a/drivers/opp/core.c > > > > +++ b/drivers/opp/core.c > > > > @@ -127,6 +127,29 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp) > > > > } > > > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_freq); > > > > > > > > +/** > > > > + * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an available opp > > > > + * @opp: opp for which peak bandwidth has to be returned for > > > > > > s/peak // > > > > Ack > > Actually, isn't this correct as is? peak bandwidth is "returned". > Average bandwidth is updated through the pointer. I think we return two values here, peak and avg bw. Just that we can't return two values from a routine, and so one is returned using a pointer. And so I though writing just bw may be better.
On Wed, Jan 8, 2020 at 8:40 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 08-01-20, 16:58, Saravana Kannan wrote: > > On Wed, Jan 8, 2020 at 3:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > /** > > > > * dev_pm_opp_get_level() - Gets the level corresponding to an available opp > > > > * @opp: opp for which level value has to be returned for > > > > @@ -299,6 +322,34 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) > > > > } > > > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq); > > > > > > > > +/** > > > > + * dev_pm_opp_get_suspend_opp_bw() - Get peak bandwidth of suspend opp in kBps > > > > > > Hmm, I wasn't expecting this. So the interconnects will also have a > > > suspend OPP ? > > > > Yes, device voting for interconnect paths might want to lower the > > bandwidth to a suspend bandwidth when they suspend. > > That's exactly what I was saying, the request for a change during > suspend should come from the device Agree. And this tells the device requesting for bandwidth, what bandwidth to vote for when it's suspending. Keep in mind that these bandwidth OPP tables are used by the consumers of the interconnects to pick from a list of bandwidths to vote for. > and you can't do it here, i.e. > they should lower their frequency requirement, which should lead to a > low bandwidth automatically. Agree, the OPP framework itself shouldn't be responsible. And I'm not doing anything here? Just giving those devices a way to look up what their suspend bandwidth is? So they can vote for it when they suspend? > > > > + * @dev: device for which we do this operation > > > > + * @avg_bw: Pointer where the corresponding average bandwidth is stored. > > > > + * Can be NULL. > > > > + * > > > > + * Return: This function returns the peak bandwidth of the OPP marked as > > > > + * suspend_opp if one is available, else returns 0; > > > > + */ > > > > +unsigned long dev_pm_opp_get_suspend_opp_bw(struct device *dev, > > > > + unsigned long *avg_bw) > > > > +{ > > > > + struct opp_table *opp_table; > > > > + unsigned long peak_bw = 0; > > > > + > > > > + opp_table = _find_opp_table(dev); > > > > + if (IS_ERR(opp_table)) > > > > + return 0; > > > > + > > > > + if (opp_table->suspend_opp && opp_table->suspend_opp->available) > > > > + peak_bw = dev_pm_opp_get_bw(opp_table->suspend_opp, avg_bw); > > > > + > > > > + dev_pm_opp_put_opp_table(opp_table); > > > > + > > > > + return peak_bw; > > > > +} > > > > +EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_bw); > > > > + > > > > int _get_opp_count(struct opp_table *opp_table) > > > > { > > > > struct dev_pm_opp *opp; > > > > @@ -343,6 +394,40 @@ int dev_pm_opp_get_opp_count(struct device *dev) > > > > } > > > > EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count); > > > > > > > > > > I think we should add function header here instead of the helpers > > > which get exact match for freq, bw or level. And then pass a enum > > > value to it, which tells what we are looking to compare. After that > > > rest of the routines will be just one liners, make them macros in > > > header file itself. > > > > Not sure I understand what you are saying here. > > Okay, lemme try again with proper example. > > enum opp_key_type { > OPP_KEY_FREQ = 0x1, > OPP_KEY_LEVEL= 0x2, > OPP_KEY_BW = 0x4, > OPP_KEY_ALL = 0x7, > } > > /** > * Add function header here.. > */ > struct dev_pm_opp *dev_pm_opp_find_opp_exact(struct device *dev, > enum opp_key_type key, > unsigned long key_value, > bool available) > { > struct opp_table *opp_table; > struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); > > opp_table = _find_opp_table(dev); > if (IS_ERR(opp_table)) { > int r = PTR_ERR(opp_table); > > dev_err(dev, "%s: OPP table not found (%d)\n", __func__, r); > return ERR_PTR(r); > } > > mutex_lock(&opp_table->lock); > > list_for_each_entry(temp_opp, &opp_table->opp_list, node) { > if (temp_opp->available == available && > !opp_compare_key(temp_opp, key, key_value)) { > opp = temp_opp; > > /* Increment the reference count of OPP */ > dev_pm_opp_get(opp); > break; > } > } > > mutex_unlock(&opp_table->lock); > dev_pm_opp_put_opp_table(opp_table); > > return opp; > } > > //Now in header file > > #define dev_pm_opp_find_freq_exact(dev, freq, available) dev_pm_opp_find_opp_exact(dev, OPP_KEY_FREQ, freq, available); > #define dev_pm_opp_find_level_exact(dev, level, available) dev_pm_opp_find_opp_exact(dev, OPP_KEY_LEVEL, level, available); > #define dev_pm_opp_find_bw_exact(dev, bw, available) dev_pm_opp_find_opp_exact(dev, OPP_KEY_BW, bw, available); Ok, but you want this done only for "exact" or for all the other helpers too? Also, this means that I'll have to implement a _opp_compare_key2() or whatever because the generic one that automatically picks the key is still needed for the generic code. Is that fine by you? Also, ack to your other response. -Saravana
On 09-01-20, 10:44, Saravana Kannan wrote: > Agree, the OPP framework itself shouldn't be responsible. And I'm not > doing anything here? Just giving those devices a way to look up what > their suspend bandwidth is? So they can vote for it when they suspend? I think this will originate by itself from the device in case of interconnects as well and you don't need to separately have that for interconnects. For example, the device (lets say GPU) will have one of its OPP (and frequency, maybe the lowest one) marked as suspend-OPP. Then the driver which is doing the co-relation normally between GPU/DDR/Cache OPPs should be able to do this conversion as well without any extra help from the interconnect table. If the minimum freq of the device correspond to the minimum freq of the DDR/Cache during normal operation, that should still work during suspend times, isn't it ? > Ok, but you want this done only for "exact" or for all the other > helpers too? All helpers that you need for PM domains and interconnects. > Also, this means that I'll have to implement a > _opp_compare_key2() or whatever because the generic one that > automatically picks the key is still needed for the generic code. Is > that fine by you? I am not concerned about the number of helpers but their optimization. I will leave it for you to do that and review it when I see how you have done it :)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index c79bbfac7289..3ff33a08198e 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -127,6 +127,29 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp) } EXPORT_SYMBOL_GPL(dev_pm_opp_get_freq); +/** + * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an available opp + * @opp: opp for which peak bandwidth has to be returned for + * @avg_bw: Pointer where the corresponding average bandwidth is stored. + * Can be NULL. + * + * Return: Peak bandwidth in kBps corresponding to the opp, else + * return 0 + */ +unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, unsigned long *avg_bw) +{ + if (IS_ERR_OR_NULL(opp) || !opp->available) { + pr_err("%s: Invalid parameters\n", __func__); + return 0; + } + + if (avg_bw) + *avg_bw = opp->avg_bw; + + return opp->peak_bw; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_get_bw); + /** * dev_pm_opp_get_level() - Gets the level corresponding to an available opp * @opp: opp for which level value has to be returned for @@ -299,6 +322,34 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) } EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq); +/** + * dev_pm_opp_get_suspend_opp_bw() - Get peak bandwidth of suspend opp in kBps + * @dev: device for which we do this operation + * @avg_bw: Pointer where the corresponding average bandwidth is stored. + * Can be NULL. + * + * Return: This function returns the peak bandwidth of the OPP marked as + * suspend_opp if one is available, else returns 0; + */ +unsigned long dev_pm_opp_get_suspend_opp_bw(struct device *dev, + unsigned long *avg_bw) +{ + struct opp_table *opp_table; + unsigned long peak_bw = 0; + + opp_table = _find_opp_table(dev); + if (IS_ERR(opp_table)) + return 0; + + if (opp_table->suspend_opp && opp_table->suspend_opp->available) + peak_bw = dev_pm_opp_get_bw(opp_table->suspend_opp, avg_bw); + + dev_pm_opp_put_opp_table(opp_table); + + return peak_bw; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_bw); + int _get_opp_count(struct opp_table *opp_table) { struct dev_pm_opp *opp; @@ -343,6 +394,40 @@ int dev_pm_opp_get_opp_count(struct device *dev) } EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count); +struct dev_pm_opp *dev_pm_opp_find_opp_exact(struct device *dev, + struct dev_pm_opp *opp_key, + bool available) +{ + struct opp_table *opp_table; + struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); + + opp_table = _find_opp_table(dev); + if (IS_ERR(opp_table)) { + int r = PTR_ERR(opp_table); + + dev_err(dev, "%s: OPP table not found (%d)\n", __func__, r); + return ERR_PTR(r); + } + + mutex_lock(&opp_table->lock); + + list_for_each_entry(temp_opp, &opp_table->opp_list, node) { + if (temp_opp->available == available && + !opp_compare_key(temp_opp, opp_key)) { + opp = temp_opp; + + /* Increment the reference count of OPP */ + dev_pm_opp_get(opp); + break; + } + } + + mutex_unlock(&opp_table->lock); + dev_pm_opp_put_opp_table(opp_table); + + return opp; +} + /** * dev_pm_opp_find_freq_exact() - search for an exact frequency * @dev: device for which we do this operation @@ -370,36 +455,53 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, unsigned long freq, bool available) { - struct opp_table *opp_table; - struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); - - opp_table = _find_opp_table(dev); - if (IS_ERR(opp_table)) { - int r = PTR_ERR(opp_table); + struct dev_pm_opp opp_key; - dev_err(dev, "%s: OPP table not found (%d)\n", __func__, r); - return ERR_PTR(r); - } + opp_key.rate = freq; + opp_key.peak_bw = 0; + opp_key.level = 0; - mutex_lock(&opp_table->lock); + return dev_pm_opp_find_opp_exact(dev, &opp_key, available); +} +EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_exact); - list_for_each_entry(temp_opp, &opp_table->opp_list, node) { - if (temp_opp->available == available && - temp_opp->rate == freq) { - opp = temp_opp; +/** + * dev_pm_opp_find_peak_bw_exact() - search for an exact peak bandwidth + * @dev: device for which we do this operation + * @peak_bw: peak bandwidth to search for + * @available: true/false - match for available opp + * + * Return: Searches for exact match in the opp table and returns pointer to the + * matching opp if found, else returns ERR_PTR in case of error and should + * be handled using IS_ERR. Error return values can be: + * EINVAL: for bad pointer + * ERANGE: no match found for search + * ENODEV: if device not found in list of registered devices + * + * Note: available is a modifier for the search. if available=true, then the + * match is for exact matching peak bandwidth and is available in the stored + * OPP table. if false, the match is for exact peak bandwidth which is not + * available. + * + * This provides a mechanism to enable an opp which is not available currently + * or the opposite as well. + * + * The callers are required to call dev_pm_opp_put() for the returned OPP after + * use. + */ +struct dev_pm_opp *dev_pm_opp_find_peak_bw_exact(struct device *dev, + unsigned int peak_bw, + bool available) +{ + struct dev_pm_opp opp_key; - /* Increment the reference count of OPP */ - dev_pm_opp_get(opp); - break; - } - } + opp_key.rate = 0; + opp_key.peak_bw = peak_bw; + opp_key.level = 0; - mutex_unlock(&opp_table->lock); - dev_pm_opp_put_opp_table(opp_table); - - return opp; + return dev_pm_opp_find_opp_exact(dev, &opp_key, available); } -EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_exact); +EXPORT_SYMBOL_GPL(dev_pm_opp_find_peak_bw_exact); /** * dev_pm_opp_find_level_exact() - search for an exact level @@ -449,18 +551,17 @@ struct dev_pm_opp *dev_pm_opp_find_level_exact(struct device *dev, } EXPORT_SYMBOL_GPL(dev_pm_opp_find_level_exact); -static noinline struct dev_pm_opp *_find_freq_ceil(struct opp_table *opp_table, - unsigned long *freq) +static struct dev_pm_opp *_find_opp_ceil(struct opp_table *opp_table, + struct dev_pm_opp *opp_key) { struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); mutex_lock(&opp_table->lock); list_for_each_entry(temp_opp, &opp_table->opp_list, node) { - if (temp_opp->available && temp_opp->rate >= *freq) { + if (temp_opp->available && + opp_compare_key(temp_opp, opp_key) >= 0) { opp = temp_opp; - *freq = opp->rate; - /* Increment the reference count of OPP */ dev_pm_opp_get(opp); break; @@ -472,6 +573,23 @@ static noinline struct dev_pm_opp *_find_freq_ceil(struct opp_table *opp_table, return opp; } +static noinline struct dev_pm_opp *_find_freq_ceil(struct opp_table *opp_table, + unsigned long *freq) +{ + struct dev_pm_opp opp_key, *opp; + + opp_key.rate = *freq; + opp_key.peak_bw = 0; + opp_key.level = 0; + + opp = _find_opp_ceil(opp_table, &opp_key); + + if (!IS_ERR(opp)) + *freq = opp->rate; + + return opp; +} + /** * dev_pm_opp_find_freq_ceil() - Search for an rounded ceil freq * @dev: device for which we do this operation @@ -514,14 +632,14 @@ struct dev_pm_opp *dev_pm_opp_find_freq_ceil(struct device *dev, EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_ceil); /** - * dev_pm_opp_find_freq_floor() - Search for a rounded floor freq + * dev_pm_opp_find_peak_bw_ceil() - Search for an rounded ceil peak bandwidth * @dev: device for which we do this operation - * @freq: Start frequency + * @peak_bw: Start peak bandwidth * - * Search for the matching floor *available* OPP from a starting freq + * Search for the matching ceil *available* OPP from a starting peak bandwidth * for a device. * - * Return: matching *opp and refreshes *freq accordingly, else returns + * Return: matching *opp and refreshes *peak_bw accordingly, else returns * ERR_PTR in case of error and should be handled using IS_ERR. Error return * values can be: * EINVAL: for bad pointer @@ -531,17 +649,43 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_ceil); * The callers are required to call dev_pm_opp_put() for the returned OPP after * use. */ -struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, - unsigned long *freq) +struct dev_pm_opp *dev_pm_opp_find_peak_bw_ceil(struct device *dev, + unsigned long *peak_bw) { struct opp_table *opp_table; - struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); + struct dev_pm_opp *opp; + struct dev_pm_opp opp_key; - if (!dev || !freq) { - dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq); + if (!dev || !peak_bw) { + dev_err(dev, "%s: Invalid argument peak_bw=%p\n", __func__, + peak_bw); return ERR_PTR(-EINVAL); } + opp_table = _find_opp_table(dev); + if (IS_ERR(opp_table)) + return ERR_CAST(opp_table); + + opp_key.rate = 0; + opp_key.peak_bw = *peak_bw; + opp_key.level = 0; + opp = _find_opp_ceil(opp_table, &opp_key); + + if (!IS_ERR(opp)) + *peak_bw = opp->rate; + + dev_pm_opp_put_opp_table(opp_table); + + return opp; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_find_peak_bw_ceil); + +struct dev_pm_opp *dev_pm_opp_find_opp_floor(struct device *dev, + struct dev_pm_opp *opp_key) +{ + struct opp_table *opp_table; + struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); + opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) return ERR_CAST(opp_table); @@ -551,7 +695,7 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, list_for_each_entry(temp_opp, &opp_table->opp_list, node) { if (temp_opp->available) { /* go to the next node, before choosing prev */ - if (temp_opp->rate > *freq) + if (opp_compare_key(temp_opp, opp_key) > 0) break; else opp = temp_opp; @@ -564,6 +708,43 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, mutex_unlock(&opp_table->lock); dev_pm_opp_put_opp_table(opp_table); + return opp; +} + +/** + * dev_pm_opp_find_freq_floor() - Search for a rounded floor freq + * @dev: device for which we do this operation + * @freq: Start frequency + * + * Search for the matching floor *available* OPP from a starting freq + * for a device. + * + * Return: matching *opp and refreshes *freq accordingly, else returns + * ERR_PTR in case of error and should be handled using IS_ERR. Error return + * values can be: + * EINVAL: for bad pointer + * ERANGE: no match found for search + * ENODEV: if device not found in list of registered devices + * + * The callers are required to call dev_pm_opp_put() for the returned OPP after + * use. + */ +struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, + unsigned long *freq) +{ + struct dev_pm_opp opp_key, *opp; + + if (!dev || !freq) { + dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq); + return ERR_PTR(-EINVAL); + } + + opp_key.rate = *freq; + opp_key.peak_bw = 0; + opp_key.level = 0; + + opp = dev_pm_opp_find_opp_floor(dev, &opp_key); + if (!IS_ERR(opp)) *freq = opp->rate; @@ -571,6 +752,48 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, } EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor); +/** + * dev_pm_opp_find_peak_bw_floor() - Search for a rounded floor peak bandwidth + * @dev: device for which we do this operation + * @peak_bw: Start peak bandwidth + * + * Search for the matching floor *available* OPP from a starting peak bandwidth + * for a device. + * + * Return: matching *opp and refreshes *peak_bw accordingly, else returns + * ERR_PTR in case of error and should be handled using IS_ERR. Error return + * values can be: + * EINVAL: for bad pointer + * ERANGE: no match found for search + * ENODEV: if device not found in list of registered devices + * + * The callers are required to call dev_pm_opp_put() for the returned OPP after + * use. + */ +struct dev_pm_opp *dev_pm_opp_find_peak_bw_floor(struct device *dev, + unsigned int *peak_bw) +{ + struct dev_pm_opp opp_key, *opp; + + if (!dev || !peak_bw) { + dev_err(dev, "%s: Invalid argument peak_bw=%p\n", __func__, + peak_bw); + return ERR_PTR(-EINVAL); + } + + opp_key.rate = 0; + opp_key.peak_bw = *peak_bw; + opp_key.level = 0; + + opp = dev_pm_opp_find_opp_floor(dev, &opp_key); + + if (!IS_ERR(opp)) + *peak_bw = opp->peak_bw; + + return opp; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_find_peak_bw_floor); + /** * dev_pm_opp_find_freq_ceil_by_volt() - Find OPP with highest frequency for * target voltage. diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 747861816f4f..aab1b9c143ac 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -83,6 +83,7 @@ void dev_pm_opp_put_opp_table(struct opp_table *opp_table); unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp); unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp); +unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, unsigned long *avg_bw); unsigned int dev_pm_opp_get_level(struct dev_pm_opp *opp); @@ -93,20 +94,29 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev); unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev); unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev); unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev); +unsigned long dev_pm_opp_get_suspend_opp_bw(struct device *dev, + unsigned long *avg_bw); struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, unsigned long freq, bool available); +struct dev_pm_opp *dev_pm_opp_find_peak_bw_exact(struct device *dev, + unsigned int peak_bw, + bool available); struct dev_pm_opp *dev_pm_opp_find_level_exact(struct device *dev, unsigned int level); struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, unsigned long *freq); +struct dev_pm_opp *dev_pm_opp_find_peak_bw_floor(struct device *dev, + unsigned int *peak_bw); struct dev_pm_opp *dev_pm_opp_find_freq_ceil_by_volt(struct device *dev, unsigned long u_volt); struct dev_pm_opp *dev_pm_opp_find_freq_ceil(struct device *dev, unsigned long *freq); +struct dev_pm_opp *dev_pm_opp_find_peak_bw_ceil(struct device *dev, + unsigned long *peak_bw); void dev_pm_opp_put(struct dev_pm_opp *opp); int dev_pm_opp_add(struct device *dev, unsigned long freq, @@ -165,6 +175,11 @@ static inline unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp) { return 0; } +static inline unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, + unsigned long *avg_bw) +{ + return 0; +} static inline unsigned int dev_pm_opp_get_level(struct dev_pm_opp *opp) { @@ -201,12 +216,26 @@ static inline unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) return 0; } +static inline unsigned long dev_pm_opp_get_suspend_opp_bw(struct device *dev, + unsigned long *avg_bw) +{ + return 0; +} + static inline struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, unsigned long freq, bool available) { return ERR_PTR(-ENOTSUPP); } +static inline struct dev_pm_opp *dev_pm_opp_find_peak_bw_exact( + struct device *dev, + unsigned int peak_bw, + bool available) +{ + return ERR_PTR(-ENOTSUPP); +} + static inline struct dev_pm_opp *dev_pm_opp_find_level_exact(struct device *dev, unsigned int level) { @@ -219,6 +248,13 @@ static inline struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, return ERR_PTR(-ENOTSUPP); } +static inline struct dev_pm_opp *dev_pm_opp_find_peak_bw_floor( + struct device *dev, + unsigned int *peak_bw) +{ + return ERR_PTR(-ENOTSUPP); +} + static inline struct dev_pm_opp *dev_pm_opp_find_freq_ceil_by_volt(struct device *dev, unsigned long u_volt) { @@ -231,6 +267,13 @@ static inline struct dev_pm_opp *dev_pm_opp_find_freq_ceil(struct device *dev, return ERR_PTR(-ENOTSUPP); } +static inline struct dev_pm_opp *dev_pm_opp_find_peak_bw_ceil( + struct device *dev, + unsigned long *peak_bw) +{ + return ERR_PTR(-ENOTSUPP); +} + static inline void dev_pm_opp_put(struct dev_pm_opp *opp) {} static inline int dev_pm_opp_add(struct device *dev, unsigned long freq,
The frequency OPP tables have helper functions to search for entries in the table based on frequency and get the frequency values for a given (or suspend) OPP entry. Add similar helper functions for bandwidth OPP tables to search for entries in the table based on peak bandwidth and to get the peak and average bandwidth for a given (or suspend) OPP entry. Signed-off-by: Saravana Kannan <saravanak@google.com> --- drivers/opp/core.c | 301 +++++++++++++++++++++++++++++++++++------ include/linux/pm_opp.h | 43 ++++++ 2 files changed, 305 insertions(+), 39 deletions(-)