Thomas Zimmermann <tzimmerm...@suse.de> writes:
> HyperV's virtual hardware does not provide vblank interrupts. Use a
> vblank timer to simulate the interrupt. Rate-limits the display's
> update frequency to the display-mode settings. Avoids excessive CPU
> overhead with compositors that do not rate-limit their output.
>
> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
> ---
Reviewed-by: Javier Martinez Canillas <javi...@redhat.com>
[...]
>
> @@ -111,11 +113,15 @@ static void hyperv_crtc_helper_atomic_enable(struct
> drm_crtc *crtc,
> crtc_state->mode.hdisplay,
> crtc_state->mode.vdisplay,
> plane_state->fb->pitches[0]);
> +
> + drm_crtc_vblank_on(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,
> };
>
I think your patch is correct due the driver not having an .atomic_disable
callback. But looking at the driver, I see that its .atomic_enable does:
static void hyperv_crtc_helper_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
...
hyperv_update_situation(hv->hdev, 1, hv->screen_depth,
crtc_state->mode.hdisplay,
crtc_state->mode.vdisplay,
plane_state->fb->pitches[0]);
}
and this function in turn does:
int hyperv_update_situation(struct hv_device *hdev, u8 active, u32 bpp,
u32 w, u32 h, u32 pitch)
{
...
msg.situ.video_output[0].active = active;
...
}
So I wonder if it should instead have a custom .atomic_disable that calls:
hyperv_update_situation(hv->hdev, 0, hv->screen_depth,
crtc_state->mode.hdisplay,
crtc_state->mode.vdisplay,
plane_state->fb->pitches[0]);
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.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
