Re: [PATCH] drm/i915/power: fix size for for_each_set_bit() in abox iteration

[Matt Roper <matthew.d.roper@intel.com>]

On Fri, Sep 05, 2025 at 01:41:49PM +0300, Jani Nikula wrote:
> for_each_set_bit() expects size to be in bits, not bytes. The abox mask
> iteration uses bytes, but it works by coincidence, because the local
> variable holding the mask is unsigned long, and the mask only ever has
> bit 2 as the highest bit. Using a smaller type could lead to subtle and
> very hard to track bugs.
> 
> Fixes: 62afef2811e4 ("drm/i915/rkl: RKL uses ABOX0 for pixel transfers")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # v5.9+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Good catch.

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 7340d5a71673..6f56ce939f00 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1174,7 +1174,7 @@ static void icl_mbus_init(struct intel_display *display)
>  	if (DISPLAY_VER(display) == 12)
>  		abox_regs |= BIT(0);
>  
> -	for_each_set_bit(i, &abox_regs, sizeof(abox_regs))
> +	for_each_set_bit(i, &abox_regs, BITS_PER_TYPE(abox_regs))
>  		intel_de_rmw(display, MBUS_ABOX_CTL(i), mask, val);
>  }
>  
> @@ -1639,11 +1639,11 @@ static void tgl_bw_buddy_init(struct intel_display *display)
>  	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, sizeof(abox_mask))
> +		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, sizeof(abox_mask)) {
> +		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);
>  
> -- 
> 2.47.2
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation