On Tue, Feb 03, 2015 at 10:46:49AM -0800, Rodrigo Vivi wrote: > On Tue, Feb 3, 2015 at 8:21 AM, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote: > > On Tue, Feb 03, 2015 at 12:57:31PM +0100, Daniel Vetter wrote: > >> On Mon, Feb 02, 2015 at 03:38:16PM -0800, Rodrigo Vivi wrote: > >> > frontbuffer bits must be updated during commit times not on atomica prepare > >> > one, otherwise we have a risk of false positive. > >> > > >> > Cc Daniel Vetter <daniel.vetter@xxxxxxxx> > >> > Cc: Sonika Jindal <sonika.jindal@xxxxxxxxx> > >> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > >> > >> atomic.fb_bits isn't used at all right now, instead the > >> begin_crtc_commit function recomputes them. That looks wrong. > > > > We build up the collection of bits in atomic.fb_bits while going through > > the atomic pipeline for each plane, then do a single call to > > > > intel_frontbuffer_flip(dev, intel_crtc->atomic.fb_bits); > > > > in intel_finish_crtc_commit to flush them all out together (and if > > check/prepare fail, we never actually get to that flush). > > > >> Also for async commits we need > >> to do the proper 2-stage flip stuff that current page_flip code does using > >> frontbuffer_flip_prepare/complete. > > > > I need to look at the frontbuffer stuff again...maybe we don't need > > atomic.fb_bits at all and should just use intel_frontbuffer_flip_prepare > > in the places we set the bits now and intel_frontbuffer_flip_complete > > where we're calling the flip mentioned above? > > > > > > Matt > > > >> > >> This patch here should have 0 effect (presuming I'm reading code > >> correctly), so what kinf of bug exactly are you seeing? > > Yeah, after I sent the patch I was thinking about that: that it should > have 0 effect, > but the symptom that this apparently fixed here is that PSR wasn't > starting at all without > doing something like going to fbcon and come back to X. > Something similar what Sonika had told on that psr-skl thread where > she couldn't get psr working on login screen. > After this patch I didn't' have to change back and forth to fbcon to > make psr work. > > Maybe just a coincidence, but anyway I believe the way it is nowadays > it is wrong. I believe frontbuffer bits and calls should be done at > commit step, not under prepare. I may be misunderstanding what you're saying, but I think this is the way things already work? We plan out which bits we're going to update in the 'check' step (and record those bits in atomic.fb_bits), but then we only actually set them with intel_frontbuffer_flip() in commit if everything looks good. If we wind up rejecting the update because we fail to check/prepare, those bits in atomic.fb_bits should just get thrown away, unless I'm missing something. Matt > > >> > >> Adding Matt&Ander. > >> -Daniel > >> -- > >> Daniel Vetter > >> Software Engineer, Intel Corporation > >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > -- > > Matt Roper > > Graphics Software Engineer > > IoTG Platform Enabling & Development > > Intel Corporation > > (916) 356-2795 > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx