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....@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
