Re: [PATCH 09/14] drm/i915/dp: Inline do_dsc_compute_compressed_bpp()

[Imre Deak <imre.deak@intel.com>]

On Fri, Jan 31, 2025 at 02:50:02PM +0200, Jani Nikula wrote:
> With just the one platform independent loop left in
> do_dsc_compute_compressed_bpp(), we don't really need the extra function
> that is simply becoming increasingly hard to even figure out a decent
> name for. Just merge the whole thing to
> dsc_compute_compressed_bpp(). Good riddance to the short lived
> do_dsc_compute_compressed_bpp().
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

It also makes sense to keep all functions short, but here having to pass
a lot of parameters to do_dsc_compute_compressed_bpp() could argue
against that:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 60 ++++++++++---------------
>  1 file changed, 23 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 11a1ac28e21e..185c9f7e8538 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2097,41 +2097,6 @@ static bool intel_dp_dsc_valid_bpp(struct intel_dp *intel_dp, int bpp_x16)
>   * Find the max compressed BPP we can find a link configuration for. The BPPs to
>   * try depend on the source (platform) and sink.
>   */
> -static int
> -do_dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
> -			      struct intel_crtc_state *pipe_config,
> -			      const struct link_config_limits *limits,
> -			      int min_bpp_x16,
> -			      int max_bpp_x16,
> -			      int bpp_step_x16,
> -			      int timeslots)
> -{
> -	struct intel_display *display = to_intel_display(intel_dp);
> -	int bpp_x16;
> -	int ret;
> -
> -	for (bpp_x16 = max_bpp_x16; bpp_x16 >= min_bpp_x16; bpp_x16 -= bpp_step_x16) {
> -		if (!intel_dp_dsc_valid_bpp(intel_dp, bpp_x16))
> -			continue;
> -
> -		ret = dsc_compute_link_config(intel_dp,
> -					      pipe_config,
> -					      limits,
> -					      bpp_x16,
> -					      timeslots);
> -		if (ret == 0) {
> -			pipe_config->dsc.compressed_bpp_x16 = bpp_x16;
> -			if (intel_dp->force_dsc_fractional_bpp_en &&
> -			    fxp_q4_to_frac(bpp_x16))
> -				drm_dbg_kms(display->drm,
> -					    "Forcing DSC fractional bpp\n");
> -
> -			return 0;
> -		}
> -	}
> -	return -EINVAL;
> -}
> -
>  static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
>  				      const struct intel_connector *connector,
>  				      struct intel_crtc_state *pipe_config,
> @@ -2147,6 +2112,8 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
>  	int min_bpp_x16, max_bpp_x16, bpp_step_x16;
>  	int dsc_joiner_max_bpp;
>  	int num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
> +	int bpp_x16;
> +	int ret;
>  
>  	dsc_min_bpp = fxp_q4_to_int_roundup(limits->link.min_bpp_x16);
>  
> @@ -2165,8 +2132,27 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
>  	output_bpp = intel_dp_output_bpp(pipe_config->output_format, pipe_bpp);
>  	max_bpp_x16 = min(max_bpp_x16, fxp_q4_from_int(output_bpp) - bpp_step_x16);
>  
> -	return do_dsc_compute_compressed_bpp(intel_dp, pipe_config, limits,
> -					     min_bpp_x16, max_bpp_x16, bpp_step_x16, timeslots);
> +	for (bpp_x16 = max_bpp_x16; bpp_x16 >= min_bpp_x16; bpp_x16 -= bpp_step_x16) {
> +		if (!intel_dp_dsc_valid_bpp(intel_dp, bpp_x16))
> +			continue;
> +
> +		ret = dsc_compute_link_config(intel_dp,
> +					      pipe_config,
> +					      limits,
> +					      bpp_x16,
> +					      timeslots);
> +		if (ret == 0) {
> +			pipe_config->dsc.compressed_bpp_x16 = bpp_x16;
> +			if (intel_dp->force_dsc_fractional_bpp_en &&
> +			    fxp_q4_to_frac(bpp_x16))
> +				drm_dbg_kms(display->drm,
> +					    "Forcing DSC fractional bpp\n");
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
>  }
>  
>  int intel_dp_dsc_min_src_input_bpc(void)
> -- 
> 2.39.5
>