Re: [PATCH 4/4] drm/i915: Extend skl+ crc sources with more planes

From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/4] drm/i915: Extend skl+ crc sources with more planes
Date: Thu, 14 Feb 2019 12:47:23 -0800	[thread overview]
Message-ID: <20190214204723.GM31265@intel.com> (raw)
In-Reply-To: <20190214192219.3858-4-ville.syrjala@linux.intel.com>

On Thu, Feb 14, 2019 at 09:22:19PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On skl the crc registers were extended to provide plane crcs
> for up to 7 planes. Add the new crc sources.
> 
> The current code uses the ivb+ register definitions for skl+
> which does happen to work as the plane1, plane2, and dmux/pf
> bits happen the match what ivb+ had. So no bug in the current
> code.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  5 ++
>  drivers/gpu/drm/i915/i915_reg.h       |  9 ++++
>  drivers/gpu/drm/i915/intel_pipe_crc.c | 76 ++++++++++++++++++++++++++-
>  3 files changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4e11d970cbcf..8607c1e9ed02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1196,6 +1196,11 @@ enum intel_pipe_crc_source {
>  	INTEL_PIPE_CRC_SOURCE_NONE,
>  	INTEL_PIPE_CRC_SOURCE_PLANE1,
>  	INTEL_PIPE_CRC_SOURCE_PLANE2,
> +	INTEL_PIPE_CRC_SOURCE_PLANE3,
> +	INTEL_PIPE_CRC_SOURCE_PLANE4,
> +	INTEL_PIPE_CRC_SOURCE_PLANE5,
> +	INTEL_PIPE_CRC_SOURCE_PLANE6,
> +	INTEL_PIPE_CRC_SOURCE_PLANE7,
>  	INTEL_PIPE_CRC_SOURCE_PIPE,
>  	/* TV/DP on pre-gen5/vlv can't use the pipe source. */
>  	INTEL_PIPE_CRC_SOURCE_TV,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0df8c6e76da7..5286536e9cb8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4017,6 +4017,15 @@ enum {
>  /* Pipe A CRC regs */
>  #define _PIPE_CRC_CTL_A			0x60050
>  #define   PIPE_CRC_ENABLE		(1 << 31)
> +/* skl+ source selection */
> +#define   PIPE_CRC_SOURCE_PLANE_1_SKL	(0 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_2_SKL	(2 << 28)
> +#define   PIPE_CRC_SOURCE_DMUX_SKL	(4 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_3_SKL	(6 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_4_SKL	(7 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_5_SKL	(5 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_6_SKL	(3 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_7_SKL	(1 << 28)

I got myself staring at spec for a while trying to
understand the logic of this sequence...

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

>  /* ivb+ source selection */
>  #define   PIPE_CRC_SOURCE_PRIMARY_IVB	(0 << 29)
>  #define   PIPE_CRC_SOURCE_SPRITE_IVB	(1 << 29)
> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
> index 66bb7b031537..e521f82ba5d9 100644
> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> @@ -34,6 +34,11 @@ static const char * const pipe_crc_sources[] = {
>  	[INTEL_PIPE_CRC_SOURCE_NONE] = "none",
>  	[INTEL_PIPE_CRC_SOURCE_PLANE1] = "plane1",
>  	[INTEL_PIPE_CRC_SOURCE_PLANE2] = "plane2",
> +	[INTEL_PIPE_CRC_SOURCE_PLANE3] = "plane3",
> +	[INTEL_PIPE_CRC_SOURCE_PLANE4] = "plane4",
> +	[INTEL_PIPE_CRC_SOURCE_PLANE5] = "plane5",
> +	[INTEL_PIPE_CRC_SOURCE_PLANE6] = "plane6",
> +	[INTEL_PIPE_CRC_SOURCE_PLANE7] = "plane7",
>  	[INTEL_PIPE_CRC_SOURCE_PIPE] = "pipe",
>  	[INTEL_PIPE_CRC_SOURCE_TV] = "TV",
>  	[INTEL_PIPE_CRC_SOURCE_DP_B] = "DP-B",
> @@ -368,6 +373,50 @@ static int ivb_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
>  	return 0;
>  }
>  
> +static int skl_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
> +				enum pipe pipe,
> +				enum intel_pipe_crc_source *source,
> +				uint32_t *val,
> +				bool set_wa)
> +{
> +	if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> +		*source = INTEL_PIPE_CRC_SOURCE_PIPE;
> +
> +	switch (*source) {
> +	case INTEL_PIPE_CRC_SOURCE_PLANE1:
> +		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_1_SKL;
> +		break;
> +	case INTEL_PIPE_CRC_SOURCE_PLANE2:
> +		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_2_SKL;
> +		break;
> +	case INTEL_PIPE_CRC_SOURCE_PLANE3:
> +		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_3_SKL;
> +		break;
> +	case INTEL_PIPE_CRC_SOURCE_PLANE4:
> +		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_4_SKL;
> +		break;
> +	case INTEL_PIPE_CRC_SOURCE_PLANE5:
> +		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_5_SKL;
> +		break;
> +	case INTEL_PIPE_CRC_SOURCE_PLANE6:
> +		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_6_SKL;
> +		break;
> +	case INTEL_PIPE_CRC_SOURCE_PLANE7:
> +		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_7_SKL;
> +		break;
> +	case INTEL_PIPE_CRC_SOURCE_PIPE:
> +		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DMUX_SKL;
> +		break;
> +	case INTEL_PIPE_CRC_SOURCE_NONE:
> +		*val = 0;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
>  			       enum pipe pipe,
>  			       enum intel_pipe_crc_source *source, u32 *val,
> @@ -381,8 +430,10 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
>  		return vlv_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
>  	else if (IS_GEN_RANGE(dev_priv, 5, 6))
>  		return ilk_pipe_crc_ctl_reg(source, val);
> -	else
> +	else if (INTEL_GEN(dev_priv) < 9)
>  		return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val, set_wa);
> +	else
> +		return skl_pipe_crc_ctl_reg(dev_priv, pipe, source, val, set_wa);
>  }
>  
>  static int
> @@ -482,6 +533,25 @@ static int ivb_crc_source_valid(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +static int skl_crc_source_valid(struct drm_i915_private *dev_priv,
> +				const enum intel_pipe_crc_source source)
> +{
> +	switch (source) {
> +	case INTEL_PIPE_CRC_SOURCE_PIPE:
> +	case INTEL_PIPE_CRC_SOURCE_PLANE1:
> +	case INTEL_PIPE_CRC_SOURCE_PLANE2:
> +	case INTEL_PIPE_CRC_SOURCE_PLANE3:
> +	case INTEL_PIPE_CRC_SOURCE_PLANE4:
> +	case INTEL_PIPE_CRC_SOURCE_PLANE5:
> +	case INTEL_PIPE_CRC_SOURCE_PLANE6:
> +	case INTEL_PIPE_CRC_SOURCE_PLANE7:
> +	case INTEL_PIPE_CRC_SOURCE_NONE:
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int
>  intel_is_valid_crc_source(struct drm_i915_private *dev_priv,
>  			  const enum intel_pipe_crc_source source)
> @@ -494,8 +564,10 @@ intel_is_valid_crc_source(struct drm_i915_private *dev_priv,
>  		return vlv_crc_source_valid(dev_priv, source);
>  	else if (IS_GEN_RANGE(dev_priv, 5, 6))
>  		return ilk_crc_source_valid(dev_priv, source);
> -	else
> +	else if (INTEL_GEN(dev_priv) < 9)
>  		return ivb_crc_source_valid(dev_priv, source);
> +	else
> +		return skl_crc_source_valid(dev_priv, source);
>  }
>  
>  const char *const *intel_crtc_get_crc_sources(struct drm_crtc *crtc,
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx