All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs
@ 2019-03-06 19:30 Gwan-gyeong Mun
  2019-03-06 19:30 ` [RFC v4 1/6] drm/i915/dp: Add a config function for YCBCR420 outputs Gwan-gyeong Mun
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Gwan-gyeong Mun @ 2019-03-06 19:30 UTC (permalink / raw)
  To: intel-gfx

On Gen 11 platform, to enable resolutions like 5K@120 (or higher) we need
to use DSC (DP 1.4) or YCbCr4:2:0 (DP 1.3 or 1.4) on DP.
In order to support YCbCr4:2:0 on DP we need to program YCBCR 4:2:0
to MSA and VSC SDP.

This patches are RFC patches that add a VSC structure for handling
Pixel Encoding/Colorimetry Formats and program YCBCR 4:2:0 to MSA and VSC SDP.

This is currently not tested, but I wanted to get some inputs on this approach.
The idea of a scaling (RGB -> YCbCr4:4:4 -> YCbCr 4:2:0) is to follow the
same approach used in YCbCr 4:2:0 on HDMI.

v2: Addressed Maarten's review comments, fixed minor coding and block comment
style. And reordered a first patch  ("drm/i915/dp: Support DP ports
YUV 4:2:0 output to GEN11") as a last patch.

v3: Addressed Ville's review comments.
Style fixed with few naming.
If lscon is active, it makes not to call intel_dp_ycbcr420_config() to avoid
to clobber of lspcon_ycbcr420_config() routine.
And it move the 420_only check into the intel_dp_ycbcr420_config().
Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
Because the pipe is running at the full bpp, keep pipe_bpp as RGB even though
YCbCr 4:2:0 output format is used.
Add a link bandwidth computation for YCbCr4:2:0 output format.

v4: Fix uninitialized return value which is reported by Dan Carpenter.

Gwan-gyeong Mun (6):
  drm/i915/dp: Add a config function for YCBCR420 outputs
  drm: Add a VSC structure for handling Pixel Encoding/Colorimetry
    Formats
  drm/i915/dp: Program VSC Header and DB for Pixel Encoding/Colorimetry
    Format
  drm/i915/dp: Add a support of YCBCR 4:2:0 to DP MSA
  drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0
    output
  drm/i915/dp: Support DP ports YUV 4:2:0 output to GEN11

 drivers/gpu/drm/i915/i915_reg.h  |   1 +
 drivers/gpu/drm/i915/intel_ddi.c |   9 ++
 drivers/gpu/drm/i915/intel_dp.c  | 179 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h |   2 +
 include/drm/drm_dp_helper.h      |  17 +++
 5 files changed, 200 insertions(+), 8 deletions(-)

-- 
2.21.0

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

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

* [RFC v4 1/6] drm/i915/dp: Add a config function for YCBCR420 outputs
  2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
@ 2019-03-06 19:30 ` Gwan-gyeong Mun
  2019-03-06 20:52   ` Ville Syrjälä
  2019-03-06 19:30 ` [RFC v4 2/6] drm: Add a VSC structure for handling Pixel Encoding/Colorimetry Formats Gwan-gyeong Mun
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Gwan-gyeong Mun @ 2019-03-06 19:30 UTC (permalink / raw)
  To: intel-gfx

This patch checks a support of YCBCR420 outputs on an encoder level.
If the input mode is YCBCR420-only mode then it prepares DP as an YCBCR420
output, else it continues with RGB output mode.
It set output_format to INTEL_OUTPUT_FORMAT_YCBCR420 in order to using
a pipe scaler as RGB to YCbCr 4:4:4.

v2:
  Addressed review comments from Ville.
  Style fixed with few naming.
  %s/config/crtc_state/
  %s/intel_crtc/crtc/
  If lscon is active, it makes not to call intel_dp_ycbcr420_config()
  to avoid to clobber of lspcon_ycbcr420_config() routine.
  And it move the 420_only check into the intel_dp_ycbcr420_config().

v3: Fix uninitialized return value and it is reported by Dan Carpenter.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 39 ++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e1a051c0fbfe..882afba6e0f1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2104,6 +2104,38 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	return 0;
 }
 
+static int
+intel_dp_ycbcr420_config(struct drm_connector *connector,
+			 struct intel_crtc_state *crtc_state)
+{
+	const struct drm_display_info *info = &connector->display_info;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	int ret;
+
+	if (drm_mode_is_420_only(info, adjusted_mode) &&
+	    connector->ycbcr_420_allowed) {
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+
+		/* YCBCR 420 output conversion needs a scaler */
+		ret = skl_update_scaler_crtc(crtc_state);
+		if (ret) {
+			DRM_DEBUG_KMS("Scaler allocation for output failed\n");
+			goto err;
+		}
+
+		intel_pch_panel_fitting(crtc, crtc_state,
+					DRM_MODE_SCALE_FULLSCREEN);
+	}
+
+	return 0;
+
+err:
+	DRM_ERROR("Can't support YCBCR420 output\n");
+	return ret;
+}
+
 int
 intel_dp_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_state *pipe_config,
@@ -2120,7 +2152,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		to_intel_digital_connector_state(conn_state);
 	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
 					   DP_DPCD_QUIRK_CONSTANT_N);
-	int ret;
+	int ret = 0;
 
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
 		pipe_config->has_pch_encoder = true;
@@ -2128,6 +2160,11 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	if (lspcon->active)
 		lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
+	else
+		ret = intel_dp_ycbcr420_config(&intel_connector->base, pipe_config);
+
+	if (ret)
+		return ret;
 
 	pipe_config->has_drrs = false;
 	if (IS_G4X(dev_priv) || port == PORT_A)
-- 
2.21.0

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

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

* [RFC v4 2/6] drm: Add a VSC structure for handling Pixel Encoding/Colorimetry Formats
  2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
  2019-03-06 19:30 ` [RFC v4 1/6] drm/i915/dp: Add a config function for YCBCR420 outputs Gwan-gyeong Mun
@ 2019-03-06 19:30 ` Gwan-gyeong Mun
  2019-03-06 19:30 ` [RFC v4 3/6] drm/i915/dp: Program VSC Header and DB for Pixel Encoding/Colorimetry Format Gwan-gyeong Mun
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Gwan-gyeong Mun @ 2019-03-06 19:30 UTC (permalink / raw)
  To: intel-gfx

SDP VSC Header and Data Block follow DP 1.4a spec, section 2.2.5.7.5,
chapter "VSC SDP Payload for Pixel Encoding/Colorimetry Format".

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 include/drm/drm_dp_helper.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 97ce790a5b5a..3793bea7b7fe 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1096,6 +1096,23 @@ struct edp_vsc_psr {
 	u8 DB8_31[24]; /* Reserved */
 } __packed;
 
+struct dp_vsc_sdp {
+	struct dp_sdp_header sdp_header;
+	u8 DB0; /* Stereo Interface */
+	u8 DB1; /* 0 - PSR State; 1 - Update RFB; 2 - CRC Valid */
+	u8 DB2; /* CRC value bits 7:0 of the R or Cr component */
+	u8 DB3; /* CRC value bits 15:8 of the R or Cr component */
+	u8 DB4; /* CRC value bits 7:0 of the G or Y component */
+	u8 DB5; /* CRC value bits 15:8 of the G or Y component */
+	u8 DB6; /* CRC value bits 7:0 of the B or Cb component */
+	u8 DB7; /* CRC value bits 15:8 of the B or Cb component */
+	u8 DB8_15[8];  /* Reserved */
+	u8 DB16; /* Pixel Encoding and Colorimetry Formats */
+	u8 DB17; /* Dynamic Range and Component Bit Depth */
+	u8 DB18; /* Content Type */
+	u8 DB19_31[13]; /* Reserved */
+} __packed;
+
 #define EDP_VSC_PSR_STATE_ACTIVE	(1<<0)
 #define EDP_VSC_PSR_UPDATE_RFB		(1<<1)
 #define EDP_VSC_PSR_CRC_VALUES_VALID	(1<<2)
-- 
2.21.0

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

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

* [RFC v4 3/6] drm/i915/dp: Program VSC Header and DB for Pixel Encoding/Colorimetry Format
  2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
  2019-03-06 19:30 ` [RFC v4 1/6] drm/i915/dp: Add a config function for YCBCR420 outputs Gwan-gyeong Mun
  2019-03-06 19:30 ` [RFC v4 2/6] drm: Add a VSC structure for handling Pixel Encoding/Colorimetry Formats Gwan-gyeong Mun
@ 2019-03-06 19:30 ` Gwan-gyeong Mun
  2019-03-06 19:31 ` [RFC v4 4/6] drm/i915/dp: Add a support of YCBCR 4:2:0 to DP MSA Gwan-gyeong Mun
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Gwan-gyeong Mun @ 2019-03-06 19:30 UTC (permalink / raw)
  To: intel-gfx

Function intel_pixel_encoding_setup_vsc handles vsc header and data block
setup for pixel encoding / colorimetry format.

Setup VSC header and data block in function intel_pixel_encoding_setup_vsc
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.

v2:
  Minor style fix. [Maarten]
  Refer to commit ids instead of patchwork. [Maarten]

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  1 +
 drivers/gpu/drm/i915/intel_dp.c  | 73 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  2 +
 3 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7e3b4e8fdf3a..d89a77a20fcd 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3400,6 +3400,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_edp_drrs_enable(intel_dp, crtc_state);
 
 	if (crtc_state->has_audio)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 882afba6e0f1..74f051428fb2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4412,6 +4412,79 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
 	return 0;
 }
 
+static void
+intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
+			       const struct intel_crtc_state *crtc_state)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct dp_vsc_sdp vsc_sdp;
+
+	if (!intel_dp->attached_connector->base.ycbcr_420_allowed)
+		return;
+
+	/* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119 */
+	memset(&vsc_sdp, 0, sizeof(vsc_sdp));
+	vsc_sdp.sdp_header.HB0 = 0;
+	vsc_sdp.sdp_header.HB1 = 0x7;
+
+	/* VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
+	 * Colorimetry Format indication. A DP Source device is allowed
+	 * to indicate the pixel encoding/colorimetry format to the DP Sink
+	 * device with VSC SDP only when the DP Sink device supports it
+	 * (i.e., VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED bit in the register
+	 * DPRX_FEATURE_ENUMERATION_LIST (DPCD Address 02210h, bit 3) is set to 1)
+	 */
+	vsc_sdp.sdp_header.HB2 = 0x5;
+
+	/* VSC SDP supporting 3D stereo, + PSR2, + Pixel Encoding/
+	 * Colorimetry Format indication (HB2 = 05h).
+	 */
+	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
+	 */
+
+	/* Commit id (25edf91501b8 "drm/i915: prepare csc unit for YCBCR420 output")
+	 * uses the BT.709 color space to perform RGB->YCBCR conversion.
+	 */
+	vsc_sdp.DB16 = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/
+	vsc_sdp.DB16 |= 0x1; /* 0x1, ITU-R BT.709 */
+
+	/* For pixel encoding formats YCbCr444, YCbCr422, YCbCr420, and Y Only,
+	 * the following Component Bit Depth values are defined:
+	 * 001b = 8bpc.
+	 * 010b = 10bpc.
+	 * 011b = 12bpc.
+	 * 100b = 16bpc.
+	 */
+	vsc_sdp.DB17 = 0x1;
+
+	/*
+	 * Content Type (Bits 2:0)
+	 * 000b = Not defined.
+	 * 001b = Graphics.
+	 * 010b = Photo.
+	 * 011b = Video.
+	 * 100b = Game
+	 * All other values are RESERVED.
+	 * Note: See CTA-861-G for the definition and expected
+	 * processing by a stream sink for the above contect types.
+	 */
+	vsc_sdp.DB18 = 0;
+
+	intel_dig_port->write_infoframe(&intel_dig_port->base,
+			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)
+{
+	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
+		return;
+
+	intel_pixel_encoding_setup_vsc(intel_dp, crtc_state);
+}
+
 static u8 intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 {
 	int status = 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5412373e2f98..b029aa5ab35a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1977,6 +1977,8 @@ u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
 				int mode_clock, int mode_hdisplay);
 u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, int mode_clock,
 				int mode_hdisplay);
+void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp,
+			       const struct intel_crtc_state *crtc_state);
 
 /* intel_vdsc.c */
 int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
-- 
2.21.0

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

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

* [RFC v4 4/6] drm/i915/dp: Add a support of YCBCR 4:2:0 to DP MSA
  2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
                   ` (2 preceding siblings ...)
  2019-03-06 19:30 ` [RFC v4 3/6] drm/i915/dp: Program VSC Header and DB for Pixel Encoding/Colorimetry Format Gwan-gyeong Mun
@ 2019-03-06 19:31 ` Gwan-gyeong Mun
  2019-03-06 19:31 ` [RFC v4 5/6] drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0 output Gwan-gyeong Mun
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Gwan-gyeong Mun @ 2019-03-06 19:31 UTC (permalink / raw)
  To: intel-gfx

When YCBCR 4:2:0 outputs is used for DP, we should program YCBCR 4:2:0 to
MSA and 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 YCBCR 420 signals
we should program MSA MISC1 fields which indicate VSC SDP for the Pixel
Encoding/Colorimetry Format.

v2: Block comment style fix.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 1 +
 drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 16ce9c609c65..7076207c90e2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9474,6 +9474,7 @@ enum skl_power_gate {
 #define  TRANS_MSA_12_BPC		(3 << 5)
 #define  TRANS_MSA_16_BPC		(4 << 5)
 #define  TRANS_MSA_CEA_RANGE		(1 << 3)
+#define  TRANS_MSA_USE_VSC_SDP		(1 << 13)
 
 /* LCPLL Control */
 #define LCPLL_CTL			_MMIO(0x130040)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d89a77a20fcd..4be076774b84 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1732,6 +1732,14 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
 	 */
 	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
 		temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
+	/*
+	 * 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.
+	 */
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		temp |= TRANS_MSA_USE_VSC_SDP;
 	I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
 }
 
-- 
2.21.0

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

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

* [RFC v4 5/6] drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0 output
  2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
                   ` (3 preceding siblings ...)
  2019-03-06 19:31 ` [RFC v4 4/6] drm/i915/dp: Add a support of YCBCR 4:2:0 to DP MSA Gwan-gyeong Mun
@ 2019-03-06 19:31 ` Gwan-gyeong Mun
  2019-03-06 21:04   ` Ville Syrjälä
  2019-03-06 19:31 ` [RFC v4 6/6] drm/i915/dp: Support DP ports YUV 4:2:0 output to GEN11 Gwan-gyeong Mun
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Gwan-gyeong Mun @ 2019-03-06 19:31 UTC (permalink / raw)
  To: intel-gfx

All of the link bandwidth and Data M/N calculations were assumed a bpp as
RGB format. But When we are using YCbCr 4:2:0 output format on DP,
we should change bpp calculations as YCbCr 4:2:0 format.
The pipe_bpp value was assumed RGB format, therefore, it was multiplied
with 3. But YCbCr 4:2:0 requires a multiplier value to 1.5.
Therefore we need to divide pipe_bpp to 2 while DP output uses YCbCr4:2:0
format.
 - RGB format bpp = bpc x 3
 - YCbCr 4:2:0 format bpp = bpc x 1.5

And it adds missed bpc values for a programming of VSC Header.
It only affects dp and edp port which use YCbCr 4:2:0 output format.
And for now, it does not consider a use case of DSC + YCbCr 4:2:0.

v2:
  Addressed review comments from Ville.
  Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
  Because the pipe is running at the full bpp, keep pipe_bpp as RGB
  even though YCbCr 4:2:0 output format is used.
  Add a link bandwidth computation for YCbCr4:2:0 output format.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 74f051428fb2..c6826cda8c5f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1723,7 +1723,8 @@ void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
 struct link_config_limits {
 	int min_clock, max_clock;
 	int min_lane_count, max_lane_count;
-	int min_bpp, max_bpp;
+	int min_bpp, max_bpp, bpp_step;
+	bool is_ycbcr420;
 };
 
 static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp,
@@ -1833,7 +1834,7 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 	int bpp, clock, lane_count;
 	int mode_rate, link_clock, link_avail;
 
-	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
+	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= limits->bpp_step) {
 		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
 						   bpp);
 
@@ -1847,7 +1848,10 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 
 				if (mode_rate <= link_avail) {
 					pipe_config->lane_count = lane_count;
-					pipe_config->pipe_bpp = bpp;
+					if (limits->is_ycbcr420)
+						pipe_config->pipe_bpp = bpp * 2;
+					else
+						pipe_config->pipe_bpp = bpp;
 					pipe_config->port_clock = link_clock;
 
 					return 0;
@@ -1869,7 +1873,7 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
 	int bpp, clock, lane_count;
 	int mode_rate, link_clock, link_avail;
 
-	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
+	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= limits->bpp_step) {
 		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
 						   bpp);
 
@@ -1883,7 +1887,10 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
 
 				if (mode_rate <= link_avail) {
 					pipe_config->lane_count = lane_count;
-					pipe_config->pipe_bpp = bpp;
+					if (limits->is_ycbcr420)
+						pipe_config->pipe_bpp = bpp * 2;
+					else
+						pipe_config->pipe_bpp = bpp;
 					pipe_config->port_clock = link_clock;
 
 					return 0;
@@ -2015,6 +2022,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	struct link_config_limits limits;
 	int common_len;
 	int ret;
+	bool is_ycbcr420 =
+		pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ? true : false;
 
 	common_len = intel_dp_common_len_rate_limit(intel_dp,
 						    intel_dp->max_link_rate);
@@ -2030,6 +2039,20 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 
 	limits.min_bpp = 6 * 3;
 	limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
+	limits.bpp_step = 2 * 3;
+
+	limits.is_ycbcr420 = is_ycbcr420;
+
+	if (is_ycbcr420) {
+		/*
+		 * bpp value was assumed to RGB therefore it was multiplied
+		 * with 3. But YCbCr 4:2:0 requires multiplier value to 1.5
+		 * therefore it divides pipe_bpp to 2.
+		 */
+		limits.min_bpp /= 2;
+		limits.max_bpp /= 2;
+		limits.bpp_step /= 2;
+	}
 
 	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
 		/*
@@ -2224,13 +2247,23 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
 	}
 
-	if (!pipe_config->dsc_params.compression_enable)
-		intel_link_compute_m_n(pipe_config->pipe_bpp,
+	if (!pipe_config->dsc_params.compression_enable) {
+		/*
+		 * Basically, pipe_bpp value is assumed to RGB.
+		 * And on the calculation of Data M and Data N, YCbCr 4:2:0
+		 * output format of the number of bytes per pixel will be half
+		 * the number of bytes of RGB pixel.
+		 */
+		int pipe_bpp = pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ?
+			pipe_config->pipe_bpp / 2 : pipe_config->pipe_bpp;
+
+		intel_link_compute_m_n(pipe_bpp,
 				       pipe_config->lane_count,
 				       adjusted_mode->crtc_clock,
 				       pipe_config->port_clock,
 				       &pipe_config->dp_m_n,
 				       constant_n);
+	}
 	else
 		intel_link_compute_m_n(pipe_config->dsc_params.compressed_bpp,
 				       pipe_config->lane_count,
@@ -4458,6 +4491,23 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
 	 * 100b = 16bpc.
 	 */
 	vsc_sdp.DB17 = 0x1;
+	switch (crtc_state->pipe_bpp / 3) {
+	case 8: /* 8bpc */
+		vsc_sdp.DB17 = 0x1;
+		break;
+	case 10: /* 10bpc */
+		vsc_sdp.DB17 = 0x2;
+		break;
+	case 12: /* 12bpc */
+		vsc_sdp.DB17 = 0x3;
+		break;
+	case 16: /* 16bpc */
+		vsc_sdp.DB17 = 0x4;
+		break;
+	default:
+		DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state->pipe_bpp);
+		break;
+	}
 
 	/*
 	 * Content Type (Bits 2:0)
-- 
2.21.0

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

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

* [RFC v4 6/6] drm/i915/dp: Support DP ports YUV 4:2:0 output to GEN11
  2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
                   ` (4 preceding siblings ...)
  2019-03-06 19:31 ` [RFC v4 5/6] drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0 output Gwan-gyeong Mun
@ 2019-03-06 19:31 ` Gwan-gyeong Mun
  2019-03-06 20:12 ` ✓ Fi.CI.BAT: success for drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs (rev4) Patchwork
  2019-03-06 22:03 ` ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 13+ messages in thread
From: Gwan-gyeong Mun @ 2019-03-06 19:31 UTC (permalink / raw)
  To: intel-gfx

Bspec describes that GEN10 only supports capability of YUV 4:2:0 output to
HDMI port and GEN11 supports capability of YUV 4:2:0 output to both DP and
HDMI ports.

v2: Minor style fix.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c6826cda8c5f..8fc9cb939533 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -7435,6 +7435,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		connector->interlace_allowed = true;
 	connector->doublescan_allowed = 0;
 
+	if (INTEL_GEN(dev_priv) >= 11)
+		connector->ycbcr_420_allowed = true;
+
 	intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
 
 	intel_dp_aux_init(intel_dp);
-- 
2.21.0

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

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

* ✓ Fi.CI.BAT: success for drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs (rev4)
  2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
                   ` (5 preceding siblings ...)
  2019-03-06 19:31 ` [RFC v4 6/6] drm/i915/dp: Support DP ports YUV 4:2:0 output to GEN11 Gwan-gyeong Mun
@ 2019-03-06 20:12 ` Patchwork
  2019-03-06 22:03 ` ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-03-06 20:12 UTC (permalink / raw)
  To: Gwan-gyeong Mun; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs (rev4)
URL   : https://patchwork.freedesktop.org/series/56059/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5712 -> Patchwork_12397
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56059/revisions/4/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@readonly-bsd2:
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] +76

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       PASS -> SKIP [fdo#109271]

  * igt@i915_pm_rpm@basic-rte:
    - fi-bsw-kefka:       PASS -> FAIL [fdo#108800]

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       PASS -> DMESG-WARN [fdo#107709]

  * igt@kms_busy@basic-flip-c:
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          PASS -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> FAIL [fdo#107709]

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-7560u:       CRASH -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-kbl-7560u:       INCOMPLETE [fdo#108044] / [fdo#108744] -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108044]: https://bugs.freedesktop.org/show_bug.cgi?id=108044
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (45 -> 40)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-cfl-guc fi-ctg-p8600 


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

    * Linux: CI_DRM_5712 -> Patchwork_12397

  CI_DRM_5712: 5d4de376b9a03c2f74e049ee6a8221df96687ba0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4875: 91908d36d0d5c90eea86e29736d2748d5ec55335 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12397: 8de1624bb2ad3bd1ca1f4f29849c78f594591778 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

8de1624bb2ad drm/i915/dp: Support DP ports YUV 4:2:0 output to GEN11
3ce464d81e62 drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0 output
c116a1f0d6f3 drm/i915/dp: Add a support of YCBCR 4:2:0 to DP MSA
a3c55fec3da1 drm/i915/dp: Program VSC Header and DB for Pixel Encoding/Colorimetry Format
f9fd6ebb5127 drm: Add a VSC structure for handling Pixel Encoding/Colorimetry Formats
294637783bfb drm/i915/dp: Add a config function for YCBCR420 outputs

== Logs ==

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

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

* Re: [RFC v4 1/6] drm/i915/dp: Add a config function for YCBCR420 outputs
  2019-03-06 19:30 ` [RFC v4 1/6] drm/i915/dp: Add a config function for YCBCR420 outputs Gwan-gyeong Mun
@ 2019-03-06 20:52   ` Ville Syrjälä
  2019-03-07 20:29     ` Mun, Gwan-gyeong
  0 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2019-03-06 20:52 UTC (permalink / raw)
  To: Gwan-gyeong Mun; +Cc: intel-gfx

On Wed, Mar 06, 2019 at 09:30:57PM +0200, Gwan-gyeong Mun wrote:
> This patch checks a support of YCBCR420 outputs on an encoder level.
> If the input mode is YCBCR420-only mode then it prepares DP as an YCBCR420
> output, else it continues with RGB output mode.
> It set output_format to INTEL_OUTPUT_FORMAT_YCBCR420 in order to using
> a pipe scaler as RGB to YCbCr 4:4:4.
> 
> v2:
>   Addressed review comments from Ville.
>   Style fixed with few naming.
>   %s/config/crtc_state/
>   %s/intel_crtc/crtc/
>   If lscon is active, it makes not to call intel_dp_ycbcr420_config()
>   to avoid to clobber of lspcon_ycbcr420_config() routine.
>   And it move the 420_only check into the intel_dp_ycbcr420_config().
> 
> v3: Fix uninitialized return value and it is reported by Dan Carpenter.
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 39 ++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e1a051c0fbfe..882afba6e0f1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2104,6 +2104,38 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	return 0;
>  }
>  
> +static int
> +intel_dp_ycbcr420_config(struct drm_connector *connector,
> +			 struct intel_crtc_state *crtc_state)
> +{
> +	const struct drm_display_info *info = &connector->display_info;
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->base.adjusted_mode;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	int ret;
> +
> +	if (drm_mode_is_420_only(info, adjusted_mode) &&
> +	    connector->ycbcr_420_allowed) {

I would invert this into an early return. That way we avoid the
extra indentation for the rest of the function.

> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> +
> +		/* YCBCR 420 output conversion needs a scaler */
> +		ret = skl_update_scaler_crtc(crtc_state);
> +		if (ret) {
> +			DRM_DEBUG_KMS("Scaler allocation for output failed\n");
> +			goto err;
> +		}
> +
> +		intel_pch_panel_fitting(crtc, crtc_state,
> +					DRM_MODE_SCALE_FULLSCREEN);
> +	}
> +
> +	return 0;
> +
> +err:
> +	DRM_ERROR("Can't support YCBCR420 output\n");

Remove the error print. That is user triggerable which means
no errors prints allowed. And we already have the debug print.

> +	return ret;
> +}
> +
>  int
>  intel_dp_compute_config(struct intel_encoder *encoder,
>  			struct intel_crtc_state *pipe_config,
> @@ -2120,7 +2152,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		to_intel_digital_connector_state(conn_state);
>  	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
>  					   DP_DPCD_QUIRK_CONSTANT_N);
> -	int ret;
> +	int ret = 0;
>  
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
>  		pipe_config->has_pch_encoder = true;
> @@ -2128,6 +2160,11 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	if (lspcon->active)
>  		lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
> +	else
> +		ret = intel_dp_ycbcr420_config(&intel_connector->base, pipe_config);
> +
> +	if (ret)
> +		return ret;
>  
>  	pipe_config->has_drrs = false;
>  	if (IS_G4X(dev_priv) || port == PORT_A)
> -- 
> 2.21.0

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

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

* Re: [RFC v4 5/6] drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0 output
  2019-03-06 19:31 ` [RFC v4 5/6] drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0 output Gwan-gyeong Mun
@ 2019-03-06 21:04   ` Ville Syrjälä
  2019-03-07 20:31     ` Mun, Gwan-gyeong
  0 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2019-03-06 21:04 UTC (permalink / raw)
  To: Gwan-gyeong Mun; +Cc: intel-gfx

On Wed, Mar 06, 2019 at 09:31:01PM +0200, Gwan-gyeong Mun wrote:
> All of the link bandwidth and Data M/N calculations were assumed a bpp as
> RGB format. But When we are using YCbCr 4:2:0 output format on DP,
> we should change bpp calculations as YCbCr 4:2:0 format.
> The pipe_bpp value was assumed RGB format, therefore, it was multiplied
> with 3. But YCbCr 4:2:0 requires a multiplier value to 1.5.
> Therefore we need to divide pipe_bpp to 2 while DP output uses YCbCr4:2:0
> format.
>  - RGB format bpp = bpc x 3
>  - YCbCr 4:2:0 format bpp = bpc x 1.5
> 
> And it adds missed bpc values for a programming of VSC Header.
> It only affects dp and edp port which use YCbCr 4:2:0 output format.
> And for now, it does not consider a use case of DSC + YCbCr 4:2:0.
> 
> v2:
>   Addressed review comments from Ville.
>   Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
>   Because the pipe is running at the full bpp, keep pipe_bpp as RGB
>   even though YCbCr 4:2:0 output format is used.
>   Add a link bandwidth computation for YCbCr4:2:0 output format.
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 64 +++++++++++++++++++++++++++++----
>  1 file changed, 57 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 74f051428fb2..c6826cda8c5f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1723,7 +1723,8 @@ void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>  struct link_config_limits {
>  	int min_clock, max_clock;
>  	int min_lane_count, max_lane_count;
> -	int min_bpp, max_bpp;
> +	int min_bpp, max_bpp, bpp_step;
> +	bool is_ycbcr420;
>  };
>  
>  static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp,
> @@ -1833,7 +1834,7 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  	int bpp, clock, lane_count;
>  	int mode_rate, link_clock, link_avail;
>  
> -	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
> +	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= limits->bpp_step) {
>  		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
>  						   bpp);
>  
> @@ -1847,7 +1848,10 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  
>  				if (mode_rate <= link_avail) {
>  					pipe_config->lane_count = lane_count;
> -					pipe_config->pipe_bpp = bpp;
> +					if (limits->is_ycbcr420)
> +						pipe_config->pipe_bpp = bpp * 2;
> +					else
> +						pipe_config->pipe_bpp = bpp;

This approach feels needlessly complicated.

Can't we just do something like this?

static int intel_dp_output_bpp(const struct intel_crtc_state *crtc_state, int bpp)
{
	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
		bpp /= 2;
 	return bpp;
}

intel_dp_compute_link_config_*(struct intel_dp *intel_dp,
{
...
	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
+		int output_bpp = intel_dp_output_bpp(pipe_config, bpp);
+
                mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
-						   bpp);
+						   output_bpp);


>  					pipe_config->port_clock = link_clock;
>  
>  					return 0;
> @@ -1869,7 +1873,7 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
>  	int bpp, clock, lane_count;
>  	int mode_rate, link_clock, link_avail;
>  
> -	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
> +	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= limits->bpp_step) {
>  		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
>  						   bpp);
>  
> @@ -1883,7 +1887,10 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
>  
>  				if (mode_rate <= link_avail) {
>  					pipe_config->lane_count = lane_count;
> -					pipe_config->pipe_bpp = bpp;
> +					if (limits->is_ycbcr420)
> +						pipe_config->pipe_bpp = bpp * 2;
> +					else
> +						pipe_config->pipe_bpp = bpp;
>  					pipe_config->port_clock = link_clock;
>  
>  					return 0;
> @@ -2015,6 +2022,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	struct link_config_limits limits;
>  	int common_len;
>  	int ret;
> +	bool is_ycbcr420 =
> +		pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ? true : false;
>  
>  	common_len = intel_dp_common_len_rate_limit(intel_dp,
>  						    intel_dp->max_link_rate);
> @@ -2030,6 +2039,20 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  
>  	limits.min_bpp = 6 * 3;
>  	limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
> +	limits.bpp_step = 2 * 3;
> +
> +	limits.is_ycbcr420 = is_ycbcr420;
> +
> +	if (is_ycbcr420) {
> +		/*
> +		 * bpp value was assumed to RGB therefore it was multiplied
> +		 * with 3. But YCbCr 4:2:0 requires multiplier value to 1.5
> +		 * therefore it divides pipe_bpp to 2.
> +		 */
> +		limits.min_bpp /= 2;
> +		limits.max_bpp /= 2;
> +		limits.bpp_step /= 2;
> +	}
>  
>  	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
>  		/*
> @@ -2224,13 +2247,23 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
>  	}
>  
> -	if (!pipe_config->dsc_params.compression_enable)
> -		intel_link_compute_m_n(pipe_config->pipe_bpp,
> +	if (!pipe_config->dsc_params.compression_enable) {
> +		/*
> +		 * Basically, pipe_bpp value is assumed to RGB.
> +		 * And on the calculation of Data M and Data N, YCbCr 4:2:0
> +		 * output format of the number of bytes per pixel will be half
> +		 * the number of bytes of RGB pixel.
> +		 */
> +		int pipe_bpp = pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ?
> +			pipe_config->pipe_bpp / 2 : pipe_config->pipe_bpp;
> +
> +		intel_link_compute_m_n(pipe_bpp,
>  				       pipe_config->lane_count,
>  				       adjusted_mode->crtc_clock,
>  				       pipe_config->port_clock,
>  				       &pipe_config->dp_m_n,
>  				       constant_n);
> +	}
>  	else
>  		intel_link_compute_m_n(pipe_config->dsc_params.compressed_bpp,
>  				       pipe_config->lane_count,
> @@ -4458,6 +4491,23 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
>  	 * 100b = 16bpc.
>  	 */
>  	vsc_sdp.DB17 = 0x1;
> +	switch (crtc_state->pipe_bpp / 3) {
> +	case 8: /* 8bpc */
> +		vsc_sdp.DB17 = 0x1;
> +		break;
> +	case 10: /* 10bpc */
> +		vsc_sdp.DB17 = 0x2;
> +		break;
> +	case 12: /* 12bpc */
> +		vsc_sdp.DB17 = 0x3;
> +		break;
> +	case 16: /* 16bpc */
> +		vsc_sdp.DB17 = 0x4;
> +		break;
> +	default:
> +		DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state->pipe_bpp);
> +		break;
> +	}
>  
>  	/*
>  	 * Content Type (Bits 2:0)
> -- 
> 2.21.0

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

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

* ✓ Fi.CI.IGT: success for drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs (rev4)
  2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
                   ` (6 preceding siblings ...)
  2019-03-06 20:12 ` ✓ Fi.CI.BAT: success for drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs (rev4) Patchwork
@ 2019-03-06 22:03 ` Patchwork
  7 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-03-06 22:03 UTC (permalink / raw)
  To: Gwan-gyeong Mun; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs (rev4)
URL   : https://patchwork.freedesktop.org/series/56059/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5712_full -> Patchwork_12397_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_param@invalid-param-get:
    - shard-skl:          NOTRUN -> FAIL [fdo#109559]

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773] / [fdo#107807]

  * igt@kms_atomic_transition@1x-modeset-transitions:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108470]

  * igt@kms_atomic_transition@5x-modeset-transitions:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +8

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956] +1

  * igt@kms_color@pipe-b-degamma:
    - shard-skl:          NOTRUN -> FAIL [fdo#104782]

  * igt@kms_color@pipe-c-legacy-gamma:
    - shard-glk:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] +1

  * igt@kms_cursor_legacy@2x-cursor-vs-flip-atomic:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#102614] +1

  * igt@kms_fbcon_fbt@psr:
    - shard-skl:          NOTRUN -> FAIL [fdo#103833]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          NOTRUN -> FAIL [fdo#105363]

  * igt@kms_flip_tiling@flip-changes-tiling-y:
    - shard-skl:          PASS -> FAIL [fdo#107931] / [fdo#108303]

  * igt@kms_flip_tiling@flip-to-yf-tiled:
    - shard-skl:          NOTRUN -> FAIL [fdo#107931]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-glk:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-blt:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] +28

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-indfb-msflip-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +77

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-move:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +88

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
    - shard-skl:          NOTRUN -> FAIL [fdo#107362]

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +2

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +6

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> FAIL [fdo#109016]

  
#### Possible fixes ####

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +6

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +7

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +4

  
#### Warnings ####

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP [fdo#109271]

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103833]: https://bugs.freedesktop.org/show_bug.cgi?id=103833
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107931]: https://bugs.freedesktop.org/show_bug.cgi?id=107931
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
  [fdo#108470]: https://bugs.freedesktop.org/show_bug.cgi?id=108470
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109559]: https://bugs.freedesktop.org/show_bug.cgi?id=109559


Participating hosts (6 -> 6)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5712 -> Patchwork_12397

  CI_DRM_5712: 5d4de376b9a03c2f74e049ee6a8221df96687ba0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4875: 91908d36d0d5c90eea86e29736d2748d5ec55335 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12397: 8de1624bb2ad3bd1ca1f4f29849c78f594591778 @ 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_12397/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC v4 1/6] drm/i915/dp: Add a config function for YCBCR420 outputs
  2019-03-06 20:52   ` Ville Syrjälä
@ 2019-03-07 20:29     ` Mun, Gwan-gyeong
  0 siblings, 0 replies; 13+ messages in thread
From: Mun, Gwan-gyeong @ 2019-03-07 20:29 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, 2019-03-06 at 22:52 +0200, Ville Syrjälä wrote:
> On Wed, Mar 06, 2019 at 09:30:57PM +0200, Gwan-gyeong Mun wrote:
> > This patch checks a support of YCBCR420 outputs on an encoder
> > level.
> > If the input mode is YCBCR420-only mode then it prepares DP as an
> > YCBCR420
> > output, else it continues with RGB output mode.
> > It set output_format to INTEL_OUTPUT_FORMAT_YCBCR420 in order to
> > using
> > a pipe scaler as RGB to YCbCr 4:4:4.
> > 
> > v2:
> >   Addressed review comments from Ville.
> >   Style fixed with few naming.
> >   %s/config/crtc_state/
> >   %s/intel_crtc/crtc/
> >   If lscon is active, it makes not to call
> > intel_dp_ycbcr420_config()
> >   to avoid to clobber of lspcon_ycbcr420_config() routine.
> >   And it move the 420_only check into the
> > intel_dp_ycbcr420_config().
> > 
> > v3: Fix uninitialized return value and it is reported by Dan
> > Carpenter.
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 39
> > ++++++++++++++++++++++++++++++++-
> >  1 file changed, 38 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index e1a051c0fbfe..882afba6e0f1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2104,6 +2104,38 @@ intel_dp_compute_link_config(struct
> > intel_encoder *encoder,
> >  	return 0;
> >  }
> >  
> > +static int
> > +intel_dp_ycbcr420_config(struct drm_connector *connector,
> > +			 struct intel_crtc_state *crtc_state)
> > +{
> > +	const struct drm_display_info *info = &connector->display_info;
> > +	const struct drm_display_mode *adjusted_mode =
> > +		&crtc_state->base.adjusted_mode;
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > +	int ret;
> > +
> > +	if (drm_mode_is_420_only(info, adjusted_mode) &&
> > +	    connector->ycbcr_420_allowed) {
> 
> I would invert this into an early return. That way we avoid the
> extra indentation for the rest of the function.
> 
I agree. In order to avoid the extra indentation, I'll invert if-clause 
on intel_dp_ycbcr420_config().
> > +		crtc_state->output_format =
> > INTEL_OUTPUT_FORMAT_YCBCR420;
> > +
> > +		/* YCBCR 420 output conversion needs a scaler */
> > +		ret = skl_update_scaler_crtc(crtc_state);
> > +		if (ret) {
> > +			DRM_DEBUG_KMS("Scaler allocation for output
> > failed\n");
> > +			goto err;
> > +		}
> > +
> > +		intel_pch_panel_fitting(crtc, crtc_state,
> > +					DRM_MODE_SCALE_FULLSCREEN);
> > +	}
> > +
> > +	return 0;
> > +
> > +err:
> > +	DRM_ERROR("Can't support YCBCR420 output\n");
> 
> Remove the error print. That is user triggerable which means
> no errors prints allowed. And we already have the debug print.
> 
Thank you for your kind explanation. I'll remove the error print from
here.
> > +	return ret;
> > +}
> > +
> >  int
> >  intel_dp_compute_config(struct intel_encoder *encoder,
> >  			struct intel_crtc_state *pipe_config,
> > @@ -2120,7 +2152,7 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >  		to_intel_digital_connector_state(conn_state);
> >  	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> >  					   DP_DPCD_QUIRK_CONSTANT_N);
> > -	int ret;
> > +	int ret = 0;
> >  
> >  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port !=
> > PORT_A)
> >  		pipe_config->has_pch_encoder = true;
> > @@ -2128,6 +2160,11 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> >  	if (lspcon->active)
> >  		lspcon_ycbcr420_config(&intel_connector->base,
> > pipe_config);
> > +	else
> > +		ret = intel_dp_ycbcr420_config(&intel_connector->base,
> > pipe_config);
> > +
> > +	if (ret)
> > +		return ret;
> >  
> >  	pipe_config->has_drrs = false;
> >  	if (IS_G4X(dev_priv) || port == PORT_A)
> > -- 
> > 2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC v4 5/6] drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0 output
  2019-03-06 21:04   ` Ville Syrjälä
@ 2019-03-07 20:31     ` Mun, Gwan-gyeong
  0 siblings, 0 replies; 13+ messages in thread
From: Mun, Gwan-gyeong @ 2019-03-07 20:31 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, 2019-03-06 at 23:04 +0200, Ville Syrjälä wrote:
> On Wed, Mar 06, 2019 at 09:31:01PM +0200, Gwan-gyeong Mun wrote:
> > All of the link bandwidth and Data M/N calculations were assumed a
> > bpp as
> > RGB format. But When we are using YCbCr 4:2:0 output format on DP,
> > we should change bpp calculations as YCbCr 4:2:0 format.
> > The pipe_bpp value was assumed RGB format, therefore, it was
> > multiplied
> > with 3. But YCbCr 4:2:0 requires a multiplier value to 1.5.
> > Therefore we need to divide pipe_bpp to 2 while DP output uses
> > YCbCr4:2:0
> > format.
> >  - RGB format bpp = bpc x 3
> >  - YCbCr 4:2:0 format bpp = bpc x 1.5
> > 
> > And it adds missed bpc values for a programming of VSC Header.
> > It only affects dp and edp port which use YCbCr 4:2:0 output
> > format.
> > And for now, it does not consider a use case of DSC + YCbCr 4:2:0.
> > 
> > v2:
> >   Addressed review comments from Ville.
> >   Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
> >   Because the pipe is running at the full bpp, keep pipe_bpp as RGB
> >   even though YCbCr 4:2:0 output format is used.
> >   Add a link bandwidth computation for YCbCr4:2:0 output format.
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 64
> > +++++++++++++++++++++++++++++----
> >  1 file changed, 57 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 74f051428fb2..c6826cda8c5f 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1723,7 +1723,8 @@ void intel_dp_compute_rate(struct intel_dp
> > *intel_dp, int port_clock,
> >  struct link_config_limits {
> >  	int min_clock, max_clock;
> >  	int min_lane_count, max_lane_count;
> > -	int min_bpp, max_bpp;
> > +	int min_bpp, max_bpp, bpp_step;
> > +	bool is_ycbcr420;
> >  };
> >  
> >  static bool intel_dp_source_supports_fec(struct intel_dp
> > *intel_dp,
> > @@ -1833,7 +1834,7 @@ intel_dp_compute_link_config_wide(struct
> > intel_dp *intel_dp,
> >  	int bpp, clock, lane_count;
> >  	int mode_rate, link_clock, link_avail;
> >  
> > -	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 *
> > 3) {
> > +	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -=
> > limits->bpp_step) {
> >  		mode_rate = intel_dp_link_required(adjusted_mode-
> > >crtc_clock,
> >  						   bpp);
> >  
> > @@ -1847,7 +1848,10 @@ intel_dp_compute_link_config_wide(struct
> > intel_dp *intel_dp,
> >  
> >  				if (mode_rate <= link_avail) {
> >  					pipe_config->lane_count =
> > lane_count;
> > -					pipe_config->pipe_bpp = bpp;
> > +					if (limits->is_ycbcr420)
> > +						pipe_config->pipe_bpp =
> > bpp * 2;
> > +					else
> > +						pipe_config->pipe_bpp =
> > bpp;
> 
> This approach feels needlessly complicated.
> 
> Can't we just do something like this?
> 
> static int intel_dp_output_bpp(const struct intel_crtc_state
> *crtc_state, int bpp)
> {
> 	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> 		bpp /= 2;
>  	return bpp;
> }
> 
> intel_dp_compute_link_config_*(struct intel_dp *intel_dp,
> {
> ...
> 	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 *
> 3) {
> +		int output_bpp = intel_dp_output_bpp(pipe_config, bpp);
> +
>                 mode_rate = intel_dp_link_required(adjusted_mode-
> >crtc_clock,
> -						   bpp);
> +						   output_bpp);
> 
I agree. Thank you for guiding me.
In order to make codes simple, I'll add and use intel_dp_output_bpp()
function.
> 
> >  					pipe_config->port_clock =
> > link_clock;
> >  
> >  					return 0;
> > @@ -1869,7 +1873,7 @@ intel_dp_compute_link_config_fast(struct
> > intel_dp *intel_dp,
> >  	int bpp, clock, lane_count;
> >  	int mode_rate, link_clock, link_avail;
> >  
> > -	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 *
> > 3) {
> > +	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -=
> > limits->bpp_step) {
> >  		mode_rate = intel_dp_link_required(adjusted_mode-
> > >crtc_clock,
> >  						   bpp);
> >  
> > @@ -1883,7 +1887,10 @@ intel_dp_compute_link_config_fast(struct
> > intel_dp *intel_dp,
> >  
> >  				if (mode_rate <= link_avail) {
> >  					pipe_config->lane_count =
> > lane_count;
> > -					pipe_config->pipe_bpp = bpp;
> > +					if (limits->is_ycbcr420)
> > +						pipe_config->pipe_bpp =
> > bpp * 2;
> > +					else
> > +						pipe_config->pipe_bpp =
> > bpp;
> >  					pipe_config->port_clock =
> > link_clock;
> >  
> >  					return 0;
> > @@ -2015,6 +2022,8 @@ intel_dp_compute_link_config(struct
> > intel_encoder *encoder,
> >  	struct link_config_limits limits;
> >  	int common_len;
> >  	int ret;
> > +	bool is_ycbcr420 =
> > +		pipe_config->output_format ==
> > INTEL_OUTPUT_FORMAT_YCBCR420 ? true : false;
> >  
> >  	common_len = intel_dp_common_len_rate_limit(intel_dp,
> >  						    intel_dp-
> > >max_link_rate);
> > @@ -2030,6 +2039,20 @@ intel_dp_compute_link_config(struct
> > intel_encoder *encoder,
> >  
> >  	limits.min_bpp = 6 * 3;
> >  	limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
> > +	limits.bpp_step = 2 * 3;
> > +
> > +	limits.is_ycbcr420 = is_ycbcr420;
> > +
> > +	if (is_ycbcr420) {
> > +		/*
> > +		 * bpp value was assumed to RGB therefore it was
> > multiplied
> > +		 * with 3. But YCbCr 4:2:0 requires multiplier value to
> > 1.5
> > +		 * therefore it divides pipe_bpp to 2.
> > +		 */
> > +		limits.min_bpp /= 2;
> > +		limits.max_bpp /= 2;
> > +		limits.bpp_step /= 2;
> > +	}
> >  
> >  	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] <
> > DP_EDP_14) {
> >  		/*
> > @@ -2224,13 +2247,23 @@ intel_dp_compute_config(struct
> > intel_encoder *encoder,
> >  			intel_conn_state->broadcast_rgb ==
> > INTEL_BROADCAST_RGB_LIMITED;
> >  	}
> >  
> > -	if (!pipe_config->dsc_params.compression_enable)
> > -		intel_link_compute_m_n(pipe_config->pipe_bpp,
> > +	if (!pipe_config->dsc_params.compression_enable) {
> > +		/*
> > +		 * Basically, pipe_bpp value is assumed to RGB.
> > +		 * And on the calculation of Data M and Data N, YCbCr
> > 4:2:0
> > +		 * output format of the number of bytes per pixel will
> > be half
> > +		 * the number of bytes of RGB pixel.
> > +		 */
> > +		int pipe_bpp = pipe_config->output_format ==
> > INTEL_OUTPUT_FORMAT_YCBCR420 ?
> > +			pipe_config->pipe_bpp / 2 : pipe_config-
> > >pipe_bpp;
> > +
> > +		intel_link_compute_m_n(pipe_bpp,
> >  				       pipe_config->lane_count,
> >  				       adjusted_mode->crtc_clock,
> >  				       pipe_config->port_clock,
> >  				       &pipe_config->dp_m_n,
> >  				       constant_n);
> > +	}
> >  	else
> >  		intel_link_compute_m_n(pipe_config-
> > >dsc_params.compressed_bpp,
> >  				       pipe_config->lane_count,
> > @@ -4458,6 +4491,23 @@ intel_pixel_encoding_setup_vsc(struct
> > intel_dp *intel_dp,
> >  	 * 100b = 16bpc.
> >  	 */
> >  	vsc_sdp.DB17 = 0x1;
> > +	switch (crtc_state->pipe_bpp / 3) {
> > +	case 8: /* 8bpc */
> > +		vsc_sdp.DB17 = 0x1;
> > +		break;
> > +	case 10: /* 10bpc */
> > +		vsc_sdp.DB17 = 0x2;
> > +		break;
> > +	case 12: /* 12bpc */
> > +		vsc_sdp.DB17 = 0x3;
> > +		break;
> > +	case 16: /* 16bpc */
> > +		vsc_sdp.DB17 = 0x4;
> > +		break;
> > +	default:
> > +		DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state-
> > >pipe_bpp);
> > +		break;
> > +	}
> >  
> >  	/*
> >  	 * Content Type (Bits 2:0)
> > -- 
> > 2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-06 19:30 [RFC v4 0/6] drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs Gwan-gyeong Mun
2019-03-06 19:30 ` [RFC v4 1/6] drm/i915/dp: Add a config function for YCBCR420 outputs Gwan-gyeong Mun
2019-03-06 20:52   ` Ville Syrjälä
2019-03-07 20:29     ` Mun, Gwan-gyeong
2019-03-06 19:30 ` [RFC v4 2/6] drm: Add a VSC structure for handling Pixel Encoding/Colorimetry Formats Gwan-gyeong Mun
2019-03-06 19:30 ` [RFC v4 3/6] drm/i915/dp: Program VSC Header and DB for Pixel Encoding/Colorimetry Format Gwan-gyeong Mun
2019-03-06 19:31 ` [RFC v4 4/6] drm/i915/dp: Add a support of YCBCR 4:2:0 to DP MSA Gwan-gyeong Mun
2019-03-06 19:31 ` [RFC v4 5/6] drm/i915/dp: Change a link bandwidth computation for DP YCbCr 4:2:0 output Gwan-gyeong Mun
2019-03-06 21:04   ` Ville Syrjälä
2019-03-07 20:31     ` Mun, Gwan-gyeong
2019-03-06 19:31 ` [RFC v4 6/6] drm/i915/dp: Support DP ports YUV 4:2:0 output to GEN11 Gwan-gyeong Mun
2019-03-06 20:12 ` ✓ Fi.CI.BAT: success for drm/i915/dp: Preliminary support for DP YCbCr4:2:0 outputs (rev4) Patchwork
2019-03-06 22:03 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.