* [PATCH 0/5] Classify the engines in class + instance (v4) @ 2017-04-07 9:15 Oscar Mateo 2017-04-07 9:15 ` [PATCH 1/5] drm/i915: Classify the engines in class + instance Oscar Mateo ` (6 more replies) 0 siblings, 7 replies; 28+ messages in thread From: Oscar Mateo @ 2017-04-07 9:15 UTC (permalink / raw) To: intel-gfx This refactoring helps simplify a few things here and there. Daniele Ceraolo Spurio (2): drm/i915: Classify the engines in class + instance drm/i915: Use the engine class to get the context size Oscar Mateo (3): drm/i915: Use the same vfunc for BSD2 ring init drm/i915: Generate the engine name based on the instance number drm/i915: Split the engine info table in two levels, using class + instance drivers/gpu/drm/i915/i915_reg.h | 8 +++ drivers/gpu/drm/i915/intel_engine_cs.c | 80 ++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_lrc.c | 33 +++++++----- drivers/gpu/drm/i915/intel_lrc.h | 6 ++- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ----- drivers/gpu/drm/i915/intel_ringbuffer.h | 9 +++- drivers/gpu/drm/i915/selftests/mock_engine.c | 3 +- 7 files changed, 100 insertions(+), 53 deletions(-) -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/5] drm/i915: Classify the engines in class + instance 2017-04-07 9:15 [PATCH 0/5] Classify the engines in class + instance (v4) Oscar Mateo @ 2017-04-07 9:15 ` Oscar Mateo 2017-04-07 16:20 ` Michal Wajdeczko 2017-04-07 9:15 ` [PATCH 2/5] drm/i915: Use the same vfunc for BSD2 ring init Oscar Mateo ` (5 subsequent siblings) 6 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-07 9:15 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> In such a way that vcs and vcs2 are just two different instances (0 and 1) of the same engine class (VIDEO_DECODE_CLASS). v2: Align the instance types (Tvrtko) v3: Don't use enums for bspec-defined stuff (Michal) Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++ drivers/gpu/drm/i915/intel_engine_cs.c | 14 ++++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++++ 3 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 11b12f4..4c72ada 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -85,6 +85,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define VECS_HW 3 #define VCS2_HW 4 +/* Engine class */ + +#define RENDER_CLASS 0 +#define VIDEO_DECODE_CLASS 1 +#define VIDEO_ENHANCEMENT_CLASS 2 +#define COPY_ENGINE_CLASS 3 +#define OTHER_CLASS 4 + /* PCI config space */ #define MCHBAR_I915 0x44 diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 854e8e0..2d3456d 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -30,6 +30,8 @@ const char *name; unsigned int exec_id; unsigned int hw_id; + u8 class; + u8 instance; u32 mmio_base; unsigned irq_shift; int (*init_legacy)(struct intel_engine_cs *engine); @@ -39,6 +41,8 @@ .name = "rcs", .hw_id = RCS_HW, .exec_id = I915_EXEC_RENDER, + .class = RENDER_CLASS, + .instance = 0, .mmio_base = RENDER_RING_BASE, .irq_shift = GEN8_RCS_IRQ_SHIFT, .init_execlists = logical_render_ring_init, @@ -48,6 +52,8 @@ .name = "bcs", .hw_id = BCS_HW, .exec_id = I915_EXEC_BLT, + .class = COPY_ENGINE_CLASS, + .instance = 0, .mmio_base = BLT_RING_BASE, .irq_shift = GEN8_BCS_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, @@ -57,6 +63,8 @@ .name = "vcs", .hw_id = VCS_HW, .exec_id = I915_EXEC_BSD, + .class = VIDEO_DECODE_CLASS, + .instance = 0, .mmio_base = GEN6_BSD_RING_BASE, .irq_shift = GEN8_VCS1_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, @@ -66,6 +74,8 @@ .name = "vcs2", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, + .class = VIDEO_DECODE_CLASS, + .instance = 1, .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, @@ -75,6 +85,8 @@ .name = "vecs", .hw_id = VECS_HW, .exec_id = I915_EXEC_VEBOX, + .class = VIDEO_ENHANCEMENT_CLASS, + .instance = 0, .mmio_base = VEBOX_RING_BASE, .irq_shift = GEN8_VECS_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, @@ -101,6 +113,8 @@ engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; engine->irq_shift = info->irq_shift; + engine->class = info->class; + engine->instance = info->instance; /* Nothing to do here, execute in order of dependencies */ engine->schedule = NULL; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index cbe61d3..f54fe7d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -193,6 +193,10 @@ struct intel_engine_cs { enum intel_engine_id id; unsigned int exec_id; unsigned int hw_id; + + u8 class; + u8 instance; + unsigned int guc_id; u32 mmio_base; unsigned int irq_shift; -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] drm/i915: Classify the engines in class + instance 2017-04-07 9:15 ` [PATCH 1/5] drm/i915: Classify the engines in class + instance Oscar Mateo @ 2017-04-07 16:20 ` Michal Wajdeczko 0 siblings, 0 replies; 28+ messages in thread From: Michal Wajdeczko @ 2017-04-07 16:20 UTC (permalink / raw) To: Oscar Mateo; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi On Fri, Apr 07, 2017 at 02:15:45AM -0700, Oscar Mateo wrote: > From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > In such a way that vcs and vcs2 are just two different instances (0 and 1) > of the same engine class (VIDEO_DECODE_CLASS). > > v2: Align the instance types (Tvrtko) > > v3: Don't use enums for bspec-defined stuff (Michal) > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> -Michal _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/5] drm/i915: Use the same vfunc for BSD2 ring init 2017-04-07 9:15 [PATCH 0/5] Classify the engines in class + instance (v4) Oscar Mateo 2017-04-07 9:15 ` [PATCH 1/5] drm/i915: Classify the engines in class + instance Oscar Mateo @ 2017-04-07 9:15 ` Oscar Mateo 2017-04-07 9:15 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo ` (4 subsequent siblings) 6 siblings, 0 replies; 28+ messages in thread From: Oscar Mateo @ 2017-04-07 9:15 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi If we needed to do something different for the init functions, we could always look at the engine instance to make the distinction. But, in any case, the two functions are virtually identical already (please notice that BSD2_RING is only used from gen8 onwards). With this, the init functions depends excusively on the engine class (a fact that we will use soon). v2: Commit message Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 -------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 2d3456d..146d6b6 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -79,7 +79,7 @@ .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd2_ring_buffer, + .init_legacy = intel_init_bsd_ring_buffer, }, [VECS] = { .name = "vecs", diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index c98acc2..81eee42 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2175,20 +2175,6 @@ int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine) return intel_init_ring_buffer(engine); } -/** - * Initialize the second BSD ring (eg. Broadwell GT3, Skylake GT3) - */ -int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine) -{ - struct drm_i915_private *dev_priv = engine->i915; - - intel_ring_default_vfuncs(dev_priv, engine); - - engine->emit_flush = gen6_bsd_ring_flush; - - return intel_init_ring_buffer(engine); -} - int intel_init_blt_ring_buffer(struct intel_engine_cs *engine) { struct drm_i915_private *dev_priv = engine->i915; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index f54fe7d..8b53ddb 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -555,7 +555,6 @@ int intel_ring_pin(struct intel_ring *ring, int intel_init_render_ring_buffer(struct intel_engine_cs *engine); int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine); -int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine); int intel_init_blt_ring_buffer(struct intel_engine_cs *engine); int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine); -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-07 9:15 [PATCH 0/5] Classify the engines in class + instance (v4) Oscar Mateo 2017-04-07 9:15 ` [PATCH 1/5] drm/i915: Classify the engines in class + instance Oscar Mateo 2017-04-07 9:15 ` [PATCH 2/5] drm/i915: Use the same vfunc for BSD2 ring init Oscar Mateo @ 2017-04-07 9:15 ` Oscar Mateo 2017-04-07 16:23 ` Michal Wajdeczko 2017-04-07 9:15 ` [PATCH 4/5] drm/i915: Split the engine info table in two levels, using class + instance Oscar Mateo ` (3 subsequent siblings) 6 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-07 9:15 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi Not really needed, but makes the next change a little bit more compact. v2: - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) - Make sure the mock engine name is null-terminated (Tvrtko, Chris) v3: Because I'm stupid (Chris) v4: Verify engine name wasn't truncated (Michal) Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- drivers/gpu/drm/i915/selftests/mock_engine.c | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 146d6b6..e40355c 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -71,7 +71,7 @@ .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs2", + .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, @@ -108,7 +108,8 @@ engine->id = id; engine->i915 = dev_priv; - engine->name = info->name; + WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", + info->name, info->instance) >= sizeof(engine->name)); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 8b53ddb..fd8994b 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -187,9 +187,11 @@ enum intel_engine_id { VECS }; +#define INTEL_ENGINE_CS_MAX_NAME 8 + struct intel_engine_cs { struct drm_i915_private *i915; - const char *name; + char name[INTEL_ENGINE_CS_MAX_NAME]; enum intel_engine_id id; unsigned int exec_id; unsigned int hw_id; diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index b89050e..9adacfb 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -140,7 +140,8 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, /* minimal engine setup for requests */ engine->base.i915 = i915; - engine->base.name = name; + WARN_ON(snprintf(engine->base.name, sizeof(engine->base.name), "%s", + name) > sizeof(engine->base.name)); engine->base.id = id++; engine->base.status_page.page_addr = (void *)(engine + 1); -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-07 9:15 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo @ 2017-04-07 16:23 ` Michal Wajdeczko 2017-04-07 17:11 ` Chris Wilson 0 siblings, 1 reply; 28+ messages in thread From: Michal Wajdeczko @ 2017-04-07 16:23 UTC (permalink / raw) To: Oscar Mateo; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi On Fri, Apr 07, 2017 at 02:15:47AM -0700, Oscar Mateo wrote: > Not really needed, but makes the next change a little bit more compact. > > v2: > - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) > - Make sure the mock engine name is null-terminated (Tvrtko, Chris) > > v3: Because I'm stupid (Chris) > > v4: Verify engine name wasn't truncated (Michal) > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> -Michal _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-07 16:23 ` Michal Wajdeczko @ 2017-04-07 17:11 ` Chris Wilson 2017-04-07 12:08 ` Oscar Mateo 0 siblings, 1 reply; 28+ messages in thread From: Chris Wilson @ 2017-04-07 17:11 UTC (permalink / raw) To: Michal Wajdeczko; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi On Fri, Apr 07, 2017 at 06:23:14PM +0200, Michal Wajdeczko wrote: > On Fri, Apr 07, 2017 at 02:15:47AM -0700, Oscar Mateo wrote: > > Not really needed, but makes the next change a little bit more compact. > > > > v2: > > - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) > > - Make sure the mock engine name is null-terminated (Tvrtko, Chris) > > > > v3: Because I'm stupid (Chris) > > > > v4: Verify engine name wasn't truncated (Michal) > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > --- > > Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> s/WARN_ON/GEM_WARN_ON/ ? The WARN_ON in mock_engine.c is off-by-one, but really we don't care too much there so just kill that warn. -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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-07 17:11 ` Chris Wilson @ 2017-04-07 12:08 ` Oscar Mateo 0 siblings, 0 replies; 28+ messages in thread From: Oscar Mateo @ 2017-04-07 12:08 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi On 04/07/2017 10:11 AM, Chris Wilson wrote: > On Fri, Apr 07, 2017 at 06:23:14PM +0200, Michal Wajdeczko wrote: >> On Fri, Apr 07, 2017 at 02:15:47AM -0700, Oscar Mateo wrote: >>> Not really needed, but makes the next change a little bit more compact. >>> >>> v2: >>> - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) >>> - Make sure the mock engine name is null-terminated (Tvrtko, Chris) >>> >>> v3: Because I'm stupid (Chris) >>> >>> v4: Verify engine name wasn't truncated (Michal) >>> >>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >>> --- >> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > s/WARN_ON/GEM_WARN_ON/ ? As in: if (GEM_WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", class_info->name, info->instance) >= sizeof(engine->name))) ; or do you want to return an error? (I don't really see the need, but...) > The WARN_ON in mock_engine.c is off-by-one, but really we don't care too > much there so just kill that warn. > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/5] drm/i915: Split the engine info table in two levels, using class + instance 2017-04-07 9:15 [PATCH 0/5] Classify the engines in class + instance (v4) Oscar Mateo ` (2 preceding siblings ...) 2017-04-07 9:15 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo @ 2017-04-07 9:15 ` Oscar Mateo 2017-04-07 16:27 ` Michal Wajdeczko 2017-04-07 9:15 ` [PATCH 5/5] drm/i915: Use the engine class to get the context size Oscar Mateo ` (2 subsequent siblings) 6 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-07 9:15 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi There are some properties that logically belong to the engine class, and some that belong to the engine instance. Make it explicit. v2: Commit message (Tvrtko) v3: - Rebased - Exec/uabi id should be per instance (Chris) v4: - Rebased - Avoid re-ordering fields for smaller diff (Tvrtko) - Bug on oob access to the class array (Michal) Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 65 ++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index e40355c..4e2b5da 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -26,71 +26,84 @@ #include "intel_ringbuffer.h" #include "intel_lrc.h" -static const struct engine_info { +struct engine_class_info { const char *name; - unsigned int exec_id; + int (*init_legacy)(struct intel_engine_cs *engine); + int (*init_execlists)(struct intel_engine_cs *engine); +}; + +static const struct engine_class_info intel_engine_classes[] = { + [RENDER_CLASS] = { + .name = "rcs", + .init_execlists = logical_render_ring_init, + .init_legacy = intel_init_render_ring_buffer, + }, + [COPY_ENGINE_CLASS] = { + .name = "bcs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_blt_ring_buffer, + }, + [VIDEO_DECODE_CLASS] = { + .name = "vcs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_bsd_ring_buffer, + }, + [VIDEO_ENHANCEMENT_CLASS] = { + .name = "vecs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_vebox_ring_buffer, + }, +}; + +struct engine_info { unsigned int hw_id; + unsigned int exec_id; u8 class; u8 instance; u32 mmio_base; unsigned irq_shift; - int (*init_legacy)(struct intel_engine_cs *engine); - int (*init_execlists)(struct intel_engine_cs *engine); -} intel_engines[] = { +}; + +static const struct engine_info intel_engines[] = { [RCS] = { - .name = "rcs", .hw_id = RCS_HW, .exec_id = I915_EXEC_RENDER, .class = RENDER_CLASS, .instance = 0, .mmio_base = RENDER_RING_BASE, .irq_shift = GEN8_RCS_IRQ_SHIFT, - .init_execlists = logical_render_ring_init, - .init_legacy = intel_init_render_ring_buffer, }, [BCS] = { - .name = "bcs", .hw_id = BCS_HW, .exec_id = I915_EXEC_BLT, .class = COPY_ENGINE_CLASS, .instance = 0, .mmio_base = BLT_RING_BASE, .irq_shift = GEN8_BCS_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_blt_ring_buffer, }, [VCS] = { - .name = "vcs", .hw_id = VCS_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 0, .mmio_base = GEN6_BSD_RING_BASE, .irq_shift = GEN8_VCS1_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 1, .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd_ring_buffer, }, [VECS] = { - .name = "vecs", .hw_id = VECS_HW, .exec_id = I915_EXEC_VEBOX, .class = VIDEO_ENHANCEMENT_CLASS, .instance = 0, .mmio_base = VEBOX_RING_BASE, .irq_shift = GEN8_VECS_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_vebox_ring_buffer, }, }; @@ -99,8 +112,12 @@ enum intel_engine_id id) { const struct engine_info *info = &intel_engines[id]; + const struct engine_class_info *class_info; struct intel_engine_cs *engine; + GEM_BUG_ON(info->class > ARRAY_SIZE(intel_engine_classes)); + class_info = &intel_engine_classes[info->class]; + GEM_BUG_ON(dev_priv->engine[id]); engine = kzalloc(sizeof(*engine), GFP_KERNEL); if (!engine) @@ -109,7 +126,7 @@ engine->id = id; engine->i915 = dev_priv; WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", - info->name, info->instance) >= sizeof(engine->name)); + class_info->name, info->instance) >= sizeof(engine->name)); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; @@ -190,12 +207,14 @@ int intel_engines_init(struct drm_i915_private *dev_priv) int err = 0; for_each_engine(engine, dev_priv, id) { + const struct engine_class_info *class_info = + &intel_engine_classes[engine->class]; int (*init)(struct intel_engine_cs *engine); if (i915.enable_execlists) - init = intel_engines[id].init_execlists; + init = class_info->init_execlists; else - init = intel_engines[id].init_legacy; + init = class_info->init_legacy; if (!init) { kfree(engine); dev_priv->engine[id] = NULL; -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] drm/i915: Split the engine info table in two levels, using class + instance 2017-04-07 9:15 ` [PATCH 4/5] drm/i915: Split the engine info table in two levels, using class + instance Oscar Mateo @ 2017-04-07 16:27 ` Michal Wajdeczko 2017-04-07 9:34 ` [PATCH v5] " Oscar Mateo 0 siblings, 1 reply; 28+ messages in thread From: Michal Wajdeczko @ 2017-04-07 16:27 UTC (permalink / raw) To: Oscar Mateo; +Cc: Paulo Zanoni, intel-gfx, Rodrigo Vivi On Fri, Apr 07, 2017 at 02:15:48AM -0700, Oscar Mateo wrote: > There are some properties that logically belong to the engine class, and some > that belong to the engine instance. Make it explicit. > > v2: Commit message (Tvrtko) > > v3: > - Rebased > - Exec/uabi id should be per instance (Chris) > > v4: > - Rebased > - Avoid re-ordering fields for smaller diff (Tvrtko) > - Bug on oob access to the class array (Michal) > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- <snip> > @@ -99,8 +112,12 @@ > enum intel_engine_id id) > { > const struct engine_info *info = &intel_engines[id]; > + const struct engine_class_info *class_info; > struct intel_engine_cs *engine; > > + GEM_BUG_ON(info->class > ARRAY_SIZE(intel_engine_classes)); > + class_info = &intel_engine_classes[info->class]; > + Hmm, I think we should check against info->class >= ARRAY_SIZE, as index 4 can't be used on array[4] -Michal _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5] drm/i915: Split the engine info table in two levels, using class + instance 2017-04-07 16:27 ` Michal Wajdeczko @ 2017-04-07 9:34 ` Oscar Mateo 2017-04-07 16:37 ` Michal Wajdeczko 0 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-07 9:34 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi There are some properties that logically belong to the engine class, and some that belong to the engine instance. Make it explicit. v2: Commit message (Tvrtko) v3: - Rebased - Exec/uabi id should be per instance (Chris) v4: - Rebased - Avoid re-ordering fields for smaller diff (Tvrtko) - Bug on oob access to the class array (Michal) v5: Bug on the right thing (Michal) Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 65 ++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index e40355c..37bd008 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -26,71 +26,84 @@ #include "intel_ringbuffer.h" #include "intel_lrc.h" -static const struct engine_info { +struct engine_class_info { const char *name; - unsigned int exec_id; + int (*init_legacy)(struct intel_engine_cs *engine); + int (*init_execlists)(struct intel_engine_cs *engine); +}; + +static const struct engine_class_info intel_engine_classes[] = { + [RENDER_CLASS] = { + .name = "rcs", + .init_execlists = logical_render_ring_init, + .init_legacy = intel_init_render_ring_buffer, + }, + [COPY_ENGINE_CLASS] = { + .name = "bcs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_blt_ring_buffer, + }, + [VIDEO_DECODE_CLASS] = { + .name = "vcs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_bsd_ring_buffer, + }, + [VIDEO_ENHANCEMENT_CLASS] = { + .name = "vecs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_vebox_ring_buffer, + }, +}; + +struct engine_info { unsigned int hw_id; + unsigned int exec_id; u8 class; u8 instance; u32 mmio_base; unsigned irq_shift; - int (*init_legacy)(struct intel_engine_cs *engine); - int (*init_execlists)(struct intel_engine_cs *engine); -} intel_engines[] = { +}; + +static const struct engine_info intel_engines[] = { [RCS] = { - .name = "rcs", .hw_id = RCS_HW, .exec_id = I915_EXEC_RENDER, .class = RENDER_CLASS, .instance = 0, .mmio_base = RENDER_RING_BASE, .irq_shift = GEN8_RCS_IRQ_SHIFT, - .init_execlists = logical_render_ring_init, - .init_legacy = intel_init_render_ring_buffer, }, [BCS] = { - .name = "bcs", .hw_id = BCS_HW, .exec_id = I915_EXEC_BLT, .class = COPY_ENGINE_CLASS, .instance = 0, .mmio_base = BLT_RING_BASE, .irq_shift = GEN8_BCS_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_blt_ring_buffer, }, [VCS] = { - .name = "vcs", .hw_id = VCS_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 0, .mmio_base = GEN6_BSD_RING_BASE, .irq_shift = GEN8_VCS1_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 1, .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd_ring_buffer, }, [VECS] = { - .name = "vecs", .hw_id = VECS_HW, .exec_id = I915_EXEC_VEBOX, .class = VIDEO_ENHANCEMENT_CLASS, .instance = 0, .mmio_base = VEBOX_RING_BASE, .irq_shift = GEN8_VECS_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_vebox_ring_buffer, }, }; @@ -99,8 +112,12 @@ enum intel_engine_id id) { const struct engine_info *info = &intel_engines[id]; + const struct engine_class_info *class_info; struct intel_engine_cs *engine; + GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes)); + class_info = &intel_engine_classes[info->class]; + GEM_BUG_ON(dev_priv->engine[id]); engine = kzalloc(sizeof(*engine), GFP_KERNEL); if (!engine) @@ -109,7 +126,7 @@ engine->id = id; engine->i915 = dev_priv; WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", - info->name, info->instance) >= sizeof(engine->name)); + class_info->name, info->instance) >= sizeof(engine->name)); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; @@ -190,12 +207,14 @@ int intel_engines_init(struct drm_i915_private *dev_priv) int err = 0; for_each_engine(engine, dev_priv, id) { + const struct engine_class_info *class_info = + &intel_engine_classes[engine->class]; int (*init)(struct intel_engine_cs *engine); if (i915.enable_execlists) - init = intel_engines[id].init_execlists; + init = class_info->init_execlists; else - init = intel_engines[id].init_legacy; + init = class_info->init_legacy; if (!init) { kfree(engine); dev_priv->engine[id] = NULL; -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v5] drm/i915: Split the engine info table in two levels, using class + instance 2017-04-07 9:34 ` [PATCH v5] " Oscar Mateo @ 2017-04-07 16:37 ` Michal Wajdeczko 0 siblings, 0 replies; 28+ messages in thread From: Michal Wajdeczko @ 2017-04-07 16:37 UTC (permalink / raw) To: Oscar Mateo; +Cc: Paulo Zanoni, intel-gfx, Rodrigo Vivi On Fri, Apr 07, 2017 at 02:34:54AM -0700, Oscar Mateo wrote: > There are some properties that logically belong to the engine class, and some > that belong to the engine instance. Make it explicit. > > v2: Commit message (Tvrtko) > > v3: > - Rebased > - Exec/uabi id should be per instance (Chris) > > v4: > - Rebased > - Avoid re-ordering fields for smaller diff (Tvrtko) > - Bug on oob access to the class array (Michal) > > v5: Bug on the right thing (Michal) > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> -Michal _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/5] drm/i915: Use the engine class to get the context size 2017-04-07 9:15 [PATCH 0/5] Classify the engines in class + instance (v4) Oscar Mateo ` (3 preceding siblings ...) 2017-04-07 9:15 ` [PATCH 4/5] drm/i915: Split the engine info table in two levels, using class + instance Oscar Mateo @ 2017-04-07 9:15 ` Oscar Mateo 2017-04-07 16:35 ` ✓ Fi.CI.BAT: success for Classify the engines in class + instance (rev5) Patchwork 2017-04-07 16:51 ` ✓ Fi.CI.BAT: success for Classify the engines in class + instance (rev6) Patchwork 6 siblings, 0 replies; 28+ messages in thread From: Oscar Mateo @ 2017-04-07 9:15 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Technically speaking, the context size is per engine class, not per instance. v2: Add MISSING_CASE (Tvrtko) v3: Rebased Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 33 +++++++++++++++++++++------------ drivers/gpu/drm/i915/intel_lrc.h | 6 +++++- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 0dc1cc4..1c6672c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1908,8 +1908,10 @@ static void execlists_init_reg_state(u32 *regs, } /** - * intel_lr_context_size() - return the size of the context for an engine - * @engine: which engine to find the context size for + * intel_lr_class_context_size() - return the size of the context for a given + * engine class + * @dev_priv: i915 device private + * @class: which engine class to find the context size for * * Each engine may require a different amount of space for a context image, * so when allocating (or copying) an image, this function can be used to @@ -1921,25 +1923,32 @@ static void execlists_init_reg_state(u32 *regs, * in LRC mode, but does not include the "shared data page" used with * GuC submission. The caller should account for this if using the GuC. */ -uint32_t intel_lr_context_size(struct intel_engine_cs *engine) +uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class) { int ret = 0; - WARN_ON(INTEL_GEN(engine->i915) < 8); + WARN_ON(INTEL_GEN(dev_priv) < 8); - switch (engine->id) { - case RCS: - if (INTEL_GEN(engine->i915) >= 9) + switch (class) { + case RENDER_CLASS: + switch (INTEL_GEN(dev_priv)) { + default: + MISSING_CASE(INTEL_GEN(dev_priv)); + case 9: ret = GEN9_LR_CONTEXT_RENDER_SIZE; - else + break; + case 8: ret = GEN8_LR_CONTEXT_RENDER_SIZE; + break; + } break; - case VCS: - case BCS: - case VECS: - case VCS2: + case VIDEO_DECODE_CLASS: + case VIDEO_ENHANCEMENT_CLASS: + case COPY_ENGINE_CLASS: ret = GEN8_LR_CONTEXT_OTHER_SIZE; break; + default: + MISSING_CASE(class); } return ret; diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index e8015e7..bde2b6e 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -78,7 +78,11 @@ enum { struct drm_i915_private; struct i915_gem_context; -uint32_t intel_lr_context_size(struct intel_engine_cs *engine); +uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class); +static inline uint32_t intel_lr_context_size(struct intel_engine_cs *engine) +{ + return intel_lr_class_context_size(engine->i915, engine->class); +} void intel_lr_context_resume(struct drm_i915_private *dev_priv); uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx, -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* ✓ Fi.CI.BAT: success for Classify the engines in class + instance (rev5) 2017-04-07 9:15 [PATCH 0/5] Classify the engines in class + instance (v4) Oscar Mateo ` (4 preceding siblings ...) 2017-04-07 9:15 ` [PATCH 5/5] drm/i915: Use the engine class to get the context size Oscar Mateo @ 2017-04-07 16:35 ` Patchwork 2017-04-07 16:51 ` ✓ Fi.CI.BAT: success for Classify the engines in class + instance (rev6) Patchwork 6 siblings, 0 replies; 28+ messages in thread From: Patchwork @ 2017-04-07 16:35 UTC (permalink / raw) To: Oscar Mateo; +Cc: intel-gfx == Series Details == Series: Classify the engines in class + instance (rev5) URL : https://patchwork.freedesktop.org/series/22535/ State : success == Summary == Series 22535v5 Classify the engines in class + instance https://patchwork.freedesktop.org/api/1.0/series/22535/revisions/5/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:430s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:432s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:578s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:503s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:546s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:486s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:483s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:407s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:428s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:484s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:487s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:457s fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:569s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:448s fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:569s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:468s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:498s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:435s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:526s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:415s 5a74def419415233546c4e853a3f02bb31daee82 drm-tip: 2017y-04m-07d-15h-00m-28s UTC integration manifest b4acaec drm/i915: Use the engine class to get the context size c1e7ada drm/i915: Split the engine info table in two levels, using class + instance e62b796 drm/i915: Generate the engine name based on the instance number a70545a2 drm/i915: Use the same vfunc for BSD2 ring init 190b146 drm/i915: Classify the engines in class + instance == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4442/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* ✓ Fi.CI.BAT: success for Classify the engines in class + instance (rev6) 2017-04-07 9:15 [PATCH 0/5] Classify the engines in class + instance (v4) Oscar Mateo ` (5 preceding siblings ...) 2017-04-07 16:35 ` ✓ Fi.CI.BAT: success for Classify the engines in class + instance (rev5) Patchwork @ 2017-04-07 16:51 ` Patchwork 6 siblings, 0 replies; 28+ messages in thread From: Patchwork @ 2017-04-07 16:51 UTC (permalink / raw) To: Oscar Mateo; +Cc: intel-gfx == Series Details == Series: Classify the engines in class + instance (rev6) URL : https://patchwork.freedesktop.org/series/22535/ State : success == Summary == Series 22535v6 Classify the engines in class + instance https://patchwork.freedesktop.org/api/1.0/series/22535/revisions/6/mbox/ Test gem_exec_suspend: Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125 fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:430s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:424s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:571s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:506s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:538s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:485s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:476s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:407s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:401s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:426s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:484s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:466s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:456s fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:567s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:447s fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:565s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:457s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:492s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:431s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:534s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:407s 5a74def419415233546c4e853a3f02bb31daee82 drm-tip: 2017y-04m-07d-15h-00m-28s UTC integration manifest 2765f03 drm/i915: Use the engine class to get the context size 9ed6b00 drm/i915: Split the engine info table in two levels, using class + instance 3ca1bf6 drm/i915: Generate the engine name based on the instance number e9ca4d4 drm/i915: Use the same vfunc for BSD2 ring init d29d43e drm/i915: Classify the engines in class + instance == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4443/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/5] Classify the engines in class + instance (v5) @ 2017-04-10 14:34 Oscar Mateo 2017-04-10 14:34 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 0 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-10 14:34 UTC (permalink / raw) To: intel-gfx This refactoring helps simplify a few things here and there. Daniele Ceraolo Spurio (2): drm/i915: Classify the engines in class + instance drm/i915: Use the engine class to get the context size Oscar Mateo (3): drm/i915: Use the same vfunc for BSD2 ring init drm/i915: Generate the engine name based on the instance number drm/i915: Split the engine info table in two levels, using class + instance drivers/gpu/drm/i915/i915_reg.h | 8 +++ drivers/gpu/drm/i915/intel_engine_cs.c | 80 ++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_lrc.c | 33 +++++++----- drivers/gpu/drm/i915/intel_lrc.h | 6 ++- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ----- drivers/gpu/drm/i915/intel_ringbuffer.h | 9 +++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 7 files changed, 99 insertions(+), 53 deletions(-) -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-10 14:34 [PATCH 0/5] Classify the engines in class + instance (v5) Oscar Mateo @ 2017-04-10 14:34 ` Oscar Mateo 0 siblings, 0 replies; 28+ messages in thread From: Oscar Mateo @ 2017-04-10 14:34 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi Not really needed, but makes the next change a little bit more compact. v2: - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) - Make sure the mock engine name is null-terminated (Tvrtko, Chris) v3: Because I'm stupid (Chris) v4: Verify engine name wasn't truncated (Michal) v5: - Kill the warning in mock engine (Chris) Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a7ffa4c..5e5cda0 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -71,7 +71,7 @@ .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs2", + .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, @@ -108,7 +108,8 @@ engine->id = id; engine->i915 = dev_priv; - engine->name = info->name; + WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", + info->name, info->instance) >= sizeof(engine->name)); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 8b53ddb..fd8994b 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -187,9 +187,11 @@ enum intel_engine_id { VECS }; +#define INTEL_ENGINE_CS_MAX_NAME 8 + struct intel_engine_cs { struct drm_i915_private *i915; - const char *name; + char name[INTEL_ENGINE_CS_MAX_NAME]; enum intel_engine_id id; unsigned int exec_id; unsigned int hw_id; diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index b89050e..b8e53bd 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, /* minimal engine setup for requests */ engine->base.i915 = i915; - engine->base.name = name; + snprintf(engine->base.name, sizeof(engine->base.name), "%s", name); engine->base.id = id++; engine->base.status_page.page_addr = (void *)(engine + 1); -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 0/5] Classify the engines in class + instance (v3) @ 2017-04-06 15:00 Oscar Mateo 2017-04-06 15:00 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 0 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-06 15:00 UTC (permalink / raw) To: intel-gfx This refactoring helps simplify a few things here and there. Daniele Ceraolo Spurio (2): drm/i915: Classify the engines in class + instance drm/i915: Use the engine class to get the context size Oscar Mateo (3): drm/i915: Use the same vfunc for BSD2 ring init drm/i915: Generate the engine name based on the instance number drm/i915: Split the engine info table in two levels, using class + instance drivers/gpu/drm/i915/intel_engine_cs.c | 86 +++++++++++++++++++--------- drivers/gpu/drm/i915/intel_lrc.c | 34 +++++++---- drivers/gpu/drm/i915/intel_lrc.h | 7 ++- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ----- drivers/gpu/drm/i915/intel_ringbuffer.h | 15 ++++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 6 files changed, 101 insertions(+), 57 deletions(-) -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 15:00 [PATCH 0/5] Classify the engines in class + instance (v3) Oscar Mateo @ 2017-04-06 15:00 ` Oscar Mateo 2017-04-07 8:12 ` Tvrtko Ursulin 2017-04-07 10:31 ` Michal Wajdeczko 0 siblings, 2 replies; 28+ messages in thread From: Oscar Mateo @ 2017-04-06 15:00 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi Not really needed, but makes the next change a little bit more compact. v2: - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) - Make sure the mock engine name is null-terminated (Tvrtko, Chris) v3: Because I'm stupid (Chris) Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index bb10847..c6a73d0 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -71,7 +71,7 @@ .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs2", + .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, @@ -108,7 +108,8 @@ engine->id = id; engine->i915 = dev_priv; - engine->name = info->name; + snprintf(engine->name, sizeof(engine->name), "%s%u", + info->name, info->instance); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 5c1a27f..d617049 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -187,9 +187,11 @@ enum intel_engine_id { VECS }; +#define INTEL_ENGINE_CS_MAX_NAME 8 + struct intel_engine_cs { struct drm_i915_private *i915; - const char *name; + char name[INTEL_ENGINE_CS_MAX_NAME]; enum intel_engine_id id; unsigned int exec_id; unsigned int hw_id; diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index b89050e..b8e53bd 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, /* minimal engine setup for requests */ engine->base.i915 = i915; - engine->base.name = name; + snprintf(engine->base.name, sizeof(engine->base.name), "%s", name); engine->base.id = id++; engine->base.status_page.page_addr = (void *)(engine + 1); -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 15:00 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo @ 2017-04-07 8:12 ` Tvrtko Ursulin 2017-04-07 10:31 ` Michal Wajdeczko 1 sibling, 0 replies; 28+ messages in thread From: Tvrtko Ursulin @ 2017-04-07 8:12 UTC (permalink / raw) To: Oscar Mateo, intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi On 06/04/2017 16:00, Oscar Mateo wrote: > Not really needed, but makes the next change a little bit more compact. > > v2: > - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) > - Make sure the mock engine name is null-terminated (Tvrtko, Chris) > > v3: Because I'm stupid (Chris) > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++-- > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- > drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- > 3 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index bb10847..c6a73d0 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -71,7 +71,7 @@ > .init_legacy = intel_init_bsd_ring_buffer, > }, > [VCS2] = { > - .name = "vcs2", > + .name = "vcs", > .hw_id = VCS2_HW, > .exec_id = I915_EXEC_BSD, > .class = VIDEO_DECODE_CLASS, > @@ -108,7 +108,8 @@ > > engine->id = id; > engine->i915 = dev_priv; > - engine->name = info->name; > + snprintf(engine->name, sizeof(engine->name), "%s%u", > + info->name, info->instance); > engine->exec_id = info->exec_id; > engine->hw_id = engine->guc_id = info->hw_id; > engine->mmio_base = info->mmio_base; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 5c1a27f..d617049 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -187,9 +187,11 @@ enum intel_engine_id { > VECS > }; > > +#define INTEL_ENGINE_CS_MAX_NAME 8 > + > struct intel_engine_cs { > struct drm_i915_private *i915; > - const char *name; > + char name[INTEL_ENGINE_CS_MAX_NAME]; > enum intel_engine_id id; > unsigned int exec_id; > unsigned int hw_id; > diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c > index b89050e..b8e53bd 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_engine.c > +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c > @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, > > /* minimal engine setup for requests */ > engine->base.i915 = i915; > - engine->base.name = name; > + snprintf(engine->base.name, sizeof(engine->base.name), "%s", name); > engine->base.id = id++; > engine->base.status_page.page_addr = (void *)(engine + 1); > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 15:00 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 2017-04-07 8:12 ` Tvrtko Ursulin @ 2017-04-07 10:31 ` Michal Wajdeczko 1 sibling, 0 replies; 28+ messages in thread From: Michal Wajdeczko @ 2017-04-07 10:31 UTC (permalink / raw) To: Oscar Mateo; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi On Thu, Apr 06, 2017 at 08:00:14AM -0700, Oscar Mateo wrote: > Not really needed, but makes the next change a little bit more compact. > > v2: > - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) > - Make sure the mock engine name is null-terminated (Tvrtko, Chris) > > v3: Because I'm stupid (Chris) > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++-- > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- > drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- > 3 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index bb10847..c6a73d0 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -71,7 +71,7 @@ > .init_legacy = intel_init_bsd_ring_buffer, > }, > [VCS2] = { > - .name = "vcs2", > + .name = "vcs", > .hw_id = VCS2_HW, > .exec_id = I915_EXEC_BSD, > .class = VIDEO_DECODE_CLASS, > @@ -108,7 +108,8 @@ > > engine->id = id; > engine->i915 = dev_priv; > - engine->name = info->name; > + snprintf(engine->name, sizeof(engine->name), "%s%u", > + info->name, info->instance); Maybe we should verify that engine name was not truncated by the snprintf? -Michal > engine->exec_id = info->exec_id; > engine->hw_id = engine->guc_id = info->hw_id; > engine->mmio_base = info->mmio_base; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 5c1a27f..d617049 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -187,9 +187,11 @@ enum intel_engine_id { > VECS > }; > > +#define INTEL_ENGINE_CS_MAX_NAME 8 > + > struct intel_engine_cs { > struct drm_i915_private *i915; > - const char *name; > + char name[INTEL_ENGINE_CS_MAX_NAME]; > enum intel_engine_id id; > unsigned int exec_id; > unsigned int hw_id; > diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c > index b89050e..b8e53bd 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_engine.c > +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c > @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, > > /* minimal engine setup for requests */ > engine->base.i915 = i915; > - engine->base.name = name; > + snprintf(engine->base.name, sizeof(engine->base.name), "%s", name); > engine->base.id = id++; > engine->base.status_page.page_addr = (void *)(engine + 1); > > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/5] Classify the engines in class + instance (v2) @ 2017-04-06 12:55 Oscar Mateo 2017-04-06 12:55 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 0 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-06 12:55 UTC (permalink / raw) To: intel-gfx This refactoring helps simplify a few things here and there. Daniele Ceraolo Spurio (2): drm/i915: Classify the engines in class + instance drm/i915: Use the engine class to get the context size Oscar Mateo (3): drm/i915: Use the same vfunc for BSD2 ring init drm/i915: Generate the engine name based on the instance number drm/i915: Split the engine info table in two levels, using class + instance drivers/gpu/drm/i915/intel_engine_cs.c | 88 +++++++++++++++++++--------- drivers/gpu/drm/i915/intel_lrc.c | 34 +++++++---- drivers/gpu/drm/i915/intel_lrc.h | 7 ++- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ----- drivers/gpu/drm/i915/intel_ringbuffer.h | 15 ++++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 6 files changed, 102 insertions(+), 58 deletions(-) -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 12:55 [PATCH 0/5] Classify the engines in class + instance (v2) Oscar Mateo @ 2017-04-06 12:55 ` Oscar Mateo 2017-04-06 20:10 ` Chris Wilson 0 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-06 12:55 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi Not really needed, but makes the next change a little bit more compact. v2: - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) - Make sure the mock engine name is null-terminated (Tvrtko, Chris) Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 6 ++++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index bb10847..2409908 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -71,7 +71,7 @@ .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs2", + .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, @@ -100,6 +100,7 @@ { const struct engine_info *info = &intel_engines[id]; struct intel_engine_cs *engine; + char instance[3] = ""; GEM_BUG_ON(dev_priv->engine[id]); engine = kzalloc(sizeof(*engine), GFP_KERNEL); @@ -108,7 +109,8 @@ engine->id = id; engine->i915 = dev_priv; - engine->name = info->name; + snprintf(instance, sizeof(instance), "%u", info->instance); + snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, instance); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 5c1a27f..d617049 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -187,9 +187,11 @@ enum intel_engine_id { VECS }; +#define INTEL_ENGINE_CS_MAX_NAME 8 + struct intel_engine_cs { struct drm_i915_private *i915; - const char *name; + char name[INTEL_ENGINE_CS_MAX_NAME]; enum intel_engine_id id; unsigned int exec_id; unsigned int hw_id; diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index b89050e..b8e53bd 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, /* minimal engine setup for requests */ engine->base.i915 = i915; - engine->base.name = name; + snprintf(engine->base.name, sizeof(engine->base.name), "%s", name); engine->base.id = id++; engine->base.status_page.page_addr = (void *)(engine + 1); -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 12:55 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo @ 2017-04-06 20:10 ` Chris Wilson 2017-04-06 13:15 ` Oscar Mateo 0 siblings, 1 reply; 28+ messages in thread From: Chris Wilson @ 2017-04-06 20:10 UTC (permalink / raw) To: Oscar Mateo; +Cc: intel-gfx, Rodrigo Vivi, Paulo Zanoni On Thu, Apr 06, 2017 at 05:55:42AM -0700, Oscar Mateo wrote: > Not really needed, but makes the next change a little bit more compact. > > v2: > - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) > - Make sure the mock engine name is null-terminated (Tvrtko, Chris) > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 6 ++++-- > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- > drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- > 3 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index bb10847..2409908 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -71,7 +71,7 @@ > .init_legacy = intel_init_bsd_ring_buffer, > }, > [VCS2] = { > - .name = "vcs2", > + .name = "vcs", > .hw_id = VCS2_HW, > .exec_id = I915_EXEC_BSD, > .class = VIDEO_DECODE_CLASS, > @@ -100,6 +100,7 @@ > { > const struct engine_info *info = &intel_engines[id]; > struct intel_engine_cs *engine; > + char instance[3] = ""; > > GEM_BUG_ON(dev_priv->engine[id]); > engine = kzalloc(sizeof(*engine), GFP_KERNEL); > @@ -108,7 +109,8 @@ > > engine->id = id; > engine->i915 = dev_priv; > - engine->name = info->name; > + snprintf(instance, sizeof(instance), "%u", info->instance); > + snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, instance); Huh? Please explain why not "%s%u", I'm feeling stupid. -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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 20:10 ` Chris Wilson @ 2017-04-06 13:15 ` Oscar Mateo 0 siblings, 0 replies; 28+ messages in thread From: Oscar Mateo @ 2017-04-06 13:15 UTC (permalink / raw) To: Chris Wilson, intel-gfx, Tvrtko Ursulin, Paulo Zanoni, Rodrigo Vivi, Daniele Ceraolo Spurio On 04/06/2017 01:10 PM, Chris Wilson wrote: > On Thu, Apr 06, 2017 at 05:55:42AM -0700, Oscar Mateo wrote: >> Not really needed, but makes the next change a little bit more compact. >> >> v2: >> - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) >> - Make sure the mock engine name is null-terminated (Tvrtko, Chris) >> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >> --- >> drivers/gpu/drm/i915/intel_engine_cs.c | 6 ++++-- >> drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- >> drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- >> 3 files changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c >> index bb10847..2409908 100644 >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >> @@ -71,7 +71,7 @@ >> .init_legacy = intel_init_bsd_ring_buffer, >> }, >> [VCS2] = { >> - .name = "vcs2", >> + .name = "vcs", >> .hw_id = VCS2_HW, >> .exec_id = I915_EXEC_BSD, >> .class = VIDEO_DECODE_CLASS, >> @@ -100,6 +100,7 @@ >> { >> const struct engine_info *info = &intel_engines[id]; >> struct intel_engine_cs *engine; >> + char instance[3] = ""; >> >> GEM_BUG_ON(dev_priv->engine[id]); >> engine = kzalloc(sizeof(*engine), GFP_KERNEL); >> @@ -108,7 +109,8 @@ >> >> engine->id = id; >> engine->i915 = dev_priv; >> - engine->name = info->name; >> + snprintf(instance, sizeof(instance), "%u", info->instance); >> + snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, instance); > Huh? Please explain why not "%s%u", I'm feeling stupid. > -Chris > No, I am the one feeling stupid. Please ignore... _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/5] Classify the engines in class + instance @ 2017-04-05 9:30 Oscar Mateo 2017-04-05 9:30 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 0 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-05 9:30 UTC (permalink / raw) To: intel-gfx This refactoring helps simplify a few things here and there. Daniele Ceraolo Spurio (2): drm/i915: Classify the engines in class + instance drm/i915: Use the engine class to get the context size Oscar Mateo (3): drm/i915: Use the same vfunc for BSD2 ring init drm/i915: Generate the engine name based on the instance number drm/i915: Split the engine info table in two levels, using class + instance drivers/gpu/drm/i915/intel_engine_cs.c | 90 +++++++++++++++++++--------- drivers/gpu/drm/i915/intel_lrc.c | 34 +++++++---- drivers/gpu/drm/i915/intel_lrc.h | 7 ++- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ----- drivers/gpu/drm/i915/intel_ringbuffer.h | 15 ++++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 6 files changed, 104 insertions(+), 58 deletions(-) -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-05 9:30 [PATCH 0/5] Classify the engines in class + instance Oscar Mateo @ 2017-04-05 9:30 ` Oscar Mateo 2017-04-06 18:02 ` Tvrtko Ursulin 0 siblings, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-05 9:30 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi Not really needed, but makes the next change a little bit more compact. Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 8 ++++++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index abc0a9c..530f822 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -71,7 +71,7 @@ .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs2", + .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, @@ -100,6 +100,7 @@ { const struct engine_info *info = &intel_engines[id]; struct intel_engine_cs *engine; + char instance[3] = ""; GEM_BUG_ON(dev_priv->engine[id]); engine = kzalloc(sizeof(*engine), GFP_KERNEL); @@ -108,7 +109,10 @@ engine->id = id; engine->i915 = dev_priv; - engine->name = info->name; + /* For historical reasons the engines are called: name, name2... */ + if (info->instance) + snprintf(instance, sizeof(instance), "%u", info->instance + 1); + snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, instance); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 5c1a27f..d617049 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -187,9 +187,11 @@ enum intel_engine_id { VECS }; +#define INTEL_ENGINE_CS_MAX_NAME 8 + struct intel_engine_cs { struct drm_i915_private *i915; - const char *name; + char name[INTEL_ENGINE_CS_MAX_NAME]; enum intel_engine_id id; unsigned int exec_id; unsigned int hw_id; diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index b89050e..4a1ffca 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, /* minimal engine setup for requests */ engine->base.i915 = i915; - engine->base.name = name; + strncpy(engine->base.name, name, sizeof(engine->base.name) - 1); engine->base.id = id++; engine->base.status_page.page_addr = (void *)(engine + 1); -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-05 9:30 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo @ 2017-04-06 18:02 ` Tvrtko Ursulin 2017-04-06 11:22 ` Oscar Mateo 2017-04-06 18:10 ` Chris Wilson 0 siblings, 2 replies; 28+ messages in thread From: Tvrtko Ursulin @ 2017-04-06 18:02 UTC (permalink / raw) To: Oscar Mateo, intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi On 05/04/2017 10:30, Oscar Mateo wrote: > Not really needed, but makes the next change a little bit more compact. > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 8 ++++++-- > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- > drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index abc0a9c..530f822 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -71,7 +71,7 @@ > .init_legacy = intel_init_bsd_ring_buffer, > }, > [VCS2] = { > - .name = "vcs2", > + .name = "vcs", Rename the field to class_name perhaps? > .hw_id = VCS2_HW, > .exec_id = I915_EXEC_BSD, > .class = VIDEO_DECODE_CLASS, > @@ -100,6 +100,7 @@ > { > const struct engine_info *info = &intel_engines[id]; > struct intel_engine_cs *engine; > + char instance[3] = ""; > > GEM_BUG_ON(dev_priv->engine[id]); > engine = kzalloc(sizeof(*engine), GFP_KERNEL); > @@ -108,7 +109,10 @@ > > engine->id = id; > engine->i915 = dev_priv; > - engine->name = info->name; > + /* For historical reasons the engines are called: name, name2... */ > + if (info->instance) > + snprintf(instance, sizeof(instance), "%u", info->instance + 1); > + snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, instance); Since Chris has recently renamed all the engines, I'd say who cares about the numbering scheme. Just drop it for simplicity. > engine->exec_id = info->exec_id; > engine->hw_id = engine->guc_id = info->hw_id; > engine->mmio_base = info->mmio_base; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 5c1a27f..d617049 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -187,9 +187,11 @@ enum intel_engine_id { > VECS > }; > > +#define INTEL_ENGINE_CS_MAX_NAME 8 > + > struct intel_engine_cs { > struct drm_i915_private *i915; > - const char *name; > + char name[INTEL_ENGINE_CS_MAX_NAME]; > enum intel_engine_id id; > unsigned int exec_id; > unsigned int hw_id; > diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c > index b89050e..4a1ffca 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_engine.c > +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c > @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, > > /* minimal engine setup for requests */ > engine->base.i915 = i915; > - engine->base.name = name; > + strncpy(engine->base.name, name, sizeof(engine->base.name) - 1); Can this miss to null-terminate? Or it relies on the mock_engine being kzalloced? > engine->base.id = id++; > engine->base.status_page.page_addr = (void *)(engine + 1); > > Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 18:02 ` Tvrtko Ursulin @ 2017-04-06 11:22 ` Oscar Mateo 2017-04-06 18:37 ` Tvrtko Ursulin 2017-04-06 18:10 ` Chris Wilson 1 sibling, 1 reply; 28+ messages in thread From: Oscar Mateo @ 2017-04-06 11:22 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi On 04/06/2017 11:02 AM, Tvrtko Ursulin wrote: > > On 05/04/2017 10:30, Oscar Mateo wrote: >> Not really needed, but makes the next change a little bit more compact. >> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >> --- >> drivers/gpu/drm/i915/intel_engine_cs.c | 8 ++++++-- >> drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- >> drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- >> 3 files changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c >> b/drivers/gpu/drm/i915/intel_engine_cs.c >> index abc0a9c..530f822 100644 >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >> @@ -71,7 +71,7 @@ >> .init_legacy = intel_init_bsd_ring_buffer, >> }, >> [VCS2] = { >> - .name = "vcs2", >> + .name = "vcs", > > Rename the field to class_name perhaps? > In the following patch, when I move .name to the class table, it becomes much more obvious what it refers to, but I can change it to class_name if you feel strongly about it (or change it here and then back to name in the next patch?). >> .hw_id = VCS2_HW, >> .exec_id = I915_EXEC_BSD, >> .class = VIDEO_DECODE_CLASS, >> @@ -100,6 +100,7 @@ >> { >> const struct engine_info *info = &intel_engines[id]; >> struct intel_engine_cs *engine; >> + char instance[3] = ""; >> >> GEM_BUG_ON(dev_priv->engine[id]); >> engine = kzalloc(sizeof(*engine), GFP_KERNEL); >> @@ -108,7 +109,10 @@ >> >> engine->id = id; >> engine->i915 = dev_priv; >> - engine->name = info->name; >> + /* For historical reasons the engines are called: name, name2... */ >> + if (info->instance) >> + snprintf(instance, sizeof(instance), "%u", info->instance + 1); >> + snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, >> instance); > > Since Chris has recently renamed all the engines, I'd say who cares > about the numbering scheme. Just drop it for simplicity. > Oooohh! Can I also use zero-based numbering, like in the Bspec? (so xcs0, xcs1 ... xcsN?) >> engine->exec_id = info->exec_id; >> engine->hw_id = engine->guc_id = info->hw_id; >> engine->mmio_base = info->mmio_base; >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h >> b/drivers/gpu/drm/i915/intel_ringbuffer.h >> index 5c1a27f..d617049 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >> @@ -187,9 +187,11 @@ enum intel_engine_id { >> VECS >> }; >> >> +#define INTEL_ENGINE_CS_MAX_NAME 8 >> + >> struct intel_engine_cs { >> struct drm_i915_private *i915; >> - const char *name; >> + char name[INTEL_ENGINE_CS_MAX_NAME]; >> enum intel_engine_id id; >> unsigned int exec_id; >> unsigned int hw_id; >> diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c >> b/drivers/gpu/drm/i915/selftests/mock_engine.c >> index b89050e..4a1ffca 100644 >> --- a/drivers/gpu/drm/i915/selftests/mock_engine.c >> +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c >> @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct >> drm_i915_private *i915, >> >> /* minimal engine setup for requests */ >> engine->base.i915 = i915; >> - engine->base.name = name; >> + strncpy(engine->base.name, name, sizeof(engine->base.name) - 1); > > Can this miss to null-terminate? Or it relies on the mock_engine being > kzalloced? > It relies on the kzalloc, but I can add a \0 at the end for extra security? >> engine->base.id = id++; >> engine->base.status_page.page_addr = (void *)(engine + 1); >> >> > > Regards, > > Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 11:22 ` Oscar Mateo @ 2017-04-06 18:37 ` Tvrtko Ursulin 2017-04-06 18:44 ` Chris Wilson 0 siblings, 1 reply; 28+ messages in thread From: Tvrtko Ursulin @ 2017-04-06 18:37 UTC (permalink / raw) To: Oscar Mateo, intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi On 06/04/2017 12:22, Oscar Mateo wrote: > On 04/06/2017 11:02 AM, Tvrtko Ursulin wrote: >> On 05/04/2017 10:30, Oscar Mateo wrote: >>> Not really needed, but makes the next change a little bit more compact. >>> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_engine_cs.c | 8 ++++++-- >>> drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- >>> drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- >>> 3 files changed, 10 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c >>> b/drivers/gpu/drm/i915/intel_engine_cs.c >>> index abc0a9c..530f822 100644 >>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >>> @@ -71,7 +71,7 @@ >>> .init_legacy = intel_init_bsd_ring_buffer, >>> }, >>> [VCS2] = { >>> - .name = "vcs2", >>> + .name = "vcs", >> >> Rename the field to class_name perhaps? >> > > In the following patch, when I move .name to the class table, it becomes > much more obvious what it refers to, but I can change it to class_name > if you feel strongly about it (or change it here and then back to name > in the next patch?). No it's fine like it is, just a consequence of not looking-ahead enough. > >>> .hw_id = VCS2_HW, >>> .exec_id = I915_EXEC_BSD, >>> .class = VIDEO_DECODE_CLASS, >>> @@ -100,6 +100,7 @@ >>> { >>> const struct engine_info *info = &intel_engines[id]; >>> struct intel_engine_cs *engine; >>> + char instance[3] = ""; >>> >>> GEM_BUG_ON(dev_priv->engine[id]); >>> engine = kzalloc(sizeof(*engine), GFP_KERNEL); >>> @@ -108,7 +109,10 @@ >>> >>> engine->id = id; >>> engine->i915 = dev_priv; >>> - engine->name = info->name; >>> + /* For historical reasons the engines are called: name, name2... */ >>> + if (info->instance) >>> + snprintf(instance, sizeof(instance), "%u", info->instance + 1); >>> + snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, >>> instance); >> >> Since Chris has recently renamed all the engines, I'd say who cares >> about the numbering scheme. Just drop it for simplicity. >> > > Oooohh! > Can I also use zero-based numbering, like in the Bspec? (so xcs0, xcs1 > ... xcsN?) Sounds like the verdict is yes. > >>> engine->exec_id = info->exec_id; >>> engine->hw_id = engine->guc_id = info->hw_id; >>> engine->mmio_base = info->mmio_base; >>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h >>> b/drivers/gpu/drm/i915/intel_ringbuffer.h >>> index 5c1a27f..d617049 100644 >>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >>> @@ -187,9 +187,11 @@ enum intel_engine_id { >>> VECS >>> }; >>> >>> +#define INTEL_ENGINE_CS_MAX_NAME 8 >>> + >>> struct intel_engine_cs { >>> struct drm_i915_private *i915; >>> - const char *name; >>> + char name[INTEL_ENGINE_CS_MAX_NAME]; >>> enum intel_engine_id id; >>> unsigned int exec_id; >>> unsigned int hw_id; >>> diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c >>> b/drivers/gpu/drm/i915/selftests/mock_engine.c >>> index b89050e..4a1ffca 100644 >>> --- a/drivers/gpu/drm/i915/selftests/mock_engine.c >>> +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c >>> @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct >>> drm_i915_private *i915, >>> >>> /* minimal engine setup for requests */ >>> engine->base.i915 = i915; >>> - engine->base.name = name; >>> + strncpy(engine->base.name, name, sizeof(engine->base.name) - 1); >> >> Can this miss to null-terminate? Or it relies on the mock_engine being >> kzalloced? >> > > It relies on the kzalloc, but I can add a \0 at the end for extra security? Not sure at the moment without checking in detail how much mock_engine already depends on being zeroed. But I guess null-terminating it at the end wouldn't harm. Or maybe snprintf to eliminate the dilemma if it creates neater code? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 18:37 ` Tvrtko Ursulin @ 2017-04-06 18:44 ` Chris Wilson 0 siblings, 0 replies; 28+ messages in thread From: Chris Wilson @ 2017-04-06 18:44 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi On Thu, Apr 06, 2017 at 07:37:20PM +0100, Tvrtko Ursulin wrote: > > On 06/04/2017 12:22, Oscar Mateo wrote: > >On 04/06/2017 11:02 AM, Tvrtko Ursulin wrote: > >>On 05/04/2017 10:30, Oscar Mateo wrote: > >>>Not really needed, but makes the next change a little bit more compact. > >>> > >>>Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > >>>Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > >>>Cc: Chris Wilson <chris@chris-wilson.co.uk> > >>>Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > >>>Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > >>>--- > >>> drivers/gpu/drm/i915/intel_engine_cs.c | 8 ++++++-- > >>> drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- > >>> drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- > >>> 3 files changed, 10 insertions(+), 4 deletions(-) > >>> > >>>diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > >>>b/drivers/gpu/drm/i915/intel_engine_cs.c > >>>index abc0a9c..530f822 100644 > >>>--- a/drivers/gpu/drm/i915/intel_engine_cs.c > >>>+++ b/drivers/gpu/drm/i915/intel_engine_cs.c > >>>@@ -71,7 +71,7 @@ > >>> .init_legacy = intel_init_bsd_ring_buffer, > >>> }, > >>> [VCS2] = { > >>>- .name = "vcs2", > >>>+ .name = "vcs", > >> > >>Rename the field to class_name perhaps? > >> > > > >In the following patch, when I move .name to the class table, it becomes > >much more obvious what it refers to, but I can change it to class_name > >if you feel strongly about it (or change it here and then back to name > >in the next patch?). > > No it's fine like it is, just a consequence of not looking-ahead enough. > > > > >>> .hw_id = VCS2_HW, > >>> .exec_id = I915_EXEC_BSD, > >>> .class = VIDEO_DECODE_CLASS, > >>>@@ -100,6 +100,7 @@ > >>> { > >>> const struct engine_info *info = &intel_engines[id]; > >>> struct intel_engine_cs *engine; > >>>+ char instance[3] = ""; > >>> > >>> GEM_BUG_ON(dev_priv->engine[id]); > >>> engine = kzalloc(sizeof(*engine), GFP_KERNEL); > >>>@@ -108,7 +109,10 @@ > >>> > >>> engine->id = id; > >>> engine->i915 = dev_priv; > >>>- engine->name = info->name; > >>>+ /* For historical reasons the engines are called: name, name2... */ > >>>+ if (info->instance) > >>>+ snprintf(instance, sizeof(instance), "%u", info->instance + 1); > >>>+ snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, > >>>instance); > >> > >>Since Chris has recently renamed all the engines, I'd say who cares > >>about the numbering scheme. Just drop it for simplicity. > >> > > > >Oooohh! > >Can I also use zero-based numbering, like in the Bspec? (so xcs0, xcs1 > >... xcsN?) > > Sounds like the verdict is yes. > > > > >>> engine->exec_id = info->exec_id; > >>> engine->hw_id = engine->guc_id = info->hw_id; > >>> engine->mmio_base = info->mmio_base; > >>>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > >>>b/drivers/gpu/drm/i915/intel_ringbuffer.h > >>>index 5c1a27f..d617049 100644 > >>>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h > >>>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > >>>@@ -187,9 +187,11 @@ enum intel_engine_id { > >>> VECS > >>> }; > >>> > >>>+#define INTEL_ENGINE_CS_MAX_NAME 8 > >>>+ > >>> struct intel_engine_cs { > >>> struct drm_i915_private *i915; > >>>- const char *name; > >>>+ char name[INTEL_ENGINE_CS_MAX_NAME]; > >>> enum intel_engine_id id; > >>> unsigned int exec_id; > >>> unsigned int hw_id; > >>>diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c > >>>b/drivers/gpu/drm/i915/selftests/mock_engine.c > >>>index b89050e..4a1ffca 100644 > >>>--- a/drivers/gpu/drm/i915/selftests/mock_engine.c > >>>+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c > >>>@@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct > >>>drm_i915_private *i915, > >>> > >>> /* minimal engine setup for requests */ > >>> engine->base.i915 = i915; > >>>- engine->base.name = name; > >>>+ strncpy(engine->base.name, name, sizeof(engine->base.name) - 1); > >> > >>Can this miss to null-terminate? Or it relies on the mock_engine being > >>kzalloced? > >> > > > >It relies on the kzalloc, but I can add a \0 at the end for extra security? > > Not sure at the moment without checking in detail how much > mock_engine already depends on being zeroed. It does depend on the kzalloc. > But I guess > null-terminating it at the end wouldn't harm. Or maybe snprintf to > eliminate the dilemma if it creates neater code? Indeed, I want my mockN! We may want multiple instances of the mock engine class in the future. -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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] drm/i915: Generate the engine name based on the instance number 2017-04-06 18:02 ` Tvrtko Ursulin 2017-04-06 11:22 ` Oscar Mateo @ 2017-04-06 18:10 ` Chris Wilson 1 sibling, 0 replies; 28+ messages in thread From: Chris Wilson @ 2017-04-06 18:10 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi On Thu, Apr 06, 2017 at 07:02:10PM +0100, Tvrtko Ursulin wrote: > > On 05/04/2017 10:30, Oscar Mateo wrote: > >@@ -100,6 +100,7 @@ > > { > > const struct engine_info *info = &intel_engines[id]; > > struct intel_engine_cs *engine; > >+ char instance[3] = ""; > > > > GEM_BUG_ON(dev_priv->engine[id]); > > engine = kzalloc(sizeof(*engine), GFP_KERNEL); > >@@ -108,7 +109,10 @@ > > > > engine->id = id; > > engine->i915 = dev_priv; > >- engine->name = info->name; > >+ /* For historical reasons the engines are called: name, name2... */ > >+ if (info->instance) > >+ snprintf(instance, sizeof(instance), "%u", info->instance + 1); > >+ snprintf(engine->name, sizeof(engine->name), "%s%s", info->name, instance); > > Since Chris has recently renamed all the engines, I'd say who cares > about the numbering scheme. Just drop it for simplicity. Seconded. class0 is ok, or if you are being fancy only use the classN format if there are multiples in the class. Though kiss says just use classN and be done with it. -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 ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2017-04-10 21:34 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-07 9:15 [PATCH 0/5] Classify the engines in class + instance (v4) Oscar Mateo 2017-04-07 9:15 ` [PATCH 1/5] drm/i915: Classify the engines in class + instance Oscar Mateo 2017-04-07 16:20 ` Michal Wajdeczko 2017-04-07 9:15 ` [PATCH 2/5] drm/i915: Use the same vfunc for BSD2 ring init Oscar Mateo 2017-04-07 9:15 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 2017-04-07 16:23 ` Michal Wajdeczko 2017-04-07 17:11 ` Chris Wilson 2017-04-07 12:08 ` Oscar Mateo 2017-04-07 9:15 ` [PATCH 4/5] drm/i915: Split the engine info table in two levels, using class + instance Oscar Mateo 2017-04-07 16:27 ` Michal Wajdeczko 2017-04-07 9:34 ` [PATCH v5] " Oscar Mateo 2017-04-07 16:37 ` Michal Wajdeczko 2017-04-07 9:15 ` [PATCH 5/5] drm/i915: Use the engine class to get the context size Oscar Mateo 2017-04-07 16:35 ` ✓ Fi.CI.BAT: success for Classify the engines in class + instance (rev5) Patchwork 2017-04-07 16:51 ` ✓ Fi.CI.BAT: success for Classify the engines in class + instance (rev6) Patchwork -- strict thread matches above, loose matches on Subject: below -- 2017-04-10 14:34 [PATCH 0/5] Classify the engines in class + instance (v5) Oscar Mateo 2017-04-10 14:34 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 2017-04-06 15:00 [PATCH 0/5] Classify the engines in class + instance (v3) Oscar Mateo 2017-04-06 15:00 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 2017-04-07 8:12 ` Tvrtko Ursulin 2017-04-07 10:31 ` Michal Wajdeczko 2017-04-06 12:55 [PATCH 0/5] Classify the engines in class + instance (v2) Oscar Mateo 2017-04-06 12:55 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 2017-04-06 20:10 ` Chris Wilson 2017-04-06 13:15 ` Oscar Mateo 2017-04-05 9:30 [PATCH 0/5] Classify the engines in class + instance Oscar Mateo 2017-04-05 9:30 ` [PATCH 3/5] drm/i915: Generate the engine name based on the instance number Oscar Mateo 2017-04-06 18:02 ` Tvrtko Ursulin 2017-04-06 11:22 ` Oscar Mateo 2017-04-06 18:37 ` Tvrtko Ursulin 2017-04-06 18:44 ` Chris Wilson 2017-04-06 18:10 ` Chris Wilson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.