Op 25-09-2019 om 12:26 schreef Ville Syrjälä: > On Wed, Sep 25, 2019 at 10:21:09AM +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. >> >> 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) >> Changes since v4: >> - Use the correct register for icl. (Ville) >> - Split hw readout to a separate patch. >> >> Signed-off-by: Maarten Lankhorst <[email protected]> >> Fixes: d9218c8f6cf4 ("drm/i915/dp: Add helpers for Compressed BPP and Slice >> Count for DSC") >> Cc: <[email protected]> # v5.0+ >> Cc: Manasi Navare <[email protected]> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 12 +- >> 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 +- >> 5 files changed, 107 insertions(+), 99 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c >> b/drivers/gpu/drm/i915/display/intel_display.c >> index 5ecf54270181..c4c9286be987 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); >> 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); > Hmm. Aren't we adding the FEC overhead twice now when DSC is also > enabled?
No? This is calculating the maximum bpp that can be used for DSC. "For cases where FEC is enabled, pixel clock is replaced by pixel clock/0.972261 in the above calculations." This bpp check here is to ensure that data M <= N. ~Maarten _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
