Re: [igt-dev] [PATCH i-g-t 5/5] RFC tests/i915/kms_frontbuffer_tracking: xe only supports MMAP_WC, BLT, RENDER

["Joshi, Kunal1" <kunal1.joshi@intel.com>]

Hello Bhanu,

On 6/8/2023 5:16 PM, Modem, Bhanuprakash wrote:
>
>
> On Tue-06-06-2023 01:32 pm, Kunal Joshi wrote:
>> xe only supports MMAP_WC, BLT and RENDER methods,
>>
>> Open :- does draw method give guarantee for fb to be rendered
>> on return, if not how to assure, ex for RENDER we have intel_bb_sync
>>
>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>
>> +static bool supported_xe_draw_method(enum igt_draw_method method)
>> +{
>> +    return method == IGT_DRAW_MMAP_WC || method == IGT_DRAW_BLT
>> +             || method == IGT_DRAW_RENDER;
>> +}
>
> Please use igt_draw_supports_method() instead of writing your own 
> wrapper.
Sure bhanu, thanks for pointing it out, will float next revision with 
this change
>
>> +
>>   static drmModeModeInfo *get_connector_smallest_mode(igt_output_t 
>> *output)
>>   {
>>       drmModeConnector *c = output->config.connector;
>> @@ -1307,10 +1317,12 @@ static void init_crcs(enum pixel_format 
>> format, enum tiling_type tiling,
>>           for (r = 0; r < pattern->n_rects; r++)
>>               for (r_ = 0; r_ <= r; r_++)
>>                   draw_rect_igt_fb(pattern, &tmp_fbs[r],
>> -                         IGT_DRAW_PWRITE, r_);
>> + is_xe_device(drm.fd)?IGT_DRAW_RENDER:IGT_DRAW_PWRITE,
>> +                         r_);
>>       } else {
>>           for (r = 0; r < pattern->n_rects; r++)
>> -            draw_rect_igt_fb(pattern, &tmp_fbs[r], IGT_DRAW_PWRITE,
>> +            draw_rect_igt_fb(pattern, &tmp_fbs[r],
>> + is_xe_device(drm.fd)?IGT_DRAW_RENDER:IGT_DRAW_PWRITE,
>>                        r);
>>       }
>>   @@ -3180,6 +3192,9 @@ static void basic_subtest(const struct 
>> test_mode *t)
>>       fb1 = params->primary.fb;
>>         for (r = 0, method = 0; method < IGT_DRAW_METHOD_COUNT; 
>> method++) {
>> +        if (is_xe_device(drm.fd) && !supported_xe_draw_method(method))
>> +            continue;
>> +
>>           if (method == IGT_DRAW_MMAP_GTT &&
>>               !gem_has_mappable_ggtt(drm.fd))
>>               continue;
>> @@ -3413,29 +3428,30 @@ static const char *tiling_str(enum 
>> tiling_type tiling)
>>   }
>>     #define TEST_MODE_ITER_BEGIN(t) \
>> -    t.format = FORMAT_DEFAULT;                       \
>> -    t.flip = FLIP_PAGEFLIP;                           \
>> -    t.tiling = opt.tiling;;                           \
>> -    for (t.feature = 0; t.feature < FEATURE_COUNT; t.feature++) 
>> {       \
>> -    for (t.pipes = 0; t.pipes < PIPE_COUNT; t.pipes++) {           \
>> -    for (t.screen = 0; t.screen < SCREEN_COUNT; t.screen++) {       \
>> -    for (t.plane = 0; t.plane < PLANE_COUNT; t.plane++) {           \
>> -    for (t.fbs = 0; t.fbs < FBS_COUNT; t.fbs++) {               \
>> -    for (t.method = 0; t.method < IGT_DRAW_METHOD_COUNT; t.method++) 
>> { \
>> -        if (t.pipes == PIPE_SINGLE && t.screen == SCREEN_SCND)       \
>> -            continue;                       \
>> -        if (t.screen == SCREEN_OFFSCREEN && t.plane != PLANE_PRI)  \
>> -            continue;                       \
>> -        if (!opt.show_hidden && t.pipes == PIPE_DUAL &&           \
>> -            t.screen == SCREEN_OFFSCREEN)               \
>> -            continue;                       \
>> -        if (!opt.show_hidden && t.feature == FEATURE_NONE)       \
>> -            continue;                       \
>> -        if (!opt.show_hidden && t.fbs == FBS_SHARED &&           \
>> -            (t.plane == PLANE_CUR || t.plane == PLANE_SPR))       \
>> +       t.format = 
>> FORMAT_DEFAULT;                                               \
>> +       t.flip = FLIP_PAGEFLIP; \
>> +       t.tiling = 
>> opt.tiling;;                                                  \
>> +       for (t.feature = 0; t.feature < FEATURE_COUNT; t.feature++) 
>> {            \
>> +       for (t.pipes = 0; t.pipes < PIPE_COUNT; t.pipes++) 
>> {                     \
>> +       for (t.screen = 0; t.screen < SCREEN_COUNT; t.screen++) 
>> {                \
>> +       for (t.plane = 0; t.plane < PLANE_COUNT; t.plane++) 
>> {                    \
>> +       for (t.fbs = 0; t.fbs < FBS_COUNT; t.fbs++) 
>> {                            \
>> +       for (t.method = 0; t.method < IGT_DRAW_METHOD_COUNT; 
>> t.method++) {       \
>> +    if (is_xe_device(drm.fd) && 
>> !supported_xe_draw_method(t.method))        \
>> +        continue; 
>
> Changes in this macro are irrelevant except above 2 lines.

Bhanu for the other line i have placed the \ at the end of the line
So it looks nice, let me know if its not required

>
> - Bhanu
>                                                       \
>> +    if (t.pipes == PIPE_SINGLE && t.screen == 
>> SCREEN_SCND)                  \
>> + continue; \
>> +    if (t.screen == SCREEN_OFFSCREEN && t.plane != 
>> PLANE_PRI)               \
>> + continue; \
>> +    if (!opt.show_hidden && t.pipes == PIPE_DUAL 
>> &&                         \
>> +           t.screen == 
>> SCREEN_OFFSCREEN)                                \
>> + continue; \
>> +    if (!opt.show_hidden && t.feature == 
>> FEATURE_NONE)                      \
>> + continue; \
>> +    if (!opt.show_hidden && t.fbs == FBS_SHARED 
>> &&                          \
>> +           (t.plane == PLANE_CUR || t.plane == 
>> PLANE_SPR))              \
>>               continue;
>>   -
>>   #define TEST_MODE_ITER_END } } } } } }
>>     struct option long_options[] = {
Thanks and Regards
Kunal Joshi