[bug#40835] [PATCH] Update to use panfrost for graphics hardware acceleration

[Brian Woodcox <bw@inskydata.com>]

[-- Attachment #1: Type: text/plain, Size: 6637 bytes --]

Hi Vagrant,

Sorry, this should have been applied to commit d4c6e06f369024efc63e11de1a5bacd3fe9f7e8d on the tip-pinebook-pro branch.

The rest of my answers below.

> On Apr 27, 2020, at 12:15 PM, Vagrant Cascadian <vagrant@debian.org> wrote:
> 
> On 2020-04-24, Brian Woodcox wrote:
>> These patches add the panfrost graphics acceleration for the Pinebook
>> Pro laptop.
> 
> Thanks! Been working with the pinebook pro for some months now running
> guix, and it's great to see others making progress on it. :)
> 
> 
>> You need to edit the /boot/extlinux/extlinux.conf file on the SD card and alter the FDTDIR line.
>> 
>> I changed mine from
>> 
>> FDTDIR /gnu/store/ls1byzmapi911cylh4s6044x0cmc61c8-linux-libre-pinebook-pro-5.6.0/lib/dtbs
>> 
>> to
>> 
>> FDTDIR /gnu/store/ls1byzmapi911cylh4s6044x0cmc61c8-linux-libre-pinebook-pro-5.6.0/lib/dtbs/rockchip
> 
> The u-boot-pinebook-pro-rk3399 on guix master works correctly as well as
> the one from wip-pinebook-pro (should be the same).
> 
> This seems like your u-boot does not contain the correct value for
> "fdtfile". It should be rockchip/rk3399-pinebook-pro.dtb. Are you
> actually running an older u-boot? Did you at any point run saveenv from
> u-boot, which saves the old u-boot configuration with an inappropriate
> fdtfile variable?
> 
> 
> It would be better to split up your patches into a separate patch
> series, it is hard to review as one single large patch changing many
> things.
> 

I’m not sure what this problem is exactly.  For some reason the rockchip folder is not being added
to the end of the patch for the FDTFILE, also, you do not need to actually specify the file as u-boot will
find it as long as it’s on the directory.


> A few targeted comments below...
> 
>> diff --git a/gnu/packages/gl.scm b/gnu/packages/gl.scm
>> index 01241cd88e..65fe389927 100644
>> --- a/gnu/packages/gl.scm
>> +++ b/gnu/packages/gl.scm
>> @@ -293,7 +294,7 @@ also known as DXTn or DXTC) for Mesa.")
>>        '(,@(match (%current-system)
>>              ((or "armhf-linux" "aarch64-linux")
>>               ;; TODO: Fix svga driver for aarch64 and armhf.
>> -              '("-Dgallium-drivers=etnaviv,freedreno,nouveau,r300,r600,swrast,tegra,v3d,vc4,virgl"))
>> +              '("-Dgallium-drivers=etnaviv,freedreno,kmsro,lima,nouveau,panfrost,r300,r600,swrast,tegra,v3d,vc4,virgl"))
>>              (_
>>               '("-Dgallium-drivers=iris,nouveau,r300,r600,radeonsi,svga,swrast,virgl")))
>>          ;; Enable various optional features.  TODO: opencl requires libclc,
> 
> This last part of your mesa patch is already on core-updates. Looking
> forward to when the rest is properly supported upstream!

Okay, thanks.

> 
> 
>> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
>> index dd088ea24f..d4a36533ab 100644
>> --- a/gnu/packages/linux.scm
>> +++ b/gnu/packages/linux.scm
>> @@ -326,7 +327,7 @@ corresponding UPSTREAM-SOURCE (an origin), using the given DEBLOB-SCRIPTS."
>>                   (with-directory-excursion dir
>>                     (setenv "PYTHON" (which "python"))
>>                     (format #t "Running deblob script...~%")
>> -                    (force-output)
>> +                    (force-output))
>>                     (invoke "/tmp/bin/deblob"))
>> 
>>                   (format #t "~%Packing new Linux-libre tarball...~%")
> 
> This looks like leftovers from your hack breaking linux-libre :P

Doh, you are correct, my mistake.  This should of course be left as the original code.

> 
> 
>> @@ -604,6 +605,7 @@ for ARCH and optionally VARIANT, or #f if there is no such configuration."
>>     ("CONFIG_SECURITY_DMESG_RESTRICT" . #t)
>>     ;; All kernels should have NAMESPACES options enabled
>>     ("CONFIG_NAMESPACES" . #t)
>> +    ("CONFIG_DRM_PANFROST" . #t)
>>     ("CONFIG_UTS_NS" . #t)
>>     ("CONFIG_IPC_NS" . #t)
>>     ("CONFIG_USER_NS" . #t)
> 
> This obviously can't be enabled on all architectures. In the
> linux-libre-arm64-generic and linux-libre-pinebook-pro kernels it's
> already enabled as a module.
> 
> It obviously makes debugging easier to be available earlier, but it also
> bloats platforms that do not use this driver.

Okay.

> 
> 
>> diff --git a/gnu/packages/patches/mesa-skip-disk-cache-test.patch b/gnu/packages/patches/mesa-skip-disk-cache-test.patch
>> index 190f6b6ee1..585bf4f648 100644
>> --- a/gnu/packages/patches/mesa-skip-disk-cache-test.patch
>> +++ b/gnu/packages/patches/mesa-skip-disk-cache-test.patch
>> @@ -1,11 +1,6 @@
>> -disk_cache_create() here looks up the users home directory from <pwd.h>
>> -which resolves to "/" in the build environment. I could not find an easy
>> -way to set the home directory to something else, so we disable this test
>> -for now.
>> -
>> --- a/src/compiler/glsl/tests/cache_test.c
>> +++ b/src/compiler/glsl/tests/cache_test.c
>> -@@ -170,11 +170,6 @@
>> +@@ -219,11 +219,6 @@
>>     unsetenv("MESA_GLSL_CACHE_DIR");
>>     unsetenv("XDG_CACHE_HOME");
> 
> This removes a comment from the refreshed patch; I presume the comment
> is still appropriate, though?

Yes, Patch should have been applied to d4c6e06f369024efc63e11de1a5bacd3fe9f7e8d as stated above.

> 
> 
>> diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
>> index 8696dc4bb6..a1e7684964 100644
>> --- a/gnu/packages/qt.scm
>> +++ b/gnu/packages/qt.scm
>> @@ -15,6 +15,7 @@
>> ;;; Copyright © 2018 John Soo <jsoo1@asu.edu>
>> ;;; Copyright © 2020 Mike Rosset <mike.rosset@gmail.com>
>> ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net>
>> +;;; Copyright © 2020 Brian C. Woodcox <bw@InSkyData.com>
>> ;;;
>> ;;; This file is part of GNU Guix.
>> ;;;
>> @@ -485,6 +486,7 @@ developers using C++ or QML, a CSS & JavaScript like language.")
>>                  "-no-compile-examples"
>>                  ;; Most "-system-..." are automatic, but some use
>>                  ;; the bundled copy by default.
>> +                 "-opengl" "es2"
>>                  "-system-sqlite"
>>                  "-system-harfbuzz"
>>                  "-system-pcre"
> 
> This might break some things where a different opengl is the default,
> some architectures or platforms may require a different opengl
> implementation.
> 
> I seem to recall some conversations in Debian about the complexities
> around which opengl to enable per-architecture or per-platform or ... a
> complicated matrix of concerns.

Open to suggestions.

> 
> 
> live well,
>  vagrant


Thanks for the feedback.

Brian.

[-- Attachment #2: Type: text/html, Size: 12874 bytes --]