2023-04-14 12:59:17

by shravan chippa

[permalink] [raw]
Subject: [PATCH v13 0/1] media: i2c: imx334: support lower bandwidth mode

From: Shravan Chippa <[email protected]>

Hi

This patch series is for imx334 sensor driver support for lower bandwidth

Some platforms may not be capable of supporting the bandwidth
required for 12 bit or 3840x2160@60 resolutions.

Add support for dynamically selecting 10 bit and 1920x1080@30
resolutions.

V12 -> V13
dropped 4 applied patches (ref link: https://git.linuxtv.org/mchehab/media-next.git/log/)

patch 5 changes:
compilation error removed
updated hblank value while changing the modes

V11 -> V12
Update default link frequency from device tree max link
frequency value.
Update init_cfg() function to update the link frequency
menu_skip_mask value.

V10 -> V11
Changed veriable type int to unsigned int
Updated CC list

V10 -> V10
PATCH RESEND with updated CC list

V9 -> V10
Added new dt-binding patch
added support for handling multiple link-frequncy
minor changes on coding style

V8 -> V9
-Updated all array values with samall later to get unifamity
in mode array values
-corrected hblank_min, hbalank, pix_clk for 1920x1080@30 updated
according to link frequency
-corrected mutex use for imx334_get_format_code function
-corrected the fmt->format.code value assinment
-in function imx334_get_format_code variable "i" value comparision
corrected

V7 -> V8
-patch drop "mimx334-odify-link-frequency" as per the commnets
linkfrquncy will be half of the line bandwidth

-changed 1920x1080@30 mode link frequency from (891000000Mbps)
to (445500000Mbps). linkfrquncy will be half of the line bandwidth

V6 -> V7
Reloved: kernel test robot warning
"drivers/media/i2c/imx334.c:767:15: warning: unused variable 'i' "

V5 -> V6
-Drop the dt-binding patch
-Optimize the code to avoid duplicating the lines
-Added proper mutex while imx334_mbus_codes array
-Modified Function __v4l2_ctrl_modify_range arguments as per the review
commants
-Added hblank dummy set ctrl
-Removed Redundant comment
-corrected code alignment
-All commit msgs are re-written

V4 -> V5
-Added 5 more patchs as per the review comments witch has below updates
-Updated 1782000000Mbps link frequency for 3840x2160@60 as per the mode
values
-Updated 1782000000Mbps link frequency in dt-bindings also
-Updated 3840x2160@60 mode array with default(reset) values

-Updated hblank __v4l2_ctrl_s_ctrl() to __v4l2_ctrl_modify_range()
Suggested-by: Jacopo Mondi <[email protected]>

-Current mode update only when we try to set V4L2_SUBDEV_FORMAT_ACTIVE
-Added link frequency (891000000Mbps) and pixel rate (74250000) to
1920x1080@30 mode
Suggested-by: Sakari Ailus <[email protected]>

-Updated commit message

V3 -> V4
- Make the 12 bit and 3840x2160 as default
- Set bus code SRGGB12 if set format fails

V2 -> V3
- Fixed the warning reported by kernel test robot

V1 -> V2
- Addressed the review comment given by Jacopo Mondi,
Which has bug in imx334_enum_frame_size() loop function,
- Renamed array codes[] to imx334_mbus_codes[]

Shravan Chippa (1):
media: i2c: imx334: update pixel, hblank and link frequency

drivers/media/i2c/imx334.c | 62 +++++++++++++++++++++++++++++---------
1 file changed, 48 insertions(+), 14 deletions(-)

--
2.34.1


2023-04-14 12:59:31

by shravan chippa

[permalink] [raw]
Subject: [PATCH v13 1/1] media: i2c: imx334: update pixel, hblank and link frequency

From: Shravan Chippa <[email protected]>

Update pixel_rate and link frequency for 1920x1080@30
while changing mode.

Update vblank value while changing mode

Add support to handle multiple link frequencies.

Add dummy ctrl cases for pixel_rate and link frequency
to avoid error while changing the modes dynamically.

Update default link frequency from device tree max link
frequency value.

Update init_cfg() function to update the link frequency
menu_skip_mask value.

Suggested-by: Sakari Ailus <[email protected]>
Signed-off-by: Shravan Chippa <[email protected]>
---
drivers/media/i2c/imx334.c | 62 +++++++++++++++++++++++++++++---------
1 file changed, 48 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 309c706114d2..d4c9986aee59 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -49,7 +49,8 @@
#define IMX334_INCLK_RATE 24000000

/* CSI2 HW configuration */
-#define IMX334_LINK_FREQ 891000000
+#define IMX334_LINK_FREQ_891M 891000000
+#define IMX334_LINK_FREQ_445M 445500000
#define IMX334_NUM_DATA_LANES 4

#define IMX334_REG_MIN 0x00
@@ -139,12 +140,14 @@ struct imx334 {
u32 vblank;
const struct imx334_mode *cur_mode;
struct mutex mutex;
+ unsigned long menu_skip_mask;
u32 cur_code;
bool streaming;
};

static const s64 link_freq[] = {
- IMX334_LINK_FREQ,
+ IMX334_LINK_FREQ_891M,
+ IMX334_LINK_FREQ_445M,
};

/* Sensor mode registers for 1920x1080@30fps */
@@ -468,7 +471,7 @@ static const struct imx334_mode supported_modes[] = {
.vblank_min = 45,
.vblank_max = 132840,
.pclk = 297000000,
- .link_freq_idx = 0,
+ .link_freq_idx = 1,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_1920x1080_regs),
.regs = mode_1920x1080_regs,
@@ -598,13 +601,23 @@ static int imx334_update_controls(struct imx334 *imx334,
if (ret)
return ret;

+ ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk,
+ mode->pclk, 1, mode->pclk);
+ if (ret)
+ return ret;
+
ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, mode->hblank,
mode->hblank, 1, mode->hblank);
if (ret)
return ret;

- return __v4l2_ctrl_modify_range(imx334->vblank_ctrl, mode->vblank_min,
+
+ ret = __v4l2_ctrl_modify_range(imx334->vblank_ctrl, mode->vblank_min,
mode->vblank_max, 1, mode->vblank);
+ if (ret)
+ return ret;
+
+ return __v4l2_ctrl_s_ctrl(imx334->vblank_ctrl, mode->vblank);
}

/**
@@ -698,6 +711,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
pm_runtime_put(imx334->dev);

break;
+ case V4L2_CID_PIXEL_RATE:
+ case V4L2_CID_LINK_FREQ:
case V4L2_CID_HBLANK:
ret = 0;
break;
@@ -885,7 +900,17 @@ static int imx334_init_pad_cfg(struct v4l2_subdev *sd,
struct v4l2_subdev_format fmt = { 0 };

fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
- imx334_fill_pad_format(imx334, &supported_modes[0], &fmt);
+
+ mutex_lock(&imx334->mutex);
+
+ imx334_fill_pad_format(imx334, imx334->cur_mode, &fmt);
+
+ __v4l2_ctrl_modify_range(imx334->link_freq_ctrl, 0,
+ __fls(imx334->menu_skip_mask),
+ ~(imx334->menu_skip_mask),
+ __ffs(imx334->menu_skip_mask));
+
+ mutex_unlock(&imx334->mutex);

return imx334_set_pad_format(sd, sd_state, &fmt);
}
@@ -1046,8 +1071,8 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
};
struct fwnode_handle *ep;
unsigned long rate;
+ unsigned int i, j;
int ret;
- int i;

if (!fwnode)
return -ENXIO;
@@ -1097,11 +1122,20 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
goto done_endpoint_free;
}

- for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++)
- if (bus_cfg.link_frequencies[i] == IMX334_LINK_FREQ)
+ for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) {
+ for (j = 0; j < ARRAY_SIZE(link_freq); j++) {
+ if (bus_cfg.link_frequencies[i] == link_freq[j]) {
+ set_bit(j, &imx334->menu_skip_mask);
+ break;
+ }
+ }
+
+ if (j == ARRAY_SIZE(link_freq)) {
+ ret = dev_err_probe(imx334->dev, -EINVAL,
+ "no supported link freq found\n");
goto done_endpoint_free;
-
- ret = -EINVAL;
+ }
+ }

done_endpoint_free:
v4l2_fwnode_endpoint_free(&bus_cfg);
@@ -1232,10 +1266,10 @@ static int imx334_init_controls(struct imx334 *imx334)
imx334->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr,
&imx334_ctrl_ops,
V4L2_CID_LINK_FREQ,
- ARRAY_SIZE(link_freq) -
- 1,
- mode->link_freq_idx,
+ __fls(imx334->menu_skip_mask),
+ __ffs(imx334->menu_skip_mask),
link_freq);
+
if (imx334->link_freq_ctrl)
imx334->link_freq_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;

@@ -1302,7 +1336,7 @@ static int imx334_probe(struct i2c_client *client)
}

/* Set default mode to max resolution */
- imx334->cur_mode = &supported_modes[0];
+ imx334->cur_mode = &supported_modes[__ffs(imx334->menu_skip_mask)];
imx334->cur_code = imx334_mbus_codes[0];
imx334->vblank = imx334->cur_mode->vblank;

--
2.34.1

2023-05-17 14:08:19

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH v13 1/1] media: i2c: imx334: update pixel, hblank and link frequency

On Fri, Apr 14, 2023 at 06:03:11PM +0530, shravan kumar wrote:
> From: Shravan Chippa <[email protected]>
>
> Update pixel_rate and link frequency for 1920x1080@30
> while changing mode.
>
> Update vblank value while changing mode
>
> Add support to handle multiple link frequencies.
>
> Add dummy ctrl cases for pixel_rate and link frequency
> to avoid error while changing the modes dynamically.
>
> Update default link frequency from device tree max link
> frequency value.
>
> Update init_cfg() function to update the link frequency
> menu_skip_mask value.
>
> Suggested-by: Sakari Ailus <[email protected]>
> Signed-off-by: Shravan Chippa <[email protected]>

Applied with the following diff:

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index d4c9986aee597..93fc1716e00a1 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -118,6 +118,7 @@ struct imx334_mode {
* @vblank: Vertical blanking in lines
* @cur_mode: Pointer to current selected sensor mode
* @mutex: Mutex for serializing sensor controls
+ * @menu_skip_mask: Menu skip mask for link_freq_ctrl
* @cur_code: current selected format code
* @streaming: Flag indicating streaming state
*/


--
Sakari Ailus