Re: [Intel-gfx] [PATCH 13/15] drm/i915/huc: report HuC as loaded even if load still in progress
On Thu, 2022-06-09 at 16:19 -0700, Ceraolo Spurio, Daniele wrote: > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c > b/drivers/gpu/drm/i915/gt/uc/intel_huc.c > index 075ec97b459d..33bfac91fa01 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c > @@ -426,7 +426,10 @@ int intel_huc_check_status(struct intel_huc *huc) > break; > } > > - return intel_huc_is_authenticated(huc); > + if (intel_huc_is_authenticated(huc)) > + return 1; > + > + return !i915_sw_fence_done(>delayed_load.fence); > Are we sure we dont have a race condition here? Perhaps a less cleaner but might tigher check might be: if (i915_sw_fence_done(..) && intel_huc_is_authenticated(..)) return 1 else if (!i915_sw_fence_done(..)) return 1 return 0; To speed things up a little, please accept a conditional R-b if you can reply back with why we might not need to worry about the race condition or if i am just reading this incorrectly. Reviewed-by: Alan Previn > } > > static bool huc_has_delayed_load(struct intel_huc *huc) > -- > 2.25.1 >
Re: [Intel-gfx] [PATCH 11/15] drm/i915/huc: track delayed HuC load with a fence
Other than the one nit below, everything looks in order as per internal
development and reviews...
but really wish we had the nit added - so we have a single location we can
build on to get all
the various stages of gsc vs pxp vs huc operation sequences across hw gens (at
least the first
gen when it changes route).
Reviewed-by: Alan Previn
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +enum intel_huc_delayed_load_status {
> + INTEL_HUC_WAITING_ON_GSC = 0,
> + INTEL_HUC_WAITING_ON_PXP,
> + INTEL_HUC_DELAYED_LOAD_ERROR,
> +};
> +
> struct intel_huc {
> /* Generic uC firmware management */
> struct intel_uc_fw fw;
> @@ -20,17 +28,28 @@ struct intel_huc {
> u32 mask;
> u32 value;
> } status;
> +
> + struct {
> + struct i915_sw_fence fence;
> + struct hrtimer timer;
> + struct notifier_block nb;
> + enum intel_huc_delayed_load_status status;
> + } delayed_load;
> };
NIT: I really wish we had a state machine diagram for the huc load status
because of the prior, this and upcoming hw design progression - will be nice to
have a single place to look at the various flows (also for without
delayed-huc-loading case). Similiar to the diagram of what we have above
the definition of enum intel_uc_fw_status.
Re: [Intel-gfx] [PATCH 00/15] HuC loading for DG2
On 6/15/2022 7:28 PM, Zhang, Carl wrote: On From: Ye, Tony Sent: Thursday, June 16, 2022 12:15 AM On 6/15/2022 3:13 AM, Tvrtko Ursulin wrote: On 15/06/2022 00:15, Ye, Tony wrote: On 6/14/2022 8:30 AM, Ceraolo Spurio, Daniele wrote: On 6/14/2022 12:44 AM, Tvrtko Ursulin wrote: On 13/06/2022 19:13, Ceraolo Spurio, Daniele wrote: On 6/13/2022 10:39 AM, Tvrtko Ursulin wrote: On 13/06/2022 18:06, Ceraolo Spurio, Daniele wrote: On 6/13/2022 9:56 AM, Tvrtko Ursulin wrote: On 13/06/2022 17:41, Ceraolo Spurio, Daniele wrote: On 6/13/2022 9:31 AM, Tvrtko Ursulin wrote: On 13/06/2022 16:39, Ceraolo Spurio, Daniele wrote: On 6/13/2022 1:16 AM, Tvrtko Ursulin wrote: On 10/06/2022 00:19, Daniele Ceraolo Spurio wrote: On DG2, HuC loading is performed by the GSC, via a PXP command. The load operation itself is relatively simple (just send a message to the GSC with the physical address of the HuC in LMEM), but there are timing changes that requires special attention. In particular, to send a PXP command we need to first export the GSC driver and then wait for the mei-gsc and mei-pxp modules to start, which means that HuC load will complete after i915 load is complete. This means that there is a small window of time after i915 is registered and before HuC is loaded during which userspace could submit and/or checking the HuC load status, although this is quite unlikely to happen (HuC is usually loaded before kernel init/resume completes). We've consulted with the media team in regards to how to handle this and they've asked us to do the following: 1) Report HuC as loaded in the getparam IOCTL even if load is still in progress. The media driver uses the IOCTL as a way to check if HuC is enabled and then includes a secondary check in the batches to get the actual status, so doing it this way allows userspace to keep working without changes. 2) Stall all userspace VCS submission until HuC is loaded. Stalls are expected to be very rare (if any), due to the fact that HuC is usually loaded before kernel init/resume is completed. Motivation to add these complications into i915 are not clear to me here. I mean there is no HuC on DG2 _yet_ is the premise of the series, right? So no backwards compatibility concerns. In this case why jump through the hoops and not let userspace handle all of this by just leaving the getparam return the true status? The main areas impacted by the fact that we can't guarantee that HuC load is complete when i915 starts accepting submissions are boot and suspend/resume, with the latter being the main problem; GT reset is not a concern because HuC now survives it. A suspend/resume can be transparent to userspace and therefore the HuC status can temporarily flip from loaded to not without userspace knowledge, especially if we start going into deeper suspend states and start causing HuC resets when we go into runtime suspend. Note that this is different from what happens during GT reset for older platforms, because in that scenario we guarantee that HuC reload is complete before we restart the submission back-end, so userspace doesn't notice that the HuC status change. We had an internal discussion about this problem with both media and i915 archs and the conclusion was that the best option is for i915 to stall media submission while HuC (re-)load is in progress. Resume is potentialy a good reason - I did not pick up on that from the cover letter. I read the statement about the unlikely and small window where HuC is not loaded during kernel init/resume and I guess did not pick up on the resume part. Waiting for GSC to load HuC from i915 resume is not an option? GSC is an aux device exported by i915, so AFAIU GSC resume can't start until i915 resume completes. I'll dig into this in the next few days since I want to understand how exactly it works. Or someone can help explain. If in the end conclusion will be that i915 resume indeed cannot wait for GSC, then I think auto-blocking of queued up contexts on media engines indeed sounds unavoidable. Otherwise, as you explained, user experience post resume wouldn't be good. Even if we could implement a wait, I'm not sure we should. GSC resume and HuC reload takes ~300ms in most cases, I don't think we want to block within the i915 resume path for that long. Yeah maybe not. But entertaining the idea that it is technically possible to block - we could perhaps add uapi for userspace to mark contexts which want HuC access. Then track if there are any such contexts with outstanding submissions and only wait in resume if there are. If that would end up significantly less code on the i915 side to maintain is an open. What would be the end result from users point of view in case where it suspended during video playback? The proposed solution from this series sees the video stuck after resume. Maybe compositor blocks as well since I am not sure how well they handle one window not providing new data. Probably depends on the compositor. And
Re: [Intel-gfx] [Intel-gfx 1/1] drm/i915/guc: Asynchronous flush of GuC log regions
On 6/6/2022 17:23, Alan Previn wrote: Both error-capture and relay-logging mechanism use the GuC log infrastructure. That means the KMD must send a log flush complete notification back to GuC after reading the data out. This call is currently being sent synchronously. However, synchronous H2Gs cause problems when the system is backed up. There is no need for this to be synchronous. The KMD wasn't even looking at the return status from it. So make it asynchronous and then there is no issue about time outs. Signed-off-by: Alan Previn Reviewed-by: John Harrison --- drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 3 ++- drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c index 97a32e610c30..ffcd7a35f8df 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c @@ -1261,7 +1261,8 @@ static int __guc_capture_flushlog_complete(struct intel_guc *guc) GUC_CAPTURE_LOG_BUFFER }; - return intel_guc_send(guc, action, ARRAY_SIZE(action)); + return intel_guc_send_nb(guc, action, ARRAY_SIZE(action), 0); + } static void __guc_capture_process_output(struct intel_guc *guc) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c index 02311ad90264..25b2d7ce6640 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c @@ -31,7 +31,7 @@ static int guc_action_flush_log_complete(struct intel_guc *guc) GUC_DEBUG_LOG_BUFFER }; - return intel_guc_send(guc, action, ARRAY_SIZE(action)); + return intel_guc_send_nb(guc, action, ARRAY_SIZE(action), 0); } static int guc_action_flush_log(struct intel_guc *guc)
Re: [Intel-gfx] [PATCH 10/15] drm/i915/dg2: setup HuC loading via GSC
LGTM. Reviewed-by: Alan Previn On Thu, 2022-06-09 at 16:19 -0700, Ceraolo Spurio, Daniele wrote: > intel_huc_wait_for_auth_complete
[Intel-gfx] ✓ Fi.CI.IGT: success for Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation (rev3)
== Series Details ==
Series: Fixes integer overflow or integer truncation issues in page lookups,
ttm place configuration and scatterlist creation (rev3)
URL : https://patchwork.freedesktop.org/series/104704/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11849_full -> Patchwork_104704v3_full
Summary
---
**SUCCESS**
No regressions found.
Participating hosts (12 -> 12)
--
No changes in participating hosts
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_104704v3_full:
### IGT changes ###
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling:
- {shard-rkl}:NOTRUN -> [SKIP][1]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-rkl-5/igt@kms_flip_scaled_...@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html
Known issues
Here are the changes found in Patchwork_104704v3_full that come from known
issues:
### IGT changes ###
Issues hit
* igt@gem_ctx_isolation@preservation-s3@vecs0:
- shard-kbl: [PASS][2] -> [DMESG-WARN][3] ([i915#180]) +3 similar
issues
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-kbl1/igt@gem_ctx_isolation@preservation...@vecs0.html
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-kbl1/igt@gem_ctx_isolation@preservation...@vecs0.html
* igt@gem_ctx_persistence@hostile:
- shard-snb: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#1099])
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-snb6/igt@gem_ctx_persiste...@hostile.html
* igt@gem_eio@unwedge-stress:
- shard-iclb: [PASS][5] -> [TIMEOUT][6] ([i915#3070])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-iclb6/igt@gem_...@unwedge-stress.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-iclb5/igt@gem_...@unwedge-stress.html
* igt@gem_exec_balancer@parallel-out-fence:
- shard-iclb: [PASS][7] -> [SKIP][8] ([i915#4525])
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-iclb1/igt@gem_exec_balan...@parallel-out-fence.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-iclb6/igt@gem_exec_balan...@parallel-out-fence.html
* igt@gem_exec_fair@basic-none-share@rcs0:
- shard-tglb: [PASS][9] -> [FAIL][10] ([i915#2842]) +1 similar issue
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-tglb2/igt@gem_exec_fair@basic-none-sh...@rcs0.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-tglb2/igt@gem_exec_fair@basic-none-sh...@rcs0.html
* igt@gem_exec_fair@basic-none@vcs1:
- shard-iclb: NOTRUN -> [FAIL][11] ([i915#2842])
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-iclb2/igt@gem_exec_fair@basic-n...@vcs1.html
* igt@gem_exec_fair@basic-pace@vcs1:
- shard-kbl: [PASS][12] -> [FAIL][13] ([i915#2842])
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-kbl1/igt@gem_exec_fair@basic-p...@vcs1.html
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-kbl1/igt@gem_exec_fair@basic-p...@vcs1.html
* igt@gem_exec_whisper@basic-queues:
- shard-snb: NOTRUN -> [SKIP][14] ([fdo#109271]) +48 similar issues
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-snb6/igt@gem_exec_whis...@basic-queues.html
* igt@gem_lmem_swapping@basic:
- shard-apl: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4613]) +1
similar issue
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-apl7/igt@gem_lmem_swapp...@basic.html
* igt@gem_lmem_swapping@parallel-random-engines:
- shard-kbl: NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#4613])
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-kbl1/igt@gem_lmem_swapp...@parallel-random-engines.html
* igt@gem_lmem_swapping@verify-random-ccs:
- shard-skl: NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#4613]) +2
similar issues
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-skl1/igt@gem_lmem_swapp...@verify-random-ccs.html
* igt@gem_pxp@display-protected-crc:
- shard-tglb: NOTRUN -> [SKIP][18] ([i915#4270])
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-tglb2/igt@gem_...@display-protected-crc.html
- shard-iclb: NOTRUN -> [SKIP][19] ([i915#4270])
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/shard-iclb8/igt@gem_...@display-protected-crc.html
* igt@gem_softpin@evict-single-offset:
- shard-apl: NOTRUN -> [FAIL][20]
[Intel-gfx] ✗ Fi.CI.IGT: failure for Add support for LMEM PCIe resizable bar
== Series Details ==
Series: Add support for LMEM PCIe resizable bar
URL : https://patchwork.freedesktop.org/series/105928/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_11849_full -> Patchwork_105928v1_full
Summary
---
**FAILURE**
Serious unknown changes coming with Patchwork_105928v1_full absolutely need
to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_105928v1_full, please notify your bug team to allow
them
to document this new failure mode, which will reduce false positives in CI.
Participating hosts (12 -> 12)
--
No changes in participating hosts
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_105928v1_full:
### IGT changes ###
Possible regressions
*
igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][1]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs-upscal...@pipe-a-default-mode.html
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling:
- {shard-rkl}:NOTRUN -> [SKIP][2]
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/shard-rkl-2/igt@kms_flip_scaled_...@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html
Known issues
Here are the changes found in Patchwork_105928v1_full that come from known
issues:
### CI changes ###
Issues hit
* boot:
- shard-skl: ([PASS][3], [PASS][4], [PASS][5], [PASS][6],
[PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12],
[PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18],
[PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24],
[PASS][25], [PASS][26]) -> ([PASS][27], [PASS][28], [PASS][29], [PASS][30],
[PASS][31], [PASS][32], [PASS][33], [PASS][34], [FAIL][35], [PASS][36],
[PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42],
[PASS][43], [PASS][44], [PASS][45]) ([i915#5032])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl9/boot.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl9/boot.html
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl9/boot.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl7/boot.html
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl7/boot.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl7/boot.html
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl7/boot.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl6/boot.html
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl6/boot.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl6/boot.html
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl4/boot.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl4/boot.html
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl4/boot.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl4/boot.html
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl3/boot.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl3/boot.html
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl3/boot.html
[20]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl3/boot.html
[21]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl1/boot.html
[22]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl1/boot.html
[23]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl1/boot.html
[24]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl10/boot.html
[25]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl10/boot.html
[26]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl10/boot.html
[27]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/shard-skl9/boot.html
[28]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/shard-skl9/boot.html
[29]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/shard-skl7/boot.html
[30]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/shard-skl7/boot.html
[31]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/shard-skl6/boot.html
[32]:
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/pvc: Implement w/a 16016694945
They both are known issues. Re-reported.
-Original Message-
From: Roper, Matthew D
Sent: Friday, July 1, 2022 8:44 AM
To: intel-gfx@lists.freedesktop.org
Cc: Sousa, Gustavo ; Vudum, Lakshminarayana
Subject: Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/pvc: Implement w/a
16016694945
On Fri, Jul 01, 2022 at 12:52:21PM +, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915/pvc: Implement w/a 16016694945
> URL : https://patchwork.freedesktop.org/series/105837/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from CI_DRM_11835_full -> Patchwork_105837v1_full
>
>
> Summary
> ---
>
> **FAILURE**
>
> Serious unknown changes coming with Patchwork_105837v1_full absolutely need
> to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_105837v1_full, please notify your bug team to allow
> them
> to document this new failure mode, which will reduce false positives in CI.
>
>
>
> Participating hosts (13 -> 13)
> --
>
> No changes in participating hosts
>
> Possible new issues
> ---
>
> Here are the unknown changes that may have been introduced in
> Patchwork_105837v1_full:
>
> ### IGT changes ###
>
> Suppressed
>
> The following results come from untrusted machines, tests, or statuses.
> They do not affect the overall result.
>
> * igt@gem_create@create-ext-cpu-access-big:
> - {shard-rkl}:NOTRUN -> [SKIP][1]
>[1]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-rkl-5/igt@gem_cre...@create-ext-cpu-access-big.html
>
> * igt@i915_query@query-regions-unallocated:
> - {shard-dg1}:NOTRUN -> [SKIP][2] +2 similar issues
>[2]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-dg1-16/igt@i915_qu...@query-regions-unallocated.html
These both look like new tests that were just added to exercise small
BAR support; I believe the skips were expected until the corresponding
kernel changes from Matt Auld landed (which just happened this morning,
probably after this series was tested). Not related to Gustavo's patch
here.
>
>
> ### Piglit changes ###
>
> Possible regressions
>
> * spec@arb_gpu_shader5@texturegatheroffset@vs-rgba-3-unorm-2drect-const:
> - pig-glk-j5005: NOTRUN -> [CRASH][3] +1 similar issue
>[3]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/pig-glk-j5005/spec@arb_gpu_shader5@texturegatheroff...@vs-rgba-3-unorm-2drect-const.html
>
> * spec@ext_texture_snorm@fbo-colormask-formats:
> - pig-skl-6260u: NOTRUN -> [INCOMPLETE][4]
>[4]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/pig-skl-6260u/spec@ext_texture_sn...@fbo-colormask-formats.html
The PVC workaround wouldn't have impacted execution on old gen9
platforms like GLK and SKL. Not related to Gustavo's patch.
Applied to drm-intel-gt-next. Thanks for the patch.
Matt
>
>
> Known issues
>
>
> Here are the changes found in Patchwork_105837v1_full that come from known
> issues:
>
> ### IGT changes ###
>
> Issues hit
>
> * igt@gem_ctx_persistence@hang:
> - shard-skl: NOTRUN -> [SKIP][5] ([fdo#109271]) +116 similar
> issues
>[5]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-skl6/igt@gem_ctx_persiste...@hang.html
>
> * igt@gem_eio@unwedge-stress:
> - shard-tglb: [PASS][6] -> [FAIL][7] ([i915#5784])
>[6]:
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-tglb7/igt@gem_...@unwedge-stress.html
>[7]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-tglb5/igt@gem_...@unwedge-stress.html
>
> * igt@gem_exec_balancer@parallel-bb-first:
> - shard-iclb: [PASS][8] -> [SKIP][9] ([i915#4525]) +1 similar
> issue
>[8]:
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb2/igt@gem_exec_balan...@parallel-bb-first.html
>[9]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb3/igt@gem_exec_balan...@parallel-bb-first.html
>
> * igt@gem_exec_fair@basic-none@vcs0:
> - shard-apl: [PASS][10] -> [FAIL][11] ([i915#2842]) +1 similar
> issue
>[10]:
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-apl8/igt@gem_exec_fair@basic-n...@vcs0.html
>[11]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl7/igt@gem_exec_fair@basic-n...@vcs0.html
>
> * igt@gem_exec_fair@basic-pace@vcs0:
> - shard-kbl: [PASS][12] -> [FAIL][13] ([i915#2842])
>[12]:
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-kbl7/igt@gem_exec_fair@basic-p...@vcs0.html
>[13]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl1/igt@gem_exec_fair@basic-p...@vcs0.html
>
> *
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Apply waitboosting before fence wait
== Series Details ==
Series: drm/i915: Apply waitboosting before fence wait
URL : https://patchwork.freedesktop.org/series/105925/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11849_full -> Patchwork_105925v1_full
Summary
---
**SUCCESS**
No regressions found.
Participating hosts (12 -> 12)
--
No changes in participating hosts
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_105925v1_full:
### IGT changes ###
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling:
- {shard-rkl}:NOTRUN -> [SKIP][1] +1 similar issue
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/shard-rkl-2/igt@kms_flip_scaled_...@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html
*
{igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-upscaling@pipe-a-default-mode}:
- shard-iclb: NOTRUN -> [SKIP][2]
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/shard-iclb3/igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-upscal...@pipe-a-default-mode.html
* igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1:
- {shard-tglu}: [PASS][3] -> [DMESG-WARN][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-tglu-5/igt@kms_pipe_crc_basic@suspend-read-...@pipe-a-hdmi-a-1.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/shard-tglu-1/igt@kms_pipe_crc_basic@suspend-read-...@pipe-a-hdmi-a-1.html
New tests
-
New tests have been introduced between CI_DRM_11849_full and
Patchwork_105925v1_full:
### New IGT tests (2) ###
* igt@i915_pm_rps@fence-order:
- Statuses : 9 pass(s)
- Exec time: [0.01, 0.07] s
* igt@kms_color@ctm-green-to-red@pipe-d-hdmi-a-1:
- Statuses : 1 pass(s)
- Exec time: [0.44] s
Known issues
Here are the changes found in Patchwork_105925v1_full that come from known
issues:
### CI changes ###
Issues hit
* boot:
- shard-skl: ([PASS][5], [PASS][6], [PASS][7], [PASS][8],
[PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14],
[PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20],
[PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25], [PASS][26],
[PASS][27], [PASS][28]) -> ([PASS][29], [PASS][30], [PASS][31], [PASS][32],
[PASS][33], [PASS][34], [PASS][35], [PASS][36], [FAIL][37], [PASS][38],
[PASS][39], [PASS][40], [PASS][41], [PASS][42], [FAIL][43], [PASS][44],
[PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50],
[PASS][51], [PASS][52]) ([i915#5032])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl9/boot.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl9/boot.html
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl9/boot.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl7/boot.html
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl7/boot.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl7/boot.html
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl7/boot.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl6/boot.html
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl6/boot.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl6/boot.html
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl4/boot.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl4/boot.html
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl4/boot.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl4/boot.html
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl3/boot.html
[20]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl3/boot.html
[21]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl3/boot.html
[22]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl3/boot.html
[23]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl1/boot.html
[24]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl1/boot.html
[25]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl1/boot.html
[26]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl10/boot.html
[27]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl10/boot.html
[28]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl10/boot.html
[29]:
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/pvc: Implement w/a 16016694945
== Series Details == Series: drm/i915/pvc: Implement w/a 16016694945 URL : https://patchwork.freedesktop.org/series/105837/ State : success == Summary == CI Bug Log - changes from CI_DRM_11835_full -> Patchwork_105837v1_full Summary --- **SUCCESS** No regressions found. Participating hosts (13 -> 13) -- No changes in participating hosts Known issues Here are the changes found in Patchwork_105837v1_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_persistence@hang: - shard-skl: NOTRUN -> [SKIP][1] ([fdo#109271]) +116 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-skl6/igt@gem_ctx_persiste...@hang.html * igt@gem_eio@unwedge-stress: - shard-tglb: [PASS][2] -> [FAIL][3] ([i915#5784]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-tglb7/igt@gem_...@unwedge-stress.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-tglb5/igt@gem_...@unwedge-stress.html * igt@gem_exec_balancer@parallel-bb-first: - shard-iclb: [PASS][4] -> [SKIP][5] ([i915#4525]) +1 similar issue [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb2/igt@gem_exec_balan...@parallel-bb-first.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb3/igt@gem_exec_balan...@parallel-bb-first.html * igt@gem_exec_fair@basic-none@vcs0: - shard-apl: [PASS][6] -> [FAIL][7] ([i915#2842]) +1 similar issue [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-apl8/igt@gem_exec_fair@basic-n...@vcs0.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl7/igt@gem_exec_fair@basic-n...@vcs0.html * igt@gem_exec_fair@basic-pace@vcs0: - shard-kbl: [PASS][8] -> [FAIL][9] ([i915#2842]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-kbl7/igt@gem_exec_fair@basic-p...@vcs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl1/igt@gem_exec_fair@basic-p...@vcs0.html * igt@gem_exec_fair@basic-pace@vcs1: - shard-iclb: NOTRUN -> [FAIL][10] ([i915#2842]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb4/igt@gem_exec_fair@basic-p...@vcs1.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-iclb: [PASS][11] -> [FAIL][12] ([i915#2849]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb6/igt@gem_exec_fair@basic-throt...@rcs0.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb5/igt@gem_exec_fair@basic-throt...@rcs0.html * igt@gem_exec_schedule@u-submit-late-slice@bcs0: - shard-glk: [PASS][13] -> [INCOMPLETE][14] ([i915#6310]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-glk9/igt@gem_exec_schedule@u-submit-late-sl...@bcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-glk2/igt@gem_exec_schedule@u-submit-late-sl...@bcs0.html * igt@gem_lmem_swapping@heavy-verify-multi: - shard-apl: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4613]) +1 similar issue [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl6/igt@gem_lmem_swapp...@heavy-verify-multi.html * igt@gem_lmem_swapping@parallel-random-verify: - shard-kbl: NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#4613]) +2 similar issues [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl7/igt@gem_lmem_swapp...@parallel-random-verify.html * igt@gem_lmem_swapping@parallel-random-verify-ccs: - shard-skl: NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#4613]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-skl3/igt@gem_lmem_swapp...@parallel-random-verify-ccs.html * igt@gem_pxp@protected-encrypted-src-copy-not-readible: - shard-tglb: NOTRUN -> [SKIP][18] ([i915#4270]) +1 similar issue [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-tglb7/igt@gem_...@protected-encrypted-src-copy-not-readible.html * igt@gem_softpin@evict-single-offset: - shard-glk: NOTRUN -> [FAIL][19] ([i915#4171]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-glk7/igt@gem_soft...@evict-single-offset.html * igt@gem_userptr_blits@dmabuf-sync: - shard-apl: NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#3323]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl1/igt@gem_userptr_bl...@dmabuf-sync.html - shard-kbl: NOTRUN -> [SKIP][21] ([fdo#109271] / [i915#3323]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl7/igt@gem_userptr_bl...@dmabuf-sync.html * igt@gem_userptr_blits@dmabuf-unsync: - shard-tglb: NOTRUN ->
[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm: Clean up drm_crtc.h (rev7)
== Series Details == Series: drm: Clean up drm_crtc.h (rev7) URL : https://patchwork.freedesktop.org/series/105073/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/105073/revisions/7/mbox/ not applied Applying: drm/vmwgfx: Stop using 'TRUE' error: corrupt patch at line 19 error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0001 drm/vmwgfx: Stop using 'TRUE' When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Re: [Intel-gfx] [PATCH v2 1/4] drm/vmwgfx: Stop using 'TRUE'
On Jun 30, 2022, at 3:51 PM, Ville Syrjala
mailto:ville.syrj...@linux.intel.com>> wrote:
From: Ville Syrjälä
mailto:ville.syrj...@linux.intel.com>>
Stop using the 'TRUE' define. This ultimately gets defined by
acpi/actypes.h that gets included here via a convoluted chain of
other headers. drm_crtc.h is part of that chain, and I'm trying
to eliminate all unnecessary includes from it to avoid pointless
rebuilds.
v2: Split out from the bigger patch
Cc: Zack Rusin mailto:za...@vmware.com>>
Cc: VMware Graphics Reviewers
mailto:linux-graphics-maintai...@vmware.com>>
Acked-by: Sam Ravnborg mailto:s...@ravnborg.org>>
Acked-by: Jani Nikula mailto:jani.nik...@intel.com>>
Signed-off-by: Ville Syrjälä
mailto:ville.syrj...@linux.intel.com>>
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 693028c31b6b..ff2f735bbe7a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -252,7 +252,7 @@ static void vmw_cursor_update_position(struct vmw_private
*dev_priv,
vmw_write(dev_priv, SVGA_REG_CURSOR4_Y, y);
vmw_write(dev_priv, SVGA_REG_CURSOR4_SCREEN_ID, SVGA3D_INVALID_ID);
vmw_write(dev_priv, SVGA_REG_CURSOR4_ON, svga_cursor_on);
- vmw_write(dev_priv, SVGA_REG_CURSOR4_SUBMIT, TRUE);
+ vmw_write(dev_priv, SVGA_REG_CURSOR4_SUBMIT, 1);
} else if (vmw_is_cursor_bypass3_enabled(dev_priv)) {
vmw_fifo_mem_write(dev_priv, SVGA_FIFO_CURSOR_ON, svga_cursor_on);
vmw_fifo_mem_write(dev_priv, SVGA_FIFO_CURSOR_X, x);
Hi, Ville.
Sorry for the delay. Looks great. In case you haven’t pushed it to
drm-misc-next yet:
Reviewed-by: Zack Rusin mailto:za...@vmware.com>>
z
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/pvc: Implement w/a 16016694945
== Series Details == Series: drm/i915/pvc: Implement w/a 16016694945 URL : https://patchwork.freedesktop.org/series/105837/ State : success == Summary == CI Bug Log - changes from CI_DRM_11835_full -> Patchwork_105837v1_full Summary --- **SUCCESS** No regressions found. Participating hosts (13 -> 13) -- No changes in participating hosts Known issues Here are the changes found in Patchwork_105837v1_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_persistence@hang: - shard-skl: NOTRUN -> [SKIP][1] ([fdo#109271]) +116 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-skl6/igt@gem_ctx_persiste...@hang.html * igt@gem_eio@unwedge-stress: - shard-tglb: [PASS][2] -> [FAIL][3] ([i915#5784]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-tglb7/igt@gem_...@unwedge-stress.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-tglb5/igt@gem_...@unwedge-stress.html * igt@gem_exec_balancer@parallel-bb-first: - shard-iclb: [PASS][4] -> [SKIP][5] ([i915#4525]) +1 similar issue [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb2/igt@gem_exec_balan...@parallel-bb-first.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb3/igt@gem_exec_balan...@parallel-bb-first.html * igt@gem_exec_fair@basic-none@vcs0: - shard-apl: [PASS][6] -> [FAIL][7] ([i915#2842]) +1 similar issue [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-apl8/igt@gem_exec_fair@basic-n...@vcs0.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl7/igt@gem_exec_fair@basic-n...@vcs0.html * igt@gem_exec_fair@basic-pace@vcs0: - shard-kbl: [PASS][8] -> [FAIL][9] ([i915#2842]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-kbl7/igt@gem_exec_fair@basic-p...@vcs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl1/igt@gem_exec_fair@basic-p...@vcs0.html * igt@gem_exec_fair@basic-pace@vcs1: - shard-iclb: NOTRUN -> [FAIL][10] ([i915#2842]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb4/igt@gem_exec_fair@basic-p...@vcs1.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-iclb: [PASS][11] -> [FAIL][12] ([i915#2849]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb6/igt@gem_exec_fair@basic-throt...@rcs0.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb5/igt@gem_exec_fair@basic-throt...@rcs0.html * igt@gem_exec_schedule@u-submit-late-slice@bcs0: - shard-glk: [PASS][13] -> [INCOMPLETE][14] ([i915#6310]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-glk9/igt@gem_exec_schedule@u-submit-late-sl...@bcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-glk2/igt@gem_exec_schedule@u-submit-late-sl...@bcs0.html * igt@gem_lmem_swapping@heavy-verify-multi: - shard-apl: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4613]) +1 similar issue [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl6/igt@gem_lmem_swapp...@heavy-verify-multi.html * igt@gem_lmem_swapping@parallel-random-verify: - shard-kbl: NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#4613]) +2 similar issues [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl7/igt@gem_lmem_swapp...@parallel-random-verify.html * igt@gem_lmem_swapping@parallel-random-verify-ccs: - shard-skl: NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#4613]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-skl3/igt@gem_lmem_swapp...@parallel-random-verify-ccs.html * igt@gem_pxp@protected-encrypted-src-copy-not-readible: - shard-tglb: NOTRUN -> [SKIP][18] ([i915#4270]) +1 similar issue [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-tglb7/igt@gem_...@protected-encrypted-src-copy-not-readible.html * igt@gem_softpin@evict-single-offset: - shard-glk: NOTRUN -> [FAIL][19] ([i915#4171]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-glk7/igt@gem_soft...@evict-single-offset.html * igt@gem_userptr_blits@dmabuf-sync: - shard-apl: NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#3323]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl1/igt@gem_userptr_bl...@dmabuf-sync.html - shard-kbl: NOTRUN -> [SKIP][21] ([fdo#109271] / [i915#3323]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl7/igt@gem_userptr_bl...@dmabuf-sync.html * igt@gem_userptr_blits@dmabuf-unsync: - shard-tglb: NOTRUN ->
[Intel-gfx] ✓ Fi.CI.BAT: success for Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation (rev3)
== Series Details == Series: Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation (rev3) URL : https://patchwork.freedesktop.org/series/104704/ State : success == Summary == CI Bug Log - changes from CI_DRM_11849 -> Patchwork_104704v3 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/index.html Participating hosts (43 -> 35) -- Additional (2): fi-kbl-x1275 fi-bxt-dsi Missing(10): fi-bdw-5557u fi-tgl-u2 bat-dg1-5 bat-dg2-8 bat-dg2-9 fi-apl-guc bat-adlp-4 bat-adln-1 bat-jsl-3 bat-jsl-1 Known issues Here are the changes found in Patchwork_104704v3 that come from known issues: ### IGT changes ### Issues hit * igt@gem_huc_copy@huc-copy: - fi-bxt-dsi: NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#2190]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-bxt-dsi/igt@gem_huc_c...@huc-copy.html - fi-kbl-x1275: NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-kbl-x1275/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@verify-random: - fi-kbl-x1275: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-kbl-x1275/igt@gem_lmem_swapp...@verify-random.html - fi-bxt-dsi: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-bxt-dsi/igt@gem_lmem_swapp...@verify-random.html * igt@gem_tiled_blits@basic: - fi-bxt-dsi: NOTRUN -> [SKIP][5] ([fdo#109271]) +12 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-bxt-dsi/igt@gem_tiled_bl...@basic.html * igt@i915_selftest@live@gt_lrc: - fi-rkl-guc: [PASS][6] -> [INCOMPLETE][7] ([i915#4983]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html * igt@i915_selftest@live@hangcheck: - fi-hsw-4770:[PASS][8] -> [INCOMPLETE][9] ([i915#4785]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html * igt@i915_suspend@basic-s3-without-i915: - fi-rkl-11600: [PASS][10] -> [INCOMPLETE][11] ([i915#5982]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_chamelium@common-hpd-after-suspend: - fi-snb-2600:NOTRUN -> [SKIP][12] ([fdo#109271] / [fdo#111827]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-snb-2600/igt@kms_chamel...@common-hpd-after-suspend.html * igt@kms_chamelium@dp-crc-fast: - fi-kbl-x1275: NOTRUN -> [SKIP][13] ([fdo#109271] / [fdo#111827]) +8 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-kbl-x1275/igt@kms_chamel...@dp-crc-fast.html * igt@kms_chamelium@hdmi-edid-read: - fi-bxt-dsi: NOTRUN -> [SKIP][14] ([fdo#109271] / [fdo#111827]) +8 similar issues [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-bxt-dsi/igt@kms_chamel...@hdmi-edid-read.html * igt@prime_vgem@basic-userptr: - fi-kbl-x1275: NOTRUN -> [SKIP][15] ([fdo#109271]) +11 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-kbl-x1275/igt@prime_v...@basic-userptr.html * igt@runner@aborted: - fi-hsw-4770:NOTRUN -> [FAIL][16] ([fdo#109271] / [i915#4312] / [i915#5594] / [i915#6246]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-hsw-4770/igt@run...@aborted.html Possible fixes * igt@i915_selftest@live@gt_heartbeat: - fi-cfl-8109u: [DMESG-FAIL][17] ([i915#5334]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html * igt@i915_selftest@live@hangcheck: - fi-snb-2600:[INCOMPLETE][19] ([i915#3921]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-snb-2600/igt@i915_selftest@l...@hangcheck.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104704v3/fi-snb-2600/igt@i915_selftest@l...@hangcheck.html *
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation (rev3)
== Series Details ==
Series: Fixes integer overflow or integer truncation issues in page lookups,
ttm place configuration and scatterlist creation (rev3)
URL : https://patchwork.freedesktop.org/series/104704/
State : warning
== Summary ==
Error: dim checkpatch failed
a9f2e87eee8d drm: Move and add a few utility macros into drm util header
6d3ba3e26e48 drm/i915/gem: Typecheck page lookups
-:89: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#89: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:374:
+#define __i915_gem_object_get_sg(obj, it, n, offset) ({ \
+ exactly_pgoff_t(n); \
+ (__i915_gem_object_get_sg)(obj, it, n, offset); \
+})
-:104: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#104: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:386:
+#define i915_gem_object_get_sg(obj, n, offset) ({ \
+ exactly_pgoff_t(n); \
+ (i915_gem_object_get_sg)(obj, n, offset); \
+})
-:119: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#119: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:398:
+#define i915_gem_object_get_sg_dma(obj, n, offset) ({ \
+ exactly_pgoff_t(n); \
+ (i915_gem_object_get_sg_dma)(obj, n, offset); \
+})
-:129: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#129: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:406:
+#define i915_gem_object_get_page(obj, n) ({ \
+ exactly_pgoff_t(n); \
+ (i915_gem_object_get_page)(obj, n); \
+})
-:139: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#139: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:414:
+#define i915_gem_object_get_dirty_page(obj, n) ({ \
+ exactly_pgoff_t(n); \
+ (i915_gem_object_get_dirty_page)(obj, n); \
+})
-:150: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#150: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:423:
+#define i915_gem_object_get_dma_address_len(obj, n, len) ({ \
+ exactly_pgoff_t(n); \
+ (i915_gem_object_get_dma_address_len)(obj, n, len); \
+})
-:160: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#160: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:431:
+#define i915_gem_object_get_dma_address(obj, n) ({ \
+ exactly_pgoff_t(n); \
+ (i915_gem_object_get_dma_address)(obj, n); \
+})
total: 0 errors, 0 warnings, 7 checks, 391 lines checked
26cca639bed5 drm/i915: Check for integer truncation on scatterlist creation
-:199: WARNING:NEW_TYPEDEFS: do not add new typedefs
#199: FILE: drivers/gpu/drm/i915/i915_scatterlist.h:222:
+typedef unsigned int __sg_size_t; /* see linux/scatterlist.h */
-:200: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in
parentheses
#200: FILE: drivers/gpu/drm/i915/i915_scatterlist.h:223:
+#define sg_alloc_table(sgt, nents, gfp) \
+ overflows_type(nents, __sg_size_t) ? -E2BIG : (sg_alloc_table)(sgt,
(__sg_size_t)(nents), gfp)
-:200: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'nents' - possible
side-effects?
#200: FILE: drivers/gpu/drm/i915/i915_scatterlist.h:223:
+#define sg_alloc_table(sgt, nents, gfp) \
+ overflows_type(nents, __sg_size_t) ? -E2BIG : (sg_alloc_table)(sgt,
(__sg_size_t)(nents), gfp)
-:201: WARNING:LONG_LINE: line length of 102 exceeds 100 columns
#201: FILE: drivers/gpu/drm/i915/i915_scatterlist.h:224:
+ overflows_type(nents, __sg_size_t) ? -E2BIG : (sg_alloc_table)(sgt,
(__sg_size_t)(nents), gfp)
-:203: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in
parentheses
#203: FILE: drivers/gpu/drm/i915/i915_scatterlist.h:226:
+#define sg_alloc_table_from_pages_segment(sgt, pages, npages, offset, size,
max_segment, gfp) \
+ overflows_type(npages, __sg_size_t) ? -E2BIG :
(sg_alloc_table_from_pages_segment)(sgt, pages, (__sg_size_t)(npages), offset,
size, max_segment, gfp)
-:203: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'npages' - possible
side-effects?
#203: FILE: drivers/gpu/drm/i915/i915_scatterlist.h:226:
+#define sg_alloc_table_from_pages_segment(sgt, pages, npages, offset, size,
max_segment, gfp) \
+ overflows_type(npages, __sg_size_t) ? -E2BIG :
(sg_alloc_table_from_pages_segment)(sgt, pages, (__sg_size_t)(npages), offset,
size, max_segment, gfp)
-:204: WARNING:LONG_LINE: line length of 157 exceeds 100 columns
#204: FILE: drivers/gpu/drm/i915/i915_scatterlist.h:227:
+ overflows_type(npages, __sg_size_t) ? -E2BIG :
(sg_alloc_table_from_pages_segment)(sgt, pages, (__sg_size_t)(npages), offset,
size, max_segment, gfp)
total: 2 errors, 3 warnings, 2 checks, 123 lines checked
97ac44a48866 drm/i915: Check for integer truncation on the configuration of ttm
place
-:33: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'ptr' - possible side-effects?
#33: FILE: drivers/gpu/drm/i915/gem/i915_gem_ttm.c:140:
+#define SAFE_CONVERSION(ptr, value) ({ \
+ if (!safe_conversion(ptr, value)) { \
+
Re: [Intel-gfx] [PATCH v2 7/7] drm/i915: Remove truncation warning for large objects
On Tue, 5 Jul 2022 15:24:55 +0300
Gwan-gyeong Mun wrote:
> From: Chris Wilson
>
> Having addressed the issues surrounding incorrect types for local
> variables and potential integer truncation in using the scatterlist API,
> we have closed all the loop holes we had previously identified with
> dangerously large object creation. As such, we can eliminate the warning
> put in place to remind us to complete the review.
>
> Signed-off-by: Chris Wilson
> Signed-off-by: Gwan-gyeong Mun
> Cc: Tvrtko Ursulin
> Cc: Brian Welty
> Cc: Matthew Auld
> Cc: Thomas Hellström
> Testcase: igt@gem_create@create-massive
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4991
> Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 15 ---
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index 31bb09dccf2f..4d614e4c1c4e 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -20,25 +20,10 @@
>
> enum intel_region_id;
>
> -/*
> - * XXX: There is a prevalence of the assumption that we fit the
> - * object's page count inside a 32bit _signed_ variable. Let's document
> - * this and catch if we ever need to fix it. In the meantime, if you do
> - * spot such a local variable, please consider fixing!
> - *
> - * We can check for invalidly typed locals with typecheck(), see for example
> - * i915_gem_object_get_sg().
> - */
> -#define GEM_CHECK_SIZE_OVERFLOW(sz) \
> - GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
> -
> static inline bool i915_gem_object_size_2big(u64 size)
> {
> struct drm_i915_gem_object *obj;
>
> - if (GEM_CHECK_SIZE_OVERFLOW(size))
> - return true;
> -
> if (overflows_type(size, obj->base.size))
> return true;
>
Re: [Intel-gfx] [PATCH v2 6/7] drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large
On Tue, 5 Jul 2022 15:24:54 +0300 Gwan-gyeong Mun wrote: > The ttm_bo_init_reserved() functions returns -ENOSPC if the size is too big > to add vma. The direct function that returns -ENOSPC is > drm_mm_insert_node_in_range(). > To handle the same error as other code returning -E2BIG when the size is > too large, it converts return value to -E2BIG. > > Signed-off-by: Gwan-gyeong Mun > Cc: Chris Wilson > Cc: Matthew Auld > Cc: Thomas Hellström > Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab > --- > drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > index d579524663b3..271f64b7e4f1 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > @@ -1249,6 +1249,17 @@ int __i915_gem_ttm_object_init(struct > intel_memory_region *mem, > bo_type, _sys_placement, > page_size >> PAGE_SHIFT, > , NULL, NULL, i915_ttm_bo_destroy); > + > + /* > + * XXX: The ttm_bo_init_reserved() functions returns -ENOSPC if the size > + * is too big to add vma. The direct function that returns -ENOSPC is > + * drm_mm_insert_node_in_range(). To handle the same error as other code > + * that returns -E2BIG when the size is too large, it converts -ENOSPC > to > + * -E2BIG. > + */ > + if (size >> PAGE_SHIFT > INT_MAX && ret == -ENOSPC) > + ret = -E2BIG; > + > if (ret) > return i915_ttm_err_to_gem(ret); >
Re: [Intel-gfx] [PATCH v2 5/7] drm/i915: Check if the size is too big while creating shmem file
On Tue, 5 Jul 2022 15:24:53 +0300 Gwan-gyeong Mun wrote: > The __shmem_file_setup() function returns -EINVAL if size is greater than > MAX_LFS_FILESIZE. To handle the same error as other code that returns > -E2BIG when the size is too large, it add a code that returns -E2BIG when > the size is larger than the size that can be handled. > > Signed-off-by: Gwan-gyeong Mun > Cc: Chris Wilson > Cc: Matthew Auld > Cc: Thomas Hellström > Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab > --- > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > index 604e8829e8ea..8495e87432f6 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > @@ -541,6 +541,15 @@ static int __create_shmem(struct drm_i915_private *i915, > > drm_gem_private_object_init(>drm, obj, size); > > + /* XXX: The __shmem_file_setup() function returns -EINVAL if size is > + * greater than MAX_LFS_FILESIZE. > + * To handle the same error as other code that returns -E2BIG when > + * the size is too large, we add a code that returns -E2BIG when the > + * size is larger than the size that can be handled. > + */ > + if (size > MAX_LFS_FILESIZE) > + return -E2BIG; > + > if (i915->mm.gemfs) > filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size, >flags);
Re: [Intel-gfx] [PATCH v2 3/7] drm/i915: Check for integer truncation on scatterlist creation
On Tue, 5 Jul 2022 15:24:51 +0300
Gwan-gyeong Mun wrote:
> From: Chris Wilson
>
> There is an impedance mismatch between the scatterlist API using unsigned
> int and our memory/page accounting in unsigned long. That is we may try
> to create a scatterlist for a large object that overflows returning a
> small table into which we try to fit very many pages. As the object size
> is under control of userspace, we have to be prudent and catch the
> conversion errors.
>
> To catch the implicit truncation as we switch from unsigned long into the
> scatterlist's unsigned int, we use overflows_type check and report
> E2BIG prior to the operation. This is already used in our create ioctls to
> indicate if the uABI request is simply too large for the backing store.
> Failing that type check, we have a second check at sg_alloc_table time
> to make sure the values we are passing into the scatterlist API are not
> truncated.
>
> It uses pgoff_t for locals that are dealing with page indices, in this
> case, the page count is the limit of the page index.
> And it uses safe_conversion() macro which performs a type conversion (cast)
> of an integer value into a new variable, checking that the destination is
> large enough to hold the source value.
>
> v2: Move added i915_utils's macro into drm_util header (Jani N)
>
> Signed-off-by: Chris Wilson
> Signed-off-by: Gwan-gyeong Mun
> Cc: Tvrtko Ursulin
> Cc: Brian Welty
> Cc: Matthew Auld
> Cc: Thomas Hellström
> Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 --
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ---
> drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 5 -
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 4
> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 -
> drivers/gpu/drm/i915/gvt/dmabuf.c| 9 +
> drivers/gpu/drm/i915/i915_scatterlist.h | 8
> 8 files changed, 33 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
> b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
> index c698f95af15f..ff2e6e780631 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
> @@ -37,10 +37,13 @@ static int i915_gem_object_get_pages_internal(struct
> drm_i915_gem_object *obj)
> struct sg_table *st;
> struct scatterlist *sg;
> unsigned int sg_page_sizes;
> - unsigned int npages;
> + pgoff_t npages; /* restricted by sg_alloc_table */
> int max_order;
> gfp_t gfp;
>
> + if (!safe_conversion(, obj->base.size >> PAGE_SHIFT))
> + return -E2BIG;
> +
> max_order = MAX_ORDER;
> #ifdef CONFIG_SWIOTLB
> if (is_swiotlb_active(obj->base.dev->dev)) {
> @@ -67,7 +70,6 @@ static int i915_gem_object_get_pages_internal(struct
> drm_i915_gem_object *obj)
> if (!st)
> return -ENOMEM;
>
> - npages = obj->base.size / PAGE_SIZE;
> if (sg_alloc_table(st, npages, GFP_KERNEL)) {
> kfree(st);
> return -ENOMEM;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index a60c6f4517d5..31bb09dccf2f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -26,9 +26,6 @@ enum intel_region_id;
> * this and catch if we ever need to fix it. In the meantime, if you do
> * spot such a local variable, please consider fixing!
> *
> - * Aside from our own locals (for which we have no excuse!):
> - * - sg_table embeds unsigned int for nents
> - *
> * We can check for invalidly typed locals with typecheck(), see for example
> * i915_gem_object_get_sg().
> */
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
> b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
> index 0d0e46dae559..88ba7266a3a5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
> @@ -28,6 +28,10 @@ static int i915_gem_object_get_pages_phys(struct
> drm_i915_gem_object *obj)
> void *dst;
> int i;
>
> + /* Contiguous chunk, with a single scatterlist element */
> + if (overflows_type(obj->base.size, sg->length))
> + return -E2BIG;
> +
> if (GEM_WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
> return -EINVAL;
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 4eed3dd90ba8..604e8829e8ea 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -193,13 +193,16 @@ static int shmem_get_pages(struct drm_i915_gem_object
> *obj)
> struct drm_i915_private *i915 = to_i915(obj->base.dev);
> struct intel_memory_region *mem = obj->mm.region;
> struct address_space *mapping =
Re: [Intel-gfx] [PATCH v2 4/7] drm/i915: Check for integer truncation on the configuration of ttm place
On Tue, 5 Jul 2022 15:24:52 +0300
Gwan-gyeong Mun wrote:
> There is an impedance mismatch between the first/last valid page
> frame number of ttm place in unsigned and our memory/page accounting in
> unsigned long.
> As the object size is under the control of userspace, we have to be prudent
> and catch the conversion errors.
> To catch the implicit truncation as we switch from unsigned long to
> unsigned, we use overflows_type check and report E2BIG or overflow_type
> prior to the operation.
>
> Signed-off-by: Gwan-gyeong Mun
> Cc: Chris Wilson
> Cc: Matthew Auld
> Cc: Thomas Hellström
> Reviewed-by: Nirmoy Das
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 12 +---
> drivers/gpu/drm/i915/intel_region_ttm.c | 16 +---
> 2 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index cdcb3ee0c433..d579524663b3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -137,19 +137,25 @@ i915_ttm_place_from_region(const struct
> intel_memory_region *mr,
> if (mr->type == INTEL_MEMORY_SYSTEM)
> return;
>
> +#define SAFE_CONVERSION(ptr, value) ({ \
> + if (!safe_conversion(ptr, value)) { \
> + GEM_BUG_ON(overflows_type(value, *ptr)); \
> + } \
> +})
> if (flags & I915_BO_ALLOC_CONTIGUOUS)
> place->flags |= TTM_PL_FLAG_CONTIGUOUS;
> if (offset != I915_BO_INVALID_OFFSET) {
> - place->fpfn = offset >> PAGE_SHIFT;
> - place->lpfn = place->fpfn + (size >> PAGE_SHIFT);
> + SAFE_CONVERSION(>fpfn, offset >> PAGE_SHIFT);
> + SAFE_CONVERSION(>lpfn, place->fpfn + (size >>
> PAGE_SHIFT));
> } else if (mr->io_size && mr->io_size < mr->total) {
> if (flags & I915_BO_ALLOC_GPU_ONLY) {
> place->flags |= TTM_PL_FLAG_TOPDOWN;
> } else {
> place->fpfn = 0;
> - place->lpfn = mr->io_size >> PAGE_SHIFT;
> + SAFE_CONVERSION(>lpfn, mr->io_size >>
> PAGE_SHIFT);
> }
> }
> +#undef SAFE_CONVERSION
Why?
> }
>
> static void
> diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
> b/drivers/gpu/drm/i915/intel_region_ttm.c
> index 62ff77445b01..8fcb8654b978 100644
> --- a/drivers/gpu/drm/i915/intel_region_ttm.c
> +++ b/drivers/gpu/drm/i915/intel_region_ttm.c
> @@ -202,24 +202,34 @@ intel_region_ttm_resource_alloc(struct
> intel_memory_region *mem,
> struct ttm_resource *res;
> int ret;
>
> +#define SAFE_CONVERSION(ptr, value) ({ \
> + if (!safe_conversion(ptr, value)) { \
> + GEM_BUG_ON(overflows_type(value, *ptr)); \
> + ret = -E2BIG; \
> + goto out; \
> + } \
> +})
It is a bad practice to change execution inside a macro.
See "12) Macros, Enums and RTL" at Documentation/process/coding-style.rst.
> if (flags & I915_BO_ALLOC_CONTIGUOUS)
> place.flags |= TTM_PL_FLAG_CONTIGUOUS;
> if (offset != I915_BO_INVALID_OFFSET) {
> - place.fpfn = offset >> PAGE_SHIFT;
> - place.lpfn = place.fpfn + (size >> PAGE_SHIFT);
> + SAFE_CONVERSION(, offset >> PAGE_SHIFT);
> + SAFE_CONVERSION(, place.fpfn + (size >> PAGE_SHIFT));
> } else if (mem->io_size && mem->io_size < mem->total) {
> if (flags & I915_BO_ALLOC_GPU_ONLY) {
> place.flags |= TTM_PL_FLAG_TOPDOWN;
> } else {
> place.fpfn = 0;
> - place.lpfn = mem->io_size >> PAGE_SHIFT;
> + SAFE_CONVERSION(, mem->io_size >>
> PAGE_SHIFT);
> }
> }
> +#undef SAFE_CONVERSION
Why?
>
> mock_bo.base.size = size;
> mock_bo.bdev = >i915->bdev;
>
> ret = man->func->alloc(man, _bo, , );
> +
> +out:
> if (ret == -ENOSPC)
> ret = -ENXIO;
> if (!ret)
[Intel-gfx] ✓ Fi.CI.BAT: success for Add support for LMEM PCIe resizable bar
== Series Details ==
Series: Add support for LMEM PCIe resizable bar
URL : https://patchwork.freedesktop.org/series/105928/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11849 -> Patchwork_105928v1
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/index.html
Participating hosts (43 -> 39)
--
Additional (2): fi-kbl-x1275 fi-bxt-dsi
Missing(6): fi-bdw-5557u fi-tgl-u2 bat-dg2-9 fi-apl-guc fi-pnv-d510
bat-jsl-3
Known issues
Here are the changes found in Patchwork_105928v1 that come from known issues:
### IGT changes ###
Issues hit
* igt@gem_huc_copy@huc-copy:
- fi-bxt-dsi: NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#2190])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-bxt-dsi/igt@gem_huc_c...@huc-copy.html
- fi-kbl-x1275: NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-kbl-x1275/igt@gem_huc_c...@huc-copy.html
* igt@gem_lmem_swapping@parallel-random-engines:
- fi-bxt-dsi: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3
similar issues
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-bxt-dsi/igt@gem_lmem_swapp...@parallel-random-engines.html
* igt@gem_lmem_swapping@verify-random:
- fi-kbl-x1275: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3
similar issues
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-kbl-x1275/igt@gem_lmem_swapp...@verify-random.html
* igt@gem_tiled_blits@basic:
- fi-bxt-dsi: NOTRUN -> [SKIP][5] ([fdo#109271]) +12 similar issues
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-bxt-dsi/igt@gem_tiled_bl...@basic.html
* igt@i915_suspend@basic-s3-without-i915:
- fi-rkl-11600: [PASS][6] -> [INCOMPLETE][7] ([i915#5982])
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html
* igt@kms_chamelium@dp-hpd-fast:
- fi-kbl-x1275: NOTRUN -> [SKIP][8] ([fdo#109271] / [fdo#111827]) +8
similar issues
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-kbl-x1275/igt@kms_chamel...@dp-hpd-fast.html
* igt@kms_chamelium@hdmi-edid-read:
- fi-bxt-dsi: NOTRUN -> [SKIP][9] ([fdo#109271] / [fdo#111827]) +8
similar issues
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-bxt-dsi/igt@kms_chamel...@hdmi-edid-read.html
* igt@prime_vgem@basic-userptr:
- fi-kbl-x1275: NOTRUN -> [SKIP][10] ([fdo#109271]) +11 similar issues
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-kbl-x1275/igt@prime_v...@basic-userptr.html
Possible fixes
* igt@i915_selftest@live@gt_heartbeat:
- fi-cfl-8109u: [DMESG-FAIL][11] ([i915#5334]) -> [PASS][12]
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@hangcheck:
- bat-dg1-6: [DMESG-FAIL][13] ([i915#4494] / [i915#4957]) ->
[PASS][14]
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html
* igt@kms_flip@basic-flip-vs-modeset@a-edp1:
- {bat-adlp-6}: [DMESG-WARN][15] ([i915#3576]) -> [PASS][16]
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/bat-adlp-6/igt@kms_flip@basic-flip-vs-mode...@a-edp1.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/bat-adlp-6/igt@kms_flip@basic-flip-vs-mode...@a-edp1.html
* igt@kms_flip@basic-flip-vs-modeset@b-edp1:
- bat-adlp-4: [DMESG-WARN][17] ([i915#3576]) -> [PASS][18]
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/bat-adlp-4/igt@kms_flip@basic-flip-vs-mode...@b-edp1.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105928v1/bat-adlp-4/igt@kms_flip@basic-flip-vs-mode...@b-edp1.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
[i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
Re: [Intel-gfx] [PATCH v2 2/7] drm/i915/gem: Typecheck page lookups
On Tue, 5 Jul 2022 15:24:50 +0300
Gwan-gyeong Mun wrote:
> From: Chris Wilson
>
> We need to check that we avoid integer overflows when looking up a page,
> and so fix all the instances where we have mistakenly used a plain
> integer instead of a more suitable long. Be pedantic and add integer
> typechecking to the lookup so that we can be sure that we are safe.
> And it also uses pgoff_t as our page lookups must remain compatible with
> the page cache, pgoff_t is currently exactly unsigned long.
>
> v2: Move added i915_utils's macro into drm_util header (Jani N)
>
> Signed-off-by: Chris Wilson
> Signed-off-by: Gwan-gyeong Mun
> Cc: Tvrtko Ursulin
> Cc: Matthew Auld
> Cc: Thomas Hellström
> Reviewed-by: Nirmoy Das
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +-
> drivers/gpu/drm/i915/gem/i915_gem_object.h| 67 ++-
> drivers/gpu/drm/i915/gem/i915_gem_pages.c | 25 ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
> .../drm/i915/gem/selftests/i915_gem_context.c | 12 ++--
> .../drm/i915/gem/selftests/i915_gem_mman.c| 8 +--
> .../drm/i915/gem/selftests/i915_gem_object.c | 8 +--
> drivers/gpu/drm/i915/i915_gem.c | 18 +++--
> drivers/gpu/drm/i915/i915_vma.c | 8 +--
> 9 files changed, 100 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index ccec4055fde3..90996fe8ad45 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -421,10 +421,11 @@ void __i915_gem_object_invalidate_frontbuffer(struct
> drm_i915_gem_object *obj,
> static void
> i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64
> offset, void *dst, int size)
> {
> + pgoff_t idx = offset >> PAGE_SHIFT;
> void *src_map;
> void *src_ptr;
>
> - src_map = kmap_atomic(i915_gem_object_get_page(obj, offset >>
> PAGE_SHIFT));
> + src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
>
> src_ptr = src_map + offset_in_page(offset);
> if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
> @@ -437,9 +438,10 @@ i915_gem_object_read_from_page_kmap(struct
> drm_i915_gem_object *obj, u64 offset,
> static void
> i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64
> offset, void *dst, int size)
> {
> + pgoff_t idx = offset >> PAGE_SHIFT;
> + dma_addr_t dma = i915_gem_object_get_dma_address(obj, idx);
> void __iomem *src_map;
> void __iomem *src_ptr;
> - dma_addr_t dma = i915_gem_object_get_dma_address(obj, offset >>
> PAGE_SHIFT);
>
> src_map = io_mapping_map_wc(>mm.region->iomap,
> dma - obj->mm.region->region.start,
> @@ -468,6 +470,7 @@ i915_gem_object_read_from_page_iomap(struct
> drm_i915_gem_object *obj, u64 offset
> */
> int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64
> offset, void *dst, int size)
> {
> + GEM_BUG_ON(overflows_type(offset >> PAGE_SHIFT, pgoff_t));
> GEM_BUG_ON(offset >= obj->base.size);
> GEM_BUG_ON(offset_in_page(offset) > PAGE_SIZE - size);
> GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index 6f0a3ce35567..a60c6f4517d5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -27,8 +27,10 @@ enum intel_region_id;
> * spot such a local variable, please consider fixing!
> *
> * Aside from our own locals (for which we have no excuse!):
> - * - sg_table embeds unsigned int for num_pages
> - * - get_user_pages*() mixed ints with longs
> + * - sg_table embeds unsigned int for nents
> + *
> + * We can check for invalidly typed locals with typecheck(), see for example
> + * i915_gem_object_get_sg().
> */
> #define GEM_CHECK_SIZE_OVERFLOW(sz) \
> GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
> @@ -366,41 +368,70 @@ int i915_gem_object_set_tiling(struct
> drm_i915_gem_object *obj,
> struct scatterlist *
> __i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
>struct i915_gem_object_page_iter *iter,
> - unsigned int n,
> - unsigned int *offset, bool dma);
> + pgoff_t n,
> + unsigned int *offset);
> +
> +#define __i915_gem_object_get_sg(obj, it, n, offset) ({ \
> + exactly_pgoff_t(n); \
> + (__i915_gem_object_get_sg)(obj, it, n, offset); \
> +})
>
> static inline struct scatterlist *
> -i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
> -unsigned int n,
> +i915_gem_object_get_sg(struct drm_i915_gem_object *obj, pgoff_t n,
> unsigned int *offset)
> {
> - return __i915_gem_object_get_sg(obj, >mm.get_page, n, offset,
>
Re: [Intel-gfx] [PATCH v2 1/7] drm: Move and add a few utility macros into drm util header
On Tue, 5 Jul 2022 15:24:49 +0300
Gwan-gyeong Mun wrote:
> It moves overflows_type utility macro into drm util header from i915_utils
> header. The overflows_type can be used to catch the truncation between data
> types. And it adds safe_conversion() macro which performs a type conversion
> (cast) of an source value into a new variable, checking that the
> destination is large enough to hold the source value.
> And it adds exact_type and exactly_pgoff_t macro to catch type mis-match
> while compiling.
>
> Signed-off-by: Gwan-gyeong Mun
> Cc: Thomas Hellström
> Cc: Matthew Auld
> Cc: Nirmoy Das
> Cc: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_utils.h | 5 +--
> include/drm/drm_util.h| 54 +++
> 2 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_utils.h
> b/drivers/gpu/drm/i915/i915_utils.h
> index c10d68cdc3ca..345e5b2dc1cd 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
>
> #ifdef CONFIG_X86
> #include
> @@ -111,10 +112,6 @@ bool i915_error_injected(void);
> #define range_overflows_end_t(type, start, size, max) \
> range_overflows_end((type)(start), (type)(size), (type)(max))
>
> -/* Note we don't consider signbits :| */
> -#define overflows_type(x, T) \
> - (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
> -
> #define ptr_mask_bits(ptr, n) ({ \
> unsigned long __v = (unsigned long)(ptr); \
> (typeof(ptr))(__v & -BIT(n)); \
> diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
> index 79952d8c4bba..c56230e39e37 100644
> --- a/include/drm/drm_util.h
> +++ b/include/drm/drm_util.h
> @@ -62,6 +62,60 @@
> */
> #define for_each_if(condition) if (!(condition)) {} else
>
> +/**
> + * overflows_type - helper for checking the truncation between data types
> + * @x: Source for overflow type comparison
> + * @T: Destination for overflow type comparison
> + *
> + * It compares the values and size of each data type between the first and
> + * second argument to check whether truncation can occur when assigning the
> + * first argument to the variable of the second argument.
> + * It does't consider signbits.
> + *
> + * Returns:
> + * True if truncation can occur, false otherwise.
> + */
> +#define overflows_type(x, T) \
> + (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
As pointed on its description, this macro only works if both types
are either signed or unsigned. However, the macro itself doesn't check
it.
It probably worth adding something there to ensure that both types are
either signed or unsigned. I would add this ancillary macro probably on
on a generic kernel header - as this can be useful outside drm:
#define sign_matches(x, y) \
(!((typeof(x))-1 >= 0) ^ ((typeof(y))-1 >= 0))
And then include use it at overflows_type:
BUILD_BUG_ON(!sign_matches(x, T))
> +
> +/**
> + * exact_type - break compile if source type and destination value's type are
> + * not the same
> + * @T: Source type
> + * @n: Destination value
> + *
> + * It is a helper macro for a poor man's -Wconversion: only allow variables
> of
> + * an exact type. It determines whether the source type and destination
> value's
> + * type are the same while compiling, and it breaks compile if two types are
> + * not the same
> + */
> +#define exact_type(T, n) \
> + BUILD_BUG_ON(!__builtin_constant_p(n) &&
> !__builtin_types_compatible_p(T, typeof(n)))
> +
> +/**
> + * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
> + * @n: value to compare pgoff_t type
> + *
> + * It breaks compile if the argument value's type is not pgoff_t type.
> + */
> +#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
> +
> +/*
> + * safe_conversion - perform a type conversion (cast) of an source value into
> + * a new variable, checking that the destination is large enough to hold the
> + * source value.
> + * @ptr: Destination pointer address
> + * @value: Source value
> + *
> + * Returns:
> + * If the value would overflow the destination, it returns false.
> + */
> +#define safe_conversion(ptr, value) ({ \
> + typeof(value) __v = (value); \
> + typeof(ptr) __ptr = (ptr); \
> + overflows_type(__v, *__ptr) ? 0 : (*__ptr = (typeof(*__ptr))__v), 1; \
> +})
> +
> /**
> * drm_can_sleep - returns true if currently okay to sleep
> *
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Add support for LMEM PCIe resizable bar
== Series Details == Series: Add support for LMEM PCIe resizable bar URL : https://patchwork.freedesktop.org/series/105928/ 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: success for drm/i915: Apply waitboosting before fence wait
== Series Details ==
Series: drm/i915: Apply waitboosting before fence wait
URL : https://patchwork.freedesktop.org/series/105925/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11849 -> Patchwork_105925v1
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/index.html
Participating hosts (43 -> 39)
--
Additional (1): fi-bxt-dsi
Missing(5): fi-bdw-5557u fi-tgl-u2 fi-cfl-guc fi-apl-guc bat-jsl-1
Known issues
Here are the changes found in Patchwork_105925v1 that come from known issues:
### IGT changes ###
Issues hit
* igt@gem_exec_suspend@basic-s3@smem:
- fi-rkl-11600: [PASS][1] -> [FAIL][2] ([fdo#103375])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-rkl-11600/igt@gem_exec_suspend@basic...@smem.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-rkl-11600/igt@gem_exec_suspend@basic...@smem.html
* igt@gem_huc_copy@huc-copy:
- fi-bxt-dsi: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-bxt-dsi/igt@gem_huc_c...@huc-copy.html
* igt@gem_lmem_swapping@parallel-random-engines:
- fi-bxt-dsi: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3
similar issues
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-bxt-dsi/igt@gem_lmem_swapp...@parallel-random-engines.html
* igt@gem_tiled_blits@basic:
- fi-bxt-dsi: NOTRUN -> [SKIP][5] ([fdo#109271]) +12 similar issues
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-bxt-dsi/igt@gem_tiled_bl...@basic.html
* igt@i915_selftest@live@gem:
- fi-pnv-d510:NOTRUN -> [DMESG-FAIL][6] ([i915#4528])
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-pnv-d510/igt@i915_selftest@l...@gem.html
* igt@i915_selftest@live@hangcheck:
- bat-dg1-5: [PASS][7] -> [DMESG-FAIL][8] ([i915#4494] /
[i915#4957])
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/bat-dg1-5/igt@i915_selftest@l...@hangcheck.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/bat-dg1-5/igt@i915_selftest@l...@hangcheck.html
* igt@i915_selftest@live@late_gt_pm:
- fi-bsw-nick:[PASS][9] -> [DMESG-FAIL][10] ([i915#3428])
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-bsw-nick/igt@i915_selftest@live@late_gt_pm.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-bsw-nick/igt@i915_selftest@live@late_gt_pm.html
* igt@kms_busy@basic@flip:
- bat-adlp-4: [PASS][11] -> [DMESG-WARN][12] ([i915#3576])
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/bat-adlp-4/igt@kms_busy@ba...@flip.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/bat-adlp-4/igt@kms_busy@ba...@flip.html
* igt@kms_chamelium@hdmi-edid-read:
- fi-bxt-dsi: NOTRUN -> [SKIP][13] ([fdo#109271] / [fdo#111827]) +8
similar issues
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-bxt-dsi/igt@kms_chamel...@hdmi-edid-read.html
*
igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
- fi-bsw-kefka: [PASS][14] -> [FAIL][15] ([i915#6298])
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html
* igt@runner@aborted:
- fi-bsw-nick:NOTRUN -> [FAIL][16] ([fdo#109271] / [i915#4312])
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-bsw-nick/igt@run...@aborted.html
Possible fixes
* igt@gem_ringfill@basic-all:
- {bat-dg2-9}:[FAIL][17] ([i915#5886]) -> [PASS][18]
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/bat-dg2-9/igt@gem_ringf...@basic-all.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/bat-dg2-9/igt@gem_ringf...@basic-all.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-cfl-8109u: [DMESG-FAIL][19] ([i915#5334]) -> [PASS][20]
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html
[20]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105925v1/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@gt_timelines:
- {bat-dg2-9}:[DMESG-WARN][21] ([i915#5763]) -> [PASS][22] +7
similar issues
[21]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/bat-dg2-9/igt@i915_selftest@live@gt_timelines.html
[22]:
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Apply waitboosting before fence wait
== Series Details ==
Series: drm/i915: Apply waitboosting before fence wait
URL : https://patchwork.freedesktop.org/series/105925/
State : warning
== Summary ==
Error: dim checkpatch failed
3102ce715842 drm/i915/gem: Look for waitboosting across the whole object prior
to individual waits
bc7ab100fb8f drm/i915: Bump GT idling delay to 2 jiffies
0c81ea0e8458 drm/i915/gt: Only kick the signal worker if there's been an update
-:23: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#23:
References: 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv workaround")
-:23: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+
chars of sha1> ("")' - ie: 'commit 047a1b877ed4 ("dma-buf &
drm/amdgpu: remove dma_resv workaround")'
#23:
References: 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv workaround")
total: 1 errors, 1 warnings, 0 checks, 9 lines checked
Re: [Intel-gfx] [RFC 05/10] drm/i915/vm_bind: Handle persistent vmas
Thanks,
Oak
> -Original Message-
> From: C, Ramalingam
> Sent: July 5, 2022 5:20 AM
> To: Zeng, Oak
> Cc: Vishwanathapura, Niranjana ;
> intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Vetter,
> Daniel ; christian.koe...@amd.com; Hellstrom,
> Thomas ; Zanoni, Paulo R
> ; Auld, Matthew
> Subject: Re: [Intel-gfx] [RFC 05/10] drm/i915/vm_bind: Handle persistent
> vmas
>
> On 2022-07-04 at 17:05:38 +, Zeng, Oak wrote:
> >
> >
> > Thanks,
> > Oak
> >
> > > -Original Message-
> > > From: Intel-gfx On Behalf
> > > Of Niranjana Vishwanathapura
> > > Sent: July 1, 2022 6:51 PM
> > > To: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> > > Cc: Zanoni, Paulo R ; Hellstrom, Thomas
> > > ; Auld, Matthew
> > > ; Vetter, Daniel ;
> > > christian.koe...@amd.com
> > > Subject: [Intel-gfx] [RFC 05/10] drm/i915/vm_bind: Handle persistent
> > > vmas
> > >
> > > Treat VM_BIND vmas as persistent and handle them during the request
> > > submission in the execbuff path.
> >
> > Hi Niranjana,
> >
> > Is the meaning of "persistent" above persistent across all the subsequent
> execbuf ioctls?
>
> Yes oak. Thats correct. persistent across multiple execbuf ioctls.
Thank you Ram. Maybe we can add that to the commit message: " Treat VM_BIND
vmas as persistent across multiple execbuf ioctls"? I think this is versus the
old execbuf mode where we bind in the execbuf ioctl and bindings are only valid
for that execbuf. For those who don't have such background knowledge, it is
hard to guess the meaning of persistent.
Thanks,
Oak
>
> Regards,
> Ram.
> >
> > Thanks,
> > Oak
> >
> > >
> > > Support eviction by maintaining a list of evicted persistent vmas
> > > for rebinding during next submission.
> > >
> > > Signed-off-by: Niranjana Vishwanathapura
> > >
> > > ---
> > > drivers/gpu/drm/i915/gem/i915_gem_object.c| 1 +
> > > drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 3 +
> > > .../drm/i915/gem/i915_gem_vm_bind_object.c| 12 ++-
> > > drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +
> > > drivers/gpu/drm/i915/gt/intel_gtt.h | 2 +
> > > drivers/gpu/drm/i915/i915_gem_gtt.h | 22 ++
> > > drivers/gpu/drm/i915/i915_vma.c | 32 +++-
> > > drivers/gpu/drm/i915/i915_vma.h | 78 +--
> > > drivers/gpu/drm/i915/i915_vma_types.h | 23 ++
> > > 9 files changed, 163 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > > index ccec4055fde3..5121f02ba95c 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > > @@ -38,6 +38,7 @@
> > > #include "i915_gem_mman.h"
> > > #include "i915_gem_object.h"
> > > #include "i915_gem_ttm.h"
> > > +#include "i915_gem_vm_bind.h"
> > > #include "i915_memcpy.h"
> > > #include "i915_trace.h"
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > > b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > > index 849bf3c1061e..eaadf5a6ab09 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > > @@ -6,6 +6,7 @@
> > > #ifndef __I915_GEM_VM_BIND_H
> > > #define __I915_GEM_VM_BIND_H
> > >
> > > +#include
> > > #include "i915_drv.h"
> > >
> > > #define assert_vm_bind_held(vm) lockdep_assert_held(&(vm)-
> > > >vm_bind_lock)
> > > @@ -26,6 +27,8 @@ static inline void i915_gem_vm_bind_unlock(struct
> > > i915_address_space *vm)
> > > mutex_unlock(>vm_bind_lock);
> > > }
> > >
> > > +#define assert_vm_priv_held(vm) assert_object_held((vm)->root_obj)
> > > +
> > > static inline int i915_gem_vm_priv_lock(struct i915_address_space *vm,
> > > struct i915_gem_ww_ctx *ww)
> > > {
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > > index 96f139cc8060..1a8efa83547f 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > > @@ -85,6 +85,13 @@ void i915_gem_vm_bind_remove(struct i915_vma
> > > *vma, bool release_obj) {
> > > assert_vm_bind_held(vma->vm);
> > >
> > > + spin_lock(>vm->vm_rebind_lock);
> > > + if (!list_empty(>vm_rebind_link))
> > > + list_del_init(>vm_rebind_link);
> > > + i915_vma_set_purged(vma);
> > > + i915_vma_set_freed(vma);
> > > + spin_unlock(>vm->vm_rebind_lock);
> > > +
> > > if (!list_empty(>vm_bind_link)) {
> > > list_del_init(>vm_bind_link);
> > > list_del_init(>non_priv_vm_bind_link);
> > > @@ -220,6 +227,7 @@ static struct i915_vma *vm_bind_get_vma(struct
> > > i915_address_space *vm,
> > >
> > > vma->start = va->start;
> > > vma->last = va->start + va->length - 1;
> > > + i915_vma_set_persistent(vma);
> > >
> > > return vma;
> > > }
> > > @@
[Intel-gfx] ✗ Fi.CI.BUILD: failure for Remove the VFIO_IOMMU_NOTIFY_DMA_UNMAP notifier (rev3)
== Series Details == Series: Remove the VFIO_IOMMU_NOTIFY_DMA_UNMAP notifier (rev3) URL : https://patchwork.freedesktop.org/series/104793/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/104793/revisions/3/mbox/ not applied Applying: vfio: Replace the DMA unmapping notifier with a callback Using index info to reconstruct a base tree... M drivers/s390/cio/vfio_ccw_private.h M include/linux/vfio.h Falling back to patching base and 3-way merge... Auto-merging include/linux/vfio.h CONFLICT (content): Merge conflict in include/linux/vfio.h Auto-merging drivers/s390/cio/vfio_ccw_private.h error: Failed to merge in the changes. hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0001 vfio: Replace the DMA unmapping notifier with a callback When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
[Intel-gfx] [PATCH v2 7/7] drm/i915: Remove truncation warning for large objects
From: Chris Wilson
Having addressed the issues surrounding incorrect types for local
variables and potential integer truncation in using the scatterlist API,
we have closed all the loop holes we had previously identified with
dangerously large object creation. As such, we can eliminate the warning
put in place to remind us to complete the review.
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Brian Welty
Cc: Matthew Auld
Cc: Thomas Hellström
Testcase: igt@gem_create@create-massive
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4991
Reviewed-by: Nirmoy Das
---
drivers/gpu/drm/i915/gem/i915_gem_object.h | 15 ---
1 file changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 31bb09dccf2f..4d614e4c1c4e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -20,25 +20,10 @@
enum intel_region_id;
-/*
- * XXX: There is a prevalence of the assumption that we fit the
- * object's page count inside a 32bit _signed_ variable. Let's document
- * this and catch if we ever need to fix it. In the meantime, if you do
- * spot such a local variable, please consider fixing!
- *
- * We can check for invalidly typed locals with typecheck(), see for example
- * i915_gem_object_get_sg().
- */
-#define GEM_CHECK_SIZE_OVERFLOW(sz) \
- GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
-
static inline bool i915_gem_object_size_2big(u64 size)
{
struct drm_i915_gem_object *obj;
- if (GEM_CHECK_SIZE_OVERFLOW(size))
- return true;
-
if (overflows_type(size, obj->base.size))
return true;
--
2.34.1
[Intel-gfx] [PATCH v2 6/7] drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large
The ttm_bo_init_reserved() functions returns -ENOSPC if the size is too big to add vma. The direct function that returns -ENOSPC is drm_mm_insert_node_in_range(). To handle the same error as other code returning -E2BIG when the size is too large, it converts return value to -E2BIG. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index d579524663b3..271f64b7e4f1 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1249,6 +1249,17 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, bo_type, _sys_placement, page_size >> PAGE_SHIFT, , NULL, NULL, i915_ttm_bo_destroy); + + /* +* XXX: The ttm_bo_init_reserved() functions returns -ENOSPC if the size +* is too big to add vma. The direct function that returns -ENOSPC is +* drm_mm_insert_node_in_range(). To handle the same error as other code +* that returns -E2BIG when the size is too large, it converts -ENOSPC to +* -E2BIG. +*/ + if (size >> PAGE_SHIFT > INT_MAX && ret == -ENOSPC) + ret = -E2BIG; + if (ret) return i915_ttm_err_to_gem(ret); -- 2.34.1
[Intel-gfx] [PATCH v2 5/7] drm/i915: Check if the size is too big while creating shmem file
The __shmem_file_setup() function returns -EINVAL if size is greater than MAX_LFS_FILESIZE. To handle the same error as other code that returns -E2BIG when the size is too large, it add a code that returns -E2BIG when the size is larger than the size that can be handled. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 604e8829e8ea..8495e87432f6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -541,6 +541,15 @@ static int __create_shmem(struct drm_i915_private *i915, drm_gem_private_object_init(>drm, obj, size); + /* XXX: The __shmem_file_setup() function returns -EINVAL if size is +* greater than MAX_LFS_FILESIZE. +* To handle the same error as other code that returns -E2BIG when +* the size is too large, we add a code that returns -E2BIG when the +* size is larger than the size that can be handled. +*/ + if (size > MAX_LFS_FILESIZE) + return -E2BIG; + if (i915->mm.gemfs) filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size, flags); -- 2.34.1
[Intel-gfx] [PATCH v2 4/7] drm/i915: Check for integer truncation on the configuration of ttm place
There is an impedance mismatch between the first/last valid page
frame number of ttm place in unsigned and our memory/page accounting in
unsigned long.
As the object size is under the control of userspace, we have to be prudent
and catch the conversion errors.
To catch the implicit truncation as we switch from unsigned long to
unsigned, we use overflows_type check and report E2BIG or overflow_type
prior to the operation.
Signed-off-by: Gwan-gyeong Mun
Cc: Chris Wilson
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 12 +---
drivers/gpu/drm/i915/intel_region_ttm.c | 16 +---
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index cdcb3ee0c433..d579524663b3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -137,19 +137,25 @@ i915_ttm_place_from_region(const struct
intel_memory_region *mr,
if (mr->type == INTEL_MEMORY_SYSTEM)
return;
+#define SAFE_CONVERSION(ptr, value) ({ \
+ if (!safe_conversion(ptr, value)) { \
+ GEM_BUG_ON(overflows_type(value, *ptr)); \
+ } \
+})
if (flags & I915_BO_ALLOC_CONTIGUOUS)
place->flags |= TTM_PL_FLAG_CONTIGUOUS;
if (offset != I915_BO_INVALID_OFFSET) {
- place->fpfn = offset >> PAGE_SHIFT;
- place->lpfn = place->fpfn + (size >> PAGE_SHIFT);
+ SAFE_CONVERSION(>fpfn, offset >> PAGE_SHIFT);
+ SAFE_CONVERSION(>lpfn, place->fpfn + (size >>
PAGE_SHIFT));
} else if (mr->io_size && mr->io_size < mr->total) {
if (flags & I915_BO_ALLOC_GPU_ONLY) {
place->flags |= TTM_PL_FLAG_TOPDOWN;
} else {
place->fpfn = 0;
- place->lpfn = mr->io_size >> PAGE_SHIFT;
+ SAFE_CONVERSION(>lpfn, mr->io_size >>
PAGE_SHIFT);
}
}
+#undef SAFE_CONVERSION
}
static void
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
b/drivers/gpu/drm/i915/intel_region_ttm.c
index 62ff77445b01..8fcb8654b978 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -202,24 +202,34 @@ intel_region_ttm_resource_alloc(struct
intel_memory_region *mem,
struct ttm_resource *res;
int ret;
+#define SAFE_CONVERSION(ptr, value) ({ \
+ if (!safe_conversion(ptr, value)) { \
+ GEM_BUG_ON(overflows_type(value, *ptr)); \
+ ret = -E2BIG; \
+ goto out; \
+ } \
+})
if (flags & I915_BO_ALLOC_CONTIGUOUS)
place.flags |= TTM_PL_FLAG_CONTIGUOUS;
if (offset != I915_BO_INVALID_OFFSET) {
- place.fpfn = offset >> PAGE_SHIFT;
- place.lpfn = place.fpfn + (size >> PAGE_SHIFT);
+ SAFE_CONVERSION(, offset >> PAGE_SHIFT);
+ SAFE_CONVERSION(, place.fpfn + (size >> PAGE_SHIFT));
} else if (mem->io_size && mem->io_size < mem->total) {
if (flags & I915_BO_ALLOC_GPU_ONLY) {
place.flags |= TTM_PL_FLAG_TOPDOWN;
} else {
place.fpfn = 0;
- place.lpfn = mem->io_size >> PAGE_SHIFT;
+ SAFE_CONVERSION(, mem->io_size >>
PAGE_SHIFT);
}
}
+#undef SAFE_CONVERSION
mock_bo.base.size = size;
mock_bo.bdev = >i915->bdev;
ret = man->func->alloc(man, _bo, , );
+
+out:
if (ret == -ENOSPC)
ret = -ENXIO;
if (!ret)
--
2.34.1
[Intel-gfx] [PATCH v2 3/7] drm/i915: Check for integer truncation on scatterlist creation
From: Chris Wilson
There is an impedance mismatch between the scatterlist API using unsigned
int and our memory/page accounting in unsigned long. That is we may try
to create a scatterlist for a large object that overflows returning a
small table into which we try to fit very many pages. As the object size
is under control of userspace, we have to be prudent and catch the
conversion errors.
To catch the implicit truncation as we switch from unsigned long into the
scatterlist's unsigned int, we use overflows_type check and report
E2BIG prior to the operation. This is already used in our create ioctls to
indicate if the uABI request is simply too large for the backing store.
Failing that type check, we have a second check at sg_alloc_table time
to make sure the values we are passing into the scatterlist API are not
truncated.
It uses pgoff_t for locals that are dealing with page indices, in this
case, the page count is the limit of the page index.
And it uses safe_conversion() macro which performs a type conversion (cast)
of an integer value into a new variable, checking that the destination is
large enough to hold the source value.
v2: Move added i915_utils's macro into drm_util header (Jani N)
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Brian Welty
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
---
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 --
drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ---
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4
drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 5 -
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 4
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 -
drivers/gpu/drm/i915/gvt/dmabuf.c| 9 +
drivers/gpu/drm/i915/i915_scatterlist.h | 8
8 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
index c698f95af15f..ff2e6e780631 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
@@ -37,10 +37,13 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
struct sg_table *st;
struct scatterlist *sg;
unsigned int sg_page_sizes;
- unsigned int npages;
+ pgoff_t npages; /* restricted by sg_alloc_table */
int max_order;
gfp_t gfp;
+ if (!safe_conversion(, obj->base.size >> PAGE_SHIFT))
+ return -E2BIG;
+
max_order = MAX_ORDER;
#ifdef CONFIG_SWIOTLB
if (is_swiotlb_active(obj->base.dev->dev)) {
@@ -67,7 +70,6 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
if (!st)
return -ENOMEM;
- npages = obj->base.size / PAGE_SIZE;
if (sg_alloc_table(st, npages, GFP_KERNEL)) {
kfree(st);
return -ENOMEM;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index a60c6f4517d5..31bb09dccf2f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -26,9 +26,6 @@ enum intel_region_id;
* this and catch if we ever need to fix it. In the meantime, if you do
* spot such a local variable, please consider fixing!
*
- * Aside from our own locals (for which we have no excuse!):
- * - sg_table embeds unsigned int for nents
- *
* We can check for invalidly typed locals with typecheck(), see for example
* i915_gem_object_get_sg().
*/
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 0d0e46dae559..88ba7266a3a5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -28,6 +28,10 @@ static int i915_gem_object_get_pages_phys(struct
drm_i915_gem_object *obj)
void *dst;
int i;
+ /* Contiguous chunk, with a single scatterlist element */
+ if (overflows_type(obj->base.size, sg->length))
+ return -E2BIG;
+
if (GEM_WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 4eed3dd90ba8..604e8829e8ea 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -193,13 +193,16 @@ static int shmem_get_pages(struct drm_i915_gem_object
*obj)
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct intel_memory_region *mem = obj->mm.region;
struct address_space *mapping = obj->base.filp->f_mapping;
- const unsigned long page_count = obj->base.size / PAGE_SIZE;
unsigned int max_segment = i915_sg_segment_size();
struct sg_table *st;
struct sgt_iter sgt_iter;
+ pgoff_t page_count;
struct page *page;
int
[Intel-gfx] [PATCH v2 2/7] drm/i915/gem: Typecheck page lookups
From: Chris Wilson
We need to check that we avoid integer overflows when looking up a page,
and so fix all the instances where we have mistakenly used a plain
integer instead of a more suitable long. Be pedantic and add integer
typechecking to the lookup so that we can be sure that we are safe.
And it also uses pgoff_t as our page lookups must remain compatible with
the page cache, pgoff_t is currently exactly unsigned long.
v2: Move added i915_utils's macro into drm_util header (Jani N)
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
---
drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +-
drivers/gpu/drm/i915/gem/i915_gem_object.h| 67 ++-
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 25 ---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
.../drm/i915/gem/selftests/i915_gem_context.c | 12 ++--
.../drm/i915/gem/selftests/i915_gem_mman.c| 8 +--
.../drm/i915/gem/selftests/i915_gem_object.c | 8 +--
drivers/gpu/drm/i915/i915_gem.c | 18 +++--
drivers/gpu/drm/i915/i915_vma.c | 8 +--
9 files changed, 100 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index ccec4055fde3..90996fe8ad45 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -421,10 +421,11 @@ void __i915_gem_object_invalidate_frontbuffer(struct
drm_i915_gem_object *obj,
static void
i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ pgoff_t idx = offset >> PAGE_SHIFT;
void *src_map;
void *src_ptr;
- src_map = kmap_atomic(i915_gem_object_get_page(obj, offset >>
PAGE_SHIFT));
+ src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
src_ptr = src_map + offset_in_page(offset);
if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
@@ -437,9 +438,10 @@ i915_gem_object_read_from_page_kmap(struct
drm_i915_gem_object *obj, u64 offset,
static void
i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ pgoff_t idx = offset >> PAGE_SHIFT;
+ dma_addr_t dma = i915_gem_object_get_dma_address(obj, idx);
void __iomem *src_map;
void __iomem *src_ptr;
- dma_addr_t dma = i915_gem_object_get_dma_address(obj, offset >>
PAGE_SHIFT);
src_map = io_mapping_map_wc(>mm.region->iomap,
dma - obj->mm.region->region.start,
@@ -468,6 +470,7 @@ i915_gem_object_read_from_page_iomap(struct
drm_i915_gem_object *obj, u64 offset
*/
int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ GEM_BUG_ON(overflows_type(offset >> PAGE_SHIFT, pgoff_t));
GEM_BUG_ON(offset >= obj->base.size);
GEM_BUG_ON(offset_in_page(offset) > PAGE_SIZE - size);
GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 6f0a3ce35567..a60c6f4517d5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -27,8 +27,10 @@ enum intel_region_id;
* spot such a local variable, please consider fixing!
*
* Aside from our own locals (for which we have no excuse!):
- * - sg_table embeds unsigned int for num_pages
- * - get_user_pages*() mixed ints with longs
+ * - sg_table embeds unsigned int for nents
+ *
+ * We can check for invalidly typed locals with typecheck(), see for example
+ * i915_gem_object_get_sg().
*/
#define GEM_CHECK_SIZE_OVERFLOW(sz) \
GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
@@ -366,41 +368,70 @@ int i915_gem_object_set_tiling(struct drm_i915_gem_object
*obj,
struct scatterlist *
__i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
struct i915_gem_object_page_iter *iter,
-unsigned int n,
-unsigned int *offset, bool dma);
+pgoff_t n,
+unsigned int *offset);
+
+#define __i915_gem_object_get_sg(obj, it, n, offset) ({ \
+ exactly_pgoff_t(n); \
+ (__i915_gem_object_get_sg)(obj, it, n, offset); \
+})
static inline struct scatterlist *
-i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
- unsigned int n,
+i915_gem_object_get_sg(struct drm_i915_gem_object *obj, pgoff_t n,
unsigned int *offset)
{
- return __i915_gem_object_get_sg(obj, >mm.get_page, n, offset,
false);
+ return __i915_gem_object_get_sg(obj, >mm.get_page, n, offset);
}
+#define i915_gem_object_get_sg(obj, n, offset) ({ \
+ exactly_pgoff_t(n); \
+ (i915_gem_object_get_sg)(obj, n, offset); \
+})
+
static inline
[Intel-gfx] [PATCH v2 1/7] drm: Move and add a few utility macros into drm util header
It moves overflows_type utility macro into drm util header from i915_utils
header. The overflows_type can be used to catch the truncation between data
types. And it adds safe_conversion() macro which performs a type conversion
(cast) of an source value into a new variable, checking that the
destination is large enough to hold the source value.
And it adds exact_type and exactly_pgoff_t macro to catch type mis-match
while compiling.
Signed-off-by: Gwan-gyeong Mun
Cc: Thomas Hellström
Cc: Matthew Auld
Cc: Nirmoy Das
Cc: Jani Nikula
---
drivers/gpu/drm/i915/i915_utils.h | 5 +--
include/drm/drm_util.h| 54 +++
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_utils.h
b/drivers/gpu/drm/i915/i915_utils.h
index c10d68cdc3ca..345e5b2dc1cd 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_X86
#include
@@ -111,10 +112,6 @@ bool i915_error_injected(void);
#define range_overflows_end_t(type, start, size, max) \
range_overflows_end((type)(start), (type)(size), (type)(max))
-/* Note we don't consider signbits :| */
-#define overflows_type(x, T) \
- (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
-
#define ptr_mask_bits(ptr, n) ({ \
unsigned long __v = (unsigned long)(ptr); \
(typeof(ptr))(__v & -BIT(n)); \
diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
index 79952d8c4bba..c56230e39e37 100644
--- a/include/drm/drm_util.h
+++ b/include/drm/drm_util.h
@@ -62,6 +62,60 @@
*/
#define for_each_if(condition) if (!(condition)) {} else
+/**
+ * overflows_type - helper for checking the truncation between data types
+ * @x: Source for overflow type comparison
+ * @T: Destination for overflow type comparison
+ *
+ * It compares the values and size of each data type between the first and
+ * second argument to check whether truncation can occur when assigning the
+ * first argument to the variable of the second argument.
+ * It does't consider signbits.
+ *
+ * Returns:
+ * True if truncation can occur, false otherwise.
+ */
+#define overflows_type(x, T) \
+ (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
+
+/**
+ * exact_type - break compile if source type and destination value's type are
+ * not the same
+ * @T: Source type
+ * @n: Destination value
+ *
+ * It is a helper macro for a poor man's -Wconversion: only allow variables of
+ * an exact type. It determines whether the source type and destination value's
+ * type are the same while compiling, and it breaks compile if two types are
+ * not the same
+ */
+#define exact_type(T, n) \
+ BUILD_BUG_ON(!__builtin_constant_p(n) &&
!__builtin_types_compatible_p(T, typeof(n)))
+
+/**
+ * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
+ * @n: value to compare pgoff_t type
+ *
+ * It breaks compile if the argument value's type is not pgoff_t type.
+ */
+#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
+
+/*
+ * safe_conversion - perform a type conversion (cast) of an source value into
+ * a new variable, checking that the destination is large enough to hold the
+ * source value.
+ * @ptr: Destination pointer address
+ * @value: Source value
+ *
+ * Returns:
+ * If the value would overflow the destination, it returns false.
+ */
+#define safe_conversion(ptr, value) ({ \
+ typeof(value) __v = (value); \
+ typeof(ptr) __ptr = (ptr); \
+ overflows_type(__v, *__ptr) ? 0 : (*__ptr = (typeof(*__ptr))__v), 1; \
+})
+
/**
* drm_can_sleep - returns true if currently okay to sleep
*
--
2.34.1
[Intel-gfx] [PATCH v2 0/7] Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation
This patch series fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation, etc. We need to check that we avoid integer overflows when looking up a page, and so fix all the instances where we have mistakenly used a plain integer instead of a more suitable long. And there is an impedance mismatch between the scatterlist API using unsigned int and our memory/page accounting in unsigned long. That is we may try to create a scatterlist for a large object that overflows returning a small table into which we try to fit very many pages. As the object size is under the control of userspace, we have to be prudent and catch the conversion errors. To catch the implicit truncation as we switch from unsigned long into the scatterlist's unsigned int, we use our overflows_type check and report E2BIG prior to the operation. This is already used in our create ioctls to indicate if the uABI request is simply too large for the backing store. And ttm place also has the same problem with scatterlist creation, and we fix the integer truncation problem with the way approached by scatterlist creation. And It corrects the error code to return -E2BIG when creating gem objects using ttm or shmem, if the size is too large in each case. In order to provide a common macro, it moves and adds a few utility macros into drm util header Testcase: igt@gem_create@create-massive Testcase: igt@gem_userptr_blits@input-checking Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4991 Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5411 Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Cc: Nirmoy Das Cc: Jani Nikula Cc: David Airlie Cc: Daniel Vetter Chris Wilson (3): drm/i915/gem: Typecheck page lookups drm/i915: Check for integer truncation on scatterlist creation drm/i915: Remove truncation warning for large objects Gwan-gyeong Mun (4): drm: Move and add a few utility macros into drm util header drm/i915: Check for integer truncation on the configuration of ttm place drm/i915: Check if the size is too big while creating shmem file drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 +- drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +- drivers/gpu/drm/i915/gem/i915_gem_object.h| 77 +++ drivers/gpu/drm/i915/gem/i915_gem_pages.c | 25 +++--- drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4 + drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 14 +++- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 29 ++- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 +- .../drm/i915/gem/selftests/i915_gem_context.c | 12 +-- .../drm/i915/gem/selftests/i915_gem_mman.c| 8 +- .../drm/i915/gem/selftests/i915_gem_object.c | 8 +- drivers/gpu/drm/i915/gvt/dmabuf.c | 9 ++- drivers/gpu/drm/i915/i915_gem.c | 18 - drivers/gpu/drm/i915/i915_scatterlist.h | 8 ++ drivers/gpu/drm/i915/i915_utils.h | 5 +- drivers/gpu/drm/i915/i915_vma.c | 8 +- drivers/gpu/drm/i915/intel_region_ttm.c | 16 +++- include/drm/drm_util.h| 54 + 18 files changed, 226 insertions(+), 87 deletions(-) -- 2.34.1
[Intel-gfx] [PATCH v3 2/2] drm/i915: Add lmem_bar_size modparam
From: Priyanka Dandamudi
For testing purposes, support forcing the lmem_bar_size through a new
modparam. In CI we only have a limited number of configurations for DG2,
but we still need to be reasonably sure we get a usable device (also
verifying we report the correct values for things like
probed_cpu_visible_size etc) with all the potential lmem_bar sizes that
we might expect see in the wild.
v2: Update commit message and a minor modification.(Matt)
v3: Optimised lmem bar size code and modified code to resize
bar maximum upto lmem_size instead of maximum supported size.(Nirmoy)
Cc: Matthew Auld
Cc: Nirmoy Das
Signed-off-by: Priyanka Dandamudi
---
drivers/gpu/drm/i915/gt/intel_region_lmem.c | 51 +++--
drivers/gpu/drm/i915/i915_params.c | 2 +
drivers/gpu/drm/i915/i915_params.h | 1 +
3 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 3e9ed395f408..2ed80c3f6064 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -14,6 +14,7 @@
#include "gt/intel_gt.h"
#include "gt/intel_gt_mcr.h"
#include "gt/intel_gt_regs.h"
+#include "../../../pci/pci.h"
static void _release_bars(struct pci_dev *pdev)
{
@@ -44,38 +45,38 @@ _resize_bar(struct drm_i915_private *i915, int resno,
resource_size_t size)
drm_info(>drm, "BAR%d resized to %dM\n", resno, 1 << bar_size);
}
-/* BAR size starts from 1MB - 2^20 */
-#define BAR_SIZE_SHIFT 20
-static resource_size_t
-_lmem_rebar_size(struct drm_i915_private *i915, int resno)
-{
- struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
- u32 rebar = pci_rebar_get_possible_sizes(pdev, resno);
- resource_size_t size;
-
- if (!rebar)
- return 0;
-
- size = 1ULL << (__fls(rebar) + BAR_SIZE_SHIFT);
-
- if (size <= pci_resource_len(pdev, resno))
- return 0;
-
- return size;
-}
-
#define LMEM_BAR_NUM 2
-static void i915_resize_lmem_bar(struct drm_i915_private *i915)
+static void i915_resize_lmem_bar(struct drm_i915_private *i915,
resource_size_t lmem_size)
{
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
struct pci_bus *root = pdev->bus;
struct resource *root_res;
- resource_size_t rebar_size = _lmem_rebar_size(i915, LMEM_BAR_NUM);
+ resource_size_t rebar_size;
+ resource_size_t current_size;
u32 pci_cmd;
int i;
- if (!rebar_size)
- return;
+ current_size = roundup_pow_of_two(pci_resource_len(pdev, LMEM_BAR_NUM));
+
+ if (i915->params.lmem_bar_size) {
+ resource_size_t min_bar_size = pci_rebar_size_to_bytes
+ (__ffs(pci_rebar_get_possible_sizes(pdev,
LMEM_BAR_NUM)));
+
+ rebar_size =
pci_rebar_size_to_bytes(__fls(i915->params.lmem_bar_size));
+
+ if (rebar_size == current_size)
+ return;
+
+ if (rebar_size >= roundup_pow_of_two(lmem_size) || rebar_size <
min_bar_size)
+ rebar_size = lmem_size;
+ } else {
+ rebar_size = current_size;
+
+ if (rebar_size != roundup_pow_of_two(lmem_size))
+ rebar_size = lmem_size;
+ else
+ return;
+ }
/* Find out if root bus contains 64bit memory addressing */
while (root->parent)
@@ -217,7 +218,7 @@ static struct intel_memory_region *setup_lmem(struct
intel_gt *gt)
lmem_size = intel_uncore_read64(>uncore, GEN12_GSMBASE);
}
- i915_resize_lmem_bar(i915);
+ i915_resize_lmem_bar(i915, lmem_size);
if (i915->params.lmem_size > 0) {
lmem_size = min_t(resource_size_t, lmem_size,
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index 701fbc98afa0..6fc475a5db61 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -204,6 +204,8 @@ i915_param_named_unsafe(request_timeout_ms, uint, 0600,
i915_param_named_unsafe(lmem_size, uint, 0400,
"Set the lmem size(in MiB) for each region. (default:
0, all memory)");
+i915_param_named_unsafe(lmem_bar_size, uint, 0400,
+ "Set the lmem bar size(in MiB).");
static __always_inline void _print_param(struct drm_printer *p,
const char *name,
diff --git a/drivers/gpu/drm/i915/i915_params.h
b/drivers/gpu/drm/i915/i915_params.h
index b5e7ea45d191..2733cb6cfe09 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -74,6 +74,7 @@ struct drm_printer;
param(char *, force_probe, CONFIG_DRM_I915_FORCE_PROBE, 0400) \
param(unsigned int, request_timeout_ms,
CONFIG_DRM_I915_REQUEST_TIMEOUT, CONFIG_DRM_I915_REQUEST_TIMEOUT ? 0600 : 0) \
[Intel-gfx] [PATCH v3 1/2] drm/i915: Add support for LMEM PCIe resizable bar
From: Akeem G Abodunrin
Add support for the local memory PICe resizable bar, so that
local memory can be resized to the maximum size supported by the device,
and mapped correctly to the PCIe memory bar. It is usual that GPU
devices expose only 256MB BARs primarily to be compatible with 32-bit
systems. So, those devices cannot claim larger memory BAR windows size due
to the system BIOS limitation. With this change, it would be possible to
reprogram the windows of the bridge directly above the requesting device
on the same BAR type.
v2:Moved code to gt/intel_region_lmem.c and used only
single underscore for function names.(Jani)
Signed-off-by: Akeem G Abodunrin
Signed-off-by: Michał Winiarski
Cc: Stuart Summers
Cc: Michael J Ruhl
Cc: Prathap Kumar Valsan
Cc: Jani Nikula
Signed-off-by: Priyanka Dandamudi
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gt/intel_region_lmem.c | 91 +
1 file changed, 91 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index fa7b86f83e7b..3e9ed395f408 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -15,6 +15,95 @@
#include "gt/intel_gt_mcr.h"
#include "gt/intel_gt_regs.h"
+static void _release_bars(struct pci_dev *pdev)
+{
+ int resno;
+
+ for (resno = PCI_STD_RESOURCES; resno < PCI_STD_RESOURCE_END; resno++) {
+ if (pci_resource_len(pdev, resno))
+ pci_release_resource(pdev, resno);
+ }
+}
+
+static void
+_resize_bar(struct drm_i915_private *i915, int resno, resource_size_t size)
+{
+ struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+ int bar_size = pci_rebar_bytes_to_size(size);
+ int ret;
+
+ _release_bars(pdev);
+
+ ret = pci_resize_resource(pdev, resno, bar_size);
+ if (ret) {
+ drm_info(>drm, "Failed to resize BAR%d to %dM (%pe)\n",
+resno, 1 << bar_size, ERR_PTR(ret));
+ return;
+ }
+
+ drm_info(>drm, "BAR%d resized to %dM\n", resno, 1 << bar_size);
+}
+
+/* BAR size starts from 1MB - 2^20 */
+#define BAR_SIZE_SHIFT 20
+static resource_size_t
+_lmem_rebar_size(struct drm_i915_private *i915, int resno)
+{
+ struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+ u32 rebar = pci_rebar_get_possible_sizes(pdev, resno);
+ resource_size_t size;
+
+ if (!rebar)
+ return 0;
+
+ size = 1ULL << (__fls(rebar) + BAR_SIZE_SHIFT);
+
+ if (size <= pci_resource_len(pdev, resno))
+ return 0;
+
+ return size;
+}
+
+#define LMEM_BAR_NUM 2
+static void i915_resize_lmem_bar(struct drm_i915_private *i915)
+{
+ struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+ struct pci_bus *root = pdev->bus;
+ struct resource *root_res;
+ resource_size_t rebar_size = _lmem_rebar_size(i915, LMEM_BAR_NUM);
+ u32 pci_cmd;
+ int i;
+
+ if (!rebar_size)
+ return;
+
+ /* Find out if root bus contains 64bit memory addressing */
+ while (root->parent)
+ root = root->parent;
+
+ pci_bus_for_each_resource(root, root_res, i) {
+ if (root_res && root_res->flags & (IORESOURCE_MEM |
+ IORESOURCE_MEM_64) && root_res->start >
0x1ull)
+ break;
+ }
+
+ /* pci_resize_resource will fail anyways */
+ if (!root_res) {
+ drm_info(>drm, "Can't resize LMEM BAR - platform support
is missing\n");
+ return;
+ }
+
+ /* First disable PCI memory decoding references */
+ pci_read_config_dword(pdev, PCI_COMMAND, _cmd);
+ pci_write_config_dword(pdev, PCI_COMMAND,
+ pci_cmd & ~PCI_COMMAND_MEMORY);
+
+ _resize_bar(i915, LMEM_BAR_NUM, rebar_size);
+
+ pci_assign_unassigned_bus_resources(pdev->bus);
+ pci_write_config_dword(pdev, PCI_COMMAND, pci_cmd);
+}
+
static int
region_lmem_release(struct intel_memory_region *mem)
{
@@ -128,6 +217,8 @@ static struct intel_memory_region *setup_lmem(struct
intel_gt *gt)
lmem_size = intel_uncore_read64(>uncore, GEN12_GSMBASE);
}
+ i915_resize_lmem_bar(i915);
+
if (i915->params.lmem_size > 0) {
lmem_size = min_t(resource_size_t, lmem_size,
mul_u32_u32(i915->params.lmem_size, SZ_1M));
--
2.27.0
[Intel-gfx] [PATCH v3 0/2] Add support for LMEM PCIe resizable bar
From: Priyanka Dandamudi Added support to resize the bar to maximum supported. Also, added new modparam lmem_bar_size which can resize the bar to one of the supported sizes. Akeem G Abodunrin (1): drm/i915: Add support for LMEM PCIe resizable bar Priyanka Dandamudi (1): drm/i915: Add lmem_bar_size modparam drivers/gpu/drm/i915/gt/intel_region_lmem.c | 92 + drivers/gpu/drm/i915/i915_params.c | 2 + drivers/gpu/drm/i915/i915_params.h | 1 + 3 files changed, 95 insertions(+) -- 2.27.0
[Intel-gfx] [PATCH i-g-t] tests/i915_query: tweak the unallocated tracking test
Make sure we always print the seed. Also make sure we use the correct
helpers for the igt_assert when comparing values of type uint64.
Signed-off-by: Matthew Auld
Cc: Nirmoy Das
---
tests/i915/i915_query.c | 36 +++-
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/tests/i915/i915_query.c b/tests/i915/i915_query.c
index 840b4864..4abd3edc 100644
--- a/tests/i915/i915_query.c
+++ b/tests/i915/i915_query.c
@@ -784,9 +784,11 @@ static void fill_unallocated(int fd, struct
drm_i915_query_item *item, int idx,
struct igt_list_head handles;
uint32_t num_handles;
uint64_t rem, total;
+ unsigned int seed;
int id;
- srand(time(NULL));
+ seed = time(NULL);
+ srand(seed);
IGT_INIT_LIST_HEAD();
@@ -829,28 +831,28 @@ static void fill_unallocated(int fd, struct
drm_i915_query_item *item, int idx,
upload(fd, , num_handles);
+ igt_debug("fill completed with seed=%u, cpu_access=%d, idx=%d,
total=%"PRIu64"KiB, num_handles=%u\n",
+ seed, cpu_access, idx, total >> 10, num_handles);
+
old_info = new_info;
memset(regions, 0, item->length);
i915_query_items(fd, item, 1);
new_info = regions->regions[idx];
- igt_assert_lte(new_info.unallocated_size,
- new_info.probed_size - total);
- igt_assert_lt(new_info.unallocated_size, old_info.unallocated_size);
+ igt_assert_lte_u64(new_info.unallocated_size,
+ new_info.probed_size - total);
+ igt_assert_lt_u64(new_info.unallocated_size, old_info.unallocated_size);
if (new_info.probed_cpu_visible_size ==
new_info.probed_size) { /* full BAR */
- igt_assert_eq(new_info.unallocated_cpu_visible_size,
- new_info.unallocated_size);
+ igt_assert_eq_u64(new_info.unallocated_cpu_visible_size,
+ new_info.unallocated_size);
} else if (cpu_access) {
- igt_assert_lt(new_info.unallocated_cpu_visible_size,
- old_info.unallocated_cpu_visible_size);
- igt_assert_lte(new_info.unallocated_cpu_visible_size,
- new_info.probed_cpu_visible_size - total);
+ igt_assert_lt_u64(new_info.unallocated_cpu_visible_size,
+ old_info.unallocated_cpu_visible_size);
+ igt_assert_lte_u64(new_info.unallocated_cpu_visible_size,
+ new_info.probed_cpu_visible_size - total);
}
- igt_debug("fill completed with idx=%d, total=%"PRIu64"KiB,
num_handles=%u\n",
- idx, total >> 10, num_handles);
-
igt_list_for_each_entry_safe(iter, tmp, , link) {
gem_close(fd, iter->handle);
free(iter);
@@ -863,11 +865,11 @@ static void fill_unallocated(int fd, struct
drm_i915_query_item *item, int idx,
i915_query_items(fd, item, 1);
new_info = regions->regions[idx];
- igt_assert(new_info.unallocated_size >=
- old_info.unallocated_size + total);
+ igt_assert_lt_u64(old_info.unallocated_size + total,
+ new_info.unallocated_size);
if (cpu_access)
- igt_assert(new_info.unallocated_cpu_visible_size >=
- old_info.unallocated_cpu_visible_size + total);
+ igt_assert_lt_u64(old_info.unallocated_cpu_visible_size + total,
+ new_info.unallocated_cpu_visible_size);
}
static void test_query_regions_unallocated(int fd)
--
2.36.1
[Intel-gfx] [PATCH 0/3] drm/i915: Apply waitboosting before fence wait
Waitboost is a heuristic that detects latency sensitive workloads waiting for
the results from previous execution. The wait can be seen as GPU
under-utilisation by RPS, Render Power State management, which might lower the
GPU frequency to save power. Limiting the frequency means more waiting for
results, which is undesirable for submissions with tight time constraints.
To circumvent this, with waitboost we iteratively check the list of fences
during gem_wait to see if any of them is stalled waiting for GPU. If such is
found, and the request hasn't yet started its execution, we temporarily bump up
the GPU frequency, so we get the required results as soon as possible.
Commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv workaround") changes
the fences order and how they are iterated. Under this new scheme, we would wait
on each fence that starts executing, rendering them not suitable for waitboost.
To avoid situation like this, inspect the entire list of fences dma-resv
earlier, before gem_wait, instead of sequentially waiting for each of them,
applying the boost when needed.
Test-with: 20220705103551.3720180-1-karolina.drob...@intel.com
Chris Wilson (3):
drm/i915/gem: Look for waitboosting across the whole object prior to
individual waits
drm/i915: Bump GT idling delay to 2 jiffies
drm/i915/gt: Only kick the signal worker if there's been an update
drivers/gpu/drm/i915/gem/i915_gem_wait.c| 35 +
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 +-
drivers/gpu/drm/i915/i915_active.c | 2 +-
3 files changed, 38 insertions(+), 2 deletions(-)
--
2.25.1
Re: [Intel-gfx] [PATCH i-g-t] tests/i915/kms_big_fb: trigger async flip with a dummy flip
On 6/28/2022 4:34 PM, Arun R Murthy wrote:
In oder to trigger the async flip, a dummy flip is required after sync
flip so as to update the watermarks for async in KMD which happens as
part of this dummy flip. Thereafter async memory update will act as a
trigger register.
Capturing the CRC is done after the async flip as async flip at some
times can consume fairly a vblank period time.
Signed-off-by: Arun R Murthy
---
tests/i915/kms_big_fb.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c
index d50fde45..6caf3c31 100644
--- a/tests/i915/kms_big_fb.c
+++ b/tests/i915/kms_big_fb.c
@@ -465,7 +465,7 @@ static bool test_pipe(data_t *data)
static bool
max_hw_stride_async_flip_test(data_t *data)
{
- uint32_t ret, startframe;
+ uint32_t ret, frame;
const uint32_t w = data->output->config.default_mode.hdisplay,
h = data->output->config.default_mode.vdisplay;
igt_plane_t *primary;
@@ -519,7 +519,19 @@ max_hw_stride_async_flip_test(data_t *data)
DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
igt_wait_for_vblank(data->drm_fd, data->display.pipes[primary->pipe->pipe].crtc_offset);
- startframe = kmstest_get_vblank(data->drm_fd, data->pipe, 0) +
1;
+ /*
+* In older platforms (<= Gen10), async address update bit is
double buffered.
+* So flip timestamp can be verified only from the second flip.
+* The first async flip just enables the async address update.
+* In platforms greater than DISPLAY13 the first async flip
will be discarded
+* in order to change the watermark levels as per the
optimization. Hence the
+* subsequent async flips will actually do the asynchronous
flips.
+*/
+ ret = drmModePageFlip(data->drm_fd,
data->output->config.crtc->crtc_id,
+
data->big_fb_flip[i].fb_id,
+ DRM_MODE_PAGE_FLIP_ASYNC,
NULL);
+ igt_wait_for_vblank(data->drm_fd,
data->display.pipes[primary->pipe->pipe].crtc_offset);
+ frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) + 1;
Hi,
Should this be added inside a gen check similar to kms_async_flips?
for (int j = 0; j < 2; j++) {
do {
@@ -528,23 +540,20 @@ max_hw_stride_async_flip_test(data_t *data)
DRM_MODE_PAGE_FLIP_ASYNC,
NULL);
} while (ret == -EBUSY);
igt_assert(ret == 0);
+ igt_pipe_crc_get_for_frame(data->drm_fd, data->pipe_crc,
+ frame, _crc);
+ frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) + 1;
do {
ret = drmModePageFlip(data->drm_fd,
data->output->config.crtc->crtc_id,
data->big_fb.fb_id,
DRM_MODE_PAGE_FLIP_ASYNC,
NULL);
} while (ret == -EBUSY);
igt_assert(ret == 0);
+ igt_pipe_crc_get_for_frame(data->drm_fd, data->pipe_crc,
+ frame, _crc);
We should be moving this IMHO. By waiting for vblank after each async
flip to capture CRC, what is the intended result? Seems to be completely
changing the existing test logic.
Also, seems like we are overwriting the CRC captured for j = 0, as
comparison is done outside this loop. Is this done on purpose?
Thanks,
Karthik.B.S
}
- igt_pipe_crc_get_for_frame(data->drm_fd, data->pipe_crc,
- startframe, _crc);
- igt_pipe_crc_get_for_frame(data->drm_fd, data->pipe_crc,
- startframe + 1, _crc);
-
- igt_assert_f(kmstest_get_vblank(data->drm_fd, data->pipe, 0) -
-startframe == 1, "lost frames\n");
-
igt_assert_f(igt_check_crc_equal(_crc,
_crc)^(i^1),
"CRC failure with async flip, crc %s match for checked
round\n",
i?"should":"shouldn't");
Re: [Intel-gfx] [PATCH i-g-t] tests/i915/kms_big_fb: trigger async flip with a dummy flip
Hi,
On 5.7.2022 12.49, Karthik B S wrote:
On 7/5/2022 3:08 PM, Murthy, Arun R wrote:
On 6/28/2022 4:34 PM, Arun R Murthy wrote:
In oder to trigger the async flip, a dummy flip is required after sync
flip so as to update the watermarks for async in KMD which happens as
part of this dummy flip. Thereafter async memory update will act as a
trigger register.
Capturing the CRC is done after the async flip as async flip at some
times can consume fairly a vblank period time.
Signed-off-by: Arun R Murthy
---
tests/i915/kms_big_fb.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c index
d50fde45..6caf3c31 100644
--- a/tests/i915/kms_big_fb.c
+++ b/tests/i915/kms_big_fb.c
@@ -465,7 +465,7 @@ static bool test_pipe(data_t *data)
static bool
max_hw_stride_async_flip_test(data_t *data)
{
- uint32_t ret, startframe;
+ uint32_t ret, frame;
const uint32_t w = data->output->config.default_mode.hdisplay,
h = data->output->config.default_mode.vdisplay;
igt_plane_t *primary;
@@ -519,7 +519,19 @@ max_hw_stride_async_flip_test(data_t *data)
DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
igt_wait_for_vblank(data->drm_fd, data-
display.pipes[primary->pipe->pipe].crtc_offset);
- startframe = kmstest_get_vblank(data->drm_fd, data->pipe,
0) + 1;
+ /*
+ * In older platforms (<= Gen10), async address update bit is
double buffered.
+ * So flip timestamp can be verified only from the second
flip.
+ * The first async flip just enables the async address update.
+ * In platforms greater than DISPLAY13 the first async flip
will
be discarded
+ * in order to change the watermark levels as per the
optimization. Hence the
+ * subsequent async flips will actually do the asynchronous
flips.
+ */
+ ret = drmModePageFlip(data->drm_fd, data->output-
config.crtc->crtc_id,
+ data->big_fb_flip[i].fb_id,
+
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
+ igt_wait_for_vblank(data->drm_fd, data-
display.pipes[primary->pipe->pipe].crtc_offset);
+ frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) +
1;
Hi,
Should this be added inside a gen check similar to kms_async_flips?
Yes sorry missed it!
for (int j = 0; j < 2; j++) {
do {
@@ -528,23 +540,20 @@ max_hw_stride_async_flip_test(data_t *data)
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
} while (ret == -EBUSY);
igt_assert(ret == 0);
+ igt_pipe_crc_get_for_frame(data->drm_fd, data-
pipe_crc,
+ frame, _crc);
+ frame = kmstest_get_vblank(data->drm_fd, data-
pipe, 0) + 1;
do {
ret = drmModePageFlip(data->drm_fd, data-
output->config.crtc->crtc_id,
data->big_fb.fb_id,
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
} while (ret == -EBUSY);
igt_assert(ret == 0);
+ igt_pipe_crc_get_for_frame(data->drm_fd, data-
pipe_crc,
+ frame, _crc);
We should be moving this IMHO. By waiting for vblank after each async
flip
to capture CRC, what is the intended result? Seems to be completely
changing the existing test logic.
We are getting the vblank count and based on that getting the crc. Now
we know for async flip at
certain times it can consume a time equivalent to a vblank period. So
in those scenarios getting crc
based on the vblank goes for a toss. Hence capturing the vblank start
time stamp at the beginning
of each flip.
Hi,
But what is the the reference CRC we're expecting? The original logic is
designed to match on one iteration and mismatch on the next using a
combination of fb's by using async flips. But with these changes that
whole logic goes for a toss?
Also, seems like we are overwriting the CRC captured for j = 0, as
comparison
is done outside this loop. Is this done on purpose?
Now with the changing the vblank start frame capture being added
before the async flip, CRC can
be captured outside the loop as well, but makes no sense. To maintain
this order moving the CRC
Capture also after each frame.
The CRC comparison is still outside the loop, so no point capturing
CRC's if we finally discard it anyway?
I think generally the idea Arun is changing is correct but he's missed
how the test work.
I see there's Ville's change on kernel side for display_ver >=13 first
async_flip is unconditionally (intentionally) missed, this is at
intel_plane_do_async_flip(..) so this test will need adjustment
What Arun seem to have missed is on test those nested loops how they
work, that part probably should've originally been commented in code bit
better.
On original code there's after loop for j two time
igt_pipe_crc_get_for_frame(..), first will capture crc from duration of
loop of j, second
Re: [Intel-gfx] [PATCH i-g-t] tests/i915/kms_big_fb: trigger async flip with a dummy flip
> On 5.7.2022 12.49, Karthik B S wrote:
> > On 7/5/2022 3:08 PM, Murthy, Arun R wrote:
> >>> On 6/28/2022 4:34 PM, Arun R Murthy wrote:
> In oder to trigger the async flip, a dummy flip is required after
> sync flip so as to update the watermarks for async in KMD which
> happens as part of this dummy flip. Thereafter async memory update
> will act as a trigger register.
> Capturing the CRC is done after the async flip as async flip at
> some times can consume fairly a vblank period time.
>
> Signed-off-by: Arun R Murthy
> ---
> tests/i915/kms_big_fb.c | 29 +++--
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c
> index
> d50fde45..6caf3c31 100644
> --- a/tests/i915/kms_big_fb.c
> +++ b/tests/i915/kms_big_fb.c
> @@ -465,7 +465,7 @@ static bool test_pipe(data_t *data)
> static bool
> max_hw_stride_async_flip_test(data_t *data)
> {
> - uint32_t ret, startframe;
> + uint32_t ret, frame;
> const uint32_t w =
> data->output->config.default_mode.hdisplay,
> h = data->output->config.default_mode.vdisplay;
> igt_plane_t *primary;
> @@ -519,7 +519,19 @@ max_hw_stride_async_flip_test(data_t *data)
>
> >>> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> igt_wait_for_vblank(data->drm_fd, data-
> display.pipes[primary->pipe->pipe].crtc_offset);
> - startframe = kmstest_get_vblank(data->drm_fd, data->pipe,
> >>> 0) + 1;
> + /*
> + * In older platforms (<= Gen10), async address update bit
> +is
> >>> double buffered.
> + * So flip timestamp can be verified only from the second
> flip.
> + * The first async flip just enables the async address update.
> + * In platforms greater than DISPLAY13 the first async
> +flip
> will
> >>> be discarded
> + * in order to change the watermark levels as per the
> >>> optimization. Hence the
> + * subsequent async flips will actually do the
> +asynchronous
> >>> flips.
> + */
> + ret = drmModePageFlip(data->drm_fd, data->output-
> config.crtc->crtc_id,
> + data->big_fb_flip[i].fb_id,
> +
> >>> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> + igt_wait_for_vblank(data->drm_fd, data-
> display.pipes[primary->pipe->pipe].crtc_offset);
> + frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) +
> >>> 1;
> >>>
> >>> Hi,
> >>>
> >>> Should this be added inside a gen check similar to kms_async_flips?
> >> Yes sorry missed it!
> >>
> for (int j = 0; j < 2; j++) {
> do {
> @@ -528,23 +540,20 @@ max_hw_stride_async_flip_test(data_t
> *data)
>
> >>> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> } while (ret == -EBUSY);
> igt_assert(ret == 0);
> + igt_pipe_crc_get_for_frame(data->drm_fd, data-
> pipe_crc,
> + frame, _crc);
>
> + frame = kmstest_get_vblank(data->drm_fd, data-
> pipe, 0) + 1;
> do {
> ret = drmModePageFlip(data->drm_fd, data-
> output->config.crtc->crtc_id,
> data->big_fb.fb_id,
>
> >>> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> } while (ret == -EBUSY);
> igt_assert(ret == 0);
> + igt_pipe_crc_get_for_frame(data->drm_fd, data-
> pipe_crc,
> + frame, _crc);
> >>> We should be moving this IMHO. By waiting for vblank after each
> >>> async flip to capture CRC, what is the intended result? Seems to be
> >>> completely changing the existing test logic.
> >> We are getting the vblank count and based on that getting the crc.
> >> Now we know for async flip at certain times it can consume a time
> >> equivalent to a vblank period. So in those scenarios getting crc
> >> based on the vblank goes for a toss. Hence capturing the vblank start
> >> time stamp at the beginning of each flip.
> >
> > Hi,
> >
> > But what is the the reference CRC we're expecting? The original logic
> > is designed to match on one iteration and mismatch on the next using a
> > combination of fb's by using async flips. But with these changes that
> > whole logic goes for a toss?
> >
> >>
> >>> Also, seems like we are overwriting the CRC captured for j = 0, as
> >>> comparison is done outside this loop. Is this done on purpose?
> >> Now with the changing the vblank start frame capture being added
> >> before the async flip, CRC can be captured outside the loop as well,
> >> but makes no sense. To maintain this order moving the CRC Capture
> >> also after each frame.
> >
> > The CRC comparison is
[Intel-gfx] [PATCH i-g-t] tests/gem_exec_capture: adjust the pi subtest
This was broken by the kernel change: 71b1669ea9bd ("drm/i915/uapi:
tweak error capture on recoverable contexts"). Update the test to use
non-recoverable context if needed.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6350
Signed-off-by: Matthew Auld
Cc: Nirmoy Das
---
tests/i915/gem_exec_capture.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index c2639082..a25f529b 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -721,6 +721,15 @@ static void prioinv(int fd, int dir, const intel_ctx_t
*ctx,
ctx2 = intel_ctx_create_all_physical(fd);
igt_assert(ctx2);
+ if (needs_recoverable_ctx(fd)) {
+ struct drm_i915_gem_context_param param = {
+ .ctx_id = ctx2->id,
+ .param = I915_CONTEXT_PARAM_RECOVERABLE,
+ .value = 0,
+ };
+
+ gem_context_set_param(fd, );
+ }
intel_allocator_init();
/* Reopen the allocator in the new process. */
--
2.36.1
[Intel-gfx] [PATCH 1/3] drm/i915/gem: Look for waitboosting across the whole object prior to individual waits
From: Chris Wilson
We employ a "waitboost" heuristic to detect when userspace is stalled
waiting for results from earlier execution. Under latency sensitive work
mixed between the gpu/cpu, the GPU is typically under-utilised and so
RPS sees that low utilisation as a reason to downclock the frequency,
causing longer stalls and lower throughput. The user left waiting for
the results is not impressed.
On applying commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv
workaround") it was observed that deinterlacing h264 on Haswell
performance dropped by 2-5x. The reason being that the natural workload
was not intense enough to trigger RPS (using HW evaluation intervals) to
upclock, and so it was depending on waitboosting for the throughput.
Commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv workaround")
changes the composition of dma-resv from keeping a single write fence +
multiple read fences, to a single array of multiple write and read
fences (a maximum of one pair of write/read fences per context). The
iteration order was also changed implicitly from all-read fences then
the single write fence, to a mix of write fences followed by read
fences. It is that ordering change that belied the fragility of
waitboosting.
Currently, a waitboost is inspected at the point of waiting on an
outstanding fence. If the GPU is backlogged such that we haven't yet
stated the request we need to wait on, we force the GPU to upclock until
the completion of that request. By changing the order in which we waited
upon requests, we ended up waiting on those requests in sequence and as
such we saw that each request was already started and so not a suitable
candidate for waitboosting.
Instead of asking whether to boost each fence in turn, we can look at
whether boosting is required for the dma-resv ensemble prior to waiting
on any fence, making the heuristic more robust to the order in which
fences are stored in the dma-resv.
Reported-by: Thomas Voegtle
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6284
Fixes: 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv workaround")
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Signed-off-by: Karolina Drobnik
Tested-by: Thomas Voegtle
---
drivers/gpu/drm/i915/gem/i915_gem_wait.c | 35
1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
index 319936f91ac5..3fbb464746e1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
@@ -9,6 +9,7 @@
#include
#include "gt/intel_engine.h"
+#include "gt/intel_rps.h"
#include "i915_gem_ioctls.h"
#include "i915_gem_object.h"
@@ -31,6 +32,38 @@ i915_gem_object_wait_fence(struct dma_fence *fence,
timeout);
}
+static void
+i915_gem_object_boost(struct dma_resv *resv, unsigned int flags)
+{
+ struct dma_resv_iter cursor;
+ struct dma_fence *fence;
+
+ /*
+* Prescan all fences for potential boosting before we begin waiting.
+*
+* When we wait, we wait on outstanding fences serially. If the
+* dma-resv contains a sequence such as 1:1, 1:2 instead of a reduced
+* form 1:2, then as we look at each wait in turn we see that each
+* request is currently executing and not worthy of boosting. But if
+* we only happen to look at the final fence in the sequence (because
+* of request coalescing or splitting between read/write arrays by
+* the iterator), then we would boost. As such our decision to boost
+* or not is delicately balanced on the order we wait on fences.
+*
+* So instead of looking for boosts sequentially, look for all boosts
+* upfront and then wait on the outstanding fences.
+*/
+
+ dma_resv_iter_begin(, resv,
+ dma_resv_usage_rw(flags & I915_WAIT_ALL));
+ dma_resv_for_each_fence_unlocked(, fence) {
+ if (dma_fence_is_i915(fence) &&
+ !i915_request_started(to_request(fence)))
+ intel_rps_boost(to_request(fence));
+ }
+ dma_resv_iter_end();
+}
+
static long
i915_gem_object_wait_reservation(struct dma_resv *resv,
unsigned int flags,
@@ -40,6 +73,8 @@ i915_gem_object_wait_reservation(struct dma_resv *resv,
struct dma_fence *fence;
long ret = timeout ?: 1;
+ i915_gem_object_boost(resv, flags);
+
dma_resv_iter_begin(, resv,
dma_resv_usage_rw(flags & I915_WAIT_ALL));
dma_resv_for_each_fence_unlocked(, fence) {
--
2.25.1
Re: [Intel-gfx] [PATCH i-g-t] tests/i915/kms_big_fb: trigger async flip with a dummy flip
On 7/5/2022 3:08 PM, Murthy, Arun R wrote:
On 6/28/2022 4:34 PM, Arun R Murthy wrote:
In oder to trigger the async flip, a dummy flip is required after sync
flip so as to update the watermarks for async in KMD which happens as
part of this dummy flip. Thereafter async memory update will act as a
trigger register.
Capturing the CRC is done after the async flip as async flip at some
times can consume fairly a vblank period time.
Signed-off-by: Arun R Murthy
---
tests/i915/kms_big_fb.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c index
d50fde45..6caf3c31 100644
--- a/tests/i915/kms_big_fb.c
+++ b/tests/i915/kms_big_fb.c
@@ -465,7 +465,7 @@ static bool test_pipe(data_t *data)
static bool
max_hw_stride_async_flip_test(data_t *data)
{
- uint32_t ret, startframe;
+ uint32_t ret, frame;
const uint32_t w = data->output->config.default_mode.hdisplay,
h = data->output->config.default_mode.vdisplay;
igt_plane_t *primary;
@@ -519,7 +519,19 @@ max_hw_stride_async_flip_test(data_t *data)
DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
igt_wait_for_vblank(data->drm_fd, data-
display.pipes[primary->pipe->pipe].crtc_offset);
- startframe = kmstest_get_vblank(data->drm_fd, data->pipe,
0) + 1;
+ /*
+* In older platforms (<= Gen10), async address update bit is
double buffered.
+* So flip timestamp can be verified only from the second flip.
+* The first async flip just enables the async address update.
+* In platforms greater than DISPLAY13 the first async flip will
be discarded
+* in order to change the watermark levels as per the
optimization. Hence the
+* subsequent async flips will actually do the asynchronous
flips.
+*/
+ ret = drmModePageFlip(data->drm_fd, data->output-
config.crtc->crtc_id,
+
data->big_fb_flip[i].fb_id,
+
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
+ igt_wait_for_vblank(data->drm_fd, data-
display.pipes[primary->pipe->pipe].crtc_offset);
+ frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) +
1;
Hi,
Should this be added inside a gen check similar to kms_async_flips?
Yes sorry missed it!
for (int j = 0; j < 2; j++) {
do {
@@ -528,23 +540,20 @@ max_hw_stride_async_flip_test(data_t *data)
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
} while (ret == -EBUSY);
igt_assert(ret == 0);
+ igt_pipe_crc_get_for_frame(data->drm_fd, data-
pipe_crc,
+ frame, _crc);
+ frame = kmstest_get_vblank(data->drm_fd, data-
pipe, 0) + 1;
do {
ret = drmModePageFlip(data->drm_fd, data-
output->config.crtc->crtc_id,
data->big_fb.fb_id,
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
} while (ret == -EBUSY);
igt_assert(ret == 0);
+ igt_pipe_crc_get_for_frame(data->drm_fd, data-
pipe_crc,
+ frame, _crc);
We should be moving this IMHO. By waiting for vblank after each async flip
to capture CRC, what is the intended result? Seems to be completely
changing the existing test logic.
We are getting the vblank count and based on that getting the crc. Now we know
for async flip at
certain times it can consume a time equivalent to a vblank period. So in those
scenarios getting crc
based on the vblank goes for a toss. Hence capturing the vblank start time
stamp at the beginning
of each flip.
Hi,
But what is the the reference CRC we're expecting? The original logic is
designed to match on one iteration and mismatch on the next using a
combination of fb's by using async flips. But with these changes that
whole logic goes for a toss?
Also, seems like we are overwriting the CRC captured for j = 0, as comparison
is done outside this loop. Is this done on purpose?
Now with the changing the vblank start frame capture being added before the
async flip, CRC can
be captured outside the loop as well, but makes no sense. To maintain this
order moving the CRC
Capture also after each frame.
The CRC comparison is still outside the loop, so no point capturing
CRC's if we finally discard it anyway?
Thanks,
Karthik.B.S
Thanks and Regards,
Arun R Murthy
Re: [Intel-gfx] [PATCH i-g-t] tests/i915/kms_big_fb: trigger async flip with a dummy flip
> On 7/5/2022 3:08 PM, Murthy, Arun R wrote:
> >> On 6/28/2022 4:34 PM, Arun R Murthy wrote:
> >>> In oder to trigger the async flip, a dummy flip is required after
> >>> sync flip so as to update the watermarks for async in KMD which
> >>> happens as part of this dummy flip. Thereafter async memory update
> >>> will act as a trigger register.
> >>> Capturing the CRC is done after the async flip as async flip at some
> >>> times can consume fairly a vblank period time.
> >>>
> >>> Signed-off-by: Arun R Murthy
> >>> ---
> >>>tests/i915/kms_big_fb.c | 29 +++--
> >>>1 file changed, 19 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c index
> >>> d50fde45..6caf3c31 100644
> >>> --- a/tests/i915/kms_big_fb.c
> >>> +++ b/tests/i915/kms_big_fb.c
> >>> @@ -465,7 +465,7 @@ static bool test_pipe(data_t *data)
> >>>static bool
> >>>max_hw_stride_async_flip_test(data_t *data)
> >>>{
> >>> - uint32_t ret, startframe;
> >>> + uint32_t ret, frame;
> >>> const uint32_t w = data->output->config.default_mode.hdisplay,
> >>> h = data->output->config.default_mode.vdisplay;
> >>> igt_plane_t *primary;
> >>> @@ -519,7 +519,19 @@ max_hw_stride_async_flip_test(data_t *data)
> >>>
> >> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> >>> igt_wait_for_vblank(data->drm_fd, data-
> >>> display.pipes[primary->pipe->pipe].crtc_offset);
> >>> - startframe = kmstest_get_vblank(data->drm_fd, data->pipe,
> >> 0) + 1;
> >>> + /*
> >>> + * In older platforms (<= Gen10), async address update bit is
> >> double buffered.
> >>> + * So flip timestamp can be verified only from the second flip.
> >>> + * The first async flip just enables the async address update.
> >>> + * In platforms greater than DISPLAY13 the first async flip will
> >> be discarded
> >>> + * in order to change the watermark levels as per the
> >> optimization. Hence the
> >>> + * subsequent async flips will actually do the asynchronous
> >> flips.
> >>> + */
> >>> + ret = drmModePageFlip(data->drm_fd, data->output-
> >>> config.crtc->crtc_id,
> >>> +
> >>> data->big_fb_flip[i].fb_id,
> >>> +
> >> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> >>> + igt_wait_for_vblank(data->drm_fd, data-
> >>> display.pipes[primary->pipe->pipe].crtc_offset);
> >>> + frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) +
> >> 1;
> >>
> >> Hi,
> >>
> >> Should this be added inside a gen check similar to kms_async_flips?
> > Yes sorry missed it!
> >
> >>> for (int j = 0; j < 2; j++) {
> >>> do {
> >>> @@ -528,23 +540,20 @@ max_hw_stride_async_flip_test(data_t *data)
> >>>
> >> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> >>> } while (ret == -EBUSY);
> >>> igt_assert(ret == 0);
> >>> + igt_pipe_crc_get_for_frame(data->drm_fd, data-
> >>> pipe_crc,
> >>> +frame, _crc);
> >>>
> >>> + frame = kmstest_get_vblank(data->drm_fd, data-
> >>> pipe, 0) + 1;
> >>> do {
> >>> ret = drmModePageFlip(data->drm_fd,
> >>> data-
> >>> output->config.crtc->crtc_id,
> >>>
> >>> data->big_fb.fb_id,
> >>>
> >> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> >>> } while (ret == -EBUSY);
> >>> igt_assert(ret == 0);
> >>> + igt_pipe_crc_get_for_frame(data->drm_fd, data-
> >>> pipe_crc,
> >>> +frame, _crc);
> >> We should be moving this IMHO. By waiting for vblank after each async
> >> flip to capture CRC, what is the intended result? Seems to be
> >> completely changing the existing test logic.
> > We are getting the vblank count and based on that getting the crc. Now
> > we know for async flip at certain times it can consume a time
> > equivalent to a vblank period. So in those scenarios getting crc based
> > on the vblank goes for a toss. Hence capturing the vblank start time stamp
> at the beginning of each flip.
>
> Hi,
>
> But what is the the reference CRC we're expecting? The original logic is
> designed to match on one iteration and mismatch on the next using a
> combination of fb's by using async flips. But with these changes that whole
> logic goes for a toss?
>
But I see this logic doesn’t work. We cannot rely on the CRC based on the
vblank time frame for each flips. Since this is a async flip and at times the
async flip can consume a vblank time period for flipping. So the CRC captured
based on starteframe+1 logic is not right.
Thanks and Regards,
Arun R Murthy
[Intel-gfx] [PATCH 3/3] drm/i915/gt: Only kick the signal worker if there's been an update
From: Chris Wilson
One impact of commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove
dma_resv workaround") is that it stores many, many more fences. Whereas
adding an exclusive fence used to remove the shared fence list, that
list is now preserved and the write fences included into the list. Not
just a single write fence, but now a write/read fence per context. That
causes us to have to track more fences than before (albeit half of those
are redundant), and we trigger more interrupts for multi-engine
workloads.
As part of reducing the impact from handling more signaling, we observe
we only need to kick the signal worker after adding a fence iff we have
good cause to believe that there is work to be done in processing the
fence i.e. we either need to enable the interrupt or the request is
already complete but we don't know if we saw the interrupt and so need
to check signaling.
References: 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv workaround")
Signed-off-by: Chris Wilson
Signed-off-by: Karolina Drobnik
---
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index 9dc9dccf7b09..ecc990ec1b95 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -399,7 +399,8 @@ static void insert_breadcrumb(struct i915_request *rq)
* the request as it may have completed and raised the interrupt as
* we were attaching it into the lists.
*/
- irq_work_queue(>irq_work);
+ if (!b->irq_armed || __i915_request_is_complete(rq))
+ irq_work_queue(>irq_work);
}
bool i915_request_enable_breadcrumb(struct i915_request *rq)
--
2.25.1
Re: [Intel-gfx] [RFC 10/10] drm/i915/vm_bind: Fix vm->vm_bind_mutex and vm->mutex nesting
On Fri, 2022-07-01 at 15:50 -0700, Niranjana Vishwanathapura wrote:
> VM_BIND functionality maintain that vm->vm_bind_mutex will never be
> taken
> while holding vm->mutex.
> However, while closing 'vm', vma is destroyed while holding vm-
> >mutex.
> But vma releasing needs to take vm->vm_bind_mutex in order to delete
> vma
> from the vm_bind_list. To avoid this, destroy the vma outside vm-
> >mutex
> while closing the 'vm'.
>
> Signed-off-by: Niranjana Vishwanathapura
First, when introducing a new feature like this, we should not need to
end the series with "Fix.." patches like this, rather whatever needs to
be fixed should be fixed where the code was introduced.
Second, an analogy whith linux kernel CPU mapping, could we instead
think of the vm_bind_lock being similar to the mmap_lock, and the
vm_mutex being similar to the i_mmap_lock, the former being used for VA
manipulation and the latter when attaching / removing the backing store
from the VA?
Then we would not need to take the vm_bind_lock from vma destruction
since the VA would already have been reclaimed at that point. For vm
destruction here we'd loop over all relevant vm bind VAs under the
vm_bind lock and call vm_unbind? Would that work?
/Thomas
>
> ---
> drivers/gpu/drm/i915/gt/intel_gtt.c | 23 ++-
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
> b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index 4ab3bda644ff..4f707d0eb3ef 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -109,7 +109,8 @@ int map_pt_dma_locked(struct i915_address_space
> *vm, struct drm_i915_gem_object
> return 0;
> }
>
> -static void clear_vm_list(struct list_head *list)
> +static void clear_vm_list(struct list_head *list,
> + struct list_head *destroy_list)
> {
> struct i915_vma *vma, *vn;
>
> @@ -138,8 +139,7 @@ static void clear_vm_list(struct list_head *list)
> i915_vm_resv_get(vma->vm);
> vma->vm_ddestroy = true;
> } else {
> - i915_vma_destroy_locked(vma);
> - i915_gem_object_put(obj);
> + list_move_tail(>vm_link, destroy_list);
> }
>
> }
> @@ -147,16 +147,29 @@ static void clear_vm_list(struct list_head
> *list)
>
> static void __i915_vm_close(struct i915_address_space *vm)
> {
> + struct i915_vma *vma, *vn;
> + struct list_head list;
> +
> + INIT_LIST_HEAD();
> +
> mutex_lock(>mutex);
>
> - clear_vm_list(>bound_list);
> - clear_vm_list(>unbound_list);
> + clear_vm_list(>bound_list, );
> + clear_vm_list(>unbound_list, );
>
> /* Check for must-fix unanticipated side-effects */
> GEM_BUG_ON(!list_empty(>bound_list));
> GEM_BUG_ON(!list_empty(>unbound_list));
>
> mutex_unlock(>mutex);
> +
> + /* Destroy vmas outside vm->mutex */
> + list_for_each_entry_safe(vma, vn, , vm_link) {
> + struct drm_i915_gem_object *obj = vma->obj;
> +
> + i915_vma_destroy(vma);
> + i915_gem_object_put(obj);
> + }
> }
>
> /* lock the vm into the current ww, if we lock one, we lock all */
Re: [Intel-gfx] [RFC 05/10] drm/i915/vm_bind: Handle persistent vmas
On 2022-07-04 at 17:05:38 +, Zeng, Oak wrote:
>
>
> Thanks,
> Oak
>
> > -Original Message-
> > From: Intel-gfx On Behalf Of
> > Niranjana Vishwanathapura
> > Sent: July 1, 2022 6:51 PM
> > To: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> > Cc: Zanoni, Paulo R ; Hellstrom, Thomas
> > ; Auld, Matthew ;
> > Vetter, Daniel ; christian.koe...@amd.com
> > Subject: [Intel-gfx] [RFC 05/10] drm/i915/vm_bind: Handle persistent vmas
> >
> > Treat VM_BIND vmas as persistent and handle them during the request
> > submission in the execbuff path.
>
> Hi Niranjana,
>
> Is the meaning of "persistent" above persistent across all the subsequent
> execbuf ioctls?
Yes oak. Thats correct. persistent across multiple execbuf ioctls.
Regards,
Ram.
>
> Thanks,
> Oak
>
> >
> > Support eviction by maintaining a list of evicted persistent vmas for
> > rebinding
> > during next submission.
> >
> > Signed-off-by: Niranjana Vishwanathapura
> >
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_object.c| 1 +
> > drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 3 +
> > .../drm/i915/gem/i915_gem_vm_bind_object.c| 12 ++-
> > drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +
> > drivers/gpu/drm/i915/gt/intel_gtt.h | 2 +
> > drivers/gpu/drm/i915/i915_gem_gtt.h | 22 ++
> > drivers/gpu/drm/i915/i915_vma.c | 32 +++-
> > drivers/gpu/drm/i915/i915_vma.h | 78 +--
> > drivers/gpu/drm/i915/i915_vma_types.h | 23 ++
> > 9 files changed, 163 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > index ccec4055fde3..5121f02ba95c 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > @@ -38,6 +38,7 @@
> > #include "i915_gem_mman.h"
> > #include "i915_gem_object.h"
> > #include "i915_gem_ttm.h"
> > +#include "i915_gem_vm_bind.h"
> > #include "i915_memcpy.h"
> > #include "i915_trace.h"
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > index 849bf3c1061e..eaadf5a6ab09 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
> > @@ -6,6 +6,7 @@
> > #ifndef __I915_GEM_VM_BIND_H
> > #define __I915_GEM_VM_BIND_H
> >
> > +#include
> > #include "i915_drv.h"
> >
> > #define assert_vm_bind_held(vm) lockdep_assert_held(&(vm)-
> > >vm_bind_lock)
> > @@ -26,6 +27,8 @@ static inline void i915_gem_vm_bind_unlock(struct
> > i915_address_space *vm)
> > mutex_unlock(>vm_bind_lock);
> > }
> >
> > +#define assert_vm_priv_held(vm) assert_object_held((vm)->root_obj)
> > +
> > static inline int i915_gem_vm_priv_lock(struct i915_address_space *vm,
> > struct i915_gem_ww_ctx *ww)
> > {
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > index 96f139cc8060..1a8efa83547f 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
> > @@ -85,6 +85,13 @@ void i915_gem_vm_bind_remove(struct i915_vma
> > *vma, bool release_obj) {
> > assert_vm_bind_held(vma->vm);
> >
> > + spin_lock(>vm->vm_rebind_lock);
> > + if (!list_empty(>vm_rebind_link))
> > + list_del_init(>vm_rebind_link);
> > + i915_vma_set_purged(vma);
> > + i915_vma_set_freed(vma);
> > + spin_unlock(>vm->vm_rebind_lock);
> > +
> > if (!list_empty(>vm_bind_link)) {
> > list_del_init(>vm_bind_link);
> > list_del_init(>non_priv_vm_bind_link);
> > @@ -220,6 +227,7 @@ static struct i915_vma *vm_bind_get_vma(struct
> > i915_address_space *vm,
> >
> > vma->start = va->start;
> > vma->last = va->start + va->length - 1;
> > + i915_vma_set_persistent(vma);
> >
> > return vma;
> > }
> > @@ -304,8 +312,10 @@ int i915_gem_vm_bind_obj(struct
> > i915_address_space *vm,
> >
> > i915_vm_bind_put_fence(vma);
> > put_vma:
> > - if (ret)
> > + if (ret) {
> > + i915_vma_set_freed(vma);
> > i915_vma_destroy(vma);
> > + }
> >
> > i915_gem_ww_ctx_fini();
> > unlock_vm:
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
> > b/drivers/gpu/drm/i915/gt/intel_gtt.c
> > index df0a8459c3c6..55d5389b2c6c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> > @@ -293,6 +293,8 @@ void i915_address_space_init(struct
> > i915_address_space *vm, int subclass)
> > INIT_LIST_HEAD(>non_priv_vm_bind_list);
> > vm->root_obj = i915_gem_object_create_internal(vm->i915,
> > PAGE_SIZE);
> > GEM_BUG_ON(IS_ERR(vm->root_obj));
> > + INIT_LIST_HEAD(>vm_rebind_list);
> > + spin_lock_init(>vm_rebind_lock);
> > }
> >
> > void *__px_vaddr(struct drm_i915_gem_object *p)
Re: [Intel-gfx] [PATCH v3 1/2] vfio: Replace the DMA unmapping notifier with a callback
On 2022.07.04 21:59:03 -0300, Jason Gunthorpe wrote:
> Instead of having drivers register the notifier with explicit code just
> have them provide a dma_unmap callback op in their driver ops and rely on
> the core code to wire it up.
>
> Suggested-by: Christoph Hellwig
> Reviewed-by: Christoph Hellwig
> Reviewed-by: Kevin Tian
> Reviewed-by: Tony Krowiak
> Reviewed-by: Eric Farman
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/gpu/drm/i915/gvt/gvt.h| 1 -
> drivers/gpu/drm/i915/gvt/kvmgt.c | 75 ---
gvt change looks fine to me.
Reviewed-by: Zhenyu Wang
> drivers/s390/cio/vfio_ccw_ops.c | 41 ++--
> drivers/s390/cio/vfio_ccw_private.h | 2 -
> drivers/s390/crypto/vfio_ap_ops.c | 53 ++-
> drivers/s390/crypto/vfio_ap_private.h | 3 -
> drivers/vfio/vfio.c | 129 +-
> drivers/vfio/vfio.h | 3 +
> include/linux/vfio.h | 21 +
> 9 files changed, 88 insertions(+), 240 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index aee1a45da74bcb..705689e6401197 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -226,7 +226,6 @@ struct intel_vgpu {
> unsigned long nr_cache_entries;
> struct mutex cache_lock;
>
> - struct notifier_block iommu_notifier;
> atomic_t released;
>
> struct kvm_page_track_notifier_node track_node;
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index e2f6c56ab3420c..ecd5bb37b63a2a 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -729,34 +729,25 @@ int intel_gvt_set_edid(struct intel_vgpu *vgpu, int
> port_num)
> return ret;
> }
>
> -static int intel_vgpu_iommu_notifier(struct notifier_block *nb,
> - unsigned long action, void *data)
> +static void intel_vgpu_dma_unmap(struct vfio_device *vfio_dev, u64 iova,
> + u64 length)
> {
> - struct intel_vgpu *vgpu =
> - container_of(nb, struct intel_vgpu, iommu_notifier);
> -
> - if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) {
> - struct vfio_iommu_type1_dma_unmap *unmap = data;
> - struct gvt_dma *entry;
> - unsigned long iov_pfn, end_iov_pfn;
> + struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
> + struct gvt_dma *entry;
> + u64 iov_pfn = iova >> PAGE_SHIFT;
> + u64 end_iov_pfn = iov_pfn + length / PAGE_SIZE;
>
> - iov_pfn = unmap->iova >> PAGE_SHIFT;
> - end_iov_pfn = iov_pfn + unmap->size / PAGE_SIZE;
> + mutex_lock(>cache_lock);
> + for (; iov_pfn < end_iov_pfn; iov_pfn++) {
> + entry = __gvt_cache_find_gfn(vgpu, iov_pfn);
> + if (!entry)
> + continue;
>
> - mutex_lock(>cache_lock);
> - for (; iov_pfn < end_iov_pfn; iov_pfn++) {
> - entry = __gvt_cache_find_gfn(vgpu, iov_pfn);
> - if (!entry)
> - continue;
> -
> - gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr,
> -entry->size);
> - __gvt_cache_remove_entry(vgpu, entry);
> - }
> - mutex_unlock(>cache_lock);
> + gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr,
> +entry->size);
> + __gvt_cache_remove_entry(vgpu, entry);
> }
> -
> - return NOTIFY_OK;
> + mutex_unlock(>cache_lock);
> }
>
> static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
> @@ -783,36 +774,20 @@ static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
> static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
> {
> struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
> - unsigned long events;
> - int ret;
> -
> - vgpu->iommu_notifier.notifier_call = intel_vgpu_iommu_notifier;
>
> - events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
> - ret = vfio_register_notifier(vfio_dev, VFIO_IOMMU_NOTIFY, ,
> - >iommu_notifier);
> - if (ret != 0) {
> - gvt_vgpu_err("vfio_register_notifier for iommu failed: %d\n",
> - ret);
> - goto out;
> - }
> -
> - ret = -EEXIST;
> if (vgpu->attached)
> - goto undo_iommu;
> + return -EEXIST;
>
> - ret = -ESRCH;
> if (!vgpu->vfio_device.kvm ||
> vgpu->vfio_device.kvm->mm != current->mm) {
> gvt_vgpu_err("KVM is required to use Intel vGPU\n");
> - goto undo_iommu;
> + return -ESRCH;
> }
>
> kvm_get_kvm(vgpu->vfio_device.kvm);
>
> - ret = -EEXIST;
> if (__kvmgt_vgpu_exist(vgpu))
> - goto undo_iommu;
> + return
[Intel-gfx] [PATCH 2/3] drm/i915: Bump GT idling delay to 2 jiffies
From: Chris Wilson In monitoring a transcode pipeline that is latency sensitive (it waits between submitting frames, and each frame requires work on rcs/vcs/vecs engines), it is found that it took longer than a single jiffy for it to sustain its workload. Allowing an extra jiffy headroom for the userspace prevents us from prematurely parking and having to exit powersaving immediately. Link: https://gitlab.freedesktop.org/drm/intel/-/issues/6284 Signed-off-by: Chris Wilson Signed-off-by: Karolina Drobnik --- drivers/gpu/drm/i915/i915_active.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index ee2b3a375362..7412abf166a8 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -974,7 +974,7 @@ void i915_active_acquire_barrier(struct i915_active *ref) GEM_BUG_ON(!intel_engine_pm_is_awake(engine)); llist_add(barrier_to_ll(node), >barrier_tasks); - intel_engine_pm_put_delay(engine, 1); + intel_engine_pm_put_delay(engine, 2); } } -- 2.25.1
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/pvc: Implement w/a 16016694945
== Series Details == Series: drm/i915/pvc: Implement w/a 16016694945 URL : https://patchwork.freedesktop.org/series/105837/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11835_full -> Patchwork_105837v1_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_105837v1_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_105837v1_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (13 -> 13) -- No changes in participating hosts Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_105837v1_full: ### Piglit changes ### Possible regressions * spec@arb_gpu_shader5@texturegatheroffset@vs-rgba-3-unorm-2drect-const: - pig-glk-j5005: NOTRUN -> [CRASH][1] +1 similar issue [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/pig-glk-j5005/spec@arb_gpu_shader5@texturegatheroff...@vs-rgba-3-unorm-2drect-const.html * spec@ext_texture_snorm@fbo-colormask-formats: - pig-skl-6260u: NOTRUN -> [INCOMPLETE][2] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/pig-skl-6260u/spec@ext_texture_sn...@fbo-colormask-formats.html Known issues Here are the changes found in Patchwork_105837v1_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_persistence@hang: - shard-skl: NOTRUN -> [SKIP][3] ([fdo#109271]) +116 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-skl6/igt@gem_ctx_persiste...@hang.html * igt@gem_eio@unwedge-stress: - shard-tglb: [PASS][4] -> [FAIL][5] ([i915#5784]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-tglb7/igt@gem_...@unwedge-stress.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-tglb5/igt@gem_...@unwedge-stress.html * igt@gem_exec_balancer@parallel-bb-first: - shard-iclb: [PASS][6] -> [SKIP][7] ([i915#4525]) +1 similar issue [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb2/igt@gem_exec_balan...@parallel-bb-first.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb3/igt@gem_exec_balan...@parallel-bb-first.html * igt@gem_exec_fair@basic-none@vcs0: - shard-apl: [PASS][8] -> [FAIL][9] ([i915#2842]) +1 similar issue [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-apl8/igt@gem_exec_fair@basic-n...@vcs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl7/igt@gem_exec_fair@basic-n...@vcs0.html * igt@gem_exec_fair@basic-pace@vcs0: - shard-kbl: [PASS][10] -> [FAIL][11] ([i915#2842]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-kbl7/igt@gem_exec_fair@basic-p...@vcs0.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl1/igt@gem_exec_fair@basic-p...@vcs0.html * igt@gem_exec_fair@basic-pace@vcs1: - shard-iclb: NOTRUN -> [FAIL][12] ([i915#2842]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb4/igt@gem_exec_fair@basic-p...@vcs1.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-iclb: [PASS][13] -> [FAIL][14] ([i915#2849]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb6/igt@gem_exec_fair@basic-throt...@rcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb5/igt@gem_exec_fair@basic-throt...@rcs0.html * igt@gem_exec_schedule@u-submit-late-slice@bcs0: - shard-glk: [PASS][15] -> [INCOMPLETE][16] ([i915#6310]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-glk9/igt@gem_exec_schedule@u-submit-late-sl...@bcs0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-glk2/igt@gem_exec_schedule@u-submit-late-sl...@bcs0.html * igt@gem_lmem_swapping@heavy-verify-multi: - shard-apl: NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#4613]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl6/igt@gem_lmem_swapp...@heavy-verify-multi.html * igt@gem_lmem_swapping@parallel-random-verify: - shard-kbl: NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#4613]) +2 similar issues [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl7/igt@gem_lmem_swapp...@parallel-random-verify.html * igt@gem_lmem_swapping@parallel-random-verify-ccs: - shard-skl: NOTRUN -> [SKIP][19] ([fdo#109271] / [i915#4613]) [19]:
Re: [Intel-gfx] [PATCH i-g-t] tests/i915/kms_big_fb: trigger async flip with a dummy flip
> On 6/28/2022 4:34 PM, Arun R Murthy wrote:
> > In oder to trigger the async flip, a dummy flip is required after sync
> > flip so as to update the watermarks for async in KMD which happens as
> > part of this dummy flip. Thereafter async memory update will act as a
> > trigger register.
> > Capturing the CRC is done after the async flip as async flip at some
> > times can consume fairly a vblank period time.
> >
> > Signed-off-by: Arun R Murthy
> > ---
> > tests/i915/kms_big_fb.c | 29 +++--
> > 1 file changed, 19 insertions(+), 10 deletions(-)
> >
> > diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c index
> > d50fde45..6caf3c31 100644
> > --- a/tests/i915/kms_big_fb.c
> > +++ b/tests/i915/kms_big_fb.c
> > @@ -465,7 +465,7 @@ static bool test_pipe(data_t *data)
> > static bool
> > max_hw_stride_async_flip_test(data_t *data)
> > {
> > - uint32_t ret, startframe;
> > + uint32_t ret, frame;
> > const uint32_t w = data->output->config.default_mode.hdisplay,
> >h = data->output->config.default_mode.vdisplay;
> > igt_plane_t *primary;
> > @@ -519,7 +519,19 @@ max_hw_stride_async_flip_test(data_t *data)
> >
> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> >
> > igt_wait_for_vblank(data->drm_fd, data-
> >display.pipes[primary->pipe->pipe].crtc_offset);
> > - startframe = kmstest_get_vblank(data->drm_fd, data->pipe,
> 0) + 1;
> > + /*
> > +* In older platforms (<= Gen10), async address update bit is
> double buffered.
> > +* So flip timestamp can be verified only from the second flip.
> > +* The first async flip just enables the async address update.
> > +* In platforms greater than DISPLAY13 the first async flip will
> be discarded
> > +* in order to change the watermark levels as per the
> optimization. Hence the
> > +* subsequent async flips will actually do the asynchronous
> flips.
> > +*/
> > + ret = drmModePageFlip(data->drm_fd, data->output-
> >config.crtc->crtc_id,
> > +
> > data->big_fb_flip[i].fb_id,
> > +
> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> > + igt_wait_for_vblank(data->drm_fd, data-
> >display.pipes[primary->pipe->pipe].crtc_offset);
> > + frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) +
> 1;
>
> Hi,
>
> Should this be added inside a gen check similar to kms_async_flips?
Yes sorry missed it!
>
> >
> > for (int j = 0; j < 2; j++) {
> > do {
> > @@ -528,23 +540,20 @@ max_hw_stride_async_flip_test(data_t *data)
> >
> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> > } while (ret == -EBUSY);
> > igt_assert(ret == 0);
> > + igt_pipe_crc_get_for_frame(data->drm_fd, data-
> >pipe_crc,
> > + frame, _crc);
> >
> > + frame = kmstest_get_vblank(data->drm_fd, data-
> >pipe, 0) + 1;
> > do {
> > ret = drmModePageFlip(data->drm_fd, data-
> >output->config.crtc->crtc_id,
> > data->big_fb.fb_id,
> >
> DRM_MODE_PAGE_FLIP_ASYNC, NULL);
> > } while (ret == -EBUSY);
> > igt_assert(ret == 0);
> > + igt_pipe_crc_get_for_frame(data->drm_fd, data-
> >pipe_crc,
> > + frame, _crc);
>
> We should be moving this IMHO. By waiting for vblank after each async flip
> to capture CRC, what is the intended result? Seems to be completely
> changing the existing test logic.
We are getting the vblank count and based on that getting the crc. Now we know
for async flip at
certain times it can consume a time equivalent to a vblank period. So in those
scenarios getting crc
based on the vblank goes for a toss. Hence capturing the vblank start time
stamp at the beginning
of each flip.
>
> Also, seems like we are overwriting the CRC captured for j = 0, as comparison
> is done outside this loop. Is this done on purpose?
Now with the changing the vblank start frame capture being added before the
async flip, CRC can
be captured outside the loop as well, but makes no sense. To maintain this
order moving the CRC
Capture also after each frame.
Thanks and Regards,
Arun R Murthy
Re: [Intel-gfx] [RFC 09/10] drm/i915/vm_bind: Skip vma_lookup for persistent vmas
On Fri, 2022-07-01 at 15:50 -0700, Niranjana Vishwanathapura wrote:
> vma_lookup is tied to segment of the object instead of section
> of VA space. Hence, it do not support aliasing (ie., multiple
> bindings to the same section of the object).
> Skip vma_lookup for persistent vmas as it supports aliasing.
>
> Signed-off-by: Niranjana Vishwanathapura
>
> ---
> drivers/gpu/drm/i915/i915_vma.c | 19 ++-
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_vma.c
> b/drivers/gpu/drm/i915/i915_vma.c
> index 6adb013579be..9aa38b772b5b 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -197,6 +197,10 @@ vma_create(struct drm_i915_gem_object *obj,
> __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(vma));
> }
>
> + if (!i915_vma_is_ggtt(vma) &&
> + (view && view->type == I915_GGTT_VIEW_PARTIAL))
> + goto skip_rb_insert;
> +
Rather than guessing that a vma with this signature is a persistent
vma, which is confusing to the reader, could we have an argument saying
we want to create a persistent vma?
> rb = NULL;
> p = >vma.tree.rb_node;
> while (*p) {
> @@ -221,6 +225,7 @@ vma_create(struct drm_i915_gem_object *obj,
> rb_link_node(>obj_node, rb, p);
> rb_insert_color(>obj_node, >vma.tree);
>
> +skip_rb_insert:
> if (i915_vma_is_ggtt(vma))
> /*
> * We put the GGTT vma at the start of the vma-list,
> followed
> @@ -292,13 +297,16 @@ i915_vma_instance(struct drm_i915_gem_object
> *obj,
> struct i915_address_space *vm,
> const struct i915_ggtt_view *view)
> {
> - struct i915_vma *vma;
> + struct i915_vma *vma = NULL;
>
> GEM_BUG_ON(!kref_read(>ref));
>
> - spin_lock(>vma.lock);
> - vma = i915_vma_lookup(obj, vm, view);
> - spin_unlock(>vma.lock);
> + if (i915_is_ggtt(vm) || !view ||
> + view->type != I915_GGTT_VIEW_PARTIAL) {
Same here?
/Thomas
> + spin_lock(>vma.lock);
> + vma = i915_vma_lookup(obj, vm, view);
> + spin_unlock(>vma.lock);
> + }
>
> /* vma_create() will resolve the race if another creates the
> vma */
> if (unlikely(!vma))
> @@ -1670,7 +1678,8 @@ static void release_references(struct i915_vma
> *vma, bool vm_ddestroy)
>
> spin_lock(>vma.lock);
> list_del(>obj_link);
> - if (!RB_EMPTY_NODE(>obj_node))
> + if (!i915_vma_is_persistent(vma) &&
> + !RB_EMPTY_NODE(>obj_node))
> rb_erase(>obj_node, >vma.tree);
>
> spin_unlock(>vma.lock);
Re: [Intel-gfx] [RFC 01/10] drm/i915/vm_bind: Introduce VM_BIND ioctl
Hi,
On Fri, 2022-07-01 at 15:50 -0700, Niranjana Vishwanathapura wrote:
> Add VM_BIND and VM_UNBIND ioctls to bind/unbind a section of an
> object at the specified GPU virtual addresses.
>
> Add I915_PARAM_VM_BIND_VERSION to indicate version of VM_BIND feature
> supported and I915_VM_CREATE_FLAGS_USE_VM_BIND for UMDs to select the
> vm_bind mode of binding.
>
> Signed-off-by: Niranjana Vishwanathapura
>
Some comments on patch ordering. In order to ease reviews and to not
introduce unwanted surprises, could we
1) Add patches that introduce needed internal functionality /
refactoring / helpers.
2) Add patches that add enable intended user-space functionality, any
yet unsupported functionality disabled.
3) Add patches that introduce additional internal functionality /
refactoring / helpers.
4) Add patches that enable that additional functionality.
Fixes that are known at series submission time squashed before a
feature is enabled.
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 20 +-
> drivers/gpu/drm/i915/gem/i915_gem_context.h | 15 ++
> drivers/gpu/drm/i915/gt/intel_gtt.h | 6 +
> drivers/gpu/drm/i915/i915_driver.c | 30 +++
> drivers/gpu/drm/i915/i915_getparam.c | 3 +
> include/uapi/drm/i915_drm.h | 192
> +++-
> 6 files changed, 248 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index dabdfe09f5e5..e3f5fbf2ac05 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -81,7 +81,6 @@
>
> #include "pxp/intel_pxp.h"
>
> -#include "i915_file_private.h"
> #include "i915_gem_context.h"
> #include "i915_trace.h"
> #include "i915_user_extensions.h"
> @@ -346,20 +345,6 @@ static int proto_context_register(struct
> drm_i915_file_private *fpriv,
> return ret;
> }
>
> -static struct i915_address_space *
> -i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id)
> -{
> - struct i915_address_space *vm;
> -
> - xa_lock(_priv->vm_xa);
> - vm = xa_load(_priv->vm_xa, id);
> - if (vm)
> - kref_get(>ref);
> - xa_unlock(_priv->vm_xa);
> -
> - return vm;
> -}
> -
> static int set_proto_ctx_vm(struct drm_i915_file_private *fpriv,
> struct i915_gem_proto_context *pc,
> const struct drm_i915_gem_context_param
> *args)
> @@ -1799,7 +1784,7 @@ int i915_gem_vm_create_ioctl(struct drm_device
> *dev, void *data,
> if (!HAS_FULL_PPGTT(i915))
> return -ENODEV;
>
> - if (args->flags)
> + if (args->flags & I915_VM_CREATE_FLAGS_UNKNOWN)
> return -EINVAL;
>
> ppgtt = i915_ppgtt_create(to_gt(i915), 0);
> @@ -1819,6 +1804,9 @@ int i915_gem_vm_create_ioctl(struct drm_device
> *dev, void *data,
> if (err)
> goto err_put;
>
> + if (args->flags & I915_VM_CREATE_FLAGS_USE_VM_BIND)
> + ppgtt->vm.vm_bind_mode = true;
> +
> GEM_BUG_ON(id == 0); /* reserved for invalid/unassigned ppgtt
> */
> args->vm_id = id;
> return 0;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h
> b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> index e5b0f66ea1fe..723bf446c934 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> @@ -12,6 +12,7 @@
> #include "gt/intel_context.h"
>
> #include "i915_drv.h"
> +#include "i915_file_private.h"
> #include "i915_gem.h"
> #include "i915_scheduler.h"
> #include "intel_device_info.h"
> @@ -139,6 +140,20 @@ int i915_gem_context_setparam_ioctl(struct
> drm_device *dev, void *data,
> int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void
> *data,
> struct drm_file *file);
>
> +static inline struct i915_address_space *
> +i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id)
> +{
> + struct i915_address_space *vm;
> +
> + xa_lock(_priv->vm_xa);
> + vm = xa_load(_priv->vm_xa, id);
> + if (vm)
> + kref_get(>ref);
> + xa_unlock(_priv->vm_xa);
> +
> + return vm;
> +}
Does this really need to be inlined?
> +
> struct i915_gem_context *
> i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32
> id);
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
> b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index e639434e97fd..c812aa9708ae 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -271,6 +271,12 @@ struct i915_address_space {
> /* Skip pte rewrite on unbind for suspend. Protected by
> @mutex */
> bool skip_pte_rewrite:1;
>
> + /**
> + * true: allow only vm_bind method of binding.
> + * false: allow only legacy execbuff method of binding.
> + */
Use
[Intel-gfx] ✗ Fi.CI.IGT: failure for Protect vma destruction with the object lock
== Series Details ==
Series: Protect vma destruction with the object lock
URL : https://patchwork.freedesktop.org/series/105902/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_11849_full -> Patchwork_105902v1_full
Summary
---
**FAILURE**
Serious unknown changes coming with Patchwork_105902v1_full absolutely need
to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_105902v1_full, please notify your bug team to allow
them
to document this new failure mode, which will reduce false positives in CI.
Participating hosts (12 -> 12)
--
No changes in participating hosts
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_105902v1_full:
### IGT changes ###
Possible regressions
* igt@syncobj_timeline@wait-any-complex:
- shard-glk: [PASS][1] -> [TIMEOUT][2] +3 similar issues
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk3/igt@syncobj_timel...@wait-any-complex.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105902v1/shard-glk5/igt@syncobj_timel...@wait-any-complex.html
Warnings
* igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_ccs:
- shard-glk: [SKIP][3] ([fdo#109271]) -> [TIMEOUT][4] +1 similar
issue
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk3/igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_ccs.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105902v1/shard-glk5/igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_ccs.html
* igt@kms_chamelium@hdmi-audio:
- shard-glk: [SKIP][5] ([fdo#109271] / [fdo#111827]) ->
[TIMEOUT][6]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk3/igt@kms_chamel...@hdmi-audio.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105902v1/shard-glk5/igt@kms_chamel...@hdmi-audio.html
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
*
{igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-default-mode}:
- shard-iclb: NOTRUN -> [SKIP][7]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105902v1/shard-iclb3/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs-upscal...@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling:
- {shard-rkl}:NOTRUN -> [SKIP][8]
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105902v1/shard-rkl-1/igt@kms_flip_scaled_...@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html
Known issues
Here are the changes found in Patchwork_105902v1_full that come from known
issues:
### CI changes ###
Issues hit
* boot:
- shard-glk: ([PASS][9], [PASS][10], [PASS][11], [PASS][12],
[PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18],
[PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24],
[PASS][25], [PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30],
[PASS][31], [PASS][32], [PASS][33]) -> ([PASS][34], [PASS][35], [PASS][36],
[PASS][37], [PASS][38], [PASS][39], [PASS][40], [FAIL][41], [PASS][42],
[PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48],
[PASS][49], [PASS][50], [PASS][51], [PASS][52], [PASS][53], [PASS][54],
[PASS][55], [PASS][56], [PASS][57], [PASS][58]) ([i915#4392])
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk5/boot.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk1/boot.html
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk1/boot.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk2/boot.html
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk2/boot.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk2/boot.html
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk9/boot.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk9/boot.html
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk9/boot.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk8/boot.html
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk8/boot.html
[20]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk3/boot.html
[21]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk8/boot.html
[22]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk7/boot.html
[23]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk3/boot.html
[24]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-glk7/boot.html
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/dp: wait on timeout before retry (rev4)
== Series Details ==
Series: drm/i915/dp: wait on timeout before retry (rev4)
URL : https://patchwork.freedesktop.org/series/105660/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11848_full -> Patchwork_105660v4_full
Summary
---
**SUCCESS**
No regressions found.
Participating hosts (13 -> 12)
--
Missing(1): shard-dg1
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_105660v4_full:
### IGT changes ###
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling:
- {shard-rkl}:NOTRUN -> [SKIP][1]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-rkl-1/igt@kms_flip_scaled_...@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html
Known issues
Here are the changes found in Patchwork_105660v4_full that come from known
issues:
### CI changes ###
Issues hit
* boot:
- shard-glk: ([PASS][2], [PASS][3], [PASS][4], [PASS][5],
[PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12],
[PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18],
[PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24],
[PASS][25]) -> ([PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30],
[PASS][31], [PASS][32], [PASS][33], [FAIL][34], [PASS][35], [PASS][36],
[PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42],
[PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48],
[PASS][49], [PASS][50]) ([i915#4392])
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk9/boot.html
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk9/boot.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk9/boot.html
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk8/boot.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk8/boot.html
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk8/boot.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk7/boot.html
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk7/boot.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk7/boot.html
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk6/boot.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk6/boot.html
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk6/boot.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk5/boot.html
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk5/boot.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk5/boot.html
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk3/boot.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk3/boot.html
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk3/boot.html
[20]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk2/boot.html
[21]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk2/boot.html
[22]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk2/boot.html
[23]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk1/boot.html
[24]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk1/boot.html
[25]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11848/shard-glk1/boot.html
[26]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk1/boot.html
[27]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk1/boot.html
[28]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk1/boot.html
[29]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk2/boot.html
[30]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk2/boot.html
[31]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk2/boot.html
[32]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk3/boot.html
[33]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk3/boot.html
[34]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk3/boot.html
[35]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk3/boot.html
[36]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk5/boot.html
[37]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105660v4/shard-glk5/boot.html
[38]:
[Intel-gfx] ✗ Fi.CI.IGT: failure for Fix TLB invalidate issues with Broadwell (rev3)
== Series Details == Series: Fix TLB invalidate issues with Broadwell (rev3) URL : https://patchwork.freedesktop.org/series/105167/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11849_full -> Patchwork_105167v3_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_105167v3_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_105167v3_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (12 -> 12) -- No changes in participating hosts Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_105167v3_full: ### IGT changes ### Possible regressions * igt@i915_selftest@mock@requests: - shard-skl: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-skl1/igt@i915_selftest@m...@requests.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105167v3/shard-skl1/igt@i915_selftest@m...@requests.html * igt@kms_flip@flip-vs-suspend@b-edp1: - shard-iclb: [PASS][3] -> [SKIP][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-iclb7/igt@kms_flip@flip-vs-susp...@b-edp1.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105167v3/shard-iclb5/igt@kms_flip@flip-vs-susp...@b-edp1.html Known issues Here are the changes found in Patchwork_105167v3_full that come from known issues: ### CI changes ### Issues hit * boot: - shard-snb: ([PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25], [PASS][26], [PASS][27], [PASS][28], [PASS][29]) -> ([PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50], [PASS][51], [PASS][52], [FAIL][53], [PASS][54]) ([i915#4338]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb7/boot.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb7/boot.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb7/boot.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb7/boot.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb7/boot.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb6/boot.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb6/boot.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb6/boot.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb6/boot.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb6/boot.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb5/boot.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb5/boot.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb5/boot.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb5/boot.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb5/boot.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb4/boot.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb4/boot.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb4/boot.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb4/boot.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb4/boot.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb2/boot.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb2/boot.html [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb2/boot.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb2/boot.html [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-snb2/boot.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105167v3/shard-snb4/boot.html [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105167v3/shard-snb4/boot.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105167v3/shard-snb2/boot.html [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105167v3/shard-snb2/boot.html [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105167v3/shard-snb2/boot.html
Re: [Intel-gfx] [PATCH i-g-t] tests/i915/kms_big_fb: trigger async flip with a dummy flip
Hi,
On 5.7.2022 13.28, Murthy, Arun R wrote:
On 5.7.2022 12.49, Karthik B S wrote:
On 7/5/2022 3:08 PM, Murthy, Arun R wrote:
On 6/28/2022 4:34 PM, Arun R Murthy wrote:
In oder to trigger the async flip, a dummy flip is required after
sync flip so as to update the watermarks for async in KMD which
happens as part of this dummy flip. Thereafter async memory update
will act as a trigger register.
Capturing the CRC is done after the async flip as async flip at
some times can consume fairly a vblank period time.
Signed-off-by: Arun R Murthy
---
tests/i915/kms_big_fb.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c
index
d50fde45..6caf3c31 100644
--- a/tests/i915/kms_big_fb.c
+++ b/tests/i915/kms_big_fb.c
@@ -465,7 +465,7 @@ static bool test_pipe(data_t *data)
static bool
max_hw_stride_async_flip_test(data_t *data)
{
- uint32_t ret, startframe;
+ uint32_t ret, frame;
const uint32_t w =
data->output->config.default_mode.hdisplay,
h = data->output->config.default_mode.vdisplay;
igt_plane_t *primary;
@@ -519,7 +519,19 @@ max_hw_stride_async_flip_test(data_t *data)
DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
igt_wait_for_vblank(data->drm_fd, data-
display.pipes[primary->pipe->pipe].crtc_offset);
- startframe = kmstest_get_vblank(data->drm_fd, data->pipe,
0) + 1;
+ /*
+ * In older platforms (<= Gen10), async address update bit
+is
double buffered.
+ * So flip timestamp can be verified only from the second
flip.
+ * The first async flip just enables the async address update.
+ * In platforms greater than DISPLAY13 the first async
+flip
will
be discarded
+ * in order to change the watermark levels as per the
optimization. Hence the
+ * subsequent async flips will actually do the
+asynchronous
flips.
+ */
+ ret = drmModePageFlip(data->drm_fd, data->output-
config.crtc->crtc_id,
+ data->big_fb_flip[i].fb_id,
+
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
+ igt_wait_for_vblank(data->drm_fd, data-
display.pipes[primary->pipe->pipe].crtc_offset);
+ frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) +
1;
Hi,
Should this be added inside a gen check similar to kms_async_flips?
Yes sorry missed it!
for (int j = 0; j < 2; j++) {
do {
@@ -528,23 +540,20 @@ max_hw_stride_async_flip_test(data_t
*data)
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
} while (ret == -EBUSY);
igt_assert(ret == 0);
+ igt_pipe_crc_get_for_frame(data->drm_fd, data-
pipe_crc,
+ frame, _crc);
+ frame = kmstest_get_vblank(data->drm_fd, data-
pipe, 0) + 1;
do {
ret = drmModePageFlip(data->drm_fd, data-
output->config.crtc->crtc_id,
data->big_fb.fb_id,
DRM_MODE_PAGE_FLIP_ASYNC, NULL);
} while (ret == -EBUSY);
igt_assert(ret == 0);
+ igt_pipe_crc_get_for_frame(data->drm_fd, data-
pipe_crc,
+ frame, _crc);
We should be moving this IMHO. By waiting for vblank after each
async flip to capture CRC, what is the intended result? Seems to be
completely changing the existing test logic.
We are getting the vblank count and based on that getting the crc.
Now we know for async flip at certain times it can consume a time
equivalent to a vblank period. So in those scenarios getting crc
based on the vblank goes for a toss. Hence capturing the vblank start
time stamp at the beginning of each flip.
Hi,
But what is the the reference CRC we're expecting? The original logic
is designed to match on one iteration and mismatch on the next using a
combination of fb's by using async flips. But with these changes that
whole logic goes for a toss?
Also, seems like we are overwriting the CRC captured for j = 0, as
comparison is done outside this loop. Is this done on purpose?
Now with the changing the vblank start frame capture being added
before the async flip, CRC can be captured outside the loop as well,
but makes no sense. To maintain this order moving the CRC Capture
also after each frame.
The CRC comparison is still outside the loop, so no point capturing
CRC's if we finally discard it anyway?
I think generally the idea Arun is changing is correct but he's missed how the
test work.
I see there's Ville's change on kernel side for display_ver >=13 first
async_flip
is unconditionally (intentionally) missed, this is at
intel_plane_do_async_flip(..) so this test will need adjustment
What Arun seem to have missed is on test those nested loops how they
work, that part probably should've originally been commented in code bit
better.
On original code there's after loop for j two time
igt_pipe_crc_get_for_frame(..), first will capture
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Support Async Flip on Linear buffers (rev3)
== Series Details ==
Series: drm/i915: Support Async Flip on Linear buffers (rev3)
URL : https://patchwork.freedesktop.org/series/103137/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11849_full -> Patchwork_103137v3_full
Summary
---
**SUCCESS**
No regressions found.
Participating hosts (12 -> 12)
--
No changes in participating hosts
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_103137v3_full:
### IGT changes ###
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
*
{igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-32bpp-yftileccs-downscaling@pipe-a-default-mode}:
- shard-iclb: NOTRUN -> [SKIP][1]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-32bpp-yftileccs-downscal...@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling:
- {shard-rkl}:NOTRUN -> [SKIP][2]
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-rkl-2/igt@kms_flip_scaled_...@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html
Known issues
Here are the changes found in Patchwork_103137v3_full that come from known
issues:
### IGT changes ###
Issues hit
* igt@gem_ctx_isolation@preservation-s3@vecs0:
- shard-kbl: [PASS][3] -> [DMESG-WARN][4] ([i915#180]) +4 similar
issues
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-kbl1/igt@gem_ctx_isolation@preservation...@vecs0.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-kbl4/igt@gem_ctx_isolation@preservation...@vecs0.html
* igt@gem_exec_balancer@parallel-keep-in-fence:
- shard-iclb: [PASS][5] -> [SKIP][6] ([i915#4525])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-iclb1/igt@gem_exec_balan...@parallel-keep-in-fence.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-iclb6/igt@gem_exec_balan...@parallel-keep-in-fence.html
* igt@gem_exec_fair@basic-none-share@rcs0:
- shard-iclb: [PASS][7] -> [FAIL][8] ([i915#2842])
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-iclb8/igt@gem_exec_fair@basic-none-sh...@rcs0.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-iclb1/igt@gem_exec_fair@basic-none-sh...@rcs0.html
* igt@gem_exec_fair@basic-none@vcs1:
- shard-kbl: [PASS][9] -> [FAIL][10] ([i915#2842]) +4 similar
issues
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11849/shard-kbl6/igt@gem_exec_fair@basic-n...@vcs1.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-kbl4/igt@gem_exec_fair@basic-n...@vcs1.html
* igt@gem_exec_fair@basic-pace@vcs1:
- shard-iclb: NOTRUN -> [FAIL][11] ([i915#2842])
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-iclb1/igt@gem_exec_fair@basic-p...@vcs1.html
* igt@gem_huc_copy@huc-copy:
- shard-kbl: NOTRUN -> [SKIP][12] ([fdo#109271] / [i915#2190])
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-kbl7/igt@gem_huc_c...@huc-copy.html
* igt@gem_lmem_swapping@basic:
- shard-apl: NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#4613]) +1
similar issue
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-apl3/igt@gem_lmem_swapp...@basic.html
* igt@gem_lmem_swapping@smem-oom:
- shard-kbl: NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#4613])
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-kbl7/igt@gem_lmem_swapp...@smem-oom.html
* igt@gem_lmem_swapping@verify-random-ccs:
- shard-skl: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4613]) +2
similar issues
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-skl1/igt@gem_lmem_swapp...@verify-random-ccs.html
* igt@gem_softpin@evict-single-offset:
- shard-apl: NOTRUN -> [FAIL][16] ([i915#4171])
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-apl3/igt@gem_soft...@evict-single-offset.html
* igt@gem_userptr_blits@dmabuf-sync:
- shard-kbl: NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#3323])
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-kbl7/igt@gem_userptr_bl...@dmabuf-sync.html
* igt@gem_userptr_blits@input-checking:
- shard-skl: NOTRUN -> [DMESG-WARN][18] ([i915#4991])
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103137v3/shard-skl10/igt@gem_userptr_bl...@input-checking.html
* igt@gem_workarounds@suspend-resume-fd:
- shard-snb: [PASS][19] -> [SKIP][20] ([fdo#109271]) +1 similar
issue
[19]:
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/pvc: Implement w/a 16016694945
== Series Details == Series: drm/i915/pvc: Implement w/a 16016694945 URL : https://patchwork.freedesktop.org/series/105837/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11835_full -> Patchwork_105837v1_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_105837v1_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_105837v1_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (13 -> 13) -- No changes in participating hosts Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_105837v1_full: ### Piglit changes ### Possible regressions * spec@arb_gpu_shader5@texturegatheroffset@vs-rgba-3-unorm-2drect-const: - pig-glk-j5005: NOTRUN -> [CRASH][1] +1 similar issue [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/pig-glk-j5005/spec@arb_gpu_shader5@texturegatheroff...@vs-rgba-3-unorm-2drect-const.html * spec@ext_texture_snorm@fbo-colormask-formats: - pig-skl-6260u: NOTRUN -> [INCOMPLETE][2] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/pig-skl-6260u/spec@ext_texture_sn...@fbo-colormask-formats.html Known issues Here are the changes found in Patchwork_105837v1_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_persistence@hang: - shard-skl: NOTRUN -> [SKIP][3] ([fdo#109271]) +116 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-skl6/igt@gem_ctx_persiste...@hang.html * igt@gem_eio@unwedge-stress: - shard-tglb: [PASS][4] -> [FAIL][5] ([i915#5784]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-tglb7/igt@gem_...@unwedge-stress.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-tglb5/igt@gem_...@unwedge-stress.html * igt@gem_exec_balancer@parallel-bb-first: - shard-iclb: [PASS][6] -> [SKIP][7] ([i915#4525]) +1 similar issue [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb2/igt@gem_exec_balan...@parallel-bb-first.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb3/igt@gem_exec_balan...@parallel-bb-first.html * igt@gem_exec_fair@basic-none@vcs0: - shard-apl: [PASS][8] -> [FAIL][9] ([i915#2842]) +1 similar issue [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-apl8/igt@gem_exec_fair@basic-n...@vcs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl7/igt@gem_exec_fair@basic-n...@vcs0.html * igt@gem_exec_fair@basic-pace@vcs0: - shard-kbl: [PASS][10] -> [FAIL][11] ([i915#2842]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-kbl7/igt@gem_exec_fair@basic-p...@vcs0.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl1/igt@gem_exec_fair@basic-p...@vcs0.html * igt@gem_exec_fair@basic-pace@vcs1: - shard-iclb: NOTRUN -> [FAIL][12] ([i915#2842]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb4/igt@gem_exec_fair@basic-p...@vcs1.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-iclb: [PASS][13] -> [FAIL][14] ([i915#2849]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-iclb6/igt@gem_exec_fair@basic-throt...@rcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-iclb5/igt@gem_exec_fair@basic-throt...@rcs0.html * igt@gem_exec_schedule@u-submit-late-slice@bcs0: - shard-glk: [PASS][15] -> [INCOMPLETE][16] ([i915#6310]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11835/shard-glk9/igt@gem_exec_schedule@u-submit-late-sl...@bcs0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-glk2/igt@gem_exec_schedule@u-submit-late-sl...@bcs0.html * igt@gem_lmem_swapping@heavy-verify-multi: - shard-apl: NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#4613]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-apl6/igt@gem_lmem_swapp...@heavy-verify-multi.html * igt@gem_lmem_swapping@parallel-random-verify: - shard-kbl: NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#4613]) +2 similar issues [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105837v1/shard-kbl7/igt@gem_lmem_swapp...@parallel-random-verify.html * igt@gem_lmem_swapping@parallel-random-verify-ccs: - shard-skl: NOTRUN -> [SKIP][19] ([fdo#109271] / [i915#4613]) [19]:
Re: [Intel-gfx] [PATCH 1/1] drm/i915/dg2: Bump up CDCLK for DG2
> -Original Message-
> From: Intel-gfx On Behalf Of
> Stanislav
> Lisovskiy
> Sent: Tuesday, June 14, 2022 6:01 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 1/1] drm/i915/dg2: Bump up CDCLK for DG2
>
> We seem to need this W/A same way as for TGL, in order to fix some of the
> underruns, which we currently have and those not related to PSR.
We need to dig deeper on the real root causes and try to identify scenarios
restricted by
hardware bandwidth limits and operate at optimum CD clock.
This should be kept under investigation with some open JIRA ticket while we can
get the
tree stabilized with this stop gap solution.
With above agreed, this is:
Reviewed-by: Uma Shankar
> Signed-off-by: Stanislav Lisovskiy
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 6e80162632dd..86a22c3766e5 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2300,7 +2300,7 @@ int intel_crtc_compute_min_cdclk(const struct
> intel_crtc_state *crtc_state)
> min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate);
>
> /*
> - * HACK. Currently for TGL platforms we calculate
> + * HACK. Currently for TGL/DG2 platforms we calculate
>* min_cdclk initially based on pixel_rate divided
>* by 2, accounting for also plane requirements,
>* however in some cases the lowest possible CDCLK @@ -2308,7 +2308,7
> @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>* Explicitly stating here that this seems to be currently
>* rather a Hack, than final solution.
>*/
> - if (IS_TIGERLAKE(dev_priv)) {
> + if (IS_TIGERLAKE(dev_priv) || IS_DG2(dev_priv)) {
> /*
>* Clamp to max_cdclk_freq in case pixel rate is higher,
>* in order not to break an 8K, but still leave W/A at place.
> --
> 2.24.1.485.gad05a3d8e5
