On Fri Feb 21 16:22:13 2025 +0530, Jai Luthra wrote:
> The imx219_get_binning() function currently returns two separate pieces
> of information, the return value is the binning mode enum, and the bin_h
> and bin_v references are updated with whether to perform binning in
> horizontal and vertical dimensions.
> 
> It is simpler to combine both of these pieces of information, and
> directly update the bin_h and bin_v references with the register value
> that we will write to the sensor, which includes if the binning is
> digital or analog mode, thus allowing us to remove the superfluous
> binning mode enum.
> 
> This is only a style change for the driver, with no functionality
> updated.
> 
> Suggested-by: Jacopo Mondi <jacopo.mo...@ideasonboard.com>
> Link: 
> https://lore.kernel.org/linux-media/ubuuob7mb3o5bxoumrxv4rufutgk3lvdmdery6d3bfc6rytfti@tcchhlechzzp/
> Reviewed-by: Jacopo Mondi <jacopo.mo...@ideasonboard.com>
> Signed-off-by: Jai Luthra <jai.lut...@ideasonboard.com>
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Signed-off-by: Hans Verkuil <hverk...@xs4all.nl>

Patch committed.

Thanks,
Hans Verkuil

 drivers/media/i2c/imx219.c | 55 +++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 30 deletions(-)

---

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index fcd98ee54768..a8c9d296daa6 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -145,12 +145,6 @@
 #define IMX219_PIXEL_ARRAY_WIDTH       3280U
 #define IMX219_PIXEL_ARRAY_HEIGHT      2464U
 
-enum binning_mode {
-       BINNING_NONE = IMX219_BINNING_NONE,
-       BINNING_X2 = IMX219_BINNING_X2,
-       BINNING_ANALOG_X2 = IMX219_BINNING_X2_ANALOG,
-};
-
 /* Mode : resolution and related config&values */
 struct imx219_mode {
        /* Frame width */
@@ -405,39 +399,43 @@ static u32 imx219_get_format_bpp(const struct 
v4l2_mbus_framefmt *format)
        }
 }
 
-static enum binning_mode imx219_get_binning(struct imx219 *imx219, u8 *bin_h,
-                                           u8 *bin_v)
+static void imx219_get_binning(struct imx219 *imx219, u8 *bin_h, u8 *bin_v)
 {
        struct v4l2_subdev_state *state =
                v4l2_subdev_get_locked_active_state(&imx219->sd);
        const struct v4l2_mbus_framefmt *format =
                v4l2_subdev_state_get_format(state, 0);
        const struct v4l2_rect *crop = v4l2_subdev_state_get_crop(state, 0);
+       u32 hbin = crop->width / format->width;
+       u32 vbin = crop->height / format->height;
 
-       *bin_h = crop->width / format->width;
-       *bin_v = crop->height / format->height;
+       *bin_h = IMX219_BINNING_NONE;
+       *bin_v = IMX219_BINNING_NONE;
 
-       if (*bin_h == 2 && *bin_v == 2)
-               return BINNING_ANALOG_X2;
-       else if (*bin_h == 2 || *bin_v == 2)
-               /*
-                * Don't use analog binning if only one dimension
-                * is binned, as it crops the other dimension
-                */
-               return BINNING_X2;
-       else
-               return BINNING_NONE;
+       /*
+        * Use analog binning only if both dimensions are binned, as it crops
+        * the other dimension.
+        */
+       if (hbin == 2 && vbin == 2) {
+               *bin_h = IMX219_BINNING_X2_ANALOG;
+               *bin_v = IMX219_BINNING_X2_ANALOG;
+
+               return;
+       }
+
+       if (hbin == 2)
+               *bin_h = IMX219_BINNING_X2;
+       if (vbin == 2)
+               *bin_v = IMX219_BINNING_X2;
 }
 
 static inline u32 imx219_get_rate_factor(struct imx219 *imx219)
 {
        u8 bin_h, bin_v;
-       enum binning_mode binning = imx219_get_binning(imx219, &bin_h, &bin_v);
 
-       if (binning == BINNING_ANALOG_X2)
-               return 2;
+       imx219_get_binning(imx219, &bin_h, &bin_v);
 
-       return 1;
+       return (bin_h & bin_v) == IMX219_BINNING_X2_ANALOG ? 2 : 1;
 }
 
 /* 
-----------------------------------------------------------------------------
@@ -673,7 +671,6 @@ static int imx219_set_framefmt(struct imx219 *imx219,
 {
        const struct v4l2_mbus_framefmt *format;
        const struct v4l2_rect *crop;
-       enum binning_mode binning;
        u8 bin_h, bin_v;
        u32 bpp;
        int ret = 0;
@@ -691,11 +688,9 @@ static int imx219_set_framefmt(struct imx219 *imx219,
        cci_write(imx219->regmap, IMX219_REG_Y_ADD_END_A,
                  crop->top - IMX219_PIXEL_ARRAY_TOP + crop->height - 1, &ret);
 
-       binning = imx219_get_binning(imx219, &bin_h, &bin_v);
-       cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_H,
-                 (bin_h == 2) ? binning : BINNING_NONE, &ret);
-       cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_V,
-                 (bin_v == 2) ? binning : BINNING_NONE, &ret);
+       imx219_get_binning(imx219, &bin_h, &bin_v);
+       cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_H, bin_h, &ret);
+       cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_V, bin_v, &ret);
 
        cci_write(imx219->regmap, IMX219_REG_X_OUTPUT_SIZE,
                  format->width, &ret);

Reply via email to