| Message ID | A3EE0A64E0D4F34AB0728B138E43077D22FA8DB3@fmsmsx111.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
FYI, for IGT patches, please do as instructed in CONTRIBUTING:
"""
Please use --subject-prefix="PATCH i-g-t" so that i-g-t patches are easily
identified in the massive amount mails on intel-gfx. To ensure this is always
done just run
git config format.subjectprefix "PATCH i-g-t"
from within your i-g-t git checkout.
"""
BR,
Jani.
On Thu, 04 Feb 2016, "Kibey, Sameer" <sameer.kibey@intel.com> wrote:
> Updated the list-workarounds script so that it
> can parse Mesa directory if provided. Moved the
> common code to a separate function to allow
> reuse for both kernel and mesa.
>
> The new command line is:
> Usage: list-workarounds [options] path-to-kernel
> -k path-to-kernel -m path-to-mesa
>
> The legacy usage is retained to avoid breaking
> backwards compatibility. New parameters -k and
> -m are added for the new behavior.
>
> Either kernel or mesa or both paths can be specified.
> If path-to-mesa is invalid, error is reported.
>
> Signed-off-by: Sameer Kibey <sameer.kibey@intel.com>
> ---
> scripts/list-workarounds | 75 ++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 54 insertions(+), 21 deletions(-)
>
> diff --git a/scripts/list-workarounds b/scripts/list-workarounds
> index d11b6a9..0b63541 100755
> --- a/scripts/list-workarounds
> +++ b/scripts/list-workarounds
> @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n):
> return start
>
> valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw',
> - 'chv', 'skl', 'bxt')
> + 'chv', 'skl', 'bxt', 'kbl', 'byt')
> def parse_platforms(line, p):
> l = p.split(',')
> for p in l:
> @@ -65,9 +65,15 @@ def execute(cmd):
> return out, err
>
> def parse_options(args):
> - usage = "Usage: list-workarounds [options] path-to-kernel"
> + usage = "Usage: list-workarounds [options] path-to-kernel -k path-to-kernel -m path-to-mesa"
> parser = optparse.OptionParser(usage, version=1.0)
>
> + parser.add_option("-k", "--kernel-path", dest="kernel_path", default=None,
> + help="path to kernel")
> +
> + parser.add_option("-m", "--mesa-path", dest="mesa_path", default=None,
> + help="path to mesa")
> +
> parser.add_option("-v", "--verbose", action="store_true",
> dest="verbose", default=False,
> help="be more verbose")
> @@ -76,30 +82,14 @@ def parse_options(args):
> help="List workarounds for the specified platform")
>
> (options, args) = parser.parse_args()
> -
> return (options, args)
>
> -if __name__ == '__main__':
> - (options, args) = parse_options(sys.argv[1:])
> - verbose = options.verbose
> -
> - if not len(args):
> - sys.stderr.write("error: A path to a kernel tree is required\n")
> - sys.exit(1)
> -
> - kernel_path = args[0]
> - kconfig = os.path.join(kernel_path, 'Kconfig')
> - if not os.path.isfile(kconfig):
> - sys.stderr.write("error: %s does not point to a kernel tree \n"
> - % kernel_path)
> - sys.exit(1)
> -
> - i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915')
> +def print_workarounds(code_path, driver_dir):
> olddir = os.getcwd()
> - os.chdir(kernel_path)
> + os.chdir(code_path)
> work_arounds, err = execute(['git', 'grep', '-n',
> '-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+',
> - i915_dir])
> + driver_dir])
> os.chdir(olddir)
> if err:
> print(err)
> @@ -111,3 +101,46 @@ if __name__ == '__main__':
> print("%s: %s" % (wa, ', '.join(workarounds[wa])))
> elif options.platform in workarounds[wa]:
> print(wa)
> +
> +
> +if __name__ == '__main__':
> + (options, args) = parse_options(sys.argv)
> + verbose = options.verbose
> + kernel_path = None
> +
> + if not len(args) and options.kernel_path == None and options.mesa_path == None:
> + sys.stderr.write("error: A path to either a kernel tree or Mesa is required\n")
> + sys.exit(1)
> +
> + if len(args):
> + kernel_path = args[0]
> + elif options.kernel_path != None:
> + kernel_path = options.kernel_path
> +
> + if kernel_path != None:
> + # --- list Kernel workarounds if path is provided ---
> + kconfig = os.path.join(kernel_path, 'Kconfig')
> + if not os.path.isfile(kconfig):
> + sys.stderr.write("error: %s does not point to a kernel tree \n"
> + % kernel_path)
> + sys.exit(1)
> +
> + i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915')
> + print ("List of workarounds found in kernel:")
> + print_workarounds(kernel_path, i915_dir)
> +
> + # --- list mesa workarounds if path is provided ---
> + if options.mesa_path != None:
> + # reset workarounds array
> + workarounds = {}
> +
> + mesa_path = options.mesa_path
> + i965_dir = os.path.join('src', 'mesa', 'drivers', 'dri', 'i965')
> + mesa_dir = os.path.join(mesa_path, i965_dir)
> + if not os.path.exists(mesa_dir):
> + sys.stderr.write("error: %s does not point to a valid mesa path \n"
> + % mesa_path)
> + sys.exit(1)
> +
> + print ("\nList of workarounds found in Mesa:")
> + print_workarounds(mesa_path, i965_dir)
On Thu, Feb 04, 2016 at 06:14:02PM +0000, Kibey, Sameer wrote: > Updated the list-workarounds script so that it > can parse Mesa directory if provided. Moved the > common code to a separate function to allow > reuse for both kernel and mesa. > > The new command line is: > Usage: list-workarounds [options] path-to-kernel > -k path-to-kernel -m path-to-mesa > > The legacy usage is retained to avoid breaking > backwards compatibility. New parameters -k and > -m are added for the new behavior. > > Either kernel or mesa or both paths can be specified. > If path-to-mesa is invalid, error is reported. > > Signed-off-by: Sameer Kibey <sameer.kibey@intel.com> Out of curiosity, how did you send the email? It doesn't seem to have been sent with git send-email and so the patch isn't picked up by our patchwork instance. Out of the comments below, I guess the only serious one is allowing both byt/vlv, but maybe mesa only uses one of the two? I wouldn't mind landing the patch with that answered. > --- > scripts/list-workarounds | 75 ++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 54 insertions(+), 21 deletions(-) > > diff --git a/scripts/list-workarounds b/scripts/list-workarounds > index d11b6a9..0b63541 100755 > --- a/scripts/list-workarounds > +++ b/scripts/list-workarounds > @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n): > return start > > valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw', > - 'chv', 'skl', 'bxt') > + 'chv', 'skl', 'bxt', 'kbl', 'byt') Do we really need both byt and vlv? that creates two different names for the same platform, which sounds like a recipe to have the actual set of W/As for this platform be the union of vlv and byt ones. > def parse_platforms(line, p): > l = p.split(',') > for p in l: > @@ -65,9 +65,15 @@ def execute(cmd): > return out, err > > def parse_options(args): > - usage = "Usage: list-workarounds [options] path-to-kernel" > + usage = "Usage: list-workarounds [options] path-to-kernel -k path-to-kernel -m path-to-mesa" > parser = optparse.OptionParser(usage, version=1.0) Quite frankly, I'd just remove the old behaviour. > + parser.add_option("-k", "--kernel-path", dest="kernel_path", default=None, > + help="path to kernel") > + > + parser.add_option("-m", "--mesa-path", dest="mesa_path", default=None, > + help="path to mesa") > + > parser.add_option("-v", "--verbose", action="store_true", > dest="verbose", default=False, > help="be more verbose") > @@ -76,30 +82,14 @@ def parse_options(args): > help="List workarounds for the specified platform") > > (options, args) = parser.parse_args() > - > return (options, args) > > -if __name__ == '__main__': > - (options, args) = parse_options(sys.argv[1:]) > - verbose = options.verbose > - > - if not len(args): > - sys.stderr.write("error: A path to a kernel tree is required\n") > - sys.exit(1) > - > - kernel_path = args[0] > - kconfig = os.path.join(kernel_path, 'Kconfig') > - if not os.path.isfile(kconfig): > - sys.stderr.write("error: %s does not point to a kernel tree \n" > - % kernel_path) > - sys.exit(1) > - > - i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') > +def print_workarounds(code_path, driver_dir): > olddir = os.getcwd() > - os.chdir(kernel_path) > + os.chdir(code_path) project_root? > work_arounds, err = execute(['git', 'grep', '-n', > '-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+', > - i915_dir]) > + driver_dir]) > os.chdir(olddir) > if err: > print(err) > @@ -111,3 +101,46 @@ if __name__ == '__main__': > print("%s: %s" % (wa, ', '.join(workarounds[wa]))) > elif options.platform in workarounds[wa]: > print(wa) > + > + > +if __name__ == '__main__': > + (options, args) = parse_options(sys.argv) > + verbose = options.verbose > + kernel_path = None > + > + if not len(args) and options.kernel_path == None and options.mesa_path == None: > + sys.stderr.write("error: A path to either a kernel tree or Mesa is required\n") > + sys.exit(1) > + > + if len(args): > + kernel_path = args[0] > + elif options.kernel_path != None: > + kernel_path = options.kernel_path > + > + if kernel_path != None: > + # --- list Kernel workarounds if path is provided --- > + kconfig = os.path.join(kernel_path, 'Kconfig') > + if not os.path.isfile(kconfig): > + sys.stderr.write("error: %s does not point to a kernel tree \n" > + % kernel_path) > + sys.exit(1) > + > + i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') > + print ("List of workarounds found in kernel:") > + print_workarounds(kernel_path, i915_dir) > + > + # --- list mesa workarounds if path is provided --- > + if options.mesa_path != None: > + # reset workarounds array > + workarounds = {} > + > + mesa_path = options.mesa_path > + i965_dir = os.path.join('src', 'mesa', 'drivers', 'dri', 'i965') > + mesa_dir = os.path.join(mesa_path, i965_dir) > + if not os.path.exists(mesa_dir): > + sys.stderr.write("error: %s does not point to a valid mesa path \n" > + % mesa_path) > + sys.exit(1) > + > + print ("\nList of workarounds found in Mesa:") > + print_workarounds(mesa_path, i965_dir) I believe the level where you start branching out between kernel and mesa is too early, or could be factored out a bit more. Notice how you're doing the same work in the two branches with some little changes (eg. the \n at the start of print()). > -- > 1.9.1 >
> -----Original Message----- > From: Lespiau, Damien > Sent: Friday, February 05, 2016 4:16 AM > To: Kibey, Sameer > Cc: intel-gfx@lists.freedesktop.org; mesa-dev@lists.freedesktop.org; Sharp, > Sarah A; Widawsky, Benjamin > Subject: Re: [PATCH] list-workarounds: Extend the script to Mesa > > On Thu, Feb 04, 2016 at 06:14:02PM +0000, Kibey, Sameer wrote: > > Updated the list-workarounds script so that it can parse Mesa > > directory if provided. Moved the common code to a separate function to > > allow reuse for both kernel and mesa. > > > > The new command line is: > > Usage: list-workarounds [options] path-to-kernel > > -k path-to-kernel -m path-to-mesa > > > > The legacy usage is retained to avoid breaking backwards > > compatibility. New parameters -k and -m are added for the new > > behavior. > > > > Either kernel or mesa or both paths can be specified. > > If path-to-mesa is invalid, error is reported. > > > > Signed-off-by: Sameer Kibey <sameer.kibey@intel.com> > > Out of curiosity, how did you send the email? It doesn't seem to have been > sent with git send-email and so the patch isn't picked up by our patchwork > instance. I sent the email manually, but will use git send-email next time. > Out of the comments below, I guess the only serious one is allowing both > byt/vlv, but maybe mesa only uses one of the two? I wouldn't mind landing > the patch with that answered. I will replace byt with vlv to keep it consistent. > > --- > > scripts/list-workarounds | 75 > > ++++++++++++++++++++++++++++++++++-------------- > > 1 file changed, 54 insertions(+), 21 deletions(-) > > > > diff --git a/scripts/list-workarounds b/scripts/list-workarounds index > > d11b6a9..0b63541 100755 > > --- a/scripts/list-workarounds > > +++ b/scripts/list-workarounds > > @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n): > > return start > > > > valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw', > > - 'chv', 'skl', 'bxt') > > + 'chv', 'skl', 'bxt', 'kbl', 'byt') > > Do we really need both byt and vlv? that creates two different names for the > same platform, which sounds like a recipe to have the actual set of W/As for > this platform be the union of vlv and byt ones. Agree, will remove byt. > > def parse_platforms(line, p): > > l = p.split(',') > > for p in l: > > @@ -65,9 +65,15 @@ def execute(cmd): > > return out, err > > > > def parse_options(args): > > - usage = "Usage: list-workarounds [options] path-to-kernel" > > + usage = "Usage: list-workarounds [options] path-to-kernel -k path- > to-kernel -m path-to-mesa" > > parser = optparse.OptionParser(usage, version=1.0) > > Quite frankly, I'd just remove the old behaviour. Originally I had removed the old behavior. Ben suggested keeping it in case some people have it in other scripts. > > + parser.add_option("-k", "--kernel-path", dest="kernel_path", > default=None, > > + help="path to kernel") > > + > > + parser.add_option("-m", "--mesa-path", dest="mesa_path", > default=None, > > + help="path to mesa") > > + > > parser.add_option("-v", "--verbose", action="store_true", > > dest="verbose", default=False, > > help="be more verbose") > > @@ -76,30 +82,14 @@ def parse_options(args): > > help="List workarounds for the specified platform") > > > > (options, args) = parser.parse_args() > > - > > return (options, args) > > > > -if __name__ == '__main__': > > - (options, args) = parse_options(sys.argv[1:]) > > - verbose = options.verbose > > - > > - if not len(args): > > - sys.stderr.write("error: A path to a kernel tree is > required\n") > > - sys.exit(1) > > - > > - kernel_path = args[0] > > - kconfig = os.path.join(kernel_path, 'Kconfig') > > - if not os.path.isfile(kconfig): > > - sys.stderr.write("error: %s does not point to a kernel tree \n" > > - % kernel_path) > > - sys.exit(1) > > - > > - i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') > > +def print_workarounds(code_path, driver_dir): > > olddir = os.getcwd() > > - os.chdir(kernel_path) > > + os.chdir(code_path) > > project_root? Will change to project_root > > work_arounds, err = execute(['git', 'grep', '-n', > > '-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+', > > - i915_dir]) > > + driver_dir]) > > os.chdir(olddir) > > if err: > > print(err) > > @@ -111,3 +101,46 @@ if __name__ == '__main__': > > print("%s: %s" % (wa, ', '.join(workarounds[wa]))) > > elif options.platform in workarounds[wa]: > > print(wa) > > + > > + > > +if __name__ == '__main__': > > + (options, args) = parse_options(sys.argv) > > + verbose = options.verbose > > + kernel_path = None > > + > > + if not len(args) and options.kernel_path == None and > options.mesa_path == None: > > + sys.stderr.write("error: A path to either a kernel tree or > Mesa is required\n") > > + sys.exit(1) > > + > > + if len(args): > > + kernel_path = args[0] > > + elif options.kernel_path != None: > > + kernel_path = options.kernel_path > > + > > + if kernel_path != None: > > + # --- list Kernel workarounds if path is provided --- > > + kconfig = os.path.join(kernel_path, 'Kconfig') > > + if not os.path.isfile(kconfig): > > + sys.stderr.write("error: %s does not point to a kernel > tree \n" > > + % kernel_path) > > + sys.exit(1) > > + > > + i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') > > + print ("List of workarounds found in kernel:") > > + print_workarounds(kernel_path, i915_dir) > > + > > + # --- list mesa workarounds if path is provided --- > > + if options.mesa_path != None: > > + # reset workarounds array > > + workarounds = {} > > + > > + mesa_path = options.mesa_path > > + i965_dir = os.path.join('src', 'mesa', 'drivers', 'dri', 'i965') > > + mesa_dir = os.path.join(mesa_path, i965_dir) > > + if not os.path.exists(mesa_dir): > > + sys.stderr.write("error: %s does not point to a valid > mesa path \n" > > + % mesa_path) > > + sys.exit(1) > > + > > + print ("\nList of workarounds found in Mesa:") > > + print_workarounds(mesa_path, i965_dir) > > I believe the level where you start branching out between kernel and mesa is > too early, or could be factored out a bit more. Notice how you're doing the > same work in the two branches with some little changes (eg. the \n at the > start of print()). I'll refactor this some more. > > -- > > 1.9.1 > >
diff --git a/scripts/list-workarounds b/scripts/list-workarounds index d11b6a9..0b63541 100755 --- a/scripts/list-workarounds +++ b/scripts/list-workarounds @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n): return start valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw', - 'chv', 'skl', 'bxt') + 'chv', 'skl', 'bxt', 'kbl', 'byt') def parse_platforms(line, p): l = p.split(',') for p in l: @@ -65,9 +65,15 @@ def execute(cmd): return out, err def parse_options(args): - usage = "Usage: list-workarounds [options] path-to-kernel" + usage = "Usage: list-workarounds [options] path-to-kernel -k path-to-kernel -m path-to-mesa" parser = optparse.OptionParser(usage, version=1.0) + parser.add_option("-k", "--kernel-path", dest="kernel_path", default=None, + help="path to kernel") + + parser.add_option("-m", "--mesa-path", dest="mesa_path", default=None, + help="path to mesa") + parser.add_option("-v", "--verbose", action="store_true", dest="verbose", default=False, help="be more verbose") @@ -76,30 +82,14 @@ def parse_options(args): help="List workarounds for the specified platform") (options, args) = parser.parse_args() - return (options, args) -if __name__ == '__main__': - (options, args) = parse_options(sys.argv[1:]) - verbose = options.verbose - - if not len(args): - sys.stderr.write("error: A path to a kernel tree is required\n") - sys.exit(1) - - kernel_path = args[0] - kconfig = os.path.join(kernel_path, 'Kconfig') - if not os.path.isfile(kconfig): - sys.stderr.write("error: %s does not point to a kernel tree \n" - % kernel_path) - sys.exit(1) - - i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') +def print_workarounds(code_path, driver_dir): olddir = os.getcwd() - os.chdir(kernel_path) + os.chdir(code_path) work_arounds, err = execute(['git', 'grep', '-n', '-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+', - i915_dir]) + driver_dir]) os.chdir(olddir) if err: print(err) @@ -111,3 +101,46 @@ if __name__ == '__main__': print("%s: %s" % (wa, ', '.join(workarounds[wa]))) elif options.platform in workarounds[wa]: print(wa) + + +if __name__ == '__main__': + (options, args) = parse_options(sys.argv) + verbose = options.verbose + kernel_path = None + + if not len(args) and options.kernel_path == None and options.mesa_path == None: + sys.stderr.write("error: A path to either a kernel tree or Mesa is required\n") + sys.exit(1) + + if len(args): + kernel_path = args[0] + elif options.kernel_path != None: + kernel_path = options.kernel_path + + if kernel_path != None: + # --- list Kernel workarounds if path is provided --- + kconfig = os.path.join(kernel_path, 'Kconfig') + if not os.path.isfile(kconfig): + sys.stderr.write("error: %s does not point to a kernel tree \n" + % kernel_path) + sys.exit(1) + + i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') + print ("List of workarounds found in kernel:") + print_workarounds(kernel_path, i915_dir) + + # --- list mesa workarounds if path is provided --- + if options.mesa_path != None: + # reset workarounds array + workarounds = {} + + mesa_path = options.mesa_path + i965_dir = os.path.join('src', 'mesa', 'drivers', 'dri', 'i965') + mesa_dir = os.path.join(mesa_path, i965_dir) + if not os.path.exists(mesa_dir): + sys.stderr.write("error: %s does not point to a valid mesa path \n" + % mesa_path) + sys.exit(1) + + print ("\nList of workarounds found in Mesa:") + print_workarounds(mesa_path, i965_dir)
Updated the list-workarounds script so that it can parse Mesa directory if provided. Moved the common code to a separate function to allow reuse for both kernel and mesa. The new command line is: Usage: list-workarounds [options] path-to-kernel -k path-to-kernel -m path-to-mesa The legacy usage is retained to avoid breaking backwards compatibility. New parameters -k and -m are added for the new behavior. Either kernel or mesa or both paths can be specified. If path-to-mesa is invalid, error is reported. Signed-off-by: Sameer Kibey <sameer.kibey@intel.com> --- scripts/list-workarounds | 75 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 21 deletions(-)