Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org, ankit.k.nautiyal@intel.com
Subject: Re: [PATCH v2] drm/i915/display: Add command line param for DC balance
Date: Wed, 16 Sep 2026 10:26:20 +0300	[thread overview]
Message-ID: <cc91cacab48c7134647a12da0bfb8c3832e2fc5b@intel.com> (raw)
In-Reply-To: <20260915040015.2451786-1-mitulkumar.ajitkumar.golani@intel.com>

On Tue, 15 Sep 2026, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add an 'enable_dc_balance' display module parameter, disabled by
> default, so the feature can be explicitly opted in at driver
> initialization on configurations where it is known to be beneficial,
> while preserving existing behaviour everywhere else.
> intel_vrr_dc_balance_compute_config() honours the parameter, so no
> recompilation is needed to try the feature.

How does this preserve existing behaviour? This disables DC balance by
default. Why?

What are you trying to do? What is the goal?

We shouldn't be adding module parameters to begin with, and the *only*
reason for adding them is *debugging* only. Nothing else.

"feature can be explicitly opted in" is *not* what we use module
parameters for at all.


BR,
Jani.


>
> --v2:
> - Make enable_dc_balance a bool and keep it disabled by default; fix the
>   parameter type/value mismatch and correct the description (Chaitanya
>   Kumar Borah, Jani Nikula)
> - Explain in the commit message why the feature is gated and why a
>   module parameter is used (Jani Nikula)
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_params.c | 4 ++++
>  drivers/gpu/drm/i915/display/intel_display_params.h | 1 +
>  drivers/gpu/drm/i915/display/intel_vrr.c            | 4 +++-
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c
> index 2aed110c5b090..ca0ef466bb103 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
> @@ -120,6 +120,10 @@ intel_display_param_named_unsafe(enable_psr, int, 0400,
>  	"(0=disabled, 1=enable up to PSR1, 2=enable up to PSR2) "
>  	"Default: -1 (use per-chip default)");
>  
> +intel_display_param_named_unsafe(enable_dc_balance, bool, 0400,
> +	"Enable VRR DC balance (0=disabled, 1=enabled). "
> +	"Default: 0 (disabled)");
> +
>  intel_display_param_named_unsafe(enable_panel_replay, int, 0400,
>  	"Enable Panel Replay (0=disabled, 1=enabled). Default: -1 (use per-chip default)");
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
> index ba01aeaf89440..5c5a1a1358c32 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(bool, enable_dp_mst, true, 0600) \
>  	param(int, enable_fbc, -1, 0600) \
>  	param(int, enable_psr, -1, 0600) \
> +	param(bool, enable_dc_balance, false, 0600) \
>  	param(int, enable_panel_replay, -1, 0600) \
>  	param(bool, psr_safest_params, false, 0400) \
>  	param(bool, enable_psr2_sel_fetch, true, 0400) \
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index e36db11744405..1698c54e258b4 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -439,10 +439,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)
>  {
> +	struct intel_display *display = to_intel_display(crtc_state);
>  	int guardband_usec, adjustment_usec;
>  	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  
> -	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;
>  
>  	crtc_state->vrr.dc_balance.vmax = crtc_state->vrr.vmax;

-- 
Jani Nikula, Intel

  parent reply	other threads:[~2026-09-16  7:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15  4:00 [PATCH v2] drm/i915/display: Add command line param for DC balance Mitul Golani
2026-09-15  6:02 ` ✓ i915.CI.BAT: success for drm/i915/display: Add command line param for DC balance (rev2) Patchwork
2026-09-15  8:53 ` ✗ i915.CI.Full: failure " Patchwork
2026-09-16  6:38 ` [PATCH v2] drm/i915/display: Add command line param for DC balance Nautiyal, Ankit K
2026-09-16  7:26 ` Jani Nikula [this message]
2026-09-16  9:20   ` Golani, Mitulkumar Ajitkumar
2026-09-16 10:11     ` Jani Nikula
2026-09-17  5:25       ` Golani, Mitulkumar Ajitkumar
2026-09-16  9:48 ` ✓ i915.CI.Full: success for drm/i915/display: Add command line param for DC balance (rev2) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cc91cacab48c7134647a12da0bfb8c3832e2fc5b@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=ankit.k.nautiyal@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=mitulkumar.ajitkumar.golani@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox