From: Javier Martinez Canillas <javi...@redhat.com> Sent: Tuesday, September 2, 2025 8:41 AM > > Thomas Zimmermann <tzimmerm...@suse.de> writes: > > [...] > > >> > >> I'm not familiar with hyperv to know whether is a problem or not for the > >> host to not be notified that the guest display is disabled. But I thought > >> that should raise this question for the folks familiar with it. > > > > The feedback I got at > > https://lore.kernel.org/dri-devel/sn6pr02mb4157f630284939e084486afed4...@sn6pr02mb4157.namprd02.prod.outlook.com/ > > > > is that the vblank timer solves the problem of excessive CPU consumption > > on hypervdrm. Ans that's also the observation I had with other drivers. > > I guess, telling the host about the disabled display would still make sense. > > > > Yes, I read the other thread you referenced and that's why I said that > your patch is correct to solve the issue. > > I just wanted to point out, since it could be that as a follow-up the > driver could need its own .atomic_disable instead of using the default > drm_crtc_vblank_atomic_disable(). Something like the following maybe: > > +static void hyperv_crtc_helper_atomic_disable(struct drm_crtc *crtc, > + struct drm_atomic_state *state) > +{ > + struct hyperv_drm_device *hv = to_hv(crtc->dev); > + struct drm_plane *plane = &hv->plane; > + struct drm_plane_state *plane_state = plane->state; > + struct drm_crtc_state *crtc_state = crtc->state; > + > + hyperv_hide_hw_ptr(hv->hdev); > + /* Notify the host that the guest display is disabled */ > + hyperv_update_situation(hv->hdev, 0, hv->screen_depth, > + crtc_state->mode.hdisplay, > + crtc_state->mode.vdisplay, > + plane_state->fb->pitches[0]); > + > + drm_crtc_vblank_off(crtc); > +} > + > static const struct drm_crtc_helper_funcs hyperv_crtc_helper_funcs = { > .atomic_check = drm_crtc_helper_atomic_check, > .atomic_flush = drm_crtc_vblank_atomic_flush, > .atomic_enable = hyperv_crtc_helper_atomic_enable, > - .atomic_disable = drm_crtc_vblank_atomic_disable, > + .atomic_disable = hyperv_crtc_helper_atomic_disable, > };
I have some historical expertise in the Hyper-V fbdev driver from back when I was a Microsoft employee (I'm now retired). The fbdev driver is similar to the DRM driver in that it tells the Hyper-V host that the device is "active" during initial setup, but there's never a time when the driver tells Hyper-V that the device is "not active". I agree that symmetry suggests having disable function that sets "active" to 0, but I don't know what the effect would be. I don't know if Hyper-V anticipates any circumstances when the driver should tell Hyper-V the device is not active. My chances are not good in finding someone on the Hyper-V team who could give a definitive answer, as it's probably an area that is not under active development. The Hyper-V VMBus frame buffer device functionality is what it is, and isn't likely to be getting enhancements. I suggest that we assume it's not necessary to add a "disable" function, and proceed with Thomas' proposed changes to the Hyper-V DRM driver. Adding "disable" now risks breaking something due to effects we're unaware of. If in the future the need arises to mark the device not active, the "disable" function can be added after a clarifying conversation with the Hyper-V team. If anyone at Microsoft wants to chime in, please do so. :-) Michael
