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 74454C7618A for ; Mon, 20 Mar 2023 02:06:22 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id ECF0010E14A; Mon, 20 Mar 2023 02:06:21 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 74F8B10E14A for ; Mon, 20 Mar 2023 02:06: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=1679277979; x=1710813979; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=DhlXG5XAIpCiTXP0IdGLLaMbOxgosxZxKCv8bpdcpJA=; b=cwl+6rxwC81HJ9pLl+Y59D7D7pnIL73eIAc+6d9kgWUKFUzx1uYabpp1 oLzdA48+pgXvM2ta23pf2MOME3iDREe+PQJ+ZkBR8Wn2kR+391XotZXb8 Peqax0oUP9ri0MlO1woIKXDIK7nyXnDflMWEoUHuhREKglgcw+R/hlhUI zC/aLtUJIilkV0VSQcKfAwSdaYHLKc3+/7Us+Fi1cft+yh2FRKLMazK3c TanqWKyK+1NXGVVrPZLR8LRv78AsZIdioTBnssoAH5eSgEpnYyMvMsSYb 1xyY3EpyGfBlMLb6SkpGOvhn/luY1FScnEyaS2sMgFUUoIvaz2hDfkQ37 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10654"; a="401135455" X-IronPort-AV: E=Sophos;i="5.98,274,1673942400"; d="scan'208";a="401135455" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Mar 2023 19:06:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10654"; a="749901188" X-IronPort-AV: E=Sophos;i="5.98,274,1673942400"; d="scan'208";a="749901188" Received: from dcwiggen-mobl1.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.250.140]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Mar 2023 19:06:18 -0700 Date: Sun, 19 Mar 2023 19:06:17 -0700 Message-ID: <87wn3ctbvq.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: <20230317231641.2815418-10-umesh.nerlige.ramappa@intel.com> References: <20230317231641.2815418-1-umesh.nerlige.ramappa@intel.com> <20230317231641.2815418-10-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 v7 09/11] drm/i915/perf: Add support for OA media units 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 Fri, 17 Mar 2023 16:16:39 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > +static void oa_init_groups(struct intel_gt *gt) > +{ > + int i, num_groups = gt->perf.num_perf_groups; > + > + for (i = 0; i < num_groups; i++) { > + struct i915_perf_group *g = >->perf.group[i]; > + > + /* Fused off engines can result in a group with num_engines == 0 */ > + if (g->num_engines == 0) > + continue; > + > + if (i == PERF_GROUP_OAG && gt->type != GT_MEDIA) { > + g->regs = __oag_regs(); > + g->type = TYPE_OAG; > + } else if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) { > + g->regs = __oam_regs(mtl_oa_base[i]); > + g->type = TYPE_OAM; > + } This if/else statement, rather the whole business of gt->perf.group array with PERF_GROUP_OAG and PERF_GROUP_OAM_SAMEDIA_0 both having the same value 0 is definitely a little klunky. Not too sure but a idr based approach might have been cleaner (so not allocate an array at all but just kmalloc each i915_perf_group entry independently and associate with an idr). It might also have been good to just drop num_perf_groups for now and use a value 1 (that is get rid of any num_perf_groups loops) and introduce num_perf_groups later when we had a platform with num_perf_groups > 1. Anyway since I said earlier let's keep it, let's do that and merge this first (there is already too much code here) and revisit afterwards and see if we can improve anything. Rest looks good now: Reviewed-by: Ashutosh Dixit