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

[Rodrigo Vivi]

SKUs that lacks on the full port F split will just time out
when touching this power well bits, causing a noisy warn.

v2: Suggested-by: Imre. Temporarily remove the aux pw id after setting
it instead of duplicating and redefining everything.
v3: Simplify even more the logic, using one that don't mix the
array size with the pw bits. Also add a comment.

Cc: Dhinakaran Pandiyan 
Cc: Lucas De Marchi 
Cc: Imre Deak 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 4b215acea439..30e50ea16960 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2544,6 +2544,16 @@ int intel_power_domains_init(struct drm_i915_private 
*dev_priv)
set_power_wells(power_domains, skl_power_wells);
} else if (IS_CANNONLAKE(dev_priv)) {
set_power_wells(power_domains, cnl_power_wells);
+
+   /*
+* DDI and Aux IO are getting enabled for all ports
+* regardless the presence or use. So, in order to avoid
+* timeouts, lets remove them from the list
+* for the SKUs without port F.
+*/
+   if (!IS_CNL_WITH_PORT_F(dev_priv))
+   power_domains->power_well_count -= 2;
+
} else if (IS_BROXTON(dev_priv)) {
set_power_wells(power_domains, bxt_power_wells);
} else if (IS_GEMINILAKE(dev_priv)) {
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Rodrigo Vivi]

On Tue, Jan 23, 2018 at 03:03:28AM +, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Mon, 2018-01-22 at 15:48 -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.
> > 
> > v2: Suggested-by: Imre. Temporarily remove the aux pw id after setting
> > it instead of duplicating and redefining everything.
> > 
> > Cc: Lucas De Marchi 
> > Cc: Imre Deak 
> > Signed-off-by: Rodrigo Vivi 
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 25 +++--
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 433048ffa5c6..7cee63860a7b 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1861,18 +1861,20 @@ void intel_display_power_put(struct 
> > drm_i915_private *dev_priv,
> >  #define CNL_DISPLAY_AUX_D_POWER_DOMAINS (  \
> > BIT_ULL(POWER_DOMAIN_AUX_D) |   \
> > BIT_ULL(POWER_DOMAIN_INIT))
> > -#define CNL_DISPLAY_AUX_F_POWER_DOMAINS (  \
> > -   BIT_ULL(POWER_DOMAIN_AUX_F) |   \
> > -   BIT_ULL(POWER_DOMAIN_INIT))
> > -#define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS (   \
> > -   BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |   \
> > -   BIT_ULL(POWER_DOMAIN_INIT))
> >  #define CNL_DISPLAY_DC_OFF_POWER_DOMAINS ( \
> > CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \
> > BIT_ULL(POWER_DOMAIN_GT_IRQ) |  \
> > BIT_ULL(POWER_DOMAIN_MODESET) | \
> > BIT_ULL(POWER_DOMAIN_AUX_A) |   \
> > BIT_ULL(POWER_DOMAIN_INIT))
> > +/* Power wells for CNL with port F after this */
> > +#define CNL_FIRST_PORT_F_PW CNL_DISP_PW_AUX_F
> > +#define CNL_DISPLAY_AUX_F_POWER_DOMAINS (  \
> > +   BIT_ULL(POWER_DOMAIN_AUX_F) |   \
> > +   BIT_ULL(POWER_DOMAIN_INIT))
> > +#define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS (   \
> > +   BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |   \
> > +   BIT_ULL(POWER_DOMAIN_INIT))
> >  
> >  static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
> > .sync_hw = i9xx_power_well_sync_hw_noop,
> > @@ -2544,6 +2546,17 @@ int intel_power_domains_init(struct drm_i915_private 
> > *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)) {
> > +   int i;
> > +
> > +   for (i = 0; i < power_domains->power_well_count; i++)
> > +   if (power_domains->power_wells[i].id ==
> > +   CNL_FIRST_PORT_F_PW)
> > +   break;
> > +   WARN_ON(power_domains->power_well_count == i - 1);
> > +   power_domains->power_well_count = i - 1;
> Shouldn't this be
>   WARN_ON(power_domains->power_well_count == i);

oh yeap... c from below without thinking. :/

>   power_domains->power_well_count = i; ?

this is also what Imre suggested, but I don't think so.

The first-of-non-port-f - 1 is the last of non-port-f.

> 
>
> > +   }
> > } 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

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

[Pandiyan, Dhinakaran]

On Mon, 2018-01-22 at 15:48 -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.
> 
> v2: Suggested-by: Imre. Temporarily remove the aux pw id after setting
> it instead of duplicating and redefining everything.
> 
> Cc: Lucas De Marchi 
> Cc: Imre Deak 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 25 +++--
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 433048ffa5c6..7cee63860a7b 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1861,18 +1861,20 @@ void intel_display_power_put(struct drm_i915_private 
> *dev_priv,
>  #define CNL_DISPLAY_AUX_D_POWER_DOMAINS (\
>   BIT_ULL(POWER_DOMAIN_AUX_D) |   \
>   BIT_ULL(POWER_DOMAIN_INIT))
> -#define CNL_DISPLAY_AUX_F_POWER_DOMAINS (\
> - BIT_ULL(POWER_DOMAIN_AUX_F) |   \
> - BIT_ULL(POWER_DOMAIN_INIT))
> -#define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \
> - BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |   \
> - BIT_ULL(POWER_DOMAIN_INIT))
>  #define CNL_DISPLAY_DC_OFF_POWER_DOMAINS (   \
>   CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \
>   BIT_ULL(POWER_DOMAIN_GT_IRQ) |  \
>   BIT_ULL(POWER_DOMAIN_MODESET) | \
>   BIT_ULL(POWER_DOMAIN_AUX_A) |   \
>   BIT_ULL(POWER_DOMAIN_INIT))
> +/* Power wells for CNL with port F after this */
> +#define CNL_FIRST_PORT_F_PW CNL_DISP_PW_AUX_F
> +#define CNL_DISPLAY_AUX_F_POWER_DOMAINS (\
> + BIT_ULL(POWER_DOMAIN_AUX_F) |   \
> + BIT_ULL(POWER_DOMAIN_INIT))
> +#define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \
> + BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |   \
> + BIT_ULL(POWER_DOMAIN_INIT))
>  
>  static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
>   .sync_hw = i9xx_power_well_sync_hw_noop,
> @@ -2544,6 +2546,17 @@ int intel_power_domains_init(struct drm_i915_private 
> *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)) {
> + int i;
> +
> + for (i = 0; i < power_domains->power_well_count; i++)
> + if (power_domains->power_wells[i].id ==
> + CNL_FIRST_PORT_F_PW)
> + break;
> + WARN_ON(power_domains->power_well_count == i - 1);
> + power_domains->power_well_count = i - 1;
Shouldn't this be
WARN_ON(power_domains->power_well_count == i);
power_domains->power_well_count = i; ?

 
> + }
>   } 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

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

[Rodrigo Vivi]

SKUs that lacks on the full port F split will just time out
when touching this power well bits, causing a noisy warn.

v2: Suggested-by: Imre. Temporarily remove the aux pw id after setting
it instead of duplicating and redefining everything.

Cc: Lucas De Marchi 
Cc: Imre Deak 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 433048ffa5c6..7cee63860a7b 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1861,18 +1861,20 @@ void intel_display_power_put(struct drm_i915_private 
*dev_priv,
 #define CNL_DISPLAY_AUX_D_POWER_DOMAINS (  \
BIT_ULL(POWER_DOMAIN_AUX_D) |   \
BIT_ULL(POWER_DOMAIN_INIT))
-#define CNL_DISPLAY_AUX_F_POWER_DOMAINS (  \
-   BIT_ULL(POWER_DOMAIN_AUX_F) |   \
-   BIT_ULL(POWER_DOMAIN_INIT))
-#define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS (   \
-   BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |   \
-   BIT_ULL(POWER_DOMAIN_INIT))
 #define CNL_DISPLAY_DC_OFF_POWER_DOMAINS ( \
CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \
BIT_ULL(POWER_DOMAIN_GT_IRQ) |  \
BIT_ULL(POWER_DOMAIN_MODESET) | \
BIT_ULL(POWER_DOMAIN_AUX_A) |   \
BIT_ULL(POWER_DOMAIN_INIT))
+/* Power wells for CNL with port F after this */
+#define CNL_FIRST_PORT_F_PW CNL_DISP_PW_AUX_F
+#define CNL_DISPLAY_AUX_F_POWER_DOMAINS (  \
+   BIT_ULL(POWER_DOMAIN_AUX_F) |   \
+   BIT_ULL(POWER_DOMAIN_INIT))
+#define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS (   \
+   BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |   \
+   BIT_ULL(POWER_DOMAIN_INIT))
 
 static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
.sync_hw = i9xx_power_well_sync_hw_noop,
@@ -2544,6 +2546,17 @@ int intel_power_domains_init(struct drm_i915_private 
*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)) {
+   int i;
+
+   for (i = 0; i < power_domains->power_well_count; i++)
+   if (power_domains->power_wells[i].id ==
+   CNL_FIRST_PORT_F_PW)
+   break;
+   WARN_ON(power_domains->power_well_count == i - 1);
+   power_domains->power_well_count = i - 1;
+   }
} else if (IS_BROXTON(dev_priv)) {
set_power_wells(power_domains, bxt_power_wells);
} else if (IS_GEMINILAKE(dev_priv)) {
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx