Re: [Intel-gfx] [PATCH v8 4/4] drm/i915/display: Split i915 specific code away from intel_fb.c
On Fri, 2023-12-01 at 17:54 +0200, Ville Syrjälä wrote:
> On Thu, Nov 30, 2023 at 04:43:38PM +0200, Jouni Högander wrote:
> > We are preparing for Xe driver. Backing object implementation is
> > differing
> > between i915 and Xe. Split i915 specific code into separate source
> > file
> > built only for i915.
> >
> > v8:
> > - return original error code from intel_fb_bo_lookup_valid_bo on
> > failure
> > v7:
> > - drop #include
> > - s/user_mode_cmd/mode_cmd/
> > - Use passed i915 pointer instead of to_i915(obj->base.dev)
> > v6: Add missing intel_fb_bo.[ch]
> > v5:
> > - Keep drm_any_plane_has_format check in intel_fb.c
> > - Use mode_cmd instead of user_mode_cmd for
> > intel_fb_bo_lookup_valid_bo
> > v4: Move drm_any_plane_has_format check into intel_fb_bo.c
> > v3: Fix failure handling in intel_framebuffer_init
> > v2: Couple of fixes to error value handling
> >
> > Signed-off-by: Jouni Högander
> > ---
> > drivers/gpu/drm/i915/Makefile | 1 +
> > drivers/gpu/drm/i915/display/intel_fb.c | 74 +++--
> > drivers/gpu/drm/i915/display/intel_fb_bo.c | 92
> > ++
> > drivers/gpu/drm/i915/display/intel_fb_bo.h | 24 ++
> > 4 files changed, 129 insertions(+), 62 deletions(-)
> > create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.c
> > create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile
> > index 65e984242089..5060c7b98a56 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -280,6 +280,7 @@ i915-y += \
> > display/intel_dsb.o \
> > display/intel_dsb_buffer.o \
> > display/intel_fb.o \
> > + display/intel_fb_bo.o \
> > display/intel_fb_pin.o \
> > display/intel_fbc.o \
> > display/intel_fdi.o \
> > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
> > b/drivers/gpu/drm/i915/display/intel_fb.c
> > index 868e39291e9f..853b3bd57461 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fb.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> > @@ -4,7 +4,6 @@
> > */
> >
> > #include
> > -#include
> > #include
> >
> > #include
> > @@ -15,6 +14,7 @@
> > #include "intel_display_types.h"
> > #include "intel_dpt.h"
> > #include "intel_fb.h"
> > +#include "intel_fb_bo.h"
> > #include "intel_frontbuffer.h"
> >
> > #define check_array_bounds(i915, a, i) drm_WARN_ON(&(i915)->drm,
> > (i) >= ARRAY_SIZE(a))
> > @@ -1985,7 +1985,6 @@ int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> > struct drm_i915_private *dev_priv =
> > to_i915(intel_bo_to_drm_bo(obj)->dev);
> > struct drm_framebuffer *fb = &intel_fb->base;
> > u32 max_stride;
> > - unsigned int tiling, stride;
> > int ret = -EINVAL;
> > int i;
> >
> > @@ -1993,32 +1992,11 @@ int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> > if (!intel_fb->frontbuffer)
> > return -ENOMEM;
> >
> > - i915_gem_object_lock(obj, NULL);
> > - tiling = i915_gem_object_get_tiling(obj);
> > - stride = i915_gem_object_get_stride(obj);
> > - i915_gem_object_unlock(obj);
> > -
> > - if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> > - /*
> > - * If there's a fence, enforce that
> > - * the fb modifier and tiling mode match.
> > - */
> > - if (tiling != I915_TILING_NONE &&
> > - tiling != intel_fb_modifier_to_tiling(mode_cmd-
> > >modifier[0])) {
> > - drm_dbg_kms(&dev_priv->drm,
> > - "tiling_mode doesn't match fb
> > modifier\n");
> > - goto err;
> > - }
> > - } else {
> > - if (tiling == I915_TILING_X) {
> > - mode_cmd->modifier[0] =
> > I915_FORMAT_MOD_X_TILED;
> > - } else if (tiling == I915_TILING_Y) {
> > - drm_dbg_kms(&dev_priv->drm,
> > - "No Y tiling for legacy
> > addfb\n");
> > - goto err;
> > - }
> > - }
> > + ret = intel_fb_bo_framebuffer_init(intel_fb, obj,
> > mode_cmd);
> > + if (ret)
> > + goto err;
> >
> > + ret = -EINVAL;
> > if (!drm_any_plane_has_format(&dev_priv->drm,
> > mode_cmd->pixel_format,
> > mode_cmd->modifier[0])) {
> > @@ -2028,17 +2006,6 @@ int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> > goto err;
> > }
> >
> > - /*
> > - * gen2/3 display engine uses the fence if present,
> > - * so the tiling mode must match the fb modifier exactly.
> > - */
> > - if (DISPLAY_VER(dev_priv) < 4 &&
> > - tiling != intel_
[Intel-gfx] linux-next: build warnings after merge of the drm-misc tree
Hi all,
After merging the drm-misc tree, today's linux-next build (htmldocs)
produced these warnings:
include/drm/drm_plane.h:60: warning: expecting prototype for struct
solid_fill_property. Prototype was for struct drm_solid_fill instead
include/drm/drm_plane.h:833: warning: Function parameter or member
'pixel_source_property' not described in 'drm_plane'
Introduced by commits
e50e5fed41c7 ("drm: Introduce pixel_source DRM plane property")
85863a4e16e7 ("drm: Introduce solid fill DRM plane property")
--
Cheers,
Stephen Rothwell
pgpZy4vfdT_jj.pgp
Description: OpenPGP digital signature
Re: [Intel-gfx] [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices
Christian König writes: > Am 01.12.23 um 06:48 schrieb Zeng, Oak: >> [SNIP] >> Besides memory eviction/oversubscription, there are a few other pain points >> when I use hmm: >> >> 1) hmm doesn't support file-back memory, so it is hard to share > memory b/t process in a gpu environment. You mentioned you have a > plan... How hard is it to support file-backed in your approach? > > As hard as it is to support it through HMM. That's what I meant that > this approach doesn't integrate well, as far as I know the problem > isn't inside HMM or any other solution but rather in the file system > layer. In what way does HMM not support file-backed memory? I was under the impression that at least hmm_range_fault() does. - Alistair > Regards, > Christian. > >> 2)virtual address range based memory attribute/hint: with hmadvise, > where do you save the memory attribute of a virtual address range? Do > you need to extend vm_area_struct to save it? With hmm, we have to > maintain such information at driver. This ends up with pretty > complicated logic to split/merge those address range. I know core mm > has similar logic to split/merge vma... >> >> Oak >> >> >>> -Weixi >>> >>> -Original Message- >>> From: Christian König >>> Sent: Thursday, November 30, 2023 4:28 PM >>> To: Zeng, Oak; Christian König >>> ; zhuweixi; linux- >>> m...@kvack.org;linux-ker...@vger.kernel.org;a...@linux-foundation.org; >>> Danilo Krummrich; Dave Airlie; Daniel >>> Vetter >>> Cc:intel-gvt-...@lists.freedesktop.org;rcampb...@nvidia.com; >>> mhairgr...@nvidia.com;j...@nvidia.com;weixi@openeuler.sh; >>> jhubb...@nvidia.com;intel-gfx@lists.freedesktop.org;apop...@nvidia.com; >>> xinhui@amd.com;amd-...@lists.freedesktop.org; >>> tvrtko.ursu...@linux.intel.com;ogab...@kernel.org;jgli...@redhat.com; dri- >>> de...@lists.freedesktop.org;z...@nvidia.com; Vivi, Rodrigo >>> ;alexander.deuc...@amd.com;leo...@nvidia.com; >>> felix.kuehl...@amd.com; Wang, Zhi A; >>> mgor...@suse.de >>> Subject: Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory >>> management) for external memory devices >>> >>> Hi Oak, >>> >>> yeah, #4 is indeed a really good point and I think Felix will agree to that >>> as well. >>> >>> HMM is basically still missing a way to advise device attributes for the CPU >>> address space. Both migration strategy as well as device specific >>> information (like >>> cache preferences) fall into this category. >>> >>> Since there is a device specific component in those attributes as well I >>> think >>> device specific IOCTLs still make sense to update them, but HMM should offer >>> the functionality to manage and store those information. >>> >>> Split and merge of VMAs only become a problem if you attach those >>> information >>> to VMAs, if you keep them completely separate than that doesn't become an >>> issue either. The down side of this approach is that you don't get >>> automatically >>> extending attribute ranges for growing VMAs for example. >>> >>> Regards, >>> Christian. >>> >>> Am 29.11.23 um 23:23 schrieb Zeng, Oak: Hi Weixi, Even though Christian has listed reasons rejecting this proposal (yes they are >>> very reasonable to me), I would open my mind and further explore the >>> possibility >>> here. Since the current GPU driver uses a hmm based implementation (AMD and >>> NV has done this; At Intel we are catching up), I want to explore how much >>> we >>> can benefit from the proposed approach and how your approach can solve some >>> pain points of our development. So basically what I am questioning here is: >>> what >>> is the advantage of your approach against hmm. To implement a UVM (unified virtual address space b/t cpu and gpu device), >>> with hmm, driver essentially need to implement below functions: 1. device page table update. Your approach requires the same because this is device specific codes 2. Some migration functions to migrate memory b/t system memory and GPU >>> local memory. My understanding is, even though you generalized this a bit, >>> such >>> as modified cpu page fault path, provided "general" gm_dev_fault handler... >>> but >>> device driver still need to provide migration functions because migration >>> functions have to be device specific (i.e., using device dma/copy engine for >>> performance purpose). Right? 3. GPU physical memory management, this part is now in drm/buddy, shared >>> by all drivers. I think with your approach, driver still need to provide >>> callback >>> functions to allocate/free physical pages. Right? Or do you let linux core >>> mm >>> buddy manage device memory directly? 4. madvise/hints/virtual address range management. This has been pain point >>> for us. Right now device driver has to maintain certain virtual address >>> range data >>> structure to maintain hints and other virtual address range based memory >>> attributes. Driver need to sync with linux vma. Driver need to explicitly >
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Replace kmap_atomic() with kmap_local_page() (rev2)
== Series Details == Series: drm/i915: Replace kmap_atomic() with kmap_local_page() (rev2) URL : https://patchwork.freedesktop.org/series/115769/ State : failure == Summary == CI Bug Log - changes from CI_DRM_13965 -> Patchwork_115769v2 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_115769v2 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_115769v2, please notify your bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/index.html Participating hosts (36 -> 33) -- Additional (2): bat-dg2-8 bat-dg2-9 Missing(5): bat-kbl-2 bat-dg1-5 bat-adlm-1 fi-snb-2520m bat-mtlp-8 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_115769v2: ### IGT changes ### Possible regressions * igt@kms_flip@basic-plain-flip@c-dp5: - bat-adlp-11:[PASS][1] -> [DMESG-WARN][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/bat-adlp-11/igt@kms_flip@basic-plain-f...@c-dp5.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-adlp-11/igt@kms_flip@basic-plain-f...@c-dp5.html New tests - New tests have been introduced between CI_DRM_13965 and Patchwork_115769v2: ### New IGT tests (12) ### * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-a-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-a-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-dp-7: - Statuses : 1 abort(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-c-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@read-crc@pipe-a-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@read-crc@pipe-c-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_pipe_crc_basic@read-crc@pipe-d-dp-7: - Statuses : 1 pass(s) - Exec time: [0.0] s Known issues Here are the changes found in Patchwork_115769v2 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_suspend@basic-s0@lmem0: - bat-dg2-8: NOTRUN -> [INCOMPLETE][3] ([i915#9275]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-8/igt@gem_exec_suspend@basic...@lmem0.html * igt@gem_mmap@basic: - bat-dg2-9: NOTRUN -> [SKIP][4] ([i915#4083]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-9/igt@gem_m...@basic.html - bat-dg2-8: NOTRUN -> [SKIP][5] ([i915#4083]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-8/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-dg2-9: NOTRUN -> [SKIP][6] ([i915#4077]) +2 other tests skip [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-9/igt@gem_mmap_...@basic.html - bat-dg2-8: NOTRUN -> [SKIP][7] ([i915#4077]) +2 other tests skip [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-8/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-dg2-9: NOTRUN -> [SKIP][8] ([i915#4079]) +1 other test skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-9/igt@gem_render_tiled_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-dg2-8: NOTRUN -> [SKIP][9] ([i915#4079]) +1 other test skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-8/igt@gem_tiled_pread_basic.html * igt@i915_pm_rps@basic-api: - bat-dg2-9: NOTRUN -> [SKIP][10] ([i915#6621]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-9/igt@i915_pm_...@basic-api.html - bat-dg2-8: NOTRUN -> [SKIP][11] ([i915#6621]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115769v2/bat-dg2-8/igt@i915_pm_...@basic-api.html * igt@i915_suspend@basic-s3-without-i915: - bat-dg2-8:
[Intel-gfx] ✗ Fi.CI.IGT: failure for Prepare intel_fb for Xe (rev10)
== Series Details ==
Series: Prepare intel_fb for Xe (rev10)
URL : https://patchwork.freedesktop.org/series/126507/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13965_full -> Patchwork_126507v10_full
Summary
---
**FAILURE**
Serious unknown changes coming with Patchwork_126507v10_full absolutely need
to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_126507v10_full, please notify your bug team
(i915-ci-in...@lists.freedesktop.org) to allow them
to document this new failure mode, which will reduce false positives in CI.
Participating hosts (9 -> 10)
--
Additional (1): shard-mtlp0
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_126507v10_full:
### IGT changes ###
Possible regressions
* igt@perf@non-zero-reason@0-rcs0:
- shard-dg1: [PASS][1] -> [FAIL][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-dg1-12/igt@perf@non-zero-rea...@0-rcs0.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/shard-dg1-12/igt@perf@non-zero-rea...@0-rcs0.html
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* {igt@kms_selftest@drm_plane_helper@drm_test_check_invalid_plane_state}:
- shard-mtlp: NOTRUN -> [FAIL][3]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/shard-mtlp-5/igt@kms_selftest@drm_plane_helper@drm_test_check_invalid_plane_state.html
- shard-glk: NOTRUN -> [FAIL][4]
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/shard-glk2/igt@kms_selftest@drm_plane_helper@drm_test_check_invalid_plane_state.html
* {igt@kms_selftest@drm_plane_helper@drm_test_check_plane_state}:
- shard-mtlp: NOTRUN -> [DMESG-FAIL][5]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/shard-mtlp-5/igt@kms_selftest@drm_plane_helper@drm_test_check_plane_state.html
- shard-glk: NOTRUN -> [DMESG-FAIL][6]
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/shard-glk2/igt@kms_selftest@drm_plane_helper@drm_test_check_plane_state.html
Known issues
Here are the changes found in Patchwork_126507v10_full that come from known
issues:
### CI changes ###
Issues hit
* boot:
- shard-glk: ([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], [FAIL][44], [PASS][45], [PASS][46],
[PASS][47], [PASS][48], [PASS][49], [PASS][50], [PASS][51], [PASS][52],
[PASS][53], [PASS][54], [PASS][55]) ([i915#8293])
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk4/boot.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk9/boot.html
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk9/boot.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk9/boot.html
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk8/boot.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk8/boot.html
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk7/boot.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk7/boot.html
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk7/boot.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk6/boot.html
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk6/boot.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk5/boot.html
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk5/boot.html
[20]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk5/boot.html
[21]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk4/boot.html
[22]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk4/boot.html
[23]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk3/boot.html
[24]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk3/boot.html
[25]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk3/boot.html
[26]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk2/boot.html
[27]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/shard-glk2/boot.html
[28]:
https://int
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Replace kmap_atomic() with kmap_local_page() (rev2)
== Series Details == Series: drm/i915: Replace kmap_atomic() with kmap_local_page() (rev2) URL : https://patchwork.freedesktop.org/series/115769/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Replace kmap_atomic() with kmap_local_page() (rev2)
== Series Details == Series: drm/i915: Replace kmap_atomic() with kmap_local_page() (rev2) URL : https://patchwork.freedesktop.org/series/115769/ State : warning == Summary == Error: dim checkpatch failed 3dbe1fea8502 drm/i915: Use kmap_local_page() in gem/i915_gem_object.c -:39: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #39: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com -:63: ERROR:CODE_INDENT: code indent should use tabs where possible #63: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.c:506: +^I + offset_in_page(offset);$ total: 1 errors, 1 warnings, 0 checks, 20 lines checked b9f8592803f4 drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c -:39: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #39: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com total: 0 errors, 1 warnings, 0 checks, 34 lines checked de0a94d29e8b drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c -:24: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #24: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com total: 0 errors, 1 warnings, 0 checks, 15 lines checked 95976ec3834b drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c -:36: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #36: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com total: 0 errors, 1 warnings, 0 checks, 22 lines checked c956111d73bd drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c -:37: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #37: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com total: 0 errors, 1 warnings, 0 checks, 48 lines checked 46aae55d8cc4 drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c -:38: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #38: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com total: 0 errors, 1 warnings, 0 checks, 30 lines checked d00c94b04735 drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c -:29: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #29: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com/T/#u total: 0 errors, 1 warnings, 0 checks, 17 lines checked 1b79798a46f6 drm/i915: Use kmap_local_page() in i915_cmd_parser.c -:34: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #34: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com total: 0 errors, 1 warnings, 0 checks, 13 lines checked 2383cd11b380 drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c -:34: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #34: [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com total: 0 errors, 1 warnings, 0 checks, 40 lines checked
[Intel-gfx] [PATCH v3 6/9] drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption.
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/i915_gem_context.c, functions cpu_fill() and
cpu_check() mainly uses mapping to flush cache and check/assign the
value.
There're 2 reasons why cpu_fill() and cpu_check() don't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. cpu_fill() and cpu_check() call
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, cpu_fill() and cpu_check() are functions where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 7021b6e9b219..89d4dc8b60c6 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -489,12 +489,12 @@ static int cpu_fill(struct drm_i915_gem_object *obj, u32
value)
for (n = 0; n < real_page_count(obj); n++) {
u32 *map;
- map = kmap_atomic(i915_gem_object_get_page(obj, n));
+ map = kmap_local_page(i915_gem_object_get_page(obj, n));
for (m = 0; m < DW_PER_PAGE; m++)
map[m] = value;
if (!has_llc)
drm_clflush_virt_range(map, PAGE_SIZE);
- kunmap_atomic(map);
+ kunmap_local(map);
}
i915_gem_object_finish_access(obj);
@@ -520,7 +520,7 @@ static noinline int cpu_check(struct drm_i915_gem_object
*obj,
for (n = 0; n < real_page_count(obj); n++) {
u32 *map, m;
- map = kmap_atomic(i915_gem_object_get_page(obj, n));
+ map = kmap_local_page(i915_gem_object_get_page(obj, n));
if (needs_flush & CLFLUSH_BEFORE)
drm_clflush_virt_range(map, PAGE_SIZE);
@@ -546,7 +546,7 @@ static noinline int cpu_check(struct drm_i915_gem_object
*obj,
}
out_unmap:
- kunmap_atomic(map);
+ kunmap_local(map);
if (err)
break;
}
--
2.34.1
[Intel-gfx] [PATCH v3 7/9] drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gt/uc/intel_us_fw.c, the function intel_uc_fw_copy_rsa()
just use the mapping to do memory copy so it doesn't need to disable
pagefaults and preemption for mapping. Thus the local mapping without
atomic context (not disable pagefaults / preemption) is enough.
Therefore, intel_uc_fw_copy_rsa() is a function where the use of
memcpy_from_page() with kmap_local_page() in place of memcpy() with
kmap_atomic() is correctly suited.
Convert the calls of memcpy() with kmap_atomic() / kunmap_atomic() to
memcpy_from_page() which uses local mapping to copy.
[1]:
https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com/T/#u
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Ira: Referred to his task document and suggestions about using
memcpy_from_page() directly.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 362639162ed6..756093eaf2ad 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -1343,16 +1343,13 @@ size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw,
void *dst, u32 max_len)
for_each_sgt_page(page, iter, uc_fw->obj->mm.pages) {
u32 len = min_t(u32, size, PAGE_SIZE - offset);
- void *vaddr;
if (idx > 0) {
idx--;
continue;
}
- vaddr = kmap_atomic(page);
- memcpy(dst, vaddr + offset, len);
- kunmap_atomic(vaddr);
+ memcpy_from_page(dst, page, offset, len);
offset = 0;
dst += len;
--
2.34.1
[Intel-gfx] [PATCH v3 8/9] drm/i915: Use kmap_local_page() in i915_cmd_parser.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
There're 2 reasons why function copy_batch() doesn't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. In i915_cmd_parser.c, copy_batch() calls
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush.
Since CLFLUSHOPT is global on x86 and WBINVD is called on each cpu
in drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, copy_batch() is a function where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index ddf49c2dbb91..2905df83e180 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1211,11 +1211,11 @@ static u32 *copy_batch(struct drm_i915_gem_object
*dst_obj,
for (n = offset >> PAGE_SHIFT; remain; n++) {
int len = min(remain, PAGE_SIZE - x);
- src = kmap_atomic(i915_gem_object_get_page(src_obj, n));
+ src = kmap_local_page(i915_gem_object_get_page(src_obj,
n));
if (src_needs_clflush)
drm_clflush_virt_range(src + x, len);
memcpy(ptr, src + x, len);
- kunmap_atomic(src);
+ kunmap_local(src);
ptr += len;
remain -= len;
--
2.34.1
[Intel-gfx] [PATCH v3 9/9] drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the calls from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
In i915_gem_execbuffer.c, eb->reloc_cache.vaddr is mapped by
kmap_atomic() in eb_relocate_entry(), and is unmapped by
kunmap_atomic() in reloc_cache_reset().
And this mapping/unmapping occurs in two places: one is in
eb_relocate_vma(), and another is in eb_relocate_vma_slow().
The function eb_relocate_vma() or eb_relocate_vma_slow() doesn't
need to disable pagefaults and preemption during the above mapping/
unmapping.
So it can simply use kmap_local_page() / kunmap_local() that can
instead do the mapping / unmapping regardless of the context.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Ira: Referred to his task document, review comments.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 683fd8d3151c..18b0f3117074 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1156,7 +1156,7 @@ static void reloc_cache_unmap(struct reloc_cache *cache)
vaddr = unmask_page(cache->vaddr);
if (cache->vaddr & KMAP)
- kunmap_atomic(vaddr);
+ kunmap_local(vaddr);
else
io_mapping_unmap_atomic((void __iomem *)vaddr);
}
@@ -1172,7 +1172,7 @@ static void reloc_cache_remap(struct reloc_cache *cache,
if (cache->vaddr & KMAP) {
struct page *page = i915_gem_object_get_page(obj, cache->page);
- vaddr = kmap_atomic(page);
+ vaddr = kmap_local_page(page);
cache->vaddr = unmask_flags(cache->vaddr) |
(unsigned long)vaddr;
} else {
@@ -1202,7 +1202,7 @@ static void reloc_cache_reset(struct reloc_cache *cache,
struct i915_execbuffer
if (cache->vaddr & CLFLUSH_AFTER)
mb();
- kunmap_atomic(vaddr);
+ kunmap_local(vaddr);
i915_gem_object_finish_access(obj);
} else {
struct i915_ggtt *ggtt = cache_to_ggtt(cache);
@@ -1234,7 +1234,7 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
struct page *page;
if (cache->vaddr) {
- kunmap_atomic(unmask_page(cache->vaddr));
+ kunmap_local(unmask_page(cache->vaddr));
} else {
unsigned int flushes;
int err;
@@ -1256,7 +1256,7 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
if (!obj->mm.dirty)
set_page_dirty(page);
- vaddr = kmap_atomic(page);
+ vaddr = kmap_local_page(page);
cache->vaddr = unmask_flags(cache->vaddr) | (unsigned long)vaddr;
cache->page = pageno;
--
2.34.1
[Intel-gfx] [PATCH v3 5/9] drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration)..
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/i915_gem_coherency.c, functions cpu_set()
and cpu_get() mainly uses mapping to flush cache and assign the value.
There're 2 reasons why cpu_set() and cpu_get() don't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. cpu_set() and cpu_get() call
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, cpu_set() and cpu_get() are functions where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
.../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
index 3bef1beec7cb..beeb3e12eccc 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
@@ -24,7 +24,6 @@ static int cpu_set(struct context *ctx, unsigned long offset,
u32 v)
{
unsigned int needs_clflush;
struct page *page;
- void *map;
u32 *cpu;
int err;
@@ -34,8 +33,7 @@ static int cpu_set(struct context *ctx, unsigned long offset,
u32 v)
goto out;
page = i915_gem_object_get_page(ctx->obj, offset >> PAGE_SHIFT);
- map = kmap_atomic(page);
- cpu = map + offset_in_page(offset);
+ cpu = kmap_local_page(page) + offset_in_page(offset);
if (needs_clflush & CLFLUSH_BEFORE)
drm_clflush_virt_range(cpu, sizeof(*cpu));
@@ -45,7 +43,7 @@ static int cpu_set(struct context *ctx, unsigned long offset,
u32 v)
if (needs_clflush & CLFLUSH_AFTER)
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap_atomic(map);
+ kunmap_local(cpu);
i915_gem_object_finish_access(ctx->obj);
out:
@@ -57,7 +55,6 @@ static int cpu_get(struct context *ctx, unsigned long offset,
u32 *v)
{
unsigned int needs_clflush;
struct page *page;
- void *map;
u32 *cpu;
int err;
@@ -67,15 +64,14 @@ static int cpu_get(struct context *ctx, unsigned long
offset, u32 *v)
goto out;
page = i915_gem_object_get_page(ctx->obj, offset >> PAGE_SHIFT);
- map = kmap_atomic(page);
- cpu = map + offset_in_page(offset);
+ cpu = kmap_local_page(page) + offset_in_page(offset);
if (needs_clflush & CLFLUSH_BEFORE)
drm_clflush_virt_range(cpu, sizeof(*cpu));
*v = *cpu;
- kunmap_atomic(map);
+ kunmap_local(cpu);
i915_gem_object_finish_access(ctx->obj);
out:
--
2.34.1
[Intel-gfx] [PATCH v3 4/9] drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/huge_pages.c, function __cpu_check_shmem()
mainly uses mapping to flush cache and check the value. There're
2 reasons why __cpu_check_shmem() doesn't need to disable pagefaults
and preemption for mapping:
1. The flush operation is safe. Function __cpu_check_shmem() calls
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, __cpu_check_shmem() is a function where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 6b9f6cf50bf6..c9e6d77abab0 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1082,7 +1082,7 @@ __cpu_check_shmem(struct drm_i915_gem_object *obj, u32
dword, u32 val)
goto err_unlock;
for (n = 0; n < obj->base.size >> PAGE_SHIFT; ++n) {
- u32 *ptr = kmap_atomic(i915_gem_object_get_page(obj, n));
+ u32 *ptr = kmap_local_page(i915_gem_object_get_page(obj, n));
if (needs_flush & CLFLUSH_BEFORE)
drm_clflush_virt_range(ptr, PAGE_SIZE);
@@ -1090,12 +1090,12 @@ __cpu_check_shmem(struct drm_i915_gem_object *obj, u32
dword, u32 val)
if (ptr[dword] != val) {
pr_err("n=%lu ptr[%u]=%u, val=%u\n",
n, dword, ptr[dword], val);
- kunmap_atomic(ptr);
+ kunmap_local(ptr);
err = -EINVAL;
break;
}
- kunmap_atomic(ptr);
+ kunmap_local(ptr);
}
i915_gem_object_finish_access(obj);
--
2.34.1
[Intel-gfx] [PATCH v3 3/9] drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
From: Zhao Liu The use of kmap_atomic() is being deprecated in favor of kmap_local_page()[1]. The main difference between atomic and local mappings is that local mappings doesn't disable page faults or preemption (the preemption is disabled for !PREEMPT_RT case, otherwise it only disables migration). With kmap_local_page(), we can avoid the often unwanted side effect of unnecessary page faults or preemption disables. In drm/i915/gem/i915_gem_shmem.c, the function shmem_pwrite() need to disable pagefault to eliminate the potential recursion fault[2]. But here __copy_from_user_inatomic() doesn't need to disable preemption and local mapping is valid for sched in/out. So it can use kmap_local_page() / kunmap_local() with pagefault_disable() / pagefault_enable() to replace atomic mapping. [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com [2]: https://patchwork.freedesktop.org/patch/295840/ Suggested-by: Ira Weiny Suggested-by: Fabio M. De Francesco Signed-off-by: Zhao Liu Reviewed-by: Ira Weiny Reviewed-by: Fabio M. De Francesco --- Suggested by credits: Ira: Referred to his suggestions about keeping pagefault_disable(). Fabio: Referred to his description about why kmap_local_page() should be preferred. --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 73a4a4eb29e0..38b72d86560f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -485,11 +485,13 @@ shmem_pwrite(struct drm_i915_gem_object *obj, if (err < 0) return err; - vaddr = kmap_atomic(page); + vaddr = kmap_local_page(page); + pagefault_disable(); unwritten = __copy_from_user_inatomic(vaddr + pg, user_data, len); - kunmap_atomic(vaddr); + pagefault_enable(); + kunmap_local(vaddr); err = aops->write_end(obj->base.filp, mapping, offset, len, len - unwritten, page, data); -- 2.34.1
[Intel-gfx] [PATCH v3 2/9] drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() + memcpy() to memcpy_[from/to]_page(), which use
kmap_local_page() to build local mapping and then do memcpy().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
In drm/i915/gem/i915_gem_phys.c, the functions
i915_gem_object_get_pages_phys() and i915_gem_object_put_pages_phys()
don't need to disable pagefaults and preemption for mapping because of
2 reasons:
1. The flush operation is safe. In drm/i915/gem/i915_gem_object.c,
i915_gem_object_get_pages_phys() and i915_gem_object_put_pages_phys()
calls drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush.
Since CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, i915_gem_object_get_pages_phys() and
i915_gem_object_put_pages_phys() are two functions where the uses of
local mappings in place of atomic mappings are correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() + memcpy() to
memcpy_from_page() and memcpy_to_page().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 5df128e2f4dc..ef85c6dc9fd5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -65,16 +65,13 @@ static int i915_gem_object_get_pages_phys(struct
drm_i915_gem_object *obj)
dst = vaddr;
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
- void *src;
page = shmem_read_mapping_page(mapping, i);
if (IS_ERR(page))
goto err_st;
- src = kmap_atomic(page);
- memcpy(dst, src, PAGE_SIZE);
+ memcpy_from_page(dst, page, 0, PAGE_SIZE);
drm_clflush_virt_range(dst, PAGE_SIZE);
- kunmap_atomic(src);
put_page(page);
dst += PAGE_SIZE;
@@ -113,16 +110,13 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object
*obj,
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
- char *dst;
page = shmem_read_mapping_page(mapping, i);
if (IS_ERR(page))
continue;
- dst = kmap_atomic(page);
drm_clflush_virt_range(src, PAGE_SIZE);
- memcpy(dst, src, PAGE_SIZE);
- kunmap_atomic(dst);
+ memcpy_to_page(page, 0, src, PAGE_SIZE);
set_page_dirty(page);
if (obj->mm.madv == I915_MADV_WILLNEED)
--
2.34.1
[Intel-gfx] [PATCH v3 1/9] drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
There're 2 reasons why i915_gem_object_read_from_page_kmap() doesn't
need to disable pagefaults and preemption for mapping:
1. The flush operation is safe. In drm/i915/gem/i915_gem_object.c,
i915_gem_object_read_from_page_kmap() calls drm_clflush_virt_range() to
use CLFLUSHOPT or WBINVD to flush. Since CLFLUSHOPT is global on x86
and WBINVD is called on each cpu in drm_clflush_virt_range(), the flush
operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, i915_gem_object_read_from_page_kmap() is a function where
the use of kmap_local_page() in place of kmap_atomic() is correctly
suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
And remove the redundant variable that stores the address of the mapped
page since kunmap_local() can accept any pointer within the page.
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index c26d87555825..a2a7e5005415 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -493,17 +493,15 @@ 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, idx));
-
- src_ptr = src_map + offset_in_page(offset);
+ src_ptr = kmap_local_page(i915_gem_object_get_page(obj, idx))
+ + offset_in_page(offset);
if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
drm_clflush_virt_range(src_ptr, size);
memcpy(dst, src_ptr, size);
- kunmap_atomic(src_map);
+ kunmap_local(src_ptr);
}
static void
--
2.34.1
[Intel-gfx] [PATCH v3 0/9] drm/i915: Replace kmap_atomic() with kmap_local_page()
From: Zhao Liu
Hi all,
I refreshed this v3 by rebasing v2 [1] on the commit 968f35f4ab1c
("Merge tag 'v6.7-rc3-smb3-client-fixes' of git://git.samba.org/sfrench/
cifs-2.6").
Based on the current code, I rechecked the substitutions in v2 and they
still stand and are valid, so no code change in v3.
Thanks for all the review! And sorry v2 was missed, I'll pay more
attention to this v3.
Purpose of This Patchset
The purpose of this pacthset is to replace all uses of kmap_atomic() in
i915 with kmap_local_page() because the use of kmap_atomic() is being
deprecated in favor of kmap_local_page()[2]. And 92b64bd (mm/highmem:
add notes about conversions from kmap{,_atomic}()) has declared the
deprecation of kmap_atomic().
Motivation for Deprecating kmap_atomic() and Using kmap_local_page()
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
Patch summary
=
Patch 1, 4-6 and 8-9 replace kmap_atomic()/kunmap_atomic() with
kmap_local_page()/kunmap_local() directly. With these local
mappings, the page faults and preemption are allowed.
Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace
kmap_atomic()/kunmap_atomic(). These two variants of memcpy()
are based on the local mapping, so page faults and preemption
are also allowed in these two interfaces.
Patch 3 replaces kmap_atomic()/kunmap_atomic() with kmap_local_page()/
kunmap_local() and also disable page fault since the for special
handling (pls see the commit message).
Reference
=
[1]:
https://lore.kernel.org/all/20230329073220.3982460-1-zhao1@linux.intel.com/
[2]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Thanks and Best Regards,
Zhao
---
Changlog:
Changes since v2:
* Rebased on 968f35f4ab1c ("Merge tag 'v6.7-rc3-smb3-client-fixes' of
git://git.samba.org/sfrench/cifs-2.6").
* Removed changelog (of v2) in commit message.
* Fixed typo in cover letter (Fabio).
* Added Reviewed-by tags from Ira and Fabio.
Changes since v1:
* Dropped hot plug related description in commit message since it has
nothing to do with kmap_local_page().
* Emphasized the motivation for using kmap_local_page() in commit
message.
* Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to
keep the "idx" variable of type pgoff_t here.
* Used memcpy_from_page() and memcpy_to_page() to replace
kmap_local_page() + memcpy() in patch 2.
---
Zhao Liu (9):
drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
drm/i915: Use kmap_local_page() in i915_cmd_parser.c
drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +-
drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++
drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 6 --
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
.../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12
.../gpu/drm/i915/gem/selftests/i915_gem_context.c| 8
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
9 files changed, 28 insertions(+), 41 deletions(-)
--
2.34.1
[Intel-gfx] ✓ Fi.CI.BAT: success for Prepare intel_fb for Xe (rev10)
== Series Details == Series: Prepare intel_fb for Xe (rev10) URL : https://patchwork.freedesktop.org/series/126507/ State : success == Summary == CI Bug Log - changes from CI_DRM_13965 -> Patchwork_126507v10 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/index.html Participating hosts (36 -> 32) -- Additional (1): bat-dg2-9 Missing(5): bat-kbl-2 bat-dg1-5 bat-adlm-1 fi-snb-2520m bat-mtlp-8 Known issues Here are the changes found in Patchwork_126507v10 that come from known issues: ### CI changes ### Issues hit * boot: - bat-jsl-1: [PASS][1] -> [FAIL][2] ([i915#8293]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/bat-jsl-1/boot.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-jsl-1/boot.html ### IGT changes ### Issues hit * igt@gem_mmap@basic: - bat-dg2-9: NOTRUN -> [SKIP][3] ([i915#4083]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-dg2-9: NOTRUN -> [SKIP][4] ([i915#4077]) +2 other tests skip [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-dg2-9: NOTRUN -> [SKIP][5] ([i915#4079]) +1 other test skip [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@gem_render_tiled_bl...@basic.html * igt@i915_pm_rps@basic-api: - bat-dg2-9: NOTRUN -> [SKIP][6] ([i915#6621]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@i915_pm_...@basic-api.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: - bat-dg2-9: NOTRUN -> [SKIP][7] ([i915#5190]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html * igt@kms_addfb_basic@basic-y-tiled-legacy: - bat-dg2-9: NOTRUN -> [SKIP][8] ([i915#4215] / [i915#5190]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@kms_addfb_ba...@basic-y-tiled-legacy.html * igt@kms_addfb_basic@framebuffer-vs-set-tiling: - bat-dg2-9: NOTRUN -> [SKIP][9] ([i915#4212]) +6 other tests skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@kms_addfb_ba...@framebuffer-vs-set-tiling.html * igt@kms_addfb_basic@tile-pitch-mismatch: - bat-dg2-9: NOTRUN -> [SKIP][10] ([i915#4212] / [i915#5608]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@kms_addfb_ba...@tile-pitch-mismatch.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - bat-dg2-9: NOTRUN -> [SKIP][11] ([i915#4103] / [i915#4213] / [i915#5608]) +1 other test skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html * igt@kms_force_connector_basic@force-load-detect: - bat-dg2-9: NOTRUN -> [SKIP][12] ([fdo#109285]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@kms_force_connector_ba...@force-load-detect.html * igt@kms_force_connector_basic@prune-stale-modes: - bat-dg2-9: NOTRUN -> [SKIP][13] ([i915#5274]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@kms_force_connector_ba...@prune-stale-modes.html * igt@kms_hdmi_inject@inject-audio: - fi-kbl-guc: [PASS][14] -> [FAIL][15] ([IGT#3]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13965/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/fi-kbl-guc/igt@kms_hdmi_inj...@inject-audio.html * igt@kms_setmode@basic-clone-single-crtc: - bat-dg2-9: NOTRUN -> [SKIP][16] ([i915#3555] / [i915#4098]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@kms_setm...@basic-clone-single-crtc.html * igt@prime_vgem@basic-fence-flip: - bat-dg2-9: NOTRUN -> [SKIP][17] ([i915#3708]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@prime_v...@basic-fence-flip.html * igt@prime_vgem@basic-fence-mmap: - bat-dg2-9: NOTRUN -> [SKIP][18] ([i915#3708] / [i915#4077]) +1 other test skip [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@prime_v...@basic-fence-mmap.html * igt@prime_vgem@basic-write: - bat-dg2-9: NOTRUN -> [SKIP][19] ([i915#3291] / [i915#3708]) +2 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126507v10/bat-dg2-9/igt@prime_v...@basic-write.html Possib
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Prepare intel_fb for Xe (rev10)
== Series Details == Series: Prepare intel_fb for Xe (rev10) URL : https://patchwork.freedesktop.org/series/126507/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit' +./arch/x86/in
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Prepare intel_fb for Xe (rev10)
== Series Details == Series: Prepare intel_fb for Xe (rev10) URL : https://patchwork.freedesktop.org/series/126507/ State : warning == Summary == Error: dim checkpatch failed 9bcd0d54b877 drm/i915/display: use intel_bo_to_drm_bo in intel_fb.c 6f1114511f45 drm/i915/display: Convert intel_fb_modifier_to_tiling as non-static fa768775a1c0 drm/i915/display: Handle invalid fb_modifier in intel_fb_modifier_to_tiling 38d5f12fa06c drm/i915/display: Split i915 specific code away from intel_fb.c Traceback (most recent call last): File "scripts/spdxcheck.py", line 6, in from ply import lex, yacc ModuleNotFoundError: No module named 'ply' -:173: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #173: new file mode 100644 -:178: WARNING:SPDX_LICENSE_TAG: Improper SPDX comment style for 'drivers/gpu/drm/i915/display/intel_fb_bo.c', please use '//' instead #178: FILE: drivers/gpu/drm/i915/display/intel_fb_bo.c:1: +/* SPDX-License-Identifier: MIT */ -:178: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1 #178: FILE: drivers/gpu/drm/i915/display/intel_fb_bo.c:1: +/* SPDX-License-Identifier: MIT */ total: 0 errors, 3 warnings, 0 checks, 239 lines checked
[Intel-gfx] [PATCH v9 3/4] drm/i915/display: Handle invalid fb_modifier in intel_fb_modifier_to_tiling
Lookup_modifier is returning INTEL_PLANE_CAP_TILING_4 on invalid
fb_modifier value. Use lookup_modifier_or_null in
intel_fb_modifier_to_tiling and return I915_TILING_NONE in case
lookup_modifier_or_null returns null.
Signed-off-by: Jouni Högander
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_fb.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index 6c89cb2f2002..868e39291e9f 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -303,7 +303,14 @@ lookup_format_info(const struct drm_format_info formats[],
unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
{
- u8 tiling_caps = lookup_modifier(fb_modifier)->plane_caps &
+ const struct intel_modifier_desc *md;
+ u8 tiling_caps;
+
+ md = lookup_modifier_or_null(fb_modifier);
+ if (!md)
+ return I915_TILING_NONE;
+
+ tiling_caps = lookup_modifier_or_null(fb_modifier)->plane_caps &
INTEL_PLANE_CAP_TILING_MASK;
switch (tiling_caps) {
--
2.34.1
[Intel-gfx] [PATCH v9 4/4] drm/i915/display: Split i915 specific code away from intel_fb.c
We are preparing for Xe driver. Backing object implementation is differing
between i915 and Xe. Split i915 specific code into separate source file
built only for i915.
v9:
- Use ERR_CAST
v8:
- return original error code from intel_fb_bo_lookup_valid_bo on failure
v7:
- drop #include
- s/user_mode_cmd/mode_cmd/
- Use passed i915 pointer instead of to_i915(obj->base.dev)
v6: Add missing intel_fb_bo.[ch]
v5:
- Keep drm_any_plane_has_format check in intel_fb.c
- Use mode_cmd instead of user_mode_cmd for intel_fb_bo_lookup_valid_bo
v4: Move drm_any_plane_has_format check into intel_fb_bo.c
v3: Fix failure handling in intel_framebuffer_init
v2: Couple of fixes to error value handling
Signed-off-by: Jouni Högander
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/display/intel_fb.c| 73 +++--
drivers/gpu/drm/i915/display/intel_fb_bo.c | 92 ++
drivers/gpu/drm/i915/display/intel_fb_bo.h | 24 ++
4 files changed, 127 insertions(+), 63 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.c
create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index eadc9e612962..e777686190ca 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -280,6 +280,7 @@ i915-y += \
display/intel_dsb.o \
display/intel_dsb_buffer.o \
display/intel_fb.o \
+ display/intel_fb_bo.o \
display/intel_fb_pin.o \
display/intel_fbc.o \
display/intel_fdi.o \
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index 868e39291e9f..33a693460420 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -4,7 +4,6 @@
*/
#include
-#include
#include
#include
@@ -15,6 +14,7 @@
#include "intel_display_types.h"
#include "intel_dpt.h"
#include "intel_fb.h"
+#include "intel_fb_bo.h"
#include "intel_frontbuffer.h"
#define check_array_bounds(i915, a, i) drm_WARN_ON(&(i915)->drm, (i) >=
ARRAY_SIZE(a))
@@ -1985,7 +1985,6 @@ int intel_framebuffer_init(struct intel_framebuffer
*intel_fb,
struct drm_i915_private *dev_priv =
to_i915(intel_bo_to_drm_bo(obj)->dev);
struct drm_framebuffer *fb = &intel_fb->base;
u32 max_stride;
- unsigned int tiling, stride;
int ret = -EINVAL;
int i;
@@ -1993,32 +1992,11 @@ int intel_framebuffer_init(struct intel_framebuffer
*intel_fb,
if (!intel_fb->frontbuffer)
return -ENOMEM;
- i915_gem_object_lock(obj, NULL);
- tiling = i915_gem_object_get_tiling(obj);
- stride = i915_gem_object_get_stride(obj);
- i915_gem_object_unlock(obj);
-
- if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
- /*
-* If there's a fence, enforce that
-* the fb modifier and tiling mode match.
-*/
- if (tiling != I915_TILING_NONE &&
- tiling !=
intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
- drm_dbg_kms(&dev_priv->drm,
- "tiling_mode doesn't match fb modifier\n");
- goto err;
- }
- } else {
- if (tiling == I915_TILING_X) {
- mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
- } else if (tiling == I915_TILING_Y) {
- drm_dbg_kms(&dev_priv->drm,
- "No Y tiling for legacy addfb\n");
- goto err;
- }
- }
+ ret = intel_fb_bo_framebuffer_init(intel_fb, obj, mode_cmd);
+ if (ret)
+ goto err;
+ ret = -EINVAL;
if (!drm_any_plane_has_format(&dev_priv->drm,
mode_cmd->pixel_format,
mode_cmd->modifier[0])) {
@@ -2028,17 +2006,6 @@ int intel_framebuffer_init(struct intel_framebuffer
*intel_fb,
goto err;
}
- /*
-* gen2/3 display engine uses the fence if present,
-* so the tiling mode must match the fb modifier exactly.
-*/
- if (DISPLAY_VER(dev_priv) < 4 &&
- tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
- drm_dbg_kms(&dev_priv->drm,
- "tiling_mode must match fb modifier exactly on
gen2/3\n");
- goto err;
- }
-
max_stride = intel_fb_max_stride(dev_priv, mode_cmd->pixel_format,
mode_cmd->modifier[0]);
if (mode_cmd->pitches[0] > max_stride) {
@@ -2050,17 +2017,6 @@ int intel_framebuffer_init(struct intel_framebuffer
*intel_fb,
goto err;
}
- /*
-* If there's a fence, enforce that
-
[Intel-gfx] [PATCH v9 0/4] Prepare intel_fb for Xe
Intel fb creation is differing between Xe and i915 due to different implementations of backing object. This patch set is splitting i915 specific code into it's own source file. Similar source files will be introduced for Xe as well. Also use intel_bo_to_drm_bo instead of directly referring i915_gem_object->base. One i915_gem_object_put is changed to drm_gem_object_put. v9: - Use ERR_CAST v8: - return original error code from intel_fb_bo_lookup_valid_bo on failure v7: - drop #include - s/user_mode_cmd/mode_cmd/ - Use passed i915 pointer instead of to_i915(obj->base.dev) v6: Add missing intel_fb_bo.[ch] v5: - Keep drm_any_plane_has_format check in intel_fb.c - Use mode_cmd instead of user_mode_cmd for intel_fb_bo_lookup_valid_bo - Use lookup_modifier_or_null in intel_fb_modifier_to_tiling and handle null value v4: Move drm_any_plane_has_format check into intel_fb_bo.c v3: Fix failure handling in intel_framebuffer_init v2: Couple of fixes to error value handling Jouni Högander (4): drm/i915/display: use intel_bo_to_drm_bo in intel_fb.c drm/i915/display: Convert intel_fb_modifier_to_tiling as non-static drm/i915/display: Handle invalid fb_modifier in intel_fb_modifier_to_tiling drm/i915/display: Split i915 specific code away from intel_fb.c drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/display/intel_fb.c| 132 +++-- drivers/gpu/drm/i915/display/intel_fb.h| 2 + drivers/gpu/drm/i915/display/intel_fb_bo.c | 92 ++ drivers/gpu/drm/i915/display/intel_fb_bo.h | 24 5 files changed, 162 insertions(+), 89 deletions(-) create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.c create mode 100644 drivers/gpu/drm/i915/display/intel_fb_bo.h -- 2.34.1
[Intel-gfx] [PATCH v9 2/4] drm/i915/display: Convert intel_fb_modifier_to_tiling as non-static
We are about to split i915 specific code from intel_fb.c. Convert
intel_fb_modifier_to_tiling as non-static to allow calling it from split
code.
Signed-off-by: Jouni Högander
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_fb.c | 40 -
drivers/gpu/drm/i915/display/intel_fb.h | 2 ++
2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index 4af5b7181fdf..6c89cb2f2002 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -301,6 +301,26 @@ lookup_format_info(const struct drm_format_info formats[],
return NULL;
}
+unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
+{
+ u8 tiling_caps = lookup_modifier(fb_modifier)->plane_caps &
+INTEL_PLANE_CAP_TILING_MASK;
+
+ switch (tiling_caps) {
+ case INTEL_PLANE_CAP_TILING_Y:
+ return I915_TILING_Y;
+ case INTEL_PLANE_CAP_TILING_X:
+ return I915_TILING_X;
+ case INTEL_PLANE_CAP_TILING_4:
+ case INTEL_PLANE_CAP_TILING_Yf:
+ case INTEL_PLANE_CAP_TILING_NONE:
+ return I915_TILING_NONE;
+ default:
+ MISSING_CASE(tiling_caps);
+ return I915_TILING_NONE;
+ }
+}
+
/**
* intel_fb_get_format_info: Get a modifier specific format information
* @cmd: FB add command structure
@@ -737,26 +757,6 @@ intel_fb_align_height(const struct drm_framebuffer *fb,
return ALIGN(height, tile_height);
}
-static unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
-{
- u8 tiling_caps = lookup_modifier(fb_modifier)->plane_caps &
-INTEL_PLANE_CAP_TILING_MASK;
-
- switch (tiling_caps) {
- case INTEL_PLANE_CAP_TILING_Y:
- return I915_TILING_Y;
- case INTEL_PLANE_CAP_TILING_X:
- return I915_TILING_X;
- case INTEL_PLANE_CAP_TILING_4:
- case INTEL_PLANE_CAP_TILING_Yf:
- case INTEL_PLANE_CAP_TILING_NONE:
- return I915_TILING_NONE;
- default:
- MISSING_CASE(tiling_caps);
- return I915_TILING_NONE;
- }
-}
-
bool intel_fb_modifier_uses_dpt(struct drm_i915_private *i915, u64 modifier)
{
return HAS_DPT(i915) && modifier != DRM_FORMAT_MOD_LINEAR;
diff --git a/drivers/gpu/drm/i915/display/intel_fb.h
b/drivers/gpu/drm/i915/display/intel_fb.h
index e85167d6bc34..23db6628f53e 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.h
+++ b/drivers/gpu/drm/i915/display/intel_fb.h
@@ -95,4 +95,6 @@ intel_user_framebuffer_create(struct drm_device *dev,
bool intel_fb_modifier_uses_dpt(struct drm_i915_private *i915, u64 modifier);
bool intel_fb_uses_dpt(const struct drm_framebuffer *fb);
+unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier);
+
#endif /* __INTEL_FB_H__ */
--
2.34.1
[Intel-gfx] [PATCH v9 1/4] drm/i915/display: use intel_bo_to_drm_bo in intel_fb.c
We are preparing for Xe driver. I915 and Xe object implementation are
differing. Do not use i915_gem_object->base directly. Instead use
intel_bo_to_drm_bo.
Also use drm_gem_object_put instead of i915_gem_object_put. This should be
ok as i915_gem_object_put is really just doing __drm_gem_object_put.
Signed-off-by: Jouni Högander
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_fb.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index 6d48aa3af95a..4af5b7181fdf 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -1657,10 +1657,10 @@ int intel_fill_fb_info(struct drm_i915_private *i915,
struct intel_framebuffer *
max_size = max(max_size, offset + size);
}
- if (mul_u32_u32(max_size, tile_size) > obj->base.size) {
+ if (mul_u32_u32(max_size, tile_size) > intel_bo_to_drm_bo(obj)->size) {
drm_dbg_kms(&i915->drm,
"fb too big for bo (need %llu bytes, have %zu
bytes)\n",
- mul_u32_u32(max_size, tile_size), obj->base.size);
+ mul_u32_u32(max_size, tile_size),
intel_bo_to_drm_bo(obj)->size);
return -EINVAL;
}
@@ -1889,7 +1889,7 @@ static int intel_user_framebuffer_create_handle(struct
drm_framebuffer *fb,
unsigned int *handle)
{
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ struct drm_i915_private *i915 = to_i915(intel_bo_to_drm_bo(obj)->dev);
if (i915_gem_object_is_userptr(obj)) {
drm_dbg(&i915->drm,
@@ -1897,7 +1897,7 @@ static int intel_user_framebuffer_create_handle(struct
drm_framebuffer *fb,
return -EINVAL;
}
- return drm_gem_handle_create(file, &obj->base, handle);
+ return drm_gem_handle_create(file, intel_bo_to_drm_bo(obj), handle);
}
struct frontbuffer_fence_cb {
@@ -1975,7 +1975,7 @@ int intel_framebuffer_init(struct intel_framebuffer
*intel_fb,
struct drm_i915_gem_object *obj,
struct drm_mode_fb_cmd2 *mode_cmd)
{
- struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+ struct drm_i915_private *dev_priv =
to_i915(intel_bo_to_drm_bo(obj)->dev);
struct drm_framebuffer *fb = &intel_fb->base;
u32 max_stride;
unsigned int tiling, stride;
@@ -2153,7 +2153,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
}
fb = intel_framebuffer_create(obj, &mode_cmd);
- i915_gem_object_put(obj);
+ drm_gem_object_put(intel_bo_to_drm_bo(obj));
return fb;
}
--
2.34.1
