| Message ID | 1310975703-20269-2-git-send-email-chad@chad-versace.us (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Montag, den 18.07.2011, 00:55 -0700 schrieb Chad Versace: > Until now, the stencil buffer was allocated as a Y tiled buffer, because > in several locations the PRM states that it is. However, it is actually > W tiled. From the PRM, 2011 Sandy Bridge, Volume 1, Part 2, Section > 4.5.2.1 W-Major Format: > W-Major Tile Format is used for separate stencil. > > The GTT is incapable of W fencing, so we allocate the stencil buffer with > I915_TILING_NONE and decode the tile's layout in software. > > This commit mutually depends on the mesa commit: > intel: Fix stencil buffer to be W tiled > Author: Chad Versace <chad@chad-versace.us> > Date: Mon Jul 18 00:37:45 2011 -0700 > > CC: Eric Anholt <eric@anholt.net> > CC: Kenneth Graunke <kenneth@whitecape.org> > Signed-off-by: Chad Versace <chad@chad-versace.us> > --- > src/intel_dri.c | 16 ++++++++++++---- > 1 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/src/intel_dri.c b/src/intel_dri.c > index 5ea7c2c..4652dc7 100644 > --- a/src/intel_dri.c > +++ b/src/intel_dri.c > @@ -336,7 +336,6 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, > switch (attachment) { > case DRI2BufferDepth: > case DRI2BufferDepthStencil: > - case DRI2BufferStencil: > case DRI2BufferHiz: > if (SUPPORTS_YTILING(intel)) { > hint |= INTEL_CREATE_PIXMAP_TILING_Y; > @@ -351,6 +350,14 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, > case DRI2BufferFrontRight: > hint |= INTEL_CREATE_PIXMAP_TILING_X; > break; > + case DRI2BufferStencil: > + /* > + * The stencil buffer is W tiled. However, we > + * request from the kernel a non-tiled buffer > + * because the GTT is incapable of W fencing. > + */ > + hint |= INTEL_CREATE_PIXMAP_TILING_NONE; > + break; > default: > free(privates); > free(buffer); > @@ -368,11 +375,12 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, > * To accomplish this, we resort to the nasty hack of doubling > * the drm region's cpp and halving its height. > * > - * If we neglect to double the pitch, then > - * drm_intel_gem_bo_map_gtt() maps the memory incorrectly. > + * If we neglect to double the pitch, then render corruption > + * occurs. The alignment does not seem to match. > */ > if (attachment == DRI2BufferStencil) { > - pixmap_height /= 2; > + pixmap_width = ALIGN(pixmap_width, 64); > + pixmap_height = ALIGN(pixmap_height / 2, 64); > pixmap_cpp *= 2; > } Thanks, Paul
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 07/18/2011 12:55 AM, Chad Versace wrote: > Until now, the stencil buffer was allocated as a Y tiled buffer, because > in several locations the PRM states that it is. However, it is actually > W tiled. From the PRM, 2011 Sandy Bridge, Volume 1, Part 2, Section > 4.5.2.1 W-Major Format: > W-Major Tile Format is used for separate stencil. > > The GTT is incapable of W fencing, so we allocate the stencil buffer with > I915_TILING_NONE and decode the tile's layout in software. > > This commit mutually depends on the mesa commit: > intel: Fix stencil buffer to be W tiled > Author: Chad Versace <chad@chad-versace.us> > Date: Mon Jul 18 00:37:45 2011 -0700 > > CC: Eric Anholt <eric@anholt.net> > CC: Kenneth Graunke <kenneth@whitecape.org> > Signed-off-by: Chad Versace <chad@chad-versace.us> > --- > src/intel_dri.c | 16 ++++++++++++---- > 1 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/src/intel_dri.c b/src/intel_dri.c > index 5ea7c2c..4652dc7 100644 > --- a/src/intel_dri.c > +++ b/src/intel_dri.c > @@ -336,7 +336,6 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, > switch (attachment) { > case DRI2BufferDepth: > case DRI2BufferDepthStencil: > - case DRI2BufferStencil: > case DRI2BufferHiz: > if (SUPPORTS_YTILING(intel)) { > hint |= INTEL_CREATE_PIXMAP_TILING_Y; > @@ -351,6 +350,14 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, > case DRI2BufferFrontRight: > hint |= INTEL_CREATE_PIXMAP_TILING_X; > break; > + case DRI2BufferStencil: > + /* > + * The stencil buffer is W tiled. However, we > + * request from the kernel a non-tiled buffer > + * because the GTT is incapable of W fencing. > + */ > + hint |= INTEL_CREATE_PIXMAP_TILING_NONE; > + break; > default: > free(privates); > free(buffer); Eh... it seems like this will break compatibility with older versions of Mesa. I haven't dug around, but there used to be a hack in DRI2 where a client would request a depth buffer and a stencil buffer, but it would get the same packed depth-stencil buffer for both. I guess that might all be up in the DRI2 layer and not in the driver... > @@ -368,11 +375,12 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, > * To accomplish this, we resort to the nasty hack of doubling > * the drm region's cpp and halving its height. > * > - * If we neglect to double the pitch, then > - * drm_intel_gem_bo_map_gtt() maps the memory incorrectly. > + * If we neglect to double the pitch, then render corruption > + * occurs. Mangled whitespace? Probably mixed tabs and spaces... > */ > if (attachment == DRI2BufferStencil) { > - pixmap_height /= 2; > + pixmap_width = ALIGN(pixmap_width, 64); > + pixmap_height = ALIGN(pixmap_height / 2, 64); > pixmap_cpp *= 2; > } > -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk4kf10ACgkQX1gOwKyEAw9jVACeOfy5BjD4Nb/YN3vyOoR5MOOv IOAAn20Lh7GAN7KSAkBFs/u5uaHq8/Sm =ICWC -----END PGP SIGNATURE-----
On 07/18/2011 11:45 AM, Ian Romanick wrote: > On 07/18/2011 12:55 AM, Chad Versace wrote: >> Until now, the stencil buffer was allocated as a Y tiled buffer, because >> in several locations the PRM states that it is. However, it is actually >> W tiled. From the PRM, 2011 Sandy Bridge, Volume 1, Part 2, Section >> 4.5.2.1 W-Major Format: >> W-Major Tile Format is used for separate stencil. > >> The GTT is incapable of W fencing, so we allocate the stencil buffer with >> I915_TILING_NONE and decode the tile's layout in software. > >> This commit mutually depends on the mesa commit: >> intel: Fix stencil buffer to be W tiled >> Author: Chad Versace <chad@chad-versace.us> >> Date: Mon Jul 18 00:37:45 2011 -0700 > >> CC: Eric Anholt <eric@anholt.net> >> CC: Kenneth Graunke <kenneth@whitecape.org> >> Signed-off-by: Chad Versace <chad@chad-versace.us> >> --- >> src/intel_dri.c | 16 ++++++++++++---- >> 1 files changed, 12 insertions(+), 4 deletions(-) > >> diff --git a/src/intel_dri.c b/src/intel_dri.c >> index 5ea7c2c..4652dc7 100644 >> --- a/src/intel_dri.c >> +++ b/src/intel_dri.c >> @@ -336,7 +336,6 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, >> switch (attachment) { >> case DRI2BufferDepth: >> case DRI2BufferDepthStencil: >> - case DRI2BufferStencil: >> case DRI2BufferHiz: >> if (SUPPORTS_YTILING(intel)) { >> hint |= INTEL_CREATE_PIXMAP_TILING_Y; >> @@ -351,6 +350,14 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, >> case DRI2BufferFrontRight: >> hint |= INTEL_CREATE_PIXMAP_TILING_X; >> break; >> + case DRI2BufferStencil: >> + /* >> + * The stencil buffer is W tiled. However, we >> + * request from the kernel a non-tiled buffer >> + * because the GTT is incapable of W fencing. >> + */ >> + hint |= INTEL_CREATE_PIXMAP_TILING_NONE; >> + break; >> default: >> free(privates); >> free(buffer); > > Eh... it seems like this will break compatibility with older versions of > Mesa. I haven't dug around, but there used to be a hack in DRI2 where a > client would request a depth buffer and a stencil buffer, but it would > get the same packed depth-stencil buffer for both. I guess that might > all be up in the DRI2 layer and not in the driver... The 'case DRI2BufferStencil' modified in this hunk was added by me when implementing separate stencil support. It was never used for this hack. That hack is implemented by an alternate definition of I830DRI2CreateBuffer() which is ifdef'd out when DRI2INFOREC_VERSION >= 2. FYI, the line that implements the hack can be found by grepping xf86-video-intel:src/intel_dri.c for } else if (attachments[i] == DRI2BufferStencil && pDepthPixmap) { > >> @@ -368,11 +375,12 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, >> * To accomplish this, we resort to the nasty hack of doubling >> * the drm region's cpp and halving its height. >> * >> - * If we neglect to double the pitch, then >> - * drm_intel_gem_bo_map_gtt() maps the memory incorrectly. >> + * If we neglect to double the pitch, then render corruption >> + * occurs. > > Mangled whitespace? Probably mixed tabs and spaces... Oops. Will fix. > >> */ >> if (attachment == DRI2BufferStencil) { >> - pixmap_height /= 2; >> + pixmap_width = ALIGN(pixmap_width, 64); >> + pixmap_height = ALIGN(pixmap_height / 2, 64); >> pixmap_cpp *= 2; >> }
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 07/18/2011 12:28 PM, Chad Versace wrote: > On 07/18/2011 11:45 AM, Ian Romanick wrote: >> On 07/18/2011 12:55 AM, Chad Versace wrote: >>> Until now, the stencil buffer was allocated as a Y tiled buffer, because >>> in several locations the PRM states that it is. However, it is actually >>> W tiled. From the PRM, 2011 Sandy Bridge, Volume 1, Part 2, Section >>> 4.5.2.1 W-Major Format: >>> W-Major Tile Format is used for separate stencil. >> >>> The GTT is incapable of W fencing, so we allocate the stencil buffer with >>> I915_TILING_NONE and decode the tile's layout in software. >> >>> This commit mutually depends on the mesa commit: >>> intel: Fix stencil buffer to be W tiled >>> Author: Chad Versace <chad@chad-versace.us> >>> Date: Mon Jul 18 00:37:45 2011 -0700 >> >>> CC: Eric Anholt <eric@anholt.net> >>> CC: Kenneth Graunke <kenneth@whitecape.org> >>> Signed-off-by: Chad Versace <chad@chad-versace.us> >>> --- >>> src/intel_dri.c | 16 ++++++++++++---- >>> 1 files changed, 12 insertions(+), 4 deletions(-) >> >>> diff --git a/src/intel_dri.c b/src/intel_dri.c >>> index 5ea7c2c..4652dc7 100644 >>> --- a/src/intel_dri.c >>> +++ b/src/intel_dri.c >>> @@ -336,7 +336,6 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, >>> switch (attachment) { >>> case DRI2BufferDepth: >>> case DRI2BufferDepthStencil: >>> - case DRI2BufferStencil: >>> case DRI2BufferHiz: >>> if (SUPPORTS_YTILING(intel)) { >>> hint |= INTEL_CREATE_PIXMAP_TILING_Y; >>> @@ -351,6 +350,14 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, >>> case DRI2BufferFrontRight: >>> hint |= INTEL_CREATE_PIXMAP_TILING_X; >>> break; >>> + case DRI2BufferStencil: >>> + /* >>> + * The stencil buffer is W tiled. However, we >>> + * request from the kernel a non-tiled buffer >>> + * because the GTT is incapable of W fencing. >>> + */ >>> + hint |= INTEL_CREATE_PIXMAP_TILING_NONE; >>> + break; >>> default: >>> free(privates); >>> free(buffer); >> >> Eh... it seems like this will break compatibility with older versions of >> Mesa. I haven't dug around, but there used to be a hack in DRI2 where a >> client would request a depth buffer and a stencil buffer, but it would >> get the same packed depth-stencil buffer for both. I guess that might >> all be up in the DRI2 layer and not in the driver... > > The 'case DRI2BufferStencil' modified in this hunk was added by me when implementing > separate stencil support. It was never used for this hack. > > That hack is implemented by an alternate definition of I830DRI2CreateBuffer() which > is ifdef'd out when DRI2INFOREC_VERSION >= 2. FYI, the line that implements the hack can > be found by grepping xf86-video-intel:src/intel_dri.c for > } else if (attachments[i] == DRI2BufferStencil && pDepthPixmap) { Ah. That sounds right. This patch (with the whitespace fix) is Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> >> >>> @@ -368,11 +375,12 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, >>> * To accomplish this, we resort to the nasty hack of doubling >>> * the drm region's cpp and halving its height. >>> * >>> - * If we neglect to double the pitch, then >>> - * drm_intel_gem_bo_map_gtt() maps the memory incorrectly. >>> + * If we neglect to double the pitch, then render corruption >>> + * occurs. >> >> Mangled whitespace? Probably mixed tabs and spaces... > > Oops. Will fix. > >> >>> */ >>> if (attachment == DRI2BufferStencil) { >>> - pixmap_height /= 2; >>> + pixmap_width = ALIGN(pixmap_width, 64); >>> + pixmap_height = ALIGN(pixmap_height / 2, 64); >>> pixmap_cpp *= 2; >>> } > > -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk4knooACgkQX1gOwKyEAw8mrACdFEbKAyQIntzWec6LVA2ZYYjz 8icAn1Fc+vHRyNeygch2QHnH7Vh9Ilqt =9yZk -----END PGP SIGNATURE-----
diff --git a/src/intel_dri.c b/src/intel_dri.c index 5ea7c2c..4652dc7 100644 --- a/src/intel_dri.c +++ b/src/intel_dri.c @@ -336,7 +336,6 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, switch (attachment) { case DRI2BufferDepth: case DRI2BufferDepthStencil: - case DRI2BufferStencil: case DRI2BufferHiz: if (SUPPORTS_YTILING(intel)) { hint |= INTEL_CREATE_PIXMAP_TILING_Y; @@ -351,6 +350,14 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, case DRI2BufferFrontRight: hint |= INTEL_CREATE_PIXMAP_TILING_X; break; + case DRI2BufferStencil: + /* + * The stencil buffer is W tiled. However, we + * request from the kernel a non-tiled buffer + * because the GTT is incapable of W fencing. + */ + hint |= INTEL_CREATE_PIXMAP_TILING_NONE; + break; default: free(privates); free(buffer); @@ -368,11 +375,12 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment, * To accomplish this, we resort to the nasty hack of doubling * the drm region's cpp and halving its height. * - * If we neglect to double the pitch, then - * drm_intel_gem_bo_map_gtt() maps the memory incorrectly. + * If we neglect to double the pitch, then render corruption + * occurs. */ if (attachment == DRI2BufferStencil) { - pixmap_height /= 2; + pixmap_width = ALIGN(pixmap_width, 64); + pixmap_height = ALIGN(pixmap_height / 2, 64); pixmap_cpp *= 2; }
Until now, the stencil buffer was allocated as a Y tiled buffer, because in several locations the PRM states that it is. However, it is actually W tiled. From the PRM, 2011 Sandy Bridge, Volume 1, Part 2, Section 4.5.2.1 W-Major Format: W-Major Tile Format is used for separate stencil. The GTT is incapable of W fencing, so we allocate the stencil buffer with I915_TILING_NONE and decode the tile's layout in software. This commit mutually depends on the mesa commit: intel: Fix stencil buffer to be W tiled Author: Chad Versace <chad@chad-versace.us> Date: Mon Jul 18 00:37:45 2011 -0700 CC: Eric Anholt <eric@anholt.net> CC: Kenneth Graunke <kenneth@whitecape.org> Signed-off-by: Chad Versace <chad@chad-versace.us> --- src/intel_dri.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)