On Sun, Feb 15, 2015 at 04:08:31PM +0200, Laurent Pinchart wrote: > Hi Daniel, > > Thank you for the patch. > > On Friday 13 February 2015 21:03:42 Daniel Vetter wrote: > > At driver load we need to tell the vblank code about the state of the > > pipes, so that the logic around reject vblank_get when the pipe is off > > works correctly. > > > > Thus far i915 used drm_vblank_off, but one of the side-effects of it > > is that it also saves the vblank counter. And for that it calls down > > into the ->get_vblank_counter hook. Which isn't really a good idea > > when the pipe is off for a few reasons: > > - With runtime pm the register might not respond. > > - If the pipe is off some datastructures might not be around or > > unitialized. > > > > The later is what blew up on gen3: We look at intel_crtc->config to > > compute the vblank counter, and for a disabled pipe at boot-up that's > > just not there. Thus far this was papered over by a check for > > intel_crtc->active, but I want to get rid of that (since it's fairly > > race, vblank hooks are called from all kinds of places). > > > > So prep for that by adding a _reset functions which only does what we > > really need to be done at driver load: Mark the vblank pipe as off, > > but don't do any vblank counter saving or event flushing - neither of > > that is required. > > > > v2: Clarify the code flow slightly as suggested by Ville. > > > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx> > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > --- > > drivers/gpu/drm/drm_irq.c | 32 ++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_display.c | 6 +++--- > > include/drm/drmP.h | 1 + > > 3 files changed, 36 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > > index 75647e7f012b..1e5fb1b994d7 100644 > > --- a/drivers/gpu/drm/drm_irq.c > > +++ b/drivers/gpu/drm/drm_irq.c > > @@ -1226,6 +1226,38 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc) > > EXPORT_SYMBOL(drm_crtc_vblank_off); > > > > /** > > + * drm_crtc_vblank_reset - reset vblank state to off on a CRTC > > + * @crtc: CRTC in question > > + * > > + * Drivers can use this function to reset the vblank state to off at load > > time. > > + * Drivers should use this together with the drm_crtc_vblank_off() and > > + * drm_crtc_vblank_on() functions. The diffrence comparet to > > s/diffrence comparet/difference compared/ > > With that fixed, > > Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> Fixed while applying, thanks for the feedback. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx