On Wed, Jan 15, 2020 at 01:52:26PM +0100, Thomas Zimmermann wrote:
In drm_atomic_helper_fake_vblank() the DRM core sends out VBLANK events if struct drm_crtc_state.no_vblank is enabled in the check() callbacks.
For drivers that have neither an enable_vblank() callback nor a check() callback, the simple-KMS helpers enable VBLANK generation by default. This simplifies bochs, udl, several tiny drivers, and drivers based upon MIPI DPI helpers. The driver for Xen explicitly disables no_vblank, as it has its own logic for sending these events.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 15fb516ae2d8..4414c7a5b2ce 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -146,10 +146,21 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, if (!plane_state->visible) return 0;
- if (!pipe->funcs || !pipe->funcs->check)
return 0;- return pipe->funcs->check(pipe, plane_state, crtc_state);
- if (pipe->funcs) {
if (pipe->funcs->check)return pipe->funcs->check(pipe, plane_state,crtc_state);if (pipe->funcs->enable_vblank)return 0;- }
- /* Drivers without VBLANK support have to fake VBLANK events. As
* there's no check() callback to enable this, set the no_vblank* field by default.*/
The ->check callback is right above this comment ... I'm confused.
- crtc_state->no_vblank = true;
That's kinda not what I meant with handling this automatically. Instead something like this:
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index 7cf3cf936547..23d2f51fc1d4 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -149,6 +149,11 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, /* Self refresh should be canceled when a new update is available */ state->active = drm_atomic_crtc_effectively_active(state); state->self_refresh_active = false; + + if (drm_dev_has_vblank(crtc->dev)) + state->no_vblank = true; + else + state->no_vblank = false; } EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 1659b13b178c..32cab3d3c872 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -81,6 +81,12 @@ */ #define DRM_REDUNDANT_VBLIRQ_THRESH_NS 1000000
+/* FIXME roll this out here in this file */ +bool drm_dev_has_vblank(dev) +{ + return dev->num_crtcs; +} + static bool drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, ktime_t *tvblank, bool in_vblank_irq);
But maybe move the default value to some other/better place in the atomic helpers, not sure what the best one is.
Plus then in the documentation patch also highlight the link between crtc_state->no_vblank and drm_dev_has_vblank respectively drm_device.num_crtcs.
That should plug this issue once for all across the board.
There's still the fun between having the vblank callbacks and the drm_vblank setup, but that's a much older can of worms ... -Daniel