mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate
@ 2026-10-28 13:23 Atanas Filipov
  2026-10-28 13:23 ` [PATCH v4 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Atanas Filipov @ 2026-10-28 13:23 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, and all four VFEs share the camera NoC AXI clock.
Each VFE sets such a clock 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, so VFE Lite 1 lowers it to 400 MHz under the running VFE
  Lite 0. With the series the clock stays at 480 MHz until VFE Lite 0
  stops.

This series lets the VFE own its clock rates:

- Patch 1 moves the minimum rate computation out of the VFE clock
  loops, with no functional change.
- Patch 2 keeps failing when the pixel rate exceeds the VFE core
  clock, and clamps the other VFE clocks to their highest listed rate,
  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 sets the VFE clock rates as minimums with
  clk_set_rate_range(), so that a shared clock runs at the highest
  minimum of its running VFEs. This applies to all SoCs.
- Patch 5 lets the VFE set the AREG clock at a fixed 400 MHz.
- 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 only covers the CAMSS VFEs
and fixes the concurrent stall today.

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, no shared clock is lowered under a running VFE, and the
camera NoC AXI clock runs at 300 or 400 MHz while streaming.

Also tested on the Lemans EVK (SA8775P) with imx577 and the CAMSS TPG,
on single and concurrent test pattern plus sensor RDI captures on all
seven IFEs: all streams complete and no shared clock is lowered under
a running IFE.

Changes in v4:
- Keep -EINVAL for the VFE core clock and clamp only the other VFE
  clocks, without a warning (Bryan O'Donoghue, Loic Poulain)
- Use clk_set_rate_range() on all SoCs and drop the vfe_clock_ranges
  flag (Loic Poulain, Bryan O'Donoghue)
- Add Suggested-by to patch 4 (Bryan O'Donoghue)
- Drop Reviewed-by from patch 2 due to changes

Changes in v3:
- Replace the CAMSS clock domains with clk_set_rate_range() and fold
  the rate ranges into patch 4 (Bryan O'Donoghue)
- Fix the VFE AREG clock at 400 MHz (Bryan O'Donoghue)
- Collect Reviewed-by tags

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 | 182 +++++++++++-------
 drivers/media/platform/qcom/camss/camss.c     |  44 ++---
 2 files changed, 136 insertions(+), 90 deletions(-)


base-commit: bd1ecbb61afefc6a8b876ccbc31e9228b44e9eab
-- 
2.34.1


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

* [PATCH v4 1/6] media: qcom: camss: factor out the VFE minimum clock rate
  2026-10-28 13:23 [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
@ 2026-10-28 13:23 ` Atanas Filipov
  2026-10-28 14:19   ` Loic Poulain
  2026-10-28 13:23 ` [PATCH v4 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Atanas Filipov @ 2026-10-28 13:23 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>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 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] 11+ messages in thread

* [PATCH v4 2/6] media: qcom: camss: clamp VFE clock rates to the highest level
  2026-10-28 13:23 [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
  2026-10-28 13:23 ` [PATCH v4 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
@ 2026-10-28 13:23 ` Atanas Filipov
  2026-10-28 14:31   ` Loic Poulain
  2026-10-28 13:24 ` [PATCH v4 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID Atanas Filipov
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Atanas Filipov @ 2026-10-28 13:23 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.

Keep failing when the pixel rate exceeds the highest rate of the VFE
core clock, and clamp the other VFE clocks to their highest listed
rate instead. Take the clamp into account for the other clocks 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>
---
Changes in v4:
- Keep -EINVAL for the VFE core clock, clamp only the other clocks,
  drop the warning (Bryan O'Donoghue, Loic Poulain)
- Dropped Reviewed-by due to changes

 drivers/media/platform/qcom/camss/camss-vfe.c | 40 +++++++++++++++++--
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 59be37f83df3..bcc2c1929af1 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -918,6 +918,20 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
 		!strcmp(clock->name, "camnoc_rt_axi"));
 }
 
+static bool vfe_match_core_clock_name(struct vfe_device *vfe,
+				      struct camss_clock *clock)
+{
+	char vfe_name[7]; /* vfeXXX\0 */
+	char vfe_lite_name[12]; /* vfe_liteXXX\0 */
+
+	snprintf(vfe_name, sizeof(vfe_name), "vfe%d", vfe->id);
+	snprintf(vfe_lite_name, sizeof(vfe_lite_name), "vfe_lite%d", vfe->id);
+
+	return (!strcmp(clock->name, vfe_name) ||
+		!strcmp(clock->name, vfe_lite_name) ||
+		!strcmp(clock->name, "vfe_lite"));
+}
+
 /*
  * vfe_check_clock_levels - Calculate and set clock rates on VFE module
  * @clock: clocks data
@@ -1001,9 +1015,14 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
 					break;
 
 			if (j == clock->nfreqs) {
-				dev_err(dev,
-					"Pixel clock is too high for VFE");
-				return -EINVAL;
+				if (vfe_match_core_clock_name(vfe, clock)) {
+					dev_err(dev,
+						"Pixel clock is too high for VFE");
+					return -EINVAL;
+				}
+
+				/* Other clocks are clamped to their highest rate */
+				j = clock->nfreqs - 1;
 			}
 
 			/* if sensor pixel clock is not available */
@@ -1045,10 +1064,23 @@ static int vfe_check_clock_rates(struct vfe_device *vfe)
 		struct camss_clock *clock = &vfe->clock[i];
 
 		if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
+			u64 req_rate = min_rate;
 			unsigned long rate;
 
+			if (!vfe_match_core_clock_name(vfe, clock)) {
+				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;
+
+				req_rate = min_t(u64, min_rate, max_rate);
+			}
+
 			rate = clk_get_rate(clock->clk);
-			if (rate < min_rate)
+			if (rate < req_rate)
 				return -EBUSY;
 		}
 	}
-- 
2.34.1


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

* [PATCH v4 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID
  2026-10-28 13:23 [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
  2026-10-28 13:23 ` [PATCH v4 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
  2026-10-28 13:23 ` [PATCH v4 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
@ 2026-10-28 13:24 ` Atanas Filipov
  2026-10-28 13:24 ` [PATCH v4 4/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Atanas Filipov @ 2026-10-28 13:24 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>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 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] 11+ messages in thread

* [PATCH v4 4/6] media: qcom: camss: share VFE clock sources by the highest rate
  2026-10-28 13:23 [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
                   ` (2 preceding siblings ...)
  2026-10-28 13:24 ` [PATCH v4 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID Atanas Filipov
@ 2026-10-28 13:24 ` Atanas Filipov
  2026-10-28 14:42   ` Loic Poulain
  2026-10-28 13:24 ` [PATCH v4 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE Atanas Filipov
  2026-10-28 13:24 ` [PATCH v4 6/6] media: qcom: camss: sm8250: set the camera NoC AXI " Atanas Filipov
  5 siblings, 1 reply; 11+ messages in thread
From: Atanas Filipov @ 2026-10-28 13:24 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.

Request the VFE clock rates as minimums with clk_set_rate_range(), so
that a shared clock runs at the highest minimum of its running VFEs.
On power off the VFE drops its minimum and sets the clock to its lowest
listed rate within the remaining minimums.

Suggested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
---
Changes in v4:
- Use clk_set_rate_range() on all SoCs, drop the vfe_clock_ranges flag
  (Loic Poulain, Bryan O'Donoghue)
- Add Suggested-by (Bryan O'Donoghue)

 drivers/media/platform/qcom/camss/camss-vfe.c | 39 +++++++++++++++++--
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index bcc2c1929af1..46674bc30e6a 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -1037,7 +1037,7 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
 				return -EINVAL;
 			}
 
-			ret = clk_set_rate(clock->clk, rate);
+			ret = clk_set_rate_range(clock->clk, clock->freq[j], ULONG_MAX);
 			if (ret < 0) {
 				dev_err(dev, "clk set rate failed: %d\n", ret);
 				return ret;
@@ -1088,6 +1088,34 @@ static int vfe_check_clock_rates(struct vfe_device *vfe)
 	return 0;
 }
 
+/*
+ * vfe_release_clock_rates - Release the minimum clock rates of VFE module
+ * @vfe: VFE device
+ *
+ * Drop the minimum rate requested by the VFE from each of its clocks and set
+ * the clock to its lowest level, or to the highest minimum rate still
+ * requested by the other users of the clock.
+ */
+static void vfe_release_clock_rates(struct vfe_device *vfe)
+{
+	int i;
+
+	for (i = 0; i < vfe->nclocks; i++) {
+		struct camss_clock *clock = &vfe->clock[i];
+		long rate;
+
+		if (!vfe_match_clock_names(vfe, clock) || !vfe_check_clock_levels(clock))
+			continue;
+
+		clk_drop_range(clock->clk);
+
+		/* Round first, so that the rate respects the other users' ranges */
+		rate = clk_round_rate(clock->clk, clock->freq[0]);
+		if (rate > 0)
+			clk_set_rate(clock->clk, rate);
+	}
+}
+
 /*
  * vfe_get - Power up and reset VFE module
  * @vfe: VFE Device
@@ -1111,12 +1139,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)
@@ -1141,7 +1169,9 @@ int vfe_get(struct vfe_device *vfe)
 error_reset:
 	camss_disable_clocks(vfe->nclocks, vfe->clock);
 
-error_pm_runtime_get:
+error_release_rates:
+	vfe_release_clock_rates(vfe);
+
 	pm_runtime_put_sync(vfe->camss->dev);
 error_domain_off:
 	vfe->res->hw_ops->pm_domain_off(vfe);
@@ -1169,6 +1199,7 @@ void vfe_put(struct vfe_device *vfe)
 			vfe->res->hw_ops->vfe_halt(vfe);
 		}
 		camss_disable_clocks(vfe->nclocks, vfe->clock);
+		vfe_release_clock_rates(vfe);
 		pm_runtime_put_sync(vfe->camss->dev);
 		vfe->res->hw_ops->pm_domain_off(vfe);
 	}
-- 
2.34.1


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

* [PATCH v4 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE
  2026-10-28 13:23 [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
                   ` (3 preceding siblings ...)
  2026-10-28 13:24 ` [PATCH v4 4/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
@ 2026-10-28 13:24 ` Atanas Filipov
  2026-10-28 13:24 ` [PATCH v4 6/6] media: qcom: camss: sm8250: set the camera NoC AXI " Atanas Filipov
  5 siblings, 0 replies; 11+ messages in thread
From: Atanas Filipov @ 2026-10-28 13:24 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(), list a single
400 MHz rate for it and drop its levels from the CSID resources. The
AREG clocks of VFE0 and VFE1 are separate clocks on the fast AHB
source, so a fixed rate keeps one VFE from lowering it under the other.

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     | 8 ++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 46674bc30e6a..6cc5d4e2aaf2 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 7fe12049b78a..9e29b713288f 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" },
@@ -2128,7 +2128,7 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
 				{ 19200000 },
 				{ 0 },
 				{ 0 },
-				{ 100000000, 200000000, 300000000, 400000000 },
+				{ 400000000 },
 				{ 350000000, 475000000, 576000000, 720000000 },
 				{ 0 },
 				{ 0 } },
@@ -2154,7 +2154,7 @@ static const struct camss_subdev_resources vfe_res_8250[] = {
 				{ 19200000 },
 				{ 0 },
 				{ 0 },
-				{ 100000000, 200000000, 300000000, 400000000 },
+				{ 400000000 },
 				{ 350000000, 475000000, 576000000, 720000000 },
 				{ 0 },
 				{ 0 } },
-- 
2.34.1


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

* [PATCH v4 6/6] media: qcom: camss: sm8250: set the camera NoC AXI rate from the VFE
  2026-10-28 13:23 [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
                   ` (4 preceding siblings ...)
  2026-10-28 13:24 ` [PATCH v4 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE Atanas Filipov
@ 2026-10-28 13:24 ` Atanas Filipov
  2026-10-28 14:37   ` Loic Poulain
  5 siblings, 1 reply; 11+ messages in thread
From: Atanas Filipov @ 2026-10-28 13:24 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 the camnoc_axi clock, 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 9e29b713288f..6c3e00e043eb 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 },
 				{ 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 },
 				{ 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] 11+ messages in thread

* Re: [PATCH v4 1/6] media: qcom: camss: factor out the VFE minimum clock rate
  2026-10-28 13:23 ` [PATCH v4 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
@ 2026-10-28 14:19   ` Loic Poulain
  0 siblings, 0 replies; 11+ messages in thread
From: Loic Poulain @ 2026-10-28 14:19 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 Wed, Oct 28, 2026 at 2:24 PM Atanas Filipov
<atanas.filipov@oss.qualcomm.com> 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>

Reviewed-by: Loic Poulain <loic.poulain@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] 11+ messages in thread

* Re: [PATCH v4 2/6] media: qcom: camss: clamp VFE clock rates to the highest level
  2026-10-28 13:23 ` [PATCH v4 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
@ 2026-10-28 14:31   ` Loic Poulain
  0 siblings, 0 replies; 11+ messages in thread
From: Loic Poulain @ 2026-10-28 14:31 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 Wed, Oct 28, 2026 at 2:24 PM 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.
>
> Keep failing when the pixel rate exceeds the highest rate of the VFE
> core clock, and clamp the other VFE clocks to their highest listed
> rate instead. Take the clamp into account for the other clocks 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>

Just a minor style nit, but the dev_err() messages could fit on a single line.
Anyway:

Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

> ---
> Changes in v4:
> - Keep -EINVAL for the VFE core clock, clamp only the other clocks,
>   drop the warning (Bryan O'Donoghue, Loic Poulain)
> - Dropped Reviewed-by due to changes
>
>  drivers/media/platform/qcom/camss/camss-vfe.c | 40 +++++++++++++++++--
>  1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index 59be37f83df3..bcc2c1929af1 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -918,6 +918,20 @@ static int vfe_match_clock_names(struct vfe_device *vfe,
>                 !strcmp(clock->name, "camnoc_rt_axi"));
>  }
>
> +static bool vfe_match_core_clock_name(struct vfe_device *vfe,
> +                                     struct camss_clock *clock)
> +{
> +       char vfe_name[7]; /* vfeXXX\0 */
> +       char vfe_lite_name[12]; /* vfe_liteXXX\0 */
> +
> +       snprintf(vfe_name, sizeof(vfe_name), "vfe%d", vfe->id);
> +       snprintf(vfe_lite_name, sizeof(vfe_lite_name), "vfe_lite%d", vfe->id);
> +
> +       return (!strcmp(clock->name, vfe_name) ||
> +               !strcmp(clock->name, vfe_lite_name) ||
> +               !strcmp(clock->name, "vfe_lite"));
> +}
> +
>  /*
>   * vfe_check_clock_levels - Calculate and set clock rates on VFE module
>   * @clock: clocks data
> @@ -1001,9 +1015,14 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
>                                         break;
>
>                         if (j == clock->nfreqs) {
> -                               dev_err(dev,
> -                                       "Pixel clock is too high for VFE");
> -                               return -EINVAL;
> +                               if (vfe_match_core_clock_name(vfe, clock)) {
> +                                       dev_err(dev,
> +                                               "Pixel clock is too high for VFE");
> +                                       return -EINVAL;
> +                               }
> +
> +                               /* Other clocks are clamped to their highest rate */
> +                               j = clock->nfreqs - 1;
>                         }
>
>                         /* if sensor pixel clock is not available */
> @@ -1045,10 +1064,23 @@ static int vfe_check_clock_rates(struct vfe_device *vfe)
>                 struct camss_clock *clock = &vfe->clock[i];
>
>                 if (vfe_match_clock_names(vfe, clock) && vfe_check_clock_levels(clock)) {
> +                       u64 req_rate = min_rate;
>                         unsigned long rate;
>
> +                       if (!vfe_match_core_clock_name(vfe, clock)) {
> +                               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;
> +
> +                               req_rate = min_t(u64, min_rate, max_rate);
> +                       }
> +
>                         rate = clk_get_rate(clock->clk);
> -                       if (rate < min_rate)
> +                       if (rate < req_rate)
>                                 return -EBUSY;
>                 }
>         }
> --
> 2.34.1
>

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

* Re: [PATCH v4 6/6] media: qcom: camss: sm8250: set the camera NoC AXI rate from the VFE
  2026-10-28 13:24 ` [PATCH v4 6/6] media: qcom: camss: sm8250: set the camera NoC AXI " Atanas Filipov
@ 2026-10-28 14:37   ` Loic Poulain
  0 siblings, 0 replies; 11+ messages in thread
From: Loic Poulain @ 2026-10-28 14:37 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 Wed, Oct 28, 2026 at 2:24 PM Atanas Filipov
<atanas.filipov@oss.qualcomm.com> 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 the camnoc_axi clock, so it runs at the highest rate
> requested by the running VFEs.
>
> Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>

So, as previously discussed, we'll probably need a smarter AXI clock
scaling/aggregation scheme at some point, but for now this looks fine.

Reviewed-by: Loic Poulain <loic.poulain@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 9e29b713288f..6c3e00e043eb 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 },
>                                 { 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 },
>                                 { 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] 11+ messages in thread

* Re: [PATCH v4 4/6] media: qcom: camss: share VFE clock sources by the highest rate
  2026-10-28 13:24 ` [PATCH v4 4/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
@ 2026-10-28 14:42   ` Loic Poulain
  0 siblings, 0 replies; 11+ messages in thread
From: Loic Poulain @ 2026-10-28 14:42 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 Wed, Oct 28, 2026 at 2:24 PM Atanas Filipov
<atanas.filipov@oss.qualcomm.com> 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.
>
> Request the VFE clock rates as minimums with clk_set_rate_range(), so
> that a shared clock runs at the highest minimum of its running VFEs.
> On power off the VFE drops its minimum and sets the clock to its lowest
> listed rate within the remaining minimums.
>
> Suggested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>

Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

> ---
> Changes in v4:
> - Use clk_set_rate_range() on all SoCs, drop the vfe_clock_ranges flag
>   (Loic Poulain, Bryan O'Donoghue)
> - Add Suggested-by (Bryan O'Donoghue)
>
>  drivers/media/platform/qcom/camss/camss-vfe.c | 39 +++++++++++++++++--
>  1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index bcc2c1929af1..46674bc30e6a 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -1037,7 +1037,7 @@ static int vfe_set_clock_rates(struct vfe_device *vfe)
>                                 return -EINVAL;
>                         }
>
> -                       ret = clk_set_rate(clock->clk, rate);
> +                       ret = clk_set_rate_range(clock->clk, clock->freq[j], ULONG_MAX);
>                         if (ret < 0) {
>                                 dev_err(dev, "clk set rate failed: %d\n", ret);
>                                 return ret;
> @@ -1088,6 +1088,34 @@ static int vfe_check_clock_rates(struct vfe_device *vfe)
>         return 0;
>  }
>
> +/*
> + * vfe_release_clock_rates - Release the minimum clock rates of VFE module
> + * @vfe: VFE device
> + *
> + * Drop the minimum rate requested by the VFE from each of its clocks and set
> + * the clock to its lowest level, or to the highest minimum rate still
> + * requested by the other users of the clock.
> + */
> +static void vfe_release_clock_rates(struct vfe_device *vfe)
> +{
> +       int i;
> +
> +       for (i = 0; i < vfe->nclocks; i++) {
> +               struct camss_clock *clock = &vfe->clock[i];
> +               long rate;
> +
> +               if (!vfe_match_clock_names(vfe, clock) || !vfe_check_clock_levels(clock))
> +                       continue;
> +
> +               clk_drop_range(clock->clk);
> +
> +               /* Round first, so that the rate respects the other users' ranges */
> +               rate = clk_round_rate(clock->clk, clock->freq[0]);
> +               if (rate > 0)
> +                       clk_set_rate(clock->clk, rate);
> +       }
> +}
> +
>  /*
>   * vfe_get - Power up and reset VFE module
>   * @vfe: VFE Device
> @@ -1111,12 +1139,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)
> @@ -1141,7 +1169,9 @@ int vfe_get(struct vfe_device *vfe)
>  error_reset:
>         camss_disable_clocks(vfe->nclocks, vfe->clock);
>
> -error_pm_runtime_get:
> +error_release_rates:
> +       vfe_release_clock_rates(vfe);
> +
>         pm_runtime_put_sync(vfe->camss->dev);
>  error_domain_off:
>         vfe->res->hw_ops->pm_domain_off(vfe);
> @@ -1169,6 +1199,7 @@ void vfe_put(struct vfe_device *vfe)
>                         vfe->res->hw_ops->vfe_halt(vfe);
>                 }
>                 camss_disable_clocks(vfe->nclocks, vfe->clock);
> +               vfe_release_clock_rates(vfe);
>                 pm_runtime_put_sync(vfe->camss->dev);
>                 vfe->res->hw_ops->pm_domain_off(vfe);
>         }
> --
> 2.34.1
>

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

end of thread, other threads:[~2026-10-28 14:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28 13:23 [PATCH v4 0/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
2026-10-28 13:23 ` [PATCH v4 1/6] media: qcom: camss: factor out the VFE minimum clock rate Atanas Filipov
2026-10-28 14:19   ` Loic Poulain
2026-10-28 13:23 ` [PATCH v4 2/6] media: qcom: camss: clamp VFE clock rates to the highest level Atanas Filipov
2026-10-28 14:31   ` Loic Poulain
2026-10-28 13:24 ` [PATCH v4 3/6] media: qcom: camss: sm8250: do not set VFE core clock rates from CSID Atanas Filipov
2026-10-28 13:24 ` [PATCH v4 4/6] media: qcom: camss: share VFE clock sources by the highest rate Atanas Filipov
2026-10-28 14:42   ` Loic Poulain
2026-10-28 13:24 ` [PATCH v4 5/6] media: qcom: camss: sm8250: set the VFE AREG clock rate from the VFE Atanas Filipov
2026-10-28 13:24 ` [PATCH v4 6/6] media: qcom: camss: sm8250: set the camera NoC AXI " Atanas Filipov
2026-10-28 14:37   ` Loic Poulain

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®