diff mbox

[3/7] drm/i915/skl+: calculate ddb minimum allocation

Message ID 1452772968-24772-4-git-send-email-shobhit.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Shobhit Jan. 14, 2016, 12:02 p.m. UTC
From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>

don't always use 8 ddb as minimum, instead calculate using proper
algorithm.

Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 57 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

Comments

Matt Roper Jan. 15, 2016, 12:39 a.m. UTC | #1
On Thu, Jan 14, 2016 at 05:32:44PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
> 
> don't always use 8 ddb as minimum, instead calculate using proper
> algorithm.
> 
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@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
>
diff mbox

Patch

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;
+
+	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);
+
+		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");
+			}
+		}
+		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];
 	}