Op 20-04-16 om 04:26 schreef Matt Roper:
> Once we move watermark calculation to the atomic check phase, we'll want
> to start rejecting display configurations that exceed out watermark
> limits. At the moment we just assume that there's always a valid set of
> watermarks, even though this may not actually be true. Let's prepare by
> passing return codes up through the call stack in preparation.
>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 10 ++--
> drivers/gpu/drm/i915/intel_pm.c | 90 ++++++++++++++++++++++--------------
> 2 files changed, 61 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 80d8f77..7e8fa88 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13312,7 +13312,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> * phase. The code here should be run after the per-crtc and per-plane 'check'
> * handlers to ensure that all derived state has been updated.
> */
> -static void calc_watermark_data(struct drm_atomic_state *state)
> +static int calc_watermark_data(struct drm_atomic_state *state)
> {
> struct drm_device *dev = state->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -13348,7 +13348,9 @@ static void calc_watermark_data(struct drm_atomic_state *state)
>
> /* Is there platform-specific watermark information to calculate? */
> if (dev_priv->display.compute_global_watermarks)
> - dev_priv->display.compute_global_watermarks(state);
> + return dev_priv->display.compute_global_watermarks(state);
> +
> + return 0;
> }
>
> /**
> @@ -13432,9 +13434,7 @@ static int intel_atomic_check(struct drm_device *dev,
> return ret;
>
> intel_fbc_choose_crtc(dev_priv, state);
> - calc_watermark_data(state);
> -
> - return 0;
> + return calc_watermark_data(state);
> }
>
> static int intel_atomic_prepare_commit(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 21fde05..f07054a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3241,13 +3241,14 @@ static bool skl_ddb_allocation_changed(const struct skl_ddb_allocation *new_ddb,
> return false;
> }
>
> -static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> - struct intel_crtc_state *cstate,
> - struct intel_plane_state *intel_pstate,
> - uint16_t ddb_allocation,
> - int level,
> - uint16_t *out_blocks, /* out */
> - uint8_t *out_lines /* out */)
> +static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> + struct intel_crtc_state *cstate,
> + struct intel_plane_state *intel_pstate,
> + uint16_t ddb_allocation,
> + int level,
> + uint16_t *out_blocks, /* out */
> + uint8_t *out_lines, /* out */
> + bool *enabled /* out */)
> {
> struct drm_plane_state *pstate = &intel_pstate->base;
> struct drm_framebuffer *fb = pstate->fb;
> @@ -3259,8 +3260,10 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> uint8_t cpp;
> uint32_t width = 0, height = 0;
>
> - if (latency == 0 || !cstate->base.active || !intel_pstate->visible)
> - return false;
> + if (latency == 0 || !cstate->base.active || !intel_pstate->visible) {
> + *enabled = false;
> + return 0;
> + }
>
> width = drm_rect_width(&intel_pstate->src) >> 16;
> height = drm_rect_height(&intel_pstate->src) >> 16;
> @@ -3321,13 +3324,16 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> res_blocks++;
> }
>
> - if (res_blocks >= ddb_allocation || res_lines > 31)
> - return false;
> + if (res_blocks >= ddb_allocation || res_lines > 31) {
> + *enabled = false;
> + return 0;
> + }
>
> *out_blocks = res_blocks;
> *out_lines = res_lines;
> + *enabled = true;
>
> - return true;
> + return 0;
> }
>
> static int
> @@ -3345,6 +3351,7 @@ skl_compute_wm_level(const struct drm_i915_private *dev_priv,
> struct intel_plane_state *intel_pstate;
> uint16_t ddb_blocks;
> enum pipe pipe = intel_crtc->pipe;
> + int ret;
>
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> int i = skl_wm_plane_id(intel_plane);
> @@ -3374,13 +3381,16 @@ skl_compute_wm_level(const struct drm_i915_private *dev_priv,
>
> ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
>
> - result->plane_en[i] = skl_compute_plane_wm(dev_priv,
> - cstate,
> - intel_pstate,
> - ddb_blocks,
> - level,
> - &result->plane_res_b[i],
> - &result->plane_res_l[i]);
> + ret = skl_compute_plane_wm(dev_priv,
> + cstate,
> + intel_pstate,
> + ddb_blocks,
> + level,
> + &result->plane_res_b[i],
> + &result->plane_res_l[i],
> + &result->plane_en[i]);
> + if (ret)
> + return ret;
> }
>
> return 0;
> @@ -3417,21 +3427,26 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
> }
> }
>
> -static void skl_build_pipe_wm(struct intel_crtc_state *cstate,
> - struct skl_ddb_allocation *ddb,
> - struct skl_pipe_wm *pipe_wm)
> +static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
> + struct skl_ddb_allocation *ddb,
> + struct skl_pipe_wm *pipe_wm)
> {
> struct drm_device *dev = cstate->base.crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> int level, max_level = ilk_wm_max_level(dev);
> + int ret;
>
> for (level = 0; level <= max_level; level++) {
> - skl_compute_wm_level(dev_priv, ddb, cstate,
> - level, &pipe_wm->wm[level]);
> + ret = skl_compute_wm_level(dev_priv, ddb, cstate,
> + level, &pipe_wm->wm[level]);
> + if (ret)
> + return ret;
> }
> pipe_wm->linetime = skl_compute_linetime_wm(cstate);
>
> skl_compute_transition_wm(cstate, &pipe_wm->trans_wm);
> +
> + return 0;
> }
>
> static void skl_compute_wm_results(struct drm_device *dev,
> @@ -3678,21 +3693,27 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
> }
> }
>
> -static bool skl_update_pipe_wm(struct drm_crtc_state *cstate,
> - struct skl_ddb_allocation *ddb, /* out */
> - struct skl_pipe_wm *pipe_wm /* out */)
> +static int skl_update_pipe_wm(struct drm_crtc_state *cstate,
> + struct skl_ddb_allocation *ddb, /* out */
> + struct skl_pipe_wm *pipe_wm, /* out */
> + bool *changed /* out */)
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(cstate->crtc);
> struct intel_crtc_state *intel_cstate = to_intel_crtc_state(cstate);
> + int ret;
>
> - skl_build_pipe_wm(intel_cstate, ddb, pipe_wm);
> + ret = skl_build_pipe_wm(intel_cstate, ddb, pipe_wm);
> + if (ret)
> + return ret;
>
> if (!memcmp(&intel_crtc->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
> - return false;
> + *changed = false;
> + else
> + *changed = true;
>
> intel_crtc->wm.active.skl = *pipe_wm;
>
> - return true;
> + return 0;
> }
>
> static void skl_update_other_pipe_wm(struct drm_device *dev,
> @@ -3725,8 +3746,8 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
> if (!intel_crtc->active)
> continue;
>
> - wm_changed = skl_update_pipe_wm(intel_crtc->base.state,
> - &r->ddb, &pipe_wm);
> + skl_update_pipe_wm(intel_crtc->base.state,
> + &r->ddb, &pipe_wm, &wm_changed);
>
> /*
> * If we end up re-computing the other pipe WM values, it's
> @@ -3827,14 +3848,15 @@ static void skl_update_wm(struct drm_crtc *crtc)
> struct skl_wm_values *results = &dev_priv->wm.skl_results;
> struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal;
> -
> + bool wm_changed;
>
> /* Clear all dirty flags */
> results->dirty_pipes = 0;
>
> skl_clear_wm(results, intel_crtc->pipe);
>
> - if (!skl_update_pipe_wm(crtc->state, &results->ddb, pipe_wm))
> + skl_update_pipe_wm(crtc->state, &results->ddb, pipe_wm, &wm_changed);
> + if (!wm_changed)
> return;
>
> skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
With the nitpicks fixed, and with CI happy.
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
