* [PATCH] drm/cirrus: Let DRM core send VBLANK events @ 2020-01-10 11:57 Thomas Zimmermann 2020-01-12 23:00 ` Daniel Vetter 0 siblings, 1 reply; 4+ messages in thread From: Thomas Zimmermann @ 2020-01-10 11:57 UTC (permalink / raw) To: airlied, kraxel, daniel; +Cc: Thomas Zimmermann, dri-devel, virtualization In drm_atomic_helper_fake_vblank() the DRM core sends out VBLANK events if struct drm_crtc_state.no_vblank is enabled. Replace cirrus' VBLANK events with the DRM core's functionality. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/cirrus/cirrus.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c index 248c9f765c45..4a1729aa7e53 100644 --- a/drivers/gpu/drm/cirrus/cirrus.c +++ b/drivers/gpu/drm/cirrus/cirrus.c @@ -38,7 +38,6 @@ #include <drm/drm_modeset_helper_vtables.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> -#include <drm/drm_vblank.h> #define DRIVER_NAME "cirrus" #define DRIVER_DESC "qemu cirrus vga" @@ -415,6 +414,8 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, { struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; + crtc_state->no_vblank = true; + cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb); cirrus_fb_blit_fullscreen(plane_state->fb); } @@ -434,13 +435,6 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, if (drm_atomic_helper_damage_merged(old_state, state, &rect)) cirrus_fb_blit_rect(pipe->plane.state->fb, &rect); - - if (crtc->state->event) { - spin_lock_irq(&crtc->dev->event_lock); - drm_crtc_send_vblank_event(crtc, crtc->state->event); - crtc->state->event = NULL; - spin_unlock_irq(&crtc->dev->event_lock); - } } static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = { -- 2.24.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/cirrus: Let DRM core send VBLANK events 2020-01-10 11:57 [PATCH] drm/cirrus: Let DRM core send VBLANK events Thomas Zimmermann @ 2020-01-12 23:00 ` Daniel Vetter 2020-01-13 9:01 ` Thomas Zimmermann 0 siblings, 1 reply; 4+ messages in thread From: Daniel Vetter @ 2020-01-12 23:00 UTC (permalink / raw) To: Thomas Zimmermann; +Cc: airlied, dri-devel, kraxel, virtualization On Fri, Jan 10, 2020 at 12:57:07PM +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. Replace cirrus' > VBLANK events with the DRM core's functionality. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/cirrus/cirrus.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c > index 248c9f765c45..4a1729aa7e53 100644 > --- a/drivers/gpu/drm/cirrus/cirrus.c > +++ b/drivers/gpu/drm/cirrus/cirrus.c > @@ -38,7 +38,6 @@ > #include <drm/drm_modeset_helper_vtables.h> > #include <drm/drm_probe_helper.h> > #include <drm/drm_simple_kms_helper.h> > -#include <drm/drm_vblank.h> > > #define DRIVER_NAME "cirrus" > #define DRIVER_DESC "qemu cirrus vga" > @@ -415,6 +414,8 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, > { > struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; > > + crtc_state->no_vblank = true; Huh, nice untended use of this stuff ... We've added this for writeback, but I guess it can be used for anything that's a virtual connector ... I've also spotted that you've done this same trick for ast & udl already. But I think before we roll this out massively we should make this official. Can you pls do a patch to update the kerneldoc for @no_vblank that virtual hw can also use this stuff? Also, computing state values in atomic_commit code is kinda uncool and fraught with peril - design assumption is that with some very few exceptions (which are kinda awkward, would be nice to make state pointers const) all the core and helper codes that state structures stay unchanged after atomic_check completed. This should be computed in atomic_check (like vc4 does). Can you pls also include patches to update ast and udl in this series? Thanks, Daniel > + > cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb); > cirrus_fb_blit_fullscreen(plane_state->fb); > } > @@ -434,13 +435,6 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, > > if (drm_atomic_helper_damage_merged(old_state, state, &rect)) > cirrus_fb_blit_rect(pipe->plane.state->fb, &rect); > - > - if (crtc->state->event) { > - spin_lock_irq(&crtc->dev->event_lock); > - drm_crtc_send_vblank_event(crtc, crtc->state->event); > - crtc->state->event = NULL; > - spin_unlock_irq(&crtc->dev->event_lock); > - } > } > > static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = { > -- > 2.24.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/cirrus: Let DRM core send VBLANK events 2020-01-12 23:00 ` Daniel Vetter @ 2020-01-13 9:01 ` Thomas Zimmermann 2020-01-14 3:52 ` Daniel Vetter 0 siblings, 1 reply; 4+ messages in thread From: Thomas Zimmermann @ 2020-01-13 9:01 UTC (permalink / raw) To: Daniel Vetter; +Cc: airlied, kraxel, dri-devel, virtualization [-- Attachment #1.1.1: Type: text/plain, Size: 3279 bytes --] Hi Am 13.01.20 um 00:00 schrieb Daniel Vetter: > On Fri, Jan 10, 2020 at 12:57:07PM +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. Replace cirrus' >> VBLANK events with the DRM core's functionality. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/cirrus/cirrus.c | 10 ++-------- >> 1 file changed, 2 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c >> index 248c9f765c45..4a1729aa7e53 100644 >> --- a/drivers/gpu/drm/cirrus/cirrus.c >> +++ b/drivers/gpu/drm/cirrus/cirrus.c >> @@ -38,7 +38,6 @@ >> #include <drm/drm_modeset_helper_vtables.h> >> #include <drm/drm_probe_helper.h> >> #include <drm/drm_simple_kms_helper.h> >> -#include <drm/drm_vblank.h> >> >> #define DRIVER_NAME "cirrus" >> #define DRIVER_DESC "qemu cirrus vga" >> @@ -415,6 +414,8 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, >> { >> struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; >> >> + crtc_state->no_vblank = true; > > Huh, nice untended use of this stuff ... We've added this for writeback, > but I guess it can be used for anything that's a virtual connector ... Oh, 'improved by accident'. I'm not quite sure what you mean by virtual connector, but it should work with any CRTC without VBLANK support. At least I've never seen any problem with ast and udl. I'll update the docs accordingly. Best regards Thomas > > I've also spotted that you've done this same trick for ast & udl already. > But I think before we roll this out massively we should make this > official. Can you pls do a patch to update the kerneldoc for @no_vblank > that virtual hw can also use this stuff? > > Also, computing state values in atomic_commit code is kinda uncool and > fraught with peril - design assumption is that with some very few > exceptions (which are kinda awkward, would be nice to make state pointers > const) all the core and helper codes that state structures stay unchanged > after atomic_check completed. This should be computed in atomic_check (like vc4 > does). Can you pls also include patches to update ast and udl in this > series? > > Thanks, Daniel > > >> + >> cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb); >> cirrus_fb_blit_fullscreen(plane_state->fb); >> } >> @@ -434,13 +435,6 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, >> >> if (drm_atomic_helper_damage_merged(old_state, state, &rect)) >> cirrus_fb_blit_rect(pipe->plane.state->fb, &rect); >> - >> - if (crtc->state->event) { >> - spin_lock_irq(&crtc->dev->event_lock); >> - drm_crtc_send_vblank_event(crtc, crtc->state->event); >> - crtc->state->event = NULL; >> - spin_unlock_irq(&crtc->dev->event_lock); >> - } >> } >> >> static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = { >> -- >> 2.24.1 >> > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/cirrus: Let DRM core send VBLANK events 2020-01-13 9:01 ` Thomas Zimmermann @ 2020-01-14 3:52 ` Daniel Vetter 0 siblings, 0 replies; 4+ messages in thread From: Daniel Vetter @ 2020-01-14 3:52 UTC (permalink / raw) To: Thomas Zimmermann Cc: Dave Airlie, Gerd Hoffmann, dri-devel, open list:VIRTIO CORE, NET... On Mon, Jan 13, 2020 at 10:01 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi > > Am 13.01.20 um 00:00 schrieb Daniel Vetter: > > On Fri, Jan 10, 2020 at 12:57:07PM +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. Replace cirrus' > >> VBLANK events with the DRM core's functionality. > >> > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >> --- > >> drivers/gpu/drm/cirrus/cirrus.c | 10 ++-------- > >> 1 file changed, 2 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c > >> index 248c9f765c45..4a1729aa7e53 100644 > >> --- a/drivers/gpu/drm/cirrus/cirrus.c > >> +++ b/drivers/gpu/drm/cirrus/cirrus.c > >> @@ -38,7 +38,6 @@ > >> #include <drm/drm_modeset_helper_vtables.h> > >> #include <drm/drm_probe_helper.h> > >> #include <drm/drm_simple_kms_helper.h> > >> -#include <drm/drm_vblank.h> > >> > >> #define DRIVER_NAME "cirrus" > >> #define DRIVER_DESC "qemu cirrus vga" > >> @@ -415,6 +414,8 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, > >> { > >> struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; > >> > >> + crtc_state->no_vblank = true; > > > > Huh, nice untended use of this stuff ... We've added this for writeback, > > but I guess it can be used for anything that's a virtual connector ... > > Oh, 'improved by accident'. > > I'm not quite sure what you mean by virtual connector, but it should > work with any CRTC without VBLANK support. At least I've never seen any > problem with ast and udl. I'll update the docs accordingly. There's a pretty huge difference between "real vblank support", which means the various vblank/crtc_sequence ioctls work, and the fake vblank even stuff we send out for all drivers right away to fulfill the atomic uapi event requirements. We'll need to highlight that stuff in the documentation I think ... On second thought, maybe we could change the atomic helpers to automatically set no_vblank when the driver/crtc doesn't support real vblanks. But is even more involved (but I think might also be the even neater soluation, but hard to be sure without typing it all up). Oh and because hilarious historical accents "no real vblank" = drm_dev->num_crtcs == 0, which we might want to put into a helper if we leak this out of the drm_vblank.c source files. Cheers, Daniel > > Best regards > Thomas > > > > > I've also spotted that you've done this same trick for ast & udl already. > > But I think before we roll this out massively we should make this > > official. Can you pls do a patch to update the kerneldoc for @no_vblank > > that virtual hw can also use this stuff? > > > > Also, computing state values in atomic_commit code is kinda uncool and > > fraught with peril - design assumption is that with some very few > > exceptions (which are kinda awkward, would be nice to make state pointers > > const) all the core and helper codes that state structures stay unchanged > > after atomic_check completed. This should be computed in atomic_check (like vc4 > > does). Can you pls also include patches to update ast and udl in this > > series? > > > > Thanks, Daniel > > > > > >> + > >> cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb); > >> cirrus_fb_blit_fullscreen(plane_state->fb); > >> } > >> @@ -434,13 +435,6 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, > >> > >> if (drm_atomic_helper_damage_merged(old_state, state, &rect)) > >> cirrus_fb_blit_rect(pipe->plane.state->fb, &rect); > >> - > >> - if (crtc->state->event) { > >> - spin_lock_irq(&crtc->dev->event_lock); > >> - drm_crtc_send_vblank_event(crtc, crtc->state->event); > >> - crtc->state->event = NULL; > >> - spin_unlock_irq(&crtc->dev->event_lock); > >> - } > >> } > >> > >> static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = { > >> -- > >> 2.24.1 > >> > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-14 3:52 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-10 11:57 [PATCH] drm/cirrus: Let DRM core send VBLANK events Thomas Zimmermann 2020-01-12 23:00 ` Daniel Vetter 2020-01-13 9:01 ` Thomas Zimmermann 2020-01-14 3:52 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).