On Mon, Nov 2, 2020 at 1:40 PM Thomas Zimmermann tzimmermann@suse.de wrote:
Hi
Am 02.11.20 um 13:09 schrieb Thomas Zimmermann:
Hi
Am 02.11.20 um 12:19 schrieb Tian Tao:
Add the detection of false for irq, so that the EINVAL is not returned when dev->irq_enabled is false.
Signed-off-by: Tian Tao tiantao6@hisilicon.com
drivers/gpu/drm/drm_irq.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 09d6e9e..7537a3d 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev) bool irq_enabled; int i;
- if (!dev->irq_enabled || !dev)
return 0;Dereferencing a pointer before NULL-checking it, is Yoda programming. :) I'd just drop the test for !dev and assume that dev is always valid.
I suggest to change the int return value to void and return nothing.
Re-reading the actual implementation of this function, this location might be too early. Further below there already is a test for irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way the vblank handlers will be disabled and erroneous callers will see a warning in the kernel's log.
In addition to these changes, you could also add a managed implementation of drm_irq_install(). The canonical name should be devm_drm_irq_install(). The function would call drm_irq_install() and register a cleanup handler via devm_add_action(). Example code is at [1].
KMS drivers, such as hibmc, would then not have to bother about uninstalling the DRM irq.
Yup, devm_ is the right fix here imo, not trying to make the uninstall hook resilient against drivers which can't keep track of stuff. So if that's all there is, imo this patch is a bad idea, since we have proper tools to make driver unloading easier on driver author's nowadays. -Daniel
Best regards Thomas
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
Best regards Thomas
irq_enabled = dev->irq_enabled; dev->irq_enabled = false;
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer