From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Singh, Gaurav K" Subject: Re: [PATCH v4 05/25] drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC Date: Fri, 14 Sep 2018 11:27:56 +0530 Message-ID: References: <20180912005607.29522-1-manasi.d.navare@intel.com> <20180912005607.29522-6-manasi.d.navare@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0270569096==" Return-path: In-Reply-To: <20180912005607.29522-6-manasi.d.navare@intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Manasi Navare , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: Anusha Srivatsa , Dhinakaran Pandiyan List-Id: dri-devel@lists.freedesktop.org This is a multi-part message in MIME format. --===============0270569096== Content-Type: multipart/alternative; boundary="------------D20558D439C121FBC666C63E" Content-Language: en-US This is a multi-part message in MIME format. --------------D20558D439C121FBC666C63E Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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 > Cc: Jani Nikula > Cc: Ville Syrjala > Cc: Anusha Srivatsa > Cc: Dhinakaran Pandiyan > Signed-off-by: Manasi Navare > Reviewed-by: Anusha Srivatsa > --- > 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) 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. > +{ > + 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) > { Otherwise, the patch looks good to me. Reviewed-by: Gaurav K Singh --------------D20558D439C121FBC666C63E Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit



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)

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.

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

Otherwise, the patch looks good to me.

Reviewed-by: Gaurav K Singh <gaurav.k.singh@intel.com>
--------------D20558D439C121FBC666C63E-- --===============0270569096== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0270569096==--