[v10,06/11] PM / QoS: Reorder pm_qos/freq_qos/dev_pm_qos structs

Commit Message

Leonard Crestez Oct. 31, 2019, 9:34 p.m. UTC

This allows dev_pm_qos to embed freq_qos structs, which is done in the
next patch. Separate commit to make it easier to review.

Signed-off-by: Leonard Crestez <[e-mail]>
---
 include/linux/pm_qos.h | 74 ++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 36 deletions(-)

Comments

Chanwoo Choi Nov. 1, 2019, 2:13 a.m. UTC | #1

Hi Leonard,

Why do you add the new patches on v10 (patch6/7/8)
in this series? If you think that need to update
the pm_qos core, you have to send the new patchset
on separate mail thread. It make the difficult
to merge the PM_QoS support of devfreq.

Please split out the patch6/7/8 from this series.

On 19. 11. 1. 오전 6:34, Leonard Crestez wrote:
> This allows dev_pm_qos to embed freq_qos structs, which is done in the
> next patch. Separate commit to make it easier to review.
> 
> Signed-off-by: Leonard Crestez <[e-mail]>
> ---
>  include/linux/pm_qos.h | 74 ++++++++++++++++++++++--------------------
>  1 file changed, 38 insertions(+), 36 deletions(-)
> 
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index c35ff21e8a40..a8e1486e3200 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -47,25 +47,10 @@ struct pm_qos_request {
>  struct pm_qos_flags_request {
>  	struct list_head node;
>  	s32 flags;	/* Do not change to 64 bit */
>  };
>  
> -enum dev_pm_qos_req_type {
> -	DEV_PM_QOS_RESUME_LATENCY = 1,
> -	DEV_PM_QOS_LATENCY_TOLERANCE,
> -	DEV_PM_QOS_FLAGS,
> -};
> -
> -struct dev_pm_qos_request {
> -	enum dev_pm_qos_req_type type;
> -	union {
> -		struct plist_node pnode;
> -		struct pm_qos_flags_request flr;
> -	} data;
> -	struct device *dev;
> -};
> -
>  enum pm_qos_type {
>  	PM_QOS_UNITIALIZED,
>  	PM_QOS_MAX,		/* return the largest value */
>  	PM_QOS_MIN,		/* return the smallest value */
>  	PM_QOS_SUM		/* return the sum */
> @@ -88,10 +73,48 @@ struct pm_qos_constraints {
>  struct pm_qos_flags {
>  	struct list_head list;
>  	s32 effective_flags;	/* Do not change to 64 bit */
>  };
>  
> +
> +#define FREQ_QOS_MIN_DEFAULT_VALUE	0
> +#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
> +
> +enum freq_qos_req_type {
> +	FREQ_QOS_MIN = 1,
> +	FREQ_QOS_MAX,
> +};
> +
> +struct freq_constraints {
> +	struct pm_qos_constraints min_freq;
> +	struct blocking_notifier_head min_freq_notifiers;
> +	struct pm_qos_constraints max_freq;
> +	struct blocking_notifier_head max_freq_notifiers;
> +};
> +
> +struct freq_qos_request {
> +	enum freq_qos_req_type type;
> +	struct plist_node pnode;
> +	struct freq_constraints *qos;
> +};
> +
> +
> +enum dev_pm_qos_req_type {
> +	DEV_PM_QOS_RESUME_LATENCY = 1,
> +	DEV_PM_QOS_LATENCY_TOLERANCE,
> +	DEV_PM_QOS_FLAGS,
> +};
> +
> +struct dev_pm_qos_request {
> +	enum dev_pm_qos_req_type type;
> +	union {
> +		struct plist_node pnode;
> +		struct pm_qos_flags_request flr;
> +	} data;
> +	struct device *dev;
> +};
> +
>  struct dev_pm_qos {
>  	struct pm_qos_constraints resume_latency;
>  	struct pm_qos_constraints latency_tolerance;
>  	struct pm_qos_flags flags;
>  	struct dev_pm_qos_request *resume_latency_req;
> @@ -253,31 +276,10 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>  {
>  	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>  }
>  #endif
>  
> -#define FREQ_QOS_MIN_DEFAULT_VALUE	0
> -#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
> -
> -enum freq_qos_req_type {
> -	FREQ_QOS_MIN = 1,
> -	FREQ_QOS_MAX,
> -};
> -
> -struct freq_constraints {
> -	struct pm_qos_constraints min_freq;
> -	struct blocking_notifier_head min_freq_notifiers;
> -	struct pm_qos_constraints max_freq;
> -	struct blocking_notifier_head max_freq_notifiers;
> -};
> -
> -struct freq_qos_request {
> -	enum freq_qos_req_type type;
> -	struct plist_node pnode;
> -	struct freq_constraints *qos;
> -};
> -
>  static inline int freq_qos_request_active(struct freq_qos_request *req)
>  {
>  	return !IS_ERR_OR_NULL(req->qos);
>  }
>  
>

Leonard Crestez Nov. 1, 2019, 2:45 p.m. UTC | #2

On 01.11.2019 04:07, Chanwoo Choi wrote:
> Hi Leonard,
> 
> Why do you add the new patches on v10 (patch6/7/8)
> in this series? If you think that need to update
> the pm_qos core, you have to send the new patchset
> on separate mail thread. It make the difficult
> to merge the PM_QoS support of devfreq.
> 
> Please split out the patch6/7/8 from this series.

Unfortunately DEV_PM_QOS_MIN/MAX_FREQUENCY was removed when cpufreq 
switched away:

     https://patchwork.kernel.org/cover/11193021/

Support for freq limits in PM QoS needs to be restored, otherwise PM QoS 
for devfreq doesn't even compile. I posted the restoration separately:

     https://patchwork.kernel.org/cover/11212887/

I guess we can wait for Rafael to review that?

We could also consider other alternatives such as using "raw" PM QoS 
aggregation inside devfreq and asking all consumers to call 
devfreq-specific APIs for frequency constraints?

In the meantime I can post the devfreq cleanups in separately. would 
that help?

> On 19. 11. 1. 오전 6:34, Leonard Crestez wrote:
>> This allows dev_pm_qos to embed freq_qos structs, which is done in the
>> next patch. Separate commit to make it easier to review.
>>
>> Signed-off-by: Leonard Crestez <[e-mail]>
>> ---
>>   include/linux/pm_qos.h | 74 ++++++++++++++++++++++--------------------
>>   1 file changed, 38 insertions(+), 36 deletions(-)
>>
>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>> index c35ff21e8a40..a8e1486e3200 100644
>> --- a/include/linux/pm_qos.h
>> +++ b/include/linux/pm_qos.h
>> @@ -47,25 +47,10 @@ struct pm_qos_request {
>>   struct pm_qos_flags_request {
>>   	struct list_head node;
>>   	s32 flags;	/* Do not change to 64 bit */
>>   };
>>   
>> -enum dev_pm_qos_req_type {
>> -	DEV_PM_QOS_RESUME_LATENCY = 1,
>> -	DEV_PM_QOS_LATENCY_TOLERANCE,
>> -	DEV_PM_QOS_FLAGS,
>> -};
>> -
>> -struct dev_pm_qos_request {
>> -	enum dev_pm_qos_req_type type;
>> -	union {
>> -		struct plist_node pnode;
>> -		struct pm_qos_flags_request flr;
>> -	} data;
>> -	struct device *dev;
>> -};
>> -
>>   enum pm_qos_type {
>>   	PM_QOS_UNITIALIZED,
>>   	PM_QOS_MAX,		/* return the largest value */
>>   	PM_QOS_MIN,		/* return the smallest value */
>>   	PM_QOS_SUM		/* return the sum */
>> @@ -88,10 +73,48 @@ struct pm_qos_constraints {
>>   struct pm_qos_flags {
>>   	struct list_head list;
>>   	s32 effective_flags;	/* Do not change to 64 bit */
>>   };
>>   
>> +
>> +#define FREQ_QOS_MIN_DEFAULT_VALUE	0
>> +#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
>> +
>> +enum freq_qos_req_type {
>> +	FREQ_QOS_MIN = 1,
>> +	FREQ_QOS_MAX,
>> +};
>> +
>> +struct freq_constraints {
>> +	struct pm_qos_constraints min_freq;
>> +	struct blocking_notifier_head min_freq_notifiers;
>> +	struct pm_qos_constraints max_freq;
>> +	struct blocking_notifier_head max_freq_notifiers;
>> +};
>> +
>> +struct freq_qos_request {
>> +	enum freq_qos_req_type type;
>> +	struct plist_node pnode;
>> +	struct freq_constraints *qos;
>> +};
>> +
>> +
>> +enum dev_pm_qos_req_type {
>> +	DEV_PM_QOS_RESUME_LATENCY = 1,
>> +	DEV_PM_QOS_LATENCY_TOLERANCE,
>> +	DEV_PM_QOS_FLAGS,
>> +};
>> +
>> +struct dev_pm_qos_request {
>> +	enum dev_pm_qos_req_type type;
>> +	union {
>> +		struct plist_node pnode;
>> +		struct pm_qos_flags_request flr;
>> +	} data;
>> +	struct device *dev;
>> +};
>> +
>>   struct dev_pm_qos {
>>   	struct pm_qos_constraints resume_latency;
>>   	struct pm_qos_constraints latency_tolerance;
>>   	struct pm_qos_flags flags;
>>   	struct dev_pm_qos_request *resume_latency_req;
>> @@ -253,31 +276,10 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>>   {
>>   	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>>   }
>>   #endif
>>   
>> -#define FREQ_QOS_MIN_DEFAULT_VALUE	0
>> -#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
>> -
>> -enum freq_qos_req_type {
>> -	FREQ_QOS_MIN = 1,
>> -	FREQ_QOS_MAX,
>> -};
>> -
>> -struct freq_constraints {
>> -	struct pm_qos_constraints min_freq;
>> -	struct blocking_notifier_head min_freq_notifiers;
>> -	struct pm_qos_constraints max_freq;
>> -	struct blocking_notifier_head max_freq_notifiers;
>> -};
>> -
>> -struct freq_qos_request {
>> -	enum freq_qos_req_type type;
>> -	struct plist_node pnode;
>> -	struct freq_constraints *qos;
>> -};
>> -
>>   static inline int freq_qos_request_active(struct freq_qos_request *req)
>>   {
>>   	return !IS_ERR_OR_NULL(req->qos);
>>   }
>>   
>>
> 
>

Chanwoo Choi Nov. 11, 2019, 5:52 a.m. UTC | #3

Hi Leonard,

On 11/1/19 11:45 PM, Leonard Crestez wrote:
> On 01.11.2019 04:07, Chanwoo Choi wrote:
>> Hi Leonard,
>>
>> Why do you add the new patches on v10 (patch6/7/8)
>> in this series? If you think that need to update
>> the pm_qos core, you have to send the new patchset
>> on separate mail thread. It make the difficult
>> to merge the PM_QoS support of devfreq.
>>
>> Please split out the patch6/7/8 from this series.
> 
> Unfortunately DEV_PM_QOS_MIN/MAX_FREQUENCY was removed when cpufreq 
> switched away:
> 
>      https://patchwork.kernel.org/cover/11193021/
> 
> Support for freq limits in PM QoS needs to be restored, otherwise PM QoS 
> for devfreq doesn't even compile. I posted the restoration separately:
> 
>      https://patchwork.kernel.org/cover/11212887/
> 
> I guess we can wait for Rafael to review that?

I'm sorry for late reply.
Thanks for the explanation.

> 
> We could also consider other alternatives such as using "raw" PM QoS 
> aggregation inside devfreq and asking all consumers to call 
> devfreq-specific APIs for frequency constraints?

Actually, I don't want to make the separate devfreq-specific something
for PM QoS feature. If possible, I think that we need to reduce the redundant
or duplicate code in the linux kernel. Even if spend the long time
for updating or fixing the issue of PM QoS, we need to do them. Thanks.


> 
> In the meantime I can post the devfreq cleanups in separately. would 
> that help?

Yes, you better to send the devfreq cleanup patches separately.

> 
>> On 19. 11. 1. 오전 6:34, Leonard Crestez wrote:
>>> This allows dev_pm_qos to embed freq_qos structs, which is done in the
>>> next patch. Separate commit to make it easier to review.
>>>
>>> Signed-off-by: Leonard Crestez <[e-mail]>
>>> ---
>>>   include/linux/pm_qos.h | 74 ++++++++++++++++++++++--------------------
>>>   1 file changed, 38 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>>> index c35ff21e8a40..a8e1486e3200 100644
>>> --- a/include/linux/pm_qos.h
>>> +++ b/include/linux/pm_qos.h
>>> @@ -47,25 +47,10 @@ struct pm_qos_request {
>>>   struct pm_qos_flags_request {
>>>   	struct list_head node;
>>>   	s32 flags;	/* Do not change to 64 bit */
>>>   };
>>>   
>>> -enum dev_pm_qos_req_type {
>>> -	DEV_PM_QOS_RESUME_LATENCY = 1,
>>> -	DEV_PM_QOS_LATENCY_TOLERANCE,
>>> -	DEV_PM_QOS_FLAGS,
>>> -};
>>> -
>>> -struct dev_pm_qos_request {
>>> -	enum dev_pm_qos_req_type type;
>>> -	union {
>>> -		struct plist_node pnode;
>>> -		struct pm_qos_flags_request flr;
>>> -	} data;
>>> -	struct device *dev;
>>> -};
>>> -
>>>   enum pm_qos_type {
>>>   	PM_QOS_UNITIALIZED,
>>>   	PM_QOS_MAX,		/* return the largest value */
>>>   	PM_QOS_MIN,		/* return the smallest value */
>>>   	PM_QOS_SUM		/* return the sum */
>>> @@ -88,10 +73,48 @@ struct pm_qos_constraints {
>>>   struct pm_qos_flags {
>>>   	struct list_head list;
>>>   	s32 effective_flags;	/* Do not change to 64 bit */
>>>   };
>>>   
>>> +
>>> +#define FREQ_QOS_MIN_DEFAULT_VALUE	0
>>> +#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
>>> +
>>> +enum freq_qos_req_type {
>>> +	FREQ_QOS_MIN = 1,
>>> +	FREQ_QOS_MAX,
>>> +};
>>> +
>>> +struct freq_constraints {
>>> +	struct pm_qos_constraints min_freq;
>>> +	struct blocking_notifier_head min_freq_notifiers;
>>> +	struct pm_qos_constraints max_freq;
>>> +	struct blocking_notifier_head max_freq_notifiers;
>>> +};
>>> +
>>> +struct freq_qos_request {
>>> +	enum freq_qos_req_type type;
>>> +	struct plist_node pnode;
>>> +	struct freq_constraints *qos;
>>> +};
>>> +
>>> +
>>> +enum dev_pm_qos_req_type {
>>> +	DEV_PM_QOS_RESUME_LATENCY = 1,
>>> +	DEV_PM_QOS_LATENCY_TOLERANCE,
>>> +	DEV_PM_QOS_FLAGS,
>>> +};
>>> +
>>> +struct dev_pm_qos_request {
>>> +	enum dev_pm_qos_req_type type;
>>> +	union {
>>> +		struct plist_node pnode;
>>> +		struct pm_qos_flags_request flr;
>>> +	} data;
>>> +	struct device *dev;
>>> +};
>>> +
>>>   struct dev_pm_qos {
>>>   	struct pm_qos_constraints resume_latency;
>>>   	struct pm_qos_constraints latency_tolerance;
>>>   	struct pm_qos_flags flags;
>>>   	struct dev_pm_qos_request *resume_latency_req;
>>> @@ -253,31 +276,10 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>>>   {
>>>   	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>>>   }
>>>   #endif
>>>   
>>> -#define FREQ_QOS_MIN_DEFAULT_VALUE	0
>>> -#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
>>> -
>>> -enum freq_qos_req_type {
>>> -	FREQ_QOS_MIN = 1,
>>> -	FREQ_QOS_MAX,
>>> -};
>>> -
>>> -struct freq_constraints {
>>> -	struct pm_qos_constraints min_freq;
>>> -	struct blocking_notifier_head min_freq_notifiers;
>>> -	struct pm_qos_constraints max_freq;
>>> -	struct blocking_notifier_head max_freq_notifiers;
>>> -};
>>> -
>>> -struct freq_qos_request {
>>> -	enum freq_qos_req_type type;
>>> -	struct plist_node pnode;
>>> -	struct freq_constraints *qos;
>>> -};
>>> -
>>>   static inline int freq_qos_request_active(struct freq_qos_request *req)
>>>   {
>>>   	return !IS_ERR_OR_NULL(req->qos);
>>>   }
>>>   
>>>
>>
>>
>

Patch

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index c35ff21e8a40..a8e1486e3200 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -47,25 +47,10 @@  struct pm_qos_request {
 struct pm_qos_flags_request {
 	struct list_head node;
 	s32 flags;	/* Do not change to 64 bit */
 };
 
-enum dev_pm_qos_req_type {
-	DEV_PM_QOS_RESUME_LATENCY = 1,
-	DEV_PM_QOS_LATENCY_TOLERANCE,
-	DEV_PM_QOS_FLAGS,
-};
-
-struct dev_pm_qos_request {
-	enum dev_pm_qos_req_type type;
-	union {
-		struct plist_node pnode;
-		struct pm_qos_flags_request flr;
-	} data;
-	struct device *dev;
-};
-
 enum pm_qos_type {
 	PM_QOS_UNITIALIZED,
 	PM_QOS_MAX,		/* return the largest value */
 	PM_QOS_MIN,		/* return the smallest value */
 	PM_QOS_SUM		/* return the sum */
@@ -88,10 +73,48 @@  struct pm_qos_constraints {
 struct pm_qos_flags {
 	struct list_head list;
 	s32 effective_flags;	/* Do not change to 64 bit */
 };
 
+
+#define FREQ_QOS_MIN_DEFAULT_VALUE	0
+#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
+
+enum freq_qos_req_type {
+	FREQ_QOS_MIN = 1,
+	FREQ_QOS_MAX,
+};
+
+struct freq_constraints {
+	struct pm_qos_constraints min_freq;
+	struct blocking_notifier_head min_freq_notifiers;
+	struct pm_qos_constraints max_freq;
+	struct blocking_notifier_head max_freq_notifiers;
+};
+
+struct freq_qos_request {
+	enum freq_qos_req_type type;
+	struct plist_node pnode;
+	struct freq_constraints *qos;
+};
+
+
+enum dev_pm_qos_req_type {
+	DEV_PM_QOS_RESUME_LATENCY = 1,
+	DEV_PM_QOS_LATENCY_TOLERANCE,
+	DEV_PM_QOS_FLAGS,
+};
+
+struct dev_pm_qos_request {
+	enum dev_pm_qos_req_type type;
+	union {
+		struct plist_node pnode;
+		struct pm_qos_flags_request flr;
+	} data;
+	struct device *dev;
+};
+
 struct dev_pm_qos {
 	struct pm_qos_constraints resume_latency;
 	struct pm_qos_constraints latency_tolerance;
 	struct pm_qos_flags flags;
 	struct dev_pm_qos_request *resume_latency_req;
@@ -253,31 +276,10 @@  static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
 {
 	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
 }
 #endif
 
-#define FREQ_QOS_MIN_DEFAULT_VALUE	0
-#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
-
-enum freq_qos_req_type {
-	FREQ_QOS_MIN = 1,
-	FREQ_QOS_MAX,
-};
-
-struct freq_constraints {
-	struct pm_qos_constraints min_freq;
-	struct blocking_notifier_head min_freq_notifiers;
-	struct pm_qos_constraints max_freq;
-	struct blocking_notifier_head max_freq_notifiers;
-};
-
-struct freq_qos_request {
-	enum freq_qos_req_type type;
-	struct plist_node pnode;
-	struct freq_constraints *qos;
-};
-
 static inline int freq_qos_request_active(struct freq_qos_request *req)
 {
 	return !IS_ERR_OR_NULL(req->qos);
 }