2023-05-31 09:05:43

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 4/4] clk: qcom: mmcc-msm8998: Fix the SMMU GDSC

The SMMU GDSC doesn't have to be ALWAYS-ON and shouldn't feature the
HW_CTRL flag (it's separate from hw_ctrl_addr). In addition to that,
it should feature a cxc entry for bimc_smmu_axi_clk and be marked as
votable.

Fix all of these issues.

Fixes: d14b15b5931c ("clk: qcom: Add MSM8998 Multimedia Clock Controller (MMCC) driver")
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/mmcc-msm8998.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/mmcc-msm8998.c b/drivers/clk/qcom/mmcc-msm8998.c
index 9b98e0fb8b91..7b1d105afbd8 100644
--- a/drivers/clk/qcom/mmcc-msm8998.c
+++ b/drivers/clk/qcom/mmcc-msm8998.c
@@ -2628,11 +2628,13 @@ static struct gdsc camss_cpp_gdsc = {
static struct gdsc bimc_smmu_gdsc = {
.gdscr = 0xe020,
.gds_hw_ctrl = 0xe024,
+ .cxcs = (unsigned int []){ 0xe008 },
+ .cxc_count = 1,
.pd = {
.name = "bimc_smmu",
},
.pwrsts = PWRSTS_OFF_ON,
- .flags = HW_CTRL | ALWAYS_ON,
+ .flags = VOTABLE,
};

static struct clk_regmap *mmcc_msm8998_clocks[] = {

--
2.40.1



2023-06-01 14:36:03

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH 4/4] clk: qcom: mmcc-msm8998: Fix the SMMU GDSC

On Wed, May 31, 2023 at 3:01 AM Konrad Dybcio <[email protected]> wrote:
>
> The SMMU GDSC doesn't have to be ALWAYS-ON and shouldn't feature the
> HW_CTRL flag (it's separate from hw_ctrl_addr). In addition to that,
> it should feature a cxc entry for bimc_smmu_axi_clk and be marked as
> votable.
>
> Fix all of these issues.
>
> Fixes: d14b15b5931c ("clk: qcom: Add MSM8998 Multimedia Clock Controller (MMCC) driver")
> Signed-off-by: Konrad Dybcio <[email protected]>

Was this tested on a system where the bootloader has enabled the
display and it is active during Linux boot?

I seem to recall that in that scenario, Linux would boot up, see that
the GDSC is on, not see any clients for it (still initializing), turn
it off, and kill the display which then results in either a mess of
errors or a bus lockup.

-Jeff

2023-06-02 08:53:10

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 4/4] clk: qcom: mmcc-msm8998: Fix the SMMU GDSC



On 1.06.2023 16:14, Jeffrey Hugo wrote:
> On Wed, May 31, 2023 at 3:01 AM Konrad Dybcio <[email protected]> wrote:
>>
>> The SMMU GDSC doesn't have to be ALWAYS-ON and shouldn't feature the
>> HW_CTRL flag (it's separate from hw_ctrl_addr). In addition to that,
>> it should feature a cxc entry for bimc_smmu_axi_clk and be marked as
>> votable.
>>
>> Fix all of these issues.
>>
>> Fixes: d14b15b5931c ("clk: qcom: Add MSM8998 Multimedia Clock Controller (MMCC) driver")
>> Signed-off-by: Konrad Dybcio <[email protected]>
>
> Was this tested on a system where the bootloader has enabled the
> display and it is active during Linux boot?
No, I only have a device whose bootloader doesn't initialize display.

>
> I seem to recall that in that scenario, Linux would boot up, see that
> the GDSC is on, not see any clients for it (still initializing), turn
> it off, and kill the display which then results in either a mess of
> errors or a bus lockup.
I see 2 possible scenarios: either the display shuts off (because
somebody hasn't described the hardware properly and Linux isn't)
aware of all dependencies, or we get bus errors because we only
shut down / initialize some hardware partially, also possibly due
to bad dependency description.

Konrad
>
> -Jeff