Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v6 0/3] DCMI set minimum cpufreq requirement
@ 2020-07-01 12:59 Benjamin Gaignard
  2020-07-01 12:59 ` [PATCH v6 1/3] dt-bindings: media: stm32-dcmi: Add DCMI min frequency property Benjamin Gaignard
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-07-01 12:59 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue, robh+dt
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, valentin.schneider, rjw, devicetree,
	Benjamin Gaignard

This series allow to STM32 camera interface (DCMI) to require a minimum
frequency to the CPUs before start streaming frames from the sensor.
The minimum frequency requirement is provided in the devide-tree node.

Setting a minimum frequency for the CPUs is needed to ensure a quick handling
of the interrupts between two sensor frames and avoid dropping half of them.

version 6:
- come back to version 4 and follow Valentin's suggestions about notifier

version 5:
- add a mutex to protect dcmi_irq_notifier_notify()
- register notifier a probe time

version 4:
- simplify irq affinity handling by using only dcmi_irq_notifier_notify() 

version 3:
- add a cpumask field to track boosted CPUs
- add irq_affinity_notify callback
- protect cpumask field with a mutex 

Benjamin Gaignard (3):
  dt-bindings: media: stm32-dcmi: Add DCMI min frequency property
  media: stm32-dcmi: Set minimum cpufreq requirement
  ARM: dts: stm32: Set DCMI frequency requirement for stm32mp15x

 .../devicetree/bindings/media/st,stm32-dcmi.yaml   |   8 ++
 arch/arm/boot/dts/stm32mp151.dtsi                  |   1 +
 drivers/media/platform/stm32/stm32-dcmi.c          | 138 +++++++++++++++++++--
 3 files changed, 139 insertions(+), 8 deletions(-)

-- 
2.15.0


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

* [PATCH v6 1/3] dt-bindings: media: stm32-dcmi: Add DCMI min frequency property
  2020-07-01 12:59 [PATCH v6 0/3] DCMI set minimum cpufreq requirement Benjamin Gaignard
@ 2020-07-01 12:59 ` Benjamin Gaignard
  2020-07-01 12:59 ` [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement Benjamin Gaignard
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-07-01 12:59 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue, robh+dt
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, valentin.schneider, rjw, devicetree,
	Benjamin Gaignard

Document st,stm32-dcmi-min-frequency property which is used to
request CPUs minimum frequency when streaming frames.

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

diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
index 3fe778cb5cc3..05ca85a2411a 100644
--- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
+++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
@@ -44,6 +44,13 @@ properties:
       bindings defined in
       Documentation/devicetree/bindings/media/video-interfaces.txt.
 
+  st,stm32-dcmi-min-frequency:
+    description: DCMI minimum CPUs frequency requirement (in KHz).
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - minimum: 0
+      - default: 0
+
 required:
   - compatible
   - reg
@@ -71,6 +78,7 @@ examples:
         clock-names = "mclk";
         dmas = <&dmamux1 75 0x400 0x0d>;
         dma-names = "tx";
+        st,stm32-dcmi-min-frequency = <650000>;
 
         port {
              dcmi_0: endpoint {
-- 
2.15.0


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

* [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement
  2020-07-01 12:59 [PATCH v6 0/3] DCMI set minimum cpufreq requirement Benjamin Gaignard
  2020-07-01 12:59 ` [PATCH v6 1/3] dt-bindings: media: stm32-dcmi: Add DCMI min frequency property Benjamin Gaignard
@ 2020-07-01 12:59 ` Benjamin Gaignard
  2020-07-01 12:59 ` [PATCH v6 3/3] ARM: dts: stm32: Set DCMI frequency requirement for stm32mp15x Benjamin Gaignard
  2020-07-01 13:02 ` [PATCH v6 0/3] DCMI set minimum cpufreq requirement Benjamin GAIGNARD
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-07-01 12:59 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue, robh+dt
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, valentin.schneider, rjw, devicetree,
	Benjamin Gaignard

Before start streaming set cpufreq minimum frequency requirement.
The cpufreq governor will adapt the frequencies and we will have
no latency for handling interrupts.
The frequency requirement is retrieved from the device-tree node.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 6:
- come back to version 4 and follow Valentin's suggestions about notifier
- add Valentin's comment about notifier set/unset

version 5:
- add a mutex to protect dcmi_irq_notifier_notify()
- register notifier a probe time

version 4:
- simplify irq affinity handling by using only dcmi_irq_notifier_notify() 

version 3:
- add a cpumask field to track boosted CPUs
- add irq_affinity_notify callback
- protect cpumask field with a mutex 

 drivers/media/platform/stm32/stm32-dcmi.c | 138 ++++++++++++++++++++++++++++--
 1 file changed, 130 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b8931490b83b..382df6e7c864 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -13,10 +13,13 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/cpufreq.h>
+#include <linux/cpumask.h>
 #include <linux/delay.h>
 #include <linux/dmaengine.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -99,6 +102,8 @@ enum state {
 
 #define OVERRUN_ERROR_THRESHOLD	3
 
+static DEFINE_PER_CPU(struct freq_qos_request, qos_req);
+
 struct dcmi_graph_entity {
 	struct v4l2_async_subdev asd;
 
@@ -133,6 +138,7 @@ struct stm32_dcmi {
 	struct resource			*res;
 	struct reset_control		*rstc;
 	int				sequence;
+	int				irq;
 	struct list_head		buffers;
 	struct dcmi_buf			*active;
 
@@ -173,6 +179,11 @@ struct stm32_dcmi {
 	struct media_device		mdev;
 	struct media_pad		vid_cap_pad;
 	struct media_pipeline		pipeline;
+
+	struct mutex			freq_lock;
+	u32				min_frequency;
+	cpumask_var_t			boosted;
+	struct irq_affinity_notify	notify;
 };
 
 static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
@@ -722,6 +733,99 @@ static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi)
 	dcmi_pipeline_s_stream(dcmi, 0);
 }
 
+static void dcmi_get_min_frequency(struct stm32_dcmi *dcmi)
+{
+	struct device_node *np = dcmi->mdev.dev->of_node;
+
+	dcmi->min_frequency = FREQ_QOS_MIN_DEFAULT_VALUE;
+
+	of_property_read_u32(np, "st,stm32-dcmi-min-frequency",
+			     &dcmi->min_frequency);
+}
+
+static void dcmi_irq_notifier_notify(struct irq_affinity_notify *notify,
+				     const cpumask_t *mask)
+{
+	struct stm32_dcmi *dcmi = container_of(notify,
+					       struct stm32_dcmi,
+					       notify);
+	struct cpufreq_policy *p;
+	int cpu;
+
+	mutex_lock(&dcmi->freq_lock);
+	/*
+	 * For all boosted CPUs check if it is still the case
+	 * if not remove the request
+	 */
+	for_each_cpu(cpu, dcmi->boosted) {
+		if (cpumask_test_cpu(cpu, mask))
+			continue;
+
+		p = cpufreq_cpu_get(cpu);
+		if (!p)
+			continue;
+
+		freq_qos_remove_request(&per_cpu(qos_req, cpu));
+		cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
+
+		cpufreq_cpu_put(p);
+	}
+
+	/*
+	 * For CPUs in the mask check if they are boosted if not add
+	 * a request
+	 */
+	for_each_cpu(cpu, mask) {
+		if (cpumask_test_cpu(cpu, dcmi->boosted))
+			continue;
+
+		p = cpufreq_cpu_get(cpu);
+		if (!p)
+			continue;
+
+		freq_qos_add_request(&p->constraints, &per_cpu(qos_req, cpu),
+				     FREQ_QOS_MIN, dcmi->min_frequency);
+		cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
+		cpufreq_cpu_put(p);
+	}
+
+	mutex_unlock(&dcmi->freq_lock);
+}
+
+static void dcmi_irq_notifier_release(struct kref *ref)
+{
+	/*
+	 * This is required by affinity notifier. We don't have anything to
+	 * free here.
+	 */
+}
+
+static void dcmi_set_min_frequency(struct stm32_dcmi *dcmi, s32 freq)
+{
+	struct irq_affinity_notify *notify = &dcmi->notify;
+	struct cpumask clear;
+
+	if (freq) {
+		/*
+		 * Register the notifier before doing any change, so the
+		 * callback can be queued if an affinity change happens *while*
+		 * we are requesting the boosts.
+		 */
+		irq_set_affinity_notifier(dcmi->irq, notify);
+		dcmi_irq_notifier_notify(notify,
+					 irq_get_affinity_mask(dcmi->irq));
+	} else {
+		/*
+		 * Unregister the notifier before clearing the boost requests,
+		 * as we don't want to boost again if an affinity change happens
+		 * *while* we are clearing the requests
+		 */
+		irq_set_affinity_notifier(dcmi->irq, NULL);
+		cpumask_clear(&clear);
+		dcmi_irq_notifier_notify(notify, &clear);
+	}
+}
+
 static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
 	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
@@ -736,11 +840,13 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 		goto err_release_buffers;
 	}
 
+	dcmi_set_min_frequency(dcmi, dcmi->min_frequency);
+
 	ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline);
 	if (ret < 0) {
 		dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n",
 			__func__, ret);
-		goto err_pm_put;
+		goto err_drop_qos;
 	}
 
 	ret = dcmi_pipeline_start(dcmi);
@@ -835,7 +941,8 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 err_media_pipeline_stop:
 	media_pipeline_stop(&dcmi->vdev->entity);
 
-err_pm_put:
+err_drop_qos:
+	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE);
 	pm_runtime_put(dcmi->dev);
 
 err_release_buffers:
@@ -863,6 +970,8 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
 
 	media_pipeline_stop(&dcmi->vdev->entity);
 
+	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE);
+
 	spin_lock_irq(&dcmi->irqlock);
 
 	/* Disable interruptions */
@@ -1834,11 +1943,11 @@ static int dcmi_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *match = NULL;
 	struct v4l2_fwnode_endpoint ep = { .bus_type = 0 };
+	struct irq_affinity_notify *notify;
 	struct stm32_dcmi *dcmi;
 	struct vb2_queue *q;
 	struct dma_chan *chan;
 	struct clk *mclk;
-	int irq;
 	int ret = 0;
 
 	match = of_match_device(of_match_ptr(stm32_dcmi_of_match), &pdev->dev);
@@ -1879,9 +1988,9 @@ static int dcmi_probe(struct platform_device *pdev)
 	dcmi->bus.bus_width = ep.bus.parallel.bus_width;
 	dcmi->bus.data_shift = ep.bus.parallel.data_shift;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0)
-		return irq ? irq : -ENXIO;
+	dcmi->irq = platform_get_irq(pdev, 0);
+	if (dcmi->irq <= 0)
+		return dcmi->irq ? dcmi->irq : -ENXIO;
 
 	dcmi->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!dcmi->res) {
@@ -1895,11 +2004,12 @@ static int dcmi_probe(struct platform_device *pdev)
 		return PTR_ERR(dcmi->regs);
 	}
 
-	ret = devm_request_threaded_irq(&pdev->dev, irq, dcmi_irq_callback,
+	ret = devm_request_threaded_irq(&pdev->dev, dcmi->irq,
+					dcmi_irq_callback,
 					dcmi_irq_thread, IRQF_ONESHOT,
 					dev_name(&pdev->dev), dcmi);
 	if (ret) {
-		dev_err(&pdev->dev, "Unable to request irq %d\n", irq);
+		dev_err(&pdev->dev, "Unable to request irq %d\n", dcmi->irq);
 		return ret;
 	}
 
@@ -1922,6 +2032,7 @@ static int dcmi_probe(struct platform_device *pdev)
 	spin_lock_init(&dcmi->irqlock);
 	mutex_init(&dcmi->lock);
 	mutex_init(&dcmi->dma_lock);
+	mutex_init(&dcmi->freq_lock);
 	init_completion(&dcmi->complete);
 	INIT_LIST_HEAD(&dcmi->buffers);
 
@@ -1930,6 +2041,13 @@ static int dcmi_probe(struct platform_device *pdev)
 	dcmi->state = STOPPED;
 	dcmi->dma_chan = chan;
 
+	if (!alloc_cpumask_var(&dcmi->boosted, GFP_KERNEL))
+		return -ENODEV;
+
+	notify = &dcmi->notify;
+	notify->notify = dcmi_irq_notifier_notify;
+	notify->release = dcmi_irq_notifier_release;
+
 	q = &dcmi->queue;
 
 	dcmi->v4l2_dev.mdev = &dcmi->mdev;
@@ -2022,6 +2140,8 @@ static int dcmi_probe(struct platform_device *pdev)
 
 	dev_info(&pdev->dev, "Probe done\n");
 
+	dcmi_get_min_frequency(dcmi);
+
 	platform_set_drvdata(pdev, dcmi);
 
 	pm_runtime_enable(&pdev->dev);
@@ -2049,6 +2169,8 @@ static int dcmi_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 
+	free_cpumask_var(dcmi->boosted);
+
 	v4l2_async_notifier_unregister(&dcmi->notifier);
 	v4l2_async_notifier_cleanup(&dcmi->notifier);
 	media_entity_cleanup(&dcmi->vdev->entity);
-- 
2.15.0


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

* [PATCH v6 3/3] ARM: dts: stm32: Set DCMI frequency requirement for stm32mp15x
  2020-07-01 12:59 [PATCH v6 0/3] DCMI set minimum cpufreq requirement Benjamin Gaignard
  2020-07-01 12:59 ` [PATCH v6 1/3] dt-bindings: media: stm32-dcmi: Add DCMI min frequency property Benjamin Gaignard
  2020-07-01 12:59 ` [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement Benjamin Gaignard
@ 2020-07-01 12:59 ` Benjamin Gaignard
  2020-07-01 13:02 ` [PATCH v6 0/3] DCMI set minimum cpufreq requirement Benjamin GAIGNARD
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-07-01 12:59 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue, robh+dt
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, valentin.schneider, rjw, devicetree,
	Benjamin Gaignard

Make sure that CPUs will at least run at 650Mhz when streaming
sensor frames.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 arch/arm/boot/dts/stm32mp151.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
index 3ea05ba48215..f6d7bf4f8231 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -1091,6 +1091,7 @@
 			clock-names = "mclk";
 			dmas = <&dmamux1 75 0x400 0x0d>;
 			dma-names = "tx";
+			st,stm32-dcmi-min-frequency = <650000>;
 			status = "disabled";
 		};
 
-- 
2.15.0


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

* Re: [PATCH v6 0/3] DCMI set minimum cpufreq requirement
  2020-07-01 12:59 [PATCH v6 0/3] DCMI set minimum cpufreq requirement Benjamin Gaignard
                   ` (2 preceding siblings ...)
  2020-07-01 12:59 ` [PATCH v6 3/3] ARM: dts: stm32: Set DCMI frequency requirement for stm32mp15x Benjamin Gaignard
@ 2020-07-01 13:02 ` Benjamin GAIGNARD
  3 siblings, 0 replies; 9+ messages in thread
From: Benjamin GAIGNARD @ 2020-07-01 13:02 UTC (permalink / raw)
  To: Hugues FRUCHET, mchehab, mcoquelin.stm32, Alexandre TORGUE, robh+dt
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, valentin.schneider, rjw, devicetree



On 7/1/20 2:59 PM, Benjamin Gaignard wrote:
> This series allow to STM32 camera interface (DCMI) to require a minimum
> frequency to the CPUs before start streaming frames from the sensor.
> The minimum frequency requirement is provided in the devide-tree node.
>
> Setting a minimum frequency for the CPUs is needed to ensure a quick handling
> of the interrupts between two sensor frames and avoid dropping half of them.
Please forget this version, the incoming version 7 should have fix your 
remarks.

Sorry,
Benjamin
> version 6:
> - come back to version 4 and follow Valentin's suggestions about notifier
>
> version 5:
> - add a mutex to protect dcmi_irq_notifier_notify()
> - register notifier a probe time
>
> version 4:
> - simplify irq affinity handling by using only dcmi_irq_notifier_notify()
>
> version 3:
> - add a cpumask field to track boosted CPUs
> - add irq_affinity_notify callback
> - protect cpumask field with a mutex
>
> Benjamin Gaignard (3):
>    dt-bindings: media: stm32-dcmi: Add DCMI min frequency property
>    media: stm32-dcmi: Set minimum cpufreq requirement
>    ARM: dts: stm32: Set DCMI frequency requirement for stm32mp15x
>
>   .../devicetree/bindings/media/st,stm32-dcmi.yaml   |   8 ++
>   arch/arm/boot/dts/stm32mp151.dtsi                  |   1 +
>   drivers/media/platform/stm32/stm32-dcmi.c          | 138 +++++++++++++++++++--
>   3 files changed, 139 insertions(+), 8 deletions(-)
>

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

* Re: [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement
  2020-06-24  9:24   ` Hugues FRUCHET
@ 2020-06-24 11:27     ` Benjamin GAIGNARD
  0 siblings, 0 replies; 9+ messages in thread
From: Benjamin GAIGNARD @ 2020-06-24 11:27 UTC (permalink / raw)
  To: Hugues FRUCHET, mchehab, mcoquelin.stm32, Alexandre TORGUE
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, valentin.schneider, rjw



On 6/24/20 11:24 AM, Hugues FRUCHET wrote:
> Hi Benjamin,
>
> test condition in set_min_frequency() to fix, appart from that:
> Acked-by: Hugues Fruchet <hugues.fruchet@st.com>
>
> BR,
> Hugues.
>
> On 6/10/20 2:24 PM, Benjamin Gaignard wrote:
>> Before start streaming set cpufreq minimum frequency requirement.
>> The cpufreq governor will adapt the frequencies and we will have
>> no latency for handling interrupts.
>> The frequency requirement is retrieved from the device-tree node.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>> version 6:
>> - come back to version 4 and follow Valentin's suggestions about notifier
>> - add Valentin's comment about notifier set/unset
>>
>> version 5:
>> - add a mutex to protect dcmi_irq_notifier_notify()
>> - register notifier a probe time
>>
>> version 4:
>> - simplify irq affinity handling by using only dcmi_irq_notifier_notify()
>>
>> version 3:
>> - add a cpumask field to track boosted CPUs
>> - add irq_affinity_notify callback
>> - protect cpumask field with a mutex
>>
>>    drivers/media/platform/stm32/stm32-dcmi.c | 138 ++++++++++++++++++++++++++++--
>>    1 file changed, 130 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
>> index b8931490b83b..382df6e7c864 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -13,10 +13,13 @@
>>    
>>    #include <linux/clk.h>
>>    #include <linux/completion.h>
>> +#include <linux/cpufreq.h>
>> +#include <linux/cpumask.h>
>>    #include <linux/delay.h>
>>    #include <linux/dmaengine.h>
>>    #include <linux/init.h>
>>    #include <linux/interrupt.h>
>> +#include <linux/irq.h>
>>    #include <linux/kernel.h>
>>    #include <linux/module.h>
>>    #include <linux/of.h>
>> @@ -99,6 +102,8 @@ enum state {
>>    
>>    #define OVERRUN_ERROR_THRESHOLD	3
>>    
>> +static DEFINE_PER_CPU(struct freq_qos_request, qos_req);
>> +
>>    struct dcmi_graph_entity {
>>    	struct v4l2_async_subdev asd;
>>    
>> @@ -133,6 +138,7 @@ struct stm32_dcmi {
>>    	struct resource			*res;
>>    	struct reset_control		*rstc;
>>    	int				sequence;
>> +	int				irq;
>>    	struct list_head		buffers;
>>    	struct dcmi_buf			*active;
>>    
>> @@ -173,6 +179,11 @@ struct stm32_dcmi {
>>    	struct media_device		mdev;
>>    	struct media_pad		vid_cap_pad;
>>    	struct media_pipeline		pipeline;
>> +
>> +	struct mutex			freq_lock;
>> +	u32				min_frequency;
>> +	cpumask_var_t			boosted;
>> +	struct irq_affinity_notify	notify;
>>    };
>>    
>>    static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
>> @@ -722,6 +733,99 @@ static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi)
>>    	dcmi_pipeline_s_stream(dcmi, 0);
>>    }
>>    
>> +static void dcmi_get_min_frequency(struct stm32_dcmi *dcmi)
>> +{
>> +	struct device_node *np = dcmi->mdev.dev->of_node;
>> +
>> +	dcmi->min_frequency = FREQ_QOS_MIN_DEFAULT_VALUE;
>> +
>> +	of_property_read_u32(np, "st,stm32-dcmi-min-frequency",
>> +			     &dcmi->min_frequency);
>> +}
>> +
>> +static void dcmi_irq_notifier_notify(struct irq_affinity_notify *notify,
>> +				     const cpumask_t *mask)
>> +{
>> +	struct stm32_dcmi *dcmi = container_of(notify,
>> +					       struct stm32_dcmi,
>> +					       notify);
>> +	struct cpufreq_policy *p;
>> +	int cpu;
>> +
>> +	mutex_lock(&dcmi->freq_lock);
>> +	/*
>> +	 * For all boosted CPUs check if it is still the case
>> +	 * if not remove the request
>> +	 */
>> +	for_each_cpu(cpu, dcmi->boosted) {
>> +		if (cpumask_test_cpu(cpu, mask))
>> +			continue;
>> +
>> +		p = cpufreq_cpu_get(cpu);
>> +		if (!p)
>> +			continue;
>> +
>> +		freq_qos_remove_request(&per_cpu(qos_req, cpu));
>> +		cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
>> +
>> +		cpufreq_cpu_put(p);
>> +	}
>> +
>> +	/*
>> +	 * For CPUs in the mask check if they are boosted if not add
>> +	 * a request
>> +	 */
>> +	for_each_cpu(cpu, mask) {
>> +		if (cpumask_test_cpu(cpu, dcmi->boosted))
>> +			continue;
>> +
>> +		p = cpufreq_cpu_get(cpu);
>> +		if (!p)
>> +			continue;
>> +
>> +		freq_qos_add_request(&p->constraints, &per_cpu(qos_req, cpu),
>> +				     FREQ_QOS_MIN, dcmi->min_frequency);
>> +		cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
>> +		cpufreq_cpu_put(p);
>> +	}
>> +
>> +	mutex_unlock(&dcmi->freq_lock);
>> +}
>> +
>> +static void dcmi_irq_notifier_release(struct kref *ref)
>> +{
>> +	/*
>> +	 * This is required by affinity notifier. We don't have anything to
>> +	 * free here.
>> +	 */
>> +}
>> +
>> +static void dcmi_set_min_frequency(struct stm32_dcmi *dcmi, s32 freq)
>> +{
>> +	struct irq_affinity_notify *notify = &dcmi->notify;
>> +	struct cpumask clear;
>> +
>> +	if (freq) {
> if (freq != FREQ_QOS_MIN_DEFAULT_VALUE)

FREQ_QOS_MIN_DEFAULT_VALUE value is zero so that won't change anything.

Thanks,
Benjamin

>> +		/*
>> +		 * Register the notifier before doing any change, so the
>> +		 * callback can be queued if an affinity change happens *while*
>> +		 * we are requesting the boosts.
>> +		 */
>> +		irq_set_affinity_notifier(dcmi->irq, notify);
>> +		dcmi_irq_notifier_notify(notify,
>> +					 irq_get_affinity_mask(dcmi->irq));
>> +	} else {
>> +		/*
>> +		 * Unregister the notifier before clearing the boost requests,
>> +		 * as we don't want to boost again if an affinity change happens
>> +		 * *while* we are clearing the requests
>> +		 */
>> +		irq_set_affinity_notifier(dcmi->irq, NULL);
>> +		cpumask_clear(&clear);
>> +		dcmi_irq_notifier_notify(notify, &clear);
>> +	}
>> +}
>> +
>>    static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
>>    {
>>    	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
>> @@ -736,11 +840,13 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
>>    		goto err_release_buffers;
>>    	}
>>    
>> +	dcmi_set_min_frequency(dcmi, dcmi->min_frequency);
>> +
>>    	ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline);
>>    	if (ret < 0) {
>>    		dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n",
>>    			__func__, ret);
>> -		goto err_pm_put;
>> +		goto err_drop_qos;
>>    	}
>>    
>>    	ret = dcmi_pipeline_start(dcmi);
>> @@ -835,7 +941,8 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
>>    err_media_pipeline_stop:
>>    	media_pipeline_stop(&dcmi->vdev->entity);
>>    
>> -err_pm_put:
>> +err_drop_qos:
>> +	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE) >   	pm_runtime_put(dcmi->dev);
>>    
>>    err_release_buffers:
>> @@ -863,6 +970,8 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
>>    
>>    	media_pipeline_stop(&dcmi->vdev->entity);
>>    
>> +	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE);
>> +
>>    	spin_lock_irq(&dcmi->irqlock);
>>    
>>    	/* Disable interruptions */
>> @@ -1834,11 +1943,11 @@ static int dcmi_probe(struct platform_device *pdev)
>>    	struct device_node *np = pdev->dev.of_node;
>>    	const struct of_device_id *match = NULL;
>>    	struct v4l2_fwnode_endpoint ep = { .bus_type = 0 };
>> +	struct irq_affinity_notify *notify;
>>    	struct stm32_dcmi *dcmi;
>>    	struct vb2_queue *q;
>>    	struct dma_chan *chan;
>>    	struct clk *mclk;
>> -	int irq;
>>    	int ret = 0;
>>    
>>    	match = of_match_device(of_match_ptr(stm32_dcmi_of_match), &pdev->dev);
>> @@ -1879,9 +1988,9 @@ static int dcmi_probe(struct platform_device *pdev)
>>    	dcmi->bus.bus_width = ep.bus.parallel.bus_width;
>>    	dcmi->bus.data_shift = ep.bus.parallel.data_shift;
>>    
>> -	irq = platform_get_irq(pdev, 0);
>> -	if (irq <= 0)
>> -		return irq ? irq : -ENXIO;
>> +	dcmi->irq = platform_get_irq(pdev, 0);
>> +	if (dcmi->irq <= 0)
>> +		return dcmi->irq ? dcmi->irq : -ENXIO;
>>    
>>    	dcmi->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>    	if (!dcmi->res) {
>> @@ -1895,11 +2004,12 @@ static int dcmi_probe(struct platform_device *pdev)
>>    		return PTR_ERR(dcmi->regs);
>>    	}
>>    
>> -	ret = devm_request_threaded_irq(&pdev->dev, irq, dcmi_irq_callback,
>> +	ret = devm_request_threaded_irq(&pdev->dev, dcmi->irq,
>> +					dcmi_irq_callback,
>>    					dcmi_irq_thread, IRQF_ONESHOT,
>>    					dev_name(&pdev->dev), dcmi);
>>    	if (ret) {
>> -		dev_err(&pdev->dev, "Unable to request irq %d\n", irq);
>> +		dev_err(&pdev->dev, "Unable to request irq %d\n", dcmi->irq);
>>    		return ret;
>>    	}
>>    
>> @@ -1922,6 +2032,7 @@ static int dcmi_probe(struct platform_device *pdev)
>>    	spin_lock_init(&dcmi->irqlock);
>>    	mutex_init(&dcmi->lock);
>>    	mutex_init(&dcmi->dma_lock);
>> +	mutex_init(&dcmi->freq_lock);
>>    	init_completion(&dcmi->complete);
>>    	INIT_LIST_HEAD(&dcmi->buffers);
>>    
>> @@ -1930,6 +2041,13 @@ static int dcmi_probe(struct platform_device *pdev)
>>    	dcmi->state = STOPPED;
>>    	dcmi->dma_chan = chan;
>>    
>> +	if (!alloc_cpumask_var(&dcmi->boosted, GFP_KERNEL))
>> +		return -ENODEV;
>> +
>> +	notify = &dcmi->notify;
>> +	notify->notify = dcmi_irq_notifier_notify;
>> +	notify->release = dcmi_irq_notifier_release;
>> +
>>    	q = &dcmi->queue;
>>    
>>    	dcmi->v4l2_dev.mdev = &dcmi->mdev;
>> @@ -2022,6 +2140,8 @@ static int dcmi_probe(struct platform_device *pdev)
>>    
>>    	dev_info(&pdev->dev, "Probe done\n");
>>    
>> +	dcmi_get_min_frequency(dcmi);
>> +
>>    	platform_set_drvdata(pdev, dcmi);
>>    
>>    	pm_runtime_enable(&pdev->dev);
>> @@ -2049,6 +2169,8 @@ static int dcmi_remove(struct platform_device *pdev)
>>    
>>    	pm_runtime_disable(&pdev->dev);
>>    
>> +	free_cpumask_var(dcmi->boosted);
>> +
>>    	v4l2_async_notifier_unregister(&dcmi->notifier);
>>    	v4l2_async_notifier_cleanup(&dcmi->notifier);
>>    	media_entity_cleanup(&dcmi->vdev->entity);

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

* Re: [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement
  2020-06-10 12:24 ` [PATCH v6 2/3] media: stm32-dcmi: Set " Benjamin Gaignard
  2020-06-10 17:16   ` Valentin Schneider
@ 2020-06-24  9:24   ` Hugues FRUCHET
  2020-06-24 11:27     ` Benjamin GAIGNARD
  1 sibling, 1 reply; 9+ messages in thread
From: Hugues FRUCHET @ 2020-06-24  9:24 UTC (permalink / raw)
  To: Benjamin GAIGNARD, mchehab, mcoquelin.stm32, Alexandre TORGUE
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, valentin.schneider, rjw


Hi Benjamin,

test condition in set_min_frequency() to fix, appart from that:
Acked-by: Hugues Fruchet <hugues.fruchet@st.com>

BR,
Hugues.

On 6/10/20 2:24 PM, Benjamin Gaignard wrote:
> Before start streaming set cpufreq minimum frequency requirement.
> The cpufreq governor will adapt the frequencies and we will have
> no latency for handling interrupts.
> The frequency requirement is retrieved from the device-tree node.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> version 6:
> - come back to version 4 and follow Valentin's suggestions about notifier
> - add Valentin's comment about notifier set/unset
> 
> version 5:
> - add a mutex to protect dcmi_irq_notifier_notify()
> - register notifier a probe time
> 
> version 4:
> - simplify irq affinity handling by using only dcmi_irq_notifier_notify()
> 
> version 3:
> - add a cpumask field to track boosted CPUs
> - add irq_affinity_notify callback
> - protect cpumask field with a mutex
> 
>   drivers/media/platform/stm32/stm32-dcmi.c | 138 ++++++++++++++++++++++++++++--
>   1 file changed, 130 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
> index b8931490b83b..382df6e7c864 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -13,10 +13,13 @@
>   
>   #include <linux/clk.h>
>   #include <linux/completion.h>
> +#include <linux/cpufreq.h>
> +#include <linux/cpumask.h>
>   #include <linux/delay.h>
>   #include <linux/dmaengine.h>
>   #include <linux/init.h>
>   #include <linux/interrupt.h>
> +#include <linux/irq.h>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
> @@ -99,6 +102,8 @@ enum state {
>   
>   #define OVERRUN_ERROR_THRESHOLD	3
>   
> +static DEFINE_PER_CPU(struct freq_qos_request, qos_req);
> +
>   struct dcmi_graph_entity {
>   	struct v4l2_async_subdev asd;
>   
> @@ -133,6 +138,7 @@ struct stm32_dcmi {
>   	struct resource			*res;
>   	struct reset_control		*rstc;
>   	int				sequence;
> +	int				irq;
>   	struct list_head		buffers;
>   	struct dcmi_buf			*active;
>   
> @@ -173,6 +179,11 @@ struct stm32_dcmi {
>   	struct media_device		mdev;
>   	struct media_pad		vid_cap_pad;
>   	struct media_pipeline		pipeline;
> +
> +	struct mutex			freq_lock;
> +	u32				min_frequency;
> +	cpumask_var_t			boosted;
> +	struct irq_affinity_notify	notify;
>   };
>   
>   static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
> @@ -722,6 +733,99 @@ static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi)
>   	dcmi_pipeline_s_stream(dcmi, 0);
>   }
>   
> +static void dcmi_get_min_frequency(struct stm32_dcmi *dcmi)
> +{
> +	struct device_node *np = dcmi->mdev.dev->of_node;
> +
> +	dcmi->min_frequency = FREQ_QOS_MIN_DEFAULT_VALUE;
> +
> +	of_property_read_u32(np, "st,stm32-dcmi-min-frequency",
> +			     &dcmi->min_frequency);
> +}
> +
> +static void dcmi_irq_notifier_notify(struct irq_affinity_notify *notify,
> +				     const cpumask_t *mask)
> +{
> +	struct stm32_dcmi *dcmi = container_of(notify,
> +					       struct stm32_dcmi,
> +					       notify);
> +	struct cpufreq_policy *p;
> +	int cpu;
> +
> +	mutex_lock(&dcmi->freq_lock);
> +	/*
> +	 * For all boosted CPUs check if it is still the case
> +	 * if not remove the request
> +	 */
> +	for_each_cpu(cpu, dcmi->boosted) {
> +		if (cpumask_test_cpu(cpu, mask))
> +			continue;
> +
> +		p = cpufreq_cpu_get(cpu);
> +		if (!p)
> +			continue;
> +
> +		freq_qos_remove_request(&per_cpu(qos_req, cpu));
> +		cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
> +
> +		cpufreq_cpu_put(p);
> +	}
> +
> +	/*
> +	 * For CPUs in the mask check if they are boosted if not add
> +	 * a request
> +	 */
> +	for_each_cpu(cpu, mask) {
> +		if (cpumask_test_cpu(cpu, dcmi->boosted))
> +			continue;
> +
> +		p = cpufreq_cpu_get(cpu);
> +		if (!p)
> +			continue;
> +
> +		freq_qos_add_request(&p->constraints, &per_cpu(qos_req, cpu),
> +				     FREQ_QOS_MIN, dcmi->min_frequency);
> +		cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
> +		cpufreq_cpu_put(p);
> +	}
> +
> +	mutex_unlock(&dcmi->freq_lock);
> +}
> +
> +static void dcmi_irq_notifier_release(struct kref *ref)
> +{
> +	/*
> +	 * This is required by affinity notifier. We don't have anything to
> +	 * free here.
> +	 */
> +}
> +
> +static void dcmi_set_min_frequency(struct stm32_dcmi *dcmi, s32 freq)
> +{
> +	struct irq_affinity_notify *notify = &dcmi->notify;
> +	struct cpumask clear;
> +
> +	if (freq) {

if (freq != FREQ_QOS_MIN_DEFAULT_VALUE)

> +		/*
> +		 * Register the notifier before doing any change, so the
> +		 * callback can be queued if an affinity change happens *while*
> +		 * we are requesting the boosts.
> +		 */
> +		irq_set_affinity_notifier(dcmi->irq, notify);
> +		dcmi_irq_notifier_notify(notify,
> +					 irq_get_affinity_mask(dcmi->irq));
> +	} else {
> +		/*
> +		 * Unregister the notifier before clearing the boost requests,
> +		 * as we don't want to boost again if an affinity change happens
> +		 * *while* we are clearing the requests
> +		 */
> +		irq_set_affinity_notifier(dcmi->irq, NULL);
> +		cpumask_clear(&clear);
> +		dcmi_irq_notifier_notify(notify, &clear);
> +	}
> +}
> +
>   static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
>   {
>   	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
> @@ -736,11 +840,13 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
>   		goto err_release_buffers;
>   	}
>   
> +	dcmi_set_min_frequency(dcmi, dcmi->min_frequency);
> +
>   	ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline);
>   	if (ret < 0) {
>   		dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n",
>   			__func__, ret);
> -		goto err_pm_put;
> +		goto err_drop_qos;
>   	}
>   
>   	ret = dcmi_pipeline_start(dcmi);
> @@ -835,7 +941,8 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
>   err_media_pipeline_stop:
>   	media_pipeline_stop(&dcmi->vdev->entity);
>   
> -err_pm_put:
> +err_drop_qos:
> +	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE) >   	pm_runtime_put(dcmi->dev);
>   
>   err_release_buffers:
> @@ -863,6 +970,8 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
>   
>   	media_pipeline_stop(&dcmi->vdev->entity);
>   
> +	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE);
> +
>   	spin_lock_irq(&dcmi->irqlock);
>   
>   	/* Disable interruptions */
> @@ -1834,11 +1943,11 @@ static int dcmi_probe(struct platform_device *pdev)
>   	struct device_node *np = pdev->dev.of_node;
>   	const struct of_device_id *match = NULL;
>   	struct v4l2_fwnode_endpoint ep = { .bus_type = 0 };
> +	struct irq_affinity_notify *notify;
>   	struct stm32_dcmi *dcmi;
>   	struct vb2_queue *q;
>   	struct dma_chan *chan;
>   	struct clk *mclk;
> -	int irq;
>   	int ret = 0;
>   
>   	match = of_match_device(of_match_ptr(stm32_dcmi_of_match), &pdev->dev);
> @@ -1879,9 +1988,9 @@ static int dcmi_probe(struct platform_device *pdev)
>   	dcmi->bus.bus_width = ep.bus.parallel.bus_width;
>   	dcmi->bus.data_shift = ep.bus.parallel.data_shift;
>   
> -	irq = platform_get_irq(pdev, 0);
> -	if (irq <= 0)
> -		return irq ? irq : -ENXIO;
> +	dcmi->irq = platform_get_irq(pdev, 0);
> +	if (dcmi->irq <= 0)
> +		return dcmi->irq ? dcmi->irq : -ENXIO;
>   
>   	dcmi->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	if (!dcmi->res) {
> @@ -1895,11 +2004,12 @@ static int dcmi_probe(struct platform_device *pdev)
>   		return PTR_ERR(dcmi->regs);
>   	}
>   
> -	ret = devm_request_threaded_irq(&pdev->dev, irq, dcmi_irq_callback,
> +	ret = devm_request_threaded_irq(&pdev->dev, dcmi->irq,
> +					dcmi_irq_callback,
>   					dcmi_irq_thread, IRQF_ONESHOT,
>   					dev_name(&pdev->dev), dcmi);
>   	if (ret) {
> -		dev_err(&pdev->dev, "Unable to request irq %d\n", irq);
> +		dev_err(&pdev->dev, "Unable to request irq %d\n", dcmi->irq);
>   		return ret;
>   	}
>   
> @@ -1922,6 +2032,7 @@ static int dcmi_probe(struct platform_device *pdev)
>   	spin_lock_init(&dcmi->irqlock);
>   	mutex_init(&dcmi->lock);
>   	mutex_init(&dcmi->dma_lock);
> +	mutex_init(&dcmi->freq_lock);
>   	init_completion(&dcmi->complete);
>   	INIT_LIST_HEAD(&dcmi->buffers);
>   
> @@ -1930,6 +2041,13 @@ static int dcmi_probe(struct platform_device *pdev)
>   	dcmi->state = STOPPED;
>   	dcmi->dma_chan = chan;
>   
> +	if (!alloc_cpumask_var(&dcmi->boosted, GFP_KERNEL))
> +		return -ENODEV;
> +
> +	notify = &dcmi->notify;
> +	notify->notify = dcmi_irq_notifier_notify;
> +	notify->release = dcmi_irq_notifier_release;
> +
>   	q = &dcmi->queue;
>   
>   	dcmi->v4l2_dev.mdev = &dcmi->mdev;
> @@ -2022,6 +2140,8 @@ static int dcmi_probe(struct platform_device *pdev)
>   
>   	dev_info(&pdev->dev, "Probe done\n");
>   
> +	dcmi_get_min_frequency(dcmi);
> +
>   	platform_set_drvdata(pdev, dcmi);
>   
>   	pm_runtime_enable(&pdev->dev);
> @@ -2049,6 +2169,8 @@ static int dcmi_remove(struct platform_device *pdev)
>   
>   	pm_runtime_disable(&pdev->dev);
>   
> +	free_cpumask_var(dcmi->boosted);
> +
>   	v4l2_async_notifier_unregister(&dcmi->notifier);
>   	v4l2_async_notifier_cleanup(&dcmi->notifier);
>   	media_entity_cleanup(&dcmi->vdev->entity);
> 

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

* Re: [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement
  2020-06-10 12:24 ` [PATCH v6 2/3] media: stm32-dcmi: Set " Benjamin Gaignard
@ 2020-06-10 17:16   ` Valentin Schneider
  2020-06-24  9:24   ` Hugues FRUCHET
  1 sibling, 0 replies; 9+ messages in thread
From: Valentin Schneider @ 2020-06-10 17:16 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue,
	linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, rjw


On 10/06/20 13:24, Benjamin Gaignard wrote:
> Before start streaming set cpufreq minimum frequency requirement.
> The cpufreq governor will adapt the frequencies and we will have
> no latency for handling interrupts.
> The frequency requirement is retrieved from the device-tree node.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

For the cpufreq qos / irq affinity parts:

Reviewed-by: Valentin Schneider <valentin.schneider@arm.com>

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

* [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement
  2020-06-10 12:24 Benjamin Gaignard
@ 2020-06-10 12:24 ` Benjamin Gaignard
  2020-06-10 17:16   ` Valentin Schneider
  2020-06-24  9:24   ` Hugues FRUCHET
  0 siblings, 2 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-06-10 12:24 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	vincent.guittot, valentin.schneider, rjw, Benjamin Gaignard

Before start streaming set cpufreq minimum frequency requirement.
The cpufreq governor will adapt the frequencies and we will have
no latency for handling interrupts.
The frequency requirement is retrieved from the device-tree node.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 6:
- come back to version 4 and follow Valentin's suggestions about notifier
- add Valentin's comment about notifier set/unset

version 5:
- add a mutex to protect dcmi_irq_notifier_notify()
- register notifier a probe time

version 4:
- simplify irq affinity handling by using only dcmi_irq_notifier_notify() 

version 3:
- add a cpumask field to track boosted CPUs
- add irq_affinity_notify callback
- protect cpumask field with a mutex 

 drivers/media/platform/stm32/stm32-dcmi.c | 138 ++++++++++++++++++++++++++++--
 1 file changed, 130 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b8931490b83b..382df6e7c864 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -13,10 +13,13 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/cpufreq.h>
+#include <linux/cpumask.h>
 #include <linux/delay.h>
 #include <linux/dmaengine.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -99,6 +102,8 @@ enum state {
 
 #define OVERRUN_ERROR_THRESHOLD	3
 
+static DEFINE_PER_CPU(struct freq_qos_request, qos_req);
+
 struct dcmi_graph_entity {
 	struct v4l2_async_subdev asd;
 
@@ -133,6 +138,7 @@ struct stm32_dcmi {
 	struct resource			*res;
 	struct reset_control		*rstc;
 	int				sequence;
+	int				irq;
 	struct list_head		buffers;
 	struct dcmi_buf			*active;
 
@@ -173,6 +179,11 @@ struct stm32_dcmi {
 	struct media_device		mdev;
 	struct media_pad		vid_cap_pad;
 	struct media_pipeline		pipeline;
+
+	struct mutex			freq_lock;
+	u32				min_frequency;
+	cpumask_var_t			boosted;
+	struct irq_affinity_notify	notify;
 };
 
 static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
@@ -722,6 +733,99 @@ static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi)
 	dcmi_pipeline_s_stream(dcmi, 0);
 }
 
+static void dcmi_get_min_frequency(struct stm32_dcmi *dcmi)
+{
+	struct device_node *np = dcmi->mdev.dev->of_node;
+
+	dcmi->min_frequency = FREQ_QOS_MIN_DEFAULT_VALUE;
+
+	of_property_read_u32(np, "st,stm32-dcmi-min-frequency",
+			     &dcmi->min_frequency);
+}
+
+static void dcmi_irq_notifier_notify(struct irq_affinity_notify *notify,
+				     const cpumask_t *mask)
+{
+	struct stm32_dcmi *dcmi = container_of(notify,
+					       struct stm32_dcmi,
+					       notify);
+	struct cpufreq_policy *p;
+	int cpu;
+
+	mutex_lock(&dcmi->freq_lock);
+	/*
+	 * For all boosted CPUs check if it is still the case
+	 * if not remove the request
+	 */
+	for_each_cpu(cpu, dcmi->boosted) {
+		if (cpumask_test_cpu(cpu, mask))
+			continue;
+
+		p = cpufreq_cpu_get(cpu);
+		if (!p)
+			continue;
+
+		freq_qos_remove_request(&per_cpu(qos_req, cpu));
+		cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
+
+		cpufreq_cpu_put(p);
+	}
+
+	/*
+	 * For CPUs in the mask check if they are boosted if not add
+	 * a request
+	 */
+	for_each_cpu(cpu, mask) {
+		if (cpumask_test_cpu(cpu, dcmi->boosted))
+			continue;
+
+		p = cpufreq_cpu_get(cpu);
+		if (!p)
+			continue;
+
+		freq_qos_add_request(&p->constraints, &per_cpu(qos_req, cpu),
+				     FREQ_QOS_MIN, dcmi->min_frequency);
+		cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
+		cpufreq_cpu_put(p);
+	}
+
+	mutex_unlock(&dcmi->freq_lock);
+}
+
+static void dcmi_irq_notifier_release(struct kref *ref)
+{
+	/*
+	 * This is required by affinity notifier. We don't have anything to
+	 * free here.
+	 */
+}
+
+static void dcmi_set_min_frequency(struct stm32_dcmi *dcmi, s32 freq)
+{
+	struct irq_affinity_notify *notify = &dcmi->notify;
+	struct cpumask clear;
+
+	if (freq) {
+		/*
+		 * Register the notifier before doing any change, so the
+		 * callback can be queued if an affinity change happens *while*
+		 * we are requesting the boosts.
+		 */
+		irq_set_affinity_notifier(dcmi->irq, notify);
+		dcmi_irq_notifier_notify(notify,
+					 irq_get_affinity_mask(dcmi->irq));
+	} else {
+		/*
+		 * Unregister the notifier before clearing the boost requests,
+		 * as we don't want to boost again if an affinity change happens
+		 * *while* we are clearing the requests
+		 */
+		irq_set_affinity_notifier(dcmi->irq, NULL);
+		cpumask_clear(&clear);
+		dcmi_irq_notifier_notify(notify, &clear);
+	}
+}
+
 static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
 	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
@@ -736,11 +840,13 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 		goto err_release_buffers;
 	}
 
+	dcmi_set_min_frequency(dcmi, dcmi->min_frequency);
+
 	ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline);
 	if (ret < 0) {
 		dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n",
 			__func__, ret);
-		goto err_pm_put;
+		goto err_drop_qos;
 	}
 
 	ret = dcmi_pipeline_start(dcmi);
@@ -835,7 +941,8 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 err_media_pipeline_stop:
 	media_pipeline_stop(&dcmi->vdev->entity);
 
-err_pm_put:
+err_drop_qos:
+	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE);
 	pm_runtime_put(dcmi->dev);
 
 err_release_buffers:
@@ -863,6 +970,8 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
 
 	media_pipeline_stop(&dcmi->vdev->entity);
 
+	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE);
+
 	spin_lock_irq(&dcmi->irqlock);
 
 	/* Disable interruptions */
@@ -1834,11 +1943,11 @@ static int dcmi_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *match = NULL;
 	struct v4l2_fwnode_endpoint ep = { .bus_type = 0 };
+	struct irq_affinity_notify *notify;
 	struct stm32_dcmi *dcmi;
 	struct vb2_queue *q;
 	struct dma_chan *chan;
 	struct clk *mclk;
-	int irq;
 	int ret = 0;
 
 	match = of_match_device(of_match_ptr(stm32_dcmi_of_match), &pdev->dev);
@@ -1879,9 +1988,9 @@ static int dcmi_probe(struct platform_device *pdev)
 	dcmi->bus.bus_width = ep.bus.parallel.bus_width;
 	dcmi->bus.data_shift = ep.bus.parallel.data_shift;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0)
-		return irq ? irq : -ENXIO;
+	dcmi->irq = platform_get_irq(pdev, 0);
+	if (dcmi->irq <= 0)
+		return dcmi->irq ? dcmi->irq : -ENXIO;
 
 	dcmi->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!dcmi->res) {
@@ -1895,11 +2004,12 @@ static int dcmi_probe(struct platform_device *pdev)
 		return PTR_ERR(dcmi->regs);
 	}
 
-	ret = devm_request_threaded_irq(&pdev->dev, irq, dcmi_irq_callback,
+	ret = devm_request_threaded_irq(&pdev->dev, dcmi->irq,
+					dcmi_irq_callback,
 					dcmi_irq_thread, IRQF_ONESHOT,
 					dev_name(&pdev->dev), dcmi);
 	if (ret) {
-		dev_err(&pdev->dev, "Unable to request irq %d\n", irq);
+		dev_err(&pdev->dev, "Unable to request irq %d\n", dcmi->irq);
 		return ret;
 	}
 
@@ -1922,6 +2032,7 @@ static int dcmi_probe(struct platform_device *pdev)
 	spin_lock_init(&dcmi->irqlock);
 	mutex_init(&dcmi->lock);
 	mutex_init(&dcmi->dma_lock);
+	mutex_init(&dcmi->freq_lock);
 	init_completion(&dcmi->complete);
 	INIT_LIST_HEAD(&dcmi->buffers);
 
@@ -1930,6 +2041,13 @@ static int dcmi_probe(struct platform_device *pdev)
 	dcmi->state = STOPPED;
 	dcmi->dma_chan = chan;
 
+	if (!alloc_cpumask_var(&dcmi->boosted, GFP_KERNEL))
+		return -ENODEV;
+
+	notify = &dcmi->notify;
+	notify->notify = dcmi_irq_notifier_notify;
+	notify->release = dcmi_irq_notifier_release;
+
 	q = &dcmi->queue;
 
 	dcmi->v4l2_dev.mdev = &dcmi->mdev;
@@ -2022,6 +2140,8 @@ static int dcmi_probe(struct platform_device *pdev)
 
 	dev_info(&pdev->dev, "Probe done\n");
 
+	dcmi_get_min_frequency(dcmi);
+
 	platform_set_drvdata(pdev, dcmi);
 
 	pm_runtime_enable(&pdev->dev);
@@ -2049,6 +2169,8 @@ static int dcmi_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 
+	free_cpumask_var(dcmi->boosted);
+
 	v4l2_async_notifier_unregister(&dcmi->notifier);
 	v4l2_async_notifier_cleanup(&dcmi->notifier);
 	media_entity_cleanup(&dcmi->vdev->entity);
-- 
2.15.0


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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 12:59 [PATCH v6 0/3] DCMI set minimum cpufreq requirement Benjamin Gaignard
2020-07-01 12:59 ` [PATCH v6 1/3] dt-bindings: media: stm32-dcmi: Add DCMI min frequency property Benjamin Gaignard
2020-07-01 12:59 ` [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement Benjamin Gaignard
2020-07-01 12:59 ` [PATCH v6 3/3] ARM: dts: stm32: Set DCMI frequency requirement for stm32mp15x Benjamin Gaignard
2020-07-01 13:02 ` [PATCH v6 0/3] DCMI set minimum cpufreq requirement Benjamin GAIGNARD
  -- strict thread matches above, loose matches on Subject: below --
2020-06-10 12:24 Benjamin Gaignard
2020-06-10 12:24 ` [PATCH v6 2/3] media: stm32-dcmi: Set " Benjamin Gaignard
2020-06-10 17:16   ` Valentin Schneider
2020-06-24  9:24   ` Hugues FRUCHET
2020-06-24 11:27     ` Benjamin GAIGNARD

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git