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 6D74EEE01E0 for ; Wed, 13 Sep 2023 16:59:22 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B152310E4BB; Wed, 13 Sep 2023 16:59:21 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id B1A1910E4BB for ; Wed, 13 Sep 2023 16:59:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1694624359; x=1726160359; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=SHuU+0/rw+gyQ7Ua4poBcl84qu1Kt7/sQxbu2ovY7gs=; b=U401s8WD+5Ic3CS06WdSk99BOZ8MbAAyxegH3WxGsKwhijJygrVBEHlZ IHkpWh373rl4o+A08wpge7mr8jo+Pm0JG4D7GJcdagU0P8jRvXib0xZxy xn/OMHD4JLyJJpaQ38u737A2FQ19zEMc54haPbZJs4+SsMT9x98AC+Y3J MytwgOIvltRISKbetcLxVLNwYhFId8RQUpPbxxdB+Yixaid43nXpaMlRb nEWVczz1mjpaYUmpQhEqGOXhZLIZw5soaMPXHs+E0tbW75RBLKgl9jIvy 56p0bKoR8BzMUFikkEQQXbfndYEidUjpo70w53sM1ZuQUhq8F+ecGN8GV w==; X-IronPort-AV: E=McAfee;i="6600,9927,10832"; a="378632881" X-IronPort-AV: E=Sophos;i="6.02,143,1688454000"; d="scan'208";a="378632881" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Sep 2023 09:59:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10832"; a="887403196" X-IronPort-AV: E=Sophos;i="6.02,143,1688454000"; d="scan'208";a="887403196" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.251.12.129]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Sep 2023 09:58:48 -0700 Date: Wed, 13 Sep 2023 09:59:18 -0700 Message-ID: <87h6nyt3fd.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: References: <20230909011626.1643734-1-ashutosh.dixit@intel.com> <20230909011626.1643734-4-ashutosh.dixit@intel.com> <87r0n8qgu7.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=US-ASCII Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915/perf: Initialize gen12 OA buffer unconditionally 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, 12 Sep 2023 18:46:12 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > On Fri, Sep 08, 2023 at 06:24:16PM -0700, Dixit, Ashutosh wrote: > > On Fri, 08 Sep 2023 18:16:26 -0700, Ashutosh Dixit wrote: > >> > > > >> From: Umesh Nerlige Ramappa > >> > >> Correct values for OAR counters are still dependent on enabling the > >> GEN12_OAG_OACONTROL_OA_COUNTER_ENABLE in OAG_OACONTROL. Enabling this > >> bit means OAG unit will write reports to the OAG buffer, so > >> initialize the OAG buffer unconditionally for all use cases. > >> > >> BSpec: 46822 > >> > >> Signed-off-by: Umesh Nerlige Ramappa > >> Signed-off-by: Ashutosh Dixit > >> --- > >> drivers/gpu/drm/i915/i915_perf.c | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > >> index 1347e4ec9dd5a..30cf37d6e79be 100644 > >> --- a/drivers/gpu/drm/i915/i915_perf.c > >> +++ b/drivers/gpu/drm/i915/i915_perf.c > >> @@ -3032,12 +3032,12 @@ static void gen12_oa_enable(struct i915_perf_stream *stream) > >> u32 val; > >> > >> /* > >> - * If we don't want OA reports from the OA buffer, then we don't even > >> - * need to program the OAG unit. > >> + * BSpec: 46822 > >> + * Correct values for OAR counters are still dependent on enabling the > >> + * GEN12_OAG_OACONTROL_OA_COUNTER_ENABLE in OAG_OACONTROL. Enabling this > >> + * bit means OAG unit will write reports to the OAG buffer, so > >> + * initialize the OAG buffer correctly. > >> */ > >> - if (!(stream->sample_flags & SAMPLE_OA_REPORT)) > >> - return; > >> - > >> gen12_init_oa_buffer(stream); > >> > >> regs = __oa_regs(stream); > > > > Looks like this should be needed, I can R-b it. > > > > However, gen12_test_mi_rpc IGT says: > > > > /* OA unit configuration: > > * DRM_I915_PERF_PROP_SAMPLE_OA is no longer required for Gen12 > > * because the OAR unit increments counters only for the > > * relevant context. No other parameters are needed since we do > > * not rely on the OA buffer anymore to normalize the counter > > * values. > > */ > > That's wrong. When TGL support was added, this was misunderstood and I > removed the OAR-OAG dependency. Ideally we should enforce user to pass > SAMPLE_OA always, but now that will break uabi. SAMPLE_OA has other effects like enabling the timer etc. Also, if we enforce user to always pass it, we can do it in the kernel itself and ignore the uapi parameter. > > For for the OAR case, let's just enable OAG unconditionally so that the OAR > counters tick correctly. While we do that, we should disable all events > that trigger a report into the OA buffer. In addition, I would also > allocate the smallest OA buffer size for this case, so that memory impact > is low. > > Needs a Fixes tag with the commit that enabled OA for TGL. If you think this patch is needed, I think it's better you do this so that you can stay the patch author and I can be the reviewer, otherwise we'll need to go hunt for another reviewer. Thanks. -- Ashutosh > > > > So gen12_test_mi_rpc doesn't set DRM_I915_PERF_PROP_SAMPLE_OA and also > > seems to be passing in CI (don't see it but there seem to be no open > > bugs). Thoughts? > > > > Thanks. > > -- > > Ashutosh