[PATCH v2 0/3] Use interconnect API in MDSS on SDM845

[PATCH v2 0/3] Use interconnect API in MDSS on SDM845

[Sravanthi Kollukuduru]

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 appropriate power/performance
profile.

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

Subsequently, there is a clean up patch to remove all the references
of the DPU custom bus scaling.

There is corresponding DT patch with the source and destination ports
defined for display driver which will be sent separately.

Changes in v2: 
	- Remove error log and unnecessary check (Jordan Crouse)
	- Fixed build error due to partial clean up

Sravanthi Kollukuduru (3):
  drm/msm/dpu: clean up references of DPU custom bus scaling
  drm/msm/dpu: Integrate interconnect API in MDSS
  dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on
    SDM845

 .../devicetree/bindings/display/msm/dpu.txt        |   8 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c      | 157 +++++++++------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h      |   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c           |  13 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c           |  50 ++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c   |  47 ++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h   |  68 ---------
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h          |  21 +--
 8 files changed, 140 insertions(+), 228 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 1/3] drm/msm/dpu: clean up references of DPU custom bus scaling

[Sravanthi Kollukuduru]

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

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c    | 157 ++++++++++-------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h    |   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c         |  13 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c |  47 ++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h |  68 ----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h        |  21 +--
 6 files changed, 86 insertions(+), 224 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 41c5191f9056..4ee6f0dd14f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -90,7 +90,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");
@@ -101,35 +100,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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_ID_MNOC],
-			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
-			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_LLCC],
-			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
-			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_EBI],
-			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI]);
+			perf->max_per_pipe_ib, perf->bw_ctl);
 }
 
 int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
@@ -142,7 +130,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");
@@ -164,31 +151,28 @@ 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_POWER_HANDLE_DBUS_ID_MNOC;
-			i < DPU_POWER_HANDLE_DBUS_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 (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
-			    (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 (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
+		    (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 */
@@ -219,9 +203,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;
@@ -234,13 +218,12 @@ 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;
@@ -260,7 +243,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");
@@ -296,10 +278,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_POWER_HANDLE_DBUS_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);
 	}
 }
 
@@ -342,11 +322,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;
@@ -373,7 +352,6 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
 	new = &dpu_cstate->new_perf;
 
 	if (_dpu_core_perf_crtc_is_power_on(crtc) && !stop_req) {
-		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
 			/*
 			 * cases for bus bandwidth update.
 			 * 1. new bandwidth vote - "ab or ib vote" is higher
@@ -381,23 +359,22 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
 			 * 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])))) {
+			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[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);
-			}
+					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 &&
@@ -405,30 +382,26 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
 				(!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_POWER_HANDLE_DBUS_ID_MNOC],
-				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
-				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI],
-				new->core_clk_rate, stop_req,
-				update_bus, update_clk);
-
-	for (i = 0; i < DPU_POWER_HANDLE_DBUS_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;
-			}
+				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 fbcbe0c7527a..0fb276929dc5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
@@ -30,8 +30,8 @@
  * @core_clk_rate: core clock rate request
  */
 struct dpu_core_perf_params {
-	u64 max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MAX];
-	u64 bw_ctl[DPU_POWER_HANDLE_DBUS_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 c5ba753d441c..841476d77f5c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1373,21 +1373,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_POWER_HANDLE_DBUS_ID_MNOC;
-			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
-		seq_printf(s, "bw_ctl[%s]: %llu\n",
-				dpu_power_handle_get_dbus_name(i),
-				dpu_crtc->cur_perf.bw_ctl[i]);
-		seq_printf(s, "max_per_pipe_ib[%s]: %llu\n",
-				dpu_power_handle_get_dbus_name(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_power_handle.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
index fc14116789f2..d58d01544731 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
@@ -24,20 +24,6 @@
 #include "dpu_power_handle.h"
 #include "dpu_trace.h"
 
-static const char *data_bus_name[DPU_POWER_HANDLE_DBUS_ID_MAX] = {
-	[DPU_POWER_HANDLE_DBUS_ID_MNOC] = "qcom,dpu-data-bus",
-	[DPU_POWER_HANDLE_DBUS_ID_LLCC] = "qcom,dpu-llcc-bus",
-	[DPU_POWER_HANDLE_DBUS_ID_EBI] = "qcom,dpu-ebi-bus",
-};
-
-const char *dpu_power_handle_get_dbus_name(u32 bus_id)
-{
-	if (bus_id < DPU_POWER_HANDLE_DBUS_ID_MAX)
-		return data_bus_name[bus_id];
-
-	return NULL;
-}
-
 static void dpu_power_event_trigger_locked(struct dpu_power_handle *phandle,
 		u32 event_type)
 {
@@ -66,7 +52,6 @@ struct dpu_power_client *dpu_power_client_create(
 
 	mutex_lock(&phandle->phandle_lock);
 	strlcpy(client->name, client_name, MAX_CLIENT_NAME_LEN);
-	client->usecase_ndx = VOTE_INDEX_DISABLE;
 	client->id = id;
 	client->active = true;
 	pr_debug("client %s created:%pK id :%d\n", client_name,
@@ -143,8 +128,6 @@ int dpu_power_resource_enable(struct dpu_power_handle *phandle,
 	struct dpu_power_client *pclient, bool enable)
 {
 	bool changed = false;
-	u32 max_usecase_ndx = VOTE_INDEX_DISABLE, prev_usecase_ndx;
-	struct dpu_power_client *client;
 	u32 event_type;
 
 	if (!phandle || !pclient) {
@@ -153,31 +136,19 @@ int dpu_power_resource_enable(struct dpu_power_handle *phandle,
 	}
 
 	mutex_lock(&phandle->phandle_lock);
-	if (enable)
+	if (enable) {
 		pclient->refcount++;
-	else if (pclient->refcount)
+		if (pclient->refcount == 1)
+			changed = true;
+	} else if (pclient->refcount) {
 		pclient->refcount--;
-
-	if (pclient->refcount)
-		pclient->usecase_ndx = VOTE_INDEX_LOW;
-	else
-		pclient->usecase_ndx = VOTE_INDEX_DISABLE;
-
-	list_for_each_entry(client, &phandle->power_client_clist, list) {
-		if (client->usecase_ndx < VOTE_INDEX_MAX &&
-		    client->usecase_ndx > max_usecase_ndx)
-			max_usecase_ndx = client->usecase_ndx;
-	}
-
-	if (phandle->current_usecase_ndx != max_usecase_ndx) {
-		changed = true;
-		prev_usecase_ndx = phandle->current_usecase_ndx;
-		phandle->current_usecase_ndx = max_usecase_ndx;
+		if (!pclient->refcount)
+			changed = true;
 	}
 
-	pr_debug("%pS: changed=%d current idx=%d request client %s id:%u enable:%d refcount:%d\n",
-		__builtin_return_address(0), changed, max_usecase_ndx,
-		pclient->name, pclient->id, enable, pclient->refcount);
+	pr_debug("%pS:changed=%d client %s id:%u enable:%d refcount:%d\n",
+		__builtin_return_address(0), changed, pclient->name,
+		pclient->id, enable, pclient->refcount);
 
 	if (!changed)
 		goto end;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
index a65b7a297f21..b1a5ae4d6f4e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
@@ -16,76 +16,28 @@
 
 #define MAX_CLIENT_NAME_LEN 128
 
-#define DPU_POWER_HANDLE_ENABLE_BUS_AB_QUOTA	0
-#define DPU_POWER_HANDLE_DISABLE_BUS_AB_QUOTA	0
-#define DPU_POWER_HANDLE_ENABLE_BUS_IB_QUOTA	1600000000
-#define DPU_POWER_HANDLE_DISABLE_BUS_IB_QUOTA	0
-
 #include "dpu_io_util.h"
 
 /* events will be triggered on power handler enable/disable */
 #define DPU_POWER_EVENT_DISABLE	BIT(0)
 #define DPU_POWER_EVENT_ENABLE	BIT(1)
 
-/**
- * mdss_bus_vote_type: register bus vote type
- * VOTE_INDEX_DISABLE: removes the client vote
- * VOTE_INDEX_LOW: keeps the lowest vote for register bus
- * VOTE_INDEX_MAX: invalid
- */
-enum mdss_bus_vote_type {
-	VOTE_INDEX_DISABLE,
-	VOTE_INDEX_LOW,
-	VOTE_INDEX_MAX,
-};
-
-/**
- * enum dpu_power_handle_data_bus_client - type of axi bus clients
- * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT: core real-time bus client
- * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT: core non-real-time bus client
- * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX: maximum number of bus client type
- */
-enum dpu_power_handle_data_bus_client {
-	DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT,
-	DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT,
-	DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX
-};
-
-/**
- * enum DPU_POWER_HANDLE_DBUS_ID - data bus identifier
- * @DPU_POWER_HANDLE_DBUS_ID_MNOC: DPU/MNOC data bus
- * @DPU_POWER_HANDLE_DBUS_ID_LLCC: MNOC/LLCC data bus
- * @DPU_POWER_HANDLE_DBUS_ID_EBI: LLCC/EBI data bus
- */
-enum DPU_POWER_HANDLE_DBUS_ID {
-	DPU_POWER_HANDLE_DBUS_ID_MNOC,
-	DPU_POWER_HANDLE_DBUS_ID_LLCC,
-	DPU_POWER_HANDLE_DBUS_ID_EBI,
-	DPU_POWER_HANDLE_DBUS_ID_MAX,
-};
-
 /**
  * struct dpu_power_client: stores the power client for dpu driver
  * @name:	name of the client
- * @usecase_ndx: current regs bus vote type
  * @refcount:	current refcount if multiple modules are using same
  *              same client for enable/disable. Power module will
  *              aggregate the refcount and vote accordingly for this
  *              client.
  * @id:		assigned during create. helps for debugging.
  * @list:	list to attach power handle master list
- * @ab:         arbitrated bandwidth for each bus client
- * @ib:         instantaneous bandwidth for each bus client
  * @active:	inidcates the state of dpu power handle
  */
 struct dpu_power_client {
 	char name[MAX_CLIENT_NAME_LEN];
-	short usecase_ndx;
 	short refcount;
 	u32 id;
 	struct list_head list;
-	u64 ab[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
-	u64 ib[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
 	bool active;
 };
 
@@ -112,14 +64,12 @@ struct dpu_power_event {
  * @client_clist: master list to store all clients
  * @phandle_lock: lock to synchronize the enable/disable
  * @dev: pointer to device structure
- * @usecase_ndx: current usecase index
  * @event_list: current power handle event list
  */
 struct dpu_power_handle {
 	struct list_head power_client_clist;
 	struct mutex phandle_lock;
 	struct device *dev;
-	u32 current_usecase_ndx;
 	struct list_head event_list;
 };
 
@@ -172,17 +122,6 @@ void dpu_power_client_destroy(struct dpu_power_handle *phandle,
 int dpu_power_resource_enable(struct dpu_power_handle *pdata,
 	struct dpu_power_client *pclient, bool enable);
 
-/**
- * dpu_power_data_bus_bandwidth_ctrl() - control data bus bandwidth enable
- * @phandle:  power handle containing the resources
- * @client: client information to bandwidth control
- * @enable: true to enable bandwidth for data base
- *
- * Return: none
- */
-void dpu_power_data_bus_bandwidth_ctrl(struct dpu_power_handle *phandle,
-		struct dpu_power_client *pclient, int enable);
-
 /**
  * dpu_power_handle_register_event - register a callback function for an event.
  *	Clients can register for multiple events with a single register.
@@ -207,11 +146,4 @@ struct dpu_power_event *dpu_power_handle_register_event(
 void dpu_power_handle_unregister_event(struct dpu_power_handle *phandle,
 		struct dpu_power_event *event);
 
-/**
- * dpu_power_handle_get_dbus_name - get name of given data bus identifier
- * @bus_id:	data bus identifier
- * Return:	Pointer to name string if success; NULL otherwise
- */
-const char *dpu_power_handle_get_dbus_name(u32 bus_id);
-
 #endif /* _DPU_POWER_HANDLE_H_ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 0c122e173892..f5ed47a1556a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -167,16 +167,13 @@ 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,
+	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)
@@ -184,20 +181,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,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/3] drm/msm/dpu: clean up references of DPU custom bus scaling

[Sean Paul]

On Wed, Oct 10, 2018 at 02:54:32PM +0530, Sravanthi Kollukuduru wrote:
> Since the upstream interconnect bus framework has landed
> upstream, the existing references of custom bus scaling
> needs to be cleaned up.

Just a few nits below.

> 
> Changes in v2:
> 	- Fixed build error due to partial clean up
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c    | 157 ++++++++++-------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h    |   4 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c         |  13 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c |  47 ++-----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h |  68 ----------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h        |  21 +--
>  6 files changed, 86 insertions(+), 224 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 41c5191f9056..4ee6f0dd14f7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -90,7 +90,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");
> @@ -101,35 +100,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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_ID_MNOC],
> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
> -			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_LLCC],
> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
> -			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_EBI],
> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI]);
> +			perf->max_per_pipe_ib, perf->bw_ctl);
>  }
>  
>  int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
> @@ -142,7 +130,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");
> @@ -164,31 +151,28 @@ 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_POWER_HANDLE_DBUS_ID_MNOC;
> -			i < DPU_POWER_HANDLE_DBUS_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 (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
> -			    (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 (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
> +		    (dpu_crtc_get_client_type(tmp_crtc) ==
> +				    curr_client_type) &&

Can go on one line now

> +		    (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;

Also fits on a line now

>  		}
>  
>  		/* convert bandwidth to kb */
> @@ -219,9 +203,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;
> @@ -234,13 +218,12 @@ 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;
> @@ -260,7 +243,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");
> @@ -296,10 +278,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_POWER_HANDLE_DBUS_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);
>  	}
>  }
>  
> @@ -342,11 +322,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;
> @@ -373,7 +352,6 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>  	new = &dpu_cstate->new_perf;
>  
>  	if (_dpu_core_perf_crtc_is_power_on(crtc) && !stop_req) {
> -		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
>  			/*
>  			 * cases for bus bandwidth update.
>  			 * 1. new bandwidth vote - "ab or ib vote" is higher
> @@ -381,23 +359,22 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>  			 * 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])))) {
> +			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)))) {

You can condense this by joining some lines together. Bonus points if you can
simplify it further.

>  				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);
> -			}
> +					new->bw_ctl, old->bw_ctl);
> +				old->bw_ctl = new->bw_ctl;
> +				old->max_per_pipe_ib =
> +						new->max_per_pipe_ib;

Fits on one line now

> +				update_bus = true;
>  		}
>  
>  		if ((params_changed &&
> @@ -405,30 +382,26 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>  				(!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_POWER_HANDLE_DBUS_ID_MNOC],
> -				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
> -				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI],
> -				new->core_clk_rate, stop_req,
> -				update_bus, update_clk);
> -
> -	for (i = 0; i < DPU_POWER_HANDLE_DBUS_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;
> -			}
> +				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 fbcbe0c7527a..0fb276929dc5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> @@ -30,8 +30,8 @@
>   * @core_clk_rate: core clock rate request
>   */
>  struct dpu_core_perf_params {
> -	u64 max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MAX];
> -	u64 bw_ctl[DPU_POWER_HANDLE_DBUS_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 c5ba753d441c..841476d77f5c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1373,21 +1373,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_POWER_HANDLE_DBUS_ID_MNOC;
> -			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> -		seq_printf(s, "bw_ctl[%s]: %llu\n",
> -				dpu_power_handle_get_dbus_name(i),
> -				dpu_crtc->cur_perf.bw_ctl[i]);
> -		seq_printf(s, "max_per_pipe_ib[%s]: %llu\n",
> -				dpu_power_handle_get_dbus_name(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_power_handle.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> index fc14116789f2..d58d01544731 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> @@ -24,20 +24,6 @@
>  #include "dpu_power_handle.h"
>  #include "dpu_trace.h"
>  
> -static const char *data_bus_name[DPU_POWER_HANDLE_DBUS_ID_MAX] = {
> -	[DPU_POWER_HANDLE_DBUS_ID_MNOC] = "qcom,dpu-data-bus",
> -	[DPU_POWER_HANDLE_DBUS_ID_LLCC] = "qcom,dpu-llcc-bus",
> -	[DPU_POWER_HANDLE_DBUS_ID_EBI] = "qcom,dpu-ebi-bus",
> -};
> -
> -const char *dpu_power_handle_get_dbus_name(u32 bus_id)
> -{
> -	if (bus_id < DPU_POWER_HANDLE_DBUS_ID_MAX)
> -		return data_bus_name[bus_id];
> -
> -	return NULL;
> -}
> -
>  static void dpu_power_event_trigger_locked(struct dpu_power_handle *phandle,
>  		u32 event_type)
>  {
> @@ -66,7 +52,6 @@ struct dpu_power_client *dpu_power_client_create(
>  
>  	mutex_lock(&phandle->phandle_lock);
>  	strlcpy(client->name, client_name, MAX_CLIENT_NAME_LEN);
> -	client->usecase_ndx = VOTE_INDEX_DISABLE;
>  	client->id = id;
>  	client->active = true;
>  	pr_debug("client %s created:%pK id :%d\n", client_name,
> @@ -143,8 +128,6 @@ int dpu_power_resource_enable(struct dpu_power_handle *phandle,
>  	struct dpu_power_client *pclient, bool enable)
>  {
>  	bool changed = false;
> -	u32 max_usecase_ndx = VOTE_INDEX_DISABLE, prev_usecase_ndx;
> -	struct dpu_power_client *client;
>  	u32 event_type;
>  
>  	if (!phandle || !pclient) {
> @@ -153,31 +136,19 @@ int dpu_power_resource_enable(struct dpu_power_handle *phandle,
>  	}
>  
>  	mutex_lock(&phandle->phandle_lock);
> -	if (enable)
> +	if (enable) {
>  		pclient->refcount++;
> -	else if (pclient->refcount)
> +		if (pclient->refcount == 1)
> +			changed = true;
> +	} else if (pclient->refcount) {
>  		pclient->refcount--;
> -
> -	if (pclient->refcount)
> -		pclient->usecase_ndx = VOTE_INDEX_LOW;
> -	else
> -		pclient->usecase_ndx = VOTE_INDEX_DISABLE;
> -
> -	list_for_each_entry(client, &phandle->power_client_clist, list) {
> -		if (client->usecase_ndx < VOTE_INDEX_MAX &&
> -		    client->usecase_ndx > max_usecase_ndx)
> -			max_usecase_ndx = client->usecase_ndx;
> -	}
> -
> -	if (phandle->current_usecase_ndx != max_usecase_ndx) {
> -		changed = true;
> -		prev_usecase_ndx = phandle->current_usecase_ndx;
> -		phandle->current_usecase_ndx = max_usecase_ndx;
> +		if (!pclient->refcount)
> +			changed = true;
>  	}
>  
> -	pr_debug("%pS: changed=%d current idx=%d request client %s id:%u enable:%d refcount:%d\n",
> -		__builtin_return_address(0), changed, max_usecase_ndx,
> -		pclient->name, pclient->id, enable, pclient->refcount);
> +	pr_debug("%pS:changed=%d client %s id:%u enable:%d refcount:%d\n",
> +		__builtin_return_address(0), changed, pclient->name,
> +		pclient->id, enable, pclient->refcount);
>  
>  	if (!changed)
>  		goto end;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> index a65b7a297f21..b1a5ae4d6f4e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> @@ -16,76 +16,28 @@
>  
>  #define MAX_CLIENT_NAME_LEN 128
>  
> -#define DPU_POWER_HANDLE_ENABLE_BUS_AB_QUOTA	0
> -#define DPU_POWER_HANDLE_DISABLE_BUS_AB_QUOTA	0
> -#define DPU_POWER_HANDLE_ENABLE_BUS_IB_QUOTA	1600000000
> -#define DPU_POWER_HANDLE_DISABLE_BUS_IB_QUOTA	0
> -
>  #include "dpu_io_util.h"
>  
>  /* events will be triggered on power handler enable/disable */
>  #define DPU_POWER_EVENT_DISABLE	BIT(0)
>  #define DPU_POWER_EVENT_ENABLE	BIT(1)
>  
> -/**
> - * mdss_bus_vote_type: register bus vote type
> - * VOTE_INDEX_DISABLE: removes the client vote
> - * VOTE_INDEX_LOW: keeps the lowest vote for register bus
> - * VOTE_INDEX_MAX: invalid
> - */
> -enum mdss_bus_vote_type {
> -	VOTE_INDEX_DISABLE,
> -	VOTE_INDEX_LOW,
> -	VOTE_INDEX_MAX,
> -};
> -
> -/**
> - * enum dpu_power_handle_data_bus_client - type of axi bus clients
> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT: core real-time bus client
> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT: core non-real-time bus client
> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX: maximum number of bus client type
> - */
> -enum dpu_power_handle_data_bus_client {
> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT,
> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT,
> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX
> -};
> -
> -/**
> - * enum DPU_POWER_HANDLE_DBUS_ID - data bus identifier
> - * @DPU_POWER_HANDLE_DBUS_ID_MNOC: DPU/MNOC data bus
> - * @DPU_POWER_HANDLE_DBUS_ID_LLCC: MNOC/LLCC data bus
> - * @DPU_POWER_HANDLE_DBUS_ID_EBI: LLCC/EBI data bus
> - */
> -enum DPU_POWER_HANDLE_DBUS_ID {
> -	DPU_POWER_HANDLE_DBUS_ID_MNOC,
> -	DPU_POWER_HANDLE_DBUS_ID_LLCC,
> -	DPU_POWER_HANDLE_DBUS_ID_EBI,
> -	DPU_POWER_HANDLE_DBUS_ID_MAX,
> -};
> -
>  /**
>   * struct dpu_power_client: stores the power client for dpu driver
>   * @name:	name of the client
> - * @usecase_ndx: current regs bus vote type
>   * @refcount:	current refcount if multiple modules are using same
>   *              same client for enable/disable. Power module will
>   *              aggregate the refcount and vote accordingly for this
>   *              client.
>   * @id:		assigned during create. helps for debugging.
>   * @list:	list to attach power handle master list
> - * @ab:         arbitrated bandwidth for each bus client
> - * @ib:         instantaneous bandwidth for each bus client
>   * @active:	inidcates the state of dpu power handle
>   */
>  struct dpu_power_client {
>  	char name[MAX_CLIENT_NAME_LEN];
> -	short usecase_ndx;
>  	short refcount;
>  	u32 id;
>  	struct list_head list;
> -	u64 ab[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
> -	u64 ib[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
>  	bool active;
>  };
>  
> @@ -112,14 +64,12 @@ struct dpu_power_event {
>   * @client_clist: master list to store all clients
>   * @phandle_lock: lock to synchronize the enable/disable
>   * @dev: pointer to device structure
> - * @usecase_ndx: current usecase index
>   * @event_list: current power handle event list
>   */
>  struct dpu_power_handle {
>  	struct list_head power_client_clist;
>  	struct mutex phandle_lock;
>  	struct device *dev;
> -	u32 current_usecase_ndx;
>  	struct list_head event_list;
>  };
>  
> @@ -172,17 +122,6 @@ void dpu_power_client_destroy(struct dpu_power_handle *phandle,
>  int dpu_power_resource_enable(struct dpu_power_handle *pdata,
>  	struct dpu_power_client *pclient, bool enable);
>  
> -/**
> - * dpu_power_data_bus_bandwidth_ctrl() - control data bus bandwidth enable
> - * @phandle:  power handle containing the resources
> - * @client: client information to bandwidth control
> - * @enable: true to enable bandwidth for data base
> - *
> - * Return: none
> - */
> -void dpu_power_data_bus_bandwidth_ctrl(struct dpu_power_handle *phandle,
> -		struct dpu_power_client *pclient, int enable);
> -
>  /**
>   * dpu_power_handle_register_event - register a callback function for an event.
>   *	Clients can register for multiple events with a single register.
> @@ -207,11 +146,4 @@ struct dpu_power_event *dpu_power_handle_register_event(
>  void dpu_power_handle_unregister_event(struct dpu_power_handle *phandle,
>  		struct dpu_power_event *event);
>  
> -/**
> - * dpu_power_handle_get_dbus_name - get name of given data bus identifier
> - * @bus_id:	data bus identifier
> - * Return:	Pointer to name string if success; NULL otherwise
> - */
> -const char *dpu_power_handle_get_dbus_name(u32 bus_id);
> -
>  #endif /* _DPU_POWER_HANDLE_H_ */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> index 0c122e173892..f5ed47a1556a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> @@ -167,16 +167,13 @@ 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,
> +	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)
> @@ -184,20 +181,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,
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/3] drm/msm/dpu: clean up references of DPU custom bus scaling

[skolluku-sgV2jX0FEOL9JmXXK+q4OQ]

On 2018-11-14 21:19, Sean Paul wrote:
> On Wed, Oct 10, 2018 at 02:54:32PM +0530, Sravanthi Kollukuduru wrote:
>> Since the upstream interconnect bus framework has landed
>> upstream, the existing references of custom bus scaling
>> needs to be cleaned up.
> 
> Just a few nits below.
Sure, will make the changes.
> 
>> 
>> Changes in v2:
>> 	- Fixed build error due to partial clean up
>> 
>> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c    | 157 
>> ++++++++++-------------
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h    |   4 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c         |  13 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c |  47 ++-----
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h |  68 ----------
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h        |  21 +--
>>  6 files changed, 86 insertions(+), 224 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 41c5191f9056..4ee6f0dd14f7 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> @@ -90,7 +90,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");
>> @@ -101,35 +100,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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_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_POWER_HANDLE_DBUS_ID_MNOC],
>> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
>> -			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_LLCC],
>> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
>> -			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_EBI],
>> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI]);
>> +			perf->max_per_pipe_ib, perf->bw_ctl);
>>  }
>> 
>>  int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>> @@ -142,7 +130,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");
>> @@ -164,31 +151,28 @@ 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_POWER_HANDLE_DBUS_ID_MNOC;
>> -			i < DPU_POWER_HANDLE_DBUS_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 (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
>> -			    (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 (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
>> +		    (dpu_crtc_get_client_type(tmp_crtc) ==
>> +				    curr_client_type) &&
> 
> Can go on one line now
> 
>> +		    (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;
> 
> Also fits on a line now
> 
>>  		}
>> 
>>  		/* convert bandwidth to kb */
>> @@ -219,9 +203,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;
>> @@ -234,13 +218,12 @@ 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;
>> @@ -260,7 +243,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");
>> @@ -296,10 +278,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_POWER_HANDLE_DBUS_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);
>>  	}
>>  }
>> 
>> @@ -342,11 +322,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;
>> @@ -373,7 +352,6 @@ int dpu_core_perf_crtc_update(struct drm_crtc 
>> *crtc,
>>  	new = &dpu_cstate->new_perf;
>> 
>>  	if (_dpu_core_perf_crtc_is_power_on(crtc) && !stop_req) {
>> -		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
>>  			/*
>>  			 * cases for bus bandwidth update.
>>  			 * 1. new bandwidth vote - "ab or ib vote" is higher
>> @@ -381,23 +359,22 @@ int dpu_core_perf_crtc_update(struct drm_crtc 
>> *crtc,
>>  			 * 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])))) {
>> +			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)))) {
> 
> You can condense this by joining some lines together. Bonus points if 
> you can
> simplify it further.
> 
>>  				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);
>> -			}
>> +					new->bw_ctl, old->bw_ctl);
>> +				old->bw_ctl = new->bw_ctl;
>> +				old->max_per_pipe_ib =
>> +						new->max_per_pipe_ib;
> 
> Fits on one line now
> 
>> +				update_bus = true;
>>  		}
>> 
>>  		if ((params_changed &&
>> @@ -405,30 +382,26 @@ int dpu_core_perf_crtc_update(struct drm_crtc 
>> *crtc,
>>  				(!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_POWER_HANDLE_DBUS_ID_MNOC],
>> -				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
>> -				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI],
>> -				new->core_clk_rate, stop_req,
>> -				update_bus, update_clk);
>> -
>> -	for (i = 0; i < DPU_POWER_HANDLE_DBUS_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;
>> -			}
>> +				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 fbcbe0c7527a..0fb276929dc5 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
>> @@ -30,8 +30,8 @@
>>   * @core_clk_rate: core clock rate request
>>   */
>>  struct dpu_core_perf_params {
>> -	u64 max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MAX];
>> -	u64 bw_ctl[DPU_POWER_HANDLE_DBUS_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 c5ba753d441c..841476d77f5c 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -1373,21 +1373,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_POWER_HANDLE_DBUS_ID_MNOC;
>> -			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
>> -		seq_printf(s, "bw_ctl[%s]: %llu\n",
>> -				dpu_power_handle_get_dbus_name(i),
>> -				dpu_crtc->cur_perf.bw_ctl[i]);
>> -		seq_printf(s, "max_per_pipe_ib[%s]: %llu\n",
>> -				dpu_power_handle_get_dbus_name(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_power_handle.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
>> index fc14116789f2..d58d01544731 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
>> @@ -24,20 +24,6 @@
>>  #include "dpu_power_handle.h"
>>  #include "dpu_trace.h"
>> 
>> -static const char *data_bus_name[DPU_POWER_HANDLE_DBUS_ID_MAX] = {
>> -	[DPU_POWER_HANDLE_DBUS_ID_MNOC] = "qcom,dpu-data-bus",
>> -	[DPU_POWER_HANDLE_DBUS_ID_LLCC] = "qcom,dpu-llcc-bus",
>> -	[DPU_POWER_HANDLE_DBUS_ID_EBI] = "qcom,dpu-ebi-bus",
>> -};
>> -
>> -const char *dpu_power_handle_get_dbus_name(u32 bus_id)
>> -{
>> -	if (bus_id < DPU_POWER_HANDLE_DBUS_ID_MAX)
>> -		return data_bus_name[bus_id];
>> -
>> -	return NULL;
>> -}
>> -
>>  static void dpu_power_event_trigger_locked(struct dpu_power_handle 
>> *phandle,
>>  		u32 event_type)
>>  {
>> @@ -66,7 +52,6 @@ struct dpu_power_client *dpu_power_client_create(
>> 
>>  	mutex_lock(&phandle->phandle_lock);
>>  	strlcpy(client->name, client_name, MAX_CLIENT_NAME_LEN);
>> -	client->usecase_ndx = VOTE_INDEX_DISABLE;
>>  	client->id = id;
>>  	client->active = true;
>>  	pr_debug("client %s created:%pK id :%d\n", client_name,
>> @@ -143,8 +128,6 @@ int dpu_power_resource_enable(struct 
>> dpu_power_handle *phandle,
>>  	struct dpu_power_client *pclient, bool enable)
>>  {
>>  	bool changed = false;
>> -	u32 max_usecase_ndx = VOTE_INDEX_DISABLE, prev_usecase_ndx;
>> -	struct dpu_power_client *client;
>>  	u32 event_type;
>> 
>>  	if (!phandle || !pclient) {
>> @@ -153,31 +136,19 @@ int dpu_power_resource_enable(struct 
>> dpu_power_handle *phandle,
>>  	}
>> 
>>  	mutex_lock(&phandle->phandle_lock);
>> -	if (enable)
>> +	if (enable) {
>>  		pclient->refcount++;
>> -	else if (pclient->refcount)
>> +		if (pclient->refcount == 1)
>> +			changed = true;
>> +	} else if (pclient->refcount) {
>>  		pclient->refcount--;
>> -
>> -	if (pclient->refcount)
>> -		pclient->usecase_ndx = VOTE_INDEX_LOW;
>> -	else
>> -		pclient->usecase_ndx = VOTE_INDEX_DISABLE;
>> -
>> -	list_for_each_entry(client, &phandle->power_client_clist, list) {
>> -		if (client->usecase_ndx < VOTE_INDEX_MAX &&
>> -		    client->usecase_ndx > max_usecase_ndx)
>> -			max_usecase_ndx = client->usecase_ndx;
>> -	}
>> -
>> -	if (phandle->current_usecase_ndx != max_usecase_ndx) {
>> -		changed = true;
>> -		prev_usecase_ndx = phandle->current_usecase_ndx;
>> -		phandle->current_usecase_ndx = max_usecase_ndx;
>> +		if (!pclient->refcount)
>> +			changed = true;
>>  	}
>> 
>> -	pr_debug("%pS: changed=%d current idx=%d request client %s id:%u 
>> enable:%d refcount:%d\n",
>> -		__builtin_return_address(0), changed, max_usecase_ndx,
>> -		pclient->name, pclient->id, enable, pclient->refcount);
>> +	pr_debug("%pS:changed=%d client %s id:%u enable:%d refcount:%d\n",
>> +		__builtin_return_address(0), changed, pclient->name,
>> +		pclient->id, enable, pclient->refcount);
>> 
>>  	if (!changed)
>>  		goto end;
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
>> index a65b7a297f21..b1a5ae4d6f4e 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
>> @@ -16,76 +16,28 @@
>> 
>>  #define MAX_CLIENT_NAME_LEN 128
>> 
>> -#define DPU_POWER_HANDLE_ENABLE_BUS_AB_QUOTA	0
>> -#define DPU_POWER_HANDLE_DISABLE_BUS_AB_QUOTA	0
>> -#define DPU_POWER_HANDLE_ENABLE_BUS_IB_QUOTA	1600000000
>> -#define DPU_POWER_HANDLE_DISABLE_BUS_IB_QUOTA	0
>> -
>>  #include "dpu_io_util.h"
>> 
>>  /* events will be triggered on power handler enable/disable */
>>  #define DPU_POWER_EVENT_DISABLE	BIT(0)
>>  #define DPU_POWER_EVENT_ENABLE	BIT(1)
>> 
>> -/**
>> - * mdss_bus_vote_type: register bus vote type
>> - * VOTE_INDEX_DISABLE: removes the client vote
>> - * VOTE_INDEX_LOW: keeps the lowest vote for register bus
>> - * VOTE_INDEX_MAX: invalid
>> - */
>> -enum mdss_bus_vote_type {
>> -	VOTE_INDEX_DISABLE,
>> -	VOTE_INDEX_LOW,
>> -	VOTE_INDEX_MAX,
>> -};
>> -
>> -/**
>> - * enum dpu_power_handle_data_bus_client - type of axi bus clients
>> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT: core real-time bus client
>> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT: core non-real-time bus 
>> client
>> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX: maximum number of bus 
>> client type
>> - */
>> -enum dpu_power_handle_data_bus_client {
>> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT,
>> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT,
>> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX
>> -};
>> -
>> -/**
>> - * enum DPU_POWER_HANDLE_DBUS_ID - data bus identifier
>> - * @DPU_POWER_HANDLE_DBUS_ID_MNOC: DPU/MNOC data bus
>> - * @DPU_POWER_HANDLE_DBUS_ID_LLCC: MNOC/LLCC data bus
>> - * @DPU_POWER_HANDLE_DBUS_ID_EBI: LLCC/EBI data bus
>> - */
>> -enum DPU_POWER_HANDLE_DBUS_ID {
>> -	DPU_POWER_HANDLE_DBUS_ID_MNOC,
>> -	DPU_POWER_HANDLE_DBUS_ID_LLCC,
>> -	DPU_POWER_HANDLE_DBUS_ID_EBI,
>> -	DPU_POWER_HANDLE_DBUS_ID_MAX,
>> -};
>> -
>>  /**
>>   * struct dpu_power_client: stores the power client for dpu driver
>>   * @name:	name of the client
>> - * @usecase_ndx: current regs bus vote type
>>   * @refcount:	current refcount if multiple modules are using same
>>   *              same client for enable/disable. Power module will
>>   *              aggregate the refcount and vote accordingly for this
>>   *              client.
>>   * @id:		assigned during create. helps for debugging.
>>   * @list:	list to attach power handle master list
>> - * @ab:         arbitrated bandwidth for each bus client
>> - * @ib:         instantaneous bandwidth for each bus client
>>   * @active:	inidcates the state of dpu power handle
>>   */
>>  struct dpu_power_client {
>>  	char name[MAX_CLIENT_NAME_LEN];
>> -	short usecase_ndx;
>>  	short refcount;
>>  	u32 id;
>>  	struct list_head list;
>> -	u64 ab[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
>> -	u64 ib[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
>>  	bool active;
>>  };
>> 
>> @@ -112,14 +64,12 @@ struct dpu_power_event {
>>   * @client_clist: master list to store all clients
>>   * @phandle_lock: lock to synchronize the enable/disable
>>   * @dev: pointer to device structure
>> - * @usecase_ndx: current usecase index
>>   * @event_list: current power handle event list
>>   */
>>  struct dpu_power_handle {
>>  	struct list_head power_client_clist;
>>  	struct mutex phandle_lock;
>>  	struct device *dev;
>> -	u32 current_usecase_ndx;
>>  	struct list_head event_list;
>>  };
>> 
>> @@ -172,17 +122,6 @@ void dpu_power_client_destroy(struct 
>> dpu_power_handle *phandle,
>>  int dpu_power_resource_enable(struct dpu_power_handle *pdata,
>>  	struct dpu_power_client *pclient, bool enable);
>> 
>> -/**
>> - * dpu_power_data_bus_bandwidth_ctrl() - control data bus bandwidth 
>> enable
>> - * @phandle:  power handle containing the resources
>> - * @client: client information to bandwidth control
>> - * @enable: true to enable bandwidth for data base
>> - *
>> - * Return: none
>> - */
>> -void dpu_power_data_bus_bandwidth_ctrl(struct dpu_power_handle 
>> *phandle,
>> -		struct dpu_power_client *pclient, int enable);
>> -
>>  /**
>>   * dpu_power_handle_register_event - register a callback function for 
>> an event.
>>   *	Clients can register for multiple events with a single register.
>> @@ -207,11 +146,4 @@ struct dpu_power_event 
>> *dpu_power_handle_register_event(
>>  void dpu_power_handle_unregister_event(struct dpu_power_handle 
>> *phandle,
>>  		struct dpu_power_event *event);
>> 
>> -/**
>> - * dpu_power_handle_get_dbus_name - get name of given data bus 
>> identifier
>> - * @bus_id:	data bus identifier
>> - * Return:	Pointer to name string if success; NULL otherwise
>> - */
>> -const char *dpu_power_handle_get_dbus_name(u32 bus_id);
>> -
>>  #endif /* _DPU_POWER_HANDLE_H_ */
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
>> index 0c122e173892..f5ed47a1556a 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
>> @@ -167,16 +167,13 @@ 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,
>> +	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)
>> @@ -184,20 +181,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,
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>> 
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 2/3] drm/msm/dpu: Integrate interconnect API in MDSS

[Sravanthi Kollukuduru]

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/latency/QoS requirements for the given
interconnected path.

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

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 50 +++++++++++++++++++++++++++++---
 1 file changed, 46 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 2235ef8129f4..27c2594e5133 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -4,6 +4,7 @@
  */
 
 #include "dpu_kms.h"
+#include <linux/interconnect.h>
 
 #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
 
@@ -16,8 +17,33 @@ 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, "port0");
+	struct icc_path *path1 = of_icc_get(dev->dev, "port1");
+	int total_num_paths  = 0;
+
+	if (IS_ERR(path0))
+		return PTR_ERR(path0);
+
+	dpu_mdss->path[0] = path0;
+	total_num_paths = 1;
+
+	if (!IS_ERR(path1)) {
+		dpu_mdss->path[1] = path1;
+		total_num_paths++;
+	}
+
+	dpu_mdss->num_paths = total_num_paths;
+
+	return 0;
+}
+
 static irqreturn_t dpu_mdss_irq(int irq, void *arg)
 {
 	struct dpu_mdss *dpu_mdss = arg;
@@ -127,7 +153,12 @@ 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 ab = (dpu_mdss->num_paths) ? 6800000000/dpu_mdss->num_paths : 0;
+	u64 ib = 6800000000;
+
+	for (i = 0; i < dpu_mdss->num_paths; i++)
+		icc_set(dpu_mdss->path[i], ab, ib);
 
 	ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true);
 	if (ret)
@@ -140,12 +171,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(dpu_mdss->path[i], 0, 0);
+
 	return ret;
 }
 
@@ -155,6 +189,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
 	struct msm_drm_private *priv = dev->dev_private;
 	struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss);
 	struct dss_module_power *mp = &dpu_mdss->mp;
+	int i;
 
 	_dpu_mdss_irq_domain_fini(dpu_mdss);
 
@@ -163,6 +198,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;
@@ -203,6 +241,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) {
@@ -224,14 +266,14 @@ int dpu_mdss_init(struct drm_device *dev)
 		goto irq_error;
 	}
 
+	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:
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 2/3] drm/msm/dpu: Integrate interconnect API in MDSS

[Matthias Kaehlcke]

Hi Sravanthi,

On Wed, Oct 10, 2018 at 02:54:33PM +0530, Sravanthi Kollukuduru wrote:
> 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/latency/QoS requirements for the given
> interconnected path.
> 
> Changes in v2:
> 	- Remove error log and unnecessary check (Jordan Crouse)
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 50 +++++++++++++++++++++++++++++---
>  1 file changed, 46 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 2235ef8129f4..27c2594e5133 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include "dpu_kms.h"
> +#include <linux/interconnect.h>
>  
>  #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
>  
> @@ -16,8 +17,33 @@ 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, "port0");
> +	struct icc_path *path1 = of_icc_get(dev->dev, "port1");
> +	int total_num_paths  = 0;

initialization is not needed

nit: the 'total_' prefix doesn't add any value here, just 'num_paths'
would be easier to parse. Actually you could get rid of the variable
completely by just initializing and incrementing dpu_mdss->num_paths.

>  static irqreturn_t dpu_mdss_irq(int irq, void *arg)
>  {
>  	struct dpu_mdss *dpu_mdss = arg;
> @@ -127,7 +153,12 @@ 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 ab = (dpu_mdss->num_paths) ? 6800000000/dpu_mdss->num_paths : 0;

parentheses are not needed

> +	u64 ib = 6800000000;

There should probably be a define for 6800000000.

nit: names like avg_bw and peak_pw would be clearer than ab and ib,
without being excessively verbose.

Cheers

Matthias
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm/msm/dpu: Integrate interconnect API in MDSS

[skolluku-sgV2jX0FEOL9JmXXK+q4OQ]

On 2018-10-24 22:09, Matthias Kaehlcke wrote:
> Hi Sravanthi,
> 
> On Wed, Oct 10, 2018 at 02:54:33PM +0530, Sravanthi Kollukuduru wrote:
>> 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/latency/QoS requirements for the given
>> interconnected path.
>> 
>> Changes in v2:
>> 	- Remove error log and unnecessary check (Jordan Crouse)
>> 
>> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 50 
>> +++++++++++++++++++++++++++++---
>>  1 file changed, 46 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 2235ef8129f4..27c2594e5133 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
>> @@ -4,6 +4,7 @@
>>   */
>> 
>>  #include "dpu_kms.h"
>> +#include <linux/interconnect.h>
>> 
>>  #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
>> 
>> @@ -16,8 +17,33 @@ 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, "port0");
>> +	struct icc_path *path1 = of_icc_get(dev->dev, "port1");
>> +	int total_num_paths  = 0;
> 
> initialization is not needed
> 
> nit: the 'total_' prefix doesn't add any value here, just 'num_paths'
> would be easier to parse. Actually you could get rid of the variable
> completely by just initializing and incrementing dpu_mdss->num_paths.
> 
Okay.
>>  static irqreturn_t dpu_mdss_irq(int irq, void *arg)
>>  {
>>  	struct dpu_mdss *dpu_mdss = arg;
>> @@ -127,7 +153,12 @@ 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 ab = (dpu_mdss->num_paths) ? 6800000000/dpu_mdss->num_paths : 0;
> 
> parentheses are not needed
> 
>> +	u64 ib = 6800000000;
> 
> There should probably be a define for 6800000000.
> 
Okay.
> nit: names like avg_bw and peak_pw would be clearer than ab and ib,
> without being excessively verbose.
> 
Sure, will make the changes.

> Cheers
> 
> Matthias
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 3/3] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845

[Sravanthi Kollukuduru]

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

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
---
 Documentation/devicetree/bindings/display/msm/dpu.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
index ad2e8830324e..abd4d99b5030 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 : pairs of phandles and interconnect provider specifier to
+  denote the edge source and destination ports of the interconnect path.
+- interconnect-names : list of interconnect path name strings sorted in the
+  same order as the interconnects property. Consumers drivers will use
+  interconnect-names to match interconnect paths with interconnect specifiers.
 
 Optional properties:
 - assigned-clocks: list of clock specifiers for clocks needing rate assignment
@@ -86,6 +91,9 @@ Example:
 		interrupt-controller;
 		#interrupt-cells = <1>;
 
+		interconnects = <&qnoc 38 &qnoc 512>;
+		interconnect-names = "port0";
+
 		iommus = <&apps_iommu 0>;
 
 		#address-cells = <2>;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 3/3] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845

[Rob Herring]

On Wed, Oct 10, 2018 at 02:54:34PM +0530, Sravanthi Kollukuduru wrote:
> 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
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/display/msm/dpu.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
> index ad2e8830324e..abd4d99b5030 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 : pairs of phandles and interconnect provider specifier to
> +  denote the edge source and destination ports of the interconnect path.
> +- interconnect-names : list of interconnect path name strings sorted in the
> +  same order as the interconnects property. Consumers drivers will use
> +  interconnect-names to match interconnect paths with interconnect specifiers.

Don't need to define common properties. Just list their constraints such 
as how many.

>  
>  Optional properties:
>  - assigned-clocks: list of clock specifiers for clocks needing rate assignment
> @@ -86,6 +91,9 @@ Example:
>  		interrupt-controller;
>  		#interrupt-cells = <1>;
>  
> +		interconnects = <&qnoc 38 &qnoc 512>;
> +		interconnect-names = "port0";

-names is kind of pointless here. IMO, interconnect-names should 
describe the path.

> +
>  		iommus = <&apps_iommu 0>;
>  
>  		#address-cells = <2>;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno