Re: [PATCH v2 01/14] media: imx219: Rename "PIXEL_ARRAY" as "CROP"

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi Sakari
  How is this related to the metadata seris ? :)

Shouldn't we aim to fast track that series instead of adding more
things as requisites ?

On Wed, Feb 11, 2026 at 11:09:07AM +0200, Sakari Ailus wrote:
> The imx219 driver uses macros for denoting different aspects of the pixel
> array. The values used for IMX219_PIXEL_ARRAY_* macros imply a crop
> configuration however, not the size of the pixel array. Reflect this in
> the naming, too.

3280x2464 is the active pixel array area, according to the datasheet.
I guess one can claim is the default crop (it is in facts used as
DEFAULT_CROP and CROP_BOUNDS rectangle), so:

Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  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 fee63bc106d9..54622c406a03 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_CROP_LEFT		8U
> +#define IMX219_CROP_TOP			8U
> +#define IMX219_CROP_WIDTH		3280U
> +#define IMX219_CROP_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_CROP_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_CROP_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_CROP_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_CROP_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_CROP_WIDTH / format->width, 2U);
> +	bin_v = min(IMX219_CROP_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_CROP_TOP;
> +		sel->r.left = IMX219_CROP_LEFT;
> +		sel->r.width = IMX219_CROP_WIDTH;
> +		sel->r.height = IMX219_CROP_HEIGHT;
>
>  		return 0;
>  	}
> --
> 2.47.3
>
>