From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10on2062a.outbound.protection.outlook.com [IPv6:2a01:111:f400:7e88::62a]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1976F10E68F for ; Thu, 17 Nov 2022 21:35:23 +0000 (UTC) Content-Type: multipart/alternative; boundary="------------NbdiHqaPLBR1040Mx0px05Yj" Message-ID: Date: Thu, 17 Nov 2022 14:35:17 -0700 From: Alex Hung To: aemad , "igt-dev@lists.freedesktop.org" References: <35b8b7adb1e5671c2c1ae43ba7ce4774@igalia.com> Content-Language: en-US In-Reply-To: <35b8b7adb1e5671c2c1ae43ba7ce4774@igalia.com> MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH RFC i-g-t 0/8] tests/kms_universal_plane: divide `functional_test_pipe` to mini-subtests List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "petri.latvala@intel.com" Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: --------------NbdiHqaPLBR1040Mx0px05Yj Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Aemad, Thanks for breaking the test into smaller subtests. Some GPU drivers, including amdgpu, do not allow primary plane off without turning crtc off (https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c#L385), but kms_universal_plane has many the below code which was guarded with is_amdgpu_device()         igt_plane_set_fb(primary, NULL);         igt_display_commit2(&data->display, COMMIT_UNIVERSAL); Many of the is_amdgpu_device() are not present in your patches and kms_universal fails. I needed to add is_amdgpu_device() to patch 2~3 and 5~7 (I didn't check 8). ------------------------------------------------------------------------ *From:* aemad *Sent:* 11 November 2022 01:31 *To:* igt-dev@lists.freedesktop.org *Cc:* petri.latvala@intel.com ; bhanuprakash.modem@intel.com ; ville.syrjala@linux.intel.com ; Siqueira, Rodrigo ; Hung, Alex ; markyacoub@chromium.org ; mwen@igalia.com ; andrealmeid@igalia.com *Subject:* Re: [PATCH RFC i-g-t 0/8] tests/kms_universal_plane: divide `functional_test_pipe` to mini-subtests Please, let me know your thoughts. On 2022-11-11 09:29, Alaa Emad wrote: > Divide the `functional_test_pipe` test into seven subtests based on CRC > comparisons because this will make it easier to debug the test and help > in detecting the failure. > > First 7 patches decouple each subtest and run it individually keeping > `functional_test_pipe' test as it is. After making sure that each > subtest can run individually with the expected result on both vkms and i915 > drivers, improve the test by creating `run_functional_test_pipe` and > call all subtests from it and call `run_functional_test_pipe` in > `igt_main`. > > > Alaa Emad (8): >   tests/kms_universal_plane: decouple verification of legacy and atomic >     api >   tests/kms_universal_plane: decouple verification of disabling primary >     plane >   tests/kms_universal_plane: decouple verification of re-enabling >     primary plane >   tests/kms_universal_plane: decouple verification of setup plane FB's >     while CRTC is disabled >   tests/kms_universal_plane: decouple verification of ablity to modeset >     with the primary plane off >   tests/kms_universal_plane: decouple verification of ablity to move the >     primary plane completely offscreen >   tests/kms_universal_plane: decouple verification of ablity to >     explicitly disable an already implicitly-disabled primary plane >   tests/kms_universal_plane: create the run_functional_test_pipe and >     call all tests from it > >  tests/kms_universal_plane.c | 416 ++++++++++++++++++++++-------------- >  1 file changed, 261 insertions(+), 155 deletions(-) --------------NbdiHqaPLBR1040Mx0px05Yj Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi Aemad,

Thanks for breaking the test into smaller subtests.

Some GPU drivers, including amdgpu, do not allow primary plane off without turning crtc off (https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c#L385), but kms_universal_plane has many the below code which was guarded with is_amdgpu_device()

        igt_plane_set_fb(primary, NULL);
        igt_display_commit2(&data->display, COMMIT_UNIVERSAL);

Many of the is_amdgpu_device() are not present in your patches and kms_universal fails. I needed to add is_amdgpu_device() to patch 2~3 and 5~7 (I didn't check 8).


Please, let me know your thoughts.

On 2022-11-11 09:29, Alaa Emad wrote:
> Divide the `functional_test_pipe` test into seven subtests based on CRC
> comparisons because this will make it easier to debug the test and help
> in detecting the failure.
>
> First 7 patches decouple each subtest and run it individually keeping
> `functional_test_pipe' test as it is. After making sure that each
> subtest can run individually with the expected result on both vkms and i915
> drivers, improve the test by creating `run_functional_test_pipe` and
> call all subtests from it and call `run_functional_test_pipe` in
> `igt_main`.
>
>
> Alaa Emad (8):
>   tests/kms_universal_plane: decouple verification of legacy and atomic
>     api
>   tests/kms_universal_plane: decouple verification of disabling primary
>     plane
>   tests/kms_universal_plane: decouple verification of re-enabling
>     primary plane
>   tests/kms_universal_plane: decouple verification of setup plane FB's
>     while CRTC is disabled
>   tests/kms_universal_plane: decouple verification of ablity to modeset
>     with the primary plane off
>   tests/kms_universal_plane: decouple verification of ablity to move the
>     primary plane completely offscreen
>   tests/kms_universal_plane: decouple verification of ablity to
>     explicitly disable an already implicitly-disabled primary plane
>   tests/kms_universal_plane: create the run_functional_test_pipe and
>     call all tests from it
>
>  tests/kms_universal_plane.c | 416 ++++++++++++++++++++++--------------
>  1 file changed, 261 insertions(+), 155 deletions(-)
--------------NbdiHqaPLBR1040Mx0px05Yj--