Em Sex, 2016-10-07 Ã s 20:11 -0400, Lyude escreveu:
> Having skl_wm_level contain all of the watermarks for each plane is
> annoying since it prevents us from having any sort of object to
> represent a single watermark level, something we take advantage of in
> the next commit to cut down on all of the copy paste code in here.
>
> Changes since v1:
> - Style nitpicks
> - Fix accidental usage of i vs. PLANE_CURSOR
> - Split out skl_pipe_wm_active_state simplification into separate
> patch
>
> Signed-off-by: Lyude
Reviewed-by: Paulo Zanoni
> Reviewed-by: Maarten Lankhorst
> Cc: Ville Syrjälä
> Cc: Paulo Zanoni
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |   6 +-
> Â drivers/gpu/drm/i915/intel_drv.h |Â Â Â 6 +-
>  drivers/gpu/drm/i915/intel_pm.c  | 207 +++
>
> Â 3 files changed, 111 insertions(+), 108 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index e9d035ea..0287c93 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1649,9 +1649,9 @@ struct skl_wm_values {
> Â };
> Â
> Â struct skl_wm_level {
> - bool plane_en[I915_MAX_PLANES];
> - uint16_t plane_res_b[I915_MAX_PLANES];
> - uint8_t plane_res_l[I915_MAX_PLANES];
> + bool plane_en;
> + uint16_t plane_res_b;
> + uint8_t plane_res_l;
> Â };
> Â
> Â /*
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 35ba282..d684f4f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -468,9 +468,13 @@ struct intel_pipe_wm {
> Â bool sprites_scaled;
> Â };
> Â
> -struct skl_pipe_wm {
> +struct skl_plane_wm {
> Â struct skl_wm_level wm[8];
> Â struct skl_wm_level trans_wm;
> +};
> +
> +struct skl_pipe_wm {
> + struct skl_plane_wm planes[I915_MAX_PLANES];
> Â uint32_t linetime;
> Â };
> Â
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index cc5d5e9..4c2ebcd 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3670,67 +3670,52 @@ static int
> Â skl_compute_wm_level(const struct drm_i915_private *dev_priv,
> Â Â Â Â Â Â struct skl_ddb_allocation *ddb,
> Â Â Â Â Â Â struct intel_crtc_state *cstate,
> + Â Â Â Â Â struct intel_plane *intel_plane,
> Â Â Â Â Â Â int level,
> Â Â Â Â Â Â struct skl_wm_level *result)
> Â {
> Â struct drm_atomic_state *state = cstate->base.state;
> Â struct intel_crtc *intel_crtc = to_intel_crtc(cstate-
> >base.crtc);
> - struct drm_plane *plane;
> - struct intel_plane *intel_plane;
> - struct intel_plane_state *intel_pstate;
> + struct drm_plane *plane = _plane->base;
> + struct intel_plane_state *intel_pstate = NULL;
> Â uint16_t ddb_blocks;
> Â enum pipe pipe = intel_crtc->pipe;
> Â int ret;
> + int i = skl_wm_plane_id(intel_plane);
> +
> + if (state)
> + intel_pstate =
> + intel_atomic_get_existing_plane_state(state,
> + Â Â Â Â Â Â intel_
> plane);
> Â
> Â /*
> - Â * We'll only calculate watermarks for planes that are
> actually
> - Â * enabled, so make sure all other planes are set as
> disabled.
> + Â * Note: If we start supporting multiple pending atomic
> commits against
> + Â * the same planes/CRTC's in the future, plane->state will
> no longer be
> + Â * the correct pre-state to use for the calculations here
> and we'll
> + Â * need to change where we get the 'unchanged' plane data
> from.
> + Â *
> + Â * For now this is fine because we only allow one queued
> commit against
> +  * a CRTC.  Even if the plane isn't modified by this
> transaction and we
> + Â * don't have a plane lock, we still have the CRTC's lock,
> so we know
> + Â * that no other transactions are racing with us to update
> it.
> Â Â */
> - memset(result, 0, sizeof(*result));
> + if (!intel_pstate)
> + intel_pstate = to_intel_plane_state(plane->state);
> Â
> - for_each_intel_plane_mask(_priv->drm,
> - Â Â intel_plane,
> - Â Â cstate->base.plane_mask) {
> - int i = skl_wm_plane_id(intel_plane);
> -
> - plane = _plane->base;
> - intel_pstate = NULL;
> - if (state)
> - intel_pstate =
> - intel_atomic_get_existing_plane_stat
> e(state,
> - Â Â Â Â
> Â Â intel_plane);
> + WARN_ON(!intel_pstate->base.fb);
> Â
> - /*
> - Â * Note: If we start supporting multiple pending
> atomic commits
> - Â * against the same planes/CRTC's in the future,
> plane->state
> - Â * will no longer be the correct