From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id 528B410E2C5 for ; Mon, 11 Sep 2023 11:25:11 +0000 (UTC) Message-ID: <3233d106-a8b9-2225-138e-e19473f95a8e@intel.com> Date: Mon, 11 Sep 2023 13:24:54 +0200 Content-Language: en-US To: "Ch, Sai Gowtham" , "Kempczynski, Zbigniew" References: <20230904110458.24177-1-sai.gowtham.ch@intel.com> <20230904110458.24177-3-sai.gowtham.ch@intel.com> <20230904123934.stxmaelmzxg3366l@zkempczy-mobl2> <20230907093628.hlpa7kh47nddjz7z@zkempczy-mobl2> From: Karolina Stolarek In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit 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 7.09.2023 15:36, Ch, Sai Gowtham wrote: > > >> -----Original Message----- >> From: Kempczynski, Zbigniew >> Sent: Thursday, September 7, 2023 3:06 PM >> To: Ch, Sai Gowtham >> Cc: igt-dev@lists.freedesktop.org; Stolarek, Karolina >> >> Subject: Re: [igt-dev] [PATCH i-g-t 2/4] lib/intel_blt: check if blt commands are >> supported by the platforms >> >> On Tue, Sep 05, 2023 at 06:11:54AM +0200, Ch, Sai Gowtham wrote: >>> >>> >>>> -----Original Message----- >>>> From: Kempczynski, Zbigniew >>>> Sent: Monday, September 4, 2023 6:10 PM >>>> To: Ch, Sai Gowtham >>>> Cc: igt-dev@lists.freedesktop.org; Stolarek, Karolina >>>> >>>> Subject: Re: [igt-dev] [PATCH i-g-t 2/4] lib/intel_blt: check if blt >>>> commands are supported by the platforms >>>> >>>> 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. >>> Do you mean path for i915 needs to be enabled ? >> >> Yes, if I'm not wrong PVC will be supported on i915 as either. >> So if blt_has_mem_set() will return true for it it should have the implementation >> in the blitter library. Generally you got almost everything you need >> (mem_set/copy code is already written so what you need is to extract this and >> support two ways of entering it (for i915 and for xe). > > Sure will make this work for both i915 and xe. >> >>>> mem-set and mem-copy commands you've introduced in 1/4 patch should >>>> be part of intel_blt.[ch] library. >>> Do you mean MEM_SET and MEM_COPY in 3/4 patch ? Can you elaborate ? >> >> Related to above comment. You need to have batch commands + prepare >> execution. Batch commands + execution for Xe you already have in the test. I >> would migrate this to intel_blt.c and add i915 execution path. >> > do you mean something similar to "emit_blt_fast_copy" for fast_copy, mem_copy/set has to be migrated to > Intel_blt.c ? I'll jump on this. That would be something similar, yes, but the interface would be different. You can't reuse blt_copy_data struct, and you'll need to define extra structs to describe source, destination and the command itself. Also, I'm not sure if we'd get much value from pipelined mem_copy/set (i.e. many copies issued in one BB), we could have just a single copy/set instead of emitting it. That could stir a debate, but, in my opinion, it should be fine to first finish writing the test as it is, and then extract the implemented functions to intel_blt. Writing an additional path for i915 will take time. Sai, could you check if this is also a priority? If yes, then I'm all for implementing it as a part of the series. If not, well, I would reconsider doing it. All the best, Karolina > > ---- > Ch Sai Gowtham >> Then in the test just call the mem_set or mem_copy depending what you want >> to test to. >> >> -- >> Zbigniew >> >>>> >>>> BTW first patch 1/4 won't compile in step-by-step mode because it >>>> uses code from following patches. >>> I'll correct that in my next patch series. >>> ---- >>> Ch Sai Gowtham >>>> >>>> -- >>>> 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 >>>>>