From 73a395c46704304b96bc5e2ee19be31124025c0c Mon Sep 17 00:00:00 2001 From: "Pavel Machek (CIP)" Date: Tue, 6 Apr 2021 13:54:14 +0200 Subject: [PATCH 1/7] drm/tegra: sor: Do not leak runtime PM reference It's theoretically possible for the runtime PM reference to leak if the code fails anywhere between the pm_runtime_resume_and_get() and pm_runtime_put() calls, so make sure to release the runtime PM reference in that case. Practically this will never happen because none of the functions will fail on Tegra, but it's better for the code to be pedantic in case these assumptions will ever become wrong. Signed-off-by: Pavel Machek (CIP) [treding@nvidia.com: add commit message] Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/sor.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index 7b88261f57bb..67a80dae1c00 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -3125,21 +3125,21 @@ static int tegra_sor_init(struct host1x_client *client) if (err < 0) { dev_err(sor->dev, "failed to acquire SOR reset: %d\n", err); - return err; + goto rpm_put; } err = reset_control_assert(sor->rst); if (err < 0) { dev_err(sor->dev, "failed to assert SOR reset: %d\n", err); - return err; + goto rpm_put; } } err = clk_prepare_enable(sor->clk); if (err < 0) { dev_err(sor->dev, "failed to enable clock: %d\n", err); - return err; + goto rpm_put; } usleep_range(1000, 3000); @@ -3150,7 +3150,7 @@ static int tegra_sor_init(struct host1x_client *client) dev_err(sor->dev, "failed to deassert SOR reset: %d\n", err); clk_disable_unprepare(sor->clk); - return err; + goto rpm_put; } reset_control_release(sor->rst); @@ -3171,6 +3171,12 @@ static int tegra_sor_init(struct host1x_client *client) } return 0; + +rpm_put: + if (sor->rst) + pm_runtime_put(sor->dev); + + return err; } static int tegra_sor_exit(struct host1x_client *client) From 0cfe5a6e758fb20be8ad3e8f10cb087cc8033eeb Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 1 Apr 2021 17:41:04 +0200 Subject: [PATCH 2/7] gpu: host1x: Split up client initalization and registration In some cases we may need to initialize the host1x client first before registering it. This commit adds a new helper that will do nothing but the initialization of the data structure. At the same time, the initialization is removed from the registration function. Note, however, that for simplicity we explicitly initialize the client when the host1x_client_register() function is called, as opposed to the low-level __host1x_client_register() function. This allows existing callers to remain unchanged. Signed-off-by: Thierry Reding --- drivers/gpu/host1x/bus.c | 30 ++++++++++++++++++++++++------ include/linux/host1x.h | 30 ++++++++++++++++++++++++------ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c index 46f69c532b6b..218e3718fd68 100644 --- a/drivers/gpu/host1x/bus.c +++ b/drivers/gpu/host1x/bus.c @@ -735,6 +735,29 @@ void host1x_driver_unregister(struct host1x_driver *driver) } EXPORT_SYMBOL(host1x_driver_unregister); +/** + * __host1x_client_init() - initialize a host1x client + * @client: host1x client + * @key: lock class key for the client-specific mutex + */ +void __host1x_client_init(struct host1x_client *client, struct lock_class_key *key) +{ + INIT_LIST_HEAD(&client->list); + __mutex_init(&client->lock, "host1x client lock", key); + client->usecount = 0; +} +EXPORT_SYMBOL(__host1x_client_init); + +/** + * host1x_client_exit() - uninitialize a host1x client + * @client: host1x client + */ +void host1x_client_exit(struct host1x_client *client) +{ + mutex_destroy(&client->lock); +} +EXPORT_SYMBOL(host1x_client_exit); + /** * __host1x_client_register() - register a host1x client * @client: host1x client @@ -747,16 +770,11 @@ EXPORT_SYMBOL(host1x_driver_unregister); * device and call host1x_device_init(), which will in turn call each client's * &host1x_client_ops.init implementation. */ -int __host1x_client_register(struct host1x_client *client, - struct lock_class_key *key) +int __host1x_client_register(struct host1x_client *client) { struct host1x *host1x; int err; - INIT_LIST_HEAD(&client->list); - __mutex_init(&client->lock, "host1x client lock", key); - client->usecount = 0; - mutex_lock(&devices_lock); list_for_each_entry(host1x, &devices, list) { diff --git a/include/linux/host1x.h b/include/linux/host1x.h index 232e1bd507a7..9b0487c88571 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -332,12 +332,30 @@ static inline struct host1x_device *to_host1x_device(struct device *dev) int host1x_device_init(struct host1x_device *device); int host1x_device_exit(struct host1x_device *device); -int __host1x_client_register(struct host1x_client *client, - struct lock_class_key *key); -#define host1x_client_register(class) \ - ({ \ - static struct lock_class_key __key; \ - __host1x_client_register(class, &__key); \ +void __host1x_client_init(struct host1x_client *client, struct lock_class_key *key); +void host1x_client_exit(struct host1x_client *client); + +#define host1x_client_init(client) \ + ({ \ + static struct lock_class_key __key; \ + __host1x_client_init(client, &__key); \ + }) + +int __host1x_client_register(struct host1x_client *client); + +/* + * Note that this wrapper calls __host1x_client_init() for compatibility + * with existing callers. Callers that want to separately initialize and + * register a host1x client must first initialize using either of the + * __host1x_client_init() or host1x_client_init() functions and then use + * the low-level __host1x_client_register() function to avoid the client + * getting reinitialized. + */ +#define host1x_client_register(client) \ + ({ \ + static struct lock_class_key __key; \ + __host1x_client_init(client, &__key); \ + __host1x_client_register(client); \ }) int host1x_client_unregister(struct host1x_client *client); From 5dea42759bcef74b0802ea64b904409bc37f9045 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 1 Apr 2021 17:41:05 +0200 Subject: [PATCH 3/7] drm/tegra: sor: Fully initialize SOR before registration Before registering the SOR host1x client, make sure that it is fully initialized. This avoids a potential race condition between the SOR's probe and the host1x device initialization in cases where the SOR is the final sub-device to register to a host1x instance. Reported-by: Jonathan Hunter Signed-off-by: Thierry Reding Tested-by: Jon Hunter Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/sor.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index 67a80dae1c00..32c83f2e386c 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -3922,17 +3922,10 @@ static int tegra_sor_probe(struct platform_device *pdev) platform_set_drvdata(pdev, sor); pm_runtime_enable(&pdev->dev); - INIT_LIST_HEAD(&sor->client.list); + host1x_client_init(&sor->client); sor->client.ops = &sor_client_ops; sor->client.dev = &pdev->dev; - err = host1x_client_register(&sor->client); - if (err < 0) { - dev_err(&pdev->dev, "failed to register host1x client: %d\n", - err); - goto rpm_disable; - } - /* * On Tegra210 and earlier, provide our own implementation for the * pad output clock. @@ -3944,13 +3937,13 @@ static int tegra_sor_probe(struct platform_device *pdev) sor->index); if (!name) { err = -ENOMEM; - goto unregister; + goto uninit; } err = host1x_client_resume(&sor->client); if (err < 0) { dev_err(sor->dev, "failed to resume: %d\n", err); - goto unregister; + goto uninit; } sor->clk_pad = tegra_clk_sor_pad_register(sor, name); @@ -3961,14 +3954,20 @@ static int tegra_sor_probe(struct platform_device *pdev) err = PTR_ERR(sor->clk_pad); dev_err(sor->dev, "failed to register SOR pad clock: %d\n", err); - goto unregister; + goto uninit; + } + + err = __host1x_client_register(&sor->client); + if (err < 0) { + dev_err(&pdev->dev, "failed to register host1x client: %d\n", + err); + goto uninit; } return 0; -unregister: - host1x_client_unregister(&sor->client); -rpm_disable: +uninit: + host1x_client_exit(&sor->client); pm_runtime_disable(&pdev->dev); remove: tegra_output_remove(&sor->output); From dc9a91d279b721aef7c4f1a2e2e33631d388446f Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Thu, 15 Apr 2021 08:29:14 -0700 Subject: [PATCH 4/7] drm/tegra: Fix shift overflow in tegra_shared_plane_atomic_update Clang warns: drivers/gpu/drm/tegra/hub.c:513:11: warning: shift count >= width of type [-Wshift-count-overflow] base |= BIT(39); ^~~~~~~ BIT is unsigned long, which is 32-bit on ARCH=arm, hence the overflow warning. Switch to BIT_ULL, which is 64-bit and will not overflow. Fixes: 7b6f846785f4 ("drm/tegra: Support sector layout on Tegra194") Link: https://github.com/ClangBuiltLinux/linux/issues/1351 Signed-off-by: Nathan Chancellor Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/hub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c index 79bff8b48271..bfae8a02f55b 100644 --- a/drivers/gpu/drm/tegra/hub.c +++ b/drivers/gpu/drm/tegra/hub.c @@ -510,7 +510,7 @@ static void tegra_shared_plane_atomic_update(struct drm_plane *plane, * dGPU sector layout. */ if (tegra_plane_state->tiling.sector_layout == TEGRA_BO_SECTOR_LAYOUT_GPU) - base |= BIT(39); + base |= BIT_ULL(39); #endif tegra_plane_writel(p, tegra_plane_state->format, DC_WIN_COLOR_DEPTH); From 1d15a10395e5a036f571ac727f202f9572e255f9 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 14 May 2021 18:13:05 -0400 Subject: [PATCH 5/7] drm/tegra: Get ref for DP AUX channel, not its ddc adapter While we're taking a reference of the DDC adapter for a DP AUX channel in tegra_sor_probe() because we're going to be using that adapter with the SOR, now that we've moved where AUX registration happens the actual device structure for the DDC adapter isn't initialized yet. Which means that we can't really take a reference from it to try to keep it around anymore. This should be fine though, because we can just take a reference of its parent instead. v2: * Avoid calling i2c_put_adapter() in tegra_output_remove() for eDP/DP cases Signed-off-by: Lyude Paul Fixes: 39c17ae60ea9 ("drm/tegra: Don't register DP AUX channels before connectors") Cc: Lyude Paul Cc: Thierry Reding Cc: Jonathan Hunter Cc: dri-devel@lists.freedesktop.org Cc: linux-tegra@vger.kernel.org Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/output.c | 5 ++++- drivers/gpu/drm/tegra/sor.c | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c index 47d26b5d9945..2dacce1ab6ee 100644 --- a/drivers/gpu/drm/tegra/output.c +++ b/drivers/gpu/drm/tegra/output.c @@ -180,10 +180,13 @@ int tegra_output_probe(struct tegra_output *output) void tegra_output_remove(struct tegra_output *output) { + int connector_type = output->connector.connector_type; + if (output->hpd_gpio) free_irq(output->hpd_irq, output); - if (output->ddc) + if (connector_type != DRM_MODE_CONNECTOR_eDP && + connector_type != DRM_MODE_CONNECTOR_DisplayPort && output->ddc) i2c_put_adapter(output->ddc); } diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index 32c83f2e386c..8f99de08b2be 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -3745,11 +3745,11 @@ static int tegra_sor_probe(struct platform_device *pdev) if (!sor->aux) return -EPROBE_DEFER; - if (get_device(&sor->aux->ddc.dev)) { - if (try_module_get(sor->aux->ddc.owner)) + if (get_device(sor->aux->dev)) { + if (try_module_get(sor->aux->dev->driver->owner)) sor->output.ddc = &sor->aux->ddc; else - put_device(&sor->aux->ddc.dev); + put_device(sor->aux->dev); } } From b79b6081c440c0c197a3e8a51e8b9cf343fb210f Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 27 May 2021 20:09:08 +0200 Subject: [PATCH 6/7] drm/tegra: sor: Fix AUX device reference leak In the case where the AUX provides an I2C-over-AUX DDC channel, a reference is taken on the AUX parent device of the DDC channel rather than the DDC channel like it would be for regular I2C controllers. To make sure the correct reference is dropped, move the unreferencing code into the SOR driver and make sure not to drop the I2C adapter reference in that case. Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/output.c | 5 +---- drivers/gpu/drm/tegra/sor.c | 29 +++++++++++++++++++---------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c index 2dacce1ab6ee..47d26b5d9945 100644 --- a/drivers/gpu/drm/tegra/output.c +++ b/drivers/gpu/drm/tegra/output.c @@ -180,13 +180,10 @@ int tegra_output_probe(struct tegra_output *output) void tegra_output_remove(struct tegra_output *output) { - int connector_type = output->connector.connector_type; - if (output->hpd_gpio) free_irq(output->hpd_irq, output); - if (connector_type != DRM_MODE_CONNECTOR_eDP && - connector_type != DRM_MODE_CONNECTOR_DisplayPort && output->ddc) + if (output->ddc) i2c_put_adapter(output->ddc); } diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index 8f99de08b2be..0ea320c1092b 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -3745,12 +3745,8 @@ static int tegra_sor_probe(struct platform_device *pdev) if (!sor->aux) return -EPROBE_DEFER; - if (get_device(sor->aux->dev)) { - if (try_module_get(sor->aux->dev->driver->owner)) - sor->output.ddc = &sor->aux->ddc; - else - put_device(sor->aux->dev); - } + if (get_device(sor->aux->dev)) + sor->output.ddc = &sor->aux->ddc; } if (!sor->aux) { @@ -3778,12 +3774,13 @@ static int tegra_sor_probe(struct platform_device *pdev) err = tegra_sor_parse_dt(sor); if (err < 0) - return err; + goto put_aux; err = tegra_output_probe(&sor->output); - if (err < 0) - return dev_err_probe(&pdev->dev, err, - "failed to probe output\n"); + if (err < 0) { + dev_err_probe(&pdev->dev, err, "failed to probe output\n"); + goto put_aux; + } if (sor->ops && sor->ops->probe) { err = sor->ops->probe(sor); @@ -3970,7 +3967,14 @@ uninit: host1x_client_exit(&sor->client); pm_runtime_disable(&pdev->dev); remove: + if (sor->aux) + sor->output.ddc = NULL; + tegra_output_remove(&sor->output); +put_aux: + if (sor->aux) + put_device(sor->aux->dev); + return err; } @@ -3988,6 +3992,11 @@ static int tegra_sor_remove(struct platform_device *pdev) pm_runtime_disable(&pdev->dev); + if (sor->aux) { + put_device(sor->aux->dev); + sor->output.ddc = NULL; + } + tegra_output_remove(&sor->output); return 0; From 671cc352acd3e2b2832b59787ed8027d9f80ccc9 Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Sun, 30 May 2021 22:55:06 +0300 Subject: [PATCH 7/7] drm/tegra: Correct DRM_FORMAT_MOD_NVIDIA_SECTOR_LAYOUT The format modifier is 64bit, while DRM_FORMAT_MOD_NVIDIA_SECTOR_LAYOUT uses BIT() macro that is 32bit on ARM32. The (modifier &= ~DRM_FORMAT_MOD_NVIDIA_SECTOR_LAYOUT) doesn't work as expected on ARM32 and tegra_fb_get_tiling() fails for the tiled formats on 32bit Tegra because modifier mask isn't applied properly. Use the BIT_ULL() macro to fix this trouble. Fixes: 7b6f846785f4 ("drm/tegra: Support sector layout on Tegra194") Signed-off-by: Dmitry Osipenko Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/drm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index 87df251c1fcf..0cb868065348 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -25,7 +25,7 @@ #include "trace.h" /* XXX move to include/uapi/drm/drm_fourcc.h? */ -#define DRM_FORMAT_MOD_NVIDIA_SECTOR_LAYOUT BIT(22) +#define DRM_FORMAT_MOD_NVIDIA_SECTOR_LAYOUT BIT_ULL(22) struct reset_control;