[PATCH] drm/vkms: Add vblank events simulated by hrtimers
Daniel Vetter
daniel at ffwll.ch
Tue Jun 26 07:54:22 UTC 2018
On Tue, Jun 26, 2018 at 4:25 AM, Rodrigo Siqueira
<rodrigosiqueiramelo at gmail.com> wrote:
> On 06/25, Ville Syrjälä wrote:
>> On Mon, Jun 25, 2018 at 02:19:22PM -0300, Rodrigo Siqueira wrote:
>> > This commit adds regular vblank events simulated through hrtimers, which
>> > is a feature required by VKMS to mimic real hardware. In this sense,
>> > this commit adopts a default frequency of 60Hz for vblank interval.
>> > Finally, this commit implements handlers for some of the atomic and
>> > vblank hooks.
>> >
>> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo at gmail.com>
>> > ---
>> > Note:
>> > - This patch depends on the patchset "drm/vkms: Updates to meet basic
>> > kms_flip requirements"
>> > link: https://lists.freedesktop.org/archives/dri-devel/2018-June/180823.html
>> >
>> > drivers/gpu/drm/vkms/vkms_crtc.c | 90 +++++++++++++++++++++++++++++++-
>> > drivers/gpu/drm/vkms/vkms_drv.c | 6 +++
>> > drivers/gpu/drm/vkms/vkms_drv.h | 9 ++++
>> > 3 files changed, 103 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
>> > index 84cc05506b09..73aae129c37d 100644
>> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
>> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
>> > @@ -10,6 +10,52 @@
>> > #include <drm/drm_atomic_helper.h>
>> > #include <drm/drm_crtc_helper.h>
>> >
>> > +static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
>> > +{
>> > + struct vkms_output *output = container_of(timer, struct vkms_output,
>> > + vblank_hrtimer);
>> > + struct drm_crtc *crtc = &output->crtc;
>> > + unsigned long flags;
>> > + int ret_overrun;
>> > + bool ret;
>> > +
>> > + ret = drm_crtc_handle_vblank(&output->crtc);
>> > + if (!ret)
>> > + DRM_ERROR("vkms failure on handling vblank");
>> > +
>> > + spin_lock_irqsave(&crtc->dev->event_lock, flags);
>> > + if (output->event) {
>> > + drm_crtc_send_vblank_event(crtc, output->event);
>> > + drm_crtc_vblank_put(crtc);
>> > + output->event = NULL;
>> > + }
>> > + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>> > +
>> > + ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer,
>> > + output->period_ns);
>> > +
>> > + return HRTIMER_RESTART;
>> > +}
>> > +
>> > +static int vkms_enable_vblank(struct drm_crtc *crtc)
>> > +{
>> > + struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
>> > +
>> > + hrtimer_init(&out->vblank_hrtimer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
>> > + out->vblank_hrtimer.function = &vkms_vblank_simulate;
>> > + out->period_ns = ktime_set(0, DEFAULT_VBLANK_NS);
>>
>> Don't you want the vblank interval to roughly match the crtc timings the
>> user asked for?
> Hi,
>
> We want to provide interactions with users by allowing them to specify
> the Vblank interval (and maybe other things); I hardcoded it for
> simplicity sake since my focus now it is simulates real hardware and
> later virtual hardware. However, IMHO the best way to provide this
> interaction in the VKMS is via sysfs; another option could be use a
> module parameter especified during the load, but I'm not sure if
> this is a good approach.
>
> Thanks for your feedback
Nah, we want that the the mode we simulate is the mode userspace
requested. That's how real hardware works.
For simulating virtual hardware the rules are slightly different:
There's no vblank support at all, and page flip events complete
immediately (or as long as it takes the virtual hw to upload the
buffers for display, if there's some copying involved).
drm_calc_timestamping_constants() is the helper you can use to
compute/fill in the necessary constants, but the atomic helpers do
that for you already. Note that you can't access
drm_crtc->state.requested_mode from the vblank handler, since your
hrtimer does not hold the required locks.
kms_flip should complain about the timestamps not matching with the
requested mode without this. You might need to force a specific edid
with a vrefresh that doesn't match your hardcoded one though to
actually see the bug.
-Daniel
> Best Regards,
> Rodrigo Siqueira
>
>> > + hrtimer_start(&out->vblank_hrtimer, out->period_ns, HRTIMER_MODE_ABS);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void vkms_disable_vblank(struct drm_crtc *crtc)
>> > +{
>> > + struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
>> > +
>> > + hrtimer_cancel(&out->vblank_hrtimer);
>> > +}
>> > +
>> > static const struct drm_crtc_funcs vkms_crtc_funcs = {
>> > .set_config = drm_atomic_helper_set_config,
>> > .destroy = drm_crtc_cleanup,
>> > @@ -17,6 +63,8 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>> > .reset = drm_atomic_helper_crtc_reset,
>> > .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>> > .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>> > + .enable_vblank = vkms_enable_vblank,
>> > + .disable_vblank = vkms_disable_vblank,
>> > };
>> >
>> > static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
>> > @@ -27,12 +75,50 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
>> >
>> > static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
>> > struct drm_crtc_state *old_state)
>> > +{
>> > + drm_crtc_vblank_on(crtc);
>> > +}
>> > +
>> > +static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
>> > + struct drm_crtc_state *old_s)
>> > +{
>> > + struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
>> > +
>> > + if (crtc->state->event) {
>> > + crtc->state->event->pipe = drm_crtc_index(crtc);
>> > + WARN_ON(drm_crtc_vblank_get(crtc) != 0);
>> > +
>> > + vkms_output->event = crtc->state->event;
>> > + crtc->state->event = NULL;
>> > + }
>> > +}
>> > +
>> > +static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
>> > + struct drm_crtc_state *old_crtc_state)
>> > +{
>> > + struct drm_pending_vblank_event *event = crtc->state->event;
>> > +
>> > + if (!event)
>> > + return;
>> > +
>> > + crtc->state->event = NULL;
>> > +
>> > + spin_lock_irq(&crtc->dev->event_lock);
>> > + if (drm_crtc_vblank_get(crtc))
>> > + drm_crtc_send_vblank_event(crtc, event);
>> > + spin_unlock_irq(&crtc->dev->event_lock);
>> > +}
>> > +
>> > +static void vkms_crtc_commit(struct drm_crtc *crtc)
>> > {
>> > }
>> >
>> > static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
>> > - .atomic_check = vkms_crtc_atomic_check,
>> > - .atomic_enable = vkms_crtc_atomic_enable,
>> > + .atomic_check = vkms_crtc_atomic_check,
>> > + .atomic_enable = vkms_crtc_atomic_enable,
>> > + .atomic_flush = vkms_crtc_atomic_flush,
>> > + .atomic_begin = vkms_crtc_atomic_begin,
>> > + .commit = vkms_crtc_commit,
>> > };
>> >
>> > int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
>> > index fe93f8c17997..c56d66d9ec56 100644
>> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
>> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
>> > @@ -102,6 +102,12 @@ static int __init vkms_init(void)
>> > goto out_fini;
>> > }
>> >
>> > + ret = drm_vblank_init(&vkms_device->drm, 1);
>> > + if (ret) {
>> > + DRM_ERROR("Failed to vblank\n");
>> > + goto out_fini;
>> > + }
>> > +
>> > ret = vkms_modeset_init(vkms_device);
>> > if (ret)
>> > goto out_unregister;
>> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
>> > index 76f1720f81a5..f115e7d1ae03 100644
>> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
>> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
>> > @@ -5,6 +5,7 @@
>> > #include <drm/drm.h>
>> > #include <drm/drm_gem.h>
>> > #include <drm/drm_encoder.h>
>> > +#include <linux/hrtimer.h>
>> >
>> > #define XRES_MIN 32
>> > #define YRES_MIN 32
>> > @@ -15,6 +16,8 @@
>> > #define XRES_MAX 8192
>> > #define YRES_MAX 8192
>> >
>> > +#define DEFAULT_VBLANK_NS 16666666
>> > +
>> > static const u32 vkms_formats[] = {
>> > DRM_FORMAT_XRGB8888,
>> > };
>> > @@ -23,6 +26,9 @@ struct vkms_output {
>> > struct drm_crtc crtc;
>> > struct drm_encoder encoder;
>> > struct drm_connector connector;
>> > + struct hrtimer vblank_hrtimer;
>> > + ktime_t period_ns;
>> > + struct drm_pending_vblank_event *event;
>> > };
>> >
>> > struct vkms_device {
>> > @@ -37,6 +43,9 @@ struct vkms_gem_object {
>> > struct page **pages;
>> > };
>> >
>> > +#define drm_crtc_to_vkms_output(target) \
>> > + container_of(target, struct vkms_output, crtc)
>> > +
>> > int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>> > struct drm_plane *primary, struct drm_plane *cursor);
>> >
>> > --
>> > 2.17.1
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> --
>> Ville Syrjälä
>> Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the dri-devel
mailing list