[PATCH v2 0/7] YCBCR 4:2:0 support for LSPCON

[PATCH v2 0/7] YCBCR 4:2:0 support for LSPCON

[Shashank Sharma]

LSPCON is a DP->HDMI active dongle, enumerated as DP dual
mode adapter on Intel GEN9 platforms. It's provided by two
different vendors
        - Mega Chips America (MCA)
        - Parade Technologies (Parade)

In order to support YCBCR 4:2:0 outputs, these are the essential
steps:
        - Convert HDMI output format from RGB to YCBCR 4:4:4.
        - Set appropriate color space in AVI infoframes.
        - Write AVI infoframes to vendor specific AUX registers.

This patch series adds 7 patches, to accommodate above steps and
enable YCBCR 4:2:0 output for LSPCON based HDMI displays. First
4 patches are from Ville Syrjälä's patch series, which makes
infoframe functions available for DDI displays, published here:
https://patchwork.freedesktop.org/series/8183/

Rest 3 patches add:
- Vendor identification for LSPCON chips.
- AVI infoframe infrastructure for LSPCON displays.
- 4:2:0 support for LSPCON displays.

Shashank Sharma (3):
  drm/i915: check LSPCON vendor OUI
  drm/i915: write AVI infoframes for LSPCON
  drm/i915: YCBCR 420 support for LSPCON

Ville Syrjälä (4):
  drm/i915: Check has_infoframes when enabling infoframes
  drm/i915: Disable infoframes when shutting down DDI HDMI
  drm/i915: Move infoframe vfuncs into intel_digital_port
  drm/i915: Init infoframe vfuncs for DP encoders as well

 drivers/gpu/drm/i915/intel_ddi.c     |  37 ++--
 drivers/gpu/drm/i915/intel_display.c |  15 +-
 drivers/gpu/drm/i915/intel_dp.c      |  16 +-
 drivers/gpu/drm/i915/intel_drv.h     |  54 ++++--
 drivers/gpu/drm/i915/intel_hdmi.c    |  97 ++++++-----
 drivers/gpu/drm/i915/intel_lspcon.c  | 317 +++++++++++++++++++++++++++++++++--
 6 files changed, 457 insertions(+), 79 deletions(-)

-- 
2.7.4

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

[PATCH v2 1/7] drm/i915: Check has_infoframes when enabling infoframes

[Shashank Sharma]

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

has_infoframe is what tells us whether infoframes should be enabled, so
let's pass that instead of has_hdmi_sink to .set_infoframes().

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

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 494fbe0..d838e8d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2150,7 +2150,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 }
 
 static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
-				      bool has_hdmi_sink,
+				      bool has_infoframe,
 				      const struct intel_crtc_state *crtc_state,
 				      const struct drm_connector_state *conn_state,
 				      struct intel_shared_dpll *pll)
@@ -2177,7 +2177,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 		cnl_ddi_vswing_sequence(encoder, level);
 
 	intel_hdmi->set_infoframes(drm_encoder,
-				   has_hdmi_sink,
+				   has_infoframe,
 				   crtc_state, conn_state);
 }
 
@@ -2197,7 +2197,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 	}
 	if (type == INTEL_OUTPUT_HDMI) {
 		intel_ddi_pre_enable_hdmi(encoder,
-					  pipe_config->has_hdmi_sink,
+					  pipe_config->has_infoframe,
 					  pipe_config, conn_state,
 					  pipe_config->shared_dpll);
 	}
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5609976..bab1c5a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1650,7 +1650,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder,
 	intel_hdmi_prepare(encoder, pipe_config);
 
 	intel_hdmi->set_infoframes(&encoder->base,
-				   pipe_config->has_hdmi_sink,
+				   pipe_config->has_infoframe,
 				   pipe_config, conn_state);
 }
 
@@ -1670,7 +1670,7 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder,
 				 0x2b247878);
 
 	intel_hdmi->set_infoframes(&encoder->base,
-				   pipe_config->has_hdmi_sink,
+				   pipe_config->has_infoframe,
 				   pipe_config, conn_state);
 
 	g4x_enable_hdmi(encoder, pipe_config, conn_state);
@@ -1742,7 +1742,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder,
 	chv_set_phy_signal_level(encoder, 128, 102, false);
 
 	intel_hdmi->set_infoframes(&encoder->base,
-				   pipe_config->has_hdmi_sink,
+				   pipe_config->has_infoframe,
 				   pipe_config, conn_state);
 
 	g4x_enable_hdmi(encoder, pipe_config, conn_state);
-- 
2.7.4

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

[PATCH v2 2/7] drm/i915: Disable infoframes when shutting down DDI HDMI

[Shashank Sharma]

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

Disabling the video DIP when shutting the port down seems like a good
idea.

Bspec says:
"When disabling both the DIP port and DIP transmission,
 first disable the port and then disable DIP."
and
"Restriction : GCP is only supported with HDMI when the bits per color is
 not equal to 8. GCP must be enabled prior to enabling TRANS_DDI_FUNC_CTL
 for HDMI with bits per color not equal to 8 and disabled after disabling
 TRANS_DDI_FUNC_CTL"

So let's do it in the .post_disable() hook.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d838e8d..fdfc314 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2238,6 +2238,13 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
 	if (wait)
 		intel_wait_ddi_buf_idle(dev_priv, port);
 
+	if (type == INTEL_OUTPUT_HDMI) {
+		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+
+		intel_hdmi->set_infoframes(encoder, false,
+					old_crtc_state, old_conn_state);
+	}
+
 	if (intel_dp) {
 		intel_edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
-- 
2.7.4

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

[PATCH v2 3/7] drm/i915: Move infoframe vfuncs into intel_digital_port

[Shashank Sharma]

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

DP ports will also want to utilize the video DIP for SDP transmission.
So let's move the vfuncs into the dig_port.

v2: Rebase due to DDI changes

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  | 22 ++++++-------
 drivers/gpu/drm/i915/intel_drv.h  | 21 +++++++------
 drivers/gpu/drm/i915/intel_hdmi.c | 66 +++++++++++++++++++++------------------
 3 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fdfc314..554d3ce 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2155,12 +2155,11 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 				      const struct drm_connector_state *conn_state,
 				      struct intel_shared_dpll *pll)
 {
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	struct intel_hdmi *intel_hdmi = &dig_port->hdmi;
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct drm_encoder *drm_encoder = &encoder->base;
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	int level = intel_ddi_hdmi_level(dev_priv, port);
-	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
 	intel_ddi_clk_select(encoder, pll);
@@ -2176,9 +2175,9 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	else if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level);
 
-	intel_hdmi->set_infoframes(drm_encoder,
-				   has_infoframe,
-				   crtc_state, conn_state);
+	dig_port->set_infoframes(&encoder->base,
+				 has_infoframe,
+				 crtc_state, conn_state);
 }
 
 static void intel_ddi_pre_enable(struct intel_encoder *encoder,
@@ -2239,9 +2238,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
 		intel_wait_ddi_buf_idle(dev_priv, port);
 
 	if (type == INTEL_OUTPUT_HDMI) {
-		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
-
-		intel_hdmi->set_infoframes(encoder, false,
+		dig_port->set_infoframes(encoder, false,
 					old_crtc_state, old_conn_state);
 	}
 
@@ -2442,7 +2439,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	struct intel_hdmi *intel_hdmi;
+	struct intel_digital_port *intel_dig_port;
 	u32 temp, flags = 0;
 
 	/* XXX: DSI transcoder paranoia */
@@ -2481,9 +2478,10 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
 	case TRANS_DDI_MODE_SELECT_HDMI:
 		pipe_config->has_hdmi_sink = true;
-		intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+		intel_dig_port = enc_to_dig_port(&encoder->base);
 
-		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
+		if (intel_dig_port->infoframe_enabled(&encoder->base,
+						      pipe_config))
 			pipe_config->has_infoframe = true;
 
 		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ee0daec..bde2547 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -909,16 +909,6 @@ struct intel_hdmi {
 	bool has_audio;
 	bool rgb_quant_range_selectable;
 	struct intel_connector *attached_connector;
-	void (*write_infoframe)(struct drm_encoder *encoder,
-				const struct intel_crtc_state *crtc_state,
-				enum hdmi_infoframe_type type,
-				const void *frame, ssize_t len);
-	void (*set_infoframes)(struct drm_encoder *encoder,
-			       bool enable,
-			       const struct intel_crtc_state *crtc_state,
-			       const struct drm_connector_state *conn_state);
-	bool (*infoframe_enabled)(struct drm_encoder *encoder,
-				  const struct intel_crtc_state *pipe_config);
 };
 
 struct intel_dp_mst_encoder;
@@ -1069,6 +1059,17 @@ struct intel_digital_port {
 	bool release_cl2_override;
 	uint8_t max_lanes;
 	enum intel_display_power_domain ddi_io_power_domain;
+
+	void (*write_infoframe)(struct drm_encoder *encoder,
+				const struct intel_crtc_state *crtc_state,
+				enum hdmi_infoframe_type type,
+				const void *frame, ssize_t len);
+	void (*set_infoframes)(struct drm_encoder *encoder,
+			       bool enable,
+			       const struct intel_crtc_state *crtc_state,
+			       const struct drm_connector_state *conn_state);
+	bool (*infoframe_enabled)(struct drm_encoder *encoder,
+				  const struct intel_crtc_state *pipe_config);
 };
 
 struct intel_dp_mst_encoder {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index bab1c5a..d1973ff 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -434,7 +434,7 @@ static void intel_write_infoframe(struct drm_encoder *encoder,
 				  const struct intel_crtc_state *crtc_state,
 				  union hdmi_infoframe *frame)
 {
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
 	uint8_t buffer[VIDEO_DIP_DATA_SIZE];
 	ssize_t len;
 
@@ -450,7 +450,8 @@ static void intel_write_infoframe(struct drm_encoder *encoder,
 	buffer[3] = 0;
 	len++;
 
-	intel_hdmi->write_infoframe(encoder, crtc_state, frame->any.type, buffer, len);
+	intel_dig_port->write_infoframe(encoder, crtc_state,
+					frame->any.type, buffer, len);
 }
 
 static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
@@ -945,6 +946,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 				  struct intel_crtc_state *pipe_config)
 {
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct intel_digital_port *dig_port = hdmi_to_dig_port(intel_hdmi);
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 tmp, flags = 0;
@@ -965,7 +967,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	if (tmp & HDMI_MODE_SELECT_HDMI)
 		pipe_config->has_hdmi_sink = true;
 
-	if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
+	if (dig_port->infoframe_enabled(&encoder->base, pipe_config))
 		pipe_config->has_infoframe = true;
 
 	if (tmp & SDVO_AUDIO_ENABLE)
@@ -1142,6 +1144,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct intel_digital_port *intel_dig_port =
+		hdmi_to_dig_port(intel_hdmi);
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
 	u32 temp;
 
@@ -1184,7 +1188,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder,
 		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 	}
 
-	intel_hdmi->set_infoframes(&encoder->base, false, old_crtc_state, old_conn_state);
+	intel_dig_port->set_infoframes(&encoder->base, false,
+				       old_crtc_state, old_conn_state);
 
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
 }
@@ -1645,13 +1650,14 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder,
 				  struct intel_crtc_state *pipe_config,
 				  struct drm_connector_state *conn_state)
 {
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct intel_digital_port *intel_dig_port =
+		enc_to_dig_port(&encoder->base);
 
 	intel_hdmi_prepare(encoder, pipe_config);
 
-	intel_hdmi->set_infoframes(&encoder->base,
-				   pipe_config->has_infoframe,
-				   pipe_config, conn_state);
+	intel_dig_port->set_infoframes(&encoder->base,
+				       pipe_config->has_infoframe,
+				       pipe_config, conn_state);
 }
 
 static void vlv_hdmi_pre_enable(struct intel_encoder *encoder,
@@ -1659,7 +1665,6 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder,
 				struct drm_connector_state *conn_state)
 {
 	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
-	struct intel_hdmi *intel_hdmi = &dport->hdmi;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
@@ -1669,9 +1674,9 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder,
 	vlv_set_phy_signal_level(encoder, 0x2b245f5f, 0x00002000, 0x5578b83a,
 				 0x2b247878);
 
-	intel_hdmi->set_infoframes(&encoder->base,
-				   pipe_config->has_infoframe,
-				   pipe_config, conn_state);
+	dport->set_infoframes(&encoder->base,
+			      pipe_config->has_infoframe,
+			      pipe_config, conn_state);
 
 	g4x_enable_hdmi(encoder, pipe_config, conn_state);
 
@@ -1731,7 +1736,6 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder,
 				struct drm_connector_state *conn_state)
 {
 	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
-	struct intel_hdmi *intel_hdmi = &dport->hdmi;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
@@ -1741,9 +1745,9 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder,
 	/* Use 800mV-0dB */
 	chv_set_phy_signal_level(encoder, 128, 102, false);
 
-	intel_hdmi->set_infoframes(&encoder->base,
-				   pipe_config->has_infoframe,
-				   pipe_config, conn_state);
+	dport->set_infoframes(&encoder->base,
+			      pipe_config->has_infoframe,
+			      pipe_config, conn_state);
 
 	g4x_enable_hdmi(encoder, pipe_config, conn_state);
 
@@ -1941,25 +1945,25 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	}
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		intel_hdmi->write_infoframe = vlv_write_infoframe;
-		intel_hdmi->set_infoframes = vlv_set_infoframes;
-		intel_hdmi->infoframe_enabled = vlv_infoframe_enabled;
+		intel_dig_port->write_infoframe = vlv_write_infoframe;
+		intel_dig_port->set_infoframes = vlv_set_infoframes;
+		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
 	} else if (IS_G4X(dev_priv)) {
-		intel_hdmi->write_infoframe = g4x_write_infoframe;
-		intel_hdmi->set_infoframes = g4x_set_infoframes;
-		intel_hdmi->infoframe_enabled = g4x_infoframe_enabled;
+		intel_dig_port->write_infoframe = g4x_write_infoframe;
+		intel_dig_port->set_infoframes = g4x_set_infoframes;
+		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
 	} else if (HAS_DDI(dev_priv)) {
-		intel_hdmi->write_infoframe = hsw_write_infoframe;
-		intel_hdmi->set_infoframes = hsw_set_infoframes;
-		intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
+		intel_dig_port->write_infoframe = hsw_write_infoframe;
+		intel_dig_port->set_infoframes = hsw_set_infoframes;
+		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
 	} else if (HAS_PCH_IBX(dev_priv)) {
-		intel_hdmi->write_infoframe = ibx_write_infoframe;
-		intel_hdmi->set_infoframes = ibx_set_infoframes;
-		intel_hdmi->infoframe_enabled = ibx_infoframe_enabled;
+		intel_dig_port->write_infoframe = ibx_write_infoframe;
+		intel_dig_port->set_infoframes = ibx_set_infoframes;
+		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
 	} else {
-		intel_hdmi->write_infoframe = cpt_write_infoframe;
-		intel_hdmi->set_infoframes = cpt_set_infoframes;
-		intel_hdmi->infoframe_enabled = cpt_infoframe_enabled;
+		intel_dig_port->write_infoframe = cpt_write_infoframe;
+		intel_dig_port->set_infoframes = cpt_set_infoframes;
+		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
 	}
 
 	if (HAS_DDI(dev_priv))
-- 
2.7.4

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

[PATCH v2 4/7] drm/i915: Init infoframe vfuncs for DP encoders as well

[Shashank Sharma]

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

DP ports may want to use the video DIP for SDP transmission, so let's
initialize the vfuncs for DP encoders as well. The only exception is
port A eDP prior to HSW as that one doesn't have a video DIP instance.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  |  2 ++
 drivers/gpu/drm/i915/intel_dp.c   |  3 +++
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 52 ++++++++++++++++++++++-----------------
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 554d3ce..9384080 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2734,6 +2734,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = 0;
 
+	intel_infoframe_init(intel_dig_port);
+
 	if (init_dp) {
 		if (!intel_ddi_init_dp_connector(intel_dig_port))
 			goto err;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2d42d09..d580243 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6150,6 +6150,9 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 	intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
 	dev_priv->hotplug.irq_port[port] = intel_dig_port;
 
+	if (port != PORT_A)
+		intel_infoframe_init(intel_dig_port);
+
 	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
 		goto err_init_connector;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bde2547..7eadac0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1650,6 +1650,7 @@ void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
 				       bool high_tmds_clock_ratio,
 				       bool scrambling);
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
+void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
 
 
 /* intel_lvds.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index d1973ff..e4a27e1 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1895,6 +1895,34 @@ static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
 	return ddc_pin;
 }
 
+void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(intel_dig_port->base.base.dev);
+
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		intel_dig_port->write_infoframe = vlv_write_infoframe;
+		intel_dig_port->set_infoframes = vlv_set_infoframes;
+		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
+	} else if (IS_G4X(dev_priv)) {
+		intel_dig_port->write_infoframe = g4x_write_infoframe;
+		intel_dig_port->set_infoframes = g4x_set_infoframes;
+		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
+	} else if (HAS_DDI(dev_priv)) {
+		intel_dig_port->write_infoframe = hsw_write_infoframe;
+		intel_dig_port->set_infoframes = hsw_set_infoframes;
+		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
+	} else if (HAS_PCH_IBX(dev_priv)) {
+		intel_dig_port->write_infoframe = ibx_write_infoframe;
+		intel_dig_port->set_infoframes = ibx_set_infoframes;
+		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
+	} else {
+		intel_dig_port->write_infoframe = cpt_write_infoframe;
+		intel_dig_port->set_infoframes = cpt_set_infoframes;
+		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
+	}
+}
+
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			       struct intel_connector *intel_connector)
 {
@@ -1944,28 +1972,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 		return;
 	}
 
-	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		intel_dig_port->write_infoframe = vlv_write_infoframe;
-		intel_dig_port->set_infoframes = vlv_set_infoframes;
-		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
-	} else if (IS_G4X(dev_priv)) {
-		intel_dig_port->write_infoframe = g4x_write_infoframe;
-		intel_dig_port->set_infoframes = g4x_set_infoframes;
-		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
-	} else if (HAS_DDI(dev_priv)) {
-		intel_dig_port->write_infoframe = hsw_write_infoframe;
-		intel_dig_port->set_infoframes = hsw_set_infoframes;
-		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
-	} else if (HAS_PCH_IBX(dev_priv)) {
-		intel_dig_port->write_infoframe = ibx_write_infoframe;
-		intel_dig_port->set_infoframes = ibx_set_infoframes;
-		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
-	} else {
-		intel_dig_port->write_infoframe = cpt_write_infoframe;
-		intel_dig_port->set_infoframes = cpt_set_infoframes;
-		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
-	}
-
 	if (HAS_DDI(dev_priv))
 		intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
 	else
@@ -2064,5 +2070,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
 	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 	intel_dig_port->max_lanes = 4;
 
+	intel_infoframe_init(intel_dig_port);
+
 	intel_hdmi_init_connector(intel_dig_port, intel_connector);
 }
-- 
2.7.4

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

[PATCH v2 5/7] drm/i915: check LSPCON vendor OUI

[Shashank Sharma]

Intel LSPCON chip is provided by 2 vendors:
- Megachips America (MCA)
- Parade technologies (Parade tech)

Its important to know the vendor of this chip, as the address to
write AVI infoframes is different for those two.

This patch reads the vendor OUI signature, and marks into LSPCON
encoder structure for future usages.

This patch also does a small re-arrangement of the code, by moving
lspcon mode change into probe function.

V2: Use dp->desc for OUI detection, dont add a helper for this
    (Ville)

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  6 ++++
 drivers/gpu/drm/i915/intel_lspcon.c | 69 +++++++++++++++++++++++++++++--------
 2 files changed, 61 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7eadac0..adab635 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1043,9 +1043,15 @@ struct intel_dp {
 	struct intel_dp_compliance compliance;
 };
 
+enum lspcon_vendor {
+	LSPCON_VENDOR_MCA,
+	LSPCON_VENDOR_PARADE
+};
+
 struct intel_lspcon {
 	bool active;
 	enum drm_lspcon_mode mode;
+	enum lspcon_vendor vendor;
 };
 
 struct intel_digital_port {
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 5abef482..93507c5 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -27,6 +27,10 @@
 #include <drm/drm_dp_dual_mode_helper.h>
 #include "intel_drv.h"
 
+/* LSPCON OUI Vendor ID(signatures) */
+#define LSPCON_VENDOR_PARADE_OUI 0x001CF8
+#define LSPCON_VENDOR_MCA_OUI 0x0060AD
+
 static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
 {
 	struct intel_digital_port *dig_port =
@@ -50,6 +54,40 @@ static const char *lspcon_mode_name(enum drm_lspcon_mode mode)
 	}
 }
 
+static bool lspcon_detect_vendor(struct intel_lspcon *lspcon)
+{
+	struct intel_dp *dp = lspcon_to_intel_dp(lspcon);
+	struct drm_dp_dpcd_ident *ident;
+	u32 vendor_oui;
+
+	if (drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd))) {
+		DRM_ERROR("Can't read description\n");
+		return false;
+	}
+
+	ident = &dp->desc.ident;
+	vendor_oui = (ident->oui[0] << 16) | (ident->oui[1] << 8) |
+		      ident->oui[2];
+
+	switch (vendor_oui) {
+	case LSPCON_VENDOR_MCA_OUI:
+		lspcon->vendor = LSPCON_VENDOR_MCA;
+		DRM_DEBUG_KMS("Vendor: Mega Chips\n");
+		break;
+
+	case LSPCON_VENDOR_PARADE_OUI:
+		lspcon->vendor = LSPCON_VENDOR_PARADE;
+		DRM_DEBUG_KMS("Vendor: Parade Tech\n");
+		break;
+
+	default:
+		DRM_ERROR("Invalid/Unknown vendor OUI\n");
+		return false;
+	}
+
+	return true;
+}
+
 static enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
 {
 	enum drm_lspcon_mode current_mode;
@@ -151,7 +189,18 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
 	/* Yay ... got a LSPCON device */
 	DRM_DEBUG_KMS("LSPCON detected\n");
 	lspcon->mode = lspcon_wait_mode(lspcon, expected_mode);
-	lspcon->active = true;
+
+	/*
+	 * In the SW state machine, lets Put LSPCON in PCON mode only.
+	 * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
+	 * 2.0 sinks.
+	 */
+	if (lspcon->active && lspcon->mode != DRM_LSPCON_MODE_PCON) {
+		if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
+			DRM_ERROR("LSPCON mode change to PCON failed\n");
+			return false;
+		}
+	}
 	return true;
 }
 
@@ -223,25 +272,17 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
 		return false;
 	}
 
-	/*
-	* In the SW state machine, lets Put LSPCON in PCON mode only.
-	* In this way, it will work with both HDMI 1.4 sinks as well as HDMI
-	* 2.0 sinks.
-	*/
-	if (lspcon->active && lspcon->mode != DRM_LSPCON_MODE_PCON) {
-		if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
-			DRM_ERROR("LSPCON mode change to PCON failed\n");
-			return false;
-		}
-	}
-
 	if (!intel_dp_read_dpcd(dp)) {
 		DRM_ERROR("LSPCON DPCD read failed\n");
 		return false;
 	}
 
-	drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd));
+	if (!lspcon_detect_vendor(lspcon)) {
+		DRM_ERROR("LSPCON vendor detection failed\n");
+		return false;
+	}
 
+	lspcon->active = true;
 	DRM_DEBUG_KMS("Success: LSPCON init\n");
 	return true;
 }
-- 
2.7.4

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

[PATCH v2 6/7] drm/i915: write AVI infoframes for LSPCON

[Shashank Sharma]

To pass AVI infoframes from display controller to LSPCON, we
have to write infoframe packets into vendor specified AUX address,
in vendor specified way.

Also, LSPCON vendors provide AUX offsets, to inform the LSPCON
chip that the AVI IF packets are written, so that the firmware
can pick it up and apply.

This patch adds function to write AVI infoframes for both MCA as
well as Parade Tech LSPCON chips. These two vendors provide different
methods for writing infoframes, so this patch contains two different
functions, one for each.

V2: Rebase

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    |  12 +-
 drivers/gpu/drm/i915/intel_drv.h    |  16 +++
 drivers/gpu/drm/i915/intel_hdmi.c   |  15 ++-
 drivers/gpu/drm/i915/intel_lspcon.c | 246 ++++++++++++++++++++++++++++++++++++
 4 files changed, 284 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9384080..08f3567 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2193,6 +2193,15 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 					pipe_config->shared_dpll,
 					intel_crtc_has_type(pipe_config,
 							    INTEL_OUTPUT_DP_MST));
+
+		if (pipe_config->lspcon_active) {
+			struct intel_digital_port *dig_port =
+					enc_to_dig_port(&encoder->base);
+
+			dig_port->set_infoframes(&encoder->base,
+				 pipe_config->has_infoframe,
+				 pipe_config, conn_state);
+		}
 	}
 	if (type == INTEL_OUTPUT_HDMI) {
 		intel_ddi_pre_enable_hdmi(encoder,
@@ -2734,8 +2743,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = 0;
 
-	intel_infoframe_init(intel_dig_port);
-
 	if (init_dp) {
 		if (!intel_ddi_init_dp_connector(intel_dig_port))
 			goto err;
@@ -2765,6 +2772,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 				port_name(port));
 	}
 
+	intel_infoframe_init(intel_dig_port);
 	return;
 
 err:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index adab635..99eaab6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1052,6 +1052,10 @@ struct intel_lspcon {
 	bool active;
 	enum drm_lspcon_mode mode;
 	enum lspcon_vendor vendor;
+
+	void (*write_infoframe)(struct drm_encoder *encoder,
+				const struct intel_crtc_state *crtc_state,
+				union hdmi_infoframe *frame);
 };
 
 struct intel_digital_port {
@@ -1978,6 +1982,18 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
 bool lspcon_init(struct intel_digital_port *intel_dig_port);
 void lspcon_resume(struct intel_lspcon *lspcon);
 void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
+bool lspcon_ycbcr420_config(struct drm_connector *connector,
+			     struct intel_crtc_state *config);
+void lspcon_write_infoframe(struct drm_encoder *encoder,
+			    const struct intel_crtc_state *crtc_state,
+			    enum hdmi_infoframe_type type,
+			    const void *buf, ssize_t len);
+void lspcon_set_infoframes(struct drm_encoder *encoder,
+			   bool enable,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state);
+bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
+			      const struct intel_crtc_state *pipe_config);
 
 /* intel_pipe_crc.c */
 int intel_pipe_crc_create(struct drm_minor *minor);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e4a27e1..5710029 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1909,9 +1909,18 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
 		intel_dig_port->set_infoframes = g4x_set_infoframes;
 		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
 	} else if (HAS_DDI(dev_priv)) {
-		intel_dig_port->write_infoframe = hsw_write_infoframe;
-		intel_dig_port->set_infoframes = hsw_set_infoframes;
-		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
+		if (intel_dig_port->lspcon.active) {
+			intel_dig_port->write_infoframe =
+						lspcon_write_infoframe;
+			intel_dig_port->set_infoframes = lspcon_set_infoframes;
+			intel_dig_port->infoframe_enabled =
+						lspcon_infoframe_enabled;
+		} else {
+			intel_dig_port->set_infoframes = hsw_set_infoframes;
+			intel_dig_port->infoframe_enabled =
+						hsw_infoframe_enabled;
+			intel_dig_port->write_infoframe = hsw_write_infoframe;
+		}
 	} else if (HAS_PCH_IBX(dev_priv)) {
 		intel_dig_port->write_infoframe = ibx_write_infoframe;
 		intel_dig_port->set_infoframes = ibx_set_infoframes;
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 93507c5..b4fcd30 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -31,6 +31,18 @@
 #define LSPCON_VENDOR_PARADE_OUI 0x001CF8
 #define LSPCON_VENDOR_MCA_OUI 0x0060AD
 
+/* AUX addresses to write AVI IF into */
+#define LSPCON_MCA_AVI_IF_WRITE_OFFSET 0x5C0
+#define LSPCON_MCA_AVI_IF_CTRL 0x5DF
+#define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
+#define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
+
+#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
+#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
+#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
+#define LSPCON_PARADE_AVI_IF_STATUS 0x51F
+#define  LSPCON_PARADE_AVI_IF_HANDLED (2 << 6)
+
 static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
 {
 	struct intel_digital_port *dig_port =
@@ -227,6 +239,240 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
 	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
 }
 
+bool lspcon_ycbcr420_config(struct drm_connector *connector,
+			     struct intel_crtc_state *config)
+{
+	struct drm_display_info *info = &connector->display_info;
+	struct drm_display_mode *mode = &config->base.adjusted_mode;
+
+	if (drm_mode_is_420_only(info, mode)) {
+
+		if (!connector->ycbcr_420_allowed) {
+			DRM_ERROR("Platform doesn't support YCBCR420 output\n");
+			return false;
+		}
+
+		config->port_clock /= 2;
+		return true;
+	}
+
+	return false;
+}
+
+static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
+					       const uint8_t *buffer,
+					       ssize_t len)
+{
+	u8 avi_if_ctrl;
+	u8 avi_if_status;
+	u8 count = 0;
+	u8 retry = 5;
+	u8 avi_buf[8] = {0, };
+	uint16_t reg;
+	ssize_t ret;
+
+	while (count++ < 4) {
+
+		do {
+			/* Is LSPCON FW ready */
+			reg = LSPCON_PARADE_AVI_IF_CTRL;
+			ret = drm_dp_dpcd_read(aux, reg, &avi_if_ctrl, 1);
+			if (ret < 0) {
+				DRM_ERROR("DPCD read failed, add:0x%x\n", reg);
+				return false;
+			}
+
+			if (avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)
+				break;
+			usleep_range(100, 200);
+		} while (--retry);
+
+		if (!(avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)) {
+			DRM_ERROR("LSPCON FW not ready for infoframes\n");
+			return false;
+		}
+
+		/*
+		 * AVI Infoframe contains 31 bytes of data:
+		 *	HB0 to HB2   (3 bytes header)
+		 *	PB0 to PB27 (28 bytes data)
+		 * As per Parade spec, while sending first block (8bytes),
+		 * byte 0 is kept for request token no, and byte1 - byte7
+		 * contain frame data. So we have to pack frame like this:
+		 *	first block of 8 bytes: <token> <HB0-HB2> <PB0-PB3>
+		 *	next 3 blocks: <PB4-PB27>
+		 */
+		if (count)
+			memcpy(avi_buf, buffer + count * 8 - 1, 8);
+		else {
+			avi_buf[0] = 1;
+			memcpy(&avi_buf[1], buffer, 7);
+		}
+
+		/* Write 8 bytes of data at a time */
+		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
+		ret = drm_dp_dpcd_write(aux, reg, avi_buf, 8);
+		if (ret < 0) {
+			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
+			return false;
+		}
+
+		/*
+		 * While sending a block of 8 byes, we need to inform block
+		 * number to FW, by programming bits[1:0] of ctrl reg with
+		 * block number
+		 */
+		avi_if_ctrl = 0x80 + count;
+		reg = LSPCON_PARADE_AVI_IF_CTRL;
+		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
+		if (ret < 0) {
+			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
+			return false;
+		}
+	}
+
+	/* Check LSPCON FW status */
+	reg = LSPCON_PARADE_AVI_IF_STATUS;
+	ret = drm_dp_dpcd_read(aux, reg, &avi_if_status, 1);
+	if (ret < 0) {
+		DRM_ERROR("DPCD write failed, address 0x%x\n", reg);
+		return false;
+	}
+
+	if (avi_if_status & LSPCON_PARADE_AVI_IF_HANDLED)
+		DRM_DEBUG_KMS("AVI IF handled by FW\n");
+
+	return true;
+}
+
+static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
+					    const uint8_t *buffer, ssize_t len)
+{
+	int ret;
+	uint32_t val = 0;
+	uint16_t reg;
+	const uint8_t *data = buffer;
+
+	reg = LSPCON_MCA_AVI_IF_WRITE_OFFSET;
+	while (val < len) {
+		ret = drm_dp_dpcd_write(aux, reg, (void *)data, 1);
+		if (ret < 0) {
+			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
+			return false;
+		}
+		val++; reg++; data++;
+	}
+
+	val = 0;
+	reg = LSPCON_MCA_AVI_IF_CTRL;
+	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
+	if (ret < 0) {
+		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
+		return false;
+	}
+
+	/* Indicate LSPCON chip about infoframe, clear bit 1 and set bit 0 */
+	val &= ~LSPCON_MCA_AVI_IF_HANDLED;
+	val |= LSPCON_MCA_AVI_IF_KICKOFF;
+
+	ret = drm_dp_dpcd_write(aux, reg, &val, 1);
+	if (ret < 0) {
+		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
+		return false;
+	}
+
+	val = 0;
+	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
+	if (ret < 0) {
+		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
+		return false;
+	}
+
+	if (val == LSPCON_MCA_AVI_IF_HANDLED)
+		DRM_DEBUG_KMS("AVI IF handled by FW\n");
+
+	return true;
+}
+
+void lspcon_write_infoframe(struct drm_encoder *encoder,
+			    const struct intel_crtc_state *crtc_state,
+			    enum hdmi_infoframe_type type,
+			    const void *frame, ssize_t len)
+{
+	bool ret;
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
+
+	/* LSPCON only needs AVI IF */
+	if (type != HDMI_INFOFRAME_TYPE_AVI)
+		return;
+
+	if (lspcon->vendor == LSPCON_VENDOR_MCA)
+		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
+						      frame, len);
+	else
+		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
+							 frame, len);
+
+	if (!ret)
+		DRM_ERROR("Failed to write AVI infoframes\n");
+	else
+		DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
+}
+
+void lspcon_set_infoframes(struct drm_encoder *encoder,
+			   bool enable,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state)
+{
+	ssize_t ret;
+	union hdmi_infoframe frame;
+	uint8_t buf[VIDEO_DIP_DATA_SIZE];
+	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+	struct intel_dp *intel_dp = &dig_port->dp;
+	struct drm_connector *connector = &intel_dp->attached_connector->base;
+	const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
+	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
+
+	if (!crtc_state->lspcon_active) {
+		DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
+		return;
+	}
+
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						       mode, is_hdmi2_sink);
+	if (ret < 0) {
+		DRM_ERROR("couldn't fill AVI infoframe\n");
+		return;
+	}
+
+	if (crtc_state->ycbcr420)
+		frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
+	else
+		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
+
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
+					   crtc_state->limited_color_range ?
+					   HDMI_QUANTIZATION_RANGE_LIMITED :
+					   HDMI_QUANTIZATION_RANGE_FULL,
+					   false);
+
+	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
+	if (ret < 0) {
+		DRM_ERROR("Failed to pack AVI IF\n");
+		return;
+	}
+
+	dig_port->write_infoframe(encoder, crtc_state, HDMI_INFOFRAME_TYPE_AVI,
+				  buf, ret);
+}
+
+bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
+			      const struct intel_crtc_state *pipe_config)
+{
+	return enc_to_intel_lspcon(encoder)->active;
+}
+
 void lspcon_resume(struct intel_lspcon *lspcon)
 {
 	enum drm_lspcon_mode expected_mode;
-- 
2.7.4

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

[PATCH v2 7/7] drm/i915: YCBCR 420 support for LSPCON

[Shashank Sharma]

LSPCON chips support YCBCR420 outputs. To be able to get
YCBCR420 output from LSPCON chip, the source should:
- Generate YCBCR444 HDMI output
- Set AVI infoframes for a YCBCR420 output.

Unlike Native HDMI 4:2:0 outputs, there is no need to
reserve a scaler in pipe, LSPCON does the downsampling from
YCBCR 4:4:4 to 4:2:0 on its own.

LSPCON FW gets the information from AVI infoframes, and generates
YCBCR420 output from a YCBCR444 input. This patch adds the necessary
changes to drive YCBCR420 output from LSPCON based HDMI output.

V2: Dont mess around with 8bpc and 12bpc clocks, its not reqd
    for DP links. (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++----
 drivers/gpu/drm/i915/intel_dp.c      | 13 ++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     | 10 +++++++++-
 drivers/gpu/drm/i915/intel_lspcon.c  |  2 ++
 4 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5a89db1..eb1d93b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4487,7 +4487,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	 */
 	need_scaling = src_w != dst_w || src_h != dst_h;
 
-	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
+	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX &&
+		!crtc_state->lspcon_active)
 		need_scaling = true;
 
 	/*
@@ -7970,9 +7971,15 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
 			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
 
 		if (config->ycbcr420) {
-			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV |
-				PIPEMISC_YUV420_ENABLE |
-				PIPEMISC_YUV420_MODE_FULL_BLEND;
+			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
+			/*
+			 * LSPCON doesn't need scaling/blending to be done in
+			 * pipe. It just needs YCBCR444 input and proper AVI
+			 * infoframes for 4:2:0 output enabling.
+			 */
+			if (!config->lspcon_active)
+				val |= PIPEMISC_YUV420_ENABLE |
+				       PIPEMISC_YUV420_MODE_FULL_BLEND;
 		}
 
 		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d580243..b7774cc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1614,7 +1614,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	enum port port = dp_to_dig_port(intel_dp)->port;
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	enum port port = dig_port->port;
+	struct intel_lspcon *lspcon = &dig_port->lspcon;
 	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	struct intel_digital_connector_state *intel_conn_state =
@@ -1635,6 +1637,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	common_len = intel_dp_common_len_rate_limit(intel_dp,
 						    intel_dp->max_link_rate);
 
+	/* LSPCON needs special handling to drive YCBCR420 outputs */
+	if (lspcon->active) {
+		struct drm_connector *connector = &intel_connector->base;
+
+		pipe_config->lspcon_active = true;
+		pipe_config->ycbcr420 = lspcon_ycbcr420_config(connector,
+							       pipe_config);
+	}
+
 	/* No common link rates between source and sink */
 	WARN_ON(common_len <= 0);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 99eaab6..b3cbe2d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -783,6 +783,9 @@ struct intel_crtc_state {
 
 	/* output format is YCBCR 4:2:0 */
 	bool ycbcr420;
+
+	/* LSPCON is active on port */
+	bool lspcon_active;
 };
 
 struct intel_crtc {
@@ -1182,6 +1185,12 @@ static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
 	return &enc_to_dig_port(encoder)->dp;
 }
 
+static inline struct intel_lspcon *
+enc_to_intel_lspcon(struct drm_encoder *encoder)
+{
+	return &enc_to_dig_port(encoder)->lspcon;
+}
+
 static inline struct intel_digital_port *
 dp_to_dig_port(struct intel_dp *intel_dp)
 {
@@ -1662,7 +1671,6 @@ void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
 void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
 
-
 /* intel_lvds.c */
 void intel_lvds_init(struct drm_i915_private *dev_priv);
 struct intel_encoder *intel_get_lvds_encoder(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index b4fcd30..8413a4c 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -504,6 +504,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
 	struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_connector *connector = &dp->attached_connector->base;
 
 	if (!IS_GEN9(dev_priv)) {
 		DRM_ERROR("LSPCON is supported on GEN9 only\n");
@@ -528,6 +529,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
 		return false;
 	}
 
+	connector->ycbcr_420_allowed = true;
 	lspcon->active = true;
 	DRM_DEBUG_KMS("Success: LSPCON init\n");
 	return true;
-- 
2.7.4

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

✗ Fi.CI.BAT: warning for YCBCR 4:2:0 support for LSPCON

[Patchwork]

== Series Details ==

Series: YCBCR 4:2:0 support for LSPCON
URL   : https://patchwork.freedesktop.org/series/28536/
State : warning

== Summary ==

Series 28536v1 YCBCR 4:2:0 support for LSPCON
https://patchwork.freedesktop.org/api/1.0/series/28536/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_busy:
        Subgroup basic-flip-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-c:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> DMESG-WARN (fi-skl-6770hq) fdo#100215 +3
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-after-cursor-atomic:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-after-cursor-legacy:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-before-cursor-atomic:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-before-cursor-legacy:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-before-cursor-varying-size:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (fi-skl-6770hq) fdo#99739
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-c:
WARNING: Long output truncated

e2586470ba9f468a2f72b1619d79aa1d510c45a7 drm-tip: 2017y-08m-08d-20h-39m-09s UTC integration manifest
3bb5d2dec1f7 drm/i915: YCBCR 420 support for LSPCON
13314ea74ed6 drm/i915: write AVI infoframes for LSPCON
8ffd6e3c7ca1 drm/i915: check LSPCON vendor OUI
d1880ab5c81f drm/i915: Init infoframe vfuncs for DP encoders as well
eb83618c7424 drm/i915: Move infoframe vfuncs into intel_digital_port
12132038736e drm/i915: Disable infoframes when shutting down DDI HDMI
57449692e789 drm/i915: Check has_infoframes when enabling infoframes

== Logs ==

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

Re: ✗ Fi.CI.BAT: warning for YCBCR 4:2:0 support for LSPCON

[Sharma, Shashank]

These failure are from Parade based LSPCON's, where the delay is not yet fine tuned:
[  197.761072] [drm:lspcon_write_infoframe [i915]] *ERROR* LSPCON FW not ready for infoframes
[  197.761100] [drm:lspcon_write_infoframe [i915]] *ERROR* Failed to write AVI infoframes

Regards
Shashank
-----Original Message-----
From: Patchwork [mailto:patchwork@emeril.freedesktop.org] 
Sent: Wednesday, August 9, 2017 12:35 PM
To: Sharma, Shashank <shashank.sharma@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: ✗ Fi.CI.BAT: warning for YCBCR 4:2:0 support for LSPCON

== Series Details ==

Series: YCBCR 4:2:0 support for LSPCON
URL   : https://patchwork.freedesktop.org/series/28536/
State : warning

== Summary ==

Series 28536v1 YCBCR 4:2:0 support for LSPCON https://patchwork.freedesktop.org/api/1.0/series/28536/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_busy:
        Subgroup basic-flip-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-c:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> DMESG-WARN (fi-skl-6770hq) fdo#100215 +3
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-after-cursor-atomic:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-after-cursor-legacy:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-before-cursor-atomic:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-before-cursor-legacy:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-before-cursor-varying-size:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (fi-skl-6770hq) fdo#99739
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup read-crc-pipe-c:
WARNING: Long output truncated

e2586470ba9f468a2f72b1619d79aa1d510c45a7 drm-tip: 2017y-08m-08d-20h-39m-09s UTC integration manifest
3bb5d2dec1f7 drm/i915: YCBCR 420 support for LSPCON
13314ea74ed6 drm/i915: write AVI infoframes for LSPCON
8ffd6e3c7ca1 drm/i915: check LSPCON vendor OUI d1880ab5c81f drm/i915: Init infoframe vfuncs for DP encoders as well
eb83618c7424 drm/i915: Move infoframe vfuncs into intel_digital_port 12132038736e drm/i915: Disable infoframes when shutting down DDI HDMI
57449692e789 drm/i915: Check has_infoframes when enabling infoframes

== Logs ==

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

Re: [PATCH v2 5/7] drm/i915: check LSPCON vendor OUI

[Maarten Lankhorst]

Hey,

Op 09-08-17 om 08:46 schreef Shashank Sharma:
> Intel LSPCON chip is provided by 2 vendors:
> - Megachips America (MCA)
> - Parade technologies (Parade tech)
>
> Its important to know the vendor of this chip, as the address to
> write AVI infoframes is different for those two.
>
> This patch reads the vendor OUI signature, and marks into LSPCON
> encoder structure for future usages.
>
> This patch also does a small re-arrangement of the code, by moving
> lspcon mode change into probe function.
>
> V2: Use dp->desc for OUI detection, dont add a helper for this
>     (Ville)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |  6 ++++
>  drivers/gpu/drm/i915/intel_lspcon.c | 69 +++++++++++++++++++++++++++++--------
>  2 files changed, 61 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7eadac0..adab635 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1043,9 +1043,15 @@ struct intel_dp {
>  	struct intel_dp_compliance compliance;
>  };
>  
> +enum lspcon_vendor {
> +	LSPCON_VENDOR_MCA,
> +	LSPCON_VENDOR_PARADE
> +};
> +
>  struct intel_lspcon {
>  	bool active;
>  	enum drm_lspcon_mode mode;
> +	enum lspcon_vendor vendor;
>  };
>  
>  struct intel_digital_port {
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 5abef482..93507c5 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -27,6 +27,10 @@
>  #include <drm/drm_dp_dual_mode_helper.h>
>  #include "intel_drv.h"
>  
> +/* LSPCON OUI Vendor ID(signatures) */
> +#define LSPCON_VENDOR_PARADE_OUI 0x001CF8
> +#define LSPCON_VENDOR_MCA_OUI 0x0060AD
> +
>  static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
>  {
>  	struct intel_digital_port *dig_port =
> @@ -50,6 +54,40 @@ static const char *lspcon_mode_name(enum drm_lspcon_mode mode)
>  	}
>  }
>  
> +static bool lspcon_detect_vendor(struct intel_lspcon *lspcon)
> +{
> +	struct intel_dp *dp = lspcon_to_intel_dp(lspcon);
> +	struct drm_dp_dpcd_ident *ident;
> +	u32 vendor_oui;
> +
> +	if (drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd))) {
> +		DRM_ERROR("Can't read description\n");
> +		return false;
> +	}
> +
> +	ident = &dp->desc.ident;
> +	vendor_oui = (ident->oui[0] << 16) | (ident->oui[1] << 8) |
> +		      ident->oui[2];
> +
> +	switch (vendor_oui) {
> +	case LSPCON_VENDOR_MCA_OUI:
> +		lspcon->vendor = LSPCON_VENDOR_MCA;
> +		DRM_DEBUG_KMS("Vendor: Mega Chips\n");
> +		break;
> +
> +	case LSPCON_VENDOR_PARADE_OUI:
> +		lspcon->vendor = LSPCON_VENDOR_PARADE;
> +		DRM_DEBUG_KMS("Vendor: Parade Tech\n");
> +		break;
> +
> +	default:
> +		DRM_ERROR("Invalid/Unknown vendor OUI\n");
MISSING_CASE(vendor_oui) ?
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  static enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
>  {
>  	enum drm_lspcon_mode current_mode;
> @@ -151,7 +189,18 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
>  	/* Yay ... got a LSPCON device */
>  	DRM_DEBUG_KMS("LSPCON detected\n");
>  	lspcon->mode = lspcon_wait_mode(lspcon, expected_mode);
> -	lspcon->active = true;
> +
> +	/*
> +	 * In the SW state machine, lets Put LSPCON in PCON mode only.
> +	 * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
> +	 * 2.0 sinks.
> +	 */
> +	if (lspcon->active && lspcon->mode != DRM_LSPCON_MODE_PCON) {
> +		if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
> +			DRM_ERROR("LSPCON mode change to PCON failed\n");
> +			return false;
> +		}
> +	}
>  	return true;
>  }
>  
> @@ -223,25 +272,17 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>  		return false;
>  	}
>  
> -	/*
> -	* In the SW state machine, lets Put LSPCON in PCON mode only.
> -	* In this way, it will work with both HDMI 1.4 sinks as well as HDMI
> -	* 2.0 sinks.
> -	*/
> -	if (lspcon->active && lspcon->mode != DRM_LSPCON_MODE_PCON) {
> -		if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
> -			DRM_ERROR("LSPCON mode change to PCON failed\n");
> -			return false;
> -		}
> -	}
> -
>  	if (!intel_dp_read_dpcd(dp)) {
>  		DRM_ERROR("LSPCON DPCD read failed\n");
>  		return false;
>  	}
>  
> -	drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd));
> +	if (!lspcon_detect_vendor(lspcon)) {
> +		DRM_ERROR("LSPCON vendor detection failed\n");
> +		return false;
> +	}
Error seems double here, lspcon_detect_vendor should already succeed.

Same for lspcon_probe I think, better to upgrade the DRM_DEBUG_KMS failure there to DRM_ERROR there and remove the one from lspcon_init.
>  
> +	lspcon->active = true;
>  	DRM_DEBUG_KMS("Success: LSPCON init\n");
>  	return true;
>  }


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

Re: [PATCH v2 6/7] drm/i915: write AVI infoframes for LSPCON

[Maarten Lankhorst]

Op 09-08-17 om 08:46 schreef Shashank Sharma:
> To pass AVI infoframes from display controller to LSPCON, we
> have to write infoframe packets into vendor specified AUX address,
> in vendor specified way.
>
> Also, LSPCON vendors provide AUX offsets, to inform the LSPCON
> chip that the AVI IF packets are written, so that the firmware
> can pick it up and apply.
>
> This patch adds function to write AVI infoframes for both MCA as
> well as Parade Tech LSPCON chips. These two vendors provide different
> methods for writing infoframes, so this patch contains two different
> functions, one for each.
>
> V2: Rebase
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
This patch will fail to compile without 7/7 applied:
- enc_to_intel_lspcon missing.
- crtc_state->lspcon_active missing.

>  drivers/gpu/drm/i915/intel_ddi.c    |  12 +-
>  drivers/gpu/drm/i915/intel_drv.h    |  16 +++
>  drivers/gpu/drm/i915/intel_hdmi.c   |  15 ++-
>  drivers/gpu/drm/i915/intel_lspcon.c | 246 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 284 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 9384080..08f3567 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2193,6 +2193,15 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
>  					pipe_config->shared_dpll,
>  					intel_crtc_has_type(pipe_config,
>  							    INTEL_OUTPUT_DP_MST));
> +
> +		if (pipe_config->lspcon_active) {
> +			struct intel_digital_port *dig_port =
> +					enc_to_dig_port(&encoder->base);
> +
> +			dig_port->set_infoframes(&encoder->base,
> +				 pipe_config->has_infoframe,
> +				 pipe_config, conn_state);
> +		}
>  	}
>  	if (type == INTEL_OUTPUT_HDMI) {
>  		intel_ddi_pre_enable_hdmi(encoder,
> @@ -2734,8 +2743,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>  	intel_encoder->cloneable = 0;
>  
> -	intel_infoframe_init(intel_dig_port);
> -
>  	if (init_dp) {
>  		if (!intel_ddi_init_dp_connector(intel_dig_port))
>  			goto err;
> @@ -2765,6 +2772,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  				port_name(port));
>  	}
>  
> +	intel_infoframe_init(intel_dig_port);
>  	return;
>  
>  err:
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index adab635..99eaab6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1052,6 +1052,10 @@ struct intel_lspcon {
>  	bool active;
>  	enum drm_lspcon_mode mode;
>  	enum lspcon_vendor vendor;
> +
> +	void (*write_infoframe)(struct drm_encoder *encoder,
> +				const struct intel_crtc_state *crtc_state,
> +				union hdmi_infoframe *frame);
>  };
>  
>  struct intel_digital_port {
> @@ -1978,6 +1982,18 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
>  bool lspcon_init(struct intel_digital_port *intel_dig_port);
>  void lspcon_resume(struct intel_lspcon *lspcon);
>  void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
> +			     struct intel_crtc_state *config);
> +void lspcon_write_infoframe(struct drm_encoder *encoder,
> +			    const struct intel_crtc_state *crtc_state,
> +			    enum hdmi_infoframe_type type,
> +			    const void *buf, ssize_t len);
> +void lspcon_set_infoframes(struct drm_encoder *encoder,
> +			   bool enable,
> +			   const struct intel_crtc_state *crtc_state,
> +			   const struct drm_connector_state *conn_state);
> +bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
> +			      const struct intel_crtc_state *pipe_config);
>  
>  /* intel_pipe_crc.c */
>  int intel_pipe_crc_create(struct drm_minor *minor);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index e4a27e1..5710029 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1909,9 +1909,18 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
>  		intel_dig_port->set_infoframes = g4x_set_infoframes;
>  		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
>  	} else if (HAS_DDI(dev_priv)) {
> -		intel_dig_port->write_infoframe = hsw_write_infoframe;
> -		intel_dig_port->set_infoframes = hsw_set_infoframes;
> -		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
> +		if (intel_dig_port->lspcon.active) {
> +			intel_dig_port->write_infoframe =
> +						lspcon_write_infoframe;
> +			intel_dig_port->set_infoframes = lspcon_set_infoframes;
> +			intel_dig_port->infoframe_enabled =
> +						lspcon_infoframe_enabled;
> +		} else {
> +			intel_dig_port->set_infoframes = hsw_set_infoframes;
> +			intel_dig_port->infoframe_enabled =
> +						hsw_infoframe_enabled;
> +			intel_dig_port->write_infoframe = hsw_write_infoframe;
> +		}
>  	} else if (HAS_PCH_IBX(dev_priv)) {
>  		intel_dig_port->write_infoframe = ibx_write_infoframe;
>  		intel_dig_port->set_infoframes = ibx_set_infoframes;
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 93507c5..b4fcd30 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -31,6 +31,18 @@
>  #define LSPCON_VENDOR_PARADE_OUI 0x001CF8
>  #define LSPCON_VENDOR_MCA_OUI 0x0060AD
>  
> +/* AUX addresses to write AVI IF into */
> +#define LSPCON_MCA_AVI_IF_WRITE_OFFSET 0x5C0
> +#define LSPCON_MCA_AVI_IF_CTRL 0x5DF
> +#define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
> +#define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
> +
> +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
> +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
> +#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
> +#define LSPCON_PARADE_AVI_IF_STATUS 0x51F
> +#define  LSPCON_PARADE_AVI_IF_HANDLED (2 << 6)
> +
>  static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
>  {
>  	struct intel_digital_port *dig_port =
> @@ -227,6 +239,240 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
>  	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
>  }
>  
> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
> +			     struct intel_crtc_state *config)
> +{
> +	struct drm_display_info *info = &connector->display_info;
> +	struct drm_display_mode *mode = &config->base.adjusted_mode;
> +
> +	if (drm_mode_is_420_only(info, mode)) {
> +
> +		if (!connector->ycbcr_420_allowed) {
> +			DRM_ERROR("Platform doesn't support YCBCR420 output\n");
> +			return false;
> +		}
> +
> +		config->port_clock /= 2;
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
> +					       const uint8_t *buffer,
> +					       ssize_t len)
> +{
> +	u8 avi_if_ctrl;
> +	u8 avi_if_status;
> +	u8 count = 0;
> +	u8 retry = 5;
> +	u8 avi_buf[8] = {0, };
Initialize the first byte to 1, and you can remove the special case for count == 0?
> +	uint16_t reg;
> +	ssize_t ret;
> +
> +	while (count++ < 4) {
> +
> +		do {
> +			/* Is LSPCON FW ready */
> +			reg = LSPCON_PARADE_AVI_IF_CTRL;
> +			ret = drm_dp_dpcd_read(aux, reg, &avi_if_ctrl, 1);
> +			if (ret < 0) {
> +				DRM_ERROR("DPCD read failed, add:0x%x\n", reg);
> +				return false;
> +			}
> +
> +			if (avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)
> +				break;
> +			usleep_range(100, 200);
> +		} while (--retry);
> +
> +		if (!(avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)) {
> +			DRM_ERROR("LSPCON FW not ready for infoframes\n");
> +			return false;
> +		}
> +
> +		/*
> +		 * AVI Infoframe contains 31 bytes of data:
> +		 *	HB0 to HB2   (3 bytes header)
> +		 *	PB0 to PB27 (28 bytes data)
> +		 * As per Parade spec, while sending first block (8bytes),
> +		 * byte 0 is kept for request token no, and byte1 - byte7
> +		 * contain frame data. So we have to pack frame like this:
> +		 *	first block of 8 bytes: <token> <HB0-HB2> <PB0-PB3>
> +		 *	next 3 blocks: <PB4-PB27>
> +		 */
> +		if (count)
> +			memcpy(avi_buf, buffer + count * 8 - 1, 8);
> +		else {
> +			avi_buf[0] = 1;
> +			memcpy(&avi_buf[1], buffer, 7);
> +		}
^This won't work, I think.

for (count = 0; count < 4; count++)
> +
> +		/* Write 8 bytes of data at a time */
> +		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
> +		ret = drm_dp_dpcd_write(aux, reg, avi_buf, 8);
> +		if (ret < 0) {
> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> +			return false;
> +		}
> +
> +		/*
> +		 * While sending a block of 8 byes, we need to inform block
> +		 * number to FW, by programming bits[1:0] of ctrl reg with
> +		 * block number
> +		 */
> +		avi_if_ctrl = 0x80 + count;
> +		reg = LSPCON_PARADE_AVI_IF_CTRL;
> +		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
> +		if (ret < 0) {
> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> +			return false;
> +		}
> +	}
> +
> +	/* Check LSPCON FW status */
> +	reg = LSPCON_PARADE_AVI_IF_STATUS;
> +	ret = drm_dp_dpcd_read(aux, reg, &avi_if_status, 1);
> +	if (ret < 0) {
> +		DRM_ERROR("DPCD write failed, address 0x%x\n", reg);
> +		return false;
> +	}
> +
> +	if (avi_if_status & LSPCON_PARADE_AVI_IF_HANDLED)
> +		DRM_DEBUG_KMS("AVI IF handled by FW\n");
you should check for (avi_if_status & AVI_IF_STATUS_MASK) == IF_HANDLED,
afaict from specs, if the status completes with an error, you still report success here.

> +	return true;
> +}
> +
> +static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
> +					    const uint8_t *buffer, ssize_t len)
> +{
> +	int ret;
> +	uint32_t val = 0;
> +	uint16_t reg;
> +	const uint8_t *data = buffer;
> +
> +	reg = LSPCON_MCA_AVI_IF_WRITE_OFFSET;
> +	while (val < len) {
> +		ret = drm_dp_dpcd_write(aux, reg, (void *)data, 1);
> +		if (ret < 0) {
> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> +			return false;
> +		}
> +		val++; reg++; data++;
> +	}
> +
> +	val = 0;
> +	reg = LSPCON_MCA_AVI_IF_CTRL;
> +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
> +	if (ret < 0) {
> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> +		return false;
> +	}
> +
> +	/* Indicate LSPCON chip about infoframe, clear bit 1 and set bit 0 */
> +	val &= ~LSPCON_MCA_AVI_IF_HANDLED;
> +	val |= LSPCON_MCA_AVI_IF_KICKOFF;
> +
> +	ret = drm_dp_dpcd_write(aux, reg, &val, 1);
> +	if (ret < 0) {
> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> +		return false;
> +	}
> +
> +	val = 0;
> +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
> +	if (ret < 0) {
> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> +		return false;
> +	}
> +
> +	if (val == LSPCON_MCA_AVI_IF_HANDLED)
> +		DRM_DEBUG_KMS("AVI IF handled by FW\n");
> +
> +	return true;
> +}
> +
> +void lspcon_write_infoframe(struct drm_encoder *encoder,
> +			    const struct intel_crtc_state *crtc_state,
> +			    enum hdmi_infoframe_type type,
> +			    const void *frame, ssize_t len)
> +{
> +	bool ret;
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
> +
> +	/* LSPCON only needs AVI IF */
> +	if (type != HDMI_INFOFRAME_TYPE_AVI)
> +		return;
> +
> +	if (lspcon->vendor == LSPCON_VENDOR_MCA)
> +		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
> +						      frame, len);
> +	else
> +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
> +							 frame, len);
> +
> +	if (!ret)
> +		DRM_ERROR("Failed to write AVI infoframes\n");
Well, both infoframe write functions already report a DRM_ERROR, this print could be lower or removed. :)
> +	else
> +		DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
> +}
> +
> +void lspcon_set_infoframes(struct drm_encoder *encoder,
> +			   bool enable,
> +			   const struct intel_crtc_state *crtc_state,
> +			   const struct drm_connector_state *conn_state)
> +{
> +	ssize_t ret;
> +	union hdmi_infoframe frame;
> +	uint8_t buf[VIDEO_DIP_DATA_SIZE];
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +	struct intel_dp *intel_dp = &dig_port->dp;
> +	struct drm_connector *connector = &intel_dp->attached_connector->base;
> +	const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
> +	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
> +
> +	if (!crtc_state->lspcon_active) {
> +		DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
> +		return;
> +	}
> +
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> +						       mode, is_hdmi2_sink);
> +	if (ret < 0) {
> +		DRM_ERROR("couldn't fill AVI infoframe\n");
> +		return;
> +	}
> +
> +	if (crtc_state->ycbcr420)
> +		frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
> +	else
> +		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> +
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> +					   crtc_state->limited_color_range ?
> +					   HDMI_QUANTIZATION_RANGE_LIMITED :
> +					   HDMI_QUANTIZATION_RANGE_FULL,
> +					   false);
> +
> +	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
> +	if (ret < 0) {
> +		DRM_ERROR("Failed to pack AVI IF\n");
In general, it would be nice if we also get the returned error code for debugging. :)
> +		return;
> +	}
> +
> +	dig_port->write_infoframe(encoder, crtc_state, HDMI_INFOFRAME_TYPE_AVI,
> +				  buf, ret);
> +}
> +
> +bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
> +			      const struct intel_crtc_state *pipe_config)
> +{
> +	return enc_to_intel_lspcon(encoder)->active;
> +}
> +
>  void lspcon_resume(struct intel_lspcon *lspcon)
>  {
>  	enum drm_lspcon_mode expected_mode;


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

Re: [PATCH v2 7/7] drm/i915: YCBCR 420 support for LSPCON

[Maarten Lankhorst]

Op 09-08-17 om 08:46 schreef Shashank Sharma:
> LSPCON chips support YCBCR420 outputs. To be able to get
> YCBCR420 output from LSPCON chip, the source should:
> - Generate YCBCR444 HDMI output
> - Set AVI infoframes for a YCBCR420 output.
>
> Unlike Native HDMI 4:2:0 outputs, there is no need to
> reserve a scaler in pipe, LSPCON does the downsampling from
> YCBCR 4:4:4 to 4:2:0 on its own.
>
> LSPCON FW gets the information from AVI infoframes, and generates
> YCBCR420 output from a YCBCR444 input. This patch adds the necessary
> changes to drive YCBCR420 output from LSPCON based HDMI output.
>
> V2: Dont mess around with 8bpc and 12bpc clocks, its not reqd
>     for DP links. (Ville)
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++----
>  drivers/gpu/drm/i915/intel_dp.c      | 13 ++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     | 10 +++++++++-
>  drivers/gpu/drm/i915/intel_lspcon.c  |  2 ++
>  4 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5a89db1..eb1d93b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4487,7 +4487,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>  	 */
>  	need_scaling = src_w != dst_w || src_h != dst_h;
>  
> -	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
> +	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX &&
> +		!crtc_state->lspcon_active)
>  		need_scaling = true;
>  
>  	/*
> @@ -7970,9 +7971,15 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>  			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
>  
>  		if (config->ycbcr420) {
> -			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV |
> -				PIPEMISC_YUV420_ENABLE |
> -				PIPEMISC_YUV420_MODE_FULL_BLEND;
> +			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
> +			/*
> +			 * LSPCON doesn't need scaling/blending to be done in
> +			 * pipe. It just needs YCBCR444 input and proper AVI
> +			 * infoframes for 4:2:0 output enabling.
> +			 */
> +			if (!config->lspcon_active)
> +				val |= PIPEMISC_YUV420_ENABLE |
> +				       PIPEMISC_YUV420_MODE_FULL_BLEND;
>  		}
>  
>  		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d580243..b7774cc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1614,7 +1614,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> -	enum port port = dp_to_dig_port(intel_dp)->port;
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +	enum port port = dig_port->port;
> +	struct intel_lspcon *lspcon = &dig_port->lspcon;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
>  	struct intel_digital_connector_state *intel_conn_state =
> @@ -1635,6 +1637,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	common_len = intel_dp_common_len_rate_limit(intel_dp,
>  						    intel_dp->max_link_rate);
>  
> +	/* LSPCON needs special handling to drive YCBCR420 outputs */
> +	if (lspcon->active) {
> +		struct drm_connector *connector = &intel_connector->base;
> +
> +		pipe_config->lspcon_active = true;
> +		pipe_config->ycbcr420 = lspcon_ycbcr420_config(connector,
> +							       pipe_config);
> +	}
> +
>  	/* No common link rates between source and sink */
>  	WARN_ON(common_len <= 0);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 99eaab6..b3cbe2d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -783,6 +783,9 @@ struct intel_crtc_state {
>  
>  	/* output format is YCBCR 4:2:0 */
>  	bool ycbcr420;
> +
> +	/* LSPCON is active on port */
> +	bool lspcon_active;
>  };
>  
>  struct intel_crtc {
> @@ -1182,6 +1185,12 @@ static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
>  	return &enc_to_dig_port(encoder)->dp;
>  }
>  
> +static inline struct intel_lspcon *
> +enc_to_intel_lspcon(struct drm_encoder *encoder)
> +{
> +	return &enc_to_dig_port(encoder)->lspcon;
> +}
> +
>  static inline struct intel_digital_port *
>  dp_to_dig_port(struct intel_dp *intel_dp)
>  {
> @@ -1662,7 +1671,6 @@ void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
>  void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>  void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
>  
> -
>  /* intel_lvds.c */
>  void intel_lvds_init(struct drm_i915_private *dev_priv);
>  struct intel_encoder *intel_get_lvds_encoder(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index b4fcd30..8413a4c 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -504,6 +504,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>  	struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_connector *connector = &dp->attached_connector->base;
>  
>  	if (!IS_GEN9(dev_priv)) {
>  		DRM_ERROR("LSPCON is supported on GEN9 only\n");
> @@ -528,6 +529,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>  		return false;
>  	}
>  
> +	connector->ycbcr_420_allowed = true;
>  	lspcon->active = true;
>  	DRM_DEBUG_KMS("Success: LSPCON init\n");
>  	return true;

Patch looks good, if the previous patch is fixed up. :)

I'll wait for the resubmit.

~Maarten

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

Re: [PATCH v2 6/7] drm/i915: write AVI infoframes for LSPCON

[Ville Syrjälä]

On Wed, Nov 01, 2017 at 10:27:23AM +0100, Maarten Lankhorst wrote:
> Op 09-08-17 om 08:46 schreef Shashank Sharma:
> > To pass AVI infoframes from display controller to LSPCON, we
> > have to write infoframe packets into vendor specified AUX address,
> > in vendor specified way.
> >
> > Also, LSPCON vendors provide AUX offsets, to inform the LSPCON
> > chip that the AVI IF packets are written, so that the firmware
> > can pick it up and apply.
> >
> > This patch adds function to write AVI infoframes for both MCA as
> > well as Parade Tech LSPCON chips. These two vendors provide different
> > methods for writing infoframes, so this patch contains two different
> > functions, one for each.
> >
> > V2: Rebase
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > ---
> This patch will fail to compile without 7/7 applied:
> - enc_to_intel_lspcon missing.
> - crtc_state->lspcon_active missing.
> 
> >  drivers/gpu/drm/i915/intel_ddi.c    |  12 +-
> >  drivers/gpu/drm/i915/intel_drv.h    |  16 +++
> >  drivers/gpu/drm/i915/intel_hdmi.c   |  15 ++-
> >  drivers/gpu/drm/i915/intel_lspcon.c | 246 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 284 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 9384080..08f3567 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2193,6 +2193,15 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
> >  					pipe_config->shared_dpll,
> >  					intel_crtc_has_type(pipe_config,
> >  							    INTEL_OUTPUT_DP_MST));
> > +
> > +		if (pipe_config->lspcon_active) {
> > +			struct intel_digital_port *dig_port =
> > +					enc_to_dig_port(&encoder->base);
> > +
> > +			dig_port->set_infoframes(&encoder->base,
> > +				 pipe_config->has_infoframe,
> > +				 pipe_config, conn_state);
> > +		}
> >  	}
> >  	if (type == INTEL_OUTPUT_HDMI) {
> >  		intel_ddi_pre_enable_hdmi(encoder,
> > @@ -2734,8 +2743,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> >  	intel_encoder->cloneable = 0;
> >  
> > -	intel_infoframe_init(intel_dig_port);
> > -
> >  	if (init_dp) {
> >  		if (!intel_ddi_init_dp_connector(intel_dig_port))
> >  			goto err;
> > @@ -2765,6 +2772,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  				port_name(port));
> >  	}
> >  
> > +	intel_infoframe_init(intel_dig_port);
> >  	return;
> >  
> >  err:
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index adab635..99eaab6 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1052,6 +1052,10 @@ struct intel_lspcon {
> >  	bool active;
> >  	enum drm_lspcon_mode mode;
> >  	enum lspcon_vendor vendor;
> > +
> > +	void (*write_infoframe)(struct drm_encoder *encoder,
> > +				const struct intel_crtc_state *crtc_state,
> > +				union hdmi_infoframe *frame);
> >  };
> >  
> >  struct intel_digital_port {
> > @@ -1978,6 +1982,18 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
> >  bool lspcon_init(struct intel_digital_port *intel_dig_port);
> >  void lspcon_resume(struct intel_lspcon *lspcon);
> >  void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
> > +bool lspcon_ycbcr420_config(struct drm_connector *connector,
> > +			     struct intel_crtc_state *config);
> > +void lspcon_write_infoframe(struct drm_encoder *encoder,
> > +			    const struct intel_crtc_state *crtc_state,
> > +			    enum hdmi_infoframe_type type,
> > +			    const void *buf, ssize_t len);
> > +void lspcon_set_infoframes(struct drm_encoder *encoder,
> > +			   bool enable,
> > +			   const struct intel_crtc_state *crtc_state,
> > +			   const struct drm_connector_state *conn_state);
> > +bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
> > +			      const struct intel_crtc_state *pipe_config);
> >  
> >  /* intel_pipe_crc.c */
> >  int intel_pipe_crc_create(struct drm_minor *minor);
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index e4a27e1..5710029 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1909,9 +1909,18 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
> >  		intel_dig_port->set_infoframes = g4x_set_infoframes;
> >  		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
> >  	} else if (HAS_DDI(dev_priv)) {
> > -		intel_dig_port->write_infoframe = hsw_write_infoframe;
> > -		intel_dig_port->set_infoframes = hsw_set_infoframes;
> > -		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
> > +		if (intel_dig_port->lspcon.active) {
> > +			intel_dig_port->write_infoframe =
> > +						lspcon_write_infoframe;
> > +			intel_dig_port->set_infoframes = lspcon_set_infoframes;
> > +			intel_dig_port->infoframe_enabled =
> > +						lspcon_infoframe_enabled;
> > +		} else {
> > +			intel_dig_port->set_infoframes = hsw_set_infoframes;
> > +			intel_dig_port->infoframe_enabled =
> > +						hsw_infoframe_enabled;
> > +			intel_dig_port->write_infoframe = hsw_write_infoframe;
> > +		}
> >  	} else if (HAS_PCH_IBX(dev_priv)) {
> >  		intel_dig_port->write_infoframe = ibx_write_infoframe;
> >  		intel_dig_port->set_infoframes = ibx_set_infoframes;
> > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> > index 93507c5..b4fcd30 100644
> > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> > @@ -31,6 +31,18 @@
> >  #define LSPCON_VENDOR_PARADE_OUI 0x001CF8
> >  #define LSPCON_VENDOR_MCA_OUI 0x0060AD
> >  
> > +/* AUX addresses to write AVI IF into */
> > +#define LSPCON_MCA_AVI_IF_WRITE_OFFSET 0x5C0
> > +#define LSPCON_MCA_AVI_IF_CTRL 0x5DF
> > +#define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
> > +#define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
> > +
> > +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
> > +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
> > +#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
> > +#define LSPCON_PARADE_AVI_IF_STATUS 0x51F
> > +#define  LSPCON_PARADE_AVI_IF_HANDLED (2 << 6)
> > +
> >  static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
> >  {
> >  	struct intel_digital_port *dig_port =
> > @@ -227,6 +239,240 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
> >  	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
> >  }
> >  
> > +bool lspcon_ycbcr420_config(struct drm_connector *connector,
> > +			     struct intel_crtc_state *config)
> > +{
> > +	struct drm_display_info *info = &connector->display_info;
> > +	struct drm_display_mode *mode = &config->base.adjusted_mode;
> > +
> > +	if (drm_mode_is_420_only(info, mode)) {
> > +
> > +		if (!connector->ycbcr_420_allowed) {
> > +			DRM_ERROR("Platform doesn't support YCBCR420 output\n");
> > +			return false;
> > +		}
> > +
> > +		config->port_clock /= 2;
> > +		return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
> > +					       const uint8_t *buffer,
> > +					       ssize_t len)
> > +{
> > +	u8 avi_if_ctrl;
> > +	u8 avi_if_status;
> > +	u8 count = 0;
> > +	u8 retry = 5;
> > +	u8 avi_buf[8] = {0, };
> Initialize the first byte to 1, and you can remove the special case for count == 0?
> > +	uint16_t reg;
> > +	ssize_t ret;
> > +
> > +	while (count++ < 4) {
> > +
> > +		do {
> > +			/* Is LSPCON FW ready */
> > +			reg = LSPCON_PARADE_AVI_IF_CTRL;
> > +			ret = drm_dp_dpcd_read(aux, reg, &avi_if_ctrl, 1);
> > +			if (ret < 0) {
> > +				DRM_ERROR("DPCD read failed, add:0x%x\n", reg);
> > +				return false;
> > +			}
> > +
> > +			if (avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)
> > +				break;
> > +			usleep_range(100, 200);
> > +		} while (--retry);
> > +
> > +		if (!(avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)) {
> > +			DRM_ERROR("LSPCON FW not ready for infoframes\n");
> > +			return false;
> > +		}
> > +
> > +		/*
> > +		 * AVI Infoframe contains 31 bytes of data:
> > +		 *	HB0 to HB2   (3 bytes header)
> > +		 *	PB0 to PB27 (28 bytes data)
> > +		 * As per Parade spec, while sending first block (8bytes),
> > +		 * byte 0 is kept for request token no, and byte1 - byte7
> > +		 * contain frame data. So we have to pack frame like this:
> > +		 *	first block of 8 bytes: <token> <HB0-HB2> <PB0-PB3>
> > +		 *	next 3 blocks: <PB4-PB27>
> > +		 */
> > +		if (count)
> > +			memcpy(avi_buf, buffer + count * 8 - 1, 8);
> > +		else {
> > +			avi_buf[0] = 1;
> > +			memcpy(&avi_buf[1], buffer, 7);
> > +		}
> ^This won't work, I think.
> 
> for (count = 0; count < 4; count++)
> > +
> > +		/* Write 8 bytes of data at a time */
> > +		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
> > +		ret = drm_dp_dpcd_write(aux, reg, avi_buf, 8);
> > +		if (ret < 0) {
> > +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> > +			return false;
> > +		}
> > +
> > +		/*
> > +		 * While sending a block of 8 byes, we need to inform block
> > +		 * number to FW, by programming bits[1:0] of ctrl reg with
> > +		 * block number
> > +		 */
> > +		avi_if_ctrl = 0x80 + count;
> > +		reg = LSPCON_PARADE_AVI_IF_CTRL;
> > +		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
> > +		if (ret < 0) {
> > +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> > +			return false;
> > +		}
> > +	}
> > +
> > +	/* Check LSPCON FW status */
> > +	reg = LSPCON_PARADE_AVI_IF_STATUS;
> > +	ret = drm_dp_dpcd_read(aux, reg, &avi_if_status, 1);
> > +	if (ret < 0) {
> > +		DRM_ERROR("DPCD write failed, address 0x%x\n", reg);
> > +		return false;
> > +	}
> > +
> > +	if (avi_if_status & LSPCON_PARADE_AVI_IF_HANDLED)
> > +		DRM_DEBUG_KMS("AVI IF handled by FW\n");
> you should check for (avi_if_status & AVI_IF_STATUS_MASK) == IF_HANDLED,
> afaict from specs, if the status completes with an error, you still report success here.
> 
> > +	return true;
> > +}
> > +
> > +static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
> > +					    const uint8_t *buffer, ssize_t len)
> > +{
> > +	int ret;
> > +	uint32_t val = 0;
> > +	uint16_t reg;
> > +	const uint8_t *data = buffer;
> > +
> > +	reg = LSPCON_MCA_AVI_IF_WRITE_OFFSET;
> > +	while (val < len) {
> > +		ret = drm_dp_dpcd_write(aux, reg, (void *)data, 1);
> > +		if (ret < 0) {
> > +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> > +			return false;
> > +		}
> > +		val++; reg++; data++;
> > +	}
> > +
> > +	val = 0;
> > +	reg = LSPCON_MCA_AVI_IF_CTRL;
> > +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
> > +	if (ret < 0) {
> > +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> > +		return false;
> > +	}
> > +
> > +	/* Indicate LSPCON chip about infoframe, clear bit 1 and set bit 0 */
> > +	val &= ~LSPCON_MCA_AVI_IF_HANDLED;
> > +	val |= LSPCON_MCA_AVI_IF_KICKOFF;
> > +
> > +	ret = drm_dp_dpcd_write(aux, reg, &val, 1);
> > +	if (ret < 0) {
> > +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> > +		return false;
> > +	}
> > +
> > +	val = 0;
> > +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
> > +	if (ret < 0) {
> > +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> > +		return false;
> > +	}
> > +
> > +	if (val == LSPCON_MCA_AVI_IF_HANDLED)
> > +		DRM_DEBUG_KMS("AVI IF handled by FW\n");
> > +
> > +	return true;
> > +}
> > +
> > +void lspcon_write_infoframe(struct drm_encoder *encoder,
> > +			    const struct intel_crtc_state *crtc_state,
> > +			    enum hdmi_infoframe_type type,
> > +			    const void *frame, ssize_t len)
> > +{
> > +	bool ret;
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
> > +
> > +	/* LSPCON only needs AVI IF */
> > +	if (type != HDMI_INFOFRAME_TYPE_AVI)
> > +		return;
> > +
> > +	if (lspcon->vendor == LSPCON_VENDOR_MCA)
> > +		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
> > +						      frame, len);
> > +	else
> > +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
> > +							 frame, len);
> > +
> > +	if (!ret)
> > +		DRM_ERROR("Failed to write AVI infoframes\n");
> Well, both infoframe write functions already report a DRM_ERROR, this print could be lower or removed. :)
> > +	else
> > +		DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
> > +}
> > +
> > +void lspcon_set_infoframes(struct drm_encoder *encoder,
> > +			   bool enable,
> > +			   const struct intel_crtc_state *crtc_state,
> > +			   const struct drm_connector_state *conn_state)
> > +{
> > +	ssize_t ret;
> > +	union hdmi_infoframe frame;
> > +	uint8_t buf[VIDEO_DIP_DATA_SIZE];
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > +	struct intel_dp *intel_dp = &dig_port->dp;
> > +	struct drm_connector *connector = &intel_dp->attached_connector->base;
> > +	const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
> > +	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
> > +
> > +	if (!crtc_state->lspcon_active) {
> > +		DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
> > +		return;
> > +	}
> > +
> > +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> > +						       mode, is_hdmi2_sink);
> > +	if (ret < 0) {
> > +		DRM_ERROR("couldn't fill AVI infoframe\n");
> > +		return;
> > +	}
> > +
> > +	if (crtc_state->ycbcr420)
> > +		frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
> > +	else
> > +		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> > +
> > +	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> > +					   crtc_state->limited_color_range ?
> > +					   HDMI_QUANTIZATION_RANGE_LIMITED :
> > +					   HDMI_QUANTIZATION_RANGE_FULL,
> > +					   false);
> > +
> > +	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
> > +	if (ret < 0) {
> > +		DRM_ERROR("Failed to pack AVI IF\n");
> In general, it would be nice if we also get the returned error code for debugging. :)

Speaking of infoframe related errors, I've started on a branch where I
move the infoframe setup/validation into .compute_config(). I guess I
should finish it and send it out.

> > +		return;
> > +	}
> > +
> > +	dig_port->write_infoframe(encoder, crtc_state, HDMI_INFOFRAME_TYPE_AVI,
> > +				  buf, ret);
> > +}
> > +
> > +bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
> > +			      const struct intel_crtc_state *pipe_config)
> > +{
> > +	return enc_to_intel_lspcon(encoder)->active;
> > +}
> > +
> >  void lspcon_resume(struct intel_lspcon *lspcon)
> >  {
> >  	enum drm_lspcon_mode expected_mode;
> 

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

Re: [PATCH v2 5/7] drm/i915: check LSPCON vendor OUI

[Sharma, Shashank]

Thanks for the review, Maarten.

My comments, inline.

Regards
Shashank
On 10/31/2017 2:30 PM, Maarten Lankhorst wrote:
> Hey,
>
> Op 09-08-17 om 08:46 schreef Shashank Sharma:
>> Intel LSPCON chip is provided by 2 vendors:
>> - Megachips America (MCA)
>> - Parade technologies (Parade tech)
>>
>> Its important to know the vendor of this chip, as the address to
>> write AVI infoframes is different for those two.
>>
>> This patch reads the vendor OUI signature, and marks into LSPCON
>> encoder structure for future usages.
>>
>> This patch also does a small re-arrangement of the code, by moving
>> lspcon mode change into probe function.
>>
>> V2: Use dp->desc for OUI detection, dont add a helper for this
>>      (Ville)
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_drv.h    |  6 ++++
>>   drivers/gpu/drm/i915/intel_lspcon.c | 69 +++++++++++++++++++++++++++++--------
>>   2 files changed, 61 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 7eadac0..adab635 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1043,9 +1043,15 @@ struct intel_dp {
>>   	struct intel_dp_compliance compliance;
>>   };
>>   
>> +enum lspcon_vendor {
>> +	LSPCON_VENDOR_MCA,
>> +	LSPCON_VENDOR_PARADE
>> +};
>> +
>>   struct intel_lspcon {
>>   	bool active;
>>   	enum drm_lspcon_mode mode;
>> +	enum lspcon_vendor vendor;
>>   };
>>   
>>   struct intel_digital_port {
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> index 5abef482..93507c5 100644
>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -27,6 +27,10 @@
>>   #include <drm/drm_dp_dual_mode_helper.h>
>>   #include "intel_drv.h"
>>   
>> +/* LSPCON OUI Vendor ID(signatures) */
>> +#define LSPCON_VENDOR_PARADE_OUI 0x001CF8
>> +#define LSPCON_VENDOR_MCA_OUI 0x0060AD
>> +
>>   static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
>>   {
>>   	struct intel_digital_port *dig_port =
>> @@ -50,6 +54,40 @@ static const char *lspcon_mode_name(enum drm_lspcon_mode mode)
>>   	}
>>   }
>>   
>> +static bool lspcon_detect_vendor(struct intel_lspcon *lspcon)
>> +{
>> +	struct intel_dp *dp = lspcon_to_intel_dp(lspcon);
>> +	struct drm_dp_dpcd_ident *ident;
>> +	u32 vendor_oui;
>> +
>> +	if (drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd))) {
>> +		DRM_ERROR("Can't read description\n");
>> +		return false;
>> +	}
>> +
>> +	ident = &dp->desc.ident;
>> +	vendor_oui = (ident->oui[0] << 16) | (ident->oui[1] << 8) |
>> +		      ident->oui[2];
>> +
>> +	switch (vendor_oui) {
>> +	case LSPCON_VENDOR_MCA_OUI:
>> +		lspcon->vendor = LSPCON_VENDOR_MCA;
>> +		DRM_DEBUG_KMS("Vendor: Mega Chips\n");
>> +		break;
>> +
>> +	case LSPCON_VENDOR_PARADE_OUI:
>> +		lspcon->vendor = LSPCON_VENDOR_PARADE;
>> +		DRM_DEBUG_KMS("Vendor: Parade Tech\n");
>> +		break;
>> +
>> +	default:
>> +		DRM_ERROR("Invalid/Unknown vendor OUI\n");
> MISSING_CASE(vendor_oui) ?
Sure, seems good.
>> +		return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>   static enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
>>   {
>>   	enum drm_lspcon_mode current_mode;
>> @@ -151,7 +189,18 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
>>   	/* Yay ... got a LSPCON device */
>>   	DRM_DEBUG_KMS("LSPCON detected\n");
>>   	lspcon->mode = lspcon_wait_mode(lspcon, expected_mode);
>> -	lspcon->active = true;
>> +
>> +	/*
>> +	 * In the SW state machine, lets Put LSPCON in PCON mode only.
>> +	 * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
>> +	 * 2.0 sinks.
>> +	 */
>> +	if (lspcon->active && lspcon->mode != DRM_LSPCON_MODE_PCON) {
>> +		if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
>> +			DRM_ERROR("LSPCON mode change to PCON failed\n");
>> +			return false;
>> +		}
>> +	}
>>   	return true;
>>   }
>>   
>> @@ -223,25 +272,17 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>>   		return false;
>>   	}
>>   
>> -	/*
>> -	* In the SW state machine, lets Put LSPCON in PCON mode only.
>> -	* In this way, it will work with both HDMI 1.4 sinks as well as HDMI
>> -	* 2.0 sinks.
>> -	*/
>> -	if (lspcon->active && lspcon->mode != DRM_LSPCON_MODE_PCON) {
>> -		if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
>> -			DRM_ERROR("LSPCON mode change to PCON failed\n");
>> -			return false;
>> -		}
>> -	}
>> -
>>   	if (!intel_dp_read_dpcd(dp)) {
>>   		DRM_ERROR("LSPCON DPCD read failed\n");
>>   		return false;
>>   	}
>>   
>> -	drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd));
>> +	if (!lspcon_detect_vendor(lspcon)) {
>> +		DRM_ERROR("LSPCON vendor detection failed\n");
>> +		return false;
>> +	}
> Error seems double here, lspcon_detect_vendor should already succeed.
>
> Same for lspcon_probe I think, better to upgrade the DRM_DEBUG_KMS failure there to DRM_ERROR there and remove the one from lspcon_init.
Yeah, seems like a good idea, initially I thought of dumping a specific 
and a generic error, but making generic error as _KMS sounds better.
- Shashank
>>   
>> +	lspcon->active = true;
>>   	DRM_DEBUG_KMS("Success: LSPCON init\n");
>>   	return true;
>>   }
>

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

Re: [PATCH v2 7/7] drm/i915: YCBCR 420 support for LSPCON

[Sharma, Shashank]

Regards

Shashank


On 11/1/2017 3:02 PM, Maarten Lankhorst wrote:
> Op 09-08-17 om 08:46 schreef Shashank Sharma:
>> LSPCON chips support YCBCR420 outputs. To be able to get
>> YCBCR420 output from LSPCON chip, the source should:
>> - Generate YCBCR444 HDMI output
>> - Set AVI infoframes for a YCBCR420 output.
>>
>> Unlike Native HDMI 4:2:0 outputs, there is no need to
>> reserve a scaler in pipe, LSPCON does the downsampling from
>> YCBCR 4:4:4 to 4:2:0 on its own.
>>
>> LSPCON FW gets the information from AVI infoframes, and generates
>> YCBCR420 output from a YCBCR444 input. This patch adds the necessary
>> changes to drive YCBCR420 output from LSPCON based HDMI output.
>>
>> V2: Dont mess around with 8bpc and 12bpc clocks, its not reqd
>>      for DP links. (Ville)
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++----
>>   drivers/gpu/drm/i915/intel_dp.c      | 13 ++++++++++++-
>>   drivers/gpu/drm/i915/intel_drv.h     | 10 +++++++++-
>>   drivers/gpu/drm/i915/intel_lspcon.c  |  2 ++
>>   4 files changed, 34 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 5a89db1..eb1d93b 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4487,7 +4487,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>>   	 */
>>   	need_scaling = src_w != dst_w || src_h != dst_h;
>>   
>> -	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
>> +	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX &&
>> +		!crtc_state->lspcon_active)
>>   		need_scaling = true;
>>   
>>   	/*
>> @@ -7970,9 +7971,15 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>>   			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
>>   
>>   		if (config->ycbcr420) {
>> -			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV |
>> -				PIPEMISC_YUV420_ENABLE |
>> -				PIPEMISC_YUV420_MODE_FULL_BLEND;
>> +			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
>> +			/*
>> +			 * LSPCON doesn't need scaling/blending to be done in
>> +			 * pipe. It just needs YCBCR444 input and proper AVI
>> +			 * infoframes for 4:2:0 output enabling.
>> +			 */
>> +			if (!config->lspcon_active)
>> +				val |= PIPEMISC_YUV420_ENABLE |
>> +				       PIPEMISC_YUV420_MODE_FULL_BLEND;
>>   		}
>>   
>>   		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index d580243..b7774cc 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1614,7 +1614,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>   	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>>   	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>> -	enum port port = dp_to_dig_port(intel_dp)->port;
>> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>> +	enum port port = dig_port->port;
>> +	struct intel_lspcon *lspcon = &dig_port->lspcon;
>>   	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>>   	struct intel_connector *intel_connector = intel_dp->attached_connector;
>>   	struct intel_digital_connector_state *intel_conn_state =
>> @@ -1635,6 +1637,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>   	common_len = intel_dp_common_len_rate_limit(intel_dp,
>>   						    intel_dp->max_link_rate);
>>   
>> +	/* LSPCON needs special handling to drive YCBCR420 outputs */
>> +	if (lspcon->active) {
>> +		struct drm_connector *connector = &intel_connector->base;
>> +
>> +		pipe_config->lspcon_active = true;
>> +		pipe_config->ycbcr420 = lspcon_ycbcr420_config(connector,
>> +							       pipe_config);
>> +	}
>> +
>>   	/* No common link rates between source and sink */
>>   	WARN_ON(common_len <= 0);
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 99eaab6..b3cbe2d 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -783,6 +783,9 @@ struct intel_crtc_state {
>>   
>>   	/* output format is YCBCR 4:2:0 */
>>   	bool ycbcr420;
>> +
>> +	/* LSPCON is active on port */
>> +	bool lspcon_active;
>>   };
>>   
>>   struct intel_crtc {
>> @@ -1182,6 +1185,12 @@ static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
>>   	return &enc_to_dig_port(encoder)->dp;
>>   }
>>   
>> +static inline struct intel_lspcon *
>> +enc_to_intel_lspcon(struct drm_encoder *encoder)
>> +{
>> +	return &enc_to_dig_port(encoder)->lspcon;
>> +}
>> +
>>   static inline struct intel_digital_port *
>>   dp_to_dig_port(struct intel_dp *intel_dp)
>>   {
>> @@ -1662,7 +1671,6 @@ void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
>>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>>   void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
>>   
>> -
>>   /* intel_lvds.c */
>>   void intel_lvds_init(struct drm_i915_private *dev_priv);
>>   struct intel_encoder *intel_get_lvds_encoder(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> index b4fcd30..8413a4c 100644
>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -504,6 +504,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>>   	struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>>   	struct drm_device *dev = intel_dig_port->base.base.dev;
>>   	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	struct drm_connector *connector = &dp->attached_connector->base;
>>   
>>   	if (!IS_GEN9(dev_priv)) {
>>   		DRM_ERROR("LSPCON is supported on GEN9 only\n");
>> @@ -528,6 +529,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>>   		return false;
>>   	}
>>   
>> +	connector->ycbcr_420_allowed = true;
>>   	lspcon->active = true;
>>   	DRM_DEBUG_KMS("Success: LSPCON init\n");
>>   	return true;
> Patch looks good, if the previous patch is fixed up. :)
>
> I'll wait for the resubmit.
>
> ~Maarten
Sure, Thanks !
- Shashank

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

Re: [PATCH v2 6/7] drm/i915: write AVI infoframes for LSPCON

[Sharma, Shashank]

Regards

Shashank


On 11/1/2017 2:57 PM, Maarten Lankhorst wrote:
> Op 09-08-17 om 08:46 schreef Shashank Sharma:
>> To pass AVI infoframes from display controller to LSPCON, we
>> have to write infoframe packets into vendor specified AUX address,
>> in vendor specified way.
>>
>> Also, LSPCON vendors provide AUX offsets, to inform the LSPCON
>> chip that the AVI IF packets are written, so that the firmware
>> can pick it up and apply.
>>
>> This patch adds function to write AVI infoframes for both MCA as
>> well as Parade Tech LSPCON chips. These two vendors provide different
>> methods for writing infoframes, so this patch contains two different
>> functions, one for each.
>>
>> V2: Rebase
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
> This patch will fail to compile without 7/7 applied:
> - enc_to_intel_lspcon missing.
> - crtc_state->lspcon_active missing.
Ahh, I might have sent an older version of _this_ patch. My bad.
>>   drivers/gpu/drm/i915/intel_ddi.c    |  12 +-
>>   drivers/gpu/drm/i915/intel_drv.h    |  16 +++
>>   drivers/gpu/drm/i915/intel_hdmi.c   |  15 ++-
>>   drivers/gpu/drm/i915/intel_lspcon.c | 246 ++++++++++++++++++++++++++++++++++++
>>   4 files changed, 284 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index 9384080..08f3567 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2193,6 +2193,15 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
>>   					pipe_config->shared_dpll,
>>   					intel_crtc_has_type(pipe_config,
>>   							    INTEL_OUTPUT_DP_MST));
>> +
>> +		if (pipe_config->lspcon_active) {
>> +			struct intel_digital_port *dig_port =
>> +					enc_to_dig_port(&encoder->base);
>> +
>> +			dig_port->set_infoframes(&encoder->base,
>> +				 pipe_config->has_infoframe,
>> +				 pipe_config, conn_state);
>> +		}
>>   	}
>>   	if (type == INTEL_OUTPUT_HDMI) {
>>   		intel_ddi_pre_enable_hdmi(encoder,
>> @@ -2734,8 +2743,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>>   	intel_encoder->cloneable = 0;
>>   
>> -	intel_infoframe_init(intel_dig_port);
>> -
>>   	if (init_dp) {
>>   		if (!intel_ddi_init_dp_connector(intel_dig_port))
>>   			goto err;
>> @@ -2765,6 +2772,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   				port_name(port));
>>   	}
>>   
>> +	intel_infoframe_init(intel_dig_port);
>>   	return;
>>   
>>   err:
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index adab635..99eaab6 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1052,6 +1052,10 @@ struct intel_lspcon {
>>   	bool active;
>>   	enum drm_lspcon_mode mode;
>>   	enum lspcon_vendor vendor;
>> +
>> +	void (*write_infoframe)(struct drm_encoder *encoder,
>> +				const struct intel_crtc_state *crtc_state,
>> +				union hdmi_infoframe *frame);
>>   };
>>   
>>   struct intel_digital_port {
>> @@ -1978,6 +1982,18 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
>>   bool lspcon_init(struct intel_digital_port *intel_dig_port);
>>   void lspcon_resume(struct intel_lspcon *lspcon);
>>   void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
>> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
>> +			     struct intel_crtc_state *config);
>> +void lspcon_write_infoframe(struct drm_encoder *encoder,
>> +			    const struct intel_crtc_state *crtc_state,
>> +			    enum hdmi_infoframe_type type,
>> +			    const void *buf, ssize_t len);
>> +void lspcon_set_infoframes(struct drm_encoder *encoder,
>> +			   bool enable,
>> +			   const struct intel_crtc_state *crtc_state,
>> +			   const struct drm_connector_state *conn_state);
>> +bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
>> +			      const struct intel_crtc_state *pipe_config);
>>   
>>   /* intel_pipe_crc.c */
>>   int intel_pipe_crc_create(struct drm_minor *minor);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index e4a27e1..5710029 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1909,9 +1909,18 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
>>   		intel_dig_port->set_infoframes = g4x_set_infoframes;
>>   		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
>>   	} else if (HAS_DDI(dev_priv)) {
>> -		intel_dig_port->write_infoframe = hsw_write_infoframe;
>> -		intel_dig_port->set_infoframes = hsw_set_infoframes;
>> -		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
>> +		if (intel_dig_port->lspcon.active) {
>> +			intel_dig_port->write_infoframe =
>> +						lspcon_write_infoframe;
>> +			intel_dig_port->set_infoframes = lspcon_set_infoframes;
>> +			intel_dig_port->infoframe_enabled =
>> +						lspcon_infoframe_enabled;
>> +		} else {
>> +			intel_dig_port->set_infoframes = hsw_set_infoframes;
>> +			intel_dig_port->infoframe_enabled =
>> +						hsw_infoframe_enabled;
>> +			intel_dig_port->write_infoframe = hsw_write_infoframe;
>> +		}
>>   	} else if (HAS_PCH_IBX(dev_priv)) {
>>   		intel_dig_port->write_infoframe = ibx_write_infoframe;
>>   		intel_dig_port->set_infoframes = ibx_set_infoframes;
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> index 93507c5..b4fcd30 100644
>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -31,6 +31,18 @@
>>   #define LSPCON_VENDOR_PARADE_OUI 0x001CF8
>>   #define LSPCON_VENDOR_MCA_OUI 0x0060AD
>>   
>> +/* AUX addresses to write AVI IF into */
>> +#define LSPCON_MCA_AVI_IF_WRITE_OFFSET 0x5C0
>> +#define LSPCON_MCA_AVI_IF_CTRL 0x5DF
>> +#define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
>> +#define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
>> +
>> +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
>> +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
>> +#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
>> +#define LSPCON_PARADE_AVI_IF_STATUS 0x51F
>> +#define  LSPCON_PARADE_AVI_IF_HANDLED (2 << 6)
>> +
>>   static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
>>   {
>>   	struct intel_digital_port *dig_port =
>> @@ -227,6 +239,240 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
>>   	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
>>   }
>>   
>> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
>> +			     struct intel_crtc_state *config)
>> +{
>> +	struct drm_display_info *info = &connector->display_info;
>> +	struct drm_display_mode *mode = &config->base.adjusted_mode;
>> +
>> +	if (drm_mode_is_420_only(info, mode)) {
>> +
>> +		if (!connector->ycbcr_420_allowed) {
>> +			DRM_ERROR("Platform doesn't support YCBCR420 output\n");
>> +			return false;
>> +		}
>> +
>> +		config->port_clock /= 2;
>> +		return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>> +static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
>> +					       const uint8_t *buffer,
>> +					       ssize_t len)
>> +{
>> +	u8 avi_if_ctrl;
>> +	u8 avi_if_status;
>> +	u8 count = 0;
>> +	u8 retry = 5;
>> +	u8 avi_buf[8] = {0, };
> Initialize the first byte to 1, and you can remove the special case for count == 0?
Actually the guidelines from Parade were not very clear on this, and I 
have recently heard of changes which are making token to be available 
for count = 2 also.
Please keep tuned *just* on this part, just keeping this comment on hold.
>> +	uint16_t reg;
>> +	ssize_t ret;
>> +
>> +	while (count++ < 4) {
>> +
>> +		do {
>> +			/* Is LSPCON FW ready */
>> +			reg = LSPCON_PARADE_AVI_IF_CTRL;
>> +			ret = drm_dp_dpcd_read(aux, reg, &avi_if_ctrl, 1);
>> +			if (ret < 0) {
>> +				DRM_ERROR("DPCD read failed, add:0x%x\n", reg);
>> +				return false;
>> +			}
>> +
>> +			if (avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)
>> +				break;
>> +			usleep_range(100, 200);
>> +		} while (--retry);
>> +
>> +		if (!(avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)) {
>> +			DRM_ERROR("LSPCON FW not ready for infoframes\n");
>> +			return false;
>> +		}
>> +
>> +		/*
>> +		 * AVI Infoframe contains 31 bytes of data:
>> +		 *	HB0 to HB2   (3 bytes header)
>> +		 *	PB0 to PB27 (28 bytes data)
>> +		 * As per Parade spec, while sending first block (8bytes),
>> +		 * byte 0 is kept for request token no, and byte1 - byte7
>> +		 * contain frame data. So we have to pack frame like this:
>> +		 *	first block of 8 bytes: <token> <HB0-HB2> <PB0-PB3>
>> +		 *	next 3 blocks: <PB4-PB27>
>> +		 */
>> +		if (count)
>> +			memcpy(avi_buf, buffer + count * 8 - 1, 8);
>> +		else {
>> +			avi_buf[0] = 1;
>> +			memcpy(&avi_buf[1], buffer, 7);
>> +		}
> ^This won't work, I think.
Why ?
>
> for (count = 0; count < 4; count++)
Yeah, this anyways seems like our general guidelines to go for for() 
when possible. I will change this.
>> +
>> +		/* Write 8 bytes of data at a time */
>> +		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
>> +		ret = drm_dp_dpcd_write(aux, reg, avi_buf, 8);
>> +		if (ret < 0) {
>> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
>> +			return false;
>> +		}
>> +
>> +		/*
>> +		 * While sending a block of 8 byes, we need to inform block
>> +		 * number to FW, by programming bits[1:0] of ctrl reg with
>> +		 * block number
>> +		 */
>> +		avi_if_ctrl = 0x80 + count;
>> +		reg = LSPCON_PARADE_AVI_IF_CTRL;
>> +		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
>> +		if (ret < 0) {
>> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
>> +			return false;
>> +		}
>> +	}
>> +
>> +	/* Check LSPCON FW status */
>> +	reg = LSPCON_PARADE_AVI_IF_STATUS;
>> +	ret = drm_dp_dpcd_read(aux, reg, &avi_if_status, 1);
>> +	if (ret < 0) {
>> +		DRM_ERROR("DPCD write failed, address 0x%x\n", reg);
>> +		return false;
>> +	}
>> +
>> +	if (avi_if_status & LSPCON_PARADE_AVI_IF_HANDLED)
>> +		DRM_DEBUG_KMS("AVI IF handled by FW\n");
> you should check for (avi_if_status & AVI_IF_STATUS_MASK) == IF_HANDLED,
> afaict from specs, if the status completes with an error, you still report success here.
Agree, will fix this.
>> +	return true;
>> +}
>> +
>> +static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
>> +					    const uint8_t *buffer, ssize_t len)
>> +{
>> +	int ret;
>> +	uint32_t val = 0;
>> +	uint16_t reg;
>> +	const uint8_t *data = buffer;
>> +
>> +	reg = LSPCON_MCA_AVI_IF_WRITE_OFFSET;
>> +	while (val < len) {
>> +		ret = drm_dp_dpcd_write(aux, reg, (void *)data, 1);
>> +		if (ret < 0) {
>> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
>> +			return false;
>> +		}
>> +		val++; reg++; data++;
>> +	}
>> +
>> +	val = 0;
>> +	reg = LSPCON_MCA_AVI_IF_CTRL;
>> +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
>> +	if (ret < 0) {
>> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
>> +		return false;
>> +	}
>> +
>> +	/* Indicate LSPCON chip about infoframe, clear bit 1 and set bit 0 */
>> +	val &= ~LSPCON_MCA_AVI_IF_HANDLED;
>> +	val |= LSPCON_MCA_AVI_IF_KICKOFF;
>> +
>> +	ret = drm_dp_dpcd_write(aux, reg, &val, 1);
>> +	if (ret < 0) {
>> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
>> +		return false;
>> +	}
>> +
>> +	val = 0;
>> +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
>> +	if (ret < 0) {
>> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
>> +		return false;
>> +	}
>> +
>> +	if (val == LSPCON_MCA_AVI_IF_HANDLED)
>> +		DRM_DEBUG_KMS("AVI IF handled by FW\n");
>> +
>> +	return true;
>> +}
>> +
>> +void lspcon_write_infoframe(struct drm_encoder *encoder,
>> +			    const struct intel_crtc_state *crtc_state,
>> +			    enum hdmi_infoframe_type type,
>> +			    const void *frame, ssize_t len)
>> +{
>> +	bool ret;
>> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>> +	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
>> +
>> +	/* LSPCON only needs AVI IF */
>> +	if (type != HDMI_INFOFRAME_TYPE_AVI)
>> +		return;
>> +
>> +	if (lspcon->vendor == LSPCON_VENDOR_MCA)
>> +		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
>> +						      frame, len);
>> +	else
>> +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
>> +							 frame, len);
>> +
>> +	if (!ret)
>> +		DRM_ERROR("Failed to write AVI infoframes\n");
> Well, both infoframe write functions already report a DRM_ERROR, this print could be lower or removed. :)
Agree.
>> +	else
>> +		DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
>> +}
>> +
>> +void lspcon_set_infoframes(struct drm_encoder *encoder,
>> +			   bool enable,
>> +			   const struct intel_crtc_state *crtc_state,
>> +			   const struct drm_connector_state *conn_state)
>> +{
>> +	ssize_t ret;
>> +	union hdmi_infoframe frame;
>> +	uint8_t buf[VIDEO_DIP_DATA_SIZE];
>> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>> +	struct intel_dp *intel_dp = &dig_port->dp;
>> +	struct drm_connector *connector = &intel_dp->attached_connector->base;
>> +	const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
>> +	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
>> +
>> +	if (!crtc_state->lspcon_active) {
>> +		DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
>> +		return;
>> +	}
>> +
>> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
>> +						       mode, is_hdmi2_sink);
>> +	if (ret < 0) {
>> +		DRM_ERROR("couldn't fill AVI infoframe\n");
>> +		return;
>> +	}
>> +
>> +	if (crtc_state->ycbcr420)
>> +		frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
>> +	else
>> +		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>> +
>> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
>> +					   crtc_state->limited_color_range ?
>> +					   HDMI_QUANTIZATION_RANGE_LIMITED :
>> +					   HDMI_QUANTIZATION_RANGE_FULL,
>> +					   false);
>> +
>> +	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
>> +	if (ret < 0) {
>> +		DRM_ERROR("Failed to pack AVI IF\n");
> In general, it would be nice if we also get the returned error code for debugging. :)
How about if we print the error code here, coz printing in the upper 
layer doesn't seem very relevant.
Or does it :) ?

- Shashank
>> +		return;
>> +	}
>> +
>> +	dig_port->write_infoframe(encoder, crtc_state, HDMI_INFOFRAME_TYPE_AVI,
>> +				  buf, ret);
>> +}
>> +
>> +bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
>> +			      const struct intel_crtc_state *pipe_config)
>> +{
>> +	return enc_to_intel_lspcon(encoder)->active;
>> +}
>> +
>>   void lspcon_resume(struct intel_lspcon *lspcon)
>>   {
>>   	enum drm_lspcon_mode expected_mode;
>

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

Re: [PATCH v2 6/7] drm/i915: write AVI infoframes for LSPCON

[Sharma, Shashank]

Regards

Shashank


On 11/1/2017 3:18 PM, Ville Syrjälä wrote:
> On Wed, Nov 01, 2017 at 10:27:23AM +0100, Maarten Lankhorst wrote:
>> Op 09-08-17 om 08:46 schreef Shashank Sharma:
>>> To pass AVI infoframes from display controller to LSPCON, we
>>> have to write infoframe packets into vendor specified AUX address,
>>> in vendor specified way.
>>>
>>> Also, LSPCON vendors provide AUX offsets, to inform the LSPCON
>>> chip that the AVI IF packets are written, so that the firmware
>>> can pick it up and apply.
>>>
>>> This patch adds function to write AVI infoframes for both MCA as
>>> well as Parade Tech LSPCON chips. These two vendors provide different
>>> methods for writing infoframes, so this patch contains two different
>>> functions, one for each.
>>>
>>> V2: Rebase
>>>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Imre Deak <imre.deak@intel.com>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> ---
>> This patch will fail to compile without 7/7 applied:
>> - enc_to_intel_lspcon missing.
>> - crtc_state->lspcon_active missing.
>>
>>>   drivers/gpu/drm/i915/intel_ddi.c    |  12 +-
>>>   drivers/gpu/drm/i915/intel_drv.h    |  16 +++
>>>   drivers/gpu/drm/i915/intel_hdmi.c   |  15 ++-
>>>   drivers/gpu/drm/i915/intel_lspcon.c | 246 ++++++++++++++++++++++++++++++++++++
>>>   4 files changed, 284 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index 9384080..08f3567 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -2193,6 +2193,15 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
>>>   					pipe_config->shared_dpll,
>>>   					intel_crtc_has_type(pipe_config,
>>>   							    INTEL_OUTPUT_DP_MST));
>>> +
>>> +		if (pipe_config->lspcon_active) {
>>> +			struct intel_digital_port *dig_port =
>>> +					enc_to_dig_port(&encoder->base);
>>> +
>>> +			dig_port->set_infoframes(&encoder->base,
>>> +				 pipe_config->has_infoframe,
>>> +				 pipe_config, conn_state);
>>> +		}
>>>   	}
>>>   	if (type == INTEL_OUTPUT_HDMI) {
>>>   		intel_ddi_pre_enable_hdmi(encoder,
>>> @@ -2734,8 +2743,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>>   	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>>>   	intel_encoder->cloneable = 0;
>>>   
>>> -	intel_infoframe_init(intel_dig_port);
>>> -
>>>   	if (init_dp) {
>>>   		if (!intel_ddi_init_dp_connector(intel_dig_port))
>>>   			goto err;
>>> @@ -2765,6 +2772,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>>   				port_name(port));
>>>   	}
>>>   
>>> +	intel_infoframe_init(intel_dig_port);
>>>   	return;
>>>   
>>>   err:
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index adab635..99eaab6 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -1052,6 +1052,10 @@ struct intel_lspcon {
>>>   	bool active;
>>>   	enum drm_lspcon_mode mode;
>>>   	enum lspcon_vendor vendor;
>>> +
>>> +	void (*write_infoframe)(struct drm_encoder *encoder,
>>> +				const struct intel_crtc_state *crtc_state,
>>> +				union hdmi_infoframe *frame);
>>>   };
>>>   
>>>   struct intel_digital_port {
>>> @@ -1978,6 +1982,18 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
>>>   bool lspcon_init(struct intel_digital_port *intel_dig_port);
>>>   void lspcon_resume(struct intel_lspcon *lspcon);
>>>   void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
>>> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
>>> +			     struct intel_crtc_state *config);
>>> +void lspcon_write_infoframe(struct drm_encoder *encoder,
>>> +			    const struct intel_crtc_state *crtc_state,
>>> +			    enum hdmi_infoframe_type type,
>>> +			    const void *buf, ssize_t len);
>>> +void lspcon_set_infoframes(struct drm_encoder *encoder,
>>> +			   bool enable,
>>> +			   const struct intel_crtc_state *crtc_state,
>>> +			   const struct drm_connector_state *conn_state);
>>> +bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
>>> +			      const struct intel_crtc_state *pipe_config);
>>>   
>>>   /* intel_pipe_crc.c */
>>>   int intel_pipe_crc_create(struct drm_minor *minor);
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index e4a27e1..5710029 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -1909,9 +1909,18 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
>>>   		intel_dig_port->set_infoframes = g4x_set_infoframes;
>>>   		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
>>>   	} else if (HAS_DDI(dev_priv)) {
>>> -		intel_dig_port->write_infoframe = hsw_write_infoframe;
>>> -		intel_dig_port->set_infoframes = hsw_set_infoframes;
>>> -		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
>>> +		if (intel_dig_port->lspcon.active) {
>>> +			intel_dig_port->write_infoframe =
>>> +						lspcon_write_infoframe;
>>> +			intel_dig_port->set_infoframes = lspcon_set_infoframes;
>>> +			intel_dig_port->infoframe_enabled =
>>> +						lspcon_infoframe_enabled;
>>> +		} else {
>>> +			intel_dig_port->set_infoframes = hsw_set_infoframes;
>>> +			intel_dig_port->infoframe_enabled =
>>> +						hsw_infoframe_enabled;
>>> +			intel_dig_port->write_infoframe = hsw_write_infoframe;
>>> +		}
>>>   	} else if (HAS_PCH_IBX(dev_priv)) {
>>>   		intel_dig_port->write_infoframe = ibx_write_infoframe;
>>>   		intel_dig_port->set_infoframes = ibx_set_infoframes;
>>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>>> index 93507c5..b4fcd30 100644
>>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>>> @@ -31,6 +31,18 @@
>>>   #define LSPCON_VENDOR_PARADE_OUI 0x001CF8
>>>   #define LSPCON_VENDOR_MCA_OUI 0x0060AD
>>>   
>>> +/* AUX addresses to write AVI IF into */
>>> +#define LSPCON_MCA_AVI_IF_WRITE_OFFSET 0x5C0
>>> +#define LSPCON_MCA_AVI_IF_CTRL 0x5DF
>>> +#define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
>>> +#define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
>>> +
>>> +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
>>> +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
>>> +#define  LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
>>> +#define LSPCON_PARADE_AVI_IF_STATUS 0x51F
>>> +#define  LSPCON_PARADE_AVI_IF_HANDLED (2 << 6)
>>> +
>>>   static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
>>>   {
>>>   	struct intel_digital_port *dig_port =
>>> @@ -227,6 +239,240 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
>>>   	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
>>>   }
>>>   
>>> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
>>> +			     struct intel_crtc_state *config)
>>> +{
>>> +	struct drm_display_info *info = &connector->display_info;
>>> +	struct drm_display_mode *mode = &config->base.adjusted_mode;
>>> +
>>> +	if (drm_mode_is_420_only(info, mode)) {
>>> +
>>> +		if (!connector->ycbcr_420_allowed) {
>>> +			DRM_ERROR("Platform doesn't support YCBCR420 output\n");
>>> +			return false;
>>> +		}
>>> +
>>> +		config->port_clock /= 2;
>>> +		return true;
>>> +	}
>>> +
>>> +	return false;
>>> +}
>>> +
>>> +static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
>>> +					       const uint8_t *buffer,
>>> +					       ssize_t len)
>>> +{
>>> +	u8 avi_if_ctrl;
>>> +	u8 avi_if_status;
>>> +	u8 count = 0;
>>> +	u8 retry = 5;
>>> +	u8 avi_buf[8] = {0, };
>> Initialize the first byte to 1, and you can remove the special case for count == 0?
>>> +	uint16_t reg;
>>> +	ssize_t ret;
>>> +
>>> +	while (count++ < 4) {
>>> +
>>> +		do {
>>> +			/* Is LSPCON FW ready */
>>> +			reg = LSPCON_PARADE_AVI_IF_CTRL;
>>> +			ret = drm_dp_dpcd_read(aux, reg, &avi_if_ctrl, 1);
>>> +			if (ret < 0) {
>>> +				DRM_ERROR("DPCD read failed, add:0x%x\n", reg);
>>> +				return false;
>>> +			}
>>> +
>>> +			if (avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)
>>> +				break;
>>> +			usleep_range(100, 200);
>>> +		} while (--retry);
>>> +
>>> +		if (!(avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)) {
>>> +			DRM_ERROR("LSPCON FW not ready for infoframes\n");
>>> +			return false;
>>> +		}
>>> +
>>> +		/*
>>> +		 * AVI Infoframe contains 31 bytes of data:
>>> +		 *	HB0 to HB2   (3 bytes header)
>>> +		 *	PB0 to PB27 (28 bytes data)
>>> +		 * As per Parade spec, while sending first block (8bytes),
>>> +		 * byte 0 is kept for request token no, and byte1 - byte7
>>> +		 * contain frame data. So we have to pack frame like this:
>>> +		 *	first block of 8 bytes: <token> <HB0-HB2> <PB0-PB3>
>>> +		 *	next 3 blocks: <PB4-PB27>
>>> +		 */
>>> +		if (count)
>>> +			memcpy(avi_buf, buffer + count * 8 - 1, 8);
>>> +		else {
>>> +			avi_buf[0] = 1;
>>> +			memcpy(&avi_buf[1], buffer, 7);
>>> +		}
>> ^This won't work, I think.
>>
>> for (count = 0; count < 4; count++)
>>> +
>>> +		/* Write 8 bytes of data at a time */
>>> +		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
>>> +		ret = drm_dp_dpcd_write(aux, reg, avi_buf, 8);
>>> +		if (ret < 0) {
>>> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
>>> +			return false;
>>> +		}
>>> +
>>> +		/*
>>> +		 * While sending a block of 8 byes, we need to inform block
>>> +		 * number to FW, by programming bits[1:0] of ctrl reg with
>>> +		 * block number
>>> +		 */
>>> +		avi_if_ctrl = 0x80 + count;
>>> +		reg = LSPCON_PARADE_AVI_IF_CTRL;
>>> +		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
>>> +		if (ret < 0) {
>>> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
>>> +			return false;
>>> +		}
>>> +	}
>>> +
>>> +	/* Check LSPCON FW status */
>>> +	reg = LSPCON_PARADE_AVI_IF_STATUS;
>>> +	ret = drm_dp_dpcd_read(aux, reg, &avi_if_status, 1);
>>> +	if (ret < 0) {
>>> +		DRM_ERROR("DPCD write failed, address 0x%x\n", reg);
>>> +		return false;
>>> +	}
>>> +
>>> +	if (avi_if_status & LSPCON_PARADE_AVI_IF_HANDLED)
>>> +		DRM_DEBUG_KMS("AVI IF handled by FW\n");
>> you should check for (avi_if_status & AVI_IF_STATUS_MASK) == IF_HANDLED,
>> afaict from specs, if the status completes with an error, you still report success here.
>>
>>> +	return true;
>>> +}
>>> +
>>> +static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
>>> +					    const uint8_t *buffer, ssize_t len)
>>> +{
>>> +	int ret;
>>> +	uint32_t val = 0;
>>> +	uint16_t reg;
>>> +	const uint8_t *data = buffer;
>>> +
>>> +	reg = LSPCON_MCA_AVI_IF_WRITE_OFFSET;
>>> +	while (val < len) {
>>> +		ret = drm_dp_dpcd_write(aux, reg, (void *)data, 1);
>>> +		if (ret < 0) {
>>> +			DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
>>> +			return false;
>>> +		}
>>> +		val++; reg++; data++;
>>> +	}
>>> +
>>> +	val = 0;
>>> +	reg = LSPCON_MCA_AVI_IF_CTRL;
>>> +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
>>> +	if (ret < 0) {
>>> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
>>> +		return false;
>>> +	}
>>> +
>>> +	/* Indicate LSPCON chip about infoframe, clear bit 1 and set bit 0 */
>>> +	val &= ~LSPCON_MCA_AVI_IF_HANDLED;
>>> +	val |= LSPCON_MCA_AVI_IF_KICKOFF;
>>> +
>>> +	ret = drm_dp_dpcd_write(aux, reg, &val, 1);
>>> +	if (ret < 0) {
>>> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
>>> +		return false;
>>> +	}
>>> +
>>> +	val = 0;
>>> +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
>>> +	if (ret < 0) {
>>> +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
>>> +		return false;
>>> +	}
>>> +
>>> +	if (val == LSPCON_MCA_AVI_IF_HANDLED)
>>> +		DRM_DEBUG_KMS("AVI IF handled by FW\n");
>>> +
>>> +	return true;
>>> +}
>>> +
>>> +void lspcon_write_infoframe(struct drm_encoder *encoder,
>>> +			    const struct intel_crtc_state *crtc_state,
>>> +			    enum hdmi_infoframe_type type,
>>> +			    const void *frame, ssize_t len)
>>> +{
>>> +	bool ret;
>>> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>> +	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
>>> +
>>> +	/* LSPCON only needs AVI IF */
>>> +	if (type != HDMI_INFOFRAME_TYPE_AVI)
>>> +		return;
>>> +
>>> +	if (lspcon->vendor == LSPCON_VENDOR_MCA)
>>> +		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
>>> +						      frame, len);
>>> +	else
>>> +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
>>> +							 frame, len);
>>> +
>>> +	if (!ret)
>>> +		DRM_ERROR("Failed to write AVI infoframes\n");
>> Well, both infoframe write functions already report a DRM_ERROR, this print could be lower or removed. :)
>>> +	else
>>> +		DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
>>> +}
>>> +
>>> +void lspcon_set_infoframes(struct drm_encoder *encoder,
>>> +			   bool enable,
>>> +			   const struct intel_crtc_state *crtc_state,
>>> +			   const struct drm_connector_state *conn_state)
>>> +{
>>> +	ssize_t ret;
>>> +	union hdmi_infoframe frame;
>>> +	uint8_t buf[VIDEO_DIP_DATA_SIZE];
>>> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>>> +	struct intel_dp *intel_dp = &dig_port->dp;
>>> +	struct drm_connector *connector = &intel_dp->attached_connector->base;
>>> +	const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
>>> +	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
>>> +
>>> +	if (!crtc_state->lspcon_active) {
>>> +		DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
>>> +		return;
>>> +	}
>>> +
>>> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
>>> +						       mode, is_hdmi2_sink);
>>> +	if (ret < 0) {
>>> +		DRM_ERROR("couldn't fill AVI infoframe\n");
>>> +		return;
>>> +	}
>>> +
>>> +	if (crtc_state->ycbcr420)
>>> +		frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
>>> +	else
>>> +		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>>> +
>>> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
>>> +					   crtc_state->limited_color_range ?
>>> +					   HDMI_QUANTIZATION_RANGE_LIMITED :
>>> +					   HDMI_QUANTIZATION_RANGE_FULL,
>>> +					   false);
>>> +
>>> +	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
>>> +	if (ret < 0) {
>>> +		DRM_ERROR("Failed to pack AVI IF\n");
>> In general, it would be nice if we also get the returned error code for debugging. :)
> Speaking of infoframe related errors, I've started on a branch where I
> move the infoframe setup/validation into .compute_config(). I guess I
> should finish it and send it out.
Sure, sounds like a good idea.
- Shashank
>>> +		return;
>>> +	}
>>> +
>>> +	dig_port->write_infoframe(encoder, crtc_state, HDMI_INFOFRAME_TYPE_AVI,
>>> +				  buf, ret);
>>> +}
>>> +
>>> +bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
>>> +			      const struct intel_crtc_state *pipe_config)
>>> +{
>>> +	return enc_to_intel_lspcon(encoder)->active;
>>> +}
>>> +
>>>   void lspcon_resume(struct intel_lspcon *lspcon)
>>>   {
>>>   	enum drm_lspcon_mode expected_mode;

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