diff mbox

[02/14] drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes

Message ID 20170317211808.14693-3-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä March 17, 2017, 9:17 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On SKL the planes are uniform so the "sprites" can use the
primary plane code perfectly fine. The only difference we
have is the color key handling, but since we never enable that
for the primary plane the same code works just fine.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 +++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h     |  5 ++---
 drivers/gpu/drm/i915/intel_sprite.c  | 18 +-----------------
 3 files changed, 14 insertions(+), 25 deletions(-)

Comments

Chris Wilson March 17, 2017, 9:48 p.m. UTC | #1 
On Fri, Mar 17, 2017 at 11:17:56PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On SKL the planes are uniform so the "sprites" can use the
> primary plane code perfectly fine. The only difference we
> have is the color key handling, but since we never enable that
> for the primary plane the same code works just fine.

Rationale works for me. Does setting the color key report an -EINVAL on
the primary? Even via the legacy setcolorkey ioctl?
-Chris
Ville Syrjälä March 20, 2017, 4:09 p.m. UTC | #2 
On Fri, Mar 17, 2017 at 09:48:26PM +0000, Chris Wilson wrote:
> On Fri, Mar 17, 2017 at 11:17:56PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On SKL the planes are uniform so the "sprites" can use the
> > primary plane code perfectly fine. The only difference we
> > have is the color key handling, but since we never enable that
> > for the primary plane the same code works just fine.
> 
> Rationale works for me. Does setting the color key report an -EINVAL on
> the primary?

Yes.

> Even via the legacy setcolorkey ioctl?

That's actually the only way to set it. We never got around to defining
a property for this stuff.
Chris Wilson March 20, 2017, 4:16 p.m. UTC | #3 
On Mon, Mar 20, 2017 at 06:09:44PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 17, 2017 at 09:48:26PM +0000, Chris Wilson wrote:
> > On Fri, Mar 17, 2017 at 11:17:56PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > On SKL the planes are uniform so the "sprites" can use the
> > > primary plane code perfectly fine. The only difference we
> > > have is the color key handling, but since we never enable that
> > > for the primary plane the same code works just fine.
> > 
> > Rationale works for me. Does setting the color key report an -EINVAL on
> > the primary?
> 
> Yes.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6ddf5c90d15c..8a9f1bd21e0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3254,7 +3254,7 @@  u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
 	return stride;
 }
 
-u32 skl_plane_ctl_format(uint32_t pixel_format)
+static u32 skl_plane_ctl_format(uint32_t pixel_format)
 {
 	switch (pixel_format) {
 	case DRM_FORMAT_C8:
@@ -3295,7 +3295,7 @@  u32 skl_plane_ctl_format(uint32_t pixel_format)
 	return 0;
 }
 
-u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
+static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
 {
 	switch (fb_modifier) {
 	case DRM_FORMAT_MOD_NONE:
@@ -3313,7 +3313,7 @@  u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
 	return 0;
 }
 
-u32 skl_plane_ctl_rotation(unsigned int rotation)
+static u32 skl_plane_ctl_rotation(unsigned int rotation)
 {
 	switch (rotation) {
 	case DRM_ROTATE_0:
@@ -3335,13 +3335,14 @@  u32 skl_plane_ctl_rotation(unsigned int rotation)
 	return 0;
 }
 
-static u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
-			 const struct intel_plane_state *plane_state)
+u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
+		  const struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(plane_state->base.plane->dev);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 plane_ctl;
 
 	plane_ctl = PLANE_CTL_ENABLE;
@@ -3357,6 +3358,11 @@  static u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
 	plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
 	plane_ctl |= skl_plane_ctl_rotation(rotation);
 
+	if (key->flags & I915_SET_COLORKEY_DESTINATION)
+		plane_ctl |= PLANE_CTL_KEY_ENABLE_DESTINATION;
+	else if (key->flags & I915_SET_COLORKEY_SOURCE)
+		plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
+
 	return plane_ctl;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 51228fe4283b..45e55b45e772 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1445,9 +1445,8 @@  static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
 	return i915_ggtt_offset(state->vma);
 }
 
-u32 skl_plane_ctl_format(uint32_t pixel_format);
-u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
-u32 skl_plane_ctl_rotation(unsigned int rotation);
+u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
+		  const struct intel_plane_state *plane_state);
 u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
 		     unsigned int rotation);
 int skl_check_plane_surface(struct intel_plane_state *plane_state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index b931d0bd7a64..455a9ed44204 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -232,23 +232,7 @@  skl_update_plane(struct drm_plane *drm_plane,
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
 
-	plane_ctl = PLANE_CTL_ENABLE;
-
-	if (!IS_GEMINILAKE(dev_priv)) {
-		plane_ctl |=
-			PLANE_CTL_PIPE_GAMMA_ENABLE |
-			PLANE_CTL_PIPE_CSC_ENABLE |
-			PLANE_CTL_PLANE_GAMMA_DISABLE;
-	}
-
-	plane_ctl |= skl_plane_ctl_format(fb->format->format);
-	plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
-	plane_ctl |= skl_plane_ctl_rotation(rotation);
-
-	if (key->flags & I915_SET_COLORKEY_DESTINATION)
-		plane_ctl |= PLANE_CTL_KEY_ENABLE_DESTINATION;
-	else if (key->flags & I915_SET_COLORKEY_SOURCE)
-		plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
+	plane_ctl = skl_plane_ctl(crtc_state, plane_state);
 
 	/* Sizes are 0 based */
 	src_w--;