Linaro-mm-sig November 2020

linaro-mm-sig@lists.linaro.org

13 participants
86 discussions

Re: [Linaro-mm-sig] [PATCH 00/19] [Set 2] Rid W=1 warnings from GPU

by Christian König

Well that's quite a patch set. First of all can you separate this a bit more by driver? I'm assuming we maintainers are supposed to pick that up and apply it. radeon and amdgpu can stick together since that is mostly Alex and me, but I'm not sure if we want to do some of the suggested changes to radeon. Going to pick up the single TTM change for upstreaming. Thanks, Christian. Am 06.11.20 um 22:49 schrieb Lee Jones: > This set is part of a larger effort attempting to clean-up W=1 > kernel builds, which are currently overwhelmingly riddled with > niggly little warnings. > > There are 5000 warnings to work through. It will take a couple more > sets. Although, ("drm/amd/display/dc/basics/fixpt31_32: Move > variables to where they're used") does take care of 2000 of them! > > Lee Jones (19): > drm/ttm/ttm_range_manager: Demote non-conformant kernel-doc header > drm/r128/ati_pcigart: Source file headers are not good candidates for > kernel-doc > drm/selftests/test-drm_dp_mst_helper: Move > 'sideband_msg_req_encode_decode' onto the heap > drm/mga/mga_dma: Demote kernel-doc abusers to standard comment blocks > drm/mga/mga_state: Remove unused variable 'buf_priv' > drm/radeon/atom: Move prototype into shared location > drm/radeon/radeon_kms: Include header containing our own prototypes > drm/omapdrm/omap_gem: Fix misnamed and missing parameter descriptions > drm/omapdrm/omap_dmm_tiler: Demote abusive use of kernel-doc format > drm/radeon/radeon: Move prototype into shared header > drm/radeon/radeon_drv: Source file headers are not good candidates for > kernel-doc > drm/amd/display/dc/basics/fixpt31_32: Move variables to where they're > used > drm/radeon/radeon_drv: Move prototypes to a shared headerfile > drm/amd/amdgpu/amdgpu_device: Provide documentation for 'reg_addr' > params > drm/radeon: Move prototypes to shared header > drm/amd/amdgpu/amdgpu_kms: Remove 'struct drm_amdgpu_info_device > dev_info' from the stack > drm/radeon/radeon_kms: Fix misnaming of 'radeon_info_ioctl's dev param > drm/radeon/atombios_crtc: Remove description of non-existent function > param 'encoder' > drm/v3d/v3d_drv: Remove unused static variable 'v3d_v3d_pm_ops' > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 104 +++++++++--------- > .../drm/amd/display/dc/basics/fixpt31_32.c | 5 + > .../gpu/drm/amd/display/include/fixed31_32.h | 6 - > drivers/gpu/drm/mga/mga_dma.c | 10 +- > drivers/gpu/drm/mga/mga_state.c | 2 - > drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 6 +- > drivers/gpu/drm/omapdrm/omap_gem.c | 3 +- > drivers/gpu/drm/r128/ati_pcigart.c | 2 +- > drivers/gpu/drm/radeon/atom.h | 6 + > drivers/gpu/drm/radeon/atombios_crtc.c | 1 - > drivers/gpu/drm/radeon/atombios_encoders.c | 4 - > drivers/gpu/drm/radeon/radeon.h | 6 + > drivers/gpu/drm/radeon/radeon_device.c | 1 + > drivers/gpu/drm/radeon/radeon_device.h | 32 ++++++ > drivers/gpu/drm/radeon/radeon_display.c | 4 - > drivers/gpu/drm/radeon/radeon_drv.c | 11 +- > drivers/gpu/drm/radeon/radeon_drv.h | 7 ++ > drivers/gpu/drm/radeon/radeon_kms.c | 3 +- > .../drm/selftests/test-drm_dp_mst_helper.c | 11 +- > drivers/gpu/drm/ttm/ttm_range_manager.c | 2 +- > drivers/gpu/drm/v3d/v3d_drv.c | 36 ------ > 22 files changed, 138 insertions(+), 126 deletions(-) > create mode 100644 drivers/gpu/drm/radeon/radeon_device.h > > Cc: Alex Deucher <alexander.deucher(a)amd.com> > Cc: amd-gfx(a)lists.freedesktop.org > Cc: Andy Gross <andy.gross(a)ti.com> > Cc: by <jhartmann(a)precisioninsight.com> > Cc: Christian Koenig <christian.koenig(a)amd.com> > Cc: "Christian König" <christian.koenig(a)amd.com> > Cc: Daniel Vetter <daniel(a)ffwll.ch> > Cc: David Airlie <airlied(a)linux.ie> > Cc: dri-devel(a)lists.freedesktop.org > Cc: Eric Anholt <eric(a)anholt.net> > Cc: Faith <faith(a)valinux.com> > Cc: Gareth Hughes <gareth(a)valinux.com> > Cc: Harry Wentland <harry.wentland(a)amd.com> > Cc: Huang Rui <ray.huang(a)amd.com> > Cc: Jeff Hartmann <jhartmann(a)valinux.com> > Cc: Keith Whitwell <keith(a)tungstengraphics.com> > Cc: Leo Li <sunpeng.li(a)amd.com> > Cc: linaro-mm-sig(a)lists.linaro.org > Cc: linux-media(a)vger.kernel.org > Cc: Philipp Zabel <p.zabel(a)pengutronix.de> > Cc: Rob Clark <rob.clark(a)linaro.org> > Cc: Rob Clark <rob(a)ti.com> > Cc: Sumit Semwal <sumit.semwal(a)linaro.org> > Cc: Tomi Valkeinen <tomi.valkeinen(a)ti.com>

3 years, 5 months

Re: [Linaro-mm-sig] [PATCH 10/19] drm/radeon/radeon: Move prototype into shared header

by Sam Ravnborg

Hi Lee, On Fri, Nov 06, 2020 at 09:49:40PM +0000, Lee Jones wrote: > Unfortunately, a suitable one didn't already exist. > > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/radeon/radeon_device.c:637:6: warning: no previous prototype for ‘radeon_device_is_virtual’ [-Wmissing-prototypes] > 637 | bool radeon_device_is_virtual(void) > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > Cc: Alex Deucher <alexander.deucher(a)amd.com> > Cc: "Christian König" <christian.koenig(a)amd.com> > Cc: David Airlie <airlied(a)linux.ie> > Cc: Daniel Vetter <daniel(a)ffwll.ch> > Cc: Sumit Semwal <sumit.semwal(a)linaro.org> > Cc: amd-gfx(a)lists.freedesktop.org > Cc: dri-devel(a)lists.freedesktop.org > Cc: linux-media(a)vger.kernel.org > Cc: linaro-mm-sig(a)lists.linaro.org > Signed-off-by: Lee Jones <lee.jones(a)linaro.org> > --- > drivers/gpu/drm/radeon/radeon_device.c | 1 + > drivers/gpu/drm/radeon/radeon_device.h | 32 ++++++++++++++++++++++++++ > drivers/gpu/drm/radeon/radeon_drv.c | 3 +-- > 3 files changed, 34 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/radeon/radeon_device.h Other public functions in radeon_device.c have their prototype in radeon.h - for example radeon_is_px() Add radeon_device_is_virtual() there so we avoiid this new header. Sam

3 years, 5 months

Re: [Linaro-mm-sig] [PATCH 00/19] [Set 2] Rid W=1 warnings from GPU

by Sam Ravnborg

Hi Lee and DRM folks. On Fri, Nov 06, 2020 at 09:49:30PM +0000, Lee Jones wrote: > This set is part of a larger effort attempting to clean-up W=1 > kernel builds, which are currently overwhelmingly riddled with > niggly little warnings. > > There are 5000 warnings to work through. It will take a couple more > sets. Although, ("drm/amd/display/dc/basics/fixpt31_32: Move > variables to where they're used") does take care of 2000 of them! > > Lee Jones (19): > drm/ttm/ttm_range_manager: Demote non-conformant kernel-doc header > drm/r128/ati_pcigart: Source file headers are not good candidates for > kernel-doc Applied > drm/selftests/test-drm_dp_mst_helper: Move > 'sideband_msg_req_encode_decode' onto the heap > drm/mga/mga_dma: Demote kernel-doc abusers to standard comment blocks > drm/mga/mga_state: Remove unused variable 'buf_priv' Applied x2 > drm/radeon/atom: Move prototype into shared location > drm/radeon/radeon_kms: Include header containing our own prototypes > drm/omapdrm/omap_gem: Fix misnamed and missing parameter descriptions > drm/omapdrm/omap_dmm_tiler: Demote abusive use of kernel-doc format > drm/radeon/radeon: Move prototype into shared header > drm/radeon/radeon_drv: Source file headers are not good candidates for > kernel-doc > drm/amd/display/dc/basics/fixpt31_32: Move variables to where they're > used > drm/radeon/radeon_drv: Move prototypes to a shared headerfile > drm/amd/amdgpu/amdgpu_device: Provide documentation for 'reg_addr' > params > drm/radeon: Move prototypes to shared header > drm/amd/amdgpu/amdgpu_kms: Remove 'struct drm_amdgpu_info_device > dev_info' from the stack > drm/radeon/radeon_kms: Fix misnaming of 'radeon_info_ioctl's dev param > drm/radeon/atombios_crtc: Remove description of non-existent function > param 'encoder' > drm/v3d/v3d_drv: Remove unused static variable 'v3d_v3d_pm_ops' I have applied the three patches that has no obvious maintainer as indicated above. I assume the respective maintaines to pick radeon, omapdrm, ttm, amd, v3d and selftest patches. Sam

3 years, 5 months

[PATCH 1/2] mm: mmap: fix fput in error path v2

by Christian König

Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..." adds a workaround for a bug in mmap_region. As the comment states ->mmap() callback can change vma->vm_file and so we might call fput() on the wrong file. Revert the workaround and proper fix this in mmap_region. v2: drop the extra if in dma_buf_mmap as well Signed-off-by: Christian König <christian.koenig(a)amd.com> Reviewed-by: Jason Gunthorpe <jgg(a)nvidia.com> --- drivers/dma-buf/dma-buf.c | 20 +++----------------- mm/mmap.c | 2 +- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index a6ba4d598f0e..08630d057cf2 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1143,9 +1143,6 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access); int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, unsigned long pgoff) { - struct file *oldfile; - int ret; - if (WARN_ON(!dmabuf || !vma)) return -EINVAL; @@ -1163,22 +1160,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, return -EINVAL; /* readjust the vma */ - get_file(dmabuf->file); - oldfile = vma->vm_file; - vma->vm_file = dmabuf->file; + fput(vma->vm_file); + vma->vm_file = get_file(dmabuf->file); vma->vm_pgoff = pgoff; - ret = dmabuf->ops->mmap(dmabuf, vma); - if (ret) { - /* restore old parameters on failure */ - vma->vm_file = oldfile; - fput(dmabuf->file); - } else { - if (oldfile) - fput(oldfile); - } - return ret; - + return dmabuf->ops->mmap(dmabuf, vma); } EXPORT_SYMBOL_GPL(dma_buf_mmap); diff --git a/mm/mmap.c b/mm/mmap.c index 40248d84ad5f..3a2670d73355 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1852,8 +1852,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr, return addr; unmap_and_free_vma: + fput(vma->vm_file); vma->vm_file = NULL; - fput(file); /* Undo any partial mapping done by a device driver. */ unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end); -- 2.17.1

3 years, 6 months

[PATCH 5.9 267/391] drm/shme-helpers: Fix dma_buf_mmap forwarding bug

by Greg Kroah-Hartman

From: Daniel Vetter <daniel.vetter(a)ffwll.ch> commit f49a51bfdc8ea717c97ccd4cc98b7e6daaa5553a upstream. When we forward an mmap to the dma_buf exporter, they get to own everything. Unfortunately drm_gem_mmap_obj() overwrote vma->vm_private_data after the driver callback, wreaking the exporter complete. This was noticed because vb2_common_vm_close blew up on mali gpu with panfrost after commit 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf"). Unfortunately drm_gem_mmap_obj also acquires a surplus reference that we need to drop in shmem helpers, which is a bit of a mislayer situation. Maybe the entire dma_buf_mmap forwarding should be pulled into core gem code. Note that the only two other drivers which forward mmap in their own code (etnaviv and exynos) get this somewhat right by overwriting the gem mmap code. But they seem to still have the leak. This might be a good excuse to move these drivers over to shmem helpers completely. Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> Acked-by: Christian König <christian.koenig(a)amd.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Lucas Stach <l.stach(a)pengutronix.de> Cc: Russell King <linux+etnaviv(a)armlinux.org.uk> Cc: Christian Gmeiner <christian.gmeiner(a)gmail.com> Cc: Inki Dae <inki.dae(a)samsung.com> Cc: Joonyoung Shim <jy0922.shim(a)samsung.com> Cc: Seung-Woo Kim <sw0312.kim(a)samsung.com> Cc: Kyungmin Park <kyungmin.park(a)samsung.com> Fixes: 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf") Cc: Boris Brezillon <boris.brezillon(a)collabora.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Gerd Hoffmann <kraxel(a)redhat.com> Cc: Rob Herring <robh(a)kernel.org> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: <stable(a)vger.kernel.org> # v5.9+ Reported-and-tested-by: piotr.oniszczuk(a)gmail.com Cc: piotr.oniszczuk(a)gmail.com Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20201027214922.3566743-1-dani… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/gpu/drm/drm_gem.c | 4 ++-- drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1085,6 +1085,8 @@ int drm_gem_mmap_obj(struct drm_gem_obje */ drm_gem_object_get(obj); + vma->vm_private_data = obj; + if (obj->funcs && obj->funcs->mmap) { ret = obj->funcs->mmap(obj, vma); if (ret) { @@ -1107,8 +1109,6 @@ int drm_gem_mmap_obj(struct drm_gem_obje vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); } - vma->vm_private_data = obj; - return 0; } EXPORT_SYMBOL(drm_gem_mmap_obj); --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -594,8 +594,13 @@ int drm_gem_shmem_mmap(struct drm_gem_ob /* Remove the fake offset */ vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); - if (obj->import_attach) + if (obj->import_attach) { + /* Drop the reference drm_gem_mmap_obj() acquired.*/ + drm_gem_object_put(obj); + vma->vm_private_data = NULL; + return dma_buf_mmap(obj->dma_buf, vma, 0); + } shmem = to_drm_gem_shmem_obj(obj);

3 years, 6 months

Patch "drm/shme-helpers: Fix dma_buf_mmap forwarding bug" has been added to the 5.9-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled drm/shme-helpers: Fix dma_buf_mmap forwarding bug to the 5.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: drm-shme-helpers-fix-dma_buf_mmap-forwarding-bug.patch and it can be found in the queue-5.9 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. >From f49a51bfdc8ea717c97ccd4cc98b7e6daaa5553a Mon Sep 17 00:00:00 2001 From: Daniel Vetter <daniel.vetter(a)ffwll.ch> Date: Tue, 27 Oct 2020 22:49:22 +0100 Subject: drm/shme-helpers: Fix dma_buf_mmap forwarding bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Daniel Vetter <daniel.vetter(a)ffwll.ch> commit f49a51bfdc8ea717c97ccd4cc98b7e6daaa5553a upstream. When we forward an mmap to the dma_buf exporter, they get to own everything. Unfortunately drm_gem_mmap_obj() overwrote vma->vm_private_data after the driver callback, wreaking the exporter complete. This was noticed because vb2_common_vm_close blew up on mali gpu with panfrost after commit 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf"). Unfortunately drm_gem_mmap_obj also acquires a surplus reference that we need to drop in shmem helpers, which is a bit of a mislayer situation. Maybe the entire dma_buf_mmap forwarding should be pulled into core gem code. Note that the only two other drivers which forward mmap in their own code (etnaviv and exynos) get this somewhat right by overwriting the gem mmap code. But they seem to still have the leak. This might be a good excuse to move these drivers over to shmem helpers completely. Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> Acked-by: Christian König <christian.koenig(a)amd.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Lucas Stach <l.stach(a)pengutronix.de> Cc: Russell King <linux+etnaviv(a)armlinux.org.uk> Cc: Christian Gmeiner <christian.gmeiner(a)gmail.com> Cc: Inki Dae <inki.dae(a)samsung.com> Cc: Joonyoung Shim <jy0922.shim(a)samsung.com> Cc: Seung-Woo Kim <sw0312.kim(a)samsung.com> Cc: Kyungmin Park <kyungmin.park(a)samsung.com> Fixes: 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf") Cc: Boris Brezillon <boris.brezillon(a)collabora.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Gerd Hoffmann <kraxel(a)redhat.com> Cc: Rob Herring <robh(a)kernel.org> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: <stable(a)vger.kernel.org> # v5.9+ Reported-and-tested-by: piotr.oniszczuk(a)gmail.com Cc: piotr.oniszczuk(a)gmail.com Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20201027214922.3566743-1-dani… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/gpu/drm/drm_gem.c | 4 ++-- drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1085,6 +1085,8 @@ int drm_gem_mmap_obj(struct drm_gem_obje */ drm_gem_object_get(obj); + vma->vm_private_data = obj; + if (obj->funcs && obj->funcs->mmap) { ret = obj->funcs->mmap(obj, vma); if (ret) { @@ -1107,8 +1109,6 @@ int drm_gem_mmap_obj(struct drm_gem_obje vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); } - vma->vm_private_data = obj; - return 0; } EXPORT_SYMBOL(drm_gem_mmap_obj); --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -594,8 +594,13 @@ int drm_gem_shmem_mmap(struct drm_gem_ob /* Remove the fake offset */ vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); - if (obj->import_attach) + if (obj->import_attach) { + /* Drop the reference drm_gem_mmap_obj() acquired.*/ + drm_gem_object_put(obj); + vma->vm_private_data = NULL; + return dma_buf_mmap(obj->dma_buf, vma, 0); + } shmem = to_drm_gem_shmem_obj(obj); Patches currently in stable-queue which might be from daniel.vetter(a)ffwll.ch are queue-5.9/drm-ast-separate-drm-driver-from-pci-code.patch queue-5.9/drm-shme-helpers-fix-dma_buf_mmap-forwarding-bug.patch

3 years, 6 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig November 2020