From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"De Marchi, Lucas" <lucas.demarchi@intel.com>
Subject: Re: [PATCH 11/11] drm/i915/cnl: Don't try to manage Port F power wells on all CNL.
Date: Thu, 28 Dec 2017 08:24:41 -0800 [thread overview]
Message-ID: <20171228162441.2oy3vgyihassoeos@intel.com> (raw)
In-Reply-To: <1514325709.22022.148.camel@dk-H97M-D3H>
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@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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)) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-12-28 16:24 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-22 23:18 [PATCH 01/11] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
2017-12-22 23:18 ` [PATCH 02/11] drm/i915/cnl: Add Port F definition Rodrigo Vivi
2017-12-26 20:50 ` Pandiyan, Dhinakaran
2017-12-28 16:45 ` Rodrigo Vivi
2018-01-11 14:22 ` Paulo Zanoni
2017-12-22 23:18 ` [PATCH 03/11] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
2017-12-26 21:48 ` Pandiyan, Dhinakaran
2017-12-28 16:48 ` Rodrigo Vivi
2017-12-22 23:18 ` [PATCH 04/11] drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition Rodrigo Vivi
2017-12-26 21:28 ` Pandiyan, Dhinakaran
2017-12-22 23:18 ` [PATCH 05/11] drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F Rodrigo Vivi
2017-12-26 22:36 ` Pandiyan, Dhinakaran
2017-12-28 16:51 ` Rodrigo Vivi
2017-12-22 23:18 ` [PATCH 06/11] drm/i915/cnl: Add right GMBUS pin number for HDMI on " Rodrigo Vivi
2018-01-11 14:29 ` Paulo Zanoni
2017-12-22 23:18 ` [PATCH 07/11] drm/i915: For HPD connected port use hpd_pin instead of port Rodrigo Vivi
2017-12-22 23:18 ` [PATCH 08/11] drm/i915/cnl: Add HPD support for Port F Rodrigo Vivi
2017-12-22 23:18 ` [PATCH 09/11] drm/i915/cnl: Enable DDI-F on Cannonlake Rodrigo Vivi
2017-12-22 23:18 ` [PATCH 10/11] drm/i915/cnl: Fix DP max rate for Cannonlake with port F Rodrigo Vivi
2017-12-22 23:18 ` [PATCH 11/11] drm/i915/cnl: Don't try to manage Port F power wells on all CNL Rodrigo Vivi
2017-12-26 21:39 ` Pandiyan, Dhinakaran
2017-12-28 16:24 ` Rodrigo Vivi [this message]
2017-12-22 23:37 ` ✓ Fi.CI.BAT: success for series starting with [01/11] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Patchwork
2017-12-23 0:40 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-01-09 18:48 ` [PATCH 01/11] " Paulo Zanoni
2018-01-10 0:21 ` Lucas De Marchi
-- strict thread matches above, loose matches on Subject: below --
2017-12-22 22:23 Rodrigo Vivi
2017-12-22 22:23 ` [PATCH 11/11] drm/i915/cnl: Don't try to manage Port F power wells on all CNL Rodrigo Vivi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171228162441.2oy3vgyihassoeos@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=dhinakaran.pandiyan@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=lucas.demarchi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.