On Mon, Sep 30, 2024 at 01:08:41PM +0530, Raag Jadav wrote: > Introduce device wedged event, which will notify userspace of wedged > (hanged/unusable) state of the DRM device through a uevent. This is > useful especially in cases where the device is no longer operating as > expected even after a hardware reset and has become unrecoverable from > driver context. > > Purpose of this implementation is to provide drivers a generic way to > recover with the help of userspace intervention. Different drivers may > have different ideas of a "wedged device" depending on their hardware > implementation, and hence the vendor agnostic nature of the event. > It is up to the drivers to decide when they see the need for recovery > and how they want to recover from the available methods. > > Current implementation defines three recovery methods, out of which, > drivers can choose to support any one or multiple of them. Preferred > recovery method will be sent in the uevent environment as WEDGED=<method>. > Userspace consumers (sysadmin) can define udev rules to parse this event > and take respective action to recover the device. > > =============== ================================== > Recovery method Consumer expectations > =============== ================================== > rebind unbind + rebind driver > bus-reset unbind + reset bus device + rebind > reboot reboot system > =============== ================================== > > v4: s/drm_dev_wedged/drm_dev_wedged_event > Use drm_info() (Jani) > Kernel doc adjustment (Aravind) > v5: Send recovery method with uevent (Lina) > v6: Access wedge_recovery_opts[] using helper function (Jani) > Use snprintf() (Jani) > v7: Convert recovery helpers into regular functions (Andy, Jani) > Aesthetic adjustments (Andy) > Handle invalid method cases > > Signed-off-by: Raag Jadav <raag.jadav@xxxxxxxxx> > --- Cc'ing amd, collabora and others as I found semi-related work at https://lore.kernel.org/dri-devel/20230627132323.115440-1-andrealmeid@xxxxxxxxxx/ https://lore.kernel.org/amd-gfx/20240725150055.1991893-1-alexander.deucher@xxxxxxx/ https://lore.kernel.org/dri-devel/20241011225906.3789965-3-adrian.larumbe@xxxxxxxxxxxxx/ https://lore.kernel.org/amd-gfx/CAAxE2A5v_RkZ9ex4=7jiBSKVb22_1FAj0AANBcmKtETt5c3gVA@xxxxxxxxxxxxxx/ Please share feedback about usefulness and adoption of this. Improvements are welcome. Raag > drivers/gpu/drm/drm_drv.c | 77 +++++++++++++++++++++++++++++++++++++++ > include/drm/drm_device.h | 23 ++++++++++++ > include/drm/drm_drv.h | 3 ++ > 3 files changed, 103 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index ac30b0ec9d93..cfe9600da2ee 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -26,6 +26,8 @@ > * DEALINGS IN THE SOFTWARE. > */ > > +#include <linux/array_size.h> > +#include <linux/build_bug.h> > #include <linux/debugfs.h> > #include <linux/fs.h> > #include <linux/module.h> > @@ -33,6 +35,7 @@ > #include <linux/mount.h> > #include <linux/pseudo_fs.h> > #include <linux/slab.h> > +#include <linux/sprintf.h> > #include <linux/srcu.h> > #include <linux/xarray.h> > > @@ -70,6 +73,42 @@ static struct dentry *drm_debugfs_root; > > DEFINE_STATIC_SRCU(drm_unplug_srcu); > > +/* > + * Available recovery methods for wedged device. To be sent along with device > + * wedged uevent. > + */ > +static const char *const drm_wedge_recovery_opts[] = { > + [DRM_WEDGE_RECOVERY_REBIND] = "rebind", > + [DRM_WEDGE_RECOVERY_BUS_RESET] = "bus-reset", > + [DRM_WEDGE_RECOVERY_REBOOT] = "reboot", > +}; > + > +static bool drm_wedge_recovery_is_valid(enum drm_wedge_recovery method) > +{ > + static_assert(ARRAY_SIZE(drm_wedge_recovery_opts) == DRM_WEDGE_RECOVERY_MAX); > + > + return method >= DRM_WEDGE_RECOVERY_REBIND && method < DRM_WEDGE_RECOVERY_MAX; > +} > + > +/** > + * drm_wedge_recovery_name - provide wedge recovery name > + * @method: method to be used for recovery > + * > + * This validates wedge recovery @method against the available ones in > + * drm_wedge_recovery_opts[] and provides respective recovery name in string > + * format if found valid. > + * > + * Returns: pointer to const recovery string on success, NULL otherwise. > + */ > +const char *drm_wedge_recovery_name(enum drm_wedge_recovery method) > +{ > + if (drm_wedge_recovery_is_valid(method)) > + return drm_wedge_recovery_opts[method]; > + > + return NULL; > +} > +EXPORT_SYMBOL(drm_wedge_recovery_name); > + > /* > * DRM Minors > * A DRM device can provide several char-dev interfaces on the DRM-Major. Each > @@ -497,6 +536,44 @@ void drm_dev_unplug(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_dev_unplug); > > +/** > + * drm_dev_wedged_event - generate a device wedged uevent > + * @dev: DRM device > + * @method: method to be used for recovery > + * > + * This generates a device wedged uevent for the DRM device specified by @dev. > + * Recovery @method from drm_wedge_recovery_opts[] (if supprted by the device) > + * is sent in the uevent environment as WEDGED=<method>, on the basis of which, > + * userspace may take respective action to recover the device. > + * > + * Returns: 0 on success, or negative error code otherwise. > + */ > +int drm_dev_wedged_event(struct drm_device *dev, enum drm_wedge_recovery method) > +{ > + /* Event string length up to 16+ characters with available methods */ > + char event_string[32] = {}; > + char *envp[] = { event_string, NULL }; > + const char *recovery; > + > + recovery = drm_wedge_recovery_name(method); > + if (!recovery) { > + drm_err(dev, "device wedged, invalid recovery method %d\n", method); > + return -EINVAL; > + } > + > + if (!test_bit(method, &dev->wedge_recovery)) { > + drm_err(dev, "device wedged, %s based recovery not supported\n", > + drm_wedge_recovery_name(method)); > + return -EOPNOTSUPP; > + } > + > + snprintf(event_string, sizeof(event_string), "WEDGED=%s", recovery); > + > + drm_info(dev, "device wedged, generating uevent for %s based recovery\n", recovery); > + return kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp); > +} > +EXPORT_SYMBOL(drm_dev_wedged_event); > + > /* > * DRM internal mount > * We want to be able to allocate our own "struct address_space" to control > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > index c91f87b5242d..fed6f20e52fb 100644 > --- a/include/drm/drm_device.h > +++ b/include/drm/drm_device.h > @@ -40,6 +40,26 @@ enum switch_power_state { > DRM_SWITCH_POWER_DYNAMIC_OFF = 3, > }; > > +/** > + * enum drm_wedge_recovery - Recovery method for wedged device in order of > + * severity. To be set as bit fields in drm_device.wedge_recovery variable. > + * Drivers can choose to support any one or multiple of them depending on > + * their needs. > + */ > +enum drm_wedge_recovery { > + /** @DRM_WEDGE_RECOVERY_REBIND: unbind + rebind driver */ > + DRM_WEDGE_RECOVERY_REBIND, > + > + /** @DRM_WEDGE_RECOVERY_BUS_RESET: unbind + reset bus device + rebind */ > + DRM_WEDGE_RECOVERY_BUS_RESET, > + > + /** @DRM_WEDGE_RECOVERY_REBOOT: reboot system */ > + DRM_WEDGE_RECOVERY_REBOOT, > + > + /** @DRM_WEDGE_RECOVERY_MAX: for bounds checking, do not use */ > + DRM_WEDGE_RECOVERY_MAX > +}; > + > /** > * struct drm_device - DRM device structure > * > @@ -317,6 +337,9 @@ struct drm_device { > * Root directory for debugfs files. > */ > struct dentry *debugfs_root; > + > + /** @wedge_recovery: Supported recovery methods for wedged device */ > + unsigned long wedge_recovery; > }; > > #endif > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 02ea4e3248fd..d8dbc77010b0 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -462,6 +462,9 @@ bool drm_dev_enter(struct drm_device *dev, int *idx); > void drm_dev_exit(int idx); > void drm_dev_unplug(struct drm_device *dev); > > +const char *drm_wedge_recovery_name(enum drm_wedge_recovery method); > +int drm_dev_wedged_event(struct drm_device *dev, enum drm_wedge_recovery method); > + > /** > * drm_dev_is_unplugged - is a DRM device unplugged > * @dev: DRM device > -- > 2.34.1 >
