On Wed, Jun 08, 2016 at 04:17:55PM +0200, Maarten Lankhorst wrote: > Op 08-06-16 om 14:18 schreef Daniel Vetter: > > atomic_flush seems to be the right place, but I'm not entirely sure > > whether this will catch them all. It could be that when disabling the > > crtc we'll miss the vblank. > > > > While at it nuke the dummy functions. > > > > v2: Be more robust and either arm, when the CRTC is on, or just send > > the event out right away. > > > > Cc: Xinliang Liu <xinliang.liu@xxxxxxxxxx> > > Cc: Xinwei Kong <kong.kongxinwei@xxxxxxxxxxxxx> > > Cc: Archit Taneja <architt@xxxxxxxxxxxxxx> > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > --- > > drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 20 ++++++++++++-------- > > 1 file changed, 12 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c > > index fba6372d060e..ed76baad525f 100644 > > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c > > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c > > @@ -502,13 +502,6 @@ static void ade_crtc_disable(struct drm_crtc *crtc) > > acrtc->enable = false; > > } > > > > -static int ade_crtc_atomic_check(struct drm_crtc *crtc, > > - struct drm_crtc_state *state) > > -{ > > - /* do nothing */ > > - return 0; > > -} > > - > > static void ade_crtc_mode_set_nofb(struct drm_crtc *crtc) > > { > > struct ade_crtc *acrtc = to_ade_crtc(crtc); > > @@ -537,6 +530,7 @@ static void ade_crtc_atomic_flush(struct drm_crtc *crtc, > > { > > struct ade_crtc *acrtc = to_ade_crtc(crtc); > > struct ade_hw_ctx *ctx = acrtc->ctx; > > + struct drm_pending_vblank_event *event = crtc->state->event; > > void __iomem *base = ctx->base; > > > > /* only crtc is enabled regs take effect */ > > @@ -545,12 +539,22 @@ static void ade_crtc_atomic_flush(struct drm_crtc *crtc, > > /* flush ade registers */ > > writel(ADE_ENABLE, base + ADE_EN); > > } > > + > > + if (event) { > > + crtc->state->event = NULL; > ^I keep wondering why we set this to NULL every time. Nothing should depend on this right? duplicate_state sets this member to NULL.. > I'd rather have crtc_state be constified after commit. Other than that.. I added a WARN_ON in hw_done to make sure drivers have indeed consumed the event. Given that lots of drivers don't bother I think that's good, since I want to avoid getting flooded with bug reports along the lines of "my atomic driver hangs" with these new helpers. WARN_ON(state->event) in hw_done together with the commen is hopefully hint enough. -Daniel > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > ~Maarten -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel