Quoting Gustavo Sousa (2025-02-12 14:59:28-03:00) >Quoting Ville Syrjälä (2025-02-12 14:52:19-03:00) >>On Wed, Feb 12, 2025 at 02:43:16PM -0300, Gustavo Sousa wrote: >>> Let the compiler know that we are intetionally using a different enum >>> type to perform arithmetic with enum intel_display_power_domain in the >>> POWER_DOMAIN_*(). Do that by explicitly casting the macro argument to >>> int. >>> >>> Reported-by: kernel test robot <lkp@xxxxxxxxx> >>> Closes: https://lore.kernel.org/oe-kbuild-all/202502120809.XfmcqkBD-lkp@xxxxxxxxx/ >>> Signed-off-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx> >>> --- >>> drivers/gpu/drm/i915/display/intel_display_power.h | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h >>> index a3a5c1be8bab..3caa3f517a32 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_display_power.h >>> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h >>> @@ -117,12 +117,12 @@ enum intel_display_power_domain { >>> POWER_DOMAIN_INVALID = POWER_DOMAIN_NUM, >>> }; >>> >>> -#define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A) >>> +#define POWER_DOMAIN_PIPE(pipe) ((int)(pipe) + POWER_DOMAIN_PIPE_A) >>> #define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \ >>> - ((pipe) + POWER_DOMAIN_PIPE_PANEL_FITTER_A) >>> + ((int)(pipe) + POWER_DOMAIN_PIPE_PANEL_FITTER_A) >>> #define POWER_DOMAIN_TRANSCODER(tran) \ >>> ((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \ >>> - (tran) + POWER_DOMAIN_TRANSCODER_A) >>> + (int)(tran) + POWER_DOMAIN_TRANSCODER_A) >> >>I've generally gone for the >>POWER_DOMAIN_TRANSCODER_A + (tran) - TRANSCODER_A >>form for such things, to also make sure it works >>even if TRANSCODER_A isn't 0 anymore. >>Does that avoid the warning as well? > >Hm... That's a good idea; and I think it might avoid the warning indeed >(maybe we would need parentheses around (tran) - TRANSCODER_A). I did a quick test and this also took care of removing the clang warning in my environment: diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index e354051e8982..d46b35dbe018 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -123,7 +123,7 @@ enum intel_display_power_domain { ((enum intel_display_power_domain)((int)(pipe) + POWER_DOMAIN_PIPE_PANEL_FITTER_A)) #define POWER_DOMAIN_TRANSCODER(tran) \ ((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \ - (enum intel_display_power_domain)((int)(tran) + POWER_DOMAIN_TRANSCODER_A)) + (enum intel_display_power_domain)(POWER_DOMAIN_TRANSCODER_A + ((tran) - TRANSCODER_A))) struct intel_power_domain_mask { DECLARE_BITMAP(bits, POWER_DOMAIN_NUM); The parentheses around (tran) - TRANSCODER_A were indeed necessary, probably for the compiler to see that as an int expression. We can get rid of the parentheses if we do (tran) - TRANSCODER_A before adding POWER_DOMAIN_TRANSCODER_A: diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index e354051e8982..b15eb4fd5062 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -123,7 +123,7 @@ enum intel_display_power_domain { ((enum intel_display_power_domain)((int)(pipe) + POWER_DOMAIN_PIPE_PANEL_FITTER_A)) #define POWER_DOMAIN_TRANSCODER(tran) \ ((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \ - (enum intel_display_power_domain)((int)(tran) + POWER_DOMAIN_TRANSCODER_A)) + (enum intel_display_power_domain)((tran) - TRANSCODER_A + POWER_DOMAIN_TRANSCODER_A)) struct intel_power_domain_mask { DECLARE_BITMAP(bits, POWER_DOMAIN_NUM); I'm tending more toward the second alternative. -- Gustavo Sousa > >> >>Maybe these should even be functions rather than macros? > >Yeah. I actually considered this possibility, but went with the macros >to keep the change simple. > >If that's welcome, I could go ahead with turning those macros into >static inline functions. > >-- >Gustavo Sousa > >> >>> >>> struct intel_power_domain_mask { >>> DECLARE_BITMAP(bits, POWER_DOMAIN_NUM); >>> -- >>> 2.48.1 >> >>-- >>Ville Syrjälä >>Intel
