From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from perceval.ideasonboard.com ([95.142.166.194]:41732 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754824Ab2GQK4U (ORCPT ); Tue, 17 Jul 2012 06:56:20 -0400 From: Laurent Pinchart To: "Hadli, Manjunath" Cc: dlos , LMML , Sakari Ailus , Hans Verkuil Subject: Re: [PATCH v4 1/2] media: add new mediabus format enums for dm365 Date: Tue, 17 Jul 2012 12:56:24 +0200 Message-ID: <9731012.hn1ecEuNnk@avalon> In-Reply-To: <1333102154-24657-2-git-send-email-manjunath.hadli@ti.com> References: <1333102154-24657-1-git-send-email-manjunath.hadli@ti.com> <1333102154-24657-2-git-send-email-manjunath.hadli@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-media-owner@vger.kernel.org List-ID: Hi Manjunath, Thank you for the patch. Just some nitpicking, please see below. On Friday 30 March 2012 10:09:13 Hadli, Manjunath wrote: > add new enum entries for supporting the media-bus formats on dm365. > These include some bayer and some non-bayer formats. > V4L2_MBUS_FMT_YDYC8_1X16 and V4L2_MBUS_FMT_UV8_1X8 are used > internal to the hardware by the resizer. > V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8 represents the bayer ALAW format > that is supported by dm365 hardware. > > Signed-off-by: Manjunath Hadli > Cc: Laurent Pinchart > Cc: Sakari Ailus > Cc: Hans Verkuil > --- > Documentation/DocBook/media/v4l/subdev-formats.xml | 171 +++++++++++++++++ > include/linux/v4l2-mediabus.h | 10 +- > 2 files changed, 179 insertions(+), 2 deletions(-) > > diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml > b/Documentation/DocBook/media/v4l/subdev-formats.xml index 49c532e..48d92bb > 100644 > --- a/Documentation/DocBook/media/v4l/subdev-formats.xml > +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml > @@ -356,6 +356,9 @@ > If the pixel components are DPCM-compressed, a mention of > the DPCM compression and the number of bits per compressed pixel > component. > + If the pixel components are ALAW-compressed, a mention of > the > + ALAW compression and the number of bits per compressed pixel > component. > + I would group ALAW and DPCM compression, as they're mutally exclusive. Something like transferred on the same number of bits. Common values are 8, 10 and 12. - If the pixel components are DPCM-compressed, a mention of - the DPCM compression and the number of bits per compressed pixel - component. - + The compression (optional). If the pixel components are + ALAW- or DPCM-compressed, a mention of the compression scheme and the + number of bits per compressed pixel component. The number of bus samples per pixel. Pixels that are wider than the bus width must be transferred in multiple samples. Common values are 1 and 2. > The number of bus samples per pixel. Pixels that are wider > than the bus width must be transferred in multiple samples. Common values > are 1 and 2. > @@ -572,6 +575,74 @@ > r1 > r0 > > + > + V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8 > + 0x3015 > + > + - > + - > + - > + - > + b7 > + b6 > + b5 > + b4 > + b3 > + b2 > + b1 > + b0 > + > + > + V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8 > + 0x3016 > + > + - > + - > + - > + - > + g7 > + g6 > + g5 > + g4 > + g3 > + g2 > + g1 > + g0 > + > + > + V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8 > + 0x3017 > + > + - > + - > + - > + - > + g7 > + g6 > + g5 > + g4 > + g3 > + g2 > + g1 > + g0 > + > + > + V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8 > + 0x3018 > + > + - > + - > + - > + - > + r7 > + r6 > + r5 > + r4 > + r3 > + r2 > + r1 > + r0 > + Please move the ALAW formats above the DPCM formats to keep them alphabetically sorted. > > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE > 0x3003 > @@ -965,6 +1036,56 @@ > y1 > y0 > > + That's a weird one. Just out of curiosity, what's the point of transferring chroma information without luma ? > + V4L2_MBUS_FMT_UV8_1X8 > + 0x2015 > + > + - > + - > + - > + - > + - > + - > + - > + - > + - > + - > + - > + - > + u7 > + u6 > + u5 > + u4 > + u3 > + u2 > + u1 > + u0 > + > + > + > + > + > + - > + - > + - > + - > + - > + - > + - > + - > + - > + - > + - > + - > + v7 > + v6 > + v5 > + v4 > + v3 > + v2 > + v1 > + v0 > + > > V4L2_MBUS_FMT_UYVY8_1_5X8 > 0x2002 > @@ -2415,6 +2536,56 @@ > u1 > u0 > > + What is this beast ? We at least need a textual description, as I have no idea what the format corresponds to. > + V4L2_MBUS_FMT_YDYC8_1X16 > + 0x2014 > + > + - > + - > + - > + - > + y7 > + y6 > + y5 > + y4 > + y3 > + y2 > + y1 > + y0 > + d7 > + d6 > + d5 > + d4 > + d3 > + d2 > + d1 > + d0 > + > + > + > + > + > + - > + - > + - > + - > + y7 > + y6 > + y5 > + y4 > + y3 > + y2 > + y1 > + y0 > + c7 > + c6 > + c5 > + c4 > + c3 > + c2 > + c1 > + c0 > + > > V4L2_MBUS_FMT_YUYV10_1X20 > 0x200d > diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h > index 5ea7f75..8d68fa1 100644 > --- a/include/linux/v4l2-mediabus.h > +++ b/include/linux/v4l2-mediabus.h > @@ -47,8 +47,9 @@ enum v4l2_mbus_pixelcode { > V4L2_MBUS_FMT_RGB565_2X8_BE = 0x1007, > V4L2_MBUS_FMT_RGB565_2X8_LE = 0x1008, > > - /* YUV (including grey) - next is 0x2014 */ > + /* YUV (including grey) - next is 0x2016 */ > V4L2_MBUS_FMT_Y8_1X8 = 0x2001, > + V4L2_MBUS_FMT_UV8_1X8 = 0x2015, > V4L2_MBUS_FMT_UYVY8_1_5X8 = 0x2002, > V4L2_MBUS_FMT_VYUY8_1_5X8 = 0x2003, > V4L2_MBUS_FMT_YUYV8_1_5X8 = 0x2004, > @@ -65,10 +66,11 @@ enum v4l2_mbus_pixelcode { > V4L2_MBUS_FMT_VYUY8_1X16 = 0x2010, > V4L2_MBUS_FMT_YUYV8_1X16 = 0x2011, > V4L2_MBUS_FMT_YVYU8_1X16 = 0x2012, > + V4L2_MBUS_FMT_YDYC8_1X16 = 0x2014, > V4L2_MBUS_FMT_YUYV10_1X20 = 0x200d, > V4L2_MBUS_FMT_YVYU10_1X20 = 0x200e, > > - /* Bayer - next is 0x3015 */ > + /* Bayer - next is 0x3019 */ > V4L2_MBUS_FMT_SBGGR8_1X8 = 0x3001, > V4L2_MBUS_FMT_SGBRG8_1X8 = 0x3013, > V4L2_MBUS_FMT_SGRBG8_1X8 = 0x3002, > @@ -77,6 +79,10 @@ enum v4l2_mbus_pixelcode { > V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8 = 0x300c, > V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8 = 0x3009, > V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8 = 0x300d, > + V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8 = 0x3015, > + V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8 = 0x3016, > + V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8 = 0x3017, > + V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8 = 0x3018, Please move the ALAW formats above the DPCM formats to keep them alphabetically sorted. > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE = 0x3003, > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE = 0x3004, > V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE = 0x3005, -- Regards, Laurent Pinchart