<Marijn[m]>
narmstrong: since you mentioned INTF TE registers being sane, you were testing on sm8550 right? That means you must've added the necessary 7xxx interrupts and other bits to the catalog. I've just done the same while rebasing before starting to work on Dmitry's review, and pushed the intermediary result at https://github.com/SoMainline/linux/commits/marijn/dpu. Can you check that it's literally "as simple as that"? Meanwhile I'll look at
<Marijn[m]>
konradybcio for running the fresh DPU additions on our sm8350 and sm8450 devices, which'll be harder to test this on as DSC 1.2 is mandatory for our panels (and doubt we have have a manual to know what panel registers to poke to turn it off while forcing lower resolution/fps...)
swdefrgthfgjhk has quit [Remote host closed the connection]
swdefrgthfgjhk has joined #linux-msm
Danct12 has joined #linux-msm
marvin24_ has joined #linux-msm
marvin24 has quit [Ping timeout: 480 seconds]
Danct12 has quit [Quit: Quitting]
<narmstrong>
Marijn[m]: yep i added the 7xxx interrupts, I will check if what you did matches
BobBeck has quit [Quit: Ping timeout (120 seconds)]
sergi has quit [Quit: Ping timeout (120 seconds)]
kholk has quit [Quit: Ping timeout (120 seconds)]
BobBeck has joined #linux-msm
<lumag>
aka_[m], could you please check the UBWC_DEC_HW_VERSION value on sm6115? See the dev_dbg in msm_mdss.c
<lumag>
konradybcio, Marijn[m] if you have sm6115 at hand maybe you can do that ^^
<lumag>
bamse, robclark ^^ same question about sc8180x, if you have time
<lumag>
I assume that it should be 0x300000, but if you can check it, that would be great
sergi has joined #linux-msm
kholk has joined #linux-msm
pevik_ has joined #linux-msm
sergi has quit []
sergi has joined #linux-msm
<Marijn[m]>
lumag: I only have sm6125, which has `UBWC_DEC_HW_VERSION: 0x30000001`, and downstream denotes UBWC version 1.0, so I call `msm_mdss_setup_ubwc_dec_30(... UBWC_1_0 ...)` to add `BIT(8)` into the static value
<Marijn[m]>
lumag: any idea why we don't base this setup_uwbc_dec_xx call (or at least validation of it) on the value read back from that register?
<lumag>
Marijn[m], ack. I'll note this, I'm just trying to doublecheck 6115 since bengal-sde.dtsi lists ubwc 1.0, while aka_[m] listed it as 2.0
<lumag>
Marijn[m], because we need other data anyway.
<Marijn[m]>
Besides the non-UBWC hw version already uniquely specifying exactly what UBWC decoder would be used together with the other parameters
<Marijn[m]>
lumag: perhaps there was a mixup between DPU HW version, UBWC decoder version, and UBWC (the format, iirc) version
<lumag>
I've sent an RFC to use this reg + platform data. abhinav__ suggested moving ubwc_dec_version to platform data too.
<lumag>
So as soon as I can verify 6115, I'll send v2
<Marijn[m]>
Saw the RFC, will have to look a little more closely at it but it sounded interesting
<Marijn[m]>
lumag: just replied to your patch; it has some benefits but also some drawbacks...
<aka_[m]>
I mentioned it to you back in time
<aka_[m]>
And that's where this reset of 0x11f? Came from
<aka_[m]>
Tho I noticed there are diferent kinds of ubwc version
<aka_[m]>
One for mdp and other for sspp
<Marijn[m]>
lumag: ugh, that downstream code also makes it really hard to read with local ubwc_version variable (really, HW decoder version) and m->ubwc_version field (really, UBWC format version per DT)
<aka_[m]>
These vigs and stuff I don't understand it much
<lumag>
there is ubwc version and there is the ubwc_dec_version
<aka_[m]>
Yea
<Marijn[m]>
lumag: if only they had renamed the variable as such ;)_
<aka_[m]>
On sm6115 these differs a bit
<lumag>
Marijn[m], yeah, while reworking the msm_mdss I spent quite some time doublechecking that I understand this piece of code correcly
<lumag>
aka_[m], ??
<aka_[m]>
One is like 0x200001 and other was like 0x2000003
<aka_[m]>
In job no access to flat will double check once get home
<lumag>
aka_[m], thank you
<narmstrong>
Marijn[m]: so I tested with disabling the mdp_vsync function on TE pinctrl state, and it's the same
<narmstrong>
so it means the TE signel isn't received
<narmstrong>
and with 0xff0 I have ~98.72Hz with modetest -v
<narmstrong>
now I'l check if I have actual a TE signal toggling
<Marijn[m]>
Ack that should be good then, let's see if the SoC can see the pin changing value
<Marijn[m]>
You might even have magic DPU debug registers for this again...
<narmstrong>
OK with `gpiomon --rising-edge --line-buffered --num-events=2 gpiochip10 86` I get the period between edge, and it roughly gives me 144Hz
<narmstrong>
so the event is sent from the panel
marvin24_ has quit [Ping timeout: 480 seconds]
<Marijn[m]>
narmstrong: ah cool, I've only recently started learning about gpiod userspace, that's much better than hacking in an extra interrupt on the kernel side 😬
<Marijn[m]>
narmstrong: cannot use that command when it's used via `disp-te` as GPIOD_IN in dsi_host though, even more reasons to just skip that DT property :(
<Marijn[m]>
narmstrong: so maybe something is missing on how I configure TE in the INTF block? But you diffed registers already; anything else (outside of the TE block, such as binding things via active ctl) that's off?
<narmstrong>
Marijn[m]: I’ll recheck the registers
<bamse>
lumag: do you have an address i can pass to devmem on my running machine?
<aka_[m]>
for sspp it appears to preprogram indeed like you do in your patch
<aka_[m]>
whatever.
<aka_[m]>
hope it can be sorted at the end
<aka_[m]>
for now using v2 does not give any artifacts or anything
<aka_[m]>
i havent got touch so haven't progressed very far
<aka_[m]>
Isn't anyone @linaro working on nt36xxx SPI driver by any chance?
<lumag>
aka_[m], heard nothing about that. Maybe somebody from pmos or somainline?
<aka_[m]>
for i2c maybe but not spi
<aka_[m]>
its dumber and require fw loading
<aka_[m]>
konradybcio: are you going to send xo_board for msm8976?
<aka_[m]>
lumag: from pmos i see guys imported ds but that require hacking on DRM.
<aka_[m]>
Is there any support for panel events?
<aka_[m]>
DS does something like we have for FB with events like DRM_EVENT_EARLY_UNBLANK and others
<konradybcio>
aka_: uh have I done anything?
<aka_[m]>
these spi ts modules are pretty fucked as they require some fw loading to start after panel gets brought up, also some dumb shit like sharing reset gpio with display
<lumag>
aka_[m], ds?
<aka_[m]>
konradybcio: i haven't seen
<aka_[m]>
lumag: downstream
<lumag>
ah
<konradybcio>
(asking about xo)
<aka_[m]>
konradybcio: well i asked you about then you said its better to keep xo per board and then i got no idea what to even do so i ask if you are going to send anything as you know there is an issue
<konradybcio>
what I meant i that we now store the clock-frequency of &xo_board in board device trees
<konradybcio>
that's to represent the fact that the physical crystal is on the board and not on the SoC
<bamse>
lumag: sshed into my sc8180x, it read as 0x1...then i woke the display and it says 0x30000001
<bryanodonoghue>
konradybcio I mean to say to you about the msm8939 review <&rpm xo> is not a thing because xo is a crystal not an RPM clock source
<aka_[m]>
its not like i can put xo definition inside board dts and reference it in msm8976.dtsi
<bamse>
lumag: i was slightly surprised that it didn't fail when i did it before waking the display
<konradybcio>
bryanodonoghue: well.. rpm provides an xo clock for PM purposes
<bamse>
lumag: was hoping it to be runtime suspended deep enough for that to be a problem
<bryanodonoghue>
GPLL0 according to my clock tree
<lumag>
heh. It should have failed, underclocked MDP_CLK
<aka_[m]>
xo is tricky isnt it?
<bryanodonoghue>
derived from xo
<aka_[m]>
i mean we provide it to lets say wcnss
<lumag>
bamse, as you see, it was
<aka_[m]>
and pretty often XO is separate crystal
<aka_[m]>
in wcn circuit
<bamse>
lumag: ah, but it has displayport enabled...so we're clocking the whole thing, just in case someone needs hpd-handling
<bamse>
internal hpd-handling i mean...the thing that we don't use
<konradybcio>
bryanodonoghue: please provide more context, I'm not sure we're talking about the same thing
<bamse>
lumag: if we fix that and power down dp, perhaps it would crash instead (or give me a Bus error, if i'm lucky)
<bryanodonoghue>
you said "try <&rpm xo>"
<bryanodonoghue>
instead of xo_board
<bryanodonoghue>
but xo_board points to a literal 19.2 MHz crystal
<lumag>
Ugh. Weren't we providing xo_board as a part of SoC.dtsi since forever/
<lumag>
?
<bryanodonoghue>
should be
<konradybcio>
bryanodonoghue: yes and there's also `&rpmcc RPM_SMD_XO_CLK_SRC`
<bryanodonoghue>
ah I see what you are getting at
<konradybcio>
this "rpm XO" holds votes, when there are none, XO shutdown occurs and you can enter LPM modes
<bamse>
lumag: no, we provide "xo-board" or "xo-board-clk" in most soc.dtsi
<konradybcio>
XO shutdown concerning the on-chip crystals
<bamse>
not xo_board
<lumag>
bamse, whatever
<bamse>
lumag: that - vs _ makes a difference ;)
<konradybcio>
xo-board is the software representation so that rpmcc and friends get a reference rate
<lumag>
bamse, <&xo_board> ;-)
<aka_[m]>
lumag: it was like that
<aka_[m]>
but lately there is requirement
<aka_[m]>
and now like you define it inside soc dtsi but add rate per device
<aka_[m]>
its always 19.2Mhz tho
<bryanodonoghue>
on chip GPLLs surely
<bamse>
aka_[m]: it's not been 19.2MHz in years :)
<bamse>
aka_[m]: but it's never going to change for a given soc
<aka_[m]>
unless some different value
<aka_[m]>
like 7150 have 3xxMhz
<aka_[m]>
bamse: im peasant with ancient hardware so its for me
<konradybcio>
bryanodonoghue: there's things like GCC_XO etc
<konradybcio>
they are handled by RPM or otherwise internally
<aka_[m]>
and real xo is 38.4 tho
<bamse>
aka_[m]: 2*19.2MHz... some newer boards come at 4 * 19.2
<bryanodonoghue>
hrmm but if the downstream lock tree points to xo meaning a 19.2 MHz crystal why would I want to repoint to GCC_XO..
<bryanodonoghue>
dts should be a representation of the hardware
<bryanodonoghue>
as krzyk is fond of telling me
<bryanodonoghue>
daily
<bamse>
bryanodonoghue: that would then be because the physical chrystal isn't represented
<bryanodonoghue>
maybe I'm slow
<aka_[m]>
crystal(38.4)->RPM(divide by2)->GCC
<bamse>
bryanodonoghue: the "xo" there meeans the clock pin on the pmic that is feeding the cxo input signal on the soc
<bryanodonoghue>
doesn't xo_board represent the crystal
<lumag>
bryanodonoghue, I thought so.
<bamse>
bryanodonoghue: xo_board is the chrystal, feeding rpmcc, which then gives you a number of different clock signals
<lumag>
As it's an input to rpmhcc
<aka_[m]>
crystal driver when
<bamse>
bryanodonoghue: in that sense, "xo" refers to the clock signal that comes into the clock controllers in the soc...
<bamse>
bryanodonoghue: and the means of shutting it down is by gating it in the pmic
<lumag>
bamse, you mean <&rpm xo> ?
<bamse>
lumag: yes
<lumag>
ack
<bryanodonoghue>
so you are saying
<bryanodonoghue>
rpm provides *another* xo as input to gpll0
<bryanodonoghue>
which can be shut down ?
<bryanodonoghue>
separate to xo_board
<bamse>
&xo_board -> rpmhcc, which produces all those RF_CLK and BB_CLK etc...one of those is &rpmhcc CXO and that feeds the CXO input pin on the soc, which then is named bi_tcxo inside the soc...which is the xo clock fed into each clock controller
<bryanodonoghue>
which is the physical input
<bryanodonoghue>
form the crystal
<bryanodonoghue>
from
<bamse>
bryanodonoghue: right, &rpm(h)cc CXO is a "gate" between &xo_board and the "xo" input clock in e.g. &gcc
<bamse>
bryanodonoghue: "gate" as per the rpmhcc driver it also does div/2 of the input signal
pevik_ has quit [Ping timeout: 480 seconds]
<bryanodonoghue>
MSM8936/MSM8939 Clock Plan
<aka_[m]>
bryanodonoghue: well yes?
<aka_[m]>
in reality it should be RPM_SMD_XO_CLK_SRC which we provide to gcc
<bryanodonoghue>
section 2.1
<bryanodonoghue>
osctillators used
<aka_[m]>
there is also RPM_SMD_XO_A_CLK_SRC which is often provided for AO GPLL clocks
<bryanodonoghue>
crystal XO 19.2 MHz
<bryanodonoghue>
section 2.2
<bryanodonoghue>
PLL configuration
<bryanodonoghue>
GPLL0 type = SR source = XO
<bryanodonoghue>
not CXO
<bryanodonoghue>
all of the PLLs define XO as the source
<bamse>
that's the "XO" your documentation refers to
<bamse>
it comes from the signal BBCLK1, in PM8916 on page 16
<bamse>
which is sourced from the XTAL19M2_IN pin (that's your &xo_board) and controlled by BBCLK1_EN - which in software is what we refer to as <&rpmcc CXO>
<bryanodonoghue>
so none of our board_xo should be there then?
<bryanodonoghue>
xo_board
<bamse>
bryanodonoghue: &rpmcc CXO is represented as a gate, so &xo_board is the thing that tells the clock framework what rate BBCLK1 has
<bamse>
bryanodonoghue: but no one other than rpmcc should reference &xo_board
<bryanodonoghue>
yes ok that adds up
<bamse>
bryanodonoghue: it was upstreamed that way because we didn't have the means of describing the chain...and it wasn't a problem until someone was going to actually gate cxo
<bamse>
bryanodonoghue: now we have the means, and the interest in gating cxo :)
<bryanodonoghue>
who is gating cxo ?
<bryanodonoghue>
:)
<bamse>
you? ;)
<bryanodonoghue>
don't tempt
<aka_[m]>
<bamse> "bryanodonoghue: now we have..." <- obv for power saving, but have anyone managed xo shutdown and these vdd minimization on something older like 8916?
<bamse>
bryanodonoghue: jokes aside, are you able to hit that point on 8939? or you still have cxo votes?
<konradybcio>
bryanodonoghue: if you don't vote on anything more than vdd_min, it gates itself
<bamse>
konradybcio: are you saying that unless someone request a non-zero performance state on vdd_cx it will but shut down automagically?
<konradybcio>
bamse I thought it worked liked that?
<konradybcio>
but yes i am saying that
<konradybcio>
not sure if i'm correct but i am saying that
<aka_[m]>
so if we want to make it ded we remove votes?
<aka_[m]>
and how we do that?
<bamse>
konradybcio: okay, i fixed that in rpmhpd...that's why i'm asking
<konradybcio>
"fixed that"?
<konradybcio>
i thought it was the expected behavior?
<bamse>
konradybcio: because i had several cases where we assumed that saying power-domains = <some-pd> would actually mean that as the driver probed some-pd would be powered
<lumag>
so I preferred the explicit required-opps <&svs> in such case, while bamse preferred an implicit one :-)
<bamse>
konradybcio: my expectation of the genpd model is that if you say pm_runtimme_get() you will have some power
<konradybcio>
right that's what one would expect
<bamse>
lumag: at the time we didn't have the means of specifying required-opps...but that would have worked as well
<lumag>
bamse, I think they were flowing in at the same moment :-)
<konradybcio>
I'm not sure if this actually happens on linux right now, as I usually attach some power domain to some consumer in early bringup
<bryanodonoghue>
do I have cxo votes ?
<bamse>
konradybcio: so in rpmhpd i changed it such that enable != disabled
<bryanodonoghue>
I mean I'm focused on bringing things up
<bryanodonoghue>
not necessarily tearing them down
<bryanodonoghue>
we do hit vdd_min in suspend AFAIK
<konradybcio>
bamse: I don't know what you mean by "enable != disabled", please clarify
<lumag>
Strange question. Is it possible to have opp entries with the same freq if the supported-hw is different?
<bryanodonoghue>
or we could prove it via some kind of counters reported in the idle patches
<konradybcio>
bryanodonoghue: you can check sleep stats in debugfs if you have the node.. otherwise copy it from 8916
<bryanodonoghue>
but we are well off of that in usptream for obviosu reasons
<bamse>
konradybcio: in rpmhpd if you ended up in ops->enable without first calling set_performance_state...we would vote for 0 (i.e. disabled)
<konradybcio>
you can then call suspend, wake up an check if it entered vddmin
<bamse>
konradybcio: or rather, we would not send that vote
<konradybcio>
bamse: oh that sounds rather counter-intuitive :D
<bryanodonoghue>
konradybcio I think we will go nowhere near suspend
<bryanodonoghue>
!PSCI
<bryanodonoghue>
and
<bamse>
konradybcio: slightly...because as i said, i have specified power-domains and i have called pm_runtime_get()...so why isn't the damn thing on
<bryanodonoghue>
!qcom-idle patches
<bryanodonoghue>
on upstream
<konradybcio>
bryanodonoghue: the rpm sleep stats will confirm that for you
<bamse>
bryanodonoghue: i like that...i would prefer such effort being spent on a modern target...
<bryanodonoghue>
I was about to say we should convince stephan to write an open source psci
<aka_[m]>
define modern
<vknecht[m]>
8939 is a fine target :-)
<aka_[m]>
bryanodonoghue: then call qcom and all vendors to sign his firmware so we can update one mil of different devices
<bamse>
bryanodonoghue: we have functional cluster idle, and we know of some targets where cxo shutdown and vdd_min has been achieved...but we have plenty of drivers not doing the right thing either pm runtime wise or system suspend wise
<bamse>
aka_[m]: >= 845
<lumag>
aka_[m], msm8974+ ;-)
<lumag>
But jokes aside, I'd second bamse.
<minecrell>
bryanodonoghue: I do have partial PSCI for 8939 but it doesn't implement cluster idle either :p
<aka_[m]>
damn i don't even own stuff like that.
<aka_[m]>
most modern stuff i have is 662
<aka_[m]>
lumag: 8974 included or not
<lumag>
included
<konradybcio>
included
<aka_[m]>
i mean A53 socs are still in use by many
<konradybcio>
bryanodonoghue: I believe ATF should have an OSS implementation
<konradybcio>
also bamse as you probably know, there's still new socs being made with smd rpm :/
<aka_[m]>
680 is like latest
<konradybcio>
what's that in normal names
<konradybcio>
6225?
<aka_[m]>
yea
<aka_[m]>
wait this 6375
<konradybcio>
6375 sounds newer
<konradybcio>
also smd
<aka_[m]>
isnt that too?
<aka_[m]>
ah, i don't think so
<bamse>
konradybcio: i know...and i don't think there's a problem there...
<aka_[m]>
should be simillar age on qcom chart
<bamse>
konradybcio: psci is probably the bigger thing
<bamse>
aka_[m]: i'm not against people trying to figure out cluster idle etc on older chipsets...but we have cluster idle on >= 845...it would be nice to see more of the system doing runtime and system suspend/resume as well
<minecrell>
konradybcio: ATF has an OSS implementation of what?
<konradybcio>
psci interface
<minecrell>
well that is the easy part
<minecrell>
hooking it up to the qcom stuff is the hard one :p
<lumag>
bamse, I have looked onto the cluster idle on 8996. Then, guess what? The firmware tells that it supports going to OSI mode. But then setting OSI mode returns an error. Well. On 8996 even setting the PC mode (the default) returns an error. Ok, downstream programs domain idle values without setting the mode at all! Quite a surprise. And the final surprise, after which I put the doman-idle on hold was when I discovered that even
<lumag>
without using cluster-idle, firmware would switch the cluster off if I switch individual cores off. Maybe using the cluster idle would allow deeper sleep state, but I didn't bother to check that yet.
<minecrell>
bamse, bryanodonoghue: FWIW: I'd be surprised if the XO shutdown stuff works on 8916/8939 and similar because there are several things that currently keep voting for XO permanently:
<minecrell>
If konradybcio is right that CXO is also gated with just voting for vdd_min everywhere then I don't understand why it's necessary to vote for the XO clock in RPM at all...
<minecrell>
1. CPU -> GPLL0 -> XO - This should be a XO_A (active-only) vote but I'm not sure how that could be properly represented in the GCC clock tree...
<minecrell>
The second one is particularly great, because having a full clock tree was once considered a good thing. Similar clocks on newer platforms don't have a parent defined anymore, so they don't end up voting for XO...
<minecrell>
2. When mpss is on the driver keeps the "iface"/"bus"/"mem" clocks permanently on as an "active_clk". Since 8916/8939 have a full clock tree in their GCC driver all of them also end up voting for XO permanently.
<konradybcio>
minecrell: I believe both no votes on rpmxo and no votes above vddlow is a requiremenet
<minecrell>
ah
<konradybcio>
+ MPM plays a role in the device waking up correctly from the deepest sleep states
<minecrell>
well, no rpmxo votes won't happen because of what I wrote above :/
<bamse>
minecrell: wouldn't voting for cxo and voting for cx be two separate things?
<konradybcio>
and we support mpm, but nobody wired it up yet
<minecrell>
bamse: yes, maybe I misunderstood what konradybcio wrote
<bamse>
minecrell: and yes we did consider a fully represtned clock tree to be a nice thing...but it means that all the clients across the tree needs to let go of their votes (and i don't know if that's enough...)
<konradybcio>
minecrell: I think these interconnect clocks don't really belong in gcc..
<konradybcio>
downstream used to include them in 8974 times and register them with rpmcc from there, but I think that was a get-things-done-quickly-after-migration kind of design decision
<minecrell>
bamse: I don't think the qcom_q6v5_mss driver for example can release the iface/bus/mem clock. It looks like it's supposed to keep them ungated for mpss forever and then mpss will probably just vote for XO itself to make sure those clocks are actually being "supplied"
<minecrell>
because the qcom_q6v5_mss doesn't know when mpss is active or not
<bamse>
minecrell: then the driver is broken, it should unprepare when the handover interrupt comes
<minecrell>
bamse: it does that for the proxy clks
<minecrell>
but these are "active" clks
<minecrell>
which are held as long as the remoteproc is up
<bamse>
minecrell: if that's the case, then as you say, we would never be able to shut down xo...and downstream can do that
<bamse>
minecrell: so per your statement, that doesn't sound right
<minecrell>
bamse: downstream doesn't have the clock tree, those clocks don't have a parent there
<bamse>
minecrell: question is if the modem will request xo and rely on apps to leave that clock ungated...or what's going on
<minecrell>
bamse: yes, I think that's exactly how it works
<bamse>
minecrell: if so, then we should detach that clock in the clock tree in linux...
<bamse>
minecrell: not that i like it...but it has to be like that...
<minecrell>
bamse: yeah, probably. The CPU problem is more tricky as you actually need the parent there to calculate rates correctly etc
<konradybcio>
voting on XO_A should be completely fine, as it's only taken into account when the platform is not sleeping
<minecrell>
bamse: downstream has the gpll0 clock twice in the GCC driver, once GPLL0 -> CXO and once as GPLL0_A -> CXO_A (active-only in RPM)
<minecrell>
but iirc that doesn't translate well to mainline
<konradybcio>
cpufreq_hw for example, consumes the _a clock
<minecrell>
konradybcio: yeah the problem is that you need gpll0 twice in the GCC driver, as it's used by the CPU but also other stuff
<bamse>
minecrell: that's how it was decided to handle active-only clocks upstream...
<bamse>
minecrell: but i don't know how we would deal with that in the clock drivers...
<konradybcio>
minecrell: can you elaborate more?
<konradybcio>
I think some socs had a gpll0_ao path..
<bamse>
konradybcio: but i believe it's only "enabled" that should be affected by ao or not...the rest of the configuration needs to be shared
<konradybcio>
if by "the rest of the configuration" you mean frequency and related pll properties, doesn't CCF just take the highest vote?
<minecrell>
konradybcio: But note how they have different parents: the normal gpll0 has xo_clk_src as parent, the gpll0_ao has xo**_a**_clk_src as parent
<aka_[m]>
wasnt it like AO was same like standard but had some different vote type?
<minecrell>
On mainline there is just one GPLL0
<minecrell>
The "soft_vote" functionality doesn't exist iirc
<minecrell>
I suppose similar to the modem clock problem the only solution is again to mess up the real clock tree and implement something like this soft vote mechanism so that there can be two variants of the gpll0 clock
<aka_[m]>
tho its alpha pll
<minecrell>
aka_[m]: looks like the same situation otherwise though, doesn't really matter if it's an alpha pll or whatever the previous one was called
<bamse>
konradybcio: so what if you represent gpll0 and gpll0_ao separately and the respective trees built from there results in different "highest vote"?
<aka_[m]>
minecrell: it appears to be for FSM
<aka_[m]>
whatever it is
<bamse>
konradybcio: clock A with parent gpll0, clock B with parent gpll0_ao...client a votes for A at 100MHz, then client b vots for B at 10MHz...what should the hardware state be?
<konradybcio>
bamse: yeah no I skipped over the part that we can't just parent them to one another because they'll never sleep
<minecrell>
fwiw the msm8916 gplls are all supposed to have fixed rates so conflicting rates are no problem there
<minecrell>
lumag: does 8996 have no XO shutdown?
<konradybcio>
all smd rpm socs (should) do
<lumag>
minecrell, no idea, never checked that
<minecrell>
lumag: because you claim that a PLL is always-on :D
<lumag>
minecrell, patches and RFT are welcome ;-)
<lumag>
Seriously, I'd prefer to fix and finish the regulators/CPR first
<aka_[m]>
cpr on which soc?
<konradybcio>
96
<minecrell>
lumag: This looks like exactly the same situation that we just discussed for MSM8916 here
<aka_[m]>
thats 3/4 mixture
<lumag>
aka_[m], no, cpr3 only
<minecrell>
you'd need a gpll0_ao (active-only) variant that ends up as a vote for the RPM_CXO_A_CLK
<aka_[m]>
uh 8953 appears to use mixture of 3/4
<minecrell>
and you'd probably like this even more because you don't just need to wait for gcc to probe but also rpmcc :D
<lumag>
konradybcio, no, CCF doesn't have votes. You just set the rate, that's it. So if two drivers sets two different rates, one of them is going to be surprised
<lumag>
minecrell, I'm flushing out the patches that at least make 8996 boot in a stable manner, XO shutdown is, I fear, miles away.
<minecrell>
lumag: yeah I understand perfectly
<minecrell>
all the low power stuff is incredibly frustrating since so much work is needed to make it work properly
<minecrell>
and if you just do half of the job you gain almost nothing :/
<lumag>
(and unfortunately not that important for my case since my hw is wall-powered)
<lumag>
And regarding the gpll0/etc, well. our CPU/CBF clocks are also marked as always-on.
<minecrell>
you do need an "always-on" vote for CXO_A, since the _A/active-only part will tell the RPM to only respect it while the CPU is active
<minecrell>
but you probably don't want an "always-on" vote for the CXO (without _A) as this would probably break the XO shutdown power saving thing
<z3ntu>
Btw regarding the XO stuff I checked Fairphone 4 schematics and it indeed has a crystal on the board itself, not part of SoC "Crystal,76.8 MHz" with part number 7CG07680A00 that's connected to PMK8003 there
<aka_[m]>
z3ntu: bamse already told on newer its 4*19.2
<minecrell>
z3ntu: did anyone claim that the crystal is part of the SoC? :D
<z3ntu>
no but I never understood the XO situation before the explanation from bamse
<minecrell>
ah
<bamse>
minecrell: i do claim that it should be in the soc.dtsi...because it will never change between devices on that soc
<minecrell>
bamse: you mean the XO fixed-clock?
<bamse>
minecrell: yes
<minecrell>
bamse: I agree, but having just the clock-frequency in the device part is even more misleading imo. It doesn't help to understand that the crystal is not part of the SoC. But it suggests that it's common to have devices with different XO frequency. I'd count that as close to impossible
<minecrell>
I think I commented that somewhere when k.r.z.k made the suggestion of moving just clock-frequency to the device dts
<bamse>
minecrell: yeah, i would like us to revert on that idea
<bamse>
minecrell: but there's no point in dancing it around until someone document the decision and write some tooling to help people get it right
<minecrell>
bamse: I think you would mostly need to convince k.r.z.k :)
<bamse>
minecrell: perhaps my issue is that i see it as platform.dtsi and not soc.dtsi...
<bamse>
minecrell: in the end i don't care as long as it clear and there's a tool that will tell me if i do it right
<bamse>
minecrell: perhaps we should move &sleep_clk to the pmic.dtsi as well? ;)
<minecrell>
bamse: Is it "generated" ("derived" is probably better) in the PMIC on all the qcom platforms?
pespin has quit []
<lumag>
bamse, two which pmic? ;-)
<lumag>
s/two/to
<minecrell>
lumag: the one that generates it preferably :p
<minecrell>
or are there multiple sleep clocks when you have multiple pmics?
<lumag>
bamse, I also saw the SoC.dtsi as some kind of platform-core, but then it was decided that all pinctrl belongs to board files
<lumag>
minecrell, I hope not!
<bryanodonoghue>
minecrell
<minecrell>
lumag: everything is so secretive about newer platforms, can't even find a schematic of RB3/RB5 to check :/
<bryanodonoghue>
on our 4.19 vendor 8939 we ended up doing sdhci and at least one other peripheral with some qcom cpu idle specific extensions
<minecrell>
and so even in 2022 msm8916 is still the most modern/only semi-open Qualcomm platform with some public docs&schematics available...
<bryanodonoghue>
I *think* the API isn't too bad
<bryanodonoghue>
but you're right
<minecrell>
wait it's 2023 already
<bryanodonoghue>
driver work is required
<bamse>
lumag: thought we decided that there was some sort of mix between what pinctrl state goes in the platform.dtsi and what goes into the board one...has this changed to have everything in the board.dts now?
<lumag>
bamse, I thought so too. But then for 8350 I was suggested to move all pinctrl to sm8350-hdk.dts
<bamse>
minecrell: there's no &sleep_clk crystal, it's just represents a pin on one of the pmics...
<minecrell>
bamse: sounds like it should indeed be part of the pmic dtsi then :p
<bamse>
lumag: again, i'm fine with that...and i think it clarifies the mess we're in...i generally don't mind the duplication...
<minecrell>
bryanodonoghue: I'd be fine even with some semi-messy recent branch that manages to reach the low power states (as reported by sleep-stats in debugfs). Should cpuidle ever become the last blocker one could still consider if we can get the non-PSCI LPM somehow or if doing PSCI in custom EL2/hyp or EL3/tz would be a more realistic choice to avoid
<minecrell>
endless discussions
<minecrell>
I still think getting in the non-PSCI cpuidle would be best though, as there is no other choice for the really old platforms like 8974
<Rayyan>
why is it that, for msm8916, on downstream the iommu is "v2" but on mainline the iommu is "v1"?
<minecrell>
and while EL2/hyp would be available to use for implementing PSCI on pretty much all 8916/8939 devices it's kind of nicer to use it for KVM or similar
<minecrell>
Rayyan: This is a mistake iirc that was discussed at some point (possibly even here), the naming is a bit of a mess
<Newbyte>
minecrell: just curious, why do you say it's "kind of nicer" to use it for KVM?
<aka_[m]>
Since 8916 qcom_iommu is just mmu500 but with qcom special software sauce on top
<minecrell>
Newbyte: Well some people like using virtualization/VMs for some stuff. Granted, it probably has very limited uses on 8916/8939 which are quite CPU/RAM limited nowadays :p
<aka_[m]>
On 8974 I was quite unable to really find what it is based on
<minecrell>
although iirc there were occasionally people asking about Xen for db410c in the 96boards forums
<Newbyte>
minecrell: and there's no tangible benefit of using PSCI?
<minecrell>
(Xen for again who knows what reason, considering DB410c even has just 1 GiB RAM and this is barely enough for Linux)
<minecrell>
Newbyte: It's not really a technical matter but more a matter of principle. ARM64 Linux requires everyone to implement PSCI for cpuidle to finally get some much needed standardization
<minecrell>
although this matter of principle is also showing its limitations, such as in this case when only Qcom can update the firmware properly, or for the Asahi/Apple M1 people where the CPU does not support EL3 at all
<Rayyan>
minecrell: ah, okay
<Rayyan>
what are the 'Qualcomm "B" family devices' described in the mainline iommuv1 documentation?
<lumag>
minecrell, msm8974 should work with SPM idle.
<lumag>
But for the arm64 the upstream position is quite firm, I believe
<minecrell>
lumag: yes, but it doesn't implement the deeper cluster idle states
<minecrell>
it only implements standalone power collapse of one core
<minecrell>
if that implementation is extended then even arm64 devices are just a compile switch away from making use of it
<minecrell>
since there is no technical reason the same code couldn't work on arm64, it's all abstracted by the cpuidle drivers subsystem
<aka_[m]>
<Rayyan> "what are the 'Qualcomm "B..." <- Something up to 8996
<aka_[m]>
Not sure if 8998 included but probably nope
<minecrell>
Rayyan: they probably refer to some set of similar platforms, but which exactly is hard to tell. iirc the IOMMU was changed quite a bit between 8064 and 8974, and then again a bit between 8974 and 8916
<minecrell>
the only thing that's clear is that the current code in mainline represents the 8916 configuration :p
<aka_[m]>
For sure everything after 8916 till 8996 is fam b
<minecrell>
as long as you're careful not to include 8994 in that list :p
<aka_[m]>
minecrell: I guess 8994 could also be somewhere, from what I heard 8994 share same bsp tree like others
<minecrell>
aka_[m]: 8994 is much more like 8974 than 8916
<Rayyan>
so does msm8226 fit into family B?
<aka_[m]>
Not sure, it was some worse 8974+8916 mix right?
<aka_[m]>
I doubt
<lumag>
I don't think we support iommu on 8974 at all
<lumag>
or on 8064
<Rayyan>
there were some patches on iommu for 8974
<aka_[m]>
Fast look iommu looks like 8974
<minecrell>
lumag: 8064 has drivers/iommu/msm_iommu.c
<lumag>
ah, true
<minecrell>
Rayyan: it's likely you need similar iommu patches as 8974 on 8226
<aka_[m]>
For now it for sure lack these bandwidth somethings bfp?
<minecrell>
iirc 8974 has some ARM SMMUv1 like thing while 8916+ has a standard ARM SMMU-500 (SMMUv2), just messed up in the tz firmware
<aka_[m]>
Mmu500 vs smmu-v2 appears to have different docs from arm
<aka_[m]>
And be different ip
<minecrell>
aka_[m]: no no no no no
<minecrell>
This is like ARMv8 and Cortex-A53
<minecrell>
one is the specification, the other an IP block
<aka_[m]>
A53 implements arm v8
<minecrell>
MMU-500 is an implementation of the SMMU-v2 spec
<aka_[m]>
And what is qcom so called qcom-smmu-v2
<aka_[m]>
One on 660 and 8996(?)
<minecrell>
It could be that qcom made a custom implementation of the SMMUv2 spec, so they didn't take the MMU-500 IP block from ARM
<minecrell>
not sure if this is the case here
<minecrell>
wouldn't be the first time they did that. There is also QGIC2 implementing the GICv2 specification (with mistakes), instead of using ARM's GIC-400
<mal>
I have that wip msm8974 iommu, I should try to finish it
<Newbyte>
that would be great
<lumag>
mal: would be great!
<mal>
I need to first send long overdue other patches like wcnss and modem for msm8226, those are ready but I haven't decided in what kind of batches to send those