Re: [PATCH 11/11] drm/i915/cnl: Don't try to manage Port F power wells on all CNL.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Dec 26, 2017 at 09:39:58PM +0000, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Fri, 2017-12-22 at 15:18 -0800, Rodrigo Vivi wrote:
> > SKUs that lacks on the full port F split will just time out
> > when touching this power well bits, causing a noisy warn.
> 
> Shouldn't be this be squashed with [PATCH 09/11] in that case?

Actually on patch 03/11 (drm/i915/cnl: Add AUX-F support)

> Why
> introduce a WARN and then fix it.

Good question. It will then just be a very ugly big patch with
this change on it. But since you saw separated already you know what
it is about and it wont be so bad for review ;)

But besides trying to make review easier I was just lazy because
I noticed the WARN latter and I was trying different attempts here
with joint lists and other approaches and this was the one I sent
when I gave up and just had time to wrap up to send before christmas break.

> 
> 
> > 
> > This macro style is a deviation from the original definition in use
> > for other platforms, but it at least avoid code duplication.
> > Other smart alternatives like providing a joint list was also considered
> > but it would require some extra memory handling that would be
> > a deviation from the original simplistic definitions here anyways,
> > plus requiring extra tests and possibly creating some corner cases
> > for one single platform. So let's move with the simplest and safest
> > approach.
> > 
> > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > Cc: Imre Deak <imre.deak@xxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 177 +++++++++++++++++---------------
> >  1 file changed, 94 insertions(+), 83 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 461c0759e2c4..a6199c6a2005 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -2324,89 +2324,96 @@ static struct i915_power_well glk_power_wells[] = {
> >  	},
> >  };
> >  
> > +#define basic_cnl_power_wells						\
> > +	{								\
> > +		.name = "always-on",					\
> > +		.always_on = 1,						\
> > +		.domains = POWER_DOMAIN_MASK,				\
> > +		.ops = &i9xx_always_on_power_well_ops,			\
> > +		.id = I915_DISP_PW_ALWAYS_ON,				\
> > +	},								\
> > +	{								\
> > +		.name = "power well 1",					\
> > +		/* Handled by the DMC firmware */			\
> > +		.domains = 0,						\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = SKL_DISP_PW_1,					\
> > +		{							\
> > +			.hsw.has_fuses = true,				\
> > +		},							\
> > +	},								\
> > +	{								\
> > +		.name = "AUX A",					\
> > +		.domains = CNL_DISPLAY_AUX_A_POWER_DOMAINS,		\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = CNL_DISP_PW_AUX_A,				\
> > +	},								\
> > +	{								\
> > +		.name = "AUX B",					\
> > +		.domains = CNL_DISPLAY_AUX_B_POWER_DOMAINS,		\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = CNL_DISP_PW_AUX_B,				\
> > +	},								\
> > +	{								\
> > +		.name = "AUX C",					\
> > +		.domains = CNL_DISPLAY_AUX_C_POWER_DOMAINS,		\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = CNL_DISP_PW_AUX_C,				\
> > +	},								\
> > +	{								\
> > +		.name = "AUX D",					\
> > +		.domains = CNL_DISPLAY_AUX_D_POWER_DOMAINS,		\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = CNL_DISP_PW_AUX_D,				\
> > +		},							\
> > +	{								\
> > +		.name = "DC off",					\
> > +		.domains = CNL_DISPLAY_DC_OFF_POWER_DOMAINS,		\
> > +		.ops = &gen9_dc_off_power_well_ops,			\
> > +		.id = SKL_DISP_PW_DC_OFF,				\
> > +	},								\
> > +	{								\
> > +		.name = "power well 2",					\
> > +		.domains = CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS,	\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = SKL_DISP_PW_2,					\
> > +		{							\
> > +			.hsw.irq_pipe_mask = BIT(PIPE_B) | BIT(PIPE_C),	\
> > +			.hsw.has_vga = true,				\
> > +			.hsw.has_fuses = true,				\
> > +		},							\
> > +	},								\
> > +	{								\
> > +		.name = "DDI A IO power well",				\
> > +		.domains = CNL_DISPLAY_DDI_A_IO_POWER_DOMAINS,		\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = CNL_DISP_PW_DDI_A,				\
> > +	},								\
> > +	{								\
> > +		.name = "DDI B IO power well",				\
> > +		.domains = CNL_DISPLAY_DDI_B_IO_POWER_DOMAINS,		\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = SKL_DISP_PW_DDI_B,				\
> > +	},								\
> > +	{								\
> > +		.name = "DDI C IO power well",				\
> > +		.domains = CNL_DISPLAY_DDI_C_IO_POWER_DOMAINS,		\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = SKL_DISP_PW_DDI_C,				\
> > +	},								\
> > +	{								\
> > +		.name = "DDI D IO power well",				\
> > +		.domains = CNL_DISPLAY_DDI_D_IO_POWER_DOMAINS,		\
> > +		.ops = &hsw_power_well_ops,				\
> > +		.id = SKL_DISP_PW_DDI_D,				\
> > +	}
> > +
> >  static struct i915_power_well cnl_power_wells[] = {
> > -	{
> > -		.name = "always-on",
> > -		.always_on = 1,
> > -		.domains = POWER_DOMAIN_MASK,
> > -		.ops = &i9xx_always_on_power_well_ops,
> > -		.id = I915_DISP_PW_ALWAYS_ON,
> > -	},
> > -	{
> > -		.name = "power well 1",
> > -		/* Handled by the DMC firmware */
> > -		.domains = 0,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = SKL_DISP_PW_1,
> > -		{
> > -			.hsw.has_fuses = true,
> > -		},
> > -	},
> > -	{
> > -		.name = "AUX A",
> > -		.domains = CNL_DISPLAY_AUX_A_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = CNL_DISP_PW_AUX_A,
> > -	},
> > -	{
> > -		.name = "AUX B",
> > -		.domains = CNL_DISPLAY_AUX_B_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = CNL_DISP_PW_AUX_B,
> > -	},
> > -	{
> > -		.name = "AUX C",
> > -		.domains = CNL_DISPLAY_AUX_C_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = CNL_DISP_PW_AUX_C,
> > -	},
> > -	{
> > -		.name = "AUX D",
> > -		.domains = CNL_DISPLAY_AUX_D_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = CNL_DISP_PW_AUX_D,
> > -	},
> > -	{
> > -		.name = "DC off",
> > -		.domains = CNL_DISPLAY_DC_OFF_POWER_DOMAINS,
> > -		.ops = &gen9_dc_off_power_well_ops,
> > -		.id = SKL_DISP_PW_DC_OFF,
> > -	},
> > -	{
> > -		.name = "power well 2",
> > -		.domains = CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = SKL_DISP_PW_2,
> > -		{
> > -			.hsw.irq_pipe_mask = BIT(PIPE_B) | BIT(PIPE_C),
> > -			.hsw.has_vga = true,
> > -			.hsw.has_fuses = true,
> > -		},
> > -	},
> > -	{
> > -		.name = "DDI A IO power well",
> > -		.domains = CNL_DISPLAY_DDI_A_IO_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = CNL_DISP_PW_DDI_A,
> > -	},
> > -	{
> > -		.name = "DDI B IO power well",
> > -		.domains = CNL_DISPLAY_DDI_B_IO_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = SKL_DISP_PW_DDI_B,
> > -	},
> > -	{
> > -		.name = "DDI C IO power well",
> > -		.domains = CNL_DISPLAY_DDI_C_IO_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = SKL_DISP_PW_DDI_C,
> > -	},
> > -	{
> > -		.name = "DDI D IO power well",
> > -		.domains = CNL_DISPLAY_DDI_D_IO_POWER_DOMAINS,
> > -		.ops = &hsw_power_well_ops,
> > -		.id = SKL_DISP_PW_DDI_D,
> > -	},
> > +	basic_cnl_power_wells,
> > +};
> > +
> > +static struct i915_power_well cnl_power_wells_with_port_f[] = {
> > +	basic_cnl_power_wells,
> >  	{
> >  		.name = "DDI F IO power well",
> >  		.domains = CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS,
> > @@ -2533,7 +2540,11 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> >  	} else if (IS_GEN9_BC(dev_priv)) {
> >  		set_power_wells(power_domains, skl_power_wells);
> >  	} else if (IS_CANNONLAKE(dev_priv)) {
> > -		set_power_wells(power_domains, cnl_power_wells);
> > +		if (IS_CNL_WITH_PORT_F(dev_priv))
> > +			set_power_wells(power_domains,
> > +					cnl_power_wells_with_port_f);
> > +		else
> > +			set_power_wells(power_domains, cnl_power_wells);
> >  	} else if (IS_BROXTON(dev_priv)) {
> >  		set_power_wells(power_domains, bxt_power_wells);
> >  	} else if (IS_GEMINILAKE(dev_priv)) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux