<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> <html> <body> <div dir="auto"> <div dir="auto"><span style="font-size: 12pt;">On April 30, 2021 23:01:44 "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:</span></div><div id="aqm-original" style="color: black;"> <blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #808080; padding-left: 0.75ex;"> <div dir="auto">On Fri, 30 Apr 2021 19:19:59 -0700, Umesh Nerlige Ramappa wrote:</div> <blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #0099CC; padding-left: 0.75ex;"> <div dir="auto"><br></div> <div dir="auto">On Fri, Apr 30, 2021 at 07:35:41PM -0500, Jason Ekstrand wrote:</div> <blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #9933CC; padding-left: 0.75ex;"> <div dir="auto">On April 30, 2021 18:00:58 "Dixit, Ashutosh" <ashutosh.dixit@intel.com></div> <div dir="auto">wrote:</div> <div dir="auto"><br></div> <div dir="auto">On Fri, 30 Apr 2021 15:26:09 -0700, Umesh Nerlige Ramappa wrote:</div> <div dir="auto"><br></div> <div dir="auto">Looks like the engine can be dropped since all timestamps are in sync.</div> <div dir="auto">I</div> <div dir="auto">just have one more question here. The timestamp itself is 36 bits.</div> <div dir="auto"> Should</div> <div dir="auto">the uapi also report the timestamp width to the user OR should I just</div> <div dir="auto">return the lower 32 bits of the timestamp?</div> <div dir="auto"><br></div> <div dir="auto">Yeah, I think reporting the timestamp width is a good idea since we're</div> <div dir="auto">reporting the period/frequency here.</div> </blockquote> <div dir="auto"><br></div> <div dir="auto">Actually, I forgot that we are handling the overflow before returning the</div> <div dir="auto">cs_cycles to the user and overflow handling was the only reason I thought</div> <div dir="auto">user should know the width. Would you stil recommend returning the width in</div> <div dir="auto">the uapi?</div> </blockquote> <div dir="auto"><br></div> <div dir="auto">The width is needed for userspace to figure out if overflow has occured</div> <div dir="auto">between two successive query calls. I don't think I see this happening in</div> <div dir="auto">the code.</div> </blockquote> </div><div dir="auto"><br></div><div dir="auto">Right... We (UMDs) currently just hard-code it to 36 bits because that's what we've had on all platforms since close enough to forever. We bake in the frequency based on PCI ID. Returning the number of bits, like I said, goes nicely with the frequency. It's not necessary, assuming sufficiently smart userspace (neither is frequency), but it seems to go with it. I guess I don't care much either way.</div><div dir="auto"><br></div><div dir="auto">Coming back to the multi-tile issue we discussed internally, I think that is something we should care about. Since this works by reading the timestamp register on an engine, I think leaving the engine specifier in there is fine. Userspace should know that there's actually only one clock and just query one of them (probably RCS). For crazy multi-device cases, we'll either query per logical device (read tile) or we'll have to make them look like a single device and sync the timestamps somehow in the UMD by carrying around an offset factor.</div><div dir="auto"><br></div><div dir="auto">As is, this patch is</div><div dir="auto"><br></div><div dir="auto">Reviewed-by: Jason Ekstrand <jason@jlekstrand.net></div><div dir="auto"><br></div><div dir="auto">I still need to review the ANV patch before we can land this though.</div><div dir="auto"><br></div><div dir="auto">--Jason</div> </div></body> </html>