Re: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc
On 2018-12-14 2:31 p.m., Wentland, Harry wrote: > On 2018-12-07 3:32 p.m., Kuehling, Felix wrote: >> On 2018-12-07 9:46 a.m., Wentland, Harry wrote: >>> On 2018-12-07 9:41 a.m., Wentland, Harry wrote: >>>> On 2018-12-07 12:40 a.m., Kuehling, Felix wrote: >>>>> This change seems to be breaking the build for me. I'm getting errors >>>>> like this: >>>>> >>>>> CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o >>>>> In file included from ../include/trace/events/tlb.h:9:0, >>>>>from ../arch/x86/include/asm/mmu_context.h:10, >>>>>from ../include/linux/mmu_context.h:5, >>>>>from ../drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h:30, >>>>>from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:85, >>>>>from >>>>> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:34: >>>>> ../include/linux/tracepoint.h:285:20: error: redefinition of >>>>> â__tpstrtab_amdgpu_dc_rregâ >>>>> static const char __tpstrtab_##name[] \ >>>>> ^ >>>>> ../include/linux/tracepoint.h:293:2: note: in expansion of macro >>>>> âDEFINE_TRACE_FNâ >>>>> DEFINE_TRACE_FN(name, NULL, NULL); >>>>> ^ >>>>> ../include/trace/define_trace.h:28:2: note: in expansion of macro >>>>> âDEFINE_TRACEâ >>>>> DEFINE_TRACE(name) >>>>> ^ >>>>> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/./amdgpu_dm_trace.h:34:1: >>>>> note: in expansion of macro âTRACE_EVENTâ >>>>>TRACE_EVENT(amdgpu_dc_rreg, >>>>>^ >>>>> >>>>> I have a local change that adds #include to amdgpu.h, >>>>> which pulls in include/trace/events/tlb.h. That seems to create some kind >>>>> of conflict with trace definitions. Any ideas how to fix this? It seems a >>>>> bit fragile if adding some random include can break the build like this. >>>>> >>>> That's the trace subsystem for you. David and I are trying to understand >>>> what's going on. I think the problem is that both tlb.h and >>>> amdgpu_dm_trace.h define trace events and we now include both here. >>>> >>>> I think we'd want to include neither trace events from amdgpu.h to avoid >>>> this problem in the future, so we'll probably have to (a) clean up the >>>> dc.h include to only contain what amdgpu.h needs and (b) clean up >>>> amdgpu_amdkfd.h to only contain what amdgpu.h needs. At the end amdgpu.h >>>> doesn't care about the tracer. The problem seems that dc.h and >>>> amdgpu_amdkfd.h are the main includes for our respective drivers but are >>>> also wholesale included in amdgpu.h. >>>> >>> Apologies for the spam. >>> >>> Just noticed that amdgpu.h includes only amdgpu_dm.h which is clean. The >>> problem is that including amdgpu.h from amdgpu_dm.c now pulls in your trace >>> events (from tlb.h) which we don't expect and care about. I think we should >>> make sure amdgpu.h won't include anything that defines TRACE_EVENTs. >> amdgpu_amdkfd.h defines a macro that uses use_mm and unuse_mm. Therefore >> it needs to include mmu_context.h, which pulls in the conflicting trace >> events. Maybe we can move that into a different header file that doesn't >> get included in amdgpu.h. Or find another way to avoid including >> amdgpu_amdkfd.h in amdgpu.h. >> > It's defined but not used in amdgpu_amdkfd.h (at least in the amd-stg tree). > Couldn't you include mmu_context.h in the files that use it > (amdgpu_amdkfd_gfx_v7/8/9.c) instead and avoid the problem that way? Yes, that's what I ended up doing. I already fixed it in this commit: commit 2a90860cfb895ff55d961a1b727d58bb68e213ba Author: Felix Kuehling Date: Fri Dec 7 17:01:27 2018 -0500 drm/amdgpu: Workaround build failure due to trace conflict Avoid including mmu_context.h in amdgpu_amdkfd.h since that may be included in other header files that define traces. This leads to conflicts due to traces defined in other headers included via mmu_context.h. Signed-off-by: Felix Kuehling Acked-by: Alex Deucher Regards, Felix > > Harry > >> Thanks, >> Felix >> >> >> >>> Harry >>> >>>
Re: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc
On 2018-12-07 3:32 p.m., Kuehling, Felix wrote: > On 2018-12-07 9:46 a.m., Wentland, Harry wrote: >> On 2018-12-07 9:41 a.m., Wentland, Harry wrote: >>> On 2018-12-07 12:40 a.m., Kuehling, Felix wrote: >>>> This change seems to be breaking the build for me. I'm getting errors like >>>> this: >>>> >>>> CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o >>>> In file included from ../include/trace/events/tlb.h:9:0, >>>>from ../arch/x86/include/asm/mmu_context.h:10, >>>>from ../include/linux/mmu_context.h:5, >>>>from ../drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h:30, >>>>from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:85, >>>>from >>>> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:34: >>>> ../include/linux/tracepoint.h:285:20: error: redefinition of >>>> â__tpstrtab_amdgpu_dc_rregâ >>>> static const char __tpstrtab_##name[] \ >>>> ^ >>>> ../include/linux/tracepoint.h:293:2: note: in expansion of macro >>>> âDEFINE_TRACE_FNâ >>>> DEFINE_TRACE_FN(name, NULL, NULL); >>>> ^ >>>> ../include/trace/define_trace.h:28:2: note: in expansion of macro >>>> âDEFINE_TRACEâ >>>> DEFINE_TRACE(name) >>>> ^ >>>> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/./amdgpu_dm_trace.h:34:1: >>>> note: in expansion of macro âTRACE_EVENTâ >>>>TRACE_EVENT(amdgpu_dc_rreg, >>>>^ >>>> >>>> I have a local change that adds #include to amdgpu.h, >>>> which pulls in include/trace/events/tlb.h. That seems to create some kind >>>> of conflict with trace definitions. Any ideas how to fix this? It seems a >>>> bit fragile if adding some random include can break the build like this. >>>> >>> That's the trace subsystem for you. David and I are trying to understand >>> what's going on. I think the problem is that both tlb.h and >>> amdgpu_dm_trace.h define trace events and we now include both here. >>> >>> I think we'd want to include neither trace events from amdgpu.h to avoid >>> this problem in the future, so we'll probably have to (a) clean up the dc.h >>> include to only contain what amdgpu.h needs and (b) clean up >>> amdgpu_amdkfd.h to only contain what amdgpu.h needs. At the end amdgpu.h >>> doesn't care about the tracer. The problem seems that dc.h and >>> amdgpu_amdkfd.h are the main includes for our respective drivers but are >>> also wholesale included in amdgpu.h. >>> >> Apologies for the spam. >> >> Just noticed that amdgpu.h includes only amdgpu_dm.h which is clean. The >> problem is that including amdgpu.h from amdgpu_dm.c now pulls in your trace >> events (from tlb.h) which we don't expect and care about. I think we should >> make sure amdgpu.h won't include anything that defines TRACE_EVENTs. > > amdgpu_amdkfd.h defines a macro that uses use_mm and unuse_mm. Therefore > it needs to include mmu_context.h, which pulls in the conflicting trace > events. Maybe we can move that into a different header file that doesn't > get included in amdgpu.h. Or find another way to avoid including > amdgpu_amdkfd.h in amdgpu.h. > It's defined but not used in amdgpu_amdkfd.h (at least in the amd-stg tree). Couldn't you include mmu_context.h in the files that use it (amdgpu_amdkfd_gfx_v7/8/9.c) instead and avoid the problem that way? Harry > Thanks, > Felix > > > >> >> Harry >> >>> Harry >>> >>>> Thanks, >>>> Felix >>>> >>>> -Original Message- >>>> From: amd-gfx On Behalf Of David >>>> Francis >>>> Sent: Friday, November 30, 2018 9:57 AM >>>> To: amd-gfx@lists.freedesktop.org >>>> Cc: Francis, David >>>> Subject: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc >>>> >>>> [Why] >>>> Tracing is a useful and cheap debug functionality >>>> >>>> [How] >>>> This creates a new trace system amdgpu_dm, currently with three trace >>>> events >>>> >>>> amdgpu_dc_rreg and amdgpu_dc_wreg report the address and value of any dc >>>> register reads and writes >>>> >>>> amdgpu_dc_performance
Re: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc
On 2018-12-07 9:46 a.m., Wentland, Harry wrote: > On 2018-12-07 9:41 a.m., Wentland, Harry wrote: >> On 2018-12-07 12:40 a.m., Kuehling, Felix wrote: >>> This change seems to be breaking the build for me. I'm getting errors like >>> this: >>> >>> CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o >>> In file included from ../include/trace/events/tlb.h:9:0, >>>from ../arch/x86/include/asm/mmu_context.h:10, >>>from ../include/linux/mmu_context.h:5, >>>from ../drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h:30, >>>from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:85, >>>from >>> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:34: >>> ../include/linux/tracepoint.h:285:20: error: redefinition of >>> â__tpstrtab_amdgpu_dc_rregâ >>> static const char __tpstrtab_##name[] \ >>> ^ >>> ../include/linux/tracepoint.h:293:2: note: in expansion of macro >>> âDEFINE_TRACE_FNâ >>> DEFINE_TRACE_FN(name, NULL, NULL); >>> ^ >>> ../include/trace/define_trace.h:28:2: note: in expansion of macro >>> âDEFINE_TRACEâ >>> DEFINE_TRACE(name) >>> ^ >>> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/./amdgpu_dm_trace.h:34:1: >>> note: in expansion of macro âTRACE_EVENTâ >>>TRACE_EVENT(amdgpu_dc_rreg, >>>^ >>> >>> I have a local change that adds #include to amdgpu.h, >>> which pulls in include/trace/events/tlb.h. That seems to create some kind >>> of conflict with trace definitions. Any ideas how to fix this? It seems a >>> bit fragile if adding some random include can break the build like this. >>> >> That's the trace subsystem for you. David and I are trying to understand >> what's going on. I think the problem is that both tlb.h and >> amdgpu_dm_trace.h define trace events and we now include both here. >> >> I think we'd want to include neither trace events from amdgpu.h to avoid >> this problem in the future, so we'll probably have to (a) clean up the dc.h >> include to only contain what amdgpu.h needs and (b) clean up amdgpu_amdkfd.h >> to only contain what amdgpu.h needs. At the end amdgpu.h doesn't care about >> the tracer. The problem seems that dc.h and amdgpu_amdkfd.h are the main >> includes for our respective drivers but are also wholesale included in >> amdgpu.h. >> > Apologies for the spam. > > Just noticed that amdgpu.h includes only amdgpu_dm.h which is clean. The > problem is that including amdgpu.h from amdgpu_dm.c now pulls in your trace > events (from tlb.h) which we don't expect and care about. I think we should > make sure amdgpu.h won't include anything that defines TRACE_EVENTs. amdgpu_amdkfd.h defines a macro that uses use_mm and unuse_mm. Therefore it needs to include mmu_context.h, which pulls in the conflicting trace events. Maybe we can move that into a different header file that doesn't get included in amdgpu.h. Or find another way to avoid including amdgpu_amdkfd.h in amdgpu.h. Thanks, Felix > > Harry > >> Harry >> >>> Thanks, >>> Felix >>> >>> -Original Message- >>> From: amd-gfx On Behalf Of David >>> Francis >>> Sent: Friday, November 30, 2018 9:57 AM >>> To: amd-gfx@lists.freedesktop.org >>> Cc: Francis, David >>> Subject: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc >>> >>> [Why] >>> Tracing is a useful and cheap debug functionality >>> >>> [How] >>> This creates a new trace system amdgpu_dm, currently with three trace events >>> >>> amdgpu_dc_rreg and amdgpu_dc_wreg report the address and value of any dc >>> register reads and writes >>> >>> amdgpu_dc_performance requires at least one of those two to be enabled. It >>> counts the register reads and writes since the last entry >>> >>> v2: Don't check for NULL before kfree >>> >>> Signed-off-by: David Francis >>> Reviewed-by: Harry Wentland >>> Acked-by: Leo Li >>> --- >>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 + >>>.../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 104 ++ >>>drivers/gpu/drm/amd/display/dc/core/dc.c | 19 >>>drivers/gpu/drm/amd/display/dc/dc_types.h | 8 ++ >>>.../amd/d
Re: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc
On 2018-12-07 9:41 a.m., Wentland, Harry wrote: > On 2018-12-07 12:40 a.m., Kuehling, Felix wrote: >> This change seems to be breaking the build for me. I'm getting errors like >> this: >> >> CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o >> In file included from ../include/trace/events/tlb.h:9:0, >>from ../arch/x86/include/asm/mmu_context.h:10, >>from ../include/linux/mmu_context.h:5, >>from ../drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h:30, >>from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:85, >>from >> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:34: >> ../include/linux/tracepoint.h:285:20: error: redefinition of >> â__tpstrtab_amdgpu_dc_rregâ >> static const char __tpstrtab_##name[] \ >> ^ >> ../include/linux/tracepoint.h:293:2: note: in expansion of macro >> âDEFINE_TRACE_FNâ >> DEFINE_TRACE_FN(name, NULL, NULL); >> ^ >> ../include/trace/define_trace.h:28:2: note: in expansion of macro >> âDEFINE_TRACEâ >> DEFINE_TRACE(name) >> ^ >> ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/./amdgpu_dm_trace.h:34:1: >> note: in expansion of macro âTRACE_EVENTâ >>TRACE_EVENT(amdgpu_dc_rreg, >>^ >> >> I have a local change that adds #include to amdgpu.h, >> which pulls in include/trace/events/tlb.h. That seems to create some kind of >> conflict with trace definitions. Any ideas how to fix this? It seems a bit >> fragile if adding some random include can break the build like this. >> > > That's the trace subsystem for you. David and I are trying to understand > what's going on. I think the problem is that both tlb.h and amdgpu_dm_trace.h > define trace events and we now include both here. > > I think we'd want to include neither trace events from amdgpu.h to avoid this > problem in the future, so we'll probably have to (a) clean up the dc.h > include to only contain what amdgpu.h needs and (b) clean up amdgpu_amdkfd.h > to only contain what amdgpu.h needs. At the end amdgpu.h doesn't care about > the tracer. The problem seems that dc.h and amdgpu_amdkfd.h are the main > includes for our respective drivers but are also wholesale included in > amdgpu.h. > Apologies for the spam. Just noticed that amdgpu.h includes only amdgpu_dm.h which is clean. The problem is that including amdgpu.h from amdgpu_dm.c now pulls in your trace events (from tlb.h) which we don't expect and care about. I think we should make sure amdgpu.h won't include anything that defines TRACE_EVENTs. Harry > Harry > >> Thanks, >> Felix >> >> -Original Message- >> From: amd-gfx On Behalf Of David >> Francis >> Sent: Friday, November 30, 2018 9:57 AM >> To: amd-gfx@lists.freedesktop.org >> Cc: Francis, David >> Subject: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc >> >> [Why] >> Tracing is a useful and cheap debug functionality >> >> [How] >> This creates a new trace system amdgpu_dm, currently with three trace events >> >> amdgpu_dc_rreg and amdgpu_dc_wreg report the address and value of any dc >> register reads and writes >> >> amdgpu_dc_performance requires at least one of those two to be enabled. It >> counts the register reads and writes since the last entry >> >> v2: Don't check for NULL before kfree >> >> Signed-off-by: David Francis >> Reviewed-by: Harry Wentland >> Acked-by: Leo Li >> --- >>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 + >>.../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 104 ++ >>drivers/gpu/drm/amd/display/dc/core/dc.c | 19 >>drivers/gpu/drm/amd/display/dc/dc_types.h | 8 ++ >>.../amd/display/dc/dcn10/dcn10_cm_common.c| 4 +- >>drivers/gpu/drm/amd/display/dc/dm_services.h | 12 +- >>6 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h >> >> 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 76b1aebdca0c..376927c8bcc6 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -23,6 +23,9 @@ >> * >> */ >> >> +/* The caprices of the preprocessor require that this be declared right >> +here */ #define
Re: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc
On 2018-12-07 12:40 a.m., Kuehling, Felix wrote: > This change seems to be breaking the build for me. I'm getting errors like > this: > >CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o > In file included from ../include/trace/events/tlb.h:9:0, > from ../arch/x86/include/asm/mmu_context.h:10, > from ../include/linux/mmu_context.h:5, > from ../drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h:30, > from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:85, > from > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:34: > ../include/linux/tracepoint.h:285:20: error: redefinition of > â__tpstrtab_amdgpu_dc_rregâ >static const char __tpstrtab_##name[] \ > ^ > ../include/linux/tracepoint.h:293:2: note: in expansion of macro > âDEFINE_TRACE_FNâ >DEFINE_TRACE_FN(name, NULL, NULL); >^ > ../include/trace/define_trace.h:28:2: note: in expansion of macro > âDEFINE_TRACEâ >DEFINE_TRACE(name) >^ > ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/./amdgpu_dm_trace.h:34:1: > note: in expansion of macro âTRACE_EVENTâ > TRACE_EVENT(amdgpu_dc_rreg, > ^ > > I have a local change that adds #include to amdgpu.h, which > pulls in include/trace/events/tlb.h. That seems to create some kind of > conflict with trace definitions. Any ideas how to fix this? It seems a bit > fragile if adding some random include can break the build like this. > That's the trace subsystem for you. David and I are trying to understand what's going on. I think the problem is that both tlb.h and amdgpu_dm_trace.h define trace events and we now include both here. I think we'd want to include neither trace events from amdgpu.h to avoid this problem in the future, so we'll probably have to (a) clean up the dc.h include to only contain what amdgpu.h needs and (b) clean up amdgpu_amdkfd.h to only contain what amdgpu.h needs. At the end amdgpu.h doesn't care about the tracer. The problem seems that dc.h and amdgpu_amdkfd.h are the main includes for our respective drivers but are also wholesale included in amdgpu.h. Harry > Thanks, >Felix > > -Original Message- > From: amd-gfx On Behalf Of David > Francis > Sent: Friday, November 30, 2018 9:57 AM > To: amd-gfx@lists.freedesktop.org > Cc: Francis, David > Subject: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc > > [Why] > Tracing is a useful and cheap debug functionality > > [How] > This creates a new trace system amdgpu_dm, currently with three trace events > > amdgpu_dc_rreg and amdgpu_dc_wreg report the address and value of any dc > register reads and writes > > amdgpu_dc_performance requires at least one of those two to be enabled. It > counts the register reads and writes since the last entry > > v2: Don't check for NULL before kfree > > Signed-off-by: David Francis > Reviewed-by: Harry Wentland > Acked-by: Leo Li > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 + > .../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 104 ++ > drivers/gpu/drm/amd/display/dc/core/dc.c | 19 > drivers/gpu/drm/amd/display/dc/dc_types.h | 8 ++ > .../amd/display/dc/dcn10/dcn10_cm_common.c| 4 +- > drivers/gpu/drm/amd/display/dc/dm_services.h | 12 +- > 6 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h > > 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 76b1aebdca0c..376927c8bcc6 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -23,6 +23,9 @@ >* >*/ > > +/* The caprices of the preprocessor require that this be declared right > +here */ #define CREATE_TRACE_POINTS > + > #include "dm_services_types.h" > #include "dc.h" > #include "dc/inc/core_types.h" > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h > new file mode 100644 > index ..d898981684d5 > --- /dev/null > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h > @@ -0,0 +1,104 @@ > +/* > + * Copyright 2018 Advanced Micro Devices, Inc. > + * > + * 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 > +lim
RE: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc
This change seems to be breaking the build for me. I'm getting errors like this: CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o In file included from ../include/trace/events/tlb.h:9:0, from ../arch/x86/include/asm/mmu_context.h:10, from ../include/linux/mmu_context.h:5, from ../drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h:30, from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:85, from ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:34: ../include/linux/tracepoint.h:285:20: error: redefinition of â__tpstrtab_amdgpu_dc_rregâ static const char __tpstrtab_##name[] \ ^ ../include/linux/tracepoint.h:293:2: note: in expansion of macro âDEFINE_TRACE_FNâ DEFINE_TRACE_FN(name, NULL, NULL); ^ ../include/trace/define_trace.h:28:2: note: in expansion of macro âDEFINE_TRACEâ DEFINE_TRACE(name) ^ ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/./amdgpu_dm_trace.h:34:1: note: in expansion of macro âTRACE_EVENTâ TRACE_EVENT(amdgpu_dc_rreg, ^ I have a local change that adds #include to amdgpu.h, which pulls in include/trace/events/tlb.h. That seems to create some kind of conflict with trace definitions. Any ideas how to fix this? It seems a bit fragile if adding some random include can break the build like this. Thanks, Felix -Original Message- From: amd-gfx On Behalf Of David Francis Sent: Friday, November 30, 2018 9:57 AM To: amd-gfx@lists.freedesktop.org Cc: Francis, David Subject: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc [Why] Tracing is a useful and cheap debug functionality [How] This creates a new trace system amdgpu_dm, currently with three trace events amdgpu_dc_rreg and amdgpu_dc_wreg report the address and value of any dc register reads and writes amdgpu_dc_performance requires at least one of those two to be enabled. It counts the register reads and writes since the last entry v2: Don't check for NULL before kfree Signed-off-by: David Francis Reviewed-by: Harry Wentland Acked-by: Leo Li --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 + .../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 104 ++ drivers/gpu/drm/amd/display/dc/core/dc.c | 19 drivers/gpu/drm/amd/display/dc/dc_types.h | 8 ++ .../amd/display/dc/dcn10/dcn10_cm_common.c| 4 +- drivers/gpu/drm/amd/display/dc/dm_services.h | 12 +- 6 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 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 76b1aebdca0c..376927c8bcc6 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -23,6 +23,9 @@ * */ +/* The caprices of the preprocessor require that this be declared right +here */ #define CREATE_TRACE_POINTS + #include "dm_services_types.h" #include "dc.h" #include "dc/inc/core_types.h" diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h new file mode 100644 index ..d898981684d5 --- /dev/null +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h @@ -0,0 +1,104 @@ +/* + * Copyright 2018 Advanced Micro Devices, Inc. + * + * 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, +sublicense, + * 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 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 NONINFRINGEMENT. IN NO EVENT +SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) 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: AMD + * + */ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM amdgpu_dm + +#if !defined(_AMDGPU_DM_TRACE_H) || defined(TRACE_HEADER_MULTI_READ) +#define _AMDGPU_DM_TRACE_H_ + +#include + +TRACE_EVENT(amdgpu_dc_rreg, + TP_PROTO(unsigned long *read_count, uint32_t reg, uint32_t value), + TP_ARGS(read_count, reg, value), + TP_STRUCT__entry( +
[PATCH 04/16 v2] drm/amd/display: Add tracing to dc
[Why]
Tracing is a useful and cheap debug functionality
[How]
This creates a new trace system amdgpu_dm, currently with
three trace events
amdgpu_dc_rreg and amdgpu_dc_wreg report the address and value
of any dc register reads and writes
amdgpu_dc_performance requires at least one of those two to be
enabled. It counts the register reads and writes since the
last entry
v2: Don't check for NULL before kfree
Signed-off-by: David Francis
Reviewed-by: Harry Wentland
Acked-by: Leo Li
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +
.../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 104 ++
drivers/gpu/drm/amd/display/dc/core/dc.c | 19
drivers/gpu/drm/amd/display/dc/dc_types.h | 8 ++
.../amd/display/dc/dcn10/dcn10_cm_common.c| 4 +-
drivers/gpu/drm/amd/display/dc/dm_services.h | 12 +-
6 files changed, 146 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
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 76b1aebdca0c..376927c8bcc6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -23,6 +23,9 @@
*
*/
+/* The caprices of the preprocessor require that this be declared right here */
+#define CREATE_TRACE_POINTS
+
#include "dm_services_types.h"
#include "dc.h"
#include "dc/inc/core_types.h"
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
new file mode 100644
index ..d898981684d5
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -0,0 +1,104 @@
+/*
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ *
+ * 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, sublicense,
+ * 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 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 NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) 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: AMD
+ *
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM amdgpu_dm
+
+#if !defined(_AMDGPU_DM_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _AMDGPU_DM_TRACE_H_
+
+#include
+
+TRACE_EVENT(amdgpu_dc_rreg,
+ TP_PROTO(unsigned long *read_count, uint32_t reg, uint32_t value),
+ TP_ARGS(read_count, reg, value),
+ TP_STRUCT__entry(
+ __field(uint32_t, reg)
+ __field(uint32_t, value)
+ ),
+ TP_fast_assign(
+ __entry->reg = reg;
+ __entry->value = value;
+ *read_count = *read_count + 1;
+ ),
+ TP_printk("reg=0x%08lx, value=0x%08lx",
+ (unsigned long)__entry->reg,
+ (unsigned long)__entry->value)
+);
+
+TRACE_EVENT(amdgpu_dc_wreg,
+ TP_PROTO(unsigned long *write_count, uint32_t reg, uint32_t value),
+ TP_ARGS(write_count, reg, value),
+ TP_STRUCT__entry(
+ __field(uint32_t, reg)
+ __field(uint32_t, value)
+ ),
+ TP_fast_assign(
+ __entry->reg = reg;
+ __entry->value = value;
+ *write_count = *write_count + 1;
+ ),
+ TP_printk("reg=0x%08lx, value=0x%08lx",
+ (unsigned long)__entry->reg,
+ (unsigned long)__entry->value)
+);
+
+
+TRACE_EVENT(amdgpu_dc_performance,
+ TP_PROTO(unsigned long read_count, unsigned long write_count,
+ unsigned long *last_read, unsigned long *last_write,
+ const char *func, unsigned int line),
+ TP_ARGS(read_count, write_count, last_read, last_write, func, line),
+ TP_STRUCT__entry(
+ __field(uint32_t, reads)
+ __field(uint32_t, writes)
+ __field(uint32_t, read_delta)
+ __field(uint32_t, write_delta)
+ __string(func, func)
+ __field(uint3
