[PATCH v4 15/15] drm/xen: Explicitly disable automatic sending of vblank event

[Daniel Vetter]

On Thu, Jan 23, 2020 at 10:21:23AM +0100, Thomas Zimmermann wrote:
> The atomic helpers automatically send out fake VBLANK events if no
> vblanking has been initialized. This would apply to xen, but xen has
> its own vblank logic. To avoid interfering with the atomic helpers,
> disable automatic vblank events explictly.
> 
> v4:
> 	* separate commit from core vblank changes
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Acked-by: Gerd Hoffmann <kraxel at redhat.com>

On all the driver patches:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> ---
>  drivers/gpu/drm/xen/xen_drm_front_kms.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> index 4f34c5208180..efde4561836f 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> @@ -220,6 +220,18 @@ static bool display_send_page_flip(struct drm_simple_display_pipe *pipe,
>  	return false;
>  }
>  
> +static int display_check(struct drm_simple_display_pipe *pipe,
> +			 struct drm_plane_state *plane_state,
> +			 struct drm_crtc_state *crtc_state)
> +{
> +	/* Make sure that DRM helpers don't send VBLANK events
> +	 * automatically. Xen has it's own logic to do so.
> +	 */
> +	crtc_state->no_vblank = false;

So this here is strictly speaking dead code, because the fake_vblank
helper makes sure to only send out the event if it hasn't been processed
yet.

Which is a bit awkward, since it does this silently, potentially hiding
bugs. We already have a warning that complains if a crtc_state->event
hasn't been processed at all. But with the auto-vblank stuff here we kinda
defeat that a bit ... I think adding a WARN_ON in fake_vblank that fires
if no_vblank is set, but the event is somehow gone, would be really
useful. That would point at some serious driver snafu ...

Care to throw that on top?
-Daniel

> +
> +	return 0;
> +}
> +
>  static void display_update(struct drm_simple_display_pipe *pipe,
>  			   struct drm_plane_state *old_plane_state)
>  {
> @@ -284,6 +296,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
>  	.enable = display_enable,
>  	.disable = display_disable,
>  	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.check = display_check,
>  	.update = display_update,
>  };
>  
> -- 
> 2.24.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch