diff mbox series

drm/arm/hdlcd: Simplify IRQ install/uninstall

Message ID 65cf7818b23c1a8629dc851f1d058ecb8a14849e.1655309413.git.robin.murphy@arm.com (mailing list archive)
State New, archived
Headers show
Series drm/arm/hdlcd: Simplify IRQ install/uninstall | expand

Commit Message

Robin Murphy June 15, 2022, 4:11 p.m. UTC 
Since we no longer need to conform to the structure of the various DRM
IRQ callbacks, we can streamline the code by consolidating the piecemeal
functions and passing around our private data structure directly. We're
also a platform device so should never see IRQ_NOTCONNECTED either.

Furthermore we can also get rid of all the unnecesary read-modify-write
operations, since on install we know we cleared the whole interrupt mask
before enabling the debug IRQs, and thus on uninstall we're always
clearing everything as well.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 62 +++++++++------------------------
 1 file changed, 16 insertions(+), 46 deletions(-)

Comments

Liviu Dudau June 16, 2022, 11:04 a.m. UTC | #1 
On Wed, Jun 15, 2022 at 05:11:09PM +0100, Robin Murphy wrote:
> Since we no longer need to conform to the structure of the various DRM
> IRQ callbacks, we can streamline the code by consolidating the piecemeal
> functions and passing around our private data structure directly. We're
> also a platform device so should never see IRQ_NOTCONNECTED either.
> 
> Furthermore we can also get rid of all the unnecesary read-modify-write
> operations, since on install we know we cleared the whole interrupt mask
> before enabling the debug IRQs, and thus on uninstall we're always
> clearing everything as well.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks for the cleanup!

Best regards,
Liviu

> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 62 +++++++++------------------------
>  1 file changed, 16 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 1f1171f2f16a..7d6aa9b3b577 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -41,8 +41,7 @@
>  
>  static irqreturn_t hdlcd_irq(int irq, void *arg)
>  {
> -	struct drm_device *drm = arg;
> -	struct hdlcd_drm_private *hdlcd = drm->dev_private;
> +	struct hdlcd_drm_private *hdlcd = arg;
>  	unsigned long irq_status;
>  
>  	irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS);
> @@ -70,61 +69,32 @@ static irqreturn_t hdlcd_irq(int irq, void *arg)
>  	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)
> +static int hdlcd_irq_install(struct hdlcd_drm_private *hdlcd)
>  {
>  	int ret;
>  
> -	if (irq == IRQ_NOTCONNECTED)
> -		return -ENOTCONN;
> +	/* Ensure interrupts are disabled */
> +	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0);
> +	hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0);
>  
> -	hdlcd_irq_preinstall(drm);
> -
> -	ret = request_irq(irq, hdlcd_irq, 0, drm->driver->name, drm);
> +	ret = request_irq(hdlcd->irq, hdlcd_irq, 0, "hdlcd", hdlcd);
>  	if (ret)
>  		return ret;
>  
> -	hdlcd_irq_postinstall(drm);
> +#ifdef CONFIG_DEBUG_FS
> +	/* enable debug interrupts */
> +	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, HDLCD_DEBUG_INT_MASK);
> +#endif
>  
>  	return 0;
>  }
>  
> -static void hdlcd_irq_uninstall(struct drm_device *drm)
> +static void hdlcd_irq_uninstall(struct hdlcd_drm_private *hdlcd)
>  {
> -	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);
> +	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0);
>  
> -#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);
> +	free_irq(hdlcd->irq, hdlcd);
>  }
>  
>  static int hdlcd_load(struct drm_device *drm, unsigned long flags)
> @@ -184,7 +154,7 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
>  		goto irq_fail;
>  	hdlcd->irq = ret;
>  
> -	ret = hdlcd_irq_install(drm, hdlcd->irq);
> +	ret = hdlcd_irq_install(hdlcd);
>  	if (ret < 0) {
>  		DRM_ERROR("failed to install IRQ handler\n");
>  		goto irq_fail;
> @@ -342,7 +312,7 @@ static int hdlcd_drm_bind(struct device *dev)
>  err_unload:
>  	of_node_put(hdlcd->crtc.port);
>  	hdlcd->crtc.port = NULL;
> -	hdlcd_irq_uninstall(drm);
> +	hdlcd_irq_uninstall(hdlcd);
>  	of_reserved_mem_device_release(drm->dev);
>  err_free:
>  	drm_mode_config_cleanup(drm);
> @@ -364,7 +334,7 @@ static void hdlcd_drm_unbind(struct device *dev)
>  	hdlcd->crtc.port = NULL;
>  	pm_runtime_get_sync(dev);
>  	drm_atomic_helper_shutdown(drm);
> -	hdlcd_irq_uninstall(drm);
> +	hdlcd_irq_uninstall(hdlcd);
>  	pm_runtime_put(dev);
>  	if (pm_runtime_enabled(dev))
>  		pm_runtime_disable(dev);
> -- 
> 2.36.1.dirty
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 1f1171f2f16a..7d6aa9b3b577 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -41,8 +41,7 @@ 
 
 static irqreturn_t hdlcd_irq(int irq, void *arg)
 {
-	struct drm_device *drm = arg;
-	struct hdlcd_drm_private *hdlcd = drm->dev_private;
+	struct hdlcd_drm_private *hdlcd = arg;
 	unsigned long irq_status;
 
 	irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS);
@@ -70,61 +69,32 @@  static irqreturn_t hdlcd_irq(int irq, void *arg)
 	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)
+static int hdlcd_irq_install(struct hdlcd_drm_private *hdlcd)
 {
 	int ret;
 
-	if (irq == IRQ_NOTCONNECTED)
-		return -ENOTCONN;
+	/* Ensure interrupts are disabled */
+	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0);
+	hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0);
 
-	hdlcd_irq_preinstall(drm);
-
-	ret = request_irq(irq, hdlcd_irq, 0, drm->driver->name, drm);
+	ret = request_irq(hdlcd->irq, hdlcd_irq, 0, "hdlcd", hdlcd);
 	if (ret)
 		return ret;
 
-	hdlcd_irq_postinstall(drm);
+#ifdef CONFIG_DEBUG_FS
+	/* enable debug interrupts */
+	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, HDLCD_DEBUG_INT_MASK);
+#endif
 
 	return 0;
 }
 
-static void hdlcd_irq_uninstall(struct drm_device *drm)
+static void hdlcd_irq_uninstall(struct hdlcd_drm_private *hdlcd)
 {
-	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);
+	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0);
 
-#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);
+	free_irq(hdlcd->irq, hdlcd);
 }
 
 static int hdlcd_load(struct drm_device *drm, unsigned long flags)
@@ -184,7 +154,7 @@  static int hdlcd_load(struct drm_device *drm, unsigned long flags)
 		goto irq_fail;
 	hdlcd->irq = ret;
 
-	ret = hdlcd_irq_install(drm, hdlcd->irq);
+	ret = hdlcd_irq_install(hdlcd);
 	if (ret < 0) {
 		DRM_ERROR("failed to install IRQ handler\n");
 		goto irq_fail;
@@ -342,7 +312,7 @@  static int hdlcd_drm_bind(struct device *dev)
 err_unload:
 	of_node_put(hdlcd->crtc.port);
 	hdlcd->crtc.port = NULL;
-	hdlcd_irq_uninstall(drm);
+	hdlcd_irq_uninstall(hdlcd);
 	of_reserved_mem_device_release(drm->dev);
 err_free:
 	drm_mode_config_cleanup(drm);
@@ -364,7 +334,7 @@  static void hdlcd_drm_unbind(struct device *dev)
 	hdlcd->crtc.port = NULL;
 	pm_runtime_get_sync(dev);
 	drm_atomic_helper_shutdown(drm);
-	hdlcd_irq_uninstall(drm);
+	hdlcd_irq_uninstall(hdlcd);
 	pm_runtime_put(dev);
 	if (pm_runtime_enabled(dev))
 		pm_runtime_disable(dev);