On Tue, Jan 21, 2025 at 10:29:25AM +0000, Hogander, Jouni wrote: > On Mon, 2025-01-20 at 16:39 +0200, Ville Syrjälä wrote: > > On Mon, Jan 20, 2025 at 07:28:52AM +0000, Hogander, Jouni wrote: > > > On Sat, 2025-01-18 at 01:07 +0200, Ville Syrjälä wrote: > > > > On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä wrote: > > > > > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > > > > > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we > > > > > > can > > > > > > allow DSB > > > > > > usage also when PSR is enabled for LunarLake onwards. > > > > > > > > > > We seem to still lack an answer as to when the PSR wakes, when > > > > > it > > > > > latches the update, and how does all that guarantee that the > > > > > DSB > > > > > interrupt fires after the update has been latched? > > > > > > > > > > Some thoughts as to how to figure this out: > > > > > 1. make sure we're in PSR > > > > > 2. sample TIMESTAMP_CTR > > > > > 3. start DSB in which > > > > > write PLANE_SURF with a new value > > > > > send push > > > > > wait for vblank > > > > > poll PLANE_SURFLIVE == new value > > > > > fire interrupt > > > > > > > > > > in the interrupt handler: > > > > > sample TIMESTAMP_CTR again > > > > > > > > > > And then compare flip timestmap vs. frame timestamp vs. the > > > > > manually sampled timestamps. And then repeat without the > > > > > SURFLIVE > > > > > poll to make sure nothing has changed. You'll need to be > > > > > careful > > > > > to make sure it will actually poll for long enough to make a > > > > > real > > > > > difference (if the poll actuall is needed), but tweaking the > > > > > poll > > > > > interval+count suitably. I don't remeber what the max poll > > > > > count was, but IIRC it wasn't too high so the duration will > > > > > have > > > > > to get bumped for long polls. > > > > > > > > > > I guess one could also try to poll for the actual PSR status, > > > > > but dunno how well that'll work. > > > > > > > > > > And we could also try to come up with different ideas on where > > > > > to sample timestamps. Unfortunately we only have the single > > > > > pipe flip timestamp register so we can only sample one > > > > > timestamp > > > > > from the DSB itself per frame. If we had more we could much > > > > > more > > > > > easily figure things out :/ > > > > > > > > > > I pushed my latest DSB selftest stuff to > > > > > https://github.com/vsyrjala/linux.git dsb_selftests_7 > > > > > which has a bunch of stuff for this kind of experimentation. > > > > > It's in a somewhat sorry state at the moment since I last used > > > > > to hunt for various DSB bugs, but at least it still builds :) > > > > > > > > > > The way I use that is that I run igt 'testdisplay -o ...' > > > > > to make sure nothing else is actively poking the hardware > > > > > and then I trigger the DSB tests via debugfs. > > > > > > > > I poked around a bit, though only on a TGL+PSR1 system (what I > > > > had > > > > at hand), so some of this might not apply to PSR2 and/or more > > > > modern platforms. > > > > > > > > General notes: > > > > - PSR1 exit is triggered by any pipe/plane register write (even > > > > the > > > > non-arming ones) > > > > > > This is same for PSR2 as well. > > > > > > > > > > > We basically have three cases to consider here: > > > > 1. PSR1 is currently inactive > > > > Obviously everything should be with the current code, > > > > vblank evasion works, wait for vblank+interrupt scheme > > > > for flip completion works > > > > > > > > 2. PSR1 is active, but DC states are not > > > > The wakeup latency here is super quick (it's < 1 scanline, how > > > > much below? I've not yet measured), and arming registers do > > > > latch > > > > nearly immediately. > > > > Actually it's only ~1usec (based on the timestamps). I also used the > > following DSB batch to test how many registers we can write there: > > > > for (i = 0; ...; i++) > > dsb_write(PLANE_SURF, i << 12) > > dsb_interrupt() > > > > and then in the interrupt handler I read PLANE_SURFLIVE, and it > > always > > shows 0xa000, meaning we only have time to write ten registers. So > > definitely not enough to guarantee that all arming registers get > > written. > > > > So, for PSR1 at least I think we'd have two options: > > 1) do a manual PSR wake around the whole commit, which doesn't > > sound very nice > > 2) evade hw_scaline==0 so that we wait until we've woken up > > from the DC state, and then proceed with the normal vblank > > evasion and arming register writes. And obviously all that > > only works in DSB_SKIP_WAITS_EN is disabled. Also if all > > the pipes are doing the full update using the DSB then we > > could perhaps also remove the explicit DC_OFF dance around > > the whole commit. > > > > > The scanline counter starts counting > > > > accordingly > > > > from vblank_start-1. However the hardware still considers PSR > > > > to be > > > > active for that short duration so DSB_SKIP_WAITS_EN _will_ skip > > > > the > > > > waits. > > > > > > > > Unfortunately being this quick I'm not convinced we have enough > > > > time > > > > to write all the registers atomically before the hardware > > > > latches > > > > something. So I'm thinking we may need to remove > > > > DSB_SKIP_WAITS_EN, > > > > in which case the vblank evasion will push the arming register > > > > writes into the next frame. This will mean the wakeup will take > > > > one full frame. > > > > > > To my understanding DSB_SKIP_WAITS_EN have impact only when in SRD > > > (PSR)/DEEP_SLEEP(PSR2). I.e. In this scenario we still do have all > > > waits as in commit without PSR. > > > > The PSR state machine is already in link off mode in this case, > > meaning the pipe has been halted, and the display has been > > signalled to scan out from its RFB (confirmed by checking the > > PSR status register in DPCD), and DSB_SKIP_WAITS_EN is already > > active. But the link is still actually up (I'm guessing it might > > be transmitting the idle pattern, but I've not confirmed that. > > Can't remeber if we even have any kind of status register that > > could show this...). So looks like the link only gets actually > > turned off by the DMC when entering the DC state. > > > > > > > > > > > > > 3. PSR1 is active and so are DC states > > > > The wakeup latency is ~5ms. During that time scanline counter > > > > reads > > > > 0, PSR is active for the purposes of DSB_SKIP_WAITS_EN. Again > > > > we > > > > pretty much need DSB_SKIP_WAITS_EN=0 here to make sure the > > > > interrupt > > > > gets signalled after the wait for vblank. vblank evasion will > > > > get > > > > skipped on account the scanline being 0. Somewhat ironically > > > > this > > > > would give us ~5ms total wakeup latency which is now faster > > > > than > > > > the > > > > previous case. > > > > > > Again my understanding is that DSB_SKIP_WAITS_EN=0/1 have impact > > > only > > > when in SRD/DEEP_SLEEP. There is SRD_CTRL/PSR2_CTRL[Idle Frames] to > > > control when entering SRD/DEEP_SLEEP. > > > > The PSR state machine is what matters. That stops the pipe, and > > causes > > DSP_SKIP_WAITS_EN to become active. And the PSR state machine is also > > what is affected by the idle frames stuff/etc. AFAIK the only stuff > > that is actually done by the DMC is turning the link/PLL/etc on and > > off. And obviously link training I guess gets triggered from there > > somehow when waking up from PSR, hence why the wakeup takes that 5ms > > longer than when DC States are disabled. > > Ok, I need do some investigation on this. The failure I was referring > was this one: > > https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-142521v2/shard-lnl-8/igt@kms_psr2_sf@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > > I.e. there is that scanline wait at the begin in DSB buffer. It was > hanging in waiting the scanline. I verified this by checking if > preceding write to 0x700300 completes and it never did. > > This was with PSR2 but not with Panel Replay. Also if I disable > DEEP_SLEEP by setting PSR2_CTL[idle frames] as 0 it didn't hang. Also > waking it up by writing e.g. CURSURFLIVE helped. Right, so the problem there really looks to be that we don't have any non-arming register writes to trigger the wakeup (because we don't have the noarm/arm split done for cursors). So yeah, we probably need to always make sure we put some kind of dummy write to the noarm section. CURSURFLIVE should be fine for that I guess. -- Ville Syrjälä Intel
