Thanks, I pushed v4 which moved the tracepoints into functions.
Cheers,
Tom
On 08/08/17 08:01 AM, Christian König wrote:
> Am 08.08.2017 um 13:54 schrieb Tom St Denis:
>> ping?
>
> Ups sorry, thought that I already send that out. One comment below.
>
>>
>> Tom
>>
>> On 02/08/17 07:52 AM, Tom St Denis wrote:
>>> This helps map DMA addresses back to physical addresses.
>>>
>>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
>>>
>>> (v2): Added tracepoints for USERPTR, SG mappings, and
>>> SWIOTBL mappings. Reformatted trace call perform
>>> PCI decoding internal to the trace.
>>>
>>> (v3): Add unmap tracepoints as well
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 56
>>> +++++++++++++++++++++++++++++++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 55
>>> ++++++++++++++++++++++++++----
>>> 2 files changed, 104 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>>> index 509f7a63d40c..0d708e8fb391 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>>> @@ -14,6 +14,62 @@
>>> #define AMDGPU_JOB_GET_TIMELINE_NAME(job) \
>>> job->base.s_fence->finished.ops->get_timeline_name(&job->base.s_fence->finished)
>>>
>>> +TRACE_EVENT(amdgpu_ttm_tt_populate,
>>> + TP_PROTO(struct amdgpu_device *adev, uint64_t dma_address,
>>> uint64_t phys_address),
>>> + TP_ARGS(adev, dma_address, phys_address),
>>> + TP_STRUCT__entry(
>>> + __field(uint16_t, domain)
>>> + __field(uint8_t, bus)
>>> + __field(uint8_t, slot)
>>> + __field(uint8_t, func)
>>> + __field(uint64_t, dma)
>>> + __field(uint64_t, phys)
>>> + ),
>>> + TP_fast_assign(
>>> + __entry->domain = pci_domain_nr(adev->pdev->bus);
>>> + __entry->bus = adev->pdev->bus->number;
>>> + __entry->slot = PCI_SLOT(adev->pdev->devfn);
>>> + __entry->func = PCI_FUNC(adev->pdev->devfn);
>>> + __entry->dma = dma_address;
>>> + __entry->phys = phys_address;
>>> + ),
>>> + TP_printk("%04x:%02x:%02x.%x: 0x%llx => 0x%llx",
>>> + (unsigned)__entry->domain,
>>> + (unsigned)__entry->bus,
>>> + (unsigned)__entry->slot,
>>> + (unsigned)__entry->func,
>>> + (unsigned long long)__entry->dma,
>>> + (unsigned long long)__entry->phys)
>>> +);
>>> +
>>> +TRACE_EVENT(amdgpu_ttm_tt_unpopulate,
>>> + TP_PROTO(struct amdgpu_device *adev, uint64_t dma_address,
>>> uint64_t phys_address),
>>> + TP_ARGS(adev, dma_address, phys_address),
>>> + TP_STRUCT__entry(
>>> + __field(uint16_t, domain)
>>> + __field(uint8_t, bus)
>>> + __field(uint8_t, slot)
>>> + __field(uint8_t, func)
>>> + __field(uint64_t, dma)
>>> + __field(uint64_t, phys)
>>> + ),
>>> + TP_fast_assign(
>>> + __entry->domain = pci_domain_nr(adev->pdev->bus);
>>> + __entry->bus = adev->pdev->bus->number;
>>> + __entry->slot = PCI_SLOT(adev->pdev->devfn);
>>> + __entry->func = PCI_FUNC(adev->pdev->devfn);
>>> + __entry->dma = dma_address;
>>> + __entry->phys = phys_address;
>>> + ),
>>> + TP_printk("%04x:%02x:%02x.%x: 0x%llx => 0x%llx",
>>> + (unsigned)__entry->domain,
>>> + (unsigned)__entry->bus,
>>> + (unsigned)__entry->slot,
>>> + (unsigned)__entry->func,
>>> + (unsigned long long)__entry->dma,
>>> + (unsigned long long)__entry->phys)
>>> +);
>>> +
>>> TRACE_EVENT(amdgpu_mm_rreg,
>>> TP_PROTO(unsigned did, uint32_t reg, uint32_t value),
>>> TP_ARGS(did, reg, value),
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 8da59d212b3b..d5f1b99b0ab2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -43,6 +43,7 @@
>>> #include <linux/pagemap.h>
>>> #include <linux/debugfs.h>
>>> #include "amdgpu.h"
>>> +#include "amdgpu_trace.h"
>>> #include "bif/bif_4_1_d.h"
>>> #define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
>>> @@ -667,7 +668,7 @@ static int amdgpu_ttm_tt_pin_userptr(struct
>>> ttm_tt *ttm)
>>> {
>>> struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
>>> struct amdgpu_ttm_tt *gtt = (void *)ttm;
>>> - unsigned nents;
>>> + unsigned i, nents;
>>> int r;
>>> int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
>>> @@ -688,6 +689,15 @@ static int amdgpu_ttm_tt_pin_userptr(struct
>>> ttm_tt *ttm)
>>> drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
>>> gtt->ttm.dma_address, ttm->num_pages);
>>> + if (unlikely(trace_amdgpu_ttm_tt_populate_enabled())) {
>>> + for (i = 0; i < ttm->num_pages; i++) {
>>> + trace_amdgpu_ttm_tt_populate(
>>> + adev,
>>> + gtt->ttm.dma_address[i],
>>> + page_to_phys(ttm->pages[i]));
>>> + }
>>> + }
>>> +
>
> It would still be nice to have that chunk in a separate function since
> it is used multiple times, but that's not a must have.
>
> Anyway patch is Reviewed-by: Christian König <christian.koenig at amd.com>
>
> Christian.
>
>>> return 0;
>>> release_sg:
>>> @@ -700,6 +710,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct
>>> ttm_tt *ttm)
>>> struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
>>> struct amdgpu_ttm_tt *gtt = (void *)ttm;
>>> struct sg_page_iter sg_iter;
>>> + unsigned i;
>>> int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
>>> enum dma_data_direction direction = write ?
>>> @@ -721,6 +732,16 @@ static void amdgpu_ttm_tt_unpin_userptr(struct
>>> ttm_tt *ttm)
>>> put_page(page);
>>> }
>>> + if (unlikely(trace_amdgpu_ttm_tt_unpopulate_enabled())) {
>>> + for (i = 0; i < ttm->num_pages; i++) {
>>> + trace_amdgpu_ttm_tt_unpopulate(
>>> + adev,
>>> + gtt->ttm.dma_address[i],
>>> + page_to_phys(ttm->pages[i]));
>>> + }
>>> + }
>>> +
>>> +
>>> sg_free_table(ttm->sg);
>>> }
>>> @@ -892,7 +913,7 @@ static struct ttm_tt
>>> *amdgpu_ttm_tt_create(struct ttm_bo_device *bdev,
>>> static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm)
>>> {
>>> - struct amdgpu_device *adev;
>>> + struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
>>> struct amdgpu_ttm_tt *gtt = (void *)ttm;
>>> unsigned i;
>>> int r;
>>> @@ -915,14 +936,14 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt
>>> *ttm)
>>> drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
>>> gtt->ttm.dma_address, ttm->num_pages);
>>> ttm->state = tt_unbound;
>>> - return 0;
>>> + r = 0;
>>> + goto trace_mappings;
>>> }
>>> - adev = amdgpu_ttm_adev(ttm->bdev);
>>> -
>>> #ifdef CONFIG_SWIOTLB
>>> if (swiotlb_nr_tbl()) {
>>> - return ttm_dma_populate(>t->ttm, adev->dev);
>>> + r = ttm_dma_populate(>t->ttm, adev->dev);
>>> + goto trace_mappings;
>>> }
>>> #endif
>>> @@ -945,7 +966,18 @@ static int amdgpu_ttm_tt_populate(struct
>>> ttm_tt *ttm)
>>> return -EFAULT;
>>> }
>>> }
>>> - return 0;
>>> +
>>> + r = 0;
>>> +trace_mappings:
>>> + if (!r && unlikely(trace_amdgpu_ttm_tt_populate_enabled())) {
>>> + for (i = 0; i < ttm->num_pages; i++) {
>>> + trace_amdgpu_ttm_tt_populate(
>>> + adev,
>>> + gtt->ttm.dma_address[i],
>>> + page_to_phys(ttm->pages[i]));
>>> + }
>>> + }
>>> + return r;
>>> }
>>> static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm)
>>> @@ -966,6 +998,15 @@ static void amdgpu_ttm_tt_unpopulate(struct
>>> ttm_tt *ttm)
>>> adev = amdgpu_ttm_adev(ttm->bdev);
>>> + if (unlikely(trace_amdgpu_ttm_tt_unpopulate_enabled())) {
>>> + for (i = 0; i < ttm->num_pages; i++) {
>>> + trace_amdgpu_ttm_tt_unpopulate(
>>> + adev,
>>> + gtt->ttm.dma_address[i],
>>> + page_to_phys(ttm->pages[i]));
>>> + }
>>> + }
>>> +
>>> #ifdef CONFIG_SWIOTLB
>>> if (swiotlb_nr_tbl()) {
>>> ttm_dma_unpopulate(>t->ttm, adev->dev);
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
