Re: [Intel-gfx] [PATCH v2] mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Chris Wilson
Quoting Andrew Morton (2020-08-21 23:34:12)
> On Fri, 21 Aug 2020 14:37:46 +0200 Joerg Roedel  wrote:
> 
> > The __apply_to_page_range() function is also used to change and/or
> > allocate page-table pages in the vmalloc area of the address space.
> > Make sure these changes get synchronized to other page-tables in the
> > system by calling arch_sync_kernel_mappings() when necessary.
> > 
> > Tested-by: Chris Wilson  #x86-32
> > Cc:  # v5.8+
> 
> I'm trying to figure out how you figured out that this is 5.8+.  Has a
> particular misbehaving commit been identified?

The two commits of relevance, in my eyes, were

  2ba3e6947aed ("mm/vmalloc: track which page-table levels were modified")
  86cf69f1d893 ("x86/mm/32: implement arch_sync_kernel_mappings()")

I can reproduce the failure on v5.8, but not on v5.7. A bisect would
seem to be plausible.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Andrew Morton
On Fri, 21 Aug 2020 14:37:46 +0200 Joerg Roedel  wrote:

> The __apply_to_page_range() function is also used to change and/or
> allocate page-table pages in the vmalloc area of the address space.
> Make sure these changes get synchronized to other page-tables in the
> system by calling arch_sync_kernel_mappings() when necessary.
> 
> Tested-by: Chris Wilson  #x86-32
> Cc:  # v5.8+

I'm trying to figure out how you figured out that this is 5.8+.  Has a
particular misbehaving commit been identified?

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v6 04/11] drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3.

2020-08-21 Thread Navare, Manasi
On Fri, Aug 21, 2020 at 03:11:45PM +0530, Manna, Animesh wrote:
> 
> On 16-07-2020 04:12, Manasi Navare wrote:
> >From: Maarten Lankhorst 
> >
> >Small changes to intel_dp_mode_valid(), allow listing modes that
> >can only be supported in the bigjoiner configuration, which is
> >not supported yet.
> >
> >eDP does not support bigjoiner, so do not expose bigjoiner only
> >modes on the eDP port.
> >
> >v5:
> >* Increase max plane width to support 8K with bigjoiner (Maarten)
> >v4:
> >* Rebase (Manasi)
> >
> >Changes since v1:
> >- Disallow bigjoiner on eDP.
> >Changes since v2:
> >- Rename intel_dp_downstream_max_dotclock to intel_dp_max_dotclock,
> >   and split off the downstream and source checking to its own function.
> >   (Ville)
> >v3:
> >* Rebase (Manasi)
> >
> >Signed-off-by: Manasi Navare 
> >Signed-off-by: Maarten Lankhorst 
> >---
> >  drivers/gpu/drm/i915/display/intel_display.c |   2 +-
> >  drivers/gpu/drm/i915/display/intel_dp.c  | 119 ++-
> >  2 files changed, 91 insertions(+), 30 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> >b/drivers/gpu/drm/i915/display/intel_display.c
> >index 78cbfefbfa62..3ecb642805a6 100644
> >--- a/drivers/gpu/drm/i915/display/intel_display.c
> >+++ b/drivers/gpu/drm/i915/display/intel_display.c
> >@@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct 
> >drm_i915_private *dev_priv,
> >  * too big for that.
> >  */
> > if (INTEL_GEN(dev_priv) >= 11) {
> >-plane_width_max = 5120;
> >+plane_width_max = 7680;
> 
> 
> Other encoder also use this function and big joiner on DP only need this 
> change. Is it good idea to add encoder check? Maybe in a cover-letter can we 
> add some description about big-joiner
> feature and current limitation.
> Overall changes looks good to me, for dsc related code better get a review 
> from someone who has worked before.

Yes I think the plane width max also should only be changed to 7680 if big 
joiner is supported so this needs
some tweak anyways. I will take a look into this.
Also will sync with Maarten on what might be the best approach.

Manasi

> 
> Regards,
> Animesh
> 
> > plane_height_max = 4320;
> > } else {
> > plane_width_max = 5120;
> >diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> >b/drivers/gpu/drm/i915/display/intel_dp.c
> >index d6295eb20b63..fbfea99fd804 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dp.c
> >+++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >@@ -248,25 +248,37 @@ intel_dp_max_data_rate(int max_link_clock, int 
> >max_lanes)
> > return max_link_clock * max_lanes;
> >  }
> >-static int
> >-intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
> >+static int source_max_dotclock(struct intel_dp *intel_dp, bool 
> >allow_bigjoiner)
> >  {
> >-struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> >-struct intel_encoder *encoder = _port->base;
> >+struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >+struct intel_encoder *encoder = _dig_port->base;
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >-int max_dotclk = dev_priv->max_dotclk_freq;
> >-int ds_max_dotclk;
> >+if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && 
> >!intel_dp_is_edp(intel_dp))
> >+return 2 * dev_priv->max_dotclk_freq;
> >+
> >+return dev_priv->max_dotclk_freq;
> >+}
> >+
> >+static int downstream_max_dotclock(struct intel_dp *intel_dp)
> >+{
> > int type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> > if (type != DP_DS_PORT_TYPE_VGA)
> >-return max_dotclk;
> >+return 0;
> >-ds_max_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> >-intel_dp->downstream_ports);
> >+return drm_dp_downstream_max_clock(intel_dp->dpcd,
> >+   intel_dp->downstream_ports);
> >+}
> >+
> >+static int
> >+intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> >+{
> >+int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> >+int ds_max_dotclk = downstream_max_dotclock(intel_dp);
> > if (ds_max_dotclk != 0)
> >-max_dotclk = min(max_dotclk, ds_max_dotclk);
> >+return min(max_dotclk, ds_max_dotclk);
> > return max_dotclk;
> >  }
> >@@ -527,7 +539,8 @@ small_joiner_ram_size_bits(struct drm_i915_private *i915)
> >  static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
> >u32 link_clock, u32 lane_count,
> >-   u32 mode_clock, u32 mode_hdisplay)
> >+   u32 mode_clock, u32 mode_hdisplay,
> >+   bool bigjoiner)
> >  {
> > u32 bits_per_pixel, max_bpp_small_joiner_ram;
> > int i;
> >@@ -545,6 +558,10 @@ static u16 intel_dp_dsc_get_output_bpp(struct 
> >drm_i915_private 

Re: [Intel-gfx] [PATCH v2] mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Pavel Machek
Hi!

> > > The __apply_to_page_range() function is also used to change and/or
> > > allocate page-table pages in the vmalloc area of the address space.
> > > Make sure these changes get synchronized to other page-tables in the
> > > system by calling arch_sync_kernel_mappings() when necessary.
> > 
> > There's no description here of the user-visible effects of the bug. 
> > Please always provide this, especially when proposing a -stable
> > backport.  Take pity upon all the downstream kernel maintainers who are
> > staring at this wondering whether they should risk adding it to their
> > kernels.
> 
> The impact appears limited to x86-32, where apply_to_page_range may miss
> updating the PMD. That leads to explosions in drivers like
> 
> [   24.227844] BUG: unable to handle page fault for address: fe036000
> [   24.228076] #PF: supervisor write access in kernel mode
> [   24.228294] #PF: error_code(0x0002) - not-present page
> [   24.228494] *pde = 
> [   24.228640] Oops: 0002 [#1] SMP
> [   24.228788] CPU: 3 PID: 1300 Comm: gem_concurrent_ Not tainted 5.9.0-rc1+ 
> #16
> [   24.228957] Hardware name:  /NUC6i3SYB, BIOS 
> SYSKLi35.86A.0024.2015.1027.2142 10/27/2015
> [   24.229297] EIP: __execlists_context_alloc+0x132/0x2d0 [i915]
> [   24.229462] Code: 31 d2 89 f0 e8 2f 55 02 00 89 45 e8 3d 00 f0 ff ff 0f 87 
> 11 01 00 00 8b 4d e8 03 4b 30 b8 5a 5a 5a 5a ba 01 00 00 00 8d 79 04  01 
> 5a 5a 5a 5a c7 81 fc 0f 00 00 5a 5a 5a 5a 83 e7 fc 29 f9 81
> [   24.229759] EAX: 5a5a5a5a EBX: f60ca000 ECX: fe036000 EDX: 0001
> [   24.229915] ESI: f43b7340 EDI: fe036004 EBP: f6389cb8 ESP: f6389c9c
> [   24.230072] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00010286
> [   24.230229] CR0: 80050033 CR2: fe036000 CR3: 2d361000 CR4: 001506d0
> [   24.230385] DR0:  DR1:  DR2:  DR3: 
> [   24.230539] DR6: fffe0ff0 DR7: 0400
> [   24.230675] Call Trace:
> [   24.230957]  execlists_context_alloc+0x10/0x20 [i915]
> [   24.231266]  intel_context_alloc_state+0x3f/0x70 [i915]
> [   24.231547]  __intel_context_do_pin+0x117/0x170 [i915]
> [   24.231850]  i915_gem_do_execbuffer+0xcc7/0x2500 [i915]
> [   24.232024]  ? __kmalloc_track_caller+0x54/0x230
> [   24.232181]  ? ktime_get+0x3e/0x120
> [   24.232333]  ? dma_fence_signal+0x34/0x50
> [   24.232617]  i915_gem_execbuffer2_ioctl+0xcd/0x1f0 [i915]
> [   24.232912]  ? i915_gem_execbuffer_ioctl+0x2e0/0x2e0 [i915]
> [   24.233084]  drm_ioctl_kernel+0x8f/0xd0
> [   24.233236]  drm_ioctl+0x223/0x3d0
> [   24.233505]  ? i915_gem_execbuffer_ioctl+0x2e0/0x2e0 [i915]
> [   24.233684]  ? pick_next_task_fair+0x1b5/0x3d0
> [   24.233873]  ? __switch_to_asm+0x36/0x50
> [   24.234021]  ? drm_ioctl_kernel+0xd0/0xd0
> [   24.234167]  __ia32_sys_ioctl+0x1ab/0x760
> [   24.234313]  ? exit_to_user_mode_prepare+0xe5/0x110
> [   24.234453]  ? syscall_exit_to_user_mode+0x23/0x130
> [   24.234601]  __do_fast_syscall_32+0x3f/0x70
> [   24.234744]  do_fast_syscall_32+0x29/0x60
> [   24.234885]  do_SYSENTER_32+0x15/0x20
> [   24.235021]  entry_SYSENTER_32+0x9f/0xf2
> [   24.235157] EIP: 0xb7f28559
> [   24.235288] Code: 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 
> 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 
> 59 c3 90 90 90 90 8d 76 00 58 b8 77 00 00 00 cd 80 90 8d 76
> [   24.235576] EAX: ffda EBX: 0005 ECX: c0406469 EDX: bf95556c
> [   24.235722] ESI: b7e68000 EDI: c0406469 EBP: 0005 ESP: bf9554d8
> [   24.235869] DS: 007b ES: 007b FS:  GS: 0033 SS: 007b EFLAGS: 0296
> [   24.236018] Modules linked in: i915 x86_pkg_temp_thermal intel_powerclamp 
> crc32_pclmul crc32c_intel intel_cstate intel_uncore intel_gtt drm_kms_helper 
> intel_pch_thermal video button autofs4 i2c_i801 i2c_smbus fan
> [   24.236336] CR2: fe036000
> 
> It looks like kasan, xen and i915 are vulnerable.

And actual impact is "on thinkpad X60 in 5.9-rc1, screen starts
blinking after 30-or-so minutes, and macine is unusable"... that is
assuming we are taking same bug.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Chris Wilson
Quoting Andrew Morton (2020-08-21 21:35:48)
> On Fri, 21 Aug 2020 14:37:46 +0200 Joerg Roedel  wrote:
> 
> > The __apply_to_page_range() function is also used to change and/or
> > allocate page-table pages in the vmalloc area of the address space.
> > Make sure these changes get synchronized to other page-tables in the
> > system by calling arch_sync_kernel_mappings() when necessary.
> 
> There's no description here of the user-visible effects of the bug. 
> Please always provide this, especially when proposing a -stable
> backport.  Take pity upon all the downstream kernel maintainers who are
> staring at this wondering whether they should risk adding it to their
> kernels.

The impact appears limited to x86-32, where apply_to_page_range may miss
updating the PMD. That leads to explosions in drivers like

[   24.227844] BUG: unable to handle page fault for address: fe036000
[   24.228076] #PF: supervisor write access in kernel mode
[   24.228294] #PF: error_code(0x0002) - not-present page
[   24.228494] *pde = 
[   24.228640] Oops: 0002 [#1] SMP
[   24.228788] CPU: 3 PID: 1300 Comm: gem_concurrent_ Not tainted 5.9.0-rc1+ #16
[   24.228957] Hardware name:  /NUC6i3SYB, BIOS 
SYSKLi35.86A.0024.2015.1027.2142 10/27/2015
[   24.229297] EIP: __execlists_context_alloc+0x132/0x2d0 [i915]
[   24.229462] Code: 31 d2 89 f0 e8 2f 55 02 00 89 45 e8 3d 00 f0 ff ff 0f 87 
11 01 00 00 8b 4d e8 03 4b 30 b8 5a 5a 5a 5a ba 01 00 00 00 8d 79 04  01 5a 
5a 5a 5a c7 81 fc 0f 00 00 5a 5a 5a 5a 83 e7 fc 29 f9 81
[   24.229759] EAX: 5a5a5a5a EBX: f60ca000 ECX: fe036000 EDX: 0001
[   24.229915] ESI: f43b7340 EDI: fe036004 EBP: f6389cb8 ESP: f6389c9c
[   24.230072] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00010286
[   24.230229] CR0: 80050033 CR2: fe036000 CR3: 2d361000 CR4: 001506d0
[   24.230385] DR0:  DR1:  DR2:  DR3: 
[   24.230539] DR6: fffe0ff0 DR7: 0400
[   24.230675] Call Trace:
[   24.230957]  execlists_context_alloc+0x10/0x20 [i915]
[   24.231266]  intel_context_alloc_state+0x3f/0x70 [i915]
[   24.231547]  __intel_context_do_pin+0x117/0x170 [i915]
[   24.231850]  i915_gem_do_execbuffer+0xcc7/0x2500 [i915]
[   24.232024]  ? __kmalloc_track_caller+0x54/0x230
[   24.232181]  ? ktime_get+0x3e/0x120
[   24.232333]  ? dma_fence_signal+0x34/0x50
[   24.232617]  i915_gem_execbuffer2_ioctl+0xcd/0x1f0 [i915]
[   24.232912]  ? i915_gem_execbuffer_ioctl+0x2e0/0x2e0 [i915]
[   24.233084]  drm_ioctl_kernel+0x8f/0xd0
[   24.233236]  drm_ioctl+0x223/0x3d0
[   24.233505]  ? i915_gem_execbuffer_ioctl+0x2e0/0x2e0 [i915]
[   24.233684]  ? pick_next_task_fair+0x1b5/0x3d0
[   24.233873]  ? __switch_to_asm+0x36/0x50
[   24.234021]  ? drm_ioctl_kernel+0xd0/0xd0
[   24.234167]  __ia32_sys_ioctl+0x1ab/0x760
[   24.234313]  ? exit_to_user_mode_prepare+0xe5/0x110
[   24.234453]  ? syscall_exit_to_user_mode+0x23/0x130
[   24.234601]  __do_fast_syscall_32+0x3f/0x70
[   24.234744]  do_fast_syscall_32+0x29/0x60
[   24.234885]  do_SYSENTER_32+0x15/0x20
[   24.235021]  entry_SYSENTER_32+0x9f/0xf2
[   24.235157] EIP: 0xb7f28559
[   24.235288] Code: 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 
74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 
c3 90 90 90 90 8d 76 00 58 b8 77 00 00 00 cd 80 90 8d 76
[   24.235576] EAX: ffda EBX: 0005 ECX: c0406469 EDX: bf95556c
[   24.235722] ESI: b7e68000 EDI: c0406469 EBP: 0005 ESP: bf9554d8
[   24.235869] DS: 007b ES: 007b FS:  GS: 0033 SS: 007b EFLAGS: 0296
[   24.236018] Modules linked in: i915 x86_pkg_temp_thermal intel_powerclamp 
crc32_pclmul crc32c_intel intel_cstate intel_uncore intel_gtt drm_kms_helper 
intel_pch_thermal video button autofs4 i2c_i801 i2c_smbus fan
[   24.236336] CR2: fe036000

It looks like kasan, xen and i915 are vulnerable.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from i915 (rev5)

2020-08-21 Thread Patchwork
== Series Details ==

Series: drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from 
i915 (rev5)
URL   : https://patchwork.freedesktop.org/series/80542/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8914_full -> Patchwork_18389_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_18389_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18389_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
---

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

### Piglit changes ###

 Possible regressions 

  * spec@!opengl 1.3@gl-1.3-texture-env:
- pig-snb-2600:   NOTRUN -> [FAIL][1] +26 similar issues
   [1]: None

  
New tests
-

  New tests have been introduced between CI_DRM_8914_full and 
Patchwork_18389_full:

### New Piglit tests (22) ###

  * spec@arb_depth_buffer_float@depthstencil-render-miplevels 1024 ds=z32f_s8:
- Statuses : 1 fail(s)
- Exec time: [0.54] s

  * spec@arb_depth_buffer_float@depthstencil-render-miplevels 1024 
s=z24_s8_d=z32f_s8:
- Statuses : 1 fail(s)
- Exec time: [0.52] s

  * spec@arb_depth_buffer_float@depthstencil-render-miplevels 585 
d=z32f_s8_s=z24_s8:
- Statuses : 1 fail(s)
- Exec time: [9.76] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 1d:
- Statuses : 1 fail(s)
- Exec time: [0.92] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 1darray:
- Statuses : 1 fail(s)
- Exec time: [0.94] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 1darrayshadow:
- Statuses : 1 fail(s)
- Exec time: [1.03] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 1dshadow:
- Statuses : 1 fail(s)
- Exec time: [1.10] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 2d:
- Statuses : 1 fail(s)
- Exec time: [0.90] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 2darray:
- Statuses : 1 fail(s)
- Exec time: [0.82] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 2darrayshadow:
- Statuses : 1 fail(s)
- Exec time: [0.98] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 2dshadow:
- Statuses : 1 fail(s)
- Exec time: [1.03] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() 3d:
- Statuses : 1 fail(s)
- Exec time: [0.92] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() cubearray:
- Statuses : 1 fail(s)
- Exec time: [0.77] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture() cubeshadow:
- Statuses : 1 fail(s)
- Exec time: [0.88] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture(bias) 1darray:
- Statuses : 1 fail(s)
- Exec time: [1.59] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture(bias) 1darrayshadow:
- Statuses : 1 fail(s)
- Exec time: [1.61] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture(bias) 2darray:
- Statuses : 1 fail(s)
- Exec time: [1.52] s

  * spec@glsl-1.30@execution@tex-miplevel-selection texture(bias) cubeshadow:
- Statuses : 1 fail(s)
- Exec time: [1.28] s

  * spec@glsl-1.30@execution@tex-miplevel-selection textureoffset 2d:
- Statuses : 1 fail(s)
- Exec time: [0.16] s

  * spec@glsl-1.50@execution@built-in-functions@gs-mix-float-float-float:
- Statuses : 1 fail(s)
- Exec time: [0.14] s

  * spec@glsl-1.50@execution@built-in-functions@gs-op-add-ivec4-ivec4:
- Statuses : 1 fail(s)
- Exec time: [0.09] s

  * spec@glsl-1.50@execution@built-in-functions@gs-op-bitxor-neg-int-ivec3:
- Statuses : 1 fail(s)
- Exec time: [0.10] s

  

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_caching@read-writes:
- shard-skl:  [PASS][2] -> [DMESG-WARN][3] ([i915#1982]) +7 similar 
issues
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8914/shard-skl8/igt@gem_cach...@read-writes.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18389/shard-skl4/igt@gem_cach...@read-writes.html

  * igt@gem_exec_create@forked:
- shard-glk:  [PASS][4] -> [DMESG-WARN][5] ([i915#118] / [i915#95])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8914/shard-glk9/igt@gem_exec_cre...@forked.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18389/shard-glk9/igt@gem_exec_cre...@forked.html

  * igt@i915_pm_rpm@system-suspend:
- shard-kbl:  [PASS][6] -> [INCOMPLETE][7] ([i915#151] / [i915#155])
   [6]: 

Re: [Intel-gfx] [PATCH v2] mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Andrew Morton
On Fri, 21 Aug 2020 14:37:46 +0200 Joerg Roedel  wrote:

> The __apply_to_page_range() function is also used to change and/or
> allocate page-table pages in the vmalloc area of the address space.
> Make sure these changes get synchronized to other page-tables in the
> system by calling arch_sync_kernel_mappings() when necessary.

There's no description here of the user-visible effects of the bug. 
Please always provide this, especially when proposing a -stable
backport.  Take pity upon all the downstream kernel maintainers who are
staring at this wondering whether they should risk adding it to their
kernels.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [RFC v2 13/20] drm/i915/dp: Extract drm_dp_downstream_read_info()

2020-08-21 Thread Sean Paul
On Thu, Aug 20, 2020 at 2:31 PM Lyude Paul  wrote:
>
> We're going to be doing the same probing process in nouveau for
> determining downstream DP port capabilities, so let's deduplicate the
> work by moving i915's code for handling this into a shared helper:
> drm_dp_downstream_read_info().
>
> Note that when we do this, we also do make some functional changes while
> we're at it:
> * We always clear the downstream port info before trying to read it,
>   just to make things easier for the caller
> * We skip reading downstream port info if the DPCD indicates that we
>   don't support downstream port info
> * We only read as many bytes as needed for the reported number of
>   downstream ports, no sense in reading the whole thing every time
>
> v2:
> * Fixup logic for calculating the downstream port length to account for
>   the fact that downstream port caps can be either 1 byte or 4 bytes
>   long. We can actually skip fixing the max_clock/max_bpc helpers here
>   since they all check for DP_DETAILED_CAP_INFO_AVAILABLE anyway.
> * Fix ret code check for drm_dp_dpcd_read
>

Thanks for sorting this out!

Reviewed-by: Sean Paul 

> Signed-off-by: Lyude Paul 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 46 +
>  drivers/gpu/drm/i915/display/intel_dp.c | 14 ++--
>  include/drm/drm_dp_helper.h |  3 ++
>  3 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 4c21cf69dad5a..4f845995f1f66 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -423,6 +423,52 @@ bool drm_dp_send_real_edid_checksum(struct drm_dp_aux 
> *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
>
> +static u8 drm_dp_downstream_port_count(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> +{
> +   u8 port_count = dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_PORT_COUNT_MASK;
> +
> +   if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE 
> && port_count > 4)
> +   port_count = 4;
> +
> +   return port_count;
> +}
> +
> +/**
> + * drm_dp_downstream_read_info() - read DPCD downstream port info if 
> available
> + * @aux: DisplayPort AUX channel
> + * @dpcd: A cached copy of the port's DPCD
> + * @downstream_ports: buffer to store the downstream port info in
> + *
> + * Returns: 0 if either the downstream port info was read successfully or
> + * there was no downstream info to read, or a negative error code otherwise.
> + */
> +int drm_dp_downstream_read_info(struct drm_dp_aux *aux,
> +   const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +   u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS])
> +{
> +   int ret;
> +   u8 len;
> +
> +   memset(downstream_ports, 0, DP_MAX_DOWNSTREAM_PORTS);
> +
> +   /* No downstream info to read */
> +   if (!drm_dp_is_branch(dpcd) ||
> +   dpcd[DP_DPCD_REV] < DP_DPCD_REV_10 ||
> +   !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
> +   return 0;
> +
> +   len = drm_dp_downstream_port_count(dpcd);
> +   if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE)
> +   len *= 4;
> +
> +   ret = drm_dp_dpcd_read(aux, DP_DOWNSTREAM_PORT_0, downstream_ports, 
> len);
> +   if (ret < 0)
> +   return ret;
> +
> +   return ret == len ? 0 : -EIO;
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_read_info);
> +
>  /**
>   * drm_dp_downstream_max_clock() - extract branch device max
>   * pixel rate for legacy VGA
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1e29d3a012856..984e49194ca31 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4685,18 +4685,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> return false;
> }
>
> -   if (!drm_dp_is_branch(intel_dp->dpcd))
> -   return true; /* native DP sink */
> -
> -   if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
> -   return true; /* no per-port downstream info */
> -
> -   if (drm_dp_dpcd_read(_dp->aux, DP_DOWNSTREAM_PORT_0,
> -intel_dp->downstream_ports,
> -DP_MAX_DOWNSTREAM_PORTS) < 0)
> -   return false; /* downstream port status fetch failed */
> -
> -   return true;
> +   return drm_dp_downstream_read_info(_dp->aux, intel_dp->dpcd,
> +  intel_dp->downstream_ports) == 0;
>  }
>
>  static bool
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 5c28199248626..1349f16564ace 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1613,6 +1613,9 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  bool drm_dp_send_real_edid_checksum(struct 

Re: [Intel-gfx] [PATCH v2] mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Linus Torvalds
On Fri, Aug 21, 2020 at 5:38 AM Joerg Roedel  wrote:
>
> From: Joerg Roedel 
>
> The __apply_to_page_range() function is also used to change and/or
> allocate page-table pages in the vmalloc area of the address space.
> Make sure these changes get synchronized to other page-tables in the
> system by calling arch_sync_kernel_mappings() when necessary.

I get the strong feeling that these functions should be using a
"struct apply_details *" or something like that (the way the
zap_page_range() code has that "zap_details" thing).

Because adding more and more arguments gets pretty painful after a
while. But maybe the compiler inlining it all makes it a non-issue.

It also strikes me that I think the only architecture that uses the
whole arch_sync_kernel_mappings() thing is now just x86-32.

[ Well, x86-64 still has it, but that's because we undid the 64-bit
removal, but it's on the verge of going away and x86-64 shouldn't
actually _need_ it any more ]

So all of this seems to be purely for 32-bit x86. Which kind of makes
this all fail the smell test.

But the patch does seem to be the minimal fix for a real issue - I'm
just pointing out ugly details, not actual problems with the patch.

IOW, a somewhat reluctant Ack, hoping that this will be cleaned up
some day. Possibly/hopefully because arch_sync_kernel_mappings() just
goes away entirely.

 Linus
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Chris Wilson
Quoting Joerg Roedel (2020-08-21 13:37:46)
> From: Joerg Roedel 
> 
> The __apply_to_page_range() function is also used to change and/or
> allocate page-table pages in the vmalloc area of the address space.
> Make sure these changes get synchronized to other page-tables in the
> system by calling arch_sync_kernel_mappings() when necessary.
> 
> Tested-by: Chris Wilson  #x86-32
> Cc:  # v5.8+
> Signed-off-by: Joerg Roedel 

I've doubled check that this patch by itself fixes our x86-32 vmapping
issue. Thanks,
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v6 05/11] drm/i915: Try to make bigjoiner work in atomic check

2020-08-21 Thread Navare, Manasi
On Fri, Aug 21, 2020 at 03:46:48PM +0530, Manna, Animesh wrote:
> 
> On 16-07-2020 04:12, Manasi Navare wrote:
> >From: Maarten Lankhorst 
> >
> >  When the clock is higher than the dotclock, try with 2 pipes enabled.
> >  If we can enable 2, then we will go into big joiner mode, and steal
> >  the adjacent crtc.
> >
> >  This only links the crtc's in software, no hardware or plane
> >  programming is done yet. Blobs are also copied from the master's
> >  crtc_state, so it doesn't depend at commit time on the other
> >  crtc_state.
> >
> >v3:
> >* Manual Rebase (Manasi)
> >  Changes since v1:
> >  - Rename pipe timings to transcoder timings, as they are now different.
> >   Changes since v2:
> >  - Rework bigjoiner checks; always disable slave when recalculating
> >master. No need to have a separate bigjoiner pass any more.
> >  - Use pipe_mode instead of transcoder_mode, to clean up the code.
> >
> >Signed-off-by: Maarten Lankhorst 
> >Signed-off-by: Manasi Navare 
> >---
> >  drivers/gpu/drm/i915/display/intel_atomic.c   |   9 +-
> >  drivers/gpu/drm/i915/display/intel_atomic.h   |   3 +-
> >  drivers/gpu/drm/i915/display/intel_display.c  | 201 --
> >  .../drm/i915/display/intel_display_types.h|   9 +
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  22 +-
> >  5 files changed, 211 insertions(+), 33 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> >b/drivers/gpu/drm/i915/display/intel_atomic.c
> >index 630f49b7aa01..b9dcdc74a10d 100644
> >--- a/drivers/gpu/drm/i915/display/intel_atomic.c
> >+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> >@@ -270,14 +270,15 @@ void intel_crtc_free_hw_state(struct intel_crtc_state 
> >*crtc_state)
> > intel_crtc_put_color_blobs(crtc_state);
> >  }
> >-void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state)
> >+void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state,
> >+ const struct intel_crtc_state *from_crtc_state)
> 
> can we use primary_crtc_state instead of from_crtc_state?

You mean more intiutive naming instead of from_crtc_state ?
I feel primary_crtc_state might be confusing with primary/secondary planes 
naming
But may be we call it master_crtc_state or primary_big_joiner_crtc_state ?

@Maarten @Ville, any thoughts on better naming here?

> 
> >  {
> > drm_property_replace_blob(_state->hw.degamma_lut,
> >-  crtc_state->uapi.degamma_lut);
> >+  from_crtc_state->uapi.degamma_lut);
> > drm_property_replace_blob(_state->hw.gamma_lut,
> >-  crtc_state->uapi.gamma_lut);
> >+  from_crtc_state->uapi.gamma_lut);
> > drm_property_replace_blob(_state->hw.ctm,
> >-  crtc_state->uapi.ctm);
> >+  from_crtc_state->uapi.ctm);
> >  }
> >  /**
> >diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h 
> >b/drivers/gpu/drm/i915/display/intel_atomic.h
> >index 11146292b06f..fc556c032c8f 100644
> >--- a/drivers/gpu/drm/i915/display/intel_atomic.h
> >+++ b/drivers/gpu/drm/i915/display/intel_atomic.h
> >@@ -43,7 +43,8 @@ struct drm_crtc_state *intel_crtc_duplicate_state(struct 
> >drm_crtc *crtc);
> >  void intel_crtc_destroy_state(struct drm_crtc *crtc,
> >struct drm_crtc_state *state);
> >  void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state);
> >-void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state);
> >+void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state,
> >+ const struct intel_crtc_state 
> >*from_crtc_state);
> >  struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
> >  void intel_atomic_state_free(struct drm_atomic_state *state);
> >  void intel_atomic_state_clear(struct drm_atomic_state *state);
> >diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> >b/drivers/gpu/drm/i915/display/intel_display.c
> >index 3ecb642805a6..955e19abb563 100644
> >--- a/drivers/gpu/drm/i915/display/intel_display.c
> >+++ b/drivers/gpu/drm/i915/display/intel_display.c
> >@@ -8016,9 +8016,24 @@ static int intel_crtc_compute_config(struct 
> >intel_crtc *crtc,
> >  struct intel_crtc_state *pipe_config)
> >  {
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >-const struct drm_display_mode *pipe_mode = _config->hw.pipe_mode;
> >+struct drm_display_mode *pipe_mode = _config->hw.pipe_mode;
> > int clock_limit = dev_priv->max_dotclk_freq;
> >+*pipe_mode = pipe_config->hw.adjusted_mode;
> >+
> >+/* Adjust pipe_mode for bigjoiner, with half the horizontal mode */
> >+if (pipe_config->bigjoiner) {
> >+pipe_mode->crtc_clock /= 2;
> >+pipe_mode->crtc_hdisplay /= 2;
> >+pipe_mode->crtc_hblank_start /= 2;
> >+pipe_mode->crtc_hblank_end /= 2;
> >+

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from i915 (rev5)

2020-08-21 Thread Patchwork
== Series Details ==

Series: drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from 
i915 (rev5)
URL   : https://patchwork.freedesktop.org/series/80542/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8914 -> Patchwork_18389


Summary
---

  **SUCCESS**

  No regressions found.

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

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_exec_suspend@basic-s0:
- fi-tgl-u2:  [PASS][1] -> [FAIL][2] ([i915#1888])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8914/fi-tgl-u2/igt@gem_exec_susp...@basic-s0.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18389/fi-tgl-u2/igt@gem_exec_susp...@basic-s0.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
- fi-bsw-kefka:   [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8914/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18389/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html

  * igt@i915_selftest@live@gt_lrc:
- fi-tgl-u2:  [PASS][5] -> [DMESG-FAIL][6] ([i915#2373])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8914/fi-tgl-u2/igt@i915_selftest@live@gt_lrc.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18389/fi-tgl-u2/igt@i915_selftest@live@gt_lrc.html

  
 Possible fixes 

  * igt@i915_module_load@reload:
- fi-tgl-u2:  [DMESG-WARN][7] ([i915#1982]) -> [PASS][8]
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8914/fi-tgl-u2/igt@i915_module_l...@reload.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18389/fi-tgl-u2/igt@i915_module_l...@reload.html

  
 Warnings 

  * igt@kms_flip@basic-flip-vs-modeset@a-dp1:
- fi-kbl-x1275:   [DMESG-WARN][9] ([i915#62] / [i915#92]) -> 
[DMESG-WARN][10] ([i915#62] / [i915#92] / [i915#95]) +2 similar issues
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8914/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-mode...@a-dp1.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18389/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-mode...@a-dp1.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-kbl-x1275:   [DMESG-WARN][11] ([i915#62] / [i915#92] / [i915#95]) 
-> [DMESG-WARN][12] ([i915#62] / [i915#92]) +2 similar issues
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8914/fi-kbl-x1275/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18389/fi-kbl-x1275/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html

  
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2373]: https://gitlab.freedesktop.org/drm/intel/issues/2373
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (38 -> 34)
--

  Missing(4): fi-byt-clapper fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


Build changes
-

  * Linux: CI_DRM_8914 -> Patchwork_18389

  CI-20190529: 20190529
  CI_DRM_8914: 1339d80a19c0da27c443a4430fd0fe8a9d924b97 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5770: f1d0c240ea2e631dfb9f493f37f8fb61cb2b1cf2 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18389: 6315f5ad4f8179c74041b391d3de28a287f909dc @ 
git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6315f5ad4f81 drm/nouveau/kms: Start using drm_dp_read_dpcd_caps()
ac723bac5833 drm/i915/dp: Extract drm_dp_read_dpcd_caps()
d141ab324b66 drm/nouveau/kms: Don't change EDID when it hasn't actually changed
1f64c8ff3b23 drm/nouveau/kms/nv50-: Add support for DP_SINK_COUNT
9239de9af4a8 drm/i915/dp: Extract drm_dp_get_sink_count()
ddbaaff99477 drm/i915/dp: Extract drm_dp_has_sink_count()
fd51a5fc1cbd drm/nouveau/kms/nv50-: Use downstream DP clock limits for mode 
validation
129326a66615 drm/i915/dp: Extract drm_dp_downstream_read_info()
b52a69d8b457 drm/nouveau/kms: Only use hpd_work for reprobing in HPD paths
f4546d625cdb drm/nouveau/kms: Move drm_dp_cec_unset_edid() into 
nouveau_connector_detect()
f495d1de4396 drm/nouveau/kms: Use new drm_dp_has_mst() helper for checking MST 
caps
b4f1b33944cb drm/i915/dp: Extract drm_dp_has_mst()
6aea2bc64968 drm/nouveau/kms/nv50-: Refactor and cleanup DP HPD handling
1d522b451ebd drm/nouveau/kms/nv50-: Use drm_dp_dpcd_(readb|writeb)() in 
nv50_sor_disable()
171d03233d42 drm/nouveau/kms: Search for encoders' connectors properly
5c42912bafc7 drm/nouveau/kms: Don't clear DP_MST_CTRL DPCD in nv50_mstm_new()
62374a48857f 

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from i915 (rev5)

2020-08-21 Thread Patchwork
== Series Details ==

Series: drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from 
i915 (rev5)
URL   : https://patchwork.freedesktop.org/series/80542/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1019:47:expected unsigned int 
[addressable] [usertype] ulClockParams
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1019:47:got restricted __le32 
[usertype]
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1019:47: warning: incorrect type 
in assignment (different base types)
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1028:50: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1029:49: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1037:47: warning: too many 
warnings
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:184:44: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:283:14: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:320:14: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:323:14: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:326:14: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:329:18: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:330:26: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:338:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:340:38: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:342:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:346:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:348:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:353:33: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:367:43: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:369:38: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:374:67: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:375:53: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:378:66: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:389:80: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:395:57: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:402:69: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:403:53: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:406:66: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:414:66: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:423:69: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:424:69: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:473:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:476:45: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:477:45: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:484:54: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:52:28: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:531:35: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:53:29: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:533:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:54:26: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:55:27: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:56:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:57:26: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:577:21: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:581:25: warning: cast to 
restricted __le32
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:58:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:583:21: warning: cast to 
restricted __le32
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:586:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:590:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:59:26: warning: cast to 
restricted __le16

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from i915 (rev5)

2020-08-21 Thread Patchwork
== Series Details ==

Series: drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from 
i915 (rev5)
URL   : https://patchwork.freedesktop.org/series/80542/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
9288d183ff61 drm/nouveau/kms: Fix some indenting in nouveau_dp_detect()
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
96588da21133 drm/nouveau/kms/nv50-: Remove open-coded drm_dp_read_desc()
-:102: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct 
nouveau_connector *' should also have an identifier name
#102: FILE: drivers/gpu/drm/nouveau/nouveau_encoder.h:109:
+int nouveau_dp_detect(struct nouveau_connector *, struct nouveau_encoder *);

-:102: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct 
nouveau_encoder *' should also have an identifier name
#102: FILE: drivers/gpu/drm/nouveau/nouveau_encoder.h:109:
+int nouveau_dp_detect(struct nouveau_connector *, struct nouveau_encoder *);

total: 0 errors, 2 warnings, 0 checks, 74 lines checked
1ac308e8c1d7 drm/nouveau/kms/nv50-: Just use drm_dp_dpcd_read() in nouveau_dp.c
62374a48857f drm/nouveau/kms/nv50-: Use macros for DP registers in nouveau_dp.c
5c42912bafc7 drm/nouveau/kms: Don't clear DP_MST_CTRL DPCD in nv50_mstm_new()
-:7: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ 
chars of sha1> ("")' - ie: 'commit fa3cdf8d0b09 ("drm/nouveau: 
Reset MST branching unit before enabling")'
#7: 
Since fa3cdf8d0b09 ("drm/nouveau: Reset MST branching unit before

total: 1 errors, 0 warnings, 0 checks, 17 lines checked
171d03233d42 drm/nouveau/kms: Search for encoders' connectors properly
1d522b451ebd drm/nouveau/kms/nv50-: Use drm_dp_dpcd_(readb|writeb)() in 
nv50_sor_disable()
6aea2bc64968 drm/nouveau/kms/nv50-: Refactor and cleanup DP HPD handling
-:53: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
(prefer a maximum 75 chars per line)
#53: 
* Enabling bits in MSTM_CTRL before calling drm_dp_mst_topology_mgr_set_mst().

-:463: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct 
drm_device *' should also have an identifier name
#463: FILE: drivers/gpu/drm/nouveau/nouveau_display.h:21:
+   void (*fini)(struct drm_device *, bool suspend, bool runtime);

total: 0 errors, 2 warnings, 0 checks, 575 lines checked
b4f1b33944cb drm/i915/dp: Extract drm_dp_has_mst()
f495d1de4396 drm/nouveau/kms: Use new drm_dp_has_mst() helper for checking MST 
caps
-:8: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 29 lines checked
f4546d625cdb drm/nouveau/kms: Move drm_dp_cec_unset_edid() into 
nouveau_connector_detect()
b52a69d8b457 drm/nouveau/kms: Only use hpd_work for reprobing in HPD paths
-:279: CHECK:UNCOMMENTED_DEFINITION: struct mutex definition without comment
#279: FILE: drivers/gpu/drm/nouveau/nouveau_drv.h:201:
+   struct mutex hpd_lock;

total: 0 errors, 0 warnings, 1 checks, 219 lines checked
129326a66615 drm/i915/dp: Extract drm_dp_downstream_read_info()
fd51a5fc1cbd drm/nouveau/kms/nv50-: Use downstream DP clock limits for mode 
validation
-:63: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#63: FILE: drivers/gpu/drm/nouveau/nouveau_dp.c:194:
+   unsigned max_clock, ds_clock, clock;

total: 0 errors, 1 warnings, 0 checks, 57 lines checked
ddbaaff99477 drm/i915/dp: Extract drm_dp_has_sink_count()
9239de9af4a8 drm/i915/dp: Extract drm_dp_get_sink_count()
-:12: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
(prefer a maximum 75 chars per line)
#12: 
* Add back comment and move back sink_count assignment in intel_dp_get_dpcd()

total: 0 errors, 1 warnings, 0 checks, 64 lines checked
1f64c8ff3b23 drm/nouveau/kms/nv50-: Add support for DP_SINK_COUNT
-:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
(prefer a maximum 75 chars per line)
#11: 
plugged into it currently results in a bogus EDID retrieval error in the kernel 
log.

total: 0 errors, 1 warnings, 0 checks, 108 lines checked
d141ab324b66 drm/nouveau/kms: Don't change EDID when it hasn't actually changed
ac723bac5833 drm/i915/dp: Extract drm_dp_read_dpcd_caps()
6315f5ad4f81 drm/nouveau/kms: Start using drm_dp_read_dpcd_caps()


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [RFC v3] drm/nouveau/kms: Search for encoders' connectors properly

2020-08-21 Thread Lyude Paul
While the way we find the associated connector for an encoder is just
fine for legacy modesetting, it's not correct for nv50+ since that uses
atomic modesetting. For reference, see the drm_encoder kdocs.

Fix this by removing nouveau_encoder_connector_get(), and replacing it
with nv04_encoder_get_connector(), nv50_outp_get_old_connector(), and
nv50_outp_get_new_connector().

v2:
* Don't line-wrap for_each_(old|new)_connector_in_state in
  nv50_outp_get_(old|new)_connector() - sravn
v3:
* Fix potential uninitialized usage of nv_connector (needs to be
  initialized to NULL at the start). Thanks kernel test robot!

Signed-off-by: Lyude Paul 
Reviewed-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv04/dac.c  |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/dfp.c  |  7 +-
 drivers/gpu/drm/nouveau/dispnv04/disp.c | 18 +
 drivers/gpu/drm/nouveau/dispnv04/disp.h |  4 +
 drivers/gpu/drm/nouveau/dispnv04/tvnv04.c   |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/tvnv17.c   |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 87 +
 drivers/gpu/drm/nouveau/nouveau_connector.c | 14 
 drivers/gpu/drm/nouveau/nouveau_encoder.h   |  6 +-
 9 files changed, 104 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/dac.c 
b/drivers/gpu/drm/nouveau/dispnv04/dac.c
index ffdd447d87068..22d10f3285597 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dac.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dac.c
@@ -419,7 +419,7 @@ static void nv04_dac_commit(struct drm_encoder *encoder)
helper->dpms(encoder, DRM_MODE_DPMS_ON);
 
NV_DEBUG(drm, "Output %s is running on CRTC %d using output %c\n",
-nouveau_encoder_connector_get(nv_encoder)->base.name,
+nv04_encoder_get_connector(nv_encoder)->base.name,
 nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
 }
 
diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c 
b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
index f9f4482c79b54..42687ea2a4ca3 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
@@ -184,7 +184,8 @@ static bool nv04_dfp_mode_fixup(struct drm_encoder *encoder,
struct drm_display_mode *adjusted_mode)
 {
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
-   struct nouveau_connector *nv_connector = 
nouveau_encoder_connector_get(nv_encoder);
+   struct nouveau_connector *nv_connector =
+   nv04_encoder_get_connector(nv_encoder);
 
if (!nv_connector->native_mode ||
nv_connector->scaling_mode == DRM_MODE_SCALE_NONE ||
@@ -478,7 +479,7 @@ static void nv04_dfp_commit(struct drm_encoder *encoder)
helper->dpms(encoder, DRM_MODE_DPMS_ON);
 
NV_DEBUG(drm, "Output %s is running on CRTC %d using output %c\n",
-nouveau_encoder_connector_get(nv_encoder)->base.name,
+nv04_encoder_get_connector(nv_encoder)->base.name,
 nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
 }
 
@@ -591,7 +592,7 @@ static void nv04_dfp_restore(struct drm_encoder *encoder)
 
if (nv_encoder->dcb->type == DCB_OUTPUT_LVDS) {
struct nouveau_connector *connector =
-   nouveau_encoder_connector_get(nv_encoder);
+   nv04_encoder_get_connector(nv_encoder);
 
if (connector && connector->native_mode)
call_lvds_script(dev, nv_encoder->dcb, head,
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c 
b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 900ab69df7e8f..3f046b917c85c 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -35,6 +35,24 @@
 
 #include 
 
+struct nouveau_connector *
+nv04_encoder_get_connector(struct nouveau_encoder *encoder)
+{
+   struct drm_device *dev = to_drm_encoder(encoder)->dev;
+   struct drm_connector *connector;
+   struct drm_connector_list_iter conn_iter;
+   struct nouveau_connector *nv_connector = NULL;
+
+   drm_connector_list_iter_begin(dev, _iter);
+   drm_for_each_connector_iter(connector, _iter) {
+   if (connector->encoder == to_drm_encoder(encoder))
+   nv_connector = nouveau_connector(connector);
+   }
+   drm_connector_list_iter_end(_iter);
+
+   return nv_connector;
+}
+
 static void
 nv04_display_fini(struct drm_device *dev, bool suspend)
 {
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.h 
b/drivers/gpu/drm/nouveau/dispnv04/disp.h
index 495d3284e8766..5ace5e906949a 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.h
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.h
@@ -6,6 +6,8 @@
 
 #include "nouveau_display.h"
 
+struct nouveau_encoder;
+
 enum nv04_fp_display_regs {
FP_DISPLAY_END,
FP_TOTAL,
@@ -93,6 +95,8 @@ nv04_display(struct drm_device *dev)
 
 /* nv04_display.c */
 int nv04_display_create(struct drm_device *);
+struct nouveau_connector *

Re: [Intel-gfx] [RFC 13/20] drm/i915/dp: Extract drm_dp_downstream_read_info()

2020-08-21 Thread Lyude Paul
On Fri, 2020-08-21 at 01:37 +0300, Imre Deak wrote:
> On Wed, Aug 19, 2020 at 05:34:15PM -0400, Lyude Paul wrote:
> > (adding Ville and Imre to the cc here, they might be interested to know
> > about
> > this, comments down below)
> > 
> > On Wed, 2020-08-19 at 11:15 -0400, Sean Paul wrote:
> > > On Tue, Aug 11, 2020 at 04:04:50PM -0400, Lyude Paul wrote:
> > > > We're going to be doing the same probing process in nouveau for
> > > > determining downstream DP port capabilities, so let's deduplicate the
> > > > work by moving i915's code for handling this into a shared helper:
> > > > drm_dp_downstream_read_info().
> > > > 
> > > > Note that when we do this, we also do make some functional changes while
> > > > we're at it:
> > > > * We always clear the downstream port info before trying to read it,
> > > >   just to make things easier for the caller
> > > > * We skip reading downstream port info if the DPCD indicates that we
> > > >   don't support downstream port info
> > > > * We only read as many bytes as needed for the reported number of
> > > >   downstream ports, no sense in reading the whole thing every time
> > > > 
> > > > Signed-off-by: Lyude Paul 
> > > > ---
> > > >  drivers/gpu/drm/drm_dp_helper.c | 32 +
> > > >  drivers/gpu/drm/i915/display/intel_dp.c | 14 ++-
> > > >  include/drm/drm_dp_helper.h |  3 +++
> > > >  3 files changed, 37 insertions(+), 12 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > > > b/drivers/gpu/drm/drm_dp_helper.c
> > > > index 4c21cf69dad5a..9703b33599c3b 100644
> > > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > > @@ -423,6 +423,38 @@ bool drm_dp_send_real_edid_checksum(struct
> > > > drm_dp_aux
> > > > *aux,
> > > >  }
> > > >  EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
> > > >  
> > > > +/**
> > > > + * drm_dp_downstream_read_info() - read DPCD downstream port info if
> > > > available
> > > > + * @aux: DisplayPort AUX channel
> > > > + * @dpcd: A cached copy of the port's DPCD
> > > > + * @downstream_ports: buffer to store the downstream port info in
> > > > + *
> > > > + * Returns: 0 if either the downstream port info was read successfully
> > > > or
> > > > + * there was no downstream info to read, or a negative error code
> > > > otherwise.
> > > > + */
> > > > +int drm_dp_downstream_read_info(struct drm_dp_aux *aux,
> > > > +   const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > > > +   u8
> > > > downstream_ports[DP_MAX_DOWNSTREAM_PORTS])
> > > > +{
> > > > +   int ret;
> > > > +   u8 len;
> > > > +
> > > > +   memset(downstream_ports, 0, DP_MAX_DOWNSTREAM_PORTS);
> > > > +
> > > > +   /* No downstream info to read */
> > > > +   if (!drm_dp_is_branch(dpcd) ||
> > > > +   dpcd[DP_DPCD_REV] < DP_DPCD_REV_10 ||
> > > > +   !(dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > > > DP_DWN_STRM_PORT_PRESENT))
> > > > +   return 0;
> > > > +
> > > > +   len = (dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_PORT_COUNT_MASK) *
> > > > 4;
> > > 
> > > I'm having a hard time rationalizing DP_MAX_DOWNSTREAM_PORTS being 16, but
> > > only
> > > having 4 ports worth of data in the DP_DOWNSTREAM_PORT_* registers. Do you
> > > know
> > > what's supposed to happen if dpcd[DP_DOWN_STREAM_PORT_COUNT] is > 4?
> > > 
> > ok!! Taking a lesson from our available_pbn/full_pbn confusion in the past,
> > I
> > squinted very hard at the specification and eventually found something that
> > I
> > think clears this up. Surprise - we definitely had this implemented
> > incorrectly
> > in i915
> 
> To me it looks correct, only DFP0's cap info is used, by also handling
> the DP_DETAILED_CAP_INFO_AVAILABLE=0/1 cases.
Ended up realizing this right after I sent this version of the RFC - yeah, it
definitely shouldn't be causing any real problems as of now

> 
> The wording is a bit unclear, but as I understand the Standard only
> calls for the above:
> 
> """
> A DP upstream device shall read the capability from DPCD Addresses 00080h
> through 00083h. A DP Branch device with multiple DFPs shall report the
> detailed
> capability information of the lowest DFP number to which a downstream device
> is connected, consistent with the DisplayID or legacy EDID access routing
> policy
> of an SST-only DP Branch device as described in Section 2.1.4.1.
> """

So-I saw this too, but notice the use of the language "A /DP Branch/ device with
multiple DFPs shall report the detailed…". This makes me think it's implying
that this is a requirement for MSTBs and not SST sinks, just a guess.
> 
> > From section 5.3.3.1:
> > 
> >Either one or four bytes are used, per DFP type indication. Therefore, up
> > to
> >16 (with 1-byte descriptor) or four (with 4-byte descriptor) DFP
> > capabilities
> >can be stored.
> > 
> > So, a couple takeaways from this:
> > 
> >  * A DisplayPort connector can have 

[Intel-gfx] ✗ Fi.CI.IGT: failure for mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Patchwork
== Series Details ==

Series: mm: Track page table modifications in __apply_to_page_range()
URL   : https://patchwork.freedesktop.org/series/80896/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8913_full -> Patchwork_18388_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_18388_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18388_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@kms_cursor_legacy@cursor-vs-flip-toggle:
- shard-hsw:  [PASS][1] -> [FAIL][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-hsw8/igt@kms_cursor_leg...@cursor-vs-flip-toggle.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-hsw1/igt@kms_cursor_leg...@cursor-vs-flip-toggle.html

  

### Piglit changes ###

 Possible regressions 

  * spec@glsl-1.50@execution@built-in-functions@gs-op-rshift-ivec2-int (NEW):
- pig-snb-2600:   NOTRUN -> [FAIL][3] +1 similar issue
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/pig-snb-2600/spec@glsl-1.50@execution@built-in-functi...@gs-op-rshift-ivec2-int.html

  
New tests
-

  New tests have been introduced between CI_DRM_8913_full and 
Patchwork_18388_full:

### New Piglit tests (2) ###

  * spec@glsl-1.50@execution@built-in-functions@gs-op-eq-bvec4-bvec4-using-if:
- Statuses : 1 fail(s)
- Exec time: [0.14] s

  * spec@glsl-1.50@execution@built-in-functions@gs-op-rshift-ivec2-int:
- Statuses : 1 fail(s)
- Exec time: [0.12] s

  

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_exec_whisper@basic-forked:
- shard-glk:  [PASS][4] -> [DMESG-WARN][5] ([i915#118] / [i915#95])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-glk1/igt@gem_exec_whis...@basic-forked.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-glk5/igt@gem_exec_whis...@basic-forked.html

  * igt@gem_flink_basic@basic:
- shard-snb:  [PASS][6] -> [TIMEOUT][7] ([i915#1958]) +3 similar 
issues
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-snb2/igt@gem_flink_ba...@basic.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-snb2/igt@gem_flink_ba...@basic.html

  * igt@gen9_exec_parse@allowed-single:
- shard-skl:  [PASS][8] -> [DMESG-WARN][9] ([i915#1436] / 
[i915#716])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-skl8/igt@gen9_exec_pa...@allowed-single.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-skl6/igt@gen9_exec_pa...@allowed-single.html

  * igt@i915_pm_rpm@system-suspend:
- shard-kbl:  [PASS][10] -> [INCOMPLETE][11] ([i915#151] / 
[i915#155])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-kbl1/igt@i915_pm_...@system-suspend.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-kbl2/igt@i915_pm_...@system-suspend.html

  * igt@kms_color@pipe-b-ctm-negative:
- shard-skl:  [PASS][12] -> [DMESG-WARN][13] ([i915#1982]) +6 
similar issues
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-skl8/igt@kms_co...@pipe-b-ctm-negative.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-skl6/igt@kms_co...@pipe-b-ctm-negative.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
- shard-glk:  [PASS][14] -> [FAIL][15] ([i915#72])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-glk2/igt@kms_cursor_leg...@2x-long-flip-vs-cursor-atomic.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-glk7/igt@kms_cursor_leg...@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
- shard-kbl:  [PASS][16] -> [DMESG-WARN][17] ([i915#180]) +1 
similar issue
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-kbl4/igt@kms_flip@flip-vs-suspend-interrupti...@c-dp1.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-kbl7/igt@kms_flip@flip-vs-suspend-interrupti...@c-dp1.html

  * igt@kms_flip@flip-vs-suspend@c-hdmi-a1:
- shard-hsw:  [PASS][18] -> [INCOMPLETE][19] ([i915#2055])
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/shard-hsw7/igt@kms_flip@flip-vs-susp...@c-hdmi-a1.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/shard-hsw4/igt@kms_flip@flip-vs-susp...@c-hdmi-a1.html

  * 

[Intel-gfx] [PATCH i-g-t v4 20/20] tests/core_hotunplug: Un-blocklist *bind* subtests

2020-08-21 Thread Janusz Krzysztofik
Subtests which don't remove the device, only unbind the driver from it,
seem relatively safe and harmless for CI.  Remove them from the CI
blocklist.

Signed-off-by: Janusz Krzysztofik 
---
 tests/intel-ci/blacklist.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt
index f9a57cb54..25b567038 100644
--- a/tests/intel-ci/blacklist.txt
+++ b/tests/intel-ci/blacklist.txt
@@ -120,7 +120,7 @@ igt@perf_pmu@cpu-hotplug
 
 # Currently fails and leaves the machine in a very bad state, and
 # causes coverage loss for other tests.
-igt@core_hotunplug@.*
+igt@core_hotunplug@.*plug.*
 
 # hangs several gens of hosts, and has no immediate fix
 igt@device_reset@reset-bound
\ No newline at end of file
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 19/20] tests/core_hotunplug: Duplicate debug messages in dmesg

2020-08-21 Thread Janusz Krzysztofik
The purpose of debug messages displayed by the test is to make
identification of a subtest phase that fails more easy.  Since issues
exhibited by the test are mostly reported to dmesg, print those debug
messages to /dev/kmsg as well.

v2: Rebase on upstream.
v3: Refresh.

Signed-off-by: Janusz Krzysztofik 
---
 tests/core_hotunplug.c | 32 +++-
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index f919fa6de..ae7fe18ad 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -52,6 +52,12 @@ struct hotunplug {
 
 /* Helpers */
 
+#define local_debug(fmt, msg...)  \
+({\
+   igt_debug(fmt, msg);   \
+   igt_kmsg(KMSG_DEBUG "%s: " fmt, igt_test_name(), msg); \
+})
+
 /**
  * Subtests must be able to close examined devices completely.  Don't
  * use drm_open_driver() since in case of an i915 device it opens it
@@ -61,7 +67,7 @@ static int local_drm_open_driver(const char *prefix, const 
char *suffix)
 {
int fd_drm;
 
-   igt_debug("%sopening device%s\n", prefix, suffix);
+   local_debug("%sopening device%s\n", prefix, suffix);
 
fd_drm = __drm_open_driver(DRIVER_ANY);
igt_assert_fd(fd_drm);
@@ -120,7 +126,7 @@ static void prepare(struct hotunplug *priv)
 static void driver_unbind(struct hotunplug *priv, const char *prefix,
  int timeout)
 {
-   igt_debug("%sunbinding the driver from the device\n", prefix);
+   local_debug("%sunbinding the driver from the device\n", prefix);
priv->failure = "Driver unbind failure!";
 
igt_set_timeout(timeout, "Driver unbind timeout!");
@@ -136,7 +142,7 @@ static void driver_unbind(struct hotunplug *priv, const 
char *prefix,
 /* Re-bind the driver to the device */
 static void driver_bind(struct hotunplug *priv, int timeout)
 {
-   igt_debug("rebinding the driver to the device\n");
+   local_debug("%s\n", "rebinding the driver to the device");
priv->failure = "Driver re-bind failure!";
 
igt_set_timeout(timeout, "Driver re-bind timeout!");
@@ -160,7 +166,7 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix,
O_DIRECTORY);
igt_assert_fd(priv->fd.sysfs_dev);
 
-   igt_debug("%sunplugging the device\n", prefix);
+   local_debug("%sunplugging the device\n", prefix);
priv->failure = "Device unplug failure!";
 
igt_set_timeout(timeout, "Device unplug timeout!");
@@ -178,7 +184,7 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix,
 /* Re-discover the device by rescanning its bus */
 static void bus_rescan(struct hotunplug *priv, int timeout)
 {
-   igt_debug("rediscovering the device\n");
+   local_debug("%s\n", "rediscovering the device");
priv->failure = "Bus rescan failure!";
 
igt_set_timeout(timeout, "Bus rescan timeout!");
@@ -231,7 +237,7 @@ static int local_i915_healthcheck(int i915)
if (hang_detected)
return -EIO;
 
-   igt_debug("running i915 GPU healthcheck\n");
+   local_debug("%s\n", "running i915 GPU healthcheck");
 
if (local_i915_is_wedged(i915))
return -EIO;
@@ -262,7 +268,7 @@ static int local_i915_healthcheck(int i915)
 
 static int local_i915_recover(int i915)
 {
-   igt_debug("forcing i915 GPU reset\n");
+   local_debug("%s\n", "forcing i915 GPU reset");
 
igt_force_gpu_reset(i915);
hang_detected = false;
@@ -369,7 +375,7 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 
driver_unbind(priv, "hot ", 0);
 
-   igt_debug("late closing the unbound device instance\n");
+   local_debug("%s\n", "late closing the unbound device instance");
priv->fd.drm = close_device(priv->fd.drm);
igt_assert_eq(priv->fd.drm, -1);
 }
@@ -380,7 +386,7 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 
device_unplug(priv, "hot ", 0);
 
-   igt_debug("late closing the removed device instance\n");
+   local_debug("%s\n", "late closing the removed device instance");
priv->fd.drm = close_device(priv->fd.drm);
igt_assert_eq(priv->fd.drm, -1);
 }
@@ -391,7 +397,7 @@ static void hotunbind_rebind(struct hotunplug *priv)
 
driver_unbind(priv, "hot ", 60);
 
-   igt_debug("late closing the unbound device instance\n");
+   local_debug("%s\n", "late closing the unbound device instance");
priv->fd.drm = close_device(priv->fd.drm);
igt_assert_eq(priv->fd.drm, -1);
 
@@ -406,7 +412,7 @@ static void hotunplug_rescan(struct hotunplug *priv)
 
device_unplug(priv, "hot ", 60);
 
-   igt_debug("late closing the removed device instance\n");
+   local_debug("%s\n", "late closing the removed device instance");
priv->fd.drm = 

[Intel-gfx] [PATCH i-g-t v4 08/20] tests/core_hotunplug: Handle device close errors

2020-08-21 Thread Janusz Krzysztofik
The test now ignores device close errors.  Those errors are believed to
have no influence on device health so there is no need to process them
the same way as we mostly do on errors, i.e., notify CI about a problem
via igt_abort.  However, those errors may indicate issues with the test
itself.  Moreover, impact of those errors on operations performed by
subtests, like driver unbind or device remove, should be perceived as
undefined.  Then, we should fail as soon as a device or device sysfs
node close error occurs in a subtest and also skip subsequent subtests.
However, once a driver unbind or device unplug operation has been
attempted by a subtest, we would still like to check the device health.

When in a subtest, store results of device close operations for future
reference.  Reuse file descriptor fields of the hotunplug structure for
that.  Unless in between of a driver remove or device unplug operation
and a successful device health check completion, fail current test
section right after a device close error occurs, warn otherwise.  If
still running, examine device file descriptor fields in subsequent
igt_fixture sections and skip on errors.

v2: Fix a typo in post_healthcheck function name.
v3: Don't fail on close error after successful health check, warn only,
  - move duplicated messages to helpers.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski  # v1
---
 tests/core_hotunplug.c | 64 +-
 1 file changed, 50 insertions(+), 14 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 4f7e89c95..f7a54010b 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -43,7 +43,7 @@ struct hotunplug {
int sysfs_dev;
int sysfs_bus;
int sysfs_drv;
-   } fd;
+   } fd;   /* >= 0: valid fd, == -1: closed, < -1: close failed */
const char *dev_bus_addr;
const char *failure;
 };
@@ -67,6 +67,25 @@ static int local_drm_open_driver(const char *prefix, const 
char *suffix)
return fd_drm;
 }
 
+static int local_close(int fd, const char *message)
+{
+   errno = 0;
+   if (igt_warn_on_f(close(fd), "%s\n", message))
+   return -errno;  /* (never -1) */
+
+   return -1;  /* success - return 'closed' */
+}
+
+static int close_device(int fd_drm)
+{
+   return local_close(fd_drm, "Device close failed");
+}
+
+static int close_sysfs(int fd_sysfs_dev)
+{
+   return local_close(fd_sysfs_dev, "Device sysfs node close failed");
+}
+
 static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 {
int len;
@@ -83,7 +102,8 @@ static void prepare_for_unbind(struct hotunplug *priv, char 
*buf, int buflen)
igt_assert(priv->dev_bus_addr++);
 
/* sysfs_dev no longer needed */
-   close(priv->fd.sysfs_dev);
+   priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
+   igt_assert_eq(priv->fd.sysfs_dev, -1);
 }
 
 static void prepare(struct hotunplug *priv, char *buf, int buflen)
@@ -142,7 +162,7 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix)
igt_reset_timeout();
priv->failure = NULL;
 
-   close(priv->fd.sysfs_dev);
+   priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -161,6 +181,7 @@ static void bus_rescan(struct hotunplug *priv)
 
 static void healthcheck(struct hotunplug *priv)
 {
+   /* preserve error code potentially stored before in priv->fd.drm */
int fd_drm;
 
/* device name may have changed, rebuild IGT device list */
@@ -176,7 +197,17 @@ static void healthcheck(struct hotunplug *priv)
priv->failure = NULL;
}
 
-   close(fd_drm);
+   fd_drm = close_device(fd_drm);
+   if (priv->fd.drm == -1) /* store result if no error code to preserve */
+   priv->fd.drm = fd_drm;
+}
+
+static void post_healthcheck(struct hotunplug *priv)
+{
+   igt_abort_on_f(priv->failure, "%s\n", priv->failure);
+
+   igt_require(priv->fd.drm == -1);
+   igt_require(priv->fd.sysfs_dev == -1);
 }
 
 static void set_filter_from_device(int fd)
@@ -203,7 +234,8 @@ static void unbind_rebind(struct hotunplug *priv)
prepare(priv, buf, sizeof(buf));
 
igt_debug("closing the device\n");
-   close(priv->fd.drm);
+   priv->fd.drm = close_device(priv->fd.drm);
+   igt_assert_eq(priv->fd.drm, -1);
 
driver_unbind(priv, "");
 
@@ -217,7 +249,8 @@ static void unplug_rescan(struct hotunplug *priv)
prepare(priv, NULL, 0);
 
igt_debug("closing the device\n");
-   close(priv->fd.drm);
+   priv->fd.drm = close_device(priv->fd.drm);
+   igt_assert_eq(priv->fd.drm, -1);
 
device_unplug(priv, "");
 
@@ -237,7 +270,7 @@ static void hotunbind_lateclose(struct hotunplug *priv)
driver_bind(priv);
 
igt_debug("late closing the unbound device 

[Intel-gfx] [PATCH i-g-t v4 18/20] tests/core_hotunplug: Add 'lateclose before restore' variants

2020-08-21 Thread Janusz Krzysztofik
If a GPU gets wedged during driver rebind or device re-plug for some
reason, current hotunbind/hotunplug test variants may time out before
lateclose phase, resulting in incomplete CI reports.  Rename those
variants to more adequate hotrebind/hotreplug-lateclose and add new
variants under the old names focused on exercising the lateclose phase
regardless of potential rediscover/rebind issues.  Moreover, add two
more variants which exercise driver rebind / device restore after late
close specifically.

v2: Rebase on upstream.
v3: Refresh,
  - further rename hotunbind/hotunplug-lateclose to hotunbind-rebind
and hotunplug-rescan respectively, then add two more variants under
the old names which only exercise late close, leaving rebind /
rescan to be cared of in the post-subtest recovery phase,
  - also update descriptions of unmodified subtests for consistency.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski  # v2
---
 tests/core_hotunplug.c | 114 +++--
 1 file changed, 109 insertions(+), 5 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 13e7aa46f..f919fa6de 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -369,8 +369,6 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 
driver_unbind(priv, "hot ", 0);
 
-   driver_bind(priv, 60);
-
igt_debug("late closing the unbound device instance\n");
priv->fd.drm = close_device(priv->fd.drm);
igt_assert_eq(priv->fd.drm, -1);
@@ -382,11 +380,69 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 
device_unplug(priv, "hot ", 0);
 
-   bus_rescan(priv, 60);
+   igt_debug("late closing the removed device instance\n");
+   priv->fd.drm = close_device(priv->fd.drm);
+   igt_assert_eq(priv->fd.drm, -1);
+}
+
+static void hotunbind_rebind(struct hotunplug *priv)
+{
+   priv->fd.drm = local_drm_open_driver("", " for hotrebind");
+
+   driver_unbind(priv, "hot ", 60);
+
+   igt_debug("late closing the unbound device instance\n");
+   priv->fd.drm = close_device(priv->fd.drm);
+   igt_assert_eq(priv->fd.drm, -1);
+
+   driver_bind(priv, 0);
+
+   healthcheck(priv, false);
+}
+
+static void hotunplug_rescan(struct hotunplug *priv)
+{
+   priv->fd.drm = local_drm_open_driver("", " for hotreplug");
+
+   device_unplug(priv, "hot ", 60);
+
+   igt_debug("late closing the removed device instance\n");
+   priv->fd.drm = close_device(priv->fd.drm);
+   igt_assert_eq(priv->fd.drm, -1);
+
+   bus_rescan(priv, 0);
+
+   healthcheck(priv, false);
+}
+
+static void hotrebind_lateclose(struct hotunplug *priv)
+{
+   priv->fd.drm = local_drm_open_driver("", " for hotrebind");
+
+   driver_unbind(priv, "hot ", 60);
+
+   driver_bind(priv, 0);
+
+   igt_debug("late closing the unbound device instance\n");
+   priv->fd.drm = close_device(priv->fd.drm);
+   igt_assert_eq(priv->fd.drm, -1);
+
+   healthcheck(priv, false);
+}
+
+static void hotreplug_lateclose(struct hotunplug *priv)
+{
+   priv->fd.drm = local_drm_open_driver("", " for hotreplug");
+
+   device_unplug(priv, "hot ", 60);
+
+   bus_rescan(priv, 0);
 
igt_debug("late closing the removed device instance\n");
priv->fd.drm = close_device(priv->fd.drm);
igt_assert_eq(priv->fd.drm, -1);
+
+   healthcheck(priv, false);
 }
 
 /* Main */
@@ -419,7 +475,7 @@ igt_main
}
 
igt_subtest_group {
-   igt_describe("Check if the driver can be cleanly unbound from a 
device believed to be closed");
+   igt_describe("Check if the driver can be cleanly unbound from a 
device believed to be closed, then rebound");
igt_subtest("unbind-rebind")
unbind_rebind();
 
@@ -431,7 +487,7 @@ igt_main
post_healthcheck();
 
igt_subtest_group {
-   igt_describe("Check if a device believed to be closed can be 
cleanly unplugged");
+   igt_describe("Check if a device believed to be closed can be 
cleanly unplugged, then restored");
igt_subtest("unplug-rescan")
unplug_rescan();
 
@@ -463,6 +519,54 @@ igt_main
recover();
}
 
+   igt_fixture
+   post_healthcheck();
+
+   igt_subtest_group {
+   igt_describe("Check if the driver can be cleanly rebound to a 
device after hotunbind-lateclose");
+   igt_subtest("hotunbind-rebind")
+   hotunbind_rebind();
+
+   igt_fixture
+   recover();
+   }
+
+   igt_fixture
+   post_healthcheck();
+
+   igt_subtest_group {
+   igt_describe("Check if a device can be cleanly restored after 
hotunplug-lateclose");
+   igt_subtest("hotunplug-rescan")
+   hotunplug_rescan();
+
+ 

[Intel-gfx] [PATCH i-g-t v4 17/20] tests/core_hotunplug: More thorough i915 healthcheck and recovery

2020-08-21 Thread Janusz Krzysztofik
The test now assumes the i915 driver is able to identify potential
hardware or driver issues while rebinding to a device and indicate them
by marking the GPU wedged.  Should that assumption occur wrong, the
health check phase of the test would happily succeed while potentially
leaving the device in an unusable state.  That would not only give us
falsely positive test results but could also potentially affect
subsequently run applications.  Then, we should examine health of the
exercised device more thoroughly and try harder to recover it from
potentially detected stalls.

We could use a gem_test_engine() library function which submits and
asserts successful execution of a NOP batch on each physical engine.
Unfortunately, on failure this function jumps out of an IGT test
section it is called from, while we would like to continue with
recovery steps, possibly not adding another level of test section group
nesting.  Moreover, the function opens the device again and doesn't
close the extra file descriptor before the jump, while we care for
being able to close the exercised device completely before running
certain subtest  operations.  Then, reimplement the function locally
with those issues fixed and use it as an i915 healthcheck.  Call it
also on test startup so operations performed by the test are never
blamed for driver or hardware issues which may potentially exist and
be possible to detect on test start.

Should the i915 GPU be found unresponsive by the health check called
from a recovery section, try harder to recover it to a usable state
with a global GPU reset.

For still more effective detection of GPU hangs, use a hang detector
provided by IGT library.  However, replace the library signal handler
with our own implementation that doesn't jump out of the current IGT
test section on GPU hang so we are still able to perform the reset and
retry.

v2: Skip i915 health check if a GPU hang has been already detected by a
previous health check run and not yet remediated with a GPU reset,
  - take care of stopping a hang detector instance possibly left
running by a failed health check attempt.

Signed-off-by: Janusz Krzysztofik 
---
 tests/core_hotunplug.c | 97 ++
 1 file changed, 89 insertions(+), 8 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 24beed81a..13e7aa46f 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -23,8 +23,10 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -195,7 +197,80 @@ static void cleanup(struct hotunplug *priv)
priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
 }
 
-static void healthcheck(struct hotunplug *priv)
+static bool local_i915_is_wedged(int i915)
+{
+   int err = 0;
+
+   if (ioctl(i915, DRM_IOCTL_I915_GEM_THROTTLE))
+   err = -errno;
+   return err == -EIO;
+}
+
+static bool hang_detected = false;
+
+static void local_sig_abort(int sig)
+{
+   errno = 0; /* inside a signal, last errno reporting is confusing */
+   hang_detected = true;
+}
+
+static int local_i915_healthcheck(int i915)
+{
+   const uint32_t bbe = MI_BATCH_BUFFER_END;
+   struct drm_i915_gem_exec_object2 obj = { };
+   struct drm_i915_gem_execbuffer2 execbuf = {
+   .buffers_ptr = to_user_pointer(),
+   .buffer_count = 1,
+   };
+   const struct intel_execution_engine2 *engine;
+
+   /* stop our hang detector possibly still runnign if we failed before */
+   igt_stop_hang_detector();
+
+   /* don't run again before GPU reset if hang has been already detected */
+   if (hang_detected)
+   return -EIO;
+
+   igt_debug("running i915 GPU healthcheck\n");
+
+   if (local_i915_is_wedged(i915))
+   return -EIO;
+
+   obj.handle = gem_create(i915, 4096);
+   gem_write(i915, obj.handle, 0, , sizeof(bbe));
+
+   igt_fork_hang_detector(i915);
+   signal(SIGIO, local_sig_abort);
+
+   __for_each_physical_engine(i915, engine) {
+   execbuf.flags = engine->flags;
+   gem_execbuf(i915, );
+   }
+
+   gem_sync(i915, obj.handle);
+   gem_close(i915, obj.handle);
+
+   igt_stop_hang_detector();
+   if (hang_detected)
+   return -EIO;
+
+   if (local_i915_is_wedged(i915))
+   return -EIO;
+
+   return 0;
+}
+
+static int local_i915_recover(int i915)
+{
+   igt_debug("forcing i915 GPU reset\n");
+
+   igt_force_gpu_reset(i915);
+   hang_detected = false;
+
+   return local_i915_healthcheck(i915);
+}
+
+static void healthcheck(struct hotunplug *priv, bool recover)
 {
/* preserve error code potentially stored before in priv->fd.drm */
bool closed = priv->fd.drm == -1;
@@ -210,9 +285,14 @@ static void healthcheck(struct hotunplug *priv)
priv->fd.drm = fd_drm;
 
if (is_i915_device(fd_drm)) {
-  

[Intel-gfx] [PATCH i-g-t v4 09/20] tests/core_hotunplug: Prepare invariant data once per test run

2020-08-21 Thread Janusz Krzysztofik
Each subtest now calls a prepare() helper which opens a couple of files
required by that subtest.  Those files are then closed after use,
either directly from the subtest body, or indirectly from inside one of
helper functions called during the subtest execution.  That approach
not only makes lifecycle of individual file descriptors difficult to
follow but also prevents us from re-running health checks on subtest
failures from follow up igt_fixture sections since we may need to retry
bus rescan or driver rebind operations.

Two of those files - device bus and driver sysfs nodes - are not
affected nor interfere with driver unbind / device unplug operations
performed by subtests.  Then, there is not much sense in closing and
reopening those nodes.  Open them once at the beginning of a test run,
then close them as late as on test completion.

The prepare() helper also populates a device bus address string used by
driver unbind / rebind operations.  Since the bus address of an
exercised device never changes, also prepare that string only once at
the beginning of a test run.  Note that it is the same as the last
component of a device filter string which is already resolved and
installed from an initial igt_fixture section of the test.  Then,
initialize the device bus address field of a hotunplug structure
instance with a pointer to the respective substring of that filter
rather than resolving it again from the device sysfs node pathname.

There is one more sysfs node - a DRM device node - now opened by the
prepare() helper for subtests which perform device remove operations.
That node can't be opened only once at the beginning of a test run
because its open file descriptor is no longer usable as soon as a
driver unbind operation is performed.  On the other hand, it can't be
opened easily from inside a device_remove() helper since some subtests
just don't open the device so its file descriptor used by
igt_sysfs_open() may just not be available.  However, note that only a
PCI sysfs node of the device, not necessarily the DRM one, is actually
required for a successful device remove operation, and that node can be
opened easily from a bus file descriptor using a device bus address
string, both already available.  Then, change the semantics of a
.fd.sysfs_dev field of the hotunplug structure from DRM to PCI device
sysfs file descriptor, then let the device_remove() helper open the
device PCI node by itself and store its file descriptor in that field.
Also, for still more easy access to the device PCI node, use a
'subsystem/devices' subnode of the PCI device as its bus sysfs location
instead of just 'subsystem', then adjust a relative path to the bus
'rescan' function accordingly.

A side benefit of using the PCI device sysfs node, not the DRM one,
while removing the device is that a future subtest may now easily
perform both driver unbind and device remove operations in a row.

v2: Rebase only.
v3: Refresh.

Suggested-by: Michał Winiarski 
Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 85 +++---
 1 file changed, 31 insertions(+), 54 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index f7a54010b..849a774ff 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -86,41 +86,30 @@ static int close_sysfs(int fd_sysfs_dev)
return local_close(fd_sysfs_dev, "Device sysfs node close failed");
 }
 
-static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
+static void prepare(struct hotunplug *priv)
 {
-   int len;
+   const char *filter = igt_device_filter_get(0), *sysfs_path;
 
-   igt_assert(buflen);
+   igt_assert(filter);
 
-   priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
-   O_DIRECTORY);
-   igt_assert_fd(priv->fd.sysfs_drv);
-
-   len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
-   buf[len] = '\0';
-   priv->dev_bus_addr = strrchr(buf, '/');
+   priv->dev_bus_addr = strrchr(filter, '/');
igt_assert(priv->dev_bus_addr++);
 
-   /* sysfs_dev no longer needed */
-   priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
-   igt_assert_eq(priv->fd.sysfs_dev, -1);
-}
-
-static void prepare(struct hotunplug *priv, char *buf, int buflen)
-{
-   priv->fd.drm = local_drm_open_driver("", " for subtest");
+   sysfs_path = strchr(filter, ':');
+   igt_assert(sysfs_path++);
 
-   priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
+   priv->fd.sysfs_dev = open(sysfs_path, O_DIRECTORY);
igt_assert_fd(priv->fd.sysfs_dev);
 
-   if (buf) {
-   prepare_for_unbind(priv, buf, buflen);
-   } else {
-   /* prepare for bus rescan */
-   priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev,
-   "device/subsystem", O_DIRECTORY);
-   

[Intel-gfx] [PATCH i-g-t v4 15/20] tests/core_hotunplug: Assert expected device presence/absence

2020-08-21 Thread Janusz Krzysztofik
Don't rely on successful write to sysfs control files, assert existence
/ non-existence of a respective device sysfs node as well.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index f280771ab..d30ad8525 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -126,6 +126,9 @@ static void driver_unbind(struct hotunplug *priv, const 
char *prefix,
   priv->dev_bus_addr),
 "Driver unbind failure!\n");
igt_reset_timeout();
+
+   igt_assert_f(faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr, F_OK, 0),
+"Unbound device still present\n");
 }
 
 /* Re-bind the driver to the device */
@@ -139,6 +142,10 @@ static void driver_bind(struct hotunplug *priv, int 
timeout)
   priv->dev_bus_addr),
 "Driver re-bind failure\n!");
igt_reset_timeout();
+
+   igt_fail_on_f(faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr,
+   F_OK, 0),
+ "Rebound device not present!\n");
 }
 
 /* Remove (virtually unplug) the device from its bus */
@@ -161,6 +168,9 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix,
 
priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
igt_assert_eq(priv->fd.sysfs_dev, -1);
+
+   igt_assert_f(faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr, F_OK, 0),
+"Unplugged device still present\n");
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -173,6 +183,10 @@ static void bus_rescan(struct hotunplug *priv, int timeout)
igt_assert_f(igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1"),
   "Bus rescan failure!\n");
igt_reset_timeout();
+
+   igt_fail_on_f(faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr,
+   F_OK, 0),
+ "Fakely unplugged device not rediscovered!\n");
 }
 
 static void cleanup(struct hotunplug *priv)
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 12/20] tests/core_hotunplug: Fail subtests on device close errors

2020-08-21 Thread Janusz Krzysztofik
Since health checks are now run from follow-up fixture sections, it is
safe to fail subtests without the need to abort the test execution.  Do
that on device close errors instead of just emitting warnings.

v2: Rebase only.
v3: Refresh.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 145593683..e048f3a15 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -153,6 +153,7 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix)
igt_reset_timeout();
 
priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
+   igt_assert_eq(priv->fd.sysfs_dev, -1);
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -270,6 +271,7 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 
igt_debug("late closing the unbound device instance\n");
priv->fd.drm = close_device(priv->fd.drm);
+   igt_assert_eq(priv->fd.drm, -1);
 }
 
 static void hotunplug_lateclose(struct hotunplug *priv)
@@ -282,6 +284,7 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 
igt_debug("late closing the removed device instance\n");
priv->fd.drm = close_device(priv->fd.drm);
+   igt_assert_eq(priv->fd.drm, -1);
 }
 
 /* Main */
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 10/20] tests/core_hotunplug: Skip selectively on sysfs close errors

2020-08-21 Thread Janusz Krzysztofik
Since we no longer open a device DRM sysfs node, only a PCI one, driver
unbind operations are no longer affected by missed or unsuccessful
sysfs file close attempts.  Skip only affected subtests if that
happens.

v2: Rebase only.
v3: Refresh.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 849a774ff..602a91cf8 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -109,7 +109,6 @@ static void prepare(struct hotunplug *priv)
igt_assert_fd(priv->fd.sysfs_bus);
 
priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
-   igt_assert_eq(priv->fd.sysfs_dev, -1);
 }
 
 /* Unbind the driver from the device */
@@ -139,6 +138,8 @@ static void driver_bind(struct hotunplug *priv)
 /* Remove (virtually unplug) the device from its bus */
 static void device_unplug(struct hotunplug *priv, const char *prefix)
 {
+   igt_require(priv->fd.sysfs_dev == -1);
+
priv->fd.sysfs_dev = openat(priv->fd.sysfs_bus, priv->dev_bus_addr,
O_DIRECTORY);
igt_assert_fd(priv->fd.sysfs_dev);
@@ -194,7 +195,6 @@ static void post_healthcheck(struct hotunplug *priv)
igt_abort_on_f(priv->failure, "%s\n", priv->failure);
 
igt_require(priv->fd.drm == -1);
-   igt_require(priv->fd.sysfs_dev == -1);
 }
 
 static void set_filter_from_device(int fd)
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 13/20] tests/core_hotunplug: Let the driver time out essential sysfs operations

2020-08-21 Thread Janusz Krzysztofik
The test now arms a timer before performing each driver unbind / rebind
or device unplug / bus rescan sysfs operation.  Then in case of issues
we may prevent the driver from showing us if and how it can handle
them.

Don't arm the timer before sysfs operations which are essential for a
subtest.

Signed-off-by: Janusz Krzysztofik 
---
 tests/core_hotunplug.c | 38 --
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index e048f3a15..572c66474 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -115,29 +115,31 @@ static void prepare(struct hotunplug *priv)
 }
 
 /* Unbind the driver from the device */
-static void driver_unbind(struct hotunplug *priv, const char *prefix)
+static void driver_unbind(struct hotunplug *priv, const char *prefix,
+ int timeout)
 {
igt_debug("%sunbinding the driver from the device\n", prefix);
priv->failure = "Driver unbind failure!";
 
-   igt_set_timeout(60, "Driver unbind timeout!");
+   igt_set_timeout(timeout, "Driver unbind timeout!");
igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
igt_reset_timeout();
 }
 
 /* Re-bind the driver to the device */
-static void driver_bind(struct hotunplug *priv)
+static void driver_bind(struct hotunplug *priv, int timeout)
 {
igt_debug("rebinding the driver to the device\n");
priv->failure = "Driver re-bind failure!";
 
-   igt_set_timeout(60, "Driver re-bind timeout!");
+   igt_set_timeout(timeout, "Driver re-bind timeout!");
igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
igt_reset_timeout();
 }
 
 /* Remove (virtually unplug) the device from its bus */
-static void device_unplug(struct hotunplug *priv, const char *prefix)
+static void device_unplug(struct hotunplug *priv, const char *prefix,
+ int timeout)
 {
igt_require(priv->fd.sysfs_dev == -1);
 
@@ -148,7 +150,7 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix)
igt_debug("%sunplugging the device\n", prefix);
priv->failure = "Device unplug failure!";
 
-   igt_set_timeout(60, "Device unplug timeout!");
+   igt_set_timeout(timeout, "Device unplug timeout!");
igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
igt_reset_timeout();
 
@@ -157,12 +159,12 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix)
 }
 
 /* Re-discover the device by rescanning its bus */
-static void bus_rescan(struct hotunplug *priv)
+static void bus_rescan(struct hotunplug *priv, int timeout)
 {
igt_debug("rediscovering the device\n");
priv->failure = "Bus rescan failure!";
 
-   igt_set_timeout(60, "Bus rescan timeout!");
+   igt_set_timeout(timeout, "Bus rescan timeout!");
igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
igt_reset_timeout();
 }
@@ -209,10 +211,10 @@ static void recover(struct hotunplug *priv)
cleanup(priv);
 
if (faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr, F_OK, 0))
-   bus_rescan(priv);
+   bus_rescan(priv, 60);
 
else if (faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr, F_OK, 0))
-   driver_bind(priv);
+   driver_bind(priv, 60);
 
if (priv->failure)
healthcheck(priv);
@@ -245,18 +247,18 @@ static void set_filter_from_device(int fd)
 
 static void unbind_rebind(struct hotunplug *priv)
 {
-   driver_unbind(priv, "");
+   driver_unbind(priv, "", 0);
 
-   driver_bind(priv);
+   driver_bind(priv, 0);
 
healthcheck(priv);
 }
 
 static void unplug_rescan(struct hotunplug *priv)
 {
-   device_unplug(priv, "");
+   device_unplug(priv, "", 0);
 
-   bus_rescan(priv);
+   bus_rescan(priv, 0);
 
healthcheck(priv);
 }
@@ -265,9 +267,9 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 {
priv->fd.drm = local_drm_open_driver("", " for hotunbind");
 
-   driver_unbind(priv, "hot ");
+   driver_unbind(priv, "hot ", 0);
 
-   driver_bind(priv);
+   driver_bind(priv, 60);
 
igt_debug("late closing the unbound device instance\n");
priv->fd.drm = close_device(priv->fd.drm);
@@ -278,9 +280,9 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 {
priv->fd.drm = local_drm_open_driver("", " for hotunplug");
 
-   device_unplug(priv, "hot ");
+   device_unplug(priv, "hot ", 0);
 
-   bus_rescan(priv);
+   bus_rescan(priv, 60);
 
igt_debug("late closing the removed device instance\n");
priv->fd.drm = close_device(priv->fd.drm);
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 11/20] tests/core_hotunplug: Recover from subtest failures

2020-08-21 Thread Janusz Krzysztofik
Subtests now forcibly call or request igt_abort on failures in order to
avoid silently leaving an exercised device in an unusable state.
However, a failure inside a subtest doesn't always mean the device is
no longer working correctly and reboot is needed.  On the other hand,
if a subtest just fails without aborting, that doesn't mean in turn the
device is healthy.  We should still perform a device health check
in that case before deciding on next steps.

Reuse the 'failure' structure field as a mark which is set before each
critical operation which must be followed by a successful health check
in order to avoid aborting the test is executed.  Then, move health
checks not essential for subtests out of those subtest bodies, or just
copy them if essentiall, to subtest associated individual follow-up
igt_fixture sections, from where device file descriptors potentially
left open are closed, device rediscover or driver rebing operation is
run as needed, and finally the health check is run if the preceding
igt_subtest section exited with the marker set.

v2: Start each recovery phase from unconditionally closing file
descriptors potentially left open by a subtest before it entered
its critical section,
  - replace igt_require() with 'if() return;' construct in recover() to
reduce noise,
  - replace "subtest failure" message used as a request for healthcheck
with a more appropriate "need healthcheck" for clarity,
  - rebase on current upstream master.
v3: Refresh,
  - move bus_rescan() and driver_bind() function calls back from
heaalthcheck() to recover() so a pure health check can still be
called from a subtest if essential,
  - move failure mark assignments back from subtests to helpers for
more adequate abort reason reporting but clean the mark only on
health check success,
  - call cleanup() also from post_healthcheck() in order to close a
device file descriptor potentially left open by a failed health
check,
  - reword commit message and update description.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski  # v1
---
 tests/core_hotunplug.c | 104 +
 1 file changed, 74 insertions(+), 30 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 602a91cf8..145593683 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -69,6 +69,9 @@ static int local_drm_open_driver(const char *prefix, const 
char *suffix)
 
 static int local_close(int fd, const char *message)
 {
+   if (fd < 0) /* not open - return current status */
+   return fd;
+
errno = 0;
if (igt_warn_on_f(close(fd), "%s\n", message))
return -errno;  /* (never -1) */
@@ -115,24 +118,22 @@ static void prepare(struct hotunplug *priv)
 static void driver_unbind(struct hotunplug *priv, const char *prefix)
 {
igt_debug("%sunbinding the driver from the device\n", prefix);
+   priv->failure = "Driver unbind failure!";
 
-   priv->failure = "Driver unbind timeout!";
-   igt_set_timeout(60, priv->failure);
+   igt_set_timeout(60, "Driver unbind timeout!");
igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
igt_reset_timeout();
-   priv->failure = NULL;
 }
 
 /* Re-bind the driver to the device */
 static void driver_bind(struct hotunplug *priv)
 {
igt_debug("rebinding the driver to the device\n");
+   priv->failure = "Driver re-bind failure!";
 
-   priv->failure = "Driver re-bind timeout!";
-   igt_set_timeout(60, priv->failure);
+   igt_set_timeout(60, "Driver re-bind timeout!");
igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
igt_reset_timeout();
-   priv->failure = NULL;
 }
 
 /* Remove (virtually unplug) the device from its bus */
@@ -145,12 +146,11 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix)
igt_assert_fd(priv->fd.sysfs_dev);
 
igt_debug("%sunplugging the device\n", prefix);
+   priv->failure = "Device unplug failure!";
 
-   priv->failure = "Device unplug timeout!";
-   igt_set_timeout(60, priv->failure);
+   igt_set_timeout(60, "Device unplug timeout!");
igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
igt_reset_timeout();
-   priv->failure = NULL;
 
priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
 }
@@ -159,17 +159,23 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix)
 static void bus_rescan(struct hotunplug *priv)
 {
igt_debug("rediscovering the device\n");
+   priv->failure = "Bus rescan failure!";
 
-   priv->failure = "Bus rescan timeout!";
-   igt_set_timeout(60, priv->failure);
+   igt_set_timeout(60, "Bus rescan timeout!");
igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
igt_reset_timeout();
-   priv->failure = NULL;
+}
+
+static void cleanup(struct hotunplug *priv)
+{
+   

[Intel-gfx] [PATCH i-g-t v4 14/20] tests/core_hotunplug: Process return values of sysfs operations

2020-08-21 Thread Janusz Krzysztofik
Return values of driver bind/unbind / device remove/recover sysfs
operations are now ignored.  Assert their correctness.

v2: Add trailing newlines missing from igt_assert messages.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 572c66474..f280771ab 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -122,7 +122,9 @@ static void driver_unbind(struct hotunplug *priv, const 
char *prefix,
priv->failure = "Driver unbind failure!";
 
igt_set_timeout(timeout, "Driver unbind timeout!");
-   igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
+   igt_assert_f(igt_sysfs_set(priv->fd.sysfs_drv, "unbind",
+  priv->dev_bus_addr),
+"Driver unbind failure!\n");
igt_reset_timeout();
 }
 
@@ -133,7 +135,9 @@ static void driver_bind(struct hotunplug *priv, int timeout)
priv->failure = "Driver re-bind failure!";
 
igt_set_timeout(timeout, "Driver re-bind timeout!");
-   igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
+   igt_assert_f(igt_sysfs_set(priv->fd.sysfs_drv, "bind",
+  priv->dev_bus_addr),
+"Driver re-bind failure\n!");
igt_reset_timeout();
 }
 
@@ -151,7 +155,8 @@ static void device_unplug(struct hotunplug *priv, const 
char *prefix,
priv->failure = "Device unplug failure!";
 
igt_set_timeout(timeout, "Device unplug timeout!");
-   igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
+   igt_assert_f(igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1"),
+"Device unplug failure\n!");
igt_reset_timeout();
 
priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
@@ -165,7 +170,8 @@ static void bus_rescan(struct hotunplug *priv, int timeout)
priv->failure = "Bus rescan failure!";
 
igt_set_timeout(timeout, "Bus rescan timeout!");
-   igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
+   igt_assert_f(igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1"),
+  "Bus rescan failure!\n");
igt_reset_timeout();
 }
 
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 01/20] tests/core_hotunplug: Use igt_assert_fd()

2020-08-21 Thread Janusz Krzysztofik
There is a new library helper that asserts validity of open file
descriptors.  Use it instead of open coding.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index e03f3b945..7431346b1 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -57,7 +57,7 @@ static void prepare_for_unbind(struct hotunplug *priv, char 
*buf, int buflen)
 
priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
O_DIRECTORY);
-   igt_assert(priv->fd.sysfs_drv >= 0);
+   igt_assert_fd(priv->fd.sysfs_drv);
 
len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
buf[len] = '\0';
@@ -72,10 +72,10 @@ static void prepare(struct hotunplug *priv, char *buf, int 
buflen)
 {
igt_debug("opening device\n");
priv->fd.drm = __drm_open_driver(DRIVER_ANY);
-   igt_assert(priv->fd.drm >= 0);
+   igt_assert_fd(priv->fd.drm);
 
priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
-   igt_assert(priv->fd.sysfs_dev >= 0);
+   igt_assert_fd(priv->fd.sysfs_dev);
 
if (buf) {
prepare_for_unbind(priv, buf, buflen);
@@ -83,7 +83,7 @@ static void prepare(struct hotunplug *priv, char *buf, int 
buflen)
/* prepare for bus rescan */
priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev,
"device/subsystem", O_DIRECTORY);
-   igt_assert(priv->fd.sysfs_bus >= 0);
+   igt_assert_fd(priv->fd.sysfs_bus);
}
 }
 
@@ -261,7 +261,7 @@ igt_main
 * a device file descriptor open for exit handler use.
 */
fd_drm = __drm_open_driver(DRIVER_ANY);
-   igt_assert(fd_drm >= 0);
+   igt_assert_fd(fd_drm);
 
if (is_i915_device(fd_drm))
igt_require_gem(fd_drm);
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 16/20] tests/core_hotunplug: Explicitly ignore unused return values

2020-08-21 Thread Janusz Krzysztofik
Some return values are not useful and can be ignored.  Wrap those cases
inside igt_ignore_warn(), not only to make sure compilers are happy but
also to clearly document our decisions.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index d30ad8525..24beed81a 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -256,7 +256,7 @@ static void set_filter_from_device(int fd)
char path[PATH_MAX + 1];
 
igt_assert(igt_sysfs_path(fd, path, PATH_MAX));
-   strncat(path, "/device", PATH_MAX - strlen(path));
+   igt_ignore_warn(strncat(path, "/device", PATH_MAX - strlen(path)));
igt_assert(realpath(path, dst));
 
igt_device_filter_free_all();
@@ -385,7 +385,7 @@ igt_main
igt_fixture {
post_healthcheck();
 
-   close(priv.fd.sysfs_bus);
-   close(priv.fd.sysfs_drv);
+   igt_ignore_warn(close(priv.fd.sysfs_bus));
+   igt_ignore_warn(close(priv.fd.sysfs_drv));
}
 }
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 05/20] tests/core_hotunplug: Assert successful device filter application

2020-08-21 Thread Janusz Krzysztofik
Return value of igt_device_filter_add() representing a number of
successfully installed device filters is now ignored.  Fail if not 1.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 5093233d7..46f9ad118 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -193,7 +193,7 @@ static void set_filter_from_device(int fd)
igt_assert(realpath(path, dst));
 
igt_device_filter_free_all();
-   igt_device_filter_add(filter);
+   igt_assert_eq(igt_device_filter_add(filter), 1);
 }
 
 /* Subtests */
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 03/20] tests/core_hotunplug: Clean up device open error handling

2020-08-21 Thread Janusz Krzysztofik
We don't use drm_driver_open() since in case of an i915 device it keeps
an extra file descriptor of the exercised device open for exit handler
use, while we would like to be able to close the device completely
before running certain test operations.  Instead, we call
__drm_driver_open() and handle its result ourselves.  Unlike
drm_driver_open() which skips on device open errors, we always fail or
abort the test in such case.  Moreover, we don't ensure that the i915
driver is idle before starting subtests like drm_open_driver() does.

Skip instead of failing on initial device open error.  Also, call
gem_quiescent_gpu() if an i915 device is detected.  For subsequent
device opens, define a local helper that fails on error and use it.  If
we think we need to abort the test execution on device open error, set
our failure marker first to trigger the abort from a follow up
igt_fixture section.

Signed-off-by: Janusz Krzysztofik 
---
 tests/core_hotunplug.c | 34 +++---
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index a4071f51e..e576a6c6c 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -49,6 +49,21 @@ struct hotunplug {
 
 /* Helpers */
 
+/**
+ * Subtests must be able to close examined devices completely.  Don't
+ * use drm_open_driver() since in case of an i915 device it opens it
+ * twice and keeps a second file descriptor open for exit handler use.
+ */
+static int local_drm_open_driver(void)
+{
+   int fd_drm;
+
+   fd_drm = __drm_open_driver(DRIVER_ANY);
+   igt_assert_fd(fd_drm);
+
+   return fd_drm;
+}
+
 static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 {
int len;
@@ -71,8 +86,7 @@ static void prepare_for_unbind(struct hotunplug *priv, char 
*buf, int buflen)
 static void prepare(struct hotunplug *priv, char *buf, int buflen)
 {
igt_debug("opening device\n");
-   priv->fd.drm = __drm_open_driver(DRIVER_ANY);
-   igt_assert_fd(priv->fd.drm);
+   priv->fd.drm = local_drm_open_driver();
 
priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
igt_assert_fd(priv->fd.sysfs_dev);
@@ -145,8 +159,9 @@ static void healthcheck(void)
igt_devices_scan(true);
 
igt_debug("reopening the device\n");
-   fd_drm = __drm_open_driver(DRIVER_ANY);
-   igt_abort_on_f(fd_drm < 0, "Device reopen failure");
+   failure = "Device reopen failure!";
+   fd_drm = local_drm_open_driver();
+   failure = NULL;
 
if (is_i915_device(fd_drm)) {
failure = "GEM failure";
@@ -255,16 +270,13 @@ igt_main
igt_fixture {
int fd_drm;
 
-   /**
-* As subtests must be able to close examined devices
-* completely, don't use drm_open_driver() as it keeps
-* a device file descriptor open for exit handler use.
-*/
fd_drm = __drm_open_driver(DRIVER_ANY);
-   igt_assert_fd(fd_drm);
+   igt_skip_on_f(fd_drm < 0, "No known DRM device found\n");
 
-   if (is_i915_device(fd_drm))
+   if (is_i915_device(fd_drm)) {
+   gem_quiescent_gpu(fd_drm);
igt_require_gem(fd_drm);
+   }
 
/* Make sure subtests always reopen the same device */
set_filter_from_device(fd_drm);
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 07/20] tests/core_hotunplug: Pass errors via a data structure field

2020-08-21 Thread Janusz Krzysztofik
A pointer to fatal error messages can be passed around via hotunplug
structure, no need to declare it as global.

v2: Rebase only.
v3: Refresh.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 96 +-
 1 file changed, 47 insertions(+), 49 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 95d326ee9..4f7e89c95 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -45,6 +45,7 @@ struct hotunplug {
int sysfs_drv;
} fd;
const char *dev_bus_addr;
+   const char *failure;
 };
 
 /* Helpers */
@@ -102,80 +103,77 @@ static void prepare(struct hotunplug *priv, char *buf, 
int buflen)
}
 }
 
-static const char *failure;
-
 /* Unbind the driver from the device */
-static void driver_unbind(int fd_sysfs_drv, const char *dev_bus_addr,
- const char *prefix)
+static void driver_unbind(struct hotunplug *priv, const char *prefix)
 {
igt_debug("%sunbinding the driver from the device\n", prefix);
 
-   failure = "Driver unbind timeout!";
-   igt_set_timeout(60, failure);
-   igt_sysfs_set(fd_sysfs_drv, "unbind", dev_bus_addr);
+   priv->failure = "Driver unbind timeout!";
+   igt_set_timeout(60, priv->failure);
+   igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
igt_reset_timeout();
-   failure = NULL;
+   priv->failure = NULL;
 
-   /* don't close fd_sysfs_drv, it will be used for driver rebinding */
+   /* don't close fd.sysfs_drv, it will be used for driver rebinding */
 }
 
 /* Re-bind the driver to the device */
-static void driver_bind(int fd_sysfs_drv, const char *dev_bus_addr)
+static void driver_bind(struct hotunplug *priv)
 {
igt_debug("rebinding the driver to the device\n");
 
-   failure = "Driver re-bind timeout!";
-   igt_set_timeout(60, failure);
-   igt_sysfs_set(fd_sysfs_drv, "bind", dev_bus_addr);
+   priv->failure = "Driver re-bind timeout!";
+   igt_set_timeout(60, priv->failure);
+   igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
igt_reset_timeout();
-   failure = NULL;
+   priv->failure = NULL;
 
-   close(fd_sysfs_drv);
+   close(priv->fd.sysfs_drv);
 }
 
 /* Remove (virtually unplug) the device from its bus */
-static void device_unplug(int fd_sysfs_dev, const char *prefix)
+static void device_unplug(struct hotunplug *priv, const char *prefix)
 {
igt_debug("%sunplugging the device\n", prefix);
 
-   failure = "Device unplug timeout!";
-   igt_set_timeout(60, failure);
-   igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
+   priv->failure = "Device unplug timeout!";
+   igt_set_timeout(60, priv->failure);
+   igt_sysfs_set(priv->fd.sysfs_dev, "device/remove", "1");
igt_reset_timeout();
-   failure = NULL;
+   priv->failure = NULL;
 
-   close(fd_sysfs_dev);
+   close(priv->fd.sysfs_dev);
 }
 
 /* Re-discover the device by rescanning its bus */
-static void bus_rescan(int fd_sysfs_bus)
+static void bus_rescan(struct hotunplug *priv)
 {
igt_debug("rediscovering the device\n");
 
-   failure = "Bus rescan timeout!";
-   igt_set_timeout(60, failure);
-   igt_sysfs_set(fd_sysfs_bus, "rescan", "1");
+   priv->failure = "Bus rescan timeout!";
+   igt_set_timeout(60, priv->failure);
+   igt_sysfs_set(priv->fd.sysfs_bus, "rescan", "1");
igt_reset_timeout();
-   failure = NULL;
+   priv->failure = NULL;
 
-   close(fd_sysfs_bus);
+   close(priv->fd.sysfs_bus);
 }
 
-static void healthcheck(void)
+static void healthcheck(struct hotunplug *priv)
 {
int fd_drm;
 
/* device name may have changed, rebuild IGT device list */
igt_devices_scan(true);
 
-   failure = "Device reopen failure!";
+   priv->failure = "Device reopen failure!";
fd_drm = local_drm_open_driver("re", " for healthcheck");
-   failure = NULL;
+   priv->failure = NULL;
 
if (is_i915_device(fd_drm)) {
-   failure = "GEM failure";
+   priv->failure = "GEM failure";
igt_require_gem(fd_drm);
-   failure = NULL;
+   priv->failure = NULL;
}
 
close(fd_drm);
@@ -207,11 +205,11 @@ static void unbind_rebind(struct hotunplug *priv)
igt_debug("closing the device\n");
close(priv->fd.drm);
 
-   driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "");
+   driver_unbind(priv, "");
 
-   driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
+   driver_bind(priv);
 
-   healthcheck();
+   healthcheck(priv);
 }
 
 static void unplug_rescan(struct hotunplug *priv)
@@ -221,11 +219,11 @@ static void unplug_rescan(struct hotunplug *priv)
igt_debug("closing the device\n");
close(priv->fd.drm);
 
-   

[Intel-gfx] [PATCH i-g-t v4 06/20] tests/core_hotunplug: Maintain a single data structure instance

2020-08-21 Thread Janusz Krzysztofik
The following changes to the test are planned:
- avoid global variables,
- skip subtest after device close errors,
- prepare invariant data only once per test run,
- move device health checks to igt_fixture sections,
- try to recover from subtest failures instead of aborting.
For that to be possible, maintain a single instance of hotunplug
structure at igt_main level and pass it down to subtests.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 56 --
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 46f9ad118..95d326ee9 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -198,68 +198,62 @@ static void set_filter_from_device(int fd)
 
 /* Subtests */
 
-static void unbind_rebind(void)
+static void unbind_rebind(struct hotunplug *priv)
 {
-   struct hotunplug priv;
char buf[PATH_MAX];
 
-   prepare(, buf, sizeof(buf));
+   prepare(priv, buf, sizeof(buf));
 
igt_debug("closing the device\n");
-   close(priv.fd.drm);
+   close(priv->fd.drm);
 
-   driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "");
+   driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "");
 
-   driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+   driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
 
healthcheck();
 }
 
-static void unplug_rescan(void)
+static void unplug_rescan(struct hotunplug *priv)
 {
-   struct hotunplug priv;
-
-   prepare(, NULL, 0);
+   prepare(priv, NULL, 0);
 
igt_debug("closing the device\n");
-   close(priv.fd.drm);
+   close(priv->fd.drm);
 
-   device_unplug(priv.fd.sysfs_dev, "");
+   device_unplug(priv->fd.sysfs_dev, "");
 
-   bus_rescan(priv.fd.sysfs_bus);
+   bus_rescan(priv->fd.sysfs_bus);
 
healthcheck();
 }
 
-static void hotunbind_lateclose(void)
+static void hotunbind_lateclose(struct hotunplug *priv)
 {
-   struct hotunplug priv;
char buf[PATH_MAX];
 
-   prepare(, buf, sizeof(buf));
+   prepare(priv, buf, sizeof(buf));
 
-   driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "hot ");
+   driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "hot ");
 
-   driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+   driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
 
igt_debug("late closing the unbound device instance\n");
-   close(priv.fd.drm);
+   close(priv->fd.drm);
 
healthcheck();
 }
 
-static void hotunplug_lateclose(void)
+static void hotunplug_lateclose(struct hotunplug *priv)
 {
-   struct hotunplug priv;
-
-   prepare(, NULL, 0);
+   prepare(priv, NULL, 0);
 
-   device_unplug(priv.fd.sysfs_dev, "hot ");
+   device_unplug(priv->fd.sysfs_dev, "hot ");
 
-   bus_rescan(priv.fd.sysfs_bus);
+   bus_rescan(priv->fd.sysfs_bus);
 
igt_debug("late closing the removed device instance\n");
-   close(priv.fd.drm);
+   close(priv->fd.drm);
 
healthcheck();
 }
@@ -268,6 +262,8 @@ static void hotunplug_lateclose(void)
 
 igt_main
 {
+   struct hotunplug priv;
+
igt_fixture {
int fd_drm;
 
@@ -287,28 +283,28 @@ igt_main
 
igt_describe("Check if the driver can be cleanly unbound from a device 
believed to be closed");
igt_subtest("unbind-rebind")
-   unbind_rebind();
+   unbind_rebind();
 
igt_fixture
igt_abort_on_f(failure, "%s\n", failure);
 
igt_describe("Check if a device believed to be closed can be cleanly 
unplugged");
igt_subtest("unplug-rescan")
-   unplug_rescan();
+   unplug_rescan();
 
igt_fixture
igt_abort_on_f(failure, "%s\n", failure);
 
igt_describe("Check if the driver can be cleanly unbound from a still 
open device, then released");
igt_subtest("hotunbind-lateclose")
-   hotunbind_lateclose();
+   hotunbind_lateclose();
 
igt_fixture
igt_abort_on_f(failure, "%s\n", failure);
 
igt_describe("Check if a still open device can be cleanly unplugged, 
then released");
igt_subtest("hotunplug-lateclose")
-   hotunplug_lateclose();
+   hotunplug_lateclose();
 
igt_fixture
igt_abort_on_f(failure, "%s\n", failure);
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 04/20] tests/core_hotunplug: Consolidate duplicated debug messages

2020-08-21 Thread Janusz Krzysztofik
Some debug messages which designate specific test operations, or their
greater parts at least, sound always the same, no matter which subtest
they are called from.  Emit them, possibly updated with subtest
specified modifiers, from inside respective helpers instead of
duplicating them in subtest bodies.

v2: Rebase only.
v3: Refresh and extend over new case (local_drm_open_driver),
  - allow callers to specify a message suffix as well where applicable.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski  # v1
---
 tests/core_hotunplug.c | 39 ---
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index e576a6c6c..5093233d7 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -54,10 +54,12 @@ struct hotunplug {
  * use drm_open_driver() since in case of an i915 device it opens it
  * twice and keeps a second file descriptor open for exit handler use.
  */
-static int local_drm_open_driver(void)
+static int local_drm_open_driver(const char *prefix, const char *suffix)
 {
int fd_drm;
 
+   igt_debug("%sopening device%s\n", prefix, suffix);
+
fd_drm = __drm_open_driver(DRIVER_ANY);
igt_assert_fd(fd_drm);
 
@@ -85,8 +87,7 @@ static void prepare_for_unbind(struct hotunplug *priv, char 
*buf, int buflen)
 
 static void prepare(struct hotunplug *priv, char *buf, int buflen)
 {
-   igt_debug("opening device\n");
-   priv->fd.drm = local_drm_open_driver();
+   priv->fd.drm = local_drm_open_driver("", " for subtest");
 
priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
igt_assert_fd(priv->fd.sysfs_dev);
@@ -104,8 +105,11 @@ static void prepare(struct hotunplug *priv, char *buf, int 
buflen)
 static const char *failure;
 
 /* Unbind the driver from the device */
-static void driver_unbind(int fd_sysfs_drv, const char *dev_bus_addr)
+static void driver_unbind(int fd_sysfs_drv, const char *dev_bus_addr,
+ const char *prefix)
 {
+   igt_debug("%sunbinding the driver from the device\n", prefix);
+
failure = "Driver unbind timeout!";
igt_set_timeout(60, failure);
igt_sysfs_set(fd_sysfs_drv, "unbind", dev_bus_addr);
@@ -118,6 +122,8 @@ static void driver_unbind(int fd_sysfs_drv, const char 
*dev_bus_addr)
 /* Re-bind the driver to the device */
 static void driver_bind(int fd_sysfs_drv, const char *dev_bus_addr)
 {
+   igt_debug("rebinding the driver to the device\n");
+
failure = "Driver re-bind timeout!";
igt_set_timeout(60, failure);
igt_sysfs_set(fd_sysfs_drv, "bind", dev_bus_addr);
@@ -128,8 +134,10 @@ static void driver_bind(int fd_sysfs_drv, const char 
*dev_bus_addr)
 }
 
 /* Remove (virtually unplug) the device from its bus */
-static void device_unplug(int fd_sysfs_dev)
+static void device_unplug(int fd_sysfs_dev, const char *prefix)
 {
+   igt_debug("%sunplugging the device\n", prefix);
+
failure = "Device unplug timeout!";
igt_set_timeout(60, failure);
igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
@@ -142,6 +150,8 @@ static void device_unplug(int fd_sysfs_dev)
 /* Re-discover the device by rescanning its bus */
 static void bus_rescan(int fd_sysfs_bus)
 {
+   igt_debug("rediscovering the device\n");
+
failure = "Bus rescan timeout!";
igt_set_timeout(60, failure);
igt_sysfs_set(fd_sysfs_bus, "rescan", "1");
@@ -158,9 +168,8 @@ static void healthcheck(void)
/* device name may have changed, rebuild IGT device list */
igt_devices_scan(true);
 
-   igt_debug("reopening the device\n");
failure = "Device reopen failure!";
-   fd_drm = local_drm_open_driver();
+   fd_drm = local_drm_open_driver("re", " for healthcheck");
failure = NULL;
 
if (is_i915_device(fd_drm)) {
@@ -199,10 +208,8 @@ static void unbind_rebind(void)
igt_debug("closing the device\n");
close(priv.fd.drm);
 
-   igt_debug("unbinding the driver from the device\n");
-   driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+   driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "");
 
-   igt_debug("rebinding the driver to the device\n");
driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
healthcheck();
@@ -217,10 +224,8 @@ static void unplug_rescan(void)
igt_debug("closing the device\n");
close(priv.fd.drm);
 
-   igt_debug("unplugging the device\n");
-   device_unplug(priv.fd.sysfs_dev);
+   device_unplug(priv.fd.sysfs_dev, "");
 
-   igt_debug("recovering the device\n");
bus_rescan(priv.fd.sysfs_bus);
 
healthcheck();
@@ -233,10 +238,8 @@ static void hotunbind_lateclose(void)
 
prepare(, buf, sizeof(buf));
 
-   igt_debug("hot unbinding the driver from the device\n");
-   driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+   driver_unbind(priv.fd.sysfs_drv, 

[Intel-gfx] [PATCH i-g-t v4 00/20] tests/core_hotunplug: Fixes and enhancements

2020-08-21 Thread Janusz Krzysztofik
Clean up the test code, add some new basic subtests, then unblock
unbind test variants.

Series changelog:
v2: New patch "Un-blocklist *bind* subtests added.
v3: Patch "Follow failed subtests with healthcheck" renamed to "Recover
from subtest failures".
  - a new patche "Clean up device open error handling" added, an old
patch "Fix missing newline" obsoleted by the new one dropped,
  - other new patches added:
- "Let the driver time out essential sysfs operations",
- "More thorough i915 healthcheck and recovery",
  - a patch "Add 'lateclose before restore' variants" from another
series included.
v4: Optional patch "Duplicate debug messages in dmesg" from another
series included.

@Michał: Since most v2/v3 updates are trivial, I've preserved your
v1/v2 Reviewd-by: except for a few patches with non-trivial changes,
where I marked your R-b as v1/v2 applicable.  Please have a look and
confirm if you are still OK with them.

@Tvrtko: Please support my attempt to remove the unbind test variants
from the blocklist.

@Petri, @Martin: Assuming CI results will be positive, please give me
your green light for merging this series if you have no objections.

Thanks,
Janusz

Janusz Krzysztofik (20):
  tests/core_hotunplug: Use igt_assert_fd()
  tests/core_hotunplug: Constify dev_bus_addr string
  tests/core_hotunplug: Clean up device open error handling
  tests/core_hotunplug: Consolidate duplicated debug messages
  tests/core_hotunplug: Assert successful device filter application
  tests/core_hotunplug: Maintain a single data structure instance
  tests/core_hotunplug: Pass errors via a data structure field
  tests/core_hotunplug: Handle device close errors
  tests/core_hotunplug: Prepare invariant data once per test run
  tests/core_hotunplug: Skip selectively on sysfs close errors
  tests/core_hotunplug: Recover from subtest failures
  tests/core_hotunplug: Fail subtests on device close errors
  tests/core_hotunplug: Let the driver time out essential sysfs
operations
  tests/core_hotunplug: Process return values of sysfs operations
  tests/core_hotunplug: Assert expected device presence/absence
  tests/core_hotunplug: Explicitly ignore unused return values
  tests/core_hotunplug: More thorough i915 healthcheck and recovery
  tests/core_hotunplug: Add 'lateclose before restore' variants
  tests/core_hotunplug: Duplicate debug messages in dmesg
  tests/core_hotunplug: Un-blocklist *bind* subtests

 tests/core_hotunplug.c   | 542 ++-
 tests/intel-ci/blacklist.txt |   2 +-
 2 files changed, 412 insertions(+), 132 deletions(-)

-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v4 02/20] tests/core_hotunplug: Constify dev_bus_addr string

2020-08-21 Thread Janusz Krzysztofik
Device bus address structure field is always initialized with a pointer
to a substring of the device sysfs path and never used for its
modification.  Declare it as a constant string.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Michał Winiarski 
---
 tests/core_hotunplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 7431346b1..a4071f51e 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -44,7 +44,7 @@ struct hotunplug {
int sysfs_bus;
int sysfs_drv;
} fd;
-   char *dev_bus_addr;
+   const char *dev_bus_addr;
 };
 
 /* Helpers */
-- 
2.21.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✓ Fi.CI.BAT: success for mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Patchwork
== Series Details ==

Series: mm: Track page table modifications in __apply_to_page_range()
URL   : https://patchwork.freedesktop.org/series/80896/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8913 -> Patchwork_18388


Summary
---

  **SUCCESS**

  No regressions found.

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

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@i915_pm_rpm@basic-pci-d3-state:
- fi-bsw-n3050:   [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/fi-bsw-n3050/igt@i915_pm_...@basic-pci-d3-state.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/fi-bsw-n3050/igt@i915_pm_...@basic-pci-d3-state.html

  * igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7500u:   [PASS][3] -> [DMESG-WARN][4] ([i915#2203])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/fi-kbl-7500u/igt@kms_chamel...@common-hpd-after-suspend.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/fi-kbl-7500u/igt@kms_chamel...@common-hpd-after-suspend.html

  
 Possible fixes 

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2:
- fi-skl-guc: [DMESG-WARN][5] ([i915#2203]) -> [PASS][6]
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html

  
 Warnings 

  * igt@i915_pm_rpm@module-reload:
- fi-kbl-x1275:   [DMESG-FAIL][7] ([i915#62] / [i915#95]) -> 
[DMESG-FAIL][8] ([i915#62])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/fi-kbl-x1275/igt@i915_pm_...@module-reload.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/fi-kbl-x1275/igt@i915_pm_...@module-reload.html

  * igt@kms_force_connector_basic@force-edid:
- fi-kbl-x1275:   [DMESG-WARN][9] ([i915#62] / [i915#92]) -> 
[DMESG-WARN][10] ([i915#62] / [i915#92] / [i915#95])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/fi-kbl-x1275/igt@kms_force_connector_ba...@force-edid.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/fi-kbl-x1275/igt@kms_force_connector_ba...@force-edid.html

  * igt@prime_vgem@basic-fence-flip:
- fi-kbl-x1275:   [DMESG-WARN][11] ([i915#62] / [i915#92] / [i915#95]) 
-> [DMESG-WARN][12] ([i915#62] / [i915#92]) +3 similar issues
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8913/fi-kbl-x1275/igt@prime_v...@basic-fence-flip.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/fi-kbl-x1275/igt@prime_v...@basic-fence-flip.html

  
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (38 -> 34)
--

  Missing(4): fi-byt-clapper fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


Build changes
-

  * Linux: CI_DRM_8913 -> Patchwork_18388

  CI-20190529: 20190529
  CI_DRM_8913: e18d8e120e73feaf39d84afe14f9a7f58b696785 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5770: f1d0c240ea2e631dfb9f493f37f8fb61cb2b1cf2 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18388: 55f8f3ceea96ca4a7b7578668aa4f239898e02f8 @ 
git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

55f8f3ceea96 mm: Track page table modifications in __apply_to_page_range()

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18388/index.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/4] drm/i915/gem: Sync the vmap PTEs upon construction

2020-08-21 Thread Chris Wilson
Quoting Linus Torvalds (2020-08-21 13:41:03)
> On Fri, Aug 21, 2020 at 1:50 AM Chris Wilson  wrote:
> >
> > Since synchronising the PTE after assignment is a manual step, use the
> > newly exported interface to flush the PTE after assigning via
> > alloc_vm_area().
> 
> This commit message doesn't make much sense to me.
> 
> Are you talking about synchronizing the page directory structure
> across processes after possibly creating new kernel page tables?
> 
> Because that has nothing to do with the PTE. It's all about making
> sure the _upper_ layers of the page directories are populated
> everywhere..
> 
> The name seems off to me too - what are you "flushing"? (And yes, I
> know about the flush_cache_vmap(), but that looks just bogus, since
> any non-mapped area shouldn't have any virtual caches to begin with,
> so I suspect that is just the crazy architectures being confused -
> flush_cache_vmap() is a no-op on any sane architecture - and powerpc
> that mis-uses it for other things).

I was trying to mimic map_kernel_range() which does the
arch_sync_kernel_mappings and flush_cache_vmap on top of the
apply_to_page_range performed by alloc_vm_area, because buried away in
there, on x86-32, is a set_pmd(). Since map_kernel_range() wrapped
map_kernel_range_noflush(), flush seemed like the right verb.

Joerg pointed out that the sync belonged to __apply_to_page_range and
fixed it in situ.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2] mm: Track page table modifications in __apply_to_page_range()

2020-08-21 Thread Joerg Roedel
From: Joerg Roedel 

The __apply_to_page_range() function is also used to change and/or
allocate page-table pages in the vmalloc area of the address space.
Make sure these changes get synchronized to other page-tables in the
system by calling arch_sync_kernel_mappings() when necessary.

Tested-by: Chris Wilson  #x86-32
Cc:  # v5.8+
Signed-off-by: Joerg Roedel 
---
 mm/memory.c | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 3a7779d9891d..1b7d846f6992 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -83,6 +83,7 @@
 #include 
 #include 
 
+#include "pgalloc-track.h"
 #include "internal.h"
 
 #if defined(LAST_CPUPID_NOT_IN_PAGE_FLAGS) && !defined(CONFIG_COMPILE_TEST)
@@ -2206,7 +2207,8 @@ EXPORT_SYMBOL(vm_iomap_memory);
 
 static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd,
 unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data, bool create)
+pte_fn_t fn, void *data, bool create,
+pgtbl_mod_mask *mask)
 {
pte_t *pte;
int err = 0;
@@ -2214,7 +2216,7 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t 
*pmd,
 
if (create) {
pte = (mm == _mm) ?
-   pte_alloc_kernel(pmd, addr) :
+   pte_alloc_kernel_track(pmd, addr, mask) :
pte_alloc_map_lock(mm, pmd, addr, );
if (!pte)
return -ENOMEM;
@@ -2235,6 +2237,7 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t 
*pmd,
break;
}
} while (addr += PAGE_SIZE, addr != end);
+   *mask |= PGTBL_PTE_MODIFIED;
 
arch_leave_lazy_mmu_mode();
 
@@ -2245,7 +2248,8 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t 
*pmd,
 
 static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
 unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data, bool create)
+pte_fn_t fn, void *data, bool create,
+pgtbl_mod_mask *mask)
 {
pmd_t *pmd;
unsigned long next;
@@ -2254,7 +2258,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, pud_t 
*pud,
BUG_ON(pud_huge(*pud));
 
if (create) {
-   pmd = pmd_alloc(mm, pud, addr);
+   pmd = pmd_alloc_track(mm, pud, addr, mask);
if (!pmd)
return -ENOMEM;
} else {
@@ -2264,7 +2268,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, pud_t 
*pud,
next = pmd_addr_end(addr, end);
if (create || !pmd_none_or_clear_bad(pmd)) {
err = apply_to_pte_range(mm, pmd, addr, next, fn, data,
-create);
+create, mask);
if (err)
break;
}
@@ -2274,14 +2278,15 @@ static int apply_to_pmd_range(struct mm_struct *mm, 
pud_t *pud,
 
 static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
 unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data, bool create)
+pte_fn_t fn, void *data, bool create,
+pgtbl_mod_mask *mask)
 {
pud_t *pud;
unsigned long next;
int err = 0;
 
if (create) {
-   pud = pud_alloc(mm, p4d, addr);
+   pud = pud_alloc_track(mm, p4d, addr, mask);
if (!pud)
return -ENOMEM;
} else {
@@ -2291,7 +2296,7 @@ static int apply_to_pud_range(struct mm_struct *mm, p4d_t 
*p4d,
next = pud_addr_end(addr, end);
if (create || !pud_none_or_clear_bad(pud)) {
err = apply_to_pmd_range(mm, pud, addr, next, fn, data,
-create);
+create, mask);
if (err)
break;
}
@@ -2301,14 +2306,15 @@ static int apply_to_pud_range(struct mm_struct *mm, 
p4d_t *p4d,
 
 static int apply_to_p4d_range(struct mm_struct *mm, pgd_t *pgd,
 unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data, bool create)
+pte_fn_t fn, void *data, bool create,
+pgtbl_mod_mask *mask)
 {
p4d_t *p4d;
unsigned long next;
int err = 0;
 
if (create) {
-   p4d = p4d_alloc(mm, pgd, addr);
+ 

Re: [Intel-gfx] [PATCH 2/4] drm/i915/gem: Sync the vmap PTEs upon construction

2020-08-21 Thread Linus Torvalds
On Fri, Aug 21, 2020 at 1:50 AM Chris Wilson  wrote:
>
> Since synchronising the PTE after assignment is a manual step, use the
> newly exported interface to flush the PTE after assigning via
> alloc_vm_area().

This commit message doesn't make much sense to me.

Are you talking about synchronizing the page directory structure
across processes after possibly creating new kernel page tables?

Because that has nothing to do with the PTE. It's all about making
sure the _upper_ layers of the page directories are populated
everywhere..

The name seems off to me too - what are you "flushing"? (And yes, I
know about the flush_cache_vmap(), but that looks just bogus, since
any non-mapped area shouldn't have any virtual caches to begin with,
so I suspect that is just the crazy architectures being confused -
flush_cache_vmap() is a no-op on any sane architecture - and powerpc
that mis-uses it for other things).

   Linus
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] mm: Track page table modifications in __apply_to_page_range() construction

2020-08-21 Thread Joerg Roedel
On Fri, Aug 21, 2020 at 12:38:03PM +0100, Chris Wilson wrote:
> In the version I tested, I also had
> 
> @@ -2216,7 +2216,7 @@ static int apply_to_pte_range(struct mm_struct *mm, 
> pmd_t *pmd,
> 
> if (create) {
> pte = (mm == _mm) ?
> -   pte_alloc_kernel(pmd, addr) :
> +   pte_alloc_kernel_track(pmd, addr, mask) :
> pte_alloc_map_lock(mm, pmd, addr, );
> if (!pte)
> return -ENOMEM;
> 
> And that PGTBL_PMD_MODIFIED makes a difference.

Right, thanks. Added that too.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] mm: Track page table modifications in __apply_to_page_range() construction

2020-08-21 Thread Chris Wilson
Quoting Chris Wilson (2020-08-21 11:39:19)
> Quoting Joerg Roedel (2020-08-21 11:23:43)
> > On Fri, Aug 21, 2020 at 11:13:36AM +0100, Chris Wilson wrote:
> > > We need to store the initial addr, as here addr == end [or earlier on
> > > earlier], so range is (start, addr).
> > 
> > Right, I missed that, thanks for pointing it out.
> 
> And with that (start, addr)
> 
> Tested-by: Chris Wilson  #x86-32

In the version I tested, I also had

@@ -2216,7 +2216,7 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t 
*pmd,

if (create) {
pte = (mm == _mm) ?
-   pte_alloc_kernel(pmd, addr) :
+   pte_alloc_kernel_track(pmd, addr, mask) :
pte_alloc_map_lock(mm, pmd, addr, );
if (!pte)
return -ENOMEM;

And that PGTBL_PMD_MODIFIED makes a difference.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Patchwork
== Series Details ==

Series: series starting with [1/4] mm: Export flush_vm_area() to sync the PTEs 
upon construction
URL   : https://patchwork.freedesktop.org/series/80892/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8911_full -> Patchwork_18386_full


Summary
---

  **SUCCESS**

  No regressions found.

  

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_exec_suspend@basic-s3:
- shard-snb:  [PASS][1] -> [TIMEOUT][2] ([i915#1958]) +1 similar 
issue
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-snb2/igt@gem_exec_susp...@basic-s3.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-snb4/igt@gem_exec_susp...@basic-s3.html

  * igt@gem_exec_whisper@basic-fds-forked-all:
- shard-glk:  [PASS][3] -> [DMESG-WARN][4] ([i915#118] / [i915#95])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-glk4/igt@gem_exec_whis...@basic-fds-forked-all.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-glk7/igt@gem_exec_whis...@basic-fds-forked-all.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
- shard-glk:  [PASS][5] -> [DMESG-FAIL][6] ([i915#118] / [i915#95])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-glk3/igt@kms_big...@x-tiled-64bpp-rotate-180.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-glk8/igt@kms_big...@x-tiled-64bpp-rotate-180.html

  * igt@kms_flip@flip-vs-fences@a-edp1:
- shard-skl:  [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +13 
similar issues
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-skl8/igt@kms_flip@flip-vs-fen...@a-edp1.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-skl5/igt@kms_flip@flip-vs-fen...@a-edp1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
- shard-kbl:  [PASS][9] -> [DMESG-WARN][10] ([i915#180]) +6 similar 
issues
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-kbl2/igt@kms_flip@flip-vs-susp...@c-dp1.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-kbl7/igt@kms_flip@flip-vs-susp...@c-dp1.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-kbl:  [PASS][11] -> [INCOMPLETE][12] ([i915#155])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-kbl1/igt@kms_frontbuffer_track...@fbc-suspend.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-kbl4/igt@kms_frontbuffer_track...@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-cpu:
- shard-tglb: [PASS][13] -> [DMESG-WARN][14] ([i915#1982]) +1 
similar issue
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-tglb5/igt@kms_frontbuffer_track...@fbcpsr-rgb565-draw-mmap-cpu.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-tglb3/igt@kms_frontbuffer_track...@fbcpsr-rgb565-draw-mmap-cpu.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl:  [PASS][15] -> [FAIL][16] ([fdo#108145] / [i915#265])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-skl1/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-skl8/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl:  [PASS][17] -> [DMESG-FAIL][18] ([fdo#108145] / 
[i915#1982])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-skl2/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-skl4/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html

  * igt@kms_plane_cursor@pipe-a-primary-size-128:
- shard-glk:  [PASS][19] -> [DMESG-WARN][20] ([i915#1982])
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-glk9/igt@kms_plane_cur...@pipe-a-primary-size-128.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-glk2/igt@kms_plane_cur...@pipe-a-primary-size-128.html

  * igt@kms_psr2_su@frontbuffer:
- shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109642] / [fdo#111068])
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-iclb2/igt@kms_psr2...@frontbuffer.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-iclb5/igt@kms_psr2...@frontbuffer.html

  * igt@perf@blocking-parameterized:
- shard-iclb: [PASS][23] -> [FAIL][24] ([i915#1542]) +1 similar 
issue
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/shard-iclb2/igt@p...@blocking-parameterized.html
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/shard-iclb2/igt@p...@blocking-parameterized.html

  
 Possible fixes 

Re: [Intel-gfx] [PATCH] mm: Track page table modifications in __apply_to_page_range() construction

2020-08-21 Thread Greg KH
On Fri, Aug 21, 2020 at 12:09:02PM +0200, Joerg Roedel wrote:
> The __apply_to_page_range() function is also used to change and/or
> allocate page-table pages in the vmalloc area of the address space.
> Make sure these changes get synchronized to other page-tables in the
> system by calling arch_sync_kernel_mappings() when necessary.
> 
> Signed-off-by: Joerg Roedel 
> ---
> (Only compile tested on x86-64 so far)
> 
>  mm/memory.c | 32 +---
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 3a7779d9891d..fd845991f14a 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -83,6 +83,7 @@
>  #include 
>  #include 
>  
> +#include "pgalloc-track.h"
>  #include "internal.h"
>  
>  #if defined(LAST_CPUPID_NOT_IN_PAGE_FLAGS) && !defined(CONFIG_COMPILE_TEST)
> @@ -2206,7 +2207,8 @@ EXPORT_SYMBOL(vm_iomap_memory);
>  
>  static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd,
>unsigned long addr, unsigned long end,
> -  pte_fn_t fn, void *data, bool create)
> +  pte_fn_t fn, void *data, bool create,
> +  pgtbl_mod_mask *mask)
>  {
>   pte_t *pte;
>   int err = 0;
> @@ -2235,6 +2237,7 @@ static int apply_to_pte_range(struct mm_struct *mm, 
> pmd_t *pmd,
>   break;
>   }
>   } while (addr += PAGE_SIZE, addr != end);
> + *mask |= PGTBL_PTE_MODIFIED;
>  
>   arch_leave_lazy_mmu_mode();
>  
> @@ -2245,7 +2248,8 @@ static int apply_to_pte_range(struct mm_struct *mm, 
> pmd_t *pmd,
>  
>  static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
>unsigned long addr, unsigned long end,
> -  pte_fn_t fn, void *data, bool create)
> +  pte_fn_t fn, void *data, bool create,
> +  pgtbl_mod_mask *mask)
>  {
>   pmd_t *pmd;
>   unsigned long next;
> @@ -2254,7 +2258,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, 
> pud_t *pud,
>   BUG_ON(pud_huge(*pud));
>  
>   if (create) {
> - pmd = pmd_alloc(mm, pud, addr);
> + pmd = pmd_alloc_track(mm, pud, addr, mask);
>   if (!pmd)
>   return -ENOMEM;
>   } else {
> @@ -2264,7 +2268,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, 
> pud_t *pud,
>   next = pmd_addr_end(addr, end);
>   if (create || !pmd_none_or_clear_bad(pmd)) {
>   err = apply_to_pte_range(mm, pmd, addr, next, fn, data,
> -  create);
> +  create, mask);
>   if (err)
>   break;
>   }
> @@ -2274,14 +2278,15 @@ static int apply_to_pmd_range(struct mm_struct *mm, 
> pud_t *pud,
>  
>  static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
>unsigned long addr, unsigned long end,
> -  pte_fn_t fn, void *data, bool create)
> +  pte_fn_t fn, void *data, bool create,
> +  pgtbl_mod_mask *mask)
>  {
>   pud_t *pud;
>   unsigned long next;
>   int err = 0;
>  
>   if (create) {
> - pud = pud_alloc(mm, p4d, addr);
> + pud = pud_alloc_track(mm, p4d, addr, mask);
>   if (!pud)
>   return -ENOMEM;
>   } else {
> @@ -2291,7 +2296,7 @@ static int apply_to_pud_range(struct mm_struct *mm, 
> p4d_t *p4d,
>   next = pud_addr_end(addr, end);
>   if (create || !pud_none_or_clear_bad(pud)) {
>   err = apply_to_pmd_range(mm, pud, addr, next, fn, data,
> -  create);
> +  create, mask);
>   if (err)
>   break;
>   }
> @@ -2301,14 +2306,15 @@ static int apply_to_pud_range(struct mm_struct *mm, 
> p4d_t *p4d,
>  
>  static int apply_to_p4d_range(struct mm_struct *mm, pgd_t *pgd,
>unsigned long addr, unsigned long end,
> -  pte_fn_t fn, void *data, bool create)
> +  pte_fn_t fn, void *data, bool create,
> +  pgtbl_mod_mask *mask)
>  {
>   p4d_t *p4d;
>   unsigned long next;
>   int err = 0;
>  
>   if (create) {
> - p4d = p4d_alloc(mm, pgd, addr);
> + p4d = p4d_alloc_track(mm, pgd, addr, mask);
>   if (!p4d)
>   return -ENOMEM;
>   } else {
> @@ -2318,7 +2324,7 @@ static int apply_to_p4d_range(struct mm_struct *mm, 
> pgd_t *pgd,
>   next = 

Re: [Intel-gfx] [PATCH] mm: Track page table modifications in __apply_to_page_range() construction

2020-08-21 Thread Chris Wilson
Quoting Joerg Roedel (2020-08-21 11:23:43)
> On Fri, Aug 21, 2020 at 11:13:36AM +0100, Chris Wilson wrote:
> > We need to store the initial addr, as here addr == end [or earlier on
> > earlier], so range is (start, addr).
> 
> Right, I missed that, thanks for pointing it out.

And with that (start, addr)

Tested-by: Chris Wilson  #x86-32
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Chris Wilson
Quoting Joerg Roedel (2020-08-21 11:22:04)
> On Fri, Aug 21, 2020 at 10:54:22AM +0100, Chris Wilson wrote:
> > Ok. I thought it had to be after assigning the *ptep. If we apply the
> > sync first, do not have to worry about PGTBL_PTE_MODIFIED from the
> > *ptep?
> 
> Hmm, if I understand the code correctly, you are re-implementing some
> generic ioremap/vmalloc mapping logic in the i915 driver. I don't know
> the reason, but if it is valid you need to manually call
> arch_sync_kernel_mappings() from your driver like this to be correct:
> 
> if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PTE_MODIFIED)
> arch_sync_kernel_mappings();
> 
> In practice this is a no-op, because nobody sets PGTBL_PTE_MODIFIED in
> ARCH_PAGE_TABLE_SYNC_MASK, so the above code would be optimized away.
> 
> But what you really care about is the tracking in apply_to_page_range(),
> as that allocates the !pte levels of your page-table, which needs
> synchronization on x86-32.
> 
> Btw, what are the reasons you can't use generic vmalloc/ioremap
> interfaces to map the range?

ioremap_prot and ioremap_page_range assume a contiguous IO address. So
we needed to allocate the vmalloc area [and would then need to iterate
over the discontiguous iomem chunks with ioremap_page_range], and since
alloc_vm_area returned the ptep, it looked clearer to then assign those
according to whether we wanted ioremapping or a plain page. So we ended
up with one call to the core to return us a vm_struct and a pte array
that worked for either backing store.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with mm: Track page table modifications in __apply_to_page_range() construction (rev2)

2020-08-21 Thread Patchwork
== Series Details ==

Series: series starting with mm: Track page table modifications in 
__apply_to_page_range() construction (rev2)
URL   : https://patchwork.freedesktop.org/series/80892/
State : failure

== Summary ==

CALLscripts/checksyscalls.sh
  CALLscripts/atomic/check-atomics.sh
  DESCEND  objtool
  CHK include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/gem/i915_gem_pages.o
drivers/gpu/drm/i915/gem/i915_gem_pages.c: In function ‘i915_gem_object_map’:
drivers/gpu/drm/i915/gem/i915_gem_pages.c:318:2: error: implicit declaration of 
function ‘flush_vm_area’; did you mean ‘free_vm_area’? 
[-Werror=implicit-function-declaration]
  flush_vm_area(area);
  ^
  free_vm_area
cc1: all warnings being treated as errors
scripts/Makefile.build:283: recipe for target 
'drivers/gpu/drm/i915/gem/i915_gem_pages.o' failed
make[4]: *** [drivers/gpu/drm/i915/gem/i915_gem_pages.o] Error 1
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1789: recipe for target 'drivers' failed
make: *** [drivers] Error 2


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] mm: Track page table modifications in __apply_to_page_range() construction

2020-08-21 Thread Joerg Roedel
On Fri, Aug 21, 2020 at 11:13:36AM +0100, Chris Wilson wrote:
> We need to store the initial addr, as here addr == end [or earlier on
> earlier], so range is (start, addr).

Right, I missed that, thanks for pointing it out.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Joerg Roedel
On Fri, Aug 21, 2020 at 10:54:22AM +0100, Chris Wilson wrote:
> Ok. I thought it had to be after assigning the *ptep. If we apply the
> sync first, do not have to worry about PGTBL_PTE_MODIFIED from the
> *ptep?

Hmm, if I understand the code correctly, you are re-implementing some
generic ioremap/vmalloc mapping logic in the i915 driver. I don't know
the reason, but if it is valid you need to manually call
arch_sync_kernel_mappings() from your driver like this to be correct:

if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PTE_MODIFIED)
arch_sync_kernel_mappings();

In practice this is a no-op, because nobody sets PGTBL_PTE_MODIFIED in
ARCH_PAGE_TABLE_SYNC_MASK, so the above code would be optimized away.

But what you really care about is the tracking in apply_to_page_range(),
as that allocates the !pte levels of your page-table, which needs
synchronization on x86-32.

Btw, what are the reasons you can't use generic vmalloc/ioremap
interfaces to map the range?

Regards,

Joerg
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v6 05/11] drm/i915: Try to make bigjoiner work in atomic check

2020-08-21 Thread Manna, Animesh



On 16-07-2020 04:12, Manasi Navare wrote:

From: Maarten Lankhorst 

  When the clock is higher than the dotclock, try with 2 pipes enabled.
  If we can enable 2, then we will go into big joiner mode, and steal
  the adjacent crtc.

  This only links the crtc's in software, no hardware or plane
  programming is done yet. Blobs are also copied from the master's
  crtc_state, so it doesn't depend at commit time on the other
  crtc_state.

v3:
* Manual Rebase (Manasi)
  Changes since v1:
  - Rename pipe timings to transcoder timings, as they are now different.
   Changes since v2:
  - Rework bigjoiner checks; always disable slave when recalculating
master. No need to have a separate bigjoiner pass any more.
  - Use pipe_mode instead of transcoder_mode, to clean up the code.

Signed-off-by: Maarten Lankhorst 
Signed-off-by: Manasi Navare 
---
  drivers/gpu/drm/i915/display/intel_atomic.c   |   9 +-
  drivers/gpu/drm/i915/display/intel_atomic.h   |   3 +-
  drivers/gpu/drm/i915/display/intel_display.c  | 201 --
  .../drm/i915/display/intel_display_types.h|   9 +
  drivers/gpu/drm/i915/display/intel_dp.c   |  22 +-
  5 files changed, 211 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index 630f49b7aa01..b9dcdc74a10d 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -270,14 +270,15 @@ void intel_crtc_free_hw_state(struct intel_crtc_state 
*crtc_state)
intel_crtc_put_color_blobs(crtc_state);
  }
  
-void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state)

+void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state,
+const struct intel_crtc_state *from_crtc_state)


can we use primary_crtc_state instead of from_crtc_state?


  {
drm_property_replace_blob(_state->hw.degamma_lut,
- crtc_state->uapi.degamma_lut);
+ from_crtc_state->uapi.degamma_lut);
drm_property_replace_blob(_state->hw.gamma_lut,
- crtc_state->uapi.gamma_lut);
+ from_crtc_state->uapi.gamma_lut);
drm_property_replace_blob(_state->hw.ctm,
- crtc_state->uapi.ctm);
+ from_crtc_state->uapi.ctm);
  }
  
  /**

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h 
b/drivers/gpu/drm/i915/display/intel_atomic.h
index 11146292b06f..fc556c032c8f 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic.h
@@ -43,7 +43,8 @@ struct drm_crtc_state *intel_crtc_duplicate_state(struct 
drm_crtc *crtc);
  void intel_crtc_destroy_state(struct drm_crtc *crtc,
   struct drm_crtc_state *state);
  void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state);
-void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state);
+void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state,
+const struct intel_crtc_state 
*from_crtc_state);
  struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
  void intel_atomic_state_free(struct drm_atomic_state *state);
  void intel_atomic_state_clear(struct drm_atomic_state *state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 3ecb642805a6..955e19abb563 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8016,9 +8016,24 @@ static int intel_crtc_compute_config(struct intel_crtc 
*crtc,
 struct intel_crtc_state *pipe_config)
  {
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   const struct drm_display_mode *pipe_mode = _config->hw.pipe_mode;
+   struct drm_display_mode *pipe_mode = _config->hw.pipe_mode;
int clock_limit = dev_priv->max_dotclk_freq;
  
+	*pipe_mode = pipe_config->hw.adjusted_mode;

+
+   /* Adjust pipe_mode for bigjoiner, with half the horizontal mode */
+   if (pipe_config->bigjoiner) {
+   pipe_mode->crtc_clock /= 2;
+   pipe_mode->crtc_hdisplay /= 2;
+   pipe_mode->crtc_hblank_start /= 2;
+   pipe_mode->crtc_hblank_end /= 2;
+   pipe_mode->crtc_hsync_start /= 2;
+   pipe_mode->crtc_hsync_end /= 2;
+   pipe_mode->crtc_htotal /= 2;
+   pipe_mode->crtc_hskew /= 2;
+   pipe_config->pipe_src_w /= 2;
+   }
+
if (INTEL_GEN(dev_priv) < 4) {
clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
  
@@ -8079,7 +8094,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,

 * WaPruneModeWithIncorrectHsyncOffset:ctg,elk,ilk,snb,ivb,vlv,hsw.
 */
if 

Re: [Intel-gfx] [PATCH] mm: Track page table modifications in __apply_to_page_range() construction

2020-08-21 Thread Chris Wilson
Quoting Joerg Roedel (2020-08-21 11:09:02)
> @@ -2333,6 +2339,7 @@ static int __apply_to_page_range(struct mm_struct *mm, 
> unsigned long addr,
> pgd_t *pgd;
> unsigned long next;
> unsigned long end = addr + size;
> +   pgtbl_mod_mask mask = 0;
> int err = 0;
>  
> if (WARN_ON(addr >= end))
> @@ -2343,11 +2350,14 @@ static int __apply_to_page_range(struct mm_struct 
> *mm, unsigned long addr,
> next = pgd_addr_end(addr, end);
> if (!create && pgd_none_or_clear_bad(pgd))
> continue;
> -   err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, 
> create);
> +   err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, 
> create, );
> if (err)
> break;
> } while (pgd++, addr = next, addr != end);
>  
> +   if (mask & ARCH_PAGE_TABLE_SYNC_MASK)
> +   arch_sync_kernel_mappings(addr, addr + size);

We need to store the initial addr, as here addr == end [or earlier on
earlier], so range is (start, addr).
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t v3 00/19] tests/core_hotunplug: Fixes and enhancements

2020-08-21 Thread Janusz Krzysztofik
On Thu, 2020-08-20 at 16:51 +0200, Janusz Krzysztofik wrote:
> Clean up the test code, add some new basic subtests, then unblock
> unbind test variants.

Hi,

CI results show that i915 recovery after a failed healthcheck still
needs some work, so please hold on with your reviews.  I'm going to
submit v4 soon.

Thanks,
Janusz

> 
> One patch has been renamed, three new patches added to the series, and
> one more patch form a formerly submitted series with new subtests
> included.
> 
> @Michał: Since most v2/v3 updates are trivial, I've preserved your
> v1/v2 Reviewd-by: except for a few patches with non-trivial changes,
> where I marked your R-b as v1/v2 applicable.  Please have a look and
> confirm if you are still OK with them.
> 
> @Tvrtko: Please support my attempt to remove the unbind test variants
> from the blocklist.
> 
> @Petri, @Martin: Please give me your green lite for merging this
> series if you have no objections.
> 
> Thanks,
> Janusz
> 
> Janusz Krzysztofik (19):
>   tests/core_hotunplug: Use igt_assert_fd()
>   tests/core_hotunplug: Constify dev_bus_addr string
>   tests/core_hotunplug: Clean up device open error handling
>   tests/core_hotunplug: Consolidate duplicated debug messages # new
>   tests/core_hotunplug: Assert successful device filter application
>   tests/core_hotunplug: Maintain a single data structure instance
>   tests/core_hotunplug: Pass errors via a data structure field
>   tests/core_hotunplug: Handle device close errors
>   tests/core_hotunplug: Prepare invariant data once per test run
>   tests/core_hotunplug: Skip selectively on sysfs close errors
>   tests/core_hotunplug: Recover from subtest failures # renamed
>   tests/core_hotunplug: Fail subtests on device close errors
>   tests/core_hotunplug: Let the driver time out essential sysfs operations # 
> new
>   tests/core_hotunplug: Process return values of sysfs operations
>   tests/core_hotunplug: Assert expected device presence/absence
>   tests/core_hotunplug: Explicitly ignore unused return values
>   tests/core_hotunplug: More thorough i915 healthcheck and recovery # new
>   tests/core_hotunplug: Add 'lateclose before restore' variants # included
>   tests/core_hotunplug: Un-blocklist *bind* subtests
> 
>  tests/core_hotunplug.c   | 525 ++-
>  tests/intel-ci/blacklist.txt |   2 +-
>  2 files changed, 396 insertions(+), 131 deletions(-)
> 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] mm: Track page table modifications in __apply_to_page_range() construction

2020-08-21 Thread Joerg Roedel
The __apply_to_page_range() function is also used to change and/or
allocate page-table pages in the vmalloc area of the address space.
Make sure these changes get synchronized to other page-tables in the
system by calling arch_sync_kernel_mappings() when necessary.

Signed-off-by: Joerg Roedel 
---
(Only compile tested on x86-64 so far)

 mm/memory.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 3a7779d9891d..fd845991f14a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -83,6 +83,7 @@
 #include 
 #include 
 
+#include "pgalloc-track.h"
 #include "internal.h"
 
 #if defined(LAST_CPUPID_NOT_IN_PAGE_FLAGS) && !defined(CONFIG_COMPILE_TEST)
@@ -2206,7 +2207,8 @@ EXPORT_SYMBOL(vm_iomap_memory);
 
 static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd,
 unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data, bool create)
+pte_fn_t fn, void *data, bool create,
+pgtbl_mod_mask *mask)
 {
pte_t *pte;
int err = 0;
@@ -2235,6 +2237,7 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t 
*pmd,
break;
}
} while (addr += PAGE_SIZE, addr != end);
+   *mask |= PGTBL_PTE_MODIFIED;
 
arch_leave_lazy_mmu_mode();
 
@@ -2245,7 +2248,8 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t 
*pmd,
 
 static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
 unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data, bool create)
+pte_fn_t fn, void *data, bool create,
+pgtbl_mod_mask *mask)
 {
pmd_t *pmd;
unsigned long next;
@@ -2254,7 +2258,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, pud_t 
*pud,
BUG_ON(pud_huge(*pud));
 
if (create) {
-   pmd = pmd_alloc(mm, pud, addr);
+   pmd = pmd_alloc_track(mm, pud, addr, mask);
if (!pmd)
return -ENOMEM;
} else {
@@ -2264,7 +2268,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, pud_t 
*pud,
next = pmd_addr_end(addr, end);
if (create || !pmd_none_or_clear_bad(pmd)) {
err = apply_to_pte_range(mm, pmd, addr, next, fn, data,
-create);
+create, mask);
if (err)
break;
}
@@ -2274,14 +2278,15 @@ static int apply_to_pmd_range(struct mm_struct *mm, 
pud_t *pud,
 
 static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
 unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data, bool create)
+pte_fn_t fn, void *data, bool create,
+pgtbl_mod_mask *mask)
 {
pud_t *pud;
unsigned long next;
int err = 0;
 
if (create) {
-   pud = pud_alloc(mm, p4d, addr);
+   pud = pud_alloc_track(mm, p4d, addr, mask);
if (!pud)
return -ENOMEM;
} else {
@@ -2291,7 +2296,7 @@ static int apply_to_pud_range(struct mm_struct *mm, p4d_t 
*p4d,
next = pud_addr_end(addr, end);
if (create || !pud_none_or_clear_bad(pud)) {
err = apply_to_pmd_range(mm, pud, addr, next, fn, data,
-create);
+create, mask);
if (err)
break;
}
@@ -2301,14 +2306,15 @@ static int apply_to_pud_range(struct mm_struct *mm, 
p4d_t *p4d,
 
 static int apply_to_p4d_range(struct mm_struct *mm, pgd_t *pgd,
 unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data, bool create)
+pte_fn_t fn, void *data, bool create,
+pgtbl_mod_mask *mask)
 {
p4d_t *p4d;
unsigned long next;
int err = 0;
 
if (create) {
-   p4d = p4d_alloc(mm, pgd, addr);
+   p4d = p4d_alloc_track(mm, pgd, addr, mask);
if (!p4d)
return -ENOMEM;
} else {
@@ -2318,7 +2324,7 @@ static int apply_to_p4d_range(struct mm_struct *mm, pgd_t 
*pgd,
next = p4d_addr_end(addr, end);
if (create || !p4d_none_or_clear_bad(p4d)) {
err = apply_to_pud_range(mm, p4d, addr, next, fn, data,
-   

Re: [Intel-gfx] [PATCH 1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Chris Wilson
Quoting Joerg Roedel (2020-08-21 10:51:29)
> On Fri, Aug 21, 2020 at 09:50:08AM +0100, Chris Wilson wrote:
> > The alloc_vm_area() is another method for drivers to
> > vmap/map_kernel_range that uses apply_to_page_range() rather than the
> > direct vmalloc walkers. This is missing the page table modification
> > tracking, and the ability to synchronize the PTE updates afterwards.
> > Provide flush_vm_area() for the users of alloc_vm_area() that assumes
> > the worst and ensures that the page directories are correctly flushed
> > upon construction.
> > 
> > The impact is most pronounced on x86_32 due to the delayed set_pmd().
> > 
> > Reported-by: Pavel Machek 
> > References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were 
> > modified")
> > References: 86cf69f1d893 ("x86/mm/32: implement 
> > arch_sync_kernel_mappings()")
> > Signed-off-by: Chris Wilson 
> > Cc: Andrew Morton 
> > Cc: Joerg Roedel 
> > Cc: Linus Torvalds 
> > Cc: Dave Airlie 
> > Cc: Joonas Lahtinen 
> > Cc: Rodrigo Vivi 
> > Cc: Pavel Machek 
> > Cc: David Vrabel 
> > Cc:  # v5.8+
> > ---
> >  include/linux/vmalloc.h |  1 +
> >  mm/vmalloc.c| 16 
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > index 0221f852a7e1..a253b27df0ac 100644
> > --- a/include/linux/vmalloc.h
> > +++ b/include/linux/vmalloc.h
> > @@ -204,6 +204,7 @@ static inline void set_vm_flush_reset_perms(void *addr)
> >  
> >  /* Allocate/destroy a 'vmalloc' VM area. */
> >  extern struct vm_struct *alloc_vm_area(size_t size, pte_t **ptes);
> > +extern void flush_vm_area(struct vm_struct *area);
> >  extern void free_vm_area(struct vm_struct *area);
> >  
> >  /* for /dev/kmem */
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index b482d240f9a2..c41934486031 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -3078,6 +3078,22 @@ struct vm_struct *alloc_vm_area(size_t size, pte_t 
> > **ptes)
> >  }
> >  EXPORT_SYMBOL_GPL(alloc_vm_area);
> >  
> > +void flush_vm_area(struct vm_struct *area)
> > +{
> > + unsigned long addr = (unsigned long)area->addr;
> > +
> > + /* apply_to_page_range() doesn't track the damage, assume the worst */
> > + if (ARCH_PAGE_TABLE_SYNC_MASK & (PGTBL_PTE_MODIFIED |
> > +  PGTBL_PMD_MODIFIED |
> > +  PGTBL_PUD_MODIFIED |
> > +  PGTBL_P4D_MODIFIED |
> > +  PGTBL_PGD_MODIFIED))
> > + arch_sync_kernel_mappings(addr, addr + area->size);
> 
> This should happen in __apply_to_page_range() directly and look like
> this:

Ok. I thought it had to be after assigning the *ptep. If we apply the
sync first, do not have to worry about PGTBL_PTE_MODIFIED from the
*ptep?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Joerg Roedel
On Fri, Aug 21, 2020 at 09:50:08AM +0100, Chris Wilson wrote:
> The alloc_vm_area() is another method for drivers to
> vmap/map_kernel_range that uses apply_to_page_range() rather than the
> direct vmalloc walkers. This is missing the page table modification
> tracking, and the ability to synchronize the PTE updates afterwards.
> Provide flush_vm_area() for the users of alloc_vm_area() that assumes
> the worst and ensures that the page directories are correctly flushed
> upon construction.
> 
> The impact is most pronounced on x86_32 due to the delayed set_pmd().
> 
> Reported-by: Pavel Machek 
> References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were 
> modified")
> References: 86cf69f1d893 ("x86/mm/32: implement arch_sync_kernel_mappings()")
> Signed-off-by: Chris Wilson 
> Cc: Andrew Morton 
> Cc: Joerg Roedel 
> Cc: Linus Torvalds 
> Cc: Dave Airlie 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Cc: Pavel Machek 
> Cc: David Vrabel 
> Cc:  # v5.8+
> ---
>  include/linux/vmalloc.h |  1 +
>  mm/vmalloc.c| 16 
>  2 files changed, 17 insertions(+)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 0221f852a7e1..a253b27df0ac 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -204,6 +204,7 @@ static inline void set_vm_flush_reset_perms(void *addr)
>  
>  /* Allocate/destroy a 'vmalloc' VM area. */
>  extern struct vm_struct *alloc_vm_area(size_t size, pte_t **ptes);
> +extern void flush_vm_area(struct vm_struct *area);
>  extern void free_vm_area(struct vm_struct *area);
>  
>  /* for /dev/kmem */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index b482d240f9a2..c41934486031 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3078,6 +3078,22 @@ struct vm_struct *alloc_vm_area(size_t size, pte_t 
> **ptes)
>  }
>  EXPORT_SYMBOL_GPL(alloc_vm_area);
>  
> +void flush_vm_area(struct vm_struct *area)
> +{
> + unsigned long addr = (unsigned long)area->addr;
> +
> + /* apply_to_page_range() doesn't track the damage, assume the worst */
> + if (ARCH_PAGE_TABLE_SYNC_MASK & (PGTBL_PTE_MODIFIED |
> +  PGTBL_PMD_MODIFIED |
> +  PGTBL_PUD_MODIFIED |
> +  PGTBL_P4D_MODIFIED |
> +  PGTBL_PGD_MODIFIED))
> + arch_sync_kernel_mappings(addr, addr + area->size);

This should happen in __apply_to_page_range() directly and look like
this:

if (ARCH_PAGE_TABLE_SYNC_MASK && create)
arch_sync_kernel_mappings(addr, addr + size);

Or even better, track whether something had to be allocated in the
__apply_to_page_range() path and check for:

if (ARCH_PAGE_TABLE_SYNC_MASK & mask)

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v6 04/11] drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3.

2020-08-21 Thread Manna, Animesh


On 16-07-2020 04:12, Manasi Navare wrote:

From: Maarten Lankhorst 

Small changes to intel_dp_mode_valid(), allow listing modes that
can only be supported in the bigjoiner configuration, which is
not supported yet.

eDP does not support bigjoiner, so do not expose bigjoiner only
modes on the eDP port.

v5:
* Increase max plane width to support 8K with bigjoiner (Maarten)
v4:
* Rebase (Manasi)

Changes since v1:
- Disallow bigjoiner on eDP.
Changes since v2:
- Rename intel_dp_downstream_max_dotclock to intel_dp_max_dotclock,
   and split off the downstream and source checking to its own function.
   (Ville)
v3:
* Rebase (Manasi)

Signed-off-by: Manasi Navare 
Signed-off-by: Maarten Lankhorst 
---
  drivers/gpu/drm/i915/display/intel_display.c |   2 +-
  drivers/gpu/drm/i915/display/intel_dp.c  | 119 ++-
  2 files changed, 91 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 78cbfefbfa62..3ecb642805a6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private 
*dev_priv,
 * too big for that.
 */
if (INTEL_GEN(dev_priv) >= 11) {
-   plane_width_max = 5120;
+   plane_width_max = 7680;



Other encoder also use this function and big joiner on DP only need this 
change. Is it good idea to add encoder check? Maybe in a cover-letter can we 
add some description about big-joiner
feature and current limitation.
Overall changes looks good to me, for dsc related code better get a review from 
someone who has worked before.

Regards,
Animesh
 


plane_height_max = 4320;
} else {
plane_width_max = 5120;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index d6295eb20b63..fbfea99fd804 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -248,25 +248,37 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
return max_link_clock * max_lanes;
  }
  
-static int

-intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
+static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
  {
-   struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-   struct intel_encoder *encoder = _port->base;
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct intel_encoder *encoder = _dig_port->base;
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-   int max_dotclk = dev_priv->max_dotclk_freq;
-   int ds_max_dotclk;
  
+	if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp))

+   return 2 * dev_priv->max_dotclk_freq;
+
+   return dev_priv->max_dotclk_freq;
+}
+
+static int downstream_max_dotclock(struct intel_dp *intel_dp)
+{
int type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
  
  	if (type != DP_DS_PORT_TYPE_VGA)

-   return max_dotclk;
+   return 0;
  
-	ds_max_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,

-   intel_dp->downstream_ports);
+   return drm_dp_downstream_max_clock(intel_dp->dpcd,
+  intel_dp->downstream_ports);
+}
+
+static int
+intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
+{
+   int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
+   int ds_max_dotclk = downstream_max_dotclock(intel_dp);
  
  	if (ds_max_dotclk != 0)

-   max_dotclk = min(max_dotclk, ds_max_dotclk);
+   return min(max_dotclk, ds_max_dotclk);
  
  	return max_dotclk;

  }
@@ -527,7 +539,8 @@ small_joiner_ram_size_bits(struct drm_i915_private *i915)
  
  static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,

   u32 link_clock, u32 lane_count,
-  u32 mode_clock, u32 mode_hdisplay)
+  u32 mode_clock, u32 mode_hdisplay,
+  bool bigjoiner)
  {
u32 bits_per_pixel, max_bpp_small_joiner_ram;
int i;
@@ -545,6 +558,10 @@ static u16 intel_dp_dsc_get_output_bpp(struct 
drm_i915_private *i915,
/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) /
mode_hdisplay;
+
+   if (bigjoiner)
+   max_bpp_small_joiner_ram *= 2;
+
drm_dbg_kms(>drm, "Max small joiner bpp: %u\n",
max_bpp_small_joiner_ram);
  
@@ -554,6 +571,15 @@ static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,

 */
bits_per_pixel = min(bits_per_pixel, 

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Patchwork
== Series Details ==

Series: series starting with [1/4] mm: Export flush_vm_area() to sync the PTEs 
upon construction
URL   : https://patchwork.freedesktop.org/series/80892/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8911 -> Patchwork_18386


Summary
---

  **SUCCESS**

  No regressions found.

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

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@i915_module_load@reload:
- fi-tgl-u2:  [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/fi-tgl-u2/igt@i915_module_l...@reload.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/fi-tgl-u2/igt@i915_module_l...@reload.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
- fi-bsw-kefka:   [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html

  * igt@i915_selftest@live@execlists:
- fi-icl-y:   [PASS][5] -> [INCOMPLETE][6] ([i915#2276])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/fi-icl-y/igt@i915_selftest@l...@execlists.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/fi-icl-y/igt@i915_selftest@l...@execlists.html

  * igt@kms_busy@basic@flip:
- fi-kbl-x1275:   [PASS][7] -> [DMESG-WARN][8] ([i915#62] / [i915#92] / 
[i915#95])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/fi-kbl-x1275/igt@kms_busy@ba...@flip.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/fi-kbl-x1275/igt@kms_busy@ba...@flip.html

  
 Possible fixes 

  * igt@i915_selftest@live@gt_lrc:
- fi-tgl-u2:  [DMESG-FAIL][9] ([i915#2373]) -> [PASS][10]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/fi-tgl-u2/igt@i915_selftest@live@gt_lrc.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/fi-tgl-u2/igt@i915_selftest@live@gt_lrc.html

  
 Warnings 

  * igt@kms_force_connector_basic@force-edid:
- fi-kbl-x1275:   [DMESG-WARN][11] ([i915#62] / [i915#92]) -> 
[DMESG-WARN][12] ([i915#62] / [i915#92] / [i915#95]) +1 similar issue
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/fi-kbl-x1275/igt@kms_force_connector_ba...@force-edid.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/fi-kbl-x1275/igt@kms_force_connector_ba...@force-edid.html

  * igt@kms_force_connector_basic@prune-stale-modes:
- fi-kbl-x1275:   [DMESG-WARN][13] ([i915#62] / [i915#92] / [i915#95]) 
-> [DMESG-WARN][14] ([i915#62] / [i915#92]) +1 similar issue
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8911/fi-kbl-x1275/igt@kms_force_connector_ba...@prune-stale-modes.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/fi-kbl-x1275/igt@kms_force_connector_ba...@prune-stale-modes.html

  
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2276]: https://gitlab.freedesktop.org/drm/intel/issues/2276
  [i915#2373]: https://gitlab.freedesktop.org/drm/intel/issues/2373
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (38 -> 34)
--

  Missing(4): fi-byt-clapper fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


Build changes
-

  * Linux: CI_DRM_8911 -> Patchwork_18386

  CI-20190529: 20190529
  CI_DRM_8911: a1029718e0c12c304c20384a838b02c95f6262d5 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5769: 4e5f76be680b65780204668e302026cf638decc9 @ 
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18386: 9e1d77a3e2faac6f5624fe505f2af1b80357ebf3 @ 
git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9e1d77a3e2fa drm/i915/gem: Replace reloc chain with terminator on error unwind
6dc443ae9dba drm/i915/gem: Use set_pte_at() for assigning the vmapped PTE
2410d6d4185d drm/i915/gem: Sync the vmap PTEs upon construction
dc5a3f725528 mm: Export flush_vm_area() to sync the PTEs upon construction

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18386/index.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t] i915/perf_pmu: Emit a semaphore to measure

2020-08-21 Thread Ramalingam C
On 2020-08-10 at 13:44:15 +0100, Chris Wilson wrote:
> Don't assume the kernel will emit a semaphore to synchronise between two
> engine, and emit the semaphore ourselves for the basis of our
> measurements. The purpose of the test is to try and ascertain the
> accuracy of the two sampling methods, semaphore busyness uses register
> polling, whereas the engine busyness may use ktime_t of the CS events.

Looks good to me.

Reviewed-by: Ramalingam C 

Tested on the platform too.
> 
> Signed-off-by: Chris Wilson 
> Cc: Ramalingam C 
> ---
>  tests/i915/perf_pmu.c | 94 +--
>  1 file changed, 64 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
> index 13e1bd93e..ecd4afbd6 100644
> --- a/tests/i915/perf_pmu.c
> +++ b/tests/i915/perf_pmu.c
> @@ -650,6 +650,7 @@ no_sema(int gem_fd, const struct intel_execution_engine2 
> *e, unsigned int flags)
>  #define MI_SEMAPHORE_WAITMI_INSTR(0x1c, 2) /* GEN8+ */
>  #define   MI_SEMAPHORE_POLL  (1<<15)
>  #define   MI_SEMAPHORE_SAD_GTE_SDD   (1<<12)
> +#define   MI_SEMAPHORE_SAD_NEQ_SDD  (5 << 12)
>  
>  static void
>  sema_wait(int gem_fd, const struct intel_execution_engine2 *e,
> @@ -751,10 +752,39 @@ sema_wait(int gem_fd, const struct 
> intel_execution_engine2 *e,
>   assert_within_epsilon(val[1] - val[0], slept, tolerance);
>  }
>  
> +static uint32_t
> +create_sema(int gem_fd, struct drm_i915_gem_relocation_entry *reloc)
> +{
> + uint32_t cs[] = {
> + /* Reset our semaphore wait */
> + MI_STORE_DWORD_IMM,
> + 0,
> + 0,
> + 1,
> +
> + /* Wait until the semaphore value is set to 0 [by caller] */
> + MI_SEMAPHORE_WAIT | MI_SEMAPHORE_POLL | 
> MI_SEMAPHORE_SAD_NEQ_SDD,
> + 1,
> + 0,
> + 0,
> +
> + MI_BATCH_BUFFER_END
> + };
> + uint32_t handle = gem_create(gem_fd, 4096);
> +
> + memset(reloc, 0, 2 * sizeof(*reloc));
> + reloc[0].target_handle = handle;
> + reloc[0].offset = 64 + 1 * sizeof(uint32_t);
> + reloc[1].target_handle = handle;
> + reloc[1].offset = 64 + 6 * sizeof(uint32_t);
> +
> + gem_write(gem_fd, handle, 64, cs, sizeof(cs));
> + return handle;
> +}
> +
>  static void
>  __sema_busy(int gem_fd, int pmu,
>   const struct intel_execution_engine2 *e,
> - const struct intel_execution_engine2 *signal,
>   int sema_pct,
>   int busy_pct)
>  {
> @@ -764,39 +794,54 @@ __sema_busy(int gem_fd, int pmu,
>   };
>   uint64_t total, sema, busy;
>   uint64_t start[2], val[2];
> - igt_spin_t *spin[2];
> + struct drm_i915_gem_relocation_entry reloc[2];
> + struct drm_i915_gem_exec_object2 obj = {
> + .handle = create_sema(gem_fd, reloc),
> + .relocation_count = 2,
> + .relocs_ptr = to_user_pointer(reloc),
> + };
> + struct drm_i915_gem_execbuffer2 eb = {
> + .batch_start_offset = 64,
> + .buffer_count = 1,
> + .buffers_ptr = to_user_pointer(),
> + .flags = e->flags,
> + };
> + igt_spin_t *spin;
> + uint32_t *map;
>  
>   /* Time spent being busy includes time waiting on semaphores */
>   igt_assert(busy_pct >= sema_pct);
>  
>   gem_quiescent_gpu(gem_fd);
>  
> - spin[0] = igt_spin_new(gem_fd,
> -.engine = signal->flags,
> -.flags = IGT_SPIN_FENCE_OUT | IGT_SPIN_POLL_RUN);
> - spin[1] = igt_spin_new(gem_fd,
> -.engine = e->flags,
> -.fence = spin[0]->out_fence,
> -.flags = IGT_SPIN_FENCE_IN);
> + map = gem_mmap__wc(gem_fd, obj.handle, 0, 4096, PROT_WRITE);
> + gem_execbuf(gem_fd, );
> + spin = igt_spin_new(gem_fd, .engine = e->flags);
>  
> - igt_spin_busywait_until_started(spin[0]);
> + /* Wait until the batch is executed and the semaphore is busy-waiting */
> + while (!READ_ONCE(*map) && gem_bo_busy(gem_fd, obj.handle))
> + ;
> + igt_assert(gem_bo_busy(gem_fd, obj.handle));
> + gem_close(gem_fd, obj.handle);
>  
>   total = pmu_read_multi(pmu, 2, start);
>  
>   sema = measured_usleep(batch_duration_ns * sema_pct / 100 / 1000);
> - igt_spin_end(spin[0]);
> + *map = 0; __sync_synchronize();
>   busy = measured_usleep(batch_duration_ns * (busy_pct - sema_pct) / 100 
> / 1000);
> - igt_spin_end(spin[1]);
> + igt_spin_end(spin);
>   measured_usleep(batch_duration_ns * (100 - busy_pct) / 100 / 1000);
>  
>   total = pmu_read_multi(pmu, 2, val) - total;
> + igt_spin_free(gem_fd, spin);
> + munmap(map, 4096);
>  
>   busy += sema;
>   val[SEMA] -= start[SEMA];
>   val[BUSY] -= start[BUSY];
>  
> - igt_info("%s<-%s, target: {%.1f%% [%d], %.1f%% [%d]}, measured: 
> {%.1f%%, %.1f%%}\n",
> -  

Re: [Intel-gfx] 5.9-rc1: graphics regression moved from -next to mainline

2020-08-21 Thread Pavel Machek
On Thu 2020-08-20 09:16:18, Linus Torvalds wrote:
> On Thu, Aug 20, 2020 at 2:23 AM Pavel Machek  wrote:
> >
> > Yes, it seems they make things work. (Chris asked for new patch to be
> > tested, so I am switching to his kernel, but it survived longer than
> > it usually does.)
> 
> Ok, so at worst we know how to solve it, at best the reverts won't be
> needed because Chris' patch will fix the issue properly.
> 
> So I'll archive this thread, but remind me if this hasn't gotten
> sorted out in the later rc's.

Yes, thank you, it seems we have a solution w/o the revert.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Patchwork
== Series Details ==

Series: series starting with [1/4] mm: Export flush_vm_area() to sync the PTEs 
upon construction
URL   : https://patchwork.freedesktop.org/series/80892/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1019:47:expected unsigned int 
[addressable] [usertype] ulClockParams
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1019:47:got restricted __le32 
[usertype]
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1019:47: warning: incorrect type 
in assignment (different base types)
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1028:50: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1029:49: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1037:47: warning: too many 
warnings
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:184:44: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:283:14: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:320:14: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:323:14: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:326:14: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:329:18: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:330:26: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:338:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:340:38: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:342:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:346:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:348:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:353:33: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:367:43: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:369:38: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:374:67: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:375:53: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:378:66: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:389:80: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:395:57: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:402:69: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:403:53: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:406:66: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:414:66: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:423:69: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:424:69: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:473:30: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:476:45: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:477:45: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:484:54: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:52:28: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:531:35: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:53:29: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:533:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:54:26: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:55:27: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:56:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:57:26: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:577:21: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:581:25: warning: cast to 
restricted __le32
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:58:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:583:21: warning: cast to 
restricted __le32
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:586:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:590:25: warning: cast to 
restricted __le16
+drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:59:26: warning: cast to 
restricted __le16

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Patchwork
== Series Details ==

Series: series starting with [1/4] mm: Export flush_vm_area() to sync the PTEs 
upon construction
URL   : https://patchwork.freedesktop.org/series/80892/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
dc5a3f725528 mm: Export flush_vm_area() to sync the PTEs upon construction
-:17: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
(prefer a maximum 75 chars per line)
#17: 
References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were 
modified")

-:17: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ 
chars of sha1> ("")' - ie: 'commit 2ba3e6947aed ("mm/vmalloc: track 
which page-table levels were modified")'
#17: 
References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were 
modified")

-:18: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ 
chars of sha1> ("")' - ie: 'commit 86cf69f1d893 ("x86/mm/32: 
implement arch_sync_kernel_mappings()")'
#18: 
References: 86cf69f1d893 ("x86/mm/32: implement arch_sync_kernel_mappings()")

-:38: CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#38: FILE: include/linux/vmalloc.h:207:
+extern void flush_vm_area(struct vm_struct *area);

total: 2 errors, 1 warnings, 1 checks, 29 lines checked
2410d6d4185d drm/i915/gem: Sync the vmap PTEs upon construction
-:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
(prefer a maximum 75 chars per line)
#11: 
References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were 
modified")

-:11: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ 
chars of sha1> ("")' - ie: 'commit 2ba3e6947aed ("mm/vmalloc: track 
which page-table levels were modified")'
#11: 
References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were 
modified")

total: 1 errors, 1 warnings, 0 checks, 7 lines checked
6dc443ae9dba drm/i915/gem: Use set_pte_at() for assigning the vmapped PTE
9e1d77a3e2fa drm/i915/gem: Replace reloc chain with terminator on error unwind


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/4] drm/i915/gem: Sync the vmap PTEs upon construction

2020-08-21 Thread Chris Wilson
Since synchronising the PTE after assignment is a manual step, use the
newly exported interface to flush the PTE after assigning via
alloc_vm_area().

Reported-by: Pavel Machek 
References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were 
modified")
Signed-off-by: Chris Wilson 
Cc: Andrew Morton 
Cc: Joerg Roedel 
Cc: Linus Torvalds 
Cc: Dave Airlie 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: Pavel Machek 
Cc:  # v5.8+
---
 drivers/gpu/drm/i915/gem/i915_gem_pages.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c 
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 7050519c87a4..0fee67f34d74 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -304,6 +304,7 @@ static void *i915_gem_object_map(struct drm_i915_gem_object 
*obj,
for_each_sgt_daddr(addr, iter, sgt)
**ptes++ = iomap_pte(iomap, addr, pgprot);
}
+   flush_vm_area(area);
 
if (mem != stack)
kvfree(mem);
-- 
2.20.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 3/4] drm/i915/gem: Use set_pte_at() for assigning the vmapped PTE

2020-08-21 Thread Chris Wilson
Use set_pte_at() to assign the PTE pointer returned by alloc_vm_area(),
rather than a direct assignment.

Fixes: 6056e50033d9 ("drm/i915/gem: Support discontiguous lmem object maps")
Signed-off-by: Chris Wilson 
Cc: Matthew Auld 
---
 drivers/gpu/drm/i915/gem/i915_gem_pages.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c 
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 0fee67f34d74..6838cf9bdae6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -286,23 +286,34 @@ static void *i915_gem_object_map(struct 
drm_i915_gem_object *obj,
}
 
if (i915_gem_object_has_struct_page(obj)) {
+   unsigned long addr = (unsigned long)area->addr;
struct sgt_iter iter;
struct page *page;
pte_t **ptes = mem;
 
-   for_each_sgt_page(page, iter, sgt)
-   **ptes++ = mk_pte(page, pgprot);
+   for_each_sgt_page(page, iter, sgt) {
+   set_pte_at(_mm, addr, *ptes, mk_pte(page, pgprot));
+   addr += PAGE_SIZE;
+   ptes++;
+   }
+   GEM_BUG_ON(addr != (unsigned long)area->addr + obj->base.size);
} else {
+   unsigned long addr = (unsigned long)area->addr;
resource_size_t iomap;
struct sgt_iter iter;
pte_t **ptes = mem;
-   dma_addr_t addr;
+   dma_addr_t offset;
 
iomap = obj->mm.region->iomap.base;
iomap -= obj->mm.region->region.start;
 
-   for_each_sgt_daddr(addr, iter, sgt)
-   **ptes++ = iomap_pte(iomap, addr, pgprot);
+   for_each_sgt_daddr(offset, iter, sgt) {
+   set_pte_at(_mm, addr, *ptes,
+  iomap_pte(iomap, offset, pgprot));
+   addr += PAGE_SIZE;
+   ptes++;
+   }
+   GEM_BUG_ON(addr != (unsigned long)area->addr + obj->base.size);
}
flush_vm_area(area);
 
-- 
2.20.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 4/4] drm/i915/gem: Replace reloc chain with terminator on error unwind

2020-08-21 Thread Chris Wilson
If we hit an error during construction of the reloc chain, we need to
replace the chain into the next batch with the terminator so that upon
flushing the relocations so far, we do not execute a hanging batch.

Reported-by: Pavel Machek 
Fixes: 964a9b0f611e ("drm/i915/gem: Use chained reloc batches")
Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
Cc: Pavel Machek 
Cc:  # v5.8+
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 31 ++-
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 24a1486d2dc5..a09f04eee417 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -972,21 +972,6 @@ static int reloc_gpu_chain(struct reloc_cache *cache)
if (err)
goto out_pool;
 
-   GEM_BUG_ON(cache->rq_size + RELOC_TAIL > PAGE_SIZE  / sizeof(u32));
-   cmd = cache->rq_cmd + cache->rq_size;
-   *cmd++ = MI_ARB_CHECK;
-   if (cache->gen >= 8)
-   *cmd++ = MI_BATCH_BUFFER_START_GEN8;
-   else if (cache->gen >= 6)
-   *cmd++ = MI_BATCH_BUFFER_START;
-   else
-   *cmd++ = MI_BATCH_BUFFER_START | MI_BATCH_GTT;
-   *cmd++ = lower_32_bits(batch->node.start);
-   *cmd++ = upper_32_bits(batch->node.start); /* Always 0 for gen<8 */
-   i915_gem_object_flush_map(cache->rq_vma->obj);
-   i915_gem_object_unpin_map(cache->rq_vma->obj);
-   cache->rq_vma = NULL;
-
err = intel_gt_buffer_pool_mark_active(pool, rq);
if (err == 0) {
i915_vma_lock(batch);
@@ -999,15 +984,31 @@ static int reloc_gpu_chain(struct reloc_cache *cache)
if (err)
goto out_pool;
 
+   GEM_BUG_ON(cache->rq_size + RELOC_TAIL > PAGE_SIZE  / sizeof(u32));
+   cmd = cache->rq_cmd + cache->rq_size;
+   *cmd++ = MI_ARB_CHECK;
+   if (cache->gen >= 8)
+   *cmd++ = MI_BATCH_BUFFER_START_GEN8;
+   else if (cache->gen >= 6)
+   *cmd++ = MI_BATCH_BUFFER_START;
+   else
+   *cmd++ = MI_BATCH_BUFFER_START | MI_BATCH_GTT;
+   *cmd++ = lower_32_bits(batch->node.start);
+   *cmd++ = upper_32_bits(batch->node.start); /* Always 0 for gen<8 */
+
cmd = i915_gem_object_pin_map(batch->obj,
  cache->has_llc ?
  I915_MAP_FORCE_WB :
  I915_MAP_FORCE_WC);
if (IS_ERR(cmd)) {
+   /* We will replace the BBS with BBE upon flushing the rq */
err = PTR_ERR(cmd);
goto out_pool;
}
 
+   i915_gem_object_flush_map(cache->rq_vma->obj);
+   i915_gem_object_unpin_map(cache->rq_vma->obj);
+
/* Return with batch mapping (cmd) still pinned */
cache->rq_cmd = cmd;
cache->rq_size = 0;
-- 
2.20.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/4] mm: Export flush_vm_area() to sync the PTEs upon construction

2020-08-21 Thread Chris Wilson
The alloc_vm_area() is another method for drivers to
vmap/map_kernel_range that uses apply_to_page_range() rather than the
direct vmalloc walkers. This is missing the page table modification
tracking, and the ability to synchronize the PTE updates afterwards.
Provide flush_vm_area() for the users of alloc_vm_area() that assumes
the worst and ensures that the page directories are correctly flushed
upon construction.

The impact is most pronounced on x86_32 due to the delayed set_pmd().

Reported-by: Pavel Machek 
References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were 
modified")
References: 86cf69f1d893 ("x86/mm/32: implement arch_sync_kernel_mappings()")
Signed-off-by: Chris Wilson 
Cc: Andrew Morton 
Cc: Joerg Roedel 
Cc: Linus Torvalds 
Cc: Dave Airlie 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: Pavel Machek 
Cc: David Vrabel 
Cc:  # v5.8+
---
 include/linux/vmalloc.h |  1 +
 mm/vmalloc.c| 16 
 2 files changed, 17 insertions(+)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 0221f852a7e1..a253b27df0ac 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -204,6 +204,7 @@ static inline void set_vm_flush_reset_perms(void *addr)
 
 /* Allocate/destroy a 'vmalloc' VM area. */
 extern struct vm_struct *alloc_vm_area(size_t size, pte_t **ptes);
+extern void flush_vm_area(struct vm_struct *area);
 extern void free_vm_area(struct vm_struct *area);
 
 /* for /dev/kmem */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index b482d240f9a2..c41934486031 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3078,6 +3078,22 @@ struct vm_struct *alloc_vm_area(size_t size, pte_t 
**ptes)
 }
 EXPORT_SYMBOL_GPL(alloc_vm_area);
 
+void flush_vm_area(struct vm_struct *area)
+{
+   unsigned long addr = (unsigned long)area->addr;
+
+   /* apply_to_page_range() doesn't track the damage, assume the worst */
+   if (ARCH_PAGE_TABLE_SYNC_MASK & (PGTBL_PTE_MODIFIED |
+PGTBL_PMD_MODIFIED |
+PGTBL_PUD_MODIFIED |
+PGTBL_P4D_MODIFIED |
+PGTBL_PGD_MODIFIED))
+   arch_sync_kernel_mappings(addr, addr + area->size);
+
+   flush_cache_vmap(addr, area->size);
+}
+EXPORT_SYMBOL_GPL(flush_vm_area);
+
 void free_vm_area(struct vm_struct *area)
 {
struct vm_struct *ret;
-- 
2.20.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/4] i915/perf: 32bit printf cleanup

2020-08-21 Thread Lionel Landwerlin

On 21/08/2020 10:54, Petri Latvala wrote:

On Fri, Aug 21, 2020 at 12:45:23AM +0300, Lionel Landwerlin wrote:

On 20/08/2020 20:26, Chris Wilson wrote:

Use PRI[du]64 as necessary for 32bit builds.

Signed-off-by: Chris Wilson 

Reviewed-by: Lionel Landwerlin 


Was this for just 1/4 or the whole series?



Just for this patch.


-Lionel



This one is for the whole series:
Reviewed-by: Petri Latvala 




Thanks!

-Lionel


---
   tests/i915/perf.c| 8 
   tools/i915-perf/i915_perf_recorder.c | 2 +-
   2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/i915/perf.c b/tests/i915/perf.c
index 92edc9f1f..a894fd382 100644
--- a/tests/i915/perf.c
+++ b/tests/i915/perf.c
@@ -2077,7 +2077,7 @@ test_blocking(uint64_t requested_oa_period, bool 
set_kernel_hrtimer, uint64_t ke
user_ns = (end_times.tms_utime - start_times.tms_utime) * tick_ns;
kernel_ns = (end_times.tms_stime - start_times.tms_stime) * tick_ns;
-   igt_debug("%d blocking reads during test with %lu Hz OA sampling (expect no 
more than %d)\n",
+   igt_debug("%d blocking reads during test with %"PRIu64" Hz OA sampling 
(expect no more than %d)\n",
  n, NSEC_PER_SEC / oa_period, max_iterations);
igt_debug("%d extra iterations seen, not related to periodic sampling (e.g. 
context switches)\n",
  n_extra_iterations);
@@ -2265,7 +2265,7 @@ test_polling(uint64_t requested_oa_period, bool 
set_kernel_hrtimer, uint64_t ker
user_ns = (end_times.tms_utime - start_times.tms_utime) * tick_ns;
kernel_ns = (end_times.tms_stime - start_times.tms_stime) * tick_ns;
-   igt_debug("%d non-blocking reads during test with %lu Hz OA sampling (expect 
no more than %d)\n",
+   igt_debug("%d non-blocking reads during test with %"PRIu64" Hz OA sampling 
(expect no more than %d)\n",
  n, NSEC_PER_SEC / oa_period, max_iterations);
igt_debug("%d extra iterations seen, not related to periodic sampling (e.g. 
context switches)\n",
  n_extra_iterations);
@@ -2357,7 +2357,7 @@ num_valid_reports_captured(struct 
drm_i915_perf_open_param *param,
int64_t start, end;
int num_reports = 0;
-   igt_debug("Expected duration = %lu\n", *duration_ns);
+   igt_debug("Expected duration = %"PRId64"\n", *duration_ns);
stream_fd = __perf_open(drm_fd, param, true);
@@ -2389,7 +2389,7 @@ num_valid_reports_captured(struct 
drm_i915_perf_open_param *param,
*duration_ns = end - start;
-   igt_debug("Actual duration = %lu\n", *duration_ns);
+   igt_debug("Actual duration = %"PRIu64"\n", *duration_ns);
return num_reports;
   }
diff --git a/tools/i915-perf/i915_perf_recorder.c 
b/tools/i915-perf/i915_perf_recorder.c
index 7671f39b4..adc41c29f 100644
--- a/tools/i915-perf/i915_perf_recorder.c
+++ b/tools/i915-perf/i915_perf_recorder.c
@@ -1001,7 +1001,7 @@ main(int argc, char *argv[])
}
ctx.oa_exponent = oa_exponent_for_period(ctx.timestamp_frequency, 
perf_period);
-   fprintf(stdout, "Opening perf stream with metric_id=%lu 
oa_exponent=%u\n",
+   fprintf(stdout, "Opening perf stream with metric_id=%"PRIu64" 
oa_exponent=%u\n",
ctx.metric_set->perf_oa_metrics_set, ctx.oa_exponent);
ctx.perf_fd = perf_open();


___
igt-dev mailing list
igt-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/4] i915/perf: 32bit printf cleanup

2020-08-21 Thread Petri Latvala
On Fri, Aug 21, 2020 at 12:45:23AM +0300, Lionel Landwerlin wrote:
> On 20/08/2020 20:26, Chris Wilson wrote:
> > Use PRI[du]64 as necessary for 32bit builds.
> > 
> > Signed-off-by: Chris Wilson 
> 
> Reviewed-by: Lionel Landwerlin 
> 

Was this for just 1/4 or the whole series?

This one is for the whole series:
Reviewed-by: Petri Latvala 



> 
> Thanks!
> 
> -Lionel
> 
> > ---
> >   tests/i915/perf.c| 8 
> >   tools/i915-perf/i915_perf_recorder.c | 2 +-
> >   2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/i915/perf.c b/tests/i915/perf.c
> > index 92edc9f1f..a894fd382 100644
> > --- a/tests/i915/perf.c
> > +++ b/tests/i915/perf.c
> > @@ -2077,7 +2077,7 @@ test_blocking(uint64_t requested_oa_period, bool 
> > set_kernel_hrtimer, uint64_t ke
> > user_ns = (end_times.tms_utime - start_times.tms_utime) * tick_ns;
> > kernel_ns = (end_times.tms_stime - start_times.tms_stime) * tick_ns;
> > -   igt_debug("%d blocking reads during test with %lu Hz OA sampling 
> > (expect no more than %d)\n",
> > +   igt_debug("%d blocking reads during test with %"PRIu64" Hz OA sampling 
> > (expect no more than %d)\n",
> >   n, NSEC_PER_SEC / oa_period, max_iterations);
> > igt_debug("%d extra iterations seen, not related to periodic sampling 
> > (e.g. context switches)\n",
> >   n_extra_iterations);
> > @@ -2265,7 +2265,7 @@ test_polling(uint64_t requested_oa_period, bool 
> > set_kernel_hrtimer, uint64_t ker
> > user_ns = (end_times.tms_utime - start_times.tms_utime) * tick_ns;
> > kernel_ns = (end_times.tms_stime - start_times.tms_stime) * tick_ns;
> > -   igt_debug("%d non-blocking reads during test with %lu Hz OA sampling 
> > (expect no more than %d)\n",
> > +   igt_debug("%d non-blocking reads during test with %"PRIu64" Hz OA 
> > sampling (expect no more than %d)\n",
> >   n, NSEC_PER_SEC / oa_period, max_iterations);
> > igt_debug("%d extra iterations seen, not related to periodic sampling 
> > (e.g. context switches)\n",
> >   n_extra_iterations);
> > @@ -2357,7 +2357,7 @@ num_valid_reports_captured(struct 
> > drm_i915_perf_open_param *param,
> > int64_t start, end;
> > int num_reports = 0;
> > -   igt_debug("Expected duration = %lu\n", *duration_ns);
> > +   igt_debug("Expected duration = %"PRId64"\n", *duration_ns);
> > stream_fd = __perf_open(drm_fd, param, true);
> > @@ -2389,7 +2389,7 @@ num_valid_reports_captured(struct 
> > drm_i915_perf_open_param *param,
> > *duration_ns = end - start;
> > -   igt_debug("Actual duration = %lu\n", *duration_ns);
> > +   igt_debug("Actual duration = %"PRIu64"\n", *duration_ns);
> > return num_reports;
> >   }
> > diff --git a/tools/i915-perf/i915_perf_recorder.c 
> > b/tools/i915-perf/i915_perf_recorder.c
> > index 7671f39b4..adc41c29f 100644
> > --- a/tools/i915-perf/i915_perf_recorder.c
> > +++ b/tools/i915-perf/i915_perf_recorder.c
> > @@ -1001,7 +1001,7 @@ main(int argc, char *argv[])
> > }
> > ctx.oa_exponent = oa_exponent_for_period(ctx.timestamp_frequency, 
> > perf_period);
> > -   fprintf(stdout, "Opening perf stream with metric_id=%lu 
> > oa_exponent=%u\n",
> > +   fprintf(stdout, "Opening perf stream with metric_id=%"PRIu64" 
> > oa_exponent=%u\n",
> > ctx.metric_set->perf_oa_metrics_set, ctx.oa_exponent);
> > ctx.perf_fd = perf_open();
> 
> 
> ___
> igt-dev mailing list
> igt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/virtio: Remove open-coded commit-tail function

2020-08-21 Thread Gerd Hoffmann
On Thu, Aug 20, 2020 at 08:32:51AM +0200, Jiri Slaby wrote:
> On 19. 08. 20, 15:24, Gerd Hoffmann wrote:
> > On Wed, Aug 19, 2020 at 02:43:28PM +0200, Jiri Slaby wrote:
> >> On 09. 07. 20, 14:33, Daniel Vetter wrote:
> >>> Exactly matches the one in the helpers.
> >>
> >> It's not that exact. The order of modeset_enables and planes is
> >> different. And this causes a regression -- no fb in qemu.
> > 
> > Does https://patchwork.freedesktop.org/patch/385980/ help?
> 
> Yes, it does.

Any chance you can send a tested-by & acked-by for the series so I can
get it merged?

thanks,
  Gerd

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx