[PATCH i-g-t] tests/intel/kms_pm_rpm: Update modifier to linear for display version 30+

[Juha-Pekka Heikkilä]

On Thu, Jan 23, 2025 at 5:03 PM Thasleem, Mohammed
<mohammed.thasleem at intel.com> wrote:
>
> Hi,
>
> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Jeevan B
> Sent: Thursday, January 23, 2025 2:46 PM
> To: igt-dev at lists.freedesktop.org
> Cc: Heikkila, Juha-pekka <juha-pekka.heikkila at intel.com>; B, Jeevan <jeevan.b at intel.com>
> Subject: [PATCH i-g-t] tests/intel/kms_pm_rpm: Update modifier to linear for display version 30+
>
> On latest platform tile-X is no longer supported, So for display version 30+ we can use linear tiling and tile-X for older platform.
> This change accounts for the limitations of gem_set_tiling, which no longer works on recent platforms and will not be supported on Xe drivers.
>
> Signed-off-by: Jeevan B <jeevan.b at intel.com>
> ---
>  tests/intel/kms_pm_rpm.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tests/intel/kms_pm_rpm.c b/tests/intel/kms_pm_rpm.c index 990c50915..0f4da426b 100644
> --- a/tests/intel/kms_pm_rpm.c
> +++ b/tests/intel/kms_pm_rpm.c
> @@ -1188,6 +1188,12 @@ static void cursor_subtest(bool dpms)
>         int rc;
>         struct igt_fb cursor_fb1, cursor_fb2, cursor_fb3;
>         uint32_t crtc_id;
> +       uint64_t modifier;
> +
> +       if (intel_display_ver(intel_get_drm_devid(drm_fd)) >= 30)
> +               modifier = DRM_FORMAT_MOD_LINEAR;
> +       else
> +               modifier = I915_FORMAT_MOD_X_TILED;
>
>         disable_all_screens_and_wait(&ms_data);
>
> @@ -1199,7 +1205,7 @@ static void cursor_subtest(bool dpms)
>         igt_create_fb(drm_fd, 64, 64, DRM_FORMAT_ARGB8888,
>                       DRM_FORMAT_MOD_LINEAR, &cursor_fb2);
>         igt_create_fb(drm_fd, 64, 64, DRM_FORMAT_XRGB8888,
> -                     I915_FORMAT_MOD_X_TILED, &cursor_fb3);
> +                     modifier, &cursor_fb3);
>  By adding this check for x tiled and liner it will check based on platform but...
> the successor code which uses the cursor_fb3..
> ...
> /* Cursor 3 was created with tiling and painted with a GTT mmap, so
>          * hopefully it has some fences around it. */
>         rc = drmModeRmFB(drm_fd, cursor_fb3.fb_id);
>         igt_assert_eq(rc, 0);
>
> If modifier was DRM_FORMAT_MOD_LINEAR then below code __gem_set_tiling call become no-op.
> As buffer already in linear layout operation below does nothing here.
> And if we add check to run on linear by skip blow line, we might miss essence of the test.
>       __gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false, cursor_fb3.strides[0]);
> @Heikkila, Juha-pekka/@Syrjala, Ville could you please share your thoughts on it?
> igt_assert(wait_for_suspended());

I'm bit double sided on this. This test has not been running correctly
on any recent platform since removal of set/get tiling and it was just
not spotted because use of __gem_set_tiling() instead of
gem_set_tiling(). As with discussion with Jeevan earlier this test
does show some semi-interesting excessive logging at least on lnl
platform with Xe driver .. but those get/set tiling anyway just return
einval on Xe and that tiling probably is the special thing on this
test. I'm thinking this test was originally from Ville(?), maybe Ville
remember if there was something more to this test?

/Juha-Pekka

> ......
>         fill_igt_fb(&cursor_fb1, 0xFF00FFFF);
>         fill_igt_fb(&cursor_fb2, 0xFF00FF00);
> --
> 2.25.1
>