[igt-dev] [PATCH RFC i-g-t] tests/kms_chamelium: disable output when resetting state

[Ser, Simon]

On Thu, 2019-07-11 at 15:14 +0200, Daniel Vetter wrote:
> On Thu, Jul 11, 2019 at 2:47 PM Ser, Simon <simon.ser at intel.com> wrote:
> > On Thu, 2019-07-11 at 14:40 +0200, Daniel Vetter wrote:
> > > On Thu, Jul 11, 2019 at 1:11 PM Simon Ser <simon.ser at intel.com> wrote:
> > > > Quoting Imre from the bug comments:
> > > > 
> > > > > If unplugging a DP-alt display while there is an active modeset on it, and
> > > > > then replugging the DP-alt display, the HW/FW/driver won't signal the
> > > > > corresponding plug-in hotplug event until the mode is disabled.
> > > > > 
> > > > > The failing tests do this:
> > > > > 
> > > > > - <have the output enabled>
> > > > > - unplug the display, keeping the output enabled
> > > > > - wait/poll for the disconnect status -> success
> > > > > - replug the display, while the output is still enabled
> > > > > - wait/poll for the connect status -> fail
> > > > > 
> > > > > This sequence is not supported with DP-alt sinks (like the USB-C to HDMI
> > > > > dongle), as I described above.
> > > > 
> > > > This patch changes reset_state to disable the output before unplugging the
> > > > connector (via chamelium_reset).
> > > > 
> > > > Signed-off-by: Simon Ser <simon.ser at intel.com>
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111045
> > > > ---
> > > > 
> > > > This is just a RFC to check that the root issue is what we think it is.
> > > 
> > > Yeah we can't merge this, this would completely break the hotunplug
> > > usecase, which is:
> > > 1. user yanks cable
> > > 2. desktop reacts to hotunplug and shuts down the screen
> > 
> > Would disabling the screen after the drmModeConnector says it's
> > disconnected be better?
> 
> I guess depends what you want to test. If it's the hotunplug event
> itself, then that should arrive without userspace doing anything at
> all. If you don't care for the uevent, not sure why you'd poll
> connector status ...

Note that we already have a test (test_basic_hotplug) that unplugs and
waits for the uevent.

> > > If the hw can't do a hotunplug while there's an active mode, then we
> > > have a chicken&egg situation, and we need to resort to some form of
> > > polling (hooray). Hotunplug better work if the display is on.
> > 
> > TBH I'm not sure why it's so important to have all tests keep the mode
> > active during an unplug-replug sequence.
> 
> That's another use-case, at least for !DP-MST: We re-train on replug
> to make sure a quick cable yank doesn't result in disappointment and
> black screens. MST is a bit different in this regard, but iirc Lyude
> has been thinking about making that happen even for mst. So also
> something that would be nice to test.

After discussion on IRC, it seems that maybe the unplug-replug scenario
shouldn't work if userspace doesn't listen to uevents.

I'd really like most Chamelium tests to "do the right thing" (disable
outputs on unplug). We could always have some "legacy tests" that are
not run on DP-MST and USB-C and try to keep the connector active. I
don't see any value in duplicating all tests for both legacy and
hotplug-aware: once we know keeping the pipe active while unplugging-
replugging works, we should be fine.

Thoughts?

> > > Historical aside: Polling is disabled on may old VGA outputs because
> > > the polling can result in screen flickering/corruption. But hw got
> > > better, so even VGA managed to handle hotunplug correctly. No DE (or
> > > user fwiw) assumes nowadays that hotunplug is broken.
> > > -Daniel
> > > 
> > > >  tests/kms_chamelium.c | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > > > index b749b5598c1d..f538ad394480 100644
> > > > --- a/tests/kms_chamelium.c
> > > > +++ b/tests/kms_chamelium.c
> > > > @@ -216,6 +216,13 @@ static void
> > > >  reset_state(data_t *data, struct chamelium_port *port)
> > > >  {
> > > >         int p;
> > > > +       drmModeConnector *connector;
> > > > +       igt_output_t *output;
> > > > +
> > > > +       connector = chamelium_port_get_connector(data->chamelium, port, false);
> > > > +       output = igt_output_from_connector(&data->display, connector);
> > > > +       igt_output_set_pipe(output, PIPE_NONE);
> > > > +       igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > > > 
> > > >         chamelium_reset(data->chamelium);
> > > > 
> > > > --
> > > > 2.22.0
> > > > 
> > > > _______________________________________________
> > > > igt-dev mailing list
> > > > igt-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
>