Re: [PATCH v2 i-g-t 2/2] benchmarks/gem_wsim: Fix mmap for discrete graphics cards

["Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>]

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@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


> 
> With above fixed
> Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Thanks
> 
>>   
>>   	/* Store initial 64b timestamp: start */
>>   	*cs++ = MI_LOAD_REGISTER_IMM(1) | MI_CS_MMIO_DST;
>> -- 
>> 2.31.1
>>