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

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

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@intel.com>
>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@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.
>>>>            */
>>>> -        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;
>>>>       }