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 B3049C07E9D for ; Tue, 27 Sep 2022 17:52:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D6DE310E021; Tue, 27 Sep 2022 17:52:03 +0000 (UTC) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id E6ECF10E021 for ; Tue, 27 Sep 2022 17:51:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1664301118; x=1695837118; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=K1lFK9vJjwDv12Hiv7yKDVNYjFf0oysZrlmZ1PRyNXc=; b=Sc7gLxjexbSDAm6fsV3afYKmkiIjA71Ust7geeFbkT+vtO8kx+XNZ6DH S6ZnEYAfF5LAds6jc9iVBmZ4tM1ZMh06oKk6HFNMZEBDBNGBUAUmgZ/w8 WKzgzA3Z5Dd3xoltud63te8eNKxLhO87i2DwqIcWpttweyIH5tauHC3HJ d00Fnml5/NqVdkT76Mjl7958LvSsJT3/4Ujh0OThzJ0r+O9SF4geS50Cy DsOquGfU33ONrnCtg9iN1gDPTukq9/27a7ulwVyNbCUHWhrsgEi4A6MeU VaTUJHG5kBJ5wiZGw/aKl6OA6+G9vRItdfAbcoY6DFViH5M6hxO4lFDUI w==; X-IronPort-AV: E=McAfee;i="6500,9779,10483"; a="365430500" X-IronPort-AV: E=Sophos;i="5.93,350,1654585200"; d="scan'208";a="365430500" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Sep 2022 10:51:58 -0700 X-IronPort-AV: E=McAfee;i="6500,9779,10483"; a="950377299" X-IronPort-AV: E=Sophos;i="5.93,350,1654585200"; d="scan'208";a="950377299" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.13.135]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Sep 2022 10:51:49 -0700 Date: Tue, 27 Sep 2022 10:51:26 -0700 Message-ID: <87zgek66zl.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: <871qrw7mbn.wl-ashutosh.dixit@intel.com> References: <20220923201154.283784-1-umesh.nerlige.ramappa@intel.com> <20220923201154.283784-15-umesh.nerlige.ramappa@intel.com> <87y1u6uo32.wl-ashutosh.dixit@intel.com> <53226382-9d11-abdf-e907-a972a8f2e535@intel.com> <8735cd7m1f.wl-ashutosh.dixit@intel.com> <871qrw7mbn.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/28.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-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Intel-gfx] [PATCH v2 14/15] drm/i915/guc: Support OA when Wa_16011777198 is enabled 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, 27 Sep 2022 10:34:52 -0700, Dixit, Ashutosh wrote: > > On Tue, 27 Sep 2022 09:11:23 -0700, Umesh Nerlige Ramappa wrote: > > > > On Mon, Sep 26, 2022 at 04:28:44PM -0700, Dixit, Ashutosh wrote: > > > On Mon, 26 Sep 2022 14:17:21 -0700, Belgaumkar, Vinay wrote: > > >> > > >> > > >> On 9/26/2022 11:19 AM, Umesh Nerlige Ramappa wrote: > > >> > On Mon, Sep 26, 2022 at 08:56:01AM -0700, Dixit, Ashutosh wrote: > > >> >> On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote: > > >> >>> > > >> >>> From: Vinay Belgaumkar > > >> >> > > >> >> Hi Umesh/Vinay, > > >> >> > > >> >>> @@ -3254,6 +3265,24 @@ static int i915_oa_stream_init(struct > > >> >>> i915_perf_stream *stream, > > >> >>> =A0=A0=A0=A0intel_engine_pm_get(stream->engine); > > >> >>> =A0=A0=A0=A0intel_uncore_forcewake_get(stream->uncore, FORCEWAKE= _ALL); > > >> >>> > > >> >>> +=A0=A0=A0 /* > > >> >>> +=A0=A0=A0=A0 * Wa_16011777198:dg2: GuC resets render as part of= the Wa. This > > >> >>> causes > > >> >>> +=A0=A0=A0=A0 * OA to lose the configuration state. Prevent this= by overriding > > >> >>> GUCRC > > >> >>> +=A0=A0=A0=A0 * mode. > > >> >>> +=A0=A0=A0=A0 */ > > >> >>> +=A0=A0=A0 if (intel_guc_slpc_is_used(>->uc.guc) && > > >> >>> +=A0=A0=A0=A0=A0=A0=A0 intel_uc_uses_guc_rc(>->uc) && > > >> >> > > >> >> Is this condition above correct? E.g. what happens when: > > >> >> > > >> >> a. GuCRC is used but SLPC is not used? > > >> > > >> Currently, we have no way to separate out GuC RC and SLPC. Both are = on when > > >> guc submission is enabled/supported. So, the above check is kind of > > >> redundant anyways. > > > > > > That is why I was suggesting changing the if check to an assert or > > > drm_err. So looks like it will work with or without GuC RC, but if we= are > > > using GuC RC we should be using SLPC. So: > > > > > > if (GuC_RC && !SLPC) > > > drm_err(); > > > > I am little confused. What's the ask here? Should I just use one of the= se > > conditions? i.e. > > > > if (intel_guc_slpc_is_used(>->uc.guc)) > > I think let's just use intel_uc_uses_guc_rc and drop > intel_guc_slpc_is_used. I am assuming if SLPC is somehow not on and we try > to use SLPC set/unset param, some kind of error will be reported. With the above this patch is also: Reviewed-by: Ashutosh Dixit > > > > ... > > > > Thanks, > > Umesh > > > > > > > >> Thanks, > > >> > > >> Vinay. > > >> > > >> >> b. GuCRC is not used. Don't we need to disable RC6 in host based = RC6 > > >> >> =A0 control? > > >> > > > >> > When using host based rc6, existing OA code is using forcewake and= a > > >> > reference to engine_pm to prevent rc6. Other questions, directing = to > > >> > @Vinay. > > >> > > > >> > Thanks, > > >> > Umesh > > >> > > > >> >> > > >> >> Do we need to worry about these cases? > > >> >> > > >> >> Or if we always expect both GuCRC and SLPC to be used on DG2 then= I > > >> >> think > > >> >> let's get rid of these from the if condition and add a drm_err() = if we > > >> >> see > > >> >> these not being used and OA is being enabled on DG2? > > >> >> > > >> >> Thanks. > > >> >> -- > > >> >> Ashutosh > > >> >> > > >> >>> + (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || > > >> >>> +=A0=A0=A0=A0=A0=A0=A0=A0 IS_DG2_GRAPHICS_STEP(gt->i915, G11, ST= EP_A0, STEP_B0))) { > > >> >>> +=A0=A0=A0=A0=A0=A0=A0 ret =3D intel_guc_slpc_override_gucrc_mod= e(>->uc.guc.slpc, > > >> >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 SLPC_GUCRC_MODE_GUCRC_NO_RC6); > > >> >>> +=A0=A0=A0=A0=A0=A0=A0 if (ret) { > > >> >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 drm_dbg(&stream->perf->i915->= drm, > > >> >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 "Unable to overri= de gucrc mode\n"); > > >> >>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto err_config; > > >> >>> +=A0=A0=A0=A0=A0=A0=A0 } > > >> >>> +=A0=A0=A0 } > > >> >>> + > > >> >>> =A0=A0=A0=A0ret =3D alloc_oa_buffer(stream); > > >> >>> =A0=A0=A0=A0if (ret) > > >> >>> =A0=A0=A0=A0=A0=A0=A0 goto err_oa_buf_alloc; > > >> >>> -- > > >> >>> 2.25.1 > > >> >>>