On Thu, Jan 14, 2016 at 05:32:44PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.ku...@intel.com>
>
> don't always use 8 ddb as minimum, instead calculate using proper
> algorithm.
>
> Signed-off-by: Kumar, Mahesh <mahesh1.ku...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 57
> +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d33c4ff..64b39ec 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2994,6 +2994,59 @@ skl_get_total_relative_data_rate(const struct
> intel_crtc_state *cstate)
> return total_data_rate;
> }
>
> +static uint16_t
> +skl_ddb_min_alloc(const struct intel_crtc *crtc,
> + const struct drm_plane *plane, int y)
> +{
> + struct drm_framebuffer *fb = plane->state->fb;
> + struct intel_plane_state *pstate = to_intel_plane_state(plane->state);
> + uint16_t min_alloc;
> + uint32_t src_w, src_h;
> +
> + /* For packed formats, no y-plane, return 0 */
> + if (y && !(fb->pixel_format == DRM_FORMAT_NV12))
> + return 0;
Your general logic all looks fine, but I'd move the simple/common case
of !tile-y up here and immediately return the constant 8 to make it a
bit more obvious. Then the remaining code below will only apply to
tile-y formats and you can even remove a level of nesting farther down.
> +
> + if (drm_rect_width(&pstate->src)) {
> + src_w = drm_rect_width(&pstate->src) >> 16;
> + src_h = drm_rect_height(&pstate->src) >> 16;
> + } else {
> + src_w = crtc->config->pipe_src_w;
> + src_h = crtc->config->pipe_src_h;
> + }
> +
> + if (intel_rotation_90_or_270(plane->state->rotation))
> + swap(src_w, src_h);
> +
> + if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> + fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
> + uint32_t min_scanlines = 8;
> + uint8_t bytes_per_pixel =
> + y ? drm_format_plane_cpp(fb->pixel_format, 1) :
> + drm_format_plane_cpp(fb->pixel_format, 0);
Isn't this backwards? As noted on an earlier patch, the second
parameter isn't a "is y" boolean, but rather a "which plane number"
integer. For NV12, Y=0, UV=1.
> +
> + if (intel_rotation_90_or_270(plane->state->rotation)) {
> + switch (bytes_per_pixel) {
> + case 1:
> + min_scanlines = 32;
> + break;
> + case 2:
> + min_scanlines = 16;
> + break;
> + case 8:
> + WARN(1, "Unsupported pixel depth for rotation");
> + }
Can we simplify this to a min_scalines = 32/bpp instead of the switch statement?
Matt
> + }
> + min_alloc = DIV_ROUND_UP((4 * src_w / (y ? 1 : 2) *
> + bytes_per_pixel), 512) * min_scanlines/4 + 3;
> + } else {
> + min_alloc = 8;
> + }
> +
> + return min_alloc;
> +}
> +
> +
> static void
> skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> struct skl_ddb_allocation *ddb /* out */)
> @@ -3038,9 +3091,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> if (plane->type == DRM_PLANE_TYPE_CURSOR)
> continue;
>
> - minimum[id] = 8;
> + minimum[id] = skl_ddb_min_alloc(intel_crtc, plane, 0);
> alloc_size -= minimum[id];
> - y_minimum[id] = (fb->pixel_format == DRM_FORMAT_NV12) ? 8 : 0;
> + y_minimum[id] = skl_ddb_min_alloc(intel_crtc, plane, 1);
> alloc_size -= y_minimum[id];
> }
>
> --
> 2.4.3
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
