Re: [PATCH] drm/i915/display: update to the BW buddy configuration

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jun 16, 2026 at 01:15:53PM +0300, Vinod Govindapillai wrote:
> Bspec has been updated for xe2_lpd+ platforms on how to handle
> the bw buddy prgramming in case no matching memory configuration
> is found w.r.t the current page mask table. The recommendation
> is to keep the default settings for the related registers as it
> is without explicitly disabling the bw buddy.
> 
> Bspec: 68871
> Suggested-by: Ville Syrjala <ville.syrjala@intel.com>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 43 +++++++++++++------
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 2e51dfcd5dce..dcfd78f48622 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1631,23 +1631,40 @@ static void tgl_bw_buddy_init(struct intel_display *display)
>  		    table[config].type == dram_info->type)
>  			break;
>  
> +	/*
> +	 * If we don't recognize the memory configuration, explicitly disable
> +	 * the address buddy logic in pre-xe2_lpd platforms as it was before.
> +	 * In xe2_lpd+ cases, page masks must be set to 0 if no matching
> +	 * configuration is found in the table. So keep the default settings
> +	 * as it is. By default, BW_BUDDY_CTL bit 31 is 0 (bw buddy enabled)
> +	 * and BW_BUDDY_PAGE_MASK is 0x0

This seems to mostly just repeat what the code already says.

> +	 *
> +	 * TODO: Revisit the buddy page masks table when bspec updates the
> +	 * table with the correct number of channels for each dram type.
> +	 */

That TODO is unrelated to the changes here, and also seems misplaced.

>  	if (table[config].page_mask == 0) {
>  		drm_dbg_kms(display->drm,
>  			    "Unknown memory configuration; disabling address buddy logic.\n");
> -		for_each_set_bit(i, &abox_mask, BITS_PER_TYPE(abox_mask))
> -			intel_de_write(display, BW_BUDDY_CTL(i),
> -				       BW_BUDDY_DISABLE);
> -	} else {
> -		for_each_set_bit(i, &abox_mask, BITS_PER_TYPE(abox_mask)) {
> -			intel_de_write(display, BW_BUDDY_PAGE_MASK(i),
> -				       table[config].page_mask);
> -
> -			/* Wa_22010178259:tgl,dg1,rkl,adl-s */
> -			if (intel_display_wa(display, INTEL_DISPLAY_WA_22010178259))
> -				intel_de_rmw(display, BW_BUDDY_CTL(i),
> -					     BW_BUDDY_TLB_REQ_TIMER_MASK,
> -					     BW_BUDDY_TLB_REQ_TIMER(0x8));
> +
> +		if (DISPLAY_VER(display) < 20) {
> +			for_each_set_bit(i, &abox_mask, BITS_PER_TYPE(abox_mask))
> +				intel_de_write(display, BW_BUDDY_CTL(i),
> +					       BW_BUDDY_DISABLE);
>  		}
> +
> +		return;
> +	}
> +
> +	/* We found a matching configuration. Program the BW_BUDDY registers. */

Seems obvious. The comment is redundant.

> +	for_each_set_bit(i, &abox_mask, BITS_PER_TYPE(abox_mask)) {
> +		intel_de_write(display, BW_BUDDY_PAGE_MASK(i),
> +			       table[config].page_mask);

Please split code restructuring from functional changes. This diff
is too hard to read.

> +
> +		/* Wa_22010178259:tgl,dg1,rkl,adl-s */
> +		if (intel_display_wa(display, INTEL_DISPLAY_WA_22010178259))
> +			intel_de_rmw(display, BW_BUDDY_CTL(i),
> +				     BW_BUDDY_TLB_REQ_TIMER_MASK,
> +				     BW_BUDDY_TLB_REQ_TIMER(0x8));
>  	}
>  }
>  
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel