| Message ID | 20201006143349.5561-16-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Futher cleanup around hpd pins and port identfiers | expand |
On Tue, Oct 06, 2020 at 05:33:44PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > No reason that I can see why we should enable the hpd detection logic > already during irq postinstall phase. We don't even do this on all > the platforms. We just need it before we actually enable the hotplug > interrupts in .hpd_irq_setup(), and in fact we already do it there as > well. Let's just eliminate the redundant early setup. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> It's needed by LSPCON resume, which happens before initing HPD interrupts. I suppose that could be done later, after HPD interrupt init, I don't see now why it would need to be done at encoder->reset() time. > --- > drivers/gpu/drm/i915/i915_irq.c | 38 +++------------------------------ > 1 file changed, 3 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 0886369e3890..b1c56a29376c 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3378,8 +3378,8 @@ static void ilk_hpd_irq_setup(struct drm_i915_private *dev_priv) > ibx_hpd_irq_setup(dev_priv); > } > > -static void __bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, > - u32 enabled_irqs) > +static void bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, > + u32 enabled_irqs) > { > u32 hotplug; > > @@ -3410,11 +3410,6 @@ static void __bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, > I915_WRITE(PCH_PORT_HOTPLUG, hotplug); > } > > -static void bxt_hpd_detection_setup(struct drm_i915_private *dev_priv) > -{ > - __bxt_hpd_detection_setup(dev_priv, BXT_DE_PORT_HOTPLUG_MASK); > -} > - > static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv) > { > u32 hotplug_irqs, enabled_irqs; > @@ -3424,7 +3419,7 @@ static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv) > > bdw_update_port_irq(dev_priv, hotplug_irqs, enabled_irqs); > > - __bxt_hpd_detection_setup(dev_priv, enabled_irqs); > + bxt_hpd_detection_setup(dev_priv, enabled_irqs); > } > > static void ibx_irq_postinstall(struct drm_i915_private *dev_priv) > @@ -3443,12 +3438,6 @@ static void ibx_irq_postinstall(struct drm_i915_private *dev_priv) > > gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR); > I915_WRITE(SDEIMR, ~mask); > - > - if (HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv) || > - HAS_PCH_LPT(dev_priv)) > - ibx_hpd_detection_setup(dev_priv); > - else > - spt_hpd_detection_setup(dev_priv); > } > > static void ilk_irq_postinstall(struct drm_i915_private *dev_priv) > @@ -3485,8 +3474,6 @@ static void ilk_irq_postinstall(struct drm_i915_private *dev_priv) > > gen5_gt_irq_postinstall(&dev_priv->gt); > > - ilk_hpd_detection_setup(dev_priv); > - > ibx_irq_postinstall(dev_priv); > > if (IS_IRONLAKE_M(dev_priv)) { > @@ -3618,12 +3605,6 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) > > GEN3_IRQ_INIT(uncore, GEN11_DE_HPD_, ~de_hpd_masked, > de_hpd_enables); > - gen11_tc_hpd_detection_setup(dev_priv); > - gen11_tbt_hpd_detection_setup(dev_priv); > - } else if (IS_GEN9_LP(dev_priv)) { > - bxt_hpd_detection_setup(dev_priv); > - } else if (IS_BROADWELL(dev_priv)) { > - ilk_hpd_detection_setup(dev_priv); > } > } > > @@ -3651,19 +3632,6 @@ static void icp_irq_postinstall(struct drm_i915_private *dev_priv) > > gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR); > I915_WRITE(SDEIMR, ~mask); > - > - if (HAS_PCH_TGP(dev_priv)) { > - icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK); > - icp_tc_hpd_detection_setup(dev_priv, TGP_TC_HPD_ENABLE_MASK); > - } else if (HAS_PCH_JSP(dev_priv)) { > - icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK); > - } else if (HAS_PCH_MCC(dev_priv)) { > - icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK); > - icp_tc_hpd_detection_setup(dev_priv, ICP_TC_HPD_ENABLE(HPD_PORT_TC1)); > - } else { > - icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK); > - icp_tc_hpd_detection_setup(dev_priv, ICP_TC_HPD_ENABLE_MASK); > - } > } > > static void gen11_irq_postinstall(struct drm_i915_private *dev_priv) > -- > 2.26.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Oct 06, 2020 at 07:20:46PM +0300, Imre Deak wrote: > On Tue, Oct 06, 2020 at 05:33:44PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > No reason that I can see why we should enable the hpd detection logic > > already during irq postinstall phase. We don't even do this on all > > the platforms. We just need it before we actually enable the hotplug > > interrupts in .hpd_irq_setup(), and in fact we already do it there as > > well. Let's just eliminate the redundant early setup. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > It's needed by LSPCON resume, which happens before initing HPD > interrupts. I suppose that could be done later, after HPD interrupt init, > I don't see now why it would need to be done at encoder->reset() time. Hmm. The ordering of the .reset() hooks vs. other init/resume stuff looks somewhat random. Might need to think how to make this consistent. At init time we no longer have the early lspcon probe, exept Uma probably has to add it back for the HDR stuff. This looks like it might need some actual thought :( > > > --- > > drivers/gpu/drm/i915/i915_irq.c | 38 +++------------------------------ > > 1 file changed, 3 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index 0886369e3890..b1c56a29376c 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -3378,8 +3378,8 @@ static void ilk_hpd_irq_setup(struct drm_i915_private *dev_priv) > > ibx_hpd_irq_setup(dev_priv); > > } > > > > -static void __bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, > > - u32 enabled_irqs) > > +static void bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, > > + u32 enabled_irqs) > > { > > u32 hotplug; > > > > @@ -3410,11 +3410,6 @@ static void __bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, > > I915_WRITE(PCH_PORT_HOTPLUG, hotplug); > > } > > > > -static void bxt_hpd_detection_setup(struct drm_i915_private *dev_priv) > > -{ > > - __bxt_hpd_detection_setup(dev_priv, BXT_DE_PORT_HOTPLUG_MASK); > > -} > > - > > static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv) > > { > > u32 hotplug_irqs, enabled_irqs; > > @@ -3424,7 +3419,7 @@ static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv) > > > > bdw_update_port_irq(dev_priv, hotplug_irqs, enabled_irqs); > > > > - __bxt_hpd_detection_setup(dev_priv, enabled_irqs); > > + bxt_hpd_detection_setup(dev_priv, enabled_irqs); > > } > > > > static void ibx_irq_postinstall(struct drm_i915_private *dev_priv) > > @@ -3443,12 +3438,6 @@ static void ibx_irq_postinstall(struct drm_i915_private *dev_priv) > > > > gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR); > > I915_WRITE(SDEIMR, ~mask); > > - > > - if (HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv) || > > - HAS_PCH_LPT(dev_priv)) > > - ibx_hpd_detection_setup(dev_priv); > > - else > > - spt_hpd_detection_setup(dev_priv); > > } > > > > static void ilk_irq_postinstall(struct drm_i915_private *dev_priv) > > @@ -3485,8 +3474,6 @@ static void ilk_irq_postinstall(struct drm_i915_private *dev_priv) > > > > gen5_gt_irq_postinstall(&dev_priv->gt); > > > > - ilk_hpd_detection_setup(dev_priv); > > - > > ibx_irq_postinstall(dev_priv); > > > > if (IS_IRONLAKE_M(dev_priv)) { > > @@ -3618,12 +3605,6 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) > > > > GEN3_IRQ_INIT(uncore, GEN11_DE_HPD_, ~de_hpd_masked, > > de_hpd_enables); > > - gen11_tc_hpd_detection_setup(dev_priv); > > - gen11_tbt_hpd_detection_setup(dev_priv); > > - } else if (IS_GEN9_LP(dev_priv)) { > > - bxt_hpd_detection_setup(dev_priv); > > - } else if (IS_BROADWELL(dev_priv)) { > > - ilk_hpd_detection_setup(dev_priv); > > } > > } > > > > @@ -3651,19 +3632,6 @@ static void icp_irq_postinstall(struct drm_i915_private *dev_priv) > > > > gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR); > > I915_WRITE(SDEIMR, ~mask); > > - > > - if (HAS_PCH_TGP(dev_priv)) { > > - icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK); > > - icp_tc_hpd_detection_setup(dev_priv, TGP_TC_HPD_ENABLE_MASK); > > - } else if (HAS_PCH_JSP(dev_priv)) { > > - icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK); > > - } else if (HAS_PCH_MCC(dev_priv)) { > > - icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK); > > - icp_tc_hpd_detection_setup(dev_priv, ICP_TC_HPD_ENABLE(HPD_PORT_TC1)); > > - } else { > > - icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK); > > - icp_tc_hpd_detection_setup(dev_priv, ICP_TC_HPD_ENABLE_MASK); > > - } > > } > > > > static void gen11_irq_postinstall(struct drm_i915_private *dev_priv) > > -- > > 2.26.2 > > > > _______________________________________________ > > 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 index 0886369e3890..b1c56a29376c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3378,8 +3378,8 @@ static void ilk_hpd_irq_setup(struct drm_i915_private *dev_priv) ibx_hpd_irq_setup(dev_priv); } -static void __bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, - u32 enabled_irqs) +static void bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, + u32 enabled_irqs) { u32 hotplug; @@ -3410,11 +3410,6 @@ static void __bxt_hpd_detection_setup(struct drm_i915_private *dev_priv, I915_WRITE(PCH_PORT_HOTPLUG, hotplug); } -static void bxt_hpd_detection_setup(struct drm_i915_private *dev_priv) -{ - __bxt_hpd_detection_setup(dev_priv, BXT_DE_PORT_HOTPLUG_MASK); -} - static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv) { u32 hotplug_irqs, enabled_irqs; @@ -3424,7 +3419,7 @@ static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv) bdw_update_port_irq(dev_priv, hotplug_irqs, enabled_irqs); - __bxt_hpd_detection_setup(dev_priv, enabled_irqs); + bxt_hpd_detection_setup(dev_priv, enabled_irqs); } static void ibx_irq_postinstall(struct drm_i915_private *dev_priv) @@ -3443,12 +3438,6 @@ static void ibx_irq_postinstall(struct drm_i915_private *dev_priv) gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR); I915_WRITE(SDEIMR, ~mask); - - if (HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv) || - HAS_PCH_LPT(dev_priv)) - ibx_hpd_detection_setup(dev_priv); - else - spt_hpd_detection_setup(dev_priv); } static void ilk_irq_postinstall(struct drm_i915_private *dev_priv) @@ -3485,8 +3474,6 @@ static void ilk_irq_postinstall(struct drm_i915_private *dev_priv) gen5_gt_irq_postinstall(&dev_priv->gt); - ilk_hpd_detection_setup(dev_priv); - ibx_irq_postinstall(dev_priv); if (IS_IRONLAKE_M(dev_priv)) { @@ -3618,12 +3605,6 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) GEN3_IRQ_INIT(uncore, GEN11_DE_HPD_, ~de_hpd_masked, de_hpd_enables); - gen11_tc_hpd_detection_setup(dev_priv); - gen11_tbt_hpd_detection_setup(dev_priv); - } else if (IS_GEN9_LP(dev_priv)) { - bxt_hpd_detection_setup(dev_priv); - } else if (IS_BROADWELL(dev_priv)) { - ilk_hpd_detection_setup(dev_priv); } } @@ -3651,19 +3632,6 @@ static void icp_irq_postinstall(struct drm_i915_private *dev_priv) gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR); I915_WRITE(SDEIMR, ~mask); - - if (HAS_PCH_TGP(dev_priv)) { - icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK); - icp_tc_hpd_detection_setup(dev_priv, TGP_TC_HPD_ENABLE_MASK); - } else if (HAS_PCH_JSP(dev_priv)) { - icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK); - } else if (HAS_PCH_MCC(dev_priv)) { - icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK); - icp_tc_hpd_detection_setup(dev_priv, ICP_TC_HPD_ENABLE(HPD_PORT_TC1)); - } else { - icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK); - icp_tc_hpd_detection_setup(dev_priv, ICP_TC_HPD_ENABLE_MASK); - } } static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)