Re: [PATCH v1] drm/i915/display: Add command line param for DC balance

[sashiko-bot@kernel.org]

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 <mitulkumar.ajitkumar.golani@intel.com>

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/20260828085037.1430881-1-mitulkumar.ajitkumar.golani@intel.com?part=1