[PATCH v5 00/17] HDMI YCBCR output handling in DRM layer

[PATCH v5 00/17] HDMI YCBCR output handling in DRM layer

[Shashank Sharma]

This patch series adds support for YCBCR HDMI output handling in DRM layer.
The main aim of this patch series was to handle YCBCR 4:2:0 output for
HDMI 2.0 displays. But while providing a framework to handle non-RGB
outputs, support for YCBCR 4:4:4 and 4:2:2 was also added.

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 3 patches focus on parsing new YCBCR 4:2:0 EDID blocks, and adding
YCBCR 4:2:0 modes in connector (only for connectors who declare themselves
as HDMI 2.0 compliant during init)

Next 6 patch create a property (hdmi_output_format) and add the framework
to handle the HDMI output type defined in the property. There are also some
helper functions provided, to help with the YCBCR HDMI output, like adding
deep color information, set color space and get the appropriate YCBCR output
based on source + sink capabilities.

A User can set the HDMI output property, and select the desired output
from YCBCR 4:4:4, 4:2:2 or 4:2:0. A driver can use framework's helper
functions to check if this source + sink + mode combination can drive
the desired output, or what would be the best suitable output.

By default the value of the property is default RGB, so if you don't
set the property, there is no change in the regular functionality of
an existing source.

More details are available with respective patches.

Last 6 patches add the I915 core implementation of the HDMI output handling.
This patch series was tested with a GLK device, ACER S277HK monitor and
ASTRO VA-1844A HDMI analyzer.

V3: Added a new patch to introduce YCBCR_420_allowed identifier.
V4: Added a new patch to re-sequence sink info parsing befor CEA modes.
V5: Added two new patches and some documentation as per review comments.

Shashank Sharma (17):
  drm: add HDMI 2.0 VIC support for AVI info-frames
  drm: add YCBCR 420 capability identifier
  drm/edid: Complete CEA modedb(VIC 1-107)
  drm: add helper to validate ycbcr420 modes
  drm/edid: parse sink information before CEA blocks
  drm/edid: rename macro for CEA extended-tag
  drm/edid: parse YCBCR 420 videomodes from EDID
  drm/edid: parse ycbcr 420 deep color information
  drm: create hdmi output property
  drm: set output colorspace in AVI infoframe
  drm: add helper functions for YCBCR output handling
  drm/i915: add compute-config for YCBCR outputs
  drm/i915: prepare scaler for YCBCR420 modeset
  drm/i915: prepare pipe for YCBCR output
  drm/i915: prepare csc unit for YCBCR HDMI output
  drm/i915: set colorspace for ycbcr outputs
  drm/i915/glk: set HDMI 2.0 identifier

 Documentation/gpu/drm-kms.rst             |  12 +
 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_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_atomic.c              |   4 +
 drivers/gpu/drm/drm_atomic_helper.c       |   4 +
 drivers/gpu/drm/drm_connector.c           |  69 ++++-
 drivers/gpu/drm/drm_crtc_internal.h       |   1 +
 drivers/gpu/drm/drm_edid.c                | 480 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_mode_config.c         |   4 +
 drivers/gpu/drm/drm_modes.c               | 335 +++++++++++++++++++++
 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_atomic.c       |   6 +
 drivers/gpu/drm/i915/intel_color.c        |  47 ++-
 drivers/gpu/drm/i915/intel_display.c      |  65 ++++
 drivers/gpu/drm/i915/intel_drv.h          |  13 +-
 drivers/gpu/drm/i915/intel_hdmi.c         | 119 +++++++-
 drivers/gpu/drm/i915/intel_modes.c        |  13 +
 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/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/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               |  50 ++++
 include/drm/drm_edid.h                    |  17 +-
 include/drm/drm_mode_config.h             |   5 +
 include/drm/drm_modes.h                   |  10 +
 39 files changed, 1266 insertions(+), 37 deletions(-)

-- 
2.7.4

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

[PATCH v5 01/17] drm: add HDMI 2.0 VIC support for 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

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_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/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/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 ++-
 21 files changed, 52 insertions(+), 21 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_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 9006578..a083211 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 9b87067..3dc40f6 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 ead1124..0b0c9de 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/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/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 v5 02/17] drm: add YCBCR 420 capability identifier

[Shashank Sharma]

This patch adds 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 YCBCR 420 outputs or not, so that
a lot of work can be done/bypassed based on this information. One such
example is adding YCBCR420 only modes.

If the driver knows that this source is not HDMI 2.0 capable, it will
not add YCBCR420-only modes while adding EDID modes, and will prevent
any runtime modeset failures.

This variable will be initialized from I915 driver in the next patch
and will be used in the EDID handling for HDMI 2.0 specific features,
in this same series.

V3: introduced the new variable
V4: changed variable name from is_hdmi2_src to ycbcr_420_allowed (Ville)
V4: adding r-b from Neil.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 include/drm/drm_connector.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ae5b7dc..8a78aaa 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -726,6 +726,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.
-- 
2.7.4

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

[PATCH v5 03/17] 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)

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..b879662 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 v5 04/17] 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.

V5: Introduced the patch in series.

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b879662..ad26c5e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2947,10 +2947,11 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
 }
 EXPORT_SYMBOL(drm_match_cea_mode);
 
-static bool drm_valid_cea_vic(u8 vic)
+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..3b53c8e3 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_ycbcr420_only - 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 (drm_valid_cea_vic(vic) && 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[] = {
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_edid.h b/include/drm/drm_edid.h
index 89c0062..b55b2a7 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -477,4 +477,5 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name,
 struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
 					   int hsize, int vsize, int fresh,
 					   bool rb);
+bool drm_valid_cea_vic(u8 vic);
 #endif /* __DRM_EDID_H__ */
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 v5 05/17] 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

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 ad26c5e..e574897 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4441,6 +4441,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
@@ -4467,8 +4474,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 v5 06/17] drm/edid: rename macro for CEA extended-tag

[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

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 e574897..d0b27b5 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)
@@ -3444,6 +3445,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;
@@ -4019,8 +4026,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

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

[PATCH v5 07/17] drm/edid: parse YCBCR 420 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.

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index d0b27b5..44be128 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 8a78aaa..225e092 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -135,6 +135,25 @@ 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)];
+
+	/**
+	 * @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;
 };
 
 /**
@@ -198,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 v5 08/17] 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)

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 44be128..944a28f 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 b55b2a7..aa58146 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 v5 09/17] drm: create hdmi output property

[Shashank Sharma]

HDMI displays can support various output types, based on
the color space and subsampling type. The possible
outputs from a HDMI 2.0 monitor could be:
 - RGB
 - YCBCR 444
 - YCBCR 422
 - YCBCR 420

This patch adds a drm property "hdmi_output_format", using which,
a user can specify its preference, for the HDMI output type. The
output type enums are similar to the mentioned outputs above. To
handle various subsampling of YCBCR output types, this property
allows two special cases:
 - DRM_HDMI_OUTPUT_YCBCR_HQ
   This indicates preferred output should be YCBCR output, with highest
   subsampling rate by the source/sink, which can be typically:
 	- ycbcr444
 	- ycbcr422
 	- ycbcr420
 - DRM_HDMI_OUTPUT_YCBCR_LQ
   This indicates preferred output should be YCBCR output, with lowest
   subsampling rate supported by source/sink, which can be:
 	- ycbcr420
 	- ycbcr422
 	- ycbcr444

Default value of the property is set to 0 = RGB, so no changes if you
dont set the property.

PS: While doing modeset for YCBCR 420 only modes, this property is
    ignored, as those timings can be supported only in YCBCR 420
    output mode.

V2: Added description for the new variable to address build warning
V3: Rebase
V4: Rebase
V5: Added get_property counterpart to fix IGT BAT failures (BAT/CI)
    Danvet:
    - Add documentation for the new property
    - Create a sub-section for HDMI properties, and add documentation for
      few more HDMI propeties. Added documentation for:
	- Broadcast RGB
	- aspect ratio

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 Documentation/gpu/drm-kms.rst       | 12 +++++++
 drivers/gpu/drm/drm_atomic.c        |  4 +++
 drivers/gpu/drm/drm_atomic_helper.c |  4 +++
 drivers/gpu/drm/drm_connector.c     | 69 ++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_crtc_internal.h |  1 +
 drivers/gpu/drm/drm_mode_config.c   |  4 +++
 drivers/gpu/drm/i915/intel_modes.c  | 13 +++++++
 include/drm/drm_connector.h         | 18 ++++++++++
 include/drm/drm_mode_config.h       |  5 +++
 9 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 3072841..dcdd6ff 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -508,6 +508,18 @@ Standard Connector Properties
 .. kernel-doc:: drivers/gpu/drm/drm_connector.c
    :doc: standard connector properties
 
+Standard HDMI Properties
+-----------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: hdmi_output_format
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: aspect ratio property
+
+.. kernel-doc:: drivers/gpu/drm/i915/intel_modes.c
+   :doc: Broadcast RGB property
+
 Plane Composition Properties
 ----------------------------
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 09ca662..adcb89d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1192,6 +1192,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
 		state->picture_aspect_ratio = val;
 	} else if (property == connector->scaling_mode_property) {
 		state->scaling_mode = val;
+	} else if (property == config->hdmi_output_property) {
+		state->hdmi_output = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -1272,6 +1274,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->picture_aspect_ratio;
 	} else if (property == connector->scaling_mode_property) {
 		*val = state->scaling_mode;
+	} else if (property == config->hdmi_output_property) {
+		*val = state->hdmi_output;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 23e4661..2e7459f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -637,6 +637,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 			if (old_connector_state->link_status !=
 			    new_connector_state->link_status)
 				new_crtc_state->connectors_changed = true;
+
+			if (old_connector_state->hdmi_output !=
+			    new_connector_state->hdmi_output)
+				new_crtc_state->connectors_changed = true;
 		}
 
 		if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 8072e6e..8357918 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev,
 					      config->edid_property,
 					      0);
 
+	if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)
+		drm_object_attach_property(&connector->base,
+					   config->hdmi_output_property,
+					   0);
+
 	drm_object_attach_property(&connector->base,
 				      config->dpms_property, 0);
 
@@ -617,6 +622,26 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
 };
 DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
 
+static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {
+	{ DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
+	{ DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" },
+	{ DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
+	{ DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" },
+	{ DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" },
+	{ DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" },
+	{ DRM_HDMI_OUTPUT_INVALID, "invalid_output" },
+};
+
+/**
+ * drm_get_hdmi_output_name - return a string for a given hdmi output enum
+ * @type: enum of output type
+ */
+const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type)
+{
+	return drm_hdmi_output_enum_list[type].name;
+}
+EXPORT_SYMBOL(drm_get_hdmi_output_name);
+
 /**
  * drm_display_info_set_bus_formats - set the supported bus formats
  * @info: display info to store bus formats in
@@ -697,7 +722,36 @@ static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = {
 	{ DRM_MODE_SUBCONNECTOR_SCART,     "SCART"     }, /* TV-out */
 };
 DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
-		 drm_tv_subconnector_enum_list)
+		 drm_tv_subconnector_enum_list);
+
+/**
+ * DOC: hdmi_output_format
+ *
+ * hdmi_output_format:
+ *	Enum property which allows a userspace to provide its preference for a
+ *	HDMI output format. Standard HDMI 1.4b outputs can support RGB/YCBCR444
+ *	YCBCR422 output formats, whereas HDMI 2.0 outputs can additionally
+ *	support YCBCR420 output. Default value of the property is HDMI output
+ *	RGB.
+ *
+ *	A driver can attach to this property, and then handle the HDMI output
+ *	based on its capabilities and sink support. There are few helper
+ *	functions available in drm_modes.c which can help in finding the best
+ *	suitable output considering a sink/source/mode combination. Refer to
+ *	drm_modes.c:drm_display_info_hdmi_output_type()
+ */
+int drm_connector_create_hdmi_properties(struct drm_device *dev)
+{
+	struct drm_property *prop;
+
+	prop = drm_property_create_enum(dev, 0, "hdmi_output_format",
+					drm_hdmi_output_enum_list,
+					ARRAY_SIZE(drm_hdmi_output_enum_list));
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.hdmi_output_property = prop;
+	return 0;
+}
 
 /**
  * DOC: standard connector properties
@@ -1024,6 +1078,19 @@ int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property);
 
+
+/**
+ * DOC: aspect ratio property
+ *
+ * aspect ratio:
+ *	Enum property to override the HDMI output's aspect ratio.
+ *	When this property is set, the aspect ratio of the frame in
+ *	AVI infoframes is set as per the property value. For example
+ *	if userspace sets the property value to DRM_MODE_PICTURE_ASPECT_4_3
+ *	the output aspect ratio is set to 4:3 (regardless of the PAR of mode)
+ *
+ */
+
 /**
  * drm_mode_create_aspect_ratio_property - create aspect ratio property
  * @dev: DRM device
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index d077c54..e6c4adc 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -140,6 +140,7 @@ int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
 				    struct drm_property *property,
 				    uint64_t value);
 int drm_connector_create_standard_properties(struct drm_device *dev);
+int drm_connector_create_hdmi_properties(struct drm_device *dev);
 const char *drm_get_connector_force_name(enum drm_connector_force force);
 
 /* IOCTL */
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index d986225..3ba9262 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -210,6 +210,10 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 	if (ret)
 		return ret;
 
+	ret = drm_connector_create_hdmi_properties(dev);
+	if (ret)
+		return ret;
+
 	prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
 					"type", drm_plane_type_enum_list,
 					ARRAY_SIZE(drm_plane_type_enum_list));
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 951e834..d07de45 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -104,6 +104,19 @@ static const struct drm_prop_enum_list broadcast_rgb_names[] = {
 	{ INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
 };
 
+/**
+ * DOC: Broadcast RGB property
+ *
+ * Broadcast RGB:
+ *	Enum property to indicate RGB color range of a HDMI output.
+ *	For limited range RGB outputs, a remapping of pixel values from 0-255
+ *	should be remaped to a range of 16-235. When this property is set to
+ *	Limited 16:235 and CTM is set, the hardware will be programmed with the
+ *	result of the multiplication of CTM by the limited  range matrix, to
+ *	ensure the pixels normaly in the range 0..1.0 are remapped to the range
+ *	16/255..235/255.
+ *
+ */
 void
 intel_attach_broadcast_rgb_property(struct drm_connector *connector)
 {
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4bc0882..3773600 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -319,6 +319,17 @@ struct drm_tv_connector_state {
 	unsigned int hue;
 };
 
+/* HDMI output pixel format */
+enum drm_hdmi_output_type {
+	DRM_HDMI_OUTPUT_DEFAULT_RGB, /* default RGB */
+	DRM_HDMI_OUTPUT_YCBCR444, /* YCBCR 444 */
+	DRM_HDMI_OUTPUT_YCBCR422, /* YCBCR 422 */
+	DRM_HDMI_OUTPUT_YCBCR420, /* YCBCR 420 */
+	DRM_HDMI_OUTPUT_YCBCR_HQ, /* Highest subsampled YUV */
+	DRM_HDMI_OUTPUT_YCBCR_LQ, /* Lowest subsampled YUV */
+	DRM_HDMI_OUTPUT_INVALID, /* Guess what ? */
+};
+
 /**
  * struct drm_connector_state - mutable connector state
  * @connector: backpointer to the connector
@@ -363,6 +374,12 @@ struct drm_connector_state {
 	 * upscaling, mostly used for built-in panels.
 	 */
 	unsigned int scaling_mode;
+
+	/**
+	 * @hdmi_output: Connector property to control the
+	 * HDMI output mode (RGB/YCBCR444/422/420).
+	 */
+	enum drm_hdmi_output_type hdmi_output;
 };
 
 /**
@@ -991,6 +1008,7 @@ static inline void drm_connector_unreference(struct drm_connector *connector)
 
 const char *drm_get_connector_status_name(enum drm_connector_status status);
 const char *drm_get_subpixel_order_name(enum subpixel_order order);
+const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type);
 const char *drm_get_dpms_name(int val);
 const char *drm_get_dvi_i_subconnector_name(int val);
 const char *drm_get_dvi_i_select_name(int val);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 4298171..1887261 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -740,6 +740,11 @@ struct drm_mode_config {
 	 * the position of the output on the host's screen.
 	 */
 	struct drm_property *suggested_y_property;
+	/**
+	 * @hdmi_output_property: output pixel format from HDMI display
+	 * Default is set for RGB
+	 */
+	struct drm_property *hdmi_output_property;
 
 	/* dumb ioctl parameters */
 	uint32_t preferred_depth, prefer_shadow;
-- 
2.7.4

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

[PATCH v5 10/17] drm: set output colorspace in AVI infoframe

[Shashank Sharma]

HDMI source must set output colorspace information in AVI
infoframes, so that the sink can decode upcoming frames
accordingly.

As this patch series is adding support for HDMI output modes
other than RGB, this patch adds a function in DRM layer, to
add the output colorspace information in the AVI infoframes.

V2: Rebase
V3: Rebase
V4: Rebase
V5: Rebase

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 40 ++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     |  5 +++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 944a28f..9d030bb 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4796,6 +4796,46 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
 
 /**
+ * drm_hdmi_avi_infoframe_set_colorspace - fill an HDMI AVI infoframe with
+ * colorspace data of the output type
+ *
+ * @frame: HDMI AVI infoframe
+ * @mode: DRM display mode
+ * @hdmi_output: HDMI output colorspace
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame,
+					 const struct drm_display_mode *mode,
+					 enum drm_hdmi_output_type hdmi_output)
+{
+	switch (hdmi_output) {
+	case DRM_HDMI_OUTPUT_YCBCR444:
+		frame->colorspace = HDMI_COLORSPACE_YUV444;
+		break;
+	case DRM_HDMI_OUTPUT_YCBCR422:
+		frame->colorspace = HDMI_COLORSPACE_YUV422;
+		break;
+	case DRM_HDMI_OUTPUT_YCBCR420:
+		frame->colorspace = HDMI_COLORSPACE_YUV420;
+		frame->pixel_repeat = 0;
+		break;
+	case DRM_HDMI_OUTPUT_DEFAULT_RGB:
+		frame->colorspace = HDMI_COLORSPACE_RGB;
+		break;
+	case DRM_HDMI_OUTPUT_YCBCR_HQ:
+	case DRM_HDMI_OUTPUT_YCBCR_LQ:
+	case DRM_HDMI_OUTPUT_INVALID:
+		DRM_ERROR("Invalid HDMI output type\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace);
+
+/**
  * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
  *                                        quantization range information
  * @frame: HDMI AVI infoframe
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index aa58146..286cff0 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -332,6 +332,7 @@ struct cea_sad {
 struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
+enum drm_hdmi_output_type;
 
 void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
 int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
@@ -354,6 +355,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 					 const struct drm_display_mode *mode,
 					 bool is_hdmi2_sink);
 int
+drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame,
+					 const struct drm_display_mode *mode,
+					 enum drm_hdmi_output_type hdmi_output);
+int
 drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
 					    const struct drm_display_mode *mode);
 void
-- 
2.7.4

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

[PATCH v5 11/17] drm: add helper functions for YCBCR output handling

[Shashank Sharma]

This patch adds set of helper functions for YCBCR HDMI output
handling. These functions provide functionality like:
- check if a given video mode is YCBCR 420 only mode.
- check if a given video mode is YCBCR 420 mode.
- get the highest subsamled ycbcr output.
- get the lowest subsamled ycbcr output.
- check if a given source and sink combination can support any
  YCBCR HDMI output.
- check if a given source and sink combination can support a particular
  YCBCR HDMI output.

Currently all these helpers are kept static, and only one wrapper function
is exposed to callers, which is "drm_find_hdmi_output_type". This function
takes inputs as current mode, user preference and src + sink capabilities,
and provides the most suitable HDMI output format to match all of these
inputs.

This wrapper function will be used in next few patches in this
series from I915 driver.

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.

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 | 307 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_modes.h     |   5 +
 2 files changed, 312 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 3b53c8e3..0c42bbb 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1604,3 +1604,310 @@ 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 support YCBCR420 output
+ * false if not.
+ */
+static bool drm_mode_is_420_only(struct drm_display_info *display,
+			struct drm_display_mode *mode)
+{
+	u8 vic = drm_match_cea_mode(mode);
+
+	/*
+	 * Requirements of a 420_only mode:
+	 * must be a valid cea mode
+	 * entry in 420_only bitmap
+	 */
+	if (!drm_valid_cea_vic(vic))
+		return false;
+
+	return test_bit(vic, display->hdmi.y420_vdb_modes);
+}
+
+/**
+ * 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 support YCBCR420 output
+ * false if not.
+ */
+static bool drm_mode_is_420_also(struct drm_display_info *display,
+			struct drm_display_mode *mode)
+{
+	u8 vic = drm_match_cea_mode(mode);
+
+	/*
+	 * Requirements of a 420_also mode:
+	 * must be a valid cea mode
+	 * entry in 420_also bitmap
+	 */
+	if (!drm_valid_cea_vic(vic))
+		return false;
+
+	return test_bit(vic, display->hdmi.y420_cmdb_modes);
+}
+
+
+static bool drm_mode_is_420(struct drm_display_info *display,
+			struct drm_display_mode *mode)
+{
+	return drm_mode_is_420_only(display, mode) ||
+			drm_mode_is_420_also(display, mode);
+}
+
+/**
+ * drm_display_supports_this_ycbcr_output - can a given source and sink
+ * combination support a particular YCBCR output type.
+ *
+ * @display: sink information.
+ * @mode: video mode from modeset
+ * @type: enum indicating YCBCR output type
+ * @source_outputs: bitmap of source supported HDMI output formats.
+ *
+ * Returns:
+ * true on success.
+ * false on failure.
+ */
+static bool
+drm_display_supports_this_ycbcr_output(struct drm_display_info *display,
+					struct drm_display_mode *mode,
+					enum drm_hdmi_output_type type,
+					u32 source_outputs)
+{
+	/* YCBCR420 output support can be per mode basis */
+	if (type == DRM_HDMI_OUTPUT_YCBCR420 &&
+		!drm_mode_is_420(display, mode))
+		return false;
+
+	return display->color_formats & source_outputs & type;
+}
+
+/**
+ * drm_display_supports_ycbcr_output - can a given source and sink combination
+ * support any YCBCR outputs ?
+ *
+ * @display: sink information.
+ * @source_outputs: bitmap of source supported HDMI output formats.
+ *
+ * Returns:
+ * true on success.
+ * false on failure.
+ */
+static bool drm_display_supports_ycbcr_output(struct drm_display_info *display,
+					u32 source_outputs)
+{
+	u32 supported_formats;
+
+	if (!source_outputs || !display->color_formats) {
+		DRM_DEBUG_KMS("Source/Sink doesn't support any output ?\n");
+		return DRM_HDMI_OUTPUT_INVALID;
+	}
+
+	/* Get the common supported fromats between source and sink */
+	supported_formats = display->color_formats & source_outputs;
+	if (!supported_formats || (supported_formats ==
+		DRM_COLOR_FORMAT_RGB444)) {
+		DRM_ERROR("No common YCBCR formats between source and sink\n");
+		return false;
+	}
+
+	DRM_DEBUG_KMS("Src and Sink combination can support YCBCR output\n");
+	return true;
+}
+
+/**
+ * drm_get_highest_quality_ycbcr_supported - get the ycbcr output mode
+ * with highest subsampling rate
+ * @display: struct drm_display_info from current connector
+ * @mode: video mode from modeset
+ * @source_output_map: bitmap of supported HDMI output modes from source
+ *
+ * Finds the best possible ycbcr output mode (based on subsampling), for the
+ * given source and sink combination.
+ *
+ * Returns:
+ * enum corresponding to best output mode on success.
+ * DRM_HDMI_OUTPUT_INVALID on failure.
+ */
+static enum drm_hdmi_output_type
+drm_get_highest_quality_ycbcr_supported(struct drm_display_info *display,
+					struct drm_display_mode *mode,
+					u32 source_output_map)
+{
+	enum drm_hdmi_output_type output = DRM_HDMI_OUTPUT_INVALID;
+	u32 supported_formats = source_output_map & display->color_formats;
+
+	/*
+	 * Get the ycbcr output with the highest possible subsampling rate.
+	 * Preference should go as:
+	 * ycbcr 444
+	 * ycbcr 422
+	 * ycbcr 420
+	 */
+	if (supported_formats & DRM_COLOR_FORMAT_YCRCB444)
+		output = DRM_COLOR_FORMAT_YCRCB444;
+	else if (supported_formats & DRM_COLOR_FORMAT_YCRCB422)
+		output = DRM_COLOR_FORMAT_YCRCB422;
+	else if (supported_formats & DRM_COLOR_FORMAT_YCRCB420 &&
+			drm_mode_is_420(display, mode))
+		output = DRM_COLOR_FORMAT_YCRCB420;
+
+	DRM_DEBUG_KMS("Highest subsampled YCBCR mode supported is %s\n",
+			drm_get_hdmi_output_name(supported_formats));
+	return output;
+}
+
+/**
+ * drm_get_lowest_quality_ycbcr_supported - get the ycbcr output mode
+ * with lowest subsampling rate
+ * @display: struct drm_display_info from current connector
+ * @mode: video mode from modeset
+ * @source_output_map: bitmap of supported HDMI output modes from source
+ *
+ * Finds the lowest possible ycbcr output mode (based on subsampling), for the
+ * given source and sink combination.
+ *
+ * Returns:
+ * enum corresponding to best output mode on success.
+ * DRM_HDMI_OUTPUT_INVALID on failure.
+ */
+static enum drm_hdmi_output_type
+drm_get_lowest_quality_ycbcr_supported(struct drm_display_info *display,
+					struct drm_display_mode *mode,
+					u32 source_output_map)
+{
+	enum drm_hdmi_output_type output = DRM_HDMI_OUTPUT_INVALID;
+	u32 supported_formats = source_output_map & display->color_formats;
+
+	/*
+	 * Get the ycbcr output with the lowest possible subsampling rate.
+	 * Preference should go as:
+	 * ycbcr 420
+	 * ycbcr 422
+	 * ycbcr 444
+	 */
+	if (supported_formats & DRM_COLOR_FORMAT_YCRCB420 &&
+		drm_mode_is_420(display, mode))
+		output = DRM_HDMI_OUTPUT_YCBCR420;
+	else if (display->color_formats & DRM_COLOR_FORMAT_YCRCB422)
+		output = DRM_HDMI_OUTPUT_YCBCR422;
+	else if (display->color_formats & DRM_COLOR_FORMAT_YCRCB444)
+		output = DRM_HDMI_OUTPUT_YCBCR420;
+
+	DRM_DEBUG_KMS("Lowest subsampled YCBCR mode supported is %s\n",
+			drm_get_hdmi_output_name(supported_formats));
+	return output;
+}
+
+/**
+ * drm_find_hdmi_output_type - get the most suitable output
+ * @info: connected display's information from connector
+ * @mode: video mode under modeset
+ * @type: user's choice for preferred mode, set via drm property
+ *	"hdmi_output_format"
+ * @src_output_cap: bitmap of source's supported outputs formats
+ *	src_output_cap = (1 << DRM_COLOR_FORMAT_RGB444) means source
+ *	supports RGB444
+ *	src_output_cap = (1 << DRM_COLOR_FORMAT_YCRCB444) means source
+ *	supports YUV444, and so on.
+ *
+ * Find the best suitable HDMI output considering mode, source capability,
+ * sink capability and user's choice (expressed in form of drm property)
+ *
+ * Returns:
+ * enum corresponding to best suitable output type on success.
+ * DRM_HDMI_OUTPUT_INVALID on failure.
+ */
+enum drm_hdmi_output_type
+drm_display_info_hdmi_output_type(struct drm_display_info *info,
+				struct drm_display_mode *mode,
+				enum drm_hdmi_output_type type,
+				u32 src_output_cap)
+{
+	bool ret;
+	bool mode_is_420_only = drm_mode_is_420_only(info, mode);
+
+	/*
+	 * If the preferred output is not set to YCBCR by user, and the mode
+	 * doesn't force us to drive YCBCR420 output, respect the user
+	 * preference for the output type. But if the mode is 420_only, we will
+	 * be force to drive YCBCR420 output.
+	 */
+	if (!mode_is_420_only) {
+		if (type == DRM_HDMI_OUTPUT_DEFAULT_RGB)
+			return DRM_HDMI_OUTPUT_DEFAULT_RGB;
+	} else {
+		type = DRM_HDMI_OUTPUT_YCBCR420;
+		DRM_DEBUG_KMS("Got a 420 only mode(%s)\n", mode->name);
+	}
+
+	/* If  this src + sink combination can support any YCBCR output */
+	ret = drm_display_supports_ycbcr_output(info, src_output_cap);
+	if (!ret) {
+		DRM_ERROR("No supported YCBCR output\n");
+		return DRM_HDMI_OUTPUT_INVALID;
+	}
+
+	switch (type) {
+	case DRM_HDMI_OUTPUT_YCBCR_HQ:
+		type = drm_get_highest_quality_ycbcr_supported(info, mode,
+								src_output_cap);
+		break;
+
+	case DRM_HDMI_OUTPUT_YCBCR_LQ:
+		type = drm_get_lowest_quality_ycbcr_supported(info, mode,
+								src_output_cap);
+		break;
+
+	case DRM_HDMI_OUTPUT_YCBCR420:
+		ret = drm_mode_is_420(info, mode);
+		if (!ret) {
+			DRM_ERROR("Mode %s doesn't support 420 output\n",
+				   mode->name);
+			type = DRM_HDMI_OUTPUT_INVALID;
+		}
+
+		break;
+
+	/* Below cases are just to satisfy checkpatch's AI */
+	case DRM_HDMI_OUTPUT_DEFAULT_RGB:
+	case DRM_HDMI_OUTPUT_YCBCR444:
+	case DRM_HDMI_OUTPUT_YCBCR422:
+		break;
+
+	default:
+		type = DRM_HDMI_OUTPUT_INVALID;
+	}
+
+	if (type == DRM_HDMI_OUTPUT_INVALID) {
+		DRM_ERROR("Can't support mode %s in YCBCR format\n",
+				mode->name);
+		return DRM_HDMI_OUTPUT_INVALID;
+	}
+
+	/* Test if this src/sink/mode combination can support selected output */
+	ret = drm_display_supports_this_ycbcr_output(info, mode, type,
+						src_output_cap);
+	if (!ret) {
+		DRM_ERROR("Output %s can't be supported\n",
+					drm_get_hdmi_output_name(type));
+		return DRM_HDMI_OUTPUT_INVALID;
+	}
+
+	DRM_DEBUG_KMS("Best supported output is: %s\n",
+			drm_get_hdmi_output_name(type));
+	return type;
+}
+EXPORT_SYMBOL(drm_display_info_hdmi_output_type);
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index f8a1268..c5824aa 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -453,6 +453,11 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
 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);
 
+enum drm_hdmi_output_type
+drm_display_info_hdmi_output_type(struct drm_display_info *info,
+			  struct drm_display_mode *mode,
+			  enum drm_hdmi_output_type type,
+			  u32 src_output_cap);
 struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
 				      int hdisplay, int vdisplay, int vrefresh,
 				      bool reduced, bool interlaced,
-- 
2.7.4

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

[PATCH v5 12/17] drm/i915: add compute-config for YCBCR outputs

[Shashank Sharma]

This patch checks encoder level support for HDMI YCBCR outputs.
HDMI output mode is a connector property, this patch checks if
source and sink can support the HDMI output type selected by user.
And if they both can, it commits the hdmi output type 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

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    | 93 ++++++++++++++++++++++++++++++++++--
 3 files changed, 94 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4e03ca6..082270d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11930,6 +11930,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(hdmi_output);
 
 	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..cc25d71 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 */
+	enum drm_hdmi_output_type hdmi_output;
 };
 
 struct intel_crtc {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index cc0d100..3a07431 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1305,7 +1305,8 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	return status;
 }
 
-static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
+static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state,
+				enum drm_hdmi_output_type hdmi_out)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(crtc_state->base.crtc->dev);
@@ -1317,6 +1318,16 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 	if (HAS_GMCH_DISPLAY(dev_priv))
 		return false;
 
+	if (hdmi_out == DRM_HDMI_OUTPUT_YCBCR422) {
+		/*
+		 * HDMI spec says YCBCR422 is 12bpc, but its not a deep
+		 * color format. So respect the spec, and not allow this
+		 * to be deep color
+		 */
+		DRM_DEBUG_KMS("Not allowing deep color for YCBCR422 output\n");
+		return false;
+	}
+
 	/*
 	 * HDMI 12bpc affects the clocks, so it's only possible
 	 * when not cloning with other encoder types.
@@ -1330,6 +1341,12 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 		if (connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
+		if (hdmi_out == DRM_HDMI_OUTPUT_YCBCR420) {
+			if (!(info->hdmi.y420_dc_modes &
+					DRM_EDID_YCBCR420_DC_36))
+				return false;
+		}
+
 		if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0)
 			return false;
 	}
@@ -1342,6 +1359,65 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 	return true;
 }
 
+static u8
+intel_hdmi_get_src_output_support(struct drm_connector *connector)
+{
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+	u8 supported_outputs = DRM_COLOR_FORMAT_RGB444;
+
+	if (dev_priv->info.gen < 7)
+		return supported_outputs;
+
+	/* Gen 7 and above support HDMI 1.4b outputs */
+	supported_outputs |= DRM_COLOR_FORMAT_YCRCB444;
+
+	if (IS_GEMINILAKE(dev_priv))
+		supported_outputs |= DRM_COLOR_FORMAT_YCRCB420;
+
+	return supported_outputs;
+}
+
+static enum drm_hdmi_output_type
+intel_hdmi_compute_ycbcr_config(struct drm_connector_state *conn_state,
+			       struct intel_crtc_state *config,
+			       int *clock_12bpc, int *clock_8bpc)
+{
+	struct drm_connector *connector = conn_state->connector;
+	struct drm_display_mode *mode = &config->base.adjusted_mode;
+	enum drm_hdmi_output_type type = conn_state->hdmi_output;
+	u8 src_output_cap = intel_hdmi_get_src_output_support(connector);
+
+	/*
+	 * Can we support any YCBCR output based on these parameters ?
+	 * current source capabilities +
+	 * current sink capabilities +
+	 * given video mode +
+	 * current user's choice for HDMI output (from connector property)
+	 */
+	type = drm_display_info_hdmi_output_type(&connector->display_info,
+						 mode, type, src_output_cap);
+	if ((type == DRM_HDMI_OUTPUT_INVALID) ||
+		(type == DRM_HDMI_OUTPUT_DEFAULT_RGB)) {
+		DRM_DEBUG_KMS("Can't support YCBCR output\n");
+		return type;
+	}
+
+	if (type == DRM_HDMI_OUTPUT_YCBCR420) {
+
+		/* YCBCR420 TMDS rate requirement is half the pixel clock */
+		config->hdmi_output = DRM_HDMI_OUTPUT_YCBCR420;
+		config->port_clock /= 2;
+		*clock_12bpc /= 2;
+		*clock_8bpc /= 2;
+
+	}
+
+	/* Encoder is capable of this output, lets commit to CRTC */
+	config->hdmi_output = type;
+	DRM_DEBUG_KMS("HDMI output: %s\n", drm_get_hdmi_output_name(type));
+	return type;
+}
+
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_state *pipe_config,
 			       struct drm_connector_state *conn_state)
@@ -1349,13 +1425,15 @@ 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;
 	int clock_12bpc = clock_8bpc * 3 / 2;
 	int desired_bpp;
 	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
+	enum drm_hdmi_output_type hdmi_out = conn_state->hdmi_output;
 
 	pipe_config->has_hdmi_sink = !force_dvi && intel_hdmi->has_hdmi_sink;
 
@@ -1379,6 +1457,15 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 		clock_12bpc *= 2;
 	}
 
+	/* Check if YCBCR HDMI output handling is required */
+	hdmi_out = intel_hdmi_compute_ycbcr_config(conn_state,
+					   pipe_config, &clock_12bpc,
+					   &clock_8bpc);
+	if (hdmi_out == DRM_HDMI_OUTPUT_INVALID) {
+		DRM_ERROR("Can't support desired HDMI output\n");
+		return false;
+	}
+
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
 		pipe_config->has_pch_encoder = true;
 
@@ -1398,7 +1485,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	 */
 	if (pipe_config->pipe_bpp > 8*3 && pipe_config->has_hdmi_sink && !force_dvi &&
 	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc, true, force_dvi) == MODE_OK &&
-	    hdmi_12bpc_possible(pipe_config)) {
+	    hdmi_12bpc_possible(pipe_config, hdmi_out)) {
 		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
 		desired_bpp = 12*3;
 
-- 
2.7.4

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

[PATCH v5 13/17] 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.

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_atomic.c  |  6 ++++++
 drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     | 10 +++++++++-
 drivers/gpu/drm/i915/intel_hdmi.c    | 10 ++++++++++
 drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
 5 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 36d4e63..a8c9ac5 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
 
 			/* panel fitter case: assign as a crtc scaler */
 			scaler_id = &scaler_state->scaler_id;
+		} else if (i == SKL_HDMI_OUTPUT_INDEX) {
+			name = "HDMI-OUTPUT";
+			idx = intel_crtc->base.base.id;
+
+			/* hdmi output case: needs a pipe scaler */
+			scaler_id = &scaler_state->scaler_id;
 		} else {
 			name = "PLANE";
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 082270d..ee1452e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4621,6 +4621,10 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	 */
 	need_scaling = src_w != dst_w || src_h != dst_h;
 
+	if (scaler_user == SKL_HDMI_OUTPUT_INDEX)
+		/* YCBCR 444 -> 420 conversion needs a scaler */
+		need_scaling = true;
+
 	/*
 	 * if plane is being disabled or scaler is no more required or force detach
 	 *  - free scaler binded to this plane/crtc
@@ -4668,6 +4672,26 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 }
 
 /**
+ * skl_update_scaler_hdmi_output - Stages update to scaler state for HDMI.
+ * HDMI YCBCR 420 output needs a scaler, for downsampling.
+ *
+ * @state: crtc's scaler state
+ *
+ * Return
+ *     0 - scaler_usage updated successfully
+ *    error - requested scaling cannot be supported or other error condition
+ */
+int skl_update_scaler_crtc_hdmi_output(struct intel_crtc_state *state)
+{
+	const struct drm_display_mode *mode = &state->base.adjusted_mode;
+
+	return skl_update_scaler(state, !state->base.active,
+		SKL_HDMI_OUTPUT_INDEX, &state->scaler_state.scaler_id,
+		state->pipe_src_w, state->pipe_src_h,
+		mode->crtc_hdisplay, mode->crtc_vdisplay);
+}
+
+/**
  * skl_update_scaler_crtc - Stages update to scaler state for a given crtc.
  *
  * @state: crtc's scaler state
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cc25d71..86af283 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,12 @@ struct intel_crtc_scaler_state {
 	 * avilability.
 	 */
 #define SKL_CRTC_INDEX 31
+
+	/*
+	 * HDMI YCBCR 420 output consume a scaler. So adding a user
+	 * for HDMI output 420 requirement.
+	 */
+#define SKL_HDMI_OUTPUT_INDEX 30
 	unsigned scaler_users;
 
 	/* scaler used by crtc for panel fitting purpose */
@@ -1483,6 +1490,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
 
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
+int skl_update_scaler_crtc_hdmi_output(struct intel_crtc_state *state);
 int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
 
 static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 3a07431..948cbce 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1403,6 +1403,7 @@ intel_hdmi_compute_ycbcr_config(struct drm_connector_state *conn_state,
 	}
 
 	if (type == DRM_HDMI_OUTPUT_YCBCR420) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(conn_state->crtc);
 
 		/* YCBCR420 TMDS rate requirement is half the pixel clock */
 		config->hdmi_output = DRM_HDMI_OUTPUT_YCBCR420;
@@ -1410,6 +1411,15 @@ intel_hdmi_compute_ycbcr_config(struct drm_connector_state *conn_state,
 		*clock_12bpc /= 2;
 		*clock_8bpc /= 2;
 
+		/* YCBCR 420 output conversion needs a scaler */
+		if (skl_update_scaler_crtc_hdmi_output(config)) {
+			DRM_ERROR("Scaler allocation for output failed\n");
+			return DRM_HDMI_OUTPUT_INVALID;
+		}
+
+		/* Bind this scaler to pipe */
+		intel_pch_panel_fitting(intel_crtc, config,
+					DRM_MODE_SCALE_FULLSCREEN);
 	}
 
 	/* Encoder is capable of this output, lets commit to CRTC */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 96c2cbd..b6a32c4 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->hdmi_output != DRM_HDMI_OUTPUT_YCBCR420)
 		goto done;
 
 	switch (fitting_mode) {
-- 
2.7.4

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

[PATCH v5 14/17] drm/i915: prepare pipe for YCBCR 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 YCBCR outputs.

V2: rebase
V3: rebase
V4: rebase
V4: added r-b from Ander

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 | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 64cc674..5aea2a9 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 ee1452e..3f87052 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8082,6 +8082,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);
+	enum drm_hdmi_output_type hdmi_out = intel_crtc->config->hdmi_output;
 
 	if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) {
 		u32 val = 0;
@@ -8107,6 +8108,15 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
 		if (intel_crtc->config->dither)
 			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
 
+		if (hdmi_out > DRM_HDMI_OUTPUT_DEFAULT_RGB) {
+			val |= PIPEMISC_OUTPUT_YCBCR;
+
+			if (hdmi_out == DRM_HDMI_OUTPUT_YCBCR420) {
+				val |= 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 v5 15/17] drm/i915: prepare csc unit for YCBCR HDMI output

[Shashank Sharma]

To support ycbcr HDMI output, we need a pipe CSC block to
do the RGB->YCBCR conversion, as the blender output is in RGB.

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

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   | 47 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 306c6b0..12d5f21 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -41,6 +41,19 @@
 
 #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
+
+/* Direct spec values for RGB->YUV conversion matrix */
+#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 +104,35 @@ 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);
+
+	/* We don't use high values for conversion */
+	I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
+	I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
+	I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0);
+
+	/* Program direct spec values for RGB to YCBCR conversion matrix */
+	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);
+
+	/* Spec postoffset values */
+	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);
+
+	/* CSC mode before gamma */
+	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 +143,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->hdmi_output > DRM_HDMI_OUTPUT_DEFAULT_RGB) {
+		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 3f87052..14110bd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6260,6 +6260,29 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
 			ilk_pipe_pixel_rate(crtc_state);
 }
 
+static int intel_crtc_ycbcr_config(struct intel_crtc_state *state)
+{
+	struct drm_crtc_state *drm_state = &state->base;
+	struct drm_i915_private *dev_priv = to_i915(drm_state->crtc->dev);
+
+	/* YCBCR420 is supported only in HDMI 2.0 controllers */
+	if ((state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420) &&
+		!IS_GEMINILAKE(dev_priv)) {
+		DRM_ERROR("YCBCR420 output is not supported\n");
+		return -EINVAL;
+	}
+
+	/* We need CSC for output conversion from RGB->YCBCR */
+	if (drm_state->ctm) {
+		DRM_ERROR("YCBCR output and CTM is not possible together\n");
+		return -EINVAL;
+	}
+
+	DRM_DEBUG_DRIVER("Output %s can be supported\n",
+			 drm_get_hdmi_output_name(state->hdmi_output));
+	return 0;
+}
+
 static int intel_crtc_compute_config(struct intel_crtc *crtc,
 				     struct intel_crtc_state *pipe_config)
 {
@@ -6289,6 +6312,13 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
+	/* YCBCR output check */
+	if (pipe_config->hdmi_output > DRM_HDMI_OUTPUT_DEFAULT_RGB &&
+		intel_crtc_ycbcr_config(pipe_config)) {
+		DRM_ERROR("Can't enable YCBCR output\n");
+		return -EINVAL;
+	}
+
 	/*
 	 * Pipe horizontal size must be even in:
 	 * - DVO ganged mode
-- 
2.7.4

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

[PATCH v5 16/17] drm/i915: set colorspace for ycbcr outputs

[Shashank Sharma]

When HDMI output is other than RGB, we have to load the
corresponding colorspace of output mode. This patch fills
the colorspace of AVI infoframe as per the HDMI output mode.

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

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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 948cbce..ba1559e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -472,6 +472,14 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 		return;
 	}
 
+	ret = drm_hdmi_avi_infoframe_set_colorspace(&frame.avi,
+						    adjusted_mode,
+						    crtc_state->hdmi_output);
+	if (ret < 0) {
+		DRM_ERROR("couldn't fill AVI colorspace\n");
+		return;
+	}
+
 	drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
-- 
2.7.4

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

[PATCH v5 17/17] 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
V4: Added r-b from Ander

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 ba1559e..cb6c658 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1967,6 +1967,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 HDMI YCBCR output handling in DRM layer

[Patchwork]

== Series Details ==

Series: HDMI YCBCR output handling in DRM layer
URL   : https://patchwork.freedesktop.org/series/26814/
State : success

== Summary ==

Series 26814v1 HDMI YCBCR output handling in DRM layer
https://patchwork.freedesktop.org/api/1.0/series/26814/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900) fdo#101516

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101516 https://bugs.freedesktop.org/show_bug.cgi?id=101516

fi-bdw-5557u     total:279  pass:264  dwarn:0   dfail:0   fail:3   skip:11  time:444s
fi-bdw-gvtdvm    total:279  pass:257  dwarn:8   dfail:0   fail:0   skip:14  time:435s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:357s
fi-bsw-n3050     total:279  pass:239  dwarn:0   dfail:0   fail:3   skip:36  time:512s
fi-bxt-j4205     total:279  pass:256  dwarn:0   dfail:0   fail:3   skip:19  time:503s
fi-byt-j1900     total:279  pass:251  dwarn:0   dfail:0   fail:3   skip:24  time:482s
fi-byt-n2820     total:279  pass:246  dwarn:1   dfail:0   fail:3   skip:28  time:473s
fi-glk-2a        total:279  pass:256  dwarn:0   dfail:0   fail:3   skip:19  time:588s
fi-hsw-4770      total:279  pass:259  dwarn:0   dfail:0   fail:3   skip:16  time:433s
fi-hsw-4770r     total:279  pass:259  dwarn:0   dfail:0   fail:3   skip:16  time:413s
fi-ilk-650       total:279  pass:225  dwarn:0   dfail:0   fail:3   skip:50  time:415s
fi-ivb-3520m     total:279  pass:257  dwarn:0   dfail:0   fail:3   skip:18  time:486s
fi-ivb-3770      total:279  pass:257  dwarn:0   dfail:0   fail:3   skip:18  time:470s
fi-kbl-7500u     total:279  pass:257  dwarn:0   dfail:0   fail:3   skip:18  time:459s
fi-kbl-7560u     total:279  pass:264  dwarn:1   dfail:0   fail:3   skip:10  time:559s
fi-kbl-r         total:279  pass:256  dwarn:1   dfail:0   fail:3   skip:18  time:567s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:560s
fi-skl-6260u     total:279  pass:265  dwarn:0   dfail:0   fail:3   skip:10  time:456s
fi-skl-6700hq    total:279  pass:219  dwarn:1   dfail:0   fail:33  skip:24  time:301s
fi-skl-6700k     total:279  pass:257  dwarn:0   dfail:0   fail:3   skip:18  time:466s
fi-skl-6770hq    total:279  pass:265  dwarn:0   dfail:0   fail:3   skip:10  time:470s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:435s
fi-snb-2520m     total:279  pass:247  dwarn:0   dfail:0   fail:3   skip:28  time:541s
fi-snb-2600      total:279  pass:246  dwarn:0   dfail:0   fail:3   skip:29  time:396s

89dc45e4c8f6a5da4ea3d3acdda73cab6f436cfc drm-tip: 2017y-07m-04d-12h-16m-28s UTC integration manifest
d988955 drm/i915/glk: set HDMI 2.0 identifier
55575ce drm/i915: set colorspace for ycbcr outputs
f69cee7 drm/i915: prepare csc unit for YCBCR HDMI output
90347f8 drm/i915: prepare pipe for YCBCR output
3d75f16 drm/i915: prepare scaler for YCBCR420 modeset
8b2f864 drm/i915: add compute-config for YCBCR outputs
e01473a drm: add helper functions for YCBCR output handling
83d7070 drm: set output colorspace in AVI infoframe
27176b4 drm: create hdmi output property
98225c1 drm/edid: parse ycbcr 420 deep color information
24cee3c drm/edid: parse YCBCR 420 videomodes from EDID
58379fb drm/edid: rename macro for CEA extended-tag
93ea723 drm/edid: parse sink information before CEA blocks
70070bf drm: add helper to validate ycbcr420 modes
19d2ba7 drm/edid: Complete CEA modedb(VIC 1-107)
38891a9 drm: add YCBCR 420 capability identifier
b44e257 drm: add HDMI 2.0 VIC support for AVI info-frames

== Logs ==

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

Re: [PATCH v5 09/17] drm: create hdmi output property

[Daniel Vetter]

On Tue, Jul 04, 2017 at 07:41:56PM +0530, Shashank Sharma wrote:
> HDMI displays can support various output types, based on
> the color space and subsampling type. The possible
> outputs from a HDMI 2.0 monitor could be:
>  - RGB
>  - YCBCR 444
>  - YCBCR 422
>  - YCBCR 420
> 
> This patch adds a drm property "hdmi_output_format", using which,
> a user can specify its preference, for the HDMI output type. The
> output type enums are similar to the mentioned outputs above. To
> handle various subsampling of YCBCR output types, this property
> allows two special cases:
>  - DRM_HDMI_OUTPUT_YCBCR_HQ
>    This indicates preferred output should be YCBCR output, with highest
>    subsampling rate by the source/sink, which can be typically:
>  	- ycbcr444
>  	- ycbcr422
>  	- ycbcr420
>  - DRM_HDMI_OUTPUT_YCBCR_LQ
>    This indicates preferred output should be YCBCR output, with lowest
>    subsampling rate supported by source/sink, which can be:
>  	- ycbcr420
>  	- ycbcr422
>  	- ycbcr444
> 
> Default value of the property is set to 0 = RGB, so no changes if you
> dont set the property.
> 
> PS: While doing modeset for YCBCR 420 only modes, this property is
>     ignored, as those timings can be supported only in YCBCR 420
>     output mode.
> 
> V2: Added description for the new variable to address build warning
> V3: Rebase
> V4: Rebase
> V5: Added get_property counterpart to fix IGT BAT failures (BAT/CI)
>     Danvet:
>     - Add documentation for the new property
>     - Create a sub-section for HDMI properties, and add documentation for
>       few more HDMI propeties. Added documentation for:
> 	- Broadcast RGB
> 	- aspect ratio
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>

Bunch more documentation nitpicks below. I'd personally also split up the
patch into documenting the existing props and adding the new format one.

Thanks, Daniel

> ---
>  Documentation/gpu/drm-kms.rst       | 12 +++++++
>  drivers/gpu/drm/drm_atomic.c        |  4 +++
>  drivers/gpu/drm/drm_atomic_helper.c |  4 +++
>  drivers/gpu/drm/drm_connector.c     | 69 ++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/drm_crtc_internal.h |  1 +
>  drivers/gpu/drm/drm_mode_config.c   |  4 +++
>  drivers/gpu/drm/i915/intel_modes.c  | 13 +++++++
>  include/drm/drm_connector.h         | 18 ++++++++++
>  include/drm/drm_mode_config.h       |  5 +++
>  9 files changed, 129 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 3072841..dcdd6ff 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -508,6 +508,18 @@ Standard Connector Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
>     :doc: standard connector properties
>  
> +Standard HDMI Properties
> +-----------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> +   :doc: hdmi_output_format
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> +   :doc: aspect ratio property

I'd have just created 1 DOC: section titled "standard HDMI properties" and
listed all of them in 1 comment block. People often forget to add the
include stanza in the .rst files, having bigger comments helps with that.

But feel free to go either way.

> +
> +.. kernel-doc:: drivers/gpu/drm/i915/intel_modes.c
> +   :doc: Broadcast RGB property

Please no generic properties documented in driver code.
> +
>  Plane Composition Properties
>  ----------------------------
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 09ca662..adcb89d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1192,6 +1192,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		state->picture_aspect_ratio = val;
>  	} else if (property == connector->scaling_mode_property) {
>  		state->scaling_mode = val;
> +	} else if (property == config->hdmi_output_property) {
> +		state->hdmi_output = val;
>  	} else if (connector->funcs->atomic_set_property) {
>  		return connector->funcs->atomic_set_property(connector,
>  				state, property, val);
> @@ -1272,6 +1274,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = state->picture_aspect_ratio;
>  	} else if (property == connector->scaling_mode_property) {
>  		*val = state->scaling_mode;
> +	} else if (property == config->hdmi_output_property) {
> +		*val = state->hdmi_output;
>  	} else if (connector->funcs->atomic_get_property) {
>  		return connector->funcs->atomic_get_property(connector,
>  				state, property, val);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 23e4661..2e7459f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -637,6 +637,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  			if (old_connector_state->link_status !=
>  			    new_connector_state->link_status)
>  				new_crtc_state->connectors_changed = true;
> +
> +			if (old_connector_state->hdmi_output !=
> +			    new_connector_state->hdmi_output)
> +				new_crtc_state->connectors_changed = true;
>  		}
>  
>  		if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 8072e6e..8357918 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev,
>  					      config->edid_property,
>  					      0);
>  
> +	if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)
> +		drm_object_attach_property(&connector->base,
> +					   config->hdmi_output_property,
> +					   0);
> +
>  	drm_object_attach_property(&connector->base,
>  				      config->dpms_property, 0);
>  
> @@ -617,6 +622,26 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>  };
>  DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>  
> +static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {
> +	{ DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
> +	{ DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" },
> +	{ DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
> +	{ DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" },
> +	{ DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" },
> +	{ DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" },
> +	{ DRM_HDMI_OUTPUT_INVALID, "invalid_output" },
> +};
> +
> +/**
> + * drm_get_hdmi_output_name - return a string for a given hdmi output enum
> + * @type: enum of output type
> + */
> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type)
> +{
> +	return drm_hdmi_output_enum_list[type].name;
> +}
> +EXPORT_SYMBOL(drm_get_hdmi_output_name);
> +
>  /**
>   * drm_display_info_set_bus_formats - set the supported bus formats
>   * @info: display info to store bus formats in
> @@ -697,7 +722,36 @@ static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = {
>  	{ DRM_MODE_SUBCONNECTOR_SCART,     "SCART"     }, /* TV-out */
>  };
>  DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
> -		 drm_tv_subconnector_enum_list)
> +		 drm_tv_subconnector_enum_list);
> +
> +/**
> + * DOC: hdmi_output_format
> + *
> + * hdmi_output_format:
> + *	Enum property which allows a userspace to provide its preference for a
> + *	HDMI output format. Standard HDMI 1.4b outputs can support RGB/YCBCR444
> + *	YCBCR422 output formats, whereas HDMI 2.0 outputs can additionally
> + *	support YCBCR420 output. Default value of the property is HDMI output
> + *	RGB.
> + *
> + *	A driver can attach to this property, and then handle the HDMI output
> + *	based on its capabilities and sink support. There are few helper
> + *	functions available in drm_modes.c which can help in finding the best
> + *	suitable output considering a sink/source/mode combination. Refer to
> + *	drm_modes.c:drm_display_info_hdmi_output_type()
> + */
> +int drm_connector_create_hdmi_properties(struct drm_device *dev)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create_enum(dev, 0, "hdmi_output_format",
> +					drm_hdmi_output_enum_list,
> +					ARRAY_SIZE(drm_hdmi_output_enum_list));
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.hdmi_output_property = prop;
> +	return 0;
> +}
>  
>  /**
>   * DOC: standard connector properties
> @@ -1024,6 +1078,19 @@ int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
>  }
>  EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property);
>  
> +
> +/**
> + * DOC: aspect ratio property
> + *
> + * aspect ratio:
> + *	Enum property to override the HDMI output's aspect ratio.
> + *	When this property is set, the aspect ratio of the frame in
> + *	AVI infoframes is set as per the property value. For example
> + *	if userspace sets the property value to DRM_MODE_PICTURE_ASPECT_4_3
> + *	the output aspect ratio is set to 4:3 (regardless of the PAR of mode)

Would be good to document all possible values. Same for the other
properties, plus insert a link to the function that creates the property
(where we have that).

> + *
> + */
> +
>  /**
>   * drm_mode_create_aspect_ratio_property - create aspect ratio property
>   * @dev: DRM device
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index d077c54..e6c4adc 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -140,6 +140,7 @@ int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
>  				    struct drm_property *property,
>  				    uint64_t value);
>  int drm_connector_create_standard_properties(struct drm_device *dev);
> +int drm_connector_create_hdmi_properties(struct drm_device *dev);
>  const char *drm_get_connector_force_name(enum drm_connector_force force);
>  
>  /* IOCTL */
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index d986225..3ba9262 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -210,6 +210,10 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  	if (ret)
>  		return ret;
>  
> +	ret = drm_connector_create_hdmi_properties(dev);
> +	if (ret)
> +		return ret;

We still create all the property values instead of just the ones that the
source hw supports. E.g. aspect ratio property is only created if needed,
not unconditionally on all connectors.

Note that users see properties through xrandr and the gui screen
configuration tools, adding properties that never do anything is
confusing, more for properties where you can select invalid values.

> +
>  	prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
>  					"type", drm_plane_type_enum_list,
>  					ARRAY_SIZE(drm_plane_type_enum_list));
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index 951e834..d07de45 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -104,6 +104,19 @@ static const struct drm_prop_enum_list broadcast_rgb_names[] = {
>  	{ INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
>  };
>  
> +/**
> + * DOC: Broadcast RGB property
> + *
> + * Broadcast RGB:
> + *	Enum property to indicate RGB color range of a HDMI output.
> + *	For limited range RGB outputs, a remapping of pixel values from 0-255
> + *	should be remaped to a range of 16-235. When this property is set to
> + *	Limited 16:235 and CTM is set, the hardware will be programmed with the
> + *	result of the multiplication of CTM by the limited  range matrix, to
> + *	ensure the pixels normaly in the range 0..1.0 are remapped to the range
> + *	16/255..235/255.
> + *
> + */
>  void
>  intel_attach_broadcast_rgb_property(struct drm_connector *connector)
>  {
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4bc0882..3773600 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -319,6 +319,17 @@ struct drm_tv_connector_state {
>  	unsigned int hue;
>  };
>  
> +/* HDMI output pixel format */
> +enum drm_hdmi_output_type {
> +	DRM_HDMI_OUTPUT_DEFAULT_RGB, /* default RGB */
> +	DRM_HDMI_OUTPUT_YCBCR444, /* YCBCR 444 */
> +	DRM_HDMI_OUTPUT_YCBCR422, /* YCBCR 422 */
> +	DRM_HDMI_OUTPUT_YCBCR420, /* YCBCR 420 */
> +	DRM_HDMI_OUTPUT_YCBCR_HQ, /* Highest subsampled YUV */
> +	DRM_HDMI_OUTPUT_YCBCR_LQ, /* Lowest subsampled YUV */
> +	DRM_HDMI_OUTPUT_INVALID, /* Guess what ? */
> +};
> +
>  /**
>   * struct drm_connector_state - mutable connector state
>   * @connector: backpointer to the connector
> @@ -363,6 +374,12 @@ struct drm_connector_state {
>  	 * upscaling, mostly used for built-in panels.
>  	 */
>  	unsigned int scaling_mode;
> +
> +	/**
> +	 * @hdmi_output: Connector property to control the
> +	 * HDMI output mode (RGB/YCBCR444/422/420).
> +	 */
> +	enum drm_hdmi_output_type hdmi_output;
>  };
>  
>  /**
> @@ -991,6 +1008,7 @@ static inline void drm_connector_unreference(struct drm_connector *connector)
>  
>  const char *drm_get_connector_status_name(enum drm_connector_status status);
>  const char *drm_get_subpixel_order_name(enum subpixel_order order);
> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type);
>  const char *drm_get_dpms_name(int val);
>  const char *drm_get_dvi_i_subconnector_name(int val);
>  const char *drm_get_dvi_i_select_name(int val);
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 4298171..1887261 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -740,6 +740,11 @@ struct drm_mode_config {
>  	 * the position of the output on the host's screen.
>  	 */
>  	struct drm_property *suggested_y_property;
> +	/**
> +	 * @hdmi_output_property: output pixel format from HDMI display
> +	 * Default is set for RGB
> +	 */
> +	struct drm_property *hdmi_output_property;
>  
>  	/* dumb ioctl parameters */
>  	uint32_t preferred_depth, prefer_shadow;
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH v5 02/17] drm: add YCBCR 420 capability identifier

[Ville Syrjälä]

On Tue, Jul 04, 2017 at 07:41:49PM +0530, Shashank Sharma wrote:
> This patch adds 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 YCBCR 420 outputs or not, so that
> a lot of work can be done/bypassed based on this information. One such
> example is adding YCBCR420 only modes.
> 
> If the driver knows that this source is not HDMI 2.0 capable, it will
> not add YCBCR420-only modes while adding EDID modes, and will prevent
> any runtime modeset failures.
> 
> This variable will be initialized from I915 driver in the next patch
> and will be used in the EDID handling for HDMI 2.0 specific features,
> in this same series.
> 
> V3: introduced the new variable
> V4: changed variable name from is_hdmi2_src to ycbcr_420_allowed (Ville)
> V4: adding r-b from Neil.
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  include/drm/drm_connector.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ae5b7dc..8a78aaa 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -726,6 +726,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;
> +

This patch should be squashed with the patch that actually needs this flag.

>  	/**
>  	 * @registered: Is this connector exposed (registered) with userspace?
>  	 * Protected by @mutex.
> -- 
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 04/17] drm: add helper to validate ycbcr420 modes

[Ville Syrjälä]

On Tue, Jul 04, 2017 at 07:41:51PM +0530, Shashank Sharma wrote:
> 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.
> 
> V5: Introduced the patch in series.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c         |  3 ++-
>  drivers/gpu/drm/drm_modes.c        | 28 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_probe_helper.c |  4 ++++
>  include/drm/drm_edid.h             |  1 +
>  include/drm/drm_modes.h            |  5 +++++
>  5 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b879662..ad26c5e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2947,10 +2947,11 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
>  }
>  EXPORT_SYMBOL(drm_match_cea_mode);
>  
> -static bool drm_valid_cea_vic(u8 vic)
> +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..3b53c8e3 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_ycbcr420_only - 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 (drm_valid_cea_vic(vic) && test_bit(vic, hdmi->y420_vdb_modes)) {

The drm_valid_cea_vic() check seems redundant to me.
Why do you think we need it?

Also I don't think this will compile since we don't have
y420_vdb_modes[] yet.

> +		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[] = {
> 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_edid.h b/include/drm/drm_edid.h
> index 89c0062..b55b2a7 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -477,4 +477,5 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name,
>  struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
>  					   int hsize, int vsize, int fresh,
>  					   bool rb);
> +bool drm_valid_cea_vic(u8 vic);
>  #endif /* __DRM_EDID_H__ */
> 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

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

Re: [PATCH v5 02/17] drm: add YCBCR 420 capability identifier

[Sharma, Shashank]

Regards

Shashank


On 7/4/2017 9:25 PM, Ville Syrjälä wrote:
> On Tue, Jul 04, 2017 at 07:41:49PM +0530, Shashank Sharma wrote:
>> This patch adds 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 YCBCR 420 outputs or not, so that
>> a lot of work can be done/bypassed based on this information. One such
>> example is adding YCBCR420 only modes.
>>
>> If the driver knows that this source is not HDMI 2.0 capable, it will
>> not add YCBCR420-only modes while adding EDID modes, and will prevent
>> any runtime modeset failures.
>>
>> This variable will be initialized from I915 driver in the next patch
>> and will be used in the EDID handling for HDMI 2.0 specific features,
>> in this same series.
>>
>> V3: introduced the new variable
>> V4: changed variable name from is_hdmi2_src to ycbcr_420_allowed (Ville)
>> V4: adding r-b from Neil.
>>
>> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   include/drm/drm_connector.h | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index ae5b7dc..8a78aaa 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -726,6 +726,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;
>> +
> This patch should be squashed with the patch that actually needs this flag.
This is used in patch 4, to filter out 420_only modes. So I should merge 
it there right ?
- Shashank
>>   	/**
>>   	 * @registered: Is this connector exposed (registered) with userspace?
>>   	 * Protected by @mutex.
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH v5 04/17] drm: add helper to validate ycbcr420 modes

[Sharma, Shashank]

Regards

Shashank


On 7/4/2017 9:26 PM, Ville Syrjälä wrote:
> On Tue, Jul 04, 2017 at 07:41:51PM +0530, Shashank Sharma wrote:
>> 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.
>>
>> V5: Introduced the patch in series.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c         |  3 ++-
>>   drivers/gpu/drm/drm_modes.c        | 28 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/drm_probe_helper.c |  4 ++++
>>   include/drm/drm_edid.h             |  1 +
>>   include/drm/drm_modes.h            |  5 +++++
>>   5 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index b879662..ad26c5e 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2947,10 +2947,11 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
>>   }
>>   EXPORT_SYMBOL(drm_match_cea_mode);
>>   
>> -static bool drm_valid_cea_vic(u8 vic)
>> +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..3b53c8e3 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_ycbcr420_only - 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 (drm_valid_cea_vic(vic) && test_bit(vic, hdmi->y420_vdb_modes)) {
> The drm_valid_cea_vic() check seems redundant to me.
> Why do you think we need it?
drm_match_cea_mode() returns 0 in case if a vic is not found. if we take 
that to test_bit() it will check bit 0
and give wrong results. So first we have to check valid vic, this is 
also an additional check for YCBCR420 mode
as they must have a valid vic
> Also I don't think this will compile since we don't have
> y420_vdb_modes[] yet.
Ah, now I recall the reason I wanted to give you, when you asked me to 
move this patch, before we add ycbcr420 modes.
So now this has to go after patch 5 right ? I would re-sequence 
accordingly.

- Shashank
>> +		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[] = {
>> 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_edid.h b/include/drm/drm_edid.h
>> index 89c0062..b55b2a7 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -477,4 +477,5 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name,
>>   struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
>>   					   int hsize, int vsize, int fresh,
>>   					   bool rb);
>> +bool drm_valid_cea_vic(u8 vic);
>>   #endif /* __DRM_EDID_H__ */
>> 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

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

Re: [PATCH v5 09/17] drm: create hdmi output property

[Sharma, Shashank]

Regards

Shashank


On 7/4/2017 9:06 PM, Daniel Vetter wrote:
> On Tue, Jul 04, 2017 at 07:41:56PM +0530, Shashank Sharma wrote:
>> HDMI displays can support various output types, based on
>> the color space and subsampling type. The possible
>> outputs from a HDMI 2.0 monitor could be:
>>   - RGB
>>   - YCBCR 444
>>   - YCBCR 422
>>   - YCBCR 420
>>
>> This patch adds a drm property "hdmi_output_format", using which,
>> a user can specify its preference, for the HDMI output type. The
>> output type enums are similar to the mentioned outputs above. To
>> handle various subsampling of YCBCR output types, this property
>> allows two special cases:
>>   - DRM_HDMI_OUTPUT_YCBCR_HQ
>>     This indicates preferred output should be YCBCR output, with highest
>>     subsampling rate by the source/sink, which can be typically:
>>   	- ycbcr444
>>   	- ycbcr422
>>   	- ycbcr420
>>   - DRM_HDMI_OUTPUT_YCBCR_LQ
>>     This indicates preferred output should be YCBCR output, with lowest
>>     subsampling rate supported by source/sink, which can be:
>>   	- ycbcr420
>>   	- ycbcr422
>>   	- ycbcr444
>>
>> Default value of the property is set to 0 = RGB, so no changes if you
>> dont set the property.
>>
>> PS: While doing modeset for YCBCR 420 only modes, this property is
>>      ignored, as those timings can be supported only in YCBCR 420
>>      output mode.
>>
>> V2: Added description for the new variable to address build warning
>> V3: Rebase
>> V4: Rebase
>> V5: Added get_property counterpart to fix IGT BAT failures (BAT/CI)
>>      Danvet:
>>      - Add documentation for the new property
>>      - Create a sub-section for HDMI properties, and add documentation for
>>        few more HDMI propeties. Added documentation for:
>> 	- Broadcast RGB
>> 	- aspect ratio
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Jose Abreu <joabreu@synopsys.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Bunch more documentation nitpicks below. I'd personally also split up the
> patch into documenting the existing props and adding the new format one.
In fact thats what I would do now. I dont want HDMI 2.0 patch series to 
get blocked due to documentation, as with all the comments
it seems like some work. Lets do it in this way:
  - This patch series will add the documentation for hdmi_output_property
- I will send a separate patch which will collectively add documentation 
for all standard HDMI properties.
> Thanks, Daniel
>
>> ---
>>   Documentation/gpu/drm-kms.rst       | 12 +++++++
>>   drivers/gpu/drm/drm_atomic.c        |  4 +++
>>   drivers/gpu/drm/drm_atomic_helper.c |  4 +++
>>   drivers/gpu/drm/drm_connector.c     | 69 ++++++++++++++++++++++++++++++++++++-
>>   drivers/gpu/drm/drm_crtc_internal.h |  1 +
>>   drivers/gpu/drm/drm_mode_config.c   |  4 +++
>>   drivers/gpu/drm/i915/intel_modes.c  | 13 +++++++
>>   include/drm/drm_connector.h         | 18 ++++++++++
>>   include/drm/drm_mode_config.h       |  5 +++
>>   9 files changed, 129 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
>> index 3072841..dcdd6ff 100644
>> --- a/Documentation/gpu/drm-kms.rst
>> +++ b/Documentation/gpu/drm-kms.rst
>> @@ -508,6 +508,18 @@ Standard Connector Properties
>>   .. kernel-doc:: drivers/gpu/drm/drm_connector.c
>>      :doc: standard connector properties
>>   
>> +Standard HDMI Properties
>> +-----------------------------
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
>> +   :doc: hdmi_output_format
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
>> +   :doc: aspect ratio property
> I'd have just created 1 DOC: section titled "standard HDMI properties" and
> listed all of them in 1 comment block. People often forget to add the
> include stanza in the .rst files, having bigger comments helps with that.
>
> But feel free to go either way.
Yes, this would be easier for me too, but this means I will have to add 
all the documentation in one place, in one file, whether the respective
property is created at that place or not. I am not sure if everyone 
would be ok with that during review. But if you think we should go ahead,
thanks for easing this up for me :-).
>
>> +
>> +.. kernel-doc:: drivers/gpu/drm/i915/intel_modes.c
>> +   :doc: Broadcast RGB property
> Please no generic properties documented in driver code.
Broadcast RGB property is created here, and also, while I was adding 
documentation for it, I realized its kept in dev_priv, which is specific to
I915 driver, So its not generic too. I was not sure if that will be OK, 
if I add an Intel specific property's description in DRM layer ?
>> +
>>   Plane Composition Properties
>>   ----------------------------
>>   
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 09ca662..adcb89d 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -1192,6 +1192,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>>   		state->picture_aspect_ratio = val;
>>   	} else if (property == connector->scaling_mode_property) {
>>   		state->scaling_mode = val;
>> +	} else if (property == config->hdmi_output_property) {
>> +		state->hdmi_output = val;
>>   	} else if (connector->funcs->atomic_set_property) {
>>   		return connector->funcs->atomic_set_property(connector,
>>   				state, property, val);
>> @@ -1272,6 +1274,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>>   		*val = state->picture_aspect_ratio;
>>   	} else if (property == connector->scaling_mode_property) {
>>   		*val = state->scaling_mode;
>> +	} else if (property == config->hdmi_output_property) {
>> +		*val = state->hdmi_output;
>>   	} else if (connector->funcs->atomic_get_property) {
>>   		return connector->funcs->atomic_get_property(connector,
>>   				state, property, val);
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 23e4661..2e7459f 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -637,6 +637,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>>   			if (old_connector_state->link_status !=
>>   			    new_connector_state->link_status)
>>   				new_crtc_state->connectors_changed = true;
>> +
>> +			if (old_connector_state->hdmi_output !=
>> +			    new_connector_state->hdmi_output)
>> +				new_crtc_state->connectors_changed = true;
>>   		}
>>   
>>   		if (funcs->atomic_check)
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 8072e6e..8357918 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev,
>>   					      config->edid_property,
>>   					      0);
>>   
>> +	if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)
>> +		drm_object_attach_property(&connector->base,
>> +					   config->hdmi_output_property,
>> +					   0);
>> +
>>   	drm_object_attach_property(&connector->base,
>>   				      config->dpms_property, 0);
>>   
>> @@ -617,6 +622,26 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>>   };
>>   DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>>   
>> +static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {
>> +	{ DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
>> +	{ DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" },
>> +	{ DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
>> +	{ DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" },
>> +	{ DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" },
>> +	{ DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" },
>> +	{ DRM_HDMI_OUTPUT_INVALID, "invalid_output" },
>> +};
>> +
>> +/**
>> + * drm_get_hdmi_output_name - return a string for a given hdmi output enum
>> + * @type: enum of output type
>> + */
>> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type)
>> +{
>> +	return drm_hdmi_output_enum_list[type].name;
>> +}
>> +EXPORT_SYMBOL(drm_get_hdmi_output_name);
>> +
>>   /**
>>    * drm_display_info_set_bus_formats - set the supported bus formats
>>    * @info: display info to store bus formats in
>> @@ -697,7 +722,36 @@ static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = {
>>   	{ DRM_MODE_SUBCONNECTOR_SCART,     "SCART"     }, /* TV-out */
>>   };
>>   DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
>> -		 drm_tv_subconnector_enum_list)
>> +		 drm_tv_subconnector_enum_list);
>> +
>> +/**
>> + * DOC: hdmi_output_format
>> + *
>> + * hdmi_output_format:
>> + *	Enum property which allows a userspace to provide its preference for a
>> + *	HDMI output format. Standard HDMI 1.4b outputs can support RGB/YCBCR444
>> + *	YCBCR422 output formats, whereas HDMI 2.0 outputs can additionally
>> + *	support YCBCR420 output. Default value of the property is HDMI output
>> + *	RGB.
>> + *
>> + *	A driver can attach to this property, and then handle the HDMI output
>> + *	based on its capabilities and sink support. There are few helper
>> + *	functions available in drm_modes.c which can help in finding the best
>> + *	suitable output considering a sink/source/mode combination. Refer to
>> + *	drm_modes.c:drm_display_info_hdmi_output_type()
>> + */
>> +int drm_connector_create_hdmi_properties(struct drm_device *dev)
>> +{
>> +	struct drm_property *prop;
>> +
>> +	prop = drm_property_create_enum(dev, 0, "hdmi_output_format",
>> +					drm_hdmi_output_enum_list,
>> +					ARRAY_SIZE(drm_hdmi_output_enum_list));
>> +	if (!prop)
>> +		return -ENOMEM;
>> +	dev->mode_config.hdmi_output_property = prop;
>> +	return 0;
>> +}
>>   
>>   /**
>>    * DOC: standard connector properties
>> @@ -1024,6 +1078,19 @@ int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
>>   }
>>   EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property);
>>   
>> +
>> +/**
>> + * DOC: aspect ratio property
>> + *
>> + * aspect ratio:
>> + *	Enum property to override the HDMI output's aspect ratio.
>> + *	When this property is set, the aspect ratio of the frame in
>> + *	AVI infoframes is set as per the property value. For example
>> + *	if userspace sets the property value to DRM_MODE_PICTURE_ASPECT_4_3
>> + *	the output aspect ratio is set to 4:3 (regardless of the PAR of mode)
> Would be good to document all possible values. Same for the other
> properties, plus insert a link to the function that creates the property
> (where we have that).
This is the part, which makes me feel that I should create this patch 
separately, as this seems like slightly bigger cleanup than
I thought, so I dont want this to be a part of HDMI 2.0 series.
>
>> + *
>> + */
>> +
>>   /**
>>    * drm_mode_create_aspect_ratio_property - create aspect ratio property
>>    * @dev: DRM device
>> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
>> index d077c54..e6c4adc 100644
>> --- a/drivers/gpu/drm/drm_crtc_internal.h
>> +++ b/drivers/gpu/drm/drm_crtc_internal.h
>> @@ -140,6 +140,7 @@ int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
>>   				    struct drm_property *property,
>>   				    uint64_t value);
>>   int drm_connector_create_standard_properties(struct drm_device *dev);
>> +int drm_connector_create_hdmi_properties(struct drm_device *dev);
>>   const char *drm_get_connector_force_name(enum drm_connector_force force);
>>   
>>   /* IOCTL */
>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>> index d986225..3ba9262 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -210,6 +210,10 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	ret = drm_connector_create_hdmi_properties(dev);
>> +	if (ret)
>> +		return ret;
> We still create all the property values instead of just the ones that the
> source hw supports. E.g. aspect ratio property is only created if needed,
> not unconditionally on all connectors.
>
> Note that users see properties through xrandr and the gui screen
> configuration tools, adding properties that never do anything is
> confusing, more for properties where you can select invalid values.
If I understood this properly, you want me to create the property in 
I915 layer, just before attaching it to object (similar to aspect ratio 
and others)
something like:
intel_attach_hdmi_output_property()
{
       if (!mode_config->hdmi_output_property)
                    mode_config->hdmi_output_property = create_prop();
         now_attach_property();
}

I was assuming that once other drivers move to HDMI 2.0, they might all 
need this for YCBCR420 outputs, so it would be ok to create it generically.
But at the same time, this also seems like a good idea.

- Shashank
>> +
>>   	prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
>>   					"type", drm_plane_type_enum_list,
>>   					ARRAY_SIZE(drm_plane_type_enum_list));
>> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
>> index 951e834..d07de45 100644
>> --- a/drivers/gpu/drm/i915/intel_modes.c
>> +++ b/drivers/gpu/drm/i915/intel_modes.c
>> @@ -104,6 +104,19 @@ static const struct drm_prop_enum_list broadcast_rgb_names[] = {
>>   	{ INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
>>   };
>>   
>> +/**
>> + * DOC: Broadcast RGB property
>> + *
>> + * Broadcast RGB:
>> + *	Enum property to indicate RGB color range of a HDMI output.
>> + *	For limited range RGB outputs, a remapping of pixel values from 0-255
>> + *	should be remaped to a range of 16-235. When this property is set to
>> + *	Limited 16:235 and CTM is set, the hardware will be programmed with the
>> + *	result of the multiplication of CTM by the limited  range matrix, to
>> + *	ensure the pixels normaly in the range 0..1.0 are remapped to the range
>> + *	16/255..235/255.
>> + *
>> + */
>>   void
>>   intel_attach_broadcast_rgb_property(struct drm_connector *connector)
>>   {
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 4bc0882..3773600 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -319,6 +319,17 @@ struct drm_tv_connector_state {
>>   	unsigned int hue;
>>   };
>>   
>> +/* HDMI output pixel format */
>> +enum drm_hdmi_output_type {
>> +	DRM_HDMI_OUTPUT_DEFAULT_RGB, /* default RGB */
>> +	DRM_HDMI_OUTPUT_YCBCR444, /* YCBCR 444 */
>> +	DRM_HDMI_OUTPUT_YCBCR422, /* YCBCR 422 */
>> +	DRM_HDMI_OUTPUT_YCBCR420, /* YCBCR 420 */
>> +	DRM_HDMI_OUTPUT_YCBCR_HQ, /* Highest subsampled YUV */
>> +	DRM_HDMI_OUTPUT_YCBCR_LQ, /* Lowest subsampled YUV */
>> +	DRM_HDMI_OUTPUT_INVALID, /* Guess what ? */
>> +};
>> +
>>   /**
>>    * struct drm_connector_state - mutable connector state
>>    * @connector: backpointer to the connector
>> @@ -363,6 +374,12 @@ struct drm_connector_state {
>>   	 * upscaling, mostly used for built-in panels.
>>   	 */
>>   	unsigned int scaling_mode;
>> +
>> +	/**
>> +	 * @hdmi_output: Connector property to control the
>> +	 * HDMI output mode (RGB/YCBCR444/422/420).
>> +	 */
>> +	enum drm_hdmi_output_type hdmi_output;
>>   };
>>   
>>   /**
>> @@ -991,6 +1008,7 @@ static inline void drm_connector_unreference(struct drm_connector *connector)
>>   
>>   const char *drm_get_connector_status_name(enum drm_connector_status status);
>>   const char *drm_get_subpixel_order_name(enum subpixel_order order);
>> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type);
>>   const char *drm_get_dpms_name(int val);
>>   const char *drm_get_dvi_i_subconnector_name(int val);
>>   const char *drm_get_dvi_i_select_name(int val);
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 4298171..1887261 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -740,6 +740,11 @@ struct drm_mode_config {
>>   	 * the position of the output on the host's screen.
>>   	 */
>>   	struct drm_property *suggested_y_property;
>> +	/**
>> +	 * @hdmi_output_property: output pixel format from HDMI display
>> +	 * Default is set for RGB
>> +	 */
>> +	struct drm_property *hdmi_output_property;
>>   
>>   	/* dumb ioctl parameters */
>>   	uint32_t preferred_depth, prefer_shadow;
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH v5 09/17] drm: create hdmi output property

[Daniel Vetter]

On Wed, Jul 05, 2017 at 11:39:30AM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 7/4/2017 9:06 PM, Daniel Vetter wrote:
> > On Tue, Jul 04, 2017 at 07:41:56PM +0530, Shashank Sharma wrote:
> > > HDMI displays can support various output types, based on
> > > the color space and subsampling type. The possible
> > > outputs from a HDMI 2.0 monitor could be:
> > >   - RGB
> > >   - YCBCR 444
> > >   - YCBCR 422
> > >   - YCBCR 420
> > > 
> > > This patch adds a drm property "hdmi_output_format", using which,
> > > a user can specify its preference, for the HDMI output type. The
> > > output type enums are similar to the mentioned outputs above. To
> > > handle various subsampling of YCBCR output types, this property
> > > allows two special cases:
> > >   - DRM_HDMI_OUTPUT_YCBCR_HQ
> > >     This indicates preferred output should be YCBCR output, with highest
> > >     subsampling rate by the source/sink, which can be typically:
> > >   	- ycbcr444
> > >   	- ycbcr422
> > >   	- ycbcr420
> > >   - DRM_HDMI_OUTPUT_YCBCR_LQ
> > >     This indicates preferred output should be YCBCR output, with lowest
> > >     subsampling rate supported by source/sink, which can be:
> > >   	- ycbcr420
> > >   	- ycbcr422
> > >   	- ycbcr444
> > > 
> > > Default value of the property is set to 0 = RGB, so no changes if you
> > > dont set the property.
> > > 
> > > PS: While doing modeset for YCBCR 420 only modes, this property is
> > >      ignored, as those timings can be supported only in YCBCR 420
> > >      output mode.
> > > 
> > > V2: Added description for the new variable to address build warning
> > > V3: Rebase
> > > V4: Rebase
> > > V5: Added get_property counterpart to fix IGT BAT failures (BAT/CI)
> > >      Danvet:
> > >      - Add documentation for the new property
> > >      - Create a sub-section for HDMI properties, and add documentation for
> > >        few more HDMI propeties. Added documentation for:
> > > 	- Broadcast RGB
> > > 	- aspect ratio
> > > 
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Cc: Jose Abreu <joabreu@synopsys.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > Bunch more documentation nitpicks below. I'd personally also split up the
> > patch into documenting the existing props and adding the new format one.
> In fact thats what I would do now. I dont want HDMI 2.0 patch series to get
> blocked due to documentation, as with all the comments
> it seems like some work. Lets do it in this way:
>  - This patch series will add the documentation for hdmi_output_property
> - I will send a separate patch which will collectively add documentation for
> all standard HDMI properties.
> > Thanks, Daniel
> > 
> > > ---
> > >   Documentation/gpu/drm-kms.rst       | 12 +++++++
> > >   drivers/gpu/drm/drm_atomic.c        |  4 +++
> > >   drivers/gpu/drm/drm_atomic_helper.c |  4 +++
> > >   drivers/gpu/drm/drm_connector.c     | 69 ++++++++++++++++++++++++++++++++++++-
> > >   drivers/gpu/drm/drm_crtc_internal.h |  1 +
> > >   drivers/gpu/drm/drm_mode_config.c   |  4 +++
> > >   drivers/gpu/drm/i915/intel_modes.c  | 13 +++++++
> > >   include/drm/drm_connector.h         | 18 ++++++++++
> > >   include/drm/drm_mode_config.h       |  5 +++
> > >   9 files changed, 129 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > > index 3072841..dcdd6ff 100644
> > > --- a/Documentation/gpu/drm-kms.rst
> > > +++ b/Documentation/gpu/drm-kms.rst
> > > @@ -508,6 +508,18 @@ Standard Connector Properties
> > >   .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > >      :doc: standard connector properties
> > > +Standard HDMI Properties
> > > +-----------------------------
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > > +   :doc: hdmi_output_format
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > > +   :doc: aspect ratio property
> > I'd have just created 1 DOC: section titled "standard HDMI properties" and
> > listed all of them in 1 comment block. People often forget to add the
> > include stanza in the .rst files, having bigger comments helps with that.
> > 
> > But feel free to go either way.
> Yes, this would be easier for me too, but this means I will have to add all
> the documentation in one place, in one file, whether the respective
> property is created at that place or not. I am not sure if everyone would be
> ok with that during review. But if you think we should go ahead,
> thanks for easing this up for me :-).

It might result in a minor merge conflict, but nothing bad should happen
if we put all the docs into one section.

> > > +.. kernel-doc:: drivers/gpu/drm/i915/intel_modes.c
> > > +   :doc: Broadcast RGB property
> > Please no generic properties documented in driver code.
> Broadcast RGB property is created here, and also, while I was adding
> documentation for it, I realized its kept in dev_priv, which is specific to
> I915 driver, So its not generic too. I was not sure if that will be OK, if I
> add an Intel specific property's description in DRM layer ?

It should be generic, but oh well it isn't. Either document it in the
core, or we'll leave this to the future when the create helper gets
extracted to the core.

> > > +
> > >   Plane Composition Properties
> > >   ----------------------------
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 09ca662..adcb89d 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -1192,6 +1192,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
> > >   		state->picture_aspect_ratio = val;
> > >   	} else if (property == connector->scaling_mode_property) {
> > >   		state->scaling_mode = val;
> > > +	} else if (property == config->hdmi_output_property) {
> > > +		state->hdmi_output = val;
> > >   	} else if (connector->funcs->atomic_set_property) {
> > >   		return connector->funcs->atomic_set_property(connector,
> > >   				state, property, val);
> > > @@ -1272,6 +1274,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > >   		*val = state->picture_aspect_ratio;
> > >   	} else if (property == connector->scaling_mode_property) {
> > >   		*val = state->scaling_mode;
> > > +	} else if (property == config->hdmi_output_property) {
> > > +		*val = state->hdmi_output;
> > >   	} else if (connector->funcs->atomic_get_property) {
> > >   		return connector->funcs->atomic_get_property(connector,
> > >   				state, property, val);
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 23e4661..2e7459f 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -637,6 +637,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > >   			if (old_connector_state->link_status !=
> > >   			    new_connector_state->link_status)
> > >   				new_crtc_state->connectors_changed = true;
> > > +
> > > +			if (old_connector_state->hdmi_output !=
> > > +			    new_connector_state->hdmi_output)
> > > +				new_crtc_state->connectors_changed = true;
> > >   		}
> > >   		if (funcs->atomic_check)
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 8072e6e..8357918 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev,
> > >   					      config->edid_property,
> > >   					      0);
> > > +	if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)
> > > +		drm_object_attach_property(&connector->base,
> > > +					   config->hdmi_output_property,
> > > +					   0);
> > > +
> > >   	drm_object_attach_property(&connector->base,
> > >   				      config->dpms_property, 0);
> > > @@ -617,6 +622,26 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> > >   };
> > >   DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> > > +static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {
> > > +	{ DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
> > > +	{ DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" },
> > > +	{ DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
> > > +	{ DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" },
> > > +	{ DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" },
> > > +	{ DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" },
> > > +	{ DRM_HDMI_OUTPUT_INVALID, "invalid_output" },
> > > +};
> > > +
> > > +/**
> > > + * drm_get_hdmi_output_name - return a string for a given hdmi output enum
> > > + * @type: enum of output type
> > > + */
> > > +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type)
> > > +{
> > > +	return drm_hdmi_output_enum_list[type].name;
> > > +}
> > > +EXPORT_SYMBOL(drm_get_hdmi_output_name);
> > > +
> > >   /**
> > >    * drm_display_info_set_bus_formats - set the supported bus formats
> > >    * @info: display info to store bus formats in
> > > @@ -697,7 +722,36 @@ static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = {
> > >   	{ DRM_MODE_SUBCONNECTOR_SCART,     "SCART"     }, /* TV-out */
> > >   };
> > >   DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
> > > -		 drm_tv_subconnector_enum_list)
> > > +		 drm_tv_subconnector_enum_list);
> > > +
> > > +/**
> > > + * DOC: hdmi_output_format
> > > + *
> > > + * hdmi_output_format:
> > > + *	Enum property which allows a userspace to provide its preference for a
> > > + *	HDMI output format. Standard HDMI 1.4b outputs can support RGB/YCBCR444
> > > + *	YCBCR422 output formats, whereas HDMI 2.0 outputs can additionally
> > > + *	support YCBCR420 output. Default value of the property is HDMI output
> > > + *	RGB.
> > > + *
> > > + *	A driver can attach to this property, and then handle the HDMI output
> > > + *	based on its capabilities and sink support. There are few helper
> > > + *	functions available in drm_modes.c which can help in finding the best
> > > + *	suitable output considering a sink/source/mode combination. Refer to
> > > + *	drm_modes.c:drm_display_info_hdmi_output_type()
> > > + */
> > > +int drm_connector_create_hdmi_properties(struct drm_device *dev)
> > > +{
> > > +	struct drm_property *prop;
> > > +
> > > +	prop = drm_property_create_enum(dev, 0, "hdmi_output_format",
> > > +					drm_hdmi_output_enum_list,
> > > +					ARRAY_SIZE(drm_hdmi_output_enum_list));
> > > +	if (!prop)
> > > +		return -ENOMEM;
> > > +	dev->mode_config.hdmi_output_property = prop;
> > > +	return 0;
> > > +}
> > >   /**
> > >    * DOC: standard connector properties
> > > @@ -1024,6 +1078,19 @@ int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
> > >   }
> > >   EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property);
> > > +
> > > +/**
> > > + * DOC: aspect ratio property
> > > + *
> > > + * aspect ratio:
> > > + *	Enum property to override the HDMI output's aspect ratio.
> > > + *	When this property is set, the aspect ratio of the frame in
> > > + *	AVI infoframes is set as per the property value. For example
> > > + *	if userspace sets the property value to DRM_MODE_PICTURE_ASPECT_4_3
> > > + *	the output aspect ratio is set to 4:3 (regardless of the PAR of mode)
> > Would be good to document all possible values. Same for the other
> > properties, plus insert a link to the function that creates the property
> > (where we have that).
> This is the part, which makes me feel that I should create this patch
> separately, as this seems like slightly bigger cleanup than
> I thought, so I dont want this to be a part of HDMI 2.0 series.

Writing good docs is hard work, but it's part of creating a new uapi :-)
But yeah you can split out the overall cleanup if you think that's easier
(it'll probably result in some fun small conflicts).

> > 
> > > + *
> > > + */
> > > +
> > >   /**
> > >    * drm_mode_create_aspect_ratio_property - create aspect ratio property
> > >    * @dev: DRM device
> > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> > > index d077c54..e6c4adc 100644
> > > --- a/drivers/gpu/drm/drm_crtc_internal.h
> > > +++ b/drivers/gpu/drm/drm_crtc_internal.h
> > > @@ -140,6 +140,7 @@ int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
> > >   				    struct drm_property *property,
> > >   				    uint64_t value);
> > >   int drm_connector_create_standard_properties(struct drm_device *dev);
> > > +int drm_connector_create_hdmi_properties(struct drm_device *dev);
> > >   const char *drm_get_connector_force_name(enum drm_connector_force force);
> > >   /* IOCTL */
> > > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > > index d986225..3ba9262 100644
> > > --- a/drivers/gpu/drm/drm_mode_config.c
> > > +++ b/drivers/gpu/drm/drm_mode_config.c
> > > @@ -210,6 +210,10 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> > >   	if (ret)
> > >   		return ret;
> > > +	ret = drm_connector_create_hdmi_properties(dev);
> > > +	if (ret)
> > > +		return ret;
> > We still create all the property values instead of just the ones that the
> > source hw supports. E.g. aspect ratio property is only created if needed,
> > not unconditionally on all connectors.
> > 
> > Note that users see properties through xrandr and the gui screen
> > configuration tools, adding properties that never do anything is
> > confusing, more for properties where you can select invalid values.
> If I understood this properly, you want me to create the property in I915
> layer, just before attaching it to object (similar to aspect ratio and
> others)
> something like:
> intel_attach_hdmi_output_property()
> {
>       if (!mode_config->hdmi_output_property)
>                    mode_config->hdmi_output_property = create_prop();
>         now_attach_property();
> }

Yes.

> I was assuming that once other drivers move to HDMI 2.0, they might all need
> this for YCBCR420 outputs, so it would be ok to create it generically.
> But at the same time, this also seems like a good idea.

gen8 isn't going to magically support hdmi2.0. We shouldn't advertise it
to users either. Same holds for other drivers and other features. Having a
generic/shared function to create it is good, but automically creating
something that on many drivers/platforms does nothing is not good, that
only confuses users (and userspace programmers).

Cheers, Daniel

> 
> - Shashank
> > > +
> > >   	prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> > >   					"type", drm_plane_type_enum_list,
> > >   					ARRAY_SIZE(drm_plane_type_enum_list));
> > > diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> > > index 951e834..d07de45 100644
> > > --- a/drivers/gpu/drm/i915/intel_modes.c
> > > +++ b/drivers/gpu/drm/i915/intel_modes.c
> > > @@ -104,6 +104,19 @@ static const struct drm_prop_enum_list broadcast_rgb_names[] = {
> > >   	{ INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
> > >   };
> > > +/**
> > > + * DOC: Broadcast RGB property
> > > + *
> > > + * Broadcast RGB:
> > > + *	Enum property to indicate RGB color range of a HDMI output.
> > > + *	For limited range RGB outputs, a remapping of pixel values from 0-255
> > > + *	should be remaped to a range of 16-235. When this property is set to
> > > + *	Limited 16:235 and CTM is set, the hardware will be programmed with the
> > > + *	result of the multiplication of CTM by the limited  range matrix, to
> > > + *	ensure the pixels normaly in the range 0..1.0 are remapped to the range
> > > + *	16/255..235/255.
> > > + *
> > > + */
> > >   void
> > >   intel_attach_broadcast_rgb_property(struct drm_connector *connector)
> > >   {
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index 4bc0882..3773600 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -319,6 +319,17 @@ struct drm_tv_connector_state {
> > >   	unsigned int hue;
> > >   };
> > > +/* HDMI output pixel format */
> > > +enum drm_hdmi_output_type {
> > > +	DRM_HDMI_OUTPUT_DEFAULT_RGB, /* default RGB */
> > > +	DRM_HDMI_OUTPUT_YCBCR444, /* YCBCR 444 */
> > > +	DRM_HDMI_OUTPUT_YCBCR422, /* YCBCR 422 */
> > > +	DRM_HDMI_OUTPUT_YCBCR420, /* YCBCR 420 */
> > > +	DRM_HDMI_OUTPUT_YCBCR_HQ, /* Highest subsampled YUV */
> > > +	DRM_HDMI_OUTPUT_YCBCR_LQ, /* Lowest subsampled YUV */
> > > +	DRM_HDMI_OUTPUT_INVALID, /* Guess what ? */
> > > +};
> > > +
> > >   /**
> > >    * struct drm_connector_state - mutable connector state
> > >    * @connector: backpointer to the connector
> > > @@ -363,6 +374,12 @@ struct drm_connector_state {
> > >   	 * upscaling, mostly used for built-in panels.
> > >   	 */
> > >   	unsigned int scaling_mode;
> > > +
> > > +	/**
> > > +	 * @hdmi_output: Connector property to control the
> > > +	 * HDMI output mode (RGB/YCBCR444/422/420).
> > > +	 */
> > > +	enum drm_hdmi_output_type hdmi_output;
> > >   };
> > >   /**
> > > @@ -991,6 +1008,7 @@ static inline void drm_connector_unreference(struct drm_connector *connector)
> > >   const char *drm_get_connector_status_name(enum drm_connector_status status);
> > >   const char *drm_get_subpixel_order_name(enum subpixel_order order);
> > > +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type);
> > >   const char *drm_get_dpms_name(int val);
> > >   const char *drm_get_dvi_i_subconnector_name(int val);
> > >   const char *drm_get_dvi_i_select_name(int val);
> > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > > index 4298171..1887261 100644
> > > --- a/include/drm/drm_mode_config.h
> > > +++ b/include/drm/drm_mode_config.h
> > > @@ -740,6 +740,11 @@ struct drm_mode_config {
> > >   	 * the position of the output on the host's screen.
> > >   	 */
> > >   	struct drm_property *suggested_y_property;
> > > +	/**
> > > +	 * @hdmi_output_property: output pixel format from HDMI display
> > > +	 * Default is set for RGB
> > > +	 */
> > > +	struct drm_property *hdmi_output_property;
> > >   	/* dumb ioctl parameters */
> > >   	uint32_t preferred_depth, prefer_shadow;
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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

Re: [PATCH v5 09/17] drm: create hdmi output property

[Sharma, Shashank]

Regards

Shashank


On 7/5/2017 12:01 PM, Daniel Vetter wrote:
> On Wed, Jul 05, 2017 at 11:39:30AM +0530, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 7/4/2017 9:06 PM, Daniel Vetter wrote:
>>> On Tue, Jul 04, 2017 at 07:41:56PM +0530, Shashank Sharma wrote:
>>>> HDMI displays can support various output types, based on
>>>> the color space and subsampling type. The possible
>>>> outputs from a HDMI 2.0 monitor could be:
>>>>    - RGB
>>>>    - YCBCR 444
>>>>    - YCBCR 422
>>>>    - YCBCR 420
>>>>
>>>> This patch adds a drm property "hdmi_output_format", using which,
>>>> a user can specify its preference, for the HDMI output type. The
>>>> output type enums are similar to the mentioned outputs above. To
>>>> handle various subsampling of YCBCR output types, this property
>>>> allows two special cases:
>>>>    - DRM_HDMI_OUTPUT_YCBCR_HQ
>>>>      This indicates preferred output should be YCBCR output, with highest
>>>>      subsampling rate by the source/sink, which can be typically:
>>>>    	- ycbcr444
>>>>    	- ycbcr422
>>>>    	- ycbcr420
>>>>    - DRM_HDMI_OUTPUT_YCBCR_LQ
>>>>      This indicates preferred output should be YCBCR output, with lowest
>>>>      subsampling rate supported by source/sink, which can be:
>>>>    	- ycbcr420
>>>>    	- ycbcr422
>>>>    	- ycbcr444
>>>>
>>>> Default value of the property is set to 0 = RGB, so no changes if you
>>>> dont set the property.
>>>>
>>>> PS: While doing modeset for YCBCR 420 only modes, this property is
>>>>       ignored, as those timings can be supported only in YCBCR 420
>>>>       output mode.
>>>>
>>>> V2: Added description for the new variable to address build warning
>>>> V3: Rebase
>>>> V4: Rebase
>>>> V5: Added get_property counterpart to fix IGT BAT failures (BAT/CI)
>>>>       Danvet:
>>>>       - Add documentation for the new property
>>>>       - Create a sub-section for HDMI properties, and add documentation for
>>>>         few more HDMI propeties. Added documentation for:
>>>> 	- Broadcast RGB
>>>> 	- aspect ratio
>>>>
>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>> Cc: Jose Abreu <joabreu@synopsys.com>
>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> Bunch more documentation nitpicks below. I'd personally also split up the
>>> patch into documenting the existing props and adding the new format one.
>> In fact thats what I would do now. I dont want HDMI 2.0 patch series to get
>> blocked due to documentation, as with all the comments
>> it seems like some work. Lets do it in this way:
>>   - This patch series will add the documentation for hdmi_output_property
>> - I will send a separate patch which will collectively add documentation for
>> all standard HDMI properties.
>>> Thanks, Daniel
>>>
>>>> ---
>>>>    Documentation/gpu/drm-kms.rst       | 12 +++++++
>>>>    drivers/gpu/drm/drm_atomic.c        |  4 +++
>>>>    drivers/gpu/drm/drm_atomic_helper.c |  4 +++
>>>>    drivers/gpu/drm/drm_connector.c     | 69 ++++++++++++++++++++++++++++++++++++-
>>>>    drivers/gpu/drm/drm_crtc_internal.h |  1 +
>>>>    drivers/gpu/drm/drm_mode_config.c   |  4 +++
>>>>    drivers/gpu/drm/i915/intel_modes.c  | 13 +++++++
>>>>    include/drm/drm_connector.h         | 18 ++++++++++
>>>>    include/drm/drm_mode_config.h       |  5 +++
>>>>    9 files changed, 129 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
>>>> index 3072841..dcdd6ff 100644
>>>> --- a/Documentation/gpu/drm-kms.rst
>>>> +++ b/Documentation/gpu/drm-kms.rst
>>>> @@ -508,6 +508,18 @@ Standard Connector Properties
>>>>    .. kernel-doc:: drivers/gpu/drm/drm_connector.c
>>>>       :doc: standard connector properties
>>>> +Standard HDMI Properties
>>>> +-----------------------------
>>>> +
>>>> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
>>>> +   :doc: hdmi_output_format
>>>> +
>>>> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
>>>> +   :doc: aspect ratio property
>>> I'd have just created 1 DOC: section titled "standard HDMI properties" and
>>> listed all of them in 1 comment block. People often forget to add the
>>> include stanza in the .rst files, having bigger comments helps with that.
>>>
>>> But feel free to go either way.
>> Yes, this would be easier for me too, but this means I will have to add all
>> the documentation in one place, in one file, whether the respective
>> property is created at that place or not. I am not sure if everyone would be
>> ok with that during review. But if you think we should go ahead,
>> thanks for easing this up for me :-).
> It might result in a minor merge conflict, but nothing bad should happen
> if we put all the docs into one section.
Got it, so the cleanup patch will have doc for all DRM level HDMI 
property in one place !
>>>> +.. kernel-doc:: drivers/gpu/drm/i915/intel_modes.c
>>>> +   :doc: Broadcast RGB property
>>> Please no generic properties documented in driver code.
>> Broadcast RGB property is created here, and also, while I was adding
>> documentation for it, I realized its kept in dev_priv, which is specific to
>> I915 driver, So its not generic too. I was not sure if that will be OK, if I
>> add an Intel specific property's description in DRM layer ?
> It should be generic, but oh well it isn't. Either document it in the
> core, or we'll leave this to the future when the create helper gets
> extracted to the core.
>
>>>> +
>>>>    Plane Composition Properties
>>>>    ----------------------------
>>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>>> index 09ca662..adcb89d 100644
>>>> --- a/drivers/gpu/drm/drm_atomic.c
>>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>>> @@ -1192,6 +1192,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>>>>    		state->picture_aspect_ratio = val;
>>>>    	} else if (property == connector->scaling_mode_property) {
>>>>    		state->scaling_mode = val;
>>>> +	} else if (property == config->hdmi_output_property) {
>>>> +		state->hdmi_output = val;
>>>>    	} else if (connector->funcs->atomic_set_property) {
>>>>    		return connector->funcs->atomic_set_property(connector,
>>>>    				state, property, val);
>>>> @@ -1272,6 +1274,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>>>>    		*val = state->picture_aspect_ratio;
>>>>    	} else if (property == connector->scaling_mode_property) {
>>>>    		*val = state->scaling_mode;
>>>> +	} else if (property == config->hdmi_output_property) {
>>>> +		*val = state->hdmi_output;
>>>>    	} else if (connector->funcs->atomic_get_property) {
>>>>    		return connector->funcs->atomic_get_property(connector,
>>>>    				state, property, val);
>>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>>> index 23e4661..2e7459f 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>>> @@ -637,6 +637,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>>>>    			if (old_connector_state->link_status !=
>>>>    			    new_connector_state->link_status)
>>>>    				new_crtc_state->connectors_changed = true;
>>>> +
>>>> +			if (old_connector_state->hdmi_output !=
>>>> +			    new_connector_state->hdmi_output)
>>>> +				new_crtc_state->connectors_changed = true;
>>>>    		}
>>>>    		if (funcs->atomic_check)
>>>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>>>> index 8072e6e..8357918 100644
>>>> --- a/drivers/gpu/drm/drm_connector.c
>>>> +++ b/drivers/gpu/drm/drm_connector.c
>>>> @@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev,
>>>>    					      config->edid_property,
>>>>    					      0);
>>>> +	if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)
>>>> +		drm_object_attach_property(&connector->base,
>>>> +					   config->hdmi_output_property,
>>>> +					   0);
>>>> +
>>>>    	drm_object_attach_property(&connector->base,
>>>>    				      config->dpms_property, 0);
>>>> @@ -617,6 +622,26 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>>>>    };
>>>>    DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>>>> +static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {
>>>> +	{ DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
>>>> +	{ DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" },
>>>> +	{ DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
>>>> +	{ DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" },
>>>> +	{ DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" },
>>>> +	{ DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" },
>>>> +	{ DRM_HDMI_OUTPUT_INVALID, "invalid_output" },
>>>> +};
>>>> +
>>>> +/**
>>>> + * drm_get_hdmi_output_name - return a string for a given hdmi output enum
>>>> + * @type: enum of output type
>>>> + */
>>>> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type)
>>>> +{
>>>> +	return drm_hdmi_output_enum_list[type].name;
>>>> +}
>>>> +EXPORT_SYMBOL(drm_get_hdmi_output_name);
>>>> +
>>>>    /**
>>>>     * drm_display_info_set_bus_formats - set the supported bus formats
>>>>     * @info: display info to store bus formats in
>>>> @@ -697,7 +722,36 @@ static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = {
>>>>    	{ DRM_MODE_SUBCONNECTOR_SCART,     "SCART"     }, /* TV-out */
>>>>    };
>>>>    DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
>>>> -		 drm_tv_subconnector_enum_list)
>>>> +		 drm_tv_subconnector_enum_list);
>>>> +
>>>> +/**
>>>> + * DOC: hdmi_output_format
>>>> + *
>>>> + * hdmi_output_format:
>>>> + *	Enum property which allows a userspace to provide its preference for a
>>>> + *	HDMI output format. Standard HDMI 1.4b outputs can support RGB/YCBCR444
>>>> + *	YCBCR422 output formats, whereas HDMI 2.0 outputs can additionally
>>>> + *	support YCBCR420 output. Default value of the property is HDMI output
>>>> + *	RGB.
>>>> + *
>>>> + *	A driver can attach to this property, and then handle the HDMI output
>>>> + *	based on its capabilities and sink support. There are few helper
>>>> + *	functions available in drm_modes.c which can help in finding the best
>>>> + *	suitable output considering a sink/source/mode combination. Refer to
>>>> + *	drm_modes.c:drm_display_info_hdmi_output_type()
>>>> + */
>>>> +int drm_connector_create_hdmi_properties(struct drm_device *dev)
>>>> +{
>>>> +	struct drm_property *prop;
>>>> +
>>>> +	prop = drm_property_create_enum(dev, 0, "hdmi_output_format",
>>>> +					drm_hdmi_output_enum_list,
>>>> +					ARRAY_SIZE(drm_hdmi_output_enum_list));
>>>> +	if (!prop)
>>>> +		return -ENOMEM;
>>>> +	dev->mode_config.hdmi_output_property = prop;
>>>> +	return 0;
>>>> +}
>>>>    /**
>>>>     * DOC: standard connector properties
>>>> @@ -1024,6 +1078,19 @@ int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
>>>>    }
>>>>    EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property);
>>>> +
>>>> +/**
>>>> + * DOC: aspect ratio property
>>>> + *
>>>> + * aspect ratio:
>>>> + *	Enum property to override the HDMI output's aspect ratio.
>>>> + *	When this property is set, the aspect ratio of the frame in
>>>> + *	AVI infoframes is set as per the property value. For example
>>>> + *	if userspace sets the property value to DRM_MODE_PICTURE_ASPECT_4_3
>>>> + *	the output aspect ratio is set to 4:3 (regardless of the PAR of mode)
>>> Would be good to document all possible values. Same for the other
>>> properties, plus insert a link to the function that creates the property
>>> (where we have that).
>> This is the part, which makes me feel that I should create this patch
>> separately, as this seems like slightly bigger cleanup than
>> I thought, so I dont want this to be a part of HDMI 2.0 series.
> Writing good docs is hard work, but it's part of creating a new uapi :-)
> But yeah you can split out the overall cleanup if you think that's easier
> (it'll probably result in some fun small conflicts).
Thanks.
>>>> + *
>>>> + */
>>>> +
>>>>    /**
>>>>     * drm_mode_create_aspect_ratio_property - create aspect ratio property
>>>>     * @dev: DRM device
>>>> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
>>>> index d077c54..e6c4adc 100644
>>>> --- a/drivers/gpu/drm/drm_crtc_internal.h
>>>> +++ b/drivers/gpu/drm/drm_crtc_internal.h
>>>> @@ -140,6 +140,7 @@ int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
>>>>    				    struct drm_property *property,
>>>>    				    uint64_t value);
>>>>    int drm_connector_create_standard_properties(struct drm_device *dev);
>>>> +int drm_connector_create_hdmi_properties(struct drm_device *dev);
>>>>    const char *drm_get_connector_force_name(enum drm_connector_force force);
>>>>    /* IOCTL */
>>>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>>>> index d986225..3ba9262 100644
>>>> --- a/drivers/gpu/drm/drm_mode_config.c
>>>> +++ b/drivers/gpu/drm/drm_mode_config.c
>>>> @@ -210,6 +210,10 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>>>>    	if (ret)
>>>>    		return ret;
>>>> +	ret = drm_connector_create_hdmi_properties(dev);
>>>> +	if (ret)
>>>> +		return ret;
>>> We still create all the property values instead of just the ones that the
>>> source hw supports. E.g. aspect ratio property is only created if needed,
>>> not unconditionally on all connectors.
>>>
>>> Note that users see properties through xrandr and the gui screen
>>> configuration tools, adding properties that never do anything is
>>> confusing, more for properties where you can select invalid values.
>> If I understood this properly, you want me to create the property in I915
>> layer, just before attaching it to object (similar to aspect ratio and
>> others)
>> something like:
>> intel_attach_hdmi_output_property()
>> {
>>        if (!mode_config->hdmi_output_property)
>>                     mode_config->hdmi_output_property = create_prop();
>>          now_attach_property();
>> }
> Yes.
>
>> I was assuming that once other drivers move to HDMI 2.0, they might all need
>> this for YCBCR420 outputs, so it would be ok to create it generically.
>> But at the same time, this also seems like a good idea.
> gen8 isn't going to magically support hdmi2.0. We shouldn't advertise it
> to users either. Same holds for other drivers and other features. Having a
> generic/shared function to create it is good, but automically creating
> something that on many drivers/platforms does nothing is not good, that
> only confuses users (and userspace programmers).
Actually, this property doesn't only control HDMI output as 420, but 
allows 444/422 outputs as well. This patch
series adds a whole framework for YCBCR handling, and any older GEN with 
HDMI 1.4b support can use this for
YCBCR444 output handling for example.

And again, when I said other drivers, I meant DRM drivers apart from 
I915, who are interested in YCBCR output handling using
this framework.

But still if you think, it would be good to create this on need basis, 
you are the boss :-)
Please let me know.

- Shashank
> Cheers, Daniel
>
>> - Shashank
>>>> +
>>>>    	prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
>>>>    					"type", drm_plane_type_enum_list,
>>>>    					ARRAY_SIZE(drm_plane_type_enum_list));
>>>> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
>>>> index 951e834..d07de45 100644
>>>> --- a/drivers/gpu/drm/i915/intel_modes.c
>>>> +++ b/drivers/gpu/drm/i915/intel_modes.c
>>>> @@ -104,6 +104,19 @@ static const struct drm_prop_enum_list broadcast_rgb_names[] = {
>>>>    	{ INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
>>>>    };
>>>> +/**
>>>> + * DOC: Broadcast RGB property
>>>> + *
>>>> + * Broadcast RGB:
>>>> + *	Enum property to indicate RGB color range of a HDMI output.
>>>> + *	For limited range RGB outputs, a remapping of pixel values from 0-255
>>>> + *	should be remaped to a range of 16-235. When this property is set to
>>>> + *	Limited 16:235 and CTM is set, the hardware will be programmed with the
>>>> + *	result of the multiplication of CTM by the limited  range matrix, to
>>>> + *	ensure the pixels normaly in the range 0..1.0 are remapped to the range
>>>> + *	16/255..235/255.
>>>> + *
>>>> + */
>>>>    void
>>>>    intel_attach_broadcast_rgb_property(struct drm_connector *connector)
>>>>    {
>>>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>>>> index 4bc0882..3773600 100644
>>>> --- a/include/drm/drm_connector.h
>>>> +++ b/include/drm/drm_connector.h
>>>> @@ -319,6 +319,17 @@ struct drm_tv_connector_state {
>>>>    	unsigned int hue;
>>>>    };
>>>> +/* HDMI output pixel format */
>>>> +enum drm_hdmi_output_type {
>>>> +	DRM_HDMI_OUTPUT_DEFAULT_RGB, /* default RGB */
>>>> +	DRM_HDMI_OUTPUT_YCBCR444, /* YCBCR 444 */
>>>> +	DRM_HDMI_OUTPUT_YCBCR422, /* YCBCR 422 */
>>>> +	DRM_HDMI_OUTPUT_YCBCR420, /* YCBCR 420 */
>>>> +	DRM_HDMI_OUTPUT_YCBCR_HQ, /* Highest subsampled YUV */
>>>> +	DRM_HDMI_OUTPUT_YCBCR_LQ, /* Lowest subsampled YUV */
>>>> +	DRM_HDMI_OUTPUT_INVALID, /* Guess what ? */
>>>> +};
>>>> +
>>>>    /**
>>>>     * struct drm_connector_state - mutable connector state
>>>>     * @connector: backpointer to the connector
>>>> @@ -363,6 +374,12 @@ struct drm_connector_state {
>>>>    	 * upscaling, mostly used for built-in panels.
>>>>    	 */
>>>>    	unsigned int scaling_mode;
>>>> +
>>>> +	/**
>>>> +	 * @hdmi_output: Connector property to control the
>>>> +	 * HDMI output mode (RGB/YCBCR444/422/420).
>>>> +	 */
>>>> +	enum drm_hdmi_output_type hdmi_output;
>>>>    };
>>>>    /**
>>>> @@ -991,6 +1008,7 @@ static inline void drm_connector_unreference(struct drm_connector *connector)
>>>>    const char *drm_get_connector_status_name(enum drm_connector_status status);
>>>>    const char *drm_get_subpixel_order_name(enum subpixel_order order);
>>>> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type);
>>>>    const char *drm_get_dpms_name(int val);
>>>>    const char *drm_get_dvi_i_subconnector_name(int val);
>>>>    const char *drm_get_dvi_i_select_name(int val);
>>>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>>>> index 4298171..1887261 100644
>>>> --- a/include/drm/drm_mode_config.h
>>>> +++ b/include/drm/drm_mode_config.h
>>>> @@ -740,6 +740,11 @@ struct drm_mode_config {
>>>>    	 * the position of the output on the host's screen.
>>>>    	 */
>>>>    	struct drm_property *suggested_y_property;
>>>> +	/**
>>>> +	 * @hdmi_output_property: output pixel format from HDMI display
>>>> +	 * Default is set for RGB
>>>> +	 */
>>>> +	struct drm_property *hdmi_output_property;
>>>>    	/* dumb ioctl parameters */
>>>>    	uint32_t preferred_depth, prefer_shadow;
>>>> -- 
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH v5 04/17] drm: add helper to validate ycbcr420 modes

[Ville Syrjälä]

On Wed, Jul 05, 2017 at 08:48:40AM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 7/4/2017 9:26 PM, Ville Syrjälä wrote:
> > On Tue, Jul 04, 2017 at 07:41:51PM +0530, Shashank Sharma wrote:
> >> 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.
> >>
> >> V5: Introduced the patch in series.
> >>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_edid.c         |  3 ++-
> >>   drivers/gpu/drm/drm_modes.c        | 28 ++++++++++++++++++++++++++++
> >>   drivers/gpu/drm/drm_probe_helper.c |  4 ++++
> >>   include/drm/drm_edid.h             |  1 +
> >>   include/drm/drm_modes.h            |  5 +++++
> >>   5 files changed, 40 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> index b879662..ad26c5e 100644
> >> --- a/drivers/gpu/drm/drm_edid.c
> >> +++ b/drivers/gpu/drm/drm_edid.c
> >> @@ -2947,10 +2947,11 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
> >>   }
> >>   EXPORT_SYMBOL(drm_match_cea_mode);
> >>   
> >> -static bool drm_valid_cea_vic(u8 vic)
> >> +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..3b53c8e3 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_ycbcr420_only - 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 (drm_valid_cea_vic(vic) && test_bit(vic, hdmi->y420_vdb_modes)) {
> > The drm_valid_cea_vic() check seems redundant to me.
> > Why do you think we need it?
> drm_match_cea_mode() returns 0 in case if a vic is not found. if we take 
> that to test_bit() it will check bit 0
> and give wrong results.

Why would bit 0 be set? That would sound like a bug in the mode parsing.

> So first we have to check valid vic, this is 
> also an additional check for YCBCR420 mode
> as they must have a valid vic
> > Also I don't think this will compile since we don't have
> > y420_vdb_modes[] yet.
> Ah, now I recall the reason I wanted to give you, when you asked me to 
> move this patch, before we add ycbcr420 modes.
> So now this has to go after patch 5 right ? I would re-sequence 
> accordingly.
> 
> - Shashank
> >> +		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[] = {
> >> 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_edid.h b/include/drm/drm_edid.h
> >> index 89c0062..b55b2a7 100644
> >> --- a/include/drm/drm_edid.h
> >> +++ b/include/drm/drm_edid.h
> >> @@ -477,4 +477,5 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name,
> >>   struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
> >>   					   int hsize, int vsize, int fresh,
> >>   					   bool rb);
> >> +bool drm_valid_cea_vic(u8 vic);
> >>   #endif /* __DRM_EDID_H__ */
> >> 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

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

Re: [PATCH v5 04/17] drm: add helper to validate ycbcr420 modes

[Sharma, Shashank]

Regards

Shashank


On 7/5/2017 3:46 PM, Ville Syrjälä wrote:
> On Wed, Jul 05, 2017 at 08:48:40AM +0530, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 7/4/2017 9:26 PM, Ville Syrjälä wrote:
>>> On Tue, Jul 04, 2017 at 07:41:51PM +0530, Shashank Sharma wrote:
>>>> 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.
>>>>
>>>> V5: Introduced the patch in series.
>>>>
>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/drm_edid.c         |  3 ++-
>>>>    drivers/gpu/drm/drm_modes.c        | 28 ++++++++++++++++++++++++++++
>>>>    drivers/gpu/drm/drm_probe_helper.c |  4 ++++
>>>>    include/drm/drm_edid.h             |  1 +
>>>>    include/drm/drm_modes.h            |  5 +++++
>>>>    5 files changed, 40 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>> index b879662..ad26c5e 100644
>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>> @@ -2947,10 +2947,11 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
>>>>    }
>>>>    EXPORT_SYMBOL(drm_match_cea_mode);
>>>>    
>>>> -static bool drm_valid_cea_vic(u8 vic)
>>>> +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..3b53c8e3 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_ycbcr420_only - 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 (drm_valid_cea_vic(vic) && test_bit(vic, hdmi->y420_vdb_modes)) {
>>> The drm_valid_cea_vic() check seems redundant to me.
>>> Why do you think we need it?
>> drm_match_cea_mode() returns 0 in case if a vic is not found. if we take
>> that to test_bit() it will check bit 0
>> and give wrong results.
> Why would bit 0 be set? That would sound like a bug in the mode parsing.
I know, and it wont be, but do we want to take the wrong VIC as input in 
first place ? Many detailed modes, and non-cea modes
will return 0 for VIC, why should we bother checking them in map at the 
first place ?

- Shashank
>> So first we have to check valid vic, this is
>> also an additional check for YCBCR420 mode
>> as they must have a valid vic
>>> Also I don't think this will compile since we don't have
>>> y420_vdb_modes[] yet.
>> Ah, now I recall the reason I wanted to give you, when you asked me to
>> move this patch, before we add ycbcr420 modes.
>> So now this has to go after patch 5 right ? I would re-sequence
>> accordingly.
>>
>> - Shashank
>>>> +		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[] = {
>>>> 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_edid.h b/include/drm/drm_edid.h
>>>> index 89c0062..b55b2a7 100644
>>>> --- a/include/drm/drm_edid.h
>>>> +++ b/include/drm/drm_edid.h
>>>> @@ -477,4 +477,5 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name,
>>>>    struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
>>>>    					   int hsize, int vsize, int fresh,
>>>>    					   bool rb);
>>>> +bool drm_valid_cea_vic(u8 vic);
>>>>    #endif /* __DRM_EDID_H__ */
>>>> 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

Re: [PATCH v5 04/17] drm: add helper to validate ycbcr420 modes

[Ville Syrjälä]

On Wed, Jul 05, 2017 at 03:49:51PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 7/5/2017 3:46 PM, Ville Syrjälä wrote:
> > On Wed, Jul 05, 2017 at 08:48:40AM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 7/4/2017 9:26 PM, Ville Syrjälä wrote:
> >>> On Tue, Jul 04, 2017 at 07:41:51PM +0530, Shashank Sharma wrote:
> >>>> 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.
> >>>>
> >>>> V5: Introduced the patch in series.
> >>>>
> >>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>> ---
> >>>>    drivers/gpu/drm/drm_edid.c         |  3 ++-
> >>>>    drivers/gpu/drm/drm_modes.c        | 28 ++++++++++++++++++++++++++++
> >>>>    drivers/gpu/drm/drm_probe_helper.c |  4 ++++
> >>>>    include/drm/drm_edid.h             |  1 +
> >>>>    include/drm/drm_modes.h            |  5 +++++
> >>>>    5 files changed, 40 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>>> index b879662..ad26c5e 100644
> >>>> --- a/drivers/gpu/drm/drm_edid.c
> >>>> +++ b/drivers/gpu/drm/drm_edid.c
> >>>> @@ -2947,10 +2947,11 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
> >>>>    }
> >>>>    EXPORT_SYMBOL(drm_match_cea_mode);
> >>>>    
> >>>> -static bool drm_valid_cea_vic(u8 vic)
> >>>> +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..3b53c8e3 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_ycbcr420_only - 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 (drm_valid_cea_vic(vic) && test_bit(vic, hdmi->y420_vdb_modes)) {
> >>> The drm_valid_cea_vic() check seems redundant to me.
> >>> Why do you think we need it?
> >> drm_match_cea_mode() returns 0 in case if a vic is not found. if we take
> >> that to test_bit() it will check bit 0
> >> and give wrong results.
> > Why would bit 0 be set? That would sound like a bug in the mode parsing.
> I know, and it wont be, but do we want to take the wrong VIC as input in 
> first place ? Many detailed modes, and non-cea modes
> will return 0 for VIC,

All non-cea modes will return 0.

> why should we bother checking them in map at the 
> first place ?

Checking the vic against some range is pointless work. Just
checking the bit blindly is the most optimal way.

> 
> - Shashank
> >> So first we have to check valid vic, this is
> >> also an additional check for YCBCR420 mode
> >> as they must have a valid vic
> >>> Also I don't think this will compile since we don't have
> >>> y420_vdb_modes[] yet.
> >> Ah, now I recall the reason I wanted to give you, when you asked me to
> >> move this patch, before we add ycbcr420 modes.
> >> So now this has to go after patch 5 right ? I would re-sequence
> >> accordingly.
> >>
> >> - Shashank
> >>>> +		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[] = {
> >>>> 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_edid.h b/include/drm/drm_edid.h
> >>>> index 89c0062..b55b2a7 100644
> >>>> --- a/include/drm/drm_edid.h
> >>>> +++ b/include/drm/drm_edid.h
> >>>> @@ -477,4 +477,5 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name,
> >>>>    struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
> >>>>    					   int hsize, int vsize, int fresh,
> >>>>    					   bool rb);
> >>>> +bool drm_valid_cea_vic(u8 vic);
> >>>>    #endif /* __DRM_EDID_H__ */
> >>>> 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

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