[{"id":13625,"web_url":"https://patchwork.libcamera.org/comment/13625/","msgid":"<20201106123306.2djzfifmopfl6og6@uno.localdomain>","date":"2020-11-06T12:33:06","subject":"Re: [libcamera-devel] [PATCH 4/4] media: i2c: imx219: Selection\n\tcompliance fixes","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hans,\n\nOn Wed, Aug 05, 2020 at 12:57:21PM +0200, Jacopo Mondi wrote:\n> From: Hans Verkuil <hverkuil@xs4all.nl>\n>\n> To comply with the intended usage of the V4L2 selection target when\n> used to retrieve a sensor image properties, adjust the rectangles\n> returned by the imx219 driver.\n>\n> The top/left crop coordinates of the TGT_CROP rectangle were set to\n> (0, 0) instead of (8, 8) which is the offset from the larger physical\n> pixel array rectangle. This was also a mismatch with the default values\n> crop rectangle value, so this is corrected. Found with v4l2-compliance.\n>\n> While at it, add V4L2_SEL_TGT_CROP_BOUNDS support: CROP_DEFAULT and\n> CROP_BOUNDS have the same size as the non-active pixels are not readable\n> using the selection API. Found with v4l2-compliance.\n>\n> Fixes: e6d4ef7d58aa7 (\"media: i2c: imx219: Implement get_selection\")\n> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>\n> [reword commit message, use macros for pixel offsets]\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nHas this fallen to the cracks ? It was part of a larger series\n(abandoned for now) but I think this patch has merits and can be broke\nout.\n\nIf that's the case I'll fix ov5647 which I have in the pipe which\nsuffer from the same issue as this one and update the libcamera side\nthat uses this information.\n\nThanks\n  j\n\n> ---\n>  drivers/media/i2c/imx219.c | 17 +++++++++--------\n>  1 file changed, 9 insertions(+), 8 deletions(-)\n>\n> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c\n> index 60b23d113fc56..ff48a95b448b1 100644\n> --- a/drivers/media/i2c/imx219.c\n> +++ b/drivers/media/i2c/imx219.c\n> @@ -473,8 +473,8 @@ static const struct imx219_mode supported_modes[] = {\n>  \t\t.width = 3280,\n>  \t\t.height = 2464,\n>  \t\t.crop = {\n> -\t\t\t.left = 0,\n> -\t\t\t.top = 0,\n> +\t\t\t.left = IMX219_PIXEL_ARRAY_LEFT,\n> +\t\t\t.top = IMX219_PIXEL_ARRAY_TOP,\n>  \t\t\t.width = 3280,\n>  \t\t\t.height = 2464\n>  \t\t},\n> @@ -489,8 +489,8 @@ static const struct imx219_mode supported_modes[] = {\n>  \t\t.width = 1920,\n>  \t\t.height = 1080,\n>  \t\t.crop = {\n> -\t\t\t.left = 680,\n> -\t\t\t.top = 692,\n> +\t\t\t.left = 688,\n> +\t\t\t.top = 700,\n>  \t\t\t.width = 1920,\n>  \t\t\t.height = 1080\n>  \t\t},\n> @@ -505,8 +505,8 @@ static const struct imx219_mode supported_modes[] = {\n>  \t\t.width = 1640,\n>  \t\t.height = 1232,\n>  \t\t.crop = {\n> -\t\t\t.left = 0,\n> -\t\t\t.top = 0,\n> +\t\t\t.left = IMX219_PIXEL_ARRAY_LEFT,\n> +\t\t\t.top = IMX219_PIXEL_ARRAY_TOP,\n>  \t\t\t.width = 3280,\n>  \t\t\t.height = 2464\n>  \t\t},\n> @@ -521,8 +521,8 @@ static const struct imx219_mode supported_modes[] = {\n>  \t\t.width = 640,\n>  \t\t.height = 480,\n>  \t\t.crop = {\n> -\t\t\t.left = 1000,\n> -\t\t\t.top = 752,\n> +\t\t\t.left = 1008,\n> +\t\t\t.top = 760,\n>  \t\t\t.width = 1280,\n>  \t\t\t.height = 960\n>  \t\t},\n> @@ -1045,6 +1045,7 @@ static int imx219_get_selection(struct v4l2_subdev *sd,\n>  \t\treturn 0;\n>\n>  \tcase V4L2_SEL_TGT_CROP_DEFAULT:\n> +\tcase V4L2_SEL_TGT_CROP_BOUNDS:\n>  \t\tsel->r.top = IMX219_PIXEL_ARRAY_TOP;\n>  \t\tsel->r.left = IMX219_PIXEL_ARRAY_LEFT;\n>  \t\tsel->r.width = IMX219_PIXEL_ARRAY_WIDTH;\n> --\n> 2.27.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0A3ACBE082\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  6 Nov 2020 12:33:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 901B762D28;\n\tFri,  6 Nov 2020 13:33:09 +0100 (CET)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F1D762D1B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  6 Nov 2020 13:33:08 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 980E4FF803;\n\tFri,  6 Nov 2020 12:33:06 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 6 Nov 2020 13:33:06 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Hans Verkuil <hverkuil@xs4all.nl>,\n\tSakari Ailus <sakari.ailus@linux.intel.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20201106123306.2djzfifmopfl6og6@uno.localdomain>","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200805105721.15445-5-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 4/4] media: i2c: imx219: Selection\n\tcompliance fixes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"Hans Verkuil <hverkuil-cisco@xs4all.nl>,\n\tRicardo Ribalda Delgado <ricardo.ribalda@gmail.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tSowjanya Komatineni <skomatineni@nvidia.com>,\n\tLinux Media Mailing List <linux-media@vger.kernel.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13891,"web_url":"https://patchwork.libcamera.org/comment/13891/","msgid":"<20201126072857.52cplpzfwq7tnejz@uno.localdomain>","date":"2020-11-26T07:28:57","subject":"Re: [libcamera-devel] [PATCH 4/4] media: i2c: imx219: Selection\n\tcompliance fixes","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hans,\n\nOn Fri, Nov 06, 2020 at 01:33:06PM +0100, Jacopo Mondi wrote:\n> Hi Hans,\n>\n> On Wed, Aug 05, 2020 at 12:57:21PM +0200, Jacopo Mondi wrote:\n> > From: Hans Verkuil <hverkuil@xs4all.nl>\n> >\n> > To comply with the intended usage of the V4L2 selection target when\n> > used to retrieve a sensor image properties, adjust the rectangles\n> > returned by the imx219 driver.\n> >\n> > The top/left crop coordinates of the TGT_CROP rectangle were set to\n> > (0, 0) instead of (8, 8) which is the offset from the larger physical\n> > pixel array rectangle. This was also a mismatch with the default values\n> > crop rectangle value, so this is corrected. Found with v4l2-compliance.\n> >\n> > While at it, add V4L2_SEL_TGT_CROP_BOUNDS support: CROP_DEFAULT and\n> > CROP_BOUNDS have the same size as the non-active pixels are not readable\n> > using the selection API. Found with v4l2-compliance.\n> >\n> > Fixes: e6d4ef7d58aa7 (\"media: i2c: imx219: Implement get_selection\")\n> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>\n> > [reword commit message, use macros for pixel offsets]\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> Has this fallen to the cracks ? It was part of a larger series\n> (abandoned for now) but I think this patch has merits and can be broke\n> out.\n>\n> If that's the case I'll fix ov5647 which I have in the pipe which\n> suffer from the same issue as this one and update the libcamera side\n> that uses this information.\n\nGentle ping\n\n>\n> Thanks\n>   j\n>\n> > ---\n> >  drivers/media/i2c/imx219.c | 17 +++++++++--------\n> >  1 file changed, 9 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c\n> > index 60b23d113fc56..ff48a95b448b1 100644\n> > --- a/drivers/media/i2c/imx219.c\n> > +++ b/drivers/media/i2c/imx219.c\n> > @@ -473,8 +473,8 @@ static const struct imx219_mode supported_modes[] = {\n> >  \t\t.width = 3280,\n> >  \t\t.height = 2464,\n> >  \t\t.crop = {\n> > -\t\t\t.left = 0,\n> > -\t\t\t.top = 0,\n> > +\t\t\t.left = IMX219_PIXEL_ARRAY_LEFT,\n> > +\t\t\t.top = IMX219_PIXEL_ARRAY_TOP,\n> >  \t\t\t.width = 3280,\n> >  \t\t\t.height = 2464\n> >  \t\t},\n> > @@ -489,8 +489,8 @@ static const struct imx219_mode supported_modes[] = {\n> >  \t\t.width = 1920,\n> >  \t\t.height = 1080,\n> >  \t\t.crop = {\n> > -\t\t\t.left = 680,\n> > -\t\t\t.top = 692,\n> > +\t\t\t.left = 688,\n> > +\t\t\t.top = 700,\n> >  \t\t\t.width = 1920,\n> >  \t\t\t.height = 1080\n> >  \t\t},\n> > @@ -505,8 +505,8 @@ static const struct imx219_mode supported_modes[] = {\n> >  \t\t.width = 1640,\n> >  \t\t.height = 1232,\n> >  \t\t.crop = {\n> > -\t\t\t.left = 0,\n> > -\t\t\t.top = 0,\n> > +\t\t\t.left = IMX219_PIXEL_ARRAY_LEFT,\n> > +\t\t\t.top = IMX219_PIXEL_ARRAY_TOP,\n> >  \t\t\t.width = 3280,\n> >  \t\t\t.height = 2464\n> >  \t\t},\n> > @@ -521,8 +521,8 @@ static const struct imx219_mode supported_modes[] = {\n> >  \t\t.width = 640,\n> >  \t\t.height = 480,\n> >  \t\t.crop = {\n> > -\t\t\t.left = 1000,\n> > -\t\t\t.top = 752,\n> > +\t\t\t.left = 1008,\n> > +\t\t\t.top = 760,\n> >  \t\t\t.width = 1280,\n> >  \t\t\t.height = 960\n> >  \t\t},\n> > @@ -1045,6 +1045,7 @@ static int imx219_get_selection(struct v4l2_subdev *sd,\n> >  \t\treturn 0;\n> >\n> >  \tcase V4L2_SEL_TGT_CROP_DEFAULT:\n> > +\tcase V4L2_SEL_TGT_CROP_BOUNDS:\n> >  \t\tsel->r.top = IMX219_PIXEL_ARRAY_TOP;\n> >  \t\tsel->r.left = IMX219_PIXEL_ARRAY_LEFT;\n> >  \t\tsel->r.width = IMX219_PIXEL_ARRAY_WIDTH;\n> > --\n> > 2.27.0\n> >\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4E33BBE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 26 Nov 2020 07:28:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E05566343F;\n\tThu, 26 Nov 2020 08:28:55 +0100 (CET)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 70C9063320\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Nov 2020 08:28:55 +0100 (CET)","from uno.localdomain (host-80-104-176-17.retail.telecomitalia.it\n\t[80.104.176.17]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 5E5EF20000E;\n\tThu, 26 Nov 2020 07:28:52 +0000 (UTC)"],"Date":"Thu, 26 Nov 2020 08:28:57 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Hans Verkuil <hverkuil@xs4all.nl>,\n\tSakari Ailus <sakari.ailus@linux.intel.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20201126072857.52cplpzfwq7tnejz@uno.localdomain>","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-5-jacopo@jmondi.org>\n\t<20201106123306.2djzfifmopfl6og6@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106123306.2djzfifmopfl6og6@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 4/4] media: i2c: imx219: Selection\n\tcompliance fixes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"Hans Verkuil <hverkuil-cisco@xs4all.nl>,\n\tRicardo Ribalda Delgado <ricardo.ribalda@gmail.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tSowjanya Komatineni <skomatineni@nvidia.com>,\n\tLinux Media Mailing List <linux-media@vger.kernel.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]