[i-g-t V2 1/5] lib/igt_kms: Add infra to handle crtc arbitrary attributes

[Kamil Konieczny]

Hi Bhanuprakash,
On 2024-09-24 at 19:56:34 +0530, Bhanuprakash Modem wrote:
> We may want to poke at various other crtc attributes
> via sysfs/debugfs. Add a mechamisn to handle crtc
> arbitrary attributes.
> 
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> ---
>  lib/igt_kms.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |   3 +
>  2 files changed, 153 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index b40470c02..c22cdb602 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -92,6 +92,7 @@
>  #define DISPLAY_TILE_BLOCK 0x12
>  
>  typedef bool (*igt_connector_attr_set)(int dir, const char *attr, const char *value);
> +typedef bool (*igt_crtc_attr_set)(int dir, const char *attr, const char *value);
>  
>  struct igt_connector_attr {
>  	uint32_t connector_type;
> @@ -102,7 +103,16 @@ struct igt_connector_attr {
>  	const char *attr, *value, *reset_value;
>  };
>  
> +struct igt_crtc_attr {
> +	int pipe;
> +	int idx;
> +	int dir;
> +	igt_crtc_attr_set set;
> +	const char *attr, *value, *reset_value;
> +};
> +
>  static struct igt_connector_attr connector_attrs[MAX_CONNECTORS];
> +static struct igt_crtc_attr crtc_attrs[IGT_MAX_PIPES];
>  
>  /**
>   * igt_kms_get_base_edid:
> @@ -1802,6 +1812,125 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
>  	igt_assert(ret != -1);
>  }
>  
> +static struct igt_crtc_attr *crtc_attr_find(int idx, enum pipe pipe,
> +					    igt_crtc_attr_set set,
> +					    const char *attr)
> +{
> +	igt_assert(pipe != PIPE_NONE || pipe >= IGT_MAX_PIPES);

Why not return NULL for PIPE_NONE?
Also imho better:

	igt_assert(pipe >= 0 && pipe < ARRAY_SIZE());

> +
> +	for (int i = 0; i < ARRAY_SIZE(crtc_attrs); i++) {
> +		struct igt_crtc_attr *c = &crtc_attrs[i];
> +
> +		if (c->idx == idx &&
> +		    c->pipe == pipe &&
> +		    c->set == set && !strcmp(c->attr, attr))
> +			return c;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct igt_crtc_attr *crtc_attr_find_free(void)
> +{
> +	for (int i = 0; i < ARRAY_SIZE(crtc_attrs); i++) {
> +		struct igt_crtc_attr *c = &crtc_attrs[i];
> +
> +		if (!c->attr)
> +			return c;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct igt_crtc_attr *crtc_attr_alloc(int idx, enum pipe pipe,
> +					     int dir, igt_crtc_attr_set set,
> +					     const char *attr, const char *reset_value)
> +{
> +	struct igt_crtc_attr *c = crtc_attr_find_free();

c could be NULL here, if find_free() failed.

Regards,
Kamil

> +
> +	c->idx = idx;
> +	c->pipe = pipe;
> +
> +	c->dir = dir;
> +	c->set = set;
> +	c->attr = attr;
> +	c->reset_value = reset_value;
> +
> +	return c;
> +}
> +
> +static void crtc_attr_free(struct igt_crtc_attr *c)
> +{
> +	memset(c, 0, sizeof(*c));
> +}
> +
> +static bool crtc_attr_set(int idx, enum pipe pipe,
> +			  int dir, igt_crtc_attr_set set,
> +			  const char *attr, const char *value,
> +			  const char *reset_value)
> +{
> +	struct igt_crtc_attr *c;
> +
> +	c = crtc_attr_find(idx, pipe, set, attr);
> +	if (!c)
> +		c = crtc_attr_alloc(idx, pipe, dir, set, attr, reset_value);
> +
> +	c->value = value;
> +
> +	if (!c->set(c->dir, c->attr, c->value)) {
> +		crtc_attr_free(c);
> +		return false;
> +	}
> +
> +	if (!strcmp(c->value, c->reset_value))
> +		crtc_attr_free(c);
> +
> +	return true;
> +}
> +
> +static bool crtc_attr_set_debugfs(int drm_fd, enum pipe pipe,
> +				  const char *attr, const char *value,
> +				  const char *reset_value)
> +{
> +	int idx, dir;
> +
> +	idx = igt_device_get_card_index(drm_fd);
> +	if (idx < 0 || idx > 63)
> +		return false;
> +
> +	dir = igt_debugfs_pipe_dir(drm_fd, pipe, O_DIRECTORY);
> +	if (dir < 0)
> +		return false;
> +
> +	if (!crtc_attr_set(idx, pipe, dir,
> +			   igt_sysfs_set, attr,
> +			   value, reset_value))
> +		return false;
> +
> +	igt_info("Crtc-%d/%s is now %s\n", pipe, attr, value);
> +
> +	return true;
> +}
> +
> +/**
> + * kmstest_set_connector_dpms:
> + *
> + * Dump all attr state on all crtcs.
> + */
> +void dump_crtc_attrs(void)
> +{
> +	igt_debug("Current crtc attrs:\n");
> +
> +	for (int i = 0; i < ARRAY_SIZE(crtc_attrs); i++) {
> +		struct igt_crtc_attr *c = &crtc_attrs[i];
> +
> +		if (!c->attr)
> +			continue;
> +
> +		igt_debug("\tcrtc-%d/%s: %s\n", c->pipe, c->attr, c->value);
> +	}
> +}
> +
>  /**
>   * sort_drm_modes_by_clk_dsc:
>   * @a: first element
> @@ -5459,6 +5588,27 @@ void igt_reset_connectors(void)
>  	}
>  }
>  
> +/**
> + * igt_reset_crtcs:
> + *
> + * Remove any forced state from the crtcs.
> + */
> +void igt_reset_crtcs(void)
> +{
> +	/*
> +	 * Reset the crtcs stored in crtc_attrs, avoiding any
> +	 * functions that are not safe to call in signal handlers
> +	 */
> +	for (int i = 0; i < ARRAY_SIZE(crtc_attrs); i++) {
> +		struct igt_crtc_attr *c = &crtc_attrs[i];
> +
> +		if (!c->attr)
> +			continue;
> +
> +		c->set(c->dir, c->attr, c->reset_value);
> +	}
> +}
> +
>  /**
>   * igt_watch_uevents:
>   *
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 25ba50916..97efbefa5 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1085,6 +1085,9 @@ void igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool force);
>  void igt_enable_connectors(int drm_fd);
>  void igt_reset_connectors(void);
>  
> +void igt_reset_crtcs(void);
> +void dump_crtc_attrs(void);
> +
>  uint32_t kmstest_get_vbl_flag(int crtc_offset);
>  
>  const struct edid *igt_kms_get_base_edid(void);
> -- 
> 2.43.0
>