* coresight: Problem caused by resetting enable_sink
@ 2016-12-26 9:17 ` Wangnan (F)
0 siblings, 0 replies; 4+ messages in thread
From: Wangnan (F) @ 2016-12-26 9:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mathieu,
I meet problems caused by your commit d52c9750f150 ('coresight:
reset "enable_sink" flag when need be'). Not only the one I
posted in the previous patch.
My use case is a simple 'perf record -e cs_etm// ls'. It works
properly before this commit, and failed when allocating aux buffer
after your commit. I can't fully understand your code, but the
problem I meet seems caused by inappropriately reseting sink.
My device is connected like this (use two etfs):
Core0--+
Core1--+-- funnel0 --> etf0
Core2--|
Core3--+
Core0--+
Core1--+-- funnel1 --> etf1
Core2--|
Core3--+
Before running perf, two etfs are activated using sysfs
enable_sink interface.
During etm_setup_aux, coresight_get_enabled_sink() finds
etf0 for core0, and automatically deactivates it.
For core1, coresight_get_enabled_sink() returns etf1.
However, etf1 is not on the link of core1, so following
coresight_build_path() fails.
I guess your commit is based on the assumption that all
sinks are in the patch for each cores. Like this:
Core0--+
Core1--+-- funnel0 --> etf0 ++
Core2--| | +--- etr
Core3--+ | |
+ replicator +
Core0--+ | |
Core1--+-- funnel1 --> etf1 ++ +--- etb
Core2--|
Core3--+
But it is not true, at least for some hisilicon board.
I have to revert your patch to make CoreSight on my board
work. Please reconsider this patch, or please give some
suggestion if you think I misunderstood your patch.
Thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread* coresight: Problem caused by resetting enable_sink
@ 2016-12-26 9:17 ` Wangnan (F)
0 siblings, 0 replies; 4+ messages in thread
From: Wangnan (F) @ 2016-12-26 9:17 UTC (permalink / raw)
To: mathieu.poirier, xiakaixu 00238161, suzuki.poulose,
linux-kernel@vger.kernel.org, linux-arm-kernel
Hi Mathieu,
I meet problems caused by your commit d52c9750f150 ('coresight:
reset "enable_sink" flag when need be'). Not only the one I
posted in the previous patch.
My use case is a simple 'perf record -e cs_etm// ls'. It works
properly before this commit, and failed when allocating aux buffer
after your commit. I can't fully understand your code, but the
problem I meet seems caused by inappropriately reseting sink.
My device is connected like this (use two etfs):
Core0--+
Core1--+-- funnel0 --> etf0
Core2--|
Core3--+
Core0--+
Core1--+-- funnel1 --> etf1
Core2--|
Core3--+
Before running perf, two etfs are activated using sysfs
enable_sink interface.
During etm_setup_aux, coresight_get_enabled_sink() finds
etf0 for core0, and automatically deactivates it.
For core1, coresight_get_enabled_sink() returns etf1.
However, etf1 is not on the link of core1, so following
coresight_build_path() fails.
I guess your commit is based on the assumption that all
sinks are in the patch for each cores. Like this:
Core0--+
Core1--+-- funnel0 --> etf0 ++
Core2--| | +--- etr
Core3--+ | |
+ replicator +
Core0--+ | |
Core1--+-- funnel1 --> etf1 ++ +--- etb
Core2--|
Core3--+
But it is not true, at least for some hisilicon board.
I have to revert your patch to make CoreSight on my board
work. Please reconsider this patch, or please give some
suggestion if you think I misunderstood your patch.
Thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread* coresight: Problem caused by resetting enable_sink
2016-12-26 9:17 ` Wangnan (F)
@ 2017-01-03 18:34 ` Mathieu Poirier
-1 siblings, 0 replies; 4+ messages in thread
From: Mathieu Poirier @ 2017-01-03 18:34 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 26, 2016 at 05:17:08PM +0800, Wangnan (F) wrote:
> Hi Mathieu,
Hello Wang,
>
> I meet problems caused by your commit d52c9750f150 ('coresight:
> reset "enable_sink" flag when need be'). Not only the one I
> posted in the previous patch.
>
> My use case is a simple 'perf record -e cs_etm// ls'. It works
> properly before this commit, and failed when allocating aux buffer
> after your commit. I can't fully understand your code, but the
> problem I meet seems caused by inappropriately reseting sink.
>
> My device is connected like this (use two etfs):
>
> Core0--+
> Core1--+-- funnel0 --> etf0
> Core2--|
> Core3--+
>
> Core0--+
> Core1--+-- funnel1 --> etf1
> Core2--|
> Core3--+
Yes, supporting this topology is a problem I long predicted.
>
> Before running perf, two etfs are activated using sysfs
> enable_sink interface.
>
> During etm_setup_aux, coresight_get_enabled_sink() finds
> etf0 for core0, and automatically deactivates it.
>
> For core1, coresight_get_enabled_sink() returns etf1.
> However, etf1 is not on the link of core1, so following
> coresight_build_path() fails.
>
> I guess your commit is based on the assumption that all
> sinks are in the patch for each cores. Like this:
>
>
> Core0--+
> Core1--+-- funnel0 --> etf0 ++
> Core2--| | +--- etr
> Core3--+ | |
> + replicator +
> Core0--+ | |
> Core1--+-- funnel1 --> etf1 ++ +--- etb
> Core2--|
> Core3--+
>
Correct - the entire solution is based on the assumption that all cores use the
same sink. When I wrote the driver not a single system enacted a topology where
cores wouldn't use the same sink for a trace session.
> But it is not true, at least for some hisilicon board.
>
> I have to revert your patch to make CoreSight on my board
> work. Please reconsider this patch, or please give some
> suggestion if you think I misunderstood your patch.
You understood the patch corretly but simply reverting it isn't the solution.
Supporting a system where different sinks are used won't be easy - a lot of
things need to be adjusted. The first and critical part that comes to mind is
the mechanic that fetches the sink definition from the cmd line when using the
perf interface (if I remember correctly the bison/flex parser can handle
multiple sink definition). From there everything needs to be tailored to handle
more than one sink.
You will likely have more questions... Get back to me and I'll be happy to help.
Regards,
Mathieu
>
> Thank you.
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: coresight: Problem caused by resetting enable_sink
@ 2017-01-03 18:34 ` Mathieu Poirier
0 siblings, 0 replies; 4+ messages in thread
From: Mathieu Poirier @ 2017-01-03 18:34 UTC (permalink / raw)
To: Wangnan (F)
Cc: xiakaixu 00238161, suzuki.poulose, linux-kernel@vger.kernel.org,
linux-arm-kernel
On Mon, Dec 26, 2016 at 05:17:08PM +0800, Wangnan (F) wrote:
> Hi Mathieu,
Hello Wang,
>
> I meet problems caused by your commit d52c9750f150 ('coresight:
> reset "enable_sink" flag when need be'). Not only the one I
> posted in the previous patch.
>
> My use case is a simple 'perf record -e cs_etm// ls'. It works
> properly before this commit, and failed when allocating aux buffer
> after your commit. I can't fully understand your code, but the
> problem I meet seems caused by inappropriately reseting sink.
>
> My device is connected like this (use two etfs):
>
> Core0--+
> Core1--+-- funnel0 --> etf0
> Core2--|
> Core3--+
>
> Core0--+
> Core1--+-- funnel1 --> etf1
> Core2--|
> Core3--+
Yes, supporting this topology is a problem I long predicted.
>
> Before running perf, two etfs are activated using sysfs
> enable_sink interface.
>
> During etm_setup_aux, coresight_get_enabled_sink() finds
> etf0 for core0, and automatically deactivates it.
>
> For core1, coresight_get_enabled_sink() returns etf1.
> However, etf1 is not on the link of core1, so following
> coresight_build_path() fails.
>
> I guess your commit is based on the assumption that all
> sinks are in the patch for each cores. Like this:
>
>
> Core0--+
> Core1--+-- funnel0 --> etf0 ++
> Core2--| | +--- etr
> Core3--+ | |
> + replicator +
> Core0--+ | |
> Core1--+-- funnel1 --> etf1 ++ +--- etb
> Core2--|
> Core3--+
>
Correct - the entire solution is based on the assumption that all cores use the
same sink. When I wrote the driver not a single system enacted a topology where
cores wouldn't use the same sink for a trace session.
> But it is not true, at least for some hisilicon board.
>
> I have to revert your patch to make CoreSight on my board
> work. Please reconsider this patch, or please give some
> suggestion if you think I misunderstood your patch.
You understood the patch corretly but simply reverting it isn't the solution.
Supporting a system where different sinks are used won't be easy - a lot of
things need to be adjusted. The first and critical part that comes to mind is
the mechanic that fetches the sink definition from the cmd line when using the
perf interface (if I remember correctly the bison/flex parser can handle
multiple sink definition). From there everything needs to be tailored to handle
more than one sink.
You will likely have more questions... Get back to me and I'll be happy to help.
Regards,
Mathieu
>
> Thank you.
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-03 18:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-26 9:17 coresight: Problem caused by resetting enable_sink Wangnan (F)
2016-12-26 9:17 ` Wangnan (F)
2017-01-03 18:34 ` Mathieu Poirier
2017-01-03 18:34 ` Mathieu Poirier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.