| Message ID | 20190312205844.6339-7-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | skl+ cursor DDB allocation fixes | expand |
On Tue, Mar 12, 2019 at 10:58:41PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Currently we disable all the watermarks above the selected max > level for every plane. That would mean that the cursor's watermarks > may also get modified when another plane causes the selected > max watermark level to change. That is not so great as we would > like to keep the cursor as indepenedent as possible to avoid > having to throttle it in resposne to other plane activity. > > To avoid that let's keep the watermarks enabled even for levels > above the max selected watermark level, iff the plane has enough > ddb for that particular level. This way the cursor's enabled > watermarks only depend on the cursor itself. This is safe because > the hardware will never choose to use a watermark level unless > all enabled planes have also enabled that level. > > Cc: Neel Desai <neel.desai@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Iirc, we also had different levels enabled for different planes with the old algorithm that calculated DDB first and watermarks second. So agreed; this should be very safe. Patches 1-6 are Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Matt > --- > drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index c866663b31bc..8afbc56ad89a 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4500,7 +4500,22 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > for (level++; level <= ilk_wm_max_level(dev_priv); level++) { > for_each_plane_id_on_crtc(intel_crtc, plane_id) { > wm = &cstate->wm.skl.optimal.planes[plane_id]; > - memset(&wm->wm[level], 0, sizeof(wm->wm[level])); > + > + /* > + * We only disable the watermarks for each plane if > + * they exceed the ddb allocation of said plane. This > + * is done so that we don't end up touching cursor > + * watermarks needlessly when some other plane reduces > + * our max possible watermark level. > + * > + * Bspec has this to say about the PLANE_WM enable bit: > + * "All the watermarks at this level for all enabled > + * planes must be enabled before the level will be used." > + * So this is actually safe to do. > + */ > + if (wm->wm[level].min_ddb_alloc > total[plane_id] || > + wm->uv_wm[level].min_ddb_alloc > uv_total[plane_id]) > + memset(&wm->wm[level], 0, sizeof(wm->wm[level])); > > /* > * Wa_1408961008:icl > -- > 2.19.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c866663b31bc..8afbc56ad89a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4500,7 +4500,22 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, for (level++; level <= ilk_wm_max_level(dev_priv); level++) { for_each_plane_id_on_crtc(intel_crtc, plane_id) { wm = &cstate->wm.skl.optimal.planes[plane_id]; - memset(&wm->wm[level], 0, sizeof(wm->wm[level])); + + /* + * We only disable the watermarks for each plane if + * they exceed the ddb allocation of said plane. This + * is done so that we don't end up touching cursor + * watermarks needlessly when some other plane reduces + * our max possible watermark level. + * + * Bspec has this to say about the PLANE_WM enable bit: + * "All the watermarks at this level for all enabled + * planes must be enabled before the level will be used." + * So this is actually safe to do. + */ + if (wm->wm[level].min_ddb_alloc > total[plane_id] || + wm->uv_wm[level].min_ddb_alloc > uv_total[plane_id]) + memset(&wm->wm[level], 0, sizeof(wm->wm[level])); /* * Wa_1408961008:icl