Re: [PATCH] drm/i915: Fix frontbuffer false positve.

Rodrigo Vivi
2015-02-03T18:46:49Z

On Tue, Feb 3, 2015 at 8:21 AM, Matt Roper <matthew.d.roper@intel.com> 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@ffwll.ch>
>> > Cc: Sonika Jindal <sonika.jindal@intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>
>> 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.

>>
>> 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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br