From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 57D66C433F5 for ; Thu, 6 Oct 2022 11:54:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230148AbiJFLyf (ORCPT ); Thu, 6 Oct 2022 07:54:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50828 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230003AbiJFLyd (ORCPT ); Thu, 6 Oct 2022 07:54:33 -0400 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::229]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 101B79D53E for ; Thu, 6 Oct 2022 04:54:31 -0700 (PDT) Received: (Authenticated sender: jacopo@jmondi.org) by mail.gandi.net (Postfix) with ESMTPSA id 6513BFF802; Thu, 6 Oct 2022 11:54:29 +0000 (UTC) Date: Thu, 6 Oct 2022 13:54:27 +0200 From: Jacopo Mondi To: Dave Stevenson Cc: paul.j.murphy@intel.com, daniele.alessandrelli@intel.com, linux-media@vger.kernel.org Subject: Re: [PATCH 13/16] media: i2c: ov9282: Add selection API calls for cropping info Message-ID: <20221006115427.unrzqizgdtvysxoc@uno.localdomain> References: <20221005152809.3785786-1-dave.stevenson@raspberrypi.com> <20221005152809.3785786-14-dave.stevenson@raspberrypi.com> <20221006094341.hz3lvo5vqrf3voas@uno.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi David On Thu, Oct 06, 2022 at 12:39:35PM +0100, Dave Stevenson wrote: > Hi Jacopo > > Thanks for the review. > > On Thu, 6 Oct 2022 at 10:43, Jacopo Mondi wrote: > > > > Hi Dave > > > > On Wed, Oct 05, 2022 at 04:28:06PM +0100, Dave Stevenson wrote: > > > As required by libcamera, add the relevant cropping targets > > > to report which portion of the sensor is being read out in > > > any mode. > > > > > > Signed-off-by: Dave Stevenson > > > --- > > > drivers/media/i2c/ov9282.c | 75 ++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 75 insertions(+) > > > > > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c > > > index 8e86aa7e4b2a..d892f53fb1ea 100644 > > > --- a/drivers/media/i2c/ov9282.c > > > +++ b/drivers/media/i2c/ov9282.c > > > @@ -67,6 +67,17 @@ > > > #define OV9282_PIXEL_RATE (OV9282_LINK_FREQ * 2 * \ > > > OV9282_NUM_DATA_LANES / 10) > > > > > > +/* > > > + * OV9282 native and active pixel array size. > > > + * 8 dummy rows/columns on each edge of a 1280x800 active array > > > + */ > > > +#define OV9282_NATIVE_WIDTH 1296U > > > +#define OV9282_NATIVE_HEIGHT 816U > > > +#define OV9282_PIXEL_ARRAY_LEFT 8U > > > +#define OV9282_PIXEL_ARRAY_TOP 8U > > > +#define OV9282_PIXEL_ARRAY_WIDTH 1280U > > > +#define OV9282_PIXEL_ARRAY_HEIGHT 800U > > > + > > > #define OV9282_REG_MIN 0x00 > > > #define OV9282_REG_MAX 0xfffff > > > > > > @@ -118,6 +129,7 @@ struct ov9282_mode { > > > u32 vblank_min; > > > u32 vblank_max; > > > u32 link_freq_idx; > > > + struct v4l2_rect crop; > > > struct ov9282_reg_list reg_list; > > > }; > > > > > > @@ -280,6 +292,16 @@ static const struct ov9282_mode supported_modes[] = { > > > .vblank_min = 41, > > > .vblank_max = 51540, > > > .link_freq_idx = 0, > > > + .crop = { > > > + /* > > > + * Note that this mode takes the top 720 lines from the > > > + * 800 of the sensor. It does not take a middle crop. > > > + */ > > > + .left = OV9282_PIXEL_ARRAY_LEFT, > > > + .top = OV9282_PIXEL_ARRAY_TOP, > > > + .width = 1280, > > > + .height = 720 > > > + }, > > > .reg_list = { > > > .num_of_regs = ARRAY_SIZE(mode_1280x720_regs), > > > .regs = mode_1280x720_regs, > > > @@ -719,6 +741,58 @@ static int ov9282_init_pad_cfg(struct v4l2_subdev *sd, > > > return ov9282_set_pad_format(sd, sd_state, &fmt); > > > } > > > > > > +static const struct v4l2_rect * > > > +__ov9282_get_pad_crop(struct ov9282 *ov9282, > > > + struct v4l2_subdev_state *sd_state, > > > + unsigned int pad, enum v4l2_subdev_format_whence which) > > > +{ > > > + switch (which) { > > > + case V4L2_SUBDEV_FORMAT_TRY: > > > + return v4l2_subdev_get_try_crop(&ov9282->sd, sd_state, pad); > > > + case V4L2_SUBDEV_FORMAT_ACTIVE: > > > + return &ov9282->cur_mode->crop; > > > + } > > > + > > > + return NULL; > > > +} > > > + > > > +static int ov9282_get_selection(struct v4l2_subdev *sd, > > > + struct v4l2_subdev_state *sd_state, > > > + struct v4l2_subdev_selection *sel) > > > +{ > > > + switch (sel->target) { > > > + case V4L2_SEL_TGT_CROP: { > > > + struct ov9282 *ov9282 = to_ov9282(sd); > > > + > > > + mutex_lock(&ov9282->mutex); > > > > As there's no set_selection, do we need the mutex here ? > > __ov9282_get_pad_crop is looking at the current mode, so the mutex is > against ov9282_set_pad_format changing the mode. Ah right Please add my tag to v2 Reviewed-by: Jacopo Mondi Thanks j > > You'll find the same pattern in imx214, imx219, ov5640, ov5647, > ov8865, and hopefully all other sensor drivers implementing > get_selection for V4L2_SEL_TGT_CROP. > > Dave > > > > + sel->r = *__ov9282_get_pad_crop(ov9282, sd_state, sel->pad, > > > + sel->which); > > > + mutex_unlock(&ov9282->mutex); > > > + > > > + return 0; > > > + } > > > + > > > + case V4L2_SEL_TGT_NATIVE_SIZE: > > > + sel->r.top = 0; > > > + sel->r.left = 0; > > > + sel->r.width = OV9282_NATIVE_WIDTH; > > > + sel->r.height = OV9282_NATIVE_HEIGHT; > > > + > > > + return 0; > > > + > > > + case V4L2_SEL_TGT_CROP_DEFAULT: > > > + case V4L2_SEL_TGT_CROP_BOUNDS: > > > + sel->r.top = OV9282_PIXEL_ARRAY_TOP; > > > + sel->r.left = OV9282_PIXEL_ARRAY_LEFT; > > > + sel->r.width = OV9282_PIXEL_ARRAY_WIDTH; > > > + sel->r.height = OV9282_PIXEL_ARRAY_HEIGHT; > > > + > > > + return 0; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > /** > > > * ov9282_start_streaming() - Start sensor stream > > > * @ov9282: pointer to ov9282 device > > > @@ -963,6 +1037,7 @@ static const struct v4l2_subdev_pad_ops ov9282_pad_ops = { > > > .enum_frame_size = ov9282_enum_frame_size, > > > .get_fmt = ov9282_get_pad_format, > > > .set_fmt = ov9282_set_pad_format, > > > + .get_selection = ov9282_get_selection, > > > }; > > > > > > static const struct v4l2_subdev_ops ov9282_subdev_ops = { > > > -- > > > 2.34.1 > > >