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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2CAE4C433EF for ; Tue, 5 Oct 2021 13:07:46 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id D3DEA61354 for ; Tue, 5 Oct 2021 13:07:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D3DEA61354 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 243936EB7B; Tue, 5 Oct 2021 13:07:45 +0000 (UTC) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id A5A7E6EB7B; Tue, 5 Oct 2021 13:07:42 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10127"; a="248991178" X-IronPort-AV: E=Sophos;i="5.85,348,1624345200"; d="scan'208";a="248991178" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Oct 2021 06:05:29 -0700 X-IronPort-AV: E=Sophos;i="5.85,348,1624345200"; d="scan'208";a="438687122" Received: from jlangp-mobl1.ger.corp.intel.com (HELO [10.249.254.71]) ([10.249.254.71]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Oct 2021 06:05:27 -0700 Message-ID: <3aa70cb9-c28b-b85d-eac0-b3f5cca5bf73@linux.intel.com> Date: Tue, 5 Oct 2021 15:05:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 Content-Language: en-US To: Tvrtko Ursulin , Intel-gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org, Tvrtko Ursulin , Daniel Vetter , Matthew Auld References: <20211005113135.768295-1-tvrtko.ursulin@linux.intel.com> From: =?UTF-8?Q?Thomas_Hellstr=c3=b6m?= In-Reply-To: <20211005113135.768295-1-tvrtko.ursulin@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Intel-gfx] [PATCH] drm/i915: Handle Intel igfx + Intel dgfx hybrid graphics setup 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Hi, Tvrtko, On 10/5/21 13:31, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > In short this makes i915 work for hybrid setups (DRI_PRIME=1 with Mesa) > when rendering is done on Intel dgfx and scanout/composition on Intel > igfx. > > Before this patch the driver was not quite ready for that setup, mainly > because it was able to emit a semaphore wait between the two GPUs, which > results in deadlocks because semaphore target location in HWSP is neither > shared between the two, nor mapped in both GGTT spaces. > > To fix it the patch adds an additional check to a couple of relevant code > paths in order to prevent using semaphores for inter-engine > synchronisation when relevant objects are not in the same GGTT space. > > v2: > * Avoid adding rq->i915. (Chris) > > v3: > * Use GGTT which describes the limit more precisely. > > Signed-off-by: Tvrtko Ursulin > Cc: Daniel Vetter > Cc: Matthew Auld > Cc: Thomas Hellström An IMO pretty important bugfix. I read up a bit on the previous discussion on this, and from what I understand the other two options were 1) Ripping out the semaphore code, 2) Consider dma-fences from other instances of the same driver as foreign. For imported dma-bufs we do 2), but particularly with lmem and p2p that's a more straightforward decision. I don't think 1) is a reasonable approach to fix this bug, (but perhaps as a general cleanup?), and for 2) yes I guess we might end up doing that, unless we find some real benefits in treating same-driver-separate-device dma-fences as local, but for this particular bug, IMO this is a reasonable fix. So, Reviewed-by: Thomas Hellström > --- > drivers/gpu/drm/i915/i915_request.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index 79da5eca60af..4f189982f67e 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1145,6 +1145,12 @@ __emit_semaphore_wait(struct i915_request *to, > return 0; > } > > +static bool > +can_use_semaphore_wait(struct i915_request *to, struct i915_request *from) > +{ > + return to->engine->gt->ggtt == from->engine->gt->ggtt; > +} > + > static int > emit_semaphore_wait(struct i915_request *to, > struct i915_request *from, > @@ -1153,6 +1159,9 @@ emit_semaphore_wait(struct i915_request *to, > const intel_engine_mask_t mask = READ_ONCE(from->engine)->mask; > struct i915_sw_fence *wait = &to->submit; > > + if (!can_use_semaphore_wait(to, from)) > + goto await_fence; > + > if (!intel_context_use_semaphores(to->context)) > goto await_fence; > > @@ -1256,7 +1265,8 @@ __i915_request_await_execution(struct i915_request *to, > * immediate execution, and so we must wait until it reaches the > * active slot. > */ > - if (intel_engine_has_semaphores(to->engine) && > + if (can_use_semaphore_wait(to, from) && > + intel_engine_has_semaphores(to->engine) && > !i915_request_has_initial_breadcrumb(to)) { > err = __emit_semaphore_wait(to, from, from->fence.seqno - 1); > if (err < 0)