Re: [Freedreno] [PATCH v2 02/14] drm/arm/hdlcd: Convert to Linux IRQ interfaces
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
> Acked-by: Sam Ravnborg
Sorry for the delayed response due to holidays.
Acked-by: Liviu Dudau
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
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -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(>buffer_underrun_count);
> +
> + if (irq_status & HDLCD_INTERRUPT_DMA_END)
> + atomic_inc(>dma_end_count);
> +
> + if (irq_status & HDLCD_INTERRUPT_BUS_ERROR)
> + atomic_inc(>bus_error_count);
> +
> + if (irq_status & HDLCD_INTERRUPT_VSYNC)
> + atomic_inc(>vsync_count);
> +
> +#endif
> + if (irq_status & HDLCD_INTERRUPT_VSYNC)
> + drm_crtc_handle_vblank(>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 = _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)
> -
[Freedreno] [PATCH v2 02/14] drm/arm/hdlcd: Convert to Linux IRQ interfaces
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
Acked-by: Sam Ravnborg
---
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
#include
#include
-#include
#include
#include
#include
@@ -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(>buffer_underrun_count);
+
+ if (irq_status & HDLCD_INTERRUPT_DMA_END)
+ atomic_inc(>dma_end_count);
+
+ if (irq_status & HDLCD_INTERRUPT_BUS_ERROR)
+ atomic_inc(>bus_error_count);
+
+ if (irq_status & HDLCD_INTERRUPT_VSYNC)
+ atomic_inc(>vsync_count);
+
+#endif
+ if (irq_status & HDLCD_INTERRUPT_VSYNC)
+ drm_crtc_handle_vblank(>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 = _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(>buffer_underrun_count);
-
- if (irq_status & HDLCD_INTERRUPT_DMA_END)
- atomic_inc(>dma_end_count);
-
- if (irq_status & HDLCD_INTERRUPT_BUS_ERROR)
- atomic_inc(>bus_error_count);
-
- if (irq_status & HDLCD_INTERRUPT_VSYNC)
- atomic_inc(>vsync_count);
-
-#endif
- if (irq_status & HDLCD_INTERRUPT_VSYNC)
-
