[Intel-gfx] [PATCH v2 2/3] drm/i915/frontbuffer: HW tracking for cursor moves to fix PSR lags.

[Rodrigo Vivi]

On Wed, Mar 07, 2018 at 03:23:13PM -0800, Rodrigo Vivi wrote:
> On Wed, Mar 07, 2018 at 11:10:35PM +0000, Pandiyan, Dhinakaran wrote:
> > On Wed, 2018-03-07 at 22:53 +0000, Chris Wilson wrote:
> > > Quoting Dhinakaran Pandiyan (2018-03-07 03:34:19)
> > > > DRM_IOCTL_MODE_CURSOR results in frontbuffer flush before the cursor
> > > > plane MMIOs are written to. But this flush should not be necessary for
> > > > PSR as hardware tracking triggers PSR exit when MMIOs are written. As
> > > > for FBC, the spec says "Flips or changes to plane size and panning" cause
> > > > FBC to be nuked. Use origin == ORIGIN_FLIP so that features can ignore
> > > > cursor updates in their frontbuffer_flush implementations.
> > > > 
> > > >  /sys/kernel/debug/dri/0/i915_fbc_status shows
> > > > "Compressing: yes" when I move the cursor around.
> > > > 
> > > > v3: Use ORIGIN_FLIP now that pin_to_display does not flush frontbuffer.
> > > > v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)
> > > > 
> > > > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 91ce8a0522a3..18b08e263ee1 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -13176,7 +13176,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
> > > >         if (ret)
> > > >                 goto out_unlock;
> > > >  
> > > > -       intel_fb_obj_flush(intel_fb_obj(fb), ORIGIN_DIRTYFB);
> > > > +       intel_fb_obj_flush(intel_fb_obj(fb), ORIGIN_FLIP);
> > > 
> > > What about prepare_plane? That should reduce to ORIGIN_FLIP as well,
> > > aiui.
> > > -Chris
> > 
> > 
> > That was the idea but there's a problem with not knowing if PSR exit is
> > fully complete before we begin updating the plane registers in
> > pipe_update_start().
> > 
> > Let's say PSR was active and display is in DC6. A flip comes in, without
> > _flush(DIRTYFB) in prepare_plane_fb(), PSR exit is delayed until vblank
> > enabling that happens in pipe_update_start. We immediately follow that
> > with programming the plane MMIO's without checking if PSR fully exited.
> > If PSR and DC6 happen to exit while we were in the middle of programming
> > plane MMIO's, the resulting vblank toggle (from PSR exit) might activate
> > partially programmed registers. _flush(DIRTYFB) gives us an opportunity
> > to exit PSR fully by starting early.
> > 
> > As for legacy_cursor_update(), since there is no vblank enabling
> > involved, we avoid updating the MMIO's in the midst of PSR exit
> 
> I don't believe you will be ever in a case that you write to any register
> and get any flip or anything without exiting DC6 before that happens.
> 
> Or the CSR mechanism of DC6 will be simply wrong.
> 
> Would this be enough?

ok... just ignore my previous comment...
I believe we can move with the safest side and maybe revisit this later.

>From what I remember of the FBC nuke needs as well I believe this is
the right move.

Although I'm asking myself now if we are not changing the meaning of the
ORIGINS here. Shouldn't we add a new origin and update the handling?

of "flip" is a good description for this call?

> 
> > 
> > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx