Re: Is LLVM 13 (git) really ready for testing/development? libclc didn't compile
One more update. without changing any cmake files the following cmdline should work: cmake ../llvm-project/libclc/ -DLLVM_CONFIG=/usr/local/llvm-git/bin/llvm-config -DCMAKE_LLAsm_FLAGS=-cl-no-stdinc -DCMAKE_CLC_FLAGS=-cl-no-stdinc Jan On Wed, Mar 10, 2021 at 1:20 AM Jan Vesely wrote: > hi, > > sorry the option is actually -cl-no-stdinc. you can add it to > 'target_compiler_options'. > I should have a patch ready soon(tm), but time is scarce. > > Jan > > On Sun, Mar 7, 2021 at 9:51 PM Dieter Nützel wrote: > >> Hello Jan, >> >> I very much appreciate your advice. >> Tried several places... >> ...where to put it? >> >> Dieter >> >> Am 06.03.2021 17:56, schrieb Jan Vesely: >> > Not Marek, but hope this answer will help. >> > libclc uses clang CLC preprocessor on .ll files, llvm/clang-13 started >> > including clc declarations by default (clang >> > cf3ef15a6ec5e5b45c6c54e8fbe3769255e815ce), >> > thus corrupting any .ll assembly files that are used by libclc. >> > Inclusion of the default declarations can be turned off using a >> > cmdline switch but that remains to be implemented in the libclc build >> > system. >> > manually adding '-cl-no-stdinc' should work as a workaround. >> > >> > Jan >> > >> > On Thu, Mar 4, 2021 at 10:27 PM Dieter Nützel >> > wrote: >> > >> >> Hello Marek, >> >> >> >> can't compile anything, here. >> >> Poor Intel Nehalem X3470. >> >> >> >> Trying LLVM 12-rc2 later. >> >> >> >> Greetings, >> >> Dieter >> >> >> >> llvm-project/libclc> cd build && cmake ../ >> >> -- The CXX compiler identification is GNU 10.2.1 >> >> -- Detecting CXX compiler ABI info >> >> -- Detecting CXX compiler ABI info - done >> >> -- Check for working CXX compiler: /usr/bin/c++ - skipped >> >> -- Detecting CXX compile features >> >> -- Detecting CXX compile features - done >> >> LLVM version: 13.0.0git >> >> LLVM system libs: >> >> LLVM libs: -lLLVM-13git >> >> LLVM libdir: /usr/local/lib >> >> LLVM bindir: /usr/local/bin >> >> LLVM ld flags: -L/usr/local/lib >> >> LLVM cxx flags: >> >> >> > >> -I/usr/local/include;-std=c++14;;;-fno-exceptions;-D_GNU_SOURCE;-D__STDC_CONSTANT_MACROS;-D__STDC_FORMAT_MACROS;-D__STDC_LIMIT_MACROS;-fno-rtti;-fno-exceptions >> >> >> >> clang: /usr/local/bin/clang >> >> llvm-as: /usr/local/bin/llvm-as >> >> llvm-link: /usr/local/bin/llvm-link >> >> opt: /usr/local/bin/opt >> >> llvm-spirv: /usr/local/bin/llvm-spirv >> >> >> >> -- Check for working CLC compiler: /usr/local/bin/clang >> >> -- Check for working CLC compiler: /usr/local/bin/clang -- works >> >> -- Check for working LLAsm compiler: /usr/local/bin/llvm-as >> >> -- Check for working LLAsm compiler: /usr/local/bin/llvm-as -- >> >> broken >> >> CMake Error at cmake/CMakeTestLLAsmCompiler.cmake:40 (message): >> >> The LLAsm compiler "/usr/local/bin/llvm-as" is not able to >> >> compile a >> >> simple >> >> test program. >> >> >> >> It fails with the following output: >> >> >> >> Change Dir: /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp >> >> >> >> Run Build Command(s):/usr/bin/gmake cmTC_87af9/fast && >> >> /usr/bin/gmake >> >> -f >> >> CMakeFiles/cmTC_87af9.dir/build.make >> >> CMakeFiles/cmTC_87af9.dir/build >> >> >> >> gmake[1]: Verzeichnis >> >> „/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird >> >> betreten >> >> >> >> Building LLAsm object >> >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc >> >> >> >> /usr/local/bin/clang -E -P -x cl >> >> >> >> >> > /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp/testLLAsmCompiler.ll >> >> >> >> -o >> >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp >> >> >> >> /usr/local/bin/llvm-as -o >> >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc >> >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp >> >> >> >> /usr/local/bin/llvm-as: >> >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp:1:1: error: >> >> expected >
Re: Is LLVM 13 (git) really ready for testing/development? libclc didn't compile
hi, sorry the option is actually -cl-no-stdinc. you can add it to 'target_compiler_options'. I should have a patch ready soon(tm), but time is scarce. Jan On Sun, Mar 7, 2021 at 9:51 PM Dieter Nützel wrote: > Hello Jan, > > I very much appreciate your advice. > Tried several places... > ...where to put it? > > Dieter > > Am 06.03.2021 17:56, schrieb Jan Vesely: > > Not Marek, but hope this answer will help. > > libclc uses clang CLC preprocessor on .ll files, llvm/clang-13 started > > including clc declarations by default (clang > > cf3ef15a6ec5e5b45c6c54e8fbe3769255e815ce), > > thus corrupting any .ll assembly files that are used by libclc. > > Inclusion of the default declarations can be turned off using a > > cmdline switch but that remains to be implemented in the libclc build > > system. > > manually adding '-cl-no-stdinc' should work as a workaround. > > > > Jan > > > > On Thu, Mar 4, 2021 at 10:27 PM Dieter Nützel > > wrote: > > > >> Hello Marek, > >> > >> can't compile anything, here. > >> Poor Intel Nehalem X3470. > >> > >> Trying LLVM 12-rc2 later. > >> > >> Greetings, > >> Dieter > >> > >> llvm-project/libclc> cd build && cmake ../ > >> -- The CXX compiler identification is GNU 10.2.1 > >> -- Detecting CXX compiler ABI info > >> -- Detecting CXX compiler ABI info - done > >> -- Check for working CXX compiler: /usr/bin/c++ - skipped > >> -- Detecting CXX compile features > >> -- Detecting CXX compile features - done > >> LLVM version: 13.0.0git > >> LLVM system libs: > >> LLVM libs: -lLLVM-13git > >> LLVM libdir: /usr/local/lib > >> LLVM bindir: /usr/local/bin > >> LLVM ld flags: -L/usr/local/lib > >> LLVM cxx flags: > >> > > > -I/usr/local/include;-std=c++14;;;-fno-exceptions;-D_GNU_SOURCE;-D__STDC_CONSTANT_MACROS;-D__STDC_FORMAT_MACROS;-D__STDC_LIMIT_MACROS;-fno-rtti;-fno-exceptions > >> > >> clang: /usr/local/bin/clang > >> llvm-as: /usr/local/bin/llvm-as > >> llvm-link: /usr/local/bin/llvm-link > >> opt: /usr/local/bin/opt > >> llvm-spirv: /usr/local/bin/llvm-spirv > >> > >> -- Check for working CLC compiler: /usr/local/bin/clang > >> -- Check for working CLC compiler: /usr/local/bin/clang -- works > >> -- Check for working LLAsm compiler: /usr/local/bin/llvm-as > >> -- Check for working LLAsm compiler: /usr/local/bin/llvm-as -- > >> broken > >> CMake Error at cmake/CMakeTestLLAsmCompiler.cmake:40 (message): > >> The LLAsm compiler "/usr/local/bin/llvm-as" is not able to > >> compile a > >> simple > >> test program. > >> > >> It fails with the following output: > >> > >> Change Dir: /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp > >> > >> Run Build Command(s):/usr/bin/gmake cmTC_87af9/fast && > >> /usr/bin/gmake > >> -f > >> CMakeFiles/cmTC_87af9.dir/build.make > >> CMakeFiles/cmTC_87af9.dir/build > >> > >> gmake[1]: Verzeichnis > >> „/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird > >> betreten > >> > >> Building LLAsm object > >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc > >> > >> /usr/local/bin/clang -E -P -x cl > >> > >> > > /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp/testLLAsmCompiler.ll > >> > >> -o > >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp > >> > >> /usr/local/bin/llvm-as -o > >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc > >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp > >> > >> /usr/local/bin/llvm-as: > >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp:1:1: error: > >> expected > >> top-level entity > >> > >> typedef unsigned char uchar; > >> > >> ^ > >> > >> gmake[1]: *** [CMakeFiles/cmTC_87af9.dir/build.make:86: > >> CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc] Fehler 1 > >> > >> gmake[1]: Verzeichnis > >> „/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird > >> verlassen > >> > >> gmake: *** [Makefile:140: cmTC_87af9/fast] Fehler 2 > >> > >> CMake will not be able to correctly generate this project. > >> Call Stack (most recent call first): > >> CMakeLists.txt:127 (enable_language) > >> > >> -- Configuring
Re: Is LLVM 13 (git) really ready for testing/development? libclc didn't compile
Not Marek, but hope this answer will help. libclc uses clang CLC preprocessor on .ll files, llvm/clang-13 started including clc declarations by default (clang cf3ef15a6ec5e5b45c6c54e8fbe3769255e815ce), thus corrupting any .ll assembly files that are used by libclc. Inclusion of the default declarations can be turned off using a cmdline switch but that remains to be implemented in the libclc build system. manually adding '-cl-no-stdinc' should work as a workaround. Jan On Thu, Mar 4, 2021 at 10:27 PM Dieter Nützel wrote: > Hello Marek, > > can't compile anything, here. > Poor Intel Nehalem X3470. > > Trying LLVM 12-rc2 later. > > Greetings, > Dieter > > llvm-project/libclc> cd build && cmake ../ > -- The CXX compiler identification is GNU 10.2.1 > -- Detecting CXX compiler ABI info > -- Detecting CXX compiler ABI info - done > -- Check for working CXX compiler: /usr/bin/c++ - skipped > -- Detecting CXX compile features > -- Detecting CXX compile features - done > LLVM version: 13.0.0git > LLVM system libs: > LLVM libs: -lLLVM-13git > LLVM libdir: /usr/local/lib > LLVM bindir: /usr/local/bin > LLVM ld flags: -L/usr/local/lib > LLVM cxx flags: > > -I/usr/local/include;-std=c++14;;;-fno-exceptions;-D_GNU_SOURCE;-D__STDC_CONSTANT_MACROS;-D__STDC_FORMAT_MACROS;-D__STDC_LIMIT_MACROS;-fno-rtti;-fno-exceptions > > clang: /usr/local/bin/clang > llvm-as: /usr/local/bin/llvm-as > llvm-link: /usr/local/bin/llvm-link > opt: /usr/local/bin/opt > llvm-spirv: /usr/local/bin/llvm-spirv > > -- Check for working CLC compiler: /usr/local/bin/clang > -- Check for working CLC compiler: /usr/local/bin/clang -- works > -- Check for working LLAsm compiler: /usr/local/bin/llvm-as > -- Check for working LLAsm compiler: /usr/local/bin/llvm-as -- broken > CMake Error at cmake/CMakeTestLLAsmCompiler.cmake:40 (message): >The LLAsm compiler "/usr/local/bin/llvm-as" is not able to compile a > simple >test program. > >It fails with the following output: > > Change Dir: /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp > > > >Run Build Command(s):/usr/bin/gmake cmTC_87af9/fast && /usr/bin/gmake > -f >CMakeFiles/cmTC_87af9.dir/build.make CMakeFiles/cmTC_87af9.dir/build > >gmake[1]: Verzeichnis >„/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird betreten > >Building LLAsm object CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc > >/usr/local/bin/clang -E -P -x cl > > /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp/testLLAsmCompiler.ll > -o >CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp > >/usr/local/bin/llvm-as -o > CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc >CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp > >/usr/local/bin/llvm-as: >CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp:1:1: error: > expected >top-level entity > >typedef unsigned char uchar; > >^ > >gmake[1]: *** [CMakeFiles/cmTC_87af9.dir/build.make:86: >CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc] Fehler 1 > >gmake[1]: Verzeichnis >„/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird verlassen > >gmake: *** [Makefile:140: cmTC_87af9/fast] Fehler 2 > > > > > > > >CMake will not be able to correctly generate this project. > Call Stack (most recent call first): >CMakeLists.txt:127 (enable_language) > > > -- Configuring incomplete, errors occurred! > See also "/opt/llvm-project/libclc/build/CMakeFiles/CMakeOutput.log". > See also "/opt/llvm-project/libclc/build/CMakeFiles/CMakeError.log". > > > CMakeError.log > Determining if the LLAsm compiler works failed with the following > output: > Change Dir: /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp > > Run Build Command(s):/usr/bin/gmake cmTC_87af9/fast && /usr/bin/gmake > -f CMakeFiles/cmTC_87af9.dir/build.make CMakeFiles/cmTC_87af9.dir/build > gmake[1]: Verzeichnis > „/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird betreten > Building LLAsm object CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc > /usr/local/bin/clang -E -P -x cl > /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp/testLLAsmCompiler.ll > -o CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp > /usr/local/bin/llvm-as -o CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc > CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp > /usr/local/bin/llvm-as: > CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp:1:1: error: expected > top-level entity > typedef unsigned char uchar; > ^ > gmake[1]: *** [CMakeFiles/cmTC_87af9.dir/build.make:86: > CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc] Fehler 1 > gmake[1]: Verzeichnis > „/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird verlassen > gmake: *** [Makefile:140: cmTC_87af9/fast] Fehler 2 > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org
Re: [PATCH libdrm] xf86drmHash: remove unused loop variable
On Wed, 2018-11-07 at 14:44 +, Eric Engestrom wrote:
> Reported-by: Jan Vesely
> Signed-off-by: Eric Engestrom
LGTM.
Reviewed-by: Jan Vesely
Jan
> ---
> xf86drmHash.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/xf86drmHash.c b/xf86drmHash.c
> index 891b732632c36d373ac9..2cf2b80ed9e02310f0ff 100644
> --- a/xf86drmHash.c
> +++ b/xf86drmHash.c
> @@ -105,7 +105,6 @@ static unsigned long HashHash(unsigned long key)
> drm_public void *drmHashCreate(void)
> {
> HashTablePtr table;
> -int i;
>
> table = drmMalloc(sizeof(*table));
> if (!table) return NULL;
signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 3/3] amdgpu: Destroy fd_hash table when the last device is removed.
On Fri, 2018-05-18 at 13:00 -0400, Jan Vesely wrote:
> Fixes memory leak on module unload.
> Analogous to mesa commit of the same name.
> Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
> ---
> amdgpu/amdgpu_device.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
> index e23dd3b3..34ac95b8 100644
> --- a/amdgpu/amdgpu_device.c
> +++ b/amdgpu/amdgpu_device.c
> @@ -128,6 +128,10 @@ static void
> amdgpu_device_free_internal(amdgpu_device_handle dev)
> {
> pthread_mutex_lock(_mutex);
> util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
> + if (util_hash_table_count(fd_tab) == 0) {
> + util_hash_table_destroy(fd_tab);
> + fd_tab = NULL;
> + }
> close(dev->fd);
> if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
> close(dev->flink_fd);
gentle ping.
Jan
signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 3/3] amdgpu: Destroy fd_hash table when the last device is removed.
Fixes memory leak on module unload.
Analogous to mesa commit of the same name.
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
---
amdgpu/amdgpu_device.c | 4
1 file changed, 4 insertions(+)
diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index e23dd3b3..34ac95b8 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -128,6 +128,10 @@ static void
amdgpu_device_free_internal(amdgpu_device_handle dev)
{
pthread_mutex_lock(_mutex);
util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
+ if (util_hash_table_count(fd_tab) == 0) {
+ util_hash_table_destroy(fd_tab);
+ fd_tab = NULL;
+ }
close(dev->fd);
if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
close(dev->flink_fd);
--
2.17.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 2/3] amdgpu/util_hash_table: Add helper function to count the number of entries in hash table
Analogous to the mesa commit of the same name.
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
---
amdgpu/util_hash_table.c | 12
amdgpu/util_hash_table.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/amdgpu/util_hash_table.c b/amdgpu/util_hash_table.c
index 89a8bf9b..e06d4415 100644
--- a/amdgpu/util_hash_table.c
+++ b/amdgpu/util_hash_table.c
@@ -237,6 +237,18 @@ drm_private void util_hash_table_foreach(struct
util_hash_table *ht,
}
}
+static void util_hash_table_inc(void *k, void *v, void *d)
+{
+ ++*(size_t *)d;
+}
+
+drm_private size_t util_hash_table_count(struct util_hash_table *ht)
+{
+ size_t count = 0;
+ util_hash_table_foreach(ht, util_hash_table_inc, );
+ return count;
+}
+
drm_private void util_hash_table_destroy(struct util_hash_table *ht)
{
struct util_hash_iter iter;
diff --git a/amdgpu/util_hash_table.h b/amdgpu/util_hash_table.h
index 5e295a81..3ab81a12 100644
--- a/amdgpu/util_hash_table.h
+++ b/amdgpu/util_hash_table.h
@@ -64,6 +64,8 @@ drm_private void util_hash_table_foreach(struct
util_hash_table *ht,
void (*callback)(void *key, void *value, void *data),
void *data);
+drm_private size_t util_hash_table_count(struct util_hash_table *ht);
+
drm_private void util_hash_table_destroy(struct util_hash_table *ht);
#endif /* U_HASH_TABLE_H_ */
--
2.17.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm v2 1/3] amdgpu: Take lock before removing devices from fd_tab hash table.
Close the file descriptors under lock as well.
v2: close fds after removing from hash table
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
---
amdgpu/amdgpu_device.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index 983b19ab..e23dd3b3 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -126,6 +126,13 @@ static int amdgpu_get_auth(int fd, int *auth)
static void amdgpu_device_free_internal(amdgpu_device_handle dev)
{
+ pthread_mutex_lock(_mutex);
+ util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
+ close(dev->fd);
+ if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
+ close(dev->flink_fd);
+ pthread_mutex_unlock(_mutex);
+
amdgpu_vamgr_deinit(>vamgr_32);
amdgpu_vamgr_deinit(>vamgr);
amdgpu_vamgr_deinit(>vamgr_high_32);
@@ -133,10 +140,6 @@ static void
amdgpu_device_free_internal(amdgpu_device_handle dev)
util_hash_table_destroy(dev->bo_flink_names);
util_hash_table_destroy(dev->bo_handles);
pthread_mutex_destroy(>bo_table_mutex);
- util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
- close(dev->fd);
- if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
- close(dev->flink_fd);
free(dev->marketing_name);
free(dev);
}
--
2.17.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 1/1] amdgpu: Take lock before removing devices from fd_tab hash table.
On Thu, 2018-05-10 at 19:33 -0400, Jan Vesely wrote:
> Close the file descriptors under lock as well.
>
> Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
> ---
> amdgpu/amdgpu_device.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
> index 983b19ab..f71fc050 100644
> --- a/amdgpu/amdgpu_device.c
> +++ b/amdgpu/amdgpu_device.c
> @@ -126,6 +126,13 @@ static int amdgpu_get_auth(int fd, int *auth)
>
> static void amdgpu_device_free_internal(amdgpu_device_handle dev)
> {
> + pthread_mutex_lock(_mutex);
> + close(dev->fd);
> + if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
> + close(dev->flink_fd);
> + util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
> + pthread_mutex_unlock(_mutex);
> +
> amdgpu_vamgr_deinit(>vamgr_32);
> amdgpu_vamgr_deinit(>vamgr);
> amdgpu_vamgr_deinit(>vamgr_high_32);
> @@ -133,10 +140,6 @@ static void
> amdgpu_device_free_internal(amdgpu_device_handle dev)
> util_hash_table_destroy(dev->bo_flink_names);
> util_hash_table_destroy(dev->bo_handles);
> pthread_mutex_destroy(>bo_table_mutex);
> - util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
> - close(dev->fd);
> - if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
> - close(dev->flink_fd);
> free(dev->marketing_name);
> free(dev);
> }
ping.
Jan
signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/1] amdgpu: Take lock before removing devices from fd_tab hash table.
Close the file descriptors under lock as well.
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
---
amdgpu/amdgpu_device.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index 983b19ab..f71fc050 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -126,6 +126,13 @@ static int amdgpu_get_auth(int fd, int *auth)
static void amdgpu_device_free_internal(amdgpu_device_handle dev)
{
+ pthread_mutex_lock(_mutex);
+ close(dev->fd);
+ if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
+ close(dev->flink_fd);
+ util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
+ pthread_mutex_unlock(_mutex);
+
amdgpu_vamgr_deinit(>vamgr_32);
amdgpu_vamgr_deinit(>vamgr);
amdgpu_vamgr_deinit(>vamgr_high_32);
@@ -133,10 +140,6 @@ static void
amdgpu_device_free_internal(amdgpu_device_handle dev)
util_hash_table_destroy(dev->bo_flink_names);
util_hash_table_destroy(dev->bo_handles);
pthread_mutex_destroy(>bo_table_mutex);
- util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
- close(dev->fd);
- if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
- close(dev->flink_fd);
free(dev->marketing_name);
free(dev);
}
--
2.17.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation
On Wed, 2017-11-29 at 16:58 -0500, Felix Kuehling wrote:
> You can see the state of the queues in debugfs:
> /sys/kernel/debug/kfd/... You can look at MQDs and HQDs.
thanks. how do I decode the information?
The rptr always stops at pos 60 which looks like this in mqds:
DIQ on device 45a2
: c0310800 4000
0020: 0001
0040:
0060:
If I understood correctly that's the queue dump, so those fs look
wrong
>
> If your application isn't stopping queues deliberately, queues get
> disabled by evictions, usually temporarily. You'll see kernel messages
> when that happens.
>
> A VM fault will result in queues of the offending process getting
> disabled permanently. Again, you'll see messages about that in the
> kernel log.
>
> The RPTR can also stop advancing if you have an infinite loop in a
> shader program, or just a shader that takes a very long time to execute.
> Or maybe if you have some dependencies (barriers) in your AQL packets
> that never get satisfied.
>
> The function you changed only affects the HIQ, the queue that KFD uses
> to control the HWS. It does not affect user mode queues. If your problem
> is with a user mode queue, your change should have no effect at all.
It's not a userspace queue that stops. I'm using kernel dbgdev to issue
wave_resume commands. (waves are halted after executing
s_sendmsg_halt).
I bumped KFD_KERNEL_QUEUE_SIZE to 16KB to make sure all 320 resume
commads fit (otherwise I get spurious ENOMEM when the queue is full but
still advancing).
thanks,
Jan
>
> Regards,
> Felix
>
>
> On 2017-11-29 04:43 PM, Jan Vesely wrote:
> > On Mon, 2017-11-20 at 14:22 -0500, Felix Kuehling wrote:
> > > I think this patch is not correct. The EOP-mem is not associated with
> > > the queue size. The EOP buffer is a separate buffer used by the firmware
> > > to handle command completion. As I understand it, this allows more
> > > concurrency, while still making it look like all commands in the queue
> > > are completing in order.
> >
> > thanks for the explanation. I was looking for a source of a CP hang
> > (rptr stops advancing), but bumping the eop size actually mode things
> > worse. Is there a way to find out if a queue got disabled and for what
> > reason? (I'm running ROCK-1.6.x based kernel)
> >
> > thanks,
> > Jan
> >
> > > Regards,
> > > Felix
> > >
> > >
> > > On 2017-11-19 03:19 AM, Oded Gabbay wrote:
> > > > On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely <jan.ves...@rutgers.edu>
> > > > wrote:
> > > > > Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
> > > > > ---
> > > > > drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
> > > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > > > b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > > > index f1d48281e322..b3bee39661ab 100644
> > > > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > > > @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue
> > > > > *kq, struct kfd_dev *dev,
> > > > > enum kfd_queue_type type, unsigned int
> > > > > queue_size)
> > > > > {
> > > > > int retval;
> > > > > + unsigned int size = ALIGN(queue_size, PAGE_SIZE);
> > > > >
> > > > > - retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
> > > > > + retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
> > > > > if (retval != 0)
> > > > > return false;
> > > > >
> > > > > kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
> > > > > kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
> > > > >
> > > > > - memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
> > > > > + memset(kq->eop_kernel_addr, 0, size);
> > > > >
> > > > > return true;
> > > > > }
> > > > > --
> > > > > 2.13.6
> > > > >
> > > > > ___
> > > > > amd-gfx mailing list
> > > > > amd-...@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > > >
> > > > Thanks!
> > > > Applied to -next tree
> > > > Oded
> > > > ___
> > > > amd-gfx mailing list
> > > > amd-...@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation
On Mon, 2017-11-20 at 14:22 -0500, Felix Kuehling wrote:
> I think this patch is not correct. The EOP-mem is not associated with
> the queue size. The EOP buffer is a separate buffer used by the firmware
> to handle command completion. As I understand it, this allows more
> concurrency, while still making it look like all commands in the queue
> are completing in order.
thanks for the explanation. I was looking for a source of a CP hang
(rptr stops advancing), but bumping the eop size actually mode things
worse. Is there a way to find out if a queue got disabled and for what
reason? (I'm running ROCK-1.6.x based kernel)
thanks,
Jan
>
> Regards,
> Felix
>
>
> On 2017-11-19 03:19 AM, Oded Gabbay wrote:
> > On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely <jan.ves...@rutgers.edu> wrote:
> > > Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
> > > ---
> > > drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > index f1d48281e322..b3bee39661ab 100644
> > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq,
> > > struct kfd_dev *dev,
> > > enum kfd_queue_type type, unsigned int queue_size)
> > > {
> > > int retval;
> > > + unsigned int size = ALIGN(queue_size, PAGE_SIZE);
> > >
> > > - retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
> > > + retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
> > > if (retval != 0)
> > > return false;
> > >
> > > kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
> > > kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
> > >
> > > - memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
> > > + memset(kq->eop_kernel_addr, 0, size);
> > >
> > > return true;
> > > }
> > > --
> > > 2.13.6
> > >
> > > ___
> > > amd-gfx mailing list
> > > amd-...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> > Thanks!
> > Applied to -next tree
> > Oded
> > ___
> > amd-gfx mailing list
> > amd-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
---
drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
index f1d48281e322..b3bee39661ab 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
@@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, struct
kfd_dev *dev,
enum kfd_queue_type type, unsigned int queue_size)
{
int retval;
+ unsigned int size = ALIGN(queue_size, PAGE_SIZE);
- retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
+ retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
if (retval != 0)
return false;
kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
- memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
+ memset(kq->eop_kernel_addr, 0, size);
return true;
}
--
2.13.6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/1] amdgpu: Do not write beyond allocated memory when parsing ids
Fixes crash when/usr/share/libdrm/amdgpu.ids contains ASIC_ID_TABLE_NUM_ENTRIES
+ 1 entries.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102432
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
---
Compile tested only.
amdgpu/amdgpu_asic_id.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
index 3a88896b..e8218974 100644
--- a/amdgpu/amdgpu_asic_id.c
+++ b/amdgpu/amdgpu_asic_id.c
@@ -186,19 +186,20 @@ int amdgpu_parse_asic_ids(struct amdgpu_asic_id
**p_asic_id_table)
table_size++;
}
- /* end of table */
- id = asic_id_table + table_size;
- memset(id, 0, sizeof(struct amdgpu_asic_id));
-
if (table_size != table_max_size) {
id = realloc(asic_id_table, (table_size + 1) *
sizeof(struct amdgpu_asic_id));
- if (!id)
+ if (!id) {
r = -ENOMEM;
- else
- asic_id_table = id;
+ goto free;
+ }
+ asic_id_table = id;
}
+ /* end of table */
+ id = asic_id_table + table_size;
+ memset(id, 0, sizeof(struct amdgpu_asic_id));
+
free:
free(line);
--
2.13.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] exynos: change the license to X11/MIT
On Thu, 2017-08-10 at 13:52 +0900, Inki Dae wrote: > Chnage GPL license of Exynos relevant code to X11/MIT. > > I'd like to keep license consistency to all Exynos code > because License checker notices two more licenses exist > in libdrm. > > For the license change I need to get your agree - all committers. > So please give me Acked-by if you agree with me. Don't really know or care why this is necessary, but few cleanup commits shouldn't block this. Acked-by: Jan Vesely <jan.ves...@rutgers.edu> Jan > > Signed-off-by: Inki Dae <inki@samsung.com> > Acked-by: Hyungwon Hwang <human.hw...@samsung.com> > Acked-by: SooChan Lim <sc1@samsung.com> > Acked-by: Sangjin LEE <lsj...@samsung.com> > Acked-by: Boram Park <boram1288.p...@samsung.com> > Acked-by: Seung-Woo Kim <sw0312@samsung.com> > Acked-by: Joonyoung Shim <jy0922.s...@samsung.com> > Acked-by: Emil Velikov <emil.l.veli...@gmail.com> > --- > exynos/exynos_fimg2d.c | 21 + > exynos/exynos_fimg2d.h | 21 + > exynos/fimg2d_reg.h| 21 + > libkms/exynos.c| 22 ++ > tests/exynos/exynos_fimg2d_event.c | 27 +-- > tests/exynos/exynos_fimg2d_perf.c | 27 +-- > tests/exynos/exynos_fimg2d_test.c | 21 + > 7 files changed, 120 insertions(+), 40 deletions(-) > > diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c > index 61340c3..5658a48 100644 > --- a/exynos/exynos_fimg2d.c > +++ b/exynos/exynos_fimg2d.c > @@ -3,11 +3,24 @@ > * Authors: > * Inki Dae <inki@samsung.com> > * > - * This program is free software; you can redistribute it and/or modify it > - * under the terms of the GNU General Public License as published by the > - * Free Software Foundation; either version 2 of the License, or (at your > - * option) any later version. > + * 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 (including the next > + * paragraph) 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 > + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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. > */ > > #ifdef HAVE_CONFIG_H > diff --git a/exynos/exynos_fimg2d.h b/exynos/exynos_fimg2d.h > index a825c68..a4dfbe7 100644 > --- a/exynos/exynos_fimg2d.h > +++ b/exynos/exynos_fimg2d.h > @@ -3,11 +3,24 @@ > * Authors: > * Inki Dae <inki@samsung.com> > * > - * This program is free software; you can redistribute it and/or modify it > - * under the terms of the GNU General Public License as published by the > - * Free Software Foundation; either version 2 of the License, or (at your > - * option) any later version. > + * 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 (including the next > + * paragraph) 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 > + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM
[PATCH libdrm 1/1] amdgpu: Add FX-9800P Bristol Ridge iGPU id
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu> --- data/amdgpu.ids | 1 + 1 file changed, 1 insertion(+) diff --git a/data/amdgpu.ids b/data/amdgpu.ids index 0b98c3c3..f6c65dd9 100644 --- a/data/amdgpu.ids +++ b/data/amdgpu.ids @@ -153,6 +153,7 @@ 9874, C5, AMD Radeon R6 Graphics 9874, C6, AMD Radeon R6 Graphics 9874, C7, AMD Radeon R5 Graphics +9874, C8, AMD Radeon R7 Graphics 9874, 81, AMD Radeon R6 Graphics 9874, 87, AMD Radeon R5 Graphics 9874, 85, AMD Radeon R6 Graphics -- 2.13.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/1] drmsltest: Check expected neighbours
Fixes: 7d8c9464081634f053e16e5eac9655a12fae1dc4
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
---
I thought I sent this out, but couldn't find it in sent mail.
This tests the behaviour set in 3b2ee2b5bfc0d68525fee936e51297a9b6c629f1
more than 2 years ago
Jan
tests/drmsl.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/tests/drmsl.c b/tests/drmsl.c
index d0ac0efa..d1b59a86 100644
--- a/tests/drmsl.c
+++ b/tests/drmsl.c
@@ -106,7 +106,9 @@ static double do_time(int size, int iter)
return usec;
}
-static void print_neighbors(void *list, unsigned long key)
+static void print_neighbors(void *list, unsigned long key,
+unsigned long expected_prev,
+unsigned long expected_next)
{
unsigned long prev_key = 0;
unsigned long next_key = 0;
@@ -119,6 +121,16 @@ static void print_neighbors(void *list, unsigned long key)
_key, _value);
printf("Neighbors of %5lu: %d %5lu %5lu\n",
key, retval, prev_key, next_key);
+if (prev_key != expected_prev) {
+fprintf(stderr, "Unexpected neighbor: %5lu. Expected: %5lu\n",
+prev_key, expected_prev);
+ exit(1);
+}
+if (next_key != expected_next) {
+fprintf(stderr, "Unexpected neighbor: %5lu. Expected: %5lu\n",
+next_key, expected_next);
+ exit(1);
+}
}
int main(void)
@@ -138,13 +150,13 @@ int main(void)
print(list);
printf("\n==\n\n");
-print_neighbors(list, 0);
-print_neighbors(list, 50);
-print_neighbors(list, 51);
-print_neighbors(list, 123);
-print_neighbors(list, 200);
-print_neighbors(list, 213);
-print_neighbors(list, 256);
+print_neighbors(list, 0, 0, 50);
+print_neighbors(list, 50, 0, 50);
+print_neighbors(list, 51, 50, 123);
+print_neighbors(list, 123, 50, 123);
+print_neighbors(list, 200, 123, 213);
+print_neighbors(list, 213, 123, 213);
+print_neighbors(list, 256, 213, 256);
printf("\n==\n\n");
drmSLDelete(list, 50);
--
2.13.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/1] drmsltest: Check expected neighbours
This test cements behaviour set in
3b2ee2b5bfc0d68525fee936e51297a9b6c629f1 drmSL: Fix neighbor lookup (2015-02-27)
Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
---
tests/drmsl.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/tests/drmsl.c b/tests/drmsl.c
index d0ac0ef..d1b59a8 100644
--- a/tests/drmsl.c
+++ b/tests/drmsl.c
@@ -106,7 +106,9 @@ static double do_time(int size, int iter)
return usec;
}
-static void print_neighbors(void *list, unsigned long key)
+static void print_neighbors(void *list, unsigned long key,
+unsigned long expected_prev,
+unsigned long expected_next)
{
unsigned long prev_key = 0;
unsigned long next_key = 0;
@@ -119,6 +121,16 @@ static void print_neighbors(void *list, unsigned long key)
_key, _value);
printf("Neighbors of %5lu: %d %5lu %5lu\n",
key, retval, prev_key, next_key);
+if (prev_key != expected_prev) {
+fprintf(stderr, "Unexpected neighbor: %5lu. Expected: %5lu\n",
+prev_key, expected_prev);
+ exit(1);
+}
+if (next_key != expected_next) {
+fprintf(stderr, "Unexpected neighbor: %5lu. Expected: %5lu\n",
+next_key, expected_next);
+ exit(1);
+}
}
int main(void)
@@ -138,13 +150,13 @@ int main(void)
print(list);
printf("\n==\n\n");
-print_neighbors(list, 0);
-print_neighbors(list, 50);
-print_neighbors(list, 51);
-print_neighbors(list, 123);
-print_neighbors(list, 200);
-print_neighbors(list, 213);
-print_neighbors(list, 256);
+print_neighbors(list, 0, 0, 50);
+print_neighbors(list, 50, 0, 50);
+print_neighbors(list, 51, 50, 123);
+print_neighbors(list, 123, 50, 123);
+print_neighbors(list, 200, 123, 213);
+print_neighbors(list, 213, 123, 213);
+print_neighbors(list, 256, 213, 256);
printf("\n==\n\n");
drmSLDelete(list, 50);
--
2.9.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] xf86drm: fix valgrind warning in drmParsePciBusInfo
On Thu, 2017-01-12 at 18:16 -0800, Nicholas Miell wrote:
> From 714d07f600db39498c87d7816f4dd3a7e6d9bbca Mon Sep 17 00:00:00 2001
> From: Nicholas Miell <nmi...@gmail.com>
> Date: Thu, 12 Jan 2017 15:43:07 -0800
> Subject: [PATCH libdrm] xf86drm: fix valgrind warning in drmParsePciBusInfo
>
> The current implementation reads (up to) 513 bytes, overwrites the 513th
> byte with '\0' and then passes the buffer off to strstr() and sscanf()
> without ever initializing the middle bytes. This causes valgrind
> warnings and potentially fails to parse PCI_SLOT_NAME if the uevent is
> unexpectedly large.
a simpler fix should also get rid of the valgrind warning:
- ret = read(fd, data, sizeof(data));
- data[sizeof(data)-1] = '\0';
+ ret = read(fd, data, sizeof(data) - 1);
close(fd);
if (ret < 0)
return -errno
+ data[ret] = '\0';
I think that dynamic memory allocation is still a more robust approach.
regards,
Jan
>
> Rewrite it using getline() to fix the valgrind errors and future-proof
> the function against uevent files larger than 513 bytes.
>
> Signed-off-by: Nicholas Miell <nmi...@gmail.com>
> ---
> xf86drm.c | 34 +-
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index b8b2cfe..33261ac 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2919,31 +2919,31 @@ static int drmParsePciBusInfo(int maj, int min,
> drmPciBusInfoPtr info)
> {
> #ifdef __linux__
> char path[PATH_MAX + 1];
> -char data[512 + 1];
> -char *str;
> +FILE *uevent;
> +char *line = NULL;
> +size_t n = 0;
> +bool found = false;
> int domain, bus, dev, func;
> -int fd, ret;
>
> snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/uevent", maj, min);
> -fd = open(path, O_RDONLY);
> -if (fd < 0)
> +uevent = fopen(path, "r");
> +if (uevent == NULL)
> return -errno;
>
> -ret = read(fd, data, sizeof(data));
> -data[sizeof(data)-1] = '\0';
> -close(fd);
> -if (ret < 0)
> -return -errno;
> +while (getline(, , uevent) != -1) {
> +if (sscanf(line, "PCI_SLOT_NAME=%04x:%02x:%02x.%1u",
> + , , , ) == 4)
> +{
> + found = true;
> + break;
> +}
> +}
> +free(line);
>
> -#define TAG "PCI_SLOT_NAME="
> -str = strstr(data, TAG);
> -if (str == NULL)
> - return -EINVAL;
> +fclose(uevent);
>
> -if (sscanf(str, TAG "%04x:%02x:%02x.%1u",
> - , , , ) != 4)
> +if (!found)
> return -EINVAL;
> -#undef TAG
>
> info->domain = domain;
> info->bus = bus;
--
Jan Vesely <jan.ves...@rutgers.edu>
signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/1] xf86drm.c: Fix mix of tabs and spaces
Remove whitespace at the end of line.
diff -b shows only two removed lines
Signed-off-by: Jan Vesely
---
xf86drm.c | 1444 ++---
1 file changed, 721 insertions(+), 723 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index 804a413..e99f2e2 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -1,5 +1,5 @@
/**
- * \file xf86drm.c
+ * \file xf86drm.c
* User-level interface to DRM device
*
* \author Rickard E. (Rik) Faith
@@ -70,13 +70,13 @@
#include "util_math.h"
#ifdef __OpenBSD__
-#define DRM_PRIMARY_MINOR_NAME "drm"
-#define DRM_CONTROL_MINOR_NAME "drmC"
-#define DRM_RENDER_MINOR_NAME "drmR"
+#define DRM_PRIMARY_MINOR_NAME "drm"
+#define DRM_CONTROL_MINOR_NAME "drmC"
+#define DRM_RENDER_MINOR_NAME "drmR"
#else
-#define DRM_PRIMARY_MINOR_NAME "card"
-#define DRM_CONTROL_MINOR_NAME "controlD"
-#define DRM_RENDER_MINOR_NAME "renderD"
+#define DRM_PRIMARY_MINOR_NAME "card"
+#define DRM_CONTROL_MINOR_NAME "controlD"
+#define DRM_RENDER_MINOR_NAME "renderD"
#endif
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ||
defined(__DragonFly__)
@@ -96,7 +96,7 @@
#endif /* __OpenBSD__ */
#ifndef DRM_MAJOR
-#define DRM_MAJOR 226 /* Linux */
+#define DRM_MAJOR 226 /* Linux */
#endif
#define DRM_MSG_VERBOSITY 3
@@ -128,18 +128,18 @@ drmDebugPrint(const char *format, va_list ap)
void
drmMsg(const char *format, ...)
{
-va_listap;
+va_list ap;
const char *env;
if (((env = getenv("LIBGL_DEBUG")) && strstr(env, "verbose")) ||
(drm_server_info && drm_server_info->debug_print))
{
- va_start(ap, format);
- if (drm_server_info) {
- drm_server_info->debug_print(format,ap);
- } else {
- drmDebugPrint(format, ap);
- }
- va_end(ap);
+va_start(ap, format);
+if (drm_server_info) {
+drm_server_info->debug_print(format,ap);
+} else {
+drmDebugPrint(format, ap);
+}
+va_end(ap);
}
}
@@ -166,10 +166,10 @@ void drmFree(void *pt)
int
drmIoctl(int fd, unsigned long request, void *arg)
{
-intret;
+int ret;
do {
- ret = ioctl(fd, request, arg);
+ret = ioctl(fd, request, arg);
} while (ret == -1 && (errno == EINTR || errno == EAGAIN));
return ret;
}
@@ -190,16 +190,16 @@ drmHashEntry *drmGetEntry(int fd)
drmHashEntry *entry;
if (!drmHashTable)
- drmHashTable = drmHashCreate();
+drmHashTable = drmHashCreate();
if (drmHashLookup(drmHashTable, key, )) {
- entry = drmMalloc(sizeof(*entry));
- entry->fd = fd;
- entry->f= NULL;
- entry->tagTable = drmHashCreate();
- drmHashInsert(drmHashTable, key, entry);
+entry = drmMalloc(sizeof(*entry));
+entry->fd = fd;
+entry->f= NULL;
+entry->tagTable = drmHashCreate();
+drmHashInsert(drmHashTable, key, entry);
} else {
- entry = value;
+entry = value;
}
return entry;
}
@@ -221,41 +221,41 @@ static int drmMatchBusID(const char *id1, const char
*id2, int pci_domain_ok)
{
/* First, check if the IDs are exactly the same */
if (strcasecmp(id1, id2) == 0)
- return 1;
+return 1;
/* Try to match old/new-style PCI bus IDs. */
if (strncasecmp(id1, "pci", 3) == 0) {
- unsigned int o1, b1, d1, f1;
- unsigned int o2, b2, d2, f2;
- int ret;
-
- ret = sscanf(id1, "pci:%04x:%02x:%02x.%u", , , , );
- if (ret != 4) {
- o1 = 0;
- ret = sscanf(id1, "PCI:%u:%u:%u", , , );
- if (ret != 3)
- return 0;
- }
-
- ret = sscanf(id2, "pci:%04x:%02x:%02x.%u", , , , );
- if (ret != 4) {
- o2 = 0;
- ret = sscanf(id2, "PCI:%u:%u:%u", , , );
- if (ret != 3)
- return 0;
- }
-
- /* If domains aren't properly supported by the kernel interface,
-* just ignore them, which sucks less than picking a totally random
-* card with "open by name"
-*/
- if (!pci_domain_ok)
- o1 = o2 = 0;
-
- if ((o1 != o2) || (b1 != b2) || (d1 != d2) || (f1 != f2))
- return 0;
- else
- return 1;
+unsigned int o1, b1, d1, f1;
+unsigned int o2, b2, d2, f2;
+int ret;
+
+ret = sscanf(id1, "pci:%04x:%02x:%02x.%u", , , , );
+if (ret != 4) {
+o1 = 0;
+ret = sscanf(id1, "PCI:%u:%u:%u", , , );
+if (ret != 3)
+return 0;
+}
+
+ret = sscanf(id2, "pci:%04x:%02x:%02x.%u&q
[PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
On Fri, 2016-01-22 at 12:48 -0500, Ilia Mirkin wrote:
> On Fri, Jan 22, 2016 at 12:47 PM, Ville Syrjälä
> wrote:
> > On Fri, Jan 22, 2016 at 05:40:54PM +, Emil Velikov wrote:
> > > On 22 January 2016 at 17:29, Ilia Mirkin
> > > wrote:
> > > > On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák > > > > wrote:
> > > > > On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov > > > > @gmail.com> wrote:
> > > > > > On 21 January 2016 at 16:58, Marek Olšák
> > > > > > wrote:
> > > > > > > On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov > > > > > > ikov at gmail.com> wrote:
> > > > > > > > On 21 January 2016 at 12:08, Marek Olšák > > > > > > > com> wrote:
> > > > > > > > > On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov > > > > > > > > l.velikov at gmail.com> wrote:
> > > > > > > > > > On 18 January 2016 at 22:53, Marek Olšák > > > > > > > > > ail.com> wrote:
> > > > > > > > > > > Try explaining that to people who have a
> > > > > > > > > > > compulsion to fix them or
> > > > > > > > > > > argue about them. :) Ignore? REALLY? IGNORE???
> > > > > > > > > > >
> > > > > > > > > > Now that we have a few people off your back can you
> > > > > > > > > > please point out
> > > > > > > > > > where this triggers warnings ?
> > > > > > > > >
> > > > > > > > > This particular warning is trigged by {}
> > > > > > > > As mentioned previously neither {} nor {0} trigger any
> > > > > > > > warning here.
> > > > > > > > Jani hinted that you might be using an old (buggy?)
> > > > > > > > compiler which
> > > > > > > > generates them.
> > > > > > > > Which version of GCC are you using ? Do you mind
> > > > > > > > showing the first few
> > > > > > > > warnings ?
> > > > > > > >
> > > > > > > > > or any { ... } which doesn't
> > > > > > > > > initialize all members.
> > > > > > > > >
> > > > > > > > Do we have any outside of intel_decode.c ? I'm failing
> > > > > > > > to spot any.
> > > > > > >
> > > > > > > amdgpu_bo.c has 7 occurences of "= {}" and they all print
> > > > > > > the warning.
> > > > > > With 200+ cases of memset and 40+ of "= *{ *0 *}". Any
> > > > > > objections if I
> > > > > > send a patch to transition to either one of these two ?
> > > > >
> > > > > That's up to you, but please note that I don't plan to stop
> > > > > using "= {}",
> > > > > because it's the most convenient way to clear memory in a lot
> > > > > of
> > > > > cases and takes only 4 bytes of text.
> > > >
> > > > I like {} too and think we should encourage that. I'd rather
> > > > transition the { 0 } stuff over to {}.
> > > >
> > > So people feel against seeing/writing single extra character 0,
> > > despite that the warning has helped catch actual bug ?
> > > And now are willing to transitions 40+ cases as opposed to ~15...
> > > that
> > > feels strange to say the least.
> >
> > Does the '= { 0 }' thing even work if the first member happens to
> > be
> > something other than an integer?
>
> No. That's why I like {}. Otherwise you end up doing
> {{0}.
ISO C99
According to 6.7.8 20 all braces but the outermost ones are optional.
{}, on the other hand, is not allowed by syntax rules.
Jan
>
> Â -ilia
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20160122/036efd7e/attachment.sig>
[PATCH 2/2] Fix one warning
On Mon, 2015-04-27 at 01:40 +, Zhou, Jammy wrote:
> 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?
in xserver:
function dr_drm_get_perms (hw/xfree86/dri/dri.c:759) copies previously
initialized values of xf86ConfigDRI.
Those values are set in configDRI (hw/xfree86/common/xf86Config.c:2166),
which is called from xf86HandleConfigFile
(hw/xfree86/common/xf86Config.c:2479)
and provided values parsed in
xf86readConfigFile(hw/xfree86/parser/read.c:89), group is only set in
dri section.
I did not really look into details. to me it looks like -1 is stored in
group unless there is a valid DRI section with group assigned name or
gid.
I did not look for other users of libdrm that might also use neg values
to indicate errors. You might want to ask someone who is more familiar
with the design than just reading the code (like I did)
regards,
jan
>
> 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:jv356 at 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
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20160106/c5694ea9/attachment.sig>
[PATCH 2/2] Fix one warning (v2)
On Mon, 2015-04-27 at 10:29 +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;
> ^
>
> v2: do 'int' cast to fix the warning
>
> Signed-off-by: Jammy Zhou
FWIW
Reviewed-by: Jan Vesely
Emil, are you OK with this approach?
> ---
> xf86drm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 4d67861..4ecb24f 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 = ((int)serv_group >= 0) ? serv_group : DRM_DEV_GID;
> chown_check_return(buf, user, group);
> chmod(buf, devmode);
> }
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150428/4a23ab1a/attachment-0001.sig>
[PATCH 2/2] Fix one warning
On Mon, 2015-04-27 at 01:40 +, Zhou, Jammy wrote:
> 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?
The one i found is in xserver:
dri_drm_get_perms (hw/xfree86/dri/dri.c:759) copies values from
xf86ConfigDRI.
xf86configDRI is initialized in
configDRI(hw/xfree86/common/xf86Config.c:2166).
However, the default value if the DRI section is not present or does not
contain group setting is -1.
it looks like it relies on libdrm to fall back to default in that case,
and it looks like that path currently broken
I don't claim to fully understand what that old code is doing/supposed
to do, but scanning through it suggests that negative values are legal
way to report errors/undefined values.
there might be other users as well
jan
>
> 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:jv356 at 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
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150428/7fae4206/attachment.sig>
[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
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150424/9ac9358f/attachment.sig>
[PATCH 1/1] drm/radeon: Fix cleanup error path.
Hi Maarten, sorry for the delay, could not fit tests that freeze the machine into my schedule. I found two patches at that bug. The first one (Use fence_add/remove_callback) is already applied in 4.0. I tried the second one (reshuffle evergreen_dma_fence_ring_emit), but it did not help. However, on the very first attempt the GPU reset worked, so I think it's just a race somewhere, maybe it has always been there and the changes since 3.17 just upset the balance. I'll try to get more details, but the machine occasionally has to run days without reboot so testing kernels can be tricky. regards, Jan On Thu, 2015-03-05 at 11:22 +0100, Maarten Lankhorst wrote: > Hey, > > On 05-03-15 02:13, Jan Vesely wrote: > > please ignore this patch, see bugzilla for details. > Just in case, can you try the patches from > https://bugzilla.kernel.org/show_bug.cgi?id=90741 ? > > ~Maarten > > -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150424/74b9061d/attachment.sig>
[PATCH] drm/nouveau/bios: 0 means success for nvbios_extend
Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=89047
CC: David Airlie
CC: Ben Skeggs
CC: dri-devel at lists.freedesktop.org
CC: linux-kernel at vger.kernel.org
Signed-off-by: Jan Vesely
---
It's needed for 3.19 too
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowacpi.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
index 8c2b7cb..f566ac8 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
@@ -44,7 +44,7 @@ shadow_fetch(struct nvkm_bios *bios, u32 upto)
const u32 limit = (upto + 3) & ~3;
const u32 start = bios->size;
void *data = mthd->data;
- if (nvbios_extend(bios, limit) > 0) {
+ if (nvbios_extend(bios, limit) >= 0) {
u32 read = mthd->func->read(data, start, limit - start, bios);
bios->size = start + read;
}
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowacpi.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowacpi.c
index 1fbd93b..f9d0eb5 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowacpi.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowacpi.c
@@ -52,7 +52,7 @@ acpi_read_fast(void *data, u32 offset, u32 length, struct
nvkm_bios *bios)
u32 start = offset & ~0x0fff;
u32 fetch = limit - start;
- if (nvbios_extend(bios, limit) > 0) {
+ if (nvbios_extend(bios, limit) >= 0) {
int ret = nouveau_acpi_get_bios_chunk(bios->data, start, fetch);
if (ret == fetch)
return fetch;
@@ -73,7 +73,7 @@ acpi_read_slow(void *data, u32 offset, u32 length, struct
nvkm_bios *bios)
u32 start = offset & ~0xfff;
u32 fetch = 0;
- if (nvbios_extend(bios, limit) > 0) {
+ if (nvbios_extend(bios, limit) >= 0) {
while (start + fetch < limit) {
int ret = nouveau_acpi_get_bios_chunk(bios->data,
start + fetch,
--
2.0.5
[PATCH libdrm v2 1/8] tests/hash: extract test out of xf86drmHash.c
< DIST_LIMIT; i++) dist[i] = 0;
> -}
> -
> -static int count_entries(HashBucketPtr bucket)
> -{
> -int count = 0;
> -
> -for (; bucket; bucket = bucket->next) ++count;
> -return count;
> -}
> -
> -static void update_dist(int count)
> -{
> -if (count >= DIST_LIMIT) ++dist[DIST_LIMIT-1];
> -else ++dist[count];
> -}
> -
> -static void compute_dist(HashTablePtr table)
> -{
> -int i;
> -HashBucketPtr bucket;
> -
> -printf("Entries = %ld, hits = %ld, partials = %ld, misses = %ld\n",
> -table->entries, table->hits, table->partials, table->misses);
> -clear_dist();
> -for (i = 0; i < HASH_SIZE; i++) {
> - bucket = table->buckets[i];
> - update_dist(count_entries(bucket));
> -}
> -for (i = 0; i < DIST_LIMIT; i++) {
> - if (i != DIST_LIMIT-1) printf("%5d %10d\n", i, dist[i]);
> - else printf("other %10d\n", dist[i]);
> -}
> -}
> -
> -static void check_table(HashTablePtr table,
> - unsigned long key, unsigned long value)
> -{
> -unsigned long retval = 0;
> -int retcode = drmHashLookup(table, key, );
> -
> -switch (retcode) {
> -case -1:
> - printf("Bad magic = 0x%08lx:"
> -" key = %lu, expected = %lu, returned = %lu\n",
> -table->magic, key, value, retval);
> - break;
> -case 1:
> - printf("Not found: key = %lu, expected = %lu returned = %lu\n",
> -key, value, retval);
> - break;
> -case 0:
> - if (value != retval)
> - printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
> -key, value, retval);
> - break;
> -default:
> - printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n",
> -retcode, key, value, retval);
> - break;
> -}
> -}
> -
> -int main(void)
> -{
> -HashTablePtr table;
> -int i;
> -
> -printf("\n* 256 consecutive integers \n");
> -table = drmHashCreate();
> -for (i = 0; i < 256; i++) drmHashInsert(table, i, i);
> -for (i = 0; i < 256; i++) check_table(table, i, i);
> -for (i = 256; i >= 0; i--) check_table(table, i, i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -printf("\n* 1024 consecutive integers \n");
> -table = drmHashCreate();
> -for (i = 0; i < 1024; i++) drmHashInsert(table, i, i);
> -for (i = 0; i < 1024; i++) check_table(table, i, i);
> -for (i = 1024; i >= 0; i--) check_table(table, i, i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -printf("\n* 1024 consecutive page addresses (4k pages) \n");
> -table = drmHashCreate();
> -for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, i);
> -for (i = 0; i < 1024; i++) check_table(table, i*4096, i);
> -for (i = 1024; i >= 0; i--) check_table(table, i*4096, i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -printf("\n* 1024 random integers \n");
> -table = drmHashCreate();
> -srandom(0xbeefbeef);
> -for (i = 0; i < 1024; i++) drmHashInsert(table, random(), i);
> -srandom(0xbeefbeef);
> -for (i = 0; i < 1024; i++) check_table(table, random(), i);
> -srandom(0xbeefbeef);
> -for (i = 0; i < 1024; i++) check_table(table, random(), i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -printf("\n* 5000 random integers \n");
> -table = drmHashCreate();
> -srandom(0xbeefbeef);
> -for (i = 0; i < 5000; i++) drmHashInsert(table, random(), i);
> -srandom(0xbeefbeef);
> -for (i = 0; i < 5000; i++) check_table(table, random(), i);
> -srandom(0xbeefbeef);
> -for (i = 0; i < 5000; i++) check_table(table, random(), i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -return 0;
> -}
> -#endif
> diff --git a/xf86drmHash.h b/xf86drmHash.h
> new file mode 100644
> index 000..38fd84b
> --- /dev/null
> +++ b/xf86drmHash.h
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> + * All Rights Reserved.
> + *
> + * 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 us
[PATCH libdrm 0/9] Rework remaining tests
On Sun, 2015-03-22 at 22:03 +, Emil Velikov wrote: > This series covers > - Remove the hackish "include xf86drmfoo.c" from dristat > - Extract the remaining two xf86drmfoo.c tests to standalone ones > - beat them into shape, and > - wire them up to make check. I finished going through the series. Feel free to ignore the internal header suggestions, I don;t think the risk of desynchronizing structures in test and implementation is real since few ppl will touch it anyway. drmHash test looks weird after cleanup. other than that looks good regards, jan > > > -Emil > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150324/4890d2f0/attachment.sig>
[PATCH libdrm v3 1/3] drmSL: Fix neighbor lookup
Commit e4a519635f75bde38aeb5b09f2ff4efbf73453e9:
Tidy up compile warnings by cleaning up types.
removed call to SLLocate which gutted the function of all functionality.
This patch restores the original behavior, with an additional fix
that zeros the update array in case SLLocate bails early.
v2: zero the update array instead of checking the return value.
SLLocate returns NULL both on failure and if the element is greater
than everything in the list
v3: Improve commit message
Signed-off-by: Jan Vesely
Acked-by: Emil Velikov
---
sorry for spamming, just realized it might be a good idea co CC the original
author and committer.
This was broken since 2.4.18 (2010). I guess it's safe to say that nobody
uses it. What are the policies on removing parts of API?
xf86drmSL.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xf86drmSL.c b/xf86drmSL.c
index acddb54..cf588ac 100644
--- a/xf86drmSL.c
+++ b/xf86drmSL.c
@@ -264,12 +264,14 @@ int drmSLLookupNeighbors(void *l, unsigned long key,
unsigned long *next_key, void **next_value)
{
SkipListPtr list = (SkipListPtr)l;
-SLEntryPtrupdate[SL_MAX_LEVEL + 1];
+SLEntryPtrupdate[SL_MAX_LEVEL + 1] = {0};
int retcode = 0;
+SLLocate(list, key, update);
+
*prev_key = *next_key = key;
*prev_value = *next_value = NULL;
-
+
if (update[0]) {
*prev_key = update[0]->key;
*prev_value = update[0]->value;
--
2.1.0
[PATCH libdrm 2/3] tests/drmsl: Extract tests out of xf86drmSL.c
v2: merge tests creation and xf86drmSL cleanup
rename tests/drmsltest -> tests/drmsl
move the test out of libudev test block
Signed-off-by: Jan Vesely
---
Hi Emil,
I know you send your R-b on the earlier version, but I thought the changes
were big enough to send v2. I modeled it after you test splitting series.
jan
.gitignore| 1 +
tests/Makefile.am | 5 +-
tests/drmsl.c | 172 ++
xf86drmSL.c | 172 ++
4 files changed, 183 insertions(+), 167 deletions(-)
create mode 100644 tests/drmsl.c
diff --git a/.gitignore b/.gitignore
index 06cc928..cb7128d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@ tdfx.kld
via.kld
tests/auth
tests/dristat
+tests/drmsl
tests/drmstat
tests/getclient
tests/getstats
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 10f54e3..ad70314 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -35,6 +35,9 @@ if HAVE_NOUVEAU
SUBDIRS += nouveau
endif
+TESTS = \
+ drmsl
+
if HAVE_LIBUDEV
check_LTLIBRARIES = libdrmtest.la
@@ -52,7 +55,7 @@ XFAIL_TESTS = \
auth\
lock
-TESTS =\
+TESTS += \
openclose \
getversion \
getclient \
diff --git a/tests/drmsl.c b/tests/drmsl.c
new file mode 100644
index 000..d0ac0ef
--- /dev/null
+++ b/tests/drmsl.c
@@ -0,0 +1,172 @@
+/* drmsl.c -- Skip list test
+ * Created: Mon May 10 09:28:13 1999 by faith at precisioninsight.com
+ *
+ * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
+ * All Rights Reserved.
+ *
+ * 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 (including the next
+ * paragraph) 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
+ * PRECISION INSIGHT AND/OR ITS SUPPLIERS 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.
+ *
+ * Authors: Rickard E. (Rik) Faith
+ *
+ * DESCRIPTION
+ *
+ * This file contains a straightforward skip list implementation.n
+ *
+ * FUTURE ENHANCEMENTS
+ *
+ * REFERENCES
+ *
+ * [Pugh90] William Pugh. Skip Lists: A Probabilistic Alternative to
+ * Balanced Trees. CACM 33(6), June 1990, pp. 668-676.
+ *
+ */
+
+#include
+#include
+#include
+
+#include "xf86drm.h"
+
+static void print(void* list)
+{
+unsigned long key;
+void *value;
+
+if (drmSLFirst(list, , )) {
+ do {
+ printf("key = %5lu, value = %p\n", key, value);
+ } while (drmSLNext(list, , ));
+}
+}
+
+static double do_time(int size, int iter)
+{
+void *list;
+inti, j;
+unsigned long keys[100];
+unsigned long previous;
+unsigned long key;
+void *value;
+struct timeval start, stop;
+double usec;
+void *ranstate;
+
+list = drmSLCreate();
+ranstate = drmRandomCreate(12345);
+
+for (i = 0; i < size; i++) {
+ keys[i] = drmRandom(ranstate);
+ drmSLInsert(list, keys[i], NULL);
+}
+
+previous = 0;
+if (drmSLFirst(list, , )) {
+ do {
+ if (key <= previous) {
+ printf( "%lu !< %lu\n", previous, key);
+ }
+ previous = key;
+ } while (drmSLNext(list, , ));
+}
+
+gettimeofday(, NULL);
+for (j = 0; j < iter; j++) {
+ for (i = 0; i < size; i++) {
+ if (drmSLLookup(list, keys[i], ))
+ printf("Error %lu %d\n", keys[i], i);
+ }
+}
+gettimeofday(, NULL);
+
+usec = (double)(stop.tv_sec * 100 + stop.tv_usec
+ - start.tv_sec * 100 - start.tv_usec) / (size * iter);
+
+printf("%0.2f microseconds for list length %d\n", usec, size);
+
+drmRandomDouble(ranstate);
+drmSLDestroy(list);
+
+return usec;
+}
+
+stati
[PATCH libdrm 7/9] tests/random: return non-zero on test failure
On Sun, 2015-03-22 at 22:03 +, Emil Velikov wrote:
> ... and wire it up to make check
>
> Signed-off-by: Emil Velikov
> ---
> tests/Makefile.am | 6 +++---
> tests/random.c| 6 --
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 9b13b2e..0603241 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -29,15 +29,15 @@ LDADD = $(top_builddir)/libdrm.la
>
> check_PROGRAMS = \
> dristat \
> - drmstat \
> - random
> + drmstat
>
> if HAVE_NOUVEAU
> SUBDIRS += nouveau
> endif
>
> TESTS = \
> - hash
> + hash \
> + random
>
> if HAVE_LIBUDEV
>
> diff --git a/tests/random.c b/tests/random.c
> index 6dc8386..6af7d33 100644
> --- a/tests/random.c
> +++ b/tests/random.c
> @@ -107,15 +107,17 @@ int main(void)
> {
> RandomState *state;
> int i;
> +int ret;
> unsigned long rand;
>
> state = drmRandomCreate(1);
> for (i = 0; i < 1; i++) {
> rand = drmRandom(state);
> }
> +ret = rand - state->check;
Since you are touching this line, I think rand != state->check would be
more readable.
> printf("After 1 iterations: %lu (%lu expected): %s\n",
> rand, state->check,
> -rand - state->check ? "*INCORRECT*" : "CORRECT");
> +ret ? "*INCORRECT*" : "CORRECT");
> drmRandomDestroy(state);
>
> printf("Checking periods...\n");
> @@ -123,5 +125,5 @@ int main(void)
> check_period(2);
> check_period(31415926);
>
> -return 0;
> +return ret;
> }
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150324/98b53062/attachment.sig>
[PATCH libdrm 6/9] tests/random: extract test out of xf86drmRandom.c
unsigned long q; /* m div a */
> +unsigned long r; /* m mod a */
> +unsigned long check;
> +unsigned long seed;
> +} RandomState;
I think this should also go to an internal header. Otherwise lgtm
> +
> +static void check_period(unsigned long seed)
> +{
> +unsigned long count = 0;
> +unsigned long initial;
> +void *state;
> +
> +state = drmRandomCreate(seed);
> +initial = drmRandom(state);
> +++count;
> +while (initial != drmRandom(state)) {
> + if (!++count) break;
> +}
> +printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
> +seed, count, count);
> +drmRandomDestroy(state);
> +}
> +
> +int main(void)
> +{
> +RandomState *state;
> +int i;
> +unsigned long rand;
> +
> +state = drmRandomCreate(1);
> +for (i = 0; i < 1; i++) {
> + rand = drmRandom(state);
> +}
> +printf("After 1 iterations: %lu (%lu expected): %s\n",
> +rand, state->check,
> +rand - state->check ? "*INCORRECT*" : "CORRECT");
> +drmRandomDestroy(state);
> +
> +printf("Checking periods...\n");
> +check_period(1);
> +check_period(2);
> +check_period(31415926);
> +
> +return 0;
> +}
> diff --git a/xf86drmRandom.c b/xf86drmRandom.c
> index 94922ad..39f3c52 100644
> --- a/xf86drmRandom.c
> +++ b/xf86drmRandom.c
> @@ -74,23 +74,11 @@
> #include
> #include
>
> -#define RANDOM_MAIN 0
> -
> -#if !RANDOM_MAIN
> -# include "xf86drm.h"
> -#endif
> +#include "xf86drm.h"
>
> #define RANDOM_MAGIC 0xfeedbeef
> #define RANDOM_DEBUG 0
>
> -#if RANDOM_MAIN
> -#define RANDOM_ALLOC malloc
> -#define RANDOM_FREE free
> -#else
> -#define RANDOM_ALLOC drmMalloc
> -#define RANDOM_FREE drmFree
> -#endif
> -
> typedef struct RandomState {
> unsigned long magic;
> unsigned long a;
> @@ -101,18 +89,11 @@ typedef struct RandomState {
> unsigned long seed;
> } RandomState;
>
> -#if RANDOM_MAIN
> -extern void *drmRandomCreate(unsigned long seed);
> -extern int drmRandomDestroy(void *state);
> -extern unsigned long drmRandom(void *state);
> -extern doubledrmRandomDouble(void *state);
> -#endif
> -
> void *drmRandomCreate(unsigned long seed)
> {
> RandomState *state;
>
> -state = RANDOM_ALLOC(sizeof(*state));
> +state = drmMalloc(sizeof(*state));
> if (!state) return NULL;
> state->magic= RANDOM_MAGIC;
> #if 0
> @@ -140,7 +121,7 @@ void *drmRandomCreate(unsigned long seed)
>
> int drmRandomDestroy(void *state)
> {
> -RANDOM_FREE(state);
> +drmFree(state);
> return 0;
> }
>
> @@ -164,45 +145,3 @@ double drmRandomDouble(void *state)
>
> return (double)drmRandom(state)/(double)s->m;
> }
> -
> -#if RANDOM_MAIN
> -static void check_period(unsigned long seed)
> -{
> -unsigned long count = 0;
> -unsigned long initial;
> -void *state;
> -
> - state = drmRandomCreate(seed);
> -initial = drmRandom(state);
> -++count;
> -while (initial != drmRandom(state)) {
> - if (!++count) break;
> -}
> -printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
> -seed, count, count);
> -drmRandomDestroy(state);
> -}
> -
> -int main(void)
> -{
> -RandomState *state;
> -int i;
> -unsigned long rand;
> -
> -state = drmRandomCreate(1);
> -for (i = 0; i < 1; i++) {
> - rand = drmRandom(state);
> -}
> -printf("After 1 iterations: %lu (%lu expected): %s\n",
> -rand, state->check,
> -rand - state->check ? "*INCORRECT*" : "CORRECT");
> -drmRandomDestroy(state);
> -
> -printf("Checking periods...\n");
> -check_period(1);
> -check_period(2);
> -check_period(31415926);
> -
> -return 0;
> -}
> -#endif
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150324/0fdaf27b/attachment.sig>
[PATCH libdrm 9/9] drm: use correct printf modifiers
On Sun, 2015-03-22 at 22:03 +, Emil Velikov wrote:
> The valies are unsigned long, thus we should use %lu.
>
> Signed-off-by: Emil Velikov
> ---
> xf86drmHash.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xf86drmHash.c b/xf86drmHash.c
> index 12baa62..887d6a7 100644
> --- a/xf86drmHash.c
> +++ b/xf86drmHash.c
> @@ -119,7 +119,7 @@ static unsigned long HashHash(unsigned long key)
>
> hash %= HASH_SIZE;
> #if DEBUG
> -printf( "Hash(%d) = %d\n", key, hash);
> +printf( "Hash(%lu) = %lu\n", key, hash);
> #endif
> return hash;
> }
> @@ -221,8 +221,9 @@ int drmHashInsert(void *t, unsigned long key, void *value)
> bucket->value= value;
> bucket->next = table->buckets[hash];
> table->buckets[hash] = bucket;
> -#if DEBUG
> printf("Inserted %d at %d/%p\n", key, hash, bucket);
> +#if DEBUG
> +printf("Inserted %lu at %lu/%p\n", key, hash, bucket);
> #endif
^^ This change looks funny in the patch. I assume you just replace the
printf call? If that's so LGTM
Reviewed-by: Jan Vesely
> return 0;/* Added to table */
> }
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150323/f07ab2bf/attachment.sig>
[PATCH libdrm 8/9] drm: replace HASH_DEBUG with DEBUG
On Sun, 2015-03-22 at 22:03 +, Emil Velikov wrote:
> ... and remove the useless SL_DEBUG and RANDOM_DEBUG
>
> Signed-off-by: Emil Velikov
> ---
> xf86drmHash.c | 5 ++---
> xf86drmRandom.c | 1 -
> xf86drmSL.c | 1 -
> 3 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/xf86drmHash.c b/xf86drmHash.c
> index 82512a8..12baa62 100644
> --- a/xf86drmHash.c
> +++ b/xf86drmHash.c
> @@ -74,7 +74,6 @@
> #include "xf86drm.h"
>
> #define HASH_MAGIC 0xdeadbeef
> -#define HASH_DEBUG 0
> #define HASH_SIZE 512 /* Good for about 100 entries */
> /* If you change this value, you probably
> have to change the HashHash hashing
> @@ -119,7 +118,7 @@ static unsigned long HashHash(unsigned long key)
> }
>
> hash %= HASH_SIZE;
> -#if HASH_DEBUG
> +#if DEBUG
> printf( "Hash(%d) = %d\n", key, hash);
> #endif
> return hash;
> @@ -222,7 +221,7 @@ int drmHashInsert(void *t, unsigned long key, void *value)
> bucket->value= value;
> bucket->next = table->buckets[hash];
> table->buckets[hash] = bucket;
> -#if HASH_DEBUG
> +#if DEBUG
> printf("Inserted %d at %d/%p\n", key, hash, bucket);
> #endif
> return 0;/* Added to table */
> diff --git a/xf86drmRandom.c b/xf86drmRandom.c
> index 39f3c52..2177d27 100644
> --- a/xf86drmRandom.c
> +++ b/xf86drmRandom.c
> @@ -77,7 +77,6 @@
> #include "xf86drm.h"
>
> #define RANDOM_MAGIC 0xfeedbeef
> -#define RANDOM_DEBUG 0
>
> typedef struct RandomState {
> unsigned long magic;
> diff --git a/xf86drmSL.c b/xf86drmSL.c
> index acddb54..9bbf8fb 100644
> --- a/xf86drmSL.c
> +++ b/xf86drmSL.c
> @@ -53,7 +53,6 @@
> #define SL_ENTRY_MAGIC 0x00fab1edLU
> #define SL_FREED_MAGIC 0xdecea5edLU
> #define SL_MAX_LEVEL 16
> -#define SL_DEBUG 0
> #define SL_RANDOM_SEED 0xc01055a1LU
>
> #if SL_MAIN
Reviewed-by: Jan Vesely
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150323/ddb41366/attachment.sig>
[PATCH libdrm 5/9] tests/hash: return non-zero on failure
On Sun, 2015-03-22 at 22:03 +, Emil Velikov wrote:
> ... and wire up to `make check' now that it's useful.
>
> Signed-off-by: Emil Velikov
> ---
> tests/Makefile.am | 12 +++-
> tests/hash.c | 26 +++---
> 2 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index ea826b5..392abf5 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -29,13 +29,15 @@ LDADD = $(top_builddir)/libdrm.la
>
> check_PROGRAMS = \
> dristat \
> - drmstat \
> - hash
> + drmstat
>
> if HAVE_NOUVEAU
> SUBDIRS += nouveau
> endif
>
> +TESTS = \
> + hash
> +
> if HAVE_LIBUDEV
>
> check_LTLIBRARIES = libdrmtest.la
> @@ -53,7 +55,7 @@ XFAIL_TESTS = \
> auth\
> lock
>
> -TESTS = \
> +TESTS += \
> openclose \
> getversion \
> getclient \
> @@ -62,6 +64,6 @@ TESTS = \
> updatedraw \
> name_from_fd
>
> -check_PROGRAMS += $(TESTS)
> -
> endif
> +
> +check_PROGRAMS += $(TESTS)
> diff --git a/tests/hash.c b/tests/hash.c
> index fa9264a..46f52f8 100644
> --- a/tests/hash.c
> +++ b/tests/hash.c
> @@ -142,7 +142,7 @@ static void compute_dist(HashTablePtr table)
> }
> }
>
> -static void check_table(HashTablePtr table,
> +static int check_table(HashTablePtr table,
> unsigned long key, unsigned long value)
> {
> unsigned long *retval;
> @@ -159,28 +159,32 @@ static void check_table(HashTablePtr table,
> key, value, *retval);
> break;
> case 0:
> -if (value != *retval)
> +if (value != *retval) {
> printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
> key, value, *retval);
> +retcode = -1;
> +}
> break;
> default:
> printf("Bad retcode = %d: key = %lu, expected = %lu, returned =
> %lu\n",
> retcode, key, value, *retval);
> break;
> }
> +return retcode;
> }
>
> int main(void)
> {
> -HashTablePtr table;
> -unsigned long i;
> +HashTablePtr table;
> +unsigned long i;
> +int ret = 0;
>
> printf("\n* 256 consecutive integers \n");
> table = drmHashCreate();
> for (i = 0; i < 256; i++)
> drmHashInsert(table, i, (void *));
> for (i = 0; i < 256; i++)
> -check_table(table, i, i);
> +ret = check_table(table, i, i) && ret;
> compute_dist(table);
> drmHashDestroy(table);
>
> @@ -189,7 +193,7 @@ int main(void)
> for (i = 0; i < 1024; i++)
> drmHashInsert(table, i, (void *));
> for (i = 0; i < 1024; i++)
> -check_table(table, i, i);
> +ret = check_table(table, i, i) && ret;
> compute_dist(table);
> drmHashDestroy(table);
>
> @@ -198,7 +202,7 @@ int main(void)
> for (i = 0; i < 1024; i++)
> drmHashInsert(table, i*4096, (void *));
> for (i = 0; i < 1024; i++)
> -check_table(table, i*4096, i);
> +ret = check_table(table, i*4096, i) && ret;
> compute_dist(table);
> drmHashDestroy(table);
>
> @@ -209,10 +213,10 @@ int main(void)
> drmHashInsert(table, random(), (void *));
> srandom(0xbeefbeef);
> for (i = 0; i < 1024; i++)
> -check_table(table, random(), i);
> +ret = check_table(table, random(), i) && ret;
> srandom(0xbeefbeef);
> for (i = 0; i < 1024; i++)
> -check_table(table, random(), i);
> +ret = check_table(table, random(), i) && ret;
> compute_dist(table);
> drmHashDestroy(table);
>
> @@ -223,10 +227,10 @@ int main(void)
> drmHashInsert(table, random(), (void *));
> srandom(0xbeefbeef);
> for (i = 0; i < 5000; i++)
> -check_table(table, random(), i);
> +ret = check_table(table, random(), i) && ret;
> srandom(0xbeefbeef);
> for (i = 0; i < 5000; i++)
> -check_table(table, random(), i);
> +ret = check_table(table, random(), i) && ret;
> compute_dist(table);
> drmHashDestroy(table);
>
Reviewed-by: Jan Vesely
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150323/bea5ac91/attachment.sig>
[PATCH libdrm 4/9] tests/hash: style fixes
On Sun, 2015-03-22 at 22:03 +, Emil Velikov wrote:
> Signed-off-by: Emil Velikov
> ---
> tests/hash.c | 102
> +++
> 1 file changed, 60 insertions(+), 42 deletions(-)
>
> diff --git a/tests/hash.c b/tests/hash.c
> index 902919f..fa9264a 100644
> --- a/tests/hash.c
> +++ b/tests/hash.c
> @@ -73,8 +73,8 @@
>
> #include "xf86drm.h"
>
> -#define HASH_SIZE 512 /* Good for about 100 entries */
> - /* If you change this value, you probably
> +#define HASH_SIZE 512 /* Good for about 100 entries */
> +/* If you change this value, you probably
> have to change the HashHash hashing
> function! */
>
> @@ -87,9 +87,9 @@ typedef struct HashBucket {
> typedef struct HashTable {
> unsigned longmagic;
> unsigned longentries;
> -unsigned longhits; /* At top of linked list */
> -unsigned longpartials; /* Not at top of linked list */
> -unsigned longmisses; /* Not in table */
> +unsigned longhits; /* At top of linked list */
> +unsigned longpartials; /* Not at top of linked list */
> +unsigned longmisses;/* Not in table */
> HashBucketPtrbuckets[HASH_SIZE];
> int p0;
> HashBucketPtrp1;
> @@ -101,21 +101,25 @@ static int dist[DIST_LIMIT];
> static void clear_dist(void) {
> int i;
>
> -for (i = 0; i < DIST_LIMIT; i++) dist[i] = 0;
> +for (i = 0; i < DIST_LIMIT; i++)
> +dist[i] = 0;
> }
>
> static int count_entries(HashBucketPtr bucket)
> {
> -int count = 0;
> +int count;
>
> -for (; bucket; bucket = bucket->next) ++count;
> +for (count = 0; bucket; bucket = bucket->next)
> +++count;
I personally prefer to initialize early, especially since it's not
for-loop iterating variable, but I don't insist.
Reviewed-by: Jan Vesely
> return count;
> }
>
> static void update_dist(int count)
> {
> -if (count >= DIST_LIMIT) ++dist[DIST_LIMIT-1];
> -else ++dist[count];
> +if (count >= DIST_LIMIT)
> +++dist[DIST_LIMIT-1];
> +else
> +++dist[count];
> }
>
> static void compute_dist(HashTablePtr table)
> @@ -124,43 +128,45 @@ static void compute_dist(HashTablePtr table)
> HashBucketPtr bucket;
>
> printf("Entries = %ld, hits = %ld, partials = %ld, misses = %ld\n",
> -table->entries, table->hits, table->partials, table->misses);
> + table->entries, table->hits, table->partials, table->misses);
> clear_dist();
> for (i = 0; i < HASH_SIZE; i++) {
> - bucket = table->buckets[i];
> - update_dist(count_entries(bucket));
> +bucket = table->buckets[i];
> +update_dist(count_entries(bucket));
> }
> for (i = 0; i < DIST_LIMIT; i++) {
> - if (i != DIST_LIMIT-1) printf("%5d %10d\n", i, dist[i]);
> - else printf("other %10d\n", dist[i]);
> +if (i != DIST_LIMIT-1)
> +printf("%5d %10d\n", i, dist[i]);
> +else
> +printf("other %10d\n", dist[i]);
> }
> }
>
> static void check_table(HashTablePtr table,
> - unsigned long key, unsigned long value)
> +unsigned long key, unsigned long value)
> {
> unsigned long *retval;
> int retcode = drmHashLookup(table, key, (void **));
>
> switch (retcode) {
> case -1:
> - printf("Bad magic = 0x%08lx:"
> -" key = %lu, expected = %lu, returned = %lu\n",
> -table->magic, key, value, *retval);
> - break;
> +printf("Bad magic = 0x%08lx:"
> + " key = %lu, expected = %lu, returned = %lu\n",
> + table->magic, key, value, *retval);
> +break;
> case 1:
> - printf("Not found: key = %lu, expected = %lu, returned = %lu\n",
> -key, value, *retval);
> - break;
> +printf("Not found: key = %lu, expected = %lu, returned = %lu\n",
> + key, value, *retval);
> +break;
> case 0:
> - if (value != *retval)
> - printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
> -key, value, *retval);
> - break;
> +if (value != *retval)
> +print
[PATCH libdrm 3/9] tests/hash: misc compilation fixes
On Sun, 2015-03-22 at 22:03 +, Emil Velikov wrote:
> Get the test from completely broken to working like a charm.
>
> - Use the same variable type for both HashInsert and HashLookup.
> - Use correct storage type for the HashLookup return value.
> - Remove useless backward iteration of HashLookup(i).
>
> Signed-off-by: Emil Velikov
> ---
> tests/hash.c | 31 ++-
> 1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/tests/hash.c b/tests/hash.c
> index d57d2dc..902919f 100644
> --- a/tests/hash.c
> +++ b/tests/hash.c
> @@ -139,27 +139,27 @@ static void compute_dist(HashTablePtr table)
> static void check_table(HashTablePtr table,
> unsigned long key, unsigned long value)
I think we should use void* for value here.
> {
> -unsigned long retval = 0;
> -int retcode = drmHashLookup(table, key, );
> +unsigned long *retval;
> +int retcode = drmHashLookup(table, key, (void **));
I don't think this is correct. If the entry is found, it stores address
to stack variable i in retval, which at this point in iteration
incidentally contains the value we are looking for.
>
> switch (retcode) {
> case -1:
> printf("Bad magic = 0x%08lx:"
> " key = %lu, expected = %lu, returned = %lu\n",
> -table->magic, key, value, retval);
> +table->magic, key, value, *retval);
> break;
> case 1:
> - printf("Not found: key = %lu, expected = %lu returned = %lu\n",
> -key, value, retval);
> + printf("Not found: key = %lu, expected = %lu, returned = %lu\n",
> +key, value, *retval);
> break;
> case 0:
> - if (value != retval)
> + if (value != *retval)
> printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
> -key, value, retval);
> +key, value, *retval);
> break;
> default:
> printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n",
> -retcode, key, value, retval);
> +retcode, key, value, *retval);
> break;
> }
> }
> @@ -167,36 +167,33 @@ static void check_table(HashTablePtr table,
> int main(void)
> {
> HashTablePtr table;
> -int i;
> +unsigned long i;
>
> printf("\n* 256 consecutive integers \n");
> table = drmHashCreate();
> -for (i = 0; i < 256; i++) drmHashInsert(table, i, i);
> +for (i = 0; i < 256; i++) drmHashInsert(table, i, (void *));
This changes the entries inserted. previously it would insert values [0,
256). Now it inserts address of i 256 times.
I think we should change the inserted values to be different from the
key (offset should be enough), to catch the kind of scenario this tests
creates.
> for (i = 0; i < 256; i++) check_table(table, i, i);
> -for (i = 256; i >= 0; i--) check_table(table, i, i);
> compute_dist(table);
> drmHashDestroy(table);
>
> printf("\n* 1024 consecutive integers \n");
> table = drmHashCreate();
> -for (i = 0; i < 1024; i++) drmHashInsert(table, i, i);
> +for (i = 0; i < 1024; i++) drmHashInsert(table, i, (void *));
> for (i = 0; i < 1024; i++) check_table(table, i, i);
> -for (i = 1024; i >= 0; i--) check_table(table, i, i);
> compute_dist(table);
> drmHashDestroy(table);
>
> printf("\n* 1024 consecutive page addresses (4k pages) \n");
> table = drmHashCreate();
> -for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, i);
> +for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, (void *));
> for (i = 0; i < 1024; i++) check_table(table, i*4096, i);
> -for (i = 1024; i >= 0; i--) check_table(table, i*4096, i);
> compute_dist(table);
> drmHashDestroy(table);
>
> printf("\n* 1024 random integers \n");
> table = drmHashCreate();
> srandom(0xbeefbeef);
> -for (i = 0; i < 1024; i++) drmHashInsert(table, random(), i);
> +for (i = 0; i < 1024; i++) drmHashInsert(table, random(), (void *));
> srandom(0xbeefbeef);
> for (i = 0; i < 1024; i++) check_table(table, random(), i);
> srandom(0xbeefbeef);
> @@ -207,7 +204,7 @@ int main(void)
> printf("\n* 5000 random integers \n");
> table = drmHashCreate();
> srandom(0xbeefbeef);
> -for (i = 0; i < 5000; i++) drmHashInsert(table, random(), i);
> +for (i = 0; i < 5000; i++) drmHashInsert(table, random(), (void *));
> srandom(0xbeefbeef);
> for (i = 0; i < 5000; i++) check_table(table, random(), i);
> srandom(0xbeefbeef);
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150323/0a4319c6/attachment.sig>
[PATCH libdrm 2/9] tests/hash: extract test out of xf86drmHash.c
te_dist(HashTablePtr table)
> -{
> -int i;
> -HashBucketPtr bucket;
> -
> -printf("Entries = %ld, hits = %ld, partials = %ld, misses = %ld\n",
> -table->entries, table->hits, table->partials, table->misses);
> -clear_dist();
> -for (i = 0; i < HASH_SIZE; i++) {
> - bucket = table->buckets[i];
> - update_dist(count_entries(bucket));
> -}
> -for (i = 0; i < DIST_LIMIT; i++) {
> - if (i != DIST_LIMIT-1) printf("%5d %10d\n", i, dist[i]);
> - else printf("other %10d\n", dist[i]);
> -}
> -}
> -
> -static void check_table(HashTablePtr table,
> - unsigned long key, unsigned long value)
> -{
> -unsigned long retval = 0;
> -int retcode = drmHashLookup(table, key, );
> -
> -switch (retcode) {
> -case -1:
> - printf("Bad magic = 0x%08lx:"
> -" key = %lu, expected = %lu, returned = %lu\n",
> -table->magic, key, value, retval);
> - break;
> -case 1:
> - printf("Not found: key = %lu, expected = %lu returned = %lu\n",
> -key, value, retval);
> - break;
> -case 0:
> - if (value != retval)
> - printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
> -key, value, retval);
> - break;
> -default:
> - printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n",
> -retcode, key, value, retval);
> - break;
> -}
> -}
> -
> -int main(void)
> -{
> -HashTablePtr table;
> -int i;
> -
> -printf("\n* 256 consecutive integers \n");
> -table = drmHashCreate();
> -for (i = 0; i < 256; i++) drmHashInsert(table, i, i);
> -for (i = 0; i < 256; i++) check_table(table, i, i);
> -for (i = 256; i >= 0; i--) check_table(table, i, i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -printf("\n* 1024 consecutive integers \n");
> -table = drmHashCreate();
> -for (i = 0; i < 1024; i++) drmHashInsert(table, i, i);
> -for (i = 0; i < 1024; i++) check_table(table, i, i);
> -for (i = 1024; i >= 0; i--) check_table(table, i, i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -printf("\n* 1024 consecutive page addresses (4k pages) \n");
> -table = drmHashCreate();
> -for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, i);
> -for (i = 0; i < 1024; i++) check_table(table, i*4096, i);
> -for (i = 1024; i >= 0; i--) check_table(table, i*4096, i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -printf("\n* 1024 random integers \n");
> -table = drmHashCreate();
> -srandom(0xbeefbeef);
> -for (i = 0; i < 1024; i++) drmHashInsert(table, random(), i);
> -srandom(0xbeefbeef);
> -for (i = 0; i < 1024; i++) check_table(table, random(), i);
> -srandom(0xbeefbeef);
> -for (i = 0; i < 1024; i++) check_table(table, random(), i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -printf("\n* 5000 random integers \n");
> -table = drmHashCreate();
> -srandom(0xbeefbeef);
> -for (i = 0; i < 5000; i++) drmHashInsert(table, random(), i);
> -srandom(0xbeefbeef);
> -for (i = 0; i < 5000; i++) check_table(table, random(), i);
> -srandom(0xbeefbeef);
> -for (i = 0; i < 5000; i++) check_table(table, random(), i);
> -compute_dist(table);
> -drmHashDestroy(table);
> -
> -return 0;
> -}
> -#endif
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150323/a40241e3/attachment-0001.sig>
[PATCH libdrm 1/9] tests/dristat: don't include C files
On Sun, 2015-03-22 at 22:03 +, Emil Velikov wrote:
> Remove the hack of including C files, by reworking the only requirement
> drmOpenMinor() to an open(buf...). After all we do know the exact name
> of the device we're going to open, so might as well use it. Replace
> hard-coded 16 with DRM_MAX_MINOR while we're here.
>
> Signed-off-by: Emil Velikov
> ---
> tests/dristat.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/tests/dristat.c b/tests/dristat.c
> index cca4b03..cc23e16 100644
> --- a/tests/dristat.c
> +++ b/tests/dristat.c
> @@ -31,13 +31,14 @@
> # include
> #endif
>
> +#include
> +#include
> #include
> #include
> +#include
> +#include
> #include
> #include "xf86drm.h"
> -#include "xf86drmRandom.c"
> -#include "xf86drmHash.c"
> -#include "xf86drm.c"
>
> #define DRM_VERSION 0x0001
> #define DRM_MEMORY 0x0002
> @@ -267,9 +268,9 @@ int main(int argc, char **argv)
> return 1;
> }
>
> -for (i = 0; i < 16; i++) if (!minor || i == minor) {
> +for (i = 0; i < DRM_MAX_MINOR; i++) if (!minor || i == minor) {
> sprintf(buf, DRM_DEV_NAME, DRM_DIR_NAME, i);
> - fd = drmOpenMinor(i, 1, DRM_NODE_PRIMARY);
> + fd = open(buf, O_RDWR, 0);
What about the "create" (second) argument? The original code creates the
node if it does not exist (on non-UDEV systems), or waits some time for
udev to create it.
Not sure how this is relevant for the test.
> if (fd >= 0) {
> printf("%s\n", buf);
> if (mask & DRM_BUSID) getbusid(fd);
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150323/aecd6028/attachment.sig>
[PATCH libdrm 1/1] tests/exynos: Fix warnings
On Wed, 2015-03-18 at 20:23 +0100, Tobias Jakobi wrote:
> Hello Jan,
>
> Jan Vesely wrote:
> > Signed-off-by: Jan Vesely
> > ---
> > tests/exynos/exynos_fimg2d_test.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/exynos/exynos_fimg2d_test.c
> > b/tests/exynos/exynos_fimg2d_test.c
> > index e7d9b72..dfb34ac 100644
> > --- a/tests/exynos/exynos_fimg2d_test.c
> > +++ b/tests/exynos/exynos_fimg2d_test.c
> > @@ -183,7 +183,7 @@ static struct exynos_bo *exynos_create_buffer(struct
> > exynos_device *dev,
> >
> > /* Allocate buffer and fill it with checkerboard pattern, where the tiles *
> > * have a random color. The caller has to free the buffer.
> > */
> > -void *create_checkerboard_pattern(unsigned int num_tiles_x,
> > +static void *create_checkerboard_pattern(unsigned int num_tiles_x,
> > unsigned int num_tiles_y,
> > unsigned int tile_size)
> > {
> > unsigned int *buf;
> Good catch with the missing static!
May I consider this a R-b for this change?
I have moved the switch-enum fix to a separate patch (that covers all
switch-enum warnings)
jan
>
>
> > @@ -573,6 +573,7 @@ static int g2d_checkerboard_test(struct exynos_device
> > *dev,
> > src_img.user_ptr[0].userptr = (unsigned long)checkerboard;
> > src_img.user_ptr[0].size = img_w * img_h * 4;
> > break;
> > + case G2D_IMGBUF_COLOR:
> > default:
> > ret = -EFAULT;
> > goto fail;
> >
> Hmm, I don't see the reason why this label should be added to the switch
> statement?
>
> With best wishes,
> Tobias
>
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150320/159baaa6/attachment.sig>
[PATCH libdrm 4/4] drmSL: Remove test parts
Signed-off-by: Jan Vesely
---
xf86drmSL.c | 172 +++-
1 file changed, 6 insertions(+), 166 deletions(-)
diff --git a/xf86drmSL.c b/xf86drmSL.c
index cf588ac..bb9ca7f 100644
--- a/xf86drmSL.c
+++ b/xf86drmSL.c
@@ -41,13 +41,7 @@
#include
#include
-#define SL_MAIN 0
-
-#if !SL_MAIN
-# include "xf86drm.h"
-#else
-# include
-#endif
+#include "xf86drm.h"
#define SL_LIST_MAGIC 0xfacade00LU
#define SL_ENTRY_MAGIC 0x00fab1edLU
@@ -56,21 +50,10 @@
#define SL_DEBUG 0
#define SL_RANDOM_SEED 0xc01055a1LU
-#if SL_MAIN
-#define SL_ALLOC malloc
-#define SL_FREE free
-#define SL_RANDOM_DECLstatic int state = 0;
-#define SL_RANDOM_INIT(seed) if (!state) { srandom(seed); ++state; }
-#define SL_RANDOM random()
-#else
-#define SL_ALLOC drmMalloc
-#define SL_FREE drmFree
#define SL_RANDOM_DECLstatic void *state = NULL
#define SL_RANDOM_INIT(seed) if (!state) state = drmRandomCreate(seed)
#define SL_RANDOM drmRandom(state)
-#endif
-
typedef struct SLEntry {
unsigned long magic; /* SL_ENTRY_MAGIC */
unsigned long key;
@@ -87,27 +70,13 @@ typedef struct SkipList {
SLEntryPtr p0; /* Position for iteration */
} SkipList, *SkipListPtr;
-#if SL_MAIN
-extern void *drmSLCreate(void);
-extern int drmSLDestroy(void *l);
-extern int drmSLLookup(void *l, unsigned long key, void **value);
-extern int drmSLInsert(void *l, unsigned long key, void *value);
-extern int drmSLDelete(void *l, unsigned long key);
-extern int drmSLNext(void *l, unsigned long *key, void **value);
-extern int drmSLFirst(void *l, unsigned long *key, void **value);
-extern void drmSLDump(void *l);
-extern int drmSLLookupNeighbors(void *l, unsigned long key,
-unsigned long *prev_key, void **prev_value,
-unsigned long *next_key, void **next_value);
-#endif
-
static SLEntryPtr SLCreateEntry(int max_level, unsigned long key, void *value)
{
SLEntryPtr entry;
if (max_level < 0 || max_level > SL_MAX_LEVEL) max_level = SL_MAX_LEVEL;
-entry = SL_ALLOC(sizeof(*entry)
+entry = drmMalloc(sizeof(*entry)
+ (max_level + 1) * sizeof(entry->forward[0]));
if (!entry) return NULL;
entry->magic = SL_ENTRY_MAGIC;
@@ -134,7 +103,7 @@ void *drmSLCreate(void)
SkipListPtr list;
int i;
-list = SL_ALLOC(sizeof(*list));
+list = drmMalloc(sizeof(*list));
if (!list) return NULL;
list->magic= SL_LIST_MAGIC;
list->level= 0;
@@ -158,11 +127,11 @@ int drmSLDestroy(void *l)
if (entry->magic != SL_ENTRY_MAGIC) return -1; /* Bad magic */
next = entry->forward[0];
entry->magic = SL_FREED_MAGIC;
- SL_FREE(entry);
+ drmFree(entry);
}
list->magic = SL_FREED_MAGIC;
-SL_FREE(list);
+drmFree(list);
return 0;
}
@@ -236,7 +205,7 @@ int drmSLDelete(void *l, unsigned long key)
}
entry->magic = SL_FREED_MAGIC;
-SL_FREE(entry);
+drmFree(entry);
while (list->level && !list->head->forward[list->level]) --list->level;
--list->count;
@@ -348,132 +317,3 @@ void drmSLDump(void *l)
}
}
}
-
-#if SL_MAIN
-static void print(SkipListPtr list)
-{
-unsigned long key;
-void *value;
-
-if (drmSLFirst(list, , )) {
- do {
- printf("key = %5lu, value = %p\n", key, value);
- } while (drmSLNext(list, , ));
-}
-}
-
-static double do_time(int size, int iter)
-{
-SkipListPtrlist;
-inti, j;
-unsigned long keys[100];
-unsigned long previous;
-unsigned long key;
-void *value;
-struct timeval start, stop;
-double usec;
-SL_RANDOM_DECL;
-
-SL_RANDOM_INIT(12345);
-
-list = drmSLCreate();
-
-for (i = 0; i < size; i++) {
- keys[i] = SL_RANDOM;
- drmSLInsert(list, keys[i], NULL);
-}
-
-previous = 0;
-if (drmSLFirst(list, , )) {
- do {
- if (key <= previous) {
- printf( "%lu !< %lu\n", previous, key);
- }
- previous = key;
- } while (drmSLNext(list, , ));
-}
-
-gettimeofday(, NULL);
-for (j = 0; j < iter; j++) {
- for (i = 0; i < size; i++) {
- if (drmSLLookup(list, keys[i], ))
- printf("Error %lu %d\n", keys[i], i);
- }
-}
-gettimeofday(, NULL);
-
-usec = (double)(stop.tv_sec * 100 + stop.tv_usec
- - start.tv_sec * 100 - start.tv_usec) / (size * iter);
-
-printf("%0.2f microseconds for list length %d\n", usec, size);
-
-drmSLDestroy(list);
-
-return usec;
-}
-
-static void print_neighb
[PATCH libdrm 3/4] drmsltest: Check expected neighbours
Signed-off-by: Jan Vesely
---
tests/drmsltest.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/tests/drmsltest.c b/tests/drmsltest.c
index d193008..d871fbf 100644
--- a/tests/drmsltest.c
+++ b/tests/drmsltest.c
@@ -106,7 +106,9 @@ static double do_time(int size, int iter)
return usec;
}
-static void print_neighbors(void *list, unsigned long key)
+static void print_neighbors(void *list, unsigned long key,
+unsigned long expected_prev,
+unsigned long expected_next)
{
unsigned long prev_key = 0;
unsigned long next_key = 0;
@@ -119,6 +121,16 @@ static void print_neighbors(void *list, unsigned long key)
_key, _value);
printf("Neighbors of %5lu: %d %5lu %5lu\n",
key, retval, prev_key, next_key);
+if (prev_key != expected_prev) {
+fprintf(stderr, "Unexpected neighbor: %5lu. Expected: %5lu\n",
+prev_key, expected_prev);
+ exit(1);
+}
+if (next_key != expected_next) {
+fprintf(stderr, "Unexpected neighbor: %5lu. Expected: %5lu\n",
+next_key, expected_next);
+ exit(1);
+}
}
int main(void)
@@ -138,13 +150,13 @@ int main(void)
print(list);
printf("\n==\n\n");
-print_neighbors(list, 0);
-print_neighbors(list, 50);
-print_neighbors(list, 51);
-print_neighbors(list, 123);
-print_neighbors(list, 200);
-print_neighbors(list, 213);
-print_neighbors(list, 256);
+print_neighbors(list, 0, 0, 50);
+print_neighbors(list, 50, 0, 50);
+print_neighbors(list, 51, 50, 123);
+print_neighbors(list, 123, 50, 123);
+print_neighbors(list, 200, 123, 213);
+print_neighbors(list, 213, 123, 213);
+print_neighbors(list, 256, 213, 256);
printf("\n==\n\n");
drmSLDelete(list, 50);
--
2.1.0
[PATCH libdrm 2/4] drmSL: Split tests to a separate file
Signed-off-by: Jan Vesely
---
.gitignore| 1 +
tests/Makefile.am | 3 +-
tests/drmsltest.c | 172 ++
3 files changed, 175 insertions(+), 1 deletion(-)
create mode 100644 tests/drmsltest.c
diff --git a/.gitignore b/.gitignore
index 06cc928..9c6ecd7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -75,6 +75,7 @@ via.kld
tests/auth
tests/dristat
tests/drmstat
+tests/drmsltest
tests/getclient
tests/getstats
tests/getversion
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 10f54e3..ca0f3c7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -59,7 +59,8 @@ TESTS = \
getstats\
setversion \
updatedraw \
- name_from_fd
+ name_from_fd\
+ drmsltest
check_PROGRAMS += $(TESTS)
diff --git a/tests/drmsltest.c b/tests/drmsltest.c
new file mode 100644
index 000..d193008
--- /dev/null
+++ b/tests/drmsltest.c
@@ -0,0 +1,172 @@
+/* xf86drmSL.c -- Skip list support
+ * Created: Mon May 10 09:28:13 1999 by faith at precisioninsight.com
+ *
+ * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
+ * All Rights Reserved.
+ *
+ * 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 (including the next
+ * paragraph) 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
+ * PRECISION INSIGHT AND/OR ITS SUPPLIERS 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.
+ *
+ * Authors: Rickard E. (Rik) Faith
+ *
+ * DESCRIPTION
+ *
+ * This file contains a straightforward skip list implementation.n
+ *
+ * FUTURE ENHANCEMENTS
+ *
+ * REFERENCES
+ *
+ * [Pugh90] William Pugh. Skip Lists: A Probabilistic Alternative to
+ * Balanced Trees. CACM 33(6), June 1990, pp. 668-676.
+ *
+ */
+
+#include
+#include
+#include
+
+#include "xf86drm.h"
+
+static void print(void* list)
+{
+unsigned long key;
+void *value;
+
+if (drmSLFirst(list, , )) {
+ do {
+ printf("key = %5lu, value = %p\n", key, value);
+ } while (drmSLNext(list, , ));
+}
+}
+
+static double do_time(int size, int iter)
+{
+void *list;
+inti, j;
+unsigned long keys[100];
+unsigned long previous;
+unsigned long key;
+void *value;
+struct timeval start, stop;
+double usec;
+void *ranstate;
+
+list = drmSLCreate();
+ranstate = drmRandomCreate(12345);
+
+for (i = 0; i < size; i++) {
+ keys[i] = drmRandom(ranstate);
+ drmSLInsert(list, keys[i], NULL);
+}
+
+previous = 0;
+if (drmSLFirst(list, , )) {
+ do {
+ if (key <= previous) {
+ printf( "%lu !< %lu\n", previous, key);
+ }
+ previous = key;
+ } while (drmSLNext(list, , ));
+}
+
+gettimeofday(, NULL);
+for (j = 0; j < iter; j++) {
+ for (i = 0; i < size; i++) {
+ if (drmSLLookup(list, keys[i], ))
+ printf("Error %lu %d\n", keys[i], i);
+ }
+}
+gettimeofday(, NULL);
+
+usec = (double)(stop.tv_sec * 100 + stop.tv_usec
+ - start.tv_sec * 100 - start.tv_usec) / (size * iter);
+
+printf("%0.2f microseconds for list length %d\n", usec, size);
+
+drmRandomDouble(ranstate);
+drmSLDestroy(list);
+
+return usec;
+}
+
+static void print_neighbors(void *list, unsigned long key)
+{
+unsigned long prev_key = 0;
+unsigned long next_key = 0;
+void *prev_value;
+void *next_value;
+int retval;
+
+retval = drmSLLookupNeighbors(list, key,
+ _key, _value,
+ _key, _value);
+printf("Neighbors of %5lu: %d %5lu %5lu\n",
+ key, retval, prev_key, next_key);
+}
+
+int main(void)
+{
+void*list;
+double usec, usec2, u
[PATCH libdrm 1/4] drmSL: Fix neighbor printing
v2: zero the update array instead of checking the return value.
SLLocate returns NULL both on failure and if the element is greater
than everything in the list
Signed-off-by: Jan Vesely
---
xf86drmSL.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xf86drmSL.c b/xf86drmSL.c
index acddb54..cf588ac 100644
--- a/xf86drmSL.c
+++ b/xf86drmSL.c
@@ -264,12 +264,14 @@ int drmSLLookupNeighbors(void *l, unsigned long key,
unsigned long *next_key, void **next_value)
{
SkipListPtr list = (SkipListPtr)l;
-SLEntryPtrupdate[SL_MAX_LEVEL + 1];
+SLEntryPtrupdate[SL_MAX_LEVEL + 1] = {0};
int retcode = 0;
+SLLocate(list, key, update);
+
*prev_key = *next_key = key;
*prev_value = *next_value = NULL;
-
+
if (update[0]) {
*prev_key = update[0]->key;
*prev_value = update[0]->value;
--
2.1.0
[PATCH libdrm 4/8] xf86drmSL: Fix neighbour printing
On Fri, 2015-03-20 at 14:01 -0400, Jan Vesely wrote:
> On Fri, 2015-03-20 at 17:38 +, Emil Velikov wrote:
> > On 27/02/15 18:07, Jan Vesely wrote:
> > > Signed-off-by: Jan Vesely
> > > ---
> > > xf86drmSL.c | 7 +--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/xf86drmSL.c b/xf86drmSL.c
> > > index acddb54..2160bb8 100644
> > > --- a/xf86drmSL.c
> > > +++ b/xf86drmSL.c
> > > @@ -266,11 +266,14 @@ int drmSLLookupNeighbors(void *l, unsigned long key,
> > > SkipListPtr list = (SkipListPtr)l;
> > > SLEntryPtrupdate[SL_MAX_LEVEL + 1];
> > > int retcode = 0;
> > > +SLEntryPtrentry;
> > > +
> > > +entry = SLLocate(list, key, update);
> > >
> > > *prev_key = *next_key = key;
> > > *prev_value = *next_value = NULL;
> > > -
> > > -if (update[0]) {
> > > +
> > > +if (entry && update[0]) {
> > From a very brief look at git log, the entry check should not be needed.
> > Must admit that I've not looked at all in the implementation of either
> > SLLocate or drmSLLookupNeighbors.
>
> SLLocate might return early and leave the array uninitialized. All other
> calls to it check the return value. I guess the warning that the
> previous commit tried to fix was "set-but-unused" variable.
I take this back. You were right. SLLocate might return NULL in
non-failing case, and we cannot rely on the return value. I'll post an
updated patch (and a move to tests) shortly
jan
>
> >
> > That said it seems that none of the three files
> > (xf86drm{SL,Hash,Random}) has been build as a program for a while. Maybe
> > we could split it out as a standalone test and let it churn at make
> > check time ?
>
> Sounds like a good idea. I'll try to take a look when time permits, but
> I'd leave that as a separate patch.
>
> thanks,
> jan
>
> >
> > Cheers,
> > Emil
>
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150320/8257aacf/attachment.sig>
[PATCH libdrm 9/8] Remove drmSetDebugMsgFunction and related infrastructure
On Fri, 2015-03-20 at 21:06 +, Emil Velikov wrote: > On 18 March 2015 at 18:37, Jan Vesely wrote: > > Not used anywhere > > > Some information from my digging around: > > Function was added with commit 79038751ffe(libdrm: add support for > server side functionality in libdrm). > It's not referenced even once in the history of the following projects > - xserver > - plymouth > - mesa > - xf86-video-ati > -xf86-video-freedreno > - xf86-video-i128 > - xf86-video-i740 > - xf86-video-intel > - xf86-video-mach64 > - xf86-video-mga > - xf86-video-nouveau > - xf86-video-r128 > - xf86-video-tdfx > - xf86-video-savage > - xf86-video-sis > - xf86-video-via > > So I'm pretty confident that we're safe to nuke it. The above ddx list > should consist of all the drivers/hardware that was ever officially > supported by drm - from dri1 era until now. > > Reviewed-by: Emil Velikov thanks for checking those. I still don;t have a clear idea which projects actually use libdrm (other than mesa) jan > > -Emil -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150320/73c96ac5/attachment.sig>
[PATCH libdrm 4/8] xf86drmSL: Fix neighbour printing
On Fri, 2015-03-20 at 17:38 +, Emil Velikov wrote:
> On 27/02/15 18:07, Jan Vesely wrote:
> > Signed-off-by: Jan Vesely
> > ---
> > xf86drmSL.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/xf86drmSL.c b/xf86drmSL.c
> > index acddb54..2160bb8 100644
> > --- a/xf86drmSL.c
> > +++ b/xf86drmSL.c
> > @@ -266,11 +266,14 @@ int drmSLLookupNeighbors(void *l, unsigned long key,
> > SkipListPtr list = (SkipListPtr)l;
> > SLEntryPtrupdate[SL_MAX_LEVEL + 1];
> > int retcode = 0;
> > +SLEntryPtrentry;
> > +
> > +entry = SLLocate(list, key, update);
> >
> > *prev_key = *next_key = key;
> > *prev_value = *next_value = NULL;
> > -
> > -if (update[0]) {
> > +
> > +if (entry && update[0]) {
> From a very brief look at git log, the entry check should not be needed.
> Must admit that I've not looked at all in the implementation of either
> SLLocate or drmSLLookupNeighbors.
SLLocate might return early and leave the array uninitialized. All other
calls to it check the return value. I guess the warning that the
previous commit tried to fix was "set-but-unused" variable.
>
> That said it seems that none of the three files
> (xf86drm{SL,Hash,Random}) has been build as a program for a while. Maybe
> we could split it out as a standalone test and let it churn at make
> check time ?
Sounds like a good idea. I'll try to take a look when time permits, but
I'd leave that as a separate patch.
thanks,
jan
>
> Cheers,
> Emil
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150320/79550235/attachment-0001.sig>
[PATCH libdrm 1/1] tests/exynos: Fix warnings
On Wed, 2015-03-18 at 20:23 +0100, Tobias Jakobi wrote:
> Hello Jan,
>
> Jan Vesely wrote:
> > Signed-off-by: Jan Vesely
> > ---
> > tests/exynos/exynos_fimg2d_test.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/exynos/exynos_fimg2d_test.c
> > b/tests/exynos/exynos_fimg2d_test.c
> > index e7d9b72..dfb34ac 100644
> > --- a/tests/exynos/exynos_fimg2d_test.c
> > +++ b/tests/exynos/exynos_fimg2d_test.c
> > @@ -183,7 +183,7 @@ static struct exynos_bo *exynos_create_buffer(struct
> > exynos_device *dev,
> >
> > /* Allocate buffer and fill it with checkerboard pattern, where the tiles *
> > * have a random color. The caller has to free the buffer.
> > */
> > -void *create_checkerboard_pattern(unsigned int num_tiles_x,
> > +static void *create_checkerboard_pattern(unsigned int num_tiles_x,
> > unsigned int num_tiles_y,
> > unsigned int tile_size)
> > {
> > unsigned int *buf;
> Good catch with the missing static!
>
>
> > @@ -573,6 +573,7 @@ static int g2d_checkerboard_test(struct exynos_device
> > *dev,
> > src_img.user_ptr[0].userptr = (unsigned long)checkerboard;
> > src_img.user_ptr[0].size = img_w * img_h * 4;
> > break;
> > + case G2D_IMGBUF_COLOR:
> > default:
> > ret = -EFAULT;
> > goto fail;
> >
> Hmm, I don't see the reason why this label should be added to the switch
> statement?
This is mostly to appease the compiler. -Wswitch-enum warns about
missing enumeration cases even if default is present. On the other hand
it also warns against branches that use values outside of the
enumeration (which might be useful).
Emil, any thoughts about possibly dropping this warning? I think more
people will scratch their heads if unused enumeration values need to be
added to default branch.
jan
>
> With best wishes,
> Tobias
>
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150318/55227cc4/attachment.sig>
[PATCH libdrm 9/8] Remove drmSetDebugMsgFunction and related infrastructure
Not used anywhere
Signed-off-by: Jan Vesely
---
xf86drm.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index a309d57..ab472ea 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -114,11 +114,6 @@ drmDebugPrint(const char *format, va_list ap)
return vfprintf(stderr, format, ap);
}
-typedef int DRM_PRINTFLIKE(1, 0) (*debug_msg_func_t)(const char *format,
-va_list ap);
-
-static debug_msg_func_t drm_debug_print = drmDebugPrint;
-
void
drmMsg(const char *format, ...)
{
@@ -130,18 +125,12 @@ drmMsg(const char *format, ...)
if (drm_server_info) {
drm_server_info->debug_print(format,ap);
} else {
- drm_debug_print(format, ap);
+ drmDebugPrint(format, ap);
}
va_end(ap);
}
}
-void
-drmSetDebugMsgFunction(debug_msg_func_t debug_msg_ptr)
-{
-drm_debug_print = debug_msg_ptr;
-}
-
static void *drmHashTable = NULL; /* Context switch callbacks */
void *drmGetHashTable(void)
--
2.1.0
[PATCH libdrm v2 8/8] Fix unused function warnings
v2: Remove the handler function instead of commenting out
split debugmsg function removal to a separate patch
Signed-off-by: Jan Vesely
---
tests/drmstat.c | 13 -
xf86drm.c | 2 ++
2 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/tests/drmstat.c b/tests/drmstat.c
index c6ff1ff..c800ebb 100644
--- a/tests/drmstat.c
+++ b/tests/drmstat.c
@@ -81,11 +81,6 @@ static void getversion(int fd)
printf( "No driver available\n" );
}
}
-
-void handler(int fd, void *oldctx, void *newctx)
-{
-printf("Got fd %d\n", fd);
-}
static void process_sigio(char *device)
{
@@ -97,7 +92,6 @@ static void process_sigio(char *device)
}
sigio_fd = fd;
-/* drmInstallSIGIOHandler(fd, handler); */
for (;;) sleep(60);
}
@@ -427,11 +421,4 @@ int main(int argc, char **argv)
return r;
}
-void DRM_PRINTFLIKE(4, 0)
-xf86VDrvMsgVerb(int scrnIndex, int type, int verb, const char *format,
-va_list args)
-{
- vfprintf(stderr, format, args);
-}
-
int xf86ConfigDRI[10];
diff --git a/xf86drm.c b/xf86drm.c
index 5032f35..a309d57 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -277,6 +277,7 @@ static int drmMatchBusID(const char *id1, const char *id2,
int pci_domain_ok)
* If any other failure happened then it will output error mesage using
* drmMsg() call.
*/
+#if !defined(UDEV)
static int chown_check_return(const char *path, uid_t owner, gid_t group)
{
int rv;
@@ -292,6 +293,7 @@ static int chown_check_return(const char *path, uid_t
owner, gid_t group)
path, errno, strerror(errno));
return -1;
}
+#endif
/**
* Open the DRM device, creating it if necessary.
--
2.1.0
[PATCH libdrm 1/1] tests/exynos: Fix warnings
Signed-off-by: Jan Vesely
---
tests/exynos/exynos_fimg2d_test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/exynos/exynos_fimg2d_test.c
b/tests/exynos/exynos_fimg2d_test.c
index e7d9b72..dfb34ac 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -183,7 +183,7 @@ static struct exynos_bo *exynos_create_buffer(struct
exynos_device *dev,
/* Allocate buffer and fill it with checkerboard pattern, where the tiles *
* have a random color. The caller has to free the buffer.*/
-void *create_checkerboard_pattern(unsigned int num_tiles_x,
+static void *create_checkerboard_pattern(unsigned int num_tiles_x,
unsigned int num_tiles_y,
unsigned int tile_size)
{
unsigned int *buf;
@@ -573,6 +573,7 @@ static int g2d_checkerboard_test(struct exynos_device *dev,
src_img.user_ptr[0].userptr = (unsigned long)checkerboard;
src_img.user_ptr[0].size = img_w * img_h * 4;
break;
+ case G2D_IMGBUF_COLOR:
default:
ret = -EFAULT;
goto fail;
--
2.1.0
[PATCH libdrm 4/8] xf86drmSL: Fix neighbour printing
Hi Kristian,
this patch partially reverts e4a519635:
Tidy up compile warnings by cleaning up types.
it looks to me that the xf86drmSL.c bits slipped in by accident.
thanks,
Jan
On Fri, 2015-02-27 at 13:07 -0500, Jan Vesely wrote:
> Signed-off-by: Jan Vesely
> ---
> xf86drmSL.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/xf86drmSL.c b/xf86drmSL.c
> index acddb54..2160bb8 100644
> --- a/xf86drmSL.c
> +++ b/xf86drmSL.c
> @@ -266,11 +266,14 @@ int drmSLLookupNeighbors(void *l, unsigned long key,
> SkipListPtr list = (SkipListPtr)l;
> SLEntryPtrupdate[SL_MAX_LEVEL + 1];
> int retcode = 0;
> +SLEntryPtrentry;
> +
> +entry = SLLocate(list, key, update);
>
> *prev_key = *next_key = key;
> *prev_value = *next_value = NULL;
> -
> -if (update[0]) {
> +
> +if (entry && update[0]) {
> *prev_key = update[0]->key;
> *prev_value = update[0]->value;
> ++retcode;
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150315/e69c32b0/attachment.sig>
[PATCH libdrm v2 1/8] Add static qualifier to local functions
v2: Don't bother marking dead functions static
(handler, xf86VDrvMsgVerb, drmSetDebugMsgFunction)
Signed-off-by: Jan Vesely
---
tests/drmstat.c | 2 +-
tests/kmstest/main.c| 2 +-
tests/modeprint/modeprint.c | 18 +-
tests/proptest/proptest.c | 2 +-
tests/radeon/radeon_ttm.c | 4 ++--
5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/tests/drmstat.c b/tests/drmstat.c
index 0ed5caf..c800ebb 100644
--- a/tests/drmstat.c
+++ b/tests/drmstat.c
@@ -82,7 +82,7 @@ static void getversion(int fd)
}
}
-void process_sigio(char *device)
+static void process_sigio(char *device)
{
int fd;
diff --git a/tests/kmstest/main.c b/tests/kmstest/main.c
index 59e6113..1d4c63e 100644
--- a/tests/kmstest/main.c
+++ b/tests/kmstest/main.c
@@ -37,7 +37,7 @@
return ret; \
}
-int test_bo(struct kms_driver *kms)
+static int test_bo(struct kms_driver *kms)
{
struct kms_bo *bo;
int ret;
diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
index 2d998d5..508c9f2 100644
--- a/tests/modeprint/modeprint.c
+++ b/tests/modeprint/modeprint.c
@@ -53,7 +53,7 @@ int crtcs;
int fbs;
char *module_name;
-const char* getConnectionText(drmModeConnection conn)
+static const char* getConnectionText(drmModeConnection conn)
{
switch (conn) {
case DRM_MODE_CONNECTED:
@@ -67,7 +67,7 @@ const char* getConnectionText(drmModeConnection conn)
}
-int printMode(struct drm_mode_modeinfo *mode)
+static int printMode(struct drm_mode_modeinfo *mode)
{
if (full_modes) {
printf("Mode: %s\n", mode->name);
@@ -91,7 +91,7 @@ int printMode(struct drm_mode_modeinfo *mode)
return 0;
}
-int printProperty(int fd, drmModeResPtr res, drmModePropertyPtr props,
uint64_t value)
+static int printProperty(int fd, drmModeResPtr res, drmModePropertyPtr props,
uint64_t value)
{
const char *name = NULL;
int j;
@@ -162,7 +162,7 @@ static const char * const output_names[] = { "None",
"DSI",
};
-int printConnector(int fd, drmModeResPtr res, drmModeConnectorPtr connector,
uint32_t id)
+static int printConnector(int fd, drmModeResPtr res, drmModeConnectorPtr
connector, uint32_t id)
{
int i = 0;
struct drm_mode_modeinfo *mode = NULL;
@@ -215,7 +215,7 @@ int printConnector(int fd, drmModeResPtr res,
drmModeConnectorPtr connector, uin
return 0;
}
-int printEncoder(int fd, drmModeResPtr res, drmModeEncoderPtr encoder,
uint32_t id)
+static int printEncoder(int fd, drmModeResPtr res, drmModeEncoderPtr encoder,
uint32_t id)
{
printf("Encoder\n");
printf("\tid :%i\n", id);
@@ -226,7 +226,7 @@ int printEncoder(int fd, drmModeResPtr res,
drmModeEncoderPtr encoder, uint32_t
return 0;
}
-int printCrtc(int fd, drmModeResPtr res, drmModeCrtcPtr crtc, uint32_t id)
+static int printCrtc(int fd, drmModeResPtr res, drmModeCrtcPtr crtc, uint32_t
id)
{
printf("Crtc\n");
printf("\tid : %i\n", id);
@@ -240,7 +240,7 @@ int printCrtc(int fd, drmModeResPtr res, drmModeCrtcPtr
crtc, uint32_t id)
return 0;
}
-int printFrameBuffer(int fd, drmModeResPtr res, drmModeFBPtr fb)
+static int printFrameBuffer(int fd, drmModeResPtr res, drmModeFBPtr fb)
{
printf("Framebuffer\n");
printf("\thandle: %i\n", fb->handle);
@@ -254,7 +254,7 @@ int printFrameBuffer(int fd, drmModeResPtr res,
drmModeFBPtr fb)
return 0;
}
-int printRes(int fd, drmModeResPtr res)
+static int printRes(int fd, drmModeResPtr res)
{
int i;
drmModeFBPtr fb;
@@ -330,7 +330,7 @@ int printRes(int fd, drmModeResPtr res)
return 0;
}
-void args(int argc, char **argv)
+static void args(int argc, char **argv)
{
int i;
diff --git a/tests/proptest/proptest.c b/tests/proptest/proptest.c
index 4b7234d..9b2eb3f 100644
--- a/tests/proptest/proptest.c
+++ b/tests/proptest/proptest.c
@@ -44,7 +44,7 @@ static inline int64_t U642I64(uint64_t val)
int fd;
drmModeResPtr res = NULL;
-const char *connector_type_str(uint32_t type)
+static const char *connector_type_str(uint32_t type)
{
switch (type) {
case DRM_MODE_CONNECTOR_Unknown:
diff --git a/tests/radeon/radeon_ttm.c b/tests/radeon/radeon_ttm.c
index ac3297a..8346e85 100644
--- a/tests/radeon/radeon_ttm.c
+++ b/tests/radeon/radeon_ttm.c
@@ -32,7 +32,7 @@
/* allocate as many single page bo to try to starve the kernel
* memory zone (below highmem)
*/
-void ttm_starve_kernel_private_memory(int fd)
+static void ttm_starve_kernel_private_memory(int fd)
{
struct list_head list;
struct rbo *bo, *tmp;
@@ -55,7 +55,7 @@ void ttm_starve_kernel_private_memory(int fd)
}
}
-int radeon_open_fd(void)
+static int radeon_open_fd(void)
{
return drmOpen("radeon", NULL);
}
--
2.1.0
[PATCH libdrm 0/8] Warnings fixes
On Fri, 2015-03-13 at 23:20 +, Emil Velikov wrote: > Hi Jan, > > On 27 February 2015 at 22:27, Emil Velikov > wrote: > > On 27/02/15 18:07, Jan Vesely wrote: > >> His series fixes most of the warnings my compiler spits out running > >> make distcheck. > >> Most of the changes do not change behaviour and those the do are in > >> separate > >> patches. > > Nicely done Jan. If you don't mind I would prefer if Theirry's series > > lands first as it will clash with some of the patches in tests. > > > Seems that Thierry's quite busy so I've went ahead and sent over a few > comments. I'm not planning to look at the patch #4, but the rest look > great. With the comments addressed feel free to add > > Reviewed-by: Emil Velikov Thank you for finding time to review these. I'll start pushing the reviewed ones tomorrow, and send v2 of the 3 you commented on. jan > > Thierry, just realised that I've made a mistake writing your name last > time. Sincerest of apologies, it was not intentional. > > Cheers, > Emil -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150313/ce223df8/attachment.sig>
[PATCH libdrm 1/8] Add static qualifier to local functions
On Fri, 2015-03-13 at 23:07 +, Emil Velikov wrote:
> On 27 February 2015 at 18:07, Jan Vesely wrote:
> > Signed-off-by: Jan Vesely
> > ---
> > tests/drmstat.c | 8
> > tests/kmstest/main.c| 2 +-
> > tests/modeprint/modeprint.c | 18 +-
> > tests/proptest/proptest.c | 2 +-
> > tests/radeon/radeon_ttm.c | 4 ++--
> > xf86drm.c | 2 +-
> > xf86drmMode.c | 2 +-
> > 7 files changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/tests/drmstat.c b/tests/drmstat.c
> > index 5935d07..36cc70d 100644
> > --- a/tests/drmstat.c
> > +++ b/tests/drmstat.c
> > @@ -81,13 +81,13 @@ static void getversion(int fd)
> > printf( "No driver available\n" );
> > }
> > }
> > -
> > -void handler(int fd, void *oldctx, void *newctx)
> > +
> > +static void handler(int fd, void *oldctx, void *newctx)
> > {
> > printf("Got fd %d\n", fd);
> > }
> >
> It's only "user" was commented out as a transition to libdrm2 afaict.
> Should be safe to nuke alongside the commented out caller.
This one got commented out in 8/8 if you prefer I can nuke it, and apply
8/8 before this one.
>
> > -void process_sigio(char *device)
> > +static void process_sigio(char *device)
> > {
> > int fd;
> >
> > @@ -427,7 +427,7 @@ int main(int argc, char **argv)
> > return r;
> > }
> >
> > -void DRM_PRINTFLIKE(4, 0)
> > +static void DRM_PRINTFLIKE(4, 0)
> > xf86VDrvMsgVerb(int scrnIndex, int type, int verb, const char *format,
> > va_list args)
> Think don't need to bother making this static and just nuke it. It
> seems like it was added by mistake (commit c3092ead642) and never
> used.
same here. it is removed in 8/8 I can reorder the patches tog et rid of
this artifact
>
> ...
> > diff --git a/xf86drm.c b/xf86drm.c
> > index e117bc6..016247f 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -131,7 +131,7 @@ drmMsg(const char *format, ...)
> > }
> > }
> >
> > -void
> > +static void
> > drmSetDebugMsgFunction(debug_msg_func_t debug_msg_ptr)
> > {
> > drm_debug_print = debug_msg_ptr;
> > diff --git a/xf86drmMode.c b/xf86drmMode.c
> > index 9ea8fe7..1c06a19 100644
> > --- a/xf86drmMode.c
> > +++ b/xf86drmMode.c
> > @@ -76,7 +76,7 @@ static inline int DRM_IOCTL(int fd, unsigned long cmd,
> > void *arg)
> > * Util functions
> > */
> >
> > -void* drmAllocCpy(void *array, int count, int entry_size)
> > +static void* drmAllocCpy(void *array, int count, int entry_size)
> Strictly speaking these could still be used, despite never being part
> of the API. Although my vote (fwiw) would be to that we're safe.
This one is heavily used in the same file.
>
> Cheers,
> Emil
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150313/bc04f98c/attachment-0001.sig>
[PATCH libdrm 7/8] Fix unused, and unused-but-set variables warnings
On Fri, 2015-03-13 at 22:56 +, Emil Velikov wrote:
> On 27 February 2015 at 18:07, Jan Vesely wrote:
> > Signed-off-by: Jan Vesely
> > ---
> > tests/exynos/exynos_fimg2d_test.c | 4 +---
> > xf86drm.c | 9 ++---
> > 2 files changed, 7 insertions(+), 6 deletions(-)
> >
> ...
> > @@ -333,7 +336,6 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
> > drm_server_info->get_perms(_group, _mode);
> > devmode = serv_mode ? serv_mode : DRM_DEV_MODE;
> > devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH);
> > - group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
> > }
> >
> > #if !defined(UDEV)
> > @@ -354,6 +356,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
> > }
> >
> > if (drm_server_info) {
> > + group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
> I think you can leave this back where it was. It doesn't seem to bring
> anything beneficial.
"group" variable is now only available #if !defined(UDEV), so I can;'t
really move it back without adding a pair of ifdefs,
moving it here looked nicer than adding another set of #if/#endif.
I wanted to move the declaration here as well, but I'm not sure how it'd
play with old/broken compilers that need to be supported.
jan
>
> -Emil
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150313/8cf0c582/attachment.sig>
[PATCH 1/1] drm/radeon: Fix cleanup error path.
please ignore this patch, see bugzilla for details.
sorry for the noise.
On Wed, 2015-03-04 at 16:10 -0500, Jan Vesely wrote:
> cleanup: target is called before fence_get
> Fixes hangs on GPU reset on Turks GPU.
>
> regression introduced by:
> commit dd7cfd641228abb2669d8d047d5ec377b1835900
> Author: Maarten Lankhorst
> Date: Tue Jan 21 13:07:31 2014 +0100
>
> drm/ttm: kill fence_lock
>
> No users are left, kill it off! :D
> Conversion to the reservation api is next on the list, after
> that the functionality can be restored with rcu.
>
> Signed-off-by: Maarten Lankhorst
>
> CC: stable at vger.kernel.org
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94081
> Signed-off-by: Jan Vesely
> ---
> drivers/gpu/drm/radeon/radeon_display.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c
> b/drivers/gpu/drm/radeon/radeon_display.c
> index 00ead8c..89a8c48 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -573,6 +573,7 @@ vblank_cleanup:
> drm_vblank_put(crtc->dev, radeon_crtc->crtc_id);
>
> pflip_cleanup:
> + fence_put(work->fence);
> if (unlikely(radeon_bo_reserve(new_rbo, false) != 0)) {
> DRM_ERROR("failed to reserve new rbo in error path\n");
> goto cleanup;
> @@ -584,7 +585,6 @@ pflip_cleanup:
>
> cleanup:
> drm_gem_object_unreference_unlocked(>old_rbo->gem_base);
> - fence_put(work->fence);
> kfree(work);
> return r;
> }
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150304/59303123/attachment.sig>
[PATCH 1/1] drm/radeon: Fix cleanup error path.
cleanup: target is called before fence_get
Fixes hangs on GPU reset on Turks GPU.
regression introduced by:
commit dd7cfd641228abb2669d8d047d5ec377b1835900
Author: Maarten Lankhorst
Date: Tue Jan 21 13:07:31 2014 +0100
drm/ttm: kill fence_lock
No users are left, kill it off! :D
Conversion to the reservation api is next on the list, after
that the functionality can be restored with rcu.
Signed-off-by: Maarten Lankhorst
CC: stable at vger.kernel.org
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94081
Signed-off-by: Jan Vesely
---
drivers/gpu/drm/radeon/radeon_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index 00ead8c..89a8c48 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -573,6 +573,7 @@ vblank_cleanup:
drm_vblank_put(crtc->dev, radeon_crtc->crtc_id);
pflip_cleanup:
+ fence_put(work->fence);
if (unlikely(radeon_bo_reserve(new_rbo, false) != 0)) {
DRM_ERROR("failed to reserve new rbo in error path\n");
goto cleanup;
@@ -584,7 +585,6 @@ pflip_cleanup:
cleanup:
drm_gem_object_unreference_unlocked(>old_rbo->gem_base);
- fence_put(work->fence);
kfree(work);
return r;
}
--
2.1.0
[libdrm][PATCH 3/2] Fix always true comparison.
On Wed, 2015-02-25 at 18:41 +, Emil Velikov wrote: > On 25/02/15 17:11, Jan Vesely wrote: > > gentle ping > > > Afaics it's very had to get in this code nowadays - drm_server_info is > set only via the legacy (?) function drmSetServerInfo. With the latter > only(?) used by the xserver when working with dri1 modules. So testing > this is likely to be very painful :-( > > This code hasn't changed since before 2007, so I doubt there are many > people that know the details about it, so we might as well leave it for > now ? fair enough. the warning fixes push it to non-libudev side, so it's not going to bug me on every build. jan > > -Emil > > > > > On Mon, 2015-02-09 at 19:10 -0500, Jan Vesely wrote: > >> The only user I found is xserver, it can return -1 under certain > >> conditions. > >> So check for -1 explicitly. > >> > >> Signed-off-by: Jan Vesely > >> --- > >> > >> I could not find whether it's actually legal to return encoded negative > >> values > >> in get_perm. This is a quick fix to detect the one case that I found. > >> > >> xf86drm.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/xf86drm.c b/xf86drm.c > >> index fb673b5..8e54ac9 100644 > >> --- a/xf86drm.c > >> +++ b/xf86drm.c > >> @@ -335,7 +335,7 @@ static int drmOpenDevice(dev_t dev, int minor, int > >> type) > >>drm_server_info->get_perms(_group, _mode); > >>devmode = serv_mode ? serv_mode : DRM_DEV_MODE; > >>devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH); > >> - group = (serv_group >= 0) ? serv_group : DRM_DEV_GID; > >> + group = (serv_group != ~0U) ? serv_group : DRM_DEV_GID; > >> } > >> > >> #if !defined(UDEV) > > > -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150302/1bf439c1/attachment.sig>
[PATCH libdrm 8/8] Fix unused function warnings
Signed-off-by: Jan Vesely
---
tests/drmstat.c | 9 ++---
xf86drm.c | 8 ++--
2 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/tests/drmstat.c b/tests/drmstat.c
index 36cc70d..6502d78 100644
--- a/tests/drmstat.c
+++ b/tests/drmstat.c
@@ -82,10 +82,12 @@ static void getversion(int fd)
}
}
+/*
static void handler(int fd, void *oldctx, void *newctx)
{
printf("Got fd %d\n", fd);
}
+*/
static void process_sigio(char *device)
{
@@ -427,11 +429,4 @@ int main(int argc, char **argv)
return r;
}
-static void DRM_PRINTFLIKE(4, 0)
-xf86VDrvMsgVerb(int scrnIndex, int type, int verb, const char *format,
-va_list args)
-{
- vfprintf(stderr, format, args);
-}
-
int xf86ConfigDRI[10];
diff --git a/xf86drm.c b/xf86drm.c
index 3c8fc0b..a660842 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -131,12 +131,6 @@ drmMsg(const char *format, ...)
}
}
-static void
-drmSetDebugMsgFunction(debug_msg_func_t debug_msg_ptr)
-{
-drm_debug_print = debug_msg_ptr;
-}
-
static void *drmHashTable = NULL; /* Context switch callbacks */
void *drmGetHashTable(void)
@@ -272,6 +266,7 @@ static int drmMatchBusID(const char *id1, const char *id2,
int pci_domain_ok)
* If any other failure happened then it will output error mesage using
* drmMsg() call.
*/
+#if !defined(UDEV)
static int chown_check_return(const char *path, uid_t owner, gid_t group)
{
int rv;
@@ -287,6 +282,7 @@ static int chown_check_return(const char *path, uid_t
owner, gid_t group)
path, errno, strerror(errno));
return -1;
}
+#endif
/**
* Open the DRM device, creating it if necessary.
--
2.1.0
[PATCH libdrm 7/8] Fix unused, and unused-but-set variables warnings
Signed-off-by: Jan Vesely
---
tests/exynos/exynos_fimg2d_test.c | 4 +---
xf86drm.c | 9 ++---
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/exynos/exynos_fimg2d_test.c
b/tests/exynos/exynos_fimg2d_test.c
index ba8d12c..38ea974 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -322,7 +322,7 @@ static int g2d_copy_with_scale_test(struct exynos_device
*dev,
{
struct g2d_context *ctx;
struct g2d_image src_img, dst_img;
- unsigned int src_x, src_y, dst_x, dst_y, img_w, img_h;
+ unsigned int src_x, src_y, img_w, img_h;
unsigned long userptr, size;
int ret;
@@ -336,8 +336,6 @@ static int g2d_copy_with_scale_test(struct exynos_device
*dev,
src_x = 0;
src_y = 0;
- dst_x = 0;
- dst_y = 0;
img_w = screen_width;
img_h = screen_height;
diff --git a/xf86drm.c b/xf86drm.c
index 016247f..3c8fc0b 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -308,10 +308,13 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
charbuf[64];
int fd;
mode_t devmode = DRM_DEV_MODE, serv_mode;
+gid_t serv_group;
+#if !defined(UDEV)
int isroot = !geteuid();
uid_t user= DRM_DEV_UID;
-gid_t group = DRM_DEV_GID, serv_group;
-
+gid_t group = DRM_DEV_GID;
+#endif
+
switch (type) {
case DRM_NODE_PRIMARY:
dev_name = DRM_DEV_NAME;
@@ -333,7 +336,6 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
drm_server_info->get_perms(_group, _mode);
devmode = serv_mode ? serv_mode : DRM_DEV_MODE;
devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH);
- group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
}
#if !defined(UDEV)
@@ -354,6 +356,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
}
if (drm_server_info) {
+ group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
chown_check_return(buf, user, group);
chmod(buf, devmode);
}
--
2.1.0
[PATCH libdrm 6/8] dristat: Handle DRM_CONSISTENT
Signed-off-by: Jan Vesely --- tests/dristat.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/dristat.c b/tests/dristat.c index 992ceb4..cca4b03 100644 --- a/tests/dristat.c +++ b/tests/dristat.c @@ -100,6 +100,7 @@ static void getvm(int fd) case DRM_SHM:typename = "SHM"; break; case DRM_AGP:typename = "AGP"; break; case DRM_SCATTER_GATHER: typename = "SG"; break; + case DRM_CONSISTENT: typename = "CON"; break; default: typename = "???"; break; } -- 2.1.0
[PATCH libdrm 5/8] Fix aliasing and switch-enum warnings
Signed-off-by: Jan Vesely
---
tests/exynos/exynos_fimg2d_test.c | 3 +++
tests/modeprint/modeprint.c | 1 +
xf86drmMode.c | 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/tests/exynos/exynos_fimg2d_test.c
b/tests/exynos/exynos_fimg2d_test.c
index dc7c5cb..ba8d12c 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -273,6 +273,7 @@ static int g2d_copy_test(struct exynos_device *dev, struct
exynos_bo *src,
src_img.user_ptr[0].userptr = userptr;
src_img.user_ptr[0].size = size;
break;
+ case G2D_IMGBUF_COLOR:
default:
type = G2D_IMGBUF_GEM;
break;
@@ -356,6 +357,7 @@ static int g2d_copy_with_scale_test(struct exynos_device
*dev,
src_img.user_ptr[0].userptr = userptr;
src_img.user_ptr[0].size = size;
break;
+ case G2D_IMGBUF_COLOR:
default:
type = G2D_IMGBUF_GEM;
break;
@@ -444,6 +446,7 @@ static int g2d_blend_test(struct exynos_device *dev,
src_img.user_ptr[0].userptr = userptr;
src_img.user_ptr[0].size = size;
break;
+ case G2D_IMGBUF_COLOR:
default:
type = G2D_IMGBUF_GEM;
break;
diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
index de59cbd..508c9f2 100644
--- a/tests/modeprint/modeprint.c
+++ b/tests/modeprint/modeprint.c
@@ -60,6 +60,7 @@ static const char* getConnectionText(drmModeConnection conn)
return "connected";
case DRM_MODE_DISCONNECTED:
return "disconnected";
+ case DRM_MODE_UNKNOWNCONNECTION:
default:
return "unknown";
}
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 61d5e01..acb6c52 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -858,7 +858,7 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx)
i = 0;
while (i < len) {
- e = (struct drm_event *) [i];
+ e = (void *) [i];
switch (e->type) {
case DRM_EVENT_VBLANK:
if (evctx->version < 1 ||
--
2.1.0
[PATCH libdrm 4/8] xf86drmSL: Fix neighbour printing
Signed-off-by: Jan Vesely
---
xf86drmSL.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/xf86drmSL.c b/xf86drmSL.c
index acddb54..2160bb8 100644
--- a/xf86drmSL.c
+++ b/xf86drmSL.c
@@ -266,11 +266,14 @@ int drmSLLookupNeighbors(void *l, unsigned long key,
SkipListPtr list = (SkipListPtr)l;
SLEntryPtrupdate[SL_MAX_LEVEL + 1];
int retcode = 0;
+SLEntryPtrentry;
+
+entry = SLLocate(list, key, update);
*prev_key = *next_key = key;
*prev_value = *next_value = NULL;
-
-if (update[0]) {
+
+if (entry && update[0]) {
*prev_key = update[0]->key;
*prev_value = update[0]->value;
++retcode;
--
2.1.0
[PATCH libdrm 3/8] Fix type-limits, pointer-arith and sign-compare warnings
Signed-off-by: Jan Vesely
---
tests/dristat.c | 4 ++--
tests/getstats.c | 2 --
xf86drmMode.c| 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/tests/dristat.c b/tests/dristat.c
index 449aa24..992ceb4 100644
--- a/tests/dristat.c
+++ b/tests/dristat.c
@@ -189,9 +189,9 @@ static void printhuman(unsigned long value, const char
*name, int mult)
static void getstats(int fd, int i)
{
drmStatsT prev, curr;
-int j;
+unsigned j;
doublerate;
-
+
printf(" System statistics:\n");
if (drmGetStats(fd, )) return;
diff --git a/tests/getstats.c b/tests/getstats.c
index 8d40d0b..8a7d299 100644
--- a/tests/getstats.c
+++ b/tests/getstats.c
@@ -45,8 +45,6 @@ int main(int argc, char **argv)
ret = ioctl(fd, DRM_IOCTL_GET_STATS, );
assert(ret == 0);
- assert(stats.count >= 0);
-
close(fd);
return 0;
}
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 1c06a19..61d5e01 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -76,7 +76,7 @@ static inline int DRM_IOCTL(int fd, unsigned long cmd, void
*arg)
* Util functions
*/
-static void* drmAllocCpy(void *array, int count, int entry_size)
+static void* drmAllocCpy(char *array, int count, int entry_size)
{
char *r;
int i;
--
2.1.0
[PATCH libdrm 2/8] tests: String literals are const char *
Signed-off-by: Jan Vesely
---
tests/kmstest/main.c | 2 +-
tests/proptest/proptest.c | 2 +-
tests/vbltest/vbltest.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/kmstest/main.c b/tests/kmstest/main.c
index d8c6c1f..1d4c63e 100644
--- a/tests/kmstest/main.c
+++ b/tests/kmstest/main.c
@@ -56,7 +56,7 @@ static int test_bo(struct kms_driver *kms)
return 0;
}
-char *drivers[] = {
+static const char *drivers[] = {
"i915",
"radeon",
"nouveau",
diff --git a/tests/proptest/proptest.c b/tests/proptest/proptest.c
index 3496065..9b2eb3f 100644
--- a/tests/proptest/proptest.c
+++ b/tests/proptest/proptest.c
@@ -303,7 +303,7 @@ static void printUsage(void)
int main(int argc, char *argv[])
{
- char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm",
"msm" };
+ const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx",
"omapdrm", "msm" };
unsigned int i, ret = 0;
for (i = 0; i < ARRAY_SIZE(modules); i++){
diff --git a/tests/vbltest/vbltest.c b/tests/vbltest/vbltest.c
index 916d494..02fefbf 100644
--- a/tests/vbltest/vbltest.c
+++ b/tests/vbltest/vbltest.c
@@ -106,7 +106,7 @@ int main(int argc, char **argv)
{
unsigned i;
int c, fd, ret;
- char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "exynos",
"omapdrm", "tilcdc", "msm", "tegra", "imx-drm" };
+ const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx",
"exynos", "omapdrm", "tilcdc", "msm", "tegra", "imx-drm" };
drmVBlank vbl;
drmEventContext evctx;
struct vbl_info handler_info;
--
2.1.0
[PATCH libdrm 1/8] Add static qualifier to local functions
Signed-off-by: Jan Vesely
---
tests/drmstat.c | 8
tests/kmstest/main.c| 2 +-
tests/modeprint/modeprint.c | 18 +-
tests/proptest/proptest.c | 2 +-
tests/radeon/radeon_ttm.c | 4 ++--
xf86drm.c | 2 +-
xf86drmMode.c | 2 +-
7 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/tests/drmstat.c b/tests/drmstat.c
index 5935d07..36cc70d 100644
--- a/tests/drmstat.c
+++ b/tests/drmstat.c
@@ -81,13 +81,13 @@ static void getversion(int fd)
printf( "No driver available\n" );
}
}
-
-void handler(int fd, void *oldctx, void *newctx)
+
+static void handler(int fd, void *oldctx, void *newctx)
{
printf("Got fd %d\n", fd);
}
-void process_sigio(char *device)
+static void process_sigio(char *device)
{
int fd;
@@ -427,7 +427,7 @@ int main(int argc, char **argv)
return r;
}
-void DRM_PRINTFLIKE(4, 0)
+static void DRM_PRINTFLIKE(4, 0)
xf86VDrvMsgVerb(int scrnIndex, int type, int verb, const char *format,
va_list args)
{
diff --git a/tests/kmstest/main.c b/tests/kmstest/main.c
index 2c87b1c..d8c6c1f 100644
--- a/tests/kmstest/main.c
+++ b/tests/kmstest/main.c
@@ -37,7 +37,7 @@
return ret; \
}
-int test_bo(struct kms_driver *kms)
+static int test_bo(struct kms_driver *kms)
{
struct kms_bo *bo;
int ret;
diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
index 6f0d039..de59cbd 100644
--- a/tests/modeprint/modeprint.c
+++ b/tests/modeprint/modeprint.c
@@ -53,7 +53,7 @@ int crtcs;
int fbs;
char *module_name;
-const char* getConnectionText(drmModeConnection conn)
+static const char* getConnectionText(drmModeConnection conn)
{
switch (conn) {
case DRM_MODE_CONNECTED:
@@ -66,7 +66,7 @@ const char* getConnectionText(drmModeConnection conn)
}
-int printMode(struct drm_mode_modeinfo *mode)
+static int printMode(struct drm_mode_modeinfo *mode)
{
if (full_modes) {
printf("Mode: %s\n", mode->name);
@@ -90,7 +90,7 @@ int printMode(struct drm_mode_modeinfo *mode)
return 0;
}
-int printProperty(int fd, drmModeResPtr res, drmModePropertyPtr props,
uint64_t value)
+static int printProperty(int fd, drmModeResPtr res, drmModePropertyPtr props,
uint64_t value)
{
const char *name = NULL;
int j;
@@ -161,7 +161,7 @@ static const char * const output_names[] = { "None",
"DSI",
};
-int printConnector(int fd, drmModeResPtr res, drmModeConnectorPtr connector,
uint32_t id)
+static int printConnector(int fd, drmModeResPtr res, drmModeConnectorPtr
connector, uint32_t id)
{
int i = 0;
struct drm_mode_modeinfo *mode = NULL;
@@ -214,7 +214,7 @@ int printConnector(int fd, drmModeResPtr res,
drmModeConnectorPtr connector, uin
return 0;
}
-int printEncoder(int fd, drmModeResPtr res, drmModeEncoderPtr encoder,
uint32_t id)
+static int printEncoder(int fd, drmModeResPtr res, drmModeEncoderPtr encoder,
uint32_t id)
{
printf("Encoder\n");
printf("\tid :%i\n", id);
@@ -225,7 +225,7 @@ int printEncoder(int fd, drmModeResPtr res,
drmModeEncoderPtr encoder, uint32_t
return 0;
}
-int printCrtc(int fd, drmModeResPtr res, drmModeCrtcPtr crtc, uint32_t id)
+static int printCrtc(int fd, drmModeResPtr res, drmModeCrtcPtr crtc, uint32_t
id)
{
printf("Crtc\n");
printf("\tid : %i\n", id);
@@ -239,7 +239,7 @@ int printCrtc(int fd, drmModeResPtr res, drmModeCrtcPtr
crtc, uint32_t id)
return 0;
}
-int printFrameBuffer(int fd, drmModeResPtr res, drmModeFBPtr fb)
+static int printFrameBuffer(int fd, drmModeResPtr res, drmModeFBPtr fb)
{
printf("Framebuffer\n");
printf("\thandle: %i\n", fb->handle);
@@ -253,7 +253,7 @@ int printFrameBuffer(int fd, drmModeResPtr res,
drmModeFBPtr fb)
return 0;
}
-int printRes(int fd, drmModeResPtr res)
+static int printRes(int fd, drmModeResPtr res)
{
int i;
drmModeFBPtr fb;
@@ -329,7 +329,7 @@ int printRes(int fd, drmModeResPtr res)
return 0;
}
-void args(int argc, char **argv)
+static void args(int argc, char **argv)
{
int i;
diff --git a/tests/proptest/proptest.c b/tests/proptest/proptest.c
index 7618f63..3496065 100644
--- a/tests/proptest/proptest.c
+++ b/tests/proptest/proptest.c
@@ -44,7 +44,7 @@ static inline int64_t U642I64(uint64_t val)
int fd;
drmModeResPtr res = NULL;
-const char *connector_type_str(uint32_t type)
+static const char *connector_type_str(uint32_t type)
{
switch (type) {
case DRM_MODE_CONNECTOR_Unknown:
diff --git a/tests/radeon/radeon_ttm.c b/tests/radeon/radeon_ttm.c
index ac3297a..8346e85 100644
--- a/tests/radeon/radeon_ttm.c
+++ b/tests/radeon/radeon_tt
[PATCH libdrm 0/8] Warnings fixes
His series fixes most of the warnings my compiler spits out running make distcheck. Most of the changes do not change behaviour and those the do are in separate patches. After this series I'm left with: 1 -Wempty-body 1 -Wformat 1 -Wpointer-to-int-cast 1 -Wsign-compare Jan Vesely (8): Add static qualifier to local functions tests: String literals are const char * Fix type-limits, pointer-arith and sign-compare warnings xf86drmSL: Fix neighbour printing Fix aliasing and switch-enum warnings dristat: Handle DRM_CONSISTENT Fix unused, and unused-but-set variables warnings Fix unused function warnings tests/dristat.c | 5 +++-- tests/drmstat.c | 15 +-- tests/exynos/exynos_fimg2d_test.c | 7 --- tests/getstats.c | 2 -- tests/kmstest/main.c | 4 ++-- tests/modeprint/modeprint.c | 19 ++- tests/proptest/proptest.c | 4 ++-- tests/radeon/radeon_ttm.c | 4 ++-- tests/vbltest/vbltest.c | 2 +- xf86drm.c | 17 - xf86drmMode.c | 4 ++-- xf86drmSL.c | 7 +-- 12 files changed, 44 insertions(+), 46 deletions(-) -- 2.1.0
[libdrm][PATCH 3/2] Fix always true comparison.
gentle ping On Mon, 2015-02-09 at 19:10 -0500, Jan Vesely wrote: > The only user I found is xserver, it can return -1 under certain conditions. > So check for -1 explicitly. > > Signed-off-by: Jan Vesely > --- > > I could not find whether it's actually legal to return encoded negative values > in get_perm. This is a quick fix to detect the one case that I found. > > xf86drm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xf86drm.c b/xf86drm.c > index fb673b5..8e54ac9 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -335,7 +335,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type) > drm_server_info->get_perms(_group, _mode); > devmode = serv_mode ? serv_mode : DRM_DEV_MODE; > devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH); > - group = (serv_group >= 0) ? serv_group : DRM_DEV_GID; > + group = (serv_group != ~0U) ? serv_group : DRM_DEV_GID; > } > > #if !defined(UDEV) -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150225/90cbc774/attachment-0001.sig>
[PATCH libdrm 00/04] Silence compiler warnings
Hi, you can add Reviewed-by: Jan Vesely to 1,2, and 4. I think 3 needs someone from exynos to say whether the function should not be used somewhere (just to be on the safe side). jan On Mon, 2015-02-23 at 14:56 +, Emil Velikov wrote: > Short follow up to my earlier series "The rocky road to building with > -Wextra". This gets rid of ~20 warnings, with ~45 remainig. > > -Emil > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150225/7e3f8e25/attachment.sig>
[PATCH 00/04 libdrm] The rocky road to building with -Wextra
On Wed, 2015-02-25 at 15:25 +, Emil Velikov wrote: > On 23 February 2015 at 13:57, Emil Velikov > wrote: > > Hi all, > > > > A few small patches, that handle the initial step of building the whole > > of libdrm with WARN_CFLAGS (-Wall -Wextra and friends). > > > > The first patch makes sure we build everything at make distcheck time, > > followed by a couple of warnings-turned-errors, and finally adding the > > cflags everywhere in the project. > > > > Individual fixes for the 66 warnings will follow up in due time :-) > > > Hi Jan, > > Slightly forgot that you're not subscribed to the list. > Considering your earlier work on this can you take a look at the series > please. > > This lays the groundwork, and a later one [1] nukes some of the warnings. Hi, I'm now subscribed to the list, but I somehow missed the series. The changes are pretty straightforward. You can add Reviewed-by: Jan Vesely I'll try to find some time to take a look at the other series too. jan > > Thank > Emil > > [1] http://lists.freedesktop.org/archives/dri-devel/2015-February/077976.html -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150225/3c2a99f2/attachment.sig>
VM on GPUs
Hi, thank you. I found a presentation on AMD APUs that mentions throughtput differences between different types of memory. This information helps me a lot. thanks again, Jan On Sat, 2015-02-21 at 10:24 -0500, Alex Deucher wrote: > On Fri, Feb 20, 2015 at 7:21 PM, Jan Vesely wrote: > > Hi, > > > > thank you for exhaustive answer. I have few more > > questions/clarifications: > > is the DMA address used to access system pages further translated using > > IOMMU (if present), or are GPUs treated specially? > > > > Yes, the address may be further translated via an IOMMU. That's why I > said dma address rather than bus address or physical address. > > > I have only seen references to TLB flush, so I guess invalidating > > individual entries is not supported? > > You can flush individual vmids, but not individual entries. > > > does it mean that if a page needs to be moved/migrated a complete VMID > > tlb flush is required? > > Yes. > > > > > I was a bit surprised to find about PCIe cache snoop, since the work I > > have seen before assumes DMA is not cache coherent. I guess there's a > > latency penalty for for using it, do you have any idea how much worse it > > gets (relatively to non-snoop access)? > > It is slower than non-snooped. I don't remember the numbers off hand. > I think some of the compute documents or HSA developer summit > presentations on the AMD developer site go into the details. > > Alex > > > > > thanks again, > > jan > > > > On Fri, 2015-02-20 at 17:19 -0500, Alex Deucher wrote: > >> On Fri, Feb 20, 2015 at 12:35 PM, Jan Vesely > >> wrote: > >> > Hello radeon devs, > >> > > >> > I have been trying to find out more about VM implementation on SI+ hw, > >> > but unfortunately I could not find much in the public documents[0]. > >> > > >> > SI ISA manual suggests that there is a limited form of privileged mode > >> > on these chips, so I wondered if it could be used for VM management too > >> > (the docs only deal with numerical exceptions). Or does it always have > >> > to be handled by host (driver)? > >> > >> These are related to trap/exception privilege for debugging for > >> example. I'm not that familiar with how that stuff works. It's > >> unrelated to GPUVM. > >> > >> > > >> > One of the older patches [1] mentions different page sizes, is there any > >> > public documentation on things like page table format, and GPU MMU > >> > hierarchy? I could only get limited picture going through the code and > >> > comments. > >> > >> There is not any public documentation on the VM hardware other than > >> what is available in the driver. I can try and give you an overview > >> of how it works. There are 16 VM contexts (8 on cayman/TN/RL) on the > >> GPU that can be active at any given time. GPUVM supports a 40 bit > >> address space. Each context has an id, we call them vmids. vmid 0 is > >> a bit special. It's called the system context and behaves a bit > >> differently to the other ones. It's designed to be for the kernel > >> driver's view of GPU accessible memory. I can go into further detail > >> if you want, but I don't think it's critical for this discussion. > >> Just think of it as the context used by the kernel driver. So that > >> leaves 16 contexts (7 on cayman and TN/RL) available for use by user > >> clients. vmid 0 has one set of configuration registers and vmids 1-15 > >> share the same configuration (other than the page tables). E.g., > >> contexts 1-15 all have to use single or 2 level page tables for > >> example. You select which VM context is used for a particular command > >> buffer by a field in the command buffer packet. Some engines (e.g., > >> UVD or the display hardware) do not support VM so they always use vmid > >> 0. Right now only the graphics, compute, and DMA engines support VM. > >> > >> With single level page tables, you just have a big array of page table > >> entries (PTEs) that represent the entire virtual address space. With > >> multi-level page tables, the address space is represented by an array > >> of page directory entries (PDEs) that point to page table blocks > >> (PTBs) which are arrays of PTEs. > >> > >> PTEs and PDEs are 64 bits per entry. > >> > >> PDE: > >> 39:12 - PTB address > >> 0 - PDE valid (the entry is valid) > >&g
VM on GPUs
Hi, thank you for exhaustive answer. I have few more questions/clarifications: is the DMA address used to access system pages further translated using IOMMU (if present), or are GPUs treated specially? I have only seen references to TLB flush, so I guess invalidating individual entries is not supported? does it mean that if a page needs to be moved/migrated a complete VMID tlb flush is required? I was a bit surprised to find about PCIe cache snoop, since the work I have seen before assumes DMA is not cache coherent. I guess there's a latency penalty for for using it, do you have any idea how much worse it gets (relatively to non-snoop access)? thanks again, jan On Fri, 2015-02-20 at 17:19 -0500, Alex Deucher wrote: > On Fri, Feb 20, 2015 at 12:35 PM, Jan Vesely > wrote: > > Hello radeon devs, > > > > I have been trying to find out more about VM implementation on SI+ hw, > > but unfortunately I could not find much in the public documents[0]. > > > > SI ISA manual suggests that there is a limited form of privileged mode > > on these chips, so I wondered if it could be used for VM management too > > (the docs only deal with numerical exceptions). Or does it always have > > to be handled by host (driver)? > > These are related to trap/exception privilege for debugging for > example. I'm not that familiar with how that stuff works. It's > unrelated to GPUVM. > > > > > One of the older patches [1] mentions different page sizes, is there any > > public documentation on things like page table format, and GPU MMU > > hierarchy? I could only get limited picture going through the code and > > comments. > > There is not any public documentation on the VM hardware other than > what is available in the driver. I can try and give you an overview > of how it works. There are 16 VM contexts (8 on cayman/TN/RL) on the > GPU that can be active at any given time. GPUVM supports a 40 bit > address space. Each context has an id, we call them vmids. vmid 0 is > a bit special. It's called the system context and behaves a bit > differently to the other ones. It's designed to be for the kernel > driver's view of GPU accessible memory. I can go into further detail > if you want, but I don't think it's critical for this discussion. > Just think of it as the context used by the kernel driver. So that > leaves 16 contexts (7 on cayman and TN/RL) available for use by user > clients. vmid 0 has one set of configuration registers and vmids 1-15 > share the same configuration (other than the page tables). E.g., > contexts 1-15 all have to use single or 2 level page tables for > example. You select which VM context is used for a particular command > buffer by a field in the command buffer packet. Some engines (e.g., > UVD or the display hardware) do not support VM so they always use vmid > 0. Right now only the graphics, compute, and DMA engines support VM. > > With single level page tables, you just have a big array of page table > entries (PTEs) that represent the entire virtual address space. With > multi-level page tables, the address space is represented by an array > of page directory entries (PDEs) that point to page table blocks > (PTBs) which are arrays of PTEs. > > PTEs and PDEs are 64 bits per entry. > > PDE: > 39:12 - PTB address > 0 - PDE valid (the entry is valid) > > PTE: > 39:12 - page address > 11:7 - fragment > 6 - write > 5 - read > 2 - CPU cache snoop (for accessing cached system memory) > 1 - system (page is in system memory rather than vram) > 0 - PTE valid (the entry is valid) > > Fragment needs some explanation. The logical/physical fragment size in > bytes = 2 ^ (12 + fragment). A fragment size of 0 means 4k, 1 means, > 8k, etc. The logical address must be aligned to the fragment size and > the memory backing it must be contiguous and at least as large as the > fragment size. Larger fragment sizes reduce the pressure on the TLB > since fewer entries are required for the same amount of memory. > > For system pages, the page address is the dma address of the page. > The system bit must be set and the snoop bit can be optionally set > depending on whether you are using cachable memory. > > For vram pages, the address is the GPU physical address of vram > (starts at 0 on dGPUs, starts at MC_VM_FB_OFFSET (dma address of > "vram" carve out) on APUs). > > You can also adjust the page table block size which controls the > number of pages per PTB. I.e., how many PDEs you need to cover the > address space. E.g., if you set the block size to 0, each PTB is 4k > which holds 512 PTEs; if you set it to 1 each PTB is 8k which holds > 1024 PTEs, etc. > > GPUVM is only concerned with memor
VM on GPUs
Hello radeon devs, I have been trying to find out more about VM implementation on SI+ hw, but unfortunately I could not find much in the public documents[0]. SI ISA manual suggests that there is a limited form of privileged mode on these chips, so I wondered if it could be used for VM management too (the docs only deal with numerical exceptions). Or does it always have to be handled by host (driver)? One of the older patches [1] mentions different page sizes, is there any public documentation on things like page table format, and GPU MMU hierarchy? I could only get limited picture going through the code and comments. thank you, Jan [0]http://developer.amd.com/resources/documentation-articles/developer-guides-manuals/ [1]http://lists.freedesktop.org/archives/dri-devel/2014-May/058858.html -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150220/ad8fe694/attachment.sig>
[PATCH 4/4] xf86drm: Unconditionally clear ioctl structs
_SAREA_CTX, ))
> @@ -2182,6 +2221,7 @@ int drmGetMap(int fd, int idx, drm_handle_t *offset,
> drmSize *size,
> {
> drm_map_t map;
>
> +memclear(map);
> map.offset = idx;
> if (drmIoctl(fd, DRM_IOCTL_GET_MAP, ))
> return -errno;
> @@ -2199,6 +2239,7 @@ int drmGetClient(int fd, int idx, int *auth, int *pid,
> int *uid,
> {
> drm_client_t client;
>
> +memclear(client);
> client.idx = idx;
> if (drmIoctl(fd, DRM_IOCTL_GET_CLIENT, ))
> return -errno;
> @@ -2215,6 +2256,7 @@ int drmGetStats(int fd, drmStatsT *stats)
> drm_stats_t s;
> unsignedi;
>
> +memclear(s);
> if (drmIoctl(fd, DRM_IOCTL_GET_STATS, ))
> return -errno;
>
> @@ -2352,6 +2394,7 @@ int drmSetInterfaceVersion(int fd, drmSetVersion
> *version)
> int retcode = 0;
> drm_set_version_t sv;
>
> +memclear(sv);
> sv.drm_di_major = version->drm_di_major;
> sv.drm_di_minor = version->drm_di_minor;
> sv.drm_dd_major = version->drm_dd_major;
> @@ -2383,12 +2426,11 @@ int drmSetInterfaceVersion(int fd, drmSetVersion
> *version)
> */
> int drmCommandNone(int fd, unsigned long drmCommandIndex)
> {
> -void *data = NULL; /* dummy */
> unsigned long request;
>
> request = DRM_IO( DRM_COMMAND_BASE + drmCommandIndex);
>
> -if (drmIoctl(fd, request, data)) {
> +if (drmIoctl(fd, request, NULL)) {
> return -errno;
> }
> return 0;
> @@ -2543,12 +2585,12 @@ void drmCloseOnce(int fd)
>
> int drmSetMaster(int fd)
> {
> - return drmIoctl(fd, DRM_IOCTL_SET_MASTER, 0);
> + return drmIoctl(fd, DRM_IOCTL_SET_MASTER, NULL);
> }
>
> int drmDropMaster(int fd)
> {
> - return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, 0);
> + return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
> }
>
> char *drmGetDeviceNameFromFd(int fd)
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150211/9758ea44/attachment.sig>
[libdrm][PATCH 2/2] Fix gcc -Wextra warnings
On Tue, 2015-02-10 at 00:27 +, Emil Velikov wrote: > On 10 February 2015 at 00:02, Jan Vesely wrote: > > On Mon, 2015-02-09 at 23:32 +, Emil Velikov wrote: > >> On 9 February 2015 at 21:39, Jan Vesely wrote: > >> > Signed-off-by: Jan Vesely > >> Nice one Jan. I've sent similar fixes for drmOpenDevice and > >> drmGetStats a few days ago. > >> > >> Considering you drop the last hunk that Ian spotted both patches are > >> Reviewed-by: Emil Velikov > > > > Thanks, I sent v2 of that patch few minutes ago. > > > > I think your 4/6 and 5/6 overlap with this one. Should I go ahead or do > > you plan to push yours? > > > I would go with your series - it handles more cases, plus already has > move reviews :-P > If you feel like looking through some of my series that would be appreciated. I wasn't subscribed to the list so I can't reply to those emails (don't know the message-ids). I looked at the series from Jan 29th [0]. 1/6[1], there is no tests/util directory, I guess it depends on Thierry's series? since it hasn't landed yet does it make sense to squash it there (like your 04.1/11 SQUASH: tests: misc cleanups) ? 2/6[2], also does not apply cleanly (needs Thierry's 5/11), if you want to push a version rebased on master you can add Reviewed-by: Jan Vesely to that one 4/6 and 5/6 were superseded, and I don't know enough about android to look at the other two, but 6/6 looks trivial enough Acked-by: Jan Vesely with a small nit: Why keep two assignments to LOCAL_SHARED_LIBRARIES in intel/Android.mk ? regards, jan [0]http://lists.freedesktop.org/archives/dri-devel/2015-January/076456.html [1]http://lists.freedesktop.org/archives/dri-devel/2015-January/076457.html [2]http://lists.freedesktop.org/archives/dri-devel/2015-January/076458.html > > >> > >> The strange part is that the normal Linux build does not show even a > >> single warning, despite the -Wextra and -Wsign-compare flags from > >> configure.ac. Perhaps my gcc does not like libdrm for some reason :P > > > > I think I just used CFLAGS= during configure, and it worked > > jan > > > Yes the issue was that we're not using WARN_CFLAGS in every Makefile in > libdrm. > Namely the top one and most of the tests. > > -Emil -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150210/5ef9f5a3/attachment-0001.sig>
[libdrm][PATCH 3/2] Fix always true comparison.
The only user I found is xserver, it can return -1 under certain conditions. So check for -1 explicitly. Signed-off-by: Jan Vesely --- I could not find whether it's actually legal to return encoded negative values in get_perm. This is a quick fix to detect the one case that I found. xf86drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xf86drm.c b/xf86drm.c index fb673b5..8e54ac9 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -335,7 +335,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type) drm_server_info->get_perms(_group, _mode); devmode = serv_mode ? serv_mode : DRM_DEV_MODE; devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH); - group = (serv_group >= 0) ? serv_group : DRM_DEV_GID; + group = (serv_group != ~0U) ? serv_group : DRM_DEV_GID; } #if !defined(UDEV) -- 2.1.0
[libdrm][PATCH 2/2] Fix gcc -Wextra warnings
On Mon, 2015-02-09 at 23:32 +, Emil Velikov wrote: > On 9 February 2015 at 21:39, Jan Vesely wrote: > > Signed-off-by: Jan Vesely > Nice one Jan. I've sent similar fixes for drmOpenDevice and > drmGetStats a few days ago. > > Considering you drop the last hunk that Ian spotted both patches are > Reviewed-by: Emil Velikov Thanks, I sent v2 of that patch few minutes ago. I think your 4/6 and 5/6 overlap with this one. Should I go ahead or do you plan to push yours? > > The strange part is that the normal Linux build does not show even a > single warning, despite the -Wextra and -Wsign-compare flags from > configure.ac. Perhaps my gcc does not like libdrm for some reason :P I think I just used CFLAGS= during configure, and it worked jan > > -Emil -- Jan Vesely -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150209/81728fb7/attachment.sig>
[libdrm][PATCH v2 1/2] random: Use unsigned long for seed
v2: Remove unrelated change in main()
This is more consistent with the rest, and avoids potential undefined
behavior (signed overflow) ind drmRandom()
Signed-off-by: Jan Vesely
---
xf86drmRandom.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/xf86drmRandom.c b/xf86drmRandom.c
index ecab9e2..94922ad 100644
--- a/xf86drmRandom.c
+++ b/xf86drmRandom.c
@@ -98,7 +98,7 @@ typedef struct RandomState {
unsigned long q; /* m div a */
unsigned long r; /* m mod a */
unsigned long check;
-long seed;
+unsigned long seed;
} RandomState;
#if RANDOM_MAIN
@@ -147,13 +147,13 @@ int drmRandomDestroy(void *state)
unsigned long drmRandom(void *state)
{
RandomState *s = (RandomState *)state;
-long hi;
-long lo;
+unsigned long hi;
+unsigned long lo;
hi = s->seed / s->q;
lo = s->seed % s->q;
s->seed = s->a * lo - s->r * hi;
-if (s->seed <= 0) s->seed += s->m;
+if ((s->a * lo) <= (s->r * hi)) s->seed += s->m;
return s->seed;
}
@@ -166,7 +166,7 @@ double drmRandomDouble(void *state)
}
#if RANDOM_MAIN
-static void check_period(long seed)
+static void check_period(unsigned long seed)
{
unsigned long count = 0;
unsigned long initial;
@@ -178,7 +178,7 @@ static void check_period(long seed)
while (initial != drmRandom(state)) {
if (!++count) break;
}
-printf("With seed of %10ld, period = %10lu (0x%08lx)\n",
+printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
seed, count, count);
drmRandomDestroy(state);
}
--
2.1.0
[libdrm][PATCH 1/2] random: Use unsigned long for seed
On Mon, 2015-02-09 at 15:11 -0800, Ian Romanick wrote:
> On 02/09/2015 01:39 PM, Jan Vesely wrote:
> > This is more consistent with the rest, and avoids potential undefined
> > behavior (signed overflow) in drmRandom()
> >
> > Signed-off-by: Jan Vesely
> > ---
> > xf86drmRandom.c | 14 +++---
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/xf86drmRandom.c b/xf86drmRandom.c
> > index ecab9e2..a084b86 100644
> > --- a/xf86drmRandom.c
> > +++ b/xf86drmRandom.c
> > @@ -98,7 +98,7 @@ typedef struct RandomState {
> > unsigned long q; /* m div a */
> > unsigned long r; /* m mod a */
> > unsigned long check;
> > -long seed;
> > +unsigned long seed;
> > } RandomState;
> >
> > #if RANDOM_MAIN
> > @@ -147,13 +147,13 @@ int drmRandomDestroy(void *state)
> > unsigned long drmRandom(void *state)
> > {
> > RandomState *s = (RandomState *)state;
> > -long hi;
> > -long lo;
> > +unsigned long hi;
> > +unsigned long lo;
> >
> > hi = s->seed / s->q;
> > lo = s->seed % s->q;
> > s->seed = s->a * lo - s->r * hi;
> > -if (s->seed <= 0) s->seed += s->m;
> > +if ((s->a * lo) <= (s->r * hi)) s->seed += s->m;
>
> This seems like an unrelated change.
This change is necessary. since s->seed is unsigned now it cannot be <
0. also the result of s->seed op s->q is unsigned.
>
> >
> > return s->seed;
> > }
> > @@ -166,7 +166,7 @@ double drmRandomDouble(void *state)
> > }
> >
> > #if RANDOM_MAIN
> > -static void check_period(long seed)
> > +static void check_period(unsigned long seed)
> > {
> > unsigned long count = 0;
> > unsigned long initial;
> > @@ -178,7 +178,7 @@ static void check_period(long seed)
> > while (initial != drmRandom(state)) {
> > if (!++count) break;
> > }
> > -printf("With seed of %10ld, period = %10lu (0x%08lx)\n",
> > +printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
> >seed, count, count);
> > drmRandomDestroy(state);
> > }
> > @@ -195,7 +195,7 @@ int main(void)
> > }
> > printf("After 1 iterations: %lu (%lu expected): %s\n",
> >rand, state->check,
> > - rand - state->check ? "*INCORRECT*" : "CORRECT");
> > + rand != state->check ? "*INCORRECT*" : "CORRECT");
>
> So does this.
I'll drop this one in v2, it shouldn't have been included
>
> > drmRandomDestroy(state);
> >
> > printf("Checking periods...\n");
> >
>
--
Jan Vesely
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150209/54aaa8b0/attachment-0001.sig>
[libdrm][PATCH 2/2] Fix gcc -Wextra warnings
Signed-off-by: Jan Vesely
---
tests/vbltest/vbltest.c | 3 ++-
xf86drm.c | 4 ++--
xf86drmMode.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/tests/vbltest/vbltest.c b/tests/vbltest/vbltest.c
index cdc1ef6..6e13c57 100644
--- a/tests/vbltest/vbltest.c
+++ b/tests/vbltest/vbltest.c
@@ -104,7 +104,8 @@ static void usage(char *name)
int main(int argc, char **argv)
{
- int i, c, fd, ret;
+ unsigned i;
+ int c, fd, ret;
char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "exynos",
"omapdrm", "tilcdc", "msm", "tegra" };
drmVBlank vbl;
drmEventContext evctx;
diff --git a/xf86drm.c b/xf86drm.c
index 345325a..fb673b5 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -303,7 +303,7 @@ static int chown_check_return(const char *path, uid_t
owner, gid_t group)
* special file node with the major and minor numbers specified by \p dev and
* parent directory if necessary and was called by root.
*/
-static int drmOpenDevice(long dev, int minor, int type)
+static int drmOpenDevice(dev_t dev, int minor, int type)
{
stat_t st;
const char *dev_name;
@@ -2213,7 +2213,7 @@ int drmGetClient(int fd, int idx, int *auth, int *pid,
int *uid,
int drmGetStats(int fd, drmStatsT *stats)
{
drm_stats_t s;
-int i;
+unsignedi;
if (drmIoctl(fd, DRM_IOCTL_GET_STATS, ))
return -errno;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 60ce369..e3e599b 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -854,7 +854,7 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx)
len = read(fd, buffer, sizeof buffer);
if (len == 0)
return 0;
- if (len < sizeof *e)
+ if (len < (int)sizeof *e)
return -1;
i = 0;
--
2.1.0
[libdrm][PATCH 1/2] random: Use unsigned long for seed
This is more consistent with the rest, and avoids potential undefined
behavior (signed overflow) in drmRandom()
Signed-off-by: Jan Vesely
---
xf86drmRandom.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/xf86drmRandom.c b/xf86drmRandom.c
index ecab9e2..a084b86 100644
--- a/xf86drmRandom.c
+++ b/xf86drmRandom.c
@@ -98,7 +98,7 @@ typedef struct RandomState {
unsigned long q; /* m div a */
unsigned long r; /* m mod a */
unsigned long check;
-long seed;
+unsigned long seed;
} RandomState;
#if RANDOM_MAIN
@@ -147,13 +147,13 @@ int drmRandomDestroy(void *state)
unsigned long drmRandom(void *state)
{
RandomState *s = (RandomState *)state;
-long hi;
-long lo;
+unsigned long hi;
+unsigned long lo;
hi = s->seed / s->q;
lo = s->seed % s->q;
s->seed = s->a * lo - s->r * hi;
-if (s->seed <= 0) s->seed += s->m;
+if ((s->a * lo) <= (s->r * hi)) s->seed += s->m;
return s->seed;
}
@@ -166,7 +166,7 @@ double drmRandomDouble(void *state)
}
#if RANDOM_MAIN
-static void check_period(long seed)
+static void check_period(unsigned long seed)
{
unsigned long count = 0;
unsigned long initial;
@@ -178,7 +178,7 @@ static void check_period(long seed)
while (initial != drmRandom(state)) {
if (!++count) break;
}
-printf("With seed of %10ld, period = %10lu (0x%08lx)\n",
+printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
seed, count, count);
drmRandomDestroy(state);
}
@@ -195,7 +195,7 @@ int main(void)
}
printf("After 1 iterations: %lu (%lu expected): %s\n",
rand, state->check,
- rand - state->check ? "*INCORRECT*" : "CORRECT");
+ rand != state->check ? "*INCORRECT*" : "CORRECT");
drmRandomDestroy(state);
printf("Checking periods...\n");
--
2.1.0
