Re: [igt-dev] [PATCH i-g-t v4] lib/igt_kms: Fix memory corruption

[Vignesh Raman <vignesh.raman@collabora.com>]

Hi Bhanu,

On 10/11/23 11:36, Modem, Bhanuprakash wrote:
>> ---
>>   lib/igt_kms.c                   |  6 +++++-
>>   lib/igt_kms.h                   | 20 +++++++++++++++++++-
>>   tests/intel-ci/blacklist.txt    | 10 +++++++++-
>>   tests/intel-ci/xe.blocklist.txt |  6 +++---
>>   tests/intel/kms_ccs.c           | 24 ++++++++++++++++++++++++
>>   tests/kms_bw.c                  |  2 +-
> 
> Probably, you need to split this patch into 3.
> 
> 1- lib
> 2- tests
> 3- intel-ci

Sure, will split the changes into 3 commits.

>> @@ -2770,6 +2770,10 @@ void igt_display_require(igt_display_t 
>> *display, int drm_fd)
>>       }
>>   #endif
>> +    igt_assert_f(resources->count_crtcs <= IGT_MAX_PIPES,
> --------^
> Could be igt_require_f()?
> 
> IMHO, we need to skip the test instead of fail. Since it is a basic 
> requirement to fill display struct.

Okay, will skip the test. I was going through the failures mentioned in
https://patchwork.freedesktop.org/series/125599/

Are these known issues?

>> --- a/tests/intel-ci/blacklist.txt
>> +++ b/tests/intel-ci/blacklist.txt
>> @@ -117,12 +117,20 @@ igt@gem_ctx_isolation@.*-s4
>>   ###############################################
>>   igt@i915_suspend@shrink
>>   ###############################################
>> -# pipe-e, pipe-f, pipe-g, pipe-h no current HW support
>> +# pipe-e, pipe-f, pipe-g, pipe-h, pipe-i, pipe-j, pipe-k, pipe-l, 
>> pipe-m, pipe-n, pipe-o, pipe-p no current HW support
>>   ###############################################
>>   igt@.*@.*pipe-e($|-.*)
>>   igt@.*@.*pipe-f($|-.*)
>>   igt@.*@.*pipe-g($|-.*)
>>   igt@.*@.*pipe-h($|-.*)
>> +igt@.*@.*pipe-i($|-.*)
>> +igt@.*@.*pipe-j($|-.*)
>> +igt@.*@.*pipe-k($|-.*)
>> +igt@.*@.*pipe-l($|-.*)
>> +igt@.*@.*pipe-m($|-.*)
>> +igt@.*@.*pipe-n($|-.*)
>> +igt@.*@.*pipe-o($|-.*)
>> +igt@.*@.*pipe-p($|-.*)
> 
> Shall we simplify the regex as below?
> 
> igt@.*@.*pipe-[e-pE-P]($|-.*)

Yes, this looks good.

> 
>>   ###############################################
>>   # Temporary workarounds for CI-impacting bugs
>> diff --git a/tests/intel-ci/xe.blocklist.txt 
>> b/tests/intel-ci/xe.blocklist.txt
>> index 135ab18a3..bec6ee23b 100644
>> --- a/tests/intel-ci/xe.blocklist.txt
>> +++ b/tests/intel-ci/xe.blocklist.txt
>> @@ -44,11 +44,11 @@ igt@syncobj_.*
>>   igt@template
>>   igt@tools_test
>>   ##################################################################
>> -# KMS: Pipe E, F, G and H are not available on Intel hardware,
>> +# KMS: Pipe E, F, G, H, I, J, K, L, M, N, O and P are not available 
>> on Intel hardware,
>>   # hence can't use more than 4 displays.
>>   ##################################################################
>> -igt@.*@.*pipe-(e|f|g|h).*
>> -igt@kms_bw@.*-(5|6|7|8)-displays-.*
>> +igt@.*@.*pipe-(e|f|g|h|i|j|k|l|m|n|o|p).*
>> +igt@kms_bw@.*-(5|6|7|8|9|10|11|12|13|14|15|16)-displays-.*
> 
> igt@.*@.*pipe-[e-pE-P].*
> igt@kms_bw@.*-([5-9]|1[0-6])-displays-.*
> 
> I used to validate my regex at https://regex101.com/
> 
> You can get my R-b after addressing above comments.

Thank you.

Regards,
Vignesh