* [PATCH v2 0/6] media: qcom: camss: share VFE clock sources on SM8250
@ 2026-10-24 9:40 Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Atanas Filipov @ 2026-10-24 9:40 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
On SM8250 several VFE clocks are fed by one clock source: the AREG
clocks of the two full VFEs share the fast AHB source, the two VFE Lite
share one core clock source, and all four VFEs share the camera NoC AXI
source. Each VFE sets such a source to its own rate, so the last VFE to
power on wins and can lower the rate under a VFE that is still running.
The CSID also sets the VFE core clocks to their lowest level when it
powers on after the VFE, and nothing sets the camera NoC AXI rate, so
the camera NoC runs from XO and two concurrent streams stall.
Two examples on RB5 (QRB5165):
- VFE0 streams the CSID test pattern and VFE1 then starts an imx577
stream. Nothing sets the camera NoC AXI rate, so the camera NoC stays
at XO (19.2 MHz) and one of the two streams stalls. With the series
the VFEs set it to 300 or 400 MHz and both streams complete.
- VFE Lite 0 streams the CSID test pattern at 480 MHz and VFE Lite 1
then starts an imx577 stream that needs 400 MHz. The two share one
core clock source, so VFE Lite 1 lowers it to 400 MHz under the
running VFE Lite 0. With the series the source stays at 480 MHz until
VFE Lite 0 stops.
This series lets the VFE own its clock rates and aggregates the shared
sources within CAMSS:
- Patch 1 moves the minimum rate computation out of the VFE clock
loops, with no functional change.
- Patch 2 clamps each VFE clock to its highest listed rate instead of
failing, as the VFE clock rate tables need not have the same number
of levels.
- Patch 3 stops the CSID from setting the VFE core clock rates.
- Patch 4 groups the VFE clocks by clock source, so that a shared
source runs at the highest rate requested by the running VFEs. It is
enabled per SoC, so far only on SM8250.
- Patch 5 lets the VFE set the AREG clock rate.
- Patch 6 drops the unused camera NoC AXI RCG from the VFE clocks and
lets the VFE set the camera NoC AXI rate.
The camera NoC AXI clock is also used by drivers outside CAMSS, and how
to aggregate their requests with the CAMSS one is still being discussed
on the CAMNOC interconnect series. Patch 6 does not depend on that
choice: it only aggregates the CAMSS VFEs, which share one source, and
fixes the concurrent stall today. Once the shared camera NoC clock is
represented, the VFE will request its rate through that interface
instead of setting the clock directly.
Tested on RB5 (QRB5165) with imx577 and ov9282 sensors and the CSID test
pattern generator, on single, concurrent test pattern plus sensor, and
concurrent dual sensor RDI captures on all four VFEs: all streams
complete, the shared sources stay at the highest requested rate while
another VFE runs, dedicated clocks drop to XO on power off, and the
camera NoC AXI clock runs at 300 or 400 MHz while streaming, without
the concurrent capture stalls seen before the series.
Changes in v2:
- Split the minimum rate computation out of the clock loops into its
own patch (Loic Poulain)
- Add the Fixes tag to the CSID patch (Bryan O'Donoghue)
- Split the clock grouping from the AREG clock change and add an
example to the grouping commit message (Bryan O'Donoghue)
- Rename the per-clock entries of a clock domain from user to vote
(Bryan O'Donoghue)
Atanas Filipov (6):
media: qcom: camss: factor out the VFE minimum clock rate
media: qcom: camss: clamp VFE clock rates to the highest level
media: qcom: camss: sm8250: do not set VFE core clock rates from CSID
media: qcom: camss: share VFE clock sources by the highest rate
media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE
media: qcom: camss: sm8250: set the camera NoC AXI rate from the VFE
drivers/media/platform/qcom/camss/camss-vfe.c | 138 ++++++-----
drivers/media/platform/qcom/camss/camss.c | 215 ++++++++++++++++--
drivers/media/platform/qcom/camss/camss.h | 13 ++
3 files changed, 272 insertions(+), 94 deletions(-)
base-commit: bd1ecbb61afefc6a8b876ccbc31e9228b44e9eab
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/6] media: qcom: camss: factor out the VFE minimum clock rate
2026-10-24 9:40 [PATCH v2 0/6] media: qcom: camss: share VFE clock sources on SM8250 Atanas Filipov
@ 2026-10-24 9:40 ` Atanas Filipov
2026-10-26 9:07 ` Bryan O'Donoghue
2026-10-24 9:40 ` [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Atanas Filipov @ 2026-10-24 9:40 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
vfe_set_clock_rates() and vfe_check_clock_rates() both compute the
minimum clock rate needed by the VFE lines from their pixel rates,
inside the loop over the VFE clocks. The result is the same for every
clock.
Move the computation into vfe_get_min_clock_rate() and call it once
per VFE, so that the clock loops only select and check rates. No
functional change.
Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss-vfe.c | 108 ++++++++----------
1 file changed, 46 insertions(+), 62 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 2680a9914765..59be37f83df3 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -935,52 +935,66 @@ static bool vfe_check_clock_levels(struct camss_clock *clock)
}
/*
- * vfe_set_clock_rates - Calculate and set clock rates on VFE module
+ * vfe_get_min_clock_rate - Get the minimum VFE clock rate for its lines
* @vfe: VFE device
*
- * Return 0 on success or a negative error code otherwise
+ * Return the highest rate needed by the VFE lines for their pixel rates,
+ * including the clock margin, or 0 if no pixel rate is available
*/
-static int vfe_set_clock_rates(struct vfe_device *vfe)
+static u64 vfe_get_min_clock_rate(struct vfe_device *vfe)
{
- struct device *dev = vfe->camss->dev;
u64 pixel_clock[VFE_LINE_NUM_MAX];
- int i, j;
- int ret;
+ u64 min_rate = 0;
+ int i;
for (i = VFE_LINE_RDI0; i < vfe->res->line_num; i++) {
- ret = camss_get_pixel_clock(&vfe->line[i].subdev.entity,
- &pixel_clock[i]);
- if (ret)
+ if (camss_get_pixel_clock(&vfe->line[i].subdev.entity,
+ &pixel_clock[i]))
pixel_clock[i] = 0;
}
- for (i = 0; i < vfe->nclocks; i++) {
- struct camss_clock *clock = &vfe->clock[i];
+ for (i = VFE_LINE_RDI0; i < vfe->res->line_num; i++) {
+ u32 tmp;
+ u8 bpp;
- if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
- u64 min_rate = 0;
- long rate;
+ if (i == VFE_LINE_PIX) {
+ tmp = pixel_clock[i];
+ } else {
+ struct vfe_line *l = &vfe->line[i];
- for (j = VFE_LINE_RDI0; j < vfe->res->line_num; j++) {
- u32 tmp;
- u8 bpp;
+ bpp = camss_format_get_bpp(l->formats,
+ l->nformats,
+ l->fmt[MSM_VFE_PAD_SINK].code);
+ tmp = pixel_clock[i] * bpp / 64;
+ }
- if (j == VFE_LINE_PIX) {
- tmp = pixel_clock[j];
- } else {
- struct vfe_line *l = &vfe->line[j];
+ if (min_rate < tmp)
+ min_rate = tmp;
+ }
- bpp = camss_format_get_bpp(l->formats,
- l->nformats,
- l->fmt[MSM_VFE_PAD_SINK].code);
- tmp = pixel_clock[j] * bpp / 64;
- }
+ camss_add_clock_margin(&min_rate);
- if (min_rate < tmp)
- min_rate = tmp;
- }
+ return min_rate;
+}
- camss_add_clock_margin(&min_rate);
+/*
+ * vfe_set_clock_rates - Calculate and set clock rates on VFE module
+ * @vfe: VFE device
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int vfe_set_clock_rates(struct vfe_device *vfe)
+{
+ struct device *dev = vfe->camss->dev;
+ u64 min_rate = vfe_get_min_clock_rate(vfe);
+ int i, j;
+ int ret;
+
+ for (i = 0; i < vfe->nclocks; i++) {
+ struct camss_clock *clock = &vfe->clock[i];
+
+ if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
+ long rate;
for (j = 0; j < clock->nfreqs; j++)
if (min_rate < clock->freq[j])
@@ -1024,45 +1038,15 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
*/
static int vfe_check_clock_rates(struct vfe_device *vfe)
{
- u64 pixel_clock[VFE_LINE_NUM_MAX];
- int i, j;
- int ret;
-
- for (i = VFE_LINE_RDI0; i < vfe->res->line_num; i++) {
- ret = camss_get_pixel_clock(&vfe->line[i].subdev.entity,
- &pixel_clock[i]);
- if (ret)
- pixel_clock[i] = 0;
- }
+ u64 min_rate = vfe_get_min_clock_rate(vfe);
+ int i;
for (i = 0; i < vfe->nclocks; i++) {
struct camss_clock *clock = &vfe->clock[i];
if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
- u64 min_rate = 0;
unsigned long rate;
- for (j = VFE_LINE_RDI0; j < vfe->res->line_num; j++) {
- u32 tmp;
- u8 bpp;
-
- if (j == VFE_LINE_PIX) {
- tmp = pixel_clock[j];
- } else {
- struct vfe_line *l = &vfe->line[j];
-
- bpp = camss_format_get_bpp(l->formats,
- l->nformats,
- l->fmt[MSM_VFE_PAD_SINK].code);
- tmp = pixel_clock[j] * bpp / 64;
- }
-
- if (min_rate < tmp)
- min_rate = tmp;
- }
-
- camss_add_clock_margin(&min_rate);
-
rate = clk_get_rate(clock->clk);
if (rate < min_rate)
return -EBUSY;
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level
2026-10-24 9:40 [PATCH v2 0/6] media: qcom: camss: share VFE clock sources on SM8250 Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
@ 2026-10-24 9:40 ` Atanas Filipov
2026-10-26 9:08 ` Bryan O'Donoghue
2026-10-26 9:28 ` Loic Poulain
2026-10-24 9:40 ` [PATCH v2 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID Atanas Filipov
` (3 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Atanas Filipov @ 2026-10-24 9:40 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
The VFE fails to power on when the pixel rate exceeds the highest rate
listed for any of its clocks. The rate tables of the VFE clocks are
independent and need not have the same number of levels, so a clock
with a lower top rate than the VFE core clock, such as the SC7280
camnoc_axi clock, makes the VFE reject pixel rates the core clock
still supports.
Clamp each clock to its highest listed rate instead, and warn that the
pixel rate exceeds it when the clock lists more than one rate. Take the
clamp into account when another line powers on an already running VFE,
so that a clock running at its highest rate does not reject the
request.
Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss-vfe.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 59be37f83df3..359b5942df89 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -996,15 +996,13 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
long rate;
- for (j = 0; j < clock->nfreqs; j++)
+ for (j = 0; j < clock->nfreqs - 1; j++)
if (min_rate < clock->freq[j])
break;
- if (j == clock->nfreqs) {
- dev_err(dev,
- "Pixel clock is too high for VFE");
- return -EINVAL;
- }
+ if (clock->nfreqs > 1 && min_rate >= clock->freq[j])
+ dev_warn(dev, "Pixel clock is too high for VFE clock %s\n",
+ clock->name);
/* if sensor pixel clock is not available */
/* set highest possible VFE clock rate */
@@ -1046,9 +1044,15 @@ static int vfe_check_clock_rates(struct vfe_device *vfe)
if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
unsigned long rate;
+ long max_rate;
+
+ /* A clock already at its highest rate cannot go higher */
+ max_rate = clk_round_rate(clock->clk, clock->freq[clock->nfreqs - 1]);
+ if (max_rate < 0)
+ return max_rate;
rate = clk_get_rate(clock->clk);
- if (rate < min_rate)
+ if (rate < min_t(u64, min_rate, max_rate))
return -EBUSY;
}
}
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID
2026-10-24 9:40 [PATCH v2 0/6] media: qcom: camss: share VFE clock sources on SM8250 Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
@ 2026-10-24 9:40 ` Atanas Filipov
2026-10-26 9:08 ` Bryan O'Donoghue
2026-10-24 9:40 ` [PATCH v2 4/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Atanas Filipov @ 2026-10-24 9:40 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
The SM8250 CSID resources list clock levels for the VFE core clocks and
for the shared VFE Lite core clock. CSID power on runs after the VFE is
powered on and sets these clocks to their lowest level, overriding the
rate selected by the VFE from the pixel rate and lowering the shared
VFE Lite clock under a running VFE Lite.
The VFE core clocks are set by the VFE. Drop their levels from the CSID
resources so that CSID only enables them.
Fixes: b4436a18eedb ("media: camss: add support for SM8250 camss")
Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index c54185c93a8a..7fe12049b78a 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -2052,7 +2052,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
.clock = { "vfe0_csid", "vfe0_cphy_rx", "vfe0", "vfe0_areg", "vfe0_ahb" },
.clock_rate = { { 400000000 },
{ 400000000 },
- { 350000000, 475000000, 576000000, 720000000 },
+ { 0 },
{ 100000000, 200000000, 300000000, 400000000 },
{ 0 } },
.reg = { "csid0" },
@@ -2069,7 +2069,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
.clock = { "vfe1_csid", "vfe1_cphy_rx", "vfe1", "vfe1_areg", "vfe1_ahb" },
.clock_rate = { { 400000000 },
{ 400000000 },
- { 350000000, 475000000, 576000000, 720000000 },
+ { 0 },
{ 100000000, 200000000, 300000000, 400000000 },
{ 0 } },
.reg = { "csid1" },
@@ -2086,7 +2086,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite", "vfe_lite_ahb" },
.clock_rate = { { 400000000 },
{ 400000000 },
- { 400000000, 480000000 },
+ { 0 },
{ 0 } },
.reg = { "csid2" },
.interrupt = { "csid2" },
@@ -2103,7 +2103,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite", "vfe_lite_ahb" },
.clock_rate = { { 400000000 },
{ 400000000 },
- { 400000000, 480000000 },
+ { 0 },
{ 0 } },
.reg = { "csid3" },
.interrupt = { "csid3" },
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 4/6] media: qcom: camss: share VFE clock sources by the highest rate
2026-10-24 9:40 [PATCH v2 0/6] media: qcom: camss: share VFE clock sources on SM8250 Atanas Filipov
` (2 preceding siblings ...)
2026-10-24 9:40 ` [PATCH v2 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID Atanas Filipov
@ 2026-10-24 9:40 ` Atanas Filipov
2026-10-26 9:13 ` Bryan O'Donoghue
2026-10-24 9:40 ` [PATCH v2 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 6/6] media: qcom: camss: sm8250: set the camera NoC AXI " Atanas Filipov
5 siblings, 1 reply; 18+ messages in thread
From: Atanas Filipov @ 2026-10-24 9:40 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
Clocks of different VFEs can be fed by one clock source, and each VFE
sets such a source to the rate it needs. A VFE started later at a lower
rate then lowers the source under a VFE that is still running.
For example, on SM8250 the two VFE Lite share one core clock source,
which the VFE sets to 400 or 480 MHz from the pixel rate. With VFE Lite
0 streaming the CSID test pattern at 480 MHz, starting an imx577 stream
on VFE Lite 1 sets the shared source to 400 MHz under the running VFE
Lite 0.
Group the VFE clocks with rate tables by their clock source at probe
time. A source shared by several VFEs follows the highest rate requested
by its running VFEs and drops to the highest remaining one when a VFE is
powered off; with no VFE left it keeps its rate, as it may feed
consumers outside CAMSS. A source used by a single VFE is dropped to its
lowest rate on power off. Clocks whose rate the VFE does not set are
left untouched.
In the example above the shared source stays at 480 MHz until VFE Lite
0 stops, and then drops to the 400 MHz still requested by VFE Lite 1.
Grouping is enabled through the vfe_clock_domains resource flag, so far
only on SM8250. The current parent of an RCG depends on its last rate,
so other SoCs keep setting their VFE clocks directly.
Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss-vfe.c | 11 +-
drivers/media/platform/qcom/camss/camss.c | 175 ++++++++++++++++++
drivers/media/platform/qcom/camss/camss.h | 13 ++
3 files changed, 195 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 359b5942df89..614497235eb4 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -1016,7 +1016,7 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
return -EINVAL;
}
- ret = clk_set_rate(clock->clk, rate);
+ ret = camss_set_clock_rate(vfe->camss, clock, rate);
if (ret < 0) {
dev_err(dev, "clk set rate failed: %d\n", ret);
return ret;
@@ -1083,12 +1083,12 @@ int vfe_get(struct vfe_device *vfe)
ret = vfe_set_clock_rates(vfe);
if (ret < 0)
- goto error_pm_runtime_get;
+ goto error_release_rates;
ret = camss_enable_clocks(vfe->nclocks, vfe->clock,
vfe->camss->dev);
if (ret < 0)
- goto error_pm_runtime_get;
+ goto error_release_rates;
ret = vfe_reset(vfe);
if (ret < 0)
@@ -1113,7 +1113,9 @@ int vfe_get(struct vfe_device *vfe)
error_reset:
camss_disable_clocks(vfe->nclocks, vfe->clock);
-error_pm_runtime_get:
+error_release_rates:
+ camss_release_clock_rates(vfe->camss, vfe->nclocks, vfe->clock);
+
pm_runtime_put_sync(vfe->camss->dev);
error_domain_off:
vfe->res->hw_ops->pm_domain_off(vfe);
@@ -1141,6 +1143,7 @@ void vfe_put(struct vfe_device *vfe)
vfe->res->hw_ops->vfe_halt(vfe);
}
camss_disable_clocks(vfe->nclocks, vfe->clock);
+ camss_release_clock_rates(vfe->camss, vfe->nclocks, vfe->clock);
pm_runtime_put_sync(vfe->camss->dev);
vfe->res->hw_ops->pm_domain_off(vfe);
}
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 7fe12049b78a..68b43a720869 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4509,6 +4509,171 @@ void camss_disable_clocks(int nclocks, struct camss_clock *clock)
clk_disable_unprepare(clock[i].clk);
}
+/*
+ * A clock domain is a clock source shared by clocks of different VFEs. Each
+ * VFE votes for the rate it needs, and the source runs at the highest vote,
+ * so that a VFE started at a lower rate never lowers the rate under a VFE
+ * that is still running.
+ */
+struct camss_clock_domain {
+ struct list_head node;
+ struct clk *parent;
+ struct list_head votes;
+ unsigned int nvotes;
+};
+
+/*
+ * A clock vote is the rate requested by one VFE clock fed by a clock domain.
+ * A rate of 0 means that the VFE is powered off and does not vote.
+ */
+struct camss_clock_vote {
+ struct list_head node;
+ struct camss_clock_domain *domain;
+ struct camss_clock *clock;
+ unsigned long rate;
+};
+
+static int camss_add_clock_vote(struct camss *camss, struct camss_clock *clock)
+{
+ struct camss_clock_domain *domain;
+ struct camss_clock_vote *vote;
+ struct clk *parent;
+
+ if (!clock->nfreqs)
+ return 0;
+
+ parent = clk_get_parent(clock->clk);
+ if (!parent)
+ return 0;
+
+ list_for_each_entry(domain, &camss->clock_domains, node)
+ if (clk_is_match(domain->parent, parent))
+ goto add_vote;
+
+ domain = devm_kzalloc(camss->dev, sizeof(*domain), GFP_KERNEL);
+ if (!domain)
+ return -ENOMEM;
+
+ domain->parent = parent;
+ INIT_LIST_HEAD(&domain->votes);
+ list_add_tail(&domain->node, &camss->clock_domains);
+
+add_vote:
+ vote = devm_kzalloc(camss->dev, sizeof(*vote), GFP_KERNEL);
+ if (!vote)
+ return -ENOMEM;
+
+ vote->domain = domain;
+ vote->clock = clock;
+ list_add_tail(&vote->node, &domain->votes);
+ domain->nvotes++;
+ clock->vote = vote;
+
+ return 0;
+}
+
+/*
+ * camss_init_clock_domains - Group the VFE clocks by clock source
+ * @camss: CAMSS device
+ *
+ * A clock whose source feeds no other VFE clock forms a dedicated domain
+ * with a single vote. Only SoCs whose VFE sets the rates of clocks with a
+ * fixed source group their clocks; the VFE clocks of other SoCs are set
+ * directly.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int camss_init_clock_domains(struct camss *camss)
+{
+ int ret, i, j;
+
+ if (!camss->res->vfe_clock_domains)
+ return 0;
+
+ for (i = 0; i < camss->res->vfe_num; i++)
+ for (j = 0; j < camss->vfe[i].nclocks; j++) {
+ ret = camss_add_clock_vote(camss, &camss->vfe[i].clock[j]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int camss_apply_clock_domain(struct camss_clock_domain *domain)
+{
+ struct camss_clock_vote *vote, *top = NULL;
+
+ list_for_each_entry(vote, &domain->votes, node)
+ if (vote->rate && (!top || vote->rate > top->rate))
+ top = vote;
+
+ if (!top)
+ return 0;
+
+ return clk_set_rate(top->clock->clk, top->rate);
+}
+
+/*
+ * camss_set_clock_rate - Set the rate of a clock
+ * @camss: CAMSS device
+ * @clock: Clock
+ * @rate: Requested rate
+ *
+ * A shared clock is set to the highest rate voted by the running VFEs
+ * of its source.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+int camss_set_clock_rate(struct camss *camss, struct camss_clock *clock,
+ unsigned long rate)
+{
+ int ret;
+
+ if (!clock->vote)
+ return clk_set_rate(clock->clk, rate);
+
+ mutex_lock(&camss->clock_lock);
+ clock->vote->rate = rate;
+ ret = camss_apply_clock_domain(clock->vote->domain);
+ mutex_unlock(&camss->clock_lock);
+
+ return ret;
+}
+
+/*
+ * camss_release_clock_rates - Release the rates requested by an engine
+ * @camss: CAMSS device
+ * @nclocks: Number of clocks of the engine
+ * @clock: Clocks of the engine
+ *
+ * A dedicated clock drops to its lowest rate. A shared clock drops to the
+ * highest rate of the VFEs still running, and keeps its rate when none is
+ * left, as its source may feed consumers outside CAMSS.
+ */
+void camss_release_clock_rates(struct camss *camss, int nclocks,
+ struct camss_clock *clock)
+{
+ int i;
+
+ mutex_lock(&camss->clock_lock);
+
+ for (i = 0; i < nclocks; i++) {
+ struct camss_clock_vote *vote = clock[i].vote;
+
+ if (!vote || !vote->rate)
+ continue;
+
+ vote->rate = 0;
+ if (vote->domain->nvotes == 1)
+ clk_set_rate(vote->clock->clk, 0);
+ else
+ camss_apply_clock_domain(vote->domain);
+ }
+
+ mutex_unlock(&camss->clock_lock);
+}
+
/*
* camss_find_sensor_pad - Find the media pad via which the sensor is linked
* @entity: Media entity to start searching from
@@ -5427,6 +5592,11 @@ static int camss_probe(struct platform_device *pdev)
atomic_set(&camss->ref_count, 0);
camss->dev = dev;
+ INIT_LIST_HEAD(&camss->clock_domains);
+
+ ret = devm_mutex_init(dev, &camss->clock_lock);
+ if (ret)
+ return ret;
platform_set_drvdata(pdev, camss);
camss->csiphy = devm_kcalloc(dev, camss->res->csiphy_num,
@@ -5474,6 +5644,10 @@ static int camss_probe(struct platform_device *pdev)
if (ret < 0)
goto err_genpd_cleanup;
+ ret = camss_init_clock_domains(camss);
+ if (ret < 0)
+ goto err_genpd_cleanup;
+
ret = dma_set_mask_and_coherent(dev, 0xffffffff);
if (ret)
goto err_genpd_cleanup;
@@ -5736,6 +5910,7 @@ static const struct camss_resources sm8250_resources = {
.csiphy_num = ARRAY_SIZE(csiphy_res_8250),
.csid_num = ARRAY_SIZE(csid_res_8250),
.vfe_num = ARRAY_SIZE(vfe_res_8250),
+ .vfe_clock_domains = true,
};
static const struct camss_resources sc8280xp_resources = {
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 7dc2fd9f03e0..03d415a5b9db 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -11,6 +11,8 @@
#define QC_MSM_CAMSS_H
#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
#include <linux/types.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
@@ -120,6 +122,7 @@ struct camss_resources {
const unsigned int tpg_num;
const unsigned int csid_num;
const unsigned int vfe_num;
+ const bool vfe_clock_domains;
};
struct camss {
@@ -141,6 +144,9 @@ struct camss {
struct icc_path *icc_path[ICC_SM8250_COUNT];
const struct camss_resources *res;
bool legacy_phy;
+ /* Protects clock_domains and the rates voted by the VFEs */
+ struct mutex clock_lock;
+ struct list_head clock_domains;
};
struct camss_camera_interface {
@@ -153,11 +159,14 @@ struct camss_async_subdev {
struct camss_camera_interface interface;
};
+struct camss_clock_vote;
+
struct camss_clock {
struct clk *clk;
const char *name;
u32 *freq;
u32 nfreqs;
+ struct camss_clock_vote *vote;
};
struct parent_dev_ops {
@@ -170,6 +179,10 @@ void camss_add_clock_margin(u64 *rate);
int camss_enable_clocks(int nclocks, struct camss_clock *clock,
struct device *dev);
void camss_disable_clocks(int nclocks, struct camss_clock *clock);
+int camss_set_clock_rate(struct camss *camss, struct camss_clock *clock,
+ unsigned long rate);
+void camss_release_clock_rates(struct camss *camss, int nclocks,
+ struct camss_clock *clock);
struct media_pad *camss_find_sensor_pad(struct media_entity *entity);
s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
unsigned int lanes);
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE
2026-10-24 9:40 [PATCH v2 0/6] media: qcom: camss: share VFE clock sources on SM8250 Atanas Filipov
` (3 preceding siblings ...)
2026-10-24 9:40 ` [PATCH v2 4/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
@ 2026-10-24 9:40 ` Atanas Filipov
2026-10-26 9:22 ` Bryan O'Donoghue
2026-10-24 9:40 ` [PATCH v2 6/6] media: qcom: camss: sm8250: set the camera NoC AXI " Atanas Filipov
5 siblings, 1 reply; 18+ messages in thread
From: Atanas Filipov @ 2026-10-24 9:40 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
The SM8250 VFE AREG clock lists four rates, 100, 200, 300 and 400 MHz,
in both the VFE and the CSID resources. The VFE only enables it, as
vfe_match_clock_names() does not match the AREG clock name, so its rate
is set by the CSID, which always selects the lowest one.
Match the VFE AREG clock in vfe_match_clock_names(), so that the VFE
selects its rate from the pixel rate of its lines like for its other
clocks, and drop its levels from the CSID resources. The AREG clocks of
VFE0 and VFE1 share the fast AHB source, which then runs at the highest
rate requested by the two VFEs.
Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss-vfe.c | 3 +++
drivers/media/platform/qcom/camss/camss.c | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 614497235eb4..948f0e12d542 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -907,12 +907,15 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
{
char vfe_name[7]; /* vfeXXX\0 */
char vfe_lite_name[12]; /* vfe_liteXXX\0 */
+ char vfe_areg_name[12]; /* vfeXXX_areg\0 */
snprintf(vfe_name, sizeof(vfe_name), "vfe%d", vfe->id);
snprintf(vfe_lite_name, sizeof(vfe_lite_name), "vfe_lite%d", vfe->id);
+ snprintf(vfe_areg_name, sizeof(vfe_areg_name), "vfe%d_areg", vfe->id);
return (!strcmp(clock->name, vfe_name) ||
!strcmp(clock->name, vfe_lite_name) ||
+ !strcmp(clock->name, vfe_areg_name) ||
!strcmp(clock->name, "vfe_lite") ||
!strcmp(clock->name, "camnoc_axi") ||
!strcmp(clock->name, "camnoc_rt_axi"));
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 68b43a720869..100612d97fa7 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -2053,7 +2053,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
.clock_rate = { { 400000000 },
{ 400000000 },
{ 0 },
- { 100000000, 200000000, 300000000, 400000000 },
+ { 0 },
{ 0 } },
.reg = { "csid0" },
.interrupt = { "csid0" },
@@ -2070,7 +2070,7 @@ static const struct camss_subdev_resources csid_res_8250[] = {
.clock_rate = { { 400000000 },
{ 400000000 },
{ 0 },
- { 100000000, 200000000, 300000000, 400000000 },
+ { 0 },
{ 0 } },
.reg = { "csid1" },
.interrupt = { "csid1" },
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 6/6] media: qcom: camss: sm8250: set the camera NoC AXI rate from the VFE
2026-10-24 9:40 [PATCH v2 0/6] media: qcom: camss: share VFE clock sources on SM8250 Atanas Filipov
` (4 preceding siblings ...)
2026-10-24 9:40 ` [PATCH v2 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE Atanas Filipov
@ 2026-10-24 9:40 ` Atanas Filipov
2026-10-26 9:25 ` Bryan O'Donoghue
5 siblings, 1 reply; 18+ messages in thread
From: Atanas Filipov @ 2026-10-24 9:40 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
The SM8250 VFE resources list both the camera NoC AXI RCG,
camnoc_axi_src, and its camnoc_axi branch. The camnoc_axi_src levels
are never applied, since vfe_match_clock_names() does not match that
name, and they list 480 MHz, which the RCG does not support. The
camnoc_axi branch has no rates, so the camera NoC runs from XO and two
concurrent streams stall.
Drop camnoc_axi_src, as the camnoc_axi branch keeps it enabled through
its parent, and list the 300 and 400 MHz camera NoC AXI rates for the
camnoc_axi branch, whose rate the VFE sets through the parent. The four
VFEs share its source through a clock domain, so it runs at the highest
rate requested by the running VFEs.
Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
---
drivers/media/platform/qcom/camss/camss.c | 28 ++++++++++-------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 100612d97fa7..9bfb634ee9f7 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -2120,13 +2120,12 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
/* VFE0 */
{
.regulators = {},
- .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ .clock = { "slow_ahb_src", "cpas_ahb",
"camnoc_axi", "vfe0_ahb", "vfe0_areg", "vfe0",
"vfe0_axi", "cam_hf_axi" },
- .clock_rate = { { 19200000, 300000000, 400000000, 480000000 },
- { 19200000, 80000000 },
+ .clock_rate = { { 19200000, 80000000 },
{ 19200000 },
- { 0 },
+ { 300000000, 400000000 },
{ 0 },
{ 100000000, 200000000, 300000000, 400000000 },
{ 350000000, 475000000, 576000000, 720000000 },
@@ -2146,13 +2145,12 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
/* VFE1 */
{
.regulators = {},
- .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ .clock = { "slow_ahb_src", "cpas_ahb",
"camnoc_axi", "vfe1_ahb", "vfe1_areg", "vfe1",
"vfe1_axi", "cam_hf_axi" },
- .clock_rate = { { 19200000, 300000000, 400000000, 480000000 },
- { 19200000, 80000000 },
+ .clock_rate = { { 19200000, 80000000 },
{ 19200000 },
- { 0 },
+ { 300000000, 400000000 },
{ 0 },
{ 100000000, 200000000, 300000000, 400000000 },
{ 350000000, 475000000, 576000000, 720000000 },
@@ -2172,13 +2170,12 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
/* VFE2 (lite) */
{
.regulators = {},
- .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ .clock = { "slow_ahb_src", "cpas_ahb",
"camnoc_axi", "vfe_lite_ahb", "vfe_lite_axi",
"vfe_lite", "cam_hf_axi" },
- .clock_rate = { { 19200000, 300000000, 400000000, 480000000 },
- { 19200000, 80000000 },
+ .clock_rate = { { 19200000, 80000000 },
{ 19200000 },
- { 0 },
+ { 300000000, 400000000 },
{ 0 },
{ 0 },
{ 400000000, 480000000 },
@@ -2196,13 +2193,12 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
/* VFE3 (lite) */
{
.regulators = {},
- .clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
+ .clock = { "slow_ahb_src", "cpas_ahb",
"camnoc_axi", "vfe_lite_ahb", "vfe_lite_axi",
"vfe_lite", "cam_hf_axi" },
- .clock_rate = { { 19200000, 300000000, 400000000, 480000000 },
- { 19200000, 80000000 },
+ .clock_rate = { { 19200000, 80000000 },
{ 19200000 },
- { 0 },
+ { 300000000, 400000000 },
{ 0 },
{ 0 },
{ 400000000, 480000000 },
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/6] media: qcom: camss: factor out the VFE minimum clock rate
2026-10-24 9:40 ` [PATCH v2 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
@ 2026-10-26 9:07 ` Bryan O'Donoghue
0 siblings, 0 replies; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-26 9:07 UTC (permalink / raw)
To: Atanas Filipov, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Vladimir Zapolskiy, Loic Poulain, Gjorgji Rosikopulos,
linux-media, linux-arm-msm, linux-kernel
On 24/10/2026 10:40, Atanas Filipov wrote:
> vfe_set_clock_rates() and vfe_check_clock_rates() both compute the
> minimum clock rate needed by the VFE lines from their pixel rates,
> inside the loop over the VFE clocks. The result is the same for every
> clock.
>
> Move the computation into vfe_get_min_clock_rate() and call it once
> per VFE, so that the clock loops only select and check rates. No
> functional change.
>
> Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level
2026-10-24 9:40 ` [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
@ 2026-10-26 9:08 ` Bryan O'Donoghue
2026-10-26 9:28 ` Loic Poulain
1 sibling, 0 replies; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-26 9:08 UTC (permalink / raw)
To: Atanas Filipov, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Vladimir Zapolskiy, Loic Poulain, Gjorgji Rosikopulos,
linux-media, linux-arm-msm, linux-kernel
On 24/10/2026 10:40, Atanas Filipov wrote:
> The VFE fails to power on when the pixel rate exceeds the highest rate
> listed for any of its clocks. The rate tables of the VFE clocks are
> independent and need not have the same number of levels, so a clock
> with a lower top rate than the VFE core clock, such as the SC7280
> camnoc_axi clock, makes the VFE reject pixel rates the core clock
> still supports.
>
> Clamp each clock to its highest listed rate instead, and warn that the
> pixel rate exceeds it when the clock lists more than one rate. Take the
> clamp into account when another line powers on an already running VFE,
> so that a clock running at its highest rate does not reject the
> request.
>
> Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID
2026-10-24 9:40 ` [PATCH v2 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID Atanas Filipov
@ 2026-10-26 9:08 ` Bryan O'Donoghue
0 siblings, 0 replies; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-26 9:08 UTC (permalink / raw)
To: Atanas Filipov, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Vladimir Zapolskiy, Loic Poulain, Gjorgji Rosikopulos,
linux-media, linux-arm-msm, linux-kernel
On 24/10/2026 10:40, Atanas Filipov wrote:
> The SM8250 CSID resources list clock levels for the VFE core clocks and
> for the shared VFE Lite core clock. CSID power on runs after the VFE is
> powered on and sets these clocks to their lowest level, overriding the
> rate selected by the VFE from the pixel rate and lowering the shared
> VFE Lite clock under a running VFE Lite.
>
> The VFE core clocks are set by the VFE. Drop their levels from the CSID
> resources so that CSID only enables them.
>
> Fixes: b4436a18eedb ("media: camss: add support for SM8250 camss")
> Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
> Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/6] media: qcom: camss: share VFE clock sources by the highest rate
2026-10-24 9:40 ` [PATCH v2 4/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
@ 2026-10-26 9:13 ` Bryan O'Donoghue
2026-10-26 13:18 ` Atanas Filipov
0 siblings, 1 reply; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-26 9:13 UTC (permalink / raw)
To: Atanas Filipov, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Vladimir Zapolskiy, Loic Poulain, Gjorgji Rosikopulos,
linux-media, linux-arm-msm, linux-kernel
On 24/10/2026 10:40, Atanas Filipov wrote:
> Clocks of different VFEs can be fed by one clock source, and each VFE
> sets such a source to the rate it needs. A VFE started later at a lower
> rate then lowers the source under a VFE that is still running.
>
> For example, on SM8250 the two VFE Lite share one core clock source,
> which the VFE sets to 400 or 480 MHz from the pixel rate. With VFE Lite
> 0 streaming the CSID test pattern at 480 MHz, starting an imx577 stream
> on VFE Lite 1 sets the shared source to 400 MHz under the running VFE
> Lite 0.
>
> Group the VFE clocks with rate tables by their clock source at probe
> time. A source shared by several VFEs follows the highest rate requested
> by its running VFEs and drops to the highest remaining one when a VFE is
> powered off; with no VFE left it keeps its rate, as it may feed
> consumers outside CAMSS. A source used by a single VFE is dropped to its
> lowest rate on power off. Clocks whose rate the VFE does not set are
> left untouched.
>
> In the example above the shared source stays at 480 MHz until VFE Lite
> 0 stops, and then drops to the 400 MHz still requested by VFE Lite 1.
>
> Grouping is enabled through the vfe_clock_domains resource flag, so far
> only on SM8250. The current parent of an RCG depends on its last rate,
> so other SoCs keep setting their VFE clocks directly.
>
> Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
I think this patch can be dropped.
clk_set_rate_range() is a CCF provided aggregator which will allow a VFE
or another consumer of the same clock to set an effective floor -
without having a driver-specific voting framework.
/* vfe_get() */
ret = clk_set_rate_range(clock->clk, rate, ULONG_MAX);
/* vfe_put() */
clk_set_rate_range(clock->clk, 0, ULONG_MAX);
This should subtract:
- camss_clock_domain
- camss_clock_vote
- camss_init_clock_domains()
- Associated boiler-plate code, mutexes etc
This holds for camnoc_axi and vfe_lite clocks.
On release, relax the range first and then clk_set_rate() to the lowest
listed rate explicitly.
Bundle that into patch #6, a driver-specific clock aggregation mechanism
isn't required.
---
bod
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE
2026-10-24 9:40 ` [PATCH v2 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE Atanas Filipov
@ 2026-10-26 9:22 ` Bryan O'Donoghue
2026-10-26 13:18 ` Atanas Filipov
0 siblings, 1 reply; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-26 9:22 UTC (permalink / raw)
To: Atanas Filipov, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Vladimir Zapolskiy, Loic Poulain, Gjorgji Rosikopulos,
linux-media, linux-arm-msm, linux-kernel
On 24/10/2026 10:40, Atanas Filipov wrote:
> The SM8250 VFE AREG clock lists four rates, 100, 200, 300 and 400 MHz,
> in both the VFE and the CSID resources. The VFE only enables it, as
> vfe_match_clock_names() does not match the AREG clock name, so its rate
> is set by the CSID, which always selects the lowest one.
>
> Match the VFE AREG clock in vfe_match_clock_names(), so that the VFE
> selects its rate from the pixel rate of its lines like for its other
> clocks, and drop its levels from the CSID resources. The AREG clocks of
> VFE0 and VFE1 share the fast AHB source, which then runs at the highest
> rate requested by the two VFEs.
>
> Signed-off-by: Atanas Filipov<atanas.filipov@oss.qualcomm.com>
> ---
Sets the lowest, isn't necessarily a bug.
Why is 100MHz a problem and if it is, why not just fix to 400MHz ?
areg is a register access / AHB clock - your worst case outcome is
'register access goes slower' - which to me accepting we don't do
driver-specific clock-aggregation => raise the clock required across VFE
and CSID so that nobody looses out on the bus times..
Again instead of having a driver-specific clock aggregation mechanism,
if we need to solve for two siblings voting for their parent's rate -
then lets do that in CCF so that every CCF consumer benefits.
That's obviously a separate and potentially large series so for this
specific case I'd suggest 400MHz iff a problem can be shown.
---
bod
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/6] media: qcom: camss: sm8250: set the camera NoC AXI rate from the VFE
2026-10-24 9:40 ` [PATCH v2 6/6] media: qcom: camss: sm8250: set the camera NoC AXI " Atanas Filipov
@ 2026-10-26 9:25 ` Bryan O'Donoghue
2026-10-26 13:18 ` Atanas Filipov
0 siblings, 1 reply; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-26 9:25 UTC (permalink / raw)
To: Atanas Filipov, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Vladimir Zapolskiy, Loic Poulain, Gjorgji Rosikopulos,
linux-media, linux-arm-msm, linux-kernel
On 24/10/2026 10:40, Atanas Filipov wrote:
> The SM8250 VFE resources list both the camera NoC AXI RCG,
> camnoc_axi_src, and its camnoc_axi branch. The camnoc_axi_src levels
> are never applied, since vfe_match_clock_names() does not match that
> name, and they list 480 MHz, which the RCG does not support. The
> camnoc_axi branch has no rates, so the camera NoC runs from XO and two
> concurrent streams stall.
>
> Drop camnoc_axi_src, as the camnoc_axi branch keeps it enabled through
> its parent, and list the 300 and 400 MHz camera NoC AXI rates for the
> camnoc_axi branch, whose rate the VFE sets through the parent. The four
> VFEs share its source through a clock domain, so it runs at the highest
> rate requested by the running VFEs.
>
> Signed-off-by: Atanas Filipov<atanas.filipov@oss.qualcomm.com>
I suggested bundling in clk_set_rate_range() to this patch but I believe
it would be more productively and correctly done as a wholesale
replacement of patch #4.
The point of your patch#4 was to fix the framework, which should be done
I agree as a standalone patch - not mushed in here.
---
bod
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level
2026-10-24 9:40 ` [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
2026-10-26 9:08 ` Bryan O'Donoghue
@ 2026-10-26 9:28 ` Loic Poulain
2026-10-26 13:18 ` Atanas Filipov
1 sibling, 1 reply; 18+ messages in thread
From: Loic Poulain @ 2026-10-26 9:28 UTC (permalink / raw)
To: Atanas Filipov
Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Vladimir Zapolskiy,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
On Sat, Oct 24, 2026 at 11:40 AM Atanas Filipov
<atanas.filipov@oss.qualcomm.com> wrote:
>
> The VFE fails to power on when the pixel rate exceeds the highest rate
> listed for any of its clocks. The rate tables of the VFE clocks are
> independent and need not have the same number of levels, so a clock
> with a lower top rate than the VFE core clock, such as the SC7280
> camnoc_axi clock, makes the VFE reject pixel rates the core clock
> still supports.
>
> Clamp each clock to its highest listed rate instead, and warn that the
> pixel rate exceeds it when the clock lists more than one rate. Take the
> clamp into account when another line powers on an already running VFE,
> so that a clock running at its highest rate does not reject the
> request.
>
> Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/camss/camss-vfe.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index 59be37f83df3..359b5942df89 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -996,15 +996,13 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
> if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
> long rate;
>
> - for (j = 0; j < clock->nfreqs; j++)
> + for (j = 0; j < clock->nfreqs - 1; j++)
> if (min_rate < clock->freq[j])
> break;
>
> - if (j == clock->nfreqs) {
> - dev_err(dev,
> - "Pixel clock is too high for VFE");
> - return -EINVAL;
> - }
> + if (clock->nfreqs > 1 && min_rate >= clock->freq[j])
> + dev_warn(dev, "Pixel clock is too high for VFE clock %s\n",
> + clock->name);
But are we still comparing the pixel rate against potentially
non-pixel clocks here, such as AXI?
>
> /* if sensor pixel clock is not available */
> /* set highest possible VFE clock rate */
> @@ -1046,9 +1044,15 @@ static int vfe_check_clock_rates(struct vfe_device *vfe)
>
> if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
> unsigned long rate;
> + long max_rate;
> +
> + /* A clock already at its highest rate cannot go higher */
> + max_rate = clk_round_rate(clock->clk, clock->freq[clock->nfreqs - 1]);
> + if (max_rate < 0)
> + return max_rate;
>
> rate = clk_get_rate(clock->clk);
> - if (rate < min_rate)
> + if (rate < min_t(u64, min_rate, max_rate))
> return -EBUSY;
> }
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level
2026-10-26 9:28 ` Loic Poulain
@ 2026-10-26 13:18 ` Atanas Filipov
0 siblings, 0 replies; 18+ messages in thread
From: Atanas Filipov @ 2026-10-26 13:18 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
On Mon, Oct 26, 2026 at 10:28 AM Loic Poulain wrote:
> But are we still comparing the pixel rate against potentially
> non-pixel clocks here, such as AXI?
Good point, thanks. Yes, AXI is selected from the same value, and this
patch does not change that. For RDI it is the line data rate (pixel
clock * bpp / 64), so a rough estimate of the write traffic rather
than a pixel rate.
A proper camera NoC bandwidth model would rather belong to the
interconnect work than to this series.
Regards,
Atanas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/6] media: qcom: camss: share VFE clock sources by the highest rate
2026-10-26 9:13 ` Bryan O'Donoghue
@ 2026-10-26 13:18 ` Atanas Filipov
0 siblings, 0 replies; 18+ messages in thread
From: Atanas Filipov @ 2026-10-26 13:18 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
On 26/10/2026 09:13, Bryan O'Donoghue wrote:
> clk_set_rate_range() is a CCF provided aggregator which will allow a VFE
> or another consumer of the same clock to set an effective floor -
> without having a driver-specific voting framework.
[...]
> On release, relax the range first and then clk_set_rate() to the lowest
> listed rate explicitly.
Agreed, v3 will replace the clock domains with clk_set_rate_range(). The
VFE will set its rate as a floor in vfe_get(); in vfe_put() it will drop
the floor and set the lowest listed rate rounded with clk_round_rate(),
so that the result stays within the floors of the other VFEs.
Tested on RB5 with two concurrent streams on VFE0/VFE1 and on the two
VFE Lite: the shared vfe_lite and camnoc_axi clocks stay at the highest
floor while both stream and drop to the remaining one when one stops.
The floors are aggregated per clock only. The AREG clocks of VFE0 and
VFE1 are two clocks fed by the fast AHB source, so the floor of one
does not protect the other, see 5/6.
Regards,
Atanas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE
2026-10-26 9:22 ` Bryan O'Donoghue
@ 2026-10-26 13:18 ` Atanas Filipov
0 siblings, 0 replies; 18+ messages in thread
From: Atanas Filipov @ 2026-10-26 13:18 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
On 26/10/2026 09:22, Bryan O'Donoghue wrote:
> Why is 100MHz a problem and if it is, why not just fix to 400MHz ?
100 MHz is not a problem by itself, and I see no failure from it.
With the rate ranges from 4/6, VFE0 at the CSID test pattern sets the
fast AHB source to 400 MHz; starting a sensor stream on VFE1 then sets
it to 100 MHz under the running VFE0, as the two AREG clocks are
separate clocks and their floors are not aggregated. All frames are
still captured.
v3 will list a single 400 MHz rate for the VFE AREG clocks, so neither
VFE lowers the source under the other.
Regards,
Atanas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/6] media: qcom: camss: sm8250: set the camera NoC AXI rate from the VFE
2026-10-26 9:25 ` Bryan O'Donoghue
@ 2026-10-26 13:18 ` Atanas Filipov
0 siblings, 0 replies; 18+ messages in thread
From: Atanas Filipov @ 2026-10-26 13:18 UTC (permalink / raw)
To: Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: atanas.filipov, Vladimir Zapolskiy, Loic Poulain,
Gjorgji Rosikopulos, linux-media, linux-arm-msm, linux-kernel
On 26/10/2026 09:25, Bryan O'Donoghue wrote:
> I suggested bundling in clk_set_rate_range() to this patch but I believe
> it would be more productively and correctly done as a wholesale
> replacement of patch #4.
Agreed, v3 will keep this patch to the camnoc_axi rate table and do the
rate ranges in 4/6.
Regards,
Atanas
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-10-26 13:18 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-24 9:40 [PATCH v2 0/6] media: qcom: camss: share VFE clock sources on SM8250 Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
2026-10-26 9:07 ` Bryan O'Donoghue
2026-10-24 9:40 ` [PATCH v2 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
2026-10-26 9:08 ` Bryan O'Donoghue
2026-10-26 9:28 ` Loic Poulain
2026-10-26 13:18 ` Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID Atanas Filipov
2026-10-26 9:08 ` Bryan O'Donoghue
2026-10-24 9:40 ` [PATCH v2 4/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
2026-10-26 9:13 ` Bryan O'Donoghue
2026-10-26 13:18 ` Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE Atanas Filipov
2026-10-26 9:22 ` Bryan O'Donoghue
2026-10-26 13:18 ` Atanas Filipov
2026-10-24 9:40 ` [PATCH v2 6/6] media: qcom: camss: sm8250: set the camera NoC AXI " Atanas Filipov
2026-10-26 9:25 ` Bryan O'Donoghue
2026-10-26 13:18 ` Atanas Filipov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®