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

[Vinod Polimera]

> -----Original Message-----
> From: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> Sent: Friday, February 25, 2022 11:43 PM
> To: quic_vpolimer <quic_vpolimer@xxxxxxxxxxx>;
> devicetree@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> freedreno@xxxxxxxxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; robdclark@xxxxxxxxx;
> dianders@xxxxxxxxxxxx; quic_kalyant <quic_kalyant@xxxxxxxxxxx>
> 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@xxxxxxxxxxx>
> > ---
> >  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)