All of lore.kernel.org
 help / color / mirror / Atom feed
* [v3 0/7] Forward Error Correction
@ 2018-10-26  4:49 Anusha Srivatsa
  2018-10-26  4:49 ` [v3 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register Anusha Srivatsa
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Anusha Srivatsa @ 2018-10-26  4:49 UTC (permalink / raw)
  To: intel-gfx

With Display Compression, the bit error in the pixel
stream can turn into a significant corruption on
the screen. The DP1.4 adds FEC - Forward Error Correction
scheme which uses Reed-Solomon parity/correction check
generated by the source and used by the sink to detect
and correct small numbers of bit errors in the compressed
stream.

v2: Avoid doing aux channel read eberytime we check
for FEC support. Instead cache the value of the DPCD
registers, similar to the DSC implementaion (Jani)

v3: Add fec as a state to crtc. Move around the code. (Ville)

This is rebased on top of Manasi's End-to-end DSC
Implementation: https://patchwork.freedesktop.org/series/47514/

Anusha Srivatsa (7):
  i915/dp/fec: Cache the FEC_CAPABLE DPCD register
  drm/dp/fec: DRM helper for Forward Error Correction
  i915/dp/fec: Check for FEC Support
  i915/dp/fec: Add can_fec to the crtc state.
  drm/i915/fec: Set FEC_READY in FEC_CONFIGURATION
  i915/dp/fec: Configure the Forward Error Correction bits.
  drm/i915/fec: Disable FEC state.

 drivers/gpu/drm/i915/i915_reg.h  |  2 ++
 drivers/gpu/drm/i915/intel_ddi.c | 60 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 58 ++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h | 11 ++++++
 include/drm/drm_dp_helper.h      |  7 ++++
 5 files changed, 136 insertions(+), 2 deletions(-)

-- 
2.17.1

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

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

* [v3 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register
  2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
@ 2018-10-26  4:49 ` Anusha Srivatsa
  2018-10-30  2:29   ` Manasi Navare
  2018-10-26  4:49 ` [v3 2/7] drm/dp/fec: DRM helper for Forward Error Correction Anusha Srivatsa
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Anusha Srivatsa @ 2018-10-26  4:49 UTC (permalink / raw)
  To: intel-gfx

Similar to DSC DPCD registers, let us cache
FEC_CAPABLE register to avoid using stale
values. With this we can avoid aux reads
everytime and instead read the cached values.

v2: Avoid using memset and array for a single
field. (Manasi,Jani)

Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 10 ++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5a638503e36a..16d1db7c9398 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4198,9 +4198,13 @@ static void intel_dp_get_dsc_sink_cap(struct intel_dp *intel_dp)
 	/*
 	 *Clear the cached register set to avoid using stale values
 	 * for the sinks that do not support DSC.
+	 * Similarly, clear the cached FEC register.
 	 */
 	memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
 
+	/* Clear fec_capable to avoid using stale values */
+	intel_dp->fec_capable = 0;
+
 	/* Cache the DSC DPCD if eDP or DP rev >= 1.4 */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x14 ||
 	    intel_dp->edp_dpcd[0] >= DP_EDP_14) {
@@ -4214,6 +4218,12 @@ static void intel_dp_get_dsc_sink_cap(struct intel_dp *intel_dp)
 			      (int)sizeof(intel_dp->dsc_dpcd),
 			      intel_dp->dsc_dpcd);
 	}
+	/* FEC is supported only on DP 1.4 */
+	if (!intel_dp_is_edp(intel_dp) && intel_dp->dpcd[DP_DPCD_REV] >= 0x14) {
+		if (drm_dp_dpcd_readb(&intel_dp->aux, DP_FEC_CAPABILITY,
+				      &intel_dp->fec_capable) < 0)
+			DRM_ERROR("Failed to read FEC DPCD register\n");
+	}
 }
 
 static bool
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 16bbc3768e02..9a94c6544bf5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1119,6 +1119,7 @@ struct intel_dp {
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	uint8_t edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
 	u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
+	u8 fec_capable;
 	/* source rates */
 	int num_source_rates;
 	const int *source_rates;
-- 
2.17.1

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

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

* [v3 2/7] drm/dp/fec: DRM helper for Forward Error Correction
  2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
  2018-10-26  4:49 ` [v3 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register Anusha Srivatsa
@ 2018-10-26  4:49 ` Anusha Srivatsa
  2018-10-30  2:30   ` Manasi Navare
  2018-10-26  4:49 ` [v3 3/7] i915/dp/fec: Check for FEC Support Anusha Srivatsa
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Anusha Srivatsa @ 2018-10-26  4:49 UTC (permalink / raw)
  To: intel-gfx

DP 1.4 has Forward Error Correction Support(FEC).
Add helper function to check if the sink device
supports FEC.

v2: Separate the helper and the code that uses the helper into
two separate patches. (Manasi)

v3:
- Move the code to drm_dp_helper.c (Manasi)
- change the return type, code style changes (Gaurav)
- Use drm_dp_dpcd_readb instead of drm_dp_dpcd_read. (Jani)

v4:
- Avoid aux reads everytime, instead read cached
values of dpcd register (jani)
- Move helper to drm_dp_helper.h like other dsc
helpers.(Anusha)

v5: rebased. Change the helper parameter suitably.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 include/drm/drm_dp_helper.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 2649529d0d8f..b08f50b852f5 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1101,6 +1101,13 @@ drm_dp_dsc_sink_max_slice_width(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
 		DP_DSC_SLICE_WIDTH_MULTIPLIER;
 }
 
+/* Forward Error Correction Support on DP 1.4 */
+static inline bool
+drm_dp_sink_supports_fec(const u8 fec_capable)
+{
+	return fec_capable & DP_FEC_CAPABLE;
+}
+
 /*
  * DisplayPort AUX channel
  */
-- 
2.17.1

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

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

* [v3 3/7] i915/dp/fec: Check for FEC Support
  2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
  2018-10-26  4:49 ` [v3 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register Anusha Srivatsa
  2018-10-26  4:49 ` [v3 2/7] drm/dp/fec: DRM helper for Forward Error Correction Anusha Srivatsa
@ 2018-10-26  4:49 ` Anusha Srivatsa
  2018-10-26  4:49 ` [v3 4/7] i915/dp/fec: Add can_fec to the crtc state Anusha Srivatsa
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Anusha Srivatsa @ 2018-10-26  4:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

For DP 1.4 and above, Display Stream compression can be
enabled only if Forward Error Correctin can be performed.

Check if the sink supports FEC using the helper.

v2: Mention External DP where ever FEC is mentioned
in the code.Check return status of dpcd reads. (Gaurav)
- Do regular mode check even if FEC is not supported. (manasi)

v3: Do not perform any dpcd writes in the atomic
check phase. (DK, Manasi)

v4: Use debug level logging for scenario where sink does
not support a feature. (DK)

v5: Correct commit message. rebase.

v6: pass single field instead of an array for helper
function. (manasi)

Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 16d1db7c9398..cfcef9e4b5d9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -680,7 +680,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
 			dsc_slice_count =
 				drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
 								true);
-		} else {
+		} else if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
 			dsc_max_output_bpp =
 				intel_dp_dsc_get_output_bpp(max_link_clock,
 							    max_lanes,
@@ -690,7 +690,8 @@ intel_dp_mode_valid(struct drm_connector *connector,
 				intel_dp_dsc_get_slice_count(intel_dp,
 							     target_clock,
 							     mode->hdisplay);
-		}
+		} else
+			DRM_DEBUG_KMS("Sink device does not Support FEC\n");
 	}
 
 	if ((mode_rate > max_rate && !(dsc_max_output_bpp && dsc_slice_count)) ||
@@ -2063,6 +2064,13 @@ static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 	if (pipe == PIPE_A && !intel_dp_is_edp(intel_dp))
 		return false;
 
+	/* DSC not supported if external DP sink does not support FEC */
+	if (!intel_dp_is_edp(intel_dp) &&
+	    !drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
+		DRM_DEBUG_KMS("Sink does not support Forward Error Correction, disabling Display Compression\n");
+		return false;
+	}
+
 	/* DSC not supported for DSC sink BPC < 8 */
 	if (limits->max_bpp < 3 * DP_DSC_MIN_SUPPORTED_BPC) {
 		DRM_DEBUG_KMS("No DSC support for less than 8bpc\n");
-- 
2.17.1

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

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

* [v3 4/7] i915/dp/fec: Add can_fec to the crtc state.
  2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
                   ` (2 preceding siblings ...)
  2018-10-26  4:49 ` [v3 3/7] i915/dp/fec: Check for FEC Support Anusha Srivatsa
@ 2018-10-26  4:49 ` Anusha Srivatsa
  2018-10-30  2:44   ` Manasi Navare
  2018-10-30 18:57   ` Ville Syrjälä
  2018-10-26  4:49 ` [v3 5/7] drm/i915/fec: Set FEC_READY in FEC_CONFIGURATION Anusha Srivatsa
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 14+ messages in thread
From: Anusha Srivatsa @ 2018-10-26  4:49 UTC (permalink / raw)
  To: intel-gfx

Add a crtc state for FEC. Currently, the state
is determined by platform, DP and DSC being
enabled. Moving forward we can use the state
to have error correction on other scenarios too
if needed.

Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cfcef9e4b5d9..4776ce6f2174 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2045,6 +2045,23 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
 	return false;
 }
 
+static bool intel_dp_can_fec(struct intel_dp *intel_dp,
+			     struct intel_crtc_state *pipe_config)
+{
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+	if (INTEL_GEN(dev_priv) < 11 || intel_dp_is_edp(intel_dp))
+		return false;
+
+	/* On Gen 11, FEC is Supported Only for DP SST modes.
+	 * Let us start by enabling FEC for Compressed streams.
+	 */
+	if (pipe_config->dsc_params.compression_enable)
+		return true;
+	else
+		return false;
+
+}
 static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 					struct intel_crtc_state *pipe_config,
 					struct link_config_limits *limits)
@@ -2129,6 +2146,11 @@ static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 		      pipe_config->dsc_params.compressed_bpp,
 		      pipe_config->dsc_params.slice_count);
 
+	/* For DP 1.4, Enable DSC if FEC can be configured */
+	pipe_config->can_fec = intel_dp_can_fec(intel_dp, pipe_config);
+	if (!pipe_config->can_fec)
+		return false;
+
 	return true;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9a94c6544bf5..9dac242ead12 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -940,6 +940,9 @@ struct intel_crtc_state {
 		u8 slice_count;
 	} dsc_params;
 	struct drm_dsc_config dp_dsc_cfg;
+
+	/* Forward Error correction State */
+	bool can_fec;
 };
 
 struct intel_crtc {
-- 
2.17.1

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

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

* [v3 5/7] drm/i915/fec: Set FEC_READY in FEC_CONFIGURATION
  2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
                   ` (3 preceding siblings ...)
  2018-10-26  4:49 ` [v3 4/7] i915/dp/fec: Add can_fec to the crtc state Anusha Srivatsa
@ 2018-10-26  4:49 ` Anusha Srivatsa
  2018-10-26  4:49 ` [v3 6/7] i915/dp/fec: Configure the Forward Error Correction bits Anusha Srivatsa
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Anusha Srivatsa @ 2018-10-26  4:49 UTC (permalink / raw)
  To: intel-gfx

If the panel supports FEC, the driver has to
set the FEC_READY bit in the dpcd register:
FEC_CONFIGURATION.

This has to happen before link training.

v2: s/intel_dp_set_fec_ready/intel_dp_sink_set_fec_ready
   - change commit message. (Gaurav)

v3: rebased. (r-b Manasi)

v4: Use fec crtc state, before setting FEC_READY
bit. (Anusha)

Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  1 +
 drivers/gpu/drm/i915/intel_dp.c  | 14 ++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  3 +++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1de0a3917d7f..efbada95dc4e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2932,6 +2932,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
 	intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
 					      DP_DECOMPRESSION_EN);
+	intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
 	intel_dp_start_link_train(intel_dp);
 	if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
 		intel_dp_stop_link_train(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4776ce6f2174..c446821c58b2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3057,6 +3057,20 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
 			      state == DP_DECOMPRESSION_EN ? "enable" : "disable");
 }
 
+void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
+				 const struct intel_crtc_state *crtc_state,
+				 int state)
+{
+	int ret;
+
+	if (!crtc_state->can_fec)
+		return;
+
+	ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_FEC_CONFIGURATION, state);
+	if (ret < 0)
+		DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
+}
+
 /* If the sink supports it, try to set the power state appropriately */
 void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9dac242ead12..a15ecdb4a706 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1796,6 +1796,9 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
 void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
 					   const struct intel_crtc_state *crtc_state,
 					   int state);
+void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
+				 const struct intel_crtc_state *crtc_state,
+				 int state);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
 void intel_dp_encoder_destroy(struct drm_encoder *encoder);
-- 
2.17.1

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

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

* [v3 6/7] i915/dp/fec: Configure the Forward Error Correction bits.
  2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
                   ` (4 preceding siblings ...)
  2018-10-26  4:49 ` [v3 5/7] drm/i915/fec: Set FEC_READY in FEC_CONFIGURATION Anusha Srivatsa
@ 2018-10-26  4:49 ` Anusha Srivatsa
  2018-10-26  4:49 ` [v3 7/7] drm/i915/fec: Disable FEC state Anusha Srivatsa
  2018-10-26  5:07 ` ✗ Fi.CI.BAT: failure for Forward Error Correction (rev3) Patchwork
  7 siblings, 0 replies; 14+ messages in thread
From: Anusha Srivatsa @ 2018-10-26  4:49 UTC (permalink / raw)
  To: intel-gfx

If FEC is supported, the corresponding
DP_TP_CTL register bits have to be configured.

The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register
and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
Also add the warn message to make sure that the control
register is already active while enabling FEC.

v2:
- Change commit message. Configure fec state after
  link training (Manasi, Gaurav)
- Remove redundent checks (Manasi)
- Remove the registers that get added automagically (Anusha)

v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav)

v4: rebased.

v5:
- Move the code to the proper spot, according to spec.(Ville)
- Use fec state as a check too.

Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 ++
 drivers/gpu/drm/i915/intel_ddi.c | 27 +++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 3 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e85f53cb9cdd..8b1753939299 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9134,6 +9134,7 @@ enum skl_power_gate {
 #define _DP_TP_CTL_B			0x64140
 #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
 #define  DP_TP_CTL_ENABLE			(1 << 31)
+#define  DP_TP_CTL_FEC_ENABLE			(1 << 30)
 #define  DP_TP_CTL_MODE_SST			(0 << 27)
 #define  DP_TP_CTL_MODE_MST			(1 << 27)
 #define  DP_TP_CTL_FORCE_ACT			(1 << 25)
@@ -9152,6 +9153,7 @@ enum skl_power_gate {
 #define _DP_TP_STATUS_A			0x64044
 #define _DP_TP_STATUS_B			0x64144
 #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A, _DP_TP_STATUS_B)
+#define  DP_TP_STATUS_FEC_ENABLE_LIVE		(1 << 28)
 #define  DP_TP_STATUS_IDLE_DONE			(1 << 25)
 #define  DP_TP_STATUS_ACT_SENT			(1 << 24)
 #define  DP_TP_STATUS_MODE_STATUS_MST		(1 << 23)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index efbada95dc4e..5badeaefb539 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2889,6 +2889,29 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
 	}
 }
 
+void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
+			       const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	enum port port = intel_dig_port->base.port;
+	u32 val;
+
+	/* FEC support exists for DP 1.4 only */
+	if (!crtc_state->can_fec)
+		return;
+
+	val = I915_READ(DP_TP_CTL(port));
+	val |= DP_TP_CTL_FEC_ENABLE;
+	I915_WRITE(DP_TP_CTL(port), val);
+
+	if (intel_wait_for_register(dev_priv, DP_TP_STATUS(port),
+				    DP_TP_STATUS_FEC_ENABLE_LIVE,
+				    DP_TP_STATUS_FEC_ENABLE_LIVE,
+				    1))
+		DRM_ERROR("Timed out waiting for FEC Enable Status\n");
+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 				    const struct intel_crtc_state *crtc_state,
 				    const struct drm_connector_state *conn_state)
@@ -2934,9 +2957,13 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 					      DP_DECOMPRESSION_EN);
 	intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
 	intel_dp_start_link_train(intel_dp);
+
 	if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
 		intel_dp_stop_link_train(intel_dp);
 
+	/* Set FEC state after link training */
+	intel_dp_enable_fec_state(intel_dp, crtc_state);
+
 	icl_enable_phy_clock_gating(dig_port);
 
 	if (!is_mst)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a15ecdb4a706..405a44b3b3a8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1799,6 +1799,8 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
 void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
 				 const struct intel_crtc_state *crtc_state,
 				 int state);
+void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
+			       const struct intel_crtc_state *crtc_state);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
 void intel_dp_encoder_destroy(struct drm_encoder *encoder);
-- 
2.17.1

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

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

* [v3 7/7] drm/i915/fec: Disable FEC state.
  2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
                   ` (5 preceding siblings ...)
  2018-10-26  4:49 ` [v3 6/7] i915/dp/fec: Configure the Forward Error Correction bits Anusha Srivatsa
@ 2018-10-26  4:49 ` Anusha Srivatsa
  2018-10-26  5:07 ` ✗ Fi.CI.BAT: failure for Forward Error Correction (rev3) Patchwork
  7 siblings, 0 replies; 14+ messages in thread
From: Anusha Srivatsa @ 2018-10-26  4:49 UTC (permalink / raw)
  To: intel-gfx

Set the suitable bits in DP_TP_CTL to stop
bit correction when DSC is disabled.

v2:
- rebased.
- Add additional check for compression state. (Gaurav)

v3: rebased.

v4:
- Move the code to the proper spot according to spec (Ville)
- Use proper checks (manasi)

Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 32 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 5badeaefb539..1e16c9aa0e65 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2912,6 +2912,31 @@ void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
 		DRM_ERROR("Timed out waiting for FEC Enable Status\n");
 }
 
+void intel_dp_disable_fec_state(struct intel_dp *intel_dp,
+				const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	enum port port = intel_dig_port->base.port;
+	u32 val;
+	u8 dsc_en_state;
+
+	if (!crtc_state->can_fec)
+		return;
+
+	if (!crtc_state->dsc_params.compression_enable)
+		return;
+
+	drm_dp_dpcd_readb(&intel_dp->aux, DP_DSC_ENABLE, &dsc_en_state);
+
+	if (!dsc_en_state) {
+		val = I915_READ(DP_TP_CTL(port));
+		val &= ~DP_TP_CTL_FEC_ENABLE;
+		I915_WRITE(DP_TP_CTL(port), val);
+		POSTING_READ(DP_TP_CTL(port));
+	}
+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 				    const struct intel_crtc_state *crtc_state,
 				    const struct drm_connector_state *conn_state)
@@ -3052,7 +3077,11 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 static void intel_disable_ddi_buf(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	struct intel_dp *intel_dp = &dig_port->dp;
+	struct intel_crtc_state *crtc_state;
 	enum port port = encoder->port;
+
 	bool wait = false;
 	u32 val;
 
@@ -3068,6 +3097,9 @@ static void intel_disable_ddi_buf(struct intel_encoder *encoder)
 	val |= DP_TP_CTL_LINK_TRAIN_PAT1;
 	I915_WRITE(DP_TP_CTL(port), val);
 
+	/* Disable FEC in DP Sink */
+	intel_dp_disable_fec_state(intel_dp, crtc_state);
+
 	if (wait)
 		intel_wait_ddi_buf_idle(dev_priv, port);
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 405a44b3b3a8..a2f4562dabe2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1801,6 +1801,8 @@ void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
 				 int state);
 void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
 			       const struct intel_crtc_state *crtc_state);
+void intel_dp_disable_fec_state(struct intel_dp *intel_dp,
+				const struct intel_crtc_state *crtc_state);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
 void intel_dp_encoder_destroy(struct drm_encoder *encoder);
-- 
2.17.1

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

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

* ✗ Fi.CI.BAT: failure for Forward Error Correction (rev3)
  2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
                   ` (6 preceding siblings ...)
  2018-10-26  4:49 ` [v3 7/7] drm/i915/fec: Disable FEC state Anusha Srivatsa
@ 2018-10-26  5:07 ` Patchwork
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-10-26  5:07 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

== Series Details ==

Series: Forward Error Correction (rev3)
URL   : https://patchwork.freedesktop.org/series/47848/
State : failure

== Summary ==

Applying: i915/dp/fec: Cache the FEC_CAPABLE DPCD register
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/intel_dp.c).
error: could not build fake ancestor
Patch failed at 0001 i915/dp/fec: Cache the FEC_CAPABLE DPCD register
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

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

* Re: [v3 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register
  2018-10-26  4:49 ` [v3 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register Anusha Srivatsa
@ 2018-10-30  2:29   ` Manasi Navare
  0 siblings, 0 replies; 14+ messages in thread
From: Manasi Navare @ 2018-10-30  2:29 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

On Thu, Oct 25, 2018 at 09:49:37PM -0700, Anusha Srivatsa wrote:
> Similar to DSC DPCD registers, let us cache
> FEC_CAPABLE register to avoid using stale
> values. With this we can avoid aux reads
> everytime and instead read the cached values.
> 
> v2: Avoid using memset and array for a single
> field. (Manasi,Jani)
> 
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5a638503e36a..16d1db7c9398 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4198,9 +4198,13 @@ static void intel_dp_get_dsc_sink_cap(struct intel_dp *intel_dp)
>  	/*
>  	 *Clear the cached register set to avoid using stale values
>  	 * for the sinks that do not support DSC.
> +	 * Similarly, clear the cached FEC register.

You dont need this comment here about clearing cached reg since you have
a separate comment below.

>  	 */
>  	memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
>  
> +	/* Clear fec_capable to avoid using stale values */
> +	intel_dp->fec_capable = 0;
> +
>  	/* Cache the DSC DPCD if eDP or DP rev >= 1.4 */
>  	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x14 ||
>  	    intel_dp->edp_dpcd[0] >= DP_EDP_14) {
> @@ -4214,6 +4218,12 @@ static void intel_dp_get_dsc_sink_cap(struct intel_dp *intel_dp)
>  			      (int)sizeof(intel_dp->dsc_dpcd),
>  			      intel_dp->dsc_dpcd);
>  	}

Why cant you embed reading FEC reg in the above condition.
Within that just check !edp and read fec reg.

Manasi

> +	/* FEC is supported only on DP 1.4 */
> +	if (!intel_dp_is_edp(intel_dp) && intel_dp->dpcd[DP_DPCD_REV] >= 0x14) {
> +		if (drm_dp_dpcd_readb(&intel_dp->aux, DP_FEC_CAPABILITY,
> +				      &intel_dp->fec_capable) < 0)
> +			DRM_ERROR("Failed to read FEC DPCD register\n");
> +	}
>  }
>  
>  static bool
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 16bbc3768e02..9a94c6544bf5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1119,6 +1119,7 @@ struct intel_dp {
>  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>  	uint8_t edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
>  	u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
> +	u8 fec_capable;
>  	/* source rates */
>  	int num_source_rates;
>  	const int *source_rates;
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [v3 2/7] drm/dp/fec: DRM helper for Forward Error Correction
  2018-10-26  4:49 ` [v3 2/7] drm/dp/fec: DRM helper for Forward Error Correction Anusha Srivatsa
@ 2018-10-30  2:30   ` Manasi Navare
  0 siblings, 0 replies; 14+ messages in thread
From: Manasi Navare @ 2018-10-30  2:30 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

On Thu, Oct 25, 2018 at 09:49:38PM -0700, Anusha Srivatsa wrote:
> DP 1.4 has Forward Error Correction Support(FEC).
> Add helper function to check if the sink device
> supports FEC.
> 
> v2: Separate the helper and the code that uses the helper into
> two separate patches. (Manasi)
> 
> v3:
> - Move the code to drm_dp_helper.c (Manasi)
> - change the return type, code style changes (Gaurav)
> - Use drm_dp_dpcd_readb instead of drm_dp_dpcd_read. (Jani)
> 
> v4:
> - Avoid aux reads everytime, instead read cached
> values of dpcd register (jani)
> - Move helper to drm_dp_helper.h like other dsc
> helpers.(Anusha)
> 
> v5: rebased. Change the helper parameter suitably.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  include/drm/drm_dp_helper.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 2649529d0d8f..b08f50b852f5 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1101,6 +1101,13 @@ drm_dp_dsc_sink_max_slice_width(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  		DP_DSC_SLICE_WIDTH_MULTIPLIER;
>  }
>  
> +/* Forward Error Correction Support on DP 1.4 */
> +static inline bool
> +drm_dp_sink_supports_fec(const u8 fec_capable)
> +{
> +	return fec_capable & DP_FEC_CAPABLE;
> +}
> +
>  /*
>   * DisplayPort AUX channel
>   */
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [v3 4/7] i915/dp/fec: Add can_fec to the crtc state.
  2018-10-26  4:49 ` [v3 4/7] i915/dp/fec: Add can_fec to the crtc state Anusha Srivatsa
@ 2018-10-30  2:44   ` Manasi Navare
  2018-10-30 18:57   ` Ville Syrjälä
  1 sibling, 0 replies; 14+ messages in thread
From: Manasi Navare @ 2018-10-30  2:44 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

On Thu, Oct 25, 2018 at 09:49:40PM -0700, Anusha Srivatsa wrote:
> Add a crtc state for FEC. Currently, the state
> is determined by platform, DP and DSC being
> enabled. Moving forward we can use the state
> to have error correction on other scenarios too
> if needed.
> 
> Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  3 +++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index cfcef9e4b5d9..4776ce6f2174 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2045,6 +2045,23 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
>  	return false;
>  }
>  
> +static bool intel_dp_can_fec(struct intel_dp *intel_dp,
> +			     struct intel_crtc_state *pipe_config)

I would prefer naming it as intel_dp_supports_fec since as per Ville's feedback
on adding helper function for DSC that will be called intel_dp_supports_dsc

> +{
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);

You dont need dig_port, you can obtain dev_priv from intel_dp directly:
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);

> +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> +	if (INTEL_GEN(dev_priv) < 11 || intel_dp_is_edp(intel_dp))
> +		return false;
> +
> +	/* On Gen 11, FEC is Supported Only for DP SST modes.
> +	 * Let us start by enabling FEC for Compressed streams.
> +	 */

Where is the check for sink FEC support?

> +	if (pipe_config->dsc_params.compression_enable)
> +		return true;

I think this logic also looks reversed. The compression_enable state should
be set based on whether can_fec is true and not the vice versa.
IMHO, can_fec is decided solely on the platform support.

Infact since can_fec or fec_support solely depends on platform and sink FEC support,
we dont need a can_fec state , that can just be obtained using a helper
intel_dp_supports_fec() which will check platform support and sink support.

Now if we are saying that we want to enable fec based on mode, then we need fec_enable
state in crtc and that can be set if intel_dp_supports_fec and for now only if compression_en
is set.

Jani, Ville what are your thoughts on this?

> +	else
> +		return false;

You can swap these and have false earlier and avoid else
Just return true at the end.

Manasi


> +
> +}
>  static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  					struct intel_crtc_state *pipe_config,
>  					struct link_config_limits *limits)
> @@ -2129,6 +2146,11 @@ static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  		      pipe_config->dsc_params.compressed_bpp,
>  		      pipe_config->dsc_params.slice_count);
>  
> +	/* For DP 1.4, Enable DSC if FEC can be configured */
> +	pipe_config->can_fec = intel_dp_can_fec(intel_dp, pipe_config);
> +	if (!pipe_config->can_fec)
> +		return false;
> +
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9a94c6544bf5..9dac242ead12 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -940,6 +940,9 @@ struct intel_crtc_state {
>  		u8 slice_count;
>  	} dsc_params;
>  	struct drm_dsc_config dp_dsc_cfg;
> +
> +	/* Forward Error correction State */
> +	bool can_fec;
>  };
>  
>  struct intel_crtc {
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [v3 4/7] i915/dp/fec: Add can_fec to the crtc state.
  2018-10-26  4:49 ` [v3 4/7] i915/dp/fec: Add can_fec to the crtc state Anusha Srivatsa
  2018-10-30  2:44   ` Manasi Navare
@ 2018-10-30 18:57   ` Ville Syrjälä
  2018-10-30 19:38     ` Ville Syrjälä
  1 sibling, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2018-10-30 18:57 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

On Thu, Oct 25, 2018 at 09:49:40PM -0700, Anusha Srivatsa wrote:
> Add a crtc state for FEC. Currently, the state
> is determined by platform, DP and DSC being
> enabled. Moving forward we can use the state
> to have error correction on other scenarios too
> if needed.
> 
> Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  3 +++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index cfcef9e4b5d9..4776ce6f2174 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2045,6 +2045,23 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
>  	return false;
>  }
>  
> +static bool intel_dp_can_fec(struct intel_dp *intel_dp,
> +			     struct intel_crtc_state *pipe_config)
> +{
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> +	if (INTEL_GEN(dev_priv) < 11 || intel_dp_is_edp(intel_dp))
> +		return false;

We don't need FEC for eDP DSC?

> +
> +	/* On Gen 11, FEC is Supported Only for DP SST modes.
> +	 * Let us start by enabling FEC for Compressed streams.
> +	 */
> +	if (pipe_config->dsc_params.compression_enable)
> +		return true;
> +	else
> +		return false;
> +
> +}
>  static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  					struct intel_crtc_state *pipe_config,
>  					struct link_config_limits *limits)
> @@ -2129,6 +2146,11 @@ static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  		      pipe_config->dsc_params.compressed_bpp,
>  		      pipe_config->dsc_params.slice_count);
>  
> +	/* For DP 1.4, Enable DSC if FEC can be configured */
> +	pipe_config->can_fec = intel_dp_can_fec(intel_dp, pipe_config);
> +	if (!pipe_config->can_fec)
> +		return false;
> +
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9a94c6544bf5..9dac242ead12 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -940,6 +940,9 @@ struct intel_crtc_state {
>  		u8 slice_count;
>  	} dsc_params;
>  	struct drm_dsc_config dp_dsc_cfg;
> +
> +	/* Forward Error correction State */
> +	bool can_fec;

'fec_enable' is what I'd call this.

We're rather inconsistent with  foo_enable vs. foo_enabled vs.
enable_foo, but one has to pick something.

>  };
>  
>  struct intel_crtc {
> -- 
> 2.17.1

-- 
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] 14+ messages in thread

* Re: [v3 4/7] i915/dp/fec: Add can_fec to the crtc state.
  2018-10-30 18:57   ` Ville Syrjälä
@ 2018-10-30 19:38     ` Ville Syrjälä
  0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2018-10-30 19:38 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

On Tue, Oct 30, 2018 at 08:57:12PM +0200, Ville Syrjälä wrote:
> On Thu, Oct 25, 2018 at 09:49:40PM -0700, Anusha Srivatsa wrote:
> > Add a crtc state for FEC. Currently, the state
> > is determined by platform, DP and DSC being
> > enabled. Moving forward we can use the state
> > to have error correction on other scenarios too
> > if needed.
> > 
> > Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |  3 +++
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index cfcef9e4b5d9..4776ce6f2174 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2045,6 +2045,23 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
> >  	return false;
> >  }
> >  
> > +static bool intel_dp_can_fec(struct intel_dp *intel_dp,
> > +			     struct intel_crtc_state *pipe_config)
> > +{
> > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > +	if (INTEL_GEN(dev_priv) < 11 || intel_dp_is_edp(intel_dp))
> > +		return false;
> 
> We don't need FEC for eDP DSC?

This stuff maybe needs another sink vs. source split and whatnot.

Maybe along these lines:

source_supportes_fec()
{
	return GEN >= 11 && port != PORT_A;
}

sink_supports_fec()
{
	return whatever;
}

supports_fec()
{
	return source_supports_fec() &&
		sink_supports_fec();
}

compute_dsc()
{
	/* eDP DSC does not need FEC */
	fec_enable = !is_edp();

	if (fec_enable && !supports_fec())
		return false;

	if (!supports_dsc())
		return false;

	...
}


> 
> > +
> > +	/* On Gen 11, FEC is Supported Only for DP SST modes.
> > +	 * Let us start by enabling FEC for Compressed streams.
> > +	 */
> > +	if (pipe_config->dsc_params.compression_enable)
> > +		return true;
> > +	else
> > +		return false;
> > +
> > +}
> >  static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >  					struct intel_crtc_state *pipe_config,
> >  					struct link_config_limits *limits)
> > @@ -2129,6 +2146,11 @@ static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >  		      pipe_config->dsc_params.compressed_bpp,
> >  		      pipe_config->dsc_params.slice_count);
> >  
> > +	/* For DP 1.4, Enable DSC if FEC can be configured */
> > +	pipe_config->can_fec = intel_dp_can_fec(intel_dp, pipe_config);
> > +	if (!pipe_config->can_fec)
> > +		return false;
> > +
> >  	return true;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 9a94c6544bf5..9dac242ead12 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -940,6 +940,9 @@ struct intel_crtc_state {
> >  		u8 slice_count;
> >  	} dsc_params;
> >  	struct drm_dsc_config dp_dsc_cfg;
> > +
> > +	/* Forward Error correction State */
> > +	bool can_fec;
> 
> 'fec_enable' is what I'd call this.
> 
> We're rather inconsistent with  foo_enable vs. foo_enabled vs.
> enable_foo, but one has to pick something.
> 
> >  };
> >  
> >  struct intel_crtc {
> > -- 
> > 2.17.1
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> 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

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

end of thread, other threads:[~2018-10-30 19:38 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-26  4:49 [v3 0/7] Forward Error Correction Anusha Srivatsa
2018-10-26  4:49 ` [v3 1/7] i915/dp/fec: Cache the FEC_CAPABLE DPCD register Anusha Srivatsa
2018-10-30  2:29   ` Manasi Navare
2018-10-26  4:49 ` [v3 2/7] drm/dp/fec: DRM helper for Forward Error Correction Anusha Srivatsa
2018-10-30  2:30   ` Manasi Navare
2018-10-26  4:49 ` [v3 3/7] i915/dp/fec: Check for FEC Support Anusha Srivatsa
2018-10-26  4:49 ` [v3 4/7] i915/dp/fec: Add can_fec to the crtc state Anusha Srivatsa
2018-10-30  2:44   ` Manasi Navare
2018-10-30 18:57   ` Ville Syrjälä
2018-10-30 19:38     ` Ville Syrjälä
2018-10-26  4:49 ` [v3 5/7] drm/i915/fec: Set FEC_READY in FEC_CONFIGURATION Anusha Srivatsa
2018-10-26  4:49 ` [v3 6/7] i915/dp/fec: Configure the Forward Error Correction bits Anusha Srivatsa
2018-10-26  4:49 ` [v3 7/7] drm/i915/fec: Disable FEC state Anusha Srivatsa
2018-10-26  5:07 ` ✗ Fi.CI.BAT: failure for Forward Error Correction (rev3) 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.