[PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Sharma, Swati2]

Hi Kunal,

On 17-Jan-24 11:14 AM, Joshi, Kunal1 wrote:
> Hello Swati,
> 
> On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
>> Hi Kunal,
>>
>> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>>> Hello Swati,
>>>
>>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>>> Hi Kunal,
>>>>
>>>> On 15-Jan-24 4:28 PM, Kunal Joshi wrote:
>>>>> add support to choose big joiner mode with environment
>>>>> variable, use mode with highest clock if no mode with big joiner
>>>>> found.
>>>>>
>>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>>> v3: avoid returning from multiple places (Bhanu)
>>>>>      avoid frequent debugfs reads (Bhanu)
>>>>>
>>>>> Cc: Karthik B S <karthik.b.s at intel.com>
>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
>>>>> Signed-off-by: Kunal Joshi <kunal1.joshi at intel.com>
>>>>> ---
>>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>>> index cb6d57c2d..2c4210d4b 100644
>>>>> --- a/lib/igt_kms.c
>>>>> +++ b/lib/igt_kms.c
>>>>> @@ -1771,8 +1771,9 @@ void 
>>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>>> drmModeConnector *connector,
>>>>>                       drmModeModeInfo *mode)
>>>>>   {
>>>>> +    bool found;
>>>>>       char *env;
>>>>> -    int i;
>>>>> +    int i, max_dotclock;
>>>>>         if (!connector->count_modes) {
>>>>>           igt_warn("no modes for connector %d\n",
>>>>> @@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int 
>>>>> drm_fd, drmModeConnector *connector,
>>>>>       }
>>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>>       if (env) {
>>>>>           /*
>>>>> -         * Only (0 or 1) and (lowest or highest) are allowed.
>>>>> +         * Only (0 or 1 or 2) and (lowest or highest or joiner) 
>>>>> are allowed.
>>>>>            *
>>>>>            * 0/lowest: Choose connector mode with lowest possible 
>>>>> resolution.
>>>>>            * 1/highest: Choose connector mode with highest possible 
>>>>> resolution.
>>>>> +         * 2/joiner: Choose connector mode with bigjoiner support 
>>>>> or with
>>>>> +                 highest clock if can't support  big joiner
>>>>
>>>> Shouldn't this be only joiner mode?
>>>>
>>>>
>>> You mean can be named as joiner mode because same can be used for big 
>>> joiner / ultra joiner?
>>
>> I guess this should be bigjoiner mode only. For ultra there might be 
>> other restrictions which we need to look at.
>>
>>> or
>>> We are returning the highest clock mode if no big joiner mode found?
>>
>> Yes, this is my query here. Why are we returning highest clock mode if 
>> no big joiner mode found?
>>
> We can return either with highest clock or highest resolution or default 
> mode,
> Since we have the other two already covered used highest clock.

But why we need to return something if bigjoiner mode not found?
If its not found, then we won't get to know only bigjoiner mode not 
found and we are falling back to default mode.
@bhanu what you suggest here?

>>>>>            */
>>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>>> +                             max_dotclock) ||
>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>> +                             sort_drm_modes_by_res_dsc, mode,
>>>>> +                             max_dotclock) ||
>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>> +                             sort_drm_modes_by_clk_dsc, mode,
>>>>> +                             max_dotclock);
>>>>> +        else if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>>               igt_sort_connector_modes(connector, 
>>>>> sort_drm_modes_by_res_dsc);
>>>>>           else if (!strcmp(env, "lowest") || !strcmp(env, "0"))
>>>>>               igt_sort_connector_modes(connector, 
>>>>> sort_drm_modes_by_res_asc);
>>>>>           else
>>>>>               goto default_mode;
>>>>> -
>>>>> -        *mode = connector->modes[0];
>>>>> +        if (!found)
>>>>> +            *mode = connector->modes[0];
>>>>>           return true;
>>>>>       }