> -----Original Message-----
> From: Lespiau, Damien
> Sent: Friday, February 05, 2016 4:16 AM
> To: Kibey, Sameer
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; mesa-dev@xxxxxxxxxxxxxxxxxxxxx; 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@xxxxxxxxx>
>
> 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
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
