On Tue, Mar 11, 2025 at 12:57:37PM +0200, Cristian Ciocaltea wrote:
> Introduce a few macros to facilitate setting custom (i.e. non-default)
> EDID data during connector initialization.
>
> This helps reducing boilerplate code while also drops some redundant
> calls to set_connector_edid().
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocal...@collabora.com>
> ---
> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 245
> ++++++++-------------
> 1 file changed, 93 insertions(+), 152 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> index
> e97efd3af9ed18e6cf8ee66b4923dfc805b34e19..a3f7f3ce31c73335c2c2643bdc5395b6ceb6f071
> 100644
> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> @@ -183,10 +183,12 @@ static const struct drm_connector_funcs
> dummy_connector_funcs = {
>
> static
> struct drm_atomic_helper_connector_hdmi_priv *
> -drm_kunit_helper_connector_hdmi_init_funcs(struct kunit *test,
> - unsigned int formats,
> - unsigned int max_bpc,
> - const struct
> drm_connector_hdmi_funcs *hdmi_funcs)
> +connector_hdmi_init_funcs_set_edid(struct kunit *test,
> + unsigned int formats,
> + unsigned int max_bpc,
> + const struct drm_connector_hdmi_funcs
> *hdmi_funcs,
> + const char *edid_data,
> + size_t edid_len)
> {
> struct drm_atomic_helper_connector_hdmi_priv *priv;
> struct drm_connector *conn;
> @@ -240,30 +242,27 @@ drm_kunit_helper_connector_hdmi_init_funcs(struct kunit
> *test,
>
> drm_mode_config_reset(drm);
>
> + if (edid_data && edid_len) {
> + ret = set_connector_edid(test, &priv->connector, edid_data,
> edid_len);
> + KUNIT_ASSERT_GT(test, ret, 0);
> + }
> +
> return priv;
> }
>
> -static
> -struct drm_atomic_helper_connector_hdmi_priv *
> -drm_kunit_helper_connector_hdmi_init(struct kunit *test,
> - unsigned int formats,
> - unsigned int max_bpc)
> -{
> - struct drm_atomic_helper_connector_hdmi_priv *priv;
> - int ret;
> +#define drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats,
> max_bpc, funcs, edid) \
> + connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, edid,
> ARRAY_SIZE(edid))
>
> - priv = drm_kunit_helper_connector_hdmi_init_funcs(test,
> - formats, max_bpc,
> -
> &dummy_connector_hdmi_funcs);
> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
> +#define drm_kunit_helper_connector_hdmi_init_funcs(test, formats, max_bpc,
> funcs) \
> + connector_hdmi_init_funcs_set_edid(test, formats, max_bpc, funcs, NULL,
> 0)
>
> - ret = set_connector_edid(test, &priv->connector,
> - test_edid_hdmi_1080p_rgb_max_200mhz,
> -
> ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
> - KUNIT_ASSERT_GT(test, ret, 0);
> +#define drm_kunit_helper_connector_hdmi_init_set_edid(test, formats,
> max_bpc, edid) \
> + drm_kunit_helper_connector_hdmi_init_funcs_set_edid(test, formats,
> max_bpc, \
> +
> &dummy_connector_hdmi_funcs, edid)
>
> - return priv;
> -}
> +#define drm_kunit_helper_connector_hdmi_init(test, formats, max_bpc)
> \
> + drm_kunit_helper_connector_hdmi_init_set_edid(test, formats, max_bpc,
> \
> +
> test_edid_hdmi_1080p_rgb_max_200mhz)
I'd really prefer to have functions to macros here. They are easier to
read, extend, and don't have any particular drawbacks.
I also don't think we need that many, looking at the tests:
- We need drm_kunit_helper_connector_hdmi_init() to setup a connector
with test_edid_hdmi_1080p_rgb_max_200mhz and
dummy_connector_hdmi_funcs()
- We need to create a
drm_kunit_helper_connector_hdmi_init_with_edid_funcs() to pass both
the funcs and edid pointers
And that's it, right?
> /*
> * Test that if we change the RGB quantization property to a different
> @@ -771,19 +770,15 @@ static void
> drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
> struct drm_crtc *crtc;
> int ret;
>
> - priv = drm_kunit_helper_connector_hdmi_init(test,
> - BIT(HDMI_COLORSPACE_RGB),
> - 10);
> + priv = drm_kunit_helper_connector_hdmi_init_set_edid(test,
> + BIT(HDMI_COLORSPACE_RGB),
> + 10,
> + test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz);
I think that convertion should be part of another patch.
Maxime
signature.asc
Description: PGP signature
