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

["Sharma, Swati2" <swati2.sharma@intel.com>]

Hi Jessica,


On 29-01-2025 08:59 am, Jessica Zhang wrote:
> From: Esha Bharadwaj <quic_ebharadw@quicinc.com>
>
> Move the get_encoder_idx() helper within the kms_setmode test to
> lib/igt_kms
>
> Signed-off-by: Esha Bharadwaj <quic_ebharadw@quicinc.com>
> Signed-off-by: Jessica Zhang <quic_jesszhan@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;
> +	igt_assert(0);
> +}
> +
>   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);
Fix indentation here. Alignment should match open parenthesis.

With this fixed,

Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
>   			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++)
>