Re: [PATCH i-g-t v2 5/7] tests/chamelium: Update connector state without reprobe when possible

[Lyude Paul <lyude@redhat.com>]

I -think- this one might be okay but I just got reminded of a rather
not-obvious part of the DRM spec for hotplugging, and I need to check
that this doesn't actually break that. will get back to you in a bit
with an actual review :)

On Tue, 2017-06-27 at 13:53 +0300, Paul Kocialkowski wrote:
> This renames the reprobe_connector function to update_connector and
> ensures that full reprobe of the connector is only done when really
> necessary (e.g. when changing the EDID).
> 
> A full reprobe takes time and is not required for updating the
> connector
> state. Thus, this allows executing tests faster.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  tests/chamelium.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/chamelium.c b/tests/chamelium.c
> index b412c6a7..346018a5 100644
> --- a/tests/chamelium.c
> +++ b/tests/chamelium.c
> @@ -103,13 +103,14 @@ require_connector_present(data_t *data,
> unsigned int type)
>  }
>  
>  static drmModeConnection
> -reprobe_connector(data_t *data, struct chamelium_port *port)
> +update_connector(data_t *data, struct chamelium_port *port, bool
> reprobe)
>  {
>  	drmModeConnector *connector;
>  	drmModeConnection status;
>  
> -	igt_debug("Reprobing %s...\n",
> chamelium_port_get_name(port));
> -	connector = chamelium_port_get_connector(data->chamelium,
> port, true);
> +	igt_debug("Updating %s...\n",
> chamelium_port_get_name(port));
> +	connector = chamelium_port_get_connector(data->chamelium,
> port,
> +						 reprobe);
>  	igt_assert(connector);
>  	status = connector->connection;
>  
> @@ -119,7 +120,7 @@ reprobe_connector(data_t *data, struct
> chamelium_port *port)
>  
>  static void
>  wait_for_connector(data_t *data, struct chamelium_port *port,
> -		   drmModeConnection status)
> +		   drmModeConnection status, bool reprobe)
>  {
>  	bool finished = false;
>  
> @@ -132,7 +133,7 @@ wait_for_connector(data_t *data, struct
> chamelium_port *port,
>  	 * that hpd events work in the event that hpd doesn't work
> on the system
>  	 */
>  	igt_until_timeout(HOTPLUG_TIMEOUT) {
> -		if (reprobe_connector(data, port) == status) {
> +		if (update_connector(data, port, reprobe) == status)
> {
>  			finished = true;
>  			return;
>  		}
> @@ -151,11 +152,12 @@ reset_state(data_t *data, struct chamelium_port
> *port)
>  	chamelium_reset(data->chamelium);
>  
>  	if (port) {
> -		wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED);
> +		wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED, false);
>  	} else {
>  		for (p = 0; p < data->port_count; p++) {
>  			port = data->ports[p];
> -			wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED);
> +			wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED,
> +					   false);
>  		}
>  	}
>  }
> @@ -175,7 +177,7 @@ test_basic_hotplug(data_t *data, struct
> chamelium_port *port, int toggle_count)
>  		/* Check if we get a sysfs hotplug event */
>  		chamelium_plug(data->chamelium, port);
>  		igt_assert(igt_hotplug_detected(mon,
> HOTPLUG_TIMEOUT));
> -		igt_assert_eq(reprobe_connector(data, port),
> +		igt_assert_eq(update_connector(data, port, false),
>  			      DRM_MODE_CONNECTED);
>  
>  		igt_flush_hotplugs(mon);
> @@ -183,7 +185,7 @@ test_basic_hotplug(data_t *data, struct
> chamelium_port *port, int toggle_count)
>  		/* Now check if we get a hotplug from disconnection
> */
>  		chamelium_unplug(data->chamelium, port);
>  		igt_assert(igt_hotplug_detected(mon,
> HOTPLUG_TIMEOUT));
> -		igt_assert_eq(reprobe_connector(data, port),
> +		igt_assert_eq(update_connector(data, port, false),
>  			      DRM_MODE_DISCONNECTED);
>  	}
>  
> @@ -204,7 +206,7 @@ test_edid_read(data_t *data, struct
> chamelium_port *port,
>  
>  	chamelium_port_set_edid(data->chamelium, port, edid_id);
>  	chamelium_plug(data->chamelium, port);
> -	wait_for_connector(data, port, DRM_MODE_CONNECTED);
> +	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
>  
>  	igt_assert(kmstest_get_property(data->drm_fd, connector-
> >connector_id,
>  					DRM_MODE_OBJECT_CONNECTOR,
> "EDID", NULL,
> @@ -247,13 +249,13 @@ try_suspend_resume_hpd(data_t *data, struct
> chamelium_port *port,
>  
>  	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
>  	if (port) {
> -		igt_assert_eq(reprobe_connector(data, port),
> connected ?
> +		igt_assert_eq(update_connector(data, port, false),
> connected ?
>  			      DRM_MODE_DISCONNECTED :
> DRM_MODE_CONNECTED);
>  	} else {
>  		for (p = 0; p < data->port_count; p++) {
>  			port = data->ports[p];
> -			igt_assert_eq(reprobe_connector(data, port),
> connected ?
> -				      DRM_MODE_DISCONNECTED :
> +			igt_assert_eq(update_connector(data, port,
> false),
> +				      connected ?
> DRM_MODE_DISCONNECTED :
>  				      DRM_MODE_CONNECTED);
>  		}
>  
> @@ -317,7 +319,7 @@ test_suspend_resume_edid_change(data_t *data,
> struct chamelium_port *port,
>  	/* First plug in the port */
>  	chamelium_port_set_edid(data->chamelium, port, edid_id);
>  	chamelium_plug(data->chamelium, port);
> -	wait_for_connector(data, port, DRM_MODE_CONNECTED);
> +	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
>  
>  	igt_flush_hotplugs(mon);
>  
> @@ -352,7 +354,7 @@ prepare_output(data_t *data,
>  	chamelium_port_set_edid(data->chamelium, port, data-
> >edid_id);
>  
>  	chamelium_plug(data->chamelium, port);
> -	wait_for_connector(data, port, DRM_MODE_CONNECTED);
> +	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
>  
>  	igt_display_init(display, data->drm_fd);
>  	output = igt_output_from_connector(display, connector);
> @@ -590,7 +592,7 @@ test_hpd_without_ddc(data_t *data, struct
> chamelium_port *port)
>  	chamelium_plug(data->chamelium, port);
>  
>  	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
> -	igt_assert_eq(reprobe_connector(data, port),
> DRM_MODE_CONNECTED);
> +	igt_assert_eq(update_connector(data, port, false),
> DRM_MODE_CONNECTED);
>  
>  	igt_cleanup_hotplug(mon);
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx