[PATCH v8 1/2] drm/i915: Introduce async plane update to i915

[Jani Nikula]

On Fri, 08 Mar 2019, Helen Koike <helen.koike at collabora.com> wrote:
> Hi Jani,
>
> On 3/8/19 6:55 AM, Jani Nikula wrote:
>> 
>> Please send drm/i915 patches also to intel-gfx at lists.freedesktop.org to
>> trigger CI on the patches among other things.
>
> Sure! Is there a way to tell the CI to pick up the dependency [1] as well?

No. You should post these as part of a self-contained series. Looks like
you already have other driver changes in the series.

BR,
Jani.


>
> [1] "[PATCH] drm: don't block fb changes for async plane updates"
>
> Regards,
> Helen
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> On Thu, 07 Mar 2019, Helen Koike <helen.koike at collabora.com> wrote:
>>> From: Gustavo Padovan <gustavo.padovan at collabora.com>
>>>
>>> Add implementation for async plane update callbacks
>>>
>>> Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.com>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo at collabora.com>
>>> Signed-off-by: Tina Zhang <tina.zhang at intel.com>
>>> Signed-off-by: Helen Koike <helen.koike at collabora.com>
>>>
>>> ---
>>> Hi,
>>>
>>> This patch just adds the callbacks for atomic async update, the next in
>>> the series will replace the legacy cursor implementation with these
>>> callbacks.
>>>
>>> I tested with igt (ksm_cursor_legacy and plane_cursor_legacy).
>>>
>>> This patch depends on "[PATCH] drm: don't block fb changes for async plane updates",
>>> otherwise there will be a regression on igt tests:
>>>
>>>         cursor-vs-flip-atomic-transitions-varying-size
>>>         cursor-vs-flip-toggle
>>>         cursor-vs-flip-varying-size
>>>
>>> with errors of type:
>>>
>>> "CRITICAL: completed 97 cursor updated in a period of 30 flips, we
>>> expect to complete approximately 15360 updates, with the threshold set
>>> at 7680"
>>>
>>> This happens because what should be async updates are being turned into
>>> syncronous updates.
>>>
>>> Thanks
>>> Helen
>>>
>>> Changes in v8:
>>>  - v7: https://lkml.org/lkml/2018/6/8/168
>>>  - v7 was splited in two, one that adds the async callbacks and another
>>>  that updates the cursor.
>>>  - rebase with drm-intel
>>>  - allow async update in all types of planes, not only cursor
>>>  - add watermark checks in async update
>>>  - remove bypass of intel_prepare_plane_fb() in case of async update
>>>  - add missing drm_atomic_helper_cleanup_planes(dev, state) call in
>>>  intel_atomic_commit().
>>>  - use swap() function in async update to set the old_fb in the
>>>  new_state object.
>>>  - use helpers intel_update_plane()/intel_disable_plane()
>>>
>>> Changes in v7:
>>> - Rebase on top of drm-intel repository. Hopefully now will play
>>>   nicely with autobuilders.
>>>
>>> Changes in v6:
>>> - Rework the intel_plane_atomic_async_update due driver changed from
>>>   last time.
>>> - Removed the mutex_lock/unlock as causes a deadlock.
>>>
>>> Changes in v5:
>>> - Call drm_atomic_helper_async_check() from the check hook
>>>
>>> Changes in v4:
>>> - Set correct vma to new state for cleanup
>>> - Move size checks back to drivers (Ville Syrjälä)
>>>
>>> Changes in v3:
>>> - Move fb setting to core and use new state (Eric Anholt)
>>>
>>>  drivers/gpu/drm/i915/intel_atomic_plane.c | 71 +++++++++++++++++++++++
>>>  drivers/gpu/drm/i915/intel_display.c      |  9 +++
>>>  2 files changed, 80 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> index 7961cf0e6951..f516b227dba9 100644
>>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> @@ -318,10 +318,81 @@ void i9xx_update_planes_on_crtc(struct intel_atomic_state *state,
>>>  	}
>>>  }
>>>  
>>> +static int intel_plane_atomic_async_check(struct drm_plane *plane,
>>> +					  struct drm_plane_state *state)
>>> +{
>>> +	struct drm_crtc_state *crtc_state;
>>> +
>>> +	crtc_state = drm_atomic_get_existing_crtc_state(state->state,
>>> +							state->crtc);
>>> +	if (WARN_ON(!crtc_state))
>>> +		return -EINVAL;
>>> +
>>> +	/*
>>> +	 * When crtc is inactive or there is a modeset pending,
>>> +	 * wait for it to complete in the slowpath
>>> +	 */
>>> +	if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
>>> +		return -EINVAL;
>>> +
>>> +	/*
>>> +	 * If any parameters change that may affect watermarks,
>>> +	 * take the slowpath. Only changing fb or position should be
>>> +	 * in the fastpath.
>>> +	 */
>>> +	if (plane->state->crtc != state->crtc ||
>>> +	    plane->state->src_w != state->src_w ||
>>> +	    plane->state->src_h != state->src_h ||
>>> +	    plane->state->crtc_w != state->crtc_w ||
>>> +	    plane->state->crtc_h != state->crtc_h ||
>>> +	    !plane->state->fb != !state->fb)
>>> +		return -EINVAL;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void intel_plane_atomic_async_update(struct drm_plane *plane,
>>> +					    struct drm_plane_state *new_state)
>>> +{
>>> +	struct intel_atomic_state *intel_new_state =
>>> +		to_intel_atomic_state(new_state->state);
>>> +	struct intel_plane *intel_plane = to_intel_plane(plane);
>>> +	struct drm_crtc *crtc = plane->state->crtc;
>>> +	struct intel_crtc_state *new_crtc_state;
>>> +	struct intel_crtc *intel_crtc;
>>> +	int i;
>>> +
>>> +	for_each_new_intel_crtc_in_state(intel_new_state, intel_crtc,
>>> +					 new_crtc_state, i)
>>> +		WARN_ON(new_crtc_state->wm.need_postvbl_update ||
>>> +			new_crtc_state->update_wm_post);
>>> +
>>> +	i915_gem_track_fb(intel_fb_obj(plane->state->fb),
>>> +			  intel_fb_obj(new_state->fb),
>>> +			  intel_plane->frontbuffer_bit);
>>> +
>>> +	plane->state->src_x = new_state->src_x;
>>> +	plane->state->src_y = new_state->src_y;
>>> +	plane->state->crtc_x = new_state->crtc_x;
>>> +	plane->state->crtc_y = new_state->crtc_y;
>>> +
>>> +	swap(plane->state->fb, new_state->fb);
>>> +
>>> +	if (plane->state->visible)
>>> +		intel_update_plane(intel_plane,
>>> +				   to_intel_crtc_state(crtc->state),
>>> +				   to_intel_plane_state(plane->state));
>>> +	else
>>> +		intel_disable_plane(intel_plane,
>>> +				    to_intel_crtc_state(crtc->state));
>>> +}
>>> +
>>>  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>>>  	.prepare_fb = intel_prepare_plane_fb,
>>>  	.cleanup_fb = intel_cleanup_plane_fb,
>>>  	.atomic_check = intel_plane_atomic_check,
>>> +	.atomic_async_check = intel_plane_atomic_async_check,
>>> +	.atomic_async_update = intel_plane_atomic_async_update,
>>>  };
>>>  
>>>  /**
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 415d8968f2c5..244e1c94277d 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -13464,6 +13464,15 @@ static int intel_atomic_commit(struct drm_device *dev,
>>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>>  	int ret = 0;
>>>  
>>> +	if (state->async_update) {
>>> +		ret = drm_atomic_helper_prepare_planes(dev, state);
>>> +		if (ret)
>>> +			return ret;
>>> +		drm_atomic_helper_async_commit(dev, state);
>>> +		drm_atomic_helper_cleanup_planes(dev, state);
>>> +		return 0;
>>> +	}
>>> +
>>>  	drm_atomic_state_get(state);
>>>  	i915_sw_fence_init(&intel_state->commit_ready,
>>>  			   intel_atomic_commit_ready);
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center