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

Kenny Ho y2kenny at gmail.com
Tue Nov 27 18:15:34 UTC 2018


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 at gmail.com> 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.

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 | \
>


More information about the amd-gfx mailing list