Discussion:
[PATCH] drm/i915: Fix IGT kms_color/gamma subtest SKIP for GLK
Swati Sharma
2018-12-06 09:18:36 UTC
Permalink
Fix the skip for kms_color/gamma subtest

Test requirement not met in function run_tests_for_pipe, file kms_color.c:858:
Test requirement: igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)
Subtest pipe-A-gamma: SKIP
Test requirement not met in function run_tests_for_pipe, file kms_color.c:858:
Test requirement: igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)
Test requirement not met in function run_tests_for_pipe, file kms_color.c:858:
Test requirement: igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)
Test requirement not met in function run_tests_for_pipe, file kms_color.c:847:
Test requirement: p < data->display.n_pipes
Test requirement not met in function run_tests_for_pipe, file kms_color.c:847:
Test requirement: p < data->display.n_pipes
Test requirement not met in function run_tests_for_pipe, file kms_color.c:847:
Test requirement: p < data->display.n_pipes

[Why]
degamma_lut_size assigned 0
[How]
degamma_lut_size should be 35
BSpec:18433
Testcase:igt/kms_color/pipe-A-gamma

Signed-off-by: Swati Sharma <***@intel.com>
---
drivers/gpu/drm/i915/i915_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 6350db5..a1e900b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -69,7 +69,7 @@
#define CHV_COLORS \
.color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }
#define GLK_COLORS \
- .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
+ .color = { .degamma_lut_size = 35, .gamma_lut_size = 1024 }

/* Keep in gen based order, and chronological order within a gen */
--
1.9.1
Matt Roper
2018-12-07 00:16:16 UTC
Permalink
Post by Swati Sharma
Fix the skip for kms_color/gamma subtest
Test requirement: igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)
Subtest pipe-A-gamma: SKIP
Test requirement: igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)
Test requirement: igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)
Test requirement: p < data->display.n_pipes
Test requirement: p < data->display.n_pipes
Test requirement: p < data->display.n_pipes
[Why]
degamma_lut_size assigned 0
[How]
degamma_lut_size should be 35
BSpec:18433
Testcase:igt/kms_color/pipe-A-gamma
---
drivers/gpu/drm/i915/i915_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 6350db5..a1e900b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -69,7 +69,7 @@
#define CHV_COLORS \
.color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }
#define GLK_COLORS \
- .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
+ .color = { .degamma_lut_size = 35, .gamma_lut_size = 1024 }
Although the hardware has a degamma table, I believe leaving it disabled
for now was intended by the original author of that code since the
hardware degamma table doesn't work the same way as our other platforms
and doesn't exactly align with our driver ABI. I.e., GLK (and gen11 now
as well) just take a single value that it uses for the red, green, and
blue channels, whereas userspace expects to be able to upload a table
with different values for each.

I think to enable degamma we need to do the following:
* Add some checks that get run during the atomic check phase that test
to see whether r=g=b for each LUT entry. If userspace tried to
upload a table that we can't express to our hardware, we need to
reject the atomic transaction. While we're adding this function, we
should also probably check that other hardware requirements are
satisfied, such as that the table entries are always flat/increasing,
never decreasing.
* Update glk_load_degamma_lut() to actually make use of the
userspace-provided table. Right now that function is simply slamming
a linear table into the hardware rather than using whatever would
be uploaded from userspace via property blob.
* Finally, expose degamma to userspace by setting the table size as
you've done in this patch.

Also, I believe 35 entries isn't exactly right for GLK. While the
hardware table does have 35 slots, only the first 33 represent the usual
0-1.0 gamma range that our current driver ABI is exposing. Slots #34
and #35 are for "extended range" values which we don't currently have a
way of expressing through our ABI (we need to be able to express
non-uniform ranges and also values outside the usual 0-1.0 range). I
believe Uma is working on coming up with a way to expose this to
userspace, but it isn't ready just yet, so I think you'd want to just
set .degamma_lut_size to 33 for now and put a hardcoded 1.0 value into
the last two entries.


Matt
Post by Swati Sharma
/* Keep in gen based order, and chronological order within a gen */
--
1.9.1
_______________________________________________
dri-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
Loading...