Hi Liviu,
On Fri, Jun 07, 2019 at 05:05:59PM +0800, Liviu Dudau wrote:
> Hi Lowry,
>
> On Thu, Jun 06, 2019 at 10:53:05AM +0100, Lowry Li (Arm Technology China) wrote:
> > From: "Lowry Li (Arm Technology China)" <Lowry.Li@xxxxxxx>
> >
> > 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@xxxxxxx>
> > ---
> > .../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 | 18 ++++++++
> > 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, 83 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 4e26a80..4a48dd6 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 8e682c7..1b9e734 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) {
>
> You don't want to return -ETIMEDOUT if dp_wait_cond() returns zero. Maybe
> follow the same flow as in d71_disconnect_iommu() ?
>
Sorry, it should be changed as in d71_disconnect_iommu(). Thanks a lot
for pointing this out.
> > + 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;
> > +}
> > +
> > 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 c92e161..e80e673 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > @@ -253,6 +253,19 @@ 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) {
> > + mdev->iommu = NULL;
> > + DRM_ERROR("connect iommu failed.\n");
>
> I thought you're going to remove these extra DRM_ERRORs.
>
> Best regards,
> Liviu
This log printing is necessary at our 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 = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
> > if (err) {
> > DRM_ERROR("create sysfs group failed.\n");
> > @@ -282,6 +295,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
> >
>
> --
> ====================
> | I would like to |
> | fix the world, |
> | but they're not |
> | giving me the |
> \ source code! /
> ---------------
> ¯\_(ツ)_/¯
--
Regards,
Lowry
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
