| Message ID | 20180216232555.23563-1-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 16, 2018 at 03:25:55PM -0800, Dhinakaran Pandiyan wrote: > PSR requires AUX IO well to be kept on and the existing AUX domain > enables other wells too. > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/i915/intel_display.h | 5 +++++ > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_psr.c | 4 ++++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ > 5 files changed, 31 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > index c4042e342f50..9857b60a7e0f 100644 > --- a/drivers/gpu/drm/i915/intel_display.h > +++ b/drivers/gpu/drm/i915/intel_display.h > @@ -175,6 +175,11 @@ enum intel_display_power_domain { > POWER_DOMAIN_AUX_C, > POWER_DOMAIN_AUX_D, > POWER_DOMAIN_AUX_F, > + POWER_DOMAIN_AUX_IO_A, > + POWER_DOMAIN_AUX_IO_B, > + POWER_DOMAIN_AUX_IO_C, > + POWER_DOMAIN_AUX_IO_D, > + POWER_DOMAIN_AUX_IO_F, I know that I had suggested something very similar, but looking now to the code it seems a big duplication of all other aux.. I mean, on CNL POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as POWER_DOMAIN_AUX__IO_{B,C,D,F} Only the AUX_IO A is different from the AUX_A because AUX_A disables DMC. So, question is: What about removing the AUX_A domain from DC_OFF and just using AUX doamins... Well, on this case calling get/put only for INTEL_GEN <= 10... So, yeap, not beautiful anyways. I'd like to hear from Imre what he had in mind for handling AUX IOs and to know if he has any better idea for this case. > POWER_DOMAIN_GMBUS, > POWER_DOMAIN_MODESET, > POWER_DOMAIN_GT_IRQ, > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index f20b25f98e5a..aa90f6a70689 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) > switch (encoder->port) { > case PORT_A: > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A; > break; > case PORT_B: > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B; > break; > case PORT_C: > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C; > break; > case PORT_D: > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > break; > case PORT_E: > /* FIXME: Check VBT for actual wiring of PORT E */ > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > break; > case PORT_F: > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F; > break; > default: > MISSING_CASE(encoder->port); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 898064e8bea7..c7c4180e41d8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1071,6 +1071,7 @@ struct intel_dp { > struct drm_dp_desc desc; > struct drm_dp_aux aux; > enum intel_display_power_domain aux_power_domain; > + enum intel_display_power_domain aux_io_power_domain; > uint8_t train_set[4]; > int panel_power_up_delay; > int panel_power_down_delay; > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index 2ef374f936b9..62de8f0632b0 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, > EDP_PSR_DEBUG_MASK_HPD | > EDP_PSR_DEBUG_MASK_LPSP); > } > + > + intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain); > } > > /** > @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > else > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > } > + > + intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain); > } > > /** > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 16790f2576ec..323c1babb0a0 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > return "AUX_D"; > case POWER_DOMAIN_AUX_F: > return "AUX_F"; > + case POWER_DOMAIN_AUX_IO_A: > + return "AUX_IO_A"; > + case POWER_DOMAIN_AUX_IO_B: > + return "AUX_IO_B"; > + case POWER_DOMAIN_AUX_IO_C: > + return "AUX_IO_C"; > + case POWER_DOMAIN_AUX_IO_D: > + return "AUX_IO_D"; > + case POWER_DOMAIN_AUX_IO_F: > + return "AUX_IO_F"; > case POWER_DOMAIN_GMBUS: > return "GMBUS"; > case POWER_DOMAIN_INIT: > @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > BIT_ULL(POWER_DOMAIN_INIT)) > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > + BIT_ULL(POWER_DOMAIN_AUX_IO_B) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define CNL_DISPLAY_AUX_C_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_AUX_C) | \ > + BIT_ULL(POWER_DOMAIN_AUX_IO_C) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_AUX_D) | \ > + BIT_ULL(POWER_DOMAIN_AUX_IO_D) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_AUX_F) | \ > + BIT_ULL(POWER_DOMAIN_AUX_IO_F) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) | \ > -- > 2.14.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Feb 16, 2018 at 04:27:04PM -0800, Rodrigo Vivi wrote: > On Fri, Feb 16, 2018 at 03:25:55PM -0800, Dhinakaran Pandiyan wrote: > > PSR requires AUX IO well to be kept on and the existing AUX domain > > enables other wells too. > > > > Cc: Imre Deak <imre.deak@intel.com> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.h | 5 +++++ > > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > drivers/gpu/drm/i915/intel_psr.c | 4 ++++ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ > > 5 files changed, 31 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > > index c4042e342f50..9857b60a7e0f 100644 > > --- a/drivers/gpu/drm/i915/intel_display.h > > +++ b/drivers/gpu/drm/i915/intel_display.h > > @@ -175,6 +175,11 @@ enum intel_display_power_domain { > > POWER_DOMAIN_AUX_C, > > POWER_DOMAIN_AUX_D, > > POWER_DOMAIN_AUX_F, > > + POWER_DOMAIN_AUX_IO_A, > > + POWER_DOMAIN_AUX_IO_B, > > + POWER_DOMAIN_AUX_IO_C, > > + POWER_DOMAIN_AUX_IO_D, > > + POWER_DOMAIN_AUX_IO_F, > > I know that I had suggested something very similar, but looking now > to the code it seems a big duplication of all other aux.. > > I mean, on CNL > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as > POWER_DOMAIN_AUX__IO_{B,C,D,F} > > Only the AUX_IO A is different from the AUX_A because AUX_A > disables DMC. > So, question is: What about removing the AUX_A domain from DC_OFF > and just using AUX doamins... Well, on this case calling get/put > only for INTEL_GEN <= 10... > > So, yeap, not beautiful anyways. > > I'd like to hear from Imre what he had in mind for handling > AUX IOs and to know if he has any better idea for this case. Hm, so in general (for AUX transfers) to enable AUX-A we first need to disable DC states _except_ if we enable AUX-A for PSR where we want DC5/6 to stay enabled. Nice. Also not sure why the FW/HW can't have its own request bit.. I don't have a better idea, but since on DDI where this matters PSR is tied to port A, we could have only a POWER_DOMAIN_PSR and map it to the AUX-A power well where it's needed. --Imre > > > POWER_DOMAIN_GMBUS, > > POWER_DOMAIN_MODESET, > > POWER_DOMAIN_GT_IRQ, > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index f20b25f98e5a..aa90f6a70689 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) > > switch (encoder->port) { > > case PORT_A: > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A; > > break; > > case PORT_B: > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B; > > break; > > case PORT_C: > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C; > > break; > > case PORT_D: > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > break; > > case PORT_E: > > /* FIXME: Check VBT for actual wiring of PORT E */ > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > break; > > case PORT_F: > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F; > > break; > > default: > > MISSING_CASE(encoder->port); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 898064e8bea7..c7c4180e41d8 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1071,6 +1071,7 @@ struct intel_dp { > > struct drm_dp_desc desc; > > struct drm_dp_aux aux; > > enum intel_display_power_domain aux_power_domain; > > + enum intel_display_power_domain aux_io_power_domain; > > uint8_t train_set[4]; > > int panel_power_up_delay; > > int panel_power_down_delay; > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > > index 2ef374f936b9..62de8f0632b0 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, > > EDP_PSR_DEBUG_MASK_HPD | > > EDP_PSR_DEBUG_MASK_LPSP); > > } > > + > > + intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain); > > } > > > > /** > > @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > > else > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > } > > + > > + intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain); > > } > > > > /** > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 16790f2576ec..323c1babb0a0 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > return "AUX_D"; > > case POWER_DOMAIN_AUX_F: > > return "AUX_F"; > > + case POWER_DOMAIN_AUX_IO_A: > > + return "AUX_IO_A"; > > + case POWER_DOMAIN_AUX_IO_B: > > + return "AUX_IO_B"; > > + case POWER_DOMAIN_AUX_IO_C: > > + return "AUX_IO_C"; > > + case POWER_DOMAIN_AUX_IO_D: > > + return "AUX_IO_D"; > > + case POWER_DOMAIN_AUX_IO_F: > > + return "AUX_IO_F"; > > case POWER_DOMAIN_GMBUS: > > return "GMBUS"; > > case POWER_DOMAIN_INIT: > > @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > > + BIT_ULL(POWER_DOMAIN_AUX_IO_B) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define CNL_DISPLAY_AUX_C_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_AUX_C) | \ > > + BIT_ULL(POWER_DOMAIN_AUX_IO_C) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_AUX_D) | \ > > + BIT_ULL(POWER_DOMAIN_AUX_IO_D) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_AUX_F) | \ > > + BIT_ULL(POWER_DOMAIN_AUX_IO_F) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) | \ > > -- > > 2.14.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Feb 19, 2018 at 04:47:41PM +0200, Imre Deak wrote: > On Fri, Feb 16, 2018 at 04:27:04PM -0800, Rodrigo Vivi wrote: > > On Fri, Feb 16, 2018 at 03:25:55PM -0800, Dhinakaran Pandiyan wrote: > > > PSR requires AUX IO well to be kept on and the existing AUX domain > > > enables other wells too. > > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.h | 5 +++++ > > > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > drivers/gpu/drm/i915/intel_psr.c | 4 ++++ > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ > > > 5 files changed, 31 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > > > index c4042e342f50..9857b60a7e0f 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.h > > > +++ b/drivers/gpu/drm/i915/intel_display.h > > > @@ -175,6 +175,11 @@ enum intel_display_power_domain { > > > POWER_DOMAIN_AUX_C, > > > POWER_DOMAIN_AUX_D, > > > POWER_DOMAIN_AUX_F, > > > + POWER_DOMAIN_AUX_IO_A, > > > + POWER_DOMAIN_AUX_IO_B, > > > + POWER_DOMAIN_AUX_IO_C, > > > + POWER_DOMAIN_AUX_IO_D, > > > + POWER_DOMAIN_AUX_IO_F, > > > > I know that I had suggested something very similar, but looking now > > to the code it seems a big duplication of all other aux.. > > > > I mean, on CNL > > > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as > > POWER_DOMAIN_AUX__IO_{B,C,D,F} > > > > Only the AUX_IO A is different from the AUX_A because AUX_A > > disables DMC. > > So, question is: What about removing the AUX_A domain from DC_OFF > > and just using AUX doamins... Well, on this case calling get/put > > only for INTEL_GEN <= 10... > > > > So, yeap, not beautiful anyways. > > > > I'd like to hear from Imre what he had in mind for handling > > AUX IOs and to know if he has any better idea for this case. > > Hm, so in general (for AUX transfers) to enable AUX-A we first need to > disable DC states _except_ if we enable AUX-A for PSR where we want > DC5/6 to stay enabled. Nice. Also not sure why the FW/HW can't have its > own request bit.. > > I don't have a better idea, but since on DDI where this matters PSR is > tied to port A, SKL+ (or maybe even BDW?) can do PSR on any port actually. We just don't make use of that capability currently. > we could have only a POWER_DOMAIN_PSR and map it to the > AUX-A power well where it's needed. > > --Imre > > > > > > POWER_DOMAIN_GMBUS, > > > POWER_DOMAIN_MODESET, > > > POWER_DOMAIN_GT_IRQ, > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > index f20b25f98e5a..aa90f6a70689 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) > > > switch (encoder->port) { > > > case PORT_A: > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A; > > > break; > > > case PORT_B: > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B; > > > break; > > > case PORT_C: > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C; > > > break; > > > case PORT_D: > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > break; > > > case PORT_E: > > > /* FIXME: Check VBT for actual wiring of PORT E */ > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > break; > > > case PORT_F: > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F; > > > break; > > > default: > > > MISSING_CASE(encoder->port); > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index 898064e8bea7..c7c4180e41d8 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -1071,6 +1071,7 @@ struct intel_dp { > > > struct drm_dp_desc desc; > > > struct drm_dp_aux aux; > > > enum intel_display_power_domain aux_power_domain; > > > + enum intel_display_power_domain aux_io_power_domain; > > > uint8_t train_set[4]; > > > int panel_power_up_delay; > > > int panel_power_down_delay; > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > > > index 2ef374f936b9..62de8f0632b0 100644 > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, > > > EDP_PSR_DEBUG_MASK_HPD | > > > EDP_PSR_DEBUG_MASK_LPSP); > > > } > > > + > > > + intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain); > > > } > > > > > > /** > > > @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > > > else > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > > } > > > + > > > + intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain); > > > } > > > > > > /** > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index 16790f2576ec..323c1babb0a0 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > > return "AUX_D"; > > > case POWER_DOMAIN_AUX_F: > > > return "AUX_F"; > > > + case POWER_DOMAIN_AUX_IO_A: > > > + return "AUX_IO_A"; > > > + case POWER_DOMAIN_AUX_IO_B: > > > + return "AUX_IO_B"; > > > + case POWER_DOMAIN_AUX_IO_C: > > > + return "AUX_IO_C"; > > > + case POWER_DOMAIN_AUX_IO_D: > > > + return "AUX_IO_D"; > > > + case POWER_DOMAIN_AUX_IO_F: > > > + return "AUX_IO_F"; > > > case POWER_DOMAIN_GMBUS: > > > return "GMBUS"; > > > case POWER_DOMAIN_INIT: > > > @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \ > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \ > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_B) | \ > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > #define CNL_DISPLAY_AUX_C_POWER_DOMAINS ( \ > > > BIT_ULL(POWER_DOMAIN_AUX_C) | \ > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_C) | \ > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \ > > > BIT_ULL(POWER_DOMAIN_AUX_D) | \ > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_D) | \ > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > #define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \ > > > BIT_ULL(POWER_DOMAIN_AUX_F) | \ > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_F) | \ > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \ > > > BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) | \ > > > -- > > > 2.14.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Feb 19, 2018 at 05:49:41PM +0200, Ville Syrjälä wrote: > On Mon, Feb 19, 2018 at 04:47:41PM +0200, Imre Deak wrote: > > On Fri, Feb 16, 2018 at 04:27:04PM -0800, Rodrigo Vivi wrote: > > > On Fri, Feb 16, 2018 at 03:25:55PM -0800, Dhinakaran Pandiyan wrote: > > > > PSR requires AUX IO well to be kept on and the existing AUX domain > > > > enables other wells too. > > > > > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/intel_display.h | 5 +++++ > > > > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ > > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > > drivers/gpu/drm/i915/intel_psr.c | 4 ++++ > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ > > > > 5 files changed, 31 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > > > > index c4042e342f50..9857b60a7e0f 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.h > > > > +++ b/drivers/gpu/drm/i915/intel_display.h > > > > @@ -175,6 +175,11 @@ enum intel_display_power_domain { > > > > POWER_DOMAIN_AUX_C, > > > > POWER_DOMAIN_AUX_D, > > > > POWER_DOMAIN_AUX_F, > > > > + POWER_DOMAIN_AUX_IO_A, > > > > + POWER_DOMAIN_AUX_IO_B, > > > > + POWER_DOMAIN_AUX_IO_C, > > > > + POWER_DOMAIN_AUX_IO_D, > > > > + POWER_DOMAIN_AUX_IO_F, > > > > > > I know that I had suggested something very similar, but looking now > > > to the code it seems a big duplication of all other aux.. > > > > > > I mean, on CNL > > > > > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as > > > POWER_DOMAIN_AUX__IO_{B,C,D,F} > > > > > > Only the AUX_IO A is different from the AUX_A because AUX_A > > > disables DMC. > > > So, question is: What about removing the AUX_A domain from DC_OFF > > > and just using AUX doamins... Well, on this case calling get/put > > > only for INTEL_GEN <= 10... > > > > > > So, yeap, not beautiful anyways. > > > > > > I'd like to hear from Imre what he had in mind for handling > > > AUX IOs and to know if he has any better idea for this case. > > > > Hm, so in general (for AUX transfers) to enable AUX-A we first need to > > disable DC states _except_ if we enable AUX-A for PSR where we want > > DC5/6 to stay enabled. Nice. Also not sure why the FW/HW can't have its > > own request bit.. > > > > I don't have a better idea, but since on DDI where this matters PSR is > > tied to port A, > > SKL+ (or maybe even BDW?) can do PSR on any port actually. We just don't > make use of that capability currently. Ok. But still, one other idea would be to avoid the duplication Rodrigo also mentioned is to have: get_psr_aux_power() { if (!HAS_DDI()) return; intel_display_power_get(port == PORT_A ? POWER_DOMAIN_AUX_IO_A : intel_dp->aux_power_domain); } and a put_psr_aux_power() accordingly. That way we only need to add one extra power domain for port A. --Imre > > > we could have only a POWER_DOMAIN_PSR and map it to the > > AUX-A power well where it's needed. > > > > --Imre > > > > > > > > > POWER_DOMAIN_GMBUS, > > > > POWER_DOMAIN_MODESET, > > > > POWER_DOMAIN_GT_IRQ, > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > index f20b25f98e5a..aa90f6a70689 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) > > > > switch (encoder->port) { > > > > case PORT_A: > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A; > > > > break; > > > > case PORT_B: > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B; > > > > break; > > > > case PORT_C: > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C; > > > > break; > > > > case PORT_D: > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > > break; > > > > case PORT_E: > > > > /* FIXME: Check VBT for actual wiring of PORT E */ > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > > break; > > > > case PORT_F: > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F; > > > > break; > > > > default: > > > > MISSING_CASE(encoder->port); > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > > index 898064e8bea7..c7c4180e41d8 100644 > > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > > @@ -1071,6 +1071,7 @@ struct intel_dp { > > > > struct drm_dp_desc desc; > > > > struct drm_dp_aux aux; > > > > enum intel_display_power_domain aux_power_domain; > > > > + enum intel_display_power_domain aux_io_power_domain; > > > > uint8_t train_set[4]; > > > > int panel_power_up_delay; > > > > int panel_power_down_delay; > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > > > > index 2ef374f936b9..62de8f0632b0 100644 > > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > > @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, > > > > EDP_PSR_DEBUG_MASK_HPD | > > > > EDP_PSR_DEBUG_MASK_LPSP); > > > > } > > > > + > > > > + intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain); > > > > } > > > > > > > > /** > > > > @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > > > > else > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > > > } > > > > + > > > > + intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain); > > > > } > > > > > > > > /** > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > index 16790f2576ec..323c1babb0a0 100644 > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > > > return "AUX_D"; > > > > case POWER_DOMAIN_AUX_F: > > > > return "AUX_F"; > > > > + case POWER_DOMAIN_AUX_IO_A: > > > > + return "AUX_IO_A"; > > > > + case POWER_DOMAIN_AUX_IO_B: > > > > + return "AUX_IO_B"; > > > > + case POWER_DOMAIN_AUX_IO_C: > > > > + return "AUX_IO_C"; > > > > + case POWER_DOMAIN_AUX_IO_D: > > > > + return "AUX_IO_D"; > > > > + case POWER_DOMAIN_AUX_IO_F: > > > > + return "AUX_IO_F"; > > > > case POWER_DOMAIN_GMBUS: > > > > return "GMBUS"; > > > > case POWER_DOMAIN_INIT: > > > > @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \ > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \ > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_B) | \ > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > #define CNL_DISPLAY_AUX_C_POWER_DOMAINS ( \ > > > > BIT_ULL(POWER_DOMAIN_AUX_C) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_C) | \ > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \ > > > > BIT_ULL(POWER_DOMAIN_AUX_D) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_D) | \ > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > #define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \ > > > > BIT_ULL(POWER_DOMAIN_AUX_F) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_F) | \ > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \ > > > > BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) | \ > > > > -- > > > > 2.14.1 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
On Tue, Feb 20, 2018 at 05:35:20PM +0200, Imre Deak wrote: > On Mon, Feb 19, 2018 at 05:49:41PM +0200, Ville Syrjälä wrote: > > On Mon, Feb 19, 2018 at 04:47:41PM +0200, Imre Deak wrote: > > > On Fri, Feb 16, 2018 at 04:27:04PM -0800, Rodrigo Vivi wrote: > > > > On Fri, Feb 16, 2018 at 03:25:55PM -0800, Dhinakaran Pandiyan wrote: > > > > > PSR requires AUX IO well to be kept on and the existing AUX domain > > > > > enables other wells too. > > > > > > > > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/intel_display.h | 5 +++++ > > > > > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > > > drivers/gpu/drm/i915/intel_psr.c | 4 ++++ > > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ > > > > > 5 files changed, 31 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > > > > > index c4042e342f50..9857b60a7e0f 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_display.h > > > > > +++ b/drivers/gpu/drm/i915/intel_display.h > > > > > @@ -175,6 +175,11 @@ enum intel_display_power_domain { > > > > > POWER_DOMAIN_AUX_C, > > > > > POWER_DOMAIN_AUX_D, > > > > > POWER_DOMAIN_AUX_F, > > > > > + POWER_DOMAIN_AUX_IO_A, > > > > > + POWER_DOMAIN_AUX_IO_B, > > > > > + POWER_DOMAIN_AUX_IO_C, > > > > > + POWER_DOMAIN_AUX_IO_D, > > > > > + POWER_DOMAIN_AUX_IO_F, > > > > > > > > I know that I had suggested something very similar, but looking now > > > > to the code it seems a big duplication of all other aux.. > > > > > > > > I mean, on CNL > > > > > > > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as > > > > POWER_DOMAIN_AUX__IO_{B,C,D,F} > > > > > > > > Only the AUX_IO A is different from the AUX_A because AUX_A > > > > disables DMC. > > > > So, question is: What about removing the AUX_A domain from DC_OFF > > > > and just using AUX doamins... Well, on this case calling get/put > > > > only for INTEL_GEN <= 10... > > > > > > > > So, yeap, not beautiful anyways. > > > > > > > > I'd like to hear from Imre what he had in mind for handling > > > > AUX IOs and to know if he has any better idea for this case. > > > > > > Hm, so in general (for AUX transfers) to enable AUX-A we first need to > > > disable DC states _except_ if we enable AUX-A for PSR where we want > > > DC5/6 to stay enabled. Nice. Also not sure why the FW/HW can't have its > > > own request bit.. > > > > > > I don't have a better idea, but since on DDI where this matters PSR is > > > tied to port A, > > > > SKL+ (or maybe even BDW?) can do PSR on any port actually. We just don't > > make use of that capability currently. > > Ok. But still, one other idea would be to avoid the duplication Rodrigo > also mentioned is to have: > > get_psr_aux_power() > { > if (!HAS_DDI()) Err, correctly the above would be if (GEN < 10) > return; > > intel_display_power_get(port == PORT_A ? POWER_DOMAIN_AUX_IO_A : > intel_dp->aux_power_domain); > } > > and a put_psr_aux_power() accordingly. That way we only need to add one > extra power domain for port A. > > --Imre > > > > > > > > > we could have only a POWER_DOMAIN_PSR and map it to the > > > AUX-A power well where it's needed. > > > > > > --Imre > > > > > > > > > > > > POWER_DOMAIN_GMBUS, > > > > > POWER_DOMAIN_MODESET, > > > > > POWER_DOMAIN_GT_IRQ, > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > > index f20b25f98e5a..aa90f6a70689 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > > @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) > > > > > switch (encoder->port) { > > > > > case PORT_A: > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A; > > > > > break; > > > > > case PORT_B: > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B; > > > > > break; > > > > > case PORT_C: > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C; > > > > > break; > > > > > case PORT_D: > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > > > break; > > > > > case PORT_E: > > > > > /* FIXME: Check VBT for actual wiring of PORT E */ > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > > > break; > > > > > case PORT_F: > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F; > > > > > break; > > > > > default: > > > > > MISSING_CASE(encoder->port); > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > > > index 898064e8bea7..c7c4180e41d8 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > > > @@ -1071,6 +1071,7 @@ struct intel_dp { > > > > > struct drm_dp_desc desc; > > > > > struct drm_dp_aux aux; > > > > > enum intel_display_power_domain aux_power_domain; > > > > > + enum intel_display_power_domain aux_io_power_domain; > > > > > uint8_t train_set[4]; > > > > > int panel_power_up_delay; > > > > > int panel_power_down_delay; > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > > > > > index 2ef374f936b9..62de8f0632b0 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > > > @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, > > > > > EDP_PSR_DEBUG_MASK_HPD | > > > > > EDP_PSR_DEBUG_MASK_LPSP); > > > > > } > > > > > + > > > > > + intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain); > > > > > } > > > > > > > > > > /** > > > > > @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > > > > > else > > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > > > > } > > > > > + > > > > > + intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain); > > > > > } > > > > > > > > > > /** > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > index 16790f2576ec..323c1babb0a0 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > > > > return "AUX_D"; > > > > > case POWER_DOMAIN_AUX_F: > > > > > return "AUX_F"; > > > > > + case POWER_DOMAIN_AUX_IO_A: > > > > > + return "AUX_IO_A"; > > > > > + case POWER_DOMAIN_AUX_IO_B: > > > > > + return "AUX_IO_B"; > > > > > + case POWER_DOMAIN_AUX_IO_C: > > > > > + return "AUX_IO_C"; > > > > > + case POWER_DOMAIN_AUX_IO_D: > > > > > + return "AUX_IO_D"; > > > > > + case POWER_DOMAIN_AUX_IO_F: > > > > > + return "AUX_IO_F"; > > > > > case POWER_DOMAIN_GMBUS: > > > > > return "GMBUS"; > > > > > case POWER_DOMAIN_INIT: > > > > > @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \ > > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \ > > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_B) | \ > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > #define CNL_DISPLAY_AUX_C_POWER_DOMAINS ( \ > > > > > BIT_ULL(POWER_DOMAIN_AUX_C) | \ > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_C) | \ > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \ > > > > > BIT_ULL(POWER_DOMAIN_AUX_D) | \ > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_D) | \ > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > #define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \ > > > > > BIT_ULL(POWER_DOMAIN_AUX_F) | \ > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_F) | \ > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \ > > > > > BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) | \ > > > > > -- > > > > > 2.14.1 > > > > > > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Feb 20, 2018 at 06:25:38PM +0200, Imre Deak wrote: > On Tue, Feb 20, 2018 at 05:35:20PM +0200, Imre Deak wrote: > > On Mon, Feb 19, 2018 at 05:49:41PM +0200, Ville Syrjälä wrote: > > > On Mon, Feb 19, 2018 at 04:47:41PM +0200, Imre Deak wrote: > > > > On Fri, Feb 16, 2018 at 04:27:04PM -0800, Rodrigo Vivi wrote: > > > > > On Fri, Feb 16, 2018 at 03:25:55PM -0800, Dhinakaran Pandiyan wrote: > > > > > > PSR requires AUX IO well to be kept on and the existing AUX domain > > > > > > enables other wells too. > > > > > > > > > > > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > > > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/intel_display.h | 5 +++++ > > > > > > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ > > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > > > > drivers/gpu/drm/i915/intel_psr.c | 4 ++++ > > > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ > > > > > > 5 files changed, 31 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > > > > > > index c4042e342f50..9857b60a7e0f 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_display.h > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.h > > > > > > @@ -175,6 +175,11 @@ enum intel_display_power_domain { > > > > > > POWER_DOMAIN_AUX_C, > > > > > > POWER_DOMAIN_AUX_D, > > > > > > POWER_DOMAIN_AUX_F, > > > > > > + POWER_DOMAIN_AUX_IO_A, > > > > > > + POWER_DOMAIN_AUX_IO_B, > > > > > > + POWER_DOMAIN_AUX_IO_C, > > > > > > + POWER_DOMAIN_AUX_IO_D, > > > > > > + POWER_DOMAIN_AUX_IO_F, > > > > > > > > > > I know that I had suggested something very similar, but looking now > > > > > to the code it seems a big duplication of all other aux.. > > > > > > > > > > I mean, on CNL > > > > > > > > > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as > > > > > POWER_DOMAIN_AUX__IO_{B,C,D,F} > > > > > > > > > > Only the AUX_IO A is different from the AUX_A because AUX_A > > > > > disables DMC. > > > > > So, question is: What about removing the AUX_A domain from DC_OFF > > > > > and just using AUX doamins... Well, on this case calling get/put > > > > > only for INTEL_GEN <= 10... > > > > > > > > > > So, yeap, not beautiful anyways. > > > > > > > > > > I'd like to hear from Imre what he had in mind for handling > > > > > AUX IOs and to know if he has any better idea for this case. > > > > > > > > Hm, so in general (for AUX transfers) to enable AUX-A we first need to > > > > disable DC states _except_ if we enable AUX-A for PSR where we want > > > > DC5/6 to stay enabled. Nice. Also not sure why the FW/HW can't have its > > > > own request bit.. > > > > > > > > I don't have a better idea, but since on DDI where this matters PSR is > > > > tied to port A, > > > > > > SKL+ (or maybe even BDW?) can do PSR on any port actually. We just don't > > > make use of that capability currently. > > > > Ok. But still, one other idea would be to avoid the duplication Rodrigo > > also mentioned is to have: > > > > get_psr_aux_power() > > { > > if (!HAS_DDI()) > > Err, correctly the above would be if (GEN < 10) Exactly what I was going to reply here :) > > > return; > > > > intel_display_power_get(port == PORT_A ? POWER_DOMAIN_AUX_IO_A : > > intel_dp->aux_power_domain); good idea. Although as Ville pointed out we don't support PSR on other ports it is better to do the right thing at least here. > > } > > > > and a put_psr_aux_power() accordingly. That way we only need to add one > > extra power domain for port A. Also maybe add a comment explaining the difference and purpose of POWER_DOMAIN_AUX_IO_A from POWER_DOMAIN_AUX_A along with POWER_DOMAIN_AUX_IO_A definition. Thanks, Rodrigo. > > > > --Imre > > > > > > > > > > > > > > > we could have only a POWER_DOMAIN_PSR and map it to the > > > > AUX-A power well where it's needed. > > > > > > > > --Imre > > > > > > > > > > > > > > > POWER_DOMAIN_GMBUS, > > > > > > POWER_DOMAIN_MODESET, > > > > > > POWER_DOMAIN_GT_IRQ, > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > > > index f20b25f98e5a..aa90f6a70689 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > > > @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) > > > > > > switch (encoder->port) { > > > > > > case PORT_A: > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A; > > > > > > break; > > > > > > case PORT_B: > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B; > > > > > > break; > > > > > > case PORT_C: > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C; > > > > > > break; > > > > > > case PORT_D: > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > > > > break; > > > > > > case PORT_E: > > > > > > /* FIXME: Check VBT for actual wiring of PORT E */ > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > > > > break; > > > > > > case PORT_F: > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F; > > > > > > break; > > > > > > default: > > > > > > MISSING_CASE(encoder->port); > > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > > > > index 898064e8bea7..c7c4180e41d8 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > > > > @@ -1071,6 +1071,7 @@ struct intel_dp { > > > > > > struct drm_dp_desc desc; > > > > > > struct drm_dp_aux aux; > > > > > > enum intel_display_power_domain aux_power_domain; > > > > > > + enum intel_display_power_domain aux_io_power_domain; > > > > > > uint8_t train_set[4]; > > > > > > int panel_power_up_delay; > > > > > > int panel_power_down_delay; > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > > > > > > index 2ef374f936b9..62de8f0632b0 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > > > > @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, > > > > > > EDP_PSR_DEBUG_MASK_HPD | > > > > > > EDP_PSR_DEBUG_MASK_LPSP); > > > > > > } > > > > > > + > > > > > > + intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain); > > > > > > } > > > > > > > > > > > > /** > > > > > > @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > > > > > > else > > > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > > > > > } > > > > > > + > > > > > > + intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain); > > > > > > } > > > > > > > > > > > > /** > > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > > index 16790f2576ec..323c1babb0a0 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > > @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > > > > > return "AUX_D"; > > > > > > case POWER_DOMAIN_AUX_F: > > > > > > return "AUX_F"; > > > > > > + case POWER_DOMAIN_AUX_IO_A: > > > > > > + return "AUX_IO_A"; > > > > > > + case POWER_DOMAIN_AUX_IO_B: > > > > > > + return "AUX_IO_B"; > > > > > > + case POWER_DOMAIN_AUX_IO_C: > > > > > > + return "AUX_IO_C"; > > > > > > + case POWER_DOMAIN_AUX_IO_D: > > > > > > + return "AUX_IO_D"; > > > > > > + case POWER_DOMAIN_AUX_IO_F: > > > > > > + return "AUX_IO_F"; > > > > > > case POWER_DOMAIN_GMBUS: > > > > > > return "GMBUS"; > > > > > > case POWER_DOMAIN_INIT: > > > > > > @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \ > > > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \ > > > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_B) | \ > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > #define CNL_DISPLAY_AUX_C_POWER_DOMAINS ( \ > > > > > > BIT_ULL(POWER_DOMAIN_AUX_C) | \ > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_C) | \ > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \ > > > > > > BIT_ULL(POWER_DOMAIN_AUX_D) | \ > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_D) | \ > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > #define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \ > > > > > > BIT_ULL(POWER_DOMAIN_AUX_F) | \ > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_F) | \ > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \ > > > > > > BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) | \ > > > > > > -- > > > > > > 2.14.1 > > > > > > > > > > > > _______________________________________________ > > > > > > Intel-gfx mailing list > > > > > > Intel-gfx@lists.freedesktop.org > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- > > > Ville Syrjälä > > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2018-02-20 at 10:44 -0800, Rodrigo Vivi wrote: > On Tue, Feb 20, 2018 at 06:25:38PM +0200, Imre Deak wrote: > > On Tue, Feb 20, 2018 at 05:35:20PM +0200, Imre Deak wrote: > > > On Mon, Feb 19, 2018 at 05:49:41PM +0200, Ville Syrjälä wrote: > > > > On Mon, Feb 19, 2018 at 04:47:41PM +0200, Imre Deak wrote: > > > > > On Fri, Feb 16, 2018 at 04:27:04PM -0800, Rodrigo Vivi wrote: > > > > > > On Fri, Feb 16, 2018 at 03:25:55PM -0800, Dhinakaran Pandiyan wrote: > > > > > > > PSR requires AUX IO well to be kept on and the existing AUX domain > > > > > > > enables other wells too. > > > > > > > > > > > > > > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > > > > > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/intel_display.h | 5 +++++ > > > > > > > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ > > > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > > > > > drivers/gpu/drm/i915/intel_psr.c | 4 ++++ > > > > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ > > > > > > > 5 files changed, 31 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > > > > > > > index c4042e342f50..9857b60a7e0f 100644 > > > > > > > --- a/drivers/gpu/drm/i915/intel_display.h > > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.h > > > > > > > @@ -175,6 +175,11 @@ enum intel_display_power_domain { > > > > > > > POWER_DOMAIN_AUX_C, > > > > > > > POWER_DOMAIN_AUX_D, > > > > > > > POWER_DOMAIN_AUX_F, > > > > > > > + POWER_DOMAIN_AUX_IO_A, > > > > > > > + POWER_DOMAIN_AUX_IO_B, > > > > > > > + POWER_DOMAIN_AUX_IO_C, > > > > > > > + POWER_DOMAIN_AUX_IO_D, > > > > > > > + POWER_DOMAIN_AUX_IO_F, > > > > > > > > > > > > I know that I had suggested something very similar, but looking now > > > > > > to the code it seems a big duplication of all other aux.. > > > > > > > > > > > > I mean, on CNL > > > > > > > > > > > > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as > > > > > > POWER_DOMAIN_AUX__IO_{B,C,D,F} > > > > > > > > > > > > Only the AUX_IO A is different from the AUX_A because AUX_A > > > > > > disables DMC. > > > > > > So, question is: What about removing the AUX_A domain from DC_OFF > > > > > > and just using AUX doamins... Well, on this case calling get/put > > > > > > only for INTEL_GEN <= 10... > > > > > > > > > > > > So, yeap, not beautiful anyways. > > > > > > > > > > > > I'd like to hear from Imre what he had in mind for handling > > > > > > AUX IOs and to know if he has any better idea for this case. > > > > > > > > > > Hm, so in general (for AUX transfers) to enable AUX-A we first need to > > > > > disable DC states _except_ if we enable AUX-A for PSR where we want > > > > > DC5/6 to stay enabled. Nice. Also not sure why the FW/HW can't have its > > > > > own request bit.. > > > > > > > > > > I don't have a better idea, but since on DDI where this matters PSR is > > > > > tied to port A, > > > > > > > > SKL+ (or maybe even BDW?) can do PSR on any port actually. We just don't > > > > make use of that capability currently. > > > > > > Ok. But still, one other idea would be to avoid the duplication Rodrigo > > > also mentioned is to have: > > > > > > get_psr_aux_power() > > > { > > > if (!HAS_DDI()) > > > > Err, correctly the above would be if (GEN < 10) > > Exactly what I was going to reply here :) > > > > > > return; > > > > > > intel_display_power_get(port == PORT_A ? POWER_DOMAIN_AUX_IO_A : > > > intel_dp->aux_power_domain); > > good idea. Although as Ville pointed out we don't support PSR on other ports > it is better to do the right thing at least here. > > > > } > > > > > > and a put_psr_aux_power() accordingly. That way we only need to add one > > > extra power domain for port A. > > Also maybe add a comment explaining the difference and purpose of POWER_DOMAIN_AUX_IO_A > from POWER_DOMAIN_AUX_A along with POWER_DOMAIN_AUX_IO_A definition. I'd sent this as a test patch to get a CI run but now that there's a consensus on how we should do this, I'll write this up as proper patch. Thanks for the feedback. -DK > > Thanks, > Rodrigo. > > > > > > > --Imre > > > > > > > > > > > > > > > > > > > > > we could have only a POWER_DOMAIN_PSR and map it to the > > > > > AUX-A power well where it's needed. > > > > > > > > > > --Imre > > > > > > > > > > > > > > > > > > POWER_DOMAIN_GMBUS, > > > > > > > POWER_DOMAIN_MODESET, > > > > > > > POWER_DOMAIN_GT_IRQ, > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > > > > index f20b25f98e5a..aa90f6a70689 100644 > > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > > > > @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) > > > > > > > switch (encoder->port) { > > > > > > > case PORT_A: > > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; > > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A; > > > > > > > break; > > > > > > > case PORT_B: > > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; > > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B; > > > > > > > break; > > > > > > > case PORT_C: > > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; > > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C; > > > > > > > break; > > > > > > > case PORT_D: > > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > > > > > break; > > > > > > > case PORT_E: > > > > > > > /* FIXME: Check VBT for actual wiring of PORT E */ > > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; > > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; > > > > > > > break; > > > > > > > case PORT_F: > > > > > > > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; > > > > > > > + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F; > > > > > > > break; > > > > > > > default: > > > > > > > MISSING_CASE(encoder->port); > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > > > > > index 898064e8bea7..c7c4180e41d8 100644 > > > > > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > > > > > @@ -1071,6 +1071,7 @@ struct intel_dp { > > > > > > > struct drm_dp_desc desc; > > > > > > > struct drm_dp_aux aux; > > > > > > > enum intel_display_power_domain aux_power_domain; > > > > > > > + enum intel_display_power_domain aux_io_power_domain; > > > > > > > uint8_t train_set[4]; > > > > > > > int panel_power_up_delay; > > > > > > > int panel_power_down_delay; > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > > > > > > > index 2ef374f936b9..62de8f0632b0 100644 > > > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > > > > > @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, > > > > > > > EDP_PSR_DEBUG_MASK_HPD | > > > > > > > EDP_PSR_DEBUG_MASK_LPSP); > > > > > > > } > > > > > > > + > > > > > > > + intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain); > > > > > > > } > > > > > > > > > > > > > > /** > > > > > > > @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > > > > > > > else > > > > > > > WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > > > > > > } > > > > > > > + > > > > > > > + intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain); > > > > > > > } > > > > > > > > > > > > > > /** > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > > > index 16790f2576ec..323c1babb0a0 100644 > > > > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > > > @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > > > > > > return "AUX_D"; > > > > > > > case POWER_DOMAIN_AUX_F: > > > > > > > return "AUX_F"; > > > > > > > + case POWER_DOMAIN_AUX_IO_A: > > > > > > > + return "AUX_IO_A"; > > > > > > > + case POWER_DOMAIN_AUX_IO_B: > > > > > > > + return "AUX_IO_B"; > > > > > > > + case POWER_DOMAIN_AUX_IO_C: > > > > > > > + return "AUX_IO_C"; > > > > > > > + case POWER_DOMAIN_AUX_IO_D: > > > > > > > + return "AUX_IO_D"; > > > > > > > + case POWER_DOMAIN_AUX_IO_F: > > > > > > > + return "AUX_IO_F"; > > > > > > > case POWER_DOMAIN_GMBUS: > > > > > > > return "GMBUS"; > > > > > > > case POWER_DOMAIN_INIT: > > > > > > > @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > > #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \ > > > > > > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > > #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \ > > > > > > > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_B) | \ > > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > > #define CNL_DISPLAY_AUX_C_POWER_DOMAINS ( \ > > > > > > > BIT_ULL(POWER_DOMAIN_AUX_C) | \ > > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_C) | \ > > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > > #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \ > > > > > > > BIT_ULL(POWER_DOMAIN_AUX_D) | \ > > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_D) | \ > > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > > #define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \ > > > > > > > BIT_ULL(POWER_DOMAIN_AUX_F) | \ > > > > > > > + BIT_ULL(POWER_DOMAIN_AUX_IO_F) | \ > > > > > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > > > > #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \ > > > > > > > BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) | \ > > > > > > > -- > > > > > > > 2.14.1 > > > > > > > > > > > > > > _______________________________________________ > > > > > > > Intel-gfx mailing list > > > > > > > Intel-gfx@lists.freedesktop.org > > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > -- > > > > Ville Syrjälä > > > > Intel OTC > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h index c4042e342f50..9857b60a7e0f 100644 --- a/drivers/gpu/drm/i915/intel_display.h +++ b/drivers/gpu/drm/i915/intel_display.h @@ -175,6 +175,11 @@ enum intel_display_power_domain { POWER_DOMAIN_AUX_C, POWER_DOMAIN_AUX_D, POWER_DOMAIN_AUX_F, + POWER_DOMAIN_AUX_IO_A, + POWER_DOMAIN_AUX_IO_B, + POWER_DOMAIN_AUX_IO_C, + POWER_DOMAIN_AUX_IO_D, + POWER_DOMAIN_AUX_IO_F, POWER_DOMAIN_GMBUS, POWER_DOMAIN_MODESET, POWER_DOMAIN_GT_IRQ, diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f20b25f98e5a..aa90f6a70689 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port) switch (encoder->port) { case PORT_A: intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A; + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A; break; case PORT_B: intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B; + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B; break; case PORT_C: intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C; + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C; break; case PORT_D: intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; break; case PORT_E: /* FIXME: Check VBT for actual wiring of PORT E */ intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D; + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D; break; case PORT_F: intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F; + intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F; break; default: MISSING_CASE(encoder->port); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 898064e8bea7..c7c4180e41d8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1071,6 +1071,7 @@ struct intel_dp { struct drm_dp_desc desc; struct drm_dp_aux aux; enum intel_display_power_domain aux_power_domain; + enum intel_display_power_domain aux_io_power_domain; uint8_t train_set[4]; int panel_power_up_delay; int panel_power_down_delay; diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2ef374f936b9..62de8f0632b0 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP); } + + intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain); } /** @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, else WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); } + + intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain); } /** diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 16790f2576ec..323c1babb0a0 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) return "AUX_D"; case POWER_DOMAIN_AUX_F: return "AUX_F"; + case POWER_DOMAIN_AUX_IO_A: + return "AUX_IO_A"; + case POWER_DOMAIN_AUX_IO_B: + return "AUX_IO_B"; + case POWER_DOMAIN_AUX_IO_C: + return "AUX_IO_C"; + case POWER_DOMAIN_AUX_IO_D: + return "AUX_IO_D"; + case POWER_DOMAIN_AUX_IO_F: + return "AUX_IO_F"; case POWER_DOMAIN_GMBUS: return "GMBUS"; case POWER_DOMAIN_INIT: @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, BIT_ULL(POWER_DOMAIN_INIT)) #define CNL_DISPLAY_AUX_A_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_AUX_A) | \ + BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define CNL_DISPLAY_AUX_B_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_AUX_B) | \ + BIT_ULL(POWER_DOMAIN_AUX_IO_B) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define CNL_DISPLAY_AUX_C_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_AUX_C) | \ + BIT_ULL(POWER_DOMAIN_AUX_IO_C) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_AUX_D) | \ + BIT_ULL(POWER_DOMAIN_AUX_IO_D) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_AUX_F) | \ + BIT_ULL(POWER_DOMAIN_AUX_IO_F) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) | \
PSR requires AUX IO well to be kept on and the existing AUX domain enables other wells too. Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/i915/intel_display.h | 5 +++++ drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_psr.c | 4 ++++ drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ 5 files changed, 31 insertions(+)