On Wed, Oct 16, 2024 at 5:13 AM Antonino Maniscalco <antomani103@xxxxxxxxx> wrote: > > On 10/8/24 11:10 PM, Kees Bakker wrote: > > Op 03-10-2024 om 18:12 schreef Antonino Maniscalco: > >> Add trace points corresponding to preemption being triggered and being > >> completed for latency measurement purposes. > >> > >> Reviewed-by: Akhil P Oommen <quic_akhilpo@xxxxxxxxxxx> > >> Tested-by: Rob Clark <robdclark@xxxxxxxxx> > >> Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> # on SM8650-QRD > >> Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> # on SM8550-QRD > >> Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> # on SM8450-HDK > >> Signed-off-by: Antonino Maniscalco <antomani103@xxxxxxxxx> > >> --- > >> drivers/gpu/drm/msm/adreno/a6xx_preempt.c | 6 ++++++ > >> drivers/gpu/drm/msm/msm_gpu_trace.h | 28 +++++++++++++++++++++ > >> +++++++ > >> 2 files changed, 34 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c b/drivers/gpu/ > >> drm/msm/adreno/a6xx_preempt.c > >> index > >> 21e333cb6342d33425eb96f97bcc853e9b041b36..6803d5af60cc8fb0f2a52ee160ffdbf0e8ef0209 100644 > >> --- a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c > >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c > >> @@ -7,6 +7,7 @@ > >> #include "a6xx_gpu.h" > >> #include "a6xx_gmu.xml.h" > >> #include "msm_mmu.h" > >> +#include "msm_gpu_trace.h" > >> /* > >> * Try to transition the preemption state from old to new. Return > >> @@ -174,6 +175,8 @@ void a6xx_preempt_irq(struct msm_gpu *gpu) > >> set_preempt_state(a6xx_gpu, PREEMPT_NONE); > >> + trace_msm_gpu_preemption_irq(a6xx_gpu->cur_ring->id); > >> + > >> /* > >> * Retrigger preemption to avoid a deadlock that might occur > >> when preemption > >> * is skipped due to it being already in flight when requested. > >> @@ -294,6 +297,9 @@ void a6xx_preempt_trigger(struct msm_gpu *gpu) > >> */ > >> ring->restore_wptr = false; > >> + trace_msm_gpu_preemption_trigger(a6xx_gpu->cur_ring->id, > >> + ring ? ring->id : -1); > >> + > > There is no need for the ternary operator. "ring" should be non-NULL, > > otherwise the code would have already crashed. > > So the change can just be > > trace_msm_gpu_preemption_trigger(a6xx_gpu->cur_ring->id, ring->id); > > You are right, we had a similar cleanup but I missed this particular > one, thanks for pointing me at it! I apologize for the late response but > I've been at XDC and therefore unable to look at my email. I will point > this out to Rob since this series is in msm-next to see if I need to > send a separate patch to clean this. Yes, please send a new commit, I don't want to re-write history on msm-next BR, -R > Best regards, > -- > Antonino Maniscalco <antomani103@xxxxxxxxx>