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

["Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>]

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@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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


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