[PATCH 0/7] drm/i915 hotplug cleanup

[PATCH 0/7] drm/i915 hotplug cleanup

[Jani Nikula]

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(-)

-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/7] drm/i915: reduce indent in i9xx_hpd_irq_handler

[Jani Nikula]

Bail out early if nothing to do. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e6bb72dca3ff..f0206f63617e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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)
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/7] drm/i915: reduce duplicate conditions in i9xx_hpd_irq_handler

[Jani Nikula]

Move dp aux irq handling within the same branch instead of duplicating
the conditions. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f0206f63617e..333b3d9dd5fc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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)
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/7] drm/i915: reduce indent in intel_hpd_irq_handler

[Jani Nikula]

Continue to loop early if there's nothing to do. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 49 ++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 333b3d9dd5fc..48e5b79a7929 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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++) {
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
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

[Jani Nikula]

There 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 <jani.nikula@intel.com>
---
 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(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a238889630d8..ca05b3f0b041 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -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);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 884b4f9b81c4..a051a0241883 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -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)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3f6cad96ee91..213273a552fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -217,6 +217,36 @@ enum hpd_pin {
 	HPD_NUM_PINS
 };
 
+struct i915_hotplug {
+	struct work_struct hotplug_work;
+
+	struct {
+		unsigned long last_jiffies;
+		int count;
+		enum {
+			HPD_ENABLED = 0,
+			HPD_DISABLED = 1,
+			HPD_MARK_DISABLED = 2
+		} state;
+	} stats[HPD_NUM_PINS];
+	u32 event_bits;
+	struct delayed_work reenable_work;
+
+	struct intel_digital_port *irq_port[I915_MAX_PORTS];
+	u32 long_port_mask;
+	u32 short_port_mask;
+	struct work_struct dig_port_work;
+
+	/*
+	 * if we get a HPD irq from DP and a HPD irq from non-DP
+	 * the non-DP HPD could block the workqueue on a mode config
+	 * mutex getting, that userspace may have taken. However
+	 * userspace is waiting on the DP workqueue to run which is
+	 * blocked behind the non-DP one.
+	 */
+	struct workqueue_struct *dp_wq;
+};
+
 #define I915_GEM_GPU_DOMAINS \
 	(I915_GEM_DOMAIN_RENDER | \
 	 I915_GEM_DOMAIN_SAMPLER | \
@@ -1679,19 +1709,7 @@ struct drm_i915_private {
 	u32 pm_rps_events;
 	u32 pipestat_irq_mask[I915_MAX_PIPES];
 
-	struct work_struct hotplug_work;
-	struct {
-		unsigned long hpd_last_jiffies;
-		int hpd_cnt;
-		enum {
-			HPD_ENABLED = 0,
-			HPD_DISABLED = 1,
-			HPD_MARK_DISABLED = 2
-		} hpd_mark;
-	} hpd_stats[HPD_NUM_PINS];
-	u32 hpd_event_bits;
-	struct delayed_work hotplug_reenable_work;
-
+	struct i915_hotplug hotplug;
 	struct i915_fbc fbc;
 	struct i915_drrs drrs;
 	struct intel_opregion opregion;
@@ -1857,20 +1875,6 @@ 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;
-
-	/*
-	 * if we get a HPD irq from DP and a HPD irq from non-DP
-	 * the non-DP HPD could block the workqueue on a mode config
-	 * mutex getting, that userspace may have taken. However
-	 * userspace is waiting on the DP workqueue to run which is
-	 * blocked behind the non-DP one.
-	 */
-	struct workqueue_struct *dp_wq;
-
 	/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
 	struct {
 		int (*execbuf_submit)(struct drm_device *dev, struct drm_file *file,
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 48e5b79a7929..91cb0b6ec47b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -832,23 +832,23 @@ static bool intel_hpd_irq_event(struct drm_device *dev,
 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);
+		container_of(work, struct drm_i915_private, hotplug.dig_port_work);
 	u32 long_port_mask, short_port_mask;
 	struct intel_digital_port *intel_dig_port;
 	int i;
 	u32 old_bits = 0;
 
 	spin_lock_irq(&dev_priv->irq_lock);
-	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;
+	long_port_mask = dev_priv->hotplug.long_port_mask;
+	dev_priv->hotplug.long_port_mask = 0;
+	short_port_mask = dev_priv->hotplug.short_port_mask;
+	dev_priv->hotplug.short_port_mask = 0;
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	for (i = 0; i < I915_MAX_PORTS; i++) {
 		bool valid = false;
 		bool long_hpd = false;
-		intel_dig_port = dev_priv->hpd_irq_port[i];
+		intel_dig_port = dev_priv->hotplug.irq_port[i];
 		if (!intel_dig_port || !intel_dig_port->hpd_pulse)
 			continue;
 
@@ -871,9 +871,9 @@ static void i915_digport_work_func(struct work_struct *work)
 
 	if (old_bits) {
 		spin_lock_irq(&dev_priv->irq_lock);
-		dev_priv->hpd_event_bits |= old_bits;
+		dev_priv->hotplug.event_bits |= old_bits;
 		spin_unlock_irq(&dev_priv->irq_lock);
-		schedule_work(&dev_priv->hotplug_work);
+		schedule_work(&dev_priv->hotplug.hotplug_work);
 	}
 }
 
@@ -885,7 +885,7 @@ static void i915_digport_work_func(struct work_struct *work)
 static void i915_hotplug_work_func(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
-		container_of(work, struct drm_i915_private, hotplug_work);
+		container_of(work, struct drm_i915_private, hotplug.hotplug_work);
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct intel_connector *intel_connector;
@@ -900,20 +900,20 @@ static void i915_hotplug_work_func(struct work_struct *work)
 
 	spin_lock_irq(&dev_priv->irq_lock);
 
-	hpd_event_bits = dev_priv->hpd_event_bits;
-	dev_priv->hpd_event_bits = 0;
+	hpd_event_bits = dev_priv->hotplug.event_bits;
+	dev_priv->hotplug.event_bits = 0;
 	list_for_each_entry(connector, &mode_config->connector_list, head) {
 		intel_connector = to_intel_connector(connector);
 		if (!intel_connector->encoder)
 			continue;
 		intel_encoder = intel_connector->encoder;
 		if (intel_encoder->hpd_pin > HPD_NONE &&
-		    dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED &&
+		    dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_MARK_DISABLED &&
 		    connector->polled == DRM_CONNECTOR_POLL_HPD) {
 			DRM_INFO("HPD interrupt storm detected on connector %s: "
 				 "switching from hotplug detection to polling\n",
 				connector->name);
-			dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark = HPD_DISABLED;
+			dev_priv->hotplug.stats[intel_encoder->hpd_pin].state = HPD_DISABLED;
 			connector->polled = DRM_CONNECTOR_POLL_CONNECT
 				| DRM_CONNECTOR_POLL_DISCONNECT;
 			hpd_disabled = true;
@@ -928,7 +928,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	  * some connectors */
 	if (hpd_disabled) {
 		drm_kms_helper_poll_enable(dev);
-		mod_delayed_work(system_wq, &dev_priv->hotplug_reenable_work,
+		mod_delayed_work(system_wq, &dev_priv->hotplug.reenable_work,
 				 msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
 	}
 
@@ -1448,7 +1448,7 @@ static void intel_hpd_irq_handler(struct drm_device *dev,
 			continue;
 
 		port = get_port_from_pin(i);
-		if (!port || !dev_priv->hpd_irq_port[port])
+		if (!port || !dev_priv->hotplug.irq_port[port])
 			continue;
 
 		if (!HAS_GMCH_DISPLAY(dev_priv)) {
@@ -1466,11 +1466,11 @@ static void intel_hpd_irq_handler(struct drm_device *dev,
 		 * but we still want HPD storm detection to function.
 		 */
 		if (long_hpd) {
-			dev_priv->long_hpd_port_mask |= (1 << port);
+			dev_priv->hotplug.long_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);
+			dev_priv->hotplug.short_port_mask |= (1 << port);
 			hotplug_trigger &= ~hpd[i];
 		}
 
@@ -1479,7 +1479,7 @@ static void intel_hpd_irq_handler(struct drm_device *dev,
 
 	for (i = 1; i < HPD_NUM_PINS; i++) {
 		if (hpd[i] & hotplug_trigger &&
-		    dev_priv->hpd_stats[i].hpd_mark == HPD_DISABLED) {
+		    dev_priv->hotplug.stats[i].state == HPD_DISABLED) {
 			/*
 			 * On GMCH platforms the interrupt mask bits only
 			 * prevent irq generation, not the setting of the
@@ -1494,29 +1494,29 @@ static void intel_hpd_irq_handler(struct drm_device *dev,
 		}
 
 		if (!(hpd[i] & hotplug_trigger) ||
-		    dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED)
+		    dev_priv->hotplug.stats[i].state != HPD_ENABLED)
 			continue;
 
 		if (!(dig_port_mask & hpd[i])) {
-			dev_priv->hpd_event_bits |= (1 << i);
+			dev_priv->hotplug.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
+		if (!time_in_range(jiffies, dev_priv->hotplug.stats[i].last_jiffies,
+				   dev_priv->hotplug.stats[i].last_jiffies
 				   + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) {
-			dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies;
-			dev_priv->hpd_stats[i].hpd_cnt = 0;
+			dev_priv->hotplug.stats[i].last_jiffies = jiffies;
+			dev_priv->hotplug.stats[i].count = 0;
 			DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - cnt: 0\n", i);
-		} else if (dev_priv->hpd_stats[i].hpd_cnt > HPD_STORM_THRESHOLD) {
-			dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED;
-			dev_priv->hpd_event_bits &= ~(1 << i);
+		} else if (dev_priv->hotplug.stats[i].count > HPD_STORM_THRESHOLD) {
+			dev_priv->hotplug.stats[i].state = HPD_MARK_DISABLED;
+			dev_priv->hotplug.event_bits &= ~(1 << i);
 			DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i);
 			storm_detected = true;
 		} else {
-			dev_priv->hpd_stats[i].hpd_cnt++;
+			dev_priv->hotplug.stats[i].count++;
 			DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - cnt: %d\n", i,
-				      dev_priv->hpd_stats[i].hpd_cnt);
+				      dev_priv->hotplug.stats[i].count);
 		}
 	}
 
@@ -1531,9 +1531,9 @@ static void intel_hpd_irq_handler(struct drm_device *dev,
 	 * deadlock.
 	 */
 	if (queue_dig)
-		queue_work(dev_priv->dp_wq, &dev_priv->dig_port_work);
+		queue_work(dev_priv->hotplug.dp_wq, &dev_priv->hotplug.dig_port_work);
 	if (queue_hp)
-		schedule_work(&dev_priv->hotplug_work);
+		schedule_work(&dev_priv->hotplug.hotplug_work);
 }
 
 static void gmbus_irq_handler(struct drm_device *dev)
@@ -3213,12 +3213,12 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
 	if (HAS_PCH_IBX(dev)) {
 		hotplug_irqs = SDE_HOTPLUG_MASK;
 		for_each_intel_encoder(dev, intel_encoder)
-			if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+			if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
 				enabled_irqs |= hpd_ibx[intel_encoder->hpd_pin];
 	} else {
 		hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
 		for_each_intel_encoder(dev, intel_encoder)
-			if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+			if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
 				enabled_irqs |= hpd_cpt[intel_encoder->hpd_pin];
 	}
 
@@ -3247,7 +3247,7 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 
 	/* Now, enable HPD */
 	for_each_intel_encoder(dev, intel_encoder) {
-		if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark
+		if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
 				== HPD_ENABLED)
 			hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
 	}
@@ -4140,7 +4140,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
 	/* Note HDMI and DP share hotplug bits */
 	/* enable bits are the same for all generations */
 	for_each_intel_encoder(dev, intel_encoder)
-		if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+		if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
 			hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
 	/* Programming the CRT detection parameters tends
 	   to generate a spurious hotplug event about three
@@ -4284,7 +4284,7 @@ static void intel_hpd_irq_reenable_work(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
 		container_of(work, typeof(*dev_priv),
-			     hotplug_reenable_work.work);
+			     hotplug.reenable_work.work);
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	int i;
@@ -4295,10 +4295,10 @@ static void intel_hpd_irq_reenable_work(struct work_struct *work)
 	for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) {
 		struct drm_connector *connector;
 
-		if (dev_priv->hpd_stats[i].hpd_mark != HPD_DISABLED)
+		if (dev_priv->hotplug.stats[i].state != HPD_DISABLED)
 			continue;
 
-		dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
+		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
 
 		list_for_each_entry(connector, &mode_config->connector_list, head) {
 			struct intel_connector *intel_connector = to_intel_connector(connector);
@@ -4331,8 +4331,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
-	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->hotplug.hotplug_work, i915_hotplug_work_func);
+	INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func);
 	INIT_WORK(&dev_priv->rps.work, gen6_pm_rps_work);
 	INIT_WORK(&dev_priv->l3_parity.error_work, ivybridge_parity_work);
 
@@ -4345,7 +4345,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 
 	INIT_DELAYED_WORK(&dev_priv->gpu_error.hangcheck_work,
 			  i915_hangcheck_elapsed);
-	INIT_DELAYED_WORK(&dev_priv->hotplug_reenable_work,
+	INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
 			  intel_hpd_irq_reenable_work);
 
 	pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
@@ -4451,8 +4451,8 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
 	int i;
 
 	for (i = 1; i < HPD_NUM_PINS; i++) {
-		dev_priv->hpd_stats[i].hpd_cnt = 0;
-		dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
+		dev_priv->hotplug.stats[i].count = 0;
+		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
 	}
 	list_for_each_entry(connector, &mode_config->connector_list, head) {
 		struct intel_connector *intel_connector = to_intel_connector(connector);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cacb07b7a8f1..62a7c7f2b5d4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -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
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index abd442af4127..5d6aaf7cffa8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -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) {
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/7] drm/i915: abstract port hotplug long pulse detection more

[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 <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 91cb0b6ec47b..90cc248691d7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -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");
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/7] drm/i915: remove useless DP and DDI encoder ->hot_plug hooks

[Jani Nikula]

The hotplug callbacks for DP and DDI effectively did nothing. Remove
them.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 15 ---------------
 drivers/gpu/drm/i915/intel_dp.c  |  7 -------
 2 files changed, 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 62a7c7f2b5d4..54b60a97ef2d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -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))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5d6aaf7cffa8..535cee3b149a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -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;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/7] drm/i915/dsi: remove non-op hot plug callback

[Jani Nikula]

Not needed or used.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 51966426addf..3a93cdac757a 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -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;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: abstract port hotplug long pulse detection more

[Daniel Vetter]

On Wed, May 27, 2015 at 03:03:43PM +0300, Jani Nikula wrote:
> 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 <jani.nikula@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 40 ++++++++++++++++------------------------
>  1 file changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 91cb0b6ec47b..90cc248691d7 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -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");
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: abstract port hotplug long pulse detection more

[Daniel Vetter]

On Wed, May 27, 2015 at 02:41:29PM +0200, Daniel Vetter wrote:
> On Wed, May 27, 2015 at 03:03:43PM +0300, Jani Nikula wrote:
> > 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 <jani.nikula@intel.com>
> 
> 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.

Forgot to add: Merged all the other patches to dinq, thanks.
-Daniel

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 40 ++++++++++++++++------------------------
> >  1 file changed, 16 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 91cb0b6ec47b..90cc248691d7 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -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");
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx