[PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer

[PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer

[Shashank Sharma]

Following YCBCR 4:4:4 and 4:2:2, YCBCR 4:2:0 is a new output format,
which is currently supported on HDMI 2.0 sources/sinks. Due to lower
chroma sub-sampling rate, YCBCR 4:2:0 can drive the video modes at half
the pixel clock than YCBCR 4:4:4 or RGB 8:8:8 outputs. For example, a CEA
4K@60, RGB 8:8:8 mode needs a clock of appx 594Mhz, but it can be driven at
297Mhz using YCBCR 4:2:0 output.

Of course, the lower rate of chroma subsampling, causes the quality of YCBCR
4:2:0 to be lower than YCBCR 4:4:4 or RGB 8:8:8.

This patch series adds support for YCBCR 4:2:0 output in DRM layer.

- First 2 patches, complete the CEA mode-db in drm driver, by adding
  new 4k modes. Current CEA mode-db contains 64 modes only (VIC 1-64),
  whereas CEA-861-F defined VICs up to 107, including 4k modes, from VIC
  range 93-107. First patch makes sure that inclusion of these modes doesn't
  break existing HDMI 1.4 monitors, across various drivers.

- Next 5 patches focus on parsing new YCBCR 4:2:0 EDID blocks, and adding
  YCBCR 4:2:0 modes in connector. They also contain a prune function, which
  will cleanup the YCBCR 4:2:0 modes from list, if the connector doesn't
  declare them supported.

- Next 2 patches add helper functions for identifing YCBCR 4:2:0 modes and
  setup the color space in AVI infoframes.

- Next 6 patches add code for I915 layer handling of YCBCR 4:2:0 output.

This patch series was initially published as a complete framework to handle
all YCBCR outputs (4:4:4, 4:2:2, 4:2:0), but based on the code reviews, now
its been published as YCBCR 4:2:0 handling series only.

The previous discussion and reviews can be found here:
    V5: https://patchwork.freedesktop.org/series/26815/
    V1-V4: https://patchwork.freedesktop.org/series/22683/

Now re-publishing this patch series as YCBCR 4:2:0 handling series here.
V2: Addressed review comments from Ville 
This series dropped one patch in V2(patch 8 from V1), so 14 patches in V2

This series has been tested with drm-tip code with following setup:
Source: Intel Geminilake device.
Sink: ACER S277HK HDMI 2.0 monitor.
Protocol testing: Astro VA-1844A HDMI analyzer.

Shashank Sharma (14):
  drm: handle HDMI 2.0 VICs in AVI info-frames
  drm/edid: complete CEA modedb(VIC 1-107)
  drm/edid: parse sink information before CEA blocks
  drm/edid: cleanup patch for CEA extended-tag macro
  drm: add helper to validate YCBCR420 modes
  drm/edid: parse YCBCR420 videomodes from EDID
  drm/edid: parse ycbcr 420 deep color information
  drm: add helper functions for YCBCR420 handling
  drm/i915: add config function for YCBCR420 outputs
  drm/i915: prepare scaler for YCBCR420 modeset
  drm/i915: prepare pipe for YCBCR420 output
  drm/i915: prepare csc unit for YCBCR420 output
  drm/i915: set colorspace for YCBCR420 outputs
  drm/i915/glk: set HDMI 2.0 identifier

 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   2 +-
 drivers/gpu/drm/bridge/analogix-anx78xx.c |   3 +-
 drivers/gpu/drm/bridge/sii902x.c          |   2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |   2 +-
 drivers/gpu/drm/drm_edid.c                | 438 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_modes.c               |  87 ++++++
 drivers/gpu/drm/drm_probe_helper.c        |   4 +
 drivers/gpu/drm/exynos/exynos_hdmi.c      |   2 +-
 drivers/gpu/drm/i2c/tda998x_drv.c         |   2 +-
 drivers/gpu/drm/i915/i915_reg.h           |   3 +
 drivers/gpu/drm/i915/intel_color.c        |  46 +++-
 drivers/gpu/drm/i915/intel_display.c      |  26 ++
 drivers/gpu/drm/i915/intel_drv.h          |   7 +-
 drivers/gpu/drm/i915/intel_hdmi.c         |  69 ++++-
 drivers/gpu/drm/i915/intel_panel.c        |   3 +-
 drivers/gpu/drm/i915/intel_sdvo.c         |   3 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c       |   2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |   2 +-
 drivers/gpu/drm/nouveau/nv50_display.c    |   3 +-
 drivers/gpu/drm/omapdrm/omap_encoder.c    |   3 +-
 drivers/gpu/drm/radeon/radeon_audio.c     |   2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c      |   2 +-
 drivers/gpu/drm/sti/sti_hdmi.c            |   2 +-
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |   2 +-
 drivers/gpu/drm/tegra/hdmi.c              |   2 +-
 drivers/gpu/drm/tegra/sor.c               |   2 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c            |   2 +-
 drivers/gpu/drm/zte/zx_hdmi.c             |   2 +-
 include/drm/drm_connector.h               |  32 +++
 include/drm/drm_edid.h                    |  11 +-
 include/drm/drm_modes.h                   |  11 +
 34 files changed, 746 insertions(+), 39 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 01/14] drm: handle HDMI 2.0 VICs in AVI info-frames

[Shashank Sharma]

HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
For any other mode, the VIC filed in AVI infoframes should be 0.
HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
extended to (VIC 1-107).

This patch adds a bool input variable, which indicates if the connected
sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
HDMI 2.0 VIC to a HDMI 1.4 sink.

This patch touches all drm drivers, who are callers of this function
drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
no change in current behavior, is_hdmi2 is kept as false.

In case of I915 driver, this patch:
- checks if the connected display is HDMI 2.0.
- HDMI infoframes carry one of this two type of information:
	- VIC for 4K modes for HDMI 1.4 sinks
	- S3D information for S3D modes
  As CEA-861-F has already defined VICs for 4K videomodes, this
  patch doesn't allow sending HDMI infoframes for HDMI 2.0 sinks,
  until the mode is 3D.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <jose.abreu@synopsys.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>

PS: This patch touches a few lines in few files, which were
already above 80 char, so checkpatch gives 80 char warning again.
- gpu/drm/omapdrm/omap_encoder.c
- gpu/drm/i915/intel_sdvo.c

V2: Rebase, Added r-b from Andrzej
V3: Addressed review comment from Ville:
	- Do not send VICs in both AVI-IF and HDMI-IF
	  send only one of it.
V4: Rebase
V5: Added r-b from Neil.
    Addressed review comments from Ville
    - Do not block HDMI vendor IF, instead check for VIC while
      handling AVI infoframes
V6: Rebase
V7: Rebase

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
 drivers/gpu/drm/bridge/analogix-anx78xx.c |  3 ++-
 drivers/gpu/drm/bridge/sii902x.c          |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
 drivers/gpu/drm/drm_edid.c                | 26 +++++++++++++++++++++++++-
 drivers/gpu/drm/exynos/exynos_hdmi.c      |  2 +-
 drivers/gpu/drm/i2c/tda998x_drv.c         |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c         |  5 ++++-
 drivers/gpu/drm/i915/intel_sdvo.c         |  3 ++-
 drivers/gpu/drm/mediatek/mtk_hdmi.c       |  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  2 +-
 drivers/gpu/drm/nouveau/nv50_display.c    |  3 ++-
 drivers/gpu/drm/omapdrm/omap_encoder.c    |  3 ++-
 drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c      |  2 +-
 drivers/gpu/drm/sti/sti_hdmi.c            |  2 +-
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  2 +-
 drivers/gpu/drm/tegra/hdmi.c              |  2 +-
 drivers/gpu/drm/tegra/sor.c               |  2 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c            |  2 +-
 drivers/gpu/drm/zte/zx_hdmi.c             |  2 +-
 include/drm/drm_edid.h                    |  3 ++-
 25 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 9f78c03..aff1f48 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1867,7 +1867,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v10_0_audio_write_sad_regs(encoder);
 	dce_v10_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 4bcf01d..2df650d 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1851,7 +1851,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v11_0_audio_write_sad_regs(encoder);
 	dce_v11_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index fd134a4..0c3891f 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1597,7 +1597,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
 	ssize_t err;
 	u32 tmp;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index a9e8695..c164bef 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1750,7 +1750,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v8_0_audio_write_sad_regs(encoder);
 	dce_v8_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index c2fac39..dc045e0 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
 
 	mutex_lock(&anx78xx->lock);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
+						       false);
 	if (err) {
 		DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
 		goto unlock;
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index b8d10e5..9efb7b8 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -269,7 +269,7 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
 	if (ret)
 		return;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index de1308b..60faf2d 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1317,7 +1317,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 	u8 val;
 
 	/* Initialise info frame from DRM mode */
-	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 
 	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
 		frame.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 2e55599..0667b07 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4334,12 +4334,14 @@ EXPORT_SYMBOL(drm_set_preferred_mode);
  *                                              data from a DRM display mode
  * @frame: HDMI AVI infoframe
  * @mode: DRM display mode
+ * @is_hdmi2_sink: Sink is HDMI 2.0 compliant
  *
  * Return: 0 on success or a negative error code on failure.
  */
 int
 drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-					 const struct drm_display_mode *mode)
+					 const struct drm_display_mode *mode,
+					 bool is_hdmi2_sink)
 {
 	int err;
 
@@ -4355,6 +4357,28 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 
 	frame->video_code = drm_match_cea_mode(mode);
 
+	/*
+	 * HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
+	 * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
+	 * have to make sure we dont break HDMI 1.4 sinks.
+	 */
+	if (!is_hdmi2_sink && frame->video_code > 64)
+		frame->video_code = 0;
+
+	/*
+	 * HDMI spec says if a mode is found in HDMI 1.4b 4K modes
+	 * we should send its VIC in vendor infoframes, else send the
+	 * VIC in AVI infoframes. Lets check if this mode is present in
+	 * HDMI 1.4b 4K modes
+	 */
+	if (frame->video_code) {
+		u8 vendor_if_vic = drm_match_hdmi_mode(mode);
+		bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
+
+		if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
+			frame->video_code = 0;
+	}
+
 	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
 
 	/*
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 06bfbe4..c953927 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -784,7 +784,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
 	}
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
-			&hdata->current_mode);
+			&hdata->current_mode, false);
 	if (!ret)
 		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
 	if (ret > 0) {
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 86f47e1..d1e7ac5 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -712,7 +712,7 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
 {
 	union hdmi_infoframe frame;
 
-	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
 	frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
 
 	tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ec0779a..cc0d100 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -459,11 +459,14 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->base.adjusted_mode;
+	struct drm_connector *connector = &intel_hdmi->attached_connector->base;
+	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
 	union hdmi_infoframe frame;
 	int ret;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       adjusted_mode);
+						       adjusted_mode,
+						       is_hdmi2_sink);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f902922..e58a47d 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -996,7 +996,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 	ssize_t len;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       &pipe_config->base.adjusted_mode);
+						       &pipe_config->base.adjusted_mode,
+						       false);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return false;
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 0a4ffd7..5c0d024 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -975,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
 	u8 buffer[17];
 	ssize_t err;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (err < 0) {
 		dev_err(hdmi->dev,
 			"Failed to get AVI infoframe from mode: %zd\n", err);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index ae40e71..13ac822 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -97,7 +97,7 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi)
 	u32 val;
 	int len;
 
-	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
 
 	len = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
 	if (len < 0) {
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 42a85c1..5f71e30 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2762,7 +2762,8 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
 	if (!drm_detect_hdmi_monitor(nv_connector->edid))
 		return;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode,
+						       false);
 	if (!ret) {
 		/* We have an AVI InfoFrame, populate it to the display */
 		args.pwr.avi_infoframe_length
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 86c977b..624f5b5 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -85,7 +85,8 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
 	if (hdmi_mode && dssdev->driver->set_hdmi_infoframe) {
 		struct hdmi_avi_infoframe avi;
 
-		r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode);
+		r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
+							     false);
 		if (r == 0)
 			dssdev->driver->set_hdmi_infoframe(dssdev, &avi);
 	}
diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index aaacac1..770e31f 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
 	if (!connector)
 		return -EINVAL;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %d\n", err);
 		return err;
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 7d9b75e..7149968 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -294,7 +294,7 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
 	union hdmi_infoframe frame;
 	int rc;
 
-	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
 
 	if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
 		frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index a59c95a..dbc6a19 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -434,7 +434,7 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
 
 	DRM_DEBUG_DRIVER("\n");
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false);
 	if (ret < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %d\n", ret);
 		return ret;
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index d3398f62..83b7a2a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -52,7 +52,7 @@ static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi,
 	u8 buffer[17];
 	int i, ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (ret < 0) {
 		DRM_ERROR("Failed to get infoframes from mode\n");
 		return ret;
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index cda0491..718d8db 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -734,7 +734,7 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
 	u8 buffer[17];
 	ssize_t err;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (err < 0) {
 		dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index a8f5289..fb2709c 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1904,7 +1904,7 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
 	value &= ~INFOFRAME_CTRL_ENABLE;
 	tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
 	if (err < 0) {
 		dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
 		return err;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index ed63d4e..406d6d8 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -395,7 +395,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 	union hdmi_infoframe frame;
 	int ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
index 0df7366..7e834e3 100644
--- a/drivers/gpu/drm/zte/zx_hdmi.c
+++ b/drivers/gpu/drm/zte/zx_hdmi.c
@@ -124,7 +124,7 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
 	union hdmi_infoframe frame;
 	int ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
 	if (ret) {
 		DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n",
 			      ret);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 7b9f48b..89c0062 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -343,7 +343,8 @@ drm_load_edid_firmware(struct drm_connector *connector)
 
 int
 drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-					 const struct drm_display_mode *mode);
+					 const struct drm_display_mode *mode,
+					 bool is_hdmi2_sink);
 int
 drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
 					    const struct drm_display_mode *mode);
-- 
2.7.4

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

[PATCH v2 02/14] drm/edid: complete CEA modedb(VIC 1-107)

[Shashank Sharma]

CEA-861-F specs defines new video modes to be used with
HDMI 2.0 EDIDs. The VIC range has been extended from 1-64 to
1-107.

Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now
to be able to parse new CEA modes using the existing methods, we have
to complete the modedb (VIC=65 onwards).

This patch adds:
- Timings for existing CEA video modes (from VIC=65 till VIC=92)
- Newly added 4k modes (from VIC=93 to VIC=107).

The patch was originaly discussed and reviewed here:
https://patchwork.freedesktop.org/patch/135810/

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>

V2: Rebase
V3: Rebase
V4: Added native bit handling as per CEA-861-F spec (Ville)
V5: Fix timings for VIC 77:1920x1080 and 104:3840x2160p (Ville)
    Remove unnecessary paranthesis from function svd_to_vic (Ville)
    Added r-b (Neil)
V6: Rebase
V7: Fix indentation for modes from VIC 80

Reviewed-by: Jose Abreu <Jose.Abreu@synopsys.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Acked-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 227 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 226 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0667b07..8de3092 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1006,6 +1006,221 @@ static const struct drm_display_mode edid_cea_modes[] = {
 		   2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
 		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
 	 .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
+	/* 65 - 1280x720@24Hz */
+	{ DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 59400, 1280, 3040,
+		   3080, 3300, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 66 - 1280x720@25Hz */
+	{ DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3700,
+		   3740, 3960, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 67 - 1280x720@30Hz */
+	{ DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3040,
+		   3080, 3300, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 68 - 1280x720@50Hz */
+	{ DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1720,
+		   1760, 1980, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 69 - 1280x720@60Hz */
+	{ DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390,
+		   1430, 1650, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 70 - 1280x720@100Hz */
+	{ DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1720,
+		   1760, 1980, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 71 - 1280x720@120Hz */
+	{ DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1390,
+		   1430, 1650, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 72 - 1920x1080@24Hz */
+	{ DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2558,
+		   2602, 2750, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 73 - 1920x1080@25Hz */
+	{ DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2448,
+		   2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 74 - 1920x1080@30Hz */
+	{ DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2008,
+		   2052, 2200, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 75 - 1920x1080@50Hz */
+	{ DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2448,
+		   2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 76 - 1920x1080@60Hz */
+	{ DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2008,
+		   2052, 2200, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 77 - 1920x1080@100Hz */
+	{ DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 297000, 1920, 2448,
+		   2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	 .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 78 - 1920x1080@120Hz */
+	{ DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 297000, 1920, 2008,
+		   2052, 2200, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	 .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 79 - 1680x720@24Hz */
+	{ DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 3040,
+		   3080, 3300, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 80 - 1680x720@25Hz */
+	{ DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 2908,
+		   2948, 3168, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 81 - 1680x720@30Hz */
+	{ DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 2380,
+		   2420, 2640, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 82 - 1680x720@50Hz */
+	{ DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 82500, 1680, 1940,
+		   1980, 2200, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 83 - 1680x720@60Hz */
+	{ DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 99000, 1680, 1940,
+		   1980, 2200, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 84 - 1680x720@100Hz */
+	{ DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 165000, 1680, 1740,
+		   1780, 2000, 0, 720, 725, 730, 825, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 85 - 1680x720@120Hz */
+	{ DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 198000, 1680, 1740,
+		   1780, 2000, 0, 720, 725, 730, 825, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 86 - 2560x1080@24Hz */
+	{ DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 99000, 2560, 3558,
+		   3602, 3750, 0, 1080, 1084, 1089, 1100, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 87 - 2560x1080@25Hz */
+	{ DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 90000, 2560, 3008,
+		   3052, 3200, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 88 - 2560x1080@30Hz */
+	{ DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 118800, 2560, 3328,
+		   3372, 3520, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 89 - 2560x1080@50Hz */
+	{ DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 185625, 2560, 3108,
+		   3152, 3300, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 90 - 2560x1080@60Hz */
+	{ DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 198000, 2560, 2808,
+		   2852, 3000, 0, 1080, 1084, 1089, 1100, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 91 - 2560x1080@100Hz */
+	{ DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 371250, 2560, 2778,
+		   2822, 2970, 0, 1080, 1084, 1089, 1250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 92 - 2560x1080@120Hz */
+	{ DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 495000, 2560, 3108,
+		   3152, 3300, 0, 1080, 1084, 1089, 1250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+	/* 93 - 3840x2160p@24Hz 16:9 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 5116,
+		   5204, 5500, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9,},
+	/* 94 - 3840x2160p@25Hz 16:9 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4896,
+		   4984, 5280, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9},
+	/* 95 - 3840x2160p@30Hz 16:9 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016,
+		   4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9},
+	/* 96 - 3840x2160p@50Hz 16:9 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4896,
+		   4984, 5280, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9},
+	/* 97 - 3840x2160p@60Hz 16:9 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4016,
+		   4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9},
+	/* 98 - 4096x2160p@24Hz 256:135 */
+	{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 5116,
+		   5204, 5500, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+	/* 99 - 4096x2160p@25Hz 256:135 */
+	{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 5064,
+		   5152, 5280, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+	/* 100 - 4096x2160p@30Hz 256:135 */
+	{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 4184,
+		   4272, 4400, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+	/* 101 - 4096x2160p@50Hz 256:135 */
+	{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 594000, 4096, 5064,
+		   5152, 5280, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+	/* 102 - 4096x2160p@60Hz 256:135 */
+	{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 594000, 4096, 4184,
+		   4272, 4400, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+	/* 103 - 3840x2160p@24Hz 64:27 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 5116,
+		   5204, 5500, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
+	/* 104 - 3840x2160p@25Hz 64:27 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4896,
+		   4984, 5280, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
+	/* 105 - 3840x2160p@30Hz 64:27 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016,
+		   4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
+	/* 106 - 3840x2160p@50Hz 64:27 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4896,
+		   4984, 5280, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
+	/* 107 - 3840x2160p@60Hz 64:27 */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4016,
+		   4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
 };
 
 /*
@@ -2902,6 +3117,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
 	return modes;
 }
 
+static u8 svd_to_vic(u8 svd)
+{
+
+	/* 0-6 bit vic, 7th bit native mode indicator */
+	if ((svd >= 1 &&  svd <= 64) || (svd >= 129 && svd <= 192))
+		return svd & 127;
+
+	return svd;
+}
+
 static struct drm_display_mode *
 drm_display_mode_from_vic_index(struct drm_connector *connector,
 				const u8 *video_db, u8 video_len,
@@ -2915,7 +3140,7 @@ drm_display_mode_from_vic_index(struct drm_connector *connector,
 		return NULL;
 
 	/* CEA modes are numbered 1..127 */
-	vic = (video_db[video_index] & 127);
+	vic = svd_to_vic(video_db[video_index]);
 	if (!drm_valid_cea_vic(vic))
 		return NULL;
 
-- 
2.7.4

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

[PATCH v2 03/14] drm/edid: parse sink information before CEA blocks

[Shashank Sharma]

CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks.
This block contains a map of indexes of CEA modes, which can
support YCBCR 420 output also. To avoid multiple parsing of same
CEA block, let's parse the sink information and get this map, before
parsing CEA modes.

This patch moves the call to drm_add_display_info function, before the
mode parsing block.

V4: Introduced new patch in the series
V5: Move this patch before 4:2:0 parsing patch (ville)
    Added r-b from Ville
V6: Rebase
V7: Rebase

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 8de3092..57c09d2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4440,6 +4440,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 	quirks = edid_get_quirks(edid);
 
 	/*
+	 * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks.
+	 * To avoid multiple parsing of same block, lets parse that map
+	 * from sink info, before parsing CEA modes.
+	 */
+	drm_add_display_info(connector, edid);
+
+	/*
 	 * EDID spec says modes should be preferred in this order:
 	 * - preferred detailed mode
 	 * - other detailed modes from base block
@@ -4466,8 +4473,6 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 	if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
 		edid_fixup_preferred(connector, quirks);
 
-	drm_add_display_info(connector, edid);
-
 	if (quirks & EDID_QUIRK_FORCE_6BPC)
 		connector->display_info.bpc = 6;
 
-- 
2.7.4

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

[PATCH v2 04/14] drm/edid: cleanup patch for CEA extended-tag macro

[Shashank Sharma]

CEA-861-F introduces extended tag codes for EDID extension blocks,
which indicates the actual type of the data block. The code for
using exteded tag is 0x7, whereas in the existing code, the
corresponding macro is named as "VIDEO_CAPABILITY_BLOCK"

This patch renames the macro and usages from "VIDEO_CAPABILITY_BLOCK"
to "USE_EXTENDED_TAG"

V2: Add extended tag code check for video capabilitiy block (ville)
V3: Ville:
	- Use suggested names for macros
	- Check the block length first, before checking the extended tag
V4: Fix commit message (David)
V5: Introduced this patch into HDMI-YCBCR-output series
V6: Rebase
V7: Rebase

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 57c09d2..16f73c5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2781,7 +2781,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 #define VIDEO_BLOCK     0x02
 #define VENDOR_BLOCK    0x03
 #define SPEAKER_BLOCK	0x04
-#define VIDEO_CAPABILITY_BLOCK	0x07
+#define USE_EXTENDED_TAG 0x07
+#define EXT_VIDEO_CAPABILITY_BLOCK 0x00
 #define EDID_BASIC_AUDIO	(1 << 6)
 #define EDID_CEA_YCRCB444	(1 << 5)
 #define EDID_CEA_YCRCB422	(1 << 4)
@@ -3443,6 +3444,12 @@ cea_db_payload_len(const u8 *db)
 }
 
 static int
+cea_db_extended_tag(const u8 *db)
+{
+	return db[1];
+}
+
+static int
 cea_db_tag(const u8 *db)
 {
 	return db[0] >> 5;
@@ -4018,8 +4025,10 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
 		return false;
 
 	for_each_cea_db(edid_ext, i, start, end) {
-		if (cea_db_tag(&edid_ext[i]) == VIDEO_CAPABILITY_BLOCK &&
-		    cea_db_payload_len(&edid_ext[i]) == 2) {
+		if (cea_db_tag(&edid_ext[i]) == USE_EXTENDED_TAG &&
+		    cea_db_payload_len(&edid_ext[i]) == 2 &&
+		    cea_db_extended_tag(&edid_ext[i]) ==
+			EXT_VIDEO_CAPABILITY_BLOCK) {
 			DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", edid_ext[i + 2]);
 			return edid_ext[i + 2] & EDID_CEA_VCDB_QS;
 		}
-- 
2.7.4

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

[PATCH v2 05/14] drm: add helper to validate YCBCR420 modes

[Shashank Sharma]

YCBCR420 modes are supported only on HDMI 2.0 capable sources.
This patch adds:
- A drm helper to validate YCBCR420-only mode on a particular
  connector. This function will help pruning the YCBCR420-only
  modes from the connector's modelist.
- A bool variable (ycbcr_420_allowed) in the drm connector structure.
  While handling the EDID from HDMI 2.0 sinks, its important to know
  if the source is capable of handling YCBCR420 output, so that no
  YCBCR 420 modes will be listed for sources which can't handle it.
  A driver should set this variable if it wants to see YCBCR420 modes
  in the modedb.

V5: Introduced the patch in series.
V6: Squashed two patches (validate YCBCR420 and add YCBCR420
	   identifier)
V7: Addressed review comments from Vile:
    - Move this patch before we add 420 modes from EDID.
    - No need for drm_valid_cea_vic() check, function back to non-static.
    - Update MODE_STATUS with NO_420 condition.
    - Introduce y420_vdb_modes variable in this patch

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c         |  1 +
 drivers/gpu/drm/drm_modes.c        | 29 +++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c |  4 ++++
 include/drm/drm_connector.h        | 17 +++++++++++++++++
 include/drm/drm_modes.h            |  5 +++++
 5 files changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 16f73c5..96eee5a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2952,6 +2952,7 @@ static bool drm_valid_cea_vic(u8 vic)
 {
 	return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
 }
+EXPORT_SYMBOL(drm_valid_cea_vic);
 
 /**
  * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index f2493b9..35630b8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1083,6 +1083,34 @@ drm_mode_validate_size(const struct drm_display_mode *mode,
 }
 EXPORT_SYMBOL(drm_mode_validate_size);
 
+/**
+ * drm_mode_validate_ycbcr420 - add 'ycbcr420-only' modes only when allowed
+ * @mode: mode to check
+ * @connector: drm connector under action
+ *
+ * This function is a helper which can be used to filter out any YCBCR420
+ * only mode, when the source doesn't support it.
+ *
+ * Returns:
+ * The mode status
+ */
+enum drm_mode_status
+drm_mode_validate_ycbcr420(const struct drm_display_mode *mode,
+			   struct drm_connector *connector)
+{
+	u8 vic = drm_match_cea_mode(mode);
+	enum drm_mode_status status = MODE_OK;
+	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
+
+	if (test_bit(vic, hdmi->y420_vdb_modes)) {
+		if (!connector->ycbcr_420_allowed)
+			status = MODE_NO_420;
+	}
+
+	return status;
+}
+EXPORT_SYMBOL(drm_mode_validate_ycbcr420);
+
 #define MODE_STATUS(status) [MODE_ ## status + 3] = #status
 
 static const char * const drm_mode_status_names[] = {
@@ -1122,6 +1150,7 @@ static const char * const drm_mode_status_names[] = {
 	MODE_STATUS(ONE_SIZE),
 	MODE_STATUS(NO_REDUCED),
 	MODE_STATUS(NO_STEREO),
+	MODE_STATUS(NO_420),
 	MODE_STATUS(STALE),
 	MODE_STATUS(BAD),
 	MODE_STATUS(ERROR),
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 00e6832..904966c 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -528,6 +528,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		if (mode->status == MODE_OK)
 			mode->status = drm_mode_validate_pipeline(mode,
 								  connector);
+
+		if (mode->status == MODE_OK)
+			mode->status = drm_mode_validate_ycbcr420(mode,
+								  connector);
 	}
 
 prune:
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ae5b7dc..26dd3eb 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -135,6 +135,14 @@ struct drm_scdc {
 struct drm_hdmi_info {
 	/** @scdc: sink's scdc support and capabilities */
 	struct drm_scdc scdc;
+
+	/**
+	 * @y420_vdb_modes: bitmap of modes which can support ycbcr420
+	 * output only (not normal RGB/YCBCR444/422 outputs). There are total
+	 * 107 VICs defined by CEA-861-F spec, so the size is 128 bits to map
+	 * upto 128 VICs;
+	 */
+	unsigned long y420_vdb_modes[BITS_TO_LONGS(128)];
 };
 
 /**
@@ -726,6 +734,15 @@ struct drm_connector {
 	bool interlace_allowed;
 	bool doublescan_allowed;
 	bool stereo_allowed;
+
+	/**
+	 * @ycbcr_420_allowed : This bool indicates if this connector is
+	 * capable of handling YCBCR 420 output. While parsing the EDID
+	 * blocks, its very helpful to know, if the source is capable of
+	 * handling YCBCR 420 outputs.
+	 */
+	bool ycbcr_420_allowed;
+
 	/**
 	 * @registered: Is this connector exposed (registered) with userspace?
 	 * Protected by @mutex.
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 94ac771..f8a1268 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -80,6 +80,7 @@ struct videomode;
  * @MODE_ONE_SIZE: only one resolution is supported
  * @MODE_NO_REDUCED: monitor doesn't accept reduced blanking
  * @MODE_NO_STEREO: stereo modes not supported
+ * @MODE_NO_420: ycbcr 420 modes not supported
  * @MODE_STALE: mode has become stale
  * @MODE_BAD: unspecified reason
  * @MODE_ERROR: error condition
@@ -124,6 +125,7 @@ enum drm_mode_status {
 	MODE_ONE_SIZE,
 	MODE_NO_REDUCED,
 	MODE_NO_STEREO,
+	MODE_NO_420,
 	MODE_STALE = -3,
 	MODE_BAD = -2,
 	MODE_ERROR = -1
@@ -496,6 +498,9 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
 enum drm_mode_status drm_mode_validate_basic(const struct drm_display_mode *mode);
 enum drm_mode_status drm_mode_validate_size(const struct drm_display_mode *mode,
 					    int maxX, int maxY);
+enum drm_mode_status
+drm_mode_validate_ycbcr420(const struct drm_display_mode *mode,
+			   struct drm_connector *connector);
 void drm_mode_prune_invalid(struct drm_device *dev,
 			    struct list_head *mode_list, bool verbose);
 void drm_mode_sort(struct list_head *mode_list);
-- 
2.7.4

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

[PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID

[Shashank Sharma]

HDMI 2.0 spec adds support for YCBCR420 sub-sampled output.
CEA-861-F adds two new blocks in EDID's CEA extension blocks,
to provide information about sink's YCBCR420 output capabilities.

These blocks are:

- YCBCR420vdb(YCBCR 420 video data block):
This block contains VICs of video modes, which can be sopported only
in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal
SVD block, valid for YCBCR420 modes only.

- YCBCR420cmdb(YCBCR 420 capability map data block):
This block gives information about video modes which can support
YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This
block contains a bitmap index of normal svd videomodes, which can
support YCBCR420 output too.
So if bit 0 from first vcb byte is set, first video mode in the svd
list can support YCBCR420 output too. Bit 1 means second video mode
from svd list can support YCBCR420 output too, and so on.

This patch adds two bitmaps in display's hdmi_info structure, one each
for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch
adds:
- VDB modes (YCBCR 420 only modes) in connector's mode list, also makes
  an entry in the vdb_bitmap per vic.
- VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: Emil Velikov <emil.l.velikov@gmail.com>

V2: Addressed
    Review comments from Emil:
    - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit.
    - Use the suggested method for updating dbmap.
    - Add documentation for YCBCR420_vcb_map to fix kbuild warning.

    Review comments from Ville:
    - Do not expose the YCBCR420 flags in uabi layer, keep it internal.
    - Save a map of YCBCR420 modes for future reference.
    - Check db length before trying to parse extended tag.
    - Add a warning if there are > 64 modes in capability map block.
    - Use y420cmdb in function names and macros while dealing with vcb
      to be aligned with spec.
    - Move the display information parsing block ahead of mode parsing
      blocks.

V3: Addressed design/review comments from Ville
    - Do not add flags in video modes, else we have to expose them to user
    - There should not be a UABI change, and kernel should detect the
      choice of the output based on type of mode, and the bitmaps.
    - Use standard bitops from kernel bitmap header, instead of calculating
      bit positions manually.

V4: Addressed review comments from Ville:
    - s/ycbcr_420_vdb/y420vdb
    - s/ycbcr_420_vcb/y420cmdb
    - Be less verbose on description of do_y420vdb_modes
    - Move newmode variable in the loop scope.
    - Use svd_to_vic() to get a VIC, instead of 0x7f
    - Remove bitmap description for CMDB modes & VDB modes
    - Dont add connector->ycbcr_420_allowed check for cmdb modes
    - Remove 'len' variable, in is_y420cmdb function, which is used
      only once
    - Add length check in is_y420vdb function
    - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap
    - Do not add print about YCBCR 420 modes
    - Fix indentation in few places
    - Move ycbcr420_dc_modes in next patch, where its used
    - Add a separate patch for movement of drm_add_display_info()

V5: Addressed review comments from Ville:
    - Add the patch which cleans up the current EXTENDED_TAG usage
    - Make y420_cmdb_map u64
    - Do not block ycbcr420 modes while parsing the EDID, rather
      add a separate helper function to prune ycbcr420-only modes from
      connector's probed modes.

V6: Rebase
V7: Move this patch after the 420_only validation patch (Ville)

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c  | 148 +++++++++++++++++++++++++++++++++++++++++++-
 include/drm/drm_connector.h |  12 ++++
 2 files changed, 158 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 96eee5a..b86afb9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2783,6 +2783,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 #define SPEAKER_BLOCK	0x04
 #define USE_EXTENDED_TAG 0x07
 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
+#define EXT_VIDEO_DATA_BLOCK_420	0x0E
+#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
 #define EDID_BASIC_AUDIO	(1 << 6)
 #define EDID_CEA_YCRCB444	(1 << 5)
 #define EDID_CEA_YCRCB422	(1 << 4)
@@ -3155,15 +3157,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector,
 	return newmode;
 }
 
+/*
+ * do_y420vdb_modes - Parse YCBCR 420 only modes
+ * @connector: connector corresponding to the HDMI sink
+ * @svds: start of the data block of CEA YCBCR 420 VDB
+ * @len: length of the CEA YCBCR 420 VDB
+ *
+ * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB)
+ * which contains modes which can be supported in YCBCR 420
+ * output format only.
+ */
+static int do_y420vdb_modes(struct drm_connector *connector,
+				   const u8 *svds, u8 svds_len)
+{
+	int modes = 0, i;
+	struct drm_device *dev = connector->dev;
+	struct drm_display_info *info = &connector->display_info;
+	struct drm_hdmi_info *hdmi = &info->hdmi;
+
+	for (i = 0; i < svds_len; i++) {
+		u8 vic = svd_to_vic(svds[i]);
+		struct drm_display_mode *newmode;
+
+		newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
+		if (!newmode)
+			break;
+		bitmap_set(hdmi->y420_vdb_modes, vic, 1);
+		drm_mode_probed_add(connector, newmode);
+		modes++;
+	}
+
+	if (modes > 0)
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
+	return modes;
+}
+
+/*
+ * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap
+ * @connector: connector corresponding to the HDMI sink
+ * @vic: CEA vic for the video mode to be added in the map
+ *
+ * Makes an entry for a videomode in the YCBCR 420 bitmap
+ */
+static void
+drm_add_cmdb_modes(struct drm_connector *connector, u8 svd)
+{
+	u8 vic = svd_to_vic(svd);
+	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
+
+	bitmap_set(hdmi->y420_cmdb_modes, vic, 1);
+}
+
 static int
 do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
 {
 	int i, modes = 0;
+	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
 
 	for (i = 0; i < len; i++) {
 		struct drm_display_mode *mode;
 		mode = drm_display_mode_from_vic_index(connector, db, len, i);
 		if (mode) {
+			/*
+			 * YCBCR420 capability block contains a bitmap which
+			 * gives the index of CEA modes from CEA VDB, which
+			 * can support YCBCR 420 sampling output also (apart
+			 * from RGB/YCBCR444 etc).
+			 * For example, if the bit 0 in bitmap is set,
+			 * first mode in VDB can support YCBCR420 output too.
+			 * Add YCBCR420 modes only if sink is HDMI 2.0 capable.
+			 */
+			if (hdmi->y420_cmdb_map & (1 << i))
+				drm_add_cmdb_modes(connector, db[i]);
+
 			drm_mode_probed_add(connector, mode);
 			modes++;
 		}
@@ -3505,9 +3571,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
 	return oui == HDMI_FORUM_IEEE_OUI;
 }
 
+static bool cea_db_is_y420cmdb(const u8 *db)
+{
+
+	if (cea_db_tag(db) != USE_EXTENDED_TAG)
+		return false;
+
+	if (!cea_db_payload_len(db))
+		return false;
+
+	if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB)
+		return false;
+
+	return true;
+}
+
+static bool cea_db_is_y420vdb(const u8 *db)
+{
+	if (cea_db_tag(db) != USE_EXTENDED_TAG)
+		return false;
+
+	if (!cea_db_payload_len(db))
+		return false;
+
+	if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420)
+		return false;
+
+	return true;
+}
+
 #define for_each_cea_db(cea, i, start, end) \
 	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
 
+static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
+				     const u8 *db)
+{
+	struct drm_display_info *info = &connector->display_info;
+	struct drm_hdmi_info *hdmi = &info->hdmi;
+	u8 map_len = cea_db_payload_len(db) - 1;
+	u8 count;
+	u64 map = 0;
+
+	if (map_len == 0) {
+		/* All CEA modes support ycbcr420 sampling also.*/
+		hdmi->y420_cmdb_map = U64_MAX;
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
+		return;
+	}
+
+	/*
+	 * This map indicates which of the existing CEA block modes
+	 * from VDB can support YCBCR420 output too. So if bit=0 is
+	 * set, first mode from VDB can support YCBCR420 output too.
+	 * We will parse and keep this map, before parsing VDB itself
+	 * to avoid going through the same block again and again.
+	 *
+	 * Spec is not clear about max possible size of this block.
+	 * Clamping max bitmap block size at 8 bytes. Every byte can
+	 * address 8 CEA modes, in this way this map can address
+	 * 8*8 = first 64 SVDs.
+	 */
+	if (WARN_ON_ONCE(map_len > 8))
+		map_len = 8;
+
+	for (count = 0; count < map_len; count++)
+		map |= (u64)db[2 + count] << (8 * count);
+
+	if (map)
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
+
+	hdmi->y420_cmdb_map = map;
+}
+
 static int
 add_cea_modes(struct drm_connector *connector, struct edid *edid)
 {
@@ -3530,10 +3665,16 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
 				video = db + 1;
 				video_len = dbl;
 				modes += do_cea_modes(connector, video, dbl);
-			}
-			else if (cea_db_is_hdmi_vsdb(db)) {
+			} else if (cea_db_is_hdmi_vsdb(db)) {
 				hdmi = db;
 				hdmi_len = dbl;
+			} else if (cea_db_is_y420vdb(db)) {
+				const u8 *vdb420 = &db[2];
+
+				/* Add 4:2:0(only) modes present in EDID */
+				modes += do_y420vdb_modes(connector,
+							  vdb420,
+							  dbl - 1);
 			}
 		}
 	}
@@ -4216,6 +4357,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 			drm_parse_hdmi_vsdb_video(connector, db);
 		if (cea_db_is_hdmi_forum_vsdb(db))
 			drm_parse_hdmi_forum_vsdb(connector, db);
+		if (cea_db_is_y420cmdb(db))
+			drm_parse_y420cmdb_bitmap(connector, db);
 	}
 }
 
@@ -4477,6 +4620,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 	num_modes += add_cea_modes(connector, edid);
 	num_modes += add_alternate_cea_modes(connector, edid);
 	num_modes += add_displayid_detailed_modes(connector, edid);
+
 	if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
 		num_modes += add_inferred_modes(connector, edid);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 26dd3eb..225e092 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -143,6 +143,17 @@ struct drm_hdmi_info {
 	 * upto 128 VICs;
 	 */
 	unsigned long y420_vdb_modes[BITS_TO_LONGS(128)];
+
+	/**
+	 * @y420_cmdb_modes: bitmap of modes which can support ycbcr420
+	 * output also, along with normal HDMI outputs. There are total 107
+	 * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto
+	 * 128 VICs;
+	 */
+	unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)];
+
+	/** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */
+	u64 y420_cmdb_map;
 };
 
 /**
@@ -206,6 +217,7 @@ struct drm_display_info {
 #define DRM_COLOR_FORMAT_RGB444		(1<<0)
 #define DRM_COLOR_FORMAT_YCRCB444	(1<<1)
 #define DRM_COLOR_FORMAT_YCRCB422	(1<<2)
+#define DRM_COLOR_FORMAT_YCRCB420	(1<<3)
 
 	/**
 	 * @color_formats: HDMI Color formats, selects between RGB and YCrCb
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 07/14] drm/edid: parse ycbcr 420 deep color information

[Shashank Sharma]

CEA-861-F spec adds ycbcr420 deep color support information
in hf-vsdb block. This patch extends the existing hf-vsdb parsing
function by adding parsing of ycbcr420 deep color support from the
EDID and adding it into display information stored.

V2: Rebase
V3: Rebase
V4: Moved definition of y420_dc_modes into this patch, where its used
    (Ville)
V5: Optimize function, if(conditions) not reqd (Ville)
V6: Rebase
V7: Rebase

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c  | 12 ++++++++++++
 include/drm/drm_connector.h |  3 +++
 include/drm/drm_edid.h      |  8 ++++++++
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b86afb9..6ef1f3f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4199,6 +4199,16 @@ drm_default_rgb_quant_range(const struct drm_display_mode *mode)
 }
 EXPORT_SYMBOL(drm_default_rgb_quant_range);
 
+static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector,
+					     const u8 *db)
+{
+	u8 dc_mask;
+	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
+
+	dc_mask = db[7] & DRM_EDID_YCBCR420_DC_MASK;
+	hdmi->y420_dc_modes |= dc_mask;
+}
+
 static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
 				 const u8 *hf_vsdb)
 {
@@ -4239,6 +4249,8 @@ static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
 				scdc->scrambling.low_rates = true;
 		}
 	}
+
+	drm_parse_ycbcr420_deep_color_info(connector, hf_vsdb);
 }
 
 static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 225e092..4bc0882 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -154,6 +154,9 @@ struct drm_hdmi_info {
 
 	/** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */
 	u64 y420_cmdb_map;
+
+	/** @y420_dc_modes: bitmap of deep color support index */
+	u8 y420_dc_modes;
 };
 
 /**
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 89c0062..1e1908a 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -213,6 +213,14 @@ struct detailed_timing {
 #define DRM_EDID_HDMI_DC_30               (1 << 4)
 #define DRM_EDID_HDMI_DC_Y444             (1 << 3)
 
+/* YCBCR 420 deep color modes */
+#define DRM_EDID_YCBCR420_DC_48		  (1 << 6)
+#define DRM_EDID_YCBCR420_DC_36		  (1 << 5)
+#define DRM_EDID_YCBCR420_DC_30		  (1 << 4)
+#define DRM_EDID_YCBCR420_DC_MASK (DRM_EDID_YCBCR420_DC_48 | \
+				    DRM_EDID_YCBCR420_DC_36 | \
+				    DRM_EDID_YCBCR420_DC_30)
+
 /* ELD Header Block */
 #define DRM_ELD_HEADER_BLOCK_SIZE	4
 
-- 
2.7.4

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

[PATCH v2 08/14] drm: add helper functions for YCBCR420 handling

[Shashank Sharma]

This patch adds helper functions for YCBCR 420 handling.
These functions do:
- check if a given video mode is YCBCR 420 only mode.
- check if a given video mode is YCBCR 420 also mode.

V2: Added YCBCR functions as helpers in DRM layer, instead of
    keeping it in I915 layer.
V3: Added handling for YCBCR-420 only modes too.
V4: EXPORT_SYMBOL(drm_find_hdmi_output_type)
V5: Addressed review comments from Danvet:
    - %s/drm_find_hdmi_output_type/drm_display_info_hdmi_output_type
    - %s/drm_can_support_ycbcr_output/drm_display_supports_ycbcr_output
    - %s/drm_can_support_this_ycbcr_output/
		drm_display_supports_this_ycbcr_output
    - pass drm_display_info instead of drm_connector for consistency
    - For drm_get_highest_quality_ycbcr_supported doc, move the variable
      description above, and then the function description.
V6: Add only YCBCR420 helpers (Ville)
V7: Addressed review comments from Ville
    - Remove cea_vic_valid() check.
    - Fix indentation.
    - Make input parameters to helpers, const.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_modes.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_modes.h     |  6 +++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 35630b8..c1aec53 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1605,3 +1605,61 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
 out:
 	return ret;
 }
+
+/**
+ * drm_mode_is_420_only - if a given videomode can be only supported in YCBCR420
+ * output format
+ *
+ * @connector: drm connector under action.
+ * @mode: video mode to be tested.
+ *
+ * Returns:
+ * true if the mode can be supported in YCBCR420 format
+ * false if not.
+ */
+bool drm_mode_is_420_only(const struct drm_display_info *display,
+			  const struct drm_display_mode *mode)
+{
+	u8 vic = drm_match_cea_mode(mode);
+
+	return test_bit(vic, display->hdmi.y420_vdb_modes);
+}
+EXPORT_SYMBOL(drm_mode_is_420_only);
+
+/**
+ * drm_mode_is_420_also - if a given videomode can be supported in YCBCR420
+ * output format also (along with RGB/YCBCR444/422)
+ *
+ * @display: display under action.
+ * @mode: video mode to be tested.
+ *
+ * Returns:
+ * true if the mode can be support YCBCR420 format
+ * false if not.
+ */
+bool drm_mode_is_420_also(const struct drm_display_info *display,
+			  const struct drm_display_mode *mode)
+{
+	u8 vic = drm_match_cea_mode(mode);
+
+	return test_bit(vic, display->hdmi.y420_cmdb_modes);
+}
+EXPORT_SYMBOL(drm_mode_is_420_also);
+/**
+ * drm_mode_is_420 - if a given videomode can be supported in YCBCR420
+ * output format
+ *
+ * @display: display under action.
+ * @mode: video mode to be tested.
+ *
+ * Returns:
+ * true if the mode can be supported in YCBCR420 format
+ * false if not.
+ */
+bool drm_mode_is_420(const struct drm_display_info *display,
+		     const struct drm_display_mode *mode)
+{
+	return drm_mode_is_420_only(display, mode) ||
+		drm_mode_is_420_also(display, mode);
+}
+EXPORT_SYMBOL(drm_mode_is_420);
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index f8a1268..e9a42591 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -452,6 +452,12 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
 			   const struct drm_mode_modeinfo *in);
 void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode);
 void drm_mode_debug_printmodeline(const struct drm_display_mode *mode);
+bool drm_mode_is_420_only(const struct drm_display_info *display,
+			  const struct drm_display_mode *mode);
+bool drm_mode_is_420_also(const struct drm_display_info *display,
+			  const struct drm_display_mode *mode);
+bool drm_mode_is_420(const struct drm_display_info *display,
+			  const struct drm_display_mode *mode);
 
 struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
 				      int hdisplay, int vdisplay, int vrefresh,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs

[Shashank Sharma]

This patch checks encoder level support for YCBCR420 outputs.
The logic goes as simple as this:
If the input mode is YCBCR420-only mode: prepare HDMI for
YCBCR420 output, else continue with RGB output mode.

It checks if the mode is YCBCR420 and source can support this
output then it marks the ycbcr_420 output indicator into crtc
state, for further staging in driver.

V2: Split the patch into two, kept helper functions in DRM layer.
V3: Changed the compute_config function based on new DRM API.
V4: Rebase
V5: Rebase
V6: Check and handle YCBCR420-only modes, discard the property
    based approach (Ville)
V7: Addressed review comments from Ville
    - add else case in 12BPC check.
    - extract ycbcr420 state inside hdmi_12bpc_possible function.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  1 +
 drivers/gpu/drm/i915/intel_drv.h     |  3 +++
 drivers/gpu/drm/i915/intel_hdmi.c    | 43 +++++++++++++++++++++++++++++++++---
 3 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2144adc..a5140e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11945,6 +11945,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
 	PIPE_CONF_CHECK_I(hdmi_scrambling);
 	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
 	PIPE_CONF_CHECK_I(has_infoframe);
+	PIPE_CONF_CHECK_I(ycbcr420);
 
 	PIPE_CONF_CHECK_I(has_audio);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a324..592243b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -780,6 +780,9 @@ struct intel_crtc_state {
 
 	/* HDMI High TMDS char rate ratio */
 	bool hdmi_high_tmds_clock_ratio;
+
+	/* HDMI output type */
+	bool ycbcr420;
 };
 
 struct intel_crtc {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index cc0d100..eb6243c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1312,6 +1312,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 	struct drm_atomic_state *state = crtc_state->base.state;
 	struct drm_connector_state *connector_state;
 	struct drm_connector *connector;
+	bool output_ycbcr420 = crtc_state->ycbcr420;
 	int i;
 
 	if (HAS_GMCH_DISPLAY(dev_priv))
@@ -1330,8 +1331,16 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 		if (connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
-		if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0)
-			return false;
+		if (output_ycbcr420) {
+			const struct drm_hdmi_info *hdmi = &info->hdmi;
+
+			if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
+				return false;
+		} else {
+
+			if (!(info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36))
+				return false;
+		}
 	}
 
 	/* Display Wa #1139 */
@@ -1342,6 +1351,25 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 	return true;
 }
 
+static bool
+intel_hdmi_ycbcr420_config(struct drm_connector *connector,
+			       struct intel_crtc_state *config,
+			       int *clock_12bpc, int *clock_8bpc)
+{
+
+	if (!connector->ycbcr_420_allowed) {
+		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
+		return false;
+	}
+
+	/* YCBCR420 TMDS rate requirement is half the pixel clock */
+	config->port_clock /= 2;
+	*clock_12bpc /= 2;
+	*clock_8bpc /= 2;
+	config->ycbcr420 = true;
+	return true;
+}
+
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_state *pipe_config,
 			       struct drm_connector_state *conn_state)
@@ -1349,7 +1377,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
+	struct drm_connector *connector = conn_state->connector;
+	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
 	struct intel_digital_connector_state *intel_conn_state =
 		to_intel_digital_connector_state(conn_state);
 	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
@@ -1379,6 +1408,14 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 		clock_12bpc *= 2;
 	}
 
+	if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) {
+		if (!intel_hdmi_ycbcr420_config(connector, pipe_config,
+						&clock_12bpc, &clock_8bpc)) {
+			DRM_ERROR("Can't support YCBCR420 output\n");
+			return false;
+		}
+	}
+
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
 		pipe_config->has_pch_encoder = true;
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset

[Shashank Sharma]

To get a YCBCR420 output from intel platforms, we need one
scaler to scale down YCBCR444 samples to YCBCR420 samples.

This patch:
- Does scaler allocation for HDMI ycbcr420 outputs.
- Programs PIPE_MISC register for ycbcr420 output.
- Adds a new scaler user "HDMI output" to plug-into existing
  scaler framework. This output type is identified using bit
  30 of the scaler users bitmap.

V2: rebase
V3: rebase
V4: rebase
V5: addressed review comments from Ander:
    - No need to check both scaler_user && hdmi_output.
      Check for scaler_user is enough.
V6: rebase
V7: Do not create a new scaler user, use existing pipe scaler user.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Ander Conselvan De Oliveira <conselvan2@gmail.com>

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  3 +++
 drivers/gpu/drm/i915/intel_drv.h     |  4 +++-
 drivers/gpu/drm/i915/intel_hdmi.c    | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a5140e4..d78f1c2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4624,6 +4624,9 @@ 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)
+		need_scaling = true;
+
 	/*
 	 * Scaling/fitting not supported in IF-ID mode in GEN9+
 	 * TODO: Interlace fetch mode doesn't support YUV420 planar formats.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 592243b..94ea6ed 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -471,7 +471,8 @@ struct intel_crtc_scaler_state {
 	 *
 	 *     If a bit is set, a user is using a scaler.
 	 *     Here user can be a plane or crtc as defined below:
-	 *       bits 0-30 - plane (bit position is index from drm_plane_index)
+	 *       bits 0-29 - plane (bit position is index from drm_plane_index)
+	 *       bit 30    - hdmi output
 	 *       bit 31    - crtc
 	 *
 	 * Instead of creating a new index to cover planes and crtc, using
@@ -484,6 +485,7 @@ struct intel_crtc_scaler_state {
 	 * avilability.
 	 */
 #define SKL_CRTC_INDEX 31
+
 	unsigned scaler_users;
 
 	/* scaler used by crtc for panel fitting purpose */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index eb6243c..49f4fb8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1356,6 +1356,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
 			       struct intel_crtc_state *config,
 			       int *clock_12bpc, int *clock_8bpc)
 {
+	struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc);
 
 	if (!connector->ycbcr_420_allowed) {
 		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
@@ -1367,6 +1368,17 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
 	*clock_12bpc /= 2;
 	*clock_8bpc /= 2;
 	config->ycbcr420 = true;
+
+	/* YCBCR 420 output conversion needs a scaler */
+	if (skl_update_scaler_crtc(config)) {
+		DRM_ERROR("Scaler allocation for output failed\n");
+		return false;
+	}
+
+	/* Bind this scaler to pipe */
+	intel_pch_panel_fitting(intel_crtc, config,
+				DRM_MODE_SCALE_FULLSCREEN);
+
 	return true;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 96c2cbd..fd2e081 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -110,7 +110,8 @@ 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)
+	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
+	    !pipe_config->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

[PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output

[Shashank Sharma]

To get HDMI YCBCR420 output, the PIPEMISC register should be
programmed to:
- Generate YCBCR output (bit 11)
- In case of YCBCR420 outputs, it should be programmed in full
  blend mode to use the scaler in 5x3 ratio (bits 26 and 27)

This patch:
- Adds definition of these bits.
- Programs PIPEMISC for YCBCR420 outputs.

V2: rebase
V3: rebase
V4: rebase
V5: added r-b from Ander
V6: Handle only YCBCR420 outputs (ville)
V7: rebase

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 3 +++
 drivers/gpu/drm/i915/intel_display.c | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c712d01..e5020d6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5227,6 +5227,9 @@ enum {
 
 #define _PIPE_MISC_A			0x70030
 #define _PIPE_MISC_B			0x71030
+#define   PIPEMISC_YCBCR420_ENABLE	(1<<27)
+#define   PIPEMISC_YCBCR420_MODE_BLEND	(1<<26)
+#define   PIPEMISC_OUTPUT_YCBCR		(1<<11)
 #define   PIPEMISC_DITHER_BPC_MASK	(7<<5)
 #define   PIPEMISC_DITHER_8_BPC		(0<<5)
 #define   PIPEMISC_DITHER_10_BPC	(1<<5)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d78f1c2..1a23ec0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8076,6 +8076,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *config = intel_crtc->config;
 
 	if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) {
 		u32 val = 0;
@@ -8101,6 +8102,12 @@ 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_YCBCR |
+				PIPEMISC_YCBCR420_ENABLE |
+				PIPEMISC_YCBCR420_MODE_BLEND;
+		}
+
 		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
 	}
 }
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 12/14] drm/i915: prepare csc unit for YCBCR420 output

[Shashank Sharma]

To support ycbcr output, we need a pipe CSC block to do
RGB->YCBCR conversion.

Current Intel platforms have only one pipe CSC unit, so
we can either do color correction using it, or we can perform
RGB->YCBCR conversion.

This function adds a csc handler, which uses recommended bspec
values to perform RGB->YCBCR conversion (target color space BT709)

V2: Rebase
V3: Rebase
V4: Rebase
V5: Addressed review comments from Ander
    - Remove extra line added in the patch
    - Add the spec details in the commit message
    - Combine two if(cond) while calling intel_crtc_compute_config
V6: Handle YCBCR420 outputs only (Ville)
V7: Addressed review comments from Ville:
    - Add description about target colorspace
    - Remove the comments from CSC function
    - DRM_DEBUG->DEBUG_KMS for atomic failure due to CSC unit busy
    - Remove unnecessary debug message about YCBCR420 possibe

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   | 46 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index f85d575..8698691 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -41,6 +41,22 @@
 
 #define LEGACY_LUT_LENGTH		(sizeof(struct drm_color_lut) * 256)
 
+/* Post offset values for RGB->YCBCR conversion */
+#define POSTOFF_RGB_TO_YUV_HI 0x800
+#define POSTOFF_RGB_TO_YUV_ME 0x100
+#define POSTOFF_RGB_TO_YUV_LO 0x800
+
+/*
+ * These values are direct register values specified in the Bspec,
+ * for RGB->YUV conversion matrix (colorspace BT709)
+ */
+#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8
+#define CSC_RGB_TO_YUV_BU 0x37e80000
+#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0
+#define CSC_RGB_TO_YUV_BY 0xb5280000
+#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
+#define CSC_RGB_TO_YUV_BV 0x1e080000
+
 /*
  * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
  * format). This macro takes the coefficient we want transformed and the
@@ -91,6 +107,31 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
 	}
 }
 
+void i9xx_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc)
+{
+	int pipe = intel_crtc->pipe;
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+
+
+	I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
+	I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
+	I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0);
+
+	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), CSC_RGB_TO_YUV_RU_GU);
+	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU);
+
+	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), CSC_RGB_TO_YUV_RY_GY);
+	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY);
+
+	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), CSC_RGB_TO_YUV_RV_GV);
+	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV);
+
+	I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI);
+	I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME);
+	I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO);
+	I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+}
+
 /* Set up the pipe CSC unit. */
 static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 {
@@ -101,7 +142,10 @@ 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 (crtc_state->ctm) {
+	if (intel_crtc_state->ycbcr420) {
+		i9xx_load_ycbcr_conversion_matrix(intel_crtc);
+		return;
+	} else if (crtc_state->ctm) {
 		struct drm_color_ctm *ctm =
 			(struct drm_color_ctm *)crtc_state->ctm->data;
 		uint64_t input[9] = { 0, };
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1a23ec0..9c6a1ed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6283,6 +6283,21 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
+	/* YCBCR420 feasibility check */
+	if (pipe_config->ycbcr420) {
+		struct drm_crtc_state *drm_state = &pipe_config->base;
+
+		/*
+		 * There is only one pipe CSC unit per pipe, and we need that
+		 * for output conversion from RGB->YCBCR. So if CTM is already
+		 * applied we can't support YCBCR420 output.
+		 */
+		if (drm_state->ctm) {
+			DRM_DEBUG_KMS("YCBCR420 and CTM is not possible\n");
+			return -EINVAL;
+		}
+	}
+
 	/*
 	 * Pipe horizontal size must be even in:
 	 * - DVO ganged mode
-- 
2.7.4

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

[PATCH v2 13/14] drm/i915: set colorspace for YCBCR420 outputs

[Shashank Sharma]

When output colorspace is YCBCR420, we have to load the
corresponding colorspace in AVI infoframe. This patch fills
the colorspace of AVI infoframe as per the output mode.

V2: Rebase
V3: Rebase
V4: Rebase
V5: Added r-b from Ander
V6: Checking RGB/YCBCR420 output only (Ville)
V7: Add colorspace info in driver(not drm layer) (Ville)

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 49f4fb8..412b6e3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -472,12 +472,18 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 		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, adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
 					   intel_hdmi->rgb_quant_range_selectable);
 
+	/* TODO: handle pixel repetition for YCBCR420 outputs */
 	intel_write_infoframe(encoder, crtc_state, &frame);
 }
 
-- 
2.7.4

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

[PATCH v2 14/14] drm/i915/glk: set HDMI 2.0 identifier

[Shashank Sharma]

This patch sets the is_hdmi2_src identifier in drm connector
for GLK platform. GLK contains a native HDMI 2.0 controller.
This identifier will help the EDID handling functions to save
lot of work which is specific to HDMI 2.0 sources.

V3: Added this patch
V4: Rebase
V4: Rebase
V5: Added r-b from Ander
V6: Rebase
V7: Rebase

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 412b6e3..bd08ae4 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1917,6 +1917,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	connector->doublescan_allowed = 0;
 	connector->stereo_allowed = 1;
 
+	if (IS_GEMINILAKE(dev_priv))
+		connector->ycbcr_420_allowed = true;
+
 	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
 
 	switch (port) {
-- 
2.7.4

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

✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev2)

[Patchwork]

== Series Details ==

Series: YCBCR 4:2:0 handling in DRM layer (rev2)
URL   : https://patchwork.freedesktop.org/series/26972/
State : success

== Summary ==

Series 26972v2 YCBCR 4:2:0 handling in DRM layer
https://patchwork.freedesktop.org/api/1.0/series/26972/revisions/2/mbox/

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:437s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:424s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:358s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:538s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:512s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:490s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:485s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:592s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:434s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:411s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:415s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:498s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:469s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:566s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:575s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:563s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:454s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:580s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:474s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:482s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:430s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:486s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:400s

8ad9e19aafea47c272163c2cbf554e06ff7f9857 drm-tip: 2017y-07m-11d-19h-08m-20s UTC integration manifest
706180a drm/i915/glk: set HDMI 2.0 identifier
23c1aaa drm/i915: set colorspace for YCBCR420 outputs
f1885be drm/i915: prepare csc unit for YCBCR420 output
d550d0a drm/i915: prepare pipe for YCBCR420 output
0d4052d drm/i915: prepare scaler for YCBCR420 modeset
230c4ad drm/i915: add config function for YCBCR420 outputs
93fe7db drm: add helper functions for YCBCR420 handling
426d8ac drm/edid: parse ycbcr 420 deep color information
1fd56bd drm/edid: parse YCBCR420 videomodes from EDID
cb40a11 drm: add helper to validate YCBCR420 modes
9bff1cf drm/edid: cleanup patch for CEA extended-tag macro
a2d60dc drm/edid: parse sink information before CEA blocks
fd87ef7 drm/edid: complete CEA modedb(VIC 1-107)
510e1eb drm: handle HDMI 2.0 VICs in AVI info-frames

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5184/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID

[Ville Syrjälä]

On Thu, Jul 13, 2017 at 09:03:12PM +0530, Shashank Sharma wrote:
> HDMI 2.0 spec adds support for YCBCR420 sub-sampled output.
> CEA-861-F adds two new blocks in EDID's CEA extension blocks,
> to provide information about sink's YCBCR420 output capabilities.
> 
> These blocks are:
> 
> - YCBCR420vdb(YCBCR 420 video data block):
> This block contains VICs of video modes, which can be sopported only
> in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal
> SVD block, valid for YCBCR420 modes only.
> 
> - YCBCR420cmdb(YCBCR 420 capability map data block):
> This block gives information about video modes which can support
> YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This
> block contains a bitmap index of normal svd videomodes, which can
> support YCBCR420 output too.
> So if bit 0 from first vcb byte is set, first video mode in the svd
> list can support YCBCR420 output too. Bit 1 means second video mode
> from svd list can support YCBCR420 output too, and so on.
> 
> This patch adds two bitmaps in display's hdmi_info structure, one each
> for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch
> adds:
> - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes
>   an entry in the vdb_bitmap per vic.
> - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: Emil Velikov <emil.l.velikov@gmail.com>
> 
> V2: Addressed
>     Review comments from Emil:
>     - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit.
>     - Use the suggested method for updating dbmap.
>     - Add documentation for YCBCR420_vcb_map to fix kbuild warning.
> 
>     Review comments from Ville:
>     - Do not expose the YCBCR420 flags in uabi layer, keep it internal.
>     - Save a map of YCBCR420 modes for future reference.
>     - Check db length before trying to parse extended tag.
>     - Add a warning if there are > 64 modes in capability map block.
>     - Use y420cmdb in function names and macros while dealing with vcb
>       to be aligned with spec.
>     - Move the display information parsing block ahead of mode parsing
>       blocks.
> 
> V3: Addressed design/review comments from Ville
>     - Do not add flags in video modes, else we have to expose them to user
>     - There should not be a UABI change, and kernel should detect the
>       choice of the output based on type of mode, and the bitmaps.
>     - Use standard bitops from kernel bitmap header, instead of calculating
>       bit positions manually.
> 
> V4: Addressed review comments from Ville:
>     - s/ycbcr_420_vdb/y420vdb
>     - s/ycbcr_420_vcb/y420cmdb
>     - Be less verbose on description of do_y420vdb_modes
>     - Move newmode variable in the loop scope.
>     - Use svd_to_vic() to get a VIC, instead of 0x7f
>     - Remove bitmap description for CMDB modes & VDB modes
>     - Dont add connector->ycbcr_420_allowed check for cmdb modes
>     - Remove 'len' variable, in is_y420cmdb function, which is used
>       only once
>     - Add length check in is_y420vdb function
>     - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap
>     - Do not add print about YCBCR 420 modes
>     - Fix indentation in few places
>     - Move ycbcr420_dc_modes in next patch, where its used
>     - Add a separate patch for movement of drm_add_display_info()
> 
> V5: Addressed review comments from Ville:
>     - Add the patch which cleans up the current EXTENDED_TAG usage
>     - Make y420_cmdb_map u64
>     - Do not block ycbcr420 modes while parsing the EDID, rather
>       add a separate helper function to prune ycbcr420-only modes from
>       connector's probed modes.
> 
> V6: Rebase
> V7: Move this patch after the 420_only validation patch (Ville)
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c  | 148 +++++++++++++++++++++++++++++++++++++++++++-
>  include/drm/drm_connector.h |  12 ++++
>  2 files changed, 158 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 96eee5a..b86afb9 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2783,6 +2783,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>  #define SPEAKER_BLOCK	0x04
>  #define USE_EXTENDED_TAG 0x07
>  #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
> +#define EXT_VIDEO_DATA_BLOCK_420	0x0E
> +#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
>  #define EDID_BASIC_AUDIO	(1 << 6)
>  #define EDID_CEA_YCRCB444	(1 << 5)
>  #define EDID_CEA_YCRCB422	(1 << 4)
> @@ -3155,15 +3157,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector,
>  	return newmode;
>  }
>  
> +/*
> + * do_y420vdb_modes - Parse YCBCR 420 only modes
> + * @connector: connector corresponding to the HDMI sink
> + * @svds: start of the data block of CEA YCBCR 420 VDB
> + * @len: length of the CEA YCBCR 420 VDB
> + *
> + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB)
> + * which contains modes which can be supported in YCBCR 420
> + * output format only.
> + */
> +static int do_y420vdb_modes(struct drm_connector *connector,
> +				   const u8 *svds, u8 svds_len)
> +{
> +	int modes = 0, i;
> +	struct drm_device *dev = connector->dev;
> +	struct drm_display_info *info = &connector->display_info;
> +	struct drm_hdmi_info *hdmi = &info->hdmi;
> +
> +	for (i = 0; i < svds_len; i++) {
> +		u8 vic = svd_to_vic(svds[i]);
> +		struct drm_display_mode *newmode;
> +

Hmm. Looks like here we will need the drm_valid_cea_vic() check
because the vic is coming from an untrusted source.

> +		newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
> +		if (!newmode)
> +			break;
> +		bitmap_set(hdmi->y420_vdb_modes, vic, 1);
> +		drm_mode_probed_add(connector, newmode);
> +		modes++;
> +	}
> +
> +	if (modes > 0)
> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
> +	return modes;
> +}
> +
> +/*
> + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap
> + * @connector: connector corresponding to the HDMI sink
> + * @vic: CEA vic for the video mode to be added in the map
> + *
> + * Makes an entry for a videomode in the YCBCR 420 bitmap
> + */
> +static void
> +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd)
> +{
> +	u8 vic = svd_to_vic(svd);
> +	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;

And maybe here. It's not actually needed since we know it's valid
because drm_display_mode_from_vic_index() returned a mode, but since
that's not immediately obvious from here we might want to have the
check. But if you disagree feel free to leave it out.

At some point I think we might want to reorganize
drm_display_mode_from_vic_index() such that we actually pass it the
vic instead. Then we could also pass the vic here directly.

> +
> +	bitmap_set(hdmi->y420_cmdb_modes, vic, 1);
> +}
> +
>  static int
>  do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
>  {
>  	int i, modes = 0;
> +	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
>  
>  	for (i = 0; i < len; i++) {
>  		struct drm_display_mode *mode;
>  		mode = drm_display_mode_from_vic_index(connector, db, len, i);
>  		if (mode) {
> +			/*
> +			 * YCBCR420 capability block contains a bitmap which
> +			 * gives the index of CEA modes from CEA VDB, which
> +			 * can support YCBCR 420 sampling output also (apart
> +			 * from RGB/YCBCR444 etc).
> +			 * For example, if the bit 0 in bitmap is set,
> +			 * first mode in VDB can support YCBCR420 output too.
> +			 * Add YCBCR420 modes only if sink is HDMI 2.0 capable.
> +			 */
> +			if (hdmi->y420_cmdb_map & (1 << i))

I think this needs to be something like
if (i < 64 && hdmi->y420_cmdb_map & (1ULL << i))

With those changes I think this part should be good to go.

> +				drm_add_cmdb_modes(connector, db[i]);
> +
>  			drm_mode_probed_add(connector, mode);
>  			modes++;
>  		}
> @@ -3505,9 +3571,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
>  	return oui == HDMI_FORUM_IEEE_OUI;
>  }
>  
> +static bool cea_db_is_y420cmdb(const u8 *db)
> +{
> +
> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
> +		return false;
> +
> +	if (!cea_db_payload_len(db))
> +		return false;
> +
> +	if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool cea_db_is_y420vdb(const u8 *db)
> +{
> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
> +		return false;
> +
> +	if (!cea_db_payload_len(db))
> +		return false;
> +
> +	if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420)
> +		return false;
> +
> +	return true;
> +}
> +
>  #define for_each_cea_db(cea, i, start, end) \
>  	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
>  
> +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
> +				     const u8 *db)
> +{
> +	struct drm_display_info *info = &connector->display_info;
> +	struct drm_hdmi_info *hdmi = &info->hdmi;
> +	u8 map_len = cea_db_payload_len(db) - 1;
> +	u8 count;
> +	u64 map = 0;
> +
> +	if (map_len == 0) {
> +		/* All CEA modes support ycbcr420 sampling also.*/
> +		hdmi->y420_cmdb_map = U64_MAX;
> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
> +		return;
> +	}
> +
> +	/*
> +	 * This map indicates which of the existing CEA block modes
> +	 * from VDB can support YCBCR420 output too. So if bit=0 is
> +	 * set, first mode from VDB can support YCBCR420 output too.
> +	 * We will parse and keep this map, before parsing VDB itself
> +	 * to avoid going through the same block again and again.
> +	 *
> +	 * Spec is not clear about max possible size of this block.
> +	 * Clamping max bitmap block size at 8 bytes. Every byte can
> +	 * address 8 CEA modes, in this way this map can address
> +	 * 8*8 = first 64 SVDs.
> +	 */
> +	if (WARN_ON_ONCE(map_len > 8))
> +		map_len = 8;
> +
> +	for (count = 0; count < map_len; count++)
> +		map |= (u64)db[2 + count] << (8 * count);
> +
> +	if (map)
> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
> +
> +	hdmi->y420_cmdb_map = map;
> +}
> +
>  static int
>  add_cea_modes(struct drm_connector *connector, struct edid *edid)
>  {
> @@ -3530,10 +3665,16 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
>  				video = db + 1;
>  				video_len = dbl;
>  				modes += do_cea_modes(connector, video, dbl);
> -			}
> -			else if (cea_db_is_hdmi_vsdb(db)) {
> +			} else if (cea_db_is_hdmi_vsdb(db)) {
>  				hdmi = db;
>  				hdmi_len = dbl;
> +			} else if (cea_db_is_y420vdb(db)) {
> +				const u8 *vdb420 = &db[2];
> +
> +				/* Add 4:2:0(only) modes present in EDID */
> +				modes += do_y420vdb_modes(connector,
> +							  vdb420,
> +							  dbl - 1);
>  			}
>  		}
>  	}
> @@ -4216,6 +4357,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>  			drm_parse_hdmi_vsdb_video(connector, db);
>  		if (cea_db_is_hdmi_forum_vsdb(db))
>  			drm_parse_hdmi_forum_vsdb(connector, db);
> +		if (cea_db_is_y420cmdb(db))
> +			drm_parse_y420cmdb_bitmap(connector, db);
>  	}
>  }
>  
> @@ -4477,6 +4620,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>  	num_modes += add_cea_modes(connector, edid);
>  	num_modes += add_alternate_cea_modes(connector, edid);
>  	num_modes += add_displayid_detailed_modes(connector, edid);
> +
>  	if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
>  		num_modes += add_inferred_modes(connector, edid);
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 26dd3eb..225e092 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -143,6 +143,17 @@ struct drm_hdmi_info {
>  	 * upto 128 VICs;
>  	 */
>  	unsigned long y420_vdb_modes[BITS_TO_LONGS(128)];
> +
> +	/**
> +	 * @y420_cmdb_modes: bitmap of modes which can support ycbcr420
> +	 * output also, along with normal HDMI outputs. There are total 107
> +	 * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto
> +	 * 128 VICs;
> +	 */
> +	unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)];
> +
> +	/** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */
> +	u64 y420_cmdb_map;
>  };
>  
>  /**
> @@ -206,6 +217,7 @@ struct drm_display_info {
>  #define DRM_COLOR_FORMAT_RGB444		(1<<0)
>  #define DRM_COLOR_FORMAT_YCRCB444	(1<<1)
>  #define DRM_COLOR_FORMAT_YCRCB422	(1<<2)
> +#define DRM_COLOR_FORMAT_YCRCB420	(1<<3)
>  
>  	/**
>  	 * @color_formats: HDMI Color formats, selects between RGB and YCrCb
> -- 
> 2.7.4

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

Re: [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID

[Sharma, Shashank]

Regards

Shashank


On 7/13/2017 9:51 PM, Ville Syrjälä wrote:
> On Thu, Jul 13, 2017 at 09:03:12PM +0530, Shashank Sharma wrote:
>> HDMI 2.0 spec adds support for YCBCR420 sub-sampled output.
>> CEA-861-F adds two new blocks in EDID's CEA extension blocks,
>> to provide information about sink's YCBCR420 output capabilities.
>>
>> These blocks are:
>>
>> - YCBCR420vdb(YCBCR 420 video data block):
>> This block contains VICs of video modes, which can be sopported only
>> in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal
>> SVD block, valid for YCBCR420 modes only.
>>
>> - YCBCR420cmdb(YCBCR 420 capability map data block):
>> This block gives information about video modes which can support
>> YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This
>> block contains a bitmap index of normal svd videomodes, which can
>> support YCBCR420 output too.
>> So if bit 0 from first vcb byte is set, first video mode in the svd
>> list can support YCBCR420 output too. Bit 1 means second video mode
>> from svd list can support YCBCR420 output too, and so on.
>>
>> This patch adds two bitmaps in display's hdmi_info structure, one each
>> for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch
>> adds:
>> - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes
>>    an entry in the vdb_bitmap per vic.
>> - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Jose Abreu <joabreu@synopsys.com>
>> Cc: Emil Velikov <emil.l.velikov@gmail.com>
>>
>> V2: Addressed
>>      Review comments from Emil:
>>      - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit.
>>      - Use the suggested method for updating dbmap.
>>      - Add documentation for YCBCR420_vcb_map to fix kbuild warning.
>>
>>      Review comments from Ville:
>>      - Do not expose the YCBCR420 flags in uabi layer, keep it internal.
>>      - Save a map of YCBCR420 modes for future reference.
>>      - Check db length before trying to parse extended tag.
>>      - Add a warning if there are > 64 modes in capability map block.
>>      - Use y420cmdb in function names and macros while dealing with vcb
>>        to be aligned with spec.
>>      - Move the display information parsing block ahead of mode parsing
>>        blocks.
>>
>> V3: Addressed design/review comments from Ville
>>      - Do not add flags in video modes, else we have to expose them to user
>>      - There should not be a UABI change, and kernel should detect the
>>        choice of the output based on type of mode, and the bitmaps.
>>      - Use standard bitops from kernel bitmap header, instead of calculating
>>        bit positions manually.
>>
>> V4: Addressed review comments from Ville:
>>      - s/ycbcr_420_vdb/y420vdb
>>      - s/ycbcr_420_vcb/y420cmdb
>>      - Be less verbose on description of do_y420vdb_modes
>>      - Move newmode variable in the loop scope.
>>      - Use svd_to_vic() to get a VIC, instead of 0x7f
>>      - Remove bitmap description for CMDB modes & VDB modes
>>      - Dont add connector->ycbcr_420_allowed check for cmdb modes
>>      - Remove 'len' variable, in is_y420cmdb function, which is used
>>        only once
>>      - Add length check in is_y420vdb function
>>      - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap
>>      - Do not add print about YCBCR 420 modes
>>      - Fix indentation in few places
>>      - Move ycbcr420_dc_modes in next patch, where its used
>>      - Add a separate patch for movement of drm_add_display_info()
>>
>> V5: Addressed review comments from Ville:
>>      - Add the patch which cleans up the current EXTENDED_TAG usage
>>      - Make y420_cmdb_map u64
>>      - Do not block ycbcr420 modes while parsing the EDID, rather
>>        add a separate helper function to prune ycbcr420-only modes from
>>        connector's probed modes.
>>
>> V6: Rebase
>> V7: Move this patch after the 420_only validation patch (Ville)
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c  | 148 +++++++++++++++++++++++++++++++++++++++++++-
>>   include/drm/drm_connector.h |  12 ++++
>>   2 files changed, 158 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 96eee5a..b86afb9 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2783,6 +2783,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>>   #define SPEAKER_BLOCK	0x04
>>   #define USE_EXTENDED_TAG 0x07
>>   #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
>> +#define EXT_VIDEO_DATA_BLOCK_420	0x0E
>> +#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
>>   #define EDID_BASIC_AUDIO	(1 << 6)
>>   #define EDID_CEA_YCRCB444	(1 << 5)
>>   #define EDID_CEA_YCRCB422	(1 << 4)
>> @@ -3155,15 +3157,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector,
>>   	return newmode;
>>   }
>>   
>> +/*
>> + * do_y420vdb_modes - Parse YCBCR 420 only modes
>> + * @connector: connector corresponding to the HDMI sink
>> + * @svds: start of the data block of CEA YCBCR 420 VDB
>> + * @len: length of the CEA YCBCR 420 VDB
>> + *
>> + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB)
>> + * which contains modes which can be supported in YCBCR 420
>> + * output format only.
>> + */
>> +static int do_y420vdb_modes(struct drm_connector *connector,
>> +				   const u8 *svds, u8 svds_len)
>> +{
>> +	int modes = 0, i;
>> +	struct drm_device *dev = connector->dev;
>> +	struct drm_display_info *info = &connector->display_info;
>> +	struct drm_hdmi_info *hdmi = &info->hdmi;
>> +
>> +	for (i = 0; i < svds_len; i++) {
>> +		u8 vic = svd_to_vic(svds[i]);
>> +		struct drm_display_mode *newmode;
>> +
> Hmm. Looks like here we will need the drm_valid_cea_vic() check
> because the vic is coming from an untrusted source.
Agree, there should be one.
>> +		newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
>> +		if (!newmode)
>> +			break;
>> +		bitmap_set(hdmi->y420_vdb_modes, vic, 1);
>> +		drm_mode_probed_add(connector, newmode);
>> +		modes++;
>> +	}
>> +
>> +	if (modes > 0)
>> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
>> +	return modes;
>> +}
>> +
>> +/*
>> + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap
>> + * @connector: connector corresponding to the HDMI sink
>> + * @vic: CEA vic for the video mode to be added in the map
>> + *
>> + * Makes an entry for a videomode in the YCBCR 420 bitmap
>> + */
>> +static void
>> +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd)
>> +{
>> +	u8 vic = svd_to_vic(svd);
>> +	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
> And maybe here. It's not actually needed since we know it's valid
> because drm_display_mode_from_vic_index() returned a mode, but since
> that's not immediately obvious from here we might want to have the
> check. But if you disagree feel free to leave it out.
I guess its a good idea to be careful while adding modes, I can add a 
check here too.
> At some point I think we might want to reorganize
> drm_display_mode_from_vic_index() such that we actually pass it the
> vic instead. Then we could also pass the vic here directly.
>
>> +
>> +	bitmap_set(hdmi->y420_cmdb_modes, vic, 1);
>> +}
>> +
>>   static int
>>   do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
>>   {
>>   	int i, modes = 0;
>> +	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
>>   
>>   	for (i = 0; i < len; i++) {
>>   		struct drm_display_mode *mode;
>>   		mode = drm_display_mode_from_vic_index(connector, db, len, i);
>>   		if (mode) {
>> +			/*
>> +			 * YCBCR420 capability block contains a bitmap which
>> +			 * gives the index of CEA modes from CEA VDB, which
>> +			 * can support YCBCR 420 sampling output also (apart
>> +			 * from RGB/YCBCR444 etc).
>> +			 * For example, if the bit 0 in bitmap is set,
>> +			 * first mode in VDB can support YCBCR420 output too.
>> +			 * Add YCBCR420 modes only if sink is HDMI 2.0 capable.
>> +			 */
>> +			if (hdmi->y420_cmdb_map & (1 << i))
> I think this needs to be something like
> if (i < 64 && hdmi->y420_cmdb_map & (1ULL << i))
Yeah, that's also one preventive measure, which we can add.
- Shashank
>
> With those changes I think this part should be good to go.
>
>> +				drm_add_cmdb_modes(connector, db[i]);
>> +
>>   			drm_mode_probed_add(connector, mode);
>>   			modes++;
>>   		}
>> @@ -3505,9 +3571,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
>>   	return oui == HDMI_FORUM_IEEE_OUI;
>>   }
>>   
>> +static bool cea_db_is_y420cmdb(const u8 *db)
>> +{
>> +
>> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (!cea_db_payload_len(db))
>> +		return false;
>> +
>> +	if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB)
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>> +static bool cea_db_is_y420vdb(const u8 *db)
>> +{
>> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (!cea_db_payload_len(db))
>> +		return false;
>> +
>> +	if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420)
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>>   #define for_each_cea_db(cea, i, start, end) \
>>   	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
>>   
>> +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
>> +				     const u8 *db)
>> +{
>> +	struct drm_display_info *info = &connector->display_info;
>> +	struct drm_hdmi_info *hdmi = &info->hdmi;
>> +	u8 map_len = cea_db_payload_len(db) - 1;
>> +	u8 count;
>> +	u64 map = 0;
>> +
>> +	if (map_len == 0) {
>> +		/* All CEA modes support ycbcr420 sampling also.*/
>> +		hdmi->y420_cmdb_map = U64_MAX;
>> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
>> +		return;
>> +	}
>> +
>> +	/*
>> +	 * This map indicates which of the existing CEA block modes
>> +	 * from VDB can support YCBCR420 output too. So if bit=0 is
>> +	 * set, first mode from VDB can support YCBCR420 output too.
>> +	 * We will parse and keep this map, before parsing VDB itself
>> +	 * to avoid going through the same block again and again.
>> +	 *
>> +	 * Spec is not clear about max possible size of this block.
>> +	 * Clamping max bitmap block size at 8 bytes. Every byte can
>> +	 * address 8 CEA modes, in this way this map can address
>> +	 * 8*8 = first 64 SVDs.
>> +	 */
>> +	if (WARN_ON_ONCE(map_len > 8))
>> +		map_len = 8;
>> +
>> +	for (count = 0; count < map_len; count++)
>> +		map |= (u64)db[2 + count] << (8 * count);
>> +
>> +	if (map)
>> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
>> +
>> +	hdmi->y420_cmdb_map = map;
>> +}
>> +
>>   static int
>>   add_cea_modes(struct drm_connector *connector, struct edid *edid)
>>   {
>> @@ -3530,10 +3665,16 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
>>   				video = db + 1;
>>   				video_len = dbl;
>>   				modes += do_cea_modes(connector, video, dbl);
>> -			}
>> -			else if (cea_db_is_hdmi_vsdb(db)) {
>> +			} else if (cea_db_is_hdmi_vsdb(db)) {
>>   				hdmi = db;
>>   				hdmi_len = dbl;
>> +			} else if (cea_db_is_y420vdb(db)) {
>> +				const u8 *vdb420 = &db[2];
>> +
>> +				/* Add 4:2:0(only) modes present in EDID */
>> +				modes += do_y420vdb_modes(connector,
>> +							  vdb420,
>> +							  dbl - 1);
>>   			}
>>   		}
>>   	}
>> @@ -4216,6 +4357,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>   			drm_parse_hdmi_vsdb_video(connector, db);
>>   		if (cea_db_is_hdmi_forum_vsdb(db))
>>   			drm_parse_hdmi_forum_vsdb(connector, db);
>> +		if (cea_db_is_y420cmdb(db))
>> +			drm_parse_y420cmdb_bitmap(connector, db);
>>   	}
>>   }
>>   
>> @@ -4477,6 +4620,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>>   	num_modes += add_cea_modes(connector, edid);
>>   	num_modes += add_alternate_cea_modes(connector, edid);
>>   	num_modes += add_displayid_detailed_modes(connector, edid);
>> +
>>   	if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
>>   		num_modes += add_inferred_modes(connector, edid);
>>   
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 26dd3eb..225e092 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -143,6 +143,17 @@ struct drm_hdmi_info {
>>   	 * upto 128 VICs;
>>   	 */
>>   	unsigned long y420_vdb_modes[BITS_TO_LONGS(128)];
>> +
>> +	/**
>> +	 * @y420_cmdb_modes: bitmap of modes which can support ycbcr420
>> +	 * output also, along with normal HDMI outputs. There are total 107
>> +	 * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto
>> +	 * 128 VICs;
>> +	 */
>> +	unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)];
>> +
>> +	/** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */
>> +	u64 y420_cmdb_map;
>>   };
>>   
>>   /**
>> @@ -206,6 +217,7 @@ struct drm_display_info {
>>   #define DRM_COLOR_FORMAT_RGB444		(1<<0)
>>   #define DRM_COLOR_FORMAT_YCRCB444	(1<<1)
>>   #define DRM_COLOR_FORMAT_YCRCB422	(1<<2)
>> +#define DRM_COLOR_FORMAT_YCRCB420	(1<<3)
>>   
>>   	/**
>>   	 * @color_formats: HDMI Color formats, selects between RGB and YCrCb
>> -- 
>> 2.7.4

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

[PATCH v8 06/14] drm/edid: parse YCBCR420 videomodes from EDID

[Shashank Sharma]

HDMI 2.0 spec adds support for YCBCR420 sub-sampled output.
CEA-861-F adds two new blocks in EDID's CEA extension blocks,
to provide information about sink's YCBCR420 output capabilities.

These blocks are:

- YCBCR420vdb(YCBCR 420 video data block):
This block contains VICs of video modes, which can be sopported only
in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal
SVD block, valid for YCBCR420 modes only.

- YCBCR420cmdb(YCBCR 420 capability map data block):
This block gives information about video modes which can support
YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This
block contains a bitmap index of normal svd videomodes, which can
support YCBCR420 output too.
So if bit 0 from first vcb byte is set, first video mode in the svd
list can support YCBCR420 output too. Bit 1 means second video mode
from svd list can support YCBCR420 output too, and so on.

This patch adds two bitmaps in display's hdmi_info structure, one each
for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch
adds:
- VDB modes (YCBCR 420 only modes) in connector's mode list, also makes
  an entry in the vdb_bitmap per vic.
- VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: Emil Velikov <emil.l.velikov@gmail.com>

V2: Addressed
    Review comments from Emil:
    - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit.
    - Use the suggested method for updating dbmap.
    - Add documentation for YCBCR420_vcb_map to fix kbuild warning.

    Review comments from Ville:
    - Do not expose the YCBCR420 flags in uabi layer, keep it internal.
    - Save a map of YCBCR420 modes for future reference.
    - Check db length before trying to parse extended tag.
    - Add a warning if there are > 64 modes in capability map block.
    - Use y420cmdb in function names and macros while dealing with vcb
      to be aligned with spec.
    - Move the display information parsing block ahead of mode parsing
      blocks.

V3: Addressed design/review comments from Ville
    - Do not add flags in video modes, else we have to expose them to user
    - There should not be a UABI change, and kernel should detect the
      choice of the output based on type of mode, and the bitmaps.
    - Use standard bitops from kernel bitmap header, instead of calculating
      bit positions manually.

V4: Addressed review comments from Ville:
    - s/ycbcr_420_vdb/y420vdb
    - s/ycbcr_420_vcb/y420cmdb
    - Be less verbose on description of do_y420vdb_modes
    - Move newmode variable in the loop scope.
    - Use svd_to_vic() to get a VIC, instead of 0x7f
    - Remove bitmap description for CMDB modes & VDB modes
    - Dont add connector->ycbcr_420_allowed check for cmdb modes
    - Remove 'len' variable, in is_y420cmdb function, which is used
      only once
    - Add length check in is_y420vdb function
    - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap
    - Do not add print about YCBCR 420 modes
    - Fix indentation in few places
    - Move ycbcr420_dc_modes in next patch, where its used
    - Add a separate patch for movement of drm_add_display_info()

V5: Addressed review comments from Ville:
    - Add the patch which cleans up the current EXTENDED_TAG usage
    - Make y420_cmdb_map u64
    - Do not block ycbcr420 modes while parsing the EDID, rather
      add a separate helper function to prune ycbcr420-only modes from
      connector's probed modes.

V6: Rebase
V7: Move this patch after the 420_only validation patch (Ville)
V8: Addressed review comments from Ville
    - use cea_vic_valid check before adding cmdb/vdb modes
    - add check for i < 64 while adding cmdb modes
    - use 1ULL while checking bitmap

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c  | 154 +++++++++++++++++++++++++++++++++++++++++++-
 include/drm/drm_connector.h |  12 ++++
 2 files changed, 164 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 96eee5a..9fa0d7f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2783,6 +2783,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 #define SPEAKER_BLOCK	0x04
 #define USE_EXTENDED_TAG 0x07
 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
+#define EXT_VIDEO_DATA_BLOCK_420	0x0E
+#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
 #define EDID_BASIC_AUDIO	(1 << 6)
 #define EDID_CEA_YCRCB444	(1 << 5)
 #define EDID_CEA_YCRCB422	(1 << 4)
@@ -3155,15 +3157,85 @@ drm_display_mode_from_vic_index(struct drm_connector *connector,
 	return newmode;
 }
 
+/*
+ * do_y420vdb_modes - Parse YCBCR 420 only modes
+ * @connector: connector corresponding to the HDMI sink
+ * @svds: start of the data block of CEA YCBCR 420 VDB
+ * @len: length of the CEA YCBCR 420 VDB
+ *
+ * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB)
+ * which contains modes which can be supported in YCBCR 420
+ * output format only.
+ */
+static int do_y420vdb_modes(struct drm_connector *connector,
+				   const u8 *svds, u8 svds_len)
+{
+	int modes = 0, i;
+	struct drm_device *dev = connector->dev;
+	struct drm_display_info *info = &connector->display_info;
+	struct drm_hdmi_info *hdmi = &info->hdmi;
+
+	for (i = 0; i < svds_len; i++) {
+		u8 vic = svd_to_vic(svds[i]);
+		struct drm_display_mode *newmode;
+
+		if (!drm_valid_cea_vic(vic))
+			continue;
+
+		newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
+		if (!newmode)
+			break;
+		bitmap_set(hdmi->y420_vdb_modes, vic, 1);
+		drm_mode_probed_add(connector, newmode);
+		modes++;
+	}
+
+	if (modes > 0)
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
+	return modes;
+}
+
+/*
+ * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap
+ * @connector: connector corresponding to the HDMI sink
+ * @vic: CEA vic for the video mode to be added in the map
+ *
+ * Makes an entry for a videomode in the YCBCR 420 bitmap
+ */
+static void
+drm_add_cmdb_modes(struct drm_connector *connector, u8 svd)
+{
+	u8 vic = svd_to_vic(svd);
+	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
+
+	if (!drm_valid_cea_vic(vic))
+		return;
+
+	bitmap_set(hdmi->y420_cmdb_modes, vic, 1);
+}
+
 static int
 do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
 {
 	int i, modes = 0;
+	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
 
 	for (i = 0; i < len; i++) {
 		struct drm_display_mode *mode;
 		mode = drm_display_mode_from_vic_index(connector, db, len, i);
 		if (mode) {
+			/*
+			 * YCBCR420 capability block contains a bitmap which
+			 * gives the index of CEA modes from CEA VDB, which
+			 * can support YCBCR 420 sampling output also (apart
+			 * from RGB/YCBCR444 etc).
+			 * For example, if the bit 0 in bitmap is set,
+			 * first mode in VDB can support YCBCR420 output too.
+			 * Add YCBCR420 modes only if sink is HDMI 2.0 capable.
+			 */
+			if (i < 64 && hdmi->y420_cmdb_map & (1ULL << i))
+				drm_add_cmdb_modes(connector, db[i]);
+
 			drm_mode_probed_add(connector, mode);
 			modes++;
 		}
@@ -3505,9 +3577,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
 	return oui == HDMI_FORUM_IEEE_OUI;
 }
 
+static bool cea_db_is_y420cmdb(const u8 *db)
+{
+
+	if (cea_db_tag(db) != USE_EXTENDED_TAG)
+		return false;
+
+	if (!cea_db_payload_len(db))
+		return false;
+
+	if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB)
+		return false;
+
+	return true;
+}
+
+static bool cea_db_is_y420vdb(const u8 *db)
+{
+	if (cea_db_tag(db) != USE_EXTENDED_TAG)
+		return false;
+
+	if (!cea_db_payload_len(db))
+		return false;
+
+	if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420)
+		return false;
+
+	return true;
+}
+
 #define for_each_cea_db(cea, i, start, end) \
 	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
 
+static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
+				     const u8 *db)
+{
+	struct drm_display_info *info = &connector->display_info;
+	struct drm_hdmi_info *hdmi = &info->hdmi;
+	u8 map_len = cea_db_payload_len(db) - 1;
+	u8 count;
+	u64 map = 0;
+
+	if (map_len == 0) {
+		/* All CEA modes support ycbcr420 sampling also.*/
+		hdmi->y420_cmdb_map = U64_MAX;
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
+		return;
+	}
+
+	/*
+	 * This map indicates which of the existing CEA block modes
+	 * from VDB can support YCBCR420 output too. So if bit=0 is
+	 * set, first mode from VDB can support YCBCR420 output too.
+	 * We will parse and keep this map, before parsing VDB itself
+	 * to avoid going through the same block again and again.
+	 *
+	 * Spec is not clear about max possible size of this block.
+	 * Clamping max bitmap block size at 8 bytes. Every byte can
+	 * address 8 CEA modes, in this way this map can address
+	 * 8*8 = first 64 SVDs.
+	 */
+	if (WARN_ON_ONCE(map_len > 8))
+		map_len = 8;
+
+	for (count = 0; count < map_len; count++)
+		map |= (u64)db[2 + count] << (8 * count);
+
+	if (map)
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
+
+	hdmi->y420_cmdb_map = map;
+}
+
 static int
 add_cea_modes(struct drm_connector *connector, struct edid *edid)
 {
@@ -3530,10 +3671,16 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
 				video = db + 1;
 				video_len = dbl;
 				modes += do_cea_modes(connector, video, dbl);
-			}
-			else if (cea_db_is_hdmi_vsdb(db)) {
+			} else if (cea_db_is_hdmi_vsdb(db)) {
 				hdmi = db;
 				hdmi_len = dbl;
+			} else if (cea_db_is_y420vdb(db)) {
+				const u8 *vdb420 = &db[2];
+
+				/* Add 4:2:0(only) modes present in EDID */
+				modes += do_y420vdb_modes(connector,
+							  vdb420,
+							  dbl - 1);
 			}
 		}
 	}
@@ -4216,6 +4363,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 			drm_parse_hdmi_vsdb_video(connector, db);
 		if (cea_db_is_hdmi_forum_vsdb(db))
 			drm_parse_hdmi_forum_vsdb(connector, db);
+		if (cea_db_is_y420cmdb(db))
+			drm_parse_y420cmdb_bitmap(connector, db);
 	}
 }
 
@@ -4477,6 +4626,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 	num_modes += add_cea_modes(connector, edid);
 	num_modes += add_alternate_cea_modes(connector, edid);
 	num_modes += add_displayid_detailed_modes(connector, edid);
+
 	if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
 		num_modes += add_inferred_modes(connector, edid);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 26dd3eb..225e092 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -143,6 +143,17 @@ struct drm_hdmi_info {
 	 * upto 128 VICs;
 	 */
 	unsigned long y420_vdb_modes[BITS_TO_LONGS(128)];
+
+	/**
+	 * @y420_cmdb_modes: bitmap of modes which can support ycbcr420
+	 * output also, along with normal HDMI outputs. There are total 107
+	 * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto
+	 * 128 VICs;
+	 */
+	unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)];
+
+	/** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */
+	u64 y420_cmdb_map;
 };
 
 /**
@@ -206,6 +217,7 @@ struct drm_display_info {
 #define DRM_COLOR_FORMAT_RGB444		(1<<0)
 #define DRM_COLOR_FORMAT_YCRCB444	(1<<1)
 #define DRM_COLOR_FORMAT_YCRCB422	(1<<2)
+#define DRM_COLOR_FORMAT_YCRCB420	(1<<3)
 
 	/**
 	 * @color_formats: HDMI Color formats, selects between RGB and YCrCb
-- 
2.7.4

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

✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev3)

[Patchwork]

== Series Details ==

Series: YCBCR 4:2:0 handling in DRM layer (rev3)
URL   : https://patchwork.freedesktop.org/series/26972/
State : success

== Summary ==

Series 26972v3 YCBCR 4:2:0 handling in DRM layer
https://patchwork.freedesktop.org/api/1.0/series/26972/revisions/3/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (fi-skl-6770hq) fdo#99739
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1

fdo#99739 https://bugs.freedesktop.org/show_bug.cgi?id=99739
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:441s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:431s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:351s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:529s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:505s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:491s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:481s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:597s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:438s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:409s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:423s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:493s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:458s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:574s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:583s
fi-pnv-d510      total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  time:562s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:456s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:585s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:468s
fi-skl-6770hq    total:279  pass:268  dwarn:0   dfail:0   fail:1   skip:10  time:467s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:432s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:472s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:399s

5182881eef4843638e993d15f93a8dbb8c89e429 drm-tip: 2017y-07m-13d-18h-13m-28s UTC integration manifest
219b18a drm/i915/glk: set HDMI 2.0 identifier
062c202 drm/i915: set colorspace for YCBCR420 outputs
4f53995 drm/i915: prepare csc unit for YCBCR420 output
bc10a02 drm/i915: prepare pipe for YCBCR420 output
eb22487 drm/i915: prepare scaler for YCBCR420 modeset
5a003aa drm/i915: add config function for YCBCR420 outputs
d646c9b drm: add helper functions for YCBCR420 handling
3834ed9 drm/edid: parse ycbcr 420 deep color information
7434121 drm/edid: parse YCBCR420 videomodes from EDID
80d0b03 drm: add helper to validate YCBCR420 modes
7d0cad6 drm/edid: cleanup patch for CEA extended-tag macro
407172b drm/edid: parse sink information before CEA blocks
0d3a770 drm/edid: complete CEA modedb(VIC 1-107)
f29dc09 drm: handle HDMI 2.0 VICs in AVI info-frames

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5188/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs

[Ville Syrjälä]

On Thu, Jul 13, 2017 at 09:03:15PM +0530, Shashank Sharma wrote:
> This patch checks encoder level support for YCBCR420 outputs.
> The logic goes as simple as this:
> If the input mode is YCBCR420-only mode: prepare HDMI for
> YCBCR420 output, else continue with RGB output mode.
> 
> It checks if the mode is YCBCR420 and source can support this
> output then it marks the ycbcr_420 output indicator into crtc
> state, for further staging in driver.
> 
> V2: Split the patch into two, kept helper functions in DRM layer.
> V3: Changed the compute_config function based on new DRM API.
> V4: Rebase
> V5: Rebase
> V6: Check and handle YCBCR420-only modes, discard the property
>     based approach (Ville)
> V7: Addressed review comments from Ville
>     - add else case in 12BPC check.
>     - extract ycbcr420 state inside hdmi_12bpc_possible function.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  1 +
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +++
>  drivers/gpu/drm/i915/intel_hdmi.c    | 43 +++++++++++++++++++++++++++++++++---
>  3 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2144adc..a5140e4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11945,6 +11945,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>  	PIPE_CONF_CHECK_I(hdmi_scrambling);
>  	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
>  	PIPE_CONF_CHECK_I(has_infoframe);
> +	PIPE_CONF_CHECK_I(ycbcr420);
>  
>  	PIPE_CONF_CHECK_I(has_audio);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d17a324..592243b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -780,6 +780,9 @@ struct intel_crtc_state {
>  
>  	/* HDMI High TMDS char rate ratio */
>  	bool hdmi_high_tmds_clock_ratio;
> +
> +	/* HDMI output type */

We'll want DP too at some point. So that's going to get stale pretty
soon.

> +	bool ycbcr420;
>  };
>  
>  struct intel_crtc {
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index cc0d100..eb6243c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1312,6 +1312,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>  	struct drm_atomic_state *state = crtc_state->base.state;
>  	struct drm_connector_state *connector_state;
>  	struct drm_connector *connector;
> +	bool output_ycbcr420 = crtc_state->ycbcr420;

Pointless variable. Just use 'crtc_state->ycbcr420' in the code
directly.

>  	int i;
>  
>  	if (HAS_GMCH_DISPLAY(dev_priv))
> @@ -1330,8 +1331,16 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>  		if (connector_state->crtc != crtc_state->base.crtc)
>  			continue;
>  
> -		if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0)
> -			return false;
> +		if (output_ycbcr420) {
> +			const struct drm_hdmi_info *hdmi = &info->hdmi;
> +
> +			if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
> +				return false;
> +		} else {
> +

Bogus whitespace.

> +			if (!(info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36))
> +				return false;
> +		}
>  	}
>  
>  	/* Display Wa #1139 */
> @@ -1342,6 +1351,25 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>  	return true;
>  }
>  
> +static bool
> +intel_hdmi_ycbcr420_config(struct drm_connector *connector,
> +			       struct intel_crtc_state *config,
> +			       int *clock_12bpc, int *clock_8bpc)
> +{
> +

Bogus whitespace.

> +	if (!connector->ycbcr_420_allowed) {
> +		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
> +		return false;
> +	}
> +
> +	/* YCBCR420 TMDS rate requirement is half the pixel clock */
> +	config->port_clock /= 2;
> +	*clock_12bpc /= 2;
> +	*clock_8bpc /= 2;
> +	config->ycbcr420 = true;
> +	return true;
> +}
> +
>  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  			       struct intel_crtc_state *pipe_config,
>  			       struct drm_connector_state *conn_state)
> @@ -1349,7 +1377,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> -	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
> +	struct drm_connector *connector = conn_state->connector;
> +	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
>  	struct intel_digital_connector_state *intel_conn_state =
>  		to_intel_digital_connector_state(conn_state);
>  	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
> @@ -1379,6 +1408,14 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  		clock_12bpc *= 2;
>  	}
>  
> +	if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) {
> +		if (!intel_hdmi_ycbcr420_config(connector, pipe_config,
> +						&clock_12bpc, &clock_8bpc)) {
> +			DRM_ERROR("Can't support YCBCR420 output\n");
> +			return false;
> +		}
> +	}
> +
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
>  		pipe_config->has_pch_encoder = true;
>  
> -- 
> 2.7.4

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

Re: [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset

[Ville Syrjälä]

On Thu, Jul 13, 2017 at 09:03:16PM +0530, Shashank Sharma wrote:
> To get a YCBCR420 output from intel platforms, we need one
> scaler to scale down YCBCR444 samples to YCBCR420 samples.
> 
> This patch:
> - Does scaler allocation for HDMI ycbcr420 outputs.
> - Programs PIPE_MISC register for ycbcr420 output.
> - Adds a new scaler user "HDMI output" to plug-into existing
>   scaler framework. This output type is identified using bit
>   30 of the scaler users bitmap.
> 
> V2: rebase
> V3: rebase
> V4: rebase
> V5: addressed review comments from Ander:
>     - No need to check both scaler_user && hdmi_output.
>       Check for scaler_user is enough.
> V6: rebase
> V7: Do not create a new scaler user, use existing pipe scaler user.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Ander Conselvan De Oliveira <conselvan2@gmail.com>
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  3 +++
>  drivers/gpu/drm/i915/intel_drv.h     |  4 +++-
>  drivers/gpu/drm/i915/intel_hdmi.c    | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
>  4 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a5140e4..d78f1c2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4624,6 +4624,9 @@ 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)
> +		need_scaling = true;
> +
>  	/*
>  	 * Scaling/fitting not supported in IF-ID mode in GEN9+
>  	 * TODO: Interlace fetch mode doesn't support YUV420 planar formats.
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 592243b..94ea6ed 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -471,7 +471,8 @@ struct intel_crtc_scaler_state {
>  	 *
>  	 *     If a bit is set, a user is using a scaler.
>  	 *     Here user can be a plane or crtc as defined below:
> -	 *       bits 0-30 - plane (bit position is index from drm_plane_index)
> +	 *       bits 0-29 - plane (bit position is index from drm_plane_index)
> +	 *       bit 30    - hdmi output

That's no longer true.

>  	 *       bit 31    - crtc
>  	 *
>  	 * Instead of creating a new index to cover planes and crtc, using
> @@ -484,6 +485,7 @@ struct intel_crtc_scaler_state {
>  	 * avilability.
>  	 */
>  #define SKL_CRTC_INDEX 31
> +

Bogus whitespace change.

>  	unsigned scaler_users;
>  
>  	/* scaler used by crtc for panel fitting purpose */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index eb6243c..49f4fb8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1356,6 +1356,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
>  			       struct intel_crtc_state *config,
>  			       int *clock_12bpc, int *clock_8bpc)
>  {
> +	struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc);
>  
>  	if (!connector->ycbcr_420_allowed) {
>  		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
> @@ -1367,6 +1368,17 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
>  	*clock_12bpc /= 2;
>  	*clock_8bpc /= 2;
>  	config->ycbcr420 = true;
> +
> +	/* YCBCR 420 output conversion needs a scaler */
> +	if (skl_update_scaler_crtc(config)) {
> +		DRM_ERROR("Scaler allocation for output failed\n");

DRM_DEBUG_KMS

> +		return false;
> +	}
> +
> +	/* Bind this scaler to pipe */

Unnecesary comment.

> +	intel_pch_panel_fitting(intel_crtc, config,
> +				DRM_MODE_SCALE_FULLSCREEN);
> +
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 96c2cbd..fd2e081 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -110,7 +110,8 @@ 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)
> +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
> +	    !pipe_config->ycbcr420)
>  		goto done;
>  
>  	switch (fitting_mode) {
> -- 
> 2.7.4

-- 
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 11/14] drm/i915: prepare pipe for YCBCR420 output

[Ville Syrjälä]

On Thu, Jul 13, 2017 at 09:03:17PM +0530, Shashank Sharma wrote:
> To get HDMI YCBCR420 output, the PIPEMISC register should be
> programmed to:
> - Generate YCBCR output (bit 11)
> - In case of YCBCR420 outputs, it should be programmed in full
>   blend mode to use the scaler in 5x3 ratio (bits 26 and 27)
> 
> This patch:
> - Adds definition of these bits.
> - Programs PIPEMISC for YCBCR420 outputs.
> 
> V2: rebase
> V3: rebase
> V4: rebase
> V5: added r-b from Ander
> V6: Handle only YCBCR420 outputs (ville)
> V7: rebase
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> 
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>  drivers/gpu/drm/i915/intel_display.c | 7 +++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c712d01..e5020d6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5227,6 +5227,9 @@ enum {
>  
>  #define _PIPE_MISC_A			0x70030
>  #define _PIPE_MISC_B			0x71030
> +#define   PIPEMISC_YCBCR420_ENABLE	(1<<27)
> +#define   PIPEMISC_YCBCR420_MODE_BLEND	(1<<26)
> +#define   PIPEMISC_OUTPUT_YCBCR		(1<<11)

Please rename to match spec. So something like:
PIPEMISC_YUV420_ENABLE       (1<<27)
PIPEMISC_YUV420_MODE_FULL_BLEND      (1<<26)
PIPEMISC_OUTPUT_COLORSPACE_YUV       (1<<11)

>  #define   PIPEMISC_DITHER_BPC_MASK	(7<<5)
>  #define   PIPEMISC_DITHER_8_BPC		(0<<5)
>  #define   PIPEMISC_DITHER_10_BPC	(1<<5)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d78f1c2..1a23ec0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8076,6 +8076,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc_state *config = intel_crtc->config;
>  
>  	if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) {
>  		u32 val = 0;
> @@ -8101,6 +8102,12 @@ 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_YCBCR |
> +				PIPEMISC_YCBCR420_ENABLE |
> +				PIPEMISC_YCBCR420_MODE_BLEND;
> +		}

I think we'll want two flags. One to specify whether we're outputting
YCbCr and the other to indicate whether we need the 4:2:0 subsamling.
So maybe something like
bool ycbcr;
bool ycbcr420;

We also need state readout for this stuff. With those two flags I think
we can do something like:

if (IS_BDW || GEN >= 9) {
	tmp = READ(PIPEMISC);

	crtc_state->ycbcr = tmp & OUTPUT_YUV;
	
 	if (IS_GLK || GEN >= 10)
		crtc_state->ycbcr420 = tmp & YUV420_ENABLE;
}

The other missing readout thing is adjustment of the clock.
ddi_dotclock_get() will need to double the dotclock when we're
outputting ycbcr420.

Pls also add something along the lines of 
DRM_DEBUG_KMS("ycbcr: %i, ycbcr420: %i\n", ...);
to intel_dump_pipe_config() so that we can actually tell when we're
outputting YCbCr and 4:2:0.


> +
>  		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
>  	}
>  }
> -- 
> 2.7.4

-- 
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 12/14] drm/i915: prepare csc unit for YCBCR420 output

[Ville Syrjälä]

On Thu, Jul 13, 2017 at 09:03:18PM +0530, Shashank Sharma wrote:
> To support ycbcr output, we need a pipe CSC block to do
> RGB->YCBCR conversion.
> 
> Current Intel platforms have only one pipe CSC unit, so
> we can either do color correction using it, or we can perform
> RGB->YCBCR conversion.
> 
> This function adds a csc handler, which uses recommended bspec
> values to perform RGB->YCBCR conversion (target color space BT709)
> 
> V2: Rebase
> V3: Rebase
> V4: Rebase
> V5: Addressed review comments from Ander
>     - Remove extra line added in the patch
>     - Add the spec details in the commit message
>     - Combine two if(cond) while calling intel_crtc_compute_config
> V6: Handle YCBCR420 outputs only (Ville)
> V7: Addressed review comments from Ville:
>     - Add description about target colorspace
>     - Remove the comments from CSC function
>     - DRM_DEBUG->DEBUG_KMS for atomic failure due to CSC unit busy
>     - Remove unnecessary debug message about YCBCR420 possibe
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 46 +++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++
>  2 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index f85d575..8698691 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -41,6 +41,22 @@
>  
>  #define LEGACY_LUT_LENGTH		(sizeof(struct drm_color_lut) * 256)
>  
> +/* Post offset values for RGB->YCBCR conversion */
> +#define POSTOFF_RGB_TO_YUV_HI 0x800
> +#define POSTOFF_RGB_TO_YUV_ME 0x100
> +#define POSTOFF_RGB_TO_YUV_LO 0x800
> +
> +/*
> + * These values are direct register values specified in the Bspec,
> + * for RGB->YUV conversion matrix (colorspace BT709)
> + */
> +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8
> +#define CSC_RGB_TO_YUV_BU 0x37e80000
> +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0
> +#define CSC_RGB_TO_YUV_BY 0xb5280000
> +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
> +#define CSC_RGB_TO_YUV_BV 0x1e080000
> +
>  /*
>   * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>   * format). This macro takes the coefficient we want transformed and the
> @@ -91,6 +107,31 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
>  	}
>  }
>  
> +void i9xx_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc)
> +{
> +	int pipe = intel_crtc->pipe;
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +
> +
> +	I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
> +	I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
> +	I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0);
> +
> +	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), CSC_RGB_TO_YUV_RU_GU);
> +	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU);
> +
> +	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), CSC_RGB_TO_YUV_RY_GY);
> +	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY);
> +
> +	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), CSC_RGB_TO_YUV_RV_GV);
> +	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV);
> +
> +	I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI);
> +	I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME);
> +	I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO);
> +	I915_WRITE(PIPE_CSC_MODE(pipe), 0);
> +}
> +
>  /* Set up the pipe CSC unit. */
>  static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
>  {
> @@ -101,7 +142,10 @@ 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 (crtc_state->ctm) {
> +	if (intel_crtc_state->ycbcr420) {
> +		i9xx_load_ycbcr_conversion_matrix(intel_crtc);
> +		return;
> +	} else if (crtc_state->ctm) {
>  		struct drm_color_ctm *ctm =
>  			(struct drm_color_ctm *)crtc_state->ctm->data;
>  		uint64_t input[9] = { 0, };
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1a23ec0..9c6a1ed 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6283,6 +6283,21 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> +	/* YCBCR420 feasibility check */

Not a useful comment.

> +	if (pipe_config->ycbcr420) {
> +		struct drm_crtc_state *drm_state = &pipe_config->base;

Having aliasing pointer for the same thing is just annoying.
We do have quite a lot of that going around, but IMO we need
to clean that all up at some point.

So pls just do this instead:

if (pipe_config->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
> +		 * applied we can't support YCBCR420 output.
> +		 */
> +		if (drm_state->ctm) {
> +			DRM_DEBUG_KMS("YCBCR420 and CTM is not possible\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	/*
>  	 * Pipe horizontal size must be even in:
>  	 * - DVO ganged mode
> -- 
> 2.7.4

-- 
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 00/14] YCBCR 4:2:0 handling in DRM layer

[Ville Syrjälä]

On Thu, Jul 13, 2017 at 09:03:06PM +0530, Shashank Sharma wrote:
> Following YCBCR 4:4:4 and 4:2:2, YCBCR 4:2:0 is a new output format,
> which is currently supported on HDMI 2.0 sources/sinks. Due to lower
> chroma sub-sampling rate, YCBCR 4:2:0 can drive the video modes at half
> the pixel clock than YCBCR 4:4:4 or RGB 8:8:8 outputs. For example, a CEA
> 4K@60, RGB 8:8:8 mode needs a clock of appx 594Mhz, but it can be driven at
> 297Mhz using YCBCR 4:2:0 output.
> 
> Of course, the lower rate of chroma subsampling, causes the quality of YCBCR
> 4:2:0 to be lower than YCBCR 4:4:4 or RGB 8:8:8.
> 
> This patch series adds support for YCBCR 4:2:0 output in DRM layer.
> 
> - First 2 patches, complete the CEA mode-db in drm driver, by adding
>   new 4k modes. Current CEA mode-db contains 64 modes only (VIC 1-64),
>   whereas CEA-861-F defined VICs up to 107, including 4k modes, from VIC
>   range 93-107. First patch makes sure that inclusion of these modes doesn't
>   break existing HDMI 1.4 monitors, across various drivers.
> 
> - Next 5 patches focus on parsing new YCBCR 4:2:0 EDID blocks, and adding
>   YCBCR 4:2:0 modes in connector. They also contain a prune function, which
>   will cleanup the YCBCR 4:2:0 modes from list, if the connector doesn't
>   declare them supported.
> 
> - Next 2 patches add helper functions for identifing YCBCR 4:2:0 modes and
>   setup the color space in AVI infoframes.
> 
> - Next 6 patches add code for I915 layer handling of YCBCR 4:2:0 output.
> 
> This patch series was initially published as a complete framework to handle
> all YCBCR outputs (4:4:4, 4:2:2, 4:2:0), but based on the code reviews, now
> its been published as YCBCR 4:2:0 handling series only.
> 
> The previous discussion and reviews can be found here:
>     V5: https://patchwork.freedesktop.org/series/26815/
>     V1-V4: https://patchwork.freedesktop.org/series/22683/
> 
> Now re-publishing this patch series as YCBCR 4:2:0 handling series here.
> V2: Addressed review comments from Ville 
> This series dropped one patch in V2(patch 8 from V1), so 14 patches in V2
> 
> This series has been tested with drm-tip code with following setup:
> Source: Intel Geminilake device.
> Sink: ACER S277HK HDMI 2.0 monitor.
> Protocol testing: Astro VA-1844A HDMI analyzer.
> 
> Shashank Sharma (14):
>   drm: handle HDMI 2.0 VICs in AVI info-frames
>   drm/edid: complete CEA modedb(VIC 1-107)
>   drm/edid: parse sink information before CEA blocks
>   drm/edid: cleanup patch for CEA extended-tag macro
>   drm: add helper to validate YCBCR420 modes
>   drm/edid: parse YCBCR420 videomodes from EDID
>   drm/edid: parse ycbcr 420 deep color information
>   drm: add helper functions for YCBCR420 handling

I just finished pushing the core bits into drm-misc-next. But I'm not
super happy how that went because there were still quite a few trivial
checkpatch warnings and indentation issues I had to fix up. All of that
should have been dealt with before even submitting the patches to the
list. Review bandwidth is a scarce resource so we don't want to squander
it on this sort of stuff.

Another issue I ran into when I tried to actually run the code. As soon
as I loaded the driver a warning and the accompanying backtrace from
the state checker flew past my terminal. That too should have been
caught before even sending the patches to the list.

So I think the next time someone asks me to review something I will
start by asking these basic questions:
- Did you configure your editor so that it automagically formats code correctly?
- Did you check for new compiler/sparse/checkpatch warnings?
- Did you check for new runtime errors/warnings from the driver?

If the answer to any of those is "no", then I won't even bother
reviewing anything.

OK, that's enough ranting. Now for the more positive feedback.
The code does work for me, for the mose part. There is some kind of
initial problem though. When I first load the driver I get some
kind of weird shifted grey screen. So I guess either we're
misconfiguring something in the source, or we're not properly telling
the sink what we're sending it. After another modeset the problem
goes away and everything appears to work correctly. So congrats
on making that happen \o/

>   drm/i915: add config function for YCBCR420 outputs
>   drm/i915: prepare scaler for YCBCR420 modeset
>   drm/i915: prepare pipe for YCBCR420 output
>   drm/i915: prepare csc unit for YCBCR420 output
>   drm/i915: set colorspace for YCBCR420 outputs
>   drm/i915/glk: set HDMI 2.0 identifier

These remaining i915 patches still have a few issues, the lack of
state readout and the grey screen issue are the major ones. I've
left more detailed feedback on the individual patches, but right
now I don't have a clear idea what's causing the grey screen.

Assuming the remaining issues I've highlighted get fixed, and the
remaining patches won't get massively altered in the process,
you can slap
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
onto the patches.

> 
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   2 +-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c |   3 +-
>  drivers/gpu/drm/bridge/sii902x.c          |   2 +-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |   2 +-
>  drivers/gpu/drm/drm_edid.c                | 438 +++++++++++++++++++++++++++++-
>  drivers/gpu/drm/drm_modes.c               |  87 ++++++
>  drivers/gpu/drm/drm_probe_helper.c        |   4 +
>  drivers/gpu/drm/exynos/exynos_hdmi.c      |   2 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c         |   2 +-
>  drivers/gpu/drm/i915/i915_reg.h           |   3 +
>  drivers/gpu/drm/i915/intel_color.c        |  46 +++-
>  drivers/gpu/drm/i915/intel_display.c      |  26 ++
>  drivers/gpu/drm/i915/intel_drv.h          |   7 +-
>  drivers/gpu/drm/i915/intel_hdmi.c         |  69 ++++-
>  drivers/gpu/drm/i915/intel_panel.c        |   3 +-
>  drivers/gpu/drm/i915/intel_sdvo.c         |   3 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |   2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |   2 +-
>  drivers/gpu/drm/nouveau/nv50_display.c    |   3 +-
>  drivers/gpu/drm/omapdrm/omap_encoder.c    |   3 +-
>  drivers/gpu/drm/radeon/radeon_audio.c     |   2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c      |   2 +-
>  drivers/gpu/drm/sti/sti_hdmi.c            |   2 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |   2 +-
>  drivers/gpu/drm/tegra/hdmi.c              |   2 +-
>  drivers/gpu/drm/tegra/sor.c               |   2 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c            |   2 +-
>  drivers/gpu/drm/zte/zx_hdmi.c             |   2 +-
>  include/drm/drm_connector.h               |  32 +++
>  include/drm/drm_edid.h                    |  11 +-
>  include/drm/drm_modes.h                   |  11 +
>  34 files changed, 746 insertions(+), 39 deletions(-)
> 
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs

[Sharma, Shashank]

Regards

Shashank


On 7/15/2017 12:00 AM, Ville Syrjälä wrote:
> On Thu, Jul 13, 2017 at 09:03:15PM +0530, Shashank Sharma wrote:
>> This patch checks encoder level support for YCBCR420 outputs.
>> The logic goes as simple as this:
>> If the input mode is YCBCR420-only mode: prepare HDMI for
>> YCBCR420 output, else continue with RGB output mode.
>>
>> It checks if the mode is YCBCR420 and source can support this
>> output then it marks the ycbcr_420 output indicator into crtc
>> state, for further staging in driver.
>>
>> V2: Split the patch into two, kept helper functions in DRM layer.
>> V3: Changed the compute_config function based on new DRM API.
>> V4: Rebase
>> V5: Rebase
>> V6: Check and handle YCBCR420-only modes, discard the property
>>      based approach (Ville)
>> V7: Addressed review comments from Ville
>>      - add else case in 12BPC check.
>>      - extract ycbcr420 state inside hdmi_12bpc_possible function.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |  1 +
>>   drivers/gpu/drm/i915/intel_drv.h     |  3 +++
>>   drivers/gpu/drm/i915/intel_hdmi.c    | 43 +++++++++++++++++++++++++++++++++---
>>   3 files changed, 44 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 2144adc..a5140e4 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11945,6 +11945,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>>   	PIPE_CONF_CHECK_I(hdmi_scrambling);
>>   	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
>>   	PIPE_CONF_CHECK_I(has_infoframe);
>> +	PIPE_CONF_CHECK_I(ycbcr420);
>>   
>>   	PIPE_CONF_CHECK_I(has_audio);
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index d17a324..592243b 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -780,6 +780,9 @@ struct intel_crtc_state {
>>   
>>   	/* HDMI High TMDS char rate ratio */
>>   	bool hdmi_high_tmds_clock_ratio;
>> +
>> +	/* HDMI output type */
> We'll want DP too at some point. So that's going to get stale pretty
> soon.
Agree, will make it output format ycbcr420
>> +	bool ycbcr420;
>>   };
>>   
>>   struct intel_crtc {
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index cc0d100..eb6243c 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1312,6 +1312,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>>   	struct drm_atomic_state *state = crtc_state->base.state;
>>   	struct drm_connector_state *connector_state;
>>   	struct drm_connector *connector;
>> +	bool output_ycbcr420 = crtc_state->ycbcr420;
> Pointless variable. Just use 'crtc_state->ycbcr420' in the code
> directly.
Ah, yes it was used at more places before we removed some of the code, 
during review.
Sure, will use that directly.
>>   	int i;
>>   
>>   	if (HAS_GMCH_DISPLAY(dev_priv))
>> @@ -1330,8 +1331,16 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>>   		if (connector_state->crtc != crtc_state->base.crtc)
>>   			continue;
>>   
>> -		if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0)
>> -			return false;
>> +		if (output_ycbcr420) {
>> +			const struct drm_hdmi_info *hdmi = &info->hdmi;
>> +
>> +			if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
>> +				return false;
>> +		} else {
>> +
> Bogus whitespace.
Got it.
>> +			if (!(info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36))
>> +				return false;
>> +		}
>>   	}
>>   
>>   	/* Display Wa #1139 */
>> @@ -1342,6 +1351,25 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>>   	return true;
>>   }
>>   
>> +static bool
>> +intel_hdmi_ycbcr420_config(struct drm_connector *connector,
>> +			       struct intel_crtc_state *config,
>> +			       int *clock_12bpc, int *clock_8bpc)
>> +{
>> +
> Bogus whitespace.
Got it.

- Shashank
>> +	if (!connector->ycbcr_420_allowed) {
>> +		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
>> +		return false;
>> +	}
>> +
>> +	/* YCBCR420 TMDS rate requirement is half the pixel clock */
>> +	config->port_clock /= 2;
>> +	*clock_12bpc /= 2;
>> +	*clock_8bpc /= 2;
>> +	config->ycbcr420 = true;
>> +	return true;
>> +}
>> +
>>   bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   			       struct intel_crtc_state *pipe_config,
>>   			       struct drm_connector_state *conn_state)
>> @@ -1349,7 +1377,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>   	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>> -	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
>> +	struct drm_connector *connector = conn_state->connector;
>> +	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
>>   	struct intel_digital_connector_state *intel_conn_state =
>>   		to_intel_digital_connector_state(conn_state);
>>   	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>> @@ -1379,6 +1408,14 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   		clock_12bpc *= 2;
>>   	}
>>   
>> +	if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) {
>> +		if (!intel_hdmi_ycbcr420_config(connector, pipe_config,
>> +						&clock_12bpc, &clock_8bpc)) {
>> +			DRM_ERROR("Can't support YCBCR420 output\n");
>> +			return false;
>> +		}
>> +	}
>> +
>>   	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
>>   		pipe_config->has_pch_encoder = true;
>>   
>> -- 
>> 2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset

[Sharma, Shashank]

Regards

Shahank


On 7/15/2017 12:00 AM, Ville Syrjälä wrote:
> On Thu, Jul 13, 2017 at 09:03:16PM +0530, Shashank Sharma wrote:
>> To get a YCBCR420 output from intel platforms, we need one
>> scaler to scale down YCBCR444 samples to YCBCR420 samples.
>>
>> This patch:
>> - Does scaler allocation for HDMI ycbcr420 outputs.
>> - Programs PIPE_MISC register for ycbcr420 output.
>> - Adds a new scaler user "HDMI output" to plug-into existing
>>    scaler framework. This output type is identified using bit
>>    30 of the scaler users bitmap.
>>
>> V2: rebase
>> V3: rebase
>> V4: rebase
>> V5: addressed review comments from Ander:
>>      - No need to check both scaler_user && hdmi_output.
>>        Check for scaler_user is enough.
>> V6: rebase
>> V7: Do not create a new scaler user, use existing pipe scaler user.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Ander Conselvan De Oliveira <conselvan2@gmail.com>
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |  3 +++
>>   drivers/gpu/drm/i915/intel_drv.h     |  4 +++-
>>   drivers/gpu/drm/i915/intel_hdmi.c    | 12 ++++++++++++
>>   drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
>>   4 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index a5140e4..d78f1c2 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4624,6 +4624,9 @@ 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)
>> +		need_scaling = true;
>> +
>>   	/*
>>   	 * Scaling/fitting not supported in IF-ID mode in GEN9+
>>   	 * TODO: Interlace fetch mode doesn't support YUV420 planar formats.
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 592243b..94ea6ed 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -471,7 +471,8 @@ struct intel_crtc_scaler_state {
>>   	 *
>>   	 *     If a bit is set, a user is using a scaler.
>>   	 *     Here user can be a plane or crtc as defined below:
>> -	 *       bits 0-30 - plane (bit position is index from drm_plane_index)
>> +	 *       bits 0-29 - plane (bit position is index from drm_plane_index)
>> +	 *       bit 30    - hdmi output
> That's no longer true.
Agree !
>>   	 *       bit 31    - crtc
>>   	 *
>>   	 * Instead of creating a new index to cover planes and crtc, using
>> @@ -484,6 +485,7 @@ struct intel_crtc_scaler_state {
>>   	 * avilability.
>>   	 */
>>   #define SKL_CRTC_INDEX 31
>> +
> Bogus whitespace change.
Got it.
>>   	unsigned scaler_users;
>>   
>>   	/* scaler used by crtc for panel fitting purpose */
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index eb6243c..49f4fb8 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1356,6 +1356,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
>>   			       struct intel_crtc_state *config,
>>   			       int *clock_12bpc, int *clock_8bpc)
>>   {
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc);
>>   
>>   	if (!connector->ycbcr_420_allowed) {
>>   		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
>> @@ -1367,6 +1368,17 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
>>   	*clock_12bpc /= 2;
>>   	*clock_8bpc /= 2;
>>   	config->ycbcr420 = true;
>> +
>> +	/* YCBCR 420 output conversion needs a scaler */
>> +	if (skl_update_scaler_crtc(config)) {
>> +		DRM_ERROR("Scaler allocation for output failed\n");
> DRM_DEBUG_KMS
Ok,
>> +		return false;
>> +	}
>> +
>> +	/* Bind this scaler to pipe */
> Unnecesary comment.
Ok,

Shashank
>> +	intel_pch_panel_fitting(intel_crtc, config,
>> +				DRM_MODE_SCALE_FULLSCREEN);
>> +
>>   	return true;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> index 96c2cbd..fd2e081 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -110,7 +110,8 @@ 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)
>> +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
>> +	    !pipe_config->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

Re: [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output

[Sharma, Shashank]

Regards

Shashank


On 7/15/2017 12:03 AM, Ville Syrjälä wrote:
> On Thu, Jul 13, 2017 at 09:03:17PM +0530, Shashank Sharma wrote:
>> To get HDMI YCBCR420 output, the PIPEMISC register should be
>> programmed to:
>> - Generate YCBCR output (bit 11)
>> - In case of YCBCR420 outputs, it should be programmed in full
>>    blend mode to use the scaler in 5x3 ratio (bits 26 and 27)
>>
>> This patch:
>> - Adds definition of these bits.
>> - Programs PIPEMISC for YCBCR420 outputs.
>>
>> V2: rebase
>> V3: rebase
>> V4: rebase
>> V5: added r-b from Ander
>> V6: Handle only YCBCR420 outputs (ville)
>> V7: rebase
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>
>> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>>   drivers/gpu/drm/i915/intel_display.c | 7 +++++++
>>   2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index c712d01..e5020d6 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5227,6 +5227,9 @@ enum {
>>   
>>   #define _PIPE_MISC_A			0x70030
>>   #define _PIPE_MISC_B			0x71030
>> +#define   PIPEMISC_YCBCR420_ENABLE	(1<<27)
>> +#define   PIPEMISC_YCBCR420_MODE_BLEND	(1<<26)
>> +#define   PIPEMISC_OUTPUT_YCBCR		(1<<11)
> Please rename to match spec. So something like:
> PIPEMISC_YUV420_ENABLE       (1<<27)
> PIPEMISC_YUV420_MODE_FULL_BLEND      (1<<26)
> PIPEMISC_OUTPUT_COLORSPACE_YUV       (1<<11)
got it.
>>   #define   PIPEMISC_DITHER_BPC_MASK	(7<<5)
>>   #define   PIPEMISC_DITHER_8_BPC		(0<<5)
>>   #define   PIPEMISC_DITHER_10_BPC	(1<<5)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index d78f1c2..1a23ec0 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -8076,6 +8076,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> +	struct intel_crtc_state *config = intel_crtc->config;
>>   
>>   	if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) {
>>   		u32 val = 0;
>> @@ -8101,6 +8102,12 @@ 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_YCBCR |
>> +				PIPEMISC_YCBCR420_ENABLE |
>> +				PIPEMISC_YCBCR420_MODE_BLEND;
>> +		}
> I think we'll want two flags. One to specify whether we're outputting
> YCbCr and the other to indicate whether we need the 4:2:0 subsamling.
> So maybe something like
> bool ycbcr;
> bool ycbcr420;
This would have been true if we were support YCBCR444/422/420 all, but 
the recent patch series only
supports 420, so if its ycbcr its ycbcr420. We might be able to add 
preference of a 420_also mode in case
of RGB Vs YCBCR420, by adding another property.
> We also need state readout for this stuff. With those two flags I think
> we can do something like:
>
> if (IS_BDW || GEN >= 9) {
> 	tmp = READ(PIPEMISC);
>
> 	crtc_state->ycbcr = tmp & OUTPUT_YUV;
We dont support YCBCR (apart from 420) anymore, as there is no HDMI 
output property.
So this doesn't look required.
> 	
>   	if (IS_GLK || GEN >= 10)
> 		crtc_state->ycbcr420 = tmp & YUV420_ENABLE;
Yes, this is a great suggestion, and I believe I was missing this. Thanks !
> }
>
> The other missing readout thing is adjustment of the clock.
> ddi_dotclock_get() will need to double the dotclock when we're
> outputting ycbcr420.
>
> Pls also add something along the lines of
> DRM_DEBUG_KMS("ycbcr: %i, ycbcr420: %i\n", ...);
> to intel_dump_pipe_config() so that we can actually tell when we're
> outputting YCbCr and 4:2:0.
Thanks, this is also what I was looking for.
- Shashank
>
>> +
>>   		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
>>   	}
>>   }
>> -- 
>> 2.7.4

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

Re: [PATCH v2 12/14] drm/i915: prepare csc unit for YCBCR420 output

[Sharma, Shashank]

Regards

Shashank


On 7/15/2017 12:06 AM, Ville Syrjälä wrote:
> On Thu, Jul 13, 2017 at 09:03:18PM +0530, Shashank Sharma wrote:
>> To support ycbcr output, we need a pipe CSC block to do
>> RGB->YCBCR conversion.
>>
>> Current Intel platforms have only one pipe CSC unit, so
>> we can either do color correction using it, or we can perform
>> RGB->YCBCR conversion.
>>
>> This function adds a csc handler, which uses recommended bspec
>> values to perform RGB->YCBCR conversion (target color space BT709)
>>
>> V2: Rebase
>> V3: Rebase
>> V4: Rebase
>> V5: Addressed review comments from Ander
>>      - Remove extra line added in the patch
>>      - Add the spec details in the commit message
>>      - Combine two if(cond) while calling intel_crtc_compute_config
>> V6: Handle YCBCR420 outputs only (Ville)
>> V7: Addressed review comments from Ville:
>>      - Add description about target colorspace
>>      - Remove the comments from CSC function
>>      - DRM_DEBUG->DEBUG_KMS for atomic failure due to CSC unit busy
>>      - Remove unnecessary debug message about YCBCR420 possibe
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_color.c   | 46 +++++++++++++++++++++++++++++++++++-
>>   drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++
>>   2 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> index f85d575..8698691 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -41,6 +41,22 @@
>>   
>>   #define LEGACY_LUT_LENGTH		(sizeof(struct drm_color_lut) * 256)
>>   
>> +/* Post offset values for RGB->YCBCR conversion */
>> +#define POSTOFF_RGB_TO_YUV_HI 0x800
>> +#define POSTOFF_RGB_TO_YUV_ME 0x100
>> +#define POSTOFF_RGB_TO_YUV_LO 0x800
>> +
>> +/*
>> + * These values are direct register values specified in the Bspec,
>> + * for RGB->YUV conversion matrix (colorspace BT709)
>> + */
>> +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8
>> +#define CSC_RGB_TO_YUV_BU 0x37e80000
>> +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0
>> +#define CSC_RGB_TO_YUV_BY 0xb5280000
>> +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
>> +#define CSC_RGB_TO_YUV_BV 0x1e080000
>> +
>>   /*
>>    * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>>    * format). This macro takes the coefficient we want transformed and the
>> @@ -91,6 +107,31 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
>>   	}
>>   }
>>   
>> +void i9xx_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc)
>> +{
>> +	int pipe = intel_crtc->pipe;
>> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>> +
>> +
>> +	I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
>> +	I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
>> +	I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0);
>> +
>> +	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), CSC_RGB_TO_YUV_RU_GU);
>> +	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU);
>> +
>> +	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), CSC_RGB_TO_YUV_RY_GY);
>> +	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY);
>> +
>> +	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), CSC_RGB_TO_YUV_RV_GV);
>> +	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV);
>> +
>> +	I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI);
>> +	I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME);
>> +	I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO);
>> +	I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>> +}
>> +
>>   /* Set up the pipe CSC unit. */
>>   static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
>>   {
>> @@ -101,7 +142,10 @@ 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 (crtc_state->ctm) {
>> +	if (intel_crtc_state->ycbcr420) {
>> +		i9xx_load_ycbcr_conversion_matrix(intel_crtc);
>> +		return;
>> +	} else if (crtc_state->ctm) {
>>   		struct drm_color_ctm *ctm =
>>   			(struct drm_color_ctm *)crtc_state->ctm->data;
>>   		uint64_t input[9] = { 0, };
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 1a23ec0..9c6a1ed 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6283,6 +6283,21 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>>   		return -EINVAL;
>>   	}
>>   
>> +	/* YCBCR420 feasibility check */
> Not a useful comment.
Got it.
>> +	if (pipe_config->ycbcr420) {
>> +		struct drm_crtc_state *drm_state = &pipe_config->base;
> Having aliasing pointer for the same thing is just annoying.
> We do have quite a lot of that going around, but IMO we need
> to clean that all up at some point.
>
> So pls just do this instead:
>
> if (pipe_config->ycbcr420 && pipe_config->base.ctm) {
> 	...
> }
Most of the time its done to cover for 80 char limit, and to make code 
look good.
but let me see if I can accommodate this one.

- Shashank
>
>> +
>> +		/*
>> +		 * There is only one pipe CSC unit per pipe, and we need that
>> +		 * for output conversion from RGB->YCBCR. So if CTM is already
>> +		 * applied we can't support YCBCR420 output.
>> +		 */
>> +		if (drm_state->ctm) {
>> +			DRM_DEBUG_KMS("YCBCR420 and CTM is not possible\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>>   	/*
>>   	 * Pipe horizontal size must be even in:
>>   	 * - DVO ganged mode
>> -- 
>> 2.7.4

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

Re: [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer

[Sharma, Shashank]

[-- Attachment #1.1: Type: text/plain, Size: 13777 bytes --]

Regards

Shashank


On 7/15/2017 12:32 AM, Ville Syrjälä wrote:
> On Thu, Jul 13, 2017 at 09:03:06PM +0530, Shashank Sharma wrote:
>> Following YCBCR 4:4:4 and 4:2:2, YCBCR 4:2:0 is a new output format,
>> which is currently supported on HDMI 2.0 sources/sinks. Due to lower
>> chroma sub-sampling rate, YCBCR 4:2:0 can drive the video modes at half
>> the pixel clock than YCBCR 4:4:4 or RGB 8:8:8 outputs. For example, a CEA
>> 4K@60, RGB 8:8:8 mode needs a clock of appx 594Mhz, but it can be driven at
>> 297Mhz using YCBCR 4:2:0 output.
>>
>> Of course, the lower rate of chroma subsampling, causes the quality of YCBCR
>> 4:2:0 to be lower than YCBCR 4:4:4 or RGB 8:8:8.
>>
>> This patch series adds support for YCBCR 4:2:0 output in DRM layer.
>>
>> - First 2 patches, complete the CEA mode-db in drm driver, by adding
>>    new 4k modes. Current CEA mode-db contains 64 modes only (VIC 1-64),
>>    whereas CEA-861-F defined VICs up to 107, including 4k modes, from VIC
>>    range 93-107. First patch makes sure that inclusion of these modes doesn't
>>    break existing HDMI 1.4 monitors, across various drivers.
>>
>> - Next 5 patches focus on parsing new YCBCR 4:2:0 EDID blocks, and adding
>>    YCBCR 4:2:0 modes in connector. They also contain a prune function, which
>>    will cleanup the YCBCR 4:2:0 modes from list, if the connector doesn't
>>    declare them supported.
>>
>> - Next 2 patches add helper functions for identifing YCBCR 4:2:0 modes and
>>    setup the color space in AVI infoframes.
>>
>> - Next 6 patches add code for I915 layer handling of YCBCR 4:2:0 output.
>>
>> This patch series was initially published as a complete framework to handle
>> all YCBCR outputs (4:4:4, 4:2:2, 4:2:0), but based on the code reviews, now
>> its been published as YCBCR 4:2:0 handling series only.
>>
>> The previous discussion and reviews can be found here:
>>      V5: https://patchwork.freedesktop.org/series/26815/
>>      V1-V4: https://patchwork.freedesktop.org/series/22683/
>>
>> Now re-publishing this patch series as YCBCR 4:2:0 handling series here.
>> V2: Addressed review comments from Ville
>> This series dropped one patch in V2(patch 8 from V1), so 14 patches in V2
>>
>> This series has been tested with drm-tip code with following setup:
>> Source: Intel Geminilake device.
>> Sink: ACER S277HK HDMI 2.0 monitor.
>> Protocol testing: Astro VA-1844A HDMI analyzer.
>>
>> Shashank Sharma (14):
>>    drm: handle HDMI 2.0 VICs in AVI info-frames
>>    drm/edid: complete CEA modedb(VIC 1-107)
>>    drm/edid: parse sink information before CEA blocks
>>    drm/edid: cleanup patch for CEA extended-tag macro
>>    drm: add helper to validate YCBCR420 modes
>>    drm/edid: parse YCBCR420 videomodes from EDID
>>    drm/edid: parse ycbcr 420 deep color information
>>    drm: add helper functions for YCBCR420 handling
> I just finished pushing the core bits into drm-misc-next. But I'm not
> super happy how that went because there were still quite a few trivial
> checkpatch warnings and indentation issues I had to fix up. All of that
> should have been dealt with before even submitting the patches to the
> list. Review bandwidth is a scarce resource so we don't want to squander
> it on this sort of stuff.
The whole patch series was checked with checkpatch every time it was 
sent for review. It had
only 1 checkpatch warning, for which I had added an explanation in 
patch's header like:
For patch 1:

PS: This patch touches a few lines in few files, which were
already above 80 char, so checkpatch gives 80 char warning again.
- gpu/drm/omapdrm/omap_encoder.c
- gpu/drm/i915/intel_sdvo.c

Apart from this, here is the checkpatch output:

../scripts/checkpatch.pl *.patch
---------------------------------------------------------
v8-0001-drm-handle-HDMI-2.0-VICs-in-AVI-info-frames.patch
---------------------------------------------------------
WARNING: line over 80 characters
#278: FILE: drivers/gpu/drm/i915/intel_sdvo.c:999:
+ &pipe_config->base.adjusted_mode,

WARNING: line over 80 characters
#332: FILE: drivers/gpu/drm/omapdrm/omap_encoder.c:88:
+               r = drm_hdmi_avi_infoframe_from_display_mode(&avi, 
adjusted_mode,

total: 0 errors, 2 warnings, 247 lines checked

v8-0001-drm-handle-HDMI-2.0-VICs-in-AVI-info-frames.patch has style 
problems, please review.
----------------------------------------------------
v8-0002-drm-edid-complete-CEA-modedb-VIC-1-107.patch
----------------------------------------------------
total: 0 errors, 0 warnings, 245 lines checked

v8-0002-drm-edid-complete-CEA-modedb-VIC-1-107.patch has no obvious 
style problems and is ready for submission.
---------------------------------------------------------------
v8-0003-drm-edid-parse-sink-information-before-CEA-blocks.patch
---------------------------------------------------------------
total: 0 errors, 0 warnings, 21 lines checked

v8-0003-drm-edid-parse-sink-information-before-CEA-blocks.patch has no 
obvious style problems and is ready for submission.
---------------------------------------------------------------
v8-0004-drm-edid-cleanup-patch-for-CEA-extended-tag-macro.patch
---------------------------------------------------------------
total: 0 errors, 0 warnings, 33 lines checked

v8-0004-drm-edid-cleanup-patch-for-CEA-extended-tag-macro.patch has no 
obvious style problems and is ready for submission.
-------------------------------------------------------
v8-0005-drm-add-helper-to-validate-YCBCR420-modes.patch
-------------------------------------------------------
total: 0 errors, 0 warnings, 110 lines checked

v8-0005-drm-add-helper-to-validate-YCBCR420-modes.patch has no obvious 
style problems and is ready for submission.
----------------------------------------------------------
v8-0006-drm-edid-parse-YCBCR420-videomodes-from-EDID.patch
----------------------------------------------------------
total: 0 errors, 0 warnings, 228 lines checked

v8-0006-drm-edid-parse-YCBCR420-videomodes-from-EDID.patch has no 
obvious style problems and is ready for submission.
-------------------------------------------------------------
v8-0007-drm-edid-parse-ycbcr-420-deep-color-information.patch
-------------------------------------------------------------
total: 0 errors, 0 warnings, 47 lines checked

v8-0007-drm-edid-parse-ycbcr-420-deep-color-information.patch has no 
obvious style problems and is ready for submission.
------------------------------------------------------------
v8-0008-drm-add-helper-functions-for-YCBCR420-handling.patch
------------------------------------------------------------
total: 0 errors, 0 warnings, 73 lines checked

v8-0008-drm-add-helper-functions-for-YCBCR420-handling.patch has no 
obvious style problems and is ready for submission.
---------------------------------------------------------------
v8-0009-drm-i915-add-config-function-for-YCBCR420-outputs.patch
---------------------------------------------------------------
total: 0 errors, 0 warnings, 89 lines checked

v8-0009-drm-i915-add-config-function-for-YCBCR420-outputs.patch has no 
obvious style problems and is ready for submission.
----------------------------------------------------------
v8-0010-drm-i915-prepare-scaler-for-YCBCR420-modeset.patch
----------------------------------------------------------
total: 0 errors, 0 warnings, 58 lines checked

v8-0010-drm-i915-prepare-scaler-for-YCBCR420-modeset.patch has no 
obvious style problems and is ready for submission.
-------------------------------------------------------
v8-0011-drm-i915-prepare-pipe-for-YCBCR420-output.patch
-------------------------------------------------------
total: 0 errors, 0 warnings, 28 lines checked

v8-0011-drm-i915-prepare-pipe-for-YCBCR420-output.patch has no obvious 
style problems and is ready for submission.
-----------------------------------------------------------
v8-0012-drm-i915-prepare-csc-unit-for-YCBCR420-output.patch
-----------------------------------------------------------
total: 0 errors, 0 warnings, 85 lines checked

v8-0012-drm-i915-prepare-csc-unit-for-YCBCR420-output.patch has no 
obvious style problems and is ready for submission.
----------------------------------------------------------
v8-0013-drm-i915-set-colorspace-for-YCBCR420-outputs.patch
----------------------------------------------------------
total: 0 errors, 0 warnings, 18 lines checked

v8-0013-drm-i915-set-colorspace-for-YCBCR420-outputs.patch has no 
obvious style problems and is ready for submission.
--------------------------------------------------
v8-0014-drm-i915-glk-set-HDMI-2.0-identifier.patch
--------------------------------------------------
total: 0 errors, 0 warnings, 9 lines checked

v8-0014-drm-i915-glk-set-HDMI-2.0-identifier.patch has no obvious style 
problems and is ready for submission.

> Another issue I ran into when I tried to actually run the code. As soon
> as I loaded the driver a warning and the accompanying backtrace from
> the state checker flew past my terminal. That too should have been
> caught before even sending the patches to the list.
Yes, I had seen that, and I knew why is that happening, but I needed a 
discussion on what is the right way to fix it.
It was about readouts of the state and pixel clock mismatch (as YCBCR420 
clock = half). But at the same time, I am
at fault, I should have mentioned this somewhere in the cover letter or 
I should have mentioned it to you on IRC.
Sorry about that, My bad :( !
> So I think the next time someone asks me to review something I will
> start by asking these basic questions:
> - Did you configure your editor so that it automagically formats code correctly?
> - Did you check for new compiler/sparse/checkpatch warnings?
> - Did you check for new runtime errors/warnings from the driver?
>
> If the answer to any of those is "no", then I won't even bother
> reviewing anything.
>
> OK, that's enough ranting. Now for the more positive feedback.
> The code does work for me, for the mose part. There is some kind of
> initial problem though. When I first load the driver I get some
> kind of weird shifted grey screen. So I guess either we're
> misconfiguring something in the source, or we're not properly telling
> the sink what we're sending it. After another modeset the problem
> goes away and everything appears to work correctly. So congrats
> on making that happen \o/
Thanks, for the detailed review, and the time you invested on this.

As such I had tested this whole series with this combination:
- GLK system & KBL NUC system
- 2 different HDMI 2.0 monitors ACER/Samsung
- 1 HDMI 1.4 monitor (Dell)
- 1 HDMI analyzer (Astro VA1844)
- Mint desktop GUI
- IGT (testdisplay)
and I dint see any greying issue even once, but May be I will try to 
test with the way you are testing.
>
>>    drm/i915: add config function for YCBCR420 outputs
>>    drm/i915: prepare scaler for YCBCR420 modeset
>>    drm/i915: prepare pipe for YCBCR420 output
>>    drm/i915: prepare csc unit for YCBCR420 output
>>    drm/i915: set colorspace for YCBCR420 outputs
>>    drm/i915/glk: set HDMI 2.0 identifier
> These remaining i915 patches still have a few issues, the lack of
> state readout and the grey screen issue are the major ones. I've
> left more detailed feedback on the individual patches, but right
> now I don't have a clear idea what's causing the grey screen.
>
> Assuming the remaining issues I've highlighted get fixed, and the
> remaining patches won't get massively altered in the process,
> you can slap
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> onto the patches.
Thanks !
- Shashank
>
>>   drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   2 +-
>>   drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   2 +-
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   2 +-
>>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   2 +-
>>   drivers/gpu/drm/bridge/analogix-anx78xx.c |   3 +-
>>   drivers/gpu/drm/bridge/sii902x.c          |   2 +-
>>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |   2 +-
>>   drivers/gpu/drm/drm_edid.c                | 438 +++++++++++++++++++++++++++++-
>>   drivers/gpu/drm/drm_modes.c               |  87 ++++++
>>   drivers/gpu/drm/drm_probe_helper.c        |   4 +
>>   drivers/gpu/drm/exynos/exynos_hdmi.c      |   2 +-
>>   drivers/gpu/drm/i2c/tda998x_drv.c         |   2 +-
>>   drivers/gpu/drm/i915/i915_reg.h           |   3 +
>>   drivers/gpu/drm/i915/intel_color.c        |  46 +++-
>>   drivers/gpu/drm/i915/intel_display.c      |  26 ++
>>   drivers/gpu/drm/i915/intel_drv.h          |   7 +-
>>   drivers/gpu/drm/i915/intel_hdmi.c         |  69 ++++-
>>   drivers/gpu/drm/i915/intel_panel.c        |   3 +-
>>   drivers/gpu/drm/i915/intel_sdvo.c         |   3 +-
>>   drivers/gpu/drm/mediatek/mtk_hdmi.c       |   2 +-
>>   drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |   2 +-
>>   drivers/gpu/drm/nouveau/nv50_display.c    |   3 +-
>>   drivers/gpu/drm/omapdrm/omap_encoder.c    |   3 +-
>>   drivers/gpu/drm/radeon/radeon_audio.c     |   2 +-
>>   drivers/gpu/drm/rockchip/inno_hdmi.c      |   2 +-
>>   drivers/gpu/drm/sti/sti_hdmi.c            |   2 +-
>>   drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |   2 +-
>>   drivers/gpu/drm/tegra/hdmi.c              |   2 +-
>>   drivers/gpu/drm/tegra/sor.c               |   2 +-
>>   drivers/gpu/drm/vc4/vc4_hdmi.c            |   2 +-
>>   drivers/gpu/drm/zte/zx_hdmi.c             |   2 +-
>>   include/drm/drm_connector.h               |  32 +++
>>   include/drm/drm_edid.h                    |  11 +-
>>   include/drm/drm_modes.h                   |  11 +
>>   34 files changed, 746 insertions(+), 39 deletions(-)
>>
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel


[-- Attachment #1.2: Type: text/html, Size: 16735 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel