ChanServ changed the topic of #asahi-dev to: Asahi Linux: porting Linux to Apple Silicon macs | General development | GitHub: https://alx.sh/g | Wiki: https://alx.sh/w | Logs: https://alx.sh/l/asahi-dev
<jannau> I haven't added the pins for t8103 but fixed the compatible string
<j`ey> cs_setup is only used if the spi core is in charge of the cs gpio
<jannau> if cs is already configured as periph it should work
<j`ey> oh hm, !spi->controller->set_cs_timing should be true actually?
<jannau> no, also if the controller has no method to set the cs timing: !spi->controller->set_cs_timing
<j`ey> yeah I missed that when i first read it, thought it was inside the if () {..
yuyichao has quit [Ping timeout: 480 seconds]
<j`ey> (I probably need to do more testing with linux.py, I always used hv)
bps has quit [Ping timeout: 480 seconds]
<marcan> jannau/j`ey: thanks for catching that!
<marcan> yup, that was a dumb typo
<marcan> did you not hit the WARN_ON(fifo_flags & APPLE_SPI_FIFO_TXOVERFLOW); ? I would expect that to fire with the bad code
rwhitby has joined #asahi-dev
bps has joined #asahi-dev
<marcan> either that or it would get stuck
<jannau> I saw tx_overflow set in the spi trace I did not notice it in the kernel log but I was not really looking at it
<marcan> it might be it was never making it out of the transfer loop
<j`ey> marcan: I managed to see one packet from the keyboard at least, but then it gets stuck
aidenfoxivey has joined #asahi-dev
StupidYui has quit [Read error: Connection reset by peer]
yuyichao has joined #asahi-dev
rwhitby has quit [Quit: rwhitby]
phiologe has joined #asahi-dev
PhilippvK has quit [Ping timeout: 480 seconds]
vx^ has quit [Ping timeout: 480 seconds]
<marcan> j`ey: can you push that branch somewhere?
<marcan> in principle we can run the HV in UP mode (or just filter out one CPU, might add an option for that) and then throw the gpiola on that one and see what linux does
<marcan> ah, saw the link
<marcan> let's see
<jannau> I suppose j`ey was using something very similar to https://github.com/jannau/linux/tree/exp/spi-keyboard except for the random delays which fix it somewhat on bare metal
<marcan> does it not need those delays with the HV *even without* SPI tracing?
<marcan> just plain HV should have relatively little performance impact, mostly the timer IRQ spam
<jannau> let me test that
<jannau> it's broken without spi tracing
<marcan> also oh god just looking at the diff context for that applespi.c makes me cringe
<marcan> such a mess of ACPI stuff mixed in there
<marcan> we definitely want to rewrite that as a HID transport properly (and someone else can add the bits to make it work with ACPI laptops too and then we can deprecate applespi in the future)
<marcan> but for now let's see if it breaks because of a driver problem or a SPI problem...
<jannau> touchpad seems to need more delay than I currently have. broken both with bare metal and hv without tracing
<marcan> [ 0.709687] applespi spi1.0: Received corrupted packet (crc mismatch)
<marcan> [ 0.716454] applespi spi1.0: Unknown touchpad model 0 - falling back to MB8 touchpad
<marcan> that, right?
<jannau> on applespi side it presents received spi data as 0x00
<jannau> not sure but I see those two messages with spi tracing and working touchpad
<marcan> that sounds like there's more than one problem
<kettenis> there are some packets that get sent early one that I haven't been able to give a meaning to
<marcan> I seem to have working touchpad with that branch as-is
<marcan> on the M1 Pro
<marcan> HV, no tracing
<jannau> increasing SPI_END_CS_DELAY_US in applespi to 100 fixes the touchpad on baremetal
<marcan> keyboard too
<jannau> caps lock led still breaks it but that ought to be an applespi problem
<jannau> keyboard seems to be more reliable
<marcan> let me actually try the touchpad (I was just catting the device)
<marcan> how does it break for you?
<marcan> seems to work for me
<marcan> ahh xorg broke it
<marcan> [ 14.081273] applespi spi1.0: Received corrupted packet (invalid message length 8 - num-fingers 0, tp-len 48)
<marcan> [ 14.089493] applespi spi1.0: Received corrupted packet (invalid message length 8 - num-fingers 0, tp-len 48)
<marcan> worked with gpm though
<marcan> keyboard does work though
<marcan> which is weird, being the same device...
<marcan> but even tracing spi after the error happens doesn't fix it
<kettenis> btw, how did you determine the input clock for the spi controller?
<jannau> the spi device might be a multiplexer with distinct keyboard and touchpad sub devices
<kettenis> jannau: almost certainly the case
<marcan> kettenis: ADT tells us (see dump_pmgr.py)
<kettenis> so is it correct that the M1 models use the 24MHz refclk and the M1 Pro/Max models a 200MHz clock?
<kettenis> hmm, dump_pmgr.py suggests spi3 uses a 120MHz clock on j293
<marcan> kettenis: no, 200MHz is for spinor
<marcan> huh, really?
<marcan> I see 24MHz on pro/max models
<marcan> I don't have a pmgr dump for j293 handy
<marcan> kettenis: can you paste that pmgr dump somewhere?
<kettenis> #123: 120000000 2 0xa0/0x23b040100: regval: 0x85100000
<kettenis> User: /device-tree/arm-io/spi3.clk[0]
<marcan> spi1 is 200MHz for spi1 on at least the mini and pro/max
<marcan> that's interesting, I wonder why that is
<marcan> spi3 is 24MHz on pro/max, and is unused on mini of course
<marcan> what does spi1 say on that machine?
<kettenis> #121: 200000000 2 0xa0/0x23b0400f8: regval: 0x85100000
<kettenis> User: /device-tree/arm-io/spi1.clk[0]
<marcan> interesting, so clock #5 is not the same frequency for those SPIs
<marcan> oh wait
<marcan> #121: 200000000 2 0xa0/0x23b0400f8: regval: 0x85100000
<marcan> User: /device-tree/arm-io/spi1.clk[0]
<marcan> #122: 120000000 2 0xa0/0x23b0400fc: regval: 0x85100000
<marcan> #123: 120000000 2 0xa0/0x23b040100: regval: 0x85100000
<marcan> #124: 120000000 2 0xa0/0x23b040104: regval: 0x85100000
<marcan> so the *unused* spis on the mac mini get set to 120MHz, heh
<marcan> I wonder if this is actually an oversight...
<marcan> also it feels really weird that #5 is 120 for spi2/3/4 but not spi1
<marcan> maybe they actually forgot to initialize that clock in iBoot (but it works anyway because the spi driver does the right thing wrt divider)
<kettenis> well, if the input frequency isn't 24MHz but 200MHz the actual SPI clock frequency won't be what I think it is
<marcan> 200MHz is only used for spi1 as far as I can tell
<marcan> for spi3 we have either 24 or 120 apparently?
<marcan> but my point is the spi driver should be computing its divider based on this information
<marcan> now the question is is it 120 on the air too...
<kettenis> of course I copied the spi-max-frequency property from corellium
<marcan> spi-max-frequency is something else, isn't it?
<marcan> that's a max, not the base clock
<marcan> the base clock comes from the clock provider
<marcan> but the clock being 5x faster by accident on j293 would explain why it's breaking for people
<kettenis> spi-max-frequency is the max frequency the spi device can run at if my understanding is correct
<marcan> yes
<kettenis> spi-max-frequency = <2000000>;
<kettenis> that is what I have
<marcan> yes, and if your clock provider is claiming the controller is clocked at 24 when it's actually at 120, then you're driving the device at 10MHz instead of 2MHz
<marcan> which is bound to cause some issues :-)
<marcan> let me try the m1 air...
<kettenis> your gpiola.py stuff can actually measure the clock frequency?
<marcan> sure, it's timed
<marcan> just capture it and eyeball it in gtkwave by selecting a range of cycles
<marcan> it's a real logic analyzer with proper delta compression ;)
<marcan> (some call it RLE)
<kettenis> I'll need to fugure out how to use it ;)
<marcan> ok, at least it's badly broken on the m1air, much more than the pro
<kettenis> also, how did you figure out which gpio pins are used?
<marcan> let's look at pmgr
<marcan> the CS pin is specified in the ADT, I guessed it was nearby pins. that worked for spi3 but not spi1, for spi1 I spammed the RegMonitor during a SPI transaction until I saw something change
<marcan> that found me the two disjointed pins
<marcan> you could also just trace, like, all pins in groups of 32 (the analyzer supports up to 32)
<kettenis> the adt doesn't actually provide the equivalent of spi-max-frequency for the spi devices isn't it?
<marcan> #123: 120000000 2 0xa0/0x23b040100: regval: 0x85100000
<marcan> User: /device-tree/arm-io/spi3.clk[0]
<marcan> indeed, it's 120 on the m1 air too
<marcan> looks like it's 120 on all the m1 devices then
<marcan> still broken though
<marcan> [GPIOTracer@/arm-io/gpio] Pin-49 W 0x876220 (GROUP=7, CFG_DONE=1, PERIPH=1, CONFIG=0, VALUE=0)
<marcan> this is wrong, it's setting the CS pin to hardware control but then the spi node is being told to use software CS
<kettenis> are you tracing macos?
<jannau> probably my tree, I think I updated just the compatible for t8103
<marcan> no, that's linux
<marcan> pins={"clk": 46, "mosi": 47, "miso": 48, "cs": 49},
<marcan> I think those are the spi3 pins for t8103 (miso unconfirmed)
<marcan> keyboard/touchpad works on j313 now
<marcan> not sure about X, I need to figure out why that breaks the touchpad, but I get the feeling that's an applespi problem, not a SPI problem
<marcan> cs-gpios should work, but then you need to pinconfig it to 0 instead
<marcan> since it defaults to 1 on these machines
<kettenis> I think we should not add cs-gpios if we want to use hardware cs control
<marcan> correct, but what I'm saying is software CS control should work too
<marcan> but then you need the pinconfig; without that it tries to use software CS control while the hardware is the one actually owning the pin
<marcan> which is why none of this was working a minute ago, CS wasn't changing
<kettenis> right
<jannau> thanks, fix pushed
<marcan> jannau: were you testing on t6k or t8103?
<jannau> I'm testing on t6000-j314c, I have no t8103 laptop
<marcan> ack
<marcan> I'll go back to t6k since that's where I have a root partition for X, but hopefully those DT changes were the only difference that matters on t8103
<kettenis> so the 24/120MHz difference suggests that we can actually use a 10MHz spi-max-frequency on t8103-j293
<marcan> does that work?
<marcan> I mean it was broken due to the CS anyway
<jannau> I'm not sure how much time we should spend on applespi. I'll work on the spi hid transport this weekend
<marcan> let me see what apple does
<jannau> I doubt kettenis uses my linux tree
<marcan> # [SPITracer@/arm-io/spi3] MMIO: W.4 CLKDIV = 0x3 ()
<marcan> so 24/4 = 6
<marcan> I wouldn't be surprised if 10MHz works if 6MHz is officially kosher
<marcan> but 3 would be 8, so apple definitely don't do 10
<marcan> so let's go with 6000000?
<kettenis> I'll give that a go with OpenBSD
<marcan> this is on j314s but I imagine it'd apply to the m1 devices too
<kettenis> jannau: that is the current version of my OpenBSD driver
<kettenis> hopefully that gives you some hints on how I think the HID stuff works
<kettenis> this version uses "raw" mode to support the touchpad
<kettenis> the previous revision uses HID mode which just emulates a mouse
<marcan> ah, so the "proper" touchpad mode isn't real HID?
<kettenis> indeed
<kettenis> it matches the linux drivers/input/bcm5974.c behaviour
<marcan> heh, that sounds like more refactoring that needs to happen to share that code...
<kettenis> yes, same issue on the OpenBSD side
<kettenis> so I just copied code ;)
<marcan> what a mess...
<marcan> I guess the right way to do this would be something like
<marcan> first write a HID transport that can do the keyboard/mouse bits via the standard HID framework
<kettenis> the usb hid group never properly standardized multi-touch
<marcan> then break up the bcm5974 driver into a core that sets up the input device and parses reports, and a transport layer
<marcan> the existing code would be the USB transport
<marcan> and then we add a SPI transport
<marcan> if both drivers are enabled the SPI/HID transport thing knows to switch modes and delegate to bcm5974 for that part
<marcan> or something like that
<kettenis> so my driver has some weird delays in it; it would be nice to get a better understanding of those
<marcan> other than the CS stuff?
<marcan> and reset
<marcan> those make sense
<marcan> ah, I see delays between command and status?
<marcan> that looks wrong, surely there's some way of knowing when a command is complete
<kettenis> the delay between command and status is defenitely necessary
<marcan> yeah, but that sounds like there must be a better way
<kettenis> and you need to keep CS# asserted
<kettenis> but maybe the hardware will assert the GPIO that is used for interrupts when it is ready
<kettenis> i've not checked that
<marcan> yeah, I was going to suggest that
<Glanzmann> marcan / jannau: I can test on macbook air if you tell me which tree I should use and what patch I should apply to the dtb.
<j`ey> marcan: fwiw I always get one corrupted packet on boot
<kettenis> I think that is just a packet with a different layout that isn't handled by the current code
<marcan> hold on, I found a thing
* j`ey holds
<marcan> all the gory details of the SPI timings aren't in the ADT but they're *somewhere*
<marcan> trying to figure out where
<marcan> I found a pile of debugs about that in the driver
<j`ey> marcan: I dont want to distract, but I'm confused now if m1air uses a 24mhz or 120mhz clock?
<marcan> m1air uses 120
<marcan> and apparently m1mbp does too
<marcan> so probably all t8103 devices use 120
<j`ey> I see, I wonder then how corelliums code worked, maybe its buggy
<marcan> corellium code, buggy?
<marcan> never!
<j`ey> and divides too much or something
<marcan> (but also, they set the max-freq to 2MHz when in fact it's at least 6MHz)
<marcan> (running a 6MHz device at 10MHz is relatively likely to work)
<kettenis> in the end, all that matters is that you end up with a divider that "works"
<j`ey> in the end it doesnt even matter
<j`ey> (any linkin park fans?)
<marcan> heh, I remember that lyric :-)
<marcan> btw, the ioreg on macos has lots of info, like the HID descriptors
<marcan> and it tells you what the interface numbers are etc
<marcan> (e.g. 3 is apparently the actuator, i.e. the force feedback thing)
<marcan> there is also apparently an accelerometer in here?
<kettenis> do you have the output for that handy?
<marcan> this is from m1air
<j`ey> marcan: im happy the air is getting some attention ^_^
<marcan> I mean I want all the machines to work :-)
<j`ey> yeah but it was always the mini you worked on, then the t6000 :P
Andre[m]1 has joined #asahi-dev
<marcan> yeah, because a lot is shared
<marcan> but now that things are starting to come together I do want to test stuff on all of them and fix whatever's broken :)
<kettenis> interface 3 and 4 only provide vendor-specific stuff
<kettenis> so the descriptors don't tell us very much
<marcan> oh derp it's in the firmware
<marcan> so I guess the device reports its own properties
<marcan> just running `strings` in the firmware yields a giant plist
<marcan> wait no
<marcan> this "firmware" *is* the plist
<marcan> and it includes the actual firmware and more stuff inside
<marcan> what
<marcan> ... so where is this stored?
<marcan> /System/Library/AssetsV2/com_apple_MobileAsset_SFRSoftwareUpdate/e1e322bee87acc15973794a58f6e551734c82adf.asset/AssetData/boot/Firmware/J293_Multitouch.im4p for one... but surely it doesn't get it from there
<FireFox317> apple really do like firmware, doesnt it
<kettenis> no real surprise that the descriptors are contained in the firmware
<marcan> I mean stuff like SPI config
<marcan> https://mrcn.st/p/ue4byz9Y this is for J314C with the firmware blobs snipped
<marcan> that stupid IDREF compression is annoying
<marcan> but anyway, you have all the timings at the end
<marcan> but there's supposed to be more stuff that I'm not finding...
<marcan> you know...
<marcan> reg = 000000007d0000000000010800000000204e0000000000000000000000000000
<marcan> the "reg" of the spi device might be the timings, now that I think about it...
<marcan> this is the kind of horrible hack apple would do...
aleasto has joined #asahi-dev
<marcan> goddammit I think it is
<marcan> *facepalm*
<j`ey> it certainly looks outside of physical memory space :P
<marcan> u32 unk, clock_period, flags, word_delay, cs_delay, cs_preamble_delay, cs_postamble_delay
<marcan> this claims the clock is 8MHz (assuming that's nsec) and the cs_delay 20us
<sven> lol
<Glanzmann> Lets ship it.
<marcan> oh, and where is AppleARMSPIDevice defined? in AppleARMPlatform of course
<marcan> so not only are they tying their SPI device abstraction to ARM platforms only, they define it in the kitchen sink driver
<marcan> seriously, people need to give us credit, the stuff we come up with is way cleaner than macOS :p
<marcan> also the controller driver is called AppleSamsungSPI so I was right that it's somehow a relative of Samsung stuff
<sven> yeah, there are some horrible hacks in some drivers
<sven> there's also the entire gpu iommu node that's just used to create a small shim driver that just calls back to the main gpu driver
<marcan> looks like the preamble/postamble ones are cs to data and data to cs, and take effect if cs_delay is zero
<marcan> so this means 20us after cs assert and before cs deassert
<marcan> I think
<marcan> the four flag bytes I think are clock_polarity, flags (I think only bit 0 does something), some other bool, and bits per word
<j`ey> marcan: either that 120MHZ clock fixed it.. or the change I made to use gpio instead of hw-cs /me reverts back to hw-cs and checks
<j`ey> yes it was the clock (obviously!)
aidenfoxivey has quit [Quit: aidenfoxivey]
aidenfoxivey has joined #asahi-dev
gladiac is now known as Guest8136
gladiac has joined #asahi-dev
bps has quit [Ping timeout: 480 seconds]
Guest8136 has quit [Ping timeout: 480 seconds]
bps2 has joined #asahi-dev
the_lanetly_052__ has joined #asahi-dev
<marcan> I think the CLKDIV must be off by one, assuming apple's driver isn't wrong
<marcan> it sets it to 3, which for 24MHz would be 8MHz under that interpretation (I had it as divisor - 1 instead which would give 6MHz)
<marcan> wonder if I can get the gpiotracer accurate enough to test that... probably measuring the entire transfer with auto-cs
aleasto has quit [Remote host closed the connection]
Gaspare has joined #asahi-dev
<j`ey> Im getting what seems to be recursive exceptions when trying to use linux.py, anyone else seeing this?
<j`ey> nevermind, I forgot to chainload
yuyichao has quit [Ping timeout: 480 seconds]
<marcan> j`ey: why going back to 24?
<j`ey> marcan: because I misunderstood what you said before
<marcan> I was talking about t6000
<marcan> but if that works then I guess it can handle 30MHz lol?
<marcan> ok, confirmed with spi.py, the CLKDIV is not -1 and the minimum is 2
<kettenis> so you end up with 200000000 / CLKDIV on the t6000?
<marcan> no, 24MHz / CLKDIV
<kettenis> ah for spi3
<marcan> ah yes
<marcan> for spi1 it would be 200M, yes
aleasto has joined #asahi-dev
yuyichao has joined #asahi-dev
kgarrington has joined #asahi-dev
kgarrington has quit [Remote host closed the connection]
yuyichao has quit [Ping timeout: 480 seconds]
mini_ has joined #asahi-dev
<marcan> ok, got gpiola running on the hypervisor while running linux; I can't see the signals accurately at 8MHz, but I can tell the CS to clock delay isn't being correctly implemented
the_lanetly_052___ has joined #asahi-dev
mini_ has quit [Quit: ZNC closing...]
<marcan> ... because applespi calls applespi_setup_read_txfrs before applespi_setup_spi, which is what sets the cs delay value
<j`ey> looks like youre missing a patch
the_lanetly_052__ has quit [Ping timeout: 480 seconds]
<marcan> that wasn't in jannau's branch I think
<marcan> yup, keyboard works now
<j`ey> oh, he must have missed it, it was in previous branches of his
<marcan> unless I dropped it while rebasing at some point, but I don't think so? at least not intentionally
<_jannau_> I must have dropped that during the rebase onto asahi
<marcan> either way obviously the way that driver is doing things is wrong; instead of that delay message it should be setting up a cs_hold
<marcan> also, is it me or is set_cs_timing broken in the spi core?
<j`ey> its not used
<marcan> there is nothing that calls it, but it existing cancels the delay code
<j`ey> broonie seemed to think it was important for some reason..
<marcan> ugh
<marcan> that would be the function for the new HID driver to se the CS delays
<j`ey> if there's a user for it, I thin it would be okay to bring back
<marcan> it bugs me that it changes the delays on the controller though
<marcan> shouldn't those values be per-device, not per-controller?
<kettenis> hmm, there are fields in struct spi_device now
<marcan> ahh right
<marcan> so I guess we're just supposed to set those directly
<marcan> not through a function
<marcan> (though no drivers do that)
<kettenis> it is a fairly recent addition
<marcan> the set_cs_timing thing is definitely wrong though
<marcan> that removal removed its usage
<marcan> the spi core needs some code to call it during transfer setup passing in spi_device values
<kettenis> spi_set_cs() handles all that isn't it?
<marcan> not if set_cs_timing exists
<marcan> also, er, I think it's wrong?
<kettenis> unless you provide the set_cs_timing() callback in your driver
<marcan> cs_setup handling is wrong lol
<marcan> it needs to happen at the end of the function
<marcan> broken code, broken code everywhere
<kettenis> mwahaha
<j`ey> marcan: oof
Gaspare has quit [Quit: Gaspare]
<marcan> j`ey, jannau: pushed the core fixes to asahi
<marcan> then on top of that, you can do this to applespi: https://mrcn.st/p/O5dbDo3P
<marcan> I'm just going to send off that spi core patch now, it's obvious
<j`ey> marcan: ty!
<marcan> ah, forgot the signoff. adding it before submission :p
<j`ey> marcan: you can set set format.signoff true in git config
<marcan> ah, yeah
<marcan> thanks
<j`ey> or commit.signoff.. I seem to have both *shrug*
<marcan> I also just added myself a fixes = Fixes: %h (\"%s\") format
<marcan> and a fixes = log -1 --format=fixes alias
Gaspare has joined #asahi-dev
balrog has quit [Quit: Bye]
balrog has joined #asahi-dev
<kettenis> fwiw, 120MHz base clock and 8MHz bus clock seems to work fine with my OpenBSD drivers
Dcow_ has joined #asahi-dev
Dcow has quit [Remote host closed the connection]
Gaspare has quit [Quit: Gaspare]
aidenfoxivey has quit [Quit: aidenfoxivey]
aidenfoxivey has joined #asahi-dev
Gaspare has joined #asahi-dev
bps2 has quit [Ping timeout: 480 seconds]
Glanzmann has quit [Quit: leaving]
bpye has quit [Quit: The Lounge - https://thelounge.chat]
Gaspare has quit [Quit: Gaspare]
the_lanetly_052___ has quit [Ping timeout: 480 seconds]
zimsneexh has joined #asahi-dev
kumoko has joined #asahi-dev
kumoko has quit []
slicey has joined #asahi-dev
ted[m] has left #asahi-dev [#asahi-dev]