On Wed, Oct 23, 2019 at 12:08:03PM +0300, Stanislav Lisovskiy wrote: > Currently intel_can_enable_sagv function contains > a mix of workarounds for different platforms > some of them are not valid for gens >= 11 already, > so lets split it into separate functions. > > v2: > - Rework watermark calculation algorithm to > attempt to calculate Level 0 watermark > with added sagv block time latency and > check if it fits in DBuf in order to > determine if SAGV can be enabled already > at this stage, just as BSpec 49325 states. > if that fails rollback to usual Level 0 > latency and disable SAGV. > - Remove unneeded tabs(James Ausmus) > > v3: Rebased the patch > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovs...@intel.com> > Cc: Ville Syrjälä <ville.syrj...@intel.com> > Cc: James Ausmus <james.aus...@intel.com> > --- > .../drm/i915/display/intel_display_types.h | 8 + > drivers/gpu/drm/i915/intel_pm.c | 228 +++++++++++++++++- > 2 files changed, 228 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 8358152e403e..f09c80c96470 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -490,6 +490,13 @@ struct intel_atomic_state { > */ > u8 active_pipe_changes; > > + /* > + * For Gen12 only after calculating watermarks with > + * additional latency, we can determine if SAGV can be enabled > + * or not for that particular configuration. > + */ > + bool gen12_can_sagv; > + > u8 active_pipes; > /* minimum acceptable cdclk for each pipe */ > int min_cdclk[I915_MAX_PIPES]; > @@ -642,6 +649,7 @@ struct skl_plane_wm { > struct skl_wm_level wm[8]; > struct skl_wm_level uv_wm[8]; > struct skl_wm_level trans_wm; > + struct skl_wm_level sagv_wm_l0; > bool is_planar; > }; > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 362234449087..c0419e4d83de 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3751,7 +3751,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv) > return 0; > } > > -bool intel_can_enable_sagv(struct intel_atomic_state *state) > +bool skl_can_enable_sagv(struct intel_atomic_state *state) > { > struct drm_device *dev = state->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > @@ -3817,6 +3817,75 @@ bool intel_can_enable_sagv(struct intel_atomic_state > *state) > return true; > } > > +bool icl_can_enable_sagv(struct intel_atomic_state *state) > +{ > + struct drm_device *dev = state->base.dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + struct intel_crtc *crtc; > + struct intel_crtc_state *new_crtc_state; > + int level, latency; > + int i; > + int plane_id; > + > + if (!intel_has_sagv(dev_priv)) > + return false; > + > + /* > + * If there are no active CRTCs, no additional checks need be performed > + */ > + if (hweight8(state->active_pipes) == 0) > + return true; > + > + for_each_new_intel_crtc_in_state(state, crtc, > + new_crtc_state, i) { > + > + if (crtc->base.state->adjusted_mode.flags & > DRM_MODE_FLAG_INTERLACE) > + return false; > + > + if (!new_crtc_state->base.enable) > + continue; > + > + for_each_plane_id_on_crtc(crtc, plane_id) { > + struct skl_plane_wm *wm = > + > &new_crtc_state->wm.skl.optimal.planes[plane_id]; > + > + /* Skip this plane if it's not enabled */ > + if (!wm->wm[0].plane_en) > + continue; > + > + /* Find the highest enabled wm level for this plane */ > + for (level = ilk_wm_max_level(dev_priv); > + !wm->wm[level].plane_en; --level) > + { } > + > + latency = dev_priv->wm.skl_latency[level]; > + > + /* > + * If any of the planes on this pipe don't enable wm > levels that > + * incur memory latencies higher than > sagv_block_time_us we > + * can't enable SAGV. > + */ > + if (latency < dev_priv->sagv_block_time_us) > + return false; > + } > + } > + > + return true; > +} > + > +bool intel_can_enable_sagv(struct intel_atomic_state *state) > +{ > + struct drm_device *dev = state->base.dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + > + if (INTEL_GEN(dev_priv) >= 12) > + return state->gen12_can_sagv;
This loses the interlaced mode check that the skl and icl functions have, which is still needed for TGL > + else if (INTEL_GEN(dev_priv) == 11) > + return icl_can_enable_sagv(state); > + > + return skl_can_enable_sagv(state); > +} > + > static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv, > const struct intel_crtc_state *crtc_state, > const u64 total_data_rate, > @@ -3936,6 +4005,7 @@ static int skl_compute_wm_params(const struct > intel_crtc_state *crtc_state, > int color_plane); > static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state, > int level, > + u32 latency, > const struct skl_wm_params *wp, > const struct skl_wm_level *result_prev, > struct skl_wm_level *result /* out */); > @@ -3958,7 +4028,8 @@ skl_cursor_allocation(const struct intel_crtc_state > *crtc_state, > WARN_ON(ret); > > for (level = 0; level <= max_level; level++) { > - skl_compute_plane_wm(crtc_state, level, &wp, &wm, &wm); > + u32 latency = dev_priv->wm.skl_latency[level]; > + skl_compute_plane_wm(crtc_state, level, latency, &wp, &wm, &wm); > if (wm.min_ddb_alloc == U16_MAX) > break; > > @@ -4310,6 +4381,73 @@ icl_get_total_relative_data_rate(struct > intel_crtc_state *crtc_state, > return total_data_rate; > } > > +static int > +tgl_check_pipe_fits_sagv_wm(struct intel_crtc_state *crtc_state, > + struct skl_ddb_allocation *ddb /* out */) > +{ > + struct drm_crtc *crtc = crtc_state->base.crtc; > + struct drm_i915_private *dev_priv = to_i915(crtc->dev); > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct skl_ddb_entry *alloc = &crtc_state->wm.skl.ddb; > + u16 alloc_size; > + u16 total[I915_MAX_PLANES] = {}; > + u64 total_data_rate; > + enum plane_id plane_id; > + int num_active; > + u64 plane_data_rate[I915_MAX_PLANES] = {}; > + u64 uv_plane_data_rate[I915_MAX_PLANES] = {}; > + u32 blocks; > + > + if (INTEL_GEN(dev_priv) >= 11) > + total_data_rate = > + icl_get_total_relative_data_rate(crtc_state, > + plane_data_rate); This function is already only called on gen12+ - could drop the if check, and the entire else block. > + else > + total_data_rate = > + skl_get_total_relative_data_rate(crtc_state, > + plane_data_rate, > + uv_plane_data_rate); > + > + > + skl_ddb_get_pipe_allocation_limits(dev_priv, crtc_state, > total_data_rate, > + ddb, alloc, &num_active); > + alloc_size = skl_ddb_entry_size(alloc); > + if (alloc_size == 0) > + return -ENOSPC; > + > + /* Allocate fixed number of blocks for cursor. */ > + total[PLANE_CURSOR] = skl_cursor_allocation(crtc_state, num_active); > + alloc_size -= total[PLANE_CURSOR]; > + crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].start = > + alloc->end - total[PLANE_CURSOR]; > + crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end; > + > + /* > + * Do check if we can fit L0 + sagv_block_time and > + * disable SAGV if we can't. > + */ > + blocks = 0; > + for_each_plane_id_on_crtc(intel_crtc, plane_id) { > + const struct skl_plane_wm *wm = > + &crtc_state->wm.skl.optimal.planes[plane_id]; > + > + if (plane_id == PLANE_CURSOR) { > + if (WARN_ON(wm->sagv_wm_l0.min_ddb_alloc > > + total[PLANE_CURSOR])) { > + blocks = U32_MAX; > + break; > + } > + continue; > + } > + > + blocks += wm->sagv_wm_l0.min_ddb_alloc; > + if (blocks > alloc_size) { > + return -ENOSPC; > + } > + } > + return 0; > +} > + > static int > skl_allocate_pipe_ddb(struct intel_crtc_state *crtc_state, > struct skl_ddb_allocation *ddb /* out */) > @@ -4739,12 +4877,12 @@ static bool skl_wm_has_lines(struct drm_i915_private > *dev_priv, int level) > > static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state, > int level, > + u32 latency, > const struct skl_wm_params *wp, > const struct skl_wm_level *result_prev, > struct skl_wm_level *result /* out */) > { > struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); > - u32 latency = dev_priv->wm.skl_latency[level]; > uint_fixed_16_16_t method1, method2; > uint_fixed_16_16_t selected_result; > u32 res_blocks, res_lines, min_ddb_alloc = 0; > @@ -4865,19 +5003,45 @@ static void skl_compute_plane_wm(const struct > intel_crtc_state *crtc_state, > static void > skl_compute_wm_levels(const struct intel_crtc_state *crtc_state, > const struct skl_wm_params *wm_params, > - struct skl_wm_level *levels) > + struct skl_plane_wm *plane_wm, > + bool yuv) > { > struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); > int level, max_level = ilk_wm_max_level(dev_priv); > + /* > + * Check which kind of plane is it and based on that calculate > + * correspondent WM levels. > + */ > + struct skl_wm_level *levels = yuv ? plane_wm->uv_wm : plane_wm->wm; > struct skl_wm_level *result_prev = &levels[0]; > > for (level = 0; level <= max_level; level++) { > struct skl_wm_level *result = &levels[level]; > + u32 latency = dev_priv->wm.skl_latency[level]; > > - skl_compute_plane_wm(crtc_state, level, wm_params, > - result_prev, result); > + skl_compute_plane_wm(crtc_state, level, latency, > + wm_params, result_prev, result); > > result_prev = result; > + if (level == 0) { > + /* > + * For Gen12 if it is an L0 we need to also > + * consider sagv_block_time when calculating > + * L0 watermark - we will need that when making > + * a decision whether enable SAGV or not. > + * For older gens we agreed to copy L0 value for > + * compatibility. > + */ > + if ((INTEL_GEN(dev_priv) >= 12)) { > + latency += dev_priv->sagv_block_time_us; > + skl_compute_plane_wm(crtc_state, level, latency, > + wm_params, result_prev, > + &plane_wm->sagv_wm_l0); > + } > + else > + memcpy(&plane_wm->sagv_wm_l0, &levels[0], > + sizeof(struct skl_wm_level)); > + } > } > } > > @@ -4971,7 +5135,7 @@ static int skl_build_plane_wm_single(struct > intel_crtc_state *crtc_state, > if (ret) > return ret; > > - skl_compute_wm_levels(crtc_state, &wm_params, wm->wm); > + skl_compute_wm_levels(crtc_state, &wm_params, wm, false); > skl_compute_transition_wm(crtc_state, &wm_params, wm); > > return 0; > @@ -4993,7 +5157,7 @@ static int skl_build_plane_wm_uv(struct > intel_crtc_state *crtc_state, > if (ret) > return ret; > > - skl_compute_wm_levels(crtc_state, &wm_params, wm->uv_wm); > + skl_compute_wm_levels(crtc_state, &wm_params, wm, true); > > return 0; > } > @@ -5544,10 +5708,13 @@ static int skl_wm_add_affected_planes(struct > intel_atomic_state *state, > static int > skl_compute_wm(struct intel_atomic_state *state) > { > + struct drm_device *dev = state->base.dev; > + const struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *crtc; > struct intel_crtc_state *new_crtc_state; > struct intel_crtc_state *old_crtc_state; > struct skl_ddb_values *results = &state->wm_results; > + struct skl_ddb_allocation *ddb = &state->wm_results.ddb; > int ret, i; > > /* Clear all dirty flags */ > @@ -5557,6 +5724,8 @@ skl_compute_wm(struct intel_atomic_state *state) > if (ret) > return ret; > > + state->gen12_can_sagv = false; > + > /* > * Calculate WM's for all pipes that are part of this transaction. > * Note that skl_ddb_add_affected_pipes may have added more CRTC's that > @@ -5579,6 +5748,49 @@ skl_compute_wm(struct intel_atomic_state *state) > results->dirty_pipes |= BIT(crtc->pipe); > } > > + if (INTEL_GEN(dev_priv) < 12) > + goto compute_ddb; I understand why you are goto'ing to avoid the extra indent below - can the block between here and compute_ddb just be extracted to a separate function instead? -James > + > + /* > + * Lets assume we can tolerate SAGV for now, > + * until watermark calculations prove the opposite > + * if any of the pipe planes in the state will > + * fail the requirements it will be assigned to false > + * in skl_compute_ddb. > + */ > + state->gen12_can_sagv = true; > + > + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > + new_crtc_state, i) { > + ret = tgl_check_pipe_fits_sagv_wm(new_crtc_state, ddb); > + if (ret) { > + state->gen12_can_sagv = false; > + break; > + } > + } > + > + if (state->gen12_can_sagv) { > + /* > + * If we determined that we can actually enable SAGV, then > + * actually use those levels tgl_check_pipe_fits_sagv_wm > + * has already taken care of checking if L0 + sagv block time > + * fits into ddb. > + */ > + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > + new_crtc_state, i) { > + struct intel_plane *plane; > + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, > plane) { > + enum plane_id plane_id = plane->id; > + struct skl_plane_wm *plane_wm = \ > + > &new_crtc_state->wm.skl.optimal.planes[plane_id]; > + struct skl_wm_level *sagv_wm0 = > &plane_wm->sagv_wm_l0; > + struct skl_wm_level *l0_wm0 = &plane_wm->wm[0]; > + memcpy(l0_wm0, sagv_wm0, sizeof(struct > skl_wm_level)); > + } > + } > + } > + > +compute_ddb: > ret = skl_compute_ddb(state); > if (ret) > return ret; > -- > 2.17.1 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx