Em Ter, 2018-10-09 às 16:55 -0700, Matt Roper escreveu: > On Thu, Oct 04, 2018 at 04:15:55PM -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. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > From a quick grep, it looks like there's a couple other CNL A0- > specific > workarounds in the codebase (in the watermark code). If we don't > want > to handle CNL A0 in this patch, then I think we should first land a > patch that removes the others and updates > intel_detect_preproduction_hw() to make it explicit that this is no > longer a supported stepping. This patch (and series) is potentially a "bug fix" due to changes in real-world not-A0 hardware (although I didn't mark this one as a fix since I doubt it will close anything in Bugzilla). The patch to remove the implementation of WAs in CNL:A0 is not a bug fix, but a rework. Fixes should always come before the reworks in order to facilitate backporting efforts. I do intend to propose patches removing the CNL A0 watermarks code (along with other reworks I have in mind), but I wanted to sort the bugs first. > > > --- > > 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 1392aa56a55a..910551e04d16 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4687,28 +4687,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)) { > > + /* Display WA #1125: skl,bxt,kbl */ > > Although the code is right, should we just write "gen9" (or > gen9display) > in this comment (and the one for 1126)? That's how the bspec lists > it > (GEN9:All), and removes the ambiguity of whether "kbl" in this > context > is also supposed to cover CFL, AML, WHL (which it does). Although that makes sense, it doesn't seem to follow the current (undocumented?) standard we have for display WAs. I can totally do this, but I would like some more opinions first. CC'ing the maintainers. Thanks for the reviews, Paulo > > > Matt > > > + 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; > > } > > > > if (INTEL_GEN(dev_priv) >= 11) { > > -- > > 2.14.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx