Sakari,
Thanks for the patch.
------------------------------------------------------------------------
BRs,
Bingbu Cao
>-----Original Message-----
>From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>Sent: Monday, December 11, 2023 10:07 PM
>To: linux-media@xxxxxxxxxxxxxxx
>Cc: laurent.pinchart@xxxxxxxxxxxxxxxx; jacopo.mondi@xxxxxxxxxxxxxxxx; Cao,
>Bingbu <bingbu.cao@xxxxxxxxx>; Qiu, Tian Shu <tian.shu.qiu@xxxxxxxxx>; Hans
>Verkuil <hverkuil-cisco@xxxxxxxxx>
>Subject: [PATCH 3/4] media: imx319: Use v4l2_link_frequencies_to_bitmap
>helper
>
>Use the v4l2_link_frequencies_to_bitmap() helper to figure out which
>driver-supported link frequencies can be used on a given system.
>
>Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>---
> drivers/media/i2c/imx319.c | 53 ++++++++++----------------------------
> 1 file changed, 14 insertions(+), 39 deletions(-)
>
>diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c index
>e47eff672e0c..3fdda6f7d01a 100644
>--- a/drivers/media/i2c/imx319.c
>+++ b/drivers/media/i2c/imx319.c
>@@ -70,7 +70,7 @@
> #define IMX319_REG_ORIENTATION 0x0101
>
> /* default link frequency and external clock */
>-#define IMX319_LINK_FREQ_DEFAULT 482400000
>+#define IMX319_LINK_FREQ_DEFAULT 482400000LL
> #define IMX319_EXT_CLK 19200000
> #define IMX319_LINK_FREQ_INDEX 0
>
>@@ -107,8 +107,7 @@ struct imx319_mode {
>
> struct imx319_hwcfg {
> u32 ext_clk; /* sensor external clk */
>- s64 *link_freqs; /* CSI-2 link frequencies */
>- unsigned int nr_of_link_freqs;
>+ unsigned long link_freq_bitmap;
> };
>
> struct imx319 {
>@@ -129,7 +128,6 @@ struct imx319 {
> const struct imx319_mode *cur_mode;
>
> struct imx319_hwcfg *hwcfg;
>- s64 link_def_freq; /* CSI-2 link default frequency */
>
> /*
> * Mutex for serialized access:
>@@ -1654,7 +1652,10 @@ static const char * const imx319_test_pattern_menu[]
>= {
> "Pseudorandom Sequence (PN9)",
> };
>
>-/* supported link frequencies */
>+/*
>+ * When adding more than the one below, make sure the disallowed ones
>+will
>+ * actually be disabled in the LINK_FREQ control.
>+ */
> static const s64 link_freq_menu_items[] = {
> IMX319_LINK_FREQ_DEFAULT,
> };
>@@ -2058,7 +2059,7 @@ imx319_set_pad_format(struct v4l2_subdev *sd,
> *framefmt = fmt->format;
> } else {
> imx319->cur_mode = mode;
>- pixel_rate = imx319->link_def_freq * 2 * 4;
>+ pixel_rate = IMX319_LINK_FREQ_DEFAULT * 2 * 4;
> do_div(pixel_rate, 10);
> __v4l2_ctrl_s_ctrl_int64(imx319->pixel_rate, pixel_rate);
> /* Update limits and set FPS to default */ @@ -2255,7 +2256,7
>@@ static int imx319_init_controls(struct imx319 *imx319)
> imx319->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
>- pixel_rate = imx319->link_def_freq * 2 * 4;
>+ pixel_rate = IMX319_LINK_FREQ_DEFAULT * 2 * 4;
> do_div(pixel_rate, 10);
> /* By default, PIXEL_RATE is read only */
> imx319->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx319_ctrl_ops,
>@@ -2332,7 +2333,6 @@ static struct imx319_hwcfg *imx319_get_hwcfg(struct
>device *dev)
> };
> struct fwnode_handle *ep;
> struct fwnode_handle *fwnode = dev_fwnode(dev);
>- unsigned int i;
> int ret;
>
> if (!fwnode)
>@@ -2364,24 +2364,14 @@ static struct imx319_hwcfg *imx319_get_hwcfg(struct
>device *dev)
> goto out_err;
> }
>
>- dev_dbg(dev, "num of link freqs: %d",
>bus_cfg.nr_of_link_frequencies);
>- if (!bus_cfg.nr_of_link_frequencies) {
>- dev_warn(dev, "no link frequencies defined");
>- goto out_err;
>- }
>-
>- cfg->nr_of_link_freqs = bus_cfg.nr_of_link_frequencies;
>- cfg->link_freqs = devm_kcalloc(dev,
>- bus_cfg.nr_of_link_frequencies + 1,
>- sizeof(*cfg->link_freqs), GFP_KERNEL);
>- if (!cfg->link_freqs)
>+ ret = v4l2_link_frequencies_to_bitmap(dev, bus_cfg.link_frequencies,
>+ bus_cfg.nr_of_link_frequencies,
>+ link_freq_menu_items,
>+ ARRAY_SIZE(link_freq_menu_items),
>+ &cfg->link_freq_bitmap);
>+ if (ret)
> goto out_err;
>
>- for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) {
>- cfg->link_freqs[i] = bus_cfg.link_frequencies[i];
>- dev_dbg(dev, "link_freq[%d] = %lld", i, cfg->link_freqs[i]);
>- }
>-
> v4l2_fwnode_endpoint_free(&bus_cfg);
> fwnode_handle_put(ep);
> return cfg;
>@@ -2397,7 +2387,6 @@ static int imx319_probe(struct i2c_client *client)
> struct imx319 *imx319;
> bool full_power;
> int ret;
>- u32 i;
>
> imx319 = devm_kzalloc(&client->dev, sizeof(*imx319), GFP_KERNEL);
> if (!imx319)
>@@ -2425,20 +2414,6 @@ static int imx319_probe(struct i2c_client *client)
> goto error_probe;
> }
>
>- imx319->link_def_freq = link_freq_menu_items[IMX319_LINK_FREQ_INDEX];
>- for (i = 0; i < imx319->hwcfg->nr_of_link_freqs; i++) {
>- if (imx319->hwcfg->link_freqs[i] == imx319->link_def_freq) {
>- dev_dbg(&client->dev, "link freq index %d matched", i);
>- break;
>- }
>- }
>-
>- if (i == imx319->hwcfg->nr_of_link_freqs) {
>- dev_err(&client->dev, "no link frequency supported");
>- ret = -EINVAL;
>- goto error_probe;
>- }
>-
Do we also need change the link frequency ctrl *max* value once we get
The bitmap?
> /* Set default mode to max resolution */
> imx319->cur_mode = &supported_modes[0];
>
>--
>2.39.2
