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 <[email protected]> > --- > 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
