diff mbox series

[2/2] drm/i915/psr: Implment WA to help reach PC10

Message ID 20240606082926.1816416-4-suraj.kandpal@intel.com (mailing list archive)
State New, archived
Headers show
Series Implement WA to fix increased power usage | expand

Commit Message

Suraj Kandpal June 6, 2024, 8:29 a.m. UTC
To reach PC10 when PKG_C_LATENCY is configure we must do the following
things
1) Enter PSR1 only when delayed_vblank < 6 lines and DC5 can be entered
2) Allow PSR2 deep sleep when DC5 can be entered
3) DC5 can be entered when all transocoder have either PSR1, PSR2 or
eDP 1.5 PR ALPM enabled and VBI is disabled and flips and pushes are
not happening.

WA: 16023497226
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 75 +++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 2 deletions(-)

Comments

Jani Nikula June 6, 2024, 11:09 a.m. UTC | #1
On Thu, 06 Jun 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
> To reach PC10 when PKG_C_LATENCY is configure we must do the following
> things
> 1) Enter PSR1 only when delayed_vblank < 6 lines and DC5 can be entered
> 2) Allow PSR2 deep sleep when DC5 can be entered
> 3) DC5 can be entered when all transocoder have either PSR1, PSR2 or
> eDP 1.5 PR ALPM enabled and VBI is disabled and flips and pushes are
> not happening.
>
> WA: 16023497226
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 75 +++++++++++++++++++++++-
>  1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 6fc88f6c6b26..b22745c019df 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -811,12 +811,81 @@ static u8 psr_compute_idle_frames(struct intel_dp *intel_dp)
>  	return idle_frames;
>  }
>  
> +static bool intel_psr_check_delayed_vblank_limit(struct drm_i915_private *i915,
> +						 enum transcoder cpu_transcoder)
> +{
> +	return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;

Please don't use the hardware to preserve the state for you. It will get
really complicated to maintain.

> +}
> +
> +static bool intel_psr_is_dpkgc_configured(struct drm_i915_private *i915)
> +{
> +	return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;

Ditto.

> +}
> +
> +static bool intel_psr_is_dc5_entry_possible(struct drm_i915_private *i915)
> +{
> +	struct intel_crtc *intel_crtc;
> +	bool ret = true;
> +
> +	for_each_intel_crtc(&i915->drm, intel_crtc) {
> +		struct intel_encoder *encoder;
> +		struct drm_crtc *crtc = &intel_crtc->base;
> +		enum pipe pipe = intel_crtc->pipe;
> +
> +		if (!crtc->active)
> +			continue;
> +
> +		if (!(i915->display.irq.de_irq_mask[pipe] & GEN8_PIPE_VBLANK))

You have no business looking directly at that. It's for display irq code
*only*.

> +			ret = false;
> +
> +		for_each_encoder_on_crtc(&i915->drm, crtc, encoder) {
> +			struct intel_dp *intel_dp = enc_to_intel_dp(_encoder);
> +			struct intel_psr *psr = &intel_dp->psr;
> +
> +			if (!psr->enabled)
> +				ret = false;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static bool wa_16023497226_check(struct intel_dp *intel_dp, bool psr1)
> +{
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
> +
> +	if (DISPLAY_VER(i915) != 20)
> +		return true;
> +
> +	if (is_dpkg_c_configured(i915)) {
> +		if (psr1 &&
> +		    (intel_psr_check_delayed_vblank_limit(i915, cpu_transcoder) ||
> +		     intel_psr_is_dc5_entry_possible(i915)))
> +			return true;
> +		else if (!psr1 && is_dc5_entry_possible(i915))
> +			return true;
> +		else
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>  static bool hsw_activate_psr1(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>  	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
>  	u32 max_sleep_time = 0x1f;
> -	u32 val = EDP_PSR_ENABLE;
> +	u32 val = 0;
> +
> +	/* WA: 16023497226*/
> +	if (wa_16023497226_check(intel_dp, true)) {
> +		val = EDP_PSR_ENABLE;
> +	} else {
> +		drm_dbg_kms(&dev_priv->drm, "PSR1 was not activated\n");

Please add reason.

> +		return false;
> +	}

Switch the condition around and use early return.

>  
>  	val |= EDP_PSR_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
>  
> @@ -910,7 +979,9 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	u32 val = EDP_PSR2_ENABLE;
>  	u32 psr_val = 0;
>  
> -	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> +	/* WA: 16023497226*/
> +	if (wa_16023497226_check(intel_dp, false))
> +		val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
>  
>  	if (DISPLAY_VER(dev_priv) < 14 && !IS_ALDERLAKE_P(dev_priv))
>  		val |= EDP_SU_TRACK_ENABLE;
kernel test robot June 6, 2024, 9:48 p.m. UTC | #2
Hi Suraj,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.10-rc2 next-20240606]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Suraj-Kandpal/drm-i915-psr-Add-return-bool-value-for-hsw_activate_psr1/20240606-163351
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:    https://lore.kernel.org/r/20240606082926.1816416-4-suraj.kandpal%40intel.com
patch subject: [PATCH 2/2] drm/i915/psr: Implment WA to help reach PC10
config: microblaze-allmodconfig (https://download.01.org/0day-ci/archive/20240607/202406070543.soJpPCOs-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070543.soJpPCOs-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406070543.soJpPCOs-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/display/intel_psr.c:35:
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'intel_psr_check_delayed_vblank_limit':
>> drivers/gpu/drm/xe/compat-i915-headers/../../i915/i915_reg.h:4158:62: error: 'dev_priv' undeclared (first use in this function); did you mean 'dev_crit'?
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^~~~~~~~
   drivers/gpu/drm/i915/display/intel_de.h:31:69: note: in definition of macro 'intel_de_read'
      31 | #define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
         |                                                                     ^~~~~~~~~~~
   drivers/gpu/drm/xe/compat-i915-headers/../../i915/display/intel_display_reg_defs.h:42:49: note: in expansion of macro '_MMIO'
      42 | #define _MMIO_TRANS2(display, tran, reg)        _MMIO(DISPLAY_INFO(display)->trans_offsets[(tran)] - \
         |                                                 ^~~~~
   drivers/gpu/drm/i915/display/intel_display_device.h:185:42: note: in expansion of macro '__to_intel_display'
     185 | #define DISPLAY_INFO(i915)              (__to_intel_display(i915)->info.__device_info)
         |                                          ^~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/xe/compat-i915-headers/../../i915/display/intel_display_reg_defs.h:42:55: note: in expansion of macro 'DISPLAY_INFO'
      42 | #define _MMIO_TRANS2(display, tran, reg)        _MMIO(DISPLAY_INFO(display)->trans_offsets[(tran)] - \
         |                                                       ^~~~~~~~~~~~
   drivers/gpu/drm/xe/compat-i915-headers/../../i915/i915_reg.h:4158:49: note: in expansion of macro '_MMIO_TRANS2'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                 ^~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c:817:36: note: in expansion of macro 'TRANS_SET_CONTEXT_LATENCY'
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/xe/compat-i915-headers/../../i915/i915_reg.h:4158:62: note: each undeclared identifier is reported only once for each function it appears in
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^~~~~~~~
   drivers/gpu/drm/i915/display/intel_de.h:31:69: note: in definition of macro 'intel_de_read'
      31 | #define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
         |                                                                     ^~~~~~~~~~~
   drivers/gpu/drm/xe/compat-i915-headers/../../i915/display/intel_display_reg_defs.h:42:49: note: in expansion of macro '_MMIO'
      42 | #define _MMIO_TRANS2(display, tran, reg)        _MMIO(DISPLAY_INFO(display)->trans_offsets[(tran)] - \
         |                                                 ^~~~~
   drivers/gpu/drm/i915/display/intel_display_device.h:185:42: note: in expansion of macro '__to_intel_display'
     185 | #define DISPLAY_INFO(i915)              (__to_intel_display(i915)->info.__device_info)
         |                                          ^~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/xe/compat-i915-headers/../../i915/display/intel_display_reg_defs.h:42:55: note: in expansion of macro 'DISPLAY_INFO'
      42 | #define _MMIO_TRANS2(display, tran, reg)        _MMIO(DISPLAY_INFO(display)->trans_offsets[(tran)] - \
         |                                                       ^~~~~~~~~~~~
   drivers/gpu/drm/xe/compat-i915-headers/../../i915/i915_reg.h:4158:49: note: in expansion of macro '_MMIO_TRANS2'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                 ^~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c:817:36: note: in expansion of macro 'TRANS_SET_CONTEXT_LATENCY'
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/display/intel_psr.c:815:66: error: parameter 'cpu_transcoder' set but not used [-Werror=unused-but-set-parameter]
     815 |                                                  enum transcoder cpu_transcoder)
         |                                                  ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'intel_psr_is_dpkgc_configured':
>> drivers/gpu/drm/i915/display/intel_psr.c:822:36: error: 'LNL_PKG_C_LATENCY' undeclared (first use in this function)
     822 |         return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;
         |                                    ^~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_de.h:31:69: note: in definition of macro 'intel_de_read'
      31 | #define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
         |                                                                     ^~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'intel_psr_is_dc5_entry_possible':
>> drivers/gpu/drm/i915/display/intel_psr.c:835:26: error: 'struct drm_crtc' has no member named 'active'
     835 |                 if (!crtc->active)
         |                          ^~
>> drivers/gpu/drm/i915/display/intel_psr.c:842:69: error: '_encoder' undeclared (first use in this function); did you mean 'encoder'?
     842 |                         struct intel_dp *intel_dp = enc_to_intel_dp(_encoder);
         |                                                                     ^~~~~~~~
         |                                                                     encoder
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'wa_16023497226_check':
>> drivers/gpu/drm/i915/display/intel_psr.c:861:13: error: implicit declaration of function 'is_dpkg_c_configured' [-Werror=implicit-function-declaration]
     861 |         if (is_dpkg_c_configured(i915)) {
         |             ^~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/display/intel_psr.c:866:35: error: implicit declaration of function 'is_dc5_entry_possible'; did you mean 'intel_psr_is_dc5_entry_possible'? [-Werror=implicit-function-declaration]
     866 |                 else if (!psr1 && is_dc5_entry_possible(i915))
         |                                   ^~~~~~~~~~~~~~~~~~~~~
         |                                   intel_psr_is_dc5_entry_possible
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'intel_psr_check_delayed_vblank_limit':
   drivers/gpu/drm/i915/display/intel_psr.c:818:1: warning: control reaches end of non-void function [-Wreturn-type]
     818 | }
         | ^
   drivers/gpu/drm/i915/display/intel_psr.c: At top level:
>> drivers/gpu/drm/i915/display/intel_psr.c:820:13: error: 'intel_psr_is_dpkgc_configured' defined but not used [-Werror=unused-function]
     820 | static bool intel_psr_is_dpkgc_configured(struct drm_i915_private *i915)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors


vim +4158 drivers/gpu/drm/xe/compat-i915-headers/../../i915/i915_reg.h

dae847991a4327 Paulo Zanoni          2012-10-15  4153  
1d53ccdc400c87 José Roberto de Souza 2021-06-16  4154  #define _TRANS_A_SET_CONTEXT_LATENCY		0x6007C
1d53ccdc400c87 José Roberto de Souza 2021-06-16  4155  #define _TRANS_B_SET_CONTEXT_LATENCY		0x6107C
1d53ccdc400c87 José Roberto de Souza 2021-06-16  4156  #define _TRANS_C_SET_CONTEXT_LATENCY		0x6207C
1d53ccdc400c87 José Roberto de Souza 2021-06-16  4157  #define _TRANS_D_SET_CONTEXT_LATENCY		0x6307C
407569ff790979 Jani Nikula           2024-04-23 @4158  #define TRANS_SET_CONTEXT_LATENCY(tran)		_MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
1d53ccdc400c87 José Roberto de Souza 2021-06-16  4159  #define  TRANS_SET_CONTEXT_LATENCY_MASK		REG_GENMASK(15, 0)
1d53ccdc400c87 José Roberto de Souza 2021-06-16  4160  #define  TRANS_SET_CONTEXT_LATENCY_VALUE(x)	REG_FIELD_PREP(TRANS_SET_CONTEXT_LATENCY_MASK, (x))
1d53ccdc400c87 José Roberto de Souza 2021-06-16  4161
kernel test robot June 6, 2024, 10:40 p.m. UTC | #3
Hi Suraj,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.10-rc2 next-20240606]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Suraj-Kandpal/drm-i915-psr-Add-return-bool-value-for-hsw_activate_psr1/20240606-163351
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:    https://lore.kernel.org/r/20240606082926.1816416-4-suraj.kandpal%40intel.com
patch subject: [PATCH 2/2] drm/i915/psr: Implment WA to help reach PC10
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20240607/202406070642.9SbQep4F-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070642.9SbQep4F-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406070642.9SbQep4F-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/display/intel_psr.c:35:
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'intel_psr_check_delayed_vblank_limit':
   drivers/gpu/drm/i915/i915_reg.h:4158:62: error: 'dev_priv' undeclared (first use in this function); did you mean 'dev_crit'?
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^~~~~~~~
   drivers/gpu/drm/i915/display/intel_de.h:31:69: note: in definition of macro 'intel_de_read'
      31 | #define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
         |                                                                     ^~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_display_reg_defs.h:42:49: note: in expansion of macro '_MMIO'
      42 | #define _MMIO_TRANS2(display, tran, reg)        _MMIO(DISPLAY_INFO(display)->trans_offsets[(tran)] - \
         |                                                 ^~~~~
   drivers/gpu/drm/i915/display/intel_display_device.h:185:42: note: in expansion of macro '__to_intel_display'
     185 | #define DISPLAY_INFO(i915)              (__to_intel_display(i915)->info.__device_info)
         |                                          ^~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_display_reg_defs.h:42:55: note: in expansion of macro 'DISPLAY_INFO'
      42 | #define _MMIO_TRANS2(display, tran, reg)        _MMIO(DISPLAY_INFO(display)->trans_offsets[(tran)] - \
         |                                                       ^~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_reg.h:4158:49: note: in expansion of macro '_MMIO_TRANS2'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                 ^~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c:817:36: note: in expansion of macro 'TRANS_SET_CONTEXT_LATENCY'
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_reg.h:4158:62: note: each undeclared identifier is reported only once for each function it appears in
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^~~~~~~~
   drivers/gpu/drm/i915/display/intel_de.h:31:69: note: in definition of macro 'intel_de_read'
      31 | #define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
         |                                                                     ^~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_display_reg_defs.h:42:49: note: in expansion of macro '_MMIO'
      42 | #define _MMIO_TRANS2(display, tran, reg)        _MMIO(DISPLAY_INFO(display)->trans_offsets[(tran)] - \
         |                                                 ^~~~~
   drivers/gpu/drm/i915/display/intel_display_device.h:185:42: note: in expansion of macro '__to_intel_display'
     185 | #define DISPLAY_INFO(i915)              (__to_intel_display(i915)->info.__device_info)
         |                                          ^~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_display_reg_defs.h:42:55: note: in expansion of macro 'DISPLAY_INFO'
      42 | #define _MMIO_TRANS2(display, tran, reg)        _MMIO(DISPLAY_INFO(display)->trans_offsets[(tran)] - \
         |                                                       ^~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_reg.h:4158:49: note: in expansion of macro '_MMIO_TRANS2'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                 ^~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c:817:36: note: in expansion of macro 'TRANS_SET_CONTEXT_LATENCY'
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c:815:66: warning: parameter 'cpu_transcoder' set but not used [-Wunused-but-set-parameter]
     815 |                                                  enum transcoder cpu_transcoder)
         |                                                  ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'intel_psr_is_dpkgc_configured':
   drivers/gpu/drm/i915/display/intel_psr.c:822:36: error: 'LNL_PKG_C_LATENCY' undeclared (first use in this function)
     822 |         return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;
         |                                    ^~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_de.h:31:69: note: in definition of macro 'intel_de_read'
      31 | #define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
         |                                                                     ^~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'intel_psr_is_dc5_entry_possible':
   drivers/gpu/drm/i915/display/intel_psr.c:835:26: error: 'struct drm_crtc' has no member named 'active'
     835 |                 if (!crtc->active)
         |                          ^~
   drivers/gpu/drm/i915/display/intel_psr.c:842:69: error: '_encoder' undeclared (first use in this function); did you mean 'encoder'?
     842 |                         struct intel_dp *intel_dp = enc_to_intel_dp(_encoder);
         |                                                                     ^~~~~~~~
         |                                                                     encoder
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'wa_16023497226_check':
   drivers/gpu/drm/i915/display/intel_psr.c:861:13: error: implicit declaration of function 'is_dpkg_c_configured' [-Werror=implicit-function-declaration]
     861 |         if (is_dpkg_c_configured(i915)) {
         |             ^~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c:866:35: error: implicit declaration of function 'is_dc5_entry_possible'; did you mean 'intel_psr_is_dc5_entry_possible'? [-Werror=implicit-function-declaration]
     866 |                 else if (!psr1 && is_dc5_entry_possible(i915))
         |                                   ^~~~~~~~~~~~~~~~~~~~~
         |                                   intel_psr_is_dc5_entry_possible
   drivers/gpu/drm/i915/display/intel_psr.c: In function 'intel_psr_check_delayed_vblank_limit':
   drivers/gpu/drm/i915/display/intel_psr.c:818:1: warning: control reaches end of non-void function [-Wreturn-type]
     818 | }
         | ^
   drivers/gpu/drm/i915/display/intel_psr.c: At top level:
>> drivers/gpu/drm/i915/display/intel_psr.c:820:13: warning: 'intel_psr_is_dpkgc_configured' defined but not used [-Wunused-function]
     820 | static bool intel_psr_is_dpkgc_configured(struct drm_i915_private *i915)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/intel_psr_is_dpkgc_configured +820 drivers/gpu/drm/i915/display/intel_psr.c

   819	
 > 820	static bool intel_psr_is_dpkgc_configured(struct drm_i915_private *i915)
   821	{
   822		return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;
   823	}
   824
kernel test robot June 7, 2024, 12:46 a.m. UTC | #4
Hi Suraj,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.10-rc2 next-20240606]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Suraj-Kandpal/drm-i915-psr-Add-return-bool-value-for-hsw_activate_psr1/20240606-163351
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:    https://lore.kernel.org/r/20240606082926.1816416-4-suraj.kandpal%40intel.com
patch subject: [PATCH 2/2] drm/i915/psr: Implment WA to help reach PC10
config: i386-buildonly-randconfig-002-20240607 (https://download.01.org/0day-ci/archive/20240607/202406070845.TnConzA7-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070845.TnConzA7-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406070845.TnConzA7-lkp@intel.com/

All errors (new ones prefixed by >>):

    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^
   include/linux/dev_printk.h:48:6: note: '_dev_crit' declared here
      48 | void _dev_crit(const struct device *dev, const char *fmt, ...);
         |      ^
   drivers/gpu/drm/i915/display/intel_psr.c:817:29: error: use of undeclared identifier 'dev_priv'; did you mean '_dev_crit'?
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^
   drivers/gpu/drm/i915/i915_reg.h:4158:55: note: expanded from macro 'TRANS_SET_CONTEXT_LATENCY'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^
   include/linux/dev_printk.h:48:6: note: '_dev_crit' declared here
      48 | void _dev_crit(const struct device *dev, const char *fmt, ...);
         |      ^
   drivers/gpu/drm/i915/display/intel_psr.c:817:29: error: controlling expression type 'void (*)(const struct device *, const char *, ...)' not compatible with any generic association type
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_reg.h:4158:55: note: expanded from macro 'TRANS_SET_CONTEXT_LATENCY'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^~~~~~~~
   drivers/gpu/drm/i915/display/intel_display_reg_defs.h:43:26: note: expanded from macro '_MMIO_TRANS2'
      43 |                                                       DISPLAY_INFO(display)->trans_offsets[TRANSCODER_A] + \
         |                                                                    ^~~~~~~
   drivers/gpu/drm/i915/display/intel_display_device.h:185:49: note: expanded from macro 'DISPLAY_INFO'
     185 | #define DISPLAY_INFO(i915)              (__to_intel_display(i915)->info.__device_info)
         |                                                             ^~~~
   drivers/gpu/drm/i915/display/intel_display_conversion.h:16:11: note: expanded from macro '__to_intel_display'
      16 |         _Generic(p,                                                     \
         |                  ^
   drivers/gpu/drm/i915/i915_reg_defs.h:267:47: note: expanded from macro '_MMIO'
     267 | #define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
         |                                               ^
   drivers/gpu/drm/i915/display/intel_de.h:31:69: note: expanded from macro 'intel_de_read'
      31 | #define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
         |                                                                     ^~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_psr.c:817:29: error: use of undeclared identifier 'dev_priv'; did you mean '_dev_crit'?
   drivers/gpu/drm/i915/i915_reg.h:4158:55: note: expanded from macro 'TRANS_SET_CONTEXT_LATENCY'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^
   include/linux/dev_printk.h:48:6: note: '_dev_crit' declared here
      48 | void _dev_crit(const struct device *dev, const char *fmt, ...);
         |      ^
   drivers/gpu/drm/i915/display/intel_psr.c:817:29: error: use of undeclared identifier 'dev_priv'; did you mean '_dev_crit'?
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^
   drivers/gpu/drm/i915/i915_reg.h:4158:55: note: expanded from macro 'TRANS_SET_CONTEXT_LATENCY'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^
   include/linux/dev_printk.h:48:6: note: '_dev_crit' declared here
      48 | void _dev_crit(const struct device *dev, const char *fmt, ...);
         |      ^
   drivers/gpu/drm/i915/display/intel_psr.c:817:29: error: use of undeclared identifier 'dev_priv'; did you mean '_dev_crit'?
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^
   drivers/gpu/drm/i915/i915_reg.h:4158:55: note: expanded from macro 'TRANS_SET_CONTEXT_LATENCY'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^
   include/linux/dev_printk.h:48:6: note: '_dev_crit' declared here
      48 | void _dev_crit(const struct device *dev, const char *fmt, ...);
         |      ^
   drivers/gpu/drm/i915/display/intel_psr.c:817:29: error: use of undeclared identifier 'dev_priv'; did you mean '_dev_crit'?
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^
   drivers/gpu/drm/i915/i915_reg.h:4158:55: note: expanded from macro 'TRANS_SET_CONTEXT_LATENCY'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^
   include/linux/dev_printk.h:48:6: note: '_dev_crit' declared here
      48 | void _dev_crit(const struct device *dev, const char *fmt, ...);
         |      ^
   drivers/gpu/drm/i915/display/intel_psr.c:817:29: error: use of undeclared identifier 'dev_priv'; did you mean '_dev_crit'?
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^
   drivers/gpu/drm/i915/i915_reg.h:4158:55: note: expanded from macro 'TRANS_SET_CONTEXT_LATENCY'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^
   include/linux/dev_printk.h:48:6: note: '_dev_crit' declared here
      48 | void _dev_crit(const struct device *dev, const char *fmt, ...);
         |      ^
   drivers/gpu/drm/i915/display/intel_psr.c:817:29: error: controlling expression type 'void (*)(const struct device *, const char *, ...)' not compatible with any generic association type
     817 |         return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_reg.h:4158:55: note: expanded from macro 'TRANS_SET_CONTEXT_LATENCY'
    4158 | #define TRANS_SET_CONTEXT_LATENCY(tran)         _MMIO_TRANS2(dev_priv, tran, _TRANS_A_SET_CONTEXT_LATENCY)
         |                                                              ^~~~~~~~
   drivers/gpu/drm/i915/display/intel_display_reg_defs.h:44:31: note: expanded from macro '_MMIO_TRANS2'
      44 |                                                       DISPLAY_MMIO_BASE(display) + (reg))
         |                                                                         ^~~~~~~
   drivers/gpu/drm/i915/display/intel_display_reg_defs.h:11:51: note: expanded from macro 'DISPLAY_MMIO_BASE'
      11 | #define DISPLAY_MMIO_BASE(dev_priv)     (DISPLAY_INFO(dev_priv)->mmio_offset)
         |                                                       ^~~~~~~~
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   drivers/gpu/drm/i915/display/intel_display_conversion.h:16:11: note: expanded from macro '__to_intel_display'
      16 |         _Generic(p,                                                     \
         |                  ^
   drivers/gpu/drm/i915/i915_reg_defs.h:267:47: note: expanded from macro '_MMIO'
     267 | #define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
         |                                               ^
   drivers/gpu/drm/i915/display/intel_de.h:31:69: note: expanded from macro 'intel_de_read'
      31 | #define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
         |                                                                     ^~~~~~~~~~~
>> drivers/gpu/drm/i915/display/intel_psr.c:822:29: error: use of undeclared identifier 'LNL_PKG_C_LATENCY'
     822 |         return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;
         |                                    ^
   fatal error: too many errors emitted, stopping now [-ferror-limit=]
   20 errors generated.


vim +/LNL_PKG_C_LATENCY +822 drivers/gpu/drm/i915/display/intel_psr.c

   819	
   820	static bool intel_psr_is_dpkgc_configured(struct drm_i915_private *i915)
   821	{
 > 822		return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;
   823	}
   824
Suraj Kandpal June 10, 2024, 4:54 a.m. UTC | #5
> Subject: Re: [PATCH 2/2] drm/i915/psr: Implment WA to help reach PC10
> 
> On Thu, 06 Jun 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
> > To reach PC10 when PKG_C_LATENCY is configure we must do the following
> > things
> > 1) Enter PSR1 only when delayed_vblank < 6 lines and DC5 can be
> > entered
> > 2) Allow PSR2 deep sleep when DC5 can be entered
> > 3) DC5 can be entered when all transocoder have either PSR1, PSR2 or
> > eDP 1.5 PR ALPM enabled and VBI is disabled and flips and pushes are
> > not happening.
> >
> > WA: 16023497226
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 75
> > +++++++++++++++++++++++-
> >  1 file changed, 73 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 6fc88f6c6b26..b22745c019df 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -811,12 +811,81 @@ static u8 psr_compute_idle_frames(struct
> intel_dp *intel_dp)
> >  	return idle_frames;
> >  }
> >
> > +static bool intel_psr_check_delayed_vblank_limit(struct drm_i915_private
> *i915,
> > +						 enum transcoder
> cpu_transcoder) {
> > +	return intel_de_read(i915,
> > +TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
> 
Hi Jani,
Thanks for the reviews

> Please don't use the hardware to preserve the state for you. It will get really
> complicated to maintain.
> 

Yes wanted to calculate the delayed vblank using the following way
Adjusted_mode->vblank_start - adjusted_mode->vblank_end
But I'll need crtc_state for that and I don't see a way of deriving it
Specially when this function is called from intel_psr_work
One way could be to have this wa check function be called from 
Intel_psr_enable_locked and save the corresponding Booleans in
Intel_psr or make in  drm_i915_private
structure and access that when intel_psr_activate is called from
Intel_psr_resume and intel_psr_work.
Do you think that could be feasible ?

> > +}
> > +
> > +static bool intel_psr_is_dpkgc_configured(struct drm_i915_private
> > +*i915) {
> > +	return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;
> 
> Ditto.
> 

Similar question as above only place that I can manage a state to see if it is configured or not
would be in drm_i915_private.

> > +}
> > +
> > +static bool intel_psr_is_dc5_entry_possible(struct drm_i915_private
> > +*i915) {
> > +	struct intel_crtc *intel_crtc;
> > +	bool ret = true;
> > +
> > +	for_each_intel_crtc(&i915->drm, intel_crtc) {
> > +		struct intel_encoder *encoder;
> > +		struct drm_crtc *crtc = &intel_crtc->base;
> > +		enum pipe pipe = intel_crtc->pipe;
> > +
> > +		if (!crtc->active)
> > +			continue;
> > +
> > +		if (!(i915->display.irq.de_irq_mask[pipe] &
> GEN8_PIPE_VBLANK))
> 
> You have no business looking directly at that. It's for display irq code *only*.
> 

Is there another way I can ensure if the vblank interrupt for the particular pipe is disabled?

> > +			ret = false;
> > +
> > +		for_each_encoder_on_crtc(&i915->drm, crtc, encoder) {
> > +			struct intel_dp *intel_dp = enc_to_intel_dp(_encoder);
> > +			struct intel_psr *psr = &intel_dp->psr;
> > +
> > +			if (!psr->enabled)
> > +				ret = false;
> > +		}
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static bool wa_16023497226_check(struct intel_dp *intel_dp, bool
> > +psr1) {
> > +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > +	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
> > +
> > +	if (DISPLAY_VER(i915) != 20)
> > +		return true;
> > +
> > +	if (is_dpkg_c_configured(i915)) {
> > +		if (psr1 &&
> > +		    (intel_psr_check_delayed_vblank_limit(i915,
> cpu_transcoder) ||
> > +		     intel_psr_is_dc5_entry_possible(i915)))
> > +			return true;
> > +		else if (!psr1 && is_dc5_entry_possible(i915))
> > +			return true;
> > +		else
> > +			return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> >  static bool hsw_activate_psr1(struct intel_dp *intel_dp)  {
> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >  	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
> >  	u32 max_sleep_time = 0x1f;
> > -	u32 val = EDP_PSR_ENABLE;
> > +	u32 val = 0;
> > +
> > +	/* WA: 16023497226*/
> > +	if (wa_16023497226_check(intel_dp, true)) {
> > +		val = EDP_PSR_ENABLE;
> > +	} else {
> > +		drm_dbg_kms(&dev_priv->drm, "PSR1 was not activated\n");
> 
> Please add reason.
> 
> > +		return false;
> > +	}
> 
> Switch the condition around and use early return.
> 

Sure will do.

Regards,
Suraj Kandpal
> >
> >  	val |= EDP_PSR_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> >
> > @@ -910,7 +979,9 @@ static void hsw_activate_psr2(struct intel_dp
> *intel_dp)
> >  	u32 val = EDP_PSR2_ENABLE;
> >  	u32 psr_val = 0;
> >
> > -	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> > +	/* WA: 16023497226*/
> > +	if (wa_16023497226_check(intel_dp, false))
> > +		val |=
> EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> >
> >  	if (DISPLAY_VER(dev_priv) < 14 && !IS_ALDERLAKE_P(dev_priv))
> >  		val |= EDP_SU_TRACK_ENABLE;
> 
> --
> Jani Nikula, Intel
Jani Nikula June 14, 2024, 1:41 p.m. UTC | #6
On Mon, 10 Jun 2024, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> Subject: Re: [PATCH 2/2] drm/i915/psr: Implment WA to help reach PC10
>> 
>> On Thu, 06 Jun 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
>> > To reach PC10 when PKG_C_LATENCY is configure we must do the following
>> > things
>> > 1) Enter PSR1 only when delayed_vblank < 6 lines and DC5 can be
>> > entered
>> > 2) Allow PSR2 deep sleep when DC5 can be entered
>> > 3) DC5 can be entered when all transocoder have either PSR1, PSR2 or
>> > eDP 1.5 PR ALPM enabled and VBI is disabled and flips and pushes are
>> > not happening.
>> >
>> > WA: 16023497226
>> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_psr.c | 75
>> > +++++++++++++++++++++++-
>> >  1 file changed, 73 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>> > b/drivers/gpu/drm/i915/display/intel_psr.c
>> > index 6fc88f6c6b26..b22745c019df 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> > @@ -811,12 +811,81 @@ static u8 psr_compute_idle_frames(struct
>> intel_dp *intel_dp)
>> >  	return idle_frames;
>> >  }
>> >
>> > +static bool intel_psr_check_delayed_vblank_limit(struct drm_i915_private
>> *i915,
>> > +						 enum transcoder
>> cpu_transcoder) {
>> > +	return intel_de_read(i915,
>> > +TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
>> 
> Hi Jani,
> Thanks for the reviews
>
>> Please don't use the hardware to preserve the state for you. It will get really
>> complicated to maintain.
>> 
>
> Yes wanted to calculate the delayed vblank using the following way
> Adjusted_mode->vblank_start - adjusted_mode->vblank_end
> But I'll need crtc_state for that and I don't see a way of deriving it
> Specially when this function is called from intel_psr_work
> One way could be to have this wa check function be called from 
> Intel_psr_enable_locked and save the corresponding Booleans in
> Intel_psr or make in  drm_i915_private
> structure and access that when intel_psr_activate is called from
> Intel_psr_resume and intel_psr_work.
> Do you think that could be feasible ?

You'll be able to figure out a lot of cases up front at compute config
time, and disable PSR beforehand.

You'll know LNL_PKG_C_LATENCY (we seem to always configure it). You'll
know TRANS_SET_CONTEXT_LATENCY. You'll know whether all transcoders have
PSR enabled.

I think you'll need to split the conditions, and disable PSR as early as
possible when it should not be enabled. Then at actual enabling time,
you'll know the conditions that already have to hold, and you can check
fewer things.

This workaround is a bummer because it's permanent. It also means we
need to do this properly. Can't just poke at random stuff, because it'll
be painful forever.

BR,
Jani.

>
>> > +}
>> > +
>> > +static bool intel_psr_is_dpkgc_configured(struct drm_i915_private
>> > +*i915) {
>> > +	return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;
>> 
>> Ditto.
>> 
>
> Similar question as above only place that I can manage a state to see if it is configured or not
> would be in drm_i915_private.
>
>> > +}
>> > +
>> > +static bool intel_psr_is_dc5_entry_possible(struct drm_i915_private
>> > +*i915) {
>> > +	struct intel_crtc *intel_crtc;
>> > +	bool ret = true;
>> > +
>> > +	for_each_intel_crtc(&i915->drm, intel_crtc) {
>> > +		struct intel_encoder *encoder;
>> > +		struct drm_crtc *crtc = &intel_crtc->base;
>> > +		enum pipe pipe = intel_crtc->pipe;
>> > +
>> > +		if (!crtc->active)
>> > +			continue;
>> > +
>> > +		if (!(i915->display.irq.de_irq_mask[pipe] &
>> GEN8_PIPE_VBLANK))
>> 
>> You have no business looking directly at that. It's for display irq code *only*.
>> 
>
> Is there another way I can ensure if the vblank interrupt for the particular pipe is disabled?
>
>> > +			ret = false;
>> > +
>> > +		for_each_encoder_on_crtc(&i915->drm, crtc, encoder) {
>> > +			struct intel_dp *intel_dp = enc_to_intel_dp(_encoder);
>> > +			struct intel_psr *psr = &intel_dp->psr;
>> > +
>> > +			if (!psr->enabled)
>> > +				ret = false;
>> > +		}
>> > +	}
>> > +
>> > +	return ret;
>> > +}
>> > +
>> > +static bool wa_16023497226_check(struct intel_dp *intel_dp, bool
>> > +psr1) {
>> > +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> > +	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
>> > +
>> > +	if (DISPLAY_VER(i915) != 20)
>> > +		return true;
>> > +
>> > +	if (is_dpkg_c_configured(i915)) {
>> > +		if (psr1 &&
>> > +		    (intel_psr_check_delayed_vblank_limit(i915,
>> cpu_transcoder) ||
>> > +		     intel_psr_is_dc5_entry_possible(i915)))
>> > +			return true;
>> > +		else if (!psr1 && is_dc5_entry_possible(i915))
>> > +			return true;
>> > +		else
>> > +			return false;
>> > +	}
>> > +
>> > +	return true;
>> > +}
>> > +
>> >  static bool hsw_activate_psr1(struct intel_dp *intel_dp)  {
>> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>> >  	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
>> >  	u32 max_sleep_time = 0x1f;
>> > -	u32 val = EDP_PSR_ENABLE;
>> > +	u32 val = 0;
>> > +
>> > +	/* WA: 16023497226*/
>> > +	if (wa_16023497226_check(intel_dp, true)) {
>> > +		val = EDP_PSR_ENABLE;
>> > +	} else {
>> > +		drm_dbg_kms(&dev_priv->drm, "PSR1 was not activated\n");
>> 
>> Please add reason.
>> 
>> > +		return false;
>> > +	}
>> 
>> Switch the condition around and use early return.
>> 
>
> Sure will do.
>
> Regards,
> Suraj Kandpal
>> >
>> >  	val |= EDP_PSR_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
>> >
>> > @@ -910,7 +979,9 @@ static void hsw_activate_psr2(struct intel_dp
>> *intel_dp)
>> >  	u32 val = EDP_PSR2_ENABLE;
>> >  	u32 psr_val = 0;
>> >
>> > -	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
>> > +	/* WA: 16023497226*/
>> > +	if (wa_16023497226_check(intel_dp, false))
>> > +		val |=
>> EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
>> >
>> >  	if (DISPLAY_VER(dev_priv) < 14 && !IS_ALDERLAKE_P(dev_priv))
>> >  		val |= EDP_SU_TRACK_ENABLE;
>> 
>> --
>> Jani Nikula, Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 6fc88f6c6b26..b22745c019df 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -811,12 +811,81 @@  static u8 psr_compute_idle_frames(struct intel_dp *intel_dp)
 	return idle_frames;
 }
 
+static bool intel_psr_check_delayed_vblank_limit(struct drm_i915_private *i915,
+						 enum transcoder cpu_transcoder)
+{
+	return intel_de_read(i915, TRANS_SET_CONTEXT_LATENCY(cpu_transcoder)) >= 6;
+}
+
+static bool intel_psr_is_dpkgc_configured(struct drm_i915_private *i915)
+{
+	return intel_de_read(i915, LNL_PKG_C_LATENCY) == U32_MAX;
+}
+
+static bool intel_psr_is_dc5_entry_possible(struct drm_i915_private *i915)
+{
+	struct intel_crtc *intel_crtc;
+	bool ret = true;
+
+	for_each_intel_crtc(&i915->drm, intel_crtc) {
+		struct intel_encoder *encoder;
+		struct drm_crtc *crtc = &intel_crtc->base;
+		enum pipe pipe = intel_crtc->pipe;
+
+		if (!crtc->active)
+			continue;
+
+		if (!(i915->display.irq.de_irq_mask[pipe] & GEN8_PIPE_VBLANK))
+			ret = false;
+
+		for_each_encoder_on_crtc(&i915->drm, crtc, encoder) {
+			struct intel_dp *intel_dp = enc_to_intel_dp(_encoder);
+			struct intel_psr *psr = &intel_dp->psr;
+
+			if (!psr->enabled)
+				ret = false;
+		}
+	}
+
+	return ret;
+}
+
+static bool wa_16023497226_check(struct intel_dp *intel_dp, bool psr1)
+{
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
+
+	if (DISPLAY_VER(i915) != 20)
+		return true;
+
+	if (is_dpkg_c_configured(i915)) {
+		if (psr1 &&
+		    (intel_psr_check_delayed_vblank_limit(i915, cpu_transcoder) ||
+		     intel_psr_is_dc5_entry_possible(i915)))
+			return true;
+		else if (!psr1 && is_dc5_entry_possible(i915))
+			return true;
+		else
+			return false;
+	}
+
+	return true;
+}
+
 static bool hsw_activate_psr1(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
 	u32 max_sleep_time = 0x1f;
-	u32 val = EDP_PSR_ENABLE;
+	u32 val = 0;
+
+	/* WA: 16023497226*/
+	if (wa_16023497226_check(intel_dp, true)) {
+		val = EDP_PSR_ENABLE;
+	} else {
+		drm_dbg_kms(&dev_priv->drm, "PSR1 was not activated\n");
+		return false;
+	}
 
 	val |= EDP_PSR_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
 
@@ -910,7 +979,9 @@  static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	u32 val = EDP_PSR2_ENABLE;
 	u32 psr_val = 0;
 
-	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
+	/* WA: 16023497226*/
+	if (wa_16023497226_check(intel_dp, false))
+		val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
 
 	if (DISPLAY_VER(dev_priv) < 14 && !IS_ALDERLAKE_P(dev_priv))
 		val |= EDP_SU_TRACK_ENABLE;