Re: [PATCH 09/12] drm/msm/a8xx: devcoredump state capture support

[Rob Clark]

On Wed, Oct 21, 2026 at 1:42 AM Konrad Dybcio
<konrad.dybcio@xxxxxxxxxxxxxxxx> 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 <robin.clark@xxxxxxxxxxxxxxxx>
> > Co-developed-by: Akhil P Oommen <akhilpo@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Rob Clark <robin.clark@xxxxxxxxxxxxxxxx>
> > ---
>
> [...]
>
> > +#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