* [PATCH 01/17] smiapp-pll: Correct clock debug prints
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 02/17] smiapp-pll: The clock tree values are unsigned --- fix " Sakari Ailus
` (15 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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] 20+ messages in thread* [PATCH 02/17] smiapp-pll: The clock tree values are unsigned --- fix debug prints
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
2014-09-17 20:45 ` [PATCH 01/17] smiapp-pll: Correct clock debug prints Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 03/17] smiapp-pll: Separate bounds checking into a separate function Sakari Ailus
` (14 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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] 20+ messages in thread* [PATCH 03/17] smiapp-pll: Separate bounds checking into a separate function
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
2014-09-17 20:45 ` [PATCH 01/17] smiapp-pll: Correct clock debug prints Sakari Ailus
2014-09-17 20:45 ` [PATCH 02/17] smiapp-pll: The clock tree values are unsigned --- fix " Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 04/17] smiapp-pll: External clock frequency isn't an output value Sakari Ailus
` (13 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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] 20+ messages in thread* [PATCH 04/17] smiapp-pll: External clock frequency isn't an output value
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (2 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 03/17] smiapp-pll: Separate bounds checking into a separate function Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 05/17] smiapp-pll: Unify OP and VT PLL structs Sakari Ailus
` (12 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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] 20+ messages in thread* [PATCH 05/17] smiapp-pll: Unify OP and VT PLL structs
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (3 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 04/17] smiapp-pll: External clock frequency isn't an output value Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 06/17] smiapp-pll: Don't validate OP clocks if there are none Sakari Ailus
` (11 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Uniform representation for VT and OP clocks.
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 daa179d..2f81c9c 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] 20+ messages in thread* [PATCH 06/17] smiapp-pll: Don't validate OP clocks if there are none
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (4 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 05/17] smiapp-pll: Unify OP and VT PLL structs Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 07/17] smiapp-pll: Calculate OP clocks only for sensors that have them Sakari Ailus
` (10 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Profile 0 sensors have no OP clocks, don't validate them.
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 40a18ba..f83f25f 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -129,6 +129,14 @@ static int check_all_bounds(struct device *dev,
limits->op.min_pix_clk_freq_hz,
limits->op.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] 20+ messages in thread* [PATCH 07/17] smiapp-pll: Calculate OP clocks only for sensors that have them
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (5 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 06/17] smiapp-pll: Don't validate OP clocks if there are none Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 08/17] smiapp: The PLL calculator handles sensors with VT clocks only Sakari Ailus
` (9 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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 f83f25f..862ca0c 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");
/*
@@ -164,10 +166,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;
@@ -204,7 +207,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);
@@ -234,8 +237,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))
@@ -248,8 +251,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;
@@ -258,14 +261,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
@@ -293,7 +301,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);
@@ -385,16 +393,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;
@@ -402,6 +413,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
@@ -457,7 +478,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] 20+ messages in thread* [PATCH 08/17] smiapp: The PLL calculator handles sensors with VT clocks only
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (6 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 07/17] smiapp-pll: Calculate OP clocks only for sensors that have them Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 09/17] smiapp: Remove validation of op_pix_clk_div Sakari Ailus
` (8 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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 2f81c9c..54cb136 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] 20+ messages in thread* [PATCH 09/17] smiapp: Remove validation of op_pix_clk_div
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (7 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 08/17] smiapp: The PLL calculator handles sensors with VT clocks only Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 10/17] smiapp-pll: Add pixel rate in pixel array as output parameters Sakari Ailus
` (7 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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] 20+ messages in thread* [PATCH 10/17] smiapp-pll: Add pixel rate in pixel array as output parameters
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (8 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 09/17] smiapp: Remove validation of op_pix_clk_div Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 11/17] smiapp: Use actual pixel rate calculated by the PLL calculator Sakari Ailus
` (6 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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] 20+ messages in thread* [PATCH 11/17] smiapp: Use actual pixel rate calculated by the PLL calculator
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (9 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 10/17] smiapp-pll: Add pixel rate in pixel array as output parameters Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 12/17] smiapp: Take mutex during PLL update in sensor initialisation Sakari Ailus
` (5 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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 54cb136..6db0e8d 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] 20+ messages in thread* [PATCH 12/17] smiapp: Take mutex during PLL update in sensor initialisation
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (10 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 11/17] smiapp: Use actual pixel rate calculated by the PLL calculator Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 13/17] smiapp: Split calculating PLL with sensor's limits from updating it Sakari Ailus
` (4 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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 6db0e8d..346ff5b 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2667,7 +2667,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] 20+ messages in thread* [PATCH 13/17] smiapp: Split calculating PLL with sensor's limits from updating it
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (11 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 12/17] smiapp: Take mutex during PLL update in sensor initialisation Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 14/17] smiapp: Gather information on valid link rate and BPP combinations Sakari Ailus
` (3 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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 346ff5b..a1244e6 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] 20+ messages in thread* [PATCH 14/17] smiapp: Gather information on valid link rate and BPP combinations
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (12 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 13/17] smiapp: Split calculating PLL with sensor's limits from updating it Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 15/17] smiapp: Take valid link frequencies into account in supported mbus codes Sakari Ailus
` (2 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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 | 73 ++++++++++++++++++++++++--------
drivers/media/i2c/smiapp/smiapp.h | 8 ++++
2 files changed, 64 insertions(+), 17 deletions(-)
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index a1244e6..b108b15 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,45 @@ 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 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;
+
+ /* Did we already gather information for this BPP? */
+ if (sensor->valid_link_freqs[f->compressed
+ - SMIAPP_COMPRESSED_BASE])
+ continue;
+
+ pll->bits_per_pixel = f->compressed;
+
+ for (j = 0; j < 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_info(&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, &sensor->valid_link_freqs[
+ f->compressed
+ - SMIAPP_COMPRESSED_BASE]);
+ }
+ }
+
if (!sensor->csi_format) {
dev_err(&client->dev, "no supported mbus code found\n");
return -EINVAL;
@@ -2479,12 +2519,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 +2600,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_cleanup;
+ }
+
for (i = 0; i < SMIAPP_SUBDEVS; i++) {
struct {
struct smiapp_subdev *ssd;
@@ -2663,17 +2713,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] 20+ messages in thread* [PATCH 15/17] smiapp: Take valid link frequencies into account in supported mbus codes
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (13 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 14/17] smiapp: Gather information on valid link rate and BPP combinations Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 16/17] smiapp: Update PLL when setting format Sakari Ailus
2014-09-17 20:45 ` [PATCH 17/17] smiapp: Decrease link frequency if media bus pixel format BPP requires Sakari Ailus
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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 b108b15..798c129 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;
- }
}
}
@@ -854,6 +846,22 @@ static int smiapp_get_mbus_formats(struct smiapp_sensor *sensor)
f->compressed
- SMIAPP_COMPRESSED_BASE]);
}
+ if (!sensor->valid_link_freqs[f->compressed
+ - SMIAPP_COMPRESSED_BASE]) {
+ 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] 20+ messages in thread* [PATCH 16/17] smiapp: Update PLL when setting format
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (14 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 15/17] smiapp: Take valid link frequencies into account in supported mbus codes Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-17 20:45 ` [PATCH 17/17] smiapp: Decrease link frequency if media bus pixel format BPP requires Sakari Ailus
16 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
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 | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 798c129..537ca92 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -1758,8 +1758,18 @@ static int smiapp_set_format(struct v4l2_subdev *subdev,
if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
if (csi_format->width !=
- sensor->csi_format->width)
+ sensor->csi_format->width) {
+ int rval;
+
+ rval = smiapp_pll_update(sensor);
+ if (rval) {
+ mutex_unlock(&sensor->mutex);
+ return rval;
+ }
+
range_changed = true;
+ }
+
sensor->csi_format = csi_format;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 17/17] smiapp: Decrease link frequency if media bus pixel format BPP requires
2014-09-17 20:45 [PATCH 00/17] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
` (15 preceding siblings ...)
2014-09-17 20:45 ` [PATCH 16/17] smiapp: Update PLL when setting format Sakari Ailus
@ 2014-09-17 20:45 ` Sakari Ailus
2014-09-26 10:44 ` Laurent Pinchart
16 siblings, 1 reply; 20+ messages in thread
From: Sakari Ailus @ 2014-09-17 20:45 UTC (permalink / raw)
To: linux-media
Decrease the link frequency to the next lower if the user chooses a media
bus code (BPP) cannot be achieved using the selected link frequency.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/smiapp/smiapp-core.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 537ca92..ce2c34d 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -286,11 +286,27 @@ static int smiapp_pll_update(struct smiapp_sensor *sensor)
pll->binning_horizontal = sensor->binning_horizontal;
pll->binning_vertical = sensor->binning_vertical;
- pll->link_freq =
- sensor->link_freq->qmenu_int[sensor->link_freq->val];
pll->scale_m = sensor->scale_m;
pll->bits_per_pixel = sensor->csi_format->compressed;
+ if (!test_bit(sensor->link_freq->val,
+ &sensor->valid_link_freqs[
+ sensor->csi_format->compressed
+ - SMIAPP_COMPRESSED_BASE])) {
+ /*
+ * Setting the link frequency will perform PLL
+ * re-calculation already, so skip that.
+ */
+ return __v4l2_ctrl_s_ctrl(
+ sensor->link_freq,
+ __ffs(sensor->valid_link_freqs[
+ sensor->csi_format->compressed
+ - SMIAPP_COMPRESSED_BASE]));
+ }
+
+ pll->link_freq =
+ sensor->link_freq->qmenu_int[sensor->link_freq->val];
+
rval = smiapp_pll_try(sensor, pll);
if (rval < 0)
return rval;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 17/17] smiapp: Decrease link frequency if media bus pixel format BPP requires
2014-09-17 20:45 ` [PATCH 17/17] smiapp: Decrease link frequency if media bus pixel format BPP requires Sakari Ailus
@ 2014-09-26 10:44 ` Laurent Pinchart
2014-09-26 11:01 ` Sakari Ailus
0 siblings, 1 reply; 20+ messages in thread
From: Laurent Pinchart @ 2014-09-26 10:44 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media
Hi Sakari,
Thank you for the patch.
On Wednesday 17 September 2014 23:45:41 Sakari Ailus wrote:
> Decrease the link frequency to the next lower if the user chooses a media
> bus code (BPP) cannot be achieved using the selected link frequency.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/media/i2c/smiapp/smiapp-core.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> b/drivers/media/i2c/smiapp/smiapp-core.c index 537ca92..ce2c34d 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -286,11 +286,27 @@ static int smiapp_pll_update(struct smiapp_sensor
> *sensor)
>
> pll->binning_horizontal = sensor->binning_horizontal;
> pll->binning_vertical = sensor->binning_vertical;
> - pll->link_freq =
> - sensor->link_freq->qmenu_int[sensor->link_freq->val];
> pll->scale_m = sensor->scale_m;
> pll->bits_per_pixel = sensor->csi_format->compressed;
>
> + if (!test_bit(sensor->link_freq->val,
> + &sensor->valid_link_freqs[
> + sensor->csi_format->compressed
> + - SMIAPP_COMPRESSED_BASE])) {
> + /*
> + * Setting the link frequency will perform PLL
> + * re-calculation already, so skip that.
> + */
> + return __v4l2_ctrl_s_ctrl(
> + sensor->link_freq,
> + __ffs(sensor->valid_link_freqs[
> + sensor->csi_format->compressed
> + - SMIAPP_COMPRESSED_BASE]));
I have an uneasy feeling about this, as smiapp_pll_update is called from the
link freq s_ctrl handler. Have you double-checked the recursion bounds ?
> + }
> +
> + pll->link_freq =
> + sensor->link_freq->qmenu_int[sensor->link_freq->val];
> +
> rval = smiapp_pll_try(sensor, pll);
> if (rval < 0)
> return rval;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 17/17] smiapp: Decrease link frequency if media bus pixel format BPP requires
2014-09-26 10:44 ` Laurent Pinchart
@ 2014-09-26 11:01 ` Sakari Ailus
0 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2014-09-26 11:01 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media
Hi Laurent,
Thank you for your comments.
On Fri, Sep 26, 2014 at 01:44:03PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Wednesday 17 September 2014 23:45:41 Sakari Ailus wrote:
> > Decrease the link frequency to the next lower if the user chooses a media
> > bus code (BPP) cannot be achieved using the selected link frequency.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > drivers/media/i2c/smiapp/smiapp-core.c | 20 ++++++++++++++++++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> > b/drivers/media/i2c/smiapp/smiapp-core.c index 537ca92..ce2c34d 100644
> > --- a/drivers/media/i2c/smiapp/smiapp-core.c
> > +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> > @@ -286,11 +286,27 @@ static int smiapp_pll_update(struct smiapp_sensor
> > *sensor)
> >
> > pll->binning_horizontal = sensor->binning_horizontal;
> > pll->binning_vertical = sensor->binning_vertical;
> > - pll->link_freq =
> > - sensor->link_freq->qmenu_int[sensor->link_freq->val];
> > pll->scale_m = sensor->scale_m;
> > pll->bits_per_pixel = sensor->csi_format->compressed;
> >
> > + if (!test_bit(sensor->link_freq->val,
> > + &sensor->valid_link_freqs[
> > + sensor->csi_format->compressed
> > + - SMIAPP_COMPRESSED_BASE])) {
> > + /*
> > + * Setting the link frequency will perform PLL
> > + * re-calculation already, so skip that.
> > + */
> > + return __v4l2_ctrl_s_ctrl(
> > + sensor->link_freq,
> > + __ffs(sensor->valid_link_freqs[
> > + sensor->csi_format->compressed
> > + - SMIAPP_COMPRESSED_BASE]));
>
> I have an uneasy feeling about this, as smiapp_pll_update is called from the
> link freq s_ctrl handler. Have you double-checked the recursion bounds ?
We haven't actually done any PLL tree calculation yet here. The condition
will evaluate true in a case when the user chooses a format which isn't
available on a given link frequency, or chooses a link frequency which isn't
available for a given format.
The condition will be false the next time the function is called since we've
just chosen a valid combination of the two.
But now that you brought the topic up, I think the link frequency selection
should just probably return -EBUSY if the selected link frquency cannot be
used. Also __ffs() should be __fls() instead in order to still come up with
the highest link freqency.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 20+ messages in thread