Re: [PATCH v2 4/4] drm/hypervdrm: Use vblank timer

[Javier Martinez Canillas <javierm@redhat.com>]

Thomas Zimmermann <tzimmermann@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/SN6PR02MB4157F630284939E084486AFED46FA@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,
 };

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat