On Mon, Mar 02, 2026 at 11:41:59AM +0100, Konrad Dybcio wrote:
> On 2/27/26 8:05 PM, Dmitry Baryshkov wrote:
> > 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 <[email protected]> 
> >>>>>> 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 <[email protected]>
> >>
> >> [...]
> >>
> >>> 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.
> 
> That's a good point. Perhaps the easiest resolution will be to leave a
> comment before the prepare_enable() explaining that this should have a
> vote, but it's easier to rely on the providers' .sync_state() keeping them
> online until the consumers fully probe.

Yep.

> 
> [...]
> 
> > 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
> 
> I have reasons to believe msm8974 v1.0 never reached store shelves.
> Let's remove this.

Please send a patch ;-)

> 
> > 1.1  - MSM8x26
> >        Probably Luca can better comment on it. Should be doable, but I
> >        don't see upstream devices using display on it.
> 
> Because there's no iommu support for these

I promised to put it on my todo list, but the list is very long.

> 
> > 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
> 
> 8936 is 39 with some CPUs fused off (unless you have info suggesting
> otherwise)

Hmm, you added 8x36 to mdp5_cfg.c, stating it is 1.8. See commit
81c4389e4835 ("drm/msm/mdp5: Add MDP5 configuration for MSM8x36.")
Author: Konrad Dybcio <[email protected]>

Please remove it from the mdp5_cfg to avoid confusion.

> 
> > 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
> 
> +Marijn, is your computer museum still running?

Should we open a Qualcomm Virtual Museum?

> 
> > 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
> 
> No upstream MDP5 support either. And it doesn't seem like that SoC had
> much uses that didn't end up with the thing glued shut..

I saw and touched devices, but that was display-less version.

> 
> > 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.
> 
> We can think about poking at some of these it one day, but certainly not
> high prio..

-- 
With best wishes
Dmitry

Reply via email to