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

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

[Jani Nikula]

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>
---
 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

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

[Matt Roper]

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

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

[Jani Nikula]

On Fri, 05 Sep 2025, Matt Roper <matthew.d.roper@intel.com> wrote:
> 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>

Thanks, pushed to din.

BR,
Jani.

-- 
Jani Nikula, Intel