| Message ID | 20241209-abhinavk-modeset-fix-v2-2-4d008f6ea8d0@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm: Allow encoder modeset when connectors are changed | expand |
On Mon, Dec 09, 2024 at 05:09:25PM -0800, Jessica Zhang wrote: > Add a subtest to check that modeset is called when the connector is > changed > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/tests/Makefile | 1 + > drivers/gpu/drm/tests/drm_atomic_state_test.c | 242 ++++++++++++++++++++++++++ > 2 files changed, 243 insertions(+) > > diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile > index 56dab563abd7a7ee7c147bd6b4927e2436b82e1d..0109bcf7faa54993cce337f522eae78f0fa6ffcb 100644 > --- a/drivers/gpu/drm/tests/Makefile > +++ b/drivers/gpu/drm/tests/Makefile > @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST_HELPERS) += \ > drm_kunit_helpers.o > > obj-$(CONFIG_DRM_KUNIT_TEST) += \ > + drm_atomic_state_test.o \ > drm_buddy_test.o \ > drm_cmdline_parser_test.o \ > drm_connector_test.o \ > diff --git a/drivers/gpu/drm/tests/drm_atomic_state_test.c b/drivers/gpu/drm/tests/drm_atomic_state_test.c > new file mode 100644 > index 0000000000000000000000000000000000000000..ffc1198ccc9ce103b9bc4c5ca8c83ca7f6ab19cc > --- /dev/null > +++ b/drivers/gpu/drm/tests/drm_atomic_state_test.c > @@ -0,0 +1,242 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Test cases for the drm_atomic_state helpers > + * > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <drm/drm_atomic.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_atomic_uapi.h> > +#include <drm/drm_kunit_helpers.h> > +#include <drm/drm_probe_helper.h> > + > +#define DRM_TEST_ENC_0 BIT(0) > +#define DRM_TEST_ENC_1 BIT(1) > +#define DRM_TEST_ENC_2 BIT(2) > + > +#define DRM_TEST_CONN_0 BIT(0) > + > +static const struct drm_display_mode drm_atomic_test_mode = { > + DRM_MODE("1024x768", 0, 65000, 1024, 1048, > + 1184, 1344, 0, 768, 771, 777, 806, 0, > + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) > +}; > + > +struct drm_atomic_check_priv { > + struct drm_device drm; > + struct drm_plane *plane; > + struct drm_crtc *crtc; > + struct drm_encoder encoders[3]; > + struct drm_connector connectors[2]; > +}; > + > +static int modeset_counter; > + > +static void drm_test_encoder_mode_set(struct drm_encoder *encoder, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + modeset_counter++; > +} > + > +static const struct drm_encoder_helper_funcs drm_test_encoder_funcs = { Name is too generic, please use something more test-suite specific. > + .atomic_mode_set = drm_test_encoder_mode_set, > +}; > + > +static const struct drm_connector_funcs dummy_connector_funcs = { > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .reset = drm_atomic_helper_connector_reset, > +}; > + > +static int drm_atomic_test_dummy_get_modes(struct drm_connector *connector) > +{ > + drm_connector_helper_get_modes_fixed(connector, &drm_atomic_test_mode); > + return 1; return drm_connector_helper_get_modes_fixed() > +} > + > +static const struct drm_connector_helper_funcs dummy_connector_helper_funcs = { > + .get_modes = drm_atomic_test_dummy_get_modes, > +}; > + > +static struct drm_atomic_check_priv * > +drm_test_init_drm_components(struct kunit *test, bool has_connectors) I think this is too generic, please use a file-specific name. > +{ > + struct drm_atomic_check_priv *priv; > + struct drm_encoder *enc; > + struct drm_device *drm; > + struct device *dev; > + int ret; > + > + dev = drm_kunit_helper_alloc_device(test); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); > + > + priv = drm_kunit_helper_alloc_drm_device(test, dev, > + struct drm_atomic_check_priv, > + drm, > + DRIVER_MODESET | DRIVER_ATOMIC); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv); > + test->priv = priv; > + > + drm = &priv->drm; > + priv->plane = drm_kunit_helper_create_primary_plane(test, drm, > + NULL, > + NULL, > + NULL, 0, > + NULL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->plane); > + > + priv->crtc = drm_kunit_helper_create_crtc(test, drm, > + priv->plane, NULL, > + NULL, > + NULL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->crtc); > + > + for (int i = 0; i < ARRAY_SIZE(priv->encoders); i++) { > + enc = &priv->encoders[i]; > + > + ret = drmm_encoder_init(drm, enc, NULL, > + DRM_MODE_ENCODER_DSI, NULL); > + KUNIT_ASSERT_EQ(test, ret, 0); > + > + enc->possible_crtcs = drm_crtc_mask(priv->crtc); > + } > + > + priv->encoders[0].possible_clones = DRM_TEST_ENC_0 | DRM_TEST_ENC_1; > + priv->encoders[1].possible_clones = DRM_TEST_ENC_0 | DRM_TEST_ENC_1; > + priv->encoders[2].possible_clones = DRM_TEST_ENC_2; > + > + if (!has_connectors) > + goto done; > + > + struct drm_connector *conn; Please move next ot other variable definitions. > + > + KUNIT_ASSERT_LE(test, ARRAY_SIZE(priv->connectors), > + ARRAY_SIZE(priv->encoders)); I'd say it's BUILD_BUG_ON > + > + > + for (int i = 0; i < ARRAY_SIZE(priv->connectors); i++) { > + conn = &priv->connectors[i]; > + > + ret = drmm_connector_init(drm, conn, &dummy_connector_funcs, > + DRM_MODE_CONNECTOR_DSI, NULL); > + KUNIT_ASSERT_EQ(test, ret, 0); > + > + drm_connector_helper_add(conn, &dummy_connector_helper_funcs); > + drm_encoder_helper_add(&priv->encoders[i], &drm_test_encoder_funcs); > + > + drm_connector_attach_encoder(conn, &priv->encoders[i]); > + } > + > +done: > + drm_mode_config_reset(drm); > + > + return priv; > +} > + > +static int set_up_atomic_state(struct kunit *test, > + struct drm_atomic_check_priv *priv, > + struct drm_connector *connector, > + struct drm_modeset_acquire_ctx *ctx) > +{ > + struct drm_device *drm = &priv->drm; > + struct drm_crtc *crtc = priv->crtc; > + struct drm_atomic_state *state; > + struct drm_connector_state *conn_state; > + struct drm_crtc_state *crtc_state; > + int ret; > + > + state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state); > + > + if (connector) { > + conn_state = drm_atomic_get_connector_state(state, connector); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state); > + > + ret = drm_atomic_set_crtc_for_connector(conn_state, crtc); > + KUNIT_EXPECT_EQ(test, ret, 0); > + } > + > + crtc_state = drm_atomic_get_crtc_state(state, crtc); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state); > + > + ret = drm_atomic_set_mode_for_crtc(crtc_state, &drm_atomic_test_mode); > + KUNIT_EXPECT_EQ(test, ret, 0); > + > + crtc_state->enable = true; > + crtc_state->active = true; > + > + if (connector) { > + ret = drm_atomic_commit(state); > + KUNIT_ASSERT_EQ(test, ret, 0); > + } else { > + // dummy connector mask > + crtc_state->connector_mask = DRM_TEST_CONN_0; > + } > + > + return 0; > +} > + Please provide a description of what the test does and what it checks (and why). See how other tests are described. > +static void drm_test_modeset(struct kunit *test) > +{ > + struct drm_atomic_check_priv *priv; > + struct drm_modeset_acquire_ctx *ctx; > + struct drm_connector *old_conn, *new_conn; > + struct drm_atomic_state *state; > + struct drm_device *drm; > + struct drm_connector_state *new_conn_state, *old_conn_state; > + int ret, initial_modeset_count; > + > + priv = drm_test_init_drm_components(test, true); > + KUNIT_ASSERT_NOT_NULL(test, priv); > + > + drm = &priv->drm; > + old_conn = &priv->connectors[0]; > + new_conn = &priv->connectors[1]; > + > + ctx = drm_kunit_helper_acquire_ctx_alloc(test); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx); > + > + // first modeset to enable > + ret = set_up_atomic_state(test, priv, old_conn, ctx); > + KUNIT_ASSERT_EQ(test, ret, 0); > + > + state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state); > + > + new_conn_state = drm_atomic_get_connector_state(state, new_conn); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state); > + > + old_conn_state = drm_atomic_get_connector_state(state, old_conn); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state); > + > + ret = drm_atomic_set_crtc_for_connector(old_conn_state, NULL); > + KUNIT_EXPECT_EQ(test, ret, 0); > + > + ret = drm_atomic_set_crtc_for_connector(new_conn_state, priv->crtc); > + KUNIT_EXPECT_EQ(test, ret, 0); > + > + initial_modeset_count = modeset_counter; > + > + // modeset_disables is called as part of the atomic commit tail > + ret = drm_atomic_commit(state); > + KUNIT_ASSERT_EQ(test, ret, 0); > + KUNIT_ASSERT_EQ(test, modeset_counter, initial_modeset_count + 1); > +} > + > +static struct kunit_case drm_test_check_modeset_test[] = { > + KUNIT_CASE(drm_test_modeset), > + {} > +}; > + > +static struct kunit_suite drm_test_check_modeset_test_suite = { > + .name = "drm_validate_modeset", > + .test_cases = drm_test_check_modeset_test, > +}; > + > +kunit_test_suite(drm_test_check_modeset_test_suite); > + > +MODULE_AUTHOR("Jessica Zhang <quic_jesszhan@quicinc.com"); > +MODULE_DESCRIPTION("Test cases for the drm_atomic_helper functions"); > +MODULE_LICENSE("GPL"); > > -- > 2.34.1 >
On 12/10/2024 1:10 PM, Dmitry Baryshkov wrote: > On Mon, Dec 09, 2024 at 05:09:25PM -0800, Jessica Zhang wrote: >> Add a subtest to check that modeset is called when the connector is >> changed >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/tests/Makefile | 1 + >> drivers/gpu/drm/tests/drm_atomic_state_test.c | 242 ++++++++++++++++++++++++++ >> 2 files changed, 243 insertions(+) >> >> diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile >> index 56dab563abd7a7ee7c147bd6b4927e2436b82e1d..0109bcf7faa54993cce337f522eae78f0fa6ffcb 100644 >> --- a/drivers/gpu/drm/tests/Makefile >> +++ b/drivers/gpu/drm/tests/Makefile >> @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST_HELPERS) += \ >> drm_kunit_helpers.o >> >> obj-$(CONFIG_DRM_KUNIT_TEST) += \ >> + drm_atomic_state_test.o \ >> drm_buddy_test.o \ >> drm_cmdline_parser_test.o \ >> drm_connector_test.o \ >> diff --git a/drivers/gpu/drm/tests/drm_atomic_state_test.c b/drivers/gpu/drm/tests/drm_atomic_state_test.c >> new file mode 100644 >> index 0000000000000000000000000000000000000000..ffc1198ccc9ce103b9bc4c5ca8c83ca7f6ab19cc >> --- /dev/null >> +++ b/drivers/gpu/drm/tests/drm_atomic_state_test.c >> @@ -0,0 +1,242 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Test cases for the drm_atomic_state helpers >> + * >> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#include <drm/drm_atomic.h> >> +#include <drm/drm_atomic_helper.h> >> +#include <drm/drm_atomic_uapi.h> >> +#include <drm/drm_kunit_helpers.h> >> +#include <drm/drm_probe_helper.h> >> + >> +#define DRM_TEST_ENC_0 BIT(0) >> +#define DRM_TEST_ENC_1 BIT(1) >> +#define DRM_TEST_ENC_2 BIT(2) >> + >> +#define DRM_TEST_CONN_0 BIT(0) >> + >> +static const struct drm_display_mode drm_atomic_test_mode = { >> + DRM_MODE("1024x768", 0, 65000, 1024, 1048, >> + 1184, 1344, 0, 768, 771, 777, 806, 0, >> + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) >> +}; >> + >> +struct drm_atomic_check_priv { >> + struct drm_device drm; >> + struct drm_plane *plane; >> + struct drm_crtc *crtc; >> + struct drm_encoder encoders[3]; >> + struct drm_connector connectors[2]; >> +}; >> + >> +static int modeset_counter; >> + >> +static void drm_test_encoder_mode_set(struct drm_encoder *encoder, >> + struct drm_crtc_state *crtc_state, >> + struct drm_connector_state *conn_state) >> +{ >> + modeset_counter++; >> +} >> + >> +static const struct drm_encoder_helper_funcs drm_test_encoder_funcs = { > > Name is too generic, please use something more test-suite specific. Hey Dmitry, Thanks for the review, ack for all of these comments. - Jessica Zhang > >> + .atomic_mode_set = drm_test_encoder_mode_set, >> +}; >> + >> +static const struct drm_connector_funcs dummy_connector_funcs = { >> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, >> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, >> + .reset = drm_atomic_helper_connector_reset, >> +}; >> + >> +static int drm_atomic_test_dummy_get_modes(struct drm_connector *connector) >> +{ >> + drm_connector_helper_get_modes_fixed(connector, &drm_atomic_test_mode); >> + return 1; > > return drm_connector_helper_get_modes_fixed() > >> +} >> + >> +static const struct drm_connector_helper_funcs dummy_connector_helper_funcs = { >> + .get_modes = drm_atomic_test_dummy_get_modes, >> +}; >> + >> +static struct drm_atomic_check_priv * >> +drm_test_init_drm_components(struct kunit *test, bool has_connectors) > > I think this is too generic, please use a file-specific name. > >> +{ >> + struct drm_atomic_check_priv *priv; >> + struct drm_encoder *enc; >> + struct drm_device *drm; >> + struct device *dev; >> + int ret; >> + >> + dev = drm_kunit_helper_alloc_device(test); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); >> + >> + priv = drm_kunit_helper_alloc_drm_device(test, dev, >> + struct drm_atomic_check_priv, >> + drm, >> + DRIVER_MODESET | DRIVER_ATOMIC); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv); >> + test->priv = priv; >> + >> + drm = &priv->drm; >> + priv->plane = drm_kunit_helper_create_primary_plane(test, drm, >> + NULL, >> + NULL, >> + NULL, 0, >> + NULL); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->plane); >> + >> + priv->crtc = drm_kunit_helper_create_crtc(test, drm, >> + priv->plane, NULL, >> + NULL, >> + NULL); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->crtc); >> + >> + for (int i = 0; i < ARRAY_SIZE(priv->encoders); i++) { >> + enc = &priv->encoders[i]; >> + >> + ret = drmm_encoder_init(drm, enc, NULL, >> + DRM_MODE_ENCODER_DSI, NULL); >> + KUNIT_ASSERT_EQ(test, ret, 0); >> + >> + enc->possible_crtcs = drm_crtc_mask(priv->crtc); >> + } >> + >> + priv->encoders[0].possible_clones = DRM_TEST_ENC_0 | DRM_TEST_ENC_1; >> + priv->encoders[1].possible_clones = DRM_TEST_ENC_0 | DRM_TEST_ENC_1; >> + priv->encoders[2].possible_clones = DRM_TEST_ENC_2; >> + >> + if (!has_connectors) >> + goto done; >> + >> + struct drm_connector *conn; > > Please move next ot other variable definitions. > >> + >> + KUNIT_ASSERT_LE(test, ARRAY_SIZE(priv->connectors), >> + ARRAY_SIZE(priv->encoders)); > > I'd say it's BUILD_BUG_ON > >> + >> + >> + for (int i = 0; i < ARRAY_SIZE(priv->connectors); i++) { >> + conn = &priv->connectors[i]; >> + >> + ret = drmm_connector_init(drm, conn, &dummy_connector_funcs, >> + DRM_MODE_CONNECTOR_DSI, NULL); >> + KUNIT_ASSERT_EQ(test, ret, 0); >> + >> + drm_connector_helper_add(conn, &dummy_connector_helper_funcs); >> + drm_encoder_helper_add(&priv->encoders[i], &drm_test_encoder_funcs); >> + >> + drm_connector_attach_encoder(conn, &priv->encoders[i]); >> + } >> + >> +done: >> + drm_mode_config_reset(drm); >> + >> + return priv; >> +} >> + >> +static int set_up_atomic_state(struct kunit *test, >> + struct drm_atomic_check_priv *priv, >> + struct drm_connector *connector, >> + struct drm_modeset_acquire_ctx *ctx) >> +{ >> + struct drm_device *drm = &priv->drm; >> + struct drm_crtc *crtc = priv->crtc; >> + struct drm_atomic_state *state; >> + struct drm_connector_state *conn_state; >> + struct drm_crtc_state *crtc_state; >> + int ret; >> + >> + state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state); >> + >> + if (connector) { >> + conn_state = drm_atomic_get_connector_state(state, connector); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state); >> + >> + ret = drm_atomic_set_crtc_for_connector(conn_state, crtc); >> + KUNIT_EXPECT_EQ(test, ret, 0); >> + } >> + >> + crtc_state = drm_atomic_get_crtc_state(state, crtc); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state); >> + >> + ret = drm_atomic_set_mode_for_crtc(crtc_state, &drm_atomic_test_mode); >> + KUNIT_EXPECT_EQ(test, ret, 0); >> + >> + crtc_state->enable = true; >> + crtc_state->active = true; >> + >> + if (connector) { >> + ret = drm_atomic_commit(state); >> + KUNIT_ASSERT_EQ(test, ret, 0); >> + } else { >> + // dummy connector mask >> + crtc_state->connector_mask = DRM_TEST_CONN_0; >> + } >> + >> + return 0; >> +} >> + > > Please provide a description of what the test does and what it checks > (and why). See how other tests are described. > >> +static void drm_test_modeset(struct kunit *test) >> +{ >> + struct drm_atomic_check_priv *priv; >> + struct drm_modeset_acquire_ctx *ctx; >> + struct drm_connector *old_conn, *new_conn; >> + struct drm_atomic_state *state; >> + struct drm_device *drm; >> + struct drm_connector_state *new_conn_state, *old_conn_state; >> + int ret, initial_modeset_count; >> + >> + priv = drm_test_init_drm_components(test, true); >> + KUNIT_ASSERT_NOT_NULL(test, priv); >> + >> + drm = &priv->drm; >> + old_conn = &priv->connectors[0]; >> + new_conn = &priv->connectors[1]; >> + >> + ctx = drm_kunit_helper_acquire_ctx_alloc(test); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx); >> + >> + // first modeset to enable >> + ret = set_up_atomic_state(test, priv, old_conn, ctx); >> + KUNIT_ASSERT_EQ(test, ret, 0); >> + >> + state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state); >> + >> + new_conn_state = drm_atomic_get_connector_state(state, new_conn); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state); >> + >> + old_conn_state = drm_atomic_get_connector_state(state, old_conn); >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state); >> + >> + ret = drm_atomic_set_crtc_for_connector(old_conn_state, NULL); >> + KUNIT_EXPECT_EQ(test, ret, 0); >> + >> + ret = drm_atomic_set_crtc_for_connector(new_conn_state, priv->crtc); >> + KUNIT_EXPECT_EQ(test, ret, 0); >> + >> + initial_modeset_count = modeset_counter; >> + >> + // modeset_disables is called as part of the atomic commit tail >> + ret = drm_atomic_commit(state); >> + KUNIT_ASSERT_EQ(test, ret, 0); >> + KUNIT_ASSERT_EQ(test, modeset_counter, initial_modeset_count + 1); >> +} >> + >> +static struct kunit_case drm_test_check_modeset_test[] = { >> + KUNIT_CASE(drm_test_modeset), >> + {} >> +}; >> + >> +static struct kunit_suite drm_test_check_modeset_test_suite = { >> + .name = "drm_validate_modeset", >> + .test_cases = drm_test_check_modeset_test, >> +}; >> + >> +kunit_test_suite(drm_test_check_modeset_test_suite); >> + >> +MODULE_AUTHOR("Jessica Zhang <quic_jesszhan@quicinc.com"); >> +MODULE_DESCRIPTION("Test cases for the drm_atomic_helper functions"); >> +MODULE_LICENSE("GPL"); >> >> -- >> 2.34.1 >> > > -- > With best wishes > Dmitry
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile index 56dab563abd7a7ee7c147bd6b4927e2436b82e1d..0109bcf7faa54993cce337f522eae78f0fa6ffcb 100644 --- a/drivers/gpu/drm/tests/Makefile +++ b/drivers/gpu/drm/tests/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST_HELPERS) += \ drm_kunit_helpers.o obj-$(CONFIG_DRM_KUNIT_TEST) += \ + drm_atomic_state_test.o \ drm_buddy_test.o \ drm_cmdline_parser_test.o \ drm_connector_test.o \ diff --git a/drivers/gpu/drm/tests/drm_atomic_state_test.c b/drivers/gpu/drm/tests/drm_atomic_state_test.c new file mode 100644 index 0000000000000000000000000000000000000000..ffc1198ccc9ce103b9bc4c5ca8c83ca7f6ab19cc --- /dev/null +++ b/drivers/gpu/drm/tests/drm_atomic_state_test.c @@ -0,0 +1,242 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Test cases for the drm_atomic_state helpers + * + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_atomic_uapi.h> +#include <drm/drm_kunit_helpers.h> +#include <drm/drm_probe_helper.h> + +#define DRM_TEST_ENC_0 BIT(0) +#define DRM_TEST_ENC_1 BIT(1) +#define DRM_TEST_ENC_2 BIT(2) + +#define DRM_TEST_CONN_0 BIT(0) + +static const struct drm_display_mode drm_atomic_test_mode = { + DRM_MODE("1024x768", 0, 65000, 1024, 1048, + 1184, 1344, 0, 768, 771, 777, 806, 0, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) +}; + +struct drm_atomic_check_priv { + struct drm_device drm; + struct drm_plane *plane; + struct drm_crtc *crtc; + struct drm_encoder encoders[3]; + struct drm_connector connectors[2]; +}; + +static int modeset_counter; + +static void drm_test_encoder_mode_set(struct drm_encoder *encoder, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + modeset_counter++; +} + +static const struct drm_encoder_helper_funcs drm_test_encoder_funcs = { + .atomic_mode_set = drm_test_encoder_mode_set, +}; + +static const struct drm_connector_funcs dummy_connector_funcs = { + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .reset = drm_atomic_helper_connector_reset, +}; + +static int drm_atomic_test_dummy_get_modes(struct drm_connector *connector) +{ + drm_connector_helper_get_modes_fixed(connector, &drm_atomic_test_mode); + return 1; +} + +static const struct drm_connector_helper_funcs dummy_connector_helper_funcs = { + .get_modes = drm_atomic_test_dummy_get_modes, +}; + +static struct drm_atomic_check_priv * +drm_test_init_drm_components(struct kunit *test, bool has_connectors) +{ + struct drm_atomic_check_priv *priv; + struct drm_encoder *enc; + struct drm_device *drm; + struct device *dev; + int ret; + + dev = drm_kunit_helper_alloc_device(test); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); + + priv = drm_kunit_helper_alloc_drm_device(test, dev, + struct drm_atomic_check_priv, + drm, + DRIVER_MODESET | DRIVER_ATOMIC); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv); + test->priv = priv; + + drm = &priv->drm; + priv->plane = drm_kunit_helper_create_primary_plane(test, drm, + NULL, + NULL, + NULL, 0, + NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->plane); + + priv->crtc = drm_kunit_helper_create_crtc(test, drm, + priv->plane, NULL, + NULL, + NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->crtc); + + for (int i = 0; i < ARRAY_SIZE(priv->encoders); i++) { + enc = &priv->encoders[i]; + + ret = drmm_encoder_init(drm, enc, NULL, + DRM_MODE_ENCODER_DSI, NULL); + KUNIT_ASSERT_EQ(test, ret, 0); + + enc->possible_crtcs = drm_crtc_mask(priv->crtc); + } + + priv->encoders[0].possible_clones = DRM_TEST_ENC_0 | DRM_TEST_ENC_1; + priv->encoders[1].possible_clones = DRM_TEST_ENC_0 | DRM_TEST_ENC_1; + priv->encoders[2].possible_clones = DRM_TEST_ENC_2; + + if (!has_connectors) + goto done; + + struct drm_connector *conn; + + KUNIT_ASSERT_LE(test, ARRAY_SIZE(priv->connectors), + ARRAY_SIZE(priv->encoders)); + + + for (int i = 0; i < ARRAY_SIZE(priv->connectors); i++) { + conn = &priv->connectors[i]; + + ret = drmm_connector_init(drm, conn, &dummy_connector_funcs, + DRM_MODE_CONNECTOR_DSI, NULL); + KUNIT_ASSERT_EQ(test, ret, 0); + + drm_connector_helper_add(conn, &dummy_connector_helper_funcs); + drm_encoder_helper_add(&priv->encoders[i], &drm_test_encoder_funcs); + + drm_connector_attach_encoder(conn, &priv->encoders[i]); + } + +done: + drm_mode_config_reset(drm); + + return priv; +} + +static int set_up_atomic_state(struct kunit *test, + struct drm_atomic_check_priv *priv, + struct drm_connector *connector, + struct drm_modeset_acquire_ctx *ctx) +{ + struct drm_device *drm = &priv->drm; + struct drm_crtc *crtc = priv->crtc; + struct drm_atomic_state *state; + struct drm_connector_state *conn_state; + struct drm_crtc_state *crtc_state; + int ret; + + state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state); + + if (connector) { + conn_state = drm_atomic_get_connector_state(state, connector); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state); + + ret = drm_atomic_set_crtc_for_connector(conn_state, crtc); + KUNIT_EXPECT_EQ(test, ret, 0); + } + + crtc_state = drm_atomic_get_crtc_state(state, crtc); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state); + + ret = drm_atomic_set_mode_for_crtc(crtc_state, &drm_atomic_test_mode); + KUNIT_EXPECT_EQ(test, ret, 0); + + crtc_state->enable = true; + crtc_state->active = true; + + if (connector) { + ret = drm_atomic_commit(state); + KUNIT_ASSERT_EQ(test, ret, 0); + } else { + // dummy connector mask + crtc_state->connector_mask = DRM_TEST_CONN_0; + } + + return 0; +} + +static void drm_test_modeset(struct kunit *test) +{ + struct drm_atomic_check_priv *priv; + struct drm_modeset_acquire_ctx *ctx; + struct drm_connector *old_conn, *new_conn; + struct drm_atomic_state *state; + struct drm_device *drm; + struct drm_connector_state *new_conn_state, *old_conn_state; + int ret, initial_modeset_count; + + priv = drm_test_init_drm_components(test, true); + KUNIT_ASSERT_NOT_NULL(test, priv); + + drm = &priv->drm; + old_conn = &priv->connectors[0]; + new_conn = &priv->connectors[1]; + + ctx = drm_kunit_helper_acquire_ctx_alloc(test); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx); + + // first modeset to enable + ret = set_up_atomic_state(test, priv, old_conn, ctx); + KUNIT_ASSERT_EQ(test, ret, 0); + + state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state); + + new_conn_state = drm_atomic_get_connector_state(state, new_conn); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state); + + old_conn_state = drm_atomic_get_connector_state(state, old_conn); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state); + + ret = drm_atomic_set_crtc_for_connector(old_conn_state, NULL); + KUNIT_EXPECT_EQ(test, ret, 0); + + ret = drm_atomic_set_crtc_for_connector(new_conn_state, priv->crtc); + KUNIT_EXPECT_EQ(test, ret, 0); + + initial_modeset_count = modeset_counter; + + // modeset_disables is called as part of the atomic commit tail + ret = drm_atomic_commit(state); + KUNIT_ASSERT_EQ(test, ret, 0); + KUNIT_ASSERT_EQ(test, modeset_counter, initial_modeset_count + 1); +} + +static struct kunit_case drm_test_check_modeset_test[] = { + KUNIT_CASE(drm_test_modeset), + {} +}; + +static struct kunit_suite drm_test_check_modeset_test_suite = { + .name = "drm_validate_modeset", + .test_cases = drm_test_check_modeset_test, +}; + +kunit_test_suite(drm_test_check_modeset_test_suite); + +MODULE_AUTHOR("Jessica Zhang <quic_jesszhan@quicinc.com"); +MODULE_DESCRIPTION("Test cases for the drm_atomic_helper functions"); +MODULE_LICENSE("GPL");
Add a subtest to check that modeset is called when the connector is changed Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- drivers/gpu/drm/tests/Makefile | 1 + drivers/gpu/drm/tests/drm_atomic_state_test.c | 242 ++++++++++++++++++++++++++ 2 files changed, 243 insertions(+)