On Tue, Aug 03, 2021 at 11:06:52AM +0200, Thomas Zimmermann wrote: > Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's > IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers > don't benefit from using it. > > DRM IRQ callbacks are now being called directly or inlined. > > Calls to platform_get_irq() can fail with a negative errno code. > Abort initialization in this case. The DRM IRQ midlayer does not > handle this case correctly. > > v2: > * name struct drm_device variables 'drm' (Sam) > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx> Sorry for the delayed response due to holidays. Acked-by: Liviu Dudau <liviu.dudau@xxxxxxx> Best regards, Liviu > --- > drivers/gpu/drm/arm/hdlcd_drv.c | 174 ++++++++++++++++++-------------- > drivers/gpu/drm/arm/hdlcd_drv.h | 1 + > 2 files changed, 97 insertions(+), 78 deletions(-) > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > index 81ae92390736..479c2422a2e0 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -29,7 +29,6 @@ > #include <drm/drm_fb_helper.h> > #include <drm/drm_gem_cma_helper.h> > #include <drm/drm_gem_framebuffer_helper.h> > -#include <drm/drm_irq.h> > #include <drm/drm_modeset_helper.h> > #include <drm/drm_of.h> > #include <drm/drm_probe_helper.h> > @@ -38,6 +37,94 @@ > #include "hdlcd_drv.h" > #include "hdlcd_regs.h" > > +static irqreturn_t hdlcd_irq(int irq, void *arg) > +{ > + struct drm_device *drm = arg; > + struct hdlcd_drm_private *hdlcd = drm->dev_private; > + unsigned long irq_status; > + > + irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS); > + > +#ifdef CONFIG_DEBUG_FS > + if (irq_status & HDLCD_INTERRUPT_UNDERRUN) > + atomic_inc(&hdlcd->buffer_underrun_count); > + > + if (irq_status & HDLCD_INTERRUPT_DMA_END) > + atomic_inc(&hdlcd->dma_end_count); > + > + if (irq_status & HDLCD_INTERRUPT_BUS_ERROR) > + atomic_inc(&hdlcd->bus_error_count); > + > + if (irq_status & HDLCD_INTERRUPT_VSYNC) > + atomic_inc(&hdlcd->vsync_count); > + > +#endif > + if (irq_status & HDLCD_INTERRUPT_VSYNC) > + drm_crtc_handle_vblank(&hdlcd->crtc); > + > + /* acknowledge interrupt(s) */ > + hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, irq_status); > + > + return IRQ_HANDLED; > +} > + > +static void hdlcd_irq_preinstall(struct drm_device *drm) > +{ > + struct hdlcd_drm_private *hdlcd = drm->dev_private; > + /* Ensure interrupts are disabled */ > + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0); > + hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0); > +} > + > +static void hdlcd_irq_postinstall(struct drm_device *drm) > +{ > +#ifdef CONFIG_DEBUG_FS > + struct hdlcd_drm_private *hdlcd = drm->dev_private; > + unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK); > + > + /* enable debug interrupts */ > + irq_mask |= HDLCD_DEBUG_INT_MASK; > + > + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask); > +#endif > +} > + > +static int hdlcd_irq_install(struct drm_device *drm, int irq) > +{ > + int ret; > + > + if (irq == IRQ_NOTCONNECTED) > + return -ENOTCONN; > + > + hdlcd_irq_preinstall(drm); > + > + ret = request_irq(irq, hdlcd_irq, 0, drm->driver->name, drm); > + if (ret) > + return ret; > + > + hdlcd_irq_postinstall(drm); > + > + return 0; > +} > + > +static void hdlcd_irq_uninstall(struct drm_device *drm) > +{ > + struct hdlcd_drm_private *hdlcd = drm->dev_private; > + /* disable all the interrupts that we might have enabled */ > + unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK); > + > +#ifdef CONFIG_DEBUG_FS > + /* disable debug interrupts */ > + irq_mask &= ~HDLCD_DEBUG_INT_MASK; > +#endif > + > + /* disable vsync interrupts */ > + irq_mask &= ~HDLCD_INTERRUPT_VSYNC; > + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask); > + > + free_irq(hdlcd->irq, drm); > +} > + > static int hdlcd_load(struct drm_device *drm, unsigned long flags) > { > struct hdlcd_drm_private *hdlcd = drm->dev_private; > @@ -90,7 +177,12 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags) > goto setup_fail; > } > > - ret = drm_irq_install(drm, platform_get_irq(pdev, 0)); > + ret = platform_get_irq(pdev, 0); > + if (ret < 0) > + goto irq_fail; > + hdlcd->irq = ret; > + > + ret = hdlcd_irq_install(drm, hdlcd->irq); > if (ret < 0) { > DRM_ERROR("failed to install IRQ handler\n"); > goto irq_fail; > @@ -122,76 +214,6 @@ static void hdlcd_setup_mode_config(struct drm_device *drm) > drm->mode_config.funcs = &hdlcd_mode_config_funcs; > } > > -static irqreturn_t hdlcd_irq(int irq, void *arg) > -{ > - struct drm_device *drm = arg; > - struct hdlcd_drm_private *hdlcd = drm->dev_private; > - unsigned long irq_status; > - > - irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS); > - > -#ifdef CONFIG_DEBUG_FS > - if (irq_status & HDLCD_INTERRUPT_UNDERRUN) > - atomic_inc(&hdlcd->buffer_underrun_count); > - > - if (irq_status & HDLCD_INTERRUPT_DMA_END) > - atomic_inc(&hdlcd->dma_end_count); > - > - if (irq_status & HDLCD_INTERRUPT_BUS_ERROR) > - atomic_inc(&hdlcd->bus_error_count); > - > - if (irq_status & HDLCD_INTERRUPT_VSYNC) > - atomic_inc(&hdlcd->vsync_count); > - > -#endif > - if (irq_status & HDLCD_INTERRUPT_VSYNC) > - drm_crtc_handle_vblank(&hdlcd->crtc); > - > - /* acknowledge interrupt(s) */ > - hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, irq_status); > - > - return IRQ_HANDLED; > -} > - > -static void hdlcd_irq_preinstall(struct drm_device *drm) > -{ > - struct hdlcd_drm_private *hdlcd = drm->dev_private; > - /* Ensure interrupts are disabled */ > - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0); > - hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0); > -} > - > -static int hdlcd_irq_postinstall(struct drm_device *drm) > -{ > -#ifdef CONFIG_DEBUG_FS > - struct hdlcd_drm_private *hdlcd = drm->dev_private; > - unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK); > - > - /* enable debug interrupts */ > - irq_mask |= HDLCD_DEBUG_INT_MASK; > - > - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask); > -#endif > - return 0; > -} > - > -static void hdlcd_irq_uninstall(struct drm_device *drm) > -{ > - struct hdlcd_drm_private *hdlcd = drm->dev_private; > - /* disable all the interrupts that we might have enabled */ > - unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK); > - > -#ifdef CONFIG_DEBUG_FS > - /* disable debug interrupts */ > - irq_mask &= ~HDLCD_DEBUG_INT_MASK; > -#endif > - > - /* disable vsync interrupts */ > - irq_mask &= ~HDLCD_INTERRUPT_VSYNC; > - > - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask); > -} > - > #ifdef CONFIG_DEBUG_FS > static int hdlcd_show_underrun_count(struct seq_file *m, void *arg) > { > @@ -236,10 +258,6 @@ DEFINE_DRM_GEM_CMA_FOPS(fops); > > static const struct drm_driver hdlcd_driver = { > .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > - .irq_handler = hdlcd_irq, > - .irq_preinstall = hdlcd_irq_preinstall, > - .irq_postinstall = hdlcd_irq_postinstall, > - .irq_uninstall = hdlcd_irq_uninstall, > DRM_GEM_CMA_DRIVER_OPS, > #ifdef CONFIG_DEBUG_FS > .debugfs_init = hdlcd_debugfs_init, > @@ -316,7 +334,7 @@ static int hdlcd_drm_bind(struct device *dev) > err_unload: > of_node_put(hdlcd->crtc.port); > hdlcd->crtc.port = NULL; > - drm_irq_uninstall(drm); > + hdlcd_irq_uninstall(drm); > of_reserved_mem_device_release(drm->dev); > err_free: > drm_mode_config_cleanup(drm); > @@ -338,7 +356,7 @@ static void hdlcd_drm_unbind(struct device *dev) > hdlcd->crtc.port = NULL; > pm_runtime_get_sync(dev); > drm_atomic_helper_shutdown(drm); > - drm_irq_uninstall(drm); > + hdlcd_irq_uninstall(drm); > pm_runtime_put(dev); > if (pm_runtime_enabled(dev)) > pm_runtime_disable(dev); > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.h b/drivers/gpu/drm/arm/hdlcd_drv.h > index fd438d177b64..909c39c28487 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.h > +++ b/drivers/gpu/drm/arm/hdlcd_drv.h > @@ -11,6 +11,7 @@ struct hdlcd_drm_private { > struct clk *clk; > struct drm_crtc crtc; > struct drm_plane *plane; > + unsigned int irq; > #ifdef CONFIG_DEBUG_FS > atomic_t buffer_underrun_count; > atomic_t bus_error_count; > -- > 2.32.0 > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯