[RFC] PM / devfreq: Add support for alerts

[RFC] PM / devfreq: Add support for alerts

[Akhil P Oommen]

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 <akhilpo@codeaurora.org>
---
 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

RE: [RFC] PM / devfreq: Add support for alerts

[MyungJoo Ham]

> 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 <akhilpo@codeaurora.org>
> ---
>  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

Re: [RFC] PM / devfreq: Add support for alerts

[Akhil P Oommen]

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 <akhilpo@codeaurora.org>
>> ---
>>   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.

Re: [RFC] PM / devfreq: Add support for alerts

[MyungJoo Ham]

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 <akhilpo@codeaurora.org> 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 <akhilpo@codeaurora.org>
>>> ---
>>>   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