[PATCH 0/3] drm/amdgpu: enable DKMS build

[Zhou, Jammy]

Thanks all for the comments. The simple changes in this series are just to use 
relative paths as Christian mentioned. It can also be helpful if some users 
want to try amdgpu driver from latest upstream on some systems with relatively 
older kernel installed, in which case the Dynamic Kernel Module Support 
mechanism is used.

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Daniel Vetter
Sent: Tuesday, November 24, 2015 6:34 PM
To: Christian König
Cc: dri-devel at lists.freedesktop.org
Subject: Re: [PATCH 0/3] drm/amdgpu: enable DKMS build

On Tue, Nov 24, 2015 at 11:08:07AM +0100, Christian König wrote:
> On 24.11.2015 10:36, Thierry Reding wrote:
> >On Tue, Nov 24, 2015 at 09:59:09AM +0100, Daniel Vetter wrote:
> >>On Tue, Nov 24, 2015 at 04:55:19PM +0800, Jammy Zhou wrote:
> >>>This series enable the DKMS build of amdgpu driver.
> >>For the curious: What's DKMS?
> >I believe in this context it's "Dynamic Kernel Module Support":
> >
> > https://wiki.debian.org/KernelDKMS
> 
> Yeah, correct.
> 
> >
> >I'm somewhat surprised that one would have to do something special to 
> >the kernel build system to "enable" such a build. But perhaps this is 
> >completely unrelated.
> 
> I'm a bit torn apart on this. On the one hand I'm not sure if we have 
> drivers upstream explicitly supporting this as well?
> 
> On the other hand it's just the Makefiles which need to be written in 
> a way which makes them relocatable. E.g. no absolute path like 
> drivers/gpu/drm/amd/... in them and that's a good idea anyway.
> 
> Maybe just changing the commit message to "don't use absolute paths in 
> the makefiles" would be sufficient?

Oh I don't mind DKMS support, it makes sense to allow amd to use upstream as 
the baseline for their enhanced blob driver stack. Just wanted to know what it 
is. Imo what we shouldn't merge upstream would be compat code to allow amdgpu 
to be built on old kernels, since that's a lot more invasive than a few 
Makefile changes. Otoh that problem is largely address with the linux 
backporting project, which solves the "new driver directoy in old sources 
problem". So as long as it's just about building free-standing I think it's 
perfectly fine.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/amdgpu/atom: Send out the full AUX address

[Zhou, Jammy]

Patch is Reviewed-by: Jammy Zhou 

I cannot find the first patch in my mailbox, did I miss something?

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Alex Deucher
Sent: Monday, August 31, 2015 11:19 PM
To: dri-devel at lists.freedesktop.org
Cc: Deucher, Alexander; stable at vger.kernel.org
Subject: [PATCH 2/2] drm/amdgpu/atom: Send out the full AUX address

AUX addresses are 20 bits long. Send out the entire address instead of just the 
low 16 bits.

Port of:
drm/radeon/atom: Send out the full AUX address to amdgpu

Signed-off-by: Alex Deucher 
Cc: stable at vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c 
b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
index 9ba0a7d..92b6aca 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
@@ -139,7 +139,8 @@ amdgpu_atombios_dp_aux_transfer(struct drm_dp_aux *aux, 
struct drm_dp_aux_msg *m

tx_buf[0] = msg->address & 0xff;
tx_buf[1] = msg->address >> 8;
-   tx_buf[2] = msg->request << 4;
+   tx_buf[2] = (msg->request << 4) |
+   ((msg->address >> 16) & 0xf);
tx_buf[3] = msg->size ? (msg->size - 1) : 0;

switch (msg->request & ~DP_AUX_I2C_MOT) {
--
1.8.3.1

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

[PATCH] amdgpu: fix missing deinit on vamgr_32

[Zhou, Jammy]

It looks like it was missed when I rebased my original patches. Patch is 
Reviewed-by: Jammy Zhou 

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Alex Deucher
Sent: Monday, August 31, 2015 10:37 PM
To: dri-devel at lists.freedesktop.org
Cc: Deucher, Alexander; Liu, Monk
Subject: [PATCH] amdgpu: fix missing deinit on vamgr_32

From: "monk.liu" 

Signed-off-by: monk.liu 
Signed-off-by: Alex Deucher 
---
 amdgpu/amdgpu_device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c index 
75b12e2..e5a923e 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -132,6 +132,8 @@ static void 
amdgpu_device_free_internal(amdgpu_device_handle dev)  {
amdgpu_vamgr_deinit(dev->vamgr);
free(dev->vamgr);
+   amdgpu_vamgr_deinit(dev->vamgr_32);
+   free(dev->vamgr_32);
util_hash_table_destroy(dev->bo_flink_names);
util_hash_table_destroy(dev->bo_handles);
pthread_mutex_destroy(>bo_table_mutex);
--
1.8.3.1

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

[PATCH 2/2] amdgpu: serialize drmPrimeFDToHandle

[Zhou, Jammy]

Both patches are Reviewed-by: Jammy Zhou 

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Monday, August 24, 2015 5:44 PM
To: dri-devel at lists.freedesktop.org
Subject: [PATCH 2/2] amdgpu: serialize drmPrimeFDToHandle

From: Christian König 

Fixes the same problem as "intel: Serialize drmPrimeFDToHandle with 
struct_mutex".

Signed-off-by: Christian König 
---
 amdgpu/amdgpu_bo.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c index dab3804..adf4253 
100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -289,6 +289,10 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
int dma_fd;
uint64_t dma_buf_size = 0;

+   /* We must maintain a list of pairs , so that we always
+* return the same amdgpu_bo instance for the same handle. */
+   pthread_mutex_lock(>bo_table_mutex);
+
/* Convert a DMA buf handle to a KMS handle now. */
if (type == amdgpu_bo_handle_type_dma_buf_fd) {
uint32_t handle;
@@ -303,6 +307,7 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
/* Query the buffer size. */
size = lseek(shared_handle, 0, SEEK_END);
if (size == (off_t)-1) {
+   pthread_mutex_unlock(>bo_table_mutex);
amdgpu_close_kms_handle(dev, handle);
return -errno;
}
@@ -312,10 +317,6 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
shared_handle = handle;
}

-   /* We must maintain a list of pairs , so that we always
-* return the same amdgpu_bo instance for the same handle. */
-   pthread_mutex_lock(>bo_table_mutex);
-
/* If we have already created a buffer with this handle, find it. */
switch (type) {
case amdgpu_bo_handle_type_gem_flink_name:
--
1.9.1

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

[PATCH] drm: fix the usage after free

[Zhou, Jammy]

> Would be more convenient if Mathias would add his Signed-off-by as well and 
> send out the patch, cause he is the original author.

Agreed. Just was not quite sure if Mathias is working on the libdrm project 
directly or not based on the comments in the bugzilla "hopefully the fix can be 
pushed to master soon".

Regards,
Jammy

-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: Monday, August 24, 2015 3:52 PM
To: Zhou, Jammy; dri-devel at lists.freedesktop.org; master.homer at gmail.com
Subject: Re: [PATCH] drm: fix the usage after free

On 24.08.2015 05:56, Jammy Zhou wrote:
> From: Mathias Tillman 
>
> For readdir_r(), the next directory entry is returned in 
> caller-allocted buffer (pointered by pent here).
>
> https://bugs.freedesktop.org/show_bug.cgi?id=91704
>
> Signed-off-by: Jammy Zhou 

Would be more convenient if Mathias would add his Signed-off-by as well and 
send out the patch, cause he is the original author.

Anyway the patch is clearly a nice catch and Reviewed-by: Christian König 


Regards,
Christian.

> ---
>   xf86drm.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 5e02969..a7cc643 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2803,11 +2803,12 @@ static char *drmGetMinorNameForFD(int fd, int 
> type)
>   
>   while (readdir_r(sysdir, pent, ) == 0 && ent != NULL) {
>   if (strncmp(ent->d_name, name, len) == 0) {
> + snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s",
> +  ent->d_name);
> +
>   free(pent);
>   closedir(sysdir);
>   
> - snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s",
> -  ent->d_name);
>   return strdup(dev_name);
>   }
>   }

[PATCH 1/4] drm: add interface to get drm devices on the system v3

[Zhou, Jammy]

Excellent. Thanks a lot.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Friday, August 21, 2015 12:28 AM
To: Zhou, Jammy
Cc: ML dri-devel
Subject: Re: [PATCH 1/4] drm: add interface to get drm devices on the system v3

On 20 August 2015 at 04:02, Zhou, Jammy  wrote:
> Hi Emil,
>
> Can I get you RB now? It looks like there was no more comment from others.
>
I'll ping (again) one the possible users and if we haven't heard any objection 
by tomorrow midday I'll commit it to master.

Cheers,
Emil

[PATCH 1/4] drm: add interface to get drm devices on the system v3

[Zhou, Jammy]

Hi Emil,

Can I get you RB now? It looks like there was no more comment from others.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Monday, August 17, 2015 10:13 PM
To: Zhou, Jammy
Cc: ML dri-devel
Subject: Re: [PATCH 1/4] drm: add interface to get drm devices on the system v3

On 17 August 2015 at 04:09, Jammy Zhou  wrote:
> From: Emil Velikov 
>
> For mutiple GPU support, the devices on the system should be 
> enumerated to get necessary information about each device, and the 
> drmGetDevices interface is added for this. Currently only PCI devices 
> are supported for the enumeration.
>
> Typical usage:
> int count;
> drmDevicePtr *foo;
> count = drmGetDevices(NULL, 0);
> foo = calloc(count, sizeof(drmDevicePtr)); count = drmGetDevices(foo, 
> count);
> /* find proper device, open correct device node, etc */ 
> drmFreeDevices(foo, count); free(foo);
>
> v2: change according to feedback from Emil
> v3: fix the signed extension for PCI device info
>
Thanks Jammy. That's better.

As I would suspect the amdgpu changes to be urgent, can we leave this around 
for a few more days. I'd suspect some (potential) users will want to like to 
take a look.

-Emil

[PATCH 2/4] amdgpu: improve amdgpu_vamgr_init

[Zhou, Jammy]

Excellent. Thanks Alex.

Regards,
Jammy

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com] 
Sent: Tuesday, August 18, 2015 4:36 AM
To: Zhou, Jammy
Cc: Maling list - DRI developers; Emil Velikov
Subject: Re: [PATCH 2/4] amdgpu: improve amdgpu_vamgr_init

On Sun, Aug 16, 2015 at 11:09 PM, Jammy Zhou  wrote:
> Make it a generic function independent of the device info.
>
> Signed-off-by: Jammy Zhou 
> Reviewed-by: Christian König 
> ---

I pushed patches 2-4.

Alex


>  amdgpu/amdgpu_vamgr.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/amdgpu/amdgpu_vamgr.c b/amdgpu/amdgpu_vamgr.c index 
> cc4a1c4..71fd2b1 100644
> --- a/amdgpu/amdgpu_vamgr.c
> +++ b/amdgpu/amdgpu_vamgr.c
> @@ -46,11 +46,12 @@ int amdgpu_va_range_query(amdgpu_device_handle dev,
> return -EINVAL;
>  }
>
> -static void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, struct 
> amdgpu_device *dev)
> +static void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, uint64_t start,
> + uint64_t max, uint64_t alignment)
>  {
> -   mgr->va_offset = dev->dev_info.virtual_address_offset;
> -   mgr->va_max = dev->dev_info.virtual_address_max;
> -   mgr->va_alignment = dev->dev_info.virtual_address_alignment;
> +   mgr->va_offset = start;
> +   mgr->va_max = max;
> +   mgr->va_alignment = alignment;
>
> list_inithead(>va_holes);
> pthread_mutex_init(>bo_va_mutex, NULL); @@ -72,7 +73,9 @@ 
> drm_private struct amdgpu_bo_va_mgr * amdgpu_vamgr_get_global(struct 
> amdgpu_devi
> ref = atomic_inc_return();
>
> if (ref == 1)
> -   amdgpu_vamgr_init(, dev);
> +   amdgpu_vamgr_init(, 
> dev->dev_info.virtual_address_offset,
> + dev->dev_info.virtual_address_max,
> + 
> + dev->dev_info.virtual_address_alignment);
> return 
>  }
>
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/5] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

Oh, I really missed that. I will get it resolved next week.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Friday, August 14, 2015 9:19 PM
To: Zhou, Jammy
Cc: emil.l.velikov at gmail.com; ML dri-devel
Subject: Re: [PATCH 1/5] drm: add interface to get drm devices on the system v2

On 14/08/15 13:45, Zhou, Jammy wrote:
> That's okay. Shall we get this patch merged now if no other objections?
> 
First we should fix the funny vendor/device/etc id issue. Otherwise the API is 
bugged badly.

-Emil

[ANNOUNCE] libdrm 2.4.63

[Zhou, Jammy]

Understood. Thanks for the clarification :-)

Regards,
Jammy

-Original Message-
From: Marek Olšák [mailto:mar...@gmail.com] 
Sent: Friday, August 14, 2015 9:26 PM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org
Subject: Re: [ANNOUNCE] libdrm 2.4.63

[- xorg-announce]

Hi Jammy,

You can push it whenever you like. I can also make a new libdrm release 
whenever I like. There is no strict schedule for libdrm. If you want another 
release (e.g. Catalyst needs it, etc.), just let me know.

Libdrm releases are mostly driven by needs of other projects (e.g.
Mesa). Here I needed libdrm_amdgpu, so I had to release it, so that I can add a 
libdrm_amdgpu 2.4.63 dependency into Mesa's configure.ac.

Marek


On Fri, Aug 14, 2015 at 2:53 PM, Zhou, Jammy  wrote:
> Hi Marek,
>
> It looks like the 32bit VA support for libdrm_amdgpu in my recent series are 
> not included.  Do you think we should wait for the next release to include 
> them?
>
> Regards,
> Jammy
>
> -Original Message-
> From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On 
> Behalf Of Marek Ol?ák
> Sent: Friday, August 14, 2015 8:28 PM
> To: xorg-announce at lists.freedesktop.org; 
> dri-devel at lists.freedesktop.org
> Subject: [ANNOUNCE] libdrm 2.4.63
>
>
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
>
> Libdrm 2.4.63 has been released. It contains the new libdrm_amdgpu library.
>
> Alan Coopersmith (1):
>   include  &  directly for major() and 
> minor()
>
> Alex Deucher (10):
>   drm: consolidate common list implementations (v2)
>   drm: add util_math.h
>   drm: add libdrm_amdgpu (v7)
>   drm: add tests/amdgpu (v3)
>   amdgpu: update to the latest kernel header
>   fix configuration when amdgpu is disabled
>   fix amdgpu cunit configure test harder
>   move up cunit workaround for ubuntu/debian
>   add a note about which version of cunit is fixed for debian/ubuntu
>   radeon: add new OLAND pci id
>
> Alexandr Akulich (1):
>   libdrm/amdgpu: Fixed drm.h include.
>
> Anuj Phogat (2):
>   i965/gen9: Pass alignment as function parameter in 
> drm_intel_gem_bo_alloc_internal()
>   Set alignment value in drm_intel_add_validate_buffer()
>
> Chris Wilson (1):
>   drm: Detect no-op drmModeAtomicRequest and return early
>
> Christian König (16):
>   amdgpu: cleanup public interface v2
>   amdgpu: add public bo list interface v3
>   amdgpu: compare the primary device names instead
>   amdgpu: remove bo_vas hash table v2
>   amdgpu: add helper for VM mapping v2
>   amdgpu: stop checking flag masks
>   amdgpu: explicitly unmap GPU mapping on BO destruction
>   amdgpu: remove flink export workaround v2
>   amdgpu: cleanup VA IOCTL handling
>   amdgpu: remove pointer arithmetic from command submission
>   amdgpu: add CS dependencies v2
>   gitignore: add some generated amdgpu files
>   amdgpu: cleanup public interface style
>   amdgpu: remove reference to AMD specific error codes
>   amdgpu: use common fence structure for dependencies as well.
>   amdgpu: fix bs buffer size for vce test
>
> Emil Velikov (15):
>   configure: default --enable-valgrind to auto
>   freedreno: zero is a valid fd number, treat it as such
>   omap: zero is a valid fd number, treat it as such
>   xf86drm: fix incorrect fd comparison in drmOpenOnce{,WithType}
>   Consistently check the fd value
>   man: remove .man_fixup workaround
>   Force enable amdgpu for the dist build/check.
>   amdgpu/util_hash: hide private symbols from global namespace
>   amdgpu/util_hash_table: hide private symbols from global namespace
>   amdgpu: add a bunch of missing config.h includes
>   amdgpu: cosmetic chances in license boilerplate
>   amdgpu: squash trivial documentation typo
>   amdgpu/amdgpu_vamgr: hide private symbols from global namespace
>   amdgpu: hide the final internal functions from global namespace
>   amdgpu: add symbols check test
>
> Jack Xiao (2):
>   amdgpu: fix round down/up page size error
>   amdgpu: add zero timeout check in amdgpu_cs_query_fence_status
>
> Jammy Zhou (24):
>   amdgpu: remove active_rb_pipes from amdgpu_gpu_info
>   amdgpu: remove AMDGPU_GEM_CREATE_CPU_GTT_UC
>   amdgpu: fix 32-on-64 support (v2)
>   amdgpu: add ctx_id for wait_cs
>   amdgpu: reuse the kernel IB flags v2
>   amdgpu: validate the upper limit of virtual address v2
>   amdgpu: fix the number of IB size enums
>   amdgpu: remove unused AMDGPU_IB_RESOURCE_PRIORITY
>   amdgpu: replace alloca with calloc v2
>   amdgpu: add amdgp

[ANNOUNCE] libdrm 2.4.63

[Zhou, Jammy]

Hi Marek,

It looks like the 32bit VA support for libdrm_amdgpu in my recent series are 
not included.  Do you think we should wait for the next release to include them?

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Marek Ol?ák
Sent: Friday, August 14, 2015 8:28 PM
To: xorg-announce at lists.freedesktop.org; dri-devel at lists.freedesktop.org
Subject: [ANNOUNCE] libdrm 2.4.63


-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1


Libdrm 2.4.63 has been released. It contains the new libdrm_amdgpu library.

Alan Coopersmith (1):
  include  &  directly for major() and minor()

Alex Deucher (10):
  drm: consolidate common list implementations (v2)
  drm: add util_math.h
  drm: add libdrm_amdgpu (v7)
  drm: add tests/amdgpu (v3)
  amdgpu: update to the latest kernel header
  fix configuration when amdgpu is disabled
  fix amdgpu cunit configure test harder
  move up cunit workaround for ubuntu/debian
  add a note about which version of cunit is fixed for debian/ubuntu
  radeon: add new OLAND pci id

Alexandr Akulich (1):
  libdrm/amdgpu: Fixed drm.h include.

Anuj Phogat (2):
  i965/gen9: Pass alignment as function parameter in 
drm_intel_gem_bo_alloc_internal()
  Set alignment value in drm_intel_add_validate_buffer()

Chris Wilson (1):
  drm: Detect no-op drmModeAtomicRequest and return early

Christian König (16):
  amdgpu: cleanup public interface v2
  amdgpu: add public bo list interface v3
  amdgpu: compare the primary device names instead
  amdgpu: remove bo_vas hash table v2
  amdgpu: add helper for VM mapping v2
  amdgpu: stop checking flag masks
  amdgpu: explicitly unmap GPU mapping on BO destruction
  amdgpu: remove flink export workaround v2
  amdgpu: cleanup VA IOCTL handling
  amdgpu: remove pointer arithmetic from command submission
  amdgpu: add CS dependencies v2
  gitignore: add some generated amdgpu files
  amdgpu: cleanup public interface style
  amdgpu: remove reference to AMD specific error codes
  amdgpu: use common fence structure for dependencies as well.
  amdgpu: fix bs buffer size for vce test

Emil Velikov (15):
  configure: default --enable-valgrind to auto
  freedreno: zero is a valid fd number, treat it as such
  omap: zero is a valid fd number, treat it as such
  xf86drm: fix incorrect fd comparison in drmOpenOnce{,WithType}
  Consistently check the fd value
  man: remove .man_fixup workaround
  Force enable amdgpu for the dist build/check.
  amdgpu/util_hash: hide private symbols from global namespace
  amdgpu/util_hash_table: hide private symbols from global namespace
  amdgpu: add a bunch of missing config.h includes
  amdgpu: cosmetic chances in license boilerplate
  amdgpu: squash trivial documentation typo
  amdgpu/amdgpu_vamgr: hide private symbols from global namespace
  amdgpu: hide the final internal functions from global namespace
  amdgpu: add symbols check test

Jack Xiao (2):
  amdgpu: fix round down/up page size error
  amdgpu: add zero timeout check in amdgpu_cs_query_fence_status

Jammy Zhou (24):
  amdgpu: remove active_rb_pipes from amdgpu_gpu_info
  amdgpu: remove AMDGPU_GEM_CREATE_CPU_GTT_UC
  amdgpu: fix 32-on-64 support (v2)
  amdgpu: add ctx_id for wait_cs
  amdgpu: reuse the kernel IB flags v2
  amdgpu: validate the upper limit of virtual address v2
  amdgpu: fix the number of IB size enums
  amdgpu: remove unused AMDGPU_IB_RESOURCE_PRIORITY
  amdgpu: replace alloca with calloc v2
  amdgpu: add amdgpu_bo_list_update interface v2
  amdgpu: add IB sharing support v2
  tests/amdgpu: add shared IB submission test v2
  amdgpu: get rid of IB pool management v3
  tests/amdgpu: manage IB in client side
  amdgpu: add amdgpu_query_gds_info
  amdgpu: cleanup gds specific alloc/free functions
  amdgpu: merge amdgpu_drm.h from kernel
  amdgpu: do NULL check for bo handle in amdgpu_bo_query_info
  amdgpu: improve the amdgpu_cs_query_fence_status interface
  drm: fix the ALIGN macro to avoid value clamp
  tests/amdgpu: remove the duplicate IB allocation for VCE test
  amdgpu: add flags parameter for amdgpu_va_range_alloc
  amdgpu: add amdgpu_bo_va_op for va map/unmap support v3
  amdgpu: expose the PCI revision ID

Jonathan Gray (3):
  xf86drmMode: Implement drmCheckModesettingSupported() for OpenBSD
  xf86drm: correct the OpenBSD DRM_MAJOR define
  xf86drm: use the correct device minor names on OpenBSD

Joonyoung Shim (1):
  Build vbltest irrespective of the presence of libudev.

Julien Cristau (1):
  Fix headers inclusion in xf86drmMode.c

Ken Wang (8):
  amdgpu: make vamgr global
  amdgpu: add max_memory_clock for interface query
  amdgpu: add vram_type and vram_bit_width for 

[PATCH 1/5] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

That's okay. Shall we get this patch merged now if no other objections?

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Friday, August 14, 2015 8:29 PM
To: Zhou, Jammy
Cc: ML dri-devel
Subject: Re: [PATCH 1/5] drm: add interface to get drm devices on the system v2

On 14 August 2015 at 13:15, Zhou, Jammy  wrote:
> Okay, I got it. Actually with current implementation, only the number 
> of PCI devices on the system is returned for drmGetDevices(NULL, 0). I 
> extracted related code below. I hope it can address your concern :-)
>
Had a blond moment and got confused with the subsystem_type default statement. 
Please ignore my earlier rambling.

-Emil

[PATCH 1/5] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

Okay, I got it. Actually with current implementation, only the number of PCI 
devices on the system is returned for drmGetDevices(NULL, 0). I extracted 
related code below. I hope it can address your concern :-)

+static int drmParseSubsystemType(const char *str) {
+char link[PATH_MAX + 1] = "";
+char *name;
+
+if (readlink(str, link, PATH_MAX) < 0)
+return -EINVAL;
+
+name = strrchr(link, '/');
+if (!name)
+return -EINVAL;
+
+name++;
+
+if (strncmp(name, "pci", 3) == 0)
+return DRM_BUS_PCI;
+
+return -EINVAL;
+}
...
+subsystem_type = drmParseSubsystemType(path);
+
+if (subsystem_type < 0)
+continue;

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Friday, August 14, 2015 6:05 PM
To: Zhou, Jammy
Cc: ML dri-devel
Subject: Re: [PATCH 1/5] drm: add interface to get drm devices on the system v2

On 14 August 2015 at 10:41, Zhou, Jammy  wrote:
>> What is the point in claiming that you have X+Y devices, if the API does not 
>> provide any information about Y of them ? It seems very misleading imho.
>
> I'm not sure if I understand your question correctly.
Easy - replace X with "pci" and Y with "platform/usb" :)

Or in other words:
user: "hey libdrm, how many devices do we have"
libdrm: "hey user, there are 10 devices here."
user: "great, tell me all about them"
libdrm: "sure... well I cannot tell you anything about 3 of them, but the rest 
are fine"
user: "why did you stay that they are 10, if there is no info for 3 of them ?"

Fwiw it can be argued either way but I'd suspect that the current behaviour is 
not too welcoming.

If people feel for the current behaviour I'kk be ok with it.

Thanks
Emil

[PATCH 1/5] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

> There have been plenty of reports opened wrt libudev/libgcc_s/libstdc++ on 
> their trackers and seemingly limited interest to fix things.

Yes, there are a bunch of issues reported for this already. But it looks like 
Valve has no plan to fix these issues. For example,
https://github.com/ValveSoftware/steam-runtime/issues/13

IMHO, we can probably use sysfs first, and when the issue is solved by Valve, 
we can switch to the udev solution later.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Friday, August 14, 2015 5:08 PM
To: Kai Wasserbäch
Cc: Zhou, Jammy; Daniel Vetter; ML dri-devel
Subject: Re: [PATCH 1/5] drm: add interface to get drm devices on the system v2

On 14 August 2015 at 09:26, Kai Wasserbäch  
wrote:
> Emil Velikov wrote on 14.08.2015 10:17:
>> On 14 August 2015 at 08:59, Kai Wasserbäch  
>> wrote:
>>> Zhou, Jammy wrote on 14.08.2015 07:59:
>>>> We tried several different ways already for the enumeration interface 
>>>> (libpciaccess, libudev, etc). But we ran into some problems with these 
>>>> options for example when run Steam games which ships 32bit libraries 
>>>> (including libudev) in the steam runtime, so finally we decided to use 
>>>> sysfs directly to avoid introducing some additional dependencies into 
>>>> libdrm.
>>>
>>> The reason sounds wrong. There was a similar discussion over at 
>>> Mesa. I think you (as in hardware/driver vendors like 
>>> AMD/Intel/Nvidia) need to push Valve (or the game devs through Valve 
>>> or directly) to fix their setup. Steam runtime is fine and all, but 
>>> please only pre-load it, if needed (ie. library foo is missing on 
>>> the system and can't be installed through the package manager). IIRC 
>>> the VMWare guys said in the Mesa discussion, they have a script in 
>>> place for their virtualisation products, that checks whether a library 
>>> needs to be loaded from their "baseline directory" or from the system.
>>>
>>> Working around a bug/design flaw in Steam's Linux version doesn't 
>>> sound like a supportable solution in the long run. As long as you 
>>> let them get away with that, you will face this problem over and 
>>> over with different libraries. (For me it's usually libstdc++ 
>>> (needed by LLVM), libncurses and a few X(CB) libraries I need to 
>>> remove from Steam, before anything works. Though I do have script 
>>> for that, that I can run after every upgrade, this is not a solution 
>>> for everyone.)
>>>
>> Helping and applying pressure to resolve the issue is the way to go.
>> But until that is resolved it's great to have a solution that does 
>> not lead to a crash. It feels rude towards you and other users to 
>> deliberately use the problematic combo and expect from you to remove 
>> libfoo.so.
>
> Well, I'd rather remove stuff from Steam's runtime than burden you and 
> other developers with maintaing code that is unnecessrily ugly. 
> (Though that's obviously just my opinion.)
>
>> When things get sorted out, we can easily replace this (a tad ugly
>> implementation) with libudev.
>
> As long as you allow this behaviour by working around it,
There is a saying (roughly translated to) "The wiser man always steps back". Or 
we could/should be like Linus - "F* you $company"

> I don't see Valve/game
> developers "invest" in a real solution (because it works now). 
> Businesses usually only move from a position, when there's outside 
> pressure and a clear advantage to do so (here: no bug reports about crashing 
> games).
>
There have been plenty of reports opened wrt libudev/libgcc_s/libstdc++ on 
their trackers and seemingly limited interest to fix things.

This is a catch 20/20 afaics. "FOSS drivers do not work thus they are s**t" is 
how a sizeable hunk of people think. They rarely consider what the actual issue 
might be, because "I installed the nvidia/amd proprietary drivers and things 
work now".

> Anyway, this was just my two cents and you can obviously decide in any 
> way you deem to be the best.
>
Personally, I'd love if there was no "options" and we can just use libfoo. Who 
knows Valve devs might get a wake up call and fix the problem ?


-Emil
P.S. Fun fact: Valve's annual "TI" Dota2 tournament managed to accumulate some 
66 million USD gross income, over 100 days.

[PATCH 1/5] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

> What is the point in claiming that you have X+Y devices, if the API does not 
> provide any information about Y of them ? It seems very misleading imho.

I'm not sure if I understand your question correctly. Do you mean if the Y 
devices will be enumerated with current implementation? If so, I think the 
answer should be 'NO', since other bus types (i.e, platform, USB) are not 
supported yet.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Friday, August 14, 2015 4:35 PM
To: Zhou, Jammy
Cc: ML dri-devel
Subject: Re: [PATCH 1/5] drm: add interface to get drm devices on the system v2

On 14 August 2015 at 06:53, Zhou, Jammy  wrote:
> Hi Emil,
>
>> If there are any other devices they will still be counted when 
>> drmGetDevices(NULL, 0)... Is that intentional ?
> Yes, I think so, so that this interface can support different kinds of 
> devices in the future. For example, we have some ARM platforms supporting 
> PCIE, in which case we can connect one PCIE graphics card, then there will be 
> one GPU with the platform bus (integrated GPU in the ARM SOC), and one 
> discrete GPU on the PCIE bus.
>
What is the point in claiming that you have X+Y devices, if the API does not 
provide any information about Y of them ? It seems very misleading imho.

>> Something funny is happening here - on my intel system vendor_id is reported 
>> as 0xff86, instead of 0x8086. Subvendor/device are also messed up - ffaa and 
>> ffda instead of 17aa + 21da.
> That's really interesting. Did you try to update the system BIOS?
>
Seems like a C Programming 101 issue to me rather than a BIOS one.The
(signed) char 0x86 gets extended/promoted to 0xff86 and then all hell breaks 
loose. Adding typecast(s) should fix it. That does not excuse me from writing 
is so weird from the start :)

Thanks for tweaking/ironing the bugs out.
Emil

[PATCH 1/5] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

We tried several different ways already for the enumeration interface 
(libpciaccess, libudev, etc). But we ran into some problems with these options 
for example when run Steam games which ships 32bit libraries (including 
libudev) in the steam runtime, so finally we decided to use sysfs directly to 
avoid introducing some additional dependencies into libdrm.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Thursday, August 13, 2015 11:27 PM
To: Daniel Vetter
Cc: Zhou, Jammy; ML dri-devel
Subject: Re: [PATCH 1/5] drm: add interface to get drm devices on the system v2

On 13 August 2015 at 16:07, Daniel Vetter  wrote:
> On Thu, Aug 13, 2015 at 11:33:41AM +0800, Jammy Zhou wrote:
>> From: Emil Velikov 
>>
>> For mutiple GPU support, the devices on the system should be 
>> enumerated to get necessary information about each device, and the 
>> drmGetDevices interface is added for this. Currently only PCI devices 
>> are supported for the enumeration.
>>
>> Typical usage:
>> int count;
>> drmDevicePtr *foo;
>> count = drmGetDevices(NULL, 0);
>> foo = calloc(count, sizeof(drmDevicePtr)); count = drmGetDevices(foo, 
>> count);
>> /* find proper device, open correct device node, etc */ 
>> drmFreeDevices(foo, count); free(foo);
>>
>> v2: change according to feedback from Emil
>>
>> Signed-off-by: Jammy Zhou 
>> ---
>>  xf86drm.c | 351 
>> ++
>>  xf86drm.h |  34 ++
>>  2 files changed, 385 insertions(+)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 5e02969..237663b 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -55,6 +55,7 @@
>>  #ifdef HAVE_SYS_MKDEV_H
>>  # include  /* defines major(), minor(), and makedev() 
>> on Solaris */  #endif
>> +#include 
>>
>>  /* Not all systems have MAP_FAILED defined */  #ifndef MAP_FAILED @@ 
>> -2829,3 +2830,353 @@ char *drmGetRenderDeviceNameFromFd(int fd)  {
>>   return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);  }
>> +
>> +#ifdef __linux__
>> +static int drmParseSubsystemType(const char *str) {
>> +char link[PATH_MAX + 1] = "";
>> +char *name;
>> +
>> +if (readlink(str, link, PATH_MAX) < 0)
>> +return -EINVAL;
>> +
>> +name = strrchr(link, '/');
>> +if (!name)
>> +return -EINVAL;
>> +
>> +name++;
>> +
>> +if (strncmp(name, "pci", 3) == 0)
>> +return DRM_BUS_PCI;
>> +
>> +return -EINVAL;
>> +}
>> +
>> +static int drmParsePciBusInfo(const char *str, drmPciBusInfoPtr 
>> +info) {
>> +int domain, bus, dev, func;
>> +char *value;
>> +
>> +if (str == NULL)
>> +return -EINVAL;
>> +
>> +value = strstr(str, "PCI_SLOT_NAME=");
>> +if (value == NULL)
>> +return -EINVAL;
>> +
>> +value += strlen("PCI_SLOT_NAME=");
>> +
>> +if (sscanf(value, "%04x:%02x:%02x.%1u",
>> +   , , , ) != 4)
>> +return -EINVAL;
>> +
>> +info->domain = domain;
>> +info->bus = bus;
>> +info->dev = dev;
>> +info->func = func;
>> +
>> +return 0;
>> +}
>> +
>> +static int drmSameDevice(drmDevicePtr a, drmDevicePtr b) {
>> +if (a->bustype != b->bustype)
>> +return 0;
>> +
>> +switch (a->bustype) {
>> +case DRM_BUS_PCI:
>> +if (memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo)) 
>> == 0)
>> +return 1;
>> +default:
>> +break;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static int drmGetNodeType(const char *name) {
>> +if (strncmp(name, DRM_PRIMARY_MINOR_NAME,
>> +sizeof(DRM_PRIMARY_MINOR_NAME) - 1) == 0)
>> +return DRM_NODE_PRIMARY;
>> +
>> +if (strncmp(name, DRM_CONTROL_MINOR_NAME,
>> +sizeof(DRM_CONTROL_MINOR_NAME ) - 1) == 0)
>> +return DRM_NODE_CONTROL;
>> +
>> +if (strncmp(name, DRM_RENDER_MINOR_NAME,
>> +sizeof(DRM_RENDER_MINOR_NAME) - 1) == 0)
>> +return DRM_NODE_RENDER;
>> +
>> +return -EINVAL;
>> +}
>> +
>> +static int drmParsePciDeviceInfo(const char *config,
>> + drmPciDeviceInfoPtr device) {
>> +if (config == NULL)
>> +return -EINVAL;
>> +
>> +device->vendor_id = config[0] | (config[1] << 8);
>> +device->device_id = config[2] | (config[3] << 8);
>> +device->revision_id = config[8];
>> +device->subvendor_id = config[44] | (config[45] << 8);
>> +device->subdevice_id = config[46] | (config[47] << 8);
>
> Why not reuse libpciaccess?
I fully agree that the implementation is not pretty, although...

Adding dependencies for optional new features doesn't seem too appealing, 
either. It will also save us some headaches when someone starts shipping 
libpciaccess.so with their binary game/program (just like libudev is today).

Would be great if we can use libudev but... just not yet.

If you have any other ideas/suggestions please fire away.

Thanks
Emi

[PATCH 1/5] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

Hi Emil,

> If there are any other devices they will still be counted when 
> drmGetDevices(NULL, 0)... Is that intentional ?
Yes, I think so, so that this interface can support different kinds of devices 
in the future. For example, we have some ARM platforms supporting PCIE, in 
which case we can connect one PCIE graphics card, then there will be one GPU 
with the platform bus (integrated GPU in the ARM SOC), and one discrete GPU on 
the PCIE bus. 

> Something funny is happening here - on my intel system vendor_id is reported 
> as 0xff86, instead of 0x8086. Subvendor/device are also messed up - ffaa and 
> ffda instead of 17aa + 21da.
That's really interesting. Did you try to update the system BIOS?

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Thursday, August 13, 2015 9:58 PM
To: Zhou, Jammy
Cc: ML dri-devel
Subject: Re: [PATCH 1/5] drm: add interface to get drm devices on the system v2

On 13 August 2015 at 04:33, Jammy Zhou  wrote:
> From: Emil Velikov 
>
> For mutiple GPU support, the devices on the system should be 
> enumerated to get necessary information about each device, and the 
> drmGetDevices interface is added for this. Currently only PCI devices 
> are supported for the enumeration.
>
If there are any other devices they will still be counted when 
drmGetDevices(NULL, 0)... Is that intentional ?


> +static int drmParsePciDeviceInfo(const char *config,
> + drmPciDeviceInfoPtr device) {
> +if (config == NULL)
> +return -EINVAL;
> +
> +device->vendor_id = config[0] | (config[1] << 8);
> +device->device_id = config[2] | (config[3] << 8);
> +device->revision_id = config[8];
> +device->subvendor_id = config[44] | (config[45] << 8);
> +device->subdevice_id = config[46] | (config[47] << 8);
> +
Something funny is happening here - on my intel system vendor_id is reported as 
0xff86, instead of 0x8086. Subvendor/device are also messed up - ffaa and ffda 
instead of 17aa + 21da.

One could bikeshed on the de-duplication error path(s), but considering that 
things work and there are no leaks we can leave that for some other day.

-Emil

[PATCH 0/5] some drm and amdgpu patches

[Zhou, Jammy]

+Alex

Sure, we can squash patch #4 into #3 when push the changes.

Regards,
Jammy

-Original Message-
From: Michel Dänzer [mailto:mic...@daenzer.net] 
Sent: Thursday, August 13, 2015 2:20 PM
To: Zhou, Jammy; dri-devel at lists.freedesktop.org
Cc: Emil Velikov
Subject: Re: [PATCH 0/5] some drm and amdgpu patches


Hi Jammy,


On 13.08.2015 12:33, Jammy Zhou wrote:
> This series is a set of patches in my side pending for merge. And I 
> rebased it with the drm_private series from Emil.
> 
> Emil Velikov (1):
>   drm: add interface to get drm devices on the system v2
> 
> Jammy Zhou (4):
>   amdgpu: improve amdgpu_vamgr_init
>   amdgpu: add flag to support 32bit VA address v3
>   amdgpu: fix one warning from previous commit
>   amdgpu: make vamgr per device v2

Please squash patch 4 (and maybe patch 5 as well?) into patch 3.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

[PATCH libdrm 0/9] amdgpu: cleanup the exported symbols

[Zhou, Jammy]

The series are Reviewed-by: Jammy Zhou 

And I will rebase my pending patches on top of this series. Thanks.

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Emil Velikov
Sent: Saturday, August 08, 2015 12:45 AM
To: dri-devel at lists.freedesktop.org
Cc: Deucher, Alexander
Subject: [PATCH libdrm 0/9] amdgpu: cleanup the exported symbols

Now that the API is stabilised we can do a bit of a cleanup, and ensure only 
the symbols part of it are exported - 62 -> 34.

This also gives us some ~10% is size reduction of the final binary.

Android support would be nice, but it's not a show stopper :)

Please review,
Emil

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

[PATCH] drm: add interface to get drm devices on the system v2

[Zhou, Jammy]

Hi Emil,

Do you have chance to look at this version?

Regards,
Jammy

-Original Message-
From: Jammy Zhou [mailto:jammy.z...@amd.com] 
Sent: Thursday, August 06, 2015 8:39 PM
To: Emil Velikov
Cc: Zhou, Jammy
Subject: [PATCH] drm: add interface to get drm devices on the system v2

From: Emil Velikov <emil.l.veli...@gmail.com>

For mutiple GPU support, the devices on the system should be enumerated to get 
necessary information about each device, and the drmGetDevices interface is 
added for this. Currently only PCI devices are supported for the enumeration.

Typical usage:
int count;
drmDevicePtr *foo;
count = drmGetDevices(NULL, 0);
foo = calloc(count, sizeof(drmDevicePtr)); count = drmGetDevices(foo, count);
/* find proper device, open correct device node, etc */ drmFreeDevices(foo, 
count); free(foo);

v2: change according to feedback from Emil

Signed-off-by: Jammy Zhou 
---
 xf86drm.c | 351 ++
 xf86drm.h |  34 ++
 2 files changed, 385 insertions(+)

diff --git a/xf86drm.c b/xf86drm.c
index 5e02969..237663b 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -55,6 +55,7 @@
 #ifdef HAVE_SYS_MKDEV_H
 # include  /* defines major(), minor(), and makedev() on Solaris 
*/  #endif
+#include 

 /* Not all systems have MAP_FAILED defined */  #ifndef MAP_FAILED @@ -2829,3 
+2830,353 @@ char *drmGetRenderDeviceNameFromFd(int fd)  {
return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);  }
+
+#ifdef __linux__
+static int drmParseSubsystemType(const char *str) {
+char link[PATH_MAX + 1] = "";
+char *name;
+
+if (readlink(str, link, PATH_MAX) < 0)
+return -EINVAL;
+
+name = strrchr(link, '/');
+if (!name)
+return -EINVAL;
+
+name++;
+
+if (strncmp(name, "pci", 3) == 0)
+return DRM_BUS_PCI;
+
+return -EINVAL;
+}
+
+static int drmParsePciBusInfo(const char *str, drmPciBusInfoPtr info) {
+int domain, bus, dev, func;
+char *value;
+
+if (str == NULL)
+return -EINVAL;
+
+value = strstr(str, "PCI_SLOT_NAME=");
+if (value == NULL)
+return -EINVAL;
+
+value += strlen("PCI_SLOT_NAME=");
+
+if (sscanf(value, "%04x:%02x:%02x.%1u",
+   , , , ) != 4)
+return -EINVAL;
+
+info->domain = domain;
+info->bus = bus;
+info->dev = dev;
+info->func = func;
+
+return 0;
+}
+
+static int drmSameDevice(drmDevicePtr a, drmDevicePtr b) {
+if (a->bustype != b->bustype)
+return 0;
+
+switch (a->bustype) {
+case DRM_BUS_PCI:
+if (memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo)) == 0)
+return 1;
+default:
+break;
+}
+
+return 0;
+}
+
+static int drmGetNodeType(const char *name) {
+if (strncmp(name, DRM_PRIMARY_MINOR_NAME,
+sizeof(DRM_PRIMARY_MINOR_NAME) - 1) == 0)
+return DRM_NODE_PRIMARY;
+
+if (strncmp(name, DRM_CONTROL_MINOR_NAME,
+sizeof(DRM_CONTROL_MINOR_NAME ) - 1) == 0)
+return DRM_NODE_CONTROL;
+
+if (strncmp(name, DRM_RENDER_MINOR_NAME,
+sizeof(DRM_RENDER_MINOR_NAME) - 1) == 0)
+return DRM_NODE_RENDER;
+
+return -EINVAL;
+}
+
+static int drmParsePciDeviceInfo(const char *config,
+ drmPciDeviceInfoPtr device) {
+if (config == NULL)
+return -EINVAL;
+
+device->vendor_id = config[0] | (config[1] << 8);
+device->device_id = config[2] | (config[3] << 8);
+device->revision_id = config[8];
+device->subvendor_id = config[44] | (config[45] << 8);
+device->subdevice_id = config[46] | (config[47] << 8);
+
+return 0;
+}
+
+static void drmFreeDevice(drmDevicePtr device) {
+int i;
+
+if (device == NULL)
+return;
+
+if (device->nodes != NULL)
+for (i = 0; i < DRM_NODE_MAX; i++)
+free(device->nodes[i]);
+
+free(device->nodes);
+free(device->businfo.pci);
+free(device->deviceinfo.pci);
+}
+
+void drmFreeDevices(drmDevicePtr devices[], int count) {
+int i;
+
+if (devices == NULL)
+return;
+
+for (i = 0; i < count; i++) {
+drmFreeDevice(devices[i]);
+free(devices[i]);
+devices[i] = NULL;
+}
+}
+
+/**
+ * Get drm devices on the system
+ *
+ * \param devices the array of devices with drmDevicePtr elements
+ *can be NULL to get the device number first
+ * \param max_devices the maximum number of devices for the array
+ *
+ * \return on error - negative error code,
+ * if devices is NULL - total number of devices available on the 
system,
+ * alternatively the number of devices stored in devices[], which is
+ * capped by the max_devices.
+ */
+int drmGetDevices(drmDevicePtr devices[], int max_devices) {
+drmDevicePtr devs = NULL;
+drmPciBusInfoPtr pcibus = NULL

Device enumeration support in libdrm

[Zhou, Jammy]

Although I don't like the method of manually iterating sysfs, it seems the last 
resort if we want to avoid introducing libudev dependency.

Besides, you mentioned that you would spend some time on the device enumeration 
interface, do you have some chance to look at it?

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Thursday, July 09, 2015 12:25 AM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org
Subject: Re: Device enumeration support in libdrm

Hello Jammy,

On 8 July 2015 at 06:53, Zhou, Jammy  wrote:
> Hi Emil,
>
>
>
> With our previous RFC patches to add interfaces to support GPU device 
> enumeration support in libdrm, the udev mechanism was used. But we 
> found that if we introduce the libudev dependency for libdrm, there 
> will be some problem to run steam application on recent distributions 
> (for example, Ubuntu 14.04 LTS) because of the conflict between the 
> system libudev.so.1 (loaded by libdrm) and the libudev.so.0 (bundled 
> in steam-runtime and loaded by steam application). There are some 
> similar issues as mentioned in the links below. Although we can argue 
> that it is application’s problem, can we get rid of libudev for the 
> device enumeration before Valve can make steam-runtime use system 
> libraries by default? It is much appreciated if you can give some 
> suggestions about how we can support device enumeration with libdrm.
>
I believe I mentioned it before, perhaps I wasn't clear enough. Rather than 
using libudev, manually iterate through sysfs. I agree it's not a not pretty 
solution yet the libdrm code already has much weirder stuff.

Cheers
Emil

Device enumeration support in libdrm

[Zhou, Jammy]

Hi Emil,

With our previous RFC patches to add interfaces to support GPU device 
enumeration support in libdrm, the udev mechanism was used. But we found that 
if we introduce the libudev dependency for libdrm, there will be some problem 
to run steam application on recent distributions (for example, Ubuntu 14.04 
LTS) because of the conflict between the system libudev.so.1 (loaded by libdrm) 
and the libudev.so.0 (bundled in steam-runtime and loaded by steam 
application). There are some similar issues as mentioned in the links below. 
Although we can argue that it is application's problem, can we get rid of 
libudev for the device enumeration before Valve can make steam-runtime use 
system libraries by default? It is much appreciated if you can give some 
suggestions about how we can support device enumeration with libdrm.

https://wiki.debian.org/Steam
https://github.com/ValveSoftware/steam-runtime/issues/13

Regards,
Jammy

-- next part --
An HTML attachment was scrubbed...
URL: 

[PATCH 01/25] drm: add generic lock-free queue implementation

[Zhou, Jammy]

Thanks Daniel. I had a look at kfifo implementation, and I think it can satisfy 
our requirements as well. 

Regards,
Jammy

-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, June 15, 2015 2:39 PM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org; Liu, Shaoyun
Subject: Re: [PATCH 01/25] drm: add generic lock-free queue implementation

On Thu, Jun 04, 2015 at 06:38:21PM +0800, Jammy Zhou wrote:
> Signed-off-by: Jammy Zhou 
> Signed-off-by: Shaoyun Liu 
> Reviewed-by: Christian K?nig 

What exactly is this needed for? Is this really something that's useful in drm 
core (smells like yet another single purpose helper for just one driver)? Why 
is the one in lib/kfifo.c not good enough?
-Daniel

> ---
>  drivers/gpu/drm/Makefile|   3 +-
>  drivers/gpu/drm/drm_queue.c | 208 
> 
>  include/drm/drm_queue.h |  47 ++
>  3 files changed, 257 insertions(+), 1 deletion(-)  create mode 100644 
> drivers/gpu/drm/drm_queue.c  create mode 100644 
> include/drm/drm_queue.h
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 
> 20f81fd..173d2b6 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -14,7 +14,8 @@ drm-y   :=  drm_auth.o drm_bufs.o drm_cache.o \
>   drm_info.o drm_debugfs.o drm_encoder_slave.o \
>   drm_trace_points.o drm_global.o drm_prime.o \
>   drm_rect.o drm_vma_manager.o drm_flip_work.o \
> - drm_modeset_lock.o drm_atomic.o drm_bridge.o
> + drm_modeset_lock.o drm_atomic.o drm_bridge.o \
> + drm_queue.o
>  
>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o diff --git 
> a/drivers/gpu/drm/drm_queue.c b/drivers/gpu/drm/drm_queue.c new file 
> mode 100644 index 000..00cd649
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_queue.c
> @@ -0,0 +1,208 @@
> +/*
> + * Copyright 2015 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be 
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
> +DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
> +OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
> +OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + *
> + * This file implement a generic circular array queue.
> + * It's lock free for single producer and single consumer.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define QUEUE_IDX(_X_)  ((_X_)&(queue->size-1))
> +
> +/**
> + * drm_queue_create - create the queue
> + *
> + * @size  total number of elements for this queue
> + * @element_size the memory size of each element entry
> + *
> + * return pointer to the drm_queue structure if succeed, NULL if 
> +failed  */ struct drm_queue * drm_queue_create(uint32_t size, 
> +uint32_t element_size) {
> + struct drm_queue *queue;
> +
> + /* check whether the size is power of 2*/
> + BUG_ON(size & (size - 1));
> +
> + queue = kzalloc(sizeof(struct drm_queue), GFP_KERNEL);
> + if (!queue)
> + return NULL;
> +
> + queue->size = size;
> + queue->element_size = element_size;
> + atomic_set(>count, 0);
> + mutex_init(>lock);
> + queue->data = kzalloc(size * element_size, GFP_KERNEL);
> + if (!queue->data) {
> + kfree(queue);
> + return NULL;
> + }
> +
> + return queue;
> +}
> +
> +EXPORT_SYMBOL(drm_queue_create);
> +
> +/**
> + * drm_queue_free - free the queue storage
> + *
> + * @queue pointer to the queue need to be destroied
> + *
> + * return 0 if succeed. -1 if failed.
> + 

[PATCH] Add device enumeration interface (v3)

[Zhou, Jammy]

> Does the above cover all the things that you have planned for the interface 
> to provide ?
Yes, exactly. That can cover what we need. Thanks.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Thursday, June 04, 2015 12:04 AM
To: Zhou, Jammy
Cc: Alex Deucher; Deucher, Alexander; Min, Frank; dri-devel at 
lists.freedesktop.org
Subject: Re: [PATCH] Add device enumeration interface (v3)

Hello Jammy,

On 1 June 2015 at 03:12, Zhou, Jammy  wrote:
>> Jammy or Frank might be able to provide some pseudo code in the interim.
>
> I think the requirement here is quite simple. We would like to have some 
> interface to enumerate the GPU devices on the system, and select some 
> specific device for different purposes (i.e, rendering, computing, 
> displaying, etc). Current libdrm interfaces are just for single device, and 
> there is no good consideration for multiple GPUs yet.
>
I believe I've read the same quote earlier, although still explains the goal, 
rather than the requirements :'-( Can you elaborate on what exactly the 
interface should provide - I can think of the following:
 - Number of devices.
 - ^^ + masked by vendor id/module name. This one seem like an overkill imho..
 - Flags indicating if the device has primary/control/render node.
 - Location (bus info) for the device(s)
 - PCI/platform device information.

Does the above cover all the things that you have planned for the interface to 
provide ?

Thanks
Emil

[PATCH] Add device enumeration interface (v3)

[Zhou, Jammy]

> Jammy or Frank might be able to provide some pseudo code in the interim.

I think the requirement here is quite simple. We would like to have some 
interface to enumerate the GPU devices on the system, and select some specific 
device for different purposes (i.e, rendering, computing, displaying, etc). 
Current libdrm interfaces are just for single device, and there is no good 
consideration for multiple GPUs yet.

Regards,
Jammy

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com] 
Sent: Friday, May 29, 2015 10:09 PM
To: Emil Velikov
Cc: Zhou, Jammy; Deucher, Alexander; Min, Frank; dri-devel at 
lists.freedesktop.org
Subject: Re: [PATCH] Add device enumeration interface (v3)

On Thu, May 28, 2015 at 10:22 AM, Emil Velikov  
wrote:
> Hi Alex,
>
> On 28 May 2015 at 14:16, Alex Deucher  wrote:
>> On Thu, May 28, 2015 at 8:44 AM, Emil Velikov  
>> wrote:
>>> On 25 May 2015 at 12:18, Zhou, Jammy  wrote:
>>>> Hi Emil,
>>>>
>>>> Do you have chance to have a look at this new version? Thanks!
>>>>
>>> Hi guys sorry for the delay.
>>>
>>> Looking at my original email response it seems that I wasn't clear 
>>> enough about my concerns. They can be looked at as independent 
>>> and/or grouped up, depending on your liking:
>>>
>>> - Adding a new interface using which it's not obvious how one can 
>>> simplify/ease existing related implementations.
>>> As mentioned before, the claiming duplication that already exists is 
>>> more complete than this API.
>>>
>>> - No open-source software uses the interface.
>>> Do you have any patches in progress that convert project foo to 
>>> using the API? Otherwise is seems like an open-source project 
>>> catering after a closed source project needs.
>>
>> It's actually in preparation for upcoming open source releases.
>>
> Ack. Fair enough - the commit message did not mention any open source 
> users. The follow-up response mentioned xserver and mesa for which 
> this API doesn't seem like a good fit.
>
>>>
>>> - Adding new dependencies.
>>> Previously libpciaccess, now libudev. The former is used only by a 
>>> single function in libdrm_intel, while the latter was optional. v3 
>>> makes the use of libudev a hard requirement.
>>> Using the library in mesa has caused problems with Steam (and 
>>> possibly other binary programs/games) which ships their own version 
>>> of various libraries.
>>
>> Can you think of a better method for device enumeration?  Rolling our 
>> own seem like a waste of effort.
>>
> Implementation might be uglier (as do other parts of libdrm), but I 
> can envision an API that does not require a new dependency, plus it 
> works with platform devices. So I'm not sure if it qualifies as better 
> or not.
>
>>
>>>
>>> Now that the patch is merged, the last issue has emerged (in a 
>>> particular form). Is there any plan to convert an open-source 
>>> project to use this API ?
>>
>> The first users actually will be open source they are just not ready 
>> for public release just yet.  We are trying to reduce our internal 
>> patch queue and get stuff upstream early and thought others might be 
>> interested, but if there are concerns we can revert it for now and 
>> re-submit it when we are ready to release the relevant code.
>>
> If you can share some pseudo code on what the requirements are/how it 
> should be used I can prep an alternative solution. A one that works 
> for your usecase and existing ones. In the mean while can we revert 
> it, pretty please ?

Sure, go for it.  Jammy or Frank might be able to provide some pseudo code in 
the interim.

Alex


>
> Thanks
> Emil

[PATCH] xf86drm: remove to open the DRM device unnecessarily

[Zhou, Jammy]

Reviewed-by: Jammy Zhou 

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Joonyoung Shim
Sent: Thursday, May 28, 2015 8:58 AM
To: dri-devel at lists.freedesktop.org
Cc: emil.l.velikov at gmail.com
Subject: [PATCH] xf86drm: remove to open the DRM device unnecessarily

This is to remove to open the DRM device unnecessarily as call
drmAvailable() when name is NULL or drm_server_info is NULL in drmOpenWithType 
function.

Signed-off-by: Joonyoung Shim 
---
 xf86drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xf86drm.c b/xf86drm.c
index b5a174b..900e4b1 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -725,7 +725,7 @@ int drmOpen(const char *name, const char *busid)
  */
 int drmOpenWithType(const char *name, const char *busid, int type)  {
-if (!drmAvailable() && name != NULL && drm_server_info) {
+if (name != NULL && drm_server_info && !drmAvailable()) {
/* try to load the kernel module */
if (!drm_server_info->load_module(name)) {
drmMsg("[drm] failed to load kernel module \"%s\"\n", name);
--
1.9.1

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

[PATCH] xf86drm: fix build error by udev dependency

[Zhou, Jammy]

Thanks for catching this. Patch is Reviewed-by: Jammy Zhou 

Regards,
Jammy

-Original Message-
From: Joonyoung Shim [mailto:jy0922.s...@samsung.com] 
Sent: Thursday, May 28, 2015 8:57 AM
To: dri-devel at lists.freedesktop.org
Cc: emil.l.velikov at gmail.com; Zhou, Jammy; Deucher, Alexander; jy0922.shim 
at samsung.com
Subject: [PATCH] xf86drm: fix build error by udev dependency

The build error is introduced by commit fde496917682 ("Add device enumeration 
interface (v4)") if don't enable udev. Can solve as check UDEV dependency.

  CC   libdrm_la-xf86drm.lo
xf86drm.c:66:21: fatal error: libudev.h: No such file or directory  #include 
"libudev.h"
 ^
compilation terminated.
make[2]: *** [libdrm_la-xf86drm.lo] Error 1

Signed-off-by: Joonyoung Shim 
---
 xf86drm.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/xf86drm.c b/xf86drm.c
index b5a174b..4a31019 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -63,7 +63,9 @@

 #include "xf86drm.h"
 #include "libdrm_macros.h"
+#if defined(UDEV)
 #include "libudev.h"
+#endif

 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || 
defined(__DragonFly__)  #define DRM_MAJOR 145 @@ -2819,6 +2821,7 @@ char 
*drmGetRenderDeviceNameFromFd(int fd)
return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);  }

+#if defined(UDEV)
 /**
 * Enumerate the GPU devices on the system
 *
@@ -2917,3 +2920,4 @@ int drmGetPciDevices(drmPciDevicePtr devSet, uint16_t 
vendorId)

return drmDevCount;
 }
+#endif
--
1.9.1

[PATCH] Add device enumeration interface (v3)

[Zhou, Jammy]

Hi Emil,

Do you have chance to have a look at this new version? Thanks!

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Zhou, Jammy
Sent: Tuesday, May 19, 2015 11:31 PM
To: dri-devel at lists.freedesktop.org; emil.l.velikov at gmail.com
Cc: Min, Frank
Subject: [PATCH] Add device enumeration interface (v3)

From: frank <frank@amd.com>

v3: switch to udev/sysfs for the enumeration

Signed-off-by: Frank Min 
Reviewed-by: Christian König 
Reviewed-by: Alex Deucher 
Reviewed-by: Jammy Zhou 
---
 Makefile.am |   7 +++--
 xf86drm.c   | 103 
 xf86drm.h   |  19 +++
 3 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 13df80c..ffd334a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -95,12 +95,15 @@ SUBDIRS = \
 libdrm_la_LTLIBRARIES = libdrm.la
 libdrm_ladir = $(libdir)
 libdrm_la_LDFLAGS = -version-number 2:4:0 -no-undefined -libdrm_la_LIBADD = 
@CLOCK_LIB@
+libdrm_la_LIBADD = \
+   @CLOCK_LIB@ \
+   @LIBUDEV_LIBS@

 libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm  AM_CFLAGS = \
$(WARN_CFLAGS) \
-   $(VALGRIND_CFLAGS)
+   $(VALGRIND_CFLAGS) \
+   $(LIBUDEV_CFLAGS)

 libdrm_la_SOURCES = $(LIBDRM_FILES)

diff --git a/xf86drm.c b/xf86drm.c
index f7c45f8..d2704de 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -63,6 +63,7 @@

 #include "xf86drm.h"
 #include "libdrm_macros.h"
+#include "libudev.h"

 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || 
defined(__DragonFly__)  #define DRM_MAJOR 145 @@ -2817,3 +2818,105 @@ char 
*drmGetRenderDeviceNameFromFd(int fd)  {
return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);  }
+
+/**
+* Enumerate the GPU devices on the system
+*
+* \param devs device array set to return the device information
+* (if NULL, the number of device is returned)
+* \param vendor the vendor ID for GPU devices to list
+* (optional, if not specified, all GPU devices are returned)
+*
+* \return the number of GPU devices
+*/
+int drmGetPciDevices(drmPciDevicePtr devSet, uint16_t vendorId) {
+   struct udev *udev = NULL;
+   struct udev_enumerate *udev_enumerate;
+   struct udev_list_entry *list_entry;
+   struct udev_device *device;
+   int drmDevCount = 0;
+   int vendor = 0;
+   int devid = 0;
+   int devclass = 0;
+   int subvendor = 0;
+   int subdevid = 0;
+   int revid = 0xff;
+   int domain = 0;
+   int bus = 0;
+   int dev = 0;
+   int func = 0;
+   char config[64] = {0};
+   char node[128] = {'\0'};
+   char slot[] = ":00:00.0";
+   char *info = NULL;
+   int minor = 0xff;
+   int fd = 0;
+   int ret = 0;
+
+   udev = udev_new();
+   if (udev == NULL) {
+   fprintf(stderr, "no context\n");
+   return -EINVAL;
+   }
+   udev_enumerate = udev_enumerate_new(udev);
+   if (udev_enumerate == NULL)
+   return -EINVAL;
+   udev_enumerate_add_match_subsystem(udev_enumerate, "drm");
+   udev_enumerate_add_match_property(udev_enumerate, "DEVTYPE", 
+"drm_minor");
+
+   udev_enumerate_scan_devices(udev_enumerate);
+
+   udev_list_entry_foreach(list_entry, 
udev_enumerate_get_list_entry(udev_enumerate)) {
+   device = 
udev_device_new_from_syspath(udev_enumerate_get_udev(udev_enumerate),
+ 
udev_list_entry_get_name(list_entry));
+   if (device != NULL) {
+   info = udev_device_get_property_value(device, "MINOR");
+   if (!strcmp(info, "0")) {
+   strcpy(node, udev_device_get_syspath(device));
+   info = strstr(node, "/drm");
+   strncpy(slot, info - strlen(slot), strlen(slot));
+   if (sscanf(slot, "%4x:%2x:%2x.%1x", , , 
, ) != 4) {
+   domain = 0;
+   bus = 0;
+   dev = 0;
+   func = 0;
+   }
+   strcpy(node + strlen(node), "/device/config");
+
+   fd = open(node, O_RDONLY);
+   if (fd >= 0) {
+   ret = read(fd, config, 64);
+   if (ret == 64) {
+   vendor = config[0] + (config[1] << 8);
+   devid = config[2] + (config[3] << 8);
+   revid = config[8];
+   devclass = config[9] + (config[10] << 
8) + (config[11] << 

[PATCH 1/2] Add device enumeration interface (v2)

[Zhou, Jammy]

> 1) work for vendor A but not for B or 2) will be OK for B but will produce 
> false positives for A.
For such kind of cases, IMHO we probably can have vendor specific 
implementations.

> A trivial lookup in sysfs will be able to provide all the required 
> information, won't you agree ?
Yes, I agree. I think we can use the udev interfaces for such kind of 
enumeration by doing the match on the drm subsystem (the assumption is that the 
drm driver is loaded for the graphics device already). We will do some 
prototyping with udev for this.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Wednesday, April 29, 2015 12:18 AM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org; Min, Frank
Subject: Re: [PATCH 1/2] Add device enumeration interface (v2)

On 28 April 2015 at 04:26, Zhou, Jammy  wrote:
> Hi Emil,
>
> This interface is intended for multiple GPU support. For example, we need to 
> know how many GPU devices are available on the system (and for some specific 
> vendor) in the client drivers, and then we can select proper devices for 
> rendering/compute/etc. For current mesa and Xserver implementations, the 
> device enumeration is done separately. I think it will be helpful if we can 
> have such kind of function in libdrm core, which can also be leveraged by 
> other new APIs requiring multi GPU support.
>
Hmm I'm not sure how the proposed interface will ease either mesa or xserver's 
implementation. The former is used only for clover(opencl) and already handles 
platform devices. While for the server I believe (haven't checked) that it just 
"throws" the PCI device information to the ddx and lets the latter do its thing.

>> Any particular reason why "3D controller" (0x32000) is omitted ?
> No. For AMD cards, we currently have 0x3 and 0x38000. Is 0x32000 used by 
> Nvidia cards? If so, I think we should add it as well.
>
What I am thinking is that using heuristics such as these will either
1) work for vendor A but not for B or 2) will be OK for B but will produce 
false positives for A.

>> Using libpciaccess, will give you the number of PCI devices available on the 
>> system rather than the ones accessible - think about platform devices and/or 
>> devices without a drm driver.
> This interface is just to enumerate the PCIE GPU devices on the system. With 
> regard to which ones are accessible, we can use drmOpen/drmOpenWithType to 
> check, and I don't want to have duplicated functionalities for these 
> interfaces. And for those non-PCIE platform devices (mostly on ARM 
> platforms), this interface shouldn't be used, and instead the client drivers 
> should handle it by themselves.
>
I am against duplication, to the point that I may have alienated a person or 
two :-\ Although this function as-is won't bring much benefit to mesa/xserver 
afaict. Plus it would be nice to keep an open mind for platform world, so that 
things will just work when AMD decides to go that road. Not to mention that 
iterating through all the devices in drmOpen* just to find that the device at 
pci:X:Y provides only a primary/render node seems a bit wasteful.


A trivial lookup in sysfs will be able to provide all the required information, 
won't you agree ?


Cheers,
Emil

[PATCH 1/2] Add device enumeration interface (v2)

[Zhou, Jammy]

Hi Emil,

This interface is intended for multiple GPU support. For example, we need to 
know how many GPU devices are available on the system (and for some specific 
vendor) in the client drivers, and then we can select proper devices for 
rendering/compute/etc. For current mesa and Xserver implementations, the device 
enumeration is done separately. I think it will be helpful if we can have such 
kind of function in libdrm core, which can also be leveraged by other new APIs 
requiring multi GPU support.

> Any particular reason why "3D controller" (0x32000) is omitted ?
No. For AMD cards, we currently have 0x3 and 0x38000. Is 0x32000 used by 
Nvidia cards? If so, I think we should add it as well.

> Using libpciaccess, will give you the number of PCI devices available on the 
> system rather than the ones accessible - think about platform devices and/or 
> devices without a drm driver.
This interface is just to enumerate the PCIE GPU devices on the system. With 
regard to which ones are accessible, we can use drmOpen/drmOpenWithType to 
check, and I don't want to have duplicated functionalities for these 
interfaces. And for those non-PCIE platform devices (mostly on ARM platforms), 
this interface shouldn't be used, and instead the client drivers should handle 
it by themselves.

Regards,
Jammy

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: Tuesday, April 28, 2015 3:07 AM
To: Zhou, Jammy; dri-devel at lists.freedesktop.org
Cc: emil.l.velikov at gmail.com; Min, Frank
Subject: Re: [PATCH 1/2] Add device enumeration interface (v2)

Hi Jammy, Frank

As far as I can see you're trying to get a different version of drmGetBusid(). 
With the DRM_IOCTL_{G,S}ET_UNIQUE ioctl being lovely as it is I do see your 
point, but I'm not sure that the current design will be too useful.

Do we have any upcoming users for this new function, can you share a bit about 
the usecase ?

On 24/04/15 03:44, Jammy Zhou wrote:
> drmGetDevices interface is added to enumernate GPU devices on the 
> system
> 
> v2: rebase the code and some improvement for the coding style
> 
> Signed-off-by: Frank Min 
> Signed-off-by: Jammy Zhou  (v2)
> ---
>  Makefile.am |  3 ++-
>  xf86drm.c   | 48 
>  xf86drm.h   | 18 ++
>  3 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am index 42d3d7f..8236ed8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -89,7 +89,8 @@ SUBDIRS = \
>  libdrm_la_LTLIBRARIES = libdrm.la
>  libdrm_ladir = $(libdir)
>  libdrm_la_LDFLAGS = -version-number 2:4:0 -no-undefined 
> -libdrm_la_LIBADD = @CLOCK_LIB@
> +libdrm_la_LIBADD = @CLOCK_LIB@ \
> + @PCIACCESS_LIBS@
>  
>  libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm  AM_CFLAGS = \ diff 
> --git a/xf86drm.c b/xf86drm.c index ffc53b8..4d67861 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -63,6 +63,7 @@
>  
>  #include "xf86drm.h"
>  #include "libdrm.h"
> +#include 
>  
>  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || 
> defined(__DragonFly__)  #define DRM_MAJOR 145 @@ -2817,3 +2818,50 @@ 
> char *drmGetRenderDeviceNameFromFd(int fd)  {
>   return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);  }
> +
> +/**
> + * Enumerate the GPU devices on the system
> + *
> + * \param devs device array set to return the device information
> + * (if NULL, the number of device is returned)
> + * \param vendor the vendor ID for GPU devices to list
> + * (optional, if not specified, all GPU devices are returned)
> + *
> + * \return the number of GPU devices
> + */
> +int drmGetDevices(drmDevicePtr devs, uint16_t vendor) {
> + struct pci_device_iterator * iter;
> + struct pci_device * dev;
> + uint32_t count = 0;
> +
> + if (pci_system_init())
> + return -EINVAL;
> +
> + iter = pci_slot_match_iterator_create(NULL);
> + if (!iter)
> + return -EINVAL;
> +
> + while ((dev = pci_device_next(iter))) {
> + if (((dev->device_class == 0x3) ||
> + (dev->device_class == 0x38000)) &&
Any particular reason why "3D controller" (0x32000) is omitted ?

> + ((vendor == 0) || (dev->vendor_id == vendor))){
> + if (devs) {
> + devs[count].domain = dev->domain;
> + devs[count].bus = dev->bus;
> + devs[count].dev = dev->dev;
> + devs[count].func = dev->func;
> + devs[count].vendor_id = dev->vendor_id;
> + devs[count].device_id = 

[PATCH 2/2] Fix one warning

[Zhou, Jammy]

Thanks for sharing the background. For [0], you mentioned that get_perms may 
return -1 in some cases for the group, can you help indicate which case it is?

Since the drmSetServerInfo is seldom used, maybe we can just do the 'int' cast 
at this moment. I will send v2 for this.

Regards,
Jammy

-Original Message-
From: Jan Vesely [mailto:jv...@scarletmail.rutgers.edu] On Behalf Of Jan Vesely
Sent: Friday, April 24, 2015 10:30 PM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org; Min, Frank
Subject: Re: [PATCH 2/2] Fix one warning

On Fri, 2015-04-24 at 11:44 +0800, Jammy Zhou wrote:
> xf86drm.c:356:2: warning: comparison of unsigned expression >= 0 is always 
> true [-Wtype-limits]
>   group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
>   ^
> 
> Signed-off-by: Jammy Zhou 
> ---
>  xf86drm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 4d67861..fbda2aa 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -353,7 +353,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
>  }
>  
>  if (drm_server_info) {
> - group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
> + group = serv_group;

I don't think this change is correct. get_perms can return errors as negative 
values. I found that xserver does, see [0] for my take on fixing this, as well 
as Emil's response [1].

I think changing the condition to:
((int)serv_group >= 0)

should be ok(I don't think there are systems with GID_MAX greater than 2^31-1), 
but I never bothered sending v2.

jan


[0]http://lists.freedesktop.org/archives/dri-devel/2015-February/077276.html
[1]http://lists.freedesktop.org/archives/dri-devel/2015-February/078171.html



>   chown_check_return(buf, user, group);
>   chmod(buf, devmode);
>  }


--
Jan Vesely 

Initial amdgpu driver release

[Zhou, Jammy]

Hi Alex,

For the core driver patch:

+config DRM_AMDGPU
+   tristate "AMD GPU"
+   depends on DRM && PCI
+   select FB_CFB_FILLRECT
+   select FB_CFB_COPYAREA
+   select FB_CFB_IMAGEBLIT
+   select FW_LOADER
+select DRM_KMS_HELPER
+   select DRM_KMS_FB_HELPER
+select DRM_TTM
+   select POWER_SUPPLY
+   select HWMON
+   select BACKLIGHT_CLASS_DEVICE
+   select DRM_AMD_GNB_BUS
+   select INTERVAL_TREE

I think DRM_AMD_GNB_BUS is not used, we can probably remove it now.

+/* TODO: Here are things that needs to be done :
+ * - surface allocator & initializer : (bit like scratch reg) should
+ *   initialize HDP_ stuff on RS600, R600, R700 hw, well anythings
+ *   related to surface
+ * - WB : write back stuff (do it bit like scratch reg things)
+ * - Vblank : look at Jesse's rework and what we should do
+ * - r600/r700: gart & cp
+ * - cs : clean cs ioctl use bitmap & things like that.
+ * - power management stuff
+ * - Barrier in gart code
+ * - Unmappabled vram ?
+ * - TESTING, TESTING, TESTING
+ */
+
+/* Initialization path:
+ *  We expect that acceleration initialization might fail for various
+ *  reasons even thought we work hard to make it works on most
+ *  configurations. In order to still have a working userspace in such
+ *  situation the init path must succeed up to the memory controller
+ *  initialization point. Failure before this point are considered as
+ *  fatal error. Here is the init callchain :
+ *  amdgpu_device_init  perform common structure, mutex initialization
+ *  asic_init   setup the GPU memory layout and perform all
+ *  one time initialization (failure in this
+ *  function are considered fatal)
+ *  asic_startupsetup the GPU acceleration, in order to
+ *  follow guideline the first thing this
+ *  function should do is setting the GPU
+ *  memory controller (only MC setup failure
+ *  are considered as fatal)
+ */
+
These should be outdated, and I think they can be removed now.

For the uapi header patch:

+#define AMDGPU_TILING_MACRO0x1
+#define AMDGPU_TILING_MICRO0x2
+#define AMDGPU_TILING_SWAP_16BIT   0x4
+#define AMDGPU_TILING_R600_NO_SCANOUT  AMDGPU_TILING_SWAP_16BIT
+#define AMDGPU_TILING_SWAP_32BIT   0x8
+/* this object requires a surface when mapped - i.e. front buffer */
+#define AMDGPU_TILING_SURFACE  0x10
+#define AMDGPU_TILING_MICRO_SQUARE 0x20
+#define AMDGPU_TILING_EG_BANKW_SHIFT   8
+#define AMDGPU_TILING_EG_BANKW_MASK0xf
+#define AMDGPU_TILING_EG_BANKH_SHIFT   12
+#define AMDGPU_TILING_EG_BANKH_MASK0xf
+#define AMDGPU_TILING_EG_MACRO_TILE_ASPECT_SHIFT   16
+#define AMDGPU_TILING_EG_MACRO_TILE_ASPECT_MASK0xf
+#define AMDGPU_TILING_EG_TILE_SPLIT_SHIFT  24
+#define AMDGPU_TILING_EG_TILE_SPLIT_MASK   0xf
+#define AMDGPU_TILING_EG_STENCIL_TILE_SPLIT_SHIFT  28
+#define AMDGPU_TILING_EG_STENCIL_TILE_SPLIT_MASK   0xf
..
+#define SI_TILE_MODE_COLOR_LINEAR_ALIGNED  8
+#define SI_TILE_MODE_COLOR_1D  13
+#define SI_TILE_MODE_COLOR_1D_SCANOUT  9
+#define SI_TILE_MODE_COLOR_2D_8BPP 14
+#define SI_TILE_MODE_COLOR_2D_16BPP15
+#define SI_TILE_MODE_COLOR_2D_32BPP16
+#define SI_TILE_MODE_COLOR_2D_64BPP17
+#define SI_TILE_MODE_COLOR_2D_SCANOUT_16BPP11
+#define SI_TILE_MODE_COLOR_2D_SCANOUT_32BPP12
+#define SI_TILE_MODE_DEPTH_STENCIL_1D  4
+#define SI_TILE_MODE_DEPTH_STENCIL_2D  0
+#define SI_TILE_MODE_DEPTH_STENCIL_2D_2AA  3
+#define SI_TILE_MODE_DEPTH_STENCIL_2D_4AA  3
+#define SI_TILE_MODE_DEPTH_STENCIL_2D_8AA  2
+
+#define CIK_TILE_MODE_DEPTH_STENCIL_1D 5

It looks these definitions are not used by libdrm_amdgpu anymore (and even by 
the kernel driver). Maybe we can remove the unused definitions, and move the 
used ones to amdgpu.h instead. Besides, I think we'd better remove 
'R600/EG/SI/CIK' from the naming.

Other than the comments above, the kernel series are Acked-by: Jammy Zhou 


Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Alex Deucher
Sent: Tuesday, April 21, 2015 6:34 AM
To: Maling list - DRI developers; mesa-dev at lists.freedesktop.org
Cc: Deucher, Alexander
Subject: Initial amdgpu driver release

I'm pleased to announce the initial release of the new amdgpu driver.
This is a partial replacement for the radeon driver for newer AMD asics.  A 
number of components are still shared.  Here is a comparison of the radeon and 
amdgpu 

[PATCH] drm/ttm: device address space != CPU address space

[Zhou, Jammy]

Patch is Reviewed-by: Jammy Zhou 

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Alex Deucher
Sent: Wednesday, March 04, 2015 1:19 PM
To: dri-devel at lists.freedesktop.org
Cc: Deucher, Alexander; thellstrom at vmware.com
Subject: [PATCH] drm/ttm: device address space != CPU address space

We need to store device offsets in 64 bit as the device address space may be 
larger than the CPU's.

Fixes GPU init failures on radeons with 4GB or more of vram on 32 bit kernels.  
We put vram at the start of the GPU's address space so the gart aperture starts 
at 4 GB causing all GPU addresses in the gart aperture to get truncated.

bug:
https://bugs.freedesktop.org/show_bug.cgi?id=89072

Signed-off-by: Alex Deucher 
Cc: thellstrom at vmware.com
---
Untested.  Someone more familiar with the ttm internals can probably point out 
any additional bits I missed.  Heading to bed now.

 drivers/gpu/drm/ttm/ttm_bo.c| 2 +-
 include/drm/ttm/ttm_bo_api.h| 2 +-
 include/drm/ttm/ttm_bo_driver.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 
d395b0b..8d9b7de 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -74,7 +74,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, 
int mem_type)
pr_err("has_type: %d\n", man->has_type);
pr_err("use_type: %d\n", man->use_type);
pr_err("flags: 0x%08X\n", man->flags);
-   pr_err("gpu_offset: 0x%08lX\n", man->gpu_offset);
+   pr_err("gpu_offset: 0x%08llX\n", man->gpu_offset);
pr_err("size: %llu\n", man->size);
pr_err("available_caching: 0x%08X\n", man->available_caching);
pr_err("default_caching: 0x%08X\n", man->default_caching);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 
0ccf7f2..c768ddf 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -249,7 +249,7 @@ struct ttm_buffer_object {
 * either of these locks held.
 */

-   unsigned long offset;
+   uint64_t offset; /* GPU address space is independent of CPU word size 
+*/
uint32_t cur_placement;

struct sg_table *sg;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h 
index 142d752..813042c 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -277,7 +277,7 @@ struct ttm_mem_type_manager {
bool has_type;
bool use_type;
uint32_t flags;
-   unsigned long gpu_offset;
+   uint64_t gpu_offset; /* GPU address space is independent of CPU word 
+size */
uint64_t size;
uint32_t available_caching;
uint32_t default_caching;
--
1.8.3.1

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

[PATCH 1/2] Add new drmOpenWithType function (v3)

[Zhou, Jammy]

Thanks for the comments, Frank. I will send out the updated patch later.

Regards,
Jammy

-Original Message-
From: Frank Binns [mailto:frank.bi...@imgtec.com] 
Sent: Wednesday, February 11, 2015 1:04 AM
To: Zhou, Jammy; dri-devel at lists.freedesktop.org
Subject: Re: [PATCH 1/2] Add new drmOpenWithType function (v3)

On 02/02/15 10:06, Jammy Zhou wrote:
> v2: Add drmGetMinorBase, and call drmOpenWithType in drmOpen
> v3: Pass 'type' to drmOpenByBusid and drmOpenDevice in drmOpenByName
>
> Signed-off-by: Jammy Zhou 
> ---
>  xf86drm.c | 63 
> ---
>  xf86drm.h |  9 -
>  2 files changed, 56 insertions(+), 16 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 345325a..810edfa 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -85,10 +85,6 @@
>  
>  #define DRM_MSG_VERBOSITY 3
>  
> -#define DRM_NODE_CONTROL 0
> -#define DRM_NODE_PRIMARY 1
> -#define DRM_NODE_RENDER 2
> -
>  static drmServerInfoPtr drm_server_info;
>  
>  void drmSetServerInfo(drmServerInfoPtr info) @@ -493,11 +489,24 @@ 
> int drmAvailable(void)
>  return retval;
>  }
>  
> +static int drmGetMinorBase(int type)
> +{
> +switch (type) {
> +case DRM_NODE_PRIMARY:
> +default:
> +return 0;
> +case DRM_NODE_CONTROL:
> +return 64;
> +case DRM_NODE_RENDER:
> +return 128;
> +};
Wouldn't it be more sensible to return -1 in the default case given that 
there's no validation of the 'type' argument in drmOpenWithType? It feels wrong 
defaulting to the primary node in this case.

> +}
>  
>  /**
>   * Open the device by bus ID.
>   *
>   * \param busid bus ID.
> + * \param type device node type.
>   *
>   * \return a file descriptor on success, or a negative value on error.
>   *
> @@ -507,16 +516,17 @@ int drmAvailable(void)
>   *
>   * \sa drmOpenMinor() and drmGetBusid().
>   */
> -static int drmOpenByBusid(const char *busid)
> +static int drmOpenByBusid(const char *busid, int type)
>  {
>  inti, pci_domain_ok = 1;
>  intfd;
>  const char *buf;
>  drmSetVersion sv;
> +intbase = drmGetMinorBase(type);
>  
>  drmMsg("drmOpenByBusid: Searching for BusID %s\n", busid);
> -for (i = 0; i < DRM_MAX_MINOR; i++) {
> - fd = drmOpenMinor(i, 1, DRM_NODE_PRIMARY);
> +for (i = base; i < base + DRM_MAX_MINOR; i++) {
> + fd = drmOpenMinor(i, 1, type);
>   drmMsg("drmOpenByBusid: drmOpenMinor returns %d\n", fd);
>   if (fd >= 0) {
>   /* We need to try for 1.4 first for proper PCI domain support @@ 
> -556,6 +566,7 @@ static int drmOpenByBusid(const char *busid)
>   * Open the device by name.
>   *
>   * \param name driver name.
> + * \param type the device node type.
>   * 
>   * \return a file descriptor on success, or a negative value on error.
>   *
> @@ -566,19 +577,20 @@ static int drmOpenByBusid(const char *busid)
>   * 
>   * \sa drmOpenMinor(), drmGetVersion() and drmGetBusid().
>   */
> -static int drmOpenByName(const char *name)
> +static int drmOpenByName(const char *name, int type)
>  {
>  int   i;
>  int   fd;
>  drmVersionPtr version;
>  char *id;
> +int   base = drmGetMinorBase(type);
>  
>  /*
>   * Open the first minor number that matches the driver name and isn't
>   * already in use.  If it's in use it will have a busid assigned already.
>   */
> -for (i = 0; i < DRM_MAX_MINOR; i++) {
> - if ((fd = drmOpenMinor(i, 1, DRM_NODE_PRIMARY)) >= 0) {
> +for (i = base; i < base + DRM_MAX_MINOR; i++) {
> + if ((fd = drmOpenMinor(i, 1, type)) >= 0) {
>   if ((version = drmGetVersion(fd))) {
>   if (!strcmp(version->name, name)) {
>   drmFreeVersion(version);
> @@ -620,9 +632,9 @@ static int drmOpenByName(const char *name)
>   for (devstring = ++pt; *pt && *pt != ' '; ++pt)
>   ;
>   if (*pt) { /* Found busid */
> - return drmOpenByBusid(++pt);
> + return drmOpenByBusid(++pt, type);
>   } else { /* No busid */
> - return drmOpenDevice(strtol(devstring, NULL, 0),i, 
> DRM_NODE_PRIMARY);
> + return drmOpenDevice(strtol(devstring, NULL, 0),i, 
> type);
>   }
>   }
>   }
> @@ -652,8 +664,29 @@ static int drmOpenByName(const char *name)
>   */
>  int drmOpen(const char *name, const char *busid)  {
> +  

[RFC PATCH 0/2] Add drmOpenWithType and drmOpenOnceWithType

[Zhou, Jammy]

Ping... Any comments on these patches?

Regards,
Jammy

-Original Message-
From: Jammy Zhou [mailto:jammy.z...@amd.com] 
Sent: Monday, February 02, 2015 6:05 PM
To: dri-devel at lists.freedesktop.org
Cc: Zhou, Jammy
Subject: [RFC PATCH 0/2] Add drmOpenWithType and drmOpenOnceWithType

For DRM render nodes, they are opened with the 'open' system call directly by 
components such as mesa, etc now, and the minor number should be specified with 
the new drmOpenRender function.

The following two patches add two new functions to open DRM device with the 
specified node type, and the minor number is transparent to the callers.

Jammy Zhou (2):
  Add new drmOpenWithType function (v3)
  Add new drmOpenOnceWithType function (v2)

 xf86drm.c | 75 ---
 xf86drm.h | 10 -
 2 files changed, 67 insertions(+), 18 deletions(-)

--
1.9.1

[PATCH 3/3] drm/amdkfd: Don't create BUG due to incorrect user parameter

[Zhou, Jammy]

The series are Reviewed-by: Jammy Zhou 

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Gabbay, Oded
Sent: Thursday, January 29, 2015 4:35 PM
To: dri-devel at lists.freedesktop.org
Subject: [PATCH 3/3] drm/amdkfd: Don't create BUG due to incorrect user 
parameter

This patch changes a BUG_ON() statement to pr_debug, in case the user tries to 
update a non-existing queue.

Signed-off-by: Oded Gabbay 
Reviewed-by: Ben Goz 
---
 drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index f37cf5e..2fda1927 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -315,7 +315,11 @@ int pqm_update_queue(struct process_queue_manager *pqm, 
unsigned int qid,
BUG_ON(!pqm);

pqn = get_queue_by_qid(pqm, qid);
-   BUG_ON(!pqn);
+   if (!pqn) {
+   pr_debug("amdkfd: No queue %d exists for update operation\n",
+   qid);
+   return -EFAULT;
+   }

pqn->q->properties.queue_address = p->queue_address;
pqn->q->properties.queue_size = p->queue_size;
--
1.9.1

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

[PATCH v2 0/3] Fix bugs related to init pipelines

[Zhou, Jammy]

The series are Reviewed-by: Jammy Zhou 

Regards,
Jammy

-Original Message-
From: Gabbay, Oded 
Sent: Thursday, January 22, 2015 6:59 PM
To: dri-devel at lists.freedesktop.org; Zhou, Jammy
Cc: alexdeucher at gmail.com
Subject: [PATCH v2 0/3] Fix bugs related to init pipelines

Following comments on original patch, I prepared this patch-set to address all 
the issues that were brought up.

Oded

Oded Gabbay (3):
  drm/radeon: Don't increment pipe_id in kgd_init_pipeline
  drm/amdkfd: Fix bug in pipelines initialization
  drm/amdkfd: Fix bug in call to init_pipelines()

 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 ++--
 drivers/gpu/drm/radeon/radeon_kfd.c   | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

--
1.9.1

[PATCH 3/3] drm/amdkfd: Handle case of invalid queue type

[Zhou, Jammy]

Patches are Reviewed-by: Jammy Zhou 

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Gabbay, Oded
Sent: Thursday, January 22, 2015 5:42 PM
To: dri-devel at lists.freedesktop.org
Subject: [PATCH 3/3] drm/amdkfd: Handle case of invalid queue type

This patch handles a case where amdkfd tries to destroy a queue but the queue 
type is invalid.
This case occurs in non-HWS path.

Signed-off-by: Oded Gabbay 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 85387c8..99e2dbb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -301,6 +301,11 @@ static int destroy_queue_nocpsch(struct 
device_queue_manager *dqm,
}
dqm->sdma_queue_count--;
deallocate_sdma_queue(dqm, q->sdma_id);
+   } else {
+   pr_debug("q->properties.type is invalid (%d)\n",
+   q->properties.type);
+   retval = -EINVAL;
+   goto out;
}

retval = mqd->destroy_mqd(mqd, q->mqd,
--
1.9.1

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

[PATCH] drm/amdkfd: Fix bug in pipelines initialization

[Zhou, Jammy]

Hi Oded,

-   pipe_hpd_addr = dqm->pipelines_addr + i * CIK_HPD_EOP_BYTES;
+   pipe_hpd_addr = dqm->pipelines_addr + inx * CIK_HPD_EOP_BYTES;
I think 'i' should still be used here, because it is the real index in the 
buffer

Besides, for the code below in init_scheduler(), it looks like the 
KFD_DQM_FIRST_PIPE is not correct, and we probably need to use 
get_first_pipe(dqm) instead.

retval = init_pipelines(dqm, get_pipes_num(dqm), KFD_DQM_FIRST_PIPE);

Regards,
Jammy

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Gabbay, Oded
Sent: Thursday, January 22, 2015 5:07 PM
To: dri-devel at lists.freedesktop.org
Subject: [PATCH] drm/amdkfd: Fix bug in pipelines initialization

This patch fixes a bug when calling to init_pipeline() interface.
The index that was passed to that function didn't take into account the 
first_pipe value, which represents the first pipe index that is under amdkfd's 
responsibility.

Signed-off-by: Oded Gabbay 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index b9626ae..fbb353f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -565,10 +565,10 @@ static int init_pipelines(struct device_queue_manager 
*dqm,

for (i = 0; i < pipes_num; i++) {
inx = i + first_pipe;
-   pipe_hpd_addr = dqm->pipelines_addr + i * CIK_HPD_EOP_BYTES;
+   pipe_hpd_addr = dqm->pipelines_addr + inx * CIK_HPD_EOP_BYTES;
pr_debug("kfd: pipeline address %llX\n", pipe_hpd_addr);
/* = log2(bytes/4)-1 */
-   kfd2kgd->init_pipeline(dqm->dev->kgd, i,
+   kfd2kgd->init_pipeline(dqm->dev->kgd, inx,
CIK_HPD_EOP_BYTES_LOG2 - 3, pipe_hpd_addr);
}

--
1.9.1

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

[PATCH] reservation: wait only with non-zero timeout specified (v3)

[Zhou, Jammy]

Sure. I will send the patches out later.

Regards,
Jammy

-Original Message-
From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] 
Sent: Wednesday, January 21, 2015 5:12 PM
To: Zhou, Jammy; Daniel Vetter
Cc: Koenig, Christian; dri-devel; Deucher, Alexander
Subject: Re: [PATCH] reservation: wait only with non-zero timeout specified (v3)

Hey,

Op 14-01-15 om 03:16 schreef Zhou, Jammy:
>>> I think it would be best to leave timeout=0 returning 0. Not handling it 
>>> differently gives the same semantics as used by fence_wait_time and 
>>> wait_event_timeout.
>>> Are there really many cases in which you don't know if timeout = 0 before 
>>> or not?
>> Yeah I think with this it's more important to be consistent with all the 
>> other wait_something primitives the kernel exposes.
> Okay. I think we can live with that from driver perspective by handling 
> timeout==0 and timeout>0 differently. 
> But it should still be worth adding some notes for 
> reservation_object_wait_timeout_rcu that  the return value cannot be used to 
> judge if the fences are signaled or not when timeout==0.
>
Oops it looks like I was wrong here..

Looking more closely at wait_event_timeout, ___wait_cond_timeout modifies __ret 
which makes it explicitly handle timeout = 0 by testing.

If you resend your patch I will ack it, but can you send a patch for fixing 
fence_wait_timeout too to clear any possible confusion?

~Maarten

[PATCH] reservation: wait only with non-zero timeout specified (v3)

[Zhou, Jammy]

>> I think it would be best to leave timeout=0 returning 0. Not handling it 
>> differently gives the same semantics as used by fence_wait_time and 
>> wait_event_timeout.
>> Are there really many cases in which you don't know if timeout = 0 before or 
>> not?

>Yeah I think with this it's more important to be consistent with all the other 
>wait_something primitives the kernel exposes.

Okay. I think we can live with that from driver perspective by handling 
timeout==0 and timeout>0 differently. 
But it should still be worth adding some notes for 
reservation_object_wait_timeout_rcu that  the return value cannot be used to 
judge if the fences are signaled or not when timeout==0.

Regards,
Jammy

[PATCH] reservation: wait only with non-zero timeout specified (v3)

[Zhou, Jammy]

> but where do you need this?
With the new upcoming amdgpu driver, we would like to use 'timeout==0' to check 
if the fences are signaled or not (without waiting). 

Besides, the jiffies accuracy is not enough for UMDs if the specified timeout 
is nsecs or usecs, and then nsecs_to_jiffies/usecs_to_jiffies may return zero. 
So the 'timeout==0' case is easy to be hit for 
reservation_object_wait_timeout_rcu.

Regards,
Jammy

-Original Message-
From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] 
Sent: Tuesday, January 13, 2015 5:04 PM
To: Zhou, Jammy; dri-devel at lists.freedesktop.org
Cc: Deucher, Alexander; Koenig, Christian
Subject: Re: [PATCH] reservation: wait only with non-zero timeout specified (v3)

Op 13-01-15 om 10:36 schreef Jammy Zhou:
> When the timeout value passed to reservation_object_wait_timeout_rcu
> is zero, no wait should be done if the fences are not signaled.
>
> Return '1' for idle and '0' for busy if the specified timeout is '0'
> to keep consistent with the case of non-zero timeout.
>
> v2: call fence_put if not signaled in the case of timeout==0
>
>
Looks more sane, but where do you need this?

~Maarten

[PATCH 1/1] reservation: wait only with non-zero timeout specified (v2)

[Zhou, Jammy]

> You can't anyway when calling with timeout = 0.
For consistency, we can always return 0 for busy (not signaled), and return 
'>0' for idle (signaled). This rule should be common for 
reservation_object_wait_timeout_rcu. So in my previous patch, '1' is returned 
for signaled case when specified timeout is zero.

> Since you have 0 returning jiffies, you would get 0 regardless of fence_wait 
> being succesful or not.
IMHO, fence_wait_timeout shouldn't be called per se when timeout==0.

> Why do you need it to return 1? Why not use 
> reservation_object_test_signaled_rcu directly?
Just as I mentioned above, return 1 is to make the returning values of 
reservation_object_wait_timeout_rcu consistent for both cases. And we can call 
reservation_object_test_signaled_rcu directly in driver side, but it will be 
better if we can also add 'timeout==0' support in 
reservation_object_wait_timeout_rcu (sometimes it isn't preventable).

Regards,
Jammy

-Original Message-
From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] 
Sent: Tuesday, January 13, 2015 4:05 PM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org; Christian König; Deucher, Alexander
Subject: Re: [PATCH 1/1] reservation: wait only with non-zero timeout specified 
(v2)

Op 13-01-15 om 08:59 schreef Zhou, Jammy:
> Hi Maarten,
>
>> Can't you simply add if (!timeout) return 
>> !reservation_object_test_signaled_rcu(obj, wait_all); to the beginning 
>> instead?
> Hmm, after looking into it, I think that can achieve the same purpose. I will 
> update the patch with this.
>
>> Also why do you need this? Why not simply return 0 with timeout = 0.
> The major purpose here is to use reservation_object_wait_timeout_rcu to 
> handle all possible timeout values (and just to check status with 
> timeout==0). If we simply return 0, we cannot determine the fence is signaled 
> or not with the return value.
You can't anyway when calling with timeout = 0.

 * fence_wait_timeout - sleep until the fence gets signaled
 *
 * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
 * remaining timeout in jiffies on success. Other error values may be
 * returned on custom implementations.

Since you have 0 returning jiffies, you would get 0 regardless of fence_wait 
being succesful or not.

I think the only right way to handle this is by returning 0 immediately if 
timeout is 0.

Why do you need it to return 1? Why not use 
reservation_object_test_signaled_rcu directly?

~Maarten

[PATCH 1/1] reservation: wait only with non-zero timeout specified (v2)

[Zhou, Jammy]

Hi Maarten,

> Can't you simply add if (!timeout) return 
> !reservation_object_test_signaled_rcu(obj, wait_all); to the beginning 
> instead?
Hmm, after looking into it, I think that can achieve the same purpose. I will 
update the patch with this.

> Also why do you need this? Why not simply return 0 with timeout = 0.
The major purpose here is to use reservation_object_wait_timeout_rcu to handle 
all possible timeout values (and just to check status with timeout==0). If we 
simply return 0, we cannot determine the fence is signaled or not with the 
return value.

Regards,
Jammy

-Original Message-
From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] 
Sent: Tuesday, January 13, 2015 2:50 PM
To: Zhou, Jammy
Cc: dri-devel at lists.freedesktop.org; Christian König; Deucher, Alexander
Subject: Re: [PATCH 1/1] reservation: wait only with non-zero timeout specified 
(v2)

Hey,

Can't you simply add if (!timeout) return 
!reservation_object_test_signaled_rcu(obj, wait_all); to the beginning instead?
Waiting with timeout = 0 is not really defined. Look at fence_default_wait for 
example. It returns timeout if the fence is signaled, but since this is 0 you 
can't distinguish between timed out wait and succesful wait.

Also why do you need this? Why not simply return 0 with timeout = 0.

~Maarten

On 13-01-15 06:50, Jammy Zhou wrote:
> When the timeout value passed to reservation_object_wait_timeout_rcu
> is zero, no wait should be done if the fences are not signaled.
> 
> Return '1' for idle and '0' for busy if the specified timeout is '0'
> to keep consistent with the case of non-zero timeout.
> 
> v2: call fence_put if not signaled in the case of timeout==0
> 
> Signed-off-by: Jammy Zhou 
> Reviewed-by: Christian König 
> Reviewed-by: Alex Deucher 
> ---
>  drivers/dma-buf/reservation.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/reservation.c 
> b/drivers/dma-buf/reservation.c index 3c97c8f..b1d554f 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -380,12 +380,19 @@ retry:
>   }
>  
>   rcu_read_unlock();
> - if (fence) {
> + if (fence && timeout) {
>   ret = fence_wait_timeout(fence, intr, ret);
>   fence_put(fence);
>   if (ret > 0 && wait_all && (i + 1 < shared_count))
>   goto retry;
>   }
> +
> + if (fence && !timeout)
> + fence_put(fence);
> +
> + if (!fence && !timeout)
> + ret = 1;
> +
>   return ret;
>  
>  unlock_retry:
>