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 55026C43334 for ; Sat, 25 Jun 2022 03:59:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7FBCD10EFB0; Sat, 25 Jun 2022 03:59:40 +0000 (UTC) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1887010EE38; Sat, 25 Jun 2022 03:59:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1656129579; x=1687665579; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=+lt1jj775IpGxnRP1hPtEExdO/EyzL8m73+oENsDHio=; b=agTFeHm8QWelyVYfshExrK0a/jWo6coCWb6BpcYujJ+XOQJf17cUKbFY 4sYhXhbS6o5vDH/Uaw2hxs76OjOGME6vEmCLP4sTLCwsIUjlXb4QXqpWZ MqAYeIWIOaYavKDh92jeHJ9GZR45ASz8VNp3OgFur9892qBjbwlCuvcDt Zc0iUnimkZClibndtqm3vHeiQtK3shyqRvPugrnItSxr4p72Z1WizJUBX MMn2tzLLyLI9m6B1B5N5ES/8xo5sFMQjgNuIoabeiMsA39UFaS5q0zsZw c2eGrB2f4BVGxIXVwC6OOvf6bOHx1z1LRBlqmpTOMgj7+SuWE5OqnC+Dh g==; X-IronPort-AV: E=McAfee;i="6400,9594,10388"; a="281187501" X-IronPort-AV: E=Sophos;i="5.92,221,1650956400"; d="scan'208";a="281187501" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2022 20:59:38 -0700 X-IronPort-AV: E=Sophos;i="5.92,221,1650956400"; d="scan'208";a="835382888" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.58.126]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2022 20:59:38 -0700 Date: Fri, 24 Jun 2022 20:59:38 -0700 Message-ID: <87k0958kb9.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Vinay Belgaumkar Subject: Re: [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest In-Reply-To: <20220623233320.11041-1-vinay.belgaumkar@intel.com> References: <20220623233320.11041-1-vinay.belgaumkar@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=US-ASCII X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Thu, 23 Jun 2022 16:33:20 -0700, Vinay Belgaumkar wrote: > > +static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq) > +{ > + struct intel_gt *gt = rps_to_gt(rps); > + u32 perf_limit_reasons; > + int err = 0; > > - igt_spinner_end(&spin); > - st_engine_heartbeat_enable(engine); > - } > + err = slpc_set_min_freq(slpc, slpc->rp0_freq); > + if (err) > + return err; > > - pr_info("Max actual frequency for %s was %d\n", > - engine->name, max_act_freq); > + *max_act_freq = intel_rps_read_actual_frequency(rps); > + if (!(*max_act_freq == slpc->rp0_freq)) { nit but '*max_act_freq != slpc->rp0_freq' > + /* Check if there was some throttling by pcode */ > + perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS); > > - /* Actual frequency should rise above min */ > - if (max_act_freq == slpc_min_freq) { > - pr_err("Actual freq did not rise above min\n"); > + /* If not, this is an error */ > + if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) { Still wrong, should be & not && > + pr_err("Pcode did not grant max freq\n"); > err = -EINVAL; > - } > + } else { > + pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons); Another question, why are we using pr_err/info here rather than drm_err/info? pr_err/info is ok for mock selftests since there is no drm device but that is not the case here, I think this is done in other selftests too but maybe fix this as well if we are making so many changes here? Anyway can do later too. So let's settle issues in v2 thread first. Thanks. -- Ashutosh 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 670BCC433EF for ; Sat, 25 Jun 2022 03:59:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DCAC710EE38; Sat, 25 Jun 2022 03:59:39 +0000 (UTC) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1887010EE38; Sat, 25 Jun 2022 03:59:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1656129579; x=1687665579; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=+lt1jj775IpGxnRP1hPtEExdO/EyzL8m73+oENsDHio=; b=agTFeHm8QWelyVYfshExrK0a/jWo6coCWb6BpcYujJ+XOQJf17cUKbFY 4sYhXhbS6o5vDH/Uaw2hxs76OjOGME6vEmCLP4sTLCwsIUjlXb4QXqpWZ MqAYeIWIOaYavKDh92jeHJ9GZR45ASz8VNp3OgFur9892qBjbwlCuvcDt Zc0iUnimkZClibndtqm3vHeiQtK3shyqRvPugrnItSxr4p72Z1WizJUBX MMn2tzLLyLI9m6B1B5N5ES/8xo5sFMQjgNuIoabeiMsA39UFaS5q0zsZw c2eGrB2f4BVGxIXVwC6OOvf6bOHx1z1LRBlqmpTOMgj7+SuWE5OqnC+Dh g==; X-IronPort-AV: E=McAfee;i="6400,9594,10388"; a="281187501" X-IronPort-AV: E=Sophos;i="5.92,221,1650956400"; d="scan'208";a="281187501" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2022 20:59:38 -0700 X-IronPort-AV: E=Sophos;i="5.92,221,1650956400"; d="scan'208";a="835382888" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.58.126]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2022 20:59:38 -0700 Date: Fri, 24 Jun 2022 20:59:38 -0700 Message-ID: <87k0958kb9.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Vinay Belgaumkar In-Reply-To: <20220623233320.11041-1-vinay.belgaumkar@intel.com> References: <20220623233320.11041-1-vinay.belgaumkar@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=US-ASCII Subject: Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest 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, dri-devel@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Thu, 23 Jun 2022 16:33:20 -0700, Vinay Belgaumkar wrote: > > +static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq) > +{ > + struct intel_gt *gt = rps_to_gt(rps); > + u32 perf_limit_reasons; > + int err = 0; > > - igt_spinner_end(&spin); > - st_engine_heartbeat_enable(engine); > - } > + err = slpc_set_min_freq(slpc, slpc->rp0_freq); > + if (err) > + return err; > > - pr_info("Max actual frequency for %s was %d\n", > - engine->name, max_act_freq); > + *max_act_freq = intel_rps_read_actual_frequency(rps); > + if (!(*max_act_freq == slpc->rp0_freq)) { nit but '*max_act_freq != slpc->rp0_freq' > + /* Check if there was some throttling by pcode */ > + perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS); > > - /* Actual frequency should rise above min */ > - if (max_act_freq == slpc_min_freq) { > - pr_err("Actual freq did not rise above min\n"); > + /* If not, this is an error */ > + if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) { Still wrong, should be & not && > + pr_err("Pcode did not grant max freq\n"); > err = -EINVAL; > - } > + } else { > + pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons); Another question, why are we using pr_err/info here rather than drm_err/info? pr_err/info is ok for mock selftests since there is no drm device but that is not the case here, I think this is done in other selftests too but maybe fix this as well if we are making so many changes here? Anyway can do later too. So let's settle issues in v2 thread first. Thanks. -- Ashutosh