* Re: [Intel-gfx] [11/12] drm/i915: Introduce intel_hpd_hotplug_irqs()
2020-09-09 0:46 ` [Intel-gfx] [11/12] drm/i915: Introduce intel_hpd_hotplug_irqs() Souza, Jose
@ 2020-09-09 19:17 ` Ville Syrjälä
0 siblings, 0 replies; 2+ messages in thread
From: Ville Syrjälä @ 2020-09-09 19:17 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org
On Wed, Sep 09, 2020 at 12:46:56AM +0000, Souza, Jose wrote:
> On Wed, 2020-07-01 at 00:56 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <
> > ville.syrjala@linux.intel.com
> > >
> >
> > Introduce intel_hpd_hotplug_irqs() as a partner to
> > intel_hpd_enabled_irqs(). There's no need to care about the
> > encoders which we're not exposing, so we can avoid hardocoding
>
> hard-coding
>
> > the masks in various places.
>
> Pretty nice patch, you only missed to do this change in the irq_handlers so we could nuke the SDE_DDI_MASKs, or are you planning to do this in a
> follow up patch? If later consider this
I didn't decide yet how to do that part. One option is to compute the
mask there too, but the other option to just use the full mask there
and rely on the fact that the IMR will keep the unused bits clear
in the IIR. Not sure if the latter approach is quite as scalable though.
Eg. in theory some new PCH type might repurpose some of the bits for
some totally different use.
>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
>
> >
> > Signed-off-by: Ville Syrjälä <
> > ville.syrjala@linux.intel.com
> > >
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 50 +++++++++++++++------------------
> > 1 file changed, 23 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 95ab4432a87d..b8a6a21f4c54 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2943,6 +2943,18 @@ static u32 intel_hpd_enabled_irqs(struct drm_i915_private *dev_priv,
> > return enabled_irqs;
> > }
> >
> > +static u32 intel_hpd_hotplug_irqs(struct drm_i915_private *dev_priv,
> > + const u32 hpd[HPD_NUM_PINS])
> > +{
> > + struct intel_encoder *encoder;
> > + u32 hotplug_irqs = 0;
> > +
> > + for_each_intel_encoder(&dev_priv->drm, encoder)
> > + hotplug_irqs |= hpd[encoder->hpd_pin];
> > +
> > + return hotplug_irqs;
> > +}
> > +
> > static void ibx_hpd_detection_setup(struct drm_i915_private *dev_priv)
> > {
> > u32 hotplug;
> > @@ -2972,12 +2984,8 @@ static void ibx_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > {
> > u32 hotplug_irqs, enabled_irqs;
> >
> > - if (HAS_PCH_IBX(dev_priv))
> > - hotplug_irqs = SDE_HOTPLUG_MASK;
> > - else
> > - hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
> > -
> > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.pch_hpd);
> > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->hotplug.pch_hpd);
> >
> > ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
> >
> > @@ -3005,13 +3013,12 @@ static void icp_tc_hpd_detection_setup(struct drm_i915_private *dev_priv,
> > }
> >
> > static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv,
> > - u32 sde_ddi_mask, u32 sde_tc_mask,
> > u32 ddi_enable_mask, u32 tc_enable_mask)
> > {
> > u32 hotplug_irqs, enabled_irqs;
> >
> > - hotplug_irqs = sde_ddi_mask | sde_tc_mask;
> > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.pch_hpd);
> > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->hotplug.pch_hpd);
> >
> > I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ);
> >
> > @@ -3029,7 +3036,6 @@ static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv,
> > static void mcc_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > {
> > icp_hpd_irq_setup(dev_priv,
> > - SDE_DDI_MASK_ICP, SDE_TC_HOTPLUG_ICP(PORT_TC1),
> > ICP_DDI_HPD_ENABLE_MASK, ICP_TC_HPD_ENABLE(PORT_TC1));
> > }
> >
> > @@ -3041,7 +3047,6 @@ static void mcc_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > static void jsp_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > {
> > icp_hpd_irq_setup(dev_priv,
> > - SDE_DDI_MASK_TGP, 0,
> > TGP_DDI_HPD_ENABLE_MASK, 0);
> > }
> >
> > @@ -3074,7 +3079,7 @@ static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > u32 val;
> >
> > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.hpd);
> > - hotplug_irqs = GEN11_DE_TC_HOTPLUG_MASK | GEN11_DE_TBT_HOTPLUG_MASK;
> > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->hotplug.hpd);
> >
> > val = I915_READ(GEN11_DE_HPD_IMR);
> > val &= ~hotplug_irqs;
> > @@ -3085,10 +3090,10 @@ static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > gen11_hpd_detection_setup(dev_priv);
> >
> > if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP)
> > - icp_hpd_irq_setup(dev_priv, SDE_DDI_MASK_TGP, SDE_TC_MASK_TGP,
> > + icp_hpd_irq_setup(dev_priv,
> > TGP_DDI_HPD_ENABLE_MASK, TGP_TC_HPD_ENABLE_MASK);
> > else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> > - icp_hpd_irq_setup(dev_priv, SDE_DDI_MASK_ICP, SDE_TC_MASK_ICP,
> > + icp_hpd_irq_setup(dev_priv,
> > ICP_DDI_HPD_ENABLE_MASK, ICP_TC_HPD_ENABLE_MASK);
> > }
> >
> > @@ -3124,8 +3129,8 @@ static void spt_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
> > I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ);
> >
> > - hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
> > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.pch_hpd);
> > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->hotplug.pch_hpd);
> >
> > ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
> >
> > @@ -3152,22 +3157,13 @@ static void ilk_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > {
> > u32 hotplug_irqs, enabled_irqs;
> >
> > - if (INTEL_GEN(dev_priv) >= 8) {
> > - hotplug_irqs = GEN8_PORT_DP_A_HOTPLUG;
> > - enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.hpd);
> > + enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.hpd);
> > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->hotplug.hpd);
> >
> > + if (INTEL_GEN(dev_priv) >= 8)
> > bdw_update_port_irq(dev_priv, hotplug_irqs, enabled_irqs);
> > - } else if (INTEL_GEN(dev_priv) >= 7) {
> > - hotplug_irqs = DE_DP_A_HOTPLUG_IVB;
> > - enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.hpd);
> > -
> > + else
> > ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> > - } else {
> > - hotplug_irqs = DE_DP_A_HOTPLUG;
> > - enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.hpd);
> > -
> > - ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> > - }
> >
> > ilk_hpd_detection_setup(dev_priv);
> >
> > @@ -3216,7 +3212,7 @@ static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > u32 hotplug_irqs, enabled_irqs;
> >
> > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.hpd);
> > - hotplug_irqs = BXT_DE_PORT_HOTPLUG_MASK;
> > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->hotplug.hpd);
> >
> > bdw_update_port_irq(dev_priv, hotplug_irqs, enabled_irqs);
> >
> >
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 2+ messages in thread