dri-devel June 2022

dri-devel@lists.freedesktop.org

332 participants
697 discussions

[PATCH 1/2] iosys-map: Add per-word read
by Lucas De Marchi 17 Jun '22

17 Jun '22

Instead of always falling back to memcpy_fromio() for any size, prefer using read{b,w,l}(). When reading struct members it's common to read individual integer variables individually. Going through memcpy_fromio() for each of them poses a high penalty. Employ a similar trick as __seqprop() by using _Generic() to generate only the specific call based on a type-compatible variable. For a pariticular i915 workload producing GPU context switches, __get_engine_usage_record() is particularly hot since the engine usage is read from device local memory with dgfx, possibly multiple times since it's racy. Test execution time for this test shows a ~12.5% improvement with DG2: Before: nrepeats = 1000; min = 7.63243e+06; max = 1.01817e+07; median = 9.52548e+06; var = 526149; After: nrepeats = 1000; min = 7.03402e+06; max = 8.8832e+06; median = 8.33955e+06; var = 333113; Other things attempted that didn't prove very useful: 1) Change the _Generic() on x86 to just dereference the memory address 2) Change __get_engine_usage_record() to do just 1 read per loop, comparing with the previous value read 3) Change __get_engine_usage_record() to access the fields directly as it was before the conversion to iosys-map (3) did gave a small improvement (~3%), but doesn't seem to scale well to other similar cases in the driver. Additional test by Chris Wilson using gem_create from igt with some changes to track object creation time. This happens to accidentally stress this code path: Pre iosys_map conversion of engine busyness: lmem0: Creating 262144 4KiB objects took 59274.2ms Unpatched: lmem0: Creating 262144 4KiB objects took 108830.2ms With readl (this patch): lmem0: Creating 262144 4KiB objects took 61348.6ms s/readl/READ_ONCE/ lmem0: Creating 262144 4KiB objects took 61333.2ms So we do take a little bit more time than before the conversion, but that is due to other factors: bringing the READ_ONCE back would be as good as just doing this conversion. v2: - Remove default from _Generic() - callers wanting to read more than u64 should use iosys_map_memcpy_from() - Add READ_ONCE() cases dereferencing the pointer when using system memory Signed-off-by: Lucas De Marchi <lucas.demarchi(a)intel.com> Reviewed-by: Christian König <christian.koenig(a)amd.com> # v1 --- include/linux/iosys-map.h | 45 +++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h index 4b8406ee8bc4..f59dd00ed202 100644 --- a/include/linux/iosys-map.h +++ b/include/linux/iosys-map.h @@ -6,6 +6,7 @@ #ifndef __IOSYS_MAP_H__ #define __IOSYS_MAP_H__ +#include <linux/compiler_types.h> #include <linux/io.h> #include <linux/string.h> @@ -333,6 +334,26 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset, memset(dst->vaddr + offset, value, len); } +#ifdef CONFIG_64BIT +#define __iosys_map_rd_io_u64_case(val_, vaddr_iomem_) \ + u64: val_ = readq(vaddr_iomem_) +#else +#define __iosys_map_rd_io_u64_case(val_, vaddr_iomem_) \ + u64: memcpy_fromio(&(val_), vaddr_iomem__, sizeof(u64)) +#endif + +#define __iosys_map_rd_io(val__, vaddr_iomem__, type__) _Generic(val__, \ + u8: val__ = readb(vaddr_iomem__), \ + u16: val__ = readw(vaddr_iomem__), \ + u32: val__ = readl(vaddr_iomem__), \ + __iosys_map_rd_io_u64_case(val__, vaddr_iomem__)) + +#define __iosys_map_rd_sys(val__, vaddr__, type__) ({ \ + compiletime_assert(sizeof(type__) <= sizeof(u64), \ + "Unsupported access size for __iosys_map_rd_sys()"); \ + val__ = READ_ONCE(*((type__ *)vaddr__)); \ +}) + /** * iosys_map_rd - Read a C-type value from the iosys_map * @@ -340,16 +361,21 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset, * @offset__: The offset from which to read * @type__: Type of the value being read * - * Read a C type value from iosys_map, handling possible un-aligned accesses to - * the mapping. + * Read a C type value (u8, u16, u32 and u64) from iosys_map. For other types or + * if pointer may be unaligned (and problematic for the architecture supported), + * use iosys_map_memcpy_from(). * * Returns: * The value read from the mapping. */ -#define iosys_map_rd(map__, offset__, type__) ({ \ - type__ val; \ - iosys_map_memcpy_from(&val, map__, offset__, sizeof(val)); \ - val; \ +#define iosys_map_rd(map__, offset__, type__) ({ \ + type__ val; \ + if ((map__)->is_iomem) { \ + __iosys_map_rd_io(val, (map__)->vaddr_iomem + (offset__), type__);\ + } else { \ + __iosys_map_rd_sys(val, (map__)->vaddr + (offset__), type__); \ + } \ + val; \ }) /** @@ -379,9 +405,10 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset, * * Read a value from iosys_map considering its layout is described by a C struct * starting at @struct_offset__. The field offset and size is calculated and its - * value read handling possible un-aligned memory accesses. For example: suppose - * there is a @struct foo defined as below and the value ``foo.field2.inner2`` - * needs to be read from the iosys_map: + * value read. If the field access would incur in un-aligned access, then either + * iosys_map_memcpy_from() needs to be used or the architecture must support it. + * For example: suppose there is a @struct foo defined as below and the value + * ``foo.field2.inner2`` needs to be read from the iosys_map: * * .. code-block:: c * -- 2.36.1

1 2

[PATCH v4] drm/i915: Improve on suspend / resume time with VT-d enabled
by Thomas Hellström 17 Jun '22

17 Jun '22

When DMAR / VT-d is enabled, the display engine uses overfetching, presumably to deal with the increased latency. To avoid display engine errors and DMAR faults, as a workaround the GGTT is populated with scatch PTEs when VT-d is enabled. However starting with gen10, Write-combined writing of scratch PTES is no longer possible and as a result, populating the full GGTT with scratch PTEs like on resume becomes very slow as uncached access is needed. Therefore, on integrated GPUs utilize the fact that the PTEs are stored in stolen memory which retain content across S3 suspend. Don't clear the PTEs on suspend and resume. This improves on resume time with around 100 ms. While 100+ms might appear like a short time it's 10% to 20% of total resume time and important in some applications. One notable exception is Intel Rapid Start Technology which may cause stolen memory to be lost across what the OS percieves as S3 suspend. If IRST is enabled or if we can't detect whether IRST is enabled, retain the old workaround, clearing and re-instating PTEs. As an additional measure, if we detect that the last ggtt pte was lost during suspend, print a warning and re-populate the GGTT ptes On discrete GPUs, the display engine scans out from LMEM which isn't subject to DMAR, and presumably the workaround is therefore not needed, but that needs to be verified and disabling the workaround for dGPU, if possible, will be deferred to a follow-up patch. v2: - Rely on retained ptes to also speed up suspend and resume re-binding. - Re-build GGTT ptes if Intel rst is enabled. v3: - Re-build GGTT ptes also if we can't detect whether Intel rst is enabled, and if the guard page PTE and end of GGTT was lost. v4: - Fix some kerneldoc issues (Matthew Auld), rebase. Signed-off-by: Thomas Hellström <thomas.hellstrom(a)linux.intel.com> Acked-by: Daniel Vetter <daniel.vetter(a)ffwll.ch> Reviewed-by: Matthew Auld <matthew.auld(a)intel.com> --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 56 +++++++++++++++++++++++++--- drivers/gpu/drm/i915/gt/intel_gtt.h | 24 ++++++++++++ drivers/gpu/drm/i915/i915_driver.c | 16 ++++++++ 3 files changed, 90 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index e6b2eb122ad7..0849a6f66309 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -22,6 +22,13 @@ #include "intel_gtt.h" #include "gen8_ppgtt.h" +static inline bool suspend_retains_ptes(struct i915_address_space *vm) +{ + return GRAPHICS_VER(vm->i915) >= 8 && + !HAS_LMEM(vm->i915) && + vm->is_ggtt; +} + static void i915_ggtt_color_adjust(const struct drm_mm_node *node, unsigned long color, u64 *start, @@ -93,6 +100,23 @@ int i915_ggtt_init_hw(struct drm_i915_private *i915) return 0; } +/* + * Return the value of the last GGTT pte cast to an u64, if + * the system is supposed to retain ptes across resume. 0 otherwise. + */ +static u64 read_last_pte(struct i915_address_space *vm) +{ + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm); + gen8_pte_t __iomem *ptep; + + if (!suspend_retains_ptes(vm)) + return 0; + + GEM_BUG_ON(GRAPHICS_VER(vm->i915) < 8); + ptep = (typeof(ptep))ggtt->gsm + (ggtt_total_entries(ggtt) - 1); + return readq(ptep); +} + /** * i915_ggtt_suspend_vm - Suspend the memory mappings for a GGTT or DPT VM * @vm: The VM to suspend the mappings for @@ -156,7 +180,10 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm) i915_gem_object_unlock(obj); } - vm->clear_range(vm, 0, vm->total); + if (!suspend_retains_ptes(vm)) + vm->clear_range(vm, 0, vm->total); + else + i915_vm_to_ggtt(vm)->probed_pte = read_last_pte(vm); vm->skip_pte_rewrite = save_skip_rewrite; @@ -299,6 +326,8 @@ static int init_ggtt(struct i915_ggtt *ggtt) struct drm_mm_node *entry; int ret; + ggtt->pte_lost = true; + /* * GuC requires all resources that we're sharing with it to be placed in * non-WOPCM memory. If GuC is not present or not in use we still need a @@ -675,11 +704,20 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm) { struct i915_vma *vma; bool write_domain_objs = false; + bool retained_ptes; drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt); - /* First fill our portion of the GTT with scratch pages */ - vm->clear_range(vm, 0, vm->total); + /* + * First fill our portion of the GTT with scratch pages if + * they were not retained across suspend. + */ + retained_ptes = suspend_retains_ptes(vm) && + !i915_vm_to_ggtt(vm)->pte_lost && + !GEM_WARN_ON(i915_vm_to_ggtt(vm)->probed_pte != read_last_pte(vm)); + + if (!retained_ptes) + vm->clear_range(vm, 0, vm->total); /* clflush objects bound into the GGTT and rebind them. */ list_for_each_entry(vma, &vm->bound_list, vm_link) { @@ -688,9 +726,10 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm) atomic_read(&vma->flags) & I915_VMA_BIND_MASK; GEM_BUG_ON(!was_bound); - vma->ops->bind_vma(vm, NULL, vma->resource, - obj ? obj->cache_level : 0, - was_bound); + if (!retained_ptes) + vma->ops->bind_vma(vm, NULL, vma->resource, + obj ? obj->cache_level : 0, + was_bound); if (obj) { /* only used during resume => exclusive access */ write_domain_objs |= fetch_and_zero(&obj->write_domain); obj->read_domains |= I915_GEM_DOMAIN_GTT; @@ -718,3 +757,8 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt) intel_ggtt_restore_fences(ggtt); } + +void i915_ggtt_mark_pte_lost(struct drm_i915_private *i915, bool val) +{ + to_gt(i915)->ggtt->pte_lost = val; +} diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h index a40d928b3888..128b31476938 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.h +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h @@ -345,6 +345,19 @@ struct i915_ggtt { bool do_idle_maps; + /** + * @pte_lost: Are ptes lost on resume? + * + * Whether the system was recently restored from hibernate and + * thus may have lost pte content. + */ + bool pte_lost; + + /** + * @probed_pte: Probed pte value on suspend. Re-checked on resume. + */ + u64 probed_pte; + int mtrr; /** Bit 6 swizzling required for X tiling */ @@ -581,6 +594,17 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm); void i915_ggtt_suspend(struct i915_ggtt *gtt); void i915_ggtt_resume(struct i915_ggtt *ggtt); +/** + * i915_ggtt_mark_pte_lost - Mark ggtt ptes as lost or clear such a marking + * @i915 The device private. + * @val whether the ptes should be marked as lost. + * + * In some cases pte content is retained across suspend, but typically lost + * across hibernate. Typically they should be marked as lost on + * hibernation restore and such marking cleared on suspend. + */ +void i915_ggtt_mark_pte_lost(struct drm_i915_private *i915, bool val); + void fill_page_dma(struct drm_i915_gem_object *p, const u64 val, unsigned int count); diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index d26dcca7e654..0e224761d0ed 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -100,6 +100,9 @@ #include "intel_region_ttm.h" #include "vlv_suspend.h" +/* Intel Rapid Start Technology ACPI device name */ +static const char irst_name[] = "INT3392"; + static const struct drm_driver i915_drm_driver; static int i915_get_bridge_dev(struct drm_i915_private *dev_priv) @@ -1441,6 +1444,8 @@ static int i915_pm_suspend(struct device *kdev) return -ENODEV; } + i915_ggtt_mark_pte_lost(i915, false); + if (i915->drm.switch_power_state == DRM_SWITCH_POWER_OFF) return 0; @@ -1493,6 +1498,14 @@ static int i915_pm_resume(struct device *kdev) if (i915->drm.switch_power_state == DRM_SWITCH_POWER_OFF) return 0; + /* + * If IRST is enabled, or if we can't detect whether it's enabled, + * then we must assume we lost the GGTT page table entries, since + * they are not retained if IRST decided to enter S4. + */ + if (!IS_ENABLED(CONFIG_ACPI) || acpi_dev_present(irst_name, NULL, -1)) + i915_ggtt_mark_pte_lost(i915, true); + return i915_drm_resume(&i915->drm); } @@ -1552,6 +1565,9 @@ static int i915_pm_restore_early(struct device *kdev) static int i915_pm_restore(struct device *kdev) { + struct drm_i915_private *i915 = kdev_to_i915(kdev); + + i915_ggtt_mark_pte_lost(i915, true); return i915_pm_resume(kdev); } -- 2.34.3

1 0

[PATCH v2 0/2] i915: Extract, polish, and document multicast handling
by Matt Roper 17 Jun '22

17 Jun '22

Multicast/replicated (MCR) registers on Intel hardware are a purely GT-specific concept. Rather than leaving MCR register handling spread across several places throughout the driver (intel_uncore.c, intel_gt.c, etc.) with confusing combinations of handler functions living in different namespaces, let's consolidate it all into a single place (intel_gt_mcr.c) and provide a more consistent and clearly-documented interface for the rest of the driver to access such registers: * intel_gt_mcr_read -- unicast read from specific instance * intel_gt_mcr_read_any[_fw] -- unicast read from any non-terminated instance * intel_gt_mcr_unicast_write -- unicast write to specific instance * intel_gt_mcr_multicast_write[_fw] -- multicast write to all instances v2: - Reference the new kerneldoc from i915.rst. (Jani) - Tweak the wording of the documentation for a couple functions to clarify the difference between "_fw" and non-"_fw" forms. Matt Roper (2): drm/i915/gt: Move multicast register handling to a dedicated file drm/i915/gt: Cleanup interface for MCR operations Documentation/gpu/i915.rst | 12 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 3 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 36 +- drivers/gpu/drm/i915/gt/intel_gt.c | 297 +----------- drivers/gpu/drm/i915/gt/intel_gt.h | 15 - drivers/gpu/drm/i915/gt/intel_gt_debugfs.c | 3 +- drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 497 ++++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_gt_mcr.h | 34 ++ drivers/gpu/drm/i915/gt/intel_region_lmem.c | 5 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 3 +- drivers/gpu/drm/i915/i915_drv.h | 2 - drivers/gpu/drm/i915/intel_uncore.c | 112 ----- drivers/gpu/drm/i915/intel_uncore.h | 8 - 15 files changed, 577 insertions(+), 460 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_mcr.c create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_mcr.h -- 2.35.3

2 4

[PATCH 1/2] drm/mediatek: Config orientation property if panel provides it
by Hsin-Yi Wang 17 Jun '22

17 Jun '22

Panel orientation property should be set before drm_dev_register(). Mediatek drm driver calls drm_dev_register() in .bind(). However, most panels sets orientation property relatively late, mostly in .get_modes() callback, since this is when they are able to get the connector and binds the orientation property to it, though the value should be known when the panel is probed. Let the drm driver check if the remote end point is a panel and if it contains the orientation property. If it does, set it before drm_dev_register() is called. Signed-off-by: Hsin-Yi Wang <hsinyi(a)chromium.org> --- This patch is to solve the same problem as [1] [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20220530081910.39… --- drivers/gpu/drm/mediatek/mtk_dsi.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index d9f10a33e6fa..091107f97ccc 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -198,6 +198,7 @@ struct mtk_dsi { unsigned long mode_flags; enum mipi_dsi_pixel_format format; + enum drm_panel_orientation orientation; unsigned int lanes; struct videomode vm; struct mtk_phy_timing phy_timing; @@ -822,6 +823,10 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi) ret = PTR_ERR(dsi->connector); goto err_cleanup_encoder; } + + if (dsi->orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) + drm_connector_set_panel_orientation(dsi->connector, dsi->orientation); + drm_connector_attach_encoder(dsi->connector, &dsi->encoder); return 0; @@ -836,6 +841,14 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) int ret; struct drm_device *drm = data; struct mtk_dsi *dsi = dev_get_drvdata(dev); + struct drm_panel *panel; + + /* Read panel orientation if existed */ + dsi->orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; + ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, NULL); + if (!ret && panel && panel->dev) { + of_drm_get_panel_orientation(panel->dev->of_node, &dsi->orientation); + } ret = mtk_dsi_encoder_init(drm, dsi); if (ret) -- 2.36.1.124.g0e6072fb45-goog

2 4

[RFC PATCH 0/5] Add option to disable implicit sync for userspace submits.
by Bas Nieuwenhuizen 17 Jun '22

17 Jun '22

This adds a context option to use DMA_RESV_USAGE_BOOKKEEP for userspace submissions, based on Christians TTM work. Disabling implicit sync is something we've wanted in radv for a while for resolving some corner cases. A more immediate thing that would be solved here is avoiding a bunch of implicit sync on GPU map/unmap operations as well, which helps with stutter around sparse maps/unmaps. I have experimental userspace in radv, but it isn't 100% ready yet. There are still issues with some games that I'm looking at, but in the meantime I'm looking for early feedback on the idea. Besides the debugging an open question is whether it is worth adding the option to wait on additional explicit syncobj in the VM map/unmap operations. My current radv code waits on the wait syncobj in userspace on a thread before doing the operation which results in some corner cases because we can't provide binary syncobj at submission time (impacting the usual sync file exports). However adding these fences adds the risk of head of line blocking because all VM operations get executed on the same ring, so all later operations get blocked by waiting on the fences as well, which can cause head of line blocking. I'm looking to get more implementation experience with different games to see if we need this, but if we need it it would be a somewhat separate addition to the UAPI. Bas Nieuwenhuizen (5): drm/ttm: Refactor num_shared into usage. drm/amdgpu: Add separate mode for syncing DMA_RESV_USAGE_BOOKKEEP. drm/amdgpu: Allow explicit sync for VM ops. drm/amdgpu: Refactor amdgpu_vm_get_pd_bo. drm/amdgpu: Add option to disable implicit sync for a context. .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 21 ++++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 19 ++++++----- drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 4 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 32 +++++++++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++--- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 11 +++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 4 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/qxl/qxl_release.c | 2 +- drivers/gpu/drm/radeon/radeon_cs.c | 5 +-- drivers/gpu/drm/radeon/radeon_gem.c | 2 +- drivers/gpu/drm/radeon/radeon_vm.c | 4 +-- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 5 ++- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 10 +++--- drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 2 +- include/drm/ttm/ttm_execbuf_util.h | 3 +- include/uapi/drm/amdgpu_drm.h | 3 ++ 28 files changed, 112 insertions(+), 63 deletions(-) -- 2.36.1

3 40

[PATCH 1/7] dt-bindings: display: mediatek: dpi: add power-domains property
by Fabien Parent 17 Jun '22

17 Jun '22

DPI is part of the display / multimedia block in MediaTek SoCs, and always have a power-domain (at least in the upstream device-trees). Add the power-domains property to the binding documentation. Signed-off-by: Fabien Parent <fparent(a)baylibre.com> --- .../devicetree/bindings/display/mediatek/mediatek,dpi.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml index 77ee1b923991..caf4c88708f4 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml @@ -57,6 +57,9 @@ properties: Output port node. This port should be connected to the input port of an attached HDMI or LVDS encoder chip. + power-domains: + maxItems: 1 + required: - compatible - reg @@ -64,6 +67,7 @@ required: - clocks - clock-names - port + - power-domains additionalProperties: false @@ -71,11 +75,13 @@ examples: - | #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/clock/mt8173-clk.h> + #include <dt-bindings/power/mt8183-power.h> dpi0: dpi@1401d000 { compatible = "mediatek,mt8173-dpi"; reg = <0x1401d000 0x1000>; interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>; + power-domains = <&spm MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DPI_PIXEL>, <&mmsys CLK_MM_DPI_ENGINE>, <&apmixedsys CLK_APMIXED_TVDPLL>; -- 2.36.1

5 17

[PATCH] drm/i915: Improve on suspend / resume time with VT-d enabled
by Thomas Hellström 17 Jun '22

17 Jun '22

When DMAR / VT-d is enabled, the display engine uses overfetching, presumably to deal with the increased latency. To avoid display engine errors and DMAR faults, as a workaround the GGTT is populated with scatch PTEs when VT-d is enabled. However starting with gen10, Write-combined writing of scratch PTES is no longer possible and as a result, populating the full GGTT with scratch PTEs like on resume becomes very slow as uncached access is needed. Therefore, on integrated GPUs utilize the fact that the PTEs are stored in stolen memory which retain content across S3 suspend. Don't clear the PTEs on suspend and resume. This improves on resume time with around 100 ms. While 100+ms might appear like a short time it's 10% to 20% of total resume time and important in some applications. One notable exception is Intel Rapid Start Technology which may cause stolen memory to be lost across what the OS percieves as S3 suspend. If IRST is enabled or if we can't detect whether IRST is enabled, retain the old workaround, clearing and re-instating PTEs. As an additional measure, if we detect that the last ggtt pte was lost during suspend, print a warning and re-populate the GGTT ptes On discrete GPUs, the display engine scans out from LMEM which isn't subject to DMAR, and presumably the workaround is therefore not needed, but that needs to be verified and disabling the workaround for dGPU, if possible, will be deferred to a follow-up patch. v2: - Rely on retained ptes to also speed up suspend and resume re-binding. - Re-build GGTT ptes if Intel rst is enabled. v3: - Re-build GGTT ptes also if we can't detect whether Intel rst is enabled, and if the guard page PTE and end of GGTT was lost. Signed-off-by: Thomas Hellström <thomas.hellstrom(a)linux.intel.com> --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 56 +++++++++++++++++++++++++--- drivers/gpu/drm/i915/gt/intel_gtt.h | 20 ++++++++++ drivers/gpu/drm/i915/i915_driver.c | 16 ++++++++ 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 04191fe2ee34..98441b1c1b75 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -23,6 +23,13 @@ #include "intel_gtt.h" #include "gen8_ppgtt.h" +static inline bool suspend_retains_ptes(struct i915_address_space *vm) +{ + return GRAPHICS_VER(vm->i915) >= 8 && + !HAS_LMEM(vm->i915) && + vm->is_ggtt; +} + static void i915_ggtt_color_adjust(const struct drm_mm_node *node, unsigned long color, u64 *start, @@ -116,6 +123,23 @@ static bool needs_idle_maps(struct drm_i915_private *i915) return false; } +/* + * Return the value of the last GGTT pte cast to an u64, if + * the system is supposed to retain ptes across resume. 0 otherwise. + */ +static u64 read_last_pte(struct i915_address_space *vm) +{ + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm); + gen8_pte_t __iomem *ptep; + + if (!suspend_retains_ptes(vm)) + return 0; + + GEM_BUG_ON(GRAPHICS_VER(vm->i915) < 8); + ptep = (typeof(ptep))ggtt->gsm + (ggtt_total_entries(ggtt) - 1); + return readq(ptep); +} + /** * i915_ggtt_suspend_vm - Suspend the memory mappings for a GGTT or DPT VM * @vm: The VM to suspend the mappings for @@ -179,7 +203,10 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm) i915_gem_object_unlock(obj); } - vm->clear_range(vm, 0, vm->total); + if (!suspend_retains_ptes(vm)) + vm->clear_range(vm, 0, vm->total); + else + i915_vm_to_ggtt(vm)->probed_pte = read_last_pte(vm); vm->skip_pte_rewrite = save_skip_rewrite; @@ -578,6 +605,8 @@ static int init_ggtt(struct i915_ggtt *ggtt) struct drm_mm_node *entry; int ret; + ggtt->pte_lost = true; + /* * GuC requires all resources that we're sharing with it to be placed in * non-WOPCM memory. If GuC is not present or not in use we still need a @@ -1309,11 +1338,20 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm) { struct i915_vma *vma; bool write_domain_objs = false; + bool retained_ptes; drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt); - /* First fill our portion of the GTT with scratch pages */ - vm->clear_range(vm, 0, vm->total); + /* + * First fill our portion of the GTT with scratch pages if + * they were not retained across suspend. + */ + retained_ptes = suspend_retains_ptes(vm) && + !i915_vm_to_ggtt(vm)->pte_lost && + !GEM_WARN_ON(i915_vm_to_ggtt(vm)->probed_pte != read_last_pte(vm)); + + if (!retained_ptes) + vm->clear_range(vm, 0, vm->total); /* clflush objects bound into the GGTT and rebind them. */ list_for_each_entry(vma, &vm->bound_list, vm_link) { @@ -1322,9 +1360,10 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm) atomic_read(&vma->flags) & I915_VMA_BIND_MASK; GEM_BUG_ON(!was_bound); - vma->ops->bind_vma(vm, NULL, vma->resource, - obj ? obj->cache_level : 0, - was_bound); + if (!retained_ptes) + vma->ops->bind_vma(vm, NULL, vma->resource, + obj ? obj->cache_level : 0, + was_bound); if (obj) { /* only used during resume => exclusive access */ write_domain_objs |= fetch_and_zero(&obj->write_domain); obj->read_domains |= I915_GEM_DOMAIN_GTT; @@ -1352,3 +1391,8 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt) intel_ggtt_restore_fences(ggtt); } + +void i915_ggtt_mark_pte_lost(struct drm_i915_private *i915, bool val) +{ + to_gt(i915)->ggtt->pte_lost = val; +} diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h index 4529b5e9f6e6..7561672c4f17 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.h +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h @@ -345,6 +345,15 @@ struct i915_ggtt { bool do_idle_maps; + /** + * Whether the system was recently restored from hibernate and + * thus may have lost pte content. + */ + bool pte_lost; + + /** Probed pte value on suspend. Re-checked on resume. */ + u64 probed_pte; + int mtrr; /** Bit 6 swizzling required for X tiling */ @@ -571,6 +580,17 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm); void i915_ggtt_suspend(struct i915_ggtt *gtt); void i915_ggtt_resume(struct i915_ggtt *ggtt); +/** + * i915_ggtt_mark_pte_lost - Mark ggtt ptes as lost or clear such a marking + * @i915 The device private. + * @val whether the ptes should be marked as lost. + * + * In some cases pte content is retained across suspend, but typically lost + * across hibernate. Typically they should be marked as lost on + * hibernation restore and such marking cleared on suspend. + */ +void i915_ggtt_mark_pte_lost(struct drm_i915_private *i915, bool val); + void fill_page_dma(struct drm_i915_gem_object *p, const u64 val, unsigned int count); diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 64e6f76861f9..f50256e4c2d2 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -98,6 +98,9 @@ #include "intel_region_ttm.h" #include "vlv_suspend.h" +/* Intel Rapid Start Technology ACPI device name */ +static const char irst_name[] = "INT3392"; + static const struct drm_driver i915_drm_driver; static int i915_get_bridge_dev(struct drm_i915_private *dev_priv) @@ -1425,6 +1428,8 @@ static int i915_pm_suspend(struct device *kdev) return -ENODEV; } + i915_ggtt_mark_pte_lost(i915, false); + if (i915->drm.switch_power_state == DRM_SWITCH_POWER_OFF) return 0; @@ -1477,6 +1482,14 @@ static int i915_pm_resume(struct device *kdev) if (i915->drm.switch_power_state == DRM_SWITCH_POWER_OFF) return 0; + /* + * If IRST is enabled, or if we can't detect whether it's enabled, + * then we must assume we lost the GGTT page table entries, since + * they are not retained if IRST decided to enter S4. + */ + if (!IS_ENABLED(CONFIG_ACPI) || acpi_dev_present(irst_name, NULL, -1)) + i915_ggtt_mark_pte_lost(i915, true); + return i915_drm_resume(&i915->drm); } @@ -1536,6 +1549,9 @@ static int i915_pm_restore_early(struct device *kdev) static int i915_pm_restore(struct device *kdev) { + struct drm_i915_private *i915 = kdev_to_i915(kdev); + + i915_ggtt_mark_pte_lost(i915, true); return i915_pm_resume(kdev); } -- 2.34.1

3 2

[PATCH v2 3/3] drm/panel: sony-acx565akm: Use backlight helpers
by Stephen Kitt 17 Jun '22

17 Jun '22

Instead of retrieving the backlight brightness in struct backlight_properties manually, and then checking whether the backlight should be on at all, use backlight_get_brightness() which does all this and insulates this from future changes. Instead of manually checking the power state in struct backlight_properties, use backlight_is_blank(). While we're at it, drop .fb_blank from the initialisation function; it is deprecated, and this helps make progress towards enabling its removal. This change makes no functional difference since FB_BLANK_UNBLANK is the default value. Signed-off-by: Stephen Kitt <steve(a)sk2.org> Cc: Thierry Reding <thierry.reding(a)gmail.com> Cc: Sam Ravnborg <sam(a)ravnborg.org> Cc: David Airlie <airlied(a)linux.ie> Cc: Daniel Vetter <daniel(a)ffwll.ch> Cc: dri-devel(a)lists.freedesktop.org --- Changes since v1: removed the last remaining .fb_blank reference --- drivers/gpu/drm/panel/panel-sony-acx565akm.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-sony-acx565akm.c b/drivers/gpu/drm/panel/panel-sony-acx565akm.c index 0d7541a33f87..3d6a286056a0 100644 --- a/drivers/gpu/drm/panel/panel-sony-acx565akm.c +++ b/drivers/gpu/drm/panel/panel-sony-acx565akm.c @@ -298,13 +298,7 @@ static void acx565akm_set_brightness(struct acx565akm_panel *lcd, int level) static int acx565akm_bl_update_status_locked(struct backlight_device *dev) { struct acx565akm_panel *lcd = dev_get_drvdata(&dev->dev); - int level; - - if (dev->props.fb_blank == FB_BLANK_UNBLANK && - dev->props.power == FB_BLANK_UNBLANK) - level = dev->props.brightness; - else - level = 0; + int level = backlight_get_brightness(dev); acx565akm_set_brightness(lcd, level); @@ -330,8 +324,7 @@ static int acx565akm_bl_get_intensity(struct backlight_device *dev) mutex_lock(&lcd->mutex); - if (dev->props.fb_blank == FB_BLANK_UNBLANK && - dev->props.power == FB_BLANK_UNBLANK) + if (!backlight_is_blank(dev)) intensity = acx565akm_get_actual_brightness(lcd); else intensity = 0; @@ -349,7 +342,6 @@ static const struct backlight_ops acx565akm_bl_ops = { static int acx565akm_backlight_init(struct acx565akm_panel *lcd) { struct backlight_properties props = { - .fb_blank = FB_BLANK_UNBLANK, .power = FB_BLANK_UNBLANK, .type = BACKLIGHT_RAW, }; -- 2.30.2

2 1

[PATCH v2 2/3] drm/panel: panel-dsi-cm: Use backlight helpers
by Stephen Kitt 17 Jun '22

17 Jun '22

Instead of retrieving the backlight brightness in struct backlight_properties manually, and then checking whether the backlight should be on at all, use backlight_get_brightness() which does all this and insulates this from future changes. Instead of setting the power state by manually updating fields in struct backlight_properties, use backlight_enable() and backlight_disable(). These also call backlight_update_status() so the separate call is no longer needed. Signed-off-by: Stephen Kitt <steve(a)sk2.org> Cc: Thierry Reding <thierry.reding(a)gmail.com> Cc: Sam Ravnborg <sam(a)ravnborg.org> Cc: David Airlie <airlied(a)linux.ie> Cc: Daniel Vetter <daniel(a)ffwll.ch> Cc: dri-devel(a)lists.freedesktop.org --- Changes since v1: removed unnecessary parentheses --- drivers/gpu/drm/panel/panel-dsi-cm.c | 29 ++++++---------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c index b58cb064975f..b0213a518f9d 100644 --- a/drivers/gpu/drm/panel/panel-dsi-cm.c +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c @@ -85,17 +85,10 @@ static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable) else return; - if (enable) { - backlight->props.fb_blank = FB_BLANK_UNBLANK; - backlight->props.state = ~(BL_CORE_FBBLANK | BL_CORE_SUSPENDED); - backlight->props.power = FB_BLANK_UNBLANK; - } else { - backlight->props.fb_blank = FB_BLANK_NORMAL; - backlight->props.power = FB_BLANK_POWERDOWN; - backlight->props.state |= BL_CORE_FBBLANK | BL_CORE_SUSPENDED; - } - - backlight_update_status(backlight); + if (enable) + backlight_enable(backlight); + else + backlight_disable(backlight); } static void hw_guard_start(struct panel_drv_data *ddata, int guard_msec) @@ -196,13 +189,7 @@ static int dsicm_bl_update_status(struct backlight_device *dev) { struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); int r = 0; - int level; - - if (dev->props.fb_blank == FB_BLANK_UNBLANK && - dev->props.power == FB_BLANK_UNBLANK) - level = dev->props.brightness; - else - level = 0; + int level = backlight_get_brightness(dev); dev_dbg(&ddata->dsi->dev, "update brightness to %d\n", level); @@ -219,11 +206,7 @@ static int dsicm_bl_update_status(struct backlight_device *dev) static int dsicm_bl_get_intensity(struct backlight_device *dev) { - if (dev->props.fb_blank == FB_BLANK_UNBLANK && - dev->props.power == FB_BLANK_UNBLANK) - return dev->props.brightness; - - return 0; + return backlight_get_brightness(dev); } static const struct backlight_ops dsicm_bl_ops = { -- 2.30.2

2 1

[PATCH v11 0/1] drm/mediatek: Add MT8195 DisplayPort PHY driver
by Bo-Chen Chen 17 Jun '22

17 Jun '22

This series display port phy drivers and separated from [1] which is original from Guillaume. The display port driver is [2]. Changes for v11: 1. Fix register definition. [1]:https://lore.kernel.org/all/20220523104758.29531-1-granquet@baylibre.com/ [2]:https://lore.kernel.org/all/20220610105522.13449-1-rex-bc.chen@mediatek.… Markus Schneider-Pargmann (1): phy: phy-mtk-dp: Add driver for DP phy MAINTAINERS | 1 + drivers/phy/mediatek/Kconfig | 8 ++ drivers/phy/mediatek/Makefile | 1 + drivers/phy/mediatek/phy-mtk-dp.c | 202 ++++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+) create mode 100644 drivers/phy/mediatek/phy-mtk-dp.c -- 2.18.0

4 5

← Newer
1
...
23
24
25
26
27
28
29
...
70
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel June 2022