linux-kernel - Re: 回复: 回复: [PATCH] staging: media: starfive: Add multiple resolution support

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: <20240812103357.GB18729@pendragon.ideasonboard.com>
Date: Mon, 12 Aug 2024 13:33:57 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Changhuang Liang <changhuang.liang@...rfivetech.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Hans Verkuil <hverkuil-cisco@...all.nl>,
	Jack Zhu <jack.zhu@...rfivetech.com>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>
Subject: Re: 回复: 回复: [PATCH]
 staging: media: starfive: Add multiple resolution support

Hi Changhuang,

On Mon, Aug 12, 2024 at 09:43:47AM +0000, Changhuang Liang wrote:
> > On Fri, Aug 09, 2024 at 12:12:01PM +0000, Changhuang Liang wrote:
> > > > On Fri, Apr 19, 2024 at 01:19:55AM -0700, Changhuang Liang wrote:
> > > > > Add multiple resolution support for video "capture_raw" device.
> > > > > Otherwise it will capture the wrong image data if the width is not 1920.
> > > > >
> > > > > Fixes: e080f339c80a ("media: staging: media: starfive: camss: Add
> > > > > capture driver")
> > > > >
> > > > > Signed-off-by: Changhuang Liang
> > > > > <changhuang.liang@...rfivetech.com>
> > > > > ---
> > > > >  drivers/staging/media/starfive/camss/stf-capture.c | 5 ++++-
> > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/staging/media/starfive/camss/stf-capture.c
> > > > > b/drivers/staging/media/starfive/camss/stf-capture.c
> > > > > index ec5169e7b391..9e853ff2596a 100644
> > > > > --- a/drivers/staging/media/starfive/camss/stf-capture.c
> > > > > +++ b/drivers/staging/media/starfive/camss/stf-capture.c
> > > > > @@ -177,9 +177,12 @@ static void stf_channel_set(struct
> > > > > stfcamss_video
> > > > > *video)  {
> > > > >  	struct stf_capture *cap = to_stf_capture(video);
> > > > >  	struct stfcamss *stfcamss = cap->video.stfcamss;
> > > > > +	struct v4l2_pix_format *pix;
> > > >
> > > > This variable can be const as you don't modify the format.
> > > >
> > > > >  	u32 val;
> > > > >
> > > > >  	if (cap->type == STF_CAPTURE_RAW) {
> > > > > +		pix = &video->active_fmt.fmt.pix;
> > > >
> > > > And it can be declared and initialized here:
> > > >
> > > > 		const struct v4l2_pix_format *pix = &video->active_fmt.fmt.pix;
> > > >
> > > > > +
> > > > >  		val = stf_syscon_reg_read(stfcamss, VIN_CHANNEL_SEL_EN);
> > > > >  		val &= ~U0_VIN_CHANNEL_SEL_MASK;
> > > > >  		val |= CHANNEL(0);
> > > > > @@ -193,7 +196,7 @@ static void stf_channel_set(struct stfcamss_video *video)
> > > > >  		val |= PIXEL_HEIGH_BIT_SEL(0);
> > > > >
> > > > >  		val &= ~U0_VIN_PIX_CNT_END_MASK;
> > > > > -		val |= PIX_CNT_END(IMAGE_MAX_WIDTH / 4 - 1);
> > > > > +		val |= PIX_CNT_END(pix->width / 4 - 1);
> > > >
> > > > Is there no need to consider the image height as well ? How does the
> > > > driver prevent buffer overflows if the sensor sends more data than expected ?
> > >
> > > Our hardware will confirm a frame of data through vblank signal, so
> > > there is no image height configuration.
> > 
> > What happens if the system expects, for instance, a 1920x1080 RAW8 image,
> > and allocates a buffer of of 1920x1080 bytes, but the sensor outputs more
> > lines ? Does the camera hardware in the SoC offer an option to prevent buffer
> > overruns ?
> 
> The hardware can confirm the image height by using the VSYNC signal.
> 
> Image will transfer when VSYNC is high.
> 
> VSYNC time = (width + h_blank) * height;

What I'm trying to understand is what happens if the ISP is configured
for 1080 lines, but the camera sensor sends more than 1080 lines (the
VSYNC signal is active for more than 1080 lines). Where in the driver is
the hardware configure with the 1080 lines limit to avoid buffer
overflows ?

> > > > >
> > > > >  		stf_syscon_reg_write(stfcamss, VIN_INRT_PIX_CFG, val);
> > > > >  	} else if (cap->type == STF_CAPTURE_YUV) {

-- 
Regards,

Laurent Pinchart