From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/4] drm/i915: extract constant offset setting Date: Thu, 13 Oct 2011 10:46:29 +0200 Message-ID: <20111013084629.GE2938@phenom.ffwll.local> References: <1318394633-11143-1-git-send-email-ben@bwidawsk.net> <1318460182-21302-1-git-send-email-ben@bwidawsk.net> <1318460182-21302-4-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ww0-f41.google.com (mail-ww0-f41.google.com [74.125.82.41]) by gabe.freedesktop.org (Postfix) with ESMTP id 52F489EB20 for ; Thu, 13 Oct 2011 01:45:48 -0700 (PDT) Received: by wwf10 with SMTP id 10so7387252wwf.0 for ; Thu, 13 Oct 2011 01:45:47 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1318460182-21302-4-git-send-email-ben@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Oct 12, 2011 at 03:56:21PM -0700, Ben Widawsky wrote: > Simple refactor. > > Signed-off-by: Ben Widawsky Imo you can fold this in with the fix. Usually it's good to separate fixes from refactoring, but it this special case of just moving around code blocks, it doesn't add anything. > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 82 ++++++++++++++++------------ > 1 files changed, 46 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 1ee1872..182a2b9 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -954,6 +954,50 @@ i915_gem_execbuffer_retire_commands(struct drm_device *dev, > } > > static int > +i915_gem_set_constant_offset(struct intel_ring_buffer *ring, int mode) > +{ > + struct drm_device *dev = ring->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + uint32_t mask = I915_EXEC_CONSTANTS_MASK; > + int ret; > + > + switch (mode) { > + case I915_EXEC_CONSTANTS_REL_GENERAL: > + case I915_EXEC_CONSTANTS_ABSOLUTE: > + case I915_EXEC_CONSTANTS_REL_SURFACE: > + if (ring == &dev_priv->ring[RCS] && > + mode != dev_priv->relative_constants_mode) { > + if (INTEL_INFO(dev)->gen < 4) > + return -EINVAL; > + > + if (INTEL_INFO(dev)->gen > 5 && > + mode == I915_EXEC_CONSTANTS_REL_SURFACE) > + return -EINVAL; > + > + /* The HW changed the meaning on this bit on gen6 */ > + if (INTEL_INFO(dev)->gen >= 6) > + mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; > + > + ret = intel_ring_begin(ring, 4); > + if (ret) > + return ret; > + > + intel_ring_emit(ring, MI_NOOP); > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > + intel_ring_emit(ring, INSTPM); > + intel_ring_emit(ring, mask << 16 | mode); > + intel_ring_advance(ring); > + > + dev_priv->relative_constants_mode = mode; > + } > + return 0; > + default: > + DRM_ERROR("execbuf with unknown constants: %d\n", mode); > + return -EINVAL; > + } > +} > + > +static int > i915_gem_do_execbuffer(struct drm_device *dev, void *data, > struct drm_file *file, > struct drm_i915_gem_execbuffer2 *args, > @@ -967,7 +1011,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > struct intel_ring_buffer *ring; > u32 exec_start, exec_len; > u32 seqno; > - u32 mask; > int ret, mode, i; > > if (!i915_gem_check_execbuffer(args)) { > @@ -1101,42 +1144,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > } > > mode = args->flags & I915_EXEC_CONSTANTS_MASK; > - mask = I915_EXEC_CONSTANTS_MASK; > - switch (mode) { > - case I915_EXEC_CONSTANTS_REL_GENERAL: > - case I915_EXEC_CONSTANTS_ABSOLUTE: > - case I915_EXEC_CONSTANTS_REL_SURFACE: > - if (ring == &dev_priv->ring[RCS] && > - mode != dev_priv->relative_constants_mode) { > - if (INTEL_INFO(dev)->gen < 4) > - return -EINVAL; > - > - if (INTEL_INFO(dev)->gen > 5 && > - mode == I915_EXEC_CONSTANTS_REL_SURFACE) > - return -EINVAL; > - > - /* The HW changed the meaning on this bit on gen6 */ > - if (INTEL_INFO(dev)->gen >= 6) > - mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; > - > - ret = intel_ring_begin(ring, 4); > - if (ret) > - goto err; > - > - intel_ring_emit(ring, MI_NOOP); > - intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > - intel_ring_emit(ring, INSTPM); > - intel_ring_emit(ring, mask << 16 | mode); > - intel_ring_advance(ring); > - > - dev_priv->relative_constants_mode = mode; > - } > - break; > - default: > - DRM_ERROR("execbuf with unknown constants: %d\n", mode); > - ret -EINVAL; > + ret = i915_gem_set_constant_offset(ring, mode); > + if (ret) > goto err; > - } > > /* Set the pending read domains for the batch buffer to COMMAND */ > if (batch_obj->base.pending_write_domain) { > -- > 1.7.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48