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 X-Spam-Level: X-Spam-Status: No, score=-4.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,HK_RANDOM_FROM,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2EFE3C433ED for ; Fri, 30 Apr 2021 13:16:01 +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 CBE1E613E8 for ; Fri, 30 Apr 2021 13:16:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CBE1E613E8 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=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E386B6E4AB; Fri, 30 Apr 2021 13:15:59 +0000 (UTC) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id 855316E4AB; Fri, 30 Apr 2021 13:15:58 +0000 (UTC) IronPort-SDR: X4zzf5BXFsqymWF1EQEhTtAAKqB1J+RFZIpKQr34PvHzVs5EOY0WZ8yP9Zea7I4TBtc2HsVYe9 8Sgk8yAuzr+g== X-IronPort-AV: E=McAfee;i="6200,9189,9969"; a="197370246" X-IronPort-AV: E=Sophos;i="5.82,262,1613462400"; d="scan'208";a="197370246" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Apr 2021 06:15:56 -0700 IronPort-SDR: Rt5e4oB+UiUfwqPzPX2fPEoiHl/dgdClUEsnQvDnr9w0e0JcSUXbR2u1kkSR/dhnxzTJv/8Gs5 anaEbQSb4ISQ== X-IronPort-AV: E=Sophos;i="5.82,262,1613462400"; d="scan'208";a="527634016" Received: from redickin-mobl2.ger.corp.intel.com (HELO [10.213.208.173]) ([10.213.208.173]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Apr 2021 06:15:55 -0700 Subject: Re: [Intel-gfx] [PATCH 16/21] drm/i915/gem: Delay context creation To: Daniel Vetter References: <20210423223131.879208-1-jason@jlekstrand.net> <20210423223131.879208-17-jason@jlekstrand.net> <1eb8d34d-463e-3199-cdb0-0dff95e17f7b@linux.intel.com> From: Tvrtko Ursulin Organization: Intel Corporation UK Plc Message-ID: <9fcd03db-3e6b-42ec-7cfa-9149f1e8f36d@linux.intel.com> Date: Fri, 30 Apr 2021 14:15:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US 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 , Maling list - DRI developers , Jason Ekstrand Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 30/04/2021 14:07, Daniel Vetter wrote: > On Fri, Apr 30, 2021 at 2:44 PM Tvrtko Ursulin > wrote: >> On 30/04/2021 13:30, Daniel Vetter wrote: >>> On Fri, Apr 30, 2021 at 1:58 PM Tvrtko Ursulin >>> wrote: >>>> On 30/04/2021 07:53, Daniel Vetter wrote: >>>>> On Thu, Apr 29, 2021 at 11:35 PM Jason Ekstrand wrote: >>>>>> >>>>>> On Thu, Apr 29, 2021 at 2:07 PM Daniel Vetter wrote: >>>>>>> >>>>>>> On Thu, Apr 29, 2021 at 02:01:16PM -0500, Jason Ekstrand wrote: >>>>>>>> On Thu, Apr 29, 2021 at 1:56 PM Daniel Vetter wrote: >>>>>>>>> On Thu, Apr 29, 2021 at 01:16:04PM -0500, Jason Ekstrand wrote: >>>>>>>>>> On Thu, Apr 29, 2021 at 10:51 AM Daniel Vetter wrote: >>>>>>>>>>>> + ret = set_proto_ctx_param(file_priv, pc, args); >>>>>>>>>>> >>>>>>>>>>> I think we should have a FIXME here of not allowing this on some future >>>>>>>>>>> platforms because just use CTX_CREATE_EXT. >>>>>>>>>> >>>>>>>>>> Done. >>>>>>>>>> >>>>>>>>>>>> + if (ret == -ENOTSUPP) { >>>>>>>>>>>> + /* Some params, specifically SSEU, can only be set on fully >>>>>>>>>>> >>>>>>>>>>> I think this needs a FIXME: that this only holds during the conversion? >>>>>>>>>>> Otherwise we kinda have a bit a problem me thinks ... >>>>>>>>>> >>>>>>>>>> I'm not sure what you mean by that. >>>>>>>>> >>>>>>>>> Well I'm at least assuming that we wont have this case anymore, i.e. >>>>>>>>> there's only two kinds of parameters: >>>>>>>>> - those which are valid only on proto context >>>>>>>>> - those which are valid on both (like priority) >>>>>>>>> >>>>>>>>> This SSEU thing looks like a 3rd parameter, which is only valid on >>>>>>>>> finalized context. That feels all kinds of wrong. Will it stay? If yes >>>>>>>>> *ugh* and why? >>>>>>>> >>>>>>>> Because I was being lazy. The SSEU stuff is a fairly complex param to >>>>>>>> parse and it's always set live. I can factor out the SSEU parsing >>>>>>>> code if you want and it shouldn't be too bad in the end. >>>>>>> >>>>>>> Yeah I think the special case here is a bit too jarring. >>>>>> >>>>>> I rolled a v5 that allows you to set SSEU as a create param. I'm not >>>>>> a huge fan of that much code duplication for the SSEU set but I guess >>>>>> that's what we get for deciding to "unify" our context creation >>>>>> parameter path with our on-the-fly parameter path.... >>>>>> >>>>>> You can look at it here: >>>>>> >>>>>> https://gitlab.freedesktop.org/jekstrand/linux/-/commit/c805f424a3374b2de405b7fc651eab551df2cdaf#474deb1194892a272db022ff175872d42004dfda_283_588 >>>>> >>>>> Hm yeah the duplication of the render engine check is a bit annoying. >>>>> What's worse, if you tthrow another set_engines on top it's probably >>>>> all wrong then. The old thing solved that by just throwing that >>>>> intel_context away. >>>>> >>>>> You're also not keeping the engine id in the proto ctx for this, so >>>>> there's probably some gaps there. We'd need to clear the SSEU if >>>>> userspace puts another context there. But also no userspace does that. >>>>> >>>>> Plus cursory review of userspace show >>>>> - mesa doesn't set this >>>>> - compute sets its right before running the batch >>>>> - media sets it as the last thing of context creation >>>> >>>> Noticed a long sub-thread so looked inside.. >>>> >>>> SSEU is a really an interesting one. >>>> >>>> For current userspace limiting to context creation is fine, since it is >>>> only allowed for Icelake/VME use case. But if you notice the comment inside: >>>> >>>> /* ABI restriction - VME use case only. */ >>>> >>>> It is a hint there was, or could be, more to this uapi than that. >>>> >>>> And from memory I think limiting to creation time will nip the hopes >>>> media had to use this dynamically on other platforms in the bud. So not >>>> that good really. They had convincing numbers what gets significantly >>>> better if we allowed dynamic control to this, just that as always, open >>>> source userspace was not there so we never allowed it. However if you >>>> come up with a new world order where it can only be done at context >>>> creation, as said already, the possibility for that improvement (aka >>>> further improving the competitive advantage) is most likely dashed. >>> >>> Hm are you sure that this is create-time only? media-driver uses it >>> like that, but from my checking compute-runtime updates SSEU mode >>> before every execbuf call. So it very much looked like we have to keep >>> this dynamic. >> >> Ah okay, I assumed it's more of the overall drive to eliminate >> set_param. If sseu set_param stays then it's fine for what I had in mind. >> >>> Or do you mean this is defacto dead code? this = compute setting it >>> before every batch I mean here. >> >> No idea, wasn't aware of the compute usage. >> >> Before every execbuf is not very ideal though since we have to inject a >> foreign context operation to update context image, which means stream of >> work belonging to the context cannot be coalesced (assuming it could to >> start with). There is also a hw cost to reconfigure the sseu which adds >> latency on top. > > They filter out no-op changes. I just meant that from look at > compute-runtime, it seems like sseu can change whenever. i915 does it as well for good measure - since the penalty is global we have to. So I guess they don't know when VME block will be used ie it is per batch. Regards, Tvrtko _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel