<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;" align="Left">
[AMD Official Use Only]<br>
</p>
<br>
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="appendonsend"></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Mark Yacoub <markyacoub@chromium.org><br>
<b>Sent:</b> 13 April 2022 13:56<br>
<b>To:</b> Hung, Alex <Alex.Hung@amd.com><br>
<b>Cc:</b> igt-dev@lists.freedesktop.org <igt-dev@lists.freedesktop.org>; markyacoub@google.com <markyacoub@google.com>; bas@basnieuwenhuizen.nl <bas@basnieuwenhuizen.nl><br>
<b>Subject:</b> Re: [igt-dev] [PATCH i-g-t][V2] tests/kms: Skip kms test cases for DCC and DCC_RETILE</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">On Thu, Apr 7, 2022 at 1:42 PM Alex Hung <alex.hung@amd.com> wrote:<br>
title of commit message should include the test name if you're fixing<br>
only 1 text. so it would be tests/kms_plane<br>
><br>
> Skip the KMS test cases for planes that has modifiers with<br>
nit: s/has/have.<br>
> DCC and DCC_RETILE on AMDGPU.<br>
><br>
> Current pixel-format and pixel-format-source-clamping subtests do not<br>
> support modifers with DCC or DCC_RETILE in kernel.<br>
nit: s/modifers/modifiers<br>
><br>
> 1. dcc_formats's cpp[1] is set to 0 and this triggers kernel errors<br>
is [1] referring to a url? i dont see it.<br>
> "Format requires non-linear modifier for plane 1" because block_size<br>
> (i.e. cpp[1]) == 0. See kernel commits 816853f9dc40 and 8db2dc852941.<br>
I would prefer you add links to make it easy to look up.<br>
><br>
> 2. the subtests cause kernel's amdgpu_display_verify_sizes to fail<br>
> because they do not provide an extra plane with compression metadata.<br>
> See kernel commit 234055fd9728 for details.<br>
ditto<br>
<br>
if this is affecting a specific subtest, please add it here.<br>
><br>
> Signed-off-by: Alex Hung <alex.hung@amd.com><br>
> ---<br>
><br>
> v2: update comments in commit messages<br>
><br>
> tests/kms_plane.c | 8 ++++++++<br>
> 1 file changed, 8 insertions(+)<br>
><br>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c<br>
> index 137f23a8..1a71822a 100644<br>
> --- a/tests/kms_plane.c<br>
> +++ b/tests/kms_plane.c<br>
> @@ -1004,6 +1004,14 @@ static bool skip_plane(data_t *data, igt_plane_t *plane)<br>
> {<br>
> int index = plane->index;<br>
><br>
> + for (int i = 0; i < plane->format_mod_count; i++) {<br>
not sure how it compiled, i think you'll need to move int i to above.</div>
<div class="PlainText"><br>
</div>
<div class="PlainText"><span style="background-color:rgb(255, 255, 255);display:inline !important">igt has many "for (int i ...)" so I use the same approach just for neatness. I will move it to above in V3.</span><br>
</div>
<div class="PlainText"><br>
> + if (AMD_FMT_MOD_GET(DCC, plane->modifiers[i]) ||<br>
> + AMD_FMT_MOD_GET(DCC_RETILE, plane->modifiers[i])) {<br>
> + igt_debug("Skipping planes with DCC or DCC_RETILE\n");<br>
> + return true;<br>
> + }<br>
> + }<br>
> +<br>
> if (data->extended)<br>
> return false;<br>
><br>
> --<br>
> 2.35.1<br>
><br>
</div>
</span></font></div>
</div>
</body>
</html>