Re: [PATCH 03/59] drm/prime: Update docs

Sam Ravnborg
2019-06-17T17:01:43Z

Hi Daniel.

> > > +/**
> > > + * DOC: overview and lifetime rules
> > > + *
> > > + * Similar to GEM global names, PRIME file descriptors are also used to share
> > > + * buffer objects across processes. They offer additional security: as file
> > > + * descriptors must be explicitly sent over UNIX domain sockets to be shared
> > > + * between applications, they can't be guessed like the globally unique GEM
> > > + * names.
> > For a newbie like me the above does not really help to understand what
> > PRIME is.
> > Yes, it is file descriptors used to share buffer objects across
> > processes.
> > But the text say "also used ..", so the main usage of PRIME is something
> > else.
>
> No, what's mean here is that prime file descriptors are used like gem
> global names, for buffer sharing. That's what the "also" means here.
> There's no other use for prime fd than sharing buffers.
>
> Not really clear why this is unclear ...
Re-reading it, things makes a bit more sense to me now.
When I wrote my comment I somehow read it:
"PRIME file descriptors are also used to share ..."
(Notice that I missed the first part).

> > > @@ -728,7 +923,11 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev);
> > > * @dma_buf: dma-buf object to import
> > > *
> > > * This is the implementation of the gem_prime_import functions for GEM drivers
> > > - * using the PRIME helpers.
> > > + * using the PRIME helpers. Drivers can use this as their
> > > + * &drm_driver.gem_prime_import implementation.
> > > + *
> > > + * Drivers must arrange to call drm_prime_gem_destroy() from their
> > > + * &drm_gem_object_funcs.free hook when using this function.
> >
> > Could we here document what function we provide as the default
> > &drm_gem_object_funcs.free hook, which do the drm_prime_gem_destroy()
> > call.
> > I read the above that drivers have to provide their own implementation
> > of a .free hook, and I do not think this is right.
>
> There's no default implementation of the free hook at all, so not sure
> what you mean.
I had expected there was a default implementation.
As there is no such thing my comment it irrelevant.

> > > /**
> > > * @gem_free_object_unlocked: deconstructor for drm_gem_objects
> > > *
> > > - * This is for drivers which are not encumbered with &drm_device.struct_mutex
> > > - * legacy locking schemes. Use this hook instead of @gem_free_object.
> > > + * This is deprecated and should not be used by new drivers. Use
> > > + * &drm_gem_object_funcs.free instead.
> > > + * Compared to @gem_free_object this is not encumbered with
> > > + * &drm_device.struct_mutex legacy locking schemes.
> > > */
> > It is confusing why the above comment refer to another callback.
> > Is this the right wording?
> > * Compared to @gem_free_object_unlocked this is not encumbered with
> > * &drm_device.struct_mutex legacy locking schemes.
> >
> > Do gem_free_object() warrant a similar comment?
>
> I'm confused ... @gem_free_object is exactly the callback I want to refer
> to. Because that one _is_ encumbered with the legacy struct_mutex locking
> scheme. Unlike the .free callback, or the gem_free_object_unlocked
> callback.
I see now, OK.

Thanks for the clarifications.

Sam