Re: [PATCH v10 1/4] drm: Introduce device wedged event

["André Almeida" <andrealmeid@igalia.com>]

Hi Raag,

Thank you for your patch.

Em 28/11/2024 12:37, Raag Jadav escreveu:

[...]

> +int drm_dev_wedged_event(struct drm_device *dev, unsigned long method)
> +{
> +	const char *recovery = NULL;
> +	unsigned int len, opt;
> +	/* Event string length up to 28+ characters with available methods */
> +	char event_string[32];
> +	char *envp[] = { event_string, NULL };
> +
> +	len = scnprintf(event_string, sizeof(event_string), "%s", "WEDGED=");
> +
> +	for_each_set_bit(opt, &method, BITS_PER_TYPE(method)) {
> +		recovery = drm_get_wedge_recovery(opt);
> +		if (drm_WARN(dev, !recovery, "device wedged, invalid recovery method %u\n", opt))
> +			break;
> +
> +		len += scnprintf(event_string + len, sizeof(event_string), "%s,", recovery);
> +	}
> +
> +	if (recovery)
> +		/* Get rid of trailing comma */
> +		event_string[len - 1] = '\0';
> +	else
> +		/* Caller is unsure about recovery, do the best we can at this point. */
> +		snprintf(event_string, sizeof(event_string), "%s", "WEDGED=unknown");
> +
> +	drm_info(dev, "device wedged, needs recovery\n");

As documented in the commit message "No explicit device recovery is 
expected from the consumer in this case", I think this should be like this:

if (method != DRM_WEDGE_RECOVERY_NONE)
     drm_info(dev, "device wedged, needs recovery\n");

and maybe a note like this:

else
     drm_info(dev, "device reseted, but managed to recover\n");

Either way, this patch is:

Reviewed-by: André Almeida <andrealmeid@igalia.com>