From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 54FCD35961 for ; Tue, 4 Feb 2025 16:30:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.9 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738686640; cv=none; b=otLPFK2g+gH4Uw1yL+Z7lwKCz4+53WuJA7ir1eMWVQgqy+1/9P9yK2hBk9K1CD/sp5sB5gv4WJ0VSr/IjLy2xcPMq2jonFHmESImYNgyGKWIcRo1ZQGXFAo/MByowTVGymsp6KTnM53r6opK6ESZENIH8iP/Y2E4vXTqq801+5M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738686640; c=relaxed/simple; bh=PkwZmtUC3B140391/0DBQB0PbujszBKzTRZVGrCKXN4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=FrK0P3lUNr9e4M3ZENl53dKYSvgmlMmRJkVVea4cAVTu7n/rKGcpeZhGUqUAL0TqrDe/38nMM756PR5065BWhQAQU60CCZTTaZ2+R+rSh5EBag6z+QKFgrvCwc+0vojDpZHG9Bbmi3Hex11/pgb2oUNg3u3CiVlo7ntVYMOkjt8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=AqVB35kg; arc=none smtp.client-ip=198.175.65.9 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="AqVB35kg" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1738686638; x=1770222638; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=PkwZmtUC3B140391/0DBQB0PbujszBKzTRZVGrCKXN4=; b=AqVB35kgqARwT09tC1G1Ox33DX2TpNDp8yUktxfGr4B5XpqUcM6UJ7iG vfqFn6z29IWMF8RUkZRcJnToujsuEjETFqL9TWjukIn2FTc3VOfNqzHFc uBzF1TFqKim5t+/tdtW/P4/6llZ/tOYCBep6GfXaW+QTUZc4BXCo45dK8 ZQcu3EReol2tU/qU9scO48hCpEKPOCnQ924u6aQcg5Rpu7UayHQC63wIM 7UZF560T4CQZblW+zM+x8H/ij6MzJfTbjzxyt5TbnYz9tpgT3yngA1ofe fq16LYHVWLd0R0aPIcEwiU1Spef5x9yp4c0Hfu705lAgXkuz1bh17/o9Y g==; X-CSE-ConnectionGUID: d7zt9tJUTbKius3JdrKIbA== X-CSE-MsgGUID: bIgJwX48QXaOPNiX5Z2XmQ== X-IronPort-AV: E=McAfee;i="6700,10204,11336"; a="61696070" X-IronPort-AV: E=Sophos;i="6.13,258,1732608000"; d="scan'208";a="61696070" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2025 08:30:37 -0800 X-CSE-ConnectionGUID: 5hE16gUPSIS5fAxF0O7Nlw== X-CSE-MsgGUID: jglkcohLRWKUEnKdW4aabg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,258,1732608000"; d="scan'208";a="110520737" Received: from irvmail002.ir.intel.com ([10.43.11.120]) by fmviesa007.fm.intel.com with ESMTP; 04 Feb 2025 08:30:34 -0800 Received: from [10.246.16.98] (mwajdecz-MOBL.ger.corp.intel.com [10.246.16.98]) by irvmail002.ir.intel.com (Postfix) with ESMTP id 2AEAF32EA2; Tue, 4 Feb 2025 16:30:33 +0000 (GMT) Message-ID: <2ced99ce-fd3e-4966-b093-c193b6c8b400@intel.com> Date: Tue, 4 Feb 2025 17:30:32 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH-resent-to-correct-ml 3/8] drm/xe: Add scoped guards for xe_force_wake To: Maarten Lankhorst , intel-xe@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Ingo Molnar , David Lechner , Peter Zijlstra , Will Deacon , Waiman Long , Boqun Feng References: <20250204132238.162608-1-dev@lankhorst.se> <20250204132238.162608-4-dev@lankhorst.se> Content-Language: en-US From: Michal Wajdeczko In-Reply-To: <20250204132238.162608-4-dev@lankhorst.se> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi Maarten, On 04.02.2025 14:22, Maarten Lankhorst wrote: > Instead of finding bugs where we may or may not release force_wake, I've > decided to be inspired by the spinlock guards, and use the same ones to > do xe_force_wake handling. You may want to take a look at [1], which was based on [2], that introduce fw guard class (and it was already acked and reviewed). Merging was postponed only due to a request to prepare larger series that would convert all existing usages to the new model. And similar guard approach for our RPM was proposed in [3] Michal [1] https://patchwork.freedesktop.org/series/141516/ [2] https://patchwork.freedesktop.org/series/134958/ [3] https://patchwork.freedesktop.org/series/134955/ > > Examples are added as documentation in xe_force_wake.c > > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/xe/xe_force_wake.c | 51 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/xe/xe_force_wake.h | 15 +++++++++ > 2 files changed, 66 insertions(+) > > diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c > index 4f6784e5abf88..805c19f6de9e7 100644 > --- a/drivers/gpu/drm/xe/xe_force_wake.c > +++ b/drivers/gpu/drm/xe/xe_force_wake.c > @@ -16,6 +16,57 @@ > > #define XE_FORCE_WAKE_ACK_TIMEOUT_MS 50 > > +/** > + * DOC: Force wake handling > + * > + * Traditionally, the force wake handling has been done using the error prone > + * set of calls: > + * > + * int func(struct xe_force_wake *fw) > + * { > + * unsigned int fw_ref = xe_force_wake_get(fw, XE_FORCEWAKE_ALL); > + * if (!fw_ref) > + * return -ETIMEDOUT; > + * > + * err = do_something(); > + * > + * xe_force_wake_put(fw, fw_ref); > + * return err; > + * } > + * > + * A new, failure-safe approach is by using the scoped helpers, > + * which changes the function to this: > + * > + * int func(struct xe_force_wake *fw) > + * { > + * scoped_cond_guard(xe_force_wake_get, return -ETIMEDOUT, fw, XE_FORCEWAKE_ALL) { > + * return do_something(); > + * } > + * } > + * > + * For completeness, the following options also work: > + * void func(struct xe_force_wake *fw) > + * { > + * scoped_guard(xe_force_wake_get, fw, XE_FORCEWAKE_ALL) { > + * do_something_only_if_fw_acquired(); > + * } > + * } > + * > + * You can use xe_force_wake instead of force_wake_get, if the code > + * must run but errors acquiring ignored: > + * void func(struct xe_force_wake *fw) > + * { > + * scoped_guard(xe_force_wake, fw, XE_FORCEWAKE_ALL) { > + * always_do_something_maybe_fw(); > + * } > + * > + * do_something_no_fw(); > + * > + * guard(xe_force_wake)(fw, XE_FORCEWAKE_ALL); > + * always_do_something_maybe_fw(); > + * } > + */ > + > static const char *str_wake_sleep(bool wake) > { > return wake ? "wake" : "sleep"; > diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h > index 0e3e84bfa51c3..0fb1baae0a3a3 100644 > --- a/drivers/gpu/drm/xe/xe_force_wake.h > +++ b/drivers/gpu/drm/xe/xe_force_wake.h > @@ -9,6 +9,8 @@ > #include "xe_assert.h" > #include "xe_force_wake_types.h" > > +#include > + > struct xe_gt; > > void xe_force_wake_init_gt(struct xe_gt *gt, > @@ -61,4 +63,17 @@ xe_force_wake_ref_has_domain(unsigned int fw_ref, enum xe_force_wake_domains dom > return fw_ref & domain; > } > > +DEFINE_LOCK_GUARD_1(xe_force_wake, struct xe_force_wake, > + _T->fw_ref = xe_force_wake_get(_T->lock, domain), > + xe_force_wake_put(_T->lock, _T->fw_ref), > + unsigned int fw_ref, enum xe_force_wake_domains domain); > + > +DEFINE_LOCK_GUARD_1_COND(xe_force_wake, _get, > + _T->fw_ref = xe_force_wake_get_all(_T->lock, domain), > + enum xe_force_wake_domains domain); > + > +/* Only useful for guard xe_force_wake, guard xe_force_wake_get gets all or nothing */ > +#define xe_force_wake_scope_has_domain(domain) \ > + (xe_force_wake_ref_has_domain(scope.fw_ref, domain)) > + > #endif