Thread (8 messages) 8 messages, 4 authors, 2020-06-03

Re: [PATCH] media: stm32-dcmi: Set minimum cpufreq requirement

From: Benjamin GAIGNARD <hidden>
Date: 2020-06-03 07:34:30
Also in: linux-media, lkml


On 6/2/20 3:35 PM, Valentin Schneider wrote:
On 02/06/20 12:37, Benjamin GAIGNARD wrote:
quoted
On 6/2/20 11:31 AM, Valentin Schneider wrote:
quoted
quoted
@@ -99,6 +100,8 @@ enum state {

   #define OVERRUN_ERROR_THRESHOLD	3

+#define DCMI_MIN_FREQ	650000 /* in KHz */
+
This assumes the handling part is guaranteed to always run on the same CPU
with the same performance profile (regardless of the platform). If that's
not guaranteed, it feels like you'd want this to be configurable in some
way.
Yes I could add a st,stm32-dcmi-min-frequency (in KHz) parameter the
device tree node.
Something like that - I'm not sure how well this fits with the DT
landscape, as you could argue it isn't really a description of the
hardware, more of a description of the performance expectations of the
software. I won't really argue here.
quoted
quoted
quoted
   struct dcmi_graph_entity {
        struct v4l2_async_subdev asd;
[...]
quoted
@@ -2020,6 +2042,8 @@ static int dcmi_probe(struct platform_device *pdev)
                goto err_cleanup;
        }

+	dcmi->policy = cpufreq_cpu_get(0);
+
Ideally you'd want to fetch the policy of the CPU your IRQ (and handling
thread) is affined to; The only compatible DTS I found describes a single
A7, which is somewhat limited in the affinity area...
If I move this code just before start streaming and use get_cpu(), would
it works ?
AFAIA streaming_start() is not necessarily executing on the same CPU as the
one that will handle the interrupt. I was thinking you could use the IRQ's
effective affinity as a hint of which CPU(s) to boost, i.e. something like:

---
     struct cpumask_var_t visited;
     struct irq_data *d = irq_get_irq_data(irq);

     err = alloc_cpumask_var(visited, GFP_KERNEL);
     /* ... */
     for_each_cpu(cpu, irq_data_get_effective_affinity_mask(d)) {
             /* check if not already spanned */
             if (cpumask_test_cpu(cpu, visited))
                     continue;

             policy = cpufreq_cpu_get(cpu);
             cpumask_or(visited, visited, policy->cpus);
             /* do the boost for that policy here */
             /* ... */
             cpufreq_cpu_put(policy);
     }
---

That of course falls apart when hotplug gets involved, and the effective
affinity changes... There's irq_set_affinity_notifier() out there, but it
seems it's only about the affinity, not the effective_affinity, I'm not
sure how valid it would be to query the effective_affinity in that
notifier.
If I wait to be in the irq it will be too late so I think I will do a 
loop over all possible CPUs
before start the streaming to change the policies.
quoted
Benjamin
quoted
quoted
        dev_info(&pdev->dev, "Probe done\n");

        platform_set_drvdata(pdev, dcmi);
@@ -2049,6 +2073,9 @@ static int dcmi_remove(struct platform_device *pdev)

        pm_runtime_disable(&pdev->dev);

+	if (dcmi->policy)
+		cpufreq_cpu_put(dcmi->policy);
+
        v4l2_async_notifier_unregister(&dcmi->notifier);
        v4l2_async_notifier_cleanup(&dcmi->notifier);
        media_entity_cleanup(&dcmi->vdev->entity);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help