This looks like it's good to go. As an aside, I don't *think* any of the compliance testing stuff I'm working on cares whether it's short of long pulse (1.1a compliance), but it will be interesting to see if/when/where it might have an effect.
Reviewed-by: Todd Previte <tprevite at gmail.com> > Dave Airlie <mailto:airlied at gmail.com> > Tuesday, June 17, 2014 6:29 PM > From: Dave Airlie <airlied at redhat.com> > > The digital ports from Ironlake and up have the ability to distinguish > between long and short HPD pulses. Displayport 1.1 only uses the short > form to request link retraining usually, so we haven't really needed > support for it until now. > > However with DP 1.2 MST we need to handle the short irqs on their > own outside the modesetting locking the long hpd's involve. This > patch adds the framework to distinguish between short/long to the > current code base, to lay the basis for future DP 1.2 MST work. > > This should mean we get better bisectability in case of regression > due to the new irq handling. > > v2: add GM45 support (untested, due to lack of hw) > > Signed-off-by: Dave Airlie <airlied at redhat.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 5 ++ > drivers/gpu/drm/i915/i915_irq.c | 160 > +++++++++++++++++++++++++++++++++++++-- > drivers/gpu/drm/i915/intel_ddi.c | 3 + > drivers/gpu/drm/i915/intel_dp.c | 20 +++++ > drivers/gpu/drm/i915/intel_drv.h | 2 + > 5 files changed, 182 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index 8f68678..5fd5bf3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1551,6 +1551,11 @@ struct drm_i915_private { > > struct i915_runtime_pm pm; > > + struct intel_digital_port *hpd_irq_port[I915_MAX_PORTS]; > + u32 long_hpd_port_mask; > + u32 short_hpd_port_mask; > + struct work_struct dig_port_work; > + > /* Old dri1 support infrastructure, beware the dragons ya fools entering > * here! */ > struct i915_dri1_state dri1; > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c > index cbf31cb..9913c08 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1096,6 +1096,53 @@ static bool intel_hpd_irq_event(struct > drm_device *dev, > return true; > } > > +static void i915_digport_work_func(struct work_struct *work) > +{ > + struct drm_i915_private *dev_priv = > + container_of(work, struct drm_i915_private, dig_port_work); > + unsigned long irqflags; > + u32 long_port_mask, short_port_mask; > + struct intel_digital_port *intel_dig_port; > + int i, ret; > + u32 old_bits = 0; > + > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > + long_port_mask = dev_priv->long_hpd_port_mask; > + dev_priv->long_hpd_port_mask = 0; > + short_port_mask = dev_priv->short_hpd_port_mask; > + dev_priv->short_hpd_port_mask = 0; > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > + > + for (i = 0; i < I915_MAX_PORTS; i++) { > + bool valid = false; > + bool long_hpd = false; > + intel_dig_port = dev_priv->hpd_irq_port[i]; > + if (!intel_dig_port || !intel_dig_port->hpd_pulse) > + continue; > + > + if (long_port_mask & (1 << i)) { > + valid = true; > + long_hpd = true; > + } else if (short_port_mask & (1 << i)) > + valid = true; > + > + if (valid) { > + ret = intel_dig_port->hpd_pulse(intel_dig_port, long_hpd); > + if (ret == true) { > + /* if we get true fallback to old school hpd */ > + old_bits |= (1 << intel_dig_port->base.hpd_pin); > + } > + } > + } > + > + if (old_bits) { > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > + dev_priv->hpd_event_bits |= old_bits; > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > + schedule_work(&dev_priv->hotplug_work); > + } > +} > + > /* > * Handle hotplug events outside the interrupt handler proper. > */ > @@ -1520,23 +1567,104 @@ static irqreturn_t gen8_gt_irq_handler(struct > drm_device *dev, > #define HPD_STORM_DETECT_PERIOD 1000 > #define HPD_STORM_THRESHOLD 5 > > +static int ilk_port_to_hotplug_shift(enum port port) > +{ > + switch (port) { > + case PORT_A: > + case PORT_E: > + default: > + return -1; > + case PORT_B: > + return 0; > + case PORT_C: > + return 8; > + case PORT_D: > + return 16; > + } > +} > + > +static int g4x_port_to_hotplug_shift(enum port port) > +{ > + switch (port) { > + case PORT_A: > + case PORT_E: > + default: > + return -1; > + case PORT_B: > + return 17; > + case PORT_C: > + return 19; > + case PORT_D: > + return 21; > + } > +} > + > +static inline enum port get_port_from_pin(enum hpd_pin pin) > +{ > + switch (pin) { > + case HPD_PORT_B: > + return PORT_B; > + case HPD_PORT_C: > + return PORT_C; > + case HPD_PORT_D: > + return PORT_D; > + default: > + return PORT_A; /* no hpd */ > + } > +} > + > static inline void intel_hpd_irq_handler(struct drm_device *dev, > u32 hotplug_trigger, > + u32 dig_hotplug_reg, > const u32 *hpd) > { > struct drm_i915_private *dev_priv = dev->dev_private; > int i; > + enum port port; > bool storm_detected = false; > + bool queue_dig = false, queue_hp = false; > + u32 dig_shift; > + u32 dig_port_mask = 0; > > if (!hotplug_trigger) > return; > > - DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", > - hotplug_trigger); > + DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, dig 0x%08x\n", > + hotplug_trigger, dig_hotplug_reg); > > spin_lock(&dev_priv->irq_lock); > for (i = 1; i < HPD_NUM_PINS; i++) { > + if (!(hpd[i] & hotplug_trigger)) > + continue; > + > + port = get_port_from_pin(i); > + if (port && dev_priv->hpd_irq_port[port]) { > + bool long_hpd; > + > + if (IS_G4X(dev)) { > + dig_shift = g4x_port_to_hotplug_shift(port); > + long_hpd = (hotplug_trigger >> dig_shift) & PORTB_HOTPLUG_LONG_DETECT; > + } else { > + dig_shift = ilk_port_to_hotplug_shift(port); > + long_hpd = (dig_hotplug_reg >> dig_shift) & PORTB_HOTPLUG_LONG_DETECT; > + } > + > + DRM_DEBUG_DRIVER("digital hpd port %d %d\n", port, long_hpd); > + /* for long HPD pulses we want to have the digital queue happen, > + but we still want HPD storm detection to function. */ > + if (long_hpd) { > + dev_priv->long_hpd_port_mask |= (1 << port); > + dig_port_mask |= hpd[i]; > + } else { > + /* for short HPD just trigger the digital queue */ > + dev_priv->short_hpd_port_mask |= (1 << port); > + hotplug_trigger &= ~hpd[i]; > + } > + queue_dig = true; > + } > + } > > + for (i = 1; i < HPD_NUM_PINS; i++) { > if (hpd[i] & hotplug_trigger && > dev_priv->hpd_stats[i].hpd_mark == HPD_DISABLED) { > /* > @@ -1556,7 +1684,11 @@ static inline void intel_hpd_irq_handler(struct > drm_device *dev, > dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) > continue; > > - dev_priv->hpd_event_bits |= (1 << i); > + if (!(dig_port_mask & hpd[i])) { > + dev_priv->hpd_event_bits |= (1 << i); > + queue_hp = true; > + } > + > if (!time_in_range(jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies, > dev_priv->hpd_stats[i].hpd_last_jiffies > + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { > @@ -1585,7 +1717,10 @@ static inline void intel_hpd_irq_handler(struct > drm_device *dev, > * queue for otherwise the flush_work in the pageflip code will > * deadlock. > */ > - schedule_work(&dev_priv->hotplug_work); > + if (queue_dig) > + schedule_work(&dev_priv->dig_port_work); > + if (queue_hp) > + schedule_work(&dev_priv->hotplug_work); > } > > static void gmbus_irq_handler(struct drm_device *dev) > @@ -1818,11 +1953,11 @@ static void i9xx_hpd_irq_handler(struct > drm_device *dev) > if (IS_G4X(dev)) { > u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X; > > - intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_g4x); > + intel_hpd_irq_handler(dev, hotplug_trigger, 0, hpd_status_g4x); > } else { > u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915; > > - intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915); > + intel_hpd_irq_handler(dev, hotplug_trigger, 0, hpd_status_i915); > } > > if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) && > @@ -1913,8 +2048,12 @@ static void ibx_irq_handler(struct drm_device > *dev, u32 pch_iir) > struct drm_i915_private *dev_priv = dev->dev_private; > int pipe; > u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK; > + u32 dig_hotplug_reg; > + > + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > > - intel_hpd_irq_handler(dev, hotplug_trigger, hpd_ibx); > + intel_hpd_irq_handler(dev, hotplug_trigger, dig_hotplug_reg, hpd_ibx); > > if (pch_iir & SDE_AUDIO_POWER_MASK) { > int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK) >> > @@ -2020,8 +2159,12 @@ static void cpt_irq_handler(struct drm_device > *dev, u32 pch_iir) > struct drm_i915_private *dev_priv = dev->dev_private; > int pipe; > u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT; > + u32 dig_hotplug_reg; > + > + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > > - intel_hpd_irq_handler(dev, hotplug_trigger, hpd_cpt); > + intel_hpd_irq_handler(dev, hotplug_trigger, dig_hotplug_reg, hpd_cpt); > > if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) { > int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK_CPT) >> > @@ -4338,6 +4481,7 @@ void intel_irq_init(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > > INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func); > + INIT_WORK(&dev_priv->dig_port_work, i915_digport_work_func); > INIT_WORK(&dev_priv->gpu_error.work, i915_error_work_func); > INIT_WORK(&dev_priv->rps.work, gen6_pm_rps_work); > INIT_WORK(&dev_priv->l3_parity.error_work, ivybridge_parity_work); > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index b17b9c7..a80cb3e 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1705,6 +1705,9 @@ void intel_ddi_init(struct drm_device *dev, enum > port port) > intel_encoder->cloneable = 0; > intel_encoder->hot_plug = intel_ddi_hot_plug; > > + intel_dig_port->hpd_pulse = intel_dp_hpd_pulse; > + dev_priv->hpd_irq_port[port] = intel_dig_port; > + > if (init_dp) > dp_connector = intel_ddi_init_dp_connector(intel_dig_port); > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > b/drivers/gpu/drm/i915/intel_dp.c > index d01bb43..f110522 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3699,6 +3699,22 @@ intel_dp_hot_plug(struct intel_encoder > *intel_encoder) > intel_dp_check_link_status(intel_dp); > } > > +bool > +intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool > long_hpd) > +{ > + struct intel_dp *intel_dp = &intel_dig_port->dp; > + > + if (long_hpd) > + return true; > + > + /* > + * we'll check the link status via the normal hot plug path later - > + * but for short hpds we should check it now > + */ > + intel_dp_check_link_status(intel_dp); > + return false; > +} > + > /* Return which DP Port should be selected for Transcoder DP control */ > int > intel_trans_dp_port_sel(struct drm_crtc *crtc) > @@ -4273,6 +4289,7 @@ intel_dp_init_connector(struct > intel_digital_port *intel_dig_port, > void > intel_dp_init(struct drm_device *dev, int output_reg, enum port port) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_digital_port *intel_dig_port; > struct intel_encoder *intel_encoder; > struct drm_encoder *encoder; > @@ -4328,6 +4345,9 @@ intel_dp_init(struct drm_device *dev, int > output_reg, enum port port) > intel_encoder->cloneable = 0; > intel_encoder->hot_plug = intel_dp_hot_plug; > > + intel_dig_port->hpd_pulse = intel_dp_hpd_pulse; > + dev_priv->hpd_irq_port[port] = intel_dig_port; > + > if (!intel_dp_init_connector(intel_dig_port, intel_connector)) { > drm_encoder_cleanup(encoder); > kfree(intel_dig_port); > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index f1d5897..9666ec3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -563,6 +563,7 @@ struct intel_digital_port { > u32 saved_port_bits; > struct intel_dp dp; > struct intel_hdmi hdmi; > + bool (*hpd_pulse)(struct intel_digital_port *, bool); > }; > > static inline int > @@ -830,6 +831,7 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp); > void intel_edp_psr_disable(struct intel_dp *intel_dp); > void intel_edp_psr_update(struct drm_device *dev); > void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate); > +bool intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, > bool long_hpd); > > /* intel_dsi.c */ > bool intel_dsi_init(struct drm_device *dev); > Dave Airlie <mailto:airlied at gmail.com> > Tuesday, June 17, 2014 6:29 PM > Can we get these merged or even looked at?, they are blocking the > whole MST progress, > and I don't have any insight to secret Intel review process. :-) > > Dave. > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sent with Postbox <http://www.getpostbox.com> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140624/cb2ac0c0/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: postbox-contact.jpg Type: image/jpeg Size: 1291 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140624/cb2ac0c0/attachment-0001.jpg>
