linux-kernel - Re: [PATCH v5 11/14] staging: media: starfive: Add ISP params video device

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: <217374f8-bad7-4dab-8a39-2226ea974d7d@xs4all.nl>
Date: Mon, 15 Jul 2024 09:12:05 +0200
From: Hans Verkuil <hverkuil@...all.nl>
To: Jacopo Mondi <jacopo.mondi@...asonboard.com>,
 Changhuang Liang <changhuang.liang@...rfivetech.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>,
 Hans Verkuil <hverkuil-cisco@...all.nl>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Jean-Michel Hautbois <jeanmichel.hautbois@...asonboard.com>,
 Benjamin Gaignard <benjamin.gaignard@...labora.com>,
 Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>,
 Mingjia Zhang <mingjia.zhang@...iatek.com>,
 Jack Zhu <jack.zhu@...rfivetech.com>,
 Keith Zhao <keith.zhao@...rfivetech.com>, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-staging@...ts.linux.dev
Subject: Re: [PATCH v5 11/14] staging: media: starfive: Add ISP params video
 device

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@...rfivetech.com>
>> ---
>>  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