linux-kernel - Re: [PATCH v2 2/7] media: chips-media: wave5: Improve performance of decoder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1318bd68f2d60b9d44cb22bd90b92399311f0b00.camel@collabora.com>
Date: Wed, 04 Jun 2025 09:47:34 -0400
From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
To: "jackson.lee" <jackson.lee@...psnmedia.com>, "mchehab@...nel.org"	
 <mchehab@...nel.org>, "hverkuil-cisco@...all.nl"
 <hverkuil-cisco@...all.nl>,  "bob.beckett@...labora.com"	
 <bob.beckett@...labora.com>
Cc: "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>, 
 "linux-kernel@...r.kernel.org"	 <linux-kernel@...r.kernel.org>,
 "lafley.kim" <lafley.kim@...psnmedia.com>,  "b-brnich@...com"	
 <b-brnich@...com>, "hverkuil@...all.nl" <hverkuil@...all.nl>, Nas Chung	
 <nas.chung@...psnmedia.com>
Subject: Re: [PATCH v2 2/7] media: chips-media: wave5: Improve performance
 of decoder

Le mercredi 04 juin 2025 à 04:09 +0000, jackson.lee a écrit :
> > Running in loop anything is never the right approach. The device_run()
> > should be run when a useful event occur and filtered by the job_ready()
> > ops. I believe I'm proposing some hint how to solve this design issue. The
> > issue is quite clear with the follow up patch trying to reduce the CPU
> > usage due to spinning.
> 
> 
> 
> Thanks for your feedback.
> But there is one thing to say to you.
> After receiving EOS from application, we have to periodically run the device_run
> to send the DEC_PIC command so that VPU can trigger interrupt  until getting all 
> decoded frames and EOS from Get_Result command.
> So even if we sent EOS to VPU once, we should run the device_run function continuously,
> the above code was added. If the job_ready returns false to prevent running the
> device_run after sending EOS to VPU, then GStreamer pipeline will not be terminated 
> normally because of not receiving all decoded frames.

This, in my opinion, boils down to a small flaw, either in the firmware or the driver.
This is why there was this code:

-
-	/*
-	 * During a resolution change and while draining, the firmware may flush
-	 * the reorder queue regardless of having a matching decoding operation
-	 * pending. Only terminate the job if there are no more IRQ coming.
-	 */
-	wave5_vpu_dec_give_command(inst, DEC_GET_QUEUE_STATUS, &q_status);
-	if (q_status.report_queue_count == 0 &&
-	    (q_status.instance_queue_count == 0 || dec_info.sequence_changed)) {
-		dev_dbg(inst->dev->dev, "%s: finishing job.\n", __func__);
-		pm_runtime_mark_last_busy(inst->dev->dev);
-		pm_runtime_put_autosuspend(inst->dev->dev);
-		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
-	}

Which you removed in this patch, as it makes it impossible to utilise the HW queues.
In the specific case you described, if my memory is right, the CMD_STOP (EOS in your
terms) comes in race with the queue being consumed, leading to possibly having no
event to figure-out when we are done with that sequenece.

V4L2 M2M is all event based, and v4l2_m2m_job_finish() is one of those. But in the
new implementation, this event no longer correlate with the HW being idle.
This is fine, don't read me wrong. It now matches the driver being ready to try and
queue more work.

So my question is, is there a way to know, at CMD_STOP call, that the HW
has gone idle, and that no more events will allow handling the EOS case?

Nicolas