drm/i915 hotplug cleanup
Subsystems (2)
cover drm/i915 hotplug cleanup
Trying to make the hotplug code a bit nicer to work with. Hopefully more to follow.
BR,
Jani.
Jani Nikula (7):
drm/i915: reduce indent in i9xx_hpd_irq_handler drm/i915: reduce duplicate conditions in i9xx_hpd_irq_handler drm/i915: reduce indent in intel_hpd_irq_handler drm/i915: group all hotplug related fields into a new struct in
dev_priv
drm/i915: abstract port hotplug long pulse detection more drm/i915: remove useless DP and DDI encoder ->hot_plug hooks drm/i915/dsi: remove non-op hot plug callback
drivers/gpu/drm/i915/i915_dma.c | 8 +- drivers/gpu/drm/i915/i915_drv.c | 12 +-- drivers/gpu/drm/i915/i915_drv.h | 58 ++++++------ drivers/gpu/drm/i915/i915_irq.c | 187 +++++++++++++++++++-------------------- drivers/gpu/drm/i915/intel_ddi.c | 17 +--- drivers/gpu/drm/i915/intel_dp.c | 13 +-- drivers/gpu/drm/i915/intel_dsi.c | 6 -- 7 files changed, 136 insertions(+), 165 deletions(-)
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/7 drm/i915: reduce indent in i9xx_hpd_irq_handler
Bail out early if nothing to do. No functional changes.
Signed-off-by Jani Nikula <[email protected]>
--- drivers/gpu/drm/i915/i915_irq.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +18 -17
@@ -1756,28 +1756,29 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;u32 hotplug_status =I915_READ (PORT_HOTPLUG_STAT ); -if (hotplug_status) { -I915_WRITE (PORT_HOTPLUG_STAT , hotplug_status); -/* -* Make sure hotplug status is cleared before we clear IIR, or else we -* may miss hotplug events. -*/ -POSTING_READ (PORT_HOTPLUG_STAT ); +if (!hotplug_status) +return ; -if (IS_G4X (dev) ||IS_VALLEYVIEW (dev)) { -u32 hotplug_trigger = hotplug_status &HOTPLUG_INT_STATUS_G4X ; +I915_WRITE (PORT_HOTPLUG_STAT , hotplug_status); +/* +* Make sure hotplug status is cleared before we clear IIR, or else we +* may miss hotplug events. +*/ +POSTING_READ (PORT_HOTPLUG_STAT ); -intel_hpd_irq_handler (dev, hotplug_trigger,0 , hpd_status_g4x); - }else { -u32 hotplug_trigger = hotplug_status &HOTPLUG_INT_STATUS_I915 ; +if (IS_G4X (dev) ||IS_VALLEYVIEW (dev)) { +u32 hotplug_trigger = hotplug_status &HOTPLUG_INT_STATUS_G4X ; -intel_hpd_irq_handler (dev, hotplug_trigger,0 , hpd_status_i915); - } +intel_hpd_irq_handler (dev, hotplug_trigger,0 , hpd_status_g4x); + }else { +u32 hotplug_trigger = hotplug_status &HOTPLUG_INT_STATUS_I915 ; -if ((IS_G4X (dev) ||IS_VALLEYVIEW (dev)) && - hotplug_status &DP_AUX_CHANNEL_MASK_INT_STATUS_G4X ) -dp_aux_irq_handler (dev); +intel_hpd_irq_handler (dev, hotplug_trigger,0 , hpd_status_i915); } + +if ((IS_G4X (dev) ||IS_VALLEYVIEW (dev)) && + hotplug_status &DP_AUX_CHANNEL_MASK_INT_STATUS_G4X ) +dp_aux_irq_handler (dev); }static irqreturn_t valleyview_irq_handler (int irq,void *arg)
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 2/7 drm/i915: reduce duplicate conditions in i9xx_hpd_irq_handler
In reply to Jani NikulaMove dp aux irq handling within the same branch instead of duplicating the conditions. No functional changes.
Signed-off-by Jani Nikula <[email protected]>
--- drivers/gpu/drm/i915/i915_irq.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +3 -4
@@ -1770,15 +1770,14 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)u32 hotplug_trigger = hotplug_status &HOTPLUG_INT_STATUS_G4X ;intel_hpd_irq_handler (dev, hotplug_trigger,0 , hpd_status_g4x); + +if (hotplug_status &DP_AUX_CHANNEL_MASK_INT_STATUS_G4X ) +dp_aux_irq_handler (dev); }else {u32 hotplug_trigger = hotplug_status &HOTPLUG_INT_STATUS_I915 ;intel_hpd_irq_handler (dev, hotplug_trigger,0 , hpd_status_i915); } - -if ((IS_G4X (dev) ||IS_VALLEYVIEW (dev)) && - hotplug_status &DP_AUX_CHANNEL_MASK_INT_STATUS_G4X ) -dp_aux_irq_handler (dev); }static irqreturn_t valleyview_irq_handler (int irq,void *arg)
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 3/7 drm/i915: reduce indent in intel_hpd_irq_handler
In reply to Jani NikulaContinue to loop early if there's nothing to do. No functional changes.
Signed-off-by Jani Nikula <[email protected]>
--- drivers/gpu/drm/i915/i915_irq.c | 49 ++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 23 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +26 -23
@@ -1442,36 +1442,39 @@ static void intel_hpd_irq_handler(struct drm_device *dev,spin_lock (&dev_priv->irq_lock);for (i =1 ; i <HPD_NUM_PINS ; i++) { +bool long_hpd; +if (!(hpd[i] & hotplug_trigger))continue ; port =get_port_from_pin (i); -if (port && dev_priv->hpd_irq_port[port]) { -bool long_hpd; +if (!port || !dev_priv->hpd_irq_port[port]) +continue ; -if (!HAS_GMCH_DISPLAY (dev_priv)) { - dig_shift =pch_port_to_hotplug_shift (port); - long_hpd = (dig_hotplug_reg >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; - }else { - dig_shift =i915_port_to_hotplug_shift (port); - long_hpd = (hotplug_trigger >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; - } +if (!HAS_GMCH_DISPLAY (dev_priv)) { + dig_shift =pch_port_to_hotplug_shift (port); + long_hpd = (dig_hotplug_reg >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; + }else { + dig_shift =i915_port_to_hotplug_shift (port); + long_hpd = (hotplug_trigger >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; + } -DRM_DEBUG_DRIVER ("digital hpd port %c - %s\n" , -port_name (port), - long_hpd ?"long" :"short" ); -/* 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; +DRM_DEBUG_DRIVER ("digital hpd port %c - %s\n" ,port_name (port), + long_hpd ?"long" :"short" ); +/* +* 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++) {
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 4/7 drm/i915: group all hotplug related fields into a new struct in dev_priv
In reply to Jani NikulaThere are plenty of hotplug related fields in struct drm_i915_private scattered all around. Group them under one hotplug struct. Clean up naming while at it. No functional changes.
Signed-off-by Jani Nikula <[email protected]>
--- drivers/gpu/drm/i915/i915_dma.c | 8 ++-- drivers/gpu/drm/i915/i915_drv.c | 12 +++--- drivers/gpu/drm/i915/i915_drv.h | 58 ++++++++++++++------------- drivers/gpu/drm/i915/i915_irq.c | 86 ++++++++++++++++++++-------------------- drivers/gpu/drm/i915/intel_ddi.c | 2 +- drivers/gpu/drm/i915/intel_dp.c | 6 +-- 6 files changed, 88 insertions(+), 84 deletions(-)
drivers/gpu/drm/i915/i915_dma.c +4 -4
@@ -933,8 +933,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_mtrrfree; } - dev_priv->dp_wq =alloc_ordered_workqueue ("i915-dp" ,0 ); -if (dev_priv->dp_wq ==NULL ) { + dev_priv->hotplug.dp_wq =alloc_ordered_workqueue ("i915-dp" ,0 ); +if (dev_priv->hotplug.dp_wq ==NULL ) {DRM_ERROR ("Failed to create our dp workqueue.\n" ); ret = -ENOMEM ; goto out_freewq;
@@ -1029,7 +1029,7 @@ out_gem_unload:pm_qos_remove_request (&dev_priv->pm_qos);destroy_workqueue (dev_priv->gpu_error.hangcheck_wq); out_freedpwq: -destroy_workqueue (dev_priv->dp_wq); +destroy_workqueue (dev_priv->hotplug.dp_wq); out_freewq:destroy_workqueue (dev_priv->wq); out_mtrrfree:
@@ -1123,7 +1123,7 @@ int i915_driver_unload(struct drm_device *dev)intel_teardown_gmbus (dev);intel_teardown_mchbar (dev); -destroy_workqueue (dev_priv->dp_wq); +destroy_workqueue (dev_priv->hotplug.dp_wq);destroy_workqueue (dev_priv->wq);destroy_workqueue (dev_priv->gpu_error.hangcheck_wq);pm_qos_remove_request (&dev_priv->pm_qos);
drivers/gpu/drm/i915/i915_drv.c +6 -6
@@ -545,15 +545,15 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) {spin_lock_irq (&dev_priv->irq_lock); - dev_priv->long_hpd_port_mask =0 ; - dev_priv->short_hpd_port_mask =0 ; - dev_priv->hpd_event_bits =0 ; + dev_priv->hotplug.long_port_mask =0 ; + dev_priv->hotplug.short_port_mask =0 ; + dev_priv->hotplug.event_bits =0 ;spin_unlock_irq (&dev_priv->irq_lock); -cancel_work_sync (&dev_priv->dig_port_work); -cancel_work_sync (&dev_priv->hotplug_work); -cancel_delayed_work_sync (&dev_priv->hotplug_reenable_work); +cancel_work_sync (&dev_priv->hotplug.dig_port_work); +cancel_work_sync (&dev_priv->hotplug.hotplug_work); +cancel_delayed_work_sync (&dev_priv->hotplug.reenable_work); }void i915_firmware_load_error_print (const char *fw_path,int err)
drivers/gpu/drm/i915/i915_drv.h +31 -27
76 lines not on this page · raw message
drivers/gpu/drm/i915/i915_irq.c +43 -43
238 lines not on this page · raw message
drivers/gpu/drm/i915/intel_ddi.c +1 -1
@@ -2832,7 +2832,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port) goto err; intel_dig_port->hpd_pulse = intel_dp_hpd_pulse; - dev_priv->hpd_irq_port[port] = intel_dig_port; + dev_priv->hotplug.irq_port[port] = intel_dig_port; }/* In theory we don't need the encoder->type check, but leave it just in
drivers/gpu/drm/i915/intel_dp.c +3 -3
@@ -5916,7 +5916,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port) 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; + dev_priv->hotplug.irq_port[port] = intel_dig_port;if (!intel_dp_init_connector (intel_dig_port, intel_connector)) {drm_encoder_cleanup (encoder);
@@ -5932,7 +5932,7 @@ void intel_dp_mst_suspend(struct drm_device *dev)/* disable MST */ for (i =0 ; i <I915_MAX_PORTS ; i++) { -struct intel_digital_port *intel_dig_port = dev_priv->hpd_irq_port[i]; +struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];if (!intel_dig_port)continue ;
@@ -5951,7 +5951,7 @@ void intel_dp_mst_resume(struct drm_device *dev)int i;for (i =0 ; i <I915_MAX_PORTS ; i++) { -struct intel_digital_port *intel_dig_port = dev_priv->hpd_irq_port[i]; +struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];if (!intel_dig_port)continue ;if (intel_dig_port->base.type ==INTEL_OUTPUT_DISPLAYPORT ) {
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 5/7 drm/i915: abstract port hotplug long pulse detection more
In reply to Jani NikulaAdd platform specific functions to decipher the register contents instead of just returning the shift value. Use macros instead of magic numbers to look at the register bits.
As a side effect, if an unsupported port is passed, consistently return false (indicating short hotplug) instead of returning -1 which was used for shifting.
Signed-off-by Jani Nikula <[email protected]>
--- drivers/gpu/drm/i915/i915_irq.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +16 -24
@@ -1375,35 +1375,31 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,#define HPD_STORM_DETECT_PERIOD 1000#define HPD_STORM_THRESHOLD 5 -static int pch_port_to_hotplug_shift (enum port port) +static bool pch_port_hotplug_long_detect (enum port port,u32 val) {switch (port) { -case PORT_A : -case PORT_E : -default : -return -1 ;case PORT_B : -return 0 ; +return val &PORTB_HOTPLUG_LONG_DETECT ;case PORT_C : -return 8 ; +return val &PORTC_HOTPLUG_LONG_DETECT ;case PORT_D : -return 16 ; +return val &PORTD_HOTPLUG_LONG_DETECT ; +default : +return false; } } -static int i915_port_to_hotplug_shift (enum port port) +static bool i915_port_hotplug_long_detect (enum port port,u32 val) {switch (port) { -case PORT_A : -case PORT_E : -default : -return -1 ;case PORT_B : -return 17 ; +return val &PORTB_HOTPLUG_INT_LONG_PULSE ;case PORT_C : -return 19 ; +return val &PORTC_HOTPLUG_INT_LONG_PULSE ;case PORT_D : -return 21 ; +return val &PORTD_HOTPLUG_INT_LONG_PULSE ; +default : +return false; } }
@@ -1431,7 +1427,6 @@ static void intel_hpd_irq_handler(struct drm_device *dev,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)
@@ -1451,13 +1446,10 @@ static void intel_hpd_irq_handler(struct drm_device *dev,if (!port || !dev_priv->hotplug.irq_port[port])continue ; -if (!HAS_GMCH_DISPLAY (dev_priv)) { - dig_shift =pch_port_to_hotplug_shift (port); - long_hpd = (dig_hotplug_reg >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; - }else { - dig_shift =i915_port_to_hotplug_shift (port); - long_hpd = (hotplug_trigger >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; - } +if (HAS_GMCH_DISPLAY (dev_priv)) + long_hpd =i915_port_hotplug_long_detect (port, hotplug_trigger); +else + long_hpd =pch_port_hotplug_long_detect (port, dig_hotplug_reg);DRM_DEBUG_DRIVER ("digital hpd port %c - %s\n" ,port_name (port), long_hpd ?"long" :"short" );
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 6/7 drm/i915: remove useless DP and DDI encoder ->hot_plug hooks
In reply to Jani NikulaThe hotplug callbacks for DP and DDI effectively did nothing. Remove them.
Signed-off-by Jani Nikula <[email protected]>
--- drivers/gpu/drm/i915/intel_ddi.c | 15 --------------- drivers/gpu/drm/i915/intel_dp.c | 7 ------- 2 files changed, 22 deletions(-)
drivers/gpu/drm/i915/intel_ddi.c +0 -15
@@ -2618,20 +2618,6 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc)I915_WRITE (_FDI_RXA_CTL, val); } -static void intel_ddi_hot_plug (struct intel_encoder *intel_encoder) -{ -struct intel_digital_port *intel_dig_port =enc_to_dig_port (&intel_encoder->base); -int type = intel_dig_port->base.type; - -if (type !=INTEL_OUTPUT_DISPLAYPORT && - type !=INTEL_OUTPUT_EDP && - type !=INTEL_OUTPUT_UNKNOWN ) { -return ; - } - -intel_dp_hot_plug (intel_encoder); -} -void intel_ddi_get_config (struct intel_encoder *encoder,struct intel_crtc_state *pipe_config) {
@@ -2825,7 +2811,6 @@ void intel_ddi_init(struct drm_device *dev, enum port port) intel_encoder->type =INTEL_OUTPUT_UNKNOWN ; intel_encoder->crtc_mask = (1 <<0 ) | (1 <<1 ) | (1 <<2 ); intel_encoder->cloneable =0 ; - intel_encoder->hot_plug = intel_ddi_hot_plug;if (init_dp) {if (!intel_ddi_init_dp_connector (intel_dig_port))
drivers/gpu/drm/i915/intel_dp.c +0 -7
@@ -4896,12 +4896,6 @@ static const struct drm_encoder_funcs intel_dp_enc_funcs = { .destroy = intel_dp_encoder_destroy, }; -void -intel_dp_hot_plug (struct intel_encoder *intel_encoder) -{ -return ; -} -enum irqreturn intel_dp_hpd_pulse (struct intel_digital_port *intel_dig_port,bool long_hpd) {
@@ -5913,7 +5907,6 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port) intel_encoder->crtc_mask = (1 <<0 ) | (1 <<1 ) | (1 <<2 ); } intel_encoder->cloneable =0 ; - intel_encoder->hot_plug = intel_dp_hot_plug; intel_dig_port->hpd_pulse = intel_dp_hpd_pulse; dev_priv->hotplug.irq_port[port] = intel_dig_port;
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 7/7 drm/i915/dsi: remove non-op hot plug callback
In reply to Jani NikulaNot needed or used.
Signed-off-by Jani Nikula <[email protected]>
--- drivers/gpu/drm/i915/intel_dsi.c | 6 ------ 1 file changed, 6 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +0 -6
@@ -261,11 +261,6 @@ static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)return intel_dsi->operation_mode ==INTEL_DSI_COMMAND_MODE ; } -static void intel_dsi_hot_plug (struct intel_encoder *encoder) -{ -DRM_DEBUG_KMS ("\n" ); -} -static bool intel_dsi_compute_config (struct intel_encoder *encoder,struct intel_crtc_state *config) {
@@ -1022,7 +1017,6 @@ void intel_dsi_init(struct drm_device *dev)drm_encoder_init (dev, encoder, &intel_dsi_funcs,DRM_MODE_ENCODER_DSI );/* XXX: very likely not all of these are needed */ - intel_encoder->hot_plug = intel_dsi_hot_plug; intel_encoder->compute_config = intel_dsi_compute_config; intel_encoder->pre_pll_enable = intel_dsi_pre_pll_enable; intel_encoder->pre_enable = intel_dsi_pre_enable;
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 5/7] drm/i915: abstract port hotplug long pulse detection more
In reply to Jani NikulaQuoted 9 lines · Jani Nikula
Add platform specific functions to decipher the register contents instead of just returning the shift value. Use macros instead of magic numbers to look at the register bits.
As a side effect, if an unsupported port is passed, consistently return false (indicating short hotplug) instead of returning -1 which was used for shifting.
Signed-off-by Jani Nikula <[email protected]>
Not convinced. Currently we have a strange mix of passing register+decode tables to intel_hpd_irq_handler for hpds and decoding the long vs. short stuff internally. That already doesn't work with things like port A which tends to be (at least on some platforms) in a totally different register. And it splits up the definitions for the hpd bits from the long vs. short bits unecessarily. I even suspect we might have some bugs with correctly clearing those additional status registers sometimes.
Imo if we clean this up we should pull it all together into one place, i.e. platform code does all the register reading&decoding and passes decode bitmasks down to generic handlers. Or something along those lines at least.
-Daniel
Quoted 89 lines
--- drivers/gpu/drm/i915/i915_irq.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-)drivers/gpu/drm/i915/i915_irq.c +16
-24@@ -1375,35 +1375,31 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,#define HPD_STORM_DETECT_PERIOD 1000#define HPD_STORM_THRESHOLD 5 -static int pch_port_to_hotplug_shift (enum port port) +static bool pch_port_hotplug_long_detect (enum port port,u32 val) {switch (port) { -case PORT_A : -case PORT_E : -default : -return -1 ;case PORT_B : -return 0 ; +return val &PORTB_HOTPLUG_LONG_DETECT ;case PORT_C : -return 8 ; +return val &PORTC_HOTPLUG_LONG_DETECT ;case PORT_D : -return 16 ; +return val &PORTD_HOTPLUG_LONG_DETECT ; +default : +return false; } } -static int i915_port_to_hotplug_shift (enum port port) +static bool i915_port_hotplug_long_detect (enum port port,u32 val) {switch (port) { -case PORT_A : -case PORT_E : -default : -return -1 ;case PORT_B : -return 17 ; +return val &PORTB_HOTPLUG_INT_LONG_PULSE ;case PORT_C : -return 19 ; +return val &PORTC_HOTPLUG_INT_LONG_PULSE ;case PORT_D : -return 21 ; +return val &PORTD_HOTPLUG_INT_LONG_PULSE ; +default : +return false; } }@@ -1431,7 +1427,6 @@ static void intel_hpd_irq_handler(struct drm_device *dev,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)@@ -1451,13 +1446,10 @@ static void intel_hpd_irq_handler(struct drm_device *dev,if (!port || !dev_priv->hotplug.irq_port[port])continue ; -if (!HAS_GMCH_DISPLAY (dev_priv)) { - dig_shift =pch_port_to_hotplug_shift (port); - long_hpd = (dig_hotplug_reg >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; - }else { - dig_shift =i915_port_to_hotplug_shift (port); - long_hpd = (hotplug_trigger >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; - } +if (HAS_GMCH_DISPLAY (dev_priv)) + long_hpd =i915_port_hotplug_long_detect (port, hotplug_trigger); +else + long_hpd =pch_port_hotplug_long_detect (port, dig_hotplug_reg);DRM_DEBUG_DRIVER ("digital hpd port %c - %s\n" ,port_name (port), long_hpd ?"long" :"short" );Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 5/7] drm/i915: abstract port hotplug long pulse detection more
Quoted 19 lines · Daniel Vetter
Quoted 9 lines · Jani Nikula
Add platform specific functions to decipher the register contents instead of just returning the shift value. Use macros instead of magic numbers to look at the register bits.
As a side effect, if an unsupported port is passed, consistently return false (indicating short hotplug) instead of returning -1 which was used for shifting.
Signed-off-by Jani Nikula <[email protected]>Not convinced. Currently we have a strange mix of passing register+decode tables to
intel_hpd_irq_handlerfor hpds and decoding the long vs. short stuff internally. That already doesn't work with things like port A which tends to be (at least on some platforms) in a totally different register. And it splits up the definitions for the hpd bits from the long vs. short bits unecessarily. I even suspect we might have some bugs with correctly clearing those additional status registers sometimes.
Quoted 4 lines
Imo if we clean this up we should pull it all together into one place, i.e. platform code does all the register reading&decoding and passes decode bitmasks down to generic handlers. Or something along those lines at least.
Forgot to add: Merged all the other patches to dinq, thanks. -Daniel
Quoted 96 lines
-Daniel
Quoted 89 lines
--- drivers/gpu/drm/i915/i915_irq.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-)drivers/gpu/drm/i915/i915_irq.c +16
-24@@ -1375,35 +1375,31 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,#define HPD_STORM_DETECT_PERIOD 1000#define HPD_STORM_THRESHOLD 5 -static int pch_port_to_hotplug_shift (enum port port) +static bool pch_port_hotplug_long_detect (enum port port,u32 val) {switch (port) { -case PORT_A : -case PORT_E : -default : -return -1 ;case PORT_B : -return 0 ; +return val &PORTB_HOTPLUG_LONG_DETECT ;case PORT_C : -return 8 ; +return val &PORTC_HOTPLUG_LONG_DETECT ;case PORT_D : -return 16 ; +return val &PORTD_HOTPLUG_LONG_DETECT ; +default : +return false; } } -static int i915_port_to_hotplug_shift (enum port port) +static bool i915_port_hotplug_long_detect (enum port port,u32 val) {switch (port) { -case PORT_A : -case PORT_E : -default : -return -1 ;case PORT_B : -return 17 ; +return val &PORTB_HOTPLUG_INT_LONG_PULSE ;case PORT_C : -return 19 ; +return val &PORTC_HOTPLUG_INT_LONG_PULSE ;case PORT_D : -return 21 ; +return val &PORTD_HOTPLUG_INT_LONG_PULSE ; +default : +return false; } }@@ -1431,7 +1427,6 @@ static void intel_hpd_irq_handler(struct drm_device *dev,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)@@ -1451,13 +1446,10 @@ static void intel_hpd_irq_handler(struct drm_device *dev,if (!port || !dev_priv->hotplug.irq_port[port])continue ; -if (!HAS_GMCH_DISPLAY (dev_priv)) { - dig_shift =pch_port_to_hotplug_shift (port); - long_hpd = (dig_hotplug_reg >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; - }else { - dig_shift =i915_port_to_hotplug_shift (port); - long_hpd = (hotplug_trigger >> dig_shift) &PORTB_HOTPLUG_LONG_DETECT ; - } +if (HAS_GMCH_DISPLAY (dev_priv)) + long_hpd =i915_port_hotplug_long_detect (port, hotplug_trigger); +else + long_hpd =pch_port_hotplug_long_detect (port, dig_hotplug_reg);DRM_DEBUG_DRIVER ("digital hpd port %c - %s\n" ,port_name (port), long_hpd ?"long" :"short" );Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfxSignature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx