linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
@ 2020-05-26 15:16 Benjamin Gaignard
  2020-05-26 15:16 ` [RFC 1/3] PM: QoS: " Benjamin Gaignard
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2020-05-26 15:16 UTC (permalink / raw)
  To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
	alexandre.torgue, pavel, len.brown, valentin.schneider,
	vincent.guittot
  Cc: linux-pm, linux-kernel, linux-media, linux-stm32,
	linux-arm-kernel, Benjamin Gaignard

A first round [1] of discussions and suggestions have already be done on 
this series but without found a solution to the problem. I resend it to
progress on this topic.

When start streaming from the sensor the CPU load could remain very low 
because almost all the capture pipeline is done in hardware (i.e. without 
using the CPU) and let believe to cpufreq governor that it could use lower 
frequencies. If the governor decides to use a too low frequency that 
becomes a problem when we need to acknowledge the interrupt during the 
blanking time.
The delay to ack the interrupt and perform all the other actions before
the next frame is very short and doesn't allow to the cpufreq governor to
provide the required burst of power. That led to drop the half of the frames.

To avoid this problem, DCMI driver informs the cpufreq governors by adding
a cpufreq minimum load QoS resquest.

Benjamin 

[1] https://lkml.org/lkml/2020/4/24/360

Benjamin Gaignard (3):
  PM: QoS: Introduce cpufreq minimum load QoS
  cpufreq: governor: Use minimum load QoS
  media: stm32-dcmi: Inform cpufreq governors about cpu load needs

 drivers/cpufreq/cpufreq_governor.c        |   5 +
 drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
 include/linux/pm_qos.h                    |  12 ++
 kernel/power/qos.c                        | 213 ++++++++++++++++++++++++++++++
 4 files changed, 238 insertions(+)

-- 
2.15.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC 1/3] PM: QoS: Introduce cpufreq minimum load QoS
  2020-05-26 15:16 [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Benjamin Gaignard
@ 2020-05-26 15:16 ` Benjamin Gaignard
  2020-05-26 15:16 ` [RFC 2/3] cpufreq: governor: Use " Benjamin Gaignard
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2020-05-26 15:16 UTC (permalink / raw)
  To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
	alexandre.torgue, pavel, len.brown, valentin.schneider,
	vincent.guittot
  Cc: linux-pm, linux-kernel, linux-media, linux-stm32,
	linux-arm-kernel, Benjamin Gaignard

Introduce cpufreq minimum load QoS, based on the "raw" low-level
PM QoS, to represent the minimum expected cpu load by various devices.

The cpufreq_minload_qos_limit() helper is defined to retrieve the
aggregated constraints.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 include/linux/pm_qos.h |  12 +++
 kernel/power/qos.c     | 213 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 225 insertions(+)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 4a69d4af3ff8..e2cc099322e3 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -316,4 +316,16 @@ int freq_qos_remove_notifier(struct freq_constraints *qos,
 			     enum freq_qos_req_type type,
 			     struct notifier_block *notifier);
 
+/* Definitions related to the cpufreq minimum load QoS. */
+
+#define CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE	0
+#define CPUFREQ_GOV_QOS_MIN_LOAD_MAX_VALUE	10
+
+s32 cpufreq_minload_qos_limit(void);
+bool cpufreq_minload_qos_request_active(struct pm_qos_request *req);
+void cpufreq_minload_qos_add_request(struct pm_qos_request *req, s32 value);
+void cpufreq_minload_qos_update_request(struct pm_qos_request *req,
+					s32 new_value);
+void cpufreq_minload_qos_remove_request(struct pm_qos_request *req);
+
 #endif
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index db0bed2cae26..df2fdd962f35 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -671,3 +671,216 @@ int freq_qos_remove_notifier(struct freq_constraints *qos,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(freq_qos_remove_notifier);
+
+/* Definitions related to the cpufreq minimum load QoS. */
+
+static struct pm_qos_constraints cpufreq_minload_constraints = {
+	.list = PLIST_HEAD_INIT(cpufreq_minload_constraints.list),
+	.target_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+	.default_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+	.no_constraint_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+	.type = PM_QOS_MAX,
+};
+
+/**
+ * cpufreq_minload_qos_limit - Return current system-wide cpufreq
+ * minimum load QoS limit.
+ */
+s32 cpufreq_minload_qos_limit(void)
+{
+	return pm_qos_read_value(&cpufreq_minload_constraints);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_limit);
+
+/**
+ * cpufreq_minload_qos_request_active - Check the given PM QoS request.
+ * @req: PM QoS request to check.
+ *
+ * Return: 'true' if @req has been added to the cpufreq minimum load
+ * QoS list, 'false' otherwise.
+ */
+bool cpufreq_minload_qos_request_active(struct pm_qos_request *req)
+{
+	return req->qos == &cpufreq_minload_constraints;
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_request_active);
+
+/**
+ * cpufreq_minload_qos_add_request - Add new cpufreq minimum load QoS request.
+ * @req: Pointer to a preallocated handle.
+ * @value: Requested constraint value.
+ *
+ * Use @value to initialize the request handle pointed to by @req, insert it as
+ * a new entry to the cpufreq minimum load QoS list and recompute the effective
+ * QoS constraint for that list.
+ *
+ * Callers need to save the handle for later use in updates and removal of the
+ * QoS request represented by it.
+ */
+void cpufreq_minload_qos_add_request(struct pm_qos_request *req, s32 value)
+{
+	if (!req)
+		return;
+
+	if (cpufreq_minload_qos_request_active(req)) {
+		WARN(1, KERN_ERR "%s called for already added request\n", __func__);
+		return;
+	}
+
+	trace_pm_qos_add_request(value);
+
+	req->qos = &cpufreq_minload_constraints;
+	pm_qos_update_target(req->qos, &req->node, PM_QOS_ADD_REQ, value);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_add_request);
+
+/**
+ * cpufreq_minload_qos_update_request - Modify existing cpufreq minimum load
+ * QoS request.
+ * @req : QoS request to update.
+ * @new_value: New requested constraint value.
+ *
+ * Use @new_value to update the QoS request represented by @req in the cpufreq
+ * minimum load QoS list along with updating the effective constraint value for
+ * that list.
+ */
+void cpufreq_minload_qos_update_request(struct pm_qos_request *req,
+					s32 new_value)
+{
+	if (!req)
+		return;
+
+	if (!cpufreq_minload_qos_request_active(req)) {
+		WARN(1, KERN_ERR "%s called for unknown object\n", __func__);
+		return;
+	}
+
+	trace_pm_qos_update_request(new_value);
+
+	if (new_value == req->node.prio)
+		return;
+
+	pm_qos_update_target(req->qos, &req->node, PM_QOS_UPDATE_REQ, new_value);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_update_request);
+
+/**
+ * cpufreq_minload_qos_remove_request - Remove existing cpufreq minimum load QoS
+ * request.
+ * @req: QoS request to remove.
+ *
+ * Remove the cpufreq minimum load QoS request represented by @req from the
+ * cpufreq minimum load QoS list along with updating the effective constraint
+ * value for that list.
+ */
+void cpufreq_minload_qos_remove_request(struct pm_qos_request *req)
+{
+	if (!req)
+		return;
+
+	if (!cpufreq_minload_qos_request_active(req)) {
+		WARN(1, KERN_ERR "%s called for unknown object\n", __func__);
+		return;
+	}
+
+	trace_pm_qos_remove_request(PM_QOS_DEFAULT_VALUE);
+
+	pm_qos_update_target(req->qos, &req->node,
+			     PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
+	memset(req, 0, sizeof(*req));
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_remove_request);
+
+/* User space interface to the cpufreq minimum load QoS via misc device. */
+
+static int cpufreq_minload_qos_open(struct inode *inode, struct file *filp)
+{
+	struct pm_qos_request *req;
+
+	req = kzalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	cpufreq_minload_qos_add_request(req, PM_QOS_DEFAULT_VALUE);
+	filp->private_data = req;
+
+	return 0;
+}
+
+static int cpufreq_minload_qos_release(struct inode *inode, struct file *filp)
+{
+	struct pm_qos_request *req = filp->private_data;
+
+	filp->private_data = NULL;
+
+	cpufreq_minload_qos_remove_request(req);
+	kfree(req);
+
+	return 0;
+}
+
+static ssize_t cpufreq_minload_qos_read(struct file *filp, char __user *buf,
+					size_t count, loff_t *f_pos)
+{
+	struct pm_qos_request *req = filp->private_data;
+	unsigned long flags;
+	s32 value;
+
+	if (!req || !cpufreq_minload_qos_request_active(req))
+		return -EINVAL;
+
+	spin_lock_irqsave(&pm_qos_lock, flags);
+	value = pm_qos_get_value(&cpufreq_minload_constraints);
+	spin_unlock_irqrestore(&pm_qos_lock, flags);
+
+	return simple_read_from_buffer(buf, count, f_pos, &value, sizeof(s32));
+}
+
+static ssize_t cpufreq_minload_qos_write(struct file *filp,
+					 const char __user *buf,
+					 size_t count, loff_t *f_pos)
+{
+	s32 value;
+
+	if (count == sizeof(s32)) {
+		if (copy_from_user(&value, buf, sizeof(s32)))
+			return -EFAULT;
+	} else {
+		int ret;
+
+		ret = kstrtos32_from_user(buf, count, 16, &value);
+		if (ret)
+			return ret;
+	}
+
+	cpufreq_minload_qos_update_request(filp->private_data, value);
+
+	return count;
+}
+
+static const struct file_operations cpufreq_minload_qos_fops = {
+	.write = cpufreq_minload_qos_write,
+	.read = cpufreq_minload_qos_read,
+	.open = cpufreq_minload_qos_open,
+	.release = cpufreq_minload_qos_release,
+	.llseek = noop_llseek,
+};
+
+static struct miscdevice cpufreq_minload_qos_miscdev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "cpufreq_minimum_load",
+	.fops = &cpufreq_minload_qos_fops,
+};
+
+static int __init cpufreq_minload_qos_init(void)
+{
+	int ret;
+
+	ret = misc_register(&cpufreq_minload_qos_miscdev);
+	if (ret < 0)
+		pr_err("%s: %s setup failed\n", __func__,
+		       cpufreq_minload_qos_miscdev.name);
+
+	return ret;
+}
+late_initcall(cpufreq_minload_qos_init);
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC 2/3] cpufreq: governor: Use minimum load QoS
  2020-05-26 15:16 [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Benjamin Gaignard
  2020-05-26 15:16 ` [RFC 1/3] PM: QoS: " Benjamin Gaignard
@ 2020-05-26 15:16 ` Benjamin Gaignard
  2020-05-26 15:16 ` [RFC 3/3] media: stm32-dcmi: Inform cpufreq governors about cpu load needs Benjamin Gaignard
  2020-05-27 10:09 ` [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Valentin Schneider
  3 siblings, 0 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2020-05-26 15:16 UTC (permalink / raw)
  To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
	alexandre.torgue, pavel, len.brown, valentin.schneider,
	vincent.guittot
  Cc: linux-pm, linux-kernel, linux-media, linux-stm32,
	linux-arm-kernel, Benjamin Gaignard

Make sure that the returned load is above the system-wide minimum
load QoS.
Devices could set this specific QoS to inform governors about their
need in terms of CPU load when computing it from idle time isn't accurate.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 drivers/cpufreq/cpufreq_governor.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index f99ae45efaea..1494e5e4c788 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -118,6 +118,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
 	unsigned int ignore_nice = dbs_data->ignore_nice_load;
 	unsigned int max_load = 0, idle_periods = UINT_MAX;
 	unsigned int sampling_rate, io_busy, j;
+	unsigned int qos_min_load;
 
 	/*
 	 * Sometimes governors may use an additional multiplier to increase
@@ -225,6 +226,10 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
 
 	policy_dbs->idle_periods = idle_periods;
 
+	qos_min_load = cpufreq_minload_qos_limit();
+	if (qos_min_load > max_load)
+		max_load = qos_min_load;
+
 	return max_load;
 }
 EXPORT_SYMBOL_GPL(dbs_update);
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC 3/3] media: stm32-dcmi: Inform cpufreq governors about cpu load needs
  2020-05-26 15:16 [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Benjamin Gaignard
  2020-05-26 15:16 ` [RFC 1/3] PM: QoS: " Benjamin Gaignard
  2020-05-26 15:16 ` [RFC 2/3] cpufreq: governor: Use " Benjamin Gaignard
@ 2020-05-26 15:16 ` Benjamin Gaignard
  2020-05-27 10:09 ` [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Valentin Schneider
  3 siblings, 0 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2020-05-26 15:16 UTC (permalink / raw)
  To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
	alexandre.torgue, pavel, len.brown, valentin.schneider,
	vincent.guittot
  Cc: linux-pm, linux-kernel, linux-media, linux-stm32,
	linux-arm-kernel, Benjamin Gaignard

When start streaming the CPU load could remain very low because almost
all the capture pipeline is done in hardware (i.e. without using the CPU)
and let believe to cpufreq governor that it could use lower frequencies.
If the governor decides to use a too low frequency that becomes a problem
when we need to acknowledge the interrupt during the blanking time.

To avoid this problem, DCMI driver informs the cpufreq governors by adding
a cpufreq minimum load QoS resquest.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 drivers/media/platform/stm32/stm32-dcmi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b8931490b83b..774f2506b2f1 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -24,6 +24,7 @@
 #include <linux/of_graph.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/pm_qos.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/videodev2.h>
@@ -173,6 +174,8 @@ struct stm32_dcmi {
 	struct media_device		mdev;
 	struct media_pad		vid_cap_pad;
 	struct media_pipeline		pipeline;
+
+	struct pm_qos_request		qos_request;
 };
 
 static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
@@ -827,6 +830,9 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 	else
 		reg_set(dcmi->regs, DCMI_IER, IT_OVR | IT_ERR);
 
+	cpufreq_minload_qos_add_request(&dcmi->qos_request,
+					CPUFREQ_GOV_QOS_MIN_LOAD_MAX_VALUE);
+
 	return 0;
 
 err_pipeline_stop:
@@ -859,6 +865,8 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
 	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
 	struct dcmi_buf *buf, *node;
 
+	cpufreq_minload_qos_remove_request(&dcmi->qos_request);
+
 	dcmi_pipeline_stop(dcmi);
 
 	media_pipeline_stop(&dcmi->vdev->entity);
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-26 15:16 [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Benjamin Gaignard
                   ` (2 preceding siblings ...)
  2020-05-26 15:16 ` [RFC 3/3] media: stm32-dcmi: Inform cpufreq governors about cpu load needs Benjamin Gaignard
@ 2020-05-27 10:09 ` Valentin Schneider
  2020-05-27 11:17   ` Benjamin GAIGNARD
  3 siblings, 1 reply; 14+ messages in thread
From: Valentin Schneider @ 2020-05-27 10:09 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
	alexandre.torgue, pavel, len.brown, vincent.guittot, linux-pm,
	linux-kernel, linux-media, linux-stm32, linux-arm-kernel


Hi Benjamin,

On 26/05/20 16:16, Benjamin Gaignard wrote:
> A first round [1] of discussions and suggestions have already be done on
> this series but without found a solution to the problem. I resend it to
> progress on this topic.
>

Apologies for sleeping on that previous thread.

So what had been suggested over there was to use uclamp to boost the
frequency of the handling thread; however if you use threaded IRQs you
get RT threads, which already get the max frequency by default (at least
with schedutil).

Does that not work for you, and if so, why?

> When start streaming from the sensor the CPU load could remain very low
> because almost all the capture pipeline is done in hardware (i.e. without
> using the CPU) and let believe to cpufreq governor that it could use lower
> frequencies. If the governor decides to use a too low frequency that
> becomes a problem when we need to acknowledge the interrupt during the
> blanking time.
> The delay to ack the interrupt and perform all the other actions before
> the next frame is very short and doesn't allow to the cpufreq governor to
> provide the required burst of power. That led to drop the half of the frames.
>
> To avoid this problem, DCMI driver informs the cpufreq governors by adding
> a cpufreq minimum load QoS resquest.
>
> Benjamin
>
> [1] https://lkml.org/lkml/2020/4/24/360
>
> Benjamin Gaignard (3):
>   PM: QoS: Introduce cpufreq minimum load QoS
>   cpufreq: governor: Use minimum load QoS
>   media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>
>  drivers/cpufreq/cpufreq_governor.c        |   5 +
>  drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
>  include/linux/pm_qos.h                    |  12 ++
>  kernel/power/qos.c                        | 213 ++++++++++++++++++++++++++++++
>  4 files changed, 238 insertions(+)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-27 10:09 ` [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Valentin Schneider
@ 2020-05-27 11:17   ` Benjamin GAIGNARD
  2020-05-27 12:14     ` Valentin Schneider
  2020-05-27 12:22     ` Vincent Guittot
  0 siblings, 2 replies; 14+ messages in thread
From: Benjamin GAIGNARD @ 2020-05-27 11:17 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: rjw, viresh.kumar, Hugues FRUCHET, mchehab, mcoquelin.stm32,
	Alexandre TORGUE, pavel, len.brown, vincent.guittot, linux-pm,
	linux-kernel, linux-media, linux-stm32, linux-arm-kernel



On 5/27/20 12:09 PM, Valentin Schneider wrote:
> Hi Benjamin,
>
> On 26/05/20 16:16, Benjamin Gaignard wrote:
>> A first round [1] of discussions and suggestions have already be done on
>> this series but without found a solution to the problem. I resend it to
>> progress on this topic.
>>
> Apologies for sleeping on that previous thread.
>
> So what had been suggested over there was to use uclamp to boost the
> frequency of the handling thread; however if you use threaded IRQs you
> get RT threads, which already get the max frequency by default (at least
> with schedutil).
>
> Does that not work for you, and if so, why?
That doesn't work because almost everything is done by the hardware blocks
without charge the CPU so the thread isn't running. I have done the 
tests with schedutil
and ondemand scheduler (which is the one I'm targeting). I have no 
issues when using
performance scheduler because it always keep the highest frequencies.


>
>> When start streaming from the sensor the CPU load could remain very low
>> because almost all the capture pipeline is done in hardware (i.e. without
>> using the CPU) and let believe to cpufreq governor that it could use lower
>> frequencies. If the governor decides to use a too low frequency that
>> becomes a problem when we need to acknowledge the interrupt during the
>> blanking time.
>> The delay to ack the interrupt and perform all the other actions before
>> the next frame is very short and doesn't allow to the cpufreq governor to
>> provide the required burst of power. That led to drop the half of the frames.
>>
>> To avoid this problem, DCMI driver informs the cpufreq governors by adding
>> a cpufreq minimum load QoS resquest.
>>
>> Benjamin
>>
>> [1] https://lkml.org/lkml/2020/4/24/360
>>
>> Benjamin Gaignard (3):
>>    PM: QoS: Introduce cpufreq minimum load QoS
>>    cpufreq: governor: Use minimum load QoS
>>    media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>
>>   drivers/cpufreq/cpufreq_governor.c        |   5 +
>>   drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
>>   include/linux/pm_qos.h                    |  12 ++
>>   kernel/power/qos.c                        | 213 ++++++++++++++++++++++++++++++
>>   4 files changed, 238 insertions(+)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-27 11:17   ` Benjamin GAIGNARD
@ 2020-05-27 12:14     ` Valentin Schneider
  2020-05-27 13:11       ` Benjamin GAIGNARD
  2020-05-27 12:22     ` Vincent Guittot
  1 sibling, 1 reply; 14+ messages in thread
From: Valentin Schneider @ 2020-05-27 12:14 UTC (permalink / raw)
  To: Benjamin GAIGNARD
  Cc: rjw, viresh.kumar, Hugues FRUCHET, mchehab, mcoquelin.stm32,
	Alexandre TORGUE, pavel, len.brown, vincent.guittot, linux-pm,
	linux-kernel, linux-media, linux-stm32, linux-arm-kernel


On 27/05/20 12:17, Benjamin GAIGNARD wrote:
> On 5/27/20 12:09 PM, Valentin Schneider wrote:
>> Hi Benjamin,
>>
>> On 26/05/20 16:16, Benjamin Gaignard wrote:
>>> A first round [1] of discussions and suggestions have already be done on
>>> this series but without found a solution to the problem. I resend it to
>>> progress on this topic.
>>>
>> Apologies for sleeping on that previous thread.
>>
>> So what had been suggested over there was to use uclamp to boost the
>> frequency of the handling thread; however if you use threaded IRQs you
>> get RT threads, which already get the max frequency by default (at least
>> with schedutil).
>>
>> Does that not work for you, and if so, why?
>
> That doesn't work because almost everything is done by the hardware blocks
> without charge the CPU so the thread isn't running.

I'm not sure I follow; the frequency of the CPU doesn't matter while
your hardware blocks are spinning, right? AIUI what matters is running
your interrupt handler / action at max freq, which you get if you use
threaded IRQs and schedutil.

I think it would help if you could clarify which tasks / parts of your
pipeline you need running at high frequencies. The point is that setting
a QoS request affects all tasks, whereas we could be smarter and only
boost the required tasks.

> I have done the
> tests with schedutil
> and ondemand scheduler (which is the one I'm targeting). I have no
> issues when using
> performance scheduler because it always keep the highest frequencies.
>
>
>>
>>> When start streaming from the sensor the CPU load could remain very low
>>> because almost all the capture pipeline is done in hardware (i.e. without
>>> using the CPU) and let believe to cpufreq governor that it could use lower
>>> frequencies. If the governor decides to use a too low frequency that
>>> becomes a problem when we need to acknowledge the interrupt during the
>>> blanking time.
>>> The delay to ack the interrupt and perform all the other actions before
>>> the next frame is very short and doesn't allow to the cpufreq governor to
>>> provide the required burst of power. That led to drop the half of the frames.
>>>
>>> To avoid this problem, DCMI driver informs the cpufreq governors by adding
>>> a cpufreq minimum load QoS resquest.
>>>
>>> Benjamin
>>>
>>> [1] https://lkml.org/lkml/2020/4/24/360
>>>
>>> Benjamin Gaignard (3):
>>>    PM: QoS: Introduce cpufreq minimum load QoS
>>>    cpufreq: governor: Use minimum load QoS
>>>    media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>>
>>>   drivers/cpufreq/cpufreq_governor.c        |   5 +
>>>   drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
>>>   include/linux/pm_qos.h                    |  12 ++
>>>   kernel/power/qos.c                        | 213 ++++++++++++++++++++++++++++++
>>>   4 files changed, 238 insertions(+)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-27 11:17   ` Benjamin GAIGNARD
  2020-05-27 12:14     ` Valentin Schneider
@ 2020-05-27 12:22     ` Vincent Guittot
  2020-05-27 12:48       ` Benjamin GAIGNARD
  1 sibling, 1 reply; 14+ messages in thread
From: Vincent Guittot @ 2020-05-27 12:22 UTC (permalink / raw)
  To: Benjamin GAIGNARD
  Cc: Valentin Schneider, rjw, viresh.kumar, Hugues FRUCHET, mchehab,
	mcoquelin.stm32, Alexandre TORGUE, pavel, len.brown, linux-pm,
	linux-kernel, linux-media, linux-stm32, linux-arm-kernel

On Wed, 27 May 2020 at 13:17, Benjamin GAIGNARD
<benjamin.gaignard@st.com> wrote:
>
>
>
> On 5/27/20 12:09 PM, Valentin Schneider wrote:
> > Hi Benjamin,
> >
> > On 26/05/20 16:16, Benjamin Gaignard wrote:
> >> A first round [1] of discussions and suggestions have already be done on
> >> this series but without found a solution to the problem. I resend it to
> >> progress on this topic.
> >>
> > Apologies for sleeping on that previous thread.
> >
> > So what had been suggested over there was to use uclamp to boost the
> > frequency of the handling thread; however if you use threaded IRQs you
> > get RT threads, which already get the max frequency by default (at least
> > with schedutil).
> >
> > Does that not work for you, and if so, why?
> That doesn't work because almost everything is done by the hardware blocks
> without charge the CPU so the thread isn't running. I have done the
> tests with schedutil
> and ondemand scheduler (which is the one I'm targeting). I have no
> issues when using
> performance scheduler because it always keep the highest frequencies.

IMHO, the only way to ensure a min frequency for anything else than a
thread is to use freq_qos_add_request() just like cpufreq cooling
device but for the opposite QoS. This can be applied only on the
frequency domain of the CPU which handles the interrupt.
Have you also checked the wakeup latency of your idle state ?

>
>
> >
> >> When start streaming from the sensor the CPU load could remain very low
> >> because almost all the capture pipeline is done in hardware (i.e. without
> >> using the CPU) and let believe to cpufreq governor that it could use lower
> >> frequencies. If the governor decides to use a too low frequency that
> >> becomes a problem when we need to acknowledge the interrupt during the
> >> blanking time.
> >> The delay to ack the interrupt and perform all the other actions before
> >> the next frame is very short and doesn't allow to the cpufreq governor to
> >> provide the required burst of power. That led to drop the half of the frames.
> >>
> >> To avoid this problem, DCMI driver informs the cpufreq governors by adding
> >> a cpufreq minimum load QoS resquest.
> >>
> >> Benjamin
> >>
> >> [1] https://lkml.org/lkml/2020/4/24/360
> >>
> >> Benjamin Gaignard (3):
> >>    PM: QoS: Introduce cpufreq minimum load QoS
> >>    cpufreq: governor: Use minimum load QoS
> >>    media: stm32-dcmi: Inform cpufreq governors about cpu load needs
> >>
> >>   drivers/cpufreq/cpufreq_governor.c        |   5 +
> >>   drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
> >>   include/linux/pm_qos.h                    |  12 ++
> >>   kernel/power/qos.c                        | 213 ++++++++++++++++++++++++++++++
> >>   4 files changed, 238 insertions(+)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-27 12:22     ` Vincent Guittot
@ 2020-05-27 12:48       ` Benjamin GAIGNARD
  2020-05-27 14:54         ` Benjamin GAIGNARD
  0 siblings, 1 reply; 14+ messages in thread
From: Benjamin GAIGNARD @ 2020-05-27 12:48 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: Valentin Schneider, rjw, viresh.kumar, Hugues FRUCHET, mchehab,
	mcoquelin.stm32, Alexandre TORGUE, pavel, len.brown, linux-pm,
	linux-kernel, linux-media, linux-stm32, linux-arm-kernel



On 5/27/20 2:22 PM, Vincent Guittot wrote:
> On Wed, 27 May 2020 at 13:17, Benjamin GAIGNARD
> <benjamin.gaignard@st.com> wrote:
>>
>>
>> On 5/27/20 12:09 PM, Valentin Schneider wrote:
>>> Hi Benjamin,
>>>
>>> On 26/05/20 16:16, Benjamin Gaignard wrote:
>>>> A first round [1] of discussions and suggestions have already be done on
>>>> this series but without found a solution to the problem. I resend it to
>>>> progress on this topic.
>>>>
>>> Apologies for sleeping on that previous thread.
>>>
>>> So what had been suggested over there was to use uclamp to boost the
>>> frequency of the handling thread; however if you use threaded IRQs you
>>> get RT threads, which already get the max frequency by default (at least
>>> with schedutil).
>>>
>>> Does that not work for you, and if so, why?
>> That doesn't work because almost everything is done by the hardware blocks
>> without charge the CPU so the thread isn't running. I have done the
>> tests with schedutil
>> and ondemand scheduler (which is the one I'm targeting). I have no
>> issues when using
>> performance scheduler because it always keep the highest frequencies.
> IMHO, the only way to ensure a min frequency for anything else than a
> thread is to use freq_qos_add_request() just like cpufreq cooling
> device but for the opposite QoS. This can be applied only on the
> frequency domain of the CPU which handles the interrupt.
I will give a try with this idea.
Thanks.
> Have you also checked the wakeup latency of your idle state ?
It just could go in WFI so latency should be minimal.
>
>>
>>>> When start streaming from the sensor the CPU load could remain very low
>>>> because almost all the capture pipeline is done in hardware (i.e. without
>>>> using the CPU) and let believe to cpufreq governor that it could use lower
>>>> frequencies. If the governor decides to use a too low frequency that
>>>> becomes a problem when we need to acknowledge the interrupt during the
>>>> blanking time.
>>>> The delay to ack the interrupt and perform all the other actions before
>>>> the next frame is very short and doesn't allow to the cpufreq governor to
>>>> provide the required burst of power. That led to drop the half of the frames.
>>>>
>>>> To avoid this problem, DCMI driver informs the cpufreq governors by adding
>>>> a cpufreq minimum load QoS resquest.
>>>>
>>>> Benjamin
>>>>
>>>> [1] https://lkml.org/lkml/2020/4/24/360
>>>>
>>>> Benjamin Gaignard (3):
>>>>     PM: QoS: Introduce cpufreq minimum load QoS
>>>>     cpufreq: governor: Use minimum load QoS
>>>>     media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>>>
>>>>    drivers/cpufreq/cpufreq_governor.c        |   5 +
>>>>    drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
>>>>    include/linux/pm_qos.h                    |  12 ++
>>>>    kernel/power/qos.c                        | 213 ++++++++++++++++++++++++++++++
>>>>    4 files changed, 238 insertions(+)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-27 12:14     ` Valentin Schneider
@ 2020-05-27 13:11       ` Benjamin GAIGNARD
  2020-05-27 15:02         ` Valentin Schneider
  0 siblings, 1 reply; 14+ messages in thread
From: Benjamin GAIGNARD @ 2020-05-27 13:11 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: rjw, viresh.kumar, Hugues FRUCHET, mchehab, mcoquelin.stm32,
	Alexandre TORGUE, pavel, len.brown, vincent.guittot, linux-pm,
	linux-kernel, linux-media, linux-stm32, linux-arm-kernel



On 5/27/20 2:14 PM, Valentin Schneider wrote:
> On 27/05/20 12:17, Benjamin GAIGNARD wrote:
>> On 5/27/20 12:09 PM, Valentin Schneider wrote:
>>> Hi Benjamin,
>>>
>>> On 26/05/20 16:16, Benjamin Gaignard wrote:
>>>> A first round [1] of discussions and suggestions have already be done on
>>>> this series but without found a solution to the problem. I resend it to
>>>> progress on this topic.
>>>>
>>> Apologies for sleeping on that previous thread.
>>>
>>> So what had been suggested over there was to use uclamp to boost the
>>> frequency of the handling thread; however if you use threaded IRQs you
>>> get RT threads, which already get the max frequency by default (at least
>>> with schedutil).
>>>
>>> Does that not work for you, and if so, why?
>> That doesn't work because almost everything is done by the hardware blocks
>> without charge the CPU so the thread isn't running.
> I'm not sure I follow; the frequency of the CPU doesn't matter while
> your hardware blocks are spinning, right? AIUI what matters is running
> your interrupt handler / action at max freq, which you get if you use
> threaded IRQs and schedutil.
Yes but not limited to schedutil.
Given the latency needed to change of frequencies I think it could 
already too late
to change the CPU frequency when handling the threaded interrupt.
>
> I think it would help if you could clarify which tasks / parts of your
> pipeline you need running at high frequencies. The point is that setting
> a QoS request affects all tasks, whereas we could be smarter and only
> boost the required tasks.
What make us drop frames is that the threaded IRQ is scheduled too late.
The not thread part of the interrupt handler where we clear the 
interrupt flags
is going fine but the thread part not.
>
>> I have done the
>> tests with schedutil
>> and ondemand scheduler (which is the one I'm targeting). I have no
>> issues when using
>> performance scheduler because it always keep the highest frequencies.
>>
>>
>>>> When start streaming from the sensor the CPU load could remain very low
>>>> because almost all the capture pipeline is done in hardware (i.e. without
>>>> using the CPU) and let believe to cpufreq governor that it could use lower
>>>> frequencies. If the governor decides to use a too low frequency that
>>>> becomes a problem when we need to acknowledge the interrupt during the
>>>> blanking time.
>>>> The delay to ack the interrupt and perform all the other actions before
>>>> the next frame is very short and doesn't allow to the cpufreq governor to
>>>> provide the required burst of power. That led to drop the half of the frames.
>>>>
>>>> To avoid this problem, DCMI driver informs the cpufreq governors by adding
>>>> a cpufreq minimum load QoS resquest.
>>>>
>>>> Benjamin
>>>>
>>>> [1] https://lkml.org/lkml/2020/4/24/360
>>>>
>>>> Benjamin Gaignard (3):
>>>>     PM: QoS: Introduce cpufreq minimum load QoS
>>>>     cpufreq: governor: Use minimum load QoS
>>>>     media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>>>
>>>>    drivers/cpufreq/cpufreq_governor.c        |   5 +
>>>>    drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
>>>>    include/linux/pm_qos.h                    |  12 ++
>>>>    kernel/power/qos.c                        | 213 ++++++++++++++++++++++++++++++
>>>>    4 files changed, 238 insertions(+)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-27 12:48       ` Benjamin GAIGNARD
@ 2020-05-27 14:54         ` Benjamin GAIGNARD
  2020-05-27 15:03           ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Benjamin GAIGNARD @ 2020-05-27 14:54 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: Valentin Schneider, rjw, viresh.kumar, Hugues FRUCHET, mchehab,
	mcoquelin.stm32, Alexandre TORGUE, pavel, len.brown, linux-pm,
	linux-kernel, linux-media, linux-stm32, linux-arm-kernel



On 5/27/20 2:48 PM, Benjamin GAIGNARD wrote:
>
>
> On 5/27/20 2:22 PM, Vincent Guittot wrote:
>> On Wed, 27 May 2020 at 13:17, Benjamin GAIGNARD
>> <benjamin.gaignard@st.com> wrote:
>>>
>>>
>>> On 5/27/20 12:09 PM, Valentin Schneider wrote:
>>>> Hi Benjamin,
>>>>
>>>> On 26/05/20 16:16, Benjamin Gaignard wrote:
>>>>> A first round [1] of discussions and suggestions have already be 
>>>>> done on
>>>>> this series but without found a solution to the problem. I resend 
>>>>> it to
>>>>> progress on this topic.
>>>>>
>>>> Apologies for sleeping on that previous thread.
>>>>
>>>> So what had been suggested over there was to use uclamp to boost the
>>>> frequency of the handling thread; however if you use threaded IRQs you
>>>> get RT threads, which already get the max frequency by default (at 
>>>> least
>>>> with schedutil).
>>>>
>>>> Does that not work for you, and if so, why?
>>> That doesn't work because almost everything is done by the hardware 
>>> blocks
>>> without charge the CPU so the thread isn't running. I have done the
>>> tests with schedutil
>>> and ondemand scheduler (which is the one I'm targeting). I have no
>>> issues when using
>>> performance scheduler because it always keep the highest frequencies.
>> IMHO, the only way to ensure a min frequency for anything else than a
>> thread is to use freq_qos_add_request() just like cpufreq cooling
>> device but for the opposite QoS. This can be applied only on the
>> frequency domain of the CPU which handles the interrupt.
> I will give a try with this idea.
> Thanks.

Adding freq_qos_add_request(FREQ_QOS_MIN) when starting streaming frames
solve my problem. I remove the request at the end of the streaming to 
restore
the default value.

Benjamin


>> Have you also checked the wakeup latency of your idle state ?
> It just could go in WFI so latency should be minimal.
>>
>>>
>>>>> When start streaming from the sensor the CPU load could remain 
>>>>> very low
>>>>> because almost all the capture pipeline is done in hardware (i.e. 
>>>>> without
>>>>> using the CPU) and let believe to cpufreq governor that it could 
>>>>> use lower
>>>>> frequencies. If the governor decides to use a too low frequency that
>>>>> becomes a problem when we need to acknowledge the interrupt during 
>>>>> the
>>>>> blanking time.
>>>>> The delay to ack the interrupt and perform all the other actions 
>>>>> before
>>>>> the next frame is very short and doesn't allow to the cpufreq 
>>>>> governor to
>>>>> provide the required burst of power. That led to drop the half of 
>>>>> the frames.
>>>>>
>>>>> To avoid this problem, DCMI driver informs the cpufreq governors 
>>>>> by adding
>>>>> a cpufreq minimum load QoS resquest.
>>>>>
>>>>> Benjamin
>>>>>
>>>>> [1] https://lkml.org/lkml/2020/4/24/360
>>>>>
>>>>> Benjamin Gaignard (3):
>>>>>     PM: QoS: Introduce cpufreq minimum load QoS
>>>>>     cpufreq: governor: Use minimum load QoS
>>>>>     media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>>>>
>>>>>    drivers/cpufreq/cpufreq_governor.c        |   5 +
>>>>>    drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
>>>>>    include/linux/pm_qos.h                    |  12 ++
>>>>>    kernel/power/qos.c                        | 213 
>>>>> ++++++++++++++++++++++++++++++
>>>>>    4 files changed, 238 insertions(+)
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-27 13:11       ` Benjamin GAIGNARD
@ 2020-05-27 15:02         ` Valentin Schneider
  0 siblings, 0 replies; 14+ messages in thread
From: Valentin Schneider @ 2020-05-27 15:02 UTC (permalink / raw)
  To: Benjamin GAIGNARD
  Cc: rjw, viresh.kumar, Hugues FRUCHET, mchehab, mcoquelin.stm32,
	Alexandre TORGUE, pavel, len.brown, vincent.guittot, linux-pm,
	linux-kernel, linux-media, linux-stm32, linux-arm-kernel


On 27/05/20 14:11, Benjamin GAIGNARD wrote:
> On 5/27/20 2:14 PM, Valentin Schneider wrote:
>> On 27/05/20 12:17, Benjamin GAIGNARD wrote:
>>> On 5/27/20 12:09 PM, Valentin Schneider wrote:
>>>> Hi Benjamin,
>>>>
>>>> On 26/05/20 16:16, Benjamin Gaignard wrote:
>>>>> A first round [1] of discussions and suggestions have already be done on
>>>>> this series but without found a solution to the problem. I resend it to
>>>>> progress on this topic.
>>>>>
>>>> Apologies for sleeping on that previous thread.
>>>>
>>>> So what had been suggested over there was to use uclamp to boost the
>>>> frequency of the handling thread; however if you use threaded IRQs you
>>>> get RT threads, which already get the max frequency by default (at least
>>>> with schedutil).
>>>>
>>>> Does that not work for you, and if so, why?
>>> That doesn't work because almost everything is done by the hardware blocks
>>> without charge the CPU so the thread isn't running.
>> I'm not sure I follow; the frequency of the CPU doesn't matter while
>> your hardware blocks are spinning, right? AIUI what matters is running
>> your interrupt handler / action at max freq, which you get if you use
>> threaded IRQs and schedutil.
> Yes but not limited to schedutil.
> Given the latency needed to change of frequencies I think it could
> already too late
> to change the CPU frequency when handling the threaded interrupt.

Right, on my Juno the transition latency (i.e. worse case) is about
1.2ms; I can see that eating into your time budget, depending on the
framerate you're going for.

Vincent's got a point, if you can limit that max-freq-hold to a single
frequency domain, that would probably be a tad better.

Thanks for persisting through my questioning :-)

>>
>> I think it would help if you could clarify which tasks / parts of your
>> pipeline you need running at high frequencies. The point is that setting
>> a QoS request affects all tasks, whereas we could be smarter and only
>> boost the required tasks.
> What make us drop frames is that the threaded IRQ is scheduled too late.
> The not thread part of the interrupt handler where we clear the
> interrupt flags
> is going fine but the thread part not.
>>
>>> I have done the
>>> tests with schedutil
>>> and ondemand scheduler (which is the one I'm targeting). I have no
>>> issues when using
>>> performance scheduler because it always keep the highest frequencies.
>>>
>>>
>>>>> When start streaming from the sensor the CPU load could remain very low
>>>>> because almost all the capture pipeline is done in hardware (i.e. without
>>>>> using the CPU) and let believe to cpufreq governor that it could use lower
>>>>> frequencies. If the governor decides to use a too low frequency that
>>>>> becomes a problem when we need to acknowledge the interrupt during the
>>>>> blanking time.
>>>>> The delay to ack the interrupt and perform all the other actions before
>>>>> the next frame is very short and doesn't allow to the cpufreq governor to
>>>>> provide the required burst of power. That led to drop the half of the frames.
>>>>>
>>>>> To avoid this problem, DCMI driver informs the cpufreq governors by adding
>>>>> a cpufreq minimum load QoS resquest.
>>>>>
>>>>> Benjamin
>>>>>
>>>>> [1] https://lkml.org/lkml/2020/4/24/360
>>>>>
>>>>> Benjamin Gaignard (3):
>>>>>     PM: QoS: Introduce cpufreq minimum load QoS
>>>>>     cpufreq: governor: Use minimum load QoS
>>>>>     media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>>>>
>>>>>    drivers/cpufreq/cpufreq_governor.c        |   5 +
>>>>>    drivers/media/platform/stm32/stm32-dcmi.c |   8 ++
>>>>>    include/linux/pm_qos.h                    |  12 ++
>>>>>    kernel/power/qos.c                        | 213 ++++++++++++++++++++++++++++++
>>>>>    4 files changed, 238 insertions(+)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
  2020-05-27 14:54         ` Benjamin GAIGNARD
@ 2020-05-27 15:03           ` Rafael J. Wysocki
  0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2020-05-27 15:03 UTC (permalink / raw)
  To: Benjamin GAIGNARD
  Cc: Vincent Guittot, Valentin Schneider, rjw, viresh.kumar,
	Hugues FRUCHET, mchehab, mcoquelin.stm32, Alexandre TORGUE,
	pavel, len.brown, linux-pm, linux-kernel, linux-media,
	linux-stm32, linux-arm-kernel

On Wed, May 27, 2020 at 4:54 PM Benjamin GAIGNARD
<benjamin.gaignard@st.com> wrote:
>
>
>
> On 5/27/20 2:48 PM, Benjamin GAIGNARD wrote:
> >
> >
> > On 5/27/20 2:22 PM, Vincent Guittot wrote:
> >> On Wed, 27 May 2020 at 13:17, Benjamin GAIGNARD
> >> <benjamin.gaignard@st.com> wrote:
> >>>
> >>>
> >>> On 5/27/20 12:09 PM, Valentin Schneider wrote:
> >>>> Hi Benjamin,
> >>>>
> >>>> On 26/05/20 16:16, Benjamin Gaignard wrote:
> >>>>> A first round [1] of discussions and suggestions have already be
> >>>>> done on
> >>>>> this series but without found a solution to the problem. I resend
> >>>>> it to
> >>>>> progress on this topic.
> >>>>>
> >>>> Apologies for sleeping on that previous thread.
> >>>>
> >>>> So what had been suggested over there was to use uclamp to boost the
> >>>> frequency of the handling thread; however if you use threaded IRQs you
> >>>> get RT threads, which already get the max frequency by default (at
> >>>> least
> >>>> with schedutil).
> >>>>
> >>>> Does that not work for you, and if so, why?
> >>> That doesn't work because almost everything is done by the hardware
> >>> blocks
> >>> without charge the CPU so the thread isn't running. I have done the
> >>> tests with schedutil
> >>> and ondemand scheduler (which is the one I'm targeting). I have no
> >>> issues when using
> >>> performance scheduler because it always keep the highest frequencies.
> >> IMHO, the only way to ensure a min frequency for anything else than a
> >> thread is to use freq_qos_add_request() just like cpufreq cooling
> >> device but for the opposite QoS. This can be applied only on the
> >> frequency domain of the CPU which handles the interrupt.
> > I will give a try with this idea.
> > Thanks.
>
> Adding freq_qos_add_request(FREQ_QOS_MIN) when starting streaming frames
> solve my problem. I remove the request at the end of the streaming to
> restore
> the default value.

You may as well add the request once at the init time with the request
value set to PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE initially and update
it as needed going forward.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC 1/3] PM: QoS: Introduce cpufreq minimum load QoS
  2020-04-24 11:40 [RFC " Benjamin Gaignard
@ 2020-04-24 11:40 ` Benjamin Gaignard
  0 siblings, 0 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2020-04-24 11:40 UTC (permalink / raw)
  To: rjw, viresh.kumar, hugues.fruchet, mchehab, mcoquelin.stm32,
	alexandre.torgue, pavel, len.brown
  Cc: linux-pm, linux-kernel, linux-media, linux-stm32,
	linux-arm-kernel, Benjamin Gaignard

Introduce cpufreq minimum load QoS, based on the "raw" low-level
PM QoS, to represent the minimum expected cpu load by various devices.

The cpufreq_minload_qos_limit() helper is defined to retrieve the
aggregated constraints.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 include/linux/pm_qos.h |  12 +++
 kernel/power/qos.c     | 213 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 225 insertions(+)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 4a69d4af3ff8..e2cc099322e3 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -316,4 +316,16 @@ int freq_qos_remove_notifier(struct freq_constraints *qos,
 			     enum freq_qos_req_type type,
 			     struct notifier_block *notifier);
 
+/* Definitions related to the cpufreq minimum load QoS. */
+
+#define CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE	0
+#define CPUFREQ_GOV_QOS_MIN_LOAD_MAX_VALUE	10
+
+s32 cpufreq_minload_qos_limit(void);
+bool cpufreq_minload_qos_request_active(struct pm_qos_request *req);
+void cpufreq_minload_qos_add_request(struct pm_qos_request *req, s32 value);
+void cpufreq_minload_qos_update_request(struct pm_qos_request *req,
+					s32 new_value);
+void cpufreq_minload_qos_remove_request(struct pm_qos_request *req);
+
 #endif
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index db0bed2cae26..df2fdd962f35 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -671,3 +671,216 @@ int freq_qos_remove_notifier(struct freq_constraints *qos,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(freq_qos_remove_notifier);
+
+/* Definitions related to the cpufreq minimum load QoS. */
+
+static struct pm_qos_constraints cpufreq_minload_constraints = {
+	.list = PLIST_HEAD_INIT(cpufreq_minload_constraints.list),
+	.target_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+	.default_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+	.no_constraint_value = CPUFREQ_GOV_QOS_MIN_LOAD_DEFAULT_VALUE,
+	.type = PM_QOS_MAX,
+};
+
+/**
+ * cpufreq_minload_qos_limit - Return current system-wide cpufreq
+ * minimum load QoS limit.
+ */
+s32 cpufreq_minload_qos_limit(void)
+{
+	return pm_qos_read_value(&cpufreq_minload_constraints);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_limit);
+
+/**
+ * cpufreq_minload_qos_request_active - Check the given PM QoS request.
+ * @req: PM QoS request to check.
+ *
+ * Return: 'true' if @req has been added to the cpufreq minimum load
+ * QoS list, 'false' otherwise.
+ */
+bool cpufreq_minload_qos_request_active(struct pm_qos_request *req)
+{
+	return req->qos == &cpufreq_minload_constraints;
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_request_active);
+
+/**
+ * cpufreq_minload_qos_add_request - Add new cpufreq minimum load QoS request.
+ * @req: Pointer to a preallocated handle.
+ * @value: Requested constraint value.
+ *
+ * Use @value to initialize the request handle pointed to by @req, insert it as
+ * a new entry to the cpufreq minimum load QoS list and recompute the effective
+ * QoS constraint for that list.
+ *
+ * Callers need to save the handle for later use in updates and removal of the
+ * QoS request represented by it.
+ */
+void cpufreq_minload_qos_add_request(struct pm_qos_request *req, s32 value)
+{
+	if (!req)
+		return;
+
+	if (cpufreq_minload_qos_request_active(req)) {
+		WARN(1, KERN_ERR "%s called for already added request\n", __func__);
+		return;
+	}
+
+	trace_pm_qos_add_request(value);
+
+	req->qos = &cpufreq_minload_constraints;
+	pm_qos_update_target(req->qos, &req->node, PM_QOS_ADD_REQ, value);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_add_request);
+
+/**
+ * cpufreq_minload_qos_update_request - Modify existing cpufreq minimum load
+ * QoS request.
+ * @req : QoS request to update.
+ * @new_value: New requested constraint value.
+ *
+ * Use @new_value to update the QoS request represented by @req in the cpufreq
+ * minimum load QoS list along with updating the effective constraint value for
+ * that list.
+ */
+void cpufreq_minload_qos_update_request(struct pm_qos_request *req,
+					s32 new_value)
+{
+	if (!req)
+		return;
+
+	if (!cpufreq_minload_qos_request_active(req)) {
+		WARN(1, KERN_ERR "%s called for unknown object\n", __func__);
+		return;
+	}
+
+	trace_pm_qos_update_request(new_value);
+
+	if (new_value == req->node.prio)
+		return;
+
+	pm_qos_update_target(req->qos, &req->node, PM_QOS_UPDATE_REQ, new_value);
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_update_request);
+
+/**
+ * cpufreq_minload_qos_remove_request - Remove existing cpufreq minimum load QoS
+ * request.
+ * @req: QoS request to remove.
+ *
+ * Remove the cpufreq minimum load QoS request represented by @req from the
+ * cpufreq minimum load QoS list along with updating the effective constraint
+ * value for that list.
+ */
+void cpufreq_minload_qos_remove_request(struct pm_qos_request *req)
+{
+	if (!req)
+		return;
+
+	if (!cpufreq_minload_qos_request_active(req)) {
+		WARN(1, KERN_ERR "%s called for unknown object\n", __func__);
+		return;
+	}
+
+	trace_pm_qos_remove_request(PM_QOS_DEFAULT_VALUE);
+
+	pm_qos_update_target(req->qos, &req->node,
+			     PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
+	memset(req, 0, sizeof(*req));
+}
+EXPORT_SYMBOL_GPL(cpufreq_minload_qos_remove_request);
+
+/* User space interface to the cpufreq minimum load QoS via misc device. */
+
+static int cpufreq_minload_qos_open(struct inode *inode, struct file *filp)
+{
+	struct pm_qos_request *req;
+
+	req = kzalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	cpufreq_minload_qos_add_request(req, PM_QOS_DEFAULT_VALUE);
+	filp->private_data = req;
+
+	return 0;
+}
+
+static int cpufreq_minload_qos_release(struct inode *inode, struct file *filp)
+{
+	struct pm_qos_request *req = filp->private_data;
+
+	filp->private_data = NULL;
+
+	cpufreq_minload_qos_remove_request(req);
+	kfree(req);
+
+	return 0;
+}
+
+static ssize_t cpufreq_minload_qos_read(struct file *filp, char __user *buf,
+					size_t count, loff_t *f_pos)
+{
+	struct pm_qos_request *req = filp->private_data;
+	unsigned long flags;
+	s32 value;
+
+	if (!req || !cpufreq_minload_qos_request_active(req))
+		return -EINVAL;
+
+	spin_lock_irqsave(&pm_qos_lock, flags);
+	value = pm_qos_get_value(&cpufreq_minload_constraints);
+	spin_unlock_irqrestore(&pm_qos_lock, flags);
+
+	return simple_read_from_buffer(buf, count, f_pos, &value, sizeof(s32));
+}
+
+static ssize_t cpufreq_minload_qos_write(struct file *filp,
+					 const char __user *buf,
+					 size_t count, loff_t *f_pos)
+{
+	s32 value;
+
+	if (count == sizeof(s32)) {
+		if (copy_from_user(&value, buf, sizeof(s32)))
+			return -EFAULT;
+	} else {
+		int ret;
+
+		ret = kstrtos32_from_user(buf, count, 16, &value);
+		if (ret)
+			return ret;
+	}
+
+	cpufreq_minload_qos_update_request(filp->private_data, value);
+
+	return count;
+}
+
+static const struct file_operations cpufreq_minload_qos_fops = {
+	.write = cpufreq_minload_qos_write,
+	.read = cpufreq_minload_qos_read,
+	.open = cpufreq_minload_qos_open,
+	.release = cpufreq_minload_qos_release,
+	.llseek = noop_llseek,
+};
+
+static struct miscdevice cpufreq_minload_qos_miscdev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "cpufreq_minimum_load",
+	.fops = &cpufreq_minload_qos_fops,
+};
+
+static int __init cpufreq_minload_qos_init(void)
+{
+	int ret;
+
+	ret = misc_register(&cpufreq_minload_qos_miscdev);
+	if (ret < 0)
+		pr_err("%s: %s setup failed\n", __func__,
+		       cpufreq_minload_qos_miscdev.name);
+
+	return ret;
+}
+late_initcall(cpufreq_minload_qos_init);
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-05-27 15:04 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 15:16 [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Benjamin Gaignard
2020-05-26 15:16 ` [RFC 1/3] PM: QoS: " Benjamin Gaignard
2020-05-26 15:16 ` [RFC 2/3] cpufreq: governor: Use " Benjamin Gaignard
2020-05-26 15:16 ` [RFC 3/3] media: stm32-dcmi: Inform cpufreq governors about cpu load needs Benjamin Gaignard
2020-05-27 10:09 ` [RFC RESEND 0/3] Introduce cpufreq minimum load QoS Valentin Schneider
2020-05-27 11:17   ` Benjamin GAIGNARD
2020-05-27 12:14     ` Valentin Schneider
2020-05-27 13:11       ` Benjamin GAIGNARD
2020-05-27 15:02         ` Valentin Schneider
2020-05-27 12:22     ` Vincent Guittot
2020-05-27 12:48       ` Benjamin GAIGNARD
2020-05-27 14:54         ` Benjamin GAIGNARD
2020-05-27 15:03           ` Rafael J. Wysocki
  -- strict thread matches above, loose matches on Subject: below --
2020-04-24 11:40 [RFC " Benjamin Gaignard
2020-04-24 11:40 ` [RFC 1/3] PM: QoS: " Benjamin Gaignard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).