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 2C243C5DF67 for ; Tue, 18 Aug 2026 04:38:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B285210E9E2; Tue, 18 Aug 2026 04:38:34 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="hEiBZSuN"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id 50C9010E9E2 for ; Tue, 18 Aug 2026 04:38: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=1787027913; x=1818563913; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=4OSEcG3ls50GbCWHXZqjAAIpy4pbQwiMBW98wOql40k=; b=hEiBZSuNnhn+/0JzOlLv4n2UD2jl73ZmCNZ31hNe805WwLc4ffmE8jFK jXqUxI16pg/lhVRQSZAaA51UDhRE+/R8cWjLki/JOe/YDG8Z3fF/pROiX KOjFo27WhytoK5oALqDa6Ymj/T6v6W3ybRwJhSFcnEZ4AkUoxA7cywf3l EZeTx+GAmUPEK0QCC47THy1lG6Vl8SiAbXTd/vdGJct5nRB7l1TSC0hig SPPzPuFDzUeOKKivLYEa/OgcxviLp9G5l3ahogvOSkyhkah14gBvAH8et KVDIH5ku2TfStFTMpDYh8e9l65G/HYmlpyKLUgUB7nNkcs2HtljrAZ/Hy g==; X-CSE-ConnectionGUID: 93wKxlq8TQ28EBOoaiyWtw== X-CSE-MsgGUID: IYkfmy57TbioQrGF0kAeug== X-IronPort-AV: E=McAfee;i="6800,10657,11878"; a="98175422" X-IronPort-AV: E=Sophos;i="6.25,230,1779174000"; d="scan'208";a="98175422" Received: from orviesa005.jf.intel.com ([10.64.159.145]) by fmvoesa103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Aug 2026 21:38:32 -0700 X-CSE-ConnectionGUID: 0tCfzQp+SQu8pWQN7OKmDA== X-CSE-MsgGUID: QEt7oZhsTfK2fYQVlxo3hg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.25,230,1779174000"; d="scan'208";a="269416906" Received: from pmotwani-mobl.amr.corp.intel.com (HELO adixit-MOBL3.intel.com) ([10.125.36.125]) by orviesa005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Aug 2026 21:38:33 -0700 Date: Mon, 17 Aug 2026 21:38:31 -0700 Message-ID: <87fr0cgi0o.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: References: <20260806224702.3563031-5-umesh.nerlige.ramappa@intel.com> <20260806224702.3563031-8-umesh.nerlige.ramappa@intel.com> <87bjb4hfo8.wl-ashutosh.dixit@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 Fri, 14 Aug 2026 12:01:50 -0700, Umesh Nerlige Ramappa wrote: > > On Fri, Aug 14, 2026 at 08:42:31AM -0700, Dixit, Ashutosh wrote: > > 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 am hesitant to write to the OA buffer at all, especially due to the > different coherence behavior between discrete and integrated. > We should just address it as an issue/bug at that point. Let me think > about it a bit and see what we can do if we see an error in future. OK, we can leave the validation in IGT, instead of the kernel, but we should do it for a long enough time, like several passes through the OA buffer, to catch any out of order cache lines, if the 32 report delay is too short. Thanks. > > > > 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. > > oh, I missed that part. Yeah, if the stream is disabled, I would need to > drain the data. I think that would just add a delay for 32 reports, based > on the timer period before updating the tail to the latest. > > Thanks, > Umesh > > > > > 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 > >>