[PATCH v2 i-g-t 2/2] benchmarks/gem_wsim: Fix mmap for discrete graphics cards
Bernatowicz, Marcin
marcin.bernatowicz at linux.intel.com
Thu Jul 25 09:52:14 UTC 2024
On 7/25/2024 11:06 AM, Michal Wajdeczko wrote:
>
>
> On 25.07.2024 09:26, Bernatowicz, Marcin wrote:
>> Hi Kamil,
>>
>> On 7/23/2024 12:36 PM, Kamil Konieczny wrote:
>>> Hi Marcin,
>>> On 2023-12-14 at 21:13:02 +0100, Marcin Bernatowicz wrote:
>>>> It appears that 'gem_mmap__wc' doesn't work for discrete graphics cards
>>>> and 'I915_MMAP_OFFSET_FIXED' is needed instead.
>>>> Adopt the mapping approach from 'lib/igt_dummyload'.
>>>>
>>>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
>>>> ---
>>>> benchmarks/gem_wsim.c | 11 ++++++++++-
>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>>>> index a1db37d4e..eafd9cab8 100644
>>>> --- a/benchmarks/gem_wsim.c
>>>> +++ b/benchmarks/gem_wsim.c
>>>> @@ -1502,7 +1502,16 @@ static unsigned int create_bb(struct w_step
>>>> *w, int self)
>>>> gem_set_domain(fd, w->bb_handle,
>>>> I915_GEM_DOMAIN_WC, I915_GEM_DOMAIN_WC);
>>>> - cs = ptr = gem_mmap__wc(fd, w->bb_handle, 0, w->bb_size,
>>>> PROT_WRITE);
>>>> + if (__gem_set_caching(fd, w->bb_handle,
>>>> + I915_CACHING_CACHED) == 0) {
>>>> + cs = ptr = gem_mmap__cpu(fd, w->bb_handle,
>>>> + 0, w->bb_size,
>>>> + PROT_READ | PROT_WRITE);
>>>> + } else
>>>
>>> Be consistent, use brace after 'else':
>>> } else {
>>>
>>>> + cs = ptr = gem_mmap__device_coherent(fd,
>>>> + w->bb_handle,
>>>> + 0, w->bb_size,
>>>> + PROT_READ | PROT_WRITE);
>>>
>>> so it will look like:
>>> } else {
>>> cs = ptr = gem_mmap__device_coherent(fd, w->bb_handle, 0,
>>> w->bb_size,
>>> PROT_READ | PROT_WRITE);
>>> }
>> ok
>>>
>>> Please use checkpatch.pl script from Linux kernel, it could spot
>>> such problems, see also CONTRIBUTE.md for some helpful options.
>>
>> Strange, I did run checkpatch.pl but it didn't shout:
>>
>> -----------------------------------------------------------------
>> ./0001-benchmarks-gem_wsim-Support-gens-without-relocations.patch
>> -----------------------------------------------------------------
>> total: 0 errors, 0 warnings, 230 lines checked
>>
>> ./0001-benchmarks-gem_wsim-Support-gens-without-relocations.patch has no
>> obvious style problems and is ready for submission.
>> -----------------------------------------------------------------
>> ./0002-benchmarks-gem_wsim-Fix-mmap-for-discrete-graphics-c.patch
>> -----------------------------------------------------------------
>> total: 0 errors, 0 warnings, 17 lines checked
>>
>
> try with --strict option
>
> CHECK: multiple assignments should be avoided
> #12: FILE: benchmarks/gem_wsim.c:1507:
> + cs = ptr = gem_mmap__cpu(fd, w->bb_handle,
>
> CHECK: Unbalanced braces around else statement
> #15: FILE: benchmarks/gem_wsim.c:1510:
> + } else
>
> CHECK: multiple assignments should be avoided
> #16: FILE: benchmarks/gem_wsim.c:1511:
> + cs = ptr = gem_mmap__device_coherent(fd,
>
> total: 0 errors, 0 warnings, 3 checks, 17 lines checked
>
Thanks, my bad, I missed the options mentioned in CONTRIBUTE.md
>
>>
>>>
>>> With above fixed
>>> Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>>
>> Thanks
>>>
>>>> /* Store initial 64b timestamp: start */
>>>> *cs++ = MI_LOAD_REGISTER_IMM(1) | MI_CS_MMIO_DST;
>>>> --
>>>> 2.31.1
>>>>
More information about the igt-dev
mailing list