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

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

On 22-04-2024 11:04 am, 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)
> v3: initialize idx (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 | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   lib/igt_kms.h |  1 +
>   2 files changed, 74 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index ee8848079..c3fd7245f 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1607,6 +1607,36 @@ static bool connector_attr_set_sysfs(int drm_fd,
>   	return true;
>   }
>   
> +static bool connector_attr_set_debugfs(int drm_fd,
> +				       drmModeConnector *connector,
> +				       const char *attr, const char *value,
> +				       const char *reset_value)
> +{
> +	char name[80];
> +	int idx, dir;
> +
> +	idx = igt_device_get_card_index(drm_fd);
> +	if (idx < 0 || idx > 63)
> +		return false;
> +
> +	snprintf(name, sizeof(name), "%s-%d",
> +		 kmstest_connector_type_str(connector->connector_type),
> +		 connector->connector_type_id);
> +
> +	dir = igt_debugfs_connector_dir(drm_fd, name, O_DIRECTORY);
> +	if (dir < 0)
> +		return false;
> +
> +	if (!connector_attr_set(idx, connector, dir,
> +				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 +1720,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";
-------------------^
Do we really need this variable?

Anyway, overall the patch LGTM
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

> +	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);
> +
> +	/*
> +	 * 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);
>