Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/5] drm/msm/dpu: clean up references of DPU custom bus scaling
       [not found] <20190508204219.31687-1-robdclark@gmail.com>
@ 2019-05-08 20:42 ` Rob Clark
  2019-05-13 14:42   ` Sean Paul
  2019-05-08 20:42 ` [PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS Rob Clark
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Rob Clark @ 2019-05-08 20:42 UTC (permalink / raw)
  To: dri-devel
  Cc: Jayant Shekhar, Sravanthi Kollukuduru, Rob Clark, Rob Clark,
	Sean Paul, David Airlie, Daniel Vetter, Jeykumar Sankaran,
	Jordan Crouse, Bruce Wang, Abhinav Kumar, linux-arm-msm,
	freedreno, linux-kernel

From: Jayant Shekhar <jshekhar@codeaurora.org>

Since the upstream interconnect bus framework has landed
upstream, the existing references of custom bus scaling
needs to be cleaned up.

Changes in v2:
	- Fixed build error due to partial clean up

Changes in v3:
	- Condense multiple lines into a single line (Sean Paul)

Changes in v4-v7:
	- None

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
Signed-off-by: Jayant Shekhar <jshekhar@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 174 +++++++-----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      |  11 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h     |  22 +--
 4 files changed, 80 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 9f20f397f77d..e231c37a9dbb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -77,7 +77,6 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
 		struct dpu_core_perf_params *perf)
 {
 	struct dpu_crtc_state *dpu_cstate;
-	int i;
 
 	if (!kms || !kms->catalog || !crtc || !state || !perf) {
 		DPU_ERROR("invalid parameters\n");
@@ -88,35 +87,24 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
 	memset(perf, 0, sizeof(struct dpu_core_perf_params));
 
 	if (!dpu_cstate->bw_control) {
-		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-			perf->bw_ctl[i] = kms->catalog->perf.max_bw_high *
+		perf->bw_ctl = kms->catalog->perf.max_bw_high *
 					1000ULL;
-			perf->max_per_pipe_ib[i] = perf->bw_ctl[i];
-		}
+		perf->max_per_pipe_ib = perf->bw_ctl;
 		perf->core_clk_rate = kms->perf.max_core_clk_rate;
 	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
-		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-			perf->bw_ctl[i] = 0;
-			perf->max_per_pipe_ib[i] = 0;
-		}
+		perf->bw_ctl = 0;
+		perf->max_per_pipe_ib = 0;
 		perf->core_clk_rate = 0;
 	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
-		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-			perf->bw_ctl[i] = kms->perf.fix_core_ab_vote;
-			perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote;
-		}
+		perf->bw_ctl = kms->perf.fix_core_ab_vote;
+		perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote;
 		perf->core_clk_rate = kms->perf.fix_core_clk_rate;
 	}
 
 	DPU_DEBUG(
-		"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n",
+		"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu\n",
 			crtc->base.id, perf->core_clk_rate,
-			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
-			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
-			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
-			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
-			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_EBI],
-			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI]);
+			perf->max_per_pipe_ib, perf->bw_ctl);
 }
 
 int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
@@ -129,7 +117,6 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
 	struct dpu_crtc_state *dpu_cstate;
 	struct drm_crtc *tmp_crtc;
 	struct dpu_kms *kms;
-	int i;
 
 	if (!crtc || !state) {
 		DPU_ERROR("invalid crtc\n");
@@ -151,31 +138,25 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
 	/* obtain new values */
 	_dpu_core_perf_calc_crtc(kms, crtc, state, &dpu_cstate->new_perf);
 
-	for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
-			i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-		bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i];
-		curr_client_type = dpu_crtc_get_client_type(crtc);
+	bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl;
+	curr_client_type = dpu_crtc_get_client_type(crtc);
 
-		drm_for_each_crtc(tmp_crtc, crtc->dev) {
-			if (tmp_crtc->enabled &&
-			    (dpu_crtc_get_client_type(tmp_crtc) ==
-					    curr_client_type) &&
-			    (tmp_crtc != crtc)) {
-				struct dpu_crtc_state *tmp_cstate =
-					to_dpu_crtc_state(tmp_crtc->state);
-
-				DPU_DEBUG("crtc:%d bw:%llu ctrl:%d\n",
-					tmp_crtc->base.id,
-					tmp_cstate->new_perf.bw_ctl[i],
-					tmp_cstate->bw_control);
-				/*
-				 * For bw check only use the bw if the
-				 * atomic property has been already set
-				 */
-				if (tmp_cstate->bw_control)
-					bw_sum_of_intfs +=
-						tmp_cstate->new_perf.bw_ctl[i];
-			}
+	drm_for_each_crtc(tmp_crtc, crtc->dev) {
+		if (tmp_crtc->enabled &&
+		    (dpu_crtc_get_client_type(tmp_crtc) ==
+				curr_client_type) && (tmp_crtc != crtc)) {
+			struct dpu_crtc_state *tmp_cstate =
+				to_dpu_crtc_state(tmp_crtc->state);
+
+			DPU_DEBUG("crtc:%d bw:%llu ctrl:%d\n",
+				tmp_crtc->base.id, tmp_cstate->new_perf.bw_ctl,
+				tmp_cstate->bw_control);
+			/*
+			 * For bw check only use the bw if the
+			 * atomic property has been already set
+			 */
+			if (tmp_cstate->bw_control)
+				bw_sum_of_intfs += tmp_cstate->new_perf.bw_ctl;
 		}
 
 		/* convert bandwidth to kb */
@@ -206,9 +187,9 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
 }
 
 static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
-		struct drm_crtc *crtc, u32 bus_id)
+		struct drm_crtc *crtc)
 {
-	struct dpu_core_perf_params perf = { { 0 } };
+	struct dpu_core_perf_params perf = { 0 };
 	enum dpu_crtc_client_type curr_client_type
 					= dpu_crtc_get_client_type(crtc);
 	struct drm_crtc *tmp_crtc;
@@ -221,13 +202,11 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
 				dpu_crtc_get_client_type(tmp_crtc)) {
 			dpu_cstate = to_dpu_crtc_state(tmp_crtc->state);
 
-			perf.max_per_pipe_ib[bus_id] =
-				max(perf.max_per_pipe_ib[bus_id],
-				dpu_cstate->new_perf.max_per_pipe_ib[bus_id]);
+			perf.max_per_pipe_ib = max(perf.max_per_pipe_ib,
+					dpu_cstate->new_perf.max_per_pipe_ib);
 
-			DPU_DEBUG("crtc=%d bus_id=%d bw=%llu\n",
-				tmp_crtc->base.id, bus_id,
-				dpu_cstate->new_perf.bw_ctl[bus_id]);
+			DPU_DEBUG("crtc=%d bw=%llu\n", tmp_crtc->base.id,
+					dpu_cstate->new_perf.bw_ctl);
 		}
 	}
 	return ret;
@@ -247,7 +226,6 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
 	struct dpu_crtc *dpu_crtc;
 	struct dpu_crtc_state *dpu_cstate;
 	struct dpu_kms *kms;
-	int i;
 
 	if (!crtc) {
 		DPU_ERROR("invalid crtc\n");
@@ -283,10 +261,8 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
 	if (kms->perf.enable_bw_release) {
 		trace_dpu_cmd_release_bw(crtc->base.id);
 		DPU_DEBUG("Release BW crtc=%d\n", crtc->base.id);
-		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-			dpu_crtc->cur_perf.bw_ctl[i] = 0;
-			_dpu_core_perf_crtc_update_bus(kms, crtc, i);
-		}
+		dpu_crtc->cur_perf.bw_ctl = 0;
+		_dpu_core_perf_crtc_update_bus(kms, crtc);
 	}
 }
 
@@ -329,11 +305,10 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
 		int params_changed, bool stop_req)
 {
 	struct dpu_core_perf_params *new, *old;
-	int update_bus = 0, update_clk = 0;
+	bool update_bus = false, update_clk = false;
 	u64 clk_rate = 0;
 	struct dpu_crtc *dpu_crtc;
 	struct dpu_crtc_state *dpu_cstate;
-	int i;
 	struct msm_drm_private *priv;
 	struct dpu_kms *kms;
 	int ret;
@@ -360,62 +335,49 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
 	new = &dpu_cstate->new_perf;
 
 	if (crtc->enabled && !stop_req) {
-		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-			/*
-			 * cases for bus bandwidth update.
-			 * 1. new bandwidth vote - "ab or ib vote" is higher
-			 *    than current vote for update request.
-			 * 2. new bandwidth vote - "ab or ib vote" is lower
-			 *    than current vote at end of commit or stop.
-			 */
-			if ((params_changed && ((new->bw_ctl[i] >
-						old->bw_ctl[i]) ||
-				  (new->max_per_pipe_ib[i] >
-						old->max_per_pipe_ib[i]))) ||
-			    (!params_changed && ((new->bw_ctl[i] <
-						old->bw_ctl[i]) ||
-				  (new->max_per_pipe_ib[i] <
-						old->max_per_pipe_ib[i])))) {
-				DPU_DEBUG(
-					"crtc=%d p=%d new_bw=%llu,old_bw=%llu\n",
-					crtc->base.id, params_changed,
-					new->bw_ctl[i], old->bw_ctl[i]);
-				old->bw_ctl[i] = new->bw_ctl[i];
-				old->max_per_pipe_ib[i] =
-						new->max_per_pipe_ib[i];
-				update_bus |= BIT(i);
-			}
+		/*
+		 * cases for bus bandwidth update.
+		 * 1. new bandwidth vote - "ab or ib vote" is higher
+		 *    than current vote for update request.
+		 * 2. new bandwidth vote - "ab or ib vote" is lower
+		 *    than current vote at end of commit or stop.
+		 */
+		if ((params_changed && ((new->bw_ctl > old->bw_ctl) ||
+			(new->max_per_pipe_ib > old->max_per_pipe_ib)))	||
+			(!params_changed && ((new->bw_ctl < old->bw_ctl) ||
+			(new->max_per_pipe_ib < old->max_per_pipe_ib)))) {
+			DPU_DEBUG("crtc=%d p=%d new_bw=%llu,old_bw=%llu\n",
+				crtc->base.id, params_changed,
+				new->bw_ctl, old->bw_ctl);
+			old->bw_ctl = new->bw_ctl;
+			old->max_per_pipe_ib = new->max_per_pipe_ib;
+			update_bus = true;
 		}
 
 		if ((params_changed &&
-				(new->core_clk_rate > old->core_clk_rate)) ||
-				(!params_changed &&
-				(new->core_clk_rate < old->core_clk_rate))) {
+			(new->core_clk_rate > old->core_clk_rate)) ||
+			(!params_changed &&
+			(new->core_clk_rate < old->core_clk_rate))) {
 			old->core_clk_rate = new->core_clk_rate;
-			update_clk = 1;
+			update_clk = true;
 		}
 	} else {
 		DPU_DEBUG("crtc=%d disable\n", crtc->base.id);
 		memset(old, 0, sizeof(*old));
 		memset(new, 0, sizeof(*new));
-		update_bus = ~0;
-		update_clk = 1;
+		update_bus = true;
+		update_clk = true;
 	}
-	trace_dpu_perf_crtc_update(crtc->base.id,
-				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
-				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
-				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI],
-				new->core_clk_rate, stop_req,
-				update_bus, update_clk);
-
-	for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-		if (update_bus & BIT(i)) {
-			ret = _dpu_core_perf_crtc_update_bus(kms, crtc, i);
-			if (ret) {
-				DPU_ERROR("crtc-%d: failed to update bw vote for bus-%d\n",
-					  crtc->base.id, i);
-				return ret;
-			}
+
+	trace_dpu_perf_crtc_update(crtc->base.id, new->bw_ctl,
+		new->core_clk_rate, stop_req, update_bus, update_clk);
+
+	if (update_bus) {
+		ret = _dpu_core_perf_crtc_update_bus(kms, crtc);
+		if (ret) {
+			DPU_ERROR("crtc-%d: failed to update bus bw vote\n",
+				  crtc->base.id);
+			return ret;
 		}
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
index 37f518815eb7..d2097ef3d716 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
@@ -42,8 +42,8 @@ enum dpu_core_perf_data_bus_id {
  * @core_clk_rate: core clock rate request
  */
 struct dpu_core_perf_params {
-	u64 max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MAX];
-	u64 bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MAX];
+	u64 max_per_pipe_ib;
+	u64 bw_ctl;
 	u64 core_clk_rate;
 };
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index dfdfa766da8f..c4db60a8672d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1235,19 +1235,14 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
 {
 	struct drm_crtc *crtc = (struct drm_crtc *) s->private;
 	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
-	int i;
 
 	seq_printf(s, "client type: %d\n", dpu_crtc_get_client_type(crtc));
 	seq_printf(s, "intf_mode: %d\n", dpu_crtc_get_intf_mode(crtc));
 	seq_printf(s, "core_clk_rate: %llu\n",
 			dpu_crtc->cur_perf.core_clk_rate);
-	for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
-			i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
-		seq_printf(s, "bw_ctl[%d]: %llu\n", i,
-				dpu_crtc->cur_perf.bw_ctl[i]);
-		seq_printf(s, "max_per_pipe_ib[%d]: %llu\n", i,
-				dpu_crtc->cur_perf.max_per_pipe_ib[i]);
-	}
+	seq_printf(s, "bw_ctl: %llu\n", dpu_crtc->cur_perf.bw_ctl);
+	seq_printf(s, "max_per_pipe_ib: %llu\n",
+				dpu_crtc->cur_perf.max_per_pipe_ib);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 8bb46090bd16..1d68e214795e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -146,16 +146,12 @@ TRACE_EVENT(dpu_trace_counter,
 )
 
 TRACE_EVENT(dpu_perf_crtc_update,
-	TP_PROTO(u32 crtc, u64 bw_ctl_mnoc, u64 bw_ctl_llcc,
-			u64 bw_ctl_ebi, u32 core_clk_rate,
-			bool stop_req, u32 update_bus, u32 update_clk),
-	TP_ARGS(crtc, bw_ctl_mnoc, bw_ctl_llcc, bw_ctl_ebi, core_clk_rate,
-		stop_req, update_bus, update_clk),
+	TP_PROTO(u32 crtc, u64 bw_ctl, u32 core_clk_rate,
+			bool stop_req, bool update_bus, bool update_clk),
+	TP_ARGS(crtc, bw_ctl, core_clk_rate, stop_req, update_bus, update_clk),
 	TP_STRUCT__entry(
 			__field(u32, crtc)
-			__field(u64, bw_ctl_mnoc)
-			__field(u64, bw_ctl_llcc)
-			__field(u64, bw_ctl_ebi)
+			__field(u64, bw_ctl)
 			__field(u32, core_clk_rate)
 			__field(bool, stop_req)
 			__field(u32, update_bus)
@@ -163,20 +159,16 @@ TRACE_EVENT(dpu_perf_crtc_update,
 	),
 	TP_fast_assign(
 			__entry->crtc = crtc;
-			__entry->bw_ctl_mnoc = bw_ctl_mnoc;
-			__entry->bw_ctl_llcc = bw_ctl_llcc;
-			__entry->bw_ctl_ebi = bw_ctl_ebi;
+			__entry->bw_ctl = bw_ctl;
 			__entry->core_clk_rate = core_clk_rate;
 			__entry->stop_req = stop_req;
 			__entry->update_bus = update_bus;
 			__entry->update_clk = update_clk;
 	),
 	 TP_printk(
-		"crtc=%d bw_mnoc=%llu bw_llcc=%llu bw_ebi=%llu clk_rate=%u stop_req=%d u_bus=%d u_clk=%d",
+		"crtc=%d bw_ctl=%llu clk_rate=%u stop_req=%d u_bus=%d u_clk=%d",
 			__entry->crtc,
-			__entry->bw_ctl_mnoc,
-			__entry->bw_ctl_llcc,
-			__entry->bw_ctl_ebi,
+			__entry->bw_ctl,
 			__entry->core_clk_rate,
 			__entry->stop_req,
 			__entry->update_bus,
-- 
2.20.1


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

* [PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS
       [not found] <20190508204219.31687-1-robdclark@gmail.com>
  2019-05-08 20:42 ` [PATCH 1/5] drm/msm/dpu: clean up references of DPU custom bus scaling Rob Clark
@ 2019-05-08 20:42 ` Rob Clark
  2019-05-13 14:47   ` Sean Paul
  2019-05-08 20:42 ` [PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845 Rob Clark
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Rob Clark @ 2019-05-08 20:42 UTC (permalink / raw)
  To: dri-devel
  Cc: Jayant Shekhar, Sravanthi Kollukuduru, Rob Clark, Rob Clark,
	Sean Paul, David Airlie, Daniel Vetter, Jordan Crouse,
	Abhinav Kumar, Stephen Boyd, Jeykumar Sankaran, linux-arm-msm,
	freedreno, linux-kernel

From: Jayant Shekhar <jshekhar@codeaurora.org>

The interconnect framework is designed to provide a
standard kernel interface to control the settings of
the interconnects on a SoC.

The interconnect API uses a consumer/provider-based model,
where the providers are the interconnect buses and the
consumers could be various drivers.

MDSS is one of the interconnect consumers which uses the
interconnect APIs to get the path between endpoints and
set its bandwidth requirement for the given interconnected
path.

Changes in v2:
	- Remove error log and unnecessary check (Jordan Crouse)

Changes in v3:
	- Code clean involving variable name change, removal
	  of extra paranthesis and variables (Matthias Kaehlcke)

Changes in v4:
	- Add comments, spacings, tabs, proper port name
	  and icc macro (Georgi Djakov)

Changes in v5:
	- Commit text and parenthesis alignment (Georgi Djakov)

Changes in v6:
	- Change to new icc_set API's (Doug Anderson)

Changes in v7:
	- Fixed a typo

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
Signed-off-by: Jayant Shekhar <jshekhar@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 49 ++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 7316b4ab1b85..e3c56ccd7357 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -4,11 +4,15 @@
  */
 
 #include "dpu_kms.h"
+#include <linux/interconnect.h>
 
 #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
 
 #define HW_INTR_STATUS			0x0010
 
+/* Max BW defined in KBps */
+#define MAX_BW				6800000
+
 struct dpu_irq_controller {
 	unsigned long enabled_mask;
 	struct irq_domain *domain;
@@ -21,8 +25,30 @@ struct dpu_mdss {
 	u32 hwversion;
 	struct dss_module_power mp;
 	struct dpu_irq_controller irq_controller;
+	struct icc_path *path[2];
+	u32 num_paths;
 };
 
+static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev,
+						struct dpu_mdss *dpu_mdss)
+{
+	struct icc_path *path0 = of_icc_get(dev->dev, "mdp0-mem");
+	struct icc_path *path1 = of_icc_get(dev->dev, "mdp1-mem");
+
+	if (IS_ERR(path0))
+		return PTR_ERR(path0);
+
+	dpu_mdss->path[0] = path0;
+	dpu_mdss->num_paths = 1;
+
+	if (!IS_ERR(path1)) {
+		dpu_mdss->path[1] = path1;
+		dpu_mdss->num_paths++;
+	}
+
+	return 0;
+}
+
 static void dpu_mdss_irq(struct irq_desc *desc)
 {
 	struct dpu_mdss *dpu_mdss = irq_desc_get_handler_data(desc);
@@ -134,7 +160,11 @@ static int dpu_mdss_enable(struct msm_mdss *mdss)
 {
 	struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
 	struct dss_module_power *mp = &dpu_mdss->mp;
-	int ret;
+	int ret, i;
+	u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0;
+
+	for (i = 0; i < dpu_mdss->num_paths; i++)
+		icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW));
 
 	ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true);
 	if (ret)
@@ -147,12 +177,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss)
 {
 	struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
 	struct dss_module_power *mp = &dpu_mdss->mp;
-	int ret;
+	int ret, i;
 
 	ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
 	if (ret)
 		DPU_ERROR("clock disable failed, ret:%d\n", ret);
 
+	for (i = 0; i < dpu_mdss->num_paths; i++)
+		icc_set_bw(dpu_mdss->path[i], 0, 0);
+
 	return ret;
 }
 
@@ -163,6 +196,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
 	struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss);
 	struct dss_module_power *mp = &dpu_mdss->mp;
 	int irq;
+	int i;
 
 	pm_runtime_suspend(dev->dev);
 	pm_runtime_disable(dev->dev);
@@ -172,6 +206,9 @@ static void dpu_mdss_destroy(struct drm_device *dev)
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
 	devm_kfree(&pdev->dev, mp->clk_config);
 
+	for (i = 0; i < dpu_mdss->num_paths; i++)
+		icc_put(dpu_mdss->path[i]);
+
 	if (dpu_mdss->mmio)
 		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
 	dpu_mdss->mmio = NULL;
@@ -211,6 +248,10 @@ int dpu_mdss_init(struct drm_device *dev)
 	}
 	dpu_mdss->mmio_len = resource_size(res);
 
+	ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss);
+	if (ret)
+		return ret;
+
 	mp = &dpu_mdss->mp;
 	ret = msm_dss_parse_clock(pdev, mp);
 	if (ret) {
@@ -232,14 +273,14 @@ int dpu_mdss_init(struct drm_device *dev)
 	irq_set_chained_handler_and_data(irq, dpu_mdss_irq,
 					 dpu_mdss);
 
+	priv->mdss = &dpu_mdss->base;
+
 	pm_runtime_enable(dev->dev);
 
 	pm_runtime_get_sync(dev->dev);
 	dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio);
 	pm_runtime_put_sync(dev->dev);
 
-	priv->mdss = &dpu_mdss->base;
-
 	return ret;
 
 irq_error:
-- 
2.20.1


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

* [PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845
       [not found] <20190508204219.31687-1-robdclark@gmail.com>
  2019-05-08 20:42 ` [PATCH 1/5] drm/msm/dpu: clean up references of DPU custom bus scaling Rob Clark
  2019-05-08 20:42 ` [PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS Rob Clark
@ 2019-05-08 20:42 ` Rob Clark
  2019-05-13 14:48   ` Sean Paul
  2019-05-08 20:42 ` [PATCH 4/5] drm/msm/dpu: add icc voting in dpu_mdss_init Rob Clark
  2019-05-08 20:42 ` [PATCH 5/5] drm/msm/mdp5: Use the interconnect API Rob Clark
  4 siblings, 1 reply; 12+ messages in thread
From: Rob Clark @ 2019-05-08 20:42 UTC (permalink / raw)
  To: dri-devel
  Cc: Jayant Shekhar, Sravanthi Kollukuduru, Rob Herring, Rob Clark,
	Rob Clark, Sean Paul, David Airlie, Daniel Vetter, Rob Herring,
	Mark Rutland, linux-arm-msm, freedreno, devicetree, linux-kernel

From: Jayant Shekhar <jshekhar@codeaurora.org>

Add interconnect properties such as interconnect provider specifier
, the edge source and destination ports which are required by the
interconnect API to configure interconnect path for MDSS.

Changes in v2:
	- None

Changes in v3:
	- Remove common property definitions (Rob Herring)

Changes in v4:
	- Use port macros and change port string names (Georgi Djakov)

Changes in v5-v7:
	- None

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
Signed-off-by: Jayant Shekhar <jshekhar@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 Documentation/devicetree/bindings/display/msm/dpu.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
index ad2e8830324e..a61dd40f3792 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -28,6 +28,11 @@ Required properties:
 - #address-cells: number of address cells for the MDSS children. Should be 1.
 - #size-cells: Should be 1.
 - ranges: parent bus address space is the same as the child bus address space.
+- interconnects : interconnect path specifier for MDSS according to
+  Documentation/devicetree/bindings/interconnect/interconnect.txt. Should be
+  2 paths corresponding to 2 AXI ports.
+- interconnect-names : MDSS will have 2 port names to differentiate between the
+  2 interconnect paths defined with interconnect specifier.
 
 Optional properties:
 - assigned-clocks: list of clock specifiers for clocks needing rate assignment
@@ -86,6 +91,11 @@ Example:
 		interrupt-controller;
 		#interrupt-cells = <1>;
 
+		interconnects = <&rsc_hlos MASTER_MDP0 &rsc_hlos SLAVE_EBI1>,
+				<&rsc_hlos MASTER_MDP1 &rsc_hlos SLAVE_EBI1>;
+
+		interconnect-names = "mdp0-mem", "mdp1-mem";
+
 		iommus = <&apps_iommu 0>;
 
 		#address-cells = <2>;
-- 
2.20.1


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

* [PATCH 4/5] drm/msm/dpu: add icc voting in dpu_mdss_init
       [not found] <20190508204219.31687-1-robdclark@gmail.com>
                   ` (2 preceding siblings ...)
  2019-05-08 20:42 ` [PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845 Rob Clark
@ 2019-05-08 20:42 ` Rob Clark
  2019-05-08 20:42 ` [PATCH 5/5] drm/msm/mdp5: Use the interconnect API Rob Clark
  4 siblings, 0 replies; 12+ messages in thread
From: Rob Clark @ 2019-05-08 20:42 UTC (permalink / raw)
  To: dri-devel
  Cc: Abhinav Kumar, Sean Paul, Rob Clark, Rob Clark, David Airlie,
	Daniel Vetter, Jordan Crouse, Jayant Shekhar, Stephen Boyd,
	Jeykumar Sankaran, linux-arm-msm, freedreno, linux-kernel

From: Abhinav Kumar <abhinavk@codeaurora.org>

dpu_mdss_destroy() can get called not just from
msm_drm_uninit() but also from msm_drm_bind() in case
of any failures.

dpu_mdss_destroy() removes the icc voting by calling
icc_put. This could accidentally remove the voting
done by pm_runtime_enable.

To make the voting balanced add a minimum vote in
dpu_mdss_init() to avoid any unclocked access.

This change depends on the following patch which
introduces interconnect binding to MDSS driver:

https://patchwork.codeaurora.org/patch/708155/

Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
Reviewed-by: Sean Paul <sean@poorly.run>
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index e3c56ccd7357..b42537a663f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -49,6 +49,16 @@ static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev,
 	return 0;
 }
 
+static void dpu_mdss_icc_request_bw(struct msm_mdss *mdss)
+{
+	struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
+	int i;
+	u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0;
+
+	for (i = 0; i < dpu_mdss->num_paths; i++)
+		icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW));
+}
+
 static void dpu_mdss_irq(struct irq_desc *desc)
 {
 	struct dpu_mdss *dpu_mdss = irq_desc_get_handler_data(desc);
@@ -160,11 +170,9 @@ static int dpu_mdss_enable(struct msm_mdss *mdss)
 {
 	struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
 	struct dss_module_power *mp = &dpu_mdss->mp;
-	int ret, i;
-	u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0;
+	int ret;
 
-	for (i = 0; i < dpu_mdss->num_paths; i++)
-		icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW));
+	dpu_mdss_icc_request_bw(mdss);
 
 	ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true);
 	if (ret)
@@ -277,6 +285,8 @@ int dpu_mdss_init(struct drm_device *dev)
 
 	pm_runtime_enable(dev->dev);
 
+	dpu_mdss_icc_request_bw(priv->mdss);
+
 	pm_runtime_get_sync(dev->dev);
 	dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio);
 	pm_runtime_put_sync(dev->dev);
-- 
2.20.1


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

* [PATCH 5/5] drm/msm/mdp5: Use the interconnect API
       [not found] <20190508204219.31687-1-robdclark@gmail.com>
                   ` (3 preceding siblings ...)
  2019-05-08 20:42 ` [PATCH 4/5] drm/msm/dpu: add icc voting in dpu_mdss_init Rob Clark
@ 2019-05-08 20:42 ` Rob Clark
  2019-05-13 14:49   ` Sean Paul
  2019-05-29 13:08   ` Georgi Djakov
  4 siblings, 2 replies; 12+ messages in thread
From: Rob Clark @ 2019-05-08 20:42 UTC (permalink / raw)
  To: dri-devel
  Cc: Georgi Djakov, Rob Clark, Rob Clark, Sean Paul, David Airlie,
	Daniel Vetter, linux-arm-msm, freedreno, linux-kernel

From: Georgi Djakov <georgi.djakov@linaro.org>

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 97179bec8902..54d2b4c2b09f 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -16,6 +16,7 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/interconnect.h>
 #include <linux/of_irq.h>
 
 #include "msm_drv.h"
@@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
 
 static int mdp5_dev_probe(struct platform_device *pdev)
 {
+	struct icc_path *path0 = of_icc_get(&pdev->dev, "port0");
+	struct icc_path *path1 = of_icc_get(&pdev->dev, "port1");
+	struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator");
+
+	if (IS_ERR(path0))
+		return PTR_ERR(path0);
+	icc_set_bw(path0, 0, MBps_to_icc(6400));
+
+	if (!IS_ERR(path1))
+		icc_set_bw(path1, 0, MBps_to_icc(6400));
+	if (!IS_ERR(path_rot))
+		icc_set_bw(path_rot, 0, MBps_to_icc(6400));
+
 	DBG("");
 	return component_add(&pdev->dev, &mdp5_ops);
 }
-- 
2.20.1


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

* Re: [PATCH 1/5] drm/msm/dpu: clean up references of DPU custom bus scaling
  2019-05-08 20:42 ` [PATCH 1/5] drm/msm/dpu: clean up references of DPU custom bus scaling Rob Clark
@ 2019-05-13 14:42   ` Sean Paul
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Paul @ 2019-05-13 14:42 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, Jayant Shekhar, Sravanthi Kollukuduru, Rob Clark,
	Sean Paul, David Airlie, Daniel Vetter, Jeykumar Sankaran,
	Jordan Crouse, Bruce Wang, Abhinav Kumar, linux-arm-msm,
	freedreno, linux-kernel

On Wed, May 08, 2019 at 01:42:11PM -0700, Rob Clark wrote:
> From: Jayant Shekhar <jshekhar@codeaurora.org>
> 
> Since the upstream interconnect bus framework has landed
> upstream, the existing references of custom bus scaling
> needs to be cleaned up.
> 
> Changes in v2:
> 	- Fixed build error due to partial clean up
> 
> Changes in v3:
> 	- Condense multiple lines into a single line (Sean Paul)
> 
> Changes in v4-v7:
> 	- None
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
> Signed-off-by: Jayant Shekhar <jshekhar@codeaurora.org>
> Signed-off-by: Rob Clark <robdclark@chromium.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 174 +++++++-----------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |   4 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      |  11 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h     |  22 +--
>  4 files changed, 80 insertions(+), 131 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 9f20f397f77d..e231c37a9dbb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -77,7 +77,6 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
>  		struct dpu_core_perf_params *perf)
>  {
>  	struct dpu_crtc_state *dpu_cstate;
> -	int i;
>  
>  	if (!kms || !kms->catalog || !crtc || !state || !perf) {
>  		DPU_ERROR("invalid parameters\n");
> @@ -88,35 +87,24 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
>  	memset(perf, 0, sizeof(struct dpu_core_perf_params));
>  
>  	if (!dpu_cstate->bw_control) {
> -		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
> -			perf->bw_ctl[i] = kms->catalog->perf.max_bw_high *
> +		perf->bw_ctl = kms->catalog->perf.max_bw_high *
>  					1000ULL;
> -			perf->max_per_pipe_ib[i] = perf->bw_ctl[i];
> -		}
> +		perf->max_per_pipe_ib = perf->bw_ctl;
>  		perf->core_clk_rate = kms->perf.max_core_clk_rate;
>  	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
> -		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
> -			perf->bw_ctl[i] = 0;
> -			perf->max_per_pipe_ib[i] = 0;
> -		}
> +		perf->bw_ctl = 0;
> +		perf->max_per_pipe_ib = 0;
>  		perf->core_clk_rate = 0;
>  	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
> -		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
> -			perf->bw_ctl[i] = kms->perf.fix_core_ab_vote;
> -			perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote;
> -		}
> +		perf->bw_ctl = kms->perf.fix_core_ab_vote;
> +		perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote;
>  		perf->core_clk_rate = kms->perf.fix_core_clk_rate;
>  	}
>  
>  	DPU_DEBUG(
> -		"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n",
> +		"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu\n",
>  			crtc->base.id, perf->core_clk_rate,
> -			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
> -			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
> -			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
> -			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
> -			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_EBI],
> -			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI]);
> +			perf->max_per_pipe_ib, perf->bw_ctl);
>  }
>  
>  int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
> @@ -129,7 +117,6 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>  	struct dpu_crtc_state *dpu_cstate;
>  	struct drm_crtc *tmp_crtc;
>  	struct dpu_kms *kms;
> -	int i;
>  
>  	if (!crtc || !state) {
>  		DPU_ERROR("invalid crtc\n");
> @@ -151,31 +138,25 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>  	/* obtain new values */
>  	_dpu_core_perf_calc_crtc(kms, crtc, state, &dpu_cstate->new_perf);
>  
> -	for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
> -			i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
> -		bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i];
> -		curr_client_type = dpu_crtc_get_client_type(crtc);
> +	bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl;
> +	curr_client_type = dpu_crtc_get_client_type(crtc);
>  
> -		drm_for_each_crtc(tmp_crtc, crtc->dev) {
> -			if (tmp_crtc->enabled &&
> -			    (dpu_crtc_get_client_type(tmp_crtc) ==
> -					    curr_client_type) &&
> -			    (tmp_crtc != crtc)) {
> -				struct dpu_crtc_state *tmp_cstate =
> -					to_dpu_crtc_state(tmp_crtc->state);
> -
> -				DPU_DEBUG("crtc:%d bw:%llu ctrl:%d\n",
> -					tmp_crtc->base.id,
> -					tmp_cstate->new_perf.bw_ctl[i],
> -					tmp_cstate->bw_control);
> -				/*
> -				 * For bw check only use the bw if the
> -				 * atomic property has been already set
> -				 */
> -				if (tmp_cstate->bw_control)
> -					bw_sum_of_intfs +=
> -						tmp_cstate->new_perf.bw_ctl[i];
> -			}
> +	drm_for_each_crtc(tmp_crtc, crtc->dev) {
> +		if (tmp_crtc->enabled &&
> +		    (dpu_crtc_get_client_type(tmp_crtc) ==
> +				curr_client_type) && (tmp_crtc != crtc)) {
> +			struct dpu_crtc_state *tmp_cstate =
> +				to_dpu_crtc_state(tmp_crtc->state);
> +
> +			DPU_DEBUG("crtc:%d bw:%llu ctrl:%d\n",
> +				tmp_crtc->base.id, tmp_cstate->new_perf.bw_ctl,
> +				tmp_cstate->bw_control);
> +			/*
> +			 * For bw check only use the bw if the
> +			 * atomic property has been already set
> +			 */
> +			if (tmp_cstate->bw_control)
> +				bw_sum_of_intfs += tmp_cstate->new_perf.bw_ctl;
>  		}
>  
>  		/* convert bandwidth to kb */
> @@ -206,9 +187,9 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>  }
>  
>  static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
> -		struct drm_crtc *crtc, u32 bus_id)
> +		struct drm_crtc *crtc)
>  {
> -	struct dpu_core_perf_params perf = { { 0 } };
> +	struct dpu_core_perf_params perf = { 0 };
>  	enum dpu_crtc_client_type curr_client_type
>  					= dpu_crtc_get_client_type(crtc);
>  	struct drm_crtc *tmp_crtc;
> @@ -221,13 +202,11 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
>  				dpu_crtc_get_client_type(tmp_crtc)) {
>  			dpu_cstate = to_dpu_crtc_state(tmp_crtc->state);
>  
> -			perf.max_per_pipe_ib[bus_id] =
> -				max(perf.max_per_pipe_ib[bus_id],
> -				dpu_cstate->new_perf.max_per_pipe_ib[bus_id]);
> +			perf.max_per_pipe_ib = max(perf.max_per_pipe_ib,
> +					dpu_cstate->new_perf.max_per_pipe_ib);
>  
> -			DPU_DEBUG("crtc=%d bus_id=%d bw=%llu\n",
> -				tmp_crtc->base.id, bus_id,
> -				dpu_cstate->new_perf.bw_ctl[bus_id]);
> +			DPU_DEBUG("crtc=%d bw=%llu\n", tmp_crtc->base.id,
> +					dpu_cstate->new_perf.bw_ctl);
>  		}
>  	}
>  	return ret;
> @@ -247,7 +226,6 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
>  	struct dpu_crtc *dpu_crtc;
>  	struct dpu_crtc_state *dpu_cstate;
>  	struct dpu_kms *kms;
> -	int i;
>  
>  	if (!crtc) {
>  		DPU_ERROR("invalid crtc\n");
> @@ -283,10 +261,8 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
>  	if (kms->perf.enable_bw_release) {
>  		trace_dpu_cmd_release_bw(crtc->base.id);
>  		DPU_DEBUG("Release BW crtc=%d\n", crtc->base.id);
> -		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
> -			dpu_crtc->cur_perf.bw_ctl[i] = 0;
> -			_dpu_core_perf_crtc_update_bus(kms, crtc, i);
> -		}
> +		dpu_crtc->cur_perf.bw_ctl = 0;
> +		_dpu_core_perf_crtc_update_bus(kms, crtc);
>  	}
>  }
>  
> @@ -329,11 +305,10 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>  		int params_changed, bool stop_req)
>  {
>  	struct dpu_core_perf_params *new, *old;
> -	int update_bus = 0, update_clk = 0;
> +	bool update_bus = false, update_clk = false;
>  	u64 clk_rate = 0;
>  	struct dpu_crtc *dpu_crtc;
>  	struct dpu_crtc_state *dpu_cstate;
> -	int i;
>  	struct msm_drm_private *priv;
>  	struct dpu_kms *kms;
>  	int ret;
> @@ -360,62 +335,49 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>  	new = &dpu_cstate->new_perf;
>  
>  	if (crtc->enabled && !stop_req) {
> -		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
> -			/*
> -			 * cases for bus bandwidth update.
> -			 * 1. new bandwidth vote - "ab or ib vote" is higher
> -			 *    than current vote for update request.
> -			 * 2. new bandwidth vote - "ab or ib vote" is lower
> -			 *    than current vote at end of commit or stop.
> -			 */
> -			if ((params_changed && ((new->bw_ctl[i] >
> -						old->bw_ctl[i]) ||
> -				  (new->max_per_pipe_ib[i] >
> -						old->max_per_pipe_ib[i]))) ||
> -			    (!params_changed && ((new->bw_ctl[i] <
> -						old->bw_ctl[i]) ||
> -				  (new->max_per_pipe_ib[i] <
> -						old->max_per_pipe_ib[i])))) {
> -				DPU_DEBUG(
> -					"crtc=%d p=%d new_bw=%llu,old_bw=%llu\n",
> -					crtc->base.id, params_changed,
> -					new->bw_ctl[i], old->bw_ctl[i]);
> -				old->bw_ctl[i] = new->bw_ctl[i];
> -				old->max_per_pipe_ib[i] =
> -						new->max_per_pipe_ib[i];
> -				update_bus |= BIT(i);
> -			}
> +		/*
> +		 * cases for bus bandwidth update.
> +		 * 1. new bandwidth vote - "ab or ib vote" is higher
> +		 *    than current vote for update request.
> +		 * 2. new bandwidth vote - "ab or ib vote" is lower
> +		 *    than current vote at end of commit or stop.
> +		 */
> +		if ((params_changed && ((new->bw_ctl > old->bw_ctl) ||
> +			(new->max_per_pipe_ib > old->max_per_pipe_ib)))	||
> +			(!params_changed && ((new->bw_ctl < old->bw_ctl) ||
> +			(new->max_per_pipe_ib < old->max_per_pipe_ib)))) {
> +			DPU_DEBUG("crtc=%d p=%d new_bw=%llu,old_bw=%llu\n",
> +				crtc->base.id, params_changed,
> +				new->bw_ctl, old->bw_ctl);
> +			old->bw_ctl = new->bw_ctl;
> +			old->max_per_pipe_ib = new->max_per_pipe_ib;
> +			update_bus = true;
>  		}
>  
>  		if ((params_changed &&
> -				(new->core_clk_rate > old->core_clk_rate)) ||
> -				(!params_changed &&
> -				(new->core_clk_rate < old->core_clk_rate))) {
> +			(new->core_clk_rate > old->core_clk_rate)) ||
> +			(!params_changed &&
> +			(new->core_clk_rate < old->core_clk_rate))) {
>  			old->core_clk_rate = new->core_clk_rate;
> -			update_clk = 1;
> +			update_clk = true;
>  		}
>  	} else {
>  		DPU_DEBUG("crtc=%d disable\n", crtc->base.id);
>  		memset(old, 0, sizeof(*old));
>  		memset(new, 0, sizeof(*new));
> -		update_bus = ~0;
> -		update_clk = 1;
> +		update_bus = true;
> +		update_clk = true;
>  	}
> -	trace_dpu_perf_crtc_update(crtc->base.id,
> -				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
> -				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
> -				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI],
> -				new->core_clk_rate, stop_req,
> -				update_bus, update_clk);
> -
> -	for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
> -		if (update_bus & BIT(i)) {
> -			ret = _dpu_core_perf_crtc_update_bus(kms, crtc, i);
> -			if (ret) {
> -				DPU_ERROR("crtc-%d: failed to update bw vote for bus-%d\n",
> -					  crtc->base.id, i);
> -				return ret;
> -			}
> +
> +	trace_dpu_perf_crtc_update(crtc->base.id, new->bw_ctl,
> +		new->core_clk_rate, stop_req, update_bus, update_clk);
> +
> +	if (update_bus) {
> +		ret = _dpu_core_perf_crtc_update_bus(kms, crtc);
> +		if (ret) {
> +			DPU_ERROR("crtc-%d: failed to update bus bw vote\n",
> +				  crtc->base.id);
> +			return ret;
>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> index 37f518815eb7..d2097ef3d716 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> @@ -42,8 +42,8 @@ enum dpu_core_perf_data_bus_id {
>   * @core_clk_rate: core clock rate request
>   */
>  struct dpu_core_perf_params {
> -	u64 max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MAX];
> -	u64 bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MAX];
> +	u64 max_per_pipe_ib;
> +	u64 bw_ctl;
>  	u64 core_clk_rate;
>  };
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index dfdfa766da8f..c4db60a8672d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1235,19 +1235,14 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
>  {
>  	struct drm_crtc *crtc = (struct drm_crtc *) s->private;
>  	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> -	int i;
>  
>  	seq_printf(s, "client type: %d\n", dpu_crtc_get_client_type(crtc));
>  	seq_printf(s, "intf_mode: %d\n", dpu_crtc_get_intf_mode(crtc));
>  	seq_printf(s, "core_clk_rate: %llu\n",
>  			dpu_crtc->cur_perf.core_clk_rate);
> -	for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
> -			i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
> -		seq_printf(s, "bw_ctl[%d]: %llu\n", i,
> -				dpu_crtc->cur_perf.bw_ctl[i]);
> -		seq_printf(s, "max_per_pipe_ib[%d]: %llu\n", i,
> -				dpu_crtc->cur_perf.max_per_pipe_ib[i]);
> -	}
> +	seq_printf(s, "bw_ctl: %llu\n", dpu_crtc->cur_perf.bw_ctl);
> +	seq_printf(s, "max_per_pipe_ib: %llu\n",
> +				dpu_crtc->cur_perf.max_per_pipe_ib);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> index 8bb46090bd16..1d68e214795e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> @@ -146,16 +146,12 @@ TRACE_EVENT(dpu_trace_counter,
>  )
>  
>  TRACE_EVENT(dpu_perf_crtc_update,
> -	TP_PROTO(u32 crtc, u64 bw_ctl_mnoc, u64 bw_ctl_llcc,
> -			u64 bw_ctl_ebi, u32 core_clk_rate,
> -			bool stop_req, u32 update_bus, u32 update_clk),
> -	TP_ARGS(crtc, bw_ctl_mnoc, bw_ctl_llcc, bw_ctl_ebi, core_clk_rate,
> -		stop_req, update_bus, update_clk),
> +	TP_PROTO(u32 crtc, u64 bw_ctl, u32 core_clk_rate,
> +			bool stop_req, bool update_bus, bool update_clk),
> +	TP_ARGS(crtc, bw_ctl, core_clk_rate, stop_req, update_bus, update_clk),
>  	TP_STRUCT__entry(
>  			__field(u32, crtc)
> -			__field(u64, bw_ctl_mnoc)
> -			__field(u64, bw_ctl_llcc)
> -			__field(u64, bw_ctl_ebi)
> +			__field(u64, bw_ctl)
>  			__field(u32, core_clk_rate)
>  			__field(bool, stop_req)
>  			__field(u32, update_bus)
> @@ -163,20 +159,16 @@ TRACE_EVENT(dpu_perf_crtc_update,
>  	),
>  	TP_fast_assign(
>  			__entry->crtc = crtc;
> -			__entry->bw_ctl_mnoc = bw_ctl_mnoc;
> -			__entry->bw_ctl_llcc = bw_ctl_llcc;
> -			__entry->bw_ctl_ebi = bw_ctl_ebi;
> +			__entry->bw_ctl = bw_ctl;
>  			__entry->core_clk_rate = core_clk_rate;
>  			__entry->stop_req = stop_req;
>  			__entry->update_bus = update_bus;
>  			__entry->update_clk = update_clk;
>  	),
>  	 TP_printk(
> -		"crtc=%d bw_mnoc=%llu bw_llcc=%llu bw_ebi=%llu clk_rate=%u stop_req=%d u_bus=%d u_clk=%d",
> +		"crtc=%d bw_ctl=%llu clk_rate=%u stop_req=%d u_bus=%d u_clk=%d",
>  			__entry->crtc,
> -			__entry->bw_ctl_mnoc,
> -			__entry->bw_ctl_llcc,
> -			__entry->bw_ctl_ebi,
> +			__entry->bw_ctl,
>  			__entry->core_clk_rate,
>  			__entry->stop_req,
>  			__entry->update_bus,
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

* Re: [PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS
  2019-05-08 20:42 ` [PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS Rob Clark
@ 2019-05-13 14:47   ` Sean Paul
  2019-05-29 13:05     ` Georgi Djakov
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Paul @ 2019-05-13 14:47 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, Jayant Shekhar, Sravanthi Kollukuduru, Rob Clark,
	Sean Paul, David Airlie, Daniel Vetter, Jordan Crouse,
	Abhinav Kumar, Stephen Boyd, Jeykumar Sankaran, linux-arm-msm,
	freedreno, linux-kernel

On Wed, May 08, 2019 at 01:42:12PM -0700, Rob Clark wrote:
> From: Jayant Shekhar <jshekhar@codeaurora.org>
> 
> The interconnect framework is designed to provide a
> standard kernel interface to control the settings of
> the interconnects on a SoC.
> 
> The interconnect API uses a consumer/provider-based model,
> where the providers are the interconnect buses and the
> consumers could be various drivers.
> 
> MDSS is one of the interconnect consumers which uses the
> interconnect APIs to get the path between endpoints and
> set its bandwidth requirement for the given interconnected
> path.
> 
> Changes in v2:
> 	- Remove error log and unnecessary check (Jordan Crouse)
> 
> Changes in v3:
> 	- Code clean involving variable name change, removal
> 	  of extra paranthesis and variables (Matthias Kaehlcke)
> 
> Changes in v4:
> 	- Add comments, spacings, tabs, proper port name
> 	  and icc macro (Georgi Djakov)
> 
> Changes in v5:
> 	- Commit text and parenthesis alignment (Georgi Djakov)
> 
> Changes in v6:
> 	- Change to new icc_set API's (Doug Anderson)
> 
> Changes in v7:
> 	- Fixed a typo
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
> Signed-off-by: Jayant Shekhar <jshekhar@codeaurora.org>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 49 ++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> index 7316b4ab1b85..e3c56ccd7357 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> @@ -4,11 +4,15 @@
>   */
>  
>  #include "dpu_kms.h"
> +#include <linux/interconnect.h>
>  
>  #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
>  
>  #define HW_INTR_STATUS			0x0010
>  
> +/* Max BW defined in KBps */
> +#define MAX_BW				6800000
> +
>  struct dpu_irq_controller {
>  	unsigned long enabled_mask;
>  	struct irq_domain *domain;
> @@ -21,8 +25,30 @@ struct dpu_mdss {
>  	u32 hwversion;
>  	struct dss_module_power mp;
>  	struct dpu_irq_controller irq_controller;
> +	struct icc_path *path[2];
> +	u32 num_paths;
>  };
>  
> +static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev,
> +						struct dpu_mdss *dpu_mdss)
> +{
> +	struct icc_path *path0 = of_icc_get(dev->dev, "mdp0-mem");
> +	struct icc_path *path1 = of_icc_get(dev->dev, "mdp1-mem");
> +
> +	if (IS_ERR(path0))

of_icc_get can also return NULL, it looks like we also want to guard against
this case and keep num_paths == 0.

> +		return PTR_ERR(path0);
> +
> +	dpu_mdss->path[0] = path0;
> +	dpu_mdss->num_paths = 1;
> +
> +	if (!IS_ERR(path1)) {
> +		dpu_mdss->path[1] = path1;
> +		dpu_mdss->num_paths++;
> +	}
> +
> +	return 0;
> +}
> +
>  static void dpu_mdss_irq(struct irq_desc *desc)
>  {
>  	struct dpu_mdss *dpu_mdss = irq_desc_get_handler_data(desc);
> @@ -134,7 +160,11 @@ static int dpu_mdss_enable(struct msm_mdss *mdss)
>  {
>  	struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
>  	struct dss_module_power *mp = &dpu_mdss->mp;
> -	int ret;
> +	int ret, i;
> +	u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0;
> +
> +	for (i = 0; i < dpu_mdss->num_paths; i++)
> +		icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW));
>  
>  	ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true);
>  	if (ret)
> @@ -147,12 +177,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss)
>  {
>  	struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
>  	struct dss_module_power *mp = &dpu_mdss->mp;
> -	int ret;
> +	int ret, i;
>  
>  	ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
>  	if (ret)
>  		DPU_ERROR("clock disable failed, ret:%d\n", ret);
>  
> +	for (i = 0; i < dpu_mdss->num_paths; i++)
> +		icc_set_bw(dpu_mdss->path[i], 0, 0);
> +
>  	return ret;
>  }
>  
> @@ -163,6 +196,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
>  	struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss);
>  	struct dss_module_power *mp = &dpu_mdss->mp;
>  	int irq;
> +	int i;
>  
>  	pm_runtime_suspend(dev->dev);
>  	pm_runtime_disable(dev->dev);
> @@ -172,6 +206,9 @@ static void dpu_mdss_destroy(struct drm_device *dev)
>  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
>  	devm_kfree(&pdev->dev, mp->clk_config);
>  
> +	for (i = 0; i < dpu_mdss->num_paths; i++)
> +		icc_put(dpu_mdss->path[i]);
> +
>  	if (dpu_mdss->mmio)
>  		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
>  	dpu_mdss->mmio = NULL;
> @@ -211,6 +248,10 @@ int dpu_mdss_init(struct drm_device *dev)
>  	}
>  	dpu_mdss->mmio_len = resource_size(res);
>  
> +	ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss);
> +	if (ret)
> +		return ret;
> +
>  	mp = &dpu_mdss->mp;
>  	ret = msm_dss_parse_clock(pdev, mp);
>  	if (ret) {
> @@ -232,14 +273,14 @@ int dpu_mdss_init(struct drm_device *dev)
>  	irq_set_chained_handler_and_data(irq, dpu_mdss_irq,
>  					 dpu_mdss);
>  
> +	priv->mdss = &dpu_mdss->base;
> +
>  	pm_runtime_enable(dev->dev);
>  
>  	pm_runtime_get_sync(dev->dev);
>  	dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio);
>  	pm_runtime_put_sync(dev->dev);
>  
> -	priv->mdss = &dpu_mdss->base;
> -
>  	return ret;
>  
>  irq_error:
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

* Re: [PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845
  2019-05-08 20:42 ` [PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845 Rob Clark
@ 2019-05-13 14:48   ` Sean Paul
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Paul @ 2019-05-13 14:48 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, Jayant Shekhar, Sravanthi Kollukuduru, Rob Herring,
	Rob Clark, Sean Paul, David Airlie, Daniel Vetter, Rob Herring,
	Mark Rutland, linux-arm-msm, freedreno, devicetree, linux-kernel

On Wed, May 08, 2019 at 01:42:13PM -0700, Rob Clark wrote:
> From: Jayant Shekhar <jshekhar@codeaurora.org>
> 
> Add interconnect properties such as interconnect provider specifier
> , the edge source and destination ports which are required by the
> interconnect API to configure interconnect path for MDSS.
> 
> Changes in v2:
> 	- None
> 
> Changes in v3:
> 	- Remove common property definitions (Rob Herring)
> 
> Changes in v4:
> 	- Use port macros and change port string names (Georgi Djakov)
> 
> Changes in v5-v7:
> 	- None
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
> Signed-off-by: Jayant Shekhar <jshekhar@codeaurora.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  Documentation/devicetree/bindings/display/msm/dpu.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
> index ad2e8830324e..a61dd40f3792 100644
> --- a/Documentation/devicetree/bindings/display/msm/dpu.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
> @@ -28,6 +28,11 @@ Required properties:
>  - #address-cells: number of address cells for the MDSS children. Should be 1.
>  - #size-cells: Should be 1.
>  - ranges: parent bus address space is the same as the child bus address space.
> +- interconnects : interconnect path specifier for MDSS according to
> +  Documentation/devicetree/bindings/interconnect/interconnect.txt. Should be
> +  2 paths corresponding to 2 AXI ports.
> +- interconnect-names : MDSS will have 2 port names to differentiate between the
> +  2 interconnect paths defined with interconnect specifier.
>  
>  Optional properties:
>  - assigned-clocks: list of clock specifiers for clocks needing rate assignment
> @@ -86,6 +91,11 @@ Example:
>  		interrupt-controller;
>  		#interrupt-cells = <1>;
>  
> +		interconnects = <&rsc_hlos MASTER_MDP0 &rsc_hlos SLAVE_EBI1>,
> +				<&rsc_hlos MASTER_MDP1 &rsc_hlos SLAVE_EBI1>;
> +
> +		interconnect-names = "mdp0-mem", "mdp1-mem";
> +
>  		iommus = <&apps_iommu 0>;
>  
>  		#address-cells = <2>;
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

* Re: [PATCH 5/5] drm/msm/mdp5: Use the interconnect API
  2019-05-08 20:42 ` [PATCH 5/5] drm/msm/mdp5: Use the interconnect API Rob Clark
@ 2019-05-13 14:49   ` Sean Paul
  2019-05-29 13:08   ` Georgi Djakov
  1 sibling, 0 replies; 12+ messages in thread
From: Sean Paul @ 2019-05-13 14:49 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, Georgi Djakov, Rob Clark, Sean Paul, David Airlie,
	Daniel Vetter, linux-arm-msm, freedreno, linux-kernel

On Wed, May 08, 2019 at 01:42:15PM -0700, Rob Clark wrote:
> From: Georgi Djakov <georgi.djakov@linaro.org>
> 
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 97179bec8902..54d2b4c2b09f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -16,6 +16,7 @@
>   * this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/interconnect.h>
>  #include <linux/of_irq.h>
>  
>  #include "msm_drv.h"
> @@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
>  
>  static int mdp5_dev_probe(struct platform_device *pdev)
>  {
> +	struct icc_path *path0 = of_icc_get(&pdev->dev, "port0");
> +	struct icc_path *path1 = of_icc_get(&pdev->dev, "port1");
> +	struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator");
> +
> +	if (IS_ERR(path0))

Same comments here re: NULL value

> +		return PTR_ERR(path0);
> +	icc_set_bw(path0, 0, MBps_to_icc(6400));
> +
> +	if (!IS_ERR(path1))
> +		icc_set_bw(path1, 0, MBps_to_icc(6400));
> +	if (!IS_ERR(path_rot))
> +		icc_set_bw(path_rot, 0, MBps_to_icc(6400));
> +
>  	DBG("");
>  	return component_add(&pdev->dev, &mdp5_ops);
>  }
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

* Re: [PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS
  2019-05-13 14:47   ` Sean Paul
@ 2019-05-29 13:05     ` Georgi Djakov
  0 siblings, 0 replies; 12+ messages in thread
From: Georgi Djakov @ 2019-05-29 13:05 UTC (permalink / raw)
  To: Sean Paul, Rob Clark
  Cc: dri-devel, Jayant Shekhar, Sravanthi Kollukuduru, Rob Clark,
	David Airlie, Daniel Vetter, Jordan Crouse, Abhinav Kumar,
	Stephen Boyd, Jeykumar Sankaran, linux-arm-msm, freedreno,
	linux-kernel

On 5/13/19 17:47, Sean Paul wrote:
> On Wed, May 08, 2019 at 01:42:12PM -0700, Rob Clark wrote:
>> From: Jayant Shekhar <jshekhar@codeaurora.org>
>>
>> The interconnect framework is designed to provide a
>> standard kernel interface to control the settings of
>> the interconnects on a SoC.
>>
>> The interconnect API uses a consumer/provider-based model,
>> where the providers are the interconnect buses and the
>> consumers could be various drivers.
>>
>> MDSS is one of the interconnect consumers which uses the
>> interconnect APIs to get the path between endpoints and
>> set its bandwidth requirement for the given interconnected
>> path.
>>
>> Changes in v2:
>> 	- Remove error log and unnecessary check (Jordan Crouse)
>>
>> Changes in v3:
>> 	- Code clean involving variable name change, removal
>> 	  of extra paranthesis and variables (Matthias Kaehlcke)
>>
>> Changes in v4:
>> 	- Add comments, spacings, tabs, proper port name
>> 	  and icc macro (Georgi Djakov)
>>
>> Changes in v5:
>> 	- Commit text and parenthesis alignment (Georgi Djakov)
>>
>> Changes in v6:
>> 	- Change to new icc_set API's (Doug Anderson)
>>
>> Changes in v7:
>> 	- Fixed a typo
>>
>> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
>> Signed-off-by: Jayant Shekhar <jshekhar@codeaurora.org>
>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 49 ++++++++++++++++++++++--
>>  1 file changed, 45 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
>> index 7316b4ab1b85..e3c56ccd7357 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
>> @@ -4,11 +4,15 @@
>>   */
>>  
>>  #include "dpu_kms.h"
>> +#include <linux/interconnect.h>
>>  
>>  #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
>>  
>>  #define HW_INTR_STATUS			0x0010
>>  
>> +/* Max BW defined in KBps */
>> +#define MAX_BW				6800000
>> +
>>  struct dpu_irq_controller {
>>  	unsigned long enabled_mask;
>>  	struct irq_domain *domain;
>> @@ -21,8 +25,30 @@ struct dpu_mdss {
>>  	u32 hwversion;
>>  	struct dss_module_power mp;
>>  	struct dpu_irq_controller irq_controller;
>> +	struct icc_path *path[2];
>> +	u32 num_paths;
>>  };
>>  
>> +static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev,
>> +						struct dpu_mdss *dpu_mdss)
>> +{
>> +	struct icc_path *path0 = of_icc_get(dev->dev, "mdp0-mem");
>> +	struct icc_path *path1 = of_icc_get(dev->dev, "mdp1-mem");
>> +
>> +	if (IS_ERR(path0))
> 
> of_icc_get can also return NULL, it looks like we also want to guard against
> this case and keep num_paths == 0.

of_icc_get() returns NULL when the interconnect API is not enabled or the DT
properties are not present. In this case, passing a NULL path to the icc
functions is just a nop. So it should be fine either way.

Acked-by: Georgi Djakov <georgi.djakov@linaro.org>

Thanks,
Georgi

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

* Re: [PATCH 5/5] drm/msm/mdp5: Use the interconnect API
  2019-05-08 20:42 ` [PATCH 5/5] drm/msm/mdp5: Use the interconnect API Rob Clark
  2019-05-13 14:49   ` Sean Paul
@ 2019-05-29 13:08   ` Georgi Djakov
  1 sibling, 0 replies; 12+ messages in thread
From: Georgi Djakov @ 2019-05-29 13:08 UTC (permalink / raw)
  To: Rob Clark, dri-devel
  Cc: Rob Clark, Sean Paul, David Airlie, Daniel Vetter, linux-arm-msm,
	freedreno, linux-kernel

On 5/8/19 23:42, Rob Clark wrote:
> From: Georgi Djakov <georgi.djakov@linaro.org>
> 

Let's put some text in the commit message:

The interconnect API provides an interface for consumer drivers to express
their bandwidth needs in the SoC. This data is aggregated and the on-chip
interconnect hardware is configured to the most appropriate power/performance
profile.

Use the API to configure the interconnects and request bandwidth between DDR and
the display hardware (MDP port(s) and rotator downscaler).


> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 97179bec8902..54d2b4c2b09f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -16,6 +16,7 @@
>   * this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/interconnect.h>
>  #include <linux/of_irq.h>
>  
>  #include "msm_drv.h"
> @@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
>  
>  static int mdp5_dev_probe(struct platform_device *pdev)
>  {
> +	struct icc_path *path0 = of_icc_get(&pdev->dev, "port0");
> +	struct icc_path *path1 = of_icc_get(&pdev->dev, "port1");
> +	struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator");

It would be better change just the names to "mdp0-mem', "mdp1-mem",
"rotator-mem" for consistency and denote that the path target is the DDR memory.

> +
> +	if (IS_ERR(path0))
> +		return PTR_ERR(path0);
> +	icc_set_bw(path0, 0, MBps_to_icc(6400));
> +
> +	if (!IS_ERR(path1))
> +		icc_set_bw(path1, 0, MBps_to_icc(6400));
> +	if (!IS_ERR(path_rot))
> +		icc_set_bw(path_rot, 0, MBps_to_icc(6400));
> +
>  	DBG("");
>  	return component_add(&pdev->dev, &mdp5_ops);
>  }
> 

Thanks,
Georgi

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

* [PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845
  2019-06-18 20:24 [PATCH 0/5] drm/msm: mdp5+dpu interconnect support Rob Clark
@ 2019-06-18 20:24 ` Rob Clark
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Clark @ 2019-06-18 20:24 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-arm-msm, Sean Paul, Georgi Djakov, Jayant Shekhar,
	Sravanthi Kollukuduru, Rob Herring, Rob Clark, Rob Clark,
	Sean Paul, David Airlie, Daniel Vetter, Rob Herring,
	Mark Rutland, freedreno, devicetree, linux-kernel

From: Jayant Shekhar <jshekhar@codeaurora.org>

Add interconnect properties such as interconnect provider specifier
, the edge source and destination ports which are required by the
interconnect API to configure interconnect path for MDSS.

Changes in v2:
	- None

Changes in v3:
	- Remove common property definitions (Rob Herring)

Changes in v4:
	- Use port macros and change port string names (Georgi Djakov)

Changes in v5-v7:
	- None

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
Signed-off-by: Jayant Shekhar <jshekhar@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 Documentation/devicetree/bindings/display/msm/dpu.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
index ad2e8830324e..a61dd40f3792 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -28,6 +28,11 @@ Required properties:
 - #address-cells: number of address cells for the MDSS children. Should be 1.
 - #size-cells: Should be 1.
 - ranges: parent bus address space is the same as the child bus address space.
+- interconnects : interconnect path specifier for MDSS according to
+  Documentation/devicetree/bindings/interconnect/interconnect.txt. Should be
+  2 paths corresponding to 2 AXI ports.
+- interconnect-names : MDSS will have 2 port names to differentiate between the
+  2 interconnect paths defined with interconnect specifier.
 
 Optional properties:
 - assigned-clocks: list of clock specifiers for clocks needing rate assignment
@@ -86,6 +91,11 @@ Example:
 		interrupt-controller;
 		#interrupt-cells = <1>;
 
+		interconnects = <&rsc_hlos MASTER_MDP0 &rsc_hlos SLAVE_EBI1>,
+				<&rsc_hlos MASTER_MDP1 &rsc_hlos SLAVE_EBI1>;
+
+		interconnect-names = "mdp0-mem", "mdp1-mem";
+
 		iommus = <&apps_iommu 0>;
 
 		#address-cells = <2>;
-- 
2.20.1


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

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190508204219.31687-1-robdclark@gmail.com>
2019-05-08 20:42 ` [PATCH 1/5] drm/msm/dpu: clean up references of DPU custom bus scaling Rob Clark
2019-05-13 14:42   ` Sean Paul
2019-05-08 20:42 ` [PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS Rob Clark
2019-05-13 14:47   ` Sean Paul
2019-05-29 13:05     ` Georgi Djakov
2019-05-08 20:42 ` [PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845 Rob Clark
2019-05-13 14:48   ` Sean Paul
2019-05-08 20:42 ` [PATCH 4/5] drm/msm/dpu: add icc voting in dpu_mdss_init Rob Clark
2019-05-08 20:42 ` [PATCH 5/5] drm/msm/mdp5: Use the interconnect API Rob Clark
2019-05-13 14:49   ` Sean Paul
2019-05-29 13:08   ` Georgi Djakov
2019-06-18 20:24 [PATCH 0/5] drm/msm: mdp5+dpu interconnect support Rob Clark
2019-06-18 20:24 ` [PATCH 3/5] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845 Rob Clark

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git