> > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > > > index a40295c18b48..780681ea77e4 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > > @@ -64,7 +64,7 @@ static int vkms_enable_vblank(struct drm_crtc *crtc) > > > struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc); > > > struct vkms_output *out = drm_crtc_to_vkms_output(crtc); > > > > > > - drm_calc_timestamping_constants(crtc, &crtc->mode); > > > + drm_calc_timestamping_constants(crtc, &crtc->legacy.mode); > > > > drm_calc_timestamping_constants(crtc, &crtc->state->mode); > > This one doesn't look safe. You want to call that during your atomic > commit already. > This was already not safe with the previous implementation? Or it is only unsafe because now I use state->mode instead of legacy.mode? After inspecting the code, I think I don't need to call it as: In `vkms_atomic_commit_tail` (used in `@vkms_mode_config_helpers.atomic_commit_tail`), we call `drm_atomic_helper_commit_modeset_disables`, which call `drm_atomic_helper_calc_timestamping_constants` which call `drm_calc_timestamping_constants` for every CRTC. I tested kms_vblank, all of them are SUCCESS/SKIP, do you know other tests that can trigger bugs? -- Louis Chauvet, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
