| Message ID | 1534994984-6080-1-git-send-email-karthik.b.s@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/icl: Avoid Gen10 watermark workarounds in Gen11 | expand |
On Thu, Aug 23, 2018 at 08:59:44AM +0530, Karthik B S wrote: > Check added to skip the watermark workarounds intended for Gen10 and > below platforms in Gen11. This seems a bit ambiguous for me, could you please improve the commit message a bit? > > Signed-off-by: Karthik B S <karthik.b.s@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 39 +++++++++++++++++++++++---------------- > 1 file changed, 23 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index d99e5fa..1928fe0 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4677,28 +4677,35 @@ 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); > - > - /* Display WA #1126: skl,bxt,kbl,glk */ > - if (level >= 1 && level <= 7) { > - if (wp->y_tiled) { > + if (INTEL_GEN(dev_priv) < 11) { > + /* Display WA #1125: skl,bxt,kbl,glk */ > + if (level == 0 && wp->rc_surface) > 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. > - */ > + /* > + * 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 (level >= 1 && level <= 7) { > if (result_prev->plane_res_b > res_blocks) > res_blocks = result_prev->plane_res_b; Everything above makes sense and I checked against spec and it is right. > + if (result_prev->plane_res_l > res_lines) > + res_lines = result_prev->plane_res_l; My on;y concern here is with this line... This seems a new addition that if needed needs to come in a separated patch with its own justification. Sorry for taking so long to review it. Thanks, Rodrigo. > } > > if (INTEL_GEN(dev_priv) >= 11) { > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 8/31/2018 8:02 PM, Rodrigo Vivi wrote: > On Thu, Aug 23, 2018 at 08:59:44AM +0530, Karthik B S wrote: >> Check added to skip the watermark workarounds intended for Gen10 and >> below platforms in Gen11. > This seems a bit ambiguous for me, could you please improve the commit > message a bit? Sure. I'll do it. > >> Signed-off-by: Karthik B S <karthik.b.s@intel.com> >> --- >> drivers/gpu/drm/i915/intel_pm.c | 39 +++++++++++++++++++++++---------------- >> 1 file changed, 23 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index d99e5fa..1928fe0 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -4677,28 +4677,35 @@ 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); >> - >> - /* Display WA #1126: skl,bxt,kbl,glk */ >> - if (level >= 1 && level <= 7) { >> - if (wp->y_tiled) { >> + if (INTEL_GEN(dev_priv) < 11) { >> + /* Display WA #1125: skl,bxt,kbl,glk */ >> + if (level == 0 && wp->rc_surface) >> 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. >> - */ >> + /* >> + * 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 (level >= 1 && level <= 7) { >> if (result_prev->plane_res_b > res_blocks) >> res_blocks = result_prev->plane_res_b; > Everything above makes sense and I checked against spec and it is right. > >> + if (result_prev->plane_res_l > res_lines) >> + res_lines = result_prev->plane_res_l; > My on;y concern here is with this line... > This seems a new addition that if needed needs to come in a > separated patch with its own justification. Sure. I will make a separate patch for the same. > > Sorry for taking so long to review it. > > Thanks, > Rodrigo. > >> } >> >> if (INTEL_GEN(dev_priv) >= 11) { >> -- >> 2.7.4 >> >> _______________________________________________ >> 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 d99e5fa..1928fe0 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4677,28 +4677,35 @@ 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); - - /* Display WA #1126: skl,bxt,kbl,glk */ - if (level >= 1 && level <= 7) { - if (wp->y_tiled) { + if (INTEL_GEN(dev_priv) < 11) { + /* Display WA #1125: skl,bxt,kbl,glk */ + if (level == 0 && wp->rc_surface) 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. - */ + /* + * 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 (level >= 1 && level <= 7) { if (result_prev->plane_res_b > res_blocks) res_blocks = result_prev->plane_res_b; + if (result_prev->plane_res_l > res_lines) + res_lines = result_prev->plane_res_l; } if (INTEL_GEN(dev_priv) >= 11) {
Check added to skip the watermark workarounds intended for Gen10 and below platforms in Gen11. Signed-off-by: Karthik B S <karthik.b.s@intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)