[Intel-gfx] [RFC PATCH v3 1/4] drm/i915: add i915_ved.c to setup bridge for VED

[Cheng, Yao]

> -----Original Message-----
> From: Beckett, Robert
> Sent: Saturday, November 29, 2014 0:59
> To: Cheng, Yao; intel-gfx at lists.freedesktop.org; dri-
> devel at lists.freedesktop.org; daniel.vetter at ffwll.ch; Kelley, Sean V; Chehab,
> John
> Cc: emil.l.velikov at gmail.com; Jiang, Fei; dh.herrmann at gmail.com;
> daniel at fooishbar.org
> Subject: Re: [Intel-gfx] [RFC PATCH v3 1/4] drm/i915: add i915_ved.c to setup
> bridge for VED
> > + * Threats:
> > + * Due to the restriction in Linux platform device model, user need
> > +manually
> > + * uninstall ipvr driver before uninstalling i915 module, otherwise
> > +he might
> > + * run into use-after-free issues after i915 removes the platform device.
> 
> Can you go in to more detail on what you consider to be the restriction in the
> platform device model?
> 
> When removing the device via platform_device_unregister, it will call the
> remove callback of any drivers handling this device (via the bus remove
> function). It is then up to the driver to ensure no further usage of the device
> from which it is being removed. This usually involves removing all user input
> vectors, disabling interrupts involved and flushing/canceling any delayed
> work. This should prevent any further use of the device by the driver.
> 
> The driver's remove function is called in a direct call chain from
> platform_device_unregister, so by the time it returns, there should be no
> further chance of accesses.
>

Bob, thx for your review comments.
The symptom is, after "rmmod i915", though drm_drv_release() is also called on the child device "ipvr", I still see the module exist in the system (check it by "lsmod"). This causes issue when I modprobe i915 and ipvr again later. Actually I don't understand why this restriction exists, but accroding to Daniel, grabbing a module refcount for the platform device doesn't work (it would pin the module forever).
 
> > +void vlv_teardown_ved(struct drm_device *dev) {
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> 
> you may want to mask the i915 interrupt for the VED block before removing
> the device.
> 

Thx, will add it.

> > +	vlv_ved_platdev_destroy(dev);
> > +	if (dev_priv->ved.irq >= 0)
> > +		irq_free_desc(dev_priv->ved.irq);
> > +}
> >
> 
> 
> Generally it is a nicely interfaced child device.
>