Re: [PATCH i-g-t 2/4] lib/igt_kms: add helpers to enable/disable force joiner

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

Hello Bhanu,

Thanks for taking a look.

On 3/5/2024 5:14 PM, Modem, Bhanuprakash wrote:
> Hi Kunal,
>
> On 03-03-2024 10:52 pm, Kunal Joshi wrote:
>> add helped to enable/disable force joiner and for
>> checking current state enabled/disabled
>>
>> 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 | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_kms.h |  2 ++
>>   2 files changed, 50 insertions(+)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 89956b9d5..e6952f56a 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -6242,6 +6242,54 @@ bool igt_check_bigjoiner_support(igt_display_t 
>> *display)
>>       return true;
>>   }
>>   +/**
>> + * Checks if the force big joiner is enabled for a specific connector.
>> + *
>> + * @param drmfd The file descriptor of the DRM device.
>> + * @param connector_name The name of the connector.
>> + * Returns:
>> + *  1 if the force big joiner is enabled, 0 if disabled, and -1 on 
>> error.
>> + */
>> +int igt_check_force_bigjoiner_status(int drmfd, char *connector_name)
>> +{
>> +    char buf[512];
>> +    int debugfs_fd, ret;
>> +
>> +    debugfs_fd = igt_debugfs_connector_dir(drmfd, connector_name, 
>> O_RDONLY);
>
> We must guard "debugfs_fd" with igt_assert() or igt_require().
Will address in next revision.
>
>> +    ret = igt_debugfs_simple_read(debugfs_fd, 
>> "i915_bigjoiner_force_enable",
>> +                                  buf, sizeof(buf));
>> +    if (ret < 0) {
>> +        igt_info("Could not read debugfs %s\n",
>> +                 strerror(-ret));
>> +        return -1;
>> +    }
>> +    if (strstr(buf, "Bigjoiner enable: 1"))
>> +        return 1;
>> +    else if (strstr(buf, "Bigjoiner enable: 0"))
>> +        return 0;
>> +    else
>> +        return -1;
>> +}
>> +
>> +/**
>> + * Forces the enable/disable state of big joiner for a specific 
>> connector.
>> + *
>> + * @param drmfd The file descriptor of the DRM device.
>> + * @param connector_name The name of the connector.
>> + * @param enable The desired state of big joiner (true for enable, 
>> false for disable).
>> + * Returns:
>> + *  >=0 on success, otherwise an error code.
>
> IMHO, let's make return value as bool. Return True if write success 
> else False.
Yes we can do like that way too, will address in next revision
>
>> + */
>> +int igt_force_bigjoiner_enable(int drmfd, char *connector_name, bool 
>> enable)
>> +{
>> +    int debugfs_fd, ret;
>> +
>> +    debugfs_fd = igt_debugfs_connector_dir(drmfd, connector_name, 
>> O_DIRECTORY);
> ----------------------------------------------------------------------^
> This could be O_RDWR or O_WRONLY?

No O_RDWR and O_WRONLY are for files not for directories.

>
> - Bhanu
>
>> +    ret = igt_sysfs_write(debugfs_fd, "i915_bigjoiner_force_enable", 
>> enable ? "1" : "0", 1);
>> +    close(debugfs_fd);
>> +    return ret;
>> +}
>> +
>>   /**
>>    * igt_parse_mode_string:
>>    * @mode_string: modeline string
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index bab8487d3..b28022db4 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -1215,6 +1215,8 @@ 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);
>> +int igt_check_force_bigjoiner_status(int drmfd, char *connector_name);
>> +int igt_force_bigjoiner_enable(int drmfd, char *connector_name, bool 
>> enable);
>>   bool igt_parse_mode_string(const char *mode_string, drmModeModeInfo 
>> *mode);
>>   bool intel_pipe_output_combo_valid(igt_display_t *display);
>>   bool igt_check_output_is_dp_mst(igt_output_t *output);
Thanks and Regards
Kunal Joshi