[PATCH] drm/rockchip: Refactor IOMMU initialisation

2022-04-05 Thread Robin Murphy
Defer the IOMMU domain setup until after successfully binding
components, so we can figure out IOMMU support directly from the VOP
devices themselves, rather than manually inferring it from the DT (which
also fails to account for whether the IOMMU driver is actually loaded).
Although this is somewhat of a logical cleanup, the main motivation is
to prepare for a change in the iommu_domain_alloc() interface.

Signed-off-by: Robin Murphy 
---

Lightly tested on RK3288. This does stand to collide with the in-flight
VOP2 driver a little, if only that that will want an equivalent
rockchip_drm_dma_init_device() call too be able to use its IOMMU. I'm
happy to help sort that out either way, it just depends on what we want
to merge first.

Thanks,
Robin.

 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 60 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  3 ++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  2 +
 3 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 4eaeb430c83a..7efd12312354 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -7,7 +7,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -34,7 +33,6 @@
 #define DRIVER_MAJOR   1
 #define DRIVER_MINOR   0
 
-static bool is_support_iommu = true;
 static const struct drm_driver rockchip_drm_driver;
 
 /*
@@ -48,7 +46,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
struct rockchip_drm_private *private = drm_dev->dev_private;
int ret;
 
-   if (!is_support_iommu)
+   if (!private->domain)
return 0;
 
ret = iommu_attach_device(private->domain, dev);
@@ -64,12 +62,22 @@ void rockchip_drm_dma_detach_device(struct drm_device 
*drm_dev,
struct device *dev)
 {
struct rockchip_drm_private *private = drm_dev->dev_private;
-   struct iommu_domain *domain = private->domain;
 
-   if (!is_support_iommu)
+   if (!private->domain)
return;
 
-   iommu_detach_device(domain, dev);
+   iommu_detach_device(private->domain, dev);
+}
+
+void rockchip_drm_dma_init_device(struct drm_device *drm_dev,
+ struct device *dev)
+{
+   struct rockchip_drm_private *private = drm_dev->dev_private;
+
+   if (!device_iommu_mapped(dev))
+   private->iommu_dev = ERR_PTR(-ENODEV);
+   else if (!private->iommu_dev)
+   private->iommu_dev = dev;
 }
 
 static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
@@ -78,10 +86,10 @@ static int rockchip_drm_init_iommu(struct drm_device 
*drm_dev)
struct iommu_domain_geometry *geometry;
u64 start, end;
 
-   if (!is_support_iommu)
+   if (IS_ERR_OR_NULL(private->iommu_dev))
return 0;
 
-   private->domain = iommu_domain_alloc(&platform_bus_type);
+   private->domain = iommu_domain_alloc(private->iommu_dev->bus);
if (!private->domain)
return -ENOMEM;
 
@@ -101,7 +109,7 @@ static void rockchip_iommu_cleanup(struct drm_device 
*drm_dev)
 {
struct rockchip_drm_private *private = drm_dev->dev_private;
 
-   if (!is_support_iommu)
+   if (!private->domain)
return;
 
drm_mm_takedown(&private->mm);
@@ -137,24 +145,24 @@ static int rockchip_drm_bind(struct device *dev)
 
drm_dev->dev_private = private;
 
-   ret = rockchip_drm_init_iommu(drm_dev);
-   if (ret)
-   goto err_free;
-
ret = drmm_mode_config_init(drm_dev);
if (ret)
-   goto err_iommu_cleanup;
+   goto err_free;
 
rockchip_drm_mode_config_init(drm_dev);
 
/* Try to bind all sub drivers. */
ret = component_bind_all(dev, drm_dev);
if (ret)
-   goto err_iommu_cleanup;
+   goto err_free;
+
+   ret = rockchip_drm_init_iommu(drm_dev);
+   if (ret)
+   goto err_unbind_all;
 
ret = drm_vblank_init(drm_dev, drm_dev->mode_config.num_crtc);
if (ret)
-   goto err_unbind_all;
+   goto err_iommu_cleanup;
 
drm_mode_config_reset(drm_dev);
 
@@ -170,10 +178,10 @@ static int rockchip_drm_bind(struct device *dev)
return 0;
 err_kms_helper_poll_fini:
drm_kms_helper_poll_fini(drm_dev);
-err_unbind_all:
-   component_unbind_all(dev, drm_dev);
 err_iommu_cleanup:
rockchip_iommu_cleanup(drm_dev);
+err_unbind_all:
+   component_unbind_all(dev, drm_dev);
 err_free:
drm_dev_put(drm_dev);
return ret;
@@ -342,8 +350,6 @@ static int rockchip_drm_platform_of_probe(struct device 
*dev)
return -ENODEV;
 
for (i = 0;; i++) {
-   struct device_node *iommu;
-
port = of_parse_phandle(np, "ports", i);
if (!port

Re: [PATCH] drm/rockchip: Refactor IOMMU initialisation

2022-04-08 Thread Sascha Hauer
On Tue, Apr 05, 2022 at 03:32:50PM +0100, Robin Murphy wrote:
> Defer the IOMMU domain setup until after successfully binding
> components, so we can figure out IOMMU support directly from the VOP
> devices themselves, rather than manually inferring it from the DT (which
> also fails to account for whether the IOMMU driver is actually loaded).
> Although this is somewhat of a logical cleanup, the main motivation is
> to prepare for a change in the iommu_domain_alloc() interface.
> 
> Signed-off-by: Robin Murphy 
> ---
> 
> Lightly tested on RK3288. This does stand to collide with the in-flight
> VOP2 driver a little, if only that that will want an equivalent
> rockchip_drm_dma_init_device() call too be able to use its IOMMU. I'm
> happy to help sort that out either way, it just depends on what we want
> to merge first.

I tested it with the VOP2 driver. It needed a little refactoring, I had
to move the call to rockchip_drm_dma_attach_device() from vop2_bind() to
vop2_enable(), but then it works as expected.

Assuming that this patch goes through Heikos tree just like the VOP2
driver we can merge it first. I'll base the next VOP2 round on it.

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] drm/rockchip: Refactor IOMMU initialisation

2022-05-02 Thread Heiko Stuebner
On Tue, 5 Apr 2022 15:32:50 +0100, Robin Murphy wrote:
> Defer the IOMMU domain setup until after successfully binding
> components, so we can figure out IOMMU support directly from the VOP
> devices themselves, rather than manually inferring it from the DT (which
> also fails to account for whether the IOMMU driver is actually loaded).
> Although this is somewhat of a logical cleanup, the main motivation is
> to prepare for a change in the iommu_domain_alloc() interface.

Applied, thanks!

[1/1] drm/rockchip: Refactor IOMMU initialisation
  commit: 421be3ee36a497949a4b564cd1e4f7f9fe755f57

Additionally tested on rk3288, rk3399 and px30.

Best regards,
-- 
Heiko Stuebner 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu