Re: [igt-dev] [i-g-t V5 2/5] tests/i915/kms_fb_coherency: Add XE support

["Modem, Bhanuprakash" <bhanuprakash.modem@intel.com>]

Hi Swati,

On Mon-23-10-2023 02:11 pm, Sharma, Swati2 wrote:
> Hi Bhanu,
> 
> On 11-Sep-23 12:52 PM, Bhanuprakash Modem wrote:
>> Add XE driver support for kms tests. This patch will add a support
>> to call the corresponding apis based on the driver (i915/xe)
>>
>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> ---
>>   tests/intel/kms_fb_coherency.c | 43 ++++++++++++++++++----------------
>>   1 file changed, 23 insertions(+), 20 deletions(-)
>>
>> diff --git a/tests/intel/kms_fb_coherency.c 
>> b/tests/intel/kms_fb_coherency.c
>> index c772ddcdc..745ffcdd2 100644
>> --- a/tests/intel/kms_fb_coherency.c
>> +++ b/tests/intel/kms_fb_coherency.c
>> @@ -16,6 +16,7 @@
>>   #include <string.h>
>>   #include "igt.h"
>> +#include "xe/xe_ioctl.h"
>>   typedef struct {
>>       int drm_fd;
>> @@ -91,7 +92,7 @@ static struct igt_fb *prepare_fb(data_t *data)
>>               0, 0, fb->width, fb->height,
>>               0, 0, fb->width << 16, fb->height << 16);
>> -    if (!gem_has_lmem(data->drm_fd)) {
>> +    if (is_i915_device(data->drm_fd) && !gem_has_lmem(data->drm_fd)) {
>>           uint32_t caching;
>>           /* make sure caching mode has become UC/WT */
>> @@ -160,7 +161,10 @@ static void test_mmap_offset_wc(data_t *data)
>>       fb = prepare_fb(data);
>> -    buf = gem_mmap_offset__wc(data->drm_fd, fb->gem_handle, 0, 
>> fb->size, PROT_WRITE);
>> +    if (is_i915_device(data->drm_fd))
>> +        buf = gem_mmap_offset__wc(data->drm_fd, fb->gem_handle, 0, 
>> fb->size, PROT_WRITE);
>> +    else
>> +        buf = xe_bo_mmap_ext(data->drm_fd, fb->gem_handle, fb->size, 
>> PROT_WRITE);
>>       check_buf_crc(data, buf, fb);
>>   }
>> @@ -226,7 +230,7 @@ igt_main
>>       data_t data;
>>       igt_fixture {
>> -        data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
>> +        data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_XE);
>>           data.devid = intel_get_drm_devid(data.drm_fd);
>> @@ -251,39 +255,38 @@ igt_main
>>        * Test category: functionality test
>>        */
>>       igt_subtest_with_dynamic("memset-crc") {
>> -        if (gem_has_mappable_ggtt(data.drm_fd)) {
>> +        if (igt_draw_supports_method(data.drm_fd, IGT_DRAW_MMAP_GTT)) {
>>               igt_dynamic("mmap-gtt")
>>                   test_mmap_gtt(&data);
>>               cleanup_crtc(&data);
>>           }
>> -        if (gem_mmap_offset__has_wc(data.drm_fd)) {
>> +        if (igt_draw_supports_method(data.drm_fd, IGT_DRAW_MMAP_WC)) {
>>               igt_dynamic("mmap-offset-wc")
>>                   test_mmap_offset_wc(&data);
>>               cleanup_crtc(&data);
>>           }
>> -        if (gem_has_lmem(data.drm_fd)) {
>> -            igt_dynamic("mmap-offset-fixed")
>> -                test_mmap_offset_fixed(&data);
>> +        if (is_i915_device(data.drm_fd)) {
> 
> Instead of using if (is_i915_device()), can we use 
> igt_require_i915(data.drm_fd) instead if these subtests are executable 
> on i915 driver only?

No, we can't use igt_require_i915() here.

For XE device, IGT will always throw a SKIP after the execution of 
"mmap-offset-wc".

BTW, we can't use igt_fixture inside igt_subtest_*().

- Bhanu

> 
>> +            if (gem_has_lmem(data.drm_fd)) {
>> +                igt_dynamic("mmap-offset-fixed")
>> +                    test_mmap_offset_fixed(&data);
>> +            } else if (gem_has_mmap_offset(data.drm_fd)) {
>> +                igt_dynamic("mmap-offset-uc")
>> +                    test_mmap_offset_uc(&data);
>> +            }
>>               cleanup_crtc(&data);
>> -        } else if (gem_has_mmap_offset(data.drm_fd)) {
>> -            igt_dynamic("mmap-offset-uc")
>> -                test_mmap_offset_uc(&data);
>> +            if (gem_has_legacy_mmap(data.drm_fd) &&
>> +                gem_mmap__has_wc(data.drm_fd)) {
>> +                igt_dynamic("mmap-legacy-wc")
>> +                    test_legacy_mmap_wc(&data);
>> -            cleanup_crtc(&data);
>> -        }
>> -
>> -        if (gem_has_legacy_mmap(data.drm_fd) &&
>> -            gem_mmap__has_wc(data.drm_fd)) {
>> -            igt_dynamic("mmap-legacy-wc")
>> -                test_legacy_mmap_wc(&data);
>> -
>> -            cleanup_crtc(&data);
>> +                cleanup_crtc(&data);
>> +            }
>>           }
>>       }