Skip to contentSkip to conversation

drm/ingenic: Convert to Linux IRQ interfaces

PATCH In review Thomas Zimmermann 3 participants 4 messages Jul 6, 2021 – Jul 12, 2021

Subsystems (4)
DRM drivers DRM misc MIPS Ingenic JZ47xx SoCs
lore
Share & tools
lore

Thomas Zimmermann

author maintainer
#0 lore raw

PATCH 1/1 drm/ingenic: 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.

Signed-off-by Thomas Zimmermann <[email protected]>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/gpu/drm/ingenic/ingenic-drm-drv.c +7 -6
index c296472164d9..a09b7da21b53 100644
@@ -33,7 +33,6 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
-#include <drm/drm_irq.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>

@@ -799,8 +798,6 @@ static const struct drm_driver ingenic_drm_driver_data = {
 	.fops			= &ingenic_drm_fops,
 	.gem_create_object	= ingenic_drm_gem_create_object,
 	DRM_GEM_CMA_DRIVER_OPS,
-
-	.irq_handler		= ingenic_drm_irq_handler,
 };
 
 static const struct drm_plane_funcs ingenic_drm_primary_plane_funcs = {

@@ -1098,7 +1095,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 		encoder->possible_clones = clone_mask;
 	}
 
-	ret = drm_irq_install(drm, irq);
+	ret = request_irq(irq, ingenic_drm_irq_handler, 0, drm->driver->name, drm);
 	if (ret) {
 		dev_err(dev, "Unable to install IRQ handler\n");
 		return ret;

@@ -1192,14 +1189,18 @@ static void ingenic_drm_unbind(struct device *dev)
 {
 	struct ingenic_drm *priv = dev_get_drvdata(dev);
 	struct clk *parent_clk = clk_get_parent(priv->pix_clk);
+	struct drm_device *drm = &priv->drm;
+	struct platform_device *pdev = to_platform_device(drm->dev);
+
+	free_irq(platform_get_irq(pdev, 0), drm);
 
 	clk_notifier_unregister(parent_clk, &priv->clock_nb);
 	if (priv->lcd_clk)
 		clk_disable_unprepare(priv->lcd_clk);
 	clk_disable_unprepare(priv->pix_clk);
 
-	drm_dev_unregister(&priv->drm);
-	drm_atomic_helper_shutdown(&priv->drm);
+	drm_dev_unregister(drm);
+	drm_atomic_helper_shutdown(drm);
 }
 
 static const struct component_master_ops ingenic_master_ops = {
Signature · 1 line
2.32.0
Reviewed-by on v1 1/1
#1 lore raw

Hi Thomas,

Quoted 40 lines · Thomas Zimmermann

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.

Signed-off-by Thomas Zimmermann <[email protected]>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/gpu/drm/ingenic/ingenic-drm-drv.c +1 -4
index c296472164d9..a09b7da21b53 100644
@@ -33,7 +33,6 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
-#include <drm/drm_irq.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>

@@ -799,8 +798,6 @@ static const struct drm_driver ingenic_drm_driver_data = {
 	.fops			= &ingenic_drm_fops,
 	.gem_create_object	= ingenic_drm_gem_create_object,
 	DRM_GEM_CMA_DRIVER_OPS,
-
-	.irq_handler		= ingenic_drm_irq_handler,
 };
 
 static const struct drm_plane_funcs ingenic_drm_primary_plane_funcs = {

@@ -1098,7 +1095,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 		encoder->possible_clones = clone_mask;
 	}
 
-	ret = drm_irq_install(drm, irq);
+	ret = request_irq(irq, ingenic_drm_irq_handler, 0, drm->driver->name, drm);
 	if (ret) {
 		dev_err(dev, "Unable to install IRQ handler\n");
 		return ret;

drivers/gpu/drm/ingenic/ingenic-drm-drv.c static void ingenic_drm_unbind(struct device *dev) · line 1195

Quoted 7 lines
drivers/gpu/drm/ingenic/ingenic-drm-drv.c +4 -0
@@ -1192,14 +1189,18 @@ static void ingenic_drm_unbind(struct device *dev)
 {
 	struct ingenic_drm *priv = dev_get_drvdata(dev);
 	struct clk *parent_clk = clk_get_parent(priv->pix_clk);
+	struct drm_device *drm = &priv->drm;
+	struct platform_device *pdev = to_platform_device(drm->dev);
+
+	free_irq(platform_get_irq(pdev, 0), drm);

The driver was missing a drm_irq_uninstall() so the above code is actually a small bug-fix. It should be mentioned in the changelog. With this fixed:

Reviewed-by Sam Ravnborg <[email protected]>

Note: I expect Paul to review too and apply.

Sam

Quoted 15 lines
drivers/gpu/drm/ingenic/ingenic-drm-drv.c +2 -2
 	clk_notifier_unregister(parent_clk, &priv->clock_nb);
 	if (priv->lcd_clk)
 		clk_disable_unprepare(priv->lcd_clk);
 	clk_disable_unprepare(priv->pix_clk);
 
-	drm_dev_unregister(&priv->drm);
-	drm_atomic_helper_shutdown(&priv->drm);
+	drm_dev_unregister(drm);
+	drm_atomic_helper_shutdown(drm);
 }
 
 static const struct component_master_ops ingenic_master_ops = {
Signature · 1 line
2.32.0
maintainer
#2 lore raw

Hi,

Quoted 45 lines · Sam Ravnborg

Hi Thomas,

Quoted 42 lines · Thomas Zimmermann

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.

Signed-off-by Thomas Zimmermann <[email protected]>

---

  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 13 +++++++------
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index c296472164d9..a09b7da21b53 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -33,7 +33,6 @@
#include <drm/drm_fourcc.h>
#include <drm/drm_gem_atomic_helper.h> #include <drm/drm_gem_framebuffer_helper.h>
-#include <drm/drm_irq.h>
#include <drm/drm_managed.h>
#include <drm/drm_of.h>
#include <drm/drm_panel.h>
@@ -799,8 +798,6 @@ static const struct drm_driver
ingenic_drm_driver_data = {
.fops = &ingenic_drm_fops,
.gem_create_object = ingenic_drm_gem_create_object,
DRM_GEM_CMA_DRIVER_OPS,
-
- .irq_handler = ingenic_drm_irq_handler,
};

static const struct drm_plane_funcs
ingenic_drm_primary_plane_funcs = {
@@ -1098,7 +1095,7 @@ static int ingenic_drm_bind(struct device
*dev, bool has_components)
encoder->possible_clones = clone_mask;
}

- ret = drm_irq_install(drm, irq);
+ ret = request_irq(irq, ingenic_drm_irq_handler, 0,
drm->driver->name, drm);

Can't you use the devm variant?

Quoted 13 lines
Quoted 4 lines

if (ret) {
dev_err(dev, "Unable to install IRQ handler\n"); return ret;
@@ -1192,14 +1189,18 @@ static void ingenic_drm_unbind(struct

Quoted 8 lines

device *dev)
{
struct ingenic_drm *priv = dev_get_drvdata(dev); struct clk *parent_clk = clk_get_parent(priv->pix_clk);
+ struct drm_device *drm = &priv->drm;
+ struct platform_device *pdev = to_platform_device(drm->dev);
+
+ free_irq(platform_get_irq(pdev, 0), drm);

Quoted 6 lines

The driver was missing a drm_irq_uninstall() so the above code is actually a small bug-fix. It should be mentioned in the changelog. With this fixed:

Reviewed-by Sam Ravnborg <[email protected]>

Note: I expect Paul to review too and apply.

I wasn't Cc'd? :(

-Paul

Quoted 18 lines

Sam

Quoted 15 lines
  	clk_notifier_unregister(parent_clk, &priv->clock_nb);
  	if (priv->lcd_clk)
  		clk_disable_unprepare(priv->lcd_clk);
  	clk_disable_unprepare(priv->pix_clk);

- drm_dev_unregister(&priv->drm);
- drm_atomic_helper_shutdown(&priv->drm);
+ drm_dev_unregister(drm);
+ drm_atomic_helper_shutdown(drm);
}

static const struct component_master_ops ingenic_master_ops = {
--
2.32.0

author maintainer
#3 lore raw

Hi

Am 10.07.21 um 12:04 schrieb Paul Cercueil:

Quoted 40 lines

Hi,

Quoted 37 lines · Sam Ravnborg

Hi Thomas,

Quoted 34 lines · Thomas Zimmermann

 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.

Signed-off-by Thomas Zimmermann <[email protected]>

 ---
  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 13 +++++++------

  1 file changed, 7 insertions(+), 6 deletions(-)

 diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index c296472164d9..a09b7da21b53 100644
 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
 +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
 @@ -33,7 +33,6 @@
  #include <drm/drm_fourcc.h>
  #include <drm/drm_gem_atomic_helper.h>
  #include <drm/drm_gem_framebuffer_helper.h>
 -#include <drm/drm_irq.h>
  #include <drm/drm_managed.h>
  #include <drm/drm_of.h>
  #include <drm/drm_panel.h>
 @@ -799,8 +798,6 @@ static const struct drm_driver ingenic_drm_driver_data = { .fops            = &ingenic_drm_fops,
      .gem_create_object    = ingenic_drm_gem_create_object, DRM_GEM_CMA_DRIVER_OPS,
 -
 -    .irq_handler        = ingenic_drm_irq_handler,
  };

  static const struct drm_plane_funcs ingenic_drm_primary_plane_funcs = {
 @@ -1098,7 +1095,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) encoder->possible_clones = clone_mask;
      }

Quoted 5 lines
Quoted 3 lines
Quoted 3 lines

 -    ret = drm_irq_install(drm, irq);
 +    ret = request_irq(irq, ingenic_drm_irq_handler, 0, drm->driver->name, drm);

Can't you use the devm variant?

I think so, I just tried to minimize the difference.

Best regards
Thomas

Quoted 44 lines
Quoted 13 lines
Quoted 12 lines

      if (ret) {
          dev_err(dev, "Unable to install IRQ handler\n"); return ret;
 @@ -1192,14 +1189,18 @@ static void ingenic_drm_unbind(struct device *dev) {
      struct ingenic_drm *priv = dev_get_drvdata(dev);
      struct clk *parent_clk = clk_get_parent(priv->pix_clk);
 +    struct drm_device *drm = &priv->drm;
 +    struct platform_device *pdev = to_platform_device(drm->dev); +
 +    free_irq(platform_get_irq(pdev, 0), drm);

Quoted 6 lines

The driver was missing a drm_irq_uninstall() so the above code is actually a small bug-fix. It should be mentioned in the changelog. With this fixed:

Reviewed-by Sam Ravnborg <[email protected]>

Note: I expect Paul to review too and apply.

I wasn't Cc'd? :(

-Paul

Quoted 18 lines

    Sam

Quoted 15 lines

      clk_notifier_unregister(parent_clk, &priv->clock_nb); if (priv->lcd_clk) clk_disable_unprepare(priv->lcd_clk); clk_disable_unprepare(priv->pix_clk);

 -    drm_dev_unregister(&priv->drm);
 -    drm_atomic_helper_shutdown(&priv->drm);
 +    drm_dev_unregister(drm);
 +    drm_atomic_helper_shutdown(drm);
  }

  static const struct component_master_ops ingenic_master_ops = { --
 2.32.0

Signature · 6 lines
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