Re: [PATCH] drm/i915: drrs_invalidate at flip schedule

Chris Wilson
2015-05-15T13:56:45Z

On Fri, May 15, 2015 at 06:54:54PM +0530, Ramalingam C wrote:
>
> On Friday 15 May 2015 05:28 PM, Chris Wilson wrote:
> >On Fri, May 15, 2015 at 02:08:22AM +0530, Ramalingam C wrote:
> >>After scheduling a flip for obj, we are supposed to invalidate the
> >>drrs.
> >>
> >>Action:
> >> Adding a call to intel_edp_drrs_invalidate at
> >> intel_frontbuffer_flip_prepare.
> >>
> >>Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> >>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Just Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >References: https://bugs.freedesktop.org/show_bug.cgi?id=90418
> >
> >Ok, looks correct. This invalidate will be paired with a flush after the
> >flip completes to reschedule the downclock of the refresh rates.
> >
> >I think a comment would be useful to explain the relationship here, or
> >better would be a new intel_edp_drrs_flip_prepare() stub so that the
> >internal details of drrs are kept out of intel_frontbuffer.c and the
> >comment can refer to the adjacent functions for reference.
> But in flip preparation we would want to invalidate the PSR
> (software implementation) also.
> In that case we could create a function called
> intel_frontbuffer_flip_invalidate() instead of
> edp_drrs_flip_prepare.
> This will be invoking the invalidation for the PSR and DRRS. And
> this function could be called from
> intel_frontbuffer_flip_prepare().
>
> Incase If FBC invalidate also needed at flip preparation, then we
> could create a common function called
> intel_frontbuffer_invalidate parallel to intel_frontbuffer_flush
> which will be used by
> intel_fb_obj_invalidate and intel_frontbuffer_flip_prepare.
>
> Please share your view. whether FBC invalidate is required on flip
> preparation?

It is (intel_disable_fbc is being directly by the flip code).

I would stick to calling it intel_frontbuffer_flip_prepare/flip_complete
to distinguish it from invalidate/flush - they are not equivalent in all
cases. And I would push that naming convention down to the backends.
Given that we have 3 (almost 4) users of this, we may want to move this
over to a notifier system and have the backends register themselves
rather than continually adapting intel_frontbuffer.c to the requirements
of different backends. Task for a rainy day.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre