On 6/2/20 3:35 PM, Valentin Schneider wrote:
> On 02/06/20 12:37, Benjamin GAIGNARD wrote:
>> On 6/2/20 11:31 AM, Valentin Schneider wrote:
>>>> @@ -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.
>
>>>> struct dcmi_graph_entity {
>>>> struct v4l2_async_subdev asd;
>>>>
>>> [...]
>>>> @@ -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.
>
>> Benjamin
>>>> 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);
