Re: [PATCH i-g-t 1/4] lib/igt_kms: move bigjoiner_mode_found to lib

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

Hello Ankit,

On 3/15/2024 9:40 AM, Nautiyal, Ankit K wrote:
>
> On 3/10/2024 7:57 PM, Kunal Joshi wrote:
>> move bigjoiner_mode_found to lib
>>
>> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Cc: Karthik B S <karthik.b.s@intel.com>
>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>> ---
>>   lib/igt_kms.c                | 25 +++++++++++++++++++++++++
>>   lib/igt_kms.h                |  2 ++
>>   tests/intel/kms_big_joiner.c | 14 +-------------
>>   3 files changed, 28 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index e18f6fe59..63c8045c7 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -6143,6 +6143,31 @@ bool igt_bigjoiner_possible(drmModeModeInfo 
>> *mode, int max_dotclock)
>>           mode->clock > max_dotclock);
>>   }
>>   +/**
>> + * bigjoiner_mode_found:
>> + * @connector: libdrm connector
>> + * @sort_method: comparator method
>> + * @mode: libdrm mode
>> + *
>
> This is incorrect.
>
>
>> + * Bigjoiner will come in to the picture when the
>> + * resolution > 5K or clock > max-dot-clock.
>> + *
>> + * Returns: True if big joiner found in connector modes
>> + */
>> +bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
>> +              int max_dotclock)
>> +{
>> +    bool found = false;
>> +
>> +    igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
>> +    found = igt_bigjoiner_possible(&connector->modes[0], max_dotclock);
>> +    if (!found) {
>> +        igt_sort_connector_modes(connector, sort_drm_modes_by_clk_dsc);
>> +        found = igt_bigjoiner_possible(&connector->modes[0], 
>> max_dotclock);
>> +    }
>> +    return found;
>> +}
>> +
>>   /**
>>    * igt_check_bigjoiner_support:
>>    * @display: a pointer to an #igt_display_t structure
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index b3882808b..bab8487d3 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -1212,6 +1212,8 @@ bool igt_max_bpc_constraint(igt_display_t 
>> *display, enum pipe pipe,
>>           igt_output_t *output, int bpc);
>>   int igt_get_max_dotclock(int fd);
>>   bool igt_bigjoiner_possible(drmModeModeInfo *mode, int max_dotclock);
>> +bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
>> +              int max_dotclock);
>>   bool igt_check_bigjoiner_support(igt_display_t *display);
>>   bool igt_parse_mode_string(const char *mode_string, drmModeModeInfo 
>> *mode);
>>   bool intel_pipe_output_combo_valid(igt_display_t *display);
>> diff --git a/tests/intel/kms_big_joiner.c b/tests/intel/kms_big_joiner.c
>> index aba2adfbe..28678b958 100644
>> --- a/tests/intel/kms_big_joiner.c
>> +++ b/tests/intel/kms_big_joiner.c
>> @@ -199,16 +199,6 @@ static void test_dual_display(data_t *data)
>>       igt_display_commit2(display, COMMIT_ATOMIC);
>>   }
>>   -static bool bigjoiner_mode_found(drmModeConnector *connector,
>> -                 int (*sort_method)(const void *, const void*),
>> -                 drmModeModeInfo *mode)
>> -{
>> -    igt_sort_connector_modes(connector, sort_method);
>> -    *mode = connector->modes[0];
>> -
>> -    return igt_bigjoiner_possible(mode, max_dotclock);
>> -}
>> -
>>   igt_main
>>   {
>>       data_t data;
>> @@ -235,9 +225,7 @@ igt_main
>>                * Bigjoiner will come in to the picture when the
>>                * resolution > 5K or clock > max-dot-clock.
>>                */
>> -            found = (bigjoiner_mode_found(connector, 
>> sort_drm_modes_by_res_dsc, &mode) ||
>> -                 bigjoiner_mode_found(connector, 
>> sort_drm_modes_by_clk_dsc, &mode)) ?
>> -                    true : false;
>> +            found = bigjoiner_mode_found(data.drm_fd, connector, 
>> max_dotclock);
>
>
> How does this work? Earlier mode was filled by the 
> bigjoiner_mode_found().
Currently it works as we will have the connector modes sorted.
But as you state for better readability will have a mode parameter.
>
> Now mode is not getting filled at all.
>
> Regards,
>
> Ankit
>
>>                 if (found) {
>>                   data.output[count].output_id = output->id;
Thanks and Regards
Kunal Joshi