On Mon, Jan 17, 2022 at 3:24 PM Michał Winiarski michal.winiarski@intel.com wrote:
igt_dp_mst_calc_pbn_mode was converted one-to-one, igt_dp_mst_sideband_msg_req_decode was refactored to parameterized test.
Signed-off-by: Michał Winiarski michal.winiarski@intel.com
drivers/gpu/drm/selftests/Makefile | 3 +- .../gpu/drm/selftests/drm_modeset_selftests.h | 2 - .../drm/selftests/test-drm_dp_mst_helper.c | 502 ++++++++++++------ .../drm/selftests/test-drm_modeset_common.h | 2 - 4 files changed, 330 insertions(+), 179 deletions(-)
diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile index 35f2f40dbaf3..77e37eebf099 100644 --- a/drivers/gpu/drm/selftests/Makefile +++ b/drivers/gpu/drm/selftests/Makefile @@ -1,7 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only test-drm_modeset-$(CONFIG_DRM_DEBUG_SELFTEST) := \ test-drm_modeset_common.o \
test-drm_dp_mst_helper.o \ test-drm_rect.oobj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o @@ -9,4 +8,4 @@ obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o obj-$(CONFIG_DRM_KUNIT_TEST) := \ test-drm_cmdline_parser.o test-drm_plane_helper.o \ test-drm_format.o test-drm_framebuffer.o \
test-drm_damage_helper.o
test-drm_damage_helper.o test-drm_dp_mst_helper.odiff --git a/drivers/gpu/drm/selftests/drm_modeset_selftests.h b/drivers/gpu/drm/selftests/drm_modeset_selftests.h index b6a6dba66b64..630770d30aba 100644 --- a/drivers/gpu/drm/selftests/drm_modeset_selftests.h +++ b/drivers/gpu/drm/selftests/drm_modeset_selftests.h @@ -10,5 +10,3 @@ selftest(drm_rect_clip_scaled_div_by_zero, igt_drm_rect_clip_scaled_div_by_zero) selftest(drm_rect_clip_scaled_not_clipped, igt_drm_rect_clip_scaled_not_clipped) selftest(drm_rect_clip_scaled_clipped, igt_drm_rect_clip_scaled_clipped) selftest(drm_rect_clip_scaled_signed_vs_unsigned, igt_drm_rect_clip_scaled_signed_vs_unsigned) -selftest(dp_mst_calc_pbn_mode, igt_dp_mst_calc_pbn_mode) -selftest(dp_mst_sideband_msg_req_decode, igt_dp_mst_sideband_msg_req_decode) diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c index 6b4759ed6bfd..d0719f3c5a42 100644 --- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c +++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c @@ -3,54 +3,97 @@
- Test cases for for the DRM DP MST helpers
*/
-#define PREFIX_STR "[drm_dp_mst_helper]"
+#include <kunit/test.h> #include <linux/random.h>
#include <drm/drm_dp_mst_helper.h> #include <drm/drm_print.h>
#include "../drm_dp_mst_topology_internal.h" -#include "test-drm_modeset_common.h"
-int igt_dp_mst_calc_pbn_mode(void *ignored) +struct dp_mst_calc_pbn_mode_test {
int rate;int bpp;int expected;bool dsc;+};
+static void dp_mst_calc_pbn_mode(struct kunit *test) {
int pbn, i;const struct {int rate;int bpp;int expected;bool dsc;} test_params[] = {{ 154000, 30, 689, false },{ 234000, 30, 1047, false },{ 297000, 24, 1063, false },{ 332880, 24, 50, true },{ 324540, 24, 49, true },};
const struct dp_mst_calc_pbn_mode_test *params = test->param_value;int pbn;
for (i = 0; i < ARRAY_SIZE(test_params); i++) {pbn = drm_dp_calc_pbn_mode(test_params[i].rate,test_params[i].bpp,test_params[i].dsc);FAIL(pbn != test_params[i].expected,"Expected PBN %d for clock %d bpp %d, got %d\n",test_params[i].expected, test_params[i].rate,test_params[i].bpp, pbn);}
pbn = drm_dp_calc_pbn_mode(params->rate,params->bpp,params->dsc);KUNIT_EXPECT_EQ(test, pbn, params->expected);+}
return 0;+static const struct dp_mst_calc_pbn_mode_test dp_mst_calc_pbn_mode_tests[] = {
{.rate = 154000,.bpp = 30,.expected = 689,.dsc = false,},{.rate = 234000,.bpp = 30,.expected = 1047,.dsc = false,},{.rate = 297000,.bpp = 24,.expected = 1063,.dsc = false,},{.rate = 332880,.bpp = 24,.expected = 50,.dsc = true,},{.rate = 324540,.bpp = 24,.expected = 49,.dsc = true,},+};
+static void dp_mst_calc_pbn_mode_desc(const struct dp_mst_calc_pbn_mode_test *t,
char *desc)+{
sprintf(desc, "rate = %d, bpp = %d, dsc = %s",t->rate, t->bpp, t->dsc ? "true" : "false");}
-static bool -sideband_msg_req_equal(const struct drm_dp_sideband_msg_req_body *in,
const struct drm_dp_sideband_msg_req_body *out)+KUNIT_ARRAY_PARAM(dp_mst_calc_pbn_mode, dp_mst_calc_pbn_mode_tests, dp_mst_calc_pbn_mode_desc);
+static void +drm_dp_mst_helper_printfn(struct drm_printer *p, struct va_format *vaf) +{
struct kunit *test = p->arg;kunit_err(test, "%pV", vaf);+}
+static void +expect_sideband_msg_req_equal(struct kunit *test,
const struct drm_dp_sideband_msg_req_body *in,const struct drm_dp_sideband_msg_req_body *out){ const struct drm_dp_remote_i2c_read_tx *txin, *txout;
struct drm_printer p = {.printfn = drm_dp_mst_helper_printfn,.arg = test}; int i; if (in->req_type != out->req_type)
return false;
goto fail;
Briefly skimming over this code, it looks like it'd be simpler to have this function remain unchanged. There's only the one caller. It could take on the responsibility of creating the drm_printer and redirect the printfn to kunit_err, afaik.
Passing in `test` would be useful if we were generating custom error messages for each of the `return false` lines, which I assume was the original motivation for doing so? But looking at this, I'd agree it seems like too much work.
Tangent: It might have been easier to do that if the kunit assertions returned pass/fail. E.g. instead of having to do
if (!<long-condition>) { KUNIT_FAIL("<long-condition> not met"); return; }
if we could do
if(!KUNIT_EXPECT_TRUE(long-condition)) return;
or if there was a new macro type
KUNIT_EXPECT_RET_TRUE(long-condition); // like ASSERT, but just return from this func on failure
switch (in->req_type) { /*@@ -65,7 +108,7 @@ sideband_msg_req_equal(const struct drm_dp_sideband_msg_req_body *in, IN.num_transactions != OUT.num_transactions || IN.port_number != OUT.port_number || IN.read_i2c_device_id != OUT.read_i2c_device_id)
return false;
goto fail; for (i = 0; i < IN.num_transactions; i++) { txin = &IN.transactions[i];@@ -76,11 +119,11 @@ sideband_msg_req_equal(const struct drm_dp_sideband_msg_req_body *in, txin->num_bytes != txout->num_bytes || txin->i2c_transaction_delay != txout->i2c_transaction_delay)
return false;
goto fail; if (memcmp(txin->bytes, txout->bytes, txin->num_bytes) != 0)
return false;
goto fail; } break;#undef IN @@ -92,9 +135,12 @@ sideband_msg_req_equal(const struct drm_dp_sideband_msg_req_body *in, if (IN.dpcd_address != OUT.dpcd_address || IN.num_bytes != OUT.num_bytes || IN.port_number != OUT.port_number)
return false;
goto fail;
return memcmp(IN.bytes, OUT.bytes, IN.num_bytes) == 0;
if (memcmp(IN.bytes, OUT.bytes, IN.num_bytes) != 0)goto fail;break;#undef IN #undef OUT
@@ -104,55 +150,65 @@ sideband_msg_req_equal(const struct drm_dp_sideband_msg_req_body *in, if (IN.port_number != OUT.port_number || IN.write_i2c_device_id != OUT.write_i2c_device_id || IN.num_bytes != OUT.num_bytes)
return false;
goto fail;
return memcmp(IN.bytes, OUT.bytes, IN.num_bytes) == 0;
if (memcmp(IN.bytes, OUT.bytes, IN.num_bytes) != 0)goto fail;break;#undef IN #undef OUT
default:
return memcmp(in, out, sizeof(*in)) == 0;
if (memcmp(in, out, sizeof(*in)) != 0)goto fail; }
return true;
return;+fail:
drm_printf(&p, "Expected:\n");drm_dp_dump_sideband_msg_req_body(in, 1, &p);drm_printf(&p, "Got:\n");drm_dp_dump_sideband_msg_req_body(out, 1, &p);KUNIT_FAIL(test, "Encode/decode failed");+}
+struct dp_mst_sideband_msg_req_decode_test {
const struct drm_dp_sideband_msg_req_body req;const struct drm_dp_sideband_msg_req_body(*f)(const struct drm_dp_sideband_msg_req_body *in);+};
+const struct drm_dp_sideband_msg_req_body +param_to_dp_mst_sideband_msg_req_body(const struct dp_mst_sideband_msg_req_decode_test *t) +{
if (t->f)return t->f(&t->req);return t->req;}
-static bool -sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in) +static void dp_mst_sideband_msg_req_decode(struct kunit *test) {
const struct drm_dp_sideband_msg_req_body in =param_to_dp_mst_sideband_msg_req_body(test->param_value); struct drm_dp_sideband_msg_req_body *out;
struct drm_printer p = drm_err_printer(PREFIX_STR); struct drm_dp_sideband_msg_tx *txmsg;int i, ret;bool result = true;out = kzalloc(sizeof(*out), GFP_KERNEL);if (!out)return false;txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);if (!txmsg)return false;drm_dp_encode_sideband_req(in, txmsg);ret = drm_dp_decode_sideband_req(txmsg, out);if (ret < 0) {drm_printf(&p, "Failed to decode sideband request: %d\n",ret);result = false;goto out;}
int i;
if (!sideband_msg_req_equal(in, out)) {drm_printf(&p, "Encode/decode failed, expected:\n");drm_dp_dump_sideband_msg_req_body(in, 1, &p);drm_printf(&p, "Got:\n");drm_dp_dump_sideband_msg_req_body(out, 1, &p);result = false;goto out;}
out = kunit_kzalloc(test, sizeof(*out), GFP_KERNEL);KUNIT_ASSERT_NOT_ERR_OR_NULL(test, out);
switch (in->req_type) {
txmsg = kunit_kzalloc(test, sizeof(*txmsg), GFP_KERNEL);KUNIT_ASSERT_NOT_ERR_OR_NULL(test, txmsg);drm_dp_encode_sideband_req(&in, txmsg);KUNIT_ASSERT_EQ(test, drm_dp_decode_sideband_req(txmsg, out), 0);expect_sideband_msg_req_equal(test, &in, out);switch (in.req_type) { case DP_REMOTE_DPCD_WRITE: kfree(out->u.dpcd_write.bytes); break;@@ -164,110 +220,210 @@ sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in) kfree(out->u.i2c_write.bytes); break; }
/* Clear everything but the req_type for the input */memset(&in->u, 0, sizeof(in->u));-out:
kfree(out);kfree(txmsg);return result;}
-int igt_dp_mst_sideband_msg_req_decode(void *unused) +static u8 data[] = { 0xff, 0x0, 0xdd };
+const struct drm_dp_sideband_msg_req_body +random_dp_query_enc_client_id(const struct drm_dp_sideband_msg_req_body *in) {
struct drm_dp_sideband_msg_req_body in = { 0 };u8 data[] = { 0xff, 0x0, 0xdd };int i;
struct drm_dp_query_stream_enc_status enc_status = { };-#define DO_TEST() FAIL_ON(!sideband_msg_req_encode_decode(&in))
in.req_type = DP_ENUM_PATH_RESOURCES;in.u.port_num.port_number = 5;DO_TEST();in.req_type = DP_POWER_UP_PHY;in.u.port_num.port_number = 5;DO_TEST();in.req_type = DP_POWER_DOWN_PHY;in.u.port_num.port_number = 5;DO_TEST();in.req_type = DP_ALLOCATE_PAYLOAD;in.u.allocate_payload.number_sdp_streams = 3;for (i = 0; i < in.u.allocate_payload.number_sdp_streams; i++)in.u.allocate_payload.sdp_stream_sink[i] = i + 1;DO_TEST();in.u.allocate_payload.port_number = 0xf;DO_TEST();in.u.allocate_payload.vcpi = 0x7f;DO_TEST();in.u.allocate_payload.pbn = U16_MAX;DO_TEST();in.req_type = DP_QUERY_PAYLOAD;in.u.query_payload.port_number = 0xf;DO_TEST();in.u.query_payload.vcpi = 0x7f;DO_TEST();in.req_type = DP_REMOTE_DPCD_READ;in.u.dpcd_read.port_number = 0xf;DO_TEST();in.u.dpcd_read.dpcd_address = 0xfedcb;DO_TEST();in.u.dpcd_read.num_bytes = U8_MAX;DO_TEST();in.req_type = DP_REMOTE_DPCD_WRITE;in.u.dpcd_write.port_number = 0xf;DO_TEST();in.u.dpcd_write.dpcd_address = 0xfedcb;DO_TEST();in.u.dpcd_write.num_bytes = ARRAY_SIZE(data);in.u.dpcd_write.bytes = data;DO_TEST();in.req_type = DP_REMOTE_I2C_READ;in.u.i2c_read.port_number = 0xf;DO_TEST();in.u.i2c_read.read_i2c_device_id = 0x7f;DO_TEST();in.u.i2c_read.num_transactions = 3;in.u.i2c_read.num_bytes_read = ARRAY_SIZE(data) * 3;for (i = 0; i < in.u.i2c_read.num_transactions; i++) {in.u.i2c_read.transactions[i].bytes = data;in.u.i2c_read.transactions[i].num_bytes = ARRAY_SIZE(data);in.u.i2c_read.transactions[i].i2c_dev_id = 0x7f & ~i;in.u.i2c_read.transactions[i].i2c_transaction_delay = 0xf & ~i;}DO_TEST();in.req_type = DP_REMOTE_I2C_WRITE;in.u.i2c_write.port_number = 0xf;DO_TEST();in.u.i2c_write.write_i2c_device_id = 0x7f;DO_TEST();in.u.i2c_write.num_bytes = ARRAY_SIZE(data);in.u.i2c_write.bytes = data;DO_TEST();in.req_type = DP_QUERY_STREAM_ENC_STATUS;in.u.enc_status.stream_id = 1;DO_TEST();get_random_bytes(in.u.enc_status.client_id,sizeof(in.u.enc_status.client_id));DO_TEST();in.u.enc_status.stream_event = 3;DO_TEST();in.u.enc_status.valid_stream_event = 0;DO_TEST();in.u.enc_status.stream_behavior = 3;DO_TEST();in.u.enc_status.valid_stream_behavior = 1;DO_TEST();-#undef DO_TEST
return 0;
get_random_bytes(enc_status.client_id, sizeof(enc_status.client_id));return (const struct drm_dp_sideband_msg_req_body) { .req_type = in->req_type,.u.enc_status = enc_status};}
+static const struct dp_mst_sideband_msg_req_decode_test dp_msg_sideband_msg_req_decode_tests[] = {
{.req = {.req_type = DP_ENUM_PATH_RESOURCES,.u.port_num.port_number = 5,},},{.req = {.req_type = DP_POWER_UP_PHY,.u.port_num.port_number = 5,},},{.req = {.req_type = DP_POWER_DOWN_PHY,.u.port_num.port_number = 5,},},{.req = {.req_type = DP_ALLOCATE_PAYLOAD,.u.allocate_payload.number_sdp_streams = 3,.u.allocate_payload.sdp_stream_sink = { 1, 2, 3 },},},{.req = {.req_type = DP_ALLOCATE_PAYLOAD,.u.allocate_payload.port_number = 0xf,},},{.req = {.req_type = DP_ALLOCATE_PAYLOAD,.u.allocate_payload.vcpi = 0x7f,},},{.req = {.req_type = DP_ALLOCATE_PAYLOAD,.u.allocate_payload.pbn = U16_MAX,},},{.req = {.req_type = DP_QUERY_PAYLOAD,.u.query_payload.port_number = 0xf,},},{.req = {.req_type = DP_QUERY_PAYLOAD,.u.query_payload.vcpi = 0x7f,},},{.req = {.req_type = DP_REMOTE_DPCD_READ,.u.dpcd_read.port_number = 0xf,},},{.req = {.req_type = DP_REMOTE_DPCD_READ,.u.dpcd_read.dpcd_address = 0xfedcb,},},{.req = {.req_type = DP_REMOTE_DPCD_READ,.u.dpcd_read.num_bytes = U8_MAX,},},{.req = {.req_type = DP_REMOTE_DPCD_WRITE,.u.dpcd_write.port_number = 0xf,},},{.req = {.req_type = DP_REMOTE_DPCD_WRITE,.u.dpcd_write.dpcd_address = 0xfedcb,},},{.req = {.req_type = DP_REMOTE_DPCD_WRITE,.u.dpcd_write.num_bytes = ARRAY_SIZE(data),.u.dpcd_write.bytes = data,},},{.req = {.req_type = DP_REMOTE_I2C_READ,.u.i2c_read.port_number = 0xf,},},{.req = {.req_type = DP_REMOTE_I2C_READ,.u.i2c_read.read_i2c_device_id = 0x7f,},},{.req = {.req_type = DP_REMOTE_I2C_READ,.u.i2c_read.num_transactions = 3,.u.i2c_read.num_bytes_read = ARRAY_SIZE(data) * 3,.u.i2c_read.transactions = {{ .bytes = data, .num_bytes = ARRAY_SIZE(data),.i2c_dev_id = 0x7f, .i2c_transaction_delay = 0xf, },{ .bytes = data, .num_bytes = ARRAY_SIZE(data),.i2c_dev_id = 0x7e, .i2c_transaction_delay = 0xe, },{ .bytes = data, .num_bytes = ARRAY_SIZE(data),.i2c_dev_id = 0x7d, .i2c_transaction_delay = 0xd, },},},},{.req = {.req_type = DP_REMOTE_I2C_WRITE,.u.i2c_write.port_number = 0xf,},},{.req = {.req_type = DP_REMOTE_I2C_WRITE,.u.i2c_write.write_i2c_device_id = 0x7f,},},{.req = {.req_type = DP_REMOTE_I2C_WRITE,.u.i2c_write.num_bytes = ARRAY_SIZE(data),.u.i2c_write.bytes = data,},},{.req = {.req_type = DP_QUERY_STREAM_ENC_STATUS,.u.enc_status.stream_id = 1,},},{.req = {.req_type = DP_QUERY_STREAM_ENC_STATUS,},.f = random_dp_query_enc_client_id,},{.req = {.req_type = DP_QUERY_STREAM_ENC_STATUS,.u.enc_status.stream_event = 3,},},{.req = {.req_type = DP_QUERY_STREAM_ENC_STATUS,.u.enc_status.valid_stream_event = 0,},},{.req = {.req_type = DP_QUERY_STREAM_ENC_STATUS,.u.enc_status.stream_behavior = 3,},},{.req = {.req_type = DP_QUERY_STREAM_ENC_STATUS,.u.enc_status.valid_stream_behavior = 1,},},+};
+KUNIT_ARRAY_PARAM(dp_mst_sideband_msg_req, dp_msg_sideband_msg_req_decode_tests, NULL);
+static struct kunit_case drm_dp_mst_helper_tests[] = {
KUNIT_CASE_PARAM(dp_mst_calc_pbn_mode, dp_mst_calc_pbn_mode_gen_params),KUNIT_CASE_PARAM(dp_mst_sideband_msg_req_decode, dp_mst_sideband_msg_req_gen_params),{}+};
+static struct kunit_suite drm_dp_mst_helper_test_suite = {
.name = "drm_dp_mst_helper_tests",.test_cases = drm_dp_mst_helper_tests,+};
+kunit_test_suite(drm_dp_mst_helper_test_suite); diff --git a/drivers/gpu/drm/selftests/test-drm_modeset_common.h b/drivers/gpu/drm/selftests/test-drm_modeset_common.h index 1501d99aee2f..c7cc5edc65f1 100644 --- a/drivers/gpu/drm/selftests/test-drm_modeset_common.h +++ b/drivers/gpu/drm/selftests/test-drm_modeset_common.h @@ -20,7 +20,5 @@ int igt_drm_rect_clip_scaled_div_by_zero(void *ignored); int igt_drm_rect_clip_scaled_not_clipped(void *ignored); int igt_drm_rect_clip_scaled_clipped(void *ignored); int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored); -int igt_dp_mst_calc_pbn_mode(void *ignored); -int igt_dp_mst_sideband_msg_req_decode(void *ignored);
#endif
2.34.1