[igt-dev] [PATCH v2 i-g-t] tests/fbcon: Don't close drm fd
Matt Atwood
matthew.s.atwood at intel.com
Thu May 21 21:17:31 UTC 2020
On Wed, May 20, 2020 at 06:47:23PM -0700, José Roberto de Souza wrote:
> For GEN9+ fbc_wait_until_update() fetches pipe CRCs to check if it is
> back to fbcon and if cursor is blinking but to return to fbcon it was
> closing the drm fd.
>
> This code was only working because CRC functions only uses drm fd to
> get the debugfs dir fd and igt_debugfs_dir() tries to fallback to the
> default path in case drm fd == -1, anything different would cause the
> tests to fail.
>
> So here not closing the drm fd and still going back to fbcon, the
> tricks to do it is implemented in restore_fbcon().
>
> Not closing drm fd allowed us to remove setup_drm() and teardown_drm()
> and only doing the setup and teardown once.
>
> v2:
> - fixed commit description
>
Reviewed-by: Matt Atwood <matthew.s.atwood at intel.com>
> Cc: Matt Atwood <matthew.s.atwood at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
> tests/kms_fbcon_fbt.c | 96 +++++++++++++++++--------------------------
> 1 file changed, 38 insertions(+), 58 deletions(-)
>
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index 7eba4ac5d..e6dd4353b 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -25,6 +25,7 @@
> */
>
> #include "igt.h"
> +#include "igt_device.h"
> #include "igt_psr.h"
> #include "igt_sysfs.h"
> #include <sys/types.h>
> @@ -38,11 +39,10 @@ IGT_TEST_DESCRIPTION("Test the relationship between fbcon and the frontbuffer "
> #define MAX_CONNECTORS 32
>
> struct drm_info {
> - int fd;
> - int debugfs_fd;
> + int fd, debugfs_fd;
> + struct igt_fb fb;
> drmModeResPtr res;
> drmModeConnectorPtr connectors[MAX_CONNECTORS];
> - uint32_t devid;
> };
>
> static void wait_user(const char *msg)
> @@ -51,44 +51,6 @@ static void wait_user(const char *msg)
> igt_debug_wait_for_keypress("fbt");
> }
>
> -static void setup_drm(struct drm_info *drm)
> -{
> - int i;
> -
> - if (drm->fd >= 0)
> - return;
> -
> - drm->fd = drm_open_driver_master(DRIVER_INTEL);
> -
> - drm->res = drmModeGetResources(drm->fd);
> - igt_require(drm->res);
> - igt_assert(drm->res->count_connectors <= MAX_CONNECTORS);
> -
> - for (i = 0; i < drm->res->count_connectors; i++)
> - drm->connectors[i] = drmModeGetConnectorCurrent(drm->fd,
> - drm->res->connectors[i]);
> -
> - drm->devid = intel_get_drm_devid(drm->fd);
> -
> - kmstest_set_vt_graphics_mode();
> -}
> -
> -static void teardown_drm(struct drm_info *drm)
> -{
> - int i;
> -
> - igt_assert_eq(drmDropMaster(drm->fd), 0);
> -
> - kmstest_restore_vt_mode();
> -
> - for (i = 0; i < drm->res->count_connectors; i++)
> - drmModeFreeConnector(drm->connectors[i]);
> -
> - drmModeFreeResources(drm->res);
> - igt_assert(close(drm->fd) == 0);
> - drm->fd = -1;
> -}
> -
> static bool fbc_supported_on_chipset(int device, int debugfs_fd)
> {
> char buf[128];
> @@ -187,7 +149,7 @@ static bool fbc_wait_until_update(struct drm_info *drm)
> * For older GENs FBC is still expected to be disabled as it still
> * relies on a tiled and fenceable framebuffer to track modifications.
> */
> - if (AT_LEAST_GEN(drm->devid, 9)) {
> + if (AT_LEAST_GEN(intel_get_drm_devid(drm->fd), 9)) {
> if (!fbc_wait_until_enabled(drm->debugfs_fd))
> return false;
>
> @@ -199,7 +161,7 @@ static bool fbc_wait_until_update(struct drm_info *drm)
>
> typedef bool (*connector_possible_fn)(drmModeConnectorPtr connector);
>
> -static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
> +static void set_mode_for_one_screen(struct drm_info *drm,
> connector_possible_fn connector_possible)
> {
> int i, rc;
> @@ -224,8 +186,8 @@ static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
>
> buffer_id = igt_create_fb(drm->fd, mode->hdisplay, mode->vdisplay,
> DRM_FORMAT_XRGB8888,
> - LOCAL_I915_FORMAT_MOD_X_TILED, fb);
> - igt_draw_fill_fb(drm->fd, fb, 0xFF);
> + LOCAL_I915_FORMAT_MOD_X_TILED, &drm->fb);
> + igt_draw_fill_fb(drm->fd, &drm->fb, 0xFF);
>
> igt_info("Setting %dx%d mode for %s connector\n",
> mode->hdisplay, mode->vdisplay,
> @@ -316,11 +278,18 @@ struct feature {
> .enable = psr_debugfs_enable,
> };
>
> -static void subtest(struct drm_info *drm, struct feature *feature, bool suspend)
> +static void restore_fbcon(struct drm_info *drm)
> {
> - struct igt_fb fb;
> + kmstest_unset_all_crtcs(drm->fd, drm->res);
> + igt_remove_fb(drm->fd, &drm->fb);
> + igt_device_drop_master(drm->fd);
> + kmstest_restore_vt_mode();
> +}
>
> - setup_drm(drm);
> +static void subtest(struct drm_info *drm, struct feature *feature, bool suspend)
> +{
> + igt_device_set_master(drm->fd);
> + kmstest_set_vt_graphics_mode();
>
> igt_require(feature->supported_on_chipset(drm->fd, drm->debugfs_fd));
>
> @@ -331,7 +300,7 @@ static void subtest(struct drm_info *drm, struct feature *feature, bool suspend)
> wait_user("Modes unset.");
> igt_assert(feature->is_disabled(drm->debugfs_fd));
>
> - set_mode_for_one_screen(drm, &fb, feature->connector_possible_fn);
> + set_mode_for_one_screen(drm, feature->connector_possible_fn);
> wait_user("Screen set.");
> igt_assert(feature->wait_until_enabled(drm->debugfs_fd));
>
> @@ -342,8 +311,7 @@ static void subtest(struct drm_info *drm, struct feature *feature, bool suspend)
> igt_assert(feature->wait_until_enabled(drm->debugfs_fd));
> }
>
> - igt_remove_fb(drm->fd, &fb);
> - teardown_drm(drm);
> + restore_fbcon(drm);
>
> /* Wait for fbcon to restore itself. */
> sleep(3);
> @@ -361,13 +329,20 @@ static void subtest(struct drm_info *drm, struct feature *feature, bool suspend)
>
> static void setup_environment(struct drm_info *drm)
> {
> - int drm_fd;
> + int i;
>
> - drm_fd = drm_open_driver_master(DRIVER_INTEL);
> - igt_require(drm_fd >= 0);
> - drm->debugfs_fd = igt_debugfs_dir(drm_fd);
> + drm->fd = drm_open_driver_master(DRIVER_INTEL);
> + igt_require(drm->fd >= 0);
> + drm->debugfs_fd = igt_debugfs_dir(drm->fd);
> igt_require(drm->debugfs_fd >= 0);
> - igt_assert(close(drm_fd) == 0);
> +
> + drm->res = drmModeGetResources(drm->fd);
> + igt_require(drm->res);
> + igt_assert(drm->res->count_connectors <= MAX_CONNECTORS);
> +
> + for (i = 0; i < drm->res->count_connectors; i++)
> + drm->connectors[i] = drmModeGetConnectorCurrent(drm->fd,
> + drm->res->connectors[i]);
>
> /*
> * igt_main()->igt_subtest_init_parse_opts()->common_init() disables the
> @@ -379,10 +354,15 @@ static void setup_environment(struct drm_info *drm)
>
> static void teardown_environment(struct drm_info *drm)
> {
> - if (drm->fd >= 0)
> - teardown_drm(drm);
> + int i;
> +
> + for (i = 0; i < drm->res->count_connectors; i++)
> + drmModeFreeConnector(drm->connectors[i]);
>
> + drmModeFreeResources(drm->res);
> close(drm->debugfs_fd);
> + close(drm->fd);
> + kmstest_restore_vt_mode();
> }
>
> igt_main
> --
> 2.26.2
>
More information about the igt-dev
mailing list