Hi Leo
On Mon, 28 Apr 2025 at 13:24, Leo Yan leo.yan@arm.com wrote:
Hi all,
On Mon, Apr 28, 2025 at 11:53:26AM +0100, Mike Leach wrote:
[...]
@@ -362,6 +362,10 @@ enum cs_mode {
- @alloc_buffer: initialises perf's ring buffer for trace collection.
- @free_buffer: release memory allocated in @get_config.
- @update_buffer: update buffer pointers after a trace session.
- @percpu_save: saves state when CPU enters idle state.
Only set for percpu sink.
- @percpu_restore: restores state when CPU exits idle state.
*/
only set for percpu sink.struct coresight_ops_sink { int (*enable)(struct coresight_device *csdev, enum cs_mode mode, @@ -374,6 +378,8 @@ struct coresight_ops_sink { unsigned long (*update_buffer)(struct coresight_device *csdev, struct perf_output_handle *handle, void *sink_config);
void (*percpu_save)(struct coresight_device *csdev);void (*percpu_restore)(struct coresight_device *csdev);Again - why this percpu_* prefix ?
};
/**
I do not think this is the best approach.
The TRBE driver has its own power management registration functions, so is it not possible for the save and restore should be handled there, through a PM notifier, just as the ETM/ETE is?
Drop the unnecessary DT entry - TRBE is a per cpu architectural device
- if a TRBE is present, we know it will power down with the PE.
The CoreSight architecture permits an ETE to drive trace to an external sink - so the TRBE might be present but unused, therefore hooking into a source driver that may not be driving trace into the device does not seem wise..
Sorry I jumped in a bit late (I saw the patch at last week and it is on my review list).
I would suggest to hold on for this patch. I am refactoring CPU PM and hotplug in CoreSight based on the CoreSight path.
The idea is when we do CPU power management for CoreSight devices, we cannot simply control individual devices. Alternatively, we need to control logics based on the linkages on CoreSight path as it can reflect dependencies crossing the components. For example, for a CoreSight path, when running into CPU low power state, we need firstly disable tracer, then links, and at the end sinks. When CPU resumes back, we need to use the reversed ordering for turning on devices.
As a result, except the tracers (ETM / ETE) should be saved and restored contexts during CPU power states, other components on the path will be controlled by traversing CoreSight paths. The benefit is if a component (e.g., a link or a sink module) is shared by multiple CoreSight paths, then the component can be disabled or enabled based on reference counter.
I know I am a bit lagged - as I also need to support the locking on CoreSight paths. Please expect in next 1~2 weeks I will send out patches for public review.
The TRBE PM can follow the model of the ETE / ETMv4 and save and restore if currently in use.
If TRBE PM is registered as a seperate PM notifier, a prominent issue is it cannot promise the depedency between ETE and TRBE when execute CPU power management. E.g., when entering CPU idle states, ETE should be disabled prior to switch off TRBE, otherwise, it might cause lockup issue in hardware. If ETE and TRBE register PM notifier separately, we cannot ensure the sequence between ETE and TRBE, this is why we need to do the operations based on CoreSight paths.
I believe that the architecture requires that if the disabled TRBE cannot receive trace then the ETE should regard the trace as having been output (A1.4 ETE spec). Incorrect sequencing should only result in missing trace, not a core lockup - unless the implementation is not compliant.
Mike
Thanks, Leo