[igt-dev] [PATCH i-g-t v2 4/4] tests/kms_chamelium: Test HPD for different mode handling scenarios

Kunal Joshi kunal1.joshi at intel.com
Thu Apr 2 02:25:34 UTC 2020


On 2020-04-01 at 19:29:12 +0300, Arkadiusz Hiler wrote:
> The default scenario is now performing all hotplugs with modes disabled
> on all connectors. This is the quickest of the tests and represents
> userspace not caring about the new display (e.g. explicitly disabled).
> 
> *-hpd-enable-disable-mode covers the most common userspace behavior
> where each hotplug event is accompanied by a corresponding enabling /
> disabling commit.
> 
> *-hpd-with-enabled-mode explicitly targets the scenario where we have
> mode enabled and never disable it as we do hotplugs to reproduce the
> issue we see with TypeC connectors for ICL and TGL.
> 
> v2:
>  - refresh igt_display output state after reprobing
>  - get mode and set pipe only after we have connector plugged in
> 
> Cc: Kunal Joshi <kunal1.joshi at intel.com>
> Cc: Imre Deak <imre.deak at intel.com>
> Issue: https://gitlab.freedesktop.org/drm/intel/issues/323
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> ---
>  lib/igt_kms.c         |   2 +-
>  lib/igt_kms.h         |   1 +
>  tests/kms_chamelium.c | 315 ++++++++++++++++++++++++++++--------------
>  3 files changed, 216 insertions(+), 102 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index b461818a..9fc9834a 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1666,7 +1666,7 @@ static void igt_display_log_shift(igt_display_t *display, int shift)
>  	igt_assert(display->log_shift >= 0);
>  }
>  
> -static void igt_output_refresh(igt_output_t *output)
> +void igt_output_refresh(igt_output_t *output)
>  {
>  	igt_display_t *display = output->display;
>  	unsigned long crtc_idx_mask = 0;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index cd3fdbc0..adca59ac 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -423,6 +423,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
>  					     int plane_type, int index);
>  igt_output_t *igt_output_from_connector(igt_display_t *display,
>      drmModeConnector *connector);
> +void igt_output_refresh(igt_output_t *output);
>  const drmModeModeInfo *igt_std_1024_mode_get(void);
>  
>  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 236e1010..3019d270 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -45,6 +45,12 @@ enum test_edid {
>  };
>  #define TEST_EDID_COUNT 5
>  
> +enum test_modeset_mode {
> +	TEST_MODESET_ON,
> +	TEST_MODESET_ON_OFF,
> +	TEST_MODESET_OFF,
> +};
> +
>  typedef struct {
>  	struct chamelium *chamelium;
>  	struct chamelium_port **ports;
> @@ -111,12 +117,17 @@ reprobe_connector(data_t *data, struct chamelium_port *port)
>  {
>  	drmModeConnector *connector;
>  	drmModeConnection status;
> +	igt_output_t *output;
>  
>  	igt_debug("Reprobing %s...\n", chamelium_port_get_name(port));
>  	connector = chamelium_port_get_connector(data->chamelium, port, true);
>  	igt_assert(connector);
>  	status = connector->connection;
>  
> +	/* let's make sure that igt_display is up to date too */
> +	output = igt_output_from_connector(&data->display, connector);
> +	igt_output_refresh(output);
> +
>  	drmModeFreeConnector(connector);
>  	return status;
>  }
> @@ -294,14 +305,140 @@ reset_state(data_t *data, struct chamelium_port *port)
>  	}
>  }
>  
> +static void chamelium_paint_xr24_pattern(uint32_t *data,
> +					 size_t width, size_t height,
> +					 size_t stride, size_t block_size)
> +{
> +	uint32_t colors[] = { 0xff000000,
> +			      0xffff0000,
> +			      0xff00ff00,
> +			      0xff0000ff,
> +			      0xffffffff };
> +	unsigned i, j;
> +
> +	for (i = 0; i < height; i++)
> +		for (j = 0; j < width; j++)
> +			*(data + i * stride / 4 + j) = colors[((j / block_size) + (i / block_size)) % 5];
> +}
> +
> +static int chamelium_get_pattern_fb(data_t *data, size_t width, size_t height,
> +				    uint32_t fourcc, size_t block_size,
> +				    struct igt_fb *fb)
> +{
> +	int fb_id;
> +	void *ptr;
> +
> +	igt_assert(fourcc == DRM_FORMAT_XRGB8888);
> +
> +	fb_id = igt_create_fb(data->drm_fd, width, height, fourcc,
> +			      LOCAL_DRM_FORMAT_MOD_NONE, fb);
> +	igt_assert(fb_id > 0);
> +
> +	ptr = igt_fb_map_buffer(fb->fd, fb);
> +	igt_assert(ptr);
> +
> +	chamelium_paint_xr24_pattern(ptr, width, height, fb->strides[0],
> +				     block_size);
> +	igt_fb_unmap_buffer(fb, ptr);
> +
> +	return fb_id;
> +}
> +
> +static void
> +enable_output(data_t *data,
> +	      struct chamelium_port *port,
> +	      igt_output_t *output,
> +	      drmModeModeInfo *mode,
> +	      struct igt_fb *fb)
> +{
> +	igt_display_t *display = output->display;
> +	igt_plane_t *primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +	drmModeConnector *connector = chamelium_port_get_connector(
> +	    data->chamelium, port, false);
> +
> +	igt_assert(primary);
> +
> +	igt_plane_set_size(primary, mode->hdisplay, mode->vdisplay);
> +	igt_plane_set_fb(primary, fb);
> +	igt_output_override_mode(output, mode);
> +
> +	/* Clear any color correction values that might be enabled */
> +	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
> +		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_DEGAMMA_LUT, NULL, 0);
> +	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT))
> +		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_GAMMA_LUT, NULL, 0);
> +	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
> +		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_CTM, NULL, 0);
> +
> +	igt_display_commit2(display, COMMIT_ATOMIC);
> +
> +	if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +		usleep(250000);
> +
> +	drmModeFreeConnector(connector);
> +}
> +
> +static enum pipe get_pipe_for_output(igt_display_t *display, igt_output_t *output)
> +{
> +	enum pipe pipe;
> +
> +	for_each_pipe(display, pipe) {
> +		if (igt_pipe_connector_valid(pipe, output)) {
> +			return pipe;
> +		}
> +	}
> +
> +	igt_assert_f(false, "No pipe found for output %s\n",
> +		     igt_output_name(output));
> +}
> +
> +static void create_fb_for_mode(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode)
> +{
> +	int fb_id;
> +
> +	fb_id = chamelium_get_pattern_fb(data, mode->hdisplay, mode->vdisplay,
> +					 DRM_FORMAT_XRGB8888, 64, fb);
> +
> +	igt_assert(fb_id > 0);
> +}
> +
> +static drmModeModeInfo get_mode_for_port(struct chamelium *chamelium,
> +					 struct chamelium_port *port)
> +{
> +	drmModeConnector *connector = chamelium_port_get_connector(chamelium,
> +								   port, false);
> +	drmModeModeInfo mode;
> +	igt_assert(&connector->modes[0] != NULL);
> +	memcpy(&mode, &connector->modes[0], sizeof(mode));
> +	drmModeFreeConnector(connector);
> +	return mode;
> +}
> +
> +static igt_output_t *get_output_for_port(data_t *data,
> +					 struct chamelium_port *port)
> +{
> +	drmModeConnector *connector = chamelium_port_get_connector(data->chamelium,
> +								   port, false);
> +	igt_output_t *output = igt_output_from_connector(&data->display,
> +							 connector);
> +	drmModeFreeConnector(connector);
> +	igt_assert(output != NULL);
> +	return output;
> +}
> +
>  static const char test_basic_hotplug_desc[] =
>  	"Check that we get uevents and updated connector status on "
>  	"hotplug and unplug";
>  static void
> -test_basic_hotplug(data_t *data, struct chamelium_port *port, int toggle_count)
> +test_hotplug(data_t *data, struct chamelium_port *port, int toggle_count,
> +	     enum test_modeset_mode modeset_mode)
>  {
> -	struct udev_monitor *mon = igt_watch_hotplug();
>  	int i;
> +	enum pipe pipe;
> +	struct igt_fb fb = {0};
> +	drmModeModeInfo mode;
> +	struct udev_monitor *mon = igt_watch_hotplug();
> +	igt_output_t *output = get_output_for_port(data, port);
>  
>  	reset_state(data, NULL);
>  	igt_hpd_storm_set_threshold(data->drm_fd, 0);
> @@ -316,15 +453,36 @@ test_basic_hotplug(data_t *data, struct chamelium_port *port, int toggle_count)
>  						 DRM_MODE_CONNECTED);
>  		igt_flush_hotplugs(mon);
>  
> +		if (modeset_mode == TEST_MODESET_ON_OFF ||
> +		    (modeset_mode == TEST_MODESET_ON && i == 0 )) {
> +			if (i == 0) {
> +				/* We can only get mode and pipe once we are connected */
> +				pipe = get_pipe_for_output(&data->display, output);
> +				mode = get_mode_for_port(data->chamelium, port);
> +				create_fb_for_mode(data, &fb, &mode);
> +			}
> +
> +			igt_output_set_pipe(output, pipe);
> +			enable_output(data, port, output, &mode, &fb);
> +		}
> +
>  		/* Now check if we get a hotplug from disconnection */
>  		chamelium_unplug(data->chamelium, port);
>  
>  		wait_for_connector_after_hotplug(data, mon, port,
>  						 DRM_MODE_DISCONNECTED);
> +
> +		igt_flush_hotplugs(mon);
> +
> +		if (modeset_mode == TEST_MODESET_ON_OFF) {
> +			igt_output_set_pipe(output, PIPE_NONE);
> +			igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +		}
>  	}
>  
>  	igt_cleanup_hotplug(mon);
>  	igt_hpd_storm_reset(data->drm_fd);
> +	igt_remove_fb(data->drm_fd, &fb);
>  }
>  
>  static const struct edid *get_edid(enum test_edid edid);
> @@ -530,10 +688,7 @@ prepare_output(data_t *data, struct chamelium_port *port, enum test_edid edid)
>  {
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
> -	drmModeConnector *connector =
> -		chamelium_port_get_connector(data->chamelium, port, false);
>  	enum pipe pipe;
> -	bool found = false;
>  
>  	/* The chamelium's default EDID has a lot of resolutions, way more then
>  	 * we need to test. Additionally the default EDID doesn't support HDMI
> @@ -546,101 +701,17 @@ prepare_output(data_t *data, struct chamelium_port *port, enum test_edid edid)
>  
>  	igt_display_reset(display);
>  
> -	output = igt_output_from_connector(display, connector);
> +	output = get_output_for_port(data, port);
>  
>  	/* Refresh pipe to update connected status */
>  	igt_output_set_pipe(output, PIPE_NONE);
>  
> -	for_each_pipe(display, pipe) {
> -		if (!igt_pipe_connector_valid(pipe, output))
> -			continue;
> -
> -		found = true;
> -		break;
> -	}
> -
> -	igt_assert_f(found, "No pipe found for output %s\n", igt_output_name(output));
> -
> +	pipe = get_pipe_for_output(display, output);
>  	igt_output_set_pipe(output, pipe);
>  
> -	drmModeFreeConnector(connector);
> -
>  	return output;
>  }
>  
> -static void
> -enable_output(data_t *data,
> -	      struct chamelium_port *port,
> -	      igt_output_t *output,
> -	      drmModeModeInfo *mode,
> -	      struct igt_fb *fb)
> -{
> -	igt_display_t *display = output->display;
> -	igt_plane_t *primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> -	drmModeConnector *connector = chamelium_port_get_connector(
> -	    data->chamelium, port, false);
> -
> -	igt_assert(primary);
> -
> -	igt_plane_set_size(primary, mode->hdisplay, mode->vdisplay);
> -	igt_plane_set_fb(primary, fb);
> -	igt_output_override_mode(output, mode);
> -
> -	/* Clear any color correction values that might be enabled */
> -	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
> -		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_DEGAMMA_LUT, NULL, 0);
> -	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT))
> -		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_GAMMA_LUT, NULL, 0);
> -	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
> -		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_CTM, NULL, 0);
> -
> -	igt_display_commit2(display, COMMIT_ATOMIC);
> -
> -	if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> -		usleep(250000);
> -
> -	drmModeFreeConnector(connector);
> -}
> -
> -static void chamelium_paint_xr24_pattern(uint32_t *data,
> -					 size_t width, size_t height,
> -					 size_t stride, size_t block_size)
> -{
> -	uint32_t colors[] = { 0xff000000,
> -			      0xffff0000,
> -			      0xff00ff00,
> -			      0xff0000ff,
> -			      0xffffffff };
> -	unsigned i, j;
> -
> -	for (i = 0; i < height; i++)
> -		for (j = 0; j < width; j++)
> -			*(data + i * stride / 4 + j) = colors[((j / block_size) + (i / block_size)) % 5];
> -}
> -
> -static int chamelium_get_pattern_fb(data_t *data, size_t width, size_t height,
> -				    uint32_t fourcc, size_t block_size,
> -				    struct igt_fb *fb)
> -{
> -	int fb_id;
> -	void *ptr;
> -
> -	igt_assert(fourcc == DRM_FORMAT_XRGB8888);
> -
> -	fb_id = igt_create_fb(data->drm_fd, width, height, fourcc,
> -			      LOCAL_DRM_FORMAT_MOD_NONE, fb);
> -	igt_assert(fb_id > 0);
> -
> -	ptr = igt_fb_map_buffer(fb->fd, fb);
> -	igt_assert(ptr);
> -
> -	chamelium_paint_xr24_pattern(ptr, width, height, fb->strides[0],
> -				     block_size);
> -	igt_fb_unmap_buffer(fb, ptr);
> -
> -	return fb_id;
> -}
> -
>  static void do_test_display(data_t *data, struct chamelium_port *port,
>  			    igt_output_t *output, drmModeModeInfo *mode,
>  			    uint32_t fourcc, enum chamelium_check check,
> @@ -2539,13 +2610,27 @@ igt_main
>  
>  		igt_describe(test_basic_hotplug_desc);
>  		connector_subtest("dp-hpd", DisplayPort)
> -			test_basic_hotplug(&data, port,
> -					   HPD_TOGGLE_COUNT_DP_HDMI);
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_DP_HDMI,
> +				     TEST_MODESET_OFF);
>  
>  		igt_describe(test_basic_hotplug_desc);
>  		connector_subtest("dp-hpd-fast", DisplayPort)
> -			test_basic_hotplug(&data, port,
> -					   HPD_TOGGLE_COUNT_FAST);
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_OFF);
> +
> +		igt_describe(test_basic_hotplug_desc);
> +		connector_subtest("dp-hpd-enable-disable-mode", DisplayPort)
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_ON_OFF);
> +
> +		igt_describe(test_basic_hotplug_desc);
> +		connector_subtest("dp-hpd-with-enabled-mode", DisplayPort)
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_ON);
>  
>  		igt_describe(test_edid_read_desc);
>  		connector_subtest("dp-edid-read", DisplayPort) {
> @@ -2634,13 +2719,27 @@ igt_main
>  
>  		igt_describe(test_basic_hotplug_desc);
>  		connector_subtest("hdmi-hpd", HDMIA)
> -			test_basic_hotplug(&data, port,
> -					   HPD_TOGGLE_COUNT_DP_HDMI);
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_DP_HDMI,
> +				     TEST_MODESET_OFF);
>  
>  		igt_describe(test_basic_hotplug_desc);
>  		connector_subtest("hdmi-hpd-fast", HDMIA)
> -			test_basic_hotplug(&data, port,
> -					   HPD_TOGGLE_COUNT_FAST);
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_OFF);
> +
> +		igt_describe(test_basic_hotplug_desc);
> +		connector_subtest("hdmi-hpd-enable-disable-mode", HDMIA)
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_ON_OFF);
> +
> +		igt_describe(test_basic_hotplug_desc);
> +		connector_subtest("hdmi-hpd-with-enabled-mode", HDMIA)
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_ON);
>  
>  		igt_describe(test_edid_read_desc);
>  		connector_subtest("hdmi-edid-read", HDMIA) {
> @@ -2795,11 +2894,25 @@ igt_main
>  
>  		igt_describe(test_basic_hotplug_desc);
>  		connector_subtest("vga-hpd", VGA)
> -			test_basic_hotplug(&data, port, HPD_TOGGLE_COUNT_VGA);
> +			test_hotplug(&data, port, HPD_TOGGLE_COUNT_VGA,
> +				     TEST_MODESET_OFF);
>  
>  		igt_describe(test_basic_hotplug_desc);
>  		connector_subtest("vga-hpd-fast", VGA)
> -			test_basic_hotplug(&data, port, HPD_TOGGLE_COUNT_FAST);
> +			test_hotplug(&data, port, HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_OFF);
> +
> +		igt_describe(test_basic_hotplug_desc);
> +		connector_subtest("hdmi-hpd-enable-disable-mode", VGA)
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_ON_OFF);
> +
> +		igt_describe(test_basic_hotplug_desc);
> +		connector_subtest("hdmi-hpd-with-enabled-mode", VGA)
> +			test_hotplug(&data, port,
> +				     HPD_TOGGLE_COUNT_FAST,
> +				     TEST_MODESET_ON);
>  
Can be rename as vga-hpd-enable-disable-mode and vga-hpd-with-enabled-mode

Otherwise looks good to me
Reviewed-by: Kunal Joshi <kunal1.joshi at intel.com>
>  		igt_describe(test_edid_read_desc);
>  		connector_subtest("vga-edid-read", VGA) {
> -- 
> 2.24.1
> 


More information about the igt-dev mailing list