Re: [Mesa-dev] [PATCH] egl/x11: avoid using freed memory if dri2 init fails

[Damien Lespiau]

On Mon, Aug 15, 2016 at 11:46:40AM -0700, Kristian Høgsberg wrote:
> On Mon, Aug 15, 2016 at 11:33 AM, ⚛ <0xe2.0x9a.0...@gmail.com> wrote:
> > On Mon, Aug 15, 2016 at 8:08 PM, Emil Velikov  
> > wrote:
> >>
> >> On 4 August 2016 at 03:13, Nicolas Boichat  wrote:
> >> > Thanks! See also related series here, which fixes the other platforms:
> >> > https://lists.freedesktop.org/archives/mesa-dev/2016-August/125147.html
> >> >
> >> > Fixes: 9ee683f877 (egl/dri2: Add reference count for dri2_egl_display)
> >> > Cc: "12.0" 
> >> > Reviewed-by: Nicolas Boichat 
> >> >
> >> This and there remaining DriverData patches are in master now.
> >
> > Thanks.
> >
> >> Jan, I believe you're ok/don't mind Patchwork. Can you please
> >> check/update things if needed. Patchwork seems unhappy whenever I look
> >> up for you.
> >
> > I looked into updating my name at Patchwork some time ago but the
> > website seems to be missing support for this.
> >
> > I failed to predict that Patchwork will infer the name from the 1st
> > patch I send.
> >
> > Is there a person with low-level access to patchwork.freedesktop.org I
> > can send an email to? There's a list of Mesa maintaners at
> > https://patchwork.freedesktop.org/project/mesa/ but I am unable to
> > tell who to contact.
> 
> Damien should be able to help with patchwork.

Sure thing, changed Jan's name.

-- 
Damien
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 i-g-t] igt/list-workarounds: Extend the script to Mesa

[Damien Lespiau]

On Fri, Feb 05, 2016 at 04:12:08PM -0800, Dylan Baker wrote:
> > >   parse(work_arounds)
> > > + print "\nList of workarounds found in %s:" % project
> 
> Hey Damien, the script says it's python 3, and this ^^^ is broken syntax
> in python 3 (but not in 2).

:(

I did notice the python2 construct, but then, of course, the diff is
missing the full context so didn't realize it was a python3 script.

Sent and pushed the obvious fix.

I really want to trust that developers run the code at least once before
submitting, even if it's a rework of the original patch. Even better
would be a simple unit test, and hook make distcheck to patchwork. I'll
look into that at some point.

-- 
Damien
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] list-workarounds: Extend the script to Mesa

[Damien Lespiau]

On Thu, Feb 04, 2016 at 06:14:02PM +, 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 

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"
> + 

Re: [Mesa-dev] [PATCH v2 i-g-t] igt/list-workarounds: Extend the script to Mesa

[Damien Lespiau]

On Fri, Feb 05, 2016 at 01:55:19PM -0800, Sameer Kibey 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 

Pushed thanks for the patch.

-- 
Damien

> ---
>  scripts/list-workarounds | 74 
> ++--
>  1 file changed, 53 insertions(+), 21 deletions(-)
> 
> diff --git a/scripts/list-workarounds b/scripts/list-workarounds
> index d11b6a9..8b41ae5 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')
>  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,38 +82,64 @@ 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(project_root, driver_dir, project):
>   olddir = os.getcwd()
> - os.chdir(kernel_path)
> + os.chdir(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)
>   sys.exit(1)
>  
>   parse(work_arounds)
> + print "\nList of workarounds found in %s:" % project
>   for wa in sorted(workarounds.keys()):
>   if not options.platform:
>   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_workarounds(kernel_path, i915_dir, "kernel")
> +
> + # --- list mesa workarounds if path is provided ---
> + if options.mesa_path != None:
> + # reset workarounds array
> +  

[Mesa-dev] [Patchwork] The infrequent patchwork update #1

[Damien Lespiau]

Hi all,

You may have noticed already, patchwork.freedesktop.org looks different.

That new version includes:
  - Some re-design. Design is very much an iterative process, thoughts
and comments are welcome,
  - Showing the number of Reviewed-by, Acked-by, Tested-by tags,
  - Some cleanup of the data base, removing stale registration email
addresses (mostly bots trying to register) that were showing in the
completion list.

That's it for the first update. I wanted to keep it small-ish to do some
heavy lifting behind the scenes (latest patchwork, latest django, new db
migration system, virtualenvs, ...)

One can open issues against the freedesktop version of patchwork (not
all is upstream just yet, but hopefully on its way there) on github:

  https://github.com/dlespiau/patchwork/issues

For administrative tasks (eg. add projects/maintainers), please use
fdo's bugzilla, product freedeskop.org, component patchwork:

  https://bugs.freedesktop.org/enter_bug.cgi?product=freedesktop.org

Future plans include a "series-aware" patchwork with the goal of
exposing series and revision of series (v2, 3, ..) to the world so one
can hook automatic testing to patchwork, read more at:

  https://lists.ozlabs.org/pipermail/patchwork/2015-September/001601.html

HTH,

-- 
Damien
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/5] i915: Remove unused IS_MOBILE macro

[Damien Lespiau]

On Thu, Mar 05, 2015 at 11:49:54AM -0800, Ian Romanick wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 Inspired by Damien's recent libdrm changes.
 
 Signed-off-by: Ian Romanick ian.d.roman...@intel.com
 Cc: Damien Lespiau damien.lesp...@intel.com

For the whole series (not that my r-b tag has a lot of value on the mesa
code base):

Reviewed-by: Damien Lespiau damien.lesp...@intel.com

-- 
Damien

 ---
  src/mesa/drivers/dri/i915/intel_chipset.h | 10 --
  1 file changed, 10 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i915/intel_chipset.h 
 b/src/mesa/drivers/dri/i915/intel_chipset.h
 index 3828085..d05fd08 100644
 --- a/src/mesa/drivers/dri/i915/intel_chipset.h
 +++ b/src/mesa/drivers/dri/i915/intel_chipset.h
 @@ -53,16 +53,6 @@
  #define IS_PNVG(devid)   (devid == PCI_CHIP_PNV_G)
  #define IS_PNV(devid) (IS_PNVG(devid) || IS_PNVGM(devid))
  
 -#define IS_MOBILE(devid) (devid == PCI_CHIP_I855_GM || \
 -  devid == PCI_CHIP_I915_GM || \
 -  devid == PCI_CHIP_I945_GM || \
 -  devid == PCI_CHIP_I945_GME || \
 -  devid == PCI_CHIP_I965_GM || \
 -  devid == PCI_CHIP_I965_GME || \
 -  devid == PCI_CHIP_GM45_GM || \
 -  IS_PNV(devid) || \
 -  devid == PCI_CHIP_ILM_G)
 -
  #define IS_915(devid)(devid == PCI_CHIP_I915_G || \
devid == PCI_CHIP_E7221_G || \
devid == PCI_CHIP_I915_GM)
 -- 
 2.1.0
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Bug in 48157b904a9 found by Coverity

[Damien Lespiau]

On Wed, Nov 19, 2014 at 12:13:41PM -0500, Ilia Mirkin wrote:
 Hey guys,
 
 Just got around to looking at the Coverity email about newly
 introduced bugs, and this seems like a legit issue. Based on the
 context of the code, I'm guessing you meant to do (input_index - 16)
  1.

Oh my. The proposed fix looks good, mind crafting a patch?

-- 
Damien

 
 Cheers,
 
   -ilia
 
 ** CID 1251308:  Bad bit shift operation  (BAD_SHIFT)
 /src/mesa/drivers/dri/i965/gen8_sf_state.c: 99 in upload_sbe()
 
 
 *** CID 1251308:  Bad bit shift operation  (BAD_SHIFT)
 /src/mesa/drivers/dri/i965/gen8_sf_state.c: 99 in upload_sbe()
 93  if (!(brw-fragment_program-Base.InputsRead 
 BITFIELD64_BIT(attr)))
 94 continue;
 95
 96  if (input_index  16)
 97 dw4 |= (GEN9_SBE_ACTIVE_COMPONENT_XYZW 
 (input_index  1));
 98  else
  CID 1251308:  Bad bit shift operation  (BAD_SHIFT)
  In expression 3  (input_index  1), left shifting by more than 
  31 bits has undefined behavior.  The shift amount, input_index  1, is 
  32.
 99 dw5 |= (GEN9_SBE_ACTIVE_COMPONENT_XYZW 
 (input_index  1));
 100
 101  ++input_index;
 102   }
 103}
 104BEGIN_BATCH(sbe_cmd_length);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Bug in 48157b904a9 found by Coverity

[Damien Lespiau]

On Wed, Nov 19, 2014 at 12:42:45PM -0500, Ilia Mirkin wrote:
 On Wed, Nov 19, 2014 at 12:28 PM, Damien Lespiau
 damien.lesp...@intel.com wrote:
  On Wed, Nov 19, 2014 at 12:13:41PM -0500, Ilia Mirkin wrote:
  Hey guys,
 
  Just got around to looking at the Coverity email about newly
  introduced bugs, and this seems like a legit issue. Based on the
  context of the code, I'm guessing you meant to do (input_index - 16)
   1.
 
  Oh my. The proposed fix looks good, mind crafting a patch?
 
 Probably best done by someone with access to the hardware/simulator or
 who has read the specs (are the gen9 specs even public yet? haven't
 checked).

Nop.

 As an aside, it seems like there can be 32 var varyings, and there are
 a bunch of additional slots on top of that... not sure if you meant to
 start at VARYING_SLOT_VAR0 or if you wanted all the varying slots, and
 there's some sort of additional mechanism that makes sure that at most
 32 can be set. [I guess this is for FS, so it can't have a whole lot
 of other inputs, but gl_FragCoord comes to mind. Perhaps that counts
 against the varying max.]

It does look fishy. That's probably a question for Ken, a real mesa
person though.

-- 
Damien
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Initialize inout_offset parameter to brw_search_cache().

[Damien Lespiau]

On Wed, Jul 24, 2013 at 10:02:22AM -0700, Kenneth Graunke wrote:
 On 07/24/2013 09:33 AM, Paul Berry wrote:
 Two callers of brw_search_cache() weren't initializing that function's
 inout_offset parameter: brw_blorp_const_color_params::get_wm_prog()
 and brw_blorp_const_color_params::get_wm_prog().
 
 That's a benign problem, since the only effect of not initializing
 inout_offset prior to calling brw_search_cache() is that the bit
 corresponding to cache_id in brw-state.dirty.cache may not be set
 reliably.  This is ok, since the cache_id's used by
 brw_blorp_const_color_params::get_wm_prog() and
 brw_blorp_blit_params::get_wm_prog() (BRW_BLORP_CONST_COLOR_PROG and
 BRW_BLORP_BLIT_PROG, respectively) correspond to dirty bits that are
 not used.
 
 However, failing to initialize this parameter causes valgrind to
 complain.  So let's go ahead and fix it to reduce valgrind noise.
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66779

Can we have this commit in the 9.2 branch? It was added in the release
blocker bug but never cherry-picked for 9.2.

Thanks!

-- 
Damien
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] gitignore: Ignore TAGS files

[Damien Lespiau]

TAGS files are generated with make tags to quickly jump through the
code. Ignore those by-products of automake/ctags.

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 .gitignore |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/.gitignore b/.gitignore
index faf2ca3..5c2ccdc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -44,3 +44,4 @@ manifest.txt
 .libs/
 Makefile
 Makefile.in
+TAGS
-- 
1.7.7.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] build: Fix the documented default value of --with-gallium-drivers

[Damien Lespiau]

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 configure.ac |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1294740..3792b12 100644
--- a/configure.ac
+++ b/configure.ac
@@ -680,13 +680,15 @@ AC_ARG_ENABLE([gallium_tests],
 [enable_gallium_tests=no])
 
 # Option for Gallium drivers
+
+# Keep this in sync with the --with-gallium-drivers help string default value
 GALLIUM_DRIVERS_DEFAULT=r300,r600,svga,swrast
 
 AC_ARG_WITH([gallium-drivers],
 [AS_HELP_STRING([--with-gallium-drivers@:@=DIRS...@:@],
 [comma delimited Gallium drivers list, e.g.
 i915,nouveau,r300,r600,radeonsi,svga,swrast
-@:@default=r300,r600,swrast@:@])],
+@:@default=r300,r600,svga,swrast@:@])],
 [with_gallium_drivers=$withval],
 [with_gallium_drivers=$GALLIUM_DRIVERS_DEFAULT])
 
-- 
1.7.7.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] build: Fix the documented default value of --with-gallium-drivers

[Damien Lespiau]

On Mon, Jan 14, 2013 at 07:18:30AM -0800, Brian Paul wrote:
 Do you nedd someone to commit this for you?

Yes please, I don't have the bits to push to mesa.

Thanks,

-- 
Damien
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev