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 9E740C636CC for ; Thu, 16 Feb 2023 05:26:48 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 049F810E317; Thu, 16 Feb 2023 05:26:48 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id D796C10E317 for ; Thu, 16 Feb 2023 05:26:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1676525205; x=1708061205; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=rE1m2Cyeca+UCiZOAqIGeVAgaOplgb4fmI4jaSFekLQ=; b=TYh6QvagG5gWEVLeASAcRCQCDF1Pu4/EBsSZzruxcY59oREMHynlUR/9 5HQbgtJ5LBwyM9JmE+GvWhiOVhYTRs7GU4VqJFcOAcVH1k3LkxbH9rgYE HBzd+YcmPzLEv5bHmEhrYCh9gKVqVw1YSxYQZ6CrnME2Q89pNJQER2x74 1OA+oK93CQxkD70WyGDJv0oPv0Jl/PcVJeZ5oRfirQTReIh05mejMC2J8 2j6b2eUi9iPV+wE1ZXnwSk2as7EJtZq4aina/utlodFnqYveXIB78iOxN 44pBpbK3ROdvYXZKvPLuYz5Y8+kWqs0aYHP/Fv3lDD47jpNQk7Lfop0h8 g==; X-IronPort-AV: E=McAfee;i="6500,9779,10622"; a="417851515" X-IronPort-AV: E=Sophos;i="5.97,301,1669104000"; d="scan'208";a="417851515" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Feb 2023 21:26:45 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10622"; a="998885578" X-IronPort-AV: E=Sophos;i="5.97,301,1669104000"; d="scan'208";a="998885578" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.251.28.186]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Feb 2023 21:26:45 -0800 Date: Wed, 15 Feb 2023 21:08:43 -0800 Message-ID: <87zg9emc2s.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa In-Reply-To: <20230215005419.2100887-4-umesh.nerlige.ramappa@intel.com> References: <20230215005419.2100887-1-umesh.nerlige.ramappa@intel.com> <20230215005419.2100887-4-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 3/9] drm/i915/perf: Validate OA sseu config outside switch 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: Lionel G Landwerlin , intel-gfx@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, 14 Feb 2023 16:54:13 -0800, Umesh Nerlige Ramappa wrote: > > Validate the OA sseu config after all params are parsed. Commit messages for all patches need to answer the "why" or the reason for the patch. In this case maybe an overkill but probably something like: Validate the OA sseu config after all params are parsed since the engine can be passed in as part of perf properties. > > Signed-off-by: Umesh Nerlige Ramappa > --- > drivers/gpu/drm/i915/i915_perf.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index a879ae4bf8d7..0b2097ad000e 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -3956,6 +3956,8 @@ static int read_properties_unlocked(struct i915_perf *perf, > u64 __user *uprop = uprops; > u32 i; > int ret; > + bool config_sseu = false; > + struct drm_i915_gem_context_param_sseu user_sseu; nit: longer lines above shorter lines > > memset(props, 0, sizeof(struct perf_open_properties)); > props->poll_oa_period = DEFAULT_POLL_PERIOD_NS; > @@ -4082,8 +4084,6 @@ static int read_properties_unlocked(struct i915_perf *perf, > props->hold_preemption = !!value; > break; > case DRM_I915_PERF_PROP_GLOBAL_SSEU: { > - struct drm_i915_gem_context_param_sseu user_sseu; > - > if (GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 50)) { > drm_dbg(&perf->i915->drm, > "SSEU config not supported on gfx %x\n", > @@ -4098,14 +4098,7 @@ static int read_properties_unlocked(struct i915_perf *perf, > "Unable to copy global sseu parameter\n"); > return -EFAULT; > } > - > - ret = get_sseu_config(&props->sseu, props->engine, &user_sseu); > - if (ret) { > - drm_dbg(&perf->i915->drm, > - "Invalid SSEU configuration\n"); > - return ret; > - } > - props->has_sseu = true; > + config_sseu = true; > break; > } > case DRM_I915_PERF_PROP_POLL_OA_PERIOD: > @@ -4125,6 +4118,15 @@ static int read_properties_unlocked(struct i915_perf *perf, > uprop += 2; > } > > + if (config_sseu) { > + ret = get_sseu_config(&props->sseu, props->engine, &user_sseu); > + if (ret) { > + DRM_DEBUG("Invalid SSEU configuration\n"); drm_dbg? DRM_DEBUG is deprecated? > + return ret; > + } > + props->has_sseu = true; > + } > + > return 0; > } After addressing the above comments: Reviewed-by: Ashutosh Dixit