[PATCH v1 0/2] drm/komeda: Add SMMU support on Komeda driver

[PATCH v1 0/2] drm/komeda: Add SMMU support on Komeda driver

[Lowry Li (Arm Technology China)]

Hi,

This serie aims at adding the support for SMMU on Komeda driver.
Also updates the device-tree doc about how to enable SMMU by devicetree.

This patch series depends on:
- https://patchwork.freedesktop.org/series/58710/
- https://patchwork.freedesktop.org/series/59000/
- https://patchwork.freedesktop.org/series/59002/

Thanks,
Lowry

Lowry Li (Arm Technology China) (2):
  drm/komeda: Adds SMMU support
  dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree

 .../devicetree/bindings/display/arm,komeda.txt     |  7 ++++
 .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
 drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
 drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
 drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
 .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
 .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
 7 files changed, 89 insertions(+)

-- 
1.9.1

[PATCH v1 0/2] drm/komeda: Add SMMU support on Komeda driver

[Lowry Li (Arm Technology China)]

Hi,

This serie aims at adding the support for SMMU on Komeda driver.
Also updates the device-tree doc about how to enable SMMU by devicetree.

This patch series depends on:
- https://patchwork.freedesktop.org/series/58710/
- https://patchwork.freedesktop.org/series/59000/
- https://patchwork.freedesktop.org/series/59002/

Thanks,
Lowry

Lowry Li (Arm Technology China) (2):
  drm/komeda: Adds SMMU support
  dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree

 .../devicetree/bindings/display/arm,komeda.txt     |  7 ++++
 .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
 drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
 drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
 drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
 .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
 .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
 7 files changed, 89 insertions(+)

-- 
1.9.1

[PATCH v1 1/2] drm/komeda: Adds SMMU support

[Lowry Li (Arm Technology China)]

Adds iommu_connect and disconnect for SMMU support, and configures
TBU translation once SMMU has been attached to the display device.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
 drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
 drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
 drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
 .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
 .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
 6 files changed, 82 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index 33ca171..9065040 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
 	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
 	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
 
+	if (kfb->is_va)
+		ctrl |= L_TBU_EN;
 	malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
 }
 
@@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component *c,
 			       fb->pitches[i] & 0xFFFF);
 	}
 
+	if (kfb->is_va)
+		ctrl |= LW_TBU_EN;
+
 	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
 	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
 	malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
index 9603de9..45c98a7 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
@@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
 	table->n_formats = ARRAY_SIZE(d71_format_caps_table);
 }
 
+static int d71_connect_iommu(struct komeda_dev *mdev)
+{
+	struct d71_dev *d71 = mdev->chip_data;
+	u32 __iomem *reg = d71->gcu_addr;
+	u32 check_bits = (d71->num_pipelines == 2) ?
+			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
+	int i, ret;
+
+	if (!d71->integrates_tbu)
+		return -1;
+
+	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
+
+	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
+			100, 1000, 1000);
+	if (ret <= 0) {
+		DRM_ERROR("connect to TCU timeout!\n");
+		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
+		return -ETIMEDOUT;
+	}
+
+	for (i = 0; i < d71->num_pipelines; i++)
+		malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
+				    LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
+	return 0;
+}
+
+static int d71_disconnect_iommu(struct komeda_dev *mdev)
+{
+	struct d71_dev *d71 = mdev->chip_data;
+	u32 __iomem *reg = d71->gcu_addr;
+	u32 check_bits = (d71->num_pipelines == 2) ?
+			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
+	int ret;
+
+	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
+
+	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
+			100, 1000, 1000);
+	if (ret <= 0) {
+		DRM_ERROR("disconnect from TCU timeout!\n");
+		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
+	}
+
+	return ret > 0 ? 0 : -1;
+}
+
 static struct komeda_dev_funcs d71_chip_funcs = {
 	.init_format_table = d71_init_fmt_tbl,
 	.enum_resources	= d71_enum_resources,
@@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
 	.on_off_vblank	= d71_on_off_vblank,
 	.change_opmode	= d71_change_opmode,
 	.flush		= d71_flush,
+	.connect_iommu	= d71_connect_iommu,
+	.disconnect_iommu = d71_disconnect_iommu,
 };
 
 struct komeda_dev_funcs *
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index e4e5b58..2d97c82 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -251,6 +251,18 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
 	dev->dma_parms = &mdev->dma_parms;
 	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
 
+	mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
+	if (!mdev->iommu)
+		DRM_INFO("continue without IOMMU support!\n");
+
+	if (mdev->iommu && mdev->funcs->connect_iommu) {
+		err = mdev->funcs->connect_iommu(mdev);
+		if (err) {
+			DRM_ERROR("connect iommu failed.\n");
+			goto err_cleanup;
+		}
+	}
+
 	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
 	if (err) {
 		DRM_ERROR("create sysfs group failed.\n");
@@ -280,6 +292,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
 	debugfs_remove_recursive(mdev->debugfs_root);
 #endif
 
+	if (mdev->iommu && mdev->funcs->disconnect_iommu)
+		if (mdev->funcs->disconnect_iommu(mdev))
+			DRM_ERROR("disconnect iommu failed.\n");
+	mdev->iommu = NULL;
+
 	for (i = 0; i < mdev->n_pipelines; i++) {
 		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
 		mdev->pipelines[i] = NULL;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
index 83ace71..dac1eda 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
@@ -92,6 +92,10 @@ struct komeda_dev_funcs {
 	int (*enum_resources)(struct komeda_dev *mdev);
 	/** @cleanup: call to chip to cleanup komeda_dev->chip data */
 	void (*cleanup)(struct komeda_dev *mdev);
+	/** @connect_iommu: Optional, connect to external iommu */
+	int (*connect_iommu)(struct komeda_dev *mdev);
+	/** @disconnect_iommu: Optional, disconnect to external iommu */
+	int (*disconnect_iommu)(struct komeda_dev *mdev);
 	/**
 	 * @irq_handler:
 	 *
@@ -184,6 +188,9 @@ struct komeda_dev {
 	 */
 	void *chip_data;
 
+	/** @iommu: iommu domain */
+	struct iommu_domain *iommu;
+
 	/** @debugfs_root: root directory of komeda debugfs */
 	struct dentry *debugfs_root;
 };
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
index d5822a3..360ab70 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
@@ -201,6 +201,8 @@ struct drm_framebuffer *
 		goto err_cleanup;
 	}
 
+	kfb->is_va = mdev->iommu ? true : false;
+
 	return &kfb->base;
 
 err_cleanup:
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
index 6cbb2f6..f4046e2 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
@@ -21,6 +21,8 @@ struct komeda_fb {
 	 * extends drm_format_info for komeda specific information
 	 */
 	const struct komeda_format_caps *format_caps;
+	/** @is_va: if smmu is enabled, it will be true */
+	bool is_va;
 	/** @aligned_w: aligned frame buffer width */
 	u32 aligned_w;
 	/** @aligned_h: aligned frame buffer height */
-- 
1.9.1

[PATCH v1 1/2] drm/komeda: Adds SMMU support

[Lowry Li (Arm Technology China)]

Adds iommu_connect and disconnect for SMMU support, and configures
TBU translation once SMMU has been attached to the display device.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
 drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
 drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
 drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
 .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
 .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
 6 files changed, 82 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index 33ca171..9065040 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
 	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
 	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
 
+	if (kfb->is_va)
+		ctrl |= L_TBU_EN;
 	malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
 }
 
@@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component *c,
 			       fb->pitches[i] & 0xFFFF);
 	}
 
+	if (kfb->is_va)
+		ctrl |= LW_TBU_EN;
+
 	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
 	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
 	malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
index 9603de9..45c98a7 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
@@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
 	table->n_formats = ARRAY_SIZE(d71_format_caps_table);
 }
 
+static int d71_connect_iommu(struct komeda_dev *mdev)
+{
+	struct d71_dev *d71 = mdev->chip_data;
+	u32 __iomem *reg = d71->gcu_addr;
+	u32 check_bits = (d71->num_pipelines == 2) ?
+			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
+	int i, ret;
+
+	if (!d71->integrates_tbu)
+		return -1;
+
+	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
+
+	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
+			100, 1000, 1000);
+	if (ret <= 0) {
+		DRM_ERROR("connect to TCU timeout!\n");
+		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
+		return -ETIMEDOUT;
+	}
+
+	for (i = 0; i < d71->num_pipelines; i++)
+		malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
+				    LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
+	return 0;
+}
+
+static int d71_disconnect_iommu(struct komeda_dev *mdev)
+{
+	struct d71_dev *d71 = mdev->chip_data;
+	u32 __iomem *reg = d71->gcu_addr;
+	u32 check_bits = (d71->num_pipelines == 2) ?
+			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
+	int ret;
+
+	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
+
+	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
+			100, 1000, 1000);
+	if (ret <= 0) {
+		DRM_ERROR("disconnect from TCU timeout!\n");
+		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
+	}
+
+	return ret > 0 ? 0 : -1;
+}
+
 static struct komeda_dev_funcs d71_chip_funcs = {
 	.init_format_table = d71_init_fmt_tbl,
 	.enum_resources	= d71_enum_resources,
@@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
 	.on_off_vblank	= d71_on_off_vblank,
 	.change_opmode	= d71_change_opmode,
 	.flush		= d71_flush,
+	.connect_iommu	= d71_connect_iommu,
+	.disconnect_iommu = d71_disconnect_iommu,
 };
 
 struct komeda_dev_funcs *
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index e4e5b58..2d97c82 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -251,6 +251,18 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
 	dev->dma_parms = &mdev->dma_parms;
 	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
 
+	mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
+	if (!mdev->iommu)
+		DRM_INFO("continue without IOMMU support!\n");
+
+	if (mdev->iommu && mdev->funcs->connect_iommu) {
+		err = mdev->funcs->connect_iommu(mdev);
+		if (err) {
+			DRM_ERROR("connect iommu failed.\n");
+			goto err_cleanup;
+		}
+	}
+
 	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
 	if (err) {
 		DRM_ERROR("create sysfs group failed.\n");
@@ -280,6 +292,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
 	debugfs_remove_recursive(mdev->debugfs_root);
 #endif
 
+	if (mdev->iommu && mdev->funcs->disconnect_iommu)
+		if (mdev->funcs->disconnect_iommu(mdev))
+			DRM_ERROR("disconnect iommu failed.\n");
+	mdev->iommu = NULL;
+
 	for (i = 0; i < mdev->n_pipelines; i++) {
 		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
 		mdev->pipelines[i] = NULL;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
index 83ace71..dac1eda 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
@@ -92,6 +92,10 @@ struct komeda_dev_funcs {
 	int (*enum_resources)(struct komeda_dev *mdev);
 	/** @cleanup: call to chip to cleanup komeda_dev->chip data */
 	void (*cleanup)(struct komeda_dev *mdev);
+	/** @connect_iommu: Optional, connect to external iommu */
+	int (*connect_iommu)(struct komeda_dev *mdev);
+	/** @disconnect_iommu: Optional, disconnect to external iommu */
+	int (*disconnect_iommu)(struct komeda_dev *mdev);
 	/**
 	 * @irq_handler:
 	 *
@@ -184,6 +188,9 @@ struct komeda_dev {
 	 */
 	void *chip_data;
 
+	/** @iommu: iommu domain */
+	struct iommu_domain *iommu;
+
 	/** @debugfs_root: root directory of komeda debugfs */
 	struct dentry *debugfs_root;
 };
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
index d5822a3..360ab70 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
@@ -201,6 +201,8 @@ struct drm_framebuffer *
 		goto err_cleanup;
 	}
 
+	kfb->is_va = mdev->iommu ? true : false;
+
 	return &kfb->base;
 
 err_cleanup:
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
index 6cbb2f6..f4046e2 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
@@ -21,6 +21,8 @@ struct komeda_fb {
 	 * extends drm_format_info for komeda specific information
 	 */
 	const struct komeda_format_caps *format_caps;
+	/** @is_va: if smmu is enabled, it will be true */
+	bool is_va;
 	/** @aligned_w: aligned frame buffer width */
 	u32 aligned_w;
 	/** @aligned_h: aligned frame buffer height */
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 2/2] dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree

[Lowry Li (Arm Technology China)]

Updates the device-tree doc about how to enable SMMU by devicetree.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 Documentation/devicetree/bindings/display/arm,komeda.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/arm,komeda.txt b/Documentation/devicetree/bindings/display/arm,komeda.txt
index 02b2265..b12c045 100644
--- a/Documentation/devicetree/bindings/display/arm,komeda.txt
+++ b/Documentation/devicetree/bindings/display/arm,komeda.txt
@@ -11,6 +11,10 @@ Required properties:
       - "pclk": for the APB interface clock
 - #address-cells: Must be 1
 - #size-cells: Must be 0
+- iommus: configure the stream id to IOMMU, Must be configured if want to
+    enable iommu in display. for how to configure this node please reference
+        devicetree/bindings/iommu/arm,smmu-v3.txt,
+        devicetree/bindings/iommu/iommu.txt
 
 Required properties for sub-node: pipeline@nq
 Each device contains one or two pipeline sub-nodes (at least one), each
@@ -44,6 +48,9 @@ Example:
 		interrupts = <0 168 4>;
 		clocks = <&dpu_mclk>, <&dpu_aclk>;
 		clock-names = "mclk", "pclk";
+		iommus = <&smmu 0>, <&smmu 1>, <&smmu 2>, <&smmu 3>,
+			<&smmu 4>, <&smmu 5>, <&smmu 6>, <&smmu 7>,
+			<&smmu 8>, <&smmu 9>;
 
 		dp0_pipe0: pipeline@0 {
 			clocks = <&fpgaosc2>, <&dpu_aclk>;
-- 
1.9.1

[PATCH v1 2/2] dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree

[Lowry Li (Arm Technology China)]

Updates the device-tree doc about how to enable SMMU by devicetree.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 Documentation/devicetree/bindings/display/arm,komeda.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/arm,komeda.txt b/Documentation/devicetree/bindings/display/arm,komeda.txt
index 02b2265..b12c045 100644
--- a/Documentation/devicetree/bindings/display/arm,komeda.txt
+++ b/Documentation/devicetree/bindings/display/arm,komeda.txt
@@ -11,6 +11,10 @@ Required properties:
       - "pclk": for the APB interface clock
 - #address-cells: Must be 1
 - #size-cells: Must be 0
+- iommus: configure the stream id to IOMMU, Must be configured if want to
+    enable iommu in display. for how to configure this node please reference
+        devicetree/bindings/iommu/arm,smmu-v3.txt,
+        devicetree/bindings/iommu/iommu.txt
 
 Required properties for sub-node: pipeline@nq
 Each device contains one or two pipeline sub-nodes (at least one), each
@@ -44,6 +48,9 @@ Example:
 		interrupts = <0 168 4>;
 		clocks = <&dpu_mclk>, <&dpu_aclk>;
 		clock-names = "mclk", "pclk";
+		iommus = <&smmu 0>, <&smmu 1>, <&smmu 2>, <&smmu 3>,
+			<&smmu 4>, <&smmu 5>, <&smmu 6>, <&smmu 7>,
+			<&smmu 8>, <&smmu 9>;
 
 		dp0_pipe0: pipeline@0 {
 			clocks = <&fpgaosc2>, <&dpu_aclk>;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v1,1/2] drm/komeda: Adds SMMU support

[james qian wang (Arm Technology China)]

On Tue, Apr 30, 2019 at 06:19:29AM +0000, Lowry Li (Arm Technology China) wrote:
> Adds iommu_connect and disconnect for SMMU support, and configures
> TBU translation once SMMU has been attached to the display device.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
>  drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
>  .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
>  .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
>  6 files changed, 82 insertions(+)
> 
> -- 
> 1.9.1
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index 33ca171..9065040 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
>  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
>  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
>  
> +	if (kfb->is_va)
> +		ctrl |= L_TBU_EN;
>  	malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
>  }
>  
> @@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component *c,
>  			       fb->pitches[i] & 0xFFFF);
>  	}
>  
> +	if (kfb->is_va)
> +		ctrl |= LW_TBU_EN;
> +
>  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
>  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
>  	malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> index 9603de9..45c98a7 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> @@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>  	table->n_formats = ARRAY_SIZE(d71_format_caps_table);
>  }
>  
> +static int d71_connect_iommu(struct komeda_dev *mdev)
> +{
> +	struct d71_dev *d71 = mdev->chip_data;
> +	u32 __iomem *reg = d71->gcu_addr;
> +	u32 check_bits = (d71->num_pipelines == 2) ?
> +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> +	int i, ret;
> +
> +	if (!d71->integrates_tbu)
> +		return -1;
> +
> +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
> +
> +	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> +			100, 1000, 1000);
> +	if (ret <= 0) {
> +		DRM_ERROR("connect to TCU timeout!\n");
> +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> +		return -ETIMEDOUT;
> +	}
> +
> +	for (i = 0; i < d71->num_pipelines; i++)
> +		malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
> +				    LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
> +	return 0;
> +}
> +
> +static int d71_disconnect_iommu(struct komeda_dev *mdev)
> +{
> +	struct d71_dev *d71 = mdev->chip_data;
> +	u32 __iomem *reg = d71->gcu_addr;
> +	u32 check_bits = (d71->num_pipelines == 2) ?
> +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> +	int ret;
> +
> +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
> +
> +	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> +			100, 1000, 1000);
> +	if (ret <= 0) {
> +		DRM_ERROR("disconnect from TCU timeout!\n");
> +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> +	}
> +
> +	return ret > 0 ? 0 : -1;
> +}
> +
>  static struct komeda_dev_funcs d71_chip_funcs = {
>  	.init_format_table = d71_init_fmt_tbl,
>  	.enum_resources	= d71_enum_resources,
> @@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>  	.on_off_vblank	= d71_on_off_vblank,
>  	.change_opmode	= d71_change_opmode,
>  	.flush		= d71_flush,
> +	.connect_iommu	= d71_connect_iommu,
> +	.disconnect_iommu = d71_disconnect_iommu,
>  };
>  
>  struct komeda_dev_funcs *
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index e4e5b58..2d97c82 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -251,6 +251,18 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>  	dev->dma_parms = &mdev->dma_parms;
>  	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
>  
> +	mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
> +	if (!mdev->iommu)
> +		DRM_INFO("continue without IOMMU support!\n");
> +
> +	if (mdev->iommu && mdev->funcs->connect_iommu) {
> +		err = mdev->funcs->connect_iommu(mdev);
> +		if (err) {
> +			DRM_ERROR("connect iommu failed.\n");
> +			goto err_cleanup;
> +		}
> +	}
> +
>  	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
>  	if (err) {
>  		DRM_ERROR("create sysfs group failed.\n");
> @@ -280,6 +292,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>  	debugfs_remove_recursive(mdev->debugfs_root);
>  #endif
>  
> +	if (mdev->iommu && mdev->funcs->disconnect_iommu)
> +		if (mdev->funcs->disconnect_iommu(mdev))
> +			DRM_ERROR("disconnect iommu failed.\n");
> +	mdev->iommu = NULL;
> +
>  	for (i = 0; i < mdev->n_pipelines; i++) {
>  		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
>  		mdev->pipelines[i] = NULL;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index 83ace71..dac1eda 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -92,6 +92,10 @@ struct komeda_dev_funcs {
>  	int (*enum_resources)(struct komeda_dev *mdev);
>  	/** @cleanup: call to chip to cleanup komeda_dev->chip data */
>  	void (*cleanup)(struct komeda_dev *mdev);
> +	/** @connect_iommu: Optional, connect to external iommu */
> +	int (*connect_iommu)(struct komeda_dev *mdev);
> +	/** @disconnect_iommu: Optional, disconnect to external iommu */
> +	int (*disconnect_iommu)(struct komeda_dev *mdev);
>  	/**
>  	 * @irq_handler:
>  	 *
> @@ -184,6 +188,9 @@ struct komeda_dev {
>  	 */
>  	void *chip_data;
>  
> +	/** @iommu: iommu domain */
> +	struct iommu_domain *iommu;
> +
>  	/** @debugfs_root: root directory of komeda debugfs */
>  	struct dentry *debugfs_root;
>  };
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> index d5822a3..360ab70 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> @@ -201,6 +201,8 @@ struct drm_framebuffer *
>  		goto err_cleanup;
>  	}
>  
> +	kfb->is_va = mdev->iommu ? true : false;
> +
>  	return &kfb->base;
>  
>  err_cleanup:
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> index 6cbb2f6..f4046e2 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> @@ -21,6 +21,8 @@ struct komeda_fb {
>  	 * extends drm_format_info for komeda specific information
>  	 */
>  	const struct komeda_format_caps *format_caps;
> +	/** @is_va: if smmu is enabled, it will be true */
> +	bool is_va;
>  	/** @aligned_w: aligned frame buffer width */
>  	u32 aligned_w;
>  	/** @aligned_h: aligned frame buffer height */

Looks good to me.

James.
-- 
Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>

Re: [v1,1/2] drm/komeda: Adds SMMU support

[james qian wang (Arm Technology China)]

On Tue, Apr 30, 2019 at 06:19:29AM +0000, Lowry Li (Arm Technology China) wrote:
> Adds iommu_connect and disconnect for SMMU support, and configures
> TBU translation once SMMU has been attached to the display device.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
>  drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
>  .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
>  .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
>  6 files changed, 82 insertions(+)
> 
> -- 
> 1.9.1
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index 33ca171..9065040 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
>  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
>  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
>  
> +	if (kfb->is_va)
> +		ctrl |= L_TBU_EN;
>  	malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
>  }
>  
> @@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component *c,
>  			       fb->pitches[i] & 0xFFFF);
>  	}
>  
> +	if (kfb->is_va)
> +		ctrl |= LW_TBU_EN;
> +
>  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
>  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
>  	malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> index 9603de9..45c98a7 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> @@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>  	table->n_formats = ARRAY_SIZE(d71_format_caps_table);
>  }
>  
> +static int d71_connect_iommu(struct komeda_dev *mdev)
> +{
> +	struct d71_dev *d71 = mdev->chip_data;
> +	u32 __iomem *reg = d71->gcu_addr;
> +	u32 check_bits = (d71->num_pipelines == 2) ?
> +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> +	int i, ret;
> +
> +	if (!d71->integrates_tbu)
> +		return -1;
> +
> +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
> +
> +	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> +			100, 1000, 1000);
> +	if (ret <= 0) {
> +		DRM_ERROR("connect to TCU timeout!\n");
> +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> +		return -ETIMEDOUT;
> +	}
> +
> +	for (i = 0; i < d71->num_pipelines; i++)
> +		malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
> +				    LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
> +	return 0;
> +}
> +
> +static int d71_disconnect_iommu(struct komeda_dev *mdev)
> +{
> +	struct d71_dev *d71 = mdev->chip_data;
> +	u32 __iomem *reg = d71->gcu_addr;
> +	u32 check_bits = (d71->num_pipelines == 2) ?
> +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> +	int ret;
> +
> +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
> +
> +	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> +			100, 1000, 1000);
> +	if (ret <= 0) {
> +		DRM_ERROR("disconnect from TCU timeout!\n");
> +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> +	}
> +
> +	return ret > 0 ? 0 : -1;
> +}
> +
>  static struct komeda_dev_funcs d71_chip_funcs = {
>  	.init_format_table = d71_init_fmt_tbl,
>  	.enum_resources	= d71_enum_resources,
> @@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>  	.on_off_vblank	= d71_on_off_vblank,
>  	.change_opmode	= d71_change_opmode,
>  	.flush		= d71_flush,
> +	.connect_iommu	= d71_connect_iommu,
> +	.disconnect_iommu = d71_disconnect_iommu,
>  };
>  
>  struct komeda_dev_funcs *
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index e4e5b58..2d97c82 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -251,6 +251,18 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>  	dev->dma_parms = &mdev->dma_parms;
>  	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
>  
> +	mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
> +	if (!mdev->iommu)
> +		DRM_INFO("continue without IOMMU support!\n");
> +
> +	if (mdev->iommu && mdev->funcs->connect_iommu) {
> +		err = mdev->funcs->connect_iommu(mdev);
> +		if (err) {
> +			DRM_ERROR("connect iommu failed.\n");
> +			goto err_cleanup;
> +		}
> +	}
> +
>  	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
>  	if (err) {
>  		DRM_ERROR("create sysfs group failed.\n");
> @@ -280,6 +292,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>  	debugfs_remove_recursive(mdev->debugfs_root);
>  #endif
>  
> +	if (mdev->iommu && mdev->funcs->disconnect_iommu)
> +		if (mdev->funcs->disconnect_iommu(mdev))
> +			DRM_ERROR("disconnect iommu failed.\n");
> +	mdev->iommu = NULL;
> +
>  	for (i = 0; i < mdev->n_pipelines; i++) {
>  		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
>  		mdev->pipelines[i] = NULL;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index 83ace71..dac1eda 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -92,6 +92,10 @@ struct komeda_dev_funcs {
>  	int (*enum_resources)(struct komeda_dev *mdev);
>  	/** @cleanup: call to chip to cleanup komeda_dev->chip data */
>  	void (*cleanup)(struct komeda_dev *mdev);
> +	/** @connect_iommu: Optional, connect to external iommu */
> +	int (*connect_iommu)(struct komeda_dev *mdev);
> +	/** @disconnect_iommu: Optional, disconnect to external iommu */
> +	int (*disconnect_iommu)(struct komeda_dev *mdev);
>  	/**
>  	 * @irq_handler:
>  	 *
> @@ -184,6 +188,9 @@ struct komeda_dev {
>  	 */
>  	void *chip_data;
>  
> +	/** @iommu: iommu domain */
> +	struct iommu_domain *iommu;
> +
>  	/** @debugfs_root: root directory of komeda debugfs */
>  	struct dentry *debugfs_root;
>  };
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> index d5822a3..360ab70 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> @@ -201,6 +201,8 @@ struct drm_framebuffer *
>  		goto err_cleanup;
>  	}
>  
> +	kfb->is_va = mdev->iommu ? true : false;
> +
>  	return &kfb->base;
>  
>  err_cleanup:
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> index 6cbb2f6..f4046e2 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> @@ -21,6 +21,8 @@ struct komeda_fb {
>  	 * extends drm_format_info for komeda specific information
>  	 */
>  	const struct komeda_format_caps *format_caps;
> +	/** @is_va: if smmu is enabled, it will be true */
> +	bool is_va;
>  	/** @aligned_w: aligned frame buffer width */
>  	u32 aligned_w;
>  	/** @aligned_h: aligned frame buffer height */

Looks good to me.

James.
-- 
Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v1, 2/2] dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree

[james qian wang (Arm Technology China)]

On Tue, Apr 30, 2019 at 06:19:34AM +0000, Lowry Li (Arm Technology China) wrote:
> Updates the device-tree doc about how to enable SMMU by devicetree.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  Documentation/devicetree/bindings/display/arm,komeda.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> -- 
> 1.9.1
> 
> diff --git a/Documentation/devicetree/bindings/display/arm,komeda.txt b/Documentation/devicetree/bindings/display/arm,komeda.txt
> index 02b2265..b12c045 100644
> --- a/Documentation/devicetree/bindings/display/arm,komeda.txt
> +++ b/Documentation/devicetree/bindings/display/arm,komeda.txt
> @@ -11,6 +11,10 @@ Required properties:
>        - "pclk": for the APB interface clock
>  - #address-cells: Must be 1
>  - #size-cells: Must be 0
> +- iommus: configure the stream id to IOMMU, Must be configured if want to
> +    enable iommu in display. for how to configure this node please reference
> +        devicetree/bindings/iommu/arm,smmu-v3.txt,
> +        devicetree/bindings/iommu/iommu.txt
>  
>  Required properties for sub-node: pipeline@nq
>  Each device contains one or two pipeline sub-nodes (at least one), each
> @@ -44,6 +48,9 @@ Example:
>  		interrupts = <0 168 4>;
>  		clocks = <&dpu_mclk>, <&dpu_aclk>;
>  		clock-names = "mclk", "pclk";
> +		iommus = <&smmu 0>, <&smmu 1>, <&smmu 2>, <&smmu 3>,
> +			<&smmu 4>, <&smmu 5>, <&smmu 6>, <&smmu 7>,
> +			<&smmu 8>, <&smmu 9>;
>  
>  		dp0_pipe0: pipeline@0 {
>  			clocks = <&fpgaosc2>, <&dpu_aclk>;

Looks good to me

James
-- 
Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>

Re: [v1, 2/2] dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree

[james qian wang (Arm Technology China)]

On Tue, Apr 30, 2019 at 06:19:34AM +0000, Lowry Li (Arm Technology China) wrote:
> Updates the device-tree doc about how to enable SMMU by devicetree.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  Documentation/devicetree/bindings/display/arm,komeda.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> -- 
> 1.9.1
> 
> diff --git a/Documentation/devicetree/bindings/display/arm,komeda.txt b/Documentation/devicetree/bindings/display/arm,komeda.txt
> index 02b2265..b12c045 100644
> --- a/Documentation/devicetree/bindings/display/arm,komeda.txt
> +++ b/Documentation/devicetree/bindings/display/arm,komeda.txt
> @@ -11,6 +11,10 @@ Required properties:
>        - "pclk": for the APB interface clock
>  - #address-cells: Must be 1
>  - #size-cells: Must be 0
> +- iommus: configure the stream id to IOMMU, Must be configured if want to
> +    enable iommu in display. for how to configure this node please reference
> +        devicetree/bindings/iommu/arm,smmu-v3.txt,
> +        devicetree/bindings/iommu/iommu.txt
>  
>  Required properties for sub-node: pipeline@nq
>  Each device contains one or two pipeline sub-nodes (at least one), each
> @@ -44,6 +48,9 @@ Example:
>  		interrupts = <0 168 4>;
>  		clocks = <&dpu_mclk>, <&dpu_aclk>;
>  		clock-names = "mclk", "pclk";
> +		iommus = <&smmu 0>, <&smmu 1>, <&smmu 2>, <&smmu 3>,
> +			<&smmu 4>, <&smmu 5>, <&smmu 6>, <&smmu 7>,
> +			<&smmu 8>, <&smmu 9>;
>  
>  		dp0_pipe0: pipeline@0 {
>  			clocks = <&fpgaosc2>, <&dpu_aclk>;

Looks good to me

James
-- 
Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>

Re: [PATCH v1 1/2] drm/komeda: Adds SMMU support

[Liviu Dudau]

Hi Lowry,

On Tue, Apr 30, 2019 at 07:19:29AM +0100, Lowry Li (Arm Technology China) wrote:
> Adds iommu_connect and disconnect for SMMU support, and configures
> TBU translation once SMMU has been attached to the display device.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
>  drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
>  .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
>  .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
>  6 files changed, 82 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index 33ca171..9065040 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
>  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
>  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
>  
> +	if (kfb->is_va)
> +		ctrl |= L_TBU_EN;
>  	malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
>  }
>  
> @@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component *c,
>  			       fb->pitches[i] & 0xFFFF);
>  	}
>  
> +	if (kfb->is_va)
> +		ctrl |= LW_TBU_EN;
> +
>  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
>  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
>  	malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> index 9603de9..45c98a7 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> @@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>  	table->n_formats = ARRAY_SIZE(d71_format_caps_table);
>  }
>  
> +static int d71_connect_iommu(struct komeda_dev *mdev)
> +{
> +	struct d71_dev *d71 = mdev->chip_data;
> +	u32 __iomem *reg = d71->gcu_addr;
> +	u32 check_bits = (d71->num_pipelines == 2) ?
> +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> +	int i, ret;
> +
> +	if (!d71->integrates_tbu)
> +		return -1;
> +
> +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
> +
> +	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> +			100, 1000, 1000);
> +	if (ret <= 0) {
> +		DRM_ERROR("connect to TCU timeout!\n");
> +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> +		return -ETIMEDOUT;
> +	}
> +
> +	for (i = 0; i < d71->num_pipelines; i++)
> +		malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
> +				    LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
> +	return 0;
> +}
> +
> +static int d71_disconnect_iommu(struct komeda_dev *mdev)
> +{
> +	struct d71_dev *d71 = mdev->chip_data;
> +	u32 __iomem *reg = d71->gcu_addr;
> +	u32 check_bits = (d71->num_pipelines == 2) ?
> +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> +	int ret;
> +
> +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
> +
> +	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> +			100, 1000, 1000);
> +	if (ret <= 0) {
> +		DRM_ERROR("disconnect from TCU timeout!\n");
> +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> +	}
> +
> +	return ret > 0 ? 0 : -1;

For consistency with d71_connect_iommu() you should return -ETIMEDOUT as well.
I'm sending a patch to convert dp_wait_cond() to return that when it times out,
so we can be consistent in the future.

> +}
> +
>  static struct komeda_dev_funcs d71_chip_funcs = {
>  	.init_format_table = d71_init_fmt_tbl,
>  	.enum_resources	= d71_enum_resources,
> @@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>  	.on_off_vblank	= d71_on_off_vblank,
>  	.change_opmode	= d71_change_opmode,
>  	.flush		= d71_flush,
> +	.connect_iommu	= d71_connect_iommu,
> +	.disconnect_iommu = d71_disconnect_iommu,
>  };
>  
>  struct komeda_dev_funcs *
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index e4e5b58..2d97c82 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -251,6 +251,18 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>  	dev->dma_parms = &mdev->dma_parms;
>  	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
>  
> +	mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
> +	if (!mdev->iommu)
> +		DRM_INFO("continue without IOMMU support!\n");
> +
> +	if (mdev->iommu && mdev->funcs->connect_iommu) {
> +		err = mdev->funcs->connect_iommu(mdev);
> +		if (err) {
> +			DRM_ERROR("connect iommu failed.\n");

connect_iommu() has already printed a DRM_ERROR(), do we need another one here?

> +			goto err_cleanup;

err_cleanup doesn't set the new mdev->iommu to NULL, forgot to add it?


> +		}
> +	}
> +
>  	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
>  	if (err) {
>  		DRM_ERROR("create sysfs group failed.\n");
> @@ -280,6 +292,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>  	debugfs_remove_recursive(mdev->debugfs_root);
>  #endif
>  
> +	if (mdev->iommu && mdev->funcs->disconnect_iommu)
> +		if (mdev->funcs->disconnect_iommu(mdev))
> +			DRM_ERROR("disconnect iommu failed.\n");

Same comment an in komeda_dev_create(), disconnect_iommu() already printed a DRM_ERROR()

> +	mdev->iommu = NULL;
> +
>  	for (i = 0; i < mdev->n_pipelines; i++) {
>  		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
>  		mdev->pipelines[i] = NULL;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index 83ace71..dac1eda 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -92,6 +92,10 @@ struct komeda_dev_funcs {
>  	int (*enum_resources)(struct komeda_dev *mdev);
>  	/** @cleanup: call to chip to cleanup komeda_dev->chip data */
>  	void (*cleanup)(struct komeda_dev *mdev);
> +	/** @connect_iommu: Optional, connect to external iommu */
> +	int (*connect_iommu)(struct komeda_dev *mdev);
> +	/** @disconnect_iommu: Optional, disconnect to external iommu */
> +	int (*disconnect_iommu)(struct komeda_dev *mdev);
>  	/**
>  	 * @irq_handler:
>  	 *
> @@ -184,6 +188,9 @@ struct komeda_dev {
>  	 */
>  	void *chip_data;
>  
> +	/** @iommu: iommu domain */
> +	struct iommu_domain *iommu;
> +
>  	/** @debugfs_root: root directory of komeda debugfs */
>  	struct dentry *debugfs_root;
>  };
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> index d5822a3..360ab70 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> @@ -201,6 +201,8 @@ struct drm_framebuffer *
>  		goto err_cleanup;
>  	}
>  
> +	kfb->is_va = mdev->iommu ? true : false;
> +
>  	return &kfb->base;
>  
>  err_cleanup:
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> index 6cbb2f6..f4046e2 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> @@ -21,6 +21,8 @@ struct komeda_fb {
>  	 * extends drm_format_info for komeda specific information
>  	 */
>  	const struct komeda_format_caps *format_caps;
> +	/** @is_va: if smmu is enabled, it will be true */
> +	bool is_va;
>  	/** @aligned_w: aligned frame buffer width */
>  	u32 aligned_w;
>  	/** @aligned_h: aligned frame buffer height */
> -- 
> 1.9.1
> 

Best regards,
Liviu


-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v1 1/2] drm/komeda: Adds SMMU support

[Liviu Dudau]

Hi Lowry,

On Tue, Apr 30, 2019 at 07:19:29AM +0100, Lowry Li (Arm Technology China) wrote:
> Adds iommu_connect and disconnect for SMMU support, and configures
> TBU translation once SMMU has been attached to the display device.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
>  drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
>  .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
>  .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
>  6 files changed, 82 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index 33ca171..9065040 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
>  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
>  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
>  
> +	if (kfb->is_va)
> +		ctrl |= L_TBU_EN;
>  	malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
>  }
>  
> @@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component *c,
>  			       fb->pitches[i] & 0xFFFF);
>  	}
>  
> +	if (kfb->is_va)
> +		ctrl |= LW_TBU_EN;
> +
>  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
>  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
>  	malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> index 9603de9..45c98a7 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> @@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>  	table->n_formats = ARRAY_SIZE(d71_format_caps_table);
>  }
>  
> +static int d71_connect_iommu(struct komeda_dev *mdev)
> +{
> +	struct d71_dev *d71 = mdev->chip_data;
> +	u32 __iomem *reg = d71->gcu_addr;
> +	u32 check_bits = (d71->num_pipelines == 2) ?
> +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> +	int i, ret;
> +
> +	if (!d71->integrates_tbu)
> +		return -1;
> +
> +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
> +
> +	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> +			100, 1000, 1000);
> +	if (ret <= 0) {
> +		DRM_ERROR("connect to TCU timeout!\n");
> +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> +		return -ETIMEDOUT;
> +	}
> +
> +	for (i = 0; i < d71->num_pipelines; i++)
> +		malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
> +				    LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
> +	return 0;
> +}
> +
> +static int d71_disconnect_iommu(struct komeda_dev *mdev)
> +{
> +	struct d71_dev *d71 = mdev->chip_data;
> +	u32 __iomem *reg = d71->gcu_addr;
> +	u32 check_bits = (d71->num_pipelines == 2) ?
> +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> +	int ret;
> +
> +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
> +
> +	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> +			100, 1000, 1000);
> +	if (ret <= 0) {
> +		DRM_ERROR("disconnect from TCU timeout!\n");
> +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> +	}
> +
> +	return ret > 0 ? 0 : -1;

For consistency with d71_connect_iommu() you should return -ETIMEDOUT as well.
I'm sending a patch to convert dp_wait_cond() to return that when it times out,
so we can be consistent in the future.

> +}
> +
>  static struct komeda_dev_funcs d71_chip_funcs = {
>  	.init_format_table = d71_init_fmt_tbl,
>  	.enum_resources	= d71_enum_resources,
> @@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>  	.on_off_vblank	= d71_on_off_vblank,
>  	.change_opmode	= d71_change_opmode,
>  	.flush		= d71_flush,
> +	.connect_iommu	= d71_connect_iommu,
> +	.disconnect_iommu = d71_disconnect_iommu,
>  };
>  
>  struct komeda_dev_funcs *
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index e4e5b58..2d97c82 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -251,6 +251,18 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>  	dev->dma_parms = &mdev->dma_parms;
>  	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
>  
> +	mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
> +	if (!mdev->iommu)
> +		DRM_INFO("continue without IOMMU support!\n");
> +
> +	if (mdev->iommu && mdev->funcs->connect_iommu) {
> +		err = mdev->funcs->connect_iommu(mdev);
> +		if (err) {
> +			DRM_ERROR("connect iommu failed.\n");

connect_iommu() has already printed a DRM_ERROR(), do we need another one here?

> +			goto err_cleanup;

err_cleanup doesn't set the new mdev->iommu to NULL, forgot to add it?


> +		}
> +	}
> +
>  	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
>  	if (err) {
>  		DRM_ERROR("create sysfs group failed.\n");
> @@ -280,6 +292,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>  	debugfs_remove_recursive(mdev->debugfs_root);
>  #endif
>  
> +	if (mdev->iommu && mdev->funcs->disconnect_iommu)
> +		if (mdev->funcs->disconnect_iommu(mdev))
> +			DRM_ERROR("disconnect iommu failed.\n");

Same comment an in komeda_dev_create(), disconnect_iommu() already printed a DRM_ERROR()

> +	mdev->iommu = NULL;
> +
>  	for (i = 0; i < mdev->n_pipelines; i++) {
>  		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
>  		mdev->pipelines[i] = NULL;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index 83ace71..dac1eda 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -92,6 +92,10 @@ struct komeda_dev_funcs {
>  	int (*enum_resources)(struct komeda_dev *mdev);
>  	/** @cleanup: call to chip to cleanup komeda_dev->chip data */
>  	void (*cleanup)(struct komeda_dev *mdev);
> +	/** @connect_iommu: Optional, connect to external iommu */
> +	int (*connect_iommu)(struct komeda_dev *mdev);
> +	/** @disconnect_iommu: Optional, disconnect to external iommu */
> +	int (*disconnect_iommu)(struct komeda_dev *mdev);
>  	/**
>  	 * @irq_handler:
>  	 *
> @@ -184,6 +188,9 @@ struct komeda_dev {
>  	 */
>  	void *chip_data;
>  
> +	/** @iommu: iommu domain */
> +	struct iommu_domain *iommu;
> +
>  	/** @debugfs_root: root directory of komeda debugfs */
>  	struct dentry *debugfs_root;
>  };
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> index d5822a3..360ab70 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> @@ -201,6 +201,8 @@ struct drm_framebuffer *
>  		goto err_cleanup;
>  	}
>  
> +	kfb->is_va = mdev->iommu ? true : false;
> +
>  	return &kfb->base;
>  
>  err_cleanup:
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> index 6cbb2f6..f4046e2 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> @@ -21,6 +21,8 @@ struct komeda_fb {
>  	 * extends drm_format_info for komeda specific information
>  	 */
>  	const struct komeda_format_caps *format_caps;
> +	/** @is_va: if smmu is enabled, it will be true */
> +	bool is_va;
>  	/** @aligned_w: aligned frame buffer width */
>  	u32 aligned_w;
>  	/** @aligned_h: aligned frame buffer height */
> -- 
> 1.9.1
> 

Best regards,
Liviu


-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v1 2/2] dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree

[Liviu Dudau]

On Tue, Apr 30, 2019 at 07:19:34AM +0100, Lowry Li (Arm Technology China) wrote:
> Updates the device-tree doc about how to enable SMMU by devicetree.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>

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

Best regards,
Liviu

> ---
>  Documentation/devicetree/bindings/display/arm,komeda.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/arm,komeda.txt b/Documentation/devicetree/bindings/display/arm,komeda.txt
> index 02b2265..b12c045 100644
> --- a/Documentation/devicetree/bindings/display/arm,komeda.txt
> +++ b/Documentation/devicetree/bindings/display/arm,komeda.txt
> @@ -11,6 +11,10 @@ Required properties:
>        - "pclk": for the APB interface clock
>  - #address-cells: Must be 1
>  - #size-cells: Must be 0
> +- iommus: configure the stream id to IOMMU, Must be configured if want to
> +    enable iommu in display. for how to configure this node please reference
> +        devicetree/bindings/iommu/arm,smmu-v3.txt,
> +        devicetree/bindings/iommu/iommu.txt
>  
>  Required properties for sub-node: pipeline@nq
>  Each device contains one or two pipeline sub-nodes (at least one), each
> @@ -44,6 +48,9 @@ Example:
>  		interrupts = <0 168 4>;
>  		clocks = <&dpu_mclk>, <&dpu_aclk>;
>  		clock-names = "mclk", "pclk";
> +		iommus = <&smmu 0>, <&smmu 1>, <&smmu 2>, <&smmu 3>,
> +			<&smmu 4>, <&smmu 5>, <&smmu 6>, <&smmu 7>,
> +			<&smmu 8>, <&smmu 9>;
>  
>  		dp0_pipe0: pipeline@0 {
>  			clocks = <&fpgaosc2>, <&dpu_aclk>;
> -- 
> 1.9.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v1 2/2] dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree

[Liviu Dudau]

On Tue, Apr 30, 2019 at 07:19:34AM +0100, Lowry Li (Arm Technology China) wrote:
> Updates the device-tree doc about how to enable SMMU by devicetree.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>

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

Best regards,
Liviu

> ---
>  Documentation/devicetree/bindings/display/arm,komeda.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/arm,komeda.txt b/Documentation/devicetree/bindings/display/arm,komeda.txt
> index 02b2265..b12c045 100644
> --- a/Documentation/devicetree/bindings/display/arm,komeda.txt
> +++ b/Documentation/devicetree/bindings/display/arm,komeda.txt
> @@ -11,6 +11,10 @@ Required properties:
>        - "pclk": for the APB interface clock
>  - #address-cells: Must be 1
>  - #size-cells: Must be 0
> +- iommus: configure the stream id to IOMMU, Must be configured if want to
> +    enable iommu in display. for how to configure this node please reference
> +        devicetree/bindings/iommu/arm,smmu-v3.txt,
> +        devicetree/bindings/iommu/iommu.txt
>  
>  Required properties for sub-node: pipeline@nq
>  Each device contains one or two pipeline sub-nodes (at least one), each
> @@ -44,6 +48,9 @@ Example:
>  		interrupts = <0 168 4>;
>  		clocks = <&dpu_mclk>, <&dpu_aclk>;
>  		clock-names = "mclk", "pclk";
> +		iommus = <&smmu 0>, <&smmu 1>, <&smmu 2>, <&smmu 3>,
> +			<&smmu 4>, <&smmu 5>, <&smmu 6>, <&smmu 7>,
> +			<&smmu 8>, <&smmu 9>;
>  
>  		dp0_pipe0: pipeline@0 {
>  			clocks = <&fpgaosc2>, <&dpu_aclk>;
> -- 
> 1.9.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v1 1/2] drm/komeda: Adds SMMU support

[Lowry Li (Arm Technology China)]

On Wed, Jun 05, 2019 at 07:19:37PM +0800, Liviu Dudau wrote:
> Hi Lowry,
> 
> On Tue, Apr 30, 2019 at 07:19:29AM +0100, Lowry Li (Arm Technology China) wrote:
> > Adds iommu_connect and disconnect for SMMU support, and configures
> > TBU translation once SMMU has been attached to the display device.
> > 
> > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > ---
> >  .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
> >  drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
> >  drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
> >  drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
> >  .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
> >  .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
> >  6 files changed, 82 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > index 33ca171..9065040 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > @@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
> >  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
> >  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
> >  
> > +	if (kfb->is_va)
> > +		ctrl |= L_TBU_EN;
> >  	malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
> >  }
> >  
> > @@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component *c,
> >  			       fb->pitches[i] & 0xFFFF);
> >  	}
> >  
> > +	if (kfb->is_va)
> > +		ctrl |= LW_TBU_EN;
> > +
> >  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
> >  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
> >  	malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > index 9603de9..45c98a7 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > @@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> >  	table->n_formats = ARRAY_SIZE(d71_format_caps_table);
> >  }
> >  
> > +static int d71_connect_iommu(struct komeda_dev *mdev)
> > +{
> > +	struct d71_dev *d71 = mdev->chip_data;
> > +	u32 __iomem *reg = d71->gcu_addr;
> > +	u32 check_bits = (d71->num_pipelines == 2) ?
> > +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> > +	int i, ret;
> > +
> > +	if (!d71->integrates_tbu)
> > +		return -1;
> > +
> > +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
> > +
> > +	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> > +			100, 1000, 1000);
> > +	if (ret <= 0) {
> > +		DRM_ERROR("connect to TCU timeout!\n");
> > +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> > +		return -ETIMEDOUT;
> > +	}
> > +
> > +	for (i = 0; i < d71->num_pipelines; i++)
> > +		malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
> > +				    LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
> > +	return 0;
> > +}
> > +
> > +static int d71_disconnect_iommu(struct komeda_dev *mdev)
> > +{
> > +	struct d71_dev *d71 = mdev->chip_data;
> > +	u32 __iomem *reg = d71->gcu_addr;
> > +	u32 check_bits = (d71->num_pipelines == 2) ?
> > +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> > +	int ret;
> > +
> > +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
> > +
> > +	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> > +			100, 1000, 1000);
> > +	if (ret <= 0) {
> > +		DRM_ERROR("disconnect from TCU timeout!\n");
> > +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> > +	}
> > +
> > +	return ret > 0 ? 0 : -1;
> 
> For consistency with d71_connect_iommu() you should return -ETIMEDOUT as well.
> I'm sending a patch to convert dp_wait_cond() to return that when it times out,
> so we can be consistent in the future.
> 
Hi Liviu,

OK, will change here to return -ETIMEDOUT. Thanks for this correction.
> > +}
> > +
> >  static struct komeda_dev_funcs d71_chip_funcs = {
> >  	.init_format_table = d71_init_fmt_tbl,
> >  	.enum_resources	= d71_enum_resources,
> > @@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> >  	.on_off_vblank	= d71_on_off_vblank,
> >  	.change_opmode	= d71_change_opmode,
> >  	.flush		= d71_flush,
> > +	.connect_iommu	= d71_connect_iommu,
> > +	.disconnect_iommu = d71_disconnect_iommu,
> >  };
> >  
> >  struct komeda_dev_funcs *
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > index e4e5b58..2d97c82 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > @@ -251,6 +251,18 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
> >  	dev->dma_parms = &mdev->dma_parms;
> >  	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
> >  
> > +	mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
> > +	if (!mdev->iommu)
> > +		DRM_INFO("continue without IOMMU support!\n");
> > +
> > +	if (mdev->iommu && mdev->funcs->connect_iommu) {
> > +		err = mdev->funcs->connect_iommu(mdev);
> > +		if (err) {
> > +			DRM_ERROR("connect iommu failed.\n");
> 
> connect_iommu() has already printed a DRM_ERROR(), do we need another one here?
> 
Will remove this printing code.
> > +			goto err_cleanup;
> 
> err_cleanup doesn't set the new mdev->iommu to NULL, forgot to add it?
Will set mdev->iommu to NULL in komeda_dev_destroy(). Thanks for
pointing this out.
> 
> 
> > +		}
> > +	}
> > +
> >  	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
> >  	if (err) {
> >  		DRM_ERROR("create sysfs group failed.\n");
> > @@ -280,6 +292,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
> >  	debugfs_remove_recursive(mdev->debugfs_root);
> >  #endif
> >  
> > +	if (mdev->iommu && mdev->funcs->disconnect_iommu)
> > +		if (mdev->funcs->disconnect_iommu(mdev))
> > +			DRM_ERROR("disconnect iommu failed.\n");
> 
> Same comment an in komeda_dev_create(), disconnect_iommu() already printed a DRM_ERROR()
> 
> > +	mdev->iommu = NULL;
> > +
> >  	for (i = 0; i < mdev->n_pipelines; i++) {
> >  		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
> >  		mdev->pipelines[i] = NULL;
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > index 83ace71..dac1eda 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > @@ -92,6 +92,10 @@ struct komeda_dev_funcs {
> >  	int (*enum_resources)(struct komeda_dev *mdev);
> >  	/** @cleanup: call to chip to cleanup komeda_dev->chip data */
> >  	void (*cleanup)(struct komeda_dev *mdev);
> > +	/** @connect_iommu: Optional, connect to external iommu */
> > +	int (*connect_iommu)(struct komeda_dev *mdev);
> > +	/** @disconnect_iommu: Optional, disconnect to external iommu */
> > +	int (*disconnect_iommu)(struct komeda_dev *mdev);
> >  	/**
> >  	 * @irq_handler:
> >  	 *
> > @@ -184,6 +188,9 @@ struct komeda_dev {
> >  	 */
> >  	void *chip_data;
> >  
> > +	/** @iommu: iommu domain */
> > +	struct iommu_domain *iommu;
> > +
> >  	/** @debugfs_root: root directory of komeda debugfs */
> >  	struct dentry *debugfs_root;
> >  };
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > index d5822a3..360ab70 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > @@ -201,6 +201,8 @@ struct drm_framebuffer *
> >  		goto err_cleanup;
> >  	}
> >  
> > +	kfb->is_va = mdev->iommu ? true : false;
> > +
> >  	return &kfb->base;
> >  
> >  err_cleanup:
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> > index 6cbb2f6..f4046e2 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> > @@ -21,6 +21,8 @@ struct komeda_fb {
> >  	 * extends drm_format_info for komeda specific information
> >  	 */
> >  	const struct komeda_format_caps *format_caps;
> > +	/** @is_va: if smmu is enabled, it will be true */
> > +	bool is_va;
> >  	/** @aligned_w: aligned frame buffer width */
> >  	u32 aligned_w;
> >  	/** @aligned_h: aligned frame buffer height */
> > -- 
> > 1.9.1
> > 
> 
> Best regards,
> Liviu
> 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

-- 
Regards,
Lowry
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/2] drm/komeda: Adds SMMU support

[Lowry Li (Arm Technology China)]

Hi Liviu,

Please ignore last email and please find the latest feedback inline as
below.

On Wed, Jun 05, 2019 at 07:19:37PM +0800, Liviu Dudau wrote:
> Hi Lowry,
> 
> On Tue, Apr 30, 2019 at 07:19:29AM +0100, Lowry Li (Arm Technology China) wrote:
> > Adds iommu_connect and disconnect for SMMU support, and configures
> > TBU translation once SMMU has been attached to the display device.
> > 
> > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > ---
> >  .../gpu/drm/arm/display/komeda/d71/d71_component.c |  5 +++
> >  drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c   | 49 ++++++++++++++++++++++
> >  drivers/gpu/drm/arm/display/komeda/komeda_dev.c    | 17 ++++++++
> >  drivers/gpu/drm/arm/display/komeda/komeda_dev.h    |  7 ++++
> >  .../drm/arm/display/komeda/komeda_framebuffer.c    |  2 +
> >  .../drm/arm/display/komeda/komeda_framebuffer.h    |  2 +
> >  6 files changed, 82 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > index 33ca171..9065040 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > @@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
> >  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
> >  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
> >  
> > +	if (kfb->is_va)
> > +		ctrl |= L_TBU_EN;
> >  	malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
> >  }
> >  
> > @@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component *c,
> >  			       fb->pitches[i] & 0xFFFF);
> >  	}
> >  
> > +	if (kfb->is_va)
> > +		ctrl |= LW_TBU_EN;
> > +
> >  	malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
> >  	malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
> >  	malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > index 9603de9..45c98a7 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > @@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> >  	table->n_formats = ARRAY_SIZE(d71_format_caps_table);
> >  }
> >  
> > +static int d71_connect_iommu(struct komeda_dev *mdev)
> > +{
> > +	struct d71_dev *d71 = mdev->chip_data;
> > +	u32 __iomem *reg = d71->gcu_addr;
> > +	u32 check_bits = (d71->num_pipelines == 2) ?
> > +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> > +	int i, ret;
> > +
> > +	if (!d71->integrates_tbu)
> > +		return -1;
> > +
> > +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
> > +
> > +	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> > +			100, 1000, 1000);
> > +	if (ret <= 0) {
> > +		DRM_ERROR("connect to TCU timeout!\n");
> > +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> > +		return -ETIMEDOUT;
> > +	}
> > +
> > +	for (i = 0; i < d71->num_pipelines; i++)
> > +		malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
> > +				    LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
> > +	return 0;
> > +}
> > +
> > +static int d71_disconnect_iommu(struct komeda_dev *mdev)
> > +{
> > +	struct d71_dev *d71 = mdev->chip_data;
> > +	u32 __iomem *reg = d71->gcu_addr;
> > +	u32 check_bits = (d71->num_pipelines == 2) ?
> > +			 GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> > +	int ret;
> > +
> > +	malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
> > +
> > +	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> > +			100, 1000, 1000);
> > +	if (ret <= 0) {
> > +		DRM_ERROR("disconnect from TCU timeout!\n");
> > +		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> > +	}
> > +
> > +	return ret > 0 ? 0 : -1;
> 
> For consistency with d71_connect_iommu() you should return -ETIMEDOUT as well.
> I'm sending a patch to convert dp_wait_cond() to return that when it times out,
> so we can be consistent in the future.
> 
Yes, will change to use -ETIMEDOUT.

> > +}
> > +
> >  static struct komeda_dev_funcs d71_chip_funcs = {
> >  	.init_format_table = d71_init_fmt_tbl,
> >  	.enum_resources	= d71_enum_resources,
> > @@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> >  	.on_off_vblank	= d71_on_off_vblank,
> >  	.change_opmode	= d71_change_opmode,
> >  	.flush		= d71_flush,
> > +	.connect_iommu	= d71_connect_iommu,
> > +	.disconnect_iommu = d71_disconnect_iommu,
> >  };
> >  
> >  struct komeda_dev_funcs *
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > index e4e5b58..2d97c82 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > @@ -251,6 +251,18 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
> >  	dev->dma_parms = &mdev->dma_parms;
> >  	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
> >  
> > +	mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
> > +	if (!mdev->iommu)
> > +		DRM_INFO("continue without IOMMU support!\n");
> > +
> > +	if (mdev->iommu && mdev->funcs->connect_iommu) {
> > +		err = mdev->funcs->connect_iommu(mdev);
> > +		if (err) {
> > +			DRM_ERROR("connect iommu failed.\n");
> 
> connect_iommu() has already printed a DRM_ERROR(), do we need another one here?
> 
This log printing is necessary at core layer for the case that some
other chip layer code doesn't have error messages. So suggest keeping
this code here.
> > +			goto err_cleanup;
> 
> err_cleanup doesn't set the new mdev->iommu to NULL, forgot to add it?
> 
> 
Sorry, I forgot adding this. Will set to NULL here once connected
failed. Thanks again.
> > +		}
> > +	}
> > +
> >  	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
> >  	if (err) {
> >  		DRM_ERROR("create sysfs group failed.\n");
> > @@ -280,6 +292,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
> >  	debugfs_remove_recursive(mdev->debugfs_root);
> >  #endif
> >  
> > +	if (mdev->iommu && mdev->funcs->disconnect_iommu)
> > +		if (mdev->funcs->disconnect_iommu(mdev))
> > +			DRM_ERROR("disconnect iommu failed.\n");
> 
> Same comment an in komeda_dev_create(), disconnect_iommu() already printed a DRM_ERROR()
> 
> > +	mdev->iommu = NULL;
> > +
> >  	for (i = 0; i < mdev->n_pipelines; i++) {
> >  		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
> >  		mdev->pipelines[i] = NULL;
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > index 83ace71..dac1eda 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > @@ -92,6 +92,10 @@ struct komeda_dev_funcs {
> >  	int (*enum_resources)(struct komeda_dev *mdev);
> >  	/** @cleanup: call to chip to cleanup komeda_dev->chip data */
> >  	void (*cleanup)(struct komeda_dev *mdev);
> > +	/** @connect_iommu: Optional, connect to external iommu */
> > +	int (*connect_iommu)(struct komeda_dev *mdev);
> > +	/** @disconnect_iommu: Optional, disconnect to external iommu */
> > +	int (*disconnect_iommu)(struct komeda_dev *mdev);
> >  	/**
> >  	 * @irq_handler:
> >  	 *
> > @@ -184,6 +188,9 @@ struct komeda_dev {
> >  	 */
> >  	void *chip_data;
> >  
> > +	/** @iommu: iommu domain */
> > +	struct iommu_domain *iommu;
> > +
> >  	/** @debugfs_root: root directory of komeda debugfs */
> >  	struct dentry *debugfs_root;
> >  };
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > index d5822a3..360ab70 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > @@ -201,6 +201,8 @@ struct drm_framebuffer *
> >  		goto err_cleanup;
> >  	}
> >  
> > +	kfb->is_va = mdev->iommu ? true : false;
> > +
> >  	return &kfb->base;
> >  
> >  err_cleanup:
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> > index 6cbb2f6..f4046e2 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
> > @@ -21,6 +21,8 @@ struct komeda_fb {
> >  	 * extends drm_format_info for komeda specific information
> >  	 */
> >  	const struct komeda_format_caps *format_caps;
> > +	/** @is_va: if smmu is enabled, it will be true */
> > +	bool is_va;
> >  	/** @aligned_w: aligned frame buffer width */
> >  	u32 aligned_w;
> >  	/** @aligned_h: aligned frame buffer height */
> > -- 
> > 1.9.1
> > 
> 
> Best regards,
> Liviu
> 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

-- 
Regards,
Lowry
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel