[PATCH v2 1/2] interconnect: qcom: icc-rpm: record slave RPM id in error log

2021-02-04 Thread Benjamin Li
Add slave RPM ID to assist with identifying incorrect RPM config.

Signed-off-by: Benjamin Li 
---
 drivers/interconnect/qcom/icc-rpm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/interconnect/qcom/icc-rpm.c 
b/drivers/interconnect/qcom/icc-rpm.c
index cc6095492cbe..54de49ca7808 100644
--- a/drivers/interconnect/qcom/icc-rpm.c
+++ b/drivers/interconnect/qcom/icc-rpm.c
@@ -59,8 +59,8 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node 
*dst)
qn->slv_rpm_id,
sum_bw);
if (ret) {
-   pr_err("qcom_icc_rpm_smd_send slv error %d\n",
-  ret);
+   pr_err("qcom_icc_rpm_smd_send slv %d error %d\n",
+  qn->slv_rpm_id, ret);
return ret;
}
}
-- 
2.17.1



[PATCH v2 2/2] interconnect: qcom: msm8939: remove rpm-ids from non-RPM nodes

2021-02-04 Thread Benjamin Li
Some nodes are incorrectly marked as RPM-controlled (they have RPM
master and slave ids assigned), but are actually controlled by the
application CPU instead. The RPM complains when we send requests for
resources that it can't control. Let's fix this by replacing the IDs,
with the default "-1" in which case no requests are sent.

See commit c497f9322af9 ("interconnect: qcom: msm8916: Remove rpm-ids
from non-RPM nodes") where this was done for msm8916.

Signed-off-by: Benjamin Li 
---
 drivers/interconnect/qcom/msm8939.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/interconnect/qcom/msm8939.c 
b/drivers/interconnect/qcom/msm8939.c
index dfbec30ed149..20f31a1b4192 100644
--- a/drivers/interconnect/qcom/msm8939.c
+++ b/drivers/interconnect/qcom/msm8939.c
@@ -131,7 +131,7 @@ DEFINE_QNODE(mas_pcnoc_sdcc_1, MSM8939_MASTER_SDCC_1, 8, 
-1, -1, MSM8939_PNOC_IN
 DEFINE_QNODE(mas_pcnoc_sdcc_2, MSM8939_MASTER_SDCC_2, 8, -1, -1, 
MSM8939_PNOC_INT_1);
 DEFINE_QNODE(mas_qdss_bam, MSM8939_MASTER_QDSS_BAM, 8, -1, -1, 
MSM8939_SNOC_QDSS_INT);
 DEFINE_QNODE(mas_qdss_etr, MSM8939_MASTER_QDSS_ETR, 8, -1, -1, 
MSM8939_SNOC_QDSS_INT);
-DEFINE_QNODE(mas_snoc_cfg, MSM8939_MASTER_SNOC_CFG, 4, 20, -1, 
MSM8939_SLAVE_SRVC_SNOC);
+DEFINE_QNODE(mas_snoc_cfg, MSM8939_MASTER_SNOC_CFG, 4, -1, -1, 
MSM8939_SLAVE_SRVC_SNOC);
 DEFINE_QNODE(mas_spdm, MSM8939_MASTER_SPDM, 4, -1, -1, MSM8939_PNOC_MAS_0);
 DEFINE_QNODE(mas_tcu0, MSM8939_MASTER_TCU0, 16, -1, -1, MSM8939_SLAVE_EBI_CH0, 
MSM8939_BIMC_SNOC_MAS, MSM8939_SLAVE_AMPSS_L2);
 DEFINE_QNODE(mas_usb_hs1, MSM8939_MASTER_USB_HS1, 4, -1, -1, 
MSM8939_PNOC_MAS_1);
@@ -156,14 +156,14 @@ DEFINE_QNODE(pcnoc_snoc_mas, MSM8939_PNOC_SNOC_MAS, 8, 
29, -1, MSM8939_PNOC_SNOC
 DEFINE_QNODE(pcnoc_snoc_slv, MSM8939_PNOC_SNOC_SLV, 8, -1, 45, 
MSM8939_SNOC_INT_0, MSM8939_SNOC_INT_BIMC, MSM8939_SNOC_INT_1);
 DEFINE_QNODE(qdss_int, MSM8939_SNOC_QDSS_INT, 8, -1, -1, MSM8939_SNOC_INT_0, 
MSM8939_SNOC_INT_BIMC);
 DEFINE_QNODE(slv_apps_l2, MSM8939_SLAVE_AMPSS_L2, 16, -1, -1, 0);
-DEFINE_QNODE(slv_apss, MSM8939_SLAVE_APSS, 4, -1, 20, 0);
+DEFINE_QNODE(slv_apss, MSM8939_SLAVE_APSS, 4, -1, -1, 0);
 DEFINE_QNODE(slv_audio, MSM8939_SLAVE_LPASS, 4, -1, -1, 0);
 DEFINE_QNODE(slv_bimc_cfg, MSM8939_SLAVE_BIMC_CFG, 4, -1, -1, 0);
 DEFINE_QNODE(slv_blsp_1, MSM8939_SLAVE_BLSP_1, 4, -1, -1, 0);
 DEFINE_QNODE(slv_boot_rom, MSM8939_SLAVE_BOOT_ROM, 4, -1, -1, 0);
 DEFINE_QNODE(slv_camera_cfg, MSM8939_SLAVE_CAMERA_CFG, 4, -1, -1, 0);
-DEFINE_QNODE(slv_cats_0, MSM8939_SLAVE_CATS_128, 16, -1, 106, 0);
-DEFINE_QNODE(slv_cats_1, MSM8939_SLAVE_OCMEM_64, 8, -1, 107, 0);
+DEFINE_QNODE(slv_cats_0, MSM8939_SLAVE_CATS_128, 16, -1, -1, 0);
+DEFINE_QNODE(slv_cats_1, MSM8939_SLAVE_OCMEM_64, 8, -1, -1, 0);
 DEFINE_QNODE(slv_clk_ctl, MSM8939_SLAVE_CLK_CTL, 4, -1, -1, 0);
 DEFINE_QNODE(slv_crypto_0_cfg, MSM8939_SLAVE_CRYPTO_0_CFG, 4, -1, -1, 0);
 DEFINE_QNODE(slv_dehr_cfg, MSM8939_SLAVE_DEHR_CFG, 4, -1, -1, 0);
@@ -187,20 +187,20 @@ DEFINE_QNODE(slv_sdcc_2, MSM8939_SLAVE_SDCC_2, 4, -1, -1, 
0);
 DEFINE_QNODE(slv_security, MSM8939_SLAVE_SECURITY, 4, -1, -1, 0);
 DEFINE_QNODE(slv_snoc_cfg, MSM8939_SLAVE_SNOC_CFG, 4, -1, -1, 0);
 DEFINE_QNODE(slv_spdm, MSM8939_SLAVE_SPDM, 4, -1, -1, 0);
-DEFINE_QNODE(slv_srvc_snoc, MSM8939_SLAVE_SRVC_SNOC, 8, -1, 29, 0);
+DEFINE_QNODE(slv_srvc_snoc, MSM8939_SLAVE_SRVC_SNOC, 8, -1, -1, 0);
 DEFINE_QNODE(slv_tcsr, MSM8939_SLAVE_TCSR, 4, -1, -1, 0);
 DEFINE_QNODE(slv_tlmm, MSM8939_SLAVE_TLMM, 4, -1, -1, 0);
 DEFINE_QNODE(slv_usb_hs1, MSM8939_SLAVE_USB_HS1, 4, -1, -1, 0);
 DEFINE_QNODE(slv_usb_hs2, MSM8939_SLAVE_USB_HS2, 4, -1, -1, 0);
 DEFINE_QNODE(slv_venus_cfg, MSM8939_SLAVE_VENUS_CFG, 4, -1, -1, 0);
-DEFINE_QNODE(snoc_bimc_0_mas, MSM8939_SNOC_BIMC_0_MAS, 16, 3, -1, 
MSM8939_SNOC_BIMC_0_SLV);
-DEFINE_QNODE(snoc_bimc_0_slv, MSM8939_SNOC_BIMC_0_SLV, 16, -1, 24, 
MSM8939_SLAVE_EBI_CH0);
+DEFINE_QNODE(snoc_bimc_0_mas, MSM8939_SNOC_BIMC_0_MAS, 16, -1, -1, 
MSM8939_SNOC_BIMC_0_SLV);
+DEFINE_QNODE(snoc_bimc_0_slv, MSM8939_SNOC_BIMC_0_SLV, 16, -1, -1, 
MSM8939_SLAVE_EBI_CH0);
 DEFINE_QNODE(snoc_bimc_1_mas, MSM8939_SNOC_BIMC_1_MAS, 16, 76, -1, 
MSM8939_SNOC_BIMC_1_SLV);
 DEFINE_QNODE(snoc_bimc_1_slv, MSM8939_SNOC_BIMC_1_SLV, 16, -1, 104, 
MSM8939_SLAVE_EBI_CH0);
 DEFINE_QNODE(snoc_bimc_2_mas, MSM8939_SNOC_BIMC_2_MAS, 16, -1, -1, 
MSM8939_SNOC_BIMC_2_SLV);
 DEFINE_QNODE(snoc_bimc_2_slv, MSM8939_SNOC_BIMC_2_SLV, 16, -1, -1, 
MSM8939_SLAVE_EBI_CH0);
 DEFINE_QNODE(snoc_int_0, MSM8939_SNOC_INT_0, 8, 99, 130, 
MSM8939_SLAVE_QDSS_STM, MSM8939_SLAVE_IMEM, MSM8939_SNOC_PNOC_MAS);
-DEFINE_QNODE(snoc_int_1, MSM8939_SNOC_INT_1, 8, 100, 131, MSM8939_SLAVE_APSS, 
MSM8939_SLAVE_CATS_128, MSM8939_SLAVE_OCMEM_64);
+DEFINE_QNODE(snoc_int_1, MSM8939_SNOC_INT_1, 8, -1, -1, MSM8939_SLAVE_APSS, 
MSM8939_SLAVE_CATS_128, MSM8939_SLAVE_OCMEM_64);
 DEFINE_QNODE(snoc_int_bimc, MSM8939_SNOC_INT_BIMC, 8, 101, 132, 
MSM8939_SNOC_BIMC_1_MAS);
 DEFINE_QNODE(snoc_pcnoc_mas, MS

[PATCH v2 0/2] Clean up MSM8939 interconnect driver

2021-02-04 Thread Benjamin Li
Following up on a review comment on commit 6c6fe5d3dc5e ("interconnect: qcom:
Add MSM8939 interconnect provider driver") to clean up some log pollution.

This is based on icc-next (which appears to contain a refactor commonizing
functions into icc-rpm.c).

v2:
- Update commit message to add a full explanation, rather than referencing a
  previous commit.

Benjamin Li (2):
  interconnect: qcom: icc-rpm: record slave RPM id in error log
  interconnect: qcom: msm8939: remove rpm-ids from non-RPM nodes

 drivers/interconnect/qcom/icc-rpm.c |  4 ++--
 drivers/interconnect/qcom/msm8939.c | 16 
 2 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.17.1



[PATCH 2/2] interconnect: qcom: msm8939: remove rpm-ids from non-RPM nodes

2021-02-03 Thread Benjamin Li
Changes corresponding to commit c497f9322af9 ("interconnect: qcom: msm8916:
Remove rpm-ids from non-RPM nodes") to remove this log spam on 8939:

[1.901376] qcom_icc_rpm_smd_send slv 24 error -6
[2.005977] qcom_icc_rpm_smd_send mas 20 error -6
[2.010250] qcom_icc_rpm_smd_send slv 20 error -6
[2.014684] qcom_icc_rpm_smd_send slv 106 error -6
[2.019338] qcom_icc_rpm_smd_send slv 107 error -6
[2.024615] qcom_icc_rpm_smd_send slv 29 error -6
[2.028782] qcom_icc_rpm_smd_send mas 3 error -6
[2.034657] qcom_icc_rpm_smd_send mas 100 error -6
(plus another slv 131 that's hidden by the mas 100 failure)

Signed-off-by: Benjamin Li 
---
 drivers/interconnect/qcom/msm8939.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/interconnect/qcom/msm8939.c 
b/drivers/interconnect/qcom/msm8939.c
index dfbec30ed149..20f31a1b4192 100644
--- a/drivers/interconnect/qcom/msm8939.c
+++ b/drivers/interconnect/qcom/msm8939.c
@@ -131,7 +131,7 @@ DEFINE_QNODE(mas_pcnoc_sdcc_1, MSM8939_MASTER_SDCC_1, 8, 
-1, -1, MSM8939_PNOC_IN
 DEFINE_QNODE(mas_pcnoc_sdcc_2, MSM8939_MASTER_SDCC_2, 8, -1, -1, 
MSM8939_PNOC_INT_1);
 DEFINE_QNODE(mas_qdss_bam, MSM8939_MASTER_QDSS_BAM, 8, -1, -1, 
MSM8939_SNOC_QDSS_INT);
 DEFINE_QNODE(mas_qdss_etr, MSM8939_MASTER_QDSS_ETR, 8, -1, -1, 
MSM8939_SNOC_QDSS_INT);
-DEFINE_QNODE(mas_snoc_cfg, MSM8939_MASTER_SNOC_CFG, 4, 20, -1, 
MSM8939_SLAVE_SRVC_SNOC);
+DEFINE_QNODE(mas_snoc_cfg, MSM8939_MASTER_SNOC_CFG, 4, -1, -1, 
MSM8939_SLAVE_SRVC_SNOC);
 DEFINE_QNODE(mas_spdm, MSM8939_MASTER_SPDM, 4, -1, -1, MSM8939_PNOC_MAS_0);
 DEFINE_QNODE(mas_tcu0, MSM8939_MASTER_TCU0, 16, -1, -1, MSM8939_SLAVE_EBI_CH0, 
MSM8939_BIMC_SNOC_MAS, MSM8939_SLAVE_AMPSS_L2);
 DEFINE_QNODE(mas_usb_hs1, MSM8939_MASTER_USB_HS1, 4, -1, -1, 
MSM8939_PNOC_MAS_1);
@@ -156,14 +156,14 @@ DEFINE_QNODE(pcnoc_snoc_mas, MSM8939_PNOC_SNOC_MAS, 8, 
29, -1, MSM8939_PNOC_SNOC
 DEFINE_QNODE(pcnoc_snoc_slv, MSM8939_PNOC_SNOC_SLV, 8, -1, 45, 
MSM8939_SNOC_INT_0, MSM8939_SNOC_INT_BIMC, MSM8939_SNOC_INT_1);
 DEFINE_QNODE(qdss_int, MSM8939_SNOC_QDSS_INT, 8, -1, -1, MSM8939_SNOC_INT_0, 
MSM8939_SNOC_INT_BIMC);
 DEFINE_QNODE(slv_apps_l2, MSM8939_SLAVE_AMPSS_L2, 16, -1, -1, 0);
-DEFINE_QNODE(slv_apss, MSM8939_SLAVE_APSS, 4, -1, 20, 0);
+DEFINE_QNODE(slv_apss, MSM8939_SLAVE_APSS, 4, -1, -1, 0);
 DEFINE_QNODE(slv_audio, MSM8939_SLAVE_LPASS, 4, -1, -1, 0);
 DEFINE_QNODE(slv_bimc_cfg, MSM8939_SLAVE_BIMC_CFG, 4, -1, -1, 0);
 DEFINE_QNODE(slv_blsp_1, MSM8939_SLAVE_BLSP_1, 4, -1, -1, 0);
 DEFINE_QNODE(slv_boot_rom, MSM8939_SLAVE_BOOT_ROM, 4, -1, -1, 0);
 DEFINE_QNODE(slv_camera_cfg, MSM8939_SLAVE_CAMERA_CFG, 4, -1, -1, 0);
-DEFINE_QNODE(slv_cats_0, MSM8939_SLAVE_CATS_128, 16, -1, 106, 0);
-DEFINE_QNODE(slv_cats_1, MSM8939_SLAVE_OCMEM_64, 8, -1, 107, 0);
+DEFINE_QNODE(slv_cats_0, MSM8939_SLAVE_CATS_128, 16, -1, -1, 0);
+DEFINE_QNODE(slv_cats_1, MSM8939_SLAVE_OCMEM_64, 8, -1, -1, 0);
 DEFINE_QNODE(slv_clk_ctl, MSM8939_SLAVE_CLK_CTL, 4, -1, -1, 0);
 DEFINE_QNODE(slv_crypto_0_cfg, MSM8939_SLAVE_CRYPTO_0_CFG, 4, -1, -1, 0);
 DEFINE_QNODE(slv_dehr_cfg, MSM8939_SLAVE_DEHR_CFG, 4, -1, -1, 0);
@@ -187,20 +187,20 @@ DEFINE_QNODE(slv_sdcc_2, MSM8939_SLAVE_SDCC_2, 4, -1, -1, 
0);
 DEFINE_QNODE(slv_security, MSM8939_SLAVE_SECURITY, 4, -1, -1, 0);
 DEFINE_QNODE(slv_snoc_cfg, MSM8939_SLAVE_SNOC_CFG, 4, -1, -1, 0);
 DEFINE_QNODE(slv_spdm, MSM8939_SLAVE_SPDM, 4, -1, -1, 0);
-DEFINE_QNODE(slv_srvc_snoc, MSM8939_SLAVE_SRVC_SNOC, 8, -1, 29, 0);
+DEFINE_QNODE(slv_srvc_snoc, MSM8939_SLAVE_SRVC_SNOC, 8, -1, -1, 0);
 DEFINE_QNODE(slv_tcsr, MSM8939_SLAVE_TCSR, 4, -1, -1, 0);
 DEFINE_QNODE(slv_tlmm, MSM8939_SLAVE_TLMM, 4, -1, -1, 0);
 DEFINE_QNODE(slv_usb_hs1, MSM8939_SLAVE_USB_HS1, 4, -1, -1, 0);
 DEFINE_QNODE(slv_usb_hs2, MSM8939_SLAVE_USB_HS2, 4, -1, -1, 0);
 DEFINE_QNODE(slv_venus_cfg, MSM8939_SLAVE_VENUS_CFG, 4, -1, -1, 0);
-DEFINE_QNODE(snoc_bimc_0_mas, MSM8939_SNOC_BIMC_0_MAS, 16, 3, -1, 
MSM8939_SNOC_BIMC_0_SLV);
-DEFINE_QNODE(snoc_bimc_0_slv, MSM8939_SNOC_BIMC_0_SLV, 16, -1, 24, 
MSM8939_SLAVE_EBI_CH0);
+DEFINE_QNODE(snoc_bimc_0_mas, MSM8939_SNOC_BIMC_0_MAS, 16, -1, -1, 
MSM8939_SNOC_BIMC_0_SLV);
+DEFINE_QNODE(snoc_bimc_0_slv, MSM8939_SNOC_BIMC_0_SLV, 16, -1, -1, 
MSM8939_SLAVE_EBI_CH0);
 DEFINE_QNODE(snoc_bimc_1_mas, MSM8939_SNOC_BIMC_1_MAS, 16, 76, -1, 
MSM8939_SNOC_BIMC_1_SLV);
 DEFINE_QNODE(snoc_bimc_1_slv, MSM8939_SNOC_BIMC_1_SLV, 16, -1, 104, 
MSM8939_SLAVE_EBI_CH0);
 DEFINE_QNODE(snoc_bimc_2_mas, MSM8939_SNOC_BIMC_2_MAS, 16, -1, -1, 
MSM8939_SNOC_BIMC_2_SLV);
 DEFINE_QNODE(snoc_bimc_2_slv, MSM8939_SNOC_BIMC_2_SLV, 16, -1, -1, 
MSM8939_SLAVE_EBI_CH0);
 DEFINE_QNODE(snoc_int_0, MSM8939_SNOC_INT_0, 8, 99, 130, 
MSM8939_SLAVE_QDSS_STM, MSM8939_SLAVE_IMEM, MSM8939_SNOC_PNOC_MAS);
-DEFINE_QNODE(snoc_int_1, MSM8939_SNOC_INT_1, 8, 100, 131, MSM8939_SLAVE_APSS, 
MSM8939_SLAVE_CATS_128, MSM8939_SLAVE_OCMEM_64);
+DEFINE_QNODE(snoc_int_1, MSM8939_SNOC_INT_1, 8, -1, -1, MSM8939_SLAVE_APSS, 
MSM8939

[PATCH 1/2] interconnect: qcom: icc-rpm: record slave RPM id in error log

2021-02-03 Thread Benjamin Li
Add slave RPM ID to assist with identifying incorrect RPM config.

Signed-off-by: Benjamin Li 
---
 drivers/interconnect/qcom/icc-rpm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/interconnect/qcom/icc-rpm.c 
b/drivers/interconnect/qcom/icc-rpm.c
index cc6095492cbe..54de49ca7808 100644
--- a/drivers/interconnect/qcom/icc-rpm.c
+++ b/drivers/interconnect/qcom/icc-rpm.c
@@ -59,8 +59,8 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node 
*dst)
qn->slv_rpm_id,
sum_bw);
if (ret) {
-   pr_err("qcom_icc_rpm_smd_send slv error %d\n",
-  ret);
+   pr_err("qcom_icc_rpm_smd_send slv %d error %d\n",
+  qn->slv_rpm_id, ret);
return ret;
}
}
-- 
2.17.1



[PATCH 0/2] Clean up MSM8939 interconnect driver

2021-02-03 Thread Benjamin Li
Following up on a review comment on commit 6c6fe5d3dc5e ("interconnect: qcom:
Add MSM8939 interconnect provider driver") to clean up some log pollution.

This is based on icc-next (which appears to contain a refactor commonizing
functions into icc-rpm.c).

Benjamin Li (2):
  interconnect: qcom: icc-rpm: record slave RPM id in error log
  interconnect: qcom: msm8939: remove rpm-ids from non-RPM nodes

 drivers/interconnect/qcom/icc-rpm.c |  4 ++--
 drivers/interconnect/qcom/msm8939.c | 16 
 2 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.17.1



Re: [PATCH v2 5/5] interconnect: qcom: Add MSM8939 interconnect provider driver

2021-02-02 Thread Benjamin Li
On 1/5/21 5:54 AM, Georgi Djakov wrote:
> On 1/2/21 13:08, Vincent Knecht wrote:
>> Le vendredi 04 décembre 2020 à 15:53 +0800, Jun Nie a écrit :
>>> Add driver for the Qualcomm interconnect buses found in MSM8939 based
>>> platforms. The topology consists of four NoCs that are controlled by
>>> a remote processor that collects the aggregated bandwidth for each
>>> master-slave pairs.
>>>
>>> Signed-off-by: Jun Nie 
>>
>> Shouldn't some rpm ids be changed like they were for msm8916 in the 
>> following patch ?
>> c497f9322af9 ("interconnect: qcom: msm8916: Remove rpm-ids from non-RPM 
>> nodes")
>> https://patchwork.kernel.org/project/linux-arm-msm/patch/20201112105140.10092-1-georgi.dja...@linaro.org/
> 
> Maybe they should. I don't have the hardware to try it, but the test will be
> to just add the NoC DT nodes, enable the driver and inspect the boot log for
> messages like:
> [    2.926647] qcom_icc_rpm_smd_send mas X error -6
> 
> Thanks,
> Georgi

Hi Vincent & Georgi,

Thanks, I ran your suggestion on an MSM8939 board (with an additional
change to print slave IDs as well). Results:

[1.901376] qcom_icc_rpm_smd_send slv 24 error -6
[2.005977] qcom_icc_rpm_smd_send mas 20 error -6
[2.010250] qcom_icc_rpm_smd_send slv 20 error -6
[2.014684] qcom_icc_rpm_smd_send slv 106 error -6
[2.019338] qcom_icc_rpm_smd_send slv 107 error -6
[2.024615] qcom_icc_rpm_smd_send slv 29 error -6
[2.028782] qcom_icc_rpm_smd_send mas 3 error -6
[2.034657] qcom_icc_rpm_smd_send mas 100 error -6
(and there's another slv 131 that's hidden by the mas 100 failure)

Jun, I'll send you the patch I tested with to silence all these errors,
if you want to just squash that into the next version of your patchset.

Ben


Re: [PATCH] drm/msm/dsi: save PLL registers across first PHY reset

2021-01-29 Thread Benjamin Li


On 10/30/20 6:55 AM, Dmitry Baryshkov wrote:
> Hello,
> 
> On 07/10/2020 03:10, benl-kernelpatc...@squareup.com wrote:
>> From: Benjamin Li 
>>
>> Take advantage of previously-added support for persisting PLL
>> registers across DSI PHY disable/enable cycles (see 328e1a6
>> 'drm/msm/dsi: Save/Restore PLL status across PHY reset') to
>> support persisting across the very first DSI PHY enable at
>> boot.
> 
> Interesting enough, this breaks exactly on 8016. On DB410c with latest 
> bootloader and w/o splash screen this patch causes boot freeze. Without this 
> patch the board would successfully boot with display routed to HDMI.

Hi Dimtry,

Thanks for your fix for the DB410c breakage ("drm/msm/dsi: do not
try reading 28nm vco rate if it's not enabled") that this patch
causes.

I re-tested my patch on top of qcom/linux for-next (3e6a8ce094759)
which now has your fix, on a DB410c with HDMI display and no splash
(which seems to be the default using the Linaro SD card image's LK),
and indeed it is fixed.

I assume you already also did the same & are okay with this going in.
Appreciate the testing!

Ben

> 
>> The bootloader may have left the PLL registers in a non-default
>> state. For example, for dsi_pll_28nm.c on 8x16/8x39, the byte
>> clock mux's power-on reset configuration is to bypass DIV1, but
>> depending on bandwidth requirements[1] the bootloader may have
>> set the DIV1 path.
>>
>> When the byte clock mux is registered with the generic clock
>> framework at probe time, the framework reads & caches the value
>> of the mux bit field (the initial clock parent). After PHY enable,
>> when clk_set_rate is called on the byte clock, the framework
>> assumes there is no need to reparent, and doesn't re-write the
>> mux bit field. But PHY enable resets PLL registers, so the mux
>> bit field actually silently reverted to the DIV1 bypass path.
>> This causes the byte clock to be off by a factor of e.g. 2 for
>> our tested WXGA panel.
>>
>> The above issue manifests as the display not working and a
>> constant stream of FIFO/LP0 contention errors.
>>
>> [1] The specific requirement for triggering the DIV1 path (and
>> thus this issue) on 28nm is a panel with pixel clock <116.7MHz
>> (one-third the minimum VCO setting). FHD/1080p (~145MHz) is fine,
>> WXGA/1280x800 (~75MHz) is not.
>>
>> Signed-off-by: Benjamin Li 
>> ---
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 16 
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c 
>> b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> index 009f5b843dd1..139b4a5aaf86 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> @@ -621,6 +621,22 @@ static int dsi_phy_driver_probe(struct platform_device 
>> *pdev)
>>   phy->pll = NULL;
>>   }
>>   +    /*
>> + * As explained in msm_dsi_phy_enable, resetting the DSI PHY (as done
>> + * in dsi_mgr_phy_enable) silently changes its PLL registers to power-on
>> + * defaults, but the generic clock framework manages and caches several
>> + * of the PLL registers. It initializes these caches at registration
>> + * time via register read.
>> + *
>> + * As a result, we need to save DSI PLL registers once at probe in order
>> + * for the first call to msm_dsi_phy_enable to successfully bring PLL
>> + * registers back in line with what the generic clock framework expects.
>> + *
>> + * Subsequent PLL restores during msm_dsi_phy_enable will always be
>> + * paired with PLL saves in msm_dsi_phy_disable.
>> + */
>> +    msm_dsi_pll_save_state(phy->pll);
>> +
>>   dsi_phy_disable_resource(phy);
>>     platform_set_drvdata(pdev, phy);
>>
> 
>