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 39C57EB64DD for ; Tue, 18 Jul 2023 17:17:16 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6F84410E390; Tue, 18 Jul 2023 17:17:15 +0000 (UTC) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 098C210E390 for ; Tue, 18 Jul 2023 17:17:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1689700633; x=1721236633; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=Ge6VKKRNC9kPsLZfSa6HJ83X12QXDbyRyZR1U37iKFs=; b=WDeqBeIouum5pzKs1Vkj30FNiIEkb4+w3Y6FD1RnK/WcKmdLC9dQzVit pVWOpue34gj6FlN2WKnCKFKVxlc0wScLUm4lPyQYRvDwfKQ+sh+EmGAvG EVhSHiUUNflqacZ8MQgZqCM4v5KW2a8FvTrOMndLsBnuQ7B1WGtOyE2OL pvc3yUbhtPXOMM9BNoiDx7DeCZ9Cl3160FmwzuCVv1mUD8dyskPCQKQn6 nl8RD33AMsC9Bfc8hHvYEsBl35ppNVK9LLebmx+IJkNGGwEPyqYv1/0cN 93gyi32QkS+ZNlgrNCYgDpBIHclsE2tPax0dvhng663Dj487WGwu+ILqP w==; X-IronPort-AV: E=McAfee;i="6600,9927,10775"; a="432444255" X-IronPort-AV: E=Sophos;i="6.01,214,1684825200"; d="scan'208";a="432444255" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2023 10:17:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10775"; a="700985379" X-IronPort-AV: E=Sophos;i="6.01,214,1684825200"; d="scan'208";a="700985379" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.20.219]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2023 10:17:11 -0700 Date: Tue, 18 Jul 2023 10:17:11 -0700 Message-ID: <87y1jd9m20.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Lionel Landwerlin In-Reply-To: <46c126f6-5287-7e83-9e79-20b3662fccbe@intel.com> References: <20230718024318.2911899-1-ashutosh.dixit@intel.com> <20230718024318.2911899-2-ashutosh.dixit@intel.com> <46c126f6-5287-7e83-9e79-20b3662fccbe@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/28.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 Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/perf: Subtract gtt_offset from hw_tail X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, 18 Jul 2023 01:39:35 -0700, Lionel Landwerlin wrote: > Hi Lionel, > On 18/07/2023 05:43, Ashutosh Dixit wrote: > > The code in oa_buffer_check_unlocked() is correct only if the OA buffer is > > 16 MB aligned (which seems to be the case today in i915). However when the > > 16 MB alignment is dropped, when we "Subtract partial amount off the tail", > > the "& (OA_BUFFER_SIZE - 1)" operation in OA_TAKEN() will result in an > > incorrect hw_tail value. > > > > Therefore hw_tail must be brought to the same base as head and read_tail > > prior to OA_TAKEN by subtracting gtt_offset from hw_tail. > > > > Signed-off-by: Ashutosh Dixit > > --- > > drivers/gpu/drm/i915/i915_perf.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > > index 49c6f1ff11284..f7888a44d1284 100644 > > --- a/drivers/gpu/drm/i915/i915_perf.c > > +++ b/drivers/gpu/drm/i915/i915_perf.c > > @@ -565,6 +565,7 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream) > > partial_report_size %= report_size; > > /* Subtract partial amount off the tail */ > > + hw_tail -= gtt_offset; > > hw_tail = OA_TAKEN(hw_tail, partial_report_size); > > /* NB: The head we observe here might effectively be a little > > > You should squash this patch with the next one. Otherwise further down this > function there is another > > hw_tail -= gtt_offset; Are you looking at old code, because this line is not there in this function any more. There have been several changes to the function lately, aging tail etc. is gone e.g. But otherwise you are right, Patch 2 basically writes over Patch 1, so the two patches can be squashed. I separated out Patch 1 since it shows the bug (incidentally the bug doesn't show up in i915 since a 16 MB BO in i915 is 16 MB aligned, I discovered the bug while porting stuff to xe). So if you are going to R-b this series I can repost after squashing. But if we wait for Umesh to return (he is out till the end of the month) and review this, I'd rather leave the two patches as they are till Umesh reviews them. Thanks. -- Ashutosh