From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id D889410E068 for ; Mon, 11 Sep 2023 11:09:44 +0000 (UTC) Message-ID: <60144754-af81-692d-a20e-84d4fbd2fcc0@intel.com> Date: Mon, 11 Sep 2023 13:09:31 +0200 Content-Language: en-US To: =?UTF-8?Q?Zbigniew_Kempczy=c5=84ski?= , References: <20230904110458.24177-1-sai.gowtham.ch@intel.com> <20230904110458.24177-3-sai.gowtham.ch@intel.com> <20230904123934.stxmaelmzxg3366l@zkempczy-mobl2> From: Karolina Stolarek In-Reply-To: <20230904123934.stxmaelmzxg3366l@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 2/4] lib/intel_blt: check if blt commands are supported by the platforms 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 4.09.2023 14:39, Zbigniew Kempczyński wrote: > On Mon, Sep 04, 2023 at 04:34:56PM +0530, sai.gowtham.ch@intel.com wrote: >> From: Sai Gowtham Ch >> >> Check if mem-copy/mem-set commands are supported by fd device. >> >> Signed-off-by: Sai Gowtham Ch >> --- >> lib/intel_blt.c | 32 ++++++++++++++++++++++++++++++++ >> lib/intel_blt.h | 2 ++ >> 2 files changed, 34 insertions(+) >> >> diff --git a/lib/intel_blt.c b/lib/intel_blt.c >> index 429511920..b55fa9b52 100644 >> --- a/lib/intel_blt.c >> +++ b/lib/intel_blt.c >> @@ -289,6 +289,38 @@ bool blt_has_block_copy(int fd) >> return blt_supports_command(cmds_info, XY_BLOCK_COPY); >> } >> >> +/** >> + * blt_has_mem_copy >> + * @fd: drm fd >> + * >> + * Check if mem copy is supported by @fd device >> + * >> + * Returns: >> + * true if it does, false otherwise. >> + */ >> +bool blt_has_mem_copy(int fd) >> +{ >> + const struct intel_cmds_info *cmds_info = GET_CMDS_INFO(fd); >> + >> + return blt_supports_command(cmds_info, MEM_COPY); >> +} >> + >> +/** >> + * blt_has_mem_set >> + * @fd: drm fd >> + * >> + * Check if mem set is supported by @fd device >> + * >> + * Returns: >> + * true if it does, false otherwise. >> + */ >> +bool blt_has_mem_set(int fd) >> +{ >> + const struct intel_cmds_info *cmds_info = GET_CMDS_INFO(fd); >> + >> + return blt_supports_command(cmds_info, MEM_SET); >> +} >> + > > It is not true if you'll use those functions in i915 igts. > mem-set and mem-copy commands you've introduced in 1/4 patch > should be part of intel_blt.[ch] library. Following this logic, we'd need to provide XY_SRC_COPY_BLT in intel_blt library as well. We don't -- mostly because intel_blt lacks support for relocations. blt_has_ are lightweight checks that only say if you can issue a specific command to a platform, and they don't depend on a driver. They don't guarantee that a specific blitter function is implemented in intel_blt. This confusion shows that we should've keep intel_cmds_info and intel_blt libraries separated from the beginning. Yes, they are closely related, but it's intel_blt quering intel_cmds_info about hardware capabilities, not implemented features. Many thanks, Karolina > > BTW first patch 1/4 won't compile in step-by-step mode because > it uses code from following patches. > > -- > Zbigniew > > >> /** >> * blt_has_fast_copy >> * @fd: drm fd >> diff --git a/lib/intel_blt.h b/lib/intel_blt.h >> index b8b3d724d..d9c8883c7 100644 >> --- a/lib/intel_blt.h >> +++ b/lib/intel_blt.h >> @@ -175,6 +175,8 @@ bool blt_cmd_has_property(const struct intel_cmds_info *cmds_info, >> uint32_t prop); >> >> bool blt_has_block_copy(int fd); >> +bool blt_has_mem_copy(int fd); >> +bool blt_has_mem_set(int fd); >> bool blt_has_fast_copy(int fd); >> bool blt_has_xy_src_copy(int fd); >> bool blt_has_xy_color(int fd); >> -- >> 2.39.1 >>