Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `enable_dc_balance` module parameter is initialized to `false` (0), unintentionally disabling the VRR DC balancing feature by default and contradicting the intended default behavior. --
commit ce08e2bbe0ea201e24fac54e608cc7792b588403 Author: Mitul Golani <[email protected]> drm/i915/display: Add command line param for DC balance This commit adds a new module parameter enable_dc_balance to allow enabling or disabling the VRR DC balance feature at runtime. The DC balance computation in intel_vrr_dc_balance_compute_config() is updated to honor this parameter. > diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c > b/drivers/gpu/drm/i915/display/intel_display_params.c > index 2aed110c5b090..ed3aa84c5d43d 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_params.c > +++ b/drivers/gpu/drm/i915/display/intel_display_params.c > @@ -120,6 +120,9 @@ intel_display_param_named_unsafe(enable_psr, int, 0400, [ ... ] > +intel_display_param_named_unsafe(enable_dc_balance, int, 0400, > + "Enable DC Balance (0=disabled, 1=enable). Default: 1 (use per-chip > default)"); [Severity: Medium] The parameter documentation in intel_display_param_named_unsafe() states that the default value is 1, but the underlying parameter definition sets it to false. > diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h > b/drivers/gpu/drm/i915/display/intel_display_params.h > index b95ecf728daab..8c28c27e2c46c 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_params.h > +++ b/drivers/gpu/drm/i915/display/intel_display_params.h > @@ -46,6 +46,7 @@ struct drm_printer; [ ... ] > param(int, enable_psr, -1, 0600) \ > + param(int, enable_dc_balance, false, 0600) \ [Severity: Medium] Does this unintentionally disable the VRR DC balance feature by default? The macro initializes enable_dc_balance to false (0), contradicting the intended default documented above. > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c > b/drivers/gpu/drm/i915/display/intel_vrr.c > index 51e4f3309b8b1..cfc0cfbff017a 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -386,10 +386,12 @@ static bool intel_vrr_dc_balance_possible(const struct > intel_crtc_state *crtc_st > static void > intel_vrr_dc_balance_compute_config(struct intel_crtc_state *crtc_state) > { [ ... ] > - if (!intel_vrr_dc_balance_possible(crtc_state) || > !crtc_state->vrr.enable) > + if (!intel_vrr_dc_balance_possible(crtc_state) || > + !crtc_state->vrr.enable || !display->params.enable_dc_balance) > return; [Severity: Medium] Because the parameter defaults to 0, !display->params.enable_dc_balance evaluates to true here. Can this cause intel_vrr_dc_balance_compute_config() to silently bail out early, disabling the feature by default on all configurations? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
