diff mbox series

[04/14] drm/i915: Polish the skl+ plane keyval/msk/max register setup

Message ID 20181101150605.18235-5-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Program SKL+ watermarks/ddb more carefully | expand

Commit Message

Ville Syrjala Nov. 1, 2018, 3:05 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Due to the constant alpha we're going to have to program two of
the the tree keying registers anyway, so might as well always
program all three.

And parametrize the plane constant alpha define while at it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h     |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c | 22 +++++++++-------------
 2 files changed, 10 insertions(+), 14 deletions(-)

Comments

Rodrigo Vivi Nov. 7, 2018, 7:55 p.m. UTC | #1 
On Thu, Nov 01, 2018 at 05:05:55PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Due to the constant alpha we're going to have to program two of
> the the tree keying registers anyway, so might as well always
> program all three.
> 
> And parametrize the plane constant alpha define while at it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'm not 100% confident we can just ignore the key->flags,
but I couldn't convince me that we need that either.

And clean-up by itself looks good, so:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> ---
>  drivers/gpu/drm/i915/i915_reg.h     |  2 +-
>  drivers/gpu/drm/i915/intel_sprite.c | 22 +++++++++-------------
>  2 files changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8d089ef848b2..b6ee863b5df2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6545,7 +6545,7 @@ enum {
>  #define  PLANE_KEYMSK_ALPHA_ENABLE		(1 << 31)
>  #define _PLANE_KEYMAX_1_A			0x701a0
>  #define _PLANE_KEYMAX_2_A			0x702a0
> -#define  PLANE_KEYMAX_ALPHA_SHIFT		24
> +#define  PLANE_KEYMAX_ALPHA(a)			((a) << 24)
>  #define _PLANE_AUX_DIST_1_A			0x701c0
>  #define _PLANE_AUX_DIST_2_A			0x702c0
>  #define _PLANE_AUX_OFFSET_1_A			0x701c4
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 399d44c57a7d..b36238282b4e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -382,31 +382,27 @@ skl_program_plane(struct intel_plane *plane,
>  	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>  	struct intel_plane *linked = plane_state->linked_plane;
> +	u8 alpha = plane_state->base.alpha >> 8;
>  	unsigned long irqflags;
> -	u32 keymsk = 0, keymax = 0;
> +	u32 keymsk, keymax;
>  
>  	/* Sizes are 0 based */
>  	src_w--;
>  	src_h--;
>  
> +	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
> +
> +	keymsk = key->channel_mask & 0x3ffffff;
> +	if (alpha < 0xff)
> +		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> +
>  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
>  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>  		I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
>  			      plane_state->color_ctl);
>  
> -	if (key->flags) {
> -		I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> -
> -		keymax |= key->max_value & 0xffffff;
> -		keymsk |= key->channel_mask & 0x3ffffff;
> -	}
> -
> -	keymax |= (plane_state->base.alpha >> 8) << PLANE_KEYMAX_ALPHA_SHIFT;
> -
> -	if (plane_state->base.alpha < 0xff00)
> -		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> -
> +	I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
>  	I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax);
>  	I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk);
>  
> -- 
> 2.18.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjala Nov. 7, 2018, 8:56 p.m. UTC | #2 
On Wed, Nov 07, 2018 at 11:55:27AM -0800, Rodrigo Vivi wrote:
> On Thu, Nov 01, 2018 at 05:05:55PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Due to the constant alpha we're going to have to program two of
> > the the tree keying registers anyway, so might as well always
> > program all three.
> > 
> > And parametrize the plane constant alpha define while at it.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I'm not 100% confident we can just ignore the key->flags,
> but I couldn't convince me that we need that either.

The colorkey registers should be nops unless we enable the relevant bits
in the control register (skl_plane_ctl()). If not we'd already have a
bug because we were leaving stale values in those registers when
colorkeying was not enabled.

> 
> And clean-up by itself looks good, so:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h     |  2 +-
> >  drivers/gpu/drm/i915/intel_sprite.c | 22 +++++++++-------------
> >  2 files changed, 10 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 8d089ef848b2..b6ee863b5df2 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6545,7 +6545,7 @@ enum {
> >  #define  PLANE_KEYMSK_ALPHA_ENABLE		(1 << 31)
> >  #define _PLANE_KEYMAX_1_A			0x701a0
> >  #define _PLANE_KEYMAX_2_A			0x702a0
> > -#define  PLANE_KEYMAX_ALPHA_SHIFT		24
> > +#define  PLANE_KEYMAX_ALPHA(a)			((a) << 24)
> >  #define _PLANE_AUX_DIST_1_A			0x701c0
> >  #define _PLANE_AUX_DIST_2_A			0x702c0
> >  #define _PLANE_AUX_OFFSET_1_A			0x701c4
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 399d44c57a7d..b36238282b4e 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -382,31 +382,27 @@ skl_program_plane(struct intel_plane *plane,
> >  	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
> >  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >  	struct intel_plane *linked = plane_state->linked_plane;
> > +	u8 alpha = plane_state->base.alpha >> 8;
> >  	unsigned long irqflags;
> > -	u32 keymsk = 0, keymax = 0;
> > +	u32 keymsk, keymax;
> >  
> >  	/* Sizes are 0 based */
> >  	src_w--;
> >  	src_h--;
> >  
> > +	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
> > +
> > +	keymsk = key->channel_mask & 0x3ffffff;
> > +	if (alpha < 0xff)
> > +		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> > +
> >  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >  
> >  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> >  		I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
> >  			      plane_state->color_ctl);
> >  
> > -	if (key->flags) {
> > -		I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> > -
> > -		keymax |= key->max_value & 0xffffff;
> > -		keymsk |= key->channel_mask & 0x3ffffff;
> > -	}
> > -
> > -	keymax |= (plane_state->base.alpha >> 8) << PLANE_KEYMAX_ALPHA_SHIFT;
> > -
> > -	if (plane_state->base.alpha < 0xff00)
> > -		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
> > -
> > +	I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> >  	I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax);
> >  	I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk);
> >  
> > -- 
> > 2.18.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8d089ef848b2..b6ee863b5df2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6545,7 +6545,7 @@  enum {
 #define  PLANE_KEYMSK_ALPHA_ENABLE		(1 << 31)
 #define _PLANE_KEYMAX_1_A			0x701a0
 #define _PLANE_KEYMAX_2_A			0x702a0
-#define  PLANE_KEYMAX_ALPHA_SHIFT		24
+#define  PLANE_KEYMAX_ALPHA(a)			((a) << 24)
 #define _PLANE_AUX_DIST_1_A			0x701c0
 #define _PLANE_AUX_DIST_2_A			0x702c0
 #define _PLANE_AUX_OFFSET_1_A			0x701c4
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 399d44c57a7d..b36238282b4e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -382,31 +382,27 @@  skl_program_plane(struct intel_plane *plane,
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	struct intel_plane *linked = plane_state->linked_plane;
+	u8 alpha = plane_state->base.alpha >> 8;
 	unsigned long irqflags;
-	u32 keymsk = 0, keymax = 0;
+	u32 keymsk, keymax;
 
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
 
+	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
+
+	keymsk = key->channel_mask & 0x3ffffff;
+	if (alpha < 0xff)
+		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
+
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
 		I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
 			      plane_state->color_ctl);
 
-	if (key->flags) {
-		I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
-
-		keymax |= key->max_value & 0xffffff;
-		keymsk |= key->channel_mask & 0x3ffffff;
-	}
-
-	keymax |= (plane_state->base.alpha >> 8) << PLANE_KEYMAX_ALPHA_SHIFT;
-
-	if (plane_state->base.alpha < 0xff00)
-		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
-
+	I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
 	I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax);
 	I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk);