Re: [PATCH v2] drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC

[Icenowy Zheng]

在 2026-03-23一的 14:32 +0100，Thomas Zimmermann写道：
> Hi
> 
> Am 23.03.26 um 12:42 schrieb Icenowy Zheng:
> [...]
> > > > drm_update_vblank_count+0xac/0x418
> > > > [    0.334960] [<ffffffff80be6270>]
> > > > drm_vblank_enable+0xf0/0x288
> > > > [    0.334960] [<ffffffff80be7010>] drm_vblank_get+0xf8/0x160
> > > > [    0.334960] [<ffffffff80beb2e0>]
> > > > drm_client_modeset_wait_for_vblank+0x50/0xa0
> > I doubt the problem here is that the function above is called with
> > hardcoded 0 by drm_fb_helper_fb_dirty() (being inlined into
> > drm_fb_helper_damage_work() by the compiler). Then on my setup it
> > failed because my primary output (laptop LCD) is on CRTC 1 (CRTC 0
> > is
> > external HDMI instead).
> 
> The CRTC 0 has vblank interrupt. The CRTC 1 does not. Is that
> correct?

Both have vblank interrupts support. The situation here is that only
the CRTC 1 is actively outputting an image; the CRTC 0 has remaining
timing programmed by the firmware and being running the timing
generator, but not doing DMA nor generating image. (I think this could
be on purpose to prevent some registers from being erased?)

BTW after this warning, the vblank of CRTC 0 is enabled, and no one
bother to disable it, so the interrupt triggers continously.

> 
> > 
> > This is new behavior introduced by d8c4bddcd8bc ("drm/fb-helper:
> > Synchronize dirty worker with vblank"), and the commit messages say
> > `This allows several screen updates to pile up and acts as a rate
> > limiter.` .
> 
> This should work if the vblanks are configured correctly.

Hardcoding 0 here sounds not correct to me, see my analyze below.

By the way, the other codepath, ioctl FBIO_WAITFORVSYNC, mentions that
hardcoding 0 there is just guessing `The first CRTC should be the
integrated panel on most drivers`, and suggests affected applications
to switch to KMS -- but the fb dirty codepath is used by fbcon, the
biggest fbdev consumer here not yet converted...

> 
> 
> > 
> > Should it wait for any vblanks instead of waiting for one on CRTC 0
> > ?
> > (Or check whether CRTC 0 is on before waiting on it?)
> 
> It should only wait for the vblank on the CRTC it uses (1 in your
> case).

I traced some code:

The second parameter of drm_client_modeset_wait_for_vblank is called
crtc_index, and it tries to retrieve the CRTC with `client->
modesets[crtc_index].crtc` ; the modesets array is initialized with:

```
	drm_for_each_crtc(crtc, dev)
		client->modesets[i++].crtc = crtc;
```

So non-active CRTCs are also included, and hardcoding 0 here can easily
hit non-active CRTC 0.

Thanks,
Icenowy

> 
> Best regards
> Thomas
> 
> 
> > 
> > Thanks,
> > Icenowy
> > 
> > > > [    0.334960] [<ffffffff80c0a144>]
> > > > drm_fb_helper_damage_work+0x8c/0x1d8
> > > > [    0.334960] [<ffffffff8028fac8>]
> > > > process_one_work+0x198/0x348
> > > > [    0.334960] [<ffffffff802906c0>] worker_thread+0x238/0x390
> > > > [    0.334960] [<ffffffff8029c0d8>] kthread+0x160/0x198
> > > > [    0.334960] [<ffffffff8021fd8c>]
> > > > ret_from_kernel_thread+0x14/0x1c
> > > > 
> > > > [    0.334960] ---[ end trace 0000000000000000 ]---
> > > > ```
> > > > 
> > > > > > > Signed-off-by: Icenowy Zheng <zhengxingda@xxxxxxxxxxx>
> > > > > > Oh forgot to attach when crafting v2:
> > > > > > 
> > > > > > ```
> > > > > > Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > > > > > ```
> > > > > > 
> > > > > > > ---
> > > > > > > Changes in v2:
> > > > > > > - Re-formatted Loongson product model numbers per request
> > > > > > > from
> > > > > > > Huacai.
> > > > > > > 
> > > > > > >     drivers/gpu/drm/loongson/lsdc_crtc.c | 1 -
> > > > > > >     1 file changed, 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c
> > > > > > > b/drivers/gpu/drm/loongson/lsdc_crtc.c
> > > > > > > index 587fbe285e9ef..b3af8e0cdb15f 100644
> > > > > > > --- a/drivers/gpu/drm/loongson/lsdc_crtc.c
> > > > > > > +++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
> > > > > > > @@ -721,7 +721,6 @@ static const struct drm_crtc_funcs
> > > > > > > ls7a1000_crtc_funcs = {
> > > > > > >     	.late_register = lsdc_crtc_late_register,
> > > > > > >     	.enable_vblank = lsdc_crtc_enable_vblank,
> > > > > > >     	.disable_vblank = lsdc_crtc_disable_vblank,
> > > > > > > -	.get_vblank_timestamp =
> > > > > > > drm_crtc_vblank_helper_get_vblank_timestamp,
> > > > > > >     	.atomic_print_state =
> > > > > > > lsdc_crtc_atomic_print_state,
> > > > > > >     };
> > > > > > >