The device name isn't really useful, we can just define it instead of exposing it in the API.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/tests/drm_client_modeset_test.c | 2 +- drivers/gpu/drm/tests/drm_kunit_helpers.c | 6 ++++-- drivers/gpu/drm/tests/drm_kunit_helpers.h | 3 +-- 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c index e469d1634e2d..6920c3ffdfdf 100644 --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c @@ -41,7 +41,7 @@ static int drm_client_modeset_test_init(struct kunit *test)

test->priv = priv;

- priv->drm = drm_kunit_helper_alloc_drm_device(test, DRIVER_MODESET, "drm-client-modeset-test"); + priv->drm = drm_kunit_helper_alloc_drm_device(test, DRIVER_MODESET); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm);

ret = drmm_connector_init(priv->drm, &priv->connector, diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c index a4ad030ed101..7f69f56f5892 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c @@ -9,6 +9,8 @@

#include "drm_kunit_helpers.h"

+#define FAKE_DEVICE_NAME "drm-kunit-fake-device" + struct kunit_dev { struct drm_device base; }; @@ -38,7 +40,7 @@ static void dev_free(struct kunit_resource *res)

struct drm_device * drm_kunit_helper_alloc_drm_device(struct kunit *test, - u32 features, char *name) + u32 features) { struct kunit_dev *kdev; struct drm_device *drm; @@ -46,7 +48,7 @@ drm_kunit_helper_alloc_drm_device(struct kunit *test, struct device *dev; int ret;

- dev = kunit_alloc_resource(test, dev_init, dev_free, GFP_KERNEL, name); + dev = kunit_alloc_resource(test, dev_init, dev_free, GFP_KERNEL, FAKE_DEVICE_NAME); if (!dev) return ERR_PTR(-ENOMEM);

diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.h b/drivers/gpu/drm/tests/drm_kunit_helpers.h index e9870c7911fe..6c12b1426ba0 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.h +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.h @@ -8,7 +8,6 @@ struct kunit;

struct drm_device * drm_kunit_helper_alloc_drm_device(struct kunit *test, - u32 features, - char *name); + u32 features);

#endif // DRM_KUNIT_HELPERS_H_

The device managed resources are ran if the device has bus, which is not the case of a root_device.

Let's use a platform_device instead.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/tests/drm_kunit_helpers.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c index 2f974f45c5b4..651aa93a98a8 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c @@ -6,6 +6,7 @@ #include <kunit/resource.h>

#include <linux/device.h> +#include <linux/platform_device.h>

#include "drm_kunit_helpers.h"

@@ -20,12 +21,26 @@ static const struct drm_mode_config_funcs drm_mode_config_funcs = {

struct device *drm_kunit_helper_alloc_device(struct kunit *test) { - return root_device_register(FAKE_DEVICE_NAME); + struct platform_device *pdev; + int ret; + + ret = platform_driver_register(&fake_platform_driver); + KUNIT_ASSERT_EQ(test, ret, 0); + + pdev = platform_device_alloc(FAKE_DEVICE_NAME, PLATFORM_DEVID_NONE); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev); + + ret = platform_device_add(pdev); + KUNIT_ASSERT_EQ(test, ret, 0); + + return &pdev->dev; }

void drm_kunit_helper_free_device(struct kunit *test, struct device *dev) { - root_device_unregister(dev); + struct platform_device *pdev = to_platform_device(dev); + + platform_device_unregister(pdev); }

struct drm_device *

The current helper to allocate a DRM device doesn't allow for any subclassing by drivers, which is going to be troublesome as we work on getting some kunit testing on atomic modesetting code.

Let's use a similar pattern to the other allocation helpers by providing the structure size and offset as arguments.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/tests/drm_client_modeset_test.c | 4 +++- drivers/gpu/drm/tests/drm_kunit_helpers.c | 19 ++++++++----------- drivers/gpu/drm/tests/drm_kunit_helpers.h | 11 +++++++++-- 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c index fe95fda350ae..086469273580 100644 --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c @@ -45,7 +45,9 @@ static int drm_client_modeset_test_init(struct kunit *test) priv->dev = drm_kunit_helper_alloc_device(test); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev);

- priv->drm = drm_kunit_helper_alloc_drm_device(test, priv->dev, DRIVER_MODESET); + priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev, + sizeof(*priv->drm), 0, + DRIVER_MODESET); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm);

ret = drmm_connector_init(priv->drm, &priv->connector, diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c index 8bd7c40d0e1a..6d614f9e62d8 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c @@ -13,10 +13,6 @@ #define WQ_TIMEOUT_MS 100 #define FAKE_DEVICE_NAME "drm-kunit-fake-device"

-struct kunit_dev { - struct drm_device base; -}; - static const struct drm_mode_config_funcs drm_mode_config_funcs = { };

@@ -104,12 +100,13 @@ void drm_kunit_helper_free_device(struct kunit *test, struct device *dev) }

struct drm_device * -drm_kunit_helper_alloc_drm_device(struct kunit *test, struct device *dev, - u32 features) +__drm_kunit_helper_alloc_drm_device(struct kunit *test, struct device *dev, + size_t size, size_t offset, + u32 features) { - struct kunit_dev *kdev; struct drm_device *drm; struct drm_driver *driver; + void *container; int ret;

driver = kunit_kzalloc(test, sizeof(*driver), GFP_KERNEL); @@ -117,11 +114,11 @@ drm_kunit_helper_alloc_drm_device(struct kunit *test, struct device *dev, return ERR_PTR(-ENOMEM);

driver->driver_features = features; - kdev = devm_drm_dev_alloc(dev, driver, struct kunit_dev, base); - if (IS_ERR(kdev)) - return ERR_CAST(kdev); + container = __devm_drm_dev_alloc(dev, driver, size, offset); + if (IS_ERR(container)) + return ERR_CAST(container);

- drm = &kdev->base; + drm = container + offset; drm->mode_config.funcs = &drm_mode_config_funcs;

ret = drmm_mode_config_init(drm); diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.h b/drivers/gpu/drm/tests/drm_kunit_helpers.h index b4277fe92c38..70abc8c806c9 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.h +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.h @@ -10,7 +10,14 @@ struct device *drm_kunit_helper_alloc_device(struct kunit *test); void drm_kunit_helper_free_device(struct kunit *test, struct device *dev);

struct drm_device * -drm_kunit_helper_alloc_drm_device(struct kunit *test, struct device *dev, - u32 features); +__drm_kunit_helper_alloc_drm_device(struct kunit *test, struct device *dev, + size_t size, size_t offset, + u32 features); + +#define drm_kunit_helper_alloc_drm_device(_test, _dev, _type, _member, _feat) \ + ((_type *) __drm_kunit_helper_alloc_drm_device(_test, _dev, \ + sizeof(type), \ + offsetof(type, member), \ + _feat))

#endif // DRM_KUNIT_HELPERS_H_

DRM-managed actions are supposed to be ran whenever the device is released. Let's introduce a basic unit test to make sure it happens.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/tests/Makefile | 1 + drivers/gpu/drm/tests/drm_managed_test.c | 68 ++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+)

diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile index b29ef1085cad..35962c6ef0c4 100644 --- a/drivers/gpu/drm/tests/Makefile +++ b/drivers/gpu/drm/tests/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \ drm_format_test.o \ drm_framebuffer_test.o \ drm_kunit_helpers.o \ + drm_managed_test.o \ drm_mm_test.o \ drm_plane_helper_test.o \ drm_rect_test.o diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c new file mode 100644 index 000000000000..4fc11b289d9e --- /dev/null +++ b/drivers/gpu/drm/tests/drm_managed_test.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <drm/drm_drv.h> +#include <drm/drm_managed.h> + +#include <kunit/resource.h> + +#include <linux/device.h> + +#include "drm_kunit_helpers.h" + +#define TEST_TIMEOUT_MS 100 + +struct managed_test_priv { + bool action_done; + wait_queue_head_t action_wq; +}; + +static void drm_action(struct drm_device *drm, void *ptr) +{ + struct managed_test_priv *priv = ptr; + + priv->action_done = true; + wake_up_interruptible(&priv->action_wq); +} + +static void drm_test_managed_run_action(struct kunit *test) +{ + struct managed_test_priv *priv; + struct drm_device *drm; + struct device *dev; + int ret; + + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv); + init_waitqueue_head(&priv->action_wq); + + dev = drm_kunit_helper_alloc_device(test); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); + + drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_MODESET); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm); + + ret = drmm_add_action_or_reset(drm, drm_action, priv); + KUNIT_EXPECT_EQ(test, ret, 0); + + ret = drm_dev_register(drm, 0); + KUNIT_ASSERT_EQ(test, ret, 0); + + drm_dev_unregister(drm); + drm_kunit_helper_free_device(test, dev); + + ret = wait_event_interruptible_timeout(priv->action_wq, priv->action_done, + msecs_to_jiffies(TEST_TIMEOUT_MS)); + KUNIT_EXPECT_GT(test, ret, 0); +} + +static struct kunit_case drm_managed_tests[] = { + KUNIT_CASE(drm_test_managed_run_action), + {} +}; + +static struct kunit_suite drm_managed_test_suite = { + .name = "drm-test-managed", + .test_cases = drm_managed_tests +}; + +kunit_test_suite(drm_managed_test_suite);

On the vc4 devices (and later), the blending is done by a single device called the HVS. The HVS has three FIFO that can operate in parallel, and route their output to 6 CRTCs and 7 encoders on the BCM2711.

Each of these CRTCs and encoders have some contraints on which FIFO they can feed from, so we need some code to take all those constraints into account and assign FIFOs to CRTCs.

The problem can be simplified by assigning those FIFOs to CRTCs by ascending output index number. We had a comment mentioning it already, but we were never actually enforcing it.

It was working still in most situations because the probe order is roughly equivalent, except for the (optional, and fairly rarely used on the Pi4) VEC which was last in the probe order sequence, but one of the earliest device to assign.

This resulted in configurations that were rejected by our code but were still valid with a different assignment.

We can fix this by making sure we assign CRTCs to FIFOs by ordering them by ascending HVS output index.

Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically") Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_kms.c | 109 +++++++++++++++++++++++++++++------------- 1 file changed, 76 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 5c97642ed66a..17a28f9a2f31 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -12,6 +12,7 @@ */

#include <linux/clk.h> +#include <linux/sort.h>

#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> @@ -776,6 +777,20 @@ static int vc4_hvs_channels_obj_init(struct vc4_dev *vc4) return drmm_add_action_or_reset(&vc4->base, vc4_hvs_channels_obj_fini, NULL); }

+static int cmp_vc4_crtc_hvs_output(const void *a, const void *b) +{ + const struct vc4_crtc *crtc_a = + to_vc4_crtc(*(const struct drm_crtc **)a); + const struct vc4_crtc_data *data_a = + vc4_crtc_to_vc4_crtc_data(crtc_a); + const struct vc4_crtc *crtc_b = + to_vc4_crtc(*(const struct drm_crtc **)b); + const struct vc4_crtc_data *data_b = + vc4_crtc_to_vc4_crtc_data(crtc_b); + + return data_a->hvs_output - data_b->hvs_output; +} + /* * The BCM2711 HVS has up to 7 outputs connected to the pixelvalves and * the TXP (and therefore all the CRTCs found on that platform). @@ -810,10 +825,11 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { struct vc4_hvs_state *hvs_new_state; - struct drm_crtc_state *old_crtc_state, *new_crtc_state; + struct drm_crtc **sorted_crtcs; struct drm_crtc *crtc; unsigned int unassigned_channels = 0; unsigned int i; + int ret;

hvs_new_state = vc4_hvs_get_global_state(state); if (IS_ERR(hvs_new_state)) @@ -823,15 +839,59 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, if (!hvs_new_state->fifo_state[i].in_use) unassigned_channels |= BIT(i);

- for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { - struct vc4_crtc_state *old_vc4_crtc_state = - to_vc4_crtc_state(old_crtc_state); - struct vc4_crtc_state *new_vc4_crtc_state = - to_vc4_crtc_state(new_crtc_state); - struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); + /* + * The problem we have to solve here is that we have up to 7 + * encoders, connected to up to 6 CRTCs. + * + * Those CRTCs, depending on the instance, can be routed to 1, 2 + * or 3 HVS FIFOs, and we need to set the change the muxing + * between FIFOs and outputs in the HVS accordingly. + * + * It would be pretty hard to come up with an algorithm that + * would generically solve this. However, the current routing + * trees we support allow us to simplify a bit the problem. + * + * Indeed, with the current supported layouts, if we try to + * assign in the ascending crtc index order the FIFOs, we can't + * fall into the situation where an earlier CRTC that had + * multiple routes is assigned one that was the only option for + * a later CRTC. + * + * If the layout changes and doesn't give us that in the future, + * we will need to have something smarter, but it works so far. + */ + sorted_crtcs = kmalloc_array(dev->num_crtcs, sizeof(*sorted_crtcs), GFP_KERNEL); + if (!sorted_crtcs) + return -ENOMEM; + + i = 0; + drm_for_each_crtc(crtc, dev) + sorted_crtcs[i++] = crtc; + + sort(sorted_crtcs, i, sizeof(*sorted_crtcs), cmp_vc4_crtc_hvs_output, NULL); + + for (i = 0; i < dev->num_crtcs; i++) { + struct vc4_crtc_state *old_vc4_crtc_state, *new_vc4_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; + struct vc4_crtc *vc4_crtc; unsigned int matching_channels; unsigned int channel;

+ crtc = sorted_crtcs[i]; + if (!crtc) + continue; + vc4_crtc = to_vc4_crtc(crtc); + + old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc); + if (!old_crtc_state) + continue; + old_vc4_crtc_state = to_vc4_crtc_state(old_crtc_state); + + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + if (!new_crtc_state) + continue; + new_vc4_crtc_state = to_vc4_crtc_state(new_crtc_state); + drm_dbg(dev, "%s: Trying to find a channel.\n", crtc->name);

/* Nothing to do here, let's skip it */ @@ -860,33 +920,11 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, continue; }

- /* - * The problem we have to solve here is that we have - * up to 7 encoders, connected to up to 6 CRTCs. - * - * Those CRTCs, depending on the instance, can be - * routed to 1, 2 or 3 HVS FIFOs, and we need to set - * the change the muxing between FIFOs and outputs in - * the HVS accordingly. - * - * It would be pretty hard to come up with an - * algorithm that would generically solve - * this. However, the current routing trees we support - * allow us to simplify a bit the problem. - * - * Indeed, with the current supported layouts, if we - * try to assign in the ascending crtc index order the - * FIFOs, we can't fall into the situation where an - * earlier CRTC that had multiple routes is assigned - * one that was the only option for a later CRTC. - * - * If the layout changes and doesn't give us that in - * the future, we will need to have something smarter, - * but it works so far. - */ matching_channels = unassigned_channels & vc4_crtc->data->hvs_available_channels; - if (!matching_channels) - return -EINVAL; + if (!matching_channels) { + ret = -EINVAL; + goto err_free_crtc_array; + }

channel = ffs(matching_channels) - 1;

@@ -896,7 +934,12 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, hvs_new_state->fifo_state[channel].in_use = true; }

+ kfree(sorted_crtcs); return 0; + +err_free_crtc_array: + kfree(sorted_crtcs); + return ret; }

static int

In order to introduce unit tests for the HVS state computation, we'll need access to the vc4_hvs_state struct definition and its associated helpers.

Let's move them in our driver header.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_drv.h | 23 +++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_kms.c | 25 +++---------------------- 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 3ff56c1821ef..6ff63036915c 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -355,6 +355,29 @@ struct vc4_hvs { bool vc5_hdmi_enable_4096by2160; };

+#define HVS_NUM_CHANNELS 3 + +struct vc4_hvs_state { + struct drm_private_state base; + unsigned long core_clock_rate; + + struct { + unsigned in_use: 1; + unsigned long fifo_load; + struct drm_crtc_commit *pending_commit; + } fifo_state[HVS_NUM_CHANNELS]; +}; + +static inline struct vc4_hvs_state * +to_vc4_hvs_state(const struct drm_private_state *priv) +{ + return container_of(priv, struct vc4_hvs_state, base); +} + +struct vc4_hvs_state *vc4_hvs_get_global_state(struct drm_atomic_state *state); +struct vc4_hvs_state *vc4_hvs_get_old_global_state(struct drm_atomic_state *state); +struct vc4_hvs_state *vc4_hvs_get_new_global_state(struct drm_atomic_state *state); + struct vc4_plane { struct drm_plane base; }; diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 17a28f9a2f31..cb2197622b2b 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -25,8 +25,6 @@ #include "vc4_drv.h" #include "vc4_regs.h"

-#define HVS_NUM_CHANNELS 3 - struct vc4_ctm_state { struct drm_private_state base; struct drm_color_ctm *ctm; @@ -39,23 +37,6 @@ to_vc4_ctm_state(const struct drm_private_state *priv) return container_of(priv, struct vc4_ctm_state, base); }

-struct vc4_hvs_state { - struct drm_private_state base; - unsigned long core_clock_rate; - - struct { - unsigned in_use: 1; - unsigned long fifo_load; - struct drm_crtc_commit *pending_commit; - } fifo_state[HVS_NUM_CHANNELS]; -}; - -static struct vc4_hvs_state * -to_vc4_hvs_state(const struct drm_private_state *priv) -{ - return container_of(priv, struct vc4_hvs_state, base); -} - struct vc4_load_tracker_state { struct drm_private_state base; u64 hvs_load; @@ -191,7 +172,7 @@ vc4_ctm_commit(struct vc4_dev *vc4, struct drm_atomic_state *state) VC4_SET_FIELD(ctm_state->fifo, SCALER_OLEDOFFS_DISPFIFO)); }

-static struct vc4_hvs_state * +struct vc4_hvs_state * vc4_hvs_get_new_global_state(struct drm_atomic_state *state) { struct vc4_dev *vc4 = to_vc4_dev(state->dev); @@ -204,7 +185,7 @@ vc4_hvs_get_new_global_state(struct drm_atomic_state *state) return to_vc4_hvs_state(priv_state); }

-static struct vc4_hvs_state * +struct vc4_hvs_state * vc4_hvs_get_old_global_state(struct drm_atomic_state *state) { struct vc4_dev *vc4 = to_vc4_dev(state->dev); @@ -217,7 +198,7 @@ vc4_hvs_get_old_global_state(struct drm_atomic_state *state) return to_vc4_hvs_state(priv_state); }

-static struct vc4_hvs_state * +struct vc4_hvs_state * vc4_hvs_get_global_state(struct drm_atomic_state *state) { struct vc4_dev *vc4 = to_vc4_dev(state->dev);

The current order of variable assignments is unneccessarily complex, let's make it simpler.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_txp.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index bd181b5a7b52..b00c6fae972f 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -490,7 +490,6 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) struct drm_device *drm = dev_get_drvdata(master); struct vc4_crtc *vc4_crtc; struct vc4_txp *txp; - struct drm_crtc *crtc; struct drm_encoder *encoder; int ret, irq;

@@ -501,18 +500,16 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) txp = drmm_kzalloc(drm, sizeof(*txp), GFP_KERNEL); if (!txp) return -ENOMEM; - vc4_crtc = &txp->base; - crtc = &vc4_crtc->base; - - vc4_crtc->pdev = pdev; - vc4_crtc->data = &vc4_txp_crtc_data; - vc4_crtc->feeds_txp = true;

txp->pdev = pdev; - txp->regs = vc4_ioremap_regs(pdev, 0); if (IS_ERR(txp->regs)) return PTR_ERR(txp->regs); + + vc4_crtc = &txp->base; + vc4_crtc->pdev = pdev; + vc4_crtc->data = &vc4_txp_crtc_data; + vc4_crtc->feeds_txp = true; vc4_crtc->regset.base = txp->regs; vc4_crtc->regset.regs = txp_regs; vc4_crtc->regset.nregs = ARRAY_SIZE(txp_regs); @@ -533,7 +530,7 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) return ret;

encoder = &txp->connector.encoder; - encoder->possible_crtcs = drm_crtc_mask(crtc); + encoder->possible_crtcs = drm_crtc_mask(&vc4_crtc->base);

ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0, dev_name(dev), txp);

It makes more sense to register the CRTC before the encoder and connectors, so let's move our call around.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_txp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index 0bb8e97d7891..c2a6bea6fd96 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -516,6 +516,11 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) vc4_crtc->regset.regs = txp_regs; vc4_crtc->regset.nregs = ARRAY_SIZE(txp_regs);

+ ret = vc4_crtc_init(drm, vc4_crtc, + &vc4_txp_crtc_funcs, &vc4_txp_crtc_helper_funcs); + if (ret) + return ret; + vc4_encoder = &txp->encoder; txp->encoder.type = VC4_ENCODER_TYPE_TXP;

@@ -537,11 +542,6 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) if (ret) return ret;

- ret = vc4_crtc_init(drm, vc4_crtc, - &vc4_txp_crtc_funcs, &vc4_txp_crtc_helper_funcs); - if (ret) - return ret; - ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0, dev_name(dev), txp); if (ret)

The current vc4_crtc_init() helper assumes that we will be using hardware planes and calls vc4_plane_init().

While it's a reasonable assumption, we'll want to mock the plane and thus provide our own. Let's create a helper that will take the plane as an argument.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 52 +++++++++++++++++++++++++++--------------- drivers/gpu/drm/vc4/vc4_drv.h | 6 +++++ 2 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 1ea190dffe87..340c39921bce 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -1278,31 +1278,20 @@ static void vc4_set_crtc_possible_masks(struct drm_device *drm, } }

-int vc4_crtc_init(struct drm_device *drm, struct platform_device *pdev, - struct vc4_crtc *vc4_crtc, - const struct vc4_crtc_data *data, - const struct drm_crtc_funcs *crtc_funcs, - const struct drm_crtc_helper_funcs *crtc_helper_funcs, - bool feeds_txp) +int __vc4_crtc_init(struct drm_device *drm, + struct platform_device *pdev, + struct vc4_crtc *vc4_crtc, + const struct vc4_crtc_data *data, + struct drm_plane *primary_plane, + const struct drm_crtc_funcs *crtc_funcs, + const struct drm_crtc_helper_funcs *crtc_helper_funcs, + bool feeds_txp) { struct vc4_dev *vc4 = to_vc4_dev(drm); struct drm_crtc *crtc = &vc4_crtc->base; - struct drm_plane *primary_plane; unsigned int i; int ret;

- /* For now, we create just the primary and the legacy cursor - * planes. We should be able to stack more planes on easily, - * but to do that we would need to compute the bandwidth - * requirement of the plane configuration, and reject ones - * that will take too much. - */ - primary_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_PRIMARY, 0); - if (IS_ERR(primary_plane)) { - dev_err(drm->dev, "failed to construct primary plane\n"); - return PTR_ERR(primary_plane); - } - vc4_crtc->data = data; vc4_crtc->pdev = pdev; vc4_crtc->feeds_txp = feeds_txp; @@ -1334,6 +1323,31 @@ int vc4_crtc_init(struct drm_device *drm, struct platform_device *pdev, return 0; }

+int vc4_crtc_init(struct drm_device *drm, struct platform_device *pdev, + struct vc4_crtc *vc4_crtc, + const struct vc4_crtc_data *data, + const struct drm_crtc_funcs *crtc_funcs, + const struct drm_crtc_helper_funcs *crtc_helper_funcs, + bool feeds_txp) +{ + struct drm_plane *primary_plane; + + /* For now, we create just the primary and the legacy cursor + * planes. We should be able to stack more planes on easily, + * but to do that we would need to compute the bandwidth + * requirement of the plane configuration, and reject ones + * that will take too much. + */ + primary_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_PRIMARY, 0); + if (IS_ERR(primary_plane)) { + dev_err(drm->dev, "failed to construct primary plane\n"); + return PTR_ERR(primary_plane); + } + + return __vc4_crtc_init(drm, pdev, vc4_crtc, data, primary_plane, + crtc_funcs, crtc_helper_funcs, feeds_txp); +} + static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) { struct platform_device *pdev = to_platform_device(dev); diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 599d24f1f55a..01ca2b25d2e3 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -886,6 +886,12 @@ int vc4_bo_debugfs_init(struct drm_minor *minor); /* vc4_crtc.c */ extern struct platform_driver vc4_crtc_driver; int vc4_crtc_disable_at_boot(struct drm_crtc *crtc); +int __vc4_crtc_init(struct drm_device *drm, struct platform_device *pdev, + struct vc4_crtc *vc4_crtc, const struct vc4_crtc_data *data, + struct drm_plane *primary_plane, + const struct drm_crtc_funcs *crtc_funcs, + const struct drm_crtc_helper_funcs *crtc_helper_funcs, + bool feeds_txp); int vc4_crtc_init(struct drm_device *drm, struct platform_device *pdev, struct vc4_crtc *vc4_crtc, const struct vc4_crtc_data *data, const struct drm_crtc_funcs *crtc_funcs,

It's fairly hard to figure out the instance of the CRTC affected by an atomic change using the default name.

Since we can provide our own to the CRTC initialization functions, let's do so to make the debugging sessions easier.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 10 +++++++++- drivers/gpu/drm/vc4/vc4_drv.h | 2 ++ drivers/gpu/drm/vc4/vc4_txp.c | 1 + 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 8bc30ad0904b..59e473059fa2 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -1118,6 +1118,7 @@ static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = {

static const struct vc4_pv_data bcm2835_pv0_data = { .base = { + .name = "pixelvalve-0", .debugfs_name = "crtc0_regs", .hvs_available_channels = BIT(0), .hvs_output = 0, @@ -1132,6 +1133,7 @@ static const struct vc4_pv_data bcm2835_pv0_data = {

static const struct vc4_pv_data bcm2835_pv1_data = { .base = { + .name = "pixelvalve-1", .debugfs_name = "crtc1_regs", .hvs_available_channels = BIT(2), .hvs_output = 2, @@ -1146,6 +1148,7 @@ static const struct vc4_pv_data bcm2835_pv1_data = {

static const struct vc4_pv_data bcm2835_pv2_data = { .base = { + .name = "pixelvalve-2", .debugfs_name = "crtc2_regs", .hvs_available_channels = BIT(1), .hvs_output = 1, @@ -1160,6 +1163,7 @@ static const struct vc4_pv_data bcm2835_pv2_data = {

static const struct vc4_pv_data bcm2711_pv0_data = { .base = { + .name = "pixelvalve-0", .debugfs_name = "crtc0_regs", .hvs_available_channels = BIT(0), .hvs_output = 0, @@ -1174,6 +1178,7 @@ static const struct vc4_pv_data bcm2711_pv0_data = {

static const struct vc4_pv_data bcm2711_pv1_data = { .base = { + .name = "pixelvalve-1", .debugfs_name = "crtc1_regs", .hvs_available_channels = BIT(0) | BIT(1) | BIT(2), .hvs_output = 3, @@ -1188,6 +1193,7 @@ static const struct vc4_pv_data bcm2711_pv1_data = {

static const struct vc4_pv_data bcm2711_pv2_data = { .base = { + .name = "pixelvalve-2", .debugfs_name = "crtc2_regs", .hvs_available_channels = BIT(0) | BIT(1) | BIT(2), .hvs_output = 4, @@ -1201,6 +1207,7 @@ static const struct vc4_pv_data bcm2711_pv2_data = {

static const struct vc4_pv_data bcm2711_pv3_data = { .base = { + .name = "pixelvalve-3", .debugfs_name = "crtc3_regs", .hvs_available_channels = BIT(1), .hvs_output = 1, @@ -1214,6 +1221,7 @@ static const struct vc4_pv_data bcm2711_pv3_data = {

static const struct vc4_pv_data bcm2711_pv4_data = { .base = { + .name = "pixelvalve-4", .debugfs_name = "crtc4_regs", .hvs_available_channels = BIT(0) | BIT(1) | BIT(2), .hvs_output = 5, @@ -1282,7 +1290,7 @@ int __vc4_crtc_init(struct drm_device *drm, vc4_crtc->feeds_txp = feeds_txp; spin_lock_init(&vc4_crtc->irq_lock); ret = drmm_crtc_init_with_planes(drm, crtc, primary_plane, NULL, - crtc_funcs, NULL); + crtc_funcs, data->name); if (ret) return ret;

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index dca7be3fccb5..54352db48476 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -512,6 +512,8 @@ struct drm_encoder *vc4_find_encoder_by_type(struct drm_device *drm, }

struct vc4_crtc_data { + const char *name; + const char *debugfs_name;

/* Bitmask of channels (FIFOs) of the HVS that the output can source from */ diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index 841da240d93a..4f7ce5d3e8ad 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -480,6 +480,7 @@ static irqreturn_t vc4_txp_interrupt(int irq, void *data) }

static const struct vc4_crtc_data vc4_txp_crtc_data = { + .name = "txp", .debugfs_name = "txp_regs", .hvs_available_channels = BIT(2), .hvs_output = 2,

In order to test the current atomic_check hooks we need to have a DRM device that has roughly the same capabilities and layout that the actual hardware. We'll also need a bunch of functions to create arbitrary atomic states.

Let's create some helpers to create a device that behaves like the real one, and some helpers to maintain the atomic state we want to check.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/Kconfig | 15 +++ drivers/gpu/drm/vc4/Makefile | 1 + drivers/gpu/drm/vc4/tests/.kunitconfig | 14 +++ drivers/gpu/drm/vc4/tests/Makefile | 7 ++ drivers/gpu/drm/vc4/tests/vc4_mock.c | 174 ++++++++++++++++++++++++++++ drivers/gpu/drm/vc4/tests/vc4_mock.h | 58 ++++++++++ drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c | 39 +++++++ drivers/gpu/drm/vc4/tests/vc4_mock_output.c | 97 ++++++++++++++++ drivers/gpu/drm/vc4/tests/vc4_mock_plane.c | 45 +++++++ drivers/gpu/drm/vc4/vc4_crtc.c | 20 ++-- drivers/gpu/drm/vc4/vc4_drv.c | 4 +- drivers/gpu/drm/vc4/vc4_drv.h | 13 +++ drivers/gpu/drm/vc4/vc4_txp.c | 2 +- 13 files changed, 476 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig index 246305d17a52..eb5e8367da67 100644 --- a/drivers/gpu/drm/vc4/Kconfig +++ b/drivers/gpu/drm/vc4/Kconfig @@ -34,3 +34,18 @@ config DRM_VC4_HDMI_CEC help Choose this option if you have a Broadcom VC4 GPU and want to use CEC. + +config DRM_VC4_KUNIT_TEST + tristate "KUnit tests for VC4" if !KUNIT_ALL_TESTS + depends on DRM_VC4 && KUNIT + default KUNIT_ALL_TESTS + help + This builds unit tests for the VC4 DRM/KMS driver. This option is + not useful for distributions or general kernels, but only for kernel + developers working on the VC4 driver. + + For more information on KUnit and unit tests in general, + please refer to the KUnit documentation in + Documentation/dev-tools/kunit/. + + If in doubt, say "N". diff --git a/drivers/gpu/drm/vc4/Makefile b/drivers/gpu/drm/vc4/Makefile index d0163e18e9ca..8984b2b5da77 100644 --- a/drivers/gpu/drm/vc4/Makefile +++ b/drivers/gpu/drm/vc4/Makefile @@ -28,3 +28,4 @@ vc4-y := \ vc4-$(CONFIG_DEBUG_FS) += vc4_debugfs.o

obj-$(CONFIG_DRM_VC4) += vc4.o +obj-$(CONFIG_DRM_VC4_KUNIT_TEST) += tests/ diff --git a/drivers/gpu/drm/vc4/tests/.kunitconfig b/drivers/gpu/drm/vc4/tests/.kunitconfig new file mode 100644 index 000000000000..bfd9899ed6e0 --- /dev/null +++ b/drivers/gpu/drm/vc4/tests/.kunitconfig @@ -0,0 +1,14 @@ +CONFIG_ARCH_BCM=y +CONFIG_ARCH_BCM2835=y +CONFIG_BCM2835_MBOX=y +CONFIG_KUNIT=y +CONFIG_DRM=y +CONFIG_DRM_KUNIT_TEST=y +CONFIG_DRM_VC4=y +CONFIG_DRM_VC4_KUNIT_TEST=y +CONFIG_MAILBOX=y +CONFIG_RASPBERRYPI_FIRMWARE=y +CONFIG_SND=y +CONFIG_SND_SOC=y +CONFIG_SOUND=y +CONFIG_COMMON_CLK=y diff --git a/drivers/gpu/drm/vc4/tests/Makefile b/drivers/gpu/drm/vc4/tests/Makefile new file mode 100644 index 000000000000..3e8a7b4f9017 --- /dev/null +++ b/drivers/gpu/drm/vc4/tests/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0 + +obj-$(CONFIG_DRM_VC4_KUNIT_TEST) += \ + vc4_mock.o \ + vc4_mock_crtc.o \ + vc4_mock_output.o \ + vc4_mock_plane.o diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock.c b/drivers/gpu/drm/vc4/tests/vc4_mock.c new file mode 100644 index 000000000000..f4c2213ea0e2 --- /dev/null +++ b/drivers/gpu/drm/vc4/tests/vc4_mock.c @@ -0,0 +1,174 @@ +#include <drm/drm_drv.h> + +#include <kunit/test.h> + +#include "../../tests/drm_kunit_helpers.h" + +#include "vc4_mock.h" + +struct vc4_mock_output_desc { + enum vc4_encoder_type vc4_encoder_type; + unsigned int encoder_type; + unsigned int connector_type; +}; + +#define VC4_MOCK_OUTPUT_DESC(_vc4_type, _etype, _ctype) \ + { \ + .vc4_encoder_type = _vc4_type, \ + .encoder_type = _etype, \ + .connector_type = _ctype, \ + } + +struct vc4_mock_pipe_desc { + const struct vc4_crtc_data *data; + const struct vc4_mock_output_desc *outputs; + unsigned int noutputs; +}; + +#define VC4_MOCK_CRTC_DESC(_data, ...) \ + { \ + .data = _data, \ + .outputs = (struct vc4_mock_output_desc[]) { __VA_ARGS__ }, \ + .noutputs = sizeof((struct vc4_mock_output_desc[]) { __VA_ARGS__ }) / \ + sizeof(struct vc4_mock_output_desc), \ + } + +#define VC4_MOCK_PIXELVALVE_DESC(_data, ...) \ + VC4_MOCK_CRTC_DESC(&(_data)->base, __VA_ARGS__) + +struct vc4_mock_desc { + const struct vc4_mock_pipe_desc *pipes; + unsigned int npipes; +}; + +#define VC4_MOCK_DESC(...) \ + { \ + .pipes = (struct vc4_mock_pipe_desc[]) { __VA_ARGS__ }, \ + .npipes = sizeof((struct vc4_mock_pipe_desc[]) { __VA_ARGS__ }) / \ + sizeof(struct vc4_mock_pipe_desc), \ + } + +static const struct vc4_mock_desc vc5_mock = + VC4_MOCK_DESC( + VC4_MOCK_CRTC_DESC(&vc4_txp_crtc_data, + VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_TXP, + DRM_MODE_ENCODER_VIRTUAL, + DRM_MODE_CONNECTOR_WRITEBACK)), + VC4_MOCK_PIXELVALVE_DESC(&bcm2711_pv0_data, + VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_DSI0, + DRM_MODE_ENCODER_DSI, + DRM_MODE_CONNECTOR_DSI), + VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_DPI, + DRM_MODE_ENCODER_DPI, + DRM_MODE_CONNECTOR_DPI)), + VC4_MOCK_PIXELVALVE_DESC(&bcm2711_pv1_data, + VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_DSI1, + DRM_MODE_ENCODER_DSI, + DRM_MODE_CONNECTOR_DSI)), + VC4_MOCK_PIXELVALVE_DESC(&bcm2711_pv2_data, + VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_HDMI0, + DRM_MODE_ENCODER_TMDS, + DRM_MODE_CONNECTOR_HDMIA)), + VC4_MOCK_PIXELVALVE_DESC(&bcm2711_pv3_data, + VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_VEC, + DRM_MODE_ENCODER_TVDAC, + DRM_MODE_CONNECTOR_Composite)), + VC4_MOCK_PIXELVALVE_DESC(&bcm2711_pv4_data, + VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_HDMI1, + DRM_MODE_ENCODER_TMDS, + DRM_MODE_CONNECTOR_HDMIA)), +); + +static int __build_one_pipe(struct kunit *test, struct drm_device *drm, + const struct vc4_mock_pipe_desc *pipe) +{ + struct vc4_dummy_plane *dummy_plane; + struct drm_plane *plane; + struct vc4_dummy_crtc *dummy_crtc; + struct drm_crtc *crtc; + unsigned int i; + + dummy_plane = vc4_dummy_plane(test, drm, DRM_PLANE_TYPE_PRIMARY); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_plane); + + plane = &dummy_plane->plane.base; + dummy_crtc = vc4_mock_pv(test, drm, plane, pipe->data); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_crtc); + + crtc = &dummy_crtc->crtc.base; + for (i = 0; i < pipe->noutputs; i++) { + const struct vc4_mock_output_desc *mock_output = &pipe->outputs[i]; + struct vc4_dummy_output *dummy_output; + + dummy_output = vc4_dummy_output(test, drm, crtc, + mock_output->vc4_encoder_type, + mock_output->encoder_type, + mock_output->connector_type); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_output); + } + + return 0; +} + +static int __build_mock(struct kunit *test, struct drm_device *drm, + const struct vc4_mock_desc *mock) +{ + + unsigned int i; + + for (i = 0; i < mock->npipes; i++) { + const struct vc4_mock_pipe_desc *pipe = &mock->pipes[i]; + int ret; + + ret = __build_one_pipe(test, drm, pipe); + KUNIT_ASSERT_EQ(test, ret, 0); + } + + return 0; +} + +static struct vc4_dev *__mock_device(struct kunit *test, bool is_vc5) +{ + struct drm_device *drm; + const struct drm_driver *drv = is_vc5 ? &vc5_drm_driver : &vc4_drm_driver; + const struct vc4_mock_desc *desc = is_vc5 ? &vc5_mock : NULL; + struct vc4_dev *vc4; + struct device *dev; + int ret; + + dev = drm_kunit_helper_alloc_device(test); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); + + vc4 = drm_kunit_helper_alloc_drm_device_with_driver(test, dev, + struct vc4_dev, base, + drv); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4); + + vc4->dev = dev; + vc4->is_vc5 = is_vc5; + + vc4->hvs = __vc4_hvs_alloc(vc4, NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4->hvs); + + drm = &vc4->base; + ret = __build_mock(test, drm, desc); + KUNIT_ASSERT_EQ(test, ret, 0); + + ret = vc4_kms_load(drm); + KUNIT_ASSERT_EQ(test, ret, 0); + + ret = drm_dev_register(drm, 0); + KUNIT_ASSERT_EQ(test, ret, 0); + + return vc4; +} + +struct vc4_dev *vc4_mock_device(struct kunit *test) +{ + return __mock_device(test, false); +} + +struct vc4_dev *vc5_mock_device(struct kunit *test) +{ + return __mock_device(test, true); +} diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock.h b/drivers/gpu/drm/vc4/tests/vc4_mock.h new file mode 100644 index 000000000000..144af2ed6c40 --- /dev/null +++ b/drivers/gpu/drm/vc4/tests/vc4_mock.h @@ -0,0 +1,58 @@ +#ifndef VC4_MOCK_H_ +#define VC4_MOCK_H_ + +#include "../vc4_drv.h" + +static inline +struct drm_crtc *vc4_find_crtc_for_encoder(struct kunit *test, + struct drm_device *drm, + struct drm_encoder *encoder) +{ + struct drm_crtc *crtc; + + KUNIT_ASSERT_EQ(test, hweight32(encoder->possible_crtcs), 1); + + drm_for_each_crtc(crtc, drm) + if (encoder->possible_crtcs & drm_crtc_mask(crtc)) + return crtc; + + return NULL; +} + +struct vc4_dummy_plane { + struct vc4_plane plane; +}; + +struct vc4_dummy_plane *vc4_dummy_plane(struct kunit *test, + struct drm_device *drm, + enum drm_plane_type type); + +struct vc4_dummy_crtc { + struct vc4_crtc crtc; +}; + +struct vc4_dummy_crtc *vc4_mock_pv(struct kunit *test, + struct drm_device *drm, + struct drm_plane *plane, + const struct vc4_crtc_data *data); + +struct vc4_dummy_output { + struct vc4_encoder encoder; + struct drm_connector connector; +}; + +struct vc4_dummy_output *vc4_dummy_output(struct kunit *test, + struct drm_device *drm, + struct drm_crtc *crtc, + enum vc4_encoder_type vc4_encoder_type, + unsigned int kms_encoder_type, + unsigned int connector_type); + +struct vc4_dev *vc4_mock_device(struct kunit *test); +struct vc4_dev *vc5_mock_device(struct kunit *test); + +int vc4_mock_atomic_add_output(struct kunit *test, struct drm_device *drm, + enum vc4_encoder_type type, + struct drm_atomic_state *state); + +#endif // VC4_MOCK_H_ diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c new file mode 100644 index 000000000000..213622f9fba2 --- /dev/null +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c @@ -0,0 +1,39 @@ +#include <drm/drm_atomic_state_helper.h> +#include <drm/drm_modeset_helper_vtables.h> + +#include <kunit/test.h> + +#include "vc4_mock.h" + +static const struct drm_crtc_helper_funcs vc4_dummy_crtc_helper_funcs = { + .atomic_check = vc4_crtc_atomic_check, +}; + +static const struct drm_crtc_funcs vc4_dummy_crtc_funcs = { + .atomic_destroy_state = vc4_crtc_destroy_state, + .atomic_duplicate_state = vc4_crtc_duplicate_state, + .reset = vc4_crtc_reset, +}; + +struct vc4_dummy_crtc *vc4_mock_pv(struct kunit *test, + struct drm_device *drm, + struct drm_plane *plane, + const struct vc4_crtc_data *data) +{ + struct vc4_dummy_crtc *dummy_crtc; + struct vc4_crtc *vc4_crtc; + int ret; + + dummy_crtc = kunit_kzalloc(test, sizeof(*dummy_crtc), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, dummy_crtc); + + vc4_crtc = &dummy_crtc->crtc; + ret = __vc4_crtc_init(drm, NULL, + vc4_crtc, data, plane, + &vc4_dummy_crtc_funcs, + &vc4_dummy_crtc_helper_funcs, + false); + KUNIT_ASSERT_EQ(test, ret, 0); + + return dummy_crtc; +} diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c new file mode 100644 index 000000000000..18bd088530fe --- /dev/null +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c @@ -0,0 +1,97 @@ +#include <drm/drm_atomic_state_helper.h> +#include <drm/drm_atomic_uapi.h> +#include <drm/drm_connector.h> +#include <drm/drm_crtc.h> +#include <drm/drm_encoder.h> +#include <drm/drm_modeset_helper_vtables.h> + +#include <kunit/test.h> + +#include "vc4_mock.h" + +static const struct drm_connector_helper_funcs vc4_dummy_connector_helper_funcs = { +}; + +static const struct drm_connector_funcs vc4_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, +}; + +struct vc4_dummy_output *vc4_dummy_output(struct kunit *test, + struct drm_device *drm, + struct drm_crtc *crtc, + enum vc4_encoder_type vc4_encoder_type, + unsigned int kms_encoder_type, + unsigned int connector_type) +{ + struct vc4_dummy_output *dummy_output; + struct drm_connector *conn; + struct drm_encoder *enc; + int ret; + + dummy_output = kunit_kzalloc(test, sizeof(*dummy_output), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_output); + dummy_output->encoder.type = vc4_encoder_type; + + enc = &dummy_output->encoder.base; + ret = drmm_encoder_init(drm, enc, + NULL, + kms_encoder_type, + NULL); + KUNIT_ASSERT_EQ(test, ret, 0); + enc->possible_crtcs = drm_crtc_mask(crtc); + + conn = &dummy_output->connector; + ret = drmm_connector_init(drm, conn, + &vc4_dummy_connector_funcs, + connector_type, + NULL); + KUNIT_ASSERT_EQ(test, ret, 0); + + drm_connector_helper_add(conn, &vc4_dummy_connector_helper_funcs); + drm_connector_attach_encoder(conn, enc); + + return dummy_output; +} + +static const struct drm_display_mode default_mode = { + DRM_SIMPLE_MODE(640, 480, 64, 48) +}; + +int vc4_mock_atomic_add_output(struct kunit *test, struct drm_device *drm, + enum vc4_encoder_type type, + struct drm_atomic_state *state) +{ + struct vc4_dummy_output *output; + struct drm_connector *conn; + struct drm_connector_state *conn_state; + struct drm_encoder *encoder; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int ret; + + encoder = vc4_find_encoder_by_type(drm, type); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder); + + crtc = vc4_find_crtc_for_encoder(test, drm, encoder); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc); + + output = container_of(encoder, struct vc4_dummy_output, encoder.base); + conn = &output->connector; + conn_state = drm_atomic_get_connector_state(state, conn); + 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, &default_mode); + KUNIT_EXPECT_EQ(test, ret, 0); + + crtc_state->active = true; + + return 0; +} diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_plane.c b/drivers/gpu/drm/vc4/tests/vc4_mock_plane.c new file mode 100644 index 000000000000..106d3bd53aa5 --- /dev/null +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_plane.c @@ -0,0 +1,45 @@ +#include <drm/drm_atomic_state_helper.h> +#include <drm/drm_fourcc.h> +#include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_plane.h> + +#include <kunit/test.h> + +#include "vc4_mock.h" + +static const struct drm_plane_helper_funcs vc4_dummy_plane_helper_funcs = { +}; + +static const struct drm_plane_funcs vc4_dummy_plane_funcs = { + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, + .reset = drm_atomic_helper_plane_reset, +}; + +static const uint32_t vc4_dummy_plane_formats[] = { + DRM_FORMAT_XRGB8888, +}; + +struct vc4_dummy_plane *vc4_dummy_plane(struct kunit *test, + struct drm_device *drm, + enum drm_plane_type type) +{ + struct vc4_dummy_plane *dummy_plane; + struct drm_plane *plane; + + dummy_plane = drmm_universal_plane_alloc(drm, + struct vc4_dummy_plane, plane.base, + 0, + &vc4_dummy_plane_funcs, + vc4_dummy_plane_formats, + ARRAY_SIZE(vc4_dummy_plane_formats), + NULL, + DRM_PLANE_TYPE_PRIMARY, + NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_plane); + + plane = &dummy_plane->plane.base; + drm_plane_helper_add(plane, &vc4_dummy_plane_helper_funcs); + + return dummy_plane; +} diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 59e473059fa2..21b40d644ace 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -675,8 +675,8 @@ void vc4_crtc_get_margins(struct drm_crtc_state *state, } }

-static int vc4_crtc_atomic_check(struct drm_crtc *crtc, - struct drm_atomic_state *state) +int vc4_crtc_atomic_check(struct drm_crtc *crtc, + struct drm_atomic_state *state) { struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc); @@ -1116,7 +1116,7 @@ static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = { .get_scanout_position = vc4_crtc_get_scanout_position, };

-static const struct vc4_pv_data bcm2835_pv0_data = { +const struct vc4_pv_data bcm2835_pv0_data = { .base = { .name = "pixelvalve-0", .debugfs_name = "crtc0_regs", @@ -1131,7 +1131,7 @@ static const struct vc4_pv_data bcm2835_pv0_data = { }, };

-static const struct vc4_pv_data bcm2835_pv1_data = { +const struct vc4_pv_data bcm2835_pv1_data = { .base = { .name = "pixelvalve-1", .debugfs_name = "crtc1_regs", @@ -1146,7 +1146,7 @@ static const struct vc4_pv_data bcm2835_pv1_data = { }, };

-static const struct vc4_pv_data bcm2835_pv2_data = { +const struct vc4_pv_data bcm2835_pv2_data = { .base = { .name = "pixelvalve-2", .debugfs_name = "crtc2_regs", @@ -1161,7 +1161,7 @@ static const struct vc4_pv_data bcm2835_pv2_data = { }, };

-static const struct vc4_pv_data bcm2711_pv0_data = { +const struct vc4_pv_data bcm2711_pv0_data = { .base = { .name = "pixelvalve-0", .debugfs_name = "crtc0_regs", @@ -1176,7 +1176,7 @@ static const struct vc4_pv_data bcm2711_pv0_data = { }, };

-static const struct vc4_pv_data bcm2711_pv1_data = { +const struct vc4_pv_data bcm2711_pv1_data = { .base = { .name = "pixelvalve-1", .debugfs_name = "crtc1_regs", @@ -1191,7 +1191,7 @@ static const struct vc4_pv_data bcm2711_pv1_data = { }, };

-static const struct vc4_pv_data bcm2711_pv2_data = { +const struct vc4_pv_data bcm2711_pv2_data = { .base = { .name = "pixelvalve-2", .debugfs_name = "crtc2_regs", @@ -1205,7 +1205,7 @@ static const struct vc4_pv_data bcm2711_pv2_data = { }, };

-static const struct vc4_pv_data bcm2711_pv3_data = { +const struct vc4_pv_data bcm2711_pv3_data = { .base = { .name = "pixelvalve-3", .debugfs_name = "crtc3_regs", @@ -1219,7 +1219,7 @@ static const struct vc4_pv_data bcm2711_pv3_data = { }, };

-static const struct vc4_pv_data bcm2711_pv4_data = { +const struct vc4_pv_data bcm2711_pv4_data = { .base = { .name = "pixelvalve-4", .debugfs_name = "crtc4_regs", diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index b66bf7aea632..af6a83ac8526 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -196,7 +196,7 @@ static const struct drm_ioctl_desc vc4_drm_ioctls[] = { DRM_IOCTL_DEF_DRV(VC4_PERFMON_GET_VALUES, vc4_perfmon_get_values_ioctl, DRM_RENDER_ALLOW), };

-static const struct drm_driver vc4_drm_driver = { +const struct drm_driver vc4_drm_driver = { .driver_features = (DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM | @@ -225,7 +225,7 @@ static const struct drm_driver vc4_drm_driver = { .patchlevel = DRIVER_PATCHLEVEL, };

-static const struct drm_driver vc5_drm_driver = { +const struct drm_driver vc5_drm_driver = { .driver_features = (DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM), diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index e0be7a81a24a..e2a56c32a124 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -24,6 +24,9 @@ struct drm_device; struct drm_gem_object;

+extern const struct drm_driver vc4_drm_driver; +extern const struct drm_driver vc5_drm_driver; + /* Don't forget to update vc4_bo.c: bo_type_names[] when adding to * this. */ @@ -523,6 +526,8 @@ struct vc4_crtc_data { int hvs_output; };

+extern const struct vc4_crtc_data vc4_txp_crtc_data; + struct vc4_pv_data { struct vc4_crtc_data base;

@@ -535,6 +540,12 @@ struct vc4_pv_data { enum vc4_encoder_type encoder_types[4]; };

+extern const struct vc4_pv_data bcm2711_pv0_data; +extern const struct vc4_pv_data bcm2711_pv1_data; +extern const struct vc4_pv_data bcm2711_pv2_data; +extern const struct vc4_pv_data bcm2711_pv3_data; +extern const struct vc4_pv_data bcm2711_pv4_data; + struct vc4_crtc { struct drm_crtc base; struct platform_device *pdev; @@ -920,6 +931,8 @@ int vc4_page_flip(struct drm_crtc *crtc, struct drm_pending_vblank_event *event, uint32_t flags, struct drm_modeset_acquire_ctx *ctx); +int vc4_crtc_atomic_check(struct drm_crtc *crtc, + struct drm_atomic_state *state); struct drm_crtc_state *vc4_crtc_duplicate_state(struct drm_crtc *crtc); void vc4_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state); diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index 4f7ce5d3e8ad..2b69454b8534 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -479,7 +479,7 @@ static irqreturn_t vc4_txp_interrupt(int irq, void *data) return IRQ_HANDLED; }

-static const struct vc4_crtc_data vc4_txp_crtc_data = { +const struct vc4_crtc_data vc4_txp_crtc_data = { .name = "txp", .debugfs_name = "txp_regs", .hvs_available_channels = BIT(2),

The HVS to PixelValve muxing code is fairly error prone and has a bunch of arbitrary constraints due to the hardware setup.

Let's create a test suite that makes sure that the possible combinations work and the invalid ones don't.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/tests/Makefile | 3 +- drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 624 +++++++++++++++++++++++++ 2 files changed, 626 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/tests/Makefile b/drivers/gpu/drm/vc4/tests/Makefile index 3e8a7b4f9017..1ae143c415a4 100644 --- a/drivers/gpu/drm/vc4/tests/Makefile +++ b/drivers/gpu/drm/vc4/tests/Makefile @@ -4,4 +4,5 @@ obj-$(CONFIG_DRM_VC4_KUNIT_TEST) += \ vc4_mock.o \ vc4_mock_crtc.o \ vc4_mock_output.o \ - vc4_mock_plane.o + vc4_mock_plane.o \ + vc4_test_pv_muxing.o diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c new file mode 100644 index 000000000000..3047fc768c09 --- /dev/null +++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c @@ -0,0 +1,624 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_state_helper.h> +#include <drm/drm_atomic_uapi.h> +#include <drm/drm_crtc.h> +#include <drm/drm_drv.h> +#include <drm/drm_fourcc.h> +#include <drm/drm_mode.h> +#include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_plane.h> + +#include <kunit/test.h> + +#include "../../tests/drm_kunit_helpers.h" + +#include "../vc4_drv.h" + +#include "vc4_mock.h" + +struct pv_muxing_priv { + struct vc4_dev *vc4; + struct drm_modeset_acquire_ctx ctx; + struct drm_atomic_state *state; +}; + +static bool check_fifo_conflict(struct kunit *test, + const struct drm_atomic_state *state) +{ + struct vc4_hvs_state *hvs_state; + unsigned int used_fifos = 0; + unsigned int i; + + hvs_state = vc4_hvs_get_new_global_state(state); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, hvs_state); + + for (i = 0; i < HVS_NUM_CHANNELS; i++) { + if (!hvs_state->fifo_state[i].in_use) + continue; + + KUNIT_EXPECT_FALSE(test, used_fifos & BIT(i)); + used_fifos |= BIT(i); + } + + return true; +} + +struct encoder_constraint { + enum vc4_encoder_type type; + unsigned int *channels; + size_t nchannels; +}; + +#define ENCODER_CONSTRAINT(_type, ...) \ + { \ + .type = _type, \ + .channels = (unsigned int[]) { __VA_ARGS__ }, \ + .nchannels = sizeof((unsigned int[]) { __VA_ARGS__ }) / \ + sizeof(unsigned int), \ + } + +static bool __check_encoder_constraints(const struct encoder_constraint *constraints, + size_t nconstraints, + enum vc4_encoder_type type, + unsigned int channel) +{ + unsigned int i; + + for (i = 0; i < nconstraints; i++) { + const struct encoder_constraint *constraint = &constraints[i]; + unsigned int j; + + if (constraint->type != type) + continue; + + for (j = 0; j < constraint->nchannels; j++) { + unsigned int _channel = constraint->channels[j]; + + if (channel != _channel) + continue; + + return true; + } + } + + return false; +} + +static const struct encoder_constraint vc5_encoder_constraints[] = { + ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_DPI, 0), + ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_DSI0, 0), + ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_VEC, 1), + ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_TXP, 0, 2), + ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_DSI1, 0, 1, 2), + ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_HDMI0, 0, 1, 2), + ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_HDMI1, 0, 1, 2), +}; + +static bool check_vc5_encoder_constraints(enum vc4_encoder_type type, unsigned int channel) +{ + return __check_encoder_constraints(vc5_encoder_constraints, + ARRAY_SIZE(vc5_encoder_constraints), + type, channel); +} + +static bool check_channel_for_encoder(struct kunit *test, + const struct drm_atomic_state *state, + enum vc4_encoder_type type) +{ + struct drm_device *drm = state->dev; + struct drm_encoder *encoder; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + struct vc4_crtc_state *new_vc4_crtc_state; + struct vc4_hvs_state *new_hvs_state; + unsigned int channel; + + new_hvs_state = vc4_hvs_get_new_global_state(state); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_hvs_state); + + encoder = vc4_find_encoder_by_type(drm, type); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder); + + crtc = vc4_find_crtc_for_encoder(test, drm, encoder); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc); + + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state); + + new_vc4_crtc_state = to_vc4_crtc_state(crtc_state); + channel = new_vc4_crtc_state->assigned_channel; + KUNIT_EXPECT_NE(test, channel, VC4_HVS_CHANNEL_DISABLED); + + KUNIT_EXPECT_TRUE(test, new_hvs_state->fifo_state[channel].in_use); + + KUNIT_EXPECT_TRUE(test, + check_vc5_encoder_constraints(type, + channel)); + + return true; +} + +struct pv_muxing_param { + const char *name; + enum vc4_encoder_type *encoders; + size_t nencoders; +}; + +#define PV_MUXING_TEST(_name, ...) \ + { \ + .name = _name, \ + .encoders = (enum vc4_encoder_type[]) { __VA_ARGS__ }, \ + .nencoders = sizeof((enum vc4_encoder_type[]) { __VA_ARGS__ }) / \ + sizeof(enum vc4_encoder_type), \ + } + +static const struct pv_muxing_param vc5_test_pv_muxing_params[] = { + PV_MUXING_TEST("1 output: DPI", + VC4_ENCODER_TYPE_DPI), + PV_MUXING_TEST("1 output: DSI0", + VC4_ENCODER_TYPE_DSI0), + PV_MUXING_TEST("1 output: DSI1", + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("1 output: HDMI0", + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("1 output: HDMI1", + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("1 output: VEC", + VC4_ENCODER_TYPE_VEC), + PV_MUXING_TEST("2 output: DPI, DSI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("2 output: DPI, HDMI0", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("2 output: DPI, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("2 output: DPI, TXP", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_TXP), + PV_MUXING_TEST("2 output: DPI, VEC", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC), + PV_MUXING_TEST("2 output: DPI, DSI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("2 output: DSI0, DSI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("2 output: DSI0, HDMI0", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("2 output: DSI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("2 output: DSI0, TXP", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_TXP), + PV_MUXING_TEST("2 output: DSI0, VEC", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC), + PV_MUXING_TEST("2 output: DSI0, DSI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("2 output: DSI1, VEC", + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_VEC), + PV_MUXING_TEST("2 output: DSI1, TXP", + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_TXP), + PV_MUXING_TEST("2 output: DSI1, HDMI0", + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("2 output: DSI1, HDMI1", + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("2 output: HDMI0, VEC", + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_VEC), + PV_MUXING_TEST("2 output: HDMI0, TXP", + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_TXP), + PV_MUXING_TEST("2 output: HDMI0, HDMI1", + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("2 output: HDMI1, VEC", + VC4_ENCODER_TYPE_HDMI1, + VC4_ENCODER_TYPE_VEC), + PV_MUXING_TEST("2 output: HDMI1, TXP", + VC4_ENCODER_TYPE_HDMI1, + VC4_ENCODER_TYPE_TXP), + PV_MUXING_TEST("2 output: TXP, VEC", + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_VEC), + PV_MUXING_TEST("3 output: DPI, VEC, TXP", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP), + PV_MUXING_TEST("3 output: DPI, VEC, DSI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("3 output: DPI, VEC, HDMI0", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("3 output: DPI, VEC, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("3 output: DPI, TXP, DSI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("3 output: DPI, TXP, HDMI0", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("3 output: DPI, TXP, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("3 output: DPI, DSI1, HDMI0", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("3 output: DPI, DSI1, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("3 output: DPI, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("3 output: DSI0, VEC, TXP", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP), + PV_MUXING_TEST("3 output: DSI0, VEC, DSI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("3 output: DSI0, VEC, HDMI0", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("3 output: DSI0, VEC, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("3 output: DSI0, TXP, DSI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("3 output: DSI0, TXP, HDMI0", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("3 output: DSI0, TXP, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("3 output: DSI0, DSI1, HDMI0", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("3 output: DSI0, DSI1, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("3 output: DSI0, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), +}; + +static void vc4_test_pv_muxing_desc(const struct pv_muxing_param *t, char *desc) +{ + strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE); +} + +KUNIT_ARRAY_PARAM(vc5_test_pv_muxing, + vc5_test_pv_muxing_params, + vc4_test_pv_muxing_desc); + +static const struct pv_muxing_param vc5_test_pv_muxing_invalid_params[] = { + PV_MUXING_TEST("DPI/DSI0 Conflict", + VC4_ENCODER_TYPE_DPI, VC4_ENCODER_TYPE_DSI0), + PV_MUXING_TEST("More than 3 output: DPI, VEC, TXP, DSI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("More than 3 output: DPI, VEC, TXP, HDMI0", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("More than 3 output: DPI, VEC, TXP, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, VEC, DSI1, HDMI0", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("More than 3 output: DPI, VEC, DSI1, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, VEC, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, TXP, DSI1, HDMI0", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("More than 3 output: DPI, TXP, DSI1, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, TXP, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, VEC, TXP, DSI1, HDMI0", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("More than 3 output: DPI, VEC, TXP, DSI1, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, VEC, TXP, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, VEC, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, TXP, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, TXP, DSI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, TXP, HDMI0", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, TXP, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, DSI1, HDMI0", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, DSI1, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, TXP, DSI1, HDMI0", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("More than 3 output: DSI0, TXP, DSI1, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, TXP, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, TXP, DSI1, HDMI0", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, TXP, DSI1, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, TXP, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, TXP, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: VEC, TXP, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DPI, VEC, TXP, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DPI, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), + PV_MUXING_TEST("More than 3 output: DSI0, VEC, TXP, DSI1, HDMI0, HDMI1", + VC4_ENCODER_TYPE_DSI0, + VC4_ENCODER_TYPE_VEC, + VC4_ENCODER_TYPE_TXP, + VC4_ENCODER_TYPE_DSI1, + VC4_ENCODER_TYPE_HDMI0, + VC4_ENCODER_TYPE_HDMI1), +}; + +KUNIT_ARRAY_PARAM(vc5_test_pv_muxing_invalid, + vc5_test_pv_muxing_invalid_params, + vc4_test_pv_muxing_desc); + +static void drm_vc5_test_pv_muxing(struct kunit *test) +{ + const struct pv_muxing_priv *priv = test->priv; + const struct pv_muxing_param *params = test->param_value; + struct vc4_dev *vc4 = priv->vc4; + struct drm_device *drm = &vc4->base; + struct drm_atomic_state *state = priv->state; + unsigned int i; + int ret; + + for (i = 0; i < params->nencoders; i++) { + enum vc4_encoder_type enc_type = params->encoders[i]; + + ret = vc4_mock_atomic_add_output(test, drm, enc_type, state); + KUNIT_ASSERT_EQ(test, ret, 0); + } + + ret = drm_atomic_check_only(state); + KUNIT_EXPECT_EQ(test, ret, 0); + + KUNIT_EXPECT_TRUE(test, + check_fifo_conflict(test, state)); + + for (i = 0; i < params->nencoders; i++) { + enum vc4_encoder_type enc_type = params->encoders[i]; + + KUNIT_EXPECT_TRUE(test, check_channel_for_encoder(test, state, enc_type)); + } +} + +static void drm_vc4_test_pv_muxing_invalid(struct kunit *test) +{ + const struct pv_muxing_priv *priv = test->priv; + const struct pv_muxing_param *params = test->param_value; + struct vc4_dev *vc4 = priv->vc4; + struct drm_device *drm = &vc4->base; + struct drm_atomic_state *state = priv->state; + unsigned int i; + int ret; + + for (i = 0; i < params->nencoders; i++) { + enum vc4_encoder_type enc_type = params->encoders[i]; + + ret = vc4_mock_atomic_add_output(test, drm, enc_type, state); + KUNIT_ASSERT_EQ(test, ret, 0); + } + + ret = drm_atomic_check_only(state); + KUNIT_EXPECT_LT(test, ret, 0); +} + +static int vc5_pv_muxing_test_init(struct kunit *test) +{ + struct drm_atomic_state *state; + struct pv_muxing_priv *priv; + struct drm_device *drm; + struct vc4_dev *vc4; + + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, priv); + test->priv = priv; + + vc4 = vc5_mock_device(test); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4); + priv->vc4 = vc4; + + drm_modeset_acquire_init(&priv->ctx, 0); + + drm = &vc4->base; + state = drm_atomic_state_alloc(drm); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state); + + state->acquire_ctx = &priv->ctx; + + priv->state = state; + + return 0; +} + +static void vc4_pv_muxing_test_exit(struct kunit *test) +{ + struct pv_muxing_priv *priv = test->priv; + struct vc4_dev *vc4 = priv->vc4; + struct drm_device *drm = &vc4->base; + struct drm_atomic_state *state = priv->state; + + drm_atomic_state_put(state); + drm_modeset_drop_locks(&priv->ctx); + drm_modeset_acquire_fini(&priv->ctx); + drm_dev_unregister(drm); + drm_kunit_helper_free_device(test, vc4->dev); +} + +static struct kunit_case vc5_pv_muxing_tests[] = { + KUNIT_CASE_PARAM(drm_vc5_test_pv_muxing, + vc5_test_pv_muxing_gen_params), + KUNIT_CASE_PARAM(drm_vc4_test_pv_muxing_invalid, + vc5_test_pv_muxing_invalid_gen_params), + {} +}; + +static struct kunit_suite vc5_pv_muxing_test_suite = { + .name = "vc5-pv-muxing", + .init = vc5_pv_muxing_test_init, + .exit = vc4_pv_muxing_test_exit, + .test_cases = vc5_pv_muxing_tests, +}; + +kunit_test_suite(vc5_pv_muxing_test_suite);

On 11/23/22 16:25, Maxime Ripard wrote:

The device name isn't really useful, we can just define it instead of exposing it in the API.

Signed-off-by: Maxime Ripard maxime@cerno.tech

drivers/gpu/drm/tests/drm_client_modeset_test.c | 2 +- drivers/gpu/drm/tests/drm_kunit_helpers.c | 6 ++++-- drivers/gpu/drm/tests/drm_kunit_helpers.h | 3 +-- 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c index e469d1634e2d..6920c3ffdfdf 100644 --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c @@ -41,7 +41,7 @@ static int drm_client_modeset_test_init(struct kunit *test) test->priv = priv;

• priv->drm = drm_kunit_helper_alloc_drm_device(test, DRIVER_MODESET, "drm-client-modeset-test");

• priv->drm = drm_kunit_helper_alloc_drm_device(test, DRIVER_MODESET); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm);

ret = drmm_connector_init(priv->drm, &priv->connector, diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c index a4ad030ed101..7f69f56f5892 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c @@ -9,6 +9,8 @@ #include "drm_kunit_helpers.h" +#define FAKE_DEVICE_NAME "drm-kunit-fake-device"

I think that KUNIT_DEVICE_NAME and "drm-kunit-mock-device" are more descriptive names here. But I'm also OK with the patch as is.

Reviewed-by: Javier Martinez Canillas javierm@redhat.com

On 11/23/22 16:25, Maxime Ripard wrote:

The device managed resources are ran if the device has bus, which is not the case of a root_device.

Let's use a platform_device instead.

Signed-off-by: Maxime Ripard maxime@cerno.tech

drivers/gpu/drm/tests/drm_kunit_helpers.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c index 2f974f45c5b4..651aa93a98a8 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c @@ -6,6 +6,7 @@ #include <kunit/resource.h> #include <linux/device.h> +#include <linux/platform_device.h> #include "drm_kunit_helpers.h" @@ -20,12 +21,26 @@ static const struct drm_mode_config_funcs drm_mode_config_funcs = { struct device *drm_kunit_helper_alloc_device(struct kunit *test) {

• return root_device_register(FAKE_DEVICE_NAME);

• struct platform_device *pdev;
• int ret;
• ret = platform_driver_register(&fake_platform_driver);

Maybe the platform_driver_register() is something that should be done in the drm_client_modeset_test_init() callback ?

I would also as a part of this series rename the kunit_suite from "drm_test_pick_cmdline_test_suite" to "drm_test_modeset_test_suite" or something like that. Since you are already setting the .init and .exit callbacks to allocate and release the devices. So it is more than just a test suite for cmdline params testing.

• KUNIT_ASSERT_EQ(test, ret, 0);
• pdev = platform_device_alloc(FAKE_DEVICE_NAME, PLATFORM_DEVID_NONE);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
• ret = platform_device_add(pdev);
• KUNIT_ASSERT_EQ(test, ret, 0);
• return &pdev->dev;

}

Probably won't do it as a part of this series to avoid making it cross subsystem, but I think there's room to further consolidation since this helper seems to be quite generic and something that would be useful for other subsystems. So it would make sense for kunit to have helpers to manage platform drivers and devices.

In fact, I see that drivers/base/test/test_async_driver_probe.c already has similar helpers that could be exposed to the rest of the kernel.

The patch looks good to me though and any of these changes could be done as a follow-up:

Reviewed-by: Javier Martinez Canillas javierm@redhat.com

On 11/23/22 16:25, Maxime Ripard wrote:

DRM-managed actions are supposed to be ran whenever the device is released. Let's introduce a basic unit test to make sure it happens.

Signed-off-by: Maxime Ripard maxime@cerno.tech

drivers/gpu/drm/tests/Makefile | 1 + drivers/gpu/drm/tests/drm_managed_test.c | 68 ++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+)

diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile index b29ef1085cad..35962c6ef0c4 100644 --- a/drivers/gpu/drm/tests/Makefile +++ b/drivers/gpu/drm/tests/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \ drm_format_test.o \ drm_framebuffer_test.o \ drm_kunit_helpers.o \

• drm_managed_test.o \ drm_mm_test.o \ drm_plane_helper_test.o \ drm_rect_test.o

diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c new file mode 100644 index 000000000000..4fc11b289d9e --- /dev/null +++ b/drivers/gpu/drm/tests/drm_managed_test.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0

+#include <drm/drm_drv.h> +#include <drm/drm_managed.h>

+#include <kunit/resource.h>

+#include <linux/device.h>

+#include "drm_kunit_helpers.h"

+#define TEST_TIMEOUT_MS 100

Why did you choose this timeout value? Maybe we can add some comments explaining the rationale.

Reviewed-by: Javier Martinez Canillas javierm@redhat.com

On 11/23/22 16:26, Maxime Ripard wrote:

It's fairly hard to figure out the instance of the CRTC affected by an atomic change using the default name.

Since we can provide our own to the CRTC initialization functions, let's do so to make the debugging sessions easier.

Signed-off-by: Maxime Ripard maxime@cerno.tech

drivers/gpu/drm/vc4/vc4_crtc.c | 10 +++++++++- drivers/gpu/drm/vc4/vc4_drv.h | 2 ++ drivers/gpu/drm/vc4/vc4_txp.c | 1 + 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 8bc30ad0904b..59e473059fa2 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -1118,6 +1118,7 @@ static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = { static const struct vc4_pv_data bcm2835_pv0_data = { .base = {

• .name = "pixelvalve-0",
  

I wonder if would make sense to add the SoC name too, but either way:

Reviewed-by: Javier Martinez Canillas javierm@redhat.com