Re: [PATCH] drm: Support drivers with threaded irqs
Hi Thomas, [auto build test WARNING on drm/drm-next] [also build test WARNING on v4.13-rc4 next-20170810] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sinclair-Yeh/drm-Support-drivers-with-threaded-irqs/20170811-025749 base: git://people.freedesktop.org/~airlied/linux.git drm-next reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/linux/init.h:1: warning: no structured comments found include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_major' description in 'fsl_mc_device_id' include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_minor' description in 'fsl_mc_device_id' kernel/sched/core.c:2080: warning: No description found for parameter 'rf' kernel/sched/core.c:2080: warning: Excess function parameter 'cookie' description in 'try_to_wake_up_local' include/linux/wait.h:555: warning: No description found for parameter 'wq' include/linux/wait.h:555: warning: Excess function parameter 'wq_head' description in 'wait_event_interruptible_hrtimeout' include/linux/wait.h:759: warning: No description found for parameter 'wq_head' include/linux/wait.h:759: warning: Excess function parameter 'wq' description in 'wait_event_killable' include/linux/kthread.h:26: warning: Excess function parameter '...' description in 'kthread_create' kernel/sys.c:1: warning: no structured comments found include/linux/device.h:968: warning: No description found for parameter 'dma_ops' drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found include/linux/sync_file.h:51: warning: No description found for parameter 'flags' include/linux/iio/iio.h:603: warning: No description found for parameter 'trig_readonly' include/linux/iio/trigger.h:151: warning: No description found for parameter 'indio_dev' include/linux/iio/trigger.h:151: warning: No description found for parameter 'trig' include/linux/device.h:969: warning: No description found for parameter 'dma_ops' drivers/ata/libata-eh.c:1449: warning: No description found for parameter 'link' drivers/ata/libata-eh.c:1449: warning: Excess function parameter 'ap' description in 'ata_eh_done' drivers/ata/libata-eh.c:1590: warning: No description found for parameter 'qc' drivers/ata/libata-eh.c:1590: warning: Excess function parameter 'dev' description in 'ata_eh_request_sense' drivers/mtd/nand/nand_base.c:2751: warning: Excess function parameter 'cached' description in 'nand_write_page' drivers/mtd/nand/nand_base.c:2751: warning: Excess function parameter 'cached' description in 'nand_write_page' arch/s390/include/asm/cmb.h:1: warning: no structured comments found drivers/scsi/scsi_lib.c:1116: warning: No description found for parameter 'rq' drivers/scsi/constants.c:1: warning: no structured comments found include/linux/usb/gadget.h:230: warning: No description found for parameter 'claimed' include/linux/usb/gadget.h:230: warning: No description found for parameter 'enabled' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_altset_not_supp' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_stall_not_supp' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_zlp_not_supp' fs/inode.c:1666: warning: No description found for parameter 'rcu' include/linux/jbd2.h:443: warning: No description found for parameter 'i_transaction' include/linux/jbd2.h:443: warning: No description found for parameter 'i_next_transaction' include/linux/jbd2.h:443: warning: No description found for parameter 'i_list' include/linux/jbd2.h:443: warning: No description found for parameter 'i_vfs_inode' include/linux/jbd2.h:443: warning: No description found for parameter 'i_flags' include/linux/jbd2.h:497: warning: No description found for parameter 'h_rsv_handle' include/linux/jbd2.h:497: warning: No description found for parameter 'h_reserved' include/linux/jbd2.h:497: warning: No description found for parameter 'h_type' include/linux/jbd2.h:497: warning: No description found for parameter 'h_line_no' include/linux/jbd2.h:497: warning: No description found for parameter 'h_start_jiffies' include/linux/jbd2.h:497: warning: No description found for parameter 'h_requested_credits' include/linux/jbd2.h:497: warning: No description found for parameter 'saved_alloc_context' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chkpt_bhs' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_devname' include/linux/jbd2.h:1050: warning: No description found for parameter
Re: [PATCH] drm: Support drivers with threaded irqs
On 08/09/2017 08:00 PM, Daniel Vetter wrote: On Wed, Aug 9, 2017 at 7:50 PM, Thomas Hellstromwrote: On 08/09/2017 07:40 PM, Daniel Vetter wrote: On Wed, Aug 09, 2017 at 07:38:29PM +0200, Daniel Vetter wrote: On Wed, Aug 09, 2017 at 07:17:54PM +0200, Sinclair Yeh wrote: From: Thomas Hellstrom See LWN article at https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_302043_=DwIBAg=uilaK90D4TOVoH58JNXRgQ=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ=NYyVOtVu5iQrNCGt-QcVtR_IU0lhhztOmFP3qnN88tc=ruWozYT16Yk3p7sIscOFDNKCC1_WHX_7ChY7q5Ko4vw= Signed-off-by: Thomas Hellstrom Reviewed-by: Deepak Singh Rawat Reviewed-by: Sinclair Yeh We definitely can't do this in general due to latency reasons. Please read the kerneldoc of drm_irq_install and just roll your own. This also seems to like break every driver's compilation. Oops misread, I thought you rename stuff instead of adding. I still don't see the point - drm_irq_install happened because of the *bsd abstraction layer, not because it's actually all that useful. -Daniel So the whole point of this is to replace the vmwgfx tasklets with treaded irqs to be nicer on the system. Seems like no other drivers use tasklets at this point. The vmwgfx driver is using drm_irq_install(), Of course we could duplicate that code into the driver, but I don't see the point in doing that nor how this would violate what's written in the drm_irq_install kerneldoc? It just extends its functionality somewhat. Ok, it only says you should roll your own if your simple needs extend to needing multiple devices, but threaded interrupts, priorities, whatever else also belongs in there. The only reason we had this abstraction was really only the *bsd stuff and maybe ums. Simply open-coding is imo much better, since then you can even do stuff like using the devm_ variants and other bits. It's not so bad that I'd outright remove it all (there's better things to clean up), but imo really no point extending it. That's pretty much the same answer as when people wanted to extend this to multiple devices 3 years ago, again with "it's just a minor extension". -Daniel OK. We'll roll our own. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Support drivers with threaded irqs
On 08/09/2017 07:40 PM, Daniel Vetter wrote: On Wed, Aug 09, 2017 at 07:38:29PM +0200, Daniel Vetter wrote: On Wed, Aug 09, 2017 at 07:17:54PM +0200, Sinclair Yeh wrote: From: Thomas HellstromSee LWN article at https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_302043_=DwIBAg=uilaK90D4TOVoH58JNXRgQ=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ=NYyVOtVu5iQrNCGt-QcVtR_IU0lhhztOmFP3qnN88tc=ruWozYT16Yk3p7sIscOFDNKCC1_WHX_7ChY7q5Ko4vw= Signed-off-by: Thomas Hellstrom Reviewed-by: Deepak Singh Rawat Reviewed-by: Sinclair Yeh We definitely can't do this in general due to latency reasons. Please read the kerneldoc of drm_irq_install and just roll your own. This also seems to like break every driver's compilation. Oops misread, I thought you rename stuff instead of adding. I still don't see the point - drm_irq_install happened because of the *bsd abstraction layer, not because it's actually all that useful. -Daniel So the whole point of this is to replace the vmwgfx tasklets with treaded irqs to be nicer on the system. Seems like no other drivers use tasklets at this point. The vmwgfx driver is using drm_irq_install(), Of course we could duplicate that code into the driver, but I don't see the point in doing that nor how this would violate what's written in the drm_irq_install kerneldoc? It just extends its functionality somewhat. Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Support drivers with threaded irqs
On Wed, Aug 9, 2017 at 7:50 PM, Thomas Hellstromwrote: > On 08/09/2017 07:40 PM, Daniel Vetter wrote: >> >> On Wed, Aug 09, 2017 at 07:38:29PM +0200, Daniel Vetter wrote: >>> >>> On Wed, Aug 09, 2017 at 07:17:54PM +0200, Sinclair Yeh wrote: From: Thomas Hellstrom See LWN article at https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_302043_=DwIBAg=uilaK90D4TOVoH58JNXRgQ=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ=NYyVOtVu5iQrNCGt-QcVtR_IU0lhhztOmFP3qnN88tc=ruWozYT16Yk3p7sIscOFDNKCC1_WHX_7ChY7q5Ko4vw= Signed-off-by: Thomas Hellstrom Reviewed-by: Deepak Singh Rawat Reviewed-by: Sinclair Yeh >>> >>> We definitely can't do this in general due to latency reasons. Please >>> read the kerneldoc of drm_irq_install and just roll your own. This also >>> seems to like break every driver's compilation. >> >> Oops misread, I thought you rename stuff instead of adding. I still don't >> see the point - drm_irq_install happened because of the *bsd abstraction >> layer, not because it's actually all that useful. >> -Daniel > > > So the whole point of this is to replace the vmwgfx tasklets with treaded > irqs to be nicer on the system. Seems like no other drivers use tasklets at > this point. > > The vmwgfx driver is using drm_irq_install(), Of course we could duplicate > that code into the driver, but I don't see the point in doing that nor how > this would violate what's written in the drm_irq_install kerneldoc? It just > extends its functionality somewhat. Ok, it only says you should roll your own if your simple needs extend to needing multiple devices, but threaded interrupts, priorities, whatever else also belongs in there. The only reason we had this abstraction was really only the *bsd stuff and maybe ums. Simply open-coding is imo much better, since then you can even do stuff like using the devm_ variants and other bits. It's not so bad that I'd outright remove it all (there's better things to clean up), but imo really no point extending it. That's pretty much the same answer as when people wanted to extend this to multiple devices 3 years ago, again with "it's just a minor extension". -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Support drivers with threaded irqs
On Wed, Aug 09, 2017 at 07:38:29PM +0200, Daniel Vetter wrote: > On Wed, Aug 09, 2017 at 07:17:54PM +0200, Sinclair Yeh wrote: > > From: Thomas Hellstrom> > > > See LWN article at > > https://lwn.net/Articles/302043/ > > > > Signed-off-by: Thomas Hellstrom > > Reviewed-by: Deepak Singh Rawat > > Reviewed-by: Sinclair Yeh > > We definitely can't do this in general due to latency reasons. Please > read the kerneldoc of drm_irq_install and just roll your own. This also > seems to like break every driver's compilation. Oops misread, I thought you rename stuff instead of adding. I still don't see the point - drm_irq_install happened because of the *bsd abstraction layer, not because it's actually all that useful. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Support drivers with threaded irqs
On Wed, Aug 09, 2017 at 07:17:54PM +0200, Sinclair Yeh wrote: > From: Thomas Hellstrom> > See LWN article at > https://lwn.net/Articles/302043/ > > Signed-off-by: Thomas Hellstrom > Reviewed-by: Deepak Singh Rawat > Reviewed-by: Sinclair Yeh We definitely can't do this in general due to latency reasons. Please read the kerneldoc of drm_irq_install and just roll your own. This also seems to like break every driver's compilation. Thanks, Daniel > --- > drivers/gpu/drm/drm_irq.c | 6 -- > include/drm/drm_drv.h | 1 + > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 3b04c2510..ef9b4be 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -127,8 +127,10 @@ int drm_irq_install(struct drm_device *dev, int irq) > if (drm_core_check_feature(dev, DRIVER_IRQ_SHARED)) > sh_flags = IRQF_SHARED; > > - ret = request_irq(irq, dev->driver->irq_handler, > - sh_flags, dev->driver->name, dev); > + ret = request_threaded_irq(irq, > +dev->driver->irq_handler, > +dev->driver->irq_thread_fn, > +sh_flags, dev->driver->name, dev); > > if (ret < 0) { > dev->irq_enabled = false; > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 81971dc..c47abbf 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -333,6 +333,7 @@ struct drm_driver { >* drivers which implement their own interrupt handling. >*/ > irqreturn_t(*irq_handler) (int irq, void *arg); > + irqreturn_t (*irq_thread_fn)(int irq, void *arg); > > /** >* @irq_preinstall: > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Support drivers with threaded irqs
From: Thomas HellstromSee LWN article at https://lwn.net/Articles/302043/ Signed-off-by: Thomas Hellstrom Reviewed-by: Deepak Singh Rawat Reviewed-by: Sinclair Yeh --- drivers/gpu/drm/drm_irq.c | 6 -- include/drm/drm_drv.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 3b04c2510..ef9b4be 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -127,8 +127,10 @@ int drm_irq_install(struct drm_device *dev, int irq) if (drm_core_check_feature(dev, DRIVER_IRQ_SHARED)) sh_flags = IRQF_SHARED; - ret = request_irq(irq, dev->driver->irq_handler, - sh_flags, dev->driver->name, dev); + ret = request_threaded_irq(irq, + dev->driver->irq_handler, + dev->driver->irq_thread_fn, + sh_flags, dev->driver->name, dev); if (ret < 0) { dev->irq_enabled = false; diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 81971dc..c47abbf 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -333,6 +333,7 @@ struct drm_driver { * drivers which implement their own interrupt handling. */ irqreturn_t(*irq_handler) (int irq, void *arg); + irqreturn_t (*irq_thread_fn)(int irq, void *arg); /** * @irq_preinstall: -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel