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@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
