[PATCH 1/3] drm/i915/skl: Allow universal planes to position

[PATCH 1/3] drm/i915/skl: Allow universal planes to position

[Sonika Jindal]

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 437a679..e1b0c4d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12183,16 +12183,21 @@ intel_check_primary_plane(struct drm_plane *plane,
 	struct drm_rect *dest = &state->dst;
 	struct drm_rect *src = &state->src;
 	const struct drm_rect *clip = &state->clip;
+	bool can_position = false;
 	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
 
+	if (INTEL_INFO(dev)->gen >= 9)
+		can_position = true;
+
 	ret = drm_plane_helper_check_update(plane, crtc, fb,
 					    src, dest, clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
-					    false, true, &state->visible);
+					    can_position, true,
+					    &state->visible);
 	if (ret)
 		return ret;
 
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Using plane state parameters instead of pipe's

[Sonika Jindal]

For primary plane, we can use the plane's state for src width and height
like sprite plane.

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e1b0c4d..afdc101 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3001,6 +3001,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	int pipe = intel_crtc->pipe;
 	u32 plane_ctl, stride_div;
 	unsigned long surf_addr;
+	struct drm_plane *plane;
 
 	if (!intel_crtc->primary_enabled) {
 		I915_WRITE(PLANE_CTL(pipe, 0), 0);
@@ -3060,14 +3061,15 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 		MISSING_CASE(fb->modifier[0]);
 	}
 
+	plane = crtc->primary;
 	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
-	if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180))
+	if (plane->state->rotation == BIT(DRM_ROTATE_180))
 		plane_ctl |= PLANE_CTL_ROTATE_180;
 
 	obj = intel_fb_obj(fb);
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
 					       fb->pixel_format);
-	surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj);
+	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj);
 
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 
@@ -3079,8 +3081,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	I915_WRITE(PLANE_POS(pipe, 0), 0);
 	I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
 	I915_WRITE(PLANE_SIZE(pipe, 0),
-		   (intel_crtc->config->pipe_src_h - 1) << 16 |
-		   (intel_crtc->config->pipe_src_w - 1));
+		   ((plane->state->src_h >> 16) - 1) << 16 |
+		   ((plane->state->src_w >> 16) - 1));
 	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
 	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
 
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Using plane state parameters instead of pipe's

[Daniel Vetter]

On Thu, Mar 05, 2015 at 02:51:27PM +0530, Sonika Jindal wrote:
> For primary plane, we can use the plane's state for src width and height
> like sprite plane.
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e1b0c4d..afdc101 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3001,6 +3001,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,

I think longer-term we need to switch this callback over to the plane
callbacks and stop passing drm_crtc around. But this is a step into the
right direction at least.
-Daniel

>  	int pipe = intel_crtc->pipe;
>  	u32 plane_ctl, stride_div;
>  	unsigned long surf_addr;
> +	struct drm_plane *plane;
>  
>  	if (!intel_crtc->primary_enabled) {
>  		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> @@ -3060,14 +3061,15 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  		MISSING_CASE(fb->modifier[0]);
>  	}
>  
> +	plane = crtc->primary;
>  	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> -	if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180))
> +	if (plane->state->rotation == BIT(DRM_ROTATE_180))
>  		plane_ctl |= PLANE_CTL_ROTATE_180;
>  
>  	obj = intel_fb_obj(fb);
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
>  					       fb->pixel_format);
> -	surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj);
> +	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj);
>  
>  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
>  
> @@ -3079,8 +3081,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	I915_WRITE(PLANE_POS(pipe, 0), 0);
>  	I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
>  	I915_WRITE(PLANE_SIZE(pipe, 0),
> -		   (intel_crtc->config->pipe_src_h - 1) << 16 |
> -		   (intel_crtc->config->pipe_src_w - 1));
> +		   ((plane->state->src_h >> 16) - 1) << 16 |
> +		   ((plane->state->src_w >> 16) - 1));
>  	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
>  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/skl: Support for 90/270 rotation

[Sonika Jindal]

v2: Moving creation of property in a function, checking for 90/270
rotation simultaneously (Chris)
Letting primary plane to be positioned
v3: Adding if/else for 90/270 and rest params programming, adding check for
pixel_format, some cleanup (review comments)
v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
and size programming (Ville)
v5: Rebased on -nightly and Tvrtko's series for gtt remapping.

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 Please note that this is on top of Tvrtko's patches for rotated gtt remapping
 titled: [PATCH v2 0/8] Skylake 90/270 display rotation
---
 drivers/gpu/drm/i915/i915_reg.h      |    2 +
 drivers/gpu/drm/i915/intel_display.c |  105 +++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |    5 ++
 drivers/gpu/drm/i915/intel_sprite.c  |   52 ++++++++++++-----
 4 files changed, 128 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 56b97c4..870e076 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4825,7 +4825,9 @@ enum skl_disp_power_wells {
 #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY	(  3 << 4)
 #define   PLANE_CTL_ROTATE_MASK			0x3
 #define   PLANE_CTL_ROTATE_0			0x0
+#define   PLANE_CTL_ROTATE_90			0x1
 #define   PLANE_CTL_ROTATE_180			0x2
+#define   PLANE_CTL_ROTATE_270			0x3
 #define _PLANE_STRIDE_1_A			0x70188
 #define _PLANE_STRIDE_2_A			0x70288
 #define _PLANE_STRIDE_3_A			0x70388
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index afdc101..9387ba0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2189,11 +2189,11 @@ static bool need_vtd_wa(struct drm_device *dev)
 	return false;
 }
 
-static int
+unsigned int
 intel_tile_height(struct drm_device *dev, uint32_t bits_per_pixel,
 		  uint64_t fb_format_modifier)
 {
-	int tile_height;
+	unsigned int tile_height;
 
 	switch (fb_format_modifier) {
 	case DRM_FORMAT_MOD_NONE:
@@ -2240,7 +2240,7 @@ intel_fb_align_height(struct drm_device *dev, int height, uint32_t pixel_format,
 {
 	uint32_t bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
 
-	return ALIGN(height, intel_fb_tile_height(dev, bits_per_pixel,
+	return ALIGN(height, intel_tile_height(dev, bits_per_pixel,
 						  fb_format_modifier));
 }
 
@@ -2380,6 +2380,28 @@ int intel_fb_ggtt_view(struct drm_plane_state *plane_state,
 		return -EINVAL;
 	}
 
+	switch (fb->pixel_format) {
+		case DRM_FORMAT_XRGB8888:
+		case DRM_FORMAT_ARGB8888:
+		case DRM_FORMAT_XBGR8888:
+		case DRM_FORMAT_ABGR8888:
+		case DRM_FORMAT_XRGB2101010:
+		case DRM_FORMAT_ARGB2101010:
+		case DRM_FORMAT_XBGR2101010:
+		case DRM_FORMAT_ABGR2101010:
+		case DRM_FORMAT_YUYV:
+		case DRM_FORMAT_YVYU:
+		case DRM_FORMAT_UYVY:
+		case DRM_FORMAT_VYUY:
+		case DRM_FORMAT_NV12:
+			break;
+
+		default:
+			DRM_DEBUG_KMS("Unsupported pixel format:%d for 90/270 rotation!\n",
+			      fb->pixel_format);
+			return -EINVAL;
+	}
+
 	return 1;
 }
 
@@ -2999,7 +3021,10 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_i915_gem_object *obj;
 	int pipe = intel_crtc->pipe;
-	u32 plane_ctl, stride_div;
+	u32 plane_ctl, stride_div, stride;
+	u32 tile_height, plane_offset, plane_size;
+	unsigned int rotation;
+	int x_offset, y_offset;
 	unsigned long surf_addr;
 	struct drm_plane *plane;
 
@@ -3063,8 +3088,21 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 
 	plane = crtc->primary;
 	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
-	if (plane->state->rotation == BIT(DRM_ROTATE_180))
+
+	rotation = plane->state->rotation;
+	switch (rotation) {
+	case BIT(DRM_ROTATE_90):
+		plane_ctl |= PLANE_CTL_ROTATE_90;
+		break;
+
+	case BIT(DRM_ROTATE_180):
 		plane_ctl |= PLANE_CTL_ROTATE_180;
+		break;
+
+	case BIT(DRM_ROTATE_270):
+		plane_ctl |= PLANE_CTL_ROTATE_270;
+		break;
+	}
 
 	obj = intel_fb_obj(fb);
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
@@ -3073,17 +3111,33 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 
+	if (rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
+		/* stride = Surface height in tiles */
+		tile_height = intel_tile_height(dev, fb->bits_per_pixel,
+						fb->modifier[0]);
+		stride = DIV_ROUND_UP(fb->height, tile_height);
+		x_offset = stride * tile_height - y - (plane->state->src_h >> 16);
+		y_offset = x;
+		plane_size = ((plane->state->src_w >> 16) - 1) << 16 |
+					((plane->state->src_h >> 16) - 1);
+	} else {
+		stride = fb->pitches[0] / stride_div;
+		x_offset = x;
+		y_offset = y;
+		plane_size = ((plane->state->src_h >> 16) - 1) << 16 |
+			((plane->state->src_w >> 16) - 1);
+	}
+	plane_offset = y_offset << 16 | x_offset;
+
 	DRM_DEBUG_KMS("Writing base %08lX %d,%d,%d,%d pitch=%d\n",
 		      surf_addr,
 		      x, y, fb->width, fb->height,
 		      fb->pitches[0]);
 
 	I915_WRITE(PLANE_POS(pipe, 0), 0);
-	I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
-	I915_WRITE(PLANE_SIZE(pipe, 0),
-		   ((plane->state->src_h >> 16) - 1) << 16 |
-		   ((plane->state->src_w >> 16) - 1));
-	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
+	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
+	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
+	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
 	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
 
 	POSTING_READ(PLANE_SURF(pipe, 0));
@@ -12445,23 +12499,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 				 intel_primary_formats, num_formats,
 				 DRM_PLANE_TYPE_PRIMARY);
 
-	if (INTEL_INFO(dev)->gen >= 4) {
-		if (!dev->mode_config.rotation_property)
-			dev->mode_config.rotation_property =
-				drm_mode_create_rotation_property(dev,
-							BIT(DRM_ROTATE_0) |
-							BIT(DRM_ROTATE_180));
-		if (dev->mode_config.rotation_property)
-			drm_object_attach_property(&primary->base.base,
-				dev->mode_config.rotation_property,
-				state->base.rotation);
-	}
+	if (INTEL_INFO(dev)->gen >= 4)
+		intel_create_rotation_property(dev, primary);
 
 	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
 
 	return &primary->base;
 }
 
+void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
+{
+	if (!dev->mode_config.rotation_property) {
+		unsigned long flags = BIT(DRM_ROTATE_0) |
+			BIT(DRM_ROTATE_180);
+
+		if (INTEL_INFO(dev)->gen >= 9)
+			flags |= BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270);
+
+		dev->mode_config.rotation_property =
+			drm_mode_create_rotation_property(dev, flags);
+	}
+	if (dev->mode_config.rotation_property)
+		drm_object_attach_property(&plane->base.base,
+				dev->mode_config.rotation_property,
+				plane->base.state->rotation);
+}
+
 static int
 intel_check_cursor_plane(struct drm_plane *plane,
 			 struct intel_plane_state *state)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 715510a..4f428f6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -988,6 +988,11 @@ intel_rotation_90_or_270(unsigned int rotation)
 }
 struct sg_table *intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 					   struct drm_i915_gem_object *obj);
+unsigned int
+intel_tile_height(struct drm_device *dev, uint32_t bits_per_pixel,
+		  uint64_t fb_modifier);
+void intel_create_rotation_property(struct drm_device *dev,
+					struct intel_plane *plane);
 
 bool intel_wm_need_update(struct drm_plane *plane,
 			  struct drm_plane_state *state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index addc90e..960f993 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -189,9 +189,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	struct intel_plane *intel_plane = to_intel_plane(drm_plane);
 	const int pipe = intel_plane->pipe;
 	const int plane = intel_plane->plane + 1;
-	u32 plane_ctl, stride_div;
+	u32 plane_ctl, stride_div, stride;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	unsigned long surf_addr;
+	u32 tile_height, plane_offset, plane_size;
+	unsigned int rotation;
+	int x_offset, y_offset;
 
 	plane_ctl = I915_READ(PLANE_CTL(pipe, plane));
 
@@ -262,8 +265,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 		MISSING_CASE(fb->modifier[0]);
 	}
 
-	if (drm_plane->state->rotation == BIT(DRM_ROTATE_180))
+	rotation = drm_plane->state->rotation;
+	switch (rotation) {
+	case BIT(DRM_ROTATE_90):
+		plane_ctl |= PLANE_CTL_ROTATE_90;
+		break;
+
+	case BIT(DRM_ROTATE_180):
 		plane_ctl |= PLANE_CTL_ROTATE_180;
+		break;
+
+	case BIT(DRM_ROTATE_270):
+		plane_ctl |= PLANE_CTL_ROTATE_270;
+		break;
+	}
 
 	plane_ctl |= PLANE_CTL_ENABLE;
 	plane_ctl |= PLANE_CTL_PIPE_CSC_ENABLE;
@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 
 	surf_addr = intel_plane_obj_offset(intel_plane, obj);
 
-	I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
-	I915_WRITE(PLANE_STRIDE(pipe, plane), fb->pitches[0] / stride_div);
+	if (rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
+		/* stride: Surface height in tiles */
+		tile_height = intel_tile_height(dev, fb->bits_per_pixel,
+							fb->modifier[0]);
+		stride = DIV_ROUND_UP(fb->height, tile_height);
+		plane_size = (src_w << 16) | src_h;
+		x_offset = stride * tile_height - y - src_h;
+		y_offset = x;
+	} else {
+		stride = fb->pitches[0] / stride_div;
+		plane_size = (src_h << 16) | src_w;
+		x_offset = x;
+		y_offset = y;
+	}
+	plane_offset = y_offset << 16 | x_offset;
+
+	I915_WRITE(PLANE_OFFSET(pipe, plane), plane_offset);
+	I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
 	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_SIZE(pipe, plane), plane_size);
 	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
 	I915_WRITE(PLANE_SURF(pipe, plane), surf_addr);
 	POSTING_READ(PLANE_SURF(pipe, plane));
@@ -1519,16 +1550,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 		goto out;
 	}
 
-	if (!dev->mode_config.rotation_property)
-		dev->mode_config.rotation_property =
-			drm_mode_create_rotation_property(dev,
-							  BIT(DRM_ROTATE_0) |
-							  BIT(DRM_ROTATE_180));
-
-	if (dev->mode_config.rotation_property)
-		drm_object_attach_property(&intel_plane->base.base,
-					   dev->mode_config.rotation_property,
-					   state->base.rotation);
+	intel_create_rotation_property(dev, intel_plane);
 
 	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
 
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/skl: Support for 90/270 rotation

[Daniel Vetter]

On Thu, Mar 05, 2015 at 02:51:28PM +0530, Sonika Jindal wrote:
> @@ -1519,16 +1550,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  		goto out;
>  	}
>  
> -	if (!dev->mode_config.rotation_property)
> -		dev->mode_config.rotation_property =
> -			drm_mode_create_rotation_property(dev,
> -							  BIT(DRM_ROTATE_0) |
> -							  BIT(DRM_ROTATE_180));
> -
> -	if (dev->mode_config.rotation_property)
> -		drm_object_attach_property(&intel_plane->base.base,
> -					   dev->mode_config.rotation_property,
> -					   state->base.rotation);
> +	intel_create_rotation_property(dev, intel_plane);

I think back from the original rotation work we've had the leftover task
to move this into common code so that we do create the property just once
without this check.

I think this should be done now.
-Daniel
>  
>  	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/skl: Support for 90/270 rotation

[Ville Syrjälä]

On Thu, Mar 05, 2015 at 01:56:53PM +0100, Daniel Vetter wrote:
> On Thu, Mar 05, 2015 at 02:51:28PM +0530, Sonika Jindal wrote:
> > @@ -1519,16 +1550,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> >  		goto out;
> >  	}
> >  
> > -	if (!dev->mode_config.rotation_property)
> > -		dev->mode_config.rotation_property =
> > -			drm_mode_create_rotation_property(dev,
> > -							  BIT(DRM_ROTATE_0) |
> > -							  BIT(DRM_ROTATE_180));
> > -
> > -	if (dev->mode_config.rotation_property)
> > -		drm_object_attach_property(&intel_plane->base.base,
> > -					   dev->mode_config.rotation_property,
> > -					   state->base.rotation);
> > +	intel_create_rotation_property(dev, intel_plane);
> 
> I think back from the original rotation work we've had the leftover task
> to move this into common code so that we do create the property just once
> without this check.
> 
> I think this should be done now.

Someone should also make it so we can again have different supported
rotation bits on different planes. I'll have need for it on CHV I think.

> -Daniel
> >  
> >  	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
> >  
> > -- 
> > 1.7.10.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/skl: Support for 90/270 rotation

[Daniel Vetter]

On Thu, Mar 05, 2015 at 03:08:17PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 05, 2015 at 01:56:53PM +0100, Daniel Vetter wrote:
> > On Thu, Mar 05, 2015 at 02:51:28PM +0530, Sonika Jindal wrote:
> > > @@ -1519,16 +1550,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> > >  		goto out;
> > >  	}
> > >  
> > > -	if (!dev->mode_config.rotation_property)
> > > -		dev->mode_config.rotation_property =
> > > -			drm_mode_create_rotation_property(dev,
> > > -							  BIT(DRM_ROTATE_0) |
> > > -							  BIT(DRM_ROTATE_180));
> > > -
> > > -	if (dev->mode_config.rotation_property)
> > > -		drm_object_attach_property(&intel_plane->base.base,
> > > -					   dev->mode_config.rotation_property,
> > > -					   state->base.rotation);
> > > +	intel_create_rotation_property(dev, intel_plane);
> > 
> > I think back from the original rotation work we've had the leftover task
> > to move this into common code so that we do create the property just once
> > without this check.
> > 
> > I think this should be done now.
> 
> Someone should also make it so we can again have different supported
> rotation bits on different planes. I'll have need for it on CHV I think.

plane->atomic_check just needs to reject them. Tbh I'm not sold on the
value of trying to tell userspace which rotation works and which doesnt -
generic userspace won't learn about y-tiling requirements either so this
feels a bit pointless tbh. And rejecting stuff in atomic_check is what
it's for.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/skl: Support for 90/270 rotation

[Ville Syrjälä]

On Thu, Mar 05, 2015 at 04:29:30PM +0100, Daniel Vetter wrote:
> On Thu, Mar 05, 2015 at 03:08:17PM +0200, Ville Syrjälä wrote:
> > On Thu, Mar 05, 2015 at 01:56:53PM +0100, Daniel Vetter wrote:
> > > On Thu, Mar 05, 2015 at 02:51:28PM +0530, Sonika Jindal wrote:
> > > > @@ -1519,16 +1550,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> > > >  		goto out;
> > > >  	}
> > > >  
> > > > -	if (!dev->mode_config.rotation_property)
> > > > -		dev->mode_config.rotation_property =
> > > > -			drm_mode_create_rotation_property(dev,
> > > > -							  BIT(DRM_ROTATE_0) |
> > > > -							  BIT(DRM_ROTATE_180));
> > > > -
> > > > -	if (dev->mode_config.rotation_property)
> > > > -		drm_object_attach_property(&intel_plane->base.base,
> > > > -					   dev->mode_config.rotation_property,
> > > > -					   state->base.rotation);
> > > > +	intel_create_rotation_property(dev, intel_plane);
> > > 
> > > I think back from the original rotation work we've had the leftover task
> > > to move this into common code so that we do create the property just once
> > > without this check.
> > > 
> > > I think this should be done now.
> > 
> > Someone should also make it so we can again have different supported
> > rotation bits on different planes. I'll have need for it on CHV I think.
> 
> plane->atomic_check just needs to reject them. Tbh I'm not sold on the
> value of trying to tell userspace which rotation works and which doesnt -
> generic userspace won't learn about y-tiling requirements either so this
> feels a bit pointless tbh. And rejecting stuff in atomic_check is what
> it's for.

By that logic we shouldn't expose pixel formats or any other useful
infromation either then.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/skl: Support for 90/270 rotation

[Daniel Vetter]

On Thu, Mar 05, 2015 at 05:56:23PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 05, 2015 at 04:29:30PM +0100, Daniel Vetter wrote:
> > On Thu, Mar 05, 2015 at 03:08:17PM +0200, Ville Syrjälä wrote:
> > > On Thu, Mar 05, 2015 at 01:56:53PM +0100, Daniel Vetter wrote:
> > > > On Thu, Mar 05, 2015 at 02:51:28PM +0530, Sonika Jindal wrote:
> > > > > @@ -1519,16 +1550,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> > > > >  		goto out;
> > > > >  	}
> > > > >  
> > > > > -	if (!dev->mode_config.rotation_property)
> > > > > -		dev->mode_config.rotation_property =
> > > > > -			drm_mode_create_rotation_property(dev,
> > > > > -							  BIT(DRM_ROTATE_0) |
> > > > > -							  BIT(DRM_ROTATE_180));
> > > > > -
> > > > > -	if (dev->mode_config.rotation_property)
> > > > > -		drm_object_attach_property(&intel_plane->base.base,
> > > > > -					   dev->mode_config.rotation_property,
> > > > > -					   state->base.rotation);
> > > > > +	intel_create_rotation_property(dev, intel_plane);
> > > > 
> > > > I think back from the original rotation work we've had the leftover task
> > > > to move this into common code so that we do create the property just once
> > > > without this check.
> > > > 
> > > > I think this should be done now.
> > > 
> > > Someone should also make it so we can again have different supported
> > > rotation bits on different planes. I'll have need for it on CHV I think.
> > 
> > plane->atomic_check just needs to reject them. Tbh I'm not sold on the
> > value of trying to tell userspace which rotation works and which doesnt -
> > generic userspace won't learn about y-tiling requirements either so this
> > feels a bit pointless tbh. And rejecting stuff in atomic_check is what
> > it's for.
> 
> By that logic we shouldn't expose pixel formats or any other useful
> infromation either then.

Well to be able to fix this we need to restrict the value-set of
properties per-attachment. Since I very much want core atomic to decodd
standardized properties, and if we create rotation per-plane then that's
going to be fairly painful.

That's quite a bit of work, and I'm not sure it's all that useful. And
yeah that argument does extend somewhat to pixel formats too since without
clueful userspace you can't really allocate a suitable buffer with just
the list of pixel formats. They are useful though for a bit of shared
input validation in the core (since most planes don't change the support
pixel formats at runtime).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/skl: Support for 90/270 rotation

[Ville Syrjälä]

On Fri, Mar 06, 2015 at 06:03:31PM +0100, Daniel Vetter wrote:
> On Thu, Mar 05, 2015 at 05:56:23PM +0200, Ville Syrjälä wrote:
> > On Thu, Mar 05, 2015 at 04:29:30PM +0100, Daniel Vetter wrote:
> > > On Thu, Mar 05, 2015 at 03:08:17PM +0200, Ville Syrjälä wrote:
> > > > On Thu, Mar 05, 2015 at 01:56:53PM +0100, Daniel Vetter wrote:
> > > > > On Thu, Mar 05, 2015 at 02:51:28PM +0530, Sonika Jindal wrote:
> > > > > > @@ -1519,16 +1550,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> > > > > >  		goto out;
> > > > > >  	}
> > > > > >  
> > > > > > -	if (!dev->mode_config.rotation_property)
> > > > > > -		dev->mode_config.rotation_property =
> > > > > > -			drm_mode_create_rotation_property(dev,
> > > > > > -							  BIT(DRM_ROTATE_0) |
> > > > > > -							  BIT(DRM_ROTATE_180));
> > > > > > -
> > > > > > -	if (dev->mode_config.rotation_property)
> > > > > > -		drm_object_attach_property(&intel_plane->base.base,
> > > > > > -					   dev->mode_config.rotation_property,
> > > > > > -					   state->base.rotation);
> > > > > > +	intel_create_rotation_property(dev, intel_plane);
> > > > > 
> > > > > I think back from the original rotation work we've had the leftover task
> > > > > to move this into common code so that we do create the property just once
> > > > > without this check.
> > > > > 
> > > > > I think this should be done now.
> > > > 
> > > > Someone should also make it so we can again have different supported
> > > > rotation bits on different planes. I'll have need for it on CHV I think.
> > > 
> > > plane->atomic_check just needs to reject them. Tbh I'm not sold on the
> > > value of trying to tell userspace which rotation works and which doesnt -
> > > generic userspace won't learn about y-tiling requirements either so this
> > > feels a bit pointless tbh. And rejecting stuff in atomic_check is what
> > > it's for.
> > 
> > By that logic we shouldn't expose pixel formats or any other useful
> > infromation either then.
> 
> Well to be able to fix this we need to restrict the value-set of
> properties per-attachment. Since I very much want core atomic to decodd
> standardized properties, and if we create rotation per-plane then that's
> going to be fairly painful.

AFAICS it should be a simple matter of
s/config->rotation_property/plane->rotation_property/
Well, unless you want to go optimize things so that we don't create multiple
properties with the same supported bitmask.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/skl: Support for 90/270 rotation

[Daniel Vetter]

On Fri, Mar 06, 2015 at 07:22:46PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 06, 2015 at 06:03:31PM +0100, Daniel Vetter wrote:
> > On Thu, Mar 05, 2015 at 05:56:23PM +0200, Ville Syrjälä wrote:
> > > On Thu, Mar 05, 2015 at 04:29:30PM +0100, Daniel Vetter wrote:
> > > > On Thu, Mar 05, 2015 at 03:08:17PM +0200, Ville Syrjälä wrote:
> > > > > On Thu, Mar 05, 2015 at 01:56:53PM +0100, Daniel Vetter wrote:
> > > > > > On Thu, Mar 05, 2015 at 02:51:28PM +0530, Sonika Jindal wrote:
> > > > > > > @@ -1519,16 +1550,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> > > > > > >  		goto out;
> > > > > > >  	}
> > > > > > >  
> > > > > > > -	if (!dev->mode_config.rotation_property)
> > > > > > > -		dev->mode_config.rotation_property =
> > > > > > > -			drm_mode_create_rotation_property(dev,
> > > > > > > -							  BIT(DRM_ROTATE_0) |
> > > > > > > -							  BIT(DRM_ROTATE_180));
> > > > > > > -
> > > > > > > -	if (dev->mode_config.rotation_property)
> > > > > > > -		drm_object_attach_property(&intel_plane->base.base,
> > > > > > > -					   dev->mode_config.rotation_property,
> > > > > > > -					   state->base.rotation);
> > > > > > > +	intel_create_rotation_property(dev, intel_plane);
> > > > > > 
> > > > > > I think back from the original rotation work we've had the leftover task
> > > > > > to move this into common code so that we do create the property just once
> > > > > > without this check.
> > > > > > 
> > > > > > I think this should be done now.
> > > > > 
> > > > > Someone should also make it so we can again have different supported
> > > > > rotation bits on different planes. I'll have need for it on CHV I think.
> > > > 
> > > > plane->atomic_check just needs to reject them. Tbh I'm not sold on the
> > > > value of trying to tell userspace which rotation works and which doesnt -
> > > > generic userspace won't learn about y-tiling requirements either so this
> > > > feels a bit pointless tbh. And rejecting stuff in atomic_check is what
> > > > it's for.
> > > 
> > > By that logic we shouldn't expose pixel formats or any other useful
> > > infromation either then.
> > 
> > Well to be able to fix this we need to restrict the value-set of
> > properties per-attachment. Since I very much want core atomic to decodd
> > standardized properties, and if we create rotation per-plane then that's
> > going to be fairly painful.
> 
> AFAICS it should be a simple matter of
> s/config->rotation_property/plane->rotation_property/
> Well, unless you want to go optimize things so that we don't create multiple
> properties with the same supported bitmask.

Hm yeah that'd work too. I guess I've been a bit dense today, time for w/e
;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/skl: Allow universal planes to position

[Daniel Vetter]

On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote:
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

Imo this needs a little more commit message, and more important it needs
igt test coverage. Best approach there is probably to take the plane test
we have already and extend it to the primary plane.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 437a679..e1b0c4d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12183,16 +12183,21 @@ intel_check_primary_plane(struct drm_plane *plane,
>  	struct drm_rect *dest = &state->dst;
>  	struct drm_rect *src = &state->src;
>  	const struct drm_rect *clip = &state->clip;
> +	bool can_position = false;
>  	int ret;
>  
>  	crtc = crtc ? crtc : plane->crtc;
>  	intel_crtc = to_intel_crtc(crtc);
>  
> +	if (INTEL_INFO(dev)->gen >= 9)
> +		can_position = true;
> +
>  	ret = drm_plane_helper_check_update(plane, crtc, fb,
>  					    src, dest, clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
> -					    false, true, &state->visible);
> +					    can_position, true,
> +					    &state->visible);
>  	if (ret)
>  		return ret;
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/skl: Allow universal planes to position

[sonika]

On Thursday 05 March 2015 06:24 PM, Daniel Vetter wrote:
> On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote:
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> Imo this needs a little more commit message, and more important it needs
> igt test coverage. Best approach there is probably to take the plane test
> we have already and extend it to the primary plane.
> -Daniel
This is just to take care of the case when the size of the fb is smaller 
than the crtc.
I have extended the rotation test (yet to be posted), to create a 
smaller primary plane fb to be used for 90/270 rotation.

Since we still set position to 0 for primary plane, I did not add any 
test case for positioning of primary plane.
That can be added as a separate activity when positioning support is added.
Right now this is just to allow smaller fb for primary plane which is 
possible with universal planes gen >=9.

Regards,
Sonika
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |    7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 437a679..e1b0c4d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12183,16 +12183,21 @@ intel_check_primary_plane(struct drm_plane *plane,
>>   	struct drm_rect *dest = &state->dst;
>>   	struct drm_rect *src = &state->src;
>>   	const struct drm_rect *clip = &state->clip;
>> +	bool can_position = false;
>>   	int ret;
>>   
>>   	crtc = crtc ? crtc : plane->crtc;
>>   	intel_crtc = to_intel_crtc(crtc);
>>   
>> +	if (INTEL_INFO(dev)->gen >= 9)
>> +		can_position = true;
>> +
>>   	ret = drm_plane_helper_check_update(plane, crtc, fb,
>>   					    src, dest, clip,
>>   					    DRM_PLANE_HELPER_NO_SCALING,
>>   					    DRM_PLANE_HELPER_NO_SCALING,
>> -					    false, true, &state->visible);
>> +					    can_position, true,
>> +					    &state->visible);
>>   	if (ret)
>>   		return ret;
>>   
>> -- 
>> 1.7.10.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/skl: Allow universal planes to position

[Daniel Vetter]

On Mon, Mar 09, 2015 at 08:33:27AM +0530, sonika wrote:
> 
> On Thursday 05 March 2015 06:24 PM, Daniel Vetter wrote:
> >On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote:
> >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >Imo this needs a little more commit message, and more important it needs
> >igt test coverage. Best approach there is probably to take the plane test
> >we have already and extend it to the primary plane.
> >-Daniel
> This is just to take care of the case when the size of the fb is smaller
> than the crtc.
> I have extended the rotation test (yet to be posted), to create a smaller
> primary plane fb to be used for 90/270 rotation.
> 
> Since we still set position to 0 for primary plane, I did not add any test
> case for positioning of primary plane.
> That can be added as a separate activity when positioning support is added.
> Right now this is just to allow smaller fb for primary plane which is
> possible with universal planes gen >=9.

Through universal planes it's already possible to position any plane
anywhere, and this code is all that makes sure this doesn't happen for the
primary plane. Since you've just changed that I think this needs a
testcase in igt.

Or maybe I missed something and it's indeed not yet possible to do this?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/skl: Allow universal planes to position

[Ville Syrjälä]

On Mon, Mar 09, 2015 at 09:46:27AM +0100, Daniel Vetter wrote:
> On Mon, Mar 09, 2015 at 08:33:27AM +0530, sonika wrote:
> > 
> > On Thursday 05 March 2015 06:24 PM, Daniel Vetter wrote:
> > >On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote:
> > >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> > >Imo this needs a little more commit message, and more important it needs
> > >igt test coverage. Best approach there is probably to take the plane test
> > >we have already and extend it to the primary plane.
> > >-Daniel
> > This is just to take care of the case when the size of the fb is smaller
> > than the crtc.
> > I have extended the rotation test (yet to be posted), to create a smaller
> > primary plane fb to be used for 90/270 rotation.
> > 
> > Since we still set position to 0 for primary plane, I did not add any test
> > case for positioning of primary plane.
> > That can be added as a separate activity when positioning support is added.
> > Right now this is just to allow smaller fb for primary plane which is
> > possible with universal planes gen >=9.
> 
> Through universal planes it's already possible to position any plane
> anywhere, and this code is all that makes sure this doesn't happen for the
> primary plane. Since you've just changed that I think this needs a
> testcase in igt.
> 
> Or maybe I missed something and it's indeed not yet possible to do this?

We're not ready to enable this just yet. First we need to change all the
plane code to use the correct derived state (eg. clipped coordinates as
opposed to the user requested ones). And to do that we must make sure
the derived state is up to date at .crtc_enable() time.

This was one of the things I had on mind when Gustavo was reworking the
primary/cursor plane code, but then Gustavo was pulled away and this
task fell through the cracks. I've now cooked up something that should
make this happen, but I haven't tested it yet. I'll give it a spin today
and post the patch if all goes well...

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/skl: Allow universal planes to position

[Daniel Vetter]

On Mon, Mar 09, 2015 at 05:20:39PM +0200, Ville Syrjälä wrote:
> On Mon, Mar 09, 2015 at 09:46:27AM +0100, Daniel Vetter wrote:
> > On Mon, Mar 09, 2015 at 08:33:27AM +0530, sonika wrote:
> > > 
> > > On Thursday 05 March 2015 06:24 PM, Daniel Vetter wrote:
> > > >On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote:
> > > >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> > > >Imo this needs a little more commit message, and more important it needs
> > > >igt test coverage. Best approach there is probably to take the plane test
> > > >we have already and extend it to the primary plane.
> > > >-Daniel
> > > This is just to take care of the case when the size of the fb is smaller
> > > than the crtc.
> > > I have extended the rotation test (yet to be posted), to create a smaller
> > > primary plane fb to be used for 90/270 rotation.
> > > 
> > > Since we still set position to 0 for primary plane, I did not add any test
> > > case for positioning of primary plane.
> > > That can be added as a separate activity when positioning support is added.
> > > Right now this is just to allow smaller fb for primary plane which is
> > > possible with universal planes gen >=9.
> > 
> > Through universal planes it's already possible to position any plane
> > anywhere, and this code is all that makes sure this doesn't happen for the
> > primary plane. Since you've just changed that I think this needs a
> > testcase in igt.
> > 
> > Or maybe I missed something and it's indeed not yet possible to do this?
> 
> We're not ready to enable this just yet. First we need to change all the
> plane code to use the correct derived state (eg. clipped coordinates as
> opposed to the user requested ones). And to do that we must make sure
> the derived state is up to date at .crtc_enable() time.
> 
> This was one of the things I had on mind when Gustavo was reworking the
> primary/cursor plane code, but then Gustavo was pulled away and this
> task fell through the cracks. I've now cooked up something that should
> make this happen, but I haven't tested it yet. I'll give it a spin today
> and post the patch if all goes well...

Hm, is there a lot of derived state around (restricting just to skl+) once
we have the wm code converted over to always look at plane_state structs
instead of the add-hoc wm struct we currently use?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/skl: Allow universal planes to position

[sonika]

On Monday 09 March 2015 02:16 PM, Daniel Vetter wrote:
> On Mon, Mar 09, 2015 at 08:33:27AM +0530, sonika wrote:
>> On Thursday 05 March 2015 06:24 PM, Daniel Vetter wrote:
>>> On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote:
>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>> Imo this needs a little more commit message, and more important it needs
>>> igt test coverage. Best approach there is probably to take the plane test
>>> we have already and extend it to the primary plane.
>>> -Daniel
>> This is just to take care of the case when the size of the fb is smaller
>> than the crtc.
>> I have extended the rotation test (yet to be posted), to create a smaller
>> primary plane fb to be used for 90/270 rotation.
>>
>> Since we still set position to 0 for primary plane, I did not add any test
>> case for positioning of primary plane.
>> That can be added as a separate activity when positioning support is added.
>> Right now this is just to allow smaller fb for primary plane which is
>> possible with universal planes gen >=9.
> Through universal planes it's already possible to position any plane
> anywhere, and this code is all that makes sure this doesn't happen for the
> primary plane. Since you've just changed that I think this needs a
> testcase in igt.
>
> Or maybe I missed something and it's indeed not yet possible to do this?
> -Daniel
Yes, it isn't possible yet. We set the position to 0 in 
skylake_update_primary_plane.
-Sonika

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/skl: Allow universal planes to position

[Daniel Vetter]

On Tue, Mar 10, 2015 at 09:23:54AM +0530, sonika wrote:
> 
> On Monday 09 March 2015 02:16 PM, Daniel Vetter wrote:
> >On Mon, Mar 09, 2015 at 08:33:27AM +0530, sonika wrote:
> >>On Thursday 05 March 2015 06:24 PM, Daniel Vetter wrote:
> >>>On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote:
> >>>>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >>>Imo this needs a little more commit message, and more important it needs
> >>>igt test coverage. Best approach there is probably to take the plane test
> >>>we have already and extend it to the primary plane.
> >>>-Daniel
> >>This is just to take care of the case when the size of the fb is smaller
> >>than the crtc.
> >>I have extended the rotation test (yet to be posted), to create a smaller
> >>primary plane fb to be used for 90/270 rotation.
> >>
> >>Since we still set position to 0 for primary plane, I did not add any test
> >>case for positioning of primary plane.
> >>That can be added as a separate activity when positioning support is added.
> >>Right now this is just to allow smaller fb for primary plane which is
> >>possible with universal planes gen >=9.
> >Through universal planes it's already possible to position any plane
> >anywhere, and this code is all that makes sure this doesn't happen for the
> >primary plane. Since you've just changed that I think this needs a
> >testcase in igt.
> >
> >Or maybe I missed something and it's indeed not yet possible to do this?
> >-Daniel
> Yes, it isn't possible yet. We set the position to 0 in
> skylake_update_primary_plane.

Hm indeed. But with your patch we no longer reject plane updates for the
primary plane which aren't positioned at 0,0. So there's now a bug. I
guess either go the full way towards implementing universal plane support
on skl (and fix up the hw programming) or we need to wait with this patch
a bit more.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx