All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
@ 2017-07-17 18:14 ` Matthias Kaehlcke
  0 siblings, 0 replies; 8+ messages in thread
From: Matthias Kaehlcke @ 2017-07-17 18:14 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Daniel Vetter
  Cc: intel-gfx, Linux Kernel Mailing List, dri-devel,
	Stéphane Marchesin, Grant Grundler, Matthias Kaehlcke

The current code uses in some instances enum transcoder for PCH
transcoders and enum pipe in others. This is error prone and clang
raises warnings like this:

drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
  from enumeration type 'enum pipe' to different enumeration type
  'enum transcoder' [-Wenum-conversion]
    intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);

Consistently use the type enum pipe for PCH transcoders.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- rebased on drm-intel/drm-intel-next-queued

 drivers/gpu/drm/i915/i915_irq.c            | 10 +++++-----
 drivers/gpu/drm/i915/intel_display.c       | 24 ++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h           |  6 +++---
 drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
 4 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1d33cea01a1b..0b6f310101ee 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2086,10 +2086,10 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 		DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n");
 
 	if (pch_iir & SDE_TRANSA_FIFO_UNDER)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
 
 	if (pch_iir & SDE_TRANSB_FIFO_UNDER)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
 }
 
 static void ivb_err_int_handler(struct drm_i915_private *dev_priv)
@@ -2123,13 +2123,13 @@ static void cpt_serr_int_handler(struct drm_i915_private *dev_priv)
 		DRM_ERROR("PCH poison interrupt\n");
 
 	if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
 
 	if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
 
 	if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C);
 
 	I915_WRITE(SERR_INT, serr_int);
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bb9c9c3c391f..5c7054c3be0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1777,7 +1777,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 
 	/* FDI must be feeding us bits for PCH ports */
 	assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder);
-	assert_fdi_rx_enabled(dev_priv, TRANSCODER_A);
+	assert_fdi_rx_enabled(dev_priv, PIPE_A);
 
 	/* Workaround: set timing override bit. */
 	val = I915_READ(TRANS_CHICKEN2(PIPE_A));
@@ -1853,16 +1853,16 @@ void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
 	I915_WRITE(TRANS_CHICKEN2(PIPE_A), val);
 }
 
-enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc)
+enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
 	WARN_ON(!crtc->config->has_pch_encoder);
 
 	if (HAS_PCH_LPT(dev_priv))
-		return TRANSCODER_A;
+		return PIPE_A;
 	else
-		return (enum transcoder) crtc->pipe;
+		return crtc->pipe;
 }
 
 /**
@@ -1901,7 +1901,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 		if (crtc->config->has_pch_encoder) {
 			/* if driving the PCH, we need FDI enabled */
 			assert_fdi_rx_pll_enabled(dev_priv,
-						  (enum pipe) intel_crtc_pch_transcoder(crtc));
+						  intel_crtc_pch_transcoder(crtc));
 			assert_fdi_tx_pll_enabled(dev_priv,
 						  (enum pipe) cpu_transcoder);
 		}
@@ -4579,7 +4579,7 @@ static void lpt_pch_enable(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
-	assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A);
+	assert_pch_transcoder_disabled(dev_priv, PIPE_A);
 
 	lpt_program_iclkip(crtc);
 
@ -5347,8 +5347,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		return;
 
 	if (intel_crtc->config->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-						      false);
+		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
@@ -5433,8 +5432,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		intel_wait_for_vblank(dev_priv, pipe);
 		intel_wait_for_vblank(dev_priv, pipe);
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
-		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-						      true);
+		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 	}
 
 	/* If we change the relative order between pipe/planes enabling, we need
@@ -5531,8 +5529,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 
 	if (intel_crtc->config->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-						      false);
+		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
 	intel_encoders_disable(crtc, old_crtc_state, old_state);
 
@@ -5560,8 +5557,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
 
 	if (old_crtc_state->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-						      true);
+		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 }
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a32437f07..0902d7cb48d9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1211,12 +1211,12 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
 bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
 					   enum pipe pipe, bool enable);
 bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
-					   enum transcoder pch_transcoder,
+					   enum pipe pch_transcoder,
 					   bool enable);
 void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 					 enum pipe pipe);
 void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
-					 enum transcoder pch_transcoder);
+					 enum pipe pch_transcoder);
 void intel_check_cpu_fifo_underruns(struct drm_i915_private *dev_priv);
 void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
 
@@ -1326,7 +1326,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 /* intel_display.c */
 void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
 void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
-enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
+enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
 void intel_update_rawclk(struct drm_i915_private *dev_priv);
 int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
 int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index d484862cc7df..5a7cca32c0fa 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -313,11 +313,11 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
  * Returns the previous state of underrun reporting.
  */
 bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
-					   enum transcoder pch_transcoder,
+					   enum pipe pch_transcoder,
 					   bool enable)
 {
 	struct intel_crtc *crtc =
-		intel_get_crtc_for_pipe(dev_priv, (enum pipe) pch_transcoder);
+		intel_get_crtc_for_pipe(dev_priv, pch_transcoder);
 	unsigned long flags;
 	bool old;
 
@@ -390,7 +390,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
  * interrupt to avoid an irq storm.
  */
 void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
-					 enum transcoder pch_transcoder)
+					 enum pipe pch_transcoder)
 {
 	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
 						  false)) {
-- 
2.13.2.932.g7449e964c-goog

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
@ 2017-07-17 18:14 ` Matthias Kaehlcke
  0 siblings, 0 replies; 8+ messages in thread
From: Matthias Kaehlcke @ 2017-07-17 18:14 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Daniel Vetter
  Cc: Grant Grundler, intel-gfx, Linux Kernel Mailing List, dri-devel,
	Stéphane Marchesin, Matthias Kaehlcke

The current code uses in some instances enum transcoder for PCH
transcoders and enum pipe in others. This is error prone and clang
raises warnings like this:

drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
  from enumeration type 'enum pipe' to different enumeration type
  'enum transcoder' [-Wenum-conversion]
    intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);

Consistently use the type enum pipe for PCH transcoders.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- rebased on drm-intel/drm-intel-next-queued

 drivers/gpu/drm/i915/i915_irq.c            | 10 +++++-----
 drivers/gpu/drm/i915/intel_display.c       | 24 ++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h           |  6 +++---
 drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
 4 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1d33cea01a1b..0b6f310101ee 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2086,10 +2086,10 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 		DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n");
 
 	if (pch_iir & SDE_TRANSA_FIFO_UNDER)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
 
 	if (pch_iir & SDE_TRANSB_FIFO_UNDER)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
 }
 
 static void ivb_err_int_handler(struct drm_i915_private *dev_priv)
@@ -2123,13 +2123,13 @@ static void cpt_serr_int_handler(struct drm_i915_private *dev_priv)
 		DRM_ERROR("PCH poison interrupt\n");
 
 	if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
 
 	if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
 
 	if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN)
-		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C);
+		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C);
 
 	I915_WRITE(SERR_INT, serr_int);
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bb9c9c3c391f..5c7054c3be0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1777,7 +1777,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 
 	/* FDI must be feeding us bits for PCH ports */
 	assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder);
-	assert_fdi_rx_enabled(dev_priv, TRANSCODER_A);
+	assert_fdi_rx_enabled(dev_priv, PIPE_A);
 
 	/* Workaround: set timing override bit. */
 	val = I915_READ(TRANS_CHICKEN2(PIPE_A));
@@ -1853,16 +1853,16 @@ void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
 	I915_WRITE(TRANS_CHICKEN2(PIPE_A), val);
 }
 
-enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc)
+enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
 	WARN_ON(!crtc->config->has_pch_encoder);
 
 	if (HAS_PCH_LPT(dev_priv))
-		return TRANSCODER_A;
+		return PIPE_A;
 	else
-		return (enum transcoder) crtc->pipe;
+		return crtc->pipe;
 }
 
 /**
@@ -1901,7 +1901,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 		if (crtc->config->has_pch_encoder) {
 			/* if driving the PCH, we need FDI enabled */
 			assert_fdi_rx_pll_enabled(dev_priv,
-						  (enum pipe) intel_crtc_pch_transcoder(crtc));
+						  intel_crtc_pch_transcoder(crtc));
 			assert_fdi_tx_pll_enabled(dev_priv,
 						  (enum pipe) cpu_transcoder);
 		}
@@ -4579,7 +4579,7 @@ static void lpt_pch_enable(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
-	assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A);
+	assert_pch_transcoder_disabled(dev_priv, PIPE_A);
 
 	lpt_program_iclkip(crtc);
 
@ -5347,8 +5347,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		return;
 
 	if (intel_crtc->config->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-						      false);
+		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
@@ -5433,8 +5432,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		intel_wait_for_vblank(dev_priv, pipe);
 		intel_wait_for_vblank(dev_priv, pipe);
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
-		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-						      true);
+		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 	}
 
 	/* If we change the relative order between pipe/planes enabling, we need
@@ -5531,8 +5529,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 
 	if (intel_crtc->config->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-						      false);
+		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
 	intel_encoders_disable(crtc, old_crtc_state, old_state);
 
@@ -5560,8 +5557,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
 
 	if (old_crtc_state->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-						      true);
+		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 }
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a32437f07..0902d7cb48d9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1211,12 +1211,12 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
 bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
 					   enum pipe pipe, bool enable);
 bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
-					   enum transcoder pch_transcoder,
+					   enum pipe pch_transcoder,
 					   bool enable);
 void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 					 enum pipe pipe);
 void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
-					 enum transcoder pch_transcoder);
+					 enum pipe pch_transcoder);
 void intel_check_cpu_fifo_underruns(struct drm_i915_private *dev_priv);
 void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
 
@@ -1326,7 +1326,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 /* intel_display.c */
 void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
 void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
-enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
+enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
 void intel_update_rawclk(struct drm_i915_private *dev_priv);
 int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
 int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index d484862cc7df..5a7cca32c0fa 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -313,11 +313,11 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
  * Returns the previous state of underrun reporting.
  */
 bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
-					   enum transcoder pch_transcoder,
+					   enum pipe pch_transcoder,
 					   bool enable)
 {
 	struct intel_crtc *crtc =
-		intel_get_crtc_for_pipe(dev_priv, (enum pipe) pch_transcoder);
+		intel_get_crtc_for_pipe(dev_priv, pch_transcoder);
 	unsigned long flags;
 	bool old;
 
@@ -390,7 +390,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
  * interrupt to avoid an irq storm.
  */
 void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
-					 enum transcoder pch_transcoder)
+					 enum pipe pch_transcoder)
 {
 	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
 						  false)) {
-- 
2.13.2.932.g7449e964c-goog

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
  2017-07-17 18:14 ` Matthias Kaehlcke
  (?)
@ 2017-07-18  6:39 ` Daniel Vetter
  2017-07-18 20:48   ` Matthias Kaehlcke
  -1 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2017-07-18  6:39 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Daniel Vetter, Jani Nikula, Daniel Vetter, intel-gfx,
	Linux Kernel Mailing List, dri-devel, Stéphane Marchesin,
	Grant Grundler

On Mon, Jul 17, 2017 at 11:14:03AM -0700, Matthias Kaehlcke wrote:
> The current code uses in some instances enum transcoder for PCH
> transcoders and enum pipe in others. This is error prone and clang
> raises warnings like this:
> 
> drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
>   from enumeration type 'enum pipe' to different enumeration type
>   'enum transcoder' [-Wenum-conversion]
>     intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> 
> Consistently use the type enum pipe for PCH transcoders.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

Somehow git apply-mbox could parse it, but manually applying using patch
worked. Not sure what's going on, maybe double-check it's all right.

Thanks, Daniel
> ---
> Changes in v2:
> - rebased on drm-intel/drm-intel-next-queued
> 
>  drivers/gpu/drm/i915/i915_irq.c            | 10 +++++-----
>  drivers/gpu/drm/i915/intel_display.c       | 24 ++++++++++--------------
>  drivers/gpu/drm/i915/intel_drv.h           |  6 +++---
>  drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
>  4 files changed, 21 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1d33cea01a1b..0b6f310101ee 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2086,10 +2086,10 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>  		DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n");
>  
>  	if (pch_iir & SDE_TRANSA_FIFO_UNDER)
> -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
> +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
>  
>  	if (pch_iir & SDE_TRANSB_FIFO_UNDER)
> -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
> +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
>  }
>  
>  static void ivb_err_int_handler(struct drm_i915_private *dev_priv)
> @@ -2123,13 +2123,13 @@ static void cpt_serr_int_handler(struct drm_i915_private *dev_priv)
>  		DRM_ERROR("PCH poison interrupt\n");
>  
>  	if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN)
> -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
> +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
>  
>  	if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN)
> -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
> +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
>  
>  	if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN)
> -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C);
> +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C);
>  
>  	I915_WRITE(SERR_INT, serr_int);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bb9c9c3c391f..5c7054c3be0e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1777,7 +1777,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
>  
>  	/* FDI must be feeding us bits for PCH ports */
>  	assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder);
> -	assert_fdi_rx_enabled(dev_priv, TRANSCODER_A);
> +	assert_fdi_rx_enabled(dev_priv, PIPE_A);
>  
>  	/* Workaround: set timing override bit. */
>  	val = I915_READ(TRANS_CHICKEN2(PIPE_A));
> @@ -1853,16 +1853,16 @@ void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
>  	I915_WRITE(TRANS_CHICKEN2(PIPE_A), val);
>  }
>  
> -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc)
> +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  
>  	WARN_ON(!crtc->config->has_pch_encoder);
>  
>  	if (HAS_PCH_LPT(dev_priv))
> -		return TRANSCODER_A;
> +		return PIPE_A;
>  	else
> -		return (enum transcoder) crtc->pipe;
> +		return crtc->pipe;
>  }
>  
>  /**
> @@ -1901,7 +1901,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>  		if (crtc->config->has_pch_encoder) {
>  			/* if driving the PCH, we need FDI enabled */
>  			assert_fdi_rx_pll_enabled(dev_priv,
> -						  (enum pipe) intel_crtc_pch_transcoder(crtc));
> +						  intel_crtc_pch_transcoder(crtc));
>  			assert_fdi_tx_pll_enabled(dev_priv,
>  						  (enum pipe) cpu_transcoder);
>  		}
> @@ -4579,7 +4579,7 @@ static void lpt_pch_enable(const struct intel_crtc_state *crtc_state)
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
> -	assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A);
> +	assert_pch_transcoder_disabled(dev_priv, PIPE_A);
>  
>  	lpt_program_iclkip(crtc);
>  
> @ -5347,8 +5347,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  		return;
>  
>  	if (intel_crtc->config->has_pch_encoder)
> -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> -						      false);
> +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>  
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>  
> @@ -5433,8 +5432,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  		intel_wait_for_vblank(dev_priv, pipe);
>  		intel_wait_for_vblank(dev_priv, pipe);
>  		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> -						      true);
> +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
>  	}
>  
>  	/* If we change the relative order between pipe/planes enabling, we need
> @@ -5531,8 +5529,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  
>  	if (intel_crtc->config->has_pch_encoder)
> -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> -						      false);
> +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>  
>  	intel_encoders_disable(crtc, old_crtc_state, old_state);
>  
> @@ -5560,8 +5557,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>  
>  	if (old_crtc_state->has_pch_encoder)
> -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> -						      true);
> +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
>  }
>  
>  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d17a32437f07..0902d7cb48d9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1211,12 +1211,12 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
>  bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
>  					   enum pipe pipe, bool enable);
>  bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> -					   enum transcoder pch_transcoder,
> +					   enum pipe pch_transcoder,
>  					   bool enable);
>  void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>  					 enum pipe pipe);
>  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> -					 enum transcoder pch_transcoder);
> +					 enum pipe pch_transcoder);
>  void intel_check_cpu_fifo_underruns(struct drm_i915_private *dev_priv);
>  void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
>  
> @@ -1326,7 +1326,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
>  /* intel_display.c */
>  void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
>  void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
>  void intel_update_rawclk(struct drm_i915_private *dev_priv);
>  int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
>  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> index d484862cc7df..5a7cca32c0fa 100644
> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> @@ -313,11 +313,11 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
>   * Returns the previous state of underrun reporting.
>   */
>  bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> -					   enum transcoder pch_transcoder,
> +					   enum pipe pch_transcoder,
>  					   bool enable)
>  {
>  	struct intel_crtc *crtc =
> -		intel_get_crtc_for_pipe(dev_priv, (enum pipe) pch_transcoder);
> +		intel_get_crtc_for_pipe(dev_priv, pch_transcoder);
>  	unsigned long flags;
>  	bool old;
>  
> @@ -390,7 +390,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>   * interrupt to avoid an irq storm.
>   */
>  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> -					 enum transcoder pch_transcoder)
> +					 enum pipe pch_transcoder)
>  {
>  	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
>  						  false)) {
> -- 
> 2.13.2.932.g7449e964c-goog
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
  2017-07-18  6:39 ` Daniel Vetter
@ 2017-07-18 20:48   ` Matthias Kaehlcke
  2017-07-19  6:30       ` Daniel Vetter
  0 siblings, 1 reply; 8+ messages in thread
From: Matthias Kaehlcke @ 2017-07-18 20:48 UTC (permalink / raw)
  To: Jani Nikula, Daniel Vetter, intel-gfx, Linux Kernel Mailing List,
	dri-devel, Stéphane Marchesin, Grant Grundler

Hi Daniel,

El Tue, Jul 18, 2017 at 08:39:50AM +0200 Daniel Vetter ha dit:

> On Mon, Jul 17, 2017 at 11:14:03AM -0700, Matthias Kaehlcke wrote:
> > The current code uses in some instances enum transcoder for PCH
> > transcoders and enum pipe in others. This is error prone and clang
> > raises warnings like this:
> > 
> > drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
> >   from enumeration type 'enum pipe' to different enumeration type
> >   'enum transcoder' [-Wenum-conversion]
> >     intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > 
> > Consistently use the type enum pipe for PCH transcoders.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> 
> Somehow git apply-mbox could parse it, but manually applying using patch
> worked. Not sure what's going on, maybe double-check it's all right.

Not sure what happened, one of the patch fragments only has one '@'
instead of two, with that fixed the patch applies.

Unfortunately the manual application missed some fragments:

drivers/gpu/drm/i915/intel_display.c:5350:51: warning: implicit
  conversion from enumeration type 'enum transcoder' to different
  enumeration type 'enum pipe' [-Wenum-conversion]
                intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:5436:51: warning: implicit
  conversion from enumeration type 'enum transcoder' to different
  enumeration type 'enum pipe' [-Wenum-conversion]
                intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:5534:51: warning: implicit
  conversion from enumeration type 'enum transcoder' to different
  enumeration type 'enum pipe' [-Wenum-conversion]
                intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:5563:51: warning: implicit
  conversion from enumeration type 'enum transcoder' to different
  enumeration type 'enum pipe' [-Wenum-conversion]
                intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,


What would be the best way forward from here? Revert the manual
application and apply again, or a fixup patch?

Matthias

> > ---
> > Changes in v2:
> > - rebased on drm-intel/drm-intel-next-queued
> > 
> >  drivers/gpu/drm/i915/i915_irq.c            | 10 +++++-----
> >  drivers/gpu/drm/i915/intel_display.c       | 24 ++++++++++--------------
> >  drivers/gpu/drm/i915/intel_drv.h           |  6 +++---
> >  drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
> >  4 files changed, 21 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 1d33cea01a1b..0b6f310101ee 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2086,10 +2086,10 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> >  		DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n");
> >  
> >  	if (pch_iir & SDE_TRANSA_FIFO_UNDER)
> > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
> > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
> >  
> >  	if (pch_iir & SDE_TRANSB_FIFO_UNDER)
> > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
> > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
> >  }
> >  
> >  static void ivb_err_int_handler(struct drm_i915_private *dev_priv)
> > @@ -2123,13 +2123,13 @@ static void cpt_serr_int_handler(struct drm_i915_private *dev_priv)
> >  		DRM_ERROR("PCH poison interrupt\n");
> >  
> >  	if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN)
> > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
> > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
> >  
> >  	if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN)
> > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
> > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
> >  
> >  	if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN)
> > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C);
> > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C);
> >  
> >  	I915_WRITE(SERR_INT, serr_int);
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bb9c9c3c391f..5c7054c3be0e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1777,7 +1777,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> >  
> >  	/* FDI must be feeding us bits for PCH ports */
> >  	assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder);
> > -	assert_fdi_rx_enabled(dev_priv, TRANSCODER_A);
> > +	assert_fdi_rx_enabled(dev_priv, PIPE_A);
> >  
> >  	/* Workaround: set timing override bit. */
> >  	val = I915_READ(TRANS_CHICKEN2(PIPE_A));
> > @@ -1853,16 +1853,16 @@ void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
> >  	I915_WRITE(TRANS_CHICKEN2(PIPE_A), val);
> >  }
> >  
> > -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc)
> > +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  
> >  	WARN_ON(!crtc->config->has_pch_encoder);
> >  
> >  	if (HAS_PCH_LPT(dev_priv))
> > -		return TRANSCODER_A;
> > +		return PIPE_A;
> >  	else
> > -		return (enum transcoder) crtc->pipe;
> > +		return crtc->pipe;
> >  }
> >  
> >  /**
> > @@ -1901,7 +1901,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> >  		if (crtc->config->has_pch_encoder) {
> >  			/* if driving the PCH, we need FDI enabled */
> >  			assert_fdi_rx_pll_enabled(dev_priv,
> > -						  (enum pipe) intel_crtc_pch_transcoder(crtc));
> > +						  intel_crtc_pch_transcoder(crtc));
> >  			assert_fdi_tx_pll_enabled(dev_priv,
> >  						  (enum pipe) cpu_transcoder);
> >  		}
> > @@ -4579,7 +4579,7 @@ static void lpt_pch_enable(const struct intel_crtc_state *crtc_state)
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  
> > -	assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A);
> > +	assert_pch_transcoder_disabled(dev_priv, PIPE_A);
> >  
> >  	lpt_program_iclkip(crtc);
> >  
> > @ -5347,8 +5347,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >  		return;
> >  
> >  	if (intel_crtc->config->has_pch_encoder)
> > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > -						      false);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> >  
> >  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> >  
> > @@ -5433,8 +5432,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >  		intel_wait_for_vblank(dev_priv, pipe);
> >  		intel_wait_for_vblank(dev_priv, pipe);
> >  		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > -						      true);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> >  	}
> >  
> >  	/* If we change the relative order between pipe/planes enabling, we need
> > @@ -5531,8 +5529,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
> >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> >  
> >  	if (intel_crtc->config->has_pch_encoder)
> > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > -						      false);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> >  
> >  	intel_encoders_disable(crtc, old_crtc_state, old_state);
> >  
> > @@ -5560,8 +5557,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
> >  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
> >  
> >  	if (old_crtc_state->has_pch_encoder)
> > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > -						      true);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> >  }
> >  
> >  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index d17a32437f07..0902d7cb48d9 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1211,12 +1211,12 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
> >  bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> >  					   enum pipe pipe, bool enable);
> >  bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > -					   enum transcoder pch_transcoder,
> > +					   enum pipe pch_transcoder,
> >  					   bool enable);
> >  void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> >  					 enum pipe pipe);
> >  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > -					 enum transcoder pch_transcoder);
> > +					 enum pipe pch_transcoder);
> >  void intel_check_cpu_fifo_underruns(struct drm_i915_private *dev_priv);
> >  void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
> >  
> > @@ -1326,7 +1326,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
> >  /* intel_display.c */
> >  void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> >  void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> > -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> > +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> >  void intel_update_rawclk(struct drm_i915_private *dev_priv);
> >  int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
> >  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > index d484862cc7df..5a7cca32c0fa 100644
> > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > @@ -313,11 +313,11 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> >   * Returns the previous state of underrun reporting.
> >   */
> >  bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > -					   enum transcoder pch_transcoder,
> > +					   enum pipe pch_transcoder,
> >  					   bool enable)
> >  {
> >  	struct intel_crtc *crtc =
> > -		intel_get_crtc_for_pipe(dev_priv, (enum pipe) pch_transcoder);
> > +		intel_get_crtc_for_pipe(dev_priv, pch_transcoder);
> >  	unsigned long flags;
> >  	bool old;
> >  
> > @@ -390,7 +390,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> >   * interrupt to avoid an irq storm.
> >   */
> >  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > -					 enum transcoder pch_transcoder)
> > +					 enum pipe pch_transcoder)
> >  {
> >  	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
> >  						  false)) {
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
  2017-07-18 20:48   ` Matthias Kaehlcke
@ 2017-07-19  6:30       ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2017-07-19  6:30 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Jani Nikula, Daniel Vetter, intel-gfx, Linux Kernel Mailing List,
	dri-devel, Stéphane Marchesin, Grant Grundler

On Tue, Jul 18, 2017 at 01:48:53PM -0700, Matthias Kaehlcke wrote:
> Hi Daniel,
> 
> El Tue, Jul 18, 2017 at 08:39:50AM +0200 Daniel Vetter ha dit:
> 
> > On Mon, Jul 17, 2017 at 11:14:03AM -0700, Matthias Kaehlcke wrote:
> > > The current code uses in some instances enum transcoder for PCH
> > > transcoders and enum pipe in others. This is error prone and clang
> > > raises warnings like this:
> > > 
> > > drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
> > >   from enumeration type 'enum pipe' to different enumeration type
> > >   'enum transcoder' [-Wenum-conversion]
> > >     intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > > 
> > > Consistently use the type enum pipe for PCH transcoders.
> > > 
> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > 
> > Somehow git apply-mbox could parse it, but manually applying using patch
> > worked. Not sure what's going on, maybe double-check it's all right.
> 
> Not sure what happened, one of the patch fragments only has one '@'
> instead of two, with that fixed the patch applies.
> 
> Unfortunately the manual application missed some fragments:
> 
> drivers/gpu/drm/i915/intel_display.c:5350:51: warning: implicit
>   conversion from enumeration type 'enum transcoder' to different
>   enumeration type 'enum pipe' [-Wenum-conversion]
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
>                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> drivers/gpu/drm/i915/intel_display.c:5436:51: warning: implicit
>   conversion from enumeration type 'enum transcoder' to different
>   enumeration type 'enum pipe' [-Wenum-conversion]
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
>                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> drivers/gpu/drm/i915/intel_display.c:5534:51: warning: implicit
>   conversion from enumeration type 'enum transcoder' to different
>   enumeration type 'enum pipe' [-Wenum-conversion]
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
>                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> drivers/gpu/drm/i915/intel_display.c:5563:51: warning: implicit
>   conversion from enumeration type 'enum transcoder' to different
>   enumeration type 'enum pipe' [-Wenum-conversion]
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> 
> 
> What would be the best way forward from here? Revert the manual
> application and apply again, or a fixup patch?

Drat I screwed up :-( drm-intel-next-queued is non-rebasing, that means I
need a fixup patch. I should have checked more carefully that I have all
the hunks, but patch -p1 seemed happy ...
-Daniel

> 
> Matthias
> 
> > > ---
> > > Changes in v2:
> > > - rebased on drm-intel/drm-intel-next-queued
> > > 
> > >  drivers/gpu/drm/i915/i915_irq.c            | 10 +++++-----
> > >  drivers/gpu/drm/i915/intel_display.c       | 24 ++++++++++--------------
> > >  drivers/gpu/drm/i915/intel_drv.h           |  6 +++---
> > >  drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
> > >  4 files changed, 21 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index 1d33cea01a1b..0b6f310101ee 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2086,10 +2086,10 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> > >  		DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n");
> > >  
> > >  	if (pch_iir & SDE_TRANSA_FIFO_UNDER)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
> > >  
> > >  	if (pch_iir & SDE_TRANSB_FIFO_UNDER)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
> > >  }
> > >  
> > >  static void ivb_err_int_handler(struct drm_i915_private *dev_priv)
> > > @@ -2123,13 +2123,13 @@ static void cpt_serr_int_handler(struct drm_i915_private *dev_priv)
> > >  		DRM_ERROR("PCH poison interrupt\n");
> > >  
> > >  	if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
> > >  
> > >  	if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
> > >  
> > >  	if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C);
> > >  
> > >  	I915_WRITE(SERR_INT, serr_int);
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index bb9c9c3c391f..5c7054c3be0e 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -1777,7 +1777,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> > >  
> > >  	/* FDI must be feeding us bits for PCH ports */
> > >  	assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder);
> > > -	assert_fdi_rx_enabled(dev_priv, TRANSCODER_A);
> > > +	assert_fdi_rx_enabled(dev_priv, PIPE_A);
> > >  
> > >  	/* Workaround: set timing override bit. */
> > >  	val = I915_READ(TRANS_CHICKEN2(PIPE_A));
> > > @@ -1853,16 +1853,16 @@ void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
> > >  	I915_WRITE(TRANS_CHICKEN2(PIPE_A), val);
> > >  }
> > >  
> > > -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc)
> > > +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >  
> > >  	WARN_ON(!crtc->config->has_pch_encoder);
> > >  
> > >  	if (HAS_PCH_LPT(dev_priv))
> > > -		return TRANSCODER_A;
> > > +		return PIPE_A;
> > >  	else
> > > -		return (enum transcoder) crtc->pipe;
> > > +		return crtc->pipe;
> > >  }
> > >  
> > >  /**
> > > @@ -1901,7 +1901,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> > >  		if (crtc->config->has_pch_encoder) {
> > >  			/* if driving the PCH, we need FDI enabled */
> > >  			assert_fdi_rx_pll_enabled(dev_priv,
> > > -						  (enum pipe) intel_crtc_pch_transcoder(crtc));
> > > +						  intel_crtc_pch_transcoder(crtc));
> > >  			assert_fdi_tx_pll_enabled(dev_priv,
> > >  						  (enum pipe) cpu_transcoder);
> > >  		}
> > > @@ -4579,7 +4579,7 @@ static void lpt_pch_enable(const struct intel_crtc_state *crtc_state)
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > >  
> > > -	assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A);
> > > +	assert_pch_transcoder_disabled(dev_priv, PIPE_A);
> > >  
> > >  	lpt_program_iclkip(crtc);
> > >  
> > > @ -5347,8 +5347,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > >  		return;
> > >  
> > >  	if (intel_crtc->config->has_pch_encoder)
> > > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > > -						      false);
> > > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > >  
> > >  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> > >  
> > > @@ -5433,8 +5432,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > >  		intel_wait_for_vblank(dev_priv, pipe);
> > >  		intel_wait_for_vblank(dev_priv, pipe);
> > >  		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> > > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > > -						      true);
> > > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > >  	}
> > >  
> > >  	/* If we change the relative order between pipe/planes enabling, we need
> > > @@ -5531,8 +5529,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
> > >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> > >  
> > >  	if (intel_crtc->config->has_pch_encoder)
> > > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > > -						      false);
> > > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > >  
> > >  	intel_encoders_disable(crtc, old_crtc_state, old_state);
> > >  
> > > @@ -5560,8 +5557,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
> > >  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
> > >  
> > >  	if (old_crtc_state->has_pch_encoder)
> > > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > > -						      true);
> > > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > >  }
> > >  
> > >  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index d17a32437f07..0902d7cb48d9 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1211,12 +1211,12 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
> > >  bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > >  					   enum pipe pipe, bool enable);
> > >  bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > > -					   enum transcoder pch_transcoder,
> > > +					   enum pipe pch_transcoder,
> > >  					   bool enable);
> > >  void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > >  					 enum pipe pipe);
> > >  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > > -					 enum transcoder pch_transcoder);
> > > +					 enum pipe pch_transcoder);
> > >  void intel_check_cpu_fifo_underruns(struct drm_i915_private *dev_priv);
> > >  void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
> > >  
> > > @@ -1326,7 +1326,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > >  /* intel_display.c */
> > >  void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> > >  void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> > > -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> > > +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> > >  void intel_update_rawclk(struct drm_i915_private *dev_priv);
> > >  int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
> > >  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> > > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > > index d484862cc7df..5a7cca32c0fa 100644
> > > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > > @@ -313,11 +313,11 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > >   * Returns the previous state of underrun reporting.
> > >   */
> > >  bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > > -					   enum transcoder pch_transcoder,
> > > +					   enum pipe pch_transcoder,
> > >  					   bool enable)
> > >  {
> > >  	struct intel_crtc *crtc =
> > > -		intel_get_crtc_for_pipe(dev_priv, (enum pipe) pch_transcoder);
> > > +		intel_get_crtc_for_pipe(dev_priv, pch_transcoder);
> > >  	unsigned long flags;
> > >  	bool old;
> > >  
> > > @@ -390,7 +390,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > >   * interrupt to avoid an irq storm.
> > >   */
> > >  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > > -					 enum transcoder pch_transcoder)
> > > +					 enum pipe pch_transcoder)
> > >  {
> > >  	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
> > >  						  false)) {
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
@ 2017-07-19  6:30       ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2017-07-19  6:30 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Grant Grundler, intel-gfx, Linux Kernel Mailing List, dri-devel,
	Stéphane Marchesin, Daniel Vetter

On Tue, Jul 18, 2017 at 01:48:53PM -0700, Matthias Kaehlcke wrote:
> Hi Daniel,
> 
> El Tue, Jul 18, 2017 at 08:39:50AM +0200 Daniel Vetter ha dit:
> 
> > On Mon, Jul 17, 2017 at 11:14:03AM -0700, Matthias Kaehlcke wrote:
> > > The current code uses in some instances enum transcoder for PCH
> > > transcoders and enum pipe in others. This is error prone and clang
> > > raises warnings like this:
> > > 
> > > drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
> > >   from enumeration type 'enum pipe' to different enumeration type
> > >   'enum transcoder' [-Wenum-conversion]
> > >     intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > > 
> > > Consistently use the type enum pipe for PCH transcoders.
> > > 
> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > 
> > Somehow git apply-mbox could parse it, but manually applying using patch
> > worked. Not sure what's going on, maybe double-check it's all right.
> 
> Not sure what happened, one of the patch fragments only has one '@'
> instead of two, with that fixed the patch applies.
> 
> Unfortunately the manual application missed some fragments:
> 
> drivers/gpu/drm/i915/intel_display.c:5350:51: warning: implicit
>   conversion from enumeration type 'enum transcoder' to different
>   enumeration type 'enum pipe' [-Wenum-conversion]
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
>                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> drivers/gpu/drm/i915/intel_display.c:5436:51: warning: implicit
>   conversion from enumeration type 'enum transcoder' to different
>   enumeration type 'enum pipe' [-Wenum-conversion]
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
>                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> drivers/gpu/drm/i915/intel_display.c:5534:51: warning: implicit
>   conversion from enumeration type 'enum transcoder' to different
>   enumeration type 'enum pipe' [-Wenum-conversion]
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
>                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> drivers/gpu/drm/i915/intel_display.c:5563:51: warning: implicit
>   conversion from enumeration type 'enum transcoder' to different
>   enumeration type 'enum pipe' [-Wenum-conversion]
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> 
> 
> What would be the best way forward from here? Revert the manual
> application and apply again, or a fixup patch?

Drat I screwed up :-( drm-intel-next-queued is non-rebasing, that means I
need a fixup patch. I should have checked more carefully that I have all
the hunks, but patch -p1 seemed happy ...
-Daniel

> 
> Matthias
> 
> > > ---
> > > Changes in v2:
> > > - rebased on drm-intel/drm-intel-next-queued
> > > 
> > >  drivers/gpu/drm/i915/i915_irq.c            | 10 +++++-----
> > >  drivers/gpu/drm/i915/intel_display.c       | 24 ++++++++++--------------
> > >  drivers/gpu/drm/i915/intel_drv.h           |  6 +++---
> > >  drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
> > >  4 files changed, 21 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index 1d33cea01a1b..0b6f310101ee 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2086,10 +2086,10 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> > >  		DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n");
> > >  
> > >  	if (pch_iir & SDE_TRANSA_FIFO_UNDER)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
> > >  
> > >  	if (pch_iir & SDE_TRANSB_FIFO_UNDER)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
> > >  }
> > >  
> > >  static void ivb_err_int_handler(struct drm_i915_private *dev_priv)
> > > @@ -2123,13 +2123,13 @@ static void cpt_serr_int_handler(struct drm_i915_private *dev_priv)
> > >  		DRM_ERROR("PCH poison interrupt\n");
> > >  
> > >  	if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A);
> > >  
> > >  	if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B);
> > >  
> > >  	if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN)
> > > -		intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C);
> > > +		intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C);
> > >  
> > >  	I915_WRITE(SERR_INT, serr_int);
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index bb9c9c3c391f..5c7054c3be0e 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -1777,7 +1777,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> > >  
> > >  	/* FDI must be feeding us bits for PCH ports */
> > >  	assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder);
> > > -	assert_fdi_rx_enabled(dev_priv, TRANSCODER_A);
> > > +	assert_fdi_rx_enabled(dev_priv, PIPE_A);
> > >  
> > >  	/* Workaround: set timing override bit. */
> > >  	val = I915_READ(TRANS_CHICKEN2(PIPE_A));
> > > @@ -1853,16 +1853,16 @@ void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
> > >  	I915_WRITE(TRANS_CHICKEN2(PIPE_A), val);
> > >  }
> > >  
> > > -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc)
> > > +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >  
> > >  	WARN_ON(!crtc->config->has_pch_encoder);
> > >  
> > >  	if (HAS_PCH_LPT(dev_priv))
> > > -		return TRANSCODER_A;
> > > +		return PIPE_A;
> > >  	else
> > > -		return (enum transcoder) crtc->pipe;
> > > +		return crtc->pipe;
> > >  }
> > >  
> > >  /**
> > > @@ -1901,7 +1901,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> > >  		if (crtc->config->has_pch_encoder) {
> > >  			/* if driving the PCH, we need FDI enabled */
> > >  			assert_fdi_rx_pll_enabled(dev_priv,
> > > -						  (enum pipe) intel_crtc_pch_transcoder(crtc));
> > > +						  intel_crtc_pch_transcoder(crtc));
> > >  			assert_fdi_tx_pll_enabled(dev_priv,
> > >  						  (enum pipe) cpu_transcoder);
> > >  		}
> > > @@ -4579,7 +4579,7 @@ static void lpt_pch_enable(const struct intel_crtc_state *crtc_state)
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > >  
> > > -	assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A);
> > > +	assert_pch_transcoder_disabled(dev_priv, PIPE_A);
> > >  
> > >  	lpt_program_iclkip(crtc);
> > >  
> > > @ -5347,8 +5347,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > >  		return;
> > >  
> > >  	if (intel_crtc->config->has_pch_encoder)
> > > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > > -						      false);
> > > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > >  
> > >  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> > >  
> > > @@ -5433,8 +5432,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > >  		intel_wait_for_vblank(dev_priv, pipe);
> > >  		intel_wait_for_vblank(dev_priv, pipe);
> > >  		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> > > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > > -						      true);
> > > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > >  	}
> > >  
> > >  	/* If we change the relative order between pipe/planes enabling, we need
> > > @@ -5531,8 +5529,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
> > >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> > >  
> > >  	if (intel_crtc->config->has_pch_encoder)
> > > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > > -						      false);
> > > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > >  
> > >  	intel_encoders_disable(crtc, old_crtc_state, old_state);
> > >  
> > > @@ -5560,8 +5557,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
> > >  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
> > >  
> > >  	if (old_crtc_state->has_pch_encoder)
> > > -		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > > -						      true);
> > > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > >  }
> > >  
> > >  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index d17a32437f07..0902d7cb48d9 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1211,12 +1211,12 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
> > >  bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > >  					   enum pipe pipe, bool enable);
> > >  bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > > -					   enum transcoder pch_transcoder,
> > > +					   enum pipe pch_transcoder,
> > >  					   bool enable);
> > >  void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > >  					 enum pipe pipe);
> > >  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > > -					 enum transcoder pch_transcoder);
> > > +					 enum pipe pch_transcoder);
> > >  void intel_check_cpu_fifo_underruns(struct drm_i915_private *dev_priv);
> > >  void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
> > >  
> > > @@ -1326,7 +1326,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > >  /* intel_display.c */
> > >  void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> > >  void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> > > -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> > > +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> > >  void intel_update_rawclk(struct drm_i915_private *dev_priv);
> > >  int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
> > >  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> > > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > > index d484862cc7df..5a7cca32c0fa 100644
> > > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > > @@ -313,11 +313,11 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > >   * Returns the previous state of underrun reporting.
> > >   */
> > >  bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
> > > -					   enum transcoder pch_transcoder,
> > > +					   enum pipe pch_transcoder,
> > >  					   bool enable)
> > >  {
> > >  	struct intel_crtc *crtc =
> > > -		intel_get_crtc_for_pipe(dev_priv, (enum pipe) pch_transcoder);
> > > +		intel_get_crtc_for_pipe(dev_priv, pch_transcoder);
> > >  	unsigned long flags;
> > >  	bool old;
> > >  
> > > @@ -390,7 +390,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > >   * interrupt to avoid an irq storm.
> > >   */
> > >  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > > -					 enum transcoder pch_transcoder)
> > > +					 enum pipe pch_transcoder)
> > >  {
> > >  	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
> > >  						  false)) {
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
  2017-07-19  6:30       ` Daniel Vetter
@ 2017-07-19 16:49         ` Matthias Kaehlcke
  -1 siblings, 0 replies; 8+ messages in thread
From: Matthias Kaehlcke @ 2017-07-19 16:49 UTC (permalink / raw)
  To: Jani Nikula, Daniel Vetter, intel-gfx, Linux Kernel Mailing List,
	dri-devel, Stéphane Marchesin, Grant Grundler

El Wed, Jul 19, 2017 at 08:30:36AM +0200 Daniel Vetter ha dit:

> On Tue, Jul 18, 2017 at 01:48:53PM -0700, Matthias Kaehlcke wrote:
> > Hi Daniel,
> > 
> > El Tue, Jul 18, 2017 at 08:39:50AM +0200 Daniel Vetter ha dit:
> > 
> > > On Mon, Jul 17, 2017 at 11:14:03AM -0700, Matthias Kaehlcke wrote:
> > > > The current code uses in some instances enum transcoder for PCH
> > > > transcoders and enum pipe in others. This is error prone and clang
> > > > raises warnings like this:
> > > > 
> > > > drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
> > > >   from enumeration type 'enum pipe' to different enumeration type
> > > >   'enum transcoder' [-Wenum-conversion]
> > > >     intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > > > 
> > > > Consistently use the type enum pipe for PCH transcoders.
> > > > 
> > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > 
> > > Somehow git apply-mbox could parse it, but manually applying using patch
> > > worked. Not sure what's going on, maybe double-check it's all right.
> > 
> > Not sure what happened, one of the patch fragments only has one '@'
> > instead of two, with that fixed the patch applies.
> > 
> > Unfortunately the manual application missed some fragments:
> > 
> > drivers/gpu/drm/i915/intel_display.c:5350:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> >                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> > drivers/gpu/drm/i915/intel_display.c:5436:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> >                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> > drivers/gpu/drm/i915/intel_display.c:5534:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> >                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> > drivers/gpu/drm/i915/intel_display.c:5563:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > 
> > 
> > What would be the best way forward from here? Revert the manual
> > application and apply again, or a fixup patch?
> 
> Drat I screwed up :-( drm-intel-next-queued is non-rebasing, that means I
> need a fixup patch. I should have checked more carefully that I have all
> the hunks, but patch -p1 seemed happy ...

Ok, I will send a fixup patch shortly

Matthias

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
@ 2017-07-19 16:49         ` Matthias Kaehlcke
  0 siblings, 0 replies; 8+ messages in thread
From: Matthias Kaehlcke @ 2017-07-19 16:49 UTC (permalink / raw)
  To: Jani Nikula, Daniel Vetter, intel-gfx, Linux Kernel Mailing List,
	dri-devel, Stéphane Marchesin, Grant Grundler

El Wed, Jul 19, 2017 at 08:30:36AM +0200 Daniel Vetter ha dit:

> On Tue, Jul 18, 2017 at 01:48:53PM -0700, Matthias Kaehlcke wrote:
> > Hi Daniel,
> > 
> > El Tue, Jul 18, 2017 at 08:39:50AM +0200 Daniel Vetter ha dit:
> > 
> > > On Mon, Jul 17, 2017 at 11:14:03AM -0700, Matthias Kaehlcke wrote:
> > > > The current code uses in some instances enum transcoder for PCH
> > > > transcoders and enum pipe in others. This is error prone and clang
> > > > raises warnings like this:
> > > > 
> > > > drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
> > > >   from enumeration type 'enum pipe' to different enumeration type
> > > >   'enum transcoder' [-Wenum-conversion]
> > > >     intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > > > 
> > > > Consistently use the type enum pipe for PCH transcoders.
> > > > 
> > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > 
> > > Somehow git apply-mbox could parse it, but manually applying using patch
> > > worked. Not sure what's going on, maybe double-check it's all right.
> > 
> > Not sure what happened, one of the patch fragments only has one '@'
> > instead of two, with that fixed the patch applies.
> > 
> > Unfortunately the manual application missed some fragments:
> > 
> > drivers/gpu/drm/i915/intel_display.c:5350:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> >                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> > drivers/gpu/drm/i915/intel_display.c:5436:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> >                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> > drivers/gpu/drm/i915/intel_display.c:5534:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> >                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^~~~~~~~~~~~
> > drivers/gpu/drm/i915/intel_display.c:5563:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > 
> > 
> > What would be the best way forward from here? Revert the manual
> > application and apply again, or a fixup patch?
> 
> Drat I screwed up :-( drm-intel-next-queued is non-rebasing, that means I
> need a fixup patch. I should have checked more carefully that I have all
> the hunks, but patch -p1 seemed happy ...

Ok, I will send a fixup patch shortly

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-07-19 16:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-17 18:14 [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders Matthias Kaehlcke
2017-07-17 18:14 ` Matthias Kaehlcke
2017-07-18  6:39 ` Daniel Vetter
2017-07-18 20:48   ` Matthias Kaehlcke
2017-07-19  6:30     ` Daniel Vetter
2017-07-19  6:30       ` Daniel Vetter
2017-07-19 16:49       ` Matthias Kaehlcke
2017-07-19 16:49         ` Matthias Kaehlcke

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.