<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="#954F72" style="word-wrap:break-word"> <div class="WordSection1"> Hello Kamil, Thanks for the inputs. I will keep note this comments and update this patch for the same. <o:p> </o:p> <div style="mso-element:para-border-div;border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: <a href="mailto:kamil.konieczny@linux.intel.com">Kamil Konieczny</a> Sent: Tuesday, January 3, 2023 8:20 PM To: <a href="mailto:igt-dev@lists.freedesktop.org">igt-dev@lists.freedesktop.org</a> Cc: <a href="mailto:dnyaneshwar.bhadane@intel.com">Bhadane, Dnyaneshwar</a>; <a href="mailto:chaitanya.kumar.borah@intel.com">Borah, Chaitanya Kumar</a>; <a href="mailto:suresh.kumar.kurmi@intel.com"> Kurmi, Suresh Kumar</a>; <a href="mailto:swati2.sharma@intel.com">Sharma, Swati2</a> Subject: Re: [igt-dev] [i-g-t] tests/kms_cursor_crc: Add Gaurd for MSO eDP for Pipe C and D </div> <o:p> </o:p> 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@intel.com> Cc: Suresh Kumar Kurmi <suresh.kumar.kurmi@intel.com> Cc: Swati Sharma <swati2.sharma@intel.com> > > Signed-off-by: bhadanednyaneshwar <dnyaneshwar.bhadane@intel.com> ---------------- ^ Please put here your name (look for example for Swati), I guess it should be Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@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 > <o:p></o:p> <o:p> </o:p> </div> </body> </html>