On Wed, Jun 20, 2018 at 06:32:58PM +0100, Lionel Landwerlin wrote: > On 19/06/18 18:53, Nanley Chery wrote: > > On Tue, Jun 19, 2018 at 11:03:17AM +0100, Lionel Landwerlin wrote: > > > On 18/06/18 19:57, Nanley Chery wrote: > > > > Change the dimension of the texture to test how i965 handles having a > > > > row pitch too large for the BLT engine. > > > > > > > > v2: Query the maximum supported texture width (Ilia Mirkin) > > > > --- > > > > tests/texturing/getteximage-simple.c | 25 +++++++++++++++++-------- > > > > 1 file changed, 17 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/tests/texturing/getteximage-simple.c > > > > b/tests/texturing/getteximage-simple.c > > > > index 525e6a392..5b87b7ccd 100644 > > > > --- a/tests/texturing/getteximage-simple.c > > > > +++ b/tests/texturing/getteximage-simple.c > > > > @@ -8,6 +8,10 @@ > > > > * texture images is executed before the readback. > > > > * > > > > * This used to crash for R300+bufmgr. > > > > + * > > > > + * This also used to stress test the blit methods in i965. The BLT > > > > engine only > > > > + * supports pitch sizes up to but not including 32768 dwords. BLORP > > > > supports > > > > + * even larger sizes. > > > > */ > > > > #include "piglit-util-gl.h" > > > > @@ -21,10 +25,10 @@ PIGLIT_GL_TEST_CONFIG_BEGIN > > > > PIGLIT_GL_TEST_CONFIG_END > > > > -#define MAX_TYPE_VAL UINT8_MAX > > > > -#define PIX_TYPE GLubyte > > > > -#define TEX_TYPE GL_UNSIGNED_BYTE > > > > -#define TEX_INT_FMT GL_RGBA8 > > > > +#define MAX_TYPE_VAL 1.0 > > > > +#define PIX_TYPE GLfloat > > > > +#define TEX_TYPE GL_FLOAT > > > > +#define TEX_INT_FMT GL_RGBA32F > > > > #define TEX_FMT GL_RGBA > > > > #define CHANNELS_PER_PIXEL 4 > > > This is changing the format of the texture we're testing with. > > Yes, a higher resolution format is needed to hit the row pitch of > > interest on the older gens which have a max width of 8K. > > > > > In patch 3 I thought you wanted to make this configurable (through > > > arguments > > > to the test maybe?), but this is just switching the format. > > In patch 3, I attempted to make the test configurable via a group of > > #defines. > > > > > Why not add a command line parameter? > > > > > I didn't want to add a command line parameter because there didn't seem > > to be any benefit to doing so. By changing the format and width, we're > > able to continue testing the issue in R300 as well as the teximage > > upload and download paths of interest in i965. Thoughts? > > Not really, I was just wondering whether that format change would mean that > some driver wouldn't get tested anymore.
Sorry for not mentioning it in the cover letter, but we do lose testing on i915 which doesn't support GL_ARB_texture_float. I didn't feel bad about this because it's not actively developed and the test was written to catch a regression in the R300 driver (which does support the extension AFACIT). Is that okay with you? -Nanley > I guess it's a common enough format that everybody probably supports it. > > Thanks for the explanation : > > > Reviewed-by: Lionel Landwerlin <lionel.g.landwer...@intel.com> > > > > > -Nanley > > > > > Thanks, > > > > > > - > > > Lionel > > > > > > > @@ -42,8 +46,8 @@ static bool test_getteximage(PIX_TYPE *data, size_t > > > > data_size, GLint w, GLint h) > > > > const unsigned pixel_channel = i % > > > > CHANNELS_PER_PIXEL; > > > > printf("GetTexImage() returns incorrect data in > > > > element %i\n", i); > > > > printf(" corresponding to (%i,%i) channel > > > > %i\n", pixel % w, pixel / w, pixel_channel); > > > > - printf(" expected: %i\n", data[i]); > > > > - printf(" got: %i\n", compare[i]); > > > > + printf(" expected: %f\n", data[i]); > > > > + printf(" got: %f\n", compare[i]); > > > > match = false; > > > > break; > > > > } > > > > @@ -53,11 +57,13 @@ static bool test_getteximage(PIX_TYPE *data, size_t > > > > data_size, GLint w, GLint h) > > > > return match; > > > > } > > > > + > > > > enum piglit_result > > > > piglit_display(void) > > > > { > > > > - GLsizei height = 16; > > > > - GLsizei width = 64; > > > > + GLsizei height = 2; > > > > + GLsizei width; > > > > + glGetIntegerv(GL_MAX_TEXTURE_SIZE, &width); > > > > /* Upload random data to a texture with the given dimensions */ > > > > const unsigned data_channels = width * height * > > > > CHANNELS_PER_PIXEL; > > > > @@ -94,6 +100,9 @@ piglit_display(void) > > > > void piglit_init(int argc, char **argv) > > > > { > > > > + if (TEX_TYPE == GL_FLOAT) > > > > + piglit_require_extension("GL_ARB_texture_float"); > > > > + > > > > GLuint tex; > > > > glGenTextures(1, &tex); > > > > _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit