Skip to content

vc4 gamma fixes#4682

Merged
pelwell merged 5 commits intoraspberrypi:rpi-5.10.yfrom
6by9:rpi-5.10.y
Nov 16, 2021
Merged

vc4 gamma fixes#4682
pelwell merged 5 commits intoraspberrypi:rpi-5.10.yfrom
6by9:rpi-5.10.y

Conversation

@6by9
Copy link
Copy Markdown
Contributor

@6by9 6by9 commented Nov 8, 2021

I need to check that Pi3 is happy. Pi4 seems to be.

@6by9
Copy link
Copy Markdown
Contributor Author

6by9 commented Nov 8, 2021

#4676

@popcornmix
Copy link
Copy Markdown
Collaborator

popcornmix commented Nov 10, 2021

Tested on Pi4. The refcount_warn_saturate warn has gone, but I've still got a bright hdmi display with two monitors connected.
It's actually HDMI0 this time (I've normally seen HDMI1 as the bright one).

@popcornmix
Copy link
Copy Markdown
Collaborator

popcornmix commented Nov 10, 2021

Interestingly leaving the Pi with bright HDMI0 for 30 minutes resulted in both displays blanking for a couple of seconds and they came back without the brightness issue. I wonder if X does send a gamma command after 30 mins (screensaver/screen blanking?). But it didn't remain black.
A reboot and HDMI 0 was bright again.

@6by9
Copy link
Copy Markdown
Contributor Author

6by9 commented Nov 10, 2021

I need to make sure I've pushed the latest changes (found a couple of other issues at the same time, and testing Pi3 has been awkward).
The latest set should disable gamma such that sudo cat /sys/kernel/debug/dri/1/hvs_gamma reports such. Are you seeing that?

6by9 added 4 commits November 10, 2021 16:11
@6by9
drm: Fix double free from checking if gamma lut has been updated
4487393 
The code falls through to "fail" under all conditions, so there is no
need for the drm_property_blob_put if the gamma lut hasn't been changed.
Fixes: 9cca266 "drm: Check whether the gamma lut has changed before updating"

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
@6by9
drm/vc4: Enable gamma block only when required.
926f58b 
With HVS5 the gamma block is now only reprogrammed with
a disable/enable. Loading the table from vc4_hvs_init_channel
(called from vc4_hvs_atomic_enable) appears to be at an
invalid point in time and so isn't applied.

Switch to enabling and disabling the gamma table instead. This
isn't safe if the pipeline is running, but it isn't now.
For HVS4 it is safe to enable and disable dynamically, so
adopt that approach there too.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
@6by9
drm/vc4: Only add gamma properties once.
faa26ca 
Two calls were made to drm_crtc_enable_color_mgmt to add gamma
and CTM, however they were both set to add the gamma properties,
so they ended up added twice.

Fixes: 766cc6b "drm/vc4: Add CTM support"
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
@6by9
dtoverlays: Remove i2c0mux and i20if status from edt-ft5406.dtsi
0339df3 
edt-ft5406.dtsi is included from vc4-kms-dsi-7inch which was
also setting i2c0mux and i2c0if status fields. This meant that
dtoverlay wouldn't apply the overlay due to multiple fragments
changing the same parameter.

Move the enable from edt-ft5406.dtsi to edt-ft5406-overlay.dts
for when it should be needed as an independent overlay.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
@6by9
Copy link
Copy Markdown
Contributor Author

6by9 commented Nov 10, 2021

Branch rebased and latest changes pushed.

I'm not seeing odd gamma tables with that.

  • If booting to console, gamma is disabled.
  • Load X and it sets a 1:1 gamma LUT.
  • Quit X and it leaves gamma at whatever it was before.
  • drm/build/tests/proptest/proptest -M vc4 76 crtc 28 0 and drm/build/tests/proptest/proptest -M vc4 83 crtc 28 0 to clear the gamma tables, and we're back to 1:1 with the gamma block disabled.

For Pi0-3, the vc4 docs are not 100% clear whether it is legitimate to enable/disable gamma whilst active, but it seems to behave itself even if you do, so I'm happy enough with the patches as they stand.

@6by9
drm/vc4: Validate the size of the gamma_lut
1cabaa2 
Add a check to vc4_hvs_gamma_check to ensure a new non-empty
gamma LUT is of the correct length before accepting it.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
@6by9
Copy link
Copy Markdown
Contributor Author

6by9 commented Nov 10, 2021

Added another patch that validates the gamma LUT blob is of the correct length as I don't believe the framework does it for us. (Rockchip is certainly validating it within the driver).

@popcornmix
Copy link
Copy Markdown
Collaborator

popcornmix commented Nov 10, 2021

I'm still getting bright hdmi0 (until 30 minutes).

HVS channel 0:
  red:
  00000000 00000c23 - x 0, c 0, grad 3107
  01500fee 000006d7 - x 21, c 4078, grad 1751
  03a01fbf 0000042f - x 58, c 8127, grad 1071
  07802ff6 000002e8 - x 120, c 12278, grad 744
  0d003ff0 0000022e - x 208, c 16368, grad 558
  14504fe2 000001b8 - x 325, c 20450, grad 440
  1db05ffb 00000166 - x 475, c 24571, grad 358
  29206ffd 0000012c - x 658, c 28669, grad 300
  36c07ff4 00000100 - x 876, c 32756, grad 256
  46c08ff7 000000de - x 1132, c 36855, grad 222
  59309ffa 000000c3 - x 1427, c 40954, grad 195
  6e20aff6 000000ae - x 1762, c 45046, grad 174
  85c0bffd 0000009c - x 2140, c 49149, grad 156
  a000cff7 0000008d - x 2560, c 53239, grad 141
  bd20dffd 00000080 - x 3026, c 57341, grad 128
  dd10effb 00000075 - x 3537, c 61435, grad 117
  green:
  00000000 00000c23 - x 0, c 0, grad 3107
  01500fee 000006d7 - x 21, c 4078, grad 1751
  03a01fbf 0000042f - x 58, c 8127, grad 1071
  07802ff6 000002e8 - x 120, c 12278, grad 744
  0d003ff0 0000022e - x 208, c 16368, grad 558
  14504fe2 000001b8 - x 325, c 20450, grad 440
  1db05ffb 00000166 - x 475, c 24571, grad 358
  29206ffd 0000012c - x 658, c 28669, grad 300
  36c07ff4 00000100 - x 876, c 32756, grad 256
  46c08ff7 000000de - x 1132, c 36855, grad 222
  59309ffa 000000c3 - x 1427, c 40954, grad 195
  6e20aff6 000000ae - x 1762, c 45046, grad 174
  85c0bffd 0000009c - x 2140, c 49149, grad 156
  a000cff7 0000008d - x 2560, c 53239, grad 141
  bd20dffd 00000080 - x 3026, c 57341, grad 128
  dd10effb 00000075 - x 3537, c 61435, grad 117
  blue:
  00000000 00000c23 - x 0, c 0, grad 3107
  01500fee 000006d7 - x 21, c 4078, grad 1751
  03a01fbf 0000042f - x 58, c 8127, grad 1071
  07802ff6 000002e8 - x 120, c 12278, grad 744
  0d003ff0 0000022e - x 208, c 16368, grad 558
  14504fe2 000001b8 - x 325, c 20450, grad 440
  1db05ffb 00000166 - x 475, c 24571, grad 358
  29206ffd 0000012c - x 658, c 28669, grad 300
  36c07ff4 00000100 - x 876, c 32756, grad 256
  46c08ff7 000000de - x 1132, c 36855, grad 222
  59309ffa 000000c3 - x 1427, c 40954, grad 195
  6e20aff6 000000ae - x 1762, c 45046, grad 174
  85c0bffd 0000009c - x 2140, c 49149, grad 156
  a000cff7 0000008d - x 2560, c 53239, grad 141
  bd20dffd 00000080 - x 3026, c 57341, grad 128
  dd10effb 00000075 - x 3537, c 61435, grad 117
HVS channel 1:
  red:
  00000000 00000100 - x 0, c 0, grad 256
  10001000 00000100 - x 256, c 4096, grad 256
  20002000 00000100 - x 512, c 8192, grad 256
  30003000 00000100 - x 768, c 12288, grad 256
  40004000 00000100 - x 1024, c 16384, grad 256
  50005000 00000104 - x 1280, c 20480, grad 260
  60006040 00000100 - x 1536, c 24640, grad 256
  70007040 00000100 - x 1792, c 28736, grad 256
  80008040 00000100 - x 2048, c 32832, grad 256
  90009040 00000100 - x 2304, c 36928, grad 256
  a000a040 00000104 - x 2560, c 41024, grad 260
  b000b080 00000100 - x 2816, c 45184, grad 256
  c000c080 00000100 - x 3072, c 49280, grad 256
  d000d080 00000100 - x 3328, c 53376, grad 256
  e000e080 00000100 - x 3584, c 57472, grad 256
  f000f080 00000104 - x 3840, c 61568, grad 260
  green:
  00000000 00000100 - x 0, c 0, grad 256
  10001000 00000100 - x 256, c 4096, grad 256
  20002000 00000100 - x 512, c 8192, grad 256
  30003000 00000100 - x 768, c 12288, grad 256
  40004000 00000100 - x 1024, c 16384, grad 256
  50005000 00000104 - x 1280, c 20480, grad 260
  60006040 00000100 - x 1536, c 24640, grad 256
  70007040 00000100 - x 1792, c 28736, grad 256
  80008040 00000100 - x 2048, c 32832, grad 256
  90009040 00000100 - x 2304, c 36928, grad 256
  a000a040 00000104 - x 2560, c 41024, grad 260
  b000b080 00000100 - x 2816, c 45184, grad 256
  c000c080 00000100 - x 3072, c 49280, grad 256
  d000d080 00000100 - x 3328, c 53376, grad 256
  e000e080 00000100 - x 3584, c 57472, grad 256
  f000f080 00000104 - x 3840, c 61568, grad 260
  blue:
  00000000 00000100 - x 0, c 0, grad 256
  10001000 00000100 - x 256, c 4096, grad 256
  20002000 00000100 - x 512, c 8192, grad 256
  30003000 00000100 - x 768, c 12288, grad 256
  40004000 00000100 - x 1024, c 16384, grad 256
  50005000 00000104 - x 1280, c 20480, grad 260
  60006040 00000100 - x 1536, c 24640, grad 256
  70007040 00000100 - x 1792, c 28736, grad 256
  80008040 00000100 - x 2048, c 32832, grad 256
  90009040 00000100 - x 2304, c 36928, grad 256
  a000a040 00000104 - x 2560, c 41024, grad 260
  b000b080 00000100 - x 2816, c 45184, grad 256
  c000c080 00000100 - x 3072, c 49280, grad 256
  d000d080 00000100 - x 3328, c 53376, grad 256
  e000e080 00000100 - x 3584, c 57472, grad 256
  f000f080 00000104 - x 3840, c 61568, grad 260

@6by9
Copy link
Copy Markdown
Contributor Author

6by9 commented Nov 10, 2021

I'm still getting bright hdmi0 (until 30 minutes).

Please confirm details to reproduce? Bullseye or Buster? Presumably X from your earlier description. Booting direct to X or console?

@popcornmix
Copy link
Copy Markdown
Collaborator

popcornmix commented Nov 10, 2021

It's bullseye. Set to boot to desktop with auto login. Two 4kp30 hdmi displays.

@6by9 6by9 mentioned this pull request Nov 15, 2021
Closed
@pelwell pelwell merged commit d6bbe47 into raspberrypi:rpi-5.10.y Nov 16, 2021
@6by9
Copy link
Copy Markdown
Contributor Author

6by9 commented Nov 16, 2021

The patches appear to have been pushed manually by popcornmix last night, so the merge of this PR actually did nothing. However the changes are in there now.

@6by9 6by9 mentioned this pull request Nov 29, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@6by9 @popcornmix @pelwell