[PATCH i-g-t 3/4] lib/igt_kms: add helper to enable/disable force joiner

[Nautiyal, Ankit K]

On 3/20/2024 6:23 PM, Nautiyal, Ankit K wrote:
>
> On 3/10/2024 7:57 PM, Kunal Joshi wrote:
>> add helpers to check whether force joiner debugfs exists
>> and to enable/disable force joiner for a specific connector.
>>
>> Cc: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> 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 | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_kms.h |  2 ++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 63c8045c7..9d0cbd329 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -6168,6 +6168,65 @@ bool bigjoiner_mode_found(int drm_fd, 
>> drmModeConnector *connector,
>>       return found;
>>   }
>>   +/**
>> + * Checks if the force big joiner is enabled for a specific connector.
>> + *
>> + * @drmfd The file descriptor of the DRM device.
>> + * @connector_name The name of the connector.
>
> I think we dont need documentation for this, as this is straight 
> forward and just static function which is called from one place.


Looking at it again, it seems we dont need this as separate function, 
just simply read and return the status in the caller.

Regards,

Ankit

>
>> + * Returns:
>> + *  true if status equals enable, false otherwise.
>> + */
>> +static bool igt_check_force_bigjoiner_status(int drmfd, char 
>> *connector_name, bool enable)
>> +{
>> +    char buf[512];
>> +    int debugfs_fd, ret;
>> +
>> +    igt_assert_f(connector_name, "Connector name cannot be NULL\n");
>> +    debugfs_fd = igt_debugfs_connector_dir(drmfd, connector_name, 
>> O_RDONLY);
>> +    igt_assert_f(debugfs_fd >= 0, "Could not open debugfs for 
>> connector %s\n", connector_name);
>> +    ret = igt_debugfs_simple_read(debugfs_fd, 
>> "i915_bigjoiner_force_enable", buf, sizeof(buf));
>> +    close(debugfs_fd);
>> +    igt_assert_f(ret > 0, "Could not read 
>> i915_bigjoiner_force_enable for connector %s\n", connector_name);
>> +    return enable ? strstr(buf, "Bigjoiner enable: 1") :
>> +                    strstr(buf, "Bigjoiner enable: 0");
>> +}
>> +
>> +bool has_force_joiner_debugfs(int drmfd, igt_output_t *output)
>
> Since this helper is expected to be used in other tests, it would be 
> good to have documentation for this.
>
> Also, imho, it would be better to use igt_* for helpers, though we 
> seem to be not following this strictly.
>
>
>> +{
>> +    char buf[512];
>> +    int debugfs_fd, ret;
>> +
>> +    igt_assert_f(output->name, "Connector name cannot be NULL\n");
>> +    debugfs_fd = igt_debugfs_connector_dir(drmfd, output->name, 
>> O_RDONLY);
>> +    igt_assert_f(debugfs_fd >= 0, "Could not open debugfs for 
>> connector %s\n", output->name);
>
> I think we should not assert here, this will fail for platforms that 
> do not support bigjoiner. Perhaps returning false will be sufficient.
>
>
> Regards,
>
> Ankit
>
>> +    ret = igt_debugfs_simple_read(debugfs_fd, 
>> "i915_bigjoiner_force_enable", buf, sizeof(buf));
>> +    close(debugfs_fd);
>> +    return ret >= 0;
>> +}
>> +
>> +/**
>> + * Forces the enable/disable state of big joiner for a specific 
>> connector.
>> + *
>> + * @drmfd The file descriptor of the DRM device.
>> + * @connector_name The name of the connector.
>> + * @enable The desired state of big joiner (true for enable, false 
>> for disable).
>> + * Returns:
>> + *  true on success, false otherwise.
>> + */
>> +bool igt_force_bigjoiner_enable(int drmfd, char *connector_name, 
>> bool enable)
>> +{
>> +    int debugfs_fd, ret;
>> +
>> +    igt_assert_f(connector_name, "Connector name cannot be NULL\n");
>> +    debugfs_fd = igt_debugfs_connector_dir(drmfd, connector_name, 
>> O_DIRECTORY);
>> +    igt_assert_f(debugfs_fd >= 0, "Could not open debugfs for 
>> connector %s\n", connector_name);
>> +    ret = igt_sysfs_write(debugfs_fd, "i915_bigjoiner_force_enable", 
>> enable ? "1" : "0", 1);
>> +    close(debugfs_fd);
>> +    igt_assert_f(ret > 0, "Could not write 
>> i915_bigjoiner_force_enable for connector %s\n", connector_name);
>> +
>> +    return igt_check_force_bigjoiner_status(drmfd, connector_name, 
>> enable);
>> +}
>> +
>>   /**
>>    * 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 bab8487d3..f13b7fd53 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -1214,6 +1214,8 @@ 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 has_force_joiner_debugfs(int drmfd, igt_output_t *output);
>> +bool igt_force_bigjoiner_enable(int drmfd, char *connector_name, 
>> bool enable);
>>   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);