[igt-dev] [PATCH] tests/i915/i915_pm_rpm: skip i2c subtest on VGA subconnector

[Ser, Simon]

On Mon, 2019-08-26 at 16:25 +0300, Oleg Vasilev wrote:
> The subtest compares EDID obtained through I2C with the one thorugh DRM
> interface. Getting EDID through I2C on VGA is not reliable, and the test
> should be skipped. However, on some machines VGA connector is
> implemented as DP branch device. More background in an fdo bug [1].
> 
> The patch enables skips of the test based on a subconnector property,
> introduced in the kernel series [2].
> 
> [1]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
> [2]: https://patchwork.freedesktop.org/series/65800/
> 
> CC: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> CC: Simon Ser <simon.ser at intel.com>
> Signed-off-by: Oleg Vasilev <oleg.vasilev at intel.com>
> ---
>  tests/i915/i915_pm_rpm.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 2168ff72..5c512597 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -389,6 +389,41 @@ static drmModePropertyBlobPtr get_connector_edid(drmModeConnectorPtr connector,
>  	return blob;
>  }
>  
> +static bool is_vga(drmModeConnectorPtr connector)
> +{
> +	bool found;
> +	uint64_t prop_value;
> +	drmModePropertyPtr prop;
> +	char *enum_name = NULL;
> +	bool is_subconnector_vga = false;
> +
> +	if (connector->connector_type == DRM_MODE_CONNECTOR_VGA)
> +		return true;
> +
> +	if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
> +		return false;

Question: is it necessary to check that the connector type is
DisplayPort?

> +	found = kmstest_get_property(drm_fd, connector->connector_id,
> +				     DRM_MODE_OBJECT_CONNECTOR, "subconnector",
> +				     NULL, &prop_value, &prop);

prop is leaked, we need to drmModeFreeProperty it.

With that fixed:

Reviewed-by: Simon Ser <simon.ser at intel.com>

> +
> +	igt_assert(found);
> +	igt_assert(prop->flags & DRM_MODE_PROP_ENUM);
> +
> +	for (int i = 0; i < prop->count_enums; i++) {
> +		if (prop->enums[i].value == prop_value) {
> +			enum_name = prop->enums[i].name;
> +			break;
> +		}
> +	}
> +	igt_assert(enum_name);
> +
> +	is_subconnector_vga = (strcmp(enum_name, "VGA") == 0);
> +	if (is_subconnector_vga)
> +		igt_debug("This DP is in fact VGA, skipping\n");
> +	return is_subconnector_vga;
> +}
> +
>  static void init_mode_set_data(struct mode_set_data *data)
>  {
>  	data->res = drmModeGetResources(drm_fd);
> @@ -679,7 +714,6 @@ static void test_i2c(struct mode_set_data *data)
>  
>  		bool got_i2c_edid = i2c_read_edid(connector_name, i2c_edid);
>  		bool got_drm_edid = drm_edid != NULL;
> -		bool is_vga = data->connectors[i]->connector_type == DRM_MODE_CONNECTOR_VGA;
>  
>  		bool edids_equal;
>  
> @@ -688,7 +722,7 @@ static void test_i2c(struct mode_set_data *data)
>  		 * the EDID reading mostly FFs and then disabling bit-banging. Since we
>  		 * don't want to reimplement everything the Kernel does, let's just
>  		 * accept the fact that some VGA outputs won't be properly detected. */
> -		if (is_vga)
> +		if (is_vga(data->connectors[i]))
>  			continue;
>  
>  		if (!got_i2c_edid && !got_drm_edid)