Re: [PATCH v4 05/25] drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC

On 9/12/2018 6:25 AM, Manasi Navare wrote:

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 gaurav.k.singh@intel.com Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Ville Syrjala ville.syrjala@linux.intel.com Cc: Anusha Srivatsa anusha.srivatsa@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: Manasi Navare manasi.d.navare@intel.com Reviewed-by: Anusha Srivatsa anusha.srivatsa@intel.com

---

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 */

This comment is misleading to get the value of 976

+#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)
  

For all the new functions being added, can we use u16 instead of uint16_t. I know for the fact that in this file, uint16_t has been used in the functions which were defined earlier. But since we are adding new functions, we can use u16 or u8 accordingly.

+{

• 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)
  

Same comment as above.

+{

• 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) {

Other than the previous comments, this patch looks good to me.

Reviewed-by: Gaurav K Singh gaurav.k.singh@intel.com