[Intel-gfx] [PATCH 11/11] drm/i915/cnl: Don't try to manage Port F power wells on all CNL.
Rodrigo Vivi
rodrigo.vivi at intel.com
Thu Dec 28 16:24:41 UTC 2017
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 at intel.com>
> > Cc: Imre Deak <imre.deak at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> > 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)) {
More information about the Intel-gfx
mailing list