From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id 83D4F10E547 for ; Thu, 1 Dec 2022 07:38:25 +0000 (UTC) Message-ID: <6ffd7d2f-8f3e-30ba-2c31-6363067b85ba@intel.com> Date: Thu, 1 Dec 2022 08:38:10 +0100 Content-Language: en-US To: =?UTF-8?Q?Zbigniew_Kempczy=c5=84ski?= References: <7ccb29a2e47d491737cc1cae57f30a5c1bdf1e0f.1669791691.git.karolina.stolarek@intel.com> <20221201071841.i35kqoffujv7sjic@zkempczy-mobl2> From: Karolina Stolarek In-Reply-To: <20221201071841.i35kqoffujv7sjic@zkempczy-mobl2> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH i-g-t 1/2] lib/intel_batchbuffer: Make find_engine() more flexible List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On 01.12.2022 08:18, Zbigniew Kempczyński wrote: > On Wed, Nov 30, 2022 at 08:07:59AM +0100, Karolina Stolarek wrote: >> find_engine() depends on intel_bb struct, making it hard to >> use outside of this context. As this function cares only about >> intel_ctx_cfg, not intel_bb itself, let's pass the context >> config in directly. >> >> Signed-off-by: Karolina Stolarek >> --- >> lib/intel_batchbuffer.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c >> index 19a1fbe4..00a263f2 100644 >> --- a/lib/intel_batchbuffer.c >> +++ b/lib/intel_batchbuffer.c >> @@ -1500,13 +1500,11 @@ static bool has_ctx_cfg(struct intel_bb *ibb) >> return ibb->cfg && ibb->cfg->num_engines > 0; >> } >> >> -static uint32_t find_engine(struct intel_bb *ibb, unsigned int class) >> +static uint32_t find_engine(const intel_ctx_cfg_t *cfg, unsigned int class) > > Probably in the future this might be subject of extension with instance. > Currently this design is limiting us to single (first) blitter. I'll argue that even in such scenario we'd still be interested in the first instance of the blitter, as in general it has more capabilities and is more versatile. > Easiest way would be put responsiblity of passing engine id to the caller, > but then we need to do more work in all igts. So for single case: > > Reviewed-by: Zbigniew Kempczyński Thanks! Karolina > -- > Zbigniew > >> { >> - intel_ctx_cfg_t *cfg; >> unsigned int i; >> uint32_t engine_id = -1; >> >> - cfg = ibb->cfg; >> for (i = 0; i < cfg->num_engines; i++) { >> if (cfg->engines[i].engine_class == class) >> engine_id = i; >> @@ -2833,7 +2831,7 @@ void intel_bb_flush_render(struct intel_bb *ibb) >> return; >> >> if (has_ctx_cfg(ibb)) >> - ring = find_engine(ibb, I915_ENGINE_CLASS_RENDER); >> + ring = find_engine(ibb->cfg, I915_ENGINE_CLASS_RENDER); >> else >> ring = I915_EXEC_RENDER; >> >> @@ -2856,7 +2854,7 @@ void intel_bb_flush_blit(struct intel_bb *ibb) >> return; >> >> if (has_ctx_cfg(ibb)) >> - ring = find_engine(ibb, I915_ENGINE_CLASS_COPY); >> + ring = find_engine(ibb->cfg, I915_ENGINE_CLASS_COPY); >> else >> ring = HAS_BLT_RING(ibb->devid) ? I915_EXEC_BLT : I915_EXEC_DEFAULT; >> >> -- >> 2.25.1 >>