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 9A074C41535 for ; Tue, 19 Dec 2023 07:13:29 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F3B6A10E15D; Tue, 19 Dec 2023 07:13:28 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id ADEA910E15D for ; Tue, 19 Dec 2023 07:13:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1702970007; x=1734506007; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=vcZYa61AH9W1Jcmi8KiFAKoodC3VVfjzg9BfCeRoJUE=; b=EPaGQtyp8rhy/Pz4nK3r55xbSESGaY0OFsZc1SkAlzEFUYgcx3BOR3Bh RwDoWgwbM4O+edsZqZwX+E6grGI86bSCKUe1WjsLvV281epZV2Y0YloDe FHRyno/wh/IG7aNCZ8H9KVfCsCn9Tu3jen05vnnozTsQc6xEIxtlliZNL N4TiCGeiiAo2gRO0e9zYL0JqXKB93XPULNQ1GDYQCaLl2A/uYjwbH2wje hEDQKSdvBEjJzvTPaoVJfJF6RrGQZ1+SAPdZ8tVjMgDyplhhKrnXNAMYr f2Ec6adsnzj4KoXrl4mcQgHpo5SvT0aki0vj9dbHl1SbJvIP6p4j+Y5tE A==; X-IronPort-AV: E=McAfee;i="6600,9927,10928"; a="2830345" X-IronPort-AV: E=Sophos;i="6.04,287,1695711600"; d="scan'208";a="2830345" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Dec 2023 23:13:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10928"; a="769129880" X-IronPort-AV: E=Sophos;i="6.04,287,1695711600"; d="scan'208";a="769129880" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.85.106]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Dec 2023 23:13:26 -0800 Date: Mon, 18 Dec 2023 23:13:06 -0800 Message-ID: <87zfy6u0l9.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa Subject: Re: [PATCH] drm/i915/perf: Update handling of MMIO triggered reports In-Reply-To: References: <20231219000543.1087706-1-umesh.nerlige.ramappa@intel.com> <874jgevjzy.wl-ashutosh.dixit@intel.com> <871qbivj2g.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/29.1 (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=ISO-8859-7 Content-Transfer-Encoding: quoted-printable 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 Mon, 18 Dec 2023 22:07:38 -0800, Umesh Nerlige Ramappa wrote: > > On Mon, Dec 18, 2023 at 09:48:39PM -0800, Dixit, Ashutosh wrote: > > On Mon, 18 Dec 2023 21:28:33 -0800, Dixit, Ashutosh wrote: > >> > >> On Mon, 18 Dec 2023 16:05:43 -0800, Umesh Nerlige Ramappa wrote: > >> > > >> > >> Hi Umesh, > >> > >> > On XEHP platforms user is not able to find MMIO triggered reports in= the > >> > OA buffer since i915 squashes the context ID fields. These context ID > >> > fields hold the MMIO trigger markers. > >> > > >> > Update logic to not squash the context ID fields of MMIO triggered > >> > reports. > >> > > >> > Fixes: cba94bbcff08 ("drm/i915/perf: Determine context valid in OA r= eports") > >> > Signed-off-by: Umesh Nerlige Ramappa > >> > --- > >> > drivers/gpu/drm/i915/i915_perf.c | 39 ++++++++++++++++++++++++++++-= --- > >> > 1 file changed, 34 insertions(+), 5 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915= /i915_perf.c > >> > index 7b1c8de2f9cb..2d695818f006 100644 > >> > --- a/drivers/gpu/drm/i915/i915_perf.c > >> > +++ b/drivers/gpu/drm/i915/i915_perf.c > >> > @@ -772,10 +772,6 @@ static int gen8_append_oa_reports(struct i915_p= erf_stream *stream, > >> > * The reason field includes flags identifying what > >> > * triggered this specific report (mostly timer > >> > * triggered or e.g. due to a context switch). > >> > - * > >> > - * In MMIO triggered reports, some platforms do not set the > >> > - * reason bit in this field and it is valid to have a reason > >> > - * field of zero. > >> > */ > >> > reason =3D oa_report_reason(stream, report); > >> > ctx_id =3D oa_context_id(stream, report32); > >> > @@ -787,8 +783,41 @@ static int gen8_append_oa_reports(struct i915_p= erf_stream *stream, > >> > * > >> > * Note: that we don't clear the valid_ctx_bit so userspace can > >> > * understand that the ID has been squashed by the kernel. > >> > + * > >> > + * Update: > >> > + * > >> > + * On XEHP platforms the behavior of context id valid bit has > >> > + * changed compared to prior platforms. To describe this, we > >> > + * define a few terms: > >> > + * > >> > + * context-switch-report: This is a report with the reason type > >> > + * being context-switch. It is generated when a context switches > >> > + * out. > >> > + * > >> > + * context-valid-bit: A bit that is set in the report ID field > >> > + * to indicate that a valid context has been loaded. > >> > + * > >> > + * gpu-idle: A condition characterized by a > >> > + * context-switch-report with context-valid-bit set to 0. > >> > + * > >> > + * On prior platforms, context-id-valid bit is set to 0 only > >> > + * when GPU goes idle. In all other reports, it is set to 1. > >> > + * > >> > + * On XEHP platforms, context-valid-bit is set to 1 in a context > >> > + * switch report if a new context switched in. For all other > >> > + * reports it is set to 0. > >> > + * > >> > + * This change in behavior causes an issue with MMIO triggered > >> > + * reports. MMIO triggered reports have the markers in the > >> > + * context ID field and the context-valid-bit is 0. The logic > >> > + * below to squash the context ID would render the report > >> > + * useless since the user will not be able to find it in the OA > >> > + * buffer. Since MMIO triggered reports exist only on XEHP, > >> > + * we should avoid squashing these for XEHP platforms. > >> > >> Hmm I am wondering if this is over-information and this comment should= be > >> made brief. > > > > Let me try: "For Gen's >=3D 12.50, the context id valid bit is reset wh= en a > > context switches out, but the context id is still valid. Because of thi= s we > > cannot squash the context id in this case". > > > > So this should affect both the regular as well as MMIO triggered cases > > afaiu. > > > > Anyway, please do what you think is right with the comment. I just thou= ght > > I'll chime in. > > The long and descriptive comment is entirely for my benefit. There is a > very good chance I will forget this, so putting it down in the code. Als= o, > I don't see this described in the spec, so thinking that we will benefit > from it by having it here. I can put it in the commit msg instead if that > helps. I've already R-b'd the patch, so entirely your call. So do whatever you're comfortable with :) Ashutosh > > > > >> For the record, here's the explanation of what is happening from Robert > >> Krzemien's email (which at least makes it simpler for me to understand > >> what is happening): > >> > >> For Gen12HP+ (ATS/DG2/PVC/MTL+) platforms, context id valid bit is > >> set only for context switch reports and when a context is being > >> loaded. When exiting a context, a context switch report is > >> generated, ctx id is not zero, but the bit is not set. It allows us > >> to distinguish whether context switch reports are generated due to > >> entering or exiting GPU contexts. Ctx id field is non-zero for > >> context switches and mmio triggers. Other types always have ctx id > >> set to 0. > >> > >> For previous platforms (like Gen12LP, Gen9/11), the bit is set for > >> all types of reports if a context is loaded. But those older > >> platforms don=A2t have mmio triggers. Ctx id field is non-zero for > >> all types of reports if a context is loaded. > >> > >> I don=A2t understand why i915 needs to set ctx id to 0xffffffff if > >> the flag is not set. It has been removed from XE KMD as I remember > >> correctly. > >> > >> > */ > >> > - if (oa_report_ctx_invalid(stream, report)) { > >> > + > >> > + if (oa_report_ctx_invalid(stream, report) && > >> > + GRAPHICS_VER_FULL(stream->engine->i915) < IP_VER(12, 50)) { > >> > ctx_id =3D INVALID_CTX_ID; > >> > oa_context_id_squash(stream, report32); > >> > } > >> > >> So I am assuming there's some unknown reason (maybe not hearing from M= esa?) > >> why we can't get rid of the squashing even for legacy platforms. But t= hat's > >> ok I guess. So this is: > >> > >> Reviewed-by: Ashutosh Dixit