Re: [PATCH] drm/hyperv: add a parameter to disable the vblank timer

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 17.10.26 um 17:34 schrieb Lanfeng Wei:
> A performance regression has been observed on Hyper-V causing serious
> frame rate drops since commit 52e6b1988334 ("drm/hypervdrm: Use vblank
> timer").
>
> The regression occurs when using Xorg with a software rendered cursor.
> The offending commit uses the DRM vblank hrtimer to pace atomic update
> completion events, making each event wait for the next simulated vblank.
> However, cursor damage invoked by mouse movement is unconstrained by
> screen refresh rate, and therefore consumes most of Xorg's update
> opportunities, starving normal graphical updates, resulting in
> plummeting frame rate.
>
> Hyper-V virtual display framebuffer is able to accept multiple writes in
> one update window, so the commit broke this expected behavior.
>
> The regression is also reported by other users:
>
> Link: https://www.mail-archive.com/ubuntu-bugs@lists.ubuntu.com/msg6302857.html
>
> Add a read-only module parameter hyperv_drm.vblank_timer, which is
> enabled by default. Disabling it prevents the timer's enable callback
> from starting the timer. The vblank framework and the existing atomic
> callbacks remain registered. When disabled, the first
> drm_crtc_vblank_get() fails through the callback, so the existing helper
> sends the atomic event immediately and skips the vblank wait, matching
> the behavior before the timer was introduced.

Yeah, what's the cause of it? Switching off features does not fix any 
bugs or improve the status quo.


We had another fix for a problem with vblank timers:

https://lore.kernel.org/dri-devel/20260914110004.211027-1-tzimmermann@suse.de/

Did you validate that this fix is unrelated?

Best regards
Thomas


>
> This provides an opt-out for affected virtual display users.
>
> Benchmark results:
>
> Workload: Inject mouse movement at 125 Hz against a flashing
> animation running at 60 Hz, matching the screen refresh rate, for 4
> seconds.
>
> Tested versions: 6.18 (the last major version before the offending
> commit), version 6.19 (the first major version containing that commit),
> version 7.1.8 (on which the patch is based), and its patched version.
>
> Version  Anim. Hz            Mouse Hz         Max client-request gap
> ====================================================================
> 6.18     59.66-60.00        122.00-123.33     17.0 ms
> 6.19     0-0.162            59.73-59.86       7.53 s
> 7.1.8    0.133-0.162        59.83-59.86       6.16 s
> Patched  59.75-60.00        122.75-124.50     14.0 ms
>
> The patch restores both animation and mouse update rates, and reduces
> the maximum client-request gap from seconds to milliseconds.
>
> Fixes: 52e6b1988334 ("drm/hypervdrm: Use vblank timer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lanfeng Wei <xlfylawarthur@gmail.com>
> ---
> The parameter defaults to enabled to preserve the current kernel
> behavior. I do not know the full set of users who rely on the vblank
> timer, so changing the default could introduce a different regression.
> Affected users can set hyperv_drm.vblank_timer=0.
>
> If the maintainers prefer the parameter to default to disabled, or if
> there is a better way to detect the affected configuration
> automatically, I'm happy to send a v2.
>   drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 23 ++++++++++++++++++++-
>   1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> index d48ca6c23..527385748 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> @@ -4,6 +4,7 @@
>    */
>   
>   #include <linux/hyperv.h>
> +#include <linux/moduleparam.h>
>   
>   #include <drm/drm_atomic.h>
>   #include <drm/drm_crtc_helper.h>
> @@ -25,6 +26,19 @@
>   
>   #include "hyperv_drm.h"
>   
> +static bool vblank_timer = true;
> +module_param(vblank_timer, bool, 0444);
> +MODULE_PARM_DESC(vblank_timer,
> +		 "Enable the Hyper-V DRM vblank timer (default: true)");
> +
> +static int hyperv_crtc_enable_vblank(struct drm_crtc *crtc)
> +{
> +	if (!vblank_timer)
> +		return -EOPNOTSUPP;
> +
> +	return drm_crtc_vblank_helper_enable_vblank_timer(crtc);
> +}
> +
>   static int hyperv_blit_to_vram_rect(struct drm_framebuffer *fb,
>   				    const struct iosys_map *vmap,
>   				    struct drm_rect *rect)
> @@ -132,7 +146,14 @@ static const struct drm_crtc_funcs hyperv_crtc_funcs = {
>   	.page_flip = drm_atomic_helper_page_flip,
>   	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> -	DRM_CRTC_VBLANK_TIMER_FUNCS,
> +	/*
> +	 * expanded DRM_CRTC_VBLANK_TIMER_FUNCS to override
> +	 * enable_vblank with hyperv_crtc_enable_vblank
> +	 */
> +	.enable_vblank = hyperv_crtc_enable_vblank,
> +	.disable_vblank = drm_crtc_vblank_helper_disable_vblank_timer,
> +	.get_vblank_timestamp =
> +		drm_crtc_vblank_helper_get_vblank_timestamp_from_timer,
>   };
>   
>   static int hyperv_plane_atomic_check(struct drm_plane *plane,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)