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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/1/2015 11:52 PM, Matt Roper wrote:

On Mon, Mar 30, 2015 at 02:04:57PM +0530, Sonika Jindal wrote:

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.
v6: Rebased on -nightly (Tvrtko's series merged)

Signed-off-by: Sonika Jindal <sonika.jindal@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_reg.h      |    2 +
  drivers/gpu/drm/i915/intel_display.c |  103 +++++++++++++++++++++++++++-------
  drivers/gpu/drm/i915/intel_drv.h     |    6 ++
  drivers/gpu/drm/i915/intel_sprite.c  |   52 ++++++++++++-----
  4 files changed, 129 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..564bbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4854,7 +4854,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 f0bbc22..86ee0f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2318,6 +2318,28 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
  		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;
+	}


Shouldn't we be matching against the list of formats the plane supports
(which may vary by platform, or by specific plane) rather than this
generic list?  We already specified what formats the plane can handle at
plane init time, so it seems like what you'd really want is just a call
to

     drm_plane_check_pixel_format(plane_state->plane, fb->pixel_format)

then follow that up with explicit checks to exclude any formats that we
can handle in 0/180, but not in 90/270.
I am not sure how it will help. drm_plane_check_pixel_format should be used to check the pixel format of the fb which we should be doing in some -check functions (I don't think we do that right now?) against what is supported by the plane. But to check for the formats which are allowed for 90/270, we would need this kind of explicit check. 


I'd also move this check to intel_plane_atomic_check(), since the
'check' code path is where I'd usually go looking for these types of
checks; the function you've got it in at the moment gets called from the
'prepare' step which works as well, but seems a bit less obvious.
Yes, I agree, but this is on top of Tvrtko's patch for secondary buffer mapping where based upon tiling and pixel format we are allowing the rotated gtt. 

Tvrtko,
Can these be moved to the intel_plane_atomic_check()


+
  	return 0;
  }

@@ -2919,8 +2941,12 @@ 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;

  	if (!intel_crtc->primary_enabled) {
  		I915_WRITE(PLANE_CTL(pipe, 0), 0);
@@ -2981,21 +3007,51 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
  	}

  	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
-	if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180))
+
+	plane = crtc->primary;
+	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],
  					       fb->pixel_format);


Minor nit; can we move this down inside the 'else' branch to make it
apparent how this serves a parallel role to 'tile_height' from the 'if'
branch.


Yes sure.



-	surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj);
+	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj);
+
+	if (rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {


Might as well use the intel_rotation_90_or_270() function since we have
it.  Same comment for the sprite path farther down.


Sure.

Regards,
Sonika

+		/* 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;

  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
  	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));
-	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));
@@ -12406,23 +12462,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 811a1db..d32025a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)
  	return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));
  }

+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 f41e872..65eb147 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
  	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);
  	const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;
  	unsigned long surf_addr;
+	u32 tile_height, plane_offset, plane_size;
+	unsigned int rotation;
+	int x_offset, y_offset;

  	plane_ctl = PLANE_CTL_ENABLE |
  		PLANE_CTL_PIPE_CSC_ENABLE;
@@ -254,8 +257,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;
+	}

  	intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h,
  				       pixel_size, true,
@@ -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 + 1);
+		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));
@@ -1310,16 +1341,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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx



_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux