On Tue, Oct 16, 2018 at 03:01:23PM -0700, Paulo Zanoni wrote: > BSpec does not show these WAs as applicable to GLK, and for CNL it > only shows them applicable for a super early pre-production stepping > we shouldn't be caring about anymore. Remove these so we can avoid > them on ICL too. > > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_pm.c | 43 ++++++++++++++++++++++------------------- > 1 file changed, 23 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 67a4d0735291..18157c6ee126 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4696,28 +4696,31 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, > res_lines = div_round_up_fixed16(selected_result, > wp->plane_blocks_per_line); > > - /* Display WA #1125: skl,bxt,kbl,glk */ > - if (level == 0 && wp->rc_surface) > - res_blocks += fixed16_to_u32_round_up(wp->y_tile_minimum); > + if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) { IS_GEN9_BC || IS_BXT would be a little easier to parse, me thinks. > + /* Display WA #1125: skl,bxt,kbl */ > + if (level == 0 && wp->rc_surface) > + res_blocks += > + fixed16_to_u32_round_up(wp->y_tile_minimum); > + > + /* Display WA #1126: skl,bxt,kbl */ > + if (level >= 1 && level <= 7) { > + if (wp->y_tiled) { > + res_blocks += > + fixed16_to_u32_round_up(wp->y_tile_minimum); > + res_lines += wp->y_min_scanlines; > + } else { > + res_blocks++; > + } > > - /* Display WA #1126: skl,bxt,kbl,glk */ > - if (level >= 1 && level <= 7) { > - if (wp->y_tiled) { > - res_blocks += fixed16_to_u32_round_up( > - wp->y_tile_minimum); > - res_lines += wp->y_min_scanlines; > - } else { > - res_blocks++; > + /* > + * Make sure result blocks for higher latency levels are > + * atleast as high as level below the current level. > + * Assumption in DDB algorithm optimization for special > + * cases. Also covers Display WA #1125 for RC. > + */ > + if (result_prev->plane_res_b > res_blocks) > + res_blocks = result_prev->plane_res_b; > } > - > - /* > - * Make sure result blocks for higher latency levels are atleast > - * as high as level below the current level. > - * Assumption in DDB algorithm optimization for special cases. > - * Also covers Display WA #1125 for RC. > - */ > - if (result_prev->plane_res_b > res_blocks) > - res_blocks = result_prev->plane_res_b; This last thing is part of the glk+ watermark formula as well. But I'm not 100% convinced that it's needed. One might assume that the the non-decrasing latency values guarantee that the resulting watermarks are also non-decreasing. But I've not actually done the math to see if that's true. Hmm. It might not actually be true on account of the 'memory latency microseconds >= line time microseconds' check when selecting which method to use to calculate the watermark. Not quite sure which way that would make things go. We also seem to be missing the res_lines handling here. But given that we only did this for compressed fbs before I don't think this patch is making things much worse by limiting this to pre-glk only. The glk+ handling and res_lines fix could be done as a followup. Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > } > > if (INTEL_GEN(dev_priv) >= 11) { > -- > 2.14.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx