From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Ekstrand Subject: Re: [PATCH] tests/kms_ccs: Fix the color/ccs surface generation Date: Fri, 4 Aug 2017 09:06:07 -0700 Message-ID: References: <1501807961-23186-1-git-send-email-jason.ekstrand@intel.com> <15dadbf22f8.277a.c6988b7ea6112e3e892765a0d4287e0c@jlekstrand.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0439418453==" Return-path: Received: from mail-wm0-x22c.google.com (mail-wm0-x22c.google.com [IPv6:2a00:1450:400c:c09::22c]) by gabe.freedesktop.org (Postfix) with ESMTPS id D5EA36E414 for ; Fri, 4 Aug 2017 16:06:09 +0000 (UTC) Received: by mail-wm0-x22c.google.com with SMTP id m85so23607455wma.1 for ; Fri, 04 Aug 2017 09:06:09 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Stone Cc: Jason Ekstrand , Daniel Vetter , intel-gfx , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org --===============0439418453== Content-Type: multipart/alternative; boundary="94eb2c127d6eced7b30555efac2d" --94eb2c127d6eced7b30555efac2d Content-Type: text/plain; charset="UTF-8" On Fri, Aug 4, 2017 at 8:42 AM, Daniel Stone wrote: > On 4 August 2017 at 15:56, Jason Ekstrand wrote: > > On August 4, 2017 2:59:56 AM Daniel Stone wrote: > >>> + width = ALIGN(f.width * 4, 32) / 32; > >>> + height = ALIGN(f.height, 16) / 16; > >>> + f.pitches[1] = ALIGN(width * 1, 128); > >>> f.modifier[1] = modifier; > >>> f.offsets[1] = size[0]; > >>> - size[1] = f.pitches[1] * ALIGN(height, 64); > >>> + size[1] = f.pitches[1] * ALIGN(height, 32); > >> > >> > >> I changed this to f.height rather than height, because otherwise the > >> kernel was rejecting the aux buffer for being too small. > > > > Congratulations, you found a bug in the kernel branch you're running. > The > > downsized height is definitely what we want and it works fine with my > kernel > > branch. > > Great. Which kernel are you running then? I'm running from here: > https://git.collabora.com/cgit/user/daniels/linux.git/refs/heads > I'm working from some branch I got from Ville a couple months ago. > Currently we have hsub/vsub defined as 16/8 (Vidya inverted this, but > I never got a clear answer on why), Here's my comment in the IGT test: /* From the Sky Lake PRM, Vol 12, "Color Control Surface": * * "The compression state of the cache-line pair is * specified by 2 bits in the CCS. Each CCS cache-line * represents an area on the main surface of 16x16 sets * of 128 byte Y-tiled cache-line-pairs. CCS is always Y * tiled." * * A "cache-line-pair" for a Y-tiled surface is two * horizontally adjacent cache lines. When operating in * bytes and rows, this gives us a scale-down factor of * 32x16. Since the main surface has a 32-bit format, we * need to multiply width by 4 to get bytes. */ We have a scaling factor, in bytes, of 32x16. However, the main surface uses 4 byes per pixel so we need to account for that. In the IGT test, we multiply the width of the main surface by 4 to get bytes. Alternatively, you can adjust the scale factor to 8x16 so long as you align things correctly. tile_width as 128, and tile_height > comes out as 32. Yes, that's a correct Y-tile. > Given the calculations in intel_fill_fb_info, I come > out with the kernel demanding either 34816 bytes for CCS (using 16/8 > hsub/vsub), or 20480 bytes (8/16) for a 1920x1080 framebuffer. Neither of those look right. I'm getting 6 pages, or 24576B when I run the test which should be correct. > Which > kernel do you have, and how are you coming out with that calculation? > Do we need to go back and re-figure out what pitch is? > > FWIW, ISL seems to get it right, according to the kernel. > Weird... --94eb2c127d6eced7b30555efac2d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On F= ri, Aug 4, 2017 at 8:42 AM, Daniel Stone <daniel@fooishbar.org><= /span> wrote:
On 4 August 2017 at 15:56, Jason Ekstrand <jason@jlekstrand.net> wrote:
> On August 4, 2017 2:59:56 AM Daniel Stone <daniel@fooishbar.org> wrote:
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0width =3D ALIGN(f.width * 4, 32) / 32;
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0height= =3D ALIGN(f.height, 16) / 16;
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0f.pitc= hes[1] =3D ALIGN(width * 1, 128);
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0f= .modifier[1] =3D modifier;
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0f= .offsets[1] =3D size[0];
>>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0size[1= ] =3D f.pitches[1] * ALIGN(height, 64);
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0size[1= ] =3D f.pitches[1] * ALIGN(height, 32);
>>
>>
>> I changed this to f.height rather than height, because otherwise t= he
>> kernel was rejecting the aux buffer for being too small.
>
> Congratulations, you found a bug in the kernel branch you're runni= ng.=C2=A0 The
> downsized height is definitely what we want and it works fine with my = kernel
> branch.

Great. Which kernel are you running then? I'm running from here:=
https://git.collabora.com/cgit/= user/daniels/linux.git/refs/heads

<= div>I'm working from some branch I got from Ville a couple months ago.<= br>
=C2=A0
Currently we have hsub/vsub defined as 16/8 (Vidya inverted this, but
I never got a clear answer on why),
=C2=A0
Here's = my comment in the IGT test:

=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 /*= From the Sky Lake PRM, Vol 12, "Color Control Surface":
=C2= =A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0*
=C2=A0=C2=A0=C2=A0 =C2=A0=C2= =A0=C2=A0 =C2=A0*=C2=A0=C2=A0=C2=A0 "The compression state of the cach= e-line pair is
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0*=C2=A0=C2=A0= =C2=A0 specified by 2 bits in the CCS.=C2=A0 Each CCS cache-line
=C2=A0= =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0*=C2=A0=C2=A0=C2=A0 represents an are= a on the main surface of 16x16 sets
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2= =A0 =C2=A0*=C2=A0=C2=A0=C2=A0 of 128 byte Y-tiled cache-line-pairs. CCS is = always Y
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0*=C2=A0=C2=A0=C2=A0= tiled."
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0*
=C2=A0=C2= =A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0* A "cache-line-pair" for a Y-= tiled surface is two
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0* horiz= ontally adjacent cache lines.=C2=A0 When operating in
=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 =C2=A0* bytes and rows, this gives us a scale-down fact= or of
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0* 32x16.=C2=A0 Since t= he main surface has a 32-bit format, we
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0= =C2=A0 =C2=A0* need to multiply width by 4 to get bytes.
=C2=A0=C2=A0=C2= =A0 =C2=A0=C2=A0=C2=A0 =C2=A0*/

We have a scaling factor, in bytes, of 32x16.=C2=A0 However, the main surface= =20 uses 4 byes per pixel so we need to account for that.=C2=A0 In the IGT test= ,=20 we multiply the width of the main surface by 4 to get bytes.=C2=A0=20 Alternatively, you can adjust the scale factor to 8x16 so long as you=20 align things correctly.

tile_width as 128, and tile_height
comes out as 32.

Yes, that= 's a correct Y-tile.
=C2=A0
Given the calculation= s in intel_fill_fb_info, I come
out with the kernel demanding either 34816 bytes for CCS (using 16/8
hsub/vsub), or 20480 bytes (8/16) for a 1920x1080 framebuffer.
=

Neither of those look right.=C2=A0 I'm getting 6 pa= ges, or 24576B when I run the test which should be correct.
= =C2=A0
Which
kernel do you have, and how are you coming out with that calculation?
Do we need to go back and re-figure out what pitch is?

FWIW, ISL seems to get it right, according to the kernel.
<= div>
Weird...
--94eb2c127d6eced7b30555efac2d-- --===============0439418453== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============0439418453==--