Re: [Intel-xe] [PATCH v1 8/8] drm/xe/uapi: Add Tile ID information to the GT info query

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

On Thu, Nov 16, 2023 at 08:44:29PM +0000, Souza, Jose wrote:
> On Thu, 2023-11-16 at 14:43 +0000, Francois Dugast wrote:
> > From: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > As an information only. So Userspace can use this information
> > and be able to correlate different GTs.
> > 
> > Make API symmetric between Engine and GT info.
> > 
> > There's no need right now to include a tile_query entry
> > since there's no other information that we need from tile
> > that is not already exposed through different queries.
> > 
> > However, this could be added later if we have different Tile
> > information that could matter to userspace. But let's keep
> > the API ready for a direct reference to Tile ID based on
> > the GT entry.
> > 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_query.c | 1 +
> >  include/uapi/drm/xe_drm.h     | 2 ++
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > index 87345ed99151..838f03795841 100644
> > --- a/drivers/gpu/drm/xe/xe_query.c
> > +++ b/drivers/gpu/drm/xe/xe_query.c
> > @@ -382,6 +382,7 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query
> >  			gt_list->gt_list[id].type = DRM_XE_QUERY_GT_TYPE_MEDIA;
> >  		else
> >  			gt_list->gt_list[id].type = DRM_XE_QUERY_GT_TYPE_MAIN;
> > +		gt_list->gt_list[id].tile_id = gt_to_tile(gt)->id;
> >  		gt_list->gt_list[id].gt_id = gt->info.id;
> >  		gt_list->gt_list[id].clock_freq = gt->info.clock_freq;
> >  		if (!IS_DGFX(xe))
> > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > index b6d3d4d52da9..8610ac461619 100644
> > --- a/include/uapi/drm/xe_drm.h
> > +++ b/include/uapi/drm/xe_drm.h
> > @@ -380,6 +380,8 @@ struct drm_xe_gt {
> >  #define DRM_XE_QUERY_GT_TYPE_MEDIA		1
> >  	/** @type: GT type: Main or Media */
> >  	__u16 type;
> > +	/** @tile_id: Tile ID where this GT lives (Information only) */
> > +	__u16 tile_id;
> 
> ack on adding it but just adding it breaks the struct alignment for 32/64bits user-space.

The padding was broken even without this pad. I had noticed this yesterday, so I
reviewed that aspect and found many more incorrect paddings. We will need to send
a separated patch to fix them all instead of hunting which patch was the culprit  now :(
> 
> >  	/** @gt_id: Unique ID of this GT within the PCI Device */
> >  	__u16 gt_id;
> >  	/** @clock_freq: A clock frequency for timestamp */
>