[igt-dev] [PATCH v4 1/2] lib/kms: Add DSC_SLICE_HEIGHT to CRTC property

[Vudum, Lakshminarayana]

https://patchwork.freedesktop.org/series/97470/#rev2
Possible regressions
igt at sysfs_heartbeat_interval@precise at vecs0:
shard-apl: PASS -> FAIL

Regression from rev2 is same as https://gitlab.freedesktop.org/drm/intel/-/issues/1755
igt at sysfs_timeslice_duration@timeout at .*| igt at sysfs_heartbeat_interval@precise at vecs0 - fail - Failed assertion: wait_for_reset(spin[0]->out_fence) == 0, error: -62 != 0, Resetting vecs0 for preemption time out

I can re-report the rev2 results.

Lakshmi.

-----Original Message-----
From: Rodrigo Siqueira Jordao <rjordrigo at amd.com> 
Sent: Wednesday, December 8, 2021 5:48 PM
To: Latvala, Petri <petri.latvala at intel.com>
Cc: Vudum, Lakshminarayana <lakshminarayana.vudum at intel.com>; igt-dev at lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH v4 1/2] lib/kms: Add DSC_SLICE_HEIGHT to CRTC property



On 2021-12-07 11:43 a.m., Rodrigo Siqueira wrote:
> This preparation work introduces a new CRTC property named 
> DSC_SLICE_HEIGHT, which will be required for amdgpu DSC tests.
> 
> Cc: Petri Latvala <petri.latvala at intel.com>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> ---
>   lib/igt_kms.c | 1 +
>   lib/igt_kms.h | 1 +
>   2 files changed, 2 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 34a2aa00..fdadb6d6 
> 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -593,6 +593,7 @@ const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>   	[IGT_CRTC_ACTIVE] = "ACTIVE",
>   	[IGT_CRTC_OUT_FENCE_PTR] = "OUT_FENCE_PTR",
>   	[IGT_CRTC_VRR_ENABLED] = "VRR_ENABLED",
> +	[IGT_CRTC_DSC_SLICE_HEIGHT] = "DSC_SLICE_HEIGHT",
>   };
>   
>   const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] 
> = { diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 
> e9ecd21e..5c7d7481 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -125,6 +125,7 @@ enum igt_atomic_crtc_properties {
>          IGT_CRTC_ACTIVE,
>          IGT_CRTC_OUT_FENCE_PTR,
>          IGT_CRTC_VRR_ENABLED,
> +       IGT_CRTC_DSC_SLICE_HEIGHT,
>          IGT_NUM_CRTC_PROPS
>   };
>   
> 

Hi Petri,

I followed your advice to search for IGT_NUM_CRTC_PROPS, and only igt_kms.{h,c} and kms_atomic uses it.

In the lib/igt_kms.c, the occurrences of IGT_NUM_CRTC_PROPS appears in the function igt_fill_pipe_props() as a parameter, in its turn, it call drmModeObjectGetProperties and drmModeGetProperty. Finally, it only populates the prop array based on the returned value from the driver. I suppose we are safe here; I don't see how this patch could regress something in this function. The other places where this function appears are in the loop condition, but all of them look correct to me.

Finally, the other place we can see it is in the kms_atomic, but again the for loop looks correct.

However, IGT CI failed:

https://patchwork.freedesktop.org/series/97470/#rev2

The potential new issue pointed by the CI is:

  Possible new issues
  Here are the unknown changes that may have been introduced in
IGTPW_6475_full:

  IGT changes
  Possible regressions
  igt at sysfs_heartbeat_interval@precise at vecs0:
  shard-apl: PASS -> FAIL

And the log says:

Stack trace:
   #0 ../lib/igt_core.c:1745 __igt_fail_assert()
   #1 ../tests/i915/sysfs_heartbeat_interval.c:156 __test_timeout()
   #2 [<unknown>+0xd0]
Dynamic subtest vecs0: FAIL (1.713s)

After I resubmitted this patch, I also noticed that I got a different CI error from the one from the V1. The reported error looks a little bit inconsistent.

Petri, do you think this can be a false-positive, or am I missing something? Maybe re-trigger IGT.CI can help?

Thanks