[PATCH 0/8] drm/i915: Hotplug cleanups and whanot

[PATCH 0/8] drm/i915: Hotplug cleanups and whanot

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

A colleciton of cleanups I had lying around, mostly in the
hotplug area, and a few other encoder related things.

Ville Syrjälä (8):
  drm/i915: Introduce for_each_intel_dp()
  drm/i915: Introduce intel_encoder_is_dig_port()
  drm/i915: Rewrite mst suspend/resume in terms of encoders
  drm/i915: Nuke dev_priv->irq_port[]
  drm/i915: s/int i/enum hpd_pin pin/
  drm/i915: Pass hpd_pin to long_pulse_detect()
  drm/i915: Assert that our hpd pin bitmasks don't overflow
  drm/i915: Print the long_mask alongside the pin_mask

 drivers/gpu/drm/i915/i915_drv.c      |   4 +-
 drivers/gpu/drm/i915/i915_drv.h      |   3 -
 drivers/gpu/drm/i915/i915_irq.c      | 111 ++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_ddi.c     |   1 -
 drivers/gpu/drm/i915/intel_display.h |   4 ++
 drivers/gpu/drm/i915/intel_dp.c      |  80 +++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h     |  44 +++++++++----
 drivers/gpu/drm/i915/intel_hotplug.c | 118 +++++++++++++----------------------
 8 files changed, 169 insertions(+), 196 deletions(-)

-- 
2.16.4

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

[PATCH 1/8] drm/i915: Introduce for_each_intel_dp()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a convenience macro for iterating DP encoders.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.h |  4 ++++
 drivers/gpu/drm/i915/intel_dp.c      | 38 +++++++-----------------------------
 drivers/gpu/drm/i915/intel_drv.h     | 14 +++++++++++++
 3 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index ca5a10f3400d..9292001cdd14 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -289,6 +289,10 @@ struct intel_link_m_n {
 			    &(dev)->mode_config.encoder_list,	\
 			    base.head)
 
+#define for_each_intel_dp(dev, intel_encoder)			\
+	for_each_intel_encoder(dev, intel_encoder)		\
+		for_each_if(intel_encoder_is_dp(intel_encoder))
+
 #define for_each_intel_connector_iter(intel_connector, iter) \
 	while ((intel_connector = to_intel_connector(drm_connector_list_iter_next(iter))))
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5be07e1d816d..e95f1181e302 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -600,14 +600,8 @@ static enum pipe vlv_find_free_pps(struct drm_i915_private *dev_priv)
 	 * We don't have power sequencer currently.
 	 * Pick one that's not used by other ports.
 	 */
-	for_each_intel_encoder(&dev_priv->drm, encoder) {
-		struct intel_dp *intel_dp;
-
-		if (encoder->type != INTEL_OUTPUT_DP &&
-		    encoder->type != INTEL_OUTPUT_EDP)
-			continue;
-
-		intel_dp = enc_to_intel_dp(&encoder->base);
+	for_each_intel_dp(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
 		if (encoder->type == INTEL_OUTPUT_EDP) {
 			WARN_ON(intel_dp->active_pipe != INVALID_PIPE &&
@@ -799,19 +793,8 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
 	 * should use them always.
 	 */
 
-	for_each_intel_encoder(&dev_priv->drm, encoder) {
-		struct intel_dp *intel_dp;
-
-		if (encoder->type != INTEL_OUTPUT_DP &&
-		    encoder->type != INTEL_OUTPUT_EDP &&
-		    encoder->type != INTEL_OUTPUT_DDI)
-			continue;
-
-		intel_dp = enc_to_intel_dp(&encoder->base);
-
-		/* Skip pure DVI/HDMI DDI encoders */
-		if (!i915_mmio_reg_valid(intel_dp->output_reg))
-			continue;
+	for_each_intel_dp(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
 		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
 
@@ -3104,16 +3087,9 @@ static void vlv_steal_power_sequencer(struct drm_i915_private *dev_priv,
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
-	for_each_intel_encoder(&dev_priv->drm, encoder) {
-		struct intel_dp *intel_dp;
-		enum port port;
-
-		if (encoder->type != INTEL_OUTPUT_DP &&
-		    encoder->type != INTEL_OUTPUT_EDP)
-			continue;
-
-		intel_dp = enc_to_intel_dp(&encoder->base);
-		port = dp_to_dig_port(intel_dp)->base.port;
+	for_each_intel_dp(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+		enum port port = encoder->port;
 
 		WARN(intel_dp->active_pipe == pipe,
 		     "stealing pipe %c power sequencer from active (e)DP port %c\n",
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e09b42d91d1e..6fe3adafaf20 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1276,6 +1276,20 @@ static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
 	return &enc_to_dig_port(encoder)->dp;
 }
 
+static inline bool intel_encoder_is_dp(struct intel_encoder *encoder)
+{
+	switch (encoder->type) {
+	case INTEL_OUTPUT_DP:
+	case INTEL_OUTPUT_EDP:
+		return true;
+	case INTEL_OUTPUT_DDI:
+		/* Skip pure HDMI/DVI DDI encoders */
+		return i915_mmio_reg_valid(enc_to_intel_dp(&encoder->base)->output_reg);
+	default:
+		return false;
+	}
+}
+
 static inline struct intel_digital_port *
 dp_to_dig_port(struct intel_dp *intel_dp)
 {
-- 
2.16.4

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

Re: [PATCH 1/8] drm/i915: Introduce for_each_intel_dp()

[Rodrigo Vivi]

On Thu, Jul 05, 2018 at 07:43:50PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add a convenience macro for iterating DP encoders.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.h |  4 ++++
>  drivers/gpu/drm/i915/intel_dp.c      | 38 +++++++-----------------------------
>  drivers/gpu/drm/i915/intel_drv.h     | 14 +++++++++++++
>  3 files changed, 25 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index ca5a10f3400d..9292001cdd14 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -289,6 +289,10 @@ struct intel_link_m_n {
>  			    &(dev)->mode_config.encoder_list,	\
>  			    base.head)
>  
> +#define for_each_intel_dp(dev, intel_encoder)			\
> +	for_each_intel_encoder(dev, intel_encoder)		\
> +		for_each_if(intel_encoder_is_dp(intel_encoder))
> +
>  #define for_each_intel_connector_iter(intel_connector, iter) \
>  	while ((intel_connector = to_intel_connector(drm_connector_list_iter_next(iter))))
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5be07e1d816d..e95f1181e302 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -600,14 +600,8 @@ static enum pipe vlv_find_free_pps(struct drm_i915_private *dev_priv)
>  	 * We don't have power sequencer currently.
>  	 * Pick one that's not used by other ports.
>  	 */
> -	for_each_intel_encoder(&dev_priv->drm, encoder) {
> -		struct intel_dp *intel_dp;
> -
> -		if (encoder->type != INTEL_OUTPUT_DP &&
> -		    encoder->type != INTEL_OUTPUT_EDP)
> -			continue;
> -
> -		intel_dp = enc_to_intel_dp(&encoder->base);
> +	for_each_intel_dp(&dev_priv->drm, encoder) {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  
>  		if (encoder->type == INTEL_OUTPUT_EDP) {
>  			WARN_ON(intel_dp->active_pipe != INVALID_PIPE &&
> @@ -799,19 +793,8 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
>  	 * should use them always.
>  	 */
>  
> -	for_each_intel_encoder(&dev_priv->drm, encoder) {
> -		struct intel_dp *intel_dp;
> -
> -		if (encoder->type != INTEL_OUTPUT_DP &&
> -		    encoder->type != INTEL_OUTPUT_EDP &&
> -		    encoder->type != INTEL_OUTPUT_DDI)
> -			continue;
> -
> -		intel_dp = enc_to_intel_dp(&encoder->base);
> -
> -		/* Skip pure DVI/HDMI DDI encoders */
> -		if (!i915_mmio_reg_valid(intel_dp->output_reg))
> -			continue;
> +	for_each_intel_dp(&dev_priv->drm, encoder) {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  
>  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
>  
> @@ -3104,16 +3087,9 @@ static void vlv_steal_power_sequencer(struct drm_i915_private *dev_priv,
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> -	for_each_intel_encoder(&dev_priv->drm, encoder) {
> -		struct intel_dp *intel_dp;
> -		enum port port;
> -
> -		if (encoder->type != INTEL_OUTPUT_DP &&
> -		    encoder->type != INTEL_OUTPUT_EDP)
> -			continue;
> -
> -		intel_dp = enc_to_intel_dp(&encoder->base);
> -		port = dp_to_dig_port(intel_dp)->base.port;
> +	for_each_intel_dp(&dev_priv->drm, encoder) {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +		enum port port = encoder->port;
>  
>  		WARN(intel_dp->active_pipe == pipe,
>  		     "stealing pipe %c power sequencer from active (e)DP port %c\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e09b42d91d1e..6fe3adafaf20 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1276,6 +1276,20 @@ static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
>  	return &enc_to_dig_port(encoder)->dp;
>  }
>  
> +static inline bool intel_encoder_is_dp(struct intel_encoder *encoder)
> +{
> +	switch (encoder->type) {
> +	case INTEL_OUTPUT_DP:
> +	case INTEL_OUTPUT_EDP:
> +		return true;
> +	case INTEL_OUTPUT_DDI:
> +		/* Skip pure HDMI/DVI DDI encoders */
> +		return i915_mmio_reg_valid(enc_to_intel_dp(&encoder->base)->output_reg);
> +	default:
> +		return false;
> +	}
> +}
> +
>  static inline struct intel_digital_port *
>  dp_to_dig_port(struct intel_dp *intel_dp)
>  {
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/8] drm/i915: Introduce intel_encoder_is_dig_port()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add intel_encoder_is_dig_port() to match intel_encoder_is_dp().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6fe3adafaf20..077189ed9e46 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1246,23 +1246,29 @@ intel_attached_encoder(struct drm_connector *connector)
 	return to_intel_connector(connector)->encoder;
 }
 
-static inline struct intel_digital_port *
-enc_to_dig_port(struct drm_encoder *encoder)
+static inline bool intel_encoder_is_dig_port(struct intel_encoder *encoder)
 {
-	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
-
-	switch (intel_encoder->type) {
+	switch (encoder->type) {
 	case INTEL_OUTPUT_DDI:
-		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
-		/* fall through */
 	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_EDP:
 	case INTEL_OUTPUT_HDMI:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static inline struct intel_digital_port *
+enc_to_dig_port(struct drm_encoder *encoder)
+{
+	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
+
+	if (intel_encoder_is_dig_port(intel_encoder))
 		return container_of(encoder, struct intel_digital_port,
 				    base.base);
-	default:
+	else
 		return NULL;
-	}
 }
 
 static inline struct intel_dp_mst_encoder *
-- 
2.16.4

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

Re: [PATCH 2/8] drm/i915: Introduce intel_encoder_is_dig_port()

[Rodrigo Vivi]

On Thu, Jul 05, 2018 at 07:43:51PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add intel_encoder_is_dig_port() to match intel_encoder_is_dp().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6fe3adafaf20..077189ed9e46 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1246,23 +1246,29 @@ intel_attached_encoder(struct drm_connector *connector)
>  	return to_intel_connector(connector)->encoder;
>  }
>  
> -static inline struct intel_digital_port *
> -enc_to_dig_port(struct drm_encoder *encoder)
> +static inline bool intel_encoder_is_dig_port(struct intel_encoder *encoder)
>  {
> -	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> -
> -	switch (intel_encoder->type) {
> +	switch (encoder->type) {
>  	case INTEL_OUTPUT_DDI:
> -		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
> -		/* fall through */
>  	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_EDP:
>  	case INTEL_OUTPUT_HDMI:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static inline struct intel_digital_port *
> +enc_to_dig_port(struct drm_encoder *encoder)
> +{
> +	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> +
> +	if (intel_encoder_is_dig_port(intel_encoder))
>  		return container_of(encoder, struct intel_digital_port,
>  				    base.base);
> -	default:
> +	else
>  		return NULL;
> -	}
>  }
>  
>  static inline struct intel_dp_mst_encoder *
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/8] drm/i915: Rewrite mst suspend/resume in terms of encoders

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than looping over all the ports and picking the encoder based on
the port, let's just loop over all the encoders instead. Gets rid of
some irq_port[] usage, which is a bit of an eye sore.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  |  4 ++--
 drivers/gpu/drm/i915/intel_dp.c  | 41 +++++++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h |  4 ++--
 3 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0db3c83cce29..1f2449d9206d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1577,7 +1577,7 @@ static int i915_drm_suspend(struct drm_device *dev)
 
 	intel_display_suspend(dev);
 
-	intel_dp_mst_suspend(dev);
+	intel_dp_mst_suspend(dev_priv);
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
 	intel_hpd_cancel_work(dev_priv);
@@ -1742,7 +1742,7 @@ static int i915_drm_resume(struct drm_device *dev)
 		dev_priv->display.hpd_irq_setup(dev_priv);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
-	intel_dp_mst_resume(dev);
+	intel_dp_mst_resume(dev_priv);
 
 	intel_display_resume(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e95f1181e302..e9bc0f9f5e54 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6490,37 +6490,44 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 	return false;
 }
 
-void intel_dp_mst_suspend(struct drm_device *dev)
+void intel_dp_mst_suspend(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	int i;
+	struct intel_encoder *encoder;
+
+	for_each_intel_encoder(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp;
+
+		if (encoder->type != INTEL_OUTPUT_DDI)
+			continue;
 
-	/* disable MST */
-	for (i = 0; i < I915_MAX_PORTS; i++) {
-		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
+		intel_dp = enc_to_intel_dp(&encoder->base);
 
-		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
+		if (!intel_dp->can_mst)
 			continue;
 
-		if (intel_dig_port->dp.is_mst)
-			drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
+		if (intel_dp->is_mst)
+			drm_dp_mst_topology_mgr_suspend(&intel_dp->mst_mgr);
 	}
 }
 
-void intel_dp_mst_resume(struct drm_device *dev)
+void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	int i;
+	struct intel_encoder *encoder;
 
-	for (i = 0; i < I915_MAX_PORTS; i++) {
-		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
+	for_each_intel_encoder(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp;
 		int ret;
 
-		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
+		if (encoder->type != INTEL_OUTPUT_DDI)
+			continue;
+
+		intel_dp = enc_to_intel_dp(&encoder->base);
+
+		if (!intel_dp->can_mst)
 			continue;
 
-		ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
+		ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr);
 		if (ret)
-			intel_dp_check_mst_status(&intel_dig_port->dp);
+			intel_dp_check_mst_status(intel_dp);
 	}
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 077189ed9e46..dc11106b2081 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1700,8 +1700,8 @@ void intel_edp_backlight_off(const struct drm_connector_state *conn_state);
 void intel_edp_panel_vdd_on(struct intel_dp *intel_dp);
 void intel_edp_panel_on(struct intel_dp *intel_dp);
 void intel_edp_panel_off(struct intel_dp *intel_dp);
-void intel_dp_mst_suspend(struct drm_device *dev);
-void intel_dp_mst_resume(struct drm_device *dev);
+void intel_dp_mst_suspend(struct drm_i915_private *dev_priv);
+void intel_dp_mst_resume(struct drm_i915_private *dev_priv);
 int intel_dp_max_link_rate(struct intel_dp *intel_dp);
 int intel_dp_max_lane_count(struct intel_dp *intel_dp);
 int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
-- 
2.16.4

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

Re: [PATCH 3/8] drm/i915: Rewrite mst suspend/resume in terms of encoders

[Rodrigo Vivi]

On Thu, Jul 05, 2018 at 07:43:52PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather than looping over all the ports and picking the encoder based on
> the port, let's just loop over all the encoders instead. Gets rid of
> some irq_port[] usage, which is a bit of an eye sore.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c  |  4 ++--
>  drivers/gpu/drm/i915/intel_dp.c  | 41 +++++++++++++++++++++++-----------------
>  drivers/gpu/drm/i915/intel_drv.h |  4 ++--
>  3 files changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0db3c83cce29..1f2449d9206d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1577,7 +1577,7 @@ static int i915_drm_suspend(struct drm_device *dev)
>  
>  	intel_display_suspend(dev);
>  
> -	intel_dp_mst_suspend(dev);
> +	intel_dp_mst_suspend(dev_priv);
>  
>  	intel_runtime_pm_disable_interrupts(dev_priv);
>  	intel_hpd_cancel_work(dev_priv);
> @@ -1742,7 +1742,7 @@ static int i915_drm_resume(struct drm_device *dev)
>  		dev_priv->display.hpd_irq_setup(dev_priv);
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
> -	intel_dp_mst_resume(dev);
> +	intel_dp_mst_resume(dev_priv);
>  
>  	intel_display_resume(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e95f1181e302..e9bc0f9f5e54 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6490,37 +6490,44 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>  	return false;
>  }
>  
> -void intel_dp_mst_suspend(struct drm_device *dev)
> +void intel_dp_mst_suspend(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int i;
> +	struct intel_encoder *encoder;
> +
> +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> +		struct intel_dp *intel_dp;
> +
> +		if (encoder->type != INTEL_OUTPUT_DDI)

INTEL_OUTPUT_DP not possible at this point?

> +			continue;
>  
> -	/* disable MST */
> -	for (i = 0; i < I915_MAX_PORTS; i++) {
> -		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> +		intel_dp = enc_to_intel_dp(&encoder->base);
>  
> -		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> +		if (!intel_dp->can_mst)
>  			continue;
>  
> -		if (intel_dig_port->dp.is_mst)
> -			drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
> +		if (intel_dp->is_mst)
> +			drm_dp_mst_topology_mgr_suspend(&intel_dp->mst_mgr);
>  	}
>  }
>  
> -void intel_dp_mst_resume(struct drm_device *dev)
> +void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int i;
> +	struct intel_encoder *encoder;
>  
> -	for (i = 0; i < I915_MAX_PORTS; i++) {
> -		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> +		struct intel_dp *intel_dp;
>  		int ret;
>  
> -		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> +		if (encoder->type != INTEL_OUTPUT_DDI)
> +			continue;
> +
> +		intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +		if (!intel_dp->can_mst)
>  			continue;
>  
> -		ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
> +		ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr);
>  		if (ret)
> -			intel_dp_check_mst_status(&intel_dig_port->dp);
> +			intel_dp_check_mst_status(intel_dp);
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 077189ed9e46..dc11106b2081 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1700,8 +1700,8 @@ void intel_edp_backlight_off(const struct drm_connector_state *conn_state);
>  void intel_edp_panel_vdd_on(struct intel_dp *intel_dp);
>  void intel_edp_panel_on(struct intel_dp *intel_dp);
>  void intel_edp_panel_off(struct intel_dp *intel_dp);
> -void intel_dp_mst_suspend(struct drm_device *dev);
> -void intel_dp_mst_resume(struct drm_device *dev);
> +void intel_dp_mst_suspend(struct drm_i915_private *dev_priv);
> +void intel_dp_mst_resume(struct drm_i915_private *dev_priv);
>  int intel_dp_max_link_rate(struct intel_dp *intel_dp);
>  int intel_dp_max_lane_count(struct intel_dp *intel_dp);
>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/8] drm/i915: Rewrite mst suspend/resume in terms of encoders

[Ville Syrjälä]

On Thu, Jul 05, 2018 at 02:29:59PM -0700, Rodrigo Vivi wrote:
> On Thu, Jul 05, 2018 at 07:43:52PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Rather than looping over all the ports and picking the encoder based on
> > the port, let's just loop over all the encoders instead. Gets rid of
> > some irq_port[] usage, which is a bit of an eye sore.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c  |  4 ++--
> >  drivers/gpu/drm/i915/intel_dp.c  | 41 +++++++++++++++++++++++-----------------
> >  drivers/gpu/drm/i915/intel_drv.h |  4 ++--
> >  3 files changed, 28 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 0db3c83cce29..1f2449d9206d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1577,7 +1577,7 @@ static int i915_drm_suspend(struct drm_device *dev)
> >  
> >  	intel_display_suspend(dev);
> >  
> > -	intel_dp_mst_suspend(dev);
> > +	intel_dp_mst_suspend(dev_priv);
> >  
> >  	intel_runtime_pm_disable_interrupts(dev_priv);
> >  	intel_hpd_cancel_work(dev_priv);
> > @@ -1742,7 +1742,7 @@ static int i915_drm_resume(struct drm_device *dev)
> >  		dev_priv->display.hpd_irq_setup(dev_priv);
> >  	spin_unlock_irq(&dev_priv->irq_lock);
> >  
> > -	intel_dp_mst_resume(dev);
> > +	intel_dp_mst_resume(dev_priv);
> >  
> >  	intel_display_resume(dev);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index e95f1181e302..e9bc0f9f5e54 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -6490,37 +6490,44 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> >  	return false;
> >  }
> >  
> > -void intel_dp_mst_suspend(struct drm_device *dev)
> > +void intel_dp_mst_suspend(struct drm_i915_private *dev_priv)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	int i;
> > +	struct intel_encoder *encoder;
> > +
> > +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> > +		struct intel_dp *intel_dp;
> > +
> > +		if (encoder->type != INTEL_OUTPUT_DDI)
> 
> INTEL_OUTPUT_DP not possible at this point?

No MST on pre-DDI platforms, and encoder->type for DDI platforms is
either DDI or EDP but never DP.

> 
> > +			continue;
> >  
> > -	/* disable MST */
> > -	for (i = 0; i < I915_MAX_PORTS; i++) {
> > -		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> > +		intel_dp = enc_to_intel_dp(&encoder->base);
> >  
> > -		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> > +		if (!intel_dp->can_mst)
> >  			continue;
> >  
> > -		if (intel_dig_port->dp.is_mst)
> > -			drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
> > +		if (intel_dp->is_mst)
> > +			drm_dp_mst_topology_mgr_suspend(&intel_dp->mst_mgr);
> >  	}
> >  }
> >  
> > -void intel_dp_mst_resume(struct drm_device *dev)
> > +void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	int i;
> > +	struct intel_encoder *encoder;
> >  
> > -	for (i = 0; i < I915_MAX_PORTS; i++) {
> > -		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> > +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> > +		struct intel_dp *intel_dp;
> >  		int ret;
> >  
> > -		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> > +		if (encoder->type != INTEL_OUTPUT_DDI)
> > +			continue;
> > +
> > +		intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> > +		if (!intel_dp->can_mst)
> >  			continue;
> >  
> > -		ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
> > +		ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr);
> >  		if (ret)
> > -			intel_dp_check_mst_status(&intel_dig_port->dp);
> > +			intel_dp_check_mst_status(intel_dp);
> >  	}
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 077189ed9e46..dc11106b2081 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1700,8 +1700,8 @@ void intel_edp_backlight_off(const struct drm_connector_state *conn_state);
> >  void intel_edp_panel_vdd_on(struct intel_dp *intel_dp);
> >  void intel_edp_panel_on(struct intel_dp *intel_dp);
> >  void intel_edp_panel_off(struct intel_dp *intel_dp);
> > -void intel_dp_mst_suspend(struct drm_device *dev);
> > -void intel_dp_mst_resume(struct drm_device *dev);
> > +void intel_dp_mst_suspend(struct drm_i915_private *dev_priv);
> > +void intel_dp_mst_resume(struct drm_i915_private *dev_priv);
> >  int intel_dp_max_link_rate(struct intel_dp *intel_dp);
> >  int intel_dp_max_lane_count(struct intel_dp *intel_dp);
> >  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
> > -- 
> > 2.16.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/8] drm/i915: Rewrite mst suspend/resume in terms of encoders

[Rodrigo Vivi]

On Fri, Jul 06, 2018 at 01:42:35PM +0300, Ville Syrjälä wrote:
> On Thu, Jul 05, 2018 at 02:29:59PM -0700, Rodrigo Vivi wrote:
> > On Thu, Jul 05, 2018 at 07:43:52PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Rather than looping over all the ports and picking the encoder based on
> > > the port, let's just loop over all the encoders instead. Gets rid of
> > > some irq_port[] usage, which is a bit of an eye sore.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c  |  4 ++--
> > >  drivers/gpu/drm/i915/intel_dp.c  | 41 +++++++++++++++++++++++-----------------
> > >  drivers/gpu/drm/i915/intel_drv.h |  4 ++--
> > >  3 files changed, 28 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index 0db3c83cce29..1f2449d9206d 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -1577,7 +1577,7 @@ static int i915_drm_suspend(struct drm_device *dev)
> > >  
> > >  	intel_display_suspend(dev);
> > >  
> > > -	intel_dp_mst_suspend(dev);
> > > +	intel_dp_mst_suspend(dev_priv);
> > >  
> > >  	intel_runtime_pm_disable_interrupts(dev_priv);
> > >  	intel_hpd_cancel_work(dev_priv);
> > > @@ -1742,7 +1742,7 @@ static int i915_drm_resume(struct drm_device *dev)
> > >  		dev_priv->display.hpd_irq_setup(dev_priv);
> > >  	spin_unlock_irq(&dev_priv->irq_lock);
> > >  
> > > -	intel_dp_mst_resume(dev);
> > > +	intel_dp_mst_resume(dev_priv);
> > >  
> > >  	intel_display_resume(dev);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index e95f1181e302..e9bc0f9f5e54 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -6490,37 +6490,44 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> > >  	return false;
> > >  }
> > >  
> > > -void intel_dp_mst_suspend(struct drm_device *dev)
> > > +void intel_dp_mst_suspend(struct drm_i915_private *dev_priv)
> > >  {
> > > -	struct drm_i915_private *dev_priv = to_i915(dev);
> > > -	int i;
> > > +	struct intel_encoder *encoder;
> > > +
> > > +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> > > +		struct intel_dp *intel_dp;
> > > +
> > > +		if (encoder->type != INTEL_OUTPUT_DDI)
> > 
> > INTEL_OUTPUT_DP not possible at this point?
> 
> No MST on pre-DDI platforms, and encoder->type for DDI platforms is
> either DDI or EDP but never DP.

Oh! indeed, thanks

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> > 
> > > +			continue;
> > >  
> > > -	/* disable MST */
> > > -	for (i = 0; i < I915_MAX_PORTS; i++) {
> > > -		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> > > +		intel_dp = enc_to_intel_dp(&encoder->base);
> > >  
> > > -		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> > > +		if (!intel_dp->can_mst)
> > >  			continue;
> > >  
> > > -		if (intel_dig_port->dp.is_mst)
> > > -			drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
> > > +		if (intel_dp->is_mst)
> > > +			drm_dp_mst_topology_mgr_suspend(&intel_dp->mst_mgr);
> > >  	}
> > >  }
> > >  
> > > -void intel_dp_mst_resume(struct drm_device *dev)
> > > +void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
> > >  {
> > > -	struct drm_i915_private *dev_priv = to_i915(dev);
> > > -	int i;
> > > +	struct intel_encoder *encoder;
> > >  
> > > -	for (i = 0; i < I915_MAX_PORTS; i++) {
> > > -		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> > > +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> > > +		struct intel_dp *intel_dp;
> > >  		int ret;
> > >  
> > > -		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> > > +		if (encoder->type != INTEL_OUTPUT_DDI)
> > > +			continue;
> > > +
> > > +		intel_dp = enc_to_intel_dp(&encoder->base);
> > > +
> > > +		if (!intel_dp->can_mst)
> > >  			continue;
> > >  
> > > -		ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
> > > +		ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr);
> > >  		if (ret)
> > > -			intel_dp_check_mst_status(&intel_dig_port->dp);
> > > +			intel_dp_check_mst_status(intel_dp);
> > >  	}
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 077189ed9e46..dc11106b2081 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1700,8 +1700,8 @@ void intel_edp_backlight_off(const struct drm_connector_state *conn_state);
> > >  void intel_edp_panel_vdd_on(struct intel_dp *intel_dp);
> > >  void intel_edp_panel_on(struct intel_dp *intel_dp);
> > >  void intel_edp_panel_off(struct intel_dp *intel_dp);
> > > -void intel_dp_mst_suspend(struct drm_device *dev);
> > > -void intel_dp_mst_resume(struct drm_device *dev);
> > > +void intel_dp_mst_suspend(struct drm_i915_private *dev_priv);
> > > +void intel_dp_mst_resume(struct drm_i915_private *dev_priv);
> > >  int intel_dp_max_link_rate(struct intel_dp *intel_dp);
> > >  int intel_dp_max_lane_count(struct intel_dp *intel_dp);
> > >  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
> > > -- 
> > > 2.16.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/8] drm/i915: Nuke dev_priv->irq_port[]

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Instead of looping over ports and hpd_pins, let's loop over
the encoders when doing hotplug processing. And instead of
depending on dev_priv->irq_port[] to tell us whether the
encoder has the ->hpd_pulse() hook or not, we can just
check for that directly. So we can just nuke irq_port[] entirely.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_ddi.c     |  1 -
 drivers/gpu/drm/i915/intel_dp.c      |  1 -
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_hotplug.c | 61 +++++++++++++++++++-----------------
 5 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 550e86dfbfe8..a808bb8aa4d8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -299,7 +299,6 @@ struct i915_hotplug {
 	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;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c74b01a52082..b15527ad0ebd 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3635,7 +3635,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 			goto err;
 
 		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
-		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 e9bc0f9f5e54..58933a3e1ae4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6471,7 +6471,6 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 	intel_encoder->port = port;
 
 	intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
-	dev_priv->hotplug.irq_port[port] = intel_dig_port;
 
 	if (port != PORT_A)
 		intel_infoframe_init(intel_dig_port);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dc11106b2081..e0c88ca53d9c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -304,6 +304,8 @@ struct intel_panel {
 	} backlight;
 };
 
+struct intel_digital_port;
+
 /*
  * This structure serves as a translation layer between the generic HDCP code
  * and the bus-specific code. What that means is that HDCP over HDMI differs
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 43aa92beff2a..f862441158e1 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -301,13 +301,18 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
 	return true;
 }
 
+static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
+{
+	return intel_encoder_is_dig_port(encoder) &&
+		enc_to_dig_port(&encoder->base)->hpd_pulse != NULL;
+}
+
 static void i915_digport_work_func(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
 		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;
+	struct intel_encoder *encoder;
 	u32 old_bits = 0;
 
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -317,27 +322,27 @@ static void i915_digport_work_func(struct work_struct *work)
 	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->hotplug.irq_port[i];
-		if (!intel_dig_port || !intel_dig_port->hpd_pulse)
+	for_each_intel_encoder(&dev_priv->drm, encoder) {
+		struct intel_digital_port *dig_port;
+		enum port port = encoder->port;
+		bool long_hpd, short_hpd;
+		enum irqreturn ret;
+
+		if (!intel_encoder_has_hpd_pulse(encoder))
 			continue;
 
-		if (long_port_mask & (1 << i))  {
-			valid = true;
-			long_hpd = true;
-		} else if (short_port_mask & (1 << i))
-			valid = true;
+		long_hpd = long_port_mask & BIT(port);
+		short_hpd = short_port_mask & BIT(port);
+
+		if (!long_hpd && !short_hpd)
+			continue;
 
-		if (valid) {
-			enum irqreturn ret;
+		dig_port = enc_to_dig_port(&encoder->base);
 
-			ret = intel_dig_port->hpd_pulse(intel_dig_port, long_hpd);
-			if (ret == IRQ_NONE) {
-				/* fall back to old school hpd */
-				old_bits |= (1 << intel_dig_port->base.hpd_pin);
-			}
+		ret = dig_port->hpd_pulse(dig_port, long_hpd);
+		if (ret == IRQ_NONE) {
+			/* fall back to old school hpd */
+			old_bits |= BIT(encoder->hpd_pin);
 		}
 	}
 
@@ -418,26 +423,24 @@ static void i915_hotplug_work_func(struct work_struct *work)
 void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 			   u32 pin_mask, u32 long_mask)
 {
-	int i;
-	enum port port;
+	struct intel_encoder *encoder;
 	bool storm_detected = false;
 	bool queue_dig = false, queue_hp = false;
-	bool is_dig_port;
 
 	if (!pin_mask)
 		return;
 
 	spin_lock(&dev_priv->irq_lock);
-	for_each_hpd_pin(i) {
+	for_each_intel_encoder(&dev_priv->drm, encoder) {
+		enum hpd_pin i = encoder->hpd_pin;
+		bool has_hpd_pulse = intel_encoder_has_hpd_pulse(encoder);
+
 		if (!(BIT(i) & pin_mask))
 			continue;
 
-		port = intel_hpd_pin_to_port(dev_priv, i);
-		is_dig_port = port != PORT_NONE &&
-			dev_priv->hotplug.irq_port[port];
-
-		if (is_dig_port) {
+		if (has_hpd_pulse) {
 			bool long_hpd = long_mask & BIT(i);
+			enum port port = encoder->port;
 
 			DRM_DEBUG_DRIVER("digital hpd port %c - %s\n", port_name(port),
 					 long_hpd ? "long" : "short");
@@ -470,7 +473,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 		if (dev_priv->hotplug.stats[i].state != HPD_ENABLED)
 			continue;
 
-		if (!is_dig_port) {
+		if (!has_hpd_pulse) {
 			dev_priv->hotplug.event_bits |= BIT(i);
 			queue_hp = true;
 		}
-- 
2.16.4

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

Re: [PATCH 4/8] drm/i915: Nuke dev_priv->irq_port[]

[Rodrigo Vivi]

On Thu, Jul 05, 2018 at 07:43:53PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Instead of looping over ports and hpd_pins, let's loop over
> the encoders when doing hotplug processing. And instead of
> depending on dev_priv->irq_port[] to tell us whether the
> encoder has the ->hpd_pulse() hook or not, we can just
> check for that directly. So we can just nuke irq_port[] entirely.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 -
>  drivers/gpu/drm/i915/intel_ddi.c     |  1 -
>  drivers/gpu/drm/i915/intel_dp.c      |  1 -
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_hotplug.c | 61 +++++++++++++++++++-----------------
>  5 files changed, 34 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 550e86dfbfe8..a808bb8aa4d8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -299,7 +299,6 @@ struct i915_hotplug {
>  	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;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index c74b01a52082..b15527ad0ebd 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3635,7 +3635,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  			goto err;
>  
>  		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> -		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 e9bc0f9f5e54..58933a3e1ae4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6471,7 +6471,6 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>  	intel_encoder->port = port;
>  
>  	intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> -	dev_priv->hotplug.irq_port[port] = intel_dig_port;
>  
>  	if (port != PORT_A)
>  		intel_infoframe_init(intel_dig_port);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index dc11106b2081..e0c88ca53d9c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -304,6 +304,8 @@ struct intel_panel {
>  	} backlight;
>  };
>  
> +struct intel_digital_port;
> +
>  /*
>   * This structure serves as a translation layer between the generic HDCP code
>   * and the bus-specific code. What that means is that HDCP over HDMI differs
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 43aa92beff2a..f862441158e1 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -301,13 +301,18 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
> +{
> +	return intel_encoder_is_dig_port(encoder) &&
> +		enc_to_dig_port(&encoder->base)->hpd_pulse != NULL;
> +}
> +
>  static void i915_digport_work_func(struct work_struct *work)
>  {
>  	struct drm_i915_private *dev_priv =
>  		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;
> +	struct intel_encoder *encoder;
>  	u32 old_bits = 0;
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
> @@ -317,27 +322,27 @@ static void i915_digport_work_func(struct work_struct *work)
>  	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->hotplug.irq_port[i];
> -		if (!intel_dig_port || !intel_dig_port->hpd_pulse)
> +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> +		struct intel_digital_port *dig_port;
> +		enum port port = encoder->port;
> +		bool long_hpd, short_hpd;
> +		enum irqreturn ret;
> +
> +		if (!intel_encoder_has_hpd_pulse(encoder))
>  			continue;
>  
> -		if (long_port_mask & (1 << i))  {
> -			valid = true;
> -			long_hpd = true;
> -		} else if (short_port_mask & (1 << i))
> -			valid = true;
> +		long_hpd = long_port_mask & BIT(port);
> +		short_hpd = short_port_mask & BIT(port);
> +
> +		if (!long_hpd && !short_hpd)
> +			continue;
>  
> -		if (valid) {
> -			enum irqreturn ret;
> +		dig_port = enc_to_dig_port(&encoder->base);
>  
> -			ret = intel_dig_port->hpd_pulse(intel_dig_port, long_hpd);
> -			if (ret == IRQ_NONE) {
> -				/* fall back to old school hpd */
> -				old_bits |= (1 << intel_dig_port->base.hpd_pin);
> -			}
> +		ret = dig_port->hpd_pulse(dig_port, long_hpd);
> +		if (ret == IRQ_NONE) {
> +			/* fall back to old school hpd */
> +			old_bits |= BIT(encoder->hpd_pin);
>  		}
>  	}
>  
> @@ -418,26 +423,24 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			   u32 pin_mask, u32 long_mask)
>  {
> -	int i;
> -	enum port port;
> +	struct intel_encoder *encoder;
>  	bool storm_detected = false;
>  	bool queue_dig = false, queue_hp = false;
> -	bool is_dig_port;
>  
>  	if (!pin_mask)
>  		return;
>  
>  	spin_lock(&dev_priv->irq_lock);
> -	for_each_hpd_pin(i) {
> +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> +		enum hpd_pin i = encoder->hpd_pin;
> +		bool has_hpd_pulse = intel_encoder_has_hpd_pulse(encoder);
> +
>  		if (!(BIT(i) & pin_mask))
>  			continue;
>  
> -		port = intel_hpd_pin_to_port(dev_priv, i);
> -		is_dig_port = port != PORT_NONE &&
> -			dev_priv->hotplug.irq_port[port];
> -
> -		if (is_dig_port) {
> +		if (has_hpd_pulse) {
>  			bool long_hpd = long_mask & BIT(i);
> +			enum port port = encoder->port;
>  
>  			DRM_DEBUG_DRIVER("digital hpd port %c - %s\n", port_name(port),
>  					 long_hpd ? "long" : "short");
> @@ -470,7 +473,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  		if (dev_priv->hotplug.stats[i].state != HPD_ENABLED)
>  			continue;
>  
> -		if (!is_dig_port) {
> +		if (!has_hpd_pulse) {
>  			dev_priv->hotplug.event_bits |= BIT(i);
>  			queue_hp = true;
>  		}
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/8] drm/i915: s/int i/enum hpd_pin pin/

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the enum hpd_pin type when talking about HPD pins, and rename the
variable from a very nondescript 'i' to 'pin', a name we already
use in other parts of the code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c      | 12 ++++++------
 drivers/gpu/drm/i915/intel_hotplug.c | 28 ++++++++++++++--------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2fcc00b06915..1c3ff07d9f2d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1712,20 +1712,20 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
 			       bool long_pulse_detect(enum port port, u32 val))
 {
 	enum port port;
-	int i;
+	enum hpd_pin pin;
 
-	for_each_hpd_pin(i) {
-		if ((hpd[i] & hotplug_trigger) == 0)
+	for_each_hpd_pin(pin) {
+		if ((hpd[pin] & hotplug_trigger) == 0)
 			continue;
 
-		*pin_mask |= BIT(i);
+		*pin_mask |= BIT(pin);
 
-		port = intel_hpd_pin_to_port(dev_priv, i);
+		port = intel_hpd_pin_to_port(dev_priv, pin);
 		if (port == PORT_NONE)
 			continue;
 
 		if (long_pulse_detect(port, dig_hotplug_reg))
-			*long_mask |= BIT(i);
+			*long_mask |= BIT(pin);
 	}
 
 	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, dig 0x%08x, pins 0x%08x\n",
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index f862441158e1..d9d61d11dd2b 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -241,25 +241,25 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
 		container_of(work, typeof(*dev_priv),
 			     hotplug.reenable_work.work);
 	struct drm_device *dev = &dev_priv->drm;
-	int i;
+	enum hpd_pin pin;
 
 	intel_runtime_pm_get(dev_priv);
 
 	spin_lock_irq(&dev_priv->irq_lock);
-	for_each_hpd_pin(i) {
+	for_each_hpd_pin(pin) {
 		struct drm_connector *connector;
 		struct drm_connector_list_iter conn_iter;
 
-		if (dev_priv->hotplug.stats[i].state != HPD_DISABLED)
+		if (dev_priv->hotplug.stats[pin].state != HPD_DISABLED)
 			continue;
 
-		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
+		dev_priv->hotplug.stats[pin].state = HPD_ENABLED;
 
 		drm_connector_list_iter_begin(dev, &conn_iter);
 		drm_for_each_connector_iter(connector, &conn_iter) {
 			struct intel_connector *intel_connector = to_intel_connector(connector);
 
-			if (intel_connector->encoder->hpd_pin == i) {
+			if (intel_connector->encoder->hpd_pin == pin) {
 				if (connector->polled != intel_connector->polled)
 					DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
 							 connector->name);
@@ -432,14 +432,14 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 
 	spin_lock(&dev_priv->irq_lock);
 	for_each_intel_encoder(&dev_priv->drm, encoder) {
-		enum hpd_pin i = encoder->hpd_pin;
+		enum hpd_pin pin = encoder->hpd_pin;
 		bool has_hpd_pulse = intel_encoder_has_hpd_pulse(encoder);
 
-		if (!(BIT(i) & pin_mask))
+		if (!(BIT(pin) & pin_mask))
 			continue;
 
 		if (has_hpd_pulse) {
-			bool long_hpd = long_mask & BIT(i);
+			bool long_hpd = long_mask & BIT(pin);
 			enum port port = encoder->port;
 
 			DRM_DEBUG_DRIVER("digital hpd port %c - %s\n", port_name(port),
@@ -458,7 +458,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 			}
 		}
 
-		if (dev_priv->hotplug.stats[i].state == HPD_DISABLED) {
+		if (dev_priv->hotplug.stats[pin].state == HPD_DISABLED) {
 			/*
 			 * On GMCH platforms the interrupt mask bits only
 			 * prevent irq generation, not the setting of the
@@ -466,20 +466,20 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 			 * interrupts on saner platforms.
 			 */
 			WARN_ONCE(!HAS_GMCH_DISPLAY(dev_priv),
-				  "Received HPD interrupt on pin %d although disabled\n", i);
+				  "Received HPD interrupt on pin %d although disabled\n", pin);
 			continue;
 		}
 
-		if (dev_priv->hotplug.stats[i].state != HPD_ENABLED)
+		if (dev_priv->hotplug.stats[pin].state != HPD_ENABLED)
 			continue;
 
 		if (!has_hpd_pulse) {
-			dev_priv->hotplug.event_bits |= BIT(i);
+			dev_priv->hotplug.event_bits |= BIT(pin);
 			queue_hp = true;
 		}
 
-		if (intel_hpd_irq_storm_detect(dev_priv, i)) {
-			dev_priv->hotplug.event_bits &= ~BIT(i);
+		if (intel_hpd_irq_storm_detect(dev_priv, pin)) {
+			dev_priv->hotplug.event_bits &= ~BIT(pin);
 			storm_detected = true;
 		}
 	}
-- 
2.16.4

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

Re: [PATCH 5/8] drm/i915: s/int i/enum hpd_pin pin/

[Rodrigo Vivi]

On Thu, Jul 05, 2018 at 07:43:54PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use the enum hpd_pin type when talking about HPD pins, and rename the
> variable from a very nondescript 'i' to 'pin', a name we already
> use in other parts of the code.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c      | 12 ++++++------
>  drivers/gpu/drm/i915/intel_hotplug.c | 28 ++++++++++++++--------------
>  2 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2fcc00b06915..1c3ff07d9f2d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1712,20 +1712,20 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
>  			       bool long_pulse_detect(enum port port, u32 val))
>  {
>  	enum port port;
> -	int i;
> +	enum hpd_pin pin;
>  
> -	for_each_hpd_pin(i) {
> -		if ((hpd[i] & hotplug_trigger) == 0)
> +	for_each_hpd_pin(pin) {
> +		if ((hpd[pin] & hotplug_trigger) == 0)
>  			continue;
>  
> -		*pin_mask |= BIT(i);
> +		*pin_mask |= BIT(pin);
>  
> -		port = intel_hpd_pin_to_port(dev_priv, i);
> +		port = intel_hpd_pin_to_port(dev_priv, pin);
>  		if (port == PORT_NONE)
>  			continue;
>  
>  		if (long_pulse_detect(port, dig_hotplug_reg))
> -			*long_mask |= BIT(i);
> +			*long_mask |= BIT(pin);
>  	}
>  
>  	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, dig 0x%08x, pins 0x%08x\n",
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index f862441158e1..d9d61d11dd2b 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -241,25 +241,25 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  		container_of(work, typeof(*dev_priv),
>  			     hotplug.reenable_work.work);
>  	struct drm_device *dev = &dev_priv->drm;
> -	int i;
> +	enum hpd_pin pin;
>  
>  	intel_runtime_pm_get(dev_priv);
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
> -	for_each_hpd_pin(i) {
> +	for_each_hpd_pin(pin) {
>  		struct drm_connector *connector;
>  		struct drm_connector_list_iter conn_iter;
>  
> -		if (dev_priv->hotplug.stats[i].state != HPD_DISABLED)
> +		if (dev_priv->hotplug.stats[pin].state != HPD_DISABLED)
>  			continue;
>  
> -		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
> +		dev_priv->hotplug.stats[pin].state = HPD_ENABLED;
>  
>  		drm_connector_list_iter_begin(dev, &conn_iter);
>  		drm_for_each_connector_iter(connector, &conn_iter) {
>  			struct intel_connector *intel_connector = to_intel_connector(connector);
>  
> -			if (intel_connector->encoder->hpd_pin == i) {
> +			if (intel_connector->encoder->hpd_pin == pin) {
>  				if (connector->polled != intel_connector->polled)
>  					DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
>  							 connector->name);
> @@ -432,14 +432,14 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  
>  	spin_lock(&dev_priv->irq_lock);
>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
> -		enum hpd_pin i = encoder->hpd_pin;
> +		enum hpd_pin pin = encoder->hpd_pin;
>  		bool has_hpd_pulse = intel_encoder_has_hpd_pulse(encoder);
>  
> -		if (!(BIT(i) & pin_mask))
> +		if (!(BIT(pin) & pin_mask))
>  			continue;
>  
>  		if (has_hpd_pulse) {
> -			bool long_hpd = long_mask & BIT(i);
> +			bool long_hpd = long_mask & BIT(pin);
>  			enum port port = encoder->port;
>  
>  			DRM_DEBUG_DRIVER("digital hpd port %c - %s\n", port_name(port),
> @@ -458,7 +458,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			}
>  		}
>  
> -		if (dev_priv->hotplug.stats[i].state == HPD_DISABLED) {
> +		if (dev_priv->hotplug.stats[pin].state == HPD_DISABLED) {
>  			/*
>  			 * On GMCH platforms the interrupt mask bits only
>  			 * prevent irq generation, not the setting of the
> @@ -466,20 +466,20 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			 * interrupts on saner platforms.
>  			 */
>  			WARN_ONCE(!HAS_GMCH_DISPLAY(dev_priv),
> -				  "Received HPD interrupt on pin %d although disabled\n", i);
> +				  "Received HPD interrupt on pin %d although disabled\n", pin);
>  			continue;
>  		}
>  
> -		if (dev_priv->hotplug.stats[i].state != HPD_ENABLED)
> +		if (dev_priv->hotplug.stats[pin].state != HPD_ENABLED)
>  			continue;
>  
>  		if (!has_hpd_pulse) {
> -			dev_priv->hotplug.event_bits |= BIT(i);
> +			dev_priv->hotplug.event_bits |= BIT(pin);
>  			queue_hp = true;
>  		}
>  
> -		if (intel_hpd_irq_storm_detect(dev_priv, i)) {
> -			dev_priv->hotplug.event_bits &= ~BIT(i);
> +		if (intel_hpd_irq_storm_detect(dev_priv, pin)) {
> +			dev_priv->hotplug.event_bits &= ~BIT(pin);
>  			storm_detected = true;
>  		}
>  	}
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/8] drm/i915: Pass hpd_pin to long_pulse_detect()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're doing a pointless translation from hpd_pin to port simply for
passing the thing to long_pulse_detect(). Let's pass the hpd_pin
directly instead.

This removes the assumption that the hpd_pin and port always
match. The only other place where we make that assumption anymore
is intel_hpd_pin_default() and that's fine as it's what determines
the relationship between the two. If we ever get hardware where
the hpd pins are wired in more interesting ways it should be
trivial to handle from now on.

This should also fix the IS_CNL_WITH_PORT_F() case as that mapped
pin E back to port F and passed that to
spt_port_hotplug2_long_detect() which would always return false
for port F. Now that we pass in pin E directly it'll actually
do the right thing.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Fixes: cf53902f48c3 ("drm/i915/cnl: Add HPD support for Port F.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 -
 drivers/gpu/drm/i915/i915_irq.c      | 95 +++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_hotplug.c | 31 ------------
 3 files changed, 45 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a808bb8aa4d8..5afadc897e76 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2746,8 +2746,6 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 void intel_hpd_init(struct drm_i915_private *dev_priv);
 void intel_hpd_init_work(struct drm_i915_private *dev_priv);
 void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
-enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
-				enum hpd_pin pin);
 enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
 				   enum port port);
 bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1c3ff07d9f2d..bb7c754979f8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1576,122 +1576,122 @@ static void gen8_gt_irq_handler(struct drm_i915_private *i915,
 	}
 }
 
-static bool gen11_port_hotplug_long_detect(enum port port, u32 val)
+static bool gen11_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_C:
+	switch (pin) {
+	case HPD_PORT_C:
 		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC1);
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC2);
-	case PORT_E:
+	case HPD_PORT_E:
 		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC3);
-	case PORT_F:
+	case HPD_PORT_F:
 		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC4);
 	default:
 		return false;
 	}
 }
 
-static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
+static bool bxt_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_A:
+	switch (pin) {
+	case HPD_PORT_A:
 		return val & PORTA_HOTPLUG_LONG_DETECT;
-	case PORT_B:
+	case HPD_PORT_B:
 		return val & PORTB_HOTPLUG_LONG_DETECT;
-	case PORT_C:
+	case HPD_PORT_C:
 		return val & PORTC_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 val)
+static bool icp_ddi_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_A:
+	switch (pin) {
+	case HPD_PORT_A:
 		return val & ICP_DDIA_HPD_LONG_DETECT;
-	case PORT_B:
+	case HPD_PORT_B:
 		return val & ICP_DDIB_HPD_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val)
+static bool icp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_C:
+	switch (pin) {
+	case HPD_PORT_C:
 		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
-	case PORT_E:
+	case HPD_PORT_E:
 		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
-	case PORT_F:
+	case HPD_PORT_F:
 		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
 	default:
 		return false;
 	}
 }
 
-static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
+static bool spt_port_hotplug2_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_E:
+	switch (pin) {
+	case HPD_PORT_E:
 		return val & PORTE_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool spt_port_hotplug_long_detect(enum port port, u32 val)
+static bool spt_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_A:
+	switch (pin) {
+	case HPD_PORT_A:
 		return val & PORTA_HOTPLUG_LONG_DETECT;
-	case PORT_B:
+	case HPD_PORT_B:
 		return val & PORTB_HOTPLUG_LONG_DETECT;
-	case PORT_C:
+	case HPD_PORT_C:
 		return val & PORTC_HOTPLUG_LONG_DETECT;
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & PORTD_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool ilk_port_hotplug_long_detect(enum port port, u32 val)
+static bool ilk_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_A:
+	switch (pin) {
+	case HPD_PORT_A:
 		return val & DIGITAL_PORTA_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool pch_port_hotplug_long_detect(enum port port, u32 val)
+static bool pch_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_B:
+	switch (pin) {
+	case HPD_PORT_B:
 		return val & PORTB_HOTPLUG_LONG_DETECT;
-	case PORT_C:
+	case HPD_PORT_C:
 		return val & PORTC_HOTPLUG_LONG_DETECT;
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & PORTD_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
+static bool i9xx_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_B:
+	switch (pin) {
+	case HPD_PORT_B:
 		return val & PORTB_HOTPLUG_INT_LONG_PULSE;
-	case PORT_C:
+	case HPD_PORT_C:
 		return val & PORTC_HOTPLUG_INT_LONG_PULSE;
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & PORTD_HOTPLUG_INT_LONG_PULSE;
 	default:
 		return false;
@@ -1709,9 +1709,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
 			       u32 *pin_mask, u32 *long_mask,
 			       u32 hotplug_trigger, u32 dig_hotplug_reg,
 			       const u32 hpd[HPD_NUM_PINS],
-			       bool long_pulse_detect(enum port port, u32 val))
+			       bool long_pulse_detect(enum hpd_pin pin, u32 val))
 {
-	enum port port;
 	enum hpd_pin pin;
 
 	for_each_hpd_pin(pin) {
@@ -1720,11 +1719,7 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
 
 		*pin_mask |= BIT(pin);
 
-		port = intel_hpd_pin_to_port(dev_priv, pin);
-		if (port == PORT_NONE)
-			continue;
-
-		if (long_pulse_detect(port, dig_hotplug_reg))
+		if (long_pulse_detect(pin, dig_hotplug_reg))
 			*long_mask |= BIT(pin);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index d9d61d11dd2b..648a13c6043c 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -76,37 +76,6 @@
  * it will use i915_hotplug_work_func where this logic is handled.
  */
 
-/**
- * intel_hpd_port - return port hard associated with certain pin.
- * @dev_priv: private driver data pointer
- * @pin: the hpd pin to get associated port
- *
- * Return port that is associatade with @pin and PORT_NONE if no port is
- * hard associated with that @pin.
- */
-enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
-				enum hpd_pin pin)
-{
-	switch (pin) {
-	case HPD_PORT_A:
-		return PORT_A;
-	case HPD_PORT_B:
-		return PORT_B;
-	case HPD_PORT_C:
-		return PORT_C;
-	case HPD_PORT_D:
-		return PORT_D;
-	case HPD_PORT_E:
-		if (IS_CNL_WITH_PORT_F(dev_priv))
-			return PORT_F;
-		return PORT_E;
-	case HPD_PORT_F:
-		return PORT_F;
-	default:
-		return PORT_NONE; /* no port for this pin */
-	}
-}
-
 /**
  * intel_hpd_pin_default - return default pin associated with certain port.
  * @dev_priv: private driver data pointer
-- 
2.16.4

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

Re: [PATCH 6/8] drm/i915: Pass hpd_pin to long_pulse_detect()

[Rodrigo Vivi]

On Thu, Jul 05, 2018 at 07:43:55PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're doing a pointless translation from hpd_pin to port simply for
> passing the thing to long_pulse_detect(). Let's pass the hpd_pin
> directly instead.
> 
> This removes the assumption that the hpd_pin and port always
> match. The only other place where we make that assumption anymore
> is intel_hpd_pin_default() and that's fine as it's what determines
> the relationship between the two. If we ever get hardware where
> the hpd pins are wired in more interesting ways it should be
> trivial to handle from now on.
> 
> This should also fix the IS_CNL_WITH_PORT_F() case as that mapped
> pin E back to port F and passed that to
> spt_port_hotplug2_long_detect() which would always return false
> for port F. Now that we pass in pin E directly it'll actually
> do the right thing.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Fixes: cf53902f48c3 ("drm/i915/cnl: Add HPD support for Port F.")

Thanks!

I could swear that I had a version similar to this somewhere,
but anyways this is much better...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 -
>  drivers/gpu/drm/i915/i915_irq.c      | 95 +++++++++++++++++-------------------
>  drivers/gpu/drm/i915/intel_hotplug.c | 31 ------------
>  3 files changed, 45 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a808bb8aa4d8..5afadc897e76 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2746,8 +2746,6 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  void intel_hpd_init(struct drm_i915_private *dev_priv);
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv);
>  void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> -enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
> -				enum hpd_pin pin);
>  enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
>  				   enum port port);
>  bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1c3ff07d9f2d..bb7c754979f8 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1576,122 +1576,122 @@ static void gen8_gt_irq_handler(struct drm_i915_private *i915,
>  	}
>  }
>  
> -static bool gen11_port_hotplug_long_detect(enum port port, u32 val)
> +static bool gen11_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_C:
> +	switch (pin) {
> +	case HPD_PORT_C:
>  		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC1);
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC2);
> -	case PORT_E:
> +	case HPD_PORT_E:
>  		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC3);
> -	case PORT_F:
> +	case HPD_PORT_F:
>  		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC4);
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> +static bool bxt_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_A:
> +	switch (pin) {
> +	case HPD_PORT_A:
>  		return val & PORTA_HOTPLUG_LONG_DETECT;
> -	case PORT_B:
> +	case HPD_PORT_B:
>  		return val & PORTB_HOTPLUG_LONG_DETECT;
> -	case PORT_C:
> +	case HPD_PORT_C:
>  		return val & PORTC_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 val)
> +static bool icp_ddi_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_A:
> +	switch (pin) {
> +	case HPD_PORT_A:
>  		return val & ICP_DDIA_HPD_LONG_DETECT;
> -	case PORT_B:
> +	case HPD_PORT_B:
>  		return val & ICP_DDIB_HPD_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val)
> +static bool icp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_C:
> +	switch (pin) {
> +	case HPD_PORT_C:
>  		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
> -	case PORT_E:
> +	case HPD_PORT_E:
>  		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
> -	case PORT_F:
> +	case HPD_PORT_F:
>  		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
> +static bool spt_port_hotplug2_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_E:
> +	switch (pin) {
> +	case HPD_PORT_E:
>  		return val & PORTE_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool spt_port_hotplug_long_detect(enum port port, u32 val)
> +static bool spt_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_A:
> +	switch (pin) {
> +	case HPD_PORT_A:
>  		return val & PORTA_HOTPLUG_LONG_DETECT;
> -	case PORT_B:
> +	case HPD_PORT_B:
>  		return val & PORTB_HOTPLUG_LONG_DETECT;
> -	case PORT_C:
> +	case HPD_PORT_C:
>  		return val & PORTC_HOTPLUG_LONG_DETECT;
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & PORTD_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool ilk_port_hotplug_long_detect(enum port port, u32 val)
> +static bool ilk_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_A:
> +	switch (pin) {
> +	case HPD_PORT_A:
>  		return val & DIGITAL_PORTA_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> +static bool pch_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_B:
> +	switch (pin) {
> +	case HPD_PORT_B:
>  		return val & PORTB_HOTPLUG_LONG_DETECT;
> -	case PORT_C:
> +	case HPD_PORT_C:
>  		return val & PORTC_HOTPLUG_LONG_DETECT;
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & PORTD_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
> +static bool i9xx_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_B:
> +	switch (pin) {
> +	case HPD_PORT_B:
>  		return val & PORTB_HOTPLUG_INT_LONG_PULSE;
> -	case PORT_C:
> +	case HPD_PORT_C:
>  		return val & PORTC_HOTPLUG_INT_LONG_PULSE;
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & PORTD_HOTPLUG_INT_LONG_PULSE;
>  	default:
>  		return false;
> @@ -1709,9 +1709,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
>  			       u32 *pin_mask, u32 *long_mask,
>  			       u32 hotplug_trigger, u32 dig_hotplug_reg,
>  			       const u32 hpd[HPD_NUM_PINS],
> -			       bool long_pulse_detect(enum port port, u32 val))
> +			       bool long_pulse_detect(enum hpd_pin pin, u32 val))
>  {
> -	enum port port;
>  	enum hpd_pin pin;
>  
>  	for_each_hpd_pin(pin) {
> @@ -1720,11 +1719,7 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
>  
>  		*pin_mask |= BIT(pin);
>  
> -		port = intel_hpd_pin_to_port(dev_priv, pin);
> -		if (port == PORT_NONE)
> -			continue;
> -
> -		if (long_pulse_detect(port, dig_hotplug_reg))
> +		if (long_pulse_detect(pin, dig_hotplug_reg))
>  			*long_mask |= BIT(pin);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index d9d61d11dd2b..648a13c6043c 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -76,37 +76,6 @@
>   * it will use i915_hotplug_work_func where this logic is handled.
>   */
>  
> -/**
> - * intel_hpd_port - return port hard associated with certain pin.
> - * @dev_priv: private driver data pointer
> - * @pin: the hpd pin to get associated port
> - *
> - * Return port that is associatade with @pin and PORT_NONE if no port is
> - * hard associated with that @pin.
> - */
> -enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
> -				enum hpd_pin pin)
> -{
> -	switch (pin) {
> -	case HPD_PORT_A:
> -		return PORT_A;
> -	case HPD_PORT_B:
> -		return PORT_B;
> -	case HPD_PORT_C:
> -		return PORT_C;
> -	case HPD_PORT_D:
> -		return PORT_D;
> -	case HPD_PORT_E:
> -		if (IS_CNL_WITH_PORT_F(dev_priv))
> -			return PORT_F;
> -		return PORT_E;
> -	case HPD_PORT_F:
> -		return PORT_F;
> -	default:
> -		return PORT_NONE; /* no port for this pin */
> -	}
> -}
> -
>  /**
>   * intel_hpd_pin_default - return default pin associated with certain port.
>   * @dev_priv: private driver data pointer
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/8] drm/i915: Assert that our hpd pin bitmasks don't overflow

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make sure our hpd pin count doesn't exceed the bitmasks we use
for tracking pending hotplugs. Not ever close to the limit yet,
but no harm in making sure either.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bb7c754979f8..c107e0837026 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1713,6 +1713,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
 {
 	enum hpd_pin pin;
 
+	BUILD_BUG_ON(HPD_NUM_PINS > 32);
+
 	for_each_hpd_pin(pin) {
 		if ((hpd[pin] & hotplug_trigger) == 0)
 			continue;
-- 
2.16.4

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

Re: [PATCH 7/8] drm/i915: Assert that our hpd pin bitmasks don't overflow

[Chris Wilson]

Quoting Ville Syrjala (2018-07-05 17:43:56)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make sure our hpd pin count doesn't exceed the bitmasks we use
> for tracking pending hotplugs. Not ever close to the limit yet,
> but no harm in making sure either.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bb7c754979f8..c107e0837026 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1713,6 +1713,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
>  {
>         enum hpd_pin pin;
>  
> +       BUILD_BUG_ON(HPD_NUM_PINS > 32);

sizeof(*pin_mask) * CHAR_BIT ?

Just looking for some explanation as where the limit comes from. Now
obviously u32, but why was u32 chosen?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/8] drm/i915: Assert that our hpd pin bitmasks don't overflow

[Ville Syrjälä]

On Thu, Jul 05, 2018 at 05:52:16PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-07-05 17:43:56)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Make sure our hpd pin count doesn't exceed the bitmasks we use
> > for tracking pending hotplugs. Not ever close to the limit yet,
> > but no harm in making sure either.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index bb7c754979f8..c107e0837026 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1713,6 +1713,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
> >  {
> >         enum hpd_pin pin;
> >  
> > +       BUILD_BUG_ON(HPD_NUM_PINS > 32);
> 
> sizeof(*pin_mask) * CHAR_BIT ?

For a slightly more pleasing appearance I suppose we might want
to steal BITS_PER_TYPE() from include/linux/net_dim.h. A quick 
grep didn't seem to reveal anything similar in a more general
location.

> 
> Just looking for some explanation as where the limit comes from. Now
> obviously u32, but why was u32 chosen?

Had the right kind of smell perhaps?

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/8] drm/i915: Assert that our hpd pin bitmasks don't overflow

[Chris Wilson]

Quoting Ville Syrjälä (2018-07-05 18:51:16)
> On Thu, Jul 05, 2018 at 05:52:16PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-07-05 17:43:56)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Make sure our hpd pin count doesn't exceed the bitmasks we use
> > > for tracking pending hotplugs. Not ever close to the limit yet,
> > > but no harm in making sure either.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_irq.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index bb7c754979f8..c107e0837026 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -1713,6 +1713,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
> > >  {
> > >         enum hpd_pin pin;
> > >  
> > > +       BUILD_BUG_ON(HPD_NUM_PINS > 32);
> > 
> > sizeof(*pin_mask) * CHAR_BIT ?
> 
> For a slightly more pleasing appearance I suppose we might want
> to steal BITS_PER_TYPE() from include/linux/net_dim.h. A quick 
> grep didn't seem to reveal anything similar in a more general
> location.

Yup, there's quite a few places that would be made more pleasant by
BITS_PER_TYPE().

> > Just looking for some explanation as where the limit comes from. Now
> > obviously u32, but why was u32 chosen?
> 
> Had the right kind of smell perhaps?

Ok, just curious if there was an intrinsic limit that could be captured
alongside the assert.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 8/8] drm/i915: Print the long_mask alongside the pin_mask

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're printing out which pins got a hotplug, so why not also print
out which pins detected the long pulse as opposed to a short pulse.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c107e0837026..a510672b8071 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1725,8 +1725,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
 			*long_mask |= BIT(pin);
 	}
 
-	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, dig 0x%08x, pins 0x%08x\n",
-			 hotplug_trigger, dig_hotplug_reg, *pin_mask);
+	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, dig 0x%08x, pins 0x%08x, long 0x%08x\n",
+			 hotplug_trigger, dig_hotplug_reg, *pin_mask, *long_mask);
 
 }
 
-- 
2.16.4

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

Re: [PATCH 8/8] drm/i915: Print the long_mask alongside the pin_mask

[Chris Wilson]

Quoting Ville Syrjala (2018-07-05 17:43:57)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're printing out which pins got a hotplug, so why not also print
> out which pins detected the long pulse as opposed to a short pulse.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Hotplug cleanups and whanot

[Patchwork]

== Series Details ==

Series: drm/i915: Hotplug cleanups and whanot
URL   : https://patchwork.freedesktop.org/series/46022/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1684bcc70562 drm/i915: Introduce for_each_intel_dp()
-:21: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#21: FILE: drivers/gpu/drm/i915/intel_display.h:292:
+#define for_each_intel_dp(dev, intel_encoder)			\
+	for_each_intel_encoder(dev, intel_encoder)		\
+		for_each_if(intel_encoder_is_dp(intel_encoder))

-:21: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'intel_encoder' - possible side-effects?
#21: FILE: drivers/gpu/drm/i915/intel_display.h:292:
+#define for_each_intel_dp(dev, intel_encoder)			\
+	for_each_intel_encoder(dev, intel_encoder)		\
+		for_each_if(intel_encoder_is_dp(intel_encoder))

total: 1 errors, 0 warnings, 1 checks, 86 lines checked
256e2e4aa625 drm/i915: Introduce intel_encoder_is_dig_port()
da7ac2ea4459 drm/i915: Rewrite mst suspend/resume in terms of encoders
bcef9f3f9672 drm/i915: Nuke dev_priv->irq_port[]
-:77: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "hpd_pulse"
#77: FILE: drivers/gpu/drm/i915/intel_hotplug.c:307:
+		enc_to_dig_port(&encoder->base)->hpd_pulse != NULL;

total: 0 errors, 0 warnings, 1 checks, 134 lines checked
74b17618be88 drm/i915: s/int i/enum hpd_pin pin/
7b7a0da658e7 drm/i915: Pass hpd_pin to long_pulse_detect()
d311117aa530 drm/i915: Assert that our hpd pin bitmasks don't overflow
f5d991118578 drm/i915: Print the long_mask alongside the pin_mask

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

✗ Fi.CI.SPARSE: warning for drm/i915: Hotplug cleanups and whanot

[Patchwork]

== Series Details ==

Series: drm/i915: Hotplug cleanups and whanot
URL   : https://patchwork.freedesktop.org/series/46022/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Introduce for_each_intel_dp()
Okay!

Commit: drm/i915: Introduce intel_encoder_is_dig_port()
Okay!

Commit: drm/i915: Rewrite mst suspend/resume in terms of encoders
Okay!

Commit: drm/i915: Nuke dev_priv->irq_port[]
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3663:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3662:16: warning: expression using sizeof(void)

Commit: drm/i915: s/int i/enum hpd_pin pin/
Okay!

Commit: drm/i915: Pass hpd_pin to long_pulse_detect()
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3662:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3660:16: warning: expression using sizeof(void)

Commit: drm/i915: Assert that our hpd pin bitmasks don't overflow
Okay!

Commit: drm/i915: Print the long_mask alongside the pin_mask
Okay!

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

✓ Fi.CI.BAT: success for drm/i915: Hotplug cleanups and whanot

[Patchwork]

== Series Details ==

Series: drm/i915: Hotplug cleanups and whanot
URL   : https://patchwork.freedesktop.org/series/46022/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4438 -> Patchwork_9552 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46022/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9552:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-cfl-8109u}:     PASS -> INCOMPLETE

    
    ==== Warnings ====

    igt@gem_exec_suspend@basic-s4-devices:
      {fi-kbl-8809g}:     INCOMPLETE -> DMESG-WARN

    
== Known issues ==

  Here are the changes found in Patchwork_9552 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-FAIL (fdo#106103, fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (47 -> 42) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4438 -> Patchwork_9552

  CI_DRM_4438: b689733af687b4b8072fb62a6bfe267c4e888f5f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4539: 8b3cc74c6911e9b2835fe6e160f84bae463a70ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9552: f5d991118578e2be7f21e86eacbacfb536838df2 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f5d991118578 drm/i915: Print the long_mask alongside the pin_mask
d311117aa530 drm/i915: Assert that our hpd pin bitmasks don't overflow
7b7a0da658e7 drm/i915: Pass hpd_pin to long_pulse_detect()
74b17618be88 drm/i915: s/int i/enum hpd_pin pin/
bcef9f3f9672 drm/i915: Nuke dev_priv->irq_port[]
da7ac2ea4459 drm/i915: Rewrite mst suspend/resume in terms of encoders
256e2e4aa625 drm/i915: Introduce intel_encoder_is_dig_port()
1684bcc70562 drm/i915: Introduce for_each_intel_dp()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9552/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Hotplug cleanups and whanot

[Patchwork]

== Series Details ==

Series: drm/i915: Hotplug cleanups and whanot
URL   : https://patchwork.freedesktop.org/series/46022/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4438_full -> Patchwork_9552_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9552_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9552_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9552_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-render:
      shard-kbl:          PASS -> SKIP

    igt@gem_mocs_settings@mocs-rc6-bsd1:
      shard-kbl:          SKIP -> PASS +1

    
== Known issues ==

  Here are the changes found in Patchwork_9552_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#103928)

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#103822)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-kbl:          INCOMPLETE (fdo#107127, fdo#103665) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          FAIL -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#107127 https://bugs.freedesktop.org/show_bug.cgi?id=107127
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4438 -> Patchwork_9552

  CI_DRM_4438: b689733af687b4b8072fb62a6bfe267c4e888f5f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4539: 8b3cc74c6911e9b2835fe6e160f84bae463a70ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9552: f5d991118578e2be7f21e86eacbacfb536838df2 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9552/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx