[PATCH 1/3] drm/xe/query: Increase timestamp width

[Matt Roper]

On Wed, Oct 09, 2024 at 11:08:12PM -0500, Lucas De Marchi wrote:
> On Wed, Oct 09, 2024 at 08:48:45PM -0700, Lucas De Marchi wrote:
> > Starting with Xe2 the timestamp is a full 64 bit counter, contrary to
> > the 36 bit that was available before. Although 36 should be sufficient
> > for any reasonable delta calculation (for Xe2, of about 30min), it's
> > surprising to userspace to get sommething truncated. Also if the
> > timestamp being compared to is coming from the GPU and the application
> > is not careful enough to apply the width there, a delta calculation
> > would be wrong.
> > 
> > Extend it to full 64-bits for non-media engines starting with Xe2.
> > 
> > Bspec: 60411
> > Cc: Szymon Morek <szymon.morek at intel.com>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_query.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > index 158629971eab3..a1f4cc25bea68 100644
> > --- a/drivers/gpu/drm/xe/xe_query.c
> > +++ b/drivers/gpu/drm/xe/xe_query.c
> > @@ -164,7 +164,11 @@ query_engine_cycles(struct xe_device *xe,
> > 			  cpu_clock);
> > 
> > 	xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> > -	resp.width = 36;
> > +
> > +	if (!xe_gt_is_media_type(gt) && GRAPHICS_VER(xe) >= 20)
> 
> I sent this to follow the spec to the letter, but I'm thinking the spec
> is not tagged correctly here.... the counter is the same, replicated to
> all engines. It doesn't make sense to be different in the media gt.

Yeah, when REMOVEDBY() tags are used against a top-level IP family
(top-level Xe2 in this case) rather than a specific release, the bspec
output often gets confused and leaves behind some stray markings like
"XE2_M" that aren't actually true.  I don't think those marks are
actually used so they don't have the proper IP inheritance rules
defined, which is why they still show up where they shouldn't.

tldr:  I think the 64-bit width here should apply to both the primary
and media GT from Xe2 onward.


Matt

> 
> In fact, testing on LNL:
> 
> $ sudo ./build/tools/intel_reg read 0x2358 0x235c 0x1c0358 0x1c035c
> Opened device: /dev/dri/card0
>                                     (0x00002358): 0x52d026c7
>                                     (0x0000235c): 0x00000062
>                                     (0x001c0358): 0x52d027b3
>                                     (0x001c035c): 0x00000062
> 
> So... the upper word returns the same thing for both rcs and vcs and
> it's > 36b.
> 
> +Matt Roper as he understand the spec tagging. Maybe I read it wrong.
> 
> It would be good if we don't have to differentiate the width based on
> gt.
> 
> 
> Lucas De Marchi
> 
> > +		resp.width = 64;
> > +	else
> > +		resp.width = 36;
> > 
> > 	/* Only write to the output fields of user query */
> > 	if (put_user(resp.cpu_timestamp, &query_ptr->cpu_timestamp))
> > -- 
> > 2.46.2
> > 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation