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

[Manasi Navare <manasi.d.navare@intel.com>]

On Wed, Feb 13, 2019 at 09:45:34AM -0500, David Francis wrote:
> 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.
>

Yes this sounds fair since its DSC spec related and can move to drm_dsc.c.
As a part of this series or later you should also consider moving the
rc_parameters struct for input bpc/output BPP combinations to DRM since that
is also purely spec related.

With this change and compute_rc_params function in DRM, please add appropriate
description of the function as part of kernel documentation.

With the documentation change, you have my r-b.

Regards
Manasi
 
> Signed-off-by: David Francis <David.Francis@amd.com>
> ---
>  drivers/gpu/drm/drm_dsc.c         | 133 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_vdsc.c | 125 +---------------------------
>  include/drm/drm_dsc.h             |   1 +
>  3 files changed, 135 insertions(+), 124 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index bc2b23adb072..4b0e3c9c3ff8 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>
>  
> @@ -226,3 +227,135 @@ 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. Some configuration
> + * fields must be present beforehand.
> + *
> + * @dsc_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 c56ba0e04044..c76cec8bfb74 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -318,129 +318,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)
>  {
> @@ -575,7 +452,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 d03f1b83421a..ad43494f1cc8 100644
> --- a/include/drm/drm_dsc.h
> +++ b/include/drm/drm_dsc.h
> @@ -481,5 +481,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
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx