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

[Jocelyn Falempe <jfalempe@redhat.com>]

On 30/01/2026 15:03, Thomas Zimmermann wrote:
> 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.

I just read continuously the SPAREREG register, just after step 2.

So I take 100000 measurement, every 10us, which should take 1s, but take 
1,2s in practice, probably due to pr_info(), and reading MGA register, 
but that's not relevant.

> 
> 
> 
>>
>> 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 didn't configure the remote interface on this machine. But this code 
is still run and should work in this case.

> 
> 
>>
>> 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.

I'm fine with that too. At least on my machine, this waits for a random 
amount of time, and that looks to work.
> 
> 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).
>>
>