| Message ID | 1424111111-23853-1-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 16, 2015 at 06:25:10PM +0000, Damien Lespiau wrote: > instpm_mode != relative_constants_mode is quite unlikely to happen, so > we can test it first to use C's && short-circuiting and not test on > 'ring'. > > I know, probably a useless micro-optimisation in the big scheme of > things, but I'm going to add another test here, so might as well do it. If you want to get pedantic, we want to move this to per-context :) -Chris
On Mon, Feb 16, 2015 at 09:03:51PM +0000, Chris Wilson wrote: > On Mon, Feb 16, 2015 at 06:25:10PM +0000, Damien Lespiau wrote: > > instpm_mode != relative_constants_mode is quite unlikely to happen, so > > we can test it first to use C's && short-circuiting and not test on > > 'ring'. > > > > I know, probably a useless micro-optimisation in the big scheme of > > things, but I'm going to add another test here, so might as well do it. > > If you want to get pedantic, we want to move this to per-context :) Do we? the API is per execbuf call, so theoretically application can change that during the context life time (it'd be silly, but they can). Or am I missing the point?
On Tue, Feb 17, 2015 at 11:48:25AM +0000, Damien Lespiau wrote: > On Mon, Feb 16, 2015 at 09:03:51PM +0000, Chris Wilson wrote: > > On Mon, Feb 16, 2015 at 06:25:10PM +0000, Damien Lespiau wrote: > > > instpm_mode != relative_constants_mode is quite unlikely to happen, so > > > we can test it first to use C's && short-circuiting and not test on > > > 'ring'. > > > > > > I know, probably a useless micro-optimisation in the big scheme of > > > things, but I'm going to add another test here, so might as well do it. > > > > If you want to get pedantic, we want to move this to per-context :) > > Do we? the API is per execbuf call, so theoretically application can > change that during the context life time (it'd be silly, but they can). > Or am I missing the point? It was that this is a context specific register and more about handling the transaction unwind if the execbuffer were to fail later i.e. an issue about to arise with new features modifying the code. But in theory we could do fewer LRI for that one application that used it... -Chris
On 16/02/15 21:03, Chris Wilson wrote: > On Mon, Feb 16, 2015 at 06:25:10PM +0000, Damien Lespiau wrote: >> instpm_mode != relative_constants_mode is quite unlikely to happen, so >> we can test it first to use C's && short-circuiting and not test on >> 'ring'. >> >> I know, probably a useless micro-optimisation in the big scheme of >> things, but I'm going to add another test here, so might as well do it. > > If you want to get pedantic, we want to move this to per-context :) > -Chris This test-and-set of instpm_mode will get reworked anyway when we add preemption support, as that means we can no longer assume the value set in the last execbuffer call is still in force at the beginning of the next. .Dave.
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index aafcef3..896641a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -691,8 +691,8 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file, if (ret) return ret; - if (ring == &dev_priv->ring[RCS] && - instp_mode != dev_priv->relative_constants_mode) { + if (instp_mode != dev_priv->relative_constants_mode && + ring == &dev_priv->ring[RCS]) { ret = intel_logical_ring_begin(ringbuf, ctx, 4); if (ret) return ret;
instpm_mode != relative_constants_mode is quite unlikely to happen, so we can test it first to use C's && short-circuiting and not test on 'ring'. I know, probably a useless micro-optimisation in the big scheme of things, but I'm going to add another test here, so might as well do it. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)