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 77C58C7EE23 for ; Thu, 1 Jun 2023 04:28:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C2D0B10E20F; Thu, 1 Jun 2023 04:28:33 +0000 (UTC) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id A8B1F10E20F for ; Thu, 1 Jun 2023 04:28:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1685593711; x=1717129711; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=csdnSxnD37G2eJZS+1B8SOe7pqZjB+dioDlQkuOFOwg=; b=eYPZ2k6qSMTQxYixba/ZVV1W/eyNpxYRI+uReMWnPW7Hke4TVEJL7vjv sNNDq4GlwdaRU7ZeEDXJxaHyFA5etQU/Zv0B9FZqUc77VPOSLQ0wtXlfJ uGKmqsD+BDTwYuCxPswoQRr+KpldqlbB0gpPXdcmrZRuYB6xrjzYXTtmo 6kixhuAUnXGPcFCrbe2j/iWp36OcipZ+fsXYUZQXhO+9ZNn6ZW7OBJSwB FbmSTnwetYyCtaBdRhzKr1HOdBIwvEXYG0sQ2k5TEYU+GO1NeVnqYPZn/ E9+tC815UniwbfyVmw1yr86+szhCZ8/Ql5LxI3i5Bq/s6j/Dye4tTlBZJ Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10727"; a="421224965" X-IronPort-AV: E=Sophos;i="6.00,209,1681196400"; d="scan'208";a="421224965" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 May 2023 21:28:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10727"; a="736930354" X-IronPort-AV: E=Sophos;i="6.00,209,1681196400"; d="scan'208";a="736930354" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.206.78]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 May 2023 21:28:32 -0700 Date: Wed, 31 May 2023 21:13:02 -0700 Message-ID: <87y1l3vn01.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: <20230531235634.1309525-4-umesh.nerlige.ramappa@intel.com> References: <20230531235634.1309525-1-umesh.nerlige.ramappa@intel.com> <20230531235634.1309525-4-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/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=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Intel-gfx] [PATCH 3/3] i915/perf: Drop the aged_tail from rewind logic 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: Lionel G Landwerlin , intel-gfx@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Wed, 31 May 2023 16:56:34 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > Instead of aged_tail use an iterator that starts from the hw_tail and > goes backward until the oa_buffer.tail looking for valid reports. Hmm I don't think this description is correct. All this patch is doing is the following: a. s/aged_tail/tail/ b. s/tail/iter/ So basically I don't think we need this patch. All we want to do here is change the variable name aged_tail to something else (to completely remove the concept of aging from the OA code) but other changes such as name change to iter etc. is unnecessary. So I would just keep the patch simple and change the name aged_tail to advertized_tail or exported_tail or read_tail, because basically stream->oa_buffer.tail is the tail which the writer updates (or advertizes or exports) for the reader. So we only should rename aged_tail here, the other changes are not needed. We could even squash this change into Patch 1 or Patch 2, since it is really a trivial variable rename. Thanks. -- Ashutosh > > Signed-off-by: Umesh Nerlige Ramappa > --- > drivers/gpu/drm/i915/i915_perf.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915= _perf.c > index beb1269422ca..39f5ab1911c8 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -543,7 +543,7 @@ static bool oa_buffer_check_unlocked(struct i915_perf= _stream *stream) > { > u32 gtt_offset =3D i915_ggtt_offset(stream->oa_buffer.vma); > int report_size =3D stream->oa_buffer.format->size; > - u32 head, tail, aged_tail; > + u32 head, tail, iter; > unsigned long flags; > bool pollin; > u32 hw_tail; > @@ -567,15 +567,14 @@ static bool oa_buffer_check_unlocked(struct i915_pe= rf_stream *stream) > /* Subtract partial amount off the tail */ > hw_tail =3D OA_TAKEN(hw_tail, partial_report_size); > > - > /* NB: The head we observe here might effectively be a little > * out of date. If a read() is in progress, the head could be > * anywhere between this head and stream->oa_buffer.tail. > */ > head =3D stream->oa_buffer.head - gtt_offset; > - aged_tail =3D stream->oa_buffer.tail - gtt_offset; > + tail =3D stream->oa_buffer.tail - gtt_offset; > > - tail =3D hw_tail; > + iter =3D hw_tail; > > /* Walk the stream backward until we find a report with report > * id and timestmap not at 0. Since the circular buffer pointers > @@ -588,23 +587,23 @@ static bool oa_buffer_check_unlocked(struct i915_pe= rf_stream *stream) > * memory in the order they were written to. > * If not : (=E2=95=AF=C2=B0=E2=96=A1=C2=B0=EF=BC=89=E2=95=AF=EF=B8=B5 = =E2=94=BB=E2=94=81=E2=94=BB > */ > - while (OA_TAKEN(tail, aged_tail) >=3D report_size) { > - void *report =3D stream->oa_buffer.vaddr + tail; > + while (OA_TAKEN(iter, tail) >=3D report_size) { > + void *report =3D stream->oa_buffer.vaddr + iter; > > if (oa_report_id(stream, report) || > oa_timestamp(stream, report)) > break; > > - tail =3D (tail - report_size) & (OA_BUFFER_SIZE - 1); > + iter =3D (iter - report_size) & (OA_BUFFER_SIZE - 1); > } > > - if (OA_TAKEN(hw_tail, tail) > report_size && > + if (OA_TAKEN(hw_tail, iter) > report_size && > __ratelimit(&stream->perf->tail_pointer_race)) > drm_notice(&stream->uncore->i915->drm, > "unlanded report(s) head=3D0x%x tail=3D0x%x hw_tail=3D0x%x\n", > head, tail, hw_tail); > > - stream->oa_buffer.tail =3D gtt_offset + tail; > + stream->oa_buffer.tail =3D gtt_offset + iter; > > pollin =3D OA_TAKEN(stream->oa_buffer.tail - gtt_offset, > stream->oa_buffer.head - gtt_offset) >=3D report_size; > -- > 2.36.1 >