[PATCH v2 0/3] Make DRM DSC helpers more generally usable

[PATCH v2 0/3] Make DRM DSC helpers more generally usable

[David Francis]

drm_dsc could use some work so that drm drivers other than
i915 can make use of it their own DSC implementations

Move rc compute, a function that forms part of the DSC spec,
into drm. Update it to DSC 1.2. Also split the PPS packing and
SDP header init functions, to allow for drivers with
their own SDP struct headers

Re-sending due to Mail Delivery System errors

v2:
Rebase onto drm-next
Refactor drm_dsc_dp_pps_header_init
Clean up documentation on new drm function

David Francis (3):
  drm/i915: Move dsc rate params compute into drm
  drm/dsc: Add native 420 and 422 support to compute_rc_params
  drm/dsc: Split DSC PPS and SDP header initialisations

 drivers/gpu/drm/drm_dsc.c         | 269 +++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_vdsc.c | 133 +--------------
 include/drm/drm_dsc.h             |   9 +-
 3 files changed, 219 insertions(+), 192 deletions(-)

-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/3] drm/i915: Move dsc rate params compute into drm

[David Francis]

The function intel_compute_rc_parameters is part of the dsc spec
and is not driver-specific. Other drm drivers might like to use
it.  The function is not changed; just moved and renamed.

Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: David Francis <David.Francis@amd.com>
---
 drivers/gpu/drm/drm_dsc.c         | 135 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_vdsc.c | 125 +--------------------------
 include/drm/drm_dsc.h             |   1 +
 3 files changed, 137 insertions(+), 124 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index bce99f95c1a3..b7f1903508a4 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/byteorder/generic.h>
+#include <drm/drm_print.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_dsc.h>
 
@@ -244,3 +245,137 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
 	/* PPS 94 - 127 are O */
 }
 EXPORT_SYMBOL(drm_dsc_pps_infoframe_pack);
+
+/**
+ * drm_dsc_compute_rc_parameters() - Write rate control
+ * parameters to the dsc configuration defined in
+ * &struct drm_dsc_config in accordance with the DSC 1.1
+ * specification. Some configuration fields must be present
+ * beforehand.
+ *
+ * @vdsc_cfg:
+ * DSC Configuration data partially filled by driver
+ */
+int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
+{
+	unsigned long groups_per_line = 0;
+	unsigned long groups_total = 0;
+	unsigned long num_extra_mux_bits = 0;
+	unsigned long slice_bits = 0;
+	unsigned long hrd_delay = 0;
+	unsigned long final_scale = 0;
+	unsigned long rbs_min = 0;
+
+	/* Number of groups used to code each line of a slice */
+	groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width,
+				       DSC_RC_PIXELS_PER_GROUP);
+
+	/* chunksize in Bytes */
+	vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *
+						  vdsc_cfg->bits_per_pixel,
+						  (8 * 16));
+
+	if (vdsc_cfg->convert_rgb)
+		num_extra_mux_bits = 3 * (vdsc_cfg->mux_word_size +
+					  (4 * vdsc_cfg->bits_per_component + 4)
+					  - 2);
+	else
+		num_extra_mux_bits = 3 * vdsc_cfg->mux_word_size +
+			(4 * vdsc_cfg->bits_per_component + 4) +
+			2 * (4 * vdsc_cfg->bits_per_component) - 2;
+	/* Number of bits in one Slice */
+	slice_bits = 8 * vdsc_cfg->slice_chunk_size * vdsc_cfg->slice_height;
+
+	while ((num_extra_mux_bits > 0) &&
+	       ((slice_bits - num_extra_mux_bits) % vdsc_cfg->mux_word_size))
+		num_extra_mux_bits--;
+
+	if (groups_per_line < vdsc_cfg->initial_scale_value - 8)
+		vdsc_cfg->initial_scale_value = groups_per_line + 8;
+
+	/* scale_decrement_interval calculation according to DSC spec 1.11 */
+	if (vdsc_cfg->initial_scale_value > 8)
+		vdsc_cfg->scale_decrement_interval = groups_per_line /
+			(vdsc_cfg->initial_scale_value - 8);
+	else
+		vdsc_cfg->scale_decrement_interval = DSC_SCALE_DECREMENT_INTERVAL_MAX;
+
+	vdsc_cfg->final_offset = vdsc_cfg->rc_model_size -
+		(vdsc_cfg->initial_xmit_delay *
+		 vdsc_cfg->bits_per_pixel + 8) / 16 + num_extra_mux_bits;
+
+	if (vdsc_cfg->final_offset >= vdsc_cfg->rc_model_size) {
+		DRM_DEBUG_KMS("FinalOfs < RcModelSze for this InitialXmitDelay\n");
+		return -ERANGE;
+	}
+
+	final_scale = (vdsc_cfg->rc_model_size * 8) /
+		(vdsc_cfg->rc_model_size - vdsc_cfg->final_offset);
+	if (vdsc_cfg->slice_height > 1)
+		/*
+		 * NflBpgOffset is 16 bit value with 11 fractional bits
+		 * hence we multiply by 2^11 for preserving the
+		 * fractional part
+		 */
+		vdsc_cfg->nfl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->first_line_bpg_offset << 11),
+							(vdsc_cfg->slice_height - 1));
+	else
+		vdsc_cfg->nfl_bpg_offset = 0;
+
+	/* 2^16 - 1 */
+	if (vdsc_cfg->nfl_bpg_offset > 65535) {
+		DRM_DEBUG_KMS("NflBpgOffset is too large for this slice height\n");
+		return -ERANGE;
+	}
+
+	/* Number of groups used to code the entire slice */
+	groups_total = groups_per_line * vdsc_cfg->slice_height;
+
+	/* slice_bpg_offset is 16 bit value with 11 fractional bits */
+	vdsc_cfg->slice_bpg_offset = DIV_ROUND_UP(((vdsc_cfg->rc_model_size -
+						    vdsc_cfg->initial_offset +
+						    num_extra_mux_bits) << 11),
+						  groups_total);
+
+	if (final_scale > 9) {
+		/*
+		 * ScaleIncrementInterval =
+		 * finaloffset/((NflBpgOffset + SliceBpgOffset)*8(finalscale - 1.125))
+		 * as (NflBpgOffset + SliceBpgOffset) has 11 bit fractional value,
+		 * we need divide by 2^11 from pstDscCfg values
+		 */
+		vdsc_cfg->scale_increment_interval =
+				(vdsc_cfg->final_offset * (1 << 11)) /
+				((vdsc_cfg->nfl_bpg_offset +
+				vdsc_cfg->slice_bpg_offset) *
+				(final_scale - 9));
+	} else {
+		/*
+		 * If finalScaleValue is less than or equal to 9, a value of 0 should
+		 * be used to disable the scale increment at the end of the slice
+		 */
+		vdsc_cfg->scale_increment_interval = 0;
+	}
+
+	if (vdsc_cfg->scale_increment_interval > 65535) {
+		DRM_DEBUG_KMS("ScaleIncrementInterval is large for slice height\n");
+		return -ERANGE;
+	}
+
+	/*
+	 * DSC spec mentions that bits_per_pixel specifies the target
+	 * bits/pixel (bpp) rate that is used by the encoder,
+	 * in steps of 1/16 of a bit per pixel
+	 */
+	rbs_min = vdsc_cfg->rc_model_size - vdsc_cfg->initial_offset +
+		DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay *
+			     vdsc_cfg->bits_per_pixel, 16) +
+		groups_per_line * vdsc_cfg->first_line_bpg_offset;
+
+	hrd_delay = DIV_ROUND_UP((rbs_min * 16), vdsc_cfg->bits_per_pixel);
+	vdsc_cfg->rc_bits = (hrd_delay * vdsc_cfg->bits_per_pixel) / 16;
+	vdsc_cfg->initial_dec_delay = hrd_delay - vdsc_cfg->initial_xmit_delay;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dsc_compute_rc_parameters);
diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
index 23abf03736e7..2d059ebc9bd0 100644
--- a/drivers/gpu/drm/i915/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/intel_vdsc.c
@@ -317,129 +317,6 @@ static int get_column_index_for_rc_params(u8 bits_per_component)
 	}
 }
 
-static int intel_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
-{
-	unsigned long groups_per_line = 0;
-	unsigned long groups_total = 0;
-	unsigned long num_extra_mux_bits = 0;
-	unsigned long slice_bits = 0;
-	unsigned long hrd_delay = 0;
-	unsigned long final_scale = 0;
-	unsigned long rbs_min = 0;
-
-	/* Number of groups used to code each line of a slice */
-	groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width,
-				       DSC_RC_PIXELS_PER_GROUP);
-
-	/* chunksize in Bytes */
-	vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *
-						  vdsc_cfg->bits_per_pixel,
-						  (8 * 16));
-
-	if (vdsc_cfg->convert_rgb)
-		num_extra_mux_bits = 3 * (vdsc_cfg->mux_word_size +
-					  (4 * vdsc_cfg->bits_per_component + 4)
-					  - 2);
-	else
-		num_extra_mux_bits = 3 * vdsc_cfg->mux_word_size +
-			(4 * vdsc_cfg->bits_per_component + 4) +
-			2 * (4 * vdsc_cfg->bits_per_component) - 2;
-	/* Number of bits in one Slice */
-	slice_bits = 8 * vdsc_cfg->slice_chunk_size * vdsc_cfg->slice_height;
-
-	while ((num_extra_mux_bits > 0) &&
-	       ((slice_bits - num_extra_mux_bits) % vdsc_cfg->mux_word_size))
-		num_extra_mux_bits--;
-
-	if (groups_per_line < vdsc_cfg->initial_scale_value - 8)
-		vdsc_cfg->initial_scale_value = groups_per_line + 8;
-
-	/* scale_decrement_interval calculation according to DSC spec 1.11 */
-	if (vdsc_cfg->initial_scale_value > 8)
-		vdsc_cfg->scale_decrement_interval = groups_per_line /
-			(vdsc_cfg->initial_scale_value - 8);
-	else
-		vdsc_cfg->scale_decrement_interval = DSC_SCALE_DECREMENT_INTERVAL_MAX;
-
-	vdsc_cfg->final_offset = vdsc_cfg->rc_model_size -
-		(vdsc_cfg->initial_xmit_delay *
-		 vdsc_cfg->bits_per_pixel + 8) / 16 + num_extra_mux_bits;
-
-	if (vdsc_cfg->final_offset >= vdsc_cfg->rc_model_size) {
-		DRM_DEBUG_KMS("FinalOfs < RcModelSze for this InitialXmitDelay\n");
-		return -ERANGE;
-	}
-
-	final_scale = (vdsc_cfg->rc_model_size * 8) /
-		(vdsc_cfg->rc_model_size - vdsc_cfg->final_offset);
-	if (vdsc_cfg->slice_height > 1)
-		/*
-		 * NflBpgOffset is 16 bit value with 11 fractional bits
-		 * hence we multiply by 2^11 for preserving the
-		 * fractional part
-		 */
-		vdsc_cfg->nfl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->first_line_bpg_offset << 11),
-							(vdsc_cfg->slice_height - 1));
-	else
-		vdsc_cfg->nfl_bpg_offset = 0;
-
-	/* 2^16 - 1 */
-	if (vdsc_cfg->nfl_bpg_offset > 65535) {
-		DRM_DEBUG_KMS("NflBpgOffset is too large for this slice height\n");
-		return -ERANGE;
-	}
-
-	/* Number of groups used to code the entire slice */
-	groups_total = groups_per_line * vdsc_cfg->slice_height;
-
-	/* slice_bpg_offset is 16 bit value with 11 fractional bits */
-	vdsc_cfg->slice_bpg_offset = DIV_ROUND_UP(((vdsc_cfg->rc_model_size -
-						    vdsc_cfg->initial_offset +
-						    num_extra_mux_bits) << 11),
-						  groups_total);
-
-	if (final_scale > 9) {
-		/*
-		 * ScaleIncrementInterval =
-		 * finaloffset/((NflBpgOffset + SliceBpgOffset)*8(finalscale - 1.125))
-		 * as (NflBpgOffset + SliceBpgOffset) has 11 bit fractional value,
-		 * we need divide by 2^11 from pstDscCfg values
-		 */
-		vdsc_cfg->scale_increment_interval =
-				(vdsc_cfg->final_offset * (1 << 11)) /
-				((vdsc_cfg->nfl_bpg_offset +
-				vdsc_cfg->slice_bpg_offset) *
-				(final_scale - 9));
-	} else {
-		/*
-		 * If finalScaleValue is less than or equal to 9, a value of 0 should
-		 * be used to disable the scale increment at the end of the slice
-		 */
-		vdsc_cfg->scale_increment_interval = 0;
-	}
-
-	if (vdsc_cfg->scale_increment_interval > 65535) {
-		DRM_DEBUG_KMS("ScaleIncrementInterval is large for slice height\n");
-		return -ERANGE;
-	}
-
-	/*
-	 * DSC spec mentions that bits_per_pixel specifies the target
-	 * bits/pixel (bpp) rate that is used by the encoder,
-	 * in steps of 1/16 of a bit per pixel
-	 */
-	rbs_min = vdsc_cfg->rc_model_size - vdsc_cfg->initial_offset +
-		DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay *
-			     vdsc_cfg->bits_per_pixel, 16) +
-		groups_per_line * vdsc_cfg->first_line_bpg_offset;
-
-	hrd_delay = DIV_ROUND_UP((rbs_min * 16), vdsc_cfg->bits_per_pixel);
-	vdsc_cfg->rc_bits = (hrd_delay * vdsc_cfg->bits_per_pixel) / 16;
-	vdsc_cfg->initial_dec_delay = hrd_delay - vdsc_cfg->initial_xmit_delay;
-
-	return 0;
-}
-
 int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
 				struct intel_crtc_state *pipe_config)
 {
@@ -574,7 +451,7 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
 	vdsc_cfg->initial_scale_value = (vdsc_cfg->rc_model_size << 3) /
 		(vdsc_cfg->rc_model_size - vdsc_cfg->initial_offset);
 
-	return intel_compute_rc_parameters(vdsc_cfg);
+	return drm_dsc_compute_rc_parameters(vdsc_cfg);
 }
 
 enum intel_display_power_domain
diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
index 9c26f083c70f..5a98b8dfdf43 100644
--- a/include/drm/drm_dsc.h
+++ b/include/drm/drm_dsc.h
@@ -604,5 +604,6 @@ struct drm_dsc_pps_infoframe {
 void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp);
 void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
 				const struct drm_dsc_config *dsc_cfg);
+int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
 
 #endif /* _DRM_DSC_H_ */
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/3] drm/dsc: Add native 420 and 422 support to compute_rc_params

[David Francis]

Native 420 and 422 transfer modes are new in DSC1.2

In these modes, each two pixels of a slice are treated as one
pixel, so the slice width is half as large (round down) for
the purposes of calucating the groups per line and chunk size
in bytes

In native 422 mode, each pixel has four components, so the
mux component of a group is larger by one additional mux word
and one additional component

Now that there is native 422 support, the configuration option
previously called enable422 is renamed to simple_422 to avoid
confusion

Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: David Francis <David.Francis@amd.com>
---
 drivers/gpu/drm/drm_dsc.c         | 33 ++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_vdsc.c |  4 ++--
 include/drm/drm_dsc.h             |  4 ++--
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index b7f1903508a4..d77570bf6ac4 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -95,7 +95,7 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
 		((dsc_cfg->bits_per_pixel & DSC_PPS_BPP_HIGH_MASK) >>
 		 DSC_PPS_MSB_SHIFT) |
 		dsc_cfg->vbr_enable << DSC_PPS_VBR_EN_SHIFT |
-		dsc_cfg->enable422 << DSC_PPS_SIMPLE422_SHIFT |
+		dsc_cfg->simple_422 << DSC_PPS_SIMPLE422_SHIFT |
 		dsc_cfg->convert_rgb << DSC_PPS_CONVERT_RGB_SHIFT |
 		dsc_cfg->block_pred_enable << DSC_PPS_BLOCK_PRED_EN_SHIFT;
 
@@ -249,7 +249,7 @@ EXPORT_SYMBOL(drm_dsc_pps_infoframe_pack);
 /**
  * drm_dsc_compute_rc_parameters() - Write rate control
  * parameters to the dsc configuration defined in
- * &struct drm_dsc_config in accordance with the DSC 1.1
+ * &struct drm_dsc_config in accordance with the DSC 1.2
  * specification. Some configuration fields must be present
  * beforehand.
  *
@@ -266,19 +266,34 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
 	unsigned long final_scale = 0;
 	unsigned long rbs_min = 0;
 
-	/* Number of groups used to code each line of a slice */
-	groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width,
-				       DSC_RC_PIXELS_PER_GROUP);
+	if (vdsc_cfg->native_420 || vdsc_cfg->native_422) {
+		/* Number of groups used to code each line of a slice */
+		groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width / 2,
+					       DSC_RC_PIXELS_PER_GROUP);
 
-	/* chunksize in Bytes */
-	vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *
-						  vdsc_cfg->bits_per_pixel,
-						  (8 * 16));
+		/* chunksize in Bytes */
+		vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width / 2 *
+							  vdsc_cfg->bits_per_pixel,
+							  (8 * 16));
+	} else {
+		/* Number of groups used to code each line of a slice */
+		groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width,
+					       DSC_RC_PIXELS_PER_GROUP);
+
+		/* chunksize in Bytes */
+		vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *
+							  vdsc_cfg->bits_per_pixel,
+							  (8 * 16));
+	}
 
 	if (vdsc_cfg->convert_rgb)
 		num_extra_mux_bits = 3 * (vdsc_cfg->mux_word_size +
 					  (4 * vdsc_cfg->bits_per_component + 4)
 					  - 2);
+	else if (vdsc_cfg->native_422)
+		num_extra_mux_bits = 4 * vdsc_cfg->mux_word_size +
+			(4 * vdsc_cfg->bits_per_component + 4) +
+			3 * (4 * vdsc_cfg->bits_per_component) - 2;
 	else
 		num_extra_mux_bits = 3 * vdsc_cfg->mux_word_size +
 			(4 * vdsc_cfg->bits_per_component + 4) +
diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
index 2d059ebc9bd0..8c8d96157333 100644
--- a/drivers/gpu/drm/i915/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/intel_vdsc.c
@@ -368,7 +368,7 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
 			DSC_1_1_MAX_LINEBUF_DEPTH_BITS : line_buf_depth;
 
 	/* Gen 11 does not support YCbCr */
-	vdsc_cfg->enable422 = false;
+	vdsc_cfg->simple_422 = false;
 	/* Gen 11 does not support VBR */
 	vdsc_cfg->vbr_enable = false;
 	vdsc_cfg->block_pred_enable =
@@ -495,7 +495,7 @@ static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder,
 		pps_val |= DSC_BLOCK_PREDICTION;
 	if (vdsc_cfg->convert_rgb)
 		pps_val |= DSC_COLOR_SPACE_CONVERSION;
-	if (vdsc_cfg->enable422)
+	if (vdsc_cfg->simple_422)
 		pps_val |= DSC_422_ENABLE;
 	if (vdsc_cfg->vbr_enable)
 		pps_val |= DSC_VBR_ENABLE;
diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
index 5a98b8dfdf43..f26a89e1b68a 100644
--- a/include/drm/drm_dsc.h
+++ b/include/drm/drm_dsc.h
@@ -101,9 +101,9 @@ struct drm_dsc_config {
 	 */
 	u16 slice_height;
 	/**
-	 * @enable422: True for 4_2_2 sampling, false for 4_4_4 sampling
+	 * @simple_422: True if simple 4_2_2 mode is enabled else False
 	 */
-	bool enable422;
+	bool simple_422;
 	/**
 	 * @pic_width: Width of the input display frame in pixels
 	 */
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm/dsc: Add native 420 and 422 support to compute_rc_params

[Jani Nikula]

On Thu, 21 Feb 2019, David Francis <David.Francis@amd.com> wrote:
> Native 420 and 422 transfer modes are new in DSC1.2
>
> In these modes, each two pixels of a slice are treated as one
> pixel, so the slice width is half as large (round down) for
> the purposes of calucating the groups per line and chunk size
> in bytes
>
> In native 422 mode, each pixel has four components, so the
> mux component of a group is larger by one additional mux word
> and one additional component
>
> Now that there is native 422 support, the configuration option
> previously called enable422 is renamed to simple_422 to avoid
> confusion
>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

This was really for patch 1/3 where it actually matters.

BR,
Jani.

> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: David Francis <David.Francis@amd.com>
> ---
>  drivers/gpu/drm/drm_dsc.c         | 33 ++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_vdsc.c |  4 ++--
>  include/drm/drm_dsc.h             |  4 ++--
>  3 files changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index b7f1903508a4..d77570bf6ac4 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -95,7 +95,7 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
>  		((dsc_cfg->bits_per_pixel & DSC_PPS_BPP_HIGH_MASK) >>
>  		 DSC_PPS_MSB_SHIFT) |
>  		dsc_cfg->vbr_enable << DSC_PPS_VBR_EN_SHIFT |
> -		dsc_cfg->enable422 << DSC_PPS_SIMPLE422_SHIFT |
> +		dsc_cfg->simple_422 << DSC_PPS_SIMPLE422_SHIFT |
>  		dsc_cfg->convert_rgb << DSC_PPS_CONVERT_RGB_SHIFT |
>  		dsc_cfg->block_pred_enable << DSC_PPS_BLOCK_PRED_EN_SHIFT;
>  
> @@ -249,7 +249,7 @@ EXPORT_SYMBOL(drm_dsc_pps_infoframe_pack);
>  /**
>   * drm_dsc_compute_rc_parameters() - Write rate control
>   * parameters to the dsc configuration defined in
> - * &struct drm_dsc_config in accordance with the DSC 1.1
> + * &struct drm_dsc_config in accordance with the DSC 1.2
>   * specification. Some configuration fields must be present
>   * beforehand.
>   *
> @@ -266,19 +266,34 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
>  	unsigned long final_scale = 0;
>  	unsigned long rbs_min = 0;
>  
> -	/* Number of groups used to code each line of a slice */
> -	groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width,
> -				       DSC_RC_PIXELS_PER_GROUP);
> +	if (vdsc_cfg->native_420 || vdsc_cfg->native_422) {
> +		/* Number of groups used to code each line of a slice */
> +		groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width / 2,
> +					       DSC_RC_PIXELS_PER_GROUP);
>  
> -	/* chunksize in Bytes */
> -	vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *
> -						  vdsc_cfg->bits_per_pixel,
> -						  (8 * 16));
> +		/* chunksize in Bytes */
> +		vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width / 2 *
> +							  vdsc_cfg->bits_per_pixel,
> +							  (8 * 16));
> +	} else {
> +		/* Number of groups used to code each line of a slice */
> +		groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width,
> +					       DSC_RC_PIXELS_PER_GROUP);
> +
> +		/* chunksize in Bytes */
> +		vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *
> +							  vdsc_cfg->bits_per_pixel,
> +							  (8 * 16));
> +	}
>  
>  	if (vdsc_cfg->convert_rgb)
>  		num_extra_mux_bits = 3 * (vdsc_cfg->mux_word_size +
>  					  (4 * vdsc_cfg->bits_per_component + 4)
>  					  - 2);
> +	else if (vdsc_cfg->native_422)
> +		num_extra_mux_bits = 4 * vdsc_cfg->mux_word_size +
> +			(4 * vdsc_cfg->bits_per_component + 4) +
> +			3 * (4 * vdsc_cfg->bits_per_component) - 2;
>  	else
>  		num_extra_mux_bits = 3 * vdsc_cfg->mux_word_size +
>  			(4 * vdsc_cfg->bits_per_component + 4) +
> diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
> index 2d059ebc9bd0..8c8d96157333 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -368,7 +368,7 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
>  			DSC_1_1_MAX_LINEBUF_DEPTH_BITS : line_buf_depth;
>  
>  	/* Gen 11 does not support YCbCr */
> -	vdsc_cfg->enable422 = false;
> +	vdsc_cfg->simple_422 = false;
>  	/* Gen 11 does not support VBR */
>  	vdsc_cfg->vbr_enable = false;
>  	vdsc_cfg->block_pred_enable =
> @@ -495,7 +495,7 @@ static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder,
>  		pps_val |= DSC_BLOCK_PREDICTION;
>  	if (vdsc_cfg->convert_rgb)
>  		pps_val |= DSC_COLOR_SPACE_CONVERSION;
> -	if (vdsc_cfg->enable422)
> +	if (vdsc_cfg->simple_422)
>  		pps_val |= DSC_422_ENABLE;
>  	if (vdsc_cfg->vbr_enable)
>  		pps_val |= DSC_VBR_ENABLE;
> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> index 5a98b8dfdf43..f26a89e1b68a 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -101,9 +101,9 @@ struct drm_dsc_config {
>  	 */
>  	u16 slice_height;
>  	/**
> -	 * @enable422: True for 4_2_2 sampling, false for 4_4_4 sampling
> +	 * @simple_422: True if simple 4_2_2 mode is enabled else False
>  	 */
> -	bool enable422;
> +	bool simple_422;
>  	/**
>  	 * @pic_width: Width of the input display frame in pixels
>  	 */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 3/3] drm/dsc: Split DSC PPS and SDP header initialisations

[David Francis]

The DP 1.4 spec defines the SDP header and SDP contents for
a Picture Parameter Set (PPS) that must be sent in advance
of DSC transmission to define the encoding characteristics.

This was done in one struct, drm_dsc_pps_infoframe, which
conatined the SDP header and PPS.  Because the PPS is
a property of DSC over any connector, not just DP, and because
drm drivers may have their own SDP structs they wish to use,
make the functions that initialise SDP and PPS headers take
the components they operate on, not drm_dsc_pps_infoframe,

Signed-off-by: David Francis <David.Francis@amd.com>
---
 drivers/gpu/drm/drm_dsc.c         | 117 +++++++++++++++---------------
 drivers/gpu/drm/i915/intel_vdsc.c |   4 +-
 include/drm/drm_dsc.h             |   4 +-
 3 files changed, 62 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index d77570bf6ac4..77f4e5ae4197 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -32,66 +32,65 @@
 /**
  * drm_dsc_dp_pps_header_init() - Initializes the PPS Header
  * for DisplayPort as per the DP 1.4 spec.
- * @pps_sdp: Secondary data packet for DSC Picture Parameter Set
- *           as defined in &struct drm_dsc_pps_infoframe
+ * @pps_header: Secondary data packet header for DSC Picture
+ *              Parameter Set as defined in &struct dp_sdp_header
  *
  * DP 1.4 spec defines the secondary data packet for sending the
  * picture parameter infoframes from the source to the sink.
- * This function populates the pps header defined in
- * &struct drm_dsc_pps_infoframe as per the header bytes defined
- * in &struct dp_sdp_header.
+ * This function populates the SDP header defined in
+ * &struct dp_sdp_header.
  */
-void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
+void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header)
 {
-	memset(&pps_sdp->pps_header, 0, sizeof(pps_sdp->pps_header));
+	memset(pps_header, 0, sizeof(*pps_header));
 
-	pps_sdp->pps_header.HB1 = DP_SDP_PPS;
-	pps_sdp->pps_header.HB2 = DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1;
+	pps_header->HB1 = DP_SDP_PPS;
+	pps_header->HB2 = DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1;
 }
 EXPORT_SYMBOL(drm_dsc_dp_pps_header_init);
 
 /**
- * drm_dsc_pps_infoframe_pack() - Populates the DSC PPS infoframe
+ * drm_dsc_pps_payload_pack() - Populates the DSC PPS
  *
- * @pps_sdp:
- * Secondary data packet for DSC Picture Parameter Set. This is defined
- * by &struct drm_dsc_pps_infoframe
+ * @pps_payload:
+ * Bitwise struct for DSC Picture Parameter Set. This is defined
+ * by &struct drm_dsc_picture_parameter_set
  * @dsc_cfg:
  * DSC Configuration data filled by driver as defined by
  * &struct drm_dsc_config
  *
- * DSC source device sends a secondary data packet filled with all the
- * picture parameter set (PPS) information required by the sink to decode
- * the compressed frame. Driver populates the dsC PPS infoframe using the DSC
- * configuration parameters in the order expected by the DSC Display Sink
- * device. For the DSC, the sink device expects the PPS payload in the big
- * endian format for the fields that span more than 1 byte.
+ * DSC source device sends a picture parameter set (PPS) containing the
+ * information required by the sink to decode the compressed frame. Driver
+ * populates the DSC PPS struct using the DSC configuration parameters in
+ * the order expected by the DSC Display Sink device. For the DSC, the sink
+ * device expects the PPS payload in big endian format for fields
+ * that span more than 1 byte.
  */
-void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
+void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload,
 				const struct drm_dsc_config *dsc_cfg)
 {
 	int i;
 
 	/* Protect against someone accidently changing struct size */
-	BUILD_BUG_ON(sizeof(pps_sdp->pps_payload) !=
+	BUILD_BUG_ON(sizeof(*pps_payload) !=
 		     DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1 + 1);
 
-	memset(&pps_sdp->pps_payload, 0, sizeof(pps_sdp->pps_payload));
+	memset(pps_payload, 0, sizeof(*pps_payload));
 
 	/* PPS 0 */
-	pps_sdp->pps_payload.dsc_version =
+	pps_payload->dsc_version =
 		dsc_cfg->dsc_version_minor |
 		dsc_cfg->dsc_version_major << DSC_PPS_VERSION_MAJOR_SHIFT;
 
 	/* PPS 1, 2 is 0 */
 
 	/* PPS 3 */
-	pps_sdp->pps_payload.pps_3 =
+	pps_payload->pps_3 =
 		dsc_cfg->line_buf_depth |
 		dsc_cfg->bits_per_component << DSC_PPS_BPC_SHIFT;
 
 	/* PPS 4 */
-	pps_sdp->pps_payload.pps_4 =
+	pps_payload->pps_4 =
 		((dsc_cfg->bits_per_pixel & DSC_PPS_BPP_HIGH_MASK) >>
 		 DSC_PPS_MSB_SHIFT) |
 		dsc_cfg->vbr_enable << DSC_PPS_VBR_EN_SHIFT |
@@ -100,7 +99,7 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
 		dsc_cfg->block_pred_enable << DSC_PPS_BLOCK_PRED_EN_SHIFT;
 
 	/* PPS 5 */
-	pps_sdp->pps_payload.bits_per_pixel_low =
+	pps_payload->bits_per_pixel_low =
 		(dsc_cfg->bits_per_pixel & DSC_PPS_LSB_MASK);
 
 	/*
@@ -111,103 +110,103 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
 	 */
 
 	/* PPS 6, 7 */
-	pps_sdp->pps_payload.pic_height = cpu_to_be16(dsc_cfg->pic_height);
+	pps_payload->pic_height = cpu_to_be16(dsc_cfg->pic_height);
 
 	/* PPS 8, 9 */
-	pps_sdp->pps_payload.pic_width = cpu_to_be16(dsc_cfg->pic_width);
+	pps_payload->pic_width = cpu_to_be16(dsc_cfg->pic_width);
 
 	/* PPS 10, 11 */
-	pps_sdp->pps_payload.slice_height = cpu_to_be16(dsc_cfg->slice_height);
+	pps_payload->slice_height = cpu_to_be16(dsc_cfg->slice_height);
 
 	/* PPS 12, 13 */
-	pps_sdp->pps_payload.slice_width = cpu_to_be16(dsc_cfg->slice_width);
+	pps_payload->slice_width = cpu_to_be16(dsc_cfg->slice_width);
 
 	/* PPS 14, 15 */
-	pps_sdp->pps_payload.chunk_size = cpu_to_be16(dsc_cfg->slice_chunk_size);
+	pps_payload->chunk_size = cpu_to_be16(dsc_cfg->slice_chunk_size);
 
 	/* PPS 16 */
-	pps_sdp->pps_payload.initial_xmit_delay_high =
+	pps_payload->initial_xmit_delay_high =
 		((dsc_cfg->initial_xmit_delay &
 		  DSC_PPS_INIT_XMIT_DELAY_HIGH_MASK) >>
 		 DSC_PPS_MSB_SHIFT);
 
 	/* PPS 17 */
-	pps_sdp->pps_payload.initial_xmit_delay_low =
+	pps_payload->initial_xmit_delay_low =
 		(dsc_cfg->initial_xmit_delay & DSC_PPS_LSB_MASK);
 
 	/* PPS 18, 19 */
-	pps_sdp->pps_payload.initial_dec_delay =
+	pps_payload->initial_dec_delay =
 		cpu_to_be16(dsc_cfg->initial_dec_delay);
 
 	/* PPS 20 is 0 */
 
 	/* PPS 21 */
-	pps_sdp->pps_payload.initial_scale_value =
+	pps_payload->initial_scale_value =
 		dsc_cfg->initial_scale_value;
 
 	/* PPS 22, 23 */
-	pps_sdp->pps_payload.scale_increment_interval =
+	pps_payload->scale_increment_interval =
 		cpu_to_be16(dsc_cfg->scale_increment_interval);
 
 	/* PPS 24 */
-	pps_sdp->pps_payload.scale_decrement_interval_high =
+	pps_payload->scale_decrement_interval_high =
 		((dsc_cfg->scale_decrement_interval &
 		  DSC_PPS_SCALE_DEC_INT_HIGH_MASK) >>
 		 DSC_PPS_MSB_SHIFT);
 
 	/* PPS 25 */
-	pps_sdp->pps_payload.scale_decrement_interval_low =
+	pps_payload->scale_decrement_interval_low =
 		(dsc_cfg->scale_decrement_interval & DSC_PPS_LSB_MASK);
 
 	/* PPS 26[7:0], PPS 27[7:5] RESERVED */
 
 	/* PPS 27 */
-	pps_sdp->pps_payload.first_line_bpg_offset =
+	pps_payload->first_line_bpg_offset =
 		dsc_cfg->first_line_bpg_offset;
 
 	/* PPS 28, 29 */
-	pps_sdp->pps_payload.nfl_bpg_offset =
+	pps_payload->nfl_bpg_offset =
 		cpu_to_be16(dsc_cfg->nfl_bpg_offset);
 
 	/* PPS 30, 31 */
-	pps_sdp->pps_payload.slice_bpg_offset =
+	pps_payload->slice_bpg_offset =
 		cpu_to_be16(dsc_cfg->slice_bpg_offset);
 
 	/* PPS 32, 33 */
-	pps_sdp->pps_payload.initial_offset =
+	pps_payload->initial_offset =
 		cpu_to_be16(dsc_cfg->initial_offset);
 
 	/* PPS 34, 35 */
-	pps_sdp->pps_payload.final_offset = cpu_to_be16(dsc_cfg->final_offset);
+	pps_payload->final_offset = cpu_to_be16(dsc_cfg->final_offset);
 
 	/* PPS 36 */
-	pps_sdp->pps_payload.flatness_min_qp = dsc_cfg->flatness_min_qp;
+	pps_payload->flatness_min_qp = dsc_cfg->flatness_min_qp;
 
 	/* PPS 37 */
-	pps_sdp->pps_payload.flatness_max_qp = dsc_cfg->flatness_max_qp;
+	pps_payload->flatness_max_qp = dsc_cfg->flatness_max_qp;
 
 	/* PPS 38, 39 */
-	pps_sdp->pps_payload.rc_model_size =
+	pps_payload->rc_model_size =
 		cpu_to_be16(DSC_RC_MODEL_SIZE_CONST);
 
 	/* PPS 40 */
-	pps_sdp->pps_payload.rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST;
+	pps_payload->rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST;
 
 	/* PPS 41 */
-	pps_sdp->pps_payload.rc_quant_incr_limit0 =
+	pps_payload->rc_quant_incr_limit0 =
 		dsc_cfg->rc_quant_incr_limit0;
 
 	/* PPS 42 */
-	pps_sdp->pps_payload.rc_quant_incr_limit1 =
+	pps_payload->rc_quant_incr_limit1 =
 		dsc_cfg->rc_quant_incr_limit1;
 
 	/* PPS 43 */
-	pps_sdp->pps_payload.rc_tgt_offset = DSC_RC_TGT_OFFSET_LO_CONST |
+	pps_payload->rc_tgt_offset = DSC_RC_TGT_OFFSET_LO_CONST |
 		DSC_RC_TGT_OFFSET_HI_CONST << DSC_PPS_RC_TGT_OFFSET_HI_SHIFT;
 
 	/* PPS 44 - 57 */
 	for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++)
-		pps_sdp->pps_payload.rc_buf_thresh[i] =
+		pps_payload->rc_buf_thresh[i] =
 			dsc_cfg->rc_buf_thresh[i];
 
 	/* PPS 58 - 87 */
@@ -216,35 +215,35 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
 	 * are as follows: Min_qp[15:11], max_qp[10:6], offset[5:0]
 	 */
 	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
-		pps_sdp->pps_payload.rc_range_parameters[i] =
+		pps_payload->rc_range_parameters[i] =
 			((dsc_cfg->rc_range_params[i].range_min_qp <<
 			  DSC_PPS_RC_RANGE_MINQP_SHIFT) |
 			 (dsc_cfg->rc_range_params[i].range_max_qp <<
 			  DSC_PPS_RC_RANGE_MAXQP_SHIFT) |
 			 (dsc_cfg->rc_range_params[i].range_bpg_offset));
-		pps_sdp->pps_payload.rc_range_parameters[i] =
-			cpu_to_be16(pps_sdp->pps_payload.rc_range_parameters[i]);
+		pps_payload->rc_range_parameters[i] =
+			cpu_to_be16(pps_payload->rc_range_parameters[i]);
 	}
 
 	/* PPS 88 */
-	pps_sdp->pps_payload.native_422_420 = dsc_cfg->native_422 |
+	pps_payload->native_422_420 = dsc_cfg->native_422 |
 		dsc_cfg->native_420 << DSC_PPS_NATIVE_420_SHIFT;
 
 	/* PPS 89 */
-	pps_sdp->pps_payload.second_line_bpg_offset =
+	pps_payload->second_line_bpg_offset =
 		dsc_cfg->second_line_bpg_offset;
 
 	/* PPS 90, 91 */
-	pps_sdp->pps_payload.nsl_bpg_offset =
+	pps_payload->nsl_bpg_offset =
 		cpu_to_be16(dsc_cfg->nsl_bpg_offset);
 
 	/* PPS 92, 93 */
-	pps_sdp->pps_payload.second_line_offset_adj =
+	pps_payload->second_line_offset_adj =
 		cpu_to_be16(dsc_cfg->second_line_offset_adj);
 
 	/* PPS 94 - 127 are O */
 }
-EXPORT_SYMBOL(drm_dsc_pps_infoframe_pack);
+EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
 
 /**
  * drm_dsc_compute_rc_parameters() - Write rate control
diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
index 8c8d96157333..3f9921ba4a76 100644
--- a/drivers/gpu/drm/i915/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/intel_vdsc.c
@@ -881,10 +881,10 @@ static void intel_dp_write_dsc_pps_sdp(struct intel_encoder *encoder,
 	struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
 
 	/* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
-	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp);
+	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp.pps_header);
 
 	/* Fill the PPS payload bytes as per DSC spec 1.2 Table 4-1 */
-	drm_dsc_pps_infoframe_pack(&dp_dsc_pps_sdp, vdsc_cfg);
+	drm_dsc_pps_payload_pack(&dp_dsc_pps_sdp.pps_payload, vdsc_cfg);
 
 	intel_dig_port->write_infoframe(encoder, crtc_state,
 					DP_SDP_PPS, &dp_dsc_pps_sdp,
diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
index f26a89e1b68a..887954cbfc60 100644
--- a/include/drm/drm_dsc.h
+++ b/include/drm/drm_dsc.h
@@ -601,8 +601,8 @@ struct drm_dsc_pps_infoframe {
 	struct drm_dsc_picture_parameter_set pps_payload;
 } __packed;
 
-void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp);
-void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
+void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header);
+void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp,
 				const struct drm_dsc_config *dsc_cfg);
 int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/dsc: Split DSC PPS and SDP header initialisations

[Manasi Navare]

On Thu, Feb 21, 2019 at 03:20:01PM -0500, David Francis wrote:
> The DP 1.4 spec defines the SDP header and SDP contents for
> a Picture Parameter Set (PPS) that must be sent in advance
> of DSC transmission to define the encoding characteristics.
> 
> This was done in one struct, drm_dsc_pps_infoframe, which
> conatined the SDP header and PPS.  Because the PPS is
> a property of DSC over any connector, not just DP, and because
> drm drivers may have their own SDP structs they wish to use,
> make the functions that initialise SDP and PPS headers take
> the components they operate on, not drm_dsc_pps_infoframe,
> 
> Signed-off-by: David Francis <David.Francis@amd.com>

The corresponding changes for the header init and payload init now
look good to me.

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/drm_dsc.c         | 117 +++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_vdsc.c |   4 +-
>  include/drm/drm_dsc.h             |   4 +-
>  3 files changed, 62 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index d77570bf6ac4..77f4e5ae4197 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -32,66 +32,65 @@
>  /**
>   * drm_dsc_dp_pps_header_init() - Initializes the PPS Header
>   * for DisplayPort as per the DP 1.4 spec.
> - * @pps_sdp: Secondary data packet for DSC Picture Parameter Set
> - *           as defined in &struct drm_dsc_pps_infoframe
> + * @pps_header: Secondary data packet header for DSC Picture
> + *              Parameter Set as defined in &struct dp_sdp_header
>   *
>   * DP 1.4 spec defines the secondary data packet for sending the
>   * picture parameter infoframes from the source to the sink.
> - * This function populates the pps header defined in
> - * &struct drm_dsc_pps_infoframe as per the header bytes defined
> - * in &struct dp_sdp_header.
> + * This function populates the SDP header defined in
> + * &struct dp_sdp_header.
>   */
> -void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp)
> +void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header)
>  {
> -	memset(&pps_sdp->pps_header, 0, sizeof(pps_sdp->pps_header));
> +	memset(pps_header, 0, sizeof(*pps_header));
>  
> -	pps_sdp->pps_header.HB1 = DP_SDP_PPS;
> -	pps_sdp->pps_header.HB2 = DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1;
> +	pps_header->HB1 = DP_SDP_PPS;
> +	pps_header->HB2 = DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1;
>  }
>  EXPORT_SYMBOL(drm_dsc_dp_pps_header_init);
>  
>  /**
> - * drm_dsc_pps_infoframe_pack() - Populates the DSC PPS infoframe
> + * drm_dsc_pps_payload_pack() - Populates the DSC PPS
>   *
> - * @pps_sdp:
> - * Secondary data packet for DSC Picture Parameter Set. This is defined
> - * by &struct drm_dsc_pps_infoframe
> + * @pps_payload:
> + * Bitwise struct for DSC Picture Parameter Set. This is defined
> + * by &struct drm_dsc_picture_parameter_set
>   * @dsc_cfg:
>   * DSC Configuration data filled by driver as defined by
>   * &struct drm_dsc_config
>   *
> - * DSC source device sends a secondary data packet filled with all the
> - * picture parameter set (PPS) information required by the sink to decode
> - * the compressed frame. Driver populates the dsC PPS infoframe using the DSC
> - * configuration parameters in the order expected by the DSC Display Sink
> - * device. For the DSC, the sink device expects the PPS payload in the big
> - * endian format for the fields that span more than 1 byte.
> + * DSC source device sends a picture parameter set (PPS) containing the
> + * information required by the sink to decode the compressed frame. Driver
> + * populates the DSC PPS struct using the DSC configuration parameters in
> + * the order expected by the DSC Display Sink device. For the DSC, the sink
> + * device expects the PPS payload in big endian format for fields
> + * that span more than 1 byte.
>   */
> -void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
> +void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload,
>  				const struct drm_dsc_config *dsc_cfg)
>  {
>  	int i;
>  
>  	/* Protect against someone accidently changing struct size */
> -	BUILD_BUG_ON(sizeof(pps_sdp->pps_payload) !=
> +	BUILD_BUG_ON(sizeof(*pps_payload) !=
>  		     DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1 + 1);
>  
> -	memset(&pps_sdp->pps_payload, 0, sizeof(pps_sdp->pps_payload));
> +	memset(pps_payload, 0, sizeof(*pps_payload));
>  
>  	/* PPS 0 */
> -	pps_sdp->pps_payload.dsc_version =
> +	pps_payload->dsc_version =
>  		dsc_cfg->dsc_version_minor |
>  		dsc_cfg->dsc_version_major << DSC_PPS_VERSION_MAJOR_SHIFT;
>  
>  	/* PPS 1, 2 is 0 */
>  
>  	/* PPS 3 */
> -	pps_sdp->pps_payload.pps_3 =
> +	pps_payload->pps_3 =
>  		dsc_cfg->line_buf_depth |
>  		dsc_cfg->bits_per_component << DSC_PPS_BPC_SHIFT;
>  
>  	/* PPS 4 */
> -	pps_sdp->pps_payload.pps_4 =
> +	pps_payload->pps_4 =
>  		((dsc_cfg->bits_per_pixel & DSC_PPS_BPP_HIGH_MASK) >>
>  		 DSC_PPS_MSB_SHIFT) |
>  		dsc_cfg->vbr_enable << DSC_PPS_VBR_EN_SHIFT |
> @@ -100,7 +99,7 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
>  		dsc_cfg->block_pred_enable << DSC_PPS_BLOCK_PRED_EN_SHIFT;
>  
>  	/* PPS 5 */
> -	pps_sdp->pps_payload.bits_per_pixel_low =
> +	pps_payload->bits_per_pixel_low =
>  		(dsc_cfg->bits_per_pixel & DSC_PPS_LSB_MASK);
>  
>  	/*
> @@ -111,103 +110,103 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
>  	 */
>  
>  	/* PPS 6, 7 */
> -	pps_sdp->pps_payload.pic_height = cpu_to_be16(dsc_cfg->pic_height);
> +	pps_payload->pic_height = cpu_to_be16(dsc_cfg->pic_height);
>  
>  	/* PPS 8, 9 */
> -	pps_sdp->pps_payload.pic_width = cpu_to_be16(dsc_cfg->pic_width);
> +	pps_payload->pic_width = cpu_to_be16(dsc_cfg->pic_width);
>  
>  	/* PPS 10, 11 */
> -	pps_sdp->pps_payload.slice_height = cpu_to_be16(dsc_cfg->slice_height);
> +	pps_payload->slice_height = cpu_to_be16(dsc_cfg->slice_height);
>  
>  	/* PPS 12, 13 */
> -	pps_sdp->pps_payload.slice_width = cpu_to_be16(dsc_cfg->slice_width);
> +	pps_payload->slice_width = cpu_to_be16(dsc_cfg->slice_width);
>  
>  	/* PPS 14, 15 */
> -	pps_sdp->pps_payload.chunk_size = cpu_to_be16(dsc_cfg->slice_chunk_size);
> +	pps_payload->chunk_size = cpu_to_be16(dsc_cfg->slice_chunk_size);
>  
>  	/* PPS 16 */
> -	pps_sdp->pps_payload.initial_xmit_delay_high =
> +	pps_payload->initial_xmit_delay_high =
>  		((dsc_cfg->initial_xmit_delay &
>  		  DSC_PPS_INIT_XMIT_DELAY_HIGH_MASK) >>
>  		 DSC_PPS_MSB_SHIFT);
>  
>  	/* PPS 17 */
> -	pps_sdp->pps_payload.initial_xmit_delay_low =
> +	pps_payload->initial_xmit_delay_low =
>  		(dsc_cfg->initial_xmit_delay & DSC_PPS_LSB_MASK);
>  
>  	/* PPS 18, 19 */
> -	pps_sdp->pps_payload.initial_dec_delay =
> +	pps_payload->initial_dec_delay =
>  		cpu_to_be16(dsc_cfg->initial_dec_delay);
>  
>  	/* PPS 20 is 0 */
>  
>  	/* PPS 21 */
> -	pps_sdp->pps_payload.initial_scale_value =
> +	pps_payload->initial_scale_value =
>  		dsc_cfg->initial_scale_value;
>  
>  	/* PPS 22, 23 */
> -	pps_sdp->pps_payload.scale_increment_interval =
> +	pps_payload->scale_increment_interval =
>  		cpu_to_be16(dsc_cfg->scale_increment_interval);
>  
>  	/* PPS 24 */
> -	pps_sdp->pps_payload.scale_decrement_interval_high =
> +	pps_payload->scale_decrement_interval_high =
>  		((dsc_cfg->scale_decrement_interval &
>  		  DSC_PPS_SCALE_DEC_INT_HIGH_MASK) >>
>  		 DSC_PPS_MSB_SHIFT);
>  
>  	/* PPS 25 */
> -	pps_sdp->pps_payload.scale_decrement_interval_low =
> +	pps_payload->scale_decrement_interval_low =
>  		(dsc_cfg->scale_decrement_interval & DSC_PPS_LSB_MASK);
>  
>  	/* PPS 26[7:0], PPS 27[7:5] RESERVED */
>  
>  	/* PPS 27 */
> -	pps_sdp->pps_payload.first_line_bpg_offset =
> +	pps_payload->first_line_bpg_offset =
>  		dsc_cfg->first_line_bpg_offset;
>  
>  	/* PPS 28, 29 */
> -	pps_sdp->pps_payload.nfl_bpg_offset =
> +	pps_payload->nfl_bpg_offset =
>  		cpu_to_be16(dsc_cfg->nfl_bpg_offset);
>  
>  	/* PPS 30, 31 */
> -	pps_sdp->pps_payload.slice_bpg_offset =
> +	pps_payload->slice_bpg_offset =
>  		cpu_to_be16(dsc_cfg->slice_bpg_offset);
>  
>  	/* PPS 32, 33 */
> -	pps_sdp->pps_payload.initial_offset =
> +	pps_payload->initial_offset =
>  		cpu_to_be16(dsc_cfg->initial_offset);
>  
>  	/* PPS 34, 35 */
> -	pps_sdp->pps_payload.final_offset = cpu_to_be16(dsc_cfg->final_offset);
> +	pps_payload->final_offset = cpu_to_be16(dsc_cfg->final_offset);
>  
>  	/* PPS 36 */
> -	pps_sdp->pps_payload.flatness_min_qp = dsc_cfg->flatness_min_qp;
> +	pps_payload->flatness_min_qp = dsc_cfg->flatness_min_qp;
>  
>  	/* PPS 37 */
> -	pps_sdp->pps_payload.flatness_max_qp = dsc_cfg->flatness_max_qp;
> +	pps_payload->flatness_max_qp = dsc_cfg->flatness_max_qp;
>  
>  	/* PPS 38, 39 */
> -	pps_sdp->pps_payload.rc_model_size =
> +	pps_payload->rc_model_size =
>  		cpu_to_be16(DSC_RC_MODEL_SIZE_CONST);
>  
>  	/* PPS 40 */
> -	pps_sdp->pps_payload.rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST;
> +	pps_payload->rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST;
>  
>  	/* PPS 41 */
> -	pps_sdp->pps_payload.rc_quant_incr_limit0 =
> +	pps_payload->rc_quant_incr_limit0 =
>  		dsc_cfg->rc_quant_incr_limit0;
>  
>  	/* PPS 42 */
> -	pps_sdp->pps_payload.rc_quant_incr_limit1 =
> +	pps_payload->rc_quant_incr_limit1 =
>  		dsc_cfg->rc_quant_incr_limit1;
>  
>  	/* PPS 43 */
> -	pps_sdp->pps_payload.rc_tgt_offset = DSC_RC_TGT_OFFSET_LO_CONST |
> +	pps_payload->rc_tgt_offset = DSC_RC_TGT_OFFSET_LO_CONST |
>  		DSC_RC_TGT_OFFSET_HI_CONST << DSC_PPS_RC_TGT_OFFSET_HI_SHIFT;
>  
>  	/* PPS 44 - 57 */
>  	for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++)
> -		pps_sdp->pps_payload.rc_buf_thresh[i] =
> +		pps_payload->rc_buf_thresh[i] =
>  			dsc_cfg->rc_buf_thresh[i];
>  
>  	/* PPS 58 - 87 */
> @@ -216,35 +215,35 @@ void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
>  	 * are as follows: Min_qp[15:11], max_qp[10:6], offset[5:0]
>  	 */
>  	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> -		pps_sdp->pps_payload.rc_range_parameters[i] =
> +		pps_payload->rc_range_parameters[i] =
>  			((dsc_cfg->rc_range_params[i].range_min_qp <<
>  			  DSC_PPS_RC_RANGE_MINQP_SHIFT) |
>  			 (dsc_cfg->rc_range_params[i].range_max_qp <<
>  			  DSC_PPS_RC_RANGE_MAXQP_SHIFT) |
>  			 (dsc_cfg->rc_range_params[i].range_bpg_offset));
> -		pps_sdp->pps_payload.rc_range_parameters[i] =
> -			cpu_to_be16(pps_sdp->pps_payload.rc_range_parameters[i]);
> +		pps_payload->rc_range_parameters[i] =
> +			cpu_to_be16(pps_payload->rc_range_parameters[i]);
>  	}
>  
>  	/* PPS 88 */
> -	pps_sdp->pps_payload.native_422_420 = dsc_cfg->native_422 |
> +	pps_payload->native_422_420 = dsc_cfg->native_422 |
>  		dsc_cfg->native_420 << DSC_PPS_NATIVE_420_SHIFT;
>  
>  	/* PPS 89 */
> -	pps_sdp->pps_payload.second_line_bpg_offset =
> +	pps_payload->second_line_bpg_offset =
>  		dsc_cfg->second_line_bpg_offset;
>  
>  	/* PPS 90, 91 */
> -	pps_sdp->pps_payload.nsl_bpg_offset =
> +	pps_payload->nsl_bpg_offset =
>  		cpu_to_be16(dsc_cfg->nsl_bpg_offset);
>  
>  	/* PPS 92, 93 */
> -	pps_sdp->pps_payload.second_line_offset_adj =
> +	pps_payload->second_line_offset_adj =
>  		cpu_to_be16(dsc_cfg->second_line_offset_adj);
>  
>  	/* PPS 94 - 127 are O */
>  }
> -EXPORT_SYMBOL(drm_dsc_pps_infoframe_pack);
> +EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
>  
>  /**
>   * drm_dsc_compute_rc_parameters() - Write rate control
> diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
> index 8c8d96157333..3f9921ba4a76 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -881,10 +881,10 @@ static void intel_dp_write_dsc_pps_sdp(struct intel_encoder *encoder,
>  	struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
>  
>  	/* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
> -	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp);
> +	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp.pps_header);
>  
>  	/* Fill the PPS payload bytes as per DSC spec 1.2 Table 4-1 */
> -	drm_dsc_pps_infoframe_pack(&dp_dsc_pps_sdp, vdsc_cfg);
> +	drm_dsc_pps_payload_pack(&dp_dsc_pps_sdp.pps_payload, vdsc_cfg);
>  
>  	intel_dig_port->write_infoframe(encoder, crtc_state,
>  					DP_SDP_PPS, &dp_dsc_pps_sdp,
> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h
> index f26a89e1b68a..887954cbfc60 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -601,8 +601,8 @@ struct drm_dsc_pps_infoframe {
>  	struct drm_dsc_picture_parameter_set pps_payload;
>  } __packed;
>  
> -void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp);
> -void drm_dsc_pps_infoframe_pack(struct drm_dsc_pps_infoframe *pps_sdp,
> +void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header);
> +void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp,
>  				const struct drm_dsc_config *dsc_cfg);
>  int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

✗ Fi.CI.CHECKPATCH: warning for Make DRM DSC helpers more generally usable

[Patchwork]

== Series Details ==

Series: Make DRM DSC helpers more generally usable
URL   : https://patchwork.freedesktop.org/series/57044/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
519067359d21 drm/i915: Move dsc rate params compute into drm
64f5f00ebe45 drm/dsc: Add native 420 and 422 support to compute_rc_params
4d004eda1b8a drm/dsc: Split DSC PPS and SDP header initialisations
-:82: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#82: FILE: drivers/gpu/drm/drm_dsc.c:70:
+void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload,
 				const struct drm_dsc_config *dsc_cfg)

-:325: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#325: FILE: include/drm/drm_dsc.h:606:
+void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp,
 				const struct drm_dsc_config *dsc_cfg);

total: 0 errors, 0 warnings, 2 checks, 291 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for Make DRM DSC helpers more generally usable

[Patchwork]

== Series Details ==

Series: Make DRM DSC helpers more generally usable
URL   : https://patchwork.freedesktop.org/series/57044/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Move dsc rate params compute into drm
Okay!

Commit: drm/dsc: Add native 420 and 422 support to compute_rc_params
Okay!

Commit: drm/dsc: Split DSC PPS and SDP header initialisations
-O:drivers/gpu/drm/drm_dsc.c:219:61:    expected restricted __be16 <noident>
-O:drivers/gpu/drm/drm_dsc.c:219:61:    got int
-O:drivers/gpu/drm/drm_dsc.c:219:61: warning: incorrect type in assignment (different base types)
-O:drivers/gpu/drm/drm_dsc.c:226:25:    expected unsigned short [unsigned] [usertype] val
-O:drivers/gpu/drm/drm_dsc.c:226:25:    got restricted __be16 <noident>
-O:drivers/gpu/drm/drm_dsc.c:226:25: warning: cast from restricted __be16
-O:drivers/gpu/drm/drm_dsc.c:226:25: warning: cast from restricted __be16
-O:drivers/gpu/drm/drm_dsc.c:226:25: warning: cast from restricted __be16
-O:drivers/gpu/drm/drm_dsc.c:226:25: warning: incorrect type in argument 1 (different base types)
+drivers/gpu/drm/drm_dsc.c:218:53:    expected restricted __be16 <noident>
+drivers/gpu/drm/drm_dsc.c:218:53:    got int
+drivers/gpu/drm/drm_dsc.c:218:53: warning: incorrect type in assignment (different base types)
+drivers/gpu/drm/drm_dsc.c:225:25:    expected unsigned short [unsigned] [usertype] val
+drivers/gpu/drm/drm_dsc.c:225:25:    got restricted __be16 <noident>
+drivers/gpu/drm/drm_dsc.c:225:25: warning: cast from restricted __be16
+drivers/gpu/drm/drm_dsc.c:225:25: warning: cast from restricted __be16
+drivers/gpu/drm/drm_dsc.c:225:25: warning: cast from restricted __be16
+drivers/gpu/drm/drm_dsc.c:225:25: warning: incorrect type in argument 1 (different base types)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Make DRM DSC helpers more generally usable

[Patchwork]

== Series Details ==

Series: Make DRM DSC helpers more generally usable
URL   : https://patchwork.freedesktop.org/series/57044/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5650 -> Patchwork_12278
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57044/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12278 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         FAIL [fdo#104008] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109527]: https://bugs.freedesktop.org/show_bug.cgi?id=109527
  [fdo#109528]: https://bugs.freedesktop.org/show_bug.cgi?id=109528
  [fdo#109530]: https://bugs.freedesktop.org/show_bug.cgi?id=109530
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720


Participating hosts (44 -> 39)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5650 -> Patchwork_12278

  CI_DRM_5650: a4c5c4791699aeebfff694c222c76abb61900fca @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4849: e5088c8218d1c2b559a9e1645d34f929d05c3889 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12278: 4d004eda1b8a42ca860a49952942b14b1e52a190 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4d004eda1b8a drm/dsc: Split DSC PPS and SDP header initialisations
64f5f00ebe45 drm/dsc: Add native 420 and 422 support to compute_rc_params
519067359d21 drm/i915: Move dsc rate params compute into drm

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12278/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Make DRM DSC helpers more generally usable

[Patchwork]

== Series Details ==

Series: Make DRM DSC helpers more generally usable
URL   : https://patchwork.freedesktop.org/series/57044/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5650_full -> Patchwork_12278_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12278_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@extended-semaphore-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109275] / [fdo#109276]

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109281] +1

  * igt@gem_ctx_param@invalid-param-get:
    - shard-snb:          NOTRUN -> FAIL [fdo#109559]

  * igt@gem_ctx_param@invalid-param-set:
    - shard-snb:          NOTRUN -> FAIL [fdo#109674]

  * igt@gem_exec_parse@oacontrol-tracking:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289]

  * igt@gem_exec_reuse@contexts:
    - shard-snb:          NOTRUN -> INCOMPLETE [fdo#105411]

  * igt@gem_exec_schedule@preempt-contexts-bsd2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +9

  * igt@gem_exec_schedule@preempt-other-chain-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +1290

  * igt@gem_mocs_settings@mocs-reset-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] / [fdo#109287]

  * igt@gem_mocs_settings@mocs-reset-ctx-render:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109287] +2

  * igt@gem_pwrite@huge-cpu-backwards:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109290]

  * igt@gem_stolen@stolen-fill-purge:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277]

  * igt@i915_pm_rpm@gem-execbuf:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#108840]

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109308]

  * igt@i915_pm_rpm@system-suspend-modeset:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +6

  * igt@i915_pm_rps@min-max-config-loaded:
    - shard-iclb:         NOTRUN -> FAIL [fdo#102250]

  * igt@kms_busy@basic-flip-d:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +144

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#107956] +5
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107956]
    - shard-hsw:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-d:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +4

  * igt@kms_ccs@pipe-a-crc-primary-basic:
    - shard-iclb:         NOTRUN -> FAIL [fdo#107725]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-apl:          PASS -> FAIL [fdo#106510] / [fdo#108145]

  * igt@kms_chamelium@hdmi-edid-read:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +2

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-onscreen:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@cursorb-vs-flipb-atomic:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +5

  * igt@kms_flip@flip-vs-suspend:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +17

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167] +1

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166]
    - shard-iclb:         PASS -> FAIL [fdo#103166] +2

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-apl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +2

  * igt@kms_psr@no_drrs:
    - shard-iclb:         PASS -> FAIL [fdo#108341]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> FAIL [fdo#109016]

  * igt@kms_setmode@basic:
    - shard-snb:          NOTRUN -> FAIL [fdo#99912]

  * igt@kms_tv_load_detect@load-detect:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109309]

  * igt@perf_pmu@rc6:
    - shard-kbl:          PASS -> SKIP [fdo#109271]

  * igt@prime_nv_pcopy@test3_3:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291]

  
#### Possible fixes ####

  * igt@i915_pm_rpm@gem-evict-pwrite:
    - shard-iclb:         INCOMPLETE [fdo#108840] -> PASS

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +3

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_fbcon_fbt@fbc:
    - shard-iclb:         DMESG-WARN [fdo#109593] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-glk:          FAIL [fdo#103167] -> PASS
    - shard-iclb:         FAIL [fdo#103167] -> PASS +2

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-glk:          FAIL [fdo#103166] -> PASS
    - shard-iclb:         FAIL [fdo#103166] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102250]: https://bugs.freedesktop.org/show_bug.cgi?id=102250
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109275]: https://bugs.freedesktop.org/show_bug.cgi?id=109275
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109281]: https://bugs.freedesktop.org/show_bug.cgi?id=109281
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109287]: https://bugs.freedesktop.org/show_bug.cgi?id=109287
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109559]: https://bugs.freedesktop.org/show_bug.cgi?id=109559
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#109674]: https://bugs.freedesktop.org/show_bug.cgi?id=109674
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (6 -> 6)
------------------------------

  Additional (1): shard-snb 
  Missing    (1): shard-skl 


Build changes
-------------

    * Linux: CI_DRM_5650 -> Patchwork_12278

  CI_DRM_5650: a4c5c4791699aeebfff694c222c76abb61900fca @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4849: e5088c8218d1c2b559a9e1645d34f929d05c3889 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12278: 4d004eda1b8a42ca860a49952942b14b1e52a190 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12278/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 0/3] Make DRM DSC helpers more generally usable

[Christian König]

Am 21.02.19 um 21:19 schrieb David Francis:
> drm_dsc could use some work so that drm drivers other than
> i915 can make use of it their own DSC implementations
>
> Move rc compute, a function that forms part of the DSC spec,
> into drm. Update it to DSC 1.2. Also split the PPS packing and
> SDP header init functions, to allow for drivers with
> their own SDP struct headers
>
> Re-sending due to Mail Delivery System errors

I have to admit that I know absolutely nothing about the technical 
background of the implemented spec.

But from a high level view it makes complete sense to have stuff which 
implements a spec in common code. Additional to that coding style etc 
seems to be ok on first glance.

So series is Acked-by: Christian König <christian.koenig@amd.com>

Regards,
Christian.

>
> v2:
> Rebase onto drm-next
> Refactor drm_dsc_dp_pps_header_init
> Clean up documentation on new drm function
>
> David Francis (3):
>    drm/i915: Move dsc rate params compute into drm
>    drm/dsc: Add native 420 and 422 support to compute_rc_params
>    drm/dsc: Split DSC PPS and SDP header initialisations
>
>   drivers/gpu/drm/drm_dsc.c         | 269 +++++++++++++++++++++++-------
>   drivers/gpu/drm/i915/intel_vdsc.c | 133 +--------------
>   include/drm/drm_dsc.h             |   9 +-
>   3 files changed, 219 insertions(+), 192 deletions(-)
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx