On 25/05/2023 10:59, Sergey Senozhatsky wrote: > On (23/05/25 09:22), Hans Verkuil wrote: >>>> Since these are m2m devices, I think this should set vfh->m2m_ctx->q_lock >>>> instead. >>>> >>>> The vb2_queue is per filehandle for such devices, so by just setting >>>> vdev->lock you will have all vb2_queues use the same mutex. >>>> >>>> Instead the struct v4l2_m2m_ctx q_lock pointer, if set, will use that >>>> mutex for all vb2 operations. >>>> >>>> I think you can set it to the 'lock' mutex in struct venus_inst. >>> >>> IIUC, the suggestion is to use the 'lock' in struct venus_inst while >>> initializing the queue. This might lead to deadlock as the same lock is used >>> during vb2 operations in driver. Might be introducing a new lock for this >>> purpose in struct venus_inst would do, unless we are trying to serialize at >>> video device (or core) context. >> >> For the record, I have not analyzed how that lock is used in the driver, >> so if a new mutex has to be added to venus_inst rather than reusing the >> existing one, then that's fine by me. >> >> But it should be a instance-specific mutex, not one at the device level. > > Thanks for your help Hans. I added a per-instance queue mutex [1], so if > no one sees any problems with it then I can send a formal patch later on. > > [1] https://lore.kernel.org/linux-media/20230525005312.GC30543@xxxxxxxxxx Note that I would like to see 'Tested-by' and 'Reviewed/Acked-by' tags before merging, preferably from the driver maintainer(s). Regards, Hans
