[PATCH v3 00/11] CCS PLL fixes and improvements

Linux Media Controller development
 help / color / mirror / Atom feed

* [PATCH v3 00/11] CCS PLL fixes and improvements
@ 2025-04-23 12:33 Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 01/11] media: ccs-pll: Start OP pre-PLL multiplier search from correct value Sakari Ailus
                   ` (10 more replies)
  0 siblings, 11 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

Hi all,

Here are a few fixes and improvements to the CCS PLL calculator.

v2 is <20250417065354.311617-1-sakari.ailus@linux.intel.com>.

v1 is <20250226132319.2517656-1-sakari.ailus@linux.intel.com>.

since v2:

- Reword commit messages based on review comments.

- Move the even PLL multiplier flag next to other non-CCS PLL flags.

- Small code readability improvements.

since v1:

- Added patches for the following:

	- Drop the now-unused LINK_DECOUPLED flag.

	- Print missing flags.

	- Print PLL calculator flags before running the calculator
	  (earlier version posted separately already).

	- Document the CCS PLL flags tersely but still in a way that is
	  useful.

- Rebased other patches to clean up things first.

Sakari Ailus (11):
  media: ccs-pll: Start OP pre-PLL multiplier search from correct value
  media: ccs-pll: Start VT pre-PLL multiplier search from correct value
  media: ccs-pll: Check for too high VT PLL multiplier in dual PLL case
  media: ccs-pll: Correct the upper limit of maximum op_pre_pll_clk_div
  media: ccs-pll: Print a debug message on too high VT PLL OP clock
  media: ccs-pll: Drop LINK_DECOUPLED flag
  media: ccs-pll: Print missing PLL flags
  media: ccs-pll: Add a flag for even PLL multipliers
  media: ccs-pll: Better validate VT PLL branch
  media: ccs-pll: Print PLL calculator flags in the beginning
  media: ccs-pll: Document the CCS PLL flags

 drivers/media/i2c/ccs-pll.c       | 49 +++++++++++++++++++++++++------
 drivers/media/i2c/ccs-pll.h       | 29 +++++++++++++++---
 drivers/media/i2c/ccs/ccs-core.c  |  1 -
 drivers/media/i2c/ccs/ccs-quirk.c |  3 +-
 4 files changed, 66 insertions(+), 16 deletions(-)

-- 
2.39.5


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 01/11] media: ccs-pll: Start OP pre-PLL multiplier search from correct value
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 02/11] media: ccs-pll: Start VT " Sakari Ailus
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

The ccs_pll_calculate() function does a search over possible PLL
configurations to find the "best" one. If the sensor does not support odd
pre-PLL divisors and the minimum value (with constraints) isn't 1, other
odd values could be errorneously searched (and selected) for the pre-PLL
divisor. Fix this.

Fixes: 415ddd993978 ("media: ccs-pll: Split limits and PLL configuration into front and back parts")
Cc: stable@vger.kernel.org
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 34ccda666524..e516ed23e899 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -815,6 +815,8 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
 			      one_or_more(
 				      DIV_ROUND_UP(op_lim_fr->max_pll_op_clk_freq_hz,
 						   pll->ext_clk_freq_hz))));
+	if (!(pll->flags & CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER))
+		min_op_pre_pll_clk_div = clk_div_even(min_op_pre_pll_clk_div);
 	dev_dbg(dev, "pll_op check: min / max op_pre_pll_clk_div: %u / %u\n",
 		min_op_pre_pll_clk_div, max_op_pre_pll_clk_div);
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 02/11] media: ccs-pll: Start VT pre-PLL multiplier search from correct value
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 01/11] media: ccs-pll: Start OP pre-PLL multiplier search from correct value Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 03/11] media: ccs-pll: Check for too high VT PLL multiplier in dual PLL case Sakari Ailus
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

The ccs_pll_calculate_vt_tree() function does a search over possible VT
PLL configurations to find the "best" one. If the sensor does not support
odd pre-PLL divisors and the minimum value (with constraints) isn't 1,
other odd values could be errorneously searched (and selected) for the
pre-PLL divisor. Fix this.

Fixes: 415ddd993978 ("media: ccs-pll: Split limits and PLL configuration into front and back parts")
Cc: stable@vger.kernel.org
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index e516ed23e899..2399cd6509b7 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -397,6 +397,8 @@ static int ccs_pll_calculate_vt_tree(struct device *dev,
 	min_pre_pll_clk_div = max_t(u16, min_pre_pll_clk_div,
 				    pll->ext_clk_freq_hz /
 				    lim_fr->max_pll_ip_clk_freq_hz);
+	if (!(pll->flags & CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER))
+		min_pre_pll_clk_div = clk_div_even(min_pre_pll_clk_div);
 
 	dev_dbg(dev, "vt min/max_pre_pll_clk_div: %u,%u\n",
 		min_pre_pll_clk_div, max_pre_pll_clk_div);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 03/11] media: ccs-pll: Check for too high VT PLL multiplier in dual PLL case
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 01/11] media: ccs-pll: Start OP pre-PLL multiplier search from correct value Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 02/11] media: ccs-pll: Start VT " Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 04/11] media: ccs-pll: Correct the upper limit of maximum op_pre_pll_clk_div Sakari Ailus
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

The check for VT PLL upper limit in dual PLL case was missing. Add it now.

Fixes: 6c7469e46b60 ("media: ccs-pll: Add trivial dual PLL support")
Cc: stable@vger.kernel.org
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 2399cd6509b7..266fcd160da6 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -312,6 +312,11 @@ __ccs_pll_calculate_vt_tree(struct device *dev,
 	dev_dbg(dev, "more_mul2: %u\n", more_mul);
 
 	pll_fr->pll_multiplier = mul * more_mul;
+	if (pll_fr->pll_multiplier > lim_fr->max_pll_multiplier) {
+		dev_dbg(dev, "pll multiplier %u too high\n",
+			pll_fr->pll_multiplier);
+		return -EINVAL;
+	}
 
 	if (pll_fr->pll_multiplier * pll_fr->pll_ip_clk_freq_hz >
 	    lim_fr->max_pll_op_clk_freq_hz)
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 04/11] media: ccs-pll: Correct the upper limit of maximum op_pre_pll_clk_div
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
                   ` (2 preceding siblings ...)
  2025-04-23 12:33 ` [PATCH v3 03/11] media: ccs-pll: Check for too high VT PLL multiplier in dual PLL case Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 05/11] media: ccs-pll: Print a debug message on too high VT PLL OP clock Sakari Ailus
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

The PLL calculator does a search of the PLL configuration space for all
valid OP pre-PLL clock dividers. The maximum did not take into account the
CCS PLL flag CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER in which case also odd PLL
dividers (other than 1) are valid. Do that now.

Fixes: 4e1e8d240dff ("media: ccs-pll: Add support for extended input PLL clock divider")
Cc: stable@vger.kernel.org
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 266fcd160da6..d985686b0a36 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -799,7 +799,7 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
 		op_lim_fr->min_pre_pll_clk_div, op_lim_fr->max_pre_pll_clk_div);
 	max_op_pre_pll_clk_div =
 		min_t(u16, op_lim_fr->max_pre_pll_clk_div,
-		      clk_div_even(pll->ext_clk_freq_hz /
+		      DIV_ROUND_UP(pll->ext_clk_freq_hz,
 				   op_lim_fr->min_pll_ip_clk_freq_hz));
 	min_op_pre_pll_clk_div =
 		max_t(u16, op_lim_fr->min_pre_pll_clk_div,
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 05/11] media: ccs-pll: Print a debug message on too high VT PLL OP clock
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
                   ` (3 preceding siblings ...)
  2025-04-23 12:33 ` [PATCH v3 04/11] media: ccs-pll: Correct the upper limit of maximum op_pre_pll_clk_div Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 06/11] media: ccs-pll: Drop LINK_DECOUPLED flag Sakari Ailus
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

In general the CCS PLL calculator prints debugging information on the
process to ease debugging. This case was not annotated, do that now.

Remove an extra multiplication while at it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index d985686b0a36..66d046d576f7 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -318,12 +318,13 @@ __ccs_pll_calculate_vt_tree(struct device *dev,
 		return -EINVAL;
 	}
 
-	if (pll_fr->pll_multiplier * pll_fr->pll_ip_clk_freq_hz >
-	    lim_fr->max_pll_op_clk_freq_hz)
-		return -EINVAL;
-
 	pll_fr->pll_op_clk_freq_hz =
 		pll_fr->pll_ip_clk_freq_hz * pll_fr->pll_multiplier;
+	if (pll_fr->pll_op_clk_freq_hz > lim_fr->max_pll_op_clk_freq_hz) {
+		dev_dbg(dev, "too high OP clock %u\n",
+			pll_fr->pll_op_clk_freq_hz);
+		return -EINVAL;
+	}
 
 	vt_div = div * more_mul;
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 06/11] media: ccs-pll: Drop LINK_DECOUPLED flag
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
                   ` (4 preceding siblings ...)
  2025-04-23 12:33 ` [PATCH v3 05/11] media: ccs-pll: Print a debug message on too high VT PLL OP clock Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 07/11] media: ccs-pll: Print missing PLL flags Sakari Ailus
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

The LINK_DECOUPLED flag isn't used by the PLL calculator other than
printing it. The number of OP/VT lanes are already printed in any case.
Thus drop the flag as it has no function.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c       | 3 +--
 drivers/media/i2c/ccs-pll.h       | 1 -
 drivers/media/i2c/ccs/ccs-core.c  | 1 -
 drivers/media/i2c/ccs/ccs-quirk.c | 3 +--
 4 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 66d046d576f7..16eb09462c8b 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -124,9 +124,8 @@ static void print_pll(struct device *dev, const struct ccs_pll *pll)
 	dev_dbg(dev, "pixel rate on CSI-2 bus:\t%u\n",
 		pll->pixel_rate_csi);
 
-	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s%s\n",
+	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s\n",
 		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
-		pll->flags & PLL_FL(LINK_DECOUPLED) ? " link-decoupled" : "",
 		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
 		" ext-ip-pll-divider" : "",
 		pll->flags & PLL_FL(FLEXIBLE_OP_PIX_CLK_DIV) ?
diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
index 6eb1b1c68e1e..ee206e5b287b 100644
--- a/drivers/media/i2c/ccs-pll.h
+++ b/drivers/media/i2c/ccs-pll.h
@@ -24,7 +24,6 @@
 #define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
 /* CCS PLL flags */
 #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)
-#define CCS_PLL_FLAG_LINK_DECOUPLED				BIT(3)
 #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)
 #define CCS_PLL_FLAG_FLEXIBLE_OP_PIX_CLK_DIV			BIT(5)
 #define CCS_PLL_FLAG_FIFO_DERATING				BIT(6)
diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 004d28c33287..06e0ba53f2a8 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -3451,7 +3451,6 @@ static int ccs_probe(struct i2c_client *client)
 				CCS_LIM(sensor, NUM_OF_VT_LANES) + 1;
 			sensor->pll.op_lanes =
 				CCS_LIM(sensor, NUM_OF_OP_LANES) + 1;
-			sensor->pll.flags |= CCS_PLL_FLAG_LINK_DECOUPLED;
 		} else {
 			sensor->pll.vt_lanes = sensor->pll.csi2.lanes;
 			sensor->pll.op_lanes = sensor->pll.csi2.lanes;
diff --git a/drivers/media/i2c/ccs/ccs-quirk.c b/drivers/media/i2c/ccs/ccs-quirk.c
index e3d4c7a275bc..e48a4fa1f5dd 100644
--- a/drivers/media/i2c/ccs/ccs-quirk.c
+++ b/drivers/media/i2c/ccs/ccs-quirk.c
@@ -190,8 +190,7 @@ static int jt8ev1_post_streamoff(struct ccs_sensor *sensor)
 
 static int jt8ev1_init(struct ccs_sensor *sensor)
 {
-	sensor->pll.flags |= CCS_PLL_FLAG_LANE_SPEED_MODEL |
-		CCS_PLL_FLAG_LINK_DECOUPLED;
+	sensor->pll.flags |= CCS_PLL_FLAG_LANE_SPEED_MODEL;
 	sensor->pll.vt_lanes = 1;
 	sensor->pll.op_lanes = sensor->pll.csi2.lanes;
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 07/11] media: ccs-pll: Print missing PLL flags
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
                   ` (5 preceding siblings ...)
  2025-04-23 12:33 ` [PATCH v3 06/11] media: ccs-pll: Drop LINK_DECOUPLED flag Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 08/11] media: ccs-pll: Add a flag for even PLL multipliers Sakari Ailus
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

Printing the OP_PIX_CLOCK_PER_LANE and NO_OP_CLOCKS CCS PLL flags is
missing, add them now.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 16eb09462c8b..ebbc5e323244 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -124,7 +124,9 @@ static void print_pll(struct device *dev, const struct ccs_pll *pll)
 	dev_dbg(dev, "pixel rate on CSI-2 bus:\t%u\n",
 		pll->pixel_rate_csi);
 
-	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s\n",
+	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s%s%s\n",
+		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
+		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
 		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
 		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
 		" ext-ip-pll-divider" : "",
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 08/11] media: ccs-pll: Add a flag for even PLL multipliers
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
                   ` (6 preceding siblings ...)
  2025-04-23 12:33 ` [PATCH v3 07/11] media: ccs-pll: Print missing PLL flags Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 21:49   ` Laurent Pinchart
  2025-04-23 12:33 ` [PATCH v3 09/11] media: ccs-pll: Better validate VT PLL branch Sakari Ailus
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

Some devices (not entirely CCS compliant) only support even PLL
multipliers. Add support for this through a PLL flag.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs-pll.c | 11 ++++++++++-
 drivers/media/i2c/ccs-pll.h |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index ebbc5e323244..f4be4e9f6777 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -124,9 +124,10 @@ static void print_pll(struct device *dev, const struct ccs_pll *pll)
 	dev_dbg(dev, "pixel rate on CSI-2 bus:\t%u\n",
 		pll->pixel_rate_csi);
 
-	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s%s%s\n",
+	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s%s%s%s\n",
 		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
 		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
+		pll->flags & PLL_FL(EVEN_PLL_MULTIPLIER) ? " even-pll-multiplier" : "",
 		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
 		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
 		" ext-ip-pll-divider" : "",
@@ -312,6 +313,10 @@ __ccs_pll_calculate_vt_tree(struct device *dev,
 	more_mul *= DIV_ROUND_UP(lim_fr->min_pll_multiplier, mul * more_mul);
 	dev_dbg(dev, "more_mul2: %u\n", more_mul);
 
+	if (pll->flags & CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER &&
+	    (mul & 1) && (more_mul & 1))
+		more_mul <<= 1;
+
 	pll_fr->pll_multiplier = mul * more_mul;
 	if (pll_fr->pll_multiplier > lim_fr->max_pll_multiplier) {
 		dev_dbg(dev, "pll multiplier %u too high\n",
@@ -668,6 +673,10 @@ ccs_pll_calculate_op(struct device *dev, const struct ccs_pll_limits *lim,
 	if (!is_one_or_even(i))
 		i <<= 1;
 
+	if (pll->flags & CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER &&
+	    mul & 1 && i & 1)
+		i <<= 1;
+
 	dev_dbg(dev, "final more_mul: %u\n", i);
 	if (i > more_mul_max) {
 		dev_dbg(dev, "final more_mul is bad, max %u\n", more_mul_max);
diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
index ee206e5b287b..754eb5f52cc4 100644
--- a/drivers/media/i2c/ccs-pll.h
+++ b/drivers/media/i2c/ccs-pll.h
@@ -22,6 +22,7 @@
 /* op pix clock is for all lanes in total normally */
 #define CCS_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			BIT(0)
 #define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
+#define CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER			BIT(3)
 /* CCS PLL flags */
 #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)
 #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 08/11] media: ccs-pll: Add a flag for even PLL multipliers
  2025-04-23 12:33 ` [PATCH v3 08/11] media: ccs-pll: Add a flag for even PLL multipliers Sakari Ailus
@ 2025-04-23 21:49   ` Laurent Pinchart
  0 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2025-04-23 21:49 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, dongcheng.yan

Hi Sakari,

Thank you for the patch.

On Wed, Apr 23, 2025 at 03:33:56PM +0300, Sakari Ailus wrote:
> Some devices (not entirely CCS compliant) only support even PLL
> multipliers. Add support for this through a PLL flag.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ccs-pll.c | 11 ++++++++++-
>  drivers/media/i2c/ccs-pll.h |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
> index ebbc5e323244..f4be4e9f6777 100644
> --- a/drivers/media/i2c/ccs-pll.c
> +++ b/drivers/media/i2c/ccs-pll.c
> @@ -124,9 +124,10 @@ static void print_pll(struct device *dev, const struct ccs_pll *pll)
>  	dev_dbg(dev, "pixel rate on CSI-2 bus:\t%u\n",
>  		pll->pixel_rate_csi);
>  
> -	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s%s%s\n",
> +	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s%s%s%s\n",
>  		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
>  		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
> +		pll->flags & PLL_FL(EVEN_PLL_MULTIPLIER) ? " even-pll-multiplier" : "",
>  		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
>  		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
>  		" ext-ip-pll-divider" : "",
> @@ -312,6 +313,10 @@ __ccs_pll_calculate_vt_tree(struct device *dev,
>  	more_mul *= DIV_ROUND_UP(lim_fr->min_pll_multiplier, mul * more_mul);
>  	dev_dbg(dev, "more_mul2: %u\n", more_mul);
>  
> +	if (pll->flags & CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER &&
> +	    (mul & 1) && (more_mul & 1))
> +		more_mul <<= 1;
> +
>  	pll_fr->pll_multiplier = mul * more_mul;
>  	if (pll_fr->pll_multiplier > lim_fr->max_pll_multiplier) {
>  		dev_dbg(dev, "pll multiplier %u too high\n",
> @@ -668,6 +673,10 @@ ccs_pll_calculate_op(struct device *dev, const struct ccs_pll_limits *lim,
>  	if (!is_one_or_even(i))
>  		i <<= 1;
>  
> +	if (pll->flags & CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER &&
> +	    mul & 1 && i & 1)
> +		i <<= 1;
> +
>  	dev_dbg(dev, "final more_mul: %u\n", i);
>  	if (i > more_mul_max) {
>  		dev_dbg(dev, "final more_mul is bad, max %u\n", more_mul_max);
> diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
> index ee206e5b287b..754eb5f52cc4 100644
> --- a/drivers/media/i2c/ccs-pll.h
> +++ b/drivers/media/i2c/ccs-pll.h
> @@ -22,6 +22,7 @@
>  /* op pix clock is for all lanes in total normally */
>  #define CCS_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			BIT(0)
>  #define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> +#define CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER			BIT(3)
>  /* CCS PLL flags */
>  #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)

I would probably have reordered the bits, but I don't mind much.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 09/11] media: ccs-pll: Better validate VT PLL branch
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
                   ` (7 preceding siblings ...)
  2025-04-23 12:33 ` [PATCH v3 08/11] media: ccs-pll: Add a flag for even PLL multipliers Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 10/11] media: ccs-pll: Print PLL calculator flags in the beginning Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 11/11] media: ccs-pll: Document the CCS PLL flags Sakari Ailus
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

Check that the VT PLL dividers are actually found, don't trust they always
are even though they should be.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index f4be4e9f6777..8de8158a413b 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -449,7 +449,7 @@ static int ccs_pll_calculate_vt_tree(struct device *dev,
 	return -EINVAL;
 }
 
-static void
+static int
 ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 		     const struct ccs_pll_branch_limits_bk *op_lim_bk,
 		     struct ccs_pll *pll, struct ccs_pll_branch_fr *pll_fr,
@@ -572,6 +572,8 @@ ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 		if (best_pix_div < SHRT_MAX >> 1)
 			break;
 	}
+	if (best_pix_div == SHRT_MAX >> 1)
+		return -EINVAL;
 
 	pll->vt_bk.sys_clk_div = DIV_ROUND_UP(vt_div, best_pix_div);
 	pll->vt_bk.pix_clk_div = best_pix_div;
@@ -584,6 +586,8 @@ ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 out_calc_pixel_rate:
 	pll->pixel_rate_pixel_array =
 		pll->vt_bk.pix_clk_freq_hz * pll->vt_lanes;
+
+	return 0;
 }
 
 /*
@@ -863,8 +867,10 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
 		if (pll->flags & CCS_PLL_FLAG_DUAL_PLL)
 			break;
 
-		ccs_pll_calculate_vt(dev, lim, op_lim_bk, pll, op_pll_fr,
-				     op_pll_bk, cphy, phy_const);
+		rval = ccs_pll_calculate_vt(dev, lim, op_lim_bk, pll, op_pll_fr,
+					    op_pll_bk, cphy, phy_const);
+		if (rval)
+			continue;
 
 		rval = check_bk_bounds(dev, lim, pll, PLL_VT);
 		if (rval)
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 10/11] media: ccs-pll: Print PLL calculator flags in the beginning
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
                   ` (8 preceding siblings ...)
  2025-04-23 12:33 ` [PATCH v3 09/11] media: ccs-pll: Better validate VT PLL branch Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:33 ` [PATCH v3 11/11] media: ccs-pll: Document the CCS PLL flags Sakari Ailus
  10 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

Print the PLL calculator flags right away when the PLL calculator is
called. Previously this was done only in a successful case and that didn't
really help solving a problem when one happened.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs-pll.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 8de8158a413b..87798616b76d 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -123,8 +123,11 @@ static void print_pll(struct device *dev, const struct ccs_pll *pll)
 		pll->pixel_rate_pixel_array);
 	dev_dbg(dev, "pixel rate on CSI-2 bus:\t%u\n",
 		pll->pixel_rate_csi);
+}
 
-	dev_dbg(dev, "flags%s%s%s%s%s%s%s%s%s%s%s\n",
+static void print_pll_flags(struct device *dev, struct ccs_pll *pll)
+{
+	dev_dbg(dev, "PLL flags%s%s%s%s%s%s%s%s%s%s%s\n",
 		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
 		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
 		pll->flags & PLL_FL(EVEN_PLL_MULTIPLIER) ? " even-pll-multiplier" : "",
@@ -738,6 +741,8 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
 	u32 i;
 	int rval = -EINVAL;
 
+	print_pll_flags(dev, pll);
+
 	if (!(pll->flags & CCS_PLL_FLAG_LANE_SPEED_MODEL)) {
 		pll->op_lanes = 1;
 		pll->vt_lanes = 1;
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 11/11] media: ccs-pll: Document the CCS PLL flags
  2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
                   ` (9 preceding siblings ...)
  2025-04-23 12:33 ` [PATCH v3 10/11] media: ccs-pll: Print PLL calculator flags in the beginning Sakari Ailus
@ 2025-04-23 12:33 ` Sakari Ailus
  2025-04-23 12:43   ` Laurent Pinchart
  10 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:33 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, dongcheng.yan

Document the CCS PLL flags with short comments. The CCS spec has more
information on them while the added documentation helps finding the
relevant information in the CCS spec.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs-pll.c |  2 +-
 drivers/media/i2c/ccs-pll.h | 27 ++++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 87798616b76d..8f9a695bd9e5 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -129,8 +129,8 @@ static void print_pll_flags(struct device *dev, struct ccs_pll *pll)
 {
 	dev_dbg(dev, "PLL flags%s%s%s%s%s%s%s%s%s%s%s\n",
 		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
-		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
 		pll->flags & PLL_FL(EVEN_PLL_MULTIPLIER) ? " even-pll-multiplier" : "",
+		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
 		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
 		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
 		" ext-ip-pll-divider" : "",
diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
index 754eb5f52cc4..c668ee4d4493 100644
--- a/drivers/media/i2c/ccs-pll.h
+++ b/drivers/media/i2c/ccs-pll.h
@@ -18,19 +18,40 @@
 #define CCS_PLL_BUS_TYPE_CSI2_DPHY				0x00
 #define CCS_PLL_BUS_TYPE_CSI2_CPHY				0x01
 
-/* Old SMIA and implementation specific flags */
-/* op pix clock is for all lanes in total normally */
+/* Old SMIA and implementation specific flags. */
+/* OP PIX clock is for all lanes in total normally. */
 #define CCS_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			BIT(0)
-#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
+/* If set, the PLL multipliers are required to be even. */
 #define CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER			BIT(3)
+
 /* CCS PLL flags */
+
+/* The sensor doesn't have OP clocks at all. */
+#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
+/* System speed model if this flag is unset. */
 #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)
+/* If set, the pre-PLL divider may have odd values, too. */
 #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)
+/*
+ * If set, the OP PIX clock doesn't have to exactly match with data rate, it may
+ * be higher. See "OP Domain Formulas" in MIPI CCS 1.1 spec.
+ */
 #define CCS_PLL_FLAG_FLEXIBLE_OP_PIX_CLK_DIV			BIT(5)
+/* If set, the VT domain may run faster than the OP domain. */
 #define CCS_PLL_FLAG_FIFO_DERATING				BIT(6)
+/* If set, the VT domain may run slower than the OP domain. */
 #define CCS_PLL_FLAG_FIFO_OVERRATING				BIT(7)
+/* If set, the PLL tree has two PLLs instead of one. */
 #define CCS_PLL_FLAG_DUAL_PLL					BIT(8)
+/*
+ * If set, the OP SYS clock is a dual data rate clock, transferring two bits per
+ * cycle instead of one.
+ */
 #define CCS_PLL_FLAG_OP_SYS_DDR					BIT(9)
+/*
+ * If set, the OP PIX clock is a dual data rate clock, transferring two bits per
+ * cycle instead of one.
+ */
 #define CCS_PLL_FLAG_OP_PIX_DDR					BIT(10)
 
 /**
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 11/11] media: ccs-pll: Document the CCS PLL flags
  2025-04-23 12:33 ` [PATCH v3 11/11] media: ccs-pll: Document the CCS PLL flags Sakari Ailus
@ 2025-04-23 12:43   ` Laurent Pinchart
  2025-04-23 12:51     ` Sakari Ailus
  2025-04-23 15:37     ` [PATCH v4 " Sakari Ailus
  0 siblings, 2 replies; 17+ messages in thread
From: Laurent Pinchart @ 2025-04-23 12:43 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, dongcheng.yan

Hi Sakari,

Thank you for the patch.

On Wed, Apr 23, 2025 at 03:33:59PM +0300, Sakari Ailus wrote:
> Document the CCS PLL flags with short comments. The CCS spec has more
> information on them while the added documentation helps finding the
> relevant information in the CCS spec.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ccs-pll.c |  2 +-
>  drivers/media/i2c/ccs-pll.h | 27 ++++++++++++++++++++++++---
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
> index 87798616b76d..8f9a695bd9e5 100644
> --- a/drivers/media/i2c/ccs-pll.c
> +++ b/drivers/media/i2c/ccs-pll.c
> @@ -129,8 +129,8 @@ static void print_pll_flags(struct device *dev, struct ccs_pll *pll)
>  {
>  	dev_dbg(dev, "PLL flags%s%s%s%s%s%s%s%s%s%s%s\n",
>  		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
> -		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
>  		pll->flags & PLL_FL(EVEN_PLL_MULTIPLIER) ? " even-pll-multiplier" : "",
> +		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
>  		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
>  		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
>  		" ext-ip-pll-divider" : "",
> diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
> index 754eb5f52cc4..c668ee4d4493 100644
> --- a/drivers/media/i2c/ccs-pll.h
> +++ b/drivers/media/i2c/ccs-pll.h
> @@ -18,19 +18,40 @@
>  #define CCS_PLL_BUS_TYPE_CSI2_DPHY				0x00
>  #define CCS_PLL_BUS_TYPE_CSI2_CPHY				0x01
>  
> -/* Old SMIA and implementation specific flags */
> -/* op pix clock is for all lanes in total normally */
> +/* Old SMIA and implementation specific flags. */
> +/* OP PIX clock is for all lanes in total normally. */
>  #define CCS_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			BIT(0)
> -#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> +/* If set, the PLL multipliers are required to be even. */
>  #define CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER			BIT(3)
> +
>  /* CCS PLL flags */
> +
> +/* The sensor doesn't have OP clocks at all. */
> +#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> +/* System speed model if this flag is unset. */
>  #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)
> +/* If set, the pre-PLL divider may have odd values, too. */
>  #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)
> +/*
> + * If set, the OP PIX clock doesn't have to exactly match with data rate, it may
> + * be higher. See "OP Domain Formulas" in MIPI CCS 1.1 spec.
> + */
>  #define CCS_PLL_FLAG_FLEXIBLE_OP_PIX_CLK_DIV			BIT(5)
> +/* If set, the VT domain may run faster than the OP domain. */
>  #define CCS_PLL_FLAG_FIFO_DERATING				BIT(6)
> +/* If set, the VT domain may run slower than the OP domain. */
>  #define CCS_PLL_FLAG_FIFO_OVERRATING				BIT(7)
> +/* If set, the PLL tree has two PLLs instead of one. */
>  #define CCS_PLL_FLAG_DUAL_PLL					BIT(8)
> +/*
> + * If set, the OP SYS clock is a dual data rate clock, transferring two bits per
> + * cycle instead of one.
> + */
>  #define CCS_PLL_FLAG_OP_SYS_DDR					BIT(9)
> +/*
> + * If set, the OP PIX clock is a dual data rate clock, transferring two bits per

Same comment as for v2.

> + * cycle instead of one.
> + */
>  #define CCS_PLL_FLAG_OP_PIX_DDR					BIT(10)
>  
>  /**

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 11/11] media: ccs-pll: Document the CCS PLL flags
  2025-04-23 12:43   ` Laurent Pinchart
@ 2025-04-23 12:51     ` Sakari Ailus
  2025-04-23 15:37     ` [PATCH v4 " Sakari Ailus
  1 sibling, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 12:51 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media, dongcheng.yan

On Wed, Apr 23, 2025 at 03:43:22PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Wed, Apr 23, 2025 at 03:33:59PM +0300, Sakari Ailus wrote:
> > Document the CCS PLL flags with short comments. The CCS spec has more
> > information on them while the added documentation helps finding the
> > relevant information in the CCS spec.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/i2c/ccs-pll.c |  2 +-
> >  drivers/media/i2c/ccs-pll.h | 27 ++++++++++++++++++++++++---
> >  2 files changed, 25 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
> > index 87798616b76d..8f9a695bd9e5 100644
> > --- a/drivers/media/i2c/ccs-pll.c
> > +++ b/drivers/media/i2c/ccs-pll.c
> > @@ -129,8 +129,8 @@ static void print_pll_flags(struct device *dev, struct ccs_pll *pll)
> >  {
> >  	dev_dbg(dev, "PLL flags%s%s%s%s%s%s%s%s%s%s%s\n",
> >  		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
> > -		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
> >  		pll->flags & PLL_FL(EVEN_PLL_MULTIPLIER) ? " even-pll-multiplier" : "",
> > +		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
> >  		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
> >  		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
> >  		" ext-ip-pll-divider" : "",
> > diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
> > index 754eb5f52cc4..c668ee4d4493 100644
> > --- a/drivers/media/i2c/ccs-pll.h
> > +++ b/drivers/media/i2c/ccs-pll.h
> > @@ -18,19 +18,40 @@
> >  #define CCS_PLL_BUS_TYPE_CSI2_DPHY				0x00
> >  #define CCS_PLL_BUS_TYPE_CSI2_CPHY				0x01
> >  
> > -/* Old SMIA and implementation specific flags */
> > -/* op pix clock is for all lanes in total normally */
> > +/* Old SMIA and implementation specific flags. */
> > +/* OP PIX clock is for all lanes in total normally. */
> >  #define CCS_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			BIT(0)
> > -#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> > +/* If set, the PLL multipliers are required to be even. */
> >  #define CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER			BIT(3)
> > +
> >  /* CCS PLL flags */
> > +
> > +/* The sensor doesn't have OP clocks at all. */
> > +#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> > +/* System speed model if this flag is unset. */
> >  #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)
> > +/* If set, the pre-PLL divider may have odd values, too. */
> >  #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)
> > +/*
> > + * If set, the OP PIX clock doesn't have to exactly match with data rate, it may
> > + * be higher. See "OP Domain Formulas" in MIPI CCS 1.1 spec.
> > + */
> >  #define CCS_PLL_FLAG_FLEXIBLE_OP_PIX_CLK_DIV			BIT(5)
> > +/* If set, the VT domain may run faster than the OP domain. */
> >  #define CCS_PLL_FLAG_FIFO_DERATING				BIT(6)
> > +/* If set, the VT domain may run slower than the OP domain. */
> >  #define CCS_PLL_FLAG_FIFO_OVERRATING				BIT(7)
> > +/* If set, the PLL tree has two PLLs instead of one. */
> >  #define CCS_PLL_FLAG_DUAL_PLL					BIT(8)
> > +/*
> > + * If set, the OP SYS clock is a dual data rate clock, transferring two bits per
> > + * cycle instead of one.
> > + */
> >  #define CCS_PLL_FLAG_OP_SYS_DDR					BIT(9)
> > +/*
> > + * If set, the OP PIX clock is a dual data rate clock, transferring two bits per
> 
> Same comment as for v2.

Oh, this one I forgot. I'll post v4 in a moment.

> 
> > + * cycle instead of one.
> > + */
> >  #define CCS_PLL_FLAG_OP_PIX_DDR					BIT(10)
> >  
> >  /**
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 11/11] media: ccs-pll: Document the CCS PLL flags
  2025-04-23 12:43   ` Laurent Pinchart
  2025-04-23 12:51     ` Sakari Ailus
@ 2025-04-23 15:37     ` Sakari Ailus
  2025-04-23 17:50       ` Laurent Pinchart
  1 sibling, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2025-04-23 15:37 UTC (permalink / raw)
  To: linux-media, Laurent Pinchart; +Cc: dongcheng.yan

Document the CCS PLL flags with short comments. The CCS spec has more
information on them while the added documentation helps finding the
relevant information in the CCS spec.

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

- Refer to pixels in CCS_PLL_FLAG_OP_PIX_DDR comment.

 drivers/media/i2c/ccs-pll.c |  2 +-
 drivers/media/i2c/ccs-pll.h | 27 ++++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 87798616b76d..8f9a695bd9e5 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -129,8 +129,8 @@ static void print_pll_flags(struct device *dev, struct ccs_pll *pll)
 {
 	dev_dbg(dev, "PLL flags%s%s%s%s%s%s%s%s%s%s%s\n",
 		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
-		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
 		pll->flags & PLL_FL(EVEN_PLL_MULTIPLIER) ? " even-pll-multiplier" : "",
+		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
 		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
 		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
 		" ext-ip-pll-divider" : "",
diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
index 754eb5f52cc4..e22903931e72 100644
--- a/drivers/media/i2c/ccs-pll.h
+++ b/drivers/media/i2c/ccs-pll.h
@@ -18,19 +18,40 @@
 #define CCS_PLL_BUS_TYPE_CSI2_DPHY				0x00
 #define CCS_PLL_BUS_TYPE_CSI2_CPHY				0x01
 
-/* Old SMIA and implementation specific flags */
-/* op pix clock is for all lanes in total normally */
+/* Old SMIA and implementation specific flags. */
+/* OP PIX clock is for all lanes in total normally. */
 #define CCS_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			BIT(0)
-#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
+/* If set, the PLL multipliers are required to be even. */
 #define CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER			BIT(3)
+
 /* CCS PLL flags */
+
+/* The sensor doesn't have OP clocks at all. */
+#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
+/* System speed model if this flag is unset. */
 #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)
+/* If set, the pre-PLL divider may have odd values, too. */
 #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)
+/*
+ * If set, the OP PIX clock doesn't have to exactly match with data rate, it may
+ * be higher. See "OP Domain Formulas" in MIPI CCS 1.1 spec.
+ */
 #define CCS_PLL_FLAG_FLEXIBLE_OP_PIX_CLK_DIV			BIT(5)
+/* If set, the VT domain may run faster than the OP domain. */
 #define CCS_PLL_FLAG_FIFO_DERATING				BIT(6)
+/* If set, the VT domain may run slower than the OP domain. */
 #define CCS_PLL_FLAG_FIFO_OVERRATING				BIT(7)
+/* If set, the PLL tree has two PLLs instead of one. */
 #define CCS_PLL_FLAG_DUAL_PLL					BIT(8)
+/*
+ * If set, the OP SYS clock is a dual data rate clock, transferring two bits per
+ * cycle instead of one.
+ */
 #define CCS_PLL_FLAG_OP_SYS_DDR					BIT(9)
+/*
+ * If set, the OP PIX clock is a dual data rate clock, transferring two pixels
+ * per cycle instead of one.
+ */
 #define CCS_PLL_FLAG_OP_PIX_DDR					BIT(10)
 
 /**
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 11/11] media: ccs-pll: Document the CCS PLL flags
  2025-04-23 15:37     ` [PATCH v4 " Sakari Ailus
@ 2025-04-23 17:50       ` Laurent Pinchart
  0 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2025-04-23 17:50 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, dongcheng.yan

On Wed, Apr 23, 2025 at 06:37:19PM +0300, Sakari Ailus wrote:
> Document the CCS PLL flags with short comments. The CCS spec has more
> information on them while the added documentation helps finding the
> relevant information in the CCS spec.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> since v3:
> 
> - Refer to pixels in CCS_PLL_FLAG_OP_PIX_DDR comment.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  drivers/media/i2c/ccs-pll.c |  2 +-
>  drivers/media/i2c/ccs-pll.h | 27 ++++++++++++++++++++++++---
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
> index 87798616b76d..8f9a695bd9e5 100644
> --- a/drivers/media/i2c/ccs-pll.c
> +++ b/drivers/media/i2c/ccs-pll.c
> @@ -129,8 +129,8 @@ static void print_pll_flags(struct device *dev, struct ccs_pll *pll)
>  {
>  	dev_dbg(dev, "PLL flags%s%s%s%s%s%s%s%s%s%s%s\n",
>  		pll->flags & PLL_FL(OP_PIX_CLOCK_PER_LANE) ? " op-pix-clock-per-lane" : "",
> -		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
>  		pll->flags & PLL_FL(EVEN_PLL_MULTIPLIER) ? " even-pll-multiplier" : "",
> +		pll->flags & PLL_FL(NO_OP_CLOCKS) ? " no-op-clocks" : "",
>  		pll->flags & PLL_FL(LANE_SPEED_MODEL) ? " lane-speed" : "",
>  		pll->flags & PLL_FL(EXT_IP_PLL_DIVIDER) ?
>  		" ext-ip-pll-divider" : "",
> diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
> index 754eb5f52cc4..e22903931e72 100644
> --- a/drivers/media/i2c/ccs-pll.h
> +++ b/drivers/media/i2c/ccs-pll.h
> @@ -18,19 +18,40 @@
>  #define CCS_PLL_BUS_TYPE_CSI2_DPHY				0x00
>  #define CCS_PLL_BUS_TYPE_CSI2_CPHY				0x01
>  
> -/* Old SMIA and implementation specific flags */
> -/* op pix clock is for all lanes in total normally */
> +/* Old SMIA and implementation specific flags. */
> +/* OP PIX clock is for all lanes in total normally. */
>  #define CCS_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			BIT(0)
> -#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> +/* If set, the PLL multipliers are required to be even. */
>  #define CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER			BIT(3)
> +
>  /* CCS PLL flags */
> +
> +/* The sensor doesn't have OP clocks at all. */
> +#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> +/* System speed model if this flag is unset. */
>  #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)
> +/* If set, the pre-PLL divider may have odd values, too. */
>  #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)
> +/*
> + * If set, the OP PIX clock doesn't have to exactly match with data rate, it may
> + * be higher. See "OP Domain Formulas" in MIPI CCS 1.1 spec.
> + */
>  #define CCS_PLL_FLAG_FLEXIBLE_OP_PIX_CLK_DIV			BIT(5)
> +/* If set, the VT domain may run faster than the OP domain. */
>  #define CCS_PLL_FLAG_FIFO_DERATING				BIT(6)
> +/* If set, the VT domain may run slower than the OP domain. */
>  #define CCS_PLL_FLAG_FIFO_OVERRATING				BIT(7)
> +/* If set, the PLL tree has two PLLs instead of one. */
>  #define CCS_PLL_FLAG_DUAL_PLL					BIT(8)
> +/*
> + * If set, the OP SYS clock is a dual data rate clock, transferring two bits per
> + * cycle instead of one.
> + */
>  #define CCS_PLL_FLAG_OP_SYS_DDR					BIT(9)
> +/*
> + * If set, the OP PIX clock is a dual data rate clock, transferring two pixels
> + * per cycle instead of one.
> + */
>  #define CCS_PLL_FLAG_OP_PIX_DDR					BIT(10)
>  
>  /**

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-04-23 21:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 12:33 [PATCH v3 00/11] CCS PLL fixes and improvements Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 01/11] media: ccs-pll: Start OP pre-PLL multiplier search from correct value Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 02/11] media: ccs-pll: Start VT " Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 03/11] media: ccs-pll: Check for too high VT PLL multiplier in dual PLL case Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 04/11] media: ccs-pll: Correct the upper limit of maximum op_pre_pll_clk_div Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 05/11] media: ccs-pll: Print a debug message on too high VT PLL OP clock Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 06/11] media: ccs-pll: Drop LINK_DECOUPLED flag Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 07/11] media: ccs-pll: Print missing PLL flags Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 08/11] media: ccs-pll: Add a flag for even PLL multipliers Sakari Ailus
2025-04-23 21:49   ` Laurent Pinchart
2025-04-23 12:33 ` [PATCH v3 09/11] media: ccs-pll: Better validate VT PLL branch Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 10/11] media: ccs-pll: Print PLL calculator flags in the beginning Sakari Ailus
2025-04-23 12:33 ` [PATCH v3 11/11] media: ccs-pll: Document the CCS PLL flags Sakari Ailus
2025-04-23 12:43   ` Laurent Pinchart
2025-04-23 12:51     ` Sakari Ailus
2025-04-23 15:37     ` [PATCH v4 " Sakari Ailus
2025-04-23 17:50       ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox