Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755088AbcJEVoM (ORCPT ); Wed, 5 Oct 2016 17:44:12 -0400 Received: from mga04.intel.com ([192.55.52.120]:61179 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754225AbcJEVoK (ORCPT ); Wed, 5 Oct 2016 17:44:10 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,451,1473145200"; d="scan'208";a="16598256" Message-ID: <1475703844.2381.74.camel@intel.com> Subject: Re: [Intel-gfx] [PATCH 5/6] drm/i915/gen9: Get rid of redundant watermark values From: Paulo Zanoni To: Lyude , intel-gfx@lists.freedesktop.org Cc: David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Daniel Vetter Date: Wed, 05 Oct 2016 18:44:04 -0300 In-Reply-To: <1475681598-12081-6-git-send-email-cpaul@redhat.com> References: <1475681598-12081-1-git-send-email-cpaul@redhat.com> <1475681598-12081-6-git-send-email-cpaul@redhat.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.20.5 (3.20.5-1.fc24) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 20149 Lines: 597 Em Qua, 2016-10-05 às 11:33 -0400, Lyude escreveu: > Now that we've make skl_wm_levels make a little more sense, we can > remove all of the redundant wm information. Up until now we'd been > storing two copies of all of the skl watermarks: one being the > skl_pipe_wm structs, the other being the global wm struct in > drm_i915_private containing the raw register values. This is > confusing > and problematic, since it means we're prone to accidentally letting > the > two copies go out of sync. So, get rid of all of the functions > responsible for computing the register values and just use a single > helper, skl_write_wm_level(), to convert and write the new watermarks > on > the fly. I like the direction of this patch too, but there are some small possible problems. See below. > > Signed-off-by: Lyude > Cc: Maarten Lankhorst > Cc: Ville Syrjälä > Cc: Matt Roper > --- >  drivers/gpu/drm/i915/i915_drv.h      |   2 - >  drivers/gpu/drm/i915/intel_display.c |  14 ++- >  drivers/gpu/drm/i915/intel_drv.h     |   6 +- >  drivers/gpu/drm/i915/intel_pm.c      | 203 ++++++++++++------------- > ---------- >  drivers/gpu/drm/i915/intel_sprite.c  |   8 +- >  5 files changed, 88 insertions(+), 145 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index 0f97d43..63519ac 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1643,8 +1643,6 @@ struct skl_ddb_allocation { >  struct skl_wm_values { >   unsigned dirty_pipes; >   struct skl_ddb_allocation ddb; > - uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8]; > - uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES]; >  }; >   >  struct skl_wm_level { > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index dd15ae2..c580d3d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3378,6 +3378,8 @@ static void skylake_update_primary_plane(struct > drm_plane *plane, >   struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state- > >base.crtc); >   struct drm_framebuffer *fb = plane_state->base.fb; >   const struct skl_wm_values *wm = &dev_priv->wm.skl_results; > + const struct skl_plane_wm *p_wm = > + &crtc_state->wm.skl.optimal.planes[0]; I wish someone would do a patch to convert all these hardcoded values to PLANE_X, and convert "int"s  to "enum plane"s everywhere. >   int pipe = intel_crtc->pipe; >   u32 plane_ctl; >   unsigned int rotation = plane_state->base.rotation; > @@ -3414,7 +3416,7 @@ static void skylake_update_primary_plane(struct > drm_plane *plane, >   intel_crtc->adjusted_y = src_y; >   >   if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) > - skl_write_plane_wm(intel_crtc, wm, 0); > + skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0); >   >   I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); >   I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x); > @@ -3448,6 +3450,8 @@ static void > skylake_disable_primary_plane(struct drm_plane *primary, >   struct drm_device *dev = crtc->dev; >   struct drm_i915_private *dev_priv = to_i915(dev); >   struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc- > >state); > + const struct skl_plane_wm *p_wm = &cstate- > >wm.skl.optimal.planes[0]; >   int pipe = intel_crtc->pipe; >   >   /* > @@ -3455,7 +3459,8 @@ static void > skylake_disable_primary_plane(struct drm_plane *primary, >    * plane's visiblity isn't actually changing neither is its > watermarks. >    */ >   if (!crtc->primary->state->visible) > - skl_write_plane_wm(intel_crtc, &dev_priv- > >wm.skl_results, 0); > + skl_write_plane_wm(intel_crtc, p_wm, > +    &dev_priv->wm.skl_results.ddb, > 0); >   >   I915_WRITE(PLANE_CTL(pipe, 0), 0); >   I915_WRITE(PLANE_SURF(pipe, 0), 0); > @@ -10819,12 +10824,15 @@ static void i9xx_update_cursor(struct > drm_crtc *crtc, u32 base, >   struct drm_device *dev = crtc->dev; >   struct drm_i915_private *dev_priv = to_i915(dev); >   struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc- > >state); >   const struct skl_wm_values *wm = &dev_priv->wm.skl_results; > + const struct skl_plane_wm *p_wm = > + &cstate->wm.skl.optimal.planes[PLANE_CURSOR]; >   int pipe = intel_crtc->pipe; >   uint32_t cntl = 0; >   >   if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & > drm_crtc_mask(crtc)) > - skl_write_cursor_wm(intel_crtc, wm); > + skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb); >   >   if (plane_state && plane_state->base.visible) { >   cntl = MCURSOR_GAMMA_ENABLE; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index d684f4f..958dc72 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1765,9 +1765,11 @@ bool skl_ddb_allocation_equals(const struct > skl_ddb_allocation *old, >  bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state, >    struct intel_crtc *intel_crtc); >  void skl_write_cursor_wm(struct intel_crtc *intel_crtc, > -  const struct skl_wm_values *wm); > +  const struct skl_plane_wm *wm, > +  const struct skl_ddb_allocation *ddb); >  void skl_write_plane_wm(struct intel_crtc *intel_crtc, > - const struct skl_wm_values *wm, > + const struct skl_plane_wm *wm, > + const struct skl_ddb_allocation *ddb, >   int plane); >  uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state > *pipe_config); >  bool ilk_disable_lp_wm(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 250f12d..7708646 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3000,6 +3000,8 @@ bool intel_can_enable_sagv(struct > drm_atomic_state *state) >   struct drm_i915_private *dev_priv = to_i915(dev); >   struct intel_atomic_state *intel_state = > to_intel_atomic_state(state); >   struct drm_crtc *crtc; > + struct intel_crtc_state *cstate; > + struct skl_plane_wm *wm; >   enum pipe pipe; >   int level, plane; >   > @@ -3020,18 +3022,21 @@ bool intel_can_enable_sagv(struct > drm_atomic_state *state) >   /* Since we're now guaranteed to only have one active > CRTC... */ >   pipe = ffs(intel_state->active_crtcs) - 1; >   crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > + cstate = intel_atomic_get_crtc_state(state, > to_intel_crtc(crtc)); >   >   if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE) >   return false; >   >   for_each_plane(dev_priv, pipe, plane) { > + wm = &cstate->wm.skl.optimal.planes[plane]; > + >   /* Skip this plane if it's not enabled */ > - if (intel_state->wm_results.plane[pipe][plane][0] == > 0) > + if (!wm->wm[0].plane_en) >   continue; >   >   /* Find the highest enabled wm level for this plane > */ > - for (level = ilk_wm_max_level(dev); > -      intel_state- > >wm_results.plane[pipe][plane][level] == 0; --level) > + for (level = ilk_wm_max_level(dev); !wm- > >wm[level].plane_en; > +      --level) >        { } >   >   /* > @@ -3777,67 +3782,6 @@ static int skl_build_pipe_wm(struct > intel_crtc_state *cstate, >   return 0; >  } >   > -static void skl_compute_wm_results(struct drm_device *dev, > -    struct skl_pipe_wm *p_wm, > -    struct skl_wm_values *r, > -    struct intel_crtc *intel_crtc) > -{ > - int level, max_level = ilk_wm_max_level(dev); > - struct skl_plane_wm *plane_wm; > - enum pipe pipe = intel_crtc->pipe; > - uint32_t temp; > - int i; > - > - for (i = 0; i < intel_num_planes(intel_crtc); i++) { > - plane_wm = &p_wm->planes[i]; > - > - for (level = 0; level <= max_level; level++) { > - temp = 0; > - > - temp |= plane_wm->wm[level].plane_res_l << > - PLANE_WM_LINES_SHIFT; > - temp |= plane_wm->wm[level].plane_res_b; > - if (plane_wm->wm[level].plane_en) > - temp |= PLANE_WM_EN; > - > - r->plane[pipe][i][level] = temp; > - } > - > - } > - > - for (level = 0; level <= max_level; level++) { > - plane_wm = &p_wm->planes[PLANE_CURSOR]; > - temp = 0; > - temp |= plane_wm->wm[level].plane_res_l << > PLANE_WM_LINES_SHIFT; > - temp |= plane_wm->wm[level].plane_res_b; > - if (plane_wm->wm[level].plane_en) > - temp |= PLANE_WM_EN; > - > - r->plane[pipe][PLANE_CURSOR][level] = temp; > - } > - > - /* transition WMs */ > - for (i = 0; i < intel_num_planes(intel_crtc); i++) { > - plane_wm = &p_wm->planes[i]; > - temp = 0; > - temp |= plane_wm->trans_wm.plane_res_l << > PLANE_WM_LINES_SHIFT; > - temp |= plane_wm->trans_wm.plane_res_b; > - if (plane_wm->trans_wm.plane_en) > - temp |= PLANE_WM_EN; > - > - r->plane_trans[pipe][i] = temp; > - } > - > - plane_wm = &p_wm->planes[PLANE_CURSOR]; > - temp = 0; > - temp |= plane_wm->trans_wm.plane_res_l << > PLANE_WM_LINES_SHIFT; > - temp |= plane_wm->trans_wm.plane_res_b; > - if (plane_wm->trans_wm.plane_en) > - temp |= PLANE_WM_EN; > - > - r->plane_trans[pipe][PLANE_CURSOR] = temp; > -} > - >  static void skl_ddb_entry_write(struct drm_i915_private *dev_priv, >   i915_reg_t reg, >   const struct skl_ddb_entry *entry) > @@ -3848,8 +3792,22 @@ static void skl_ddb_entry_write(struct > drm_i915_private *dev_priv, >   I915_WRITE(reg, 0); >  } >   > +static void skl_write_wm_level(struct drm_i915_private *dev_priv, > +        i915_reg_t reg, > +        const struct skl_wm_level *level) > +{ > + uint32_t val = 0; > + > + val |= level->plane_res_b; > + val |= level->plane_res_l << PLANE_WM_LINES_SHIFT; > + val |= level->plane_en; The line above seems wrong, you should check for plane_en and then set PLANE_WM_EN. IMHO it would even better if we completely zeroed the register in case plane_en is false, so we could have: uint32_t val = 0; if (level->plane_en) { val |= PLANE_WM_EN; val |= level->plane_res_b; val |= level->plane_res_l << PLANE_WM_LINES_SHIFT; } > + > + I915_WRITE(reg, val); > +} > + >  void skl_write_plane_wm(struct intel_crtc *intel_crtc, > - const struct skl_wm_values *wm, > + const struct skl_plane_wm *wm, > + const struct skl_ddb_allocation *ddb, >   int plane) >  { >   struct drm_crtc *crtc = &intel_crtc->base; > @@ -3859,19 +3817,21 @@ void skl_write_plane_wm(struct intel_crtc > *intel_crtc, >   enum pipe pipe = intel_crtc->pipe; >   >   for (level = 0; level <= max_level; level++) { > - I915_WRITE(PLANE_WM(pipe, plane, level), > -    wm->plane[pipe][plane][level]); > + skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane, > level), > +    &wm->wm[level]); >   } > - I915_WRITE(PLANE_WM_TRANS(pipe, plane), wm- > >plane_trans[pipe][plane]); > + skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane), > +    &wm->trans_wm); >   >   skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane), > -     &wm->ddb.plane[pipe][plane]); > +     &ddb->plane[pipe][plane]); >   skl_ddb_entry_write(dev_priv, PLANE_NV12_BUF_CFG(pipe, > plane), > -     &wm->ddb.y_plane[pipe][plane]); > +     &ddb->y_plane[pipe][plane]); >  } >   >  void skl_write_cursor_wm(struct intel_crtc *intel_crtc, > -  const struct skl_wm_values *wm) > +  const struct skl_plane_wm *wm, > +  const struct skl_ddb_allocation *ddb) >  { >   struct drm_crtc *crtc = &intel_crtc->base; >   struct drm_device *dev = crtc->dev; > @@ -3880,13 +3840,13 @@ void skl_write_cursor_wm(struct intel_crtc > *intel_crtc, >   enum pipe pipe = intel_crtc->pipe; >   >   for (level = 0; level <= max_level; level++) { > - I915_WRITE(CUR_WM(pipe, level), > -    wm->plane[pipe][PLANE_CURSOR][level]); > + skl_write_wm_level(dev_priv, CUR_WM(pipe, level), > +    &wm->wm[level]); >   } > - I915_WRITE(CUR_WM_TRANS(pipe), wm- > >plane_trans[pipe][PLANE_CURSOR]); > + skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm- > >trans_wm); >   >   skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), > -     &wm->ddb.plane[pipe][PLANE_CURSOR]); > +     &ddb->plane[pipe][PLANE_CURSOR]); >  } >   >  static inline bool skl_ddb_entries_overlap(const struct > skl_ddb_entry *a, > @@ -4064,11 +4024,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values > *dst, >        struct skl_wm_values *src, >        enum pipe pipe) >  { > - memcpy(dst->plane[pipe], src->plane[pipe], > -        sizeof(dst->plane[pipe])); > - memcpy(dst->plane_trans[pipe], src->plane_trans[pipe], > -        sizeof(dst->plane_trans[pipe])); > - >   memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe], >          sizeof(dst->ddb.y_plane[pipe])); >   memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe], > @@ -4117,7 +4072,6 @@ skl_compute_wm(struct drm_atomic_state *state) >    * no suitable watermark values can be found. >    */ >   for_each_crtc_in_state(state, crtc, cstate, i) { > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >   struct intel_crtc_state *intel_cstate = >   to_intel_crtc_state(cstate); >   > @@ -4135,7 +4089,6 @@ skl_compute_wm(struct drm_atomic_state *state) >   continue; >   >   intel_cstate->update_wm_pre = true; > - skl_compute_wm_results(crtc->dev, pipe_wm, results, > intel_crtc); >   } >   >   return 0; > @@ -4169,9 +4122,11 @@ static void skl_update_wm(struct drm_crtc > *crtc) >   int plane; >   >   for (plane = 0; plane < > intel_num_planes(intel_crtc); plane++) > - skl_write_plane_wm(intel_crtc, results, > plane); > + skl_write_plane_wm(intel_crtc, &pipe_wm- > >planes[plane], > +    &results->ddb, plane); >   > - skl_write_cursor_wm(intel_crtc, results); > + skl_write_cursor_wm(intel_crtc, &pipe_wm- > >planes[PLANE_CURSOR], > +     &results->ddb); >   } >   >   skl_copy_wm_for_pipe(hw_vals, results, pipe); > @@ -4256,28 +4211,13 @@ static void ilk_optimize_watermarks(struct > intel_crtc_state *cstate) >   mutex_unlock(&dev_priv->wm.wm_mutex); >  } >   > -static void skl_pipe_wm_active_state(uint32_t val, > -      struct skl_pipe_wm *active, > -      bool is_transwm, > -      int i, > -      int level) > +static inline void skl_wm_level_from_reg_val(uint32_t val, > +      struct skl_wm_level > *level) >  { > - struct skl_plane_wm *plane_wm = &active->planes[i]; > - bool is_enabled = (val & PLANE_WM_EN) != 0; > - > - if (!is_transwm) { > - plane_wm->wm[level].plane_en = is_enabled; > - plane_wm->wm[level].plane_res_b = val & > PLANE_WM_BLOCKS_MASK; > - plane_wm->wm[level].plane_res_l = > - (val >> PLANE_WM_LINES_SHIFT) & > - PLANE_WM_LINES_MASK; > - } else { > - plane_wm->trans_wm.plane_en = is_enabled; > - plane_wm->trans_wm.plane_res_b = val & > PLANE_WM_BLOCKS_MASK; > - plane_wm->trans_wm.plane_res_l = > - (val >> PLANE_WM_LINES_SHIFT) & > - PLANE_WM_LINES_MASK; > - } > + level->plane_en = val & PLANE_WM_EN; > + level->plane_res_b = val & PLANE_WM_BLOCKS_MASK; > + level->plane_res_l = (val & PLANE_WM_LINES_MASK) >> > + PLANE_WM_LINES_SHIFT; This also looks wrong, since PLANE_WM_LINES_MASK is 0x1f. We should do like the original code did: shifting before masking. >  } >   >  static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc) > @@ -4287,23 +4227,33 @@ static void skl_pipe_wm_get_hw_state(struct > drm_crtc *crtc) >   struct skl_wm_values *hw = &dev_priv->wm.skl_hw; >   struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >   struct intel_crtc_state *cstate = to_intel_crtc_state(crtc- > >state); > + struct intel_plane *intel_plane; >   struct skl_pipe_wm *active = &cstate->wm.skl.optimal; > + struct skl_plane_wm *wm; >   enum pipe pipe = intel_crtc->pipe; > - int level, i, max_level; > - uint32_t temp; > + int level, id, max_level = ilk_wm_max_level(dev); > + uint32_t val; >   > - max_level = ilk_wm_max_level(dev); > + for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { > + id = skl_wm_plane_id(intel_plane); > + wm = &cstate->wm.skl.optimal.planes[id]; >   > - for (level = 0; level <= max_level; level++) { > - for (i = 0; i < intel_num_planes(intel_crtc); i++) > - hw->plane[pipe][i][level] = > - I915_READ(PLANE_WM(pipe, i, > level)); > - hw->plane[pipe][PLANE_CURSOR][level] = > I915_READ(CUR_WM(pipe, level)); > - } > + for (level = 0; level <= max_level; level++) { > + if (id != PLANE_CURSOR) > + val = I915_READ(PLANE_WM(pipe, id, > level)); > + else > + val = I915_READ(CUR_WM(pipe, > level)); > + > + skl_wm_level_from_reg_val(val, &wm- > >wm[level]); > + } >   > - for (i = 0; i < intel_num_planes(intel_crtc); i++) > - hw->plane_trans[pipe][i] = > I915_READ(PLANE_WM_TRANS(pipe, i)); > - hw->plane_trans[pipe][PLANE_CURSOR] = > I915_READ(CUR_WM_TRANS(pipe)); > + if (id != PLANE_CURSOR) > + val = I915_READ(PLANE_WM_TRANS(pipe, id)); > + else > + val = I915_READ(CUR_WM_TRANS(pipe)); > + > + skl_wm_level_from_reg_val(val, &wm->trans_wm); > + } >   >   if (!intel_crtc->active) >   return; > @@ -4311,25 +4261,6 @@ static void skl_pipe_wm_get_hw_state(struct > drm_crtc *crtc) >   hw->dirty_pipes |= drm_crtc_mask(crtc); >   >   active->linetime = I915_READ(PIPE_WM_LINETIME(pipe)); > - > - for (level = 0; level <= max_level; level++) { > - for (i = 0; i < intel_num_planes(intel_crtc); i++) { > - temp = hw->plane[pipe][i][level]; > - skl_pipe_wm_active_state(temp, active, > false, i, level); > - } > - temp = hw->plane[pipe][PLANE_CURSOR][level]; > - skl_pipe_wm_active_state(temp, active, false, i, > level); > - } > - > - for (i = 0; i < intel_num_planes(intel_crtc); i++) { > - temp = hw->plane_trans[pipe][i]; > - skl_pipe_wm_active_state(temp, active, true, i, 0); > - } > - > - temp = hw->plane_trans[pipe][PLANE_CURSOR]; > - skl_pipe_wm_active_state(temp, active, true, i, 0); > - > - intel_crtc->wm.active.skl = *active; As far as I understood, we should still be setting intel_crtc- >wm.active.skl. Shouldn't we? If not, why? Now, due to the problems above, weren't you getting some WARNs about the hw state not matching what it should? In case not, maybe you could investigate why. >  } >   >  void skl_wm_get_hw_state(struct drm_device *dev) > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index 73a521f..0fb775b 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -208,6 +208,8 @@ skl_update_plane(struct drm_plane *drm_plane, >   struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >   const int pipe = intel_plane->pipe; >   const int plane = intel_plane->plane + 1; > + const struct skl_plane_wm *p_wm = > + &crtc_state->wm.skl.optimal.planes[plane]; >   u32 plane_ctl; >   const struct drm_intel_sprite_colorkey *key = &plane_state- > >ckey; >   u32 surf_addr = plane_state->main.offset; > @@ -232,7 +234,7 @@ skl_update_plane(struct drm_plane *drm_plane, >   plane_ctl |= skl_plane_ctl_rotation(rotation); >   >   if (wm->dirty_pipes & drm_crtc_mask(crtc)) > - skl_write_plane_wm(intel_crtc, wm, plane); > + skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, > plane); >   >   if (key->flags) { >   I915_WRITE(PLANE_KEYVAL(pipe, plane), key- > >min_value); > @@ -289,6 +291,7 @@ skl_disable_plane(struct drm_plane *dplane, > struct drm_crtc *crtc) >   struct drm_device *dev = dplane->dev; >   struct drm_i915_private *dev_priv = to_i915(dev); >   struct intel_plane *intel_plane = to_intel_plane(dplane); > + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc- > >state); >   const int pipe = intel_plane->pipe; >   const int plane = intel_plane->plane + 1; >   > @@ -298,7 +301,8 @@ skl_disable_plane(struct drm_plane *dplane, > struct drm_crtc *crtc) >    */ >   if (!dplane->state->visible) >   skl_write_plane_wm(to_intel_crtc(crtc), > -    &dev_priv->wm.skl_results, > plane); > +    &cstate- > >wm.skl.optimal.planes[plane], > +    &dev_priv->wm.skl_results.ddb, > plane); >   >   I915_WRITE(PLANE_CTL(pipe, plane), 0); >