[Intel-gfx] [PATCH 0/2] drm/i915/gvt: clean up makefile

[Intel-gfx] [PATCH 0/2] drm/i915/gvt: clean up makefile

[Jani Nikula]

This is [1] rebased on gvt-next. (Which means it won't build on CI.)

BR,
Jani.


[1] https://patchwork.freedesktop.org/series/102003/


Cc: Zhi Wang <zhi.wang.linux@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>


Jani Nikula (2):
  drm/i915/gvt: fix trace TRACE_INCLUDE_PATH
  drm/i915/gvt: better align the Makefile with i915 Makefile

 drivers/gpu/drm/i915/Makefile     |  6 +++---
 drivers/gpu/drm/i915/gvt/Makefile | 30 +++++++++++++++++++++++-------
 drivers/gpu/drm/i915/gvt/trace.h  |  2 +-
 3 files changed, 27 insertions(+), 11 deletions(-)

-- 
2.30.2

[Intel-gfx] [PATCH 1/2] drm/i915/gvt: fix trace TRACE_INCLUDE_PATH

[Jani Nikula]

TRACE_INCLUDE_PATH should be a path relative to define_trace.h, not the
file including it. (See the comment in include/trace/define_trace.h.)

Cc: Zhi Wang <zhi.wang.linux@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/gvt/trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/trace.h b/drivers/gpu/drm/i915/gvt/trace.h
index 6d787750d279..020f1aa28322 100644
--- a/drivers/gpu/drm/i915/gvt/trace.h
+++ b/drivers/gpu/drm/i915/gvt/trace.h
@@ -377,7 +377,7 @@ TRACE_EVENT(render_mmio,
 
 /* This part must be out of protection */
 #undef TRACE_INCLUDE_PATH
-#define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_PATH ../../drivers/gpu/drm/i915/gvt
 #define TRACE_INCLUDE_FILE trace
 #include <trace/define_trace.h>
-- 
2.30.2

Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: fix trace TRACE_INCLUDE_PATH

[Christoph Hellwig]

On Wed, Apr 13, 2022 at 03:25:38PM +0300, Jani Nikula wrote:
> TRACE_INCLUDE_PATH should be a path relative to define_trace.h, not the
> file including it. (See the comment in include/trace/define_trace.h.)

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[Intel-gfx] [PATCH 2/2] drm/i915/gvt: better align the Makefile with i915 Makefile

[Jani Nikula]

Drop extra ccflags, drop extra intermediate variables, list object files
one per line alphabetically.

Cc: Zhi Wang <zhi.wang.linux@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/Makefile     |  6 +++---
 drivers/gpu/drm/i915/gvt/Makefile | 30 +++++++++++++++++++++++-------
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 75546b5f6994..b36864fae741 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -321,10 +321,10 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \
 # virtual gpu code
 i915-y += i915_vgpu.o
 
-ifeq ($(CONFIG_DRM_I915_GVT),y)
-i915-y += intel_gvt.o intel_gvt_mmio_table.o
+i915-$(CONFIG_DRM_I915_GVT) += \
+	intel_gvt.o \
+	intel_gvt_mmio_table.o
 include $(src)/gvt/Makefile
-endif
 
 obj-$(CONFIG_DRM_I915) += i915.o
 obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index ea8324abc784..584661047945 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -1,9 +1,25 @@
 # SPDX-License-Identifier: GPL-2.0
-GVT_DIR := gvt
-GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
-	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
-	execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o debugfs.o \
-	fb_decoder.o dmabuf.o page_track.o
 
-ccflags-y				+= -I $(srctree)/$(src) -I $(srctree)/$(src)/$(GVT_DIR)/
-i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
+i915-$(CONFIG_DRM_I915_GVT) += \
+	gvt/aperture_gm.o \
+	gvt/cfg_space.o \
+	gvt/cmd_parser.o \
+	gvt/debugfs.o \
+	gvt/display.o \
+	gvt/dmabuf.o \
+	gvt/edid.o \
+	gvt/execlist.o \
+	gvt/fb_decoder.o \
+	gvt/firmware.o \
+	gvt/gtt.o \
+	gvt/gvt.o \
+	gvt/handlers.o \
+	gvt/interrupt.o \
+	gvt/mmio.o \
+	gvt/mmio_context.o \
+	gvt/opregion.o \
+	gvt/page_track.o \
+	gvt/sched_policy.o \
+	gvt/scheduler.o \
+	gvt/trace_points.o \
+	gvt/vgpu.o
-- 
2.30.2

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gvt: better align the Makefile with i915 Makefile

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [Intel-gfx] [PATCH 0/2] drm/i915/gvt: clean up makefile

[Wang, Zhi A]

On 4/13/22 12:25 PM, Jani Nikula wrote:
> This is [1] rebased on gvt-next. (Which means it won't build on CI.)
> 
> BR,
> Jani.
> 
> 
> [1] https://patchwork.freedesktop.org/series/102003/
> 
> 
> Cc: Zhi Wang <zhi.wang.linux@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> 
> 
> Jani Nikula (2):
>   drm/i915/gvt: fix trace TRACE_INCLUDE_PATH
>   drm/i915/gvt: better align the Makefile with i915 Makefile
> 
>  drivers/gpu/drm/i915/Makefile     |  6 +++---
>  drivers/gpu/drm/i915/gvt/Makefile | 30 +++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/gvt/trace.h  |  2 +-
>  3 files changed, 27 insertions(+), 11 deletions(-)
> 

Thanks so much for the patch. 
Reviewed-by: Zhi Wang <zhi.a.wang@intel.com>

Re: [Intel-gfx] [PATCH 0/2] drm/i915/gvt: clean up makefile

[Christoph Hellwig]

On Wed, Apr 13, 2022 at 03:25:37PM +0300, Jani Nikula wrote:
> This is [1] rebased on gvt-next. (Which means it won't build on CI.)

Btw, now that I found that gvt-next branch:

Zhi, your commits still have per-commit changelog like:

v6:
 - Remove the reference of intel_gvt_device_info.(Christoph)
 - Refine the save_mmio() function. (Christoph)

which don't belong in the commit log.  It might be worth to fix that
before sending a pull request.

Re: [Intel-gfx] [PATCH 0/2] drm/i915/gvt: clean up makefile

[Jani Nikula]

On Wed, 13 Apr 2022, Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Apr 13, 2022 at 03:25:37PM +0300, Jani Nikula wrote:
>> This is [1] rebased on gvt-next. (Which means it won't build on CI.)
>
> Btw, now that I found that gvt-next branch:
>
> Zhi, your commits still have per-commit changelog like:
>
> v6:
>  - Remove the reference of intel_gvt_device_info.(Christoph)
>  - Refine the save_mmio() function. (Christoph)
>
> which don't belong in the commit log.  It might be worth to fix that
> before sending a pull request.

I'll just note, without any value judgement, that it is customary in the
drm land to do so. They are fine here even if they're not accepted in
other subsystems. I'm not going to insist one way or the other.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 0/2] drm/i915/gvt: clean up makefile

[Christoph Hellwig]

On Wed, Apr 13, 2022 at 07:04:54PM +0300, Jani Nikula wrote:
> > v6:
> >  - Remove the reference of intel_gvt_device_info.(Christoph)
> >  - Refine the save_mmio() function. (Christoph)
> >
> > which don't belong in the commit log.  It might be worth to fix that
> > before sending a pull request.
> 
> I'll just note, without any value judgement, that it is customary in the
> drm land to do so. They are fine here even if they're not accepted in
> other subsystems. I'm not going to insist one way or the other.

Ok.  Seems very odd to me, but I guess I don't spend enough time in
drm land to know this particular quirk.

Re: [Intel-gfx] [PATCH 0/2] drm/i915/gvt: clean up makefile

[Jason Gunthorpe]

On Wed, Apr 13, 2022 at 06:12:12PM +0200, Christoph Hellwig wrote:
> On Wed, Apr 13, 2022 at 07:04:54PM +0300, Jani Nikula wrote:
> > > v6:
> > >  - Remove the reference of intel_gvt_device_info.(Christoph)
> > >  - Refine the save_mmio() function. (Christoph)
> > >
> > > which don't belong in the commit log.  It might be worth to fix that
> > > before sending a pull request.
> > 
> > I'll just note, without any value judgement, that it is customary in the
> > drm land to do so. They are fine here even if they're not accepted in
> > other subsystems. I'm not going to insist one way or the other.
> 
> Ok.  Seems very odd to me, but I guess I don't spend enough time in
> drm land to know this particular quirk.

FWIW, I've seen netdev has being doing this too sometimes..

IMHO I prefer maintainers put the Link: headers then we can get the
changelog/etc from lore.

Jason