From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A2BB2202963 for ; Thu, 16 Apr 2026 13:14:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776345297; cv=none; b=juZZ1ALlZ3tcla6ezqRfk3dFY+Kf8KLDrCgjHY7cU6Au5QcfsdOA75aU+uiWpTa9Jfss2Ex/8Yr3UXLlFrKKiIcehkMLmZdiPw7Oh3uWUHuFqtbC2lGTIGqt5IssJs09afsHWKpPAFZY3emZNMNn6Ivh7yvxl+smKSy7CNzER5k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776345297; c=relaxed/simple; bh=ut+TYfWVtZqHNWoz4G96CqKMq12Q17fBcOZZMr68K2U=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DTDkU7/MooF3QAKByI04zi2H11NngafJ6cJoNOnUZcGguVxe1piAp9AnriSBhYPQfDf3zLucc7W+D+6GedIYMJZQVPZqNA5IQXy+LH4xVYpv1JTVldEigNN7yB+tFqVHXbc53GhJ+3BOI5QU9FzeL8A2i2pHd35PSf6YoWZNVks= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=DJzC4ghR; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="DJzC4ghR" Received: from killaraus.ideasonboard.com (2001-14ba-703d-e500--2a1.rev.dnainternet.fi [IPv6:2001:14ba:703d:e500::2a1]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 79868BB; Thu, 16 Apr 2026 15:13:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1776345192; bh=ut+TYfWVtZqHNWoz4G96CqKMq12Q17fBcOZZMr68K2U=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=DJzC4ghRoUle3IBN+arWOht4FFhSHEBHrx35fvCMZ1DR5O1+PbWnAldrckrh0irSF i5trQM86LNc32XuLSvmeNPeKGOda+bRGo6gS+qTu5q6rQlqhp+YLRfyq4vnKVi99lw OeBeCtMnPiTxdzSOZVKTP75WQDfn+WPah0DKvj3s= Date: Thu, 16 Apr 2026 16:14:44 +0300 From: Laurent Pinchart To: Sakari Ailus Cc: linux-media@vger.kernel.org, hans@jjverkuil.nl, Prabhakar , Kate Hsuan , Dave Stevenson , Tommaso Merciai , Benjamin Mugnier , Sylvain Petinot , Christophe JAILLET , Julien Massot , Naushir Patuck , "Yan, Dongcheng" , "Cao, Bingbu" , "Qiu, Tian Shu" , Stefan Klug , Mirela Rabulea , =?utf-8?B?QW5kcsOp?= Apitzsch , Heimir Thor Sverrisson , Kieran Bingham , Mehdi Djait , Ricardo Ribalda Delgado , Hans de Goede , Jacopo Mondi , Tomi Valkeinen , David Plowman , "Yu, Ong Hock" , "Ng, Khai Wen" , Jai Luthra , Rishikesh Donadkar Subject: Re: [PATCH v4 01/29] media: imx219: Rename "PIXEL_ARRAY" as "VISIBLE" Message-ID: <20260416131444.GA1775831@killaraus.ideasonboard.com> References: <20260408153939.969381-1-sakari.ailus@linux.intel.com> <20260408153939.969381-2-sakari.ailus@linux.intel.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20260408153939.969381-2-sakari.ailus@linux.intel.com> On Wed, Apr 08, 2026 at 06:39:10PM +0300, Sakari Ailus wrote: > The imx219 driver uses macros for denoting the size of the pixel array. > The values reflect the area of manufacturer-designated visible pixels, > reflect this in the naming by calling it "VISIBLE" instead of > "PIXEL_ARRAY". The name "pixel array" is indeed bad. I'm not sure if "visible" is the best name though. The datasheet documents those pixels as "active area". The same name seems to be used in the documentation of other Sony sensors, so we could standardize on that. How about naming the macros IMX219_ACTIVE_AREA_* ? If you agree, Reviewed-by: Laurent Pinchart > Signed-off-by: Sakari Ailus > --- > drivers/media/i2c/imx219.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index 7da02ce5da15..cbd151d4af5f 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -142,10 +142,10 @@ > /* IMX219 native and active pixel array size. */ > #define IMX219_NATIVE_WIDTH 3296U > #define IMX219_NATIVE_HEIGHT 2480U > -#define IMX219_PIXEL_ARRAY_LEFT 8U > -#define IMX219_PIXEL_ARRAY_TOP 8U > -#define IMX219_PIXEL_ARRAY_WIDTH 3280U > -#define IMX219_PIXEL_ARRAY_HEIGHT 2464U > +#define IMX219_VISIBLE_LEFT 8U > +#define IMX219_VISIBLE_TOP 8U > +#define IMX219_VISIBLE_WIDTH 3280U > +#define IMX219_VISIBLE_HEIGHT 2464U > > /* Mode : resolution and related config&values */ > struct imx219_mode { > @@ -675,13 +675,13 @@ static int imx219_set_framefmt(struct imx219 *imx219, > bpp = imx219_get_format_bpp(format); > > cci_write(imx219->regmap, IMX219_REG_X_ADD_STA_A, > - crop->left - IMX219_PIXEL_ARRAY_LEFT, &ret); > + crop->left - IMX219_VISIBLE_LEFT, &ret); > cci_write(imx219->regmap, IMX219_REG_X_ADD_END_A, > - crop->left - IMX219_PIXEL_ARRAY_LEFT + crop->width - 1, &ret); > + crop->left - IMX219_VISIBLE_LEFT + crop->width - 1, &ret); > cci_write(imx219->regmap, IMX219_REG_Y_ADD_STA_A, > - crop->top - IMX219_PIXEL_ARRAY_TOP, &ret); > + crop->top - IMX219_VISIBLE_TOP, &ret); > cci_write(imx219->regmap, IMX219_REG_Y_ADD_END_A, > - crop->top - IMX219_PIXEL_ARRAY_TOP + crop->height - 1, &ret); > + crop->top - IMX219_VISIBLE_TOP + crop->height - 1, &ret); > > imx219_get_binning(state, &bin_h, &bin_v); > cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_H, bin_h, &ret); > @@ -867,8 +867,8 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd, > * Use binning to maximize the crop rectangle size, and centre it in the > * sensor. > */ > - bin_h = min(IMX219_PIXEL_ARRAY_WIDTH / format->width, 2U); > - bin_v = min(IMX219_PIXEL_ARRAY_HEIGHT / format->height, 2U); > + bin_h = min(IMX219_VISIBLE_WIDTH / format->width, 2U); > + bin_v = min(IMX219_VISIBLE_HEIGHT / format->height, 2U); > > /* Ensure bin_h and bin_v are same to avoid 1:2 or 2:1 stretching */ > binning = min(bin_h, bin_v); > @@ -967,10 +967,10 @@ static int imx219_get_selection(struct v4l2_subdev *sd, > > case V4L2_SEL_TGT_CROP_DEFAULT: > case V4L2_SEL_TGT_CROP_BOUNDS: > - sel->r.top = IMX219_PIXEL_ARRAY_TOP; > - sel->r.left = IMX219_PIXEL_ARRAY_LEFT; > - sel->r.width = IMX219_PIXEL_ARRAY_WIDTH; > - sel->r.height = IMX219_PIXEL_ARRAY_HEIGHT; > + sel->r.top = IMX219_VISIBLE_TOP; > + sel->r.left = IMX219_VISIBLE_LEFT; > + sel->r.width = IMX219_VISIBLE_WIDTH; > + sel->r.height = IMX219_VISIBLE_HEIGHT; > > return 0; > } -- Regards, Laurent Pinchart