| Message ID | 20180625171922.5ofev566kghxxxwu@smtp.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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? > + 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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 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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel
On Tue, Jun 26, 2018 at 4:25 AM, Rodrigo Siqueira <rodrigosiqueiramelo@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@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@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> -- >> Ville Syrjälä >> Intel
On 06/26, Daniel Vetter wrote: > On Tue, Jun 26, 2018 at 4:25 AM, Rodrigo Siqueira > <rodrigosiqueiramelo@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@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 I think I understood your points, and I already started to work on a new version. First, I will focus on the real hardware simulation and try to make all the kms_flip tests pass (at this moment around 1/3 of the tests are passing). I will talk with you in the irc for trying to better understand the details. Thanks > > 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@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
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); + 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);
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@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(-)