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 E97B5C433FE for ; Wed, 23 Nov 2022 03:26:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D599110E03E; Wed, 23 Nov 2022 03:26:02 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1B72B10E03E for ; Wed, 23 Nov 2022 03:25:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1669173959; x=1700709959; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=qUX8r0LhJvoX5mpW20AySj1ZoGiAnpqRt5ZBQt3Td5E=; b=OYyRqKeAkXyvOzomQak5LQepFIQkVmf2L3Qmjm0+i5SKBXd1uUL8MLnS ZEMRFe7JliYqoYVIcJQgMiJznXUzpVInsAZpK3e0o5PxOGTz0aT8JGQBs RO0pLXJvxLI73tGfxHgooceafXPNrEk78IT7bLoMre17sOb/qkE1meihA ACCsZ6D6Fkjze7J4wFbvRL6VsqdVlYTKfleWxbwIpkdOYI/HTHzM+EVbK 4h0kt59pestFNW3Hq5l+zqaCMOroY6Yvm0hsC1i4sRdef19PLnqPpqMB8 hCHo0LEgSVKI5tBqWqgQOfDApCGld4B6/NsZodnARW16/imi6quB7NWSe Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10539"; a="400261845" X-IronPort-AV: E=Sophos;i="5.96,186,1665471600"; d="scan'208";a="400261845" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Nov 2022 19:25:58 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10539"; a="643954582" X-IronPort-AV: E=Sophos;i="5.96,186,1665471600"; d="scan'208";a="643954582" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.210.227]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Nov 2022 19:25:58 -0800 Date: Tue, 22 Nov 2022 19:25:36 -0800 Message-ID: <87sfiawdvz.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: <20221123020700.603256-1-umesh.nerlige.ramappa@intel.com> References: <20221123020700.603256-1-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=US-ASCII Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/perf: Do not parse context image for HSW 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, 22 Nov 2022 18:07:00 -0800, Umesh Nerlige Ramappa wrote: > Hi Umesh, > An earlier commit introduced a mechanism to parse the context image to > find the OA context control offset. This resulted in an NPD on haswell > when gem_context was passed into i915_perf_open_ioctl params. Haswell > does not support logical ring contexts, so ensure that the context image > is parsed only for platforms with logical ring contexts and also > validate lrc_reg_state. > > v2: Fix build failure > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7432 > Fixes: a5c3a3cbf029 ("drm/i915/perf: Determine gen12 oa ctx offset at runtime") > Signed-off-by: Umesh Nerlige Ramappa > --- > drivers/gpu/drm/i915/i915_perf.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 00e09bb18b13..dbd785974f20 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -1383,6 +1383,9 @@ static u32 oa_context_image_offset(struct intel_context *ce, u32 reg) > u32 offset, len = (ce->engine->context_size - PAGE_SIZE) / 4; > u32 *state = ce->lrc_reg_state; > > + if (drm_WARN_ON(&ce->engine->i915->drm, state == NULL)) > + return U32_MAX; > + So if we didn't add the HAS_LOGICAL_RING_CONTEXTS check below state would be NULL correct? I couldn't figure out how it is NULL on HSW looking at the code (with the context image pin/unpin). > for (offset = 0; offset < len; ) { > if (IS_MI_LRI_CMD(state[offset])) { > /* > @@ -1447,7 +1450,8 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) > if (IS_ERR(ce)) > return PTR_ERR(ce); > > - if (engine_supports_mi_query(stream->engine)) { > + if (engine_supports_mi_query(stream->engine) && > + HAS_LOGICAL_RING_CONTEXTS(stream->perf->i915)) { This check looks fine since we seem to be looking inside ce->lrc_reg_state for oactxctrl. Overall looks fine so this is: Reviewed-by: Ashutosh Dixit > /* > * We are enabling perf query here. If we don't find the context > * offset here, just return an error. > -- > 2.36.1 >