All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups
@ 2014-10-02  8:45 Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 01/18] smiapp: Take mutex during PLL update in sensor initialisation Sakari Ailus
                   ` (17 more replies)
  0 siblings, 18 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

Hi,

This is an update of my previous set of smiapp PLL improvements.

The previous set can be found here:

<URL:http://www.spinics.net/lists/linux-media/msg80864.html>

Changes since v1:

- "smiapp: Clean up smiapp_set_format()" has been added to the set.

- "smiapp: Decrease link frequency if media bus pixel format BPP
  requires" is no longer needed since the control framework handles
  validation of the menu items (based on the mask supplied by the driver).

- A bug in the loop condition in "smiapp: Gather information on valid link
  rate and BPP combinations" has been fixed. In the same patch, fixed use of
  a non-existent label.

- Print available link frequencies using dev_dbg() rather than dev_info().

-- 
Kind regards,
Sakari

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

* [PATCH v2 01/18] smiapp: Take mutex during PLL update in sensor initialisation
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 02/18] smiapp-pll: Correct clock debug prints Sakari Ailus
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

The mutex does not serialise anything in this case but avoids a lockdep
warning from the control framework.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 932ed9b..6174a59 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2677,7 +2677,9 @@ static int smiapp_registered(struct v4l2_subdev *subdev)
 		pll->flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS;
 	pll->scale_n = sensor->limits[SMIAPP_LIMIT_SCALER_N_MIN];
 
+	mutex_lock(&sensor->mutex);
 	rval = smiapp_update_mode(sensor);
+	mutex_unlock(&sensor->mutex);
 	if (rval) {
 		dev_err(&client->dev, "update mode failed\n");
 		goto out_nvm_release;
-- 
1.7.10.4


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

* [PATCH v2 02/18] smiapp-pll: Correct clock debug prints
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 01/18] smiapp: Take mutex during PLL update in sensor initialisation Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 03/18] smiapp-pll: The clock tree values are unsigned --- fix " Sakari Ailus
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

The PLL flags were not used correctly.

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

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index 2335529..ab5d9a3 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -67,7 +67,7 @@ static void print_pll(struct device *dev, struct smiapp_pll *pll)
 {
 	dev_dbg(dev, "pre_pll_clk_div\t%d\n",  pll->pre_pll_clk_div);
 	dev_dbg(dev, "pll_multiplier \t%d\n",  pll->pll_multiplier);
-	if (pll->flags != SMIAPP_PLL_FLAG_NO_OP_CLOCKS) {
+	if (!(pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) {
 		dev_dbg(dev, "op_sys_clk_div \t%d\n", pll->op_sys_clk_div);
 		dev_dbg(dev, "op_pix_clk_div \t%d\n", pll->op_pix_clk_div);
 	}
@@ -77,7 +77,7 @@ static void print_pll(struct device *dev, struct smiapp_pll *pll)
 	dev_dbg(dev, "ext_clk_freq_hz \t%d\n", pll->ext_clk_freq_hz);
 	dev_dbg(dev, "pll_ip_clk_freq_hz \t%d\n", pll->pll_ip_clk_freq_hz);
 	dev_dbg(dev, "pll_op_clk_freq_hz \t%d\n", pll->pll_op_clk_freq_hz);
-	if (pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS) {
+	if (!(pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) {
 		dev_dbg(dev, "op_sys_clk_freq_hz \t%d\n",
 			pll->op_sys_clk_freq_hz);
 		dev_dbg(dev, "op_pix_clk_freq_hz \t%d\n",
-- 
1.7.10.4


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

* [PATCH v2 03/18] smiapp-pll: The clock tree values are unsigned --- fix debug prints
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 01/18] smiapp: Take mutex during PLL update in sensor initialisation Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 02/18] smiapp-pll: Correct clock debug prints Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 04/18] smiapp-pll: Separate bounds checking into a separate function Sakari Ailus
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

These values are unsigned, so use %u instead of %d.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp-pll.c |   94 ++++++++++++++++++++--------------------
 1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index ab5d9a3..d14af5c 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -65,26 +65,26 @@ static int bounds_check(struct device *dev, uint32_t val,
 
 static void print_pll(struct device *dev, struct smiapp_pll *pll)
 {
-	dev_dbg(dev, "pre_pll_clk_div\t%d\n",  pll->pre_pll_clk_div);
-	dev_dbg(dev, "pll_multiplier \t%d\n",  pll->pll_multiplier);
+	dev_dbg(dev, "pre_pll_clk_div\t%u\n",  pll->pre_pll_clk_div);
+	dev_dbg(dev, "pll_multiplier \t%u\n",  pll->pll_multiplier);
 	if (!(pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) {
-		dev_dbg(dev, "op_sys_clk_div \t%d\n", pll->op_sys_clk_div);
-		dev_dbg(dev, "op_pix_clk_div \t%d\n", pll->op_pix_clk_div);
+		dev_dbg(dev, "op_sys_clk_div \t%u\n", pll->op_sys_clk_div);
+		dev_dbg(dev, "op_pix_clk_div \t%u\n", pll->op_pix_clk_div);
 	}
-	dev_dbg(dev, "vt_sys_clk_div \t%d\n",  pll->vt_sys_clk_div);
-	dev_dbg(dev, "vt_pix_clk_div \t%d\n",  pll->vt_pix_clk_div);
+	dev_dbg(dev, "vt_sys_clk_div \t%u\n",  pll->vt_sys_clk_div);
+	dev_dbg(dev, "vt_pix_clk_div \t%u\n",  pll->vt_pix_clk_div);
 
-	dev_dbg(dev, "ext_clk_freq_hz \t%d\n", pll->ext_clk_freq_hz);
-	dev_dbg(dev, "pll_ip_clk_freq_hz \t%d\n", pll->pll_ip_clk_freq_hz);
-	dev_dbg(dev, "pll_op_clk_freq_hz \t%d\n", pll->pll_op_clk_freq_hz);
+	dev_dbg(dev, "ext_clk_freq_hz \t%u\n", pll->ext_clk_freq_hz);
+	dev_dbg(dev, "pll_ip_clk_freq_hz \t%u\n", pll->pll_ip_clk_freq_hz);
+	dev_dbg(dev, "pll_op_clk_freq_hz \t%u\n", pll->pll_op_clk_freq_hz);
 	if (!(pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) {
-		dev_dbg(dev, "op_sys_clk_freq_hz \t%d\n",
+		dev_dbg(dev, "op_sys_clk_freq_hz \t%u\n",
 			pll->op_sys_clk_freq_hz);
-		dev_dbg(dev, "op_pix_clk_freq_hz \t%d\n",
+		dev_dbg(dev, "op_pix_clk_freq_hz \t%u\n",
 			pll->op_pix_clk_freq_hz);
 	}
-	dev_dbg(dev, "vt_sys_clk_freq_hz \t%d\n", pll->vt_sys_clk_freq_hz);
-	dev_dbg(dev, "vt_pix_clk_freq_hz \t%d\n", pll->vt_pix_clk_freq_hz);
+	dev_dbg(dev, "vt_sys_clk_freq_hz \t%u\n", pll->vt_sys_clk_freq_hz);
+	dev_dbg(dev, "vt_pix_clk_freq_hz \t%u\n", pll->vt_pix_clk_freq_hz);
 }
 
 /*
@@ -123,11 +123,11 @@ static int __smiapp_pll_calculate(struct device *dev,
 	 * Get pre_pll_clk_div so that our pll_op_clk_freq_hz won't be
 	 * too high.
 	 */
-	dev_dbg(dev, "pre_pll_clk_div %d\n", pll->pre_pll_clk_div);
+	dev_dbg(dev, "pre_pll_clk_div %u\n", pll->pre_pll_clk_div);
 
 	/* Don't go above max pll multiplier. */
 	more_mul_max = limits->max_pll_multiplier / mul;
-	dev_dbg(dev, "more_mul_max: max_pll_multiplier check: %d\n",
+	dev_dbg(dev, "more_mul_max: max_pll_multiplier check: %u\n",
 		more_mul_max);
 	/* Don't go above max pll op frequency. */
 	more_mul_max =
@@ -135,30 +135,30 @@ static int __smiapp_pll_calculate(struct device *dev,
 		      more_mul_max,
 		      limits->max_pll_op_freq_hz
 		      / (pll->ext_clk_freq_hz / pll->pre_pll_clk_div * mul));
-	dev_dbg(dev, "more_mul_max: max_pll_op_freq_hz check: %d\n",
+	dev_dbg(dev, "more_mul_max: max_pll_op_freq_hz check: %u\n",
 		more_mul_max);
 	/* Don't go above the division capability of op sys clock divider. */
 	more_mul_max = min(more_mul_max,
 			   limits->op.max_sys_clk_div * pll->pre_pll_clk_div
 			   / div);
-	dev_dbg(dev, "more_mul_max: max_op_sys_clk_div check: %d\n",
+	dev_dbg(dev, "more_mul_max: max_op_sys_clk_div check: %u\n",
 		more_mul_max);
 	/* Ensure we won't go above min_pll_multiplier. */
 	more_mul_max = min(more_mul_max,
 			   DIV_ROUND_UP(limits->max_pll_multiplier, mul));
-	dev_dbg(dev, "more_mul_max: min_pll_multiplier check: %d\n",
+	dev_dbg(dev, "more_mul_max: min_pll_multiplier check: %u\n",
 		more_mul_max);
 
 	/* Ensure we won't go below min_pll_op_freq_hz. */
 	more_mul_min = DIV_ROUND_UP(limits->min_pll_op_freq_hz,
 				    pll->ext_clk_freq_hz / pll->pre_pll_clk_div
 				    * mul);
-	dev_dbg(dev, "more_mul_min: min_pll_op_freq_hz check: %d\n",
+	dev_dbg(dev, "more_mul_min: min_pll_op_freq_hz check: %u\n",
 		more_mul_min);
 	/* Ensure we won't go below min_pll_multiplier. */
 	more_mul_min = max(more_mul_min,
 			   DIV_ROUND_UP(limits->min_pll_multiplier, mul));
-	dev_dbg(dev, "more_mul_min: min_pll_multiplier check: %d\n",
+	dev_dbg(dev, "more_mul_min: min_pll_multiplier check: %u\n",
 		more_mul_min);
 
 	if (more_mul_min > more_mul_max) {
@@ -168,23 +168,23 @@ static int __smiapp_pll_calculate(struct device *dev,
 	}
 
 	more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div;
-	dev_dbg(dev, "more_mul_factor: %d\n", more_mul_factor);
+	dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor);
 	more_mul_factor = lcm(more_mul_factor, limits->op.min_sys_clk_div);
-	dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n",
+	dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %u\n",
 		more_mul_factor);
 	i = roundup(more_mul_min, more_mul_factor);
 	if (!is_one_or_even(i))
 		i <<= 1;
 
-	dev_dbg(dev, "final more_mul: %d\n", i);
+	dev_dbg(dev, "final more_mul: %u\n", i);
 	if (i > more_mul_max) {
-		dev_dbg(dev, "final more_mul is bad, max %d\n", more_mul_max);
+		dev_dbg(dev, "final more_mul is bad, max %u\n", more_mul_max);
 		return -EINVAL;
 	}
 
 	pll->pll_multiplier = mul * i;
 	pll->op_sys_clk_div = div * i / pll->pre_pll_clk_div;
-	dev_dbg(dev, "op_sys_clk_div: %d\n", pll->op_sys_clk_div);
+	dev_dbg(dev, "op_sys_clk_div: %u\n", pll->op_sys_clk_div);
 
 	pll->pll_ip_clk_freq_hz = pll->ext_clk_freq_hz
 		/ pll->pre_pll_clk_div;
@@ -197,7 +197,7 @@ static int __smiapp_pll_calculate(struct device *dev,
 		pll->pll_op_clk_freq_hz / pll->op_sys_clk_div;
 
 	pll->op_pix_clk_div = pll->bits_per_pixel;
-	dev_dbg(dev, "op_pix_clk_div: %d\n", pll->op_pix_clk_div);
+	dev_dbg(dev, "op_pix_clk_div: %u\n", pll->op_pix_clk_div);
 
 	pll->op_pix_clk_freq_hz =
 		pll->op_sys_clk_freq_hz / pll->op_pix_clk_div;
@@ -214,7 +214,7 @@ static int __smiapp_pll_calculate(struct device *dev,
 		vt_op_binning_div = pll->binning_horizontal;
 	else
 		vt_op_binning_div = 1;
-	dev_dbg(dev, "vt_op_binning_div: %d\n", vt_op_binning_div);
+	dev_dbg(dev, "vt_op_binning_div: %u\n", vt_op_binning_div);
 
 	/*
 	 * Profile 2 supports vt_pix_clk_div E [4, 10]
@@ -227,30 +227,30 @@ static int __smiapp_pll_calculate(struct device *dev,
 	 *
 	 * Find absolute limits for the factor of vt divider.
 	 */
-	dev_dbg(dev, "scale_m: %d\n", pll->scale_m);
+	dev_dbg(dev, "scale_m: %u\n", pll->scale_m);
 	min_vt_div = DIV_ROUND_UP(pll->op_pix_clk_div * pll->op_sys_clk_div
 				  * pll->scale_n,
 				  lane_op_clock_ratio * vt_op_binning_div
 				  * pll->scale_m);
 
 	/* Find smallest and biggest allowed vt divisor. */
-	dev_dbg(dev, "min_vt_div: %d\n", min_vt_div);
+	dev_dbg(dev, "min_vt_div: %u\n", min_vt_div);
 	min_vt_div = max(min_vt_div,
 			 DIV_ROUND_UP(pll->pll_op_clk_freq_hz,
 				      limits->vt.max_pix_clk_freq_hz));
-	dev_dbg(dev, "min_vt_div: max_vt_pix_clk_freq_hz: %d\n",
+	dev_dbg(dev, "min_vt_div: max_vt_pix_clk_freq_hz: %u\n",
 		min_vt_div);
 	min_vt_div = max_t(uint32_t, min_vt_div,
 			   limits->vt.min_pix_clk_div
 			   * limits->vt.min_sys_clk_div);
-	dev_dbg(dev, "min_vt_div: min_vt_clk_div: %d\n", min_vt_div);
+	dev_dbg(dev, "min_vt_div: min_vt_clk_div: %u\n", min_vt_div);
 
 	max_vt_div = limits->vt.max_sys_clk_div * limits->vt.max_pix_clk_div;
-	dev_dbg(dev, "max_vt_div: %d\n", max_vt_div);
+	dev_dbg(dev, "max_vt_div: %u\n", max_vt_div);
 	max_vt_div = min(max_vt_div,
 			 DIV_ROUND_UP(pll->pll_op_clk_freq_hz,
 				      limits->vt.min_pix_clk_freq_hz));
-	dev_dbg(dev, "max_vt_div: min_vt_pix_clk_freq_hz: %d\n",
+	dev_dbg(dev, "max_vt_div: min_vt_pix_clk_freq_hz: %u\n",
 		max_vt_div);
 
 	/*
@@ -258,28 +258,28 @@ static int __smiapp_pll_calculate(struct device *dev,
 	 * with all values of pix_clk_div.
 	 */
 	min_sys_div = limits->vt.min_sys_clk_div;
-	dev_dbg(dev, "min_sys_div: %d\n", min_sys_div);
+	dev_dbg(dev, "min_sys_div: %u\n", min_sys_div);
 	min_sys_div = max(min_sys_div,
 			  DIV_ROUND_UP(min_vt_div,
 				       limits->vt.max_pix_clk_div));
-	dev_dbg(dev, "min_sys_div: max_vt_pix_clk_div: %d\n", min_sys_div);
+	dev_dbg(dev, "min_sys_div: max_vt_pix_clk_div: %u\n", min_sys_div);
 	min_sys_div = max(min_sys_div,
 			  pll->pll_op_clk_freq_hz
 			  / limits->vt.max_sys_clk_freq_hz);
-	dev_dbg(dev, "min_sys_div: max_pll_op_clk_freq_hz: %d\n", min_sys_div);
+	dev_dbg(dev, "min_sys_div: max_pll_op_clk_freq_hz: %u\n", min_sys_div);
 	min_sys_div = clk_div_even_up(min_sys_div);
-	dev_dbg(dev, "min_sys_div: one or even: %d\n", min_sys_div);
+	dev_dbg(dev, "min_sys_div: one or even: %u\n", min_sys_div);
 
 	max_sys_div = limits->vt.max_sys_clk_div;
-	dev_dbg(dev, "max_sys_div: %d\n", max_sys_div);
+	dev_dbg(dev, "max_sys_div: %u\n", max_sys_div);
 	max_sys_div = min(max_sys_div,
 			  DIV_ROUND_UP(max_vt_div,
 				       limits->vt.min_pix_clk_div));
-	dev_dbg(dev, "max_sys_div: min_vt_pix_clk_div: %d\n", max_sys_div);
+	dev_dbg(dev, "max_sys_div: min_vt_pix_clk_div: %u\n", max_sys_div);
 	max_sys_div = min(max_sys_div,
 			  DIV_ROUND_UP(pll->pll_op_clk_freq_hz,
 				       limits->vt.min_pix_clk_freq_hz));
-	dev_dbg(dev, "max_sys_div: min_vt_pix_clk_freq_hz: %d\n", max_sys_div);
+	dev_dbg(dev, "max_sys_div: min_vt_pix_clk_freq_hz: %u\n", max_sys_div);
 
 	/*
 	 * Find pix_div such that a legal pix_div * sys_div results
@@ -296,7 +296,7 @@ static int __smiapp_pll_calculate(struct device *dev,
 			if (pix_div < limits->vt.min_pix_clk_div
 			    || pix_div > limits->vt.max_pix_clk_div) {
 				dev_dbg(dev,
-					"pix_div %d too small or too big (%d--%d)\n",
+					"pix_div %u too small or too big (%u--%u)\n",
 					pix_div,
 					limits->vt.min_pix_clk_div,
 					limits->vt.max_pix_clk_div);
@@ -390,9 +390,9 @@ int smiapp_pll_calculate(struct device *dev,
 		lane_op_clock_ratio = pll->csi2.lanes;
 	else
 		lane_op_clock_ratio = 1;
-	dev_dbg(dev, "lane_op_clock_ratio: %d\n", lane_op_clock_ratio);
+	dev_dbg(dev, "lane_op_clock_ratio: %u\n", lane_op_clock_ratio);
 
-	dev_dbg(dev, "binning: %dx%d\n", pll->binning_horizontal,
+	dev_dbg(dev, "binning: %ux%u\n", pll->binning_horizontal,
 		pll->binning_vertical);
 
 	switch (pll->bus_type) {
@@ -411,7 +411,7 @@ int smiapp_pll_calculate(struct device *dev,
 	}
 
 	/* Figure out limits for pre-pll divider based on extclk */
-	dev_dbg(dev, "min / max pre_pll_clk_div: %d / %d\n",
+	dev_dbg(dev, "min / max pre_pll_clk_div: %u / %u\n",
 		limits->min_pre_pll_clk_div, limits->max_pre_pll_clk_div);
 	max_pre_pll_clk_div =
 		min_t(uint16_t, limits->max_pre_pll_clk_div,
@@ -422,20 +422,20 @@ int smiapp_pll_calculate(struct device *dev,
 		      clk_div_even_up(
 			      DIV_ROUND_UP(pll->ext_clk_freq_hz,
 					   limits->max_pll_ip_freq_hz)));
-	dev_dbg(dev, "pre-pll check: min / max pre_pll_clk_div: %d / %d\n",
+	dev_dbg(dev, "pre-pll check: min / max pre_pll_clk_div: %u / %u\n",
 		min_pre_pll_clk_div, max_pre_pll_clk_div);
 
 	i = gcd(pll->pll_op_clk_freq_hz, pll->ext_clk_freq_hz);
 	mul = div_u64(pll->pll_op_clk_freq_hz, i);
 	div = pll->ext_clk_freq_hz / i;
-	dev_dbg(dev, "mul %d / div %d\n", mul, div);
+	dev_dbg(dev, "mul %u / div %u\n", mul, div);
 
 	min_pre_pll_clk_div =
 		max_t(uint16_t, min_pre_pll_clk_div,
 		      clk_div_even_up(
 			      DIV_ROUND_UP(mul * pll->ext_clk_freq_hz,
 					   limits->max_pll_op_freq_hz)));
-	dev_dbg(dev, "pll_op check: min / max pre_pll_clk_div: %d / %d\n",
+	dev_dbg(dev, "pll_op check: min / max pre_pll_clk_div: %u / %u\n",
 		min_pre_pll_clk_div, max_pre_pll_clk_div);
 
 	for (pll->pre_pll_clk_div = min_pre_pll_clk_div;
-- 
1.7.10.4


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

* [PATCH v2 04/18] smiapp-pll: Separate bounds checking into a separate function
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (2 preceding siblings ...)
  2014-10-02  8:45 ` [PATCH v2 03/18] smiapp-pll: The clock tree values are unsigned --- fix " Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 05/18] smiapp-pll: External clock frequency isn't an output value Sakari Ailus
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

Enough work for this function already.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp-pll.c |  110 +++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 51 deletions(-)

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index d14af5c..bde8eb8 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -87,6 +87,64 @@ static void print_pll(struct device *dev, struct smiapp_pll *pll)
 	dev_dbg(dev, "vt_pix_clk_freq_hz \t%u\n", pll->vt_pix_clk_freq_hz);
 }
 
+static int check_all_bounds(struct device *dev,
+			    const struct smiapp_pll_limits *limits,
+			    struct smiapp_pll *pll)
+{
+	int rval;
+
+	rval = bounds_check(dev, pll->pll_ip_clk_freq_hz,
+			    limits->min_pll_ip_freq_hz,
+			    limits->max_pll_ip_freq_hz,
+			    "pll_ip_clk_freq_hz");
+	if (!rval)
+		rval = bounds_check(
+			dev, pll->pll_multiplier,
+			limits->min_pll_multiplier, limits->max_pll_multiplier,
+			"pll_multiplier");
+	if (!rval)
+		rval = bounds_check(
+			dev, pll->pll_op_clk_freq_hz,
+			limits->min_pll_op_freq_hz, limits->max_pll_op_freq_hz,
+			"pll_op_clk_freq_hz");
+	if (!rval)
+		rval = bounds_check(
+			dev, pll->op_sys_clk_div,
+			limits->op.min_sys_clk_div, limits->op.max_sys_clk_div,
+			"op_sys_clk_div");
+	if (!rval)
+		rval = bounds_check(
+			dev, pll->op_pix_clk_div,
+			limits->op.min_pix_clk_div, limits->op.max_pix_clk_div,
+			"op_pix_clk_div");
+	if (!rval)
+		rval = bounds_check(
+			dev, pll->op_sys_clk_freq_hz,
+			limits->op.min_sys_clk_freq_hz,
+			limits->op.max_sys_clk_freq_hz,
+			"op_sys_clk_freq_hz");
+	if (!rval)
+		rval = bounds_check(
+			dev, pll->op_pix_clk_freq_hz,
+			limits->op.min_pix_clk_freq_hz,
+			limits->op.max_pix_clk_freq_hz,
+			"op_pix_clk_freq_hz");
+	if (!rval)
+		rval = bounds_check(
+			dev, pll->vt_sys_clk_freq_hz,
+			limits->vt.min_sys_clk_freq_hz,
+			limits->vt.max_sys_clk_freq_hz,
+			"vt_sys_clk_freq_hz");
+	if (!rval)
+		rval = bounds_check(
+			dev, pll->vt_pix_clk_freq_hz,
+			limits->vt.min_pix_clk_freq_hz,
+			limits->vt.max_pix_clk_freq_hz,
+			"vt_pix_clk_freq_hz");
+
+	return rval;
+}
+
 /*
  * Heuristically guess the PLL tree for a given common multiplier and
  * divisor. Begin with the operational timing and continue to video
@@ -117,7 +175,6 @@ static int __smiapp_pll_calculate(struct device *dev,
 	uint32_t min_vt_div, max_vt_div, vt_div;
 	uint32_t min_sys_div, max_sys_div;
 	unsigned int i;
-	int rval;
 
 	/*
 	 * Get pre_pll_clk_div so that our pll_op_clk_freq_hz won't be
@@ -323,56 +380,7 @@ static int __smiapp_pll_calculate(struct device *dev,
 	pll->pixel_rate_csi =
 		pll->op_pix_clk_freq_hz * lane_op_clock_ratio;
 
-	rval = bounds_check(dev, pll->pll_ip_clk_freq_hz,
-			    limits->min_pll_ip_freq_hz,
-			    limits->max_pll_ip_freq_hz,
-			    "pll_ip_clk_freq_hz");
-	if (!rval)
-		rval = bounds_check(
-			dev, pll->pll_multiplier,
-			limits->min_pll_multiplier, limits->max_pll_multiplier,
-			"pll_multiplier");
-	if (!rval)
-		rval = bounds_check(
-			dev, pll->pll_op_clk_freq_hz,
-			limits->min_pll_op_freq_hz, limits->max_pll_op_freq_hz,
-			"pll_op_clk_freq_hz");
-	if (!rval)
-		rval = bounds_check(
-			dev, pll->op_sys_clk_div,
-			limits->op.min_sys_clk_div, limits->op.max_sys_clk_div,
-			"op_sys_clk_div");
-	if (!rval)
-		rval = bounds_check(
-			dev, pll->op_pix_clk_div,
-			limits->op.min_pix_clk_div, limits->op.max_pix_clk_div,
-			"op_pix_clk_div");
-	if (!rval)
-		rval = bounds_check(
-			dev, pll->op_sys_clk_freq_hz,
-			limits->op.min_sys_clk_freq_hz,
-			limits->op.max_sys_clk_freq_hz,
-			"op_sys_clk_freq_hz");
-	if (!rval)
-		rval = bounds_check(
-			dev, pll->op_pix_clk_freq_hz,
-			limits->op.min_pix_clk_freq_hz,
-			limits->op.max_pix_clk_freq_hz,
-			"op_pix_clk_freq_hz");
-	if (!rval)
-		rval = bounds_check(
-			dev, pll->vt_sys_clk_freq_hz,
-			limits->vt.min_sys_clk_freq_hz,
-			limits->vt.max_sys_clk_freq_hz,
-			"vt_sys_clk_freq_hz");
-	if (!rval)
-		rval = bounds_check(
-			dev, pll->vt_pix_clk_freq_hz,
-			limits->vt.min_pix_clk_freq_hz,
-			limits->vt.max_pix_clk_freq_hz,
-			"vt_pix_clk_freq_hz");
-
-	return rval;
+	return check_all_bounds(dev, limits, pll);
 }
 
 int smiapp_pll_calculate(struct device *dev,
-- 
1.7.10.4


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

* [PATCH v2 05/18] smiapp-pll: External clock frequency isn't an output value
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (3 preceding siblings ...)
  2014-10-02  8:45 ` [PATCH v2 04/18] smiapp-pll: Separate bounds checking into a separate function Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 06/18] smiapp-pll: Unify OP and VT PLL structs Sakari Ailus
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

It's input. Move it elsewhere in the struct.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp-pll.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/smiapp-pll.h b/drivers/media/i2c/smiapp-pll.h
index 5ce2b61..2885cd7 100644
--- a/drivers/media/i2c/smiapp-pll.h
+++ b/drivers/media/i2c/smiapp-pll.h
@@ -53,6 +53,7 @@ struct smiapp_pll {
 	uint8_t scale_n;
 	uint8_t bits_per_pixel;
 	uint32_t link_freq;
+	uint32_t ext_clk_freq_hz;
 
 	/* output values */
 	uint16_t pre_pll_clk_div;
@@ -62,7 +63,6 @@ struct smiapp_pll {
 	uint16_t vt_sys_clk_div;
 	uint16_t vt_pix_clk_div;
 
-	uint32_t ext_clk_freq_hz;
 	uint32_t pll_ip_clk_freq_hz;
 	uint32_t pll_op_clk_freq_hz;
 	uint32_t op_sys_clk_freq_hz;
-- 
1.7.10.4


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

* [PATCH v2 06/18] smiapp-pll: Unify OP and VT PLL structs
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (4 preceding siblings ...)
  2014-10-02  8:45 ` [PATCH v2 05/18] smiapp-pll: External clock frequency isn't an output value Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 07/18] smiapp-pll: Calculate OP clocks only for sensors that have them Sakari Ailus
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

Uniform representation for VT and OP clocks. This is preparation for
calculating the VT clocks using the OP clock code.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp-pll.c         |   60 ++++++++++++++++----------------
 drivers/media/i2c/smiapp-pll.h         |   18 +++++-----
 drivers/media/i2c/smiapp/smiapp-core.c |   14 ++++----
 3 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index bde8eb8..40a18ba 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -68,23 +68,23 @@ static void print_pll(struct device *dev, struct smiapp_pll *pll)
 	dev_dbg(dev, "pre_pll_clk_div\t%u\n",  pll->pre_pll_clk_div);
 	dev_dbg(dev, "pll_multiplier \t%u\n",  pll->pll_multiplier);
 	if (!(pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) {
-		dev_dbg(dev, "op_sys_clk_div \t%u\n", pll->op_sys_clk_div);
-		dev_dbg(dev, "op_pix_clk_div \t%u\n", pll->op_pix_clk_div);
+		dev_dbg(dev, "op_sys_clk_div \t%u\n", pll->op.sys_clk_div);
+		dev_dbg(dev, "op_pix_clk_div \t%u\n", pll->op.pix_clk_div);
 	}
-	dev_dbg(dev, "vt_sys_clk_div \t%u\n",  pll->vt_sys_clk_div);
-	dev_dbg(dev, "vt_pix_clk_div \t%u\n",  pll->vt_pix_clk_div);
+	dev_dbg(dev, "vt_sys_clk_div \t%u\n",  pll->vt.sys_clk_div);
+	dev_dbg(dev, "vt_pix_clk_div \t%u\n",  pll->vt.pix_clk_div);
 
 	dev_dbg(dev, "ext_clk_freq_hz \t%u\n", pll->ext_clk_freq_hz);
 	dev_dbg(dev, "pll_ip_clk_freq_hz \t%u\n", pll->pll_ip_clk_freq_hz);
 	dev_dbg(dev, "pll_op_clk_freq_hz \t%u\n", pll->pll_op_clk_freq_hz);
 	if (!(pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) {
 		dev_dbg(dev, "op_sys_clk_freq_hz \t%u\n",
-			pll->op_sys_clk_freq_hz);
+			pll->op.sys_clk_freq_hz);
 		dev_dbg(dev, "op_pix_clk_freq_hz \t%u\n",
-			pll->op_pix_clk_freq_hz);
+			pll->op.pix_clk_freq_hz);
 	}
-	dev_dbg(dev, "vt_sys_clk_freq_hz \t%u\n", pll->vt_sys_clk_freq_hz);
-	dev_dbg(dev, "vt_pix_clk_freq_hz \t%u\n", pll->vt_pix_clk_freq_hz);
+	dev_dbg(dev, "vt_sys_clk_freq_hz \t%u\n", pll->vt.sys_clk_freq_hz);
+	dev_dbg(dev, "vt_pix_clk_freq_hz \t%u\n", pll->vt.pix_clk_freq_hz);
 }
 
 static int check_all_bounds(struct device *dev,
@@ -109,35 +109,35 @@ static int check_all_bounds(struct device *dev,
 			"pll_op_clk_freq_hz");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->op_sys_clk_div,
+			dev, pll->op.sys_clk_div,
 			limits->op.min_sys_clk_div, limits->op.max_sys_clk_div,
 			"op_sys_clk_div");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->op_pix_clk_div,
+			dev, pll->op.pix_clk_div,
 			limits->op.min_pix_clk_div, limits->op.max_pix_clk_div,
 			"op_pix_clk_div");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->op_sys_clk_freq_hz,
+			dev, pll->op.sys_clk_freq_hz,
 			limits->op.min_sys_clk_freq_hz,
 			limits->op.max_sys_clk_freq_hz,
 			"op_sys_clk_freq_hz");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->op_pix_clk_freq_hz,
+			dev, pll->op.pix_clk_freq_hz,
 			limits->op.min_pix_clk_freq_hz,
 			limits->op.max_pix_clk_freq_hz,
 			"op_pix_clk_freq_hz");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->vt_sys_clk_freq_hz,
+			dev, pll->vt.sys_clk_freq_hz,
 			limits->vt.min_sys_clk_freq_hz,
 			limits->vt.max_sys_clk_freq_hz,
 			"vt_sys_clk_freq_hz");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->vt_pix_clk_freq_hz,
+			dev, pll->vt.pix_clk_freq_hz,
 			limits->vt.min_pix_clk_freq_hz,
 			limits->vt.max_pix_clk_freq_hz,
 			"vt_pix_clk_freq_hz");
@@ -240,8 +240,8 @@ static int __smiapp_pll_calculate(struct device *dev,
 	}
 
 	pll->pll_multiplier = mul * i;
-	pll->op_sys_clk_div = div * i / pll->pre_pll_clk_div;
-	dev_dbg(dev, "op_sys_clk_div: %u\n", pll->op_sys_clk_div);
+	pll->op.sys_clk_div = div * i / pll->pre_pll_clk_div;
+	dev_dbg(dev, "op_sys_clk_div: %u\n", pll->op.sys_clk_div);
 
 	pll->pll_ip_clk_freq_hz = pll->ext_clk_freq_hz
 		/ pll->pre_pll_clk_div;
@@ -250,14 +250,14 @@ static int __smiapp_pll_calculate(struct device *dev,
 		* pll->pll_multiplier;
 
 	/* Derive pll_op_clk_freq_hz. */
-	pll->op_sys_clk_freq_hz =
-		pll->pll_op_clk_freq_hz / pll->op_sys_clk_div;
+	pll->op.sys_clk_freq_hz =
+		pll->pll_op_clk_freq_hz / pll->op.sys_clk_div;
 
-	pll->op_pix_clk_div = pll->bits_per_pixel;
-	dev_dbg(dev, "op_pix_clk_div: %u\n", pll->op_pix_clk_div);
+	pll->op.pix_clk_div = pll->bits_per_pixel;
+	dev_dbg(dev, "op_pix_clk_div: %u\n", pll->op.pix_clk_div);
 
-	pll->op_pix_clk_freq_hz =
-		pll->op_sys_clk_freq_hz / pll->op_pix_clk_div;
+	pll->op.pix_clk_freq_hz =
+		pll->op.sys_clk_freq_hz / pll->op.pix_clk_div;
 
 	/*
 	 * Some sensors perform analogue binning and some do this
@@ -285,7 +285,7 @@ static int __smiapp_pll_calculate(struct device *dev,
 	 * Find absolute limits for the factor of vt divider.
 	 */
 	dev_dbg(dev, "scale_m: %u\n", pll->scale_m);
-	min_vt_div = DIV_ROUND_UP(pll->op_pix_clk_div * pll->op_sys_clk_div
+	min_vt_div = DIV_ROUND_UP(pll->op.pix_clk_div * pll->op.sys_clk_div
 				  * pll->scale_n,
 				  lane_op_clock_ratio * vt_op_binning_div
 				  * pll->scale_m);
@@ -369,16 +369,16 @@ static int __smiapp_pll_calculate(struct device *dev,
 			break;
 	}
 
-	pll->vt_sys_clk_div = DIV_ROUND_UP(min_vt_div, best_pix_div);
-	pll->vt_pix_clk_div = best_pix_div;
+	pll->vt.sys_clk_div = DIV_ROUND_UP(min_vt_div, best_pix_div);
+	pll->vt.pix_clk_div = best_pix_div;
 
-	pll->vt_sys_clk_freq_hz =
-		pll->pll_op_clk_freq_hz / pll->vt_sys_clk_div;
-	pll->vt_pix_clk_freq_hz =
-		pll->vt_sys_clk_freq_hz / pll->vt_pix_clk_div;
+	pll->vt.sys_clk_freq_hz =
+		pll->pll_op_clk_freq_hz / pll->vt.sys_clk_div;
+	pll->vt.pix_clk_freq_hz =
+		pll->vt.sys_clk_freq_hz / pll->vt.pix_clk_div;
 
 	pll->pixel_rate_csi =
-		pll->op_pix_clk_freq_hz * lane_op_clock_ratio;
+		pll->op.pix_clk_freq_hz * lane_op_clock_ratio;
 
 	return check_all_bounds(dev, limits, pll);
 }
diff --git a/drivers/media/i2c/smiapp-pll.h b/drivers/media/i2c/smiapp-pll.h
index 2885cd7..b7c0e66 100644
--- a/drivers/media/i2c/smiapp-pll.h
+++ b/drivers/media/i2c/smiapp-pll.h
@@ -35,6 +35,13 @@
 #define SMIAPP_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			(1 << 0)
 #define SMIAPP_PLL_FLAG_NO_OP_CLOCKS				(1 << 1)
 
+struct smiapp_pll_branch {
+	uint16_t sys_clk_div;
+	uint16_t pix_clk_div;
+	uint32_t sys_clk_freq_hz;
+	uint32_t pix_clk_freq_hz;
+};
+
 struct smiapp_pll {
 	/* input values */
 	uint8_t bus_type;
@@ -58,17 +65,10 @@ struct smiapp_pll {
 	/* output values */
 	uint16_t pre_pll_clk_div;
 	uint16_t pll_multiplier;
-	uint16_t op_sys_clk_div;
-	uint16_t op_pix_clk_div;
-	uint16_t vt_sys_clk_div;
-	uint16_t vt_pix_clk_div;
-
 	uint32_t pll_ip_clk_freq_hz;
 	uint32_t pll_op_clk_freq_hz;
-	uint32_t op_sys_clk_freq_hz;
-	uint32_t op_pix_clk_freq_hz;
-	uint32_t vt_sys_clk_freq_hz;
-	uint32_t vt_pix_clk_freq_hz;
+	struct smiapp_pll_branch vt;
+	struct smiapp_pll_branch op;
 
 	uint32_t pixel_rate_csi;
 };
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 6174a59..389e775 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -205,12 +205,12 @@ static int smiapp_pll_configure(struct smiapp_sensor *sensor)
 	int rval;
 
 	rval = smiapp_write(
-		sensor, SMIAPP_REG_U16_VT_PIX_CLK_DIV, pll->vt_pix_clk_div);
+		sensor, SMIAPP_REG_U16_VT_PIX_CLK_DIV, pll->vt.pix_clk_div);
 	if (rval < 0)
 		return rval;
 
 	rval = smiapp_write(
-		sensor, SMIAPP_REG_U16_VT_SYS_CLK_DIV, pll->vt_sys_clk_div);
+		sensor, SMIAPP_REG_U16_VT_SYS_CLK_DIV, pll->vt.sys_clk_div);
 	if (rval < 0)
 		return rval;
 
@@ -227,17 +227,17 @@ static int smiapp_pll_configure(struct smiapp_sensor *sensor)
 	/* Lane op clock ratio does not apply here. */
 	rval = smiapp_write(
 		sensor, SMIAPP_REG_U32_REQUESTED_LINK_BIT_RATE_MBPS,
-		DIV_ROUND_UP(pll->op_sys_clk_freq_hz, 1000000 / 256 / 256));
+		DIV_ROUND_UP(pll->op.sys_clk_freq_hz, 1000000 / 256 / 256));
 	if (rval < 0 || sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0)
 		return rval;
 
 	rval = smiapp_write(
-		sensor, SMIAPP_REG_U16_OP_PIX_CLK_DIV, pll->op_pix_clk_div);
+		sensor, SMIAPP_REG_U16_OP_PIX_CLK_DIV, pll->op.pix_clk_div);
 	if (rval < 0)
 		return rval;
 
 	return smiapp_write(
-		sensor, SMIAPP_REG_U16_OP_SYS_CLK_DIV, pll->op_sys_clk_div);
+		sensor, SMIAPP_REG_U16_OP_SYS_CLK_DIV, pll->op.sys_clk_div);
 }
 
 static int smiapp_pll_update(struct smiapp_sensor *sensor)
@@ -299,7 +299,7 @@ static int smiapp_pll_update(struct smiapp_sensor *sensor)
 		return rval;
 
 	__v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_parray,
-				 pll->vt_pix_clk_freq_hz);
+				 pll->vt.pix_clk_freq_hz);
 	__v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_csi, pll->pixel_rate_csi);
 
 	return 0;
@@ -904,7 +904,7 @@ static int smiapp_update_mode(struct smiapp_sensor *sensor)
 	dev_dbg(&client->dev, "hblank\t\t%d\n", sensor->hblank->val);
 
 	dev_dbg(&client->dev, "real timeperframe\t100/%d\n",
-		sensor->pll.vt_pix_clk_freq_hz /
+		sensor->pll.vt.pix_clk_freq_hz /
 		((sensor->pixel_array->crop[SMIAPP_PA_PAD_SRC].width
 		  + sensor->hblank->val) *
 		 (sensor->pixel_array->crop[SMIAPP_PA_PAD_SRC].height
-- 
1.7.10.4


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

* [PATCH v2 07/18] smiapp-pll: Calculate OP clocks only for sensors that have them
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (5 preceding siblings ...)
  2014-10-02  8:45 ` [PATCH v2 06/18] smiapp-pll: Unify OP and VT PLL structs Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 08/18] smiapp-pll: Don't validate OP clocks if there are none Sakari Ailus
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

Profile 0 sensors have no OP clock branck in the clock tree. The PLL
calculator still calculated them, they just weren't used for anything.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp-pll.c |   82 +++++++++++++++++++++++++---------------
 1 file changed, 52 insertions(+), 30 deletions(-)

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index 40a18ba..cac1407 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -89,7 +89,9 @@ static void print_pll(struct device *dev, struct smiapp_pll *pll)
 
 static int check_all_bounds(struct device *dev,
 			    const struct smiapp_pll_limits *limits,
-			    struct smiapp_pll *pll)
+			    const struct smiapp_pll_branch_limits *op_limits,
+			    struct smiapp_pll *pll,
+			    struct smiapp_pll_branch *op_pll)
 {
 	int rval;
 
@@ -109,25 +111,25 @@ static int check_all_bounds(struct device *dev,
 			"pll_op_clk_freq_hz");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->op.sys_clk_div,
-			limits->op.min_sys_clk_div, limits->op.max_sys_clk_div,
+			dev, op_pll->sys_clk_div,
+			op_limits->min_sys_clk_div, op_limits->max_sys_clk_div,
 			"op_sys_clk_div");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->op.pix_clk_div,
-			limits->op.min_pix_clk_div, limits->op.max_pix_clk_div,
+			dev, op_pll->pix_clk_div,
+			op_limits->min_pix_clk_div, op_limits->max_pix_clk_div,
 			"op_pix_clk_div");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->op.sys_clk_freq_hz,
-			limits->op.min_sys_clk_freq_hz,
-			limits->op.max_sys_clk_freq_hz,
+			dev, op_pll->sys_clk_freq_hz,
+			op_limits->min_sys_clk_freq_hz,
+			op_limits->max_sys_clk_freq_hz,
 			"op_sys_clk_freq_hz");
 	if (!rval)
 		rval = bounds_check(
-			dev, pll->op.pix_clk_freq_hz,
-			limits->op.min_pix_clk_freq_hz,
-			limits->op.max_pix_clk_freq_hz,
+			dev, op_pll->pix_clk_freq_hz,
+			op_limits->min_pix_clk_freq_hz,
+			op_limits->max_pix_clk_freq_hz,
 			"op_pix_clk_freq_hz");
 	if (!rval)
 		rval = bounds_check(
@@ -156,10 +158,11 @@ static int check_all_bounds(struct device *dev,
  *
  * @return Zero on success, error code on error.
  */
-static int __smiapp_pll_calculate(struct device *dev,
-				  const struct smiapp_pll_limits *limits,
-				  struct smiapp_pll *pll, uint32_t mul,
-				  uint32_t div, uint32_t lane_op_clock_ratio)
+static int __smiapp_pll_calculate(
+	struct device *dev, const struct smiapp_pll_limits *limits,
+	const struct smiapp_pll_branch_limits *op_limits,
+	struct smiapp_pll *pll, struct smiapp_pll_branch *op_pll, uint32_t mul,
+	uint32_t div, uint32_t lane_op_clock_ratio)
 {
 	uint32_t sys_div;
 	uint32_t best_pix_div = INT_MAX >> 1;
@@ -196,7 +199,7 @@ static int __smiapp_pll_calculate(struct device *dev,
 		more_mul_max);
 	/* Don't go above the division capability of op sys clock divider. */
 	more_mul_max = min(more_mul_max,
-			   limits->op.max_sys_clk_div * pll->pre_pll_clk_div
+			   op_limits->max_sys_clk_div * pll->pre_pll_clk_div
 			   / div);
 	dev_dbg(dev, "more_mul_max: max_op_sys_clk_div check: %u\n",
 		more_mul_max);
@@ -226,8 +229,8 @@ static int __smiapp_pll_calculate(struct device *dev,
 
 	more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div;
 	dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor);
-	more_mul_factor = lcm(more_mul_factor, limits->op.min_sys_clk_div);
-	dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %u\n",
+	more_mul_factor = lcm(more_mul_factor, op_limits->min_sys_clk_div);
+	dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n",
 		more_mul_factor);
 	i = roundup(more_mul_min, more_mul_factor);
 	if (!is_one_or_even(i))
@@ -240,8 +243,8 @@ static int __smiapp_pll_calculate(struct device *dev,
 	}
 
 	pll->pll_multiplier = mul * i;
-	pll->op.sys_clk_div = div * i / pll->pre_pll_clk_div;
-	dev_dbg(dev, "op_sys_clk_div: %u\n", pll->op.sys_clk_div);
+	op_pll->sys_clk_div = div * i / pll->pre_pll_clk_div;
+	dev_dbg(dev, "op_sys_clk_div: %u\n", op_pll->sys_clk_div);
 
 	pll->pll_ip_clk_freq_hz = pll->ext_clk_freq_hz
 		/ pll->pre_pll_clk_div;
@@ -250,14 +253,19 @@ static int __smiapp_pll_calculate(struct device *dev,
 		* pll->pll_multiplier;
 
 	/* Derive pll_op_clk_freq_hz. */
-	pll->op.sys_clk_freq_hz =
-		pll->pll_op_clk_freq_hz / pll->op.sys_clk_div;
+	op_pll->sys_clk_freq_hz =
+		pll->pll_op_clk_freq_hz / op_pll->sys_clk_div;
 
-	pll->op.pix_clk_div = pll->bits_per_pixel;
-	dev_dbg(dev, "op_pix_clk_div: %u\n", pll->op.pix_clk_div);
+	op_pll->pix_clk_div = pll->bits_per_pixel;
+	dev_dbg(dev, "op_pix_clk_div: %u\n", op_pll->pix_clk_div);
 
-	pll->op.pix_clk_freq_hz =
-		pll->op.sys_clk_freq_hz / pll->op.pix_clk_div;
+	op_pll->pix_clk_freq_hz =
+		op_pll->sys_clk_freq_hz / op_pll->pix_clk_div;
+
+	if (pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS) {
+		/* No OP clocks --- VT clocks are used instead. */
+		goto out_skip_vt_calc;
+	}
 
 	/*
 	 * Some sensors perform analogue binning and some do this
@@ -285,7 +293,7 @@ static int __smiapp_pll_calculate(struct device *dev,
 	 * Find absolute limits for the factor of vt divider.
 	 */
 	dev_dbg(dev, "scale_m: %u\n", pll->scale_m);
-	min_vt_div = DIV_ROUND_UP(pll->op.pix_clk_div * pll->op.sys_clk_div
+	min_vt_div = DIV_ROUND_UP(op_pll->pix_clk_div * op_pll->sys_clk_div
 				  * pll->scale_n,
 				  lane_op_clock_ratio * vt_op_binning_div
 				  * pll->scale_m);
@@ -377,16 +385,19 @@ static int __smiapp_pll_calculate(struct device *dev,
 	pll->vt.pix_clk_freq_hz =
 		pll->vt.sys_clk_freq_hz / pll->vt.pix_clk_div;
 
+out_skip_vt_calc:
 	pll->pixel_rate_csi =
-		pll->op.pix_clk_freq_hz * lane_op_clock_ratio;
+		op_pll->pix_clk_freq_hz * lane_op_clock_ratio;
 
-	return check_all_bounds(dev, limits, pll);
+	return check_all_bounds(dev, limits, op_limits, pll, op_pll);
 }
 
 int smiapp_pll_calculate(struct device *dev,
 			 const struct smiapp_pll_limits *limits,
 			 struct smiapp_pll *pll)
 {
+	const struct smiapp_pll_branch_limits *op_limits = &limits->op;
+	struct smiapp_pll_branch *op_pll = &pll->op;
 	uint16_t min_pre_pll_clk_div;
 	uint16_t max_pre_pll_clk_div;
 	uint32_t lane_op_clock_ratio;
@@ -394,6 +405,16 @@ int smiapp_pll_calculate(struct device *dev,
 	unsigned int i;
 	int rval = -EINVAL;
 
+	if (pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS) {
+		/*
+		 * If there's no OP PLL at all, use the VT values
+		 * instead. The OP values are ignored for the rest of
+		 * the PLL calculation.
+		 */
+		op_limits = &limits->vt;
+		op_pll = &pll->vt;
+	}
+
 	if (pll->flags & SMIAPP_PLL_FLAG_OP_PIX_CLOCK_PER_LANE)
 		lane_op_clock_ratio = pll->csi2.lanes;
 	else
@@ -449,7 +470,8 @@ int smiapp_pll_calculate(struct device *dev,
 	for (pll->pre_pll_clk_div = min_pre_pll_clk_div;
 	     pll->pre_pll_clk_div <= max_pre_pll_clk_div;
 	     pll->pre_pll_clk_div += 2 - (pll->pre_pll_clk_div & 1)) {
-		rval = __smiapp_pll_calculate(dev, limits, pll, mul, div,
+		rval = __smiapp_pll_calculate(dev, limits, op_limits, pll,
+					      op_pll, mul, div,
 					      lane_op_clock_ratio);
 		if (rval)
 			continue;
-- 
1.7.10.4


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

* [PATCH v2 08/18] smiapp-pll: Don't validate OP clocks if there are none
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (6 preceding siblings ...)
  2014-10-02  8:45 ` [PATCH v2 07/18] smiapp-pll: Calculate OP clocks only for sensors that have them Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:45 ` [PATCH v2 09/18] smiapp: The PLL calculator handles sensors with VT clocks only Sakari Ailus
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

For profile 0 sensors (which have no OP clocks), the OP limits are in fact
VT limits. Do not verify them again.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp-pll.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index cac1407..862ca0c 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -131,6 +131,14 @@ static int check_all_bounds(struct device *dev,
 			op_limits->min_pix_clk_freq_hz,
 			op_limits->max_pix_clk_freq_hz,
 			"op_pix_clk_freq_hz");
+
+	/*
+	 * If there are no OP clocks, the VT clocks are contained in
+	 * the OP clock struct.
+	 */
+	if (pll->flags & SMIAPP_PLL_FLAG_NO_OP_CLOCKS)
+		return rval;
+
 	if (!rval)
 		rval = bounds_check(
 			dev, pll->vt.sys_clk_freq_hz,
-- 
1.7.10.4


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

* [PATCH v2 09/18] smiapp: The PLL calculator handles sensors with VT clocks only
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (7 preceding siblings ...)
  2014-10-02  8:45 ` [PATCH v2 08/18] smiapp-pll: Don't validate OP clocks if there are none Sakari Ailus
@ 2014-10-02  8:45 ` Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 10/18] smiapp: Remove validation of op_pix_clk_div Sakari Ailus
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:45 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

No need to pretend the OP limits are there anymore.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 389e775..d0ea7a3 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -277,16 +277,6 @@ static int smiapp_pll_update(struct smiapp_sensor *sensor)
 	struct smiapp_pll *pll = &sensor->pll;
 	int rval;
 
-	if (sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0) {
-		/*
-		 * Fill in operational clock divisors limits from the
-		 * video timing ones. On profile 0 sensors the
-		 * requirements regarding them are essentially the
-		 * same as on VT ones.
-		 */
-		lim.op = lim.vt;
-	}
-
 	pll->binning_horizontal = sensor->binning_horizontal;
 	pll->binning_vertical = sensor->binning_vertical;
 	pll->link_freq =
-- 
1.7.10.4


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

* [PATCH v2 10/18] smiapp: Remove validation of op_pix_clk_div
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (8 preceding siblings ...)
  2014-10-02  8:45 ` [PATCH v2 09/18] smiapp: The PLL calculator handles sensors with VT clocks only Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 11/18] smiapp-pll: Add pixel rate in pixel array as output parameters Sakari Ailus
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

op_pix_clk_div is directly assigned and not calculated. There's no need to
verify it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp-pll.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index 862ca0c..0d5c503 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -116,11 +116,6 @@ static int check_all_bounds(struct device *dev,
 			"op_sys_clk_div");
 	if (!rval)
 		rval = bounds_check(
-			dev, op_pll->pix_clk_div,
-			op_limits->min_pix_clk_div, op_limits->max_pix_clk_div,
-			"op_pix_clk_div");
-	if (!rval)
-		rval = bounds_check(
 			dev, op_pll->sys_clk_freq_hz,
 			op_limits->min_sys_clk_freq_hz,
 			op_limits->max_sys_clk_freq_hz,
-- 
1.7.10.4


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

* [PATCH v2 11/18] smiapp-pll: Add pixel rate in pixel array as output parameters
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (9 preceding siblings ...)
  2014-10-02  8:46 ` [PATCH v2 10/18] smiapp: Remove validation of op_pix_clk_div Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 12/18] smiapp: Use actual pixel rate calculated by the PLL calculator Sakari Ailus
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

The actual pixel array pixel rate may be something else than vt_pix_clk_freq
on some implementations. Add a new field which contains the corrected value.

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

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index 0d5c503..e40d902 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -391,6 +391,7 @@ static int __smiapp_pll_calculate(
 out_skip_vt_calc:
 	pll->pixel_rate_csi =
 		op_pll->pix_clk_freq_hz * lane_op_clock_ratio;
+	pll->pixel_rate_pixel_array = pll->vt.pix_clk_freq_hz;
 
 	return check_all_bounds(dev, limits, op_limits, pll, op_pll);
 }
diff --git a/drivers/media/i2c/smiapp-pll.h b/drivers/media/i2c/smiapp-pll.h
index b7c0e66..e8f035a 100644
--- a/drivers/media/i2c/smiapp-pll.h
+++ b/drivers/media/i2c/smiapp-pll.h
@@ -71,6 +71,7 @@ struct smiapp_pll {
 	struct smiapp_pll_branch op;
 
 	uint32_t pixel_rate_csi;
+	uint32_t pixel_rate_pixel_array;
 };
 
 struct smiapp_pll_branch_limits {
-- 
1.7.10.4


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

* [PATCH v2 12/18] smiapp: Use actual pixel rate calculated by the PLL calculator
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (10 preceding siblings ...)
  2014-10-02  8:46 ` [PATCH v2 11/18] smiapp-pll: Add pixel rate in pixel array as output parameters Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 13/18] smiapp: Split calculating PLL with sensor's limits from updating it Sakari Ailus
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index d0ea7a3..861312e 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -289,7 +289,7 @@ static int smiapp_pll_update(struct smiapp_sensor *sensor)
 		return rval;
 
 	__v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_parray,
-				 pll->vt.pix_clk_freq_hz);
+				 pll->pixel_rate_pixel_array);
 	__v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_csi, pll->pixel_rate_csi);
 
 	return 0;
@@ -894,7 +894,7 @@ static int smiapp_update_mode(struct smiapp_sensor *sensor)
 	dev_dbg(&client->dev, "hblank\t\t%d\n", sensor->hblank->val);
 
 	dev_dbg(&client->dev, "real timeperframe\t100/%d\n",
-		sensor->pll.vt.pix_clk_freq_hz /
+		sensor->pll.pixel_rate_pixel_array /
 		((sensor->pixel_array->crop[SMIAPP_PA_PAD_SRC].width
 		  + sensor->hblank->val) *
 		 (sensor->pixel_array->crop[SMIAPP_PA_PAD_SRC].height
-- 
1.7.10.4


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

* [PATCH v2 13/18] smiapp: Split calculating PLL with sensor's limits from updating it
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (11 preceding siblings ...)
  2014-10-02  8:46 ` [PATCH v2 12/18] smiapp: Use actual pixel rate calculated by the PLL calculator Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 14/18] smiapp: Gather information on valid link rate and BPP combinations Sakari Ailus
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

From: Sakari Ailus <sakari.ailus@linux.intel.com>

The first one is handy for just trying out a PLL configuration without a
need to apply it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 861312e..4d3dc25 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -240,7 +240,8 @@ static int smiapp_pll_configure(struct smiapp_sensor *sensor)
 		sensor, SMIAPP_REG_U16_OP_SYS_CLK_DIV, pll->op.sys_clk_div);
 }
 
-static int smiapp_pll_update(struct smiapp_sensor *sensor)
+static int smiapp_pll_try(struct smiapp_sensor *sensor,
+			  struct smiapp_pll *pll)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
 	struct smiapp_pll_limits lim = {
@@ -274,6 +275,12 @@ static int smiapp_pll_update(struct smiapp_sensor *sensor)
 		.min_line_length_pck_bin = sensor->limits[SMIAPP_LIMIT_MIN_LINE_LENGTH_PCK_BIN],
 		.min_line_length_pck = sensor->limits[SMIAPP_LIMIT_MIN_LINE_LENGTH_PCK],
 	};
+
+	return smiapp_pll_calculate(&client->dev, &lim, pll);
+}
+
+static int smiapp_pll_update(struct smiapp_sensor *sensor)
+{
 	struct smiapp_pll *pll = &sensor->pll;
 	int rval;
 
@@ -284,7 +291,7 @@ static int smiapp_pll_update(struct smiapp_sensor *sensor)
 	pll->scale_m = sensor->scale_m;
 	pll->bits_per_pixel = sensor->csi_format->compressed;
 
-	rval = smiapp_pll_calculate(&client->dev, &lim, pll);
+	rval = smiapp_pll_try(sensor, pll);
 	if (rval < 0)
 		return rval;
 
-- 
1.7.10.4


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

* [PATCH v2 14/18] smiapp: Gather information on valid link rate and BPP combinations
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (12 preceding siblings ...)
  2014-10-02  8:46 ` [PATCH v2 13/18] smiapp: Split calculating PLL with sensor's limits from updating it Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 15/18] smiapp: Take valid link frequencies into account in supported mbus codes Sakari Ailus
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

Not all link rates are possible with all BPP values.

Also rearrange other initialisation a little. Obtaining possible PLL
configurations earlier requires that.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |   69 ++++++++++++++++++++++++--------
 drivers/media/i2c/smiapp/smiapp.h      |    8 ++++
 2 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 4d3dc25..d65521a 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -742,6 +742,7 @@ static int smiapp_get_limits_binning(struct smiapp_sensor *sensor)
 static int smiapp_get_mbus_formats(struct smiapp_sensor *sensor)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
+	struct smiapp_pll *pll = &sensor->pll;
 	unsigned int type, n;
 	unsigned int i, pixel_order;
 	int rval;
@@ -816,6 +817,41 @@ static int smiapp_get_mbus_formats(struct smiapp_sensor *sensor)
 		}
 	}
 
+	/* Figure out which BPP values can be used with which formats. */
+	pll->binning_horizontal = 1;
+	pll->binning_vertical = 1;
+	pll->scale_m = sensor->scale_m;
+
+	for (i = 0; i < ARRAY_SIZE(smiapp_csi_data_formats); i++) {
+		const struct smiapp_csi_data_format *f =
+			&smiapp_csi_data_formats[i];
+		unsigned long *valid_link_freqs =
+			&sensor->valid_link_freqs[
+				f->compressed - SMIAPP_COMPRESSED_BASE];
+		unsigned int j;
+
+		BUG_ON(f->compressed < SMIAPP_COMPRESSED_BASE);
+		BUG_ON(f->compressed > SMIAPP_COMPRESSED_MAX);
+
+		if (!(sensor->default_mbus_frame_fmts & 1 << i))
+			continue;
+
+		pll->bits_per_pixel = f->compressed;
+
+		for (j = 0; sensor->platform_data->op_sys_clock[j]; j++) {
+			pll->link_freq = sensor->platform_data->op_sys_clock[j];
+
+			rval = smiapp_pll_try(sensor, pll);
+			dev_dbg(&client->dev, "link freq %u Hz, bpp %u %s\n",
+				pll->link_freq, pll->bits_per_pixel,
+				rval ? "not ok" : "ok");
+			if (rval)
+				continue;
+
+			set_bit(j, valid_link_freqs);
+		}
+	}
+
 	if (!sensor->csi_format) {
 		dev_err(&client->dev, "no supported mbus code found\n");
 		return -EINVAL;
@@ -2479,12 +2515,6 @@ static int smiapp_registered(struct v4l2_subdev *subdev)
 		goto out_power_off;
 	}
 
-	rval = smiapp_get_mbus_formats(sensor);
-	if (rval) {
-		rval = -ENODEV;
-		goto out_power_off;
-	}
-
 	if (sensor->limits[SMIAPP_LIMIT_BINNING_CAPABILITY]) {
 		u32 val;
 
@@ -2566,6 +2596,22 @@ static int smiapp_registered(struct v4l2_subdev *subdev)
 
 	sensor->scale_m = sensor->limits[SMIAPP_LIMIT_SCALER_N_MIN];
 
+	/* prepare PLL configuration input values */
+	pll->bus_type = SMIAPP_PLL_BUS_TYPE_CSI2;
+	pll->csi2.lanes = sensor->platform_data->lanes;
+	pll->ext_clk_freq_hz = sensor->platform_data->ext_clk;
+	pll->flags = smiapp_call_quirk(sensor, pll_flags);
+	pll->scale_n = sensor->limits[SMIAPP_LIMIT_SCALER_N_MIN];
+	/* Profile 0 sensors have no separate OP clock branch. */
+	if (sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0)
+		pll->flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS;
+
+	rval = smiapp_get_mbus_formats(sensor);
+	if (rval) {
+		rval = -ENODEV;
+		goto out_nvm_release;
+	}
+
 	for (i = 0; i < SMIAPP_SUBDEVS; i++) {
 		struct {
 			struct smiapp_subdev *ssd;
@@ -2663,17 +2709,6 @@ static int smiapp_registered(struct v4l2_subdev *subdev)
 	if (rval < 0)
 		goto out_nvm_release;
 
-	/* prepare PLL configuration input values */
-	pll->bus_type = SMIAPP_PLL_BUS_TYPE_CSI2;
-	pll->csi2.lanes = sensor->platform_data->lanes;
-	pll->ext_clk_freq_hz = sensor->platform_data->ext_clk;
-	pll->flags = smiapp_call_quirk(sensor, pll_flags);
-
-	/* Profile 0 sensors have no separate OP clock branch. */
-	if (sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0)
-		pll->flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS;
-	pll->scale_n = sensor->limits[SMIAPP_LIMIT_SCALER_N_MIN];
-
 	mutex_lock(&sensor->mutex);
 	rval = smiapp_update_mode(sensor);
 	mutex_unlock(&sensor->mutex);
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index 874b49f..f88f8ec 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -156,6 +156,11 @@ struct smiapp_csi_data_format {
 #define SMIAPP_PAD_SRC			1
 #define SMIAPP_PADS			2
 
+#define SMIAPP_COMPRESSED_BASE		8
+#define SMIAPP_COMPRESSED_MAX		12
+#define SMIAPP_NR_OF_COMPRESSED		(SMIAPP_COMPRESSED_MAX - \
+					 SMIAPP_COMPRESSED_BASE + 1)
+
 struct smiapp_binning_subtype {
 	u8 horizontal:4;
 	u8 vertical:4;
@@ -232,6 +237,9 @@ struct smiapp_sensor {
 
 	struct smiapp_pll pll;
 
+	/* Is a default format supported for a given BPP? */
+	unsigned long valid_link_freqs[SMIAPP_NR_OF_COMPRESSED];
+
 	/* Pixel array controls */
 	struct v4l2_ctrl *analog_gain;
 	struct v4l2_ctrl *exposure;
-- 
1.7.10.4


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

* [PATCH v2 15/18] smiapp: Take valid link frequencies into account in supported mbus codes
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (13 preceding siblings ...)
  2014-10-02  8:46 ` [PATCH v2 14/18] smiapp: Gather information on valid link rate and BPP combinations Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 16/18] smiapp: Clean up smiapp_set_format() Sakari Ailus
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

Some media bus codes may be unavailable depending on the available media bus
codes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index d65521a..926f60c 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -806,14 +806,6 @@ static int smiapp_get_mbus_formats(struct smiapp_sensor *sensor)
 			dev_dbg(&client->dev, "jolly good! %d\n", j);
 
 			sensor->default_mbus_frame_fmts |= 1 << j;
-			if (!sensor->csi_format
-			    || f->width > sensor->csi_format->width
-			    || (f->width == sensor->csi_format->width
-				&& f->compressed
-				> sensor->csi_format->compressed)) {
-				sensor->csi_format = f;
-				sensor->internal_csi_format = f;
-			}
 		}
 	}
 
@@ -850,6 +842,22 @@ static int smiapp_get_mbus_formats(struct smiapp_sensor *sensor)
 
 			set_bit(j, valid_link_freqs);
 		}
+
+		if (!*valid_link_freqs) {
+			dev_info(&client->dev,
+				 "no valid link frequencies for %u bpp\n",
+				 f->compressed);
+			sensor->default_mbus_frame_fmts &= ~BIT(i);
+			continue;
+		}
+
+		if (!sensor->csi_format
+		    || f->width > sensor->csi_format->width
+		    || (f->width == sensor->csi_format->width
+			&& f->compressed > sensor->csi_format->compressed)) {
+			sensor->csi_format = f;
+			sensor->internal_csi_format = f;
+		}
 	}
 
 	if (!sensor->csi_format) {
-- 
1.7.10.4


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

* [PATCH v2 16/18] smiapp: Clean up smiapp_set_format()
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (14 preceding siblings ...)
  2014-10-02  8:46 ` [PATCH v2 15/18] smiapp: Take valid link frequencies into account in supported mbus codes Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  2014-10-02 12:09   ` [PATCH v2.1 " Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 17/18] smiapp: Set valid link frequency range Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 18/18] smiapp: Update PLL when setting format Sakari Ailus
  17 siblings, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

smiapp_set_format() has accumulated a fair amount of changes without a
needed refactoring, do the cleanup now. There's also an unlocked version of
v4l2_ctrl_range_changed(), using that fixes a small serialisation issue with
the user space interface.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |   65 +++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 926f60c..cf8eba8 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -1728,6 +1728,42 @@ static const struct smiapp_csi_data_format
 	return csi_format;
 }
 
+static int smiapp_set_format_source(struct v4l2_subdev *subdev,
+				    struct v4l2_subdev_fh *fh,
+				    struct v4l2_subdev_format *fmt)
+{
+	struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
+	const struct smiapp_csi_data_format *csi_format,
+		*old_csi_format = sensor->csi_format;
+	u32 code = fmt->format.code;
+	unsigned int i;
+	int rval;
+
+	rval = __smiapp_get_format(subdev, fh, fmt);
+	if (rval)
+		return rval;
+
+	if (subdev != &sensor->src->sd)
+		return 0;
+
+	csi_format = smiapp_validate_csi_data_format(sensor, code);
+
+	fmt->format.code = csi_format->code;
+
+	if (fmt->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+		return 0;
+
+	sensor->csi_format = csi_format;
+
+	if (csi_format->width != old_csi_format->width)
+		for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++)
+			__v4l2_ctrl_modify_range(
+				sensor->test_data[i], 0,
+				(1 << csi_format->width) - 1, 1, 0);
+
+	return 0;
+}
+
 static int smiapp_set_format(struct v4l2_subdev *subdev,
 			     struct v4l2_subdev_fh *fh,
 			     struct v4l2_subdev_format *fmt)
@@ -1743,36 +1779,13 @@ static int smiapp_set_format(struct v4l2_subdev *subdev,
 	 * other source pads we just get format here.
 	 */
 	if (fmt->pad == ssd->source_pad) {
-		u32 code = fmt->format.code;
-		int rval = __smiapp_get_format(subdev, fh, fmt);
-		bool range_changed = false;
-		unsigned int i;
-
-		if (!rval && subdev == &sensor->src->sd) {
-			const struct smiapp_csi_data_format *csi_format =
-				smiapp_validate_csi_data_format(sensor, code);
+		int rval;
 
-			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
-				if (csi_format->width !=
-				    sensor->csi_format->width)
-					range_changed = true;
-
-				sensor->csi_format = csi_format;
-			}
-
-			fmt->format.code = csi_format->code;
-		}
+		rval = smiapp_set_format_source(subdev, fh, fmt);
 
 		mutex_unlock(&sensor->mutex);
-		if (rval || !range_changed)
-			return rval;
-
-		for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++)
-			v4l2_ctrl_modify_range(
-				sensor->test_data[i],
-				0, (1 << sensor->csi_format->width) - 1, 1, 0);
 
-		return 0;
+		return rval;
 	}
 
 	/* Sink pad. Width and height are changeable here. */
-- 
1.7.10.4


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

* [PATCH v2 17/18] smiapp: Set valid link frequency range
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (15 preceding siblings ...)
  2014-10-02  8:46 ` [PATCH v2 16/18] smiapp: Clean up smiapp_set_format() Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  2014-10-02  8:46 ` [PATCH v2 18/18] smiapp: Update PLL when setting format Sakari Ailus
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

Set supported link frequencies in the menu in control initialisation and
when the bpp changes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |   19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index cf8eba8..87d4d5a 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -523,6 +523,8 @@ static const struct v4l2_ctrl_ops smiapp_ctrl_ops = {
 static int smiapp_init_controls(struct smiapp_sensor *sensor)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
+	unsigned long *valid_link_freqs = &sensor->valid_link_freqs[
+		sensor->csi_format->compressed - SMIAPP_COMPRESSED_BASE];
 	unsigned int max, i;
 	int rval;
 
@@ -605,8 +607,8 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor)
 
 	sensor->link_freq = v4l2_ctrl_new_int_menu(
 		&sensor->src->ctrl_handler, &smiapp_ctrl_ops,
-		V4L2_CID_LINK_FREQ, max, 0,
-		sensor->platform_data->op_sys_clock);
+		V4L2_CID_LINK_FREQ, __fls(*valid_link_freqs),
+		__ffs(*valid_link_freqs), sensor->platform_data->op_sys_clock);
 
 	sensor->pixel_rate_csi = v4l2_ctrl_new_std(
 		&sensor->src->ctrl_handler, &smiapp_ctrl_ops,
@@ -1735,6 +1737,7 @@ static int smiapp_set_format_source(struct v4l2_subdev *subdev,
 	struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
 	const struct smiapp_csi_data_format *csi_format,
 		*old_csi_format = sensor->csi_format;
+	unsigned long *valid_link_freqs;
 	u32 code = fmt->format.code;
 	unsigned int i;
 	int rval;
@@ -1761,6 +1764,18 @@ static int smiapp_set_format_source(struct v4l2_subdev *subdev,
 				sensor->test_data[i], 0,
 				(1 << csi_format->width) - 1, 1, 0);
 
+	if (csi_format->compressed == old_csi_format->compressed)
+		return 0;
+
+	valid_link_freqs = 
+		&sensor->valid_link_freqs[sensor->csi_format->compressed
+					  - SMIAPP_COMPRESSED_BASE];
+
+	__v4l2_ctrl_modify_range(
+		sensor->link_freq, 0,
+		__fls(*valid_link_freqs), ~*valid_link_freqs,
+		__ffs(*valid_link_freqs));
+
 	return 0;
 }
 
-- 
1.7.10.4


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

* [PATCH v2 18/18] smiapp: Update PLL when setting format
  2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
                   ` (16 preceding siblings ...)
  2014-10-02  8:46 ` [PATCH v2 17/18] smiapp: Set valid link frequency range Sakari Ailus
@ 2014-10-02  8:46 ` Sakari Ailus
  17 siblings, 0 replies; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02  8:46 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

The media bus format BPP does affect PLL. Recalculate PLL if the format
changes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 87d4d5a..c938778 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -1776,7 +1776,7 @@ static int smiapp_set_format_source(struct v4l2_subdev *subdev,
 		__fls(*valid_link_freqs), ~*valid_link_freqs,
 		__ffs(*valid_link_freqs));
 
-	return 0;
+	return smiapp_pll_update(sensor);
 }
 
 static int smiapp_set_format(struct v4l2_subdev *subdev,
-- 
1.7.10.4


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

* [PATCH v2.1 16/18] smiapp: Clean up smiapp_set_format()
  2014-10-02  8:46 ` [PATCH v2 16/18] smiapp: Clean up smiapp_set_format() Sakari Ailus
@ 2014-10-02 12:09   ` Sakari Ailus
  2014-10-02 13:53     ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2014-10-02 12:09 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

smiapp_set_format() has accumulated a fair amount of changes without a
needed refactoring, do the cleanup now. There's also an unlocked version of
v4l2_ctrl_range_changed(), using that fixes a small serialisation issue with
the user space interface.

__v4l2_ctrl_modify_range() is used instead of v4l2_ctrl_modify_range() in
smiapp_set_format_source() since the mutex is now held during the function
call.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c |   73 +++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 30 deletions(-)

since v2:

- Move comment on changed media bus codes to smiapp_set_format_source().

- Add a comment to the patch description on the use of the unlocked variant
  of v4l2_ctrl_modify_range().

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 926f60c..416b7bd 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -1728,51 +1728,64 @@ static const struct smiapp_csi_data_format
 	return csi_format;
 }
 
-static int smiapp_set_format(struct v4l2_subdev *subdev,
-			     struct v4l2_subdev_fh *fh,
-			     struct v4l2_subdev_format *fmt)
+static int smiapp_set_format_source(struct v4l2_subdev *subdev,
+				    struct v4l2_subdev_fh *fh,
+				    struct v4l2_subdev_format *fmt)
 {
 	struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
-	struct smiapp_subdev *ssd = to_smiapp_subdev(subdev);
-	struct v4l2_rect *crops[SMIAPP_PADS];
+	const struct smiapp_csi_data_format *csi_format,
+		*old_csi_format = sensor->csi_format;
+	u32 code = fmt->format.code;
+	unsigned int i;
+	int rval;
 
-	mutex_lock(&sensor->mutex);
+	rval = __smiapp_get_format(subdev, fh, fmt);
+	if (rval)
+		return rval;
 
 	/*
 	 * Media bus code is changeable on src subdev's source pad. On
 	 * other source pads we just get format here.
 	 */
-	if (fmt->pad == ssd->source_pad) {
-		u32 code = fmt->format.code;
-		int rval = __smiapp_get_format(subdev, fh, fmt);
-		bool range_changed = false;
-		unsigned int i;
-
-		if (!rval && subdev == &sensor->src->sd) {
-			const struct smiapp_csi_data_format *csi_format =
-				smiapp_validate_csi_data_format(sensor, code);
+	if (subdev != &sensor->src->sd)
+		return 0;
 
-			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
-				if (csi_format->width !=
-				    sensor->csi_format->width)
-					range_changed = true;
+	csi_format = smiapp_validate_csi_data_format(sensor, code);
 
-				sensor->csi_format = csi_format;
-			}
+	fmt->format.code = csi_format->code;
 
-			fmt->format.code = csi_format->code;
-		}
+	if (fmt->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+		return 0;
 
-		mutex_unlock(&sensor->mutex);
-		if (rval || !range_changed)
-			return rval;
+	sensor->csi_format = csi_format;
 
+	if (csi_format->width != old_csi_format->width)
 		for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++)
-			v4l2_ctrl_modify_range(
-				sensor->test_data[i],
-				0, (1 << sensor->csi_format->width) - 1, 1, 0);
+			__v4l2_ctrl_modify_range(
+				sensor->test_data[i], 0,
+				(1 << csi_format->width) - 1, 1, 0);
 
-		return 0;
+	return 0;
+}
+
+static int smiapp_set_format(struct v4l2_subdev *subdev,
+			     struct v4l2_subdev_fh *fh,
+			     struct v4l2_subdev_format *fmt)
+{
+	struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
+	struct smiapp_subdev *ssd = to_smiapp_subdev(subdev);
+	struct v4l2_rect *crops[SMIAPP_PADS];
+
+	mutex_lock(&sensor->mutex);
+
+	if (fmt->pad == ssd->source_pad) {
+		int rval;
+
+		rval = smiapp_set_format_source(subdev, fh, fmt);
+
+		mutex_unlock(&sensor->mutex);
+
+		return rval;
 	}
 
 	/* Sink pad. Width and height are changeable here. */
-- 
1.7.10.4


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

* Re: [PATCH v2.1 16/18] smiapp: Clean up smiapp_set_format()
  2014-10-02 12:09   ` [PATCH v2.1 " Sakari Ailus
@ 2014-10-02 13:53     ` Laurent Pinchart
  0 siblings, 0 replies; 21+ messages in thread
From: Laurent Pinchart @ 2014-10-02 13:53 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media

Hi Sakari,

Thank you for the patch.

On Thursday 02 October 2014 15:09:14 Sakari Ailus wrote:
> smiapp_set_format() has accumulated a fair amount of changes without a
> needed refactoring, do the cleanup now. There's also an unlocked version of
> v4l2_ctrl_range_changed(), using that fixes a small serialisation issue with
> the user space interface.
> 
> __v4l2_ctrl_modify_range() is used instead of v4l2_ctrl_modify_range() in
> smiapp_set_format_source() since the mutex is now held during the function
> call.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

For the whole series,

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

> ---
>  drivers/media/i2c/smiapp/smiapp-core.c |   73 ++++++++++++++++-------------
>  1 file changed, 43 insertions(+), 30 deletions(-)
> 
> since v2:
> 
> - Move comment on changed media bus codes to smiapp_set_format_source().
> 
> - Add a comment to the patch description on the use of the unlocked variant
>   of v4l2_ctrl_modify_range().
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> b/drivers/media/i2c/smiapp/smiapp-core.c index 926f60c..416b7bd 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -1728,51 +1728,64 @@ static const struct smiapp_csi_data_format
>  	return csi_format;
>  }
> 
> -static int smiapp_set_format(struct v4l2_subdev *subdev,
> -			     struct v4l2_subdev_fh *fh,
> -			     struct v4l2_subdev_format *fmt)
> +static int smiapp_set_format_source(struct v4l2_subdev *subdev,
> +				    struct v4l2_subdev_fh *fh,
> +				    struct v4l2_subdev_format *fmt)
>  {
>  	struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
> -	struct smiapp_subdev *ssd = to_smiapp_subdev(subdev);
> -	struct v4l2_rect *crops[SMIAPP_PADS];
> +	const struct smiapp_csi_data_format *csi_format,
> +		*old_csi_format = sensor->csi_format;
> +	u32 code = fmt->format.code;
> +	unsigned int i;
> +	int rval;
> 
> -	mutex_lock(&sensor->mutex);
> +	rval = __smiapp_get_format(subdev, fh, fmt);
> +	if (rval)
> +		return rval;
> 
>  	/*
>  	 * Media bus code is changeable on src subdev's source pad. On
>  	 * other source pads we just get format here.
>  	 */
> -	if (fmt->pad == ssd->source_pad) {
> -		u32 code = fmt->format.code;
> -		int rval = __smiapp_get_format(subdev, fh, fmt);
> -		bool range_changed = false;
> -		unsigned int i;
> -
> -		if (!rval && subdev == &sensor->src->sd) {
> -			const struct smiapp_csi_data_format *csi_format =
> -				smiapp_validate_csi_data_format(sensor, code);
> +	if (subdev != &sensor->src->sd)
> +		return 0;
> 
> -			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> -				if (csi_format->width !=
> -				    sensor->csi_format->width)
> -					range_changed = true;
> +	csi_format = smiapp_validate_csi_data_format(sensor, code);
> 
> -				sensor->csi_format = csi_format;
> -			}
> +	fmt->format.code = csi_format->code;
> 
> -			fmt->format.code = csi_format->code;
> -		}
> +	if (fmt->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> +		return 0;
> 
> -		mutex_unlock(&sensor->mutex);
> -		if (rval || !range_changed)
> -			return rval;
> +	sensor->csi_format = csi_format;
> 
> +	if (csi_format->width != old_csi_format->width)
>  		for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++)
> -			v4l2_ctrl_modify_range(
> -				sensor->test_data[i],
> -				0, (1 << sensor->csi_format->width) - 1, 1, 0);
> +			__v4l2_ctrl_modify_range(
> +				sensor->test_data[i], 0,
> +				(1 << csi_format->width) - 1, 1, 0);
> 
> -		return 0;
> +	return 0;
> +}
> +
> +static int smiapp_set_format(struct v4l2_subdev *subdev,
> +			     struct v4l2_subdev_fh *fh,
> +			     struct v4l2_subdev_format *fmt)
> +{
> +	struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
> +	struct smiapp_subdev *ssd = to_smiapp_subdev(subdev);
> +	struct v4l2_rect *crops[SMIAPP_PADS];
> +
> +	mutex_lock(&sensor->mutex);
> +
> +	if (fmt->pad == ssd->source_pad) {
> +		int rval;
> +
> +		rval = smiapp_set_format_source(subdev, fh, fmt);
> +
> +		mutex_unlock(&sensor->mutex);
> +
> +		return rval;
>  	}
> 
>  	/* Sink pad. Width and height are changeable here. */

-- 
Regards,

Laurent Pinchart


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

end of thread, other threads:[~2014-10-02 13:53 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-02  8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 01/18] smiapp: Take mutex during PLL update in sensor initialisation Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 02/18] smiapp-pll: Correct clock debug prints Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 03/18] smiapp-pll: The clock tree values are unsigned --- fix " Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 04/18] smiapp-pll: Separate bounds checking into a separate function Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 05/18] smiapp-pll: External clock frequency isn't an output value Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 06/18] smiapp-pll: Unify OP and VT PLL structs Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 07/18] smiapp-pll: Calculate OP clocks only for sensors that have them Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 08/18] smiapp-pll: Don't validate OP clocks if there are none Sakari Ailus
2014-10-02  8:45 ` [PATCH v2 09/18] smiapp: The PLL calculator handles sensors with VT clocks only Sakari Ailus
2014-10-02  8:46 ` [PATCH v2 10/18] smiapp: Remove validation of op_pix_clk_div Sakari Ailus
2014-10-02  8:46 ` [PATCH v2 11/18] smiapp-pll: Add pixel rate in pixel array as output parameters Sakari Ailus
2014-10-02  8:46 ` [PATCH v2 12/18] smiapp: Use actual pixel rate calculated by the PLL calculator Sakari Ailus
2014-10-02  8:46 ` [PATCH v2 13/18] smiapp: Split calculating PLL with sensor's limits from updating it Sakari Ailus
2014-10-02  8:46 ` [PATCH v2 14/18] smiapp: Gather information on valid link rate and BPP combinations Sakari Ailus
2014-10-02  8:46 ` [PATCH v2 15/18] smiapp: Take valid link frequencies into account in supported mbus codes Sakari Ailus
2014-10-02  8:46 ` [PATCH v2 16/18] smiapp: Clean up smiapp_set_format() Sakari Ailus
2014-10-02 12:09   ` [PATCH v2.1 " Sakari Ailus
2014-10-02 13:53     ` Laurent Pinchart
2014-10-02  8:46 ` [PATCH v2 17/18] smiapp: Set valid link frequency range Sakari Ailus
2014-10-02  8:46 ` [PATCH v2 18/18] smiapp: Update PLL when setting format Sakari Ailus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.