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 9F1CCC433FE for ; Thu, 20 Oct 2022 18:35:18 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0317910E274; Thu, 20 Oct 2022 18:35:01 +0000 (UTC) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7679010E1F8; Thu, 20 Oct 2022 18:34:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1666290897; x=1697826897; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=wgVR47zUSSPQJfkNmSS45OnoxNGBfkYtueSX7wOHeEc=; b=L5EtQ3c/iE9ns2h1ES/DtCqRvwUBjx9qcQr3HjaiAStt+0MC8a/jR4oQ NG+zsPNZFPv+ayJJ//i7ooqZbwIhT4ssRLkq1Rxe297WbQ9J+ucQYTBdX bQlOWY1vtHWcWSU/7u7ZZAvcXljeOS/jqq2wm0B4K8odyK37qptwiUfWx ObHwU3zmIxTYWda+ZU6nQ2pXliGUfC+wdIAyl0mCDm6conl364TfHgFHQ NAD2NpH7F51/3QpO7Yjx7HsjB9nH5F/V8p6VYJyNQjw/Pao730H3EhoVJ OgRl/boXinm54sdaasUOcNmB7K2EnpX870jma54Idepp00MqtTzb7i/gV g==; X-IronPort-AV: E=McAfee;i="6500,9779,10506"; a="308485416" X-IronPort-AV: E=Sophos;i="5.95,199,1661842800"; d="scan'208";a="308485416" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2022 11:34:56 -0700 X-IronPort-AV: E=McAfee;i="6500,9779,10506"; a="772539734" X-IronPort-AV: E=Sophos;i="5.95,199,1661842800"; d="scan'208";a="772539734" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.153.148]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2022 11:34:55 -0700 Date: Thu, 20 Oct 2022 11:33:23 -0700 Message-ID: <874jvyqr9o.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Vinay Belgaumkar In-Reply-To: <20221020002944.4228-1-vinay.belgaumkar@intel.com> References: <20221020002944.4228-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.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 v3] drm/i915/slpc: Optmize waitboost for SLPC 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 Wed, 19 Oct 2022 17:29:44 -0700, Vinay Belgaumkar wrote: > Hi Vinay, > Waitboost (when SLPC is enabled) results in a H2G message. This can result > in thousands of messages during a stress test and fill up an already full > CTB. There is no need to request for RP0 if GuC is already requesting the > same. But how are we sure that the freq will remain at RP0 in the future (when the waiting request or any requests which are ahead execute)? In the current waitboost implementation, set_param is sent to GuC ahead of the waiting request to ensure that the freq would be max when this waiting request executed on the GPU and the freq is kept at max till this request retires (considering just one waiting request). How can we ensure this if we don't send the waitboost set_param to GuC? I had assumed we'll do this optimization for server parts where min is already RP0 in which case we can completely disable waitboost. But this patch is something else. Thanks. -- Ashutosh > > v2: Add the tracing back, and check requested freq > in the worker thread (Tvrtko) > v3: Check requested freq in dec_waiters as well > > Signed-off-by: Vinay Belgaumkar > --- > drivers/gpu/drm/i915/gt/intel_rps.c | 3 +++ > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 14 +++++++++++--- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c > index fc23c562d9b2..18b75cf08d1b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > @@ -1016,6 +1016,9 @@ void intel_rps_boost(struct i915_request *rq) > if (rps_uses_slpc(rps)) { > slpc = rps_to_slpc(rps); > > + GT_TRACE(rps_to_gt(rps), "boost fence:%llx:%llx\n", > + rq->fence.context, rq->fence.seqno); > + > /* Return if old value is non zero */ > if (!atomic_fetch_inc(&slpc->num_waiters)) > schedule_work(&slpc->boost_work); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > index b7cdeec44bd3..9dbdbab1515a 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > @@ -227,14 +227,19 @@ static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq) > static void slpc_boost_work(struct work_struct *work) > { > struct intel_guc_slpc *slpc = container_of(work, typeof(*slpc), boost_work); > + struct intel_rps *rps = &slpc_to_gt(slpc)->rps; > int err; > > /* > * Raise min freq to boost. It's possible that > * this is greater than current max. But it will > * certainly be limited by RP0. An error setting > - * the min param is not fatal. > + * the min param is not fatal. No need to boost > + * if we are already requesting it. > */ > + if (intel_rps_get_requested_frequency(rps) == slpc->boost_freq) > + return; > + > mutex_lock(&slpc->lock); > if (atomic_read(&slpc->num_waiters)) { > err = slpc_force_min_freq(slpc, slpc->boost_freq); > @@ -728,6 +733,7 @@ int intel_guc_slpc_set_boost_freq(struct intel_guc_slpc *slpc, u32 val) > > void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc) > { > + struct intel_rps *rps = &slpc_to_gt(slpc)->rps; > /* > * Return min back to the softlimit. > * This is called during request retire, > @@ -735,8 +741,10 @@ void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc) > * set_param fails. > */ > mutex_lock(&slpc->lock); > - if (atomic_dec_and_test(&slpc->num_waiters)) > - slpc_force_min_freq(slpc, slpc->min_freq_softlimit); > + if (atomic_dec_and_test(&slpc->num_waiters)) { > + if (intel_rps_get_requested_frequency(rps) != slpc->min_freq_softlimit) > + slpc_force_min_freq(slpc, slpc->min_freq_softlimit); > + } > mutex_unlock(&slpc->lock); > } > > -- > 2.35.1 > 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 A8E4AC43217 for ; Thu, 20 Oct 2022 18:35:05 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4A6F710E1F8; Thu, 20 Oct 2022 18:35:01 +0000 (UTC) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7679010E1F8; Thu, 20 Oct 2022 18:34:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1666290897; x=1697826897; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=wgVR47zUSSPQJfkNmSS45OnoxNGBfkYtueSX7wOHeEc=; b=L5EtQ3c/iE9ns2h1ES/DtCqRvwUBjx9qcQr3HjaiAStt+0MC8a/jR4oQ NG+zsPNZFPv+ayJJ//i7ooqZbwIhT4ssRLkq1Rxe297WbQ9J+ucQYTBdX bQlOWY1vtHWcWSU/7u7ZZAvcXljeOS/jqq2wm0B4K8odyK37qptwiUfWx ObHwU3zmIxTYWda+ZU6nQ2pXliGUfC+wdIAyl0mCDm6conl364TfHgFHQ NAD2NpH7F51/3QpO7Yjx7HsjB9nH5F/V8p6VYJyNQjw/Pao730H3EhoVJ OgRl/boXinm54sdaasUOcNmB7K2EnpX870jma54Idepp00MqtTzb7i/gV g==; X-IronPort-AV: E=McAfee;i="6500,9779,10506"; a="308485416" X-IronPort-AV: E=Sophos;i="5.95,199,1661842800"; d="scan'208";a="308485416" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2022 11:34:56 -0700 X-IronPort-AV: E=McAfee;i="6500,9779,10506"; a="772539734" X-IronPort-AV: E=Sophos;i="5.95,199,1661842800"; d="scan'208";a="772539734" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.153.148]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2022 11:34:55 -0700 Date: Thu, 20 Oct 2022 11:33:23 -0700 Message-ID: <874jvyqr9o.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Vinay Belgaumkar Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/slpc: Optmize waitboost for SLPC In-Reply-To: <20221020002944.4228-1-vinay.belgaumkar@intel.com> References: <20221020002944.4228-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.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 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: Tvrtko Ursulin , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, 19 Oct 2022 17:29:44 -0700, Vinay Belgaumkar wrote: > Hi Vinay, > Waitboost (when SLPC is enabled) results in a H2G message. This can result > in thousands of messages during a stress test and fill up an already full > CTB. There is no need to request for RP0 if GuC is already requesting the > same. But how are we sure that the freq will remain at RP0 in the future (when the waiting request or any requests which are ahead execute)? In the current waitboost implementation, set_param is sent to GuC ahead of the waiting request to ensure that the freq would be max when this waiting request executed on the GPU and the freq is kept at max till this request retires (considering just one waiting request). How can we ensure this if we don't send the waitboost set_param to GuC? I had assumed we'll do this optimization for server parts where min is already RP0 in which case we can completely disable waitboost. But this patch is something else. Thanks. -- Ashutosh > > v2: Add the tracing back, and check requested freq > in the worker thread (Tvrtko) > v3: Check requested freq in dec_waiters as well > > Signed-off-by: Vinay Belgaumkar > --- > drivers/gpu/drm/i915/gt/intel_rps.c | 3 +++ > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 14 +++++++++++--- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c > index fc23c562d9b2..18b75cf08d1b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > @@ -1016,6 +1016,9 @@ void intel_rps_boost(struct i915_request *rq) > if (rps_uses_slpc(rps)) { > slpc = rps_to_slpc(rps); > > + GT_TRACE(rps_to_gt(rps), "boost fence:%llx:%llx\n", > + rq->fence.context, rq->fence.seqno); > + > /* Return if old value is non zero */ > if (!atomic_fetch_inc(&slpc->num_waiters)) > schedule_work(&slpc->boost_work); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > index b7cdeec44bd3..9dbdbab1515a 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > @@ -227,14 +227,19 @@ static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq) > static void slpc_boost_work(struct work_struct *work) > { > struct intel_guc_slpc *slpc = container_of(work, typeof(*slpc), boost_work); > + struct intel_rps *rps = &slpc_to_gt(slpc)->rps; > int err; > > /* > * Raise min freq to boost. It's possible that > * this is greater than current max. But it will > * certainly be limited by RP0. An error setting > - * the min param is not fatal. > + * the min param is not fatal. No need to boost > + * if we are already requesting it. > */ > + if (intel_rps_get_requested_frequency(rps) == slpc->boost_freq) > + return; > + > mutex_lock(&slpc->lock); > if (atomic_read(&slpc->num_waiters)) { > err = slpc_force_min_freq(slpc, slpc->boost_freq); > @@ -728,6 +733,7 @@ int intel_guc_slpc_set_boost_freq(struct intel_guc_slpc *slpc, u32 val) > > void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc) > { > + struct intel_rps *rps = &slpc_to_gt(slpc)->rps; > /* > * Return min back to the softlimit. > * This is called during request retire, > @@ -735,8 +741,10 @@ void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc) > * set_param fails. > */ > mutex_lock(&slpc->lock); > - if (atomic_dec_and_test(&slpc->num_waiters)) > - slpc_force_min_freq(slpc, slpc->min_freq_softlimit); > + if (atomic_dec_and_test(&slpc->num_waiters)) { > + if (intel_rps_get_requested_frequency(rps) != slpc->min_freq_softlimit) > + slpc_force_min_freq(slpc, slpc->min_freq_softlimit); > + } > mutex_unlock(&slpc->lock); > } > > -- > 2.35.1 >