From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73F2FC38142 for ; Sun, 22 Jan 2023 00:16:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229734AbjAVAQc (ORCPT ); Sat, 21 Jan 2023 19:16:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37436 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229463AbjAVAQa (ORCPT ); Sat, 21 Jan 2023 19:16:30 -0500 Received: from mail.z3ntu.xyz (mail.z3ntu.xyz [128.199.32.197]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8C06C17CEE; Sat, 21 Jan 2023 16:16:29 -0800 (PST) Received: from g550jk.localnet (unknown [62.108.10.64]) by mail.z3ntu.xyz (Postfix) with ESMTPSA id 99EB2CD524; Sun, 22 Jan 2023 00:15:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=z3ntu.xyz; s=z3ntu; t=1674346557; bh=rxdK0Pvqp3/9pqht+d0sJXIl3kqk5gJLUoGob3Ljric=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=hgX3ZyMFfsAwFqNcvovSRvjsFLh7muu5H+1Yr7F2fRpUYYsDZh0iDQAJ7ccONMNwO MV9/Qgv5BRjZ4bU+RPrTrJ0XPIrLDVrCEimVK/1f8RF+tSxhZT/qiHby/C/SyMbVzb 9uIqhIB6hJtaqA10lKnpeFRmUHNCGvuzg+UVEHr8= From: Luca Weiss To: agross@kernel.org, andersson@kernel.org, konrad.dybcio@somainline.org, mturquette@baylibre.com, sboyd@kernel.org, mka@chromium.org, Rajendra Nayak Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, johan+linaro@kernel.org, quic_kriskura@quicinc.com, dianders@chromium.org, linux-clk@vger.kernel.org, angelogioacchino.delregno@collabora.com, Rajendra Nayak , ~postmarketos/upstreaming@lists.sr.ht Subject: Re: [PATCH v3 1/3] clk: qcom: gdsc: Fix the handling of PWRSTS_RET support Date: Sun, 22 Jan 2023 01:15:56 +0100 Message-ID: <5897497.lOV4Wx5bFT@g550jk> In-Reply-To: <20220920111517.10407-1-quic_rjendra@quicinc.com> References: <20220920111517.10407-1-quic_rjendra@quicinc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rajendra, On Dienstag, 20. September 2022 13:15:15 CET Rajendra Nayak wrote: > GDSCs cannot be transitioned into a Retention state in SW. > When either the RETAIN_MEM bit, or both the RETAIN_MEM and > RETAIN_PERIPH bits are set, and the GDSC is left ON, the HW > takes care of retaining the memory/logic for the domain when > the parent domain transitions to power collapse/power off state. > > On some platforms where the parent domains lowest power state > itself is Retention, just leaving the GDSC in ON (without any > RETAIN_MEM/RETAIN_PERIPH bits being set) will also transition > it to Retention. > > The existing logic handling the PWRSTS_RET seems to set the > RETAIN_MEM/RETAIN_PERIPH bits if the cxcs offsets are specified > but then explicitly turns the GDSC OFF as part of _gdsc_disable(). > Fix that by leaving the GDSC in ON state. > > Signed-off-by: Rajendra Nayak > Cc: AngeloGioacchino Del Regno > --- > v3: > Updated changelog > > There are a few existing users of PWRSTS_RET and I am not > sure if they would be impacted with this change > > 1. mdss_gdsc in mmcc-msm8974.c, I am expecting that the > gdsc is actually transitioning to OFF and might be left > ON as part of this change, atleast till we hit system wide > low power state. > If we really leak more power because of this > change, the right thing to do would be to update .pwrsts for > mdss_gdsc to PWRSTS_OFF_ON instead of PWRSTS_RET_ON > I dont have a msm8974 hardware, so if anyone who has can report > any issues I can take a look further on how to fix it. Unfortunately indeed this patch makes problems on msm8974, at least on fairphone-fp2 hardware. With this patch in place, the screen doesn't initialize correctly in maybe 80% of boots and is stuck in weird states, mostly just becomes completely blue. Kernel log at least sometimes includes messages like this: [ 25.847541] dsi_cmds2buf_tx: cmd dma tx failed, type=0x39, data0=0x51, len=8, ret=-110 Do you have anything I can try on msm8974? For now, reverting this patch makes display work again on v6.1 Regards Luca > > 2. gpu_gx_gdsc in gpucc-msm8998.c and > gpu_gx_gdsc in gpucc-sdm660.c > Both of these seem to add support for 3 power state > OFF, RET and ON, however I dont see any logic in gdsc > driver to handle 3 different power states. > So I am expecting that these are infact just transitioning > between ON and OFF and RET state is never really used. > The ideal fix for them would be to just update their resp. > .pwrsts to PWRSTS_OFF_ON only. > > drivers/clk/qcom/gdsc.c | 10 ++++++++++ > drivers/clk/qcom/gdsc.h | 5 +++++ > 2 files changed, 15 insertions(+) > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index d3244006c661..ccf63771e852 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -368,6 +368,16 @@ static int _gdsc_disable(struct gdsc *sc) > if (sc->pwrsts & PWRSTS_OFF) > gdsc_clear_mem_on(sc); > > + /* > + * If the GDSC supports only a Retention state, apart from ON, > + * leave it in ON state. > + * There is no SW control to transition the GDSC into > + * Retention state. This happens in HW when the parent > + * domain goes down to a Low power state > + */ > + if (sc->pwrsts == PWRSTS_RET_ON) > + return 0; > + > ret = gdsc_toggle_logic(sc, GDSC_OFF); > if (ret) > return ret; > diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h > index 5de48c9439b2..981a12c8502d 100644 > --- a/drivers/clk/qcom/gdsc.h > +++ b/drivers/clk/qcom/gdsc.h > @@ -49,6 +49,11 @@ struct gdsc { > const u8 pwrsts; > /* Powerdomain allowable state bitfields */ > #define PWRSTS_OFF BIT(0) > +/* > + * There is no SW control to transition a GDSC into > + * PWRSTS_RET. This happens in HW when the parent > + * domain goes down to a low power state > + */ > #define PWRSTS_RET BIT(1) > #define PWRSTS_ON BIT(2) > #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON)