On Thu, 2025-11-27 at 19:49 +0200, Imre Deak wrote:
> Convert intel_dp_output_bpp() and intel_dp_mode_min_output_bpp() to
> return an x16 fixed point bpp value, as this value will be always the
> link BPP (either compressed or uncompressed) tracked in the same x16
> fixed point format.
> 
> While at it rename

This line break can be avoided.

> intel_dp_output_bpp() to intel_dp_output_format_link_bpp_x16() and
> intel_dp_mode_min_output_bpp() to intel_dp_mode_min_link_bpp_x16() to
> better reflect that these functions return an x16 link BPP value
> specific to a particular output format or mode.
> 
> Also rename intel_dp_output_bpp()'s bpp parameter to pipe_bpp, to
> clarify which kind of (pipe vs. link) BPP the parameter is.
> 
> Signed-off-by: Imre Deak <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     | 41 +++++++++++----------
>  drivers/gpu/drm/i915/display/intel_dp.h     |  3 +-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  4 +-
>  3 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 6d232c15a0b5a..beda340d05923 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1234,7 +1234,7 @@ int intel_dp_min_bpp(enum intel_output_format 
> output_format)
>               return 8 * 3;
>  }
>  
> -int intel_dp_output_bpp(enum intel_output_format output_format, int bpp)
> +int intel_dp_output_format_link_bpp_x16(enum intel_output_format 
> output_format, int pipe_bpp)
>  {
>       /*
>        * bpp value was assumed to RGB format. And YCbCr 4:2:0 output
> @@ -1242,9 +1242,9 @@ int intel_dp_output_bpp(enum intel_output_format 
> output_format, int bpp)
>        * of bytes of RGB pixel.
>        */
>       if (output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> -             bpp /= 2;
> +             pipe_bpp /= 2;
>  
> -     return bpp;
> +     return fxp_q4_from_int(pipe_bpp);
>  }
>  
>  static enum intel_output_format
> @@ -1260,8 +1260,8 @@ intel_dp_sink_format(struct intel_connector *connector,
>  }
>  
>  static int
> -intel_dp_mode_min_output_bpp(struct intel_connector *connector,
> -                          const struct drm_display_mode *mode)
> +intel_dp_mode_min_link_bpp_x16(struct intel_connector *connector,
> +                            const struct drm_display_mode *mode)
>  {
>       enum intel_output_format output_format, sink_format;
>  
> @@ -1269,7 +1269,8 @@ intel_dp_mode_min_output_bpp(struct intel_connector 
> *connector,
>  
>       output_format = intel_dp_output_format(connector, sink_format);
>  
> -     return intel_dp_output_bpp(output_format, 
> intel_dp_min_bpp(output_format));
> +     return intel_dp_output_format_link_bpp_x16(output_format,
> +                                                
> intel_dp_min_bpp(output_format));
>  }
>  
>  static bool intel_dp_hdisplay_bad(struct intel_display *display,
> @@ -1341,11 +1342,11 @@ intel_dp_mode_valid_downstream(struct intel_connector 
> *connector,
>  
>       /* If PCON supports FRL MODE, check FRL bandwidth constraints */
>       if (intel_dp->dfp.pcon_max_frl_bw) {
> +             int link_bpp_x16 = intel_dp_mode_min_link_bpp_x16(connector, 
> mode);
>               int target_bw;
>               int max_frl_bw;
> -             int bpp = intel_dp_mode_min_output_bpp(connector, mode);
>  
> -             target_bw = bpp * target_clock;
> +             target_bw = fxp_q4_to_int_roundup(link_bpp_x16) * target_clock;
>  
>               max_frl_bw = intel_dp->dfp.pcon_max_frl_bw;
>  
> @@ -1460,6 +1461,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>       enum drm_mode_status status;
>       bool dsc = false;
>       int num_joined_pipes;
> +     int link_bpp_x16;
>  
>       status = intel_cpu_transcoder_mode_valid(display, mode);
>       if (status != MODE_OK)
> @@ -1502,8 +1504,8 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  
>       max_rate = intel_dp_max_link_data_rate(intel_dp, max_link_clock, 
> max_lanes);
>  
> -     mode_rate = intel_dp_link_required(target_clock,
> -                                        
> intel_dp_mode_min_output_bpp(connector, mode));
> +     link_bpp_x16 = intel_dp_mode_min_link_bpp_x16(connector, mode);
> +     mode_rate = intel_dp_link_required(target_clock, 
> fxp_q4_to_int_roundup(link_bpp_x16));
>  
>       if (intel_dp_has_dsc(connector)) {
>               int pipe_bpp;
> @@ -1815,9 +1817,10 @@ intel_dp_compute_link_config_wide(struct intel_dp 
> *intel_dp,
>       for (bpp = fxp_q4_to_int(limits->link.max_bpp_x16);
>            bpp >= fxp_q4_to_int(limits->link.min_bpp_x16);
>            bpp -= 2 * 3) {
> -             int link_bpp = intel_dp_output_bpp(pipe_config->output_format, 
> bpp);
> +             int link_bpp_x16 =
> +                     
> intel_dp_output_format_link_bpp_x16(pipe_config->output_format, bpp);
>  
> -             mode_rate = intel_dp_link_required(clock, link_bpp);
> +             mode_rate = intel_dp_link_required(clock, 
> fxp_q4_to_int_roundup(link_bpp_x16));
>  
>               for (i = 0; i < intel_dp->num_common_rates; i++) {
>                       link_rate = intel_dp_common_rate(intel_dp, i);
> @@ -2201,10 +2204,10 @@ static int dsc_compute_compressed_bpp(struct intel_dp 
> *intel_dp,
>       struct intel_display *display = to_intel_display(intel_dp);
>       const struct intel_connector *connector = 
> to_intel_connector(conn_state->connector);
>       const struct drm_display_mode *adjusted_mode = 
> &pipe_config->hw.adjusted_mode;
> -     int output_bpp;
>       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 link_bpp_x16;
>       int bpp_x16;
>       int ret;
>  
> @@ -2216,8 +2219,8 @@ static int dsc_compute_compressed_bpp(struct intel_dp 
> *intel_dp,
>       bpp_step_x16 = intel_dp_dsc_bpp_step_x16(connector);
>  
>       /* Compressed BPP should be less than the Input DSC bpp */
> -     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);
> +     link_bpp_x16 = 
> intel_dp_output_format_link_bpp_x16(pipe_config->output_format, pipe_bpp);
> +     max_bpp_x16 = min(max_bpp_x16, link_bpp_x16 - bpp_step_x16);
>  
>       drm_WARN_ON(display->drm, !is_power_of_2(bpp_step_x16));
>       min_bpp_x16 = round_up(limits->link.min_bpp_x16, bpp_step_x16);
> @@ -3267,8 +3270,8 @@ int intel_dp_compute_min_hblank(struct intel_crtc_state 
> *crtc_state,
>       if (crtc_state->dsc.compression_enable)
>               link_bpp_x16 = crtc_state->dsc.compressed_bpp_x16;
>       else
> -             link_bpp_x16 = 
> fxp_q4_from_int(intel_dp_output_bpp(crtc_state->output_format,
> -                                                                
> crtc_state->pipe_bpp));
> +             link_bpp_x16 = 
> intel_dp_output_format_link_bpp_x16(crtc_state->output_format,
> +                                                                
> crtc_state->pipe_bpp);
>  
>       /* Calculate min Hblank Link Layer Symbol Cycle Count for 8b/10b MST & 
> 128b/132b */
>       hactive_sym_cycles = drm_dp_link_symbol_cycles(max_lane_count,
> @@ -3378,8 +3381,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>       if (pipe_config->dsc.compression_enable)
>               link_bpp_x16 = pipe_config->dsc.compressed_bpp_x16;
>       else
> -             link_bpp_x16 = 
> fxp_q4_from_int(intel_dp_output_bpp(pipe_config->output_format,
> -                                                                
> pipe_config->pipe_bpp));
> +             link_bpp_x16 = 
> intel_dp_output_format_link_bpp_x16(pipe_config->output_format,
> +                                                                
> pipe_config->pipe_bpp);
>  
>       if (intel_dp->mso_link_count) {
>               int n = intel_dp->mso_link_count;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h 
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index 200a8b267f647..97e361458f760 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -193,7 +193,8 @@ void intel_dp_pcon_dsc_configure(struct intel_dp 
> *intel_dp,
>  
>  void intel_dp_invalidate_source_oui(struct intel_dp *intel_dp);
>  void intel_dp_wait_source_oui(struct intel_dp *intel_dp);
> -int intel_dp_output_bpp(enum intel_output_format output_format, int bpp);
> +int intel_dp_output_format_link_bpp_x16(enum intel_output_format 
> output_format,
> +                                     int pipe_bpp);

I'm not the biggest fan of very long function names like this, but I
can't come up with anything better...


>  
>  bool intel_dp_compute_config_limits(struct intel_dp *intel_dp,
>                                   struct drm_connector_state *conn_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 4c0b943fe86f1..1a4784f0cd6bd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -344,8 +344,8 @@ int intel_dp_mtp_tu_compute_config(struct intel_dp 
> *intel_dp,
>               }
>  
>               link_bpp_x16 = dsc ? bpp_x16 :
> -                     
> fxp_q4_from_int(intel_dp_output_bpp(crtc_state->output_format,
> -                                                         
> fxp_q4_to_int(bpp_x16)));
> +                     
> intel_dp_output_format_link_bpp_x16(crtc_state->output_format,
> +                                                         
> fxp_q4_to_int(bpp_x16));
>  
>               local_bw_overhead = intel_dp_mst_bw_overhead(crtc_state,
>                                                            false, 
> dsc_slice_count, link_bpp_x16);

Reviewed-by: Luca Coelho <[email protected]>

--
Cheers,
Luca.

Reply via email to