Thanks Maíra, this is: Reviewed-by: Iago Toral Quiroga <[email protected]>
El lun, 27-07-2026 a las 11:32 -0300, Maíra Canal escribió: > The binner BO is a single 16MB buffer split into 512KB slots that are > handed out to jobs at submission time and recycled as jobs complete, > without ever being cleared. Each slot holds the job's Tile State Data > Array (TSDA) at its start, followed by the tile allocation pool. > > While the tile allocation pool is only walked by the render thread > through branches the binner generated during the current job, the > TSDA is the PTB's own per-tile bookkeeping and is consumed by the > hardware itself. Although the kernel sets the "Auto-initialise Tile > State Data Array" flag in the tile binning mode configuration, the > PTB demonstrably still acts on stale tile state left by the slot's > previous user: the binner ends up creating invalid command streams > with invalid primitive streams and branches, which can cause GPU > hangs > as observed in [1][2]. > > Zero the TSDA when the job's binning slot is configured. This clears > 48 bytes per tile (~24KB for a 1080p frame) in the submission path, > and > guarantees the PTB never sees another job's tile state. > > The tile count is only checked for being non-zero today, so the 8-bit > fields it comes from can describe a tile state array almost six times > larger than the slot it has to live in. Bound it before the slot is > handed out, since such size decides how much of the slot is left for > the tile alloc pool. > > Link: https://github.com/raspberrypi/linux/issues/3221 [1] > Link: https://github.com/raspberrypi/linux/issues/5780 [2] > Fixes: 553c942f8b2c ("drm/vc4: Allow using more than 256MB of CMA > memory.") > Cc: [email protected] > Signed-off-by: Maíra Canal <[email protected]> > --- > drivers/gpu/drm/vc4/vc4_validate.c | 29 +++++++++++++++++++++++----- > - > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_validate.c > b/drivers/gpu/drm/vc4/vc4_validate.c > index 7f2fadfde7a8..d2a65c968b1f 100644 > --- a/drivers/gpu/drm/vc4/vc4_validate.c > +++ b/drivers/gpu/drm/vc4/vc4_validate.c > @@ -385,6 +385,23 @@ validate_tile_binning_config(VALIDATE_ARGS) > return -EINVAL; > } > > + /* The tile state data array is 48 bytes per tile, and we > put it at > + * the start of a BO containing both it and the tile alloc. > + */ > + tile_state_size = 48 * tile_count; > + > + /* Since the tile alloc array will follow us, align. */ > + tile_state_size = roundup(tile_state_size, 4096); > + > + /* Reject configurations whose tile state would leave no > room for > + * the tile alloc pool that follows it in the slot. > + */ > + if (tile_state_size >= vc4->bin_alloc_size) { > + DRM_DEBUG("Tile binning config of %dx%d too > large\n", > + exec->bin_tiles_x, exec->bin_tiles_y); > + return -EINVAL; > + } > + > bin_slot = vc4_v3d_get_bin_slot(vc4); > if (bin_slot < 0) { > if (bin_slot != -EINTR && bin_slot != -ERESTARTSYS) > { > @@ -400,13 +417,13 @@ validate_tile_binning_config(VALIDATE_ARGS) > exec->bin_slots |= BIT(bin_slot); > bin_addr = vc4->bin_bo->base.dma_addr + bin_slot * vc4- > >bin_alloc_size; > > - /* The tile state data array is 48 bytes per tile, and we > put it at > - * the start of a BO containing both it and the tile alloc. > - */ > - tile_state_size = 48 * tile_count; > + exec->tile_alloc_offset = bin_addr + tile_state_size; > > - /* Since the tile alloc array will follow us, align. */ > - exec->tile_alloc_offset = bin_addr + > roundup(tile_state_size, 4096); > + /* The TSDA area must be zeroed out before use, otherwise > the PTB might > + * consume a stale tile state. > + */ > + memset(vc4->bin_bo->base.vaddr + bin_slot * vc4- > >bin_alloc_size, 0, > + tile_state_size); > > *(uint8_t *)(validated + 14) = > ((flags & > ~(VC4_BIN_CONFIG_ALLOC_INIT_BLOCK_SIZE_MASK | >
