Re: [Beignet] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Mar 13, 2015 at 09:51:57AM -0700, Jeff McGee wrote:
> On Fri, Mar 13, 2015 at 05:32:41PM +0100, Daniel Vetter wrote:
> > On Fri, Mar 13, 2015 at 05:09:46PM +0800, Zhigang Gong wrote:
> > > My only concern is about the following macros:
> > > 
> > > > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL      33
> > > > +#define LOCAL_I915_PARAM_EU_TOTAL    34
> > > 
> > > How about to just use the definitons in the kernel header file?
> > > For an example:
> > > 
> > >   #include <drm/i915_drm.h>
> > > 
> > >   #ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL
> > >   //Put all the code into this block.
> > >   #endif
> > > 
> > > Then we can avoid put the same definitions in different files,
> > > and we can avoid unecessary testing on an old kernel which doesn't
> > > have this kernel interface.
> > > 
> > > For all the other part, it LGTM.
> > > 
> > > Reviewed-by: Zhigang Gong <zhigang.gong@linux.intel.com>
> > 
> > Once we update the libdrm requirements in igt we tend to go around and
> > replace all the now obsolete LOCAL_ defines. Imo not worth doing extra
> > work until then.
> > 
> > Patch applied, thanks.
> > -Daniel
> > 
> 
> Patch applied? Do you want me to make the name change first? Should the
> kernel part be reviewed and merged first?

Forgot my own review comment already ;-) Fixed up with a follow-up patch.
And I'll pull the kernel part in now too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx