[PATCH v3 5/7] drm/qxl: use drm_gem_object_funcs callbacks
Switch qxl to use drm_gem_object_funcs callbacks
instead of drm_driver callbacks.
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_drv.c| 8
drivers/gpu/drm/qxl/qxl_object.c | 12
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 2b726a51a302..996d428fa7e6 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -258,16 +258,8 @@ static struct drm_driver qxl_driver = {
#endif
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
- .gem_prime_pin = qxl_gem_prime_pin,
- .gem_prime_unpin = qxl_gem_prime_unpin,
- .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
.gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
- .gem_prime_vmap = qxl_gem_prime_vmap,
- .gem_prime_vunmap = qxl_gem_prime_vunmap,
.gem_prime_mmap = qxl_gem_prime_mmap,
- .gem_free_object_unlocked = qxl_gem_object_free,
- .gem_open_object = qxl_gem_object_open,
- .gem_close_object = qxl_gem_object_close,
.fops = _fops,
.ioctls = qxl_ioctls,
.irq_handler = qxl_irq_handler,
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 548dfe6f3b26..29aab7b14513 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -77,6 +77,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32
domain, bool pinned)
}
}
+static const struct drm_gem_object_funcs qxl_object_funcs = {
+ .free = qxl_gem_object_free,
+ .open = qxl_gem_object_open,
+ .close = qxl_gem_object_close,
+ .pin = qxl_gem_prime_pin,
+ .unpin = qxl_gem_prime_unpin,
+ .get_sg_table = qxl_gem_prime_get_sg_table,
+ .vmap = qxl_gem_prime_vmap,
+ .vunmap = qxl_gem_prime_vunmap,
+};
+
int qxl_bo_create(struct qxl_device *qdev,
unsigned long size, bool kernel, bool pinned, u32 domain,
struct qxl_surface *surf,
@@ -100,6 +111,7 @@ int qxl_bo_create(struct qxl_device *qdev,
kfree(bo);
return r;
}
+ bo->tbo.base.funcs = _object_funcs;
bo->type = domain;
bo->pin_count = pinned ? 1 : 0;
bo->surface_id = 0;
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 7/7] drm/vram: fix Kconfig
select isn't recursive, so we can't turn on DRM_TTM + DRM_TTM_HELPER in config DRM_VRAM_HELPER, we have to select them on the vram users instead. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/Kconfig | 2 -- drivers/gpu/drm/ast/Kconfig | 2 ++ drivers/gpu/drm/bochs/Kconfig | 2 ++ drivers/gpu/drm/hisilicon/hibmc/Kconfig | 3 ++- drivers/gpu/drm/mgag200/Kconfig | 2 ++ drivers/gpu/drm/vboxvideo/Kconfig | 2 ++ 6 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 1be8ad30d8fe..cd11a3bde19c 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -168,8 +168,6 @@ config DRM_TTM config DRM_VRAM_HELPER tristate depends on DRM - select DRM_TTM - select DRM_TTM_HELPER help Helpers for VRAM memory management diff --git a/drivers/gpu/drm/ast/Kconfig b/drivers/gpu/drm/ast/Kconfig index 829620d5326c..fbcf2f45cef5 100644 --- a/drivers/gpu/drm/ast/Kconfig +++ b/drivers/gpu/drm/ast/Kconfig @@ -4,6 +4,8 @@ config DRM_AST depends on DRM && PCI && MMU select DRM_KMS_HELPER select DRM_VRAM_HELPER + select DRM_TTM + select DRM_TTM_HELPER help Say yes for experimental AST GPU driver. Do not enable this driver without having a working -modesetting, diff --git a/drivers/gpu/drm/bochs/Kconfig b/drivers/gpu/drm/bochs/Kconfig index 32b043abb668..7bcdf294fed8 100644 --- a/drivers/gpu/drm/bochs/Kconfig +++ b/drivers/gpu/drm/bochs/Kconfig @@ -4,6 +4,8 @@ config DRM_BOCHS depends on DRM && PCI && MMU select DRM_KMS_HELPER select DRM_VRAM_HELPER + select DRM_TTM + select DRM_TTM_HELPER help Choose this option for qemu. If M is selected the module will be called bochs-drm. diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig b/drivers/gpu/drm/hisilicon/hibmc/Kconfig index f20eedf0073a..8ad9a5b12e40 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig @@ -4,7 +4,8 @@ config DRM_HISI_HIBMC depends on DRM && PCI && MMU select DRM_KMS_HELPER select DRM_VRAM_HELPER - + select DRM_TTM + select DRM_TTM_HELPER help Choose this option if you have a Hisilicon Hibmc soc chipset. If M is selected the module will be called hibmc-drm. diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 76fee0fbdcae..aed11f4f4c55 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -4,6 +4,8 @@ config DRM_MGAG200 depends on DRM && PCI && MMU select DRM_KMS_HELPER select DRM_VRAM_HELPER + select DRM_TTM + select DRM_TTM_HELPER help This is a KMS driver for the MGA G200 server chips, it does not support the original MGA G200 or any of the desktop diff --git a/drivers/gpu/drm/vboxvideo/Kconfig b/drivers/gpu/drm/vboxvideo/Kconfig index 56ba510f21a2..45fe135d6e43 100644 --- a/drivers/gpu/drm/vboxvideo/Kconfig +++ b/drivers/gpu/drm/vboxvideo/Kconfig @@ -4,6 +4,8 @@ config DRM_VBOXVIDEO depends on DRM && X86 && PCI select DRM_KMS_HELPER select DRM_VRAM_HELPER + select DRM_TTM + select DRM_TTM_HELPER select GENERIC_ALLOCATOR help This is a KMS driver for the virtual Graphics Card used in -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/7] drm/ttm: add drm gem ttm helpers, starting with drm_gem_ttm_print_info()
Now with ttm_buffer_object being a subclass of drm_gem_object we can
easily lookup ttm_buffer_object for a given drm_gem_object, which in
turn allows to create common helper functions.
This patch starts off with a drm_gem_ttm_print_info() helper function
which adds some ttm specific lines to the debug output.
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
Reviewed-by: Daniel Vetter
---
include/drm/drm_gem_ttm_helper.h | 19 ++
drivers/gpu/drm/drm_gem_ttm_helper.c | 56
Documentation/gpu/drm-mm.rst | 12 ++
drivers/gpu/drm/Kconfig | 7
drivers/gpu/drm/Makefile | 3 ++
5 files changed, 97 insertions(+)
create mode 100644 include/drm/drm_gem_ttm_helper.h
create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c
diff --git a/include/drm/drm_gem_ttm_helper.h b/include/drm/drm_gem_ttm_helper.h
new file mode 100644
index ..6268f89c5a48
--- /dev/null
+++ b/include/drm/drm_gem_ttm_helper.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef DRM_GEM_TTM_HELPER_H
+#define DRM_GEM_TTM_HELPER_H
+
+#include
+
+#include
+#include
+#include
+#include
+
+#define drm_gem_ttm_of_gem(gem_obj) \
+ container_of(gem_obj, struct ttm_buffer_object, base)
+
+void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int indent,
+ const struct drm_gem_object *gem);
+
+#endif
diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
b/drivers/gpu/drm/drm_gem_ttm_helper.c
new file mode 100644
index ..9a4bafcf20df
--- /dev/null
+++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include
+
+#include
+
+/**
+ * DOC: overview
+ *
+ * This library provides helper functions for gem objects backed by
+ * ttm.
+ */
+
+/**
+ * drm_gem_ttm_print_info() - Print _buffer_object info for debugfs
+ * @p: DRM printer
+ * @indent: Tab indentation level
+ * @gem: GEM object
+ *
+ * This function can be used as _gem_object_funcs.print_info
+ * callback.
+ */
+void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int indent,
+ const struct drm_gem_object *gem)
+{
+ static const char const *plname[] = {
+ [ TTM_PL_SYSTEM ] = "system",
+ [ TTM_PL_TT ] = "tt",
+ [ TTM_PL_VRAM ] = "vram",
+ [ TTM_PL_PRIV ] = "priv",
+
+ [ 16 ]= "cached",
+ [ 17 ]= "uncached",
+ [ 18 ]= "wc",
+ [ 19 ]= "contig",
+
+ [ 21 ]= "pinned", /* NO_EVICT */
+ [ 22 ]= "topdown",
+ };
+ const struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
+
+ drm_printf_indent(p, indent, "placement=");
+ drm_print_bits(p, bo->mem.placement, plname, 0, ARRAY_SIZE(plname));
+ drm_printf(p, "\n");
+
+ if (bo->mem.bus.is_iomem) {
+ drm_printf_indent(p, indent, "bus.base=%lx\n",
+ (unsigned long)bo->mem.bus.base);
+ drm_printf_indent(p, indent, "bus.offset=%lx\n",
+ (unsigned long)bo->mem.bus.offset);
+ }
+}
+EXPORT_SYMBOL(drm_gem_ttm_print_info);
+
+MODULE_DESCRIPTION("DRM gem ttm helpers");
+MODULE_LICENSE("GPL");
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index b664f054c259..a70a1d9f30ec 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -412,6 +412,18 @@ VRAM MM Helper Functions Reference
.. kernel-doc:: drivers/gpu/drm/drm_vram_mm_helper.c
:export:
+GEM TTM Helper Functions Reference
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_gem_ttm_helper.c
+ :doc: overview
+
+.. kernel-doc:: include/drm/drm_gem_ttm_helper.h
+ :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_gem_ttm_helper.c
+ :export:
+
VMA Offset Manager
==
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e6f40fb54c9a..f7b25519f95c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -172,6 +172,13 @@ config DRM_VRAM_HELPER
help
Helpers for VRAM memory management
+config DRM_TTM_HELPER
+ tristate
+ depends on DRM
+ select DRM_TTM
+ help
+ Helpers for ttm-based gem objects
+
config DRM_GEM_CMA_HELPER
bool
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 10f8329a8b71..545c61d6528b 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -37,6 +37,9 @@ drm_vram_helper-y := drm_gem_vram_helper.o \
drm_vram_mm_helper.o
obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
+drm_ttm_helper-y := drm_gem_ttm_helper.o
+obj-$(CONFIG_DRM_TTM_HELPER) += drm_ttm_helper.o
+
drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
[PATCH v3 0/7] drm: add some ttm/vram info to debugfs
Gerd Hoffmann (7): drm: add drm_print_bits drm/ttm: add drm gem ttm helpers, starting with drm_gem_ttm_print_info() drm/vram: use drm_gem_ttm_print_info drm/vram: add vram-mm debugfs file drm/qxl: use drm_gem_object_funcs callbacks drm/qxl: use drm_gem_ttm_print_info drm/vram: fix Kconfig drivers/gpu/drm/qxl/qxl_drv.h | 1 + include/drm/drm_gem_ttm_helper.h| 19 + include/drm/drm_gem_vram_helper.h | 1 + include/drm/drm_print.h | 3 ++ include/drm/drm_vram_mm_helper.h| 1 + drivers/gpu/drm/drm_gem_ttm_helper.c| 56 + drivers/gpu/drm/drm_gem_vram_helper.c | 4 +- drivers/gpu/drm/drm_print.c | 33 +++ drivers/gpu/drm/drm_vram_mm_helper.c| 44 +++ drivers/gpu/drm/qxl/qxl_drv.c | 8 drivers/gpu/drm/qxl/qxl_object.c| 13 ++ Documentation/gpu/drm-mm.rst| 12 ++ drivers/gpu/drm/Kconfig | 8 +++- drivers/gpu/drm/Makefile| 3 ++ drivers/gpu/drm/ast/Kconfig | 2 + drivers/gpu/drm/bochs/Kconfig | 2 + drivers/gpu/drm/hisilicon/hibmc/Kconfig | 3 +- drivers/gpu/drm/mgag200/Kconfig | 2 + drivers/gpu/drm/vboxvideo/Kconfig | 2 + 19 files changed, 206 insertions(+), 11 deletions(-) create mode 100644 include/drm/drm_gem_ttm_helper.h create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/7] drm/vram: use drm_gem_ttm_print_info
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_gem_vram_helper.c | 4 +++-
drivers/gpu/drm/Kconfig | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index fd751078bae1..71552f757b4a 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#include
#include
#include
#include
@@ -633,5 +634,6 @@ static const struct drm_gem_object_funcs
drm_gem_vram_object_funcs = {
.pin= drm_gem_vram_object_pin,
.unpin = drm_gem_vram_object_unpin,
.vmap = drm_gem_vram_object_vmap,
- .vunmap = drm_gem_vram_object_vunmap
+ .vunmap = drm_gem_vram_object_vunmap,
+ .print_info = drm_gem_ttm_print_info,
};
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f7b25519f95c..1be8ad30d8fe 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -169,6 +169,7 @@ config DRM_VRAM_HELPER
tristate
depends on DRM
select DRM_TTM
+ select DRM_TTM_HELPER
help
Helpers for VRAM memory management
--
2.18.1
[PATCH v3 6/7] drm/qxl: use drm_gem_ttm_print_info
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_drv.h| 1 +
drivers/gpu/drm/qxl/qxl_object.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 9e034c5fa87d..d4051409ce64 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 29aab7b14513..c013c516f561 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -86,6 +86,7 @@ static const struct drm_gem_object_funcs qxl_object_funcs = {
.get_sg_table = qxl_gem_prime_get_sg_table,
.vmap = qxl_gem_prime_vmap,
.vunmap = qxl_gem_prime_vunmap,
+ .print_info = drm_gem_ttm_print_info,
};
int qxl_bo_create(struct qxl_device *qdev,
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/7] drm/vram: add vram-mm debugfs file
Wire up drm_mm_print() for vram helpers, using a new
debugfs file, so one can see how vram is used:
# cat /sys/kernel/debug/dri/0/vram-mm
0x-0x0300: 768: used
0x0300-0x0600: 768: used
0x0600-0x0900: 768: used
0x0900-0x0c00: 768: used
0x0c00-0x4000: 13312: free
total: 16384, used 3072 free 13312
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
include/drm/drm_gem_vram_helper.h| 1 +
include/drm/drm_vram_mm_helper.h | 1 +
drivers/gpu/drm/drm_vram_mm_helper.c | 44
3 files changed, 46 insertions(+)
diff --git a/include/drm/drm_gem_vram_helper.h
b/include/drm/drm_gem_vram_helper.h
index ac217d768456..91c6934138f2 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -122,6 +122,7 @@ int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file
*file,
* drm_driver with default functions.
*/
#define DRM_GEM_VRAM_DRIVER \
+ .debugfs_init = drm_vram_mm_debugfs_init, \
.dumb_create = drm_gem_vram_driver_dumb_create, \
.dumb_map_offset = drm_gem_vram_driver_dumb_mmap_offset, \
.gem_prime_mmap = drm_gem_prime_mmap
diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h
index 2aacfb1ccfae..9e0ac9aaac7d 100644
--- a/include/drm/drm_vram_mm_helper.h
+++ b/include/drm/drm_vram_mm_helper.h
@@ -60,6 +60,7 @@ static inline struct drm_vram_mm *drm_vram_mm_of_bdev(
return container_of(bdev, struct drm_vram_mm, bdev);
}
+int drm_vram_mm_debugfs_init(struct drm_minor *minor);
int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev,
uint64_t vram_base, size_t vram_size,
const struct drm_vram_mm_funcs *funcs);
diff --git a/drivers/gpu/drm/drm_vram_mm_helper.c
b/drivers/gpu/drm/drm_vram_mm_helper.c
index c911781d6728..3c2f0cbcad5b 100644
--- a/drivers/gpu/drm/drm_vram_mm_helper.c
+++ b/drivers/gpu/drm/drm_vram_mm_helper.c
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#include
#include
#include
+#include
#include
#include
@@ -148,6 +150,48 @@ static struct ttm_bo_driver bo_driver = {
* struct drm_vram_mm
*/
+#if defined(CONFIG_DEBUG_FS)
+static int drm_vram_mm_debugfs(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_vram_mm *vmm = node->minor->dev->vram_mm;
+ struct drm_mm *mm = vmm->bdev.man[TTM_PL_VRAM].priv;
+ struct ttm_bo_global *glob = vmm->bdev.glob;
+ struct drm_printer p = drm_seq_file_printer(m);
+
+ spin_lock(>lru_lock);
+ drm_mm_print(mm, );
+ spin_unlock(>lru_lock);
+ return 0;
+}
+
+static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
+ { "vram-mm", drm_vram_mm_debugfs, 0, NULL },
+};
+#endif
+
+/**
+ * drm_vram_mm_debugfs_init() - Register VRAM MM debugfs file.
+ *
+ * @minor: drm minor device.
+ *
+ * Returns:
+ * 0 on success, or
+ * a negative error code otherwise.
+ */
+int drm_vram_mm_debugfs_init(struct drm_minor *minor)
+{
+ int ret = 0;
+
+#if defined(CONFIG_DEBUG_FS)
+ ret = drm_debugfs_create_files(drm_vram_mm_debugfs_list,
+ ARRAY_SIZE(drm_vram_mm_debugfs_list),
+ minor->debugfs_root, minor);
+#endif
+ return ret;
+}
+EXPORT_SYMBOL(drm_vram_mm_debugfs_init);
+
/**
* drm_vram_mm_init() - Initialize an instance of VRAM MM.
* @vmm: the VRAM MM instance to initialize
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/7] drm: add drm_print_bits
New helper to print named bits of some value (think flags fields).
Signed-off-by: Gerd Hoffmann
---
include/drm/drm_print.h | 3 +++
drivers/gpu/drm/drm_print.c | 33 +
2 files changed, 36 insertions(+)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 112165d3195d..12d4916254b4 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -89,6 +89,9 @@ __printf(2, 3)
void drm_printf(struct drm_printer *p, const char *f, ...);
void drm_puts(struct drm_printer *p, const char *str);
void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32
*regset);
+void drm_print_bits(struct drm_printer *p,
+ unsigned long value, const char *bits[],
+ unsigned int from, unsigned int to);
__printf(2, 0)
/**
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ad302d71..dfa27367ebb8 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -185,6 +185,39 @@ void drm_printf(struct drm_printer *p, const char *f, ...)
}
EXPORT_SYMBOL(drm_printf);
+/**
+ * drm_print_bits - print bits to a _printer stream
+ *
+ * Print bits (in flag fields for example) in human readable form.
+ * The first name in the @bits array is for the bit indexed by @from.
+ *
+ * @p: the _printer
+ * @value: field value.
+ * @bits: Array with bit names.
+ * @from: start of bit range to print (inclusive).
+ * @to: end of bit range to print (exclusive).
+ */
+void drm_print_bits(struct drm_printer *p,
+ unsigned long value, const char *bits[],
+ unsigned int from, unsigned int to)
+{
+ bool first = true;
+ unsigned int i;
+
+ for (i = from; i < to; i++) {
+ if (!(value & (1 << i)))
+ continue;
+ if (WARN_ON_ONCE(!bits[i-from]))
+ continue;
+ drm_printf(p, "%s%s", first ? "" : ",",
+ bits[i-from]);
+ first = false;
+ }
+ if (first)
+ drm_printf(p, "(none)");
+}
+EXPORT_SYMBOL(drm_print_bits);
+
void drm_dev_printk(const struct device *dev, const char *level,
const char *format, ...)
{
--
2.18.1
[Bug 111551] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma1 timeout
https://bugs.freedesktop.org/show_bug.cgi?id=111551 --- Comment #2 from yanhua <78666...@qq.com> --- Created attachment 145260 --> https://bugs.freedesktop.org/attachment.cgi?id=145260=edit The previous dmesg.txt has messages been overwriten. from the dmesg-full.txt can see more information -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111555] [amdgpu/Navi] [powerplay] Failed to send message errors
https://bugs.freedesktop.org/show_bug.cgi?id=111555 Bug ID: 111555 Summary: [amdgpu/Navi] [powerplay] Failed to send message errors Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: not set Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: shtetl...@gmail.com I get periodic errors like this in dmesg, which coincides with intermittent system stalls: [Wed Sep 4 00:43:43 2019] amdgpu: [powerplay] Failed to send message 0x12, response 0xfffb param 0x6 [Wed Sep 4 00:43:43 2019] amdgpu: [powerplay] Failed to export SMU metrics table! [Wed Sep 4 00:44:53 2019] amdgpu: [powerplay] Failed to send message 0xe, response 0xfffb, param 0x80 [Wed Sep 4 00:44:53 2019] amdgpu: [powerplay] Failed to send message 0xf, response 0xfffb, param 0xa9 [Wed Sep 4 00:45:30 2019] amdgpu: [powerplay] Failed to send message 0x12, response 0xfffb, param 0x6 [Wed Sep 4 00:45:35 2019] amdgpu: [powerplay] Failed to send message 0x12, response 0xfffb param 0x6 [Wed Sep 4 00:45:35 2019] amdgpu: [powerplay] Failed to export SMU metrics table! I'm running kernel 5.3-rc7 GPU: Sapphire Pulse RX 5700XT (Navi 10) with firmware from https://people.freedesktop.org/~agd5f/radeon_ucode/navi10/ Distro: Debian testing / KDE. I noticed, that it starts happening often when I'm using ksysguard, which queries lm-sensors for amdgpu temperature and fan speed. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204181] NULL pointer dereference regression in amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=204181 --- Comment #46 from Tom Seewald (tseew...@gmail.com) --- Will these patches[1] be back ported to 5.2/5.3 or will we need to wait until this hopefully lands in 5.4? [1] https://patchwork.freedesktop.org/series/64505/ -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 02/11] drm/msm: remove unlikely() from WARN_ON() conditions
On Thu 29 Aug 09:50 PDT 2019, Denis Efremov wrote:
> "unlikely(WARN_ON(x))" is excessive. WARN_ON() already uses unlikely()
> internally.
>
> Signed-off-by: Denis Efremov
> Cc: Rob Clark
> Cc: Sean Paul
> Cc: Joe Perches
> Cc: Andrew Morton
> Cc: linux-arm-...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Bjorn Andersson
> ---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c | 4 ++--
> drivers/gpu/drm/msm/disp/mdp_format.c| 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c
> index 4804cf40de14..030279d7b64b 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c
> @@ -253,7 +253,7 @@ int mdp5_ctl_set_cursor(struct mdp5_ctl *ctl, struct
> mdp5_pipeline *pipeline,
> u32 blend_cfg;
> struct mdp5_hw_mixer *mixer = pipeline->mixer;
>
> - if (unlikely(WARN_ON(!mixer))) {
> + if (WARN_ON(!mixer)) {
> DRM_DEV_ERROR(ctl_mgr->dev->dev, "CTL %d cannot find LM",
> ctl->id);
> return -EINVAL;
> @@ -695,7 +695,7 @@ struct mdp5_ctl_manager *mdp5_ctlm_init(struct drm_device
> *dev,
> goto fail;
> }
>
> - if (unlikely(WARN_ON(ctl_cfg->count > MAX_CTL))) {
> + if (WARN_ON(ctl_cfg->count > MAX_CTL)) {
> DRM_DEV_ERROR(dev->dev, "Increase static pool size to at least
> %d\n",
> ctl_cfg->count);
> ret = -ENOSPC;
> diff --git a/drivers/gpu/drm/msm/disp/mdp_format.c
> b/drivers/gpu/drm/msm/disp/mdp_format.c
> index 8afb0f9c04bb..5495d8b3f5b9 100644
> --- a/drivers/gpu/drm/msm/disp/mdp_format.c
> +++ b/drivers/gpu/drm/msm/disp/mdp_format.c
> @@ -174,7 +174,7 @@ const struct msm_format *mdp_get_format(struct msm_kms
> *kms, uint32_t format,
>
> struct csc_cfg *mdp_get_default_csc_cfg(enum csc_type type)
> {
> - if (unlikely(WARN_ON(type >= CSC_MAX)))
> + if (WARN_ON(type >= CSC_MAX))
> return NULL;
>
> return _convert[type];
> --
> 2.21.0
>
Re: [PATCH v5, 05/32] dt-bindings: mediatek: add mutex description for mt8183 display
Hi, Yongqiang: On Thu, 2019-08-29 at 22:50 +0800, yongqiang@mediatek.com wrote: > From: Yongqiang Niu > > This patch add mutex description for mt8183 display Applied to mediatek-drm-next-5.5 [1], thanks. [1] https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-next-5.5 Regards, CK > > Signed-off-by: Yongqiang Niu > --- > Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > index afd3c90..c7e2eb8 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > @@ -52,6 +52,7 @@ Required properties (all function blocks): >For most function blocks this is just a single clock input. Only the DSI > and >DPI controller nodes have multiple clock inputs. These are documented in >mediatek,dsi.txt and mediatek,dpi.txt, respectively. > + An exception is that the mt8183 mutex is always free running with no > clocks property. > > Required properties (DMA function blocks): > - compatible: Should be one of
Re: [PATCH v5, 04/32] dt-bindings: mediatek: add dither description for mt8183 display
Hi, Yongqiang: On Thu, 2019-08-29 at 22:50 +0800, yongqiang@mediatek.com wrote: > From: Yongqiang Niu > > Update device tree binding documention for the display subsystem for > Mediatek MT8183 SOCs Applied to mediatek-drm-next-5.5 [1], thanks. [1] https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-next-5.5 Regards, CK > > Signed-off-by: Yongqiang Niu > Reviewed-by: Rob Herring > --- > Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > index cf5fb08..afd3c90 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > @@ -33,6 +33,7 @@ Required properties (all function blocks): > "mediatek,-disp-wdma" - write DMA > "mediatek,-disp-ccorr"- color correction > "mediatek,-disp-color"- color processor > + "mediatek,-disp-dither" - dither > "mediatek,-disp-aal" - adaptive ambient light > controller > "mediatek,-disp-gamma"- gamma correction > "mediatek,-disp-merge"- merge streams from two RDMA > sources
Re: [PATCH v5, 03/32] dt-bindings: mediatek: add ccorr description for mt8183 display
Hi, Yongqiang: On Thu, 2019-08-29 at 22:50 +0800, yongqiang@mediatek.com wrote: > From: Yongqiang Niu > > Update device tree binding documention for the display subsystem for > Mediatek MT8183 SOCs Applied to mediatek-drm-next-5.5 [1], thanks. [1] https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-next-5.5 Regards, CK > > Signed-off-by: Yongqiang Niu > Reviewed-by: Rob Herring > --- > Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > index 8c4700f..cf5fb08 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > @@ -31,6 +31,7 @@ Required properties (all function blocks): > "mediatek,-disp-ovl-2l" - overlay (2 layers, blending, > csc) > "mediatek,-disp-rdma" - read DMA / line buffer > "mediatek,-disp-wdma" - write DMA > + "mediatek,-disp-ccorr"- color correction > "mediatek,-disp-color"- color processor > "mediatek,-disp-aal" - adaptive ambient light > controller > "mediatek,-disp-gamma"- gamma correction
Re: [PATCH v5, 02/32] dt-bindings: mediatek: add ovl_2l description for mt8183 display
Hi, Yongqiang: On Thu, 2019-08-29 at 22:50 +0800, yongqiang@mediatek.com wrote: > From: Yongqiang Niu > > Update device tree binding documention for the display subsystem for > Mediatek MT8183 SOCs Applied to mediatek-drm-next-5.5 [1], thanks. [1] https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-next-5.5 Regards, CK > > Signed-off-by: Yongqiang Niu > Reviewed-by: Rob Herring > --- > .../bindings/display/mediatek/mediatek,disp.txt| 27 > +++--- > 1 file changed, 14 insertions(+), 13 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > index 464b92f..8c4700f 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > @@ -27,19 +27,20 @@ > Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt. > > Required properties (all function blocks): > - compatible: "mediatek,-disp-", one of > - "mediatek,-disp-ovl" - overlay (4 layers, blending, csc) > - "mediatek,-disp-rdma" - read DMA / line buffer > - "mediatek,-disp-wdma" - write DMA > - "mediatek,-disp-color" - color processor > - "mediatek,-disp-aal" - adaptive ambient light controller > - "mediatek,-disp-gamma" - gamma correction > - "mediatek,-disp-merge" - merge streams from two RDMA sources > - "mediatek,-disp-split" - split stream to two encoders > - "mediatek,-disp-ufoe" - data compression engine > - "mediatek,-dsi"- DSI controller, see mediatek,dsi.txt > - "mediatek,-dpi"- DPI controller, see mediatek,dpi.txt > - "mediatek,-disp-mutex" - display mutex > - "mediatek,-disp-od"- overdrive > + "mediatek,-disp-ovl" - overlay (4 layers, blending, > csc) > + "mediatek,-disp-ovl-2l" - overlay (2 layers, blending, > csc) > + "mediatek,-disp-rdma" - read DMA / line buffer > + "mediatek,-disp-wdma" - write DMA > + "mediatek,-disp-color"- color processor > + "mediatek,-disp-aal" - adaptive ambient light > controller > + "mediatek,-disp-gamma"- gamma correction > + "mediatek,-disp-merge"- merge streams from two RDMA > sources > + "mediatek,-disp-split"- split stream to two encoders > + "mediatek,-disp-ufoe" - data compression engine > + "mediatek,-dsi" - DSI controller, see > mediatek,dsi.txt > + "mediatek,-dpi" - DPI controller, see > mediatek,dpi.txt > + "mediatek,-disp-mutex"- display mutex > + "mediatek,-disp-od" - overdrive >the supported chips are mt2701, mt2712 and mt8173. > - reg: Physical base address and length of the function block register space > - interrupts: The interrupt signal from the function block (required, except > for ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111482] Sapphire Pulse RX 5700 XT power consumption
https://bugs.freedesktop.org/show_bug.cgi?id=111482 --- Comment #8 from Andrew Sheldon --- I just did some more tests, and in my case, it wasn't strictly the refresh rate, but the timings being too aggressive (which I needed to do to lower the pixel clock enough due to driver limits, which are quite conservative). This wasn't as much of a problem with Vega, since the idle power usage was about the same (12-15W), but it is with Navi. I will also add that during my tests, I found it was possible to leave the system in a state where I couldn't leave the high memclock/power usage situation after a while, even when switching to 60hz, requiring a reboot. So that might be what is happening on your system, Robert. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Adreno crash on i.MX53 running 5.3-rc6
Hi Rob, On Tue, Sep 3, 2019 at 9:12 PM Rob Clark wrote: > In the mean time, it is a bit ugly, but I guess something like this should > work: Yes, this works on a i.MX53 board, thanks: Tested-by: Fabio Estevam Is this something you could submit for 5.3? Thanks ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108718] Raven Ridge: ring sdma0 timeout on heavy CSS website with Firefox WebRender
https://bugs.freedesktop.org/show_bug.cgi?id=108718 --- Comment #3 from Matias N. Goldberg --- I'm on: OpenGL renderer string: AMD RAVEN (DRM 3.30.0, 5.1.15-050115-generic, LLVM 8.0.1) OpenGL core profile version string: 4.5 (Core Profile) Mesa 19.2.0-devel (git-8305766 2019-07-11 bionic-oibaf-ppa) And I can also still repro this issue. Firefox isn't the only way to trigger it, Chromium can trigger it too. Is it too old already? How recent the kernel and mesa must be? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Adreno crash on i.MX53 running 5.3-rc6
On Tue, Sep 3, 2019 at 12:31 PM Fabio Estevam wrote:
>
> Hi Jonathan,
>
> On Tue, Sep 3, 2019 at 4:25 PM Jonathan Marek wrote:
> >
> > Hi,
> >
> > I tried this and it works with patches 4+5 from Rob's series and
> > changing gpummu to use sg_phys(sg) instead of sg->dma_address
> > (dma_address isn't set now that dma_map_sg isn't used).
>
> Thanks for testing it. I haven't had a chance to test it yet.
>
> Rob,
>
> I assume your series is targeted to 5.4, correct?
maybe, although Christoph Hellwig didn't seem like a big fan of
exposing cache ops, and would rather add a new allocation API for
uncached pages.. so I'm not entirely sure what the way forward will
be.
In the mean time, it is a bit ugly, but I guess something like this should work:
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 7263f4373f07..5a6a79fbc9d6 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -52,7 +52,7 @@ static void sync_for_device(struct msm_gem_object *msm_obj)
{
struct device *dev = msm_obj->base.dev->dev;
-if (get_dma_ops(dev)) {
+if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) {
dma_sync_sg_for_device(dev, msm_obj->sgt->sgl,
msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
} else {
@@ -65,7 +65,7 @@ static void sync_for_cpu(struct msm_gem_object *msm_obj)
{
struct device *dev = msm_obj->base.dev->dev;
-if (get_dma_ops(dev)) {
+if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) {
dma_sync_sg_for_cpu(dev, msm_obj->sgt->sgl,
msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
} else {
BR,
-R
> If this is the case, what we should do about the i.MX5 regression on 5.3?
>
> Would a revert of the two commits be acceptable in 5.3 in order to
> avoid the regression?
>
> Please advise.
>
> Thanks
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #23 from Matt Turner --- Can you make an apitrace of the application that demonstrates the problem? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
> On Sep 3, 2019, at 3:15 PM, Thomas Hellström (VMware) > wrote: > >> On 9/4/19 12:08 AM, Thomas Hellström (VMware) wrote: >>> On 9/3/19 11:46 PM, Andy Lutomirski wrote: >>> On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) >>> wrote: On 9/3/19 10:51 PM, Dave Hansen wrote: >> On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote: >> So the question here should really be, can we determine already at mmap >> time whether backing memory will be unencrypted and adjust the *real* >> vma->vm_page_prot under the mmap_sem? >> >> Possibly, but that requires populating the buffer with memory at mmap >> time rather than at first fault time. > I'm not connecting the dots. > > vma->vm_page_prot is used to create a VMA's PTEs regardless of if they > are created at mmap() or fault time. If we establish a good > vma->vm_page_prot, can't we just use it forever for demand faults? With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory.. > Or, are you concerned that if an attempt is made to demand-fault page > that's incompatible with vma->vm_page_prot that we have to SEGV? > >> And it still requires knowledge whether the device DMA is always >> unencrypted (or if SEV is active). > I may be getting mixed up on MKTME (the Intel memory encryption) and > SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, > anonymous? Or just anonymous? SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory). >>> I'm still lost. You have some fancy VMA where the backing pages >>> change behind the application's back. This isn't particularly novel >>> -- plain old anonymous memory and plain old mapped files do this too. >>> Can't you all the insert_pfn APIs and call it a day? What's so >>> special that you need all this magic? ISTM you should be able to >>> allocate memory that's addressable by the device (dma_alloc_coherent() >>> or whatever) and then map it into user memory just like you'd map any >>> other page. >>> >>> I feel like I'm missing something here. >> >> Yes, so in this case we use dma_alloc_coherent(). >> >> With SEV, that gives us unencrypted pages. (Pages whose linear kernel map is >> marked unencrypted). With SME that (typcially) gives us encrypted pages. In >> both these cases, vm_get_page_prot() returns >> an encrypted page protection, which lands in vma->vm_page_prot. >> >> In the SEV case, we therefore need to modify the page protection to >> unencrypted. Hence we need to know whether we're running under SEV and >> therefore need to modify the protection. If not, the user-space PTE would >> incorrectly have the encryption flag set. >> I’m still confused. You got unencrypted pages with an unencrypted PFN. Why do you need to fiddle? You have a PFN, and you’re inserting it with vmf_insert_pfn(). This should just work, no? There doesn’t seem to be any real funny business in dma_mmap_attrs() or dma_common_mmap(). But, reading this, I have more questions: Can’t you get rid of cvma by using vmf_insert_pfn_prot()? Would it make sense to add a vmf_insert_dma_page() to directly do exactly what you’re trying to do? And a broader question just because I’m still confused: why isn’t the encryption bit in the PFN? The whole SEV/SME system seems like it’s trying a bit to hard to be fully invisible to the kernel. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
Thomas, this series has garnered a nak and a whole pile of thoroughly confused reviewers. Could you take another stab at this along with a more ample changelog explaining the context of the problem? I suspect that's a better place to start than having us all piece together the disparate parts of the thread. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On 9/4/19 12:08 AM, Thomas Hellström (VMware) wrote: On 9/3/19 11:46 PM, Andy Lutomirski wrote: On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) wrote: On 9/3/19 10:51 PM, Dave Hansen wrote: On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote: So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem? Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time. I'm not connecting the dots. vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults? With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory.. Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV? And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active). I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous? SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory). I'm still lost. You have some fancy VMA where the backing pages change behind the application's back. This isn't particularly novel -- plain old anonymous memory and plain old mapped files do this too. Can't you all the insert_pfn APIs and call it a day? What's so special that you need all this magic? ISTM you should be able to allocate memory that's addressable by the device (dma_alloc_coherent() or whatever) and then map it into user memory just like you'd map any other page. I feel like I'm missing something here. Yes, so in this case we use dma_alloc_coherent(). With SEV, that gives us unencrypted pages. (Pages whose linear kernel map is marked unencrypted). With SME that (typcially) gives us encrypted pages. In both these cases, vm_get_page_prot() returns an encrypted page protection, which lands in vma->vm_page_prot. In the SEV case, we therefore need to modify the page protection to unencrypted. Hence we need to know whether we're running under SEV and therefore need to modify the protection. If not, the user-space PTE would incorrectly have the encryption flag set. /Thomas And, of course, had we not been "fancy", we could have used dma_mmap_coherent(), which in theory should set up the correct user-space page protection. But now we're moving stuff around so we can't. /Thomas
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On 9/3/19 11:46 PM, Andy Lutomirski wrote: On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) wrote: On 9/3/19 10:51 PM, Dave Hansen wrote: On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote: So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem? Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time. I'm not connecting the dots. vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults? With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory.. Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV? And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active). I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous? SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory). I'm still lost. You have some fancy VMA where the backing pages change behind the application's back. This isn't particularly novel -- plain old anonymous memory and plain old mapped files do this too. Can't you all the insert_pfn APIs and call it a day? What's so special that you need all this magic? ISTM you should be able to allocate memory that's addressable by the device (dma_alloc_coherent() or whatever) and then map it into user memory just like you'd map any other page. I feel like I'm missing something here. Yes, so in this case we use dma_alloc_coherent(). With SEV, that gives us unencrypted pages. (Pages whose linear kernel map is marked unencrypted). With SME that (typcially) gives us encrypted pages. In both these cases, vm_get_page_prot() returns an encrypted page protection, which lands in vma->vm_page_prot. In the SEV case, we therefore need to modify the page protection to unencrypted. Hence we need to know whether we're running under SEV and therefore need to modify the protection. If not, the user-space PTE would incorrectly have the encryption flag set. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm/dp_mst: Combine redundant cases in drm_dp_encode_sideband_req()
Noticed this while working on adding a drm_dp_decode_sideband_req().
DP_POWER_DOWN_PHY/DP_POWER_UP_PHY both use the same struct as
DP_ENUM_PATH_RESOURCES, so we can just combine their cases.
Changes since v2:
* Fix commit message
Signed-off-by: Lyude Paul
Reviewed-by: Dave Airlie
Cc: Daniel Vetter
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
---
drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 6f7f449ca12b..1c862749cb63 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -271,6 +271,8 @@ static void drm_dp_encode_sideband_req(struct
drm_dp_sideband_msg_req_body *req,
switch (req->req_type) {
case DP_ENUM_PATH_RESOURCES:
+ case DP_POWER_DOWN_PHY:
+ case DP_POWER_UP_PHY:
buf[idx] = (req->u.port_num.port_number & 0xf) << 4;
idx++;
break;
@@ -358,12 +360,6 @@ static void drm_dp_encode_sideband_req(struct
drm_dp_sideband_msg_req_body *req,
memcpy([idx], req->u.i2c_write.bytes,
req->u.i2c_write.num_bytes);
idx += req->u.i2c_write.num_bytes;
break;
-
- case DP_POWER_DOWN_PHY:
- case DP_POWER_UP_PHY:
- buf[idx] = (req->u.port_num.port_number & 0xf) << 4;
- idx++;
- break;
}
raw->cur_len = idx;
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) wrote: > > On 9/3/19 10:51 PM, Dave Hansen wrote: > > On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote: > >> So the question here should really be, can we determine already at mmap > >> time whether backing memory will be unencrypted and adjust the *real* > >> vma->vm_page_prot under the mmap_sem? > >> > >> Possibly, but that requires populating the buffer with memory at mmap > >> time rather than at first fault time. > > I'm not connecting the dots. > > > > vma->vm_page_prot is used to create a VMA's PTEs regardless of if they > > are created at mmap() or fault time. If we establish a good > > vma->vm_page_prot, can't we just use it forever for demand faults? > > With SEV I think that we could possibly establish the encryption flags > at vma creation time. But thinking of it, it would actually break with > SME where buffer content can be moved between encrypted system memory > and unencrypted graphics card PCI memory behind user-space's back. That > would imply killing all user-space encrypted PTEs and at fault time set > up new ones pointing to unencrypted PCI memory.. > > > > > Or, are you concerned that if an attempt is made to demand-fault page > > that's incompatible with vma->vm_page_prot that we have to SEGV? > > > >> And it still requires knowledge whether the device DMA is always > >> unencrypted (or if SEV is active). > > I may be getting mixed up on MKTME (the Intel memory encryption) and > > SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, > > anonymous? Or just anonymous? > > SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a > SWIOTLB backed by unencrypted memory, and it also flips coherent DMA > memory to unencrypted (which is a very slow operation and patch 4 deals > with caching such memory). > I'm still lost. You have some fancy VMA where the backing pages change behind the application's back. This isn't particularly novel -- plain old anonymous memory and plain old mapped files do this too. Can't you all the insert_pfn APIs and call it a day? What's so special that you need all this magic? ISTM you should be able to allocate memory that's addressable by the device (dma_alloc_coherent() or whatever) and then map it into user memory just like you'd map any other page. I feel like I'm missing something here.
Re: [PATCH v2 15/27] drm/dp_mst: Cleanup drm_dp_send_link_address() a bit
On Wed, 4 Sep 2019 at 06:48, Lyude Paul wrote: > > Declare local pointer to the drm_dp_link_address_ack_reply struct > instead of constantly dereferencing it through the union in > txmsg->reply. Then, invert the order of conditionals so we don't have to > do the bulk of the work inside them, and can wrap lines even less. Then > finally, rearrange variable declarations a bit. > > Cc: Juston Li > Cc: Imre Deak > Cc: Ville Syrjälä > Cc: Harry Wentland > Cc: Daniel Vetter > Signed-off-by: Lyude Paul Reviewed-by: Dave Airlie ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] x86/mm: Export force_dma_unencrypted
On Tue, Sep 3, 2019 at 1:46 PM Thomas Hellström (VMware) wrote: > > On 9/3/19 6:22 PM, Christoph Hellwig wrote: > > On Tue, Sep 03, 2019 at 04:32:45PM +0200, Thomas Hellström (VMware) wrote: > >> Is this a layer violation concern, that is, would you be ok with a similar > >> helper for TTM, or is it that you want to force the graphics drivers into > >> adhering strictly to the DMA api, even when it from an engineering > >> perspective makes no sense? > > >From looking at DRM I strongly believe that making DRM use the DMA > > mapping properly makes a lot of sense from the engineering perspective, > > and this series is a good argument for that positions. > > What I mean with "from an engineering perspective" is that drivers would > end up with a non-trivial amount of code supporting purely academic > cases: Setups where software rendering would be faster than gpu > accelerated, and setups on platforms where the driver would never run > anyway because the device would never be supported on that platform... > > > If DRM was using > > the DMA properl we would not need this series to start with, all the > > SEV handling is hidden behind the DMA API. While we had occasional > > bugs in that support fixing it meant that it covered all drivers > > properly using that API. > > That is not really true. The dma API can't handle faulting of coherent > pages which is what this series is really all about supporting also with > SEV active. To handle the case where we move graphics buffers or send > them to swap space while user-space have them mapped. > > To do that and still be fully dma-api compliant we would ideally need, > for example, an exported dma_pgprot(). (dma_pgprot() by the way is still > suffering from one of the bugs that you mention above). > > Still, I need a way forward and my questions weren't really answered by > this. > > I read this patch, I read force_dma_encrypted(), I read the changelog again, and I haven't the faintest clue what TTM could possibly be doing with force_dma_encrypted(). You're saying that TTM needs to transparently change mappings to relocate objects in memory between system memory and device memory. Great, I don't see the problem. Is the issue that you need to allocate system memory that is addressable by the GPU and that, if the GPU has insufficient PA bits, you need unencrypted memory? If so, this sounds like an excellent use for the DMA API. Rather than kludging directly knowledge of force_dma_encrypted() into the driver, can't you at least add, if needed, a new helper specifically to allocate memory that can be addressed by the device? Like dma_alloc_coherent()? Or, if for some reason, dma_alloc_coherent() doesn't do what you need or your driver isn't ready to use it, then explain *why* and introduce a new function to solve your problem? Keep in mind that, depending on just how MKTME ends up being supported in Linux, it's entirely possible that it will be *backwards* from what you expect -- high address bits will be needed to ask for *unencrypted* memory. --Andy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/27] drm/dp_mst: Constify guid in drm_dp_get_mst_branch_by_guid()
On Wed, 4 Sep 2019 at 06:48, Lyude Paul wrote:
>
> And it's helper, we'll be using this in just a moment.
>
Reviewed-by: Dave Airlie
> Cc: Juston Li
> Cc: Imre Deak
> Cc: Ville Syrjälä
> Cc: Harry Wentland
> Cc: Daniel Vetter
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 43452872efad..b44d3696c09a 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2060,7 +2060,7 @@ static struct drm_dp_mst_branch
> *drm_dp_get_mst_branch_device(struct drm_dp_mst_
>
> static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
> struct drm_dp_mst_branch *mstb,
> - uint8_t *guid)
> + const uint8_t *guid)
> {
> struct drm_dp_mst_branch *found_mstb;
> struct drm_dp_mst_port *port;
> @@ -2084,7 +2084,7 @@ static struct drm_dp_mst_branch
> *get_mst_branch_device_by_guid_helper(
>
> static struct drm_dp_mst_branch *
> drm_dp_get_mst_branch_device_by_guid(struct drm_dp_mst_topology_mgr *mgr,
> -uint8_t *guid)
> +const uint8_t *guid)
> {
> struct drm_dp_mst_branch *mstb;
> int ret;
> --
> 2.21.0
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/27] drm/dp_mst: Combine redundant cases in drm_dp_encode_sideband_req()
On Wed, 4 Sep 2019 at 06:48, Lyude Paul wrote: > > Noticed this while working on adding a drm_dp_decode_sideband_req(). > DP_POWER_DOWN_PHY/DP_POWER_UP_PHY both use the same struct, so we can > just combine their cases. both use the same struct as enum path resources? Since otherwise the patch doesn't make sense. With that fixed: Reviewed-by: Dave Airlie ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109389] memory leak in `amdgpu_bo_create()`
https://bugs.freedesktop.org/show_bug.cgi?id=109389 Czcibor Bohusz-Dobosz changed: What|Removed |Added Attachment #145256|0 |1 is obsolete|| --- Comment #6 from Czcibor Bohusz-Dobosz --- Created attachment 145257 --> https://bugs.freedesktop.org/attachment.cgi?id=145257=edit Galactic Civilizations III memleak log with DXVK Apologies, looks like I had forgotten to update the methodology in several places of the DXVK memleak log - this one should be much more accurate. The updated methodology had however, to my understanding, showcased something that I had not expected: apparently, the memory allocated by amdgpu_bo_create() does not actually accumulate in a linear fashion, instead, it seems like it is replaced the second time the game is launched. Because of that, there is a chance that more than the 65 megabytes were actually unavailable after the test without DXVK, perhaps a sum of all the amdgpu_bo_create() calls' allocations. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204725] black screen
https://bugzilla.kernel.org/show_bug.cgi?id=204725 --- Comment #51 from Dmitri Seletski (drj...@gmail.com) --- Created attachment 284811 --> https://bugzilla.kernel.org/attachment.cgi?id=284811=edit .config file -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204725] black screen
https://bugzilla.kernel.org/show_bug.cgi?id=204725 --- Comment #50 from Dmitri Seletski (drj...@gmail.com) --- (In reply to Mike Lothian from comment #49) > Did you try it? https://youtu.be/_6CRYa4MzuU Have a look at video please. Console works for sure. Ill add .config that I have now. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204725] black screen
https://bugzilla.kernel.org/show_bug.cgi?id=204725 --- Comment #49 from Mike Lothian (m...@fireburn.co.uk) --- Did you try it? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204725] black screen
https://bugzilla.kernel.org/show_bug.cgi?id=204725 Dmitri Seletski (drj...@gmail.com) changed: What|Removed |Added URL||https://youtu.be/_6CRYa4Mzu ||U -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204725] black screen
https://bugzilla.kernel.org/show_bug.cgi?id=204725 --- Comment #48 from Dmitri Seletski (drj...@gmail.com) --- (In reply to Mike Lothian from comment #47) > It's selected automatically if you set DRM_FBDEV_EMULATION - which is > "Enable legacy fbdev support for your modesetting driver" and unset above > > That should get your console working Mike, just to clarify, console is working until AMDGPU driver is loaded. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On 9/3/19 10:51 PM, Dave Hansen wrote: On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote: So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem? Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time. I'm not connecting the dots. vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults? With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory.. Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV? And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active). I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous? SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory). /Thomas
Re: [PATCH 10/10] drm/msm: add atomic traces
On Thu, Aug 29, 2019 at 09:45:18AM -0700, Rob Clark wrote:
> From: Rob Clark
>
> This was useful for debugging fps drops. I suspect it will be useful
> again.
>
> Signed-off-by: Rob Clark
I'm a simple man, I see tracepoints patches and R-b tracepoints patches :)
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/msm_atomic.c | 24 +++-
> drivers/gpu/drm/msm/msm_atomic_trace.h | 110 +++
> drivers/gpu/drm/msm/msm_atomic_tracepoints.c | 3 +
> drivers/gpu/drm/msm/msm_gpu_trace.h | 2 +-
> 5 files changed, 136 insertions(+), 4 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/msm_atomic_trace.h
> create mode 100644 drivers/gpu/drm/msm/msm_atomic_tracepoints.c
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 7a05cbf2f820..1579cf0d828f 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -75,6 +75,7 @@ msm-y := \
> disp/dpu1/dpu_rm.o \
> disp/dpu1/dpu_vbif.o \
> msm_atomic.o \
> + msm_atomic_tracepoints.o \
> msm_debugfs.o \
> msm_drv.o \
> msm_fb.o \
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c
> b/drivers/gpu/drm/msm/msm_atomic.c
> index 80536538967b..fb247aa1081e 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -6,6 +6,7 @@
>
> #include
>
> +#include "msm_atomic_trace.h"
> #include "msm_drv.h"
> #include "msm_gem.h"
> #include "msm_kms.h"
> @@ -33,11 +34,13 @@ static void msm_atomic_async_commit(struct msm_kms *kms,
> int crtc_idx)
> {
> unsigned crtc_mask = BIT(crtc_idx);
>
> + trace_msm_atomic_async_commit_start(crtc_mask);
> +
> mutex_lock(>commit_lock);
>
> if (!(kms->pending_crtc_mask & crtc_mask)) {
> mutex_unlock(>commit_lock);
> - return;
> + goto out;
> }
>
> kms->pending_crtc_mask &= ~crtc_mask;
> @@ -47,19 +50,24 @@ static void msm_atomic_async_commit(struct msm_kms *kms,
> int crtc_idx)
> /*
>* Flush hardware updates:
>*/
> - DRM_DEBUG_ATOMIC("triggering async commit\n");
> + trace_msm_atomic_flush_commit(crtc_mask);
> kms->funcs->flush_commit(kms, crtc_mask);
> mutex_unlock(>commit_lock);
>
> /*
>* Wait for flush to complete:
>*/
> + trace_msm_atomic_wait_flush_start(crtc_mask);
> kms->funcs->wait_flush(kms, crtc_mask);
> + trace_msm_atomic_wait_flush_finish(crtc_mask);
>
> mutex_lock(>commit_lock);
> kms->funcs->complete_commit(kms, crtc_mask);
> mutex_unlock(>commit_lock);
> kms->funcs->disable_commit(kms);
> +
> +out:
> + trace_msm_atomic_async_commit_finish(crtc_mask);
> }
>
> static enum hrtimer_restart msm_atomic_pending_timer(struct hrtimer *t)
> @@ -144,13 +152,17 @@ void msm_atomic_commit_tail(struct drm_atomic_state
> *state)
> bool async = kms->funcs->vsync_time &&
> can_do_async(state, _crtc);
>
> + trace_msm_atomic_commit_tail_start(async, crtc_mask);
> +
> kms->funcs->enable_commit(kms);
>
> /*
>* Ensure any previous (potentially async) commit has
>* completed:
>*/
> + trace_msm_atomic_wait_flush_start(crtc_mask);
> kms->funcs->wait_flush(kms, crtc_mask);
> + trace_msm_atomic_wait_flush_finish(crtc_mask);
>
> mutex_lock(>commit_lock);
>
> @@ -201,6 +213,8 @@ void msm_atomic_commit_tail(struct drm_atomic_state
> *state)
> drm_atomic_helper_commit_hw_done(state);
> drm_atomic_helper_cleanup_planes(dev, state);
>
> + trace_msm_atomic_commit_tail_finish(async, crtc_mask);
> +
> return;
> }
>
> @@ -213,14 +227,16 @@ void msm_atomic_commit_tail(struct drm_atomic_state
> *state)
> /*
>* Flush hardware updates:
>*/
> - DRM_DEBUG_ATOMIC("triggering commit\n");
> + trace_msm_atomic_flush_commit(crtc_mask);
> kms->funcs->flush_commit(kms, crtc_mask);
> mutex_unlock(>commit_lock);
>
> /*
>* Wait for flush to complete:
>*/
> + trace_msm_atomic_wait_flush_start(crtc_mask);
> kms->funcs->wait_flush(kms, crtc_mask);
> + trace_msm_atomic_wait_flush_finish(crtc_mask);
>
> mutex_lock(>commit_lock);
> kms->funcs->complete_commit(kms, crtc_mask);
> @@ -229,4 +245,6 @@ void msm_atomic_commit_tail(struct drm_atomic_state
> *state)
>
> drm_atomic_helper_commit_hw_done(state);
> drm_atomic_helper_cleanup_planes(dev, state);
> +
> + trace_msm_atomic_commit_tail_finish(async, crtc_mask);
> }
> diff --git a/drivers/gpu/drm/msm/msm_atomic_trace.h
> b/drivers/gpu/drm/msm/msm_atomic_trace.h
> new file mode 100644
> index ..b4ca0ed3b4a3
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/msm_atomic_trace.h
> @@ -0,0 +1,110 @@
> +/* SPDX-License-Identifier:
Re: [PATCH 09/10] drm/msm/dpu: async commit support
On Thu, Aug 29, 2019 at 09:45:17AM -0700, Rob Clark wrote:
> From: Rob Clark
>
> In addition, moving to kms->flush_commit() lets us drop the only user
> of kms->commit().
>
> Signed-off-by: Rob Clark
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 13 --
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 5 +++
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 46 +++--
> drivers/gpu/drm/msm/msm_atomic.c| 5 +--
> drivers/gpu/drm/msm/msm_kms.h | 3 --
> 6 files changed, 34 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 31debd31ab8c..f38a7d27a1c0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -606,7 +606,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> - int ret;
>
> /*
>* If no mixers has been allocated in dpu_crtc_atomic_check(),
> @@ -626,17 +625,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> crtc->state->encoder_mask)
> dpu_encoder_prepare_for_kickoff(encoder);
>
> - /* wait for previous frame_event_done completion */
> - DPU_ATRACE_BEGIN("wait_for_frame_done_event");
> - ret = _dpu_crtc_wait_for_frame_done(crtc);
> - DPU_ATRACE_END("wait_for_frame_done_event");
> - if (ret) {
> - DPU_ERROR("crtc%d wait for frame done failed;frame_pending%d\n",
> - crtc->base.id,
> - atomic_read(_crtc->frame_pending));
> - goto end;
> - }
> -
> if (atomic_inc_return(_crtc->frame_pending) == 1) {
> /* acquire bandwidth and other resources */
> DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
> @@ -650,7 +638,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
> dpu_encoder_kickoff(encoder);
>
> -end:
> reinit_completion(_crtc->frame_done_comp);
> DPU_ATRACE_END("crtc_commit");
> }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ac2d534bf59e..3a69b93d8fb6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1678,8 +1678,7 @@ static u32 _dpu_encoder_calculate_linetime(struct
> dpu_encoder_virt *dpu_enc,
> return line_time;
> }
>
> -static int _dpu_encoder_wakeup_time(struct drm_encoder *drm_enc,
> - ktime_t *wakeup_time)
> +int dpu_encoder_vsync_time(struct drm_encoder *drm_enc, ktime_t *wakeup_time)
> {
> struct drm_display_mode *mode;
> struct dpu_encoder_virt *dpu_enc;
> @@ -1766,7 +1765,7 @@ static void dpu_encoder_vsync_event_work_handler(struct
> kthread_work *work)
> return;
> }
>
> - if (_dpu_encoder_wakeup_time(_enc->base, _time))
> + if (dpu_encoder_vsync_time(_enc->base, _time))
> return;
>
> trace_dpu_enc_vsync_event_work(DRMID(_enc->base), wakeup_time);
> @@ -1840,7 +1839,7 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
> }
>
> if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI &&
> - !_dpu_encoder_wakeup_time(drm_enc, _time)) {
> + !dpu_encoder_vsync_time(drm_enc, _time)) {
> trace_dpu_enc_early_kickoff(DRMID(drm_enc),
> ktime_to_ms(wakeup_time));
> mod_timer(_enc->vsync_event_timer,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index 8465b37adf3b..b4913465e602 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -85,6 +85,11 @@ void dpu_encoder_trigger_kickoff_pending(struct
> drm_encoder *encoder);
> */
> void dpu_encoder_kickoff(struct drm_encoder *encoder);
>
> +/**
> + * dpu_encoder_wakeup_time - get the time of the next vsync
> + */
> +int dpu_encoder_vsync_time(struct drm_encoder *drm_enc, ktime_t
> *wakeup_time);
> +
> /**
> * dpu_encoder_wait_for_event - Waits for encoder events
> * @encoder: encoder pointer
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index d54741f3ad9f..af41af1731c2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -260,6 +260,20 @@ static void dpu_kms_disable_commit(struct msm_kms *kms)
> pm_runtime_put_sync(_kms->pdev->dev);
> }
>
> +static ktime_t
[Bug 109389] memory leak in `amdgpu_bo_create()`
https://bugs.freedesktop.org/show_bug.cgi?id=109389 Czcibor Bohusz-Dobosz changed: What|Removed |Added See Also||https://bugs.freedesktop.or ||g/show_bug.cgi?id=107899 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote: > So the question here should really be, can we determine already at mmap > time whether backing memory will be unencrypted and adjust the *real* > vma->vm_page_prot under the mmap_sem? > > Possibly, but that requires populating the buffer with memory at mmap > time rather than at first fault time. I'm not connecting the dots. vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults? Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV? > And it still requires knowledge whether the device DMA is always > unencrypted (or if SEV is active). I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/10] drm/msm: async commit support
On Thu, Aug 29, 2019 at 09:45:16AM -0700, Rob Clark wrote:
> From: Rob Clark
>
> Now that flush/wait/complete is decoupled from the "synchronous" part of
> atomic commit_tail(), add support to defer flush to a timer that expires
> shortly before vblank for async commits. In this way, multiple atomic
> commits (for example, cursor updates) can be coalesced into a single
> flush at the end of the frame.
>
> v2: don't hold lock over ->wait_flush(), to avoid locking interaction
> that was causing fps drop when combining page flips or non-async
> atomic commits and lots of legacy cursor updates
>
> Signed-off-by: Rob Clark
Some nits, with them addressed,
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/msm_atomic.c | 156 ++-
> drivers/gpu/drm/msm/msm_drv.c| 1 +
> drivers/gpu/drm/msm/msm_drv.h| 4 +
> drivers/gpu/drm/msm/msm_kms.h| 50 ++
> 4 files changed, 210 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c
> b/drivers/gpu/drm/msm/msm_atomic.c
> index 614fb9c5bb58..8f8f74337cb4 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -29,6 +29,95 @@ int msm_atomic_prepare_fb(struct drm_plane *plane,
> return msm_framebuffer_prepare(new_state->fb, kms->aspace);
> }
>
> +static void msm_atomic_async_commit(struct msm_kms *kms, int crtc_idx)
> +{
> + unsigned crtc_mask = BIT(crtc_idx);
> +
> + mutex_lock(>commit_lock);
> +
> + if (!(kms->pending_crtc_mask & crtc_mask)) {
> + mutex_unlock(>commit_lock);
> + return;
> + }
> +
> + kms->pending_crtc_mask &= ~crtc_mask;
> +
> + kms->funcs->enable_commit(kms);
> +
> + /*
> + * Flush hardware updates:
> + */
> + DRM_DEBUG_ATOMIC("triggering async commit\n");
> + kms->funcs->flush_commit(kms, crtc_mask);
> + mutex_unlock(>commit_lock);
> +
> + /*
> + * Wait for flush to complete:
> + */
> + kms->funcs->wait_flush(kms, crtc_mask);
> +
> + mutex_lock(>commit_lock);
> + kms->funcs->complete_commit(kms, crtc_mask);
> + mutex_unlock(>commit_lock);
> + kms->funcs->disable_commit(kms);
> +}
> +
> +static enum hrtimer_restart msm_atomic_pending_timer(struct hrtimer *t)
> +{
> + struct msm_pending_timer *timer = container_of(t,
> + struct msm_pending_timer, timer);
> + struct msm_drm_private *priv = timer->kms->dev->dev_private;
> +
As discussed on IRC, a TODO here explaining that we'd like to remove this
worker eventually but there are mutexes further down that need to be removed.
> + queue_work(priv->wq, >work);
> +
> + return HRTIMER_NORESTART;
> +}
> +
> +static void msm_atomic_pending_work(struct work_struct *work)
> +{
> + struct msm_pending_timer *timer = container_of(work,
> + struct msm_pending_timer, work);
> +
> + msm_atomic_async_commit(timer->kms, timer->crtc_idx);
> +}
> +
> +void msm_atomic_init_pending_timer(struct msm_pending_timer *timer,
> + struct msm_kms *kms, int crtc_idx)
> +{
> + timer->kms = kms;
> + timer->crtc_idx = crtc_idx;
> + hrtimer_init(>timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
> + timer->timer.function = msm_atomic_pending_timer;
> + INIT_WORK(>work, msm_atomic_pending_work);
> +}
> +
> +static bool can_do_async(struct drm_atomic_state *state,
> + struct drm_crtc **async_crtc)
> +{
> + struct drm_connector_state *connector_state;
> + struct drm_connector *connector;
> + struct drm_crtc_state *crtc_state;
> + struct drm_crtc *crtc;
> + int i, num_crtcs = 0;
> +
> + if (!(state->legacy_cursor_update || state->async_update))
> + return false;
> +
> + /* any connector change, means slow path: */
> + for_each_new_connector_in_state(state, connector, connector_state, i)
> + return false;
> +
> + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> + if (drm_atomic_crtc_needs_modeset(crtc_state))
> + return false;
> + if (++num_crtcs > 1)
> + return false;
> + *async_crtc = crtc;
> + }
> +
> + return true;
> +}
> +
> /* Get bitmask of crtcs that will need to be flushed. The bitmask
> * can be used with for_each_crtc_mask() iterator, to iterate
> * effected crtcs without needing to preserve the atomic state.
> @@ -50,9 +139,25 @@ void msm_atomic_commit_tail(struct drm_atomic_state
> *state)
> struct drm_device *dev = state->dev;
> struct msm_drm_private *priv = dev->dev_private;
> struct msm_kms *kms = priv->kms;
> + struct drm_crtc *async_crtc = NULL;
> unsigned crtc_mask = get_crtc_mask(state);
> + bool async = kms->funcs->vsync_time &&
> + can_do_async(state, _crtc);
>
> kms->funcs->enable_commit(kms);
> +
> + /*
> + * Ensure any previous (potentially async)
[PATCH v2 25/27] drm/dp_mst: Add basic topology reprobing when resuming
Finally! For a very long time, our MST helpers have had one very
annoying issue: They don't know how to reprobe the topology state when
coming out of suspend. This means that if a user has a machine connected
to an MST topology and decides to suspend their machine, we lose all
topology changes that happened during that period. That can be a big
problem if the machine was connected to a different topology on the same
port before resuming, as we won't bother reprobing any of the ports and
likely cause the user's monitors not to come back up as expected.
So, we start fixing this by teaching our MST helpers how to reprobe the
link addresses of each connected topology when resuming. As it turns
out, the behavior that we want here is identical to the behavior we want
when initially probing a newly connected MST topology, with a couple of
important differences:
- We need to be more careful about handling the potential races between
events from the MST hub that could change the topology state as we're
performing the link address reprobe
- We need to be more careful about handling unlikely state changes on
ports - such as an input port turning into an output port, something
that would be far more likely to happen in situations like the MST hub
we're connected to being changed while we're suspend
Both of which have been solved by previous commits. That leaves one
requirement:
- We need to prune any MST ports in our in-memory topology state that
were present when suspending, but have not appeared in the post-resume
link address response from their parent branch device
Which we can now handle in this commit by modifying
drm_dp_send_link_address(). We then introduce suspend/resume reprobing
by introducing drm_dp_mst_topology_mgr_invalidate_mstb(), which we call
in drm_dp_mst_topology_mgr_suspend() to traverse the in-memory topology
state to indicate that each mstb needs it's link address resent and PBN
resources reprobed.
On resume, we start back up >work and have it reprobe the topology
in the same way we would on a hotplug, removing any leftover ports that
no longer appear in the topology state.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
drivers/gpu/drm/drm_dp_mst_topology.c | 138 +-
drivers/gpu/drm/i915/display/intel_dp.c | 3 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +-
include/drm/drm_dp_mst_helper.h | 3 +-
5 files changed, 112 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4d3c8bff77da..27ee3e045b86 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -973,7 +973,7 @@ static void s3_handle_mst(struct drm_device *dev, bool
suspend)
if (suspend) {
drm_dp_mst_topology_mgr_suspend(mgr);
} else {
- ret = drm_dp_mst_topology_mgr_resume(mgr);
+ ret = drm_dp_mst_topology_mgr_resume(mgr, true);
if (ret < 0) {
drm_dp_mst_topology_mgr_set_mst(mgr, false);
need_hotplug = true;
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index e407aba1fbd2..2fe24e366925 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2020,6 +2020,14 @@ drm_dp_mst_handle_link_address_port(struct
drm_dp_mst_branch *mstb,
goto fail_unlock;
}
+ /*
+* If this port wasn't just created, then we're reprobing because
+* we're coming out of suspend. In this case, always resend the link
+* address if there's an MSTB on this port
+*/
+ if (!created && port->pdt == DP_PEER_DEVICE_MST_BRANCHING)
+ send_link_addr = true;
+
if (send_link_addr) {
mutex_lock(>lock);
if (port->mstb) {
@@ -2530,7 +2538,8 @@ static void drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
{
struct drm_dp_sideband_msg_tx *txmsg;
struct drm_dp_link_address_ack_reply *reply;
- int i, len, ret;
+ struct drm_dp_mst_port *port, *tmp;
+ int i, len, ret, port_mask = 0;
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
if (!txmsg)
@@ -2560,9 +2569,28 @@ static void drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
drm_dp_check_mstb_guid(mstb, reply->guid);
- for (i = 0; i < reply->nports; i++)
+ for (i = 0; i < reply->nports; i++) {
+ port_mask |= BIT(reply->ports[i].port_number);
drm_dp_mst_handle_link_address_port(mstb, mgr->dev,
[PATCH v2 26/27] drm/dp_mst: Also print unhashed pointers for malloc/topology references
Currently we only print mstb/port pointer addresses in our malloc and
topology refcount functions using the hashed-by-default %p, but
unfortunately if you're trying to debug a use-after-free error caused by
a refcounting error then this really isn't terribly useful. On the other
hand though, everything in the rest of the DP MST helpers uses hashed
pointer values as well and probably isn't useful to convert to unhashed.
So, let's just get the best of both worlds and print both the hashed and
unhashed pointer in our malloc/topology refcount debugging output. This
will hopefully make it a lot easier to figure out which port/mstb is
causing KASAN to get upset.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 34 ---
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 2fe24e366925..5b5c0b3b3c0e 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1327,7 +1327,8 @@ static void
drm_dp_mst_get_mstb_malloc(struct drm_dp_mst_branch *mstb)
{
kref_get(>malloc_kref);
- DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(>malloc_kref));
+ DRM_DEBUG("mstb %p/%px (%d)\n",
+ mstb, mstb, kref_read(>malloc_kref));
}
/**
@@ -1344,7 +1345,8 @@ drm_dp_mst_get_mstb_malloc(struct drm_dp_mst_branch *mstb)
static void
drm_dp_mst_put_mstb_malloc(struct drm_dp_mst_branch *mstb)
{
- DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(>malloc_kref) - 1);
+ DRM_DEBUG("mstb %p/%px (%d)\n",
+ mstb, mstb, kref_read(>malloc_kref) - 1);
kref_put(>malloc_kref, drm_dp_free_mst_branch_device);
}
@@ -1379,7 +1381,8 @@ void
drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port)
{
kref_get(>malloc_kref);
- DRM_DEBUG("port %p (%d)\n", port, kref_read(>malloc_kref));
+ DRM_DEBUG("port %p/%px (%d)\n",
+ port, port, kref_read(>malloc_kref));
}
EXPORT_SYMBOL(drm_dp_mst_get_port_malloc);
@@ -1396,7 +1399,8 @@ EXPORT_SYMBOL(drm_dp_mst_get_port_malloc);
void
drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port)
{
- DRM_DEBUG("port %p (%d)\n", port, kref_read(>malloc_kref) - 1);
+ DRM_DEBUG("port %p/%px (%d)\n",
+ port, port, kref_read(>malloc_kref) - 1);
kref_put(>malloc_kref, drm_dp_free_mst_port);
}
EXPORT_SYMBOL(drm_dp_mst_put_port_malloc);
@@ -1447,8 +1451,8 @@ drm_dp_mst_topology_try_get_mstb(struct drm_dp_mst_branch
*mstb)
int ret = kref_get_unless_zero(>topology_kref);
if (ret)
- DRM_DEBUG("mstb %p (%d)\n", mstb,
- kref_read(>topology_kref));
+ DRM_DEBUG("mstb %p/%px (%d)\n",
+ mstb, mstb, kref_read(>topology_kref));
return ret;
}
@@ -1471,7 +1475,8 @@ static void drm_dp_mst_topology_get_mstb(struct
drm_dp_mst_branch *mstb)
{
WARN_ON(kref_read(>topology_kref) == 0);
kref_get(>topology_kref);
- DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(>topology_kref));
+ DRM_DEBUG("mstb %p/%px (%d)\n",
+ mstb, mstb, kref_read(>topology_kref));
}
/**
@@ -1489,8 +1494,8 @@ static void drm_dp_mst_topology_get_mstb(struct
drm_dp_mst_branch *mstb)
static void
drm_dp_mst_topology_put_mstb(struct drm_dp_mst_branch *mstb)
{
- DRM_DEBUG("mstb %p (%d)\n",
- mstb, kref_read(>topology_kref) - 1);
+ DRM_DEBUG("mstb %p/%px (%d)\n",
+ mstb, mstb, kref_read(>topology_kref) - 1);
kref_put(>topology_kref, drm_dp_destroy_mst_branch_device);
}
@@ -1546,8 +1551,8 @@ drm_dp_mst_topology_try_get_port(struct drm_dp_mst_port
*port)
int ret = kref_get_unless_zero(>topology_kref);
if (ret)
- DRM_DEBUG("port %p (%d)\n", port,
- kref_read(>topology_kref));
+ DRM_DEBUG("port %p/%px (%d)\n",
+ port, port, kref_read(>topology_kref));
return ret;
}
@@ -1569,7 +1574,8 @@ static void drm_dp_mst_topology_get_port(struct
drm_dp_mst_port *port)
{
WARN_ON(kref_read(>topology_kref) == 0);
kref_get(>topology_kref);
- DRM_DEBUG("port %p (%d)\n", port, kref_read(>topology_kref));
+ DRM_DEBUG("port %p/%px (%d)\n",
+ port, port, kref_read(>topology_kref));
}
/**
@@ -1585,8 +1591,8 @@ static void drm_dp_mst_topology_get_port(struct
drm_dp_mst_port *port)
*/
static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port)
{
- DRM_DEBUG("port %p (%d)\n",
- port, kref_read(>topology_kref) - 1);
+ DRM_DEBUG("port %p/%px (%d)\n",
+ port, port, kref_read(>topology_kref) - 1);
kref_put(>topology_kref, drm_dp_destroy_port);
}
--
2.21.0
[PATCH v2 27/27] drm/dp_mst: Add topology ref history tracking for debugging
For very subtle mistakes with topology refs, it can be rather difficult
to trace them down with the debugging info that we already have. I had
one such issue recently while trying to implement suspend/resume
reprobing for MST, and ended up coming up with this.
Inspired by Chris Wilson's wakeref tracking for i915, this adds a very
similar feature to the DP MST helpers, which allows for partial tracking
of topology refs for both ports and branch devices. This is a lot less
advanced then wakeref tracking: we merely keep a count of all of the
spots where a topology ref has been grabbed or dropped, then dump out
that history in chronological order when a port or branch device's
topology refcount reaches 0. So far, I've found this incredibly useful
for debugging topology refcount errors.
Since this has the potential to be somewhat slow and loud, we add an
expert kernel config option to enable or disable this feature,
CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS.
Changes since v1:
* Don't forget to destroy topology_ref_history_lock
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/Kconfig | 14 ++
drivers/gpu/drm/drm_dp_mst_topology.c | 233 +-
include/drm/drm_dp_mst_helper.h | 45 +
3 files changed, 288 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e67c194c2aca..44fc2c2a6e2c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -93,6 +93,20 @@ config DRM_KMS_FB_HELPER
help
FBDEV helpers for KMS drivers.
+config DRM_DEBUG_DP_MST_TOPOLOGY_REFS
+bool "Enable refcount backtrace history in the DP MST helpers"
+select STACKDEPOT
+depends on DRM_KMS_HELPER
+depends on DEBUG_KERNEL
+depends on EXPERT
+help
+ Enables debug tracing for topology refs in DRM's DP MST helpers. A
+ history of each topology reference/dereference will be printed to the
+ kernel log once a port or branch device's topology refcount reaches
0.
+
+ This has the potential to use a lot of memory and print some very
+ large kernel messages. If in doubt, say "N".
+
config DRM_FBDEV_EMULATION
bool "Enable legacy fbdev support for your modesetting driver"
depends on DRM
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5b5c0b3b3c0e..18f9a02927d9 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -28,6 +28,13 @@
#include
#include
+#if IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS)
+#include
+#include
+#include
+#include
+#endif
+
#include
#include
#include
@@ -1405,12 +1412,189 @@ drm_dp_mst_put_port_malloc(struct drm_dp_mst_port
*port)
}
EXPORT_SYMBOL(drm_dp_mst_put_port_malloc);
+#if IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS)
+
+#define STACK_DEPTH 8
+
+static noinline void
+__topology_ref_save(struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_topology_ref_history *history,
+ enum drm_dp_mst_topology_ref_type type)
+{
+ struct drm_dp_mst_topology_ref_entry *entry = NULL;
+ depot_stack_handle_t backtrace;
+ ulong stack_entries[STACK_DEPTH];
+ uint n;
+ int i;
+
+ n = stack_trace_save(stack_entries, ARRAY_SIZE(stack_entries), 1);
+ backtrace = stack_depot_save(stack_entries, n, GFP_KERNEL);
+ if (!backtrace)
+ goto fail_alloc;
+
+ /* Try to find an existing entry for this backtrace */
+ for (i = 0; i < history->len; i++) {
+ if (history->entries[i].backtrace == backtrace) {
+ entry = >entries[i];
+ break;
+ }
+ }
+
+ /* Otherwise add one */
+ if (!entry) {
+ struct drm_dp_mst_topology_ref_entry *new;
+ int new_len = history->len + 1;
+
+ new = krealloc(history->entries, sizeof(*new) * new_len,
+ GFP_KERNEL);
+ if (!new)
+ goto fail_alloc;
+
+ entry = [history->len];
+ history->len = new_len;
+ history->entries = new;
+
+ entry->backtrace = backtrace;
+ entry->type = type;
+ entry->count = 0;
+ }
+ entry->count++;
+ entry->ts_nsec = ktime_get_ns();
+
+ return;
+fail_alloc:
+ DRM_WARN_ONCE("Failed to allocate memory for topology refcount
backtrace\n");
+}
+
+static int
+topology_ref_history_cmp(const void *a, const void *b)
+{
+ const struct drm_dp_mst_topology_ref_entry *entry_a = a, *entry_b = b;
+
+ if (entry_a->ts_nsec > entry_b->ts_nsec)
+ return 1;
+ else if (entry_a->ts_nsec < entry_b->ts_nsec)
+ return -1;
+ else
+
[PATCH v2 23/27] drm/amdgpu: Iterate through DRM connectors correctly
Currently, every single piece of code in amdgpu that loops through
connectors does it incorrectly and doesn't use the proper list iteration
helpers, drm_connector_list_iter_begin() and
drm_connector_list_iter_end(). Yeesh.
So, do that.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 13 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 20 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 5 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 40 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 5 ++-
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 34
drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 34
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 40 ++-
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 34
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 33 ---
.../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 10 -
11 files changed, 195 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index ece55c8fa673..bd31bb595c04 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -1022,8 +1022,12 @@ amdgpu_connector_dvi_detect(struct drm_connector
*connector, bool force)
*/
if (amdgpu_connector->shared_ddc && (ret ==
connector_status_connected)) {
struct drm_connector *list_connector;
+ struct drm_connector_list_iter iter;
struct amdgpu_connector *list_amdgpu_connector;
- list_for_each_entry(list_connector,
>mode_config.connector_list, head) {
+
+ drm_connector_list_iter_begin(dev, );
+ drm_for_each_connector_iter(list_connector,
+ ) {
if (connector == list_connector)
continue;
list_amdgpu_connector =
to_amdgpu_connector(list_connector);
@@ -1040,6 +1044,7 @@ amdgpu_connector_dvi_detect(struct drm_connector
*connector, bool force)
}
}
}
+ drm_connector_list_iter_end();
}
}
}
@@ -1501,6 +1506,7 @@ amdgpu_connector_add(struct amdgpu_device *adev,
{
struct drm_device *dev = adev->ddev;
struct drm_connector *connector;
+ struct drm_connector_list_iter iter;
struct amdgpu_connector *amdgpu_connector;
struct amdgpu_connector_atom_dig *amdgpu_dig_connector;
struct drm_encoder *encoder;
@@ -1515,10 +1521,12 @@ amdgpu_connector_add(struct amdgpu_device *adev,
return;
/* see if we already added it */
- list_for_each_entry(connector, >mode_config.connector_list, head) {
+ drm_connector_list_iter_begin(dev, );
+ drm_for_each_connector_iter(connector, ) {
amdgpu_connector = to_amdgpu_connector(connector);
if (amdgpu_connector->connector_id == connector_id) {
amdgpu_connector->devices |= supported_device;
+ drm_connector_list_iter_end();
return;
}
if (amdgpu_connector->ddc_bus && i2c_bus->valid) {
@@ -1533,6 +1541,7 @@ amdgpu_connector_add(struct amdgpu_device *adev,
}
}
}
+ drm_connector_list_iter_end();
/* check if it's a dp bridge */
list_for_each_entry(encoder, >mode_config.encoder_list, head) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2f884699eaef..acd39ce9b08e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3004,6 +3004,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool
suspend, bool fbcon)
struct amdgpu_device *adev;
struct drm_crtc *crtc;
struct drm_connector *connector;
+ struct drm_connector_list_iter iter;
int r;
if (dev == NULL || dev->dev_private == NULL) {
@@ -3026,9 +3027,11 @@ int amdgpu_device_suspend(struct drm_device *dev, bool
suspend, bool fbcon)
if (!amdgpu_device_has_dc_support(adev)) {
/* turn off display hw */
drm_modeset_lock_all(dev);
- list_for_each_entry(connector,
>mode_config.connector_list, head) {
- drm_helper_connector_dpms(connector,
[PATCH v2 19/27] drm/dp_mst: Handle UP requests asynchronously
Once upon a time, hotplugging devices on MST branches actually worked in
DRM. Now, it only works in amdgpu (likely because of how it's hotplug
handlers are implemented). On both i915 and nouveau, hotplug
notifications from MST branches are noticed - but trying to respond to
them causes messaging timeouts and causes the whole topology state to go
out of sync with reality, usually resulting in the user needing to
replug the entire topology in hopes that it actually fixes things.
The reason for this is because the way we currently handle UP requests
in MST is completely bogus. drm_dp_mst_handle_up_req() is called from
drm_dp_mst_hpd_irq(), which is usually called from the driver's hotplug
handler. Because we handle sending the hotplug event from this function,
we actually cause the driver's hotplug handler (and in turn, all
sideband transactions) to block on
drm_device->mode_config.connection_mutex. This makes it impossible to
send any sideband messages from the driver's connector probing
functions, resulting in the aforementioned sideband message timeout.
There's even more problems with this beyond breaking hotplugging on MST
branch devices. It also makes it almost impossible to protect
drm_dp_mst_port struct members under a lock because we then have to
worry about dealing with all of the lock dependency issues that ensue.
So, let's finally actually fix this issue by handling the processing of
up requests asyncronously. This way we can send sideband messages from
most contexts without having to deal with getting blocked if we hold
connection_mutex. This also fixes MST branch device hotplugging on i915,
finally!
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 146 +++---
include/drm/drm_dp_mst_helper.h | 16 +++
2 files changed, 122 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index cfaf9eb7ace9..5101eeab4485 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -46,6 +46,12 @@
* protocol. The helpers contain a topology manager and bandwidth manager.
* The helpers encapsulate the sending and received of sideband msgs.
*/
+struct drm_dp_pending_up_req {
+ struct drm_dp_sideband_msg_hdr hdr;
+ struct drm_dp_sideband_msg_req_body msg;
+ struct list_head next;
+};
+
static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
char *buf);
@@ -3109,6 +3115,7 @@ void drm_dp_mst_topology_mgr_suspend(struct
drm_dp_mst_topology_mgr *mgr)
drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL,
DP_MST_EN | DP_UPSTREAM_IS_SRC);
mutex_unlock(>lock);
+ flush_work(>up_req_work);
flush_work(>work);
flush_work(>delayed_destroy_work);
}
@@ -3281,12 +3288,70 @@ static int drm_dp_mst_handle_down_rep(struct
drm_dp_mst_topology_mgr *mgr)
return 0;
}
+static inline void
+drm_dp_mst_process_up_req(struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_pending_up_req *up_req)
+{
+ struct drm_dp_mst_branch *mstb = NULL;
+ struct drm_dp_sideband_msg_req_body *msg = _req->msg;
+ struct drm_dp_sideband_msg_hdr *hdr = _req->hdr;
+
+ if (hdr->broadcast) {
+ const u8 *guid = NULL;
+
+ if (msg->req_type == DP_CONNECTION_STATUS_NOTIFY)
+ guid = msg->u.conn_stat.guid;
+ else if (msg->req_type == DP_RESOURCE_STATUS_NOTIFY)
+ guid = msg->u.resource_stat.guid;
+
+ mstb = drm_dp_get_mst_branch_device_by_guid(mgr, guid);
+ } else {
+ mstb = drm_dp_get_mst_branch_device(mgr, hdr->lct, hdr->rad);
+ }
+
+ if (!mstb) {
+ DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
+ hdr->lct);
+ return;
+ }
+
+ /* TODO: Add missing handler for DP_RESOURCE_STATUS_NOTIFY events */
+ if (msg->req_type == DP_CONNECTION_STATUS_NOTIFY) {
+ drm_dp_mst_handle_conn_stat(mstb, >u.conn_stat);
+ drm_kms_helper_hotplug_event(mgr->dev);
+ }
+
+ drm_dp_mst_topology_put_mstb(mstb);
+}
+
+static void drm_dp_mst_up_req_work(struct work_struct *work)
+{
+ struct drm_dp_mst_topology_mgr *mgr =
+ container_of(work, struct drm_dp_mst_topology_mgr,
+up_req_work);
+ struct drm_dp_pending_up_req *up_req;
+
+ while (true) {
+ mutex_lock(>up_req_lock);
+ up_req = list_first_entry_or_null(>up_req_list,
+ struct drm_dp_pending_up_req,
+ next);
+ if (up_req)
+ list_del(_req->next);
+
[PATCH v2 20/27] drm/dp_mst: Protect drm_dp_mst_port members with connection_mutex
Yes-you read that right. Currently there is literally no locking in
place for any of the drm_dp_mst_port struct members that can be modified
in response to a link address response, or a connection status response.
Which literally means if we're unlucky enough to have any sort of
hotplugging event happen before we're finished with reprobing link
addresses, we'll race and the contents of said struct members becomes
undefined. Fun!
So, finally add some simple locking protections to our MST helpers by
protecting any drm_dp_mst_port members which can be changed by link
address responses or connection status notifications under
drm_device->mode_config.connection_mutex.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 144 +++---
include/drm/drm_dp_mst_helper.h | 39 +--
2 files changed, 133 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5101eeab4485..259634c5d6dc 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1354,6 +1354,7 @@ static void drm_dp_free_mst_port(struct kref *kref)
container_of(kref, struct drm_dp_mst_port, malloc_kref);
drm_dp_mst_put_mstb_malloc(port->parent);
+ mutex_destroy(>lock);
kfree(port);
}
@@ -1906,6 +1907,36 @@ void drm_dp_mst_connector_early_unregister(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister);
+static void
+drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
+ struct drm_dp_mst_port *port)
+{
+ struct drm_dp_mst_topology_mgr *mgr = port->mgr;
+ char proppath[255];
+ int ret;
+
+ build_mst_prop_path(mstb, port->port_num, proppath, sizeof(proppath));
+ port->connector = mgr->cbs->add_connector(mgr, port, proppath);
+ if (!port->connector) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
+port->pdt == DP_PEER_DEVICE_SST_SINK) &&
+ port->port_num >= DP_MST_LOGICAL_PORT_0) {
+ port->cached_edid = drm_get_edid(port->connector,
+>aux.ddc);
+ drm_connector_set_tile_property(port->connector);
+ }
+
+ mgr->cbs->register_connector(port->connector);
+ return;
+
+error:
+ DRM_ERROR("Failed to create connector for port %p: %d\n", port, ret);
+}
+
static void
drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
struct drm_device *dev,
@@ -1913,8 +1944,12 @@ drm_dp_mst_handle_link_address_port(struct
drm_dp_mst_branch *mstb,
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- bool created = false;
- int old_ddps = 0;
+ struct drm_dp_mst_branch *child_mstb = NULL;
+ struct drm_connector *connector_to_destroy = NULL;
+ int old_ddps = 0, ret;
+ u8 new_pdt = DP_PEER_DEVICE_NONE;
+ bool created = false, send_link_addr = false,
+create_connector = false;
port = drm_dp_get_port(mstb, port_msg->port_number);
if (!port) {
@@ -1923,6 +1958,7 @@ drm_dp_mst_handle_link_address_port(struct
drm_dp_mst_branch *mstb,
return;
kref_init(>topology_kref);
kref_init(>malloc_kref);
+ mutex_init(>lock);
port->parent = mstb;
port->port_num = port_msg->port_number;
port->mgr = mgr;
@@ -1937,11 +1973,17 @@ drm_dp_mst_handle_link_address_port(struct
drm_dp_mst_branch *mstb,
drm_dp_mst_get_mstb_malloc(mstb);
created = true;
- } else {
- old_ddps = port->ddps;
}
+ mutex_lock(>lock);
+ drm_modeset_lock(>mode_config.connection_mutex, NULL);
+
+ if (!created)
+ old_ddps = port->ddps;
+
port->input = port_msg->input_port;
+ if (!port->input)
+ new_pdt = port_msg->peer_device_type;
port->mcs = port_msg->mcs;
port->ddps = port_msg->ddps;
port->ldps = port_msg->legacy_device_plug_status;
@@ -1969,44 +2011,58 @@ drm_dp_mst_handle_link_address_port(struct
drm_dp_mst_branch *mstb,
}
}
- if (!port->input) {
- int ret = drm_dp_port_set_pdt(port,
- port_msg->peer_device_type);
- if (ret == 1) {
- drm_dp_send_link_address(mgr, port->mstb);
- } else if (ret < 0) {
- DRM_ERROR("Failed to change PDT on port %p: %d\n",
- port, ret);
- goto fail;
+ ret =
[PATCH v2 21/27] drm/dp_mst: Don't forget to update port->input in drm_dp_mst_handle_conn_stat()
This probably hasn't caused any problems up until now since it's
probably nearly impossible to encounter this in the wild, however if we
were to receive a connection status notification from the MST hub after
resume while we're in the middle of reprobing the link addresses for a
topology then there's a much larger chance that a port could have
changed from being an output port to input port (or vice versa). If we
forget to update this bit of information, we'll potentially ignore a
valid PDT change on a downstream port because we think it's an input
port.
So, make sure we read the input_port field in connection status
notifications in drm_dp_mst_handle_conn_stat() to prevent this from
happening once we've implemented suspend/resume reprobing.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 51 +++
1 file changed, 37 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 259634c5d6dc..e407aba1fbd2 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2078,18 +2078,23 @@ static void
drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
struct drm_dp_connection_status_notify *conn_stat)
{
- struct drm_device *dev = mstb->mgr->dev;
+ struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
+ struct drm_device *dev = mgr->dev;
struct drm_dp_mst_port *port;
- int old_ddps;
- bool dowork = false;
+ struct drm_connector *connector_to_destroy = NULL;
+ int old_ddps, ret;
+ u8 new_pdt;
+ bool dowork = false, create_connector = false;
port = drm_dp_get_port(mstb, conn_stat->port_number);
if (!port)
return;
+ mutex_lock(>lock);
drm_modeset_lock(>mode_config.connection_mutex, NULL);
old_ddps = port->ddps;
+ port->input = conn_stat->input_port;
port->mcs = conn_stat->message_capability_status;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2102,23 +2107,41 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
}
}
- if (!port->input) {
- int ret = drm_dp_port_set_pdt(port,
- conn_stat->peer_device_type);
- if (ret == 1) {
- dowork = true;
- } else if (ret < 0) {
- DRM_ERROR("Failed to change PDT for port %p: %d\n",
- port, ret);
- dowork = false;
- }
+ new_pdt = port->input ? DP_PEER_DEVICE_NONE :
conn_stat->peer_device_type;
+
+ ret = drm_dp_port_set_pdt(port, new_pdt);
+ if (ret == 1) {
+ dowork = true;
+ } else if (ret < 0) {
+ DRM_ERROR("Failed to change PDT for port %p: %d\n",
+ port, ret);
+ dowork = false;
+ }
+
+ /*
+* We unset port->connector before dropping connection_mutex so that
+* there's no chance any of the atomic MST helpers can accidentally
+* associate a to-be-destroyed connector with a port.
+*/
+ if (port->connector && port->input) {
+ connector_to_destroy = port->connector;
+ port->connector = NULL;
+ } else if (!port->connector && !port->input) {
+ create_connector = true;
}
drm_modeset_unlock(>mode_config.connection_mutex);
+
+ if (connector_to_destroy)
+ mgr->cbs->destroy_connector(mgr, connector_to_destroy);
+ else if (create_connector)
+ drm_dp_mst_port_add_connector(mstb, port);
+
+ mutex_unlock(>lock);
+
drm_dp_mst_topology_put_port(port);
if (dowork)
queue_work(system_long_wq, >mgr->work);
-
}
static struct drm_dp_mst_branch *drm_dp_get_mst_branch_device(struct
drm_dp_mst_topology_mgr *mgr,
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 22/27] drm/nouveau: Don't grab runtime PM refs for HPD IRQs
In order for suspend/resume reprobing to work, we need to be able to
perform sideband communications during suspend/resume, along with
runtime PM suspend/resume. In order to do so, we also need to make sure
that nouveau doesn't bother grabbing a runtime PM reference to do so,
since otherwise we'll start deadlocking runtime PM again.
Note that we weren't able to do this before, because of the DP MST
helpers processing UP requests from topologies in the same context as
drm_dp_mst_hpd_irq() which would have caused us to open ourselves up to
receiving hotplug events and deadlocking with runtime suspend/resume.
Now that those requests are handled asynchronously, this change should
be completely safe.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/nouveau_connector.c | 33 +++--
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 56871d34e3fb..f276918d3f3b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1131,6 +1131,16 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
const char *name = connector->name;
struct nouveau_encoder *nv_encoder;
int ret;
+ bool plugged = (rep->mask != NVIF_NOTIFY_CONN_V0_UNPLUG);
+
+ if (rep->mask & NVIF_NOTIFY_CONN_V0_IRQ) {
+ NV_DEBUG(drm, "service %s\n", name);
+ drm_dp_cec_irq(_connector->aux);
+ if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP)))
+ nv50_mstm_service(nv_encoder->dp.mstm);
+
+ return NVIF_NOTIFY_KEEP;
+ }
ret = pm_runtime_get(drm->dev->dev);
if (ret == 0) {
@@ -1151,25 +1161,16 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
return NVIF_NOTIFY_DROP;
}
- if (rep->mask & NVIF_NOTIFY_CONN_V0_IRQ) {
- NV_DEBUG(drm, "service %s\n", name);
- drm_dp_cec_irq(_connector->aux);
- if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP)))
- nv50_mstm_service(nv_encoder->dp.mstm);
- } else {
- bool plugged = (rep->mask != NVIF_NOTIFY_CONN_V0_UNPLUG);
-
+ if (!plugged)
+ drm_dp_cec_unset_edid(_connector->aux);
+ NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", name);
+ if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP))) {
if (!plugged)
- drm_dp_cec_unset_edid(_connector->aux);
- NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", name);
- if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP))) {
- if (!plugged)
- nv50_mstm_remove(nv_encoder->dp.mstm);
- }
-
- drm_helper_hpd_irq_event(connector->dev);
+ nv50_mstm_remove(nv_encoder->dp.mstm);
}
+ drm_helper_hpd_irq_event(connector->dev);
+
pm_runtime_mark_last_busy(drm->dev->dev);
pm_runtime_put_autosuspend(drm->dev->dev);
return NVIF_NOTIFY_KEEP;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 17/27] drm/dp_mst: Rename drm_dp_add_port and drm_dp_update_port
The names for these functions are rather confusing. drm_dp_add_port()
sounds like a function that would simply create a port and add it to a
topology, and do nothing more. Similarly, drm_dp_update_port() would be
assumed to be the function that should be used to update port
information after initial creation.
While those assumptions are currently correct in how these functions are
used, a quick glance at drm_dp_add_port() reveals that drm_dp_add_port()
can also update the information on a port, and seems explicitly designed
to do so. This can be explained pretty simply by the fact that there's
more situations that would involve updating the port information based
on a link address response as opposed to a connection status
notification than the driver's initial topology probe. Case in point:
reprobing link addresses after suspend/resume.
Since we're about to start using drm_dp_add_port() differently for
suspend/resume reprobing, let's rename both functions to clarify what
they actually do.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 9944ef2ce885..cfaf9eb7ace9 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1900,9 +1900,10 @@ void drm_dp_mst_connector_early_unregister(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister);
-static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
- struct drm_device *dev,
- struct drm_dp_link_addr_reply_port *port_msg)
+static void
+drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
+ struct drm_device *dev,
+ struct drm_dp_link_addr_reply_port
*port_msg)
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
@@ -2011,8 +2012,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
*mstb,
drm_dp_mst_topology_put_port(port);
}
-static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
- struct drm_dp_connection_status_notify
*conn_stat)
+static void
+drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
+ struct drm_dp_connection_status_notify *conn_stat)
{
struct drm_dp_mst_port *port;
int old_ddps;
@@ -2464,7 +2466,8 @@ static void drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
drm_dp_check_mstb_guid(mstb, reply->guid);
for (i = 0; i < reply->nports; i++)
- drm_dp_add_port(mstb, mgr->dev, >ports[i]);
+ drm_dp_mst_handle_link_address_port(mstb, mgr->dev,
+ >ports[i]);
drm_kms_helper_hotplug_event(mgr->dev);
@@ -3324,7 +3327,7 @@ static int drm_dp_mst_handle_up_req(struct
drm_dp_mst_topology_mgr *mgr)
}
if (msg.req_type == DP_CONNECTION_STATUS_NOTIFY) {
- drm_dp_update_port(mstb, _stat);
+ drm_dp_mst_handle_conn_stat(mstb, _stat);
DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d
pdt: %d\n",
msg.u.conn_stat.port_number,
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109389] memory leak in `amdgpu_bo_create()`
https://bugs.freedesktop.org/show_bug.cgi?id=109389 --- Comment #5 from Czcibor Bohusz-Dobosz --- Created attachment 145256 --> https://bugs.freedesktop.org/attachment.cgi?id=145256=edit Galactic Civilizations III memleak log with DXVK For comparison, I also attach a similar log that I made with the DXVK translation layer enabled, which caused the game to leak much larger amounts of memory, to the point of making it unplayable. While Galactic Civilizations III is the only game which I've confirmed to permanently leak memory through this call when DXVK is not used, virtually all D3D games I've tried to translate to Vulkan so far have leaks like this; unfortunately, I don't currently have my hands on any native Vulkan production to test. In the logs I am only launching the game until it reaches the main menu, thus the leak is, well, pretty serious in my case... :) The Vulkan driver reports itself as AMD RADV KAVERI (LLVM 8.0.1) 1.9.15. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 14/27] drm/dp_mst: Destroy topology_mgr mutexes
Turns out we've been forgetting for a while now to actually destroy any of the mutexes that we create in drm_dp_mst_topology_mgr. So, let's do that. Cc: Juston Li Cc: Imre Deak Cc: Ville Syrjälä Cc: Harry Wentland Cc: Daniel Vetter Signed-off-by: Lyude Paul --- drivers/gpu/drm/drm_dp_mst_topology.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 74161f442584..2f88cc173500 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4339,6 +4339,11 @@ void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr) mgr->aux = NULL; drm_atomic_private_obj_fini(>base); mgr->funcs = NULL; + + mutex_destroy(>delayed_destroy_lock); + mutex_destroy(>payload_lock); + mutex_destroy(>qlock); + mutex_destroy(>lock); } EXPORT_SYMBOL(drm_dp_mst_topology_mgr_destroy); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 24/27] drm/amdgpu/dm: Resume short HPD IRQs before resuming MST topology
Since we're going to be reprobing the entire topology state on resume
now using sideband transactions, we need to ensure that we actually have
short HPD irqs enabled before calling drm_dp_mst_topology_mgr_resume().
So, do that.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 73630e2940d4..4d3c8bff77da 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1185,15 +1185,15 @@ static int dm_resume(void *handle)
/* program HPD filter */
dc_resume(dm->dc);
- /* On resume we need to rewrite the MSTM control bits to enamble MST*/
- s3_handle_mst(ddev, false);
-
/*
* early enable HPD Rx IRQ, should be done before set mode as short
* pulse interrupts are used for MST
*/
amdgpu_dm_irq_resume_early(adev);
+ /* On resume we need to rewrite the MSTM control bits to enamble MST*/
+ s3_handle_mst(ddev, false);
+
/* Do detection*/
drm_connector_list_iter_begin(ddev, );
drm_for_each_connector_iter(connector, ) {
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 10/27] drm/dp_mst: Remove huge conditional in drm_dp_mst_handle_up_req()
Which reduces indentation and makes this function more legible.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 90 +--
1 file changed, 45 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 241c66f75bed..43452872efad 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3245,7 +3245,9 @@ static int drm_dp_mst_handle_down_rep(struct
drm_dp_mst_topology_mgr *mgr)
static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
{
- int ret = 0;
+ struct drm_dp_sideband_msg_req_body msg;
+ struct drm_dp_mst_branch *mstb = NULL;
+ bool seqno;
if (!drm_dp_get_one_sb_msg(mgr, true)) {
memset(>up_req_recv, 0,
@@ -3253,62 +3255,60 @@ static int drm_dp_mst_handle_up_req(struct
drm_dp_mst_topology_mgr *mgr)
return 0;
}
- if (mgr->up_req_recv.have_eomt) {
- struct drm_dp_sideband_msg_req_body msg;
- struct drm_dp_mst_branch *mstb = NULL;
- bool seqno;
-
- if (!mgr->up_req_recv.initial_hdr.broadcast) {
- mstb = drm_dp_get_mst_branch_device(mgr,
-
mgr->up_req_recv.initial_hdr.lct,
-
mgr->up_req_recv.initial_hdr.rad);
- if (!mstb) {
- DRM_DEBUG_KMS("Got MST reply from unknown
device %d\n", mgr->up_req_recv.initial_hdr.lct);
- memset(>up_req_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
- return 0;
- }
+ if (!mgr->up_req_recv.have_eomt)
+ return 0;
+
+ if (!mgr->up_req_recv.initial_hdr.broadcast) {
+ mstb = drm_dp_get_mst_branch_device(mgr,
+
mgr->up_req_recv.initial_hdr.lct,
+
mgr->up_req_recv.initial_hdr.rad);
+ if (!mstb) {
+ DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
mgr->up_req_recv.initial_hdr.lct);
+ memset(>up_req_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
+ return 0;
}
+ }
- seqno = mgr->up_req_recv.initial_hdr.seqno;
- drm_dp_sideband_parse_req(>up_req_recv, );
+ seqno = mgr->up_req_recv.initial_hdr.seqno;
+ drm_dp_sideband_parse_req(>up_req_recv, );
- if (msg.req_type == DP_CONNECTION_STATUS_NOTIFY) {
- drm_dp_send_up_ack_reply(mgr, mgr->mst_primary,
msg.req_type, seqno, false);
+ if (msg.req_type == DP_CONNECTION_STATUS_NOTIFY) {
+ drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type,
seqno, false);
- if (!mstb)
- mstb =
drm_dp_get_mst_branch_device_by_guid(mgr, msg.u.conn_stat.guid);
+ if (!mstb)
+ mstb = drm_dp_get_mst_branch_device_by_guid(mgr,
msg.u.conn_stat.guid);
- if (!mstb) {
- DRM_DEBUG_KMS("Got MST reply from unknown
device %d\n", mgr->up_req_recv.initial_hdr.lct);
- memset(>up_req_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
- return 0;
- }
+ if (!mstb) {
+ DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
mgr->up_req_recv.initial_hdr.lct);
+ memset(>up_req_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
+ return 0;
+ }
- drm_dp_update_port(mstb, _stat);
+ drm_dp_update_port(mstb, _stat);
- DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d
ip: %d pdt: %d\n", msg.u.conn_stat.port_number,
msg.u.conn_stat.legacy_device_plug_status,
msg.u.conn_stat.displayport_device_plug_status,
msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port,
msg.u.conn_stat.peer_device_type);
- drm_kms_helper_hotplug_event(mgr->dev);
-
- } else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
- drm_dp_send_up_ack_reply(mgr, mgr->mst_primary,
msg.req_type, seqno, false);
- if (!mstb)
- mstb =
drm_dp_get_mst_branch_device_by_guid(mgr, msg.u.resource_stat.guid);
+ DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d
pdt: %d\n", msg.u.conn_stat.port_number,
msg.u.conn_stat.legacy_device_plug_status,
[PATCH v2 12/27] drm/dp_mst: Refactor drm_dp_mst_handle_up_req()
There's a couple of changes here, so to summarize:
* Remove the big ugly mgr->up_req_recv.have_eomt conditional to save on
indenting
* Store >up_req_recv.initial_hdr in a variable so we don't keep
going over 80 character long lines
* De-duplicate code for calling drm_dp_send_up_ack_reply() and getting
the MSTB via it's GUID
* Remove all of the duplicate calls to memset() and just use a goto
instead
* Actually do line wrapping
* Remove the unnecessary if (mstb) check before calling
drm_dp_mst_topology_put_mstb() - we are guaranteed to always have
mstb != NULL at that point in the function
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Reviewed-by: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 75 ++-
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b44d3696c09a..64487098158a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3246,68 +3246,69 @@ static int drm_dp_mst_handle_down_rep(struct
drm_dp_mst_topology_mgr *mgr)
static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
{
struct drm_dp_sideband_msg_req_body msg;
+ struct drm_dp_sideband_msg_hdr *hdr = >up_req_recv.initial_hdr;
struct drm_dp_mst_branch *mstb = NULL;
+ const u8 *guid;
bool seqno;
- if (!drm_dp_get_one_sb_msg(mgr, true)) {
- memset(>up_req_recv, 0,
- sizeof(struct drm_dp_sideband_msg_rx));
- return 0;
- }
+ if (!drm_dp_get_one_sb_msg(mgr, true))
+ goto out;
if (!mgr->up_req_recv.have_eomt)
return 0;
- if (!mgr->up_req_recv.initial_hdr.broadcast) {
- mstb = drm_dp_get_mst_branch_device(mgr,
-
mgr->up_req_recv.initial_hdr.lct,
-
mgr->up_req_recv.initial_hdr.rad);
+ if (!hdr->broadcast) {
+ mstb = drm_dp_get_mst_branch_device(mgr, hdr->lct, hdr->rad);
if (!mstb) {
- DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
mgr->up_req_recv.initial_hdr.lct);
- memset(>up_req_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
- return 0;
+ DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
+ hdr->lct);
+ goto out;
}
}
- seqno = mgr->up_req_recv.initial_hdr.seqno;
+ seqno = hdr->seqno;
drm_dp_sideband_parse_req(>up_req_recv, );
- if (msg.req_type == DP_CONNECTION_STATUS_NOTIFY) {
- drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type,
seqno, false);
+ if (msg.req_type == DP_CONNECTION_STATUS_NOTIFY)
+ guid = msg.u.conn_stat.guid;
+ else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY)
+ guid = msg.u.resource_stat.guid;
+ else
+ goto out;
- if (!mstb)
- mstb = drm_dp_get_mst_branch_device_by_guid(mgr,
msg.u.conn_stat.guid);
+ drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno,
+false);
+ if (!mstb) {
+ mstb = drm_dp_get_mst_branch_device_by_guid(mgr, guid);
if (!mstb) {
- DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
mgr->up_req_recv.initial_hdr.lct);
- memset(>up_req_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
- return 0;
+ DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
+ hdr->lct);
+ goto out;
}
+ }
+ if (msg.req_type == DP_CONNECTION_STATUS_NOTIFY) {
drm_dp_update_port(mstb, _stat);
- DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d
pdt: %d\n", msg.u.conn_stat.port_number,
msg.u.conn_stat.legacy_device_plug_status,
msg.u.conn_stat.displayport_device_plug_status,
msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port,
msg.u.conn_stat.peer_device_type);
- drm_kms_helper_hotplug_event(mgr->dev);
+ DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d
pdt: %d\n",
+ msg.u.conn_stat.port_number,
+ msg.u.conn_stat.legacy_device_plug_status,
+ msg.u.conn_stat.displayport_device_plug_status,
+ msg.u.conn_stat.message_capability_status,
+ msg.u.conn_stat.input_port,
+ msg.u.conn_stat.peer_device_type);
+
[PATCH v2 15/27] drm/dp_mst: Cleanup drm_dp_send_link_address() a bit
Declare local pointer to the drm_dp_link_address_ack_reply struct
instead of constantly dereferencing it through the union in
txmsg->reply. Then, invert the order of conditionals so we don't have to
do the bulk of the work inside them, and can wrap lines even less. Then
finally, rearrange variable declarations a bit.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 42 +++
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 2f88cc173500..d1610434a0cb 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2398,9 +2398,9 @@ drm_dp_dump_link_address(struct
drm_dp_link_address_ack_reply *reply)
static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_branch *mstb)
{
- int len;
struct drm_dp_sideband_msg_tx *txmsg;
- int ret;
+ struct drm_dp_link_address_ack_reply *reply;
+ int i, len, ret;
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
if (!txmsg)
@@ -2412,28 +2412,32 @@ static void drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
mstb->link_address_sent = true;
drm_dp_queue_down_tx(mgr, txmsg);
+ /* FIXME: Actually do some real error handling here */
ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
- if (ret > 0) {
- int i;
+ if (ret <= 0) {
+ DRM_ERROR("Sending link address failed with %d\n", ret);
+ goto out;
+ }
+ if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) {
+ DRM_ERROR("link address NAK received\n");
+ ret = -EIO;
+ goto out;
+ }
- if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) {
- DRM_DEBUG_KMS("link address nak received\n");
- } else {
- DRM_DEBUG_KMS("link address reply: %d\n",
txmsg->reply.u.link_addr.nports);
- drm_dp_dump_link_address(>reply.u.link_addr);
+ reply = >reply.u.link_addr;
+ DRM_DEBUG_KMS("link address reply: %d\n", reply->nports);
+ drm_dp_dump_link_address(reply);
- drm_dp_check_mstb_guid(mstb,
txmsg->reply.u.link_addr.guid);
+ drm_dp_check_mstb_guid(mstb, reply->guid);
- for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
- drm_dp_add_port(mstb, mgr->dev,
>reply.u.link_addr.ports[i]);
- }
- drm_kms_helper_hotplug_event(mgr->dev);
- }
- } else {
- mstb->link_address_sent = false;
- DRM_DEBUG_KMS("link address failed %d\n", ret);
- }
+ for (i = 0; i < reply->nports; i++)
+ drm_dp_add_port(mstb, mgr->dev, >ports[i]);
+
+ drm_kms_helper_hotplug_event(mgr->dev);
+out:
+ if (ret <= 0)
+ mstb->link_address_sent = false;
kfree(txmsg);
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 07/27] drm/dp_mst: Add sideband down request tracing + selftests
Unfortunately the DP MST helpers do not have much in the way of
debugging utilities. So, let's add some!
This adds basic debugging output for down sideband requests that we send
from the driver, so that we can actually discern what's happening when
sideband requests timeout.
Since there wasn't really a good way of testing that any of this worked,
I ended up writing simple selftests that lightly test sideband message
encoding and decoding as well. Enjoy!
Changes since v1:
* Clean up DO_TEST() and sideband_msg_req_encode_decode() - danvet
* Get rid of pr_fmt(), just define a prefix string instead and use
drm_printf()
* Check highest bit of VCPI in drm_dp_decode_sideband_req() - danvet
* Make the switch case order between drm_dp_decode_sideband_req() and
drm_dp_encode_sideband_req() the same - danvet
* Only check DRM_UT_DP - danvet
* Clean up sideband_msg_req_equal() from selftests a bit, and add
comments explaining why we can't just use memcmp - danvet
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Reviewed-by: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 309 +-
.../gpu/drm/drm_dp_mst_topology_internal.h| 24 ++
.../gpu/drm/selftests/drm_modeset_selftests.h | 1 +
.../drm/selftests/test-drm_dp_mst_helper.c| 204
.../drm/selftests/test-drm_modeset_common.h | 1 +
include/drm/drm_dp_mst_helper.h | 2 +-
6 files changed, 536 insertions(+), 5 deletions(-)
create mode 100644 drivers/gpu/drm/drm_dp_mst_topology_internal.h
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 1c862749cb63..f5f1d8b50fb6 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -37,6 +37,7 @@
#include
#include "drm_crtc_helper_internal.h"
+#include "drm_dp_mst_topology_internal.h"
/**
* DOC: dp mst helper
@@ -73,6 +74,8 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux
*aux);
static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux);
static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
+#define DBG_PREFIX "[dp_mst]"
+
#define DP_STR(x) [DP_ ## x] = #x
static const char *drm_dp_mst_req_type_str(u8 req_type)
@@ -129,6 +132,43 @@ static const char *drm_dp_mst_nak_reason_str(u8 nak_reason)
}
#undef DP_STR
+#define DP_STR(x) [DRM_DP_SIDEBAND_TX_ ## x] = #x
+
+static const char *drm_dp_mst_sideband_tx_state_str(int state)
+{
+ static const char * const sideband_reason_str[] = {
+ DP_STR(QUEUED),
+ DP_STR(START_SEND),
+ DP_STR(SENT),
+ DP_STR(RX),
+ DP_STR(TIMEOUT),
+ };
+
+ if (state >= ARRAY_SIZE(sideband_reason_str) ||
+ !sideband_reason_str[state])
+ return "unknown";
+
+ return sideband_reason_str[state];
+}
+
+static int
+drm_dp_mst_rad_to_str(const u8 rad[8], u8 lct, char *out, size_t len)
+{
+ int i;
+ u8 unpacked_rad[16];
+
+ for (i = 0; i < lct; i++) {
+ if (i % 2)
+ unpacked_rad[i] = rad[i / 2] >> 4;
+ else
+ unpacked_rad[i] = rad[i / 2] & BIT_MASK(4);
+ }
+
+ /* TODO: Eventually add something to printk so we can format the rad
+* like this: 1.2.3
+*/
+ return snprintf(out, len, "%*phC", lct, unpacked_rad);
+}
/* sideband msg handling */
static u8 drm_dp_msg_header_crc4(const uint8_t *data, size_t num_nibbles)
@@ -261,8 +301,9 @@ static bool drm_dp_decode_sideband_msg_hdr(struct
drm_dp_sideband_msg_hdr *hdr,
return true;
}
-static void drm_dp_encode_sideband_req(struct drm_dp_sideband_msg_req_body
*req,
- struct drm_dp_sideband_msg_tx *raw)
+void
+drm_dp_encode_sideband_req(const struct drm_dp_sideband_msg_req_body *req,
+ struct drm_dp_sideband_msg_tx *raw)
{
int idx = 0;
int i;
@@ -363,6 +404,251 @@ static void drm_dp_encode_sideband_req(struct
drm_dp_sideband_msg_req_body *req,
}
raw->cur_len = idx;
}
+EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_dp_encode_sideband_req);
+
+/* Decode a sideband request we've encoded, mainly used for debugging */
+int
+drm_dp_decode_sideband_req(const struct drm_dp_sideband_msg_tx *raw,
+ struct drm_dp_sideband_msg_req_body *req)
+{
+ const u8 *buf = raw->msg;
+ int i, idx = 0;
+
+ req->req_type = buf[idx++] & 0x7f;
+ switch (req->req_type) {
+ case DP_ENUM_PATH_RESOURCES:
+ case DP_POWER_DOWN_PHY:
+ case DP_POWER_UP_PHY:
+ req->u.port_num.port_number = (buf[idx] >> 4) & 0xf;
+ break;
+ case DP_ALLOCATE_PAYLOAD:
+ {
+ struct drm_dp_allocate_payload *a =
+ >u.allocate_payload;
+
+
[PATCH v2 08/27] drm/dp_mst: Remove PDT teardown in drm_dp_destroy_port() and refactor
This will allow us to add some locking for port->* members, in
particular the PDT and ->connector, which can't be done from
drm_dp_destroy_port() since we don't know what locks the caller might be
holding.
Changes since v2:
* Clarify commit message
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 40 +++
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index f5f1d8b50fb6..af3189df28aa 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1511,31 +1511,22 @@ static void drm_dp_destroy_port(struct kref *kref)
container_of(kref, struct drm_dp_mst_port, topology_kref);
struct drm_dp_mst_topology_mgr *mgr = port->mgr;
- if (!port->input) {
- kfree(port->cached_edid);
+ /* There's nothing that needs locking to destroy an input port yet */
+ if (port->input) {
+ drm_dp_mst_put_port_malloc(port);
+ return;
+ }
- /*
-* The only time we don't have a connector
-* on an output port is if the connector init
-* fails.
-*/
- if (port->connector) {
- /* we can't destroy the connector here, as
-* we might be holding the mode_config.mutex
-* from an EDID retrieval */
+ kfree(port->cached_edid);
- mutex_lock(>delayed_destroy_lock);
- list_add(>next, >destroy_port_list);
- mutex_unlock(>delayed_destroy_lock);
- schedule_work(>delayed_destroy_work);
- return;
- }
- /* no need to clean up vcpi
-* as if we have no connector we never setup a vcpi */
- drm_dp_port_teardown_pdt(port, port->pdt);
- port->pdt = DP_PEER_DEVICE_NONE;
- }
- drm_dp_mst_put_port_malloc(port);
+ /*
+* we can't destroy the connector here, as we might be holding the
+* mode_config.mutex from an EDID retrieval
+*/
+ mutex_lock(>delayed_destroy_lock);
+ list_add(>next, >destroy_port_list);
+ mutex_unlock(>delayed_destroy_lock);
+ schedule_work(>delayed_destroy_work);
}
/**
@@ -3998,7 +3989,8 @@ static void drm_dp_tx_work(struct work_struct *work)
static inline void
drm_dp_delayed_destroy_port(struct drm_dp_mst_port *port)
{
- port->mgr->cbs->destroy_connector(port->mgr, port->connector);
+ if (port->connector)
+ port->mgr->cbs->destroy_connector(port->mgr, port->connector);
drm_dp_port_teardown_pdt(port, port->pdt);
port->pdt = DP_PEER_DEVICE_NONE;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 16/27] drm/dp_mst: Refactor pdt setup/teardown, add more locking
Since we're going to be implementing suspend/resume reprobing very soon,
we need to make sure we are extra careful to ensure that our locking
actually protects the topology state where we expect it to. Turns out
this isn't the case with drm_dp_port_setup_pdt() and
drm_dp_port_teardown_pdt(), both of which change port->mstb without
grabbing >lock.
Additionally, since most callers of these functions are just using it to
teardown the port's previous PDT and setup a new one we can simplify
things a bit and combine drm_dp_port_setup_pdt() and
drm_dp_port_teardown_pdt() into a single function:
drm_dp_port_set_pdt(). This function also handles actually ensuring that
we grab the correct locks when we need to modify port->mstb.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 181 +++---
include/drm/drm_dp_mst_helper.h | 6 +-
2 files changed, 110 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index d1610434a0cb..9944ef2ce885 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1487,24 +1487,6 @@ drm_dp_mst_topology_put_mstb(struct drm_dp_mst_branch
*mstb)
kref_put(>topology_kref, drm_dp_destroy_mst_branch_device);
}
-static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt)
-{
- struct drm_dp_mst_branch *mstb;
-
- switch (old_pdt) {
- case DP_PEER_DEVICE_DP_LEGACY_CONV:
- case DP_PEER_DEVICE_SST_SINK:
- /* remove i2c over sideband */
- drm_dp_mst_unregister_i2c_bus(>aux);
- break;
- case DP_PEER_DEVICE_MST_BRANCHING:
- mstb = port->mstb;
- port->mstb = NULL;
- drm_dp_mst_topology_put_mstb(mstb);
- break;
- }
-}
-
static void drm_dp_destroy_port(struct kref *kref)
{
struct drm_dp_mst_port *port =
@@ -1714,38 +1696,79 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port
*port,
return parent_lct + 1;
}
-/*
- * return sends link address for new mstb
- */
-static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
+static int drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt)
{
- int ret;
- u8 rad[6], lct;
- bool send_link = false;
+ struct drm_dp_mst_topology_mgr *mgr = port->mgr;
+ struct drm_dp_mst_branch *mstb;
+ u8 rad[8], lct;
+ int ret = 0;
+
+ if (port->pdt == new_pdt)
+ return 0;
+
+ /* Teardown the old pdt, if there is one */
+ switch (port->pdt) {
+ case DP_PEER_DEVICE_DP_LEGACY_CONV:
+ case DP_PEER_DEVICE_SST_SINK:
+ /*
+* If the new PDT would also have an i2c bus, don't bother
+* with reregistering it
+*/
+ if (new_pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
+ new_pdt == DP_PEER_DEVICE_SST_SINK) {
+ port->pdt = new_pdt;
+ return 0;
+ }
+
+ /* remove i2c over sideband */
+ drm_dp_mst_unregister_i2c_bus(>aux);
+ break;
+ case DP_PEER_DEVICE_MST_BRANCHING:
+ mutex_lock(>lock);
+ drm_dp_mst_topology_put_mstb(port->mstb);
+ port->mstb = NULL;
+ mutex_unlock(>lock);
+ break;
+ }
+
+ port->pdt = new_pdt;
switch (port->pdt) {
case DP_PEER_DEVICE_DP_LEGACY_CONV:
case DP_PEER_DEVICE_SST_SINK:
/* add i2c over sideband */
ret = drm_dp_mst_register_i2c_bus(>aux);
break;
+
case DP_PEER_DEVICE_MST_BRANCHING:
lct = drm_dp_calculate_rad(port, rad);
+ mstb = drm_dp_add_mst_branch_device(lct, rad);
+ if (!mstb) {
+ ret = -ENOMEM;
+ DRM_ERROR("Failed to create MSTB for port %p", port);
+ goto out;
+ }
- port->mstb = drm_dp_add_mst_branch_device(lct, rad);
- if (port->mstb) {
- port->mstb->mgr = port->mgr;
- port->mstb->port_parent = port;
- /*
-* Make sure this port's memory allocation stays
-* around until its child MSTB releases it
-*/
- drm_dp_mst_get_port_malloc(port);
+ mutex_lock(>lock);
+ port->mstb = mstb;
+ mstb->mgr = port->mgr;
+ mstb->port_parent = port;
- send_link = true;
- }
+ /*
+* Make sure this port's memory allocation stays
+* around until its child MSTB
[PATCH v2 11/27] drm/dp_mst: Constify guid in drm_dp_get_mst_branch_by_guid()
And it's helper, we'll be using this in just a moment.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 43452872efad..b44d3696c09a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2060,7 +2060,7 @@ static struct drm_dp_mst_branch
*drm_dp_get_mst_branch_device(struct drm_dp_mst_
static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
struct drm_dp_mst_branch *mstb,
- uint8_t *guid)
+ const uint8_t *guid)
{
struct drm_dp_mst_branch *found_mstb;
struct drm_dp_mst_port *port;
@@ -2084,7 +2084,7 @@ static struct drm_dp_mst_branch
*get_mst_branch_device_by_guid_helper(
static struct drm_dp_mst_branch *
drm_dp_get_mst_branch_device_by_guid(struct drm_dp_mst_topology_mgr *mgr,
-uint8_t *guid)
+const uint8_t *guid)
{
struct drm_dp_mst_branch *mstb;
int ret;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 18/27] drm/dp_mst: Remove lies in {up, down}_rep_recv documentation
These are most certainly accessed from far more than the mgr work. In
fact, up_req_recv is -only- ever accessed from outside the mgr work.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
include/drm/drm_dp_mst_helper.h | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index f253ee43e9d9..8ba2a01324bb 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -489,15 +489,11 @@ struct drm_dp_mst_topology_mgr {
int conn_base_id;
/**
-* @down_rep_recv: Message receiver state for down replies. This and
-* @up_req_recv are only ever access from the work item, which is
-* serialised.
+* @down_rep_recv: Message receiver state for down replies.
*/
struct drm_dp_sideband_msg_rx down_rep_recv;
/**
-* @up_req_recv: Message receiver state for up requests. This and
-* @down_rep_recv are only ever access from the work item, which is
-* serialised.
+* @up_req_recv: Message receiver state for up requests.
*/
struct drm_dp_sideband_msg_rx up_req_recv;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 13/27] drm/dp_mst: Refactor drm_dp_mst_handle_down_rep()
* Remove the big ugly have_eomt conditional
* Store >down_rep_recv.initial_hdr in a var to make line wrapping
easier
* Remove duplicate memset() calls
* Actually wrap lines
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Reviewed-by: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 102 +-
1 file changed, 50 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 64487098158a..74161f442584 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3179,68 +3179,66 @@ static bool drm_dp_get_one_sb_msg(struct
drm_dp_mst_topology_mgr *mgr, bool up)
static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr)
{
- int ret = 0;
+ struct drm_dp_sideband_msg_tx *txmsg;
+ struct drm_dp_mst_branch *mstb;
+ struct drm_dp_sideband_msg_hdr *hdr = >down_rep_recv.initial_hdr;
+ int slot = -1;
+
+ if (!drm_dp_get_one_sb_msg(mgr, false))
+ goto clear_down_rep_recv;
- if (!drm_dp_get_one_sb_msg(mgr, false)) {
- memset(>down_rep_recv, 0,
- sizeof(struct drm_dp_sideband_msg_rx));
+ if (!mgr->down_rep_recv.have_eomt)
return 0;
+
+ mstb = drm_dp_get_mst_branch_device(mgr, hdr->lct, hdr->rad);
+ if (!mstb) {
+ DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
+ hdr->lct);
+ goto clear_down_rep_recv;
}
- if (mgr->down_rep_recv.have_eomt) {
- struct drm_dp_sideband_msg_tx *txmsg;
- struct drm_dp_mst_branch *mstb;
- int slot = -1;
- mstb = drm_dp_get_mst_branch_device(mgr,
-
mgr->down_rep_recv.initial_hdr.lct,
-
mgr->down_rep_recv.initial_hdr.rad);
+ /* find the message */
+ slot = hdr->seqno;
+ mutex_lock(>qlock);
+ txmsg = mstb->tx_slots[slot];
+ /* remove from slots */
+ mutex_unlock(>qlock);
- if (!mstb) {
- DRM_DEBUG_KMS("Got MST reply from unknown device %d\n",
mgr->down_rep_recv.initial_hdr.lct);
- memset(>down_rep_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
- return 0;
- }
+ if (!txmsg) {
+ DRM_DEBUG_KMS("Got MST reply with no msg %p %d %d %02x %02x\n",
+ mstb, hdr->seqno, hdr->lct, hdr->rad[0],
+ mgr->down_rep_recv.msg[0]);
+ goto no_msg;
+ }
- /* find the message */
- slot = mgr->down_rep_recv.initial_hdr.seqno;
- mutex_lock(>qlock);
- txmsg = mstb->tx_slots[slot];
- /* remove from slots */
- mutex_unlock(>qlock);
-
- if (!txmsg) {
- DRM_DEBUG_KMS("Got MST reply with no msg %p %d %d %02x
%02x\n",
- mstb,
- mgr->down_rep_recv.initial_hdr.seqno,
- mgr->down_rep_recv.initial_hdr.lct,
- mgr->down_rep_recv.initial_hdr.rad[0],
- mgr->down_rep_recv.msg[0]);
- drm_dp_mst_topology_put_mstb(mstb);
- memset(>down_rep_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
- return 0;
- }
+ drm_dp_sideband_parse_reply(>down_rep_recv, >reply);
- drm_dp_sideband_parse_reply(>down_rep_recv, >reply);
+ if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK)
+ DRM_DEBUG_KMS("Got NAK reply: req 0x%02x (%s), reason 0x%02x
(%s), nak data 0x%02x\n",
+ txmsg->reply.req_type,
+ drm_dp_mst_req_type_str(txmsg->reply.req_type),
+ txmsg->reply.u.nak.reason,
+
drm_dp_mst_nak_reason_str(txmsg->reply.u.nak.reason),
+ txmsg->reply.u.nak.nak_data);
- if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK)
- DRM_DEBUG_KMS("Got NAK reply: req 0x%02x (%s), reason
0x%02x (%s), nak data 0x%02x\n",
- txmsg->reply.req_type,
-
drm_dp_mst_req_type_str(txmsg->reply.req_type),
- txmsg->reply.u.nak.reason,
-
drm_dp_mst_nak_reason_str(txmsg->reply.u.nak.reason),
- txmsg->reply.u.nak.nak_data);
-
- memset(>down_rep_recv, 0, sizeof(struct
drm_dp_sideband_msg_rx));
-
[PATCH v2 09/27] drm/dp_mst: Refactor drm_dp_send_enum_path_resources
Use more pointers so we don't have to write out
txmsg->reply.u.path_resources each time. Also, fix line wrapping +
rearrange local variables.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Reviewed-by: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index af3189df28aa..241c66f75bed 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2437,12 +2437,14 @@ static void drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
kfree(txmsg);
}
-static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
- struct drm_dp_mst_branch *mstb,
- struct drm_dp_mst_port *port)
+static int
+drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_branch *mstb,
+ struct drm_dp_mst_port *port)
{
- int len;
+ struct drm_dp_enum_path_resources_ack_reply *path_res;
struct drm_dp_sideband_msg_tx *txmsg;
+ int len;
int ret;
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
@@ -2456,14 +2458,20 @@ static int drm_dp_send_enum_path_resources(struct
drm_dp_mst_topology_mgr *mgr,
ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
if (ret > 0) {
+ path_res = >reply.u.path_resources;
+
if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) {
DRM_DEBUG_KMS("enum path resources nak received\n");
} else {
- if (port->port_num !=
txmsg->reply.u.path_resources.port_number)
+ if (port->port_num != path_res->port_number)
DRM_ERROR("got incorrect port in response\n");
- DRM_DEBUG_KMS("enum path resources %d: %d %d\n",
txmsg->reply.u.path_resources.port_number,
txmsg->reply.u.path_resources.full_payload_bw_number,
-
txmsg->reply.u.path_resources.avail_payload_bw_number);
- port->available_pbn =
txmsg->reply.u.path_resources.avail_payload_bw_number;
+
+ DRM_DEBUG_KMS("enum path resources %d: %d %d\n",
+ path_res->port_number,
+ path_res->full_payload_bw_number,
+ path_res->avail_payload_bw_number);
+ port->available_pbn =
+ path_res->avail_payload_bw_number;
}
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 06/27] drm/dp_mst: Combine redundant cases in drm_dp_encode_sideband_req()
Noticed this while working on adding a drm_dp_decode_sideband_req().
DP_POWER_DOWN_PHY/DP_POWER_UP_PHY both use the same struct, so we can
just combine their cases.
Signed-off-by: Lyude Paul
Cc: Daniel Vetter
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
---
drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 6f7f449ca12b..1c862749cb63 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -271,6 +271,8 @@ static void drm_dp_encode_sideband_req(struct
drm_dp_sideband_msg_req_body *req,
switch (req->req_type) {
case DP_ENUM_PATH_RESOURCES:
+ case DP_POWER_DOWN_PHY:
+ case DP_POWER_UP_PHY:
buf[idx] = (req->u.port_num.port_number & 0xf) << 4;
idx++;
break;
@@ -358,12 +360,6 @@ static void drm_dp_encode_sideband_req(struct
drm_dp_sideband_msg_req_body *req,
memcpy([idx], req->u.i2c_write.bytes,
req->u.i2c_write.num_bytes);
idx += req->u.i2c_write.num_bytes;
break;
-
- case DP_POWER_DOWN_PHY:
- case DP_POWER_UP_PHY:
- buf[idx] = (req->u.port_num.port_number & 0xf) << 4;
- idx++;
- break;
}
raw->cur_len = idx;
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 04/27] drm/dp_mst: Move test_calc_pbn_mode() into an actual selftest
Yes, apparently we've been testing this for every single driver load for
quite a long time now. At least that means our PBN calculation is solid!
Anyway, introduce self tests for MST and move this into there.
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Reviewed-by: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 27 ---
drivers/gpu/drm/selftests/Makefile| 2 +-
.../gpu/drm/selftests/drm_modeset_selftests.h | 1 +
.../drm/selftests/test-drm_dp_mst_helper.c| 34 +++
.../drm/selftests/test-drm_modeset_common.h | 1 +
5 files changed, 37 insertions(+), 28 deletions(-)
create mode 100644 drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 738f260d4b15..6f7f449ca12b 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -47,7 +47,6 @@
*/
static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
char *buf);
-static int test_calc_pbn_mode(void);
static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port);
@@ -3561,30 +3560,6 @@ int drm_dp_calc_pbn_mode(int clock, int bpp)
}
EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
-static int test_calc_pbn_mode(void)
-{
- int ret;
- ret = drm_dp_calc_pbn_mode(154000, 30);
- if (ret != 689) {
- DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
expected PBN %d, actual PBN %d.\n",
- 154000, 30, 689, ret);
- return -EINVAL;
- }
- ret = drm_dp_calc_pbn_mode(234000, 30);
- if (ret != 1047) {
- DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
expected PBN %d, actual PBN %d.\n",
- 234000, 30, 1047, ret);
- return -EINVAL;
- }
- ret = drm_dp_calc_pbn_mode(297000, 24);
- if (ret != 1063) {
- DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
expected PBN %d, actual PBN %d.\n",
- 297000, 24, 1063, ret);
- return -EINVAL;
- }
- return 0;
-}
-
/* we want to kick the TX after we've ack the up/down IRQs. */
static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr)
{
@@ -4033,8 +4008,6 @@ int drm_dp_mst_topology_mgr_init(struct
drm_dp_mst_topology_mgr *mgr,
if (!mgr->proposed_vcpis)
return -ENOMEM;
set_bit(0, >payload_mask);
- if (test_calc_pbn_mode() < 0)
- DRM_ERROR("MST PBN self-test failed\n");
mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
if (mst_state == NULL)
diff --git a/drivers/gpu/drm/selftests/Makefile
b/drivers/gpu/drm/selftests/Makefile
index aae88f8a016c..d2137342b371 100644
--- a/drivers/gpu/drm/selftests/Makefile
+++ b/drivers/gpu/drm/selftests/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o \
test-drm_format.o test-drm_framebuffer.o \
- test-drm_damage_helper.o
+ test-drm_damage_helper.o test-drm_dp_mst_helper.o
obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o
test-drm_cmdline_parser.o
diff --git a/drivers/gpu/drm/selftests/drm_modeset_selftests.h
b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
index 464753746013..dec3ee3ec96f 100644
--- a/drivers/gpu/drm/selftests/drm_modeset_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
@@ -32,3 +32,4 @@ selftest(damage_iter_damage_one_intersect,
igt_damage_iter_damage_one_intersect)
selftest(damage_iter_damage_one_outside, igt_damage_iter_damage_one_outside)
selftest(damage_iter_damage_src_moved, igt_damage_iter_damage_src_moved)
selftest(damage_iter_damage_not_visible, igt_damage_iter_damage_not_visible)
+selftest(dp_mst_calc_pbn_mode, igt_dp_mst_calc_pbn_mode)
diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
new file mode 100644
index ..9baa5171988d
--- /dev/null
+++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test cases for for the DRM DP MST helpers
+ */
+
+#include
+#include
+
+#include "test-drm_modeset_common.h"
+
+int igt_dp_mst_calc_pbn_mode(void *ignored)
+{
+ int pbn, i;
+ const struct {
+ int rate;
+ int bpp;
+ int expected;
+ } test_params[] = {
+ { 154000, 30, 689 },
+ { 234000, 30, 1047 },
+ { 297000, 24, 1063 },
+ };
+
+ for (i = 0; i < ARRAY_SIZE(test_params); i++) {
+ pbn = drm_dp_calc_pbn_mode(test_params[i].rate,
+
[PATCH v2 03/27] drm/dp_mst: Destroy MSTBs asynchronously
When reprobing an MST topology during resume, we have to account for the
fact that while we were suspended it's possible that mstbs may have been
removed from any ports in the topology. Since iterating downwards in the
topology requires that we hold >lock, destroying MSTBs from this
context would result in attempting to lock >lock a second time and
deadlocking.
So, fix this by first moving destruction of MSTBs into
destroy_connector_work, then rename destroy_connector_work and friends
to reflect that they now destroy both ports and mstbs.
Changes since v1:
* s/destroy_connector_list/destroy_port_list/
s/connector_destroy_lock/delayed_destroy_lock/
s/connector_destroy_work/delayed_destroy_work/
s/drm_dp_finish_destroy_branch_device/drm_dp_delayed_destroy_mstb/
s/drm_dp_finish_destroy_port/drm_dp_delayed_destroy_port/
- danvet
* Use two loops in drm_dp_delayed_destroy_work() - danvet
* Better explain why we need to do this - danvet
* Use cancel_work_sync() instead of flush_work() - flush_work() doesn't
account for work requeing
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 162 +-
include/drm/drm_dp_mst_helper.h | 26 +++--
2 files changed, 127 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 3054ec622506..738f260d4b15 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1113,34 +1113,17 @@ static void drm_dp_destroy_mst_branch_device(struct
kref *kref)
struct drm_dp_mst_branch *mstb =
container_of(kref, struct drm_dp_mst_branch, topology_kref);
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
- struct drm_dp_mst_port *port, *tmp;
- bool wake_tx = false;
- mutex_lock(>lock);
- list_for_each_entry_safe(port, tmp, >ports, next) {
- list_del(>next);
- drm_dp_mst_topology_put_port(port);
- }
- mutex_unlock(>lock);
-
- /* drop any tx slots msg */
- mutex_lock(>mgr->qlock);
- if (mstb->tx_slots[0]) {
- mstb->tx_slots[0]->state = DRM_DP_SIDEBAND_TX_TIMEOUT;
- mstb->tx_slots[0] = NULL;
- wake_tx = true;
- }
- if (mstb->tx_slots[1]) {
- mstb->tx_slots[1]->state = DRM_DP_SIDEBAND_TX_TIMEOUT;
- mstb->tx_slots[1] = NULL;
- wake_tx = true;
- }
- mutex_unlock(>mgr->qlock);
+ INIT_LIST_HEAD(>destroy_next);
- if (wake_tx)
- wake_up_all(>mgr->tx_waitq);
-
- drm_dp_mst_put_mstb_malloc(mstb);
+ /*
+* This can get called under mgr->mutex, so we need to perform the
+* actual destruction of the mstb in another worker
+*/
+ mutex_lock(>delayed_destroy_lock);
+ list_add(>destroy_next, >destroy_branch_device_list);
+ mutex_unlock(>delayed_destroy_lock);
+ schedule_work(>delayed_destroy_work);
}
/**
@@ -1255,10 +1238,10 @@ static void drm_dp_destroy_port(struct kref *kref)
* we might be holding the mode_config.mutex
* from an EDID retrieval */
- mutex_lock(>destroy_connector_lock);
- list_add(>next, >destroy_connector_list);
- mutex_unlock(>destroy_connector_lock);
- schedule_work(>destroy_connector_work);
+ mutex_lock(>delayed_destroy_lock);
+ list_add(>next, >destroy_port_list);
+ mutex_unlock(>delayed_destroy_lock);
+ schedule_work(>delayed_destroy_work);
return;
}
/* no need to clean up vcpi
@@ -2792,7 +2775,7 @@ void drm_dp_mst_topology_mgr_suspend(struct
drm_dp_mst_topology_mgr *mgr)
DP_MST_EN | DP_UPSTREAM_IS_SRC);
mutex_unlock(>lock);
flush_work(>work);
- flush_work(>destroy_connector_work);
+ flush_work(>delayed_destroy_work);
}
EXPORT_SYMBOL(drm_dp_mst_topology_mgr_suspend);
@@ -3740,34 +3723,104 @@ static void drm_dp_tx_work(struct work_struct *work)
mutex_unlock(>qlock);
}
-static void drm_dp_destroy_connector_work(struct work_struct *work)
+static inline void
+drm_dp_delayed_destroy_port(struct drm_dp_mst_port *port)
{
- struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct
drm_dp_mst_topology_mgr, destroy_connector_work);
- struct drm_dp_mst_port *port;
- bool send_hotplug = false;
+ port->mgr->cbs->destroy_connector(port->mgr, port->connector);
+
+ drm_dp_port_teardown_pdt(port, port->pdt);
+ port->pdt = DP_PEER_DEVICE_NONE;
+
+ drm_dp_mst_put_port_malloc(port);
+}
+
+static inline void
+drm_dp_delayed_destroy_mstb(struct
[PATCH v2 02/27] drm/dp_mst: Get rid of list clear in destroy_connector_work
This seems to be some leftover detritus from before the port/mstb kref cleanup and doesn't do anything anymore, so get rid of it. Cc: Juston Li Cc: Imre Deak Cc: Ville Syrjälä Cc: Harry Wentland Reviewed-by: Daniel Vetter Signed-off-by: Lyude Paul --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 36db66a0ddb1..3054ec622506 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3760,8 +3760,6 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) list_del(>next); mutex_unlock(>destroy_connector_lock); - INIT_LIST_HEAD(>next); - mgr->cbs->destroy_connector(mgr, port->connector); drm_dp_port_teardown_pdt(port, port->pdt); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 05/27] drm/print: Add drm_err_printer()
A simple convienence function that returns a drm_printer which prints
using pr_err()
Changes since v1:
* Make __drm_printfn_err() more consistent with DRM_ERROR() - danvet
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Reviewed-by: Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_print.c | 6 ++
include/drm/drm_print.h | 17 +
2 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index a17c8a14dba4..ad302d71 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -147,6 +147,12 @@ void __drm_printfn_debug(struct drm_printer *p, struct
va_format *vaf)
}
EXPORT_SYMBOL(__drm_printfn_debug);
+void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
+{
+ pr_err("*ERROR* %s %pV", p->prefix, vaf);
+}
+EXPORT_SYMBOL(__drm_printfn_err);
+
/**
* drm_puts - print a const string to a _printer stream
* @p: the printer
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a5d6f2f3e430..112165d3195d 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -83,6 +83,7 @@ void __drm_printfn_seq_file(struct drm_printer *p, struct
va_format *vaf);
void __drm_puts_seq_file(struct drm_printer *p, const char *str);
void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf);
void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
+void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
__printf(2, 3)
void drm_printf(struct drm_printer *p, const char *f, ...);
@@ -227,6 +228,22 @@ static inline struct drm_printer drm_debug_printer(const
char *prefix)
return p;
}
+/**
+ * drm_err_printer - construct a _printer that outputs to pr_err()
+ * @prefix: debug output prefix
+ *
+ * RETURNS:
+ * The _printer object
+ */
+static inline struct drm_printer drm_err_printer(const char *prefix)
+{
+ struct drm_printer p = {
+ .printfn = __drm_printfn_err,
+ .prefix = prefix
+ };
+ return p;
+}
+
/*
* The following categories are defined:
*
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 00/27] DP MST Refactors + debugging tools + suspend/resume reprobing
This is the large series for adding MST suspend/resume reprobing that
I've been working on for quite a while now. In addition, I:
- Refactored and cleaned up any code I ended up digging through in the
process of understanding how some parts of these helpers worked.
- Added some debugging tools along the way that I ended up needing to
figure out some issues in my own code
Note that there's still one important part of this process missing
that's not included in this patch series: EDID reprobing, which I
believe Stanislav Lisovskiy from Intel is currently working on. The main
purpose of this series is to fix the issue of the in-memory topology
state (e.g. connectors connected to an MST hub, branch devices, etc.)
going out of sync if a topology connected to a connector is swapped out
with a different topology while the system is resumed, or while the
device housing said connector is in runtime suspend.
As well, the debugging tools that are added in this include:
- A limited debugging utility for dumping the list of topology
references on an MST port or branch connector whose topology reference
count has reached 0
- Sideband down request dumping, along with some basic selftests for
testing our encoding/decoding functions
Patchseries wide changes since v1
- Add "Combine redundant cases in drm_dp_encode_sideband_req()" to
fulfill some of the danvet's review requests
Lyude Paul (27):
drm/dp_mst: Move link address dumping into a function
drm/dp_mst: Get rid of list clear in destroy_connector_work
drm/dp_mst: Destroy MSTBs asynchronously
drm/dp_mst: Move test_calc_pbn_mode() into an actual selftest
drm/print: Add drm_err_printer()
drm/dp_mst: Combine redundant cases in drm_dp_encode_sideband_req()
drm/dp_mst: Add sideband down request tracing + selftests
drm/dp_mst: Remove PDT teardown in drm_dp_destroy_port() and refactor
drm/dp_mst: Refactor drm_dp_send_enum_path_resources
drm/dp_mst: Remove huge conditional in drm_dp_mst_handle_up_req()
drm/dp_mst: Constify guid in drm_dp_get_mst_branch_by_guid()
drm/dp_mst: Refactor drm_dp_mst_handle_up_req()
drm/dp_mst: Refactor drm_dp_mst_handle_down_rep()
drm/dp_mst: Destroy topology_mgr mutexes
drm/dp_mst: Cleanup drm_dp_send_link_address() a bit
drm/dp_mst: Refactor pdt setup/teardown, add more locking
drm/dp_mst: Rename drm_dp_add_port and drm_dp_update_port
drm/dp_mst: Remove lies in {up,down}_rep_recv documentation
drm/dp_mst: Handle UP requests asynchronously
drm/dp_mst: Protect drm_dp_mst_port members with connection_mutex
drm/dp_mst: Don't forget to update port->input in
drm_dp_mst_handle_conn_stat()
drm/nouveau: Don't grab runtime PM refs for HPD IRQs
drm/amdgpu: Iterate through DRM connectors correctly
drm/amdgpu/dm: Resume short HPD IRQs before resuming MST topology
drm/dp_mst: Add basic topology reprobing when resuming
drm/dp_mst: Also print unhashed pointers for malloc/topology
references
drm/dp_mst: Add topology ref history tracking for debugging
drivers/gpu/drm/Kconfig | 14 +
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 13 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 20 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 40 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c |5 +-
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 34 +-
drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 34 +-
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 40 +-
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 34 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 +-
.../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 10 +-
drivers/gpu/drm/drm_dp_mst_topology.c | 1633 +
.../gpu/drm/drm_dp_mst_topology_internal.h| 24 +
drivers/gpu/drm/drm_print.c |6 +
drivers/gpu/drm/i915/display/intel_dp.c |3 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c |6 +-
drivers/gpu/drm/nouveau/nouveau_connector.c | 33 +-
drivers/gpu/drm/selftests/Makefile|2 +-
.../gpu/drm/selftests/drm_modeset_selftests.h |2 +
.../drm/selftests/test-drm_dp_mst_helper.c| 238 +++
.../drm/selftests/test-drm_modeset_common.h |2 +
include/drm/drm_dp_mst_helper.h | 143 +-
include/drm/drm_print.h | 17 +
24 files changed, 1873 insertions(+), 526 deletions(-)
create mode 100644 drivers/gpu/drm/drm_dp_mst_topology_internal.h
create mode 100644 drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] x86/mm: Export force_dma_unencrypted
On 9/3/19 6:22 PM, Christoph Hellwig wrote: On Tue, Sep 03, 2019 at 04:32:45PM +0200, Thomas Hellström (VMware) wrote: Is this a layer violation concern, that is, would you be ok with a similar helper for TTM, or is it that you want to force the graphics drivers into adhering strictly to the DMA api, even when it from an engineering perspective makes no sense? >From looking at DRM I strongly believe that making DRM use the DMA mapping properly makes a lot of sense from the engineering perspective, and this series is a good argument for that positions. What I mean with "from an engineering perspective" is that drivers would end up with a non-trivial amount of code supporting purely academic cases: Setups where software rendering would be faster than gpu accelerated, and setups on platforms where the driver would never run anyway because the device would never be supported on that platform... If DRM was using the DMA properl we would not need this series to start with, all the SEV handling is hidden behind the DMA API. While we had occasional bugs in that support fixing it meant that it covered all drivers properly using that API. That is not really true. The dma API can't handle faulting of coherent pages which is what this series is really all about supporting also with SEV active. To handle the case where we move graphics buffers or send them to swap space while user-space have them mapped. To do that and still be fully dma-api compliant we would ideally need, for example, an exported dma_pgprot(). (dma_pgprot() by the way is still suffering from one of the bugs that you mention above). Still, I need a way forward and my questions weren't really answered by this. /Thomas
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #64 from tempel.jul...@gmail.com --- I can try that. But I really wonder why there are differences between systems showing the issue or not. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v4 01/16] drm: Add drm_minor_for_each
On Tue, Sep 3, 2019 at 4:12 PM Daniel Vetter wrote:
> On Tue, Sep 3, 2019 at 9:45 PM Kenny Ho wrote:
> > On Tue, Sep 3, 2019 at 3:57 AM Daniel Vetter wrote:
> > > Iterating over minors for cgroups sounds very, very wrong. Why do we care
> > > whether a buffer was allocated through kms dumb vs render nodes?
> > >
> > > I'd expect all the cgroup stuff to only work on drm_device, if it does
> > > care about devices.
> > >
> > > (I didn't look through the patch series to find out where exactly you're
> > > using this, so maybe I'm off the rails here).
> >
> > I am exposing this to remove the need to keep track of a separate list
> > of available drm_device in the system (to remove the registering and
> > unregistering of drm_device to the cgroup subsystem and just use
> > drm_minor as the single source of truth.) I am only filtering out the
> > render nodes minor because they point to the same drm_device and is
> > confusing.
> >
> > Perhaps I missed an obvious way to list the drm devices without
> > iterating through the drm_minors? (I probably jumped to the minors
> > because $major:$minor is the convention to address devices in cgroup.)
>
> Create your own if there's nothing, because you need to anyway:
> - You need special locking anyway, we can't just block on the idr lock
> for everything.
> - This needs to refcount drm_device, no the minors.
>
> Iterating over stuff still feels kinda wrong still, because normally
> the way we register/unregister userspace api (and cgroups isn't
> anything else from a drm driver pov) is by adding more calls to
> drm_dev_register/unregister. If you put a drm_cg_register/unregister
> call in there we have a clean separation, and you can track all the
> currently active devices however you want. Iterating over objects that
> can be hotunplugged any time tends to get really complicated really
> quickly.
Um... I thought this is what I had previously. Did I misunderstood
your feedback from v3? Doesn't drm_minor already include all these
facilities so isn't creating my own kind of reinventing the wheel?
(as I did previously?) drm_minor_register is called inside
drm_dev_register so isn't leveraging existing drm_minor facilities
much better solution?
Kenny
>
>
> >
> > Kenny
> >
> > > -Daniel
> > >
> > > > ---
> > > > drivers/gpu/drm/drm_drv.c | 19 +++
> > > > drivers/gpu/drm/drm_internal.h | 4
> > > > include/drm/drm_drv.h | 4
> > > > 3 files changed, 23 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > > index 862621494a93..000cddabd970 100644
> > > > --- a/drivers/gpu/drm/drm_drv.c
> > > > +++ b/drivers/gpu/drm/drm_drv.c
> > > > @@ -254,11 +254,13 @@ struct drm_minor *drm_minor_acquire(unsigned int
> > > > minor_id)
> > > >
> > > > return minor;
> > > > }
> > > > +EXPORT_SYMBOL(drm_minor_acquire);
> > > >
> > > > void drm_minor_release(struct drm_minor *minor)
> > > > {
> > > > drm_dev_put(minor->dev);
> > > > }
> > > > +EXPORT_SYMBOL(drm_minor_release);
> > > >
> > > > /**
> > > > * DOC: driver instance overview
> > > > @@ -1078,6 +1080,23 @@ int drm_dev_set_unique(struct drm_device *dev,
> > > > const char *name)
> > > > }
> > > > EXPORT_SYMBOL(drm_dev_set_unique);
> > > >
> > > > +/**
> > > > + * drm_minor_for_each - Iterate through all stored DRM minors
> > > > + * @fn: Function to be called for each pointer.
> > > > + * @data: Data passed to callback function.
> > > > + *
> > > > + * The callback function will be called for each @drm_minor entry,
> > > > passing
> > > > + * the minor, the entry and @data.
> > > > + *
> > > > + * If @fn returns anything other than %0, the iteration stops and that
> > > > + * value is returned from this function.
> > > > + */
> > > > +int drm_minor_for_each(int (*fn)(int id, void *p, void *data), void
> > > > *data)
> > > > +{
> > > > + return idr_for_each(_minors_idr, fn, data);
> > > > +}
> > > > +EXPORT_SYMBOL(drm_minor_for_each);
> > > > +
> > > > /*
> > > > * DRM Core
> > > > * The DRM core module initializes all global DRM objects and makes
> > > > them
> > > > diff --git a/drivers/gpu/drm/drm_internal.h
> > > > b/drivers/gpu/drm/drm_internal.h
> > > > index e19ac7ca602d..6bfad76f8e78 100644
> > > > --- a/drivers/gpu/drm/drm_internal.h
> > > > +++ b/drivers/gpu/drm/drm_internal.h
> > > > @@ -54,10 +54,6 @@ void drm_prime_destroy_file_private(struct
> > > > drm_prime_file_private *prime_fpriv);
> > > > void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private
> > > > *prime_fpriv,
> > > > struct dma_buf *dma_buf);
> > > >
> > > > -/* drm_drv.c */
> > > > -struct drm_minor *drm_minor_acquire(unsigned int minor_id);
> > > > -void drm_minor_release(struct drm_minor *minor);
> > > > -
> > > > /* drm_vblank.c */
> > > > void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int
> > > > pipe);
> > > > void
[Bug 109389] memory leak in `amdgpu_bo_create()`
https://bugs.freedesktop.org/show_bug.cgi?id=109389 --- Comment #4 from Czcibor Bohusz-Dobosz --- Created attachment 145255 --> https://bugs.freedesktop.org/attachment.cgi?id=145255=edit Galactic Civilizations III memleak log without DXVK As far as I'm understanding the logs that I've gotten, this memory leak does still occur with Linux 5.2.11-arch1-1-ARCH and Mesa 1.9.15. In my case, it is most prevalent when a Direct3D game is launched with the use of Wine accompanied by the DXVK translation layer that converts the D3D calls to Vulkan - just going to a game's main menu can eat up large amounts of memory, which are then never freed, not even as the game is closed, until caches are manually dropped with a command. However, this seems to also occur to a much smaller extent with DXVK turned off; I attach a bcc memleak log that showcases the issue with the use of Galactic Civilizations III v3.9, as the smaller amounts of memory leaked when DXVK is not in use make tracing the exact call that permanently leaked memory easier - if I'm not anyhow mistaken, that would make it the one that leaked 68550656 bytes in this log. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/10] drm/msm: split power control from prepare/complete_commit
On Thu, Aug 29, 2019 at 09:45:15AM -0700, Rob Clark wrote:
> From: Rob Clark
>
> With atomic commit, ->prepare_commit() and ->complete_commit() may not
> be evenly balanced (although ->complete_commit() will complete each
> crtc that had been previously prepared). So these will no longer be
> a good place to enable/disable clocks needed for hw access.
>
> Signed-off-by: Rob Clark
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 17 ++---
> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 19 ++-
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 20 ++--
> drivers/gpu/drm/msm/msm_atomic.c | 2 ++
> drivers/gpu/drm/msm/msm_kms.h| 10 ++
> 5 files changed, 54 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index efbf8fd343de..d54741f3ad9f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -248,6 +248,18 @@ static void dpu_kms_disable_vblank(struct msm_kms *kms,
> struct drm_crtc *crtc)
> dpu_crtc_vblank(crtc, false);
> }
>
> +static void dpu_kms_enable_commit(struct msm_kms *kms)
> +{
> + struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> + pm_runtime_get_sync(_kms->pdev->dev);
> +}
> +
> +static void dpu_kms_disable_commit(struct msm_kms *kms)
> +{
> + struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> + pm_runtime_put_sync(_kms->pdev->dev);
> +}
> +
> static void dpu_kms_prepare_commit(struct msm_kms *kms,
> struct drm_atomic_state *state)
> {
> @@ -267,7 +279,6 @@ static void dpu_kms_prepare_commit(struct msm_kms *kms,
> if (!dev || !dev->dev_private)
> return;
> priv = dev->dev_private;
> - pm_runtime_get_sync(_kms->pdev->dev);
>
> /* Call prepare_commit for all affected encoders */
> for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> @@ -335,8 +346,6 @@ static void dpu_kms_complete_commit(struct msm_kms *kms,
> unsigned crtc_mask)
> for_each_crtc_mask(dpu_kms->dev, crtc, crtc_mask)
> dpu_crtc_complete_commit(crtc);
>
> - pm_runtime_put_sync(_kms->pdev->dev);
> -
> DPU_ATRACE_END("kms_complete_commit");
> }
>
> @@ -682,6 +691,8 @@ static const struct msm_kms_funcs kms_funcs = {
> .irq_preinstall = dpu_irq_preinstall,
> .irq_uninstall = dpu_irq_uninstall,
> .irq = dpu_irq,
> + .enable_commit = dpu_kms_enable_commit,
> + .disable_commit = dpu_kms_disable_commit,
> .prepare_commit = dpu_kms_prepare_commit,
> .flush_commit= dpu_kms_flush_commit,
> .commit = dpu_kms_commit,
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index 78ce2c8a9a38..500e5b08c11f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@ -93,15 +93,24 @@ static int mdp4_hw_init(struct msm_kms *kms)
> return ret;
> }
>
> -static void mdp4_prepare_commit(struct msm_kms *kms, struct drm_atomic_state
> *state)
> +static void mdp4_enable_commit(struct msm_kms *kms)
> +{
> + struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
> + mdp4_enable(mdp4_kms);
> +}
> +
> +static void mdp4_disable_commit(struct msm_kms *kms)
> {
> struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
> + mdp4_disable(mdp4_kms);
> +}
> +
> +static void mdp4_prepare_commit(struct msm_kms *kms, struct drm_atomic_state
> *state)
> +{
> int i;
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
>
> - mdp4_enable(mdp4_kms);
> -
> /* see 119ecb7fd */
> for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> drm_crtc_vblank_get(crtc);
> @@ -129,8 +138,6 @@ static void mdp4_complete_commit(struct msm_kms *kms,
> unsigned crtc_mask)
> /* see 119ecb7fd */
> for_each_crtc_mask(mdp4_kms->dev, crtc, crtc_mask)
> drm_crtc_vblank_put(crtc);
> -
> - mdp4_disable(mdp4_kms);
> }
>
> static long mdp4_round_pixclk(struct msm_kms *kms, unsigned long rate,
> @@ -182,6 +189,8 @@ static const struct mdp_kms_funcs kms_funcs = {
> .irq = mdp4_irq,
> .enable_vblank = mdp4_enable_vblank,
> .disable_vblank = mdp4_disable_vblank,
> + .enable_commit = mdp4_enable_commit,
> + .disable_commit = mdp4_disable_commit,
> .prepare_commit = mdp4_prepare_commit,
> .flush_commit= mdp4_flush_commit,
> .wait_flush = mdp4_wait_flush,
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index eff1b000258e..ba67bde1dbef 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -140,16 +140,25 @@ static int
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On 9/3/19 9:55 PM, Dave Hansen wrote: On 9/3/19 12:51 PM, Daniel Vetter wrote: The thing we need to stop is having mixed encryption rules under one VMA. The point here is that we want this. We need to be able to move the buffer between device ptes and system memory ptes, transparently, behind userspace back, without races. And the fast path (which is "no pte exists for this vma") must be real fast, so taking mmap_sem and replacing the vma is no-go. So, when the user asks for encryption and we say, "sure, we'll encrypt that", then we want the device driver to be able to transparently undo that encryption under the covers for device memory? That seems suboptimal. I'd rather the device driver just say: "Nope, you can't encrypt my VMA". Because that's the truth. The thing here is that it's the underlying physical memory that define the correct encryption flags. If it's DMA memory and SEV is active or PCI memory. It's always unencrypted. User-space in a SEV vm should always, from a data protection point of view, *assume* that graphics buffers are unencrypted. (Which will of course limit the use of gpus and display controllers in a SEV vm). Platform code sets the vma encryption to on by default. So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem? Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time. And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active). /Thomas
Re: [PATCH 05/10] drm/msm: convert kms->complete_commit() to crtc_mask
On Thu, Aug 29, 2019 at 09:45:13AM -0700, Rob Clark wrote:
> From: Rob Clark
>
> Prep work for async commits, in which case this will be called after we
> no longer have the atomic state object.
>
> This drops some wait_for_vblanks(), but those should be unnecessary, as
> we call this after waiting for flush to complete.
>
> Signed-off-by: Rob Clark
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 7 +--
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 4 +---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 20
> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 8 ++--
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 4 +---
> drivers/gpu/drm/msm/msm_atomic.c | 2 +-
> drivers/gpu/drm/msm/msm_kms.h| 2 +-
> 7 files changed, 11 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index e7354aef9805..31debd31ab8c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -389,13 +389,8 @@ static void dpu_crtc_frame_event_cb(void *data, u32
> event)
> kthread_queue_work(>event_thread[crtc_id].worker, >work);
> }
>
> -void dpu_crtc_complete_commit(struct drm_crtc *crtc,
> - struct drm_crtc_state *old_state)
> +void dpu_crtc_complete_commit(struct drm_crtc *crtc)
> {
> - if (!crtc || !crtc->state) {
> - DPU_ERROR("invalid crtc\n");
> - return;
> - }
> trace_dpu_crtc_complete_commit(DRMID(crtc));
> }
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 10f78459f6c2..5174e86124cc 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -244,10 +244,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc);
> /**
> * dpu_crtc_complete_commit - callback signalling completion of current
> commit
> * @crtc: Pointer to drm crtc object
> - * @old_state: Pointer to drm crtc old state object
> */
> -void dpu_crtc_complete_commit(struct drm_crtc *crtc,
> - struct drm_crtc_state *old_state);
> +void dpu_crtc_complete_commit(struct drm_crtc *crtc);
>
> /**
> * dpu_crtc_init - create a new crtc object
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index df421b986bc3..606815e50625 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -320,27 +320,15 @@ static void dpu_kms_commit(struct msm_kms *kms, struct
> drm_atomic_state *state)
> }
> }
>
> -static void dpu_kms_complete_commit(struct msm_kms *kms,
> - struct drm_atomic_state *old_state)
> +static void dpu_kms_complete_commit(struct msm_kms *kms, unsigned crtc_mask)
> {
> - struct dpu_kms *dpu_kms;
> - struct msm_drm_private *priv;
> + struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> struct drm_crtc *crtc;
> - struct drm_crtc_state *old_crtc_state;
> - int i;
> -
> - if (!kms || !old_state)
> - return;
> - dpu_kms = to_dpu_kms(kms);
> -
> - if (!dpu_kms->dev || !dpu_kms->dev->dev_private)
> - return;
> - priv = dpu_kms->dev->dev_private;
>
> DPU_ATRACE_BEGIN("kms_complete_commit");
>
> - for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i)
> - dpu_crtc_complete_commit(crtc, old_crtc_state);
> + for_each_crtc_mask(dpu_kms->dev, crtc, crtc_mask)
> + dpu_crtc_complete_commit(crtc);
>
> pm_runtime_put_sync(_kms->pdev->dev);
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index 32dcb1d7860c..a6a056df5878 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@ -116,17 +116,13 @@ static void mdp4_wait_flush(struct msm_kms *kms,
> unsigned crtc_mask)
> mdp4_crtc_wait_for_commit_done(crtc);
> }
>
> -static void mdp4_complete_commit(struct msm_kms *kms, struct
> drm_atomic_state *state)
> +static void mdp4_complete_commit(struct msm_kms *kms, unsigned crtc_mask)
> {
> struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
> - int i;
> struct drm_crtc *crtc;
> - struct drm_crtc_state *crtc_state;
> -
> - drm_atomic_helper_wait_for_vblanks(mdp4_kms->dev, state);
>
> /* see 119ecb7fd */
> - for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> + for_each_crtc_mask(mdp4_kms->dev, crtc, crtc_mask)
> drm_crtc_vblank_put(crtc);
>
> mdp4_disable(mdp4_kms);
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 440e000c8c3d..7a19526eef50 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -163,14 +163,12 @@ static void
Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
On 2019-09-03 20:08, Jernej Škrabec wrote:
> Hi!
>
> Dne torek, 03. september 2019 ob 20:00:33 CEST je Neil Armstrong napisal(a):
>> Hi,
>>
>> Le 03/09/2019 à 11:53, Neil Armstrong a écrit :
>>> Hi,
>>>
>>> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
From: Yakir Yang
When transmitting IEC60985 linear PCM audio, we configure the
Audio Sample Channel Status information of all the channel
status bits in the IEC60958 frame.
Refer to 60958-3 page 10 for frequency, original frequency, and
wordlength setting.
This fix the issue that audio does not come out on some monitors
(e.g. LG 22CV241)
Signed-off-by: Yakir Yang
Signed-off-by: Cheng-Yi Chiang
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++
drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20
2 files changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
bd65d0479683..34d46e25d610 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int
freq, unsigned long pixel_clk)>>
return n;
}
+static void hdmi_set_schnl(struct dw_hdmi *hdmi)
+{
+ u8 aud_schnl_samplerate;
+ u8 aud_schnl_8;
+
+ /* These registers are on RK3288 using version 2.0a. */
+ if (hdmi->version != 0x200a)
+ return;
>>> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
>>> SoCs ?
>> After investigations, Amlogic sets these registers on their 2.0a version
>> aswell, and Jernej (added in Cc) reported me Allwinner sets them on their
>> < 2.0a and > 2.0a IPs versions.
>>
>> Can you check on the Rockchip IP versions in RK3399 ?
>>
>> For reference, the HDMI 1.4a IP version allwinner setups is:
>> https://github.com/Allwinner-Homlet/H3-BSP4.4-linux/blob/master/drivers/vide
>> o/fbdev/sunxi/disp2/hdmi/hdmi_bsp_sun8iw7.c#L531-L539 (registers a
>> "scrambled" but a custom bit can reset to the original mapping, 0x1066 ...
>> 0x106f)
> For easier reading, here is similar, but annotated version: http://ix.io/1Ub6
> Check function bsp_hdmi_audio().
>
> Unless there is a special reason, you can just remove that check.
Agree, this check should not be needed, AUDSCHNLS7 used to be configured in my
old
multi-channel patches that have seen lot of testing on Amlogic, Allwinner and
Rockchip SoCs.
>
> Best regards,
> Jernej
>
>> Neil
>>
+
+ switch (hdmi->sample_rate) {
+ case 32000:
+ aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
+ break;
+ case 44100:
+ aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
+ break;
+ case 48000:
+ aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
+ break;
+ case 88200:
+ aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
+ break;
+ case 96000:
+ aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
+ break;
+ case 176400:
+ aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
+ break;
+ case 192000:
+ aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
+ break;
+ case 768000:
+ aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
+ break;
+ default:
+ dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
+ hdmi->sample_rate);
+ return;
+ }
+
+ /* set channel status register */
+ hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
+HDMI_FC_AUDSCHNLS7);
+
+ /*
+ * Set original frequency to be the same as frequency.
+ * Use one-complement value as stated in IEC60958-3 page 13.
+ */
+ aud_schnl_8 = (~aud_schnl_samplerate) <<
+ HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
+
+ /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
+ aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
This looks wrong, user can use 16 and 24 bit wide audio streams.
+
+ hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
+}
+
static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
unsigned long pixel_clk, unsigned int sample_rate)
{
@@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi
*hdmi,>>
hdmi->audio_cts = cts;
hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
spin_unlock_irq(>audio_lock);
+
+ hdmi_set_schnl(hdmi);
I will suggest this function is called from or merged with
Re: [PATCH] drm/virtio: Use vmalloc for command buffer allocations.
On Sun, Sep 1, 2019 at 10:28 PM Gerd Hoffmann wrote: > > On Fri, Aug 30, 2019 at 10:49:25AM -0700, David Riley wrote: > > Hi Gerd, > > > > On Fri, Aug 30, 2019 at 4:16 AM Gerd Hoffmann wrote: > > > > > > Hi, > > > > > > > > > - kfree(vbuf->data_buf); > > > > > > + kvfree(vbuf->data_buf); > > > > > > > > > > if (is_vmalloc_addr(vbuf->data_buf)) ... > > > > > > > > > > needed here I gues? > > > > > > > > > > > > > kvfree() handles vmalloc/kmalloc/kvmalloc internally by doing that > > > > check. > > > > > > Ok. > > > > > > > - videobuf_vmalloc_to_sg in drivers/media/v4l2-core/videobuf-dma-sg.c, > > > > assumes contiguous array of scatterlist and that the buffer being > > > > converted > > > > is page aligned > > > > > > Well, vmalloc memory _is_ page aligned. > > > > True, but this function gets called for all potential enqueuings (eg > > resource_create_3d, resource_attach_backing) and I was concerned that > > some other usage in the future might not have that guarantee. > > The vmalloc_to_sg call is wrapped into "if (is_vmalloc())", so this > should not be a problem. > > > > sg_alloc_table_from_pages() does alot of what you need, you just need a > > > small loop around vmalloc_to_page() create a struct page array > > > beforehand. > > > > That feels like an extra allocation when under memory pressure and > > more work, to not gain much -- there still needs to be a function that > > iterates through all the pages. But I don't feel super strongly about > > it and can change it if you think that it will be less maintenance > > overhead. > > Lets see how vmalloc_to_sg looks like when it assumes page-aligned > memory. It's probably noticeable shorter then. It's not really. The allocation of the table is one unit less, and doesn't need to take into account that data might be an offset within the page. It still needs error handling, partial final page handling, and marking of the end of the scatterlist. Things could be slightly simplified to assume that you can always get a contiguous allocation of the table instead of using sg_alloc_table/for_each_sg, but given that we're only going down this path when memory is fragmented and in a fallback, doesn't seem worthwhile to make that trade-off. I've written a different version of vmalloc_to_sgt which uses sg_alloc_table_from_pages under the covers and it comes in slightly shorter (39 lines vs 55 lines), but incurs another allocation as previously so I'm personally in favour of things as written. fpga_mgr_buf_load is another function which roughly does the same sort of operation and it's a bit longer. I'll post a v2 shortly, but if you think it's worth making the extra allocation of the pages array to use, I can post that instead. > cheers, > Gerd > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: gnome-shell stuck because of amdgpu driver [5.3 RC5]
On Tue, Sep 3, 2019 at 8:07 PM Mikhail Gavrilov wrote: > > On Tue, 3 Sep 2019 at 13:21, Hillf Danton wrote: > > > > Describe the problems you are experiencing please. > > Say is the screen locked up? Machine lockedup? > > Anything unnormal after you see the warning? > > > > According to my observations, all "gnome shell stuck warning" happened > when me not sitting on the computer and the computer was locked. > > I did not notice any problems at the morning (I did not even look at > the kernel logs), I found that the problem happened when I remotely > connected to my computer via ssh from work and accidently look dmesg > output. > > At the evening after work, I even played in the "Division", and still > not noted any problems. > > Now 11:01pm and "gnome shell stuck warning" not appear since 19:17. So > looks like issue happens only when computer blocked and monitor in > power save mode. I'd bet on runtime pm or some other power saving feature in amdgpu shutting the interrupt handling down before we've handled all the interrupts. That would then result in a stuck fence. Do we already know which fence is stuck? All the debuggin on the dma_fence_wait side is just looking at the messenger, this isn't the source of the problem. -Daniel > > $ dmesg -T | grep gnome > > ---> I am goto sleep > [Tue Sep 3 01:00:10 2019] gnome shell stuck warning > [Tue Sep 3 01:00:55 2019] gnome shell stuck warning > [Tue Sep 3 06:54:50 2019] gnome shell stuck warning > <--- I am wake up at 8:00 am and sitting again on the computer > ---> I am went to work at 9:30 > [Tue Sep 3 10:00:05 2019] gnome shell stuck warning > [Tue Sep 3 10:10:01 2019] gnome shell stuck warning > [Tue Sep 3 10:13:43 2019] gnome shell stuck warning > [Tue Sep 3 10:23:37 2019] gnome shell stuck warning > [Tue Sep 3 10:42:07 2019] gnome shell stuck warning > [Tue Sep 3 10:42:57 2019] gnome shell stuck warning > [Tue Sep 3 10:59:25 2019] gnome shell stuck warning > [Tue Sep 3 11:08:35 2019] gnome shell stuck warning > [Tue Sep 3 11:13:19 2019] gnome shell stuck warning > [Tue Sep 3 11:15:20 2019] gnome shell stuck warning > [Tue Sep 3 11:26:20 2019] gnome shell stuck warning > [Tue Sep 3 11:26:20 2019] gnome shell stuck warning > [Tue Sep 3 11:36:30 2019] gnome shell stuck warning > [Tue Sep 3 11:46:08 2019] gnome shell stuck warning > [Tue Sep 3 11:53:52 2019] gnome shell stuck warning > [Tue Sep 3 11:56:36 2019] gnome shell stuck warning > [Tue Sep 3 12:17:10 2019] gnome shell stuck warning > [Tue Sep 3 12:20:20 2019] gnome shell stuck warning > [Tue Sep 3 12:20:20 2019] gnome shell stuck warning > [Tue Sep 3 12:30:46 2019] gnome shell stuck warning > [Tue Sep 3 12:40:52 2019] gnome shell stuck warning > [Tue Sep 3 12:55:30 2019] gnome shell stuck warning > [Tue Sep 3 12:57:52 2019] gnome shell stuck warning > [Tue Sep 3 13:04:00 2019] gnome shell stuck warning > [Tue Sep 3 13:12:38 2019] gnome shell stuck warning > [Tue Sep 3 13:14:32 2019] gnome shell stuck warning > [Tue Sep 3 13:53:12 2019] gnome shell stuck warning > [Tue Sep 3 14:12:52 2019] gnome shell stuck warning > [Tue Sep 3 14:15:54 2019] gnome shell stuck warning > [Tue Sep 3 14:17:04 2019] gnome shell stuck warning > [Tue Sep 3 14:21:57 2019] gnome shell stuck warning > [Tue Sep 3 14:22:10 2019] gnome shell stuck warning > [Tue Sep 3 14:37:42 2019] gnome shell stuck warning > [Tue Sep 3 14:41:51 2019] gnome shell stuck warning > [Tue Sep 3 14:42:52 2019] gnome shell stuck warning > [Tue Sep 3 14:46:35 2019] gnome shell stuck warning > [Tue Sep 3 15:03:18 2019] gnome shell stuck warning > [Tue Sep 3 15:16:50 2019] gnome shell stuck warning > [Tue Sep 3 15:27:30 2019] gnome shell stuck warning > [Tue Sep 3 15:27:41 2019] gnome shell stuck warning > [Tue Sep 3 16:08:06 2019] gnome shell stuck warning > [Tue Sep 3 16:24:16 2019] gnome shell stuck warning > [Tue Sep 3 16:33:04 2019] gnome shell stuck warning > [Tue Sep 3 16:52:10 2019] gnome shell stuck warning > [Tue Sep 3 17:18:27 2019] gnome shell stuck warning > [Tue Sep 3 17:25:30 2019] gnome shell stuck warning > [Tue Sep 3 17:41:16 2019] gnome shell stuck warning > [Tue Sep 3 17:43:32 2019] gnome shell stuck warning > [Tue Sep 3 17:51:10 2019] gnome shell stuck warning > [Tue Sep 3 18:41:44 2019] gnome shell stuck warning > [Tue Sep 3 18:44:18 2019] gnome shell stuck warning > [Tue Sep 3 19:03:07 2019] gnome shell stuck warning > [Tue Sep 3 19:17:58 2019] gnome shell stuck warning > <--- Returned to home and sitting again on the computer > > -- > Best Regards, > Mike Gavrilov. -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment
On Tue, Sep 3, 2019 at 10:01 PM Sasha Levin wrote: > > On Tue, Sep 03, 2019 at 07:03:47PM +0200, Greg KH wrote: > >On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote: > >> On 2019-09-03 6:23 p.m., Sasha Levin wrote: > >> > From: Yu Zhao > >> > > >> > [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ] > >> > >> Hold your horses! > >> > >> This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be > >> reverted, as they caused regressions. See commits > >> 25ec429e86bb790e40387a550f0501d0ac55a47c & > >> 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 . > >> > >> > >> This isn't bolstering confidence in how these patches are selected... > > > >The patch _itself_ said to be backported to the stable trees from 4.2 > >and newer. Why wouldn't we be confident in doing this? > > > >If the patch doesn't want to be backported, then do not add the cc: > >stable line to it... > > This patch was picked because it has a stable tag, which you presumably > saw as your Reviewed-by tag is in the patch. This is why it was > backported; it doesn't take AI to backport patches tagged for stable... > > The revert of this patch, however: > > 1. Didn't have a stable tag. > 2. Didn't have a "Fixes:" tag. > 3. Didn't have the usual "the reverts commit ..." string added by git > when one does a revert. > > Which is why we still kick patches for review, even though they had a > stable tag, just so people could take a look and confirm we're not > missing anything - like we did here. > > I'm not sure what you expected me to do differently here. Yeah this looks like fail on the revert side, they need to reference the reverted commit somehow ... Alex, why got this dropped? Is this more fallout from the back shuffling you're doing between your internal branches behind the firewall, and the public history? Also adding Dave Airlie. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v4 01/16] drm: Add drm_minor_for_each
On Tue, Sep 3, 2019 at 9:45 PM Kenny Ho wrote:
>
> On Tue, Sep 3, 2019 at 3:57 AM Daniel Vetter wrote:
> >
> > On Thu, Aug 29, 2019 at 02:05:18AM -0400, Kenny Ho wrote:
> > > To allow other subsystems to iterate through all stored DRM minors and
> > > act upon them.
> > >
> > > Also exposes drm_minor_acquire and drm_minor_release for other subsystem
> > > to handle drm_minor. DRM cgroup controller is the initial consumer of
> > > this new features.
> > >
> > > Change-Id: I7c4b67ce6b31f06d1037b03435386ff5b8144ca5
> > > Signed-off-by: Kenny Ho
> >
> > Iterating over minors for cgroups sounds very, very wrong. Why do we care
> > whether a buffer was allocated through kms dumb vs render nodes?
> >
> > I'd expect all the cgroup stuff to only work on drm_device, if it does
> > care about devices.
> >
> > (I didn't look through the patch series to find out where exactly you're
> > using this, so maybe I'm off the rails here).
>
> I am exposing this to remove the need to keep track of a separate list
> of available drm_device in the system (to remove the registering and
> unregistering of drm_device to the cgroup subsystem and just use
> drm_minor as the single source of truth.) I am only filtering out the
> render nodes minor because they point to the same drm_device and is
> confusing.
>
> Perhaps I missed an obvious way to list the drm devices without
> iterating through the drm_minors? (I probably jumped to the minors
> because $major:$minor is the convention to address devices in cgroup.)
Create your own if there's nothing, because you need to anyway:
- You need special locking anyway, we can't just block on the idr lock
for everything.
- This needs to refcount drm_device, no the minors.
Iterating over stuff still feels kinda wrong still, because normally
the way we register/unregister userspace api (and cgroups isn't
anything else from a drm driver pov) is by adding more calls to
drm_dev_register/unregister. If you put a drm_cg_register/unregister
call in there we have a clean separation, and you can track all the
currently active devices however you want. Iterating over objects that
can be hotunplugged any time tends to get really complicated really
quickly.
-Daniel
>
> Kenny
>
> > -Daniel
> >
> > > ---
> > > drivers/gpu/drm/drm_drv.c | 19 +++
> > > drivers/gpu/drm/drm_internal.h | 4
> > > include/drm/drm_drv.h | 4
> > > 3 files changed, 23 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > index 862621494a93..000cddabd970 100644
> > > --- a/drivers/gpu/drm/drm_drv.c
> > > +++ b/drivers/gpu/drm/drm_drv.c
> > > @@ -254,11 +254,13 @@ struct drm_minor *drm_minor_acquire(unsigned int
> > > minor_id)
> > >
> > > return minor;
> > > }
> > > +EXPORT_SYMBOL(drm_minor_acquire);
> > >
> > > void drm_minor_release(struct drm_minor *minor)
> > > {
> > > drm_dev_put(minor->dev);
> > > }
> > > +EXPORT_SYMBOL(drm_minor_release);
> > >
> > > /**
> > > * DOC: driver instance overview
> > > @@ -1078,6 +1080,23 @@ int drm_dev_set_unique(struct drm_device *dev,
> > > const char *name)
> > > }
> > > EXPORT_SYMBOL(drm_dev_set_unique);
> > >
> > > +/**
> > > + * drm_minor_for_each - Iterate through all stored DRM minors
> > > + * @fn: Function to be called for each pointer.
> > > + * @data: Data passed to callback function.
> > > + *
> > > + * The callback function will be called for each @drm_minor entry,
> > > passing
> > > + * the minor, the entry and @data.
> > > + *
> > > + * If @fn returns anything other than %0, the iteration stops and that
> > > + * value is returned from this function.
> > > + */
> > > +int drm_minor_for_each(int (*fn)(int id, void *p, void *data), void
> > > *data)
> > > +{
> > > + return idr_for_each(_minors_idr, fn, data);
> > > +}
> > > +EXPORT_SYMBOL(drm_minor_for_each);
> > > +
> > > /*
> > > * DRM Core
> > > * The DRM core module initializes all global DRM objects and makes them
> > > diff --git a/drivers/gpu/drm/drm_internal.h
> > > b/drivers/gpu/drm/drm_internal.h
> > > index e19ac7ca602d..6bfad76f8e78 100644
> > > --- a/drivers/gpu/drm/drm_internal.h
> > > +++ b/drivers/gpu/drm/drm_internal.h
> > > @@ -54,10 +54,6 @@ void drm_prime_destroy_file_private(struct
> > > drm_prime_file_private *prime_fpriv);
> > > void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private
> > > *prime_fpriv,
> > > struct dma_buf *dma_buf);
> > >
> > > -/* drm_drv.c */
> > > -struct drm_minor *drm_minor_acquire(unsigned int minor_id);
> > > -void drm_minor_release(struct drm_minor *minor);
> > > -
> > > /* drm_vblank.c */
> > > void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int
> > > pipe);
> > > void drm_vblank_cleanup(struct drm_device *dev);
> > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > > index 68ca736c548d..24f8d054c570 100644
> > > --- a/include/drm/drm_drv.h
> >
Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment
On Tue, Sep 03, 2019 at 07:03:47PM +0200, Greg KH wrote: On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote: On 2019-09-03 6:23 p.m., Sasha Levin wrote: > From: Yu Zhao > > [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ] Hold your horses! This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be reverted, as they caused regressions. See commits 25ec429e86bb790e40387a550f0501d0ac55a47c & 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 . This isn't bolstering confidence in how these patches are selected... The patch _itself_ said to be backported to the stable trees from 4.2 and newer. Why wouldn't we be confident in doing this? If the patch doesn't want to be backported, then do not add the cc: stable line to it... This patch was picked because it has a stable tag, which you presumably saw as your Reviewed-by tag is in the patch. This is why it was backported; it doesn't take AI to backport patches tagged for stable... The revert of this patch, however: 1. Didn't have a stable tag. 2. Didn't have a "Fixes:" tag. 3. Didn't have the usual "the reverts commit ..." string added by git when one does a revert. Which is why we still kick patches for review, even though they had a stable tag, just so people could take a look and confirm we're not missing anything - like we did here. I'm not sure what you expected me to do differently here. -- Thanks, Sasha ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On 9/3/19 12:51 PM, Daniel Vetter wrote: >> The thing we need to stop is having mixed encryption rules under one VMA. > The point here is that we want this. We need to be able to move the > buffer between device ptes and system memory ptes, transparently, > behind userspace back, without races. And the fast path (which is "no > pte exists for this vma") must be real fast, so taking mmap_sem and > replacing the vma is no-go. So, when the user asks for encryption and we say, "sure, we'll encrypt that", then we want the device driver to be able to transparently undo that encryption under the covers for device memory? That seems suboptimal. I'd rather the device driver just say: "Nope, you can't encrypt my VMA". Because that's the truth. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203781] AMDGPU Radeon VII crashes with dual monitors
https://bugzilla.kernel.org/show_bug.cgi?id=203781 sehell...@gmail.com changed: What|Removed |Added Kernel Version|5.3-rc6, 5.2.x, 5.1.x |5.3-rc7, 5.2.x, 5.1.x -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On Tue, Sep 3, 2019 at 9:38 PM Dave Hansen wrote:
>
> This whole thing looks like a fascinating collection of hacks. :)
>
> ttm is taking a stack-alllocated "VMA" and handing it to vmf_insert_*()
> which obviously are expecting "real" VMAs that are linked into the mm.
> It's extracting some pgprot_t information from the real VMA, making a
> psuedo-temporary VMA, then passing the temporary one back into the
> insertion functions:
>
> > static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
> > {
> ...
> > struct vm_area_struct cvma;
> ...
> > if (vma->vm_flags & VM_MIXEDMAP)
> > ret = vmf_insert_mixed(, address,
> > __pfn_to_pfn_t(pfn, PFN_DEV));
> > else
> > ret = vmf_insert_pfn(, address, pfn);
>
> I can totally see why this needs new exports. But, man, it doesn't seem
> like something we want to keep *feeding*.
>
> The real problem here is that the encryption bits from the device VMA's
> "true" vma->vm_page_prot don't match the ones that actually get
> inserted, probably because the device ptes need the encryption bits
> cleared but the system memory PTEs need them set *and* they're mixed
> under one VMA.
>
> The thing we need to stop is having mixed encryption rules under one VMA.
The point here is that we want this. We need to be able to move the
buffer between device ptes and system memory ptes, transparently,
behind userspace back, without races. And the fast path (which is "no
pte exists for this vma") must be real fast, so taking mmap_sem and
replacing the vma is no-go.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v4 00/16] new cgroup controller for gpu/drm subsystem
On Tue, Sep 3, 2019 at 8:50 PM Tejun Heo wrote: > > Hello, Daniel. > > On Tue, Sep 03, 2019 at 09:55:50AM +0200, Daniel Vetter wrote: > > > * While breaking up and applying control to different types of > > > internal objects may seem attractive to folks who work day in and > > > day out with the subsystem, they aren't all that useful to users and > > > the siloed controls are likely to make the whole mechanism a lot > > > less useful. We had the same problem with cgroup1 memcg - putting > > > control of different uses of memory under separate knobs. It made > > > the whole thing pretty useless. e.g. if you constrain all knobs > > > tight enough to control the overall usage, overall utilization > > > suffers, but if you don't, you really don't have control over actual > > > usage. For memcg, what has to be allocated and controlled is > > > physical memory, no matter how they're used. It's not like you can > > > go buy more "socket" memory. At least from the looks of it, I'm > > > afraid gpu controller is repeating the same mistakes. > > > > We do have quite a pile of different memories and ranges, so I don't > > thinkt we're doing the same mistake here. But it is maybe a bit too > > I see. One thing which caught my eyes was the system memory control. > Shouldn't that be controlled by memcg? Is there something special > about system memory used by gpus? I think system memory separate from vram makes sense. For one, vram is like 10x+ faster than system memory, so we definitely want to have good control on that. But maybe we only want one vram bucket overall for the entire system? The trouble with system memory is that gpu tasks pin that memory to prep execution. There's two solutions: - i915 has a shrinker. Lots (and I really mean lots) of pain with direct reclaim recursion, which often means we can't free memory, and we're angering the oom killer a lot. Plus it introduces real bad latency spikes everywhere (gpu workloads are occasionally really slow, think "worse than pageout to spinning rust" to get memory freed). - ttm just has a global limit, set to 50% of system memory. I do think a global system memory limit to tame the shrinker, without the ttm approach of possible just wasting half your memory, could be useful. > > complicated, and exposes stuff that most users really don't care about. > > Could be from me not knowing much about gpus but definitely looks too > complex to me. I don't see how users would be able to alloate, vram, > system memory and GART with reasonable accuracy. memcg on cgroup2 > deals with just single number and that's already plenty challenging. Yeah, especially wrt GART and some of the other more specialized things I don't think there's any modern gpu were you can actually run out of that stuff. At least not before you run out of every other kind of memory (GART is just a remapping table to make system memory visible to the gpu). I'm also not sure of the bw limits, given all the fun we have on the block io cgroups side. Aside from that the current bw limit only controls the bw the kernel uses, userspace can submit unlimited amounts of copying commands that use the same pcie links directly to the gpu, bypassing this cg knob. Also, controlling execution time for gpus is very tricky, since they work a lot more like a block io device or maybe a network controller with packet scheduling, than a cpu. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v4 01/16] drm: Add drm_minor_for_each
On Tue, Sep 3, 2019 at 3:57 AM Daniel Vetter wrote:
>
> On Thu, Aug 29, 2019 at 02:05:18AM -0400, Kenny Ho wrote:
> > To allow other subsystems to iterate through all stored DRM minors and
> > act upon them.
> >
> > Also exposes drm_minor_acquire and drm_minor_release for other subsystem
> > to handle drm_minor. DRM cgroup controller is the initial consumer of
> > this new features.
> >
> > Change-Id: I7c4b67ce6b31f06d1037b03435386ff5b8144ca5
> > Signed-off-by: Kenny Ho
>
> Iterating over minors for cgroups sounds very, very wrong. Why do we care
> whether a buffer was allocated through kms dumb vs render nodes?
>
> I'd expect all the cgroup stuff to only work on drm_device, if it does
> care about devices.
>
> (I didn't look through the patch series to find out where exactly you're
> using this, so maybe I'm off the rails here).
I am exposing this to remove the need to keep track of a separate list
of available drm_device in the system (to remove the registering and
unregistering of drm_device to the cgroup subsystem and just use
drm_minor as the single source of truth.) I am only filtering out the
render nodes minor because they point to the same drm_device and is
confusing.
Perhaps I missed an obvious way to list the drm devices without
iterating through the drm_minors? (I probably jumped to the minors
because $major:$minor is the convention to address devices in cgroup.)
Kenny
> -Daniel
>
> > ---
> > drivers/gpu/drm/drm_drv.c | 19 +++
> > drivers/gpu/drm/drm_internal.h | 4
> > include/drm/drm_drv.h | 4
> > 3 files changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 862621494a93..000cddabd970 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -254,11 +254,13 @@ struct drm_minor *drm_minor_acquire(unsigned int
> > minor_id)
> >
> > return minor;
> > }
> > +EXPORT_SYMBOL(drm_minor_acquire);
> >
> > void drm_minor_release(struct drm_minor *minor)
> > {
> > drm_dev_put(minor->dev);
> > }
> > +EXPORT_SYMBOL(drm_minor_release);
> >
> > /**
> > * DOC: driver instance overview
> > @@ -1078,6 +1080,23 @@ int drm_dev_set_unique(struct drm_device *dev, const
> > char *name)
> > }
> > EXPORT_SYMBOL(drm_dev_set_unique);
> >
> > +/**
> > + * drm_minor_for_each - Iterate through all stored DRM minors
> > + * @fn: Function to be called for each pointer.
> > + * @data: Data passed to callback function.
> > + *
> > + * The callback function will be called for each @drm_minor entry, passing
> > + * the minor, the entry and @data.
> > + *
> > + * If @fn returns anything other than %0, the iteration stops and that
> > + * value is returned from this function.
> > + */
> > +int drm_minor_for_each(int (*fn)(int id, void *p, void *data), void *data)
> > +{
> > + return idr_for_each(_minors_idr, fn, data);
> > +}
> > +EXPORT_SYMBOL(drm_minor_for_each);
> > +
> > /*
> > * DRM Core
> > * The DRM core module initializes all global DRM objects and makes them
> > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> > index e19ac7ca602d..6bfad76f8e78 100644
> > --- a/drivers/gpu/drm/drm_internal.h
> > +++ b/drivers/gpu/drm/drm_internal.h
> > @@ -54,10 +54,6 @@ void drm_prime_destroy_file_private(struct
> > drm_prime_file_private *prime_fpriv);
> > void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private
> > *prime_fpriv,
> > struct dma_buf *dma_buf);
> >
> > -/* drm_drv.c */
> > -struct drm_minor *drm_minor_acquire(unsigned int minor_id);
> > -void drm_minor_release(struct drm_minor *minor);
> > -
> > /* drm_vblank.c */
> > void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int
> > pipe);
> > void drm_vblank_cleanup(struct drm_device *dev);
> > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > index 68ca736c548d..24f8d054c570 100644
> > --- a/include/drm/drm_drv.h
> > +++ b/include/drm/drm_drv.h
> > @@ -799,5 +799,9 @@ static inline bool drm_drv_uses_atomic_modeset(struct
> > drm_device *dev)
> >
> > int drm_dev_set_unique(struct drm_device *dev, const char *name);
> >
> > +int drm_minor_for_each(int (*fn)(int id, void *p, void *data), void *data);
> > +
> > +struct drm_minor *drm_minor_acquire(unsigned int minor_id);
> > +void drm_minor_release(struct drm_minor *minor);
> >
> > #endif
> > --
> > 2.22.0
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
This whole thing looks like a fascinating collection of hacks. :)
ttm is taking a stack-alllocated "VMA" and handing it to vmf_insert_*()
which obviously are expecting "real" VMAs that are linked into the mm.
It's extracting some pgprot_t information from the real VMA, making a
psuedo-temporary VMA, then passing the temporary one back into the
insertion functions:
> static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
> {
...
> struct vm_area_struct cvma;
...
> if (vma->vm_flags & VM_MIXEDMAP)
> ret = vmf_insert_mixed(, address,
> __pfn_to_pfn_t(pfn, PFN_DEV));
> else
> ret = vmf_insert_pfn(, address, pfn);
I can totally see why this needs new exports. But, man, it doesn't seem
like something we want to keep *feeding*.
The real problem here is that the encryption bits from the device VMA's
"true" vma->vm_page_prot don't match the ones that actually get
inserted, probably because the device ptes need the encryption bits
cleared but the system memory PTEs need them set *and* they're mixed
under one VMA.
The thing we need to stop is having mixed encryption rules under one VMA.
Re: Adreno crash on i.MX53 running 5.3-rc6
Hi Jonathan, On Tue, Sep 3, 2019 at 4:25 PM Jonathan Marek wrote: > > Hi, > > I tried this and it works with patches 4+5 from Rob's series and > changing gpummu to use sg_phys(sg) instead of sg->dma_address > (dma_address isn't set now that dma_map_sg isn't used). Thanks for testing it. I haven't had a chance to test it yet. Rob, I assume your series is targeted to 5.4, correct? If this is the case, what we should do about the i.MX5 regression on 5.3? Would a revert of the two commits be acceptable in 5.3 in order to avoid the regression? Please advise. Thanks ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v4 00/16] new cgroup controller for gpu/drm subsystem
On Tue, Sep 3, 2019 at 5:20 AM Daniel Vetter wrote: > > On Tue, Sep 3, 2019 at 10:24 AM Koenig, Christian > wrote: > > > > Am 03.09.19 um 10:02 schrieb Daniel Vetter: > > > On Thu, Aug 29, 2019 at 02:05:17AM -0400, Kenny Ho wrote: > > >> With this RFC v4, I am hoping to have some consensus on a merge plan. I > > >> believe > > >> the GEM related resources (drm.buffer.*) introduced in previous RFC and, > > >> hopefully, the logical GPU concept (drm.lgpu.*) introduced in this RFC > > >> are > > >> uncontroversial and ready to move out of RFC and into a more formal > > >> review. I > > >> will continue to work on the memory backend resources (drm.memory.*). > > >> > > >> The cover letter from v1 is copied below for reference. > > >> > > >> [v1]: > > >> https://lists.freedesktop.org/archives/dri-devel/2018-November/197106.html > > >> [v2]: https://www.spinics.net/lists/cgroups/msg22074.html > > >> [v3]: > > >> https://lists.freedesktop.org/archives/amd-gfx/2019-June/036026.html > > > So looking at all this doesn't seem to have changed much, and the old > > > discussion didn't really conclude anywhere (aside from some details). > > > > > > One more open though that crossed my mind, having read a ton of ttm again > > > recently: How does this all interact with ttm global limits? I'd say the > > > ttm global limits is the ur-cgroups we have in drm, and not looking at > > > that seems kinda bad. > > > > At least my hope was to completely replace ttm globals with those > > limitations here when it is ready. > > You need more, at least some kind of shrinker to cut down bo placed in > system memory when we're under memory pressure. Which drags in a > pretty epic amount of locking lols (see i915's shrinker fun, where we > attempt that). Probably another good idea to share at least some > concepts, maybe even code. I am still looking into your shrinker suggestion so the memory.* resources are untouch from RFC v3. The main change for the buffer.* resources is the removal of buffer sharing restriction as you suggested and additional documentation of that behaviour. (I may have neglected mentioning it in the cover.) The other key part of RFC v4 is the "logical GPU/lgpu" concept. I am hoping to get it out there early for feedback while I continue to work on the memory.* parts. Kenny > -Daniel > > > > > Christian. > > > > > -Daniel > > > > > >> v4: > > >> Unchanged (no review needed) > > >> * drm.memory.*/ttm resources (Patch 9-13, I am still working on memory > > >> bandwidth > > >> and shrinker) > > >> Base on feedbacks on v3: > > >> * update nominclature to drmcg > > >> * embed per device drmcg properties into drm_device > > >> * split GEM buffer related commits into stats and limit > > >> * rename function name to align with convention > > >> * combined buffer accounting and check into a try_charge function > > >> * support buffer stats without limit enforcement > > >> * removed GEM buffer sharing limitation > > >> * updated documentations > > >> New features: > > >> * introducing logical GPU concept > > >> * example implementation with AMD KFD > > >> > > >> v3: > > >> Base on feedbacks on v2: > > >> * removed .help type file from v2 > > >> * conform to cgroup convention for default and max handling > > >> * conform to cgroup convention for addressing device specific limits > > >> (with major:minor) > > >> New function: > > >> * adopted memparse for memory size related attributes > > >> * added macro to marshall drmcgrp cftype private (DRMCG_CTF_PRIV, etc.) > > >> * added ttm buffer usage stats (per cgroup, for system, tt, vram.) > > >> * added ttm buffer usage limit (per cgroup, for vram.) > > >> * added per cgroup bandwidth stats and limiting (burst and average > > >> bandwidth) > > >> > > >> v2: > > >> * Removed the vendoring concepts > > >> * Add limit to total buffer allocation > > >> * Add limit to the maximum size of a buffer allocation > > >> > > >> v1: cover letter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v4 00/16] new cgroup controller for gpu/drm subsystem
Hi Tejun, Thanks for looking into this. I can definitely help where I can and I am sure other experts will jump in if I start misrepresenting the reality :) (as Daniel already have done.) Regarding your points, my understanding is that there isn't really a TTM vs GEM situation anymore (there is an lwn.net article about that, but it is more than a decade old.) I believe GEM is the common interface at this point and more and more features are being refactored into it. For example, AMD's driver uses TTM internally but things are exposed via the GEM interface. This GEM resource is actually the single number resource you just referred to. A GEM buffer (the drm.buffer.* resources) can be backed by VRAM, or system memory or other type of memory. The more fine grain control is the drm.memory.* resources which still need more discussion. (As some of the functionalities in TTM are being refactored into the GEM level. I have seen some patches that make TTM a subclass of GEM.) This RFC can be grouped into 3 areas and they are fairly independent so they can be reviewed separately: high level device memory control (buffer.*), fine grain memory control and bandwidth (memory.*) and compute resources (lgpu.*) I think the memory.* resources are the most controversial part but I think it's still needed. Perhaps an analogy may help. For a system, we have CPUs and memory. And within memory, it can be backed by RAM or swap. For GPU, each device can have LGPUs and buffers. And within the buffers, it can be backed by VRAM, or system RAM or even swap. As for setting the right amount, I think that's where the profiling aspect of the *.stats comes in. And while one can't necessary buy more VRAM, it is still a useful knob to adjust if the intention is to pack more work into a GPU device with predictable performance. This research on various GPU workload may be of interest: A Taxonomy of GPGPU Performance Scaling http://www.computermachines.org/joe/posters/iiswc2015_taxonomy.pdf http://www.computermachines.org/joe/publications/pdfs/iiswc2015_taxonomy.pdf (summary: GPU workload can be memory bound or compute bound. So it's possible to pack different workload together to improve utilization.) Regards, Kenny On Tue, Sep 3, 2019 at 2:50 PM Tejun Heo wrote: > > Hello, Daniel. > > On Tue, Sep 03, 2019 at 09:55:50AM +0200, Daniel Vetter wrote: > > > * While breaking up and applying control to different types of > > > internal objects may seem attractive to folks who work day in and > > > day out with the subsystem, they aren't all that useful to users and > > > the siloed controls are likely to make the whole mechanism a lot > > > less useful. We had the same problem with cgroup1 memcg - putting > > > control of different uses of memory under separate knobs. It made > > > the whole thing pretty useless. e.g. if you constrain all knobs > > > tight enough to control the overall usage, overall utilization > > > suffers, but if you don't, you really don't have control over actual > > > usage. For memcg, what has to be allocated and controlled is > > > physical memory, no matter how they're used. It's not like you can > > > go buy more "socket" memory. At least from the looks of it, I'm > > > afraid gpu controller is repeating the same mistakes. > > > > We do have quite a pile of different memories and ranges, so I don't > > thinkt we're doing the same mistake here. But it is maybe a bit too > > I see. One thing which caught my eyes was the system memory control. > Shouldn't that be controlled by memcg? Is there something special > about system memory used by gpus? > > > complicated, and exposes stuff that most users really don't care about. > > Could be from me not knowing much about gpus but definitely looks too > complex to me. I don't see how users would be able to alloate, vram, > system memory and GART with reasonable accuracy. memcg on cgroup2 > deals with just single number and that's already plenty challenging. > > Thanks. > > -- > tejun ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/atomic: Take the atomic toys away from X
The -modesetting ddx has a totally broken idea of how atomic works: - doesn't disable old connectors, assuming they get auto-disable like with the legacy setcrtc - assumes ASYNC_FLIP is wired through for the atomic ioctl - not a single call to TEST_ONLY Iow the implementation is a 1:1 translation of legacy ioctls to atomic, which is a) broken b) pointless. We already have bugs in both i915 and amdgpu-DC where this prevents us from enabling neat features. If anyone ever cares about atomic in X we can easily add a new atomic level (req->value == 2) for X to get back the shiny toys. Since these broken versions of -modesetting have been shipping, there's really no other way to get out of this bind. References: https://gitlab.freedesktop.org/xorg/xserver/issues/629 References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180 Cc: Maarten Lankhorst Cc: Michel Dänzer Cc: Alex Deucher Cc: Adam Jackson Cc: Sean Paul Cc: David Airlie Cc: sta...@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_ioctl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 2c120c58f72d..1cb7b4c3c87c 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -334,6 +334,9 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) file_priv->universal_planes = req->value; break; case DRM_CLIENT_CAP_ATOMIC: + /* The modesetting DDX has a totally broken idea of atomic. */ + if (strstr(current->comm, "X")) + return -EOPNOTSUPP; if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) return -EOPNOTSUPP; if (req->value > 1) -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/atomic: Reject FLIP_ASYNC unconditionally
It's never been wired up. Only userspace that tried to use it (and didn't actually check whether anything works, but hey it builds) is the -modesetting atomic implementation. And we just shut that up. If there's anyone else then we need to silently accept this flag no matter what, and find a new one. Because once a flag is tainted, it's lost. Cc: Maarten Lankhorst Cc: Michel Dänzer Cc: Alex Deucher Cc: Adam Jackson Cc: Sean Paul Cc: David Airlie Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic_uapi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 5a5b42db6f2a..7a26bfb5329c 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1305,8 +1305,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, if (arg->reserved) return -EINVAL; - if ((arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) && - !dev->mode_config.async_page_flip) + if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) return -EINVAL; /* can't test and expect an event at the same time. */ -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/atomic: Rename crtc_state->pageflip_flags to async_flip
It's the only flag anyone actually cares about. Plus if we're unlucky,
the atomic ioctl might need a different flag for async flips. So
better to abstract this away from the uapi a bit.
Cc: Maarten Lankhorst
Cc: Michel Dänzer
Cc: Alex Deucher
Cc: Adam Jackson
Cc: Sean Paul
Cc: David Airlie
Signed-off-by: Daniel Vetter
Cc: Maxime Ripard
Cc: Daniel Vetter
Cc: Nicholas Kazlauskas
Cc: Leo Li
Cc: Harry Wentland
Cc: David Francis
Cc: Mario Kleiner
Cc: Bhawanpreet Lakha
Cc: Ben Skeggs
Cc: "Christian König"
Cc: Ilia Mirkin
Cc: Sam Ravnborg
Cc: Chris Wilson
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
drivers/gpu/drm/drm_atomic_helper.c | 2 +-
drivers/gpu/drm/drm_atomic_state_helper.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 ++--
include/drm/drm_crtc.h| 8
5 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0a71ed1e7762..2f0ef0820f00 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5756,8 +5756,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
* change FB pitch, DCC state, rotation or mirroing.
*/
bundle->flip_addrs[planes_count].flip_immediate =
- (crtc->state->pageflip_flags &
-DRM_MODE_PAGE_FLIP_ASYNC) != 0 &&
+ crtc->state->async_flip &&
acrtc_state->update_type == UPDATE_TYPE_FAST;
timestamp_ns = ktime_get_ns();
@@ -6334,7 +6333,7 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
amdgpu_dm_enable_crtc_interrupts(dev, state, true);
for_each_new_crtc_in_state(state, crtc, new_crtc_state, j)
- if (new_crtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC)
+ if (new_crtc_state->async_flip)
wait_for_vblank = false;
/* update planes when needed per crtc*/
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index e9c6112e7f73..1e5293eb66e3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3263,7 +3263,7 @@ static int page_flip_common(struct drm_atomic_state
*state,
return PTR_ERR(crtc_state);
crtc_state->event = event;
- crtc_state->pageflip_flags = flags;
+ crtc_state->async_flip = flags & DRM_MODE_PAGE_FLIP_ASYNC;
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state))
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 46dc264a248b..d0a937fb0c56 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -128,7 +128,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct
drm_crtc *crtc,
state->zpos_changed = false;
state->commit = NULL;
state->event = NULL;
- state->pageflip_flags = 0;
+ state->async_flip = false;
/* Self refresh should be canceled when a new update is available */
state->active = drm_atomic_crtc_effectively_active(state);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 2db029371c91..5193b6257061 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -267,7 +267,7 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw, bool
modeset,
asyw->image.pitch[0] = fb->base.pitches[0];
}
- if (!(asyh->state.pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC))
+ if (!asyh->state.async_flip)
asyw->image.interval = 1;
else
asyw->image.interval = 0;
@@ -383,7 +383,7 @@ nv50_wndw_atomic_check_lut(struct nv50_wndw *wndw,
}
/* Can't do an immediate flip while changing the LUT. */
- asyh->state.pageflip_flags &= ~DRM_MODE_PAGE_FLIP_ASYNC;
+ asyh->state.async_flip = false;
}
static int
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 7e2963cad543..900ae8d452b8 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -284,12 +284,12 @@ struct drm_crtc_state {
u32 target_vblank;
/**
-* @pageflip_flags:
+* @async_flip:
*
-* DRM_MODE_PAGE_FLIP_* flags, as passed to the page flip ioctl.
-* Zero in any other case.
+* This is set when DRM_MODE_PAGE_FLIP_ASYNC is set in the legacy
+* PAGE_FLIP IOCTL. It's not wired up for the atomic IOCTL itself yet.
*/
- u32 pageflip_flags;
+ bool async_flip;
/**
*
[PATCH 3/3] fixup
---
drivers/gpu/drm/drm_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 3c015dd3c94b..1cb7b4c3c87c 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -335,7 +335,7 @@ drm_setclientcap(struct drm_device *dev, void *data, struct
drm_file *file_priv)
break;
case DRM_CLIENT_CAP_ATOMIC:
/* The modesetting DDX has a totally broken idea of atomic. */
- if (strstr("X", current->comm))
+ if (strstr(current->comm, "X"))
return -EOPNOTSUPP;
if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
return -EOPNOTSUPP;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111229] Unable to unbind GPU from amdgpu
https://bugs.freedesktop.org/show_bug.cgi?id=111229 --- Comment #9 from weden...@yandex.ru --- I'll do more testing, but it seems that unbind works with kernel 5.3-rc7. There is still this error in the log: [drm:amdgpu_pci_remove [amdgpu]] *ERROR* Device removal is currently not supported outside of fbcon without any backtraces and unbind seems to succeed with and without X running (on other gpu, of course). It'd be nice to have confirmation from other people. Note that to bind gpu to vfio-pci reset app must be used after unbinding from amdgpu: https://forum.level1techs.com/t/vega-10-and-12-reset-application/145666 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/atomic: Reject FLIP_ASYNC unconditionally
It's never been wired up. Only userspace that tried to use it (and didn't actually check whether anything works, but hey it builds) is the -modesetting atomic implementation. And we just shut that up. If there's anyone else then we need to silently accept this flag no matter what, and find a new one. Because once a flag is tainted, it's lost. Cc: Maarten Lankhorst Cc: Michel Dänzer Cc: Alex Deucher Cc: Adam Jackson Cc: Sean Paul Cc: David Airlie Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic_uapi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 5a5b42db6f2a..7a26bfb5329c 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1305,8 +1305,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, if (arg->reserved) return -EINVAL; - if ((arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) && - !dev->mode_config.async_page_flip) + if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) return -EINVAL; /* can't test and expect an event at the same time. */ -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/atomic: Rename crtc_state->pageflip_flags to async_flip
It's the only flag anyone actually cares about. Plus if we're unlucky,
the atomic ioctl might need a different flag for async flips. So
better to abstract this away from the uapi a bit.
Cc: Maarten Lankhorst
Cc: Michel Dänzer
Cc: Alex Deucher
Cc: Adam Jackson
Cc: Sean Paul
Cc: David Airlie
Signed-off-by: Daniel Vetter
Cc: Maxime Ripard
Cc: Daniel Vetter
Cc: Nicholas Kazlauskas
Cc: Leo Li
Cc: Harry Wentland
Cc: David Francis
Cc: Mario Kleiner
Cc: Bhawanpreet Lakha
Cc: Ben Skeggs
Cc: "Christian König"
Cc: Ilia Mirkin
Cc: Sam Ravnborg
Cc: Chris Wilson
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
drivers/gpu/drm/drm_atomic_helper.c | 2 +-
drivers/gpu/drm/drm_atomic_state_helper.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 ++--
include/drm/drm_crtc.h| 8
5 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 028a710c1b46..b3c5ab3d09d5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5740,8 +5740,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
* change FB pitch, DCC state, rotation or mirroing.
*/
bundle->flip_addrs[planes_count].flip_immediate =
- (crtc->state->pageflip_flags &
-DRM_MODE_PAGE_FLIP_ASYNC) != 0 &&
+ crtc->state->async_flip &&
acrtc_state->update_type == UPDATE_TYPE_FAST;
timestamp_ns = ktime_get_ns();
@@ -6335,7 +6334,7 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
amdgpu_dm_enable_crtc_interrupts(dev, state, true);
for_each_new_crtc_in_state(state, crtc, new_crtc_state, j)
- if (new_crtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC)
+ if (new_crtc_state->async_flip)
wait_for_vblank = false;
/* update planes when needed per crtc*/
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 9f17746f4251..8dbf416e2807 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3262,7 +3262,7 @@ static int page_flip_common(struct drm_atomic_state
*state,
return PTR_ERR(crtc_state);
crtc_state->event = event;
- crtc_state->pageflip_flags = flags;
+ crtc_state->async_flip = flags & DRM_MODE_PAGE_FLIP_ASYNC;
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state))
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 46dc264a248b..d0a937fb0c56 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -128,7 +128,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct
drm_crtc *crtc,
state->zpos_changed = false;
state->commit = NULL;
state->event = NULL;
- state->pageflip_flags = 0;
+ state->async_flip = false;
/* Self refresh should be canceled when a new update is available */
state->active = drm_atomic_crtc_effectively_active(state);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 2db029371c91..5193b6257061 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -267,7 +267,7 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw, bool
modeset,
asyw->image.pitch[0] = fb->base.pitches[0];
}
- if (!(asyh->state.pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC))
+ if (!asyh->state.async_flip)
asyw->image.interval = 1;
else
asyw->image.interval = 0;
@@ -383,7 +383,7 @@ nv50_wndw_atomic_check_lut(struct nv50_wndw *wndw,
}
/* Can't do an immediate flip while changing the LUT. */
- asyh->state.pageflip_flags &= ~DRM_MODE_PAGE_FLIP_ASYNC;
+ asyh->state.async_flip = false;
}
static int
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 7d14c11bdc0a..c4528eb5d168 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -285,12 +285,12 @@ struct drm_crtc_state {
u32 target_vblank;
/**
-* @pageflip_flags:
+* @async_flip:
*
-* DRM_MODE_PAGE_FLIP_* flags, as passed to the page flip ioctl.
-* Zero in any other case.
+* This is set when DRM_MODE_PAGE_FLIP_ASYNC is set in the legacy
+* PAGE_FLIP IOCTL. It's not wired up for the atomic IOCTL itself yet.
*/
- u32 pageflip_flags;
+ bool async_flip;
/**
*
