From: Lyude Paul <lyude@redhat.com>
To: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible
Date: Thu, 06 Jul 2017 12:02:08 -0400 [thread overview]
Message-ID: <1499356928.20388.17.camel@redhat.com> (raw)
In-Reply-To: <20170704133319.5130-1-paul.kocialkowski@linux.intel.com>
On Tue, 2017-07-04 at 16:33 +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.
After thinking on this for a while I still have to NAK it unfortunately
:(. While this stuff will work for i915 since it's not really following
the expected DRM HPD behavior there's not much guarantee it'll work for
anything else. I think we need to have a more well-defined hotplug
event model in DRM before we consider adding patches like this.
>
> 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);
> }
--
Cheers,
Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
prev parent reply other threads:[~2017-07-06 16:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-04 13:33 [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible Paul Kocialkowski
2017-07-04 13:33 ` [PATCH i-g-t v4 2/3] chamelium: Remove init reset duplicate in favor of per-test reset Paul Kocialkowski
2017-07-06 15:45 ` Lyude Paul
2017-07-04 13:33 ` [PATCH i-g-t v4 3/3] Make igtrc configuration common, with configurable suspend/resume delay Paul Kocialkowski
2017-07-06 15:57 ` Lyude Paul
2017-07-06 16:02 ` Lyude Paul [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1499356928.20388.17.camel@redhat.com \
--to=lyude@redhat.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=paul.kocialkowski@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).