[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 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?

> 
> 
> > _______________________________________________
> > 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