Re: [PATCH i-g-t 1/2] lib/igt_kms: add support to force big joiner

["Modem, Bhanuprakash" <bhanuprakash.modem@intel.com>]

Hi Kunal,

On 18-01-2024 09:48 pm, Kunal Joshi wrote:
> add helpers to enable/disable force joiner on a given connector
> and check status for a given connector
> 
> Cc: Swati Sharma <swati2.sharma@intel.com>
> 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 | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   lib/igt_kms.h |  2 ++
>   2 files changed, 56 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e4dea1a60..ad815d1b7 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -6217,6 +6217,60 @@ 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)

IMHO, we need to use this helper as a bool.
Return true if the force bigjoiner is enabled else false (even if there 
is no debugfs).

> +{
> +	char file_name[128] = {0};
> +	char buf[512];
> +	int ret;
> +	int debugfs_fd = igt_debugfs_dir(drmfd);

We can get the connector debugfs dir as igt_debugfs_connector_dir(), 
then we can avoid below sprintf().

> +
> +	sprintf(file_name, "%s/i915_bigjoiner_force_enable", connector_name);
> +	ret = igt_debugfs_simple_read(debugfs_fd, file_name, buf, sizeof(buf));
> +        if (ret < 0) {
> +                igt_info("Could not read %s: %s\n",
> +                         file_name, strerror(-ret));
> +                return -1;
> +        }

Please fix the indentation.

> +	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.
> + * This function allows the user to force 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.
> + */
> +int igt_force_bigjoiner_enable(int drmfd, char *connector_name, bool enable)
> +{
> +	int debugfs_fd = igt_debugfs_dir(drmfd);

Please read above comments.

- Bhanu

> +	int len = enable ? 1 : 0;
> +	int ret;
> +	char file_path[128] = {0};
> +
> +	sprintf(file_path, "%s/i915_bigjoiner_force_enable", connector_name);
> +	ret = igt_sysfs_write(debugfs_fd, file_path, enable ? "1" : "0", len);
> +	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 b3882808b..5f2fd5fbf 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1213,6 +1213,8 @@ bool igt_max_bpc_constraint(igt_display_t *display, enum pipe pipe,
>   int igt_get_max_dotclock(int fd);
>   bool igt_bigjoiner_possible(drmModeModeInfo *mode, 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);