Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
> > On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> >> On 1/12/26 9:25 AM, yuanjiey wrote:
> >>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@xxxxxxxxxxxxxxxx> wrote:
> >>>>>
> >>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >>>>>>> From: Yuanjie Yang <yuanjie.yang@xxxxxxxxxxxxxxxx>
> 
> [...]
> 
> > Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
> > dpu_runtime_suspend, then we should be able to also skip setting OPP
> > corner in dpu_runtime_resume(), because the previously set corner should
> > be viable until drm/msm driver commits new state / new modes.
> 
> That matches my understanding.
> 
> > The only important issue is to set the corner before starting up the
> > DPU, where we already have code to set MDP_CLK to the max frequency.
> > 
> > Which means, we only need to drop the dev_pm_set_rate call from the
> > dpu_runtime_suspend().
> 
> I concur.
> 
> >> For MDSS, we're currently generally describing the MDSS_AHB clock, the
> >> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
> > 
> > No. As far as I remember, MDP_CLK is necessary to access MDSS registers
> > (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
> > platforms")), I don't remember if accessing HW_REV without MDP_CLK
> > resulted in a zero reads or in a crash. At the same time it needs to be
> > enabled to any rate, which means that for most of the operations
> > msm_mdss.c can rely on DPU keeping the clock up and running.
> > 
> >> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
> >> MDSS_AHB one seems to have 3 actually configurable performance points
> >> that neither we nor seemingly the downstream driver seem to really care
> >> about (i.e. both just treat it as on/off). If we need to scale it, we
> >> should add an OPP table, if we don't, we should at least add required-opps.
> > 
> > I think, dispcc already has a minimal vote on the MMCX, which fulfill
> > these needs.
> 
> I have slightly mixed feelings, but I suppose that as we accepted Commit
> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
> we can generally agree that it makes sense that calling genpd->on() actually
> turns on the power indeed
> 
> What I'm worried about is if the clock is pre-configured to run at a high
> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
> and doesn't impact the state of M/N/D at a glance), we may get a brownout
> 
> This rings the "downstream really did it better with putting clock dvfs states
> into the clk driver" bell, but I suppose the way to fight this would be to
> simply set_rate(fmax) there too..
> 
> I attempted an experiment with pulling out the plug. MMCX enabled with the
> AHB clock off results in a read-as-zero. I tried really hard to disable the
> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
> *really* can't just disable it" type behavior (verified with debugcc)

I think, in 8996 it was possible to disable it. Not sure about
8998/630/660.

> 
> 
> There's a possible race condition if we don't do it:
> 
> ------- bootloader --------
> configure display, mdp_clk=turbo
> ------- linux -------------
> load rpmhpd     |
> load venus      |
> set mmcx=lowsvs | mdp_clk is @ turbo
>                 | brownout
>                 | 
>                 | <mdss would only probe here>
> 
> *but* that should be made impossible because of .sync_state().

Yep, sync_state should prevent MMCX or CX from dropping under the boot
level.

> 
> This may impact hacky setups like simplefb, but as the name implies,
> that's hacky.
> 
> Relying on .sync_state() however will not cover the case if the mdss
> module is removed and re-inserted later, possibly with mmcx disabled
> entirely but the clock not parked at a sufficiently low rate.
> 
> 
> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
> plug the MDP opp table into it and set_rate(fmax) during mdss init

And what will drop it afterwards? MDSS will still vote on the MMCX / CX
level even though DPU will change the clock freq.

> 
> >> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
> >> targets supported by these may not even have OPP tables and are generally
> >> not super high priority for spending time on.. that can, I'd kick down the
> >> road unless someone really wants to step up
> > 
> > I'd really not bother with those two drivers, unless we start seing
> > crashes. For MDP4 platforms we don't implement power domains at all, no
> > interconnects, etc., which means that the system will never go to a
> > lower power state.
> 
> Right. Might be that 2030-something arrives and armv7 is gone before someone
> randomly decides to work on that!
> 
> > MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
> > SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
> > has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
> > should start there by adding missing bits adding corresponding
> > dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
> 
> A bit off-topic, but:
> 
> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> 
> 96 is in DPU. any other candidates that should be moved over?

Let's go through them.

All SoC except those currently supported in DPU require SMP (shared
memory pool) support to be ported from the MDP5 driver.

Most of the remaining platforms (except MSM8994/92) also had HW cursor
implemented in a fancy way, in the LM rather than in a separate pipe.
I'd really like to postpone those, possibly first completing migration
of the other platforms and dropping support for them from MDP5.

1.0  - old MSM8974
       I'd rather not touch it, it had bugs and I don't have HW
1.1  - MSM8x26
       Probably Luca can better comment on it. Should be doable, but I
       don't see upstream devices using display on it.
1.2  - MSM8974
       I think it also had issues, no IOMMU support in upstream, etc.
1.3  - APQ8084
       Had hw issues, no testing base, no MDSS in upstream DT
1.6  - MSM8916 / MSM8939
       Can be done, low-hanging fruit for testing
1.7  - MSM8996
       Supported in DPU
1.8  - MSM8936
       No upsteram testing base
1.9  - MSM8994
       No upstream testing base, no MDSS in upstream DT, normal CURSOR planes
1.10 - MSM8992
       Even less testing base, no MDSS in upstream DT, normal CURSOR planes
1.11 - MSM8956 / 76
       No complete display configurations upstream
1.14 - MSM8937
       Supported in DPU
1.15 - MSM8917
       Supported in DPU
1.16 - MSM8953
       Supported in DPU
1.17 - QCS405
       Zero testing base, no MDSS in upstream DT

MSM8994/92 would have been an ideal testbeds for SMP testing, but...
they mostly don't exist (please correct me if I'm wrong). Which means
that the next viable targets are MSM8916, MSM8x26 and MSM8956/76. All of
them require SMP support and don't make sense without cursor handling.

> > A note to myself to also add OPP tables support to the HDMI driver.
> 
> Eliza!

MSM8996 / 98!

-- 
With best wishes
Dmitry