Re: [PATCH 05/17] media: v4l2-mem2mem: support running multiple jobs in parallel

["Sven Püschel" <s.pueschel@pengutronix.de>]

Hi,

On 7/10/26 11:18 PM, Nicolas Dufresne wrote:
> Hi,
>
> Le samedi 06 juin 2026 à 00:06 +0200, Sven Püschel a écrit :
>> Add support for running multiple jobs in parallel for SoCs containing
>> multiple identical devices. An example is the Rockchip RK3588 SoC,
>> which contains two identical RGA3 devices. Therefore it is desirable to
>> have the kernel schedule the work across all available devices and only
>> expose one video device to the userspace.
>>
>> Previously the curr_ctx member of a v4l2_m2m_dev was used to track the
>> currently running context. But the currently running context will always
>> be at the top of the job_queue. As the TRANS_RUNNING flag can be used to
>> check if the queue head is already running, the curr_ctx member can be
>> completely dropped
>>
>> To avoid queueing too many parallel jobs, the
>> v4l2_m2m_set_max_parallel_jobs method is added. It allows a driver
>> to set the number of parallel jobs and avoids calling device_run when
>> the given number of jobs is already running. This is set to 1 by default
>> to prevent parallel job runs. Drivers with the need and support for
>> scheduling jobs can adjust this value accordingly.
>>
>> Note that this change doesn't allow a context to be used multiple times
>> in parallel. So a single stream won't be able to utilize multiple devices
>> at once, but N streams can utilize up to N devices. This is caused by the
>> fact that a context is not added multiple times to the job_list and also
>> holds the job_flags to distinguish if it's currently running.
> I do prefer this over Detlev proposal, so let's move toward this. Would be it
> cleaner though to first remove curr_ctx and then add
> max_parallel_jobs ?

Nice idea. I could move the max_parallel_jobs variable and the new 
function to a new small commit.

I could also move the whole looping and counting of running jobs over 
the jobs to the new commit. But this would cause replacing the curr_ctx 
variable with a `list_first_entry(...)->job_flags & TRANS_RUNNING` and 
drop it in the commit afterwards to replace it with loops.

While the commits would look a bit nicer in the latter example (as the 
removal of curr_ctx wouldn't also prepare for parallel jobs), I think 
the addition and direct removal style is frowned upon and therefore I 
tend towards the first option. On the other side I'm unsure if a 10 line 
patch to just add max_parallel_jobs variable and function with 
everything done in a (removal) patch provides benefit or harms to get to 
the related changes.

>> @@ -252,13 +266,11 @@ EXPORT_SYMBOL(v4l2_m2m_get_curr_priv);
>>   static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev)
>>   {
>>   	unsigned long flags;
>> +	struct v4l2_m2m_ctx *ctx;
>> +	struct v4l2_m2m_ctx *chosen_ctx = NULL;
>> +	u32 running_jobs = 0;
>>   
>>   	spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
>> -	if (NULL != m2m_dev->curr_ctx) {
>> -		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
>> -		dprintk("Another instance is running, won't run now\n");
>> -		return;
>> -	}
>>   
>>   	if (list_empty(&m2m_dev->job_queue)) {
>>   		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
>> @@ -272,13 +284,30 @@ static void v4l2_m2m_try_run(struct v4l2_m2m_dev
>> *m2m_dev)
>>   		return;
>>   	}
>>   
>> -	m2m_dev->curr_ctx = list_first_entry(&m2m_dev->job_queue,
>> -				   struct v4l2_m2m_ctx, queue);
>> -	m2m_dev->curr_ctx->job_flags |= TRANS_RUNNING;
>> +	list_for_each_entry(ctx, &m2m_dev->job_queue, queue) {
>> +		if (!(ctx->job_flags & TRANS_RUNNING)) {
>> +			chosen_ctx = ctx;
>> +			break;
>> +		}
>> +
>> +		running_jobs++;
>> +	}
>> +	if (running_jobs >= m2m_dev->max_parallel_jobs) {
>> +		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
>> +		dprintk("Maximum number of parallel jobs reached\n");
>> +		return;
>> +	}
>> +	if (!chosen_ctx) {
>> +		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
>> +		dprintk("All jobs already running\n");
>> +		return;
>> +	}
>> +
>> +	chosen_ctx->job_flags |= TRANS_RUNNING;
>>   	spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);

This is the most prominent example on how to split it up. E.g. either 
keep everything in the curr_ctx removal commit (and just use 1 for 
max_parallel_jobs) or use list_first_head to check if we have a curr_ctx 
and drop it afterwards for the counting logic.


Sincerely
     Sven