diff mbox series

[v2,03/10] interconnect: qcom: rpm: Always set QoS params on QNoC

Message ID 20230110132202.956619-4-konrad.dybcio@linaro.org
State New
Headers show
Series The great interconnecification fixation | expand

Commit Message

Konrad Dybcio Jan. 10, 2023, 1:21 p.m. UTC
On newer SoCs, QoS parameters and RPM bandwidth requests are wholly
separate. Setting one should only depend on the description of the
interconnect node and not whether the other is present. If we don't
vote through RPM, QoS parameters should be set regardless, as we're
requesting additional bandwidth by setting the interconnect clock
rates.

With NoC (the old-SoC bus type), this is not the case and they are
mutually exclusive (so, the current upstream logic is correct).

For BIMC however, newer SoCs expect QoS params to be always set
(like QNoC) whereas older ones (like MSM8998) hang up completely when
doing so, hence this will be addressed in the next commit.

The Fixes tag references the commit in which this logic was added, it
has since been shuffled around to a different file, but it's the one
where it originates from.

Fixes: f80a1d414328 ("interconnect: qcom: Add SDM660 interconnect provider driver")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/interconnect/qcom/icc-rpm.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Bryan O'Donoghue Jan. 10, 2023, 11:36 p.m. UTC | #1
On 10/01/2023 13:21, Konrad Dybcio wrote:
> On newer SoCs, QoS parameters and RPM bandwidth requests are wholly
> separate. Setting one should only depend on the description of the
> interconnect node and not whether the other is present. If we don't
> vote through RPM, QoS parameters should be set regardless, as we're
> requesting additional bandwidth by setting the interconnect clock
> rates.
> 
> With NoC (the old-SoC bus type), this is not the case and they are
> mutually exclusive (so, the current upstream logic is correct).
> 
> For BIMC however, newer SoCs expect QoS params to be always set
> (like QNoC) whereas older ones (like MSM8998) hang up completely when
> doing so, hence this will be addressed in the next commit.
> 
> The Fixes tag references the commit in which this logic was added, it
> has since been shuffled around to a different file, but it's the one
> where it originates from.
> 
> Fixes: f80a1d414328 ("interconnect: qcom: Add SDM660 interconnect provider driver")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   drivers/interconnect/qcom/icc-rpm.c | 20 ++++++++++++++++----
>   1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
> index cd1eab3d93ba..0516b74abdc7 100644
> --- a/drivers/interconnect/qcom/icc-rpm.c
> +++ b/drivers/interconnect/qcom/icc-rpm.c
> @@ -246,15 +246,27 @@ static int qcom_icc_rpm_set(int mas_rpm_id, int slv_rpm_id, u64 sum_bw)
>   static int __qcom_icc_set(struct icc_node *n, struct qcom_icc_node *qn,
>   			  u64 sum_bw)
>   {
> +	struct qcom_icc_provider *qp = to_qcom_provider(n->provider);
> +	bool vote_ap, vote_rpm;
>   	int ret;
>   
> -	if (!qn->qos.ap_owned) {
> -		/* send bandwidth request message to the RPM processor */
> +	if (qp->type == QCOM_ICC_QNOC) {
> +		vote_ap = true;
> +		vote_rpm = true;
> +	} else {
> +		vote_ap = qn->qos.ap_owned;
> +		vote_rpm = !vote_ap;
> +	}
> +
> +	if (vote_rpm) {
> +		/* Send bandwidth request message to the RPM processor */
>   		ret = qcom_icc_rpm_set(qn->mas_rpm_id, qn->slv_rpm_id, sum_bw);
>   		if (ret)
>   			return ret;
> -	} else if (qn->qos.qos_mode != -1) {
> -		/* set bandwidth directly from the AP */
> +	}
> +
> +	if (vote_ap && qn->qos.qos_mode != NOC_QOS_MODE_INVALID) {
> +		/* Set QoS params from the AP */
>   		ret = qcom_icc_qos_set(n, sum_bw);
>   		if (ret)
>   			return ret;

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
diff mbox series

Patch

diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
index cd1eab3d93ba..0516b74abdc7 100644
--- a/drivers/interconnect/qcom/icc-rpm.c
+++ b/drivers/interconnect/qcom/icc-rpm.c
@@ -246,15 +246,27 @@  static int qcom_icc_rpm_set(int mas_rpm_id, int slv_rpm_id, u64 sum_bw)
 static int __qcom_icc_set(struct icc_node *n, struct qcom_icc_node *qn,
 			  u64 sum_bw)
 {
+	struct qcom_icc_provider *qp = to_qcom_provider(n->provider);
+	bool vote_ap, vote_rpm;
 	int ret;
 
-	if (!qn->qos.ap_owned) {
-		/* send bandwidth request message to the RPM processor */
+	if (qp->type == QCOM_ICC_QNOC) {
+		vote_ap = true;
+		vote_rpm = true;
+	} else {
+		vote_ap = qn->qos.ap_owned;
+		vote_rpm = !vote_ap;
+	}
+
+	if (vote_rpm) {
+		/* Send bandwidth request message to the RPM processor */
 		ret = qcom_icc_rpm_set(qn->mas_rpm_id, qn->slv_rpm_id, sum_bw);
 		if (ret)
 			return ret;
-	} else if (qn->qos.qos_mode != -1) {
-		/* set bandwidth directly from the AP */
+	}
+
+	if (vote_ap && qn->qos.qos_mode != NOC_QOS_MODE_INVALID) {
+		/* Set QoS params from the AP */
 		ret = qcom_icc_qos_set(n, sum_bw);
 		if (ret)
 			return ret;