Hi

On Thu, Jan 23, 2014 at 9:52 AM, Daniel Vetter daniel.vetter@ffwll.ch wrote:

• This is _not_ a generic interface to create gem objects, but just an interface to make early boot services (like boot splash) with a generic KMS userspace driver possible. Hence it's better to move the documentation for this from the GEM section to the KMS section, next to the creation of framebuffer objects.

• Make it really clear that the returned handle isn't necessarily a GEM object (it can also be e.g. a TTM handle when running on top of vmwgfx).

• Add a paragraph to make it clear that this is just for unaccelarated userspace - gpu drivers need to have their own buffer object creation ioctl which is hardware specific.

Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com> Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

---

Documentation/DocBook/drm.tmpl | 125 ++++++++++++++++++++++------------------- 1 file changed, 68 insertions(+), 57 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ed1d6d289022..9c3fdd59c995 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -830,62 +830,6 @@ char *date;</synopsis> </para> </sect3> <sect3>

•    <title>Dumb GEM Objects</title>
  

•    <para>
  

•      The GEM API doesn't standardize GEM objects creation and leaves it to
  

•      driver-specific ioctls. While not an issue for full-fledged graphics
  

•      stacks that include device-specific userspace components (in libdrm for
  

•      instance), this limit makes DRM-based early boot graphics unnecessarily
  

•      complex.
  

•    </para>
  

•    <para>
  

•      Dumb GEM objects partly alleviate the problem by providing a standard
  

•      API to create dumb buffers suitable for scanout, which can then be used
  

•      to create KMS frame buffers.
  

•    </para>
  

•    <para>
  

•      To support dumb GEM objects drivers must implement the
  

•      <methodname>dumb_create</methodname>,
  

•      <methodname>dumb_destroy</methodname> and
  

•      <methodname>dumb_map_offset</methodname> operations.
  

•    </para>
  

•    <itemizedlist>
  

•      <listitem>
  

•        <synopsis>int (*dumb_create)(struct drm_file *file_priv, struct drm_device *dev,
  

•                 struct drm_mode_create_dumb *args);</synopsis>
  

•        <para>
  

•          The <methodname>dumb_create</methodname> operation creates a GEM
  

•          object suitable for scanout based on the width, height and depth
  

•          from the struct <structname>drm_mode_create_dumb</structname>
  

•          argument. It fills the argument's <structfield>handle</structfield>,
  

•          <structfield>pitch</structfield> and <structfield>size</structfield>
  

•          fields with a handle for the newly created GEM object and its line
  

•          pitch and size in bytes.
  

•        </para>
  

•      </listitem>
  

•      <listitem>
  

•        <synopsis>int (*dumb_destroy)(struct drm_file *file_priv, struct drm_device *dev,
  

•                  uint32_t handle);</synopsis>
  

•        <para>
  

•          The <methodname>dumb_destroy</methodname> operation destroys a dumb
  

•          GEM object created by <methodname>dumb_create</methodname>.
  

•        </para>
  

•      </listitem>
  

•      <listitem>
  

•        <synopsis>int (*dumb_map_offset)(struct drm_file *file_priv, struct drm_device *dev,
  

•                     uint32_t handle, uint64_t *offset);</synopsis>
  

•        <para>
  

•          The <methodname>dumb_map_offset</methodname> operation associates an
  

•          mmap fake offset with the GEM object given by the handle and returns
  

•          it. Drivers must use the
  

•          <function>drm_gem_create_mmap_offset</function> function to
  

•          associate the fake offset as described in
  

•          <xref linkend="drm-gem-objects-mapping"/>.
  

•        </para>
  

•      </listitem>
  

•    </itemizedlist>
  

•  </sect3>
  

•  <sect3>
     <title>Memory Coherency</title>
     <para>
       When mapped to the device or used in a command buffer, backing pages
  

@@ -970,7 +914,9 @@ int max_width, max_height;</synopsis> handle (or a list of memory handles for multi-planar formats) through the <parameter>drm_mode_fb_cmd2</parameter> argument. This document assumes that the driver uses GEM, those handles thus reference GEM

•    objects.
  

•    objects. But drivers are free to use their own backing storage object
  

•   handles, e.g. vmwgfx directly exposes special TTM handles to userspace
  

•   and so expects TTM handles in the create ioctl and not GEM objects.
  

Maybe remove the sentence saying "this document assumes that the driver uses GEM". I don't see where we explicitly do that. Otherwise the patch looks fine.

Thanks David

   </para>
   <para>
     Drivers must first validate the requested frame buffer parameters passed

@@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis> <function>drm_framebuffer_unregister_private</function>. </sect2> <sect2>

•  <title>Dumb GEM Objects</title>
  

•  <para>
  

•   The KMS API doesn't standardize backing storage object creation and
  

•   leaves it to driver-specific ioctls. Furthermore actually creating a
  

•   buffer object even for GEM-based drivers is done through a
  

•   driver-specific ioctl - GEM only has a common userspace interface for
  

•   sharing and destroying objects. While not an issue for full-fledged
  

•   graphics stacks that include device-specific userspace components (in
  

•   libdrm for instance), this limit makes DRM-based early boot graphics
  

•   unnecessarily complex.
  

•  </para>
  

•  <para>
  

•    Dumb objects partly alleviate the problem by providing a standard
  

•    API to create dumb buffers suitable for scanout, which can then be used
  

•    to create KMS frame buffers.
  

•  </para>
  

•  <para>
  

•    To support dumb objects drivers must implement the
  

•    <methodname>dumb_create</methodname>,
  

•    <methodname>dumb_destroy</methodname> and
  

•    <methodname>dumb_map_offset</methodname> operations.
  

•  </para>
  

•  <itemizedlist>
  

•    <listitem>
  

•      <synopsis>int (*dumb_create)(struct drm_file *file_priv, struct drm_device *dev,
  

•               struct drm_mode_create_dumb *args);</synopsis>
  

•      <para>
  

•        The <methodname>dumb_create</methodname> operation creates a driver
  

•       object (GEM or TTM handle) object suitable for scanout based on the
  

•       width, height and depth from the struct
  

•       <structname>drm_mode_create_dumb</structname> argument. It fills the
  

•       argument's <structfield>handle</structfield>,
  

•       <structfield>pitch</structfield> and <structfield>size</structfield>
  

•       fields with a handle for the newly created object and its line
  

•        pitch and size in bytes.
  

•      </para>
  

•    </listitem>
  

•    <listitem>
  

•      <synopsis>int (*dumb_destroy)(struct drm_file *file_priv, struct drm_device *dev,
  

•                uint32_t handle);</synopsis>
  

•      <para>
  

•        The <methodname>dumb_destroy</methodname> operation destroys a dumb
  

•        object created by <methodname>dumb_create</methodname>.
  

•      </para>
  

•    </listitem>
  

•    <listitem>
  

•      <synopsis>int (*dumb_map_offset)(struct drm_file *file_priv, struct drm_device *dev,
  

•                   uint32_t handle, uint64_t *offset);</synopsis>
  

•      <para>
  

•        The <methodname>dumb_map_offset</methodname> operation associates an
  

•        mmap fake offset with the object given by the handle and returns
  

•        it. Drivers must use the
  

•        <function>drm_gem_create_mmap_offset</function> function to
  

•        associate the fake offset as described in
  

•        <xref linkend="drm-gem-objects-mapping"/>.
  

•      </para>
  

•    </listitem>
  

•  </itemizedlist>
  

•  <para>
  

•    Note that dumb objects may not be used for gpu accelaration, as has been
  

•   attempted on some ARM embedded platforms. Such drivers really must have
  

•   a hardware-specific ioctl to allocate suitable objects.
  

•  </para>
  

• </sect2>

• <sect2> <title>Output Polling</title> <synopsis>void (*output_poll_changed)(struct drm_device *dev);</synopsis> <para>

-- 1.8.5.2

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

- This is _not_ a generic interface to create gem objects, but just an interface to make early boot services (like boot splash) with a generic KMS userspace driver possible. Hence it's better to move the documentation for this from the GEM section to the KMS section, next to the creation of framebuffer objects.

- Make it really clear that the returned handle isn't necessarily a GEM object (it can also be e.g. a TTM handle when running on top of vmwgfx).

- Add a paragraph to make it clear that this is just for unaccelarated userspace - gpu drivers need to have their own buffer object creation ioctl which is hardware specific.

v2: Clarify that the documentation doesn't just apply to GEM-based drivers only but is now generally valid, as suggested by David.

Cc: David Herrmann dh.herrmann@gmail.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- Documentation/DocBook/drm.tmpl | 129 ++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 59 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ed1d6d289022..809bf5f7e1c6 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -830,62 +830,6 @@ char *date;</synopsis> </para> </sect3> <sect3> - <title>Dumb GEM Objects</title> - <para> - The GEM API doesn't standardize GEM objects creation and leaves it to - driver-specific ioctls. While not an issue for full-fledged graphics - stacks that include device-specific userspace components (in libdrm for - instance), this limit makes DRM-based early boot graphics unnecessarily - complex. - </para> - <para> - Dumb GEM objects partly alleviate the problem by providing a standard - API to create dumb buffers suitable for scanout, which can then be used - to create KMS frame buffers. - </para> - <para> - To support dumb GEM objects drivers must implement the - <methodname>dumb_create</methodname>, - <methodname>dumb_destroy</methodname> and - <methodname>dumb_map_offset</methodname> operations. - </para> - <itemizedlist> - <listitem> - <synopsis>int (*dumb_create)(struct drm_file *file_priv, struct drm_device *dev, - struct drm_mode_create_dumb *args);</synopsis> - <para> - The <methodname>dumb_create</methodname> operation creates a GEM - object suitable for scanout based on the width, height and depth - from the struct <structname>drm_mode_create_dumb</structname> - argument. It fills the argument's <structfield>handle</structfield>, - <structfield>pitch</structfield> and <structfield>size</structfield> - fields with a handle for the newly created GEM object and its line - pitch and size in bytes. - </para> - </listitem> - <listitem> - <synopsis>int (*dumb_destroy)(struct drm_file *file_priv, struct drm_device *dev, - uint32_t handle);</synopsis> - <para> - The <methodname>dumb_destroy</methodname> operation destroys a dumb - GEM object created by <methodname>dumb_create</methodname>. - </para> - </listitem> - <listitem> - <synopsis>int (*dumb_map_offset)(struct drm_file *file_priv, struct drm_device *dev, - uint32_t handle, uint64_t *offset);</synopsis> - <para> - The <methodname>dumb_map_offset</methodname> operation associates an - mmap fake offset with the GEM object given by the handle and returns - it. Drivers must use the - <function>drm_gem_create_mmap_offset</function> function to - associate the fake offset as described in - <xref linkend="drm-gem-objects-mapping"/>. - </para> - </listitem> - </itemizedlist> - </sect3> - <sect3> <title>Memory Coherency</title> <para> When mapped to the device or used in a command buffer, backing pages @@ -968,9 +912,11 @@ int max_width, max_height;</synopsis> Frame buffers rely on the underneath memory manager for low-level memory operations. When creating a frame buffer applications pass a memory handle (or a list of memory handles for multi-planar formats) through - the <parameter>drm_mode_fb_cmd2</parameter> argument. This document - assumes that the driver uses GEM, those handles thus reference GEM - objects. + the <parameter>drm_mode_fb_cmd2</parameter> argument. For drivers using + GEM as their userspace buffer management interface this would be a GEM + handle. But drivers are free to use their own backing storage object + handles, e.g. vmwgfx directly exposes special TTM handles to userspace + and so expects TTM handles in the create ioctl and not GEM objects. </para> <para> Drivers must first validate the requested frame buffer parameters passed @@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis> <function>drm_framebuffer_unregister_private</function>. </sect2> <sect2> + <title>Dumb GEM Objects</title> + <para> + The KMS API doesn't standardize backing storage object creation and + leaves it to driver-specific ioctls. Furthermore actually creating a + buffer object even for GEM-based drivers is done through a + driver-specific ioctl - GEM only has a common userspace interface for + sharing and destroying objects. While not an issue for full-fledged + graphics stacks that include device-specific userspace components (in + libdrm for instance), this limit makes DRM-based early boot graphics + unnecessarily complex. + </para> + <para> + Dumb objects partly alleviate the problem by providing a standard + API to create dumb buffers suitable for scanout, which can then be used + to create KMS frame buffers. + </para> + <para> + To support dumb objects drivers must implement the + <methodname>dumb_create</methodname>, + <methodname>dumb_destroy</methodname> and + <methodname>dumb_map_offset</methodname> operations. + </para> + <itemizedlist> + <listitem> + <synopsis>int (*dumb_create)(struct drm_file *file_priv, struct drm_device *dev, + struct drm_mode_create_dumb *args);</synopsis> + <para> + The <methodname>dumb_create</methodname> operation creates a driver + object (GEM or TTM handle) object suitable for scanout based on the + width, height and depth from the struct + <structname>drm_mode_create_dumb</structname> argument. It fills the + argument's <structfield>handle</structfield>, + <structfield>pitch</structfield> and <structfield>size</structfield> + fields with a handle for the newly created object and its line + pitch and size in bytes. + </para> + </listitem> + <listitem> + <synopsis>int (*dumb_destroy)(struct drm_file *file_priv, struct drm_device *dev, + uint32_t handle);</synopsis> + <para> + The <methodname>dumb_destroy</methodname> operation destroys a dumb + object created by <methodname>dumb_create</methodname>. + </para> + </listitem> + <listitem> + <synopsis>int (*dumb_map_offset)(struct drm_file *file_priv, struct drm_device *dev, + uint32_t handle, uint64_t *offset);</synopsis> + <para> + The <methodname>dumb_map_offset</methodname> operation associates an + mmap fake offset with the object given by the handle and returns + it. Drivers must use the + <function>drm_gem_create_mmap_offset</function> function to + associate the fake offset as described in + <xref linkend="drm-gem-objects-mapping"/>. + </para> + </listitem> + </itemizedlist> + <para> + Note that dumb objects may not be used for gpu accelaration, as has been + attempted on some ARM embedded platforms. Such drivers really must have + a hardware-specific ioctl to allocate suitable objects. + </para> + </sect2> + <sect2> <title>Output Polling</title> <synopsis>void (*output_poll_changed)(struct drm_device *dev);</synopsis> <para>

- This is _not_ a generic interface to create gem objects, but just an interface to make early boot services (like boot splash) with a generic KMS userspace driver possible. Hence it's better to move the documentation for this from the GEM section to the KMS section, next to the creation of framebuffer objects.

- Make it really clear that the returned handle isn't necessarily a GEM object (it can also be e.g. a TTM handle when running on top of vmwgfx).

- Add a paragraph to make it clear that this is just for unaccelarated userspace - gpu drivers need to have their own buffer object creation ioctl which is hardware specific.

v2: Clarify that the documentation doesn't just apply to GEM-based drivers only but is now generally valid, as suggested by David.

v3: Polish the intro sentence a bit and one s/objects/handles/ for clarification, both suggested by Laurent.

Cc: David Herrmann dh.herrmann@gmail.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- Documentation/DocBook/drm.tmpl | 129 ++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 59 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ed1d6d289022..767318d5ddb6 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -830,62 +830,6 @@ char *date;</synopsis> </para> </sect3> <sect3> - <title>Dumb GEM Objects</title> - <para> - The GEM API doesn't standardize GEM objects creation and leaves it to - driver-specific ioctls. While not an issue for full-fledged graphics - stacks that include device-specific userspace components (in libdrm for - instance), this limit makes DRM-based early boot graphics unnecessarily - complex. - </para> - <para> - Dumb GEM objects partly alleviate the problem by providing a standard - API to create dumb buffers suitable for scanout, which can then be used - to create KMS frame buffers. - </para> - <para> - To support dumb GEM objects drivers must implement the - <methodname>dumb_create</methodname>, - <methodname>dumb_destroy</methodname> and - <methodname>dumb_map_offset</methodname> operations. - </para> - <itemizedlist> - <listitem> - <synopsis>int (*dumb_create)(struct drm_file *file_priv, struct drm_device *dev, - struct drm_mode_create_dumb *args);</synopsis> - <para> - The <methodname>dumb_create</methodname> operation creates a GEM - object suitable for scanout based on the width, height and depth - from the struct <structname>drm_mode_create_dumb</structname> - argument. It fills the argument's <structfield>handle</structfield>, - <structfield>pitch</structfield> and <structfield>size</structfield> - fields with a handle for the newly created GEM object and its line - pitch and size in bytes. - </para> - </listitem> - <listitem> - <synopsis>int (*dumb_destroy)(struct drm_file *file_priv, struct drm_device *dev, - uint32_t handle);</synopsis> - <para> - The <methodname>dumb_destroy</methodname> operation destroys a dumb - GEM object created by <methodname>dumb_create</methodname>. - </para> - </listitem> - <listitem> - <synopsis>int (*dumb_map_offset)(struct drm_file *file_priv, struct drm_device *dev, - uint32_t handle, uint64_t *offset);</synopsis> - <para> - The <methodname>dumb_map_offset</methodname> operation associates an - mmap fake offset with the GEM object given by the handle and returns - it. Drivers must use the - <function>drm_gem_create_mmap_offset</function> function to - associate the fake offset as described in - <xref linkend="drm-gem-objects-mapping"/>. - </para> - </listitem> - </itemizedlist> - </sect3> - <sect3> <title>Memory Coherency</title> <para> When mapped to the device or used in a command buffer, backing pages @@ -968,9 +912,11 @@ int max_width, max_height;</synopsis> Frame buffers rely on the underneath memory manager for low-level memory operations. When creating a frame buffer applications pass a memory handle (or a list of memory handles for multi-planar formats) through - the <parameter>drm_mode_fb_cmd2</parameter> argument. This document - assumes that the driver uses GEM, those handles thus reference GEM - objects. + the <parameter>drm_mode_fb_cmd2</parameter> argument. For drivers using + GEM as their userspace buffer management interface this would be a GEM + handle. Drivers are however free to use their own backing storage object + handles, e.g. vmwgfx directly exposes special TTM handles to userspace + and so expects TTM handles in the create ioctl and not GEM handles. </para> <para> Drivers must first validate the requested frame buffer parameters passed @@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis> <function>drm_framebuffer_unregister_private</function>. </sect2> <sect2> + <title>Dumb GEM Objects</title> + <para> + The KMS API doesn't standardize backing storage object creation and + leaves it to driver-specific ioctls. Furthermore actually creating a + buffer object even for GEM-based drivers is done through a + driver-specific ioctl - GEM only has a common userspace interface for + sharing and destroying objects. While not an issue for full-fledged + graphics stacks that include device-specific userspace components (in + libdrm for instance), this limit makes DRM-based early boot graphics + unnecessarily complex. + </para> + <para> + Dumb objects partly alleviate the problem by providing a standard + API to create dumb buffers suitable for scanout, which can then be used + to create KMS frame buffers. + </para> + <para> + To support dumb objects drivers must implement the + <methodname>dumb_create</methodname>, + <methodname>dumb_destroy</methodname> and + <methodname>dumb_map_offset</methodname> operations. + </para> + <itemizedlist> + <listitem> + <synopsis>int (*dumb_create)(struct drm_file *file_priv, struct drm_device *dev, + struct drm_mode_create_dumb *args);</synopsis> + <para> + The <methodname>dumb_create</methodname> operation creates a driver + object (GEM or TTM handle) object suitable for scanout based on the + width, height and depth from the struct + <structname>drm_mode_create_dumb</structname> argument. It fills the + argument's <structfield>handle</structfield>, + <structfield>pitch</structfield> and <structfield>size</structfield> + fields with a handle for the newly created object and its line + pitch and size in bytes. + </para> + </listitem> + <listitem> + <synopsis>int (*dumb_destroy)(struct drm_file *file_priv, struct drm_device *dev, + uint32_t handle);</synopsis> + <para> + The <methodname>dumb_destroy</methodname> operation destroys a dumb + object created by <methodname>dumb_create</methodname>. + </para> + </listitem> + <listitem> + <synopsis>int (*dumb_map_offset)(struct drm_file *file_priv, struct drm_device *dev, + uint32_t handle, uint64_t *offset);</synopsis> + <para> + The <methodname>dumb_map_offset</methodname> operation associates an + mmap fake offset with the object given by the handle and returns + it. Drivers must use the + <function>drm_gem_create_mmap_offset</function> function to + associate the fake offset as described in + <xref linkend="drm-gem-objects-mapping"/>. + </para> + </listitem> + </itemizedlist> + <para> + Note that dumb objects may not be used for gpu accelaration, as has been + attempted on some ARM embedded platforms. Such drivers really must have + a hardware-specific ioctl to allocate suitable objects. + </para> + </sect2> + <sect2> <title>Output Polling</title> <synopsis>void (*output_poll_changed)(struct drm_device *dev);</synopsis> <para>

On Fri, Jan 24, 2014 at 12:13:11PM +0100, Laurent Pinchart wrote:

Hi Daniel,

Thank you for the patch.

One last round of nitpicking (including a typo fix, which gives me an excuse for a couple more comments :-)).

On Thursday 23 January 2014 14:50:25 Daniel Vetter wrote:

...

• This is _not_ a generic interface to create gem objects, but just an interface to make early boot services (like boot splash) with a generic KMS userspace driver possible. Hence it's better to move the documentation for this from the GEM section to the KMS section, next to the creation of framebuffer objects.

• Make it really clear that the returned handle isn't necessarily a GEM object (it can also be e.g. a TTM handle when running on top of vmwgfx).

• Add a paragraph to make it clear that this is just for unaccelarated userspace - gpu drivers need to have their own buffer object creation ioctl which is hardware specific.

v2: Clarify that the documentation doesn't just apply to GEM-based drivers only but is now generally valid, as suggested by David.

v3: Polish the intro sentence a bit and one s/objects/handles/ for clarification, both suggested by Laurent.

Cc: David Herrmann dh.herrmann@gmail.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

---

Documentation/DocBook/drm.tmpl | 129 ++++++++++++++++++++------------------ 1 file changed, 70 insertions(+), 59 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ed1d6d289022..767318d5ddb6 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl

[snip]

...

@@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis> <function>drm_framebuffer_unregister_private</function>. </sect2> <sect2>

•  <title>Dumb GEM Objects</title>
  

What about calling this "Dumb Memory Objects" (or something similar), as they're not specific to GEM ?

I've gone through the entire section to remove all GEM-specific language and missed the title. That's some good fail right there ;-) I'm going with buffer objects, which I think is the most established language for gfx memory management - GL also uses it in specs.

[snip]

...

• attempted on some ARM embedded platforms. Such drivers really must have
• a hardware-specific ioctl to allocate suitable objects.

What about s/objects/memory objects/ ? "object" alone is rather vague for people not too familiar with DRM/KMS.

Also opted for buffer object here. Other suggestions applies unchaged.

Thanks, Daniel

Am 24.01.2014 17:58, schrieb Daniel Vetter:

Just two typos ('said' my spellchecker...;-)

Regards, Dieter

• This is _not_ a generic interface to create gem objects, but just an interface to make early boot services (like boot splash) with a generic KMS userspace driver possible. Hence it's better to move the documentation for this from the GEM section to the KMS section, next to the creation of framebuffer objects.

• Make it really clear that the returned handle isn't necessarily a GEM object (it can also be e.g. a TTM handle when running on top of vmwgfx).

• Add a paragraph to make it clear that this is just for unaccelarated userspace - gpu drivers need to have their own buffer object creation ioctl which is hardware specific.

v2: Clarify that the documentation doesn't just apply to GEM-based drivers only but is now generally valid, as suggested by David.

v3: Polish the intro sentence a bit and one s/objects/handles/ for clarification, both suggested by Laurent.

v4: More text polish from Laurent's review.

Cc: David Herrmann dh.herrmann@gmail.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

---

Documentation/DocBook/drm.tmpl | 131 ++++++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 60 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ed1d6d289022..67cfe184749c 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -830,62 +830,6 @@ char *date;</synopsis> </para> </sect3> <sect3>

•    <title>Dumb GEM Objects</title>
  

•    <para>
  

•      The GEM API doesn't standardize GEM objects creation and 
  

leaves it to

•      driver-specific ioctls. While not an issue for full-fledged 
  

graphics

•      stacks that include device-specific userspace components
  

(in libdrm for

•      instance), this limit makes DRM-based early boot graphics
  

unnecessarily

•      complex.
  

•    </para>
  

•    <para>
  

•      Dumb GEM objects partly alleviate the problem by providing a 
  

standard

•      API to create dumb buffers suitable for scanout, which can
  

then be used

•      to create KMS frame buffers.
  

•    </para>
  

•    <para>
  

•      To support dumb GEM objects drivers must implement the
  

•      <methodname>dumb_create</methodname>,
  

•      <methodname>dumb_destroy</methodname> and
  

•      <methodname>dumb_map_offset</methodname> operations.
  

•    </para>
  

•    <itemizedlist>
  

•      <listitem>
  

•        <synopsis>int (*dumb_create)(struct drm_file *file_priv,
  

struct drm_device *dev,

•                 struct drm_mode_create_dumb *args);</synopsis>
  

•        <para>
  

•          The <methodname>dumb_create</methodname> operation 
  

creates a GEM

•          object suitable for scanout based on the width, height 
  

and depth

•          from the struct 
  

<structname>drm_mode_create_dumb</structname>

•          argument. It fills the argument's
  

<structfield>handle</structfield>,

•          <structfield>pitch</structfield> and
  

<structfield>size</structfield>

•          fields with a handle for the newly created GEM object
  

and its line

•          pitch and size in bytes.
  

•        </para>
  

•      </listitem>
  

•      <listitem>
  

•        <synopsis>int (*dumb_destroy)(struct drm_file *file_priv,
  

struct drm_device *dev,

•                  uint32_t handle);</synopsis>
  

•        <para>
  

•          The <methodname>dumb_destroy</methodname> operation
  

destroys a dumb

•          GEM object created by 
  

<methodname>dumb_create</methodname>.

•        </para>
  

•      </listitem>
  

•      <listitem>
  

•        <synopsis>int (*dumb_map_offset)(struct drm_file
  

*file_priv, struct drm_device *dev,

•                     uint32_t handle, uint64_t 
  

*offset);</synopsis>

•        <para>
  

•          The <methodname>dumb_map_offset</methodname> operation
  

associates an

•          mmap fake offset with the GEM object given by the
  

handle and returns

•          it. Drivers must use the
  

•          <function>drm_gem_create_mmap_offset</function> function 
  

to

•          associate the fake offset as described in
  

•          <xref linkend="drm-gem-objects-mapping"/>.
  

•        </para>
  

•      </listitem>
  

•    </itemizedlist>
  

•  </sect3>
  

•  <sect3>
     <title>Memory Coherency</title>
     <para>
       When mapped to the device or used in a command buffer, 
  

backing pages @@ -968,9 +912,11 @@ int max_width, max_height;</synopsis> Frame buffers rely on the underneath memory manager for low-level memory operations. When creating a frame buffer applications pass a memory handle (or a list of memory handles for multi-planar formats) through

•    the <parameter>drm_mode_fb_cmd2</parameter> argument. This 
  

document

•    assumes that the driver uses GEM, those handles thus reference 
  

GEM

•    objects.
  

• the <parameter>drm_mode_fb_cmd2</parameter> argument. For drivers

using

• GEM as their userspace buffer management interface this would be a

GEM

• handle. Drivers are however free to use their own backing storage

object

• handles, e.g. vmwgfx directly exposes special TTM handles to

userspace

• and so expects TTM handles in the create ioctl and not GEM handles.

  </para> <para> Drivers must first validate the requested frame buffer

parameters passed @@ -992,7 +938,7 @@ int max_width, max_height;</synopsis> </para>

   <para>

• The initailization of the new framebuffer instance is finalized with

a

• The initilization of the new framebuffer instance is finalized with a

=> initialization ;-)

call to <function>drm_framebuffer_init</function> which takes a pointer to DRM frame buffer operations (struct <structname>drm_framebuffer_funcs</structname>). Note that this function @@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis> <function>drm_framebuffer_unregister_private</function>. </sect2> <sect2>

•  <title>Dumb Buffer Objects</title>
  

•  <para>
  

• The KMS API doesn't standardize backing storage object creation and
• leaves it to driver-specific ioctls. Furthermore actually creating a
• buffer object even for GEM-based drivers is done through a
• driver-specific ioctl - GEM only has a common userspace interface for
• sharing and destroying objects. While not an issue for full-fledged
• graphics stacks that include device-specific userspace components (in
• libdrm for instance), this limit makes DRM-based early boot graphics
• unnecessarily complex.

•  </para>
  

•  <para>
  

•    Dumb objects partly alleviate the problem by providing a 
  

standard

•    API to create dumb buffers suitable for scanout, which can 
  

then be used

•    to create KMS frame buffers.
  

•  </para>
  

•  <para>
  

•    To support dumb objects drivers must implement the
  

•    <methodname>dumb_create</methodname>,
  

•    <methodname>dumb_destroy</methodname> and
  

•    <methodname>dumb_map_offset</methodname> operations.
  

•  </para>
  

•  <itemizedlist>
  

•    <listitem>
  

•      <synopsis>int (*dumb_create)(struct drm_file *file_priv,
  

struct drm_device *dev,

•               struct drm_mode_create_dumb *args);</synopsis>
  

•      <para>
  

•        The <methodname>dumb_create</methodname> operation creates 
  

a driver

•    object (GEM or TTM handle) suitable for scanout based on the
  

•    width, height and depth from the struct
  

•    <structname>drm_mode_create_dumb</structname> argument. It fills 
  

the

•    argument's <structfield>handle</structfield>,
  

•    <structfield>pitch</structfield> and 
  

<structfield>size</structfield>

•    fields with a handle for the newly created object and its line
  

•        pitch and size in bytes.
  

•      </para>
  

•    </listitem>
  

•    <listitem>
  

•      <synopsis>int (*dumb_destroy)(struct drm_file *file_priv,
  

struct drm_device *dev,

•                uint32_t handle);</synopsis>
  

•      <para>
  

•        The <methodname>dumb_destroy</methodname> operation 
  

destroys a dumb

•        object created by <methodname>dumb_create</methodname>.
  

•      </para>
  

•    </listitem>
  

•    <listitem>
  

•      <synopsis>int (*dumb_map_offset)(struct drm_file
  

*file_priv, struct drm_device *dev,

•                   uint32_t handle, uint64_t *offset);</synopsis>
  

•      <para>
  

•        The <methodname>dumb_map_offset</methodname> operation
  

associates an

•        mmap fake offset with the object given by the handle and 
  

returns

•        it. Drivers must use the
  

•        <function>drm_gem_create_mmap_offset</function> function 
  

to

•        associate the fake offset as described in
  

•        <xref linkend="drm-gem-objects-mapping"/>.
  

•      </para>
  

•    </listitem>
  

•  </itemizedlist>
  

•  <para>
  

•    Note that dumb objects may not be used for gpu accelaration,
  

=> acceleration

as has been

• attempted on some ARM embedded platforms. Such drivers really must

have

• a hardware-specific ioctl to allocate suitable buffer objects.

•  </para>
  

• </sect2>

• <sect2> <title>Output Polling</title> <synopsis>void (*output_poll_changed)(struct drm_device

*dev);</synopsis> <para>

Hi Daniel,

Thank you for the patch.

On Thursday 23 January 2014 09:52:26 Daniel Vetter wrote:

• This is _not_ a generic interface to create gem objects, but just an interface to make early boot services (like boot splash) with a generic KMS userspace driver possible. Hence it's better to move the documentation for this from the GEM section to the KMS section, next to the creation of framebuffer objects.

• Make it really clear that the returned handle isn't necessarily a GEM object (it can also be e.g. a TTM handle when running on top of vmwgfx).

• Add a paragraph to make it clear that this is just for unaccelarated userspace - gpu drivers need to have their own buffer object creation ioctl which is hardware specific.

Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com> Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

---

Documentation/DocBook/drm.tmpl | 125 ++++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 57 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ed1d6d289022..9c3fdd59c995 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -830,62 +830,6 @@ char *date;</synopsis> </para> </sect3> <sect3>

•    <title>Dumb GEM Objects</title>
  

•    <para>
  

•      The GEM API doesn't standardize GEM objects creation and leaves
  

it to - driver-specific ioctls. While not an issue for full-fledged graphics - stacks that include device-specific userspace components (in libdrm for - instance), this limit makes DRM-based early boot graphics unnecessarily - complex.

•    </para>
  

•    <para>
  

•      Dumb GEM objects partly alleviate the problem by providing a
  

standard - API to create dumb buffers suitable for scanout, which can then be used - to create KMS frame buffers.

•    </para>
  

•    <para>
  

•      To support dumb GEM objects drivers must implement the
  

•      <methodname>dumb_create</methodname>,
  

•      <methodname>dumb_destroy</methodname> and
  

•      <methodname>dumb_map_offset</methodname> operations.
  

•    </para>
  

•    <itemizedlist>
  

•      <listitem>
  

•        <synopsis>int (*dumb_create)(struct drm_file *file_priv, struct
  

drm_device *dev, - struct drm_mode_create_dumb *args);</synopsis> - <para>

•          The <methodname>dumb_create</methodname> operation creates a
  

GEM - object suitable for scanout based on the width, height and depth - from the struct <structname>drm_mode_create_dumb</structname> - argument. It fills the argument's <structfield>handle</structfield>, - <structfield>pitch</structfield> and <structfield>size</structfield> - fields with a handle for the newly created GEM object and its line

•          pitch and size in bytes.
  

•        </para>
  

•      </listitem>
  

•      <listitem>
  

•        <synopsis>int (*dumb_destroy)(struct drm_file *file_priv,
  

struct drm_device *dev, - uint32_t handle);</synopsis>

•        <para>
  

•          The <methodname>dumb_destroy</methodname> operation destroys
  

a dumb - GEM object created by <methodname>dumb_create</methodname>. - </para>

•      </listitem>
  

•      <listitem>
  

•        <synopsis>int (*dumb_map_offset)(struct drm_file *file_priv,
  

struct drm_device *dev, - uint32_t handle, uint64_t *offset);</synopsis> - <para>

•          The <methodname>dumb_map_offset</methodname> operation
  

associates an - mmap fake offset with the GEM object given by the handle and returns - it. Drivers must use the

•          <function>drm_gem_create_mmap_offset</function> function to
  

•          associate the fake offset as described in
  

•          <xref linkend="drm-gem-objects-mapping"/>.
  

•        </para>
  

•      </listitem>
  

•    </itemizedlist>
  

•  </sect3>
  

•  <sect3>
     <title>Memory Coherency</title>
     <para>
       When mapped to the device or used in a command buffer, backing
  

pages @@ -970,7 +914,9 @@ int max_width, max_height;</synopsis> handle (or a list of memory handles for multi-planar formats) through the <parameter>drm_mode_fb_cmd2</parameter> argument. This document assumes that the driver uses GEM, those handles thus reference GEM - objects.

•    objects. But drivers are free to use their own backing storage
  

object

• handles, e.g. vmwgfx directly exposes special TTM handles to userspace
• and so expects TTM handles in the create ioctl and not GEM objects.

Nitpicking, I would say

"Drivers are however free to use their own backing storage object handles, e.g. vmwgfx directly exposes special TTM handles to userspace and so expects TTM handles in the create ioctl, not GEM objects."

   <para>
     Drivers must first validate the requested frame buffer parameters

passed @@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis> <function>drm_framebuffer_unregister_private</function>. </sect2> <sect2>

•  <title>Dumb GEM Objects</title>
  

•  <para>
  

• The KMS API doesn't standardize backing storage object creation and

Strictly speaking isn't it the DRM API that's responsible for memory management, not the KMS API ?

• leaves it to driver-specific ioctls. Furthermore actually creating a
• buffer object even for GEM-based drivers is done through a
• driver-specific ioctl - GEM only has a common userspace interface for
• sharing and destroying objects. While not an issue for full-fledged
• graphics stacks that include device-specific userspace components (in
• libdrm for instance), this limit makes DRM-based early boot graphics
• unnecessarily complex.

•  </para>
  

This feels a bit out of place, can't we leave the section where it was ?

•  <para>
  

•    Dumb objects partly alleviate the problem by providing a standard
  

•    API to create dumb buffers suitable for scanout, which can then be
  

used + to create KMS frame buffers.

•  </para>
  

•  <para>
  

•    To support dumb objects drivers must implement the
  

•    <methodname>dumb_create</methodname>,
  

•    <methodname>dumb_destroy</methodname> and
  

•    <methodname>dumb_map_offset</methodname> operations.
  

•  </para>
  

•  <itemizedlist>
  

•    <listitem>
  

•      <synopsis>int (*dumb_create)(struct drm_file *file_priv, struct
  

drm_device *dev, + struct drm_mode_create_dumb *args);</synopsis> + <para>

•        The <methodname>dumb_create</methodname> operation creates a
  

driver + object (GEM or TTM handle) object suitable for scanout based on the + width, height and depth from the struct

•    <structname>drm_mode_create_dumb</structname> argument. It fills the
  

•    argument's <structfield>handle</structfield>,
  

•    <structfield>pitch</structfield> and <structfield>size</structfield>
  

•    fields with a handle for the newly created object and its line
  

•        pitch and size in bytes.
  

•      </para>
  

•    </listitem>
  

•    <listitem>
  

•      <synopsis>int (*dumb_destroy)(struct drm_file *file_priv, struct
  

drm_device *dev, + uint32_t handle);</synopsis>

•      <para>
  

•        The <methodname>dumb_destroy</methodname> operation destroys a
  

dumb + object created by <methodname>dumb_create</methodname>. + </para>

•    </listitem>
  

•    <listitem>
  

•      <synopsis>int (*dumb_map_offset)(struct drm_file *file_priv,
  

struct drm_device *dev, + uint32_t handle, uint64_t *offset);</synopsis> + <para>

•        The <methodname>dumb_map_offset</methodname> operation
  

associates an + mmap fake offset with the object given by the handle and returns + it. Drivers must use the

•        <function>drm_gem_create_mmap_offset</function> function to
  

•        associate the fake offset as described in
  

•        <xref linkend="drm-gem-objects-mapping"/>.
  

•      </para>
  

•    </listitem>
  

•  </itemizedlist>
  

•  <para>
  

•    Note that dumb objects may not be used for gpu accelaration, as has
  

been + attempted on some ARM embedded platforms. Such drivers really must have + a hardware-specific ioctl to allocate suitable objects.

•  </para>
  

• </sect2>

• <sect2> <title>Output Polling</title> <synopsis>void (*output_poll_changed)(struct drm_device

*dev);</synopsis> <para>

Hi Daniel,

On Thursday 23 January 2014 13:47:31 Daniel Vetter wrote:

On Thu, Jan 23, 2014 at 12:21:42PM +0100, Laurent Pinchart wrote:

...

On Thursday 23 January 2014 09:52:26 Daniel Vetter wrote:

...

• This is _not_ a generic interface to create gem objects, but just an interface to make early boot services (like boot splash) with a generic KMS userspace driver possible. Hence it's better to move the documentation for this from the GEM section to the KMS section, next to the creation of framebuffer objects.

• Make it really clear that the returned handle isn't necessarily a GEM object (it can also be e.g. a TTM handle when running on top of vmwgfx).

• Add a paragraph to make it clear that this is just for unaccelarated userspace - gpu drivers need to have their own buffer object creation ioctl which is hardware specific.

Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com> Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

---

Documentation/DocBook/drm.tmpl | 125 ++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 57 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ed1d6d289022..9c3fdd59c995 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl

[snip]

...

...

pages @@ -970,7 +914,9 @@ int max_width, max_height;</synopsis> handle (or a list of memory handles for multi-planar formats) through the <parameter>drm_mode_fb_cmd2</parameter> argument. This document assumes that the driver uses GEM, those handles thus reference GEM

•    objects.
  

•    objects. But drivers are free to use their own backing storage
  

object

• handles, e.g. vmwgfx directly exposes special TTM handles to

userspace

• and so expects TTM handles in the create ioctl and not GEM objects.

Nitpicking, I would say

"Drivers are however free to use their own backing storage object handles, e.g. vmwgfx directly exposes special TTM handles to userspace and so expects TTM handles in the create ioctl, not GEM objects."

I've already adjusted this a bit but haven't yet sent out the new version of the patch. Slightly different wording now.

OK. Please also see my reply to David.

...

...

   <para>
     Drivers must first validate the requested frame buffer
     parameters passed

@@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis> <function>drm_framebuffer_unregister_private</function>. </sect2> <sect2>

•  <title>Dumb GEM Objects</title>
  

•  <para>
  

• The KMS API doesn't standardize backing storage object creation and

Strictly speaking isn't it the DRM API that's responsible for memory management, not the KMS API ?

The driver's private api is responsible for memory management, but the crucial thing here is that the KMS ioctls don't mandate anything specific (beyong that it needs to use uint32_t for handles).

Sure, but my point was that I believe memory management is the responsibility of DRM, not KMS. It of course needs to be driver-specific.

...

...

• leaves it to driver-specific ioctls. Furthermore actually creating a
• buffer object even for GEM-based drivers is done through a
• driver-specific ioctl - GEM only has a common userspace interface for
• sharing and destroying objects. While not an issue for full-fledged
• graphics stacks that include device-specific userspace components (in
• libdrm for instance), this limit makes DRM-based early boot graphics
• unnecessarily complex.

•  </para>
  

This feels a bit out of place, can't we leave the section where it was ?

Imo the justification for why we have the dumb ioctls should be here. And I wanted to mention both that KMS doesn't mandate a particular bo interface like GEM and that on top GEM wouldn't even provide a common allocation function anyway.

I agree that a justification here is a good idea, but I would just add one paragraph that references the dumb GEM objects section instead of scattering GEM documentation around.

But besides that I think there's some room for improvement in the GEM section to clarify what is the actual core interfaces, what is more helper library in nature and what in GEM is more just a common design pattern for driver ioctls but not specified in a mandatory way anywhere. E.g. atm all drivers which implement a GEM interface (radeon, i915, ...) have a mostly implicitly synchronizing buffer access interface, but there's nothing in GEM mandating this. Or the usual confusing between TTM directly exposed to userspace and TTM hidden behind a GEM-based ioctl interface.

I agree, the GEM section should be improved, and TTM documentation would be nice as well. I'm lacking time though (as well as knowledge about TTM).

Hi Daniel,

On Thursday 23 January 2014 14:46:40 Daniel Vetter wrote:

On Thu, Jan 23, 2014 at 01:56:51PM +0100, Laurent Pinchart wrote:

...

...

...

...

   <para>

     Drivers must first validate the requested frame buffer
     parameters passed

@@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis>

<function>drm_framebuffer_unregister_private</function>.

 </sect2>
 <sect2>

•  <title>Dumb GEM Objects</title>
  

•  <para>
  

• The KMS API doesn't standardize backing storage object creation

and

Strictly speaking isn't it the DRM API that's responsible for memory management, not the KMS API ?

The driver's private api is responsible for memory management, but the crucial thing here is that the KMS ioctls don't mandate anything specific (beyong that it needs to use uint32_t for handles).

Sure, but my point was that I believe memory management is the responsibility of DRM, not KMS. It of course needs to be driver-specific.

Well imo the dumb ioctls are part of kms. DRM itself doesn't have any memory management interfaces (at least if you ignore all the horror stories around legacy ums/dri1 drivers). My reasons are:

• If you implement a kms driver you really should implement the dumb interfaces. Even when you have almost no memory management like the simpledrm driver.
• If you have a driver with memory management and command submission but no KMS, there's no reason at all to implement the dumb interfaces.
• ARM people abused dumb buffers for accelaration "because it worked", so imo moving it's documentation as far away as possible from the memory management section is a feature.

So the dumb stuff is a KMS interface to abstract away the lowest common denominator between all kms drivers. Actually memory manament needs a real interface, and is obviously separate.

OK. Those ioctls are not available on render nodes, which I suppose is another sign that they're KMS ioctls.

What about the device-specific memory allocation ioctls though ? Are they considered part of DRM or KMS ?

...

...

...

...

• leaves it to driver-specific ioctls. Furthermore actually

creating a

• buffer object even for GEM-based drivers is done through a
• driver-specific ioctl - GEM only has a common userspace

interface for

• sharing and destroying objects. While not an issue for

full-fledged

• graphics stacks that include device-specific userspace

components (in

• libdrm for instance), this limit makes DRM-based early boot

graphics

• unnecessarily complex.

•  </para>
  

This feels a bit out of place, can't we leave the section where it was ?

Imo the justification for why we have the dumb ioctls should be here. And I wanted to mention both that KMS doesn't mandate a particular bo interface like GEM and that on top GEM wouldn't even provide a common allocation function anyway.

I agree that a justification here is a good idea, but I would just add one paragraph that references the dumb GEM objects section instead of scattering GEM documentation around.

I've pretty much removed all mention of dumb gem objects ;-) There's one mention of dumb_create left in the GEM/memory management section, but that one is just an example for the lifetime and reference counting rules. So not relevant.

Fair enough. I'm fine with that.

...

...

But besides that I think there's some room for improvement in the GEM section to clarify what is the actual core interfaces, what is more helper library in nature and what in GEM is more just a common design pattern for driver ioctls but not specified in a mandatory way anywhere. E.g. atm all drivers which implement a GEM interface (radeon, i915, ...) have a mostly implicitly synchronizing buffer access interface, but there's nothing in GEM mandating this. Or the usual confusing between TTM directly exposed to userspace and TTM hidden behind a GEM-based ioctl interface.

I agree, the GEM section should be improved, and TTM documentation would be nice as well. I'm lacking time though (as well as knowledge about TTM).

I have a few ideas, but I think I'll postpone this until I get around to documenting the i915 GEM code a bit. Having a concrete driver to talk about should help greatly to separate common concepts from artifacts of the i915 implementation. I guess that review will also lead to some abi cleanups to remove i915-ism from core gem.

Soudns good to me.

BTW, while you're at it, could you squash this typo fix in ?

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ed1d6d2..1e6579f 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -992,7 +992,7 @@ int max_width, max_height;</synopsis> </para>

<para> - The initailization of the new framebuffer instance is finalized with a + The initialization of the new framebuffer instance is finalized with a call to <function>drm_framebuffer_init</function> which takes a pointer to DRM frame buffer operations (struct <structname>drm_framebuffer_funcs</structname>). Note that this function

Hi

On Thu, Jan 23, 2014 at 9:52 AM, Daniel Vetter daniel.vetter@ffwll.ch wrote:

Fairly incomplete, but at least a start.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

Documentation/DocBook/drm.tmpl | 6 +++- drivers/gpu/drm/drm_gem.c | 63 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 9c3fdd59c995..0660bae6928f 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -868,7 +868,11 @@ char *date;</synopsis> abstracted from the client in libdrm. </para> </sect3>

• </sect2>

•  <sect2>
  

•    <title>GEM Function Reference</title>
  

+!Edrivers/gpu/drm/drm_gem.c

•  </sect2>
  

•  </sect2>
  

  </sect1>

  <!-- Internals: mode setting -->

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 5bbad873c798..2136052ccee1 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -85,9 +85,9 @@ #endif

/**

• • Initialize the GEM device fields

• • drm_gem_init - Initialize the GEM device fields
• • @dev: drm_devic structure to initialize
  */

int drm_gem_init(struct drm_device *dev) { @@ -120,6 +120,11 @@ drm_gem_destroy(struct drm_device *dev) }

/**

• • drm_gem_object_init - initialize an allocated shmem-backed GEM object
• • @dev: drm_device the object should be initialized for
• • @obj: drm_gem_object to initialize
• • @size: object size
• • Initialize an already allocated GEM object of the specified size with
  • shmfs backing store.
  */

@@ -141,6 +146,11 @@ int drm_gem_object_init(struct drm_device *dev, EXPORT_SYMBOL(drm_gem_object_init);

/**

• • drm_gem_object_init - initialize an allocated private GEM object
• • @dev: drm_device the object should be initialized for
• • @obj: drm_gem_object to initialize
• • @size: object size
• • Initialize an already allocated GEM object of the specified size with
  • no GEM provided backing store. Instead the caller is responsible for
  • backing the object and handling it.

@@ -176,6 +186,9 @@ drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) }

/**

• • drm_gem_object_free - release resources bound to userspace handles
• • @obj: GEM object to clean up.
• • Called after the last handle to the object has been closed
  • Removes any name for the object. Note that this must be

@@ -225,7 +238,12 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) }

/**

• • Removes the mapping from handle to filp for this object.

• • drm_gem_handle_delete - deletes the given file-private handle
• • @filp: drm file-private structure to use for the handle look up
• • @handle: userspace handle to delete
• • Removes the GEM handle from the @filp lookup table and if this is the last
• • handle also cleans up linked resources like GEM names.
  */

int drm_gem_handle_delete(struct drm_file *filp, u32 handle) @@ -270,6 +288,9 @@ EXPORT_SYMBOL(drm_gem_handle_delete);

/**

• drm_gem_dumb_destroy - dumb fb callback helper for gem based drivers

• • @file: drm file-private structure to remove the dumb handle from
• • @dev: corresponding drm_device
• • @handle: the dumb handle to remove
  • This implements the ->dumb_destroy kms driver callback for drivers which use
  • gem to manage their backing storage.

@@ -284,6 +305,9 @@ EXPORT_SYMBOL(drm_gem_dumb_destroy);

/**

• drm_gem_handle_create_tail - internal functions to create a handle

• • @file_priv: drm file-private structure to register the handle for
• • @obj: object to register
• • @handlep: pionter to return the created handle to the caller
  • This expects the dev->object_name_lock to be held already and will drop it
  • before returning. Used to avoid races in establishing new handles when

@@ -336,6 +360,11 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, }

/**

• • gem_handle_create - create a gem handle for an object
• • @file_priv: drm file-private structure to register the handle for
• • @obj: object to register
• • @handlep: pionter to return the created handle to the caller
• • Create a handle for this object. This adds a handle reference
  • to the object, which includes a regular reference count. Callers
  • will likely want to dereference the object afterwards.

@@ -536,6 +565,11 @@ drm_gem_object_lookup(struct drm_device *dev, struct drm_file *filp, EXPORT_SYMBOL(drm_gem_object_lookup);

/**

• • drm_gem_close_ioctl - implementation of the GEM_CLOSE ioctl
• • @dev: drm_device
• • @data: ioctl data
• • @file_priv: drm file-private structure
• • Releases the handle to an mm object.
  */

int @@ -554,6 +588,11 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, }

/**

• • drm_gem_flink_ioctl - implementation of the GEM_FLINK ioctl
• • @dev: drm_device
• • @data: ioctl data
• • @file_priv: drm file-private structure
• • Create a global name for an object, returning the name.
  • Note that the name does not hold a reference; when the object

@@ -601,6 +640,11 @@ err: }

/**

• • drm_gem_open - implementation of the GEM_OPEN ioctl
• • @dev: drm_device
• • @data: ioctl data
• • @file_priv: drm file-private structure
• • Open an object using the global name, returning a handle and the size.
  • This handle (of course) holds a reference to the object, so the object

@@ -640,6 +684,10 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data, }

/**

• • gem_gem_open - initalizes GEM file-private structures at devnode open time
• • @dev: drm_device which is being opened by userspace
• • @file_private: drm file-private structure to set up
• • Called at device open time, sets up the structure for handling refcounting
  • of mm objects.
  */

@@ -650,7 +698,7 @@ drm_gem_open(struct drm_device *dev, struct drm_file *file_private) spin_lock_init(&file_private->table_lock); }

-/** +/*

• Called at device close to release the file's
• handle references on objects.

*/ @@ -674,6 +722,10 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) }

/**

• • drm_gem_release - release file-private GEM resources
• • @dev: drm_device which is being closed by userspace
• • @file_private: drm file-private structure to clean up
• • Called at close time when the filp is going away.
  • Releases any remaining references on objects by this filp.

@@ -697,6 +749,9 @@ drm_gem_object_release(struct drm_gem_object *obj) EXPORT_SYMBOL(drm_gem_object_release);

drm_gem_object_release() is still missing. Lets at least add a kernel-doc entry ala:

/** * drm_gem_object_release() - destroy GEM object * @obj: drm-object to release * * This is the counter-part to drm_gem_object_init() and drm_gem_private_object_init() and * must be called by a driver in its gem_free_object() callback to release any gem-internal * resources of the GEM-bo. */

/**

• • drm_gem_object_free - free a GEM object
• • @kref: kref of the object to free
• • Called after the last reference to the object has been lost.
  • Must be called holding struct_ mutex

drm_gem_object_free() is internal to drm_gem.c. We only export it because our object_unreference() helper is "static inline" in the header. I don't think we should include this in the kernel-doc. No-one should call this directly (same drm_gem_object_release_handle()).

Thanks David

-- 1.8.5.2

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

Hi

On Thu, Jan 23, 2014 at 9:52 AM, Daniel Vetter daniel.vetter@ffwll.ch wrote:

For giant hilarity the DocBook reference overview is only generated when in a level 2 section, not in a level 3 section. So we need to move this up a bit as a side-by-side section to the main PRIME documentation.

Whatever.

I tried digging through scripts/kernel-doc but.. ugh.. it's perl! No idea how to fix that. But sect2 seems fine as the whole section is PRIME-related.

Thanks David

To have a complete set of references add the missing kerneldoc for all functions exported to modules with the exception of the file private init/destry functions - drivers have no business calling those, so let's just drop the EXPORT_SYMBOL instead.

Also reflow the function parameters to align correctly and break at 80 chars - my OCD couldn't stand them while writing the kerneldoc ;-)

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

Documentation/DocBook/drm.tmpl | 6 ++- drivers/gpu/drm/drm_prime.c | 110 +++++++++++++++++++++++++++++++++-------- 2 files changed, 94 insertions(+), 22 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 0cc1d85d04de..07abe52b1176 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -898,10 +898,14 @@ struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev, </para> </sect3> <sect3>

•      <title>PRIME Helper Functions Reference</title>
  

•      <title>PRIME Helper Functions</title>
  

!Pdrivers/gpu/drm/drm_prime.c PRIME Helpers </sect3> </sect2>

•  <sect2>
  

•   <title>PRIME Function References</title>
  

+!Edrivers/gpu/drm/drm_prime.c

•  </sect2>
  

  </sect1>

  <!-- Internals: mode setting -->

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 56805c39c906..f1437b6c8dbf 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -68,7 +68,8 @@ struct drm_prime_attachment { enum dma_data_direction dir; };

-static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle) +static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,

•                               struct dma_buf *dma_buf, uint32_t handle)
  

{ struct drm_prime_member *member;

@@ -174,7 +175,7 @@ void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpr }

static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,

•           enum dma_data_direction dir)
  

•                                       enum dma_data_direction dir)
  

{ struct drm_prime_attachment *prime_attach = attach->priv; struct drm_gem_object *obj = attach->dmabuf->priv; @@ -211,11 +212,19 @@ static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, }

static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,

•           struct sg_table *sgt, enum dma_data_direction dir)
  

•                             struct sg_table *sgt,
  

•                             enum dma_data_direction dir)
  

{ /* nothing to be done here */ }

+/**

• • drm_gem_dmabuf_release - dma_buf release implementation for GEM
• • @dma_buf: buffer to be released
• • Generic release function for dma_bufs exported as PRIME buffers. GEM drivers
• • must use this in their dma_buf ops structure as the release callback.
• */

void drm_gem_dmabuf_release(struct dma_buf *dma_buf) { struct drm_gem_object *obj = dma_buf->priv; @@ -242,30 +251,30 @@ static void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) }

static void *drm_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,

•           unsigned long page_num)
  

•                                   unsigned long page_num)
  

{ return NULL; }

static void drm_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,

•           unsigned long page_num, void *addr)
  

•                                    unsigned long page_num, void *addr)
  

{

} static void *drm_gem_dmabuf_kmap(struct dma_buf *dma_buf,

•           unsigned long page_num)
  

•                            unsigned long page_num)
  

{ return NULL; }

static void drm_gem_dmabuf_kunmap(struct dma_buf *dma_buf,

•           unsigned long page_num, void *addr)
  

•                             unsigned long page_num, void *addr)
  

{

}

static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,

•           struct vm_area_struct *vma)
  

•                          struct vm_area_struct *vma)
  

{ struct drm_gem_object *obj = dma_buf->priv; struct drm_device *dev = obj->dev; @@ -315,6 +324,15 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {

• driver's scatter/gather table

*/

+/**

• • drm_gem_prime_export - helper library implemention of the export callback
• • @dev: drm_device to export from
• • @obj: GEM object to export
• • @flags: flags like DRM_CLOEXEC
• • This is the implementation of the gem_prime_export functions for GEM drivers
• • using the PRIME helpers.
• */

struct dma_buf *drm_gem_prime_export(struct drm_device *dev, struct drm_gem_object *obj, int flags) { @@ -355,9 +373,23 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, return dmabuf; }

+/**

• • drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
• • @dev: dev to export the buffer from
• • @file_priv: drm file-private structure
• • @handle: buffer handle to export
• • @flags: flags like DRM_CLOEXEC
• • @prime_fd: pointer to storage for the fd id of the create dma-buf
• • This is the PRIME export function which must be used mandatorily by GEM
• • drivers to ensure correct lifetime management of the underlying GEM object.
• • The actual exporting from GEM object to a dma-buf is done through the
• • gem_prime_export driver callback.
• */

int drm_gem_prime_handle_to_fd(struct drm_device *dev,

•           struct drm_file *file_priv, uint32_t handle, uint32_t flags,
  

•           int *prime_fd)
  

•                          struct drm_file *file_priv, uint32_t handle,
  

•                          uint32_t flags,
  

•                          int *prime_fd)
  

{ struct drm_gem_object *obj; int ret = 0; @@ -441,6 +473,14 @@ out_unlock: } EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);

+/**

• • drm_gem_prime_import - helper library implemention of the import callback
• • @dev: drm_device to import into
• • @dma_buf: dma-buf object to import
• • This is the implementation of the gem_prime_import functions for GEM drivers
• • using the PRIME helpers.
• */

struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf) { @@ -496,8 +536,21 @@ fail_detach: } EXPORT_SYMBOL(drm_gem_prime_import);

+/**

• • drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
• • @dev: dev to export the buffer from
• • @file_priv: drm file-private structure
• • @prime_fd: fd id of the dma-buf which should be imported
• • @handle: pointer to storage for the handle of the imported buffer object
• • This is the PRIME import function which must be used mandatorily by GEM
• • drivers to ensure correct lifetime management of the underlying GEM object.
• • The actual importing of GEM object from the dma-buf is done through the
• • gem_import_export driver callback.
• */

int drm_gem_prime_fd_to_handle(struct drm_device *dev,

•           struct drm_file *file_priv, int prime_fd, uint32_t *handle)
  

•                          struct drm_file *file_priv, int prime_fd,
  

•                          uint32_t *handle)
  

{ struct dma_buf *dma_buf; struct drm_gem_object *obj; @@ -598,12 +651,14 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, args->fd, &args->handle); }

-/*

• • drm_prime_pages_to_sg

+/**

• • drm_prime_pages_to_sg - converts a page array into an sg list
• • @pages: pointer to the array of page pointers to convert
• • @nr_pages: length of the page vector

• • this helper creates an sg table object from a set of pages

• • This helper creates an sg table object from a set of pages
  • the driver is responsible for mapping the pages into the

• • importers address space

• • importers address space for use with dma_buf itself.
  */

struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages) { @@ -628,9 +683,16 @@ out: } EXPORT_SYMBOL(drm_prime_pages_to_sg);

-/* export an sg table into an array of pages and addresses

• this is currently required by the TTM driver in order to do correct fault
• handling */

+/**

• • drm_prime_sg_to_page_addr_arrays - convert an sg table into a page array
• • @sgt: scatter-gather table to convert
• • @pages: array of page pointers to store the page array in
• • @addrs: optional array to store the dma bus address of each page
• • @max_pages: size of both the passed-in arrays
• • Exports an sg table into an array of pages and addresses. This is currently
• • required by the TTM driver in order to do correct fault handling.
• */

int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, dma_addr_t *addrs, int max_pages) { @@ -663,7 +725,15 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, return 0; } EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays); -/* helper function to cleanup a GEM/prime object */

+/**

• • drm_prime_gem_destroy - helper to clean up a PRIME-imported GEM object
• • @obj: GEM object which was created from a dma-buf
• • @sg: the sg-table which was pinned at import time
• • This is the cleanup functions which GEM drivers need to call when they use
• • @drm_gem_prime_import to import dma-bufs.
• */

void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg) { struct dma_buf_attachment *attach; @@ -683,11 +753,9 @@ void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv) INIT_LIST_HEAD(&prime_fpriv->head); mutex_init(&prime_fpriv->lock); } -EXPORT_SYMBOL(drm_prime_init_file_private);

void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv) { /* by now drm_gem_release should've made sure the list is empty */ WARN_ON(!list_empty(&prime_fpriv->head)); }

-EXPORT_SYMBOL(drm_prime_destroy_file_private);

1.8.5.2

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

Hi

On Thu, Jan 23, 2014 at 10:37 AM, Daniel Vetter daniel@ffwll.ch wrote:

On Thu, Jan 23, 2014 at 10:28:42AM +0100, David Herrmann wrote:

...

Hi

On Thu, Jan 23, 2014 at 9:52 AM, Daniel Vetter daniel.vetter@ffwll.ch wrote:

...

For giant hilarity the DocBook reference overview is only generated when in a level 2 section, not in a level 3 section. So we need to move this up a bit as a side-by-side section to the main PRIME documentation.

Whatever.

I tried digging through scripts/kernel-doc but.. ugh.. it's perl! No idea how to fix that. But sect2 seems fine as the whole section is PRIME-related.

Well we have two sect2 now: One for the PRIME overview, the other for the reference documenation. I've done the same split for drm_mm btw. If we want to fix this I think this is actually in the DocBook stylesheet, not in the kerneldoc thing. I've checked the intermediate xml and all the function references are there, they even get generated as html files and you can xref them from within the docbook. There's just no section topic for sect3 levels, so you never see a link to that separate page without an explicit reference.

Hm, why not this:

</sect3> + <title>PRIME Function References</title> +!Edrivers/gpu/drm/drm_prime.c + </sect2>

So you just put it at the end of the prime-sect2?

The kernel-doc script at least has "<sect2>" hardcoded, but yeah, no idea where to fix that. So I think this is fine.

Thanks David

...

Thanks David

...

To have a complete set of references add the missing kerneldoc for all functions exported to modules with the exception of the file private init/destry functions - drivers have no business calling those, so let's just drop the EXPORT_SYMBOL instead.

Also reflow the function parameters to align correctly and break at 80 chars - my OCD couldn't stand them while writing the kerneldoc ;-)

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

Documentation/DocBook/drm.tmpl | 6 ++- drivers/gpu/drm/drm_prime.c | 110 +++++++++++++++++++++++++++++++++-------- 2 files changed, 94 insertions(+), 22 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 0cc1d85d04de..07abe52b1176 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -898,10 +898,14 @@ struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev, </para> </sect3> <sect3>

•      <title>PRIME Helper Functions Reference</title>
  

•      <title>PRIME Helper Functions</title>
  

!Pdrivers/gpu/drm/drm_prime.c PRIME Helpers </sect3> </sect2>

•  <sect2>
  

•   <title>PRIME Function References</title>
  

+!Edrivers/gpu/drm/drm_prime.c

•  </sect2>
  

  </sect1>

  <!-- Internals: mode setting -->

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 56805c39c906..f1437b6c8dbf 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -68,7 +68,8 @@ struct drm_prime_attachment { enum dma_data_direction dir; };

-static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle) +static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,

•                               struct dma_buf *dma_buf, uint32_t handle)
  

{ struct drm_prime_member *member;

@@ -174,7 +175,7 @@ void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpr }

static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,

•           enum dma_data_direction dir)
  

•                                       enum dma_data_direction dir)
  

{ struct drm_prime_attachment *prime_attach = attach->priv; struct drm_gem_object *obj = attach->dmabuf->priv; @@ -211,11 +212,19 @@ static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, }

static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,

•           struct sg_table *sgt, enum dma_data_direction dir)
  

•                             struct sg_table *sgt,
  

•                             enum dma_data_direction dir)
  

{ /* nothing to be done here */ }

+/**

• • drm_gem_dmabuf_release - dma_buf release implementation for GEM
• • @dma_buf: buffer to be released
• • Generic release function for dma_bufs exported as PRIME buffers. GEM drivers
• • must use this in their dma_buf ops structure as the release callback.
• */

void drm_gem_dmabuf_release(struct dma_buf *dma_buf) { struct drm_gem_object *obj = dma_buf->priv; @@ -242,30 +251,30 @@ static void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) }

static void *drm_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,

•           unsigned long page_num)
  

•                                   unsigned long page_num)
  

{ return NULL; }

static void drm_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,

•           unsigned long page_num, void *addr)
  

•                                    unsigned long page_num, void *addr)
  

{

} static void *drm_gem_dmabuf_kmap(struct dma_buf *dma_buf,

•           unsigned long page_num)
  

•                            unsigned long page_num)
  

{ return NULL; }

static void drm_gem_dmabuf_kunmap(struct dma_buf *dma_buf,

•           unsigned long page_num, void *addr)
  

•                             unsigned long page_num, void *addr)
  

{

}

static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,

•           struct vm_area_struct *vma)
  

•                          struct vm_area_struct *vma)
  

{ struct drm_gem_object *obj = dma_buf->priv; struct drm_device *dev = obj->dev; @@ -315,6 +324,15 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {

• driver's scatter/gather table

*/

+/**

• • drm_gem_prime_export - helper library implemention of the export callback
• • @dev: drm_device to export from
• • @obj: GEM object to export
• • @flags: flags like DRM_CLOEXEC
• • This is the implementation of the gem_prime_export functions for GEM drivers
• • using the PRIME helpers.
• */

struct dma_buf *drm_gem_prime_export(struct drm_device *dev, struct drm_gem_object *obj, int flags) { @@ -355,9 +373,23 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, return dmabuf; }

+/**

• • drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
• • @dev: dev to export the buffer from
• • @file_priv: drm file-private structure
• • @handle: buffer handle to export
• • @flags: flags like DRM_CLOEXEC
• • @prime_fd: pointer to storage for the fd id of the create dma-buf
• • This is the PRIME export function which must be used mandatorily by GEM
• • drivers to ensure correct lifetime management of the underlying GEM object.
• • The actual exporting from GEM object to a dma-buf is done through the
• • gem_prime_export driver callback.
• */

int drm_gem_prime_handle_to_fd(struct drm_device *dev,

•           struct drm_file *file_priv, uint32_t handle, uint32_t flags,
  

•           int *prime_fd)
  

•                          struct drm_file *file_priv, uint32_t handle,
  

•                          uint32_t flags,
  

•                          int *prime_fd)
  

{ struct drm_gem_object *obj; int ret = 0; @@ -441,6 +473,14 @@ out_unlock: } EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);

+/**

• • drm_gem_prime_import - helper library implemention of the import callback
• • @dev: drm_device to import into
• • @dma_buf: dma-buf object to import
• • This is the implementation of the gem_prime_import functions for GEM drivers
• • using the PRIME helpers.
• */

struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf) { @@ -496,8 +536,21 @@ fail_detach: } EXPORT_SYMBOL(drm_gem_prime_import);

+/**

• • drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
• • @dev: dev to export the buffer from
• • @file_priv: drm file-private structure
• • @prime_fd: fd id of the dma-buf which should be imported
• • @handle: pointer to storage for the handle of the imported buffer object
• • This is the PRIME import function which must be used mandatorily by GEM
• • drivers to ensure correct lifetime management of the underlying GEM object.
• • The actual importing of GEM object from the dma-buf is done through the
• • gem_import_export driver callback.
• */

int drm_gem_prime_fd_to_handle(struct drm_device *dev,

•           struct drm_file *file_priv, int prime_fd, uint32_t *handle)
  

•                          struct drm_file *file_priv, int prime_fd,
  

•                          uint32_t *handle)
  

{ struct dma_buf *dma_buf; struct drm_gem_object *obj; @@ -598,12 +651,14 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, args->fd, &args->handle); }

-/*

• • drm_prime_pages_to_sg

+/**

• • drm_prime_pages_to_sg - converts a page array into an sg list
• • @pages: pointer to the array of page pointers to convert
• • @nr_pages: length of the page vector

• • this helper creates an sg table object from a set of pages

• • This helper creates an sg table object from a set of pages
  • the driver is responsible for mapping the pages into the

• • importers address space

• • importers address space for use with dma_buf itself.
  */

struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages) { @@ -628,9 +683,16 @@ out: } EXPORT_SYMBOL(drm_prime_pages_to_sg);

-/* export an sg table into an array of pages and addresses

• this is currently required by the TTM driver in order to do correct fault
• handling */

+/**

• • drm_prime_sg_to_page_addr_arrays - convert an sg table into a page array
• • @sgt: scatter-gather table to convert
• • @pages: array of page pointers to store the page array in
• • @addrs: optional array to store the dma bus address of each page
• • @max_pages: size of both the passed-in arrays
• • Exports an sg table into an array of pages and addresses. This is currently
• • required by the TTM driver in order to do correct fault handling.
• */

int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, dma_addr_t *addrs, int max_pages) { @@ -663,7 +725,15 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, return 0; } EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays); -/* helper function to cleanup a GEM/prime object */

+/**

• • drm_prime_gem_destroy - helper to clean up a PRIME-imported GEM object
• • @obj: GEM object which was created from a dma-buf
• • @sg: the sg-table which was pinned at import time
• • This is the cleanup functions which GEM drivers need to call when they use
• • @drm_gem_prime_import to import dma-bufs.
• */

void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg) { struct dma_buf_attachment *attach; @@ -683,11 +753,9 @@ void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv) INIT_LIST_HEAD(&prime_fpriv->head); mutex_init(&prime_fpriv->lock); } -EXPORT_SYMBOL(drm_prime_init_file_private);

void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv) { /* by now drm_gem_release should've made sure the list is empty */ WARN_ON(!list_empty(&prime_fpriv->head)); }

-EXPORT_SYMBOL(drm_prime_destroy_file_private);

1.8.5.2

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Hi

On Thu, Jan 23, 2014 at 9:52 AM, Daniel Vetter daniel.vetter@ffwll.ch wrote:

Driver modules really don't have much business frobbing around with this: Splitting up modeset resources (if we ever get around to enable this for real) should be something purely controlled and managed by the drm core in a driver-agnostic fashion. As usual imx disagrees, but that's due to the convoluted setup sequence. Russell King is working on a proper fix for that, so this patch needs to wait for those to land to avoid breaking imx.

I tried working on "modeset-object splitting" as part of the DRM-Master rework, but that's all really racy and we'd break current user-space. For instance the object-indices (compared to object-IDs) need to be constant, but with our current object-lists, removing an object will change indices (thus breaking possible_crtcs/encoders).

There are ways to make that work, but it'd require huge drm_crtc.c changes. And I also think our locking doesn't provide for that. Furthermore, resource-retrieval is not atomic (we need multiple ioctls: GetResources, GetCrtc, ...) so if we change the objects in between, we may break running apps in a subtle way.

So I'm all for making object-lists immutable after drm_dev_register() was called. Imo, hardware should wait for all sub-devices to be present and then call drm_dev_register(). Once the first sub-device is removed, you call drm_dev_unregister(). And I think that's what Russel's compound-device infrastructure does by hiding the sub-devices in a compound device.

If there's ever hardware that truly supports sub-device hotplugging, we can support that by adding a DRM-ClientCap like 3D modes. But as I understand, the current hardware is still a single piece of hardware which just might get probed via different buses and thus is not an atomic entity. So the device is still hotplugged as a whole, we just don't get the events through a single path.

Thanks David

Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

---

drivers/gpu/drm/drm_crtc.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index cf134540b675..98084413a842 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1264,7 +1264,6 @@ int drm_mode_group_init_legacy_group(struct drm_device *dev,

    return 0;

} -EXPORT_SYMBOL(drm_mode_group_init_legacy_group);

/**

• drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo

-- 1.8.5.2

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel