| Message ID | 1470842530-35854-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/08/16 16:22, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Until now code was calling hweight32 to figure out the > number from device_info->ring_mask at runtime. Instead > we can cache it at engine init time and use directly. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > drivers/gpu/drm/i915/intel_engine_cs.c | 10 +++++----- > drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++--- > 5 files changed, 11 insertions(+), 10 deletions(-) LGTM. Reviewed-by: Dave Gordon <david.s.gordon@intel.com> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 83f40e869955..c461072da142 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3226,7 +3226,7 @@ static int i915_semaphore_status(struct seq_file *m, void *unused) > struct drm_device *dev = node->minor->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_engine_cs *engine; > - int num_rings = hweight32(INTEL_INFO(dev)->ring_mask); > + int num_rings = INTEL_INFO(dev)->num_rings; > enum intel_engine_id id; > int j, ret; > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7f2754a070a5..7971c76852df 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -793,6 +793,7 @@ struct intel_device_info { > u8 gen; > u16 gen_mask; > u8 ring_mask; /* Rings supported by the HW */ > + u8 num_rings; > DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON); > /* Register offsets for the various display pipes and transcoders */ > int pipe_offsets[I915_MAX_TRANSCODERS]; > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index bb72af5320b0..547caf26a6b9 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -568,7 +568,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) > const int num_rings = > /* Use an extended w/a on ivb+ if signalling from other rings */ > i915.semaphores ? > - hweight32(INTEL_INFO(dev_priv)->ring_mask) - 1 : > + INTEL_INFO(dev_priv)->num_rings - 1 : > 0; > int len, ret; > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 0dd3d1de18aa..186c12d07f99 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -109,6 +109,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv, > int intel_engines_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = to_i915(dev); > + struct intel_device_info *device_info = mkwrite_device_info(dev_priv); > unsigned int mask = 0; > int (*init)(struct intel_engine_cs *engine); > unsigned int i; > @@ -142,11 +143,10 @@ int intel_engines_init(struct drm_device *dev) > * are added to the driver by a warning and disabling the forgotten > * engines. > */ > - if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) { > - struct intel_device_info *info = > - (struct intel_device_info *)&dev_priv->info; > - info->ring_mask = mask; > - } > + if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) > + device_info->ring_mask = mask; > + > + device_info->num_rings = hweight32(mask); > > return 0; > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 16b726fe33eb..a05a2a13ea7c 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1317,7 +1317,7 @@ static int gen8_rcs_signal(struct drm_i915_gem_request *req) > enum intel_engine_id id; > int ret, num_rings; > > - num_rings = hweight32(INTEL_INFO(dev_priv)->ring_mask); > + num_rings = INTEL_INFO(dev_priv)->num_rings; > ret = intel_ring_begin(req, (num_rings-1) * 8); > if (ret) > return ret; > @@ -1354,7 +1354,7 @@ static int gen8_xcs_signal(struct drm_i915_gem_request *req) > enum intel_engine_id id; > int ret, num_rings; > > - num_rings = hweight32(INTEL_INFO(dev_priv)->ring_mask); > + num_rings = INTEL_INFO(dev_priv)->num_rings; > ret = intel_ring_begin(req, (num_rings-1) * 6); > if (ret) > return ret; > @@ -1389,7 +1389,7 @@ static int gen6_signal(struct drm_i915_gem_request *req) > enum intel_engine_id id; > int ret, num_rings; > > - num_rings = hweight32(INTEL_INFO(dev_priv)->ring_mask); > + num_rings = INTEL_INFO(dev_priv)->num_rings; > ret = intel_ring_begin(req, round_up((num_rings-1) * 3, 2)); > if (ret) > return ret; >
On Wed, Aug 10, 2016 at 04:22:10PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Until now code was calling hweight32 to figure out the > number from device_info->ring_mask at runtime. Instead > we can cache it at engine init time and use directly. And calling hweight32() rather than hweight() on a u8. > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Momentary concern as I had 4 hweights inside intel_ringbuffer.c - then realised I've a patch to add one more. -Chris
On 10/08/16 18:27, Patchwork wrote: > == Series Details == > > Series: drm/i915: Store number of active engines in device info > URL : https://patchwork.freedesktop.org/series/10921/ > State : failure > > == Summary == > > Series 10921v1 drm/i915: Store number of active engines in device info > http://patchwork.freedesktop.org/api/1.0/series/10921/revisions/1/mbox > > Test kms_cursor_legacy: > Subgroup basic-cursor-vs-flip-varying-size: > fail -> PASS (ro-ilk1-i5-650) > Subgroup basic-flip-vs-cursor-legacy: > fail -> PASS (ro-skl3-i5-6260u) > fail -> PASS (ro-bdw-i7-5557U) > Subgroup basic-flip-vs-cursor-varying-size: > fail -> PASS (ro-bdw-i5-5250u) > Test kms_pipe_crc_basic: > Subgroup nonblocking-crc-pipe-b-frame-sequence: > pass -> FAIL (ro-bdw-i7-5600u) (kms_pipe_crc_basic:8255) CRITICAL: Failed assertion: crcs[j].frame + 1 == crcs[j + 1].frame Some modesetting problem, no idea, raised a new bug: https://bugs.freedesktop.org/show_bug.cgi?id=97294 > Subgroup suspend-read-crc-pipe-b: > dmesg-warn -> SKIP (ro-bdw-i7-5557U) > Subgroup suspend-read-crc-pipe-c: > skip -> DMESG-WARN (ro-bdw-i5-5250u) https://bugs.freedesktop.org/show_bug.cgi?id=96614 [ 482.345981] [drm:intel_dp_link_training_clock_recovery [i915]] *ERROR* failed to enable link training [ 482.431185] [drm:intel_dp_start_link_train [i915]] *ERROR* failed to start channel equalization > > ro-bdw-i5-5250u total:240 pass:219 dwarn:2 dfail:0 fail:1 skip:18 > ro-bdw-i7-5557U total:240 pass:220 dwarn:1 dfail:0 fail:0 skip:19 > ro-bdw-i7-5600u total:240 pass:206 dwarn:0 dfail:0 fail:2 skip:32 > ro-bsw-n3050 total:240 pass:194 dwarn:0 dfail:0 fail:4 skip:42 > ro-byt-n2820 total:240 pass:197 dwarn:0 dfail:0 fail:3 skip:40 > ro-hsw-i3-4010u total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26 > ro-hsw-i7-4770r total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26 > ro-ilk1-i5-650 total:235 pass:174 dwarn:0 dfail:0 fail:1 skip:60 > ro-ivb-i7-3770 total:240 pass:205 dwarn:0 dfail:0 fail:0 skip:35 > ro-ivb2-i7-3770 total:240 pass:209 dwarn:0 dfail:0 fail:0 skip:31 > ro-skl3-i5-6260u total:240 pass:223 dwarn:0 dfail:0 fail:3 skip:14 > > Results at /archive/results/CI_IGT_test/RO_Patchwork_1825/ > > 3aec82c drm-intel-nightly: 2016y-08m-10d-15h-08m-03s UTC integration manifest > 11c946d drm/i915: Store number of active engines in device info Merged to dinq, thanks for the review! Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 83f40e869955..c461072da142 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3226,7 +3226,7 @@ static int i915_semaphore_status(struct seq_file *m, void *unused) struct drm_device *dev = node->minor->dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_engine_cs *engine; - int num_rings = hweight32(INTEL_INFO(dev)->ring_mask); + int num_rings = INTEL_INFO(dev)->num_rings; enum intel_engine_id id; int j, ret; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7f2754a070a5..7971c76852df 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -793,6 +793,7 @@ struct intel_device_info { u8 gen; u16 gen_mask; u8 ring_mask; /* Rings supported by the HW */ + u8 num_rings; DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON); /* Register offsets for the various display pipes and transcoders */ int pipe_offsets[I915_MAX_TRANSCODERS]; diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index bb72af5320b0..547caf26a6b9 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -568,7 +568,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) const int num_rings = /* Use an extended w/a on ivb+ if signalling from other rings */ i915.semaphores ? - hweight32(INTEL_INFO(dev_priv)->ring_mask) - 1 : + INTEL_INFO(dev_priv)->num_rings - 1 : 0; int len, ret; diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 0dd3d1de18aa..186c12d07f99 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -109,6 +109,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv, int intel_engines_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); + struct intel_device_info *device_info = mkwrite_device_info(dev_priv); unsigned int mask = 0; int (*init)(struct intel_engine_cs *engine); unsigned int i; @@ -142,11 +143,10 @@ int intel_engines_init(struct drm_device *dev) * are added to the driver by a warning and disabling the forgotten * engines. */ - if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) { - struct intel_device_info *info = - (struct intel_device_info *)&dev_priv->info; - info->ring_mask = mask; - } + if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) + device_info->ring_mask = mask; + + device_info->num_rings = hweight32(mask); return 0; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 16b726fe33eb..a05a2a13ea7c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1317,7 +1317,7 @@ static int gen8_rcs_signal(struct drm_i915_gem_request *req) enum intel_engine_id id; int ret, num_rings; - num_rings = hweight32(INTEL_INFO(dev_priv)->ring_mask); + num_rings = INTEL_INFO(dev_priv)->num_rings; ret = intel_ring_begin(req, (num_rings-1) * 8); if (ret) return ret; @@ -1354,7 +1354,7 @@ static int gen8_xcs_signal(struct drm_i915_gem_request *req) enum intel_engine_id id; int ret, num_rings; - num_rings = hweight32(INTEL_INFO(dev_priv)->ring_mask); + num_rings = INTEL_INFO(dev_priv)->num_rings; ret = intel_ring_begin(req, (num_rings-1) * 6); if (ret) return ret; @@ -1389,7 +1389,7 @@ static int gen6_signal(struct drm_i915_gem_request *req) enum intel_engine_id id; int ret, num_rings; - num_rings = hweight32(INTEL_INFO(dev_priv)->ring_mask); + num_rings = INTEL_INFO(dev_priv)->num_rings; ret = intel_ring_begin(req, round_up((num_rings-1) * 3, 2)); if (ret) return ret;