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@xxxxxxxxx>
> >> Signed-off-by: A.Sunil Kamath <sunil.kamath@xxxxxxxxx>
> >> ---
> >> 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@xxxxxxxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxxxx
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
