On 9/5/20 5:30 PM, Dmitry Osipenko wrote:
05.09.2020 13:34, Mikko Perttunen пишет:
...
+
+/**
+ * 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)
  {
@@ -471,16 +478,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);

Hello, Mikko!

What do you think about to open-code all the host1x structs by moving
them all out into the public linux/host1x.h? Then we could inline all
these trivial single-line functions by having them defined in the public
header. This will avoid all the unnecessary overhead by allowing
compiler to optimize the code nicely.

Of course this could be a separate change and it could be done sometime
later, I just wanted to share this quick thought for the start of the
review.


Hi :)

I think for such micro-optimizations we should have a benchmark to evaluate against. I'm not sure we have all that many function calls into here overall that it would make a noticeable difference. In any case, as you said, I'd prefer to keep further refactoring to a separate series to avoid growing this series too much.

Mikko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to