linux-kernel - Re: [PATCH 4/5] ispccdc: Configure CCDC_SYN_MODE register for UYVY8_2X8 and YUYV8

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: <201109211106.41677.laurent.pinchart@ideasonboard.com>
Date:	Wed, 21 Sep 2011 11:06:41 +0200
From:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:	"Ravi, Deepthy" <deepthy.ravi@...com>
Cc:	"mchehab@...radead.org" <mchehab@...radead.org>,
	"tony@...mide.com" <tony@...mide.com>,
	"Hiremath, Vaibhav" <hvaibhav@...com>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"kyungmin.park@...sung.com" <kyungmin.park@...sung.com>,
	"hverkuil@...all.nl" <hverkuil@...all.nl>,
	"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
	"g.liakhovetski@....de" <g.liakhovetski@....de>,
	"Shilimkar, Santosh" <santosh.shilimkar@...com>,
	"khilman@...prootsystems.com" <khilman@...prootsystems.com>,
	"david.woodhouse@...el.com" <david.woodhouse@...el.com>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	Sakari Ailus <sakari.ailus@...well.research.nokia.com>
Subject: Re: [PATCH 4/5] ispccdc: Configure CCDC_SYN_MODE register for UYVY8_2X8 and YUYV8_2X8 formats

Hi Deepthy,

On Wednesday 21 September 2011 07:32:44 Ravi, Deepthy wrote:
> On Wednesday, September 21, 2011 4:56 AM Laurent Pinchart wrote:
> > On Tuesday 20 September 2011 16:56:51 Deepthy Ravi wrote:
> >> Configure INPMOD and PACK8 fileds of CCDC_SYN_MODE
> >> register for UYVY8_2X8 and YUYV8_2X8 formats.
> >> 
> >> Signed-off-by: Deepthy Ravi <deepthy.ravi@...com>
> >> ---
> >> 
> >>  drivers/media/video/omap3isp/ispccdc.c |   11 ++++++++---
> >>  1 files changed, 8 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/media/video/omap3isp/ispccdc.c
> >> b/drivers/media/video/omap3isp/ispccdc.c index 418ba65..1dcf180 100644
> >> --- a/drivers/media/video/omap3isp/ispccdc.c
> >> +++ b/drivers/media/video/omap3isp/ispccdc.c
> >> @@ -985,8 +985,12 @@ static void ccdc_config_sync_if(struct
> >> isp_ccdc_device
> >> *ccdc,
> >> 
> >>       syn_mode &= ~ISPCCDC_SYN_MODE_INPMOD_MASK;
> >>       if (format->code == V4L2_MBUS_FMT_YUYV8_2X8 ||
> >> 
> >> -         format->code == V4L2_MBUS_FMT_UYVY8_2X8)
> >> -             syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR8;
> >> +         format->code == V4L2_MBUS_FMT_UYVY8_2X8){
> >> +             if (pdata && pdata->bt656)
> >> +                     syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR8;
> >> +             else
> >> +                     syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR16;
> >> +     }
> >> 
> >>       else if (format->code == V4L2_MBUS_FMT_YUYV8_1X16 ||
> >>       
> >>                format->code == V4L2_MBUS_FMT_UYVY8_1X16)
> >>               
> >>               syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR16;
> >> 
> >> @@ -1172,7 +1176,8 @@ static void ccdc_configure(struct isp_ccdc_device
> >> *ccdc) syn_mode &= ~ISPCCDC_SYN_MODE_SDR2RSZ;
> >> 
> >>       /* Use PACK8 mode for 1byte per pixel formats. */
> >> 
> >> -     if (omap3isp_video_format_info(format->code)->width <= 8)
> >> +     if ((omap3isp_video_format_info(format->code)->width <= 8) &&
> >> +                     (omap3isp_video_format_info(format->code)->bpp <=
> >> 8))
> > 
> > I'm not sure to follow you. This will clear the PACK8 bit for the
> > YUYV8_2X8 formats. Those formats are 8 bits wide, shouldn't PACK8 be set
> > to store samples on 8 bits instead of 16 bits ?
> > 
> > Is this patch intended to support YUYV8_2X8 sensors in non BT.656 mode
> > with the bridge enabled ? In that case, what would you think about setting
> > the CCDC input format to YUYV8_1X16 instead ? This would better reflect
> > the reality, as the bridge converts YUYV8_2X8 to YUYV8_1X16, and the CCDC
> > is then fed with YUYV8_1X16.
> 
> Yes this is intended for  YUYV8_2X8 sensors in non BT.656 with 8 to 16 bit
> bridge enabled. So the data has to be stored as 16 bits per sample. Thats
> why PACK8 is cleared . I am not sure about using YUYV8_1X16.

My original idea when I wrote the YV support patches was to implement this use 
case with YUYV8_2X8 at the sensor output and YUYV8_1X16 at the CCDC input. The 
ISP driver could then enable the bridge automatically. I'm not sure if that's 
the best solution though, it might be confusing for the users. What I would 
like to keep, however, is the idea of enabling the bridge automatically.

Sakari, any opinion on this ?

> >>               syn_mode |= ISPCCDC_SYN_MODE_PACK8;
> >>       else
> >>               syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/