Re: [i-g-t,3/3] lib/kms: Generalize forced_connectors[] to handle arbitrary attributes

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

[-- Attachment #1: Type: text/plain, Size: 7229 bytes --]

Hello Ville,

On 4/10/2024 8:32 PM, Ville Syrjala wrote:
> From: Ville Syrjälä<ville.syrjala@linux.intel.com>
>
> We may want to poke at various other connector attributes
> via sysfs/debugfs. Generalize the existing the force_connectors
> mechamisn to handle arbitrary attributes.
>
> Cc: Kunal Joshi<kunal1.joshi@intel.com>
> Signed-off-by: Ville Syrjälä<ville.syrjala@linux.intel.com>
> ---
>   lib/igt_kms.c | 131 ++++++++++++++++++++++++++++++++------------------
>   1 file changed, 85 insertions(+), 46 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 19bb4ac66ece..5b23fada94d9 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -91,14 +91,18 @@
>   #define MAX_EDID 2
>   #define DISPLAY_TILE_BLOCK 0x12
>   
> -struct igt_forced_connector {
> +typedef bool (*igt_connector_attr_set)(int dir, const char *attr, const char *value);
> +
> +struct igt_connector_attr {
>   	uint32_t connector_type;
>   	uint32_t connector_type_id;
>   	int idx;
>   	int dir;
> +	igt_connector_attr_set set;
> +	const char *attr, *value, *reset_value;
>   };
>   
> -static struct igt_forced_connector forced_connectors[MAX_CONNECTORS + 1];
> +static struct igt_connector_attr connector_attrs[MAX_CONNECTORS + 1];
>   
>   /**
>    * igt_kms_get_base_edid:
> @@ -1495,39 +1499,80 @@ int igt_connector_sysfs_open(int drm_fd,
>   	return conn_dir;
>   }
>   
> -static bool connector_is_forced(int idx, drmModeConnector *connector)
> +static struct igt_connector_attr *connector_attr_find(int idx, drmModeConnector *connector,
> +						      igt_connector_attr_set set,
> +						      const char *attr)
>   {
>   	igt_assert(connector->connector_type != 0);
>   
> -	for (int i = 0; forced_connectors[i].connector_type; i++) {
> -		struct igt_forced_connector *c = &forced_connectors[i];
> +	for (int i = 0; connector_attrs[i].connector_type; i++) {
> +		struct igt_connector_attr *c = &connector_attrs[i];
>   
>   		if (c->idx == idx &&
>   		    c->connector_type == connector->connector_type &&
> -		    c->connector_type_id == connector->connector_type_id)
> -			return true;
> +		    c->connector_type_id == connector->connector_type_id &&
> +		    c->set == set && !strcmp(c->attr, attr))
> +			return c;
>   	}
>   
> -	return false;
> +	return NULL;
>   }
>   
> -static struct igt_forced_connector *forced_connector_alloc(void)
> +static struct igt_connector_attr *connector_attr_alloc(int idx, drmModeConnector *connector,
> +						       int dir, igt_connector_attr_set set,
> +						       const char *attr, const char *reset_value)
>   {
> -	int i;
> +	struct igt_connector_attr *c = NULL;
>   
> -	for (i = 0; forced_connectors[i].connector_type; i++)
> -		;
> +	for (int i = 0; connector_attrs[i].connector_type; i++) {
> +		c = &connector_attrs[i];
> +		break;
> +	}
> +
> +	c->idx = idx;
> +	c->connector_type = connector->connector_type;
> +	c->connector_type_id = connector->connector_type_id;
> +
> +	c->dir = dir;
> +	c->set = set;
> +	c->attr = attr;
> +	c->reset_value = reset_value;

c can be null if we don't find any valid connector_attrs

Thanks and Regards Kunal Joshi

> +
> +	return c;
> +}
> +
> +static void connector_attr_free(struct igt_connector_attr *c)
> +{
> +	memset(c, 0, sizeof(*c));
> +}
> +
> +static bool connector_attr_set(int idx, drmModeConnector *connector,
> +			       int dir, igt_connector_attr_set set,
> +			       const char *attr, const char *value,
> +			       const char *reset_value)
> +{
> +	struct igt_connector_attr *c;
> +
> +	c = connector_attr_find(idx, connector, set, attr);
> +	if (!c)
> +		c = connector_attr_alloc(idx, connector, dir, set,
> +					 attr, reset_value);
> +
> +	if (!c->set(c->dir, c->attr, c->value)) {
> +		connector_attr_free(c);
> +		return false;
> +	}
>   
> -	igt_assert_lt(i, ARRAY_SIZE(forced_connectors));
> +	c->value = value;
>   
> -	return &forced_connectors[i];
> +	return true;
>   }
>   
> -static bool force_connector(int drm_fd,
> -			    drmModeConnector *connector,
> -			    const char *value)
> +static bool connector_attr_set_sysfs(int drm_fd,
> +				     drmModeConnector *connector,
> +				     const char *attr, const char *value,
> +				     const char *reset_value)
>   {
> -	struct igt_forced_connector *c;
>   	char name[80];
>   	int idx, dir;
>   
> @@ -1543,45 +1588,39 @@ static bool force_connector(int drm_fd,
>   	if (dir < 0)
>   		return false;
>   
> -	if (!igt_sysfs_set(dir, "status", value)) {
> -		close(dir);
> +	if (!connector_attr_set(idx, connector, dir,
> +				igt_sysfs_set, attr, value, reset_value))
>   		return false;
> -	}
>   
> -	igt_debug("Connector %s is now forced %s\n", name, value);
> -
> -	/* already tracked? */
> -	if (connector_is_forced(idx, connector)) {
> -		close(dir);
> -		return true;
> -	}
> -
> -	c = forced_connector_alloc();
> -
> -	c->idx = idx;
> -	c->connector_type = connector->connector_type;
> -	c->connector_type_id = connector->connector_type_id;
> -	c->dir = dir;
> +	igt_debug("Connector %s/%s is now %s\n", name, attr, value);
>   
>   	return true;
>   }
>   
> -static void dump_forced_connectors(void)
> +static void dump_connector_attrs(void)
>   {
>   	char name[80];
>   
> -	igt_debug("Current forced connectors:\n");
> +	igt_debug("Current connector attrs:\n");
>   
> -	for (int i = 0; forced_connectors[i].connector_type; i++) {
> -		struct igt_forced_connector *c = &forced_connectors[i];
> +	for (int i = 0; connector_attrs[i].connector_type; i++) {
> +		struct igt_connector_attr *c = &connector_attrs[i];
>   
>   		kmstest_connector_dirname(c->idx, c->connector_type,
>   					  c->connector_type_id,
>   					  name, sizeof(name));
> -		igt_debug("\t%s\n", name);
> +		igt_debug("\t%s/%s: %s\n", name, c->attr, c->value);
>   	}
>   }
>   
> +static bool force_connector(int drm_fd,
> +			    drmModeConnector *connector,
> +			    const char *value)
> +{
> +	return connector_attr_set_sysfs(drm_fd, connector,
> +					"status", value, "detect");
> +}
> +
>   /**
>    * kmstest_force_connector:
>    * @fd: drm file descriptor
> @@ -1626,7 +1665,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
>   	if (!force_connector(drm_fd, connector, value))
>   		return false;
>   
> -	dump_forced_connectors();
> +	dump_connector_attrs();
>   
>   	igt_install_exit_handler(reset_connectors_at_exit);
>   
> @@ -2619,7 +2658,7 @@ static void igt_handle_spurious_hpd(igt_display_t *display)
>   			 conn->connector_type_id);
>   	}
>   
> -	dump_forced_connectors();
> +	dump_connector_attrs();
>   }
>   
>   /**
> @@ -5303,12 +5342,12 @@ void igt_enable_connectors(int drm_fd)
>    */
>   void igt_reset_connectors(void)
>   {
> -	/* reset the connectors stored in forced_connectors, avoiding any
> +	/* reset the connectors stored in connector_attrs, avoiding any
>   	 * functions that are not safe to call in signal handlers */
> -	for (int i = 0; i < forced_connectors[i].connector_type; i++) {
> -		struct igt_forced_connector *c = &forced_connectors[i];
> +	for (int i = 0; i < connector_attrs[i].connector_type; i++) {
> +		struct igt_connector_attr *c = &connector_attrs[i];
>   
> -		igt_sysfs_set(c->dir, "status",  "detect");
> +		c->set(c->dir, c->attr, c->reset_value);
>   	}
>   }
>   

[-- Attachment #2: Type: text/html, Size: 7822 bytes --]