Re: [PATCH v2 8/9] drm/i915: Perform vblank evasion around legacy cursor updates

2024-01-17 Thread Hogander, Jouni




Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

2024-01-17 Thread Sharma, Swati2

On 17-Jan-24 11:31 PM, Jani Nikula wrote:

On Wed, 17 Jan 2024, "Sharma, Swati2"  wrote:

Hi Ville,

On 25-Oct-23 6:29 PM, Ville Syrjälä wrote:

On Wed, Oct 18, 2023 at 04:24:00PM +0300, Stanislav Lisovskiy wrote:

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 4f6835a7578e..6a9148232a9c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1153,7 +1153,11 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
if (!intel_dp_can_bigjoiner(intel_dp))
return false;
   
-	return clock > i915->max_dotclk_freq || hdisplay > 5120;

+   if (intel_dp->force_bigjoiner_enable)
+   drm_dbg_kms(>drm, "Forcing bigjoiner mode\n");


That's going to cause excessive dmesg spam.


Then how from dmesg we will get to know, big joiner was forced?
Shouldn't we have atleast one debug print to know this?


Yeah. But this gets called for every single display mode via
intel_dp_mode_valid().


ohh okay, can we add dbg print in intel_dp_compute_link_config()
from where intel_dp_need_bigjoiner() is called?



BR,
Jani.




✓ Fi.CI.BAT: success for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev7)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev7)
URL   : https://patchwork.freedesktop.org/series/123813/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_14135 -> Patchwork_123813v7


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/index.html

Participating hosts (36 -> 24)
--

  Additional (1): bat-rpls-2 
  Missing(13): bat-kbl-2 bat-adlp-9 fi-bsw-n3050 bat-adlm-1 fi-ilk-650 
fi-snb-2520m bat-adln-1 fi-glk-j4005 bat-jsl-1 bat-atsm-1 fi-pnv-d510 bat-jsl-3 
bat-mtlp-8 

Known issues


  Here are the changes found in Patchwork_123813v7 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@debugfs_test@basic-hwmon:
- bat-rpls-2: NOTRUN -> [SKIP][1] ([i915#9318])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@debugfs_t...@basic-hwmon.html

  * igt@gem_lmem_swapping@verify-random:
- bat-rpls-2: NOTRUN -> [SKIP][2] ([i915#4613]) +3 other tests skip
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@gem_lmem_swapp...@verify-random.html

  * igt@gem_tiled_pread_basic:
- bat-rpls-2: NOTRUN -> [SKIP][3] ([i915#3282])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
- bat-rpls-2: NOTRUN -> [SKIP][4] ([i915#6621])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@i915_pm_...@basic-api.html

  * igt@i915_selftest@live@gt_pm:
- bat-rpls-2: NOTRUN -> [DMESG-FAIL][5] ([i915#10010])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@i915_selftest@live@gt_pm.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-rpls-2: NOTRUN -> [SKIP][6] ([i915#4103]) +1 other test skip
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
- bat-rpls-2: NOTRUN -> [SKIP][7] ([i915#3555] / [i915#3840] / 
[i915#9886])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@kms_...@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
- bat-rpls-2: NOTRUN -> [SKIP][8] ([fdo#109285])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@kms_force_connector_ba...@force-load-detect.html

  * igt@kms_pm_backlight@basic-brightness:
- bat-rpls-2: NOTRUN -> [SKIP][9] ([i915#5354])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@kms_pm_backli...@basic-brightness.html

  * igt@kms_setmode@basic-clone-single-crtc:
- bat-rpls-2: NOTRUN -> [SKIP][10] ([i915#3555])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@kms_setm...@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-read:
- bat-rpls-2: NOTRUN -> [SKIP][11] ([fdo#109295] / [i915#3708]) +2 
other tests skip
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v7/bat-rpls-2/igt@prime_v...@basic-fence-read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#10010]: https://gitlab.freedesktop.org/drm/intel/issues/10010
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
  [i915#9732]: https://gitlab.freedesktop.org/drm/intel/issues/9732
  [i915#9886]: https://gitlab.freedesktop.org/drm/intel/issues/9886


Build changes
-

  * Linux: CI_DRM_14135 -> Patchwork_123813v7

  CI-20190529: 20190529
  CI_DRM_14135: 2d422dec58e0dfd61d6c9842de7f83289803dccb @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7678: 9f0b63bae98fa6164dd2262f8890a964fb1fc23d @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_123813v7: 2d422dec58e0dfd61d6c9842de7f83289803dccb @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

8e6505d0f0b1 drm/i915/guc: Enable Wa_14019159160

✗ Fi.CI.SPARSE: warning for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev7)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev7)
URL   : https://patchwork.freedesktop.org/series/123813/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




✗ Fi.CI.CHECKPATCH: warning for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev7)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev7)
URL   : https://patchwork.freedesktop.org/series/123813/
State : warning

== Summary ==

Error: dim checkpatch failed
9d7e1cd4555f drm/i915: Enable Wa_16019325821
13d2fbbcd57b drm/i915/guc: Add support for w/a KLVs
-:105: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#105: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:829:
+   GEM_BUG_ON(iosys_map_is_null(>ads_map));

total: 0 errors, 1 warnings, 0 checks, 159 lines checked
068b49ffec6a drm/i915/guc: Enable Wa_14019159160
-:101: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#101: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:830:
+   GEM_BUG_ON(remain < size);

total: 0 errors, 1 warnings, 0 checks, 99 lines checked




✗ Fi.CI.BAT: failure for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev6)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev6)
URL   : https://patchwork.freedesktop.org/series/123813/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14134 -> Patchwork_123813v6


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_123813v6 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_123813v6, please notify your bug team 
(i915-ci-in...@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v6/index.html

Participating hosts (36 -> 34)
--

  Missing(2): bat-rpls-2 fi-snb-2520m 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_123813v6:

### IGT changes ###

 Possible regressions 

  * igt@i915_selftest@live@hangcheck:
- fi-ivb-3770:[PASS][1] -> [ABORT][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14134/fi-ivb-3770/igt@i915_selftest@l...@hangcheck.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v6/fi-ivb-3770/igt@i915_selftest@l...@hangcheck.html

  
Known issues


  Here are the changes found in Patchwork_123813v6 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@i915_suspend@basic-s3-without-i915:
- bat-atsm-1: NOTRUN -> [SKIP][3] ([i915#6645])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v6/bat-atsm-1/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
- bat-atsm-1: NOTRUN -> [SKIP][4] ([i915#1836])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v6/bat-atsm-1/igt@kms_pipe_crc_ba...@suspend-read-crc.html

  
 Possible fixes 

  * igt@i915_selftest@live@gem_migrate:
- bat-atsm-1: [INCOMPLETE][5] -> [PASS][6]
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14134/bat-atsm-1/igt@i915_selftest@live@gem_migrate.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v6/bat-atsm-1/igt@i915_selftest@live@gem_migrate.html

  
  [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645


Build changes
-

  * Linux: CI_DRM_14134 -> Patchwork_123813v6

  CI-20190529: 20190529
  CI_DRM_14134: 6002d529f9fe420895d407e481d39b19b58ec001 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7678: 9f0b63bae98fa6164dd2262f8890a964fb1fc23d @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_123813v6: 6002d529f9fe420895d407e481d39b19b58ec001 @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

48dbf071d516 drm/i915/guc: Enable Wa_14019159160
2517daba1fc2 drm/i915/guc: Add support for w/a KLVs
e382efc391b6 drm/i915: Enable Wa_16019325821

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v6/index.html


✗ Fi.CI.SPARSE: warning for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev6)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev6)
URL   : https://patchwork.freedesktop.org/series/123813/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




✗ Fi.CI.CHECKPATCH: warning for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev6)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev6)
URL   : https://patchwork.freedesktop.org/series/123813/
State : warning

== Summary ==

Error: dim checkpatch failed
75a9987b2382 drm/i915: Enable Wa_16019325821
c54ac284eef0 drm/i915/guc: Add support for w/a KLVs
-:105: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#105: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:829:
+   GEM_BUG_ON(iosys_map_is_null(>ads_map));

total: 0 errors, 1 warnings, 0 checks, 159 lines checked
7106400f52a2 drm/i915/guc: Enable Wa_14019159160
-:101: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#101: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:830:
+   GEM_BUG_ON(remain < size);

total: 0 errors, 1 warnings, 0 checks, 99 lines checked




Re: [PATCH] drm/i915: move interrupt save/restore into vblank section helpers

2024-01-17 Thread kernel test robot
Hi Luca,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip linus/master next-20240117]
[cannot apply to drm-intel/for-linux-next-fixes v6.7]
[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/Luca-Coelho/drm-i915-move-interrupt-save-restore-into-vblank-section-helpers/20240117-174910
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:
https://lore.kernel.org/r/20240117094613.1401573-1-luciano.coelho%40intel.com
patch subject: [PATCH] drm/i915: move interrupt save/restore into vblank 
section helpers
config: i386-randconfig-011-20240117 
(https://download.01.org/0day-ci/archive/20240118/202401180456.xkw1s0m1-...@intel.com/config)
compiler: ClangBuiltLinux clang version 17.0.6 
(https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240118/202401180456.xkw1s0m1-...@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 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202401180456.xkw1s0m1-...@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/display/intel_vblank.c:282:17: error: use of undeclared 
>> identifier 'irqflags'
 282 | local_irq_save(irqflags);
 |^
>> drivers/gpu/drm/i915/display/intel_vblank.c:282:17: error: use of undeclared 
>> identifier 'irqflags'
>> drivers/gpu/drm/i915/display/intel_vblank.c:282:17: error: use of undeclared 
>> identifier 'irqflags'
>> drivers/gpu/drm/i915/display/intel_vblank.c:282:17: error: use of undeclared 
>> identifier 'irqflags'
   drivers/gpu/drm/i915/display/intel_vblank.c:294:20: error: use of undeclared 
identifier 'irqflags'
 294 | local_irq_restore(irqflags);
 |   ^
   drivers/gpu/drm/i915/display/intel_vblank.c:294:20: error: use of undeclared 
identifier 'irqflags'
   drivers/gpu/drm/i915/display/intel_vblank.c:294:20: error: use of undeclared 
identifier 'irqflags'
   drivers/gpu/drm/i915/display/intel_vblank.c:294:20: error: use of undeclared 
identifier 'irqflags'
   drivers/gpu/drm/i915/display/intel_vblank.c:309:16: warning: unused variable 
'irqflags' [-Wunused-variable]
 309 | unsigned long irqflags;
 |   ^~~~
   drivers/gpu/drm/i915/display/intel_vblank.c:441:16: warning: unused variable 
'irqflags' [-Wunused-variable]
 441 | unsigned long irqflags;
 |   ^~~~
   2 warnings and 8 errors generated.


vim +/irqflags +282 drivers/gpu/drm/i915/display/intel_vblank.c

   267  
   268  /*
   269   * These functions help enter and exit vblank critical sections.  When
   270   * entering, they disable interrupts and, for i915, acquire the
   271   * uncore's spinlock.  Conversely, when exiting, they release the
   272   * spinlock and restore the interrupts state.
   273   *
   274   * This lock in i915 is needed because some old platforms (at least
   275   * IVB and possibly HSW as well), which are not supported in Xe, need
   276   * all register accesses to the same cacheline to be serialized,
   277   * otherwise they may hang.
   278   */
   279  static void intel_vblank_section_enter(struct drm_i915_private *i915)
   280  __acquires(i915->uncore.lock)
   281  {
 > 282  local_irq_save(irqflags);
   283  #ifdef I915
   284  spin_lock(>uncore.lock);
   285  #endif
   286  }
   287  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


✗ Fi.CI.BAT: failure for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev5)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev5)
URL   : https://patchwork.freedesktop.org/series/123813/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14133 -> Patchwork_123813v5


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_123813v5 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_123813v5, please notify your bug team 
(i915-ci-in...@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/index.html

Participating hosts (36 -> 36)
--

  Additional (2): bat-dg2-9 fi-bsw-n3050 
  Missing(2): bat-dg2-8 fi-snb-2520m 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_123813v5:

### IGT changes ###

 Possible regressions 

  * igt@i915_suspend@basic-s2idle-without-i915:
- fi-rkl-11600:   [PASS][1] -> [FAIL][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14133/fi-rkl-11600/igt@i915_susp...@basic-s2idle-without-i915.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/fi-rkl-11600/igt@i915_susp...@basic-s2idle-without-i915.html

  
Known issues


  Here are the changes found in Patchwork_123813v5 that come from known issues:

### CI changes ###

 Possible fixes 

  * boot:
- bat-jsl-1:  [FAIL][3] ([i915#8293]) -> [PASS][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14133/bat-jsl-1/boot.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-jsl-1/boot.html

  

### IGT changes ###

 Issues hit 

  * igt@debugfs_test@basic-hwmon:
- bat-jsl-1:  NOTRUN -> [SKIP][5] ([i915#9318])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-jsl-1/igt@debugfs_t...@basic-hwmon.html

  * igt@gem_huc_copy@huc-copy:
- bat-jsl-1:  NOTRUN -> [SKIP][6] ([i915#2190])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-jsl-1/igt@gem_huc_c...@huc-copy.html

  * igt@gem_lmem_swapping@random-engines:
- fi-bsw-n3050:   NOTRUN -> [SKIP][7] ([fdo#109271]) +15 other tests 
skip
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/fi-bsw-n3050/igt@gem_lmem_swapp...@random-engines.html

  * igt@gem_lmem_swapping@verify-random:
- bat-jsl-1:  NOTRUN -> [SKIP][8] ([i915#4613]) +3 other tests skip
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-jsl-1/igt@gem_lmem_swapp...@verify-random.html

  * igt@gem_mmap@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][9] ([i915#4083])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-dg2-9/igt@gem_m...@basic.html

  * igt@gem_mmap_gtt@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][10] ([i915#4077]) +2 other tests skip
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-dg2-9/igt@gem_mmap_...@basic.html

  * igt@gem_render_tiled_blits@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][11] ([i915#4079]) +1 other test skip
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-dg2-9/igt@gem_render_tiled_bl...@basic.html

  * igt@i915_pm_rps@basic-api:
- bat-dg2-9:  NOTRUN -> [SKIP][12] ([i915#6621])
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-dg2-9/igt@i915_pm_...@basic-api.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-rpls-2: [PASS][13] -> [ABORT][14] ([i915#7978])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14133/bat-rpls-2/igt@i915_susp...@basic-s3-without-i915.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-rpls-2/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][15] ([i915#5190])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-dg2-9/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][16] ([i915#4215] / [i915#5190])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-dg2-9/igt@kms_addfb_ba...@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@framebuffer-vs-set-tiling:
- bat-dg2-9:  NOTRUN -> [SKIP][17] ([i915#4212]) +7 other tests skip
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123813v5/bat-dg2-9/igt@kms_addfb_ba...@framebuffer-vs-set-tiling.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][18] ([i915#4103] / [i915#4213]) +1 
other test skip
   [18]: 

✗ Fi.CI.SPARSE: warning for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev5)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev5)
URL   : https://patchwork.freedesktop.org/series/123813/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




✗ Fi.CI.CHECKPATCH: warning for Enable Wa_14019159160 and Wa_16019325821 for MTL (rev5)

2024-01-17 Thread Patchwork
== Series Details ==

Series: Enable Wa_14019159160 and Wa_16019325821 for MTL (rev5)
URL   : https://patchwork.freedesktop.org/series/123813/
State : warning

== Summary ==

Error: dim checkpatch failed
1290ab959552 drm/i915: Enable Wa_16019325821
972bfeee9fdc drm/i915/guc: Add support for w/a KLVs
-:105: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#105: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:829:
+   GEM_BUG_ON(iosys_map_is_null(>ads_map));

total: 0 errors, 1 warnings, 0 checks, 159 lines checked
89a33a9f082f drm/i915/guc: Enable Wa_14019159160
-:101: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely 
unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of 
BUG() or variants
#101: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:830:
+   GEM_BUG_ON(remain < size);

total: 0 errors, 1 warnings, 0 checks, 99 lines checked




Re: [i-g-t 1/1] drm/i915/display: Dump display parameters to i915_display_capabilities

2024-01-17 Thread Jani Nikula
On Wed, 17 Jan 2024, Bhanuprakash Modem  wrote:
> Include Display parameters in i915_display_capabilities debugfs.

Maybe remove them from i915_capabilities? What's the point in having
them in two places? (Or three, if you count the actual debugfs interface
to the params!)

>
> Cc: Jouni Högander 
> Signed-off-by: Bhanuprakash Modem 
> ---
>  drivers/gpu/drm/i915/display/intel_display_debugfs.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 6f2d13c8ccf7..1eb296b5c38a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -625,6 +625,10 @@ static int i915_display_capabilities(struct seq_file *m, 
> void *unused)
>   struct drm_i915_private *i915 = node_to_i915(m->private);
>   struct drm_printer p = drm_seq_file_printer(m);
>  
> + kernel_param_lock(THIS_MODULE);
> + intel_display_params_dump(i915, );
> + kernel_param_unlock(THIS_MODULE);

The kernel_param_lock/unlock are just cargo cult from the time these
were actually module parameters. They should be removed from
i915_capabilities as well.

BR,
Jani.

> +
>   intel_display_device_info_print(DISPLAY_INFO(i915),
>   DISPLAY_RUNTIME_INFO(i915), );

-- 
Jani Nikula, Intel


Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

2024-01-17 Thread Jani Nikula
On Wed, 17 Jan 2024, "Sharma, Swati2"  wrote:
> Hi Ville,
>
> On 25-Oct-23 6:29 PM, Ville Syrjälä wrote:
>> On Wed, Oct 18, 2023 at 04:24:00PM +0300, Stanislav Lisovskiy wrote:
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index 4f6835a7578e..6a9148232a9c 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -1153,7 +1153,11 @@ bool intel_dp_need_bigjoiner(struct intel_dp 
>>> *intel_dp,
>>> if (!intel_dp_can_bigjoiner(intel_dp))
>>> return false;
>>>   
>>> -   return clock > i915->max_dotclk_freq || hdisplay > 5120;
>>> +   if (intel_dp->force_bigjoiner_enable)
>>> +   drm_dbg_kms(>drm, "Forcing bigjoiner mode\n");
>> 
>> That's going to cause excessive dmesg spam.
>
> Then how from dmesg we will get to know, big joiner was forced? 
> Shouldn't we have atleast one debug print to know this?

Yeah. But this gets called for every single display mode via
intel_dp_mode_valid().

BR,
Jani.


-- 
Jani Nikula, Intel


Re: [PATCH v3 05/16] drm/i915: Disable the "binder"

2024-01-17 Thread Nirmoy Das



On 1/17/2024 3:13 PM, Michał Winiarski wrote:

On Tue, Jan 16, 2024 at 09:56:25AM +0200, Ville Syrjala wrote:

From: Ville Syrjälä 

Now that the GGTT PTE updates go straight to GSMBASE (bypassing
GTTMMADR) there should be no more risk of system hangs? So the
"binder" (ie. update the PTEs via MI_UPDATE_GTT) is no longer
necessary, disable it.

My main worry with the MI_UPDATE_GTT are:
- only used on this one platform so very limited testing coverage
- async so more opprtunities to screw things up
- what happens if the engine hangs while we're waiting for MI_UPDATE_GTT
   to finish?
- requires working command submission, so even getting a working
   display now depends on a lot more extra components working correctly

TODO: MI_UPDATE_GTT might be interesting as an optimization
though, so perhaps someone should look into always using it
(assuming the GPU is alive and well)?

v2: Keep using MI_UPDATE_GTT on VM guests

Cc: Paz Zcharya 
Cc: Nirmoy Das 
Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/gt/intel_gtt.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 86f73fe558ca..e83dabc56a14 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -24,7 +24,8 @@
  bool i915_ggtt_require_binder(struct drm_i915_private *i915)
  {
/* Wa_13010847436 & Wa_14019519902 */
-   return MEDIA_VER_FULL(i915) == IP_VER(13, 0);
+   return i915_run_as_guest() &&
+   MEDIA_VER_FULL(i915) == IP_VER(13, 0);

Note that i915_run_as_guest() is not the most reliable way to decide
whether to use MI_UPDATE_GTT or straight to GSMBASE, as it requires the
hypervisor to "opt-in" and set the X86_FEATURE_HYPERVISOR.
If it's not set - the driver will go into GSMBASE, which is not mapped
inside the guest.
Does the system firmware advertise whether GSMBASE is "open" or "closed"
to CPU access in any way?


Had a chat with David from IVE team, David suggested to read 0x138914 to 
determine that.  "GOP needs to qualify the WA by reading GFX MMIO offset 
0x138914 and verify the value there is 0x1." -> as per the HSD-22018444074




Regards,

Nirmoy



-Michał


  }
  
  static bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)

--
2.41.0



Re: [PATCH] drm/i915: move interrupt save/restore into vblank section helpers

2024-01-17 Thread kernel test robot
Hi Luca,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip linus/master next-20240117]
[cannot apply to drm-intel/for-linux-next-fixes v6.7]
[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/Luca-Coelho/drm-i915-move-interrupt-save-restore-into-vblank-section-helpers/20240117-174910
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:
https://lore.kernel.org/r/20240117094613.1401573-1-luciano.coelho%40intel.com
patch subject: [PATCH] drm/i915: move interrupt save/restore into vblank 
section helpers
config: i386-buildonly-randconfig-004-20240117 
(https://download.01.org/0day-ci/archive/20240118/202401180149.bsppqd72-...@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240118/202401180149.bsppqd72-...@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 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202401180149.bsppqd72-...@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/bitops.h:7:0,
from include/linux/kernel.h:23,
from arch/x86/include/asm/percpu.h:27,
from arch/x86/include/asm/current.h:10,
from include/linux/mutex.h:14,
from include/linux/notifier.h:14,
from include/linux/pm_qos.h:16,
from drivers/gpu/drm/i915/i915_drv.h:35,
from drivers/gpu/drm/i915/display/intel_vblank.c:6:
   drivers/gpu/drm/i915/display/intel_vblank.c: In function 
'intel_vblank_section_enter':
>> drivers/gpu/drm/i915/display/intel_vblank.c:282:17: error: 'irqflags' 
>> undeclared (first use in this function); did you mean 'mf_flags'?
 local_irq_save(irqflags);
^
   include/linux/typecheck.h:11:9: note: in definition of macro 'typecheck'
 typeof(x) __dummy2; \
^
   include/linux/irqflags.h:245:36: note: in expansion of macro 
'raw_local_irq_save'
#define local_irq_save(flags) do { raw_local_irq_save(flags); } while (0)
   ^~
   drivers/gpu/drm/i915/display/intel_vblank.c:282:2: note: in expansion of 
macro 'local_irq_save'
 local_irq_save(irqflags);
 ^~
   drivers/gpu/drm/i915/display/intel_vblank.c:282:17: note: each undeclared 
identifier is reported only once for each function it appears in
 local_irq_save(irqflags);
^
   include/linux/typecheck.h:11:9: note: in definition of macro 'typecheck'
 typeof(x) __dummy2; \
^
   include/linux/irqflags.h:245:36: note: in expansion of macro 
'raw_local_irq_save'
#define local_irq_save(flags) do { raw_local_irq_save(flags); } while (0)
   ^~
   drivers/gpu/drm/i915/display/intel_vblank.c:282:2: note: in expansion of 
macro 'local_irq_save'
 local_irq_save(irqflags);
 ^~
   include/linux/typecheck.h:12:18: warning: comparison of distinct pointer 
types lacks a cast
 (void)(&__dummy == &__dummy2); \
 ^
   include/linux/irqflags.h:178:3: note: in expansion of macro 'typecheck'
  typecheck(unsigned long, flags); \
  ^
   include/linux/irqflags.h:245:36: note: in expansion of macro 
'raw_local_irq_save'
#define local_irq_save(flags) do { raw_local_irq_save(flags); } while (0)
   ^~
   drivers/gpu/drm/i915/display/intel_vblank.c:282:2: note: in expansion of 
macro 'local_irq_save'
 local_irq_save(irqflags);
 ^~
   drivers/gpu/drm/i915/display/intel_vblank.c: In function 
'intel_vblank_section_exit':
   drivers/gpu/drm/i915/display/intel_vblank.c:294:20: error: 'irqflags' 
undeclared (first use in this function); did you mean 'mf_flags'?
 local_irq_restore(irqflags);
   ^
   include/linux/typecheck.h:11:9: note: in definition of macro 'typecheck'
 typeof(x) __dummy2; \
^
   include/linux/irqflags.h:246:39: note: in expansion of macro 
'raw_local_irq_restore'
#define local_irq_restore(flags) do { raw_local_irq_restore(flags); } while 
(0)
  ^
   drivers/gpu/drm/i915/display/intel_vblank.c:294:2: note: in expansion of 
macro 'local_irq_restore'
 local_irq_restore(irqflags);
 ^
   include/linux/typecheck.h:12:18: warning: comparison of dis

✗ Fi.CI.BAT: failure for QGV/SAGV related fixes (rev4)

2024-01-17 Thread Patchwork
== Series Details ==

Series: QGV/SAGV related fixes (rev4)
URL   : https://patchwork.freedesktop.org/series/126962/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14132 -> Patchwork_126962v4


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_126962v4 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_126962v4, please notify your bug team 
(i915-ci-in...@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/index.html

Participating hosts (38 -> 37)
--

  Additional (1): bat-mtlp-8 
  Missing(2): bat-dg2-9 fi-snb-2520m 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_126962v4:

### IGT changes ###

 Possible regressions 

  * igt@i915_selftest@live@hangcheck:
- bat-rpls-2: [PASS][1] -> [ABORT][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14132/bat-rpls-2/igt@i915_selftest@l...@hangcheck.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-rpls-2/igt@i915_selftest@l...@hangcheck.html

  
 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@gem_contexts:
- {bat-adls-6}:   [PASS][3] -> [INCOMPLETE][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14132/bat-adls-6/igt@i915_selftest@live@gem_contexts.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-adls-6/igt@i915_selftest@live@gem_contexts.html

  
Known issues


  Here are the changes found in Patchwork_126962v4 that come from known issues:

### CI changes ###

 Issues hit 

  * boot:
- bat-jsl-1:  [PASS][5] -> [FAIL][6] ([i915#8293])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14132/bat-jsl-1/boot.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-jsl-1/boot.html

  

### IGT changes ###

 Issues hit 

  * igt@debugfs_test@basic-hwmon:
- bat-mtlp-8: NOTRUN -> [SKIP][7] ([i915#9318])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html

  * igt@gem_lmem_swapping@verify-random:
- bat-mtlp-8: NOTRUN -> [SKIP][8] ([i915#4613]) +3 other tests skip
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html

  * igt@gem_mmap@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#4083])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@gem_m...@basic.html

  * igt@gem_mmap_gtt@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#4077]) +2 other tests skip
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@gem_mmap_...@basic.html

  * igt@gem_render_tiled_blits@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#4079]) +1 other test skip
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html

  * igt@i915_pm_rps@basic-api:
- bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#6621])
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@i915_pm_...@basic-api.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#6645])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][14] ([i915#5190])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#4212]) +8 other tests skip
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][16] ([i915#4213]) +1 other test skip
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
- bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#3555] / [i915#3840] / 
[i915#9159])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v4/bat-mtlp-8/igt@kms_...@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
- bat-mtlp-8: NOTRUN -> [SKIP][18] ([fdo#109285])
   [18]: 

[i-g-t 1/1] drm/i915/display: Dump display parameters to i915_display_capabilities

2024-01-17 Thread Bhanuprakash Modem
Include Display parameters in i915_display_capabilities debugfs.

Cc: Jouni Högander 
Signed-off-by: Bhanuprakash Modem 
---
 drivers/gpu/drm/i915/display/intel_display_debugfs.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 6f2d13c8ccf7..1eb296b5c38a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -625,6 +625,10 @@ static int i915_display_capabilities(struct seq_file *m, 
void *unused)
struct drm_i915_private *i915 = node_to_i915(m->private);
struct drm_printer p = drm_seq_file_printer(m);
 
+   kernel_param_lock(THIS_MODULE);
+   intel_display_params_dump(i915, );
+   kernel_param_unlock(THIS_MODULE);
+
intel_display_device_info_print(DISPLAY_INFO(i915),
DISPLAY_RUNTIME_INFO(i915), );
 
-- 
2.40.0



✗ Fi.CI.SPARSE: warning for QGV/SAGV related fixes (rev4)

2024-01-17 Thread Patchwork
== Series Details ==

Series: QGV/SAGV related fixes (rev4)
URL   : https://patchwork.freedesktop.org/series/126962/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:173:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:175:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:179:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:181:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:181:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:185:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:187:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:191:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:194:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:194:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:236:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:238:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:66:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:92:1: warning: unreplaced symbol 'return'
+./drivers/gpu/drm/i915/intel_uncore.h:346:1: warning: trying to copy 
expression type 31
+./include/asm-generic/bitops/generic-non-atomic.h:100:17: warning: unreplaced 
symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:100:23: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:100:9: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:105:1: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:107:9: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:108:9: warning: unreplaced 
symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:109:9: warning: unreplaced 
symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:10: warning: unreplaced 
symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:111:14: warning: unreplaced 
symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:20: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:17: warning: unreplaced 
symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:112:23: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:9: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:121:1: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:128:9: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:166:1: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:168:9: warning: unreplaced 
symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:169:9: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:170:9: warning: unreplaced 
symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:19: warning: unreplaced 
symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:25: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:172:9: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:28:1: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:30:9: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:31:9: warning: unreplaced 
symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:10: warning: unreplaced 
symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:16: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:37:1: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:39:9: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:40:9: warning: unreplaced 
symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:10: warning: unreplaced 
symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:16: warning: unreplaced 
symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:55:1: warning: unreplaced 
symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:57:9: warning: 

✗ Fi.CI.CHECKPATCH: warning for QGV/SAGV related fixes (rev4)

2024-01-17 Thread Patchwork
== Series Details ==

Series: QGV/SAGV related fixes (rev4)
URL   : https://patchwork.freedesktop.org/series/126962/
State : warning

== Summary ==

Error: dim checkpatch failed
9d8849950f20 drm/i915: Add meaningful traces for QGV point info error handling
27f440447eae drm/i915: Extract code required to calculate max qgv/psf gv point
b8d3de777d4d drm/i915: Disable SAGV on bw init, to force QGV point recalculation
-:9: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible 
unwrapped commit description?)
#9: 
(i.e all points allowed), however in reality we might get them all restricted,

-:54: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#54: FILE: drivers/gpu/drm/i915/display/intel_bw.c:873:
+   drm_dbg_kms(>drm, "Forcing SAGV disable: leaving QGV point %d\n",
+   max_bw_qgv_point);

total: 0 errors, 1 warnings, 1 checks, 42 lines checked




Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

2024-01-17 Thread Sharma, Swati2

Hi Ville,

On 25-Oct-23 6:29 PM, Ville Syrjälä wrote:

On Wed, Oct 18, 2023 at 04:24:00PM +0300, Stanislav Lisovskiy wrote:

For validation purposes, it might be useful to be able to
force Bigjoiner mode, even if current dotclock/resolution
do not require that.
Lets add such to option to debugfs.

v2: - Apparently intel_dp_need_bigjoiner can't be used, when
   debugfs entry is created so lets just check manually
   the DISPLAY_VER.

v3: - Switch to intel_connector from drm_connector(Jani Nikula)
 - Remove redundant modeset lock(Jani Nikula)
 - Use kstrtobool_from_user for boolean value(Jani Nikula)

v4: - Apply the changes to proper function(Jani Nikula)

Signed-off-by: Stanislav Lisovskiy 
---
  .../drm/i915/display/intel_display_debugfs.c  | 66 +++
  .../drm/i915/display/intel_display_types.h|  2 +
  drivers/gpu/drm/i915/display/intel_dp.c   |  6 +-
  3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index fbe75d47a165..9b810c6f96ea 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1398,6 +1398,30 @@ out: 
drm_modeset_unlock(>mode_config.connection_mutex);
return ret;
  }
  
+static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)

+{
+   struct intel_connector *connector = to_intel_connector(m->private);
+   struct drm_crtc *crtc;
+   struct intel_encoder *encoder = intel_attached_encoder(connector);
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+   int ret = 0;
+
+   if (!encoder)
+   return -ENODEV;
+
+   crtc = connector->base.state->crtc;
+   if (connector->base.status != connector_status_connected || !crtc) {
+   ret = -ENODEV;


Why do we need this extra complication?


+   goto out;
+   }
+
+   seq_printf(m, "Bigjoiner enable: %d\n", 
intel_dp->force_bigjoiner_enable);
+
+out:
+
+   return ret;
+}
+
  static ssize_t i915_dsc_output_format_write(struct file *file,
const char __user *ubuf,
size_t len, loff_t *offp)
@@ -1419,12 +1443,39 @@ static ssize_t i915_dsc_output_format_write(struct file 
*file,
return len;
  }
  
+static ssize_t i915_bigjoiner_enable_fops_write(struct file *file,

+   const char __user *ubuf,
+   size_t len, loff_t *offp)
+{
+   struct seq_file *m = file->private_data;
+   struct intel_connector *connector = m->private;
+   struct intel_encoder *encoder = intel_attached_encoder(connector);
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+   bool bigjoiner_en = 0;
+   int ret;
+
+   ret = kstrtobool_from_user(ubuf, len, _en);
+   if (ret < 0)
+   return ret;
+
+   intel_dp->force_bigjoiner_enable = bigjoiner_en;
+   *offp += len;
+
+   return len;
+}
+
  static int i915_dsc_output_format_open(struct inode *inode,
   struct file *file)
  {
return single_open(file, i915_dsc_output_format_show, inode->i_private);
  }
  
+static int i915_bigjoiner_enable_open(struct inode *inode,

+ struct file *file)
+{
+   return single_open(file, i915_bigjoiner_enable_show, inode->i_private);
+}
+
  static const struct file_operations i915_dsc_output_format_fops = {
.owner = THIS_MODULE,
.open = i915_dsc_output_format_open,
@@ -1434,6 +1485,15 @@ static const struct file_operations 
i915_dsc_output_format_fops = {
.write = i915_dsc_output_format_write
  };
  
+static const struct file_operations i915_bigjoiner_enable_fops = {

+   .owner = THIS_MODULE,
+   .open = i915_bigjoiner_enable_open,
+   .read = seq_read,
+   .llseek = seq_lseek,
+   .release = single_release,
+   .write = i915_bigjoiner_enable_fops_write
+};
+
  /*
   * Returns the Current CRTC's bpc.
   * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
@@ -1513,6 +1573,12 @@ void intel_connector_debugfs_add(struct intel_connector 
*intel_connector)
connector, _dsc_output_format_fops);
}
  
+	if (DISPLAY_VER(dev_priv) >= 11 &&

+   intel_connector->base.connector_type == 
DRM_MODE_CONNECTOR_DisplayPort) {


eDP missing


+   debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
+   _connector->base, 
_bigjoiner_enable_fops);
+   }
+
if (connector->connector_type == DRM_MODE_CONNECTOR_DSI ||
connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
diff --git 

✗ Fi.CI.BAT: failure for drm/i915: (stolen) memory region related fixes (rev7)

2024-01-17 Thread Patchwork
== Series Details ==

Series: drm/i915: (stolen) memory region related fixes (rev7)
URL   : https://patchwork.freedesktop.org/series/127721/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14132 -> Patchwork_127721v7


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_127721v7 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_127721v7, please notify your bug team 
(i915-ci-in...@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/index.html

Participating hosts (38 -> 37)
--

  Additional (1): bat-mtlp-8 
  Missing(2): fi-snb-2520m fi-pnv-d510 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_127721v7:

### IGT changes ###

 Possible regressions 

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1:
- bat-dg2-8:  [PASS][1] -> [FAIL][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14132/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-seque...@pipe-c-dp-1.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-seque...@pipe-c-dp-1.html

  
Known issues


  Here are the changes found in Patchwork_127721v7 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@debugfs_test@basic-hwmon:
- bat-mtlp-8: NOTRUN -> [SKIP][3] ([i915#9318])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html

  * igt@gem_exec_suspend@basic-s0@smem:
- bat-jsl-3:  [PASS][4] -> [INCOMPLETE][5] ([i915#9883])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14132/bat-jsl-3/igt@gem_exec_suspend@basic...@smem.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-jsl-3/igt@gem_exec_suspend@basic...@smem.html

  * igt@gem_lmem_swapping@verify-random:
- bat-mtlp-8: NOTRUN -> [SKIP][6] ([i915#4613]) +3 other tests skip
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html

  * igt@gem_mmap@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][7] ([i915#4083])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@gem_m...@basic.html

  * igt@gem_mmap_gtt@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][8] ([i915#4077]) +2 other tests skip
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@gem_mmap_...@basic.html

  * igt@gem_render_tiled_blits@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#4079]) +1 other test skip
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html

  * igt@i915_pm_rps@basic-api:
- bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#6621])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@i915_pm_...@basic-api.html

  * igt@i915_suspend@basic-s2idle-without-i915:
- fi-bsw-nick:[PASS][11] -> [DMESG-WARN][12] ([i915#1982])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14132/fi-bsw-nick/igt@i915_susp...@basic-s2idle-without-i915.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/fi-bsw-nick/igt@i915_susp...@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-jsl-3:  [PASS][13] -> [FAIL][14] ([i915#10031])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14132/bat-jsl-3/igt@i915_susp...@basic-s3-without-i915.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-jsl-3/igt@i915_susp...@basic-s3-without-i915.html
- bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#6645])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][16] ([i915#5190])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#4212]) +8 other tests skip
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127721v7/bat-mtlp-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][18] ([i915#4213]) +1 other test skip
   [18]: 

✗ Fi.CI.SPARSE: warning for drm/i915: (stolen) memory region related fixes (rev7)

2024-01-17 Thread Patchwork
== Series Details ==

Series: drm/i915: (stolen) memory region related fixes (rev7)
URL   : https://patchwork.freedesktop.org/series/127721/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




✗ Fi.CI.CHECKPATCH: warning for drm/i915: (stolen) memory region related fixes (rev7)

2024-01-17 Thread Patchwork
== Series Details ==

Series: drm/i915: (stolen) memory region related fixes (rev7)
URL   : https://patchwork.freedesktop.org/series/127721/
State : warning

== Summary ==

Error: dim checkpatch failed
420bd052296b drm/i915: Use struct resource for memory region IO as well
-:388: WARNING:LONG_LINE: line length of 105 exceeds 100 columns
#388: FILE: drivers/gpu/drm/i915/intel_region_ttm.c:227:
+   if (WARN_ON(overflows_type(resource_size(>io) >> 
PAGE_SHIFT, place.lpfn))) {

total: 0 errors, 1 warnings, 0 checks, 281 lines checked
36939988182b drm/i915: Print memory region info during probe
b068f5dd348b drm/i915: Remove ad-hoc lmem/stolen debugs
5c3f5c028395 drm/i915: Bypass LMEMBAR/GTTMMADR for MTL stolen memory access
9d4356c8b4f9 drm/i915: Disable the "binder"
eeeddbd1c219 drm/i915: Rename the DSM/GSM registers
0f97740ee08d drm/i915: Fix PTE decode during initial plane readout
3e1f008c0575 drm/i915: Fix region start during initial plane readout
376b52c85c46 drm/i915: Fix MTL initial plane readout
9d7013c0a8a2 drm/i915: s/phys_base/dma_addr/
ad263486e9a0 drm/i915: Split the smem and lmem plane readout apart
3ae104619d2c drm/i915: Simplify intel_initial_plane_config() calling convention
4fe3e6f3e4ec drm/i915/fbdev: Fix smem_start for LMEMBAR stolen objects
e1f6060ee679 drm/i915: Tweak BIOS fb reuse check
0e59c24c2a0a drm/i915: Try to relocate the BIOS fb to the start of ggtt
-:104: CHECK:LINE_SPACING: Please use a blank line after 
function/struct/union/enum declarations
#104: FILE: drivers/gpu/drm/i915/display/i9xx_plane.h:51:
 }
+static inline bool i9xx_fixup_initial_plane_config(struct intel_crtc *crtc,

-:105: WARNING:LONG_LINE: line length of 105 exceeds 100 columns
#105: FILE: drivers/gpu/drm/i915/display/i9xx_plane.h:52:
+  const struct 
intel_initial_plane_config *plane_config)

total: 0 errors, 1 warnings, 1 checks, 229 lines checked
d5cdc16e4e91 drm/i915: Annotate more of the BIOS fb takeover failure paths




[PATCH 2/3] drm/i915: Extract code required to calculate max qgv/psf gv point

2024-01-17 Thread Stanislav Lisovskiy
We need that in order to force disable SAGV in next patch.
Also it is beneficial to separate that code, as in majority cases,
when SAGV is enabled, we don't even need those calculations.
Also we probably need to determine max PSF GV point as well, however
currently we don't do that when we disable SAGV, which might be
actually causing some issues in that case.

v2: - Introduce helper adl_qgv_bw(counterpart to adl_psf_bw)
  (Ville Syrjälä)
- Don't restrict psf gv points for SAGV disable case
  (Ville Syrjälä)

Signed-off-by: Stanislav Lisovskiy 
---
 drivers/gpu/drm/i915/display/intel_bw.c | 81 -
 1 file changed, 53 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
b/drivers/gpu/drm/i915/display/intel_bw.c
index 77886cc21211..7baa1c13eccd 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -652,15 +652,31 @@ static unsigned int tgl_max_bw_index(struct 
drm_i915_private *dev_priv,
return 0;
 }
 
-static unsigned int adl_psf_bw(struct drm_i915_private *dev_priv,
+static unsigned int adl_psf_bw(struct drm_i915_private *i915,
   int psf_gv_point)
 {
const struct intel_bw_info *bi =
-   _priv->display.bw.max[0];
+   >display.bw.max[0];
 
return bi->psf_bw[psf_gv_point];
 }
 
+static unsigned int adl_qgv_bw(struct drm_i915_private *i915,
+  int qgv_point, int num_active_planes)
+{
+   unsigned int idx;
+
+   if (DISPLAY_VER(i915) > 11)
+   idx = tgl_max_bw_index(i915, num_active_planes, qgv_point);
+   else
+   idx = icl_max_bw_index(i915, num_active_planes, qgv_point);
+
+   if (idx >= ARRAY_SIZE(i915->display.bw.max))
+   return 0;
+
+   return i915->display.bw.max[idx].deratedbw[qgv_point];
+}
+
 void intel_bw_init_hw(struct drm_i915_private *dev_priv)
 {
if (!HAS_DISPLAY(dev_priv))
@@ -806,6 +822,36 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state)
return to_intel_bw_state(bw_state);
 }
 
+static unsigned int icl_max_bw_qgv_point(struct drm_i915_private *i915,
+int num_active_planes)
+{
+   unsigned int max_bw_point = 0;
+   unsigned int max_bw = 0;
+   unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
+   int i;
+
+   for (i = 0; i < num_qgv_points; i++) {
+   unsigned int max_data_rate;
+
+   max_data_rate = adl_qgv_bw(i915, i, num_active_planes);
+
+   /*
+* We need to know which qgv point gives us
+* maximum bandwidth in order to disable SAGV
+* if we find that we exceed SAGV block time
+* with watermarks. By that moment we already
+* have those, as it is calculated earlier in
+* intel_atomic_check,
+*/
+   if (max_data_rate > max_bw) {
+   max_bw_point = i;
+   max_bw = max_data_rate;
+   }
+   }
+
+   return max_bw_point;
+}
+
 static int mtl_find_qgv_points(struct drm_i915_private *i915,
   unsigned int data_rate,
   unsigned int num_active_planes,
@@ -883,8 +929,6 @@ static int icl_find_qgv_points(struct drm_i915_private 
*i915,
   const struct intel_bw_state *old_bw_state,
   struct intel_bw_state *new_bw_state)
 {
-   unsigned int max_bw_point = 0;
-   unsigned int max_bw = 0;
unsigned int num_psf_gv_points = 
i915->display.bw.max[0].num_psf_gv_points;
unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
u16 psf_points = 0;
@@ -897,31 +941,10 @@ static int icl_find_qgv_points(struct drm_i915_private 
*i915,
return ret;
 
for (i = 0; i < num_qgv_points; i++) {
-   unsigned int idx;
unsigned int max_data_rate;
 
-   if (DISPLAY_VER(i915) >= 12)
-   idx = tgl_max_bw_index(i915, num_active_planes, i);
-   else
-   idx = icl_max_bw_index(i915, num_active_planes, i);
-
-   if (idx >= ARRAY_SIZE(i915->display.bw.max))
-   continue;
-
-   max_data_rate = i915->display.bw.max[idx].deratedbw[i];
+   max_data_rate = adl_qgv_bw(i915, i, num_active_planes);
 
-   /*
-* We need to know which qgv point gives us
-* maximum bandwidth in order to disable SAGV
-* if we find that we exceed SAGV block time
-* with watermarks. By that moment we already
-* have those, as it is calculated earlier in
-* intel_atomic_check,
-*/
-   if (max_data_rate 

[PATCH 3/3] drm/i915: Disable SAGV on bw init, to force QGV point recalculation

2024-01-17 Thread Stanislav Lisovskiy
Problem is that on some platforms, we do get QGV point mask in wrong
state on boot. However driver assumes it is set to 0
(i.e all points allowed), however in reality we might get them all restricted,
causing issues.
Lets disable SAGV initially to force proper QGV point state.
If more QGV points are available, driver will recalculate and update
those then after next commit.

v2: - Added trace to see which QGV/PSF GV point is used when SAGV is
  disabled.
v3: - Move force disable function to intel_bw_init in order to initialize
  bw state as well, so that hw/sw are immediately in sync after init.
v4: - Don't try sending PCode request, seems like it is not possible at
  intel_bw_init, however assigning bw->state to be restricted as if
  SAGV is off, still forces driveer to send PCode request anyway on
  next modeset, so the solution still works.
  However we still need to address the case, when no display is connected,
  which anyway requires much more changes.

Signed-off-by: Stanislav Lisovskiy 
---
 drivers/gpu/drm/i915/display/intel_bw.c | 24 
 drivers/gpu/drm/i915/display/intel_bw.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
b/drivers/gpu/drm/i915/display/intel_bw.c
index 7baa1c13eccd..36a6304207ba 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -852,6 +852,27 @@ static unsigned int icl_max_bw_qgv_point(struct 
drm_i915_private *i915,
return max_bw_point;
 }
 
+void icl_force_disable_sagv(struct drm_i915_private *i915, struct 
intel_bw_state *bw_state)
+{
+   unsigned int max_bw_qgv_point = icl_max_bw_qgv_point(i915, 0);
+   unsigned int qgv_points;
+   unsigned int psf_points;
+
+   qgv_points = BIT(max_bw_qgv_point);
+
+   /*
+* We don't restrict PSF GV points, when disabling SAGV
+*/
+   psf_points = 0;
+
+   bw_state->qgv_points_mask = ~(ICL_PCODE_REQ_QGV_PT(qgv_points) |
+ ADLS_PCODE_REQ_PSF_PT(psf_points)) &
+ icl_qgv_points_mask(i915);
+
+   drm_dbg_kms(>drm, "Forcing SAGV disable: leaving QGV point %d\n",
+   max_bw_qgv_point);
+}
+
 static int mtl_find_qgv_points(struct drm_i915_private *i915,
   unsigned int data_rate,
   unsigned int num_active_planes,
@@ -1351,5 +1372,8 @@ int intel_bw_init(struct drm_i915_private *dev_priv)
intel_atomic_global_obj_init(dev_priv, _priv->display.bw.obj,
 >base, _bw_funcs);
 
+   if (DISPLAY_VER(dev_priv) < 14)
+   icl_force_disable_sagv(dev_priv, state);
+
return 0;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h 
b/drivers/gpu/drm/i915/display/intel_bw.h
index 59cb4fc5db76..243192fd4cae 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -74,5 +74,7 @@ int intel_bw_calc_min_cdclk(struct intel_atomic_state *state,
bool *need_cdclk_calc);
 int intel_bw_min_cdclk(struct drm_i915_private *i915,
   const struct intel_bw_state *bw_state);
+void icl_force_disable_sagv(struct drm_i915_private *dev_priv,
+   struct intel_bw_state *bw_state);
 
 #endif /* __INTEL_BW_H__ */
-- 
2.37.3



[PATCH 1/3] drm/i915: Add meaningful traces for QGV point info error handling

2024-01-17 Thread Stanislav Lisovskiy
For debug purposes we need those - error path won't flood the log,
however there has been already numerous cases, when due to lack
of debugs, we couldn't immediately tell what was the problem on
customer machine, which slowed down the investigation, requiring
to get access to target device and adding those traces manually.

v2: - Make the debug more generic and move it to intel_dram_detect
  (Gustavo Sousa)

Signed-off-by: Stanislav Lisovskiy 
---
 drivers/gpu/drm/i915/display/intel_bw.c | 4 +++-
 drivers/gpu/drm/i915/soc/intel_dram.c   | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
b/drivers/gpu/drm/i915/display/intel_bw.c
index 7f2a50b4f494..77886cc21211 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -290,8 +290,10 @@ static int icl_get_qgv_points(struct drm_i915_private 
*dev_priv,
struct intel_qgv_point *sp = >points[i];
 
ret = intel_read_qgv_point_info(dev_priv, sp, i);
-   if (ret)
+   if (ret) {
+   drm_dbg_kms(_priv->drm, "Could not read QGV %d 
info\n", i);
return ret;
+   }
 
drm_dbg_kms(_priv->drm,
"QGV %d: DCLK=%d tRP=%d tRDPRE=%d tRAS=%d tRCD=%d 
tRC=%d\n",
diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c 
b/drivers/gpu/drm/i915/soc/intel_dram.c
index 15492b69f698..e957be5bfb35 100644
--- a/drivers/gpu/drm/i915/soc/intel_dram.c
+++ b/drivers/gpu/drm/i915/soc/intel_dram.c
@@ -681,6 +681,8 @@ void intel_dram_detect(struct drm_i915_private *i915)
if (ret)
return;
 
+   drm_dbg_kms(>drm, "Num qgv points %d\n", 
dram_info->num_qgv_points);
+
drm_dbg_kms(>drm, "DRAM channels: %u\n", dram_info->num_channels);
 
drm_dbg_kms(>drm, "Watermark level 0 adjustment needed: %s\n",
-- 
2.37.3



[PATCH 0/3] QGV/SAGV related fixes

2024-01-17 Thread Stanislav Lisovskiy
We have couple of customer issues, related to SAGV/QGV point
calculation. Those patches contain fixes plus some additional
debugs for those issues.

Stanislav Lisovskiy (3):
  drm/i915: Add meaningful traces for QGV point info error handling
  drm/i915: Extract code required to calculate max qgv/psf gv point
  drm/i915: Disable SAGV on bw init, to force QGV point recalculation

 drivers/gpu/drm/i915/display/intel_bw.c | 109 +---
 drivers/gpu/drm/i915/display/intel_bw.h |   2 +
 drivers/gpu/drm/i915/soc/intel_dram.c   |   2 +
 3 files changed, 84 insertions(+), 29 deletions(-)

-- 
2.37.3



Re: [PATCH v3 05/16] drm/i915: Disable the "binder"

2024-01-17 Thread Michał Winiarski
On Tue, Jan 16, 2024 at 09:56:25AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Now that the GGTT PTE updates go straight to GSMBASE (bypassing
> GTTMMADR) there should be no more risk of system hangs? So the
> "binder" (ie. update the PTEs via MI_UPDATE_GTT) is no longer
> necessary, disable it.
> 
> My main worry with the MI_UPDATE_GTT are:
> - only used on this one platform so very limited testing coverage
> - async so more opprtunities to screw things up
> - what happens if the engine hangs while we're waiting for MI_UPDATE_GTT
>   to finish?
> - requires working command submission, so even getting a working
>   display now depends on a lot more extra components working correctly
> 
> TODO: MI_UPDATE_GTT might be interesting as an optimization
> though, so perhaps someone should look into always using it
> (assuming the GPU is alive and well)?
> 
> v2: Keep using MI_UPDATE_GTT on VM guests
> 
> Cc: Paz Zcharya 
> Cc: Nirmoy Das 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/gt/intel_gtt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
> b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index 86f73fe558ca..e83dabc56a14 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -24,7 +24,8 @@
>  bool i915_ggtt_require_binder(struct drm_i915_private *i915)
>  {
>   /* Wa_13010847436 & Wa_14019519902 */
> - return MEDIA_VER_FULL(i915) == IP_VER(13, 0);
> + return i915_run_as_guest() &&
> + MEDIA_VER_FULL(i915) == IP_VER(13, 0);

Note that i915_run_as_guest() is not the most reliable way to decide
whether to use MI_UPDATE_GTT or straight to GSMBASE, as it requires the
hypervisor to "opt-in" and set the X86_FEATURE_HYPERVISOR.
If it's not set - the driver will go into GSMBASE, which is not mapped
inside the guest.
Does the system firmware advertise whether GSMBASE is "open" or "closed"
to CPU access in any way?

-Michał

>  }
>  
>  static bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
> -- 
> 2.41.0
> 


✓ Fi.CI.BAT: success for drm/i915/opregion: remove unused lid_state

2024-01-17 Thread Patchwork
== Series Details ==

Series: drm/i915/opregion: remove unused lid_state
URL   : https://patchwork.freedesktop.org/series/128878/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_14132 -> Patchwork_128878v1


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/index.html

Participating hosts (38 -> 36)
--

  Additional (1): bat-mtlp-8 
  Missing(3): bat-dg2-8 bat-rpls-2 fi-snb-2520m 

Known issues


  Here are the changes found in Patchwork_128878v1 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@debugfs_test@basic-hwmon:
- bat-mtlp-8: NOTRUN -> [SKIP][1] ([i915#9318])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html

  * igt@gem_exec_suspend@basic-s0@lmem0:
- bat-dg2-9:  [PASS][2] -> [INCOMPLETE][3] ([i915#9275])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14132/bat-dg2-9/igt@gem_exec_suspend@basic...@lmem0.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-dg2-9/igt@gem_exec_suspend@basic...@lmem0.html

  * igt@gem_lmem_swapping@verify-random:
- bat-mtlp-8: NOTRUN -> [SKIP][4] ([i915#4613]) +3 other tests skip
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html

  * igt@gem_mmap@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][5] ([i915#4083])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@gem_m...@basic.html

  * igt@gem_mmap_gtt@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][6] ([i915#4077]) +2 other tests skip
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@gem_mmap_...@basic.html

  * igt@gem_render_tiled_blits@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][7] ([i915#4079]) +1 other test skip
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html

  * igt@i915_pm_rps@basic-api:
- bat-mtlp-8: NOTRUN -> [SKIP][8] ([i915#6621])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#6645])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#5190])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#4212]) +8 other tests skip
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#4213]) +1 other test skip
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
- bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#3555] / [i915#3840] / 
[i915#9159])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@kms_...@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
- bat-mtlp-8: NOTRUN -> [SKIP][14] ([fdo#109285])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@kms_force_connector_ba...@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
- bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#5274])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@kms_force_connector_ba...@prune-stale-modes.html

  * igt@kms_setmode@basic-clone-single-crtc:
- bat-mtlp-8: NOTRUN -> [SKIP][16] ([i915#3555] / [i915#8809])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@kms_setm...@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-mmap:
- bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#3708] / [i915#4077]) +1 
other test skip
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@prime_v...@basic-fence-mmap.html

  * igt@prime_vgem@basic-fence-read:
- bat-mtlp-8: NOTRUN -> [SKIP][18] ([i915#3708]) +2 other tests skip
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128878v1/bat-mtlp-8/igt@prime_v...@basic-fence-read.html

  
 Possible fixes 

  * igt@i915_selftest@live@hangcheck:
- {bat-rpls-3}:   [DMESG-WARN][19] ([i915#5591]) -> [PASS][20]
   [19]: 

✗ Fi.CI.BUILD: failure for drm/i915: move interrupt save/restore into vblank section helpers

2024-01-17 Thread Patchwork
== Series Details ==

Series: drm/i915: move interrupt save/restore into vblank section helpers
URL   : https://patchwork.freedesktop.org/series/128869/
State : failure

== Summary ==

Error: make failed
  CALLscripts/checksyscalls.sh
  DESCEND objtool
  INSTALL libsubcmd_headers
  CC [M]  drivers/gpu/drm/i915/display/intel_vblank.o
In file included from ./include/linux/bitops.h:7,
 from ./include/linux/kernel.h:23,
 from ./arch/x86/include/asm/percpu.h:27,
 from ./arch/x86/include/asm/current.h:10,
 from ./include/linux/mutex.h:14,
 from ./include/linux/notifier.h:14,
 from ./include/linux/pm_qos.h:16,
 from ./drivers/gpu/drm/i915/i915_drv.h:35,
 from drivers/gpu/drm/i915/display/intel_vblank.c:6:
drivers/gpu/drm/i915/display/intel_vblank.c: In function 
‘intel_vblank_section_enter’:
drivers/gpu/drm/i915/display/intel_vblank.c:282:17: error: ‘irqflags’ 
undeclared (first use in this function); did you mean ‘ifr_flags’?
  282 |  local_irq_save(irqflags);
  | ^~~~
./include/linux/typecheck.h:11:9: note: in definition of macro ‘typecheck’
   11 |  typeof(x) __dummy2; \
  | ^
./include/linux/irqflags.h:222:3: note: in expansion of macro 
‘raw_local_irq_save’
  222 |   raw_local_irq_save(flags);  \
  |   ^~
drivers/gpu/drm/i915/display/intel_vblank.c:282:2: note: in expansion of macro 
‘local_irq_save’
  282 |  local_irq_save(irqflags);
  |  ^~
drivers/gpu/drm/i915/display/intel_vblank.c:282:17: note: each undeclared 
identifier is reported only once for each function it appears in
  282 |  local_irq_save(irqflags);
  | ^~~~
./include/linux/typecheck.h:11:9: note: in definition of macro ‘typecheck’
   11 |  typeof(x) __dummy2; \
  | ^
./include/linux/irqflags.h:222:3: note: in expansion of macro 
‘raw_local_irq_save’
  222 |   raw_local_irq_save(flags);  \
  |   ^~
drivers/gpu/drm/i915/display/intel_vblank.c:282:2: note: in expansion of macro 
‘local_irq_save’
  282 |  local_irq_save(irqflags);
  |  ^~
./include/linux/typecheck.h:12:18: error: comparison of distinct pointer types 
lacks a cast [-Werror]
   12 |  (void)(&__dummy == &__dummy2); \
  |  ^~
./include/linux/irqflags.h:178:3: note: in expansion of macro ‘typecheck’
  178 |   typecheck(unsigned long, flags); \
  |   ^
./include/linux/irqflags.h:222:3: note: in expansion of macro 
‘raw_local_irq_save’
  222 |   raw_local_irq_save(flags);  \
  |   ^~
drivers/gpu/drm/i915/display/intel_vblank.c:282:2: note: in expansion of macro 
‘local_irq_save’
  282 |  local_irq_save(irqflags);
  |  ^~
./include/linux/typecheck.h:12:18: error: comparison of distinct pointer types 
lacks a cast [-Werror]
   12 |  (void)(&__dummy == &__dummy2); \
  |  ^~
./include/linux/irqflags.h:194:3: note: in expansion of macro ‘typecheck’
  194 |   typecheck(unsigned long, flags); \
  |   ^
./include/linux/irqflags.h:223:8: note: in expansion of macro 
‘raw_irqs_disabled_flags’
  223 |   if (!raw_irqs_disabled_flags(flags)) \
  |^~~
drivers/gpu/drm/i915/display/intel_vblank.c:282:2: note: in expansion of macro 
‘local_irq_save’
  282 |  local_irq_save(irqflags);
  |  ^~
drivers/gpu/drm/i915/display/intel_vblank.c: In function 
‘intel_vblank_section_exit’:
drivers/gpu/drm/i915/display/intel_vblank.c:294:20: error: ‘irqflags’ 
undeclared (first use in this function); did you mean ‘ifr_flags’?
  294 |  local_irq_restore(irqflags);
  |^~~~
./include/linux/typecheck.h:11:9: note: in definition of macro ‘typecheck’
   11 |  typeof(x) __dummy2; \
  | ^
./include/linux/irqflags.h:229:8: note: in expansion of macro 
‘raw_irqs_disabled_flags’
  229 |   if (!raw_irqs_disabled_flags(flags)) \
  |^~~
drivers/gpu/drm/i915/display/intel_vblank.c:294:2: note: in expansion of macro 
‘local_irq_restore’
  294 |  local_irq_restore(irqflags);
  |  ^
./include/linux/typecheck.h:12:18: error: comparison of distinct pointer types 
lacks a cast [-Werror]
   12 |  (void)(&__dummy == &__dummy2); \
  |  ^~
./include/linux/irqflags.h:194:3: note: in expansion of macro ‘typecheck’
  194 |   typecheck(unsigned long, flags); \
  |   ^
./include/linux/irqflags.h:229:8: note: in expansion of macro 
‘raw_irqs_disabled_flags’
  229 |   if (!raw_irqs_disabled_flags(flags)) \
  |^~~
drivers/gpu/drm/i915/display/intel_vblank.c:294:2: note: in expansion of macro 
‘local_irq_restore’
  294 |  local_irq_restore(irqflags);
  |  ^
./include/linux/typecheck.h:12:18: error: comparison of distinct pointer types 
lacks a cast [-Werror]
   

Re: [PATCH v2 8/9] drm/i915: Perform vblank evasion around legacy cursor updates

2024-01-17 Thread Hogander, Jouni


Re: [PATCH v2 2/4] drm/uAPI: Add "force color format" drm property as setting for userspace

2024-01-17 Thread Andri Yngvason
mið., 17. jan. 2024 kl. 09:21 skrifaði Pekka Paalanen :
>
> On Tue, 16 Jan 2024 14:11:43 +
> Andri Yngvason  wrote:
>
> > þri., 16. jan. 2024 kl. 13:29 skrifaði Sebastian Wick
> > :
> > >
> > > On Tue, Jan 16, 2024 at 01:13:13PM +, Andri Yngvason wrote:
> > [...]
> > > > şri., 16. jan. 2024 kl. 11:42 skrifaği Sebastian Wick
> > > > :
> > > > >
> > > > > On Mon, Jan 15, 2024 at 04:05:52PM +, Andri Yngvason wrote:
> > > > > > From: Werner Sembach 
> > > > > >
> > > > > > Add a new general drm property "force color format" which can be 
> > > > > > used
> > > > > > by userspace to tell the graphics driver which color format to use.
> > > > >
> > > > > I don't like the "force" in the name. This just selects the color
> > > > > format, let's just call it "color format" then.
> > > > >
> > > >
> > > > In previous revisions, this was "preferred color format" and "actual
> > > > color format", of which the latter has been dropped. I recommend
> > > > reading the discussion for previous revisions.
> > >
> > > Please don't imply that I didn't read the thread I'm answering to.
>
> FYI, I have not read this thread.
>

pq, You did not read this summary?
https://lore.kernel.org/dri-devel/cafnqbqwjejax6b4oewpgasmud5_nxzymxufdog294ctvgbt...@mail.gmail.com/

You partook in the discussion on IRC. Please read it and tell me if I
misunderstood anything.

Sebastian, I apologise. You clearly read it as you even replied to it!

> > >
> > > > There are arguments for adding "actual color format" later and if it
> > > > is added later, we'd end up with "color format" and "actual color
> > > > format", which might be confusing, and it is why I chose to call it
> > > > "force color format" because it clearly communicates intent and
> > > > disambiguates it from "actual color format".
> > >
> > > There is no such thing as "actual color format" in upstream though.
> > > Basing your naming on discarded ideas is not useful. The thing that sets
> > > the color space for example is called "Colorspace", not "force
> > > colorspace".
> > >
> >
> > Sure, I'm happy with calling it whatever people want. Maybe we can
> > have a vote on it?
>
> It would sound strange to say "force color format" = "auto". Just drop
> the "force" of it.
>
> If and when we need the feedback counterpart, it could be an immutable
> prop called "active color format" where "auto" is not a valid value, or
> something in the new "output properties" design Sima has been thinking
> of.

There seems to be consensus for calling it "color format"

>
> > > > [...]
> > > > > > @@ -1396,6 +1404,15 @@ static const u32 dp_colorspaces =
> > > > > >   *   drm_connector_attach_max_bpc_property() to create and attach 
> > > > > > the
> > > > > >   *   property to the connector during initialization.
> > > > > >   *
> > > > > > + * force color format:
> > > > > > + *   This property is used by userspace to change the used color 
> > > > > > format. When
> > > > > > + *   used the driver will use the selected format if valid for the 
> > > > > > hardware,
> > > > >
> > > > > All properties are always "used", they just can have different values.
> > > > > You probably want to talk about the auto mode here.
> > > >
> > > > Maybe we can say something like: If userspace does not set the
> > > > property or if it is explicitly set to zero, the driver will select
> > > > the appropriate color format based on other constraints.
> > >
> > > The property can be in any state without involvement from user space.
> > > Don't talk about setting it, talk about the state it is in:
> > >
> > >   When the color format is auto, the driver will select a format.
> > >
> >
> > Ok.
> >
> > > > >
> > > > > > + *   sink, and current resolution and refresh rate combination. 
> > > > > > Drivers to
> > > > >
> > > > > If valid? So when a value is not actually supported user space can 
> > > > > still
> > > > > set it? What happens then? How should user space figure out if the
> > > > > driver and the sink support the format?
> > > >
> > > > The kernel does not expose this property unless it's implemented in the 
> > > > driver.
> > >
> > > If the driver simply doesn't support *one format*, the enum value for
> > > that format should not be exposed, period. This isn't about the property
> > > on its own.
> >
> > Right, understood. You mean that enum should only contain values that
> > are supported by the driver.
>
> Yes. When a driver installs a property, it can choose which of the enum
> entries are exposed. That cannot be changed later though, so the list
> cannot live by the currently connected sink, only by what the driver
> and display controlled could ever do.

Yes, and I think that basing it also on the connected sink's
capabilities would just add complexity for very little gain. In fact,
I think that limiting it based on the driver's capabilities is also
over-engineering, but I don't mind adding it if that's what people
really want.

>
> > > > This was originally "preferred color format". 

Re: [PATCH] drm/i915/opregion: remove unused lid_state

2024-01-17 Thread Ville Syrjälä
On Wed, Jan 17, 2024 at 02:25:46PM +0200, Jani Nikula wrote:
> Not sure if lid_state has ever been used, but at least not for a long
> time. Remove it.

It was probably used when we had that disgusting lid notifier
thing, but that got killed some years ago.

> 
> Suggested-by: Ville Syrjälä 
> Signed-off-by: Jani Nikula 

Reviewed-by: Ville Syrjälä 

> ---
>  drivers/gpu/drm/i915/display/intel_opregion.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c 
> b/drivers/gpu/drm/i915/display/intel_opregion.c
> index 3f5a20f9153e..f242bb320610 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> @@ -266,7 +266,6 @@ struct intel_opregion {
>   void *vbt_firmware;
>   const void *vbt;
>   u32 vbt_size;
> - u32 *lid_state;
>   struct work_struct asle_work;
>   struct notifier_block acpi_notifier;
>  };
> @@ -958,7 +957,6 @@ int intel_opregion_setup(struct drm_i915_private 
> *dev_priv)
>   goto err_out;
>   }
>   opregion->header = base;
> - opregion->lid_state = base + ACPI_CLID;
>  
>   drm_dbg(_priv->drm, "ACPI OpRegion version %u.%u.%u\n",
>   opregion->header->over.major,
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel


Re: [PATCH 6/6] drm/i915/opregion: make struct intel_opregion opaque

2024-01-17 Thread Jani Nikula
On Wed, 17 Jan 2024, Ville Syrjälä  wrote:
> lid_state is unused btw. Someone should throw a nuke at it.

Done.

https://patchwork.freedesktop.org/patch/msgid/20240117122546.1551400-1-jani.nik...@intel.com


-- 
Jani Nikula, Intel


Re: [PATCH 3/5] drm/ttm: replace busy placement with flags v6

2024-01-17 Thread Thomas Hellström



On 1/17/24 11:47, Thomas Hellström wrote:

Hi, Christian

Xe changes look good. Will send the series to xe ci to check for 
regressions.


Hmm, there are some checkpatch warnings about author / SOB email mismatch,

But worserthere are some regressions in the dma-buf ktest (it tests 
evicting of a dynamic dma-buf),


https://patchwork.freedesktop.org/series/128873/

I'll take a look later today or tomorrow.

/Thomas





/Thomas


On 1/12/24 13:51, Christian König wrote:

From: Somalapuram Amaranath 

Instead of a list of separate busy placement add flags which indicate
that a placement should only be used when there is room or if we need to
evict.

v2: add missing TTM_PL_FLAG_IDLE for i915
v3: fix auto build test ERROR on drm-tip/drm-tip
v4: fix some typos pointed out by checkpatch
v5: cleanup some rebase problems with VMWGFX
v6: implement some missing VMWGFX functionality pointed out by Zack,
 rename the flags as suggested by Michel, rebase on drm-tip and
 adjust XE as well

Signed-off-by: Christian König 
Signed-off-by: Somalapuram Amaranath 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 11 +---
  drivers/gpu/drm/drm_gem_vram_helper.c  |  2 -
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c    | 37 +--
  drivers/gpu/drm/loongson/lsdc_ttm.c    |  2 -
  drivers/gpu/drm/nouveau/nouveau_bo.c   | 59 +++--
  drivers/gpu/drm/nouveau/nouveau_bo.h   |  1 -
  drivers/gpu/drm/qxl/qxl_object.c   |  2 -
  drivers/gpu/drm/qxl/qxl_ttm.c  |  2 -
  drivers/gpu/drm/radeon/radeon_object.c |  2 -
  drivers/gpu/drm/radeon/radeon_ttm.c    |  8 +--
  drivers/gpu/drm/radeon/radeon_uvd.c    |  1 -
  drivers/gpu/drm/ttm/ttm_bo.c   | 21 ---
  drivers/gpu/drm/ttm/ttm_resource.c | 73 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 33 +++---
  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  4 --
  drivers/gpu/drm/xe/xe_bo.c | 33 +-
  include/drm/ttm/ttm_placement.h    | 10 +--
  include/drm/ttm/ttm_resource.h |  8 +--
  19 files changed, 118 insertions(+), 197 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 425cebcc5cbf..b671b0665492 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -220,9 +220,6 @@ void amdgpu_bo_placement_from_domain(struct 
amdgpu_bo *abo, u32 domain)

    placement->num_placement = c;
  placement->placement = places;
-
-    placement->num_busy_placement = c;
-    placement->busy_placement = places;
  }
    /**
@@ -1397,8 +1394,7 @@ vm_fault_t 
amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)

  AMDGPU_GEM_DOMAIN_GTT);
    /* Avoid costly evictions; only set GTT as a busy placement */
-    abo->placement.num_busy_placement = 1;
-    abo->placement.busy_placement = >placements[1];
+    abo->placements[0].flags |= TTM_PL_FLAG_DESIRED;
    r = ttm_bo_validate(bo, >placement, );
  if (unlikely(r == -EBUSY || r == -ERESTARTSYS))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

index 75c9fd2c6c2a..8722beba494e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -102,23 +102,19 @@ static void amdgpu_evict_flags(struct 
ttm_buffer_object *bo,

  /* Don't handle scatter gather BOs */
  if (bo->type == ttm_bo_type_sg) {
  placement->num_placement = 0;
-    placement->num_busy_placement = 0;
  return;
  }
    /* Object isn't an AMDGPU object so ignore */
  if (!amdgpu_bo_is_amdgpu_bo(bo)) {
  placement->placement = 
-    placement->busy_placement = 
  placement->num_placement = 1;
-    placement->num_busy_placement = 1;
  return;
  }
    abo = ttm_to_amdgpu_bo(bo);
  if (abo->flags & AMDGPU_GEM_CREATE_DISCARDABLE) {
  placement->num_placement = 0;
-    placement->num_busy_placement = 0;
  return;
  }
  @@ -128,13 +124,13 @@ static void amdgpu_evict_flags(struct 
ttm_buffer_object *bo,

  case AMDGPU_PL_OA:
  case AMDGPU_PL_DOORBELL:
  placement->num_placement = 0;
-    placement->num_busy_placement = 0;
  return;
    case TTM_PL_VRAM:
  if (!adev->mman.buffer_funcs_enabled) {
  /* Move to system memory */
  amdgpu_bo_placement_from_domain(abo, 
AMDGPU_GEM_DOMAIN_CPU);

+
  } else if (!amdgpu_gmc_vram_full_visible(>gmc) &&
 !(abo->flags & 
AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) &&

 amdgpu_bo_in_cpu_visible_vram(abo)) {
@@ -149,8 +145,7 @@ static void amdgpu_evict_flags(struct 
ttm_buffer_object *bo,

  AMDGPU_GEM_DOMAIN_CPU);
  abo->placements[0].fpfn = adev->gmc.visible_vram_size 

[PATCH] drm/i915/opregion: remove unused lid_state

2024-01-17 Thread Jani Nikula
Not sure if lid_state has ever been used, but at least not for a long
time. Remove it.

Suggested-by: Ville Syrjälä 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/display/intel_opregion.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c 
b/drivers/gpu/drm/i915/display/intel_opregion.c
index 3f5a20f9153e..f242bb320610 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.c
+++ b/drivers/gpu/drm/i915/display/intel_opregion.c
@@ -266,7 +266,6 @@ struct intel_opregion {
void *vbt_firmware;
const void *vbt;
u32 vbt_size;
-   u32 *lid_state;
struct work_struct asle_work;
struct notifier_block acpi_notifier;
 };
@@ -958,7 +957,6 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
goto err_out;
}
opregion->header = base;
-   opregion->lid_state = base + ACPI_CLID;
 
drm_dbg(_priv->drm, "ACPI OpRegion version %u.%u.%u\n",
opregion->header->over.major,
-- 
2.39.2



Re: [PATCH 0/9] drm/i915: Cursor vblank evasion

2024-01-17 Thread Ville Syrjälä
On Wed, Jan 17, 2024 at 11:30:10AM +, Shankar, Uma wrote:
> 
> 
> > -Original Message-
> > From: Intel-gfx  On Behalf Of Ville
> > Syrjala
> > Sent: Wednesday, December 13, 2023 3:55 PM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [PATCH 0/9] drm/i915: Cursor vblank evasion
> > 
> > From: Ville Syrjälä 
> > 
> > MTL seems very good at racing the cursor mailbox updates against the vblank,
> > causing things to not latch for long enough to cause GTT faults. Attempt to 
> > hook
> > up vblank evasions into the legacy cursor path to avoid this.
> > 
> > Also revert a dangerous wm/ddb change related to cursors.
> 
> I have already RB'ed the changes in the series but somehow patchwork had 
> issues and
> not reflecting the same.
> 
> FWIW, this series is
> Reviewed-by: Uma Shankar 
> 
> Please merge once the CI results show green, seems one test
> igt@kms_cursor_legacy@torture-move@pipe-a throwing some warnings.

Seems we're still seeing some timeouts from the vblank evasion,
indicating the schedule_timeout(1ms) took approximately some
integer multiple of the frame time :(

To confirm what's happening I might need to pull the full
vblank evasion debugs to into the cursor path as well...

-- 
Ville Syrjälä
Intel


RE: [PATCH 0/9] drm/i915: Cursor vblank evasion

2024-01-17 Thread Shankar, Uma


> -Original Message-
> From: Intel-gfx  On Behalf Of Ville
> Syrjala
> Sent: Wednesday, December 13, 2023 3:55 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH 0/9] drm/i915: Cursor vblank evasion
> 
> From: Ville Syrjälä 
> 
> MTL seems very good at racing the cursor mailbox updates against the vblank,
> causing things to not latch for long enough to cause GTT faults. Attempt to 
> hook
> up vblank evasions into the legacy cursor path to avoid this.
> 
> Also revert a dangerous wm/ddb change related to cursors.

I have already RB'ed the changes in the series but somehow patchwork had issues 
and
not reflecting the same.

FWIW, this series is
Reviewed-by: Uma Shankar 

Please merge once the CI results show green, seems one test
igt@kms_cursor_legacy@torture-move@pipe-a throwing some warnings.

Regards,
Uma Shankar

> Ville Syrjälä (9):
>   drm/i915: Decouple intel_crtc_vblank_evade_scanlines() from atomic
> commits
>   drm/i915: Reorder drm_vblank_put() vs. need_vlv_dsi_wa
>   drm/i915: Introduce struct intel_vblank_evade_ctx
>   drm/i915: Include need_vlv_dsi_wa in intel_vblank_evade_ctx
>   drm/i915: Extract intel_vblank_evade()
>   drm/i915: Move the min/max scanline sanity check into
> intel_vblank_evade()
>   drm/i915: Move intel_vblank_evade() & co. into intel_vblank.c
>   drm/i915: Perform vblank evasion around legacy cursor updates
>   Revert "drm/i915/xe2lpd: Treat cursor plane as regular plane for DDB
> allocation"
> 
>  .../gpu/drm/i915/display/intel_atomic_plane.c |   6 +-
>  drivers/gpu/drm/i915/display/intel_crtc.c | 128 ++---
>  drivers/gpu/drm/i915/display/intel_cursor.c   |  16 ++-
>  drivers/gpu/drm/i915/display/intel_vblank.c   | 130 ++
>  drivers/gpu/drm/i915/display/intel_vblank.h   |  12 ++
>  drivers/gpu/drm/i915/display/skl_watermark.c  |  16 +--
>  6 files changed, 170 insertions(+), 138 deletions(-)
> 
> --
> 2.41.0



Re: [PATCH 6/6] drm/i915/opregion: make struct intel_opregion opaque

2024-01-17 Thread Ville Syrjälä
On Thu, Jan 11, 2024 at 07:21:19PM +0200, Jani Nikula wrote:
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c 
> b/drivers/gpu/drm/i915/display/intel_opregion.c
> index 26aacb01f9ec..3f5a20f9153e 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> @@ -250,11 +250,37 @@ struct opregion_asle_ext {
>  
>  #define MAX_DSLP 1500
>  
> +#define OPREGION_SIZE(8 * 1024)
> +
> +struct intel_opregion {
> + struct drm_i915_private *i915;
> +
> + struct opregion_header *header;
> + struct opregion_acpi *acpi;
> + struct opregion_swsci *swsci;
> + u32 swsci_gbda_sub_functions;
> + u32 swsci_sbcb_sub_functions;
> + struct opregion_asle *asle;
> + struct opregion_asle_ext *asle_ext;
> + void *rvda;
> + void *vbt_firmware;
> + const void *vbt;
> + u32 vbt_size;
> + u32 *lid_state;

lid_state is unused btw. Someone should throw a nuke at it.

> + struct work_struct asle_work;
> + struct notifier_block acpi_notifier;
> +};
> +

-- 
Ville Syrjälä
Intel


Re: [PATCH 2/3] drm/i915: Extract code required to calculate max qgv/psf gv point

2024-01-17 Thread Lisovskiy, Stanislav
On Wed, Jan 17, 2024 at 01:12:49PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 17, 2024 at 12:12:18PM +0200, Lisovskiy, Stanislav wrote:
> > On Fri, Jan 12, 2024 at 07:35:24PM +0200, Ville Syrjälä wrote:
> > > On Tue, Nov 28, 2023 at 10:37:53AM +0200, Stanislav Lisovskiy wrote:
> > > > We need that in order to force disable SAGV in next patch.
> > > > Also it is beneficial to separate that code, as in majority cases,
> > > > when SAGV is enabled, we don't even need those calculations.
> > > > Also we probably need to determine max PSF GV point as well, however
> > > > currently we don't do that when we disable SAGV, which might be
> > > > actually causing some issues in that case.
> > > > 
> > > > Signed-off-by: Stanislav Lisovskiy 
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_bw.c | 82 -
> > > >  1 file changed, 65 insertions(+), 17 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
> > > > b/drivers/gpu/drm/i915/display/intel_bw.c
> > > > index 583cd2ebdf89..efd408e96e8a 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > > > @@ -805,6 +805,64 @@ intel_atomic_get_bw_state(struct 
> > > > intel_atomic_state *state)
> > > > return to_intel_bw_state(bw_state);
> > > >  }
> > > >  
> > > > +static unsigned int icl_max_bw_qgv_point(struct drm_i915_private *i915,
> > > > +int num_active_planes)
> > > > +{
> > > > +   unsigned int max_bw_point = 0;
> > > > +   unsigned int max_bw = 0;
> > > > +   unsigned int num_qgv_points = 
> > > > i915->display.bw.max[0].num_qgv_points;
> > > > +   int i;
> > > > +
> > > > +   for (i = 0; i < num_qgv_points; i++) {
> > > > +   unsigned int idx;
> > > > +   unsigned int max_data_rate;
> > > > +
> > > > +   if (DISPLAY_VER(i915) > 11)
> > > > +   idx = tgl_max_bw_index(i915, num_active_planes, 
> > > > i);
> > > > +   else
> > > > +   idx = icl_max_bw_index(i915, num_active_planes, 
> > > > i);
> > > > +
> > > > +   if (idx >= ARRAY_SIZE(i915->display.bw.max))
> > > > +   continue;
> > > > +
> > > > +   max_data_rate = i915->display.bw.max[idx].deratedbw[i];
> > > 
> > > Looks like that that part could be extracted to a helper
> > > to be used by both codepaths (would be a natural counterpart
> > > to adl_psf_bw()).
> > > 
> > > > +
> > > > +   /*
> > > > +* We need to know which qgv point gives us
> > > > +* maximum bandwidth in order to disable SAGV
> > > > +* if we find that we exceed SAGV block time
> > > > +* with watermarks. By that moment we already
> > > > +* have those, as it is calculated earlier in
> > > > +* intel_atomic_check,
> > > > +*/
> > > > +   if (max_data_rate > max_bw) {
> > > > +   max_bw_point = i;
> > > > +   max_bw = max_data_rate;
> > > > +   }
> > > > +   }
> > > > +
> > > > +   return max_bw_point;
> > > > +}
> > > > +
> > > > +unsigned int icl_max_bw_psf_gv_point(struct drm_i915_private *i915)
> > > > +{
> > > > +   unsigned int num_psf_gv_points = 
> > > > i915->display.bw.max[0].num_psf_gv_points;
> > > > +   unsigned int max_bw = 0;
> > > > +   unsigned int max_bw_point = 0;
> > > > +   int i;
> > > > +
> > > > +   for (i = 0; i < num_psf_gv_points; i++) {
> > > > +   unsigned int max_data_rate = adl_psf_bw(i915, i);
> > > > +
> > > > +   if (max_data_rate > max_bw) {
> > > > +   max_bw_point = i;
> > > > +   max_bw = max_data_rate;
> > > > +   }
> > > > +   }
> > > > +
> > > > +   return max_bw_point;
> > > > +}
> > > > +
> > > >  static int mtl_find_qgv_points(struct drm_i915_private *i915,
> > > >unsigned int data_rate,
> > > >unsigned int num_active_planes,
> > > > @@ -882,8 +940,6 @@ static int icl_find_qgv_points(struct 
> > > > drm_i915_private *i915,
> > > >const struct intel_bw_state 
> > > > *old_bw_state,
> > > >struct intel_bw_state *new_bw_state)
> > > >  {
> > > > -   unsigned int max_bw_point = 0;
> > > > -   unsigned int max_bw = 0;
> > > > unsigned int num_psf_gv_points = 
> > > > i915->display.bw.max[0].num_psf_gv_points;
> > > > unsigned int num_qgv_points = 
> > > > i915->display.bw.max[0].num_qgv_points;
> > > > u16 psf_points = 0;
> > > > @@ -909,18 +965,6 @@ static int icl_find_qgv_points(struct 
> > > > drm_i915_private *i915,
> > > >  
> > > > max_data_rate = i915->display.bw.max[idx].deratedbw[i];
> > > >  
> > > > -

Re: [PATCH 2/3] drm/i915: Extract code required to calculate max qgv/psf gv point

2024-01-17 Thread Ville Syrjälä
On Wed, Jan 17, 2024 at 12:12:18PM +0200, Lisovskiy, Stanislav wrote:
> On Fri, Jan 12, 2024 at 07:35:24PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 28, 2023 at 10:37:53AM +0200, Stanislav Lisovskiy wrote:
> > > We need that in order to force disable SAGV in next patch.
> > > Also it is beneficial to separate that code, as in majority cases,
> > > when SAGV is enabled, we don't even need those calculations.
> > > Also we probably need to determine max PSF GV point as well, however
> > > currently we don't do that when we disable SAGV, which might be
> > > actually causing some issues in that case.
> > > 
> > > Signed-off-by: Stanislav Lisovskiy 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_bw.c | 82 -
> > >  1 file changed, 65 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
> > > b/drivers/gpu/drm/i915/display/intel_bw.c
> > > index 583cd2ebdf89..efd408e96e8a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > > @@ -805,6 +805,64 @@ intel_atomic_get_bw_state(struct intel_atomic_state 
> > > *state)
> > >   return to_intel_bw_state(bw_state);
> > >  }
> > >  
> > > +static unsigned int icl_max_bw_qgv_point(struct drm_i915_private *i915,
> > > +  int num_active_planes)
> > > +{
> > > + unsigned int max_bw_point = 0;
> > > + unsigned int max_bw = 0;
> > > + unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
> > > + int i;
> > > +
> > > + for (i = 0; i < num_qgv_points; i++) {
> > > + unsigned int idx;
> > > + unsigned int max_data_rate;
> > > +
> > > + if (DISPLAY_VER(i915) > 11)
> > > + idx = tgl_max_bw_index(i915, num_active_planes, i);
> > > + else
> > > + idx = icl_max_bw_index(i915, num_active_planes, i);
> > > +
> > > + if (idx >= ARRAY_SIZE(i915->display.bw.max))
> > > + continue;
> > > +
> > > + max_data_rate = i915->display.bw.max[idx].deratedbw[i];
> > 
> > Looks like that that part could be extracted to a helper
> > to be used by both codepaths (would be a natural counterpart
> > to adl_psf_bw()).
> > 
> > > +
> > > + /*
> > > +  * We need to know which qgv point gives us
> > > +  * maximum bandwidth in order to disable SAGV
> > > +  * if we find that we exceed SAGV block time
> > > +  * with watermarks. By that moment we already
> > > +  * have those, as it is calculated earlier in
> > > +  * intel_atomic_check,
> > > +  */
> > > + if (max_data_rate > max_bw) {
> > > + max_bw_point = i;
> > > + max_bw = max_data_rate;
> > > + }
> > > + }
> > > +
> > > + return max_bw_point;
> > > +}
> > > +
> > > +unsigned int icl_max_bw_psf_gv_point(struct drm_i915_private *i915)
> > > +{
> > > + unsigned int num_psf_gv_points = 
> > > i915->display.bw.max[0].num_psf_gv_points;
> > > + unsigned int max_bw = 0;
> > > + unsigned int max_bw_point = 0;
> > > + int i;
> > > +
> > > + for (i = 0; i < num_psf_gv_points; i++) {
> > > + unsigned int max_data_rate = adl_psf_bw(i915, i);
> > > +
> > > + if (max_data_rate > max_bw) {
> > > + max_bw_point = i;
> > > + max_bw = max_data_rate;
> > > + }
> > > + }
> > > +
> > > + return max_bw_point;
> > > +}
> > > +
> > >  static int mtl_find_qgv_points(struct drm_i915_private *i915,
> > >  unsigned int data_rate,
> > >  unsigned int num_active_planes,
> > > @@ -882,8 +940,6 @@ static int icl_find_qgv_points(struct 
> > > drm_i915_private *i915,
> > >  const struct intel_bw_state *old_bw_state,
> > >  struct intel_bw_state *new_bw_state)
> > >  {
> > > - unsigned int max_bw_point = 0;
> > > - unsigned int max_bw = 0;
> > >   unsigned int num_psf_gv_points = 
> > > i915->display.bw.max[0].num_psf_gv_points;
> > >   unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
> > >   u16 psf_points = 0;
> > > @@ -909,18 +965,6 @@ static int icl_find_qgv_points(struct 
> > > drm_i915_private *i915,
> > >  
> > >   max_data_rate = i915->display.bw.max[idx].deratedbw[i];
> > >  
> > > - /*
> > > -  * We need to know which qgv point gives us
> > > -  * maximum bandwidth in order to disable SAGV
> > > -  * if we find that we exceed SAGV block time
> > > -  * with watermarks. By that moment we already
> > > -  * have those, as it is calculated earlier in
> > > -  * intel_atomic_check,
> > > -  */
> > > - if (max_data_rate > max_bw) {
> > > - max_bw_point = i;
> > > - max_bw = max_data_rate;
> > > - }
> > >   if (max_data_rate >= data_rate)
> > >   qgv_points |= BIT(i);
> > >  
> > > @@ 

Re: [PATCH 3/5] drm/ttm: replace busy placement with flags v6

2024-01-17 Thread Thomas Hellström

Hi, Christian

Xe changes look good. Will send the series to xe ci to check for 
regressions.


/Thomas


On 1/12/24 13:51, Christian König wrote:

From: Somalapuram Amaranath 

Instead of a list of separate busy placement add flags which indicate
that a placement should only be used when there is room or if we need to
evict.

v2: add missing TTM_PL_FLAG_IDLE for i915
v3: fix auto build test ERROR on drm-tip/drm-tip
v4: fix some typos pointed out by checkpatch
v5: cleanup some rebase problems with VMWGFX
v6: implement some missing VMWGFX functionality pointed out by Zack,
 rename the flags as suggested by Michel, rebase on drm-tip and
 adjust XE as well

Signed-off-by: Christian König 
Signed-off-by: Somalapuram Amaranath 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 11 +---
  drivers/gpu/drm/drm_gem_vram_helper.c  |  2 -
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 37 +--
  drivers/gpu/drm/loongson/lsdc_ttm.c|  2 -
  drivers/gpu/drm/nouveau/nouveau_bo.c   | 59 +++--
  drivers/gpu/drm/nouveau/nouveau_bo.h   |  1 -
  drivers/gpu/drm/qxl/qxl_object.c   |  2 -
  drivers/gpu/drm/qxl/qxl_ttm.c  |  2 -
  drivers/gpu/drm/radeon/radeon_object.c |  2 -
  drivers/gpu/drm/radeon/radeon_ttm.c|  8 +--
  drivers/gpu/drm/radeon/radeon_uvd.c|  1 -
  drivers/gpu/drm/ttm/ttm_bo.c   | 21 ---
  drivers/gpu/drm/ttm/ttm_resource.c | 73 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 33 +++---
  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  4 --
  drivers/gpu/drm/xe/xe_bo.c | 33 +-
  include/drm/ttm/ttm_placement.h| 10 +--
  include/drm/ttm/ttm_resource.h |  8 +--
  19 files changed, 118 insertions(+), 197 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 425cebcc5cbf..b671b0665492 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -220,9 +220,6 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, 
u32 domain)
  
  	placement->num_placement = c;

placement->placement = places;
-
-   placement->num_busy_placement = c;
-   placement->busy_placement = places;
  }
  
  /**

@@ -1397,8 +1394,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct 
ttm_buffer_object *bo)
AMDGPU_GEM_DOMAIN_GTT);
  
  	/* Avoid costly evictions; only set GTT as a busy placement */

-   abo->placement.num_busy_placement = 1;
-   abo->placement.busy_placement = >placements[1];
+   abo->placements[0].flags |= TTM_PL_FLAG_DESIRED;
  
  	r = ttm_bo_validate(bo, >placement, );

if (unlikely(r == -EBUSY || r == -ERESTARTSYS))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 75c9fd2c6c2a..8722beba494e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -102,23 +102,19 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
*bo,
/* Don't handle scatter gather BOs */
if (bo->type == ttm_bo_type_sg) {
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
}
  
  	/* Object isn't an AMDGPU object so ignore */

if (!amdgpu_bo_is_amdgpu_bo(bo)) {
placement->placement = 
-   placement->busy_placement = 
placement->num_placement = 1;
-   placement->num_busy_placement = 1;
return;
}
  
  	abo = ttm_to_amdgpu_bo(bo);

if (abo->flags & AMDGPU_GEM_CREATE_DISCARDABLE) {
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
}
  
@@ -128,13 +124,13 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,

case AMDGPU_PL_OA:
case AMDGPU_PL_DOORBELL:
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
  
  	case TTM_PL_VRAM:

if (!adev->mman.buffer_funcs_enabled) {
/* Move to system memory */
amdgpu_bo_placement_from_domain(abo, 
AMDGPU_GEM_DOMAIN_CPU);
+
} else if (!amdgpu_gmc_vram_full_visible(>gmc) &&
   !(abo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) 
&&
   amdgpu_bo_in_cpu_visible_vram(abo)) {
@@ -149,8 +145,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
AMDGPU_GEM_DOMAIN_CPU);
abo->placements[0].fpfn = adev->gmc.visible_vram_size 
>> PAGE_SHIFT;
abo->placements[0].lpfn = 0;
-   

Re: [PATCH 2/5] drm/ttm: return ENOSPC from ttm_bo_mem_space

2024-01-17 Thread Thomas Hellström

Hi,

On 1/12/24 13:51, Christian König wrote:

Only convert it to ENOMEM in ttm_bo_validate.

This allows ttm_bo_validate to distinct between an out of memory

NIT: s/distinct/distinguish/

situation and just out of space in a placement domain.


In fact it would be nice if this could be propagated back to drivers as 
well at some point, but then perhaps guarded with a flag in the 
operation context.


In any case

Reviewed-by: Thomas Hellström 



Signed-off-by: Christian König 
---
  drivers/gpu/drm/ttm/ttm_bo.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index edf10618fe2b..8c1eaa74fa21 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -830,7 +830,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
goto error;
}
  
-	ret = -ENOMEM;

+   ret = -ENOSPC;
if (!type_found) {
pr_err(TTM_PFX "No compatible memory type found\n");
ret = -EINVAL;
@@ -916,6 +916,9 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
return -EINVAL;
  
  	ret = ttm_bo_move_buffer(bo, placement, ctx);

+   /* For backward compatibility with userspace */
+   if (ret == -ENOSPC)
+   return -ENOMEM;
if (ret)
return ret;
  


Re: [PATCH 2/3] drm/i915: Extract code required to calculate max qgv/psf gv point

2024-01-17 Thread Lisovskiy, Stanislav
On Fri, Jan 12, 2024 at 07:35:24PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 28, 2023 at 10:37:53AM +0200, Stanislav Lisovskiy wrote:
> > We need that in order to force disable SAGV in next patch.
> > Also it is beneficial to separate that code, as in majority cases,
> > when SAGV is enabled, we don't even need those calculations.
> > Also we probably need to determine max PSF GV point as well, however
> > currently we don't do that when we disable SAGV, which might be
> > actually causing some issues in that case.
> > 
> > Signed-off-by: Stanislav Lisovskiy 
> > ---
> >  drivers/gpu/drm/i915/display/intel_bw.c | 82 -
> >  1 file changed, 65 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
> > b/drivers/gpu/drm/i915/display/intel_bw.c
> > index 583cd2ebdf89..efd408e96e8a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -805,6 +805,64 @@ intel_atomic_get_bw_state(struct intel_atomic_state 
> > *state)
> > return to_intel_bw_state(bw_state);
> >  }
> >  
> > +static unsigned int icl_max_bw_qgv_point(struct drm_i915_private *i915,
> > +int num_active_planes)
> > +{
> > +   unsigned int max_bw_point = 0;
> > +   unsigned int max_bw = 0;
> > +   unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
> > +   int i;
> > +
> > +   for (i = 0; i < num_qgv_points; i++) {
> > +   unsigned int idx;
> > +   unsigned int max_data_rate;
> > +
> > +   if (DISPLAY_VER(i915) > 11)
> > +   idx = tgl_max_bw_index(i915, num_active_planes, i);
> > +   else
> > +   idx = icl_max_bw_index(i915, num_active_planes, i);
> > +
> > +   if (idx >= ARRAY_SIZE(i915->display.bw.max))
> > +   continue;
> > +
> > +   max_data_rate = i915->display.bw.max[idx].deratedbw[i];
> 
> Looks like that that part could be extracted to a helper
> to be used by both codepaths (would be a natural counterpart
> to adl_psf_bw()).
> 
> > +
> > +   /*
> > +* We need to know which qgv point gives us
> > +* maximum bandwidth in order to disable SAGV
> > +* if we find that we exceed SAGV block time
> > +* with watermarks. By that moment we already
> > +* have those, as it is calculated earlier in
> > +* intel_atomic_check,
> > +*/
> > +   if (max_data_rate > max_bw) {
> > +   max_bw_point = i;
> > +   max_bw = max_data_rate;
> > +   }
> > +   }
> > +
> > +   return max_bw_point;
> > +}
> > +
> > +unsigned int icl_max_bw_psf_gv_point(struct drm_i915_private *i915)
> > +{
> > +   unsigned int num_psf_gv_points = 
> > i915->display.bw.max[0].num_psf_gv_points;
> > +   unsigned int max_bw = 0;
> > +   unsigned int max_bw_point = 0;
> > +   int i;
> > +
> > +   for (i = 0; i < num_psf_gv_points; i++) {
> > +   unsigned int max_data_rate = adl_psf_bw(i915, i);
> > +
> > +   if (max_data_rate > max_bw) {
> > +   max_bw_point = i;
> > +   max_bw = max_data_rate;
> > +   }
> > +   }
> > +
> > +   return max_bw_point;
> > +}
> > +
> >  static int mtl_find_qgv_points(struct drm_i915_private *i915,
> >unsigned int data_rate,
> >unsigned int num_active_planes,
> > @@ -882,8 +940,6 @@ static int icl_find_qgv_points(struct drm_i915_private 
> > *i915,
> >const struct intel_bw_state *old_bw_state,
> >struct intel_bw_state *new_bw_state)
> >  {
> > -   unsigned int max_bw_point = 0;
> > -   unsigned int max_bw = 0;
> > unsigned int num_psf_gv_points = 
> > i915->display.bw.max[0].num_psf_gv_points;
> > unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
> > u16 psf_points = 0;
> > @@ -909,18 +965,6 @@ static int icl_find_qgv_points(struct drm_i915_private 
> > *i915,
> >  
> > max_data_rate = i915->display.bw.max[idx].deratedbw[i];
> >  
> > -   /*
> > -* We need to know which qgv point gives us
> > -* maximum bandwidth in order to disable SAGV
> > -* if we find that we exceed SAGV block time
> > -* with watermarks. By that moment we already
> > -* have those, as it is calculated earlier in
> > -* intel_atomic_check,
> > -*/
> > -   if (max_data_rate > max_bw) {
> > -   max_bw_point = i;
> > -   max_bw = max_data_rate;
> > -   }
> > if (max_data_rate >= data_rate)
> > qgv_points |= BIT(i);
> >  
> > @@ -964,9 +1008,13 @@ static int icl_find_qgv_points(struct 
> > drm_i915_private *i915,
> >  * cause.
> >  */
> > if (!intel_can_enable_sagv(i915, new_bw_state)) {
> > -

[PATCH] drm/i915: move interrupt save/restore into vblank section helpers

2024-01-17 Thread Luca Coelho
In all cases when we call the new helper functions, we save/restore
the interrupts, so we can move this to the helpers themselves.  This
improves the semantics of the helper functions by having all
functionality needed to keep the section tight.

This makes a slight functional change by calling the irq_save/restore
functions twice in intel_crtc_update_active_timings().  This shouldn't
be a problem because nesting irq_save/restore calls is safe.
Nevertheless, the commit that originally introduced these helper
functions did not include the irq_save/restore calls in the helpers
themselves because of this exact, though minimal, functional change.

Cc: Rodrigo Vivi 
Cc: Tvrtko Ursulin 
Signed-off-by: Luca Coelho 
---
 drivers/gpu/drm/i915/display/intel_vblank.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
b/drivers/gpu/drm/i915/display/intel_vblank.c
index fe256bf7b485..57ace171a94f 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -266,9 +266,10 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int 
scanline)
 }
 
 /*
- * The uncore version of the spin lock functions is used to decide
- * whether we need to lock the uncore lock or not.  This is only
- * needed in i915, not in Xe.
+ * These functions help enter and exit vblank critical sections.  When
+ * entering, they disable interrupts and, for i915, acquire the
+ * uncore's spinlock.  Conversely, when exiting, they release the
+ * spinlock and restore the interrupts state.
  *
  * This lock in i915 is needed because some old platforms (at least
  * IVB and possibly HSW as well), which are not supported in Xe, need
@@ -278,6 +279,7 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int 
scanline)
 static void intel_vblank_section_enter(struct drm_i915_private *i915)
__acquires(i915->uncore.lock)
 {
+   local_irq_save(irqflags);
 #ifdef I915
spin_lock(>uncore.lock);
 #endif
@@ -289,6 +291,7 @@ static void intel_vblank_section_exit(struct 
drm_i915_private *i915)
 #ifdef I915
spin_unlock(>uncore.lock);
 #endif
+   local_irq_restore(irqflags);
 }
 
 static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
@@ -332,7 +335,6 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
 * timing critical raw register reads, potentially with
 * preemption disabled, so the following code must not block.
 */
-   local_irq_save(irqflags);
intel_vblank_section_enter(dev_priv);
 
/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
@@ -402,7 +404,6 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
/* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
 
intel_vblank_section_exit(dev_priv);
-   local_irq_restore(irqflags);
 
/*
 * While in vblank, position will be negative
@@ -440,13 +441,11 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
unsigned long irqflags;
int position;
 
-   local_irq_save(irqflags);
intel_vblank_section_enter(dev_priv);
 
position = __intel_get_crtc_scanline(crtc);
 
intel_vblank_section_exit(dev_priv);
-   local_irq_restore(irqflags);
 
return position;
 }
@@ -569,6 +568,13 @@ void intel_crtc_update_active_timings(const struct 
intel_crtc_state *crtc_state,
 * Need to audit everything to make sure it's safe.
 */
spin_lock_irqsave(>drm.vblank_time_lock, irqflags);
+
+   /*
+* At this point we have already disabled interrupts, and
+* intel_vblank_section_enter() does that too.  But the
+* nesting is safe here, so it shouldn't be a problem to do it
+* twice.
+   */
intel_vblank_section_enter(i915);
 
drm_calc_timestamping_constants(>base, _mode);
-- 
2.39.2



Re: [PATCH v2 2/4] drm/uAPI: Add "force color format" drm property as setting for userspace

2024-01-17 Thread Pekka Paalanen
On Tue, 16 Jan 2024 14:11:43 +
Andri Yngvason  wrote:

> þri., 16. jan. 2024 kl. 13:29 skrifaði Sebastian Wick
> :
> >
> > On Tue, Jan 16, 2024 at 01:13:13PM +, Andri Yngvason wrote:  
> [...]
> > > şri., 16. jan. 2024 kl. 11:42 skrifaği Sebastian Wick
> > > :  
> > > >
> > > > On Mon, Jan 15, 2024 at 04:05:52PM +, Andri Yngvason wrote:  
> > > > > From: Werner Sembach 
> > > > >
> > > > > Add a new general drm property "force color format" which can be used
> > > > > by userspace to tell the graphics driver which color format to use.  
> > > >
> > > > I don't like the "force" in the name. This just selects the color
> > > > format, let's just call it "color format" then.
> > > >  
> > >
> > > In previous revisions, this was "preferred color format" and "actual
> > > color format", of which the latter has been dropped. I recommend
> > > reading the discussion for previous revisions.  
> >
> > Please don't imply that I didn't read the thread I'm answering to.

FYI, I have not read this thread.

> >  
> > > There are arguments for adding "actual color format" later and if it
> > > is added later, we'd end up with "color format" and "actual color
> > > format", which might be confusing, and it is why I chose to call it
> > > "force color format" because it clearly communicates intent and
> > > disambiguates it from "actual color format".  
> >
> > There is no such thing as "actual color format" in upstream though.
> > Basing your naming on discarded ideas is not useful. The thing that sets
> > the color space for example is called "Colorspace", not "force
> > colorspace".
> >  
> 
> Sure, I'm happy with calling it whatever people want. Maybe we can
> have a vote on it?

It would sound strange to say "force color format" = "auto". Just drop
the "force" of it.

If and when we need the feedback counterpart, it could be an immutable
prop called "active color format" where "auto" is not a valid value, or
something in the new "output properties" design Sima has been thinking
of.

> > > [...]  
> > > > > @@ -1396,6 +1404,15 @@ static const u32 dp_colorspaces =
> > > > >   *   drm_connector_attach_max_bpc_property() to create and attach the
> > > > >   *   property to the connector during initialization.
> > > > >   *
> > > > > + * force color format:
> > > > > + *   This property is used by userspace to change the used color 
> > > > > format. When
> > > > > + *   used the driver will use the selected format if valid for the 
> > > > > hardware,  
> > > >
> > > > All properties are always "used", they just can have different values.
> > > > You probably want to talk about the auto mode here.  
> > >
> > > Maybe we can say something like: If userspace does not set the
> > > property or if it is explicitly set to zero, the driver will select
> > > the appropriate color format based on other constraints.  
> >
> > The property can be in any state without involvement from user space.
> > Don't talk about setting it, talk about the state it is in:
> >
> >   When the color format is auto, the driver will select a format.
> >  
> 
> Ok.
> 
> > > >  
> > > > > + *   sink, and current resolution and refresh rate combination. 
> > > > > Drivers to  
> > > >
> > > > If valid? So when a value is not actually supported user space can still
> > > > set it? What happens then? How should user space figure out if the
> > > > driver and the sink support the format?  
> > >
> > > The kernel does not expose this property unless it's implemented in the 
> > > driver.  
> >
> > If the driver simply doesn't support *one format*, the enum value for
> > that format should not be exposed, period. This isn't about the property
> > on its own.  
> 
> Right, understood. You mean that enum should only contain values that
> are supported by the driver.

Yes. When a driver installs a property, it can choose which of the enum
entries are exposed. That cannot be changed later though, so the list
cannot live by the currently connected sink, only by what the driver
and display controlled could ever do.

> > > This was originally "preferred color format". Perhaps the
> > > documentation should better reflect that it is now a mandatory
> > > constraint which fails the modeset if not satisfied.  
> >
> > That would definitely help.
> >  
> > > >
> > > > For the Colorspace prop, the kernel just exposes all formats it supports
> > > > (independent of the sink) and then makes it the job of user space to
> > > > figure out if the sink supports it.
> > > >
> > > > The same could be done here. Property value is exposed if the driver
> > > > supports it in general, commits can fail if the driver can't support it
> > > > for a specific commit because e.g. the resolution or refresh rate. User
> > > > space must look at the EDID/DisplayID/mode to figure out the supported
> > > > format for the sink.  
> > >
> > > Yes, we can make it possible for userspace to discover which modes are
> > > supported by the monitor, but there are other constraints that 

Re: [PATCH] Revert "drm/i915/dsi: Do display on sequence later on icl+"

2024-01-17 Thread Jani Nikula
On Tue, 16 Jan 2024, Ville Syrjala  wrote:
> From: Ville Syrjälä 
>
> This reverts commit 88b065943cb583e890324d618e8d4b23460d51a3.
>
> Lenovo 82TQ is unhappy if we do the display on sequence this
> late. The display output shows severe corruption.
>
> It's unclear if this is a failure on our part (perhaps
> something to do with sending commands in LP mode after HS
> /video mode transmission has been started? Though the backlight
> on command at least seems to work) or simply that there are
> some commands in the sequence that are needed to be done
> earlier (eg. could be some DSC init stuff?). If the latter
> then I don't think the current Windows code would work
> either, but maybe this was originally tested with an older
> driver, who knows.
>
> Root causing this fully would likely require a lot of
> experimentation which isn't really feasible without direct
> access to the machine, so let's just accept failure and
> go back to the original sequence.
>
> Cc: sta...@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10071
> Signed-off-by: Ville Syrjälä 

Acked-by: Jani Nikula 

> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c 
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index ac456a2275db..eda4a8b88590 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1155,6 +1155,7 @@ static void gen11_dsi_powerup_panel(struct 
> intel_encoder *encoder)
>   }
>  
>   intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
> + intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
>  
>   /* ensure all panel commands dispatched before enabling transcoder */
>   wait_for_cmds_dispatched_to_panel(encoder);
> @@ -1255,8 +1256,6 @@ static void gen11_dsi_enable(struct intel_atomic_state 
> *state,
>   /* step6d: enable dsi transcoder */
>   gen11_dsi_enable_transcoder(encoder);
>  
> - intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
> -
>   /* step7: enable backlight */
>   intel_backlight_enable(crtc_state, conn_state);
>   intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);

-- 
Jani Nikula, Intel


Re: [PATCH 3/5] drm/ttm: replace busy placement with flags v6

2024-01-17 Thread Thomas Zimmermann



Am 12.01.24 um 13:51 schrieb Christian König:

From: Somalapuram Amaranath 

Instead of a list of separate busy placement add flags which indicate
that a placement should only be used when there is room or if we need to
evict.

v2: add missing TTM_PL_FLAG_IDLE for i915
v3: fix auto build test ERROR on drm-tip/drm-tip
v4: fix some typos pointed out by checkpatch
v5: cleanup some rebase problems with VMWGFX
v6: implement some missing VMWGFX functionality pointed out by Zack,
 rename the flags as suggested by Michel, rebase on drm-tip and
 adjust XE as well

Signed-off-by: Christian König 
Signed-off-by: Somalapuram Amaranath 


For the VRAM helpers:

Reviewed-by: Thomas Zimmermann 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 11 +---
  drivers/gpu/drm/drm_gem_vram_helper.c  |  2 -
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 37 +--
  drivers/gpu/drm/loongson/lsdc_ttm.c|  2 -
  drivers/gpu/drm/nouveau/nouveau_bo.c   | 59 +++--
  drivers/gpu/drm/nouveau/nouveau_bo.h   |  1 -
  drivers/gpu/drm/qxl/qxl_object.c   |  2 -
  drivers/gpu/drm/qxl/qxl_ttm.c  |  2 -
  drivers/gpu/drm/radeon/radeon_object.c |  2 -
  drivers/gpu/drm/radeon/radeon_ttm.c|  8 +--
  drivers/gpu/drm/radeon/radeon_uvd.c|  1 -
  drivers/gpu/drm/ttm/ttm_bo.c   | 21 ---
  drivers/gpu/drm/ttm/ttm_resource.c | 73 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 33 +++---
  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  4 --
  drivers/gpu/drm/xe/xe_bo.c | 33 +-
  include/drm/ttm/ttm_placement.h| 10 +--
  include/drm/ttm/ttm_resource.h |  8 +--
  19 files changed, 118 insertions(+), 197 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 425cebcc5cbf..b671b0665492 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -220,9 +220,6 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, 
u32 domain)
  
  	placement->num_placement = c;

placement->placement = places;
-
-   placement->num_busy_placement = c;
-   placement->busy_placement = places;
  }
  
  /**

@@ -1397,8 +1394,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct 
ttm_buffer_object *bo)
AMDGPU_GEM_DOMAIN_GTT);
  
  	/* Avoid costly evictions; only set GTT as a busy placement */

-   abo->placement.num_busy_placement = 1;
-   abo->placement.busy_placement = >placements[1];
+   abo->placements[0].flags |= TTM_PL_FLAG_DESIRED;
  
  	r = ttm_bo_validate(bo, >placement, );

if (unlikely(r == -EBUSY || r == -ERESTARTSYS))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 75c9fd2c6c2a..8722beba494e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -102,23 +102,19 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
*bo,
/* Don't handle scatter gather BOs */
if (bo->type == ttm_bo_type_sg) {
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
}
  
  	/* Object isn't an AMDGPU object so ignore */

if (!amdgpu_bo_is_amdgpu_bo(bo)) {
placement->placement = 
-   placement->busy_placement = 
placement->num_placement = 1;
-   placement->num_busy_placement = 1;
return;
}
  
  	abo = ttm_to_amdgpu_bo(bo);

if (abo->flags & AMDGPU_GEM_CREATE_DISCARDABLE) {
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
}
  
@@ -128,13 +124,13 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,

case AMDGPU_PL_OA:
case AMDGPU_PL_DOORBELL:
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
  
  	case TTM_PL_VRAM:

if (!adev->mman.buffer_funcs_enabled) {
/* Move to system memory */
amdgpu_bo_placement_from_domain(abo, 
AMDGPU_GEM_DOMAIN_CPU);
+
} else if (!amdgpu_gmc_vram_full_visible(>gmc) &&
   !(abo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) 
&&
   amdgpu_bo_in_cpu_visible_vram(abo)) {
@@ -149,8 +145,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
AMDGPU_GEM_DOMAIN_CPU);
abo->placements[0].fpfn = adev->gmc.visible_vram_size 
>> PAGE_SHIFT;
abo->placements[0].lpfn = 0;
-   abo->placement.busy_placement = >placements[1];
-  

✓ Fi.CI.BAT: success for drm/i915: Cursor vblank evasion (rev3)

2024-01-17 Thread Patchwork
== Series Details ==

Series: drm/i915: Cursor vblank evasion (rev3)
URL   : https://patchwork.freedesktop.org/series/127744/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_14129 -> Patchwork_127744v3


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v3/index.html

Participating hosts (39 -> 37)
--

  Missing(2): bat-rpls-2 fi-snb-2520m 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_127744v3:

### IGT changes ###

 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@execlists:
- {bat-adls-6}:   [PASS][1] -> [TIMEOUT][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14129/bat-adls-6/igt@i915_selftest@l...@execlists.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v3/bat-adls-6/igt@i915_selftest@l...@execlists.html

  
Known issues


  Here are the changes found in Patchwork_127744v3 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@core_hotunplug@unbind-rebind:
- bat-adlp-6: [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14129/bat-adlp-6/igt@core_hotunp...@unbind-rebind.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v3/bat-adlp-6/igt@core_hotunp...@unbind-rebind.html

  * igt@i915_selftest@live@hangcheck:
- fi-skl-guc: [PASS][5] -> [DMESG-FAIL][6] ([i915#10112])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14129/fi-skl-guc/igt@i915_selftest@l...@hangcheck.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v3/fi-skl-guc/igt@i915_selftest@l...@hangcheck.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-dp-1:
- bat-dg2-8:  [PASS][7] -> [INCOMPLETE][8] ([i915#9280])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14129/bat-dg2-8/igt@kms_pipe_crc_basic@suspend-read-...@pipe-b-dp-1.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v3/bat-dg2-8/igt@kms_pipe_crc_basic@suspend-read-...@pipe-b-dp-1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#10112]: https://gitlab.freedesktop.org/drm/intel/issues/10112
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#9280]: https://gitlab.freedesktop.org/drm/intel/issues/9280
  [i915#9943]: https://gitlab.freedesktop.org/drm/intel/issues/9943


Build changes
-

  * Linux: CI_DRM_14129 -> Patchwork_127744v3

  CI-20190529: 20190529
  CI_DRM_14129: b6b50ad4c8d61b14de0ffcf0d52ae2adc0ef39cf @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7675: ffde49e0583ee5053f25a065356bce6bce91047a @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_127744v3: b6b50ad4c8d61b14de0ffcf0d52ae2adc0ef39cf @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

5540e8a678ff Revert "drm/i915/xe2lpd: Treat cursor plane as regular plane for 
DDB allocation"
629c34748213 drm/i915: Perform vblank evasion around legacy cursor updates
18df1fe0b300 drm/i915: Move intel_vblank_evade() & co. into intel_vblank.c
1a40377765af drm/i915: Move the min/max scanline sanity check into 
intel_vblank_evade()
03003b9bdff4 drm/i915: Extract intel_vblank_evade()
fb121f57f07d drm/i915: Include need_vlv_dsi_wa in intel_vblank_evade_ctx
6a323300dd09 drm/i915: Introduce struct intel_vblank_evade_ctx
d8be1b741b18 drm/i915: Reorder drm_vblank_put() vs. need_vlv_dsi_wa
49c2682be890 drm/i915: Decouple intel_crtc_vblank_evade_scanlines() from atomic 
commits

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v3/index.html


✗ Fi.CI.BAT: failure for drm/i915/lnl: Block pkgC entry for performance improvemnt

2024-01-17 Thread Patchwork
== Series Details ==

Series: drm/i915/lnl: Block pkgC entry for performance improvemnt
URL   : https://patchwork.freedesktop.org/series/128863/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14129 -> Patchwork_128863v1


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_128863v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_128863v1, please notify your bug team 
(i915-ci-in...@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128863v1/index.html

Participating hosts (39 -> 36)
--

  Missing(3): bat-dg2-8 fi-snb-2520m bat-mtlp-8 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_128863v1:

### IGT changes ###

 Possible regressions 

  * igt@i915_selftest@live@gt_lrc:
- bat-adln-1: [PASS][1] -> [INCOMPLETE][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14129/bat-adln-1/igt@i915_selftest@live@gt_lrc.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128863v1/bat-adln-1/igt@i915_selftest@live@gt_lrc.html

  
Known issues


  Here are the changes found in Patchwork_128863v1 that come from known issues:

### CI changes ###

 Issues hit 

  * boot:
- bat-jsl-1:  [PASS][3] -> [FAIL][4] ([i915#8293])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14129/bat-jsl-1/boot.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128863v1/bat-jsl-1/boot.html

  

### IGT changes ###

 Issues hit 

  * igt@i915_module_load@reload:
- bat-adlp-6: [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14129/bat-adlp-6/igt@i915_module_l...@reload.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128863v1/bat-adlp-6/igt@i915_module_l...@reload.html

  
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293


Build changes
-

  * Linux: CI_DRM_14129 -> Patchwork_128863v1

  CI-20190529: 20190529
  CI_DRM_14129: b6b50ad4c8d61b14de0ffcf0d52ae2adc0ef39cf @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7675: ffde49e0583ee5053f25a065356bce6bce91047a @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_128863v1: b6b50ad4c8d61b14de0ffcf0d52ae2adc0ef39cf @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

8787b230b13c drm/i915/lnl: Block pkgC entry for performance improvemnt

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128863v1/index.html