[PATCH 0/2] Link frequency helper for receivers

[PATCH 0/2] Link frequency helper for receivers

[Sakari Ailus]

Hi folks,

Here's a small helper for getting the link frequency from the transmitter
drivers. Besides being easy to use, it allows receiver drivers to work
with old sensor drivers implementing just V4L2_CID_PIXEL_RATE control but
not V4L2_CID_LINK_FREQ control. Using V4L2_CID_PIXEL_RATE is supported on
parallel and CSI-2 D-PHY busses.

Sakari Ailus (2):
  v4l: Add a helper for obtaining the link frequency
  ipu3-cio2: Use v4l2_get_link_freq helper

 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 33 +++++++++---------------
 drivers/media/v4l2-core/v4l2-common.c    | 33 ++++++++++++++++++++++++
 include/media/v4l2-common.h              | 21 +++++++++++++++
 3 files changed, 66 insertions(+), 21 deletions(-)

-- 
2.27.0

[PATCH 1/2] v4l: Add a helper for obtaining the link frequency

[Sakari Ailus]

Add a helper for obtaining the link frequency from transmitter drivers.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 33 +++++++++++++++++++++++++++
 include/media/v4l2-common.h           | 21 +++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 3dc17ebe14fa..78007dba4677 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -441,3 +441,36 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
+
+s64 v4l2_get_link_rate(struct v4l2_ctrl_handler *handler, unsigned int mul,
+		       unsigned int div)
+{
+	struct v4l2_ctrl *ctrl;
+	s64 freq;
+
+	ctrl = v4l2_ctrl_find(handler, V4L2_CID_LINK_FREQ);
+	if (ctrl) {
+		struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
+		int ret;
+
+		qm.index = v4l2_ctrl_g_ctrl(ctrl);
+
+		ret = v4l2_querymenu(handler, &qm);
+		if (ret)
+			return -ENOENT;
+
+		freq = qm.value;
+	} else {
+		if (!mul || !div)
+			return -ENOENT;
+
+		ctrl = v4l2_ctrl_find(handler, V4L2_CID_PIXEL_RATE);
+		if (!ctrl)
+			return -ENOENT;
+
+		freq = div_u64(v4l2_ctrl_g_ctrl_int64(ctrl) * mul, div);
+	}
+
+	return freq > 0 ? freq : -EINVAL;
+}
+EXPORT_SYMBOL_GPL(v4l2_get_link_rate);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index a3083529b698..be36cbdcc1bd 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -519,6 +519,27 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
 int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
 			u32 width, u32 height);
 
+/**
+ * v4l2_get_link_rate - Get link rate from transmitter
+ *
+ * @handler: The transmitter's control handler
+ * @mul: The multiplier between pixel rate and link frequency. Bits per pixel on
+ *	 D-PHY, samples per clock on parallel. 0 otherwise.
+ * @div: The divisor between pixel rate and link frequency. Number of data lanes
+ *	 times two on D-PHY, 1 on parallel. 0 otherwise.
+ *
+ * This function is intended for obtaining the link frequency from the
+ * transmitter sub-devices. It returns the link rate, either from the
+ * V4L2_CID_LINK_FREQ control implemented by the transmitter, or value
+ * calculated based on the V4L2_CID_PIXEL_RATE implemented by the transmitter.
+ *
+ * Returns link frequency on success, otherwise a negative error code:
+ *	-ENOENT: Link frequency or pixel rate control not found
+ *	-EINVAL: Invalid link frequency value
+ */
+s64 v4l2_get_link_rate(struct v4l2_ctrl_handler *handler, unsigned int mul,
+		       unsigned int div);
+
 static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf)
 {
 	/*
-- 
2.27.0

[PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper

[Sakari Ailus]

Use v4l2_get_link_freq helper and add support for sensor drivers
implementing only V4L2_CID_PIXEL_RATE.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 33 +++++++++---------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index c557d189200b..7acfdb3511e3 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -33,6 +33,7 @@ struct ipu3_cio2_fmt {
 	u32 mbus_code;
 	u32 fourcc;
 	u8 mipicode;
+	u8 bpp;
 };
 
 /*
@@ -46,18 +47,22 @@ static const struct ipu3_cio2_fmt formats[] = {
 		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
 		.fourcc		= V4L2_PIX_FMT_IPU3_SGRBG10,
 		.mipicode	= 0x2b,
+		.bpp		= 10,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
 		.fourcc		= V4L2_PIX_FMT_IPU3_SGBRG10,
 		.mipicode	= 0x2b,
+		.bpp		= 10,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
 		.fourcc		= V4L2_PIX_FMT_IPU3_SBGGR10,
 		.mipicode	= 0x2b,
+		.bpp		= 10,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
 		.fourcc		= V4L2_PIX_FMT_IPU3_SRGGB10,
 		.mipicode	= 0x2b,
+		.bpp		= 10,
 	},
 };
 
@@ -288,36 +293,22 @@ static s32 cio2_rx_timing(s32 a, s32 b, s64 freq, int def)
 
 /* Calculate the the delay value for termination enable of clock lane HS Rx */
 static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
-				 struct cio2_csi2_timing *timing)
+				 struct cio2_csi2_timing *timing,
+				 unsigned int bpp, unsigned int lanes)
 {
 	struct device *dev = &cio2->pci_dev->dev;
-	struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
-	struct v4l2_ctrl *link_freq;
 	s64 freq;
 	int r;
 
 	if (!q->sensor)
 		return -ENODEV;
 
-	link_freq = v4l2_ctrl_find(q->sensor->ctrl_handler, V4L2_CID_LINK_FREQ);
-	if (!link_freq) {
-		dev_err(dev, "failed to find LINK_FREQ\n");
-		return -EPIPE;
+	freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
+	if (freq < 0) {
+		dev_err(dev, "error %ld, invalid link_freq\n", freq);
+		return freq;
 	}
 
-	qm.index = v4l2_ctrl_g_ctrl(link_freq);
-	r = v4l2_querymenu(q->sensor->ctrl_handler, &qm);
-	if (r) {
-		dev_err(dev, "failed to get menu item\n");
-		return r;
-	}
-
-	if (!qm.value) {
-		dev_err(dev, "error invalid link_freq\n");
-		return -EINVAL;
-	}
-	freq = qm.value;
-
 	timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
 					    CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
 					    freq,
@@ -364,7 +355,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 
 	lanes = q->csi2.lanes;
 
-	r = cio2_csi2_calc_timing(cio2, q, &timing);
+	r = cio2_csi2_calc_timing(cio2, q, &timing, fmt->bpp, lanes);
 	if (r)
 		return r;
 
-- 
2.27.0

Re: [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4474 bytes --]

Hi Sakari,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20201013]
[cannot apply to v5.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
base:   git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/cf4ab5e39eb042b02f1d5660b5cbd88197a05520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
        git checkout cf4ab5e39eb042b02f1d5660b5cbd88197a05520
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/pci.h:37,
                    from drivers/media/pci/intel/ipu3/ipu3-cio2.c:18:
   drivers/media/pci/intel/ipu3/ipu3-cio2.c: In function 'cio2_csi2_calc_timing':
>> drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 's64' {aka 'long long int'} [-Wformat=]
     308 |   dev_err(dev, "error %ld, invalid link_freq\n", freq);
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
      19 | #define dev_fmt(fmt) fmt
         |                      ^~~
   drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:3: note: in expansion of macro 'dev_err'
     308 |   dev_err(dev, "error %ld, invalid link_freq\n", freq);
         |   ^~~~~~~
   drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:25: note: format string is defined here
     308 |   dev_err(dev, "error %ld, invalid link_freq\n", freq);
         |                       ~~^
         |                         |
         |                         long int
         |                       %lld
   drivers/media/pci/intel/ipu3/ipu3-cio2.c:301:6: warning: unused variable 'r' [-Wunused-variable]
     301 |  int r;
         |      ^

vim +308 drivers/media/pci/intel/ipu3/ipu3-cio2.c

   293	
   294	/* Calculate the the delay value for termination enable of clock lane HS Rx */
   295	static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
   296					 struct cio2_csi2_timing *timing,
   297					 unsigned int bpp, unsigned int lanes)
   298	{
   299		struct device *dev = &cio2->pci_dev->dev;
   300		s64 freq;
   301		int r;
   302	
   303		if (!q->sensor)
   304			return -ENODEV;
   305	
   306		freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
   307		if (freq < 0) {
 > 308			dev_err(dev, "error %ld, invalid link_freq\n", freq);
   309			return freq;
   310		}
   311	
   312		timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
   313						    CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
   314						    freq,
   315						    CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
   316		timing->clk_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_A,
   317						    CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_B,
   318						    freq,
   319						    CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
   320		timing->dat_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_A,
   321						    CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_B,
   322						    freq,
   323						    CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
   324		timing->dat_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_A,
   325						    CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_B,
   326						    freq,
   327						    CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
   328	
   329		dev_dbg(dev, "freq ct value is %d\n", timing->clk_termen);
   330		dev_dbg(dev, "freq cs value is %d\n", timing->clk_settle);
   331		dev_dbg(dev, "freq dt value is %d\n", timing->dat_termen);
   332		dev_dbg(dev, "freq ds value is %d\n", timing->dat_settle);
   333	
   334		return 0;
   335	};
   336	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74757 bytes --]

Re: [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4577 bytes --]

Hi Sakari,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20201013]
[cannot apply to v5.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
base:   git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/cf4ab5e39eb042b02f1d5660b5cbd88197a05520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
        git checkout cf4ab5e39eb042b02f1d5660b5cbd88197a05520
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/pci.h:37,
                    from drivers/media/pci/intel/ipu3/ipu3-cio2.c:18:
   drivers/media/pci/intel/ipu3/ipu3-cio2.c: In function 'cio2_csi2_calc_timing':
>> drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 's64' {aka 'long long int'} [-Wformat=]
     308 |   dev_err(dev, "error %ld, invalid link_freq\n", freq);
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
      19 | #define dev_fmt(fmt) fmt
         |                      ^~~
   drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:3: note: in expansion of macro 'dev_err'
     308 |   dev_err(dev, "error %ld, invalid link_freq\n", freq);
         |   ^~~~~~~
   drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:25: note: format string is defined here
     308 |   dev_err(dev, "error %ld, invalid link_freq\n", freq);
         |                       ~~^
         |                         |
         |                         long int
         |                       %lld
   drivers/media/pci/intel/ipu3/ipu3-cio2.c:301:6: warning: unused variable 'r' [-Wunused-variable]
     301 |  int r;
         |      ^

vim +308 drivers/media/pci/intel/ipu3/ipu3-cio2.c

   293	
   294	/* Calculate the the delay value for termination enable of clock lane HS Rx */
   295	static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
   296					 struct cio2_csi2_timing *timing,
   297					 unsigned int bpp, unsigned int lanes)
   298	{
   299		struct device *dev = &cio2->pci_dev->dev;
   300		s64 freq;
   301		int r;
   302	
   303		if (!q->sensor)
   304			return -ENODEV;
   305	
   306		freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
   307		if (freq < 0) {
 > 308			dev_err(dev, "error %ld, invalid link_freq\n", freq);
   309			return freq;
   310		}
   311	
   312		timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
   313						    CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
   314						    freq,
   315						    CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
   316		timing->clk_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_A,
   317						    CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_B,
   318						    freq,
   319						    CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
   320		timing->dat_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_A,
   321						    CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_B,
   322						    freq,
   323						    CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
   324		timing->dat_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_A,
   325						    CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_B,
   326						    freq,
   327						    CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
   328	
   329		dev_dbg(dev, "freq ct value is %d\n", timing->clk_termen);
   330		dev_dbg(dev, "freq cs value is %d\n", timing->clk_settle);
   331		dev_dbg(dev, "freq dt value is %d\n", timing->dat_termen);
   332		dev_dbg(dev, "freq ds value is %d\n", timing->dat_settle);
   333	
   334		return 0;
   335	};
   336	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 74757 bytes --]

[PATCH v2 2/2] ipu3-cio2: Use v4l2_get_link_freq helper

[Sakari Ailus]

Use v4l2_get_link_freq helper and add support for sensor drivers
implementing only V4L2_CID_PIXEL_RATE.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v1:

- Use %lld for printing long long int

- Remove r (unused variable)

 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 34 +++++++++---------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index c557d189200b..d060cfe473d8 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -33,6 +33,7 @@ struct ipu3_cio2_fmt {
 	u32 mbus_code;
 	u32 fourcc;
 	u8 mipicode;
+	u8 bpp;
 };
 
 /*
@@ -46,18 +47,22 @@ static const struct ipu3_cio2_fmt formats[] = {
 		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
 		.fourcc		= V4L2_PIX_FMT_IPU3_SGRBG10,
 		.mipicode	= 0x2b,
+		.bpp		= 10,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
 		.fourcc		= V4L2_PIX_FMT_IPU3_SGBRG10,
 		.mipicode	= 0x2b,
+		.bpp		= 10,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
 		.fourcc		= V4L2_PIX_FMT_IPU3_SBGGR10,
 		.mipicode	= 0x2b,
+		.bpp		= 10,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
 		.fourcc		= V4L2_PIX_FMT_IPU3_SRGGB10,
 		.mipicode	= 0x2b,
+		.bpp		= 10,
 	},
 };
 
@@ -288,35 +293,20 @@ static s32 cio2_rx_timing(s32 a, s32 b, s64 freq, int def)
 
 /* Calculate the the delay value for termination enable of clock lane HS Rx */
 static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
-				 struct cio2_csi2_timing *timing)
+				 struct cio2_csi2_timing *timing,
+				 unsigned int bpp, unsigned int lanes)
 {
 	struct device *dev = &cio2->pci_dev->dev;
-	struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
-	struct v4l2_ctrl *link_freq;
 	s64 freq;
-	int r;
 
 	if (!q->sensor)
 		return -ENODEV;
 
-	link_freq = v4l2_ctrl_find(q->sensor->ctrl_handler, V4L2_CID_LINK_FREQ);
-	if (!link_freq) {
-		dev_err(dev, "failed to find LINK_FREQ\n");
-		return -EPIPE;
-	}
-
-	qm.index = v4l2_ctrl_g_ctrl(link_freq);
-	r = v4l2_querymenu(q->sensor->ctrl_handler, &qm);
-	if (r) {
-		dev_err(dev, "failed to get menu item\n");
-		return r;
-	}
-
-	if (!qm.value) {
-		dev_err(dev, "error invalid link_freq\n");
-		return -EINVAL;
+	freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
+	if (freq < 0) {
+		dev_err(dev, "error %lld, invalid link_freq\n", freq);
+		return freq;
 	}
-	freq = qm.value;
 
 	timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
 					    CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
@@ -364,7 +354,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
 
 	lanes = q->csi2.lanes;
 
-	r = cio2_csi2_calc_timing(cio2, q, &timing);
+	r = cio2_csi2_calc_timing(cio2, q, &timing, fmt->bpp, lanes);
 	if (r)
 		return r;
 
-- 
2.27.0

Re: [PATCH v2 2/2] ipu3-cio2: Use v4l2_get_link_freq helper

[Laurent Pinchart]

Hi Sakari,

Replying to an old thread.

On Wed, Oct 14, 2020 at 11:30:15AM +0300, Sakari Ailus wrote:
> Use v4l2_get_link_freq helper and add support for sensor drivers
> implementing only V4L2_CID_PIXEL_RATE.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> since v1:
> 
> - Use %lld for printing long long int
> 
> - Remove r (unused variable)
> 
>  drivers/media/pci/intel/ipu3/ipu3-cio2.c | 34 +++++++++---------------
>  1 file changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> index c557d189200b..d060cfe473d8 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> @@ -33,6 +33,7 @@ struct ipu3_cio2_fmt {
>  	u32 mbus_code;
>  	u32 fourcc;
>  	u8 mipicode;
> +	u8 bpp;
>  };
>  
>  /*
> @@ -46,18 +47,22 @@ static const struct ipu3_cio2_fmt formats[] = {
>  		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
>  		.fourcc		= V4L2_PIX_FMT_IPU3_SGRBG10,
>  		.mipicode	= 0x2b,
> +		.bpp		= 10,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
>  		.fourcc		= V4L2_PIX_FMT_IPU3_SGBRG10,
>  		.mipicode	= 0x2b,
> +		.bpp		= 10,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
>  		.fourcc		= V4L2_PIX_FMT_IPU3_SBGGR10,
>  		.mipicode	= 0x2b,
> +		.bpp		= 10,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
>  		.fourcc		= V4L2_PIX_FMT_IPU3_SRGGB10,
>  		.mipicode	= 0x2b,
> +		.bpp		= 10,
>  	},
>  };
>  
> @@ -288,35 +293,20 @@ static s32 cio2_rx_timing(s32 a, s32 b, s64 freq, int def)
>  
>  /* Calculate the the delay value for termination enable of clock lane HS Rx */
>  static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
> -				 struct cio2_csi2_timing *timing)
> +				 struct cio2_csi2_timing *timing,
> +				 unsigned int bpp, unsigned int lanes)
>  {
>  	struct device *dev = &cio2->pci_dev->dev;
> -	struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
> -	struct v4l2_ctrl *link_freq;
>  	s64 freq;
> -	int r;
>  
>  	if (!q->sensor)
>  		return -ENODEV;
>  
> -	link_freq = v4l2_ctrl_find(q->sensor->ctrl_handler, V4L2_CID_LINK_FREQ);
> -	if (!link_freq) {
> -		dev_err(dev, "failed to find LINK_FREQ\n");
> -		return -EPIPE;
> -	}
> -
> -	qm.index = v4l2_ctrl_g_ctrl(link_freq);
> -	r = v4l2_querymenu(q->sensor->ctrl_handler, &qm);
> -	if (r) {
> -		dev_err(dev, "failed to get menu item\n");
> -		return r;
> -	}
> -
> -	if (!qm.value) {
> -		dev_err(dev, "error invalid link_freq\n");
> -		return -EINVAL;
> +	freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);

Shouldn't this use lanes * 2 ?

> +	if (freq < 0) {
> +		dev_err(dev, "error %lld, invalid link_freq\n", freq);
> +		return freq;
>  	}
> -	freq = qm.value;
>  
>  	timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
>  					    CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
> @@ -364,7 +354,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
>  
>  	lanes = q->csi2.lanes;
>  
> -	r = cio2_csi2_calc_timing(cio2, q, &timing);
> +	r = cio2_csi2_calc_timing(cio2, q, &timing, fmt->bpp, lanes);
>  	if (r)
>  		return r;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/2] ipu3-cio2: Use v4l2_get_link_freq helper

[Sakari Ailus]

Hi Laurent,

On Mon, Feb 15, 2021 at 02:12:23AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Replying to an old thread.
> 
> On Wed, Oct 14, 2020 at 11:30:15AM +0300, Sakari Ailus wrote:
> > Use v4l2_get_link_freq helper and add support for sensor drivers
> > implementing only V4L2_CID_PIXEL_RATE.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > since v1:
> > 
> > - Use %lld for printing long long int
> > 
> > - Remove r (unused variable)
> > 
> >  drivers/media/pci/intel/ipu3/ipu3-cio2.c | 34 +++++++++---------------
> >  1 file changed, 12 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> > index c557d189200b..d060cfe473d8 100644
> > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> > @@ -33,6 +33,7 @@ struct ipu3_cio2_fmt {
> >  	u32 mbus_code;
> >  	u32 fourcc;
> >  	u8 mipicode;
> > +	u8 bpp;
> >  };
> >  
> >  /*
> > @@ -46,18 +47,22 @@ static const struct ipu3_cio2_fmt formats[] = {
> >  		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
> >  		.fourcc		= V4L2_PIX_FMT_IPU3_SGRBG10,
> >  		.mipicode	= 0x2b,
> > +		.bpp		= 10,
> >  	}, {
> >  		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
> >  		.fourcc		= V4L2_PIX_FMT_IPU3_SGBRG10,
> >  		.mipicode	= 0x2b,
> > +		.bpp		= 10,
> >  	}, {
> >  		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
> >  		.fourcc		= V4L2_PIX_FMT_IPU3_SBGGR10,
> >  		.mipicode	= 0x2b,
> > +		.bpp		= 10,
> >  	}, {
> >  		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
> >  		.fourcc		= V4L2_PIX_FMT_IPU3_SRGGB10,
> >  		.mipicode	= 0x2b,
> > +		.bpp		= 10,
> >  	},
> >  };
> >  
> > @@ -288,35 +293,20 @@ static s32 cio2_rx_timing(s32 a, s32 b, s64 freq, int def)
> >  
> >  /* Calculate the the delay value for termination enable of clock lane HS Rx */
> >  static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
> > -				 struct cio2_csi2_timing *timing)
> > +				 struct cio2_csi2_timing *timing,
> > +				 unsigned int bpp, unsigned int lanes)
> >  {
> >  	struct device *dev = &cio2->pci_dev->dev;
> > -	struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
> > -	struct v4l2_ctrl *link_freq;
> >  	s64 freq;
> > -	int r;
> >  
> >  	if (!q->sensor)
> >  		return -ENODEV;
> >  
> > -	link_freq = v4l2_ctrl_find(q->sensor->ctrl_handler, V4L2_CID_LINK_FREQ);
> > -	if (!link_freq) {
> > -		dev_err(dev, "failed to find LINK_FREQ\n");
> > -		return -EPIPE;
> > -	}
> > -
> > -	qm.index = v4l2_ctrl_g_ctrl(link_freq);
> > -	r = v4l2_querymenu(q->sensor->ctrl_handler, &qm);
> > -	if (r) {
> > -		dev_err(dev, "failed to get menu item\n");
> > -		return r;
> > -	}
> > -
> > -	if (!qm.value) {
> > -		dev_err(dev, "error invalid link_freq\n");
> > -		return -EINVAL;
> > +	freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
> 
> Shouldn't this use lanes * 2 ?

Yes, it should. Good catch!

I'll send a patch...

> 
> > +	if (freq < 0) {
> > +		dev_err(dev, "error %lld, invalid link_freq\n", freq);
> > +		return freq;
> >  	}
> > -	freq = qm.value;
> >  
> >  	timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
> >  					    CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
> > @@ -364,7 +354,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
> >  
> >  	lanes = q->csi2.lanes;
> >  
> > -	r = cio2_csi2_calc_timing(cio2, q, &timing);
> > +	r = cio2_csi2_calc_timing(cio2, q, &timing, fmt->bpp, lanes);
> >  	if (r)
> >  		return r;
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Sakari Ailus

Re: [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4218 bytes --]

Hi Sakari,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20201015]
[cannot apply to v5.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-a012-20201015 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project e7b4feea8e1bf520b34ad8c116abab6677344b74)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/cf4ab5e39eb042b02f1d5660b5cbd88197a05520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
        git checkout cf4ab5e39eb042b02f1d5660b5cbd88197a05520
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:50: warning: format specifies type 'long' but the argument has type 's64' (aka 'long long') [-Wformat]
                   dev_err(dev, "error %ld, invalid link_freq\n", freq);
                                       ~~~                        ^~~~
                                       %lld
   include/linux/dev_printk.h:104:32: note: expanded from macro 'dev_err'
           _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
                                 ~~~     ^~~~~~~~~~~
   drivers/media/pci/intel/ipu3/ipu3-cio2.c:301:6: warning: unused variable 'r' [-Wunused-variable]
           int r;
               ^
   2 warnings generated.

vim +308 drivers/media/pci/intel/ipu3/ipu3-cio2.c

   293	
   294	/* Calculate the the delay value for termination enable of clock lane HS Rx */
   295	static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
   296					 struct cio2_csi2_timing *timing,
   297					 unsigned int bpp, unsigned int lanes)
   298	{
   299		struct device *dev = &cio2->pci_dev->dev;
   300		s64 freq;
   301		int r;
   302	
   303		if (!q->sensor)
   304			return -ENODEV;
   305	
   306		freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
   307		if (freq < 0) {
 > 308			dev_err(dev, "error %ld, invalid link_freq\n", freq);
   309			return freq;
   310		}
   311	
   312		timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
   313						    CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
   314						    freq,
   315						    CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
   316		timing->clk_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_A,
   317						    CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_B,
   318						    freq,
   319						    CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
   320		timing->dat_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_A,
   321						    CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_B,
   322						    freq,
   323						    CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
   324		timing->dat_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_A,
   325						    CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_B,
   326						    freq,
   327						    CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
   328	
   329		dev_dbg(dev, "freq ct value is %d\n", timing->clk_termen);
   330		dev_dbg(dev, "freq cs value is %d\n", timing->clk_settle);
   331		dev_dbg(dev, "freq dt value is %d\n", timing->dat_termen);
   332		dev_dbg(dev, "freq ds value is %d\n", timing->dat_settle);
   333	
   334		return 0;
   335	};
   336	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33706 bytes --]

Re: [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4314 bytes --]

Hi Sakari,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20201015]
[cannot apply to v5.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-a012-20201015 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project e7b4feea8e1bf520b34ad8c116abab6677344b74)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/cf4ab5e39eb042b02f1d5660b5cbd88197a05520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
        git checkout cf4ab5e39eb042b02f1d5660b5cbd88197a05520
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:50: warning: format specifies type 'long' but the argument has type 's64' (aka 'long long') [-Wformat]
                   dev_err(dev, "error %ld, invalid link_freq\n", freq);
                                       ~~~                        ^~~~
                                       %lld
   include/linux/dev_printk.h:104:32: note: expanded from macro 'dev_err'
           _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
                                 ~~~     ^~~~~~~~~~~
   drivers/media/pci/intel/ipu3/ipu3-cio2.c:301:6: warning: unused variable 'r' [-Wunused-variable]
           int r;
               ^
   2 warnings generated.

vim +308 drivers/media/pci/intel/ipu3/ipu3-cio2.c

   293	
   294	/* Calculate the the delay value for termination enable of clock lane HS Rx */
   295	static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
   296					 struct cio2_csi2_timing *timing,
   297					 unsigned int bpp, unsigned int lanes)
   298	{
   299		struct device *dev = &cio2->pci_dev->dev;
   300		s64 freq;
   301		int r;
   302	
   303		if (!q->sensor)
   304			return -ENODEV;
   305	
   306		freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
   307		if (freq < 0) {
 > 308			dev_err(dev, "error %ld, invalid link_freq\n", freq);
   309			return freq;
   310		}
   311	
   312		timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
   313						    CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
   314						    freq,
   315						    CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
   316		timing->clk_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_A,
   317						    CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_B,
   318						    freq,
   319						    CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
   320		timing->dat_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_A,
   321						    CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_B,
   322						    freq,
   323						    CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
   324		timing->dat_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_A,
   325						    CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_B,
   326						    freq,
   327						    CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
   328	
   329		dev_dbg(dev, "freq ct value is %d\n", timing->clk_termen);
   330		dev_dbg(dev, "freq cs value is %d\n", timing->clk_settle);
   331		dev_dbg(dev, "freq dt value is %d\n", timing->dat_termen);
   332		dev_dbg(dev, "freq ds value is %d\n", timing->dat_settle);
   333	
   334		return 0;
   335	};
   336	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33706 bytes --]