| Message ID | 1479917907-2468-6-git-send-email-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2016-11-23 at 21:48 +0530, Animesh Manna wrote: > While suspending the device hpd related interrupts are enabled > to get the interrupt when device is in suspend state. > > Though display is in DC9 but system can be in S0 or S0i3 state. > Hot plug during S0 state will generate de_port_interrupt but if > system is in S0i3 state then display driver will get hotplug > interrupt as pcu_hpd_interrupt which will come via pmc. So > added the interrupt handling for pcu hpd interrupt. > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 56 ++++++++++++++++++++++++++++++++++++++--- > drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++ > 2 files changed, 65 insertions(+), 3 deletions(-) > mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > old mode 100644 > new mode 100755 > index cb8a75f..2f9b604 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -110,9 +110,9 @@ > > /* BXT hpd list */ > static const u32 hpd_bxt[HPD_NUM_PINS] = { > - [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA, > - [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, > - [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC > + [HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA), > + [HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB), > + [HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC) These are bits programmed to GEN8_DE_PORT_*, so adding the PCU bits here is bogus. > }; > > /* IIR can theoretically queue up two events. Be paranoid. */ > @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv, > DRM_ERROR("The master control interrupt lied (DE PORT)!\n"); > } > > + if (master_ctl & GEN8_PCU_IRQ) { > + iir = I915_READ(GEN8_PCU_IIR); > + if (iir) { > + u32 tmp_mask; > + > + I915_WRITE(GEN8_PCU_IIR, iir); > + ret = IRQ_HANDLED; > + if (IS_BROXTON(dev_priv)) { > + tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK; > + if (tmp_mask) > + bxt_hpd_irq_handler(dev_priv, tmp_mask, > + hpd_bxt); > + } else > + DRM_ERROR("Unexpected PCU interrupt\n"); > + } else > + DRM_ERROR("The master control interrupt lied (PCU)!\n"); > + } > + > for_each_pipe(dev_priv, pipe) { > u32 flip_done, fault_errors; > > @@ -4294,6 +4312,19 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > dev_priv->pm.irqs_enabled = false; > } > > +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv) > +{ > + u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier; > + > + de_pcu_hpd_enable_mask = GEN9_DE_PCU_PORTA_HOTPLUG | > + GEN9_DE_PCU_PORTB_HOTPLUG | > + GEN9_DE_PCU_PORTC_HOTPLUG; > + > + de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0); Typo. > + de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask); > + GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier); > +} > + > /** > * intel_runtime_pm_disable_interrupts - runtime interrupt disabling > * @dev_priv: i915 device instance > @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > */ > void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) > { > + unsigned long flags = 0; > + > dev_priv->drm.driver->irq_uninstall(&dev_priv->drm); > dev_priv->pm.irqs_enabled = false; > + > + if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) { > + > + /* Enable HPD related interrupts during DC9 for HPD wakeup */ > + > + I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > + POSTING_READ(GEN8_MASTER_IRQ); > + > + spin_lock_irqsave(&dev_priv->irq_lock, flags); > + if (dev_priv->display.hpd_irq_setup) > + dev_priv->display.hpd_irq_setup(dev_priv); > + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > + > + bxt_enable_pcu_interrupt(dev_priv); The lock should be around the whole IRQ programming sequence. > + > + dev_priv->pm.irqs_enabled = true; > + } > synchronize_irq(dev_priv->drm.irq); This should come before the IRQ enabling. > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 3361d7f..df89025 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6030,6 +6030,18 @@ enum { > #define GEN8_PCU_IIR _MMIO(0x444e8) > #define GEN8_PCU_IER _MMIO(0x444ec) > > +/* BXT PCU DC9 hotplug control */ > +#define BXT_PCU_DC9_HP_DDIA (1<<31) > +#define BXT_PCU_DC9_HP_DDIB (1<<30) > +#define BXT_PCU_DC9_HP_DDIC (1<<29) > +#define BXT_PCU_DC9_HOTPLUG_MASK (BXT_PCU_DC9_HP_DDIA | \ > + BXT_PCU_DC9_HP_DDIB | \ > + BXT_PCU_DC9_HP_DDIC) > + > +#define GEN9_DE_PCU_PORTA_HOTPLUG (1 << 31) > +#define GEN9_DE_PCU_PORTB_HOTPLUG (1 << 30) > +#define GEN9_DE_PCU_PORTC_HOTPLUG (1 << 29) Redundant copy of the BXT_ defines? > + > #define ILK_DISPLAY_CHICKEN2 _MMIO(0x42004) > /* Required on all Ironlake and Sandybridge according to the B-Spec. */ > #define ILK_ELPIN_409_SELECT (1 << 25)
On Wed, Nov 23, 2016 at 09:48:27PM +0530, Animesh Manna wrote: > While suspending the device hpd related interrupts are enabled > to get the interrupt when device is in suspend state. > > Though display is in DC9 but system can be in S0 or S0i3 state. > Hot plug during S0 state will generate de_port_interrupt but if > system is in S0i3 state then display driver will get hotplug > interrupt as pcu_hpd_interrupt which will come via pmc. So > added the interrupt handling for pcu hpd interrupt. > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 56 ++++++++++++++++++++++++++++++++++++++--- > drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++ > 2 files changed, 65 insertions(+), 3 deletions(-) > mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > old mode 100644 > new mode 100755 > index cb8a75f..2f9b604 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -110,9 +110,9 @@ > > /* BXT hpd list */ > static const u32 hpd_bxt[HPD_NUM_PINS] = { > - [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA, > - [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, > - [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC > + [HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA), > + [HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB), > + [HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC) hpd_bxt_pcu[] > }; > > /* IIR can theoretically queue up two events. Be paranoid. */ > @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv, > DRM_ERROR("The master control interrupt lied (DE PORT)!\n"); > } > > + if (master_ctl & GEN8_PCU_IRQ) { > + iir = I915_READ(GEN8_PCU_IIR); > + if (iir) { > + u32 tmp_mask; > + Please add a proper pcu irq ack/handler pair. I actually have such a thing sitting on a branch: git://github.com/vsyrjala/linux.git pcode_irq > + I915_WRITE(GEN8_PCU_IIR, iir); > + ret = IRQ_HANDLED; > + if (IS_BROXTON(dev_priv)) { > + tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK; > + if (tmp_mask) > + bxt_hpd_irq_handler(dev_priv, tmp_mask, > + hpd_bxt); Umm. Does the PCH_HOTPLUG reg actually work in this "hpd redirected to pcu" state? As in are we going to detect long vs. short pulses correctly if you just use bxt_hpd_irq_handler()? > + } else > + DRM_ERROR("Unexpected PCU interrupt\n"); > + } else > + DRM_ERROR("The master control interrupt lied (PCU)!\n"); > + } > + > for_each_pipe(dev_priv, pipe) { > u32 flip_done, fault_errors; > > @@ -4294,6 +4312,19 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > dev_priv->pm.irqs_enabled = false; > } > > +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv) > +{ > + u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier; > + > + de_pcu_hpd_enable_mask = GEN9_DE_PCU_PORTA_HOTPLUG | > + GEN9_DE_PCU_PORTB_HOTPLUG | > + GEN9_DE_PCU_PORTC_HOTPLUG; > + > + de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0); > + de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask); You'll want a bdw_update_pcu_irq() or some such thing. > + GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier); > +} > + > /** > * intel_runtime_pm_disable_interrupts - runtime interrupt disabling > * @dev_priv: i915 device instance > @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > */ > void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) > { > + unsigned long flags = 0; > + > dev_priv->drm.driver->irq_uninstall(&dev_priv->drm); > dev_priv->pm.irqs_enabled = false; > + > + if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) { > + > + /* Enable HPD related interrupts during DC9 for HPD wakeup */ > + > + I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > + POSTING_READ(GEN8_MASTER_IRQ); > + > + spin_lock_irqsave(&dev_priv->irq_lock, flags); > + if (dev_priv->display.hpd_irq_setup) > + dev_priv->display.hpd_irq_setup(dev_priv); > + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > + > + bxt_enable_pcu_interrupt(dev_priv); > + > + dev_priv->pm.irqs_enabled = true; > + } > synchronize_irq(dev_priv->drm.irq); > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 3361d7f..df89025 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6030,6 +6030,18 @@ enum { > #define GEN8_PCU_IIR _MMIO(0x444e8) > #define GEN8_PCU_IER _MMIO(0x444ec) > > +/* BXT PCU DC9 hotplug control */ > +#define BXT_PCU_DC9_HP_DDIA (1<<31) > +#define BXT_PCU_DC9_HP_DDIB (1<<30) > +#define BXT_PCU_DC9_HP_DDIC (1<<29) > +#define BXT_PCU_DC9_HOTPLUG_MASK (BXT_PCU_DC9_HP_DDIA | \ > + BXT_PCU_DC9_HP_DDIB | \ > + BXT_PCU_DC9_HP_DDIC) > + > +#define GEN9_DE_PCU_PORTA_HOTPLUG (1 << 31) > +#define GEN9_DE_PCU_PORTB_HOTPLUG (1 << 30) > +#define GEN9_DE_PCU_PORTC_HOTPLUG (1 << 29) Two names for the same bits? > + > #define ILK_DISPLAY_CHICKEN2 _MMIO(0x42004) > /* Required on all Ironlake and Sandybridge according to the B-Spec. */ > #define ILK_ELPIN_409_SELECT (1 << 25) > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 11/23/2016 10:31 PM, Imre Deak wrote: > On Wed, 2016-11-23 at 21:48 +0530, Animesh Manna wrote: >> While suspending the device hpd related interrupts are enabled >> to get the interrupt when device is in suspend state. >> >> Though display is in DC9 but system can be in S0 or S0i3 state. >> Hot plug during S0 state will generate de_port_interrupt but if >> system is in S0i3 state then display driver will get hotplug >> interrupt as pcu_hpd_interrupt which will come via pmc. So >> added the interrupt handling for pcu hpd interrupt. >> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> >> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 56 ++++++++++++++++++++++++++++++++++++++--- >> drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++ >> 2 files changed, 65 insertions(+), 3 deletions(-) >> mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> old mode 100644 >> new mode 100755 >> index cb8a75f..2f9b604 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -110,9 +110,9 @@ >> >> /* BXT hpd list */ >> static const u32 hpd_bxt[HPD_NUM_PINS] = { >> - [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA, >> - [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, >> - [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC >> + [HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA), >> + [HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB), >> + [HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC) > These are bits programmed to GEN8_DE_PORT_*, so adding the PCU bits > here is bogus. Thanks Imre for review. I understood the "hpd_bxt" array is to store all bits which validate hpd interrupt in irq_handler from the respective port. Previously only from DE_PORT interrupt used to come and after implementing HPD as wake feature interrupt source will be both DE_PORT and PCU. So added pcu related bits in the same array. Do you want two different array for DE_PORT and PCU. I can do it by creating a new array named "hpd_bxt_pcu" and change the existing one as "hpd_bxt_de". Please let me know your suggestion. >> }; >> >> /* IIR can theoretically queue up two events. Be paranoid. */ >> @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv, >> DRM_ERROR("The master control interrupt lied (DE PORT)!\n"); >> } >> >> + if (master_ctl & GEN8_PCU_IRQ) { >> + iir = I915_READ(GEN8_PCU_IIR); >> + if (iir) { >> + u32 tmp_mask; >> + >> + I915_WRITE(GEN8_PCU_IIR, iir); >> + ret = IRQ_HANDLED; >> + if (IS_BROXTON(dev_priv)) { >> + tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK; >> + if (tmp_mask) >> + bxt_hpd_irq_handler(dev_priv, tmp_mask, >> + hpd_bxt); >> + } else >> + DRM_ERROR("Unexpected PCU interrupt\n"); >> + } else >> + DRM_ERROR("The master control interrupt lied (PCU)!\n"); >> + } >> + >> for_each_pipe(dev_priv, pipe) { >> u32 flip_done, fault_errors; >> >> @@ -4294,6 +4312,19 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) >> dev_priv->pm.irqs_enabled = false; >> } >> >> +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv) >> +{ >> + u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier; >> + >> + de_pcu_hpd_enable_mask = GEN9_DE_PCU_PORTA_HOTPLUG | >> + GEN9_DE_PCU_PORTB_HOTPLUG | >> + GEN9_DE_PCU_PORTC_HOTPLUG; >> + >> + de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0); > Typo. Will remove "de" tag from all pcu related variables, for anything else let me know. GEN8 and GEN9 is using same pcu interrupt registers so used the same macro "GEN8_PCU_IMR". > >> + de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask); >> + GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier); >> +} >> + >> /** >> * intel_runtime_pm_disable_interrupts - runtime interrupt disabling >> * @dev_priv: i915 device instance >> @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) >> */ >> void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) >> { >> + unsigned long flags = 0; >> + >> dev_priv->drm.driver->irq_uninstall(&dev_priv->drm); >> dev_priv->pm.irqs_enabled = false; >> + >> + if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) { >> + >> + /* Enable HPD related interrupts during DC9 for HPD wakeup */ >> + >> + I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); >> + POSTING_READ(GEN8_MASTER_IRQ); >> + >> + spin_lock_irqsave(&dev_priv->irq_lock, flags); >> + if (dev_priv->display.hpd_irq_setup) >> + dev_priv->display.hpd_irq_setup(dev_priv); >> + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); >> + >> + bxt_enable_pcu_interrupt(dev_priv); > The lock should be around the whole IRQ programming sequence. Just to make happy the "assert_spin_locked" which present inside hpd_irq_setup() taken the irq_lock. As we are disabling all the inerrupts in initial stage, is it ok to take lock before and after enabling display port and pcu interrupts. BTW, intel_runtime_pm_disable_interrupts function don't have any lock before. > >> + >> + dev_priv->pm.irqs_enabled = true; >> + } >> synchronize_irq(dev_priv->drm.irq); > This should come before the IRQ enabling. Sure, will do it in next patchset. > >> } >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 3361d7f..df89025 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -6030,6 +6030,18 @@ enum { >> #define GEN8_PCU_IIR _MMIO(0x444e8) >> #define GEN8_PCU_IER _MMIO(0x444ec) >> >> +/* BXT PCU DC9 hotplug control */ >> +#define BXT_PCU_DC9_HP_DDIA (1<<31) >> +#define BXT_PCU_DC9_HP_DDIB (1<<30) >> +#define BXT_PCU_DC9_HP_DDIC (1<<29) >> +#define BXT_PCU_DC9_HOTPLUG_MASK (BXT_PCU_DC9_HP_DDIA | \ >> + BXT_PCU_DC9_HP_DDIB | \ >> + BXT_PCU_DC9_HP_DDIC) >> + >> +#define GEN9_DE_PCU_PORTA_HOTPLUG (1 << 31) >> +#define GEN9_DE_PCU_PORTB_HOTPLUG (1 << 30) >> +#define GEN9_DE_PCU_PORTC_HOTPLUG (1 << 29) > Redundant copy of the BXT_ defines? will remove in the next patchset. > >> + >> #define ILK_DISPLAY_CHICKEN2 _MMIO(0x42004) >> /* Required on all Ironlake and Sandybridge according to the B-Spec. */ >> #define ILK_ELPIN_409_SELECT (1 << 25)
On ma, 2016-11-28 at 19:09 +0530, Animesh Manna wrote: > > On 11/23/2016 10:31 PM, Imre Deak wrote: > > On Wed, 2016-11-23 at 21:48 +0530, Animesh Manna wrote: > > > While suspending the device hpd related interrupts are enabled > > > to get the interrupt when device is in suspend state. > > > > > > Though display is in DC9 but system can be in S0 or S0i3 state. > > > Hot plug during S0 state will generate de_port_interrupt but if > > > system is in S0i3 state then display driver will get hotplug > > > interrupt as pcu_hpd_interrupt which will come via pmc. So > > > added the interrupt handling for pcu hpd interrupt. > > > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_irq.c | 56 ++++++++++++++++++++++++++++++++++++++--- > > > drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++ > > > 2 files changed, 65 insertions(+), 3 deletions(-) > > > mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > > old mode 100644 > > > new mode 100755 > > > index cb8a75f..2f9b604 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -110,9 +110,9 @@ > > > > > > /* BXT hpd list */ > > > static const u32 hpd_bxt[HPD_NUM_PINS] = { > > > - [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA, > > > - [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, > > > - [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC > > > + [HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA), > > > + [HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB), > > > + [HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC) > > These are bits programmed to GEN8_DE_PORT_*, so adding the PCU bits > > here is bogus. > Thanks Imre for review. I understood the "hpd_bxt" array is to store all > bits which validate hpd interrupt in irq_handler from the respective port. > Previously only from DE_PORT interrupt used to come and after > implementing HPD as wake feature interrupt source will be both DE_PORT > and PCU. > So added pcu related bits in the same array. > Do you want two different array for DE_PORT and PCU. I can do it by > creating a new array named "hpd_bxt_pcu" and change the existing one as > "hpd_bxt_de". > Please let me know your suggestion. The problem is that - for example - bxt_hpd_irq_setup() will program now these PCU bits to GEN8_DE_PORT_IMR which is wrong. There's also a WARN that will trigger because of this. Yes, using a separate struct would work I think. > > > }; > > > > > > /* IIR can theoretically queue up two events. Be paranoid. */ > > > @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv, > > > DRM_ERROR("The master control interrupt lied (DE PORT)!\n"); > > > } > > > > > > + if (master_ctl & GEN8_PCU_IRQ) { > > > + iir = I915_READ(GEN8_PCU_IIR); > > > + if (iir) { > > > + u32 tmp_mask; > > > + > > > + I915_WRITE(GEN8_PCU_IIR, iir); > > > + ret = IRQ_HANDLED; > > > + if (IS_BROXTON(dev_priv)) { > > > + tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK; > > > + if (tmp_mask) > > > + bxt_hpd_irq_handler(dev_priv, tmp_mask, > > > + hpd_bxt); > > > + } else > > > + DRM_ERROR("Unexpected PCU interrupt\n"); > > > + } else > > > + DRM_ERROR("The master control interrupt lied (PCU)!\n"); > > > + } > > > + > > > for_each_pipe(dev_priv, pipe) { > > > u32 flip_done, fault_errors; > > > > > > @@ -4294,6 +4312,19 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > > > dev_priv->pm.irqs_enabled = false; > > > } > > > > > > +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv) > > > +{ > > > + u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier; > > > + > > > + de_pcu_hpd_enable_mask = GEN9_DE_PCU_PORTA_HOTPLUG | > > > + GEN9_DE_PCU_PORTB_HOTPLUG | > > > + GEN9_DE_PCU_PORTC_HOTPLUG; > > > + > > > + de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0); > > Typo. > Will remove "de" tag from all pcu related variables, for anything else > let me know. > GEN8 and GEN9 is using same pcu interrupt registers so used the same > macro "GEN8_PCU_IMR". I meant the '& 0x0' part, looks like '& ~de_pcu_hpd_enable_mask' is what you meant. > > > > > + de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask); > > > + GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier); > > > +} > > > + > > > /** > > > * intel_runtime_pm_disable_interrupts - runtime interrupt disabling > > > * @dev_priv: i915 device instance > > > @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > > > */ > > > void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) > > > { > > > + unsigned long flags = 0; > > > + > > > dev_priv->drm.driver->irq_uninstall(&dev_priv->drm); > > > dev_priv->pm.irqs_enabled = false; > > > + > > > + if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) { > > > + > > > + /* Enable HPD related interrupts during DC9 for HPD wakeup */ > > > + > > > + I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > > > + POSTING_READ(GEN8_MASTER_IRQ); > > > + > > > + spin_lock_irqsave(&dev_priv->irq_lock, flags); > > > + if (dev_priv->display.hpd_irq_setup) > > > + dev_priv->display.hpd_irq_setup(dev_priv); > > > + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > > > + > > > + bxt_enable_pcu_interrupt(dev_priv); > > The lock should be around the whole IRQ programming sequence. > Just to make happy the "assert_spin_locked" which present inside > hpd_irq_setup() taken the irq_lock. > As we are disabling all the inerrupts in initial stage, is it ok to take > lock before and after enabling display port and pcu interrupts. > BTW, intel_runtime_pm_disable_interrupts function don't have any lock > before. The problem is that with the above sequence once you enable interrupts the handler may be called with irqs_enabled = false and ignoring the interrupt. But looking at it again, taking the lock around the whole sequence won't actually solve that, you need to set GEN8_MASTER_IRQ_CONTROL as the last step (and move synchronize_irq before the enabling). > > > > > + > > > + dev_priv->pm.irqs_enabled = true; > > > + } > > > synchronize_irq(dev_priv->drm.irq); > > This should come before the IRQ enabling. > Sure, will do it in next patchset. > > > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index 3361d7f..df89025 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -6030,6 +6030,18 @@ enum { > > > #define GEN8_PCU_IIR _MMIO(0x444e8) > > > #define GEN8_PCU_IER _MMIO(0x444ec) > > > > > > +/* BXT PCU DC9 hotplug control */ > > > +#define BXT_PCU_DC9_HP_DDIA (1<<31) > > > +#define BXT_PCU_DC9_HP_DDIB (1<<30) > > > +#define BXT_PCU_DC9_HP_DDIC (1<<29) > > > +#define BXT_PCU_DC9_HOTPLUG_MASK (BXT_PCU_DC9_HP_DDIA | \ > > > + BXT_PCU_DC9_HP_DDIB | \ > > > + BXT_PCU_DC9_HP_DDIC) > > > + > > > +#define GEN9_DE_PCU_PORTA_HOTPLUG (1 << 31) > > > +#define GEN9_DE_PCU_PORTB_HOTPLUG (1 << 30) > > > +#define GEN9_DE_PCU_PORTC_HOTPLUG (1 << 29) > > Redundant copy of the BXT_ defines? > will remove in the next patchset. > > > > > + > > > #define ILK_DISPLAY_CHICKEN2 _MMIO(0x42004) > > > /* Required on all Ironlake and Sandybridge according to the B-Spec. */ > > > #define ILK_ELPIN_409_SELECT (1 << 25) >
On 11/23/2016 10:40 PM, Ville Syrjälä wrote: > On Wed, Nov 23, 2016 at 09:48:27PM +0530, Animesh Manna wrote: >> While suspending the device hpd related interrupts are enabled >> to get the interrupt when device is in suspend state. >> >> Though display is in DC9 but system can be in S0 or S0i3 state. >> Hot plug during S0 state will generate de_port_interrupt but if >> system is in S0i3 state then display driver will get hotplug >> interrupt as pcu_hpd_interrupt which will come via pmc. So >> added the interrupt handling for pcu hpd interrupt. >> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> >> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 56 ++++++++++++++++++++++++++++++++++++++--- >> drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++ >> 2 files changed, 65 insertions(+), 3 deletions(-) >> mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> old mode 100644 >> new mode 100755 >> index cb8a75f..2f9b604 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -110,9 +110,9 @@ >> >> /* BXT hpd list */ >> static const u32 hpd_bxt[HPD_NUM_PINS] = { >> - [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA, >> - [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, >> - [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC >> + [HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA), >> + [HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB), >> + [HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC) > hpd_bxt_pcu[] Thanks Ville for review. yes, will add a separate struct for pcu. > >> }; >> >> /* IIR can theoretically queue up two events. Be paranoid. */ >> @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv, >> DRM_ERROR("The master control interrupt lied (DE PORT)!\n"); >> } >> >> + if (master_ctl & GEN8_PCU_IRQ) { >> + iir = I915_READ(GEN8_PCU_IIR); >> + if (iir) { >> + u32 tmp_mask; >> + > Please add a proper pcu irq ack/handler pair. I actually have such a > thing sitting on a branch: > > git://github.com/vsyrjala/linux.git pcode_irq Downloaded the code, can you please help with the commit you are referring. I can see some old commit as last commit (will try to sync with you over irc): amanna@DispDev:~/PROJECT_CODEBASE/drm-intel-ville/pcode_irq$ git log commit 20624d17963c737bbd9f242402bf3136cb664d10 Merge: 9a08da1 f4274e2 Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Wed Apr 8 15:12:25 2015 -0700 Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux > >> + I915_WRITE(GEN8_PCU_IIR, iir); >> + ret = IRQ_HANDLED; >> + if (IS_BROXTON(dev_priv)) { >> + tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK; >> + if (tmp_mask) >> + bxt_hpd_irq_handler(dev_priv, tmp_mask, >> + hpd_bxt); > Umm. Does the PCH_HOTPLUG reg actually work in this "hpd redirected to > pcu" state? As in are we going to detect long vs. short pulses correctly > if you just use bxt_hpd_irq_handler()? After programming the PMC_HPD_CTL register we can get PCU interrupt only during S0ix for HPD. AFAIK for both DP and HDMI we will be getting long pulse for hotplug. And In non-S0ix scenario HPD interrupt will always come from DE_PORT. So do we need to handle short pulse interrupt? > >> + } else >> + DRM_ERROR("Unexpected PCU interrupt\n"); >> + } else >> + DRM_ERROR("The master control interrupt lied (PCU)!\n"); >> + } >> + >> for_each_pipe(dev_priv, pipe) { >> u32 flip_done, fault_errors; >> >> @@ -4294,6 +4312,19 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) >> dev_priv->pm.irqs_enabled = false; >> } >> >> +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv) >> +{ >> + u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier; >> + >> + de_pcu_hpd_enable_mask = GEN9_DE_PCU_PORTA_HOTPLUG | >> + GEN9_DE_PCU_PORTB_HOTPLUG | >> + GEN9_DE_PCU_PORTC_HOTPLUG; >> + >> + de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0); >> + de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask); > You'll want a bdw_update_pcu_irq() or some such thing. Yes, better to use bdw tag, will update in next patchset. > >> + GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier); >> +} >> + >> /** >> * intel_runtime_pm_disable_interrupts - runtime interrupt disabling >> * @dev_priv: i915 device instance >> @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) >> */ >> void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) >> { >> + unsigned long flags = 0; >> + >> dev_priv->drm.driver->irq_uninstall(&dev_priv->drm); >> dev_priv->pm.irqs_enabled = false; >> + >> + if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) { >> + >> + /* Enable HPD related interrupts during DC9 for HPD wakeup */ >> + >> + I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); >> + POSTING_READ(GEN8_MASTER_IRQ); >> + >> + spin_lock_irqsave(&dev_priv->irq_lock, flags); >> + if (dev_priv->display.hpd_irq_setup) >> + dev_priv->display.hpd_irq_setup(dev_priv); >> + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); >> + >> + bxt_enable_pcu_interrupt(dev_priv); >> + >> + dev_priv->pm.irqs_enabled = true; >> + } >> synchronize_irq(dev_priv->drm.irq); >> } >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 3361d7f..df89025 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -6030,6 +6030,18 @@ enum { >> #define GEN8_PCU_IIR _MMIO(0x444e8) >> #define GEN8_PCU_IER _MMIO(0x444ec) >> >> +/* BXT PCU DC9 hotplug control */ >> +#define BXT_PCU_DC9_HP_DDIA (1<<31) >> +#define BXT_PCU_DC9_HP_DDIB (1<<30) >> +#define BXT_PCU_DC9_HP_DDIC (1<<29) >> +#define BXT_PCU_DC9_HOTPLUG_MASK (BXT_PCU_DC9_HP_DDIA | \ >> + BXT_PCU_DC9_HP_DDIB | \ >> + BXT_PCU_DC9_HP_DDIC) >> + >> +#define GEN9_DE_PCU_PORTA_HOTPLUG (1 << 31) >> +#define GEN9_DE_PCU_PORTB_HOTPLUG (1 << 30) >> +#define GEN9_DE_PCU_PORTC_HOTPLUG (1 << 29) > Two names for the same bits? will correct it. - Animesh > >> + >> #define ILK_DISPLAY_CHICKEN2 _MMIO(0x42004) >> /* Required on all Ironlake and Sandybridge according to the B-Spec. */ >> #define ILK_ELPIN_409_SELECT (1 << 25) >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Nov 28, 2016 at 09:19:36PM +0530, Animesh Manna wrote: > > > On 11/23/2016 10:40 PM, Ville Syrjälä wrote: > > On Wed, Nov 23, 2016 at 09:48:27PM +0530, Animesh Manna wrote: > >> While suspending the device hpd related interrupts are enabled > >> to get the interrupt when device is in suspend state. > >> > >> Though display is in DC9 but system can be in S0 or S0i3 state. > >> Hot plug during S0 state will generate de_port_interrupt but if > >> system is in S0i3 state then display driver will get hotplug > >> interrupt as pcu_hpd_interrupt which will come via pmc. So > >> added the interrupt handling for pcu hpd interrupt. > >> > >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> > >> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_irq.c | 56 ++++++++++++++++++++++++++++++++++++++--- > >> drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++ > >> 2 files changed, 65 insertions(+), 3 deletions(-) > >> mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c > >> > >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > >> old mode 100644 > >> new mode 100755 > >> index cb8a75f..2f9b604 > >> --- a/drivers/gpu/drm/i915/i915_irq.c > >> +++ b/drivers/gpu/drm/i915/i915_irq.c > >> @@ -110,9 +110,9 @@ > >> > >> /* BXT hpd list */ > >> static const u32 hpd_bxt[HPD_NUM_PINS] = { > >> - [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA, > >> - [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, > >> - [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC > >> + [HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA), > >> + [HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB), > >> + [HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC) > > hpd_bxt_pcu[] > Thanks Ville for review. > yes, will add a separate struct for pcu. > > > >> }; > >> > >> /* IIR can theoretically queue up two events. Be paranoid. */ > >> @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv, > >> DRM_ERROR("The master control interrupt lied (DE PORT)!\n"); > >> } > >> > >> + if (master_ctl & GEN8_PCU_IRQ) { > >> + iir = I915_READ(GEN8_PCU_IIR); > >> + if (iir) { > >> + u32 tmp_mask; > >> + > > Please add a proper pcu irq ack/handler pair. I actually have such a > > thing sitting on a branch: > > > > git://github.com/vsyrjala/linux.git pcode_irq > Downloaded the code, can you please help with the commit you are referring. > I can see some old commit as last commit (will try to sync with you over > irc): > amanna@DispDev:~/PROJECT_CODEBASE/drm-intel-ville/pcode_irq$ git log > commit 20624d17963c737bbd9f242402bf3136cb664d10 > Merge: 9a08da1 f4274e2 > Author: Linus Torvalds <torvalds@linux-foundation.org> > Date: Wed Apr 8 15:12:25 2015 -0700 > > Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux git checkout pcode_irq > > > > > >> + I915_WRITE(GEN8_PCU_IIR, iir); > >> + ret = IRQ_HANDLED; > >> + if (IS_BROXTON(dev_priv)) { > >> + tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK; > >> + if (tmp_mask) > >> + bxt_hpd_irq_handler(dev_priv, tmp_mask, > >> + hpd_bxt); > > Umm. Does the PCH_HOTPLUG reg actually work in this "hpd redirected to > > pcu" state? As in are we going to detect long vs. short pulses correctly > > if you just use bxt_hpd_irq_handler()? > After programming the PMC_HPD_CTL register we can get PCU interrupt only > during S0ix for HPD. > AFAIK for both DP and HDMI we will be getting long pulse for hotplug. > And In non-S0ix scenario HPD interrupt will always come from DE_PORT. So > do we need to handle short pulse interrupt? Short pulse probably isn't needed during s0ix since I presume we can't have any active displays anyway. The only slight concern might be DP branch devices which would signal downstream port HPDs via a short pulse. I have a vague recollection that there might be a way to aske them for a long HPD instead via some DPCD register. > > > >> + } else > >> + DRM_ERROR("Unexpected PCU interrupt\n"); > >> + } else > >> + DRM_ERROR("The master control interrupt lied (PCU)!\n"); > >> + } > >> + > >> for_each_pipe(dev_priv, pipe) { > >> u32 flip_done, fault_errors; > >> > >> @@ -4294,6 +4312,19 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > >> dev_priv->pm.irqs_enabled = false; > >> } > >> > >> +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv) > >> +{ > >> + u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier; > >> + > >> + de_pcu_hpd_enable_mask = GEN9_DE_PCU_PORTA_HOTPLUG | > >> + GEN9_DE_PCU_PORTB_HOTPLUG | > >> + GEN9_DE_PCU_PORTC_HOTPLUG; > >> + > >> + de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0); > >> + de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask); > > You'll want a bdw_update_pcu_irq() or some such thing. > Yes, better to use bdw tag, will update in next patchset. > > > >> + GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier); > >> +} > >> + > >> /** > >> * intel_runtime_pm_disable_interrupts - runtime interrupt disabling > >> * @dev_priv: i915 device instance > >> @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > >> */ > >> void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) > >> { > >> + unsigned long flags = 0; > >> + > >> dev_priv->drm.driver->irq_uninstall(&dev_priv->drm); > >> dev_priv->pm.irqs_enabled = false; > >> + > >> + if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) { > >> + > >> + /* Enable HPD related interrupts during DC9 for HPD wakeup */ > >> + > >> + I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > >> + POSTING_READ(GEN8_MASTER_IRQ); > >> + > >> + spin_lock_irqsave(&dev_priv->irq_lock, flags); > >> + if (dev_priv->display.hpd_irq_setup) > >> + dev_priv->display.hpd_irq_setup(dev_priv); > >> + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > >> + > >> + bxt_enable_pcu_interrupt(dev_priv); > >> + > >> + dev_priv->pm.irqs_enabled = true; > >> + } > >> synchronize_irq(dev_priv->drm.irq); > >> } > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > >> index 3361d7f..df89025 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -6030,6 +6030,18 @@ enum { > >> #define GEN8_PCU_IIR _MMIO(0x444e8) > >> #define GEN8_PCU_IER _MMIO(0x444ec) > >> > >> +/* BXT PCU DC9 hotplug control */ > >> +#define BXT_PCU_DC9_HP_DDIA (1<<31) > >> +#define BXT_PCU_DC9_HP_DDIB (1<<30) > >> +#define BXT_PCU_DC9_HP_DDIC (1<<29) > >> +#define BXT_PCU_DC9_HOTPLUG_MASK (BXT_PCU_DC9_HP_DDIA | \ > >> + BXT_PCU_DC9_HP_DDIB | \ > >> + BXT_PCU_DC9_HP_DDIC) > >> + > >> +#define GEN9_DE_PCU_PORTA_HOTPLUG (1 << 31) > >> +#define GEN9_DE_PCU_PORTB_HOTPLUG (1 << 30) > >> +#define GEN9_DE_PCU_PORTC_HOTPLUG (1 << 29) > > Two names for the same bits? > will correct it. > > - Animesh > > > >> + > >> #define ILK_DISPLAY_CHICKEN2 _MMIO(0x42004) > >> /* Required on all Ironlake and Sandybridge according to the B-Spec. */ > >> #define ILK_ELPIN_409_SELECT (1 << 25) > >> -- > >> 1.9.1 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c old mode 100644 new mode 100755 index cb8a75f..2f9b604 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -110,9 +110,9 @@ /* BXT hpd list */ static const u32 hpd_bxt[HPD_NUM_PINS] = { - [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA, - [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, - [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC + [HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA), + [HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB), + [HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC) }; /* IIR can theoretically queue up two events. Be paranoid. */ @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv, DRM_ERROR("The master control interrupt lied (DE PORT)!\n"); } + if (master_ctl & GEN8_PCU_IRQ) { + iir = I915_READ(GEN8_PCU_IIR); + if (iir) { + u32 tmp_mask; + + I915_WRITE(GEN8_PCU_IIR, iir); + ret = IRQ_HANDLED; + if (IS_BROXTON(dev_priv)) { + tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK; + if (tmp_mask) + bxt_hpd_irq_handler(dev_priv, tmp_mask, + hpd_bxt); + } else + DRM_ERROR("Unexpected PCU interrupt\n"); + } else + DRM_ERROR("The master control interrupt lied (PCU)!\n"); + } + for_each_pipe(dev_priv, pipe) { u32 flip_done, fault_errors; @@ -4294,6 +4312,19 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) dev_priv->pm.irqs_enabled = false; } +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv) +{ + u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier; + + de_pcu_hpd_enable_mask = GEN9_DE_PCU_PORTA_HOTPLUG | + GEN9_DE_PCU_PORTB_HOTPLUG | + GEN9_DE_PCU_PORTC_HOTPLUG; + + de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0); + de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask); + GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier); +} + /** * intel_runtime_pm_disable_interrupts - runtime interrupt disabling * @dev_priv: i915 device instance @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) */ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) { + unsigned long flags = 0; + dev_priv->drm.driver->irq_uninstall(&dev_priv->drm); dev_priv->pm.irqs_enabled = false; + + if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) { + + /* Enable HPD related interrupts during DC9 for HPD wakeup */ + + I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); + POSTING_READ(GEN8_MASTER_IRQ); + + spin_lock_irqsave(&dev_priv->irq_lock, flags); + if (dev_priv->display.hpd_irq_setup) + dev_priv->display.hpd_irq_setup(dev_priv); + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); + + bxt_enable_pcu_interrupt(dev_priv); + + dev_priv->pm.irqs_enabled = true; + } synchronize_irq(dev_priv->drm.irq); } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3361d7f..df89025 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6030,6 +6030,18 @@ enum { #define GEN8_PCU_IIR _MMIO(0x444e8) #define GEN8_PCU_IER _MMIO(0x444ec) +/* BXT PCU DC9 hotplug control */ +#define BXT_PCU_DC9_HP_DDIA (1<<31) +#define BXT_PCU_DC9_HP_DDIB (1<<30) +#define BXT_PCU_DC9_HP_DDIC (1<<29) +#define BXT_PCU_DC9_HOTPLUG_MASK (BXT_PCU_DC9_HP_DDIA | \ + BXT_PCU_DC9_HP_DDIB | \ + BXT_PCU_DC9_HP_DDIC) + +#define GEN9_DE_PCU_PORTA_HOTPLUG (1 << 31) +#define GEN9_DE_PCU_PORTB_HOTPLUG (1 << 30) +#define GEN9_DE_PCU_PORTC_HOTPLUG (1 << 29) + #define ILK_DISPLAY_CHICKEN2 _MMIO(0x42004) /* Required on all Ironlake and Sandybridge according to the B-Spec. */ #define ILK_ELPIN_409_SELECT (1 << 25)