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 EC217C43334 for ; Mon, 25 Jul 2022 18:25:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 15392C1685; Mon, 25 Jul 2022 18:25:12 +0000 (UTC) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id E7D1BC1682; Mon, 25 Jul 2022 18:25:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1658773509; x=1690309509; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=tOMtX78eE+IXoIUQx+zeTWTujw13O8VU3gTwY0k38Pc=; b=KlAq+QMgOvdHm1OgQfxqOYZMuN3dE58h+sSvZcg9lFoVmDO2tD+GSIM3 4My2hSoRU2foqo+zWQ0wuUdWqVF0Q1yIgwxuvAV298Kd+itzwd/V0BbL6 8NpfiwjDouTQ48NIqoSGomrRoKu5gEY1D7id60X+wn3/Bl/OCK0SWVLb9 qxbA/U3vUTBPnl4DFDuixhZy7GmLnhNfDjcPoHlX07Bf0R/VZuA/AuDI5 C66C7u+HVlXokXs7X8uY0pmMWvNqIoJv8VwFdbe3+AtfTQAX28XrleInH f0bgeOzMDfz2cNjz2ngL8zV1evpAIS0EUuoIurmikRrDBmT+iVdFCNIp/ w==; X-IronPort-AV: E=McAfee;i="6400,9594,10419"; a="270802115" X-IronPort-AV: E=Sophos;i="5.93,193,1654585200"; d="scan'208";a="270802115" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jul 2022 11:25:09 -0700 X-IronPort-AV: E=Sophos;i="5.93,193,1654585200"; d="scan'208";a="741923418" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.182.7]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jul 2022 11:25:08 -0700 Date: Mon, 25 Jul 2022 11:18:40 -0700 Message-ID: <87k081ginz.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Zhanjun Dong In-Reply-To: <20220715211313.143645-1-zhanjun.dong@intel.com> References: <20220715211313.143645-1-zhanjun.dong@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: Check for ct enabled while waiting for response 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 Fri, 15 Jul 2022 14:13:13 -0700, Zhanjun Dong wrote: > > -static int wait_for_ct_request_update(struct ct_request *req, u32 *status) > +static int wait_for_ct_request_update(struct intel_guc_ct *ct, struct ct_request *req, u32 *status) > { > int err; > + bool ct_enabled; > > /* > * Fast commands should complete in less than 10us, so sample quickly > @@ -481,12 +483,15 @@ static int wait_for_ct_request_update(struct ct_request *req, u32 *status) > #define GUC_CTB_RESPONSE_TIMEOUT_SHORT_MS 10 > #define GUC_CTB_RESPONSE_TIMEOUT_LONG_MS 1000 > #define done \ > - (FIELD_GET(GUC_HXG_MSG_0_ORIGIN, READ_ONCE(req->status)) == \ > + (!(ct_enabled = intel_guc_ct_enabled(ct)) || \ > + FIELD_GET(GUC_HXG_MSG_0_ORIGIN, READ_ONCE(req->status)) == \ > GUC_HXG_ORIGIN_GUC) > err = wait_for_us(done, GUC_CTB_RESPONSE_TIMEOUT_SHORT_MS); > if (err) > err = wait_for(done, GUC_CTB_RESPONSE_TIMEOUT_LONG_MS); > #undef done > + if (!ct_enabled) > + err = -ENODEV; Good, -ENODEV seems to be the correct return value in this case. > > *status = req->status; > return err; > @@ -703,11 +708,18 @@ static int ct_send(struct intel_guc_ct *ct, > > intel_guc_notify(ct_to_guc(ct)); > > - err = wait_for_ct_request_update(&request, status); > + err = wait_for_ct_request_update(ct, &request, status); > g2h_release_space(ct, GUC_CTB_HXG_MSG_MAX_LEN); > if (unlikely(err)) { > - CT_ERROR(ct, "No response for request %#x (fence %u)\n", > - action[0], request.fence); > + if (err == -ENODEV) > + /* wait_for_ct_request_update returns -ENODEV on reset/suspend in progress. > + * In this case, output is debug rather than error info > + */ > + CT_DEBUG(ct, "Request %#x (fence %u) cancelled as CTB is disabled\n", > + action[0], request.fence); > + else > + CT_ERROR(ct, "No response for request %#x (fence %u)\n", > + action[0], request.fence); A nit but I would probably prefer to move the CT_DEBUG() inside wait_for_ct_request_update() (so we only keep 'if (err != -ENODEV)' checks here) though it would mean adding the action argument also to wait_for_ct_request_update(). In any case, since we have discussed this patch ad nauseam previously, this is now: Reviewed-by: Ashutosh Dixit 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 D9A62C433EF for ; Mon, 25 Jul 2022 18:25:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 81AF3C1683; Mon, 25 Jul 2022 18:25:11 +0000 (UTC) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id E7D1BC1682; Mon, 25 Jul 2022 18:25:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1658773509; x=1690309509; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=tOMtX78eE+IXoIUQx+zeTWTujw13O8VU3gTwY0k38Pc=; b=KlAq+QMgOvdHm1OgQfxqOYZMuN3dE58h+sSvZcg9lFoVmDO2tD+GSIM3 4My2hSoRU2foqo+zWQ0wuUdWqVF0Q1yIgwxuvAV298Kd+itzwd/V0BbL6 8NpfiwjDouTQ48NIqoSGomrRoKu5gEY1D7id60X+wn3/Bl/OCK0SWVLb9 qxbA/U3vUTBPnl4DFDuixhZy7GmLnhNfDjcPoHlX07Bf0R/VZuA/AuDI5 C66C7u+HVlXokXs7X8uY0pmMWvNqIoJv8VwFdbe3+AtfTQAX28XrleInH f0bgeOzMDfz2cNjz2ngL8zV1evpAIS0EUuoIurmikRrDBmT+iVdFCNIp/ w==; X-IronPort-AV: E=McAfee;i="6400,9594,10419"; a="270802115" X-IronPort-AV: E=Sophos;i="5.93,193,1654585200"; d="scan'208";a="270802115" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jul 2022 11:25:09 -0700 X-IronPort-AV: E=Sophos;i="5.93,193,1654585200"; d="scan'208";a="741923418" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.182.7]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jul 2022 11:25:08 -0700 Date: Mon, 25 Jul 2022 11:18:40 -0700 Message-ID: <87k081ginz.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Zhanjun Dong Subject: Re: [PATCH] drm/i915/guc: Check for ct enabled while waiting for response In-Reply-To: <20220715211313.143645-1-zhanjun.dong@intel.com> References: <20220715211313.143645-1-zhanjun.dong@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 Fri, 15 Jul 2022 14:13:13 -0700, Zhanjun Dong wrote: > > -static int wait_for_ct_request_update(struct ct_request *req, u32 *status) > +static int wait_for_ct_request_update(struct intel_guc_ct *ct, struct ct_request *req, u32 *status) > { > int err; > + bool ct_enabled; > > /* > * Fast commands should complete in less than 10us, so sample quickly > @@ -481,12 +483,15 @@ static int wait_for_ct_request_update(struct ct_request *req, u32 *status) > #define GUC_CTB_RESPONSE_TIMEOUT_SHORT_MS 10 > #define GUC_CTB_RESPONSE_TIMEOUT_LONG_MS 1000 > #define done \ > - (FIELD_GET(GUC_HXG_MSG_0_ORIGIN, READ_ONCE(req->status)) == \ > + (!(ct_enabled = intel_guc_ct_enabled(ct)) || \ > + FIELD_GET(GUC_HXG_MSG_0_ORIGIN, READ_ONCE(req->status)) == \ > GUC_HXG_ORIGIN_GUC) > err = wait_for_us(done, GUC_CTB_RESPONSE_TIMEOUT_SHORT_MS); > if (err) > err = wait_for(done, GUC_CTB_RESPONSE_TIMEOUT_LONG_MS); > #undef done > + if (!ct_enabled) > + err = -ENODEV; Good, -ENODEV seems to be the correct return value in this case. > > *status = req->status; > return err; > @@ -703,11 +708,18 @@ static int ct_send(struct intel_guc_ct *ct, > > intel_guc_notify(ct_to_guc(ct)); > > - err = wait_for_ct_request_update(&request, status); > + err = wait_for_ct_request_update(ct, &request, status); > g2h_release_space(ct, GUC_CTB_HXG_MSG_MAX_LEN); > if (unlikely(err)) { > - CT_ERROR(ct, "No response for request %#x (fence %u)\n", > - action[0], request.fence); > + if (err == -ENODEV) > + /* wait_for_ct_request_update returns -ENODEV on reset/suspend in progress. > + * In this case, output is debug rather than error info > + */ > + CT_DEBUG(ct, "Request %#x (fence %u) cancelled as CTB is disabled\n", > + action[0], request.fence); > + else > + CT_ERROR(ct, "No response for request %#x (fence %u)\n", > + action[0], request.fence); A nit but I would probably prefer to move the CT_DEBUG() inside wait_for_ct_request_update() (so we only keep 'if (err != -ENODEV)' checks here) though it would mean adding the action argument also to wait_for_ct_request_update(). In any case, since we have discussed this patch ad nauseam previously, this is now: Reviewed-by: Ashutosh Dixit