Re: [RFC 09/10] drm/tegra: Add IOMMU support
On Wed, Oct 01, 2014 at 11:54:11AM -0400, Sean Paul wrote:
On Tue, Sep 30, 2014 at 2:48 PM, Sean Paul seanp...@google.com wrote:
On Thu, Jun 26, 2014 at 4:49 PM, Thierry Reding thierry.red...@gmail.com
wrote:
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
[...]
static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo)
{
- dma_free_writecombine(drm-dev, bo-gem.size, bo-vaddr,
bo-paddr);
+ if (!bo-pages)
+ dma_free_writecombine(drm-dev, bo-gem.size, bo-vaddr,
+ bo-paddr);
One more thing. If tegra_bo_alloc fails, we'll have bo-vaddr == NULL
and bo-paddr == ~0 here, which causes a crash.
I posted https://lkml.org/lkml/2014/9/30/659 to check for the error
condition in the mm code, but it seems like reviewer consensus is to
check for this before calling free.
As such, we'll need to make sure bo-vaddr != NULL before calling
dma_free_writecombine to avoid this situation.
Would you prefer I send a patch up to fix this separately, or would
you like to roll this into your next version?
I've rolled this check into my series because I touch that area of code
anyway.
Thanks for bringing it up.
Thierry
pgpqjHaLne__g.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 09/10] drm/tegra: Add IOMMU support
On Tue, Sep 30, 2014 at 02:48:35PM -0400, Sean Paul wrote:
On Thu, Jun 26, 2014 at 4:49 PM, Thierry Reding
thierry.red...@gmail.com wrote:
From: Thierry Reding tred...@nvidia.com
When an IOMMU device is available on the platform bus, allocate an IOMMU
domain and attach the display controllers to it. The display controllers
can then scan out non-contiguous buffers by mapping them through the
IOMMU.
Hi Thierry,
A few comments from Stéphane and myself that came up while we were
reviewing this for our tree.
I just realized that I hadn't integrated these comments completely yet,
but I've done so now in my local tree. I'm running a couple of tests to
verify that it's all handled correctly.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
drivers/gpu/drm/tegra/dc.c | 21
drivers/gpu/drm/tegra/drm.c | 17
drivers/gpu/drm/tegra/drm.h | 3 +
drivers/gpu/drm/tegra/fb.c | 16 ++-
drivers/gpu/drm/tegra/gem.c | 236
+++-
drivers/gpu/drm/tegra/gem.h | 4 +
6 files changed, 273 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index afcca04f5367..0f7452d04811 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -9,6 +9,7 @@
#include linux/clk.h
#include linux/debugfs.h
+#include linux/iommu.h
#include linux/reset.h
#include dc.h
@@ -1283,8 +1284,18 @@ static int tegra_dc_init(struct host1x_client
*client)
{
struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
+ if (tegra-domain) {
+ err = iommu_attach_device(tegra-domain, dc-dev);
+ if (err 0) {
+ dev_err(dc-dev, failed to attach to IOMMU: %d\n,
+ err);
+ return err;
+ }
[from Stéphane]
shouldn't we call detach in the error paths below?
This was mostly rewritten for universal plane support, but I've made
sure that the DC properly detaches from the IOMMU in case of failure
during the code below.
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
[...]
@@ -8,6 +8,7 @@
*/
#include linux/host1x.h
+#include linux/iommu.h
#include drm.h
#include gem.h
@@ -33,6 +34,16 @@ static int tegra_drm_load(struct drm_device *drm,
unsigned long flags)
if (!tegra)
return -ENOMEM;
+ if (iommu_present(platform_bus_type)) {
+ tegra-domain = iommu_domain_alloc(platform_bus_type);
+ if (IS_ERR(tegra-domain)) {
+ kfree(tegra);
+ return PTR_ERR(tegra-domain);
+ }
+
+ drm_mm_init(tegra-mm, 0, SZ_2G);
[from Stéphane]:
none of these are freed in the error path below (iommu_domain_free and
drm_mm_takedown)
also |tegra| isn't freed either?
None of the resources were actually being cleaned up, but I think I have
it all handled properly now.
@@ -108,22 +240,33 @@ struct tegra_bo *tegra_bo_create(struct drm_device
*drm, unsigned int size,
host1x_bo_init(bo-base, tegra_bo_ops);
size = round_up(size, PAGE_SIZE);
- bo-vaddr = dma_alloc_writecombine(drm-dev, size, bo-paddr,
- GFP_KERNEL | __GFP_NOWARN);
- if (!bo-vaddr) {
- dev_err(drm-dev, failed to allocate buffer with size
%u\n,
- size);
- err = -ENOMEM;
- goto err_dma;
- }
-
err = drm_gem_object_init(drm, bo-gem, size);
if (err)
- goto err_init;
+ goto free;
err = drm_gem_create_mmap_offset(bo-gem);
We need to call drm_gem_free_mmap_offset if one of the calls below
fails, otherwise we'll try to free the mmap_offset on an already
destroyed bo.
drm_gem_object_release() (below) already calls drm_gem_free_mmap_offset()
for us implicitly.
Thierry
pgpXeJxxc7EMp.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 09/10] drm/tegra: Add IOMMU support
On Wed, Oct 01, 2014 at 11:54:11AM -0400, Sean Paul wrote:
On Tue, Sep 30, 2014 at 2:48 PM, Sean Paul seanp...@google.com wrote:
On Thu, Jun 26, 2014 at 4:49 PM, Thierry Reding
thierry.red...@gmail.com wrote:
From: Thierry Reding tred...@nvidia.com
When an IOMMU device is available on the platform bus, allocate an IOMMU
domain and attach the display controllers to it. The display controllers
can then scan out non-contiguous buffers by mapping them through the
IOMMU.
Hi Thierry,
A few comments from Stéphane and myself that came up while we were
reviewing this for our tree.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
drivers/gpu/drm/tegra/dc.c | 21
drivers/gpu/drm/tegra/drm.c | 17
drivers/gpu/drm/tegra/drm.h | 3 +
drivers/gpu/drm/tegra/fb.c | 16 ++-
drivers/gpu/drm/tegra/gem.c | 236
+++-
drivers/gpu/drm/tegra/gem.h | 4 +
6 files changed, 273 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index afcca04f5367..0f7452d04811 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -9,6 +9,7 @@
#include linux/clk.h
#include linux/debugfs.h
+#include linux/iommu.h
#include linux/reset.h
#include dc.h
@@ -1283,8 +1284,18 @@ static int tegra_dc_init(struct host1x_client
*client)
{
struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
+ if (tegra-domain) {
+ err = iommu_attach_device(tegra-domain, dc-dev);
+ if (err 0) {
+ dev_err(dc-dev, failed to attach to IOMMU: %d\n,
+ err);
+ return err;
+ }
[from Stéphane]
shouldn't we call detach in the error paths below?
+ }
+
drm_crtc_init(drm, dc-base, tegra_crtc_funcs);
drm_mode_crtc_set_gamma_size(dc-base, 256);
drm_crtc_helper_add(dc-base, tegra_crtc_helper_funcs);
@@ -1318,7 +1329,9 @@ static int tegra_dc_init(struct host1x_client
*client)
static int tegra_dc_exit(struct host1x_client *client)
{
+ struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
devm_free_irq(dc-dev, dc-irq, dc);
@@ -1335,6 +1348,8 @@ static int tegra_dc_exit(struct host1x_client
*client)
return err;
}
+ iommu_detach_device(tegra-domain, dc-dev);
+
return 0;
}
@@ -1462,6 +1477,12 @@ static int tegra_dc_probe(struct platform_device
*pdev)
return -ENXIO;
}
+ err = iommu_attach(pdev-dev);
+ if (err 0) {
+ dev_err(pdev-dev, failed to attach to IOMMU: %d\n,
err);
+ return err;
+ }
+
INIT_LIST_HEAD(dc-client.list);
dc-client.ops = dc_client_ops;
dc-client.dev = pdev-dev;
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 59736bb810cd..1d2bbafad982 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -8,6 +8,7 @@
*/
#include linux/host1x.h
+#include linux/iommu.h
#include drm.h
#include gem.h
@@ -33,6 +34,16 @@ static int tegra_drm_load(struct drm_device *drm,
unsigned long flags)
if (!tegra)
return -ENOMEM;
+ if (iommu_present(platform_bus_type)) {
+ tegra-domain = iommu_domain_alloc(platform_bus_type);
+ if (IS_ERR(tegra-domain)) {
+ kfree(tegra);
+ return PTR_ERR(tegra-domain);
+ }
+
+ drm_mm_init(tegra-mm, 0, SZ_2G);
[from Stéphane]:
none of these are freed in the error path below (iommu_domain_free and
drm_mm_takedown)
also |tegra| isn't freed either?
+ }
+
mutex_init(tegra-clients_lock);
INIT_LIST_HEAD(tegra-clients);
drm-dev_private = tegra;
@@ -71,6 +82,7 @@ static int tegra_drm_load(struct drm_device *drm,
unsigned long flags)
static int tegra_drm_unload(struct drm_device *drm)
{
struct host1x_device *device = to_host1x_device(drm-dev);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
drm_kms_helper_poll_fini(drm);
@@ -82,6 +94,11 @@ static int tegra_drm_unload(struct drm_device *drm)
if (err 0)
return err;
+ if (tegra-domain) {
+ iommu_domain_free(tegra-domain);
+ drm_mm_takedown(tegra-mm);
+ }
+
return 0;
}
diff --git
Re: [RFC 09/10] drm/tegra: Add IOMMU support
On Thu, Jun 26, 2014 at 4:49 PM, Thierry Reding
thierry.red...@gmail.com wrote:
From: Thierry Reding tred...@nvidia.com
When an IOMMU device is available on the platform bus, allocate an IOMMU
domain and attach the display controllers to it. The display controllers
can then scan out non-contiguous buffers by mapping them through the
IOMMU.
Hi Thierry,
A few comments from Stéphane and myself that came up while we were
reviewing this for our tree.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
drivers/gpu/drm/tegra/dc.c | 21
drivers/gpu/drm/tegra/drm.c | 17
drivers/gpu/drm/tegra/drm.h | 3 +
drivers/gpu/drm/tegra/fb.c | 16 ++-
drivers/gpu/drm/tegra/gem.c | 236
+++-
drivers/gpu/drm/tegra/gem.h | 4 +
6 files changed, 273 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index afcca04f5367..0f7452d04811 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -9,6 +9,7 @@
#include linux/clk.h
#include linux/debugfs.h
+#include linux/iommu.h
#include linux/reset.h
#include dc.h
@@ -1283,8 +1284,18 @@ static int tegra_dc_init(struct host1x_client *client)
{
struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
+ if (tegra-domain) {
+ err = iommu_attach_device(tegra-domain, dc-dev);
+ if (err 0) {
+ dev_err(dc-dev, failed to attach to IOMMU: %d\n,
+ err);
+ return err;
+ }
[from Stéphane]
shouldn't we call detach in the error paths below?
+ }
+
drm_crtc_init(drm, dc-base, tegra_crtc_funcs);
drm_mode_crtc_set_gamma_size(dc-base, 256);
drm_crtc_helper_add(dc-base, tegra_crtc_helper_funcs);
@@ -1318,7 +1329,9 @@ static int tegra_dc_init(struct host1x_client *client)
static int tegra_dc_exit(struct host1x_client *client)
{
+ struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
devm_free_irq(dc-dev, dc-irq, dc);
@@ -1335,6 +1348,8 @@ static int tegra_dc_exit(struct host1x_client *client)
return err;
}
+ iommu_detach_device(tegra-domain, dc-dev);
+
return 0;
}
@@ -1462,6 +1477,12 @@ static int tegra_dc_probe(struct platform_device *pdev)
return -ENXIO;
}
+ err = iommu_attach(pdev-dev);
+ if (err 0) {
+ dev_err(pdev-dev, failed to attach to IOMMU: %d\n, err);
+ return err;
+ }
+
INIT_LIST_HEAD(dc-client.list);
dc-client.ops = dc_client_ops;
dc-client.dev = pdev-dev;
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 59736bb810cd..1d2bbafad982 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -8,6 +8,7 @@
*/
#include linux/host1x.h
+#include linux/iommu.h
#include drm.h
#include gem.h
@@ -33,6 +34,16 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
if (!tegra)
return -ENOMEM;
+ if (iommu_present(platform_bus_type)) {
+ tegra-domain = iommu_domain_alloc(platform_bus_type);
+ if (IS_ERR(tegra-domain)) {
+ kfree(tegra);
+ return PTR_ERR(tegra-domain);
+ }
+
+ drm_mm_init(tegra-mm, 0, SZ_2G);
[from Stéphane]:
none of these are freed in the error path below (iommu_domain_free and
drm_mm_takedown)
also |tegra| isn't freed either?
+ }
+
mutex_init(tegra-clients_lock);
INIT_LIST_HEAD(tegra-clients);
drm-dev_private = tegra;
@@ -71,6 +82,7 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
static int tegra_drm_unload(struct drm_device *drm)
{
struct host1x_device *device = to_host1x_device(drm-dev);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
drm_kms_helper_poll_fini(drm);
@@ -82,6 +94,11 @@ static int tegra_drm_unload(struct drm_device *drm)
if (err 0)
return err;
+ if (tegra-domain) {
+ iommu_domain_free(tegra-domain);
+ drm_mm_takedown(tegra-mm);
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 96d754e7b3eb..a07c796b7edc 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -39,6 +39,9 @@ struct tegra_fbdev {
struct tegra_drm {
struct drm_device *drm;
+ struct iommu_domain *domain;
+ struct drm_mm mm;
+
Re: [RFC 09/10] drm/tegra: Add IOMMU support
On Tue, Sep 30, 2014 at 2:48 PM, Sean Paul seanp...@google.com wrote:
On Thu, Jun 26, 2014 at 4:49 PM, Thierry Reding
thierry.red...@gmail.com wrote:
From: Thierry Reding tred...@nvidia.com
When an IOMMU device is available on the platform bus, allocate an IOMMU
domain and attach the display controllers to it. The display controllers
can then scan out non-contiguous buffers by mapping them through the
IOMMU.
Hi Thierry,
A few comments from Stéphane and myself that came up while we were
reviewing this for our tree.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
drivers/gpu/drm/tegra/dc.c | 21
drivers/gpu/drm/tegra/drm.c | 17
drivers/gpu/drm/tegra/drm.h | 3 +
drivers/gpu/drm/tegra/fb.c | 16 ++-
drivers/gpu/drm/tegra/gem.c | 236
+++-
drivers/gpu/drm/tegra/gem.h | 4 +
6 files changed, 273 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index afcca04f5367..0f7452d04811 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -9,6 +9,7 @@
#include linux/clk.h
#include linux/debugfs.h
+#include linux/iommu.h
#include linux/reset.h
#include dc.h
@@ -1283,8 +1284,18 @@ static int tegra_dc_init(struct host1x_client *client)
{
struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
+ if (tegra-domain) {
+ err = iommu_attach_device(tegra-domain, dc-dev);
+ if (err 0) {
+ dev_err(dc-dev, failed to attach to IOMMU: %d\n,
+ err);
+ return err;
+ }
[from Stéphane]
shouldn't we call detach in the error paths below?
+ }
+
drm_crtc_init(drm, dc-base, tegra_crtc_funcs);
drm_mode_crtc_set_gamma_size(dc-base, 256);
drm_crtc_helper_add(dc-base, tegra_crtc_helper_funcs);
@@ -1318,7 +1329,9 @@ static int tegra_dc_init(struct host1x_client *client)
static int tegra_dc_exit(struct host1x_client *client)
{
+ struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
devm_free_irq(dc-dev, dc-irq, dc);
@@ -1335,6 +1348,8 @@ static int tegra_dc_exit(struct host1x_client *client)
return err;
}
+ iommu_detach_device(tegra-domain, dc-dev);
+
return 0;
}
@@ -1462,6 +1477,12 @@ static int tegra_dc_probe(struct platform_device
*pdev)
return -ENXIO;
}
+ err = iommu_attach(pdev-dev);
+ if (err 0) {
+ dev_err(pdev-dev, failed to attach to IOMMU: %d\n, err);
+ return err;
+ }
+
INIT_LIST_HEAD(dc-client.list);
dc-client.ops = dc_client_ops;
dc-client.dev = pdev-dev;
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 59736bb810cd..1d2bbafad982 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -8,6 +8,7 @@
*/
#include linux/host1x.h
+#include linux/iommu.h
#include drm.h
#include gem.h
@@ -33,6 +34,16 @@ static int tegra_drm_load(struct drm_device *drm,
unsigned long flags)
if (!tegra)
return -ENOMEM;
+ if (iommu_present(platform_bus_type)) {
+ tegra-domain = iommu_domain_alloc(platform_bus_type);
+ if (IS_ERR(tegra-domain)) {
+ kfree(tegra);
+ return PTR_ERR(tegra-domain);
+ }
+
+ drm_mm_init(tegra-mm, 0, SZ_2G);
[from Stéphane]:
none of these are freed in the error path below (iommu_domain_free and
drm_mm_takedown)
also |tegra| isn't freed either?
+ }
+
mutex_init(tegra-clients_lock);
INIT_LIST_HEAD(tegra-clients);
drm-dev_private = tegra;
@@ -71,6 +82,7 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
static int tegra_drm_unload(struct drm_device *drm)
{
struct host1x_device *device = to_host1x_device(drm-dev);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
drm_kms_helper_poll_fini(drm);
@@ -82,6 +94,11 @@ static int tegra_drm_unload(struct drm_device *drm)
if (err 0)
return err;
+ if (tegra-domain) {
+ iommu_domain_free(tegra-domain);
+ drm_mm_takedown(tegra-mm);
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 96d754e7b3eb..a07c796b7edc 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -39,6 +39,9 @@ struct tegra_fbdev {
struct tegra_drm {
struct
Re: [RFC 09/10] drm/tegra: Add IOMMU support
Thierry Reding thierry.red...@gmail.com writes:
From: Thierry Reding tred...@nvidia.com
When an IOMMU device is available on the platform bus, allocate an IOMMU
domain and attach the display controllers to it. The display controllers
can then scan out non-contiguous buffers by mapping them through the
IOMMU.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
drivers/gpu/drm/tegra/dc.c | 21
drivers/gpu/drm/tegra/drm.c | 17
drivers/gpu/drm/tegra/drm.h | 3 +
drivers/gpu/drm/tegra/fb.c | 16 ++-
drivers/gpu/drm/tegra/gem.c | 236
+++-
drivers/gpu/drm/tegra/gem.h | 4 +
6 files changed, 273 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index afcca04f5367..0f7452d04811 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -9,6 +9,7 @@
#include linux/clk.h
#include linux/debugfs.h
+#include linux/iommu.h
#include linux/reset.h
#include dc.h
@@ -1283,8 +1284,18 @@ static int tegra_dc_init(struct host1x_client *client)
{
struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
+ if (tegra-domain) {
+ err = iommu_attach_device(tegra-domain, dc-dev);
I wanted to keep device drivers iommu-free with the following:
http://patchwork.ozlabs.org/patch/354074/
+ if (err 0) {
+ dev_err(dc-dev, failed to attach to IOMMU: %d\n,
+ err);
+ return err;
+ }
+ }
+
drm_crtc_init(drm, dc-base, tegra_crtc_funcs);
drm_mode_crtc_set_gamma_size(dc-base, 256);
drm_crtc_helper_add(dc-base, tegra_crtc_helper_funcs);
@@ -1318,7 +1329,9 @@ static int tegra_dc_init(struct host1x_client *client)
static int tegra_dc_exit(struct host1x_client *client)
{
+ struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
devm_free_irq(dc-dev, dc-irq, dc);
@@ -1335,6 +1348,8 @@ static int tegra_dc_exit(struct host1x_client *client)
return err;
}
+ iommu_detach_device(tegra-domain, dc-dev);
+
return 0;
}
@@ -1462,6 +1477,12 @@ static int tegra_dc_probe(struct platform_device *pdev)
return -ENXIO;
}
+ err = iommu_attach(pdev-dev);
+ if (err 0) {
+ dev_err(pdev-dev, failed to attach to IOMMU: %d\n, err);
+ return err;
+ }
+
INIT_LIST_HEAD(dc-client.list);
dc-client.ops = dc_client_ops;
dc-client.dev = pdev-dev;
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 59736bb810cd..1d2bbafad982 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -8,6 +8,7 @@
*/
#include linux/host1x.h
+#include linux/iommu.h
#include drm.h
#include gem.h
@@ -33,6 +34,16 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
if (!tegra)
return -ENOMEM;
+ if (iommu_present(platform_bus_type)) {
+ tegra-domain = iommu_domain_alloc(platform_bus_type);
Can we use dma_iommu_mapping instead of domain?
I thought that DMA API is on the top of IOMMU API so that it may be
cleaner to use only DMA API.
+ if (IS_ERR(tegra-domain)) {
+ kfree(tegra);
+ return PTR_ERR(tegra-domain);
+ }
+
+ drm_mm_init(tegra-mm, 0, SZ_2G);
+ }
+
mutex_init(tegra-clients_lock);
INIT_LIST_HEAD(tegra-clients);
drm-dev_private = tegra;
@@ -71,6 +82,7 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
static int tegra_drm_unload(struct drm_device *drm)
{
struct host1x_device *device = to_host1x_device(drm-dev);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
drm_kms_helper_poll_fini(drm);
@@ -82,6 +94,11 @@ static int tegra_drm_unload(struct drm_device *drm)
if (err 0)
return err;
+ if (tegra-domain) {
+ iommu_domain_free(tegra-domain);
+ drm_mm_takedown(tegra-mm);
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 96d754e7b3eb..a07c796b7edc 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -39,6 +39,9 @@ struct tegra_fbdev {
struct tegra_drm {
struct drm_device *drm;
+ struct iommu_domain *domain;
+ struct drm_mm mm;
+
struct mutex clients_lock;
struct list_head clients;
diff --git
Re: [RFC 09/10] drm/tegra: Add IOMMU support
On Friday 27 June 2014 12:46:14 Hiroshi DOyu wrote:
Thierry Reding thierry.red...@gmail.com writes:
From: Thierry Reding tred...@nvidia.com
When an IOMMU device is available on the platform bus, allocate an IOMMU
domain and attach the display controllers to it. The display controllers
can then scan out non-contiguous buffers by mapping them through the
IOMMU.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
@@ -1283,8 +1284,18 @@ static int tegra_dc_init(struct host1x_client
*client)
{
struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
+ if (tegra-domain) {
+ err = iommu_attach_device(tegra-domain, dc-dev);
I wanted to keep device drivers iommu-free with the following:
http://patchwork.ozlabs.org/patch/354074/
We definitely need something like your series to make iommus work transparently
on ARM for normal devices, using the of_dma_configure() to look up the correct
iommu per device and initialize it.
However, any devices that work with multiple iommu domains cannot do that
and still need to use the iommu API directy. I believe the tegra drm code
falls into this category.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 09/10] drm/tegra: Add IOMMU support
On Fri, Jun 27, 2014 at 12:46:14PM +0300, Hiroshi DOyu wrote:
Thierry Reding thierry.red...@gmail.com writes:
[...]
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
[...]
+ if (tegra-domain) {
+ err = iommu_attach_device(tegra-domain, dc-dev);
I wanted to keep device drivers iommu-free with the following:
http://patchwork.ozlabs.org/patch/354074/
That patch only addresses the probe ordering problem that happens if the
user of an IOMMU is probed before the IOMMU. What this patch does is a
whole lot more.
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 59736bb810cd..1d2bbafad982 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -8,6 +8,7 @@
*/
#include linux/host1x.h
+#include linux/iommu.h
#include drm.h
#include gem.h
@@ -33,6 +34,16 @@ static int tegra_drm_load(struct drm_device *drm,
unsigned long flags)
if (!tegra)
return -ENOMEM;
+ if (iommu_present(platform_bus_type)) {
+ tegra-domain = iommu_domain_alloc(platform_bus_type);
Can we use dma_iommu_mapping instead of domain?
I thought that DMA API is on the top of IOMMU API so that it may be
cleaner to use only DMA API.
Using the DMA API doesn't work for Tegra DRM because it assumes a 1:1
mapping between a device and an IOMMU domain. For Tegra DRM we have two
devices (two display controllers) that need to be able to access the
same buffers, therefore they need to share one IOMMU domain. This can't
be done using the DMA API.
The DMA API is fine to be used by devices that operate on private DMA
buffers (SDMMC, USB, ...).
iommu_map_sg() could be implemented as iommu_ops-map_sg() for the
better perf since iommu_map() needs some pagetable cache operations. If
we do those cache operations at once, it would bring some perf benefit.
Yes, I agree that eventually this should be moved into the IOMMU core.
We could add a .map_sg() to IOMMU ops for devices where mapping a whole
sg_table at once would have significant performance benefits and change
this generic implementation to be used by devices that don't implement
.map_sg(). Then the IOMMU core's iommu_map_sg() can call into the driver
directly or fallback to the generic implementation.
I think that we don't need unmap_sg(), instead normal iommu_unmap() for
a whole area could do the same at once?
Yes, I suppose that's true. I'll see if it can be safely dropped. It
might give us the same benefit as the iommu_map_sg() regarding cache
maintenance, though.
+static int iommu_unmap_sg(struct iommu_domain *domain, struct sg_table
*sgt,
+ dma_addr_t iova)
+{
+ unsigned long offset = 0;
+ struct scatterlist *sg;
+ unsigned int i;
+
+ for_each_sg(sgt-sgl, sg, sgt-nents, i) {
+ dma_addr_t phys = sg_phys(sg);
+ size_t length = sg-offset;
+
+ phys = sg_phys(sg) - sg-offset;
+ length = sg-length + sg-offset;
+
+ iommu_unmap(domain, iova + offset, length);
+ offset += length;
+ }
+
+ return 0;
+}
Can the rest of IOMMU API be replaced with DMA API too?
As I explained above, I don't see how it could be done for this driver.
But I don't think it has to. After all the IOMMU API does exist, so we
shouldn't shy away from using it when appropriate.
Thierry
pgpHVro3zbV17.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 09/10] drm/tegra: Add IOMMU support
Arnd Bergmann a...@arndb.de writes:
On Friday 27 June 2014 12:46:14 Hiroshi DOyu wrote:
Thierry Reding thierry.red...@gmail.com writes:
From: Thierry Reding tred...@nvidia.com
When an IOMMU device is available on the platform bus, allocate an IOMMU
domain and attach the display controllers to it. The display controllers
can then scan out non-contiguous buffers by mapping them through the
IOMMU.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
@@ -1283,8 +1284,18 @@ static int tegra_dc_init(struct host1x_client
*client)
{
struct drm_device *drm = dev_get_drvdata(client-parent);
struct tegra_dc *dc = host1x_client_to_dc(client);
+ struct tegra_drm *tegra = drm-dev_private;
int err;
+ if (tegra-domain) {
+ err = iommu_attach_device(tegra-domain, dc-dev);
I wanted to keep device drivers iommu-free with the following:
http://patchwork.ozlabs.org/patch/354074/
We definitely need something like your series to make iommus work
transparently
on ARM for normal devices, using the of_dma_configure() to look up the correct
iommu per device and initialize it.
OK
However, any devices that work with multiple iommu domains cannot do that
and still need to use the iommu API directy. I believe the tegra drm code
falls into this category.
I think that the similar thing can be said to DMA API vs IOMMU
API. Most of traditional devices would be ok with the existing DMA API
which can be backed by IOMMU. OTOH, some smart devices may need to dive
into IOMMU API for more precise control of it.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
