From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id E51216E39B for ; Fri, 18 Jun 2021 03:42:07 +0000 (UTC) Date: Thu, 17 Jun 2021 20:42:06 -0700 Message-ID: <87k0mret4h.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20210617191256.577244-2-jason@jlekstrand.net> References: <20210617191256.577244-1-jason@jlekstrand.net> <20210617191256.577244-2-jason@jlekstrand.net> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH i-g-t 01/79] lib/i915/gem_submission_measure: Take an optional intel_ctx_cfg_t List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Jason Ekstrand Cc: igt-dev@lists.freedesktop.org List-ID: On Thu, 17 Jun 2021 12:12:08 -0700, Jason Ekstrand wrote: > > If provided, the engine (or ALL_ENGINES) is relative to the given > context config. This is intended to be transitional. We'll get rid of > all the __for_each_physical_engine stuff later. Please add your SOB here. There are couple of comments below but this is: Reviewed-by: Ashutosh Dixit > @@ -372,8 +373,10 @@ __measure_ringsize(int i915, unsigned int engine) > return count / 2 - 2; > } > > -unsigned int gem_submission_measure(int i915, unsigned int engine) > +unsigned int gem_submission_measure(int i915, const intel_ctx_cfg_t *cfg, > + unsigned int engine) I was thinking since we are already creating an intel_ctx_t in the function we can just create an intel_ctx_t for the "engine" arg, so shouldn't need the "cfg". But turns out "engine" is intel_execution_engine2->flags and there is at least for now no straightforward way to create a intel_ctx_t for intel_execution_engine2->flags. That is probably why "cfg" is introduced. > @@ -381,19 +384,41 @@ unsigned int gem_submission_measure(int i915, unsigned int engine) > if (!nonblock) > fcntl(i915, F_SETFL, fcntl(i915, F_GETFL) | O_NONBLOCK); > > + if (cfg) { > + if (gem_has_contexts(i915)) > + ctx = intel_ctx_create(i915, cfg); > + else > + ctx = intel_ctx_0(i915); I would say this check should be done in intel_ctx_create() itself. Right now it is being done in intel_ctx_create_all_physical() but we should move it inside intel_ctx_create() to avoid these kinds of situations. Anyway not an issue for now, we can clean it up later. _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev