* [PATCH 01/23] drm/i915/dp: Fix dsc bpp calculations, v2.
@ 2019-09-20 11:42 Maarten Lankhorst
2019-09-20 16:38 ` [Intel-gfx] " Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Maarten Lankhorst @ 2019-09-20 11:42 UTC (permalink / raw)
To: intel-gfx; +Cc: Maarten Lankhorst, stable, Manasi Navare
There was a integer wraparound when mode_clock became too high,
and we didn't correct for the FEC overhead factor when dividing,
with the calculations breaking at HBR3.
As a result our calculated bpp was way too high, and the link width
limitation never came into effect.
Print out the resulting bpp calcululations as a sanity check, just
in case we ever have to debug it later on again.
We also used the wrong factor for FEC. While bspec mentions 2.4%,
all the calculations use 1/0.972261, and the same ratio should be
applied to data M/N as well, so use it there when FEC is enabled.
Make sure we don't break hw readout, and read out FEC enable state
and correct the DDI clock readout for the new values.
Together with the next commit, this causes FEC to work correctly
with big joiner, while also having the correct refresh rate
reported in kms_setmode.basic.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
Cc: <stable@vger.kernel.org> # v5.0+
Cc: Manasi Navare <manasi.d.navare@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 19 +-
drivers/gpu/drm/i915/display/intel_display.c | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 195 ++++++++++---------
drivers/gpu/drm/i915/display/intel_dp.h | 6 +-
4 files changed, 128 insertions(+), 93 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 3e6394139964..1b59b852874b 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1479,6 +1479,10 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
if (pipe_config->pixel_multiplier)
dotclock /= pipe_config->pixel_multiplier;
+ /* fec adds overhead to the data M/N values, correct for it */
+ if (pipe_config->fec_enable)
+ dotclock = intel_dp_fec_to_mode_clock(dotclock);
+
pipe_config->base.adjusted_mode.crtc_clock = dotclock;
}
@@ -4031,7 +4035,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
case TRANS_DDI_MODE_SELECT_FDI:
pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
break;
- case TRANS_DDI_MODE_SELECT_DP_SST:
+ case TRANS_DDI_MODE_SELECT_DP_SST: {
+ struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
if (encoder->type == INTEL_OUTPUT_EDP)
pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
else
@@ -4039,7 +4045,18 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
pipe_config->lane_count =
((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
intel_dp_get_m_n(intel_crtc, pipe_config);
+
+ if (INTEL_GEN(dev_priv) >= 11) {
+ pipe_config->fec_enable =
+ I915_READ(intel_dp->regs.dp_tp_ctl) &
+ DP_TP_CTL_FEC_ENABLE;
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
+ encoder->base.base.id, encoder->base.name,
+ pipe_config->fec_enable);
+ }
+
break;
+ }
case TRANS_DDI_MODE_SELECT_DP_MST:
pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
pipe_config->lane_count =
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e0033d99f6e3..7996864e6f7c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12773,6 +12773,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
PIPE_CONF_CHECK_BOOL(has_infoframe);
+ PIPE_CONF_CHECK_BOOL(fec_enable);
PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index ccaf9f00b747..4dfb78dc7fa2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -76,8 +76,8 @@
#define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
#define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
-/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
-#define DP_DSC_FEC_OVERHEAD_FACTOR 976
+/* DP DSC FEC Overhead factor = 1/(0.972261) */
+#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
/* Compliance test status bits */
#define INTEL_DP_RESOLUTION_SHIFT_MASK 0
@@ -492,6 +492,104 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
return 0;
}
+static inline u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
+{
+ return div_u64(mul_u32_u32(mode_clock, 1000000U),
+ DP_DSC_FEC_OVERHEAD_FACTOR);
+}
+
+u32 intel_dp_fec_to_mode_clock(u32 fec_clock)
+{
+ return div_u64(mul_u32_u32(fec_clock,
+ DP_DSC_FEC_OVERHEAD_FACTOR),
+ 1000000U);
+}
+
+static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
+ u32 mode_clock, u32 mode_hdisplay)
+{
+ u32 bits_per_pixel, max_bpp_small_joiner_ram;
+ int i;
+
+ /*
+ * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
+ * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
+ * for SST -> TimeSlotsPerMTP is 1,
+ * for MST -> TimeSlotsPerMTP has to be calculated
+ */
+ bits_per_pixel = (link_clock * lane_count * 8) /
+ intel_dp_mode_to_fec_clock(mode_clock);
+ DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
+
+ /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
+ max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
+ DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
+
+ /*
+ * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
+ * check, output bpp from small joiner RAM check)
+ */
+ bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
+
+ /* Error out if the max bpp is less than smallest allowed valid bpp */
+ if (bits_per_pixel < valid_dsc_bpp[0]) {
+ DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
+ bits_per_pixel, valid_dsc_bpp[0]);
+ return 0;
+ }
+
+ /* Find the nearest match in the array of known BPPs from VESA */
+ for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
+ if (bits_per_pixel < valid_dsc_bpp[i + 1])
+ break;
+ }
+ bits_per_pixel = valid_dsc_bpp[i];
+
+ /*
+ * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
+ * fractional part is 0
+ */
+ return bits_per_pixel << 4;
+}
+
+static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
+ int mode_clock, int mode_hdisplay)
+{
+ u8 min_slice_count, i;
+ int max_slice_width;
+
+ if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
+ min_slice_count = DIV_ROUND_UP(mode_clock,
+ DP_DSC_MAX_ENC_THROUGHPUT_0);
+ else
+ min_slice_count = DIV_ROUND_UP(mode_clock,
+ DP_DSC_MAX_ENC_THROUGHPUT_1);
+
+ max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
+ if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
+ DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
+ max_slice_width);
+ return 0;
+ }
+ /* Also take into account max slice width */
+ min_slice_count = min_t(u8, min_slice_count,
+ DIV_ROUND_UP(mode_hdisplay,
+ max_slice_width));
+
+ /* Find the closest match to the valid slice count values */
+ for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
+ if (valid_dsc_slicecount[i] >
+ drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
+ false))
+ break;
+ if (min_slice_count <= valid_dsc_slicecount[i])
+ return valid_dsc_slicecount[i];
+ }
+
+ DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
+ return 0;
+}
+
static enum drm_mode_status
intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
@@ -2182,6 +2280,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
DP_DPCD_QUIRK_CONSTANT_N);
int ret = 0, output_bpp;
+ u32 data_clock;
if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
pipe_config->has_pch_encoder = true;
@@ -2244,9 +2343,14 @@ intel_dp_compute_config(struct intel_encoder *encoder,
else
output_bpp = intel_dp_output_bpp(pipe_config, pipe_config->pipe_bpp);
+ if (pipe_config->fec_enable)
+ data_clock = intel_dp_mode_to_fec_clock(adjusted_mode->crtc_clock);
+ else
+ data_clock = adjusted_mode->crtc_clock;
+
intel_link_compute_m_n(output_bpp,
pipe_config->lane_count,
- adjusted_mode->crtc_clock,
+ data_clock,
pipe_config->port_clock,
&pipe_config->dp_m_n,
constant_n);
@@ -4363,91 +4467,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
DP_DPRX_ESI_LEN;
}
-u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
- int mode_clock, int mode_hdisplay)
-{
- u16 bits_per_pixel, max_bpp_small_joiner_ram;
- int i;
-
- /*
- * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
- * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
- * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
- * for MST -> TimeSlotsPerMTP has to be calculated
- */
- bits_per_pixel = (link_clock * lane_count * 8 *
- DP_DSC_FEC_OVERHEAD_FACTOR) /
- mode_clock;
-
- /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
- max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
- mode_hdisplay;
-
- /*
- * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
- * check, output bpp from small joiner RAM check)
- */
- bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
-
- /* Error out if the max bpp is less than smallest allowed valid bpp */
- if (bits_per_pixel < valid_dsc_bpp[0]) {
- DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
- return 0;
- }
-
- /* Find the nearest match in the array of known BPPs from VESA */
- for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
- if (bits_per_pixel < valid_dsc_bpp[i + 1])
- break;
- }
- bits_per_pixel = valid_dsc_bpp[i];
-
- /*
- * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
- * fractional part is 0
- */
- return bits_per_pixel << 4;
-}
-
-u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
- int mode_clock,
- int mode_hdisplay)
-{
- u8 min_slice_count, i;
- int max_slice_width;
-
- if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
- min_slice_count = DIV_ROUND_UP(mode_clock,
- DP_DSC_MAX_ENC_THROUGHPUT_0);
- else
- min_slice_count = DIV_ROUND_UP(mode_clock,
- DP_DSC_MAX_ENC_THROUGHPUT_1);
-
- max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
- if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
- DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
- max_slice_width);
- return 0;
- }
- /* Also take into account max slice width */
- min_slice_count = min_t(u8, min_slice_count,
- DIV_ROUND_UP(mode_hdisplay,
- max_slice_width));
-
- /* Find the closest match to the valid slice count values */
- for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
- if (valid_dsc_slicecount[i] >
- drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
- false))
- break;
- if (min_slice_count <= valid_dsc_slicecount[i])
- return valid_dsc_slicecount[i];
- }
-
- DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
- return 0;
-}
-
static void
intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index e01d1f89409d..e9f11e698697 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
bool
intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
-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);
bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
@@ -119,4 +115,6 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
return ~((1 << lane_count) - 1) & 0xf;
}
+u32 intel_dp_fec_to_mode_clock(u32 fec_clock);
+
#endif /* __INTEL_DP_H__ */
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 01/23] drm/i915/dp: Fix dsc bpp calculations, v2.
2019-09-20 11:42 [PATCH 01/23] drm/i915/dp: Fix dsc bpp calculations, v2 Maarten Lankhorst
@ 2019-09-20 16:38 ` Ville Syrjälä
2019-09-23 12:52 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3 Maarten Lankhorst
0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2019-09-20 16:38 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, stable
On Fri, Sep 20, 2019 at 01:42:13PM +0200, Maarten Lankhorst wrote:
> There was a integer wraparound when mode_clock became too high,
> and we didn't correct for the FEC overhead factor when dividing,
> with the calculations breaking at HBR3.
>
> As a result our calculated bpp was way too high, and the link width
> limitation never came into effect.
>
> Print out the resulting bpp calcululations as a sanity check, just
> in case we ever have to debug it later on again.
>
> We also used the wrong factor for FEC. While bspec mentions 2.4%,
> all the calculations use 1/0.972261, and the same ratio should be
> applied to data M/N as well, so use it there when FEC is enabled.
>
> Make sure we don't break hw readout, and read out FEC enable state
> and correct the DDI clock readout for the new values.
>
> Together with the next commit, this causes FEC to work correctly
> with big joiner, while also having the correct refresh rate
> reported in kms_setmode.basic.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
> Cc: <stable@vger.kernel.org> # v5.0+
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 19 +-
> drivers/gpu/drm/i915/display/intel_display.c | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 195 ++++++++++---------
> drivers/gpu/drm/i915/display/intel_dp.h | 6 +-
> 4 files changed, 128 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 3e6394139964..1b59b852874b 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1479,6 +1479,10 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
> if (pipe_config->pixel_multiplier)
> dotclock /= pipe_config->pixel_multiplier;
>
> + /* fec adds overhead to the data M/N values, correct for it */
> + if (pipe_config->fec_enable)
> + dotclock = intel_dp_fec_to_mode_clock(dotclock);
> +
> pipe_config->base.adjusted_mode.crtc_clock = dotclock;
> }
>
> @@ -4031,7 +4035,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> case TRANS_DDI_MODE_SELECT_FDI:
> pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
> break;
> - case TRANS_DDI_MODE_SELECT_DP_SST:
> + case TRANS_DDI_MODE_SELECT_DP_SST: {
> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> if (encoder->type == INTEL_OUTPUT_EDP)
> pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> else
> @@ -4039,7 +4045,18 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> pipe_config->lane_count =
> ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> intel_dp_get_m_n(intel_crtc, pipe_config);
> +
> + if (INTEL_GEN(dev_priv) >= 11) {
> + pipe_config->fec_enable =
> + I915_READ(intel_dp->regs.dp_tp_ctl) &
> + DP_TP_CTL_FEC_ENABLE;
Side note: That looks broken for the init/resume readout.
I knew there was a reason I didn't quite like the idea of
intel_dp->regs...
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
> + encoder->base.base.id, encoder->base.name,
> + pipe_config->fec_enable);
> + }
> +
> break;
> + }
> case TRANS_DDI_MODE_SELECT_DP_MST:
> pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> pipe_config->lane_count =
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e0033d99f6e3..7996864e6f7c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12773,6 +12773,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> PIPE_CONF_CHECK_BOOL(has_infoframe);
> + PIPE_CONF_CHECK_BOOL(fec_enable);
>
> PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index ccaf9f00b747..4dfb78dc7fa2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -76,8 +76,8 @@
> #define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
> #define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
>
> -/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
> -#define DP_DSC_FEC_OVERHEAD_FACTOR 976
> +/* DP DSC FEC Overhead factor = 1/(0.972261) */
> +#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
>
> /* Compliance test status bits */
> #define INTEL_DP_RESOLUTION_SHIFT_MASK 0
> @@ -492,6 +492,104 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> return 0;
> }
>
> +static inline u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
> +{
> + return div_u64(mul_u32_u32(mode_clock, 1000000U),
> + DP_DSC_FEC_OVERHEAD_FACTOR);
> +}
> +
> +u32 intel_dp_fec_to_mode_clock(u32 fec_clock)
> +{
> + return div_u64(mul_u32_u32(fec_clock,
> + DP_DSC_FEC_OVERHEAD_FACTOR),
> + 1000000U);
> +}
> +
> +static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
> + u32 mode_clock, u32 mode_hdisplay)
> +{
> + u32 bits_per_pixel, max_bpp_small_joiner_ram;
> + int i;
> +
> + /*
> + * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> + * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
> + * for SST -> TimeSlotsPerMTP is 1,
> + * for MST -> TimeSlotsPerMTP has to be calculated
> + */
> + bits_per_pixel = (link_clock * lane_count * 8) /
> + intel_dp_mode_to_fec_clock(mode_clock);
> + DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
> +
> + /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> + max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
> + DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
> +
> + /*
> + * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
> + * check, output bpp from small joiner RAM check)
> + */
> + bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> +
> + /* Error out if the max bpp is less than smallest allowed valid bpp */
> + if (bits_per_pixel < valid_dsc_bpp[0]) {
> + DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
> + bits_per_pixel, valid_dsc_bpp[0]);
> + return 0;
> + }
> +
> + /* Find the nearest match in the array of known BPPs from VESA */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> + if (bits_per_pixel < valid_dsc_bpp[i + 1])
> + break;
> + }
> + bits_per_pixel = valid_dsc_bpp[i];
> +
> + /*
> + * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> + * fractional part is 0
> + */
> + return bits_per_pixel << 4;
> +}
> +
> +static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> + int mode_clock, int mode_hdisplay)
> +{
> + u8 min_slice_count, i;
> + int max_slice_width;
> +
> + if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_0);
> + else
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_1);
> +
> + max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> + if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> + DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> + max_slice_width);
> + return 0;
> + }
> + /* Also take into account max slice width */
> + min_slice_count = min_t(u8, min_slice_count,
> + DIV_ROUND_UP(mode_hdisplay,
> + max_slice_width));
> +
> + /* Find the closest match to the valid slice count values */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> + if (valid_dsc_slicecount[i] >
> + drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> + false))
> + break;
> + if (min_slice_count <= valid_dsc_slicecount[i])
> + return valid_dsc_slicecount[i];
> + }
> +
> + DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> + return 0;
> +}
> +
> static enum drm_mode_status
> intel_dp_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> @@ -2182,6 +2280,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> DP_DPCD_QUIRK_CONSTANT_N);
> int ret = 0, output_bpp;
> + u32 data_clock;
>
> if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
> pipe_config->has_pch_encoder = true;
> @@ -2244,9 +2343,14 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> else
> output_bpp = intel_dp_output_bpp(pipe_config, pipe_config->pipe_bpp);
>
> + if (pipe_config->fec_enable)
> + data_clock = intel_dp_mode_to_fec_clock(adjusted_mode->crtc_clock);
> + else
> + data_clock = adjusted_mode->crtc_clock;
This looks wrong to me. The link M/N are used the regenerate the
stream clock from the link symbol clock. AFAICS the only effect FEC
should have is that the overhead may require us to bump the LS clock
a bit higher. But the M/N is stil computed simply as the ratio
between LS clock and stream clock.
The data M/N do need to account for FEC. I guess that's the problem
you're trying to address here?
> +
> intel_link_compute_m_n(output_bpp,
> pipe_config->lane_count,
> - adjusted_mode->crtc_clock,
> + data_clock,
> pipe_config->port_clock,
> &pipe_config->dp_m_n,
> constant_n);
> @@ -4363,91 +4467,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
> DP_DPRX_ESI_LEN;
> }
>
> -u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
> - int mode_clock, int mode_hdisplay)
> -{
> - u16 bits_per_pixel, max_bpp_small_joiner_ram;
> - int i;
> -
> - /*
> - * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> - * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
> - * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
> - * for MST -> TimeSlotsPerMTP has to be calculated
> - */
> - bits_per_pixel = (link_clock * lane_count * 8 *
> - DP_DSC_FEC_OVERHEAD_FACTOR) /
> - mode_clock;
> -
> - /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> - max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
> - mode_hdisplay;
> -
> - /*
> - * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
> - * check, output bpp from small joiner RAM check)
> - */
> - bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> -
> - /* Error out if the max bpp is less than smallest allowed valid bpp */
> - if (bits_per_pixel < valid_dsc_bpp[0]) {
> - DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
> - return 0;
> - }
> -
> - /* Find the nearest match in the array of known BPPs from VESA */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> - if (bits_per_pixel < valid_dsc_bpp[i + 1])
> - break;
> - }
> - bits_per_pixel = valid_dsc_bpp[i];
> -
> - /*
> - * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> - * fractional part is 0
> - */
> - return bits_per_pixel << 4;
> -}
> -
> -u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> - int mode_clock,
> - int mode_hdisplay)
> -{
> - u8 min_slice_count, i;
> - int max_slice_width;
> -
> - if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_0);
> - else
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_1);
> -
> - max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> - if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> - DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> - max_slice_width);
> - return 0;
> - }
> - /* Also take into account max slice width */
> - min_slice_count = min_t(u8, min_slice_count,
> - DIV_ROUND_UP(mode_hdisplay,
> - max_slice_width));
> -
> - /* Find the closest match to the valid slice count values */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> - if (valid_dsc_slicecount[i] >
> - drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> - false))
> - break;
> - if (min_slice_count <= valid_dsc_slicecount[i])
> - return valid_dsc_slicecount[i];
> - }
> -
> - DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> - return 0;
> -}
> -
> static void
> intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index e01d1f89409d..e9f11e698697 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
> bool
> intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
> -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);
>
> bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
> @@ -119,4 +115,6 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> return ~((1 << lane_count) - 1) & 0xf;
> }
>
> +u32 intel_dp_fec_to_mode_clock(u32 fec_clock);
> +
> #endif /* __INTEL_DP_H__ */
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3.
2019-09-20 16:38 ` [Intel-gfx] " Ville Syrjälä
@ 2019-09-23 12:52 ` Maarten Lankhorst
2019-09-23 13:03 ` Ville Syrjälä
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Maarten Lankhorst @ 2019-09-23 12:52 UTC (permalink / raw)
To: intel-gfx
Cc: Ville Syrjälä, Maarten Lankhorst, stable, Manasi Navare
There was a integer wraparound when mode_clock became too high,
and we didn't correct for the FEC overhead factor when dividing,
with the calculations breaking at HBR3.
As a result our calculated bpp was way too high, and the link width
limitation never came into effect.
Print out the resulting bpp calcululations as a sanity check, just
in case we ever have to debug it later on again.
We also used the wrong factor for FEC. While bspec mentions 2.4%,
all the calculations use 1/0.972261, and the same ratio should be
applied to data M/N as well, so use it there when FEC is enabled.
Make sure we don't break hw readout, and read out FEC enable state
and correct the DDI clock readout for the new values.
This fixes the FIFO underrun we are seeing with FEC enabled.
Changes since v2:
- Handle fec_enable in intel_link_compute_m_n, so only data M/N is adjusted. (Ville)
- Fix initial hardware readout for FEC. (Ville)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
Cc: <stable@vger.kernel.org> # v5.0+
Cc: Manasi Navare <manasi.d.navare@intel.com>
---
Thanks, that fixed the FIFO underrun, making the disablement patch obsolete.
Bigjoiner is now completely working as intended. :)
drivers/gpu/drm/i915/display/intel_ddi.c | 21 ++
drivers/gpu/drm/i915/display/intel_display.c | 13 +-
drivers/gpu/drm/i915/display/intel_display.h | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c | 191 ++++++++++---------
drivers/gpu/drm/i915/display/intel_dp.h | 7 +-
drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
6 files changed, 137 insertions(+), 99 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 0c0da9f6c2e8..3e77b30d91d5 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1479,6 +1479,10 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
if (pipe_config->pixel_multiplier)
dotclock /= pipe_config->pixel_multiplier;
+ /* fec adds overhead to the data M/N values, correct for it */
+ if (pipe_config->fec_enable)
+ dotclock = intel_dp_fec_to_mode_clock(dotclock);
+
pipe_config->base.adjusted_mode.crtc_clock = dotclock;
}
@@ -4045,6 +4049,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
pipe_config->lane_count =
((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
intel_dp_get_m_n(intel_crtc, pipe_config);
+
+ if (INTEL_GEN(dev_priv) >= 11) {
+ i915_reg_t dp_tp_ctl;
+
+ if (IS_GEN(dev_priv, 11))
+ dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
+ else
+ dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
+
+ pipe_config->fec_enable =
+ I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
+
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
+ encoder->base.base.id, encoder->base.name,
+ pipe_config->fec_enable);
+ }
+
break;
case TRANS_DDI_MODE_SELECT_DP_MST:
pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5ecf54270181..31698a57773f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7291,7 +7291,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
pipe_config->fdi_lanes = lane;
intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
- link_bw, &pipe_config->fdi_m_n, false);
+ link_bw, &pipe_config->fdi_m_n, false, false);
ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
if (ret == -EDEADLK)
@@ -7538,11 +7538,15 @@ void
intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
int pixel_clock, int link_clock,
struct intel_link_m_n *m_n,
- bool constant_n)
+ bool constant_n, bool fec_enable)
{
- m_n->tu = 64;
+ u32 data_clock = bits_per_pixel * pixel_clock;
+
+ if (fec_enable)
+ data_clock = intel_dp_mode_to_fec_clock(data_clock);
- compute_m_n(bits_per_pixel * pixel_clock,
+ m_n->tu = 64;
+ compute_m_n(data_clock,
link_clock * nlanes * 8,
&m_n->gmch_m, &m_n->gmch_n,
constant_n);
@@ -12832,6 +12836,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
PIPE_CONF_CHECK_BOOL(has_infoframe);
+ PIPE_CONF_CHECK_BOOL(fec_enable);
PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 5cea6f8e107a..4b9e18e5a263 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -443,7 +443,7 @@ enum phy_fia {
void intel_link_compute_m_n(u16 bpp, int nlanes,
int pixel_clock, int link_clock,
struct intel_link_m_n *m_n,
- bool constant_n);
+ bool constant_n, bool fec_enable);
bool is_ccs_modifier(u64 modifier);
void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 829559f97440..2d3f4183f99d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -76,8 +76,8 @@
#define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
#define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
-/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
-#define DP_DSC_FEC_OVERHEAD_FACTOR 976
+/* DP DSC FEC Overhead factor = 1/(0.972261) */
+#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
/* Compliance test status bits */
#define INTEL_DP_RESOLUTION_SHIFT_MASK 0
@@ -492,6 +492,104 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
return 0;
}
+u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
+{
+ return div_u64(mul_u32_u32(mode_clock, 1000000U),
+ DP_DSC_FEC_OVERHEAD_FACTOR);
+}
+
+u32 intel_dp_fec_to_mode_clock(u32 fec_clock)
+{
+ return div_u64(mul_u32_u32(fec_clock,
+ DP_DSC_FEC_OVERHEAD_FACTOR),
+ 1000000U);
+}
+
+static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
+ u32 mode_clock, u32 mode_hdisplay)
+{
+ u32 bits_per_pixel, max_bpp_small_joiner_ram;
+ int i;
+
+ /*
+ * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
+ * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
+ * for SST -> TimeSlotsPerMTP is 1,
+ * for MST -> TimeSlotsPerMTP has to be calculated
+ */
+ bits_per_pixel = (link_clock * lane_count * 8) /
+ intel_dp_mode_to_fec_clock(mode_clock);
+ DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
+
+ /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
+ max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
+ DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
+
+ /*
+ * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
+ * check, output bpp from small joiner RAM check)
+ */
+ bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
+
+ /* Error out if the max bpp is less than smallest allowed valid bpp */
+ if (bits_per_pixel < valid_dsc_bpp[0]) {
+ DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
+ bits_per_pixel, valid_dsc_bpp[0]);
+ return 0;
+ }
+
+ /* Find the nearest match in the array of known BPPs from VESA */
+ for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
+ if (bits_per_pixel < valid_dsc_bpp[i + 1])
+ break;
+ }
+ bits_per_pixel = valid_dsc_bpp[i];
+
+ /*
+ * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
+ * fractional part is 0
+ */
+ return bits_per_pixel << 4;
+}
+
+static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
+ int mode_clock, int mode_hdisplay)
+{
+ u8 min_slice_count, i;
+ int max_slice_width;
+
+ if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
+ min_slice_count = DIV_ROUND_UP(mode_clock,
+ DP_DSC_MAX_ENC_THROUGHPUT_0);
+ else
+ min_slice_count = DIV_ROUND_UP(mode_clock,
+ DP_DSC_MAX_ENC_THROUGHPUT_1);
+
+ max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
+ if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
+ DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
+ max_slice_width);
+ return 0;
+ }
+ /* Also take into account max slice width */
+ min_slice_count = min_t(u8, min_slice_count,
+ DIV_ROUND_UP(mode_hdisplay,
+ max_slice_width));
+
+ /* Find the closest match to the valid slice count values */
+ for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
+ if (valid_dsc_slicecount[i] >
+ drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
+ false))
+ break;
+ if (min_slice_count <= valid_dsc_slicecount[i])
+ return valid_dsc_slicecount[i];
+ }
+
+ DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
+ return 0;
+}
+
static enum drm_mode_status
intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
@@ -2259,7 +2357,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
adjusted_mode->crtc_clock,
pipe_config->port_clock,
&pipe_config->dp_m_n,
- constant_n);
+ constant_n, pipe_config->fec_enable);
if (intel_connector->panel.downclock_mode != NULL &&
dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
@@ -2269,7 +2367,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
intel_connector->panel.downclock_mode->clock,
pipe_config->port_clock,
&pipe_config->dp_m2_n2,
- constant_n);
+ constant_n, pipe_config->fec_enable);
}
if (!HAS_DDI(dev_priv))
@@ -4373,91 +4471,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
DP_DPRX_ESI_LEN;
}
-u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
- int mode_clock, int mode_hdisplay)
-{
- u16 bits_per_pixel, max_bpp_small_joiner_ram;
- int i;
-
- /*
- * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
- * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
- * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
- * for MST -> TimeSlotsPerMTP has to be calculated
- */
- bits_per_pixel = (link_clock * lane_count * 8 *
- DP_DSC_FEC_OVERHEAD_FACTOR) /
- mode_clock;
-
- /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
- max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
- mode_hdisplay;
-
- /*
- * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
- * check, output bpp from small joiner RAM check)
- */
- bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
-
- /* Error out if the max bpp is less than smallest allowed valid bpp */
- if (bits_per_pixel < valid_dsc_bpp[0]) {
- DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
- return 0;
- }
-
- /* Find the nearest match in the array of known BPPs from VESA */
- for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
- if (bits_per_pixel < valid_dsc_bpp[i + 1])
- break;
- }
- bits_per_pixel = valid_dsc_bpp[i];
-
- /*
- * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
- * fractional part is 0
- */
- return bits_per_pixel << 4;
-}
-
-u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
- int mode_clock,
- int mode_hdisplay)
-{
- u8 min_slice_count, i;
- int max_slice_width;
-
- if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
- min_slice_count = DIV_ROUND_UP(mode_clock,
- DP_DSC_MAX_ENC_THROUGHPUT_0);
- else
- min_slice_count = DIV_ROUND_UP(mode_clock,
- DP_DSC_MAX_ENC_THROUGHPUT_1);
-
- max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
- if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
- DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
- max_slice_width);
- return 0;
- }
- /* Also take into account max slice width */
- min_slice_count = min_t(u8, min_slice_count,
- DIV_ROUND_UP(mode_hdisplay,
- max_slice_width));
-
- /* Find the closest match to the valid slice count values */
- for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
- if (valid_dsc_slicecount[i] >
- drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
- false))
- break;
- if (min_slice_count <= valid_dsc_slicecount[i])
- return valid_dsc_slicecount[i];
- }
-
- DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
- return 0;
-}
-
static void
intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index e01d1f89409d..2147d3c14870 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
bool
intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
-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);
bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
@@ -119,4 +115,7 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
return ~((1 << lane_count) - 1) & 0xf;
}
+u32 intel_dp_fec_to_mode_clock(u32 fec_clock);
+u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
+
#endif /* __INTEL_DP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index eeeb3f933aa4..cf4d851a5139 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -81,7 +81,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
adjusted_mode->crtc_clock,
crtc_state->port_clock,
&crtc_state->dp_m_n,
- constant_n);
+ constant_n, crtc_state->fec_enable);
crtc_state->dp_m_n.tu = slots;
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3.
2019-09-23 12:52 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3 Maarten Lankhorst
@ 2019-09-23 13:03 ` Ville Syrjälä
2019-09-23 14:49 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4 Maarten Lankhorst
2019-09-23 14:22 ` [Intel-gfx] [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3 kbuild test robot
2019-09-23 15:53 ` Manasi Navare
2 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2019-09-23 13:03 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, stable, Manasi Navare
On Mon, Sep 23, 2019 at 02:52:52PM +0200, Maarten Lankhorst wrote:
> There was a integer wraparound when mode_clock became too high,
> and we didn't correct for the FEC overhead factor when dividing,
> with the calculations breaking at HBR3.
>
> As a result our calculated bpp was way too high, and the link width
> limitation never came into effect.
>
> Print out the resulting bpp calcululations as a sanity check, just
> in case we ever have to debug it later on again.
>
> We also used the wrong factor for FEC. While bspec mentions 2.4%,
> all the calculations use 1/0.972261, and the same ratio should be
> applied to data M/N as well, so use it there when FEC is enabled.
>
> Make sure we don't break hw readout, and read out FEC enable state
> and correct the DDI clock readout for the new values.
>
> This fixes the FIFO underrun we are seeing with FEC enabled.
>
> Changes since v2:
> - Handle fec_enable in intel_link_compute_m_n, so only data M/N is adjusted. (Ville)
> - Fix initial hardware readout for FEC. (Ville)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
> Cc: <stable@vger.kernel.org> # v5.0+
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
> Thanks, that fixed the FIFO underrun, making the disablement patch obsolete.
> Bigjoiner is now completely working as intended. :)
>
> drivers/gpu/drm/i915/display/intel_ddi.c | 21 ++
> drivers/gpu/drm/i915/display/intel_display.c | 13 +-
> drivers/gpu/drm/i915/display/intel_display.h | 2 +-
> drivers/gpu/drm/i915/display/intel_dp.c | 191 ++++++++++---------
> drivers/gpu/drm/i915/display/intel_dp.h | 7 +-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> 6 files changed, 137 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 0c0da9f6c2e8..3e77b30d91d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1479,6 +1479,10 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
> if (pipe_config->pixel_multiplier)
> dotclock /= pipe_config->pixel_multiplier;
>
> + /* fec adds overhead to the data M/N values, correct for it */
> + if (pipe_config->fec_enable)
> + dotclock = intel_dp_fec_to_mode_clock(dotclock);
That's still nonsense.
> +
> pipe_config->base.adjusted_mode.crtc_clock = dotclock;
> }
>
> @@ -4045,6 +4049,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> pipe_config->lane_count =
> ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> intel_dp_get_m_n(intel_crtc, pipe_config);
> +
> + if (INTEL_GEN(dev_priv) >= 11) {
> + i915_reg_t dp_tp_ctl;
> +
> + if (IS_GEN(dev_priv, 11))
> + dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
> + else
> + dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
> +
> + pipe_config->fec_enable =
> + I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
> +
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
> + encoder->base.base.id, encoder->base.name,
> + pipe_config->fec_enable);
> + }
> +
> break;
> case TRANS_DDI_MODE_SELECT_DP_MST:
> pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5ecf54270181..31698a57773f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7291,7 +7291,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> pipe_config->fdi_lanes = lane;
>
> intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> - link_bw, &pipe_config->fdi_m_n, false);
> + link_bw, &pipe_config->fdi_m_n, false, false);
>
> ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> if (ret == -EDEADLK)
> @@ -7538,11 +7538,15 @@ void
> intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> - bool constant_n)
> + bool constant_n, bool fec_enable)
> {
> - m_n->tu = 64;
> + u32 data_clock = bits_per_pixel * pixel_clock;
> +
> + if (fec_enable)
> + data_clock = intel_dp_mode_to_fec_clock(data_clock);
>
> - compute_m_n(bits_per_pixel * pixel_clock,
> + m_n->tu = 64;
> + compute_m_n(data_clock,
> link_clock * nlanes * 8,
> &m_n->gmch_m, &m_n->gmch_n,
> constant_n);
> @@ -12832,6 +12836,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> PIPE_CONF_CHECK_BOOL(has_infoframe);
> + PIPE_CONF_CHECK_BOOL(fec_enable);
>
> PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 5cea6f8e107a..4b9e18e5a263 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -443,7 +443,7 @@ enum phy_fia {
> void intel_link_compute_m_n(u16 bpp, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> - bool constant_n);
> + bool constant_n, bool fec_enable);
> bool is_ccs_modifier(u64 modifier);
> void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
> u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 829559f97440..2d3f4183f99d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -76,8 +76,8 @@
> #define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
> #define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
>
> -/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
> -#define DP_DSC_FEC_OVERHEAD_FACTOR 976
> +/* DP DSC FEC Overhead factor = 1/(0.972261) */
> +#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
>
> /* Compliance test status bits */
> #define INTEL_DP_RESOLUTION_SHIFT_MASK 0
> @@ -492,6 +492,104 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> return 0;
> }
>
> +u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
> +{
> + return div_u64(mul_u32_u32(mode_clock, 1000000U),
> + DP_DSC_FEC_OVERHEAD_FACTOR);
> +}
> +
> +u32 intel_dp_fec_to_mode_clock(u32 fec_clock)
> +{
> + return div_u64(mul_u32_u32(fec_clock,
> + DP_DSC_FEC_OVERHEAD_FACTOR),
> + 1000000U);
> +}
> +
> +static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
> + u32 mode_clock, u32 mode_hdisplay)
> +{
> + u32 bits_per_pixel, max_bpp_small_joiner_ram;
> + int i;
> +
> + /*
> + * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> + * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
> + * for SST -> TimeSlotsPerMTP is 1,
> + * for MST -> TimeSlotsPerMTP has to be calculated
> + */
> + bits_per_pixel = (link_clock * lane_count * 8) /
> + intel_dp_mode_to_fec_clock(mode_clock);
> + DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
> +
> + /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> + max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
> + DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
> +
> + /*
> + * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
> + * check, output bpp from small joiner RAM check)
> + */
> + bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> +
> + /* Error out if the max bpp is less than smallest allowed valid bpp */
> + if (bits_per_pixel < valid_dsc_bpp[0]) {
> + DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
> + bits_per_pixel, valid_dsc_bpp[0]);
> + return 0;
> + }
> +
> + /* Find the nearest match in the array of known BPPs from VESA */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> + if (bits_per_pixel < valid_dsc_bpp[i + 1])
> + break;
> + }
> + bits_per_pixel = valid_dsc_bpp[i];
> +
> + /*
> + * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> + * fractional part is 0
> + */
> + return bits_per_pixel << 4;
> +}
> +
> +static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> + int mode_clock, int mode_hdisplay)
> +{
> + u8 min_slice_count, i;
> + int max_slice_width;
> +
> + if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_0);
> + else
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_1);
> +
> + max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> + if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> + DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> + max_slice_width);
> + return 0;
> + }
> + /* Also take into account max slice width */
> + min_slice_count = min_t(u8, min_slice_count,
> + DIV_ROUND_UP(mode_hdisplay,
> + max_slice_width));
> +
> + /* Find the closest match to the valid slice count values */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> + if (valid_dsc_slicecount[i] >
> + drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> + false))
> + break;
> + if (min_slice_count <= valid_dsc_slicecount[i])
> + return valid_dsc_slicecount[i];
> + }
> +
> + DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> + return 0;
> +}
> +
> static enum drm_mode_status
> intel_dp_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> @@ -2259,7 +2357,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> adjusted_mode->crtc_clock,
> pipe_config->port_clock,
> &pipe_config->dp_m_n,
> - constant_n);
> + constant_n, pipe_config->fec_enable);
>
> if (intel_connector->panel.downclock_mode != NULL &&
> dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> @@ -2269,7 +2367,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> intel_connector->panel.downclock_mode->clock,
> pipe_config->port_clock,
> &pipe_config->dp_m2_n2,
> - constant_n);
> + constant_n, pipe_config->fec_enable);
> }
>
> if (!HAS_DDI(dev_priv))
> @@ -4373,91 +4471,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
> DP_DPRX_ESI_LEN;
> }
>
> -u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
> - int mode_clock, int mode_hdisplay)
> -{
> - u16 bits_per_pixel, max_bpp_small_joiner_ram;
> - int i;
> -
> - /*
> - * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> - * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
> - * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
> - * for MST -> TimeSlotsPerMTP has to be calculated
> - */
> - bits_per_pixel = (link_clock * lane_count * 8 *
> - DP_DSC_FEC_OVERHEAD_FACTOR) /
> - mode_clock;
> -
> - /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> - max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
> - mode_hdisplay;
> -
> - /*
> - * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
> - * check, output bpp from small joiner RAM check)
> - */
> - bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> -
> - /* Error out if the max bpp is less than smallest allowed valid bpp */
> - if (bits_per_pixel < valid_dsc_bpp[0]) {
> - DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
> - return 0;
> - }
> -
> - /* Find the nearest match in the array of known BPPs from VESA */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> - if (bits_per_pixel < valid_dsc_bpp[i + 1])
> - break;
> - }
> - bits_per_pixel = valid_dsc_bpp[i];
> -
> - /*
> - * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> - * fractional part is 0
> - */
> - return bits_per_pixel << 4;
> -}
> -
> -u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> - int mode_clock,
> - int mode_hdisplay)
> -{
> - u8 min_slice_count, i;
> - int max_slice_width;
> -
> - if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_0);
> - else
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_1);
> -
> - max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> - if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> - DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> - max_slice_width);
> - return 0;
> - }
> - /* Also take into account max slice width */
> - min_slice_count = min_t(u8, min_slice_count,
> - DIV_ROUND_UP(mode_hdisplay,
> - max_slice_width));
> -
> - /* Find the closest match to the valid slice count values */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> - if (valid_dsc_slicecount[i] >
> - drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> - false))
> - break;
> - if (min_slice_count <= valid_dsc_slicecount[i])
> - return valid_dsc_slicecount[i];
> - }
> -
> - DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> - return 0;
> -}
> -
> static void
> intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index e01d1f89409d..2147d3c14870 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
> bool
> intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
> -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);
>
> bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
> @@ -119,4 +115,7 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> return ~((1 << lane_count) - 1) & 0xf;
> }
>
> +u32 intel_dp_fec_to_mode_clock(u32 fec_clock);
> +u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
> +
> #endif /* __INTEL_DP_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index eeeb3f933aa4..cf4d851a5139 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -81,7 +81,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> adjusted_mode->crtc_clock,
> crtc_state->port_clock,
> &crtc_state->dp_m_n,
> - constant_n);
> + constant_n, crtc_state->fec_enable);
> crtc_state->dp_m_n.tu = slots;
>
> return 0;
> --
> 2.20.1
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3.
2019-09-23 12:52 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3 Maarten Lankhorst
2019-09-23 13:03 ` Ville Syrjälä
@ 2019-09-23 14:22 ` kbuild test robot
2019-09-23 15:53 ` Manasi Navare
2 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2019-09-23 14:22 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: kbuild-all, intel-gfx, stable
[-- Attachment #1: Type: text/plain, Size: 7236 bytes --]
Hi Maarten,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[cannot apply to v5.3 next-20190920]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-i915-dp-Fix-dsc-bpp-calculations-v3/20190923-205540
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_ddi.c: In function 'intel_ddi_get_config':
>> drivers/gpu/drm/i915/display/intel_ddi.c:3905:17: error: implicit declaration of function 'TGL_DP_TP_CTL'; did you mean 'DP_TP_CTL'? [-Werror=implicit-function-declaration]
dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
^~~~~~~~~~~~~
DP_TP_CTL
>> drivers/gpu/drm/i915/display/intel_ddi.c:3905:15: error: incompatible types when assigning to type 'i915_reg_t {aka struct <anonymous>}' from type 'int'
dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
^
cc1: some warnings being treated as errors
vim +3905 drivers/gpu/drm/i915/display/intel_ddi.c
3826
3827 void intel_ddi_get_config(struct intel_encoder *encoder,
3828 struct intel_crtc_state *pipe_config)
3829 {
3830 struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
3831 struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
3832 enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
3833 u32 temp, flags = 0;
3834
3835 /* XXX: DSI transcoder paranoia */
3836 if (WARN_ON(transcoder_is_dsi(cpu_transcoder)))
3837 return;
3838
3839 temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
3840 if (temp & TRANS_DDI_PHSYNC)
3841 flags |= DRM_MODE_FLAG_PHSYNC;
3842 else
3843 flags |= DRM_MODE_FLAG_NHSYNC;
3844 if (temp & TRANS_DDI_PVSYNC)
3845 flags |= DRM_MODE_FLAG_PVSYNC;
3846 else
3847 flags |= DRM_MODE_FLAG_NVSYNC;
3848
3849 pipe_config->base.adjusted_mode.flags |= flags;
3850
3851 switch (temp & TRANS_DDI_BPC_MASK) {
3852 case TRANS_DDI_BPC_6:
3853 pipe_config->pipe_bpp = 18;
3854 break;
3855 case TRANS_DDI_BPC_8:
3856 pipe_config->pipe_bpp = 24;
3857 break;
3858 case TRANS_DDI_BPC_10:
3859 pipe_config->pipe_bpp = 30;
3860 break;
3861 case TRANS_DDI_BPC_12:
3862 pipe_config->pipe_bpp = 36;
3863 break;
3864 default:
3865 break;
3866 }
3867
3868 switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
3869 case TRANS_DDI_MODE_SELECT_HDMI:
3870 pipe_config->has_hdmi_sink = true;
3871
3872 pipe_config->infoframes.enable |=
3873 intel_hdmi_infoframes_enabled(encoder, pipe_config);
3874
3875 if (pipe_config->infoframes.enable)
3876 pipe_config->has_infoframe = true;
3877
3878 if (temp & TRANS_DDI_HDMI_SCRAMBLING)
3879 pipe_config->hdmi_scrambling = true;
3880 if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
3881 pipe_config->hdmi_high_tmds_clock_ratio = true;
3882 /* fall through */
3883 case TRANS_DDI_MODE_SELECT_DVI:
3884 pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
3885 pipe_config->lane_count = 4;
3886 break;
3887 case TRANS_DDI_MODE_SELECT_FDI:
3888 pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
3889 break;
3890 case TRANS_DDI_MODE_SELECT_DP_SST:
3891 if (encoder->type == INTEL_OUTPUT_EDP)
3892 pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
3893 else
3894 pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
3895 pipe_config->lane_count =
3896 ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
3897 intel_dp_get_m_n(intel_crtc, pipe_config);
3898
3899 if (INTEL_GEN(dev_priv) >= 11) {
3900 i915_reg_t dp_tp_ctl;
3901
3902 if (IS_GEN(dev_priv, 11))
3903 dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
3904 else
> 3905 dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
3906
3907 pipe_config->fec_enable =
3908 I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
3909
3910 DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
3911 encoder->base.base.id, encoder->base.name,
3912 pipe_config->fec_enable);
3913 }
3914
3915 break;
3916 case TRANS_DDI_MODE_SELECT_DP_MST:
3917 pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
3918 pipe_config->lane_count =
3919 ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
3920 intel_dp_get_m_n(intel_crtc, pipe_config);
3921 break;
3922 default:
3923 break;
3924 }
3925
3926 pipe_config->has_audio =
3927 intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
3928
3929 if (encoder->type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.bpp &&
3930 pipe_config->pipe_bpp > dev_priv->vbt.edp.bpp) {
3931 /*
3932 * This is a big fat ugly hack.
3933 *
3934 * Some machines in UEFI boot mode provide us a VBT that has 18
3935 * bpp and 1.62 GHz link bandwidth for eDP, which for reasons
3936 * unknown we fail to light up. Yet the same BIOS boots up with
3937 * 24 bpp and 2.7 GHz link. Use the same bpp as the BIOS uses as
3938 * max, not what it tells us to use.
3939 *
3940 * Note: This will still be broken if the eDP panel is not lit
3941 * up by the BIOS, and thus we can't get the mode at module
3942 * load.
3943 */
3944 DRM_DEBUG_KMS("pipe has %d bpp for eDP panel, overriding BIOS-provided max %d bpp\n",
3945 pipe_config->pipe_bpp, dev_priv->vbt.edp.bpp);
3946 dev_priv->vbt.edp.bpp = pipe_config->pipe_bpp;
3947 }
3948
3949 intel_ddi_clock_get(encoder, pipe_config);
3950
3951 if (IS_GEN9_LP(dev_priv))
3952 pipe_config->lane_lat_optim_mask =
3953 bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
3954
3955 intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
3956
3957 intel_hdmi_read_gcp_infoframe(encoder, pipe_config);
3958
3959 intel_read_infoframe(encoder, pipe_config,
3960 HDMI_INFOFRAME_TYPE_AVI,
3961 &pipe_config->infoframes.avi);
3962 intel_read_infoframe(encoder, pipe_config,
3963 HDMI_INFOFRAME_TYPE_SPD,
3964 &pipe_config->infoframes.spd);
3965 intel_read_infoframe(encoder, pipe_config,
3966 HDMI_INFOFRAME_TYPE_VENDOR,
3967 &pipe_config->infoframes.hdmi);
3968 intel_read_infoframe(encoder, pipe_config,
3969 HDMI_INFOFRAME_TYPE_DRM,
3970 &pipe_config->infoframes.drm);
3971 }
3972
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28068 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4.
2019-09-23 13:03 ` Ville Syrjälä
@ 2019-09-23 14:49 ` Maarten Lankhorst
2019-09-23 14:50 ` Maarten Lankhorst
2019-09-23 14:57 ` Ville Syrjälä
0 siblings, 2 replies; 11+ messages in thread
From: Maarten Lankhorst @ 2019-09-23 14:49 UTC (permalink / raw)
To: intel-gfx
Cc: Ville Syrjälä, Maarten Lankhorst, stable, Manasi Navare
There was a integer wraparound when mode_clock became too high,
and we didn't correct for the FEC overhead factor when dividing,
with the calculations breaking at HBR3.
As a result our calculated bpp was way too high, and the link width
limitation never came into effect.
Print out the resulting bpp calcululations as a sanity check, just
in case we ever have to debug it later on again.
We also used the wrong factor for FEC. While bspec mentions 2.4%,
all the calculations use 1/0.972261, and the same ratio should be
applied to data M/N as well, so use it there when FEC is enabled.
Make sure we don't break hw readout, and read out FEC enable state
and correct the DDI clock readout for the new values.
This fixes the FIFO underrun we are seeing with FEC enabled.
Changes since v2:
- Handle fec_enable in intel_link_compute_m_n, so only data M/N is adjusted. (Ville)
- Fix initial hardware readout for FEC. (Ville)
Changes since v3:
- Remove bogus fec_to_mode_clock. (Ville)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
Cc: <stable@vger.kernel.org> # v5.0+
Cc: Manasi Navare <manasi.d.navare@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 17 ++
drivers/gpu/drm/i915/display/intel_display.c | 13 +-
drivers/gpu/drm/i915/display/intel_display.h | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c | 184 ++++++++++---------
drivers/gpu/drm/i915/display/intel_dp.h | 6 +-
drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
6 files changed, 125 insertions(+), 99 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 0c0da9f6c2e8..1cb297abd111 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4045,6 +4045,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
pipe_config->lane_count =
((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
intel_dp_get_m_n(intel_crtc, pipe_config);
+
+ if (INTEL_GEN(dev_priv) >= 11) {
+ i915_reg_t dp_tp_ctl;
+
+ if (IS_GEN(dev_priv, 11))
+ dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
+ else
+ dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
+
+ pipe_config->fec_enable =
+ I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
+
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
+ encoder->base.base.id, encoder->base.name,
+ pipe_config->fec_enable);
+ }
+
break;
case TRANS_DDI_MODE_SELECT_DP_MST:
pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5ecf54270181..31698a57773f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7291,7 +7291,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
pipe_config->fdi_lanes = lane;
intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
- link_bw, &pipe_config->fdi_m_n, false);
+ link_bw, &pipe_config->fdi_m_n, false, false);
ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
if (ret == -EDEADLK)
@@ -7538,11 +7538,15 @@ void
intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
int pixel_clock, int link_clock,
struct intel_link_m_n *m_n,
- bool constant_n)
+ bool constant_n, bool fec_enable)
{
- m_n->tu = 64;
+ u32 data_clock = bits_per_pixel * pixel_clock;
+
+ if (fec_enable)
+ data_clock = intel_dp_mode_to_fec_clock(data_clock);
- compute_m_n(bits_per_pixel * pixel_clock,
+ m_n->tu = 64;
+ compute_m_n(data_clock,
link_clock * nlanes * 8,
&m_n->gmch_m, &m_n->gmch_n,
constant_n);
@@ -12832,6 +12836,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
PIPE_CONF_CHECK_BOOL(has_infoframe);
+ PIPE_CONF_CHECK_BOOL(fec_enable);
PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 5cea6f8e107a..4b9e18e5a263 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -443,7 +443,7 @@ enum phy_fia {
void intel_link_compute_m_n(u16 bpp, int nlanes,
int pixel_clock, int link_clock,
struct intel_link_m_n *m_n,
- bool constant_n);
+ bool constant_n, bool fec_enable);
bool is_ccs_modifier(u64 modifier);
void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 829559f97440..2b1e71f992b0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -76,8 +76,8 @@
#define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
#define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
-/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
-#define DP_DSC_FEC_OVERHEAD_FACTOR 976
+/* DP DSC FEC Overhead factor = 1/(0.972261) */
+#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
/* Compliance test status bits */
#define INTEL_DP_RESOLUTION_SHIFT_MASK 0
@@ -492,6 +492,97 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
return 0;
}
+u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
+{
+ return div_u64(mul_u32_u32(mode_clock, 1000000U),
+ DP_DSC_FEC_OVERHEAD_FACTOR);
+}
+
+static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
+ u32 mode_clock, u32 mode_hdisplay)
+{
+ u32 bits_per_pixel, max_bpp_small_joiner_ram;
+ int i;
+
+ /*
+ * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
+ * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
+ * for SST -> TimeSlotsPerMTP is 1,
+ * for MST -> TimeSlotsPerMTP has to be calculated
+ */
+ bits_per_pixel = (link_clock * lane_count * 8) /
+ intel_dp_mode_to_fec_clock(mode_clock);
+ DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
+
+ /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
+ max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
+ DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
+
+ /*
+ * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
+ * check, output bpp from small joiner RAM check)
+ */
+ bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
+
+ /* Error out if the max bpp is less than smallest allowed valid bpp */
+ if (bits_per_pixel < valid_dsc_bpp[0]) {
+ DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
+ bits_per_pixel, valid_dsc_bpp[0]);
+ return 0;
+ }
+
+ /* Find the nearest match in the array of known BPPs from VESA */
+ for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
+ if (bits_per_pixel < valid_dsc_bpp[i + 1])
+ break;
+ }
+ bits_per_pixel = valid_dsc_bpp[i];
+
+ /*
+ * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
+ * fractional part is 0
+ */
+ return bits_per_pixel << 4;
+}
+
+static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
+ int mode_clock, int mode_hdisplay)
+{
+ u8 min_slice_count, i;
+ int max_slice_width;
+
+ if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
+ min_slice_count = DIV_ROUND_UP(mode_clock,
+ DP_DSC_MAX_ENC_THROUGHPUT_0);
+ else
+ min_slice_count = DIV_ROUND_UP(mode_clock,
+ DP_DSC_MAX_ENC_THROUGHPUT_1);
+
+ max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
+ if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
+ DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
+ max_slice_width);
+ return 0;
+ }
+ /* Also take into account max slice width */
+ min_slice_count = min_t(u8, min_slice_count,
+ DIV_ROUND_UP(mode_hdisplay,
+ max_slice_width));
+
+ /* Find the closest match to the valid slice count values */
+ for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
+ if (valid_dsc_slicecount[i] >
+ drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
+ false))
+ break;
+ if (min_slice_count <= valid_dsc_slicecount[i])
+ return valid_dsc_slicecount[i];
+ }
+
+ DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
+ return 0;
+}
+
static enum drm_mode_status
intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
@@ -2259,7 +2350,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
adjusted_mode->crtc_clock,
pipe_config->port_clock,
&pipe_config->dp_m_n,
- constant_n);
+ constant_n, pipe_config->fec_enable);
if (intel_connector->panel.downclock_mode != NULL &&
dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
@@ -2269,7 +2360,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
intel_connector->panel.downclock_mode->clock,
pipe_config->port_clock,
&pipe_config->dp_m2_n2,
- constant_n);
+ constant_n, pipe_config->fec_enable);
}
if (!HAS_DDI(dev_priv))
@@ -4373,91 +4464,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
DP_DPRX_ESI_LEN;
}
-u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
- int mode_clock, int mode_hdisplay)
-{
- u16 bits_per_pixel, max_bpp_small_joiner_ram;
- int i;
-
- /*
- * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
- * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
- * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
- * for MST -> TimeSlotsPerMTP has to be calculated
- */
- bits_per_pixel = (link_clock * lane_count * 8 *
- DP_DSC_FEC_OVERHEAD_FACTOR) /
- mode_clock;
-
- /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
- max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
- mode_hdisplay;
-
- /*
- * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
- * check, output bpp from small joiner RAM check)
- */
- bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
-
- /* Error out if the max bpp is less than smallest allowed valid bpp */
- if (bits_per_pixel < valid_dsc_bpp[0]) {
- DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
- return 0;
- }
-
- /* Find the nearest match in the array of known BPPs from VESA */
- for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
- if (bits_per_pixel < valid_dsc_bpp[i + 1])
- break;
- }
- bits_per_pixel = valid_dsc_bpp[i];
-
- /*
- * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
- * fractional part is 0
- */
- return bits_per_pixel << 4;
-}
-
-u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
- int mode_clock,
- int mode_hdisplay)
-{
- u8 min_slice_count, i;
- int max_slice_width;
-
- if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
- min_slice_count = DIV_ROUND_UP(mode_clock,
- DP_DSC_MAX_ENC_THROUGHPUT_0);
- else
- min_slice_count = DIV_ROUND_UP(mode_clock,
- DP_DSC_MAX_ENC_THROUGHPUT_1);
-
- max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
- if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
- DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
- max_slice_width);
- return 0;
- }
- /* Also take into account max slice width */
- min_slice_count = min_t(u8, min_slice_count,
- DIV_ROUND_UP(mode_hdisplay,
- max_slice_width));
-
- /* Find the closest match to the valid slice count values */
- for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
- if (valid_dsc_slicecount[i] >
- drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
- false))
- break;
- if (min_slice_count <= valid_dsc_slicecount[i])
- return valid_dsc_slicecount[i];
- }
-
- DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
- return 0;
-}
-
static void
intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index e01d1f89409d..a194b5b6da05 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
bool
intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
-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);
bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
@@ -119,4 +115,6 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
return ~((1 << lane_count) - 1) & 0xf;
}
+u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
+
#endif /* __INTEL_DP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index eeeb3f933aa4..cf4d851a5139 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -81,7 +81,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
adjusted_mode->crtc_clock,
crtc_state->port_clock,
&crtc_state->dp_m_n,
- constant_n);
+ constant_n, crtc_state->fec_enable);
crtc_state->dp_m_n.tu = slots;
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4.
2019-09-23 14:49 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4 Maarten Lankhorst
@ 2019-09-23 14:50 ` Maarten Lankhorst
2019-09-23 14:57 ` Ville Syrjälä
1 sibling, 0 replies; 11+ messages in thread
From: Maarten Lankhorst @ 2019-09-23 14:50 UTC (permalink / raw)
To: intel-gfx; +Cc: Ville Syrjälä, stable, Manasi Navare
Op 23-09-2019 om 16:49 schreef Maarten Lankhorst:
> There was a integer wraparound when mode_clock became too high,
> and we didn't correct for the FEC overhead factor when dividing,
> with the calculations breaking at HBR3.
>
> As a result our calculated bpp was way too high, and the link width
> limitation never came into effect.
>
> Print out the resulting bpp calcululations as a sanity check, just
> in case we ever have to debug it later on again.
>
> We also used the wrong factor for FEC. While bspec mentions 2.4%,
> all the calculations use 1/0.972261, and the same ratio should be
> applied to data M/N as well, so use it there when FEC is enabled.
>
> Make sure we don't break hw readout, and read out FEC enable state
> and correct the DDI clock readout for the new values.
Note to self, needs this removed from commit msg.
> This fixes the FIFO underrun we are seeing with FEC enabled.
>
> Changes since v2:
> - Handle fec_enable in intel_link_compute_m_n, so only data M/N is adjusted. (Ville)
> - Fix initial hardware readout for FEC. (Ville)
> Changes since v3:
> - Remove bogus fec_to_mode_clock. (Ville)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
> Cc: <stable@vger.kernel.org> # v5.0+
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 17 ++
> drivers/gpu/drm/i915/display/intel_display.c | 13 +-
> drivers/gpu/drm/i915/display/intel_display.h | 2 +-
> drivers/gpu/drm/i915/display/intel_dp.c | 184 ++++++++++---------
> drivers/gpu/drm/i915/display/intel_dp.h | 6 +-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> 6 files changed, 125 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 0c0da9f6c2e8..1cb297abd111 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4045,6 +4045,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> pipe_config->lane_count =
> ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> intel_dp_get_m_n(intel_crtc, pipe_config);
> +
> + if (INTEL_GEN(dev_priv) >= 11) {
> + i915_reg_t dp_tp_ctl;
> +
> + if (IS_GEN(dev_priv, 11))
> + dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
> + else
> + dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
> +
> + pipe_config->fec_enable =
> + I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
> +
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
> + encoder->base.base.id, encoder->base.name,
> + pipe_config->fec_enable);
> + }
> +
> break;
> case TRANS_DDI_MODE_SELECT_DP_MST:
> pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5ecf54270181..31698a57773f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7291,7 +7291,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> pipe_config->fdi_lanes = lane;
>
> intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> - link_bw, &pipe_config->fdi_m_n, false);
> + link_bw, &pipe_config->fdi_m_n, false, false);
>
> ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> if (ret == -EDEADLK)
> @@ -7538,11 +7538,15 @@ void
> intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> - bool constant_n)
> + bool constant_n, bool fec_enable)
> {
> - m_n->tu = 64;
> + u32 data_clock = bits_per_pixel * pixel_clock;
> +
> + if (fec_enable)
> + data_clock = intel_dp_mode_to_fec_clock(data_clock);
>
> - compute_m_n(bits_per_pixel * pixel_clock,
> + m_n->tu = 64;
> + compute_m_n(data_clock,
> link_clock * nlanes * 8,
> &m_n->gmch_m, &m_n->gmch_n,
> constant_n);
> @@ -12832,6 +12836,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> PIPE_CONF_CHECK_BOOL(has_infoframe);
> + PIPE_CONF_CHECK_BOOL(fec_enable);
>
> PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 5cea6f8e107a..4b9e18e5a263 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -443,7 +443,7 @@ enum phy_fia {
> void intel_link_compute_m_n(u16 bpp, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> - bool constant_n);
> + bool constant_n, bool fec_enable);
> bool is_ccs_modifier(u64 modifier);
> void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
> u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 829559f97440..2b1e71f992b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -76,8 +76,8 @@
> #define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
> #define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
>
> -/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
> -#define DP_DSC_FEC_OVERHEAD_FACTOR 976
> +/* DP DSC FEC Overhead factor = 1/(0.972261) */
> +#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
>
> /* Compliance test status bits */
> #define INTEL_DP_RESOLUTION_SHIFT_MASK 0
> @@ -492,6 +492,97 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> return 0;
> }
>
> +u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
> +{
> + return div_u64(mul_u32_u32(mode_clock, 1000000U),
> + DP_DSC_FEC_OVERHEAD_FACTOR);
> +}
> +
> +static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
> + u32 mode_clock, u32 mode_hdisplay)
> +{
> + u32 bits_per_pixel, max_bpp_small_joiner_ram;
> + int i;
> +
> + /*
> + * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> + * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
> + * for SST -> TimeSlotsPerMTP is 1,
> + * for MST -> TimeSlotsPerMTP has to be calculated
> + */
> + bits_per_pixel = (link_clock * lane_count * 8) /
> + intel_dp_mode_to_fec_clock(mode_clock);
> + DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
> +
> + /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> + max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
> + DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
> +
> + /*
> + * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
> + * check, output bpp from small joiner RAM check)
> + */
> + bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> +
> + /* Error out if the max bpp is less than smallest allowed valid bpp */
> + if (bits_per_pixel < valid_dsc_bpp[0]) {
> + DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
> + bits_per_pixel, valid_dsc_bpp[0]);
> + return 0;
> + }
> +
> + /* Find the nearest match in the array of known BPPs from VESA */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> + if (bits_per_pixel < valid_dsc_bpp[i + 1])
> + break;
> + }
> + bits_per_pixel = valid_dsc_bpp[i];
> +
> + /*
> + * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> + * fractional part is 0
> + */
> + return bits_per_pixel << 4;
> +}
> +
> +static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> + int mode_clock, int mode_hdisplay)
> +{
> + u8 min_slice_count, i;
> + int max_slice_width;
> +
> + if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_0);
> + else
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_1);
> +
> + max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> + if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> + DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> + max_slice_width);
> + return 0;
> + }
> + /* Also take into account max slice width */
> + min_slice_count = min_t(u8, min_slice_count,
> + DIV_ROUND_UP(mode_hdisplay,
> + max_slice_width));
> +
> + /* Find the closest match to the valid slice count values */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> + if (valid_dsc_slicecount[i] >
> + drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> + false))
> + break;
> + if (min_slice_count <= valid_dsc_slicecount[i])
> + return valid_dsc_slicecount[i];
> + }
> +
> + DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> + return 0;
> +}
> +
> static enum drm_mode_status
> intel_dp_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> @@ -2259,7 +2350,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> adjusted_mode->crtc_clock,
> pipe_config->port_clock,
> &pipe_config->dp_m_n,
> - constant_n);
> + constant_n, pipe_config->fec_enable);
>
> if (intel_connector->panel.downclock_mode != NULL &&
> dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> @@ -2269,7 +2360,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> intel_connector->panel.downclock_mode->clock,
> pipe_config->port_clock,
> &pipe_config->dp_m2_n2,
> - constant_n);
> + constant_n, pipe_config->fec_enable);
> }
>
> if (!HAS_DDI(dev_priv))
> @@ -4373,91 +4464,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
> DP_DPRX_ESI_LEN;
> }
>
> -u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
> - int mode_clock, int mode_hdisplay)
> -{
> - u16 bits_per_pixel, max_bpp_small_joiner_ram;
> - int i;
> -
> - /*
> - * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> - * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
> - * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
> - * for MST -> TimeSlotsPerMTP has to be calculated
> - */
> - bits_per_pixel = (link_clock * lane_count * 8 *
> - DP_DSC_FEC_OVERHEAD_FACTOR) /
> - mode_clock;
> -
> - /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> - max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
> - mode_hdisplay;
> -
> - /*
> - * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
> - * check, output bpp from small joiner RAM check)
> - */
> - bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> -
> - /* Error out if the max bpp is less than smallest allowed valid bpp */
> - if (bits_per_pixel < valid_dsc_bpp[0]) {
> - DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
> - return 0;
> - }
> -
> - /* Find the nearest match in the array of known BPPs from VESA */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> - if (bits_per_pixel < valid_dsc_bpp[i + 1])
> - break;
> - }
> - bits_per_pixel = valid_dsc_bpp[i];
> -
> - /*
> - * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> - * fractional part is 0
> - */
> - return bits_per_pixel << 4;
> -}
> -
> -u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> - int mode_clock,
> - int mode_hdisplay)
> -{
> - u8 min_slice_count, i;
> - int max_slice_width;
> -
> - if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_0);
> - else
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_1);
> -
> - max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> - if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> - DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> - max_slice_width);
> - return 0;
> - }
> - /* Also take into account max slice width */
> - min_slice_count = min_t(u8, min_slice_count,
> - DIV_ROUND_UP(mode_hdisplay,
> - max_slice_width));
> -
> - /* Find the closest match to the valid slice count values */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> - if (valid_dsc_slicecount[i] >
> - drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> - false))
> - break;
> - if (min_slice_count <= valid_dsc_slicecount[i])
> - return valid_dsc_slicecount[i];
> - }
> -
> - DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> - return 0;
> -}
> -
> static void
> intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index e01d1f89409d..a194b5b6da05 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
> bool
> intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
> -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);
>
> bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
> @@ -119,4 +115,6 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> return ~((1 << lane_count) - 1) & 0xf;
> }
>
> +u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
> +
> #endif /* __INTEL_DP_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index eeeb3f933aa4..cf4d851a5139 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -81,7 +81,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> adjusted_mode->crtc_clock,
> crtc_state->port_clock,
> &crtc_state->dp_m_n,
> - constant_n);
> + constant_n, crtc_state->fec_enable);
> crtc_state->dp_m_n.tu = slots;
>
> return 0;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4.
2019-09-23 14:49 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4 Maarten Lankhorst
2019-09-23 14:50 ` Maarten Lankhorst
@ 2019-09-23 14:57 ` Ville Syrjälä
2019-09-23 15:56 ` Manasi Navare
2019-09-23 15:56 ` [Intel-gfx] " Ville Syrjälä
1 sibling, 2 replies; 11+ messages in thread
From: Ville Syrjälä @ 2019-09-23 14:57 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, stable, Manasi Navare
On Mon, Sep 23, 2019 at 04:49:47PM +0200, Maarten Lankhorst wrote:
> There was a integer wraparound when mode_clock became too high,
> and we didn't correct for the FEC overhead factor when dividing,
> with the calculations breaking at HBR3.
>
> As a result our calculated bpp was way too high, and the link width
> limitation never came into effect.
>
> Print out the resulting bpp calcululations as a sanity check, just
> in case we ever have to debug it later on again.
>
> We also used the wrong factor for FEC. While bspec mentions 2.4%,
> all the calculations use 1/0.972261, and the same ratio should be
> applied to data M/N as well, so use it there when FEC is enabled.
>
> Make sure we don't break hw readout, and read out FEC enable state
> and correct the DDI clock readout for the new values.
>
> This fixes the FIFO underrun we are seeing with FEC enabled.
>
> Changes since v2:
> - Handle fec_enable in intel_link_compute_m_n, so only data M/N is adjusted. (Ville)
> - Fix initial hardware readout for FEC. (Ville)
> Changes since v3:
> - Remove bogus fec_to_mode_clock. (Ville)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
> Cc: <stable@vger.kernel.org> # v5.0+
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 17 ++
> drivers/gpu/drm/i915/display/intel_display.c | 13 +-
> drivers/gpu/drm/i915/display/intel_display.h | 2 +-
> drivers/gpu/drm/i915/display/intel_dp.c | 184 ++++++++++---------
> drivers/gpu/drm/i915/display/intel_dp.h | 6 +-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> 6 files changed, 125 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 0c0da9f6c2e8..1cb297abd111 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4045,6 +4045,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> pipe_config->lane_count =
> ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> intel_dp_get_m_n(intel_crtc, pipe_config);
> +
> + if (INTEL_GEN(dev_priv) >= 11) {
> + i915_reg_t dp_tp_ctl;
> +
> + if (IS_GEN(dev_priv, 11))
> + dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
> + else
> + dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
> +
> + pipe_config->fec_enable =
> + I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
Can you split the fec_enable readout/state check into a separate
patch?
I wonder how the lack of this stuff was missed when FEC was adeed...
> +
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
> + encoder->base.base.id, encoder->base.name,
> + pipe_config->fec_enable);
I'd just include it as part of the normal state dump.
> + }
> +
> break;
> case TRANS_DDI_MODE_SELECT_DP_MST:
> pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5ecf54270181..31698a57773f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7291,7 +7291,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> pipe_config->fdi_lanes = lane;
>
> intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> - link_bw, &pipe_config->fdi_m_n, false);
> + link_bw, &pipe_config->fdi_m_n, false, false);
>
> ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> if (ret == -EDEADLK)
> @@ -7538,11 +7538,15 @@ void
> intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> - bool constant_n)
> + bool constant_n, bool fec_enable)
> {
> - m_n->tu = 64;
> + u32 data_clock = bits_per_pixel * pixel_clock;
> +
> + if (fec_enable)
> + data_clock = intel_dp_mode_to_fec_clock(data_clock);
>
> - compute_m_n(bits_per_pixel * pixel_clock,
> + m_n->tu = 64;
> + compute_m_n(data_clock,
> link_clock * nlanes * 8,
> &m_n->gmch_m, &m_n->gmch_n,
> constant_n);
> @@ -12832,6 +12836,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> PIPE_CONF_CHECK_BOOL(has_infoframe);
> + PIPE_CONF_CHECK_BOOL(fec_enable);
>
> PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 5cea6f8e107a..4b9e18e5a263 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -443,7 +443,7 @@ enum phy_fia {
> void intel_link_compute_m_n(u16 bpp, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> - bool constant_n);
> + bool constant_n, bool fec_enable);
> bool is_ccs_modifier(u64 modifier);
> void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
> u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 829559f97440..2b1e71f992b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -76,8 +76,8 @@
> #define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
> #define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
>
> -/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
> -#define DP_DSC_FEC_OVERHEAD_FACTOR 976
> +/* DP DSC FEC Overhead factor = 1/(0.972261) */
> +#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
>
> /* Compliance test status bits */
> #define INTEL_DP_RESOLUTION_SHIFT_MASK 0
> @@ -492,6 +492,97 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> return 0;
> }
>
> +u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
> +{
> + return div_u64(mul_u32_u32(mode_clock, 1000000U),
> + DP_DSC_FEC_OVERHEAD_FACTOR);
> +}
> +
> +static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
> + u32 mode_clock, u32 mode_hdisplay)
> +{
> + u32 bits_per_pixel, max_bpp_small_joiner_ram;
> + int i;
> +
> + /*
> + * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> + * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
> + * for SST -> TimeSlotsPerMTP is 1,
> + * for MST -> TimeSlotsPerMTP has to be calculated
> + */
> + bits_per_pixel = (link_clock * lane_count * 8) /
> + intel_dp_mode_to_fec_clock(mode_clock);
> + DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
> +
> + /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> + max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
> + DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
> +
> + /*
> + * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
> + * check, output bpp from small joiner RAM check)
> + */
> + bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> +
> + /* Error out if the max bpp is less than smallest allowed valid bpp */
> + if (bits_per_pixel < valid_dsc_bpp[0]) {
> + DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
> + bits_per_pixel, valid_dsc_bpp[0]);
> + return 0;
> + }
> +
> + /* Find the nearest match in the array of known BPPs from VESA */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> + if (bits_per_pixel < valid_dsc_bpp[i + 1])
> + break;
> + }
> + bits_per_pixel = valid_dsc_bpp[i];
> +
> + /*
> + * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> + * fractional part is 0
> + */
> + return bits_per_pixel << 4;
> +}
> +
> +static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> + int mode_clock, int mode_hdisplay)
> +{
> + u8 min_slice_count, i;
> + int max_slice_width;
> +
> + if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_0);
> + else
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_1);
> +
> + max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> + if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> + DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> + max_slice_width);
> + return 0;
> + }
> + /* Also take into account max slice width */
> + min_slice_count = min_t(u8, min_slice_count,
> + DIV_ROUND_UP(mode_hdisplay,
> + max_slice_width));
> +
> + /* Find the closest match to the valid slice count values */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> + if (valid_dsc_slicecount[i] >
> + drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> + false))
> + break;
> + if (min_slice_count <= valid_dsc_slicecount[i])
> + return valid_dsc_slicecount[i];
> + }
> +
> + DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> + return 0;
> +}
> +
> static enum drm_mode_status
> intel_dp_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> @@ -2259,7 +2350,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> adjusted_mode->crtc_clock,
> pipe_config->port_clock,
> &pipe_config->dp_m_n,
> - constant_n);
> + constant_n, pipe_config->fec_enable);
>
> if (intel_connector->panel.downclock_mode != NULL &&
> dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> @@ -2269,7 +2360,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> intel_connector->panel.downclock_mode->clock,
> pipe_config->port_clock,
> &pipe_config->dp_m2_n2,
> - constant_n);
> + constant_n, pipe_config->fec_enable);
> }
>
> if (!HAS_DDI(dev_priv))
> @@ -4373,91 +4464,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
> DP_DPRX_ESI_LEN;
> }
>
> -u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
> - int mode_clock, int mode_hdisplay)
> -{
> - u16 bits_per_pixel, max_bpp_small_joiner_ram;
> - int i;
> -
> - /*
> - * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> - * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
> - * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
> - * for MST -> TimeSlotsPerMTP has to be calculated
> - */
> - bits_per_pixel = (link_clock * lane_count * 8 *
> - DP_DSC_FEC_OVERHEAD_FACTOR) /
> - mode_clock;
> -
> - /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> - max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
> - mode_hdisplay;
> -
> - /*
> - * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
> - * check, output bpp from small joiner RAM check)
> - */
> - bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> -
> - /* Error out if the max bpp is less than smallest allowed valid bpp */
> - if (bits_per_pixel < valid_dsc_bpp[0]) {
> - DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
> - return 0;
> - }
> -
> - /* Find the nearest match in the array of known BPPs from VESA */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> - if (bits_per_pixel < valid_dsc_bpp[i + 1])
> - break;
> - }
> - bits_per_pixel = valid_dsc_bpp[i];
> -
> - /*
> - * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> - * fractional part is 0
> - */
> - return bits_per_pixel << 4;
> -}
> -
> -u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> - int mode_clock,
> - int mode_hdisplay)
> -{
> - u8 min_slice_count, i;
> - int max_slice_width;
> -
> - if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_0);
> - else
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_1);
> -
> - max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> - if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> - DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> - max_slice_width);
> - return 0;
> - }
> - /* Also take into account max slice width */
> - min_slice_count = min_t(u8, min_slice_count,
> - DIV_ROUND_UP(mode_hdisplay,
> - max_slice_width));
> -
> - /* Find the closest match to the valid slice count values */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> - if (valid_dsc_slicecount[i] >
> - drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> - false))
> - break;
> - if (min_slice_count <= valid_dsc_slicecount[i])
> - return valid_dsc_slicecount[i];
> - }
> -
> - DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> - return 0;
> -}
> -
> static void
> intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index e01d1f89409d..a194b5b6da05 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
> bool
> intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
> -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);
>
> bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
> @@ -119,4 +115,6 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> return ~((1 << lane_count) - 1) & 0xf;
> }
>
> +u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
> +
> #endif /* __INTEL_DP_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index eeeb3f933aa4..cf4d851a5139 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -81,7 +81,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> adjusted_mode->crtc_clock,
> crtc_state->port_clock,
> &crtc_state->dp_m_n,
> - constant_n);
> + constant_n, crtc_state->fec_enable);
> crtc_state->dp_m_n.tu = slots;
>
> return 0;
> --
> 2.20.1
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3.
2019-09-23 12:52 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3 Maarten Lankhorst
2019-09-23 13:03 ` Ville Syrjälä
2019-09-23 14:22 ` [Intel-gfx] [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3 kbuild test robot
@ 2019-09-23 15:53 ` Manasi Navare
2 siblings, 0 replies; 11+ messages in thread
From: Manasi Navare @ 2019-09-23 15:53 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, Ville Syrjälä, stable
On Mon, Sep 23, 2019 at 02:52:52PM +0200, Maarten Lankhorst wrote:
> There was a integer wraparound when mode_clock became too high,
> and we didn't correct for the FEC overhead factor when dividing,
> with the calculations breaking at HBR3.
>
> As a result our calculated bpp was way too high, and the link width
> limitation never came into effect.
>
> Print out the resulting bpp calcululations as a sanity check, just
> in case we ever have to debug it later on again.
>
> We also used the wrong factor for FEC. While bspec mentions 2.4%,
> all the calculations use 1/0.972261, and the same ratio should be
> applied to data M/N as well, so use it there when FEC is enabled.
>
> Make sure we don't break hw readout, and read out FEC enable state
> and correct the DDI clock readout for the new values.
>
> This fixes the FIFO underrun we are seeing with FEC enabled.
>
> Changes since v2:
> - Handle fec_enable in intel_link_compute_m_n, so only data M/N is adjusted. (Ville)
> - Fix initial hardware readout for FEC. (Ville)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
> Cc: <stable@vger.kernel.org> # v5.0+
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
> Thanks, that fixed the FIFO underrun, making the disablement patch obsolete.
> Bigjoiner is now completely working as intended. :)
Thats great, so the FEC adjustments in the compute m_n was the thing that was mising
because of which we were seeing the underruns?
I think Anusha missed it when she implemented FEC along with DSC because back then
we only had eDP DSC panels where we do no enable FEC and didnt have any FEC DSC DP panels.
Thanks for tha catch and the fix.
Regards
Manasi
>
> drivers/gpu/drm/i915/display/intel_ddi.c | 21 ++
> drivers/gpu/drm/i915/display/intel_display.c | 13 +-
> drivers/gpu/drm/i915/display/intel_display.h | 2 +-
> drivers/gpu/drm/i915/display/intel_dp.c | 191 ++++++++++---------
> drivers/gpu/drm/i915/display/intel_dp.h | 7 +-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> 6 files changed, 137 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 0c0da9f6c2e8..3e77b30d91d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1479,6 +1479,10 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
> if (pipe_config->pixel_multiplier)
> dotclock /= pipe_config->pixel_multiplier;
>
> + /* fec adds overhead to the data M/N values, correct for it */
> + if (pipe_config->fec_enable)
> + dotclock = intel_dp_fec_to_mode_clock(dotclock);
> +
> pipe_config->base.adjusted_mode.crtc_clock = dotclock;
> }
>
> @@ -4045,6 +4049,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> pipe_config->lane_count =
> ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> intel_dp_get_m_n(intel_crtc, pipe_config);
> +
> + if (INTEL_GEN(dev_priv) >= 11) {
> + i915_reg_t dp_tp_ctl;
> +
> + if (IS_GEN(dev_priv, 11))
> + dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
> + else
> + dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
> +
> + pipe_config->fec_enable =
> + I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
> +
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
> + encoder->base.base.id, encoder->base.name,
> + pipe_config->fec_enable);
> + }
> +
> break;
> case TRANS_DDI_MODE_SELECT_DP_MST:
> pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5ecf54270181..31698a57773f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7291,7 +7291,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> pipe_config->fdi_lanes = lane;
>
> intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> - link_bw, &pipe_config->fdi_m_n, false);
> + link_bw, &pipe_config->fdi_m_n, false, false);
>
> ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> if (ret == -EDEADLK)
> @@ -7538,11 +7538,15 @@ void
> intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> - bool constant_n)
> + bool constant_n, bool fec_enable)
> {
> - m_n->tu = 64;
> + u32 data_clock = bits_per_pixel * pixel_clock;
> +
> + if (fec_enable)
> + data_clock = intel_dp_mode_to_fec_clock(data_clock);
>
> - compute_m_n(bits_per_pixel * pixel_clock,
> + m_n->tu = 64;
> + compute_m_n(data_clock,
> link_clock * nlanes * 8,
> &m_n->gmch_m, &m_n->gmch_n,
> constant_n);
> @@ -12832,6 +12836,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> PIPE_CONF_CHECK_BOOL(has_infoframe);
> + PIPE_CONF_CHECK_BOOL(fec_enable);
>
> PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 5cea6f8e107a..4b9e18e5a263 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -443,7 +443,7 @@ enum phy_fia {
> void intel_link_compute_m_n(u16 bpp, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> - bool constant_n);
> + bool constant_n, bool fec_enable);
> bool is_ccs_modifier(u64 modifier);
> void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
> u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 829559f97440..2d3f4183f99d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -76,8 +76,8 @@
> #define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
> #define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
>
> -/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
> -#define DP_DSC_FEC_OVERHEAD_FACTOR 976
> +/* DP DSC FEC Overhead factor = 1/(0.972261) */
> +#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
>
> /* Compliance test status bits */
> #define INTEL_DP_RESOLUTION_SHIFT_MASK 0
> @@ -492,6 +492,104 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> return 0;
> }
>
> +u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
> +{
> + return div_u64(mul_u32_u32(mode_clock, 1000000U),
> + DP_DSC_FEC_OVERHEAD_FACTOR);
> +}
> +
> +u32 intel_dp_fec_to_mode_clock(u32 fec_clock)
> +{
> + return div_u64(mul_u32_u32(fec_clock,
> + DP_DSC_FEC_OVERHEAD_FACTOR),
> + 1000000U);
> +}
> +
> +static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
> + u32 mode_clock, u32 mode_hdisplay)
> +{
> + u32 bits_per_pixel, max_bpp_small_joiner_ram;
> + int i;
> +
> + /*
> + * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> + * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
> + * for SST -> TimeSlotsPerMTP is 1,
> + * for MST -> TimeSlotsPerMTP has to be calculated
> + */
> + bits_per_pixel = (link_clock * lane_count * 8) /
> + intel_dp_mode_to_fec_clock(mode_clock);
> + DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
> +
> + /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> + max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
> + DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
> +
> + /*
> + * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
> + * check, output bpp from small joiner RAM check)
> + */
> + bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> +
> + /* Error out if the max bpp is less than smallest allowed valid bpp */
> + if (bits_per_pixel < valid_dsc_bpp[0]) {
> + DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
> + bits_per_pixel, valid_dsc_bpp[0]);
> + return 0;
> + }
> +
> + /* Find the nearest match in the array of known BPPs from VESA */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> + if (bits_per_pixel < valid_dsc_bpp[i + 1])
> + break;
> + }
> + bits_per_pixel = valid_dsc_bpp[i];
> +
> + /*
> + * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> + * fractional part is 0
> + */
> + return bits_per_pixel << 4;
> +}
> +
> +static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> + int mode_clock, int mode_hdisplay)
> +{
> + u8 min_slice_count, i;
> + int max_slice_width;
> +
> + if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_0);
> + else
> + min_slice_count = DIV_ROUND_UP(mode_clock,
> + DP_DSC_MAX_ENC_THROUGHPUT_1);
> +
> + max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> + if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> + DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> + max_slice_width);
> + return 0;
> + }
> + /* Also take into account max slice width */
> + min_slice_count = min_t(u8, min_slice_count,
> + DIV_ROUND_UP(mode_hdisplay,
> + max_slice_width));
> +
> + /* Find the closest match to the valid slice count values */
> + for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> + if (valid_dsc_slicecount[i] >
> + drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> + false))
> + break;
> + if (min_slice_count <= valid_dsc_slicecount[i])
> + return valid_dsc_slicecount[i];
> + }
> +
> + DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> + return 0;
> +}
> +
> static enum drm_mode_status
> intel_dp_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> @@ -2259,7 +2357,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> adjusted_mode->crtc_clock,
> pipe_config->port_clock,
> &pipe_config->dp_m_n,
> - constant_n);
> + constant_n, pipe_config->fec_enable);
>
> if (intel_connector->panel.downclock_mode != NULL &&
> dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> @@ -2269,7 +2367,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> intel_connector->panel.downclock_mode->clock,
> pipe_config->port_clock,
> &pipe_config->dp_m2_n2,
> - constant_n);
> + constant_n, pipe_config->fec_enable);
> }
>
> if (!HAS_DDI(dev_priv))
> @@ -4373,91 +4471,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
> DP_DPRX_ESI_LEN;
> }
>
> -u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
> - int mode_clock, int mode_hdisplay)
> -{
> - u16 bits_per_pixel, max_bpp_small_joiner_ram;
> - int i;
> -
> - /*
> - * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> - * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
> - * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
> - * for MST -> TimeSlotsPerMTP has to be calculated
> - */
> - bits_per_pixel = (link_clock * lane_count * 8 *
> - DP_DSC_FEC_OVERHEAD_FACTOR) /
> - mode_clock;
> -
> - /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> - max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
> - mode_hdisplay;
> -
> - /*
> - * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
> - * check, output bpp from small joiner RAM check)
> - */
> - bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> -
> - /* Error out if the max bpp is less than smallest allowed valid bpp */
> - if (bits_per_pixel < valid_dsc_bpp[0]) {
> - DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
> - return 0;
> - }
> -
> - /* Find the nearest match in the array of known BPPs from VESA */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> - if (bits_per_pixel < valid_dsc_bpp[i + 1])
> - break;
> - }
> - bits_per_pixel = valid_dsc_bpp[i];
> -
> - /*
> - * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> - * fractional part is 0
> - */
> - return bits_per_pixel << 4;
> -}
> -
> -u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> - int mode_clock,
> - int mode_hdisplay)
> -{
> - u8 min_slice_count, i;
> - int max_slice_width;
> -
> - if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_0);
> - else
> - min_slice_count = DIV_ROUND_UP(mode_clock,
> - DP_DSC_MAX_ENC_THROUGHPUT_1);
> -
> - max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> - if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> - DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> - max_slice_width);
> - return 0;
> - }
> - /* Also take into account max slice width */
> - min_slice_count = min_t(u8, min_slice_count,
> - DIV_ROUND_UP(mode_hdisplay,
> - max_slice_width));
> -
> - /* Find the closest match to the valid slice count values */
> - for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> - if (valid_dsc_slicecount[i] >
> - drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> - false))
> - break;
> - if (min_slice_count <= valid_dsc_slicecount[i])
> - return valid_dsc_slicecount[i];
> - }
> -
> - DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> - return 0;
> -}
> -
> static void
> intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index e01d1f89409d..2147d3c14870 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
> bool
> intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
> -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);
>
> bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
> @@ -119,4 +115,7 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> return ~((1 << lane_count) - 1) & 0xf;
> }
>
> +u32 intel_dp_fec_to_mode_clock(u32 fec_clock);
> +u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
> +
> #endif /* __INTEL_DP_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index eeeb3f933aa4..cf4d851a5139 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -81,7 +81,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> adjusted_mode->crtc_clock,
> crtc_state->port_clock,
> &crtc_state->dp_m_n,
> - constant_n);
> + constant_n, crtc_state->fec_enable);
> crtc_state->dp_m_n.tu = slots;
>
> return 0;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4.
2019-09-23 14:57 ` Ville Syrjälä
@ 2019-09-23 15:56 ` Manasi Navare
2019-09-23 15:56 ` [Intel-gfx] " Ville Syrjälä
1 sibling, 0 replies; 11+ messages in thread
From: Manasi Navare @ 2019-09-23 15:56 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Maarten Lankhorst, intel-gfx, stable
On Mon, Sep 23, 2019 at 05:57:57PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 23, 2019 at 04:49:47PM +0200, Maarten Lankhorst wrote:
> > There was a integer wraparound when mode_clock became too high,
> > and we didn't correct for the FEC overhead factor when dividing,
> > with the calculations breaking at HBR3.
> >
> > As a result our calculated bpp was way too high, and the link width
> > limitation never came into effect.
> >
> > Print out the resulting bpp calcululations as a sanity check, just
> > in case we ever have to debug it later on again.
> >
> > We also used the wrong factor for FEC. While bspec mentions 2.4%,
> > all the calculations use 1/0.972261, and the same ratio should be
> > applied to data M/N as well, so use it there when FEC is enabled.
> >
> > Make sure we don't break hw readout, and read out FEC enable state
> > and correct the DDI clock readout for the new values.
> >
> > This fixes the FIFO underrun we are seeing with FEC enabled.
> >
> > Changes since v2:
> > - Handle fec_enable in intel_link_compute_m_n, so only data M/N is adjusted. (Ville)
> > - Fix initial hardware readout for FEC. (Ville)
> > Changes since v3:
> > - Remove bogus fec_to_mode_clock. (Ville)
> >
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
> > Cc: <stable@vger.kernel.org> # v5.0+
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 17 ++
> > drivers/gpu/drm/i915/display/intel_display.c | 13 +-
> > drivers/gpu/drm/i915/display/intel_display.h | 2 +-
> > drivers/gpu/drm/i915/display/intel_dp.c | 184 ++++++++++---------
> > drivers/gpu/drm/i915/display/intel_dp.h | 6 +-
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> > 6 files changed, 125 insertions(+), 99 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 0c0da9f6c2e8..1cb297abd111 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4045,6 +4045,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> > pipe_config->lane_count =
> > ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> > intel_dp_get_m_n(intel_crtc, pipe_config);
> > +
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > + i915_reg_t dp_tp_ctl;
> > +
> > + if (IS_GEN(dev_priv, 11))
> > + dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
> > + else
> > + dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
> > +
> > + pipe_config->fec_enable =
> > + I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
>
> Can you split the fec_enable readout/state check into a separate
> patch?
>
> I wonder how the lack of this stuff was missed when FEC was adeed...
>
We never had any FEC DP panel when this initial FEC enabling stuff was added,
Anusha did test it with the FPGA based emulator that had FEC but we still missed the m_n
correction as there were no underruns reported that time.
Thanks Ville and Maarten for catching this and fixing it.
Manasi
> > +
> > + DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
> > + encoder->base.base.id, encoder->base.name,
> > + pipe_config->fec_enable);
>
> I'd just include it as part of the normal state dump.
>
> > + }
> > +
> > break;
> > case TRANS_DDI_MODE_SELECT_DP_MST:
> > pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 5ecf54270181..31698a57773f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7291,7 +7291,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> > pipe_config->fdi_lanes = lane;
> >
> > intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> > - link_bw, &pipe_config->fdi_m_n, false);
> > + link_bw, &pipe_config->fdi_m_n, false, false);
> >
> > ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> > if (ret == -EDEADLK)
> > @@ -7538,11 +7538,15 @@ void
> > intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
> > int pixel_clock, int link_clock,
> > struct intel_link_m_n *m_n,
> > - bool constant_n)
> > + bool constant_n, bool fec_enable)
> > {
> > - m_n->tu = 64;
> > + u32 data_clock = bits_per_pixel * pixel_clock;
> > +
> > + if (fec_enable)
> > + data_clock = intel_dp_mode_to_fec_clock(data_clock);
> >
> > - compute_m_n(bits_per_pixel * pixel_clock,
> > + m_n->tu = 64;
> > + compute_m_n(data_clock,
> > link_clock * nlanes * 8,
> > &m_n->gmch_m, &m_n->gmch_n,
> > constant_n);
> > @@ -12832,6 +12836,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> > PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> > PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> > PIPE_CONF_CHECK_BOOL(has_infoframe);
> > + PIPE_CONF_CHECK_BOOL(fec_enable);
> >
> > PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index 5cea6f8e107a..4b9e18e5a263 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -443,7 +443,7 @@ enum phy_fia {
> > void intel_link_compute_m_n(u16 bpp, int nlanes,
> > int pixel_clock, int link_clock,
> > struct intel_link_m_n *m_n,
> > - bool constant_n);
> > + bool constant_n, bool fec_enable);
> > bool is_ccs_modifier(u64 modifier);
> > void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
> > u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 829559f97440..2b1e71f992b0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -76,8 +76,8 @@
> > #define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
> > #define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
> >
> > -/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
> > -#define DP_DSC_FEC_OVERHEAD_FACTOR 976
> > +/* DP DSC FEC Overhead factor = 1/(0.972261) */
> > +#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
> >
> > /* Compliance test status bits */
> > #define INTEL_DP_RESOLUTION_SHIFT_MASK 0
> > @@ -492,6 +492,97 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > return 0;
> > }
> >
> > +u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
> > +{
> > + return div_u64(mul_u32_u32(mode_clock, 1000000U),
> > + DP_DSC_FEC_OVERHEAD_FACTOR);
> > +}
> > +
> > +static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
> > + u32 mode_clock, u32 mode_hdisplay)
> > +{
> > + u32 bits_per_pixel, max_bpp_small_joiner_ram;
> > + int i;
> > +
> > + /*
> > + * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> > + * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
> > + * for SST -> TimeSlotsPerMTP is 1,
> > + * for MST -> TimeSlotsPerMTP has to be calculated
> > + */
> > + bits_per_pixel = (link_clock * lane_count * 8) /
> > + intel_dp_mode_to_fec_clock(mode_clock);
> > + DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
> > +
> > + /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> > + max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
> > + DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
> > +
> > + /*
> > + * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
> > + * check, output bpp from small joiner RAM check)
> > + */
> > + bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> > +
> > + /* Error out if the max bpp is less than smallest allowed valid bpp */
> > + if (bits_per_pixel < valid_dsc_bpp[0]) {
> > + DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
> > + bits_per_pixel, valid_dsc_bpp[0]);
> > + return 0;
> > + }
> > +
> > + /* Find the nearest match in the array of known BPPs from VESA */
> > + for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> > + if (bits_per_pixel < valid_dsc_bpp[i + 1])
> > + break;
> > + }
> > + bits_per_pixel = valid_dsc_bpp[i];
> > +
> > + /*
> > + * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> > + * fractional part is 0
> > + */
> > + return bits_per_pixel << 4;
> > +}
> > +
> > +static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> > + int mode_clock, int mode_hdisplay)
> > +{
> > + u8 min_slice_count, i;
> > + int max_slice_width;
> > +
> > + if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> > + min_slice_count = DIV_ROUND_UP(mode_clock,
> > + DP_DSC_MAX_ENC_THROUGHPUT_0);
> > + else
> > + min_slice_count = DIV_ROUND_UP(mode_clock,
> > + DP_DSC_MAX_ENC_THROUGHPUT_1);
> > +
> > + max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> > + if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> > + DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> > + max_slice_width);
> > + return 0;
> > + }
> > + /* Also take into account max slice width */
> > + min_slice_count = min_t(u8, min_slice_count,
> > + DIV_ROUND_UP(mode_hdisplay,
> > + max_slice_width));
> > +
> > + /* Find the closest match to the valid slice count values */
> > + for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> > + if (valid_dsc_slicecount[i] >
> > + drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> > + false))
> > + break;
> > + if (min_slice_count <= valid_dsc_slicecount[i])
> > + return valid_dsc_slicecount[i];
> > + }
> > +
> > + DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> > + return 0;
> > +}
> > +
> > static enum drm_mode_status
> > intel_dp_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *mode)
> > @@ -2259,7 +2350,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > adjusted_mode->crtc_clock,
> > pipe_config->port_clock,
> > &pipe_config->dp_m_n,
> > - constant_n);
> > + constant_n, pipe_config->fec_enable);
> >
> > if (intel_connector->panel.downclock_mode != NULL &&
> > dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> > @@ -2269,7 +2360,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > intel_connector->panel.downclock_mode->clock,
> > pipe_config->port_clock,
> > &pipe_config->dp_m2_n2,
> > - constant_n);
> > + constant_n, pipe_config->fec_enable);
> > }
> >
> > if (!HAS_DDI(dev_priv))
> > @@ -4373,91 +4464,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
> > DP_DPRX_ESI_LEN;
> > }
> >
> > -u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
> > - int mode_clock, int mode_hdisplay)
> > -{
> > - u16 bits_per_pixel, max_bpp_small_joiner_ram;
> > - int i;
> > -
> > - /*
> > - * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> > - * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
> > - * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
> > - * for MST -> TimeSlotsPerMTP has to be calculated
> > - */
> > - bits_per_pixel = (link_clock * lane_count * 8 *
> > - DP_DSC_FEC_OVERHEAD_FACTOR) /
> > - mode_clock;
> > -
> > - /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> > - max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
> > - mode_hdisplay;
> > -
> > - /*
> > - * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
> > - * check, output bpp from small joiner RAM check)
> > - */
> > - bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> > -
> > - /* Error out if the max bpp is less than smallest allowed valid bpp */
> > - if (bits_per_pixel < valid_dsc_bpp[0]) {
> > - DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
> > - return 0;
> > - }
> > -
> > - /* Find the nearest match in the array of known BPPs from VESA */
> > - for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> > - if (bits_per_pixel < valid_dsc_bpp[i + 1])
> > - break;
> > - }
> > - bits_per_pixel = valid_dsc_bpp[i];
> > -
> > - /*
> > - * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> > - * fractional part is 0
> > - */
> > - return bits_per_pixel << 4;
> > -}
> > -
> > -u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> > - int mode_clock,
> > - int mode_hdisplay)
> > -{
> > - u8 min_slice_count, i;
> > - int max_slice_width;
> > -
> > - if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> > - min_slice_count = DIV_ROUND_UP(mode_clock,
> > - DP_DSC_MAX_ENC_THROUGHPUT_0);
> > - else
> > - min_slice_count = DIV_ROUND_UP(mode_clock,
> > - DP_DSC_MAX_ENC_THROUGHPUT_1);
> > -
> > - max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> > - if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> > - DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> > - max_slice_width);
> > - return 0;
> > - }
> > - /* Also take into account max slice width */
> > - min_slice_count = min_t(u8, min_slice_count,
> > - DIV_ROUND_UP(mode_hdisplay,
> > - max_slice_width));
> > -
> > - /* Find the closest match to the valid slice count values */
> > - for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> > - if (valid_dsc_slicecount[i] >
> > - drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> > - false))
> > - break;
> > - if (min_slice_count <= valid_dsc_slicecount[i])
> > - return valid_dsc_slicecount[i];
> > - }
> > -
> > - DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> > - return 0;
> > -}
> > -
> > static void
> > intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> > const struct intel_crtc_state *crtc_state)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index e01d1f89409d..a194b5b6da05 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> > bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
> > bool
> > intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
> > -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);
> >
> > bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
> > @@ -119,4 +115,6 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> > return ~((1 << lane_count) - 1) & 0xf;
> > }
> >
> > +u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
> > +
> > #endif /* __INTEL_DP_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index eeeb3f933aa4..cf4d851a5139 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -81,7 +81,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> > adjusted_mode->crtc_clock,
> > crtc_state->port_clock,
> > &crtc_state->dp_m_n,
> > - constant_n);
> > + constant_n, crtc_state->fec_enable);
> > crtc_state->dp_m_n.tu = slots;
> >
> > return 0;
> > --
> > 2.20.1
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4.
2019-09-23 14:57 ` Ville Syrjälä
2019-09-23 15:56 ` Manasi Navare
@ 2019-09-23 15:56 ` Ville Syrjälä
1 sibling, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2019-09-23 15:56 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, stable
On Mon, Sep 23, 2019 at 05:57:57PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 23, 2019 at 04:49:47PM +0200, Maarten Lankhorst wrote:
> > There was a integer wraparound when mode_clock became too high,
> > and we didn't correct for the FEC overhead factor when dividing,
> > with the calculations breaking at HBR3.
> >
> > As a result our calculated bpp was way too high, and the link width
> > limitation never came into effect.
> >
> > Print out the resulting bpp calcululations as a sanity check, just
> > in case we ever have to debug it later on again.
> >
> > We also used the wrong factor for FEC. While bspec mentions 2.4%,
> > all the calculations use 1/0.972261, and the same ratio should be
> > applied to data M/N as well, so use it there when FEC is enabled.
> >
> > Make sure we don't break hw readout, and read out FEC enable state
> > and correct the DDI clock readout for the new values.
> >
> > This fixes the FIFO underrun we are seeing with FEC enabled.
> >
> > Changes since v2:
> > - Handle fec_enable in intel_link_compute_m_n, so only data M/N is adjusted. (Ville)
> > - Fix initial hardware readout for FEC. (Ville)
> > Changes since v3:
> > - Remove bogus fec_to_mode_clock. (Ville)
> >
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC")
> > Cc: <stable@vger.kernel.org> # v5.0+
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 17 ++
> > drivers/gpu/drm/i915/display/intel_display.c | 13 +-
> > drivers/gpu/drm/i915/display/intel_display.h | 2 +-
> > drivers/gpu/drm/i915/display/intel_dp.c | 184 ++++++++++---------
> > drivers/gpu/drm/i915/display/intel_dp.h | 6 +-
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> > 6 files changed, 125 insertions(+), 99 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 0c0da9f6c2e8..1cb297abd111 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4045,6 +4045,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> > pipe_config->lane_count =
> > ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> > intel_dp_get_m_n(intel_crtc, pipe_config);
> > +
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > + i915_reg_t dp_tp_ctl;
> > +
> > + if (IS_GEN(dev_priv, 11))
> > + dp_tp_ctl = DP_TP_CTL(pipe_config->cpu_transcoder);
Oh, and pre-tgl DP_TP_CTL is per port.
> > + else
> > + dp_tp_ctl = TGL_DP_TP_CTL(pipe_config->cpu_transcoder);
> > +
> > + pipe_config->fec_enable =
> > + I915_READ(dp_tp_ctl) & DP_TP_CTL_FEC_ENABLE;
>
> Can you split the fec_enable readout/state check into a separate
> patch?
>
> I wonder how the lack of this stuff was missed when FEC was adeed...
>
> > +
> > + DRM_DEBUG_KMS("[ENCODER:%d:%s] Fec status: %u\n",
> > + encoder->base.base.id, encoder->base.name,
> > + pipe_config->fec_enable);
>
> I'd just include it as part of the normal state dump.
>
> > + }
> > +
> > break;
> > case TRANS_DDI_MODE_SELECT_DP_MST:
> > pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 5ecf54270181..31698a57773f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7291,7 +7291,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> > pipe_config->fdi_lanes = lane;
> >
> > intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> > - link_bw, &pipe_config->fdi_m_n, false);
> > + link_bw, &pipe_config->fdi_m_n, false, false);
> >
> > ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> > if (ret == -EDEADLK)
> > @@ -7538,11 +7538,15 @@ void
> > intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
> > int pixel_clock, int link_clock,
> > struct intel_link_m_n *m_n,
> > - bool constant_n)
> > + bool constant_n, bool fec_enable)
> > {
> > - m_n->tu = 64;
> > + u32 data_clock = bits_per_pixel * pixel_clock;
> > +
> > + if (fec_enable)
> > + data_clock = intel_dp_mode_to_fec_clock(data_clock);
> >
> > - compute_m_n(bits_per_pixel * pixel_clock,
> > + m_n->tu = 64;
> > + compute_m_n(data_clock,
> > link_clock * nlanes * 8,
> > &m_n->gmch_m, &m_n->gmch_n,
> > constant_n);
> > @@ -12832,6 +12836,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> > PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> > PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> > PIPE_CONF_CHECK_BOOL(has_infoframe);
> > + PIPE_CONF_CHECK_BOOL(fec_enable);
> >
> > PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index 5cea6f8e107a..4b9e18e5a263 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -443,7 +443,7 @@ enum phy_fia {
> > void intel_link_compute_m_n(u16 bpp, int nlanes,
> > int pixel_clock, int link_clock,
> > struct intel_link_m_n *m_n,
> > - bool constant_n);
> > + bool constant_n, bool fec_enable);
> > bool is_ccs_modifier(u64 modifier);
> > void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
> > u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 829559f97440..2b1e71f992b0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -76,8 +76,8 @@
> > #define DP_DSC_MAX_ENC_THROUGHPUT_0 340000
> > #define DP_DSC_MAX_ENC_THROUGHPUT_1 400000
> >
> > -/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
> > -#define DP_DSC_FEC_OVERHEAD_FACTOR 976
> > +/* DP DSC FEC Overhead factor = 1/(0.972261) */
> > +#define DP_DSC_FEC_OVERHEAD_FACTOR 972261
> >
> > /* Compliance test status bits */
> > #define INTEL_DP_RESOLUTION_SHIFT_MASK 0
> > @@ -492,6 +492,97 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > return 0;
> > }
> >
> > +u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
> > +{
> > + return div_u64(mul_u32_u32(mode_clock, 1000000U),
> > + DP_DSC_FEC_OVERHEAD_FACTOR);
> > +}
> > +
> > +static u16 intel_dp_dsc_get_output_bpp(u32 link_clock, u32 lane_count,
> > + u32 mode_clock, u32 mode_hdisplay)
> > +{
> > + u32 bits_per_pixel, max_bpp_small_joiner_ram;
> > + int i;
> > +
> > + /*
> > + * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> > + * (LinkSymbolClock)* 8 * (TimeSlotsPerMTP)
> > + * for SST -> TimeSlotsPerMTP is 1,
> > + * for MST -> TimeSlotsPerMTP has to be calculated
> > + */
> > + bits_per_pixel = (link_clock * lane_count * 8) /
> > + intel_dp_mode_to_fec_clock(mode_clock);
> > + DRM_DEBUG_KMS("Max link bpp: %u\n", bits_per_pixel);
> > +
> > + /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> > + max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER / mode_hdisplay;
> > + DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
> > +
> > + /*
> > + * Greatest allowed DSC BPP = MIN (output BPP from available Link BW
> > + * check, output bpp from small joiner RAM check)
> > + */
> > + bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> > +
> > + /* Error out if the max bpp is less than smallest allowed valid bpp */
> > + if (bits_per_pixel < valid_dsc_bpp[0]) {
> > + DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
> > + bits_per_pixel, valid_dsc_bpp[0]);
> > + return 0;
> > + }
> > +
> > + /* Find the nearest match in the array of known BPPs from VESA */
> > + for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> > + if (bits_per_pixel < valid_dsc_bpp[i + 1])
> > + break;
> > + }
> > + bits_per_pixel = valid_dsc_bpp[i];
> > +
> > + /*
> > + * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> > + * fractional part is 0
> > + */
> > + return bits_per_pixel << 4;
> > +}
> > +
> > +static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> > + int mode_clock, int mode_hdisplay)
> > +{
> > + u8 min_slice_count, i;
> > + int max_slice_width;
> > +
> > + if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> > + min_slice_count = DIV_ROUND_UP(mode_clock,
> > + DP_DSC_MAX_ENC_THROUGHPUT_0);
> > + else
> > + min_slice_count = DIV_ROUND_UP(mode_clock,
> > + DP_DSC_MAX_ENC_THROUGHPUT_1);
> > +
> > + max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> > + if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> > + DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> > + max_slice_width);
> > + return 0;
> > + }
> > + /* Also take into account max slice width */
> > + min_slice_count = min_t(u8, min_slice_count,
> > + DIV_ROUND_UP(mode_hdisplay,
> > + max_slice_width));
> > +
> > + /* Find the closest match to the valid slice count values */
> > + for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> > + if (valid_dsc_slicecount[i] >
> > + drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> > + false))
> > + break;
> > + if (min_slice_count <= valid_dsc_slicecount[i])
> > + return valid_dsc_slicecount[i];
> > + }
> > +
> > + DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> > + return 0;
> > +}
> > +
> > static enum drm_mode_status
> > intel_dp_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *mode)
> > @@ -2259,7 +2350,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > adjusted_mode->crtc_clock,
> > pipe_config->port_clock,
> > &pipe_config->dp_m_n,
> > - constant_n);
> > + constant_n, pipe_config->fec_enable);
> >
> > if (intel_connector->panel.downclock_mode != NULL &&
> > dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> > @@ -2269,7 +2360,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > intel_connector->panel.downclock_mode->clock,
> > pipe_config->port_clock,
> > &pipe_config->dp_m2_n2,
> > - constant_n);
> > + constant_n, pipe_config->fec_enable);
> > }
> >
> > if (!HAS_DDI(dev_priv))
> > @@ -4373,91 +4464,6 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
> > DP_DPRX_ESI_LEN;
> > }
> >
> > -u16 intel_dp_dsc_get_output_bpp(int link_clock, u8 lane_count,
> > - int mode_clock, int mode_hdisplay)
> > -{
> > - u16 bits_per_pixel, max_bpp_small_joiner_ram;
> > - int i;
> > -
> > - /*
> > - * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
> > - * (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
> > - * FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
> > - * for MST -> TimeSlotsPerMTP has to be calculated
> > - */
> > - bits_per_pixel = (link_clock * lane_count * 8 *
> > - DP_DSC_FEC_OVERHEAD_FACTOR) /
> > - mode_clock;
> > -
> > - /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> > - max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
> > - mode_hdisplay;
> > -
> > - /*
> > - * Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
> > - * check, output bpp from small joiner RAM check)
> > - */
> > - bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> > -
> > - /* Error out if the max bpp is less than smallest allowed valid bpp */
> > - if (bits_per_pixel < valid_dsc_bpp[0]) {
> > - DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
> > - return 0;
> > - }
> > -
> > - /* Find the nearest match in the array of known BPPs from VESA */
> > - for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> > - if (bits_per_pixel < valid_dsc_bpp[i + 1])
> > - break;
> > - }
> > - bits_per_pixel = valid_dsc_bpp[i];
> > -
> > - /*
> > - * Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
> > - * fractional part is 0
> > - */
> > - return bits_per_pixel << 4;
> > -}
> > -
> > -u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> > - int mode_clock,
> > - int mode_hdisplay)
> > -{
> > - u8 min_slice_count, i;
> > - int max_slice_width;
> > -
> > - if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> > - min_slice_count = DIV_ROUND_UP(mode_clock,
> > - DP_DSC_MAX_ENC_THROUGHPUT_0);
> > - else
> > - min_slice_count = DIV_ROUND_UP(mode_clock,
> > - DP_DSC_MAX_ENC_THROUGHPUT_1);
> > -
> > - max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> > - if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> > - DRM_DEBUG_KMS("Unsupported slice width %d by DP DSC Sink device\n",
> > - max_slice_width);
> > - return 0;
> > - }
> > - /* Also take into account max slice width */
> > - min_slice_count = min_t(u8, min_slice_count,
> > - DIV_ROUND_UP(mode_hdisplay,
> > - max_slice_width));
> > -
> > - /* Find the closest match to the valid slice count values */
> > - for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> > - if (valid_dsc_slicecount[i] >
> > - drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> > - false))
> > - break;
> > - if (min_slice_count <= valid_dsc_slicecount[i])
> > - return valid_dsc_slicecount[i];
> > - }
> > -
> > - DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> > - return 0;
> > -}
> > -
> > static void
> > intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> > const struct intel_crtc_state *crtc_state)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index e01d1f89409d..a194b5b6da05 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -103,10 +103,6 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> > bool intel_dp_source_supports_hbr3(struct intel_dp *intel_dp);
> > bool
> > intel_dp_get_link_status(struct intel_dp *intel_dp, u8 *link_status);
> > -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);
> >
> > bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp);
> > @@ -119,4 +115,6 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> > return ~((1 << lane_count) - 1) & 0xf;
> > }
> >
> > +u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
> > +
> > #endif /* __INTEL_DP_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index eeeb3f933aa4..cf4d851a5139 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -81,7 +81,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> > adjusted_mode->crtc_clock,
> > crtc_state->port_clock,
> > &crtc_state->dp_m_n,
> > - constant_n);
> > + constant_n, crtc_state->fec_enable);
> > crtc_state->dp_m_n.tu = slots;
> >
> > return 0;
> > --
> > 2.20.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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-09-23 15:56 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-20 11:42 [PATCH 01/23] drm/i915/dp: Fix dsc bpp calculations, v2 Maarten Lankhorst
2019-09-20 16:38 ` [Intel-gfx] " Ville Syrjälä
2019-09-23 12:52 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3 Maarten Lankhorst
2019-09-23 13:03 ` Ville Syrjälä
2019-09-23 14:49 ` [PATCH] drm/i915/dp: Fix dsc bpp calculations, v4 Maarten Lankhorst
2019-09-23 14:50 ` Maarten Lankhorst
2019-09-23 14:57 ` Ville Syrjälä
2019-09-23 15:56 ` Manasi Navare
2019-09-23 15:56 ` [Intel-gfx] " Ville Syrjälä
2019-09-23 14:22 ` [Intel-gfx] [PATCH] drm/i915/dp: Fix dsc bpp calculations, v3 kbuild test robot
2019-09-23 15:53 ` Manasi Navare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).