Re: [igt-dev] [PATCH i-g-t] tests/chamelium: Add test for hotplug workaround

[Imre Deak <imre.deak@intel.com>]

Hi Jose,

On Tue, Mar 12, 2019 at 05:59:45PM -0700, José Roberto de Souza wrote:
> It is know that some unpowered type-c dongles can take some time to
> boot and be responsible in the DDC/aux transaction lines so a
> workaround was implemented in kernel(drm/i915: Enable hotplug retry)
> to fix it but is possible that this could happen to other DP sinks.
> 
> So this test will try to simulate the sceneario described above, it
> will disable the DDC lines and plug the connector, the hotplug should
> fail and then enabling the DDC lines kernel should report the
> connector as connected.
> 
> The workaround will reprobe connector after 1 second after kernel
> gives up on the first try to probe the connector, so that is why a
> smaller timeout to detect hotplug was needed.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  tests/kms_chamelium.c | 70 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index c2090037..50a553f2 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -45,6 +45,8 @@ typedef struct {
>  
>  #define HOTPLUG_TIMEOUT 20 /* seconds */
>  
> +#define FAST_HOTPLUG_SEC_TIMEOUT (1)
> +
>  #define HPD_STORM_PULSE_INTERVAL_DP 100 /* ms */
>  #define HPD_STORM_PULSE_INTERVAL_HDMI 200 /* ms */
>  
> @@ -107,6 +109,21 @@ reprobe_connector(data_t *data, struct chamelium_port *port)
>  	return status;
>  }
>  
> +static drmModeConnection
> +connector_status_get(data_t *data, struct chamelium_port *port)
> +{
> +	drmModeConnector *connector;
> +	drmModeConnection status;
> +
> +	igt_debug("Getting connector state %s...\n", chamelium_port_get_name(port));
> +	connector = chamelium_port_get_connector(data->chamelium, port, false);
> +	igt_assert(connector);
> +	status = connector->connection;
> +
> +	drmModeFreeConnector(connector);
> +	return status;
> +}
> +
>  static void
>  wait_for_connector(data_t *data, struct chamelium_port *port,
>  		   drmModeConnection status)
> @@ -253,6 +270,56 @@ test_basic_hotplug(data_t *data, struct chamelium_port *port, int toggle_count)
>  	igt_hpd_storm_reset(data->drm_fd);
>  }
>  
> +/*
> + * Test kernel workaround for sinks that takes some time to have the DDC/aux
> + * channel responsive after the hotplug
> + */
> +static void
> +test_late_aux_wa(data_t *data, struct chamelium_port *port)
> +{
> +	struct udev_monitor *mon = igt_watch_hotplug();
> +	drmModeConnection status;
> +
> +	/* Reset will unplug all connectors */
> +	reset_state(data, NULL);
> +
> +	/* Check if it device can act on hotplugs fast enough for this test */
> +	igt_flush_hotplugs(mon);
> +	chamelium_plug(data->chamelium, port);
> +	igt_assert(igt_hotplug_detected(mon, FAST_HOTPLUG_SEC_TIMEOUT));
> +	status = connector_status_get(data, port);
> +	igt_require(status == DRM_MODE_CONNECTED);
> +
> +	igt_flush_hotplugs(mon);
> +	chamelium_unplug(data->chamelium, port);
> +	igt_assert(igt_hotplug_detected(mon, FAST_HOTPLUG_SEC_TIMEOUT));
> +	status = connector_status_get(data, port);
> +	igt_require(status == DRM_MODE_DISCONNECTED);

Do you know on which platforms the hotplug processing is that slow and
what could be the reason?

> +
> +	/* It is fast enough, lets disable the DDC lines and plug again */
> +	igt_flush_hotplugs(mon);
> +	chamelium_port_set_ddc_state(data->chamelium, port, false);
> +	chamelium_plug(data->chamelium, port);
> +	igt_assert(!chamelium_port_get_ddc_state(data->chamelium, port));
> +
> +	/*
> +	 * Give some time to kernel try to process hotplug but it should fail
> +	 */
> +	igt_hotplug_detected(mon, FAST_HOTPLUG_SEC_TIMEOUT);
> +	status = connector_status_get(data, port);
> +	igt_assert(status == DRM_MODE_DISCONNECTED);

Hm, how does a second disconnect event get signaled here? After the
previous hotplug event above where the state was disconnected already there
shouldn't have been any change to the state, hence there shouldn't be
any event sent by the driver.

> +
> +	/*
> +	 * Enable the DDC line and the kernel workaround should reprobe and
> +	 * report as connected
> +	 */
> +	chamelium_port_set_ddc_state(data->chamelium, port, true);
> +	igt_assert(chamelium_port_get_ddc_state(data->chamelium, port));
> +	igt_assert(igt_hotplug_detected(mon, FAST_HOTPLUG_SEC_TIMEOUT));
> +	status = connector_status_get(data, port);
> +	igt_assert(status == DRM_MODE_CONNECTED);
> +}
> +
>  static void
>  test_edid_read(data_t *data, struct chamelium_port *port,
>  	       int edid_id, const unsigned char *edid)
> @@ -1308,6 +1375,9 @@ igt_main
>  
>  		connector_subtest("dp-frame-dump", DisplayPort)
>  			test_display_frame_dump(&data, port);
> +
> +		connector_subtest("dp-late-aux-wa", DisplayPort)
> +			test_late_aux_wa(&data, port);
>  	}
>  
>  	igt_subtest_group {
> -- 
> 2.21.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev