diff mbox series

[v2,2/2] drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table

Message ID 1645804670-21898-3-git-send-email-quic_vpolimer@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Update mdp clk to max supported value to support higher refresh rates | expand

Commit Message

Vinod Polimera Feb. 25, 2022, 3:57 p.m. UTC
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(+)

Comments

Stephen Boyd Feb. 25, 2022, 6:12 p.m. UTC | #1
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)
Vinod Polimera March 3, 2022, 8:21 a.m. UTC | #2
> -----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)
diff mbox series

Patch

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)