*** BLURB HERE ***
Vinod Polimera (2): arm64/dts/qcom/sc7280: remove assigned-clock-rate property for mdp clk drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table
arch/arm64/boot/dts/qcom/sc7280.dtsi | 9 ++------- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ 2 files changed, 5 insertions(+), 7 deletions(-)
Kernel clock driver assumes that initial rate is the max rate for that clock and was not allowing it to scale beyond the assigned clock value.
drop the assigned clock rate property and set it during resume sequence with max value in the opp table.
Fixes: 62fbdce91("arm64: dts: qcom: sc7280: add display dt nodes")
Signed-off-by: Vinod Polimera quic_vpolimer@quicinc.com --- arch/arm64/boot/dts/qcom/sc7280.dtsi | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index baf1653..408cf6c 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -2856,9 +2856,6 @@ "ahb", "core";
- assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>; - assigned-clock-rates = <300000000>; - interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; interrupt-controller; #interrupt-cells = <1>; @@ -2892,11 +2889,9 @@ "lut", "core", "vsync"; - assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>, - <&dispcc DISP_CC_MDSS_VSYNC_CLK>, + assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>, <&dispcc DISP_CC_MDSS_AHB_CLK>; - assigned-clock-rates = <300000000>, - <19200000>, + assigned-clock-rates = <19200000>, <19200000>; operating-points-v2 = <&mdp_opp_table>; power-domains = <&rpmhpd SC7280_CX>;
Quoting Vinod Polimera (2022-02-25 07:57:49)
Kernel clock driver assumes that initial rate is the max rate for that clock and was not allowing it to scale beyond the assigned clock value.
drop the assigned clock rate property and set it during resume sequence with max value in the opp table.
Fixes: 62fbdce91("arm64: dts: qcom: sc7280: add display dt nodes")
There shouldn't be a newline here. The fixes tag should be directly attached to the SoB area.
Signed-off-by: Vinod Polimera quic_vpolimer@quicinc.com
arch/arm64/boot/dts/qcom/sc7280.dtsi | 9 ++-------
Can you also update sc7180.dtsi? Are there others SoCs that need an update because they're using an OPP table?
Doesn't this need to come after the driver change?
use max clock during resume sequence from the opp table. The clock will be scaled down when framework sends an update.
Fixes: 62fbdce91("arm64: dts: qcom: sc7280: add display dt nodes")
Signed-off-by: Vinod Polimera quic_vpolimer@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index d550f90..3288f52 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1319,6 +1319,7 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev) struct drm_device *ddev; struct dss_module_power *mp = &dpu_kms->mp; int i; + unsigned long max_freq = ULONG_MAX;
ddev = dpu_kms->dev;
@@ -1333,6 +1334,8 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev) return rc; }
+ dev_pm_opp_find_freq_floor(dev, &max_freq); + dev_pm_opp_set_rate(dev, max_freq); dpu_vbif_init_memtypes(dpu_kms);
drm_for_each_encoder(encoder, ddev)
Quoting Vinod Polimera (2022-02-25 07:57:50)
use max clock during resume sequence from the opp table.
s/use/Use/
The clock will be scaled down when framework sends an update.
Fixes: 62fbdce91("arm64: dts: qcom: sc7280: add display dt nodes")
Presumably this is the wrong fixes tag, see below.
Signed-off-by: Vinod Polimera quic_vpolimer@quicinc.com
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index d550f90..3288f52 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1319,6 +1319,7 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev) struct drm_device *ddev; struct dss_module_power *mp = &dpu_kms->mp; int i;
unsigned long max_freq = ULONG_MAX; ddev = dpu_kms->dev;@@ -1333,6 +1334,8 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev) return rc; }
dev_pm_opp_find_freq_floor(dev, &max_freq);
This isn't exactly a cheap thing to do every runtime resume. Please get the max frequency during probe and stash it somewhere to avoid making this call over and over again.
dev_pm_opp_set_rate(dev, max_freq);
This is entirely new. The assigned clock rates from DT aren't set during runtime resume, only during device probe. My question is, if this is needed now then does it mean we've been running the clk at low speed after the first runtime suspend and never been pushing it back up again?
dpu_vbif_init_memtypes(dpu_kms); drm_for_each_encoder(encoder, ddev)
-----Original Message----- From: Stephen Boyd swboyd@chromium.org Sent: Friday, February 25, 2022 11:43 PM To: quic_vpolimer quic_vpolimer@quicinc.com; devicetree@vger.kernel.org; dri-devel@lists.freedesktop.org; freedreno@lists.freedesktop.org; linux-arm-msm@vger.kernel.org Cc: linux-kernel@vger.kernel.org; robdclark@gmail.com; dianders@chromium.org; quic_kalyant quic_kalyant@quicinc.com Subject: Re: [PATCH v2 2/2] drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table
WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
Quoting Vinod Polimera (2022-02-25 07:57:50)
use max clock during resume sequence from the opp table.
s/use/Use/
The clock will be scaled down when framework sends an update.
Fixes: 62fbdce91("arm64: dts: qcom: sc7280: add display dt nodes")
Presumably this is the wrong fixes tag, see below.
Signed-off-by: Vinod Polimera quic_vpolimer@quicinc.com
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index d550f90..3288f52 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1319,6 +1319,7 @@ static int __maybe_unused
dpu_runtime_resume(struct device *dev)
struct drm_device *ddev; struct dss_module_power *mp = &dpu_kms->mp; int i;
unsigned long max_freq = ULONG_MAX; ddev = dpu_kms->dev;@@ -1333,6 +1334,8 @@ static int __maybe_unused
dpu_runtime_resume(struct device *dev)
return rc; }
dev_pm_opp_find_freq_floor(dev, &max_freq);This isn't exactly a cheap thing to do every runtime resume. Please get the max frequency during probe and stash it somewhere to avoid making this call over and over again.
dev_pm_opp_set_rate(dev, max_freq);This is entirely new. The assigned clock rates from DT aren't set during runtime resume, only during device probe. My question is, if this is needed now then does it mean we've been running the clk at low speed after the first runtime suspend and never been pushing it back up again?
mdp clock rate is set based on the calculated value for each update and does not require to set the clock to max value in opp table. So, dropping this patch.
dpu_vbif_init_memtypes(dpu_kms); drm_for_each_encoder(encoder, ddev)
dri-devel@lists.freedesktop.org
-
Stephen Boyd -
Vinod Polimera -
Vinod Polimera