[igt-dev] [PATCH i-g-t 3/3] tests/kms_chamelium: Introduce HPD short storm tests
Ville Syrjälä
ville.syrjala at linux.intel.com
Tue Dec 11 17:34:26 UTC 2018
On Tue, Dec 11, 2018 at 07:09:26PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 20, 2018 at 07:33:38PM -0500, Lyude wrote:
> > From: Lyude Paul <lyude at redhat.com>
> >
> > This is to follow up with the changes that were recently made to HPD
> > storm detection in https://patchwork.freedesktop.org/patch/260597/ , as
> > now there are systems which will be counting short IRQs towards HPD
> > storms. As such, we introduce helpers to control HPD short storm
> > detection, along with some new kms_chamelium tests for it.
> >
> > Please note that at the time of writing this: these tests currently
> > fail. That is because the i915_hpd_storm_ctl debugfs currently forgets
> > to synchronize with all IRQs and hotplugging work before returning the
> > HPD storm status to the user. A patch for this will be sent onto the ML
> > very shortly.
>
> That went in no? If yes, this paragraph seems pointless now.
>
> >
> > Signed-off-by: Lyude Paul <lyude at redhat.com>
> > ---
> > lib/igt_debugfs.c | 85 +++++++++++++++++++++++++++++++++++++++++++
> > lib/igt_debugfs.h | 4 ++
> > tests/kms_chamelium.c | 45 +++++++++++++++++++++++
> > 3 files changed, 134 insertions(+)
> >
> > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> > index 358b4cab..530b70b7 100644
> > --- a/lib/igt_debugfs.c
> > +++ b/lib/igt_debugfs.c
> > @@ -543,6 +543,7 @@ static void igt_hpd_storm_exit_handler(int sig)
> > int fd = drm_open_driver(DRIVER_INTEL);
> >
> > /* Here we assume that only one i915 device will be ever present */
> > + igt_hpd_short_storm_reset(fd);
> > igt_hpd_storm_reset(fd);
> >
> > close(fd);
> > @@ -660,6 +661,90 @@ void igt_require_hpd_storm_ctl(int drm_fd)
> >
> > igt_require_f(fd > 0, "No i915_hpd_storm_ctl found in debugfs\n");
> > close(fd);
> > +
> > + fd = igt_debugfs_open(drm_fd, "i915_hpd_short_storm_ctl", O_RDONLY);
> > + igt_require_f(fd > 0, "No i915_hpd_short_storm_ctl found in debugfs\n");
> > + close(fd);
> > +}
> > +
> > +/**
> > + * igt_hpd_short_storm_set_enabled:
> > + *
> > + * Convienence helper to enable or disable HPD short storm detection. If hotplug
> > + * detection was disabled on any ports due to an HPD storm, it will be
> > + * immediately re-enabled.
> > + *
> > + * If the system does not support HPD storm detection, this function does
> > + * nothing.
> > + *
> > + * See: https://01.org/linuxgraphics/gfx-docs/drm/gpu/i915.html#hotplug
> > + */
> > +void igt_hpd_short_storm_set_enabled(int drm_fd, bool enabled)
> > +{
> > + int fd = igt_debugfs_open(drm_fd, "i915_hpd_short_storm_ctl", O_WRONLY);
> > + const char *buf = enabled ? "y" : "n";
> > +
> > + if (fd < 0)
> > + return;
> > +
> > + igt_debug("%sabling HPD short storm detection\n",
> > + enabled ? "En" : "Dis");
> > + igt_assert_eq(write(fd, buf, strlen(buf)), strlen(buf));
> > +
> > + close(fd);
> > +}
> > +
> > +/**
> > + * igt_hpd_short_storm_reset:
> > + *
> > + * Convienence helper to reset HPD short storm detection to it's default settings.
> > + * If hotplug detection was disabled on any ports due to an HPD storm, it will
> > + * be immediately re-enabled. Always called on exit if the HPD storm detection
> > + * threshold was modified during any tests.
> > + *
> > + * If the system does not support HPD storm detection, this function does
> > + * nothing.
> > + *
> > + * See: https://01.org/linuxgraphics/gfx-docs/drm/gpu/i915.html#hotplug
> > + */
> > +void igt_hpd_short_storm_reset(int drm_fd)
> > +{
> > + int fd = igt_debugfs_open(drm_fd, "i915_hpd_short_storm_ctl", O_WRONLY);
> > + const char *buf = "reset";
> > +
> > + if (fd < 0)
> > + return;
> > +
> > + igt_debug("Resetting HPD short storm detection\n");
> > + igt_assert_eq(write(fd, buf, strlen(buf)), strlen(buf));
> > +
> > + close(fd);
> > +}
> > +
> > +/**
> > + * igt_require_hpd_short_storm_on_by_default:
> > + *
> > + * Skips the current test if the system does not have HPD short storm detection
> > + * enabled by default.
> > + *
> > + * See: https://01.org/linuxgraphics/gfx-docs/drm/gpu/i915.html#hotplug
> > + */
> > +void igt_require_hpd_short_storm_on_by_default(int drm_fd)
> > +{
> > + int fd;
> > + char buf[16] = {0};
> > +
> > + /* First reset short storm detection to the system default */
> > + igt_hpd_short_storm_reset(drm_fd);
> > +
> > + /* Now check if the default is enabled or not */
> > + fd = igt_debugfs_open(drm_fd, "i915_hpd_short_storm_ctl", O_RDONLY);
> > +
> > + igt_require_f(fd > 0, "No i915_hpd_short_storm_ctl found in debugfs\n");
> > + igt_assert_lt(0, read(fd, buf, sizeof(buf)));
> > + igt_require_f(strncmp(buf, "Enabled: yes\n", sizeof(buf)) == 0,
> > + "System keeps HPD short storm detection disabled by default\n");
> > + close(fd);
> > }
> >
> > static igt_pipe_crc_t *
> > diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> > index 1233cd8f..21b382a3 100644
> > --- a/lib/igt_debugfs.h
> > +++ b/lib/igt_debugfs.h
> > @@ -143,6 +143,10 @@ void igt_hpd_storm_reset(int fd);
> > bool igt_hpd_storm_detected(int fd);
> > void igt_require_hpd_storm_ctl(int fd);
> >
> > +void igt_hpd_short_storm_set_enabled(int fd, bool enabled);
> > +void igt_hpd_short_storm_reset(int fd);
> > +void igt_require_hpd_short_storm_on_by_default(int fd);
> > +
> > /*
> > * Drop caches
> > */
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 0097dbcc..7ab33830 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -45,6 +45,7 @@ typedef struct {
> > #define HOTPLUG_TIMEOUT 20 /* seconds */
> >
> > #define HPD_STORM_PULSE_INTERVAL_DP 100 /* ms */
> > +#define HPD_SHORT_STORM_PULSE_INTERVAL_DP 2 /* ms */
>
> A sink should deassert hpd for .5 to 1 ms to generate a short hpd.
> 2 ms is the max short pulse duration accepted by a source device.
> So if if we want to guarantee that we generate a short hpd as
> opposed to a long hpd we should probably not aim for the full
> 2 ms here.
Ah. So we split it 1:1 for the deassert and assert phases.
But the docs do not explain how the resulting pulse will
look like.
Ie. do we get
_________
/ \_________
<- D ms -><- A ms ->
or
_________
_________/ \
<- D ms -><- A ms ->
or
__________________
/ \
<- D ms -><- A ms ->
or
_________/\_________
<- D ms -><- A ms ->
I'm guessin it's not the last one :). The third one would also be
a bit weird perhaps but could be the case I suppose, and that would
give us the full 2 ms pulse. My money would be on the first option
though, in which case the 2ms total duration seems fine.
>
> > #define HPD_STORM_PULSE_INTERVAL_HDMI 200 /* ms */
> >
> > #define HPD_TOGGLE_COUNT_VGA 5
> > @@ -775,6 +776,44 @@ test_hpd_storm_disable(data_t *data, struct chamelium_port *port, int width)
> > igt_hpd_storm_reset(data->drm_fd);
> > }
> >
> > +static void
> > +test_hpd_short_storm_detect(data_t *data, struct chamelium_port *port)
> > +{
> > + igt_require_hpd_storm_ctl(data->drm_fd);
> > + igt_require_hpd_short_storm_on_by_default(data->drm_fd);
> > + reset_state(data, port);
> > +
> > + /* Leave room in the threshold for the two long pulses this might
> > + * cause
> > + */
> > + igt_hpd_storm_set_threshold(data->drm_fd, 21);
> > + chamelium_fire_hpd_pulses(data->chamelium, port,
> > + HPD_SHORT_STORM_PULSE_INTERVAL_DP, 25, false);
> > + igt_assert(igt_hpd_storm_detected(data->drm_fd));
> > +
> > + igt_hpd_storm_reset(data->drm_fd);
> > +}
> > +
> > +static void
> > +test_hpd_short_storm_disable(data_t *data, struct chamelium_port *port)
> > +{
> > + igt_require_hpd_storm_ctl(data->drm_fd);
> > + reset_state(data, port);
> > +
> > + igt_hpd_short_storm_set_enabled(data->drm_fd, false);
> > +
> > + /* Leave room in the threshold for the two long pulses this might
> > + * cause
> > + */
> > + igt_hpd_storm_set_threshold(data->drm_fd, 21);
> > + chamelium_fire_hpd_pulses(data->chamelium, port,
> > + HPD_SHORT_STORM_PULSE_INTERVAL_DP, 25, false);
> > + igt_assert(!igt_hpd_storm_detected(data->drm_fd));
> > +
> > + igt_hpd_short_storm_reset(data->drm_fd);
> > + igt_hpd_storm_reset(data->drm_fd);
> > +}
> > +
> > #define for_each_port(p, port) \
> > for (p = 0, port = data.ports[p]; \
> > p < data.port_count; \
> > @@ -856,6 +895,12 @@ igt_main
> > test_hpd_storm_disable(&data, port,
> > HPD_STORM_PULSE_INTERVAL_DP);
> >
> > + connector_subtest("dp-hpd-short-storm", DisplayPort)
> > + test_hpd_short_storm_detect(&data, port);
> > +
> > + connector_subtest("dp-hpd-short-storm-disable", DisplayPort)
> > + test_hpd_short_storm_disable(&data, port);
> > +
> > connector_subtest("dp-edid-change-during-suspend", DisplayPort)
> > test_suspend_resume_edid_change(&data, port,
> > SUSPEND_STATE_MEM,
> > --
> > 2.19.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
More information about the igt-dev
mailing list