| Message ID | 20230507175335.2321503-2-dmitry.baryshkov@linaro.org |
|---|---|
| State | Accepted |
| Commit | 4e13c7a55cf752887f2b8d8008711dbbc64ea796 |
| Headers | show |
| Series | None | expand |
On 08/05/2023 11:27, Konrad Dybcio wrote: > > > On 7.05.2023 19:53, Dmitry Baryshkov wrote: >> Using PWRSTS_RET on msm8974's MDSS_GDSC causes display to stop working. >> The gdsc doesn't fully come out of retention mode. Change it's pwrsts >> flags to PWRSTS_OFF_ON. >> >> Fixes: d399723950c4 ("clk: qcom: gdsc: Fix the handling of PWRSTS_RET support") >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- > This is a stopgap solution, not exactly a fix, all signs on Heaven and > Earth say that 8974 should support retention on this GDSC! > > *although* > > pre-v2.2 chips don't Mine is apq8074 v2.2, if I'm not mistaken. > > Konrad >> drivers/clk/qcom/mmcc-msm8974.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/qcom/mmcc-msm8974.c b/drivers/clk/qcom/mmcc-msm8974.c >> index aa29c79fcd55..277ef0065aae 100644 >> --- a/drivers/clk/qcom/mmcc-msm8974.c >> +++ b/drivers/clk/qcom/mmcc-msm8974.c >> @@ -2401,7 +2401,7 @@ static struct gdsc mdss_gdsc = { >> .pd = { >> .name = "mdss", >> }, >> - .pwrsts = PWRSTS_RET_ON, >> + .pwrsts = PWRSTS_OFF_ON, >> }; >> >> static struct gdsc camss_jpeg_gdsc = {
On Sun, May 07, 2023 at 08:53:35PM +0300, Dmitry Baryshkov wrote: > Using PWRSTS_RET on msm8974's MDSS_GDSC causes display to stop working. > The gdsc doesn't fully come out of retention mode. Change it's pwrsts > flags to PWRSTS_OFF_ON. > What does "stop working" implies? Does it work during boot and randomly stopped working or it stopped working after resume from suspend? Even though reverting to non-retention mode works, I think the issue might be somewhere else. Like the vote might be missing to get the GDSC out of retention mode. - Mani > Fixes: d399723950c4 ("clk: qcom: gdsc: Fix the handling of PWRSTS_RET support") > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/clk/qcom/mmcc-msm8974.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/qcom/mmcc-msm8974.c b/drivers/clk/qcom/mmcc-msm8974.c > index aa29c79fcd55..277ef0065aae 100644 > --- a/drivers/clk/qcom/mmcc-msm8974.c > +++ b/drivers/clk/qcom/mmcc-msm8974.c > @@ -2401,7 +2401,7 @@ static struct gdsc mdss_gdsc = { > .pd = { > .name = "mdss", > }, > - .pwrsts = PWRSTS_RET_ON, > + .pwrsts = PWRSTS_OFF_ON, > }; > > static struct gdsc camss_jpeg_gdsc = { > -- > 2.39.2 >
On 11/05/2023 11:15, Manivannan Sadhasivam wrote: > On Tue, May 09, 2023 at 02:20:07PM +0300, Dmitry Baryshkov wrote: >> On Tue, 9 May 2023 at 08:50, Manivannan Sadhasivam >> <manivannan.sadhasivam@linaro.org> wrote: >>> >>> On Sun, May 07, 2023 at 08:53:35PM +0300, Dmitry Baryshkov wrote: >>>> Using PWRSTS_RET on msm8974's MDSS_GDSC causes display to stop working. >>>> The gdsc doesn't fully come out of retention mode. Change it's pwrsts >>>> flags to PWRSTS_OFF_ON. >>>> >>> >>> What does "stop working" implies? Does it work during boot and randomly stopped >>> working or it stopped working after resume from suspend? >> >> It stops working during the boot. I observed the MDP not starting up >> properly. Mea culpa, I did not look deep enough into the details, just >> stomped upon this change which fixes the problem for me. >> > > IIUC, GDSC will be transitioned to retention mode only if the parent domain goes > to low power mode. So if the MDSS GDSC goes to retention mode during boot, then > it suggests that the parent domain is not voted properly. Unless I misunderstood > something... Not sure, what is the parent domain here. Note, it is a pretty old implementation. > > Or is the GDSC behavior changes between RPM and RPMh? > > - Mani > >>> >>> Even though reverting to non-retention mode works, I think the issue might be >>> somewhere else. Like the vote might be missing to get the GDSC out of retention >>> mode. >> >> I don't think there is a vote missing. The driver votes on MDSS_GDSC >> before enabling access to any of the registers from the MDSS region. >> >>> >>> - Mani >>> >>>> Fixes: d399723950c4 ("clk: qcom: gdsc: Fix the handling of PWRSTS_RET support") >>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>> --- >>>> drivers/clk/qcom/mmcc-msm8974.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/clk/qcom/mmcc-msm8974.c b/drivers/clk/qcom/mmcc-msm8974.c >>>> index aa29c79fcd55..277ef0065aae 100644 >>>> --- a/drivers/clk/qcom/mmcc-msm8974.c >>>> +++ b/drivers/clk/qcom/mmcc-msm8974.c >>>> @@ -2401,7 +2401,7 @@ static struct gdsc mdss_gdsc = { >>>> .pd = { >>>> .name = "mdss", >>>> }, >>>> - .pwrsts = PWRSTS_RET_ON, >>>> + .pwrsts = PWRSTS_OFF_ON, >>>> }; >>>> >>>> static struct gdsc camss_jpeg_gdsc = { >>>> -- >>>> 2.39.2 >>>> >>> >>> -- >>> மணிவண்ணன் சதாசிவம் >> >> >> >> -- >> With best wishes >> Dmitry >
diff --git a/drivers/clk/qcom/mmcc-msm8974.c b/drivers/clk/qcom/mmcc-msm8974.c index aa29c79fcd55..277ef0065aae 100644 --- a/drivers/clk/qcom/mmcc-msm8974.c +++ b/drivers/clk/qcom/mmcc-msm8974.c @@ -2401,7 +2401,7 @@ static struct gdsc mdss_gdsc = { .pd = { .name = "mdss", }, - .pwrsts = PWRSTS_RET_ON, + .pwrsts = PWRSTS_OFF_ON, }; static struct gdsc camss_jpeg_gdsc = {
Using PWRSTS_RET on msm8974's MDSS_GDSC causes display to stop working. The gdsc doesn't fully come out of retention mode. Change it's pwrsts flags to PWRSTS_OFF_ON. Fixes: d399723950c4 ("clk: qcom: gdsc: Fix the handling of PWRSTS_RET support") Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/clk/qcom/mmcc-msm8974.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)