On Tue, Jan 18, 2022 at 12:48:38PM +0200, Stanislav Lisovskiy wrote:
> This optimization allows to achieve higher perfomance
> during async flips.
> For the first async flip we have to still temporarily
> switch to sync flip, in order to reprogram plane
> watermarks, so this requires taking into account
> old plane state's do_async_flip flag.
>
> v2: - Removed redundant new_plane_state->do_async_flip
> check from needs_async_flip_wm_override condition
> (Ville Syrjälä)
> - Extract dg2_async_flip_optimization to separate
> function(Ville Syrjälä)
> - Check for plane->async_flip instead of plane_id
> (Ville Syrjälä)
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++++++-
> drivers/gpu/drm/i915/intel_pm.c | 12 +++++++++++-
> 2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f3ce29c42bc3..9a5126310014 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4908,6 +4908,15 @@ static bool needs_scaling(const struct intel_plane_state *state)
> return (src_w != dst_w || src_h != dst_h);
> }
>
> +static bool needs_async_flip_wm_override(struct intel_plane *plane,
> + const struct intel_plane_state *new_plane_state,
> + const struct intel_plane_state *old_plane_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
I'd probably put all the require logic into this one function.
Perhaps somehting like?
intel_plane_do_async_flip()
{
if (!plane->async_flip)
return false;
if (!new_crtc_state->uapi.async_flip)
return false;
/* some explanation what's going on here */
return DISPLAY_VER < 13 || old_crtc_state->uapi.async_flip;
}
>+
> + return DISPLAY_VER(dev_priv) >= 13 && !old_plane_state->do_async_flip;
> +}
> +
> int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
> struct intel_crtc_state *new_crtc_state,
> const struct intel_plane_state *old_plane_state,
> @@ -5027,7 +5036,8 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
> needs_scaling(new_plane_state))))
> new_crtc_state->disable_lp_wm = true;
>
> - if (new_crtc_state->uapi.async_flip && plane->async_flip)
> + if (new_crtc_state->uapi.async_flip &&
> + !needs_async_flip_wm_override(plane, new_plane_state, old_plane_state))
> new_plane_state->do_async_flip = true;
> else
> new_plane_state->do_async_flip = false;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index dc1203d21c46..5d350ddc447f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5510,6 +5510,15 @@ static int skl_wm_max_lines(struct drm_i915_private *dev_priv)
> return 31;
> }
>
> +bool dg2_async_flip_optimization(struct drm_i915_private *i915,
> + const struct intel_crtc_state *crtc_state,
> + const struct intel_plane *plane)
I'd drop the platform specific suffix from this since it's more of
a policy quesion rather than really a platform specific question.
We may or may not want to extend this to other platforms too if we
figure out that it can actually help (although my initial quick
tests did show a potentially negative impact for some unknown reason).
In fact maybe it should be just called use_minimal_wm0_only()
or something since the callers don't really need to know why it's
being done, only that we want to limit to just wm0 with minimal ddb
allocation?
> +{
> + return DISPLAY_VER(i915) >= 13 &&
> + crtc_state->uapi.async_flip &&
> + plane->async_flip;
> +}
> +
> static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> const struct intel_plane *plane,
> int level,
> @@ -5523,7 +5532,8 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
> uint_fixed_16_16_t selected_result;
> u32 blocks, lines, min_ddb_alloc = 0;
>
> - if (latency == 0) {
> + if (latency == 0 ||
> + (dg2_async_flip_optimization(dev_priv, crtc_state, plane) && level > 0)) {
> /* reject it */
> result->min_ddb_alloc = U16_MAX;
> return;
> --
> 2.24.1.485.gad05a3d8e5
--
Ville Syrjälä
Intel
