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 5D4CFD60D0A for ; Tue, 19 Nov 2024 01:36:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2C8FF10E1A6; Tue, 19 Nov 2024 01:36:03 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="bUNheYDD"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9277E10E1A6 for ; Tue, 19 Nov 2024 01:36:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1731980162; x=1763516162; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=/s9T2in4VrPqDstepKaJocu87J+CUHKcRBStotziGQg=; b=bUNheYDDP4vlC38AwRiR3QKj5Zej6fTG7IWb7IQzFJDOT0bRl2shQxVt f8nlNsx7Xx35Lsp/PeURtv8NmH+tZm2wJjbD4JVaRlqQgeL6QL2XHB1Ad ELrDo62NAS+S1CA4UuxGuanbL20GB1Fh0wz95yoL9EPlgXbpnJN4lMrgm iSlhr3O9odRNmWoGFSZhZ7w9n1N1hy70U2rH1yzJGviUKuz3W3D9V8d+z UpDd3j0LckCn+JlCD7nz6vLPoXgzRpXIDhAcN8xK+pIyMF/TUenJZTIHU o00RitP6cDn8u4qgUHZvVKSISADDeTrv6YNz07JaGN5+552lxWp95NbFL Q==; X-CSE-ConnectionGUID: 1v/gwoz5TvidRBzRR+B5mw== X-CSE-MsgGUID: K0MDOeIdRTa0uFYvDUuT7A== X-IronPort-AV: E=McAfee;i="6700,10204,11260"; a="43032086" X-IronPort-AV: E=Sophos;i="6.12,165,1728975600"; d="scan'208";a="43032086" Received: from fmviesa006.fm.intel.com ([10.60.135.146]) by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Nov 2024 17:36:02 -0800 X-CSE-ConnectionGUID: 3Ht1f+60T0WzkrCL/bR9pA== X-CSE-MsgGUID: sE3GVELFR8uJW7pzz5XNag== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.12,165,1728975600"; d="scan'208";a="88963619" Received: from orsosgc001.jf.intel.com (HELO orsosgc001.intel.com) ([10.165.21.142]) by fmviesa006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Nov 2024 17:36:01 -0800 Date: Mon, 18 Nov 2024 17:36:00 -0800 Message-ID: <85serovwcf.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Cavitt, Jonathan" Cc: "intel-xe@lists.freedesktop.org" , "Nerlige Ramappa, Umesh" Subject: Re: [PATCH 2/2] drm/xe/oa: Allow subsequent OA streams to be opened on an exec_queue In-Reply-To: References: <20241108230602.2984959-1-ashutosh.dixit@intel.com> <20241108230602.2984959-3-ashutosh.dixit@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-redhat-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: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Fri, 08 Nov 2024 15:42:40 -0800, Cavitt, Jonathan wrote: > Hi Jonathan, > -----Original Message----- > From: Intel-xe On Behalf Of Ashutosh Dixit > Sent: Friday, November 8, 2024 3:06 PM > To: intel-xe@lists.freedesktop.org > Cc: Nerlige Ramappa, Umesh > Subject: [PATCH 2/2] drm/xe/oa: Allow subsequent OA streams to be opened on an exec_queue > > > > The restriction that OA not be enabled on an active exec queue only applies > > to the first OA stream opened on the exec queue. Subsequent OA streams can > > be opened on the exec queue since this will not toggle the > > OAC_CONTEXT_ENABLE bit. > > > > Fixes: 2f4a730fcd2d ("drm/xe/oa: Add OAR support") > > Cc: stable@vger.kernel.org > > Signed-off-by: Ashutosh Dixit > > --- > > drivers/gpu/drm/xe/xe_exec_queue_types.h | 2 ++ > > drivers/gpu/drm/xe/xe_oa.c | 8 ++++++-- > > 2 files changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h b/drivers/gpu/drm/xe/xe_exec_queue_types.h > > index 1158b6062a6cd..6e3311c22404e 100644 > > --- a/drivers/gpu/drm/xe/xe_exec_queue_types.h > > +++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h > > @@ -142,6 +142,8 @@ struct xe_exec_queue { > > u64 tlb_flush_seqno; > > /** @hw_engine_group_link: link into exec queues in the same hw engine group */ > > struct list_head hw_engine_group_link; > > + /** @oa: this exec_queue is used for OA (OAR/OAC) */ > > + bool oa; > > Opening the first OA stream sets the OAC_CONTEXT_ENABLE bit. Would it be possible > to read that bit instead of adding an additional bool to the exec queue? > > I won't block on this, just asking. It was indeed possible, thanks for the suggestion. I have implemented it in v2 of Patch 1 and dropped this Patch 2, since it doesn't seem to be needed if we read the bit directly. Note though the register is read from the context image, it is not an mmio register. I have also dropped your R-b from Patch 1, since the patch is significantly different now (after addressing both yours and Matt Brost's reviews). So if you could please take a quick look again that would be great. Thanks. Ashutosh > -Jonathan Cavitt > > > /** @lrc: logical ring context for this exec queue */ > > struct xe_lrc *lrc[] __counted_by(width); > > }; > > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c > > index b0692b8ca0a3d..53d2946ea3052 100644 > > --- a/drivers/gpu/drm/xe/xe_oa.c > > +++ b/drivers/gpu/drm/xe/xe_oa.c > > @@ -1915,6 +1915,9 @@ static int xe_oa_stream_open_ioctl_locked(struct xe_oa *oa, > > goto err_disable; > > } > > > > + if (stream->exec_q && !stream->exec_q->oa) > > + stream->exec_q->oa = true; > > + > > /* Hold a reference on the drm device till stream_fd is released */ > > drm_dev_get(&stream->oa->xe->drm); > > > > @@ -2068,9 +2071,10 @@ int xe_oa_stream_open_ioctl(struct drm_device *dev, u64 data, struct drm_file *f > > /* > > * Disallow OA from being enabled on active exec_queue's. Enabling OA toggles > > * the OAC_CONTEXT_ENABLE bit in CTXT_SR_CTL register, which changes the size > > - * and layout of the underlying HW context image and can cause hangs. > > + * and layout of the underlying HW context image and can cause hangs. This > > + * restriction holds only for the first OA stream opened on the exec queue. > > */ > > - if (XE_IOCTL_DBG(oa->xe, exec_queue_enabled(param.exec_q))) { > > + if (XE_IOCTL_DBG(oa->xe, !param.exec_q->oa && exec_queue_enabled(param.exec_q))) { > > ret = -EADDRINUSE; > > goto err_exec_q; > > } > > -- > > 2.41.0 > > > >