Am 18.08.2017 um 14:54 schrieb Tom St Denis: > Also exports two functions that vendor drivers can call > to trace DMA mappings. This is meant to help translate > IOMMU mappings of bus addresses back to physical pages. > > Used by the umr amdgpu debugger for instance. Sorry to have even more notes on this, but just a few more minor nit picks below. > > Signed-off-by: Tom St Denis <tom.stdenis at amd.com> > > (v2): Use dev_name() to get PCI path instead. > --- > drivers/gpu/drm/ttm/Makefile | 4 +- > drivers/gpu/drm/ttm/ttm_debug.c | 75 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/ttm/ttm_trace.h | 87 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/ttm/ttm_tracepoints.c | 46 ++++++++++++++++++ > include/drm/ttm/ttm_debug.h | 31 +++++++++++++ > 5 files changed, 241 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/ttm/ttm_debug.c > create mode 100644 drivers/gpu/drm/ttm/ttm_trace.h > create mode 100644 drivers/gpu/drm/ttm/ttm_tracepoints.c > create mode 100644 include/drm/ttm/ttm_debug.h > > diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile > index f92325800f8a..fd3da00c0bf2 100644 > --- a/drivers/gpu/drm/ttm/Makefile > +++ b/drivers/gpu/drm/ttm/Makefile > @@ -1,11 +1,11 @@ > # > # Makefile for the drm device driver. This driver provides support for the > > -ccflags-y := -Iinclude/drm > +ccflags-y := -Iinclude/drm -I$(src)/. > ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \ > ttm_bo_util.o ttm_bo_vm.o ttm_module.o \ > ttm_object.o ttm_lock.o ttm_execbuf_util.o ttm_page_alloc.o \ > - ttm_bo_manager.o ttm_page_alloc_dma.o > + ttm_bo_manager.o ttm_page_alloc_dma.o ttm_debug.o ttm_tracepoints.o > ttm-$(CONFIG_AGP) += ttm_agp_backend.o > > obj-$(CONFIG_DRM_TTM) += ttm.o > diff --git a/drivers/gpu/drm/ttm/ttm_debug.c b/drivers/gpu/drm/ttm/ttm_debug.c > new file mode 100644 > index 000000000000..dd158c6ef90d > --- /dev/null > +++ b/drivers/gpu/drm/ttm/ttm_debug.c > @@ -0,0 +1,75 @@ > +/************************************************************************** > + * > + * Copyright (c) 2017 Advanced Micro Devices, Inc. > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sub license, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portions > + * of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE > + * USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + **************************************************************************/ > +/* > + * Authors: Tom St Denis <tom.stdenis at amd.com> > + */ > +#include <linux/sched.h> > +#include <linux/highmem.h> > +#include <linux/pagemap.h> > +#include <linux/shmem_fs.h> > +#include <linux/file.h> > +#include <linux/swap.h> > +#include <linux/slab.h> > +#include <linux/export.h> > +#include <drm/drm_cache.h> > +#include <drm/drm_mem_util.h> > +#include <drm/ttm/ttm_module.h> > +#include <drm/ttm/ttm_bo_driver.h> > +#include <drm/ttm/ttm_placement.h> > +#include <drm/ttm/ttm_page_alloc.h> > +#include "ttm_trace.h" > + > +void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt) > +{ > + unsigned i; > + > + if (unlikely(trace_ttm_dma_map_enabled())) { > + for (i = 0; i < tt->ttm.num_pages; i++) { > + trace_ttm_dma_map( > + dev, > + tt->ttm.pages[i], > + tt->dma_address[i]); > + } > + } > +} > +EXPORT_SYMBOL(ttm_trace_dma_map); > + > +void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt) > +{ > + unsigned i; > + > + if (unlikely(trace_ttm_dma_unmap_enabled())) { > + for (i = 0; i < tt->ttm.num_pages; i++) { > + trace_ttm_dma_unmap( > + dev, > + tt->ttm.pages[i], > + tt->dma_address[i]); > + } > + } > +} > +EXPORT_SYMBOL(ttm_trace_dma_unmap); > + > diff --git a/drivers/gpu/drm/ttm/ttm_trace.h b/drivers/gpu/drm/ttm/ttm_trace.h > new file mode 100644 > index 000000000000..0d63ce2a5f11 > --- /dev/null > +++ b/drivers/gpu/drm/ttm/ttm_trace.h > @@ -0,0 +1,87 @@ > +/************************************************************************** > + * > + * Copyright (c) 2017 Advanced Micro Devices, Inc. > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sub license, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portions > + * of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE > + * USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + **************************************************************************/ > +/* > + * Authors: Tom St Denis <tom.stdenis at amd.com> > + */ > +#if !defined(_TTM_TRACE_H) || defined(TRACE_HEADER_MULTI_READ) > +#define _TTM_TRACE_H_ > + > +#include <linux/stringify.h> > +#include <linux/types.h> > +#include <linux/tracepoint.h> > + > +#include <drm/drmP.h> > + > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM ttm > +#define TRACE_INCLUDE_FILE ttm_trace > + > +TRACE_EVENT(ttm_dma_map, > + TP_PROTO(struct device *dev, struct page *page, uint64_t dma_address), > + TP_ARGS(dev, page, dma_address), > + TP_STRUCT__entry( > + __string(device, dev_name(dev)) > + __field(uint64_t, dma) That should have dma_addr_t. > + __field(uint64_t, phys) And that one should have phys_addr_t. > + ), > + TP_fast_assign( > + __assign_str(device, dev_name(dev)); > + __entry->dma = dma_address; > + __entry->phys = page_to_phys(page); > + ), > + TP_printk("%s: 0x%llx => 0x%llx", > + __get_str(device), > + (unsigned long long)__entry->dma, > + (unsigned long long)__entry->phys) From printk-formats.txt: > Physical addresses types ``phys_addr_t`` > ======================================== > > :: > > %pa[p] 0x01234567 or 0x0123456789abcdef > > For printing a ``phys_addr_t`` type (and its derivatives, such as > ``resource_size_t``) which can vary based on build options, regardless of > the width of the CPU data path. Passed by reference. > > DMA addresses types ``dma_addr_t`` > ================================== > > :: > > %pad 0x01234567 or 0x0123456789abcdef > > For printing a ``dma_addr_t`` type which can vary based on build options, > regardless of the width of the CPU data path. Passed by reference. Those should work here as well, so I think we should use them instead of the cast. Has the clear advantage that this even looks good on 32bit machines. Regards, Christian. > +); > + > +TRACE_EVENT(ttm_dma_unmap, > + TP_PROTO(struct device *dev, struct page *page, uint64_t dma_address), > + TP_ARGS(dev, page, dma_address), > + TP_STRUCT__entry( > + __string(device, dev_name(dev)) > + __field(uint64_t, dma) > + __field(uint64_t, phys) > + ), > + TP_fast_assign( > + __assign_str(device, dev_name(dev)); > + __entry->dma = dma_address; > + __entry->phys = page_to_phys(page); > + ), > + TP_printk("%s: 0x%llx => 0x%llx", > + __get_str(device), > + (unsigned long long)__entry->dma, > + (unsigned long long)__entry->phys) > +); > + > +#endif > + > +/* This part must be outside protection */ > +#undef TRACE_INCLUDE_PATH > +#define TRACE_INCLUDE_PATH . > +#include <trace/define_trace.h> > + > diff --git a/drivers/gpu/drm/ttm/ttm_tracepoints.c b/drivers/gpu/drm/ttm/ttm_tracepoints.c > new file mode 100644 > index 000000000000..c13b9f5b6496 > --- /dev/null > +++ b/drivers/gpu/drm/ttm/ttm_tracepoints.c > @@ -0,0 +1,46 @@ > +/************************************************************************** > + * > + * Copyright (c) 2017 Advanced Micro Devices, Inc. > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sub license, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portions > + * of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE > + * USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + **************************************************************************/ > +/* > + * Authors: Tom St Denis <tom.stdenis at amd.com> > + */ > +#include <linux/sched.h> > +#include <linux/highmem.h> > +#include <linux/pagemap.h> > +#include <linux/shmem_fs.h> > +#include <linux/file.h> > +#include <linux/swap.h> > +#include <linux/slab.h> > +#include <linux/export.h> > +#include <drm/drm_cache.h> > +#include <drm/drm_mem_util.h> > +#include <drm/ttm/ttm_module.h> > +#include <drm/ttm/ttm_bo_driver.h> > +#include <drm/ttm/ttm_placement.h> > +#include <drm/ttm/ttm_page_alloc.h> > + > +#define CREATE_TRACE_POINTS > +#include "ttm_trace.h" > diff --git a/include/drm/ttm/ttm_debug.h b/include/drm/ttm/ttm_debug.h > new file mode 100644 > index 000000000000..b5e460fa5086 > --- /dev/null > +++ b/include/drm/ttm/ttm_debug.h > @@ -0,0 +1,31 @@ > +/************************************************************************** > + * > + * Copyright (c) 2017 Advanced Micro Devices, Inc. > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sub license, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portions > + * of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE > + * USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + **************************************************************************/ > +/* > + * Authors: Tom St Denis <tom.stdenis at amd.com> > + */ > +extern void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt); > +extern void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt);