From patchwork Mon Mar 20 18:44:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mikko Perttunen X-Patchwork-Id: 741141 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vn4dR1C9wz9s3w for ; Tue, 21 Mar 2017 05:45:31 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755954AbdCTSpa (ORCPT ); Mon, 20 Mar 2017 14:45:30 -0400 Received: from mail.kapsi.fi ([217.30.184.167]:41780 "EHLO mail.kapsi.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755886AbdCTSp0 (ORCPT ); Mon, 20 Mar 2017 14:45:26 -0400 Received: from dsl-espbrasgw1-54f9c1-183.dhcp.inet.fi ([84.249.193.183] helo=toshino.dhcp.inet.fi) by mail.kapsi.fi with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1cq2J2-0002QD-Ac; Mon, 20 Mar 2017 20:45:24 +0200 From: Mikko Perttunen To: thierry.reding@gmail.com Cc: linux-tegra@vger.kernel.org, dri-devel@lists.freedesktop.org, Mikko Perttunen Subject: [PATCH] gpu: host1x: Refactor/fix channel allocation code Date: Mon, 20 Mar 2017 20:44:02 +0200 Message-Id: <20170320184402.32723-1-mperttunen@nvidia.com> X-Mailer: git-send-email 2.11.1 X-SA-Exim-Connect-IP: 84.249.193.183 X-SA-Exim-Mail-From: mperttunen@nvidia.com X-SA-Exim-Scanned: No (on mail.kapsi.fi); SAEximRunCond expanded to false Sender: linux-tegra-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org This is largely a rewrite of the Host1x channel allocation code in channel.c, bringing several changes: - The previous code could deadlock due to an interaction between the 'reflock' mutex and CDMA timeout handling. This gets rid of the mutex. - In the future, per-user channel allocation will be useful to have. This paves the way for that by allowing host1x_channel_request to wait for a free channel, and freeing channels when their refcount drops to zero. - Support for more than 32 channels, required for Tegra186 - General refactoring, including better encapsulation of channel ownership handling into channel.c Signed-off-by: Mikko Perttunen --- Ran though test suite at github.com/cyndis/host1x_test. Fixes the timeout test there. drivers/gpu/drm/tegra/gr2d.c | 6 +- drivers/gpu/drm/tegra/gr3d.c | 6 +- drivers/gpu/drm/tegra/vic.c | 6 +- drivers/gpu/host1x/cdma.c | 2 +- drivers/gpu/host1x/channel.c | 154 +++++++++++++++++++++++-------------- drivers/gpu/host1x/channel.h | 28 ++++--- drivers/gpu/host1x/debug.c | 49 +++++------- drivers/gpu/host1x/dev.c | 6 +- drivers/gpu/host1x/dev.h | 8 +- drivers/gpu/host1x/hw/channel_hw.c | 12 +-- include/linux/host1x.h | 1 - 11 files changed, 155 insertions(+), 123 deletions(-) diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c index 02cd3e37a6ec..936ce1a55987 100644 --- a/drivers/gpu/drm/tegra/gr2d.c +++ b/drivers/gpu/drm/tegra/gr2d.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2013, NVIDIA Corporation. + * Copyright (c) 2012-2017, NVIDIA Corporation. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -38,7 +38,7 @@ static int gr2d_init(struct host1x_client *client) client->syncpts[0] = host1x_syncpt_request(client->dev, flags); if (!client->syncpts[0]) { - host1x_channel_free(gr2d->channel); + host1x_channel_put(gr2d->channel); return -ENOMEM; } @@ -57,7 +57,7 @@ static int gr2d_exit(struct host1x_client *client) return err; host1x_syncpt_free(client->syncpts[0]); - host1x_channel_free(gr2d->channel); + host1x_channel_put(gr2d->channel); return 0; } diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c index 13f0d1b7cd98..da02230c58ef 100644 --- a/drivers/gpu/drm/tegra/gr3d.c +++ b/drivers/gpu/drm/tegra/gr3d.c @@ -1,6 +1,6 @@ /* * Copyright (C) 2013 Avionic Design GmbH - * Copyright (C) 2013 NVIDIA Corporation + * Copyright (C) 2013-2017 NVIDIA Corporation * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -48,7 +48,7 @@ static int gr3d_init(struct host1x_client *client) client->syncpts[0] = host1x_syncpt_request(client->dev, flags); if (!client->syncpts[0]) { - host1x_channel_free(gr3d->channel); + host1x_channel_put(gr3d->channel); return -ENOMEM; } @@ -67,7 +67,7 @@ static int gr3d_exit(struct host1x_client *client) return err; host1x_syncpt_free(client->syncpts[0]); - host1x_channel_free(gr3d->channel); + host1x_channel_put(gr3d->channel); return 0; } diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c index cd804e404a11..5a36adfc01f6 100644 --- a/drivers/gpu/drm/tegra/vic.c +++ b/drivers/gpu/drm/tegra/vic.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, NVIDIA Corporation. + * Copyright (c) 2015-2017, NVIDIA Corporation. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -182,7 +182,7 @@ static int vic_init(struct host1x_client *client) free_syncpt: host1x_syncpt_free(client->syncpts[0]); free_channel: - host1x_channel_free(vic->channel); + host1x_channel_put(vic->channel); detach_device: if (tegra->domain) iommu_detach_device(tegra->domain, vic->dev); @@ -203,7 +203,7 @@ static int vic_exit(struct host1x_client *client) return err; host1x_syncpt_free(client->syncpts[0]); - host1x_channel_free(vic->channel); + host1x_channel_put(vic->channel); if (vic->domain) { iommu_detach_device(vic->domain, vic->dev); diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c index 28541b280739..d7c1e269ed67 100644 --- a/drivers/gpu/host1x/cdma.c +++ b/drivers/gpu/host1x/cdma.c @@ -1,7 +1,7 @@ /* * Tegra host1x Command DMA * - * Copyright (c) 2010-2013, NVIDIA Corporation. + * Copyright (c) 2010-2017, NVIDIA Corporation. * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, diff --git a/drivers/gpu/host1x/channel.c b/drivers/gpu/host1x/channel.c index 8f437d924c10..819e97ccb00d 100644 --- a/drivers/gpu/host1x/channel.c +++ b/drivers/gpu/host1x/channel.c @@ -1,7 +1,7 @@ /* * Tegra host1x Channel * - * Copyright (c) 2010-2013, NVIDIA Corporation. + * Copyright (c) 2010-2017, NVIDIA Corporation. * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, @@ -24,19 +24,36 @@ #include "job.h" /* Constructor for the host1x device list */ -int host1x_channel_list_init(struct host1x *host) +int host1x_channel_list_init(struct host1x_channel_list *chlist, + unsigned int num_channels) { - INIT_LIST_HEAD(&host->chlist.list); - mutex_init(&host->chlist_mutex); - - if (host->info->nb_channels > BITS_PER_LONG) { - WARN(1, "host1x hardware has more channels than supported by the driver\n"); - return -ENOSYS; + chlist->channels = kcalloc(num_channels, sizeof(struct host1x_channel), + GFP_KERNEL); + if (!chlist->channels) + return -ENOMEM; + + chlist->allocated_channels = + kcalloc(BITS_TO_LONGS(num_channels), sizeof(unsigned long), + GFP_KERNEL); + if (!chlist->allocated_channels) { + kfree(chlist->channels); + return -ENOMEM; } + bitmap_zero(chlist->allocated_channels, num_channels); + + mutex_init(&chlist->alloc_lock); + sema_init(&chlist->alloc_sema, num_channels); + return 0; } +void host1x_channel_list_free(struct host1x_channel_list *chlist) +{ + kfree(chlist->allocated_channels); + kfree(chlist->channels); +} + int host1x_job_submit(struct host1x_job *job) { struct host1x *host = dev_get_drvdata(job->channel->dev->parent); @@ -47,86 +64,107 @@ EXPORT_SYMBOL(host1x_job_submit); struct host1x_channel *host1x_channel_get(struct host1x_channel *channel) { - int err = 0; - - mutex_lock(&channel->reflock); - - if (channel->refcount == 0) - err = host1x_cdma_init(&channel->cdma); + kref_get(&channel->refcount); - if (!err) - channel->refcount++; + return channel; +} +EXPORT_SYMBOL(host1x_channel_get); - mutex_unlock(&channel->reflock); +/** + * host1x_channel_get_index() - Attempt to get channel reference by index + * @host: Host1x device object + * @index: Index of channel + * + * If channel number @index is currently allocated, increase its refcount + * and return a pointer to it. Otherwise, return NULL. + */ +struct host1x_channel *host1x_channel_get_index(struct host1x *host, + unsigned int index) +{ + struct host1x_channel *ch = &host->channel_list.channels[index]; - return err ? NULL : channel; + if (kref_get_unless_zero(&ch->refcount)) + return ch; + else + return NULL; } -EXPORT_SYMBOL(host1x_channel_get); -void host1x_channel_put(struct host1x_channel *channel) +static void release_channel(struct kref *kref) { - mutex_lock(&channel->reflock); + struct host1x_channel *channel = + container_of(kref, struct host1x_channel, refcount); + struct host1x *host = dev_get_drvdata(channel->dev->parent); + struct host1x_channel_list *chlist = &host->channel_list; - if (channel->refcount == 1) { - struct host1x *host = dev_get_drvdata(channel->dev->parent); + host1x_hw_cdma_stop(host, &channel->cdma); + host1x_cdma_deinit(&channel->cdma); - host1x_hw_cdma_stop(host, &channel->cdma); - host1x_cdma_deinit(&channel->cdma); - } + clear_bit(channel->id, chlist->allocated_channels); - channel->refcount--; + up(&chlist->alloc_sema); +} - mutex_unlock(&channel->reflock); +void host1x_channel_put(struct host1x_channel *channel) +{ + kref_put(&channel->refcount, release_channel); } EXPORT_SYMBOL(host1x_channel_put); +/** + * host1x_channel_request() - Allocate a channel + * @device: Host1x unit this channel will be used to send commands to + * + * Allocates a new host1x channel for @device. If there are no free channels, + * this will sleep until one becomes available. May return NULL if CDMA + * initialization fails. + */ struct host1x_channel *host1x_channel_request(struct device *dev) { struct host1x *host = dev_get_drvdata(dev->parent); + struct host1x_channel_list *chlist = &host->channel_list; unsigned int max_channels = host->info->nb_channels; struct host1x_channel *channel = NULL; - unsigned long index; + unsigned int index; int err; - mutex_lock(&host->chlist_mutex); + down(&chlist->alloc_sema); + + mutex_lock(&chlist->alloc_lock); - index = find_first_zero_bit(&host->allocated_channels, max_channels); - if (index >= max_channels) - goto fail; + index = find_first_zero_bit(chlist->allocated_channels, max_channels); + if (index >= max_channels) { + mutex_unlock(&chlist->alloc_lock); + dev_err(dev, "failed to find free channel\n"); + return NULL; + } - channel = kzalloc(sizeof(*channel), GFP_KERNEL); - if (!channel) - goto fail; + set_bit(index, chlist->allocated_channels); - err = host1x_hw_channel_init(host, channel, index); - if (err < 0) - goto fail; + mutex_unlock(&chlist->alloc_lock); - /* Link device to host1x_channel */ + channel = &chlist->channels[index]; + + kref_init(&channel->refcount); + mutex_init(&channel->submit_lock); + channel->id = index; channel->dev = dev; - /* Add to channel list */ - list_add_tail(&channel->list, &host->chlist.list); + err = host1x_hw_channel_init(host, channel, index); + if (err < 0) + goto free; - host->allocated_channels |= BIT(index); + err = host1x_cdma_init(&channel->cdma); + if (err < 0) + goto free; - mutex_unlock(&host->chlist_mutex); return channel; -fail: - dev_err(dev, "failed to init channel\n"); - kfree(channel); - mutex_unlock(&host->chlist_mutex); - return NULL; -} -EXPORT_SYMBOL(host1x_channel_request); +free: + clear_bit(channel->id, chlist->allocated_channels); + up(&chlist->alloc_sema); -void host1x_channel_free(struct host1x_channel *channel) -{ - struct host1x *host = dev_get_drvdata(channel->dev->parent); + dev_err(dev, "failed to initialize channel\n"); - host->allocated_channels &= ~BIT(channel->id); - list_del(&channel->list); - kfree(channel); + return NULL; } -EXPORT_SYMBOL(host1x_channel_free); +EXPORT_SYMBOL(host1x_channel_request); diff --git a/drivers/gpu/host1x/channel.h b/drivers/gpu/host1x/channel.h index df767cf90d51..91a40f221415 100644 --- a/drivers/gpu/host1x/channel.h +++ b/drivers/gpu/host1x/channel.h @@ -1,7 +1,7 @@ /* * Tegra host1x Channel * - * Copyright (c) 2010-2013, NVIDIA Corporation. + * Copyright (c) 2010-2017, NVIDIA Corporation. * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, @@ -20,27 +20,37 @@ #define __HOST1X_CHANNEL_H #include +#include #include "cdma.h" struct host1x; +struct host1x_channel; + +struct host1x_channel_list { + struct host1x_channel *channels; + + /* Allows clients to wait for a free channel */ + struct semaphore alloc_sema; + struct mutex alloc_lock; + unsigned long *allocated_channels; +}; struct host1x_channel { - struct list_head list; + struct kref refcount; - unsigned int refcount; unsigned int id; - struct mutex reflock; - struct mutex submitlock; + struct mutex submit_lock; void __iomem *regs; struct device *dev; struct host1x_cdma cdma; }; /* channel list operations */ -int host1x_channel_list_init(struct host1x *host); - -#define host1x_for_each_channel(host, channel) \ - list_for_each_entry(channel, &host->chlist.list, list) +int host1x_channel_list_init(struct host1x_channel_list *chlist, + unsigned int num_channels); +void host1x_channel_list_free(struct host1x_channel_list *chlist); +struct host1x_channel *host1x_channel_get_index(struct host1x *host, + unsigned int index); #endif diff --git a/drivers/gpu/host1x/debug.c b/drivers/gpu/host1x/debug.c index d9330fcc62ad..0e8b66fea29f 100644 --- a/drivers/gpu/host1x/debug.c +++ b/drivers/gpu/host1x/debug.c @@ -2,7 +2,7 @@ * Copyright (C) 2010 Google, Inc. * Author: Erik Gilling * - * Copyright (C) 2011-2013 NVIDIA Corporation + * Copyright (C) 2011-2017 NVIDIA Corporation * * This software is licensed under the terms of the GNU General Public * License version 2, as published by the Free Software Foundation, and @@ -43,24 +43,19 @@ void host1x_debug_output(struct output *o, const char *fmt, ...) o->fn(o->ctx, o->buf, len); } -static int show_channels(struct host1x_channel *ch, void *data, bool show_fifo) +static int show_channel(struct host1x_channel *ch, void *data, bool show_fifo) { struct host1x *m = dev_get_drvdata(ch->dev->parent); struct output *o = data; - mutex_lock(&ch->reflock); + mutex_lock(&ch->cdma.lock); - if (ch->refcount) { - mutex_lock(&ch->cdma.lock); + if (show_fifo) + host1x_hw_show_channel_fifo(m, ch, o); - if (show_fifo) - host1x_hw_show_channel_fifo(m, ch, o); + host1x_hw_show_channel_cdma(m, ch, o); - host1x_hw_show_channel_cdma(m, ch, o); - mutex_unlock(&ch->cdma.lock); - } - - mutex_unlock(&ch->reflock); + mutex_unlock(&ch->cdma.lock); return 0; } @@ -94,28 +89,22 @@ static void show_syncpts(struct host1x *m, struct output *o) host1x_debug_output(o, "\n"); } -static void show_all(struct host1x *m, struct output *o) +static void show_all(struct host1x *m, struct output *o, bool show_fifo) { - struct host1x_channel *ch; + int i; host1x_hw_show_mlocks(m, o); show_syncpts(m, o); host1x_debug_output(o, "---- channels ----\n"); - host1x_for_each_channel(m, ch) - show_channels(ch, o, true); -} - -static void show_all_no_fifo(struct host1x *host1x, struct output *o) -{ - struct host1x_channel *ch; - - host1x_hw_show_mlocks(host1x, o); - show_syncpts(host1x, o); - host1x_debug_output(o, "---- channels ----\n"); + for (i = 0; i < m->info->nb_channels; ++i) { + struct host1x_channel *ch = host1x_channel_get_index(m, i); - host1x_for_each_channel(host1x, ch) - show_channels(ch, o, false); + if (ch) { + show_channel(ch, o, show_fifo); + host1x_channel_put(ch); + } + } } static int host1x_debug_show_all(struct seq_file *s, void *unused) @@ -125,7 +114,7 @@ static int host1x_debug_show_all(struct seq_file *s, void *unused) .ctx = s }; - show_all(s->private, &o); + show_all(s->private, &o, true); return 0; } @@ -137,7 +126,7 @@ static int host1x_debug_show(struct seq_file *s, void *unused) .ctx = s }; - show_all_no_fifo(s->private, &o); + show_all(s->private, &o, false); return 0; } @@ -216,7 +205,7 @@ void host1x_debug_dump(struct host1x *host1x) .fn = write_to_printk }; - show_all(host1x, &o); + show_all(host1x, &o, true); } void host1x_debug_dump_syncpts(struct host1x *host1x) diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index a8234bb49403..f57ef45f75c9 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -1,7 +1,7 @@ /* * Tegra host1x driver * - * Copyright (c) 2010-2013, NVIDIA Corporation. + * Copyright (c) 2010-2017, NVIDIA Corporation. * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, @@ -189,7 +189,8 @@ static int host1x_probe(struct platform_device *pdev) host->iova_end = geometry->aperture_end; } - err = host1x_channel_list_init(host); + err = host1x_channel_list_init(&host->channel_list, + host->info->nb_channels); if (err) { dev_err(&pdev->dev, "failed to initialize channel list\n"); goto fail_detach_device; @@ -247,6 +248,7 @@ static int host1x_remove(struct platform_device *pdev) host1x_intr_deinit(host); host1x_syncpt_deinit(host); clk_disable_unprepare(host->clk); + host1x_channel_list_free(&host->channel_list); if (host->domain) { put_iova_domain(&host->iova); diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h index 1ee79dbd1882..a558e39bf7f5 100644 --- a/drivers/gpu/host1x/dev.h +++ b/drivers/gpu/host1x/dev.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2012-2016 NVIDIA CORPORATION. All rights reserved. + * Copyright (C) 2012-2017 NVIDIA CORPORATION. All rights reserved. * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, @@ -130,10 +130,8 @@ struct host1x { struct host1x_syncpt *nop_sp; struct mutex syncpt_mutex; - struct mutex chlist_mutex; - struct host1x_channel chlist; - unsigned long allocated_channels; - unsigned int num_allocated_channels; + + struct host1x_channel_list channel_list; struct dentry *debugfs; diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c index 27dc78f4c400..286c81aad308 100644 --- a/drivers/gpu/host1x/hw/channel_hw.c +++ b/drivers/gpu/host1x/hw/channel_hw.c @@ -138,13 +138,13 @@ static int channel_submit(struct host1x_job *job) } /* get submit lock */ - err = mutex_lock_interruptible(&ch->submitlock); + err = mutex_lock_interruptible(&ch->submit_lock); if (err) goto error; completed_waiter = kzalloc(sizeof(*completed_waiter), GFP_KERNEL); if (!completed_waiter) { - mutex_unlock(&ch->submitlock); + mutex_unlock(&ch->submit_lock); err = -ENOMEM; goto error; } @@ -152,7 +152,7 @@ static int channel_submit(struct host1x_job *job) /* begin a CDMA submit */ err = host1x_cdma_begin(&ch->cdma, job); if (err) { - mutex_unlock(&ch->submitlock); + mutex_unlock(&ch->submit_lock); goto error; } @@ -190,7 +190,7 @@ static int channel_submit(struct host1x_job *job) completed_waiter = NULL; WARN(err, "Failed to set submit complete interrupt"); - mutex_unlock(&ch->submitlock); + mutex_unlock(&ch->submit_lock); return 0; @@ -202,10 +202,6 @@ static int channel_submit(struct host1x_job *job) static int host1x_channel_init(struct host1x_channel *ch, struct host1x *dev, unsigned int index) { - ch->id = index; - mutex_init(&ch->reflock); - mutex_init(&ch->submitlock); - ch->regs = dev->regs + index * HOST1X_CHANNEL_SIZE; return 0; } diff --git a/include/linux/host1x.h b/include/linux/host1x.h index fd9b526486e0..921ec6465952 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -157,7 +157,6 @@ struct host1x_channel; struct host1x_job; struct host1x_channel *host1x_channel_request(struct device *dev); -void host1x_channel_free(struct host1x_channel *channel); struct host1x_channel *host1x_channel_get(struct host1x_channel *channel); void host1x_channel_put(struct host1x_channel *channel); int host1x_job_submit(struct host1x_job *job);