| Message ID | 20170718124950.26675-4-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 18-07-17 om 14:49 schreef Mahesh Kumar: > From: "Kumar, Mahesh" <mahesh1.kumar@intel.com> > > GEN > 9 require transition WM to be programmed if IPC is enabled. > This patch calculates & enable transition WM for supported platforms. > If transition WM is enabled, Plane read requests are sent at high > priority until filling above the transition watermark, then the > requests are sent at lower priority until dropping below the level-0 WM. > The lower priority requests allow other memory clients to have better > memory access. > > transition minimum is the minimum amount needed for trans_wm to work to > ensure the demote does not happen before enough data has been read to > meet the level 0 watermark requirements. > > transition amount is configurable value. Higher values will > tend to cause longer periods of high priority reads followed by longer > periods of lower priority reads. Tuning to lower values will tend to > cause shorter periods of high and lower priority reads. > > Keeping transition amount to 10 in this patch, as suggested by HW team. > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 49 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index b2bd65847d9b..9a2ed1b734d5 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4579,12 +4579,55 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate) > } > > static void skl_compute_transition_wm(struct intel_crtc_state *cstate, > + struct skl_wm_params *wp, > + struct skl_wm_level *wm_l0, > + uint16_t ddb_allocation, > struct skl_wm_level *trans_wm /* out */) > { > + struct drm_device *dev = cstate->base.crtc->dev; > + const struct drm_i915_private *dev_priv = to_i915(dev); > + uint16_t trans_min, trans_y_tile_min; > + uint16_t trans_amount = 10; /* This is configurable amount */ should probably be a const since it's a tweakable. > + uint16_t trans_offset_b, res_blocks; > + > if (!cstate->base.active) > + goto exit; > + > + /* Transition WM are not recommended by HW team for GEN9 */ > + if (INTEL_GEN(dev_priv) <= 9) > + goto exit; > + > + /* Transition WM don't make any sense if ipc is disabled */ > + if (!dev_priv->ipc_enabled) > + goto exit; > + > + if (INTEL_GEN(dev_priv) >= 10) > + trans_min = 4; > + > + trans_offset_b = trans_min + trans_amount; > + trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2, > + wp->y_tile_minimum); > + > + if (wp->y_tiled) { > + res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) + > + trans_offset_b; Perhaps only calculate trans_y_tile_min if y-tiled? > + } else { > + res_blocks = wm_l0->plane_res_b + trans_offset_b; > + } > + > + res_blocks += 1; > + > + /* WA BUG:1938466 add one block for non y-tile planes */ > + if (!wp->y_tiled && IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0)) > + res_blocks += 1; Could be added to the !y_tiled branch? Patch looks sane though, and seems to match bspec exactly. I couldn't find the value for trans_amount, but I'll take your word it's the right one. :) With the minor fixes: Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b2bd65847d9b..9a2ed1b734d5 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4579,12 +4579,55 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate) } static void skl_compute_transition_wm(struct intel_crtc_state *cstate, + struct skl_wm_params *wp, + struct skl_wm_level *wm_l0, + uint16_t ddb_allocation, struct skl_wm_level *trans_wm /* out */) { + struct drm_device *dev = cstate->base.crtc->dev; + const struct drm_i915_private *dev_priv = to_i915(dev); + uint16_t trans_min, trans_y_tile_min; + uint16_t trans_amount = 10; /* This is configurable amount */ + uint16_t trans_offset_b, res_blocks; + if (!cstate->base.active) + goto exit; + + /* Transition WM are not recommended by HW team for GEN9 */ + if (INTEL_GEN(dev_priv) <= 9) + goto exit; + + /* Transition WM don't make any sense if ipc is disabled */ + if (!dev_priv->ipc_enabled) + goto exit; + + if (INTEL_GEN(dev_priv) >= 10) + trans_min = 4; + + trans_offset_b = trans_min + trans_amount; + trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2, + wp->y_tile_minimum); + + if (wp->y_tiled) { + res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) + + trans_offset_b; + } else { + res_blocks = wm_l0->plane_res_b + trans_offset_b; + } + + res_blocks += 1; + + /* WA BUG:1938466 add one block for non y-tile planes */ + if (!wp->y_tiled && IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0)) + res_blocks += 1; + + if (res_blocks < ddb_allocation) { + trans_wm->plane_res_b = res_blocks; + trans_wm->plane_en = true; return; + } - /* Until we know more, just disable transition WMs */ +exit: trans_wm->plane_en = false; } @@ -4611,8 +4654,11 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate, to_intel_plane_state(pstate); enum plane_id plane_id = to_intel_plane(plane)->id; struct skl_wm_params wm_params; + enum pipe pipe = to_intel_crtc(cstate->base.crtc)->pipe; + uint16_t ddb_blocks; wm = &pipe_wm->planes[plane_id]; + ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][plane_id]); memset(&wm_params, 0, sizeof(struct skl_wm_params)); ret = skl_compute_plane_wm_params(dev_priv, cstate, @@ -4624,7 +4670,8 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate, intel_pstate, &wm_params, wm); if (ret) return ret; - skl_compute_transition_wm(cstate, &wm->trans_wm); + skl_compute_transition_wm(cstate, &wm_params, &wm->wm[0], + ddb_blocks, &wm->trans_wm); } pipe_wm->linetime = skl_compute_linetime_wm(cstate);