[PATCH i-g-t 2/3] tests/kms_bw: allow physically connected only

[Joshi, Kunal1]

Hello Aurabindo,

On 5/23/2024 6:26 PM, Aurabindo Pillai wrote:
> Hi Kunal,
>
> On 5/23/24 6:02 AM, Kunal Joshi wrote:
>> modify kms_bw test to only run on physically connected displays,
>> avoiding issues with forcing a 4K EDID on disconnected connectors.
>>
>> For instance, consider a system with 2 DP and 2 HDMI connectors
>> (DP-1, DP-2, HDMI-1, HDMI-2). Forcing EDID on DP is currently 
>> problematic,
>> and we only support forcing for HDMI. However, if there's only one
>
> Is this limitation intel specific?

Yes,

bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
                              enum kmstest_force_connector_state state)
{
         const char *value;
         drmModeConnector *temp;

         /*
          * Forcing DP connectors doesn't currently work, so
          * fail early to allow the test to skip if required.
          */
         if (is_intel_device(drm_fd) &&
             connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
                 return false;

>
>> TMDS encoder, we can only drive one HDMI display, not two.
>>
>> For now, the test will only run on physically connected displays.
>> Future improvements may consider the encoder count to handle such
>> scenarios more gracefully.
>>
>
> Making it run only on physical connected displays limits us from 
> catching any issues in backend programming related to bandwidth 
> calculation, so this change isnt helpful.
>
> How about adding a subtest that can be tested only on physically 
> connected ones ?
Thanks for the suggestion and the comments :),
Have done the same, https://patchwork.freedesktop.org/series/134822/


Regards
Kunal Joshi

>> Cc: Aurabindo Pillai <aurabindo.pillai at amd.com>
>> Signed-off-by: Kunal Joshi <kunal1.joshi at intel.com>
>> ---
>>   tests/kms_bw.c | 43 ++++++++++++++++++-------------------------
>>   1 file changed, 18 insertions(+), 25 deletions(-)
>>
>> diff --git a/tests/kms_bw.c b/tests/kms_bw.c
>> index b50f324af..4aabb02b7 100644
>> --- a/tests/kms_bw.c
>> +++ b/tests/kms_bw.c
>> @@ -61,6 +61,7 @@ typedef struct data {
>>           int h[IGT_MAX_PIPES];
>>           int fd;
>>       int num_pipes;
>> +    int num_outputs;
>>   } data_t;
>>     static drmModeModeInfo test_mode[] = {
>> @@ -105,8 +106,9 @@ static drmModeModeInfo test_mode[] = {
>>   static void test_init(data_t *data)
>>   {
>>       igt_display_t *display = &data->display;
>> -    int i, max_pipes = display->n_pipes;
>> +    int i;
>>       igt_output_t *output;
>> +    data->num_outputs = 0;
>>         for_each_pipe(display, i) {
>>           data->pipe_id[i] = i;
>> @@ -118,23 +120,22 @@ static void test_init(data_t *data)
>>                        IGT_PIPE_CRC_SOURCE_AUTO);
>>       }
>>   -    for (i = 0; i < display->n_outputs && i < max_pipes; i++) {
>> -        if (!data->pipe[i])
>> -            continue;
>> -
>> -        output = &display->outputs[i];
>> -
>> -        data->output[i] = output;
>> +    i = 0;
>>   +    for_each_output(display, output) {
>>           /* Only allow physically connected displays for the tests. */
>>           if (!igt_output_is_connected(output))
>>               continue;
>>   +        data->output[i] = output;
>> +        data->num_outputs++;
>> +
>>           igt_assert(kmstest_get_connector_default_mode(
>>               data->fd, output->config.connector, &data->mode[i]));
>>             data->w[i] = data->mode[i].hdisplay;
>>           data->h[i] = data->mode[i].vdisplay;
>> +        i++;
>>       }
>>     @@ -156,27 +157,12 @@ static void test_fini(data_t *data)
>>       igt_display_commit_atomic(display, 
>> DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
>>   }
>>   -/* Forces a mode for a connector. */
>> -static void force_output_mode(data_t *d, igt_output_t *output,
>> -                  const drmModeModeInfo *mode)
>> -{
>> -    /* This allows us to create a virtual sink. */
>> -    if (!igt_output_is_connected(output)) {
>> -        kmstest_force_edid(d->fd, output->config.connector,
>> -                   igt_kms_get_4k_edid());
>> -
>> -        kmstest_force_connector(d->fd, output->config.connector,
>> -                    FORCE_CONNECTOR_DIGITAL);
>> -    }
>> -
>> -    igt_output_override_mode(output, mode);
>> -}
>> -
>>   static void run_test_linear_tiling(data_t *data, int pipe, const 
>> drmModeModeInfo *mode) {
>>       igt_display_t *display = &data->display;
>>       igt_output_t *output;
>>       struct igt_fb buffer[IGT_MAX_PIPES];
>>       igt_crc_t zero, captured[IGT_MAX_PIPES];
>> +    int i = 0;
>>       int ret;
>>         igt_skip_on_f(pipe >= data->num_pipes,
>> @@ -184,6 +170,10 @@ static void run_test_linear_tiling(data_t *data, 
>> int pipe, const drmModeModeInfo
>>         test_init(data);
>>   +    igt_skip_on_f(pipe >= data->num_outputs,
>> +                  "%d connected outputs required but found %d\n",
>> +                  pipe+1, data->num_outputs+1);
>> +
>>       /* create buffers */
>>       for (i = 0; i <= pipe; i++) {
>>           output = data->output[i];
>> @@ -191,7 +181,7 @@ static void run_test_linear_tiling(data_t *data, 
>> int pipe, const drmModeModeInfo
>>               continue;
>>           }
>>   -        force_output_mode(data, output, mode);
>> +        igt_output_override_mode(output, mode);
>>             igt_create_color_fb(display->drm_fd, mode->hdisplay,
>>                       mode->vdisplay, DRM_FORMAT_XRGB8888,
>> @@ -199,6 +189,9 @@ static void run_test_linear_tiling(data_t *data, 
>> int pipe, const drmModeModeInfo
>>                       &buffer[i]);
>>             igt_output_set_pipe(output, i);
>> +        igt_info("Assigned output %s to pipe %s with mode %dx%d@%d\n",
>> +                 igt_output_name(output), kmstest_pipe_name(i),
>> +                 mode->hdisplay, mode->vdisplay, mode->vrefresh);
>>             igt_plane_set_fb(data->primary[i], &buffer[i]);
>>       }
>