dri-devel September 2021

dri-devel@lists.freedesktop.org

348 participants
817 discussions

[PATCH v2 0/6] drm/i915: Suspend / resume backup- and restore of LMEM.
by Thomas Hellström 10 Sep '21

10 Sep '21

Implement backup and restore of LMEM during suspend / resume. What complicates things a bit is handling of pinned LMEM memory during suspend and the fact that we might be dealing with unmappable LMEM in the future, which makes us want to restrict the number of pinned objects that need memcpy resume. The first two patches are prereq patches implementing object content copy and a generic means of iterating through all objects in a region. The third patch adds the backup / recover / restore functions and the two last patches deal with restricting the number of objects we need to use memcpy for. v2: - Some polishing of patch 4/6, see patch commit message for details (Chris Wilson) - Rework of patch 3/6. Thomas Hellström (6): drm/i915/ttm: Implement a function to copy the contents of two TTM-base objects drm/i915/gem: Implement a function to process all gem objects of a region drm/i915 Implement LMEM backup and restore for suspend / resume drm/i915/gt: Register the migrate contexts with their engines drm/i915: Don't back up pinned LMEM context images and rings during suspend drm/i915: Reduce the number of objects subject to memcpy recover drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gem/i915_gem_context.c | 4 +- .../gpu/drm/i915/gem/i915_gem_object_types.h | 21 +- drivers/gpu/drm/i915/gem/i915_gem_pm.c | 94 +++++++- drivers/gpu/drm/i915/gem/i915_gem_pm.h | 3 +- drivers/gpu/drm/i915/gem/i915_gem_region.c | 70 ++++++ drivers/gpu/drm/i915/gem/i915_gem_region.h | 33 +++ drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 98 ++++++-- drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 14 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c | 210 ++++++++++++++++++ drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.h | 24 ++ .../gpu/drm/i915/gem/selftests/huge_pages.c | 2 +- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 2 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 5 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 4 +- drivers/gpu/drm/i915/gt/intel_context_types.h | 8 + drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 + drivers/gpu/drm/i915/gt/intel_engine_pm.c | 23 ++ drivers/gpu/drm/i915/gt/intel_engine_pm.h | 2 + drivers/gpu/drm/i915/gt/intel_engine_types.h | 7 + .../drm/i915/gt/intel_execlists_submission.c | 2 + drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 4 +- drivers/gpu/drm/i915/gt/intel_gtt.c | 3 +- drivers/gpu/drm/i915/gt/intel_gtt.h | 9 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 3 +- drivers/gpu/drm/i915/gt/intel_migrate.c | 2 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 13 +- drivers/gpu/drm/i915/gt/intel_ring.c | 3 +- .../gpu/drm/i915/gt/intel_ring_submission.c | 3 + drivers/gpu/drm/i915/gt/mock_engine.c | 2 + drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 12 +- drivers/gpu/drm/i915/gvt/scheduler.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 10 +- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 +- 38 files changed, 649 insertions(+), 60 deletions(-) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.h -- 2.31.1

2 15

[PATCH RESEND] drm/i915: Mark GPU wedging on driver unregister unrecoverable
by Janusz Krzysztofik 10 Sep '21

10 Sep '21

GPU wedged flag now set on driver unregister to prevent from further using the GPU can be then cleared unintentionally when calling __intel_gt_unset_wedged() still before the flag is finally marked unrecoverable. We need to have it marked unrecoverable earlier. Implement that by replacing a call to intel_gt_set_wedged() in intel_gt_driver_unregister() with intel_gt_set_wedged_on_fini(). With the above in place, intel_gt_set_wedged_on_fini() is now called twice on driver remove, second time from __intel_gt_disable(). This seems harmless, while dropping intel_gt_set_wedged_on_fini() from __intel_gt_disable() proved to break some driver probe error unwind paths as well as mock selftest exit path. Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik(a)linux.intel.com> Cc: Michał Winiarski <michal.winiarski(a)intel.com> --- Resending with Cc: dri-devel(a)lists.freedesktop.org as requested. Thanks, Janusz drivers/gpu/drm/i915/gt/intel_gt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 62d40c986642..173b53cb2b47 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -750,7 +750,7 @@ void intel_gt_driver_unregister(struct intel_gt *gt) * all in-flight requests so that we can quickly unbind the active * resources. */ - intel_gt_set_wedged(gt); + intel_gt_set_wedged_on_fini(gt); /* Scrub all HW state upon release */ with_intel_runtime_pm(gt->uncore->rpm, wakeref) -- 2.25.1

2 1

[PATCH v8 00/17] drm/i915: Introduce Intel PXP
by Daniele Ceraolo Spurio 10 Sep '21

10 Sep '21

PXP (Protected Xe Path) is an i915 component, available on GEN12+, that helps to establish the hardware protected session and manage the status of the alive software session, as well as its life cycle. The main change from v7 is that we no longer increase the guilty count when a context is banned due to PXP invalidation Tested with: https://patchwork.freedesktop.org/series/87570/ Cc: Gaurav Kumar <kumar.gaurav(a)intel.com> Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Rodrigo Vivi <rodrigo.vivi(a)intel.com> Cc: Joonas Lahtinen <joonas.lahtinen(a)linux.intel.com> Cc: Juston Li <juston.li(a)intel.com> Cc: Alan Previn <alan.previn.teres.alexis(a)intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin(a)intel.com> Cc: Jason Ekstrand <jason(a)jlekstrand.net> Cc: Daniel Vetter <daniel.vetter(a)intel.com> Anshuman Gupta (2): drm/i915/pxp: Add plane decryption support drm/i915/pxp: black pixels on pxp disabled Daniele Ceraolo Spurio (9): drm/i915/pxp: Define PXP component interface drm/i915/pxp: define PXP device flag and kconfig drm/i915/pxp: allocate a vcs context for pxp usage drm/i915/pxp: set KCR reg init drm/i915/pxp: interfaces for using protected objects drm/i915/pxp: start the arb session on demand drm/i915/pxp: add pxp debugfs drm/i915/pxp: add PXP documentation drm/i915/pxp: enable PXP for integrated Gen12 Huang, Sean Z (5): drm/i915/pxp: Implement funcs to create the TEE channel drm/i915/pxp: Create the arbitrary session after boot drm/i915/pxp: Implement arb session teardown drm/i915/pxp: Implement PXP irq handler drm/i915/pxp: Enable PXP power management Vitaly Lubart (1): mei: pxp: export pavp client to me client bus Documentation/gpu/i915.rst | 8 + drivers/gpu/drm/i915/Kconfig | 11 + drivers/gpu/drm/i915/Makefile | 10 + drivers/gpu/drm/i915/display/intel_display.c | 34 +++ .../drm/i915/display/intel_display_types.h | 6 + .../drm/i915/display/skl_universal_plane.c | 49 ++- drivers/gpu/drm/i915/gem/i915_gem_context.c | 100 +++++- drivers/gpu/drm/i915/gem/i915_gem_context.h | 6 + .../gpu/drm/i915/gem/i915_gem_context_types.h | 28 ++ drivers/gpu/drm/i915/gem/i915_gem_create.c | 72 +++-- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 18 ++ drivers/gpu/drm/i915/gem/i915_gem_object.c | 1 + drivers/gpu/drm/i915/gem/i915_gem_object.h | 6 + .../gpu/drm/i915/gem/i915_gem_object_types.h | 8 + .../gpu/drm/i915/gem/selftests/mock_context.c | 4 +- drivers/gpu/drm/i915/gt/debugfs_gt.c | 2 + drivers/gpu/drm/i915/gt/intel_engine.h | 2 + drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 22 +- drivers/gpu/drm/i915/gt/intel_gt.c | 5 + drivers/gpu/drm/i915/gt/intel_gt_irq.c | 7 + drivers/gpu/drm/i915/gt/intel_gt_pm.c | 15 +- drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 + drivers/gpu/drm/i915/i915_drv.c | 2 + drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_pci.c | 2 + drivers/gpu/drm/i915/i915_reg.h | 48 +++ drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/pxp/intel_pxp.c | 287 ++++++++++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp.h | 67 ++++ drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c | 141 +++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_cmd.h | 15 + drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 78 +++++ drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h | 21 ++ drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 100 ++++++ drivers/gpu/drm/i915/pxp/intel_pxp_irq.h | 32 ++ drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 40 +++ drivers/gpu/drm/i915/pxp/intel_pxp_pm.h | 23 ++ drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 175 +++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_session.h | 15 + drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 172 +++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_tee.h | 17 ++ .../drm/i915/pxp/intel_pxp_tee_interface.h | 37 +++ drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 83 +++++ drivers/misc/mei/Kconfig | 2 + drivers/misc/mei/Makefile | 1 + drivers/misc/mei/pxp/Kconfig | 13 + drivers/misc/mei/pxp/Makefile | 7 + drivers/misc/mei/pxp/mei_pxp.c | 229 ++++++++++++++ drivers/misc/mei/pxp/mei_pxp.h | 18 ++ include/drm/i915_component.h | 1 + include/drm/i915_pxp_tee_interface.h | 42 +++ include/uapi/drm/i915_drm.h | 55 +++- 52 files changed, 2102 insertions(+), 42 deletions(-) create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp.h create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_cmd.h create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_irq.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_irq.h create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_pm.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_pm.h create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_session.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_session.h create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_tee.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_tee.h create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_tee_interface.h create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_types.h create mode 100644 drivers/misc/mei/pxp/Kconfig create mode 100644 drivers/misc/mei/pxp/Makefile create mode 100644 drivers/misc/mei/pxp/mei_pxp.c create mode 100644 drivers/misc/mei/pxp/mei_pxp.h create mode 100644 include/drm/i915_pxp_tee_interface.h -- 2.25.1

2 21

i915 ttm_tt shmem backend
by Matthew Auld 10 Sep '21

10 Sep '21

Hi Christian, We are looking into using shmem as a ttm_tt backend in i915 for cached system memory objects. We would also like to make such objects visible to the i915-gem shrinker, so that they may be swapped out or discarded when under memory pressure. One idea for handling this is roughly something like: - Add a new TTM_PAGE_FLAG_SHMEM flag, or similar. - Skip the ttm_pages_allocated accounting on such objects, similar to how FLAG_SG is already handled. - Skip all the page->mapping and page->index related bits, like in tt_add_mapping, since it looks like these are set and used by shmem. Not sure what functionally this might break, but looks like it's maybe only driver specific? - Skip calling into ttm_bo_swap_out/in and just have ttm_populate/unpopulate handle this directly for such objects. - Make such objects visible to the i915-gem shrinker. Does this approach look acceptable?

4 8

[PATCH] drm/ttm: add a BUG_ON in ttm_set_driver_manager when array bounds
by Guchun Chen 10 Sep '21

10 Sep '21

Vendor will define their own memory types on top of TTM_PL_PRIV, but call ttm_set_driver_manager directly without checking mem_type value when setting up memory manager. So add such check to aware the case when array bounds. Signed-off-by: Leslie Shi <Yuliang.Shi(a)amd.com> Signed-off-by: Guchun Chen <guchun.chen(a)amd.com> --- include/drm/ttm/ttm_device.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index 7a0f561c57ee..24ad76ca8022 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -308,6 +308,7 @@ ttm_manager_type(struct ttm_device *bdev, int mem_type) static inline void ttm_set_driver_manager(struct ttm_device *bdev, int type, struct ttm_resource_manager *manager) { + BUG_ON(type >= TTM_NUM_MEM_TYPES); bdev->man_drv[type] = manager; } -- 2.17.1

4 3

[bug report] vmwgfx: Implement fence objects
by Dan Carpenter 10 Sep '21

10 Sep '21

Hello Thomas Hellstrom, The patch ae2a104058e2: "vmwgfx: Implement fence objects" from Sep 1, 2011, leads to the following Smatch static checker warning: drivers/dma-buf/dma-fence.c:790 dma_fence_default_wait() warn: user controlled unbound timeout drivers/gpu/drm/vmwgfx/vmwgfx_fence.c 784 int vmw_fence_obj_wait_ioctl(struct drm_device *dev, void *data, 785 struct drm_file *file_priv) 786 { 787 struct drm_vmw_fence_wait_arg *arg = 788 (struct drm_vmw_fence_wait_arg *)data; 789 unsigned long timeout; 790 struct ttm_base_object *base; 791 struct vmw_fence_obj *fence; 792 struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile; 793 int ret; 794 uint64_t wait_timeout = ((uint64_t)arg->timeout_us * HZ); timeout comes from the ioctl. 795 796 /* 797 * 64-bit division not present on 32-bit systems, so do an 798 * approximation. (Divide by 1000000). 799 */ 800 801 wait_timeout = (wait_timeout >> 20) + (wait_timeout >> 24) - 802 (wait_timeout >> 26); 803 804 if (!arg->cookie_valid) { 805 arg->cookie_valid = 1; 806 arg->kernel_cookie = jiffies + wait_timeout; 807 } 808 809 base = vmw_fence_obj_lookup(tfile, arg->handle); 810 if (IS_ERR(base)) 811 return PTR_ERR(base); 812 813 fence = &(container_of(base, struct vmw_user_fence, base)->fence); 814 815 timeout = jiffies; 816 if (time_after_eq(timeout, (unsigned long)arg->kernel_cookie)) { 817 ret = ((vmw_fence_obj_signaled(fence)) ? 818 0 : -EBUSY); 819 goto out; 820 } 821 822 timeout = (unsigned long)arg->kernel_cookie - timeout; 823 824 ret = vmw_fence_obj_wait(fence, arg->lazy, true, timeout); This is a new Smatch warning. To try figure out places which can trigger sysbot "task hung" warnings. I don't know if an upper bound on timeout is appropriate here because this is new experimental check... 825 826 out: 827 ttm_base_object_unref(&base); regards, dan carpenter

1 0

[PULL] drm-misc-fixes
by Thomas Zimmermann 10 Sep '21

10 Sep '21

Hi Dave and Daniel, here's this week's PR for drm-misc-fixes. One patch is a potential deadlock in TTM, the other enables an additional plane in kmb. I'm slightly unhappy that the latter one ended up in -fixes as it's not a bugfix AFAICT. Best regards Thomas drm-misc-fixes-2021-09-08: Short summary of fixes pull: * kmb: Emable second plane * ttm: Fix potential deadlock during swap The following changes since commit fa0b1ef5f7a694f48e00804a391245f3471aa155: drm: Copy drm_wait_vblank to user before returning (2021-08-17 13:56:03 -0400) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2021-09-08 for you to fetch changes up to c8704b7ec182f9293e6a994310c7d4203428cdfb: drm/kmb: Enable alpha blended second plane (2021-09-07 10:10:30 -0700) ---------------------------------------------------------------- Short summary of fixes pull: * kmb: Emable second plane * ttm: Fix potential deadlock during swap ---------------------------------------------------------------- Edmund Dea (1): drm/kmb: Enable alpha blended second plane xinhui pan (1): drm/ttm: Fix a deadlock if the target BO is not idle during swap drivers/gpu/drm/kmb/kmb_drv.c | 8 ++-- drivers/gpu/drm/kmb/kmb_drv.h | 5 +++ drivers/gpu/drm/kmb/kmb_plane.c | 81 ++++++++++++++++++++++++++++++++++++----- drivers/gpu/drm/kmb/kmb_plane.h | 5 ++- drivers/gpu/drm/kmb/kmb_regs.h | 3 ++ drivers/gpu/drm/ttm/ttm_bo.c | 6 +-- 6 files changed, 90 insertions(+), 18 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer

3 3

Re: linux-next: manual merge of the amdgpu tree with Linus' tree
by Stephen Rothwell 10 Sep '21

10 Sep '21

Hi all, On Thu, 2 Sep 2021 11:22:08 +1000 Stephen Rothwell <sfr(a)canb.auug.org.au> wrote: > > Today's linux-next merge of the amdgpu tree got a conflict in: > > drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c > > between commit: > > 477f70cd2a67 ("Merge tag 'drm-next-2021-08-31-1' of git://anongit.freedesktop.org/drm/drm") > > from Linus' tree and commit: > > cc947bf91bad ("drm/amdgpu: Process any VBIOS RAS EEPROM address") > > from the amdgpu tree. > > I fixed it up (I used the latter version) and can carry the fix as > necessary. This is now fixed as far as linux-next is concerned, but any > non trivial conflicts should be mentioned to your upstream maintainer > when your tree is submitted for merging. You may also want to consider > cooperating with the maintainer of the conflicting tree to minimise any > particularly complex conflicts. This is now a conflict between the drm tree and Linus' tree. -- Cheers, Stephen Rothwell

1 0

Re: [Intel-gfx] [PATCH] drm/i915/gt: Add separate MOCS table for Gen12 devices other than TGL/RKL
by Ville Syrjälä 09 Sep '21

09 Sep '21

On Thu, Sep 09, 2021 at 10:15:56AM -0700, Matt Roper wrote: > On Thu, Sep 09, 2021 at 06:09:55PM +0300, Ville Syrjälä wrote: > > On Thu, Sep 09, 2021 at 08:00:02AM -0700, Matt Roper wrote: > > > On Thu, Sep 09, 2021 at 05:39:26PM +0300, Ville Syrjälä wrote: > > > > On Thu, Sep 09, 2021 at 07:29:33AM -0700, Matt Roper wrote: > > > > > On Thu, Sep 09, 2021 at 04:58:50PM +0300, Ville Syrjälä wrote: > > > > > > On Tue, Sep 07, 2021 at 11:19:29AM -0700, Matt Roper wrote: > > > > > > > On Tue, Sep 07, 2021 at 08:41:06PM +0300, Ville Syrjälä wrote: > > > > > > > > On Tue, Sep 07, 2021 at 10:27:28AM -0700, Matt Roper wrote: > > > > > > > > > On Tue, Sep 07, 2021 at 10:46:39PM +0530, Ayaz A Siddiqui wrote: > > > > > > > > > > MOCS table of TGL/RKL has MOCS[1] set to L3_UC. > > > > > > > > > > While for other gen12 devices we need to set MOCS[1] as L3_WB, > > > > > > > > > > So adding a new MOCS table for other gen 12 devices eg. ADL. > > > > > > > > > > > > > > > > > > > > Fixes: cfbe5291a189 ("drm/i915/gt: Initialize unused MOCS entries with device specific values") > > > > > > > > > > Cc: Matt Roper <matthew.d.roper(a)intel.com> > > > > > > > > > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui(a)intel.com> > > > > > > > > > > > > > > > > > > Yep, we overlooked that the TGL table still had an explicit entry for > > > > > > > > > I915_MOCS_PTE and wasn't just using an implicit 'unused_entries' lookup > > > > > > > > > for MOCS[1]. The new table is the same as the TGL table, just with > > > > > > > > > I915_MOCS_PTE (1) removed. > > > > > > > > > > > > > > > > And just how are people planning on handling display cacheability > > > > > > > > control without a PTE MOCS entry? Is Mesa/etc. already making all > > > > > > > > external bos uncached on these platforms just in case we might > > > > > > > > scan out said bo? > > > > > > > > > > > > > > MOCS entry 1 has never been considered a valid MOCS table entry on gen12 > > > > > > > platforms (despite the old #define, it's not actually related to PTE, > > > > > > > display, etc. anymore). > > > > > > > > > > > > So can someone finally explain to me how we're supposed to cache > > > > > > anything that might become a scanout buffer later (eg. window system > > > > > > buffers)? Or are we just making everything like that UC now, and is > > > > > > everyone happy with that? Is userspace actually following that? > > > > > > > > > > Table entry #1 has never had anything to do with scanout on gen12+. I > > > > > would assume that UMDs are either using the display entry in the MOCS > > > > > table (which is 61 on gen12+) or some other UC entry. > > > > > > > > If 61 is meant to to be the new PTE entry wy hasn't it been defines as > > > > such in the code? And I know for a fact that userspace (Mesa) is not > > > > > > There is no "PTE" entry anymore. But 61 is already documented as > > > "displayable" in both the spec and the code: > > > > > > /* HW Special Case (Displayable) */ > > > MOCS_ENTRY(61, > > > > Why is it called a "HW special case"? I don't think there's any hw > > magic in there? > > > > And why aren't we setting it to PTE to get some cacheability for > > window back buffers and such? > > Who is "we" here? We who care about the performance of the system. > The MOCS table is a pre-defined set of per-platform > magic numbers. The software teams don't get to decide what the values > are, we just program the hardware with the per-platform numbers that > have been agreed upon as part of a platform-wide stack (everything from > low-level firmware to high level userspace should be working from the > same table, defined in the bspec). The magic numbers must be based on something. If that something is purely Windows behaviour/performance then we might be shooting ourselves in the foot here. > > Once we know what the per-platform magic numbers are, we're supposed to > pick the table entry that matches the behavior we're trying to > accomplish. If you want some specific level of cacheability, then you > select a table row that gives you that. Maybe 61 isn't the best > setting, I don't know; userspace can pick whichever defined setting is > actually best, using the data from the table. But table row #1 is > already well-documented as reserved/dontuse across the full stack; the > fact that row #1 had values similar to PTE on Icelake hardware doesn't > carry forward to any post-gen11 platform. The only way you can get LLC cacheability for an external BO (window back buffers and such) is by using a MOCS entry that directs the hardware to consult the PTEs. Otherwise the client doing the rendering would have to know ahead of time whether the buffer is going to be directly scanned out by the compositor or not, for which there is no protocol in X or wayland. Historically I believe LLC cacheability has been on average a win. Some workloads can do better with UC though. So if we are giving up on LLC cacheability we should have some numbers to back up that decision so that we're not dropping tons of performance on the floor. > > > > using entry 61. I think there is a massive communication gap here > > > > where everyone just seems to assume the other side is doing something. > > > > > > > > Could someone actually come up with a clear abi definition for this > > > > and get all the stakeholders to sign off on it? > > > > > > The agreement between the i915 team, various userspace teams, Windows > > > driver team, hardware architects, software architects, and bspec writers > > > was just completed; that's what triggered the kernel updates here (and > > > I'm guessing is triggering similar work on the UMD side). It's also why > > > we held off on removing the force_probe flag on ADL until now since we > > > couldn't consider enablement of the platform complete until the > > > agreement and definitions here was finalized. > > > > Can we get that agreement visible on the mailing list? Since MOCS is > > abi I don't see why we shouldn't follow the normal abi rules for these, > > ie. post to dri-devel, get acks from relevant people, links to agreed > > userspace changes, etc. > > The ABI design here was designed and agreed upon years ago, during early > gen11 development. The ABI design is that the kernel driver will > faithfully initialize the hardware with the pre-determined set of magic > numbers documented by the hardware team. Since these are > well-documented and unchanging numbers per-platform, there's no > ambiguity for userspace, firmware, etc. about what a specific mocs index > means, and no need to provide additional ABI for userspace to query what > the kernel used in each row or anything like that. The specific magic > numbers are also ABI in the sense that we can't change the set of > defined values once they're set for a platform (and it's been a long > road to get the hardware and other OS software teams to understand and > agree to this requirement), but we don't get to define or overrule what > the initial values and order of those magic numbers are. And this apporach has clearly not worked considering userspace and kernel have not agreed on what the abi is. We need to do better. > > What is a bit vague in the formal documentation is what should be done > about the reserved/dontuse table entries. In theory it wouldn't matter > since they'd never be used anyway, but in reality userspace can still > use them by accident, such as by forgetting to update their MOCS > selection logic from past platforms (e.g., still trying to use row #1 on > platforms where it isn't defined). Given that it's legal for entries to > be added to MOCS tables, but never removed/modified, it follows that we > should always initialize the undefined entries to fully cached; if a > MOCS table update happens in the future and new rows show up, they can > only become more coherent, and any userspace software that was > incorrectly trying to use them previously will remain functionally > correct. > > What you're proposing would be a change to existing ABI --- instead of > following the agreed upon contract, i915 would start defining its own > set of magic numbers that potentially contradict the documentation that > every other team is depending on. We already made this mistake on > TGL/RKL, so due to an i915 bug we're outside the spec; if entry #1 ever > becomes a formally defined setting in the future, the rest of the > software stack will need to explicitly work around i915's bug since we > can't fix it now without breaking ABI. > > If you really want to redefine how the MOCS ABI works and have i915 no > longer follow the current contract, I think you need to do the > following: I want the abi to be actually defined properly, and some assurance that all the stakeholders implement it correctly. Following the proper abi rules for kernel development would guarantee that. I would also like if the abi can give us the best performance rather than potentially crippling it. Ie. I would expect to have a PTE MOCS setting for potential scanout buffers, or some proof that pure UC will in fact be a better choice. > > * Re-add force_probe requirement to ADL and add appropriate Fixes: so > that the platform isn't enabled yet. > * Document clearly how you want i915 to select the MOCS settings it > uses for each table row if we're no longer going to follow the > documented values in the bspec. > * Provide a way for userspace to determine how i915 has defined the > MOCS settings (since they can no longer rely on us following the > previously agreed upon contract). > * Get acks from all the userspace teams on the new direction you're > proposing. > * Get an ack from the GuC team to make sure that programming > MOCS values differently than documented in the bspec won't have any > kind of impact on their operation. > > Alternatively, you could lobby for a new table row #1 to be added to the > formal MOCS table for gen12. It's legal for new MOCS entries to show up > in the future, so if that happens, we can follow up with the > corresponding change in i915; since we'll be moving to a more coherent > value (from today's fully cached entry), we'll be becoming more > permissive from a correctness point of view. But we absolutely should > not try to add entries unilaterally in i915 that haven't been formally > agreed upon because they may clash with a different definition of the > row that shows up in the future through formal channels. -- Ville Syrjälä Intel

2 4

[Bug 213917] New: Screen starts flickering when laptop(amdgpu) wakes up after suspend.
by bugzilla-daemon＠bugzilla.kernel.org 09 Sep '21

09 Sep '21

https://bugzilla.kernel.org/show_bug.cgi?id=213917 Bug ID: 213917 Summary: Screen starts flickering when laptop(amdgpu) wakes up after suspend. Product: Drivers Version: 2.5 Kernel Version: 5.13.6 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri(a)kernel-bugs.osdl.org Reporter: utkarsh.naveen(a)gmail.com Regression: No I have an ASUS FX505DT with an AMD Ryzen 3550H APU. This is not a new bug and it was fixed in kernel v5.6 with commit hash: eb916a5a93a64c182b0a8f43886aa6bb4c3e52b0 I haven't had the time to test out each of the kernel versions individually, but this bug isn't there in Linux v5.12.6 and only appeared after I updated to v5.13.5. ## Steps to reproduce 1. Let the laptop suspend. 2. Wake it up with some keypresses or something. 3. After waking up, the screen starts flickering on/off every second. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.

1 2

← Newer
1
...
61
62
63
64
65
66
67
...
82
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel September 2021