Re: [PATCH i-g-t 4/7] lib/igt_kms: add support for forcing bigjoiner on particular connector

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

Hello,

On 4/19/2024 3:56 PM, Modem, Bhanuprakash wrote:
>
> On 19-04-2024 12:09 pm, Kunal Joshi wrote:
>> add kmstest_force_connector_bigjoiner function which
>> force bigjoiner on provided connector as well as resets on exit.
>>
>> v2: remove redundant function (Bhanu)
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>> ---
>>   lib/igt_kms.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_kms.h |  1 +
>>   2 files changed, 70 insertions(+)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 378c97f88..324b0527f 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -1607,6 +1607,32 @@ static bool connector_attr_set_sysfs(int drm_fd,
>>       return true;
>>   }
>>   +static bool connector_attr_set_debugfs(int drm_fd,
>
> How connector_attr_set_debugfs() is different than 
> connector_attr_set_sysfs()?
>
>> +                       drmModeConnector *connector,
>> +                       const char *attr, const char *value,
>> +                       const char *reset_value)
>> +{
>> +    char name[80];
>> +    int idx, dir;
>> +
>> +    snprintf(name, sizeof(name), "%s-%d",
>> + kmstest_connector_type_str(connector->connector_type),
>> +         connector->connector_type_id);
Above differs
>>
>> +
>> +    dir = igt_debugfs_connector_dir(drm_fd, name, O_DIRECTORY);
above differs
>> +    if (dir < 0)
>> +        return false;
>> +
>> +    if (!connector_attr_set(idx, connector, dir,
> --------------------------------^
> Passing uninitialized idx?
Hmm, not sure how could this work still,
Will assign proper value to idx in next revision.


Thanks and Regards
Kunal Joshi

>> +                igt_sysfs_set, attr,
>> +                value, reset_value))
>> +        return false;
>> +
>> +    igt_info("Connector %s/%s is now %s\n", name, attr, value);
>> +
>> +    return true;
>> +}
>> +
>>   static void dump_connector_attrs(void)
>>   {
>>       char name[80];
>> @@ -1690,6 +1716,49 @@ bool kmstest_force_connector(int drm_fd, 
>> drmModeConnector *connector,
>>       return true;
>>   }
>>   +static bool force_connector_bigjoiner(int drm_fd,
>> +                      drmModeConnector *connector,
>> +                      const char *value)
>> +{
>> +    return connector_attr_set_debugfs(drm_fd, connector,
>> +                      "i915_bigjoiner_force_enable",
>> +                      value, "0");
>> +}
>> +
>> +/**
>> + * kmstest_force_connector_bigjoiner:
>> + * @fd: drm file descriptor
>> + * @connector: connector
>> + *
>> + * Enable force bigjoiner state on the specified connector
>> + * and install exit handler for resetting
>> + *
>> + * Returns: True on success
>> + */
>> +bool kmstest_force_connector_bigjoiner(int drm_fd, drmModeConnector 
>> *connector)
>> +{
>> +    const char *value = "1";
>> +    drmModeConnector *temp;
>> +
>> +    if (!is_intel_device(drm_fd))
>> +        return false;
>> +
>> +    if (!force_connector_bigjoiner(drm_fd, connector, value))
>> +        return false;
>> +
>> +    dump_connector_attrs();
>> +    igt_install_exit_handler(reset_connectors_at_exit);
> ---------------------------------^
> This will reset all connectors, Instead of resetting all connectors 
> I'm expecting to reset a particular one. Maybe we can take this as a 
> separate activity.
>
> - Bhanu
>
>> +
>> +    /*
>> +     * To allow callers to always use GetConnectorCurrent we need to 
>> force a
>> +     * redetection here.
>> +     */
>> +    temp = drmModeGetConnector(drm_fd, connector->connector_id);
>> +    drmModeFreeConnector(temp);
>> +
>> +    return true;
>> +}
>> +
>>   /**
>>    * kmstest_force_edid:
>>    * @drm_fd: drm file descriptor
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 6d13e5851..0abf450c1 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -262,6 +262,7 @@ struct edid;
>>     bool kmstest_force_connector(int fd, drmModeConnector *connector,
>>                    enum kmstest_force_connector_state state);
>> +bool kmstest_force_connector_bigjoiner(int drm_fd, drmModeConnector 
>> *connector);
>>   void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
>>               const struct edid *edid);