[{"id":10973,"web_url":"https://patchwork.libcamera.org/comment/10973/","msgid":"<8f9e76ed-8c78-f6ae-c0c9-fc6d0927325b@collabora.com>","date":"2020-06-29T16:54:43","subject":"Re: [libcamera-devel] [PATCH 19/25] media: ov5647: Implement\n\tset_fmt pad operation","submitter":{"id":46,"url":"https://patchwork.libcamera.org/api/people/46/","name":"Dafna Hirschfeld","email":"dafna.hirschfeld@collabora.com"},"content":"On 23.06.20 18:49, Jacopo Mondi wrote:\n> Now that the driver supports more than a single mode, implement the\n> .set_fmt pad operation and adjust the existing .get_fmt one to report\n> the currently applied format.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>   drivers/media/i2c/ov5647.c | 67 +++++++++++++++++++++++++++++++++++---\n>   1 file changed, 62 insertions(+), 5 deletions(-)\n> \n> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c\n> index af9e6d43967d8..39e320f321bd8 100644\n> --- a/drivers/media/i2c/ov5647.c\n> +++ b/drivers/media/i2c/ov5647.c\n> @@ -1016,15 +1016,72 @@ static int ov5647_enum_frame_size(struct v4l2_subdev *sd,\n>   \treturn 0;\n>   }\n>   \n> -static int ov5647_set_get_fmt(struct v4l2_subdev *sd,\n> +static int ov5647_get_pad_fmt(struct v4l2_subdev *sd,\n>   \t\t\t      struct v4l2_subdev_pad_config *cfg,\n>   \t\t\t      struct v4l2_subdev_format *format)\n>   {\n>   \tstruct v4l2_mbus_framefmt *fmt = &format->format;\n> +\tstruct v4l2_mbus_framefmt *sensor_format;\n> +\tstruct ov5647 *sensor = to_sensor(sd);\n>   \n> -\t/* Only one format is supported, so return that. */\n> +\tmutex_lock(&sensor->lock);\n>   \tmemset(fmt, 0, sizeof(*fmt));\n> -\t*fmt = OV5647_DEFAULT_FORMAT;\n> +\n> +\tswitch (format->which) {\n> +\tcase V4L2_SUBDEV_FORMAT_TRY:\n> +\t\tsensor_format = v4l2_subdev_get_try_format(sd, cfg, format->pad);\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tsensor_format = &sensor->mode->format;\n> +\t\tbreak;\n> +\t}\n> +\n> +\t*fmt = *sensor_format;\n> +\tmutex_unlock(&sensor->lock);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,\n> +\t\t\t      struct v4l2_subdev_pad_config *cfg,\n> +\t\t\t      struct v4l2_subdev_format *format)\n> +{\n> +\tstruct v4l2_mbus_framefmt *fmt = &format->format;\n> +\tstruct ov5647 *sensor = to_sensor(sd);\n> +\tstruct ov5647_mode *ov5647_mode_list;\n> +\tstruct ov5647_mode *mode;\n> +\tunsigned int num_modes;\n> +\n> +\t/*\n> +\t * Default mbus code MEDIA_BUS_FMT_SBGGR10_1X10 if the requested one\n> +\t * is not supported.\n\nIn previous patch you added macros OV5647_DEFAULT_MODE, OV5647_DEFAULT_FORMAT\nwhich comes from first format in the array 'ov5647_formats' which is MEDIA_BUS_FMT_SBGGR8_1X8.\nBut here you set the default format to MEDIA_BUS_FMT_SBGGR10_1X10\n\n> +\t */\n> +\tif (fmt->code == MEDIA_BUS_FMT_SBGGR8_1X8) {\n> +\t\tov5647_mode_list = ov5647_sbggr8_modes;\n> +\t\tnum_modes = ARRAY_SIZE(ov5647_sbggr8_modes);\n> +\t} else {\n> +\t\tov5647_mode_list = ov5647_sbggr10_modes;\n> +\t\tnum_modes = ARRAY_SIZE(ov5647_sbggr10_modes);\n> +\t}\n> +\n> +\tmode = v4l2_find_nearest_size(ov5647_mode_list, num_modes,\n> +\t\t\t\t      format.width, format.height,\n> +\t\t\t\t      fmt->width, fmt->height);\n> +\n> +\tif (format->which == V4L2_SUBDEV_FORMAT_TRY) {\n> +\t\tmutex_lock(&sensor->lock);\n> +\t\t*v4l2_subdev_get_try_format(sd, cfg, format->pad) = mode->format;\n> +\t\t*fmt = mode->format;\n> +\t\tmutex_unlock(&sensor->lock);\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\t/* Update the sensor mode and apply at it at streamon time. */\n> +\tmutex_lock(&sensor->lock);\n> +\tsensor->mode = mode;\n> +\t*fmt = mode->format;\n> +\tmutex_unlock(&sensor->lock);\n>   \n>   \treturn 0;\n>   }\n> @@ -1068,8 +1125,8 @@ static int ov5647_get_selection(struct v4l2_subdev *sd,\n>   static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {\n>   \t.enum_mbus_code\t\t= ov5647_enum_mbus_code,\n>   \t.enum_frame_size\t= ov5647_enum_frame_size,\n> -\t.set_fmt\t\t= ov5647_set_get_fmt,\n> -\t.get_fmt\t\t= ov5647_set_get_fmt,\n> +\t.set_fmt\t\t= ov5647_set_pad_fmt,\n> +\t.get_fmt\t\t= ov5647_get_pad_fmt,\n>   \t.get_selection\t\t= ov5647_get_selection,\n>   };\n>   \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 877FDBF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Jun 2020 16:54:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D4A4609DB;\n\tMon, 29 Jun 2020 18:54:49 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AD991603B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Jun 2020 18:54:47 +0200 (CEST)","from [IPv6:2003:cb:8737:cf00:84c7:ee07:61e9:a21f]\n\t(p200300cb8737cf0084c7ee0761e9a21f.dip0.t-ipconnect.de\n\t[IPv6:2003:cb:8737:cf00:84c7:ee07:61e9:a21f])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits))\n\t(No client certificate requested) (Authenticated sender: dafna)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 834CF260CC6;\n\tMon, 29 Jun 2020 17:54:46 +0100 (BST)"],"To":"Jacopo Mondi <jacopo@jmondi.org>, mchehab@kernel.org,\n\tsakari.ailus@linux.intel.com, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, roman.kovalivskyi@globallogic.com, \n\tdave.stevenson@raspberrypi.org, naush@raspberrypi.com","References":"<20200623100815.10674-1-jacopo@jmondi.org>\n\t<20200623164911.45147-4-jacopo@jmondi.org>","From":"Dafna Hirschfeld <dafna.hirschfeld@collabora.com>","Message-ID":"<8f9e76ed-8c78-f6ae-c0c9-fc6d0927325b@collabora.com>","Date":"Mon, 29 Jun 2020 18:54:43 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200623164911.45147-4-jacopo@jmondi.org>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 19/25] media: ov5647: Implement\n\tset_fmt pad operation","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":"andrew_gabbasov@mentor.com, mrodin@de.adit-jv.com, mripard@kernel.org,\n\tlibcamera-devel@lists.libcamera.org, sudipi@jp.adit-jv.com,\n\thugues.fruchet@st.com, erosca@de.adit-jv.com, aford173@gmail.com,\n\tlinux-media@vger.kernel.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11003,"web_url":"https://patchwork.libcamera.org/comment/11003/","msgid":"<20200630101338.dz7toga2mhah7rsq@uno.localdomain>","date":"2020-06-30T10:13:38","subject":"Re: [libcamera-devel] [PATCH 19/25] media: ov5647: Implement\n\tset_fmt pad operation","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Dafna,\n\nOn Mon, Jun 29, 2020 at 06:54:43PM +0200, Dafna Hirschfeld wrote:\n>\n>\n> On 23.06.20 18:49, Jacopo Mondi wrote:\n> > Now that the driver supports more than a single mode, implement the\n> > .set_fmt pad operation and adjust the existing .get_fmt one to report\n> > the currently applied format.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >   drivers/media/i2c/ov5647.c | 67 +++++++++++++++++++++++++++++++++++---\n> >   1 file changed, 62 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c\n> > index af9e6d43967d8..39e320f321bd8 100644\n> > --- a/drivers/media/i2c/ov5647.c\n> > +++ b/drivers/media/i2c/ov5647.c\n> > @@ -1016,15 +1016,72 @@ static int ov5647_enum_frame_size(struct v4l2_subdev *sd,\n> >   \treturn 0;\n> >   }\n> > -static int ov5647_set_get_fmt(struct v4l2_subdev *sd,\n> > +static int ov5647_get_pad_fmt(struct v4l2_subdev *sd,\n> >   \t\t\t      struct v4l2_subdev_pad_config *cfg,\n> >   \t\t\t      struct v4l2_subdev_format *format)\n> >   {\n> >   \tstruct v4l2_mbus_framefmt *fmt = &format->format;\n> > +\tstruct v4l2_mbus_framefmt *sensor_format;\n> > +\tstruct ov5647 *sensor = to_sensor(sd);\n> > -\t/* Only one format is supported, so return that. */\n> > +\tmutex_lock(&sensor->lock);\n> >   \tmemset(fmt, 0, sizeof(*fmt));\n> > -\t*fmt = OV5647_DEFAULT_FORMAT;\n> > +\n> > +\tswitch (format->which) {\n> > +\tcase V4L2_SUBDEV_FORMAT_TRY:\n> > +\t\tsensor_format = v4l2_subdev_get_try_format(sd, cfg, format->pad);\n> > +\t\tbreak;\n> > +\tdefault:\n> > +\t\tsensor_format = &sensor->mode->format;\n> > +\t\tbreak;\n> > +\t}\n> > +\n> > +\t*fmt = *sensor_format;\n> > +\tmutex_unlock(&sensor->lock);\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,\n> > +\t\t\t      struct v4l2_subdev_pad_config *cfg,\n> > +\t\t\t      struct v4l2_subdev_format *format)\n> > +{\n> > +\tstruct v4l2_mbus_framefmt *fmt = &format->format;\n> > +\tstruct ov5647 *sensor = to_sensor(sd);\n> > +\tstruct ov5647_mode *ov5647_mode_list;\n> > +\tstruct ov5647_mode *mode;\n> > +\tunsigned int num_modes;\n> > +\n> > +\t/*\n> > +\t * Default mbus code MEDIA_BUS_FMT_SBGGR10_1X10 if the requested one\n> > +\t * is not supported.\n>\n> In previous patch you added macros OV5647_DEFAULT_MODE, OV5647_DEFAULT_FORMAT\n> which comes from first format in the array 'ov5647_formats' which is MEDIA_BUS_FMT_SBGGR8_1X8.\n\nOh well, that's just an arbitrary selection of the format the sensor\nis initialized with.\n\n> But here you set the default format to MEDIA_BUS_FMT_SBGGR10_1X10\n>\n\nI chose the _1x10 version as it supports more resolution than the _1X8\none. The v4l2-spec says if the requested format is not supported the\nclosed possible match should be reported. It is easy to identify what\na closes possible match is when considering the image size, but for image\nformats the \"closes possible match\" might be tricky to define.\n\nI can change the sensor initial default state if you think that's the\ncase, but I don't think the initial configuration and the adjusted format\nreturned from s_stream() should be considered related. Do you agree or\nis there any part of the specs I'm overlooking ?\n\nThanks\n  j\n\n\n> > +\t */\n> > +\tif (fmt->code == MEDIA_BUS_FMT_SBGGR8_1X8) {\n> > +\t\tov5647_mode_list = ov5647_sbggr8_modes;\n> > +\t\tnum_modes = ARRAY_SIZE(ov5647_sbggr8_modes);\n> > +\t} else {\n> > +\t\tov5647_mode_list = ov5647_sbggr10_modes;\n> > +\t\tnum_modes = ARRAY_SIZE(ov5647_sbggr10_modes);\n> > +\t}\n> > +\n> > +\tmode = v4l2_find_nearest_size(ov5647_mode_list, num_modes,\n> > +\t\t\t\t      format.width, format.height,\n> > +\t\t\t\t      fmt->width, fmt->height);\n> > +\n> > +\tif (format->which == V4L2_SUBDEV_FORMAT_TRY) {\n> > +\t\tmutex_lock(&sensor->lock);\n> > +\t\t*v4l2_subdev_get_try_format(sd, cfg, format->pad) = mode->format;\n> > +\t\t*fmt = mode->format;\n> > +\t\tmutex_unlock(&sensor->lock);\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\t/* Update the sensor mode and apply at it at streamon time. */\n> > +\tmutex_lock(&sensor->lock);\n> > +\tsensor->mode = mode;\n> > +\t*fmt = mode->format;\n> > +\tmutex_unlock(&sensor->lock);\n> >   \treturn 0;\n> >   }\n> > @@ -1068,8 +1125,8 @@ static int ov5647_get_selection(struct v4l2_subdev *sd,\n> >   static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {\n> >   \t.enum_mbus_code\t\t= ov5647_enum_mbus_code,\n> >   \t.enum_frame_size\t= ov5647_enum_frame_size,\n> > -\t.set_fmt\t\t= ov5647_set_get_fmt,\n> > -\t.get_fmt\t\t= ov5647_set_get_fmt,\n> > +\t.set_fmt\t\t= ov5647_set_pad_fmt,\n> > +\t.get_fmt\t\t= ov5647_get_pad_fmt,\n> >   \t.get_selection\t\t= ov5647_get_selection,\n> >   };\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 44568BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 30 Jun 2020 10:10:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC21960C57;\n\tTue, 30 Jun 2020 12:10:16 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ADFE4609C7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 30 Jun 2020 12:10:15 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id D1D2F100002;\n\tTue, 30 Jun 2020 10:10:09 +0000 (UTC)"],"Date":"Tue, 30 Jun 2020 12:13:38 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Dafna Hirschfeld <dafna.hirschfeld@collabora.com>","Message-ID":"<20200630101338.dz7toga2mhah7rsq@uno.localdomain>","References":"<20200623100815.10674-1-jacopo@jmondi.org>\n\t<20200623164911.45147-4-jacopo@jmondi.org>\n\t<8f9e76ed-8c78-f6ae-c0c9-fc6d0927325b@collabora.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<8f9e76ed-8c78-f6ae-c0c9-fc6d0927325b@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH 19/25] media: ov5647: Implement\n\tset_fmt pad operation","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":"andrew_gabbasov@mentor.com, dave.stevenson@raspberrypi.org,\n\tmrodin@de.adit-jv.com, erosca@de.adit-jv.com, mripard@kernel.org,\n\troman.kovalivskyi@globallogic.com, libcamera-devel@lists.libcamera.org,\n\tlinux-media@vger.kernel.org, sakari.ailus@linux.intel.com,\n\thugues.fruchet@st.com, mchehab@kernel.org, aford173@gmail.com,\n\tsudipi@jp.adit-jv.com","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":11013,"web_url":"https://patchwork.libcamera.org/comment/11013/","msgid":"<86ac2fa1-32c1-8f85-1a92-6a69ea1e5735@collabora.com>","date":"2020-06-30T11:09:57","subject":"Re: [libcamera-devel] [PATCH 19/25] media: ov5647: Implement\n\tset_fmt pad operation","submitter":{"id":46,"url":"https://patchwork.libcamera.org/api/people/46/","name":"Dafna Hirschfeld","email":"dafna.hirschfeld@collabora.com"},"content":"On 30.06.20 12:13, Jacopo Mondi wrote:\n> Hi Dafna,\n> \n> On Mon, Jun 29, 2020 at 06:54:43PM +0200, Dafna Hirschfeld wrote:\n>>\n>>\n>> On 23.06.20 18:49, Jacopo Mondi wrote:\n>>> Now that the driver supports more than a single mode, implement the\n>>> .set_fmt pad operation and adjust the existing .get_fmt one to report\n>>> the currently applied format.\n>>>\n>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>>> ---\n>>>    drivers/media/i2c/ov5647.c | 67 +++++++++++++++++++++++++++++++++++---\n>>>    1 file changed, 62 insertions(+), 5 deletions(-)\n>>>\n>>> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c\n>>> index af9e6d43967d8..39e320f321bd8 100644\n>>> --- a/drivers/media/i2c/ov5647.c\n>>> +++ b/drivers/media/i2c/ov5647.c\n>>> @@ -1016,15 +1016,72 @@ static int ov5647_enum_frame_size(struct v4l2_subdev *sd,\n>>>    \treturn 0;\n>>>    }\n>>> -static int ov5647_set_get_fmt(struct v4l2_subdev *sd,\n>>> +static int ov5647_get_pad_fmt(struct v4l2_subdev *sd,\n>>>    \t\t\t      struct v4l2_subdev_pad_config *cfg,\n>>>    \t\t\t      struct v4l2_subdev_format *format)\n>>>    {\n>>>    \tstruct v4l2_mbus_framefmt *fmt = &format->format;\n>>> +\tstruct v4l2_mbus_framefmt *sensor_format;\n>>> +\tstruct ov5647 *sensor = to_sensor(sd);\n>>> -\t/* Only one format is supported, so return that. */\n>>> +\tmutex_lock(&sensor->lock);\n>>>    \tmemset(fmt, 0, sizeof(*fmt));\n>>> -\t*fmt = OV5647_DEFAULT_FORMAT;\n>>> +\n>>> +\tswitch (format->which) {\n>>> +\tcase V4L2_SUBDEV_FORMAT_TRY:\n>>> +\t\tsensor_format = v4l2_subdev_get_try_format(sd, cfg, format->pad);\n>>> +\t\tbreak;\n>>> +\tdefault:\n>>> +\t\tsensor_format = &sensor->mode->format;\n>>> +\t\tbreak;\n>>> +\t}\n>>> +\n>>> +\t*fmt = *sensor_format;\n>>> +\tmutex_unlock(&sensor->lock);\n>>> +\n>>> +\treturn 0;\n>>> +}\n>>> +\n>>> +static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,\n>>> +\t\t\t      struct v4l2_subdev_pad_config *cfg,\n>>> +\t\t\t      struct v4l2_subdev_format *format)\n>>> +{\n>>> +\tstruct v4l2_mbus_framefmt *fmt = &format->format;\n>>> +\tstruct ov5647 *sensor = to_sensor(sd);\n>>> +\tstruct ov5647_mode *ov5647_mode_list;\n>>> +\tstruct ov5647_mode *mode;\n>>> +\tunsigned int num_modes;\n>>> +\n>>> +\t/*\n>>> +\t * Default mbus code MEDIA_BUS_FMT_SBGGR10_1X10 if the requested one\n>>> +\t * is not supported.\n>>\n>> In previous patch you added macros OV5647_DEFAULT_MODE, OV5647_DEFAULT_FORMAT\n>> which comes from first format in the array 'ov5647_formats' which is MEDIA_BUS_FMT_SBGGR8_1X8.\n> \n> Oh well, that's just an arbitrary selection of the format the sensor\n> is initialized with.\n> \n>> But here you set the default format to MEDIA_BUS_FMT_SBGGR10_1X10\n>>\n> \n> I chose the _1x10 version as it supports more resolution than the _1X8\n> one. The v4l2-spec says if the requested format is not supported the\n> closed possible match should be reported. It is easy to identify what\n> a closes possible match is when considering the image size, but for image\n> formats the \"closes possible match\" might be tricky to define.\n> \n> I can change the sensor initial default state if you think that's the\n> case, but I don't think the initial configuration and the adjusted format\n> returned from s_stream() should be considered related. Do you agree or\n> is there any part of the specs I'm overlooking ?\n\nI also don't see it in the spec, but I think it is nicer that\ns_fmt default to the initial value. This is also the first value\nreturned in the enumeration.\n\nThanks,\n    d\n\n> \n> Thanks\n>    j\n> \n> \n>>> +\t */\n>>> +\tif (fmt->code == MEDIA_BUS_FMT_SBGGR8_1X8) {\n>>> +\t\tov5647_mode_list = ov5647_sbggr8_modes;\n>>> +\t\tnum_modes = ARRAY_SIZE(ov5647_sbggr8_modes);\n>>> +\t} else {\n>>> +\t\tov5647_mode_list = ov5647_sbggr10_modes;\n>>> +\t\tnum_modes = ARRAY_SIZE(ov5647_sbggr10_modes);\n>>> +\t}\n>>> +\n>>> +\tmode = v4l2_find_nearest_size(ov5647_mode_list, num_modes,\n>>> +\t\t\t\t      format.width, format.height,\n>>> +\t\t\t\t      fmt->width, fmt->height);\n>>> +\n>>> +\tif (format->which == V4L2_SUBDEV_FORMAT_TRY) {\n>>> +\t\tmutex_lock(&sensor->lock);\n>>> +\t\t*v4l2_subdev_get_try_format(sd, cfg, format->pad) = mode->format;\n>>> +\t\t*fmt = mode->format;\n>>> +\t\tmutex_unlock(&sensor->lock);\n>>> +\n>>> +\t\treturn 0;\n>>> +\t}\n>>> +\n>>> +\t/* Update the sensor mode and apply at it at streamon time. */\n>>> +\tmutex_lock(&sensor->lock);\n>>> +\tsensor->mode = mode;\n>>> +\t*fmt = mode->format;\n>>> +\tmutex_unlock(&sensor->lock);\n>>>    \treturn 0;\n>>>    }\n>>> @@ -1068,8 +1125,8 @@ static int ov5647_get_selection(struct v4l2_subdev *sd,\n>>>    static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {\n>>>    \t.enum_mbus_code\t\t= ov5647_enum_mbus_code,\n>>>    \t.enum_frame_size\t= ov5647_enum_frame_size,\n>>> -\t.set_fmt\t\t= ov5647_set_get_fmt,\n>>> -\t.get_fmt\t\t= ov5647_set_get_fmt,\n>>> +\t.set_fmt\t\t= ov5647_set_pad_fmt,\n>>> +\t.get_fmt\t\t= ov5647_get_pad_fmt,\n>>>    \t.get_selection\t\t= ov5647_get_selection,\n>>>    };\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 651A5BFFE2\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 30 Jun 2020 11:10:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E76B660C56;\n\tTue, 30 Jun 2020 13:10:03 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FB25603B4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 30 Jun 2020 13:10:02 +0200 (CEST)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: dafna) with ESMTPSA id 0B0E62A3CE5"],"To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20200623100815.10674-1-jacopo@jmondi.org>\n\t<20200623164911.45147-4-jacopo@jmondi.org>\n\t<8f9e76ed-8c78-f6ae-c0c9-fc6d0927325b@collabora.com>\n\t<20200630101338.dz7toga2mhah7rsq@uno.localdomain>","From":"Dafna Hirschfeld <dafna.hirschfeld@collabora.com>","Message-ID":"<86ac2fa1-32c1-8f85-1a92-6a69ea1e5735@collabora.com>","Date":"Tue, 30 Jun 2020 13:09:57 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200630101338.dz7toga2mhah7rsq@uno.localdomain>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 19/25] media: ov5647: Implement\n\tset_fmt pad operation","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":"andrew_gabbasov@mentor.com, dave.stevenson@raspberrypi.org,\n\tmrodin@de.adit-jv.com, erosca@de.adit-jv.com, mripard@kernel.org,\n\troman.kovalivskyi@globallogic.com, libcamera-devel@lists.libcamera.org,\n\tlinux-media@vger.kernel.org, sakari.ailus@linux.intel.com,\n\thugues.fruchet@st.com, mchehab@kernel.org, aford173@gmail.com,\n\tsudipi@jp.adit-jv.com","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]