Re: [PATCH i-g-t v10 31/49] tests/unigraf: Add basic unigraf tests

[Louis Chauvet <louis.chauvet@bootlin.com>]

On 4/20/26 18:49, Kory Maincent wrote:
> On Tue, 31 Mar 2026 19:11:48 +0200
> Louis Chauvet <louis.chauvet@bootlin.com> wrote:
> 
>> Adds two tests to validate unigraf communication.
>>
>> unigraf-connect checks if the device is connected and if the EDID is
>> properly applied.
>>
>> ungiraf-reconnect checks if the device can be connected/reconnected
>> using SST and MST configurations.
>>
> 
> Only nitpicking here.
> 
> With these fixed you can add my Rd-by.
> 
> ...
> 
>> +int igt_main()
>> +{
>> +	int drm_fd;
>> +
>> +	igt_fixture() {
>> +		drm_fd = drm_open_driver_master(DRIVER_ANY);
>> +	}
>> +
>> +	igt_describe("Make sure that the unigraf device is connected to the
>> DUT and EDID is properly detected");
>> +	igt_subtest("unigraf-connect-edid") {
>> +		drmModePropertyBlobPtr edid_blob = NULL;
>> +		struct igt_display display;
>> +		uint64_t edid_blob_id;
>> +		igt_output_t *output;
>> +		uint32_t unigraf_edid_len;
>> +		void *unigraf_edid;
>> +		bool found = false;
>> +
>> +		/*
>> +		 * Sleep are required to allow hardware to configure/detect
>> the current
>> +		 * configuration
>> +		 */
>> +		unigraf_require_device(drm_fd);
>> +		unigraf_hpd_deassert();
>> +		sleep(igt_default_display_detect_timeout());
>> +		unigraf_set_sst();
>> +		unigraf_hpd_assert();
>> +		sleep(igt_default_display_detect_timeout());
>> +		igt_display_require(&display, drm_fd);
>> +		sleep(igt_default_display_detect_timeout());
>> +
>> +		unigraf_edid = unigraf_read_edid(0, &unigraf_edid_len);
>> +
>> +		for_each_connected_output(&display, output) {
>> +			if (output->config.connector->connector_type ==
>> +			    DRM_MODE_CONNECTOR_DisplayPort) {
> 
> Will there be other connector type check added in the future?
> Else you could simply do:
> if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
> 	continue;

Some unigraf devices supports HDMI too, and AFAIU, they use the same 
library. But I will switch, your proposition is easier to understand and 
it is easy to switch back when an other person wants to implement HDMI.

>> +				igt_assert(kmstest_get_property(drm_fd,
>> +
>> output->config.connector->connector_id,
>> +
>> DRM_MODE_OBJECT_CONNECTOR, "EDID",
>> +								NULL,
>> &edid_blob_id, NULL));
>> +				edid_blob = drmModeGetPropertyBlob(drm_fd,
>> edid_blob_id);
>> +				if (!edid_blob)
>> +					continue;
>> +
>> +				if (!memcmp(unigraf_edid, edid_blob->data,
>> +					    min(edid_blob->length,
>> unigraf_edid_len)))
>> +					found = true;
>> +
>> +				drmModeFreePropertyBlob(edid_blob);
>> +
>> +				if (found)
>> +					break;
>> +			}
>> +		}
>> +		igt_assert_f(found, "No output with the correct EDID was
>> found\n"); +
>> +		free(unigraf_edid);
>> +	}
>> +
>> +	igt_describe("Make sure that the unigraf device can be used as a MST
>> device");
>> +	igt_subtest("unigraf-connect-mst") {
>> +		int newly_connected_count, already_connected_count, diff_len;
>> +		uint32_t *newly_connected = NULL, *already_connected = NULL,
>> *diff = NULL;
>> +		int max_count;
>> +
>> +		unigraf_require_device(drm_fd);
>> +		max_count = unigraf_get_mst_stream_max_count();
>> +
>> +		unigraf_hpd_deassert();
>> +
>> +		already_connected_count =
>> igt_get_connected_connectors(drm_fd, &already_connected); +
>> +		igt_debug("Already connected count: %d\n",
>> already_connected_count); +
>> +		// i = 0 is SST so we need to process max_count + 1 streams
> 
> /* ... */
> I don't know the igt rules but in kernel /* ... */ is preferable.
> 
>> +		for (int i = 0; i <= max_count; i++) {
>> +			unigraf_hpd_deassert();
>> +			// Let the hardware detect the new state
> 
> Same.
> 
>> +			sleep(igt_default_display_detect_timeout());
>> +
>> +			unigraf_set_mst_stream_count(max(i, 1));
>> +			if (!i)
>> +				unigraf_set_sst();
>> +			else
>> +				unigraf_set_mst();
>> +
>> +			unigraf_hpd_assert();
>> +			// Let the hardware detect the new state
> 
> Same
> 
>> +			sleep(igt_default_display_detect_timeout());
>> +
>> +			newly_connected_count =
>> kms_wait_for_new_connectors(&newly_connected,
>> +
>> already_connected,
>> +
>> already_connected_count,
>> +
>> drm_fd); +
>> +			diff_len = get_array_diff(newly_connected,
>> newly_connected_count,
>> +						  already_connected,
>> already_connected_count, &diff); +
>> +			igt_assert_f(diff_len == max(i, 1),
>> +				     "Invalid connected connector count,
>> expected %d found %d\n",
>> +				     max(i, 1), diff_len);
>> +		}
>> +	}
>> +}
>>
> 
> 
>