Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HVS Core Clock Request Fix #4402

Merged

Conversation

mripard
Copy link
Contributor

@mripard mripard commented Jun 21, 2021

Hi,

Here's a fix for the breakage introduced by #4365. This significantly reworks the current code to drop the new, buggy, accessors and use the load tracker infrastructure that was already in place.

I haven't tested kmstest --flip yet, but the other issues seem to be gone.

@aBUGSworstnightmare-rpi
Copy link
Contributor

@maxime: what does 'kmstest --flip' and what is the full command line for testing it?
I'm compiling DSI8x driver now and I saw you mentioning issues with that test.

@6by9
Copy link
Contributor

6by9 commented Jun 21, 2021

(Wrong @ mripard, not maxime)

what does 'kmstest --flip' and what is the full command line for testing it?
I'm compiling DSI8x driver now and I saw you mentioning issues with that test.

See the dri-devel discussion. https://github.com/tomba/kmsxx/tree/master/utils

git clone https://github.com/tomba/kmsxx
cd kmsxx
sudo apt install libfmt-dev
meson build
ninja -C build
./build/utils/kmstest --flip

@6by9
Copy link
Contributor

6by9 commented Jun 21, 2021

And that builds and seems to run kmstest --flip fine.

@aBUGSworstnightmare-rpi
Copy link
Contributor

hmm ... not a Linux pro here, so something is missing

pi@raspberrypi:~ $ git clone https://github.com/tomba/kmsxx
Cloning into 'kmsxx'...
remote: Enumerating objects: 4309, done.
remote: Counting objects: 100% (275/275), done.
remote: Compressing objects: 100% (159/159), done.
remote: Total 4309 (delta 161), reused 199 (delta 102), pack-reused 4034
Receiving objects: 100% (4309/4309), 847.21 KiB | 1.61 MiB/s, done.
Resolving deltas: 100% (2877/2877), done.
pi@raspberrypi:~ $ cd kmsxx
pi@raspberrypi:~/kmsxx $ sudo apt install libfmt-dev
Reading package lists... Done
Building dependency tree       
Reading state information... Done
The following package was automatically installed and is no longer required:
  python-colorzero
Use 'sudo apt autoremove' to remove it.
Suggested packages:
  libfmt-doc
The following NEW packages will be installed:
  libfmt-dev
0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
Need to get 124 kB of archives.
After this operation, 681 kB of additional disk space will be used.
Get:1 http://debian.bio.lmu.de/raspbian/raspbian buster/main armhf libfmt-dev armhf 5.2.1+ds-2 [124 kB]
Fetched 124 kB in 1s (94.8 kB/s)   
Selecting previously unselected package libfmt-dev.
(Reading database ... 168469 files and directories currently installed.)
Preparing to unpack .../libfmt-dev_5.2.1+ds-2_armhf.deb ...
Unpacking libfmt-dev (5.2.1+ds-2) ...
Setting up libfmt-dev (5.2.1+ds-2) ...
pi@raspberrypi:~/kmsxx $ meson build
bash: meson: command not found

@popcornmix
Copy link
Collaborator

sudo apt install meson

@aBUGSworstnightmare-rpi
Copy link
Contributor

aBUGSworstnightmare-rpi commented Jun 21, 2021

Thanks for the hint, but NOPE!

pi@raspberrypi:~/kmsxx $ sudo apt install meson
Reading package lists... Done
Building dependency tree       
Reading state information... Done
The following package was automatically installed and is no longer required:
  python-colorzero
Use 'sudo apt autoremove' to remove it.
The following additional packages will be installed:
  ninja-build
The following NEW packages will be installed:
  meson ninja-build
0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded.
Need to get 526 kB of archives.
After this operation, 3,107 kB of additional disk space will be used.
Do you want to continue? [Y/n] y
Get:1 http://archive.raspberrypi.org/debian buster/main armhf ninja-build armhf 1.10.1-1~bpo10+1 [88.9 kB]
Get:2 http://archive.raspberrypi.org/debian buster/main armhf meson all 0.56.1-1~bpo10+1 [437 kB]
Fetched 526 kB in 1s (501 kB/s)
Selecting previously unselected package ninja-build.
(Reading database ... 168491 files and directories currently installed.)
Preparing to unpack .../ninja-build_1.10.1-1~bpo10+1_armhf.deb ...
Unpacking ninja-build (1.10.1-1~bpo10+1) ...
Selecting previously unselected package meson.
Preparing to unpack .../meson_0.56.1-1~bpo10+1_all.deb ...
Unpacking meson (0.56.1-1~bpo10+1) ...
Setting up ninja-build (1.10.1-1~bpo10+1) ...
Setting up meson (0.56.1-1~bpo10+1) ...
Processing triggers for man-db (2.8.5-2) ...
pi@raspberrypi:~/kmsxx $ meson build
The Meson build system
Version: 0.56.1
Source dir: /home/pi/kmsxx
Build dir: /home/pi/kmsxx/build
Build type: native build
Project name: kms++
Project version: 0.0.0
C++ compiler for the host machine: c++ (gcc 8.3.0 "c++ (Raspbian 8.3.0-6+rpi1) 8.3.0")
C++ linker for the host machine: c++ ld.bfd 2.31.1
Host machine cpu family: arm
Host machine cpu: armv7l
Compiler for C++ supports arguments -Wno-psabi: YES 
Compiler for C++ supports arguments -Wno-c99-designator: NO 
Found pkg-config: /usr/bin/pkg-config (0.29)
Did not find CMake 'cmake'
Found CMake: NO
Run-time dependency fmt found: NO (tried pkgconfig)

meson.build:35:0: ERROR: Dependency "fmt" not found, tried pkgconfig

A full log can be found at /home/pi/kmsxx/build/meson-logs/meson-log.txt
pi@raspberrypi:~/kmsxx $ 

Edit: btw I'm on 6by9's MAREK DSI8x branch

pi@raspberrypi:~/kmsxx $ uname -a
Linux raspberrypi 5.10.44-v7l+ #1 SMP Mon Jun 21 17:08:23 CEST 2021 armv7l GNU/Linux
pi@raspberrypi:~/kmsxx $ 

@popcornmix
Copy link
Collaborator

popcornmix commented Jun 21, 2021

@aBUGSworstnightmare-rpi this isn't the place to debug build issues of an unrelated package. Google for error then apt install the package referenced. If still stuck use the forum.

Tested and it fixes kmstest --flip but breaks kodi

2021-06-21 17:53:31.867 T:1165    ERROR <general>: 
                                                   Object: plane        ID: 91
                                                     Property: FB_ID    ID: 17  Value: 0
                                                     Property: CRTC_ID  ID: 20  Value: 0
                                                   Object: plane        ID: 70
                                                     Property: SRC_X    ID: 9   Value: 0
                                                     Property: SRC_Y    ID: 10  Value: 0
                                                     Property: SRC_W    ID: 11  Value: 251527168
                                                     Property: SRC_H    ID: 12  Value: 141557760
                                                     Property: CRTC_X   ID: 13  Value: 0
                                                     Property: CRTC_Y   ID: 14  Value: 0
                                                     Property: CRTC_W   ID: 15  Value: 1919
                                                     Property: CRTC_H   ID: 16  Value: 1080
                                                     Property: FB_ID    ID: 17  Value: 229
                                                     Property: CRTC_ID  ID: 20  Value: 76
2021-06-21 17:53:31.867 T:1165    ERROR <general>: CDRMAtomic::DrmAtomicCommit - atomic commit failed: Invalid argument
2021-06-21 17:53:31.907 T:1165    ERROR <general>: CDRMAtomic::DrmAtomicCommit - test commit failed: (No space left on device)

I don't think we want the hvs tracker enabled, as it is far too pessimistic about what is possible (e.g. here 1920x1200 RGBA32 + 4K P30 video is rejected).

@popcornmix
Copy link
Collaborator

You can reproduce failure with modetest:

modetest -M vc4 -F tiles,gradient -s 32:1920x1080-60 -P91@76:3840x2160+0+0@XR24 -P97@76:1920x1080+0+0@YV12
setting mode 1920x1080-60.00Hz on connectors 32, crtc 76
failed to set gamma: Invalid argument
testing 3840x2160@XR24 overlay plane 91
testing 1920x1080@YV12 overlay plane 97
failed to enable plane: No space left on device

@graysky2
Copy link

Changes fix the issue manifesting in greeters (tested with sddm) but as @popcornmix pointed out, the changes break kodi playback.

vc4_crtc_config_pv() retrieves the encoder again, even though its only
caller, vc4_crtc_atomic_enable(), already did.

Pass the encoder pointer as an argument instead of going through all the
connectors to retrieve it again.

Signed-off-by: Maxime Ripard <[email protected]>
It turns out the encoder retrieval code, in addition to being
unnecessarily complicated, has a bug when only the planes and crtcs are
affected by a given atomic commit.

Indeed, in such a case, either drm_atomic_get_old_connector_state or
drm_atomic_get_new_connector_state will return NULL and thus our encoder
retrieval code will not match on anything.

We can however simplify the code by using drm_for_each_encoder_mask, the
drm_crtc_state storing the encoders a given CRTC is connected to
directly and without relying on any other state.

Signed-off-by: Maxime Ripard <[email protected]>
The encoder retrieval code has been a source of bugs and glitches in the
past and the crtc <-> encoder association been wrong in a number of
different ways.

Add some logging to quickly spot issues if they occur.

Signed-off-by: Maxime Ripard <[email protected]>
The load tracker was initially designed to report and warn about a load
too high for the HVS. To do so, it computes for each plane the impact
it's going to have on the HVS, and will warn (if it's enabled) if we go
over what the hardware can process.

While the limits being used are a bit irrelevant to the BCM2711, the
algorithm to compute the HVS load will be one component used in order to
compute the core clock rate on the BCM2711.

Let's remove the hooks to prevent the load tracker to do its
computation, but since we don't have the same limits, don't check them
against them, and prevent the debugfs file to enable it from being
created.

Signed-off-by: Maxime Ripard <[email protected]>
Depending on a given HVS output (HVS to PixelValves) and input (planes
attached to a channel) load, the HVS needs for the core clock to be
raised above its boot time default.

Failing to do so will result in a vblank timeout and a stalled display
pipeline.

Signed-off-by: Maxime Ripard <[email protected]>
@mripard mripard force-pushed the rpi/5.10-core-clock-request-fix branch from 3d4ce3c to 953532f Compare June 23, 2021 08:58
@mripard
Copy link
Contributor Author

mripard commented Jun 23, 2021

I just pushed a new version that should address the Kodi (and modetest) issue

@graysky2
Copy link

graysky2 commented Jun 23, 2021

@mripard - I built this patched on the latest rpi-5.10.y commit and confirm kodi functionality against a library of 12 different test clips (8/10-bit mix of x265/h264/mpeg2/divx/av1). I can also confirm sddm works as expected, good job. Arch ARM/aarch64.

@popcornmix
Copy link
Collaborator

kmstest --flip and kodi is working for me. Happy for this to be merged.

@pelwell pelwell merged commit 69a25f0 into raspberrypi:rpi-5.10.y Jun 23, 2021
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Jun 23, 2021
See: raspberrypi/linux#4402

kernel: DPI mode 6 18bpp support
See: raspberrypi/linux#4400

firmware: bcm_host: Recognise all Pi 4 variants, add BCM2711
See: raspberrypi/userland#695
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Jun 23, 2021
See: raspberrypi/linux#4402

kernel: DPI mode 6 18bpp support
See: raspberrypi/linux#4400

firmware: bcm_host: Recognise all Pi 4 variants, add BCM2711
See: raspberrypi/userland#695
@mripard mripard deleted the rpi/5.10-core-clock-request-fix branch June 29, 2021 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants