Re: [PATCH i-g-t v3 2/6] test/kms_colorop_helper: only check if a given enum value exist

[Alex Hung <alex.hung@amd.com>]

On 8/11/26 08:23, Melissa Wen wrote:
> Don't trigger unnecessary colorop property changes to just probe that a
> prop enum exists and can be apply. Make can_use_colorop() read-only.
> 
> Assisted-by: Claude:claude-opus-4-7
> Tested-by: Alex Hung <alex.hung@amd.com>
> Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> 
> --
> v2:
> - remove extra space (Chaitanya)
> ---
>   lib/igt_kms.c              | 26 ++++++++++++++++++++++++++
>   lib/igt_kms.h              |  4 ++++
>   tests/kms_colorop_helper.c |  4 +++-
>   3 files changed, 33 insertions(+), 1 deletion(-)

The patch touches both lib and tests - should the subject uses 
"lib/tests/..."?

or the should should use "tests", not "test"

> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 1e3cea781..900f88d0f 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -4506,6 +4506,32 @@ igt_colorop_replace_prop_blob(igt_colorop_t *colorop, enum igt_atomic_colorop_pr
>   	igt_colorop_set_prop_changed(colorop, prop);
>   }
>   
> +/**
> + * igt_colorop_has_prop_enum_value:
> + * @colorop: Target colorop.
> + * @prop: Enum property to check.
> + * @val: Enum value (as string) to look up.
> + *
> + * Read-only counterpart to igt_colorop_try_prop_enum(): returns whether
> + * @colorop's @prop accepts the enum value @val, without setting it or
> + * marking @prop as changed in IGT's cache.
> + *
> + * Returns: true if @val is a valid value of the enum @prop on @colorop.
> + */
> +bool igt_colorop_has_prop_enum_value(igt_colorop_t *colorop,
> +				     enum igt_atomic_colorop_properties prop,
> +				     const char *val)
> +{
> +	igt_display_t *display = colorop->plane->crtc->display;
> +	uint64_t uval;
> +
> +	igt_assert(colorop->props[prop]);
> +
> +	return igt_mode_object_get_prop_enum_value(display->drm_fd,
> +						   colorop->props[prop],
> +						   val, &uval);
> +}
> +
>   /**
>    * igt_colorop_try_prop_enum:
>    * @colorop: Target colorop.
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 521a03c01..ace8c2b1f 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1023,6 +1023,10 @@ uint64_t igt_colorop_get_prop(igt_display_t *display, igt_colorop_t *colorop, en
>   	} while (0)
>   
>   
> +extern bool igt_colorop_has_prop_enum_value(igt_colorop_t *colorop,
> +					    enum igt_atomic_colorop_properties prop,
> +					    const char *val);
> +
>   extern bool igt_colorop_try_prop_enum(igt_colorop_t *colorop,
>   				      enum igt_atomic_colorop_properties prop,
>   				      const char *val);
> diff --git a/tests/kms_colorop_helper.c b/tests/kms_colorop_helper.c
> index aaee4e567..5b79fe789 100644
> --- a/tests/kms_colorop_helper.c
> +++ b/tests/kms_colorop_helper.c
> @@ -205,7 +205,9 @@ static bool can_use_colorop(igt_display_t *display, igt_colorop_t *colorop, kms_
>   	switch (desired->type) {
>   	case KMS_COLOROP_ENUMERATED_LUT1D:
>   		if (igt_colorop_get_prop(display, colorop, IGT_COLOROP_TYPE) == DRM_COLOROP_1D_CURVE &&
> -		    igt_colorop_try_prop_enum(colorop, IGT_COLOROP_CURVE_1D_TYPE, kms_colorop_lut1d_tf_names[desired->enumerated_lut1d_info.tf]))
> +		    igt_colorop_has_prop_enum_value(colorop,
> +						    IGT_COLOROP_CURVE_1D_TYPE,
> +						    kms_colorop_lut1d_tf_names[desired->enumerated_lut1d_info.tf]))
>   			return true;
>   		return false;
>   	case KMS_COLOROP_CTM_3X4: