Re: [PATCH v5 2/4] drm: Expose wedge recovery methods

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 17 Sep 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> Now that we have device wedged event in place, add wedge_recovery sysfs
> attribute which will expose recovery methods supported by the DRM device.
> This is useful for userspace consumers in cases where the device supports
> multiple recovery methods which can be used as fallbacks.
>
> $ cat /sys/class/drm/card0/wedge_recovery
> rebind
> bus-reset
> reboot
>
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index fb3bbb6adcd1..b88cdbfa3b5e 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -36,6 +36,8 @@
>  #define to_drm_minor(d) dev_get_drvdata(d)
>  #define to_drm_connector(d) dev_get_drvdata(d)
>  
> +extern const char *const wedge_recovery_opts[];

Data is not an interface. Please add a function for this.

Side note, extern declarations for outside stuff don't belong in .c
files anyway.

BR,
Jani.

> +
>  /**
>   * DOC: overview
>   *
> @@ -508,6 +510,26 @@ void drm_sysfs_connector_property_event(struct drm_connector *connector,
>  }
>  EXPORT_SYMBOL(drm_sysfs_connector_property_event);
>  
> +static ssize_t wedge_recovery_show(struct device *device,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct drm_minor *minor = to_drm_minor(device);
> +	struct drm_device *dev = minor->dev;
> +	int opt, count = 0;
> +
> +	for_each_set_bit(opt, &dev->wedge_recovery, DRM_WEDGE_RECOVERY_MAX)
> +		count += sysfs_emit_at(buf, count, "%s\n", wedge_recovery_opts[opt]);
> +
> +	return count;
> +}
> +static DEVICE_ATTR_RO(wedge_recovery);
> +
> +static struct attribute *minor_dev_attrs[] = {
> +	&dev_attr_wedge_recovery.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(minor_dev);
> +
>  struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
>  {
>  	const char *minor_str;
> @@ -532,6 +554,7 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
>  		kdev->devt = MKDEV(DRM_MAJOR, minor->index);
>  		kdev->class = drm_class;
>  		kdev->type = &drm_sysfs_device_minor;
> +		kdev->groups = minor_dev_groups;
>  	}
>  
>  	kdev->parent = minor->dev->dev;

-- 
Jani Nikula, Intel