diff mbox

[20/20] drm/i915: Enable skylake sprite plane scaling using shared scalers

Message ID 1427943589-6254-21-git-send-email-chandra.konduru@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chandra Konduru April 2, 2015, 2:59 a.m. UTC 
This patch enables skylake sprite plane display scaling using shared
scalers atomic desgin.

v2:
-use single copy of scaler limits (Matt)

v3:
-detaching scalers moved to crtc commit path (Matt)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c |   60 +++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 10 deletions(-)

Comments

Shuang He April 2, 2015, 2:44 p.m. UTC | #1 
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6118
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              272/272              271/272
ILK                                  302/302              302/302
SNB                                  303/303              303/303
IVB                                  338/338              338/338
BYT                 -1              287/287              286/287
HSW                                  361/361              361/361
BDW                                  308/308              308/308
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gen3_render_tiledx_blits      FAIL(6)PASS(3)      FAIL(2)
*BYT  igt@gem_exec_bad_domains@conflicting-write-domain      PASS(9)      FAIL(1)PASS(1)
Note: You need to pay more attention to line start with '*'
Chandra Konduru April 2, 2015, 5:20 p.m. UTC | #2 
Hi Shuang,
Looking at starting with '*':
*BYT  igt@gem_exec_bad_domains@conflicting-write-domain      PASS(9) FAIL(1)PASS(1)

Above failure seems unrelated to my patch series. I suspect this 
pre-exist before my changes.
Can you double check whether above failure is pre-existing before 
any action is taken from my side?

Also I just ran it on skl (that's the system I have) without any failures:
~/gfx/sources/intel-gpu-tools/tests$ sudo ./gem_exec_bad_domains 
IGT-Version: 1.10-g4f076bc (x86_64) (Linux: 4.0.0-rc3+ x86_64)
Subtest cpu-domain: SUCCESS (0.004s)
Subtest gtt-domain: SUCCESS (0.000s)
Subtest conflicting-write-domain: SUCCESS (0.000s)
Subtest double-write-domain: SUCCESS (0.000s)
Subtest invalid-gpu-domain: SUCCESS (0.000s)
~/gfx/sources/intel-gpu-tools/tests$


-Chandra

> -----Original Message-----
> From: He, Shuang
> Sent: Thursday, April 02, 2015 7:45 AM
> To: He, Shuang; Gao, Ethan; intel-gfx@lists.freedesktop.org; Konduru, Chandra
> Subject: RE: [Intel-gfx] [PATCH 20/20] drm/i915: Enable skylake sprite plane
> scaling using shared scalers
> 
> Tested-By: PRC QA PRTS (Patch Regression Test System Contact:
> shuang.he@intel.com) Task id: 6118
> -------------------------------------Summary-------------------------------------
> Platform          Delta          drm-intel-nightly          Series Applied
> PNV                 -1              272/272              271/272
> ILK                                  302/302              302/302
> SNB                                  303/303              303/303
> IVB                                  338/338              338/338
> BYT                 -1              287/287              286/287
> HSW                                  361/361              361/361
> BDW                                  308/308              308/308
> -------------------------------------Detailed-------------------------------------
> Platform  Test                                drm-intel-nightly          Series Applied
>  PNV  igt@gen3_render_tiledx_blits      FAIL(6)PASS(3)      FAIL(2)
> *BYT  igt@gem_exec_bad_domains@conflicting-write-domain      PASS(9)
> FAIL(1)PASS(1)
> Note: You need to pay more attention to line start with '*'
Matt Roper April 2, 2015, 11:05 p.m. UTC | #3 
On Wed, Apr 01, 2015 at 07:59:49PM -0700, Chandra Konduru wrote:
> This patch enables skylake sprite plane display scaling using shared
> scalers atomic desgin.
> 
> v2:
> -use single copy of scaler limits (Matt)
> 
> v3:
> -detaching scalers moved to crtc commit path (Matt)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c |   60 +++++++++++++++++++++++++++++------
>  1 file changed, 50 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c8feff7..562f90c 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -33,6 +33,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_rect.h>
> +#include <drm/drm_atomic.h>
>  #include <drm/drm_plane_helper.h>
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
> @@ -194,6 +195,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;
>  	unsigned long surf_addr;
> +	struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
> +	int scaler_id;
>  
>  	plane_ctl = PLANE_CTL_ENABLE |
>  		PLANE_CTL_PIPE_CSC_ENABLE;
> @@ -264,6 +267,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
>  					       fb->pixel_format);
>  
> +	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
> +
>  	/* Sizes are 0 based */
>  	src_w--;
>  	src_h--;
> @@ -283,10 +288,30 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  
>  	surf_addr = intel_plane_obj_offset(intel_plane, obj);
>  
> +	/* program plane scaler */
> +	if (scaler_id >= 0) {
> +		uint32_t ps_ctrl = 0;
> +
> +		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane,
> +			PS_PLANE_SEL(plane));
> +		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) |
> +			crtc_state->scaler_state.scalers[scaler_id].mode |
> +			crtc_state->scaler_state.scalers[scaler_id].filter;
> +		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
> +		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> +		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
> +		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id),
> +			((crtc_w + 1) << 16)|(crtc_h + 1));
> +
> +		I915_WRITE(PLANE_POS(pipe, plane), 0);
> +		I915_WRITE(PLANE_SIZE(pipe, plane), (src_h << 16) | src_w);
> +	} else {
> +		I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
> +		I915_WRITE(PLANE_SIZE(pipe, plane), (crtc_h << 16) | crtc_w);
> +	}
> +
>  	I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
>  	I915_WRITE(PLANE_STRIDE(pipe, plane), fb->pitches[0] / stride_div);
> -	I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
> -	I915_WRITE(PLANE_SIZE(pipe, plane), (crtc_h << 16) | crtc_w);
>  	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
>  	I915_WRITE(PLANE_SURF(pipe, plane), surf_addr);
>  	POSTING_READ(PLANE_SURF(pipe, plane));
> @@ -863,7 +888,9 @@ static int
>  intel_check_sprite_plane(struct drm_plane *plane,
>  			 struct intel_plane_state *state)
>  {
> +	struct drm_device *dev = plane->dev;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
> +	struct intel_crtc_state *crtc_state;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct drm_framebuffer *fb = state->base.fb;
>  	int crtc_x, crtc_y;
> @@ -872,11 +899,16 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  	struct drm_rect *src = &state->src;
>  	struct drm_rect *dst = &state->dst;
>  	const struct drm_rect *clip = &state->clip;
> +	struct intel_crtc_scaler_state *scaler_state;
>  	int hscale, vscale;
>  	int max_scale, min_scale;
>  	int pixel_size;
> +	int ret;
>  
>  	intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
> +	crtc_state = state->base.state ?
> +		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
> +	scaler_state = crtc_state ? &crtc_state->scaler_state : NULL;
>  
>  	if (!fb) {
>  		state->visible = false;
> @@ -903,6 +935,11 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  	max_scale = intel_plane->max_downscale << 16;
>  	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
>  
> +	if (INTEL_INFO(dev)->gen >= 9 && scaler_state && scaler_state->num_scalers) {
> +		min_scale = 1;
> +		max_scale = (100 << 16) / scaler_state->min_hsr;
> +	}
> +
>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
>  			state->base.rotation);
>  
> @@ -998,8 +1035,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  		width_bytes = ((src_x * pixel_size) & 63) +
>  					src_w * pixel_size;
>  
> -		if (src_w > 2048 || src_h > 2048 ||
> -		    width_bytes > 4096 || fb->pitches[0] > 4096) {
> +		if (INTEL_INFO(dev)->gen < 9 && (src_w > 2048 || src_h > 2048 ||
> +		    width_bytes > 4096 || fb->pitches[0] > 4096)) {
>  			DRM_DEBUG_KMS("Source dimensions exceed hardware limits\n");
>  			return -EINVAL;
>  		}
> @@ -1053,6 +1090,13 @@ finish:
>  		}
>  	}
>  
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		ret = skl_update_scaler_users(intel_crtc, crtc_state, intel_plane,
> +			state, 0);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1264,12 +1308,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  		}
>  		break;
>  	case 9:
> -		/*
> -		 * FIXME: Skylake planes can be scaled (with some restrictions),
> -		 * but this is for another time.
> -		 */
> -		intel_plane->can_scale = false;
> -		intel_plane->max_downscale = 1;
> +		intel_plane->can_scale = true;
> +		intel_plane->max_downscale = 2; /* updated later */

Again, not true about being updated later.  We just wind up not using
the value on SKL.

>  		intel_plane->update_plane = skl_update_plane;
>  		intel_plane->disable_plane = skl_disable_plane;
>  		state->scaler_id = -1;
> -- 
> 1.7.9.5
>
Shuang He April 3, 2015, 2:50 a.m. UTC | #4 
Hi, Chandra
	Yes, I agree
	There is some noise pre-existing bugs, you can ignore it for now. We're figuring out an way to filter them out

Thanks
	--Shuang

> -----Original Message-----
> From: Konduru, Chandra
> Sent: Friday, April 3, 2015 1:21 AM
> To: He, Shuang; Gao, Ethan; intel-gfx@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH 20/20] drm/i915: Enable skylake sprite plane
> scaling using shared scalers
> 
> Hi Shuang,
> Looking at starting with '*':
> *BYT  igt@gem_exec_bad_domains@conflicting-write-domain      PASS(9)
> FAIL(1)PASS(1)
> 
> Above failure seems unrelated to my patch series. I suspect this
> pre-exist before my changes.
> Can you double check whether above failure is pre-existing before
> any action is taken from my side?
> 
> Also I just ran it on skl (that's the system I have) without any failures:
> ~/gfx/sources/intel-gpu-tools/tests$ sudo ./gem_exec_bad_domains
> IGT-Version: 1.10-g4f076bc (x86_64) (Linux: 4.0.0-rc3+ x86_64)
> Subtest cpu-domain: SUCCESS (0.004s)
> Subtest gtt-domain: SUCCESS (0.000s)
> Subtest conflicting-write-domain: SUCCESS (0.000s)
> Subtest double-write-domain: SUCCESS (0.000s)
> Subtest invalid-gpu-domain: SUCCESS (0.000s)
> ~/gfx/sources/intel-gpu-tools/tests$
> 
> 
> -Chandra
> 
> > -----Original Message-----
> > From: He, Shuang
> > Sent: Thursday, April 02, 2015 7:45 AM
> > To: He, Shuang; Gao, Ethan; intel-gfx@lists.freedesktop.org; Konduru,
> Chandra
> > Subject: RE: [Intel-gfx] [PATCH 20/20] drm/i915: Enable skylake sprite plane
> > scaling using shared scalers
> >
> > Tested-By: PRC QA PRTS (Patch Regression Test System Contact:
> > shuang.he@intel.com) Task id: 6118
> > -------------------------------------Summary-------------------------------------
> > Platform          Delta          drm-intel-nightly          Series Applied
> > PNV                 -1              272/272              271/272
> > ILK                                  302/302              302/302
> > SNB                                  303/303              303/303
> > IVB                                  338/338              338/338
> > BYT                 -1              287/287              286/287
> > HSW                                  361/361              361/361
> > BDW                                  308/308              308/308
> > -------------------------------------Detailed-------------------------------------
> > Platform  Test                                drm-intel-nightly          Series Applied
> >  PNV  igt@gen3_render_tiledx_blits      FAIL(6)PASS(3)      FAIL(2)
> > *BYT  igt@gem_exec_bad_domains@conflicting-write-domain      PASS(9)
> > FAIL(1)PASS(1)
> > Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c8feff7..562f90c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -33,6 +33,7 @@ 
 #include <drm/drm_crtc.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_rect.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_plane_helper.h>
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
@@ -194,6 +195,8 @@  skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;
 	unsigned long surf_addr;
+	struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
+	int scaler_id;
 
 	plane_ctl = PLANE_CTL_ENABLE |
 		PLANE_CTL_PIPE_CSC_ENABLE;
@@ -264,6 +267,8 @@  skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
 					       fb->pixel_format);
 
+	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -283,10 +288,30 @@  skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 
 	surf_addr = intel_plane_obj_offset(intel_plane, obj);
 
+	/* program plane scaler */
+	if (scaler_id >= 0) {
+		uint32_t ps_ctrl = 0;
+
+		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane,
+			PS_PLANE_SEL(plane));
+		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) |
+			crtc_state->scaler_state.scalers[scaler_id].mode |
+			crtc_state->scaler_state.scalers[scaler_id].filter;
+		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
+		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
+		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
+		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id),
+			((crtc_w + 1) << 16)|(crtc_h + 1));
+
+		I915_WRITE(PLANE_POS(pipe, plane), 0);
+		I915_WRITE(PLANE_SIZE(pipe, plane), (src_h << 16) | src_w);
+	} else {
+		I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
+		I915_WRITE(PLANE_SIZE(pipe, plane), (crtc_h << 16) | crtc_w);
+	}
+
 	I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
 	I915_WRITE(PLANE_STRIDE(pipe, plane), fb->pitches[0] / stride_div);
-	I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
-	I915_WRITE(PLANE_SIZE(pipe, plane), (crtc_h << 16) | crtc_w);
 	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
 	I915_WRITE(PLANE_SURF(pipe, plane), surf_addr);
 	POSTING_READ(PLANE_SURF(pipe, plane));
@@ -863,7 +888,9 @@  static int
 intel_check_sprite_plane(struct drm_plane *plane,
 			 struct intel_plane_state *state)
 {
+	struct drm_device *dev = plane->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
+	struct intel_crtc_state *crtc_state;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = state->base.fb;
 	int crtc_x, crtc_y;
@@ -872,11 +899,16 @@  intel_check_sprite_plane(struct drm_plane *plane,
 	struct drm_rect *src = &state->src;
 	struct drm_rect *dst = &state->dst;
 	const struct drm_rect *clip = &state->clip;
+	struct intel_crtc_scaler_state *scaler_state;
 	int hscale, vscale;
 	int max_scale, min_scale;
 	int pixel_size;
+	int ret;
 
 	intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
+	crtc_state = state->base.state ?
+		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
+	scaler_state = crtc_state ? &crtc_state->scaler_state : NULL;
 
 	if (!fb) {
 		state->visible = false;
@@ -903,6 +935,11 @@  intel_check_sprite_plane(struct drm_plane *plane,
 	max_scale = intel_plane->max_downscale << 16;
 	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
 
+	if (INTEL_INFO(dev)->gen >= 9 && scaler_state && scaler_state->num_scalers) {
+		min_scale = 1;
+		max_scale = (100 << 16) / scaler_state->min_hsr;
+	}
+
 	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
 			state->base.rotation);
 
@@ -998,8 +1035,8 @@  intel_check_sprite_plane(struct drm_plane *plane,
 		width_bytes = ((src_x * pixel_size) & 63) +
 					src_w * pixel_size;
 
-		if (src_w > 2048 || src_h > 2048 ||
-		    width_bytes > 4096 || fb->pitches[0] > 4096) {
+		if (INTEL_INFO(dev)->gen < 9 && (src_w > 2048 || src_h > 2048 ||
+		    width_bytes > 4096 || fb->pitches[0] > 4096)) {
 			DRM_DEBUG_KMS("Source dimensions exceed hardware limits\n");
 			return -EINVAL;
 		}
@@ -1053,6 +1090,13 @@  finish:
 		}
 	}
 
+	if (INTEL_INFO(dev)->gen >= 9) {
+		ret = skl_update_scaler_users(intel_crtc, crtc_state, intel_plane,
+			state, 0);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -1264,12 +1308,8 @@  intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 		}
 		break;
 	case 9:
-		/*
-		 * FIXME: Skylake planes can be scaled (with some restrictions),
-		 * but this is for another time.
-		 */
-		intel_plane->can_scale = false;
-		intel_plane->max_downscale = 1;
+		intel_plane->can_scale = true;
+		intel_plane->max_downscale = 2; /* updated later */
 		intel_plane->update_plane = skl_update_plane;
 		intel_plane->disable_plane = skl_disable_plane;
 		state->scaler_id = -1;