| Message ID | 1316530612-23075-5-git-send-email-deepthy.ravi@ti.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1R61nR-0003Xd-Gr; Tue, 20 Sep 2011 16:59:41 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-2) with esmtp id 1R61nQ-0000c5-Ic; Tue, 20 Sep 2011 16:59:41 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751833Ab1ITO5n (ORCPT <rfc822;patchwork@linuxtv.org> + 5 others); Tue, 20 Sep 2011 10:57:43 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:44146 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751295Ab1ITO5k (ORCPT <rfc822;linux-media@vger.kernel.org>); Tue, 20 Sep 2011 10:57:40 -0400 Received: from dbdp20.itg.ti.com ([172.24.170.38]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id p8KEvB8F008087 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 20 Sep 2011 09:57:14 -0500 Received: from dbde71.ent.ti.com (localhost [127.0.0.1]) by dbdp20.itg.ti.com (8.13.8/8.13.8) with ESMTP id p8KEv56P026758; Tue, 20 Sep 2011 20:27:11 +0530 (IST) Received: from dbdp31.itg.ti.com (172.24.170.98) by DBDE71.ent.ti.com (172.24.170.149) with Microsoft SMTP Server id 8.3.106.1; Tue, 20 Sep 2011 20:27:07 +0530 Received: from localhost.localdomain (dbdp20.itg.ti.com [172.24.170.38]) by dbdp31.itg.ti.com (8.13.8/8.13.8) with ESMTP id p8KEurLn024824; Tue, 20 Sep 2011 20:27:02 +0530 (IST) From: Deepthy Ravi <deepthy.ravi@ti.com> To: <laurent.pinchart@ideasonboard.com>, <mchehab@infradead.org>, <tony@atomide.com>, <hvaibhav@ti.com>, <linux-media@vger.kernel.org>, <linux@arm.linux.org.uk>, <linux-arm-kernel@lists.infradead.org>, <kyungmin.park@samsung.com>, <hverkuil@xs4all.nl>, <m.szyprowski@samsung.com>, <g.liakhovetski@gmx.de>, <santosh.shilimkar@ti.com>, <khilman@deeprootsystems.com>, <david.woodhouse@intel.com>, <akpm@linux-foundation.org>, <linux-kernel@vger.kernel.org> CC: <linux-omap@vger.kernel.org>, Deepthy Ravi <deepthy.ravi@ti.com> Subject: [PATCH 4/5] ispccdc: Configure CCDC_SYN_MODE register for UYVY8_2X8 and YUYV8_2X8 formats Date: Tue, 20 Sep 2011 20:26:51 +0530 Message-ID: <1316530612-23075-5-git-send-email-deepthy.ravi@ti.com> X-Mailer: git-send-email 1.6.2.4 In-Reply-To: <1316530612-23075-1-git-send-email-deepthy.ravi@ti.com> References: <1316530612-23075-1-git-send-email-deepthy.ravi@ti.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2011.9.20.145116 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, MSGID_ADDED_BY_MTA 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1700_1799 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, __ANY_URI 0, __CP_MEDIA_BODY 0, __CP_URI_IN_BODY 0, __CT 0, __CT_TEXT_PLAIN 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_RCPTS_TO_X5 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS ' X-LSpam-Score: 1.0 (+) X-LSpam-Report: No, score=1.0 required=5.0 tests=BAYES_00=-1.9, KB_DATE_CONTAINS_TAB=2.751, RCVD_IN_DNSWL_MED=-2.3, TAB_IN_FROM=2.494 autolearn=no |
Commit Message
Deepthy Ravi
Sept. 20, 2011, 2:56 p.m. UTC
Configure INPMOD and PACK8 fileds of CCDC_SYN_MODE
register for UYVY8_2X8 and YUYV8_2X8 formats.
Signed-off-by: Deepthy Ravi <deepthy.ravi@ti.com>
---
drivers/media/video/omap3isp/ispccdc.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
Comments
Hi Deepthy, Thanks for the patch. 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@ti.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. > syn_mode |= ISPCCDC_SYN_MODE_PACK8; > else > syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;
Hi Laurent, > ________________________________________ > From: Laurent Pinchart [laurent.pinchart@ideasonboard.com] > Sent: Wednesday, September 21, 2011 4:56 AM > To: Ravi, Deepthy > Cc: mchehab@infradead.org; tony@atomide.com; Hiremath, Vaibhav; > linux-media@vger.kernel.org; linux@arm.linux.org.uk; > linux-arm-kernel@lists.infradead.org; kyungmin.park@samsung.com; > hverkuil@xs4all.nl; m.szyprowski@samsung.com; g.liakhovetski@gmx.de; > Shilimkar, Santosh; khilman@deeprootsystems.com; david.woodhouse@intel.com; > akpm@linux-foundation.org; linux-kernel@vger.kernel.org; > linux-omap@vger.kernel.org; Sakari Ailus > Subject: Re: [PATCH 4/5] ispccdc: Configure CCDC_SYN_MODE register for > UYVY8_2X8 and YUYV8_2X8 formats > > Hi Deepthy, > > Thanks for the patch. > > 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@ti.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. > [Deepthy Ravi] 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. >> syn_mode |= ISPCCDC_SYN_MODE_PACK8; >> else >> syn_mode &= ~ISPCCDC_SYN_MODE_PACK8; > > -- > Regards, > > Laurent Pinchart >
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@ti.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;
> ________________________________________ > From: Laurent Pinchart [laurent.pinchart@ideasonboard.com] > Sent: Wednesday, September 21, 2011 2:36 PM > To: Ravi, Deepthy > Cc: mchehab@infradead.org; tony@atomide.com; Hiremath, Vaibhav; linux-media@vger.kernel.org; linux@arm.linux.org.uk; linux-arm-kernel@lists.infradead.org; kyungmin.park@samsung.com; hverkuil@xs4all.nl; m.szyprowski@samsung.com; g.liakhovetski@gmx.de; Shilimkar, Santosh; khilman@deeprootsystems.com; david.woodhouse@intel.com; akpm@linux-foundation.org; linux-kernel@vger.kernel.org; linux-omap@vger.kernel.org; Sakari Ailus > 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@ti.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. > [Deepthy Ravi] But for streaming to start, the formats on both ends of the link should match. I believe setting different formats at sensor output and ccdc input will give a broken pipe error. Is my understanding correct ? If so, how do you propose to handle the situation ? > 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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 21, 2011 at 11:06:41AM +0200, Laurent Pinchart wrote: > Hi Deepthy, Hi, (Dropped most of people and lists from cc. I don't think Andrew Morton, for example, has immediate interest towards this topic. Feel free to prome me wrong.) > 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@ti.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 ? Are there any complications from the CCDC also listing the 2X8 formats as supported in its sink pad? I'd rather support them since doing it the other way might be somewhat confusing to the users. This would also mean media-ctl users would have to set both sink and source formats rather than just the source format when configuring the format for the link. > > >> 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-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sakari, On Sunday 25 September 2011 13:17:17 Sakari Ailus wrote: > On Wed, Sep 21, 2011 at 11:06:41AM +0200, Laurent Pinchart wrote: > > 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@ti.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 ? > > Are there any complications from the CCDC also listing the 2X8 formats as > supported in its sink pad? No, and the 2X8 formats need to be supported at the CCDC input, as BT.656 mode is not compatible with the bridge. > I'd rather support them since doing it the other way might be somewhat > confusing to the users. It can be confusing indeed, but it reflects reality :-) It's also how we currently handle the lane shifter. If we incorporate the bridge inside the CCDC, only YUV 2X8 formats will be supported at the CCDC input. Whether to enable the bridge will then be a platform decision and hardcoded in board code instead of being configurable by the user. Couldn't that be an issue ? > This would also mean media-ctl users would have to set both sink and source > formats rather than just the source format when configuring the format for > the link. > > > > >> syn_mode |= ISPCCDC_SYN_MODE_PACK8; > > > >> > > > >> else > > > >> > > > >> syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;
Hi Deepthy, On Friday 23 September 2011 13:53:39 Ravi, Deepthy wrote: > On Wednesday, September 21, 2011 2:36 PM Laurent Pinchart wrote: > > 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@ti.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. > > But for streaming to start, the formats on both ends of the link should > match. I believe setting different formats at sensor output and ccdc input > will give a broken pipe error. Is my understanding correct ? If so, how do > you propose to handle the situation ? We already handle that situation for the lane shifter in ispvideo.c. The code could be extended.
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)) syn_mode |= ISPCCDC_SYN_MODE_PACK8; else syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;