[igt-dev] [PATCH v4 2/8] tests/fbdev: Map framebuffer in igt_fixture

[Thomas Zimmermann]

Hi

Am 17.11.20 um 14:44 schrieb Chris Wilson:
> Quoting Thomas Zimmermann (2020-11-17 12:52:51)
>> The mapping of the framebuffer memory will be useful for read/write
>> tests. Move it into an igt_fixture block.
>>
>> v4:
>>         * declare map as volatile (Petri)
>>         * test struct fb_fix_screeninfo.smem_len with igt_assert()
>>         * remove mmap test from CI
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>>  tests/fbdev.c                         | 14 ++++++--------
>>  tests/intel-ci/fast-feedback.testlist |  1 -
>>  2 files changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/tests/fbdev.c b/tests/fbdev.c
>> index 44a26a6d..eebd2a83 100644
>> --- a/tests/fbdev.c
>> +++ b/tests/fbdev.c
>> @@ -63,22 +63,20 @@ static void mode_tests(int fd)
>>  static void framebuffer_tests(int fd)
>>  {
>>         struct fb_fix_screeninfo fix_info;
>> +       void * volatile map;
>>  
>>         igt_fixture {
>>                 igt_require(ioctl(fd, FBIOGET_FSCREENINFO, &fix_info) == 0);
>> -       }
>> -
>> -       igt_describe("Check mmap operations on framebuffer memory");
>> -       igt_subtest("mmap") {
>> -               void *map;
>> -
>> -               igt_require(fix_info.smem_len);
>> +               igt_assert(fix_info.smem_len);
>>  
>>                 map = mmap(NULL, fix_info.smem_len,
>>                            PROT_WRITE, MAP_SHARED, fd, 0);
>> -               igt_require(map != MAP_FAILED);
>> +               igt_assert(map != MAP_FAILED);
> 
> And now we go back to failing all tests if no mmap, rather than
> skipping unsupported later tests. In the fixture, require would
> definitely be preferable (as the test is abandoned before it is even
> run).

The tests use igt_assert() for pread() and pwrite(). My reasoning was
that mmap() is similar: part driver, part kernel. So I wanted to use
igt_assert(). That it switches to igt_require() was a mistake in the
patchset. But if you say igt_require() is better suited, let's use this
instead.

Best regards
Thomas

> 
>>  
>>                 memset(map, 0, fix_info.smem_len);
>> +       }
>> +
>> +       igt_fixture {
>>                 munmap(map, fix_info.smem_len);
>>         }
>>  }

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer