Re: [PATCH] drm/doc/rfc: drop the i915_gem_lmem.h header

2021-05-12 Thread Daniel Vetter
On Wed, May 12, 2021 at 08:38:55AM +0100, Matthew Auld wrote:
> On 11/05/2021 18:29, Daniel Vetter wrote:
> > On Tue, May 11, 2021 at 07:28:08PM +0200, Daniel Vetter wrote:
> > > On Tue, May 11, 2021 at 06:03:56PM +0100, Matthew Auld wrote:
> > > > The proper headers have now landed in include/uapi/drm/i915_drm.h, so we
> > > > can drop i915_gem_lmem.h and instead just reference the real headers for
> > > > pulling in the kernel doc.
> > > > 
> > > > Suggested-by: Daniel Vetter 
> > > > Signed-off-by: Matthew Auld 
> > > 
> > > Reviewed-by: Daniel Vetter 
> > > 
> > > I guess we need to have a note that when we land the pciid for dg1 to move
> > > all the remaining bits over to real docs and delete the i915 lmem rfc. But
> > > everything in due time.
> > 
> > One thing I forgot: The include stanza will I think result in the
> > explicitly included functions not showing up in the normal driver uapi
> > docs. Which I think is fine while we settle all this. Or do I get this
> > wrong?
> 
> It all looks ok in the rendered html, but yeah the explicitly inlcuded
> functions/structs don't seem to link back to driver-uapi, and instead just
> link to the "local version" in i915_gem_lmem.

I think that's all ok. Thanks for checking.
-Daniel
> 
> > -Daniel
> > 
> > > -Daniel
> > > 
> > > > ---
> > > >   Documentation/gpu/rfc/i915_gem_lmem.h   | 237 
> > > >   Documentation/gpu/rfc/i915_gem_lmem.rst |   6 +-
> > > >   2 files changed, 3 insertions(+), 240 deletions(-)
> > > >   delete mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
> > > > 
> > > > diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
> > > > b/Documentation/gpu/rfc/i915_gem_lmem.h
> > > > deleted file mode 100644
> > > > index d9c61bea0556..
> > > > --- a/Documentation/gpu/rfc/i915_gem_lmem.h
> > > > +++ /dev/null
> > > > @@ -1,237 +0,0 @@
> > > > -/**
> > > > - * enum drm_i915_gem_memory_class - Supported memory classes
> > > > - */
> > > > -enum drm_i915_gem_memory_class {
> > > > -   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
> > > > -   I915_MEMORY_CLASS_SYSTEM = 0,
> > > > -   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
> > > > -   I915_MEMORY_CLASS_DEVICE,
> > > > -};
> > > > -
> > > > -/**
> > > > - * struct drm_i915_gem_memory_class_instance - Identify particular 
> > > > memory region
> > > > - */
> > > > -struct drm_i915_gem_memory_class_instance {
> > > > -   /** @memory_class: See enum drm_i915_gem_memory_class */
> > > > -   __u16 memory_class;
> > > > -
> > > > -   /** @memory_instance: Which instance */
> > > > -   __u16 memory_instance;
> > > > -};
> > > > -
> > > > -/**
> > > > - * struct drm_i915_memory_region_info - Describes one region as known 
> > > > to the
> > > > - * driver.
> > > > - *
> > > > - * Note that we reserve some stuff here for potential future work. As 
> > > > an example
> > > > - * we might want expose the capabilities for a given region, which 
> > > > could include
> > > > - * things like if the region is CPU mappable/accessible, what are the 
> > > > supported
> > > > - * mapping types etc.
> > > > - *
> > > > - * Note that to extend struct drm_i915_memory_region_info and struct
> > > > - * drm_i915_query_memory_regions in the future the plan is to do the 
> > > > following:
> > > > - *
> > > > - * .. code-block:: C
> > > > - *
> > > > - * struct drm_i915_memory_region_info {
> > > > - * struct drm_i915_gem_memory_class_instance region;
> > > > - * union {
> > > > - * __u32 rsvd0;
> > > > - * __u32 new_thing1;
> > > > - * };
> > > > - * ...
> > > > - * union {
> > > > - * __u64 rsvd1[8];
> > > > - * struct {
> > > > - * __u64 new_thing2;
> > > > - * __u64 new_thing3;
> > > > - * ...
> > > > - * };
> > > > - * };
> > > > - * };
> > > > - *
> > > > - * With this things should remain source compatible between versions 
> > > > for
> > > > - * userspace, even as we add new fields.
> > > > - *
> > > > - * Note this is using both struct drm_i915_query_item and struct 
> > > > drm_i915_query.
> > > > - * For this new query we are adding the new query id 
> > > > DRM_I915_QUERY_MEMORY_REGIONS
> > > > - * at &drm_i915_query_item.query_id.
> > > > - */
> > > > -struct drm_i915_memory_region_info {
> > > > -   /** @region: The class:instance pair encoding */
> > > > -   struct drm_i915_gem_memory_class_instance region;
> > > > -
> > > > -   /** @rsvd0: MBZ */
> > > > -   __u32 rsvd0;
> > > > -
> > > > -   /** @probed_size: Memory probed by the driver (-1 = unknown) */
> > > > -   __u64 probed_size;
> > > > -
> > > > -   /** @unallocated_size: Estimate of memory remaining (-1 = 
> > > > unknown) */
> > > > -   __u64 unallocated_size;
> > > > -
> > >

Re: [PATCH] drm/doc/rfc: drop the i915_gem_lmem.h header

2021-05-12 Thread Matthew Auld

On 11/05/2021 18:29, Daniel Vetter wrote:

On Tue, May 11, 2021 at 07:28:08PM +0200, Daniel Vetter wrote:

On Tue, May 11, 2021 at 06:03:56PM +0100, Matthew Auld wrote:

The proper headers have now landed in include/uapi/drm/i915_drm.h, so we
can drop i915_gem_lmem.h and instead just reference the real headers for
pulling in the kernel doc.

Suggested-by: Daniel Vetter 
Signed-off-by: Matthew Auld 


Reviewed-by: Daniel Vetter 

I guess we need to have a note that when we land the pciid for dg1 to move
all the remaining bits over to real docs and delete the i915 lmem rfc. But
everything in due time.


One thing I forgot: The include stanza will I think result in the
explicitly included functions not showing up in the normal driver uapi
docs. Which I think is fine while we settle all this. Or do I get this
wrong?


It all looks ok in the rendered html, but yeah the explicitly inlcuded 
functions/structs don't seem to link back to driver-uapi, and instead 
just link to the "local version" in i915_gem_lmem.



-Daniel


-Daniel


---
  Documentation/gpu/rfc/i915_gem_lmem.h   | 237 
  Documentation/gpu/rfc/i915_gem_lmem.rst |   6 +-
  2 files changed, 3 insertions(+), 240 deletions(-)
  delete mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h

diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
b/Documentation/gpu/rfc/i915_gem_lmem.h
deleted file mode 100644
index d9c61bea0556..
--- a/Documentation/gpu/rfc/i915_gem_lmem.h
+++ /dev/null
@@ -1,237 +0,0 @@
-/**
- * enum drm_i915_gem_memory_class - Supported memory classes
- */
-enum drm_i915_gem_memory_class {
-   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
-   I915_MEMORY_CLASS_SYSTEM = 0,
-   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
-   I915_MEMORY_CLASS_DEVICE,
-};
-
-/**
- * struct drm_i915_gem_memory_class_instance - Identify particular memory 
region
- */
-struct drm_i915_gem_memory_class_instance {
-   /** @memory_class: See enum drm_i915_gem_memory_class */
-   __u16 memory_class;
-
-   /** @memory_instance: Which instance */
-   __u16 memory_instance;
-};
-
-/**
- * struct drm_i915_memory_region_info - Describes one region as known to the
- * driver.
- *
- * Note that we reserve some stuff here for potential future work. As an 
example
- * we might want expose the capabilities for a given region, which could 
include
- * things like if the region is CPU mappable/accessible, what are the supported
- * mapping types etc.
- *
- * Note that to extend struct drm_i915_memory_region_info and struct
- * drm_i915_query_memory_regions in the future the plan is to do the following:
- *
- * .. code-block:: C
- *
- * struct drm_i915_memory_region_info {
- * struct drm_i915_gem_memory_class_instance region;
- * union {
- * __u32 rsvd0;
- * __u32 new_thing1;
- * };
- * ...
- * union {
- * __u64 rsvd1[8];
- * struct {
- * __u64 new_thing2;
- * __u64 new_thing3;
- * ...
- * };
- * };
- * };
- *
- * With this things should remain source compatible between versions for
- * userspace, even as we add new fields.
- *
- * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
- * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS
- * at &drm_i915_query_item.query_id.
- */
-struct drm_i915_memory_region_info {
-   /** @region: The class:instance pair encoding */
-   struct drm_i915_gem_memory_class_instance region;
-
-   /** @rsvd0: MBZ */
-   __u32 rsvd0;
-
-   /** @probed_size: Memory probed by the driver (-1 = unknown) */
-   __u64 probed_size;
-
-   /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
-   __u64 unallocated_size;
-
-   /** @rsvd1: MBZ */
-   __u64 rsvd1[8];
-};
-
-/**
- * struct drm_i915_query_memory_regions
- *
- * The region info query enumerates all regions known to the driver by filling
- * in an array of struct drm_i915_memory_region_info structures.
- *
- * Example for getting the list of supported regions:
- *
- * .. code-block:: C
- *
- * struct drm_i915_query_memory_regions *info;
- * struct drm_i915_query_item item = {
- * .query_id = DRM_I915_QUERY_MEMORY_REGIONS;
- * };
- * struct drm_i915_query query = {
- * .num_items = 1,
- * .items_ptr = (uintptr_t)&item,
- * };
- * int err, i;
- *
- * // First query the size of the blob we need, this needs to be large
- * // enough to hold our array of regions. The kernel will fill out the
- * // item.length for us, which is the number of bytes we need.
- * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
- * if (err) ...
- *
- * info = calloc(1, item.length);
- * // Now 

Re: [PATCH] drm/doc/rfc: drop the i915_gem_lmem.h header

2021-05-11 Thread Daniel Vetter
On Tue, May 11, 2021 at 07:28:08PM +0200, Daniel Vetter wrote:
> On Tue, May 11, 2021 at 06:03:56PM +0100, Matthew Auld wrote:
> > The proper headers have now landed in include/uapi/drm/i915_drm.h, so we
> > can drop i915_gem_lmem.h and instead just reference the real headers for
> > pulling in the kernel doc.
> > 
> > Suggested-by: Daniel Vetter 
> > Signed-off-by: Matthew Auld 
> 
> Reviewed-by: Daniel Vetter 
> 
> I guess we need to have a note that when we land the pciid for dg1 to move
> all the remaining bits over to real docs and delete the i915 lmem rfc. But
> everything in due time.

One thing I forgot: The include stanza will I think result in the
explicitly included functions not showing up in the normal driver uapi
docs. Which I think is fine while we settle all this. Or do I get this
wrong?
-Daniel

> -Daniel
> 
> > ---
> >  Documentation/gpu/rfc/i915_gem_lmem.h   | 237 
> >  Documentation/gpu/rfc/i915_gem_lmem.rst |   6 +-
> >  2 files changed, 3 insertions(+), 240 deletions(-)
> >  delete mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
> > 
> > diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
> > b/Documentation/gpu/rfc/i915_gem_lmem.h
> > deleted file mode 100644
> > index d9c61bea0556..
> > --- a/Documentation/gpu/rfc/i915_gem_lmem.h
> > +++ /dev/null
> > @@ -1,237 +0,0 @@
> > -/**
> > - * enum drm_i915_gem_memory_class - Supported memory classes
> > - */
> > -enum drm_i915_gem_memory_class {
> > -   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
> > -   I915_MEMORY_CLASS_SYSTEM = 0,
> > -   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
> > -   I915_MEMORY_CLASS_DEVICE,
> > -};
> > -
> > -/**
> > - * struct drm_i915_gem_memory_class_instance - Identify particular memory 
> > region
> > - */
> > -struct drm_i915_gem_memory_class_instance {
> > -   /** @memory_class: See enum drm_i915_gem_memory_class */
> > -   __u16 memory_class;
> > -
> > -   /** @memory_instance: Which instance */
> > -   __u16 memory_instance;
> > -};
> > -
> > -/**
> > - * struct drm_i915_memory_region_info - Describes one region as known to 
> > the
> > - * driver.
> > - *
> > - * Note that we reserve some stuff here for potential future work. As an 
> > example
> > - * we might want expose the capabilities for a given region, which could 
> > include
> > - * things like if the region is CPU mappable/accessible, what are the 
> > supported
> > - * mapping types etc.
> > - *
> > - * Note that to extend struct drm_i915_memory_region_info and struct
> > - * drm_i915_query_memory_regions in the future the plan is to do the 
> > following:
> > - *
> > - * .. code-block:: C
> > - *
> > - * struct drm_i915_memory_region_info {
> > - * struct drm_i915_gem_memory_class_instance region;
> > - * union {
> > - * __u32 rsvd0;
> > - * __u32 new_thing1;
> > - * };
> > - * ...
> > - * union {
> > - * __u64 rsvd1[8];
> > - * struct {
> > - * __u64 new_thing2;
> > - * __u64 new_thing3;
> > - * ...
> > - * };
> > - * };
> > - * };
> > - *
> > - * With this things should remain source compatible between versions for
> > - * userspace, even as we add new fields.
> > - *
> > - * Note this is using both struct drm_i915_query_item and struct 
> > drm_i915_query.
> > - * For this new query we are adding the new query id 
> > DRM_I915_QUERY_MEMORY_REGIONS
> > - * at &drm_i915_query_item.query_id.
> > - */
> > -struct drm_i915_memory_region_info {
> > -   /** @region: The class:instance pair encoding */
> > -   struct drm_i915_gem_memory_class_instance region;
> > -
> > -   /** @rsvd0: MBZ */
> > -   __u32 rsvd0;
> > -
> > -   /** @probed_size: Memory probed by the driver (-1 = unknown) */
> > -   __u64 probed_size;
> > -
> > -   /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
> > -   __u64 unallocated_size;
> > -
> > -   /** @rsvd1: MBZ */
> > -   __u64 rsvd1[8];
> > -};
> > -
> > -/**
> > - * struct drm_i915_query_memory_regions
> > - *
> > - * The region info query enumerates all regions known to the driver by 
> > filling
> > - * in an array of struct drm_i915_memory_region_info structures.
> > - *
> > - * Example for getting the list of supported regions:
> > - *
> > - * .. code-block:: C
> > - *
> > - * struct drm_i915_query_memory_regions *info;
> > - * struct drm_i915_query_item item = {
> > - * .query_id = DRM_I915_QUERY_MEMORY_REGIONS;
> > - * };
> > - * struct drm_i915_query query = {
> > - * .num_items = 1,
> > - * .items_ptr = (uintptr_t)&item,
> > - * };
> > - * int err, i;
> > - *
> > - * // First query the size of the blob we need, this needs to be large
> > - * // enough to hold our array of regions. The kernel will fill out the
> > - * // item.length for us, which is the number of bytes we need.
> > - * err = ioctl(fd, DRM_IOCTL_I915_QU

Re: [PATCH] drm/doc/rfc: drop the i915_gem_lmem.h header

2021-05-11 Thread Daniel Vetter
On Tue, May 11, 2021 at 06:03:56PM +0100, Matthew Auld wrote:
> The proper headers have now landed in include/uapi/drm/i915_drm.h, so we
> can drop i915_gem_lmem.h and instead just reference the real headers for
> pulling in the kernel doc.
> 
> Suggested-by: Daniel Vetter 
> Signed-off-by: Matthew Auld 

Reviewed-by: Daniel Vetter 

I guess we need to have a note that when we land the pciid for dg1 to move
all the remaining bits over to real docs and delete the i915 lmem rfc. But
everything in due time.
-Daniel

> ---
>  Documentation/gpu/rfc/i915_gem_lmem.h   | 237 
>  Documentation/gpu/rfc/i915_gem_lmem.rst |   6 +-
>  2 files changed, 3 insertions(+), 240 deletions(-)
>  delete mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
> 
> diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
> b/Documentation/gpu/rfc/i915_gem_lmem.h
> deleted file mode 100644
> index d9c61bea0556..
> --- a/Documentation/gpu/rfc/i915_gem_lmem.h
> +++ /dev/null
> @@ -1,237 +0,0 @@
> -/**
> - * enum drm_i915_gem_memory_class - Supported memory classes
> - */
> -enum drm_i915_gem_memory_class {
> - /** @I915_MEMORY_CLASS_SYSTEM: System memory */
> - I915_MEMORY_CLASS_SYSTEM = 0,
> - /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
> - I915_MEMORY_CLASS_DEVICE,
> -};
> -
> -/**
> - * struct drm_i915_gem_memory_class_instance - Identify particular memory 
> region
> - */
> -struct drm_i915_gem_memory_class_instance {
> - /** @memory_class: See enum drm_i915_gem_memory_class */
> - __u16 memory_class;
> -
> - /** @memory_instance: Which instance */
> - __u16 memory_instance;
> -};
> -
> -/**
> - * struct drm_i915_memory_region_info - Describes one region as known to the
> - * driver.
> - *
> - * Note that we reserve some stuff here for potential future work. As an 
> example
> - * we might want expose the capabilities for a given region, which could 
> include
> - * things like if the region is CPU mappable/accessible, what are the 
> supported
> - * mapping types etc.
> - *
> - * Note that to extend struct drm_i915_memory_region_info and struct
> - * drm_i915_query_memory_regions in the future the plan is to do the 
> following:
> - *
> - * .. code-block:: C
> - *
> - *   struct drm_i915_memory_region_info {
> - *   struct drm_i915_gem_memory_class_instance region;
> - *   union {
> - *   __u32 rsvd0;
> - *   __u32 new_thing1;
> - *   };
> - *   ...
> - *   union {
> - *   __u64 rsvd1[8];
> - *   struct {
> - *   __u64 new_thing2;
> - *   __u64 new_thing3;
> - *   ...
> - *   };
> - *   };
> - *   };
> - *
> - * With this things should remain source compatible between versions for
> - * userspace, even as we add new fields.
> - *
> - * Note this is using both struct drm_i915_query_item and struct 
> drm_i915_query.
> - * For this new query we are adding the new query id 
> DRM_I915_QUERY_MEMORY_REGIONS
> - * at &drm_i915_query_item.query_id.
> - */
> -struct drm_i915_memory_region_info {
> - /** @region: The class:instance pair encoding */
> - struct drm_i915_gem_memory_class_instance region;
> -
> - /** @rsvd0: MBZ */
> - __u32 rsvd0;
> -
> - /** @probed_size: Memory probed by the driver (-1 = unknown) */
> - __u64 probed_size;
> -
> - /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
> - __u64 unallocated_size;
> -
> - /** @rsvd1: MBZ */
> - __u64 rsvd1[8];
> -};
> -
> -/**
> - * struct drm_i915_query_memory_regions
> - *
> - * The region info query enumerates all regions known to the driver by 
> filling
> - * in an array of struct drm_i915_memory_region_info structures.
> - *
> - * Example for getting the list of supported regions:
> - *
> - * .. code-block:: C
> - *
> - *   struct drm_i915_query_memory_regions *info;
> - *   struct drm_i915_query_item item = {
> - *   .query_id = DRM_I915_QUERY_MEMORY_REGIONS;
> - *   };
> - *   struct drm_i915_query query = {
> - *   .num_items = 1,
> - *   .items_ptr = (uintptr_t)&item,
> - *   };
> - *   int err, i;
> - *
> - *   // First query the size of the blob we need, this needs to be large
> - *   // enough to hold our array of regions. The kernel will fill out the
> - *   // item.length for us, which is the number of bytes we need.
> - *   err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
> - *   if (err) ...
> - *
> - *   info = calloc(1, item.length);
> - *   // Now that we allocated the required number of bytes, we call the ioctl
> - *   // again, this time with the data_ptr pointing to our newly allocated
> - *   // blob, which the kernel can then populate with the all the region 
> info.
> - *   item.data_ptr = (uintptr_t)&info,
> - *
> - *   err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
> - *   if (err) ...
> - *
> - *   // We can