Currently, DEVFREQ reevaluates the device state periodically and/or
based on the OPP list changes. Private API has to be exposed to allow
the device driver to alert/notify the governor to reevaluate when a new
set of data is available. This makes the governor more coupled to a
particular device driver. We can improve here by exposing a DEVFREQ API
to allow the device drivers to send generic alerts to the governor.
Signed-off-by: Akhil P Oommen <[email protected]>
---
drivers/devfreq/devfreq.c | 21 +++++++++++++++++++++
drivers/devfreq/governor.h | 1 +
include/linux/devfreq.h | 5 +++++
3 files changed, 27 insertions(+)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index fe2af6a..24a8046 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1532,3 +1532,24 @@ void devm_devfreq_unregister_notifier(struct device *dev,
devm_devfreq_dev_match, devfreq));
}
EXPORT_SYMBOL(devm_devfreq_unregister_notifier);
+
+/**
+ * devfreq_alert_governor() - Alert governor about an event
+ * @devfreq: The devfreq object.
+ * @type: Optional alert type.
+ *
+ * This function lets the device alert the governor about an event.
+ * A governor may not implement or choose to completely ignore this alert.
+ */
+void devfreq_alert_governor(struct devfreq *devfreq, unsigned int type)
+{
+ /* Don't let someone change the governor until we are done here. */
+ mutex_lock(&devfreq_list_lock);
+
+ if (devfreq)
+ devfreq->governor->event_handler(devfreq,
+ DEVFREQ_GOV_ALERT, &type);
+
+ mutex_unlock(&devfreq_list_lock);
+}
+EXPORT_SYMBOL(devfreq_alert_governor);
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index cfc50a6..e5da3442 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -24,6 +24,7 @@
#define DEVFREQ_GOV_INTERVAL 0x3
#define DEVFREQ_GOV_SUSPEND 0x4
#define DEVFREQ_GOV_RESUME 0x5
+#define DEVFREQ_GOV_ALERT 0x6
/**
* struct devfreq_governor - Devfreq policy governor
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 3aae5b3..740c228 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -193,6 +193,7 @@ extern struct devfreq *devm_devfreq_add_device(struct device *dev,
void *data);
extern void devm_devfreq_remove_device(struct device *dev,
struct devfreq *devfreq);
+extern void devfreq_alert_governor(struct devfreq *devfreq, unsigned int type);
/* Supposed to be called by PM callbacks */
extern int devfreq_suspend_device(struct devfreq *devfreq);
@@ -306,6 +307,10 @@ static inline void devm_devfreq_remove_device(struct device *dev,
{
}
+static void devfreq_alert_governor(struct devfreq *devfreq, unsigned int type)
+{
+}
+
static inline int devfreq_suspend_device(struct devfreq *devfreq)
{
return 0;
--
1.9.1
> Currently, DEVFREQ reevaluates the device state periodically and/or
> based on the OPP list changes. Private API has to be exposed to allow
> the device driver to alert/notify the governor to reevaluate when a new
> set of data is available. This makes the governor more coupled to a
> particular device driver. We can improve here by exposing a DEVFREQ API
> to allow the device drivers to send generic alerts to the governor.
>
> Signed-off-by: Akhil P Oommen <[email protected]>
> ---
> drivers/devfreq/devfreq.c | 21 +++++++++++++++++++++
> drivers/devfreq/governor.h | 1 +
> include/linux/devfreq.h | 5 +++++
> 3 files changed, 27 insertions(+)
>
Hello Akhil,
It appears that this will have the same effect with
"[PATCH 08/11] PM / devfreq: Make update_devfreq() public" from Matthias Kaehlcke, doesn't it?
Cheers,
MyungJoo
On 5/31/2018 11:47 AM, MyungJoo Ham wrote:
>> Currently, DEVFREQ reevaluates the device state periodically and/or
>> based on the OPP list changes. Private API has to be exposed to allow
>> the device driver to alert/notify the governor to reevaluate when a new
>> set of data is available. This makes the governor more coupled to a
>> particular device driver. We can improve here by exposing a DEVFREQ API
>> to allow the device drivers to send generic alerts to the governor.
>>
>> Signed-off-by: Akhil P Oommen <[email protected]>
>> ---
>> drivers/devfreq/devfreq.c | 21 +++++++++++++++++++++
>> drivers/devfreq/governor.h | 1 +
>> include/linux/devfreq.h | 5 +++++
>> 3 files changed, 27 insertions(+)
>>
> Hello Akhil,
>
> It appears that this will have the same effect with
> "[PATCH 08/11] PM / devfreq: Make update_devfreq() public" from Matthias Kaehlcke, doesn't it?
>
>
> Cheers,
> MyungJoo
>
Hi MyngJoo,
The patch you mentioned is a step in the right direction. But this patch
allows:
1. the governor to decide whether to reevaluate or not. I feel it would
be a better architecture (better Separation of Concern) if that decision
is left to the governor alone.
2. the devices to share multiple types of alerts. A governor may use
these alerts for internal bookkeeping/algorithm and decide to reevaluate
policy when it is necessary. Since we are opening up a new devfreq API
for devices, isn't it better to go for a generic one?
Regards,
Akhil.
Hi Akhil,
Actually, this provides custom events for governors, with different
behaviors per each governor, while Matthias's give you the effects not
targeted for a specific governor.
Could you please show me why (the usage cases) you need this
additional event for governors? (You need to implement an event
handler in each governor for this approach).
Cheers,
MyungJoo
On Fri, Jun 1, 2018 at 8:43 PM, Akhil P Oommen <[email protected]> wrote:
>
>
> On 5/31/2018 11:47 AM, MyungJoo Ham wrote:
>>>
>>> Currently, DEVFREQ reevaluates the device state periodically and/or
>>> based on the OPP list changes. Private API has to be exposed to allow
>>> the device driver to alert/notify the governor to reevaluate when a new
>>> set of data is available. This makes the governor more coupled to a
>>> particular device driver. We can improve here by exposing a DEVFREQ API
>>> to allow the device drivers to send generic alerts to the governor.
>>>
>>> Signed-off-by: Akhil P Oommen <[email protected]>
>>> ---
>>> drivers/devfreq/devfreq.c | 21 +++++++++++++++++++++
>>> drivers/devfreq/governor.h | 1 +
>>> include/linux/devfreq.h | 5 +++++
>>> 3 files changed, 27 insertions(+)
>>>
>> Hello Akhil,
>>
>> It appears that this will have the same effect with
>> "[PATCH 08/11] PM / devfreq: Make update_devfreq() public" from Matthias
>> Kaehlcke, doesn't it?
>>
>>
>> Cheers,
>> MyungJoo
>>
> Hi MyngJoo,
>
> The patch you mentioned is a step in the right direction. But this patch
> allows:
> 1. the governor to decide whether to reevaluate or not. I feel it would be a
> better architecture (better Separation of Concern) if that decision is left
> to the governor alone.
> 2. the devices to share multiple types of alerts. A governor may use these
> alerts for internal bookkeeping/algorithm and decide to reevaluate policy
> when it is necessary. Since we are opening up a new devfreq API for devices,
> isn't it better to go for a generic one?
>
> Regards,
> Akhil.
--
MyungJoo Ham, Ph.D.
S/W Center, Samsung Electronics