On Mon, 2023-10-16 at 14:16 +0300, Jouni Högander wrote:
> Signed-off-by: Jouni Högander <jouni.hogan...@intel.com>
> ---
> .../gpu/drm/i915/display/intel_display_params.c | 15 +++++++++++++++
> .../gpu/drm/i915/display/intel_display_params.h | 5 +++++
> drivers/gpu/drm/i915/display/intel_psr.c | 14 +++++++-------
> drivers/gpu/drm/i915/i915_params.c | 15 ---------------
> drivers/gpu/drm/i915/i915_params.h | 3 ---
> 5 files changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c
> b/drivers/gpu/drm/i915/display/intel_display_params.c
> index 330613cd64db..eac82deede4c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
> @@ -31,6 +31,21 @@ intel_display_param_named_unsafe(enable_fbc, int, 0400,
> "Enable frame buffer compression for power savings "
> "(default: -1 (use per-chip default))");
>
> +intel_display_param_named_unsafe(enable_psr, int, 0400,
> + "Enable PSR "
> + "(0=disabled, 1=enable up to PSR1, 2=enable up to PSR2) "
> + "Default: -1 (use per-chip default)");
> +
> +intel_display_param_named(psr_safest_params, bool, 0400,
> + "Replace PSR VBT parameters by the safest and not optimal ones. This "
> + "is helpful to detect if PSR issues are related to bad values set in "
> + " VBT. (0=use VBT parameters, 1=use safest parameters)");
You don't say that the default here is 0.
> +
> +intel_display_param_named_unsafe(enable_psr2_sel_fetch, bool, 0400,
> + "Enable PSR2 selective fetch "
> + "(0=disabled, 1=enabled) "
> + "Default: 0");
> +
And here you say the default is 0, but...
> __maybe_unused
> static void _param_print_bool(struct drm_printer *p, const char *driver_name,
> const char *name, bool val)
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h
> b/drivers/gpu/drm/i915/display/intel_display_params.h
> index f1bdf2c6e5cd..99b79bed9363 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.h
> @@ -6,6 +6,8 @@
> #ifndef _INTEL_DISPLAY_PARAMS_H_
> #define _INTEL_DISPLAY_PARAMS_H_
>
> +#include <linux/types.h>
> +
> struct drm_printer;
> struct drm_i915_private;
>
> @@ -23,6 +25,9 @@ struct drm_i915_private;
> */
> #define INTEL_DISPLAY_PARAMS_FOR_EACH(param) \
> param(int, enable_fbc, -1, 0600) \
> + param(int, enable_psr, -1, 0600) \
> + param(bool, psr_safest_params, false, 0400) \
> + param(bool, enable_psr2_sel_fetch, true, 0400) \
...it's actually true.
>
> #define MEMBER(T, member, ...) T member;
> struct intel_display_params {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 4f1f31fc9529..ecd24a0b86cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -179,9 +179,9 @@ static bool psr_global_enabled(struct intel_dp *intel_dp)
>
> switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
> case I915_PSR_DEBUG_DEFAULT:
> - if (i915->params.enable_psr == -1)
> + if (i915->display.params.enable_psr == -1)
> return connector->panel.vbt.psr.enable;
> - return i915->params.enable_psr;
> + return i915->display.params.enable_psr;
> case I915_PSR_DEBUG_DISABLE:
> return false;
> default:
> @@ -198,7 +198,7 @@ static bool psr2_global_enabled(struct intel_dp *intel_dp)
> case I915_PSR_DEBUG_FORCE_PSR1:
> return false;
> default:
> - if (i915->params.enable_psr == 1)
> + if (i915->display.params.enable_psr == 1)
> return false;
> return true;
> }
> @@ -606,7 +606,7 @@ static u32 intel_psr1_get_tp_time(struct intel_dp
> *intel_dp)
> if (DISPLAY_VER(dev_priv) >= 11)
> val |= EDP_PSR_TP4_TIME_0us;
>
> - if (dev_priv->params.psr_safest_params) {
> + if (dev_priv->display.params.psr_safest_params) {
> val |= EDP_PSR_TP1_TIME_2500us;
> val |= EDP_PSR_TP2_TP3_TIME_2500us;
> goto check_tp3_sel;
> @@ -700,7 +700,7 @@ static u32 intel_psr2_get_tp_time(struct intel_dp
> *intel_dp)
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> u32 val = 0;
>
> - if (dev_priv->params.psr_safest_params)
> + if (dev_priv->display.params.psr_safest_params)
> return EDP_PSR2_TP2_TIME_2500us;
>
> if (connector->panel.vbt.psr.psr2_tp2_tp3_wakeup_time_us >= 0 &&
> @@ -943,7 +943,7 @@ static bool intel_psr2_sel_fetch_config_valid(struct
> intel_dp *intel_dp,
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>
> - if (!dev_priv->params.enable_psr2_sel_fetch &&
> + if (!dev_priv->display.params.enable_psr2_sel_fetch &&
> intel_dp->psr.debug != I915_PSR_DEBUG_ENABLE_SEL_FETCH) {
> drm_dbg_kms(&dev_priv->drm,
> "PSR2 sel fetch not enabled, disabled by
> parameter\n");
> @@ -1056,7 +1056,7 @@ static bool _compute_psr2_wake_times(struct intel_dp
> *intel_dp,
> fast_wake_lines > max_wake_lines)
> return false;
>
> - if (i915->params.psr_safest_params)
> + if (i915->display.params.psr_safest_params)
> io_wake_lines = fast_wake_lines = max_wake_lines;
>
> /* According to Bspec lower limit should be set as 7 lines. */
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 42700b854b79..c65e3314ae48 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -102,21 +102,6 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400,
> "WARNING: Disabling this can cause system wide hangs. "
> "(default: true)");
>
> -i915_param_named_unsafe(enable_psr, int, 0400,
> - "Enable PSR "
> - "(0=disabled, 1=enable up to PSR1, 2=enable up to PSR2) "
> - "Default: -1 (use per-chip default)");
> -
> -i915_param_named(psr_safest_params, bool, 0400,
> - "Replace PSR VBT parameters by the safest and not optimal ones. This "
> - "is helpful to detect if PSR issues are related to bad values set in "
> - " VBT. (0=use VBT parameters, 1=use safest parameters)");
> -
> -i915_param_named_unsafe(enable_psr2_sel_fetch, bool, 0400,
> - "Enable PSR2 selective fetch "
> - "(0=disabled, 1=enabled) "
> - "Default: 0");
> -
> i915_param_named_unsafe(enable_sagv, bool, 0600,
> "Enable system agent voltage/frequency scaling (SAGV) (default: true)");
>
> diff --git a/drivers/gpu/drm/i915/i915_params.h
> b/drivers/gpu/drm/i915/i915_params.h
> index e674de29f92c..47a05c4a8e89 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -52,10 +52,7 @@ struct drm_printer;
> param(int, panel_use_ssc, -1, 0600) \
> param(int, vbt_sdvo_panel_type, -1, 0400) \
> param(int, enable_dc, -1, 0400) \
> - param(int, enable_psr, -1, 0600) \
> param(bool, enable_dpt, true, 0400) \
> - param(bool, psr_safest_params, false, 0400) \
> - param(bool, enable_psr2_sel_fetch, true, 0400) \
I can see that it was wrong already before, but better fix it while at
it.
--
Cheers,
Luca.
