From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 87AB7C5CFCF for ; Fri, 14 Aug 2026 15:42:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 45F9510F696; Fri, 14 Aug 2026 15:42:35 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="cDDKgAlI"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7057A10F696 for ; Fri, 14 Aug 2026 15:42:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1786722153; x=1818258153; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=yfTjGCdVUBIpvl6DSdPMYxrfa50NgWexwe9T1OkOdCE=; b=cDDKgAlIZ2s6HN2jXXtl8bpWjp5PZyOul2SPlt+pjf6fabFpb3Z5Gmjh 707mny13D2v6Xn4c+DK3VjOXtgo2+s5k5EYILmiA+8s9OKliGyuwiEqiZ HNbq/ihMhkgeJ3g3HDoIxLWLcICvZ5Rxxqji6DGRfnI/AzS2RBP6YClVN ONlTPyCCC6mCcn46Lr5hVpkH3Op6NpTDAxkAghuRVHGZKySxnCN868lhT F/ZX1TDt9SDXNMh4b14J3AAeSZVnlpUo6wVIdOkzQhAMuMPZgT/OMiICh mgKZVDUkii4o/WlvTKZX5/T9PzeE882Iqlh34CKms0U1ICvv9s1IUwJjE Q==; X-CSE-ConnectionGUID: Lhcz9ndrTsqVDzpYRcp+xQ== X-CSE-MsgGUID: P1rBcPJUQw6G/KQ0sBVavQ== X-IronPort-AV: E=McAfee;i="6800,10657,11875"; a="86264165" X-IronPort-AV: E=Sophos;i="6.25,222,1779174000"; d="scan'208";a="86264165" Received: from fmviesa004.fm.intel.com ([10.60.135.144]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Aug 2026 08:42:33 -0700 X-CSE-ConnectionGUID: gNJnUvH1QKSpenGVfWKrJg== X-CSE-MsgGUID: bqUIyVAPRRSiRsApfKQr7g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.25,222,1779174000"; d="scan'208";a="266245427" Received: from rosadoer-mobl1.amr.corp.intel.com (HELO adixit-MOBL3.intel.com) ([10.125.32.6]) by fmviesa004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Aug 2026 08:42:33 -0700 Date: Fri, 14 Aug 2026 08:42:31 -0700 Message-ID: <87bjb4hfo8.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa Cc: Subject: Re: [PATCH 3/3] drm/xe/xe_oa: Add a lag to the reports that is exported to user In-Reply-To: <20260806224702.3563031-8-umesh.nerlige.ramappa@intel.com> References: <20260806224702.3563031-5-umesh.nerlige.ramappa@intel.com> <20260806224702.3563031-8-umesh.nerlige.ramappa@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, 06 Aug 2026 15:47:06 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > When running heavy workloads, reading the OA reports too soon does not > guarantee that the report has landed in memory. To make sure correct > reports are copied to user buffer, only return reports that lag the > current HW_TAIL register by 32 reports. This is an empirical number > based on a heavy render workload and several test iterations. Looks good overall, but I want to discuss a couple of further points: 1. I know you are doing some verification of OA data in IGT, but I am thinking it would be good to have some verification of OA data (of the sort that is removed in Patch 2) also in the kernel. So e.g. after a report is read, we could set the timestamp field in the report to 0. Then when we advance the SW tail pointer, we could check if the timestamp fields for new reports are non-zero. If we see a 0 timestamp value, this would mean that the 32 report delay is not sufficient (for who knows what will happen in future platforms). An error in dmesg if we see a 0 timestamp should suffise. I understand that, because cachelines are landing out of order, a non-zero timestamp value doesn't absolutely guarantee that all data is correct. But I am thinking statistially we should see 0 timestamp values, once in a while, if the 32 report delay were insufficient. So at least we'll have some indication from the kernel if that were to happen. 2. The second point is about "what happens in the end", the UMD never sees the last 32 reports? Maybe we could do the following to address this: when OA stream is disabled, we advance the SW tail pointer to the HW tail (overriding the 32 report delay). Then when UMD reads the last bit of data, after disabling the stream, they will get all data. Though all cachelines might still not have landed, but at least we will have advanced the SW tail pointer. Thoughts? Thanks. -- Ashutosh > > Signed-off-by: Umesh Nerlige Ramappa > --- > v2: Fix the LAG logic by using sliding window (Sashiko) > v3: Fix checkpatch warning > --- > drivers/gpu/drm/xe/xe_oa.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c > index 5952010e8f51..dc8b402d059a 100644 > --- a/drivers/gpu/drm/xe/xe_oa.c > +++ b/drivers/gpu/drm/xe/xe_oa.c > @@ -224,7 +224,7 @@ static bool mert_wa_14026633728(struct xe_oa_stream *s) > static bool xe_oa_buffer_check_unlocked(struct xe_oa_stream *stream) > { > u32 gtt_offset = xe_bo_ggtt_addr(stream->oa_buffer.bo); > - u32 hw_tail, partial_report_size, available; > + u32 hw_tail, partial_report_size, available, lag; > int report_size = stream->oa_buffer.format->size; > unsigned long flags; > > @@ -234,17 +234,21 @@ static bool xe_oa_buffer_check_unlocked(struct xe_oa_stream *stream) > hw_tail -= gtt_offset; > > /* > - * The tail pointer increases in 64 byte (cacheline size), not in report_size > + * The hw_tail pointer increases in 64 byte (cacheline size), not in report_size > * increments. Also report size may not be a power of 2. Compute potential > * partially landed report in OA buffer. > */ > partial_report_size = xe_oa_circ_diff(stream, hw_tail, stream->oa_buffer.tail); > partial_report_size %= report_size; > > - /* Subtract partial amount off the tail */ > + /* Subtract partial amount off the hw_tail */ > hw_tail = xe_oa_circ_diff(stream, hw_tail, partial_report_size); > > - stream->oa_buffer.tail = hw_tail; > +#define LAG_REPORTS 32 > + lag = xe_oa_circ_diff(stream, hw_tail, stream->oa_buffer.tail); > + if (lag > LAG_REPORTS * report_size) > + stream->oa_buffer.tail = xe_oa_circ_diff(stream, hw_tail, > + LAG_REPORTS * report_size); > > available = xe_oa_circ_diff(stream, stream->oa_buffer.tail, stream->oa_buffer.head); > stream->pollin = available >= stream->wait_num_reports * report_size; > -- > 2.51.0 >