<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">On 9/12/2018 6:25 AM, Manasi Navare wrote: </div> <blockquote type="cite" cite="mid:20180912005607.29522-6-manasi.d.navare@intel.com"> <pre wrap="">This patch adds helpers for calculating the maximum compressed BPP supported with small joiner. This also adds a helper for calculating the slice count in case of small joiner. These are inside intel_dp since they take into account hardware limitations. v6: * Take mode_clock and mode_hdisplay as input arguments so that this can be called in intel_dp_mode_valid (Manasi) v5: * Get the max slice width from DPCD * Check against Min_Slice_width of 2560 (Anusha) v4: * #defines for PPR in slice count helper (Gaurav) v3: * Simply logic for bpp (DK) * Limit the valid slice count by max supported by Sink (Manasi) v2: * Change the small joiner RAM buffer constant as bspec changed (Manasi) * rename it as SMALL_JOINER since we are not enabling big joiner yet (Anusha) Cc: Gaurav K Singh <a class="moz-txt-link-rfc2396E" href="mailto:gaurav.k.singh@intel.com"><gaurav.k.singh@intel.com></a> Cc: Jani Nikula <a class="moz-txt-link-rfc2396E" href="mailto:jani.nikula@linux.intel.com"><jani.nikula@linux.intel.com></a> Cc: Ville Syrjala <a class="moz-txt-link-rfc2396E" href="mailto:ville.syrjala@linux.intel.com"><ville.syrjala@linux.intel.com></a> Cc: Anusha Srivatsa <a class="moz-txt-link-rfc2396E" href="mailto:anusha.srivatsa@intel.com"><anusha.srivatsa@intel.com></a> Cc: Dhinakaran Pandiyan <a class="moz-txt-link-rfc2396E" href="mailto:dhinakaran.pandiyan@intel.com"><dhinakaran.pandiyan@intel.com></a> Signed-off-by: Manasi Navare <a class="moz-txt-link-rfc2396E" href="mailto:manasi.d.navare@intel.com"><manasi.d.navare@intel.com></a> Reviewed-by: Anusha Srivatsa <a class="moz-txt-link-rfc2396E" href="mailto:anusha.srivatsa@intel.com"><anusha.srivatsa@intel.com></a> --- drivers/gpu/drm/i915/intel_dp.c | 104 +++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 4 ++ 2 files changed, 108 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1dfcceb55182..719c2e426c28 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -45,6 +45,17 @@ #define DP_DPRX_ESI_LEN 14 +/* DP DSC small joiner has 2 FIFOs each of 640 x 6 bytes */ +#define DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER 61440 + +/* DP DSC throughput values used for slice count calculations KPixels/s */ +#define DP_DSC_PEAK_PIXEL_RATE 2720000 +#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 */</pre> </blockquote> This comment is misleading to get the value of 976 <blockquote type="cite" cite="mid:20180912005607.29522-6-manasi.d.navare@intel.com"> <pre wrap="">+#define DP_DSC_FEC_OVERHEAD_FACTOR 976 + /* Compliance test status bits */ #define INTEL_DP_RESOLUTION_SHIFT_MASK 0 #define INTEL_DP_RESOLUTION_PREFERRED (1 << INTEL_DP_RESOLUTION_SHIFT_MASK) @@ -93,6 +104,14 @@ static const struct dp_link_dpll chv_dpll[] = { { .p1 = 4, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c00000 } }, }; +/* Constants for DP DSC configurations */ +static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15}; + +/* With Single pipe configuration, HW is capable of supporting maximum + * of 4 slices per line. + */ +static const u8 valid_dsc_slicecount[] = {1, 2, 4}; + /** * intel_dp_is_edp - is the given port attached to an eDP panel (either CPU or PCH) * @intel_dp: DP struct @@ -4080,6 +4099,91 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) DP_DPRX_ESI_LEN; } +uint16_t intel_dp_dsc_get_output_bpp(int link_clock, uint8_t lane_count, + int mode_clock, int mode_hdisplay)</pre> </blockquote> Can we use u16 here. I know that for functions defined earlier in this file, we have used uint16_t. But since we are adding new functions, we can follow u16 or u8 accordingly. <blockquote type="cite" cite="mid:20180912005607.29522-6-manasi.d.navare@intel.com"> <pre wrap=""> +{ + 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; +} + +uint8_t intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, + int mode_clock, + int mode_hdisplay)</pre> </blockquote> Same comment as above. <blockquote type="cite" cite="mid:20180912005607.29522-6-manasi.d.navare@intel.com"> <pre wrap=""> +{ + 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(uint8_t, 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 uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp) { int status = 0; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 0828fcb7af90..c56e3092be25 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1744,6 +1744,10 @@ 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, uint8_t link_status[DP_LINK_STATUS_SIZE]); +uint16_t intel_dp_dsc_get_output_bpp(int link_clock, uint8_t lane_count, + int mode_clock, int mode_hdisplay); +uint8_t intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, int mode_clock, + int mode_hdisplay); static inline unsigned int intel_dp_unused_lane_mask(int lane_count) { </pre> </blockquote> Otherwise, the patch looks good to me. Reviewed-by: Gaurav K Singh <a class="moz-txt-link-rfc2396E" href="mailto:gaurav.k.singh@intel.com"><gaurav.k.singh@intel.com></a> </body> </html>