dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/8] MDSS reg bus interconnect
@ 2023-07-12 12:11 Dmitry Baryshkov
  2023-07-12 12:11 ` [PATCH v2 1/8] dt-bindings: display/msm: Add reg bus and rotator interconnects Dmitry Baryshkov
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

Per agreement with Konrad, picked up this patch series.

Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
another path that needs to be handled to ensure MDSS functions properly,
namely the "reg bus", a.k.a the CPU-MDSS interconnect.

Gating that path may have a variety of effects. from none to otherwise
inexplicable DSI timeouts.

This series tries to address the lack of that.

Changes since v1:
- Dropped the DPU part, the MDSS vote seems to be enough
- Reworked MDSS voting patch. Replaced static bw value with the
  per-platform confgurable values.
- Added sm8450 DT patch.

Dmitry Baryshkov (6):
  drm/msm/mdss: correct UBWC programming for SM8550
  drm/msm/mdss: switch mdss to use devm_of_icc_get()
  drm/msm/mdss: inline msm_mdss_icc_request_bw()
  drm/msm/mdss: populate missing data
  drm/msm/mdss: Handle the reg bus ICC path
  arm64: dts: qcom: sm8450: provide MDSS cfg interconnect

Konrad Dybcio (2):
  dt-bindings: display/msm: Add reg bus and rotator interconnects
  drm/msm/mdss: Rename path references to mdp_path

 .../bindings/display/msm/mdss-common.yaml     |   2 +
 arch/arm64/boot/dts/qcom/sm8450.dtsi          |   9 +-
 drivers/gpu/drm/msm/msm_mdss.c                | 138 +++++++++++++-----
 3 files changed, 108 insertions(+), 41 deletions(-)

-- 
2.40.1


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

* [PATCH v2 1/8] dt-bindings: display/msm: Add reg bus and rotator interconnects
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
@ 2023-07-12 12:11 ` Dmitry Baryshkov
  2023-07-12 19:24   ` Krzysztof Kozlowski
  2023-07-12 12:11 ` [PATCH v2 2/8] drm/msm/mdss: correct UBWC programming for SM8550 Dmitry Baryshkov
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

From: Konrad Dybcio <konrad.dybcio@linaro.org>

Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there are
other connection paths:
- a path that connects rotator block to the DDR.
- a path that needs to be handled to ensure MDSS register access
  functions properly, namely the "reg bus", a.k.a the CPU-MDSS CFG
  interconnect.

Describe these paths bindings to allow using them in device trees and in
the driver

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 Documentation/devicetree/bindings/display/msm/mdss-common.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
index ccd7d6417523..30a8aed4289a 100644
--- a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
+++ b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
@@ -66,12 +66,14 @@ properties:
     items:
       - description: Interconnect path from mdp0 (or a single mdp) port to the data bus
       - description: Interconnect path from mdp1 port to the data bus
+      - description: Interconnect path from CPU to the reg bus
 
   interconnect-names:
     minItems: 1
     items:
       - const: mdp0-mem
       - const: mdp1-mem
+      - const: cpu-cfg
 
   resets:
     items:
-- 
2.40.1


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

* [PATCH v2 2/8] drm/msm/mdss: correct UBWC programming for SM8550
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
  2023-07-12 12:11 ` [PATCH v2 1/8] dt-bindings: display/msm: Add reg bus and rotator interconnects Dmitry Baryshkov
@ 2023-07-12 12:11 ` Dmitry Baryshkov
  2023-07-12 22:02   ` Abhinav Kumar
  2023-07-12 12:11 ` [PATCH v2 3/8] drm/msm/mdss: switch mdss to use devm_of_icc_get() Dmitry Baryshkov
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

The SM8550 platform employs newer UBWC decoder, which requires slightly
different programming.

Fixes: a2f33995c19d ("drm/msm: mdss: add support for SM8550")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 05648c910c68..798bd4f3b662 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -189,6 +189,7 @@ static int _msm_mdss_irq_domain_add(struct msm_mdss *msm_mdss)
 #define UBWC_2_0 0x20000000
 #define UBWC_3_0 0x30000000
 #define UBWC_4_0 0x40000000
+#define UBWC_4_3 0x40030000
 
 static void msm_mdss_setup_ubwc_dec_20(struct msm_mdss *msm_mdss)
 {
@@ -227,7 +228,10 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss)
 		writel_relaxed(1, msm_mdss->mmio + UBWC_CTRL_2);
 		writel_relaxed(0, msm_mdss->mmio + UBWC_PREDICTION_MODE);
 	} else {
-		writel_relaxed(2, msm_mdss->mmio + UBWC_CTRL_2);
+		if (data->ubwc_dec_version == UBWC_4_3)
+			writel_relaxed(3, msm_mdss->mmio + UBWC_CTRL_2);
+		else
+			writel_relaxed(2, msm_mdss->mmio + UBWC_CTRL_2);
 		writel_relaxed(1, msm_mdss->mmio + UBWC_PREDICTION_MODE);
 	}
 }
@@ -271,6 +275,7 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 		msm_mdss_setup_ubwc_dec_30(msm_mdss);
 		break;
 	case UBWC_4_0:
+	case UBWC_4_3:
 		msm_mdss_setup_ubwc_dec_40(msm_mdss);
 		break;
 	default:
@@ -569,6 +574,16 @@ static const struct msm_mdss_data sm8250_data = {
 	.macrotile_mode = 1,
 };
 
+static const struct msm_mdss_data sm8550_data = {
+	.ubwc_version = UBWC_4_0,
+	.ubwc_dec_version = UBWC_4_3,
+	.ubwc_swizzle = 6,
+	.ubwc_static = 1,
+	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
+	.highest_bank_bit = 3,
+	.macrotile_mode = 1,
+};
+
 static const struct of_device_id mdss_dt_match[] = {
 	{ .compatible = "qcom,mdss" },
 	{ .compatible = "qcom,msm8998-mdss" },
@@ -585,7 +600,7 @@ static const struct of_device_id mdss_dt_match[] = {
 	{ .compatible = "qcom,sm8250-mdss", .data = &sm8250_data },
 	{ .compatible = "qcom,sm8350-mdss", .data = &sm8250_data },
 	{ .compatible = "qcom,sm8450-mdss", .data = &sm8250_data },
-	{ .compatible = "qcom,sm8550-mdss", .data = &sm8250_data },
+	{ .compatible = "qcom,sm8550-mdss", .data = &sm8550_data },
 	{}
 };
 MODULE_DEVICE_TABLE(of, mdss_dt_match);
-- 
2.40.1


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

* [PATCH v2 3/8] drm/msm/mdss: switch mdss to use devm_of_icc_get()
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
  2023-07-12 12:11 ` [PATCH v2 1/8] dt-bindings: display/msm: Add reg bus and rotator interconnects Dmitry Baryshkov
  2023-07-12 12:11 ` [PATCH v2 2/8] drm/msm/mdss: correct UBWC programming for SM8550 Dmitry Baryshkov
@ 2023-07-12 12:11 ` Dmitry Baryshkov
  2023-07-12 12:11 ` [PATCH v2 4/8] drm/msm/mdss: Rename path references to mdp_path Dmitry Baryshkov
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

Stop using hand-written reset function for ICC release, use
devm_of_icc_get() instead.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 798bd4f3b662..304a6509e1fb 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -58,14 +58,14 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
 	struct icc_path *path0;
 	struct icc_path *path1;
 
-	path0 = of_icc_get(dev, "mdp0-mem");
+	path0 = devm_of_icc_get(dev, "mdp0-mem");
 	if (IS_ERR_OR_NULL(path0))
 		return PTR_ERR_OR_ZERO(path0);
 
 	msm_mdss->path[0] = path0;
 	msm_mdss->num_paths = 1;
 
-	path1 = of_icc_get(dev, "mdp1-mem");
+	path1 = devm_of_icc_get(dev, "mdp1-mem");
 	if (!IS_ERR_OR_NULL(path1)) {
 		msm_mdss->path[1] = path1;
 		msm_mdss->num_paths++;
@@ -74,15 +74,6 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
 	return 0;
 }
 
-static void msm_mdss_put_icc_path(void *data)
-{
-	struct msm_mdss *msm_mdss = data;
-	int i;
-
-	for (i = 0; i < msm_mdss->num_paths; i++)
-		icc_put(msm_mdss->path[i]);
-}
-
 static void msm_mdss_icc_request_bw(struct msm_mdss *msm_mdss, unsigned long bw)
 {
 	int i;
@@ -389,9 +380,6 @@ static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5
 	dev_dbg(&pdev->dev, "mapped mdss address space @%pK\n", msm_mdss->mmio);
 
 	ret = msm_mdss_parse_data_bus_icc_path(&pdev->dev, msm_mdss);
-	if (ret)
-		return ERR_PTR(ret);
-	ret = devm_add_action_or_reset(&pdev->dev, msm_mdss_put_icc_path, msm_mdss);
 	if (ret)
 		return ERR_PTR(ret);
 
-- 
2.40.1


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

* [PATCH v2 4/8] drm/msm/mdss: Rename path references to mdp_path
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
                   ` (2 preceding siblings ...)
  2023-07-12 12:11 ` [PATCH v2 3/8] drm/msm/mdss: switch mdss to use devm_of_icc_get() Dmitry Baryshkov
@ 2023-07-12 12:11 ` Dmitry Baryshkov
  2023-07-12 12:11 ` [PATCH v2 5/8] drm/msm/mdss: inline msm_mdss_icc_request_bw() Dmitry Baryshkov
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

From: Konrad Dybcio <konrad.dybcio@linaro.org>

The DPU1 driver needs to handle all MDPn<->DDR paths, as well as
CPU<->SLAVE_DISPLAY_CFG. The former ones share how their values are
calculated, but the latter one has static predefines spanning all SoCs.

In preparation for supporting the CPU<->SLAVE_DISPLAY_CFG path, rename
the path-related struct members to include "mdp_".

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 304a6509e1fb..809c93b22c9c 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -48,8 +48,8 @@ struct msm_mdss {
 		struct irq_domain *domain;
 	} irq_controller;
 	const struct msm_mdss_data *mdss_data;
-	struct icc_path *path[2];
-	u32 num_paths;
+	struct icc_path *mdp_path[2];
+	u32 num_mdp_paths;
 };
 
 static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
@@ -62,13 +62,13 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
 	if (IS_ERR_OR_NULL(path0))
 		return PTR_ERR_OR_ZERO(path0);
 
-	msm_mdss->path[0] = path0;
-	msm_mdss->num_paths = 1;
+	msm_mdss->mdp_path[0] = path0;
+	msm_mdss->num_mdp_paths = 1;
 
 	path1 = devm_of_icc_get(dev, "mdp1-mem");
 	if (!IS_ERR_OR_NULL(path1)) {
-		msm_mdss->path[1] = path1;
-		msm_mdss->num_paths++;
+		msm_mdss->mdp_path[1] = path1;
+		msm_mdss->num_mdp_paths++;
 	}
 
 	return 0;
@@ -78,8 +78,8 @@ static void msm_mdss_icc_request_bw(struct msm_mdss *msm_mdss, unsigned long bw)
 {
 	int i;
 
-	for (i = 0; i < msm_mdss->num_paths; i++)
-		icc_set_bw(msm_mdss->path[i], 0, Bps_to_icc(bw));
+	for (i = 0; i < msm_mdss->num_mdp_paths; i++)
+		icc_set_bw(msm_mdss->mdp_path[i], 0, Bps_to_icc(bw));
 }
 
 static void msm_mdss_irq(struct irq_desc *desc)
-- 
2.40.1


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

* [PATCH v2 5/8] drm/msm/mdss: inline msm_mdss_icc_request_bw()
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
                   ` (3 preceding siblings ...)
  2023-07-12 12:11 ` [PATCH v2 4/8] drm/msm/mdss: Rename path references to mdp_path Dmitry Baryshkov
@ 2023-07-12 12:11 ` Dmitry Baryshkov
  2023-07-12 12:11 ` [PATCH v2 6/8] drm/msm/mdss: populate missing data Dmitry Baryshkov
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

There are just two places where we set the bandwidth: in the resume and
in the suspend paths. Drop the wrapping function
msm_mdss_icc_request_bw() and call icc_set_bw() directly.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 809c93b22c9c..eed96976e260 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -74,14 +74,6 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
 	return 0;
 }
 
-static void msm_mdss_icc_request_bw(struct msm_mdss *msm_mdss, unsigned long bw)
-{
-	int i;
-
-	for (i = 0; i < msm_mdss->num_mdp_paths; i++)
-		icc_set_bw(msm_mdss->mdp_path[i], 0, Bps_to_icc(bw));
-}
-
 static void msm_mdss_irq(struct irq_desc *desc)
 {
 	struct msm_mdss *msm_mdss = irq_desc_get_handler_data(desc);
@@ -229,14 +221,15 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss)
 
 static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 {
-	int ret;
+	int ret, i;
 
 	/*
 	 * Several components have AXI clocks that can only be turned on if
 	 * the interconnect is enabled (non-zero bandwidth). Let's make sure
 	 * that the interconnects are at least at a minimum amount.
 	 */
-	msm_mdss_icc_request_bw(msm_mdss, MIN_IB_BW);
+	for (i = 0; i < msm_mdss->num_mdp_paths; i++)
+		icc_set_bw(msm_mdss->mdp_path[i], 0, Bps_to_icc(MIN_IB_BW));
 
 	ret = clk_bulk_prepare_enable(msm_mdss->num_clocks, msm_mdss->clocks);
 	if (ret) {
@@ -284,8 +277,12 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 
 static int msm_mdss_disable(struct msm_mdss *msm_mdss)
 {
+	int i;
+
 	clk_bulk_disable_unprepare(msm_mdss->num_clocks, msm_mdss->clocks);
-	msm_mdss_icc_request_bw(msm_mdss, 0);
+
+	for (i = 0; i < msm_mdss->num_mdp_paths; i++)
+		icc_set_bw(msm_mdss->mdp_path[i], 0, 0);
 
 	return 0;
 }
-- 
2.40.1


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

* [PATCH v2 6/8] drm/msm/mdss: populate missing data
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
                   ` (4 preceding siblings ...)
  2023-07-12 12:11 ` [PATCH v2 5/8] drm/msm/mdss: inline msm_mdss_icc_request_bw() Dmitry Baryshkov
@ 2023-07-12 12:11 ` Dmitry Baryshkov
  2023-07-12 12:11 ` [PATCH v2 7/8] drm/msm/mdss: Handle the reg bus ICC path Dmitry Baryshkov
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

As we are going to use MDSS data for DPU programming, populate missing
MDSS data. The UBWC 1.0 and no UBWC cases do not require MDSS
programming, so skip them.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index eed96976e260..b7765e63d549 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -252,6 +252,10 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 	 * UBWC_n and the rest of params comes from hw data.
 	 */
 	switch (msm_mdss->mdss_data->ubwc_dec_version) {
+	case 0: /* no UBWC */
+	case UBWC_1_0:
+		/* do nothing */
+		break;
 	case UBWC_2_0:
 		msm_mdss_setup_ubwc_dec_20(msm_mdss);
 		break;
@@ -491,10 +495,22 @@ static int mdss_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct msm_mdss_data msm8998_data = {
+	.ubwc_version = UBWC_1_0,
+	.ubwc_dec_version = UBWC_1_0,
+	.highest_bank_bit = 1,
+};
+
+static const struct msm_mdss_data qcm2290_data = {
+	/* no UBWC */
+	.highest_bank_bit = 0x2,
+};
+
 static const struct msm_mdss_data sc7180_data = {
 	.ubwc_version = UBWC_2_0,
 	.ubwc_dec_version = UBWC_2_0,
 	.ubwc_static = 0x1e,
+	.highest_bank_bit = 0x3,
 };
 
 static const struct msm_mdss_data sc7280_data = {
@@ -547,6 +563,7 @@ static const struct msm_mdss_data sm6115_data = {
 	.ubwc_dec_version = UBWC_2_0,
 	.ubwc_swizzle = 7,
 	.ubwc_static = 0x11f,
+	.highest_bank_bit = 0x1,
 };
 
 static const struct msm_mdss_data sm8250_data = {
@@ -571,8 +588,8 @@ static const struct msm_mdss_data sm8550_data = {
 
 static const struct of_device_id mdss_dt_match[] = {
 	{ .compatible = "qcom,mdss" },
-	{ .compatible = "qcom,msm8998-mdss" },
-	{ .compatible = "qcom,qcm2290-mdss" },
+	{ .compatible = "qcom,msm8998-mdss", .data = &msm8998_data },
+	{ .compatible = "qcom,qcm2290-mdss", .data = &qcm2290_data },
 	{ .compatible = "qcom,sdm845-mdss", .data = &sdm845_data },
 	{ .compatible = "qcom,sc7180-mdss", .data = &sc7180_data },
 	{ .compatible = "qcom,sc7280-mdss", .data = &sc7280_data },
-- 
2.40.1


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

* [PATCH v2 7/8] drm/msm/mdss: Handle the reg bus ICC path
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
                   ` (5 preceding siblings ...)
  2023-07-12 12:11 ` [PATCH v2 6/8] drm/msm/mdss: populate missing data Dmitry Baryshkov
@ 2023-07-12 12:11 ` Dmitry Baryshkov
  2023-07-15 13:19   ` Konrad Dybcio
  2023-07-12 12:11 ` [PATCH v2 8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect Dmitry Baryshkov
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
another path that needs to be handled to ensure MDSS functions properly,
namely the "reg bus", a.k.a the CPU-MDSS interconnect.

Gating that path may have a variety of effects, from none to otherwise
inexplicable DSI timeouts.

Provide a way for MDSS driver to vote on this bus.

A note regarding vote values. Newer platforms have corresponding
bandwidth values in the vendor DT files. For the older platforms there
was a static vote in the mdss_mdp and rotator drivers. I choose to be
conservative here and choose this value as a default.

Co-developed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 51 +++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index b7765e63d549..ee31a9ab88d4 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -26,6 +26,8 @@
 
 #define MIN_IB_BW	400000000UL /* Min ib vote 400MB */
 
+#define DEFAULT_REG_BW	153600000UL /* Used in mdss fbdev driver */
+
 struct msm_mdss_data {
 	u32 ubwc_version;
 	/* can be read from register 0x58 */
@@ -34,6 +36,8 @@ struct msm_mdss_data {
 	u32 ubwc_static;
 	u32 highest_bank_bit;
 	u32 macrotile_mode;
+
+	unsigned long reg_bus_bw;
 };
 
 struct msm_mdss {
@@ -50,6 +54,7 @@ struct msm_mdss {
 	const struct msm_mdss_data *mdss_data;
 	struct icc_path *mdp_path[2];
 	u32 num_mdp_paths;
+	struct icc_path *reg_bus_path;
 };
 
 static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
@@ -57,6 +62,7 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
 {
 	struct icc_path *path0;
 	struct icc_path *path1;
+	struct icc_path *reg_bus_path;
 
 	path0 = devm_of_icc_get(dev, "mdp0-mem");
 	if (IS_ERR_OR_NULL(path0))
@@ -71,6 +77,10 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
 		msm_mdss->num_mdp_paths++;
 	}
 
+	reg_bus_path = of_icc_get(dev, "cpu-cfg");
+	if (!IS_ERR_OR_NULL(reg_bus_path))
+		msm_mdss->reg_bus_path = reg_bus_path;
+
 	return 0;
 }
 
@@ -231,6 +241,13 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 	for (i = 0; i < msm_mdss->num_mdp_paths; i++)
 		icc_set_bw(msm_mdss->mdp_path[i], 0, Bps_to_icc(MIN_IB_BW));
 
+	if (msm_mdss->mdss_data && msm_mdss->mdss_data->reg_bus_bw)
+		icc_set_bw(msm_mdss->reg_bus_path, 0,
+			   Bps_to_icc(msm_mdss->mdss_data->reg_bus_bw));
+	else
+		icc_set_bw(msm_mdss->reg_bus_path, 0,
+			   Bps_to_icc(DEFAULT_REG_BW));
+
 	ret = clk_bulk_prepare_enable(msm_mdss->num_clocks, msm_mdss->clocks);
 	if (ret) {
 		dev_err(msm_mdss->dev, "clock enable failed, ret:%d\n", ret);
@@ -288,6 +305,9 @@ static int msm_mdss_disable(struct msm_mdss *msm_mdss)
 	for (i = 0; i < msm_mdss->num_mdp_paths; i++)
 		icc_set_bw(msm_mdss->mdp_path[i], 0, 0);
 
+	if (msm_mdss->reg_bus_path)
+		icc_set_bw(msm_mdss->reg_bus_path, 0, 0);
+
 	return 0;
 }
 
@@ -374,6 +394,8 @@ static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5
 	if (!msm_mdss)
 		return ERR_PTR(-ENOMEM);
 
+	msm_mdss->mdss_data = of_device_get_match_data(&pdev->dev);
+
 	msm_mdss->mmio = devm_platform_ioremap_resource_byname(pdev, is_mdp5 ? "mdss_phys" : "mdss");
 	if (IS_ERR(msm_mdss->mmio))
 		return ERR_CAST(msm_mdss->mmio);
@@ -464,8 +486,6 @@ static int mdss_probe(struct platform_device *pdev)
 	if (IS_ERR(mdss))
 		return PTR_ERR(mdss);
 
-	mdss->mdss_data = of_device_get_match_data(&pdev->dev);
-
 	platform_set_drvdata(pdev, mdss);
 
 	/*
@@ -499,11 +519,13 @@ static const struct msm_mdss_data msm8998_data = {
 	.ubwc_version = UBWC_1_0,
 	.ubwc_dec_version = UBWC_1_0,
 	.highest_bank_bit = 1,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data qcm2290_data = {
 	/* no UBWC */
 	.highest_bank_bit = 0x2,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data sc7180_data = {
@@ -511,6 +533,7 @@ static const struct msm_mdss_data sc7180_data = {
 	.ubwc_dec_version = UBWC_2_0,
 	.ubwc_static = 0x1e,
 	.highest_bank_bit = 0x3,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data sc7280_data = {
@@ -520,6 +543,7 @@ static const struct msm_mdss_data sc7280_data = {
 	.ubwc_static = 1,
 	.highest_bank_bit = 1,
 	.macrotile_mode = 1,
+	.reg_bus_bw = 74000 * 1000,
 };
 
 static const struct msm_mdss_data sc8180x_data = {
@@ -527,6 +551,7 @@ static const struct msm_mdss_data sc8180x_data = {
 	.ubwc_dec_version = UBWC_3_0,
 	.highest_bank_bit = 3,
 	.macrotile_mode = 1,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data sc8280xp_data = {
@@ -536,12 +561,14 @@ static const struct msm_mdss_data sc8280xp_data = {
 	.ubwc_static = 1,
 	.highest_bank_bit = 2,
 	.macrotile_mode = 1,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data sdm845_data = {
 	.ubwc_version = UBWC_2_0,
 	.ubwc_dec_version = UBWC_2_0,
 	.highest_bank_bit = 2,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data sm6350_data = {
@@ -550,12 +577,14 @@ static const struct msm_mdss_data sm6350_data = {
 	.ubwc_swizzle = 6,
 	.ubwc_static = 0x1e,
 	.highest_bank_bit = 1,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data sm8150_data = {
 	.ubwc_version = UBWC_3_0,
 	.ubwc_dec_version = UBWC_3_0,
 	.highest_bank_bit = 2,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data sm6115_data = {
@@ -564,6 +593,7 @@ static const struct msm_mdss_data sm6115_data = {
 	.ubwc_swizzle = 7,
 	.ubwc_static = 0x11f,
 	.highest_bank_bit = 0x1,
+	.reg_bus_bw = 76800 * 1000,
 };
 
 static const struct msm_mdss_data sm8250_data = {
@@ -574,6 +604,18 @@ static const struct msm_mdss_data sm8250_data = {
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
 	.macrotile_mode = 1,
+	.reg_bus_bw = 76800 * 1000,
+};
+
+static const struct msm_mdss_data sm8350_data = {
+	.ubwc_version = UBWC_4_0,
+	.ubwc_dec_version = UBWC_4_0,
+	.ubwc_swizzle = 6,
+	.ubwc_static = 1,
+	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
+	.highest_bank_bit = 3,
+	.macrotile_mode = 1,
+	.reg_bus_bw = 74000 * 1000,
 };
 
 static const struct msm_mdss_data sm8550_data = {
@@ -584,6 +626,7 @@ static const struct msm_mdss_data sm8550_data = {
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
 	.macrotile_mode = 1,
+	.reg_bus_bw = 57000 * 1000,
 };
 
 static const struct of_device_id mdss_dt_match[] = {
@@ -600,8 +643,8 @@ static const struct of_device_id mdss_dt_match[] = {
 	{ .compatible = "qcom,sm6375-mdss", .data = &sm6350_data },
 	{ .compatible = "qcom,sm8150-mdss", .data = &sm8150_data },
 	{ .compatible = "qcom,sm8250-mdss", .data = &sm8250_data },
-	{ .compatible = "qcom,sm8350-mdss", .data = &sm8250_data },
-	{ .compatible = "qcom,sm8450-mdss", .data = &sm8250_data },
+	{ .compatible = "qcom,sm8350-mdss", .data = &sm8350_data },
+	{ .compatible = "qcom,sm8450-mdss", .data = &sm8350_data },
 	{ .compatible = "qcom,sm8550-mdss", .data = &sm8550_data },
 	{}
 };
-- 
2.40.1


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

* [PATCH v2 8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
                   ` (6 preceding siblings ...)
  2023-07-12 12:11 ` [PATCH v2 7/8] drm/msm/mdss: Handle the reg bus ICC path Dmitry Baryshkov
@ 2023-07-12 12:11 ` Dmitry Baryshkov
  2023-07-13  8:41   ` Konrad Dybcio
  2023-07-14  5:33 ` (subset) [PATCH v2 0/8] MDSS reg bus interconnect Bjorn Andersson
  2023-07-18  0:04 ` Abhinav Kumar
  9 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 12:11 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Abhinav Kumar, Marijn Suijten, Rob Herring, Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

Add support for the MDSS cfg-cpu bus vote on the SM8450 platform.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 595533aeafc4..0b01f3027ee3 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -13,6 +13,7 @@
 #include <dt-bindings/mailbox/qcom-ipcc.h>
 #include <dt-bindings/phy/phy-qcom-qmp.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
+#include <dt-bindings/interconnect/qcom,icc.h>
 #include <dt-bindings/interconnect/qcom,sm8450.h>
 #include <dt-bindings/soc/qcom,gpr.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
@@ -2672,8 +2673,12 @@ mdss: display-subsystem@ae00000 {
 
 			/* same path used twice */
 			interconnects = <&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>,
-					<&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>;
-			interconnect-names = "mdp0-mem", "mdp1-mem";
+					<&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>,
+					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_DISPLAY_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
+			interconnect-names = "mdp0-mem",
+					     "mdp1-mem",
+					     "cpu-cfg";
 
 			resets = <&dispcc DISP_CC_MDSS_CORE_BCR>;
 
-- 
2.40.1


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

* Re: [PATCH v2 1/8] dt-bindings: display/msm: Add reg bus and rotator interconnects
  2023-07-12 12:11 ` [PATCH v2 1/8] dt-bindings: display/msm: Add reg bus and rotator interconnects Dmitry Baryshkov
@ 2023-07-12 19:24   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-12 19:24 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, Rob Herring,
	Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

On 12/07/2023 14:11, Dmitry Baryshkov wrote:
> From: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
> Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there are
> other connection paths:
> - a path that connects rotator block to the DDR.
> - a path that needs to be handled to ensure MDSS register access
>   functions properly, namely the "reg bus", a.k.a the CPU-MDSS CFG
>   interconnect.
> 
> Describe these paths bindings to allow using them in device trees and in
> the driver
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


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

* Re: [PATCH v2 2/8] drm/msm/mdss: correct UBWC programming for SM8550
  2023-07-12 12:11 ` [PATCH v2 2/8] drm/msm/mdss: correct UBWC programming for SM8550 Dmitry Baryshkov
@ 2023-07-12 22:02   ` Abhinav Kumar
  2023-07-12 22:05     ` Dmitry Baryshkov
  0 siblings, 1 reply; 17+ messages in thread
From: Abhinav Kumar @ 2023-07-12 22:02 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Clark, Sean Paul, Marijn Suijten, Rob Herring,
	Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno



On 7/12/2023 5:11 AM, Dmitry Baryshkov wrote:
> The SM8550 platform employs newer UBWC decoder, which requires slightly
> different programming.
> 
> Fixes: a2f33995c19d ("drm/msm: mdss: add support for SM8550")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

Do we also need another fixes tag

Fixes: d68db6069a8e ("drm/msm/mdss: convert UBWC setup to use match data")

Also, this was previously part of 
https://patchwork.freedesktop.org/series/118074/ .

In this one its after the bindings change.

For easier picking into -fixes, will you be moving this ahead of the 
bindings change and OR do you want to keep this part of the old series 
as it seems better suited there.

I think even if I pick this for -fixes, rest of this series can be 
rebased without issues. But let me know what you would prefer.

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

* Re: [PATCH v2 2/8] drm/msm/mdss: correct UBWC programming for SM8550
  2023-07-12 22:02   ` Abhinav Kumar
@ 2023-07-12 22:05     ` Dmitry Baryshkov
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-12 22:05 UTC (permalink / raw)
  To: Abhinav Kumar, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Clark, Sean Paul, Marijn Suijten, Rob Herring,
	Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

On 13/07/2023 01:02, Abhinav Kumar wrote:
> 
> 
> On 7/12/2023 5:11 AM, Dmitry Baryshkov wrote:
>> The SM8550 platform employs newer UBWC decoder, which requires slightly
>> different programming.
>>
>> Fixes: a2f33995c19d ("drm/msm: mdss: add support for SM8550")
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> Do we also need another fixes tag
> 
> Fixes: d68db6069a8e ("drm/msm/mdss: convert UBWC setup to use match data")

No. We do not need to fix the conversion. Only the sm8550 setup.

> 
> Also, this was previously part of 
> https://patchwork.freedesktop.org/series/118074/ .
> 
> In this one its after the bindings change.
> 
> For easier picking into -fixes, will you be moving this ahead of the 
> bindings change and OR do you want to keep this part of the old series 
> as it seems better suited there.
> 
> I think even if I pick this for -fixes, rest of this series can be 
> rebased without issues. But let me know what you would prefer.


I had rejects with the original series (and 0 reviews), so it was easier 
to push that as a part of this series too.

I'm fine with you pulling either of the patches into -fixes.

-- 
With best wishes
Dmitry


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

* Re: [PATCH v2 8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect
  2023-07-12 12:11 ` [PATCH v2 8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect Dmitry Baryshkov
@ 2023-07-13  8:41   ` Konrad Dybcio
  2023-07-13  9:04     ` Dmitry Baryshkov
  0 siblings, 1 reply; 17+ messages in thread
From: Konrad Dybcio @ 2023-07-13  8:41 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Clark,
	Sean Paul, Abhinav Kumar, Marijn Suijten, Rob Herring,
	Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

On 12.07.2023 14:11, Dmitry Baryshkov wrote:
> Add support for the MDSS cfg-cpu bus vote on the SM8450 platform.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 595533aeafc4..0b01f3027ee3 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -13,6 +13,7 @@
>  #include <dt-bindings/mailbox/qcom-ipcc.h>
>  #include <dt-bindings/phy/phy-qcom-qmp.h>
>  #include <dt-bindings/power/qcom-rpmpd.h>
> +#include <dt-bindings/interconnect/qcom,icc.h>
>  #include <dt-bindings/interconnect/qcom,sm8450.h>
>  #include <dt-bindings/soc/qcom,gpr.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> @@ -2672,8 +2673,12 @@ mdss: display-subsystem@ae00000 {
>  
>  			/* same path used twice */
>  			interconnects = <&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>,
> -					<&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>;
> -			interconnect-names = "mdp0-mem", "mdp1-mem";
> +					<&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>,
> +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
> +					 &config_noc SLAVE_DISPLAY_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
Looking at icc_set_tag occurences in msm-5.10/techpack/display,
I *think* active-only is only possible for the data bus (MDP-EBI)

Moreover, I think Linux is supposed to cast MDSS votes through the
APPS RSC (so, nodes without _DISP [1][2]) and conversely, DISP_RSC is
supposed to active-only votes

Konrad

[1] not that it matters today because it's not implemented yet
[2] https://lore.kernel.org/linux-arm-msm/20230708-topic-rpmh_icc_rsc-v1-0-b223bd2ac8dd@linaro.org

> +			interconnect-names = "mdp0-mem",
> +					     "mdp1-mem",
> +					     "cpu-cfg";
>  
>  			resets = <&dispcc DISP_CC_MDSS_CORE_BCR>;
>  

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

* Re: [PATCH v2 8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect
  2023-07-13  8:41   ` Konrad Dybcio
@ 2023-07-13  9:04     ` Dmitry Baryshkov
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2023-07-13  9:04 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: devicetree, Bjorn Andersson, Abhinav Kumar, Rob Herring,
	Stephen Boyd, Andy Gross, dri-devel, Krzysztof Kozlowski,
	linux-arm-msm, Marijn Suijten, freedreno, Sean Paul

On Thu, 13 Jul 2023 at 11:41, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 12.07.2023 14:11, Dmitry Baryshkov wrote:
> > Add support for the MDSS cfg-cpu bus vote on the SM8450 platform.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8450.dtsi | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > index 595533aeafc4..0b01f3027ee3 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > @@ -13,6 +13,7 @@
> >  #include <dt-bindings/mailbox/qcom-ipcc.h>
> >  #include <dt-bindings/phy/phy-qcom-qmp.h>
> >  #include <dt-bindings/power/qcom-rpmpd.h>
> > +#include <dt-bindings/interconnect/qcom,icc.h>
> >  #include <dt-bindings/interconnect/qcom,sm8450.h>
> >  #include <dt-bindings/soc/qcom,gpr.h>
> >  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> > @@ -2672,8 +2673,12 @@ mdss: display-subsystem@ae00000 {
> >
> >                       /* same path used twice */
> >                       interconnects = <&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>,
> > -                                     <&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>;
> > -                     interconnect-names = "mdp0-mem", "mdp1-mem";
> > +                                     <&mmss_noc MASTER_MDP_DISP 0 &mc_virt SLAVE_EBI1_DISP 0>,
> > +                                     <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
> > +                                      &config_noc SLAVE_DISPLAY_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
> Looking at icc_set_tag occurences in msm-5.10/techpack/display,
> I *think* active-only is only possible for the data bus (MDP-EBI)

Here I followed the vendor mdss fbdev driver (mdss_mdp.c), which
explicitly states:

static struct msm_bus_scale_pdata mdp_reg_bus_scale_table = {
        .usecase = mdp_reg_bus_usecases,
        .num_usecases = ARRAY_SIZE(mdp_reg_bus_usecases),
        .name = "mdss_reg",
        .active_only = true,
};

>
> Moreover, I think Linux is supposed to cast MDSS votes through the
> APPS RSC (so, nodes without _DISP [1][2]) and conversely, DISP_RSC is
> supposed to active-only votes

We can change this once your DISP_RSC lands. Anyway, I think we will
have to add the LLCC-MEM vote at some point later.

>
> Konrad
>
> [1] not that it matters today because it's not implemented yet
> [2] https://lore.kernel.org/linux-arm-msm/20230708-topic-rpmh_icc_rsc-v1-0-b223bd2ac8dd@linaro.org
>
> > +                     interconnect-names = "mdp0-mem",
> > +                                          "mdp1-mem",
> > +                                          "cpu-cfg";
> >
> >                       resets = <&dispcc DISP_CC_MDSS_CORE_BCR>;
> >



-- 
With best wishes
Dmitry

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

* Re: (subset) [PATCH v2 0/8] MDSS reg bus interconnect
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
                   ` (7 preceding siblings ...)
  2023-07-12 12:11 ` [PATCH v2 8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect Dmitry Baryshkov
@ 2023-07-14  5:33 ` Bjorn Andersson
  2023-07-18  0:04 ` Abhinav Kumar
  9 siblings, 0 replies; 17+ messages in thread
From: Bjorn Andersson @ 2023-07-14  5:33 UTC (permalink / raw)
  To: Andy Gross, Konrad Dybcio, Rob Clark, Sean Paul, Abhinav Kumar,
	Marijn Suijten, Rob Herring, Krzysztof Kozlowski,
	Dmitry Baryshkov
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno


On Wed, 12 Jul 2023 15:11:37 +0300, Dmitry Baryshkov wrote:
> Per agreement with Konrad, picked up this patch series.
> 
> Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
> another path that needs to be handled to ensure MDSS functions properly,
> namely the "reg bus", a.k.a the CPU-MDSS interconnect.
> 
> Gating that path may have a variety of effects. from none to otherwise
> inexplicable DSI timeouts.
> 
> [...]

Applied, thanks!

[8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect
      commit: 4e125191e6cb00d6c3f3a8e1b67fd242e639b3c3

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

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

* Re: [PATCH v2 7/8] drm/msm/mdss: Handle the reg bus ICC path
  2023-07-12 12:11 ` [PATCH v2 7/8] drm/msm/mdss: Handle the reg bus ICC path Dmitry Baryshkov
@ 2023-07-15 13:19   ` Konrad Dybcio
  0 siblings, 0 replies; 17+ messages in thread
From: Konrad Dybcio @ 2023-07-15 13:19 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Clark,
	Sean Paul, Abhinav Kumar, Marijn Suijten, Rob Herring,
	Krzysztof Kozlowski
  Cc: devicetree, linux-arm-msm, dri-devel, Stephen Boyd, freedreno

On 12.07.2023 14:11, Dmitry Baryshkov wrote:
> Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
> another path that needs to be handled to ensure MDSS functions properly,
> namely the "reg bus", a.k.a the CPU-MDSS interconnect.
> 
> Gating that path may have a variety of effects, from none to otherwise
> inexplicable DSI timeouts.
> 
> Provide a way for MDSS driver to vote on this bus.
> 
> A note regarding vote values. Newer platforms have corresponding
> bandwidth values in the vendor DT files. For the older platforms there
> was a static vote in the mdss_mdp and rotator drivers. I choose to be
> conservative here and choose this value as a default.
> 
> Co-developed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
We can store data in icc units (without the *1000).

Konrad
>  drivers/gpu/drm/msm/msm_mdss.c | 51 +++++++++++++++++++++++++++++++---
>  1 file changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index b7765e63d549..ee31a9ab88d4 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -26,6 +26,8 @@
>  
>  #define MIN_IB_BW	400000000UL /* Min ib vote 400MB */
>  
> +#define DEFAULT_REG_BW	153600000UL /* Used in mdss fbdev driver */
> +
>  struct msm_mdss_data {
>  	u32 ubwc_version;
>  	/* can be read from register 0x58 */
> @@ -34,6 +36,8 @@ struct msm_mdss_data {
>  	u32 ubwc_static;
>  	u32 highest_bank_bit;
>  	u32 macrotile_mode;
> +
> +	unsigned long reg_bus_bw;
>  };
>  
>  struct msm_mdss {
> @@ -50,6 +54,7 @@ struct msm_mdss {
>  	const struct msm_mdss_data *mdss_data;
>  	struct icc_path *mdp_path[2];
>  	u32 num_mdp_paths;
> +	struct icc_path *reg_bus_path;
>  };
>  
>  static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
> @@ -57,6 +62,7 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
>  {
>  	struct icc_path *path0;
>  	struct icc_path *path1;
> +	struct icc_path *reg_bus_path;
>  
>  	path0 = devm_of_icc_get(dev, "mdp0-mem");
>  	if (IS_ERR_OR_NULL(path0))
> @@ -71,6 +77,10 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
>  		msm_mdss->num_mdp_paths++;
>  	}
>  
> +	reg_bus_path = of_icc_get(dev, "cpu-cfg");
> +	if (!IS_ERR_OR_NULL(reg_bus_path))
> +		msm_mdss->reg_bus_path = reg_bus_path;
> +
>  	return 0;
>  }
>  
> @@ -231,6 +241,13 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
>  	for (i = 0; i < msm_mdss->num_mdp_paths; i++)
>  		icc_set_bw(msm_mdss->mdp_path[i], 0, Bps_to_icc(MIN_IB_BW));
>  
> +	if (msm_mdss->mdss_data && msm_mdss->mdss_data->reg_bus_bw)
> +		icc_set_bw(msm_mdss->reg_bus_path, 0,
> +			   Bps_to_icc(msm_mdss->mdss_data->reg_bus_bw));
> +	else
> +		icc_set_bw(msm_mdss->reg_bus_path, 0,
> +			   Bps_to_icc(DEFAULT_REG_BW));
> +
>  	ret = clk_bulk_prepare_enable(msm_mdss->num_clocks, msm_mdss->clocks);
>  	if (ret) {
>  		dev_err(msm_mdss->dev, "clock enable failed, ret:%d\n", ret);
> @@ -288,6 +305,9 @@ static int msm_mdss_disable(struct msm_mdss *msm_mdss)
>  	for (i = 0; i < msm_mdss->num_mdp_paths; i++)
>  		icc_set_bw(msm_mdss->mdp_path[i], 0, 0);
>  
> +	if (msm_mdss->reg_bus_path)
> +		icc_set_bw(msm_mdss->reg_bus_path, 0, 0);
> +
>  	return 0;
>  }
>  
> @@ -374,6 +394,8 @@ static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5
>  	if (!msm_mdss)
>  		return ERR_PTR(-ENOMEM);
>  
> +	msm_mdss->mdss_data = of_device_get_match_data(&pdev->dev);
> +
>  	msm_mdss->mmio = devm_platform_ioremap_resource_byname(pdev, is_mdp5 ? "mdss_phys" : "mdss");
>  	if (IS_ERR(msm_mdss->mmio))
>  		return ERR_CAST(msm_mdss->mmio);
> @@ -464,8 +486,6 @@ static int mdss_probe(struct platform_device *pdev)
>  	if (IS_ERR(mdss))
>  		return PTR_ERR(mdss);
>  
> -	mdss->mdss_data = of_device_get_match_data(&pdev->dev);
> -
>  	platform_set_drvdata(pdev, mdss);
>  
>  	/*
> @@ -499,11 +519,13 @@ static const struct msm_mdss_data msm8998_data = {
>  	.ubwc_version = UBWC_1_0,
>  	.ubwc_dec_version = UBWC_1_0,
>  	.highest_bank_bit = 1,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data qcm2290_data = {
>  	/* no UBWC */
>  	.highest_bank_bit = 0x2,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data sc7180_data = {
> @@ -511,6 +533,7 @@ static const struct msm_mdss_data sc7180_data = {
>  	.ubwc_dec_version = UBWC_2_0,
>  	.ubwc_static = 0x1e,
>  	.highest_bank_bit = 0x3,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data sc7280_data = {
> @@ -520,6 +543,7 @@ static const struct msm_mdss_data sc7280_data = {
>  	.ubwc_static = 1,
>  	.highest_bank_bit = 1,
>  	.macrotile_mode = 1,
> +	.reg_bus_bw = 74000 * 1000,
>  };
>  
>  static const struct msm_mdss_data sc8180x_data = {
> @@ -527,6 +551,7 @@ static const struct msm_mdss_data sc8180x_data = {
>  	.ubwc_dec_version = UBWC_3_0,
>  	.highest_bank_bit = 3,
>  	.macrotile_mode = 1,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data sc8280xp_data = {
> @@ -536,12 +561,14 @@ static const struct msm_mdss_data sc8280xp_data = {
>  	.ubwc_static = 1,
>  	.highest_bank_bit = 2,
>  	.macrotile_mode = 1,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data sdm845_data = {
>  	.ubwc_version = UBWC_2_0,
>  	.ubwc_dec_version = UBWC_2_0,
>  	.highest_bank_bit = 2,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data sm6350_data = {
> @@ -550,12 +577,14 @@ static const struct msm_mdss_data sm6350_data = {
>  	.ubwc_swizzle = 6,
>  	.ubwc_static = 0x1e,
>  	.highest_bank_bit = 1,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data sm8150_data = {
>  	.ubwc_version = UBWC_3_0,
>  	.ubwc_dec_version = UBWC_3_0,
>  	.highest_bank_bit = 2,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data sm6115_data = {
> @@ -564,6 +593,7 @@ static const struct msm_mdss_data sm6115_data = {
>  	.ubwc_swizzle = 7,
>  	.ubwc_static = 0x11f,
>  	.highest_bank_bit = 0x1,
> +	.reg_bus_bw = 76800 * 1000,
>  };
>  
>  static const struct msm_mdss_data sm8250_data = {
> @@ -574,6 +604,18 @@ static const struct msm_mdss_data sm8250_data = {
>  	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
>  	.highest_bank_bit = 3,
>  	.macrotile_mode = 1,
> +	.reg_bus_bw = 76800 * 1000,
> +};
> +
> +static const struct msm_mdss_data sm8350_data = {
> +	.ubwc_version = UBWC_4_0,
> +	.ubwc_dec_version = UBWC_4_0,
> +	.ubwc_swizzle = 6,
> +	.ubwc_static = 1,
> +	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
> +	.highest_bank_bit = 3,
> +	.macrotile_mode = 1,
> +	.reg_bus_bw = 74000 * 1000,
>  };
>  
>  static const struct msm_mdss_data sm8550_data = {
> @@ -584,6 +626,7 @@ static const struct msm_mdss_data sm8550_data = {
>  	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
>  	.highest_bank_bit = 3,
>  	.macrotile_mode = 1,
> +	.reg_bus_bw = 57000 * 1000,
>  };
>  
>  static const struct of_device_id mdss_dt_match[] = {
> @@ -600,8 +643,8 @@ static const struct of_device_id mdss_dt_match[] = {
>  	{ .compatible = "qcom,sm6375-mdss", .data = &sm6350_data },
>  	{ .compatible = "qcom,sm8150-mdss", .data = &sm8150_data },
>  	{ .compatible = "qcom,sm8250-mdss", .data = &sm8250_data },
> -	{ .compatible = "qcom,sm8350-mdss", .data = &sm8250_data },
> -	{ .compatible = "qcom,sm8450-mdss", .data = &sm8250_data },
> +	{ .compatible = "qcom,sm8350-mdss", .data = &sm8350_data },
> +	{ .compatible = "qcom,sm8450-mdss", .data = &sm8350_data },
>  	{ .compatible = "qcom,sm8550-mdss", .data = &sm8550_data },
>  	{}
>  };

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

* Re: (subset) [PATCH v2 0/8] MDSS reg bus interconnect
  2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
                   ` (8 preceding siblings ...)
  2023-07-14  5:33 ` (subset) [PATCH v2 0/8] MDSS reg bus interconnect Bjorn Andersson
@ 2023-07-18  0:04 ` Abhinav Kumar
  9 siblings, 0 replies; 17+ messages in thread
From: Abhinav Kumar @ 2023-07-18  0:04 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Clark, Sean Paul,
	Marijn Suijten, Rob Herring, Krzysztof Kozlowski,
	Dmitry Baryshkov
  Cc: devicetree, linux-arm-msm, Abhinav Kumar, dri-devel,
	Stephen Boyd, freedreno


On Wed, 12 Jul 2023 15:11:37 +0300, Dmitry Baryshkov wrote:
> Per agreement with Konrad, picked up this patch series.
> 
> Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
> another path that needs to be handled to ensure MDSS functions properly,
> namely the "reg bus", a.k.a the CPU-MDSS interconnect.
> 
> Gating that path may have a variety of effects. from none to otherwise
> inexplicable DSI timeouts.
> 
> [...]

Applied, thanks!

[2/8] drm/msm/mdss: correct UBWC programming for SM8550
      https://gitlab.freedesktop.org/drm/msm/-/commit/a85c238c5ccd

Best regards,
-- 
Abhinav Kumar <quic_abhinavk@quicinc.com>

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

end of thread, other threads:[~2023-07-18  0:05 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-12 12:11 [PATCH v2 0/8] MDSS reg bus interconnect Dmitry Baryshkov
2023-07-12 12:11 ` [PATCH v2 1/8] dt-bindings: display/msm: Add reg bus and rotator interconnects Dmitry Baryshkov
2023-07-12 19:24   ` Krzysztof Kozlowski
2023-07-12 12:11 ` [PATCH v2 2/8] drm/msm/mdss: correct UBWC programming for SM8550 Dmitry Baryshkov
2023-07-12 22:02   ` Abhinav Kumar
2023-07-12 22:05     ` Dmitry Baryshkov
2023-07-12 12:11 ` [PATCH v2 3/8] drm/msm/mdss: switch mdss to use devm_of_icc_get() Dmitry Baryshkov
2023-07-12 12:11 ` [PATCH v2 4/8] drm/msm/mdss: Rename path references to mdp_path Dmitry Baryshkov
2023-07-12 12:11 ` [PATCH v2 5/8] drm/msm/mdss: inline msm_mdss_icc_request_bw() Dmitry Baryshkov
2023-07-12 12:11 ` [PATCH v2 6/8] drm/msm/mdss: populate missing data Dmitry Baryshkov
2023-07-12 12:11 ` [PATCH v2 7/8] drm/msm/mdss: Handle the reg bus ICC path Dmitry Baryshkov
2023-07-15 13:19   ` Konrad Dybcio
2023-07-12 12:11 ` [PATCH v2 8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect Dmitry Baryshkov
2023-07-13  8:41   ` Konrad Dybcio
2023-07-13  9:04     ` Dmitry Baryshkov
2023-07-14  5:33 ` (subset) [PATCH v2 0/8] MDSS reg bus interconnect Bjorn Andersson
2023-07-18  0:04 ` Abhinav Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).