Re: [PATCH i-g-t v2 03/14] lib/igt_kms: Rework pipe properties to be more atomic, v7.

[Mika Kahola <mika.kahola@intel.com>]

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

On Thu, 2017-10-12 at 13:54 +0200, Maarten Lankhorst wrote:
> In the future I want to allow tests to commit more properties,
> but for this to work I have to fix all properties to work better
> with atomic commit. Instead of special casing each
> property make a bitmask for all property changed flags, and try to
> commit all properties.
> 
> This has been the most involved one, since legacy pipe commit still
> handles a lot of the properties differently from the rest.
> 
> Changes since v1:
> - Dump all changed properties on commit.
> - Fix bug in igt_pipe_refresh().
> Changes since v2:
> - Set pipe ACTIVE property changed flag on init.
> Changes since v3:
> - Add a missing igt_pipe_refresh() to kms_atomic_interruptible.
> Changes since v4:
> - Perform error handling when setting custom crtc properties.
> Changes since v5:
> - Only attempt to commit changes properties.
> Changes since v6:
> - Clear OUT_FENCE_PTR on succesful commit.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  lib/igt_kms.c                     | 231 ++++++++++++++++++++------
> ------------
>  lib/igt_kms.h                     |  77 +++++--------
>  tests/kms_atomic_interruptible.c  |   8 +-
>  tests/kms_atomic_transition.c     |   2 +-
>  tests/kms_crtc_background_color.c |   2 +-
>  5 files changed, 159 insertions(+), 161 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e77ae5d696da..02de39b8fc7f 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -259,8 +259,8 @@ igt_atomic_fill_connector_props(igt_display_t
> *display, igt_output_t *output,
>  }
>  
>  static void
> -igt_atomic_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
> -			int num_crtc_props, const char
> **crtc_prop_names)
> +igt_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
> +		    int num_crtc_props, const char
> **crtc_prop_names)
>  {
>  	drmModeObjectPropertiesPtr props;
>  	int i, j, fd;
> @@ -278,7 +278,7 @@ igt_atomic_fill_pipe_props(igt_display_t
> *display, igt_pipe_t *pipe,
>  			if (strcmp(prop->name, crtc_prop_names[j])
> != 0)
>  				continue;
>  
> -			pipe->atomic_props_crtc[j] = props-
> >props[i];
> +			pipe->props[j] = props->props[i];
>  			break;
>  		}
>  
> @@ -1620,13 +1620,6 @@ get_crtc_property(int drm_fd, uint32_t
> crtc_id, const char *name,
>  				    name, prop_id, value, prop);
>  }
>  
> -static void
> -igt_crtc_set_property(igt_pipe_t *pipe, uint32_t prop_id, uint64_t
> value)
> -{
> -	drmModeObjectSetProperty(pipe->display->drm_fd,
> -		pipe->crtc_id, DRM_MODE_OBJECT_CRTC, prop_id,
> value);
> -}
> -
>  /*
>   * Walk a plane's property list to determine its type.  If we don't
>   * find a type property, then the kernel doesn't support universal
> @@ -1690,7 +1683,6 @@ void igt_display_init(igt_display_t *display,
> int drm_fd)
>  		int p = 1;
>  		int j, type;
>  		uint8_t last_plane = 0, n_planes = 0;
> -		uint64_t prop_value;
>  
>  		pipe->crtc_id = resources->crtcs[i];
>  		pipe->display = display;
> @@ -1700,29 +1692,16 @@ void igt_display_init(igt_display_t *display,
> int drm_fd)
>  		pipe->planes = NULL;
>  		pipe->out_fence_fd = -1;
>  
> +		igt_fill_pipe_props(display, pipe,
> IGT_NUM_CRTC_PROPS, igt_crtc_prop_names);
> +
> +		/* Force modeset disable on first commit */
> +		igt_pipe_obj_set_prop_changed(pipe,
> IGT_CRTC_MODE_ID);
> +		igt_pipe_obj_set_prop_changed(pipe,
> IGT_CRTC_ACTIVE);
> +
>  		get_crtc_property(display->drm_fd, pipe->crtc_id,
> -				    "background_color",
> -				    &pipe->background_property,
> -				    &prop_value,
> +				    "background_color", NULL,
> +				    &pipe-
> >values[IGT_CRTC_BACKGROUND],
>  				    NULL);
> -		pipe->background = (uint32_t)prop_value;
> -		get_crtc_property(display->drm_fd, pipe->crtc_id,
> -				  "DEGAMMA_LUT",
> -				  &pipe->degamma_property,
> -				  NULL,
> -				  NULL);
> -		get_crtc_property(display->drm_fd, pipe->crtc_id,
> -				  "CTM",
> -				  &pipe->ctm_property,
> -				  NULL,
> -				  NULL);
> -		get_crtc_property(display->drm_fd, pipe->crtc_id,
> -				  "GAMMA_LUT",
> -				  &pipe->gamma_property,
> -				  NULL,
> -				  NULL);
> -
> -		igt_atomic_fill_pipe_props(display, pipe,
> IGT_NUM_CRTC_PROPS, igt_crtc_prop_names);
>  
>  		/* count number of valid planes */
>  		for (j = 0; j < plane_resources->count_planes; j++)
> {
> @@ -1813,8 +1792,6 @@ void igt_display_init(igt_display_t *display,
> int drm_fd)
>  			igt_assert_eq(p, n_planes);
>  
>  		pipe->n_planes = n_planes;
> -
> -		pipe->mode_changed = true;
>  	}
>  
>  	/*
> @@ -2291,7 +2268,7 @@ static int
> igt_primary_plane_commit_legacy(igt_plane_t *primary,
>  
>  	if (!igt_plane_is_prop_changed(primary, IGT_PLANE_FB_ID) &&
>  	    !(primary->changed & IGT_PLANE_COORD_CHANGED_MASK) &&
> -	    !primary->pipe->mode_changed)
> +	    !igt_pipe_obj_is_prop_changed(primary->pipe,
> IGT_CRTC_MODE_ID))
>  		return 0;
>  
>  	crtc_id = pipe->crtc_id;
> @@ -2360,6 +2337,16 @@ static int igt_plane_commit(igt_plane_t
> *plane,
>  	}
>  }
>  
> +static bool is_atomic_prop(enum igt_atomic_crtc_properties prop)
> +{
> +       if (prop == IGT_CRTC_MODE_ID ||
> +	   prop == IGT_CRTC_ACTIVE ||
> +	   prop == IGT_CRTC_OUT_FENCE_PTR)
> +		return true;
> +
> +	return false;
> +}
> +
>  /*
>   * Commit all plane changes to an output.  Note that if @s is
> COMMIT_LEGACY,
>   * enabling/disabling the primary plane will also enable/disable the
> CRTC.
> @@ -2377,19 +2364,17 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>  	int i;
>  	int ret;
>  
> -	if (pipe->background_changed) {
> -		igt_crtc_set_property(pipe, pipe-
> >background_property,
> -			pipe->background);
> -	}
> +	for (i = 0; i < IGT_NUM_CRTC_PROPS; i++)
> +		if (igt_pipe_obj_is_prop_changed(pipe, i) &&
> +		    !is_atomic_prop(i)) {
> +			igt_assert(pipe->props[i]);
>  
> -	if (pipe->color_mgmt_changed) {
> -		igt_crtc_set_property(pipe, pipe->degamma_property,
> -				      pipe->degamma_blob);
> -		igt_crtc_set_property(pipe, pipe->ctm_property,
> -				      pipe->ctm_blob);
> -		igt_crtc_set_property(pipe, pipe->gamma_property,
> -				      pipe->gamma_blob);
> -	}
> +			ret = drmModeObjectSetProperty(pipe-
> >display->drm_fd,
> +				pipe->crtc_id, DRM_MODE_OBJECT_CRTC,
> +				pipe->props[i], pipe->values[i]);
> +
> +			CHECK_RETURN(ret, fail_on_error);
> +		}
>  
>  	for (i = 0; i < pipe->n_planes; i++) {
>  		igt_plane_t *plane = &pipe->planes[i];
> @@ -2402,9 +2387,10 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>  }
>  
>  static void
> -igt_pipe_replace_blob(igt_pipe_t *pipe, uint64_t *blob, void *ptr,
> size_t length)
> +igt_pipe_replace_blob(igt_pipe_t *pipe, enum
> igt_atomic_crtc_properties prop, void *ptr, size_t length)
>  {
>  	igt_display_t *display = pipe->display;
> +	uint64_t *blob = &pipe->values[prop];
>  	uint32_t blob_id = 0;
>  
>  	if (*blob != 0)
> @@ -2416,6 +2402,7 @@ igt_pipe_replace_blob(igt_pipe_t *pipe,
> uint64_t *blob, void *ptr, size_t length
>  						     ptr, length,
> &blob_id) == 0);
>  
>  	*blob = blob_id;
> +	igt_pipe_obj_set_prop_changed(pipe, prop);
>  }
>  
>  /*
> @@ -2423,51 +2410,23 @@ igt_pipe_replace_blob(igt_pipe_t *pipe,
> uint64_t *blob, void *ptr, size_t length
>   */
>  static void igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj,
> drmModeAtomicReq *req)
>  {
> -	if (pipe_obj->background_changed)
> -		igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_BACKGROUND, pipe_obj->background);
> -
> -	if (pipe_obj->color_mgmt_changed) {
> -		igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_DEGAMMA_LUT, pipe_obj->degamma_blob);
> -		igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_CTM, pipe_obj->ctm_blob);
> -		igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_GAMMA_LUT, pipe_obj->gamma_blob);
> -	}
> -
> -	if (pipe_obj->mode_changed) {
> -		igt_output_t *output =
> igt_pipe_get_output(pipe_obj);
> -
> -		if (!output) {
> -			igt_pipe_replace_blob(pipe_obj, &pipe_obj-
> >mode_blob, NULL, 0);
> -
> -			LOG(pipe_obj->display, "%s: Setting NULL
> mode\n",
> -			    kmstest_pipe_name(pipe_obj->pipe));
> -		} else {
> -			drmModeModeInfo *mode =
> igt_output_get_mode(output);
> +	int i;
>  
> -			igt_pipe_replace_blob(pipe_obj, &pipe_obj-
> >mode_blob, mode, sizeof(*mode));
> +	for (i = 0; i < IGT_NUM_CRTC_PROPS; i++) {
> +		if (!igt_pipe_obj_is_prop_changed(pipe_obj, i))
> +			continue;
>  
> -			LOG(pipe_obj->display, "%s: Setting mode %s
> from %s\n",
> -			    kmstest_pipe_name(pipe_obj->pipe),
> -			    mode->name, igt_output_name(output));
> -		}
> +		igt_debug("Pipe %s: Setting property \"%s\" to
> 0x%"PRIx64"/%"PRIi64"\n",
> +			kmstest_pipe_name(pipe_obj->pipe),
> igt_crtc_prop_names[i],
> +			pipe_obj->values[i], pipe_obj->values[i]);
>  
> -		igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_MODE_ID, pipe_obj->mode_blob);
> -		igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_ACTIVE, !!output);
> +		igt_assert_lt(0, drmModeAtomicAddProperty(req,
> pipe_obj->crtc_id, pipe_obj->props[i], pipe_obj->values[i]));
>  	}
>  
>  	if (pipe_obj->out_fence_fd != -1) {
>  		close(pipe_obj->out_fence_fd);
>  		pipe_obj->out_fence_fd = -1;
>  	}
> -
> -	if (pipe_obj->out_fence_requested)
> -	{
> -		igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_OUT_FENCE_PTR,
> -		    (uint64_t)(uintptr_t) &pipe_obj->out_fence_fd);
> -	}
> -
> -	/*
> -	 *	TODO: Add all crtc level properties here
> -	 */
>  }
>  
>  /*
> @@ -2558,15 +2517,22 @@ display_commit_changed(igt_display_t
> *display, enum igt_commit_style s)
>  		igt_pipe_t *pipe_obj = &display->pipes[pipe];
>  		igt_plane_t *plane;
>  
> -		pipe_obj->color_mgmt_changed = false;
> -		pipe_obj->background_changed = false;
> +		if (s == COMMIT_ATOMIC) {
> +			if (igt_pipe_obj_is_prop_changed(pipe_obj,
> IGT_CRTC_OUT_FENCE_PTR))
> +				igt_assert(pipe_obj->out_fence_fd >=
> 0);
>  
> -		if (s != COMMIT_UNIVERSAL)
> -			pipe_obj->mode_changed = false;
> -
> -		if (s == COMMIT_ATOMIC && pipe_obj-
> >out_fence_requested) {
> -			pipe_obj->out_fence_requested = false;
> -			igt_assert(pipe_obj->out_fence_fd >= 0);
> +			pipe_obj->values[IGT_CRTC_OUT_FENCE_PTR] =
> 0;
> +			pipe_obj->changed = 0;
> +		} else {
> +			igt_pipe_obj_clear_prop_changed(pipe_obj,
> IGT_CRTC_BACKGROUND);
> +			igt_pipe_obj_clear_prop_changed(pipe_obj,
> IGT_CRTC_CTM);
> +			igt_pipe_obj_clear_prop_changed(pipe_obj,
> IGT_CRTC_DEGAMMA_LUT);
> +			igt_pipe_obj_clear_prop_changed(pipe_obj,
> IGT_CRTC_GAMMA_LUT);
> +
> +			if (s != COMMIT_UNIVERSAL) {
> +				igt_pipe_obj_clear_prop_changed(pipe
> _obj, IGT_CRTC_MODE_ID);
> +				igt_pipe_obj_clear_prop_changed(pipe
> _obj, IGT_CRTC_ACTIVE);
> +			}
>  		}
>  
>  		for_each_plane_on_pipe(display, pipe, plane) {
> @@ -2820,33 +2786,83 @@ void igt_output_override_mode(igt_output_t
> *output, drmModeModeInfo *mode)
>  
>  	output->use_override_mode = !!mode;
>  
> -	if (pipe)
> -		pipe->mode_changed = true;
> +	if (pipe) {
> +		if (output->display->is_atomic)
> +			igt_pipe_replace_blob(pipe,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(*mode));
> +		else
> +			igt_pipe_obj_set_prop_changed(pipe,
> IGT_CRTC_MODE_ID);
> +	}
>  }
>  
>  void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
>  {
>  	igt_display_t *display = output->display;
> -	igt_pipe_t *old_pipe;
> +	igt_pipe_t *old_pipe = NULL, *pipe_obj = NULL;;
>  
>  	igt_assert(output->name);
>  
> -	if (output->pending_pipe != PIPE_NONE) {
> +	if (output->pending_pipe != PIPE_NONE)
>  		old_pipe = igt_output_get_driving_pipe(output);
>  
> -		old_pipe->mode_changed = true;
> -	}
> +	if (pipe != PIPE_NONE)
> +		pipe_obj = &display->pipes[pipe];
>  
>  	LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
>  	    kmstest_pipe_name(pipe));
>  	output->pending_pipe = pipe;
>  
> -	if (pipe != PIPE_NONE)
> -		display->pipes[pipe].mode_changed = true;
> +	if (old_pipe) {
> +		igt_output_t *old_output;
> +
> +		old_output = igt_pipe_get_output(old_pipe);
> +		if (!old_output) {
> +			if (display->is_atomic)
> +				igt_pipe_replace_blob(old_pipe,
> IGT_CRTC_MODE_ID, NULL, 0);
> +			else
> +				igt_pipe_obj_set_prop_changed(old_pi
> pe, IGT_CRTC_MODE_ID);
> +
> +			igt_pipe_obj_set_prop_value(old_pipe,
> IGT_CRTC_ACTIVE, 0);
> +		}
> +	}
>  
>  	igt_output_set_prop_value(output, IGT_CONNECTOR_CRTC_ID,
> pipe == PIPE_NONE ? 0 : display->pipes[pipe].crtc_id);
>  
>  	igt_output_refresh(output);
> +
> +	if (pipe_obj) {
> +		if (display->is_atomic)
> +			igt_pipe_replace_blob(pipe_obj,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output),
> sizeof(drmModeModeInfo));
> +		else
> +			igt_pipe_obj_set_prop_changed(pipe_obj,
> IGT_CRTC_MODE_ID);
> +
> +		igt_pipe_obj_set_prop_value(pipe_obj,
> IGT_CRTC_ACTIVE, 1);
> +	}
> +}
> +
> +/*
> + * igt_pipe_refresh:
> + * @display: a pointer to an #igt_display_t structure
> + * @pipe: Pipe to refresh
> + * @force: Should be set to true if mode_blob is no longer
> considered
> + * to be valid, for example after doing an atomic commit during fork
> or closing display fd.
> + *
> + * Requests the pipe to be part of the state on next update.
> + * This is useful when state may have been out of sync after
> + * a fork, or we just want to be sure the pipe is included
> + * in the next commit.
> + */
> +void igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool
> force)
> +{
> +	igt_pipe_t *pipe_obj = &display->pipes[pipe];
> +
> +	if (force && display->is_atomic) {
> +		igt_output_t *output =
> igt_pipe_get_output(pipe_obj);
> +
> +		pipe_obj->values[IGT_CRTC_MODE_ID] = 0;
> +		if (output)
> +			igt_pipe_replace_blob(pipe_obj,
> IGT_CRTC_MODE_ID, igt_output_get_mode(output),
> sizeof(drmModeModeInfo));
> +	} else
> +		igt_pipe_obj_set_prop_changed(pipe_obj,
> IGT_CRTC_MODE_ID);
>  }
>  
>  void igt_output_set_scaling_mode(igt_output_t *output, uint64_t
> scaling_mode)
> @@ -3052,28 +3068,25 @@ void igt_plane_set_rotation(igt_plane_t
> *plane, igt_rotation_t rotation)
>   */
>  void igt_pipe_request_out_fence(igt_pipe_t *pipe)
>  {
> -	pipe->out_fence_requested = true;
> +	igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR,
> (ptrdiff_t)&pipe->out_fence_fd);
>  }
>  
>  void
>  igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>  {
> -	igt_pipe_replace_blob(pipe, &pipe->degamma_blob, ptr,
> length);
> -	pipe->color_mgmt_changed = 1;
> +	igt_pipe_replace_blob(pipe, IGT_CRTC_DEGAMMA_LUT, ptr,
> length);
>  }
>  
>  void
>  igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length)
>  {
> -	igt_pipe_replace_blob(pipe, &pipe->ctm_blob, ptr, length);
> -	pipe->color_mgmt_changed = 1;
> +	igt_pipe_replace_blob(pipe, IGT_CRTC_CTM, ptr, length);
>  }
>  
>  void
>  igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>  {
> -	igt_pipe_replace_blob(pipe, &pipe->gamma_blob, ptr, length);
> -	pipe->color_mgmt_changed = 1;
> +	igt_pipe_replace_blob(pipe, IGT_CRTC_GAMMA_LUT, ptr,
> length);
>  }
>  
>  /**
> @@ -3094,9 +3107,7 @@ void igt_crtc_set_background(igt_pipe_t *pipe,
> uint64_t background)
>  	    kmstest_pipe_name(pipe->pipe),
>  	    pipe->pipe, background);
>  
> -	pipe->background = background;
> -
> -	pipe->background_changed = true;
> +	igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_BACKGROUND,
> background);
>  }
>  
>  void igt_wait_for_vblank_count(int drm_fd, enum pipe pipe, int
> count)
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index f87f8be31421..b53127ffef5f 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -313,27 +313,13 @@ struct igt_pipe {
>  	int plane_primary;
>  	igt_plane_t *planes;
>  
> -	uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
> -
> -	uint64_t background; /* Background color MSB BGR 16bpc LSB
> */
> -	uint32_t background_changed : 1;
> -	uint32_t background_property;
> -
> -	uint64_t degamma_blob;
> -	uint32_t degamma_property;
> -	uint64_t ctm_blob;
> -	uint32_t ctm_property;
> -	uint64_t gamma_blob;
> -	uint32_t gamma_property;
> -	uint32_t color_mgmt_changed : 1;
> +	uint64_t changed;
> +	uint32_t props[IGT_NUM_CRTC_PROPS];
> +	uint64_t values[IGT_NUM_CRTC_PROPS];
>  
>  	uint32_t crtc_id;
>  
> -	uint64_t mode_blob;
> -	bool mode_changed;
> -
>  	int32_t out_fence_fd;
> -	bool out_fence_requested;
>  };
>  
>  typedef struct {
> @@ -527,17 +513,6 @@ static inline bool
> igt_output_is_connected(igt_output_t *output)
>  		igt_plane_set_prop_changed(plane, prop); \
>  	} while (0)
>  
> -/**
> - * igt_atomic_populate_crtc_req:
> - * @req: A pointer to drmModeAtomicReq
> - * @pipe: A pointer igt_pipe_t
> - * @prop: one of igt_atomic_crtc_properties
> - * @value: the value to add
> - */
> -#define igt_atomic_populate_crtc_req(req, pipe, prop, value) \
> -	igt_assert_lt(0, drmModeAtomicAddProperty(req, pipe-
> >crtc_id,\
> -						  pipe-
> >atomic_props_crtc[prop], value))
> -
>  #define igt_output_is_prop_changed(output, prop) \
>  	(!!((output)->changed & (1 << (prop))))
>  #define igt_output_set_prop_changed(output, prop) \
> @@ -552,26 +527,34 @@ static inline bool
> igt_output_is_connected(igt_output_t *output)
>  		igt_output_set_prop_changed(output, prop); \
>  	} while (0)
>  
> -/*
> - * igt_pipe_refresh:
> - * @display: a pointer to an #igt_display_t structure
> - * @pipe: Pipe to refresh
> - * @force: Should be set to true if mode_blob is no longer
> considered
> - * to be valid, for example after doing an atomic commit during fork
> or closing display fd.
> - *
> - * Requests the pipe to be part of the state on next update.
> - * This is useful when state may have been out of sync after
> - * a fork, or we just want to be sure the pipe is included
> - * in the next commit.
> - */
> -static inline void
> -igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool force)
> -{
> -	if (force)
> -		display->pipes[pipe].mode_blob = 0;
> +#define igt_pipe_obj_is_prop_changed(pipe_obj, prop) \
> +	(!!((pipe_obj)->changed & (1 << (prop))))
>  
> -	display->pipes[pipe].mode_changed = true;
> -}
> +#define igt_pipe_is_prop_changed(display, pipe, prop) \
> +	igt_pipe_obj_is_prop_changed(&(display)->pipes[(pipe)],
> prop)
> +
> +#define igt_pipe_obj_set_prop_changed(pipe_obj, prop) \
> +	(pipe_obj)->changed |= 1 << (prop)
> +
> +#define igt_pipe_set_prop_changed(display, pipe, prop) \
> +	igt_pipe_obj_set_prop_changed(&(display)->pipes[(pipe)],
> prop)
> +
> +#define igt_pipe_obj_clear_prop_changed(pipe_obj, prop) \
> +	(pipe_obj)->changed &= ~(1 << (prop))
> +
> +#define igt_pipe_clear_prop_changed(display, pipe, prop) \
> +	igt_pipe_obj_clear_prop_changed(&(display)->pipes[(pipe)],
> prop)
> +
> +#define igt_pipe_obj_set_prop_value(pipe_obj, prop, value) \
> +	do { \
> +		(pipe_obj)->values[prop] = (value); \
> +		igt_pipe_obj_set_prop_changed(pipe_obj, prop); \
> +	} while (0)
> +
> +#define igt_pipe_set_prop_value(display, pipe, prop, value) \
> +	igt_pipe_obj_set_prop_value(&(display)->pipes[(pipe)], prop,
> value)
> +
> +void igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool
> force);
>  
>  void igt_enable_connectors(void);
>  void igt_reset_connectors(void);
> diff --git a/tests/kms_atomic_interruptible.c
> b/tests/kms_atomic_interruptible.c
> index 4a2a577412cc..64a005597a21 100644
> --- a/tests/kms_atomic_interruptible.c
> +++ b/tests/kms_atomic_interruptible.c
> @@ -158,8 +158,8 @@ static void run_plane_test(igt_display_t
> *display, enum pipe pipe, igt_output_t
>  				uint32_t count_props[3] = { 2, 1, 6
> };
>  				uint32_t props[] = {
>  					/* crtc: 2 props */
> -					plane->pipe-
> >atomic_props_crtc[IGT_CRTC_MODE_ID],
> -					plane->pipe-
> >atomic_props_crtc[IGT_CRTC_ACTIVE],
> +					plane->pipe-
> >props[IGT_CRTC_MODE_ID],
> +					plane->pipe-
> >props[IGT_CRTC_ACTIVE],
>  					/* connector: 1 prop */
>  					output-
> >props[IGT_CONNECTOR_CRTC_ID],
>  					/* plane: remainder props */
> @@ -255,6 +255,10 @@ static void run_plane_test(igt_display_t
> *display, enum pipe pipe, igt_output_t
>  
>  	igt_waitchildren();
>  
> +	/* The mode is unset by the forked helper, force a refresh
> here */
> +	if (test_type == test_legacy_modeset || test_type ==
> test_atomic_modeset)
> +		igt_pipe_refresh(display, pipe, true);
> +
>  	igt_plane_set_fb(plane, NULL);
>  	igt_plane_set_fb(primary, NULL);
>  	igt_output_set_pipe(output, PIPE_NONE);
> diff --git a/tests/kms_atomic_transition.c
> b/tests/kms_atomic_transition.c
> index 2ae75f2d6630..7ddb65cea183 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -633,7 +633,7 @@ static unsigned set_combinations(igt_display_t
> *display, unsigned mask, struct i
>  		drmModeModeInfo *mode = NULL;
>  
>  		if (!(mask & (1 << pipe))) {
> -			if (display->pipes[pipe].mode_blob) {
> +			if (igt_pipe_is_prop_changed(display, pipe,
> IGT_CRTC_ACTIVE)) {
>  				event_mask |= 1 << pipe;
>  				igt_plane_set_fb(plane, NULL);
>  			}
> diff --git a/tests/kms_crtc_background_color.c
> b/tests/kms_crtc_background_color.c
> index e12e163449f8..659a30b90219 100644
> --- a/tests/kms_crtc_background_color.c
> +++ b/tests/kms_crtc_background_color.c
> @@ -137,7 +137,7 @@ static void test_crtc_background(data_t *data)
>  		igt_output_set_pipe(output, pipe);
>  
>  		plane = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> -		igt_require(plane->pipe->background_property);
> +		igt_require(plane->pipe-
> >props[IGT_CRTC_BACKGROUND]);
>  
>  		prepare_crtc(data, output, pipe, plane, 1, PURPLE,
> BLACK64);
>  
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx