Re: [PATCH v2 2/2] drm/xe: Use for_each_gt to define gt_count

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Jul 28, 2025 at 03:29:06PM -0300, Gustavo Sousa wrote:
> We are currently bumping gt_count as we define GTs for each tile. While
> that works with our current code, there are reasons to improve that:
> 
> * That section of the code is dedicated to define each tile's set of GTs
>   and their respective info. The gt_count can be seen as a device level
>   property. While it is fair to bump it as we define each GT, we can
>   also focus on the GT themselves and count after we are done with the
>   definitions.
> 
> * More *importantly*, gt_count should reflect the number of GTs that we
>   would get when looping over them with for_each_gt(). Bumping gt_count
>   the way we are currently doing makes that value not really connected
>   to for_each_gt(). This could bite us in the future if in the loop gets
>   extra checks that are not accounted for in each existing "gt_count++".
> 
> As such, let's use for_each_gt() and extract the calculation of gt_count
> into a separate block, just after we define the set of GTs for each
> tile.
> 
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>



Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> ---
>  drivers/gpu/drm/xe/xe_pci.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 0d7073a04bb2..12de65756599 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -734,7 +734,6 @@ static int xe_info_init(struct xe_device *xe,
>  		gt->info.id = tile->id * xe->info.max_gt_per_tile;
>  		gt->info.has_indirect_ring_state = graphics_desc->has_indirect_ring_state;
>  		gt->info.engine_mask = graphics_desc->hw_engine_mask;
> -		xe->info.gt_count++;
>  
>  		err = xe_tile_alloc_vram(tile);
>  		if (err)
> @@ -759,9 +758,15 @@ static int xe_info_init(struct xe_device *xe,
>  		gt->info.id = tile->id * xe->info.max_gt_per_tile + 1;
>  		gt->info.has_indirect_ring_state = media_desc->has_indirect_ring_state;
>  		gt->info.engine_mask = media_desc->hw_engine_mask;
> -		xe->info.gt_count++;
>  	}
>  
> +	/*
> +	 * Now that we have tiles and GTs defined, let's loop over valid GTs
> +	 * in order to define gt_count.
> +	 */
> +	for_each_gt(gt, xe, id)
> +		xe->info.gt_count++;
> +
>  	return 0;
>  }
>  
> 
> -- 
> 2.50.1
>