diff mbox

[5/9] drm/i915/skl: Add support for blending modes

Message ID 1453130143-7228-6-git-send-email-vandita.kulkarni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kulkarni, Vandita Jan. 18, 2016, 3:15 p.m. UTC
From: Damien Lespiau <damien.lespiau@intel.com>

This patch adds support for blending modes involving
color.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_sprite.c  | 12 +++++++-----
 3 files changed, 36 insertions(+), 5 deletions(-)

Comments

Matt Roper Jan. 21, 2016, 10:33 p.m. UTC | #1
On Mon, Jan 18, 2016 at 08:45:39PM +0530, Vandita Kulkarni wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> This patch adds support for blending modes involving
> color.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_sprite.c  | 12 +++++++-----
>  3 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7e59a49..39e9db4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11901,6 +11901,7 @@ static int intel_plane_state_check_blend(struct drm_plane_state *plane_state)
>  
>  	state->premultiplied_alpha = false;
>  	state->drop_alpha = false;
> +	state->use_plane_alpha = false;
>  
>  	switch (mode->func) {
>  	/*
> @@ -11929,6 +11930,27 @@ static int intel_plane_state_check_blend(struct drm_plane_state *plane_state)
>  		if (!has_per_pixel_blending)
>  			return -EINVAL;
>  		break;
> +	/* plane alpha */
> +	case DRM_BLEND_FUNC(CONSTANT_ALPHA, ONE_MINUS_CONSTANT_ALPHA):
> +		if (has_per_pixel_blending)
> +			state->drop_alpha = true;
> +		state->use_plane_alpha = true;
> +		break;
> +	/* plane alpha, pre-multiplied fb */
> +	case DRM_BLEND_FUNC(CONSTANT_ALPHA,
> +			    ONE_MINUS_CONSTANT_ALPHA_TIMES_SRC_ALPHA):
> +		if (!has_per_pixel_blending)
> +			return -EINVAL;
> +		state->premultiplied_alpha = true;
> +		state->use_plane_alpha = true;
> +		break;
> +	/* plane alpha, non pre-multiplied fb */
> +	case DRM_BLEND_FUNC(CONSTANT_ALPHA_TIMES_SRC_ALPHA,
> +			    ONE_MINUS_CONSTANT_ALPHA_TIMES_SRC_ALPHA):
> +		if (!has_per_pixel_blending)
> +			return -EINVAL;
> +		state->use_plane_alpha = true;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -14272,6 +14294,12 @@ void intel_plane_add_blend_properties(struct intel_plane *plane)
>  	if (prop)
>  		drm_object_attach_property(&plane->base.base, prop,
>  					   DRM_BLEND_FUNC(AUTO, AUTO));
> +
> +	prop = dev->mode_config.prop_blend_color;
> +	if (prop)
> +		drm_object_attach_property(&plane->base.base, prop,
> +					   DRM_MODE_COLOR(0xffff, 0xffff,
> +							  0xffff, 0xffff));
>  }
>  
>  static int
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f99e1d9..8226c8e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -299,6 +299,7 @@ struct intel_plane_state {
>  	 */
>  	bool premultiplied_alpha;	/* is the fb pre-multiplied? */
>  	bool drop_alpha;		/* drop the fb alpha channel */
> +	bool use_plane_alpha;		/* use per-plane alpha */
>  
>  	struct drm_intel_sprite_colorkey ckey;
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 9538658..9175152 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -227,12 +227,14 @@ skl_update_plane(struct drm_plane *drm_plane,
>  	crtc_w--;
>  	crtc_h--;
>  
> -	if (key->flags) {
> +	I915_WRITE(PLANE_KEYMAX(pipe, plane),
> +		   (DRM_MODE_COLOR_ALPHA_8(plane_state->base.blend_mode.color)
> +			<< 24) | (key->max_value & 0x00ffffff));
> +	I915_WRITE(PLANE_KEYMSK(pipe, plane),
> +		   (plane_state->use_plane_alpha << 31) |
> +		   (key->channel_mask & GENMASK(0, 26)));

I'm having trouble reconciling the bspec description of these two
registers.  The bspec says that PLANE_KEYMAX is only used for YUV
framebuffers and that the register is unused with RGB buffers.  However
the description of PLANE_KEYMSK's bit 31 says it uses the top byte of
PLANE_KEYMAX as the constant alpha value (which seems to apply to all
buffer formats).

Is this a bug (or poor wording) in the bspec or am I completely
misinterpreting?  Assuming the hardware works the way your code is
written here, it seems like the bspec description of PLANE_KEYMAX should
actually say that bits 0-23 are exclusive to YUV buffers but that 24-31
are usable on any buffer format?

> +	if (key->flags)
>  		I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);

Does it matter that we're writing possible garbage bits from
key->max_value and key->channel_mask unconditionally now?  Previously
they were guarded by key->flags.


Matt

> -		I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value);
> -		I915_WRITE(PLANE_KEYMSK(pipe, plane), key->channel_mask);
> -	}
> -
>  	if (key->flags & I915_SET_COLORKEY_DESTINATION)
>  		plane_ctl |= PLANE_CTL_KEY_ENABLE_DESTINATION;
>  	else if (key->flags & I915_SET_COLORKEY_SOURCE)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7e59a49..39e9db4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11901,6 +11901,7 @@  static int intel_plane_state_check_blend(struct drm_plane_state *plane_state)
 
 	state->premultiplied_alpha = false;
 	state->drop_alpha = false;
+	state->use_plane_alpha = false;
 
 	switch (mode->func) {
 	/*
@@ -11929,6 +11930,27 @@  static int intel_plane_state_check_blend(struct drm_plane_state *plane_state)
 		if (!has_per_pixel_blending)
 			return -EINVAL;
 		break;
+	/* plane alpha */
+	case DRM_BLEND_FUNC(CONSTANT_ALPHA, ONE_MINUS_CONSTANT_ALPHA):
+		if (has_per_pixel_blending)
+			state->drop_alpha = true;
+		state->use_plane_alpha = true;
+		break;
+	/* plane alpha, pre-multiplied fb */
+	case DRM_BLEND_FUNC(CONSTANT_ALPHA,
+			    ONE_MINUS_CONSTANT_ALPHA_TIMES_SRC_ALPHA):
+		if (!has_per_pixel_blending)
+			return -EINVAL;
+		state->premultiplied_alpha = true;
+		state->use_plane_alpha = true;
+		break;
+	/* plane alpha, non pre-multiplied fb */
+	case DRM_BLEND_FUNC(CONSTANT_ALPHA_TIMES_SRC_ALPHA,
+			    ONE_MINUS_CONSTANT_ALPHA_TIMES_SRC_ALPHA):
+		if (!has_per_pixel_blending)
+			return -EINVAL;
+		state->use_plane_alpha = true;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -14272,6 +14294,12 @@  void intel_plane_add_blend_properties(struct intel_plane *plane)
 	if (prop)
 		drm_object_attach_property(&plane->base.base, prop,
 					   DRM_BLEND_FUNC(AUTO, AUTO));
+
+	prop = dev->mode_config.prop_blend_color;
+	if (prop)
+		drm_object_attach_property(&plane->base.base, prop,
+					   DRM_MODE_COLOR(0xffff, 0xffff,
+							  0xffff, 0xffff));
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f99e1d9..8226c8e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -299,6 +299,7 @@  struct intel_plane_state {
 	 */
 	bool premultiplied_alpha;	/* is the fb pre-multiplied? */
 	bool drop_alpha;		/* drop the fb alpha channel */
+	bool use_plane_alpha;		/* use per-plane alpha */
 
 	struct drm_intel_sprite_colorkey ckey;
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9538658..9175152 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -227,12 +227,14 @@  skl_update_plane(struct drm_plane *drm_plane,
 	crtc_w--;
 	crtc_h--;
 
-	if (key->flags) {
+	I915_WRITE(PLANE_KEYMAX(pipe, plane),
+		   (DRM_MODE_COLOR_ALPHA_8(plane_state->base.blend_mode.color)
+			<< 24) | (key->max_value & 0x00ffffff));
+	I915_WRITE(PLANE_KEYMSK(pipe, plane),
+		   (plane_state->use_plane_alpha << 31) |
+		   (key->channel_mask & GENMASK(0, 26)));
+	if (key->flags)
 		I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
-		I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value);
-		I915_WRITE(PLANE_KEYMSK(pipe, plane), key->channel_mask);
-	}
-
 	if (key->flags & I915_SET_COLORKEY_DESTINATION)
 		plane_ctl |= PLANE_CTL_KEY_ENABLE_DESTINATION;
 	else if (key->flags & I915_SET_COLORKEY_SOURCE)