On Tue, 2017-06-27 at 17:26 -0400, Lyude Paul wrote:
> 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 :)

Martin was really expecting this to come around, so I hope it is in accordance
with the DRM spec. At least, it works properly with i915 on SKL.

Thanks for double-checking!

> 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 <[email protected]>
> > ---
> >  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);
> >  }
-- 
Paul Kocialkowski <[email protected]>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to