From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/4] drm/i915: extract constant offset setting
Date: Thu, 13 Oct 2011 10:46:29 +0200 [thread overview]
Message-ID: <20111013084629.GE2938@phenom.ffwll.local> (raw)
In-Reply-To: <1318460182-21302-4-git-send-email-ben@bwidawsk.net>
On Wed, Oct 12, 2011 at 03:56:21PM -0700, Ben Widawsky wrote:
> Simple refactor.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
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
next prev parent reply other threads:[~2011-10-13 8:45 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-12 4:43 [PATCH 0/5] execbuffer cleanups + fixes Ben Widawsky
2011-10-12 4:43 ` [PATCH 1/5] drm/i915: execbuffer simplification Ben Widawsky
2011-10-12 4:43 ` [PATCH 2/5] drm/i915: extract constant offset setting Ben Widawsky
2011-10-12 16:46 ` Ben Widawsky
2011-10-12 4:43 ` [PATCH 3/5] drm/i915: make eb structure do more Ben Widawsky
2011-10-12 4:43 ` [PATCH 4/5] drm/i915: relative_constants_mode race fix Ben Widawsky
2011-10-12 4:43 ` [PATCH 5/5] drm/i915: Force sync command ordering (Gen6+) Ben Widawsky
2011-10-12 8:01 ` Ben Widawsky
2011-10-12 9:51 ` Daniel Vetter
2011-10-12 16:59 ` [PATCH 0/5] execbuffer cleanups + fixes Ben Widawsky
2011-10-12 22:56 ` [PATCH 0/4] pre-v2 series Ben Widawsky
2011-10-12 22:56 ` [PATCH 1/4] drm/i915: relative_constants_mode race fix Ben Widawsky
2011-10-13 8:35 ` Daniel Vetter
2011-10-13 17:25 ` Ben Widawsky
2011-10-12 22:56 ` [PATCH 2/4] drm/i915: Force sync command ordering (Gen6+) Ben Widawsky
2011-10-13 8:44 ` Daniel Vetter
2011-10-12 22:56 ` [PATCH 3/4] drm/i915: extract constant offset setting Ben Widawsky
2011-10-13 8:46 ` Daniel Vetter [this message]
2011-10-13 17:14 ` Ben Widawsky
2011-10-12 22:56 ` [PATCH 4/4] drm/i915: Use eb for more stuff Ben Widawsky
2011-10-13 8:58 ` Daniel Vetter
2011-10-13 12:00 ` Chris Wilson
2011-10-13 12:49 ` Daniel Vetter
2011-10-13 17:31 ` Ben Widawsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111013084629.GE2938@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=ben@bwidawsk.net \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox