[Piglit] [PATCH 2/4] framework: Add a driver classifier based on the renderer string.

Dylan Baker dylan at pnwbakers.com
Tue Aug 23 18:16:20 UTC 2016


Quoting Eric Anholt (2016-08-22 18:41:34)
> For the apitrace test runner, I need to be able to store known-good
> images for various drivers, which means I need some way to group
> drivers.  Not all Mesa drivers we care about are covered here, but
> this is a sample to get things started.
> ---
>  framework/driver_classifier.py                | 126 ++++++++++++++++++++++++++
>  unittests/framework/test_driver_classifier.py |  59 ++++++++++++
>  2 files changed, 185 insertions(+)
>  create mode 100644 framework/driver_classifier.py
>  create mode 100644 unittests/framework/test_driver_classifier.py
> 
> diff --git a/framework/driver_classifier.py b/framework/driver_classifier.py
> new file mode 100644
> index 000000000000..e35c40c5f3fd
> --- /dev/null
> +++ b/framework/driver_classifier.py
> @@ -0,0 +1,126 @@
> +# Copyright (c) 2016 Broadcom
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +import re, subprocess

Style nit: Generally we import modules one per line, things from modules
on in a single line,
ex:
    import os
    import re

    from framework.grouptools import join, split

Also, you need to import errno, since you're using it.

Finally, can you copy the 'from __future__ import (...' from one of the
other modules?

> +
> +__all__ = [
> +    'DriverClassifier',
> +]
> +
> +class DriverClassifier(object):
> +    def __init__(self):
> +        self.categories = []
> +
> +        self.collect_driver_info()
> +        self.find_categories()
> +
> +
> +    def collect_driver_info(self):
> +        """Method for collecting the GL driver renderer string.
> +
> +        Currently only glxinfo is used.
> +        """
> +        self.renderer = ''
> +
> +        self.collect_glxinfo()
> +
> +
> +    def collect_glxinfo(self):
> +        """Calls glxinfo and parses the output to find the GL driver
> +
> +        vendor/renderer strings.
> +        """
> +
> +        try:
> +            output = subprocess.check_output(['glxinfo'],
> +                                             stderr=subprocess.STDOUT)
> +        except OSError as e:
> +            if e.errno not in [errno.ENOENT, errno.EACCES]:
> +                raise
> +            return
> +        except subprocess.CalledProcessError:
> +            if e.errno not in [errno.ENOENT, errno.EACCES]:
> +                raise

The if statement above looks like a copy-and-paste error

> +            return
> +
> +        for line in output.splitlines():
> +            m = re.match('OpenGL renderer string: (.*)', line)
> +            if m is not None:
> +                self.renderer = m.group(1)
> +                break

I wonder if we can share some of the code in core.collect_system_info
for some of this work? Obviously this does more work than just
collecting the contents of glxinfo. Maybe extend collect_system_info to
take a parameter?

> +
> +
> +    def find_categories(self):
> +        """Parses the vendor/renderer strings to decide what categories
> +
> +        the driver falls under.
> +        """
> +        if self.renderer.startswith(('Mesa ', 'Gallium ')):
> +            self.categories.append('mesa')
> +
> +            m = re.match('.* VC4(.*)', self.renderer)
> +            if m is not None:
> +                self.categories.append('vc4')
> +                m = re.match(' V3D ([0-9])+\.([0-9])+', m.group(1))
> +                if m is not None:
> +                    self.categories.append('vc4-{}.{}'.format(m.group(1),
> +                                                              m.group(2)))
> +
> +            m = re.match('Mesa DRI R200 ', self.renderer)
> +            if m is not None:
> +                self.categories.append('r200')
> +
> +            m = re.match('Mesa DRI Intel[^ ]* (.*)', self.renderer)
> +            if m is not None:
> +                tail = m.group(1)
> +
> +                i965_chipdict = {
> +                    '965': 'brw',
> +                    '946': 'brw',
> +                    '.*[GQ]4[35]': 'g4x',
> +                    'Ironlake': 'ilk',
> +                    'Sandybridge': 'snb',
> +                    'Ivybridge': 'ivb',
> +                    'Haswell': 'hsw',
> +                    'Baytrail': 'byt',
> +                    'Broadwell': 'bdw',
> +                    'Skylake': 'skl',
> +                    'Kabylake': 'kbl',
> +                    '.*Cherryview': 'chv',
> +                    '.*Broxton': 'bxt',
> +                }
> +
> +                for chip, abbrev in i965_chipdict.items():
> +                    m = re.match(chip, tail)
> +                    if m is not None:
> +                        self.categories.append('i965')
> +                        self.categories.append('i965-{}'.format(abbrev))
> +                        break
> +
> +        self.categories.reverse()
> +
> +
> +    def get_categories(self):
> +
> +        """Returns a list of the driver's categories, from most specific
> +        specific to least specific."""
> +
> +        return self.categories

I don't know that this method is all that valuable, since categories is
already a public attribute.

> diff --git a/unittests/framework/test_driver_classifier.py b/unittests/framework/test_driver_classifier.py
> new file mode 100644
> index 000000000000..e846be996d37
> --- /dev/null
> +++ b/unittests/framework/test_driver_classifier.py
> @@ -0,0 +1,59 @@
> +# Copyright © 2016 Broadcom

you either need to adding an encoding string above this line
('# encoding=utf-8' will work), or replace the copyright symbol with (c)
or similar

> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +import pytest
> +import six
> +from framework import driver_classifier
> +
> +class DriverClassifierTester(driver_classifier.DriverClassifier):
> +    """Test class for the driver classifier, taking in a fixed
> +
> +    renderer string instead of calling glxinfo.
> +    """
> +
> +    def __init__(self, renderer):
> +        self.override_renderer = renderer
> +        super(DriverClassifierTester, self).__init__()
> +
> +
> +    def collect_driver_info(self):
> +        self.renderer = self.override_renderer
> +
> +
> +class TestDriverClassifier(object):
> +    """Tests for the DriverClassifier class."""
> +
> +    @pytest.mark.parametrize(
> +        "renderer, categories",
> +        [
> +            ('Mesa DRI Intel(R) Haswell Mobile', ['i965-hsw', 'i965', 'mesa']),
> +            # Old VC4 string
> +            ('Gallium 0.4 on VC4', ['vc4', 'mesa']),
> +            # Modern VC4 string
> +            ('Gallium 0.4 on VC4 V3D 2.1', ['vc4-2.1', 'vc4', 'mesa']),
> +        ],
> +        ids=six.text_type)
> +    def test_renderer_categorization(self, renderer, categories):
> +        """Test that when given a certain renderer string, the correct
> +
> +        categories list comes back.
> +        """
> +        assert DriverClassifierTester(renderer).get_categories() == categories
> -- 
> 2.9.3

Thanks for writing tests, I really appreciate that a lot.

> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20160823/8923b430/attachment-0001.sig>


More information about the Piglit mailing list