Re: [PATCH 7/7] drm/i915: Throw out WIP CHV power well definitions

[Deepak S <deepak.s@linux.intel.com>]

On Friday 10 April 2015 08:51 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Expecting CHV power wells to be just an extended versions of the VLV
> power wells, a bunch of commented out power wells were added in
> anticipation when Punit folks would implement it all. Turns out they
> never did, and instead CHV has fewer power wells than VLV. Rip out all
> the #if 0'ed junk that's not needed.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h         |  4 --
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 97 +--------------------------------
>   2 files changed, 3 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 34c366a..f2e0d58 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -595,10 +595,6 @@ enum punit_power_well {
>   	PUNIT_POWER_WELL_DPIO_RX0		= 10,
>   	PUNIT_POWER_WELL_DPIO_RX1		= 11,
>   	PUNIT_POWER_WELL_DPIO_CMN_D		= 12,
> -	/* FIXME: guesswork below */
> -	PUNIT_POWER_WELL_DPIO_TX_D_LANES_01	= 13,
> -	PUNIT_POWER_WELL_DPIO_TX_D_LANES_23	= 14,
> -	PUNIT_POWER_WELL_DPIO_RX2		= 15,
>   
>   	PUNIT_POWER_WELL_NUM,
>   };
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index d9e00d3..f208806 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -977,18 +977,6 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
>   	BIT(POWER_DOMAIN_AUX_C) |		\
>   	BIT(POWER_DOMAIN_INIT))
>   
> -#define CHV_PIPE_A_POWER_DOMAINS (	\
> -	BIT(POWER_DOMAIN_PIPE_A) |	\
> -	BIT(POWER_DOMAIN_INIT))
> -
> -#define CHV_PIPE_B_POWER_DOMAINS (	\
> -	BIT(POWER_DOMAIN_PIPE_B) |	\
> -	BIT(POWER_DOMAIN_INIT))
> -
> -#define CHV_PIPE_C_POWER_DOMAINS (	\
> -	BIT(POWER_DOMAIN_PIPE_C) |	\
> -	BIT(POWER_DOMAIN_INIT))
> -
>   #define CHV_DPIO_CMN_BC_POWER_DOMAINS (		\
>   	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |	\
>   	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |	\
> @@ -1004,17 +992,6 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
>   	BIT(POWER_DOMAIN_AUX_D) |		\
>   	BIT(POWER_DOMAIN_INIT))
>   
> -#define CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS (	\
> -	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |	\
> -	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |	\
> -	BIT(POWER_DOMAIN_AUX_D) |		\
> -	BIT(POWER_DOMAIN_INIT))
> -
> -#define CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS (	\
> -	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |	\
> -	BIT(POWER_DOMAIN_AUX_D) |		\
> -	BIT(POWER_DOMAIN_INIT))
> -
>   static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
>   	.sync_hw = i9xx_always_on_power_well_noop,
>   	.enable = i9xx_always_on_power_well_noop,
> @@ -1172,40 +1149,16 @@ static struct i915_power_well chv_power_wells[] = {
>   		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
>   		.ops = &i9xx_always_on_power_well_ops,
>   	},
> -#if 0
> -	{
> -		.name = "display",
> -		.domains = VLV_DISPLAY_POWER_DOMAINS,
> -		.data = PUNIT_POWER_WELL_DISP2D,
> -		.ops = &vlv_display_power_well_ops,
> -	},
> -#endif
>   	{
>   		.name = "pipe-a",
>   		/*
> -		 * FIXME: pipe A power well seems to be the new disp2d well.
> -		 * At least all registers seem to be housed there. Figure
> -		 * out if this a a temporary situation in pre-production
> -		 * hardware or a permanent state of affairs.
> +		 * pipe A power well is the new disp2d well.
> +		 * pipe B and C wells don't actually exist.

Can we add a comment saying "enabling pipe A sub system will also enable pipe B & c"
Because it is confusing, We says pipe B and C wells don't actually exist,
then if we use PIPE B to drive. how is it working without powering up the well?

Other than this. patch looks fine
Reviewed-by:  Deepak S<deepak.s@linux.intel.com>

>   		 */
> -		.domains = CHV_PIPE_A_POWER_DOMAINS | VLV_DISPLAY_POWER_DOMAINS,
> +		.domains = VLV_DISPLAY_POWER_DOMAINS,
>   		.data = PIPE_A,
>   		.ops = &chv_pipe_power_well_ops,
>   	},
> -#if 0
> -	{
> -		.name = "pipe-b",
> -		.domains = CHV_PIPE_B_POWER_DOMAINS,
> -		.data = PIPE_B,
> -		.ops = &chv_pipe_power_well_ops,
> -	},
> -	{
> -		.name = "pipe-c",
> -		.domains = CHV_PIPE_C_POWER_DOMAINS,
> -		.data = PIPE_C,
> -		.ops = &chv_pipe_power_well_ops,
> -	},
> -#endif
>   	{
>   		.name = "dpio-common-bc",
>   		.domains = CHV_DPIO_CMN_BC_POWER_DOMAINS,
> @@ -1218,50 +1171,6 @@ static struct i915_power_well chv_power_wells[] = {
>   		.data = PUNIT_POWER_WELL_DPIO_CMN_D,
>   		.ops = &chv_dpio_cmn_power_well_ops,
>   	},
> -#if 0
> -	{
> -		.name = "dpio-tx-b-01",
> -		.domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS |
> -			   VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS,
> -		.ops = &vlv_dpio_power_well_ops,
> -		.data = PUNIT_POWER_WELL_DPIO_TX_B_LANES_01,
> -	},
> -	{
> -		.name = "dpio-tx-b-23",
> -		.domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS |
> -			   VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS,
> -		.ops = &vlv_dpio_power_well_ops,
> -		.data = PUNIT_POWER_WELL_DPIO_TX_B_LANES_23,
> -	},
> -	{
> -		.name = "dpio-tx-c-01",
> -		.domains = VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS |
> -			   VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS,
> -		.ops = &vlv_dpio_power_well_ops,
> -		.data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_01,
> -	},
> -	{
> -		.name = "dpio-tx-c-23",
> -		.domains = VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS |
> -			   VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS,
> -		.ops = &vlv_dpio_power_well_ops,
> -		.data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_23,
> -	},
> -	{
> -		.name = "dpio-tx-d-01",
> -		.domains = CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS |
> -			   CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS,
> -		.ops = &vlv_dpio_power_well_ops,
> -		.data = PUNIT_POWER_WELL_DPIO_TX_D_LANES_01,
> -	},
> -	{
> -		.name = "dpio-tx-d-23",
> -		.domains = CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS |
> -			   CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS,
> -		.ops = &vlv_dpio_power_well_ops,
> -		.data = PUNIT_POWER_WELL_DPIO_TX_D_LANES_23,
> -	},
> -#endif
>   };
>   
>   static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_priv,

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx