[rock-dkms-bin] Fix DRM_VER in dkms Makefile

[agravgaard]

DRM_VER will only be defined for a hardcoded selection of OS names. Arch (and Manjaro) is not in that list, so the dkms compilation will fail.

This change patches the makefile to provide a default, which assumes same conditions as under some other OS names. That condition being that $(kdir) is defined and that it contains a Makefile with VERSION and PATCHLEVEL

This compiles successfully with 4.19 and 5.6 but not 5.7
(Because pci_platform_rom was removed in 5.7 )

5.6 boots normally with the module installed, but I'm not sure how to check if it was actually loaded, or if it's still using the one in the kernel?

P.S. Unfortunately clinfo still reports opencl 2.0 (with rocm-opencl-runtime) so, unless the rock module didn't actually load, this wasn't the solution to that problem...

Related issue:

ROCK-Kernel-Driver#94

[acxz]

Can anyone else comment on the validity/test this out?

Just for ref here is the previous patch we had. https://github.com/rocm-arch/rocm-arch/blob/master/rock-dkms/Makefile.patch

Would this be a patch that upstream would be interested in? If so I recommend trying to create a PR there and getting their review as well.

[tpkessler]

Thank you for working on this @agravgaard. I also don't know why clinfo still reports version 2.0.

Kernel 5.7 was added the arch repos yesterday. I think we have to wait for AMD to fix rock-dkms-bin.

[agravgaard]

Makes sense, I'll create an issue (and/or merge request) to rock during the weekend, if there isn't one already.

As I haven't written much PKGBUILDs for general use. Is patch files preferred to sed fixes? Or is it just a matter of convenience and size of the patch?

[acxz]

Makes sense, I'll create an issue (and/or merge request) to rock during the weekend, if there isn't one already.

Sweet. I'll be willing to merge this in much more quickly if a PR is made upstream regarding this.

Also considering this concern does not have its own issue, it would be best if an issue is created so that further discussion on this can be made there and the PR specific discussion occurs in the associated PR.

Is patch files preferred to sed fixes? Or is it just a matter of convenience and size of the patch?

I personally prefer patch files, since they help modularize the issue out of the PKGBUILD, even if the change can be accomplished with a simple sed.

[acxz]

@agravgaard thx for creating the upstream issue and PR.

Can you now change your commits here so that instead of using sed we pull in the PR as a patch and apply? This ensures we are using the same patch as the PR and gives upstream a datapoint that shows the PR working increasing the chance of it getting merged.

See the following PKGBUILD on how to do so if you are not familiar: https://github.com/ros-noetic-arch/ros-noetic-moveit-ros-planning-interface/blob/master/PKGBUILD#L46

You can get the link by just adding .patch at the end of the PR url, pretty nifty.

[acxz]

I get the following error:

==> Starting package()...
/home/acxz/vcs/git/github/agravgaard/rocm-arch/rock-dkms-bin/PKGBUILD: line 26: filterdiff: command not found
==> ERROR: A failure occurred in package().

Can things be done without using filterdiff?

[agravgaard]

Thanks for the .patch tip, that's pretty cool!

Yep, I'll find a way to do without filterdiff.

It seems 5.6 and 4.19 still works fine with this change.
However, it doesn't completely fix 5.7, as I get the following error:

  MODPOST 4 modules
ERROR: modpost: "kallsyms_lookup_name" [/var/lib/dkms/amdgpu-3.5/30/build/amd/amdkcl/amdkcl.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1
make: *** [Makefile:1642: modules] Error 2
make: Leaving directory '/usr/lib/modules/5.7.0-3-MANJARO/build'

Other than the above, there are some warnings, but no other errors in the log. So I can't quite guess why amdkcl.ko is not being made.

EDIT: Turns out 5.7 broke this: anbox/anbox-modules#49

[agravgaard]

With the recent changes to the upstream PR (ROCm/ROCK-Kernel-Driver#95), this seems to be working with 5.7.
#WorksOnMyMachine

(Not particularly pretty with the grep'ing, I know, but I couldn't find a cleaner way that didn't require excessive bash scripting.)

[acxz]

Sweet thats great! LGTM

[tpkessler]

Thank you for your great work @agravgaard! I've successfully compiled the module and can confirm that it runs on my machine (Ryzen 2700X, Vega 56 with Arch Linux, kernel 5.7.2). Still, only OpenCL 2.0 works. I tried some OpenCL 2.2 examples but they didn't compile. For a collection of OpenCL examples from AMDAPP SDK, see here

[agravgaard]

Still, only OpenCL 2.0 works. I tried some OpenCL 2.2 examples but they didn't compile. For a collection of OpenCL examples from AMDAPP SDK, see here

@tpkessler Yeah, it's odd... Since clinfo doesn't report spir support either (see here) I was originally thinking it'll either require a driver update, or that maybe it's only supported on the server type gpus (or yet-to-be-released gpus). However, I think the platform should report 2.2 (if they truly have added support*) even if the device doesn't support it (?)

*Do you know if anyone has tried on one of the officially supported platforms yet?

[tpkessler]

I tried the rocm-terminal container from AMD but it also reported OpenCL 2.0. The reason for this may be that a docker container shares the kernel with the host system. I haven't installed one of the supported distros yet. I plan to install ubuntu 18.04 on my PC and see if OpenCL 2.2 works there.

[tpkessler]

*Do you know if anyone has tried on one of the officially supported platforms yet?

I installed ROCm on Ubuntu 18.04 LTS and clinfo also reports OpenCL 2.0 (and no SPIR-V). I opened an issue upstream: ROCm/ROCm-OpenCL-Runtime#122

[acxz]

Let us move this conversation to a separate issue, since it should not be happening on a PR. See: #267