Re: [PATCH v2 4/6] drm/i915: Make commit call blocking in case of async flips

[Karthik B S <karthik.b.s@xxxxxxxxx>]

On 4/24/2020 11:16 PM, Paulo Zanoni wrote:
> Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
> > Make the commit call blocking in case of async flips
> > so that there is no delay in completing the flip.
>
> I'm trying to understand the code. Can you please elaborate more here
> in this commit message? Why exactly does the call need to block? What
> would be the problem of not having this patch? And how does blocking
> ensures no delay?

My initial assumption was that blocking call would ensure lesser delay 
as commit tail is immediately called, but after running some 
benchmarking tests, its clearly not the case.

So no point in having this, removed this patch in V3.

Thanks,
Karthik.B.S
> > v2: -Rebased
> >
> > Signed-off-by: Karthik B S <karthik.b.s@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++-----
> >   1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 8601b159f425..a5203de24045 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -15563,7 +15563,9 @@ static int intel_atomic_commit(struct drm_device *dev,
> >   {
> >   	struct intel_atomic_state *state = to_intel_atomic_state(_state);
> >   	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	int ret = 0;
> > +	struct intel_crtc_state *new_crtc_state;
> > +	struct intel_crtc *crtc;
> > +	int ret = 0, i;
> >   
> >   	state->wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> >   
> > @@ -15589,10 +15591,6 @@ static int intel_atomic_commit(struct drm_device *dev,
> >   	 * (assuming we had any) would solve these problems.
> >   	 */
> >   	if (INTEL_GEN(dev_priv) < 9 && state->base.legacy_cursor_update) {
> > -		struct intel_crtc_state *new_crtc_state;
> > -		struct intel_crtc *crtc;
> > -		int i;
> > -
> >   		for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
> >   			if (new_crtc_state->wm.need_postvbl_update ||
> >   			    new_crtc_state->update_wm_post)
> > @@ -15634,6 +15632,13 @@ static int intel_atomic_commit(struct drm_device *dev,
> >   	drm_atomic_state_get(&state->base);
> >   	INIT_WORK(&state->base.commit_work, intel_atomic_commit_work);
> >   
> > +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> > +		if (new_crtc_state->uapi.async_flip) {
> > +			nonblock = false;
> > +			break;
> > +		}
> > +	}
> > +
> >   	i915_sw_fence_commit(&state->commit_ready);
> >   	if (nonblock && state->modeset) {
> >   		queue_work(dev_priv->modeset_wq, &state->base.commit_work);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx