All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Prep. for DP audio MST support
@ 2016-08-16  0:00 Dhinakaran Pandiyan
  2016-08-16  0:00 ` [PATCH v2 1/3] drm/i915: Add function to return port from an encoder Dhinakaran Pandiyan
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Dhinakaran Pandiyan @ 2016-08-16  0:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: libin.yang, Dhinakaran Pandiyan

This series lays the groundwork for more DP MST audio work that will
follow. 

(c) got possibly reverted because enc_to_dig_port() was incorrectly used
on MST encoders. Let's bring it back with the fix introduced in (a).

v2:
(a) -different solution replacing the enc_to_dig_port() fix (Ville, Lyude)
(b) and (c) -no changes since Lyude reviewed them.
Reordered the patches (Lyude) 


Dhinakaran Pandiyan (2):
  (a) drm/i915: Add function to return port from an encoder
  (b) drm/i915: Move audio_connector to intel_encoder

Libin Yang (1):
  (c) drm/i915: start adding dp mst audio

 drivers/gpu/drm/i915/i915_debugfs.c   | 19 ++++++++++-
 drivers/gpu/drm/i915/intel_audio.c    | 10 +++---
 drivers/gpu/drm/i915/intel_ddi.c      | 62 +++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_display.c  | 18 ++++++++++
 drivers/gpu/drm/i915/intel_dp_mst.c   | 21 ++++++++++++
 drivers/gpu/drm/i915/intel_drv.h      | 14 ++++++--
 drivers/gpu/drm/i915/intel_opregion.c |  2 +-
 7 files changed, 100 insertions(+), 46 deletions(-)

-- 
2.5.0

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] drm/i915: Add function to return port from an encoder
  2016-08-16  0:00 [PATCH v2 0/3] Prep. for DP audio MST support Dhinakaran Pandiyan
@ 2016-08-16  0:00 ` Dhinakaran Pandiyan
  2016-08-19  4:39   ` Rodrigo Vivi
  2016-08-19  8:02   ` Daniel Vetter
  2016-08-16  0:00 ` [PATCH v2 2/3] drm/i915: Move audio_connector to intel_encoder Dhinakaran Pandiyan
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Dhinakaran Pandiyan @ 2016-08-16  0:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: libin.yang, Dhinakaran Pandiyan

There are places in the driver where we just need the 'port' associated
with an encoder and not 'struct intel_digital_port' that contains it.
This basically is a generic implementation of intel_ddi_get_encoder_port()
handling both DDI and Non-DDI encoders. The handling of the analog encoder
is delegated to the DDI specific function.

The idea to have a generic implementation that returned the 'enum port'
from 'struct intel_encoder' came from Ville.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c      | 42 ++++++++++-------------------------
 drivers/gpu/drm/i915/intel_display.c  | 18 +++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h      |  8 ++++++-
 drivers/gpu/drm/i915/intel_opregion.c |  2 +-
 4 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c2df4e4..13ceef4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -301,24 +301,6 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
 	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
 };
 
-enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
-{
-	switch (encoder->type) {
-	case INTEL_OUTPUT_DP_MST:
-		return enc_to_mst(&encoder->base)->primary->port;
-	case INTEL_OUTPUT_DP:
-	case INTEL_OUTPUT_EDP:
-	case INTEL_OUTPUT_HDMI:
-	case INTEL_OUTPUT_UNKNOWN:
-		return enc_to_dig_port(&encoder->base)->port;
-	case INTEL_OUTPUT_ANALOG:
-		return PORT_E;
-	default:
-		MISSING_CASE(encoder->type);
-		return PORT_A;
-	}
-}
-
 static const struct ddi_buf_trans *
 bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 {
@@ -421,7 +403,7 @@ void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
 	int i, n_dp_entries, n_edp_entries, size;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = intel_get_encoder_port(encoder);
 	const struct ddi_buf_trans *ddi_translations_fdi;
 	const struct ddi_buf_trans *ddi_translations_dp;
 	const struct ddi_buf_trans *ddi_translations_edp;
@@ -499,7 +481,7 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
 	int n_hdmi_entries, hdmi_level;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = intel_get_encoder_port(encoder);
 	const struct ddi_buf_trans *ddi_translations_hdmi;
 
 	if (IS_BROXTON(dev_priv))
@@ -987,7 +969,7 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
 				struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = intel_get_encoder_port(encoder);
 	uint32_t dpll = port;
 
 	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
@@ -1130,7 +1112,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	enum pipe pipe = intel_crtc->pipe;
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
-	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	enum port port = intel_get_encoder_port(intel_encoder);
 	int type = intel_encoder->type;
 	uint32_t temp;
 
@@ -1226,7 +1208,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_encoder *intel_encoder = intel_connector->encoder;
 	int type = intel_connector->base.connector_type;
-	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	enum port port = intel_get_encoder_port(intel_encoder);
 	enum pipe pipe = 0;
 	enum transcoder cpu_transcoder;
 	enum intel_display_power_domain power_domain;
@@ -1286,7 +1268,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = intel_get_encoder_port(encoder);
 	enum intel_display_power_domain power_domain;
 	u32 tmp;
 	int i;
@@ -1361,7 +1343,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
-	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	enum port port = intel_get_encoder_port(intel_encoder);
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 
 	if (cpu_transcoder != TRANSCODER_EDP)
@@ -1589,7 +1571,7 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
 			  const struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = intel_get_encoder_port(encoder);
 
 	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
 		uint32_t dpll = pipe_config->ddi_pll_sel;
@@ -1616,7 +1598,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
 	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
-	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	enum port port = intel_get_encoder_port(intel_encoder);
 	int type = intel_encoder->type;
 
 	if (type == INTEL_OUTPUT_HDMI) {
@@ -1668,7 +1650,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	enum port port = intel_get_encoder_port(intel_encoder);
 	int type = intel_encoder->type;
 	uint32_t val;
 	bool wait = false;
@@ -1715,7 +1697,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	enum port port = intel_get_encoder_port(intel_encoder);
 	int type = intel_encoder->type;
 
 	if (type == INTEL_OUTPUT_HDMI) {
@@ -2276,7 +2258,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	int type = encoder->type;
-	int port = intel_ddi_get_encoder_port(encoder);
+	int port = intel_get_encoder_port(encoder);
 	int ret;
 
 	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9cbf543..44d20bd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16277,6 +16277,24 @@ err:
 	return ret;
 }
 
+enum port intel_get_encoder_port(struct intel_encoder *encoder)
+{
+        switch (encoder->type) {
+        case INTEL_OUTPUT_DP_MST:
+                return enc_to_mst(&encoder->base)->primary->port;
+        case INTEL_OUTPUT_DP:
+        case INTEL_OUTPUT_EDP:
+        case INTEL_OUTPUT_HDMI:
+        case INTEL_OUTPUT_UNKNOWN:
+                return enc_to_dig_port(&encoder->base)->port;
+        case INTEL_OUTPUT_ANALOG:
+                return intel_ddi_get_analog_port();
+        default:
+                MISSING_CASE(encoder->type);
+                return PORT_A;
+        }
+}
+
 void intel_connector_unregister(struct drm_connector *connector)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b1fc67e..19aab7b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1111,7 +1111,12 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
 void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder);
 void hsw_fdi_link_train(struct drm_crtc *crtc);
 void intel_ddi_init(struct drm_device *dev, enum port port);
-enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
+
+static inline enum port intel_ddi_get_analog_port(void)
+{
+	return PORT_A;
+}
+
 bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
 void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc);
 void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
@@ -1189,6 +1194,7 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
 		 (1 << INTEL_OUTPUT_DP_MST) |
 		 (1 << INTEL_OUTPUT_EDP));
 }
+enum port intel_get_encoder_port(struct intel_encoder *encoder);
 static inline void
 intel_wait_for_vblank(struct drm_device *dev, int pipe)
 {
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index adca262..9c0043e 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -366,7 +366,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 	if (intel_encoder->type == INTEL_OUTPUT_DSI)
 		port = 0;
 	else
-		port = intel_ddi_get_encoder_port(intel_encoder);
+		port = intel_get_encoder_port(intel_encoder);
 
 	if (port == PORT_E)  {
 		port = 0;
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] drm/i915: Move audio_connector to intel_encoder
  2016-08-16  0:00 [PATCH v2 0/3] Prep. for DP audio MST support Dhinakaran Pandiyan
  2016-08-16  0:00 ` [PATCH v2 1/3] drm/i915: Add function to return port from an encoder Dhinakaran Pandiyan
@ 2016-08-16  0:00 ` Dhinakaran Pandiyan
  2016-08-16  0:00 ` [PATCH v2 3/3] drm/i915: start adding dp mst audio Dhinakaran Pandiyan
  2016-08-16  6:25 ` ✗ Ro.CI.BAT: failure for Prep. for DP audio MST support Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Dhinakaran Pandiyan @ 2016-08-16  0:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: libin.yang, Dhinakaran Pandiyan

With DP MST, a digital_port can carry more than one audio stream. Hence,
more than one audio_connector needs to be attached to intel_digital_port in
such cases. However, each stream is associated with an unique encoder. So,
instead of creating an array of audio_connectors per port, move
audio_connector from struct intel_digital_port to struct intel_encoder.
This also simplifies access to the right audio_connector from codec
functions in intel_audio.c that receive intel_encoder.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 10 ++++------
 drivers/gpu/drm/i915/intel_drv.h   |  4 ++--
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index d32f586..ef20875 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -523,7 +523,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
 						     adjusted_mode);
 
 	mutex_lock(&dev_priv->av_mutex);
-	intel_dig_port->audio_connector = connector;
+	intel_encoder->audio_connector = connector;
 	/* referred in audio callbacks */
 	dev_priv->dig_port_map[port] = intel_encoder;
 	mutex_unlock(&dev_priv->av_mutex);
@@ -552,7 +552,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 		dev_priv->display.audio_codec_disable(intel_encoder);
 
 	mutex_lock(&dev_priv->av_mutex);
-	intel_dig_port->audio_connector = NULL;
+	intel_encoder->audio_connector = NULL;
 	dev_priv->dig_port_map[port] = NULL;
 	mutex_unlock(&dev_priv->av_mutex);
 
@@ -713,7 +713,6 @@ static int i915_audio_component_get_eld(struct device *dev, int port,
 {
 	struct drm_i915_private *dev_priv = dev_to_i915(dev);
 	struct intel_encoder *intel_encoder;
-	struct intel_digital_port *intel_dig_port;
 	const u8 *eld;
 	int ret = -EINVAL;
 
@@ -722,10 +721,9 @@ static int i915_audio_component_get_eld(struct device *dev, int port,
 	/* intel_encoder might be NULL for DP MST */
 	if (intel_encoder) {
 		ret = 0;
-		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
-		*enabled = intel_dig_port->audio_connector != NULL;
+		*enabled = intel_encoder->audio_connector != NULL;
 		if (*enabled) {
-			eld = intel_dig_port->audio_connector->eld;
+			eld = intel_encoder->audio_connector->eld;
 			ret = drm_eld_size(eld);
 			memcpy(buf, eld, min(max_bytes, ret));
 		}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 19aab7b..c20fb49 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -220,6 +220,8 @@ struct intel_encoder {
 	void (*suspend)(struct intel_encoder *);
 	int crtc_mask;
 	enum hpd_pin hpd_pin;
+	/* for communication with audio component; protected by av_mutex */
+	const struct drm_connector *audio_connector;
 };
 
 struct intel_panel {
@@ -930,8 +932,6 @@ struct intel_digital_port {
 	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
 	bool release_cl2_override;
 	uint8_t max_lanes;
-	/* for communication with audio component; protected by av_mutex */
-	const struct drm_connector *audio_connector;
 };
 
 struct intel_dp_mst_encoder {
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] drm/i915: start adding dp mst audio
  2016-08-16  0:00 [PATCH v2 0/3] Prep. for DP audio MST support Dhinakaran Pandiyan
  2016-08-16  0:00 ` [PATCH v2 1/3] drm/i915: Add function to return port from an encoder Dhinakaran Pandiyan
  2016-08-16  0:00 ` [PATCH v2 2/3] drm/i915: Move audio_connector to intel_encoder Dhinakaran Pandiyan
@ 2016-08-16  0:00 ` Dhinakaran Pandiyan
  2016-08-16  6:25 ` ✗ Ro.CI.BAT: failure for Prep. for DP audio MST support Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Dhinakaran Pandiyan @ 2016-08-16  0:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: libin.yang

From: Libin Yang <libin.yang@linux.intel.com>

(This patch is developed by Dave Airlie <airlied@redhat.com> originally)

This patch adds support for DP MST audio in i915.

Enable audio codec when DP MST is enabled if has_audio flag is set.
Disable audio codec when DP MST is disabled if has_audio flag is set.

Another separated patches to support DP MST audio will be implemented
in audio driver.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Reviewed-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++++++++++++-
 drivers/gpu/drm/i915/intel_ddi.c    | 20 +++++++++++++++-----
 drivers/gpu/drm/i915/intel_dp_mst.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h    |  2 ++
 4 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9bd4158..31c5e38 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2973,6 +2973,20 @@ static void intel_dp_info(struct seq_file *m,
 		intel_panel_info(m, &intel_connector->panel);
 }
 
+static void intel_dp_mst_info(struct seq_file *m,
+			  struct intel_connector *intel_connector)
+{
+	struct intel_encoder *intel_encoder = intel_connector->encoder;
+	struct intel_dp_mst_encoder *intel_mst =
+		enc_to_mst(&intel_encoder->base);
+	struct intel_digital_port *intel_dig_port = intel_mst->primary;
+	struct intel_dp *intel_dp = &intel_dig_port->dp;
+	bool has_audio = drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
+					intel_connector->port);
+
+	seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
+}
+
 static void intel_hdmi_info(struct seq_file *m,
 			    struct intel_connector *intel_connector)
 {
@@ -3015,7 +3029,10 @@ static void intel_connector_info(struct seq_file *m,
 	switch (connector->connector_type) {
 	case DRM_MODE_CONNECTOR_DisplayPort:
 	case DRM_MODE_CONNECTOR_eDP:
-		intel_dp_info(m, intel_connector);
+		if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
+			intel_dp_mst_info(m, intel_connector);
+		else
+			intel_dp_info(m, intel_connector);
 		break;
 	case DRM_MODE_CONNECTOR_LVDS:
 		if (intel_encoder->type == INTEL_OUTPUT_LVDS)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 13ceef4..0ce42cb 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2155,6 +2155,19 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc)
 	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
 }
 
+bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
+				 struct intel_crtc *intel_crtc)
+{
+	u32 temp;
+
+	if (intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
+		temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+		if (temp & AUDIO_OUTPUT_ENABLE(intel_crtc->pipe))
+			return true;
+	}
+	return false;
+}
+
 void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config)
 {
@@ -2220,11 +2233,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		break;
 	}
 
-	if (intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
-		temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
-		if (temp & AUDIO_OUTPUT_ENABLE(intel_crtc->pipe))
-			pipe_config->has_audio = true;
-	}
+	pipe_config->has_audio =
+		intel_ddi_is_audio_enabled(dev_priv, intel_crtc);
 
 	if (encoder->type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.bpp &&
 	    pipe_config->pipe_bpp > dev_priv->vbt.edp.bpp) {
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 629337d..c8b036b 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -76,6 +76,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 		return false;
 	}
 
+	if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, found->port))
+		pipe_config->has_audio = true;
 	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
 
 	pipe_config->pbn = mst_pbn;
@@ -97,6 +99,10 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = encoder->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int ret;
 
 	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
@@ -107,6 +113,10 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
 	if (ret) {
 		DRM_ERROR("failed to update payload %d\n", ret);
 	}
+	if (intel_crtc->config->has_audio) {
+		intel_audio_codec_disable(encoder);
+		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
+	}
 }
 
 static void intel_mst_post_disable_dp(struct intel_encoder *encoder)
@@ -207,7 +217,9 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder)
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	enum port port = intel_dig_port->port;
+	enum pipe pipe = crtc->pipe;
 	int ret;
 
 	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
@@ -222,6 +234,12 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder)
 	ret = drm_dp_check_act_status(&intel_dp->mst_mgr);
 
 	ret = drm_dp_update_payload_part2(&intel_dp->mst_mgr);
+	if (crtc->config->has_audio) {
+		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
+				 pipe_name(pipe));
+		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+		intel_audio_codec_enable(encoder);
+	}
 }
 
 static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
@@ -245,6 +263,9 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
 	u32 temp, flags = 0;
 
+	pipe_config->has_audio =
+		intel_ddi_is_audio_enabled(dev_priv, crtc);
+
 	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if (temp & TRANS_DDI_PHSYNC)
 		flags |= DRM_MODE_FLAG_PHSYNC;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c20fb49..96af4e8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1129,6 +1129,8 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
 bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
 void intel_ddi_fdi_disable(struct drm_crtc *crtc);
+bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
+				 struct intel_crtc *intel_crtc);
 void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config);
 struct intel_encoder *
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ Ro.CI.BAT: failure for Prep. for DP audio MST support
  2016-08-16  0:00 [PATCH v2 0/3] Prep. for DP audio MST support Dhinakaran Pandiyan
                   ` (2 preceding siblings ...)
  2016-08-16  0:00 ` [PATCH v2 3/3] drm/i915: start adding dp mst audio Dhinakaran Pandiyan
@ 2016-08-16  6:25 ` Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2016-08-16  6:25 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx

== Series Details ==

Series: Prep. for DP audio MST support
URL   : https://patchwork.freedesktop.org/series/11129/
State : failure

== Summary ==

Series 11129v1 Prep. for DP audio MST support
http://patchwork.freedesktop.org/api/1.0/series/11129/revisions/1/mbox

Test kms_cursor_legacy:
        Subgroup basic-flip-vs-cursor-legacy:
                fail       -> PASS       (ro-skl3-i5-6260u)
        Subgroup basic-flip-vs-cursor-varying-size:
                fail       -> PASS       (ro-byt-n2820)
                pass       -> FAIL       (ro-skl3-i5-6260u)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (ro-bdw-i7-5600u)
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-kbl-qkkr      total:244  pass:186  dwarn:29  dfail:0   fail:3   skip:26 
ro-bdw-i5-5250u  total:240  pass:219  dwarn:2   dfail:0   fail:1   skip:18 
ro-bdw-i7-5600u  total:240  pass:206  dwarn:1   dfail:0   fail:1   skip:32 
ro-bsw-n3050     total:240  pass:195  dwarn:0   dfail:0   fail:3   skip:42 
ro-byt-n2820     total:240  pass:198  dwarn:0   dfail:0   fail:2   skip:40 
ro-hsw-i3-4010u  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26 
ro-hsw-i7-4770r  total:240  pass:185  dwarn:0   dfail:0   fail:0   skip:55 
ro-ilk1-i5-650   total:235  pass:174  dwarn:0   dfail:0   fail:1   skip:60 
ro-ivb-i7-3770   total:240  pass:205  dwarn:0   dfail:0   fail:0   skip:35 
ro-ivb2-i7-3770  total:240  pass:209  dwarn:0   dfail:0   fail:0   skip:31 
ro-skl3-i5-6260u total:240  pass:223  dwarn:0   dfail:0   fail:3   skip:14 
fi-skl-i7-6700k failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1881/

bc5705c drm-intel-nightly: 2016y-08m-15d-15h-16m-01s UTC integration manifest
ddcb6c2 drm/i915: start adding dp mst audio
882db3f drm/i915: Move audio_connector to intel_encoder
687ade4 drm/i915: Add function to return port from an encoder

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Add function to return port from an encoder
  2016-08-16  0:00 ` [PATCH v2 1/3] drm/i915: Add function to return port from an encoder Dhinakaran Pandiyan
@ 2016-08-19  4:39   ` Rodrigo Vivi
  2016-08-19 16:57     ` Pandiyan, Dhinakaran
  2016-08-19  8:02   ` Daniel Vetter
  1 sibling, 1 reply; 10+ messages in thread
From: Rodrigo Vivi @ 2016-08-19  4:39 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, libin.yang

On Mon, Aug 15, 2016 at 05:00:53PM -0700, Dhinakaran Pandiyan wrote:
> There are places in the driver where we just need the 'port' associated
> with an encoder and not 'struct intel_digital_port' that contains it.
> This basically is a generic implementation of intel_ddi_get_encoder_port()
> handling both DDI and Non-DDI encoders. The handling of the analog encoder
> is delegated to the DDI specific function.
> 
> The idea to have a generic implementation that returned the 'enum port'
> from 'struct intel_encoder' came from Ville.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c      | 42 ++++++++++-------------------------
>  drivers/gpu/drm/i915/intel_display.c  | 18 +++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h      |  8 ++++++-
>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
>  4 files changed, 38 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index c2df4e4..13ceef4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -301,24 +301,6 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
>  	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
>  };
>  
> -enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> -{
> -	switch (encoder->type) {
> -	case INTEL_OUTPUT_DP_MST:
> -		return enc_to_mst(&encoder->base)->primary->port;
> -	case INTEL_OUTPUT_DP:
> -	case INTEL_OUTPUT_EDP:
> -	case INTEL_OUTPUT_HDMI:
> -	case INTEL_OUTPUT_UNKNOWN:
> -		return enc_to_dig_port(&encoder->base)->port;
> -	case INTEL_OUTPUT_ANALOG:
> -		return PORT_E;
> -	default:
> -		MISSING_CASE(encoder->type);
> -		return PORT_A;
> -	}
> -}
> -
>  static const struct ddi_buf_trans *
>  bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> @@ -421,7 +403,7 @@ void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	u32 iboost_bit = 0;
>  	int i, n_dp_entries, n_edp_entries, size;
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  	const struct ddi_buf_trans *ddi_translations_fdi;
>  	const struct ddi_buf_trans *ddi_translations_dp;
>  	const struct ddi_buf_trans *ddi_translations_edp;
> @@ -499,7 +481,7 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	u32 iboost_bit = 0;
>  	int n_hdmi_entries, hdmi_level;
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  	const struct ddi_buf_trans *ddi_translations_hdmi;
>  
>  	if (IS_BROXTON(dev_priv))
> @@ -987,7 +969,7 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
>  				struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  	uint32_t dpll = port;
>  
>  	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
> @@ -1130,7 +1112,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	enum pipe pipe = intel_crtc->pipe;
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	int type = intel_encoder->type;
>  	uint32_t temp;
>  
> @@ -1226,7 +1208,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_encoder *intel_encoder = intel_connector->encoder;
>  	int type = intel_connector->base.connector_type;
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	enum pipe pipe = 0;
>  	enum transcoder cpu_transcoder;
>  	enum intel_display_power_domain power_domain;
> @@ -1286,7 +1268,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  {
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  	enum intel_display_power_domain power_domain;
>  	u32 tmp;
>  	int i;
> @@ -1361,7 +1343,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  
>  	if (cpu_transcoder != TRANSCODER_EDP)
> @@ -1589,7 +1571,7 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
>  			  const struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  
>  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>  		uint32_t dpll = pipe_config->ddi_pll_sel;
> @@ -1616,7 +1598,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	int type = intel_encoder->type;
>  
>  	if (type == INTEL_OUTPUT_HDMI) {
> @@ -1668,7 +1650,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	int type = intel_encoder->type;
>  	uint32_t val;
>  	bool wait = false;
> @@ -1715,7 +1697,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	int type = intel_encoder->type;
>  
>  	if (type == INTEL_OUTPUT_HDMI) {
> @@ -2276,7 +2258,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	int type = encoder->type;
> -	int port = intel_ddi_get_encoder_port(encoder);
> +	int port = intel_get_encoder_port(encoder);
>  	int ret;
>  
>  	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9cbf543..44d20bd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -16277,6 +16277,24 @@ err:
>  	return ret;
>  }
>  
> +enum port intel_get_encoder_port(struct intel_encoder *encoder)
> +{
> +        switch (encoder->type) {
> +        case INTEL_OUTPUT_DP_MST:
> +                return enc_to_mst(&encoder->base)->primary->port;
> +        case INTEL_OUTPUT_DP:
> +        case INTEL_OUTPUT_EDP:
> +        case INTEL_OUTPUT_HDMI:
> +        case INTEL_OUTPUT_UNKNOWN:
> +                return enc_to_dig_port(&encoder->base)->port;
> +        case INTEL_OUTPUT_ANALOG:
> +                return intel_ddi_get_analog_port();
> +        default:
> +                MISSING_CASE(encoder->type);
> +                return PORT_A;
> +        }
> +}

I don't see the difference here. For me this patch looks more a rename and
re-positioning than a change on the enum port x intel_digital_port as advertised.
What am I missing?


> +
>  void intel_connector_unregister(struct drm_connector *connector)
>  {
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b1fc67e..19aab7b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1111,7 +1111,12 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
>  void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder);
>  void hsw_fdi_link_train(struct drm_crtc *crtc);
>  void intel_ddi_init(struct drm_device *dev, enum port port);
> -enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
> +
> +static inline enum port intel_ddi_get_analog_port(void)
> +{
> +	return PORT_A;

PORT_E?

Anyway unless this is used in more places I think I prefer the old way.

> +}
> +
>  bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
>  void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc);
>  void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
> @@ -1189,6 +1194,7 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
>  		 (1 << INTEL_OUTPUT_DP_MST) |
>  		 (1 << INTEL_OUTPUT_EDP));
>  }
> +enum port intel_get_encoder_port(struct intel_encoder *encoder);
>  static inline void
>  intel_wait_for_vblank(struct drm_device *dev, int pipe)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index adca262..9c0043e 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -366,7 +366,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>  	if (intel_encoder->type == INTEL_OUTPUT_DSI)
>  		port = 0;
>  	else
> -		port = intel_ddi_get_encoder_port(intel_encoder);
> +		port = intel_get_encoder_port(intel_encoder);
>  
>  	if (port == PORT_E)  {
>  		port = 0;
> -- 
> 2.5.0
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Add function to return port from an encoder
  2016-08-16  0:00 ` [PATCH v2 1/3] drm/i915: Add function to return port from an encoder Dhinakaran Pandiyan
  2016-08-19  4:39   ` Rodrigo Vivi
@ 2016-08-19  8:02   ` Daniel Vetter
  2016-08-19 18:03     ` Pandiyan, Dhinakaran
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2016-08-19  8:02 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, libin.yang

On Mon, Aug 15, 2016 at 05:00:53PM -0700, Dhinakaran Pandiyan wrote:
> There are places in the driver where we just need the 'port' associated
> with an encoder and not 'struct intel_digital_port' that contains it.
> This basically is a generic implementation of intel_ddi_get_encoder_port()
> handling both DDI and Non-DDI encoders. The handling of the analog encoder
> is delegated to the DDI specific function.
> 
> The idea to have a generic implementation that returned the 'enum port'
> from 'struct intel_encoder' came from Ville.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c      | 42 ++++++++++-------------------------
>  drivers/gpu/drm/i915/intel_display.c  | 18 +++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h      |  8 ++++++-
>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
>  4 files changed, 38 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index c2df4e4..13ceef4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -301,24 +301,6 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
>  	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
>  };
>  
> -enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> -{
> -	switch (encoder->type) {
> -	case INTEL_OUTPUT_DP_MST:
> -		return enc_to_mst(&encoder->base)->primary->port;
> -	case INTEL_OUTPUT_DP:
> -	case INTEL_OUTPUT_EDP:
> -	case INTEL_OUTPUT_HDMI:
> -	case INTEL_OUTPUT_UNKNOWN:
> -		return enc_to_dig_port(&encoder->base)->port;
> -	case INTEL_OUTPUT_ANALOG:
> -		return PORT_E;
> -	default:
> -		MISSING_CASE(encoder->type);
> -		return PORT_A;
> -	}
> -}

tbh if we really need this in general, I'd just store the port enum in
intel_encoder and that's it. Maybe add a PORT_NONE = -1 for all the cases
where it's not a port. A bit more churn to roll it tou, but this maze of
indirections and casting here is really not pretty.

The give-away is that we have this massive type switch block, for
something which is designed after OO priniciples: The correct way to fix
that is by moving stuff up in the inheritence hirarchy.
-Daniel

> -
>  static const struct ddi_buf_trans *
>  bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> @@ -421,7 +403,7 @@ void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	u32 iboost_bit = 0;
>  	int i, n_dp_entries, n_edp_entries, size;
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  	const struct ddi_buf_trans *ddi_translations_fdi;
>  	const struct ddi_buf_trans *ddi_translations_dp;
>  	const struct ddi_buf_trans *ddi_translations_edp;
> @@ -499,7 +481,7 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	u32 iboost_bit = 0;
>  	int n_hdmi_entries, hdmi_level;
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  	const struct ddi_buf_trans *ddi_translations_hdmi;
>  
>  	if (IS_BROXTON(dev_priv))
> @@ -987,7 +969,7 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
>  				struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  	uint32_t dpll = port;
>  
>  	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
> @@ -1130,7 +1112,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	enum pipe pipe = intel_crtc->pipe;
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	int type = intel_encoder->type;
>  	uint32_t temp;
>  
> @@ -1226,7 +1208,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_encoder *intel_encoder = intel_connector->encoder;
>  	int type = intel_connector->base.connector_type;
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	enum pipe pipe = 0;
>  	enum transcoder cpu_transcoder;
>  	enum intel_display_power_domain power_domain;
> @@ -1286,7 +1268,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  {
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  	enum intel_display_power_domain power_domain;
>  	u32 tmp;
>  	int i;
> @@ -1361,7 +1343,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  
>  	if (cpu_transcoder != TRANSCODER_EDP)
> @@ -1589,7 +1571,7 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
>  			  const struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	enum port port = intel_ddi_get_encoder_port(encoder);
> +	enum port port = intel_get_encoder_port(encoder);
>  
>  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>  		uint32_t dpll = pipe_config->ddi_pll_sel;
> @@ -1616,7 +1598,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	int type = intel_encoder->type;
>  
>  	if (type == INTEL_OUTPUT_HDMI) {
> @@ -1668,7 +1650,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	int type = intel_encoder->type;
>  	uint32_t val;
>  	bool wait = false;
> @@ -1715,7 +1697,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	enum port port = intel_get_encoder_port(intel_encoder);
>  	int type = intel_encoder->type;
>  
>  	if (type == INTEL_OUTPUT_HDMI) {
> @@ -2276,7 +2258,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	int type = encoder->type;
> -	int port = intel_ddi_get_encoder_port(encoder);
> +	int port = intel_get_encoder_port(encoder);
>  	int ret;
>  
>  	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9cbf543..44d20bd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -16277,6 +16277,24 @@ err:
>  	return ret;
>  }
>  
> +enum port intel_get_encoder_port(struct intel_encoder *encoder)
> +{
> +        switch (encoder->type) {
> +        case INTEL_OUTPUT_DP_MST:
> +                return enc_to_mst(&encoder->base)->primary->port;
> +        case INTEL_OUTPUT_DP:
> +        case INTEL_OUTPUT_EDP:
> +        case INTEL_OUTPUT_HDMI:
> +        case INTEL_OUTPUT_UNKNOWN:
> +                return enc_to_dig_port(&encoder->base)->port;
> +        case INTEL_OUTPUT_ANALOG:
> +                return intel_ddi_get_analog_port();
> +        default:
> +                MISSING_CASE(encoder->type);
> +                return PORT_A;
> +        }
> +}
> +
>  void intel_connector_unregister(struct drm_connector *connector)
>  {
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b1fc67e..19aab7b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1111,7 +1111,12 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
>  void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder);
>  void hsw_fdi_link_train(struct drm_crtc *crtc);
>  void intel_ddi_init(struct drm_device *dev, enum port port);
> -enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
> +
> +static inline enum port intel_ddi_get_analog_port(void)
> +{
> +	return PORT_A;
> +}
> +
>  bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
>  void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc);
>  void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
> @@ -1189,6 +1194,7 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
>  		 (1 << INTEL_OUTPUT_DP_MST) |
>  		 (1 << INTEL_OUTPUT_EDP));
>  }
> +enum port intel_get_encoder_port(struct intel_encoder *encoder);
>  static inline void
>  intel_wait_for_vblank(struct drm_device *dev, int pipe)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index adca262..9c0043e 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -366,7 +366,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>  	if (intel_encoder->type == INTEL_OUTPUT_DSI)
>  		port = 0;
>  	else
> -		port = intel_ddi_get_encoder_port(intel_encoder);
> +		port = intel_get_encoder_port(intel_encoder);
>  
>  	if (port == PORT_E)  {
>  		port = 0;
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Add function to return port from an encoder
  2016-08-19  4:39   ` Rodrigo Vivi
@ 2016-08-19 16:57     ` Pandiyan, Dhinakaran
  0 siblings, 0 replies; 10+ messages in thread
From: Pandiyan, Dhinakaran @ 2016-08-19 16:57 UTC (permalink / raw)
  To: rodrigo.vivi; +Cc: intel-gfx, libin.yang

On Thu, 2016-08-18 at 21:39 -0700, Rodrigo Vivi wrote:
> On Mon, Aug 15, 2016 at 05:00:53PM -0700, Dhinakaran Pandiyan wrote:
> > There are places in the driver where we just need the 'port' associated
> > with an encoder and not 'struct intel_digital_port' that contains it.
> > This basically is a generic implementation of intel_ddi_get_encoder_port()
> > handling both DDI and Non-DDI encoders. The handling of the analog encoder
> > is delegated to the DDI specific function.
> > 
> > The idea to have a generic implementation that returned the 'enum port'
> > from 'struct intel_encoder' came from Ville.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c      | 42 ++++++++++-------------------------
> >  drivers/gpu/drm/i915/intel_display.c  | 18 +++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h      |  8 ++++++-
> >  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
> >  4 files changed, 38 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index c2df4e4..13ceef4 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -301,24 +301,6 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
> >  	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
> >  };
> >  
> > -enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> > -{
> > -	switch (encoder->type) {
> > -	case INTEL_OUTPUT_DP_MST:
> > -		return enc_to_mst(&encoder->base)->primary->port;
> > -	case INTEL_OUTPUT_DP:
> > -	case INTEL_OUTPUT_EDP:
> > -	case INTEL_OUTPUT_HDMI:
> > -	case INTEL_OUTPUT_UNKNOWN:
> > -		return enc_to_dig_port(&encoder->base)->port;
> > -	case INTEL_OUTPUT_ANALOG:
> > -		return PORT_E;
> > -	default:
> > -		MISSING_CASE(encoder->type);
> > -		return PORT_A;
> > -	}
> > -}
> > -
> >  static const struct ddi_buf_trans *
> >  bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> >  {
> > @@ -421,7 +403,7 @@ void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	u32 iboost_bit = 0;
> >  	int i, n_dp_entries, n_edp_entries, size;
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  	const struct ddi_buf_trans *ddi_translations_fdi;
> >  	const struct ddi_buf_trans *ddi_translations_dp;
> >  	const struct ddi_buf_trans *ddi_translations_edp;
> > @@ -499,7 +481,7 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	u32 iboost_bit = 0;
> >  	int n_hdmi_entries, hdmi_level;
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  	const struct ddi_buf_trans *ddi_translations_hdmi;
> >  
> >  	if (IS_BROXTON(dev_priv))
> > @@ -987,7 +969,7 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
> >  				struct intel_crtc_state *pipe_config)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  	uint32_t dpll = port;
> >  
> >  	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
> > @@ -1130,7 +1112,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  	uint32_t temp;
> >  
> > @@ -1226,7 +1208,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct intel_encoder *intel_encoder = intel_connector->encoder;
> >  	int type = intel_connector->base.connector_type;
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	enum pipe pipe = 0;
> >  	enum transcoder cpu_transcoder;
> >  	enum intel_display_power_domain power_domain;
> > @@ -1286,7 +1268,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> >  {
> >  	struct drm_device *dev = encoder->base.dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  	enum intel_display_power_domain power_domain;
> >  	u32 tmp;
> >  	int i;
> > @@ -1361,7 +1343,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> >  
> >  	if (cpu_transcoder != TRANSCODER_EDP)
> > @@ -1589,7 +1571,7 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
> >  			  const struct intel_crtc_state *pipe_config)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  
> >  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> >  		uint32_t dpll = pipe_config->ddi_pll_sel;
> > @@ -1616,7 +1598,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> >  	struct drm_encoder *encoder = &intel_encoder->base;
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >  	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  
> >  	if (type == INTEL_OUTPUT_HDMI) {
> > @@ -1668,7 +1650,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> >  	struct drm_encoder *encoder = &intel_encoder->base;
> >  	struct drm_device *dev = encoder->dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  	uint32_t val;
> >  	bool wait = false;
> > @@ -1715,7 +1697,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  	struct drm_device *dev = encoder->dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  
> >  	if (type == INTEL_OUTPUT_HDMI) {
> > @@ -2276,7 +2258,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	int type = encoder->type;
> > -	int port = intel_ddi_get_encoder_port(encoder);
> > +	int port = intel_get_encoder_port(encoder);
> >  	int ret;
> >  
> >  	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9cbf543..44d20bd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -16277,6 +16277,24 @@ err:
> >  	return ret;
> >  }
> >  
> > +enum port intel_get_encoder_port(struct intel_encoder *encoder)
> > +{
> > +        switch (encoder->type) {
> > +        case INTEL_OUTPUT_DP_MST:
> > +                return enc_to_mst(&encoder->base)->primary->port;
> > +        case INTEL_OUTPUT_DP:
> > +        case INTEL_OUTPUT_EDP:
> > +        case INTEL_OUTPUT_HDMI:
> > +        case INTEL_OUTPUT_UNKNOWN:
> > +                return enc_to_dig_port(&encoder->base)->port;
> > +        case INTEL_OUTPUT_ANALOG:
> > +                return intel_ddi_get_analog_port();
> > +        default:
> > +                MISSING_CASE(encoder->type);
> > +                return PORT_A;
> > +        }
> > +}
> 
> I don't see the difference here. For me this patch looks more a rename and
> re-positioning than a change on the enum port x intel_digital_port as advertised.
> What am I missing?
> 
> 

The original implementation was to append MST handling to
enc_to_dig_port(). But, we decided not to mess with that function as we
only cared about the member intel_digital_port.port.This solution is to
re-use intel_ddi_get_encoder_port() functionality by making it generic.

> > +
> >  void intel_connector_unregister(struct drm_connector *connector)
> >  {
> >  	struct intel_connector *intel_connector = to_intel_connector(connector);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index b1fc67e..19aab7b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1111,7 +1111,12 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
> >  void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder);
> >  void hsw_fdi_link_train(struct drm_crtc *crtc);
> >  void intel_ddi_init(struct drm_device *dev, enum port port);
> > -enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
> > +
> > +static inline enum port intel_ddi_get_analog_port(void)
> > +{
> > +	return PORT_A;
> 
> PORT_E?

Thanks for spotting, I have to fix this.

> 
> Anyway unless this is used in more places I think I prefer the old way.

Several functions in intel_audio.c, where we need the port from encoder,
will make use of this. We need a generic conversion function for all
encoder output types and not tied to DDI platforms.

> 
> > +}
> > +
> >  bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
> >  void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc);
> >  void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
> > @@ -1189,6 +1194,7 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
> >  		 (1 << INTEL_OUTPUT_DP_MST) |
> >  		 (1 << INTEL_OUTPUT_EDP));
> >  }
> > +enum port intel_get_encoder_port(struct intel_encoder *encoder);
> >  static inline void
> >  intel_wait_for_vblank(struct drm_device *dev, int pipe)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index adca262..9c0043e 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -366,7 +366,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> >  	if (intel_encoder->type == INTEL_OUTPUT_DSI)
> >  		port = 0;
> >  	else
> > -		port = intel_ddi_get_encoder_port(intel_encoder);
> > +		port = intel_get_encoder_port(intel_encoder);
> >  
> >  	if (port == PORT_E)  {
> >  		port = 0;
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > 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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Add function to return port from an encoder
  2016-08-19  8:02   ` Daniel Vetter
@ 2016-08-19 18:03     ` Pandiyan, Dhinakaran
  2016-08-22  7:51       ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Pandiyan, Dhinakaran @ 2016-08-19 18:03 UTC (permalink / raw)
  To: daniel; +Cc: intel-gfx, libin.yang

On Fri, 2016-08-19 at 10:02 +0200, Daniel Vetter wrote:
> On Mon, Aug 15, 2016 at 05:00:53PM -0700, Dhinakaran Pandiyan wrote:
> > There are places in the driver where we just need the 'port' associated
> > with an encoder and not 'struct intel_digital_port' that contains it.
> > This basically is a generic implementation of intel_ddi_get_encoder_port()
> > handling both DDI and Non-DDI encoders. The handling of the analog encoder
> > is delegated to the DDI specific function.
> > 
> > The idea to have a generic implementation that returned the 'enum port'
> > from 'struct intel_encoder' came from Ville.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c      | 42 ++++++++++-------------------------
> >  drivers/gpu/drm/i915/intel_display.c  | 18 +++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h      |  8 ++++++-
> >  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
> >  4 files changed, 38 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index c2df4e4..13ceef4 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -301,24 +301,6 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
> >  	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
> >  };
> >  
> > -enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> > -{
> > -	switch (encoder->type) {
> > -	case INTEL_OUTPUT_DP_MST:
> > -		return enc_to_mst(&encoder->base)->primary->port;
> > -	case INTEL_OUTPUT_DP:
> > -	case INTEL_OUTPUT_EDP:
> > -	case INTEL_OUTPUT_HDMI:
> > -	case INTEL_OUTPUT_UNKNOWN:
> > -		return enc_to_dig_port(&encoder->base)->port;
> > -	case INTEL_OUTPUT_ANALOG:
> > -		return PORT_E;
> > -	default:
> > -		MISSING_CASE(encoder->type);
> > -		return PORT_A;
> > -	}
> > -}
> 
> tbh if we really need this in general, I'd just store the port enum in
> intel_encoder and that's it. Maybe add a PORT_NONE = -1 for all the cases
> where it's not a port. A bit more churn to roll it tou, but this maze of
> indirections and casting here is really not pretty.
> 
A cursory check shows around 12 places (combined) in intel_ddi.c,
intel_audio.c and intel_hdmi.c where this can be useful.


Out of curiosity, what are the cases where we don't have a port? Before
hdmi/dp initialization? Or wireless display? 

> The give-away is that we have this massive type switch block, for
> something which is designed after OO priniciples: The correct way to fix
> that is by moving stuff up in the inheritence hirarchy.
> -Daniel
> 
I get your point. Something like this?

struct intel_encoder{
	...
	enum port attached_port;

}



> > -
> >  static const struct ddi_buf_trans *
> >  bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> >  {
> > @@ -421,7 +403,7 @@ void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	u32 iboost_bit = 0;
> >  	int i, n_dp_entries, n_edp_entries, size;
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  	const struct ddi_buf_trans *ddi_translations_fdi;
> >  	const struct ddi_buf_trans *ddi_translations_dp;
> >  	const struct ddi_buf_trans *ddi_translations_edp;
> > @@ -499,7 +481,7 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	u32 iboost_bit = 0;
> >  	int n_hdmi_entries, hdmi_level;
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  	const struct ddi_buf_trans *ddi_translations_hdmi;
> >  
> >  	if (IS_BROXTON(dev_priv))
> > @@ -987,7 +969,7 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
> >  				struct intel_crtc_state *pipe_config)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  	uint32_t dpll = port;
> >  
> >  	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
> > @@ -1130,7 +1112,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  	uint32_t temp;
> >  
> > @@ -1226,7 +1208,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct intel_encoder *intel_encoder = intel_connector->encoder;
> >  	int type = intel_connector->base.connector_type;
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	enum pipe pipe = 0;
> >  	enum transcoder cpu_transcoder;
> >  	enum intel_display_power_domain power_domain;
> > @@ -1286,7 +1268,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> >  {
> >  	struct drm_device *dev = encoder->base.dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  	enum intel_display_power_domain power_domain;
> >  	u32 tmp;
> >  	int i;
> > @@ -1361,7 +1343,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> >  
> >  	if (cpu_transcoder != TRANSCODER_EDP)
> > @@ -1589,7 +1571,7 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
> >  			  const struct intel_crtc_state *pipe_config)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	enum port port = intel_get_encoder_port(encoder);
> >  
> >  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> >  		uint32_t dpll = pipe_config->ddi_pll_sel;
> > @@ -1616,7 +1598,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> >  	struct drm_encoder *encoder = &intel_encoder->base;
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >  	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  
> >  	if (type == INTEL_OUTPUT_HDMI) {
> > @@ -1668,7 +1650,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> >  	struct drm_encoder *encoder = &intel_encoder->base;
> >  	struct drm_device *dev = encoder->dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  	uint32_t val;
> >  	bool wait = false;
> > @@ -1715,7 +1697,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  	struct drm_device *dev = encoder->dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	enum port port = intel_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  
> >  	if (type == INTEL_OUTPUT_HDMI) {
> > @@ -2276,7 +2258,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	int type = encoder->type;
> > -	int port = intel_ddi_get_encoder_port(encoder);
> > +	int port = intel_get_encoder_port(encoder);
> >  	int ret;
> >  
> >  	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9cbf543..44d20bd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -16277,6 +16277,24 @@ err:
> >  	return ret;
> >  }
> >  
> > +enum port intel_get_encoder_port(struct intel_encoder *encoder)
> > +{
> > +        switch (encoder->type) {
> > +        case INTEL_OUTPUT_DP_MST:
> > +                return enc_to_mst(&encoder->base)->primary->port;
> > +        case INTEL_OUTPUT_DP:
> > +        case INTEL_OUTPUT_EDP:
> > +        case INTEL_OUTPUT_HDMI:
> > +        case INTEL_OUTPUT_UNKNOWN:
> > +                return enc_to_dig_port(&encoder->base)->port;
> > +        case INTEL_OUTPUT_ANALOG:
> > +                return intel_ddi_get_analog_port();
> > +        default:
> > +                MISSING_CASE(encoder->type);
> > +                return PORT_A;
> > +        }
> > +}
> > +
> >  void intel_connector_unregister(struct drm_connector *connector)
> >  {
> >  	struct intel_connector *intel_connector = to_intel_connector(connector);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index b1fc67e..19aab7b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1111,7 +1111,12 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
> >  void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder);
> >  void hsw_fdi_link_train(struct drm_crtc *crtc);
> >  void intel_ddi_init(struct drm_device *dev, enum port port);
> > -enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
> > +
> > +static inline enum port intel_ddi_get_analog_port(void)
> > +{
> > +	return PORT_A;
> > +}
> > +
> >  bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
> >  void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc);
> >  void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
> > @@ -1189,6 +1194,7 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
> >  		 (1 << INTEL_OUTPUT_DP_MST) |
> >  		 (1 << INTEL_OUTPUT_EDP));
> >  }
> > +enum port intel_get_encoder_port(struct intel_encoder *encoder);
> >  static inline void
> >  intel_wait_for_vblank(struct drm_device *dev, int pipe)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index adca262..9c0043e 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -366,7 +366,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> >  	if (intel_encoder->type == INTEL_OUTPUT_DSI)
> >  		port = 0;
> >  	else
> > -		port = intel_ddi_get_encoder_port(intel_encoder);
> > +		port = intel_get_encoder_port(intel_encoder);
> >  
> >  	if (port == PORT_E)  {
> >  		port = 0;
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Add function to return port from an encoder
  2016-08-19 18:03     ` Pandiyan, Dhinakaran
@ 2016-08-22  7:51       ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-08-22  7:51 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: libin.yang, intel-gfx

On Fri, Aug 19, 2016 at 06:03:06PM +0000, Pandiyan, Dhinakaran wrote:
> On Fri, 2016-08-19 at 10:02 +0200, Daniel Vetter wrote:
> > On Mon, Aug 15, 2016 at 05:00:53PM -0700, Dhinakaran Pandiyan wrote:
> > > There are places in the driver where we just need the 'port' associated
> > > with an encoder and not 'struct intel_digital_port' that contains it.
> > > This basically is a generic implementation of intel_ddi_get_encoder_port()
> > > handling both DDI and Non-DDI encoders. The handling of the analog encoder
> > > is delegated to the DDI specific function.
> > > 
> > > The idea to have a generic implementation that returned the 'enum port'
> > > from 'struct intel_encoder' came from Ville.
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c      | 42 ++++++++++-------------------------
> > >  drivers/gpu/drm/i915/intel_display.c  | 18 +++++++++++++++
> > >  drivers/gpu/drm/i915/intel_drv.h      |  8 ++++++-
> > >  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
> > >  4 files changed, 38 insertions(+), 32 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index c2df4e4..13ceef4 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -301,24 +301,6 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
> > >  	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
> > >  };
> > >  
> > > -enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> > > -{
> > > -	switch (encoder->type) {
> > > -	case INTEL_OUTPUT_DP_MST:
> > > -		return enc_to_mst(&encoder->base)->primary->port;
> > > -	case INTEL_OUTPUT_DP:
> > > -	case INTEL_OUTPUT_EDP:
> > > -	case INTEL_OUTPUT_HDMI:
> > > -	case INTEL_OUTPUT_UNKNOWN:
> > > -		return enc_to_dig_port(&encoder->base)->port;
> > > -	case INTEL_OUTPUT_ANALOG:
> > > -		return PORT_E;
> > > -	default:
> > > -		MISSING_CASE(encoder->type);
> > > -		return PORT_A;
> > > -	}
> > > -}
> > 
> > tbh if we really need this in general, I'd just store the port enum in
> > intel_encoder and that's it. Maybe add a PORT_NONE = -1 for all the cases
> > where it's not a port. A bit more churn to roll it tou, but this maze of
> > indirections and casting here is really not pretty.
> > 
> A cursory check shows around 12 places (combined) in intel_ddi.c,
> intel_audio.c and intel_hdmi.c where this can be useful.
> 
> 
> Out of curiosity, what are the cases where we don't have a port? Before
> hdmi/dp initialization? Or wireless display? 

We shouldn't look at encoders before they're fully set up. And we don't
support wireless display. The PORT_NONE case is more for older
platforms/encoders like lvds, tv. (s)dvo does have the concept of
different ports.
> 
> > The give-away is that we have this massive type switch block, for
> > something which is designed after OO priniciples: The correct way to fix
> > that is by moving stuff up in the inheritence hirarchy.
> > -Daniel
> > 
> I get your point. Something like this?
> 
> struct intel_encoder{
> 	...
> 	enum port attached_port;
> 
> }

Yup.
-Daniel

> 
> 
> 
> > > -
> > >  static const struct ddi_buf_trans *
> > >  bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> > >  {
> > > @@ -421,7 +403,7 @@ void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >  	u32 iboost_bit = 0;
> > >  	int i, n_dp_entries, n_edp_entries, size;
> > > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > > +	enum port port = intel_get_encoder_port(encoder);
> > >  	const struct ddi_buf_trans *ddi_translations_fdi;
> > >  	const struct ddi_buf_trans *ddi_translations_dp;
> > >  	const struct ddi_buf_trans *ddi_translations_edp;
> > > @@ -499,7 +481,7 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >  	u32 iboost_bit = 0;
> > >  	int n_hdmi_entries, hdmi_level;
> > > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > > +	enum port port = intel_get_encoder_port(encoder);
> > >  	const struct ddi_buf_trans *ddi_translations_hdmi;
> > >  
> > >  	if (IS_BROXTON(dev_priv))
> > > @@ -987,7 +969,7 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
> > >  				struct intel_crtc_state *pipe_config)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > > +	enum port port = intel_get_encoder_port(encoder);
> > >  	uint32_t dpll = port;
> > >  
> > >  	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
> > > @@ -1130,7 +1112,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > >  	enum pipe pipe = intel_crtc->pipe;
> > >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> > > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > > +	enum port port = intel_get_encoder_port(intel_encoder);
> > >  	int type = intel_encoder->type;
> > >  	uint32_t temp;
> > >  
> > > @@ -1226,7 +1208,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > >  	struct intel_encoder *intel_encoder = intel_connector->encoder;
> > >  	int type = intel_connector->base.connector_type;
> > > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > > +	enum port port = intel_get_encoder_port(intel_encoder);
> > >  	enum pipe pipe = 0;
> > >  	enum transcoder cpu_transcoder;
> > >  	enum intel_display_power_domain power_domain;
> > > @@ -1286,7 +1268,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> > >  {
> > >  	struct drm_device *dev = encoder->base.dev;
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > > +	enum port port = intel_get_encoder_port(encoder);
> > >  	enum intel_display_power_domain power_domain;
> > >  	u32 tmp;
> > >  	int i;
> > > @@ -1361,7 +1343,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> > >  	struct drm_device *dev = crtc->dev;
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > >  	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> > > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > > +	enum port port = intel_get_encoder_port(intel_encoder);
> > >  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> > >  
> > >  	if (cpu_transcoder != TRANSCODER_EDP)
> > > @@ -1589,7 +1571,7 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
> > >  			  const struct intel_crtc_state *pipe_config)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > -	enum port port = intel_ddi_get_encoder_port(encoder);
> > > +	enum port port = intel_get_encoder_port(encoder);
> > >  
> > >  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> > >  		uint32_t dpll = pipe_config->ddi_pll_sel;
> > > @@ -1616,7 +1598,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> > >  	struct drm_encoder *encoder = &intel_encoder->base;
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> > >  	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> > > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > > +	enum port port = intel_get_encoder_port(intel_encoder);
> > >  	int type = intel_encoder->type;
> > >  
> > >  	if (type == INTEL_OUTPUT_HDMI) {
> > > @@ -1668,7 +1650,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> > >  	struct drm_encoder *encoder = &intel_encoder->base;
> > >  	struct drm_device *dev = encoder->dev;
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > > +	enum port port = intel_get_encoder_port(intel_encoder);
> > >  	int type = intel_encoder->type;
> > >  	uint32_t val;
> > >  	bool wait = false;
> > > @@ -1715,7 +1697,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
> > >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > >  	struct drm_device *dev = encoder->dev;
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > > -	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > > +	enum port port = intel_get_encoder_port(intel_encoder);
> > >  	int type = intel_encoder->type;
> > >  
> > >  	if (type == INTEL_OUTPUT_HDMI) {
> > > @@ -2276,7 +2258,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >  	int type = encoder->type;
> > > -	int port = intel_ddi_get_encoder_port(encoder);
> > > +	int port = intel_get_encoder_port(encoder);
> > >  	int ret;
> > >  
> > >  	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 9cbf543..44d20bd 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -16277,6 +16277,24 @@ err:
> > >  	return ret;
> > >  }
> > >  
> > > +enum port intel_get_encoder_port(struct intel_encoder *encoder)
> > > +{
> > > +        switch (encoder->type) {
> > > +        case INTEL_OUTPUT_DP_MST:
> > > +                return enc_to_mst(&encoder->base)->primary->port;
> > > +        case INTEL_OUTPUT_DP:
> > > +        case INTEL_OUTPUT_EDP:
> > > +        case INTEL_OUTPUT_HDMI:
> > > +        case INTEL_OUTPUT_UNKNOWN:
> > > +                return enc_to_dig_port(&encoder->base)->port;
> > > +        case INTEL_OUTPUT_ANALOG:
> > > +                return intel_ddi_get_analog_port();
> > > +        default:
> > > +                MISSING_CASE(encoder->type);
> > > +                return PORT_A;
> > > +        }
> > > +}
> > > +
> > >  void intel_connector_unregister(struct drm_connector *connector)
> > >  {
> > >  	struct intel_connector *intel_connector = to_intel_connector(connector);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index b1fc67e..19aab7b 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1111,7 +1111,12 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
> > >  void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder);
> > >  void hsw_fdi_link_train(struct drm_crtc *crtc);
> > >  void intel_ddi_init(struct drm_device *dev, enum port port);
> > > -enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
> > > +
> > > +static inline enum port intel_ddi_get_analog_port(void)
> > > +{
> > > +	return PORT_A;
> > > +}
> > > +
> > >  bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
> > >  void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc);
> > >  void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
> > > @@ -1189,6 +1194,7 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
> > >  		 (1 << INTEL_OUTPUT_DP_MST) |
> > >  		 (1 << INTEL_OUTPUT_EDP));
> > >  }
> > > +enum port intel_get_encoder_port(struct intel_encoder *encoder);
> > >  static inline void
> > >  intel_wait_for_vblank(struct drm_device *dev, int pipe)
> > >  {
> > > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > > index adca262..9c0043e 100644
> > > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > > @@ -366,7 +366,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> > >  	if (intel_encoder->type == INTEL_OUTPUT_DSI)
> > >  		port = 0;
> > >  	else
> > > -		port = intel_ddi_get_encoder_port(intel_encoder);
> > > +		port = intel_get_encoder_port(intel_encoder);
> > >  
> > >  	if (port == PORT_E)  {
> > >  		port = 0;
> > > -- 
> > > 2.5.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-08-22  7:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-16  0:00 [PATCH v2 0/3] Prep. for DP audio MST support Dhinakaran Pandiyan
2016-08-16  0:00 ` [PATCH v2 1/3] drm/i915: Add function to return port from an encoder Dhinakaran Pandiyan
2016-08-19  4:39   ` Rodrigo Vivi
2016-08-19 16:57     ` Pandiyan, Dhinakaran
2016-08-19  8:02   ` Daniel Vetter
2016-08-19 18:03     ` Pandiyan, Dhinakaran
2016-08-22  7:51       ` Daniel Vetter
2016-08-16  0:00 ` [PATCH v2 2/3] drm/i915: Move audio_connector to intel_encoder Dhinakaran Pandiyan
2016-08-16  0:00 ` [PATCH v2 3/3] drm/i915: start adding dp mst audio Dhinakaran Pandiyan
2016-08-16  6:25 ` ✗ Ro.CI.BAT: failure for Prep. for DP audio MST support Patchwork

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.