linux-sgx.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: "Asahi Lina" <lina@asahilina.net>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	"Luben Tuikov" <luben.tuikov@amd.com>,
	"Jarkko Sakkinen" <jarkko@kernel.org>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"Alyssa Rosenzweig" <alyssa@rosenzweig.io>,
	"Karol Herbst" <kherbst@redhat.com>,
	"Ella Stanforth" <ella@iglunix.org>,
	"Faith Ekstrand" <faith.ekstrand@collabora.com>,
	Mary <mary@mary.zone>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	rust-for-linux@vger.kernel.org, linux-media@vger.kernel.org,
	linaro-mm-sig@lists.linaro.org, linux-sgx@vger.kernel.org,
	asahi@lists.linux.dev, "Daniel Vetter" <daniel@ffwll.ch>
Subject: Re: [PATCH RFC 07/18] rust: drm: mm: Add DRM MM Range Allocator abstraction
Date: Thu, 6 Apr 2023 17:45:34 +0200	[thread overview]
Message-ID: <ZC7pHlLFutMWJF1s@phenom.ffwll.local> (raw)
In-Reply-To: <CANiq72=E2k7+_cwYw_EjVezk0mWCRqbH4MUjsEmxWgJMh=3AEQ@mail.gmail.com>

On Thu, Apr 06, 2023 at 05:28:59PM +0200, Miguel Ojeda wrote:
> On Thu, Apr 6, 2023 at 4:15 PM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > Documentation:
> >
> > In drm we try to document all the interfaces that drivers use with formal
> > docs. Yes there's some areas that are not great for historical reasons,
> > but for new stuff and new wrappers we're really trying:
> >
> > - This helps in telling internal (even across .c files or in rust across
> >   modules within a crate) from stuff drivers access. Sure you have static
> >   in C or pub in rust, but that doesn't tell you whether it's public all
> >   the way to drivers.
> 
> I think you may be talking about the value high-level docs here, but
> just in case, visibility in Rust is flexible enough to expose (or not)
> APIs to those that you need. In other words, it does tell you (and
> enforces!) whether it is public all the way to drivers.
> 
> There is also the possibility of even more fancy visibility, but so
> far we just needed `pub(crate)`.
> 
> `rustdoc` also shows/hides things as needed, thus the generated docs
> for the crate should only show what is usable by others.
> 
> Then there is the `kernel` crate split, too.
> 
> > - ideally docs have a short intro section that explains the main concepts
> >   and links to the main data structures and functions. Just to give
> >   readers a good starting point to explore.
> 
> Agreed, this is typically done in Rust in the top-level doc comments
> (module or crate). For the Rust side of the kernel, we are definitely
> trying to emphasize the quality of the docs, including compile- and
> runtime-tested examples.
> 
> Regarding linking, `rustdoc` already generates a listing with the
> contents of each crate/module even if there is no other docs. So as
> long as the short descriptions of the items are good, it may be fairly
> readable already, e.g. see
> https://rust-for-linux.github.io/docs/rust/kernel/sync/index.html for
> an example in our old `rust` branch. But, of course, you can add extra
> docs at that level too when there are many things or is unclear what
> should be used.
> 
> Also note that, sometimes, the docs we write are in the type, rather
> than the module, e.g. see the nice examples Wedson wrote for `RBTree`:
> https://rust-for-linux.github.io/docs/rust/kernel/rbtree/struct.RBTree.html.

Yeah this all looks great and very hyperlinked.

I think the only nit I have is that for types with two or more type
variables (like the rbtree) what each of them should represent in the top
intro. I can guess it's <Key, Value> and not the other way round, but
confirmation takes quite a bit of scrolling to check with the function
types.

Otherwise I think perfect api docs.

> > - Linking all the things, so that readers can connect the different parts.
> >   This is really important in C where e.g. get/put() or any such function
> >   pairs all needed to be linked together. With rust I'm hoping that
> >   rustdoc liberally sprinkles links already and we don't have to do this
> >   as much.
> 
> If you mean within doc comments, it does! :) It is called "intra-doc
> links". Basically, you just write something in-between square
> brackets, and it is able to create the link to the right thing (in
> most cases, otherwise you can help it more), e.g.
> 
>     /// Returns a new [`Foo`].
> 
> And, of course, for the rest of things that aren't inside comments, it
> automatically provides links etc.
> 
> There has been work on `rustdoc` on getting "Jump to Definition" and
> similar features to work on the source view, too.
> 
> > - Short explainers for parameters. For rust this also means type
> >   parameters, for those even simplified examples of how drivers are
> >   supposed to use them would help a lot in reading docs & understanding
> >   concepts.
> 
> For parameters, we are not forcing to write explanations for every
> parameter (as in providing a list), but rather writing what is
> actually useful to know (referring to the parameters as needed). So it
> depends on a case-by-case.
> 
> In any case, in general is clearer what parameters are compared to C,
> due to the stronger typing. Of course, if one uses integers
> everywhere, it is as confusing as C. But if one has a type, it is
> easier to tell, plus one may jump with a click into the explanation of
> that type etc.
> 
> Regarding examples, 100% agreed. And not only that, the examples are
> enforced to be kept up to date by compiling and running them via KUnit
> (not yet submitted for mainline, but we have been enforcing it for our
> old `rust` branch for a long time).
> 
> > - Ideally links from the rust to the sphinx side to linke relevant
> >   chapters together. Often the bigger explanations are in .rst files with
> >   DOT graphs (kms has a bunch I've added) or similar, and it doesn't make
> >   that much sense to duplicate all that on the rust side I guess. But it
> >   needs to be discoverable.
> 
> Definitely. One next step is having easy-to-write links to the rST
> docs. For this, a couple years ago I talked with the `rustdoc`
> maintainers about having a "External references map file" feature, so
> that we can link rST documents from the Rust docs, including generated
> C docs too. For instance, ideally we would be able to use the square
> brackets around a C type and have it work:
> 
>     /// Exposes the kernel’s [`struct wait_queue_head`] as a condition variable.
> 
> Regarding the bigger explanations: we are trying to keep most of the
> docs close to the Rust code where it makes sense, as
> module-level/crate-level docs, rather than as rST docs. This has
> several benefits, like keeping them closer to the code, the linking
> features, having them organized equally as the code, no need to know
> whether there is a doc somewhere or not (e.g. if it is, it is near the
> code), examples are compiled, etc.

Just a quick comment on this, that's the same we do on the C side. Most
overview chapters are actually DOC: sections pulled in from the code.

What I meant here is that for big overview stuff (like for modesetting how
the display pipe structures tie together as an example:
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#overview)
it doesn't make sense to duplicate that in rustdoc once more.

> Of course, sometimes longer-form docs and other documents may not make
> sense as part of any code in particular, or may be shared across C and
> Rust, etc., and there it may more sense to use `Documentation/` files
> instead.
> 
> But, in general, the idea is that, compared to C, most of the docs go
> into the code. To give an idea of the difference: so far, in our old
> `rust` branch, we only needed a few documents in `Documentation/`
> (e.g. the Quick Start guide etc.), and everything else went into the
> code itself.

Maybe drm is the exception, but if you look at our .rst files we also have
most of our docs in the code:

https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/drm-kms-helpers.rst

The rst files just provide the scaffolding because C dosn't have
crates/modules hierarchy that would do this for you automatically.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

  reply	other threads:[~2023-04-06 15:45 UTC|newest]

Thread overview: 122+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-07 14:25 [PATCH RFC 00/18] Rust DRM subsystem abstractions (& preview AGX driver) Asahi Lina
2023-03-07 14:25 ` [PATCH RFC 01/18] rust: drm: ioctl: Add DRM ioctl abstraction Asahi Lina
2023-03-07 14:48   ` Karol Herbst
2023-03-07 14:51     ` Karol Herbst
2023-03-07 15:32   ` Maíra Canal
2023-03-09  5:32     ` Asahi Lina
2023-03-09  6:15       ` Dave Airlie
2023-03-09 12:09         ` Maíra Canal
2023-03-07 17:34   ` Björn Roy Baron
2023-03-09  6:04     ` Asahi Lina
2023-03-09 20:24       ` Faith Ekstrand
2023-03-09 20:39         ` Karol Herbst
2023-03-10  6:21           ` Asahi Lina
2023-04-13  9:23   ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 02/18] rust: drm: Add Device and Driver abstractions Asahi Lina
2023-03-07 18:19   ` Björn Roy Baron
2023-03-09  6:10     ` Asahi Lina
2023-03-10 18:56   ` Boqun Feng
2023-03-11  5:41   ` Boqun Feng
2023-04-05 17:10   ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 03/18] rust: drm: file: Add File abstraction Asahi Lina
2023-03-09 21:16   ` Faith Ekstrand
2023-03-09 22:16     ` Asahi Lina
2023-03-13 17:49       ` Faith Ekstrand
2023-03-14  2:07         ` Boqun Feng
2023-04-05 11:25           ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 04/18] rust: drm: gem: Add GEM object abstraction Asahi Lina
2023-04-05 11:08   ` Daniel Vetter
2023-04-05 11:19     ` Miguel Ojeda
2023-04-05 11:22       ` Daniel Vetter
2023-04-05 12:32         ` Miguel Ojeda
2023-04-05 12:36           ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 05/18] drm/gem-shmem: Export VM ops functions Asahi Lina
2023-03-07 14:25 ` [PATCH RFC 06/18] rust: drm: gem: shmem: Add DRM shmem helper abstraction Asahi Lina
2023-03-08 13:38   ` Maíra Canal
2023-03-09  5:25     ` Asahi Lina
2023-03-09 11:47       ` Maíra Canal
2023-03-09 14:16         ` Asahi Lina
2023-03-07 14:25 ` [PATCH RFC 07/18] rust: drm: mm: Add DRM MM Range Allocator abstraction Asahi Lina
2023-04-06 14:15   ` Daniel Vetter
2023-04-06 15:28     ` Miguel Ojeda
2023-04-06 15:45       ` Daniel Vetter [this message]
2023-04-06 17:19         ` Miguel Ojeda
2023-04-06 15:53     ` Asahi Lina
2023-04-06 16:13       ` [Linaro-mm-sig] " Daniel Vetter
2023-04-06 16:39         ` Asahi Lina
2023-03-07 14:25 ` [PATCH RFC 08/18] rust: dma_fence: Add DMA Fence abstraction Asahi Lina
2023-04-05 11:10   ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 09/18] rust: drm: syncobj: Add DRM Sync Object abstraction Asahi Lina
2023-04-05 12:33   ` Daniel Vetter
2023-04-06 16:04     ` Asahi Lina
2023-03-07 14:25 ` [PATCH RFC 10/18] drm/scheduler: Add can_run_job callback Asahi Lina
2023-03-08  8:46   ` Christian König
2023-03-08  9:41     ` Asahi Lina
2023-03-08 10:00       ` Christian König
2023-03-08 14:53         ` Asahi Lina
2023-03-08 15:30           ` Christian König
2023-03-08 16:44             ` Asahi Lina
2023-03-08 17:57               ` Christian König
2023-03-08 19:05                 ` Asahi Lina
2023-03-08 19:12                   ` Christian König
2023-03-08 19:45                     ` Asahi Lina
2023-03-08 20:14                       ` Christian König
2023-03-09  6:30                         ` Asahi Lina
2023-03-09  8:05                           ` Christian König
2023-03-09  9:14                             ` Asahi Lina
2023-03-09 18:50                               ` Faith Ekstrand
2023-03-10  9:16                                 ` Asahi Lina
2023-03-08 12:39     ` Karol Herbst
2023-03-08 13:47       ` Christian König
2023-03-08 14:43         ` Karol Herbst
2023-03-08 15:02           ` Christian König
2023-03-08 15:19             ` Karol Herbst
2023-03-16 13:40               ` Daniel Vetter
2023-04-05 13:40   ` Daniel Vetter
2023-04-05 14:14     ` Christian König
2023-04-05 14:21       ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 11/18] drm/scheduler: Clean up jobs when the scheduler is torn down Asahi Lina
2023-03-08  9:57   ` Maarten Lankhorst
2023-03-08 10:03     ` Christian König
2023-03-08 15:18       ` Asahi Lina
2023-03-08 15:42         ` Christian König
2023-03-08 17:32           ` Asahi Lina
2023-03-08 18:12             ` Christian König
2023-03-08 19:37               ` Asahi Lina
2023-03-09  8:42                 ` Christian König
2023-03-09  9:43                   ` Asahi Lina
2023-03-09 11:47                     ` Christian König
2023-03-09 13:48                       ` Asahi Lina
2023-03-09 19:59                     ` Faith Ekstrand
2023-03-10  9:58                       ` Asahi Lina
2023-03-13 20:11                         ` Faith Ekstrand
2023-03-08 17:39           ` alyssa
2023-03-08 17:44             ` Asahi Lina
2023-03-08 18:13             ` Christian König
2023-04-05 13:52   ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 12/18] rust: drm: sched: Add GPU scheduler abstraction Asahi Lina
2023-04-05 15:43   ` Daniel Vetter
2023-04-05 19:29     ` Daniel Vetter
2023-04-18  8:45       ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 13/18] drm/gem: Add a flag to control whether objects can be exported Asahi Lina
2023-04-05 14:55   ` Daniel Vetter
2023-03-07 14:25 ` [PATCH RFC 14/18] rust: drm: gem: Add set_exportable() method Asahi Lina
2023-03-07 14:25 ` [PATCH RFC 15/18] drm/asahi: Add the Asahi driver UAPI [DO NOT MERGE] Asahi Lina
2023-03-07 15:28   ` Karol Herbst
2023-03-07 14:25 ` [PATCH RFC 16/18] rust: bindings: Bind the Asahi DRM UAPI Asahi Lina
2023-03-07 14:25 ` [PATCH RFC 17/18] rust: macros: Add versions macro Asahi Lina
2023-03-07 16:17 ` [PATCH RFC 00/18] Rust DRM subsystem abstractions (& preview AGX driver) Asahi Lina
     [not found] ` <20230307-rust-drm-v1-18-917ff5bc80a8@asahilina.net>
2023-04-05 14:44   ` [PATCH RFC 18/18] drm/asahi: Add the Asahi driver for Apple AGX GPUs Daniel Vetter
2023-04-06  5:02     ` Asahi Lina
2023-04-06  5:09       ` Asahi Lina
2023-04-06 11:25       ` [Linaro-mm-sig] " Daniel Vetter
2023-04-06 13:32         ` Asahi Lina
2023-04-06 13:54           ` Daniel Vetter
     [not found]   ` <ZC2HtBOaoUAzVCVH@phenom.ffwll.local>
2023-04-06  4:44     ` Asahi Lina
2023-04-06  5:09       ` Asahi Lina
2023-04-06 11:26         ` Daniel Vetter
2023-04-06 10:42       ` [Linaro-mm-sig] " Daniel Vetter
2023-04-06 11:55       ` Daniel Vetter
2023-04-06 13:15         ` Asahi Lina
2023-04-06 13:48           ` Daniel Vetter
2023-04-06 15:19             ` Asahi Lina

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=ZC7pHlLFutMWJF1s@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=airlied@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=alyssa@rosenzweig.io \
    --cc=asahi@lists.linux.dev \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=christian.koenig@amd.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ella@iglunix.org \
    --cc=faith.ekstrand@collabora.com \
    --cc=gary@garyguo.net \
    --cc=jarkko@kernel.org \
    --cc=kherbst@redhat.com \
    --cc=lina@asahilina.net \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=luben.tuikov@amd.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mary@mary.zone \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=mripard@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tzimmermann@suse.de \
    --cc=wedsonaf@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).