[PATCH i-g-t] tests/kms_properties: drop immutability checks
Daniel Vetter
daniel.vetter at ffwll.ch
Thu Jul 18 10:00:57 UTC 2024
On Wed, Jul 17, 2024 at 05:39:47PM +0300, Dmitry Baryshkov wrote:
> Following the discussion on IRC, it is actually an error to require that
> properties that can not be chaged are marked as immutable.
>
> First of all, it creates inconsistent uAPI. Some drivers might have an
> immutable property, while others will have it mutable. Yes, there are
> known examples for such behaviour (e.g. zpos), but they are clearly
> documented in this way.
>
> Second, by the nature of the flag, the DRM_MODE_PROP_IMMUTABLE defines
> more of the 'direction' of the property (whether it is set by the kernel
> or it is expected to be set by the userspace) rather than simply states
> that there is no way for the userspace to change the property.
>
> Drop the single-value-is-immutable tests.
>
> Fixes: 29ae12bd764e ("tests/kms_properties: Validate properties harder")
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Link: https://oftc.irclog.whitequark.org/dri-devel/2024-07-16#33374622
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
Makes sense, but we might drop some test coverage for the properties where
we _do_ want the single value to be immutable. Like zpos.
So I think at least a cursory audit of existing properties would be good,
checking that the kerneldoc is accurate and then maybe limiting these
checks here to properties which are documented to have this behaviour?
I know that's a bunch more work, but I fear if we just drop this we only
move from one a bit confusing state of the uapi to another one, without
real improvements.
Ofc if all the compositor folks tell me that this doesn't matter anyway,
I'll shut up :-)
Cheers, Sima
> ---
> tests/kms_properties.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/tests/kms_properties.c b/tests/kms_properties.c
> index c0e5be1c90a8..c41b88bef76f 100644
> --- a/tests/kms_properties.c
> +++ b/tests/kms_properties.c
> @@ -421,11 +421,9 @@ static void validate_range_prop(const struct drm_mode_get_property *prop,
> {
> const uint64_t *values = from_user_pointer(prop->values_ptr);
> bool is_unsigned = prop->flags & DRM_MODE_PROP_RANGE;
> - bool immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
>
> igt_assert_eq(prop->count_values, 2);
> igt_assert_eq(prop->count_enum_blobs, 0);
> - igt_assert(values[0] != values[1] || immutable);
>
> if (is_unsigned) {
> igt_assert_lte_u64(values[0], values[1]);
> @@ -461,12 +459,10 @@ static void validate_enum_prop(const struct drm_mode_get_property *prop,
> uint64_t value)
> {
> const uint64_t *values = from_user_pointer(prop->values_ptr);
> - bool immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
> int i;
>
> igt_assert_lte(1, prop->count_values);
> igt_assert_eq(prop->count_enum_blobs, prop->count_values);
> - igt_assert(prop->count_values != 1 || immutable);
>
> for (i = 0; i < prop->count_values; i++) {
> if (value == values[i])
> @@ -481,12 +477,10 @@ static void validate_bitmask_prop(const struct drm_mode_get_property *prop,
> uint64_t value)
> {
> const uint64_t *values = from_user_pointer(prop->values_ptr);
> - bool immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
> uint64_t mask = 0;
>
> igt_assert_lte(1, prop->count_values);
> igt_assert_eq(prop->count_enum_blobs, prop->count_values);
> - igt_assert(prop->count_values != 1 || immutable);
>
> for (int i = 0; i < prop->count_values; i++) {
> igt_assert_lte_u64(values[i], 63);
> @@ -535,7 +529,6 @@ static void validate_object_prop(int fd,
> uint64_t value)
> {
> const uint64_t *values = from_user_pointer(prop->values_ptr);
> - bool immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
> struct drm_mode_crtc crtc;
> struct drm_mode_fb_cmd fb;
>
> @@ -543,7 +536,6 @@ static void validate_object_prop(int fd,
> igt_assert_eq(prop->count_enum_blobs, 0);
>
> igt_assert_lte_u64(value, 0xffffffff);
> - igt_assert(!immutable || value != 0);
>
> switch (values[0]) {
> case DRM_MODE_OBJECT_CRTC:
> --
> 2.43.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the igt-dev
mailing list