| Message ID | 1473209539-6689-6-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2016-09-06 at 21:52 -0300, Paulo Zanoni wrote: > During watermarks calculations, this value is used in 3 different > places. Only one of them was not using a hardcoded 4. Move the code > up > so everybody can benefit from the actual value. > > This should only help on situations with Y tiling + 90/270 rotation + > 1 or 2 bpp or NV12. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 56 +++++++++++++++++++++++------ > ------------ > 1 file changed, 32 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index f8ac928..5a23a91 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3499,7 +3499,8 @@ static uint32_t skl_wm_method1(uint32_t > pixel_rate, uint8_t cpp, uint32_t latenc > > static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t > pipe_htotal, > uint32_t horiz_pixels, uint8_t cpp, > - uint64_t tiling, uint32_t latency) > + uint64_t tiling, uint32_t latency, > + uint32_t y_min_scanlines) > { > uint32_t ret; > uint32_t plane_bytes_per_line, plane_blocks_per_line; > @@ -3512,9 +3513,9 @@ static uint32_t skl_wm_method2(uint32_t > pixel_rate, uint32_t pipe_htotal, > > if (tiling == I915_FORMAT_MOD_Y_TILED || > tiling == I915_FORMAT_MOD_Yf_TILED) { > - plane_bytes_per_line *= 4; > + plane_bytes_per_line *= y_min_scanlines; > plane_blocks_per_line = > DIV_ROUND_UP(plane_bytes_per_line, 512); > - plane_blocks_per_line /= 4; > + plane_blocks_per_line /= y_min_scanlines; > } else if (tiling == DRM_FORMAT_MOD_NONE) { > plane_blocks_per_line = > DIV_ROUND_UP(plane_bytes_per_line, 512) + 1; > } else { > @@ -3571,6 +3572,7 @@ static int skl_compute_plane_wm(const struct > drm_i915_private *dev_priv, > uint8_t cpp; > uint32_t width = 0, height = 0; > uint32_t plane_pixel_rate; > + uint32_t y_min_scanlines; > > if (latency == 0 || !cstate->base.active || !intel_pstate- > >base.visible) { > *enabled = false; > @@ -3586,38 +3588,44 @@ static int skl_compute_plane_wm(const struct > drm_i915_private *dev_priv, > cpp = drm_format_plane_cpp(fb->pixel_format, 0); > plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate, > intel_pstate); > > + if (intel_rotation_90_or_270(pstate->rotation)) { > + int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ? > + drm_format_plane_cpp(fb->pixel_format, 1) : > + drm_format_plane_cpp(fb->pixel_format, 0); > + > + switch (cpp) { > + case 1: > + y_min_scanlines = 16; > + break; > + case 2: > + y_min_scanlines = 8; > + break; > + default: > + WARN(1, "Unsupported pixel depth for > rotation"); This looks like it's leftover from the code that you moved around, but we should be erroring out here and returning -EINVAL here so the modeset actually fails. > + case 4: > + y_min_scanlines = 4; > + break; > + } > + } else { > + y_min_scanlines = 4; > + } > + > method1 = skl_wm_method1(plane_pixel_rate, cpp, latency); > method2 = skl_wm_method2(plane_pixel_rate, > cstate- > >base.adjusted_mode.crtc_htotal, > width, > cpp, > fb->modifier[0], > - latency); > + latency, > + y_min_scanlines); > > plane_bytes_per_line = width * cpp; > plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, > 512); > > if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || > fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) { > - uint32_t min_scanlines = 4; > - uint32_t y_tile_minimum; > - if (intel_rotation_90_or_270(pstate->rotation)) { > - int cpp = (fb->pixel_format == > DRM_FORMAT_NV12) ? > - drm_format_plane_cpp(fb- > >pixel_format, 1) : > - drm_format_plane_cpp(fb- > >pixel_format, 0); > - > - switch (cpp) { > - case 1: > - min_scanlines = 16; > - break; > - case 2: > - min_scanlines = 8; > - break; > - case 8: > - WARN(1, "Unsupported pixel depth for > rotation"); > - } > - } > - y_tile_minimum = plane_blocks_per_line * > min_scanlines; > + uint32_t y_tile_minimum = plane_blocks_per_line * > + y_min_scanlines; > selected_result = max(method2, y_tile_minimum); > } else { > if ((ddb_allocation / plane_blocks_per_line) >= 1) > @@ -3632,7 +3640,7 @@ static int skl_compute_plane_wm(const struct > drm_i915_private *dev_priv, > if (level >= 1 && level <= 7) { > if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || > fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) > - res_lines += 4; > + res_lines += y_min_scanlines; > else > res_blocks++; > }
Em Qua, 2016-09-07 às 19:03 -0400, Lyude escreveu: > On Tue, 2016-09-06 at 21:52 -0300, Paulo Zanoni wrote: > > > > During watermarks calculations, this value is used in 3 different > > places. Only one of them was not using a hardcoded 4. Move the code > > up > > so everybody can benefit from the actual value. > > > > This should only help on situations with Y tiling + 90/270 rotation > > + > > 1 or 2 bpp or NV12. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 56 +++++++++++++++++++++++------ > > ------------ > > 1 file changed, 32 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index f8ac928..5a23a91 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3499,7 +3499,8 @@ static uint32_t skl_wm_method1(uint32_t > > pixel_rate, uint8_t cpp, uint32_t latenc > > > > static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t > > pipe_htotal, > > uint32_t horiz_pixels, uint8_t cpp, > > - uint64_t tiling, uint32_t latency) > > + uint64_t tiling, uint32_t latency, > > + uint32_t y_min_scanlines) > > { > > uint32_t ret; > > uint32_t plane_bytes_per_line, plane_blocks_per_line; > > @@ -3512,9 +3513,9 @@ static uint32_t skl_wm_method2(uint32_t > > pixel_rate, uint32_t pipe_htotal, > > > > if (tiling == I915_FORMAT_MOD_Y_TILED || > > tiling == I915_FORMAT_MOD_Yf_TILED) { > > - plane_bytes_per_line *= 4; > > + plane_bytes_per_line *= y_min_scanlines; > > plane_blocks_per_line = > > DIV_ROUND_UP(plane_bytes_per_line, 512); > > - plane_blocks_per_line /= 4; > > + plane_blocks_per_line /= y_min_scanlines; > > } else if (tiling == DRM_FORMAT_MOD_NONE) { > > plane_blocks_per_line = > > DIV_ROUND_UP(plane_bytes_per_line, 512) + 1; > > } else { > > @@ -3571,6 +3572,7 @@ static int skl_compute_plane_wm(const struct > > drm_i915_private *dev_priv, > > uint8_t cpp; > > uint32_t width = 0, height = 0; > > uint32_t plane_pixel_rate; > > + uint32_t y_min_scanlines; > > > > if (latency == 0 || !cstate->base.active || !intel_pstate- > > > > > > base.visible) { > > *enabled = false; > > @@ -3586,38 +3588,44 @@ static int skl_compute_plane_wm(const > > struct > > drm_i915_private *dev_priv, > > cpp = drm_format_plane_cpp(fb->pixel_format, 0); > > plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate, > > intel_pstate); > > > > + if (intel_rotation_90_or_270(pstate->rotation)) { > > + int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ? > > + drm_format_plane_cpp(fb->pixel_format, 1) > > : > > + drm_format_plane_cpp(fb->pixel_format, 0); > > + > > + switch (cpp) { > > + case 1: > > + y_min_scanlines = 16; > > + break; > > + case 2: > > + y_min_scanlines = 8; > > + break; > > + default: > > + WARN(1, "Unsupported pixel depth for > > rotation"); > > This looks like it's leftover from the code that you moved around, > but > we should be erroring out here and returning -EINVAL here so the > modeset actually fails. Good catch. I'll do this as a separate patch since it's a separate fix. > > > > > + case 4: > > + y_min_scanlines = 4; > > + break; > > + } > > + } else { > > + y_min_scanlines = 4; > > + } > > + > > method1 = skl_wm_method1(plane_pixel_rate, cpp, latency); > > method2 = skl_wm_method2(plane_pixel_rate, > > cstate- > > > > > > base.adjusted_mode.crtc_htotal, > > width, > > cpp, > > fb->modifier[0], > > - latency); > > + latency, > > + y_min_scanlines); > > > > plane_bytes_per_line = width * cpp; > > plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, > > 512); > > > > if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || > > fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) { > > - uint32_t min_scanlines = 4; > > - uint32_t y_tile_minimum; > > - if (intel_rotation_90_or_270(pstate->rotation)) { > > - int cpp = (fb->pixel_format == > > DRM_FORMAT_NV12) ? > > - drm_format_plane_cpp(fb- > > > > > > pixel_format, 1) : > > - drm_format_plane_cpp(fb- > > > > > > pixel_format, 0); > > - > > - switch (cpp) { > > - case 1: > > - min_scanlines = 16; > > - break; > > - case 2: > > - min_scanlines = 8; > > - break; > > - case 8: > > - WARN(1, "Unsupported pixel depth > > for > > rotation"); > > - } > > - } > > - y_tile_minimum = plane_blocks_per_line * > > min_scanlines; > > + uint32_t y_tile_minimum = plane_blocks_per_line * > > + y_min_scanlines; > > selected_result = max(method2, y_tile_minimum); > > } else { > > if ((ddb_allocation / plane_blocks_per_line) >= 1) > > @@ -3632,7 +3640,7 @@ static int skl_compute_plane_wm(const struct > > drm_i915_private *dev_priv, > > if (level >= 1 && level <= 7) { > > if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || > > fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) > > - res_lines += 4; > > + res_lines += y_min_scanlines; > > else > > res_blocks++; > > }
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f8ac928..5a23a91 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3499,7 +3499,8 @@ static uint32_t skl_wm_method1(uint32_t pixel_rate, uint8_t cpp, uint32_t latenc static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal, uint32_t horiz_pixels, uint8_t cpp, - uint64_t tiling, uint32_t latency) + uint64_t tiling, uint32_t latency, + uint32_t y_min_scanlines) { uint32_t ret; uint32_t plane_bytes_per_line, plane_blocks_per_line; @@ -3512,9 +3513,9 @@ static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal, if (tiling == I915_FORMAT_MOD_Y_TILED || tiling == I915_FORMAT_MOD_Yf_TILED) { - plane_bytes_per_line *= 4; + plane_bytes_per_line *= y_min_scanlines; plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512); - plane_blocks_per_line /= 4; + plane_blocks_per_line /= y_min_scanlines; } else if (tiling == DRM_FORMAT_MOD_NONE) { plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512) + 1; } else { @@ -3571,6 +3572,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, uint8_t cpp; uint32_t width = 0, height = 0; uint32_t plane_pixel_rate; + uint32_t y_min_scanlines; if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) { *enabled = false; @@ -3586,38 +3588,44 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, cpp = drm_format_plane_cpp(fb->pixel_format, 0); plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate, intel_pstate); + if (intel_rotation_90_or_270(pstate->rotation)) { + int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ? + drm_format_plane_cpp(fb->pixel_format, 1) : + drm_format_plane_cpp(fb->pixel_format, 0); + + switch (cpp) { + case 1: + y_min_scanlines = 16; + break; + case 2: + y_min_scanlines = 8; + break; + default: + WARN(1, "Unsupported pixel depth for rotation"); + case 4: + y_min_scanlines = 4; + break; + } + } else { + y_min_scanlines = 4; + } + method1 = skl_wm_method1(plane_pixel_rate, cpp, latency); method2 = skl_wm_method2(plane_pixel_rate, cstate->base.adjusted_mode.crtc_htotal, width, cpp, fb->modifier[0], - latency); + latency, + y_min_scanlines); plane_bytes_per_line = width * cpp; plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512); if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) { - uint32_t min_scanlines = 4; - uint32_t y_tile_minimum; - if (intel_rotation_90_or_270(pstate->rotation)) { - int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ? - drm_format_plane_cpp(fb->pixel_format, 1) : - drm_format_plane_cpp(fb->pixel_format, 0); - - switch (cpp) { - case 1: - min_scanlines = 16; - break; - case 2: - min_scanlines = 8; - break; - case 8: - WARN(1, "Unsupported pixel depth for rotation"); - } - } - y_tile_minimum = plane_blocks_per_line * min_scanlines; + uint32_t y_tile_minimum = plane_blocks_per_line * + y_min_scanlines; selected_result = max(method2, y_tile_minimum); } else { if ((ddb_allocation / plane_blocks_per_line) >= 1) @@ -3632,7 +3640,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if (level >= 1 && level <= 7) { if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) - res_lines += 4; + res_lines += y_min_scanlines; else res_blocks++; }
During watermarks calculations, this value is used in 3 different places. Only one of them was not using a hardcoded 4. Move the code up so everybody can benefit from the actual value. This should only help on situations with Y tiling + 90/270 rotation + 1 or 2 bpp or NV12. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 56 +++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 24 deletions(-)