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

[Manasi Navare via amd-gfx]

On Wed, Feb 13, 2019 at 09:45:35AM -0500, David Francis 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
> 
> Signed-off-by: David Francis 

This looks good and verified that the DSC 1.2 spec actually renames it
as simple_422.

Reviewed-by: Manasi Navare 

Manasi

> ---
>  drivers/gpu/drm/drm_dsc.c | 31 +++
>  drivers/gpu/drm/i915/intel_vdsc.c |  4 ++--
>  include/drm/drm_dsc.h |  4 ++--
>  3 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index 4b0e3c9c3ff8..9e675dd39a44 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -77,7 +77,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;
>  
> @@ -246,19 +246,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 c76cec8bfb74..7702c5c8b3f2 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -369,7 +369,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 =
> @@ -496,7 +496,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)
>   

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

[Wentland, Harry]

On 2019-02-13 9:45 a.m., David Francis 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
> 
> Signed-off-by: David Francis 

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/drm_dsc.c | 31 +++
>  drivers/gpu/drm/i915/intel_vdsc.c |  4 ++--
>  include/drm/drm_dsc.h |  4 ++--
>  3 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index 4b0e3c9c3ff8..9e675dd39a44 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -77,7 +77,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;
>  
> @@ -246,19 +246,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 c76cec8bfb74..7702c5c8b3f2 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -369,7 +369,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 =
> @@ -496,7 +496,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;
> 

[PATCH 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

Signed-off-by: David Francis 
---
 drivers/gpu/drm/drm_dsc.c | 31 +++
 drivers/gpu/drm/i915/intel_vdsc.c |  4 ++--
 include/drm/drm_dsc.h |  4 ++--
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index 4b0e3c9c3ff8..9e675dd39a44 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -77,7 +77,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;
 
@@ -246,19 +246,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 c76cec8bfb74..7702c5c8b3f2 100644
--- a/drivers/gpu/drm/i915/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/intel_vdsc.c
@@ -369,7 +369,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 =
@@ -496,7 +496,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 ad43494f1cc8..4e55e37943d7 100644
--- a/include/drm/drm_dsc.h
+++ b/include/drm/drm_dsc.h
@@ -70,10 +70,10 @@ struct