[PATCH v2 0/6] drm/i915/dp: Support for DP HDR outputs

[PATCH v2 0/6] drm/i915/dp: Support for DP HDR outputs

[Gwan-gyeong Mun]

Support for HDR10 video was introduced in DisplayPort 1.4.
On GLK+ platform, in order to use DisplayPort HDR10, we need to support
BT.2020 colorimetry and HDR Static metadata.
It implements the CTA-861-G standard for transport of static HDR metadata.
It enables writing of HDR metadata infoframe SDP to the panel.
The HDR Metadata will be provided by userspace compositors, based on
blending policies and passed to the driver through a blob property.
And It refactors, renames and extends a function which handled vsc sdp
header and data block setup for supporting colorimetry format.
And It attaches the colorspace connector property and HDR metadata property
to a DisplayPort connector.

These patches tested on below test environment.
Test Environment:
 - Tested System: GLK and Gen11 platform.
 - Monitor: Dell UP2718Q 4K HDR Monitor.
 - In order to test DP HDR10, test environment uses patched Kodi-gbm,
   patched Media driver and HDR10 video.

   You can find these on below.
   [patched Kodi-gbm]
    - repo: https://github.com/Kwiboo/xbmc/tree/drmprime-hdr 
   [download 4K HDR video file]
    - link: https://4kmedia.org/lg-new-york-hdr-uhd-4k-demo/
   [Media Driver for GLK]
    - repo https://gitlab.freedesktop.org/emersion/intel-vaapi-driver
  	  master branch
   [Media Driver for ICL]
    - repo: https://github.com/harishkrupo/media-driver/tree/p010_composite

v2:
 - Add a missed blank line after function declaration.
 - Remove useless parentheses.
 - Minor style fix.

Gwan-gyeong Mun (6):
  drm/i915/dp: Extend program of VSC Header and DB for Colorimetry
    Format
  drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA
  drm: Add DisplayPort colorspace property
  drm/i915/dp: Attach colorspace property
  drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static
    Metadata
  drm/i915/dp: Attach HDR metadata property to DP connector

 drivers/gpu/drm/drm_connector.c               |   4 +-
 drivers/gpu/drm/i915/display/intel_ddi.c      |  11 +-
 drivers/gpu/drm/i915/display/intel_display.h  |   2 -
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 191 ++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_dp.h       |   7 +
 drivers/gpu/drm/i915/i915_reg.h               |   1 +
 7 files changed, 199 insertions(+), 20 deletions(-)

-- 
2.22.0

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

[PATCH v2 1/6] drm/i915/dp: Extend program of VSC Header and DB for Colorimetry Format

[Gwan-gyeong Mun]

It refactors and renames a function which handled vsc sdp header and data
block setup for supporting colorimetry format.
Function intel_dp_setup_vsc_sdp handles vsc sdp header and data block
setup for pixel encoding / colorimetry format.
In order to use colorspace information of a connector, it adds an argument
of drm_connector_state type.

Setup VSC header and data block in function intel_dp_setup_vsc_sdp for
pixel encoding / colorimetry format as per dp 1.4a spec, section 2.2.5.7.1,
table 2-119: VSC SDP Header Bytes, section 2.2.5.7.5,
table 2-120: VSC SDP Payload for DB16 through DB18.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/display/intel_display.h |  2 -
 drivers/gpu/drm/i915/display/intel_dp.c      | 68 ++++++++++++++++----
 drivers/gpu/drm/i915/display/intel_dp.h      |  3 +
 4 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 8eb2b3ec01ed..4f7ea0a35976 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3475,7 +3475,7 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
 
 	intel_edp_backlight_on(crtc_state, conn_state);
 	intel_psr_enable(intel_dp, crtc_state);
-	intel_dp_ycbcr_420_enable(intel_dp, crtc_state);
+	intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
 	intel_edp_drrs_enable(intel_dp, crtc_state);
 
 	if (crtc_state->has_audio)
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index e57e6969051d..7bd59241fc32 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -499,8 +499,6 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
 		      struct intel_crtc_state *pipe_config);
 void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
 		      enum link_m_n_set m_n);
-void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp,
-			       const struct intel_crtc_state *crtc_state);
 int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
 bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state,
 			struct dpll *best_clock);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 5c45a3bb102d..55d5ab97061c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4409,8 +4409,9 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
 }
 
 static void
-intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
-			       const struct intel_crtc_state *crtc_state)
+intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
+		       const struct intel_crtc_state *crtc_state,
+		       const struct drm_connector_state *conn_state)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct dp_sdp vsc_sdp = {};
@@ -4431,13 +4432,55 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
 	 */
 	vsc_sdp.sdp_header.HB3 = 0x13;
 
-	/*
-	 * YCbCr 420 = 3h DB16[7:4] ITU-R BT.601 = 0h, ITU-R BT.709 = 1h
-	 * DB16[3:0] DP 1.4a spec, Table 2-120
-	 */
-	vsc_sdp.db[16] = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/
-	/* RGB->YCBCR color conversion uses the BT.709 color space. */
-	vsc_sdp.db[16] |= 0x1; /* 0x1, ITU-R BT.709 */
+	/* DP 1.4a spec, Table 2-120 */
+	switch (crtc_state->output_format) {
+	case INTEL_OUTPUT_FORMAT_YCBCR444:
+		vsc_sdp.db[16] = 0x1 << 4; /* YCbCr 444 : DB16[7:4] = 1h */
+		break;
+	case INTEL_OUTPUT_FORMAT_YCBCR420:
+		vsc_sdp.db[16] = 0x3 << 4; /* YCbCr 420 : DB16[7:4] = 3h */
+		break;
+	case INTEL_OUTPUT_FORMAT_RGB:
+	default:
+		/* RGB: DB16[7:4] = 0h */
+		break;
+	}
+
+	switch (conn_state->colorspace) {
+	case DRM_MODE_COLORIMETRY_BT709_YCC:
+		vsc_sdp.db[16] |= 0x1;
+		break;
+	case DRM_MODE_COLORIMETRY_XVYCC_601:
+		vsc_sdp.db[16] |= 0x2;
+		break;
+	case DRM_MODE_COLORIMETRY_XVYCC_709:
+		vsc_sdp.db[16] |= 0x3;
+		break;
+	case DRM_MODE_COLORIMETRY_SYCC_601:
+		vsc_sdp.db[16] |= 0x4;
+		break;
+	case DRM_MODE_COLORIMETRY_OPYCC_601:
+		vsc_sdp.db[16] |= 0x5;
+		break;
+	case DRM_MODE_COLORIMETRY_BT2020_CYCC:
+	case DRM_MODE_COLORIMETRY_BT2020_RGB:
+		vsc_sdp.db[16] |= 0x6;
+		break;
+	case DRM_MODE_COLORIMETRY_BT2020_YCC:
+		vsc_sdp.db[16] |= 0x7;
+		break;
+	case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
+	case DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
+		vsc_sdp.db[16] |= 0x4; /* DCI-P3 (SMPTE RP 431-2) */
+		break;
+	default:
+		/* sRGB (IEC 61966-2-1) / ITU-R BT.601: DB16[0:3] = 0h */
+
+		/* RGB->YCBCR color conversion uses the BT.709 color space. */
+		if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+			vsc_sdp.db[16] |= 0x1; /* 0x1, ITU-R BT.709 */
+		break;
+	}
 
 	/*
 	 * For pixel encoding formats YCbCr444, YCbCr422, YCbCr420, and Y Only,
@@ -4489,13 +4532,14 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
 			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));
 }
 
-void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp,
-			       const struct intel_crtc_state *crtc_state)
+void intel_dp_vsc_enable(struct intel_dp *intel_dp,
+			 const struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state)
 {
 	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
 		return;
 
-	intel_pixel_encoding_setup_vsc(intel_dp, crtc_state);
+	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);
 }
 
 static u8 intel_dp_autotest_link_training(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 657bbb1f5ed0..91a0ee6058fe 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -111,6 +111,9 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
 bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
 int intel_dp_link_required(int pixel_clock, int bpp);
 int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
+void intel_dp_vsc_enable(struct intel_dp *intel_dp,
+			 const struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state);
 bool intel_digital_port_connected(struct intel_encoder *encoder);
 
 static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
-- 
2.22.0

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

[PATCH v2 2/6] drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA

[Gwan-gyeong Mun]

When BT.2020 Colorimetry output is used for DP, we should program BT.2020
Colorimetry to MSA and VSC SDP. It adds output_colorspace to
intel_crtc_state struct as a place holder of pipe's output colorspace.
In order to distinguish needed colorimetry for VSC SDP, it adds
intel_dp_needs_vsc_colorimetry function.
If the output colorspace requires vsc sdp or output format is YCbCr 4:2:0,
it uses MSA with VSC SDP.

As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication of
Color Encoding Format and Content Color Gamut] while sending
BT.2020 Colorimetry signals we should program MSA MISC1 fields which
indicate VSC SDP for the Pixel Encoding/Colorimetry Format.

v2: Remove useless parentheses

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      |  8 +++---
 .../drm/i915/display/intel_display_types.h    |  3 +++
 drivers/gpu/drm/i915/display/intel_dp.c       | 25 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_dp.h       |  1 +
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 4f7ea0a35976..5c42b58c1c2f 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1737,11 +1737,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
 	/*
 	 * As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication
 	 * of Color Encoding Format and Content Color Gamut] while sending
-	 * YCBCR 420 signals we should program MSA MISC1 fields which
-	 * indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
+	 * YCBCR 420, HDR BT.2020 signals we should program MSA MISC1 fields
+	 * which indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
 	 */
-	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+	    intel_dp_needs_vsc_colorimetry(crtc_state->output_colorspace))
 		temp |= TRANS_MSA_USE_VSC_SDP;
+
 	I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 449abaea619f..9845abcf6f29 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -964,6 +964,9 @@ struct intel_crtc_state {
 	/* Output format RGB/YCBCR etc */
 	enum intel_output_format output_format;
 
+	/* Output colorspace sRGB/BT.2020 etc */
+	u32 output_colorspace;
+
 	/* Output down scaling is done in LSPCON device */
 	bool lspcon_downsampling;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 55d5ab97061c..295d5ed2be96 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2164,6 +2164,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		pipe_config->has_pch_encoder = true;
 
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
+	pipe_config->output_colorspace = intel_conn_state->base.colorspace;
+
 	if (lspcon->active)
 		lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
 	else
@@ -4408,6 +4410,26 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
 	return 0;
 }
 
+bool
+intel_dp_needs_vsc_colorimetry(u32 colorspace)
+{
+	bool ret = false;
+
+	switch (colorspace) {
+	case DRM_MODE_COLORIMETRY_SYCC_601:
+	case DRM_MODE_COLORIMETRY_OPYCC_601:
+	case DRM_MODE_COLORIMETRY_BT2020_YCC:
+	case DRM_MODE_COLORIMETRY_BT2020_RGB:
+	case DRM_MODE_COLORIMETRY_BT2020_CYCC:
+		ret = true;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
 static void
 intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
 		       const struct intel_crtc_state *crtc_state,
@@ -4536,7 +4558,8 @@ void intel_dp_vsc_enable(struct intel_dp *intel_dp,
 			 const struct intel_crtc_state *crtc_state,
 			 const struct drm_connector_state *conn_state)
 {
-	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
+	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
+	    !intel_dp_needs_vsc_colorimetry(conn_state->colorspace))
 		return;
 
 	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 91a0ee6058fe..b2da7c9998f7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -111,6 +111,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
 bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
 int intel_dp_link_required(int pixel_clock, int bpp);
 int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
+bool intel_dp_needs_vsc_colorimetry(u32 colorspace);
 void intel_dp_vsc_enable(struct intel_dp *intel_dp,
 			 const struct intel_crtc_state *crtc_state,
 			 const struct drm_connector_state *conn_state);
-- 
2.22.0

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

[PATCH v2 3/6] drm: Add DisplayPort colorspace property

[Gwan-gyeong Mun]

In order to use colorspace property to Display Port connectors, it extends
DRM_MODE_CONNECTOR_DisplayPort connector_type on
drm_mode_create_colorspace_property function.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/drm_connector.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4c766624b20d..655ada9d4c16 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1703,7 +1703,9 @@ int drm_mode_create_colorspace_property(struct drm_connector *connector)
 	struct drm_property *prop;
 
 	if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
-	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
+	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB ||
+	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
 		prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
 						"Colorspace",
 						hdmi_colorspaces,
-- 
2.22.0

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

[PATCH v2 4/6] drm/i915/dp: Attach colorspace property

[Gwan-gyeong Mun]

It attaches the colorspace connector property to a DisplayPort connector.
Based on colorspace change, modeset will be triggered to switch to a new
colorspace.

Based on colorspace property value create a VSC SDP packet with appropriate
colorspace. This would help to enable wider color gamut like BT2020 on a
sink device.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 295d5ed2be96..7218e159f55d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6402,6 +6402,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	else if (INTEL_GEN(dev_priv) >= 5)
 		drm_connector_attach_max_bpc_property(connector, 6, 12);
 
+	intel_attach_colorspace_property(connector);
+
 	if (intel_dp_is_edp(intel_dp)) {
 		u32 allowed_scalers;
 
-- 
2.22.0

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

[PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata

[Gwan-gyeong Mun]

Function intel_dp_setup_hdr_metadata_infoframe_sdp handles Infoframe SDP
header and data block setup for HDR Static Metadata. It enables writing of
HDR metadata infoframe SDP to panel. Support for HDR video was introduced
in DisplayPort 1.4. It implements the CTA-861-G standard for transport of
static HDR metadata. The HDR Metadata will be provided by userspace
compositors, based on blending policies and passed to the driver through
a blob property.

Because each of GEN11 and prior GEN11 have different register size for
HDR Metadata Infoframe SDP packet, it adds and uses different register
size.

Setup Infoframe SDP header and data block in function
intel_dp_setup_hdr_metadata_infoframe_sdp for HDR Static Metadata as per
dp 1.4 spec and CTA-861-F spec.
As per DP 1.4 spec, 2.2.2.5 SDP Formats. It enables Dynamic Range and
Mastering Infoframe for HDR content, which is defined in CTA-861-F spec.
According to DP 1.4 spec and CEA-861-F spec Table 5, in order to transmit
static HDR metadata, we have to use Non-audio INFOFRAME SDP v1.3.

+--------------------------------+-------------------------------+
|      [ Packet Type Value ]     |       [ Packet Type ]         |
+--------------------------------+-------------------------------+
| 80h + Non-audio INFOFRAME Type | CEA-861-F Non-audio INFOFRAME |
+--------------------------------+-------------------------------+
|      [Transmission Timing]                                     |
+----------------------------------------------------------------+
| As per CEA-861-F for INFOFRAME, including CEA-861.3 within     |
| which Dynamic Range and Mastering INFOFRAME are defined        |
+----------------------------------------------------------------+

v2: Add a missed blank line after function declaration.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c |  1 +
 drivers/gpu/drm/i915/display/intel_dp.c  | 91 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.h  |  3 +
 drivers/gpu/drm/i915/i915_reg.h          |  1 +
 4 files changed, 96 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 5c42b58c1c2f..9f5bea941bcd 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3478,6 +3478,7 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
 	intel_edp_backlight_on(crtc_state, conn_state);
 	intel_psr_enable(intel_dp, crtc_state);
 	intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
+	intel_dp_hdr_metadata_enable(intel_dp, crtc_state, conn_state);
 	intel_edp_drrs_enable(intel_dp, crtc_state);
 
 	if (crtc_state->has_audio)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7218e159f55d..00da8221eaba 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4554,6 +4554,85 @@ intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
 			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));
 }
 
+static int
+intel_dp_setup_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
+					  const struct intel_crtc_state *crtc_state,
+					  const struct drm_connector_state *conn_state)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+	struct dp_sdp infoframe_sdp = {};
+	struct hdmi_drm_infoframe drm_infoframe = {};
+	const int infoframe_size = HDMI_INFOFRAME_HEADER_SIZE + HDMI_DRM_INFOFRAME_SIZE;
+	unsigned char buf[HDMI_INFOFRAME_HEADER_SIZE + HDMI_DRM_INFOFRAME_SIZE];
+	ssize_t len;
+	int ret;
+
+	ret = drm_hdmi_infoframe_set_hdr_metadata(&drm_infoframe, conn_state);
+	if (ret) {
+		DRM_DEBUG_KMS("couldn't set HDR metadata in infoframe\n");
+		return ret;
+	}
+
+	len = hdmi_drm_infoframe_pack_only(&drm_infoframe, buf, sizeof(buf));
+	if (len < 0) {
+		DRM_DEBUG_KMS("buffer size is smaller than hdr metadata infoframe\n");
+		return (int)len;
+	}
+
+	if (len != infoframe_size) {
+		DRM_DEBUG_KMS("wrong static hdr metadata size\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Set up the infoframe sdp packet for HDR static metadata.
+	 * Prepare VSC Header for SU as per DP 1.4a spec,
+	 * Table 2-100 and Table 2-101
+	 */
+
+	/* Packet ID, 00h for non-Audio INFOFRAME */
+	infoframe_sdp.sdp_header.HB0 = 0;
+	/*
+	 * Packet Type 80h + Non-audio INFOFRAME Type value
+	 * HDMI_INFOFRAME_TYPE_DRM: 0x87,
+	 */
+	infoframe_sdp.sdp_header.HB1 = drm_infoframe.type;
+	/*
+	 * Least Significant Eight Bits of (Data Byte Count – 1)
+	 * infoframe_size - 1,
+	 */
+	infoframe_sdp.sdp_header.HB2 = 0x1D;
+	/* INFOFRAME SDP Version Number */
+	infoframe_sdp.sdp_header.HB3 = (0x13 << 2);
+	/* CTA Header Byte 2 (INFOFRAME Version Number) */
+	infoframe_sdp.db[0] = drm_infoframe.version;
+	/* CTA Header Byte 3 (Length of INFOFRAME): HDMI_DRM_INFOFRAME_SIZE */
+	infoframe_sdp.db[1] = drm_infoframe.length;
+	/*
+	 * Copy HDMI_DRM_INFOFRAME_SIZE size from a buffer after
+	 * HDMI_INFOFRAME_HEADER_SIZE
+	 */
+	memcpy(&infoframe_sdp.db[2], &buf[HDMI_INFOFRAME_HEADER_SIZE],
+	       HDMI_DRM_INFOFRAME_SIZE);
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		intel_dig_port->write_infoframe(&intel_dig_port->base,
+						crtc_state,
+						HDMI_PACKET_TYPE_GAMUT_METADATA,
+						&infoframe_sdp,
+						VIDEO_DIP_GMP_DATA_SIZE);
+	else
+		/* Prior to GEN11, Header size: 4 bytes, Data size: 28 bytes */
+		intel_dig_port->write_infoframe(&intel_dig_port->base,
+						crtc_state,
+						HDMI_PACKET_TYPE_GAMUT_METADATA,
+						&infoframe_sdp,
+						VIDEO_DIP_DATA_SIZE);
+
+	return 0;
+}
+
 void intel_dp_vsc_enable(struct intel_dp *intel_dp,
 			 const struct intel_crtc_state *crtc_state,
 			 const struct drm_connector_state *conn_state)
@@ -4565,6 +4644,18 @@ void intel_dp_vsc_enable(struct intel_dp *intel_dp,
 	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);
 }
 
+void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
+				  const struct intel_crtc_state *crtc_state,
+				  const struct drm_connector_state *conn_state)
+{
+	if (!conn_state->hdr_output_metadata)
+		return;
+
+	intel_dp_setup_hdr_metadata_infoframe_sdp(intel_dp,
+						  crtc_state,
+						  conn_state);
+}
+
 static u8 intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 {
 	int status = 0;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index b2da7c9998f7..c3593691dd38 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -115,6 +115,9 @@ bool intel_dp_needs_vsc_colorimetry(u32 colorspace);
 void intel_dp_vsc_enable(struct intel_dp *intel_dp,
 			 const struct intel_crtc_state *crtc_state,
 			 const struct drm_connector_state *conn_state);
+void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
+				  const struct intel_crtc_state *crtc_state,
+				  const struct drm_connector_state *conn_state);
 bool intel_digital_port_connected(struct intel_encoder *encoder);
 
 static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ea2f0fa2402d..92885416d539 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4645,6 +4645,7 @@ enum {
  * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds to each byte
  * of the infoframe structure specified by CEA-861. */
 #define   VIDEO_DIP_DATA_SIZE	32
+#define   VIDEO_DIP_GMP_DATA_SIZE	36
 #define   VIDEO_DIP_VSC_DATA_SIZE	36
 #define   VIDEO_DIP_PPS_DATA_SIZE	132
 #define VIDEO_DIP_CTL		_MMIO(0x61170)
-- 
2.22.0

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

[PATCH v2 6/6] drm/i915/dp: Attach HDR metadata property to DP connector

[Gwan-gyeong Mun]

It attaches HDR metadata property to DP connector on GLK+.
It enables HDR metadata infoframe sdp on GLK+ to be used to send
HDR metadata to DP sink.

v2: Minor style fix

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 00da8221eaba..899923ecd1c9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6495,6 +6495,11 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 
 	intel_attach_colorspace_property(connector);
 
+	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11)
+		drm_object_attach_property(&connector->base,
+					   connector->dev->mode_config.hdr_output_metadata_property,
+					   0);
+
 	if (intel_dp_is_edp(intel_dp)) {
 		u32 allowed_scalers;
 
-- 
2.22.0

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

✓ Fi.CI.BAT: success for drm/i915/dp: Support for DP HDR outputs (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: Support for DP HDR outputs (rev2)
URL   : https://patchwork.freedesktop.org/series/65656/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6777 -> Patchwork_14165
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/

Known issues
------------

  Here are the changes found in Patchwork_14165 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@busy-all:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/fi-icl-u3/igt@gem_busy@busy-all.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/fi-icl-u3/igt@gem_busy@busy-all.html

  
#### Possible fixes ####

  * igt@gem_ctx_switch@legacy-render:
    - fi-bxt-dsi:         [INCOMPLETE][3] ([fdo#103927] / [fdo#111381]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-hsw-4770r:       [DMESG-WARN][5] ([fdo#107732]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/fi-hsw-4770r/igt@i915_module_load@reload-with-fault-injection.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/fi-hsw-4770r/igt@i915_module_load@reload-with-fault-injection.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107732]: https://bugs.freedesktop.org/show_bug.cgi?id=107732
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407


Participating hosts (53 -> 47)
------------------------------

  Additional (2): fi-icl-u2 fi-gdg-551 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6777 -> Patchwork_14165

  CI-20190529: 20190529
  CI_DRM_6777: f3035d74f2d44bab3dbc6673f6660b447cbefd54 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5148: 50390dd7adaccae21cafa85b866c17606cec94c3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14165: 5ff56bdecdaa49b183ea7772ca57aa58534aad6b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5ff56bdecdaa drm/i915/dp: Attach HDR metadata property to DP connector
24f7c1d6e601 drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata
27431f30d2a4 drm/i915/dp: Attach colorspace property
7026bc6acdb2 drm: Add DisplayPort colorspace property
7c7a93da4d89 drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA
00ce2a101d47 drm/i915/dp: Extend program of VSC Header and DB for Colorimetry Format

== Logs ==

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

✗ Fi.CI.IGT: failure for drm/i915/dp: Support for DP HDR outputs (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: Support for DP HDR outputs (rev2)
URL   : https://patchwork.freedesktop.org/series/65656/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6777_full -> Patchwork_14165_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14165_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14165_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14165_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rps@reset:
    - shard-iclb:         [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb2/igt@i915_pm_rps@reset.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb7/igt@i915_pm_rps@reset.html

  
Known issues
------------

  Here are the changes found in Patchwork_14165_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#111325]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb3/igt@gem_exec_schedule@in-order-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb1/igt@gem_exec_schedule@in-order-bsd.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][5] -> [FAIL][6] ([fdo#103167])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt:
    - shard-skl:          [PASS][7] -> [FAIL][8] ([fdo#108040])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-skl3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-skl10/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-wc:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#103167])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-skl3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-wc.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-skl10/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-wc.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][11] -> [DMESG-WARN][12] ([fdo#108566]) +5 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#108145] / [fdo#110403])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-skl:          [PASS][15] -> [DMESG-WARN][16] ([fdo#106885])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-skl7/igt@kms_plane_multiple@atomic-pipe-c-tiling-yf.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-skl5/igt@kms_plane_multiple@atomic-pipe-c-tiling-yf.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109441])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb1/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][19] -> [FAIL][20] ([fdo#99912])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-apl8/igt@kms_setmode@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-apl1/igt@kms_setmode@basic.html

  * igt@prime_busy@after-bsd2:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109276]) +13 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb1/igt@prime_busy@after-bsd2.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb8/igt@prime_busy@after-bsd2.html

  
#### Possible fixes ####

  * igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd:
    - shard-iclb:         [SKIP][23] ([fdo#111325]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb4/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb8/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd.html

  * igt@gem_exec_schedule@preempt-queue-contexts-render:
    - shard-apl:          [INCOMPLETE][25] ([fdo#103927]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-apl2/igt@gem_exec_schedule@preempt-queue-contexts-render.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-apl1/igt@gem_exec_schedule@preempt-queue-contexts-render.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [DMESG-WARN][27] ([fdo#108566]) -> [PASS][28] +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-apl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-apl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@2x-flip-vs-suspend:
    - shard-hsw:          [INCOMPLETE][29] ([fdo#103540]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-hsw2/igt@kms_flip@2x-flip-vs-suspend.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-hsw1/igt@kms_flip@2x-flip-vs-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][31] ([fdo#105363]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-skl2/igt@kms_flip@flip-vs-expired-vblank.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-skl3/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         [FAIL][33] ([fdo#103167]) -> [PASS][34] +3 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-kbl:          [FAIL][35] ([fdo#103375]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-skl:          [DMESG-WARN][37] ([fdo#106885]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-skl2/igt@kms_plane_multiple@atomic-pipe-a-tiling-yf.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-skl3/igt@kms_plane_multiple@atomic-pipe-a-tiling-yf.html

  * igt@kms_psr@psr2_primary_render:
    - shard-iclb:         [SKIP][39] ([fdo#109441]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb5/igt@kms_psr@psr2_primary_render.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb2/igt@kms_psr@psr2_primary_render.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][41] ([fdo#99912]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-kbl2/igt@kms_setmode@basic.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-kbl7/igt@kms_setmode@basic.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][43] ([fdo#109276]) -> [PASS][44] +11 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb6/igt@prime_busy@hang-bsd2.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb1/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][45] ([fdo#111329]) -> [SKIP][46] ([fdo#109276])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb4/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb6/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][47] ([fdo#111330]) -> [SKIP][48] ([fdo#109276])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-iclb4/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-iclb8/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc:
    - shard-skl:          [FAIL][49] ([fdo#103167]) -> [FAIL][50] ([fdo#108040])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6777/shard-skl7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14165/shard-skl5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111472]: https://bugs.freedesktop.org/show_bug.cgi?id=111472
  [fdo#111473 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111473 
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (8 -> 9)
------------------------------

  Additional (1): pig-hsw-4770r 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6777 -> Patchwork_14165

  CI-20190529: 20190529
  CI_DRM_6777: f3035d74f2d44bab3dbc6673f6660b447cbefd54 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5148: 50390dd7adaccae21cafa85b866c17606cec94c3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14165: 5ff56bdecdaa49b183ea7772ca57aa58534aad6b @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH v2 1/6] drm/i915/dp: Extend program of VSC Header and DB for Colorimetry Format

[Shankar, Uma]

>-----Original Message-----
>From: Mun, Gwan-gyeong
>Sent: Friday, August 23, 2019 3:22 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>
>Subject: [PATCH v2 1/6] drm/i915/dp: Extend program of VSC Header and DB for
>Colorimetry Format
>
>It refactors and renames a function which handled vsc sdp header and data block
>setup for supporting colorimetry format.
>Function intel_dp_setup_vsc_sdp handles vsc sdp header and data block setup for
>pixel encoding / colorimetry format.
>In order to use colorspace information of a connector, it adds an argument of
>drm_connector_state type.
>
>Setup VSC header and data block in function intel_dp_setup_vsc_sdp for pixel
>encoding / colorimetry format as per dp 1.4a spec, section 2.2.5.7.1, table 2-119: VSC
>SDP Header Bytes, section 2.2.5.7.5, table 2-120: VSC SDP Payload for DB16 through
>DB18.

The changes look good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_ddi.c     |  2 +-
> drivers/gpu/drm/i915/display/intel_display.h |  2 -
> drivers/gpu/drm/i915/display/intel_dp.c      | 68 ++++++++++++++++----
> drivers/gpu/drm/i915/display/intel_dp.h      |  3 +
> 4 files changed, 60 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>b/drivers/gpu/drm/i915/display/intel_ddi.c
>index 8eb2b3ec01ed..4f7ea0a35976 100644
>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>@@ -3475,7 +3475,7 @@ static void intel_enable_ddi_dp(struct intel_encoder
>*encoder,
>
> 	intel_edp_backlight_on(crtc_state, conn_state);
> 	intel_psr_enable(intel_dp, crtc_state);
>-	intel_dp_ycbcr_420_enable(intel_dp, crtc_state);
>+	intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
> 	intel_edp_drrs_enable(intel_dp, crtc_state);
>
> 	if (crtc_state->has_audio)
>diff --git a/drivers/gpu/drm/i915/display/intel_display.h
>b/drivers/gpu/drm/i915/display/intel_display.h
>index e57e6969051d..7bd59241fc32 100644
>--- a/drivers/gpu/drm/i915/display/intel_display.h
>+++ b/drivers/gpu/drm/i915/display/intel_display.h
>@@ -499,8 +499,6 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
> 		      struct intel_crtc_state *pipe_config);  void
>intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> 		      enum link_m_n_set m_n);
>-void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp,
>-			       const struct intel_crtc_state *crtc_state);
> int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);  bool
>bxt_find_best_dpll(struct intel_crtc_state *crtc_state,
> 			struct dpll *best_clock);
>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>b/drivers/gpu/drm/i915/display/intel_dp.c
>index 5c45a3bb102d..55d5ab97061c 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.c
>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
>@@ -4409,8 +4409,9 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>}
>
> static void
>-intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
>-			       const struct intel_crtc_state *crtc_state)
>+intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
>+		       const struct intel_crtc_state *crtc_state,
>+		       const struct drm_connector_state *conn_state)
> {
> 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> 	struct dp_sdp vsc_sdp = {};
>@@ -4431,13 +4432,55 @@ intel_pixel_encoding_setup_vsc(struct intel_dp
>*intel_dp,
> 	 */
> 	vsc_sdp.sdp_header.HB3 = 0x13;
>
>-	/*
>-	 * YCbCr 420 = 3h DB16[7:4] ITU-R BT.601 = 0h, ITU-R BT.709 = 1h
>-	 * DB16[3:0] DP 1.4a spec, Table 2-120
>-	 */
>-	vsc_sdp.db[16] = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/
>-	/* RGB->YCBCR color conversion uses the BT.709 color space. */
>-	vsc_sdp.db[16] |= 0x1; /* 0x1, ITU-R BT.709 */
>+	/* DP 1.4a spec, Table 2-120 */
>+	switch (crtc_state->output_format) {
>+	case INTEL_OUTPUT_FORMAT_YCBCR444:
>+		vsc_sdp.db[16] = 0x1 << 4; /* YCbCr 444 : DB16[7:4] = 1h */
>+		break;
>+	case INTEL_OUTPUT_FORMAT_YCBCR420:
>+		vsc_sdp.db[16] = 0x3 << 4; /* YCbCr 420 : DB16[7:4] = 3h */
>+		break;
>+	case INTEL_OUTPUT_FORMAT_RGB:
>+	default:
>+		/* RGB: DB16[7:4] = 0h */
>+		break;
>+	}
>+
>+	switch (conn_state->colorspace) {
>+	case DRM_MODE_COLORIMETRY_BT709_YCC:
>+		vsc_sdp.db[16] |= 0x1;
>+		break;
>+	case DRM_MODE_COLORIMETRY_XVYCC_601:
>+		vsc_sdp.db[16] |= 0x2;
>+		break;
>+	case DRM_MODE_COLORIMETRY_XVYCC_709:
>+		vsc_sdp.db[16] |= 0x3;
>+		break;
>+	case DRM_MODE_COLORIMETRY_SYCC_601:
>+		vsc_sdp.db[16] |= 0x4;
>+		break;
>+	case DRM_MODE_COLORIMETRY_OPYCC_601:
>+		vsc_sdp.db[16] |= 0x5;
>+		break;
>+	case DRM_MODE_COLORIMETRY_BT2020_CYCC:
>+	case DRM_MODE_COLORIMETRY_BT2020_RGB:
>+		vsc_sdp.db[16] |= 0x6;
>+		break;
>+	case DRM_MODE_COLORIMETRY_BT2020_YCC:
>+		vsc_sdp.db[16] |= 0x7;
>+		break;
>+	case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
>+	case DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
>+		vsc_sdp.db[16] |= 0x4; /* DCI-P3 (SMPTE RP 431-2) */
>+		break;
>+	default:
>+		/* sRGB (IEC 61966-2-1) / ITU-R BT.601: DB16[0:3] = 0h */
>+
>+		/* RGB->YCBCR color conversion uses the BT.709 color space. */
>+		if (crtc_state->output_format ==
>INTEL_OUTPUT_FORMAT_YCBCR420)
>+			vsc_sdp.db[16] |= 0x1; /* 0x1, ITU-R BT.709 */
>+		break;
>+	}
>
> 	/*
> 	 * For pixel encoding formats YCbCr444, YCbCr422, YCbCr420, and Y Only,
>@@ -4489,13 +4532,14 @@ intel_pixel_encoding_setup_vsc(struct intel_dp
>*intel_dp,
> 			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));  }
>
>-void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp,
>-			       const struct intel_crtc_state *crtc_state)
>+void intel_dp_vsc_enable(struct intel_dp *intel_dp,
>+			 const struct intel_crtc_state *crtc_state,
>+			 const struct drm_connector_state *conn_state)
> {
> 	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
> 		return;
>
>-	intel_pixel_encoding_setup_vsc(intel_dp, crtc_state);
>+	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);
> }
>
> static u8 intel_dp_autotest_link_training(struct intel_dp *intel_dp) diff --git
>a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>index 657bbb1f5ed0..91a0ee6058fe 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.h
>+++ b/drivers/gpu/drm/i915/display/intel_dp.h
>@@ -111,6 +111,9 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);  bool
>intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);  int
>intel_dp_link_required(int pixel_clock, int bpp);  int intel_dp_max_data_rate(int
>max_link_clock, int max_lanes);
>+void intel_dp_vsc_enable(struct intel_dp *intel_dp,
>+			 const struct intel_crtc_state *crtc_state,
>+			 const struct drm_connector_state *conn_state);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
>
> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
>--
>2.22.0

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

RE: [PATCH v2 2/6] drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA

[Shankar, Uma]

>-----Original Message-----
>From: Mun, Gwan-gyeong
>Sent: Friday, August 23, 2019 3:22 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>
>Subject: [PATCH v2 2/6] drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA
>
>When BT.2020 Colorimetry output is used for DP, we should program BT.2020
>Colorimetry to MSA and VSC SDP. It adds output_colorspace to intel_crtc_state struct
>as a place holder of pipe's output colorspace.
>In order to distinguish needed colorimetry for VSC SDP, it adds
>intel_dp_needs_vsc_colorimetry function.
>If the output colorspace requires vsc sdp or output format is YCbCr 4:2:0, it uses MSA
>with VSC SDP.
>
>As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication of Color Encoding
>Format and Content Color Gamut] while sending
>BT.2020 Colorimetry signals we should program MSA MISC1 fields which indicate VSC
>SDP for the Pixel Encoding/Colorimetry Format.
>
>v2: Remove useless parentheses

The changes look good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_ddi.c      |  8 +++---
> .../drm/i915/display/intel_display_types.h    |  3 +++
> drivers/gpu/drm/i915/display/intel_dp.c       | 25 ++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_dp.h       |  1 +
> 4 files changed, 33 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>b/drivers/gpu/drm/i915/display/intel_ddi.c
>index 4f7ea0a35976..5c42b58c1c2f 100644
>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>@@ -1737,11 +1737,13 @@ void intel_ddi_set_pipe_settings(const struct
>intel_crtc_state *crtc_state)
> 	/*
> 	 * As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication
> 	 * of Color Encoding Format and Content Color Gamut] while sending
>-	 * YCBCR 420 signals we should program MSA MISC1 fields which
>-	 * indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
>+	 * YCBCR 420, HDR BT.2020 signals we should program MSA MISC1 fields
>+	 * which indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
> 	 */
>-	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
>+	    intel_dp_needs_vsc_colorimetry(crtc_state->output_colorspace))
> 		temp |= TRANS_MSA_USE_VSC_SDP;
>+
> 	I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);  }
>
>diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>b/drivers/gpu/drm/i915/display/intel_display_types.h
>index 449abaea619f..9845abcf6f29 100644
>--- a/drivers/gpu/drm/i915/display/intel_display_types.h
>+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>@@ -964,6 +964,9 @@ struct intel_crtc_state {
> 	/* Output format RGB/YCBCR etc */
> 	enum intel_output_format output_format;
>
>+	/* Output colorspace sRGB/BT.2020 etc */
>+	u32 output_colorspace;
>+
> 	/* Output down scaling is done in LSPCON device */
> 	bool lspcon_downsampling;
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>b/drivers/gpu/drm/i915/display/intel_dp.c
>index 55d5ab97061c..295d5ed2be96 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.c
>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
>@@ -2164,6 +2164,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> 		pipe_config->has_pch_encoder = true;
>
> 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>+	pipe_config->output_colorspace = intel_conn_state->base.colorspace;
>+
> 	if (lspcon->active)
> 		lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
> 	else
>@@ -4408,6 +4410,26 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp
>*intel_dp,
> 	return 0;
> }
>
>+bool
>+intel_dp_needs_vsc_colorimetry(u32 colorspace) {
>+	bool ret = false;
>+
>+	switch (colorspace) {
>+	case DRM_MODE_COLORIMETRY_SYCC_601:
>+	case DRM_MODE_COLORIMETRY_OPYCC_601:
>+	case DRM_MODE_COLORIMETRY_BT2020_YCC:
>+	case DRM_MODE_COLORIMETRY_BT2020_RGB:
>+	case DRM_MODE_COLORIMETRY_BT2020_CYCC:
>+		ret = true;
>+		break;
>+	default:
>+		break;
>+	}
>+
>+	return ret;
>+}
>+
> static void
> intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
> 		       const struct intel_crtc_state *crtc_state, @@ -4536,7 +4558,8
>@@ void intel_dp_vsc_enable(struct intel_dp *intel_dp,
> 			 const struct intel_crtc_state *crtc_state,
> 			 const struct drm_connector_state *conn_state)  {
>-	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
>+	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
>+	    !intel_dp_needs_vsc_colorimetry(conn_state->colorspace))
> 		return;
>
> 	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state); diff --git
>a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>index 91a0ee6058fe..b2da7c9998f7 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.h
>+++ b/drivers/gpu/drm/i915/display/intel_dp.h
>@@ -111,6 +111,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);  bool
>intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);  int
>intel_dp_link_required(int pixel_clock, int bpp);  int intel_dp_max_data_rate(int
>max_link_clock, int max_lanes);
>+bool intel_dp_needs_vsc_colorimetry(u32 colorspace);
> void intel_dp_vsc_enable(struct intel_dp *intel_dp,
> 			 const struct intel_crtc_state *crtc_state,
> 			 const struct drm_connector_state *conn_state);
>--
>2.22.0

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

Re: [PATCH v2 3/6] drm: Add DisplayPort colorspace property

[Shankar, Uma]

>-----Original Message-----
>From: Mun, Gwan-gyeong
>Sent: Friday, August 23, 2019 3:22 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>
>Subject: [PATCH v2 3/6] drm: Add DisplayPort colorspace property
>
>In order to use colorspace property to Display Port connectors, it extends
>DRM_MODE_CONNECTOR_DisplayPort connector_type on
>drm_mode_create_colorspace_property function.

The changes look good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>---
> drivers/gpu/drm/drm_connector.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>index 4c766624b20d..655ada9d4c16 100644
>--- a/drivers/gpu/drm/drm_connector.c
>+++ b/drivers/gpu/drm/drm_connector.c
>@@ -1703,7 +1703,9 @@ int drm_mode_create_colorspace_property(struct
>drm_connector *connector)
> 	struct drm_property *prop;
>
> 	if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
>-	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
>+	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB ||
>+	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>+	    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> 		prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
> 						"Colorspace",
> 						hdmi_colorspaces,
>--
>2.22.0

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

RE: [PATCH v2 4/6] drm/i915/dp: Attach colorspace property

[Shankar, Uma]

>-----Original Message-----
>From: Mun, Gwan-gyeong
>Sent: Friday, August 23, 2019 3:23 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>
>Subject: [PATCH v2 4/6] drm/i915/dp: Attach colorspace property
>
>It attaches the colorspace connector property to a DisplayPort connector.
>Based on colorspace change, modeset will be triggered to switch to a new
>colorspace.
>
>Based on colorspace property value create a VSC SDP packet with appropriate
>colorspace. This would help to enable wider color gamut like BT2020 on a sink device.

The changes look good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>b/drivers/gpu/drm/i915/display/intel_dp.c
>index 295d5ed2be96..7218e159f55d 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.c
>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
>@@ -6402,6 +6402,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
>drm_connector *connect
> 	else if (INTEL_GEN(dev_priv) >= 5)
> 		drm_connector_attach_max_bpc_property(connector, 6, 12);
>
>+	intel_attach_colorspace_property(connector);
>+
> 	if (intel_dp_is_edp(intel_dp)) {
> 		u32 allowed_scalers;
>
>--
>2.22.0

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

Re: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata

[Shankar, Uma]

>-----Original Message-----
>From: Mun, Gwan-gyeong
>Sent: Friday, August 23, 2019 3:23 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>
>Subject: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB for
>HDR Static Metadata
>
>Function intel_dp_setup_hdr_metadata_infoframe_sdp handles Infoframe SDP
>header and data block setup for HDR Static Metadata. It enables writing of HDR
>metadata infoframe SDP to panel. Support for HDR video was introduced in
>DisplayPort 1.4. It implements the CTA-861-G standard for transport of static HDR
>metadata. The HDR Metadata will be provided by userspace compositors, based on
>blending policies and passed to the driver through a blob property.
>
>Because each of GEN11 and prior GEN11 have different register size for HDR
>Metadata Infoframe SDP packet, it adds and uses different register size.
>
>Setup Infoframe SDP header and data block in function
>intel_dp_setup_hdr_metadata_infoframe_sdp for HDR Static Metadata as per dp 1.4
>spec and CTA-861-F spec.
>As per DP 1.4 spec, 2.2.2.5 SDP Formats. It enables Dynamic Range and Mastering
>Infoframe for HDR content, which is defined in CTA-861-F spec.
>According to DP 1.4 spec and CEA-861-F spec Table 5, in order to transmit static HDR
>metadata, we have to use Non-audio INFOFRAME SDP v1.3.
>
>+--------------------------------+-------------------------------+
>|      [ Packet Type Value ]     |       [ Packet Type ]         |
>+--------------------------------+-------------------------------+
>| 80h + Non-audio INFOFRAME Type | CEA-861-F Non-audio INFOFRAME |
>+--------------------------------+-------------------------------+
>|      [Transmission Timing]                                     |
>+----------------------------------------------------------------+
>| As per CEA-861-F for INFOFRAME, including CEA-861.3 within     |
>| which Dynamic Range and Mastering INFOFRAME are defined        |
>+----------------------------------------------------------------+
>
>v2: Add a missed blank line after function declaration.
>
>Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_ddi.c |  1 +
>drivers/gpu/drm/i915/display/intel_dp.c  | 91 ++++++++++++++++++++++++
>drivers/gpu/drm/i915/display/intel_dp.h  |  3 +
> drivers/gpu/drm/i915/i915_reg.h          |  1 +
> 4 files changed, 96 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>b/drivers/gpu/drm/i915/display/intel_ddi.c
>index 5c42b58c1c2f..9f5bea941bcd 100644
>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>@@ -3478,6 +3478,7 @@ static void intel_enable_ddi_dp(struct intel_encoder
>*encoder,
> 	intel_edp_backlight_on(crtc_state, conn_state);
> 	intel_psr_enable(intel_dp, crtc_state);
> 	intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
>+	intel_dp_hdr_metadata_enable(intel_dp, crtc_state, conn_state);
> 	intel_edp_drrs_enable(intel_dp, crtc_state);
>
> 	if (crtc_state->has_audio)
>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>b/drivers/gpu/drm/i915/display/intel_dp.c
>index 7218e159f55d..00da8221eaba 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.c
>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
>@@ -4554,6 +4554,85 @@ intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
> 			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));  }
>
>+static int
>+intel_dp_setup_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
>+					  const struct intel_crtc_state *crtc_state,
>+					  const struct drm_connector_state

The return value is not handled, you may convert it as void.

>*conn_state) {
>+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>+	struct dp_sdp infoframe_sdp = {};
>+	struct hdmi_drm_infoframe drm_infoframe = {};
>+	const int infoframe_size = HDMI_INFOFRAME_HEADER_SIZE +
>HDMI_DRM_INFOFRAME_SIZE;
>+	unsigned char buf[HDMI_INFOFRAME_HEADER_SIZE +
>HDMI_DRM_INFOFRAME_SIZE];
>+	ssize_t len;
>+	int ret;
>+
>+	ret = drm_hdmi_infoframe_set_hdr_metadata(&drm_infoframe, conn_state);
>+	if (ret) {
>+		DRM_DEBUG_KMS("couldn't set HDR metadata in infoframe\n");
>+		return ret;
>+	}
>+
>+	len = hdmi_drm_infoframe_pack_only(&drm_infoframe, buf, sizeof(buf));
>+	if (len < 0) {
>+		DRM_DEBUG_KMS("buffer size is smaller than hdr metadata
>infoframe\n");
>+		return (int)len;

If made void, this will not be required.

>+	}
>+
>+	if (len != infoframe_size) {
>+		DRM_DEBUG_KMS("wrong static hdr metadata size\n");
>+		return -EINVAL;
>+	}
>+
>+	/*
>+	 * Set up the infoframe sdp packet for HDR static metadata.
>+	 * Prepare VSC Header for SU as per DP 1.4a spec,
>+	 * Table 2-100 and Table 2-101
>+	 */
>+
>+	/* Packet ID, 00h for non-Audio INFOFRAME */
>+	infoframe_sdp.sdp_header.HB0 = 0;
>+	/*
>+	 * Packet Type 80h + Non-audio INFOFRAME Type value
>+	 * HDMI_INFOFRAME_TYPE_DRM: 0x87,
>+	 */
>+	infoframe_sdp.sdp_header.HB1 = drm_infoframe.type;
>+	/*
>+	 * Least Significant Eight Bits of (Data Byte Count – 1)
>+	 * infoframe_size - 1,
>+	 */
>+	infoframe_sdp.sdp_header.HB2 = 0x1D;
>+	/* INFOFRAME SDP Version Number */
>+	infoframe_sdp.sdp_header.HB3 = (0x13 << 2);
>+	/* CTA Header Byte 2 (INFOFRAME Version Number) */
>+	infoframe_sdp.db[0] = drm_infoframe.version;
>+	/* CTA Header Byte 3 (Length of INFOFRAME): HDMI_DRM_INFOFRAME_SIZE
>*/
>+	infoframe_sdp.db[1] = drm_infoframe.length;
>+	/*
>+	 * Copy HDMI_DRM_INFOFRAME_SIZE size from a buffer after
>+	 * HDMI_INFOFRAME_HEADER_SIZE
>+	 */
>+	memcpy(&infoframe_sdp.db[2], &buf[HDMI_INFOFRAME_HEADER_SIZE],
>+	       HDMI_DRM_INFOFRAME_SIZE);
>+
>+	if (INTEL_GEN(dev_priv) >= 11)
>+		intel_dig_port->write_infoframe(&intel_dig_port->base,
>+						crtc_state,
>+
>	HDMI_PACKET_TYPE_GAMUT_METADATA,
>+						&infoframe_sdp,
>+						VIDEO_DIP_GMP_DATA_SIZE);

This new VIDEO_DIP_GMP_DATA_SIZE doesn't seem to be handled in hsw_write_infoframe
(hsw_dip_data_size). Can you please check this.

>+	else
>+		/* Prior to GEN11, Header size: 4 bytes, Data size: 28 bytes */
>+		intel_dig_port->write_infoframe(&intel_dig_port->base,
>+						crtc_state,
>+
>	HDMI_PACKET_TYPE_GAMUT_METADATA,
>+						&infoframe_sdp,
>+						VIDEO_DIP_DATA_SIZE);
>+

Also can you update the series to handle state checking also for metadata sent to DP sink.

>+	return 0;
>+}
>+
> void intel_dp_vsc_enable(struct intel_dp *intel_dp,
> 			 const struct intel_crtc_state *crtc_state,
> 			 const struct drm_connector_state *conn_state) @@ -
>4565,6 +4644,18 @@ void intel_dp_vsc_enable(struct intel_dp *intel_dp,
> 	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);  }
>
>+void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
>+				  const struct intel_crtc_state *crtc_state,
>+				  const struct drm_connector_state *conn_state) {
>+	if (!conn_state->hdr_output_metadata)
>+		return;
>+
>+	intel_dp_setup_hdr_metadata_infoframe_sdp(intel_dp,
>+						  crtc_state,
>+						  conn_state);
>+}
>+
> static u8 intel_dp_autotest_link_training(struct intel_dp *intel_dp)  {
> 	int status = 0;
>diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
>b/drivers/gpu/drm/i915/display/intel_dp.h
>index b2da7c9998f7..c3593691dd38 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.h
>+++ b/drivers/gpu/drm/i915/display/intel_dp.h
>@@ -115,6 +115,9 @@ bool intel_dp_needs_vsc_colorimetry(u32 colorspace);  void
>intel_dp_vsc_enable(struct intel_dp *intel_dp,
> 			 const struct intel_crtc_state *crtc_state,
> 			 const struct drm_connector_state *conn_state);
>+void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
>+				  const struct intel_crtc_state *crtc_state,
>+				  const struct drm_connector_state *conn_state);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
>
> static inline unsigned int intel_dp_unused_lane_mask(int lane_count) diff --git
>a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>ea2f0fa2402d..92885416d539 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -4645,6 +4645,7 @@ enum {
>  * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds to each byte
>  * of the infoframe structure specified by CEA-861. */
> #define   VIDEO_DIP_DATA_SIZE	32
>+#define   VIDEO_DIP_GMP_DATA_SIZE	36
> #define   VIDEO_DIP_VSC_DATA_SIZE	36
> #define   VIDEO_DIP_PPS_DATA_SIZE	132
> #define VIDEO_DIP_CTL		_MMIO(0x61170)
>--
>2.22.0

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

Re: [PATCH v2 6/6] drm/i915/dp: Attach HDR metadata property to DP connector

[Shankar, Uma]

>-----Original Message-----
>From: Mun, Gwan-gyeong
>Sent: Friday, August 23, 2019 3:23 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>
>Subject: [PATCH v2 6/6] drm/i915/dp: Attach HDR metadata property to DP connector
>
>It attaches HDR metadata property to DP connector on GLK+.
>It enables HDR metadata infoframe sdp on GLK+ to be used to send HDR metadata to
>DP sink.
>
>v2: Minor style fix

The changes look good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>b/drivers/gpu/drm/i915/display/intel_dp.c
>index 00da8221eaba..899923ecd1c9 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.c
>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
>@@ -6495,6 +6495,11 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
>drm_connector *connect
>
> 	intel_attach_colorspace_property(connector);
>
>+	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11)
>+		drm_object_attach_property(&connector->base,
>+					   connector->dev-
>>mode_config.hdr_output_metadata_property,
>+					   0);
>+
> 	if (intel_dp_is_edp(intel_dp)) {
> 		u32 allowed_scalers;
>
>--
>2.22.0

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

Re: [PATCH v2 2/6] drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA

[Ville Syrjälä]

On Fri, Aug 23, 2019 at 12:52:28PM +0300, Gwan-gyeong Mun wrote:
> When BT.2020 Colorimetry output is used for DP, we should program BT.2020
> Colorimetry to MSA and VSC SDP. It adds output_colorspace to
> intel_crtc_state struct as a place holder of pipe's output colorspace.
> In order to distinguish needed colorimetry for VSC SDP, it adds
> intel_dp_needs_vsc_colorimetry function.
> If the output colorspace requires vsc sdp or output format is YCbCr 4:2:0,
> it uses MSA with VSC SDP.
> 
> As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication of
> Color Encoding Format and Content Color Gamut] while sending
> BT.2020 Colorimetry signals we should program MSA MISC1 fields which
> indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
> 
> v2: Remove useless parentheses
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      |  8 +++---
>  .../drm/i915/display/intel_display_types.h    |  3 +++
>  drivers/gpu/drm/i915/display/intel_dp.c       | 25 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_dp.h       |  1 +
>  4 files changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4f7ea0a35976..5c42b58c1c2f 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1737,11 +1737,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
>  	/*
>  	 * As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication
>  	 * of Color Encoding Format and Content Color Gamut] while sending
> -	 * YCBCR 420 signals we should program MSA MISC1 fields which
> -	 * indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
> +	 * YCBCR 420, HDR BT.2020 signals we should program MSA MISC1 fields
> +	 * which indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
>  	 */
> -	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> +	    intel_dp_needs_vsc_colorimetry(crtc_state->output_colorspace))
>  		temp |= TRANS_MSA_USE_VSC_SDP;
> +
>  	I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 449abaea619f..9845abcf6f29 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -964,6 +964,9 @@ struct intel_crtc_state {
>  	/* Output format RGB/YCBCR etc */
>  	enum intel_output_format output_format;
>  
> +	/* Output colorspace sRGB/BT.2020 etc */
> +	u32 output_colorspace;
> +
>  	/* Output down scaling is done in LSPCON device */
>  	bool lspcon_downsampling;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 55d5ab97061c..295d5ed2be96 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2164,6 +2164,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		pipe_config->has_pch_encoder = true;
>  
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +	pipe_config->output_colorspace = intel_conn_state->base.colorspace;
> +
>  	if (lspcon->active)
>  		lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
>  	else
> @@ -4408,6 +4410,26 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>  	return 0;
>  }
>  
> +bool
> +intel_dp_needs_vsc_colorimetry(u32 colorspace)

I would pass the entire crtc state here so you handle handle the 4:2:0
case here as well.

> +{
> +	bool ret = false;

Pointless variable.

> +
> +	switch (colorspace) {
> +	case DRM_MODE_COLORIMETRY_SYCC_601:
> +	case DRM_MODE_COLORIMETRY_OPYCC_601:
> +	case DRM_MODE_COLORIMETRY_BT2020_YCC:
> +	case DRM_MODE_COLORIMETRY_BT2020_RGB:
> +	case DRM_MODE_COLORIMETRY_BT2020_CYCC:
> +		ret = true;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
>  static void
>  intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
>  		       const struct intel_crtc_state *crtc_state,
> @@ -4536,7 +4558,8 @@ void intel_dp_vsc_enable(struct intel_dp *intel_dp,
>  			 const struct intel_crtc_state *crtc_state,
>  			 const struct drm_connector_state *conn_state)
>  {
> -	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
> +	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
> +	    !intel_dp_needs_vsc_colorimetry(conn_state->colorspace))
>  		return;
>  
>  	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 91a0ee6058fe..b2da7c9998f7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -111,6 +111,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
>  bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
>  int intel_dp_link_required(int pixel_clock, int bpp);
>  int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> +bool intel_dp_needs_vsc_colorimetry(u32 colorspace);
>  void intel_dp_vsc_enable(struct intel_dp *intel_dp,
>  			 const struct intel_crtc_state *crtc_state,
>  			 const struct drm_connector_state *conn_state);
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v2 3/6] drm: Add DisplayPort colorspace property

[Ville Syrjälä]

On Fri, Aug 23, 2019 at 12:52:29PM +0300, Gwan-gyeong Mun wrote:
> In order to use colorspace property to Display Port connectors, it extends
> DRM_MODE_CONNECTOR_DisplayPort connector_type on
> drm_mode_create_colorspace_property function.
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..655ada9d4c16 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1703,7 +1703,9 @@ int drm_mode_create_colorspace_property(struct drm_connector *connector)
>  	struct drm_property *prop;
>  
>  	if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> -	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
> +	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB ||
> +	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {

We don't need a separate set of enum values for DP?

>  		prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
>  						"Colorspace",
>  						hdmi_colorspaces,
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v2 2/6] drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA

[Mun, Gwan-gyeong]

On Mon, 2019-09-02 at 17:43 +0300, Ville Syrjälä wrote:
> On Fri, Aug 23, 2019 at 12:52:28PM +0300, Gwan-gyeong Mun wrote:
> > When BT.2020 Colorimetry output is used for DP, we should program
> > BT.2020
> > Colorimetry to MSA and VSC SDP. It adds output_colorspace to
> > intel_crtc_state struct as a place holder of pipe's output
> > colorspace.
> > In order to distinguish needed colorimetry for VSC SDP, it adds
> > intel_dp_needs_vsc_colorimetry function.
> > If the output colorspace requires vsc sdp or output format is YCbCr
> > 4:2:0,
> > it uses MSA with VSC SDP.
> > 
> > As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication of
> > Color Encoding Format and Content Color Gamut] while sending
> > BT.2020 Colorimetry signals we should program MSA MISC1 fields
> > which
> > indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
> > 
> > v2: Remove useless parentheses
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      |  8 +++---
> >  .../drm/i915/display/intel_display_types.h    |  3 +++
> >  drivers/gpu/drm/i915/display/intel_dp.c       | 25
> > ++++++++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_dp.h       |  1 +
> >  4 files changed, 33 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 4f7ea0a35976..5c42b58c1c2f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1737,11 +1737,13 @@ void intel_ddi_set_pipe_settings(const
> > struct intel_crtc_state *crtc_state)
> >  	/*
> >  	 * As per DP 1.4a spec section 2.2.4.3 [MSA Field for
> > Indication
> >  	 * of Color Encoding Format and Content Color Gamut] while
> > sending
> > -	 * YCBCR 420 signals we should program MSA MISC1 fields which
> > -	 * indicate VSC SDP for the Pixel Encoding/Colorimetry Format.
> > +	 * YCBCR 420, HDR BT.2020 signals we should program MSA MISC1
> > fields
> > +	 * which indicate VSC SDP for the Pixel Encoding/Colorimetry
> > Format.
> >  	 */
> > -	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> > +	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420
> > ||
> > +	    intel_dp_needs_vsc_colorimetry(crtc_state-
> > >output_colorspace))
> >  		temp |= TRANS_MSA_USE_VSC_SDP;
> > +
> >  	I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 449abaea619f..9845abcf6f29 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -964,6 +964,9 @@ struct intel_crtc_state {
> >  	/* Output format RGB/YCBCR etc */
> >  	enum intel_output_format output_format;
> >  
> > +	/* Output colorspace sRGB/BT.2020 etc */
> > +	u32 output_colorspace;
> > +
> >  	/* Output down scaling is done in LSPCON device */
> >  	bool lspcon_downsampling;
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 55d5ab97061c..295d5ed2be96 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -2164,6 +2164,8 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >  		pipe_config->has_pch_encoder = true;
> >  
> >  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> > +	pipe_config->output_colorspace = intel_conn_state-
> > >base.colorspace;
> > +
> >  	if (lspcon->active)
> >  		lspcon_ycbcr420_config(&intel_connector->base,
> > pipe_config);
> >  	else
> > @@ -4408,6 +4410,26 @@ u8 intel_dp_dsc_get_slice_count(struct
> > intel_dp *intel_dp,
> >  	return 0;
> >  }
> >  
> > +bool
> > +intel_dp_needs_vsc_colorimetry(u32 colorspace)
> 
> I would pass the entire crtc state here so you handle handle the
> 4:2:0
> case here as well.
> 
Okay, I'll pass the entire intel_crtc_state stucture value to
intel_dp_needs_vsc_colorimetry() for checking output format (YCbCr
4:2:0)  and output colorspace here. And I'll change the fuction name to
intel_dp_needs_vsc_sdp().
> > +{
> > +	bool ret = false;
> 
> Pointless variable.
> 
I'll remove pointless variables.
> > +
> > +	switch (colorspace) {
> > +	case DRM_MODE_COLORIMETRY_SYCC_601:
> > +	case DRM_MODE_COLORIMETRY_OPYCC_601:
> > +	case DRM_MODE_COLORIMETRY_BT2020_YCC:
> > +	case DRM_MODE_COLORIMETRY_BT2020_RGB:
> > +	case DRM_MODE_COLORIMETRY_BT2020_CYCC:
> > +		ret = true;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  static void
> >  intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
> >  		       const struct intel_crtc_state *crtc_state,
> > @@ -4536,7 +4558,8 @@ void intel_dp_vsc_enable(struct intel_dp
> > *intel_dp,
> >  			 const struct intel_crtc_state *crtc_state,
> >  			 const struct drm_connector_state *conn_state)
> >  {
> > -	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
> > +	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420
> > &&
> > +	    !intel_dp_needs_vsc_colorimetry(conn_state->colorspace))
> >  		return;
> >  
> >  	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> > b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 91a0ee6058fe..b2da7c9998f7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -111,6 +111,7 @@ bool intel_dp_read_dpcd(struct intel_dp
> > *intel_dp);
> >  bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
> >  int intel_dp_link_required(int pixel_clock, int bpp);
> >  int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > +bool intel_dp_needs_vsc_colorimetry(u32 colorspace);
> >  void intel_dp_vsc_enable(struct intel_dp *intel_dp,
> >  			 const struct intel_crtc_state *crtc_state,
> >  			 const struct drm_connector_state *conn_state);
> > -- 
> > 2.22.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/6] drm: Add DisplayPort colorspace property

[Mun, Gwan-gyeong]

On Mon, 2019-09-02 at 17:44 +0300, Ville Syrjälä wrote:
> On Fri, Aug 23, 2019 at 12:52:29PM +0300, Gwan-gyeong Mun wrote:
> > In order to use colorspace property to Display Port connectors, it
> > extends
> > DRM_MODE_CONNECTOR_DisplayPort connector_type on
> > drm_mode_create_colorspace_property function.
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> >  drivers/gpu/drm/drm_connector.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index 4c766624b20d..655ada9d4c16 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1703,7 +1703,9 @@ int
> > drm_mode_create_colorspace_property(struct drm_connector
> > *connector)
> >  	struct drm_property *prop;
> >  
> >  	if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> > -	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
> > +	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB ||
> > +	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort
> > ||
> > +	    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> 
> We don't need a separate set of enum values for DP?
> 
I checked DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel
Encoding/Colorimetry again,
Followed your comments, the spec requires more new colorimetry options
for DP 1.4a colorimetry format.
I'll add missed colorimetry options and will separate set of
colorimetry enum values for DP.

> >  		prop = drm_property_create_enum(dev,
> > DRM_MODE_PROP_ENUM,
> >  						"Colorspace",
> >  						hdmi_colorspaces,
> > -- 
> > 2.22.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata

[Mun, Gwan-gyeong]

On Tue, 2019-08-27 at 01:14 +0530, Shankar, Uma wrote:
> > -----Original Message-----
> > From: Mun, Gwan-gyeong
> > Sent: Friday, August 23, 2019 3:23 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <
> > uma.shankar@intel.com>;
> > Sharma, Shashank <shashank.sharma@intel.com>
> > Subject: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP
> > Header and DB for
> > HDR Static Metadata
> > 
> > Function intel_dp_setup_hdr_metadata_infoframe_sdp handles
> > Infoframe SDP
> > header and data block setup for HDR Static Metadata. It enables
> > writing of HDR
> > metadata infoframe SDP to panel. Support for HDR video was
> > introduced in
> > DisplayPort 1.4. It implements the CTA-861-G standard for transport
> > of static HDR
> > metadata. The HDR Metadata will be provided by userspace
> > compositors, based on
> > blending policies and passed to the driver through a blob property.
> > 
> > Because each of GEN11 and prior GEN11 have different register size
> > for HDR
> > Metadata Infoframe SDP packet, it adds and uses different register
> > size.
> > 
> > Setup Infoframe SDP header and data block in function
> > intel_dp_setup_hdr_metadata_infoframe_sdp for HDR Static Metadata
> > as per dp 1.4
> > spec and CTA-861-F spec.
> > As per DP 1.4 spec, 2.2.2.5 SDP Formats. It enables Dynamic Range
> > and Mastering
> > Infoframe for HDR content, which is defined in CTA-861-F spec.
> > According to DP 1.4 spec and CEA-861-F spec Table 5, in order to
> > transmit static HDR
> > metadata, we have to use Non-audio INFOFRAME SDP v1.3.
> > 
> > +--------------------------------+-------------------------------+
> > >      [ Packet Type Value ]     |       [ Packet Type ]         |
> > +--------------------------------+-------------------------------+
> > > 80h + Non-audio INFOFRAME Type | CEA-861-F Non-audio INFOFRAME |
> > +--------------------------------+-------------------------------+
> > >      [Transmission Timing]                                     |
> > +----------------------------------------------------------------+
> > > As per CEA-861-F for INFOFRAME, including CEA-861.3 within     |
> > > which Dynamic Range and Mastering INFOFRAME are defined        |
> > +----------------------------------------------------------------+
> > 
> > v2: Add a missed blank line after function declaration.
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c |  1 +
> > drivers/gpu/drm/i915/display/intel_dp.c  | 91
> > ++++++++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_dp.h  |  3 +
> > drivers/gpu/drm/i915/i915_reg.h          |  1 +
> > 4 files changed, 96 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 5c42b58c1c2f..9f5bea941bcd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3478,6 +3478,7 @@ static void intel_enable_ddi_dp(struct
> > intel_encoder
> > *encoder,
> > 	intel_edp_backlight_on(crtc_state, conn_state);
> > 	intel_psr_enable(intel_dp, crtc_state);
> > 	intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
> > +	intel_dp_hdr_metadata_enable(intel_dp, crtc_state, conn_state);
> > 	intel_edp_drrs_enable(intel_dp, crtc_state);
> > 
> > 	if (crtc_state->has_audio)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 7218e159f55d..00da8221eaba 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4554,6 +4554,85 @@ intel_dp_setup_vsc_sdp(struct intel_dp
> > *intel_dp,
> > 			crtc_state, DP_SDP_VSC, &vsc_sdp,
> > sizeof(vsc_sdp));  }
> > 
> > +static int
> > +intel_dp_setup_hdr_metadata_infoframe_sdp(struct intel_dp
> > *intel_dp,
> > +					  const struct intel_crtc_state
> > *crtc_state,
> > +					  const struct
> > drm_connector_state
> 
> The return value is not handled, you may convert it as void.
> 
Okay, I'll remove the return values which is not handled from
intel_dp_setup_hdr_metadata_infoframe_sdp().

> > *conn_state) {
> > +	struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> > >base.base.dev);
> > +	struct dp_sdp infoframe_sdp = {};
> > +	struct hdmi_drm_infoframe drm_infoframe = {};
> > +	const int infoframe_size = HDMI_INFOFRAME_HEADER_SIZE +
> > HDMI_DRM_INFOFRAME_SIZE;
> > +	unsigned char buf[HDMI_INFOFRAME_HEADER_SIZE +
> > HDMI_DRM_INFOFRAME_SIZE];
> > +	ssize_t len;
> > +	int ret;
> > +
> > +	ret = drm_hdmi_infoframe_set_hdr_metadata(&drm_infoframe,
> > conn_state);
> > +	if (ret) {
> > +		DRM_DEBUG_KMS("couldn't set HDR metadata in
> > infoframe\n");
> > +		return ret;
> > +	}
> > +
> > +	len = hdmi_drm_infoframe_pack_only(&drm_infoframe, buf,
> > sizeof(buf));
> > +	if (len < 0) {
> > +		DRM_DEBUG_KMS("buffer size is smaller than hdr metadata
> > infoframe\n");
> > +		return (int)len;
> 
> If made void, this will not be required.
> 
> > +	}
> > +
> > +	if (len != infoframe_size) {
> > +		DRM_DEBUG_KMS("wrong static hdr metadata size\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/*
> > +	 * Set up the infoframe sdp packet for HDR static metadata.
> > +	 * Prepare VSC Header for SU as per DP 1.4a spec,
> > +	 * Table 2-100 and Table 2-101
> > +	 */
> > +
> > +	/* Packet ID, 00h for non-Audio INFOFRAME */
> > +	infoframe_sdp.sdp_header.HB0 = 0;
> > +	/*
> > +	 * Packet Type 80h + Non-audio INFOFRAME Type value
> > +	 * HDMI_INFOFRAME_TYPE_DRM: 0x87,
> > +	 */
> > +	infoframe_sdp.sdp_header.HB1 = drm_infoframe.type;
> > +	/*
> > +	 * Least Significant Eight Bits of (Data Byte Count – 1)
> > +	 * infoframe_size - 1,
> > +	 */
> > +	infoframe_sdp.sdp_header.HB2 = 0x1D;
> > +	/* INFOFRAME SDP Version Number */
> > +	infoframe_sdp.sdp_header.HB3 = (0x13 << 2);
> > +	/* CTA Header Byte 2 (INFOFRAME Version Number) */
> > +	infoframe_sdp.db[0] = drm_infoframe.version;
> > +	/* CTA Header Byte 3 (Length of INFOFRAME):
> > HDMI_DRM_INFOFRAME_SIZE
> > */
> > +	infoframe_sdp.db[1] = drm_infoframe.length;
> > +	/*
> > +	 * Copy HDMI_DRM_INFOFRAME_SIZE size from a buffer after
> > +	 * HDMI_INFOFRAME_HEADER_SIZE
> > +	 */
> > +	memcpy(&infoframe_sdp.db[2], &buf[HDMI_INFOFRAME_HEADER_SIZE],
> > +	       HDMI_DRM_INFOFRAME_SIZE);
> > +
> > +	if (INTEL_GEN(dev_priv) >= 11)
> > +		intel_dig_port->write_infoframe(&intel_dig_port->base,
> > +						crtc_state,
> > +
> > 	HDMI_PACKET_TYPE_GAMUT_METADATA,
> > +						&infoframe_sdp,
> > +						VIDEO_DIP_GMP_DATA_SIZE
> > );
> 
> This new VIDEO_DIP_GMP_DATA_SIZE doesn't seem to be handled in
> hsw_write_infoframe
> (hsw_dip_data_size). Can you please check this.
> 
Okay, I'll add missed handling of VIDEO_DIP_GMP_DATA_SIZE on
hsw_dip_data_size().
> > +	else
> > +		/* Prior to GEN11, Header size: 4 bytes, Data size: 28
> > bytes */
> > +		intel_dig_port->write_infoframe(&intel_dig_port->base,
> > +						crtc_state,
> > +
> > 	HDMI_PACKET_TYPE_GAMUT_METADATA,
> > +						&infoframe_sdp,
> > +						VIDEO_DIP_DATA_SIZE);
> > +
> 
> Also can you update the series to handle state checking also for
> metadata sent to DP sink.
> 
Does it mean a similar logic of "intel_read_infoframe(encoder,
pipe_config,  HDMI_INFOFRAME_TYPE_DRM, &pipe_config->infoframes.drm);"

if then, because current DP packet related implementation of i915 does
not hold DP packets (ex. VSC SDP, HDR Metadata Infoframe SDP...) to the
pipe state structure. In my opinion, as a diffenent series, first, it
would be better to add a storing of infoframe packets to pipe_state
structure for later comparing call (ex.  intel_pipe_config_compare() ).
after then we can compare them.

> > +	return 0;
> > +}
> > +
> > void intel_dp_vsc_enable(struct intel_dp *intel_dp,
> > 			 const struct intel_crtc_state *crtc_state,
> > 			 const struct drm_connector_state *conn_state)
> > @@ -
> > 4565,6 +4644,18 @@ void intel_dp_vsc_enable(struct intel_dp
> > *intel_dp,
> > 	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);  }
> > 
> > +void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> > +				  const struct intel_crtc_state
> > *crtc_state,
> > +				  const struct drm_connector_state
> > *conn_state) {
> > +	if (!conn_state->hdr_output_metadata)
> > +		return;
> > +
> > +	intel_dp_setup_hdr_metadata_infoframe_sdp(intel_dp,
> > +						  crtc_state,
> > +						  conn_state);
> > +}
> > +
> > static u8 intel_dp_autotest_link_training(struct intel_dp
> > *intel_dp)  {
> > 	int status = 0;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> > b/drivers/gpu/drm/i915/display/intel_dp.h
> > index b2da7c9998f7..c3593691dd38 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -115,6 +115,9 @@ bool intel_dp_needs_vsc_colorimetry(u32
> > colorspace);  void
> > intel_dp_vsc_enable(struct intel_dp *intel_dp,
> > 			 const struct intel_crtc_state *crtc_state,
> > 			 const struct drm_connector_state *conn_state);
> > +void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> > +				  const struct intel_crtc_state
> > *crtc_state,
> > +				  const struct drm_connector_state
> > *conn_state);
> > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > 
> > static inline unsigned int intel_dp_unused_lane_mask(int
> > lane_count) diff --git
> > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index
> > ea2f0fa2402d..92885416d539 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4645,6 +4645,7 @@ enum {
> >  * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds
> > to each byte
> >  * of the infoframe structure specified by CEA-861. */
> > #define   VIDEO_DIP_DATA_SIZE	32
> > +#define   VIDEO_DIP_GMP_DATA_SIZE	36
> > #define   VIDEO_DIP_VSC_DATA_SIZE	36
> > #define   VIDEO_DIP_PPS_DATA_SIZE	132
> > #define VIDEO_DIP_CTL		_MMIO(0x61170)
> > --
> > 2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata

[Shankar, Uma]

>-----Original Message-----
>From: Mun, Gwan-gyeong
>Sent: Tuesday, September 3, 2019 10:54 AM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org
>Cc: dri-devel@lists.freedesktop.org; Sharma, Shashank
><shashank.sharma@intel.com>
>Subject: Re: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB
>for HDR Static Metadata
>
>On Tue, 2019-08-27 at 01:14 +0530, Shankar, Uma wrote:
>> > -----Original Message-----
>> > From: Mun, Gwan-gyeong
>> > Sent: Friday, August 23, 2019 3:23 PM
>> > To: intel-gfx@lists.freedesktop.org
>> > Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <
>> > uma.shankar@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>
>> > Subject: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header
>> > and DB for HDR Static Metadata
>> >
>> > Function intel_dp_setup_hdr_metadata_infoframe_sdp handles Infoframe
>> > SDP header and data block setup for HDR Static Metadata. It enables
>> > writing of HDR metadata infoframe SDP to panel. Support for HDR
>> > video was introduced in DisplayPort 1.4. It implements the CTA-861-G
>> > standard for transport of static HDR metadata. The HDR Metadata will
>> > be provided by userspace compositors, based on blending policies and
>> > passed to the driver through a blob property.
>> >
>> > Because each of GEN11 and prior GEN11 have different register size
>> > for HDR Metadata Infoframe SDP packet, it adds and uses different
>> > register size.
>> >
>> > Setup Infoframe SDP header and data block in function
>> > intel_dp_setup_hdr_metadata_infoframe_sdp for HDR Static Metadata as
>> > per dp 1.4 spec and CTA-861-F spec.
>> > As per DP 1.4 spec, 2.2.2.5 SDP Formats. It enables Dynamic Range
>> > and Mastering Infoframe for HDR content, which is defined in
>> > CTA-861-F spec.
>> > According to DP 1.4 spec and CEA-861-F spec Table 5, in order to
>> > transmit static HDR metadata, we have to use Non-audio INFOFRAME SDP
>> > v1.3.
>> >
>> > +--------------------------------+-------------------------------+
>> > >      [ Packet Type Value ]     |       [ Packet Type ]         |
>> > +--------------------------------+-------------------------------+
>> > > 80h + Non-audio INFOFRAME Type | CEA-861-F Non-audio INFOFRAME |
>> > +--------------------------------+-------------------------------+
>> > >      [Transmission Timing]                                     |
>> > +----------------------------------------------------------------+
>> > > As per CEA-861-F for INFOFRAME, including CEA-861.3 within     |
>> > > which Dynamic Range and Mastering INFOFRAME are defined        |
>> > +----------------------------------------------------------------+
>> >
>> > v2: Add a missed blank line after function declaration.
>> >
>> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_ddi.c |  1 +
>> > drivers/gpu/drm/i915/display/intel_dp.c  | 91
>> > ++++++++++++++++++++++++
>> > drivers/gpu/drm/i915/display/intel_dp.h  |  3 +
>> > drivers/gpu/drm/i915/i915_reg.h          |  1 +
>> > 4 files changed, 96 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>> > b/drivers/gpu/drm/i915/display/intel_ddi.c
>> > index 5c42b58c1c2f..9f5bea941bcd 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> > @@ -3478,6 +3478,7 @@ static void intel_enable_ddi_dp(struct
>> > intel_encoder *encoder,
>> > 	intel_edp_backlight_on(crtc_state, conn_state);
>> > 	intel_psr_enable(intel_dp, crtc_state);
>> > 	intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
>> > +	intel_dp_hdr_metadata_enable(intel_dp, crtc_state, conn_state);
>> > 	intel_edp_drrs_enable(intel_dp, crtc_state);
>> >
>> > 	if (crtc_state->has_audio)
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 7218e159f55d..00da8221eaba 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -4554,6 +4554,85 @@ intel_dp_setup_vsc_sdp(struct intel_dp
>> > *intel_dp,
>> > 			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));  }
>> >
>> > +static int
>> > +intel_dp_setup_hdr_metadata_infoframe_sdp(struct intel_dp
>> > *intel_dp,
>> > +					  const struct intel_crtc_state
>> > *crtc_state,
>> > +					  const struct
>> > drm_connector_state
>>
>> The return value is not handled, you may convert it as void.
>>
>Okay, I'll remove the return values which is not handled from
>intel_dp_setup_hdr_metadata_infoframe_sdp().
>
>> > *conn_state) {
>> > +	struct intel_digital_port *intel_dig_port =
>> > dp_to_dig_port(intel_dp);
>> > +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
>> > >base.base.dev);
>> > +	struct dp_sdp infoframe_sdp = {};
>> > +	struct hdmi_drm_infoframe drm_infoframe = {};
>> > +	const int infoframe_size = HDMI_INFOFRAME_HEADER_SIZE +
>> > HDMI_DRM_INFOFRAME_SIZE;
>> > +	unsigned char buf[HDMI_INFOFRAME_HEADER_SIZE +
>> > HDMI_DRM_INFOFRAME_SIZE];
>> > +	ssize_t len;
>> > +	int ret;
>> > +
>> > +	ret = drm_hdmi_infoframe_set_hdr_metadata(&drm_infoframe,
>> > conn_state);
>> > +	if (ret) {
>> > +		DRM_DEBUG_KMS("couldn't set HDR metadata in
>> > infoframe\n");
>> > +		return ret;
>> > +	}
>> > +
>> > +	len = hdmi_drm_infoframe_pack_only(&drm_infoframe, buf,
>> > sizeof(buf));
>> > +	if (len < 0) {
>> > +		DRM_DEBUG_KMS("buffer size is smaller than hdr metadata
>> > infoframe\n");
>> > +		return (int)len;
>>
>> If made void, this will not be required.
>>
>> > +	}
>> > +
>> > +	if (len != infoframe_size) {
>> > +		DRM_DEBUG_KMS("wrong static hdr metadata size\n");
>> > +		return -EINVAL;
>> > +	}
>> > +
>> > +	/*
>> > +	 * Set up the infoframe sdp packet for HDR static metadata.
>> > +	 * Prepare VSC Header for SU as per DP 1.4a spec,
>> > +	 * Table 2-100 and Table 2-101
>> > +	 */
>> > +
>> > +	/* Packet ID, 00h for non-Audio INFOFRAME */
>> > +	infoframe_sdp.sdp_header.HB0 = 0;
>> > +	/*
>> > +	 * Packet Type 80h + Non-audio INFOFRAME Type value
>> > +	 * HDMI_INFOFRAME_TYPE_DRM: 0x87,
>> > +	 */
>> > +	infoframe_sdp.sdp_header.HB1 = drm_infoframe.type;
>> > +	/*
>> > +	 * Least Significant Eight Bits of (Data Byte Count – 1)
>> > +	 * infoframe_size - 1,
>> > +	 */
>> > +	infoframe_sdp.sdp_header.HB2 = 0x1D;
>> > +	/* INFOFRAME SDP Version Number */
>> > +	infoframe_sdp.sdp_header.HB3 = (0x13 << 2);
>> > +	/* CTA Header Byte 2 (INFOFRAME Version Number) */
>> > +	infoframe_sdp.db[0] = drm_infoframe.version;
>> > +	/* CTA Header Byte 3 (Length of INFOFRAME):
>> > HDMI_DRM_INFOFRAME_SIZE
>> > */
>> > +	infoframe_sdp.db[1] = drm_infoframe.length;
>> > +	/*
>> > +	 * Copy HDMI_DRM_INFOFRAME_SIZE size from a buffer after
>> > +	 * HDMI_INFOFRAME_HEADER_SIZE
>> > +	 */
>> > +	memcpy(&infoframe_sdp.db[2], &buf[HDMI_INFOFRAME_HEADER_SIZE],
>> > +	       HDMI_DRM_INFOFRAME_SIZE);
>> > +
>> > +	if (INTEL_GEN(dev_priv) >= 11)
>> > +		intel_dig_port->write_infoframe(&intel_dig_port->base,
>> > +						crtc_state,
>> > +
>> > 	HDMI_PACKET_TYPE_GAMUT_METADATA,
>> > +						&infoframe_sdp,
>> > +						VIDEO_DIP_GMP_DATA_SIZE
>> > );
>>
>> This new VIDEO_DIP_GMP_DATA_SIZE doesn't seem to be handled in
>> hsw_write_infoframe (hsw_dip_data_size). Can you please check this.
>>
>Okay, I'll add missed handling of VIDEO_DIP_GMP_DATA_SIZE on
>hsw_dip_data_size().
>> > +	else
>> > +		/* Prior to GEN11, Header size: 4 bytes, Data size: 28
>> > bytes */
>> > +		intel_dig_port->write_infoframe(&intel_dig_port->base,
>> > +						crtc_state,
>> > +
>> > 	HDMI_PACKET_TYPE_GAMUT_METADATA,
>> > +						&infoframe_sdp,
>> > +						VIDEO_DIP_DATA_SIZE);
>> > +
>>
>> Also can you update the series to handle state checking also for
>> metadata sent to DP sink.
>>
>Does it mean a similar logic of "intel_read_infoframe(encoder, pipe_config,
>HDMI_INFOFRAME_TYPE_DRM, &pipe_config->infoframes.drm);"
>
>if then, because current DP packet related implementation of i915 does not hold DP
>packets (ex. VSC SDP, HDR Metadata Infoframe SDP...) to the pipe state structure. In
>my opinion, as a diffenent series, first, it would be better to add a storing of
>infoframe packets to pipe_state structure for later comparing call (ex.
>intel_pipe_config_compare() ).
>after then we can compare them.

Yeah that’s the idea. We should be having some kind of state checking for DP SDP data as well.
However, we can get this series in and later work on the DP state checking as a separate activity.

>> > +	return 0;
>> > +}
>> > +
>> > void intel_dp_vsc_enable(struct intel_dp *intel_dp,
>> > 			 const struct intel_crtc_state *crtc_state,
>> > 			 const struct drm_connector_state *conn_state) @@ -
>> > 4565,6 +4644,18 @@ void intel_dp_vsc_enable(struct intel_dp
>> > *intel_dp,
>> > 	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);  }
>> >
>> > +void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
>> > +				  const struct intel_crtc_state
>> > *crtc_state,
>> > +				  const struct drm_connector_state
>> > *conn_state) {
>> > +	if (!conn_state->hdr_output_metadata)
>> > +		return;
>> > +
>> > +	intel_dp_setup_hdr_metadata_infoframe_sdp(intel_dp,
>> > +						  crtc_state,
>> > +						  conn_state);
>> > +}
>> > +
>> > static u8 intel_dp_autotest_link_training(struct intel_dp
>> > *intel_dp)  {
>> > 	int status = 0;
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
>> > b/drivers/gpu/drm/i915/display/intel_dp.h
>> > index b2da7c9998f7..c3593691dd38 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> > @@ -115,6 +115,9 @@ bool intel_dp_needs_vsc_colorimetry(u32
>> > colorspace);  void
>> > intel_dp_vsc_enable(struct intel_dp *intel_dp,
>> > 			 const struct intel_crtc_state *crtc_state,
>> > 			 const struct drm_connector_state *conn_state);
>> > +void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
>> > +				  const struct intel_crtc_state
>> > *crtc_state,
>> > +				  const struct drm_connector_state
>> > *conn_state);
>> > bool intel_digital_port_connected(struct intel_encoder *encoder);
>> >
>> > static inline unsigned int intel_dp_unused_lane_mask(int
>> > lane_count) diff --git
>> > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index
>> > ea2f0fa2402d..92885416d539 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -4645,6 +4645,7 @@ enum {
>> >  * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds
>> > to each byte
>> >  * of the infoframe structure specified by CEA-861. */
>> > #define   VIDEO_DIP_DATA_SIZE	32
>> > +#define   VIDEO_DIP_GMP_DATA_SIZE	36
>> > #define   VIDEO_DIP_VSC_DATA_SIZE	36
>> > #define   VIDEO_DIP_PPS_DATA_SIZE	132
>> > #define VIDEO_DIP_CTL		_MMIO(0x61170)
>> > --
>> > 2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx