On Fri, 2017-06-16 at 17:17 +0300, Paul Kocialkowski wrote:
> This adds two new tests: common-hpd-after-suspend and
> common-hpd-after-hibernate that are aimed at testing HPD change during
> suspend/hibernate for both DP and HDMI, at the same time.
> 
> The interest in bringing this test up is to reduce the time spent in
> testing, with the downside of less precision regarding the test's
> outcome. The per-connector tests are still available to get a more
> precise idea of which connector causes a failure, when that happens.
> 
> VGA is explicitly excluded from this test as there is currently no
> way of doing delayed hpd pulses for it.
> 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkow...@linux.intel.com>
> ---
>  tests/chamelium.c | 86 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/tests/chamelium.c b/tests/chamelium.c
> index 76b14e95..d569c8ee 100644
> --- a/tests/chamelium.c
> +++ b/tests/chamelium.c
> @@ -241,6 +241,80 @@ test_suspend_resume_hpd(data_t *data, struct 
> chamelium_port *port,
>  }
>  
>  static void
> +test_suspend_resume_hpd_common(data_t *data, enum igt_suspend_state state,
> +                            enum igt_suspend_test test)
> +{
> +     struct udev_monitor *mon = igt_watch_hotplug();
> +     struct chamelium_port *port;
> +     int p;
> +
> +     for (p = 0; p < data->port_count; p++) {
> +             port = data->ports[p];
> +             if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +                     continue;
> +
> +             reset_state(data, port);
> +
> +             igt_debug("Testing port %s\n", chamelium_port_get_name(port));
> +     }
> +
> +     igt_set_autoresume_delay(SUSPEND_RESUME_DELAY);
> +     igt_flush_hotplugs(mon);
> +
> +     /* Make sure we notice new connectors after resuming */
> +     for (p = 0; p < data->port_count; p++) {
> +             port = data->ports[p];
> +             if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +                     continue;
> +
> +             chamelium_async_hpd_pulse_start(data->chamelium, port, false,
> +                                             SUSPEND_RESUME_DELAY / 2);
> +     }
> +
> +     igt_system_suspend_autoresume(state, test);
> +     chamelium_async_hpd_pulse_finish(data->chamelium);
> +
> +     igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
> +
> +     for (p = 0; p < data->port_count; p++) {
> +             port = data->ports[p];
> +             if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +                     continue;
> +
> +             igt_assert_eq(reprobe_connector(data, port),
> +                           DRM_MODE_CONNECTED);
> +     }
> +
> +     igt_flush_hotplugs(mon);
> +
> +     /* Now make sure we notice disconnected connectors after resuming */
> +     for (p = 0; p < data->port_count; p++) {
> +             port = data->ports[p];
> +             if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +                     continue;
> +
> +             chamelium_async_hpd_pulse_start(data->chamelium, port, true,
> +                                             SUSPEND_RESUME_DELAY / 2);
> +     }
> +
> +     igt_system_suspend_autoresume(state, test);
> +     chamelium_async_hpd_pulse_finish(data->chamelium);
> +
> +     igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
> +
> +     for (p = 0; p < data->port_count; p++) {
> +             port = data->ports[p];
> +             if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +                     continue;
> +
> +             igt_assert_eq(reprobe_connector(data, port),
> +                           DRM_MODE_DISCONNECTED);
> +     }
> +
> +     igt_cleanup_hotplug(mon);
> +}
> +
> +static void

For smaller bits of code this isn't a big deal since we're talking
about tests here, but this is a lot of code to have for a single
function like this. Mind splitting this into something like (feel free
to take some liberty with the function naming)

test_suspend_resume_hpd(data_t *data, enum igt_suspend_state state, enum 
igt_suspend_test test)
  → try_suspend_resume_hpd(data_t *data, enum igt_suspend_state state, enum 
igt_suspend_test test, bool connected)

Other then that, looks good. With the changes mentioned:

Reviewed-by: Lyude Paul <ly...@redhat.com>

>  test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
>                               enum igt_suspend_state state,
>                               enum igt_suspend_test test,
> @@ -750,6 +824,18 @@ igt_main
>                       test_hpd_without_ddc(&data, port);
>       }
>  
> +     igt_subtest_group {
> +             igt_subtest("common-hpd-after-suspend")
> +                     test_suspend_resume_hpd_common(&data,
> +                                                    SUSPEND_STATE_MEM,
> +                                                    SUSPEND_TEST_NONE);
> +
> +             igt_subtest("common-hpd-after-hibernate")
> +                     test_suspend_resume_hpd_common(&data,
> +                                                    SUSPEND_STATE_DISK,
> +                                                    SUSPEND_TEST_DEVICES);
> +     }
> +
>       igt_fixture {
>               close(data.drm_fd);
>       }
-- 
Cheers,
        Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to