drm/i915: Fix IGT kms_color/gamma subtest SKIP for GLK

Submitted by Swati Sharma on Dec. 6, 2018, 9:18 a.m.

Details

Message ID 1544087916-13906-1-git-send-email-swati2.sharma@intel.com
State New
Series "drm/i915: Fix IGT kms_color/gamma subtest SKIP for GLK"
Headers show

Commit Message

Swati Sharma Dec. 6, 2018, 9:18 a.m.
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 <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

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 */
 

Comments

Matt Roper Dec. 7, 2018, 12:16 a.m.
On Thu, Dec 06, 2018 at 02:48:36PM +0530, Swati Sharma wrote:
> 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 <swati2.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 }

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

>  
>  /* Keep in gen based order, and chronological order within a gen */
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel