Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and link frequency
From: Sakari Ailus <sakari.ailus@iki.fi>
Date: 2023-02-22 09:16:40
Also in:
linux-devicetree, linux-media, lkml
Hi Shravan, On Mon, Feb 06, 2023 at 04:43:42AM +0000, Shravan.Chippa@microchip.com wrote:
Hi Sakari,quoted
-----Original Message----- From: shravan Chippa - I35088 Sent: 27 January 2023 05:40 AM To: Sakari Ailus <sakari.ailus@iki.fi> Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; kernel@pengutronix.de; linux-imx@nxp.com; linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- kernel@lists.infradead.org Subject: RE: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and link frequency Hi Sakari,quoted
-----Original Message----- From: Sakari Ailus <sakari.ailus@iki.fi> Sent: 25 January 2023 03:12 PM To: shravan Chippa - I35088 <redacted> Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com; kernel@pengutronix.de; linux-imx@nxp.com; linux-media@vger.kernel.org;quoted
linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and link frequency EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe Hi Shravan, On Tue, Jan 24, 2023 at 05:34:02AM +0000, Shravan.Chippa@microchip.com wrote:quoted
Hi Sakari,quoted
-----Original Message----- From: Sakari Ailus <sakari.ailus@iki.fi> Sent: 24 January 2023 04:33 AM To: shravan Chippa - I35088 <redacted> Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com; mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org; shawnguo@kernel.org; s.hauer@pengutronix.de;festevam@gmail.com;quoted
quoted
quoted
kernel@pengutronix.de; linux-imx@nxp.com; linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- kernel@lists.infradead.org Subject: Re: [PATCH RESEND v10 5/5] media: i2c: imx334: update pixel and link frequency EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe Hi Shravan, On Sat, Jan 21, 2023 at 09:07:13AM +0530, shravan kumar wrote:quoted
From: Shravan Chippa <redacted> Update pixel_rate and link frequency for 1920x1080@30 while changing mode. Add dummy ctrl cases for pixel_rate and link frequency to avoid error while changing the modes dynamically. Add support to handle multiple link frequencies. Suggested-by: Sakari Ailus <sakari.ailus@iki.fi> Signed-off-by: Shravan Chippa <redacted> --- drivers/media/i2c/imx334.c | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-)diff --git a/drivers/media/i2c/imx334.cb/drivers/media/i2c/imx334.c index 309c706114d2..62b104eaa437 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,6 +601,11 @@ 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,quoted
quoted
quoted
mode->hblank, 1, mode->hblank); if (ret)@@ -698,6 +706,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;@@ -1047,7 +1057,7 @@ static int imx334_parse_hw_config(structimx334*imx334)quoted
struct fwnode_handle *ep; unsigned long rate; int ret; - int i; + int i, j;unsigned int would be nicer.I will change.quoted
quoted
if (!fwnode) return -ENXIO;@@ -1097,11 +1107,20 @@ static int imx334_parse_hw_config(structimx334*imx334)quoted
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);Is there a guarantee that you'll only be using the modes with the listed frequencies? I don't see one but I might have missed it.If I understand it correctly, the question here is, the listed freqeunceis and modes are one to one mapped? Then yes.I don't see this being checked in imx334_set_pad_format(), for instance. If a frequency isn't in DT, the driver isn't supposed to be using it either.Yes, there is no check. But, if a frequency is not in DT, the driver will not add in menu items. So, the function imx334_set_pad_format() -> imx334_update_controls() fails, if we set the frequencies which are not there in the DT or menu items.Are you ok with the above explanation or any changes you are expecting? Please do let me know if there are any changes needed. I am planning to send the next version.
There doesn't seem to be anything that would prevent selecting a format with a wrong link frequency in these patches. Could you address that, or point me to where this is done? The control can't be changed by the user, but that's not enough. -- Kind regards, Sakari Ailus _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel