intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON
@ 2018-02-07 10:17 Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx

This patch series adds YCBCR 4:2:0 output support for LSPCON displays.
In order to indicate the color format of output, to the LSPCON device,
a source has to set and send proper AVI infoframes to LSPCON. So this
patch series:
- introduces concept of CRTC output format.
- adds AVI infoframes support for LSPCON.
- then adds YCBCR 4:2:0 and 4:4:4 output support for LSPCON.

Previous versions of this series and its review can be found here:
https://patchwork.freedesktop.org/series/28536/
https://patchwork.freedesktop.org/series/33794/

In order to address review comment from V2, I have added 2 new patches
in this series, hence sent V3, and then V4 and to address additional
comments. V5 of the series adds a new patch (first patch) just to
introduce CRTC_OUTPUT_FORMAT concept, and later adds patches for 4:2:0
and 4:4:4 outputs. 

Sharma, Shashank (4):
  drm/i915: Check LSPCON vendor OUI
  drm/i915: Add AVI infoframe support for LSPCON
  drm/i915: Write AVI infoframes for MCA LSPCON
  drm/i915: Write AVI infoframes for Parade LSPCON

Shashank Sharma (4):
  drm/i915: Introduce CRTC output format
  drm/i915: Add CRTC output format YCBCR 4:2:0
  drm/i915: Add CRTC output format YCBCR 4:4:4
  drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON

 drivers/gpu/drm/i915/i915_reg.h      |   2 +
 drivers/gpu/drm/i915/intel_color.c   |   3 +-
 drivers/gpu/drm/i915/intel_crt.c     |   3 +
 drivers/gpu/drm/i915/intel_ddi.c     |  28 ++-
 drivers/gpu/drm/i915/intel_display.c |  81 ++++++---
 drivers/gpu/drm/i915/intel_dp.c      |   5 +
 drivers/gpu/drm/i915/intel_dp_mst.c  |   1 +
 drivers/gpu/drm/i915/intel_drv.h     |  39 +++-
 drivers/gpu/drm/i915/intel_dsi.c     |   1 +
 drivers/gpu/drm/i915/intel_dvo.c     |   1 +
 drivers/gpu/drm/i915/intel_hdmi.c    |  24 ++-
 drivers/gpu/drm/i915/intel_lspcon.c  | 341 +++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_lvds.c    |   2 +
 drivers/gpu/drm/i915/intel_panel.c   |   2 +-
 drivers/gpu/drm/i915/intel_sdvo.c    |   1 +
 drivers/gpu/drm/i915/intel_tv.c      |   1 +
 16 files changed, 483 insertions(+), 52 deletions(-)

-- 
2.7.4

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

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

* [PATCH v5 1/8] drm/i915: Introduce CRTC output format
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
@ 2018-02-07 10:17 ` Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0 Shashank Sharma
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx

This patch adds an enum "intel_output_format" to represent
the output format of a particular CRTC. This enum will be
used to produce a RGB/YCBCR4:4:4/YCBCR4:2:0 output format
during the atomic modeset calculations.

V5:
- Created this separate patch to introduce and init output_format.
- Initialize parameters of output_format_str respectively (Jani N).
- Call it intel_output_format than crtc_output_format(Ville).
- Set output format in pipe_config for every encoder (Ville).
- Get rid of extra DRM_DEBUG_KMS during get_pipe_config (Ville)

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      |  1 +
 drivers/gpu/drm/i915/intel_dp_mst.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h     |  8 ++++++++
 drivers/gpu/drm/i915/intel_dsi.c     |  1 +
 drivers/gpu/drm/i915/intel_dvo.c     |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c    |  1 +
 drivers/gpu/drm/i915/intel_lvds.c    |  2 ++
 drivers/gpu/drm/i915/intel_sdvo.c    |  1 +
 drivers/gpu/drm/i915/intel_tv.c      |  1 +
 11 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 1cd4a7c..270db38 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -337,6 +337,7 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
 				     struct intel_crtc_state *pipe_config,
 				     struct drm_connector_state *conn_state)
 {
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	return true;
 }
 
@@ -345,6 +346,7 @@ static bool pch_crt_compute_config(struct intel_encoder *encoder,
 				   struct drm_connector_state *conn_state)
 {
 	pipe_config->has_pch_encoder = true;
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	return true;
 }
@@ -356,6 +358,7 @@ static bool hsw_crt_compute_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
 	pipe_config->has_pch_encoder = true;
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	/* LPT FDI RX only supports 8bpc. */
 	if (HAS_PCH_LPT(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 60ba5bb..3c1c484 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9255,6 +9255,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		}
 	}
 
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
 	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
 		power_domain_mask |= BIT_ULL(power_domain);
@@ -10588,6 +10589,18 @@ static void snprintf_output_types(char *buf, size_t len,
 	WARN_ON_ONCE(output_types != 0);
 }
 
+static const char * const output_format_str[] = {
+	[INTEL_OUTPUT_FORMAT_INVALID] = "Invalid",
+	[INTEL_OUTPUT_FORMAT_RGB] = "RGB",
+};
+
+static const char *output_formats(enum intel_output_format format)
+{
+	if (format != INTEL_OUTPUT_FORMAT_RGB)
+		format = INTEL_OUTPUT_FORMAT_INVALID;
+	return output_format_str[format];
+}
+
 static void intel_dump_pipe_config(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config,
 				   const char *context)
@@ -10607,6 +10620,9 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("output_types: %s (0x%x)\n",
 		      buf, pipe_config->output_types);
 
+	DRM_DEBUG_KMS("output format: %s\n",
+		output_formats(pipe_config->output_format));
+
 	DRM_DEBUG_KMS("cpu_transcoder: %s, pipe bpp: %i, dithering: %i\n",
 		      transcoder_name(pipe_config->cpu_transcoder),
 		      pipe_config->pipe_bpp, pipe_config->dither);
@@ -11192,6 +11208,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
 	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_end);
 
 	PIPE_CONF_CHECK_I(pixel_multiplier);
+	PIPE_CONF_CHECK_I(output_format);
 	PIPE_CONF_CHECK_BOOL(has_hdmi_sink);
 	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f10a143..d9e3a8a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1706,6 +1706,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
 		pipe_config->has_pch_encoder = true;
 
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->has_drrs = false;
 	if (IS_G4X(dev_priv) || port == PORT_A)
 		pipe_config->has_audio = false;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index c3de091..5d7abbd 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -48,6 +48,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
 					   DP_DPCD_QUIRK_LIMITED_M_N);
 
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->has_pch_encoder = false;
 	bpp = 24;
 	if (intel_dp->compliance.test_data.bpc) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 468ec1e..ed0d070 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -691,6 +691,11 @@ struct intel_crtc_wm_state {
 	bool need_postvbl_update;
 };
 
+enum intel_output_format {
+	INTEL_OUTPUT_FORMAT_INVALID,
+	INTEL_OUTPUT_FORMAT_RGB,
+};
+
 struct intel_crtc_state {
 	struct drm_crtc_state base;
 
@@ -879,6 +884,9 @@ struct intel_crtc_state {
 
 	/* output format is YCBCR 4:2:0 */
 	bool ycbcr420;
+
+	/* Output format RGB/YCBCR etc */
+	enum intel_output_format output_format;
 };
 
 struct intel_crtc {
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 51a1d68..c815e67 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -314,6 +314,7 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 	int ret;
 
 	DRM_DEBUG_KMS("\n");
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	if (fixed_mode) {
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 59c066c..2545cf3 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -253,6 +253,7 @@ static bool intel_dvo_compute_config(struct intel_encoder *encoder,
 	if (fixed_mode)
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	return true;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f5d7bfb..7c09e42 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1677,6 +1677,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	int desired_bpp;
 	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
 
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->has_hdmi_sink = !force_dvi && intel_hdmi->has_hdmi_sink;
 
 	if (pipe_config->has_hdmi_sink)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index ef80499..dad6f8f 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -412,6 +412,8 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 		pipe_config->pipe_bpp = lvds_bpp;
 	}
 
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
+
 	/*
 	 * We have timings from the BIOS for the panel, put them in
 	 * to the adjusted mode.  The CRTC will be set up for this mode,
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 0bf97ed..7554ab1 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1128,6 +1128,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("forcing bpc to 8 for SDVO\n");
 	pipe_config->pipe_bpp = 8*3;
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 
 	if (HAS_PCH_SPLIT(to_i915(encoder->base.dev)))
 		pipe_config->has_pch_encoder = true;
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index b3dabc2..ebc77cf 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -883,6 +883,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
 	if (!tv_mode)
 		return false;
 
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->base.adjusted_mode.crtc_clock = tv_mode->clock;
 	DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
 	pipe_config->pipe_bpp = 8*3;
-- 
2.7.4

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

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

* [PATCH v5 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
@ 2018-02-07 10:17 ` Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 3/8] drm/i915: Add CRTC output format YCBCR 4:4:4 Shashank Sharma
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx

Currently, we are using a bool in CRTC state (state->ycbcr420),
to indicate modeset, that the output format is YCBCR 4:2:0. Now in
order to support other YCBCR formats, we will need more such flags.

This patch adds a new enum parameter for YCBCR 4:2:0 outputs, in the
CRTC output formats and then plugs it during the modeset.

V3: Added this patch in the series, to address review comments from
    second patchset.
V4: Added r-b from Maarten (on v3)
    Addressed review comments from Ville:
	- Change the enum name to intel_output_format from crtc_output_format.
	- Start the enum value (INVALID) from 0 instaed of 1.
        - Set the crtc's output_format to RGB in encoder's compute_config.
V5: Broke previous patch 1 into two parts,
    - first patch to add CRTC output format in general
    - second patch (this one) to add YCBCR 4:2:0 output
      format specifically.
- Use ARRAY_SIZE(format_str) for output format validity check (Ville)

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 52 ++++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |  4 +--
 drivers/gpu/drm/i915/intel_hdmi.c    |  6 ++---
 drivers/gpu/drm/i915/intel_panel.c   |  2 +-
 6 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index aa66e95..56e2824 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -141,7 +141,7 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 	uint16_t coeffs[9] = { 0, };
 	struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(crtc_state);
 
-	if (intel_crtc_state->ycbcr420) {
+	if (intel_crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
 		i9xx_load_ycbcr_conversion_matrix(intel_crtc);
 		return;
 	} else if (crtc_state->ctm) {
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cfcd9cb..5104256 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1264,7 +1264,7 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
 	else
 		dotclock = pipe_config->port_clock;
 
-	if (pipe_config->ycbcr420)
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 		dotclock *= 2;
 
 	if (pipe_config->pixel_multiplier)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3c1c484..850c407 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4674,7 +4674,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->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+	    scaler_user == SKL_CRTC_INDEX)
 		need_scaling = true;
 
 	/*
@@ -6388,7 +6389,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
-	if (pipe_config->ycbcr420 && pipe_config->base.ctm) {
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+	    pipe_config->base.ctm) {
 		/*
 		 * There is only one pipe CSC unit per pipe, and we need that
 		 * for output conversion from RGB->YCBCR. So if CTM is already
@@ -8218,10 +8220,10 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
 		if (intel_crtc->config->dither)
 			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
 
-		if (config->ycbcr420) {
-			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV |
-				PIPEMISC_YUV420_ENABLE |
-				PIPEMISC_YUV420_MODE_FULL_BLEND;
+		if (config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
+			val |= PIPEMISC_YUV420_ENABLE |
+			       PIPEMISC_YUV420_MODE_FULL_BLEND;
 		}
 
 		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
@@ -9240,22 +9242,28 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 
 	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
 		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
-		bool clrspace_yuv = tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV;
-
-		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) {
-			bool blend_mode_420 = tmp &
-					      PIPEMISC_YUV420_MODE_FULL_BLEND;
+		enum intel_output_format output = INTEL_OUTPUT_FORMAT_RGB;
+
+		if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) {
+			bool ycbcr420_enabled = tmp & PIPEMISC_YUV420_ENABLE;
+			bool blend = tmp & PIPEMISC_YUV420_MODE_FULL_BLEND;
+
+			if (ycbcr420_enabled) {
+				/* We support 4:2:0 in full blend mode only */
+				if (!blend)
+					output = INTEL_OUTPUT_FORMAT_INVALID;
+				else if (!(IS_GEMINILAKE(dev_priv) ||
+					   INTEL_GEN(dev_priv) >= 10))
+					output = INTEL_OUTPUT_FORMAT_INVALID;
+				else
+					output = INTEL_OUTPUT_FORMAT_YCBCR420;
+			}
 
-			pipe_config->ycbcr420 = tmp & PIPEMISC_YUV420_ENABLE;
-			if (pipe_config->ycbcr420 != clrspace_yuv ||
-			    pipe_config->ycbcr420 != blend_mode_420)
-				DRM_DEBUG_KMS("Bad 4:2:0 mode (%08x)\n", tmp);
-		} else if (clrspace_yuv) {
-			DRM_DEBUG_KMS("YCbCr 4:2:0 Unsupported\n");
 		}
+
+		pipe_config->output_format = output;
 	}
 
-	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
 	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
 		power_domain_mask |= BIT_ULL(power_domain);
@@ -10592,11 +10600,13 @@ static void snprintf_output_types(char *buf, size_t len,
 static const char * const output_format_str[] = {
 	[INTEL_OUTPUT_FORMAT_INVALID] = "Invalid",
 	[INTEL_OUTPUT_FORMAT_RGB] = "RGB",
+	[INTEL_OUTPUT_FORMAT_YCBCR420] = "YCBCR4:2:0",
 };
 
 static const char *output_formats(enum intel_output_format format)
 {
-	if (format != INTEL_OUTPUT_FORMAT_RGB)
+	if (format < INTEL_OUTPUT_FORMAT_RGB ||
+		format > ARRAY_SIZE(output_format_str))
 		format = INTEL_OUTPUT_FORMAT_INVALID;
 	return output_format_str[format];
 }
@@ -10632,9 +10642,6 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 				      pipe_config->fdi_lanes,
 				      &pipe_config->fdi_m_n);
 
-	if (pipe_config->ycbcr420)
-		DRM_DEBUG_KMS("YCbCr 4:2:0 output enabled\n");
-
 	if (intel_crtc_has_dp_encoder(pipe_config)) {
 		intel_dump_m_n_config(pipe_config, "dp m_n",
 				pipe_config->lane_count, &pipe_config->dp_m_n);
@@ -11217,7 +11224,6 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
 	PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
 	PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
 	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
-	PIPE_CONF_CHECK_BOOL(ycbcr420);
 
 	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ed0d070..bb82914 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -694,6 +694,7 @@ struct intel_crtc_wm_state {
 enum intel_output_format {
 	INTEL_OUTPUT_FORMAT_INVALID,
 	INTEL_OUTPUT_FORMAT_RGB,
+	INTEL_OUTPUT_FORMAT_YCBCR420,
 };
 
 struct intel_crtc_state {
@@ -882,9 +883,6 @@ struct intel_crtc_state {
 	/* HDMI High TMDS char rate ratio */
 	bool hdmi_high_tmds_clock_ratio;
 
-	/* output format is YCBCR 4:2:0 */
-	bool ycbcr420;
-
 	/* Output format RGB/YCBCR etc */
 	enum intel_output_format output_format;
 };
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 7c09e42..dbcc0e5 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -479,7 +479,7 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 		return;
 	}
 
-	if (crtc_state->ycbcr420)
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 		frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
 	else
 		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
@@ -1612,7 +1612,7 @@ static bool hdmi_12bpc_possible(const struct intel_crtc_state *crtc_state)
 		if (connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
-		if (crtc_state->ycbcr420) {
+		if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
 			const struct drm_hdmi_info *hdmi = &info->hdmi;
 
 			if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
@@ -1647,7 +1647,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
 	config->port_clock /= 2;
 	*clock_12bpc /= 2;
 	*clock_8bpc /= 2;
-	config->ycbcr420 = true;
+	config->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
 	/* YCBCR 420 output conversion needs a scaler */
 	if (skl_update_scaler_crtc(config)) {
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index e702a64..61b4547 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -111,7 +111,7 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 	/* Native modes don't need fitting */
 	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
-	    !pipe_config->ycbcr420)
+	    pipe_config->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
 		goto done;
 
 	switch (fitting_mode) {
-- 
2.7.4

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

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

* [PATCH v5 3/8] drm/i915: Add CRTC output format YCBCR 4:4:4
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0 Shashank Sharma
@ 2018-02-07 10:17 ` Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 4/8] drm/i915: Check LSPCON vendor OUI Shashank Sharma
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch adds support for YCBCR 4:4:4 CRTC output format.
To do this, this patch extends the existing YCBCR 4:2:0
framework by:
- Adding new parameter in for YCBCR 4:4:4 enum crtc_iutput_format.
- Adding case for YCBCR 4:4:4 in while setting AVI infoframes.
- Adding necessary checks in modeset sequence.

V3: Added this patch in the series
V4: Added r-b from Maarten (for v3)
    Addressed review comment from Ville:
    Do not use (config->output_format > CRTC_OUTPUT_RGB)
V5: Rebase

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   |  3 ++-
 drivers/gpu/drm/i915/intel_display.c | 13 +++++++++----
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c    |  2 ++
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 56e2824..e32dccd 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -141,7 +141,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 	uint16_t coeffs[9] = { 0, };
 	struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(crtc_state);
 
-	if (intel_crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+	if (intel_crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+	    intel_crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
 		i9xx_load_ycbcr_conversion_matrix(intel_crtc);
 		return;
 	} else if (crtc_state->ctm) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 850c407..35dd26c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6389,8 +6389,9 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
-	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
-	    pipe_config->base.ctm) {
+	if ((pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
+	     pipe_config->base.ctm) {
 		/*
 		 * There is only one pipe CSC unit per pipe, and we need that
 		 * for output conversion from RGB->YCBCR. So if CTM is already
@@ -8220,11 +8221,13 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
 		if (intel_crtc->config->dither)
 			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
 
-		if (config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+		if (config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+		    config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
 			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
+
+		if (config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 			val |= PIPEMISC_YUV420_ENABLE |
 			       PIPEMISC_YUV420_MODE_FULL_BLEND;
-		}
 
 		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
 	}
@@ -9257,6 +9260,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 					output = INTEL_OUTPUT_FORMAT_INVALID;
 				else
 					output = INTEL_OUTPUT_FORMAT_YCBCR420;
+			} else {
+				output = INTEL_OUTPUT_FORMAT_YCBCR444;
 			}
 
 		}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bb82914..87516b8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -695,6 +695,7 @@ enum intel_output_format {
 	INTEL_OUTPUT_FORMAT_INVALID,
 	INTEL_OUTPUT_FORMAT_RGB,
 	INTEL_OUTPUT_FORMAT_YCBCR420,
+	INTEL_OUTPUT_FORMAT_YCBCR444,
 };
 
 struct intel_crtc_state {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index dbcc0e5..049707b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -481,6 +481,8 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 
 	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
 		frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
+	else if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
+		frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
 	else
 		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
 
-- 
2.7.4

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

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

* [PATCH v5 4/8] drm/i915: Check LSPCON vendor OUI
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
                   ` (2 preceding siblings ...)
  2018-02-07 10:17 ` [PATCH v5 3/8] drm/i915: Add CRTC output format YCBCR 4:4:4 Shashank Sharma
@ 2018-02-07 10:17 ` Shashank Sharma
  2018-02-07 11:19   ` Jani Nikula
  2018-02-07 10:17 ` [PATCH v5 5/8] drm/i915: Add AVI infoframe support for LSPCON Shashank Sharma
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx

From: "Sharma, Shashank" <shashank.sharma@intel.com>

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)
V3: Rebase, Added r-b from Maarten
V4: Rebase
V5: Rebase

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Lankhorst <maarten.lankhorst@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 87516b8..0b46f09 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1137,9 +1137,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 8ae8f42..40929c2 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;
@@ -159,7 +197,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;
 }
 
@@ -230,25 +279,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

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

* [PATCH v5 5/8] drm/i915: Add AVI infoframe support for LSPCON
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
                   ` (3 preceding siblings ...)
  2018-02-07 10:17 ` [PATCH v5 4/8] drm/i915: Check LSPCON vendor OUI Shashank Sharma
@ 2018-02-07 10:17 ` Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 6/8] drm/i915: Write AVI infoframes for MCA LSPCON Shashank Sharma
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx; +Cc: Imre Deak

From: "Sharma, Shashank" <shashank.sharma@intel.com>

In order to pass AVI infoframes to LSPCON devices, a source has to
write them in a vendor recommended method and location.

This patch series:
- adds generic LSPCON infoframe setup functions.
- registers these functions into existing AVI infoframe framework.
- triggers these functions from modeset sequence.

Next patches in the series will add vendor specific code.

V2: Added new parameter to align with new definition of
    drm_hdmi_avi_infoframe_quant_range
V3: Added r-b from Maarten (for V2)
    Added new parameter output_format in struct lspcon to accommodate
    Ville's review comments on last patch of the series
V4: Addressed Ville's review comment
    - Do not add output_format in LSPCON state, as its non-atomic. Add
      this into CRTC state (added in a later patch).
V5: Rebase

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    | 19 +++++++++++---
 drivers/gpu/drm/i915/intel_drv.h    | 13 +++++++++-
 drivers/gpu/drm/i915/intel_hdmi.c   | 13 +++++++---
 drivers/gpu/drm/i915/intel_lspcon.c | 49 +++++++++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 5104256..caa16e9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2267,10 +2267,22 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
-	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		intel_ddi_pre_enable_hdmi(encoder, crtc_state, conn_state);
-	else
+	} else {
+		struct intel_lspcon *lspcon =
+				enc_to_intel_lspcon(&encoder->base);
+
 		intel_ddi_pre_enable_dp(encoder, crtc_state, conn_state);
+		if (lspcon->active) {
+			struct intel_digital_port *dig_port =
+					enc_to_dig_port(&encoder->base);
+
+			dig_port->set_infoframes(&encoder->base,
+						 crtc_state->has_infoframe,
+						 crtc_state, conn_state);
+		}
+	}
 }
 
 static void intel_disable_ddi_buf(struct intel_encoder *encoder)
@@ -2974,8 +2986,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;
@@ -3005,6 +3015,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 0b46f09..30a4b23 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1271,6 +1271,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)
 {
@@ -1787,7 +1793,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);
@@ -2128,6 +2133,12 @@ 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);
+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 049707b..b7cfbf0 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2287,9 +2287,16 @@ 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->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 40929c2..56526eb 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -234,6 +234,55 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
 	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\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_lspcon *lspcon = &dig_port->lspcon;
+	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 (!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;
+	}
+
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
+					   crtc_state->limited_color_range ?
+					   HDMI_QUANTIZATION_RANGE_LIMITED :
+					   HDMI_QUANTIZATION_RANGE_FULL,
+					   false, is_hdmi2_sink);
+
+	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

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

* [PATCH v5 6/8] drm/i915: Write AVI infoframes for MCA LSPCON
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
                   ` (4 preceding siblings ...)
  2018-02-07 10:17 ` [PATCH v5 5/8] drm/i915: Add AVI infoframe support for LSPCON Shashank Sharma
@ 2018-02-07 10:17 ` Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 7/8] drm/i915: Write AVI infoframes for Parade LSPCON Shashank Sharma
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx

From: "Sharma, Shashank" <shashank.sharma@intel.com>

As LSPCON is a DP branch device, LSPCON vendors define
specific methods to pass AVI infoframes to the the chip.
This patch adds:
- a generic wrapper function for writing AVI infoframes for
  all LSPCON devices.
- a vendor specific function to wrire AVI infoframes into
  MCA LSPCON devices.

V2: Rebase
V3: Added r-b from Maarten
V4: Rebase
V5: Rebase

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  4 ++
 drivers/gpu/drm/i915/intel_hdmi.c   |  2 +
 drivers/gpu/drm/i915/intel_lspcon.c | 80 +++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 30a4b23..062e3e4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2133,6 +2133,10 @@ 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);
+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,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index b7cfbf0..a665adb 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2288,6 +2288,8 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
 		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
 	} else if (HAS_DDI(dev_priv)) {
 		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;
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 56526eb..149fdf1 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -31,6 +31,12 @@
 #define LSPCON_VENDOR_PARADE_OUI 0x001CF8
 #define LSPCON_VENDOR_MCA_OUI 0x0060AD
 
+/* AUX addresses to write MCA AVI IF */
+#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)
+
 static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
 {
 	struct intel_digital_port *dig_port =
@@ -234,6 +240,80 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
 	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
 }
 
+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 = true;
+	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);
+	if (!ret) {
+		DRM_ERROR("Failed to write AVI infoframes\n");
+		return;
+	}
+
+	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,
-- 
2.7.4

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

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

* [PATCH v5 7/8] drm/i915: Write AVI infoframes for Parade LSPCON
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
                   ` (5 preceding siblings ...)
  2018-02-07 10:17 ` [PATCH v5 6/8] drm/i915: Write AVI infoframes for MCA LSPCON Shashank Sharma
@ 2018-02-07 10:17 ` Shashank Sharma
  2018-02-07 10:17 ` [PATCH v5 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON Shashank Sharma
  2018-02-07 10:57 ` ✗ Fi.CI.BAT: warning for YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3) Patchwork
  8 siblings, 0 replies; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx

From: "Sharma, Shashank" <shashank.sharma@intel.com>

Different LSPCON vendors specify their custom methods to pass
AVI infoframes to the LSPCON chip, so does Parade tech.

This patch adds functions to arrange and write AVI infoframes
into Parade LSPCON chips.

V2: rebase
V3: Added r-b from Maarten
V4: rebase
V5: rebase

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_lspcon.c | 119 +++++++++++++++++++++++++++++++++++-
 1 file changed, 118 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 149fdf1..c7755ee 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -37,6 +37,12 @@
 #define  LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
 #define  LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
 
+/* AUX addresses to write Parade AVI IF */
+#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_DATA_SIZE 32
+
 static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
 {
 	struct intel_digital_port *dig_port =
@@ -240,6 +246,113 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
 	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
 }
 
+static bool lspcon_parade_fw_ready(struct drm_dp_aux *aux)
+{
+	u8 avi_if_ctrl;
+	u8 retry;
+	ssize_t ret;
+
+	/* Check if LSPCON FW is ready for data */
+	for (retry = 0; retry < 5; retry++) {
+
+		if (retry)
+			usleep_range(200, 300);
+
+		ret = drm_dp_dpcd_read(aux, LSPCON_PARADE_AVI_IF_CTRL,
+				       &avi_if_ctrl, 1);
+		if (ret < 0) {
+			DRM_ERROR("Failed to read AVI IF control\n");
+			return false;
+		}
+
+		if ((avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF) == 0)
+			return true;
+	}
+
+	DRM_ERROR("Parade FW not ready to accept AVI IF\n");
+	return false;
+}
+
+static bool _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
+					       uint8_t *avi_buf)
+{
+	u8 avi_if_ctrl;
+	u8 block_count = 0;
+	u8 *data;
+	uint16_t reg;
+	ssize_t ret;
+
+	while (block_count < 4) {
+
+		if (!lspcon_parade_fw_ready(aux)) {
+			DRM_DEBUG_KMS("LSPCON FW not ready, block %d\n",
+				       block_count);
+			return false;
+		}
+
+		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
+		data = avi_buf + block_count * 8;
+		ret = drm_dp_dpcd_write(aux, reg, data, 8);
+		if (ret < 0) {
+			DRM_ERROR("Failed to write AVI IF block %d\n",
+				   block_count);
+			return false;
+		}
+
+		/*
+		 * Once a block of data is written, we have to inform the FW
+		 * about this by writing into avi infoframe control register:
+		 * - set the kickoff bit[7] to 1
+		 * - write the block no. to bits[1:0]
+		 */
+		reg = LSPCON_PARADE_AVI_IF_CTRL;
+		avi_if_ctrl = LSPCON_PARADE_AVI_IF_KICKOFF | block_count;
+		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
+		if (ret < 0) {
+			DRM_ERROR("Failed to update (0x%x), block %d\n",
+					reg, block_count);
+			return false;
+		}
+
+		block_count++;
+	}
+
+	DRM_DEBUG_KMS("Wrote AVI IF blocks successfully\n");
+	return true;
+}
+
+static bool _lspcon_write_avi_infoframe_parade(struct drm_dp_aux *aux,
+					       const uint8_t *frame,
+					       ssize_t len)
+{
+	uint8_t avi_if[LSPCON_PARADE_AVI_IF_DATA_SIZE] = {1, };
+
+	/*
+	 * Parade's frames contains 32 bytes of data, divided
+	 * into 4 frames:
+	 *	Token byte (first byte of first frame, must be non-zero)
+	 *	HB0 to HB2	 from AVI IF (3 bytes header)
+	 *	PB0 to PB27 from AVI IF (28 bytes data)
+	 * So it should look like this
+	 *	first block: | <token> <HB0-HB2> <DB0-DB3> |
+	 *	next 3 blocks: |<DB4-DB11>|<DB12-DB19>|<DB20-DB28>|
+	 */
+
+	if (len > LSPCON_PARADE_AVI_IF_DATA_SIZE - 1) {
+		DRM_ERROR("Invalid length of infoframes\n");
+		return false;
+	}
+
+	memcpy(&avi_if[1], frame, len);
+
+	if (!_lspcon_parade_write_infoframe_blocks(aux, avi_if)) {
+		DRM_DEBUG_KMS("Failed to write infoframe blocks\n");
+		return false;
+	}
+
+	return true;
+}
+
 static bool _lspcon_write_avi_infoframe_mca(struct drm_dp_aux *aux,
 					     const uint8_t *buffer, ssize_t len)
 {
@@ -294,7 +407,7 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
 			     enum hdmi_infoframe_type type,
 			     const void *frame, ssize_t len)
 {
-	bool ret = true;
+	bool ret;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
 
@@ -305,6 +418,10 @@ void lspcon_write_infoframe(struct drm_encoder *encoder,
 	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");
 		return;
-- 
2.7.4

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

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

* [PATCH v5 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
                   ` (6 preceding siblings ...)
  2018-02-07 10:17 ` [PATCH v5 7/8] drm/i915: Write AVI infoframes for Parade LSPCON Shashank Sharma
@ 2018-02-07 10:17 ` Shashank Sharma
  2018-02-07 10:57 ` ✗ Fi.CI.BAT: warning for YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3) Patchwork
  8 siblings, 0 replies; 14+ messages in thread
From: Shashank Sharma @ 2018-02-07 10:17 UTC (permalink / raw)
  To: intel-gfx

LSPCON chips can generate YCBCR outputs, if asked nicely :).

In order to generate YCBCR 4:2:0 outputs, a source must:
- send YCBCR 4:4:4 signals to LSPCON
- program color space as 4:2:0 in AVI infoframes

Whereas for YCBCR 4:4:4 outputs, the source must:
- send YCBCR 4:4:4 signals to LSPCON
- program color space as 4:4:4 in AVI infoframes

So for both 4:2:0 as well as 4:4:4 outputs, we are driving the
pipe for YCBCR 4:4:4 output, but AVI infoframe's color space
information indicates LSPCON FW to start scaling down from YCBCR
4:4:4 and generate YCBCR 4:2:0 output. As the scaling is done by
LSPCON device, we need not to reserve a scaler for 4:2:0 outputs.

V2: rebase
V3: Addressed review comments from Ville
    - add enum crtc_output_format instead of bool ycbcr420
    - use crtc_output_format=4:4:4 for modeset of LSPCON 4:2:0 output
      cases in this way we will have YCBCR 4:4:4 framework ready (except
      the ABI part)
V4: Added r-b from Maarten (for v3)
    Addressed review comments from Ville:
    - Do not add a non-atomic state variable to determine lspcon output.
      Instead add bool in CRTC state to indicate lspcon based scaling.
V5: Addressed review comments from Ville:
    - Change the state bool name from external scaling to something more
      relavent.
    - Keep the info and adjusted_mode structures const.
    - use crtc_state instead of pipe_config.
    - Push all the config change into lspcon_ycbcr420_config function.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_ddi.c     |  7 +++++++
 drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
 drivers/gpu/drm/i915/intel_dp.c      |  4 ++++
 drivers/gpu/drm/i915/intel_drv.h     |  5 +++++
 drivers/gpu/drm/i915/intel_lspcon.c  | 26 ++++++++++++++++++++++++++
 6 files changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e9c79b5..61ef124 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8766,6 +8766,8 @@ enum skl_power_gate {
 #define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC)
 
 #define  TRANS_MSA_SYNC_CLK		(1<<0)
+#define  TRANS_MSA_SAMPLING_444        (2<<1)
+#define  TRANS_MSA_CLRSP_YCBCR		(2<<3)
 #define  TRANS_MSA_6_BPC		(0<<5)
 #define  TRANS_MSA_8_BPC		(1<<5)
 #define  TRANS_MSA_10_BPC		(2<<5)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index caa16e9..540d465 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1499,6 +1499,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
 		break;
 	}
 
+	/*
+	 * As per DP 1.2 spec section 2.3.4.3 while sending
+	 * YCBCR 444 signals we should program MSA MISC1/0 fields with
+	 * colorspace information. The output colorspace encoding is BT601.
+	 */
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
+		temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
 	I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 35dd26c..26418e1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9243,6 +9243,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->gamma_mode =
 		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
 
+	pipe_config->lspcon_downsampling = false;
 	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
 		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
 		enum intel_output_format output = INTEL_OUTPUT_FORMAT_RGB;
@@ -9261,6 +9262,14 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 				else
 					output = INTEL_OUTPUT_FORMAT_YCBCR420;
 			} else {
+				/*
+				 * As there is no interface defined (yet)
+				 * to get the user's preference for output
+				 * format, YCBCR444 output format is only
+				 * possible with LSPCON YCBCR420 output,
+				 * which uses LSPCON's (external )scaler.
+				 */
+				pipe_config->lspcon_downsampling = true;
 				output = INTEL_OUTPUT_FORMAT_YCBCR444;
 			}
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d9e3a8a..469dc05 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1677,6 +1677,7 @@ 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);
+	struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
 	enum port port = encoder->port;
 	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
@@ -1707,6 +1708,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		pipe_config->has_pch_encoder = true;
 
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
+	if (lspcon->active)
+		lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
+
 	pipe_config->has_drrs = false;
 	if (IS_G4X(dev_priv) || port == PORT_A)
 		pipe_config->has_audio = false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 062e3e4..40127ac 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -886,6 +886,9 @@ struct intel_crtc_state {
 
 	/* Output format RGB/YCBCR etc */
 	enum intel_output_format output_format;
+
+	/* Output down scaling is done in LSPCON device */
+	bool lspcon_downsampling;
 };
 
 struct intel_crtc {
@@ -2143,6 +2146,8 @@ void lspcon_set_infoframes(struct drm_encoder *encoder,
 			   const struct drm_connector_state *conn_state);
 bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
 			      const struct intel_crtc_state *pipe_config);
+void lspcon_ycbcr420_config(struct drm_connector *connector,
+			    struct intel_crtc_state *crtc_state);
 
 /* intel_pipe_crc.c */
 int intel_pipe_crc_create(struct drm_minor *minor);
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index c7755ee..61f2840 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -180,6 +180,21 @@ static bool lspcon_wake_native_aux_ch(struct intel_lspcon *lspcon)
 	return true;
 }
 
+void lspcon_ycbcr420_config(struct drm_connector *connector,
+			    struct intel_crtc_state *crtc_state)
+{
+	const struct drm_display_info *info = &connector->display_info;
+	const struct drm_display_mode *adjusted_mode =
+					&crtc_state->base.adjusted_mode;
+
+	if (drm_mode_is_420_only(info, adjusted_mode) &&
+	    connector->ycbcr_420_allowed) {
+		crtc_state->port_clock /= 2;
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444;
+		crtc_state->lspcon_downsampling = true;
+	}
+}
+
 static bool lspcon_probe(struct intel_lspcon *lspcon)
 {
 	int retry;
@@ -458,6 +473,15 @@ void lspcon_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) {
+		if (crtc_state->lspcon_downsampling)
+			frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
+		else
+			frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
+	} 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 :
@@ -511,6 +535,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 (!HAS_LSPCON(dev_priv)) {
 		DRM_ERROR("LSPCON is not supported on this platform\n");
@@ -535,6 +560,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

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

* ✗ Fi.CI.BAT: warning for YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3)
  2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
                   ` (7 preceding siblings ...)
  2018-02-07 10:17 ` [PATCH v5 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON Shashank Sharma
@ 2018-02-07 10:57 ` Patchwork
  2018-02-27 10:08   ` Ville Syrjälä
  8 siblings, 1 reply; 14+ messages in thread
From: Patchwork @ 2018-02-07 10:57 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: intel-gfx

== Series Details ==

Series: YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3)
URL   : https://patchwork.freedesktop.org/series/36068/
State : warning

== Summary ==

Series 36068v3 YCBCR 4:2:0/4:4:4 output support for LSPCON
https://patchwork.freedesktop.org/api/1.0/series/36068/revisions/3/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> DMESG-WARN (fi-elk-e7500) fdo#103989 +11
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-bwr-2160)
Test gem_busy:
        Subgroup basic-hang-default:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-pnv-d510)
Test gem_exec_fence:
        Subgroup await-hang-default:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#104572
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600) fdo#103880
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-hsw-4770) fdo#103375 +3
                pass       -> DMESG-WARN (fi-bsw-n3050)
                pass       -> DMESG-WARN (fi-kbl-7567u)
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-hsw-4770)
                pass       -> DMESG-WARN (fi-bsw-n3050)
Test gem_sync:
        Subgroup basic-all:
                skip       -> PASS       (fi-pnv-d510)
        Subgroup basic-each:
                skip       -> PASS       (fi-pnv-d510)
        Subgroup basic-many-each:
                skip       -> PASS       (fi-pnv-d510)
        Subgroup basic-store-all:
                skip       -> PASS       (fi-pnv-d510)
        Subgroup basic-store-each:
                skip       -> PASS       (fi-pnv-d510)
Test gem_tiled_blits:
        Subgroup basic:
                skip       -> PASS       (fi-pnv-d510)
Test gem_tiled_fence_blits:
        Subgroup basic:
                skip       -> PASS       (fi-pnv-d510)
Test gem_wait:
        Subgroup basic-busy-all:
                skip       -> PASS       (fi-pnv-d510)
        Subgroup basic-wait-all:
                skip       -> PASS       (fi-pnv-d510)
        Subgroup basic-await-all:
                skip       -> PASS       (fi-pnv-d510)
Test kms_busy:
        Subgroup basic-flip-a:
                pass       -> DMESG-WARN (fi-blb-e6850)
                skip       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-bwr-2160)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-hsw-4770)
        Subgroup basic-flip-b:
                pass       -> DMESG-WARN (fi-blb-e6850)
                skip       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-bwr-2160) fdo#103182 +1
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-hsw-4770)
        Subgroup basic-flip-c:
WARNING: Long output truncated
fi-gdg-551 failed to collect. IGT log at Patchwork_7919/fi-gdg-551/igt.log

e5f22cbeec1da222b22367ee3ac165188fb2a36d drm-tip: 2018y-02m-07d-08h-09m-07s UTC integration manifest
b59091d0ff1e drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON
7b6f79b2e3fc drm/i915: Write AVI infoframes for Parade LSPCON
4b43699703b3 drm/i915: Write AVI infoframes for MCA LSPCON
a7fa7986ecb7 drm/i915: Add AVI infoframe support for LSPCON
e8cf87702de5 drm/i915: Check LSPCON vendor OUI
b821737fad2e drm/i915: Add CRTC output format YCBCR 4:4:4
b2fe4b2ff326 drm/i915: Add CRTC output format YCBCR 4:2:0
c681a49823c7 drm/i915: Introduce CRTC output format

== Logs ==

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

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

* Re: [PATCH v5 4/8] drm/i915: Check LSPCON vendor OUI
  2018-02-07 10:17 ` [PATCH v5 4/8] drm/i915: Check LSPCON vendor OUI Shashank Sharma
@ 2018-02-07 11:19   ` Jani Nikula
  2018-02-07 12:42     ` Sharma, Shashank
  0 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2018-02-07 11:19 UTC (permalink / raw)
  To: Shashank Sharma, intel-gfx

On Wed, 07 Feb 2018, Shashank Sharma <shashank.sharma@intel.com> wrote:
> From: "Sharma, Shashank" <shashank.sharma@intel.com>
>
> 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)
> V3: Rebase, Added r-b from Maarten
> V4: Rebase
> V5: Rebase
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Lankhorst <maarten.lankhorst@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 87516b8..0b46f09 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1137,9 +1137,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 8ae8f42..40929c2 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;

Sorry to jump in at such a late stage of review, but I'm a bit uneasy
about this. I wonder if we should just default to, say, MCA behaviour
when we don't identify the chip. Or skip the vendor specific parts.

Just generally limp along as much as we can instead of detecting some
sort of LSPCON but failing to use it because we can't identify it.

BR,
Jani.


> +	}
> +
> +	return true;
> +}
> +
>  static enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
>  {
>  	enum drm_lspcon_mode current_mode;
> @@ -159,7 +197,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;
>  }
>  
> @@ -230,25 +279,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;
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v5 4/8] drm/i915: Check LSPCON vendor OUI
  2018-02-07 11:19   ` Jani Nikula
@ 2018-02-07 12:42     ` Sharma, Shashank
  0 siblings, 0 replies; 14+ messages in thread
From: Sharma, Shashank @ 2018-02-07 12:42 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx

Hello Jani,

Thanks for the review comments, mine inline.

Regards
Shashank
On 2/7/2018 4:49 PM, Jani Nikula wrote:
> On Wed, 07 Feb 2018, Shashank Sharma <shashank.sharma@intel.com> wrote:
>> From: "Sharma, Shashank" <shashank.sharma@intel.com>
>>
>> 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)
>> V3: Rebase, Added r-b from Maarten
>> V4: Rebase
>> V5: Rebase
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Reviewed-by: Lankhorst <maarten.lankhorst@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 87516b8..0b46f09 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1137,9 +1137,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 8ae8f42..40929c2 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;
> Sorry to jump in at such a late stage of review, but I'm a bit uneasy
> about this. I wonder if we should just default to, say, MCA behaviour
> when we don't identify the chip. Or skip the vendor specific parts.
>
> Just generally limp along as much as we can instead of detecting some
> sort of LSPCON but failing to use it because we can't identify it.
On the first thought, I really liked this idea, that probably we can 
drive minimum stuff assuming a MCA lspcon, instead of failing it 
altogether,  But It might be difficult what to define as minimum 
features. We can surely not drive YCBCR outputs or HDR stuff, as 
anything which is dependent on AVI infoframes, becomes vendor specific. 
Also there are few type-2 dongles which register themselves as generic 
dp-hdmi adapters, and they do not implement the stuff as per the spec. 
If we drive them forward, we never know where are we going to fail, and 
that might be difficult to debug at that stage. I remember debugging of 
one such type-1 HDMI jitter handler adapter with a Lenovo laptop, which 
was causing chaos as it dint implement the stuff properly.

But at the same time I was thinking can we drive them with a warning, or 
debug message which says we are trying our best but this device is 
non-reliable .... you think so ?

- Shashank
> BR,
> Jani.
>
>
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>   static enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
>>   {
>>   	enum drm_lspcon_mode current_mode;
>> @@ -159,7 +197,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;
>>   }
>>   
>> @@ -230,25 +279,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;
>>   }

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

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

* Re: ✗ Fi.CI.BAT: warning for YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3)
  2018-02-07 10:57 ` ✗ Fi.CI.BAT: warning for YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3) Patchwork
@ 2018-02-27 10:08   ` Ville Syrjälä
  2018-03-20 15:31     ` Ville Syrjälä
  0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2018-02-27 10:08 UTC (permalink / raw)
  To: intel-gfx

On Wed, Feb 07, 2018 at 10:57:59AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3)
> URL   : https://patchwork.freedesktop.org/series/36068/
> State : warning
> 
> == Summary ==
> 
> Series 36068v3 YCBCR 4:2:0/4:4:4 output support for LSPCON
> https://patchwork.freedesktop.org/api/1.0/series/36068/revisions/3/mbox/
> 
> Test debugfs_test:
>         Subgroup read_all_entries:
>                 pass       -> DMESG-WARN (fi-elk-e7500) fdo#103989 +11
>                 pass       -> DMESG-WARN (fi-ilk-650)
>                 pass       -> DMESG-WARN (fi-snb-2600)
>                 pass       -> DMESG-WARN (fi-byt-j1900)
>                 pass       -> DMESG-WARN (fi-byt-n2820)
>                 pass       -> DMESG-WARN (fi-bsw-n3050)

Looks like all of these are due to missing readout in
i9xx/ilk_get_pipe_config().

>         Subgroup basic-s4-devices:
>                 pass       -> DMESG-WARN (fi-blb-e6850)
>                 pass       -> DMESG-WARN (fi-pnv-d510)
>                 pass       -> DMESG-WARN (fi-ilk-650)
>                 pass       -> DMESG-WARN (fi-snb-2520m)
>                 pass       -> DMESG-WARN (fi-snb-2600)
>                 pass       -> DMESG-WARN (fi-ivb-3520m)
>                 pass       -> DMESG-WARN (fi-ivb-3770)
>                 pass       -> DMESG-WARN (fi-byt-j1900)
>                 pass       -> DMESG-WARN (fi-byt-n2820)
>                 pass       -> DMESG-WARN (fi-hsw-4770)

The hsw case must be something a bit more subtle since you did add
something to hsw_get_pipe_config(). I can't immediately see what would
cause this. Probably best to fix the other platforms first and get
a new set of results with the extra noise removed, and then we can see
if you hit the same warn on the other platforms as you have on hsw.

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

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

* Re: ✗ Fi.CI.BAT: warning for YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3)
  2018-02-27 10:08   ` Ville Syrjälä
@ 2018-03-20 15:31     ` Ville Syrjälä
  0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2018-03-20 15:31 UTC (permalink / raw)
  To: intel-gfx

On Tue, Feb 27, 2018 at 12:08:25PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 07, 2018 at 10:57:59AM -0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3)
> > URL   : https://patchwork.freedesktop.org/series/36068/
> > State : warning
> > 
> > == Summary ==
> > 
> > Series 36068v3 YCBCR 4:2:0/4:4:4 output support for LSPCON
> > https://patchwork.freedesktop.org/api/1.0/series/36068/revisions/3/mbox/
> > 
> > Test debugfs_test:
> >         Subgroup read_all_entries:
> >                 pass       -> DMESG-WARN (fi-elk-e7500) fdo#103989 +11
> >                 pass       -> DMESG-WARN (fi-ilk-650)
> >                 pass       -> DMESG-WARN (fi-snb-2600)
> >                 pass       -> DMESG-WARN (fi-byt-j1900)
> >                 pass       -> DMESG-WARN (fi-byt-n2820)
> >                 pass       -> DMESG-WARN (fi-bsw-n3050)
> 
> Looks like all of these are due to missing readout in
> i9xx/ilk_get_pipe_config().

OK, just to be clear. I don't think there's much use in me looking at
this stuff until this obvious fail is fixed. So fix this and repost
whatever changed, and then we can see what fails remain.

> 
> >         Subgroup basic-s4-devices:
> >                 pass       -> DMESG-WARN (fi-blb-e6850)
> >                 pass       -> DMESG-WARN (fi-pnv-d510)
> >                 pass       -> DMESG-WARN (fi-ilk-650)
> >                 pass       -> DMESG-WARN (fi-snb-2520m)
> >                 pass       -> DMESG-WARN (fi-snb-2600)
> >                 pass       -> DMESG-WARN (fi-ivb-3520m)
> >                 pass       -> DMESG-WARN (fi-ivb-3770)
> >                 pass       -> DMESG-WARN (fi-byt-j1900)
> >                 pass       -> DMESG-WARN (fi-byt-n2820)
> >                 pass       -> DMESG-WARN (fi-hsw-4770)
> 
> The hsw case must be something a bit more subtle since you did add
> something to hsw_get_pipe_config(). I can't immediately see what would
> cause this. Probably best to fix the other platforms first and get
> a new set of results with the extra noise removed, and then we can see
> if you hit the same warn on the other platforms as you have on hsw.
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

end of thread, other threads:[~2018-03-20 15:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-07 10:17 [PATCH v5 0/8] YCBCR 4:2:0/4:4:4 output support for LSPCON Shashank Sharma
2018-02-07 10:17 ` [PATCH v5 1/8] drm/i915: Introduce CRTC output format Shashank Sharma
2018-02-07 10:17 ` [PATCH v5 2/8] drm/i915: Add CRTC output format YCBCR 4:2:0 Shashank Sharma
2018-02-07 10:17 ` [PATCH v5 3/8] drm/i915: Add CRTC output format YCBCR 4:4:4 Shashank Sharma
2018-02-07 10:17 ` [PATCH v5 4/8] drm/i915: Check LSPCON vendor OUI Shashank Sharma
2018-02-07 11:19   ` Jani Nikula
2018-02-07 12:42     ` Sharma, Shashank
2018-02-07 10:17 ` [PATCH v5 5/8] drm/i915: Add AVI infoframe support for LSPCON Shashank Sharma
2018-02-07 10:17 ` [PATCH v5 6/8] drm/i915: Write AVI infoframes for MCA LSPCON Shashank Sharma
2018-02-07 10:17 ` [PATCH v5 7/8] drm/i915: Write AVI infoframes for Parade LSPCON Shashank Sharma
2018-02-07 10:17 ` [PATCH v5 8/8] drm/i915: Add YCBCR 4:2:0/4:4:4 support for LSPCON Shashank Sharma
2018-02-07 10:57 ` ✗ Fi.CI.BAT: warning for YCBCR 4:2:0/4:4:4 output support for LSPCON (rev3) Patchwork
2018-02-27 10:08   ` Ville Syrjälä
2018-03-20 15:31     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).