Re: [RFC v1 05/11] drm/i915/buf_trans: add intel_dp_above_hbr1() helper

[Jani Nikula <jani.nikula@linux.intel.com>]

On Sun, 08 Mar 2026, Michał Grzelak <michal.grzelak@intel.com> wrote:
> Check if port_clock is above HBR1 inside separate function.

I've got a mixture of reactions to this.

- For better or for worse, I think most people want to have these checks
  inline instead of hidden in a function.

- This whole "above hbr1" is also oddly specific. If we wanted to
  abstract this somehow, it should be generic. We have
  intel_dp_is_uhbr() because it changes everything about the link. None
  of the other rates are that special.

- If we wanted to hide the magic numbers, we'd need to define some RBR,
  HBR, etc. macros for the link rates... but then the problem is you'd
  have to remember what they mean. The numbers are easy.

- Realizing this kind of change is contentious, and unrelated to the
  feature at hand, you're probably better off dropping it (or suggesting
  it in another thread, another time) and focusing on the feature.

BR,
Jani.


>
> Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
> ---
>  .../drm/i915/display/intel_ddi_buf_trans.c    | 27 ++++++++++++-------
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> index 395dba8c9e4d..ee6a78a20dac 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> @@ -1184,6 +1184,13 @@ bool is_hobl_buf_trans(const struct intel_ddi_buf_trans *table)
>  	return table == &tgl_combo_phy_trans_edp_hbr2_hobl;
>  }
>  
> +static bool intel_dp_above_hbr1(const struct intel_crtc_state *crtc_state)
> +{
> +	if (crtc_state->port_clock > 270000)
> +		return true;
> +	return false;
> +}
> +
>  static bool use_edp_hobl(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> @@ -1396,7 +1403,7 @@ icl_get_mg_buf_trans_dp(struct intel_encoder *encoder,
>  			const struct intel_crtc_state *crtc_state,
>  			int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000) {
> +	if (intel_dp_above_hbr1(crtc_state)) {
>  		return intel_get_buf_trans(&icl_mg_phy_trans_hbr2_hbr3,
>  					   n_entries);
>  	} else {
> @@ -1421,7 +1428,7 @@ ehl_get_combo_buf_trans_edp(struct intel_encoder *encoder,
>  			    const struct intel_crtc_state *crtc_state,
>  			    int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000)
> +	if (intel_dp_above_hbr1(crtc_state))
>  		return intel_get_buf_trans(&ehl_combo_phy_trans_edp_hbr2, n_entries);
>  	else
>  		return intel_get_buf_trans(&icl_combo_phy_trans_edp_hbr2, n_entries);
> @@ -1446,7 +1453,7 @@ jsl_get_combo_buf_trans_edp(struct intel_encoder *encoder,
>  			    const struct intel_crtc_state *crtc_state,
>  			    int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000)
> +	if (intel_dp_above_hbr1(crtc_state))
>  		return intel_get_buf_trans(&jsl_combo_phy_trans_edp_hbr2, n_entries);
>  	else
>  		return intel_get_buf_trans(&jsl_combo_phy_trans_edp_hbr, n_entries);
> @@ -1473,7 +1480,7 @@ tgl_get_combo_buf_trans_dp(struct intel_encoder *encoder,
>  {
>  	struct intel_display *display = to_intel_display(encoder);
>  
> -	if (crtc_state->port_clock > 270000) {
> +	if (intel_dp_above_hbr1(crtc_state)) {
>  		if (display->platform.tigerlake_uy) {
>  			return intel_get_buf_trans(&tgl_uy_combo_phy_trans_dp_hbr2,
>  						   n_entries);
> @@ -1524,7 +1531,7 @@ dg1_get_combo_buf_trans_dp(struct intel_encoder *encoder,
>  			   const struct intel_crtc_state *crtc_state,
>  			   int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000)
> +	if (intel_dp_above_hbr1(crtc_state))
>  		return intel_get_buf_trans(&dg1_combo_phy_trans_dp_hbr2_hbr3,
>  					   n_entries);
>  	else
> @@ -1568,7 +1575,7 @@ rkl_get_combo_buf_trans_dp(struct intel_encoder *encoder,
>  			   const struct intel_crtc_state *crtc_state,
>  			   int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000)
> +	if (intel_dp_above_hbr1(crtc_state))
>  		return intel_get_buf_trans(&rkl_combo_phy_trans_dp_hbr2_hbr3, n_entries);
>  	else
>  		return intel_get_buf_trans(&rkl_combo_phy_trans_dp_hbr, n_entries);
> @@ -1611,7 +1618,7 @@ adls_get_combo_buf_trans_dp(struct intel_encoder *encoder,
>  			    const struct intel_crtc_state *crtc_state,
>  			    int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000)
> +	if (intel_dp_above_hbr1(crtc_state))
>  		return intel_get_buf_trans(&adls_combo_phy_trans_dp_hbr2_hbr3, n_entries);
>  	else
>  		return intel_get_buf_trans(&tgl_combo_phy_trans_dp_hbr, n_entries);
> @@ -1650,7 +1657,7 @@ adlp_get_combo_buf_trans_dp(struct intel_encoder *encoder,
>  			    const struct intel_crtc_state *crtc_state,
>  			    int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000)
> +	if (intel_dp_above_hbr1(crtc_state))
>  		return intel_get_buf_trans(&adlp_combo_phy_trans_dp_hbr2_hbr3, n_entries);
>  	else
>  		return intel_get_buf_trans(&adlp_combo_phy_trans_dp_hbr, n_entries);
> @@ -1693,7 +1700,7 @@ tgl_get_dkl_buf_trans_dp(struct intel_encoder *encoder,
>  			 const struct intel_crtc_state *crtc_state,
>  			 int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000) {
> +	if (intel_dp_above_hbr1(crtc_state)) {
>  		return intel_get_buf_trans(&tgl_dkl_phy_trans_dp_hbr2,
>  					   n_entries);
>  	} else {
> @@ -1718,7 +1725,7 @@ adlp_get_dkl_buf_trans_dp(struct intel_encoder *encoder,
>  			  const struct intel_crtc_state *crtc_state,
>  			  int *n_entries)
>  {
> -	if (crtc_state->port_clock > 270000) {
> +	if (intel_dp_above_hbr1(crtc_state)) {
>  		return intel_get_buf_trans(&adlp_dkl_phy_trans_dp_hbr2_hbr3,
>  					   n_entries);
>  	} else {

-- 
Jani Nikula, Intel