On 10/07/2024 15:07, Jacopo Mondi wrote: > Hi Changhuang > > + Hans for one question on the vb2 queue mem_ops to use. > > On Tue, Jul 09, 2024 at 01:38:21AM GMT, Changhuang Liang wrote: >> Add ISP params video device to write ISP parameters for 3A. >> >> Signed-off-by: Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx> >> --- >> drivers/staging/media/starfive/camss/Makefile | 2 + >> .../staging/media/starfive/camss/stf-camss.c | 23 +- >> .../staging/media/starfive/camss/stf-camss.h | 3 + >> .../media/starfive/camss/stf-isp-params.c | 240 ++++++++++++++++++ >> .../staging/media/starfive/camss/stf-isp.h | 4 + >> .../staging/media/starfive/camss/stf-output.c | 83 ++++++ >> .../staging/media/starfive/camss/stf-output.h | 22 ++ >> 7 files changed, 376 insertions(+), 1 deletion(-) >> create mode 100644 drivers/staging/media/starfive/camss/stf-isp-params.c >> create mode 100644 drivers/staging/media/starfive/camss/stf-output.c >> create mode 100644 drivers/staging/media/starfive/camss/stf-output.h >> <snip> >> +int stf_isp_params_register(struct stfcamss_video *video, >> + struct v4l2_device *v4l2_dev, >> + const char *name) >> +{ >> + struct video_device *vdev = &video->vdev; >> + struct vb2_queue *q; >> + struct media_pad *pad = &video->pad; >> + int ret; >> + >> + mutex_init(&video->q_lock); >> + mutex_init(&video->lock); > > are two mutexes required for the vb2 queue and the video node ? I see, > in example, rkisp1-params.c uses a single one > >> + >> + q = &video->vb2_q; >> + q->drv_priv = video; >> + q->mem_ops = &vb2_dma_contig_memops; > > Now, I might be wrong, but unless you need to allocate memory from a > DMA-capable area, you shouldn't need to use vb2_dma_contig_memops. > > Looking at the next patches you apply configuration parameters to the > ISP by inspecting the user supplied parameters one by one, not by > transfering the whole parameters buffer to a memory area. Hans what do > you think ? Yes, if the data is not DMAed to the hardware, then use vb2_vmalloc_memops. Regards, Hans