* Re: [Intel-gfx] [11/12] drm/i915: Introduce intel_hpd_hotplug_irqs()
[not found] <20200630215601.28557-12-ville.syrjala@linux.intel.com>
@ 2020-09-09 0:46 ` Souza, Jose
2020-09-09 19:17 ` Ville Syrjälä
0 siblings, 1 reply; 2+ messages in thread
From: Souza, Jose @ 2020-09-09 0:46 UTC (permalink / raw)
To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org
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
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);
>
>
_______________________________________________
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
* 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
end of thread, other threads:[~2020-09-09 19:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200630215601.28557-12-ville.syrjala@linux.intel.com>
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ä
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox