[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/hwmon: Fix static analysis tool errors in i915 hwmon (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/hwmon: Fix static analysis tool errors in i915 hwmon (rev2)
URL   : https://patchwork.freedesktop.org/series/127034/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13957 -> Patchwork_127034v2


Summary
---

  **SUCCESS**

  No regressions found.

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

Participating hosts (37 -> 36)
--

  Missing(1): fi-snb-2520m 

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_lmem_swapping@basic:
- fi-kbl-7567u:   NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#4613]) +3 
other tests skip
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/fi-kbl-7567u/igt@gem_lmem_swapp...@basic.html

  * igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [PASS][2] -> [DMESG-FAIL][3] ([i915#5334])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-5:
- bat-adlp-11:[PASS][4] -> [DMESG-WARN][5] ([i915#6868])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/bat-adlp-11/igt@kms_pipe_crc_basic@nonblocking-...@pipe-d-dp-5.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/bat-adlp-11/igt@kms_pipe_crc_basic@nonblocking-...@pipe-d-dp-5.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
- bat-rplp-1: [PASS][6] -> [ABORT][7] ([i915#8668])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html

  * igt@kms_pipe_crc_basic@read-crc@pipe-a-dp-5:
- bat-adlp-11:[PASS][8] -> [ABORT][9] ([i915#8668])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/bat-adlp-11/igt@kms_pipe_crc_basic@read-...@pipe-a-dp-5.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/bat-adlp-11/igt@kms_pipe_crc_basic@read-...@pipe-a-dp-5.html

  * igt@kms_setmode@basic-clone-single-crtc:
- fi-kbl-7567u:   NOTRUN -> [SKIP][10] ([fdo#109271])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/fi-kbl-7567u/igt@kms_setm...@basic-clone-single-crtc.html

  
 Possible fixes 

  * igt@i915_selftest@live@gem_contexts:
- bat-mtlp-8: [DMESG-FAIL][11] -> [PASS][12]
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/bat-mtlp-8/igt@i915_selftest@live@gem_contexts.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/bat-mtlp-8/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@mman:
- bat-rpls-1: [TIMEOUT][13] ([i915#6794] / [i915#7392]) -> 
[PASS][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/bat-rpls-1/igt@i915_selftest@l...@mman.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/bat-rpls-1/igt@i915_selftest@l...@mman.html

  * igt@i915_suspend@basic-s2idle-without-i915:
- bat-rpls-1: [WARN][15] ([i915#8747]) -> [PASS][16]
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/bat-rpls-1/igt@i915_susp...@basic-s2idle-without-i915.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/bat-rpls-1/igt@i915_susp...@basic-s2idle-without-i915.html

  * igt@kms_flip@basic-flip-vs-modeset@b-dp6:
- bat-adlp-11:[FAIL][17] ([i915#6121]) -> [PASS][18] +4 other tests 
pass
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/bat-adlp-11/igt@kms_flip@basic-flip-vs-mode...@b-dp6.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-mode...@b-dp6.html

  * igt@kms_flip@basic-flip-vs-modeset@c-dp5:
- bat-adlp-11:[DMESG-WARN][19] ([i915#6868]) -> [PASS][20]
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/bat-adlp-11/igt@kms_flip@basic-flip-vs-mode...@c-dp5.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-mode...@c-dp5.html

  * igt@kms_hdmi_inject@inject-audio:
- fi-kbl-guc: [FAIL][21] ([IGT#3]) -> [PASS][22]
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13957/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127034v2/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html

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

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[Alistair Popple]

"Zeng, Oak"  writes:

> See inline comments
>
>> -Original Message-
>> From: dri-devel  On Behalf Of
>> zhuweixi
>> Sent: Thursday, November 30, 2023 5:48 AM
>> To: Christian König ; Zeng, Oak
>> ; Christian König ; linux-
>> m...@kvack.org; linux-ker...@vger.kernel.org; a...@linux-foundation.org;
>> Danilo Krummrich ; Dave Airlie ; Daniel
>> Vetter 
>> Cc: tvrtko.ursu...@linux.intel.com; rcampb...@nvidia.com; apop...@nvidia.com;
>> z...@nvidia.com; weixi@openeuler.sh; jhubb...@nvidia.com; intel-
>> g...@lists.freedesktop.org; mhairgr...@nvidia.com; Wang, Zhi A
>> ; xinhui@amd.com; amd-...@lists.freedesktop.org;
>> jgli...@redhat.com; dri-de...@lists.freedesktop.org; j...@nvidia.com; Vivi,
>> Rodrigo ; alexander.deuc...@amd.com;
>> felix.kuehl...@amd.com; intel-gvt-...@lists.freedesktop.org;
>> ogab...@kernel.org; leo...@nvidia.com; mgor...@suse.de
>> Subject: RE: [RFC PATCH 0/6] Supporting GMEM (generalized memory
>> management) for external memory devices
>> 
>> Glad to know that there is a common demand for a new syscall like 
>> hmadvise(). I
>> expect it would also be useful for homogeneous NUMA cases. Credits to
>> cudaMemAdvise() API which brought this idea to GMEM's design.
>> 
>> To answer @Oak's questions about GMEM vs. HMM,
>> 
>> Here is the major difference:
>>   GMEM's main target is to stop drivers from reinventing MM code, while
>> HMM/MMU notifiers provide a compatible struct page solution and a
>> coordination mechanism for existing device driver MMs that requires adding
>> extra code to interact with CPU MM.
>> 
>> A straightforward qualitative result for the main target: after integrating 
>> Huawei's
>> Ascend NPU driver with GMEM's interface, 30,000 lines of MM code were cut,
>> leaving <100 lines invoking GMEM interface and 3,700 lines implementing 
>> vendor-
>> specific functions. Some code from the 3,700 lines should be further moved to
>> GMEM as a generalized feature like device memory oversubscription, but not
>> included in this RFC patch yet.
>> 
>> A list of high-level differences:
>>   1. With HMM/MMU notifiers, drivers need to first implement a full MM
>> subsystem. With GMEM, drivers can reuse Linux's core MM.
>
> A full mm subsystem essentially has below functions:
>
> Physical memory management: neither your approach nor hmm-based
> solution provide device physical memory management. You mentioned you
> have a plan but at least for now driver need to mange device physical
> memory.
>
> Virtual address space management: both approach leverage linux core mm, vma 
> for this.
>
> Data eviction, migration: with hmm, driver need to implement this. It
> is not clear whether gmem has this function. I guess even gmem has it,
> it might be slow cpu data copy, compared to modern gpu's fast data
> copy engine.
>
> Device page table update, va-pa mapping: I think it is driver's 
> responsibility in both approach.
>
> So from the point of re-use core MM, I don't see big difference. Maybe
> you did it more elegantly. I think it is very possible with your
> approach driver can be simpler, less codes.
>
>> 
>>   2. HMM encodes device mapping information in the CPU arch-dependent PTEs,
>> while GMEM proposes an abstraction layer in vm_object. Since GMEM's
>> approach further decouples the arch-related stuff, drivers do not need to
>> implement separate code for X86/ARM and etc.
>
> I don't understand this...with hmm, when a virtual address range's
> backing store is in device memory, cpu pte is encoded to point to
> device memory. Device page table is also encoded to point to the same
> device memory location. But since device memory is not accessible to
> CPU (DEVICE_PRIVATE), so when cpu access this virtual address, there
> is a cpu page fault. Device mapping info is still in device page
> table, not in cpu ptes.
>
> I do not see with hmm why driver need to implement x86/arm
> code... driver only take cares of device page table. Hmm/core mm take
> care of cpu page table, right?

I see our replies have crossed, but that is my understanding as well.

>> 
>>   3. MMU notifiers register hooks at certain core MM events, while GMEM
>> declares basic functions and internally invokes them. GMEM requires less from
>> the driver side -- no need to understand what core MM behaves at certain MMU
>> events. GMEM also expects fewer bugs than MMU notifiers: implementing basic
>> operations with standard declarations vs. implementing whatever random device
>> MM logic in MMU notifiers.
>
> This seems true to me. I feel the mmu notifier thing, especially the
> synchronization/lock design (those sequence numbers, interacting with
> driver lock, and the mmap lock) are very complicated. I indeed spent
> time to understand the specification documented in hmm.rst...

No argument there, but I think that's something we could look at
providing an improved interface for. I don't think it needs a whole new
subsystem to fix. Probably just a version of hmm_range_fault() that
takes the lock and s

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[Alistair Popple]

zhuweixi  writes:

> Glad to know that there is a common demand for a new syscall like
> hmadvise(). I expect it would also be useful for homogeneous NUMA
> cases. Credits to cudaMemAdvise() API which brought this idea to
> GMEM's design.

It's not clear to me that this would need to be a new syscall. Scanning
the patches it looks like your adding a NUMA node anyway, so the
existing interfaces (eg. madvise) with its various options
(MPOL_PREFERRED/PREFERRED_MANY) and
set_mempolicy/set_mempolicy_home_node() could potentially cover this for
both NUMA and hNUMA nodes. The main advantage here would be providing a
common userspace interface for setting these kind of hints.

> To answer @Oak's questions about GMEM vs. HMM,
>
> Here is the major difference:
>   GMEM's main target is to stop drivers from reinventing MM code,
> while HMM/MMU notifiers provide a compatible struct page solution and
> a coordination mechanism for existing device driver MMs that requires
> adding extra code to interact with CPU MM.
>
> A straightforward qualitative result for the main target: after
> integrating Huawei's Ascend NPU driver with GMEM's interface, 30,000
> lines of MM code were cut, leaving <100 lines invoking GMEM interface
> and 3,700 lines implementing vendor-specific functions. Some code from
> the 3,700 lines should be further moved to GMEM as a generalized
> feature like device memory oversubscription, but not included in this
> RFC patch yet.

I think it would be helpful if you could be a bit more specific on what
functionality the current HMM/migrate_vma/MMU notifier interfaces are
missing that every driver has to implement in a common way. Because I'm
not convinced we can't either improve those interfaces to provide what's
needed or add specific components (eg. a physical page allocator)
instead of a whole new framework.

> A list of high-level differences: 
>   1. With HMM/MMU notifiers, drivers need to first implement a full MM 
> subsystem. With GMEM, drivers can reuse Linux's core MM.

What do the common bits of this full MM subsystem look like?
Fundamentally the existing HMM functionality can already make use of
Linux core MM to manage page tables and migrate pages and everything
else seems pretty device specific (ie. actual copying of data,
programming of MMUs, TLBs, etc.)

I can see that there would be scope to have say a generic memory
allocator, which I vaguely recall discussing in relation to
DEIVCE_PRIVATE pages in the past but @Oak suggests something close
already exists (drm/buddy).

Potentially I suppose there is VA allocation that might be common across
devices. However I have not had any experience working with devices with
VA requirements different enough from the CPU to matter. If they are so
different I'm not convinced it would be easy to have a common
implementation anyway.

>   2. HMM encodes device mapping information in the CPU arch-dependent
> PTEs, while GMEM proposes an abstraction layer in vm_object. Since
> GMEM's approach further decouples the arch-related stuff, drivers do
> not need to implement separate code for X86/ARM and etc.

I'm not following this. At present all HMM encodes in CPU PTEs is the
fact a page has been migrated to the device and what permissions it
has. I'm not aware of needing to treat X86 and ARM differently for
example here. Are you saying you want somewhere to store other bits
attached to a particular VA?

>   3. MMU notifiers register hooks at certain core MM events, while
> GMEM declares basic functions and internally invokes them. GMEM
> requires less from the driver side -- no need to understand what core
> MM behaves at certain MMU events. GMEM also expects fewer bugs than
> MMU notifiers: implementing basic operations with standard
> declarations vs. implementing whatever random device MM logic in MMU
> notifiers.

How is this proposal any different though? From what I can see it
replaces MMU notifier callbacks with TLB invalidation callbacks, but
that is essentially what MMU notifier callbacks are anyway. The "random
device MM logic" should just be clearing device TLBs. What other MM
logic has to be implemented in the MMU notifier callbacks that is the
same between devices?

>   4. GMEM plans to support a more lightweight physical memory
> management. The discussion about this part can be found in my cover
> letter. The question is whether struct page should be compatible
> (directly use HMM's ZONE_DEVICE solution) or a trimmed, smaller struct
> page that satisfies generalized demands from accelerators is more
> preferrable?

What is wrong with the current ZONE_DEVICE solution? You mention size of
struct page, but that is already being worked on through the conversion
to folios. Admittedly higher order HMM ZONE_DEVICE folios are not
currently supported, but that is something I'm actively working on at
the moment.

>   5. GMEM has been demonstrated to allow device memory
> oversubscription (a GMEM-based 32GB NPU card can run a GPT model
> oversubscribin

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[Zeng, Oak]

See inline comments

> -Original Message-
> From: dri-devel  On Behalf Of
> zhuweixi
> Sent: Thursday, November 30, 2023 5:48 AM
> To: Christian König ; Zeng, Oak
> ; Christian König ; linux-
> m...@kvack.org; linux-ker...@vger.kernel.org; a...@linux-foundation.org;
> Danilo Krummrich ; Dave Airlie ; Daniel
> Vetter 
> Cc: tvrtko.ursu...@linux.intel.com; rcampb...@nvidia.com; apop...@nvidia.com;
> z...@nvidia.com; weixi@openeuler.sh; jhubb...@nvidia.com; intel-
> g...@lists.freedesktop.org; mhairgr...@nvidia.com; Wang, Zhi A
> ; xinhui@amd.com; amd-...@lists.freedesktop.org;
> jgli...@redhat.com; dri-de...@lists.freedesktop.org; j...@nvidia.com; Vivi,
> Rodrigo ; alexander.deuc...@amd.com;
> felix.kuehl...@amd.com; intel-gvt-...@lists.freedesktop.org;
> ogab...@kernel.org; leo...@nvidia.com; mgor...@suse.de
> Subject: RE: [RFC PATCH 0/6] Supporting GMEM (generalized memory
> management) for external memory devices
> 
> Glad to know that there is a common demand for a new syscall like hmadvise(). 
> I
> expect it would also be useful for homogeneous NUMA cases. Credits to
> cudaMemAdvise() API which brought this idea to GMEM's design.
> 
> To answer @Oak's questions about GMEM vs. HMM,
> 
> Here is the major difference:
>   GMEM's main target is to stop drivers from reinventing MM code, while
> HMM/MMU notifiers provide a compatible struct page solution and a
> coordination mechanism for existing device driver MMs that requires adding
> extra code to interact with CPU MM.
> 
> A straightforward qualitative result for the main target: after integrating 
> Huawei's
> Ascend NPU driver with GMEM's interface, 30,000 lines of MM code were cut,
> leaving <100 lines invoking GMEM interface and 3,700 lines implementing 
> vendor-
> specific functions. Some code from the 3,700 lines should be further moved to
> GMEM as a generalized feature like device memory oversubscription, but not
> included in this RFC patch yet.
> 
> A list of high-level differences:
>   1. With HMM/MMU notifiers, drivers need to first implement a full MM
> subsystem. With GMEM, drivers can reuse Linux's core MM.

A full mm subsystem essentially has below functions:

Physical memory management: neither your approach nor hmm-based solution 
provide device physical memory management. You mentioned you have a plan but at 
least for now driver need to mange device physical memory.

Virtual address space management: both approach leverage linux core mm, vma for 
this.

Data eviction, migration: with hmm, driver need to implement this. It is not 
clear whether gmem has this function. I guess even gmem has it, it might be 
slow cpu data copy, compared to modern gpu's fast data copy engine.

Device page table update, va-pa mapping: I think it is driver's responsibility 
in both approach.

So from the point of re-use core MM, I don't see big difference. Maybe you did 
it more elegantly. I think it is very possible with your approach driver can be 
simpler, less codes.

> 
>   2. HMM encodes device mapping information in the CPU arch-dependent PTEs,
> while GMEM proposes an abstraction layer in vm_object. Since GMEM's
> approach further decouples the arch-related stuff, drivers do not need to
> implement separate code for X86/ARM and etc.

I don't understand this...with hmm, when a virtual address range's backing 
store is in device memory, cpu pte is encoded to point to device memory. Device 
page table is also encoded to point to the same device memory location. But 
since device memory is not accessible to CPU (DEVICE_PRIVATE), so when cpu 
access this virtual address, there is a cpu page fault. Device mapping info is 
still in device page table, not in cpu ptes.

I do not see with hmm why driver need to implement x86/arm code... driver only 
take cares of device page table. Hmm/core mm take care of cpu page table, right?

> 
>   3. MMU notifiers register hooks at certain core MM events, while GMEM
> declares basic functions and internally invokes them. GMEM requires less from
> the driver side -- no need to understand what core MM behaves at certain MMU
> events. GMEM also expects fewer bugs than MMU notifiers: implementing basic
> operations with standard declarations vs. implementing whatever random device
> MM logic in MMU notifiers.

This seems true to me. I feel the mmu notifier thing, especially the 
synchronization/lock design (those sequence numbers, interacting with driver 
lock, and the mmap lock) are very complicated. I indeed spent time to 
understand the specification documented in hmm.rst...

Your approach seems better.

> 
>   4. GMEM plans to support a more lightweight physical memory management.
> The discussion about this part can be found in my cover letter. The question 
> is
> whether struct page should be compatible (directly use HMM's ZONE_DEVICE
> solution) or a trimmed, smaller struct page that satisfies generalized demands
> from accelerators is more preferrable?
> 
>   5. GMEM has been demonstrated to allow

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for Resolve suspend-resume racing with GuC destroy-context-worker (rev8)

[Teres Alexis, Alan Previn]

On Fri, 2023-12-01 at 02:20 +, Patchwork wrote:
> Patch Details
> Series: Resolve suspend-resume racing with GuC destroy-context-worker (rev8)
> URL:https://patchwork.freedesktop.org/series/121916/
> State:  failure
> Details:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v8/index.html

alan:snip
> Here are the unknown changes that may have been introduced in 
> Patchwork_121916v8:
> 
> IGT changes
> Possible regressions
> 
>   *   igt@i915_selftest@live@requests:
>  *   bat-mtlp-6: 
> PASS
>  -> 
> DMESG-FAIL
alan: inspecting the selftest code that is failing WRT changes in this targets, 
i really dont see any relation.
the series is changing how we handle context deregistratoin either thru the 
regular context-close or through the
flushing during suspend... however the selftest reporting a reset would be an 
issue where the context is not
closed (because it would be hanging) - also its a kernel context - which should 
not get closed. 
I will run another retest after i verify trybot sees no issue.

[Intel-gfx] [PATCH] drm/i915/hwmon: Fix static analysis tool errors in i915 hwmon

[Karthik Poosa]

Updated i915 hwmon with fixes for issues reported by static analysis tool.
Fixed unintentional buffer overflow (OVERFLOW_BEFORE_WIDEN) with upcasting.

v2: Updated commit message with details of issue [Jani].

Signed-off-by: Karthik Poosa 
---
 drivers/gpu/drm/i915/i915_hwmon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c 
b/drivers/gpu/drm/i915/i915_hwmon.c
index 975da8e7f2a9..8c3f443c8347 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -175,7 +175,7 @@ hwm_power1_max_interval_show(struct device *dev, struct 
device_attribute *attr,
 * tau4 = (4 | x) << y
 * but add 2 when doing the final right shift to account for units
 */
-   tau4 = ((1 << x_w) | x) << y;
+   tau4 = (u64)((1 << x_w) | x) << y;
/* val in hwmon interface units (millisec) */
out = mul_u64_u32_shr(tau4, SF_TIME, hwmon->scl_shift_time + x_w);
 
@@ -211,7 +211,7 @@ hwm_power1_max_interval_store(struct device *dev,
r = FIELD_PREP(PKG_MAX_WIN, PKG_MAX_WIN_DEFAULT);
x = REG_FIELD_GET(PKG_MAX_WIN_X, r);
y = REG_FIELD_GET(PKG_MAX_WIN_Y, r);
-   tau4 = ((1 << x_w) | x) << y;
+   tau4 = (u64)((1 << x_w) | x) << y;
max_win = mul_u64_u32_shr(tau4, SF_TIME, hwmon->scl_shift_time + x_w);
 
if (val > max_win)
-- 
2.25.1

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/display/dp: Add the remaining Square PHY patterns DPCD register definitions (rev2)

[Patchwork]

== Series Details ==

Series: drm/display/dp: Add the remaining Square PHY patterns DPCD register 
definitions (rev2)
URL   : https://patchwork.freedesktop.org/series/123149/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13956 -> Patchwork_123149v2


Summary
---

  **FAILURE**

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

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

Participating hosts (39 -> 38)
--

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

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@i915_selftest@live@execlists:
- bat-atsm-1: [PASS][1] -> [INCOMPLETE][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-atsm-1/igt@i915_selftest@l...@execlists.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123149v2/bat-atsm-1/igt@i915_selftest@l...@execlists.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_lmem_swapping@basic:
- fi-apl-guc: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 
other tests skip
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123149v2/fi-apl-guc/igt@gem_lmem_swapp...@basic.html

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

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

  * igt@gem_tiled_pread_basic:
- bat-dg2-8:  NOTRUN -> [SKIP][6] ([i915#4079]) +1 other test skip
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123149v2/bat-dg2-8/igt@gem_tiled_pread_basic.html

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

  * igt@i915_selftest@live@mman:
- bat-rpls-1: [PASS][8] -> [TIMEOUT][9] ([i915#6794] / [i915#7392])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-rpls-1/igt@i915_selftest@l...@mman.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123149v2/bat-rpls-1/igt@i915_selftest@l...@mman.html

  * igt@i915_suspend@basic-s2idle-without-i915:
- bat-rpls-1: [PASS][10] -> [WARN][11] ([i915#8747])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-rpls-1/igt@i915_susp...@basic-s2idle-without-i915.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123149v2/bat-rpls-1/igt@i915_susp...@basic-s2idle-without-i915.html

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

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

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

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

  * igt@kms_addfb_basic@tile-pitch-mismatch:
- bat-dg2-8:  NOTRUN -> [SKIP][16] ([i915#4212] / [i915#5608])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_123149v2/bat-dg2-8/igt@kms_addfb_ba...@tile-pitch-mismatch.html

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

  * igt@kms_force_connector_basic@force-load-detect:
- bat-

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/display/dp: Add the remaining Square PHY patterns DPCD register definitions (rev2)

[Patchwork]

== Series Details ==

Series: drm/display/dp: Add the remaining Square PHY patterns DPCD register 
definitions (rev2)
URL   : https://patchwork.freedesktop.org/series/123149/
State : warning

== Summary ==

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915/dp: Use LINK_QUAL_PATTERN_* Phy test pattern names

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/dp: Use LINK_QUAL_PATTERN_* Phy 
test pattern names
URL   : https://patchwork.freedesktop.org/series/127146/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13956 -> Patchwork_127146v1


Summary
---

  **FAILURE**

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

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

Participating hosts (39 -> 34)
--

  Missing(5): fi-bsw-n3050 fi-snb-2520m bat-atsm-1 fi-pnv-d510 bat-mtlp-8 

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-b-dp-6:
- bat-adlp-11:[PASS][1] -> [DMESG-WARN][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-adlp-11/igt@kms_pipe_crc_basic@nonblocking-...@pipe-b-dp-6.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/bat-adlp-11/igt@kms_pipe_crc_basic@nonblocking-...@pipe-b-dp-6.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_lmem_swapping@basic:
- fi-apl-guc: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 
other tests skip
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/fi-apl-guc/igt@gem_lmem_swapp...@basic.html

  * igt@kms_hdmi_inject@inject-audio:
- fi-kbl-guc: [PASS][4] -> [FAIL][5] ([IGT#3])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-5:
- bat-adlp-11:[PASS][6] -> [DMESG-FAIL][7] ([i915#6868])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-n...@pipe-c-dp-5.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-n...@pipe-c-dp-5.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-d-dp-5:
- bat-adlp-11:[PASS][8] -> [FAIL][9] ([i915#9666])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-n...@pipe-d-dp-5.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-n...@pipe-d-dp-5.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence:
- bat-dg2-11: NOTRUN -> [SKIP][10] ([i915#1845] / [i915#9197])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/bat-dg2-11/igt@kms_pipe_crc_ba...@read-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc@pipe-b-dp-6:
- bat-adlp-11:[PASS][11] -> [ABORT][12] ([i915#8668])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-adlp-11/igt@kms_pipe_crc_basic@read-...@pipe-b-dp-6.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/bat-adlp-11/igt@kms_pipe_crc_basic@read-...@pipe-b-dp-6.html

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

  
 Possible fixes 

  * igt@core_hotunplug@unbind-rebind:
- fi-apl-guc: [ABORT][14] ([i915#8213] / [i915#8668]) -> [PASS][15]
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/fi-apl-guc/igt@core_hotunp...@unbind-rebind.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/fi-apl-guc/igt@core_hotunp...@unbind-rebind.html

  * igt@gem_exec_suspend@basic-s3@smem:
- bat-rpls-1: [ABORT][16] ([i915#7978]) -> [PASS][17]
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-rpls-1/igt@gem_exec_suspend@basic...@smem.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127146v1/bat-rpls-1/igt@gem_exec_suspend@basic...@smem.html

  
  [IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#6868]: https://gitlab

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[zhuweixi]

Thanks! I am planning to present GMEM in Linux MM Alignment Sessions so I can 
collect more input from the mm developers.

@Christian @Oak I will also send you invitations once a presentation is 
scheduled. :)

-Weixi

-Original Message-
From: David Hildenbrand  
Sent: Thursday, November 30, 2023 10:55 PM
To: zhuweixi ; Dave Airlie ; Christian 
König 
Cc: linux...@kvack.org; linux-ker...@vger.kernel.org; 
a...@linux-foundation.org; weixi@openeuler.sh; mgor...@suse.de; 
jgli...@redhat.com; rcampb...@nvidia.com; jhubb...@nvidia.com; 
apop...@nvidia.com; mhairgr...@nvidia.com; z...@nvidia.com; 
alexander.deuc...@amd.com; xinhui@amd.com; amd-...@lists.freedesktop.org; 
felix.kuehl...@amd.com; ogab...@kernel.org; dri-de...@lists.freedesktop.org; 
j...@nvidia.com; leo...@nvidia.com; zhen...@linux.intel.com; 
zhi.a.w...@intel.com; intel-gvt-...@lists.freedesktop.org; 
intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com; 
joonas.lahti...@linux.intel.com; rodrigo.v...@intel.com; 
tvrtko.ursu...@linux.intel.com
Subject: Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory management) 
for external memory devices

On 29.11.23 09:27, zhuweixi wrote:
> Glad to hear that more sharable code is desirable.
> IMHO, for a common MM subsystem, it is more beneficial for GMEM to 
> extend core MM instead of building a separate one.

More core-mm complexity, awesome, we all love that! ;)

--
Cheers,

David / dhildenb

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[zhuweixi]

From your argument on KVM I can see that the biggest miscommunication between 
us is that you believed that GMEM wanted to share the whole address space. No, 
it is not the case. GMEM is only providing coordination via certain mmap() 
calls. So you are raising a case supporting GMEM again -- passthrough part of 
the CPU addresses space instead of passthrough the whole CPU address space, is 
exactly what GMEM can do. On the other side, the IOMMU SVA feature wildly binds 
the whole address space -- since the hardware feature is to directly share the 
whole CPU page table.

"We really should never ever encourage people to bind their device address 
space to the CPU address space. This is a very special use case and limits the 
driver design to only this use case.
We have exercised this approach to a rather extreme degree with KFD and I can 
clearly say that doing this was a really big mistake.
As far as I can see you are about to repeat that mistake and even encourage 
others to do so as well."

-- The behavior of internally "attach device context to mm_struct" in GMEM is 
ultimately a different approach to coordinate CPU and devices. I want to 
replace MMU notifiers with this approach because I want to protect core MM from 
random interactions with external driver MMs. Both GMEM and MMU notifiers are 
binding device contexts to the CPU context, not putting them in the same 
address space. If someone is against GMEM's approach for binding CPU and device 
context, then someone should be against MMU notifiers as well.

Currently, from our discussion I think I received two messages:
1. The original AMDKFD design was rejected because of inserting 
vendor-specific stuff to the generic core MM.
2. The rejection from #1 led to your opinion that anyone cannot mix 
device and core MM together.

I think #1 really encouraged me that GMEM could help the AMDKFD driver. However 
I am also confused that why GMEM must be compared with a vendor-specific 
driver. AMDKFD was only considering a very special use case: AMD GPUs using AMD 
IOMMU. 
However, GMEM is trying to consider all generalized cases of memory devices. 
The device can be Nvidia's GPU and Huawei's NPU that use their own MMUs, or 
AMD/Intel GPUs that use IOMMUs, or other hundreds of new accelerator vendors.

-Weixi

-Original Message-
From: Christian König  
Sent: Thursday, November 30, 2023 9:05 PM
To: zhuweixi ; Dave Airlie 
Cc: linux...@kvack.org; linux-ker...@vger.kernel.org; 
a...@linux-foundation.org; weixi@openeuler.sh; mgor...@suse.de; 
jgli...@redhat.com; rcampb...@nvidia.com; jhubb...@nvidia.com; 
apop...@nvidia.com; mhairgr...@nvidia.com; z...@nvidia.com; 
alexander.deuc...@amd.com; xinhui@amd.com; amd-...@lists.freedesktop.org; 
felix.kuehl...@amd.com; ogab...@kernel.org; dri-de...@lists.freedesktop.org; 
j...@nvidia.com; leo...@nvidia.com; zhen...@linux.intel.com; 
zhi.a.w...@intel.com; intel-gvt-...@lists.freedesktop.org; 
intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com; 
joonas.lahti...@linux.intel.com; rodrigo.v...@intel.com; 
tvrtko.ursu...@linux.intel.com; Danilo Krummrich ; Daniel 
Vetter ; Zeng, Oak 
Subject: Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory management) 
for external memory devices

Am 30.11.23 um 08:22 schrieb zhuweixi:
> Add @Oak to the KFD discussion. I will reply separately elaborating your 
> questions on GMEM's difference from HMM/MMU notifiers.
>
> Christian, thanks for pointing me to that AMDKFD discussion. I have read the 
> discussion around the AMDKFD skeleton patch and found the previous discussion 
> in the following URLs:
> https://lore.kernel.org/dri-devel/1405028848-5660-1-git-send-email-ode
> d.gab...@amd.com/#r 
> https://lore.kernel.org/dri-devel/20140711154231.gb1...@gmail.com/
>
> I believe AMDKFD's original patch was rejected mostly because of inserting 
> vendor-specific stuff to the generic core MM.  Jérôme has clearly stated this 
> issue in the second URL. If the code is vendor-specific then it has no place 
> in core MM, period.
>
> But why does that vendor-specific solution relate to a generalized solution 
> like GMEM? The initial AMDKFD patch doesn't work for Nvidia or Intel.

KFD was meant to be a vendor agnostic framework, very similar to what you 
propose here.

It's just that it was seen as vendor specific because nobody else actually 
wanted to design the their drivers this way.

>
> In fact I think the rejection of the initial AMDKFD patch supports GMEM's 
> idea -- there could have been a simpler AMDKFD implementation if the core MM 
> was extended by GMEM. Also, after 9 years, there are so many other companies 
> building their accelerators over the past few years, especially now the 
> GPT-family has made a much bigger success. Don't we want to advance Linux's 
> core MM for more friendly and generalized support for the upcoming new 
> vendors?

Well exactly that's the big point: Absolutely not!

We really should never ever encourage peop

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/dp: Use LINK_QUAL_PATTERN_* Phy test pattern names

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/dp: Use LINK_QUAL_PATTERN_* Phy 
test pattern names
URL   : https://patchwork.freedesktop.org/series/127146/
State : warning

== Summary ==

Error: dim checkpatch failed
fde39bbbd88d drm/i915/dp: Use LINK_QUAL_PATTERN_* Phy test pattern names
bebe1422443c drm/i915/dp: Add TPS4 PHY test pattern support
-:46: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#46: FILE: drivers/gpu/drm/i915/display/intel_dp.c:4744:
+   intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
+   DP_TP_CTL_TRAIN_PAT4_SEL_MASK | 
DP_TP_CTL_LINK_TRAIN_MASK,

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for Resolve suspend-resume racing with GuC destroy-context-worker (rev8)

[Patchwork]

== Series Details ==

Series: Resolve suspend-resume racing with GuC destroy-context-worker (rev8)
URL   : https://patchwork.freedesktop.org/series/121916/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13956 -> Patchwork_121916v8


Summary
---

  **FAILURE**

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

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

Participating hosts (39 -> 38)
--

  Missing(1): fi-snb-2520m 

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@i915_selftest@live@requests:
- bat-mtlp-6: [PASS][1] -> [DMESG-FAIL][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-mtlp-6/igt@i915_selftest@l...@requests.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v8/bat-mtlp-6/igt@i915_selftest@l...@requests.html

  
Known issues


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

### CI changes ###

 Issues hit 

  * boot:
- fi-bsw-n3050:   [PASS][3] -> [FAIL][4] ([i915#8293])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/fi-bsw-n3050/boot.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v8/fi-bsw-n3050/boot.html

  

### IGT changes ###

 Issues hit 

  * igt@gem_lmem_swapping@basic:
- fi-apl-guc: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 
other tests skip
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v8/fi-apl-guc/igt@gem_lmem_swapp...@basic.html

  * igt@kms_hdmi_inject@inject-audio:
- fi-kbl-guc: [PASS][6] -> [FAIL][7] ([IGT#3])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v8/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
- bat-rplp-1: [PASS][8] -> [ABORT][9] ([i915#8668])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v8/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html

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

  
 Possible fixes 

  * igt@core_hotunplug@unbind-rebind:
- fi-apl-guc: [ABORT][11] ([i915#8213] / [i915#8668]) -> [PASS][12]
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/fi-apl-guc/igt@core_hotunp...@unbind-rebind.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v8/fi-apl-guc/igt@core_hotunp...@unbind-rebind.html

  * igt@gem_exec_suspend@basic-s3@smem:
- bat-rpls-1: [ABORT][13] ([i915#7978]) -> [PASS][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-rpls-1/igt@gem_exec_suspend@basic...@smem.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v8/bat-rpls-1/igt@gem_exec_suspend@basic...@smem.html

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

  [IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#8213]: https://gitlab.freedesktop.org/drm/intel/issues/8213
  [i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
  [i915#8981]: https://gitlab.freedesktop.org/drm/intel/issues/8981


Build changes
-

  * Linux: CI_DRM_13956 -> Patchwork_121916v8

  CI-20190529: 20190529
  CI_DRM_13956: b59a0a6520764f36a79ba6b4c590e243ac6b913d @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7612: b5c47966901ee1060bcb9d4bccdd3ccec9651ef4 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_121916v8: b59a0a6520764f36a79ba6b4c590e243ac6b913d @ 

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Resolve suspend-resume racing with GuC destroy-context-worker (rev8)

[Patchwork]

== Series Details ==

Series: Resolve suspend-resume racing with GuC destroy-context-worker (rev8)
URL   : https://patchwork.freedesktop.org/series/121916/
State : warning

== Summary ==

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base

[Patchwork]

== Series Details ==

Series: drm/i915/display: Check GGTT to determine phys_base
URL   : https://patchwork.freedesktop.org/series/127130/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13956 -> Patchwork_127130v1


Summary
---

  **FAILURE**

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

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

Participating hosts (39 -> 37)
--

  Missing(2): fi-snb-2520m fi-pnv-d510 

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@i915_module_load@load:
- fi-ivb-3770:[PASS][1] -> [DMESG-WARN][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/fi-ivb-3770/igt@i915_module_l...@load.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v1/fi-ivb-3770/igt@i915_module_l...@load.html
- bat-mtlp-8: [PASS][3] -> [INCOMPLETE][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-mtlp-8/igt@i915_module_l...@load.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v1/bat-mtlp-8/igt@i915_module_l...@load.html

  * igt@i915_selftest@live@workarounds:
- bat-rpls-1: [PASS][5] -> [INCOMPLETE][6]
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-rpls-1/igt@i915_selftest@l...@workarounds.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v1/bat-rpls-1/igt@i915_selftest@l...@workarounds.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_lmem_swapping@basic:
- fi-apl-guc: NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#4613]) +3 
other tests skip
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v1/fi-apl-guc/igt@gem_lmem_swapp...@basic.html

  * igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: NOTRUN -> [DMESG-FAIL][8] ([i915#5334])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-jsl-3:  [PASS][9] -> [FAIL][10] ([fdo#103375])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/bat-jsl-3/igt@i915_susp...@basic-s3-without-i915.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v1/bat-jsl-3/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_hdmi_inject@inject-audio:
- fi-kbl-guc: [PASS][11] -> [FAIL][12] ([IGT#3])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v1/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html

  
 Possible fixes 

  * igt@core_hotunplug@unbind-rebind:
- fi-apl-guc: [ABORT][13] ([i915#8213] / [i915#8668]) -> [PASS][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13956/fi-apl-guc/igt@core_hotunp...@unbind-rebind.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v1/fi-apl-guc/igt@core_hotunp...@unbind-rebind.html

  
  [IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#8213]: https://gitlab.freedesktop.org/drm/intel/issues/8213
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668


Build changes
-

  * Linux: CI_DRM_13956 -> Patchwork_127130v1

  CI-20190529: 20190529
  CI_DRM_13956: b59a0a6520764f36a79ba6b4c590e243ac6b913d @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7612: b5c47966901ee1060bcb9d4bccdd3ccec9651ef4 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_127130v1: b59a0a6520764f36a79ba6b4c590e243ac6b913d @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

ae964dda09d2 drm/i915/display: Check GGTT to determine phys_base

== Logs ==

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

Re: [Intel-gfx] [PATCH v7 2/2] drm/i915/guc: Close deregister-context race against CT-loss

[Teres Alexis, Alan Previn]

> As far as i can tell, its only if we started resetting / wedging right after 
> this
> queued worker got started.

alan: hope Daniele can proof read my tracing and confirm if got it right.

Re: [Intel-gfx] [PATCH v7 2/2] drm/i915/guc: Close deregister-context race against CT-loss

[Teres Alexis, Alan Previn]

On Thu, 2023-11-30 at 16:18 -0500, Vivi, Rodrigo wrote:
> On Wed, Nov 29, 2023 at 04:20:13PM -0800, Alan Previn wrote:
alan:snip
> > +
> > if (unlikely(disabled)) {
> > release_guc_id(guc, ce);
> > __guc_context_destroy(ce);
> > -   return;
> > +   return 0;
> 
> is success the right return case here?
alan: yes: we may discover "disabled == true" if submission_disabled
found that gt-is-wedged. I dont believe such a case will happen as
part of flushing destroyed_worker_func during suspend but may occur
as part of regular runtime context closing that just happens to
get queued in the middle of a gt-reset/wedging process. In such a case,
the reset-prepare code will sanitize everytihng including cleaning up
the pending-destructoin-contexts-link-list. So its either we pick it
from here and dump it ... or reset picks it first and dumps it there
(where both dumpings only occur if guc got disabled first).


Supplemental: How regular context cleanup leads to the same path -->

i915_sw_fence_notify -> engines_notify -> free_engines_rcu ->
intel_context_put -> kref_put(&ce->ref..) -> ce->ops->destroy ->

(where ce->ops = engine->cops and engine->cops = guc_context_ops)
*and, guc_context_ops->destroy == guc_context_destroy so ->

ce->ops->destroy -> guc_context_destroy ->
queue_work(..&guc->submission_state.destroyed_worker);
-> [eventually] -> the same guc_lrc_unpin above

However with additional "if (!intel_guc_is_ready(guc))" in destroyed_worker_func
as part of this patch, hitting this "disabled==true" case will be even less 
likely.
As far as i can tell, its only if we started resetting / wedging right after 
this
queued worker got started. (i ran out of time to check if reset can ever occur
from within the same system_unbound_wq but then i recall we also have CI using
debugfs to force wedging for select (/all?) igt tests) so i suspect it can still
happen in parallel. NOTE: the original checking of the "is disabled" is not new
code - its the original code.

...alan

P.S.- oh man, that took a lot of code tracing as i can't remember these paths 
by heart.

[Intel-gfx] [PATCH v2] drm/display/dp: Add the remaining Square PHY patterns DPCD register definitions

[Khaled Almahallawy]

DP2.1 Specs added new DPCDs definitions for square pattern configs[1]
These new definitions are used for UHBR Source Transmitter
Equalizations tests[2]. Add the 3 new values for square pattern.

v2: rebase

[1]: DP2.1 Specs - 2.12.3.6.5 Square Pattern
[2]: DP2.1 PHY CTS specs - 4.3 UHBR Source Transmitter Equalization

Cc: Jani Nikula 
Cc: Imre Deak 
Cc: Lee Shawn C 
Signed-off-by: Khaled Almahallawy 
---
 include/drm/display/drm_dp.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index 83d2039c018b..3731828825bd 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -651,6 +651,9 @@
 # define DP_LINK_QUAL_PATTERN_PRSBS31   0x38
 # define DP_LINK_QUAL_PATTERN_CUSTOM0x40
 # define DP_LINK_QUAL_PATTERN_SQUARE0x48
+# define DP_LINK_QUAL_PATTERN_SQUARE_PRESHOOT_DISABLED   0x49
+# define DP_LINK_QUAL_PATTERN_SQUARE_DEEMPHASIS_DISABLED 0x4a
+# define DP_LINK_QUAL_PATTERN_SQUARE_PRESHOOT_DEEMPHASIS_DISABLED0x4b
 
 #define DP_TRAINING_LANE0_1_SET2   0x10f
 #define DP_TRAINING_LANE2_3_SET2   0x110
-- 
2.25.1

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gem: Atomically invalidate userptr on mmu-notifier (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/gem: Atomically invalidate userptr on mmu-notifier (rev3)
URL   : https://patchwork.freedesktop.org/series/126998/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13955 -> Patchwork_126998v3


Summary
---

  **SUCCESS**

  No regressions found.

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

Participating hosts (36 -> 38)
--

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

Known issues


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

### IGT changes ###

 Issues hit 

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

  * igt@fbdev@info:
- bat-rpls-1: NOTRUN -> [SKIP][3] ([i915#1849] / [i915#2582])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126998v3/bat-rpls-1/igt@fb...@info.html

  * igt@fbdev@write:
- bat-rpls-1: NOTRUN -> [SKIP][4] ([i915#2582]) +3 other tests skip
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126998v3/bat-rpls-1/igt@fb...@write.html

  * igt@gem_exec_suspend@basic-s0@smem:
- bat-dg2-9:  [PASS][5] -> [INCOMPLETE][6] ([i915#9275])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13955/bat-dg2-9/igt@gem_exec_suspend@basic...@smem.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126998v3/bat-dg2-9/igt@gem_exec_suspend@basic...@smem.html

  * igt@gem_lmem_swapping@basic:
- fi-pnv-d510:NOTRUN -> [SKIP][7] ([fdo#109271]) +25 other tests 
skip
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126998v3/fi-pnv-d510/igt@gem_lmem_swapp...@basic.html

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

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

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

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

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

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

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

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

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

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

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

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
- bat-rpls-1: NOTRUN -> [SKIP][20] ([i915#1845]) +17 other tests 
skip
   [20]: 
h

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/gem: Atomically invalidate userptr on mmu-notifier (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/gem: Atomically invalidate userptr on mmu-notifier (rev3)
URL   : https://patchwork.freedesktop.org/series/126998/
State : warning

== Summary ==

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gem: Atomically invalidate userptr on mmu-notifier (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/gem: Atomically invalidate userptr on mmu-notifier (rev3)
URL   : https://patchwork.freedesktop.org/series/126998/
State : warning

== Summary ==

Error: dim checkpatch failed
5a0f231565eb drm/i915/gem: Atomically invalidate userptr on mmu-notifier
-:117: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does 
MAINTAINERS need updating?
#117: 
deleted file mode 100644

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for Prepare intel_fb for Xe (rev9)

[Patchwork]

== Series Details ==

Series: Prepare intel_fb for Xe (rev9)
URL   : https://patchwork.freedesktop.org/series/126507/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13955 -> Patchwork_126507v9


Summary
---

  **FAILURE**

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

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

Participating hosts (36 -> 38)
--

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

Possible new issues
---

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

### IGT changes ###

 Possible regressions 

  * igt@kms_force_connector_basic@force-connector-state:
- bat-dg1-7:  [PASS][1] -> [ABORT][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13955/bat-dg1-7/igt@kms_force_connector_ba...@force-connector-state.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v9/bat-dg1-7/igt@kms_force_connector_ba...@force-connector-state.html

  
Known issues


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

### CI changes ###

 Issues hit 

  * boot:
- fi-bsw-n3050:   [PASS][3] -> [FAIL][4] ([i915#8293])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13955/fi-bsw-n3050/boot.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v9/fi-bsw-n3050/boot.html

  

### IGT changes ###

 Issues hit 

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

  * igt@fbdev@info:
- bat-rpls-1: NOTRUN -> [SKIP][7] ([i915#1849] / [i915#2582])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v9/bat-rpls-1/igt@fb...@info.html

  * igt@fbdev@write:
- bat-rpls-1: NOTRUN -> [SKIP][8] ([i915#2582]) +3 other tests skip
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v9/bat-rpls-1/igt@fb...@write.html

  * igt@gem_lmem_swapping@basic:
- fi-pnv-d510:NOTRUN -> [SKIP][9] ([fdo#109271]) +25 other tests 
skip
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v9/fi-pnv-d510/igt@gem_lmem_swapp...@basic.html

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

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

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

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

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

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

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

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

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][19] ([i915#5190])
   [19

[Intel-gfx] [PATCH v2 2/2] drm/i915/dp: Add TPS4 PHY test pattern support

[Khaled Almahallawy]

Adding support for TPS4 (CP2520 Pattern 3) PHY pattern source tests.

v2: rebase

Bspec: 50482, 50484
Cc: Jani Nikula 
Cc: Imre Deak 
Cc: Lee Shawn C 
Signed-off-by: Khaled Almahallawy 
---
 drivers/gpu/drm/i915/display/intel_dp.c | 11 +++
 drivers/gpu/drm/i915/i915_reg.h |  4 
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index a1e63ab5761b..8908221edfa9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4679,6 +4679,7 @@ static void intel_dp_phy_pattern_update(struct intel_dp 
*intel_dp,
struct drm_dp_phy_test_params *data =
&intel_dp->compliance.test_data.phytest;
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+   struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
enum pipe pipe = crtc->pipe;
u32 pattern_val;
 
@@ -4686,6 +4687,9 @@ static void intel_dp_phy_pattern_update(struct intel_dp 
*intel_dp,
case DP_LINK_QUAL_PATTERN_DISABLE:
drm_dbg_kms(&dev_priv->drm, "Disable Phy Test Pattern\n");
intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), 0x0);
+   intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
+DP_TP_CTL_TRAIN_PAT4_SEL_MASK | 
DP_TP_CTL_LINK_TRAIN_MASK,
+DP_TP_CTL_LINK_TRAIN_NORMAL);
break;
case DP_LINK_QUAL_PATTERN_D10_2:
drm_dbg_kms(&dev_priv->drm, "Set D10.2 Phy Test Pattern\n");
@@ -4733,6 +4737,13 @@ static void intel_dp_phy_pattern_update(struct intel_dp 
*intel_dp,
   DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
   pattern_val);
break;
+   case DP_LINK_QUAL_PATTERN_CP2520_PAT_3:
+   drm_dbg_kms(&dev_priv->drm, "Set TPS4 compliance Phy Test 
Pattern\n");
+   intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), 0x0);
+   intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
+   DP_TP_CTL_TRAIN_PAT4_SEL_MASK | 
DP_TP_CTL_LINK_TRAIN_MASK,
+   DP_TP_CTL_TRAIN_PAT4_SEL_TP4a | 
DP_TP_CTL_LINK_TRAIN_PAT4);
+   break;
default:
WARN(1, "Invalid Phy Test Pattern\n");
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 27dc903f0553..7feb1e1478ee 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5652,6 +5652,10 @@ enum skl_power_gate {
 #define  DP_TP_CTL_MODE_SST(0 << 27)
 #define  DP_TP_CTL_MODE_MST(1 << 27)
 #define  DP_TP_CTL_FORCE_ACT   (1 << 25)
+#define  DP_TP_CTL_TRAIN_PAT4_SEL_MASK (3 << 19)
+#define  DP_TP_CTL_TRAIN_PAT4_SEL_TP4a (0 << 19)
+#define  DP_TP_CTL_TRAIN_PAT4_SEL_TP4b (1 << 19)
+#define  DP_TP_CTL_TRAIN_PAT4_SEL_TP4c (2 << 19)
 #define  DP_TP_CTL_ENHANCED_FRAME_ENABLE   (1 << 18)
 #define  DP_TP_CTL_FDI_AUTOTRAIN   (1 << 15)
 #define  DP_TP_CTL_LINK_TRAIN_MASK (7 << 8)
-- 
2.25.1

[Intel-gfx] [PATCH v2 1/2] drm/i915/dp: Use LINK_QUAL_PATTERN_* Phy test pattern names

[Khaled Almahallawy]

Starting from DP2.0 specs, DPCD 248h is renamed
LINK_QUAL_PATTERN_SELECT and it has the same values of registers
DPCD 10Bh-10Eh.
Use the PHY pattern names defined for DPCD 10Bh-10Eh in order to add
CP2520 Pattern 3 (TPS4) phy pattern support in the next
patch of this series and DP2.1 PHY patterns for future series.

v2: rebase

Cc: Jani Nikula 
Cc: Imre Deak 
Cc: Lee Shawn C 
Signed-off-by: Khaled Almahallawy 
---
 drivers/gpu/drm/i915/display/intel_dp.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 3b2482bf683f..a1e63ab5761b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4683,27 +4683,27 @@ static void intel_dp_phy_pattern_update(struct intel_dp 
*intel_dp,
u32 pattern_val;
 
switch (data->phy_pattern) {
-   case DP_PHY_TEST_PATTERN_NONE:
+   case DP_LINK_QUAL_PATTERN_DISABLE:
drm_dbg_kms(&dev_priv->drm, "Disable Phy Test Pattern\n");
intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), 0x0);
break;
-   case DP_PHY_TEST_PATTERN_D10_2:
+   case DP_LINK_QUAL_PATTERN_D10_2:
drm_dbg_kms(&dev_priv->drm, "Set D10.2 Phy Test Pattern\n");
intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe),
   DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
break;
-   case DP_PHY_TEST_PATTERN_ERROR_COUNT:
+   case DP_LINK_QUAL_PATTERN_ERROR_RATE:
drm_dbg_kms(&dev_priv->drm, "Set Error Count Phy Test 
Pattern\n");
intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe),
   DDI_DP_COMP_CTL_ENABLE |
   DDI_DP_COMP_CTL_SCRAMBLED_0);
break;
-   case DP_PHY_TEST_PATTERN_PRBS7:
+   case DP_LINK_QUAL_PATTERN_PRBS7:
drm_dbg_kms(&dev_priv->drm, "Set PRBS7 Phy Test Pattern\n");
intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe),
   DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
break;
-   case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
+   case DP_LINK_QUAL_PATTERN_80BIT_CUSTOM:
/*
 * FIXME: Ideally pattern should come from DPCD 0x250. As
 * current firmware of DPR-100 could not set it, so hardcoding
@@ -4721,7 +4721,7 @@ static void intel_dp_phy_pattern_update(struct intel_dp 
*intel_dp,
   DDI_DP_COMP_CTL_ENABLE |
   DDI_DP_COMP_CTL_CUSTOM80);
break;
-   case DP_PHY_TEST_PATTERN_CP2520:
+   case DP_LINK_QUAL_PATTERN_CP2520_PAT_1:
/*
 * FIXME: Ideally pattern should come from DPCD 0x24A. As
 * current firmware of DPR-100 could not set it, so hardcoding
-- 
2.25.1

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Prepare intel_fb for Xe (rev9)

[Patchwork]

== Series Details ==

Series: Prepare intel_fb for Xe (rev9)
URL   : https://patchwork.freedesktop.org/series/126507/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/inc

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Prepare intel_fb for Xe (rev9)

[Patchwork]

== Series Details ==

Series: Prepare intel_fb for Xe (rev9)
URL   : https://patchwork.freedesktop.org/series/126507/
State : warning

== Summary ==

Error: dim checkpatch failed
98aab4aecbcc drm/i915/display: use intel_bo_to_drm_bo in intel_fb.c
bbe829fb2e51 drm/i915/display: Convert intel_fb_modifier_to_tiling as non-static
06edcb314cab drm/i915/display: Handle invalid fb_modifier in 
intel_fb_modifier_to_tiling
9874e160b824 drm/i915/display: Split i915 specific code away from intel_fb.c
Traceback (most recent call last):
  File "scripts/spdxcheck.py", line 6, in 
from ply import lex, yacc
ModuleNotFoundError: No module named 'ply'
-:171: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does 
MAINTAINERS need updating?
#171: 
new file mode 100644

-:176: WARNING:SPDX_LICENSE_TAG: Improper SPDX comment style for 
'drivers/gpu/drm/i915/display/intel_fb_bo.c', please use '//' instead
#176: FILE: drivers/gpu/drm/i915/display/intel_fb_bo.c:1:
+/* SPDX-License-Identifier: MIT */

-:176: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier 
tag in line 1
#176: FILE: drivers/gpu/drm/i915/display/intel_fb_bo.c:1:
+/* SPDX-License-Identifier: MIT */

total: 0 errors, 3 warnings, 0 checks, 240 lines checked

Re: [Intel-gfx] [PATCH v7 2/2] drm/i915/guc: Close deregister-context race against CT-loss

[Rodrigo Vivi]

On Wed, Nov 29, 2023 at 04:20:13PM -0800, Alan Previn wrote:
> If we are at the end of suspend or very early in resume
> its possible an async fence signal (via rcu_call) is triggered
> to free_engines which could lead us to the execution of
> the context destruction worker (after a prior worker flush).
> 
> Thus, when suspending, insert rcu_barriers at the start
> of i915_gem_suspend (part of driver's suspend prepare) and
> again in i915_gem_suspend_late so that all such cases have
> completed and context destruction list isn't missing anything.
> 
> In destroyed_worker_func, close the race against CT-loss
> by checking that CT is enabled before calling into
> deregister_destroyed_contexts.
> 
> Based on testing, guc_lrc_desc_unpin may still race and fail
> as we traverse the GuC's context-destroy list because the
> CT could be disabled right before calling GuC's CT send function.
> 
> We've witnessed this race condition once every ~6000-8000
> suspend-resume cycles while ensuring workloads that render
> something onscreen is continuously started just before
> we suspend (and the workload is small enough to complete
> and trigger the queued engine/context free-up either very
> late in suspend or very early in resume).
> 
> In such a case, we need to unroll the entire process because
> guc-lrc-unpin takes a gt wakeref which only gets released in
> the G2H IRQ reply that never comes through in this corner
> case. Without the unroll, the taken wakeref is leaked and will
> cascade into a kernel hang later at the tail end of suspend in
> this function:
> 
>intel_wakeref_wait_for_idle(>->wakeref)
>(called by) - intel_gt_pm_wait_for_idle
>(called by) - wait_for_suspend
> 
> Thus, do an unroll in guc_lrc_desc_unpin and deregister_destroyed_-
> contexts if guc_lrc_desc_unpin fails due to CT send falure.
> When unrolling, keep the context in the GuC's destroy-list so
> it can get picked up on the next destroy worker invocation
> (if suspend aborted) or get fully purged as part of a GuC
> sanitization (end of suspend) or a reset flow.
> 
> Signed-off-by: Alan Previn 
> Signed-off-by: Anshuman Gupta 
> Tested-by: Mousumi Jana 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_pm.c| 10 +++
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 73 +--
>  2 files changed, 78 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> index 0d812f4d787d..3b27218aabe2 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> @@ -28,6 +28,13 @@ void i915_gem_suspend(struct drm_i915_private *i915)
>   GEM_TRACE("%s\n", dev_name(i915->drm.dev));
>  
>   intel_wakeref_auto(&i915->runtime_pm.userfault_wakeref, 0);
> + /*
> +  * On rare occasions, we've observed the fence completion triggers
> +  * free_engines asynchronously via rcu_call. Ensure those are done.
> +  * This path is only called on suspend, so it's an acceptable cost.
> +  */
> + rcu_barrier();
> +
>   flush_workqueue(i915->wq);
>  
>   /*
> @@ -160,6 +167,9 @@ void i915_gem_suspend_late(struct drm_i915_private *i915)
>* machine in an unusable condition.
>*/
>  
> + /* Like i915_gem_suspend, flush tasks staged from fence triggers */
> + rcu_barrier();
> +
>   for_each_gt(gt, i915, i)
>   intel_gt_suspend_late(gt);
>  
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 6e3fb2fcce4f..a7228bebec39 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -236,6 +236,13 @@ set_context_destroyed(struct intel_context *ce)
>   ce->guc_state.sched_state |= SCHED_STATE_DESTROYED;
>  }
>  
> +static inline void
> +clr_context_destroyed(struct intel_context *ce)
> +{
> + lockdep_assert_held(&ce->guc_state.lock);
> + ce->guc_state.sched_state &= ~SCHED_STATE_DESTROYED;
> +}
> +
>  static inline bool context_pending_disable(struct intel_context *ce)
>  {
>   return ce->guc_state.sched_state & SCHED_STATE_PENDING_DISABLE;
> @@ -613,6 +620,8 @@ static int guc_submission_send_busy_loop(struct intel_guc 
> *guc,
>u32 g2h_len_dw,
>bool loop)
>  {
> + int ret;
> +
>   /*
>* We always loop when a send requires a reply (i.e. g2h_len_dw > 0),
>* so we don't handle the case where we don't get a reply because we
> @@ -623,7 +632,11 @@ static int guc_submission_send_busy_loop(struct 
> intel_guc *guc,
>   if (g2h_len_dw)
>   atomic_inc(&guc->outstanding_submission_g2h);
>  
> - return intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
> + ret = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
> + if (ret)
> + atomic_dec(&guc->outstanding_submis

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: handle uncore spinlock when not available (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: handle uncore spinlock when not available (rev3)
URL   : https://patchwork.freedesktop.org/series/125442/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13955 -> Patchwork_125442v3


Summary
---

  **SUCCESS**

  No regressions found.

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

Participating hosts (36 -> 37)
--

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

Known issues


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

### IGT changes ###

 Issues hit 

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

  * igt@fbdev@info:
- bat-rpls-1: NOTRUN -> [SKIP][2] ([i915#1849] / [i915#2582])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/bat-rpls-1/igt@fb...@info.html

  * igt@fbdev@write:
- bat-rpls-1: NOTRUN -> [SKIP][3] ([i915#2582]) +3 other tests skip
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/bat-rpls-1/igt@fb...@write.html

  * igt@gem_lmem_swapping@basic:
- fi-pnv-d510:NOTRUN -> [SKIP][4] ([fdo#109271]) +25 other tests 
skip
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/fi-pnv-d510/igt@gem_lmem_swapp...@basic.html

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

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

  * igt@i915_module_load@reload:
- fi-apl-guc: [PASS][7] -> [DMESG-WARN][8] ([i915#180] / 
[i915#1982] / [i915#8585]) +1 other test dmesg-warn
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13955/fi-apl-guc/igt@i915_module_l...@reload.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/fi-apl-guc/igt@i915_module_l...@reload.html

  * igt@i915_pm_rpm@module-reload:
- fi-apl-guc: [PASS][9] -> [DMESG-WARN][10] ([i915#180] / 
[i915#8585])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13955/fi-apl-guc/igt@i915_pm_...@module-reload.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/fi-apl-guc/igt@i915_pm_...@module-reload.html

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

  * igt@i915_selftest@live@execlists:
- fi-bsw-n3050:   [PASS][12] -> [ABORT][13] ([i915#7911])
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13955/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html

  * igt@i915_selftest@live@reset:
- fi-apl-guc: [PASS][14] -> [DMESG-WARN][15] ([i915#9730]) +36 
other tests dmesg-warn
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13955/fi-apl-guc/igt@i915_selftest@l...@reset.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/fi-apl-guc/igt@i915_selftest@l...@reset.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
- bat-rpls-1: NOTRUN -> [SKIP][16] ([i915#1845]) +17 other tests 
skip
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/bat-rpls-1/igt@kms_cursor_leg...@basic-flip-after-cursor-legacy.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
- bat-adlp-11:[PASS][17] -> [DMESG-WARN][18] ([i915#6868])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13955/bat-adlp-11/igt@kms_cursor_leg...@basic-flip-before-cursor-legacy.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/bat-adlp-11/igt@kms_cursor_leg...@basic-flip-before-cursor-legacy.html

  * igt@kms_flip@basic-flip-vs-modeset:
- bat-rpls-1: NOTRUN -> [SKIP][19] ([i915#3637]) +3 other tests skip
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125442v3/bat-rpls-1/igt@kms_f...@basic-flip-vs-modeset.html

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

  * igt@kms_frontbuffer_tracking@basic:
- bat-rpls-1: NOTRUN -> [SKIP][21] ([i915#1849] / [i915#5354])
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: handle uncore spinlock when not available (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: handle uncore spinlock when not available (rev3)
URL   : https://patchwork.freedesktop.org/series/125442/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+drivers/gpu/drm/i915/display/intel_vblank.c:278:13: warning: context imbalance 
in 'intel_vblank_section_enter' - wrong count at exit
+drivers/gpu/drm/i915/display/intel_vblank.c:285:13: warning: context imbalance 
in 'intel_vblank_section_exit' - unexpected unlock

Re: [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table

[Rodrigo Vivi]

On Wed, Nov 29, 2023 at 12:51:21PM -0800, Matt Atwood wrote:
> Function reg_in_range_table is useful in more places, move function to
> intel_uncore.h to make available to more places.
> 
> Signed-off-by: Matt Atwood 
> ---
>  drivers/gpu/drm/i915/i915_perf.c| 13 +
>  drivers/gpu/drm/i915/intel_uncore.h | 12 
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
> b/drivers/gpu/drm/i915/i915_perf.c
> index 7b1c8de2f9cb3..5e5dc73621379 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -219,6 +219,7 @@
>  #include "i915_perf.h"
>  #include "i915_perf_oa_regs.h"
>  #include "i915_reg.h"
> +#include "intel_uncore.h"
>  
>  /* HW requires this to be a power of two, between 128k and 16M, though driver
>   * is currently generally designed assuming the largest 16M size is used such
> @@ -4324,18 +4325,6 @@ static bool gen8_is_valid_flex_addr(struct i915_perf 
> *perf, u32 addr)
>   return false;
>  }
>  
> -static bool reg_in_range_table(u32 addr, const struct i915_range *table)
> -{
> - while (table->start || table->end) {
> - if (addr >= table->start && addr <= table->end)
> - return true;
> -
> - table++;
> - }
> -
> - return false;
> -}
> -
>  #define REG_EQUAL(addr, mmio) \
>   ((addr) == i915_mmio_reg_offset(mmio))
>  
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
> b/drivers/gpu/drm/i915/intel_uncore.h
> index f419c311a0dea..1e85eaec1fc5a 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -496,6 +496,18 @@ static inline int intel_uncore_write_and_verify(struct 
> intel_uncore *uncore,
>   return (reg_val & mask) != expected_val ? -EINVAL : 0;
>  }
>  
> +static inline bool reg_in_range_table(u32 addr, const struct i915_range 
> *table)

exported functions should carry the name of the component that is exposing it.

something like intel_uncore_reg_in_range()

but also, based on your second patch, this doesn't seem to be the right thing 
to do.

although I hate the i915_utils.h, that sounds like a better place for a 
function like this.

or on a second thought... maybe just duplicate the logic that is inside this
function if only one extra call... or maybe duplicate this function along
the other table definition instead of having the table in one place and using
the helper from another place.

> +{
> + while (table->start || table->end) {
> + if (addr >= table->start && addr <= table->end)
> + return true;
> +
> + table++;
> + }
> +
> + return false;
> +}
> +
>  static inline void __iomem *intel_uncore_regs(struct intel_uncore *uncore)
>  {
>   return uncore->regs;
> -- 
> 2.40.1
> 

Re: [Intel-gfx] [PULL] drm-intel-fixes

[Ville Syrjälä]

On Thu, Nov 30, 2023 at 04:58:48PM +0200, Jani Nikula wrote:
> 
> Hi Dave & Sima -
> 
> i915 fixes for v6.7-rc4.
> 
> drm-intel-fixes-2023-11-30:
> drm/i915 fixes for v6.7-rc4:
> - Mark internal GSC engine with reserved uabi class
> - Take VGA converters into account in eDP probe
> - Fix intel_pre_plane_updates() call to ensure workarounds get applied
> 
> BR,
> Jani.
> 
> The following changes since commit 2cc14f52aeb78ce3f29677c2de1f06c0e91471ab:
> 
>   Linux 6.7-rc3 (2023-11-26 19:59:33 -0800)
> 
> are available in the Git repository at:
> 
>   git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2023-11-30
> 
> for you to fetch changes up to d21a3962d3042e6f56ad324cf18bdd64a1e6ecfa:
> 
>   drm/i915: Call intel_pre_plane_updates() also for pipes getting enabled 
> (2023-11-29 10:23:25 +0200)
> 
> 
> drm/i915 fixes for v6.7-rc4:
> - Mark internal GSC engine with reserved uabi class
> - Take VGA converters into account in eDP probe
> - Fix intel_pre_plane_updates() call to ensure workarounds get applied
> 
> 
> Tvrtko Ursulin (1):
>   drm/i915/gsc: Mark internal GSC engine with reserved uabi class
> 
> Ville Syrjälä (2):
>   drm/i915: Also check for VGA converter in eDP probe
>   drm/i915: Call intel_pre_plane_updates() also for pipes getting enabled

That last one might also require
commit bc53c4d56eb2 ("drm/i915: Check pipe active state in 
{planes,vrr}_{enabling,disabling}()")

The vrr stuff in particular might go wonky otherwise.

> 
>  drivers/gpu/drm/i915/display/intel_display.c |  3 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c  | 28 +++-
>  drivers/gpu/drm/i915/gt/intel_engine_user.c  | 39 
> 
>  3 files changed, 46 insertions(+), 24 deletions(-)
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH] drm/i915/display: Fix phys_base to be relative not absolute

[Paz Zcharya]

On Tue, Nov 28, 2023 at 12:12:08PM +0100, Andrzej Hajda wrote:
> On 28.11.2023 04:47, Paz Zcharya wrote:
> > 
> > On Mon, Nov 27, 2023 at 8:20 PM Paz Zcharya  wrote:
> > > 
> > > On 21.11.2023 13:06, Andrzej Hajda wrote:
> > > 
> > > > The simplest approach would be then do the same as in case of DGFX:
> > > >   gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
> > > >   gen8_pte_t pte;
> > > > 
> > > >   gte += base / I915_GTT_PAGE_SIZE;
> > > > 
> > > >   pte = ioread64(gte);
> > > >   phys_base = pte & I915_GTT_PAGE_MASK;
> > > > 
> > > > Regards
> > > > Andrzej
> > 
> > Hey Andrzej,
> > 
> > On a second thought, what do you think about something like
> > 
> > +   gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
> > +   gen8_pte_t pte;
> > +   gte += base / I915_GTT_PAGE_SIZE;
> > +   pte = ioread64(gte);
> > +   pte = pte & I915_GTT_PAGE_MASK;
> > +   phys_base = pte - i915->mm.stolen_region->region.start;
> > 
> > The only difference is the last line.
> 
> Bingo :) It seems to be generic algorithm to get phys_base for all
> platforms:
> - on older platforms stolen_region points to system memory which starts at
> 0,
> - on DG2 it uses lmem region which starts at 0 as well,
> - on MTL stolen_region points to stolen-local which starts at 0x80.
> 
> So this whole "if (IS_DGFX(i915)) {...} else {...}" could be replaced
> with sth generic.
> 1. Find pte.
> 2. if(IS_DGFX(i915) && pte & GEN12_GGTT_PTE_LM) mem =
> i915->mm.regions[INTEL_REGION_LMEM_0] else mem = i915->mm.stolen_region
> 3. phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
> 
> Regards
> Andrzej
> 
> 

Hey Andrzej,

I uploaded https://patchwork.freedesktop.org/series/127130/ based on
algorithm. Please take a look and let me know if you'd like me to change
anything.

Really appreciate all of your help!


Best,
Paz

[Intel-gfx] [PATCH] drm/i915/display: Check GGTT to determine phys_base

[Paz Zcharya]

There was an assumption that for iGPU there should be a 1:1 mapping
of GGTT to physical address pointing to actual framebuffer.
This assumption is not valid anymore for MTL.
Fix that by checking GGTT to determine the phys address.

The following algorithm for phys_base should be valid for all platforms:
1. Find pte
2. if(IS_DGFX(i915) && pte & GEN12_GGTT_PTE_LM) mem =
i915->mm.regions[INTEL_REGION_LMEM_0] else mem = i915->mm.stolen_region
3. phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;

- On older platforms, stolen_region points to system memory, starting at 0
- on DG2, it uses lmem region which starts at 0 as well
- on MTL, stolen_region points to stolen-local which starts at 0x80

Signed-off-by: Paz Zcharya 
---

 .../drm/i915/display/intel_plane_initial.c| 45 +--
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c 
b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index a55c09cbd0e4..c4bf02ea966c 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -51,6 +51,8 @@ initial_plane_vma(struct drm_i915_private *i915,
struct intel_memory_region *mem;
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
+   gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
+   gen8_pte_t pte;
resource_size_t phys_base;
u32 base, size;
u64 pinctl;
@@ -59,44 +61,41 @@ initial_plane_vma(struct drm_i915_private *i915,
return NULL;
 
base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT);
-   if (IS_DGFX(i915)) {
-   gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
-   gen8_pte_t pte;
+   gte += base / I915_GTT_PAGE_SIZE;
 
-   gte += base / I915_GTT_PAGE_SIZE;
+   pte = ioread64(gte);
 
-   pte = ioread64(gte);
+   if (IS_DGFX(i915)) {
if (!(pte & GEN12_GGTT_PTE_LM)) {
drm_err(&i915->drm,
"Initial plane programming missing PTE_LM 
bit\n");
return NULL;
}
-
-   phys_base = pte & I915_GTT_PAGE_MASK;
mem = i915->mm.regions[INTEL_REGION_LMEM_0];
-
-   /*
-* We don't currently expect this to ever be placed in the
-* stolen portion.
-*/
-   if (phys_base >= resource_size(&mem->region)) {
-   drm_err(&i915->drm,
-   "Initial plane programming using invalid range, 
phys_base=%pa\n",
-   &phys_base);
-   return NULL;
-   }
-
-   drm_dbg(&i915->drm,
-   "Using phys_base=%pa, based on initial plane 
programming\n",
-   &phys_base);
} else {
-   phys_base = base;
mem = i915->mm.stolen_region;
}
 
+   phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
+
if (!mem)
return NULL;
 
+   /*
+* We don't currently expect this to ever be placed in the
+* stolen portion.
+*/
+   if (phys_base >= resource_size(&mem->region)) {
+   drm_err(&i915->drm,
+   "Initial plane programming using invalid range, 
phys_base=%pa\n",
+   &phys_base);
+   return NULL;
+   }
+
+   drm_dbg(&i915->drm,
+   "Using phys_base=%pa, based on initial plane programming\n",
+   &phys_base);
+
size = round_up(plane_config->base + plane_config->size,
mem->min_page_size);
size -= base;
-- 
2.43.0.rc1.413.gea7ed67945-goog

Re: [Intel-gfx] [Intel-xe] [PATCH v6] drm/i915: handle uncore spinlock when not available

[Coelho, Luciano]

On Thu, 2023-11-30 at 09:31 -0500, Rodrigo Vivi wrote:
> On Thu, Nov 30, 2023 at 01:54:13PM +, Coelho, Luciano wrote:
> > On Thu, 2023-11-30 at 13:24 +, Tvrtko Ursulin wrote:
> > > On 30/11/2023 12:26, Coelho, Luciano wrote:
> > > > On Thu, 2023-11-30 at 12:21 +, Tvrtko Ursulin wrote:
> > > > > On 30/11/2023 11:35, Luca Coelho wrote:
> > > > > > The uncore code may not always be available (e.g. when we build the
> > > > > > display code with Xe), so we can't always rely on having the 
> > > > > > uncore's
> > > > > > spinlock.
> > > > > > 
> > > > > > To handle this, split the spin_lock/unlock_irqsave/restore() into
> > > > > > spin_lock/unlock() followed by a call to local_irq_save/restore() 
> > > > > > and
> > > > > > create wrapper functions for locking and unlocking the uncore's
> > > > > > spinlock.  In these functions, we have a condition check and only
> > > > > > actually try to lock/unlock the spinlock when I915 is defined, and
> > > > > > thus uncore is available.
> > > > > > 
> > > > > > This keeps the ifdefs contained in these new functions and all such
> > > > > > logic inside the display code.
> > > > > > 
> > > > > > Cc: Tvrtko Ursulin 
> > > > > > Cc: Jani Nikula 
> > > > > > Cc: Ville Syrjala 
> > > > > > Reviewed-by: Rodrigo Vivi 
> > > > > > Signed-off-by: Luca Coelho 
> > > > > > ---
> > > > > > 
> > > > > > 
> > > > > > In v2:
> > > > > > 
> > > > > >  * Renamed uncore_spin_*() to intel_spin_*()
> > > > > >  * Corrected the order: save, lock, unlock, restore
> > > > > > 
> > > > > > In v3:
> > > > > > 
> > > > > >  * Undid the change to pass drm_i915_private instead of the lock
> > > > > >itself, since we would have to include i915_drv.h and that 
> > > > > > pulls
> > > > > >in a truckload of other includes.
> > > > > > 
> > > > > > In v4:
> > > > > > 
> > > > > >  * After a brief attempt to replace this with a different patch,
> > > > > >we're back to this one;
> > > > > >  * Pass drm_i195_private again, and move the functions to
> > > > > >intel_vblank.c, so we don't need to include i915_drv.h in a
> > > > > >header file and it's already included in intel_vblank.c;
> > > > > > 
> > > > > > In v5:
> > > > > > 
> > > > > >  * Remove stray include in intel_display.h;
> > > > > >  * Remove unnecessary inline modifiers in the new functions.
> > > > > > 
> > > > > > In v6:
> > > > > > 
> > > > > >  * Just removed the umlauts from Ville's name, because patchwork
> > > > > >didn't catch my patch and I suspect it was some UTF-8 
> > > > > > confusion.
> > > > > > 
> > > > > >drivers/gpu/drm/i915/display/intel_vblank.c | 49 
> > > > > > -
> > > > > >1 file changed, 39 insertions(+), 10 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
> > > > > > b/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > > > index 2cec2abf9746..221fcd6bf77b 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > > > @@ -265,6 +265,30 @@ int intel_crtc_scanline_to_hw(struct 
> > > > > > intel_crtc *crtc, int scanline)
> > > > > > return (scanline + vtotal - crtc->scanline_offset) % vtotal;
> > > > > >}
> > > > > >
> > > > > > +/*
> > > > > > + * The uncore version of the spin lock functions is used to decide
> > > > > > + * whether we need to lock the uncore lock or not.  This is only
> > > > > > + * needed in i915, not in Xe.
> > > > > > + *
> > > > > > + * This lock in i915 is needed because some old platforms (at least
> > > > > > + * IVB and possibly HSW as well), which are not supported in Xe, 
> > > > > > need
> > > > > > + * all register accesses to the same cacheline to be serialized,
> > > > > > + * otherwise they may hang.
> > > > > > + */
> > > > > > +static void intel_vblank_section_enter(struct drm_i915_private 
> > > > > > *i915)
> > > > > > +{
> > > > > > +#ifdef I915
> > > > > > +   spin_lock(&i915->uncore.lock);
> > > > > > +#endif
> > > > > > +}
> > > > > > +
> > > > > > +static void intel_vblank_section_exit(struct drm_i915_private 
> > > > > > *i915)
> > > > > > +{
> > > > > > +#ifdef I915
> > > > > > +   spin_unlock(&i915->uncore.lock);
> > > > > > +#endif
> > > > > > +}
> > > > > > +
> > > > > >static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
> > > > > >  bool in_vblank_irq,
> > > > > >  int *vpos, int *hpos,
> > > > > > @@ -302,11 +326,12 @@ static bool i915_get_crtc_scanoutpos(struct 
> > > > > > drm_crtc *_crtc,
> > > > > > }
> > > > > >
> > > > > > /*
> > > > > > -* Lock uncore.lock, as we will do multiple timing critical raw
> > > > > > -* register reads, potentially with preemption disabled, so the
> > > > > > -* following code must not block on uncore.lock.
> > > > > > +* Enter vblank critical section, as we will do multipl

Re: [Intel-gfx] [PATCH] drm/i915/display: do not use cursor size reduction on MTL

[Andi Shyti]

Hi Andrzej,

On Fri, Nov 24, 2023 at 08:53:04AM +0100, Andrzej Hajda wrote:
> Cursor size reduction is not supported since MTL.
> 
> Signed-off-by: Andrzej Hajda 

Reviewed-by: Andi Shyti 

Thanks,
Andi

[Intel-gfx] [PULL] drm-intel-fixes

[Jani Nikula]

Hi Dave & Sima -

i915 fixes for v6.7-rc4.

drm-intel-fixes-2023-11-30:
drm/i915 fixes for v6.7-rc4:
- Mark internal GSC engine with reserved uabi class
- Take VGA converters into account in eDP probe
- Fix intel_pre_plane_updates() call to ensure workarounds get applied

BR,
Jani.

The following changes since commit 2cc14f52aeb78ce3f29677c2de1f06c0e91471ab:

  Linux 6.7-rc3 (2023-11-26 19:59:33 -0800)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2023-11-30

for you to fetch changes up to d21a3962d3042e6f56ad324cf18bdd64a1e6ecfa:

  drm/i915: Call intel_pre_plane_updates() also for pipes getting enabled 
(2023-11-29 10:23:25 +0200)


drm/i915 fixes for v6.7-rc4:
- Mark internal GSC engine with reserved uabi class
- Take VGA converters into account in eDP probe
- Fix intel_pre_plane_updates() call to ensure workarounds get applied


Tvrtko Ursulin (1):
  drm/i915/gsc: Mark internal GSC engine with reserved uabi class

Ville Syrjälä (2):
  drm/i915: Also check for VGA converter in eDP probe
  drm/i915: Call intel_pre_plane_updates() also for pipes getting enabled

 drivers/gpu/drm/i915/display/intel_display.c |  3 ++-
 drivers/gpu/drm/i915/display/intel_dp.c  | 28 +++-
 drivers/gpu/drm/i915/gt/intel_engine_user.c  | 39 
 3 files changed, 46 insertions(+), 24 deletions(-)

-- 
Jani Nikula, Intel

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[David Hildenbrand]

On 29.11.23 09:27, zhuweixi wrote:

Glad to hear that more sharable code is desirable.
IMHO, for a common MM subsystem, it is more beneficial for
GMEM to extend core MM instead of building a separate one.


More core-mm complexity, awesome, we all love that! ;)

--
Cheers,

David / dhildenb

Re: [Intel-gfx] [PATCH v7 4/4] drm/i915/display: Split i915 specific code away from intel_fb.c

[Hogander, Jouni]

On Tue, 2023-11-28 at 21:44 +0200, Ville Syrjälä wrote:
> On Tue, Nov 28, 2023 at 05:39:20PM +0200, Jouni Högander wrote:
> > We are preparing for Xe driver. Backing object implementation is
> > differing
> > between i915 and Xe. Split i915 specific code into separate source
> > file
> > built only for i915.
> > 
> > v7:
> >   - drop #include 
> >   - s/user_mode_cmd/mode_cmd/
> >   - Use passed i915 pointer instead of to_i915(obj->base.dev)
> > v6: Add missing intel_fb_bo.[ch]
> > v5:
> >   - Keep drm_any_plane_has_format check in intel_fb.c
> >   - Use mode_cmd instead of user_mode_cmd for
> > intel_fb_bo_lookup_valid_bo
> > v4: Move drm_any_plane_has_format check into intel_fb_bo.c
> > v3: Fix failure handling in intel_framebuffer_init
> > v2: Couple of fixes to error value handling
> > 
> > Signed-off-by: Jouni Högander 
> 
> Reviewed-by: Ville Syrjälä 

There were still couple of kms_addfb_basic failures on dg1/2 due to
unexpected error values:

https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v8/shard-dg2-1/igt@kms_addfb_ba...@invalid-smem-bo-on-discrete.html

Sent out new version.

BR,

Jouni Högander
> 
> > ---
> >  drivers/gpu/drm/i915/Makefile  |  1 +
> >  drivers/gpu/drm/i915/display/intel_fb.c    | 69 ++--
> >  drivers/gpu/drm/i915/display/intel_fb_bo.c | 92
> > ++
> >  drivers/gpu/drm/i915/display/intel_fb_bo.h | 24 ++
> >  4 files changed, 125 insertions(+), 61 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.h
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile
> > index 65e984242089..5060c7b98a56 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -280,6 +280,7 @@ i915-y += \
> > display/intel_dsb.o \
> > display/intel_dsb_buffer.o \
> > display/intel_fb.o \
> > +   display/intel_fb_bo.o \
> > display/intel_fb_pin.o \
> > display/intel_fbc.o \
> > display/intel_fdi.o \
> > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
> > b/drivers/gpu/drm/i915/display/intel_fb.c
> > index 868e39291e9f..2c37806b379a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fb.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> > @@ -4,7 +4,6 @@
> >   */
> >  
> >  #include 
> > -#include 
> >  #include 
> >  
> >  #include 
> > @@ -15,6 +14,7 @@
> >  #include "intel_display_types.h"
> >  #include "intel_dpt.h"
> >  #include "intel_fb.h"
> > +#include "intel_fb_bo.h"
> >  #include "intel_frontbuffer.h"
> >  
> >  #define check_array_bounds(i915, a, i) drm_WARN_ON(&(i915)->drm,
> > (i) >= ARRAY_SIZE(a))
> > @@ -1985,7 +1985,6 @@ int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> > struct drm_i915_private *dev_priv =
> > to_i915(intel_bo_to_drm_bo(obj)->dev);
> > struct drm_framebuffer *fb = &intel_fb->base;
> > u32 max_stride;
> > -   unsigned int tiling, stride;
> > int ret = -EINVAL;
> > int i;
> >  
> > @@ -1993,32 +1992,11 @@ int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> > if (!intel_fb->frontbuffer)
> > return -ENOMEM;
> >  
> > -   i915_gem_object_lock(obj, NULL);
> > -   tiling = i915_gem_object_get_tiling(obj);
> > -   stride = i915_gem_object_get_stride(obj);
> > -   i915_gem_object_unlock(obj);
> > -
> > -   if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> > -   /*
> > -    * If there's a fence, enforce that
> > -    * the fb modifier and tiling mode match.
> > -    */
> > -   if (tiling != I915_TILING_NONE &&
> > -   tiling != intel_fb_modifier_to_tiling(mode_cmd-
> > >modifier[0])) {
> > -   drm_dbg_kms(&dev_priv->drm,
> > -   "tiling_mode doesn't match fb
> > modifier\n");
> > -   goto err;
> > -   }
> > -   } else {
> > -   if (tiling == I915_TILING_X) {
> > -   mode_cmd->modifier[0] =
> > I915_FORMAT_MOD_X_TILED;
> > -   } else if (tiling == I915_TILING_Y) {
> > -   drm_dbg_kms(&dev_priv->drm,
> > -   "No Y tiling for legacy
> > addfb\n");
> > -   goto err;
> > -   }
> > -   }
> > +   ret = intel_fb_bo_framebuffer_init(intel_fb, obj,
> > mode_cmd);
> > +   if (ret)
> > +   goto err;
> >  
> > +   ret = -EINVAL;
> > if (!drm_any_plane_has_format(&dev_priv->drm,
> >   mode_cmd->pixel_format,
> >   mode_cmd->modifier[0])) {
> > @@ -2028,17 +2006,6 @@ int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> > goto err;
> > }
> >  
> > -   /*
> > -    * gen2

[Intel-gfx] [PATCH v8 4/4] drm/i915/display: Split i915 specific code away from intel_fb.c

[Jouni Högander]

We are preparing for Xe driver. Backing object implementation is differing
between i915 and Xe. Split i915 specific code into separate source file
built only for i915.

v8:
  - return original error code from intel_fb_bo_lookup_valid_bo on failure
v7:
  - drop #include 
  - s/user_mode_cmd/mode_cmd/
  - Use passed i915 pointer instead of to_i915(obj->base.dev)
v6: Add missing intel_fb_bo.[ch]
v5:
  - Keep drm_any_plane_has_format check in intel_fb.c
  - Use mode_cmd instead of user_mode_cmd for intel_fb_bo_lookup_valid_bo
v4: Move drm_any_plane_has_format check into intel_fb_bo.c
v3: Fix failure handling in intel_framebuffer_init
v2: Couple of fixes to error value handling

Signed-off-by: Jouni Högander 
---
 drivers/gpu/drm/i915/Makefile  |  1 +
 drivers/gpu/drm/i915/display/intel_fb.c| 74 +++--
 drivers/gpu/drm/i915/display/intel_fb_bo.c | 92 ++
 drivers/gpu/drm/i915/display/intel_fb_bo.h | 24 ++
 4 files changed, 129 insertions(+), 62 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 65e984242089..5060c7b98a56 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -280,6 +280,7 @@ i915-y += \
display/intel_dsb.o \
display/intel_dsb_buffer.o \
display/intel_fb.o \
+   display/intel_fb_bo.o \
display/intel_fb_pin.o \
display/intel_fbc.o \
display/intel_fdi.o \
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c 
b/drivers/gpu/drm/i915/display/intel_fb.c
index 868e39291e9f..853b3bd57461 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -4,7 +4,6 @@
  */
 
 #include 
-#include 
 #include 
 
 #include 
@@ -15,6 +14,7 @@
 #include "intel_display_types.h"
 #include "intel_dpt.h"
 #include "intel_fb.h"
+#include "intel_fb_bo.h"
 #include "intel_frontbuffer.h"
 
 #define check_array_bounds(i915, a, i) drm_WARN_ON(&(i915)->drm, (i) >= 
ARRAY_SIZE(a))
@@ -1985,7 +1985,6 @@ int intel_framebuffer_init(struct intel_framebuffer 
*intel_fb,
struct drm_i915_private *dev_priv = 
to_i915(intel_bo_to_drm_bo(obj)->dev);
struct drm_framebuffer *fb = &intel_fb->base;
u32 max_stride;
-   unsigned int tiling, stride;
int ret = -EINVAL;
int i;
 
@@ -1993,32 +1992,11 @@ int intel_framebuffer_init(struct intel_framebuffer 
*intel_fb,
if (!intel_fb->frontbuffer)
return -ENOMEM;
 
-   i915_gem_object_lock(obj, NULL);
-   tiling = i915_gem_object_get_tiling(obj);
-   stride = i915_gem_object_get_stride(obj);
-   i915_gem_object_unlock(obj);
-
-   if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
-   /*
-* If there's a fence, enforce that
-* the fb modifier and tiling mode match.
-*/
-   if (tiling != I915_TILING_NONE &&
-   tiling != 
intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
-   drm_dbg_kms(&dev_priv->drm,
-   "tiling_mode doesn't match fb modifier\n");
-   goto err;
-   }
-   } else {
-   if (tiling == I915_TILING_X) {
-   mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
-   } else if (tiling == I915_TILING_Y) {
-   drm_dbg_kms(&dev_priv->drm,
-   "No Y tiling for legacy addfb\n");
-   goto err;
-   }
-   }
+   ret = intel_fb_bo_framebuffer_init(intel_fb, obj, mode_cmd);
+   if (ret)
+   goto err;
 
+   ret = -EINVAL;
if (!drm_any_plane_has_format(&dev_priv->drm,
  mode_cmd->pixel_format,
  mode_cmd->modifier[0])) {
@@ -2028,17 +2006,6 @@ int intel_framebuffer_init(struct intel_framebuffer 
*intel_fb,
goto err;
}
 
-   /*
-* gen2/3 display engine uses the fence if present,
-* so the tiling mode must match the fb modifier exactly.
-*/
-   if (DISPLAY_VER(dev_priv) < 4 &&
-   tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
-   drm_dbg_kms(&dev_priv->drm,
-   "tiling_mode must match fb modifier exactly on 
gen2/3\n");
-   goto err;
-   }
-
max_stride = intel_fb_max_stride(dev_priv, mode_cmd->pixel_format,
 mode_cmd->modifier[0]);
if (mode_cmd->pitches[0] > max_stride) {
@@ -2050,17 +2017,6 @@ int intel_framebuffer_init(struct intel_framebuffer 
*intel_fb,
goto err;
}
 
-   /*
-* If there's a fence, enforce that
-* the fb pitch and fence stride match.
-

[Intel-gfx] [PATCH v8 3/4] drm/i915/display: Handle invalid fb_modifier in intel_fb_modifier_to_tiling

[Jouni Högander]

Lookup_modifier is returning INTEL_PLANE_CAP_TILING_4 on invalid
fb_modifier value. Use lookup_modifier_or_null in
intel_fb_modifier_to_tiling and return I915_TILING_NONE in case
lookup_modifier_or_null returns null.

Signed-off-by: Jouni Högander 
Reviewed-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_fb.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fb.c 
b/drivers/gpu/drm/i915/display/intel_fb.c
index 6c89cb2f2002..868e39291e9f 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -303,7 +303,14 @@ lookup_format_info(const struct drm_format_info formats[],
 
 unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
 {
-   u8 tiling_caps = lookup_modifier(fb_modifier)->plane_caps &
+   const struct intel_modifier_desc *md;
+   u8 tiling_caps;
+
+   md = lookup_modifier_or_null(fb_modifier);
+   if (!md)
+   return I915_TILING_NONE;
+
+   tiling_caps = lookup_modifier_or_null(fb_modifier)->plane_caps &
 INTEL_PLANE_CAP_TILING_MASK;
 
switch (tiling_caps) {
-- 
2.34.1

[Intel-gfx] [PATCH v8 2/4] drm/i915/display: Convert intel_fb_modifier_to_tiling as non-static

[Jouni Högander]

We are about to split i915 specific code from intel_fb.c. Convert
intel_fb_modifier_to_tiling as non-static to allow calling it from split
code.

Signed-off-by: Jouni Högander 
Reviewed-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_fb.c | 40 -
 drivers/gpu/drm/i915/display/intel_fb.h |  2 ++
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fb.c 
b/drivers/gpu/drm/i915/display/intel_fb.c
index 4af5b7181fdf..6c89cb2f2002 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -301,6 +301,26 @@ lookup_format_info(const struct drm_format_info formats[],
return NULL;
 }
 
+unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
+{
+   u8 tiling_caps = lookup_modifier(fb_modifier)->plane_caps &
+INTEL_PLANE_CAP_TILING_MASK;
+
+   switch (tiling_caps) {
+   case INTEL_PLANE_CAP_TILING_Y:
+   return I915_TILING_Y;
+   case INTEL_PLANE_CAP_TILING_X:
+   return I915_TILING_X;
+   case INTEL_PLANE_CAP_TILING_4:
+   case INTEL_PLANE_CAP_TILING_Yf:
+   case INTEL_PLANE_CAP_TILING_NONE:
+   return I915_TILING_NONE;
+   default:
+   MISSING_CASE(tiling_caps);
+   return I915_TILING_NONE;
+   }
+}
+
 /**
  * intel_fb_get_format_info: Get a modifier specific format information
  * @cmd: FB add command structure
@@ -737,26 +757,6 @@ intel_fb_align_height(const struct drm_framebuffer *fb,
return ALIGN(height, tile_height);
 }
 
-static unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
-{
-   u8 tiling_caps = lookup_modifier(fb_modifier)->plane_caps &
-INTEL_PLANE_CAP_TILING_MASK;
-
-   switch (tiling_caps) {
-   case INTEL_PLANE_CAP_TILING_Y:
-   return I915_TILING_Y;
-   case INTEL_PLANE_CAP_TILING_X:
-   return I915_TILING_X;
-   case INTEL_PLANE_CAP_TILING_4:
-   case INTEL_PLANE_CAP_TILING_Yf:
-   case INTEL_PLANE_CAP_TILING_NONE:
-   return I915_TILING_NONE;
-   default:
-   MISSING_CASE(tiling_caps);
-   return I915_TILING_NONE;
-   }
-}
-
 bool intel_fb_modifier_uses_dpt(struct drm_i915_private *i915, u64 modifier)
 {
return HAS_DPT(i915) && modifier != DRM_FORMAT_MOD_LINEAR;
diff --git a/drivers/gpu/drm/i915/display/intel_fb.h 
b/drivers/gpu/drm/i915/display/intel_fb.h
index e85167d6bc34..23db6628f53e 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.h
+++ b/drivers/gpu/drm/i915/display/intel_fb.h
@@ -95,4 +95,6 @@ intel_user_framebuffer_create(struct drm_device *dev,
 bool intel_fb_modifier_uses_dpt(struct drm_i915_private *i915, u64 modifier);
 bool intel_fb_uses_dpt(const struct drm_framebuffer *fb);
 
+unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier);
+
 #endif /* __INTEL_FB_H__ */
-- 
2.34.1

[Intel-gfx] [PATCH v8 1/4] drm/i915/display: use intel_bo_to_drm_bo in intel_fb.c

[Jouni Högander]

We are preparing for Xe driver. I915 and Xe object implementation are
differing. Do not use  i915_gem_object->base directly. Instead use
intel_bo_to_drm_bo.

Also use drm_gem_object_put instead of i915_gem_object_put. This should be
ok as i915_gem_object_put is really just doing  __drm_gem_object_put.

Signed-off-by: Jouni Högander 
Reviewed-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_fb.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fb.c 
b/drivers/gpu/drm/i915/display/intel_fb.c
index 6d48aa3af95a..4af5b7181fdf 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -1657,10 +1657,10 @@ int intel_fill_fb_info(struct drm_i915_private *i915, 
struct intel_framebuffer *
max_size = max(max_size, offset + size);
}
 
-   if (mul_u32_u32(max_size, tile_size) > obj->base.size) {
+   if (mul_u32_u32(max_size, tile_size) > intel_bo_to_drm_bo(obj)->size) {
drm_dbg_kms(&i915->drm,
"fb too big for bo (need %llu bytes, have %zu 
bytes)\n",
-   mul_u32_u32(max_size, tile_size), obj->base.size);
+   mul_u32_u32(max_size, tile_size), 
intel_bo_to_drm_bo(obj)->size);
return -EINVAL;
}
 
@@ -1889,7 +1889,7 @@ static int intel_user_framebuffer_create_handle(struct 
drm_framebuffer *fb,
unsigned int *handle)
 {
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-   struct drm_i915_private *i915 = to_i915(obj->base.dev);
+   struct drm_i915_private *i915 = to_i915(intel_bo_to_drm_bo(obj)->dev);
 
if (i915_gem_object_is_userptr(obj)) {
drm_dbg(&i915->drm,
@@ -1897,7 +1897,7 @@ static int intel_user_framebuffer_create_handle(struct 
drm_framebuffer *fb,
return -EINVAL;
}
 
-   return drm_gem_handle_create(file, &obj->base, handle);
+   return drm_gem_handle_create(file, intel_bo_to_drm_bo(obj), handle);
 }
 
 struct frontbuffer_fence_cb {
@@ -1975,7 +1975,7 @@ int intel_framebuffer_init(struct intel_framebuffer 
*intel_fb,
   struct drm_i915_gem_object *obj,
   struct drm_mode_fb_cmd2 *mode_cmd)
 {
-   struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+   struct drm_i915_private *dev_priv = 
to_i915(intel_bo_to_drm_bo(obj)->dev);
struct drm_framebuffer *fb = &intel_fb->base;
u32 max_stride;
unsigned int tiling, stride;
@@ -2153,7 +2153,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
}
 
fb = intel_framebuffer_create(obj, &mode_cmd);
-   i915_gem_object_put(obj);
+   drm_gem_object_put(intel_bo_to_drm_bo(obj));
 
return fb;
 }
-- 
2.34.1

[Intel-gfx] [PATCH v8 0/4] Prepare intel_fb for Xe

[Jouni Högander]

Intel fb creation is differing between Xe and i915 due to different
implementations of backing object. This patch set is splitting i915
specific code into it's own source file. Similar source files will be
introduced for Xe as well.

Also use intel_bo_to_drm_bo instead of directly referring
i915_gem_object->base. One i915_gem_object_put is changed to
drm_gem_object_put.

v8:
  - return original error code from intel_fb_bo_lookup_valid_bo on failure
v7:
  - drop #include 
  - s/user_mode_cmd/mode_cmd/
  - Use passed i915 pointer instead of to_i915(obj->base.dev)
v6: Add missing intel_fb_bo.[ch]
v5:
  - Keep drm_any_plane_has_format check in intel_fb.c
  - Use mode_cmd instead of user_mode_cmd for intel_fb_bo_lookup_valid_bo
  - Use lookup_modifier_or_null in intel_fb_modifier_to_tiling and
handle null value
v4: Move drm_any_plane_has_format check into intel_fb_bo.c
v3: Fix failure handling in intel_framebuffer_init
v2: Couple of fixes to error value handling

Cc: Rodrigo Vivi 
Cc: Maarten Lankhorst 
Cc: Jani Nikula 
Cc: Uma Shankar 

Jouni Högander (4):
  drm/i915/display: use intel_bo_to_drm_bo in intel_fb.c
  drm/i915/display: Convert intel_fb_modifier_to_tiling as non-static
  drm/i915/display: Handle invalid fb_modifier in
intel_fb_modifier_to_tiling
  drm/i915/display: Split i915 specific code away from intel_fb.c

 drivers/gpu/drm/i915/Makefile  |   1 +
 drivers/gpu/drm/i915/display/intel_fb.c| 133 +++--
 drivers/gpu/drm/i915/display/intel_fb.h|   2 +
 drivers/gpu/drm/i915/display/intel_fb_bo.c |  92 ++
 drivers/gpu/drm/i915/display/intel_fb_bo.h |  24 
 5 files changed, 164 insertions(+), 88 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.h

-- 
2.34.1

Re: [Intel-gfx] [Intel-xe] [PATCH v6] drm/i915: handle uncore spinlock when not available

[Rodrigo Vivi]

On Thu, Nov 30, 2023 at 01:54:13PM +, Coelho, Luciano wrote:
> On Thu, 2023-11-30 at 13:24 +, Tvrtko Ursulin wrote:
> > On 30/11/2023 12:26, Coelho, Luciano wrote:
> > > On Thu, 2023-11-30 at 12:21 +, Tvrtko Ursulin wrote:
> > > > On 30/11/2023 11:35, Luca Coelho wrote:
> > > > > The uncore code may not always be available (e.g. when we build the
> > > > > display code with Xe), so we can't always rely on having the uncore's
> > > > > spinlock.
> > > > > 
> > > > > To handle this, split the spin_lock/unlock_irqsave/restore() into
> > > > > spin_lock/unlock() followed by a call to local_irq_save/restore() and
> > > > > create wrapper functions for locking and unlocking the uncore's
> > > > > spinlock.  In these functions, we have a condition check and only
> > > > > actually try to lock/unlock the spinlock when I915 is defined, and
> > > > > thus uncore is available.
> > > > > 
> > > > > This keeps the ifdefs contained in these new functions and all such
> > > > > logic inside the display code.
> > > > > 
> > > > > Cc: Tvrtko Ursulin 
> > > > > Cc: Jani Nikula 
> > > > > Cc: Ville Syrjala 
> > > > > Reviewed-by: Rodrigo Vivi 
> > > > > Signed-off-by: Luca Coelho 
> > > > > ---
> > > > > 
> > > > > 
> > > > > In v2:
> > > > > 
> > > > >  * Renamed uncore_spin_*() to intel_spin_*()
> > > > >  * Corrected the order: save, lock, unlock, restore
> > > > > 
> > > > > In v3:
> > > > > 
> > > > >  * Undid the change to pass drm_i915_private instead of the lock
> > > > >itself, since we would have to include i915_drv.h and that 
> > > > > pulls
> > > > >in a truckload of other includes.
> > > > > 
> > > > > In v4:
> > > > > 
> > > > >  * After a brief attempt to replace this with a different patch,
> > > > >we're back to this one;
> > > > >  * Pass drm_i195_private again, and move the functions to
> > > > >intel_vblank.c, so we don't need to include i915_drv.h in a
> > > > >header file and it's already included in intel_vblank.c;
> > > > > 
> > > > > In v5:
> > > > > 
> > > > >  * Remove stray include in intel_display.h;
> > > > >  * Remove unnecessary inline modifiers in the new functions.
> > > > > 
> > > > > In v6:
> > > > > 
> > > > >  * Just removed the umlauts from Ville's name, because patchwork
> > > > >didn't catch my patch and I suspect it was some UTF-8 
> > > > > confusion.
> > > > > 
> > > > >drivers/gpu/drm/i915/display/intel_vblank.c | 49 
> > > > > -
> > > > >1 file changed, 39 insertions(+), 10 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
> > > > > b/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > > index 2cec2abf9746..221fcd6bf77b 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > > @@ -265,6 +265,30 @@ int intel_crtc_scanline_to_hw(struct intel_crtc 
> > > > > *crtc, int scanline)
> > > > >   return (scanline + vtotal - crtc->scanline_offset) % vtotal;
> > > > >}
> > > > >
> > > > > +/*
> > > > > + * The uncore version of the spin lock functions is used to decide
> > > > > + * whether we need to lock the uncore lock or not.  This is only
> > > > > + * needed in i915, not in Xe.
> > > > > + *
> > > > > + * This lock in i915 is needed because some old platforms (at least
> > > > > + * IVB and possibly HSW as well), which are not supported in Xe, need
> > > > > + * all register accesses to the same cacheline to be serialized,
> > > > > + * otherwise they may hang.
> > > > > + */
> > > > > +static void intel_vblank_section_enter(struct drm_i915_private *i915)
> > > > > +{
> > > > > +#ifdef I915
> > > > > + spin_lock(&i915->uncore.lock);
> > > > > +#endif
> > > > > +}
> > > > > +
> > > > > +static void intel_vblank_section_exit(struct drm_i915_private *i915)
> > > > > +{
> > > > > +#ifdef I915
> > > > > + spin_unlock(&i915->uncore.lock);
> > > > > +#endif
> > > > > +}
> > > > > +
> > > > >static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
> > > > >bool in_vblank_irq,
> > > > >int *vpos, int *hpos,
> > > > > @@ -302,11 +326,12 @@ static bool i915_get_crtc_scanoutpos(struct 
> > > > > drm_crtc *_crtc,
> > > > >   }
> > > > >
> > > > >   /*
> > > > > -  * Lock uncore.lock, as we will do multiple timing critical raw
> > > > > -  * register reads, potentially with preemption disabled, so the
> > > > > -  * following code must not block on uncore.lock.
> > > > > +  * Enter vblank critical section, as we will do multiple
> > > > > +  * timing critical raw register reads, potentially with
> > > > > +  * preemption disabled, so the following code must not block.
> > > > >*/
> > > > > - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > > > > + local_irq_save(irqflags);
> > > > > + intel_vblank_se

Re: [Intel-gfx] [PATCH v6] drm/i915: handle uncore spinlock when not available

[Coelho, Luciano]

On Thu, 2023-11-30 at 13:24 +, Tvrtko Ursulin wrote:
> On 30/11/2023 12:26, Coelho, Luciano wrote:
> > On Thu, 2023-11-30 at 12:21 +, Tvrtko Ursulin wrote:
> > > On 30/11/2023 11:35, Luca Coelho wrote:
> > > > The uncore code may not always be available (e.g. when we build the
> > > > display code with Xe), so we can't always rely on having the uncore's
> > > > spinlock.
> > > > 
> > > > To handle this, split the spin_lock/unlock_irqsave/restore() into
> > > > spin_lock/unlock() followed by a call to local_irq_save/restore() and
> > > > create wrapper functions for locking and unlocking the uncore's
> > > > spinlock.  In these functions, we have a condition check and only
> > > > actually try to lock/unlock the spinlock when I915 is defined, and
> > > > thus uncore is available.
> > > > 
> > > > This keeps the ifdefs contained in these new functions and all such
> > > > logic inside the display code.
> > > > 
> > > > Cc: Tvrtko Ursulin 
> > > > Cc: Jani Nikula 
> > > > Cc: Ville Syrjala 
> > > > Reviewed-by: Rodrigo Vivi 
> > > > Signed-off-by: Luca Coelho 
> > > > ---
> > > > 
> > > > 
> > > > In v2:
> > > > 
> > > >  * Renamed uncore_spin_*() to intel_spin_*()
> > > >  * Corrected the order: save, lock, unlock, restore
> > > > 
> > > > In v3:
> > > > 
> > > >  * Undid the change to pass drm_i915_private instead of the lock
> > > >itself, since we would have to include i915_drv.h and that pulls
> > > >in a truckload of other includes.
> > > > 
> > > > In v4:
> > > > 
> > > >  * After a brief attempt to replace this with a different patch,
> > > >we're back to this one;
> > > >  * Pass drm_i195_private again, and move the functions to
> > > >intel_vblank.c, so we don't need to include i915_drv.h in a
> > > >header file and it's already included in intel_vblank.c;
> > > > 
> > > > In v5:
> > > > 
> > > >  * Remove stray include in intel_display.h;
> > > >  * Remove unnecessary inline modifiers in the new functions.
> > > > 
> > > > In v6:
> > > > 
> > > >  * Just removed the umlauts from Ville's name, because patchwork
> > > >didn't catch my patch and I suspect it was some UTF-8 confusion.
> > > > 
> > > >drivers/gpu/drm/i915/display/intel_vblank.c | 49 
> > > > -
> > > >1 file changed, 39 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
> > > > b/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > index 2cec2abf9746..221fcd6bf77b 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> > > > @@ -265,6 +265,30 @@ int intel_crtc_scanline_to_hw(struct intel_crtc 
> > > > *crtc, int scanline)
> > > > return (scanline + vtotal - crtc->scanline_offset) % vtotal;
> > > >}
> > > >
> > > > +/*
> > > > + * The uncore version of the spin lock functions is used to decide
> > > > + * whether we need to lock the uncore lock or not.  This is only
> > > > + * needed in i915, not in Xe.
> > > > + *
> > > > + * This lock in i915 is needed because some old platforms (at least
> > > > + * IVB and possibly HSW as well), which are not supported in Xe, need
> > > > + * all register accesses to the same cacheline to be serialized,
> > > > + * otherwise they may hang.
> > > > + */
> > > > +static void intel_vblank_section_enter(struct drm_i915_private *i915)
> > > > +{
> > > > +#ifdef I915
> > > > +   spin_lock(&i915->uncore.lock);
> > > > +#endif
> > > > +}
> > > > +
> > > > +static void intel_vblank_section_exit(struct drm_i915_private *i915)
> > > > +{
> > > > +#ifdef I915
> > > > +   spin_unlock(&i915->uncore.lock);
> > > > +#endif
> > > > +}
> > > > +
> > > >static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
> > > >  bool in_vblank_irq,
> > > >  int *vpos, int *hpos,
> > > > @@ -302,11 +326,12 @@ static bool i915_get_crtc_scanoutpos(struct 
> > > > drm_crtc *_crtc,
> > > > }
> > > >
> > > > /*
> > > > -* Lock uncore.lock, as we will do multiple timing critical raw
> > > > -* register reads, potentially with preemption disabled, so the
> > > > -* following code must not block on uncore.lock.
> > > > +* Enter vblank critical section, as we will do multiple
> > > > +* timing critical raw register reads, potentially with
> > > > +* preemption disabled, so the following code must not block.
> > > >  */
> > > > -   spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > > > +   local_irq_save(irqflags);
> > > > +   intel_vblank_section_enter(dev_priv);
> > > 
> > > Shouldn't local_irq_save go into intel_vblank_section_enter()? It seems
> > > all callers from both i915 and xe end up doing that anyway and naming
> > > "vblank_start" was presumed there would be more to the section than
> > > cacheline m

Re: [Intel-gfx] [PATCH v6] drm/i915: handle uncore spinlock when not available

[Tvrtko Ursulin]

On 30/11/2023 12:26, Coelho, Luciano wrote:

On Thu, 2023-11-30 at 12:21 +, Tvrtko Ursulin wrote:

On 30/11/2023 11:35, Luca Coelho wrote:

The uncore code may not always be available (e.g. when we build the
display code with Xe), so we can't always rely on having the uncore's
spinlock.

To handle this, split the spin_lock/unlock_irqsave/restore() into
spin_lock/unlock() followed by a call to local_irq_save/restore() and
create wrapper functions for locking and unlocking the uncore's
spinlock.  In these functions, we have a condition check and only
actually try to lock/unlock the spinlock when I915 is defined, and
thus uncore is available.

This keeps the ifdefs contained in these new functions and all such
logic inside the display code.

Cc: Tvrtko Ursulin 
Cc: Jani Nikula 
Cc: Ville Syrjala 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: Luca Coelho 
---


In v2:

 * Renamed uncore_spin_*() to intel_spin_*()
 * Corrected the order: save, lock, unlock, restore

In v3:

 * Undid the change to pass drm_i915_private instead of the lock
   itself, since we would have to include i915_drv.h and that pulls
   in a truckload of other includes.

In v4:

 * After a brief attempt to replace this with a different patch,
   we're back to this one;
 * Pass drm_i195_private again, and move the functions to
   intel_vblank.c, so we don't need to include i915_drv.h in a
   header file and it's already included in intel_vblank.c;

In v5:

 * Remove stray include in intel_display.h;
 * Remove unnecessary inline modifiers in the new functions.

In v6:

 * Just removed the umlauts from Ville's name, because patchwork
   didn't catch my patch and I suspect it was some UTF-8 confusion.

   drivers/gpu/drm/i915/display/intel_vblank.c | 49 -
   1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
b/drivers/gpu/drm/i915/display/intel_vblank.c
index 2cec2abf9746..221fcd6bf77b 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -265,6 +265,30 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int 
scanline)
return (scanline + vtotal - crtc->scanline_offset) % vtotal;
   }
   
+/*

+ * The uncore version of the spin lock functions is used to decide
+ * whether we need to lock the uncore lock or not.  This is only
+ * needed in i915, not in Xe.
+ *
+ * This lock in i915 is needed because some old platforms (at least
+ * IVB and possibly HSW as well), which are not supported in Xe, need
+ * all register accesses to the same cacheline to be serialized,
+ * otherwise they may hang.
+ */
+static void intel_vblank_section_enter(struct drm_i915_private *i915)
+{
+#ifdef I915
+   spin_lock(&i915->uncore.lock);
+#endif
+}
+
+static void intel_vblank_section_exit(struct drm_i915_private *i915)
+{
+#ifdef I915
+   spin_unlock(&i915->uncore.lock);
+#endif
+}
+
   static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
 bool in_vblank_irq,
 int *vpos, int *hpos,
@@ -302,11 +326,12 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc 
*_crtc,
}
   
   	/*

-* Lock uncore.lock, as we will do multiple timing critical raw
-* register reads, potentially with preemption disabled, so the
-* following code must not block on uncore.lock.
+* Enter vblank critical section, as we will do multiple
+* timing critical raw register reads, potentially with
+* preemption disabled, so the following code must not block.
 */
-   spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+   local_irq_save(irqflags);
+   intel_vblank_section_enter(dev_priv);


Shouldn't local_irq_save go into intel_vblank_section_enter()? It seems
all callers from both i915 and xe end up doing that anyway and naming
"vblank_start" was presumed there would be more to the section than
cacheline mmio bug. I mean that there is some benefit from keeping the
readout timings tight.



The reason is that there is one caller that has already disabled
interrupts when this function is called (see below), so we shouldn't do
it again.


Yeah I saw that but with irqsave/restore it is safe to nest. So for me 
it is more a fundamental question which I raise above.


Regards,

Tvrtko



   
   	/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
   
@@ -374,7 +399,8 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
   
   	/* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
   
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);

+   intel_vblank_section_exit(dev_priv);
+   local_irq_restore(irqflags);
   
   	/*

 * While in vblank, position will be negative
@@ -412,9 +438,13 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
unsigned long irqflags;

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[Christian König]

Am 30.11.23 um 08:22 schrieb zhuweixi:

Add @Oak to the KFD discussion. I will reply separately elaborating your 
questions on GMEM's difference from HMM/MMU notifiers.

Christian, thanks for pointing me to that AMDKFD discussion. I have read the 
discussion around the AMDKFD skeleton patch and found the previous discussion 
in the following URLs:
https://lore.kernel.org/dri-devel/1405028848-5660-1-git-send-email-oded.gab...@amd.com/#r
https://lore.kernel.org/dri-devel/20140711154231.gb1...@gmail.com/

I believe AMDKFD's original patch was rejected mostly because of inserting 
vendor-specific stuff to the generic core MM.  Jérôme has clearly stated this 
issue in the second URL. If the code is vendor-specific then it has no place in 
core MM, period.

But why does that vendor-specific solution relate to a generalized solution 
like GMEM? The initial AMDKFD patch doesn't work for Nvidia or Intel.


KFD was meant to be a vendor agnostic framework, very similar to what 
you propose here.


It's just that it was seen as vendor specific because nobody else 
actually wanted to design the their drivers this way.




In fact I think the rejection of the initial AMDKFD patch supports GMEM's idea 
-- there could have been a simpler AMDKFD implementation if the core MM was 
extended by GMEM. Also, after 9 years, there are so many other companies 
building their accelerators over the past few years, especially now the 
GPT-family has made a much bigger success. Don't we want to advance Linux's 
core MM for more friendly and generalized support for the upcoming new vendors?


Well exactly that's the big point: Absolutely not!

We really should never ever encourage people to bind their device 
address space to the CPU address space. This is a very special use case 
and limits the driver design to only this use case.


We have exercised this approach to a rather extreme degree with KFD and 
I can clearly say that doing this was a really big mistake.


As far as I can see you are about to repeat that mistake and even 
encourage others to do so as well.



Now answering Christian's design concerns:

1. "There are cases that do not want to share CPU address space"
Maybe, but I am not fully convinced. The current case we can find is when a NIC 
utilizes IOMMU for security. For this case, GMEM implemented a generalized VMA 
support and tested it with NICs using both Intel-IOMMU/Arm-SMMU. This cut 600 
LoC of IOVA management code from the IOMMU driver, but it is still not included 
in this RFC patch -- I cannot find other cases demanding this isolation. The 
isolation is also unnecessary -- the NIC can enable the IOMMU SVM feature to 
share the CPU address space. As of KVM, it is essentially a host process that 
utilizes two different MMUs within the same address space, so it fits GMEM's 
design...


Maybe I don't completely follow here how you want to save LoC for the 
IOMMU implementation of NICs, but at least for the PASID/PRI support AMD 
just recently gone exactly the opposite direction:


commit 5a0b11a180a9b82b4437a4be1cf73530053f139b
Author: Vasant Hegde 
Date:   Fri Oct 6 09:57:02 2023 +

    iommu/amd: Remove iommu_v2 module

    AMD GPU driver which was the only in-kernel user of iommu_v2 module
    removed dependency on iommu_v2 module.

    Also we are working on adding SVA support in AMD IOMMU driver. Device
    drivers are expected to use common SVA framework to enable device
    PASID/PRI features.

    Removing iommu_v2 module and then adding SVA simplifies the 
development.

    Hence remove iommu_v2 module.

As I wrote before this IOMMU V2 driver was basically binding the CPU 
address space to IOMMU devices using the PASID. For an example see 
function amd_iommu_bind_pasid().


This turned out to be not as useful as we hoped it would be. Essentially 
the use case where you want to give a device access to the whole address 
space of a process are extremely limited. That's why we are removing it 
and switching over to a separate SVA implementation which doesn't depend 
on the CPU address space.



But the virtualization use case I mentioned is completely independent of 
IOMMU. In KVM/XEN/etc.. there is a functionality called native context, 
basically what this means is that instead of passing through complete 
device isolated by IOMMU only specific kernel functionalities are 
exposed to the guest operating system through QEMU.


See here for an example how OpenGL is implemented on top of this: 
https://docs.mesa3d.org/drivers/virgl.html


This is actually using the separation between device memory management 
and CPU memory management and is basically a killer argument why those 
two topics should be separated. Otherwise it's impossible for QEMU to 
actually handle multiple independent device memory address spaces inside 
a single CPU memory address space.



2. "This does not integrate well with the filesystem layer in Linux..."
To be honest, not using a logical page table for anonymous memory is why Li

Re: [Intel-gfx] [PATCH v6] drm/i915: handle uncore spinlock when not available

[Coelho, Luciano]

On Thu, 2023-11-30 at 12:21 +, Tvrtko Ursulin wrote:
> On 30/11/2023 11:35, Luca Coelho wrote:
> > The uncore code may not always be available (e.g. when we build the
> > display code with Xe), so we can't always rely on having the uncore's
> > spinlock.
> > 
> > To handle this, split the spin_lock/unlock_irqsave/restore() into
> > spin_lock/unlock() followed by a call to local_irq_save/restore() and
> > create wrapper functions for locking and unlocking the uncore's
> > spinlock.  In these functions, we have a condition check and only
> > actually try to lock/unlock the spinlock when I915 is defined, and
> > thus uncore is available.
> > 
> > This keeps the ifdefs contained in these new functions and all such
> > logic inside the display code.
> > 
> > Cc: Tvrtko Ursulin 
> > Cc: Jani Nikula 
> > Cc: Ville Syrjala 
> > Reviewed-by: Rodrigo Vivi 
> > Signed-off-by: Luca Coelho 
> > ---
> > 
> > 
> > In v2:
> > 
> > * Renamed uncore_spin_*() to intel_spin_*()
> > * Corrected the order: save, lock, unlock, restore
> > 
> > In v3:
> > 
> > * Undid the change to pass drm_i915_private instead of the lock
> >   itself, since we would have to include i915_drv.h and that pulls
> >   in a truckload of other includes.
> > 
> > In v4:
> > 
> > * After a brief attempt to replace this with a different patch,
> >   we're back to this one;
> > * Pass drm_i195_private again, and move the functions to
> >   intel_vblank.c, so we don't need to include i915_drv.h in a
> >   header file and it's already included in intel_vblank.c;
> > 
> > In v5:
> > 
> > * Remove stray include in intel_display.h;
> > * Remove unnecessary inline modifiers in the new functions.
> > 
> > In v6:
> > 
> > * Just removed the umlauts from Ville's name, because patchwork
> >   didn't catch my patch and I suspect it was some UTF-8 confusion.
> > 
> >   drivers/gpu/drm/i915/display/intel_vblank.c | 49 -
> >   1 file changed, 39 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
> > b/drivers/gpu/drm/i915/display/intel_vblank.c
> > index 2cec2abf9746..221fcd6bf77b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> > @@ -265,6 +265,30 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, 
> > int scanline)
> > return (scanline + vtotal - crtc->scanline_offset) % vtotal;
> >   }
> >   
> > +/*
> > + * The uncore version of the spin lock functions is used to decide
> > + * whether we need to lock the uncore lock or not.  This is only
> > + * needed in i915, not in Xe.
> > + *
> > + * This lock in i915 is needed because some old platforms (at least
> > + * IVB and possibly HSW as well), which are not supported in Xe, need
> > + * all register accesses to the same cacheline to be serialized,
> > + * otherwise they may hang.
> > + */
> > +static void intel_vblank_section_enter(struct drm_i915_private *i915)
> > +{
> > +#ifdef I915
> > +   spin_lock(&i915->uncore.lock);
> > +#endif
> > +}
> > +
> > +static void intel_vblank_section_exit(struct drm_i915_private *i915)
> > +{
> > +#ifdef I915
> > +   spin_unlock(&i915->uncore.lock);
> > +#endif
> > +}
> > +
> >   static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
> >  bool in_vblank_irq,
> >  int *vpos, int *hpos,
> > @@ -302,11 +326,12 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc 
> > *_crtc,
> > }
> >   
> > /*
> > -* Lock uncore.lock, as we will do multiple timing critical raw
> > -* register reads, potentially with preemption disabled, so the
> > -* following code must not block on uncore.lock.
> > +* Enter vblank critical section, as we will do multiple
> > +* timing critical raw register reads, potentially with
> > +* preemption disabled, so the following code must not block.
> >  */
> > -   spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > +   local_irq_save(irqflags);
> > +   intel_vblank_section_enter(dev_priv);
> 
> Shouldn't local_irq_save go into intel_vblank_section_enter()? It seems 
> all callers from both i915 and xe end up doing that anyway and naming 
> "vblank_start" was presumed there would be more to the section than 
> cacheline mmio bug. I mean that there is some benefit from keeping the 
> readout timings tight.
> 

The reason is that there is one caller that has already disabled
interrupts when this function is called (see below), so we shouldn't do
it again. 


> >   
> > /* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
> >   
> > @@ -374,7 +399,8 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc 
> > *_crtc,
> >   
> > /* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
> >   
> > -   spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > +   intel_vblank_section_exit(dev_priv);
> > +   local_irq_res

Re: [Intel-gfx] [PATCH v6] drm/i915: handle uncore spinlock when not available

[Tvrtko Ursulin]

On 30/11/2023 11:35, Luca Coelho wrote:

The uncore code may not always be available (e.g. when we build the
display code with Xe), so we can't always rely on having the uncore's
spinlock.

To handle this, split the spin_lock/unlock_irqsave/restore() into
spin_lock/unlock() followed by a call to local_irq_save/restore() and
create wrapper functions for locking and unlocking the uncore's
spinlock.  In these functions, we have a condition check and only
actually try to lock/unlock the spinlock when I915 is defined, and
thus uncore is available.

This keeps the ifdefs contained in these new functions and all such
logic inside the display code.

Cc: Tvrtko Ursulin 
Cc: Jani Nikula 
Cc: Ville Syrjala 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: Luca Coelho 
---


In v2:

* Renamed uncore_spin_*() to intel_spin_*()
* Corrected the order: save, lock, unlock, restore

In v3:

* Undid the change to pass drm_i915_private instead of the lock
  itself, since we would have to include i915_drv.h and that pulls
  in a truckload of other includes.

In v4:

* After a brief attempt to replace this with a different patch,
  we're back to this one;
* Pass drm_i195_private again, and move the functions to
  intel_vblank.c, so we don't need to include i915_drv.h in a
  header file and it's already included in intel_vblank.c;

In v5:

* Remove stray include in intel_display.h;
* Remove unnecessary inline modifiers in the new functions.

In v6:

* Just removed the umlauts from Ville's name, because patchwork
  didn't catch my patch and I suspect it was some UTF-8 confusion.

  drivers/gpu/drm/i915/display/intel_vblank.c | 49 -
  1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
b/drivers/gpu/drm/i915/display/intel_vblank.c
index 2cec2abf9746..221fcd6bf77b 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -265,6 +265,30 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int 
scanline)
return (scanline + vtotal - crtc->scanline_offset) % vtotal;
  }
  
+/*

+ * The uncore version of the spin lock functions is used to decide
+ * whether we need to lock the uncore lock or not.  This is only
+ * needed in i915, not in Xe.
+ *
+ * This lock in i915 is needed because some old platforms (at least
+ * IVB and possibly HSW as well), which are not supported in Xe, need
+ * all register accesses to the same cacheline to be serialized,
+ * otherwise they may hang.
+ */
+static void intel_vblank_section_enter(struct drm_i915_private *i915)
+{
+#ifdef I915
+   spin_lock(&i915->uncore.lock);
+#endif
+}
+
+static void intel_vblank_section_exit(struct drm_i915_private *i915)
+{
+#ifdef I915
+   spin_unlock(&i915->uncore.lock);
+#endif
+}
+
  static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
 bool in_vblank_irq,
 int *vpos, int *hpos,
@@ -302,11 +326,12 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc 
*_crtc,
}
  
  	/*

-* Lock uncore.lock, as we will do multiple timing critical raw
-* register reads, potentially with preemption disabled, so the
-* following code must not block on uncore.lock.
+* Enter vblank critical section, as we will do multiple
+* timing critical raw register reads, potentially with
+* preemption disabled, so the following code must not block.
 */
-   spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+   local_irq_save(irqflags);
+   intel_vblank_section_enter(dev_priv);


Shouldn't local_irq_save go into intel_vblank_section_enter()? It seems 
all callers from both i915 and xe end up doing that anyway and naming 
"vblank_start" was presumed there would be more to the section than 
cacheline mmio bug. I mean that there is some benefit from keeping the 
readout timings tight.


Regards,

Tvrtko

  
  	/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
  
@@ -374,7 +399,8 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
  
  	/* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
  
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);

+   intel_vblank_section_exit(dev_priv);
+   local_irq_restore(irqflags);
  
  	/*

 * While in vblank, position will be negative
@@ -412,9 +438,13 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
unsigned long irqflags;
int position;
  
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);

+   local_irq_save(irqflags);
+   intel_vblank_section_enter(dev_priv);
+
position = __intel_get_crtc_scanline(crtc);
-   spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
+   intel_vblank_section_exit(dev_priv);
+   local_irq_restore(irqflags);
  
  	return position;

  }
@@ -537,7 +567,7 @@ 

[Intel-gfx] [PATCH v6] drm/i915: handle uncore spinlock when not available

[Luca Coelho]

The uncore code may not always be available (e.g. when we build the
display code with Xe), so we can't always rely on having the uncore's
spinlock.

To handle this, split the spin_lock/unlock_irqsave/restore() into
spin_lock/unlock() followed by a call to local_irq_save/restore() and
create wrapper functions for locking and unlocking the uncore's
spinlock.  In these functions, we have a condition check and only
actually try to lock/unlock the spinlock when I915 is defined, and
thus uncore is available.

This keeps the ifdefs contained in these new functions and all such
logic inside the display code.

Cc: Tvrtko Ursulin 
Cc: Jani Nikula 
Cc: Ville Syrjala 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: Luca Coelho 
---


In v2:

   * Renamed uncore_spin_*() to intel_spin_*()
   * Corrected the order: save, lock, unlock, restore

In v3:

   * Undid the change to pass drm_i915_private instead of the lock
 itself, since we would have to include i915_drv.h and that pulls
 in a truckload of other includes.

In v4:

   * After a brief attempt to replace this with a different patch,
 we're back to this one;
   * Pass drm_i195_private again, and move the functions to
 intel_vblank.c, so we don't need to include i915_drv.h in a
 header file and it's already included in intel_vblank.c;

In v5:

   * Remove stray include in intel_display.h;
   * Remove unnecessary inline modifiers in the new functions.

In v6:

   * Just removed the umlauts from Ville's name, because patchwork
 didn't catch my patch and I suspect it was some UTF-8 confusion.

 drivers/gpu/drm/i915/display/intel_vblank.c | 49 -
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c 
b/drivers/gpu/drm/i915/display/intel_vblank.c
index 2cec2abf9746..221fcd6bf77b 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -265,6 +265,30 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int 
scanline)
return (scanline + vtotal - crtc->scanline_offset) % vtotal;
 }
 
+/*
+ * The uncore version of the spin lock functions is used to decide
+ * whether we need to lock the uncore lock or not.  This is only
+ * needed in i915, not in Xe.
+ *
+ * This lock in i915 is needed because some old platforms (at least
+ * IVB and possibly HSW as well), which are not supported in Xe, need
+ * all register accesses to the same cacheline to be serialized,
+ * otherwise they may hang.
+ */
+static void intel_vblank_section_enter(struct drm_i915_private *i915)
+{
+#ifdef I915
+   spin_lock(&i915->uncore.lock);
+#endif
+}
+
+static void intel_vblank_section_exit(struct drm_i915_private *i915)
+{
+#ifdef I915
+   spin_unlock(&i915->uncore.lock);
+#endif
+}
+
 static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
 bool in_vblank_irq,
 int *vpos, int *hpos,
@@ -302,11 +326,12 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc 
*_crtc,
}
 
/*
-* Lock uncore.lock, as we will do multiple timing critical raw
-* register reads, potentially with preemption disabled, so the
-* following code must not block on uncore.lock.
+* Enter vblank critical section, as we will do multiple
+* timing critical raw register reads, potentially with
+* preemption disabled, so the following code must not block.
 */
-   spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+   local_irq_save(irqflags);
+   intel_vblank_section_enter(dev_priv);
 
/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
 
@@ -374,7 +399,8 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
 
/* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
 
-   spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+   intel_vblank_section_exit(dev_priv);
+   local_irq_restore(irqflags);
 
/*
 * While in vblank, position will be negative
@@ -412,9 +438,13 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
unsigned long irqflags;
int position;
 
-   spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+   local_irq_save(irqflags);
+   intel_vblank_section_enter(dev_priv);
+
position = __intel_get_crtc_scanline(crtc);
-   spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
+   intel_vblank_section_exit(dev_priv);
+   local_irq_restore(irqflags);
 
return position;
 }
@@ -537,7 +567,7 @@ void intel_crtc_update_active_timings(const struct 
intel_crtc_state *crtc_state,
 * Need to audit everything to make sure it's safe.
 */
spin_lock_irqsave(&i915->drm.vblank_time_lock, irqflags);
-   spin_lock(&i915->uncore.lock);
+   intel_vblank_section_enter(i915);
 
drm_calc_timestamping_constants(&crtc->base, &adjusted_mode)

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[zhuweixi]

Glad to know that there is a common demand for a new syscall like hmadvise(). I 
expect it would also be useful for homogeneous NUMA cases. Credits to 
cudaMemAdvise() API which brought this idea to GMEM's design.

To answer @Oak's questions about GMEM vs. HMM,

Here is the major difference:
  GMEM's main target is to stop drivers from reinventing MM code, while HMM/MMU 
notifiers provide a compatible struct page solution and a coordination 
mechanism for existing device driver MMs that requires adding extra code to 
interact with CPU MM.

A straightforward qualitative result for the main target: after integrating 
Huawei's Ascend NPU driver with GMEM's interface, 30,000 lines of MM code were 
cut, leaving <100 lines invoking GMEM interface and 3,700 lines implementing 
vendor-specific functions. Some code from the 3,700 lines should be further 
moved to GMEM as a generalized feature like device memory oversubscription, but 
not included in this RFC patch yet. 

A list of high-level differences: 
  1. With HMM/MMU notifiers, drivers need to first implement a full MM 
subsystem. With GMEM, drivers can reuse Linux's core MM.

  2. HMM encodes device mapping information in the CPU arch-dependent PTEs, 
while GMEM proposes an abstraction layer in vm_object. Since GMEM's approach 
further decouples the arch-related stuff, drivers do not need to implement 
separate code for X86/ARM and etc.

  3. MMU notifiers register hooks at certain core MM events, while GMEM 
declares basic functions and internally invokes them. GMEM requires less from 
the driver side -- no need to understand what core MM behaves at certain MMU 
events. GMEM also expects fewer bugs than MMU notifiers: implementing basic 
operations with standard declarations vs. implementing whatever random device 
MM logic in MMU notifiers.

  4. GMEM plans to support a more lightweight physical memory management. The 
discussion about this part can be found in my cover letter. The question is 
whether struct page should be compatible (directly use HMM's ZONE_DEVICE 
solution) or a trimmed, smaller struct page that satisfies generalized demands 
from accelerators is more preferrable?

  5. GMEM has been demonstrated to allow device memory oversubscription (a 
GMEM-based 32GB NPU card can run a GPT model oversubscribing 500GB host DDR), 
while drivers using HMM/MMU notifier must implement this logic one by one. I 
will submit this part in a future RFC patch.

I want to reiterate that GMEM's shared address space support is a bonus result, 
not a main contribution... It was done because it was not difficult to 
implement internal CPU-device coordination mechanism when core MM is extended 
by GMEM to support devices.

-Weixi

-Original Message-
From: Christian König  
Sent: Thursday, November 30, 2023 4:28 PM
To: Zeng, Oak ; Christian König ; 
zhuweixi ; linux...@kvack.org; 
linux-ker...@vger.kernel.org; a...@linux-foundation.org; Danilo Krummrich 
; Dave Airlie ; Daniel Vetter 

Cc: intel-gvt-...@lists.freedesktop.org; rcampb...@nvidia.com; 
mhairgr...@nvidia.com; j...@nvidia.com; weixi@openeuler.sh; 
jhubb...@nvidia.com; intel-gfx@lists.freedesktop.org; apop...@nvidia.com; 
xinhui@amd.com; amd-...@lists.freedesktop.org; 
tvrtko.ursu...@linux.intel.com; ogab...@kernel.org; jgli...@redhat.com; 
dri-de...@lists.freedesktop.org; z...@nvidia.com; Vivi, Rodrigo 
; alexander.deuc...@amd.com; leo...@nvidia.com; 
felix.kuehl...@amd.com; Wang, Zhi A ; mgor...@suse.de
Subject: Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory management) 
for external memory devices

Hi Oak,

yeah, #4 is indeed a really good point and I think Felix will agree to that as 
well.

HMM is basically still missing a way to advise device attributes for the CPU 
address space. Both migration strategy as well as device specific information 
(like cache preferences) fall into this category.

Since there is a device specific component in those attributes as well I think 
device specific IOCTLs still make sense to update them, but HMM should offer 
the functionality to manage and store those information.

Split and merge of VMAs only become a problem if you attach those information 
to VMAs, if you keep them completely separate than that doesn't become an issue 
either. The down side of this approach is that you don't get automatically 
extending attribute ranges for growing VMAs for example.

Regards,
Christian.

Am 29.11.23 um 23:23 schrieb Zeng, Oak:
> Hi Weixi,
>
> Even though Christian has listed reasons rejecting this proposal (yes they 
> are very reasonable to me), I would open my mind and further explore the 
> possibility here. Since the current GPU driver uses a hmm based 
> implementation (AMD and NV has done this; At Intel we are catching up), I 
> want to explore how much we can benefit from the proposed approach and how 
> your approach can solve some pain points of our development. So basically 
> what I am questioning here is: what is the advantage of your ap

Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Javier Martinez Canillas]

Maxime Ripard  writes:

> On Thu, Nov 30, 2023 at 11:46:00AM +0200, Jani Nikula wrote:

[...]

>> 
>> Then we'll have a ping pong of people not using W=1 or
>> CONFIG_DRM_EXTRA_CHECKS introducing warnings, and people using them
>> fixing the warnings...
>> 
>> I really do think making it unconditional is the only way.
>
> Yeah, I agree.
>
> Plus, if we need to have an extra Kconfig option, it's pretty equivalent
> to using W=1
>

Yeah, with the difference that having a Kconfig symbol allows for example
distros to set it in their kernel configs.

It's a fair point though that probably the only option is to enable it
unconditionally.

Acked-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Thomas Zimmermann]

Am 29.11.23 um 19:12 schrieb Jani Nikula:

At least the i915 and amd drivers enable a bunch more compiler warnings
than the kernel defaults.

Extend the W=1 warnings to the entire drm subsystem by default. Use the
copy-pasted warnings from scripts/Makefile.extrawarn with
s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
up with them in the future.

This is similar to the approach currently used in i915.

Some of the -Wextra warnings do need to be disabled, just like in
Makefile.extrawarn, but take care to not disable them for W=2 or W=3
builds, depending on the warning.

Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: Alex Deucher 
Cc: Christian König 
Cc: Pan, Xinhui 
Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Danilo Krummrich 
Cc: Rob Clark 
Cc: Abhinav Kumar 
Cc: Dmitry Baryshkov 
Cc: Sean Paul 
Cc: Marijn Suijten 
Signed-off-by: Jani Nikula 


Acked-by: Thomas Zimmermann 



---

With my admittedly limited and very much x86 focused kernel config, I
get some -Wunused-but-set-variable and -Wformat-truncation= warnings,
but nothing we can't handle.

We could fix them up front, or disable the extra warnings on a per
driver basis with a FIXME comment in their respective Makefiles.

With the experience from i915, I think this would significantly reduce
the constant loop of warnings added by people not using W=1 and
subsequently fixed by people using W=1.

Note: I've Cc'd the maintainers of drm, drm misc and some of the biggest
drivers.
---
  drivers/gpu/drm/Makefile | 27 +++
  1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index b4cb0835620a..6939e4ea13d5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -5,6 +5,33 @@
  
  CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
  
+# Unconditionally enable W=1 warnings locally

+# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
+subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
+subdir-ccflags-y += -Wmissing-declarations
+subdir-ccflags-y += $(call cc-option, -Wrestrict)
+subdir-ccflags-y += -Wmissing-format-attribute
+subdir-ccflags-y += -Wmissing-prototypes
+subdir-ccflags-y += -Wold-style-definition
+subdir-ccflags-y += -Wmissing-include-dirs
+subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
+subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
+subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
+subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
+subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
+subdir-ccflags-y += $(call cc-option, -Wstringop-overflow)
+subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
+# The following turn off the warnings enabled by -Wextra
+ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-shift-negative-value
+endif
+ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
+subdir-ccflags-y += -Wno-sign-compare
+endif
+# --- end copy-paste
+
  drm-y := \
drm_aperture.o \
drm_atomic.o \


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Maxime Ripard]

On Thu, Nov 30, 2023 at 11:46:00AM +0200, Jani Nikula wrote:
> On Thu, 30 Nov 2023, Javier Martinez Canillas  wrote:
> > Maxime Ripard  writes:
> >
> >> Hi,
> >>
> >> On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote:
> >>> On Wed, 29 Nov 2023, Hamza Mahfooz  wrote:
> >>> > Cc: Nathan Chancellor 
> >>> >
> >>> > On 11/29/23 13:12, Jani Nikula wrote:
> >>> >> At least the i915 and amd drivers enable a bunch more compiler warnings
> >>> >> than the kernel defaults.
> >>> >> 
> >>> >> Extend the W=1 warnings to the entire drm subsystem by default. Use the
> >>> >> copy-pasted warnings from scripts/Makefile.extrawarn with
> >>> >> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
> >>> >> up with them in the future.
> >>> >> 
> >>> >> This is similar to the approach currently used in i915.
> >>> >> 
> >>> >> Some of the -Wextra warnings do need to be disabled, just like in
> >>> >> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
> >>> >> builds, depending on the warning.
> >>> >
> >>> > I think this should go in after drm-misc-next has a clean build (for
> >>> > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a
> >>> > lot of build configs.
> >>> 
> >>> Oh, I'm absolutely not suggesting this should be merged before known
> >>> warnings have been addressed one way or another. Either by fixing them
> >>> or by disabling said warning in driver local Makefiles, depending on the
> >>> case.
> >>
> >> I'm all for it, but yeah, we need some easy way to opt-in/opt-out. Some
> >> drivers are pretty much unmaintained now and are likely to never fix
> >> those warnings.
> 
> Then I'd go for enabling in drm level and disabling individual warnings
> in the driver Makefile level if they won't get fixed.
> 
> > Maybe add a Kconfig symbol for it instead of making unconditional?
> >
> > Something like:
> >
> > +# Unconditionally enable W=1 warnings locally
> > +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
> > +subdir-ccflags-$(CONFIG_DRM_EXTRA_CHECKS)  += -Wextra -Wunused 
> > -Wno-unused-parameter
> > ...
> 
> Then we'll have a ping pong of people not using W=1 or
> CONFIG_DRM_EXTRA_CHECKS introducing warnings, and people using them
> fixing the warnings...
> 
> I really do think making it unconditional is the only way.

Yeah, I agree.

Plus, if we need to have an extra Kconfig option, it's pretty equivalent
to using W=1

Maxime


signature.asc
Description: PGP signature

Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Javier Martinez Canillas]

Jani Nikula  writes:

[...]

>
> Then I'd go for enabling in drm level and disabling individual warnings
> in the driver Makefile level if they won't get fixed.
>
>> Maybe add a Kconfig symbol for it instead of making unconditional?
>>
>> Something like:
>>
>> +# Unconditionally enable W=1 warnings locally
>> +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
>> +subdir-ccflags-$(CONFIG_DRM_EXTRA_CHECKS)  += -Wextra -Wunused 
>> -Wno-unused-parameter
>> ...
>
> Then we'll have a ping pong of people not using W=1 or
> CONFIG_DRM_EXTRA_CHECKS introducing warnings, and people using them
> fixing the warnings...
>
> I really do think making it unconditional is the only way.
>

Fair enough.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Jani Nikula]

On Thu, 30 Nov 2023, Javier Martinez Canillas  wrote:
> Maxime Ripard  writes:
>
>> Hi,
>>
>> On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote:
>>> On Wed, 29 Nov 2023, Hamza Mahfooz  wrote:
>>> > Cc: Nathan Chancellor 
>>> >
>>> > On 11/29/23 13:12, Jani Nikula wrote:
>>> >> At least the i915 and amd drivers enable a bunch more compiler warnings
>>> >> than the kernel defaults.
>>> >> 
>>> >> Extend the W=1 warnings to the entire drm subsystem by default. Use the
>>> >> copy-pasted warnings from scripts/Makefile.extrawarn with
>>> >> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
>>> >> up with them in the future.
>>> >> 
>>> >> This is similar to the approach currently used in i915.
>>> >> 
>>> >> Some of the -Wextra warnings do need to be disabled, just like in
>>> >> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
>>> >> builds, depending on the warning.
>>> >
>>> > I think this should go in after drm-misc-next has a clean build (for
>>> > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a
>>> > lot of build configs.
>>> 
>>> Oh, I'm absolutely not suggesting this should be merged before known
>>> warnings have been addressed one way or another. Either by fixing them
>>> or by disabling said warning in driver local Makefiles, depending on the
>>> case.
>>
>> I'm all for it, but yeah, we need some easy way to opt-in/opt-out. Some
>> drivers are pretty much unmaintained now and are likely to never fix
>> those warnings.

Then I'd go for enabling in drm level and disabling individual warnings
in the driver Makefile level if they won't get fixed.

> Maybe add a Kconfig symbol for it instead of making unconditional?
>
> Something like:
>
> +# Unconditionally enable W=1 warnings locally
> +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
> +subdir-ccflags-$(CONFIG_DRM_EXTRA_CHECKS)  += -Wextra -Wunused 
> -Wno-unused-parameter
> ...

Then we'll have a ping pong of people not using W=1 or
CONFIG_DRM_EXTRA_CHECKS introducing warnings, and people using them
fixing the warnings...

I really do think making it unconditional is the only way.


BR,
Jani.


-- 
Jani Nikula, Intel

Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Javier Martinez Canillas]

Maxime Ripard  writes:

> Hi,
>
> On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote:
>> On Wed, 29 Nov 2023, Hamza Mahfooz  wrote:
>> > Cc: Nathan Chancellor 
>> >
>> > On 11/29/23 13:12, Jani Nikula wrote:
>> >> At least the i915 and amd drivers enable a bunch more compiler warnings
>> >> than the kernel defaults.
>> >> 
>> >> Extend the W=1 warnings to the entire drm subsystem by default. Use the
>> >> copy-pasted warnings from scripts/Makefile.extrawarn with
>> >> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
>> >> up with them in the future.
>> >> 
>> >> This is similar to the approach currently used in i915.
>> >> 
>> >> Some of the -Wextra warnings do need to be disabled, just like in
>> >> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
>> >> builds, depending on the warning.
>> >
>> > I think this should go in after drm-misc-next has a clean build (for
>> > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a
>> > lot of build configs.
>> 
>> Oh, I'm absolutely not suggesting this should be merged before known
>> warnings have been addressed one way or another. Either by fixing them
>> or by disabling said warning in driver local Makefiles, depending on the
>> case.
>
> I'm all for it, but yeah, we need some easy way to opt-in/opt-out. Some
> drivers are pretty much unmaintained now and are likely to never fix
> those warnings.
>

Maybe add a Kconfig symbol for it instead of making unconditional?

Something like:

+# Unconditionally enable W=1 warnings locally
+# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
+subdir-ccflags-$(CONFIG_DRM_EXTRA_CHECKS)  += -Wextra -Wunused 
-Wno-unused-parameter
...

> Maxime

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Maxime Ripard]

Hi,

On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote:
> On Wed, 29 Nov 2023, Hamza Mahfooz  wrote:
> > Cc: Nathan Chancellor 
> >
> > On 11/29/23 13:12, Jani Nikula wrote:
> >> At least the i915 and amd drivers enable a bunch more compiler warnings
> >> than the kernel defaults.
> >> 
> >> Extend the W=1 warnings to the entire drm subsystem by default. Use the
> >> copy-pasted warnings from scripts/Makefile.extrawarn with
> >> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
> >> up with them in the future.
> >> 
> >> This is similar to the approach currently used in i915.
> >> 
> >> Some of the -Wextra warnings do need to be disabled, just like in
> >> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
> >> builds, depending on the warning.
> >
> > I think this should go in after drm-misc-next has a clean build (for
> > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a
> > lot of build configs.
> 
> Oh, I'm absolutely not suggesting this should be merged before known
> warnings have been addressed one way or another. Either by fixing them
> or by disabling said warning in driver local Makefiles, depending on the
> case.

I'm all for it, but yeah, we need some easy way to opt-in/opt-out. Some
drivers are pretty much unmaintained now and are likely to never fix
those warnings.

Maxime


signature.asc
Description: PGP signature

[Intel-gfx] [PULL] drm-misc-next

[Maxime Ripard]

Hi,

Here's this week drm-misc-next PR

Thanks!
Maxime

drm-misc-next-2023-11-30:
drm-misc-next for 6.8:

UAPI Changes:
 - Introduction of DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
 - Introduction of DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT

Cross-subsystem Changes:
 - fbdev: Convert a big chunks of drivers to helper macros

Core Changes:
 - atomic: Add support for mouse hotspots

Driver Changes:
 - ast: Model Detection improvements
 - imagination: plenty of fixes
 - nouveau: use GPUVM, scheduling improvements
The following changes since commit a13fee31f56449fc600d9e064c7b32302f92dcef:

  Merge v6.7-rc3 into drm-next (2023-11-28 11:55:56 +0100)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2023-11-30

for you to fetch changes up to 9ee33dc47772724ff583b060bb37c62b92b2d9c4:

  drm/imagination: Fix IS_ERR() vs NULL bug in pvr_request_firmware() 
(2023-11-30 09:16:59 +0100)


drm-misc-next for 6.8:

UAPI Changes:
 - Introduction of DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
 - Introduction of DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT

Cross-subsystem Changes:
 - fbdev: Convert a big chunks of drivers to helper macros

Core Changes:
 - atomic: Add support for mouse hotspots

Driver Changes:
 - ast: Model Detection improvements
 - imagination: plenty of fixes
 - nouveau: use GPUVM, scheduling improvements


André Almeida (1):
  drm: Refuse to async flip with atomic prop changes

Bert Karwatzki (1):
  drm/sched: Partial revert of "Qualify drm_sched_wakeup() by 
drm_sched_entity_is_ready()"

Colin Ian King (1):
  drm/imagination: Fix a couple of spelling mistakes in literal strings

Dan Carpenter (2):
  drm/imagination: Fix error codes in pvr_device_clk_init()
  drm/imagination: Fix IS_ERR() vs NULL bug in pvr_request_firmware()

Danilo Krummrich (6):
  drm/nouveau: use GPUVM common infrastructure
  drm/nouveau: implement 1:1 scheduler - entity relationship
  drm/nouveau: enable dynamic job-flow control
  drm/imagination: vm: prevent duplicate drm_gpuvm_bo instances
  drm/imagination: vm: check for drm_gpuvm_range_valid()
  drm/imagination: vm: fix drm_gpuvm reference count

Dario Binacchi (1):
  drm/bridge: Fix typo in post_disable() description

Donald Robson (1):
  drm/imagination: Numerous documentation fixes.

Hsin-Yi Wang (3):
  drm/panel-edp: Add override_edid_mode quirk for generic edp
  drm/panel-edp: Add auo_b116xa3_mode
  drm/panel-edp: Avoid adding multiple preferred modes

Javier Martinez Canillas (5):
  drm: Allow drivers to indicate the damage helpers to ignore damage clips
  drm/virtio: Disable damage clipping if FB changed since last page-flip
  drm/vmwgfx: Disable damage clipping if FB changed since last page-flip
  drm/plane: Extend damage tracking kernel-doc
  drm/todo: Add entry about implementing buffer age for damage tracking

Jean Delvare (1):
  drm/loongson: Add platform dependency

Liu Ying (1):
  drm/bridge: imx93-mipi-dsi: Fix a couple of building warnings

Luben Tuikov (4):
  drm/sched: Fix bounds limiting when given a malformed entity
  drm/sched: Rename priority MIN to LOW
  drm/sched: Reverse run-queue priority enumeration
  drm/sched: Fix compilation issues with DRM priority rename

Michael Banack (1):
  drm: Introduce documentation for hotspot properties

Rajneesh Bhardwaj (1):
  drm/ttm: Schedule delayed_delete worker closer

Rob Herring (1):
  drm: Use device_get_match_data()

Simon Ser (2):
  drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits
  drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP

Thomas Hellström (1):
  Documentation/gpu: VM_BIND locking document

Thomas Zimmermann (43):
  Merge drm/drm-next into drm-misc-next
  drm/ast: Turn ioregs_lock to modeset_lock
  drm/ast: Rework I/O register setup
  drm/ast: Retrieve I/O-memory ranges without ast device
  drm/ast: Add I/O helpers without ast device
  drm/ast: Enable VGA without ast device instance
  drm/ast: Enable MMIO without ast device instance
  drm/ast: Partially implement POST without ast device instance
  drm/ast: Add enum ast_config_mode
  drm/ast: Detect ast device type and config mode without ast device
  drm/ast: Move detection code into PCI probe helper
  fbdev/acornfb: Fix name of fb_ops initializer macro
  fbdev/sm712fb: Use correct initializer macros for struct fb_ops
  fbdev/vfb: Set FBINFO_VIRTFB flag
  fbdev/vfb: Initialize fb_ops with fbdev macros
  fbdev/arcfb: Set FBINFO_VIRTFB flag
  fbdev/arcfb: Use generator macros for deferred I/O
  auxdisplay/cfag12864bfb: Set FBINFO_VIRTFB flag
  auxdisplay/cfag12864bfb: Initialize fb_ops with fbdev macros
  auxdisplay/ht16k33: Set FBINFO_VIRTFB flag
  auxdisplay/ht16k33: Initialize fb_ops with fb

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v1,1/1] drm/i915/display: Don't use "proxy" headers (rev2)

[Jani Nikula]

On Thu, 30 Nov 2023, Patchwork  wrote:
> == Series Details ==
>
> Series: series starting with [v1,1/1] drm/i915/display: Don't use "proxy" 
> headers (rev2)
> URL   : https://patchwork.freedesktop.org/series/127051/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from CI_DRM_13951 -> Patchwork_127051v2
> 
>
> Summary
> ---
>
>   **FAILURE**
>
>   Serious unknown changes coming with Patchwork_127051v2 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_127051v2, please notify your bug team 
> (i915-ci-in...@lists.freedesktop.org) to allow them
>   to document this new failure mode, which will reduce false positives in CI.
>
>   External URL: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/index.html
>
> Participating hosts (36 -> 35)
> --
>
>   Additional (2): bat-kbl-2 bat-dg2-9 
>   Missing(3): fi-pnv-d510 fi-snb-2520m bat-dg1-5 
>
> Possible new issues
> ---
>
>   Here are the unknown changes that may have been introduced in 
> Patchwork_127051v2:
>
> ### IGT changes ###
>
>  Possible regressions 
>
>   * igt@i915_selftest@live@memory_region:
> - bat-jsl-3:  [PASS][1] -> [INCOMPLETE][2]
>[1]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-jsl-3/igt@i915_selftest@live@memory_region.html
>[2]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-jsl-3/igt@i915_selftest@live@memory_region.html

An #include change can't cause this. Please re-report.

BR,
Jani.



>
>   
> Known issues
> 
>
>   Here are the changes found in Patchwork_127051v2 that come from known 
> issues:
>
> ### IGT changes ###
>
>  Issues hit 
>
>   * igt@fbdev@info:
> - bat-kbl-2:  NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1849])
>[3]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-kbl-2/igt@fb...@info.html
>
>   * igt@gem_lmem_swapping@parallel-random-engines:
> - bat-kbl-2:  NOTRUN -> [SKIP][4] ([fdo#109271]) +20 other tests 
> skip
>[4]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html
>
>   * igt@gem_mmap@basic:
> - bat-dg2-9:  NOTRUN -> [SKIP][5] ([i915#4083])
>[5]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@gem_m...@basic.html
>
>   * igt@gem_mmap_gtt@basic:
> - bat-dg2-9:  NOTRUN -> [SKIP][6] ([i915#4077]) +2 other tests 
> skip
>[6]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@gem_mmap_...@basic.html
>
>   * igt@gem_render_tiled_blits@basic:
> - bat-dg2-9:  NOTRUN -> [SKIP][7] ([i915#4079]) +1 other test skip
>[7]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@gem_render_tiled_bl...@basic.html
>
>   * igt@i915_hangman@error-state-basic:
> - bat-mtlp-6: [PASS][8] -> [ABORT][9] ([i915#9414])
>[8]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-mtlp-6/igt@i915_hang...@error-state-basic.html
>[9]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-mtlp-6/igt@i915_hang...@error-state-basic.html
>
>   * igt@i915_pm_rps@basic-api:
> - bat-dg2-9:  NOTRUN -> [SKIP][10] ([i915#6621])
>[10]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@i915_pm_...@basic-api.html
>
>   * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
> - bat-dg2-9:  NOTRUN -> [SKIP][11] ([i915#5190])
>[11]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html
>
>   * igt@kms_addfb_basic@basic-y-tiled-legacy:
> - bat-dg2-9:  NOTRUN -> [SKIP][12] ([i915#4215] / [i915#5190])
>[12]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@kms_addfb_ba...@basic-y-tiled-legacy.html
>
>   * igt@kms_addfb_basic@framebuffer-vs-set-tiling:
> - bat-dg2-9:  NOTRUN -> [SKIP][13] ([i915#4212]) +6 other tests 
> skip
>[13]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@kms_addfb_ba...@framebuffer-vs-set-tiling.html
>
>   * igt@kms_addfb_basic@tile-pitch-mismatch:
> - bat-dg2-9:  NOTRUN -> [SKIP][14] ([i915#4212] / [i915#5608])
>[14]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@kms_addfb_ba...@tile-pitch-mismatch.html
>
>   * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
> - bat-dg2-9:  NOTRUN -> [SKIP][15] ([i915#4103] / [i915#4213] / 
> [i915#5608]) +1 other test skip
>[15]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127051v2/bat-dg2-9/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html
>
>   * igt@kms_force_connector_basic@force-load-detect:
> 

Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Jani Nikula]

On Wed, 29 Nov 2023, Hamza Mahfooz  wrote:
> Cc: Nathan Chancellor 
>
> On 11/29/23 13:12, Jani Nikula wrote:
>> At least the i915 and amd drivers enable a bunch more compiler warnings
>> than the kernel defaults.
>> 
>> Extend the W=1 warnings to the entire drm subsystem by default. Use the
>> copy-pasted warnings from scripts/Makefile.extrawarn with
>> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
>> up with them in the future.
>> 
>> This is similar to the approach currently used in i915.
>> 
>> Some of the -Wextra warnings do need to be disabled, just like in
>> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
>> builds, depending on the warning.
>
> I think this should go in after drm-misc-next has a clean build (for
> COMPILE_TEST builds) with this patch applied. Otherwise, it will break a
> lot of build configs.

Oh, I'm absolutely not suggesting this should be merged before known
warnings have been addressed one way or another. Either by fixing them
or by disabling said warning in driver local Makefiles, depending on the
case.

While I'm suggesting this, I obviously (I hope) can't take on fixing all
the warnings this exposes. One way to scale would be for folks to apply
this locally, see what it uncovers in their drivers, and fix some.

As an intermediate step we might also apply this to a topic branch in
drm-tip, so you'd see the warnings when building drm-tip, but not in
indidual branches or in anything that's merged upstream.

BR,
Jani.


>
>> 
>> Cc: David Airlie 
>> Cc: Daniel Vetter 
>> Cc: Maarten Lankhorst 
>> Cc: Maxime Ripard 
>> Cc: Thomas Zimmermann 
>> Cc: Alex Deucher 
>> Cc: Christian König 
>> Cc: Pan, Xinhui 
>> Cc: Karol Herbst 
>> Cc: Lyude Paul 
>> Cc: Danilo Krummrich 
>> Cc: Rob Clark 
>> Cc: Abhinav Kumar 
>> Cc: Dmitry Baryshkov 
>> Cc: Sean Paul 
>> Cc: Marijn Suijten 
>> Signed-off-by: Jani Nikula 
>> 
>> ---
>> 
>> With my admittedly limited and very much x86 focused kernel config, I
>> get some -Wunused-but-set-variable and -Wformat-truncation= warnings,
>> but nothing we can't handle.
>> 
>> We could fix them up front, or disable the extra warnings on a per
>> driver basis with a FIXME comment in their respective Makefiles.
>> 
>> With the experience from i915, I think this would significantly reduce
>> the constant loop of warnings added by people not using W=1 and
>> subsequently fixed by people using W=1.
>> 
>> Note: I've Cc'd the maintainers of drm, drm misc and some of the biggest
>> drivers.
>> ---
>>   drivers/gpu/drm/Makefile | 27 +++
>>   1 file changed, 27 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index b4cb0835620a..6939e4ea13d5 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -5,6 +5,33 @@
>>   
>>   CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
>>   
>> +# Unconditionally enable W=1 warnings locally
>> +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
>> +subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
>> +subdir-ccflags-y += -Wmissing-declarations
>> +subdir-ccflags-y += $(call cc-option, -Wrestrict)
>> +subdir-ccflags-y += -Wmissing-format-attribute
>> +subdir-ccflags-y += -Wmissing-prototypes
>> +subdir-ccflags-y += -Wold-style-definition
>> +subdir-ccflags-y += -Wmissing-include-dirs
>> +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
>> +subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
>> +subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
>> +subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
>> +subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
>> +subdir-ccflags-y += $(call cc-option, -Wstringop-overflow)
>> +subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
>> +# The following turn off the warnings enabled by -Wextra
>> +ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
>> +subdir-ccflags-y += -Wno-missing-field-initializers
>> +subdir-ccflags-y += -Wno-type-limits
>> +subdir-ccflags-y += -Wno-shift-negative-value
>> +endif
>> +ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
>> +subdir-ccflags-y += -Wno-sign-compare
>> +endif
>> +# --- end copy-paste
>> +
>>   drm-y := \
>>  drm_aperture.o \
>>  drm_atomic.o \

-- 
Jani Nikula, Intel

Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices

[Christian König]

Hi Oak,

yeah, #4 is indeed a really good point and I think Felix will agree to 
that as well.


HMM is basically still missing a way to advise device attributes for the 
CPU address space. Both migration strategy as well as device specific 
information (like cache preferences) fall into this category.


Since there is a device specific component in those attributes as well I 
think device specific IOCTLs still make sense to update them, but HMM 
should offer the functionality to manage and store those information.


Split and merge of VMAs only become a problem if you attach those 
information to VMAs, if you keep them completely separate than that 
doesn't become an issue either. The down side of this approach is that 
you don't get automatically extending attribute ranges for growing VMAs 
for example.


Regards,
Christian.

Am 29.11.23 um 23:23 schrieb Zeng, Oak:

Hi Weixi,

Even though Christian has listed reasons rejecting this proposal (yes they are 
very reasonable to me), I would open my mind and further explore the 
possibility here. Since the current GPU driver uses a hmm based implementation 
(AMD and NV has done this; At Intel we are catching up), I want to explore how 
much we can benefit from the proposed approach and how your approach can solve 
some pain points of our development. So basically what I am questioning here 
is: what is the advantage of your approach against hmm.

To implement a UVM (unified virtual address space b/t cpu and gpu device), with 
hmm, driver essentially need to implement below functions:

1. device page table update. Your approach requires the same because this is 
device specific codes

2. Some migration functions to migrate memory b/t system memory and GPU local memory. My 
understanding is, even though you generalized this a bit, such as modified cpu page fault 
path, provided "general" gm_dev_fault handler... but device driver still need 
to provide migration functions because migration functions have to be device specific 
(i.e., using device dma/copy engine for performance purpose). Right?

3. GPU physical memory management, this part is now in drm/buddy, shared by all 
drivers. I think with your approach, driver still need to provide callback 
functions to allocate/free physical pages. Right? Or do you let linux core mm 
buddy manage device memory directly?

4. madvise/hints/virtual address range management. This has been pain point for 
us. Right now device driver has to maintain certain virtual address range data 
structure to maintain hints and other virtual address range based memory 
attributes. Driver need to sync with linux vma. Driver need to explicitly deal 
with range split/merging... HMM doesn't provide support in this area. Your 
approach seems cleaner/simpler to me...


So in above, I have examined the some key factors of a gpu UVM memory manager. 
I think for #1 and #2, hmm has provide pretty good abstraction/tools for 
address space mirroring and migration helpers. For #3, since we have a common 
drm/buddy layer, I don't think it is a big problem for driver writer now.

I do see #4 is something you solved more beautifully, requires new system call 
though.

Oak



-Original Message-
From: dri-devel  On Behalf Of
Christian König
Sent: Tuesday, November 28, 2023 8:09 AM
To: Weixi Zhu ; linux...@kvack.org; linux-
ker...@vger.kernel.org; a...@linux-foundation.org; Danilo Krummrich
; Dave Airlie ; Daniel Vetter

Cc: dri-de...@lists.freedesktop.org; leo...@nvidia.com; apop...@nvidia.com;
amd-...@lists.freedesktop.org; mgor...@suse.de; z...@nvidia.com; Wang, Zhi
A ; rcampb...@nvidia.com; j...@nvidia.com;
weixi@openeuler.sh; jhubb...@nvidia.com; intel-gfx@lists.freedesktop.org;
mhairgr...@nvidia.com; jgli...@redhat.com; Vivi, Rodrigo
; intel-gvt-...@lists.freedesktop.org;
tvrtko.ursu...@linux.intel.com; felix.kuehl...@amd.com; xinhui@amd.com;
alexander.deuc...@amd.com; ogab...@kernel.org
Subject: Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory
management) for external memory devices

Adding a few missing important people to the explicit to list.

Am 28.11.23 um 13:50 schrieb Weixi Zhu:

The problem:

Accelerator driver developers are forced to reinvent external MM subsystems
case by case, because Linux core MM only considers host memory resources.
These reinvented MM subsystems have similar orders of magnitude of LoC as
Linux MM (80K), e.g. Nvidia-UVM has 70K, AMD GPU has 14K and Huawei NPU

has

30K. Meanwhile, more and more vendors are implementing their own
accelerators, e.g. Microsoft's Maia 100. At the same time,
application-level developers suffer from poor programmability -- they must
consider parallel address spaces and be careful about the limited device
DRAM capacity. This can be alleviated if a malloc()-ed virtual address can
be shared by the accelerator, or the abundant host DRAM can further
transparently backup the device local memory.

These external MM systems share similar mechanisms except for the
hardware-d

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/syncmap: squelch a sparse warning (rev2)

[Jani Nikula]

On Thu, 30 Nov 2023, Patchwork  wrote:
> == Series Details ==
>
> Series: drm/i915/syncmap: squelch a sparse warning (rev2)
> URL   : https://patchwork.freedesktop.org/series/117802/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from CI_DRM_13951 -> Patchwork_117802v2
> 
>
> Summary
> ---
>
>   **FAILURE**
>
>   Serious unknown changes coming with Patchwork_117802v2 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_117802v2, please notify your bug team 
> (i915-ci-in...@lists.freedesktop.org) to allow them
>   to document this new failure mode, which will reduce false positives in CI.
>
>   External URL: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/index.html
>
> Participating hosts (36 -> 35)
> --
>
>   Additional (1): bat-kbl-2 
>   Missing(2): fi-snb-2520m bat-dg1-5 
>
> Possible new issues
> ---
>
>   Here are the unknown changes that may have been introduced in 
> Patchwork_117802v2:
>
> ### IGT changes ###
>
>  Possible regressions 
>
>   * igt@i915_pm_rpm@module-reload:
> - bat-atsm-1: [PASS][1] -> [SKIP][2]
>[1]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-atsm-1/igt@i915_pm_...@module-reload.html
>[2]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/bat-atsm-1/igt@i915_pm_...@module-reload.html

Unrelated, please re-report.

BR,
Jani.

>
>   
> Known issues
> 
>
>   Here are the changes found in Patchwork_117802v2 that come from known 
> issues:
>
> ### IGT changes ###
>
>  Issues hit 
>
>   * igt@fbdev@info:
> - bat-kbl-2:  NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1849])
>[3]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/bat-kbl-2/igt@fb...@info.html
>
>   * igt@gem_lmem_swapping@parallel-random-engines:
> - bat-kbl-2:  NOTRUN -> [SKIP][4] ([fdo#109271]) +20 other tests 
> skip
>[4]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html
>
>   * igt@i915_selftest@live@gt_heartbeat:
> - fi-kbl-guc: [PASS][5] -> [DMESG-FAIL][6] ([i915#5334] / 
> [i915#7872])
>[5]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/fi-kbl-guc/igt@i915_selftest@live@gt_heartbeat.html
>[6]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/fi-kbl-guc/igt@i915_selftest@live@gt_heartbeat.html
>
>   * igt@i915_selftest@live@workarounds:
> - bat-dg2-11: [PASS][7] -> [DMESG-FAIL][8] ([i915#9500])
>[7]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-dg2-11/igt@i915_selftest@l...@workarounds.html
>[8]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/bat-dg2-11/igt@i915_selftest@l...@workarounds.html
>
>   * igt@kms_pipe_crc_basic@nonblocking-crc:
> - bat-dg2-11: NOTRUN -> [SKIP][9] ([i915#1845] / [i915#9197])
>[9]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/bat-dg2-11/igt@kms_pipe_crc_ba...@nonblocking-crc.html
>
>   * igt@kms_pipe_crc_basic@read-crc-frame-sequence:
> - bat-kbl-2:  NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#1845]) 
> +14 other tests skip
>[10]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/bat-kbl-2/igt@kms_pipe_crc_ba...@read-crc-frame-sequence.html
>
>   
>  Possible fixes 
>
>   * igt@i915_selftest@live@gt_heartbeat:
> - fi-apl-guc: [DMESG-FAIL][11] ([i915#5334]) -> [PASS][12]
>[11]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
>[12]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
>
>   * igt@kms_hdmi_inject@inject-audio:
> - fi-kbl-guc: [FAIL][13] ([IGT#3]) -> [PASS][14]
>[13]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html
>[14]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html
>
>   * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
> - bat-rplp-1: [ABORT][15] ([i915#8668]) -> [PASS][16]
>[15]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html
>[16]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117802v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html
>
>   
>   {name}: This element is suppressed. This means it is ignored when computing
>   the status of the difference (SUCCESS, WARNING, or FAILURE).
>
>   [IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [i915#1845]: https://