Re: [Intel-gfx] [PATCH 08/14] drm/i915: Clean up ivb+ sprite plane registers

From: "Souza, Jose" <jose.souza@intel.com>
To: "ville.syrjala@linux.intel.com" <ville.syrjala@linux.intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 08/14] drm/i915: Clean up ivb+ sprite plane registers
Date: Fri, 14 Jan 2022 16:26:32 +0000	[thread overview]
Message-ID: <b658a305d48158351de037c1c0708fb55d5654ec.camel@intel.com> (raw)
In-Reply-To: <20211201152552.7821-9-ville.syrjala@linux.intel.com>

On Wed, 2021-12-01 at 17:25 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use REG_BIT() & co. to polish the ivb+ sprite plane registers.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_sprite.c | 20 +++--
>  drivers/gpu/drm/i915/i915_reg.h             | 81 +++++++++++++--------
>  2 files changed, 62 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 2067a7bca4a8..70083d04a9fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -700,7 +700,7 @@ static u32 ivb_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
>  	u32 sprctl = 0;
>  
>  	if (crtc_state->gamma_enable)
> -		sprctl |= SPRITE_GAMMA_ENABLE;
> +		sprctl |= SPRITE_PIPE_GAMMA_ENABLE;
>  
>  	if (crtc_state->csc_enable)
>  		sprctl |= SPRITE_PIPE_CSC_ENABLE;
> @@ -770,7 +770,7 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
>  	}
>  
>  	if (!ivb_need_sprite_gamma(plane_state))
> -		sprctl |= SPRITE_INT_GAMMA_DISABLE;
> +		sprctl |= SPRITE_PLANE_GAMMA_DISABLE;
>  
>  	if (plane_state->hw.color_encoding == DRM_COLOR_YCBCR_BT709)
>  		sprctl |= SPRITE_YUV_TO_RGB_CSC_FORMAT_BT709;
> @@ -863,14 +863,18 @@ ivb_sprite_update_noarm(struct intel_plane *plane,
>  	unsigned long irqflags;
>  
>  	if (crtc_w != src_w || crtc_h != src_h)
> -		sprscale = SPRITE_SCALE_ENABLE | ((src_w - 1) << 16) | (src_h - 1);
> +		sprscale = SPRITE_SCALE_ENABLE |
> +			SPRITE_SRC_WIDTH(src_w - 1) |
> +			SPRITE_SRC_HEIGHT(src_h - 1);
>  
>  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
>  	intel_de_write_fw(dev_priv, SPRSTRIDE(pipe),
>  			  plane_state->view.color_plane[0].mapping_stride);
> -	intel_de_write_fw(dev_priv, SPRPOS(pipe), (crtc_y << 16) | crtc_x);
> -	intel_de_write_fw(dev_priv, SPRSIZE(pipe), ((crtc_h - 1) << 16) | (crtc_w - 1));
> +	intel_de_write_fw(dev_priv, SPRPOS(pipe),
> +			  SPRITE_POS_Y(crtc_y) | SPRITE_POS_X(crtc_x));
> +	intel_de_write_fw(dev_priv, SPRSIZE(pipe),
> +			  SPRITE_HEIGHT(crtc_h - 1) | SPRITE_WIDTH(crtc_w - 1));
>  	if (IS_IVYBRIDGE(dev_priv))
>  		intel_de_write_fw(dev_priv, SPRSCALE(pipe), sprscale);
>  
> @@ -907,10 +911,12 @@ ivb_sprite_update_arm(struct intel_plane *plane,
>  	/* HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET
>  	 * register */
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> -		intel_de_write_fw(dev_priv, SPROFFSET(pipe), (y << 16) | x);
> +		intel_de_write_fw(dev_priv, SPROFFSET(pipe),
> +				  SPRITE_OFFSET_Y(y) | SPRITE_OFFSET_X(x));
>  	} else {
>  		intel_de_write_fw(dev_priv, SPRLINOFF(pipe), linear_offset);
> -		intel_de_write_fw(dev_priv, SPRTILEOFF(pipe), (y << 16) | x);
> +		intel_de_write_fw(dev_priv, SPRTILEOFF(pipe),
> +				  SPRITE_OFFSET_Y(y) | SPRITE_OFFSET_X(x));
>  	}
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8678cbab1d33..0bd47a929f5d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7046,50 +7046,67 @@ enum {
>  #define DVSGAMCMAX_ILK(pipe, i) _MMIO(_PIPE(pipe, _DVSAGAMCMAX_ILK, _DVSBGAMCMAX_ILK) + (i) * 4) /* 3 x u1.10 */
>  
>  #define _SPRA_CTL		0x70280
> -#define   SPRITE_ENABLE			(1 << 31)
> -#define   SPRITE_GAMMA_ENABLE		(1 << 30)
> -#define   SPRITE_YUV_RANGE_CORRECTION_DISABLE	(1 << 28)
> -#define   SPRITE_PIXFORMAT_MASK		(7 << 25)
> -#define   SPRITE_FORMAT_YUV422		(0 << 25)
> -#define   SPRITE_FORMAT_RGBX101010	(1 << 25)
> -#define   SPRITE_FORMAT_RGBX888		(2 << 25)
> -#define   SPRITE_FORMAT_RGBX161616	(3 << 25)
> -#define   SPRITE_FORMAT_YUV444		(4 << 25)
> -#define   SPRITE_FORMAT_XR_BGR101010	(5 << 25) /* Extended range */
> -#define   SPRITE_PIPE_CSC_ENABLE	(1 << 24)
> -#define   SPRITE_SOURCE_KEY		(1 << 22)
> -#define   SPRITE_RGB_ORDER_RGBX		(1 << 20) /* only for 888 and 161616 */
> -#define   SPRITE_YUV_TO_RGB_CSC_DISABLE	(1 << 19)
> -#define   SPRITE_YUV_TO_RGB_CSC_FORMAT_BT709	(1 << 18) /* 0 is BT601 */
> -#define   SPRITE_YUV_ORDER_MASK		(3 << 16)
> -#define   SPRITE_YUV_ORDER_YUYV		(0 << 16)
> -#define   SPRITE_YUV_ORDER_UYVY		(1 << 16)
> -#define   SPRITE_YUV_ORDER_YVYU		(2 << 16)
> -#define   SPRITE_YUV_ORDER_VYUY		(3 << 16)
> -#define   SPRITE_ROTATE_180		(1 << 15)
> -#define   SPRITE_TRICKLE_FEED_DISABLE	(1 << 14)
> -#define   SPRITE_INT_GAMMA_DISABLE	(1 << 13)
> -#define   SPRITE_TILED			(1 << 10)
> -#define   SPRITE_DEST_KEY		(1 << 2)
> +#define   SPRITE_ENABLE				REG_BIT(31)
> +#define   SPRITE_PIPE_GAMMA_ENABLE		REG_BIT(30)
> +#define   SPRITE_YUV_RANGE_CORRECTION_DISABLE	REG_BIT(28)
> +#define   SPRITE_FORMAT_MASK			REG_GENMASK(27, 25)
> +#define   SPRITE_FORMAT_YUV422			REG_FIELD_PREP(SPRITE_FORMAT_MASK, 0)
> +#define   SPRITE_FORMAT_RGBX101010		REG_FIELD_PREP(SPRITE_FORMAT_MASK, 1)
> +#define   SPRITE_FORMAT_RGBX888			REG_FIELD_PREP(SPRITE_FORMAT_MASK, 2)
> +#define   SPRITE_FORMAT_RGBX161616		REG_FIELD_PREP(SPRITE_FORMAT_MASK, 3)
> +#define   SPRITE_FORMAT_YUV444			REG_FIELD_PREP(SPRITE_FORMAT_MASK, 4)
> +#define   SPRITE_FORMAT_XR_BGR101010		REG_FIELD_PREP(SPRITE_FORMAT_MASK, 5) /* Extended range */
> +#define   SPRITE_PIPE_CSC_ENABLE		REG_BIT(24)
> +#define   SPRITE_SOURCE_KEY			REG_BIT(22)
> +#define   SPRITE_RGB_ORDER_RGBX			REG_BIT(20) /* only for 888 and 161616 */
> +#define   SPRITE_YUV_TO_RGB_CSC_DISABLE		REG_BIT(19)
> +#define   SPRITE_YUV_TO_RGB_CSC_FORMAT_BT709	REG_BIT(18) /* 0 is BT601 */
> +#define   SPRITE_YUV_ORDER_MASK			REG_GENMASK(17, 16)
> +#define   SPRITE_YUV_ORDER_YUYV			REG_FIELD_PREP(SPRITE_YUV_ORDER_MASK, 0)
> +#define   SPRITE_YUV_ORDER_UYVY			REG_FIELD_PREP(SPRITE_YUV_ORDER_MASK, 1)
> +#define   SPRITE_YUV_ORDER_YVYU			REG_FIELD_PREP(SPRITE_YUV_ORDER_MASK, 2)
> +#define   SPRITE_YUV_ORDER_VYUY			REG_FIELD_PREP(SPRITE_YUV_ORDER_MASK, 3)
> +#define   SPRITE_ROTATE_180			REG_BIT(15)
> +#define   SPRITE_TRICKLE_FEED_DISABLE		REG_BIT(14)
> +#define   SPRITE_PLANE_GAMMA_DISABLE		REG_BIT(13)
> +#define   SPRITE_TILED				REG_BIT(10)
> +#define   SPRITE_DEST_KEY			REG_BIT(2)
>  #define _SPRA_LINOFF		0x70284
>  #define _SPRA_STRIDE		0x70288
>  #define _SPRA_POS		0x7028c
> +#define   SPRITE_POS_Y_MASK	REG_GENMASK(31, 16)
> +#define   SPRITE_POS_Y(y)	REG_FIELD_PREP(SPRITE_POS_Y_MASK, (y))
> +#define   SPRITE_POS_X_MASK	REG_GENMASK(15, 0)
> +#define   SPRITE_POS_X(x)	REG_FIELD_PREP(SPRITE_POS_X_MASK, (x))
>  #define _SPRA_SIZE		0x70290
> +#define   SPRITE_HEIGHT_MASK	REG_GENMASK(31, 16)
> +#define   SPRITE_HEIGHT(h)	REG_FIELD_PREP(SPRITE_HEIGHT_MASK, (h))
> +#define   SPRITE_WIDTH_MASK	REG_GENMASK(15, 0)
> +#define   SPRITE_WIDTH(w)	REG_FIELD_PREP(SPRITE_WIDTH_MASK, (w))
>  #define _SPRA_KEYVAL		0x70294
>  #define _SPRA_KEYMSK		0x70298
>  #define _SPRA_SURF		0x7029c
> +#define   SPRITE_ADDR_MASK	REG_GENMASK(31, 12)
>  #define _SPRA_KEYMAX		0x702a0
>  #define _SPRA_TILEOFF		0x702a4
> +#define   SPRITE_OFFSET_Y_MASK	REG_GENMASK(31, 16)
> +#define   SPRITE_OFFSET_Y(y)	REG_FIELD_PREP(SPRITE_OFFSET_Y_MASK, (y))
> +#define   SPRITE_OFFSET_X_MASK	REG_GENMASK(15, 0)
> +#define   SPRITE_OFFSET_X(x)	REG_FIELD_PREP(SPRITE_OFFSET_X_MASK, (x))
>  #define _SPRA_OFFSET		0x702a4
>  #define _SPRA_SURFLIVE		0x702ac
>  #define _SPRA_SCALE		0x70304
> -#define   SPRITE_SCALE_ENABLE	(1 << 31)
> -#define   SPRITE_FILTER_MASK	(3 << 29)
> -#define   SPRITE_FILTER_MEDIUM	(0 << 29)
> -#define   SPRITE_FILTER_ENHANCING	(1 << 29)
> -#define   SPRITE_FILTER_SOFTENING	(2 << 29)
> -#define   SPRITE_VERTICAL_OFFSET_HALF	(1 << 28) /* must be enabled below */
> -#define   SPRITE_VERTICAL_OFFSET_ENABLE	(1 << 27)
> +#define   SPRITE_SCALE_ENABLE			REG_BIT(31)
> +#define   SPRITE_FILTER_MASK			REG_GENMASK(30, 29)
> +#define   SPRITE_FILTER_MEDIUM			REG_FIELD_PREP(SPRITE_FILTER_MASK, 0)
> +#define   SPRITE_FILTER_ENHANCING		REG_FIELD_PREP(SPRITE_FILTER_MASK, 1)
> +#define   SPRITE_FILTER_SOFTENING		REG_FIELD_PREP(SPRITE_FILTER_MASK, 2)
> +#define   SPRITE_VERTICAL_OFFSET_HALF		REG_BIT(28) /* must be enabled below */
> +#define   SPRITE_VERTICAL_OFFSET_ENABLE		REG_BIT(27)
> +#define   SPRITE_SRC_WIDTH_MASK			REG_GENMASK(26, 16)
> +#define   SPRITE_SRC_WIDTH(w)			REG_FIELD_PREP(SPRITE_SRC_WIDTH_MASK, (w))
> +#define   SPRITE_SRC_HEIGHT_MASK		REG_GENMASK(10, 0)
> +#define   SPRITE_SRC_HEIGHT(h)			REG_FIELD_PREP(SPRITE_SRC_HEIGHT_MASK, (h))
>  #define _SPRA_GAMC		0x70400
>  #define _SPRA_GAMC16		0x70440
>  #define _SPRA_GAMC17		0x7044c