From: Thierry Reding <thierry.reding@gmail.com>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v1 1/5] drm/tegra: dc: Balance IOMMU group refcounting
Date: Fri, 4 May 2018 12:45:15 +0200 [thread overview]
Message-ID: <20180504104515.GM13459@ulmo> (raw)
In-Reply-To: <20180503234723.4368-2-digetx@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 4435 bytes --]
On Fri, May 04, 2018 at 02:47:19AM +0300, Dmitry Osipenko wrote:
> Remove unneeded iommu_group_get() and add missing iommu_group_put(),
> correcting IOMMU group refcount. This is a minor correction / cleanup that
> doesn't really fix anything because Tegra's IOMMU driver are built-in and
> hence groups refcounting can't hold IOMMU driver from unloading.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> drivers/gpu/drm/tegra/dc.c | 35 +++++++++++++++++------------------
> drivers/gpu/drm/tegra/dc.h | 2 +-
> drivers/gpu/drm/tegra/drm.h | 2 +-
> 3 files changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index d7a0f62c6e2e..9fc34c6a7218 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1927,7 +1927,6 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
> static int tegra_dc_init(struct host1x_client *client)
> {
> struct drm_device *drm = dev_get_drvdata(client->parent);
> - struct iommu_group *group = iommu_group_get(client->dev);
> unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED;
> struct tegra_dc *dc = host1x_client_to_dc(client);
> struct tegra_drm *tegra = drm->dev_private;
> @@ -1939,20 +1938,21 @@ static int tegra_dc_init(struct host1x_client *client)
> if (!dc->syncpt)
> dev_warn(dc->dev, "failed to allocate syncpoint\n");
>
> - if (group && tegra->domain) {
> - if (group != tegra->group) {
> - err = iommu_attach_group(tegra->domain, group);
> + if (tegra->domain) {
> + dc->group = iommu_group_get(client->dev);
> +
> + if (dc->group && dc->group != tegra->dc_group) {
> + err = iommu_attach_group(tegra->domain, dc->group);
> if (err < 0) {
> dev_err(dc->dev,
> "failed to attach to domain: %d\n",
> err);
> + iommu_group_put(dc->group);
> return err;
> }
>
> - tegra->group = group;
> + tegra->dc_group = dc->group;
> }
> -
> - dc->domain = tegra->domain;
> }
>
> if (dc->soc->wgrps)
> @@ -2017,13 +2017,13 @@ static int tegra_dc_init(struct host1x_client *client)
> if (!IS_ERR(primary))
> drm_plane_cleanup(primary);
>
> - if (group && dc->domain) {
> - if (group == tegra->group) {
> - iommu_detach_group(dc->domain, group);
> - tegra->group = NULL;
> + if (dc->group) {
> + if (dc->group && dc->group == tegra->dc_group) {
You can drop the dc->group check from the second conditional.
> + iommu_detach_group(tegra->domain, dc->group);
> + tegra->dc_group = NULL;
> }
>
> - dc->domain = NULL;
> + iommu_group_put(dc->group);
> }
>
> return err;
> @@ -2032,7 +2032,6 @@ static int tegra_dc_init(struct host1x_client *client)
> static int tegra_dc_exit(struct host1x_client *client)
> {
> struct drm_device *drm = dev_get_drvdata(client->parent);
> - struct iommu_group *group = iommu_group_get(client->dev);
> struct tegra_dc *dc = host1x_client_to_dc(client);
> struct tegra_drm *tegra = drm->dev_private;
> int err;
> @@ -2045,13 +2044,13 @@ static int tegra_dc_exit(struct host1x_client *client)
> return err;
> }
>
> - if (group && dc->domain) {
> - if (group == tegra->group) {
> - iommu_detach_group(dc->domain, group);
> - tegra->group = NULL;
> + if (dc->group) {
> + if (dc->group && dc->group == tegra->dc_group) {
> + iommu_detach_group(tegra->domain, dc->group);
> + tegra->dc_group = NULL;
> }
>
> - dc->domain = NULL;
> + iommu_group_put(dc->group);
> }
>
> host1x_syncpt_free(dc->syncpt);
> diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
> index ca5cac6bf8ea..5ca4e07333bb 100644
> --- a/drivers/gpu/drm/tegra/dc.h
> +++ b/drivers/gpu/drm/tegra/dc.h
> @@ -94,7 +94,7 @@ struct tegra_dc {
>
> const struct tegra_dc_soc_info *soc;
>
> - struct iommu_domain *domain;
> + struct iommu_group *group;
> };
>
> static inline struct tegra_dc *
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 4f41aaec8530..7777640a6911 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -46,7 +46,7 @@ struct tegra_drm {
> struct drm_device *drm;
>
> struct iommu_domain *domain;
> - struct iommu_group *group;
> + struct iommu_group *dc_group;
It's not obvious to me why this needs to be renamed.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: dri-devel@lists.freedesktop.org, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/5] drm/tegra: dc: Balance IOMMU group refcounting
Date: Fri, 4 May 2018 12:45:15 +0200 [thread overview]
Message-ID: <20180504104515.GM13459@ulmo> (raw)
In-Reply-To: <20180503234723.4368-2-digetx@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4435 bytes --]
On Fri, May 04, 2018 at 02:47:19AM +0300, Dmitry Osipenko wrote:
> Remove unneeded iommu_group_get() and add missing iommu_group_put(),
> correcting IOMMU group refcount. This is a minor correction / cleanup that
> doesn't really fix anything because Tegra's IOMMU driver are built-in and
> hence groups refcounting can't hold IOMMU driver from unloading.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> drivers/gpu/drm/tegra/dc.c | 35 +++++++++++++++++------------------
> drivers/gpu/drm/tegra/dc.h | 2 +-
> drivers/gpu/drm/tegra/drm.h | 2 +-
> 3 files changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index d7a0f62c6e2e..9fc34c6a7218 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1927,7 +1927,6 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
> static int tegra_dc_init(struct host1x_client *client)
> {
> struct drm_device *drm = dev_get_drvdata(client->parent);
> - struct iommu_group *group = iommu_group_get(client->dev);
> unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED;
> struct tegra_dc *dc = host1x_client_to_dc(client);
> struct tegra_drm *tegra = drm->dev_private;
> @@ -1939,20 +1938,21 @@ static int tegra_dc_init(struct host1x_client *client)
> if (!dc->syncpt)
> dev_warn(dc->dev, "failed to allocate syncpoint\n");
>
> - if (group && tegra->domain) {
> - if (group != tegra->group) {
> - err = iommu_attach_group(tegra->domain, group);
> + if (tegra->domain) {
> + dc->group = iommu_group_get(client->dev);
> +
> + if (dc->group && dc->group != tegra->dc_group) {
> + err = iommu_attach_group(tegra->domain, dc->group);
> if (err < 0) {
> dev_err(dc->dev,
> "failed to attach to domain: %d\n",
> err);
> + iommu_group_put(dc->group);
> return err;
> }
>
> - tegra->group = group;
> + tegra->dc_group = dc->group;
> }
> -
> - dc->domain = tegra->domain;
> }
>
> if (dc->soc->wgrps)
> @@ -2017,13 +2017,13 @@ static int tegra_dc_init(struct host1x_client *client)
> if (!IS_ERR(primary))
> drm_plane_cleanup(primary);
>
> - if (group && dc->domain) {
> - if (group == tegra->group) {
> - iommu_detach_group(dc->domain, group);
> - tegra->group = NULL;
> + if (dc->group) {
> + if (dc->group && dc->group == tegra->dc_group) {
You can drop the dc->group check from the second conditional.
> + iommu_detach_group(tegra->domain, dc->group);
> + tegra->dc_group = NULL;
> }
>
> - dc->domain = NULL;
> + iommu_group_put(dc->group);
> }
>
> return err;
> @@ -2032,7 +2032,6 @@ static int tegra_dc_init(struct host1x_client *client)
> static int tegra_dc_exit(struct host1x_client *client)
> {
> struct drm_device *drm = dev_get_drvdata(client->parent);
> - struct iommu_group *group = iommu_group_get(client->dev);
> struct tegra_dc *dc = host1x_client_to_dc(client);
> struct tegra_drm *tegra = drm->dev_private;
> int err;
> @@ -2045,13 +2044,13 @@ static int tegra_dc_exit(struct host1x_client *client)
> return err;
> }
>
> - if (group && dc->domain) {
> - if (group == tegra->group) {
> - iommu_detach_group(dc->domain, group);
> - tegra->group = NULL;
> + if (dc->group) {
> + if (dc->group && dc->group == tegra->dc_group) {
> + iommu_detach_group(tegra->domain, dc->group);
> + tegra->dc_group = NULL;
> }
>
> - dc->domain = NULL;
> + iommu_group_put(dc->group);
> }
>
> host1x_syncpt_free(dc->syncpt);
> diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
> index ca5cac6bf8ea..5ca4e07333bb 100644
> --- a/drivers/gpu/drm/tegra/dc.h
> +++ b/drivers/gpu/drm/tegra/dc.h
> @@ -94,7 +94,7 @@ struct tegra_dc {
>
> const struct tegra_dc_soc_info *soc;
>
> - struct iommu_domain *domain;
> + struct iommu_group *group;
> };
>
> static inline struct tegra_dc *
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 4f41aaec8530..7777640a6911 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -46,7 +46,7 @@ struct tegra_drm {
> struct drm_device *drm;
>
> struct iommu_domain *domain;
> - struct iommu_group *group;
> + struct iommu_group *dc_group;
It's not obvious to me why this needs to be renamed.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2018-05-04 10:45 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-03 23:47 [PATCH v1 0/5] Support IOMMU for 2d/3d on Tegra30/114 Dmitry Osipenko
2018-05-03 23:47 ` [PATCH v1 1/5] drm/tegra: dc: Balance IOMMU group refcounting Dmitry Osipenko
2018-05-04 10:45 ` Thierry Reding [this message]
2018-05-04 10:45 ` Thierry Reding
2018-05-04 11:28 ` Dmitry Osipenko
2018-05-04 12:19 ` Thierry Reding
2018-05-04 12:19 ` Thierry Reding
2018-05-03 23:47 ` [PATCH v1 2/5] drm/tegra: gr2d: Add IOMMU support Dmitry Osipenko
2018-05-04 10:54 ` Thierry Reding
2018-05-04 10:54 ` Thierry Reding
2018-05-04 11:31 ` Dmitry Osipenko
2018-05-04 12:20 ` Thierry Reding
2018-05-04 12:20 ` Thierry Reding
2018-05-03 23:47 ` [PATCH v1 3/5] drm/tegra: gr3d: " Dmitry Osipenko
2018-05-04 12:20 ` Thierry Reding
2018-05-04 12:20 ` Thierry Reding
2018-05-03 23:47 ` [PATCH v1 4/5] ARM: dts: tegra30: Add IOMMU nodes to Host1x and its clients Dmitry Osipenko
2018-05-04 15:22 ` Thierry Reding
2018-05-04 15:22 ` Thierry Reding
2018-05-03 23:47 ` [PATCH v1 5/5] ARM: dts: tegra114: " Dmitry Osipenko
2018-05-04 15:22 ` Thierry Reding
2018-05-04 10:56 ` [PATCH v1 0/5] Support IOMMU for 2d/3d on Tegra30/114 Thierry Reding
2018-05-04 10:56 ` Thierry Reding
2018-05-04 12:03 ` Dmitry Osipenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180504104515.GM13459@ulmo \
--to=thierry.reding@gmail.com \
--cc=digetx@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.