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

[Detlev Casanova]

Hi,

On Wednesday, 15 July 2026 05:44:01 EDT Sven Püschel wrote:
> 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.

I rebased the rkvdec multicore support on this, and also used the same 
components approcah for cores probing ([1]).

All works well, I'll make sure to rebase on these changes when they happen.

Thanks for your work !

[1]: https://lore.kernel.org/all/20260810-rkvdec-multicore-v2-0-986f89d22cdc@xxxxxxxxxxxxx/

> >> @@ -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
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-rockchip