DisplayLink / evdi

Extensible Virtual Display Interface
MIT License
705 stars 184 forks source link

Add support for kernel 6.0 + a minor fix #381

Closed SimPilotAdamT closed 1 year ago

SimPilotAdamT commented 2 years ago

Fixes https://github.com/DisplayLink/evdi/issues/376 by implementing @Crashdummyy's fix as posted here, + fixes the style used in evdi_painter.c

Before submitting the PR please make sure you have run ci scripts:

If you have more than one change consider creating separate PRs for them; it will make the review process much easier. Also provide some description (links to source code or mailing list are welcome - this might help to understand the change).

Thanks for the contribution!

SimPilotAdamT commented 2 years ago

This works for me. I am by no means a C programmer, but I think there needs to be a way to make sure the #include <drm/drm_framebuffer.h> line added to module/evdi_drm_drv.h is only done on Kernel versions 6.0.0 and newer. I have no clue how to do this... What about you guys?

Crashdummyy commented 2 years ago

This works for me. I am by no means a C programmer, but I think there needs to be a way to make sure the #include <drm/drm_framebuffer.h> line added to module/evdi_drm_drv.h is only done on Kernel versions 6.0.0 and newer. I have no clue how to do this... What about you guys?

Well of course it would break on every kernel < 6.0. You need some compiler flags ( look at the lines 28 through 32 ).

I think you need to do something like this

#if KERNEL_VERSION(6, 0, 0) >= LINUX_VERSION_CODE
# All kernel versions greather than 6.0.0 use the framebuffer of drm_framebuffer.h with the install method migrated to drm_legacy.h
#include <drm/drm_legacy.h>
#include <drm/drm_framebuffer.h>
#else
# Below 6.0.0 the framebuffer struct still resides in drm_irq.h
#include <drm/drm_irq.h>
#endif

Sorry Im on the phone right now cant test ist

SimPilotAdamT commented 2 years ago

Oh I think ik how to do it, commit coming soon

SimPilotAdamT commented 2 years ago

OK that's done... I do have an issue using the CI build script checking the build on Kernel versions older than 5.17...

Here is the error, which really stems from a warning using -Werror. This was run using Kernel version 5.16, but the same issue is present when I try on any other older version.

  CC      /home/adam/evdi/tmp/linux-5.16/tools/objtool/help.o
In file included from help.c:12:
In function ‘xrealloc’,
    inlined from ‘add_cmdname’ at help.c:24:2:
subcmd-util.h:56:23: error: pointer may be used after ‘realloc’ [-Werror=use-after-free]
   56 |                 ret = realloc(ptr, size);
      |                       ^~~~~~~~~~~~~~~~~~
subcmd-util.h:52:21: note: call to ‘realloc’ here
   52 |         void *ret = realloc(ptr, size);
      |                     ^~~~~~~~~~~~~~~~~~
subcmd-util.h:58:31: error: pointer may be used after ‘realloc’ [-Werror=use-after-free]
   58 |                         ret = realloc(ptr, 1);
      |                               ^~~~~~~~~~~~~~~
subcmd-util.h:52:21: note: call to ‘realloc’ here
   52 |         void *ret = realloc(ptr, size);
      |                     ^~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [/home/adam/evdi/tmp/linux-5.16/tools/build/Makefile.build:97: /home/adam/evdi/tmp/linux-5.16/tools/objtool/help.o] Error 1
make[3]: *** [Makefile:59: /home/adam/evdi/tmp/linux-5.16/tools/objtool/libsubcmd-in.o] Error 2
make[2]: *** [Makefile:63: /home/adam/evdi/tmp/linux-5.16/tools/objtool/libsubcmd.a] Error 2
make[1]: *** [Makefile:69: objtool] Error 2
make: *** [Makefile:1349: tools/objtool] Error 2
SimPilotAdamT commented 2 years ago

(Changed again bc I'm so dumb and couldn't fathom >= and <=. The error still happens.)

SimPilotAdamT commented 2 years ago

ok now it should properly work, don't quote me on that

displaylink-emajewsk commented 1 year ago

Thanks for the PR, looks good, but let me verify it

Crashdummyy commented 1 year ago

Thanks for the PR, looks good, but let me verify it

looks good to me but you should check if the dependency to drm_legacy.h is still required. The migrated methods were drm_irq_uninstall and drm_irq_install I dont see them being used in the implementation so maybe its not needed at all

SimPilotAdamT commented 1 year ago

Is anyone able to help me test smth? The CI build script passes this branch on my repo on kernel versions 5.16 and newer, but I'm not sure if it will actually install the module properly... I don't have a machine I can test on myself

displaylink-emajewsk commented 1 year ago

Is anyone able to help me test smth? The CI build script passes this branch on my repo on kernel versions 5.16 and newer, but I'm not sure if it will actually install the module properly... I don't have a machine I can test on myself

Seems fine to me. If you merge those branches, we can pull it in. We'll do a thorough testing before a release :)

SimPilotAdamT commented 1 year ago

Seems fine to me. If you merge those branches, we can pull it in. We'll do a thorough testing before a release :)

Merged

dimitarsh1 commented 1 year ago

I just updated to kernel 6 on Debian testing

I get the following:

$ sudo dkms install -m evdi -v 1.12.0
Sign command: /usr/lib/linux-kbuild-6.0/scripts/sign-file
Signing key: /var/lib/dkms/mok.key
Public certificate (MOK): /var/lib/dkms/mok.pub

Building module:
Cleaning build area...
make -j8 KERNELRELEASE=6.0.0-2-amd64 all INCLUDEDIR=/lib/modules/6.0.0-2-amd64/build/include KVERSION=6.0.0-2-amd64 DKMS_BUILD=1...(bad exit status: 2)
Error! Bad return status for module build on kernel: 6.0.0-2-amd64 (x86_64)
Consult /var/lib/dkms/evdi/1.12.0/build/make.log for more information.

I have pulled the latest devel update.

Any suggestions

SimPilotAdamT commented 1 year ago

Any suggestions

Can you send that make.log file?

dimitarsh1 commented 1 year ago

Yup, here it is.

make.log

Crashdummyy commented 1 year ago

Yup, here it is.

make.log

error: field ‘base’ has incomplete type Are you sure youre on the latest devel ? Because that is the exact error that is thrown before the fix.

Further please dont use a ancient distro

dimitarsh1 commented 1 year ago
$ git status
On branch devel
Your branch is up to date with 'origin/devel'.

nothing to commit, working tree clean
$ git pull
Already up to date.

I don't get the last comment ;)

SimPilotAdamT commented 1 year ago

@dimitarsh1 it might be a good idea to make another issue... Might be a bug in this PR that wasn't spotted. I'd ignore the last comment.

Crashdummyy commented 1 year ago
$ git status
On branch devel
Your branch is up to date with 'origin/devel'.

nothing to commit, working tree clean
$ git pull
Already up to date.

I don't get the last comment ;)

Mhm that looks okay to me. I dont have my laptop with me ( vacation ) so I can test it. But someone here might try to compile it using distrobox or docker.

Regarding my last comment: The Problem with old distros ( eg Debian ) are old packages and an old kernel as well. Youll hit a lot of these obstacles if you run Debian on a machine of this decade and youll make your life a lot easier if you just install fedora or arch on your device. Im pretty sure the reason you cant compile is that either your compiler itself or some dependencies evdi relies on are too old on your system.

dimitarsh1 commented 1 year ago

Dear all,

After updating my system it is all fine now. I assume some other libraries had to be updated...

Thanks for all the support.

Kind regards, Dimitar

Skesov commented 1 year ago

When you push tag on these changes?