[PATCH i-g-t v5 1/3] lib/igt_kms: Add helper to get encoder index

[Kamil Konieczny]

Hi Jessica,
On 2025-01-28 at 19:29:47 -0800, Jessica Zhang wrote:
> From: Esha Bharadwaj <quic_ebharadw at quicinc.com>
> 
> Move the get_encoder_idx() helper within the kms_setmode test to
> lib/igt_kms

please correct Bhanu address to:
	Bhanuprakash Modem <bhanuprakash.modem at gmail.com>


> 
> Signed-off-by: Esha Bharadwaj <quic_ebharadw at quicinc.com>
> Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
> ---
>  lib/igt_kms.c       | 15 +++++++++++++++
>  lib/igt_kms.h       |  1 +
>  tests/kms_setmode.c | 14 ++------------
>  3 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 90f44b4d3a8607d1caedccf50691fae7aaf2d079..4b14c94e226a9a103bc3a3bc26099566a6eb274b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3324,6 +3324,21 @@ void igt_display_fini(igt_display_t *display)
>  	display->planes = NULL;
>  }
>  
> +/**
> + * kmstest_get_encoder_idx:
> + * @resources: libdrm resources pointer
> + * @encoder: libdrm encoder pointer
> + *
> + * Get encoder index from encoder_id
> + */
> +int kmstest_get_encoder_idx(drmModeRes *resources, drmModeEncoder *encoder)
> +{
> +	for (int i = 0; i < resources->count_encoders; i++)
> +		if (resources->encoders[i] == encoder->encoder_id)
> +			return i;

Add newline here.

> +	igt_assert(0);

imho it is better to print more info here, like:
	igt_assert_f(0, "Error: cannot find encoder id %d in range of 0...%d\n",
		     encoder->encoder_id, resources->count_encoders - 1);

This should improve debugging a little. With this

Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

Regards,
Kamil

> +}
> +
>  static void igt_display_refresh(igt_display_t *display)
>  {
>  	igt_output_t *output;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 8810123fbebeb68f5c549c434d6ca69a123cfb90..e4f7b6c62fd536aa4ea3eb3153d8e1935cf560fd 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -314,6 +314,7 @@ void kmstest_dumb_destroy(int fd, uint32_t handle);
>  void kmstest_wait_for_pageflip(int fd);
>  void kmstest_wait_for_pageflip_timeout(int fd, uint64_t timeout_us);
>  unsigned int kmstest_get_vblank(int fd, int pipe, unsigned int flags);
> +int kmstest_get_encoder_idx(drmModeRes *resources, drmModeEncoder *encoder);
>  
>  bool kms_has_vblank(int fd);
>  
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index d61cfeb9af22c12ae1f5aad5d06ccb9e01fc52d3..600524d98359fbf713be99de931e53b8ecf167b0 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -268,16 +268,6 @@ found:
>  	*mode_ret = *mode;
>  }
>  
> -static int get_encoder_idx(drmModeRes *resources, drmModeEncoder *encoder)
> -{
> -	int i;
> -
> -	for (i = 0; i < resources->count_encoders; i++)
> -		if (resources->encoders[i] == encoder->encoder_id)
> -			return i;
> -	igt_assert(0);
> -}
> -
>  static void get_crtc_config_str(struct crtc_config *crtc, char *buf,
>  				size_t buf_size)
>  {
> @@ -370,7 +360,7 @@ static void setup_crtcs(const struct test_config *tconf,
>  			config_valid &= !!(encoder->possible_crtcs &
>  					  (1 << crtc->crtc_idx));
>  
> -			encoder_mask |= 1 << get_encoder_idx(resources,
> +			encoder_mask |= 1 << kmstest_get_encoder_idx(resources,
>  							     encoder);
>  			config_valid &= !(encoder_mask &
>  					  ~encoder->possible_clones);
> @@ -396,7 +386,7 @@ static void setup_crtcs(const struct test_config *tconf,
>  			idx = cconf[i].crtc_idx;
>  
>  		encoder = drmModeGetEncoder(drm_fd, connector->encoders[idx]);
> -		encoder_usage_count[get_encoder_idx(resources, encoder)]++;
> +		encoder_usage_count[kmstest_get_encoder_idx(resources, encoder)]++;
>  		drmModeFreeEncoder(encoder);
>  	}
>  	for (i = 0; i < resources->count_encoders; i++)
> 
> -- 
> 2.34.1
>