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

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


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

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

Signed-off-by: Lowry Li (Arm Technology China) <[email protected]>
---
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

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

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) <[email protected]>
---
.../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

2019-04-30 07:36:56

by James Qian Wang

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

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) <[email protected]>
> ---
> .../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) <[email protected]>

2019-04-30 07:38:48

by James Qian Wang

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

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) <[email protected]>
> ---
> 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) <[email protected]>

2019-06-05 11:22:07

by Liviu Dudau

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

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) <[email protected]>

Reviewed-by: Liviu Dudau <[email protected]>

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! /
---------------
¯\_(ツ)_/¯

2019-06-05 11:22:56

by Liviu Dudau

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

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) <[email protected]>
> ---
> .../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! /
---------------
¯\_(ツ)_/¯