[igt-dev] [i-g-t] tests/kms_cursor_crc: Add Gaurd for MSO eDP for Pipe C and D

[Bhadane, Dnyaneshwar]

Hello Kamil,
Thanks for the inputs.
I will keep note this comments and update this patch for the same.

From: Kamil Konieczny<mailto:kamil.konieczny at linux.intel.com>
Sent: Tuesday, January 3, 2023 8:20 PM
To: igt-dev at lists.freedesktop.org<mailto:igt-dev at lists.freedesktop.org>
Cc: Bhadane, Dnyaneshwar<mailto:dnyaneshwar.bhadane at intel.com>; Borah, Chaitanya Kumar<mailto:chaitanya.kumar.borah at intel.com>; Kurmi, Suresh Kumar<mailto:suresh.kumar.kurmi at intel.com>; Sharma, Swati2<mailto:swati2.sharma at intel.com>
Subject: Re: [igt-dev] [i-g-t] tests/kms_cursor_crc: Add Gaurd for MSO eDP for Pipe C and D

Hi,

On 2022-12-30 at 12:16:38 +0530, bhadanednyaneshwar wrote:
> MSO eDP is not supported on pipe C and D. Added a test condition
------------------------------------------- ^
> to prevent tests from execution on pipe C and D.This condition was
> missed for cursor-size-change,cursor-alpha-opaque and
> cursor-alpha-transparent testcases.

Please improve a little commit message, you just added skips for
not supported cursors in those subtests.

>
> Inside require_cursor_size() checks first for eligiblity to igt commit
> using test buffer.For MSO eDP, It is fail to commit for pipe C/pipe D
> and require_cursor_size() return non zero value. So it will skip
> the dynamic testcase for pipe C and D.

The above paragraph is spelling what you done. Will it only skip
for pipe C and D ? Or any pipe on which it cannot create cursor ?

You can add cc here:

Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>
Cc: Suresh Kumar Kurmi <suresh.kumar.kurmi at intel.com>
Cc: Swati Sharma <swati2.sharma at intel.com>
>
> Signed-off-by: bhadanednyaneshwar <dnyaneshwar.bhadane at intel.com>
---------------- ^
Please put here your name (look for example for Swati), I guess
it should be

Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane at intel.com>

but if I guess wrong please put correct one (it is best to put this
in global git config).

> ---
>  tests/kms_cursor_crc.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 17f294d6..d8fb9c0d 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -786,7 +786,12 @@ static void run_tests_on_pipe(data_t *data)
>        igt_subtest_with_dynamic("cursor-size-change") {
>                for_each_pipe(&data->display, pipe) {
>                        data->pipe = pipe;
> -
> +                     create_cursor_fb(data, data->cursor_max_w, data->cursor_max_h);
----------------------- ^
Do you really need this here ? It is used elsewhere within fixup
(there is assert in this function), in other tests below
require_cursor_size is called without create_cursor_fb

> +                     if (require_cursor_size(data, data->cursor_max_w, data->cursor_max_h)) {
> +                             igt_debug("Cursor size %dx%d not supported by driver\n",
> +                                       data->cursor_max_w, data->cursor_max_h);
> +                             continue;
> +                     }

Put newline here.

>                        igt_dynamic_f("pipe-%s-%s",
>                                      kmstest_pipe_name(pipe),
>                                      data->output->name)
> @@ -800,7 +805,12 @@ static void run_tests_on_pipe(data_t *data)
>        igt_subtest_with_dynamic("cursor-alpha-opaque") {
>                for_each_pipe(&data->display, pipe) {
>                        data->pipe = pipe;
> -
> +                     create_cursor_fb(data, data->cursor_max_w, data->cursor_max_h);
----------------------- ^
> +                     if (require_cursor_size(data, data->cursor_max_w, data->cursor_max_h)) {
> +                             igt_debug("Cursor size %dx%d not supported by driver\n",
> +                                       data->cursor_max_w, data->cursor_max_h);
> +                             continue;
> +                     }

Put newline here.

>                        igt_dynamic_f("pipe-%s-%s",
>                                      kmstest_pipe_name(pipe),
>                                      data->output->name)
> @@ -814,7 +824,12 @@ static void run_tests_on_pipe(data_t *data)
>        igt_subtest_with_dynamic("cursor-alpha-transparent") {
>                for_each_pipe(&data->display, pipe) {
>                        data->pipe = pipe;
> -
> +                     create_cursor_fb(data, data->cursor_max_w, data->cursor_max_h);
----------------------- ^
> +                     if (require_cursor_size(data, data->cursor_max_w, data->cursor_max_h)) {
> +                             igt_debug("Cursor size %dx%d not supported by driver\n",
> +                                       data->cursor_max_w, data->cursor_max_h);
> +                             continue;
> +                     }

Put newline here.

Regards,
Kamil

>                        igt_dynamic_f("pipe-%s-%s",
>                                      kmstest_pipe_name(pipe),
>                                      data->output->name)
> --
> 2.35.1
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20230105/d68668e4/attachment-0001.htm>