| Message ID | 1529530565-8985-1-git-send-email-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Qua, 2018-06-20 às 14:36 -0700, Anusha Srivatsa escreveu: > This patch addresses Interrupts from south display engine (SDE). > > ICP has two registers - SHOTPLUG_CTL_DDI and SHOTPLUG_CTL_TC. > Introduce these registers and their intended values. > > Introduce icp_irq_handler(). > > v2: > - remove redundant register defines.(Lucas) > - Change register names to be more consistent with > previous platforms (Lucas) > > Cc: Lucas De Marchi <lucas.de.marchi@gmail.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > [Paulo: coding style bikesheds and rebases]. > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 134 > +++++++++++++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/i915_reg.h | 40 ++++++++++++ > 2 files changed, 172 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c > index 46aaef5..7a7c4a2 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -122,6 +122,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = { > [HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG > }; > > +static const u32 hpd_icp[HPD_NUM_PINS] = { > + [HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP, > + [HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP, > + [HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP, > + [HPD_PORT_D] = SDE_TC2_HOTPLUG_ICP, > + [HPD_PORT_E] = SDE_TC3_HOTPLUG_ICP, > + [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP > +}; > + > /* IIR can theoretically queue up two events. Be paranoid. */ > #define GEN8_IRQ_RESET_NDX(type, which) do { \ > I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \ > @@ -1586,6 +1595,34 @@ static bool bxt_port_hotplug_long_detect(enum > port port, u32 val) > } > } > > +static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 > val) > +{ > + switch (port) { > + case PORT_A: > + return val & ICP_DDIA_HPD_LONG_DETECT; > + case PORT_B: > + return val & ICP_DDIB_HPD_LONG_DETECT; > + default: > + return false; > + } > +} > + > +static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val) > +{ > + switch (port) { > + case PORT_C: > + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1); > + case PORT_D: > + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2); > + case PORT_E: > + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3); > + case PORT_F: > + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4); > + default: > + return false; > + } > +} > + > static bool spt_port_hotplug2_long_detect(enum port port, u32 val) > { > switch (port) { > @@ -2385,6 +2422,43 @@ static void cpt_irq_handler(struct > drm_i915_private *dev_priv, u32 pch_iir) > cpt_serr_int_handler(dev_priv); > } > > +static void icp_irq_handler(struct drm_i915_private *dev_priv, u32 > pch_iir) > +{ > + u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP; > + u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP; > + u32 pin_mask = 0, long_mask = 0; > + > + if (ddi_hotplug_trigger) { > + u32 dig_hotplug_reg; > + > + dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI); > + I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg); > + > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, > + ddi_hotplug_trigger, > + dig_hotplug_reg, hpd_icp, > + icp_ddi_port_hotplug_long_detect) > ; > + } > + > + if (tc_hotplug_trigger) { > + u32 dig_hotplug_reg; > + > + dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC); > + I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg); > + > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, > + tc_hotplug_trigger, > + dig_hotplug_reg, hpd_icp, > + icp_tc_port_hotplug_long_detect); > + } > + > + if (pin_mask) > + intel_hpd_irq_handler(dev_priv, pin_mask, > long_mask); > + > + if (pch_iir & SDE_GMBUS_ICP) > + gmbus_irq_handler(dev_priv); > +} > + > static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 > pch_iir) > { > u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT & > @@ -2804,8 +2878,11 @@ gen8_de_irq_handler(struct drm_i915_private > *dev_priv, u32 master_ctl) > I915_WRITE(SDEIIR, iir); > ret = IRQ_HANDLED; > > - if (HAS_PCH_SPT(dev_priv) || > HAS_PCH_KBP(dev_priv) || > - HAS_PCH_CNP(dev_priv)) > + if (HAS_PCH_ICP(dev_priv)) > + icp_irq_handler(dev_priv, iir); > + else if (HAS_PCH_SPT(dev_priv) || > + HAS_PCH_KBP(dev_priv) || > + HAS_PCH_CNP(dev_priv)) > spt_irq_handler(dev_priv, iir); > else > cpt_irq_handler(dev_priv, iir); > @@ -3584,6 +3661,9 @@ static void gen11_irq_reset(struct drm_device > *dev) > GEN3_IRQ_RESET(GEN11_DE_HPD_); > GEN3_IRQ_RESET(GEN11_GU_MISC_); > GEN3_IRQ_RESET(GEN8_PCU_); > + > + if (HAS_PCH_ICP(dev_priv)) > + GEN3_IRQ_RESET(SDE); > } > > void gen8_irq_power_well_post_enable(struct drm_i915_private > *dev_priv, > @@ -3700,6 +3780,35 @@ static void ibx_hpd_irq_setup(struct > drm_i915_private *dev_priv) > ibx_hpd_detection_setup(dev_priv); > } > > +static void icp_hpd_detection_setup(struct drm_i915_private > *dev_priv) > +{ > + u32 hotplug; > + > + hotplug = I915_READ(SHOTPLUG_CTL_DDI); > + hotplug |= ICP_DDIA_HPD_ENABLE | > + ICP_DDIB_HPD_ENABLE; > + I915_WRITE(SHOTPLUG_CTL_DDI, hotplug); > + > + hotplug = I915_READ(SHOTPLUG_CTL_TC); > + hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) | > + ICP_TC_HPD_ENABLE(PORT_TC2) | > + ICP_TC_HPD_ENABLE(PORT_TC3) | > + ICP_TC_HPD_ENABLE(PORT_TC4); > + I915_WRITE(SHOTPLUG_CTL_TC, hotplug); > +} > + > +static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv) > +{ > + u32 hotplug_irqs, enabled_irqs; > + > + hotplug_irqs = SDE_DDI_MASK_ICP | SDE_TC_MASK_ICP; > + enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_icp); > + > + ibx_display_interrupt_update(dev_priv, hotplug_irqs, > enabled_irqs); > + > + icp_hpd_detection_setup(dev_priv); > +} > + > static void gen11_hpd_detection_setup(struct drm_i915_private > *dev_priv) > { > u32 hotplug; > @@ -3733,6 +3842,9 @@ static void gen11_hpd_irq_setup(struct > drm_i915_private *dev_priv) > POSTING_READ(GEN11_DE_HPD_IMR); > > gen11_hpd_detection_setup(dev_priv); > + > + if (HAS_PCH_ICP(dev_priv)) > + icp_hpd_irq_setup(dev_priv); > } > > static void spt_hpd_detection_setup(struct drm_i915_private > *dev_priv) > @@ -4168,11 +4280,29 @@ static void gen11_gt_irq_postinstall(struct > drm_i915_private *dev_priv) > I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK, ~0); > } > > +static void icp_irq_postinstall(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = to_i915(dev); > + u32 mask = SDE_GMBUS_ICP; > + > + WARN_ON(I915_READ(SDEIER) != 0); > + I915_WRITE(SDEIER, 0xffffffff); > + POSTING_READ(SDEIER); > + > + gen3_assert_iir_is_zero(dev_priv, SDEIIR); > + I915_WRITE(SDEIMR, ~mask); > + > + icp_hpd_detection_setup(dev_priv); Having a sentence/paragraph on the commit message explaining why we can afford to do this instead of going the ibx_irq_pre_postinstall() followed by ibx_irq_postinstall() done by the previous platforms would help. My understanding is that this is OK, but a confirmation would always be fine. > +} > + > static int gen11_irq_postinstall(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > u32 gu_misc_masked = GEN11_GU_MISC_GSE; > > + if (HAS_PCH_ICP(dev_priv)) > + icp_irq_postinstall(dev); > + > gen11_gt_irq_postinstall(dev_priv); > gen8_de_irq_postinstall(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 4bfd7a9..e347055 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7517,6 +7517,46 @@ enum { > #define PORTE_HOTPLUG_SHORT_DETECT (1 << 0) > #define PORTE_HOTPLUG_LONG_DETECT (2 << 0) > > +/* ICP */ > +#define SDE_TC4_HOTPLUG_ICP (1 << 27) > +#define SDE_TC3_HOTPLUG_ICP (1 << 26) > +#define SDE_TC2_HOTPLUG_ICP (1 << 25) > +#define SDE_TC1_HOTPLUG_ICP (1 << 24) > +#define SDE_GMBUS_ICP (1 << 23) > +#define SDE_DDIB_HOTPLUG_ICP (1 << 17) > +#define SDE_DDIA_HOTPLUG_ICP (1 << 16) > + > +#define SDE_DDI_MASK_ICP (SDE_DDIB_HOTPLUG_ICP | > \ > + SDE_DDIA_HOTPLUG_ICP) > + > +#define SDE_TC_MASK_ICP (SDE_TC4_HOTPLUG_ICP > | \ > + SDE_TC3_HOTPLUG_ICP | > \ > + SDE_TC2_HOTPLUG_ICP | > \ > + SDE_TC1_HOTPLUG_ICP) Now these definitions are right below PCH_PORT_HOTPLUG2, but these bits are not for that register. Please move the definitions to the place that has the other SDE definitions. Also please change that /* ICP */ comment to match the style used by the other PCHs: /* south display engine interrupt: ICP */ (also possibly fixing the "CPT/PPT" comment to "CPT - CNP" or something like that. > + > +/* This register is a reuse of PCH_PORT_HOTPLUG register. The Spec > + * has the name SHOTPLUG_CTL_DDI for ICP. Let us stick to the Spec. I think the best way to describe would be to say that what was previously done by a single register (PCH_PORT_HOTPLUG) is now done by two registers, one for DDI and the other for TC, and that's why we have brand new definitions. Otherwise, looks good. Thanks, Paulo > + */ > + > +#define SHOTPLUG_CTL_DDI _MMIO(0xc4030) > +#define ICP_DDIB_HPD_ENABLE (1 << 7) > +#define ICP_DDIB_HPD_STATUS_MASK (3 << 4) > +#define ICP_DDIB_HPD_NO_DETECT (0 << 4) > +#define ICP_DDIB_HPD_SHORT_DETECT (1 << 4) > +#define ICP_DDIB_HPD_LONG_DETECT (2 << 4) > +#define ICP_DDIB_HPD_SHORT_LONG_DETECT (3 << 4) > +#define ICP_DDIA_HPD_ENABLE (1 << 3) > +#define ICP_DDIA_HPD_STATUS_MASK (3 << 0) > +#define ICP_DDIA_HPD_NO_DETECT (0 << 0) > +#define ICP_DDIA_HPD_SHORT_DETECT (1 << 0) > +#define ICP_DDIA_HPD_LONG_DETECT (2 << 0) > +#define ICP_DDIA_HPD_SHORT_LONG_DETECT (3 << 0) > + > +#define SHOTPLUG_CTL_TC _MMIO(0xc4034 > ) > +#define ICP_TC_HPD_ENABLE(tc_port) (8 << (tc_port) > * 4) > +#define ICP_TC_HPD_LONG_DETECT(tc_port) (2 << (tc_port) * > 4) > +#define ICP_TC_HPD_SHORT_DETECT(tc_port) (1 << (tc_port) * > 4) > + > #define PCH_GPIOA _MMIO(0xc5010) > #define PCH_GPIOB _MMIO(0xc5014) > #define PCH_GPIOC _MMIO(0xc5018)
Em Ter, 2018-06-26 às 11:32 -0700, Srivatsa, Anusha escreveu: > ________________________________________ > From: Zanoni, Paulo R > Sent: Monday, June 25, 2018 4:17 PM > To: Srivatsa, Anusha; intel-gfx@lists.freedesktop.org > Cc: Pandiyan, Dhinakaran > Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/icp: Add Interrupt > Support > > Em Qua, 2018-06-20 às 14:36 -0700, Anusha Srivatsa escreveu: > > This patch addresses Interrupts from south display engine (SDE). > > > > ICP has two registers - SHOTPLUG_CTL_DDI and SHOTPLUG_CTL_TC. > > Introduce these registers and their intended values. > > > > Introduce icp_irq_handler(). > > > > v2: > > - remove redundant register defines.(Lucas) > > - Change register names to be more consistent with > > previous platforms (Lucas) > > > > Cc: Lucas De Marchi <lucas.de.marchi@gmail.com> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > [Paulo: coding style bikesheds and rebases]. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 134 > > +++++++++++++++++++++++++++++++++++++++- > > drivers/gpu/drm/i915/i915_reg.h | 40 ++++++++++++ > > 2 files changed, 172 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 46aaef5..7a7c4a2 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -122,6 +122,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = { > > [HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG > > }; > > > > +static const u32 hpd_icp[HPD_NUM_PINS] = { > > + [HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP, > > + [HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP, > > + [HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP, > > + [HPD_PORT_D] = SDE_TC2_HOTPLUG_ICP, > > + [HPD_PORT_E] = SDE_TC3_HOTPLUG_ICP, > > + [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP > > +}; > > + > > /* IIR can theoretically queue up two events. Be paranoid. */ > > #define GEN8_IRQ_RESET_NDX(type, which) do { \ > > I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \ > > @@ -1586,6 +1595,34 @@ static bool > > bxt_port_hotplug_long_detect(enum > > port port, u32 val) > > } > > } > > > > +static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 > > val) > > +{ > > + switch (port) { > > + case PORT_A: > > + return val & ICP_DDIA_HPD_LONG_DETECT; > > + case PORT_B: > > + return val & ICP_DDIB_HPD_LONG_DETECT; > > + default: > > + return false; > > + } > > +} > > + > > +static bool icp_tc_port_hotplug_long_detect(enum port port, u32 > > val) > > +{ > > + switch (port) { > > + case PORT_C: > > + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1); > > + case PORT_D: > > + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2); > > + case PORT_E: > > + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3); > > + case PORT_F: > > + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4); > > + default: > > + return false; > > + } > > +} > > + > > static bool spt_port_hotplug2_long_detect(enum port port, u32 val) > > { > > switch (port) { > > @@ -2385,6 +2422,43 @@ static void cpt_irq_handler(struct > > drm_i915_private *dev_priv, u32 pch_iir) > > cpt_serr_int_handler(dev_priv); > > } > > > > +static void icp_irq_handler(struct drm_i915_private *dev_priv, u32 > > pch_iir) > > +{ > > + u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP; > > + u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP; > > + u32 pin_mask = 0, long_mask = 0; > > + > > + if (ddi_hotplug_trigger) { > > + u32 dig_hotplug_reg; > > + > > + dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI); > > + I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg); > > + > > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, > > + ddi_hotplug_trigger, > > + dig_hotplug_reg, hpd_icp, > > + icp_ddi_port_hotplug_long_detect) > > ; > > + } > > + > > + if (tc_hotplug_trigger) { > > + u32 dig_hotplug_reg; > > + > > + dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC); > > + I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg); > > + > > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, > > + tc_hotplug_trigger, > > + dig_hotplug_reg, hpd_icp, > > + icp_tc_port_hotplug_long_detect); > > + } > > + > > + if (pin_mask) > > + intel_hpd_irq_handler(dev_priv, pin_mask, > > long_mask); > > + > > + if (pch_iir & SDE_GMBUS_ICP) > > + gmbus_irq_handler(dev_priv); > > +} > > + > > static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 > > pch_iir) > > { > > u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT & > > @@ -2804,8 +2878,11 @@ gen8_de_irq_handler(struct drm_i915_private > > *dev_priv, u32 master_ctl) > > I915_WRITE(SDEIIR, iir); > > ret = IRQ_HANDLED; > > > > - if (HAS_PCH_SPT(dev_priv) || > > HAS_PCH_KBP(dev_priv) || > > - HAS_PCH_CNP(dev_priv)) > > + if (HAS_PCH_ICP(dev_priv)) > > + icp_irq_handler(dev_priv, iir); > > + else if (HAS_PCH_SPT(dev_priv) || > > + HAS_PCH_KBP(dev_priv) || > > + HAS_PCH_CNP(dev_priv)) > > spt_irq_handler(dev_priv, iir); > > else > > cpt_irq_handler(dev_priv, iir); > > @@ -3584,6 +3661,9 @@ static void gen11_irq_reset(struct drm_device > > *dev) > > GEN3_IRQ_RESET(GEN11_DE_HPD_); > > GEN3_IRQ_RESET(GEN11_GU_MISC_); > > GEN3_IRQ_RESET(GEN8_PCU_); > > + > > + if (HAS_PCH_ICP(dev_priv)) > > + GEN3_IRQ_RESET(SDE); > > } > > > > void gen8_irq_power_well_post_enable(struct drm_i915_private > > *dev_priv, > > @@ -3700,6 +3780,35 @@ static void ibx_hpd_irq_setup(struct > > drm_i915_private *dev_priv) > > ibx_hpd_detection_setup(dev_priv); > > } > > > > +static void icp_hpd_detection_setup(struct drm_i915_private > > *dev_priv) > > +{ > > + u32 hotplug; > > + > > + hotplug = I915_READ(SHOTPLUG_CTL_DDI); > > + hotplug |= ICP_DDIA_HPD_ENABLE | > > + ICP_DDIB_HPD_ENABLE; > > + I915_WRITE(SHOTPLUG_CTL_DDI, hotplug); > > + > > + hotplug = I915_READ(SHOTPLUG_CTL_TC); > > + hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) | > > + ICP_TC_HPD_ENABLE(PORT_TC2) | > > + ICP_TC_HPD_ENABLE(PORT_TC3) | > > + ICP_TC_HPD_ENABLE(PORT_TC4); > > + I915_WRITE(SHOTPLUG_CTL_TC, hotplug); > > +} > > + > > +static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv) > > +{ > > + u32 hotplug_irqs, enabled_irqs; > > + > > + hotplug_irqs = SDE_DDI_MASK_ICP | SDE_TC_MASK_ICP; > > + enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_icp); > > + > > + ibx_display_interrupt_update(dev_priv, hotplug_irqs, > > enabled_irqs); > > + > > + icp_hpd_detection_setup(dev_priv); > > +} > > + > > static void gen11_hpd_detection_setup(struct drm_i915_private > > *dev_priv) > > { > > u32 hotplug; > > @@ -3733,6 +3842,9 @@ static void gen11_hpd_irq_setup(struct > > drm_i915_private *dev_priv) > > POSTING_READ(GEN11_DE_HPD_IMR); > > > > gen11_hpd_detection_setup(dev_priv); > > + > > + if (HAS_PCH_ICP(dev_priv)) > > + icp_hpd_irq_setup(dev_priv); > > } > > > > static void spt_hpd_detection_setup(struct drm_i915_private > > *dev_priv) > > @@ -4168,11 +4280,29 @@ static void gen11_gt_irq_postinstall(struct > > drm_i915_private *dev_priv) > > I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK, ~0); > > } > > > > +static void icp_irq_postinstall(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(dev); > > + u32 mask = SDE_GMBUS_ICP; > > + > > + WARN_ON(I915_READ(SDEIER) != 0); > > + I915_WRITE(SDEIER, 0xffffffff); > > + POSTING_READ(SDEIER); > > + > > + gen3_assert_iir_is_zero(dev_priv, SDEIIR); > > + I915_WRITE(SDEIMR, ~mask); > > + > > + icp_hpd_detection_setup(dev_priv); > > Having a sentence/paragraph on the commit message explaining why we > can > afford to do this instead of going the ibx_irq_pre_postinstall() > followed by ibx_irq_postinstall() done by the previous platforms > would > help. > > My understanding is that this is OK, but a confirmation would always > be > fine. > > > > +} > > + > > static int gen11_irq_postinstall(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > u32 gu_misc_masked = GEN11_GU_MISC_GSE; > > > > + if (HAS_PCH_ICP(dev_priv)) > > + icp_irq_postinstall(dev); > > + > > gen11_gt_irq_postinstall(dev_priv); > > gen8_de_irq_postinstall(dev_priv); > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 4bfd7a9..e347055 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7517,6 +7517,46 @@ enum { > > #define PORTE_HOTPLUG_SHORT_DETECT (1 << 0) > > #define PORTE_HOTPLUG_LONG_DETECT (2 << 0) > > > > +/* ICP */ > > +#define SDE_TC4_HOTPLUG_ICP (1 << 27) > > +#define SDE_TC3_HOTPLUG_ICP (1 << 26) > > +#define SDE_TC2_HOTPLUG_ICP (1 << 25) > > +#define SDE_TC1_HOTPLUG_ICP (1 << 24) > > +#define SDE_GMBUS_ICP (1 << 23) > > +#define SDE_DDIB_HOTPLUG_ICP (1 << 17) > > +#define SDE_DDIA_HOTPLUG_ICP (1 << 16) > > + > > +#define SDE_DDI_MASK_ICP (SDE_DDIB_HOTPLUG_ICP | > > \ > > + SDE_DDIA_HOTPLUG_ICP) > > + > > +#define SDE_TC_MASK_ICP (SDE_TC4_HOTPLUG_ICP > > > \ > > > > + SDE_TC3_HOTPLUG_ICP | > > \ > > + SDE_TC2_HOTPLUG_ICP | > > \ > > + SDE_TC1_HOTPLUG_ICP) > > Now these definitions are right below PCH_PORT_HOTPLUG2, but these > bits > are not for that register. Please move the definitions to the place > that has the other SDE definitions. Also please change that /* ICP */ > comment to match the style used by the other PCHs: /* south display > engine interrupt: ICP */ (also possibly fixing the "CPT/PPT" comment > to > "CPT - CNP" or something like that. > Yes. That is good point. > QQ, the change of "CPT/PPT...." will be a separate patch though, > right? Since it's just a single line comment change and it's related to your current change (update to the bits of the register in question), I would say there's no need to be in a separate patch IMHO, but of course we would also accept the change in a separate patch. > > > + > > +/* This register is a reuse of PCH_PORT_HOTPLUG register. The Spec > > + * has the name SHOTPLUG_CTL_DDI for ICP. Let us stick to the > > Spec. > > I think the best way to describe would be to say that what was > previously done by a single register (PCH_PORT_HOTPLUG) is now done > by > two registers, one for DDI and the other for TC, and that's why we > have > brand new definitions. > > Thanks paulo for the reviews. Will make the changes. > > Anusha > Otherwise, looks good. Please fix your email client to add ">" when you reply to emails, this email was super confusing. > > Thanks, > Paulo > > > + */ > > + > > +#define SHOTPLUG_CTL_DDI _MMIO(0xc4030) > > +#define ICP_DDIB_HPD_ENABLE (1 << 7) > > +#define ICP_DDIB_HPD_STATUS_MASK (3 << 4) > > +#define ICP_DDIB_HPD_NO_DETECT (0 << 4) > > +#define ICP_DDIB_HPD_SHORT_DETECT (1 << 4) > > +#define ICP_DDIB_HPD_LONG_DETECT (2 << 4) > > +#define ICP_DDIB_HPD_SHORT_LONG_DETECT (3 << 4) > > +#define ICP_DDIA_HPD_ENABLE (1 << 3) > > +#define ICP_DDIA_HPD_STATUS_MASK (3 << 0) > > +#define ICP_DDIA_HPD_NO_DETECT (0 << 0) > > +#define ICP_DDIA_HPD_SHORT_DETECT (1 << 0) > > +#define ICP_DDIA_HPD_LONG_DETECT (2 << 0) > > +#define ICP_DDIA_HPD_SHORT_LONG_DETECT (3 << 0) > > + > > +#define SHOTPLUG_CTL_TC _MMIO(0xc4034 > > ) > > +#define ICP_TC_HPD_ENABLE(tc_port) (8 << (tc_port) > > * 4) > > +#define ICP_TC_HPD_LONG_DETECT(tc_port) (2 << (tc_port) * > > 4) > > +#define ICP_TC_HPD_SHORT_DETECT(tc_port) (1 << (tc_port) * > > 4) > > + > > #define PCH_GPIOA _MMIO(0xc5010) > > #define PCH_GPIOB _MMIO(0xc5014) > > #define PCH_GPIOC _MMIO(0xc5018)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 46aaef5..7a7c4a2 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -122,6 +122,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = { [HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG }; +static const u32 hpd_icp[HPD_NUM_PINS] = { + [HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP, + [HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP, + [HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP, + [HPD_PORT_D] = SDE_TC2_HOTPLUG_ICP, + [HPD_PORT_E] = SDE_TC3_HOTPLUG_ICP, + [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP +}; + /* IIR can theoretically queue up two events. Be paranoid. */ #define GEN8_IRQ_RESET_NDX(type, which) do { \ I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \ @@ -1586,6 +1595,34 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val) } } +static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 val) +{ + switch (port) { + case PORT_A: + return val & ICP_DDIA_HPD_LONG_DETECT; + case PORT_B: + return val & ICP_DDIB_HPD_LONG_DETECT; + default: + return false; + } +} + +static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val) +{ + switch (port) { + case PORT_C: + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1); + case PORT_D: + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2); + case PORT_E: + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3); + case PORT_F: + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4); + default: + return false; + } +} + static bool spt_port_hotplug2_long_detect(enum port port, u32 val) { switch (port) { @@ -2385,6 +2422,43 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir) cpt_serr_int_handler(dev_priv); } +static void icp_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir) +{ + u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP; + u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP; + u32 pin_mask = 0, long_mask = 0; + + if (ddi_hotplug_trigger) { + u32 dig_hotplug_reg; + + dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI); + I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg); + + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, + ddi_hotplug_trigger, + dig_hotplug_reg, hpd_icp, + icp_ddi_port_hotplug_long_detect); + } + + if (tc_hotplug_trigger) { + u32 dig_hotplug_reg; + + dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC); + I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg); + + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, + tc_hotplug_trigger, + dig_hotplug_reg, hpd_icp, + icp_tc_port_hotplug_long_detect); + } + + if (pin_mask) + intel_hpd_irq_handler(dev_priv, pin_mask, long_mask); + + if (pch_iir & SDE_GMBUS_ICP) + gmbus_irq_handler(dev_priv); +} + static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir) { u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT & @@ -2804,8 +2878,11 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) I915_WRITE(SDEIIR, iir); ret = IRQ_HANDLED; - if (HAS_PCH_SPT(dev_priv) || HAS_PCH_KBP(dev_priv) || - HAS_PCH_CNP(dev_priv)) + if (HAS_PCH_ICP(dev_priv)) + icp_irq_handler(dev_priv, iir); + else if (HAS_PCH_SPT(dev_priv) || + HAS_PCH_KBP(dev_priv) || + HAS_PCH_CNP(dev_priv)) spt_irq_handler(dev_priv, iir); else cpt_irq_handler(dev_priv, iir); @@ -3584,6 +3661,9 @@ static void gen11_irq_reset(struct drm_device *dev) GEN3_IRQ_RESET(GEN11_DE_HPD_); GEN3_IRQ_RESET(GEN11_GU_MISC_); GEN3_IRQ_RESET(GEN8_PCU_); + + if (HAS_PCH_ICP(dev_priv)) + GEN3_IRQ_RESET(SDE); } void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, @@ -3700,6 +3780,35 @@ static void ibx_hpd_irq_setup(struct drm_i915_private *dev_priv) ibx_hpd_detection_setup(dev_priv); } +static void icp_hpd_detection_setup(struct drm_i915_private *dev_priv) +{ + u32 hotplug; + + hotplug = I915_READ(SHOTPLUG_CTL_DDI); + hotplug |= ICP_DDIA_HPD_ENABLE | + ICP_DDIB_HPD_ENABLE; + I915_WRITE(SHOTPLUG_CTL_DDI, hotplug); + + hotplug = I915_READ(SHOTPLUG_CTL_TC); + hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) | + ICP_TC_HPD_ENABLE(PORT_TC2) | + ICP_TC_HPD_ENABLE(PORT_TC3) | + ICP_TC_HPD_ENABLE(PORT_TC4); + I915_WRITE(SHOTPLUG_CTL_TC, hotplug); +} + +static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv) +{ + u32 hotplug_irqs, enabled_irqs; + + hotplug_irqs = SDE_DDI_MASK_ICP | SDE_TC_MASK_ICP; + enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_icp); + + ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs); + + icp_hpd_detection_setup(dev_priv); +} + static void gen11_hpd_detection_setup(struct drm_i915_private *dev_priv) { u32 hotplug; @@ -3733,6 +3842,9 @@ static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv) POSTING_READ(GEN11_DE_HPD_IMR); gen11_hpd_detection_setup(dev_priv); + + if (HAS_PCH_ICP(dev_priv)) + icp_hpd_irq_setup(dev_priv); } static void spt_hpd_detection_setup(struct drm_i915_private *dev_priv) @@ -4168,11 +4280,29 @@ static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv) I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK, ~0); } +static void icp_irq_postinstall(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + u32 mask = SDE_GMBUS_ICP; + + WARN_ON(I915_READ(SDEIER) != 0); + I915_WRITE(SDEIER, 0xffffffff); + POSTING_READ(SDEIER); + + gen3_assert_iir_is_zero(dev_priv, SDEIIR); + I915_WRITE(SDEIMR, ~mask); + + icp_hpd_detection_setup(dev_priv); +} + static int gen11_irq_postinstall(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; u32 gu_misc_masked = GEN11_GU_MISC_GSE; + if (HAS_PCH_ICP(dev_priv)) + icp_irq_postinstall(dev); + gen11_gt_irq_postinstall(dev_priv); gen8_de_irq_postinstall(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4bfd7a9..e347055 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7517,6 +7517,46 @@ enum { #define PORTE_HOTPLUG_SHORT_DETECT (1 << 0) #define PORTE_HOTPLUG_LONG_DETECT (2 << 0) +/* ICP */ +#define SDE_TC4_HOTPLUG_ICP (1 << 27) +#define SDE_TC3_HOTPLUG_ICP (1 << 26) +#define SDE_TC2_HOTPLUG_ICP (1 << 25) +#define SDE_TC1_HOTPLUG_ICP (1 << 24) +#define SDE_GMBUS_ICP (1 << 23) +#define SDE_DDIB_HOTPLUG_ICP (1 << 17) +#define SDE_DDIA_HOTPLUG_ICP (1 << 16) + +#define SDE_DDI_MASK_ICP (SDE_DDIB_HOTPLUG_ICP | \ + SDE_DDIA_HOTPLUG_ICP) + +#define SDE_TC_MASK_ICP (SDE_TC4_HOTPLUG_ICP | \ + SDE_TC3_HOTPLUG_ICP | \ + SDE_TC2_HOTPLUG_ICP | \ + SDE_TC1_HOTPLUG_ICP) + +/* This register is a reuse of PCH_PORT_HOTPLUG register. The Spec + * has the name SHOTPLUG_CTL_DDI for ICP. Let us stick to the Spec. + */ + +#define SHOTPLUG_CTL_DDI _MMIO(0xc4030) +#define ICP_DDIB_HPD_ENABLE (1 << 7) +#define ICP_DDIB_HPD_STATUS_MASK (3 << 4) +#define ICP_DDIB_HPD_NO_DETECT (0 << 4) +#define ICP_DDIB_HPD_SHORT_DETECT (1 << 4) +#define ICP_DDIB_HPD_LONG_DETECT (2 << 4) +#define ICP_DDIB_HPD_SHORT_LONG_DETECT (3 << 4) +#define ICP_DDIA_HPD_ENABLE (1 << 3) +#define ICP_DDIA_HPD_STATUS_MASK (3 << 0) +#define ICP_DDIA_HPD_NO_DETECT (0 << 0) +#define ICP_DDIA_HPD_SHORT_DETECT (1 << 0) +#define ICP_DDIA_HPD_LONG_DETECT (2 << 0) +#define ICP_DDIA_HPD_SHORT_LONG_DETECT (3 << 0) + +#define SHOTPLUG_CTL_TC _MMIO(0xc4034) +#define ICP_TC_HPD_ENABLE(tc_port) (8 << (tc_port) * 4) +#define ICP_TC_HPD_LONG_DETECT(tc_port) (2 << (tc_port) * 4) +#define ICP_TC_HPD_SHORT_DETECT(tc_port) (1 << (tc_port) * 4) + #define PCH_GPIOA _MMIO(0xc5010) #define PCH_GPIOB _MMIO(0xc5014) #define PCH_GPIOC _MMIO(0xc5018)