Re: [PATCH 02/17] drm/i915/icl: add ICL support to cnl_set_procmon_ref_values

[James Ausmus <james.ausmus@intel.com>]

On Fri, Feb 02, 2018 at 02:23:04PM -0200, Paulo Zanoni wrote:
> On ICL we have two sets of registers: one for port A and another for
> port B. The set of port A registers is the same as the CNL registers.
> 
> Since the procmon table on ICL is the same we want to reuse the CNL
> function. To do that we add a port argument and make CNL always call
> the function passing port A. This way, we'll be able to easily reuse
> the function on ICL when we add icl_display_core_init().
> 
> v2: Don't use _PICK() when you can use a ternary operator.
> v3: Don't use a ternary operation when you can use _MMIO_PORT (Ville).
>     Add an extra comment about why we're passing PORT_A (James).
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h         | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 22 +++++++++++++++-------
>  2 files changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 65ba10ad1fe5..f6e1677e8211 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2104,6 +2104,28 @@ enum i915_power_well_id {
>  #define CNL_PORT_COMP_DW9		_MMIO(0x162124)
>  #define CNL_PORT_COMP_DW10		_MMIO(0x162128)
>  
> +#define _ICL_PORT_COMP_DW0_A		0x162100
> +#define _ICL_PORT_COMP_DW0_B		0x6C100
> +#define ICL_PORT_COMP_DW0(port)		_MMIO_PORT(port, _ICL_PORT_COMP_DW0_A, \
> +							 _ICL_PORT_COMP_DW0_B)
> +#define _ICL_PORT_COMP_DW1_A		0x162104
> +#define _ICL_PORT_COMP_DW1_B		0x6C104
> +#define ICL_PORT_COMP_DW1(port)		_MMIO_PORT(port, _ICL_PORT_COMP_DW1_A, \
> +							 _ICL_PORT_COMP_DW1_B)
> +#define _ICL_PORT_COMP_DW3_A		0x16210C
> +#define _ICL_PORT_COMP_DW3_B		0x6C10C
> +#define ICL_PORT_COMP_DW3(port)		_MMIO_PORT(port, _ICL_PORT_COMP_DW3_A, \
> +							 _ICL_PORT_COMP_DW3_B)
> +#define _ICL_PORT_COMP_DW9_A		0x162124
> +#define _ICL_PORT_COMP_DW9_B		0x6C124
> +#define ICL_PORT_COMP_DW9(port)		_MMIO_PORT(port, _ICL_PORT_COMP_DW9_A, \
> +							 _ICL_PORT_COMP_DW9_B)
> +#define _ICL_PORT_COMP_DW10_A		0x162128
> +#define _ICL_PORT_COMP_DW10_B		0x6C128
> +#define ICL_PORT_COMP_DW10(port)	_MMIO_PORT(port, \
> +						   _ICL_PORT_COMP_DW10_A, \
> +						   _ICL_PORT_COMP_DW10_B)
> +
>  /* BXT PHY Ref registers */
>  #define _PORT_REF_DW3_A			0x16218C
>  #define _PORT_REF_DW3_BC		0x6C18C
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 70e659772a7a..b4ef7875f055 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2794,12 +2794,19 @@ static const struct cnl_procmon {
>  		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 = 0x8AE38FF1, },
>  };
>  
> -static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv)
> +/*
> + * CNL has just one set of registers, while ICL has two sets: one for port A and
> + * the other for port B. The CNL registers are equivalent to the ICL port A
> + * registers, that's why we call the ICL macros even though the function has CNL
> + * on its name.
> + */
> +static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
> +				       enum port port)
>  {
>  	const struct cnl_procmon *procmon;
>  	u32 val;
>  
> -	val = I915_READ(CNL_PORT_COMP_DW3);
> +	val = I915_READ(ICL_PORT_COMP_DW3(port));
>  	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
>  	default:
>  		MISSING_CASE(val);
> @@ -2820,13 +2827,13 @@ static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv)
>  		break;
>  	}
>  
> -	val = I915_READ(CNL_PORT_COMP_DW1);
> +	val = I915_READ(ICL_PORT_COMP_DW1(port));
>  	val &= ~((0xff << 16) | 0xff);
>  	val |= procmon->dw1;
> -	I915_WRITE(CNL_PORT_COMP_DW1, val);
> +	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
>  
> -	I915_WRITE(CNL_PORT_COMP_DW9, procmon->dw9);
> -	I915_WRITE(CNL_PORT_COMP_DW10, procmon->dw10);
> +	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> +	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
>  }
>  
>  static void cnl_display_core_init(struct drm_i915_private *dev_priv, bool resume)
> @@ -2847,7 +2854,8 @@ static void cnl_display_core_init(struct drm_i915_private *dev_priv, bool resume
>  	val &= ~CNL_COMP_PWR_DOWN;
>  	I915_WRITE(CHICKEN_MISC_2, val);
>  
> -	cnl_set_procmon_ref_values(dev_priv);
> +	/* Dummy PORT_A to get the correct CNL register from the ICL macro */
> +	cnl_set_procmon_ref_values(dev_priv, PORT_A);
>  
>  	val = I915_READ(CNL_PORT_COMP_DW0);
>  	val |= COMP_INIT;
> -- 
> 2.14.3
> 
> _______________________________________________
> 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