Re: [PATCH] drm/mgag200: sleep instead of busy wait for BMC

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi,

I don't understand this.

Am 30.01.26 um 14:27 schrieb Jocelyn Falempe:
> Hi,
>
> To take some measurement, I've put this instead of step 3a of 
> mgag200_bmc_stop_scanout()
>
> for (i = 0; i < 100000; i++) {
>     WREG8(DAC_INDEX, MGA1064_SPAREREG);
>     tmp = RREG8(DAC_DATA);
>     pr_info("MGA Sparereg %02x\n", tmp);
>     udelay(10);
> }
> return;

What do you actually measure? The loop in 3a is supposed to end as soon 
as bit 0x1 signals that the hsync is active.

Are you sure that the pr_info() doesn't interfere with the loop? This is 
a tight loop to catch the bit when it flips. Putting that pr_info() 
there in the loop can take plenty of time.



>
> It's called at boot at
> [   45.110616] MGA STOP SCANOUT
> [   45.110631] MGA Sparereg 84
> it oscillates between 80, 81, 82, 83, 84 for ~4310us
> [   45.114941] MGA Sparereg 81
> then stays at 81 for ~227ms
> [   45.342492] MGA Sparereg 81
> [   45.342504] MGA Sparereg 80
> and stays at 80 for 1136ms, until the end of the loop.
> [   46.356152] MGA Sparereg 80
>
> Then it's called a few time when my display go blank and each time a 
> different behavior is seen
>
> [  729.448040] MGA STOP SCANOUT
> [  729.448055] MGA Sparereg 80
> it oscillates between 80, 81, 82, 83, 84 for ~39258us
> [  729.487313] MGA Sparereg 81
> then stays at 81 for ~230ms
> [  729.717349] MGA Sparereg 81
> [  729.717363] MGA Sparereg 80
> then back to 80
>
> This one is strange, it stays at 0x81 for 1191ms
> [  838.307042] MGA STOP SCANOUT
> [  838.307055] MGA Sparereg 81
> [  839.498450] MGA Sparereg 81
>
> And the last one, this time it stays at 0x80 for 1235ms
>
> [ 4318.439032] MGA STOP SCANOUT
> [ 4318.439047] MGA Sparereg 80
> [ 4319.674140] MGA Sparereg 80
>
> So my conclusion, is that the bit 2 is almost never seen when polling 
> at 10us, so there is no chance to see it if polling at 1000us like 
> it's done by the driver. So the step 3b won't work at all on my setup.



>
> But even the bit 1 can stay set or unset for more than 1s, so it looks 
> very unreliable to rely on it, at least on this hardware.

Did you connect to the BMC virtual display while performing the test?


>
> I feel like doing a msleep(300) is probably the best bet.
> If you still trust the hardware, maybe it should wait for ~100us, then 
> check the bit 1 and wait until it goes back to 0.

Here's an example calculation: with 1920x1080@60Hz, there are 1125 lines 
overall. So

   (1,000,000 usec/sec / 60 Hz) / 1125 lines ~= 14.8 usec / line.

There are 2200 pixels on each scanline. So

   (1 - (1920 pixels / 2200 pixels) ) * 14.8 usec / line ~= 1.88 usec

This is roughly the time that the CRTC spends in each scanline's blank 
area and likely the upper bound for the duration of a single polling 
with that display mode. Otherwise, we might miss the blank.

Honestly, I'd just take the proposed patch as it is and not bother any 
further. I think this is the correct fix unless we can figure out the 
exact meaning of these bits and the BMC.

If anything, we could try to reduce the polling time to 1 usec and 
reduce the number of iterations to 50. This would give us 3 scanlines to 
catch the bit.


Best regards
Thomas


>
> You can find below the raw dmesg (I just removed the lines where the 
> value is equal to the previous and next line, to make it smaller).
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)