On Wed, Oct 21, 2026 at 1:42 AM Konrad Dybcio <[email protected]> wrote: > > On 10/20/26 10:46 PM, Rob Clark wrote: > > This attempts to make it easier to compare to kgsl, the downstream > > driver, to make it easier to spot bugs or missing things. Such as > > re-using function names, etc. > > > > Signed-off-by: Rob Clark <[email protected]> > > Co-developed-by: Akhil P Oommen <[email protected]> > > Signed-off-by: Rob Clark <[email protected]> > > --- > > [...] > > > +#if defined(CONFIG_DRM_MSM_GPU_STATE) > > + .show = a8xx_show, > > +#endif > > .gpu_busy = a8xx_gpu_busy, > > .gpu_get_freq = a6xx_gmu_get_freq, > > .gpu_set_freq = a6xx_gpu_set_freq, > > +#if defined(CONFIG_DRM_MSM_GPU_STATE) > > + .gpu_state_get = a8xx_gpu_state_get, > > + .gpu_state_put = a8xx_gpu_state_put, > > +#endif > > It's odd to have 2 identical compile bounds side by side, > I'd just group them > > [...] > > > > +static void gen8_cx_dbgc_debugbus_select(struct msm_gpu *gpu, u32 > > block_id, u32 index) > > +{ > > + u32 reg; > > + > > + reg = A7XX_DBGC_CFG_DBGBUS_SEL_D_PING_INDEX(index) | > > + A7XX_DBGC_CFG_DBGBUS_SEL_D_PING_BLK_SEL(block_id); > > + > > + gpu_write(gpu, REG_A6XX_DBGC_CFG_DBGBUS_SEL_A, reg); > > + gpu_write(gpu, REG_A6XX_DBGC_CFG_DBGBUS_SEL_B, reg); > > + gpu_write(gpu, REG_A6XX_DBGC_CFG_DBGBUS_SEL_C, reg); > > + gpu_write(gpu, REG_A6XX_DBGC_CFG_DBGBUS_SEL_D, reg); > > + > > + /* Wait 1 us to make sure the data is flowing */ > > + udelay(1); > > Should that be a readback instead?
I'll update to a better comment from a (different?) version of kgsl: /* * There needs to be a delay of 1 us to ensure enough time for correct * data is funneled into the trace buffer */ Hard to say, but sounds like the delay is about more than making sure the AHB write lands. Same goes for other delays and barriers. I guess downstream snapshot is well tested during bringup, so it should be safe to align with what it does. BR, -R
