Re: [igt-dev] [v4 i-g-t 01/14] lib/igt_kms: helper to override the mode on all connectors

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

Hi Bhanu,

I realized that the helper given is also called for tests which use 
legacy commit. Currently it tries igt_display_try_commit_atomic()

For legacy commit support, can we make use of igt_display_try_commit2( ) 
which takes commit style also as argument.

the function igt_display_try_commit_2() for legacy commit will do 
drmModesetcrtc for each pipe with the given mode individually and return 
false if any of them fail.


Another suggestion which I missed earlier: perhaps we can drop 'link' in 
the name igt_override_all_active_output_modes_to_fit_link_bw(), as its 
not only for MST but in general.


Regards,

Ankit


On 5/5/2021 4:34 PM, Nautiyal, Ankit K wrote:
> LGTM.
>
> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>
> On 5/4/2021 6:13 AM, Bhanuprakash Modem wrote:
>> This helper will iterate through all connectors those have a
>> pending_pipe != PIPE_NONE set by the test upto the point of
>> calling this helper. And find the combination by using
>> ATOMIC_TEST_ONLY then return to the test.
>>
>> This helper would override the mode on all connectors that will
>> be modeset by the next igt_display_commit() call in the test.
>>
>> V2:
>> * Remove MST specific logic (Daniel)
>> V3:
>> * Sort connector modes in descending order
>> V4:
>> * Fine tune the logic to reduce #of interations (Ankit)
>> * Update the documentation (Ankit)
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> ---
>>   lib/igt_kms.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_kms.h |  1 +
>>   2 files changed, 73 insertions(+)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 47b829b0c..d604c1b38 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -4048,6 +4048,78 @@ void igt_output_set_pipe(igt_output_t *output, 
>> enum pipe pipe)
>>       }
>>   }
>>   +#define for_each_connector_mode(output)        \
>> +    for (int i__ = 0;  i__ < output->config.connector->count_modes; 
>> i__++)
>> +
>> +static int sort_drm_modes(const void *a, const void *b)
>> +{
>> +    const drmModeModeInfo *mode1 = a, *mode2 = b;
>> +
>> +    return (mode1->clock < mode2->clock) - (mode2->clock < 
>> mode1->clock);
>> +}
>> +
>> +static
>> +bool __override_all_active_output_modes_to_fit_link_bw(igt_display_t 
>> *display,
>> +                            igt_output_t *outputs[IGT_MAX_PIPES],
>> +                            const int n_outputs,
>> +                            int base)
>> +{
>> +    igt_output_t *output = NULL;
>> +
>> +    if (base >= n_outputs)
>> +        return false;
>> +
>> +    output = outputs[base];
>> +
>> +    for_each_connector_mode(output) {
>> +        igt_output_override_mode(output, 
>> &output->config.connector->modes[i__]);
>> +
>> +        if 
>> (__override_all_active_output_modes_to_fit_link_bw(display, outputs, 
>> n_outputs, base + 1))
>> +            return true;
>> +
>> +        if (igt_display_try_commit_atomic(display,
>> +                DRM_MODE_ATOMIC_TEST_ONLY |
>> +                DRM_MODE_ATOMIC_ALLOW_MODESET,
>> +                NULL) == 0)
>> +            return true;
>> +    }
>> +
>> +    return false;
>> +}
>> +
>> +/**
>> + * igt_override_all_active_output_modes_to_fit_link_bw:
>> + * @display: a pointer to an #igt_display_t structure
>> + *
>> + * Override the mode on all active outputs (i.e. pending_pipe != 
>> PIPE_NONE)
>> + * on basis of bandwidth.
>> + *
>> + * Returns: true if a valid connector mode combo found, else false
>> + */
>> +bool 
>> igt_override_all_active_output_modes_to_fit_link_bw(igt_display_t 
>> *display)
>> +{
>> +    int i, n_outputs = 0;
>> +    igt_output_t *outputs[IGT_MAX_PIPES];
>> +
>> +    for (i = 0 ; i < display->n_outputs; i++) {
>> +        igt_output_t *output = &display->outputs[i];
>> +
>> +        if (output->pending_pipe == PIPE_NONE)
>> +            continue;
>> +
>> +        /* Sort the modes in descending order by clock freq. */
>> +        qsort(output->config.connector->modes,
>> +              output->config.connector->count_modes,
>> +              sizeof(drmModeModeInfo),
>> +              sort_drm_modes);
>> +
>> +        outputs[n_outputs++] = output;
>> +    }
>> +    igt_require_f(n_outputs, "No active outputs found.\n");
>> +
>> +    return 
>> __override_all_active_output_modes_to_fit_link_bw(display, outputs, 
>> n_outputs, 0);
>> +}
>> +
>>   /*
>>    * igt_pipe_refresh:
>>    * @display: a pointer to an #igt_display_t structure
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 85f0769c9..95463b1c5 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -915,5 +915,6 @@ void igt_require_pipe(igt_display_t *display,
>>     void igt_dump_connectors_fd(int drmfd);
>>   void igt_dump_crtcs_fd(int drmfd);
>> +bool 
>> igt_override_all_active_output_modes_to_fit_link_bw(igt_display_t 
>> *display);
>>     #endif /* __IGT_KMS_H__ */
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev