gpu: host1x: Cleanup and refcounting for syncpoints
authorMikko Perttunen <mperttunen@nvidia.com>
Mon, 29 Mar 2021 13:38:32 +0000 (16:38 +0300)
committerThierry Reding <treding@nvidia.com>
Wed, 31 Mar 2021 15:42:13 +0000 (17:42 +0200)
Add reference counting for allocated syncpoints to allow keeping
them allocated while jobs are referencing them. Additionally,
clean up various places using syncpoint IDs to use host1x_syncpt
pointers instead.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
15 files changed:
drivers/gpu/drm/tegra/dc.c
drivers/gpu/drm/tegra/drm.c
drivers/gpu/drm/tegra/gr2d.c
drivers/gpu/drm/tegra/gr3d.c
drivers/gpu/drm/tegra/vic.c
drivers/gpu/host1x/cdma.c
drivers/gpu/host1x/dev.h
drivers/gpu/host1x/hw/cdma_hw.c
drivers/gpu/host1x/hw/channel_hw.c
drivers/gpu/host1x/hw/debug_hw.c
drivers/gpu/host1x/job.c
drivers/gpu/host1x/syncpt.c
drivers/gpu/host1x/syncpt.h
drivers/staging/media/tegra-video/vi.c
include/linux/host1x.h

index c9385cf..cfda71e 100644 (file)
@@ -2141,7 +2141,7 @@ cleanup:
                drm_plane_cleanup(primary);
 
        host1x_client_iommu_detach(client);
-       host1x_syncpt_free(dc->syncpt);
+       host1x_syncpt_put(dc->syncpt);
 
        return err;
 }
@@ -2166,7 +2166,7 @@ static int tegra_dc_exit(struct host1x_client *client)
        }
 
        host1x_client_iommu_detach(client);
-       host1x_syncpt_free(dc->syncpt);
+       host1x_syncpt_put(dc->syncpt);
 
        return 0;
 }
index 90709c3..ce5bdc5 100644 (file)
@@ -174,7 +174,7 @@ int tegra_drm_submit(struct tegra_drm_context *context,
        struct drm_tegra_syncpt syncpt;
        struct host1x *host1x = dev_get_drvdata(drm->dev->parent);
        struct drm_gem_object **refs;
-       struct host1x_syncpt *sp;
+       struct host1x_syncpt *sp = NULL;
        struct host1x_job *job;
        unsigned int num_refs;
        int err;
@@ -301,8 +301,8 @@ int tegra_drm_submit(struct tegra_drm_context *context,
                goto fail;
        }
 
-       /* check whether syncpoint ID is valid */
-       sp = host1x_syncpt_get(host1x, syncpt.id);
+       /* Syncpoint ref will be dropped on job release. */
+       sp = host1x_syncpt_get_by_id(host1x, syncpt.id);
        if (!sp) {
                err = -ENOENT;
                goto fail;
@@ -311,7 +311,7 @@ int tegra_drm_submit(struct tegra_drm_context *context,
        job->is_addr_reg = context->client->ops->is_addr_reg;
        job->is_valid_class = context->client->ops->is_valid_class;
        job->syncpt_incrs = syncpt.incrs;
-       job->syncpt_id = syncpt.id;
+       job->syncpt = sp;
        job->timeout = 10000;
 
        if (args->timeout && args->timeout < 10000)
@@ -383,7 +383,7 @@ static int tegra_syncpt_read(struct drm_device *drm, void *data,
        struct drm_tegra_syncpt_read *args = data;
        struct host1x_syncpt *sp;
 
-       sp = host1x_syncpt_get(host, args->id);
+       sp = host1x_syncpt_get_by_id_noref(host, args->id);
        if (!sp)
                return -EINVAL;
 
@@ -398,7 +398,7 @@ static int tegra_syncpt_incr(struct drm_device *drm, void *data,
        struct drm_tegra_syncpt_incr *args = data;
        struct host1x_syncpt *sp;
 
-       sp = host1x_syncpt_get(host1x, args->id);
+       sp = host1x_syncpt_get_by_id_noref(host1x, args->id);
        if (!sp)
                return -EINVAL;
 
@@ -412,7 +412,7 @@ static int tegra_syncpt_wait(struct drm_device *drm, void *data,
        struct drm_tegra_syncpt_wait *args = data;
        struct host1x_syncpt *sp;
 
-       sp = host1x_syncpt_get(host1x, args->id);
+       sp = host1x_syncpt_get_by_id_noref(host1x, args->id);
        if (!sp)
                return -EINVAL;
 
index adbe2dd..de288cb 100644 (file)
@@ -67,7 +67,7 @@ static int gr2d_init(struct host1x_client *client)
 detach:
        host1x_client_iommu_detach(client);
 free:
-       host1x_syncpt_free(client->syncpts[0]);
+       host1x_syncpt_put(client->syncpts[0]);
 put:
        host1x_channel_put(gr2d->channel);
        return err;
@@ -86,7 +86,7 @@ static int gr2d_exit(struct host1x_client *client)
                return err;
 
        host1x_client_iommu_detach(client);
-       host1x_syncpt_free(client->syncpts[0]);
+       host1x_syncpt_put(client->syncpts[0]);
        host1x_channel_put(gr2d->channel);
 
        return 0;
index b0b8154..24442ad 100644 (file)
@@ -76,7 +76,7 @@ static int gr3d_init(struct host1x_client *client)
 detach:
        host1x_client_iommu_detach(client);
 free:
-       host1x_syncpt_free(client->syncpts[0]);
+       host1x_syncpt_put(client->syncpts[0]);
 put:
        host1x_channel_put(gr3d->channel);
        return err;
@@ -94,7 +94,7 @@ static int gr3d_exit(struct host1x_client *client)
                return err;
 
        host1x_client_iommu_detach(client);
-       host1x_syncpt_free(client->syncpts[0]);
+       host1x_syncpt_put(client->syncpts[0]);
        host1x_channel_put(gr3d->channel);
 
        return 0;
index 77e1288..72aea1c 100644 (file)
@@ -214,7 +214,7 @@ static int vic_init(struct host1x_client *client)
        return 0;
 
 free_syncpt:
-       host1x_syncpt_free(client->syncpts[0]);
+       host1x_syncpt_put(client->syncpts[0]);
 free_channel:
        host1x_channel_put(vic->channel);
 detach:
@@ -238,7 +238,7 @@ static int vic_exit(struct host1x_client *client)
        if (err < 0)
                return err;
 
-       host1x_syncpt_free(client->syncpts[0]);
+       host1x_syncpt_put(client->syncpts[0]);
        host1x_channel_put(vic->channel);
        host1x_client_iommu_detach(client);
 
index e8d3fda..6e6ca77 100644 (file)
@@ -273,15 +273,13 @@ static int host1x_cdma_wait_pushbuffer_space(struct host1x *host1x,
 static void cdma_start_timer_locked(struct host1x_cdma *cdma,
                                    struct host1x_job *job)
 {
-       struct host1x *host = cdma_to_host1x(cdma);
-
        if (cdma->timeout.client) {
                /* timer already started */
                return;
        }
 
        cdma->timeout.client = job->client;
-       cdma->timeout.syncpt = host1x_syncpt_get(host, job->syncpt_id);
+       cdma->timeout.syncpt = job->syncpt;
        cdma->timeout.syncpt_val = job->syncpt_end;
        cdma->timeout.start_ktime = ktime_get();
 
@@ -312,7 +310,6 @@ static void stop_cdma_timer_locked(struct host1x_cdma *cdma)
 static void update_cdma_locked(struct host1x_cdma *cdma)
 {
        bool signal = false;
-       struct host1x *host1x = cdma_to_host1x(cdma);
        struct host1x_job *job, *n;
 
        /* If CDMA is stopped, queue is cleared and we can return */
@@ -324,8 +321,7 @@ static void update_cdma_locked(struct host1x_cdma *cdma)
         * to consume as many sync queue entries as possible without blocking
         */
        list_for_each_entry_safe(job, n, &cdma->sync_queue, list) {
-               struct host1x_syncpt *sp =
-                       host1x_syncpt_get(host1x, job->syncpt_id);
+               struct host1x_syncpt *sp = job->syncpt;
 
                /* Check whether this syncpt has completed, and bail if not */
                if (!host1x_syncpt_is_expired(sp, job->syncpt_end)) {
@@ -499,8 +495,7 @@ int host1x_cdma_begin(struct host1x_cdma *cdma, struct host1x_job *job)
                if (!cdma->timeout.initialized) {
                        int err;
 
-                       err = host1x_hw_cdma_timeout_init(host1x, cdma,
-                                                         job->syncpt_id);
+                       err = host1x_hw_cdma_timeout_init(host1x, cdma);
                        if (err) {
                                mutex_unlock(&cdma->lock);
                                return err;
index f781a9b..63010ae 100644 (file)
@@ -37,7 +37,7 @@ struct host1x_cdma_ops {
        void (*start)(struct host1x_cdma *cdma);
        void (*stop)(struct host1x_cdma *cdma);
        void (*flush)(struct  host1x_cdma *cdma);
-       int (*timeout_init)(struct host1x_cdma *cdma, unsigned int syncpt);
+       int (*timeout_init)(struct host1x_cdma *cdma);
        void (*timeout_destroy)(struct host1x_cdma *cdma);
        void (*freeze)(struct host1x_cdma *cdma);
        void (*resume)(struct host1x_cdma *cdma, u32 getptr);
@@ -261,10 +261,9 @@ static inline void host1x_hw_cdma_flush(struct host1x *host,
 }
 
 static inline int host1x_hw_cdma_timeout_init(struct host1x *host,
-                                             struct host1x_cdma *cdma,
-                                             unsigned int syncpt)
+                                             struct host1x_cdma *cdma)
 {
-       return host->cdma_op->timeout_init(cdma, syncpt);
+       return host->cdma_op->timeout_init(cdma);
 }
 
 static inline void host1x_hw_cdma_timeout_destroy(struct host1x *host,
index 2f3bf94..e49cd5b 100644 (file)
@@ -295,7 +295,7 @@ static void cdma_timeout_handler(struct work_struct *work)
 /*
  * Init timeout resources
  */
-static int cdma_timeout_init(struct host1x_cdma *cdma, unsigned int syncpt)
+static int cdma_timeout_init(struct host1x_cdma *cdma)
 {
        INIT_DELAYED_WORK(&cdma->timeout.wq, cdma_timeout_handler);
        cdma->timeout.initialized = true;
index 5eaa29d..d4c28fa 100644 (file)
@@ -86,8 +86,7 @@ static void submit_gathers(struct host1x_job *job)
 
 static inline void synchronize_syncpt_base(struct host1x_job *job)
 {
-       struct host1x *host = dev_get_drvdata(job->channel->dev->parent);
-       struct host1x_syncpt *sp = host->syncpt + job->syncpt_id;
+       struct host1x_syncpt *sp = job->syncpt;
        unsigned int id;
        u32 value;
 
@@ -118,7 +117,7 @@ static void host1x_channel_set_streamid(struct host1x_channel *channel)
 static int channel_submit(struct host1x_job *job)
 {
        struct host1x_channel *ch = job->channel;
-       struct host1x_syncpt *sp;
+       struct host1x_syncpt *sp = job->syncpt;
        u32 user_syncpt_incrs = job->syncpt_incrs;
        u32 prev_max = 0;
        u32 syncval;
@@ -126,10 +125,9 @@ static int channel_submit(struct host1x_job *job)
        struct host1x_waitlist *completed_waiter = NULL;
        struct host1x *host = dev_get_drvdata(ch->dev->parent);
 
-       sp = host->syncpt + job->syncpt_id;
        trace_host1x_channel_submit(dev_name(ch->dev),
                                    job->num_gathers, job->num_relocs,
-                                   job->syncpt_id, job->syncpt_incrs);
+                                   job->syncpt->id, job->syncpt_incrs);
 
        /* before error checks, return current max */
        prev_max = job->syncpt_end = host1x_syncpt_read_max(sp);
@@ -163,7 +161,7 @@ static int channel_submit(struct host1x_job *job)
                host1x_cdma_push(&ch->cdma,
                                 host1x_opcode_setclass(HOST1X_CLASS_HOST1X,
                                        host1x_uclass_wait_syncpt_r(), 1),
-                                host1x_class_host_wait_syncpt(job->syncpt_id,
+                                host1x_class_host_wait_syncpt(job->syncpt->id,
                                        host1x_syncpt_read_max(sp)));
        }
 
index f31bcfa..ceb4822 100644 (file)
@@ -204,7 +204,7 @@ static void show_channel_gathers(struct output *o, struct host1x_cdma *cdma)
                unsigned int i;
 
                host1x_debug_output(o, "\n%p: JOB, syncpt_id=%d, syncpt_val=%d, first_get=%08x, timeout=%d num_slots=%d, num_handles=%d\n",
-                                   job, job->syncpt_id, job->syncpt_end,
+                                   job, job->syncpt->id, job->syncpt_end,
                                    job->first_get, job->timeout,
                                    job->num_slots, job->num_unpins);
 
index 82d0a60..adbdc22 100644 (file)
@@ -79,6 +79,9 @@ static void job_free(struct kref *ref)
 {
        struct host1x_job *job = container_of(ref, struct host1x_job, ref);
 
+       if (job->syncpt)
+               host1x_syncpt_put(job->syncpt);
+
        kfree(job);
 }
 
@@ -674,7 +677,7 @@ EXPORT_SYMBOL(host1x_job_unpin);
  */
 void host1x_job_dump(struct device *dev, struct host1x_job *job)
 {
-       dev_dbg(dev, "    SYNCPT_ID   %d\n", job->syncpt_id);
+       dev_dbg(dev, "    SYNCPT_ID   %d\n", job->syncpt->id);
        dev_dbg(dev, "    SYNCPT_VAL  %d\n", job->syncpt_end);
        dev_dbg(dev, "    FIRST_GET   0x%x\n", job->first_get);
        dev_dbg(dev, "    TIMEOUT     %d\n", job->timeout);
index 8da4bbc..7bb5de8 100644 (file)
@@ -90,6 +90,8 @@ struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
        else
                sp->client_managed = false;
 
+       kref_init(&sp->ref);
+
        mutex_unlock(&host->syncpt_mutex);
        return sp;
 
@@ -383,7 +385,7 @@ int host1x_syncpt_init(struct host1x *host)
  * host1x client drivers can use this function to allocate a syncpoint for
  * subsequent use. A syncpoint returned by this function will be reserved for
  * use by the client exclusively. When no longer using a syncpoint, a host1x
- * client driver needs to release it using host1x_syncpt_free().
+ * client driver needs to release it using host1x_syncpt_put().
  */
 struct host1x_syncpt *host1x_syncpt_request(struct host1x_client *client,
                                            unsigned long flags)
@@ -394,20 +396,9 @@ struct host1x_syncpt *host1x_syncpt_request(struct host1x_client *client,
 }
 EXPORT_SYMBOL(host1x_syncpt_request);
 
-/**
- * host1x_syncpt_free() - free a requested syncpoint
- * @sp: host1x syncpoint
- *
- * Release a syncpoint previously allocated using host1x_syncpt_request(). A
- * host1x client driver should call this when the syncpoint is no longer in
- * use. Note that client drivers must ensure that the syncpoint doesn't remain
- * under the control of hardware after calling this function, otherwise two
- * clients may end up trying to access the same syncpoint concurrently.
- */
-void host1x_syncpt_free(struct host1x_syncpt *sp)
+static void syncpt_release(struct kref *ref)
 {
-       if (!sp)
-               return;
+       struct host1x_syncpt *sp = container_of(ref, struct host1x_syncpt, ref);
 
        mutex_lock(&sp->host->syncpt_mutex);
 
@@ -419,7 +410,23 @@ void host1x_syncpt_free(struct host1x_syncpt *sp)
 
        mutex_unlock(&sp->host->syncpt_mutex);
 }
-EXPORT_SYMBOL(host1x_syncpt_free);
+
+/**
+ * host1x_syncpt_put() - free a requested syncpoint
+ * @sp: host1x syncpoint
+ *
+ * Release a syncpoint previously allocated using host1x_syncpt_request(). A
+ * host1x client driver should call this when the syncpoint is no longer in
+ * use.
+ */
+void host1x_syncpt_put(struct host1x_syncpt *sp)
+{
+       if (!sp)
+               return;
+
+       kref_put(&sp->ref, syncpt_release);
+}
+EXPORT_SYMBOL(host1x_syncpt_put);
 
 void host1x_syncpt_deinit(struct host1x *host)
 {
@@ -486,16 +493,48 @@ unsigned int host1x_syncpt_nb_mlocks(struct host1x *host)
 }
 
 /**
- * host1x_syncpt_get() - obtain a syncpoint by ID
+ * host1x_syncpt_get_by_id() - obtain a syncpoint by ID
+ * @host: host1x controller
+ * @id: syncpoint ID
+ */
+struct host1x_syncpt *host1x_syncpt_get_by_id(struct host1x *host,
+                                             unsigned int id)
+{
+       if (id >= host->info->nb_pts)
+               return NULL;
+
+       if (kref_get_unless_zero(&host->syncpt[id].ref))
+               return &host->syncpt[id];
+       else
+               return NULL;
+}
+EXPORT_SYMBOL(host1x_syncpt_get_by_id);
+
+/**
+ * host1x_syncpt_get_by_id_noref() - obtain a syncpoint by ID but don't
+ *     increase the refcount.
  * @host: host1x controller
  * @id: syncpoint ID
  */
-struct host1x_syncpt *host1x_syncpt_get(struct host1x *host, unsigned int id)
+struct host1x_syncpt *host1x_syncpt_get_by_id_noref(struct host1x *host,
+                                                   unsigned int id)
 {
        if (id >= host->info->nb_pts)
                return NULL;
 
-       return host->syncpt + id;
+       return &host->syncpt[id];
+}
+EXPORT_SYMBOL(host1x_syncpt_get_by_id_noref);
+
+/**
+ * host1x_syncpt_get() - increment syncpoint refcount
+ * @sp: syncpoint
+ */
+struct host1x_syncpt *host1x_syncpt_get(struct host1x_syncpt *sp)
+{
+       kref_get(&sp->ref);
+
+       return sp;
 }
 EXPORT_SYMBOL(host1x_syncpt_get);
 
index 3aa6b25..a6766f8 100644 (file)
@@ -11,6 +11,7 @@
 #include <linux/atomic.h>
 #include <linux/host1x.h>
 #include <linux/kernel.h>
+#include <linux/kref.h>
 #include <linux/sched.h>
 
 #include "intr.h"
@@ -26,6 +27,8 @@ struct host1x_syncpt_base {
 };
 
 struct host1x_syncpt {
+       struct kref ref;
+
        unsigned int id;
        atomic_t min_val;
        atomic_t max_val;
index 7a09061..df5ca35 100644 (file)
@@ -1131,8 +1131,8 @@ static void tegra_channel_host1x_syncpts_free(struct tegra_vi_channel *chan)
        int i;
 
        for (i = 0; i < chan->numgangports; i++) {
-               host1x_syncpt_free(chan->mw_ack_sp[i]);
-               host1x_syncpt_free(chan->frame_start_sp[i]);
+               host1x_syncpt_put(chan->mw_ack_sp[i]);
+               host1x_syncpt_put(chan->frame_start_sp[i]);
        }
 }
 
@@ -1177,7 +1177,7 @@ static int tegra_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
                mw_sp = host1x_syncpt_request(&vi->client, flags);
                if (!mw_sp) {
                        dev_err(vi->dev, "failed to request memory ack syncpoint\n");
-                       host1x_syncpt_free(fs_sp);
+                       host1x_syncpt_put(fs_sp);
                        ret = -ENOMEM;
                        goto free_syncpts;
                }
index 7137ce0..107aea2 100644 (file)
@@ -142,7 +142,9 @@ struct host1x_syncpt_base;
 struct host1x_syncpt;
 struct host1x;
 
-struct host1x_syncpt *host1x_syncpt_get(struct host1x *host, u32 id);
+struct host1x_syncpt *host1x_syncpt_get_by_id(struct host1x *host, u32 id);
+struct host1x_syncpt *host1x_syncpt_get_by_id_noref(struct host1x *host, u32 id);
+struct host1x_syncpt *host1x_syncpt_get(struct host1x_syncpt *sp);
 u32 host1x_syncpt_id(struct host1x_syncpt *sp);
 u32 host1x_syncpt_read_min(struct host1x_syncpt *sp);
 u32 host1x_syncpt_read_max(struct host1x_syncpt *sp);
@@ -153,7 +155,7 @@ int host1x_syncpt_wait(struct host1x_syncpt *sp, u32 thresh, long timeout,
                       u32 *value);
 struct host1x_syncpt *host1x_syncpt_request(struct host1x_client *client,
                                            unsigned long flags);
-void host1x_syncpt_free(struct host1x_syncpt *sp);
+void host1x_syncpt_put(struct host1x_syncpt *sp);
 struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
                                          unsigned long flags,
                                          const char *name);
@@ -221,7 +223,7 @@ struct host1x_job {
        dma_addr_t *reloc_addr_phys;
 
        /* Sync point id, number of increments and end related to the submit */
-       u32 syncpt_id;
+       struct host1x_syncpt *syncpt;
        u32 syncpt_incrs;
        u32 syncpt_end;