Re: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression
Hi Am 04.09.19 um 08:27 schrieb Feng Tang: >> Thank you for testing. But don't get too excited, because the patch >> simulates a bug that was present in the original mgag200 code. A >> significant number of frames are simply skipped. That is apparently the >> reason why it's faster. > > Thanks for the detailed info, so the original code skips time-consuming > work inside atomic context on purpose. Is there any space to optmise it? > If 2 scheduled update worker are handled at almost same time, can one be > skipped? To my knowledge, there's only one instance of the worker. Re-scheduling the worker before a previous instance started, will not create a second instance. The worker's instance will complete all pending updates. So in some way, skipping workers already happens. Best regards Thomas > > Thanks, > Feng > >> >> Best regards >> Thomas > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg) signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support AMD memory encryption
On 9/4/19 1:15 AM, Andy Lutomirski wrote: 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? OK now I see what causes the confusion. With SEV, the encryption state is, while *physically* encoded in an address bit, from what I can tell, not *logically* encoded in the pfn, but in the page_prot for cpu mapping purposes. That is, page_to_pfn() returns the same pfn whether the page is encrypted or unencrypted. Hence nobody can't tell from the pfn whether the page is unencrypted or encrypted. For device DMA address purposes, the encryption status is encoded in the dma address by the dma layer in phys_to_dma(). There doesn’t seem to be any real funny business in dma_mmap_attrs() or dma_common_mmap(). No, from what I can tell the call in these functions to dma_pgprot() generates an incorrect page protection since it doesn't take unencrypted coherent memory into account. I don't think anybody has used these functions yet with SEV. But, reading this, I have more questions: Can’t you get rid of cvma by using vmf_insert_pfn_prot()? It looks like that, although there are comments in the code about serious performance problems using VM_PFNMAP / vmf_insert_pfn() with write-combining and PAT, so that would require some serious testing with hardware I don't have. But I guess there is definitely room for improvement here. Ideally we'd like to be able to change the vma->vm_page_prot within fault(). But we can Would it make sense to add a vmf_insert_dma_page() to directly do exactly what you’re trying to do? Yes, but as a longer term solution I would prefer a general dma_pgprot() exported, so that we could, in a dma-compliant way, use coherent pages with other apis, like kmap_atomic_prot() and vmap(). That is, basically split coherent page allocation in two steps: Allocation and mapping. And
[Bug 108979] Graphical glitch of popupping missing texture on Mesa version >18.0.5 (Padoka Stable + Unstable/Oibaf/ubuntu-x-swat PPAs)
https://bugs.freedesktop.org/show_bug.cgi?id=108979 --- Comment #6 from Timothy Arceri --- Is this still a 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 6/7] drm/omap: dss: platform_register_drivers() to dss.c and remove core.c
On 03/09/2019 18:34, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > Missing "Move" in the subject after "dss: " ? > That was intentional to keep the subject short enough. But it looks like it is just bellow 76 chars (80 - 4 char indent) even with "Move" added to it. BR, Jyri > On Mon, Sep 02, 2019 at 03:53:58PM +0300, Tomi Valkeinen wrote: >> From: Jyri Sarha >> >> The core.c just for registering the drivers is kind of useless. Let's >> get rid of it and register the dss drivers in dss.c. >> >> Signed-off-by: Jyri Sarha >> Signed-off-by: Tomi Valkeinen > > Reviewed-by: Laurent Pinchart > >> --- >> drivers/gpu/drm/omapdrm/dss/Makefile | 2 +- >> drivers/gpu/drm/omapdrm/dss/core.c | 55 >> drivers/gpu/drm/omapdrm/dss/dss.c| 37 +++ >> 3 files changed, 38 insertions(+), 56 deletions(-) >> delete mode 100644 drivers/gpu/drm/omapdrm/dss/core.c >> >> diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile >> b/drivers/gpu/drm/omapdrm/dss/Makefile >> index 904101c5e79d..5950c3f52c2e 100644 >> --- a/drivers/gpu/drm/omapdrm/dss/Makefile >> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile >> @@ -6,7 +6,7 @@ omapdss-base-y := base.o display.o dss-of.o output.o >> >> obj-$(CONFIG_OMAP2_DSS) += omapdss.o >> # Core DSS files >> -omapdss-y := core.o dss.o dispc.o dispc_coefs.o \ >> +omapdss-y := dss.o dispc.o dispc_coefs.o \ >> pll.o video-pll.o >> omapdss-$(CONFIG_OMAP2_DSS_DPI) += dpi.o >> omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o >> diff --git a/drivers/gpu/drm/omapdrm/dss/core.c >> b/drivers/gpu/drm/omapdrm/dss/core.c >> deleted file mode 100644 >> index 6ac497b63711.. >> --- a/drivers/gpu/drm/omapdrm/dss/core.c >> +++ /dev/null >> @@ -1,55 +0,0 @@ >> -// SPDX-License-Identifier: GPL-2.0-only >> -/* >> - * Copyright (C) 2009 Nokia Corporation >> - * Author: Tomi Valkeinen >> - * >> - * Some code and ideas taken from drivers/video/omap/ driver >> - * by Imre Deak. >> - */ >> - >> -#define DSS_SUBSYS_NAME "CORE" >> - >> -#include >> -#include >> -#include >> - >> -#include "omapdss.h" >> -#include "dss.h" >> - >> -/* INIT */ >> -static struct platform_driver * const omap_dss_drivers[] = { >> -&omap_dsshw_driver, >> -&omap_dispchw_driver, >> -#ifdef CONFIG_OMAP2_DSS_DSI >> -&omap_dsihw_driver, >> -#endif >> -#ifdef CONFIG_OMAP2_DSS_VENC >> -&omap_venchw_driver, >> -#endif >> -#ifdef CONFIG_OMAP4_DSS_HDMI >> -&omapdss_hdmi4hw_driver, >> -#endif >> -#ifdef CONFIG_OMAP5_DSS_HDMI >> -&omapdss_hdmi5hw_driver, >> -#endif >> -}; >> - >> -static int __init omap_dss_init(void) >> -{ >> -return platform_register_drivers(omap_dss_drivers, >> - ARRAY_SIZE(omap_dss_drivers)); >> -} >> - >> -static void __exit omap_dss_exit(void) >> -{ >> -platform_unregister_drivers(omap_dss_drivers, >> -ARRAY_SIZE(omap_dss_drivers)); >> -} >> - >> -module_init(omap_dss_init); >> -module_exit(omap_dss_exit); >> - >> -MODULE_AUTHOR("Tomi Valkeinen "); >> -MODULE_DESCRIPTION("OMAP2/3 Display Subsystem"); >> -MODULE_LICENSE("GPL v2"); >> - >> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c >> b/drivers/gpu/drm/omapdrm/dss/dss.c >> index e226324adb69..41d495a360d8 100644 >> --- a/drivers/gpu/drm/omapdrm/dss/dss.c >> +++ b/drivers/gpu/drm/omapdrm/dss/dss.c >> @@ -1598,3 +1598,40 @@ struct platform_driver omap_dsshw_driver = { >> .suppress_bind_attrs = true, >> }, >> }; >> + >> +/* INIT */ >> +static struct platform_driver * const omap_dss_drivers[] = { >> +&omap_dsshw_driver, >> +&omap_dispchw_driver, >> +#ifdef CONFIG_OMAP2_DSS_DSI >> +&omap_dsihw_driver, >> +#endif >> +#ifdef CONFIG_OMAP2_DSS_VENC >> +&omap_venchw_driver, >> +#endif >> +#ifdef CONFIG_OMAP4_DSS_HDMI >> +&omapdss_hdmi4hw_driver, >> +#endif >> +#ifdef CONFIG_OMAP5_DSS_HDMI >> +&omapdss_hdmi5hw_driver, >> +#endif >> +}; >> + >> +static int __init omap_dss_init(void) >> +{ >> +return platform_register_drivers(omap_dss_drivers, >> + ARRAY_SIZE(omap_dss_drivers)); >> +} >> + >> +static void __exit omap_dss_exit(void) >> +{ >> +platform_unregister_drivers(omap_dss_drivers, >> +ARRAY_SIZE(omap_dss_drivers)); >> +} >> + >> +module_init(omap_dss_init); >> +module_exit(omap_dss_exit); >> + >> +MODULE_AUTHOR("Tomi Valkeinen "); >> +MODULE_DESCRIPTION("OMAP2/3/4/5 Display Subsystem"); >> +MODULE_LICENSE("GPL v2"); > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: manual merge of the drm tree with the kbuild tree
Hi all, Today's linux-next merge of the drm tree got conflicts in: drivers/gpu/drm/amd/display/dc/calcs/Makefile drivers/gpu/drm/amd/display/dc/dml/Makefile drivers/gpu/drm/amd/display/dc/dsc/Makefile between commit: 30851871d5ab ("kbuild: change *FLAGS_.o to take the path relative to $(obj)") from the kbuild tree and commit: 0f0727d971f6 ("drm/amd/display: readd -msse2 to prevent Clang from emitting libcalls to undefined SW FP routines") from the drm tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/gpu/drm/amd/display/dc/calcs/Makefile index d930df63772c,16614d73a5fc.. --- a/drivers/gpu/drm/amd/display/dc/calcs/Makefile +++ b/drivers/gpu/drm/amd/display/dc/calcs/Makefile @@@ -32,9 -32,13 +32,13 @@@ endi calcs_ccflags := -mhard-float -msse $(cc_stack_align) + ifdef CONFIG_CC_IS_CLANG + calcs_ccflags += -msse2 + endif + -CFLAGS_dcn_calcs.o := $(calcs_ccflags) -CFLAGS_dcn_calc_auto.o := $(calcs_ccflags) -CFLAGS_dcn_calc_math.o := $(calcs_ccflags) -Wno-tautological-compare +CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calcs.o := $(calcs_ccflags) +CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calc_auto.o := $(calcs_ccflags) +CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calc_math.o := $(calcs_ccflags) -Wno-tautological-compare BW_CALCS = dce_calcs.o bw_fixed.o custom_float.o diff --cc drivers/gpu/drm/amd/display/dc/dml/Makefile index 83792e2c0f0e,95fd2beca80c.. --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile @@@ -32,16 -32,25 +32,20 @@@ endi dml_ccflags := -mhard-float -msse $(cc_stack_align) + ifdef CONFIG_CC_IS_CLANG + dml_ccflags += -msse2 + endif + -CFLAGS_display_mode_lib.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/display_mode_lib.o := $(dml_ccflags) ifdef CONFIG_DRM_AMD_DC_DCN2_0 -CFLAGS_display_mode_vba.o := $(dml_ccflags) -CFLAGS_display_mode_vba_20.o := $(dml_ccflags) -CFLAGS_display_rq_dlg_calc_20.o := $(dml_ccflags) -CFLAGS_display_mode_vba_20v2.o := $(dml_ccflags) -CFLAGS_display_rq_dlg_calc_20v2.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/display_mode_vba.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_rq_dlg_calc_20.o := $(dml_ccflags) endif -ifdef CONFIG_DRM_AMD_DCN3AG -CFLAGS_display_mode_vba_3ag.o := $(dml_ccflags) -endif -CFLAGS_dml1_display_rq_dlg_calc.o := $(dml_ccflags) -CFLAGS_display_rq_dlg_helpers.o := $(dml_ccflags) -CFLAGS_dml_common_defs.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/dml1_display_rq_dlg_calc.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/display_rq_dlg_helpers.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/dml_common_defs.o := $(dml_ccflags) DML = display_mode_lib.o display_rq_dlg_helpers.o dml1_display_rq_dlg_calc.o \ dml_common_defs.o diff --cc drivers/gpu/drm/amd/display/dc/dsc/Makefile index c3922d6e7696,17db603f2d1f.. --- a/drivers/gpu/drm/amd/display/dc/dsc/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dsc/Makefile @@@ -9,9 -9,14 +9,13 @@@ endi dsc_ccflags := -mhard-float -msse $(cc_stack_align) + ifdef CONFIG_CC_IS_CLANG + dsc_ccflags += -msse2 + endif + -CFLAGS_rc_calc.o := $(dsc_ccflags) -CFLAGS_rc_calc_dpi.o := $(dsc_ccflags) -CFLAGS_codec_main_amd.o := $(dsc_ccflags) -CFLAGS_dc_dsc.o := $(dsc_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dsc/rc_calc.o := $(dsc_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dsc/rc_calc_dpi.o := $(dsc_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dsc/dc_dsc.o := $(dsc_ccflags) DSC = dc_dsc.o rc_calc.o rc_calc_dpi.o pgpIrfWsl3LDd.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108973] The game Evil Twin segfaults when loading saved state.
https://bugs.freedesktop.org/show_bug.cgi?id=108973 Timothy Arceri changed: What|Removed |Added Status|NEEDINFO|RESOLVED Resolution|--- |INVALID --- Comment #2 from Timothy Arceri --- No reply so closing. -- 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: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression
Hi Thomas, On Wed, Aug 28, 2019 at 12:51:40PM +0200, Thomas Zimmermann wrote: > Hi > > Am 28.08.19 um 11:37 schrieb Rong Chen: > > Hi Thomas, > > > > On 8/28/19 1:16 AM, Thomas Zimmermann wrote: > >> Hi > >> > >> Am 27.08.19 um 14:33 schrieb Chen, Rong A: > >>> Both patches have little impact on the performance from our side. > >> Thanks for testing. Too bad they doesn't solve the issue. > >> > >> There's another patch attached. Could you please tests this as well? > >> Thanks a lot! > >> > >> The patch comes from Daniel Vetter after discussing the problem on IRC. > >> The idea of the patch is that the old mgag200 code might display much > >> less frames that the generic code, because mgag200 only prints from > >> non-atomic context. If we simulate this with the generic code, we should > >> see roughly the original performance. > >> > >> > > > > It's cool, the patch "usecansleep.patch" can fix the issue. > > Thank you for testing. But don't get too excited, because the patch > simulates a bug that was present in the original mgag200 code. A > significant number of frames are simply skipped. That is apparently the > reason why it's faster. Thanks for the detailed info, so the original code skips time-consuming work inside atomic context on purpose. Is there any space to optmise it? If 2 scheduled update worker are handled at almost same time, can one be skipped? Thanks, Feng > > Best regards > Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[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 = &qxl_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 = &qxl_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 &ttm_buffer_object info for debugfs + * @p: DRM printer + * @indent: Tab indentation level + * @gem: GEM object + * + * This function can be used as &drm_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.
[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 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 &drm_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 &drm_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
[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, * &struct 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(&glob->lru_lock); + drm_mm_print(mm, &p); + spin_unlock(&glob->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
[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&action=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 &csc_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(&buf[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&action=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&action=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(&kms->commit_lock); > > if (!(kms->pending_crtc_mask & crtc_mask)) { > mutex_unlock(&kms->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(&kms->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(&kms->commit_lock); > kms->funcs->complete_commit(kms, crtc_mask); > mutex_unlock(&kms->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, &async_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(&kms->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(&kms->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(&kms->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
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(&dpu_crtc->frame_pending)); > - goto end; > - } > - > if (atomic_inc_return(&dpu_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(&dpu_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(&dpu_enc->base, &wakeup_time)) > + if (dpu_encoder_vsync_time(&dpu_enc->base, &wakeup_time)) > return; > > trace_dpu_enc_vsync_event_work(DRMID(&dpu_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, &wakeup_time)) { > + !dpu_encoder_vsync_time(drm_enc, &wakeup_time)) { > trace_dpu_enc_early_kickoff(DRMID(drm_enc), > ktime_to_ms(wakeup_time)); > mod_timer(&dpu_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_s
[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(&kms->commit_lock); > + > + if (!(kms->pending_crtc_mask & crtc_mask)) { > + mutex_unlock(&kms->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(&kms->commit_lock); > + > + /* > + * Wait for flush to complete: > + */ > + kms->funcs->wait_flush(kms, crtc_mask); > + > + mutex_lock(&kms->commit_lock); > + kms->funcs->complete_commit(kms, crtc_mask); > + mutex_unlock(&kms->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, &timer->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->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); > + timer->timer.function = msm_atomic_pending_timer; > + INIT_WORK(&timer->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, &async_crtc); > > kms->funcs->enable_commit(kms); > + > + /* >
[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 &mgr->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(&mgr->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 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 = &history->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 = &new[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 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(&mstb->malloc_kref); - DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(&mstb->malloc_kref)); + DRM_DEBUG("mstb %p/%px (%d)\n", + mstb, mstb, kref_read(&mstb->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(&mstb->malloc_kref) - 1); + DRM_DEBUG("mstb %p/%px (%d)\n", + mstb, mstb, kref_read(&mstb->malloc_kref) - 1); kref_put(&mstb->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(&port->malloc_kref); - DRM_DEBUG("port %p (%d)\n", port, kref_read(&port->malloc_kref)); + DRM_DEBUG("port %p/%px (%d)\n", + port, port, kref_read(&port->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(&port->malloc_kref) - 1); + DRM_DEBUG("port %p/%px (%d)\n", + port, port, kref_read(&port->malloc_kref) - 1); kref_put(&port->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(&mstb->topology_kref); if (ret) - DRM_DEBUG("mstb %p (%d)\n", mstb, - kref_read(&mstb->topology_kref)); + DRM_DEBUG("mstb %p/%px (%d)\n", + mstb, mstb, kref_read(&mstb->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(&mstb->topology_kref) == 0); kref_get(&mstb->topology_kref); - DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(&mstb->topology_kref)); + DRM_DEBUG("mstb %p/%px (%d)\n", + mstb, mstb, kref_read(&mstb->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(&mstb->topology_kref) - 1); + DRM_DEBUG("mstb %p/%px (%d)\n", + mstb, mstb, kref_read(&mstb->topology_kref) - 1); kref_put(&mstb->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(&port->topology_kref); if (ret) - DRM_DEBUG("port %p (%d)\n", port, - kref_read(&port->topology_kref)); + DRM_DEBUG("port %p/%px (%d)\n", + port, port, kref_read(&port->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(&port->topology_kref) == 0); kref_get(&port->topology_kref); - DRM_DEBUG("port %p (%d)\n", port, kref_read(&port->topology_kref)); + DRM_DEBUG("port %p/%px (%d)\n", + port, port, kref_read(&port->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(&port->topology_kref) - 1); + DRM_DEBUG
[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, &dev->mode_config.connector_list, head) { + + drm_connector_list_iter_begin(dev, &iter); + drm_for_each_connector_iter(list_connector, + &iter) { 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(&iter); } } } @@ -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, &dev->mode_config.connector_list, head) { + drm_connector_list_iter_begin(dev, &iter); + drm_for_each_connector_iter(connector, &iter) { amdgpu_connector = to_amdgpu_connector(connector); if (amdgpu_connector->connector_id == connector_id) { amdgpu_connector->devices |= supported_device; + drm_connector_list_iter_end(&iter); 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(&iter); /* check if it's a dp bridge */ list_for_each_entry(encoder, &dev->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, &dev->mode_config.connector_list, head) { -
[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(&mgr->lock); + flush_work(&mgr->up_req_work); flush_work(&mgr->work); flush_work(&mgr->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 = &up_req->msg; + struct drm_dp_sideband_msg_hdr *hdr = &up_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, &msg->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(&mgr->up_req_lock); + up_req = list_first_entry_or_null(&mgr->up_req_list, + struct drm_dp_pending_up_req, + next); + if (up_req) +
[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(&port->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, +&port->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(&port->topology_kref); kref_init(&port->malloc_kref); + mutex_init(&port->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(&port->lock); + drm_modeset_lock(&dev->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); -
[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(&port->lock); drm_modeset_lock(&dev->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(&dev->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(&port->lock); + drm_dp_mst_topology_put_port(port); if (dowork) queue_work(system_long_wq, &mstb->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(&nv_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(&nv_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(&nv_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(&nv_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, &reply->ports[i]); + drm_dp_mst_handle_link_address_port(mstb, mgr->dev, + &reply->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, &msg.u.conn_stat); + drm_dp_mst_handle_conn_stat(mstb, &msg.u.conn_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&action=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(&mgr->base); mgr->funcs = NULL; + + mutex_destroy(&mgr->delayed_destroy_lock); + mutex_destroy(&mgr->payload_lock); + mutex_destroy(&mgr->qlock); + mutex_destroy(&mgr->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, &iter); drm_for_each_connector_iter(connector, &iter) { -- 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(&mgr->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(&mgr->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(&mgr->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(&mgr->up_req_recv, &msg); + seqno = mgr->up_req_recv.initial_hdr.seqno; + drm_dp_sideband_parse_req(&mgr->up_req_recv, &msg); - 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(&mgr->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(&mgr->up_req_recv, 0, sizeof(struct drm_dp_sideband_msg_rx)); + return 0; + } - drm_dp_update_port(mstb, &msg.u.conn_stat); + drm_dp_update_port(mstb, &msg.u.conn_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
[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 &mgr->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 = &mgr->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(&mgr->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(&mgr->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(&mgr->up_req_recv, &msg); - 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(&mgr->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, &msg.u.conn_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.
[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(&txmsg->reply.u.link_addr); + reply = &txmsg->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, &txmsg->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, &reply->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 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(&mgr->delayed_destroy_lock); - list_add(&port->next, &mgr->destroy_port_list); - mutex_unlock(&mgr->delayed_destroy_lock); - schedule_work(&mgr->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(&mgr->delayed_destroy_lock); + list_add(&port->next, &mgr->destroy_port_list); + mutex_unlock(&mgr->delayed_destroy_lock); + schedule_work(&mgr->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 &mgr->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(&mstb->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(&port->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(&port->aux); + break; + case DP_PEER_DEVICE_MST_BRANCHING: + mutex_lock(&mgr->lock); + drm_dp_mst_topology_put_mstb(port->mstb); + port->mstb = NULL; + mutex_unlock(&mgr->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(&port->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(&mgr->lock); + port->mstb = mstb; + mstb->mgr = port->mgr; + mstb->port_parent = port; - send_link = true; - } + /* +* Make sure this port's memory allocation stays +
[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 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 = + &req->u.allocate_payload; + + a->num
[PATCH v2 01/27] drm/dp_mst: Move link address dumping into a function
Makes things easier to read. 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 | 35 ++- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 82add736e17d..36db66a0ddb1 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2103,6 +2103,28 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr, mutex_unlock(&mgr->qlock); } +static void +drm_dp_dump_link_address(struct drm_dp_link_address_ack_reply *reply) +{ + struct drm_dp_link_addr_reply_port *port_reply; + int i; + + for (i = 0; i < reply->nports; i++) { + port_reply = &reply->ports[i]; + DRM_DEBUG_KMS("port %d: input %d, pdt: %d, pn: %d, dpcd_rev: %02x, mcs: %d, ddps: %d, ldps %d, sdp %d/%d\n", + i, + port_reply->input_port, + port_reply->peer_device_type, + port_reply->port_number, + port_reply->dpcd_revision, + port_reply->mcs, + port_reply->ddps, + port_reply->legacy_device_plug_status, + port_reply->num_sdp_streams, + port_reply->num_sdp_stream_sinks); + } +} + static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_branch *mstb) { @@ -2128,18 +2150,7 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr, DRM_DEBUG_KMS("link address nak received\n"); } else { DRM_DEBUG_KMS("link address reply: %d\n", txmsg->reply.u.link_addr.nports); - for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) { - DRM_DEBUG_KMS("port %d: input %d, pdt: %d, pn: %d, dpcd_rev: %02x, mcs: %d, ddps: %d, ldps %d, sdp %d/%d\n", i, - txmsg->reply.u.link_addr.ports[i].input_port, - txmsg->reply.u.link_addr.ports[i].peer_device_type, - txmsg->reply.u.link_addr.ports[i].port_number, - txmsg->reply.u.link_addr.ports[i].dpcd_revision, - txmsg->reply.u.link_addr.ports[i].mcs, - txmsg->reply.u.link_addr.ports[i].ddps, - txmsg->reply.u.link_addr.ports[i].legacy_device_plug_status, - txmsg->reply.u.link_addr.ports[i].num_sdp_streams, - txmsg->reply.u.link_addr.ports[i].num_sdp_stream_sinks); - } + drm_dp_dump_link_address(&txmsg->reply.u.link_addr); drm_dp_check_mstb_guid(mstb, txmsg->reply.u.link_addr.guid); -- 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 &mgr->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 = &mgr->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(&mgr->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(&mgr->qlock); + txmsg = mstb->tx_slots[slot]; + /* remove from slots */ + mutex_unlock(&mgr->qlock); - if (!mstb) { - DRM_DEBUG_KMS("Got MST reply from unknown device %d\n", mgr->down_rep_recv.initial_hdr.lct); - memset(&mgr->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(&mgr->qlock); - txmsg = mstb->tx_slots[slot]; - /* remove from slots */ - mutex_unlock(&mgr->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(&mgr->down_rep_recv, 0, sizeof(struct drm_dp_sideband_msg_rx)); - return 0; - } + drm_dp_sideband_parse_reply(&mgr->down_rep_recv, &txmsg->reply); - drm_dp_sideband_parse_reply(&mgr->down_rep_recv, &txmsg->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(&mgr->down_rep_recv, 0, sizeof(struct
[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 = &txmsg->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(&buf[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, &mgr->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 &mgr->lock, destroying MSTBs from this context would result in attempting to lock &mgr->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(&mgr->lock); - list_for_each_entry_safe(port, tmp, &mstb->ports, next) { - list_del(&port->next); - drm_dp_mst_topology_put_port(port); - } - mutex_unlock(&mgr->lock); - - /* drop any tx slots msg */ - mutex_lock(&mstb->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(&mstb->mgr->qlock); + INIT_LIST_HEAD(&mstb->destroy_next); - if (wake_tx) - wake_up_all(&mstb->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(&mgr->delayed_destroy_lock); + list_add(&mstb->destroy_next, &mgr->destroy_branch_device_list); + mutex_unlock(&mgr->delayed_destroy_lock); + schedule_work(&mgr->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(&mgr->destroy_connector_lock); - list_add(&port->next, &mgr->destroy_connector_list); - mutex_unlock(&mgr->destroy_connector_lock); - schedule_work(&mgr->destroy_connector_work); + mutex_lock(&mgr->delayed_destroy_lock); + list_add(&port->next, &mgr->destroy_port_list); + mutex_unlock(&mgr->delayed_destroy_lock); + schedule_work(&mgr->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(&mgr->lock); flush_work(&mgr->work); - flush_work(&mgr->destroy_connector_work); + flush_work(&mgr->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(&mgr->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, po
[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(&port->next); mutex_unlock(&mgr->destroy_connector_lock); - INIT_LIST_HEAD(&port->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 &drm_printer stream * @p: the &drm 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 &drm_printer that outputs to pr_err() + * @prefix: debug output prefix + * + * RETURNS: + * The &drm_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(&drm_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_vb
[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&action=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(&dpu_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(&dpu_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(&dpu_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(&dpu_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 @@ sta
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(&priv->event_thread[crtc_id].worker, &fevent->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(&dpu_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 v
Re: [PATCH 06/10] drm/msm: add kms->flush_commit()
On Thu, Aug 29, 2019 at 09:45:14AM -0700, Rob Clark wrote: > From: Rob Clark > > Add ->flush_commit(crtc_mask). Currently a no-op, but kms backends > should migrate writing flush registers to this hook, so we can decouple > pushing updates to hardware, and flushing the updates. > > Once we add async commit support, the hw updates will be pushed down to > the hw synchronously, but flushing the updates will be deferred until as > close to vblank as possible, so that multiple updates can be combined in > a single frame. > > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 6 > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 > drivers/gpu/drm/msm/msm_atomic.c | 9 -- > drivers/gpu/drm/msm/msm_kms.h| 40 ++-- > 5 files changed, 63 insertions(+), 4 deletions(-) > /snip > diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h > index 10dd171b43f8..bb70c1758c72 100644 > --- a/drivers/gpu/drm/msm/msm_kms.h > +++ b/drivers/gpu/drm/msm/msm_kms.h > @@ -30,12 +30,47 @@ struct msm_kms_funcs { > irqreturn_t (*irq)(struct msm_kms *kms); > int (*enable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc); > void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc); > - /* modeset, bracketing atomic_commit(): */ > + > + /* > + * Atomic commit handling: > + */ > + > + /** > + * Prepare for atomic commit. This is called after any previous > + * (async or otherwise) commit has completed. > + */ > void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state > *state); > + > + /** > + * Flush an atomic commit. This is called after the hardware > + * updates have already been pushed down to effected planes/ > + * crtcs/encoders/connectors. > + */ > + void (*flush_commit)(struct msm_kms *kms, unsigned crtc_mask); > + > + /* TODO remove ->commit(), use ->flush_commit() instead: */ > void (*commit)(struct msm_kms *kms, struct drm_atomic_state *state); > - void (*complete_commit)(struct msm_kms *kms, unsigned crtc_mask); > + > + /** > + * Wait for any in-progress flush to complete on the specified > + * crtcs. This should not block if there is no in-progress > + * commit (ie. don't just wait for a vblank), as it will also > + * be called before ->prepare_commit() to ensure any potential > + * "async" commit has completed. > + */ > void (*wait_flush)(struct msm_kms *kms, unsigned crtc_mask); > > + /** > + * Clean up are commit is completed. This is called after s/are/our/? With that fixed, Reviewed-by: Sean Paul > + * ->wait_flush(), to give the backend a chance to do any > + * post-commit cleanup. > + */ > + void (*complete_commit)(struct msm_kms *kms, unsigned crtc_mask); > + > + /* > + * Format handling: > + */ > + > /* get msm_format w/ optional format modifiers from drm_mode_fb_cmd2 */ > const struct msm_format *(*get_format)(struct msm_kms *kms, > const uint32_t format, > @@ -45,6 +80,7 @@ struct msm_kms_funcs { > const struct msm_format *msm_fmt, > const struct drm_mode_fb_cmd2 *cmd, > struct drm_gem_object **bos); > + > /* misc: */ > long (*round_pixclk)(struct msm_kms *kms, unsigned long rate, > struct drm_encoder *encoder); > -- > 2.21.0 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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(&hdmi->audio_lock); + + hdmi_set_schnl(hdmi); I will suggest this function is called from or merged with dw_hdmi
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&forth 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(&drm_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(&cvma, address, > > __pfn_to_pfn_t(pfn, PFN_DEV)); > > else > > ret = vmf_insert_pfn(&cvma, 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(&drm_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(&cvma, address, > __pfn_to_pfn_t(pfn, PFN_DEV)); > else > ret = vmf_insert_pfn(&cvma, 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