On Mon, Oct 26, 2015 at 09:31:48AM +0100, Maarten Lankhorst wrote: > Op 22-10-15 om 17:15 schreef Ville Syrjälä: > > On Thu, Oct 22, 2015 at 04:50:05PM +0200, Maarten Lankhorst wrote: > >> Op 22-10-15 om 15:30 schreef Ville Syrjälä: > >>> On Thu, Oct 22, 2015 at 01:56:28PM +0200, Maarten Lankhorst wrote: > >>>> By handling this after the atomic helper waits for vblanks there will > >>>> be one less wait for vblank in the atomic path. > >>>> > >>>> Also get rid of the double wait_for_vblank on broadwell, looks like > >>>> it's a bug doing the vblank wait twice. > >>>> > >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > >>>> --- > >>>> drivers/gpu/drm/i915/intel_display.c | 28 +++------------------------- > >>>> drivers/gpu/drm/i915/intel_drv.h | 1 - > >>>> 2 files changed, 3 insertions(+), 26 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >>>> index 051a1e2b1c55..b8e1a5471bed 100644 > >>>> --- a/drivers/gpu/drm/i915/intel_display.c > >>>> +++ b/drivers/gpu/drm/i915/intel_display.c > >>>> @@ -4672,14 +4672,6 @@ intel_post_enable_primary(struct drm_crtc *crtc) > >>>> int pipe = intel_crtc->pipe; > >>>> > >>>> /* > >>>> - * BDW signals flip done immediately if the plane > >>>> - * is disabled, even if the plane enable is already > >>>> - * armed to occur at the next vblank :( > >>>> - */ > >>>> - if (IS_BROADWELL(dev)) > >>>> - intel_wait_for_vblank(dev, pipe); > >>> The right fix for this crap is to not use the flip done interrupt. But > >>> apparently no one wants to fix that. > >> This is fixed later on when converting page_flip to atomic. I've modified > >> page_flip_finished to ignore flips when visible_changed happens on broadwell. > > Sounds like another hack. > It's not a hack when it's a workaround for buggy hw. ;-) > > > >>> So now I'm worried that we now depend on the atomic helper doing > >>> synchornous vblank waits here. Are we expecting those vblank waits to > >>> remain there? I would have expected to get rid of all synchronois vblank > >>> waits in plane codepaths (apart from ones for hw workarounds). > >> In my unify page flip and atomic series I first add all post_plane_update and > >> unpinning to unpin_work_fn. > >> > >> After that I have a patch to convert the last part of atomic after wait_for_vblanks > >> to schedule unpin_work_fn, next I'll get more bold and zap the wait_for_vblanks and run it async. > >> It depends on work->can_async_unpin, if it's false it has to wait for everything to finish before queueing the next flip, > >> so it will finish before next one starts. > >> > >>> Also does the helper actually wait for vblanks when the plane gets > >>> enabled w/o the framebuffer actually changing? > >> afaict it does, as long as the crtc is enabled.a > > Where is that code? All I see is a old_fb != new_fb check. > Oh indeed, I missed that one. That would make it unreliable to use though. :-/ > > I think drm_atomic_helper_wait_for_vblanks should be fixed to look at crtc->state->active and maybe take a force flag to force a vblank wait.. > > Daniel would you be ok with such a patch? The helper vblank wait is just to avoid tearing, which can't happen if the fb doesn't change. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx