Re: [Intel-gfx] [PATCH 3/6] drm/i915: keep declarations in i915_drv.h
On to, 2016-09-22 at 13:50 +0200, Daniel Vetter wrote: > we also need to split up our headers I think, and I plan to start doing > that when extracting bits and pieces from intel_display.c. In drm core at > least having 1:1 between headers and source files seems to work rather > well. +1 on that, I think all sane projects do that. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: keep declarations in i915_drv.h
On Tue, Sep 20, 2016 at 12:58:20PM +0300, Jani Nikula wrote: > On Tue, 20 Sep 2016, Chris Wilson wrote: > > On Tue, Sep 20, 2016 at 11:57:06AM +0300, Jani Nikula wrote: > >> On Mon, 19 Sep 2016, Joonas Lahtinen > >> wrote: > >> > On to, 2016-09-15 at 16:28 +0300, Jani Nikula wrote: > >> >> Fix sparse warnings: > >> >> > >> >> drivers/gpu/drm/i915/i915_drv.c:1179:5: warning: symbol > >> >> 'i915_driver_load' was not declared. Should it be static? > >> >> > >> >> drivers/gpu/drm/i915/i915_drv.c:1267:6: warning: symbol > >> >> 'i915_driver_unload' was not declared. Should it be static? > >> >> > >> >> drivers/gpu/drm/i915/i915_drv.c:2444:25: warning: symbol 'i915_pm_ops' > >> >> was not declared. Should it be static? > >> >> > >> > > >> > Hmm, Chris, were not these change in the middle of a series, did the > >> > cleanup patches just fall off? > >> > >> Can we just please use whichever patches that fix the issue? > > > > It was deliberate placement to avoid having those symbols exposed to > > every user of i915_drv.h, i.e. everyone. Limit i915_drv.h to the > > interface exposed by i915_drv.c and so used only by a handful of files. > > I can see that, but I prefer to get the warnings gone. Yeah, non-static declarations need to be in header files, otherwise there's not much point in them really (since the compiler can't compare the delcaration with the definition). we also need to split up our headers I think, and I plan to start doing that when extracting bits and pieces from intel_display.c. In drm core at least having 1:1 between headers and source files seems to work rather well. Meanwhile on Jani's patch: Reviewed-by: Daniel Vetter -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: keep declarations in i915_drv.h
On Thu, 22 Sep 2016, Daniel Vetter wrote: > On Tue, Sep 20, 2016 at 12:58:20PM +0300, Jani Nikula wrote: >> On Tue, 20 Sep 2016, Chris Wilson wrote: >> > On Tue, Sep 20, 2016 at 11:57:06AM +0300, Jani Nikula wrote: >> >> On Mon, 19 Sep 2016, Joonas Lahtinen >> >> wrote: >> >> > On to, 2016-09-15 at 16:28 +0300, Jani Nikula wrote: >> >> >> Fix sparse warnings: >> >> >> >> >> >> drivers/gpu/drm/i915/i915_drv.c:1179:5: warning: symbol >> >> >> 'i915_driver_load' was not declared. Should it be static? >> >> >> >> >> >> drivers/gpu/drm/i915/i915_drv.c:1267:6: warning: symbol >> >> >> 'i915_driver_unload' was not declared. Should it be static? >> >> >> >> >> >> drivers/gpu/drm/i915/i915_drv.c:2444:25: warning: symbol 'i915_pm_ops' >> >> >> was not declared. Should it be static? >> >> >> >> >> > >> >> > Hmm, Chris, were not these change in the middle of a series, did the >> >> > cleanup patches just fall off? >> >> >> >> Can we just please use whichever patches that fix the issue? >> > >> > It was deliberate placement to avoid having those symbols exposed to >> > every user of i915_drv.h, i.e. everyone. Limit i915_drv.h to the >> > interface exposed by i915_drv.c and so used only by a handful of files. >> >> I can see that, but I prefer to get the warnings gone. > > Yeah, non-static declarations need to be in header files, otherwise > there's not much point in them really (since the compiler can't compare > the delcaration with the definition). > > we also need to split up our headers I think, and I plan to start doing > that when extracting bits and pieces from intel_display.c. In drm core at > least having 1:1 between headers and source files seems to work rather > well. > > Meanwhile on Jani's patch: > > Reviewed-by: Daniel Vetter Thanks, pushed to dinq. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: keep declarations in i915_drv.h
On Tue, 20 Sep 2016, Chris Wilson wrote: > On Tue, Sep 20, 2016 at 11:57:06AM +0300, Jani Nikula wrote: >> On Mon, 19 Sep 2016, Joonas Lahtinen wrote: >> > On to, 2016-09-15 at 16:28 +0300, Jani Nikula wrote: >> >> Fix sparse warnings: >> >> >> >> drivers/gpu/drm/i915/i915_drv.c:1179:5: warning: symbol >> >> 'i915_driver_load' was not declared. Should it be static? >> >> >> >> drivers/gpu/drm/i915/i915_drv.c:1267:6: warning: symbol >> >> 'i915_driver_unload' was not declared. Should it be static? >> >> >> >> drivers/gpu/drm/i915/i915_drv.c:2444:25: warning: symbol 'i915_pm_ops' >> >> was not declared. Should it be static? >> >> >> > >> > Hmm, Chris, were not these change in the middle of a series, did the >> > cleanup patches just fall off? >> >> Can we just please use whichever patches that fix the issue? > > It was deliberate placement to avoid having those symbols exposed to > every user of i915_drv.h, i.e. everyone. Limit i915_drv.h to the > interface exposed by i915_drv.c and so used only by a handful of files. I can see that, but I prefer to get the warnings gone. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: keep declarations in i915_drv.h
On Tue, Sep 20, 2016 at 11:57:06AM +0300, Jani Nikula wrote: > On Mon, 19 Sep 2016, Joonas Lahtinen wrote: > > On to, 2016-09-15 at 16:28 +0300, Jani Nikula wrote: > >> Fix sparse warnings: > >> > >> drivers/gpu/drm/i915/i915_drv.c:1179:5: warning: symbol > >> 'i915_driver_load' was not declared. Should it be static? > >> > >> drivers/gpu/drm/i915/i915_drv.c:1267:6: warning: symbol > >> 'i915_driver_unload' was not declared. Should it be static? > >> > >> drivers/gpu/drm/i915/i915_drv.c:2444:25: warning: symbol 'i915_pm_ops' > >> was not declared. Should it be static? > >> > > > > Hmm, Chris, were not these change in the middle of a series, did the > > cleanup patches just fall off? > > Can we just please use whichever patches that fix the issue? It was deliberate placement to avoid having those symbols exposed to every user of i915_drv.h, i.e. everyone. Limit i915_drv.h to the interface exposed by i915_drv.c and so used only by a handful of files. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: keep declarations in i915_drv.h
On Mon, 19 Sep 2016, Joonas Lahtinen wrote: > On to, 2016-09-15 at 16:28 +0300, Jani Nikula wrote: >> Fix sparse warnings: >> >> drivers/gpu/drm/i915/i915_drv.c:1179:5: warning: symbol >> 'i915_driver_load' was not declared. Should it be static? >> >> drivers/gpu/drm/i915/i915_drv.c:1267:6: warning: symbol >> 'i915_driver_unload' was not declared. Should it be static? >> >> drivers/gpu/drm/i915/i915_drv.c:2444:25: warning: symbol 'i915_pm_ops' >> was not declared. Should it be static? >> > > Hmm, Chris, were not these change in the middle of a series, did the > cleanup patches just fall off? Can we just please use whichever patches that fix the issue? BR, Jani. > > Regards, Joonas -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: keep declarations in i915_drv.h
On to, 2016-09-15 at 16:28 +0300, Jani Nikula wrote: > Fix sparse warnings: > > drivers/gpu/drm/i915/i915_drv.c:1179:5: warning: symbol > 'i915_driver_load' was not declared. Should it be static? > > drivers/gpu/drm/i915/i915_drv.c:1267:6: warning: symbol > 'i915_driver_unload' was not declared. Should it be static? > > drivers/gpu/drm/i915/i915_drv.c:2444:25: warning: symbol 'i915_pm_ops' > was not declared. Should it be static? > Hmm, Chris, were not these change in the middle of a series, did the cleanup patches just fall off? Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
