Re: [PATCH RFC 5/5] drm/amdgpu: Add accounting of buffer object creation request via DRM cgroup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 27.11.18 um 19:15 schrieb Kenny Ho:
Hey Christian,

Sorry for the late reply, I missed this for some reason.

On Wed, Nov 21, 2018 at 5:00 AM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 370e9a5536ef..531726443104 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -72,6 +72,18 @@ extern "C" {
   #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
   #define DRM_IOCTL_AMDGPU_SCHED              DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)

+enum amdgpu_mem_domain {
+     AMDGPU_MEM_DOMAIN_CPU,
+     AMDGPU_MEM_DOMAIN_GTT,
+     AMDGPU_MEM_DOMAIN_VRAM,
+     AMDGPU_MEM_DOMAIN_GDS,
+     AMDGPU_MEM_DOMAIN_GWS,
+     AMDGPU_MEM_DOMAIN_OA,
+     __MAX_AMDGPU_MEM_DOMAIN
+};
Well that is a clear NAK since it duplicates the TTM defines. Please use
that one instead and don't make this UAPI.
This is defined to help with the chunk of changes below.  The
AMDGPU_GEM_DOMAIN* already exists and this is similar to how TTM has
TTM_PL_* to help with the creation of TTM_PL_FLAG_*:
https://elixir.bootlin.com/linux/v4.20-rc4/source/include/drm/ttm/ttm_placement.h#L36

I don't disagree that there is a duplication here but it's
pre-existing so if you can help clarify my confusion that would be
much appreciated.

The AMDGPU_GEM_DOMAIN are masks which are used in the frontend IOCTL interface to create BOs.

TTM defines the backend pools where the memory is then allocated from to fill the BOs.

So you are mixing frontend and backend here.

In other words for the whole cgroup interface you should not make a single change to amdgpu_drm.h or otherwise you are doing something wrong.

Regards,
Christian.


Reards,
Kenny

+
+extern char const *amdgpu_mem_domain_names[];
+
   /**
    * DOC: memory domains
    *
@@ -95,12 +107,12 @@ extern "C" {
    * %AMDGPU_GEM_DOMAIN_OA    Ordered append, used by 3D or Compute engines
    * for appending data.
    */
-#define AMDGPU_GEM_DOMAIN_CPU                0x1
-#define AMDGPU_GEM_DOMAIN_GTT                0x2
-#define AMDGPU_GEM_DOMAIN_VRAM               0x4
-#define AMDGPU_GEM_DOMAIN_GDS                0x8
-#define AMDGPU_GEM_DOMAIN_GWS                0x10
-#define AMDGPU_GEM_DOMAIN_OA         0x20
+#define AMDGPU_GEM_DOMAIN_CPU                (1 << AMDGPU_MEM_DOMAIN_CPU)
+#define AMDGPU_GEM_DOMAIN_GTT                (1 << AMDGPU_MEM_DOMAIN_GTT)
+#define AMDGPU_GEM_DOMAIN_VRAM               (1 << AMDGPU_MEM_DOMAIN_VRAM)
+#define AMDGPU_GEM_DOMAIN_GDS                (1 << AMDGPU_MEM_DOMAIN_GDS)
+#define AMDGPU_GEM_DOMAIN_GWS                (1 << AMDGPU_MEM_DOMAIN_GWS)
+#define AMDGPU_GEM_DOMAIN_OA         (1 << AMDGPU_MEM_DOMAIN_OA)
   #define AMDGPU_GEM_DOMAIN_MASK              (AMDGPU_GEM_DOMAIN_CPU | \
                                        AMDGPU_GEM_DOMAIN_GTT | \
                                        AMDGPU_GEM_DOMAIN_VRAM | \
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux