On Wed, 22 Feb 2023, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Pair each <platform>_hotplug_enables() function with > a corresponding <platform>_hotplug_mask() function so that > we can determine right bits to clear on a per hpd_pin basis. > We'll need this for turning on HPD sense for a specific > encoder rather than just all of them. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_irq.c | 231 ++++++++++++++++++++++---------- > 1 file changed, 160 insertions(+), 71 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 13ada0916c2a..ecfa6dad145a 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2835,8 +2835,25 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv) > vlv_display_irq_reset(dev_priv); > spin_unlock_irq(&dev_priv->irq_lock); > } > > +static u32 ibx_hotplug_mask(struct drm_i915_private *i915, > + enum hpd_pin hpd_pin) What's the reason for passing i915 to these functions? I see no use in this series, am I missing something? Seems like it makes some calls a bit convoluted. BR, Jani. > +{ > + switch (hpd_pin) { > + case HPD_PORT_A: > + return PORTA_HOTPLUG_ENABLE; > + case HPD_PORT_B: > + return PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_MASK; > + case HPD_PORT_C: > + return PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_MASK; > + case HPD_PORT_D: > + return PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_MASK; > + default: > + return 0; > + } > +} > + > static u32 ibx_hotplug_enables(struct intel_encoder *encoder) > { > struct drm_i915_private *i915 = to_i915(encoder->base.dev); > > @@ -2869,15 +2886,12 @@ static void ibx_hpd_detection_setup(struct drm_i915_private *dev_priv) > * duration to 2ms (which is the minimum in the Display Port spec). > * The pulse duration bits are reserved on LPT+. > */ > intel_uncore_rmw(&dev_priv->uncore, PCH_PORT_HOTPLUG, > - PORTA_HOTPLUG_ENABLE | > - PORTB_HOTPLUG_ENABLE | > - PORTC_HOTPLUG_ENABLE | > - PORTD_HOTPLUG_ENABLE | > - PORTB_PULSE_DURATION_MASK | > - PORTC_PULSE_DURATION_MASK | > - PORTD_PULSE_DURATION_MASK, > + ibx_hotplug_mask(dev_priv, HPD_PORT_A) | > + ibx_hotplug_mask(dev_priv, HPD_PORT_B) | > + ibx_hotplug_mask(dev_priv, HPD_PORT_C) | > + ibx_hotplug_mask(dev_priv, HPD_PORT_D), > intel_hpd_hotplug_enables(dev_priv, ibx_hotplug_enables)); > } > > static void ibx_hpd_irq_setup(struct drm_i915_private *dev_priv) > @@ -2891,55 +2905,75 @@ static void ibx_hpd_irq_setup(struct drm_i915_private *dev_priv) > > ibx_hpd_detection_setup(dev_priv); > } > > +static u32 _icp_ddi_hotplug_enables(enum hpd_pin hpd_pin) > +{ > + switch (hpd_pin) { > + case HPD_PORT_A: > + case HPD_PORT_B: > + case HPD_PORT_C: > + case HPD_PORT_D: > + return SHOTPLUG_CTL_DDI_HPD_ENABLE(hpd_pin); > + default: > + return 0; > + } > +} > + > +static u32 icp_ddi_hotplug_mask(struct drm_i915_private *i915, enum hpd_pin hpd_pin) > +{ > + return _icp_ddi_hotplug_enables(hpd_pin); > +} > + > static u32 icp_ddi_hotplug_enables(struct intel_encoder *encoder) > { > - switch (encoder->hpd_pin) { > - case HPD_PORT_A: > - case HPD_PORT_B: > - case HPD_PORT_C: > - case HPD_PORT_D: > - return SHOTPLUG_CTL_DDI_HPD_ENABLE(encoder->hpd_pin); > + return _icp_ddi_hotplug_enables(encoder->hpd_pin); > +} > + > +static u32 _icp_tc_hotplug_enables(enum hpd_pin hpd_pin) > +{ > + switch (hpd_pin) { > + case HPD_PORT_TC1: > + case HPD_PORT_TC2: > + case HPD_PORT_TC3: > + case HPD_PORT_TC4: > + case HPD_PORT_TC5: > + case HPD_PORT_TC6: > + return ICP_TC_HPD_ENABLE(hpd_pin); > default: > return 0; > } > } > > +static u32 icp_tc_hotplug_mask(struct drm_i915_private *i915, enum hpd_pin hpd_pin) > +{ > + return _icp_tc_hotplug_enables(hpd_pin); > +} > + > static u32 icp_tc_hotplug_enables(struct intel_encoder *encoder) > { > - switch (encoder->hpd_pin) { > - case HPD_PORT_TC1: > - case HPD_PORT_TC2: > - case HPD_PORT_TC3: > - case HPD_PORT_TC4: > - case HPD_PORT_TC5: > - case HPD_PORT_TC6: > - return ICP_TC_HPD_ENABLE(encoder->hpd_pin); > - default: > - return 0; > - } > + return _icp_tc_hotplug_enables(encoder->hpd_pin); > } > > static void icp_ddi_hpd_detection_setup(struct drm_i915_private *dev_priv) > { > intel_uncore_rmw(&dev_priv->uncore, SHOTPLUG_CTL_DDI, > - SHOTPLUG_CTL_DDI_HPD_ENABLE(HPD_PORT_A) | > - SHOTPLUG_CTL_DDI_HPD_ENABLE(HPD_PORT_B) | > - SHOTPLUG_CTL_DDI_HPD_ENABLE(HPD_PORT_C) | > - SHOTPLUG_CTL_DDI_HPD_ENABLE(HPD_PORT_D), > + icp_ddi_hotplug_mask(dev_priv, HPD_PORT_A) | > + icp_ddi_hotplug_mask(dev_priv, HPD_PORT_B) | > + icp_ddi_hotplug_mask(dev_priv, HPD_PORT_C) | > + icp_ddi_hotplug_mask(dev_priv, HPD_PORT_D), > intel_hpd_hotplug_enables(dev_priv, icp_ddi_hotplug_enables)); > } > > static void icp_tc_hpd_detection_setup(struct drm_i915_private *dev_priv) > { > intel_uncore_rmw(&dev_priv->uncore, SHOTPLUG_CTL_TC, > - ICP_TC_HPD_ENABLE(HPD_PORT_TC1) | > - ICP_TC_HPD_ENABLE(HPD_PORT_TC2) | > - ICP_TC_HPD_ENABLE(HPD_PORT_TC3) | > - ICP_TC_HPD_ENABLE(HPD_PORT_TC4) | > - ICP_TC_HPD_ENABLE(HPD_PORT_TC5) | > - ICP_TC_HPD_ENABLE(HPD_PORT_TC6), > + icp_tc_hotplug_mask(dev_priv, HPD_PORT_TC1) | > + icp_tc_hotplug_mask(dev_priv, HPD_PORT_TC2) | > + icp_tc_hotplug_mask(dev_priv, HPD_PORT_TC3) | > + icp_tc_hotplug_mask(dev_priv, HPD_PORT_TC4) | > + icp_tc_hotplug_mask(dev_priv, HPD_PORT_TC5) | > + icp_tc_hotplug_mask(dev_priv, HPD_PORT_TC6), > intel_hpd_hotplug_enables(dev_priv, icp_tc_hotplug_enables)); > } > > static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv) > @@ -2957,21 +2991,31 @@ static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv) > icp_ddi_hpd_detection_setup(dev_priv); > icp_tc_hpd_detection_setup(dev_priv); > } > > +static u32 _gen11_hotplug_enables(enum hpd_pin hpd_pin) > +{ > + switch (hpd_pin) { > + case HPD_PORT_TC1: > + case HPD_PORT_TC2: > + case HPD_PORT_TC3: > + case HPD_PORT_TC4: > + case HPD_PORT_TC5: > + case HPD_PORT_TC6: > + return GEN11_HOTPLUG_CTL_ENABLE(hpd_pin); > + default: > + return 0; > + } > +} > + > +static u32 gen11_hotplug_mask(struct drm_i915_private *i915, enum hpd_pin hpd_pin) > +{ > + return _gen11_hotplug_enables(hpd_pin); > +} > + > static u32 gen11_hotplug_enables(struct intel_encoder *encoder) > { > - switch (encoder->hpd_pin) { > - case HPD_PORT_TC1: > - case HPD_PORT_TC2: > - case HPD_PORT_TC3: > - case HPD_PORT_TC4: > - case HPD_PORT_TC5: > - case HPD_PORT_TC6: > - return GEN11_HOTPLUG_CTL_ENABLE(encoder->hpd_pin); > - default: > - return 0; > - } > + return _gen11_hotplug_enables(encoder->hpd_pin); > } > > static void dg1_hpd_invert(struct drm_i915_private *i915) > { > @@ -2990,26 +3034,26 @@ static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv) > > static void gen11_tc_hpd_detection_setup(struct drm_i915_private *dev_priv) > { > intel_uncore_rmw(&dev_priv->uncore, GEN11_TC_HOTPLUG_CTL, > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC1) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC2) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC3) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC4) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC5) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC6), > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC1) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC2) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC3) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC4) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC5) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC6), > intel_hpd_hotplug_enables(dev_priv, gen11_hotplug_enables)); > } > > static void gen11_tbt_hpd_detection_setup(struct drm_i915_private *dev_priv) > { > intel_uncore_rmw(&dev_priv->uncore, GEN11_TBT_HOTPLUG_CTL, > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC1) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC2) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC3) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC4) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC5) | > - GEN11_HOTPLUG_CTL_ENABLE(HPD_PORT_TC6), > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC1) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC2) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC3) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC4) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC5) | > + gen11_hotplug_mask(dev_priv, HPD_PORT_TC6), > intel_hpd_hotplug_enables(dev_priv, gen11_hotplug_enables)); > } > > static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv) > @@ -3029,11 +3073,11 @@ static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv) > if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) > icp_hpd_irq_setup(dev_priv); > } > > -static u32 spt_hotplug_enables(struct intel_encoder *encoder) > +static u32 _spt_hotplug_enables(enum hpd_pin hpd_pin) > { > - switch (encoder->hpd_pin) { > + switch (hpd_pin) { > case HPD_PORT_A: > return PORTA_HOTPLUG_ENABLE; > case HPD_PORT_B: > return PORTB_HOTPLUG_ENABLE; > @@ -3045,18 +3089,38 @@ static u32 spt_hotplug_enables(struct intel_encoder *encoder) > return 0; > } > } > > -static u32 spt_hotplug2_enables(struct intel_encoder *encoder) > +static u32 spt_hotplug_mask(struct drm_i915_private *i915, enum hpd_pin hpd_pin) > { > - switch (encoder->hpd_pin) { > + return _spt_hotplug_enables(hpd_pin); > +} > + > +static u32 spt_hotplug_enables(struct intel_encoder *encoder) > +{ > + return _spt_hotplug_enables(encoder->hpd_pin); > +} > + > +static u32 _spt_hotplug2_enables(enum hpd_pin hpd_pin) > +{ > + switch (hpd_pin) { > case HPD_PORT_E: > return PORTE_HOTPLUG_ENABLE; > default: > return 0; > } > } > > +static u32 spt_hotplug2_mask(struct drm_i915_private *i915, enum hpd_pin hpd_pin) > +{ > + return _spt_hotplug2_enables(hpd_pin); > +} > + > +static u32 spt_hotplug2_enables(struct intel_encoder *encoder) > +{ > + return _spt_hotplug2_enables(encoder->hpd_pin); > +} > + > static void spt_hpd_detection_setup(struct drm_i915_private *dev_priv) > { > /* Display WA #1179 WaHardHangonHotPlug: cnp */ > if (HAS_PCH_CNP(dev_priv)) { > @@ -3065,15 +3129,16 @@ static void spt_hpd_detection_setup(struct drm_i915_private *dev_priv) > } > > /* Enable digital hotplug on the PCH */ > intel_uncore_rmw(&dev_priv->uncore, PCH_PORT_HOTPLUG, > - PORTA_HOTPLUG_ENABLE | > - PORTB_HOTPLUG_ENABLE | > - PORTC_HOTPLUG_ENABLE | > - PORTD_HOTPLUG_ENABLE, > + spt_hotplug_mask(dev_priv, HPD_PORT_A) | > + spt_hotplug_mask(dev_priv, HPD_PORT_B) | > + spt_hotplug_mask(dev_priv, HPD_PORT_C) | > + spt_hotplug_mask(dev_priv, HPD_PORT_D), > intel_hpd_hotplug_enables(dev_priv, spt_hotplug_enables)); > > - intel_uncore_rmw(&dev_priv->uncore, PCH_PORT_HOTPLUG2, PORTE_HOTPLUG_ENABLE, > + intel_uncore_rmw(&dev_priv->uncore, PCH_PORT_HOTPLUG2, > + spt_hotplug2_mask(dev_priv, HPD_PORT_E), > intel_hpd_hotplug_enables(dev_priv, spt_hotplug2_enables)); > } > > static void spt_hpd_irq_setup(struct drm_i915_private *dev_priv) > @@ -3090,8 +3155,19 @@ static void spt_hpd_irq_setup(struct drm_i915_private *dev_priv) > > spt_hpd_detection_setup(dev_priv); > } > > +static u32 ilk_hotplug_mask(struct drm_i915_private *i915, enum hpd_pin hpd_pin) > +{ > + switch (hpd_pin) { > + case HPD_PORT_A: > + return DIGITAL_PORTA_HOTPLUG_ENABLE | > + DIGITAL_PORTA_PULSE_DURATION_MASK; > + default: > + return 0; > + } > +} > + > static u32 ilk_hotplug_enables(struct intel_encoder *encoder) > { > switch (encoder->hpd_pin) { > case HPD_PORT_A: > @@ -3109,9 +3185,9 @@ static void ilk_hpd_detection_setup(struct drm_i915_private *dev_priv) > * duration to 2ms (which is the minimum in the Display Port spec) > * The pulse duration bits are reserved on HSW+. > */ > intel_uncore_rmw(&dev_priv->uncore, DIGITAL_PORT_HOTPLUG_CNTRL, > - DIGITAL_PORTA_HOTPLUG_ENABLE | DIGITAL_PORTA_PULSE_DURATION_MASK, > + ilk_hotplug_mask(dev_priv, HPD_PORT_A), > intel_hpd_hotplug_enables(dev_priv, ilk_hotplug_enables)); > } > > static void ilk_hpd_irq_setup(struct drm_i915_private *dev_priv) > @@ -3130,8 +3206,22 @@ static void ilk_hpd_irq_setup(struct drm_i915_private *dev_priv) > > ibx_hpd_irq_setup(dev_priv); > } > > +static u32 bxt_hotplug_mask(struct drm_i915_private *i915, enum hpd_pin hpd_pin) > +{ > + switch (hpd_pin) { > + case HPD_PORT_A: > + return PORTA_HOTPLUG_ENABLE | BXT_DDIA_HPD_INVERT; > + case HPD_PORT_B: > + return PORTB_HOTPLUG_ENABLE | BXT_DDIB_HPD_INVERT; > + case HPD_PORT_C: > + return PORTC_HOTPLUG_ENABLE | BXT_DDIC_HPD_INVERT; > + default: > + return 0; > + } > +} > + > static u32 bxt_hotplug_enables(struct intel_encoder *encoder) > { > u32 hotplug; > > @@ -3158,12 +3248,11 @@ static u32 bxt_hotplug_enables(struct intel_encoder *encoder) > > static void bxt_hpd_detection_setup(struct drm_i915_private *dev_priv) > { > intel_uncore_rmw(&dev_priv->uncore, PCH_PORT_HOTPLUG, > - PORTA_HOTPLUG_ENABLE | > - PORTB_HOTPLUG_ENABLE | > - PORTC_HOTPLUG_ENABLE | > - BXT_DDI_HPD_INVERT_MASK, > + bxt_hotplug_mask(dev_priv, HPD_PORT_A) | > + bxt_hotplug_mask(dev_priv, HPD_PORT_B) | > + bxt_hotplug_mask(dev_priv, HPD_PORT_C), > intel_hpd_hotplug_enables(dev_priv, bxt_hotplug_enables)); > } > > static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv) -- Jani Nikula, Intel Open Source Graphics Center