Re: [PATCH v3 20/20] Documentation: gpu: vc4: Add KUnit Tests Section

Re: [PATCH v3 20/20] Documentation: gpu: vc4: Add KUnit Tests Section

[Maíra Canal]

On 12/1/22 12:11, Maxime Ripard wrote:
> Now that we have VC4-specific tests in place, let's document them
> properly.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Just a small nit below.

> ---
>   Documentation/gpu/vc4.rst | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/gpu/vc4.rst b/Documentation/gpu/vc4.rst
> index 5df1d98b9544..a2375f1584e6 100644
> --- a/Documentation/gpu/vc4.rst
> +++ b/Documentation/gpu/vc4.rst
> @@ -54,6 +54,22 @@ VEC (Composite TV out) encoder
>   .. kernel-doc:: drivers/gpu/drm/vc4/vc4_vec.c
>      :doc: VC4 SDTV module
>   
> +KUnit Tests
> +===========
> +
> +The VC4 Driver uses KUnit to perform driver-specific unit and
> +integration tests.
> +
> +These tests are using a mock driver and can be ran using the
> +command::
> +	./tools/testing/kunit/kunit.py run \
> +		--kunitconfig=drivers/gpu/drm/vc4/tests/.kunitconfig \
> +		--cross_compile aarch64-linux-gnu- --arch arm64

I believe you could use a code block here, like:

.. code-block:: bash
	
	$ ./tools/testing/kunit/kunit.py run \
		--kunitconfig=drivers/gpu/drm/vc4/tests/.kunitconfig \
		--cross_compile aarch64-linux-gnu- --arch arm64

You could also mention that you can run the tests with --arch arm.

Best Regards,
- Maíra Canal

> +
> +Parts of the driver that are currently covered by tests are:
> + * The HVS to PixelValve dynamic FIFO assignment, for the BCM2835-7
> +   and BCM2711.
> +
>   Memory Management and 3D Command Submission
>   ===========================================
>   
> 

Re: [PATCH v3 02/20] drm/tests: Introduce a config option for the KUnit helpers

[Maíra Canal]

On 12/1/22 12:11, Maxime Ripard wrote:
> Driver-specific tests will need access to the helpers without pulling
> every DRM framework test. Let's create an intermediate Kconfig options
> for the helpers.
> 
> Suggested-by: Maíra Canal <mcanal@igalia.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Best Regards,
- Maíra Canal

> ---
>   drivers/gpu/drm/Kconfig        | 7 +++++++
>   drivers/gpu/drm/Makefile       | 2 +-
>   drivers/gpu/drm/tests/Makefile | 4 +++-
>   3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 315cbdf61979..9f019cd053e1 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -63,6 +63,12 @@ config DRM_USE_DYNAMIC_DEBUG
>   	  bytes per callsite, the .data costs can be substantial, and
>   	  are therefore configurable.
>   
> +config DRM_KUNIT_TEST_HELPERS
> +	tristate
> +	depends on DRM && KUNIT
> +	help
> +	  KUnit Helpers for KMS drivers.
> +
>   config DRM_KUNIT_TEST
>   	tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS
>   	depends on DRM && KUNIT
> @@ -73,6 +79,7 @@ config DRM_KUNIT_TEST
>   	select DRM_KMS_HELPER
>   	select DRM_BUDDY
>   	select DRM_EXPORT_FOR_TESTS if m
> +	select DRM_KUNIT_TEST_HELPERS
>   	default KUNIT_ALL_TESTS
>   	help
>   	  This builds unit tests for DRM. This option is not useful for
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index f92cd7892711..8d61fbdfdfac 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -125,7 +125,7 @@ obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>   # Drivers and the rest
>   #
>   
> -obj-$(CONFIG_DRM_KUNIT_TEST) += tests/
> +obj-y			+= tests/
>   
>   obj-$(CONFIG_DRM_MIPI_DBI) += drm_mipi_dbi.o
>   obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
> index 94fe546d937d..ef14bd481139 100644
> --- a/drivers/gpu/drm/tests/Makefile
> +++ b/drivers/gpu/drm/tests/Makefile
> @@ -1,5 +1,8 @@
>   # SPDX-License-Identifier: GPL-2.0
>   
> +obj-$(CONFIG_DRM_KUNIT_TEST_HELPERS) += \
> +	drm_kunit_helpers.o
> +
>   obj-$(CONFIG_DRM_KUNIT_TEST) += \
>   	drm_buddy_test.o \
>   	drm_cmdline_parser_test.o \
> @@ -9,7 +12,6 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
>   	drm_format_helper_test.o \
>   	drm_format_test.o \
>   	drm_framebuffer_test.o \
> -	drm_kunit_helpers.o \
>   	drm_mm_test.o \
>   	drm_modes_test.o \
>   	drm_plane_helper_test.o \
> 

Re: [PATCH v3 04/20] drm/tests: helpers: Switch to EXPORT_SYMBOL_GPL

[Maíra Canal]

On 12/1/22 12:11, Maxime Ripard wrote:
> drm_kunit_device_init() among other things will allocate a device and
> wrap around root_device_register. This function is exported with
> EXPORT_SYMBOL_GPL, so we can't really change the license.
> 
> Fixes: 199557fab925 ("drm/tests: helpers: Add missing export")
> Suggested-by: Javier Martinez Canillas <javierm@redhat.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Best Regards,
- Maíra Canal

> ---
>   drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c
> index 9ed3cfc2ac03..4fe131141718 100644
> --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
> +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
> @@ -82,7 +82,7 @@ struct drm_device *drm_kunit_device_init(struct kunit *test, u32 features, char
>   
>   	return drm;
>   }
> -EXPORT_SYMBOL(drm_kunit_device_init);
> +EXPORT_SYMBOL_GPL(drm_kunit_device_init);
>   
>   MODULE_AUTHOR("Maxime Ripard <maxime@cerno.tech>");
>   MODULE_LICENSE("GPL");
> 

Re: [PATCH v3 17/20] drm/vc4: tests: Introduce a mocking infrastructure

[Maíra Canal]

On 12/1/22 12:11, Maxime Ripard wrote:
> 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.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

But it would be nice if you could fix the checkpatch problems in the 
vc4/tests folder.

Best Regards,
- Maíra Canal

> ---
>   drivers/gpu/drm/vc4/Kconfig                 |  16 +++
>   drivers/gpu/drm/vc4/Makefile                |   6 +
>   drivers/gpu/drm/vc4/tests/.kunitconfig      |  13 ++
>   drivers/gpu/drm/vc4/tests/vc4_mock.c        | 200 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/vc4/tests/vc4_mock.h        |  60 +++++++++
>   drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c   |  41 ++++++
>   drivers/gpu/drm/vc4/tests/vc4_mock_output.c |  99 ++++++++++++++
>   drivers/gpu/drm/vc4/tests/vc4_mock_plane.c  |  47 +++++++
>   drivers/gpu/drm/vc4/vc4_crtc.c              |  20 +--
>   drivers/gpu/drm/vc4/vc4_drv.c               |   4 +-
>   drivers/gpu/drm/vc4/vc4_drv.h               |  16 +++
>   drivers/gpu/drm/vc4/vc4_txp.c               |   2 +-
>   12 files changed, 511 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
> index 246305d17a52..f423941c028d 100644
> --- a/drivers/gpu/drm/vc4/Kconfig
> +++ b/drivers/gpu/drm/vc4/Kconfig
> @@ -34,3 +34,19 @@ 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
> +	bool "KUnit tests for VC4" if !KUNIT_ALL_TESTS
> +	depends on DRM_VC4 && KUNIT
> +	select DRM_KUNIT_TEST_HELPERS
> +	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..f974a8c80e2f 100644
> --- a/drivers/gpu/drm/vc4/Makefile
> +++ b/drivers/gpu/drm/vc4/Makefile
> @@ -25,6 +25,12 @@ vc4-y := \
>   	vc4_validate.o \
>   	vc4_validate_shaders.o
>   
> +vc4-$(CONFIG_DRM_VC4_KUNIT_TEST) += \
> +	tests/vc4_mock.o \
> +	tests/vc4_mock_crtc.o \
> +	tests/vc4_mock_output.o \
> +	tests/vc4_mock_plane.o
> +
>   vc4-$(CONFIG_DEBUG_FS) += vc4_debugfs.o
>   
>   obj-$(CONFIG_DRM_VC4)  += vc4.o
> diff --git a/drivers/gpu/drm/vc4/tests/.kunitconfig b/drivers/gpu/drm/vc4/tests/.kunitconfig
> new file mode 100644
> index 000000000000..b503e9036c7f
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/tests/.kunitconfig
> @@ -0,0 +1,13 @@
> +CONFIG_ARCH_BCM=y
> +CONFIG_ARCH_BCM2835=y
> +CONFIG_BCM2835_MBOX=y
> +CONFIG_KUNIT=y
> +CONFIG_DRM=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/vc4_mock.c b/drivers/gpu/drm/vc4/tests/vc4_mock.c
> new file mode 100644
> index 000000000000..a4bed26af32f
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock.c
> @@ -0,0 +1,200 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <drm/drm_drv.h>
> +#include <drm/drm_kunit_helpers.h>
> +
> +#include <kunit/test.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 vc4_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(&bcm2835_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(&bcm2835_pv1_data,
> +					 VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_DSI1,
> +							      DRM_MODE_ENCODER_DSI,
> +							      DRM_MODE_CONNECTOR_DSI)),
> +		VC4_MOCK_PIXELVALVE_DESC(&bcm2835_pv2_data,
> +					 VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_HDMI0,
> +							      DRM_MODE_ENCODER_TMDS,
> +							      DRM_MODE_CONNECTOR_HDMIA),
> +					 VC4_MOCK_OUTPUT_DESC(VC4_ENCODER_TYPE_VEC,
> +							      DRM_MODE_ENCODER_TVDAC,
> +							      DRM_MODE_CONNECTOR_Composite)),
> +);
> +
> +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 : &vc4_mock;
> +	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..ace5b2e00f4a
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#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..5d12d7beef0e
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#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..cb16ab4451f7
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#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..62b18f5f41db
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_plane.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#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 195d2b61839d..7d1a696477ce 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 5990d8f8c363..3b0ae2c3e33c 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..418f4f308e36 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,15 @@ struct vc4_pv_data {
>   	enum vc4_encoder_type encoder_types[4];
>   };
>   
> +extern const struct vc4_pv_data bcm2835_pv0_data;
> +extern const struct vc4_pv_data bcm2835_pv1_data;
> +extern const struct vc4_pv_data bcm2835_pv2_data;
> +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 +934,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),
> 

Re: [PATCH v3 19/20] drm/vc4: tests: Add unit test suite for the PV muxing

[Maíra Canal]

On 12/1/22 12:11, Maxime Ripard wrote:
> 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.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Best Regards,
- Maíra Canal

> ---
>   drivers/gpu/drm/vc4/Makefile                   |    3 +-
>   drivers/gpu/drm/vc4/tests/vc4_mock.h           |    9 +-
>   drivers/gpu/drm/vc4/tests/vc4_mock_output.c    |   49 +-
>   drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 1039 ++++++++++++++++++++++++
>   4 files changed, 1091 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/Makefile b/drivers/gpu/drm/vc4/Makefile
> index f974a8c80e2f..c41f89a15a55 100644
> --- a/drivers/gpu/drm/vc4/Makefile
> +++ b/drivers/gpu/drm/vc4/Makefile
> @@ -29,7 +29,8 @@ vc4-$(CONFIG_DRM_VC4_KUNIT_TEST) += \
>   	tests/vc4_mock.o \
>   	tests/vc4_mock_crtc.o \
>   	tests/vc4_mock_output.o \
> -	tests/vc4_mock_plane.o
> +	tests/vc4_mock_plane.o \
> +	tests/vc4_test_pv_muxing.o
>   
>   vc4-$(CONFIG_DEBUG_FS) += vc4_debugfs.o
>   
> diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock.h b/drivers/gpu/drm/vc4/tests/vc4_mock.h
> index ace5b2e00f4a..db8e9a141ef8 100644
> --- a/drivers/gpu/drm/vc4/tests/vc4_mock.h
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock.h
> @@ -53,8 +53,11 @@ struct vc4_dummy_output *vc4_dummy_output(struct kunit *test,
>   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);
> +int vc4_mock_atomic_add_output(struct kunit *test,
> +			       struct drm_atomic_state *state,
> +			       enum vc4_encoder_type type);
> +int vc4_mock_atomic_del_output(struct kunit *test,
> +			       struct drm_atomic_state *state,
> +			       enum vc4_encoder_type type);
>   
>   #endif // VC4_MOCK_H_
> diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> index cb16ab4451f7..8d33be828d9a 100644
> --- a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> @@ -61,16 +61,17 @@ 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)
> +int vc4_mock_atomic_add_output(struct kunit *test,
> +			       struct drm_atomic_state *state,
> +			       enum vc4_encoder_type type)
>   {
> +	struct drm_device *drm = state->dev;
> +	struct drm_connector_state *conn_state;
> +	struct drm_crtc_state *crtc_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);
> @@ -97,3 +98,41 @@ int vc4_mock_atomic_add_output(struct kunit *test, struct drm_device *drm,
>   
>   	return 0;
>   }
> +
> +int vc4_mock_atomic_del_output(struct kunit *test,
> +			       struct drm_atomic_state *state,
> +			       enum vc4_encoder_type type)
> +{
> +	struct drm_device *drm = state->dev;
> +	struct drm_connector_state *conn_state;
> +	struct drm_crtc_state *crtc_state;
> +	struct vc4_dummy_output *output;
> +	struct drm_connector *conn;
> +	struct drm_encoder *encoder;
> +	struct drm_crtc *crtc;
> +	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);
> +
> +	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
> +
> +	crtc_state->active = false;
> +
> +	ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	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, NULL);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	return 0;
> +}
> 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..ae0bd0f81698
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
> @@ -0,0 +1,1039 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.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_kunit_helpers.h>
> +#include <drm/drm_mode.h>
> +#include <drm/drm_modeset_helper_vtables.h>
> +#include <drm/drm_plane.h>
> +
> +#include <kunit/test.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 vc4_encoder_constraints[] = {
> +	ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_DPI, 0),
> +	ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_DSI0, 0),
> +	ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_HDMI0, 1),
> +	ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_VEC, 1),
> +	ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_TXP, 2),
> +	ENCODER_CONSTRAINT(VC4_ENCODER_TYPE_DSI1, 2),
> +};
> +
> +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_vc4_encoder_constraints(enum vc4_encoder_type type, unsigned int channel)
> +{
> +	return __check_encoder_constraints(vc4_encoder_constraints,
> +					   ARRAY_SIZE(vc4_encoder_constraints),
> +					   type, channel);
> +}
> +
> +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 struct vc4_crtc_state *
> +get_vc4_crtc_state_for_encoder(struct kunit *test,
> +			       const struct drm_atomic_state *state,
> +			       enum vc4_encoder_type type)
> +{
> +	struct drm_device *drm = state->dev;
> +	struct drm_crtc_state *new_crtc_state;
> +	struct drm_encoder *encoder;
> +	struct drm_crtc *crtc;
> +
> +	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);
> +
> +	new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	if (!new_crtc_state)
> +		return NULL;
> +
> +	return to_vc4_crtc_state(new_crtc_state);
> +}
> +
> +static bool check_channel_for_encoder(struct kunit *test,
> +				      const struct drm_atomic_state *state,
> +				      enum vc4_encoder_type type,
> +				      bool (*check_fn)(enum vc4_encoder_type type, unsigned int channel))
> +{
> +	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);
> +
> +	new_vc4_crtc_state = get_vc4_crtc_state_for_encoder(test, state, type);
> +	KUNIT_ASSERT_NOT_NULL(test, new_vc4_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_fn(type, channel));
> +
> +	return true;
> +}
> +
> +struct pv_muxing_param {
> +	const char *name;
> +	struct vc4_dev *(*mock_fn)(struct kunit *test);
> +	bool (*check_fn)(enum vc4_encoder_type type, unsigned int channel);
> +	enum vc4_encoder_type *encoders;
> +	size_t nencoders;
> +};
> +
> +static void vc4_test_pv_muxing_desc(const struct pv_muxing_param *t, char *desc)
> +{
> +	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
> +}
> +
> +#define PV_MUXING_TEST(_name, _mock_fn, _check_fn, ...)					\
> +	{										\
> +		.name = _name,								\
> +		.mock_fn = &_mock_fn,							\
> +		.check_fn = &_check_fn,							\
> +		.encoders = (enum vc4_encoder_type[]) { __VA_ARGS__ },			\
> +		.nencoders = sizeof((enum vc4_encoder_type[]) { __VA_ARGS__ }) /	\
> +			     sizeof(enum vc4_encoder_type),				\
> +	}
> +
> +#define VC4_PV_MUXING_TEST(_name, ...)		\
> +	PV_MUXING_TEST(_name, vc4_mock_device, check_vc4_encoder_constraints, __VA_ARGS__)
> +
> +#define VC5_PV_MUXING_TEST(_name, ...)		\
> +	PV_MUXING_TEST(_name, vc5_mock_device, check_vc5_encoder_constraints, __VA_ARGS__)
> +
> +static const struct pv_muxing_param vc4_test_pv_muxing_params[] = {
> +	VC4_PV_MUXING_TEST("1 output: DSI0",
> +			   VC4_ENCODER_TYPE_DSI0),
> +	VC4_PV_MUXING_TEST("1 output: DPI",
> +			   VC4_ENCODER_TYPE_DPI),
> +	VC4_PV_MUXING_TEST("1 output: HDMI0",
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC4_PV_MUXING_TEST("1 output: VEC",
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC4_PV_MUXING_TEST("1 output: DSI1",
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("1 output: TXP",
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("2 outputs: DSI0, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC4_PV_MUXING_TEST("2 outputs: DSI0, VEC",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC4_PV_MUXING_TEST("2 outputs: DSI0, DSI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("2 outputs: DSI0, TXP",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("2 outputs: DPI, HDMI0",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC4_PV_MUXING_TEST("2 outputs: DPI, VEC",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC4_PV_MUXING_TEST("2 outputs: DPI, DSI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("2 outputs: DPI, TXP",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("2 outputs: HDMI0, DSI1",
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("2 outputs: HDMI0, TXP",
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("2 outputs: VEC, DSI1",
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("2 outputs: VEC, TXP",
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("3 outputs: DSI0, HDMI0, DSI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("3 outputs: DSI0, HDMI0, TXP",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("3 outputs: DSI0, VEC, DSI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("3 outputs: DSI0, VEC, TXP",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("3 outputs: DPI, HDMI0, DSI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("3 outputs: DPI, HDMI0, TXP",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("3 outputs: DPI, VEC, DSI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("3 outputs: DPI, VEC, TXP",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP),
> +};
> +
> +KUNIT_ARRAY_PARAM(vc4_test_pv_muxing,
> +		  vc4_test_pv_muxing_params,
> +		  vc4_test_pv_muxing_desc);
> +
> +static const struct pv_muxing_param vc4_test_pv_muxing_invalid_params[] = {
> +	VC4_PV_MUXING_TEST("DPI/DSI0 Conflict",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_DSI0),
> +	VC4_PV_MUXING_TEST("TXP/DSI1 Conflict",
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC4_PV_MUXING_TEST("HDMI0/VEC Conflict",
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC4_PV_MUXING_TEST("More than 3 outputs: DSI0, HDMI0, DSI1, TXP",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("More than 3 outputs: DSI0, VEC, DSI1, TXP",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("More than 3 outputs: DPI, HDMI0, DSI1, TXP",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC4_PV_MUXING_TEST("More than 3 outputs: DPI, VEC, DSI1, TXP",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_TXP),
> +};
> +
> +KUNIT_ARRAY_PARAM(vc4_test_pv_muxing_invalid,
> +		  vc4_test_pv_muxing_invalid_params,
> +		  vc4_test_pv_muxing_desc);
> +
> +static const struct pv_muxing_param vc5_test_pv_muxing_params[] = {
> +	VC5_PV_MUXING_TEST("1 output: DPI",
> +			   VC4_ENCODER_TYPE_DPI),
> +	VC5_PV_MUXING_TEST("1 output: DSI0",
> +			   VC4_ENCODER_TYPE_DSI0),
> +	VC5_PV_MUXING_TEST("1 output: DSI1",
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("1 output: HDMI0",
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("1 output: HDMI1",
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("1 output: VEC",
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC5_PV_MUXING_TEST("2 outputs: DPI, DSI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("2 outputs: DPI, HDMI0",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("2 outputs: DPI, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("2 outputs: DPI, TXP",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC5_PV_MUXING_TEST("2 outputs: DPI, VEC",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC5_PV_MUXING_TEST("2 outputs: DPI, DSI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI0, DSI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI0, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI0, TXP",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI0, VEC",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI0, DSI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI1, VEC",
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI1, TXP",
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI1, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("2 outputs: DSI1, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("2 outputs: HDMI0, VEC",
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC5_PV_MUXING_TEST("2 outputs: HDMI0, TXP",
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC5_PV_MUXING_TEST("2 outputs: HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("2 outputs: HDMI1, VEC",
> +			   VC4_ENCODER_TYPE_HDMI1,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC5_PV_MUXING_TEST("2 outputs: HDMI1, TXP",
> +			   VC4_ENCODER_TYPE_HDMI1,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC5_PV_MUXING_TEST("2 outputs: TXP, VEC",
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_VEC),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, VEC, TXP",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, VEC, DSI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, VEC, HDMI0",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, VEC, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, TXP, DSI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, TXP, HDMI0",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, TXP, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, DSI1, HDMI0",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, DSI1, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DPI, HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, VEC, TXP",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, VEC, DSI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, VEC, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, VEC, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, TXP, DSI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, TXP, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, TXP, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, DSI1, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, DSI1, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("3 outputs: DSI0, HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +};
> +
> +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[] = {
> +	VC5_PV_MUXING_TEST("DPI/DSI0 Conflict",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_DSI0),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, VEC, TXP, DSI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, VEC, TXP, HDMI0",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, VEC, TXP, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, VEC, DSI1, HDMI0",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, VEC, DSI1, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, VEC, HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, TXP, DSI1, HDMI0",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, TXP, DSI1, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, TXP, HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DPI, DSI1, HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DPI,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, VEC, TXP, DSI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, VEC, TXP, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, VEC, TXP, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, VEC, DSI1, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, VEC, DSI1, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, VEC, HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_VEC,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, TXP, DSI1, HDMI0",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, TXP, DSI1, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, TXP, HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_TXP,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: DSI0, DSI1, HDMI0, HDMI1",
> +			   VC4_ENCODER_TYPE_DSI0,
> +			   VC4_ENCODER_TYPE_DSI1,
> +			   VC4_ENCODER_TYPE_HDMI0,
> +			   VC4_ENCODER_TYPE_HDMI1),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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),
> +	VC5_PV_MUXING_TEST("More than 3 outputs: 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_vc4_test_pv_muxing(struct kunit *test)
> +{
> +	const struct pv_muxing_param *params = test->param_value;
> +	const struct pv_muxing_priv *priv = test->priv;
> +	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, state, enc_type);
> +		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,
> +								  params->check_fn));
> +	}
> +}
> +
> +static void drm_vc4_test_pv_muxing_invalid(struct kunit *test)
> +{
> +	const struct pv_muxing_param *params = test->param_value;
> +	const struct pv_muxing_priv *priv = test->priv;
> +	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, state, enc_type);
> +		KUNIT_ASSERT_EQ(test, ret, 0);
> +	}
> +
> +	ret = drm_atomic_check_only(state);
> +	KUNIT_EXPECT_LT(test, ret, 0);
> +}
> +
> +static int vc4_pv_muxing_test_init(struct kunit *test)
> +{
> +	const struct pv_muxing_param *params = test->param_value;
> +	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 = params->mock_fn(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 vc4_pv_muxing_tests[] = {
> +	KUNIT_CASE_PARAM(drm_vc4_test_pv_muxing,
> +			 vc4_test_pv_muxing_gen_params),
> +	KUNIT_CASE_PARAM(drm_vc4_test_pv_muxing_invalid,
> +			 vc4_test_pv_muxing_invalid_gen_params),
> +	{}
> +};
> +
> +static struct kunit_suite vc4_pv_muxing_test_suite = {
> +	.name = "vc4-pv-muxing-combinations",
> +	.init = vc4_pv_muxing_test_init,
> +	.exit = vc4_pv_muxing_test_exit,
> +	.test_cases = vc4_pv_muxing_tests,
> +};
> +
> +static struct kunit_case vc5_pv_muxing_tests[] = {
> +	KUNIT_CASE_PARAM(drm_vc4_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-combinations",
> +	.init = vc4_pv_muxing_test_init,
> +	.exit = vc4_pv_muxing_test_exit,
> +	.test_cases = vc5_pv_muxing_tests,
> +};
> +
> +/* See
> + * https://lore.kernel.org/all/3e113525-aa89-b1e2-56b7-ca55bd41d057@samsung.com/
> + * and
> + * https://lore.kernel.org/dri-devel/20200917121623.42023-1-maxime@cerno.tech/
> + */
> +static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *test)
> +{
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	struct vc4_crtc_state *new_vc4_crtc_state;
> +	struct vc4_hvs_state *new_hvs_state;
> +	unsigned int hdmi0_channel;
> +	unsigned int hdmi1_channel;
> +	struct drm_device *drm;
> +	struct vc4_dev *vc4;
> +	int ret;
> +
> +	vc4 = vc5_mock_device(test);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4);
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +	drm = &vc4->base;
> +	state = drm_atomic_state_alloc(drm);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
> +
> +	state->acquire_ctx = &ctx;
> +
> +	ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = drm_atomic_check_only(state);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	new_hvs_state = vc4_hvs_get_new_global_state(state);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_hvs_state);
> +
> +	new_vc4_crtc_state = get_vc4_crtc_state_for_encoder(test, state,
> +							    VC4_ENCODER_TYPE_HDMI0);
> +	KUNIT_ASSERT_NOT_NULL(test, new_vc4_crtc_state);
> +
> +	hdmi0_channel = new_vc4_crtc_state->assigned_channel;
> +	KUNIT_ASSERT_NE(test, hdmi0_channel, VC4_HVS_CHANNEL_DISABLED);
> +	KUNIT_ASSERT_TRUE(test, new_hvs_state->fifo_state[hdmi0_channel].in_use);
> +
> +	ret = drm_atomic_helper_swap_state(state, false);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	drm_atomic_state_put(state);
> +
> +	state = drm_atomic_state_alloc(drm);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
> +
> +	state->acquire_ctx = &ctx;
> +
> +	ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI1);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = drm_atomic_check_only(state);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	new_hvs_state = vc4_hvs_get_new_global_state(state);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_hvs_state);
> +
> +	new_vc4_crtc_state = get_vc4_crtc_state_for_encoder(test, state,
> +							    VC4_ENCODER_TYPE_HDMI1);
> +	KUNIT_ASSERT_NOT_NULL(test, new_vc4_crtc_state);
> +
> +	hdmi1_channel = new_vc4_crtc_state->assigned_channel;
> +	KUNIT_ASSERT_NE(test, hdmi1_channel, VC4_HVS_CHANNEL_DISABLED);
> +	KUNIT_ASSERT_TRUE(test, new_hvs_state->fifo_state[hdmi1_channel].in_use);
> +
> +	KUNIT_EXPECT_NE(test, hdmi0_channel, hdmi1_channel);
> +
> +	drm_atomic_state_put(state);
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	drm_dev_unregister(drm);
> +	drm_kunit_helper_free_device(test, vc4->dev);
> +}
> +
> +static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
> +{
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	struct vc4_crtc_state *new_vc4_crtc_state;
> +	struct vc4_hvs_state *new_hvs_state;
> +	unsigned int old_hdmi0_channel;
> +	unsigned int old_hdmi1_channel;
> +	struct drm_device *drm;
> +	struct vc4_dev *vc4;
> +	int ret;
> +
> +	vc4 = vc5_mock_device(test);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4);
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +	drm = &vc4->base;
> +	state = drm_atomic_state_alloc(drm);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
> +
> +	state->acquire_ctx = &ctx;
> +
> +	ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI1);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = drm_atomic_check_only(state);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	new_hvs_state = vc4_hvs_get_new_global_state(state);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_hvs_state);
> +
> +	new_vc4_crtc_state = get_vc4_crtc_state_for_encoder(test, state,
> +							    VC4_ENCODER_TYPE_HDMI0);
> +	KUNIT_ASSERT_NOT_NULL(test, new_vc4_crtc_state);
> +
> +	old_hdmi0_channel = new_vc4_crtc_state->assigned_channel;
> +	KUNIT_ASSERT_NE(test, old_hdmi0_channel, VC4_HVS_CHANNEL_DISABLED);
> +	KUNIT_ASSERT_TRUE(test, new_hvs_state->fifo_state[old_hdmi0_channel].in_use);
> +
> +	new_vc4_crtc_state = get_vc4_crtc_state_for_encoder(test, state,
> +							    VC4_ENCODER_TYPE_HDMI1);
> +	KUNIT_ASSERT_NOT_NULL(test, new_vc4_crtc_state);
> +
> +	old_hdmi1_channel = new_vc4_crtc_state->assigned_channel;
> +	KUNIT_ASSERT_NE(test, old_hdmi1_channel, VC4_HVS_CHANNEL_DISABLED);
> +	KUNIT_ASSERT_TRUE(test, new_hvs_state->fifo_state[old_hdmi1_channel].in_use);
> +
> +	ret = drm_atomic_helper_swap_state(state, false);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	drm_atomic_state_put(state);
> +
> +	state = drm_atomic_state_alloc(drm);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
> +
> +	state->acquire_ctx = &ctx;
> +
> +	ret = vc4_mock_atomic_del_output(test, state, VC4_ENCODER_TYPE_HDMI0);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = drm_atomic_check_only(state);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	new_hvs_state = vc4_hvs_get_new_global_state(state);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_hvs_state);
> +
> +	new_vc4_crtc_state = get_vc4_crtc_state_for_encoder(test, state,
> +							    VC4_ENCODER_TYPE_HDMI1);
> +
> +	if (new_vc4_crtc_state) {
> +		unsigned int hdmi1_channel;
> +
> +		hdmi1_channel = new_vc4_crtc_state->assigned_channel;
> +		KUNIT_ASSERT_NE(test, hdmi1_channel, VC4_HVS_CHANNEL_DISABLED);
> +		KUNIT_ASSERT_TRUE(test, new_hvs_state->fifo_state[hdmi1_channel].in_use);
> +
> +		KUNIT_EXPECT_EQ(test, old_hdmi1_channel, hdmi1_channel);
> +	}
> +
> +	drm_atomic_state_put(state);
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	drm_dev_unregister(drm);
> +	drm_kunit_helper_free_device(test, vc4->dev);
> +}
> +
> +static void
> +drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct kunit *test)
> +{
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	struct vc4_crtc_state *new_vc4_crtc_state;
> +	struct drm_device *drm;
> +	struct vc4_dev *vc4;
> +	int ret;
> +
> +	vc4 = vc5_mock_device(test);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vc4);
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +	drm = &vc4->base;
> +	state = drm_atomic_state_alloc(drm);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
> +
> +	state->acquire_ctx = &ctx;
> +
> +	ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI0);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = drm_atomic_check_only(state);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = drm_atomic_helper_swap_state(state, false);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	drm_atomic_state_put(state);
> +
> +	state = drm_atomic_state_alloc(drm);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
> +
> +	state->acquire_ctx = &ctx;
> +
> +	ret = vc4_mock_atomic_add_output(test, state, VC4_ENCODER_TYPE_HDMI1);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = drm_atomic_check_only(state);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	new_vc4_crtc_state = get_vc4_crtc_state_for_encoder(test, state,
> +							    VC4_ENCODER_TYPE_HDMI0);
> +	KUNIT_EXPECT_NULL(test, new_vc4_crtc_state);
> +
> +	drm_atomic_state_put(state);
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	drm_dev_unregister(drm);
> +	drm_kunit_helper_free_device(test, vc4->dev);
> +}
> +
> +static struct kunit_case vc5_pv_muxing_bugs_tests[] = {
> +	KUNIT_CASE(drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable),
> +	KUNIT_CASE(drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state),
> +	KUNIT_CASE(drm_test_vc5_pv_muxing_bugs_stable_fifo),
> +	{}
> +};
> +
> +static struct kunit_suite vc5_pv_muxing_bugs_test_suite = {
> +	.name = "vc5-pv-muxing-bugs",
> +	.test_cases = vc5_pv_muxing_bugs_tests,
> +};
> +
> +kunit_test_suites(
> +	&vc4_pv_muxing_test_suite,
> +	&vc5_pv_muxing_test_suite,
> +	&vc5_pv_muxing_bugs_test_suite
> +);
> 

Re: [PATCH v3 18/20] drm/vc4: tests: Fail the current test if we access a register

[Maíra Canal]

On 12/1/22 12:11, Maxime Ripard wrote:
> Accessing a register when running under kunit is a bad idea since our
> device is completely mocked.
> 
> Fail the current test if we ever access any of our hardware registers.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Just a small nit: I believe that macros with multiple statements should 
be enclosed in a do-while block [1], even READ macros. I saw that you 
enclosed the WRITE macros on a do-while block, but not the READ macros.

[1] 
https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl

Best Regards,
- Maíra Canal

> ---
>   drivers/gpu/drm/vc4/vc4_crtc.c      | 13 +++++++++++--
>   drivers/gpu/drm/vc4/vc4_dpi.c       | 13 +++++++++++--
>   drivers/gpu/drm/vc4/vc4_drv.h       | 29 +++++++++++++++++++++++++----
>   drivers/gpu/drm/vc4/vc4_dsi.c       |  9 ++++++++-
>   drivers/gpu/drm/vc4/vc4_hdmi_regs.h |  4 ++++
>   drivers/gpu/drm/vc4/vc4_txp.c       | 13 +++++++++++--
>   drivers/gpu/drm/vc4/vc4_vec.c       | 13 +++++++++++--
>   7 files changed, 81 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 7d1a696477ce..a1a3465948c4 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -50,8 +50,17 @@
>   
>   #define HVS_FIFO_LATENCY_PIX	6
>   
> -#define CRTC_WRITE(offset, val) writel(val, vc4_crtc->regs + (offset))
> -#define CRTC_READ(offset) readl(vc4_crtc->regs + (offset))
> +#define CRTC_WRITE(offset, val)								\
> +	do {										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		writel(val, vc4_crtc->regs + (offset));					\
> +	} while (0)
> +
> +#define CRTC_READ(offset)								\
> +	({										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		readl(vc4_crtc->regs + (offset));					\
> +	})
>   
>   static const struct debugfs_reg32 crtc_regs[] = {
>   	VC4_REG32(PV_CONTROL),
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index 1f8f44b7b5a5..0edf3c4c98c8 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -103,8 +103,17 @@ to_vc4_dpi(struct drm_encoder *encoder)
>   	return container_of(encoder, struct vc4_dpi, encoder.base);
>   }
>   
> -#define DPI_READ(offset) readl(dpi->regs + (offset))
> -#define DPI_WRITE(offset, val) writel(val, dpi->regs + (offset))
> +#define DPI_READ(offset)								\
> +	({										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		readl(dpi->regs + (offset));						\
> +	})
> +
> +#define DPI_WRITE(offset, val)								\
> +	do {										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		writel(val, dpi->regs + (offset));					\
> +	} while (0)
>   
>   static const struct debugfs_reg32 dpi_regs[] = {
>   	VC4_REG32(DPI_C),
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 418f4f308e36..78fda5332cb3 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -19,6 +19,8 @@
>   #include <drm/drm_mm.h>
>   #include <drm/drm_modeset_lock.h>
>   
> +#include <kunit/test-bug.h>
> +
>   #include "uapi/drm/vc4_drm.h"
>   
>   struct drm_device;
> @@ -645,10 +647,29 @@ to_vc4_crtc_state(const struct drm_crtc_state *crtc_state)
>   	return container_of(crtc_state, struct vc4_crtc_state, base);
>   }
>   
> -#define V3D_READ(offset) readl(vc4->v3d->regs + offset)
> -#define V3D_WRITE(offset, val) writel(val, vc4->v3d->regs + offset)
> -#define HVS_READ(offset) readl(hvs->regs + offset)
> -#define HVS_WRITE(offset, val) writel(val, hvs->regs + offset)
> +#define V3D_READ(offset)								\
> +	({										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		readl(vc4->v3d->regs + (offset));						\
> +	})
> +
> +#define V3D_WRITE(offset, val)								\
> +	do {										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		writel(val, vc4->v3d->regs + (offset));					\
> +	} while (0)
> +
> +#define HVS_READ(offset)								\
> +	({										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		readl(hvs->regs + (offset));						\
> +	})
> +
> +#define HVS_WRITE(offset, val)								\
> +	do {										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		writel(val, hvs->regs + (offset));					\
> +	} while (0)
>   
>   #define VC4_REG32(reg) { .name = #reg, .offset = reg }
>   
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index 878e05d79e81..2c9cb27903a0 100644
> --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> @@ -617,6 +617,8 @@ dsi_dma_workaround_write(struct vc4_dsi *dsi, u32 offset, u32 val)
>   	dma_cookie_t cookie;
>   	int ret;
>   
> +	kunit_fail_current_test("Accessing a register in a unit test!\n");
> +
>   	/* DSI0 should be able to write normally. */
>   	if (!chan) {
>   		writel(val, dsi->regs + offset);
> @@ -645,7 +647,12 @@ dsi_dma_workaround_write(struct vc4_dsi *dsi, u32 offset, u32 val)
>   		DRM_ERROR("Failed to wait for DMA: %d\n", ret);
>   }
>   
> -#define DSI_READ(offset) readl(dsi->regs + (offset))
> +#define DSI_READ(offset)								\
> +	({										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		readl(dsi->regs + (offset));						\
> +	})
> +
>   #define DSI_WRITE(offset, val) dsi_dma_workaround_write(dsi, offset, val)
>   #define DSI_PORT_READ(offset) \
>   	DSI_READ(dsi->variant->port ? DSI1_##offset : DSI0_##offset)
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
> index 48db438550b1..b04b2fc8d831 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
> @@ -456,6 +456,8 @@ static inline u32 vc4_hdmi_read(struct vc4_hdmi *hdmi,
>   
>   	WARN_ON(pm_runtime_status_suspended(&hdmi->pdev->dev));
>   
> +	kunit_fail_current_test("Accessing an HDMI register in a unit test!\n");
> +
>   	if (reg >= variant->num_registers) {
>   		dev_warn(&hdmi->pdev->dev,
>   			 "Invalid register ID %u\n", reg);
> @@ -486,6 +488,8 @@ static inline void vc4_hdmi_write(struct vc4_hdmi *hdmi,
>   
>   	WARN_ON(pm_runtime_status_suspended(&hdmi->pdev->dev));
>   
> +	kunit_fail_current_test("Accessing an HDMI register in a unit test!\n");
> +
>   	if (reg >= variant->num_registers) {
>   		dev_warn(&hdmi->pdev->dev,
>   			 "Invalid register ID %u\n", reg);
> diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
> index 2b69454b8534..ef5cab2a3aa9 100644
> --- a/drivers/gpu/drm/vc4/vc4_txp.c
> +++ b/drivers/gpu/drm/vc4/vc4_txp.c
> @@ -145,8 +145,17 @@
>   /* Number of lines received and committed to memory. */
>   #define TXP_PROGRESS		0x10
>   
> -#define TXP_READ(offset) readl(txp->regs + (offset))
> -#define TXP_WRITE(offset, val) writel(val, txp->regs + (offset))
> +#define TXP_READ(offset)								\
> +	({										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		readl(txp->regs + (offset));						\
> +	})
> +
> +#define TXP_WRITE(offset, val)								\
> +	do {										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		writel(val, txp->regs + (offset));					\
> +	} while (0)
>   
>   struct vc4_txp {
>   	struct vc4_crtc	base;
> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> index e270a4099be3..f589ab918f4d 100644
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -207,8 +207,17 @@ struct vc4_vec {
>   	struct debugfs_regset32 regset;
>   };
>   
> -#define VEC_READ(offset) readl(vec->regs + (offset))
> -#define VEC_WRITE(offset, val) writel(val, vec->regs + (offset))
> +#define VEC_READ(offset)								\
> +	({										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		readl(vec->regs + (offset));						\
> +	})
> +
> +#define VEC_WRITE(offset, val)								\
> +	do {										\
> +		kunit_fail_current_test("Accessing a register in a unit test!\n");	\
> +		writel(val, vec->regs + (offset));					\
> +	} while (0)
>   
>   static inline struct vc4_vec *
>   encoder_to_vc4_vec(struct drm_encoder *encoder)
> 

Re: [PATCH v3 18/20] drm/vc4: tests: Fail the current test if we access a register

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

Hi Maíra,

Thanks for your review!

On Wed, Dec 07, 2022 at 11:26:13AM -0300, Maíra Canal wrote:
> On 12/1/22 12:11, Maxime Ripard wrote:
> > Accessing a register when running under kunit is a bad idea since our
> > device is completely mocked.
> > 
> > Fail the current test if we ever access any of our hardware registers.
> > 
> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> 
> Reviewed-by: Maíra Canal <mcanal@igalia.com>
> 
> Just a small nit: I believe that macros with multiple statements should be
> enclosed in a do-while block [1], even READ macros. I saw that you enclosed
> the WRITE macros on a do-while block, but not the READ macros.

This is on purpose: do-while blocks don't return a value, but ({ ... })
blocks do. So we can do a write macros with a do-while, but can't for
read since we expect to get a value back.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]