On Thu, Jul 24, 2014 at 05:04:09PM +0100, Thomas Daniel wrote:
> From: Oscar Mateo <oscar.mateo@xxxxxxxxx>
>
> In this patch:
>
> commit 78382593e921c88371abd019aca8978db3248a8f
> Author: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> Date: Thu Jul 3 16:28:05 2014 +0100
>
> drm/i915: Extract the actual workload submission mechanism from execbuffer
>
> So that we isolate the legacy ringbuffer submission mechanism, which becomes
> a good candidate to be abstracted away. This is prep-work for Execlists (which
> will its own workload submission mechanism).
>
> No functional changes.
>
> I changed the order in which the args checking is done. I don't know why I did (brain
> fade?) but itś not right. I haven't seen any ill effect from this, but the Execlists
> version of this function will have problems if the order is not correct.
>
> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
I don't think this matters - the point of no return for legacy execbuf is
the call to ring->dispatch. After that nothing may fail any more. But as
long as we track state correctly (e.g. if we've switched the context
already) we'll be fine.
So presuming I'm not blind I dont' think this is needed. But maybe Chris
spots something.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 86 ++++++++++++++--------------
> 1 file changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 60998fc..c5115957 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1042,6 +1042,43 @@ legacy_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
> u32 instp_mask;
> int i, ret = 0;
>
> + instp_mode = args->flags & I915_EXEC_CONSTANTS_MASK;
> + instp_mask = I915_EXEC_CONSTANTS_MASK;
> + switch (instp_mode) {
> + case I915_EXEC_CONSTANTS_REL_GENERAL:
> + case I915_EXEC_CONSTANTS_ABSOLUTE:
> + case I915_EXEC_CONSTANTS_REL_SURFACE:
> + if (instp_mode != 0 && ring != &dev_priv->ring[RCS]) {
> + DRM_DEBUG("non-0 rel constants mode on non-RCS\n");
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + if (instp_mode != dev_priv->relative_constants_mode) {
> + if (INTEL_INFO(dev)->gen < 4) {
> + DRM_DEBUG("no rel constants on pre-gen4\n");
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + if (INTEL_INFO(dev)->gen > 5 &&
> + instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) {
> + DRM_DEBUG("rel surface constants mode invalid on gen5+\n");
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + /* The HW changed the meaning on this bit on gen6 */
> + if (INTEL_INFO(dev)->gen >= 6)
> + instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE;
> + }
> + break;
> + default:
> + DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode);
> + ret = -EINVAL;
> + goto error;
> + }
> +
> if (args->num_cliprects != 0) {
> if (ring != &dev_priv->ring[RCS]) {
> DRM_DEBUG("clip rectangles are only valid with the render ring\n");
> @@ -1085,6 +1122,12 @@ legacy_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
> }
> }
>
> + if (args->flags & I915_EXEC_GEN7_SOL_RESET) {
> + ret = i915_reset_gen7_sol_offsets(dev, ring);
> + if (ret)
> + goto error;
> + }
> +
> ret = i915_gem_execbuffer_move_to_gpu(ring, vmas);
> if (ret)
> goto error;
> @@ -1093,43 +1136,6 @@ legacy_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
> if (ret)
> goto error;
>
> - instp_mode = args->flags & I915_EXEC_CONSTANTS_MASK;
> - instp_mask = I915_EXEC_CONSTANTS_MASK;
> - switch (instp_mode) {
> - case I915_EXEC_CONSTANTS_REL_GENERAL:
> - case I915_EXEC_CONSTANTS_ABSOLUTE:
> - case I915_EXEC_CONSTANTS_REL_SURFACE:
> - if (instp_mode != 0 && ring != &dev_priv->ring[RCS]) {
> - DRM_DEBUG("non-0 rel constants mode on non-RCS\n");
> - ret = -EINVAL;
> - goto error;
> - }
> -
> - if (instp_mode != dev_priv->relative_constants_mode) {
> - if (INTEL_INFO(dev)->gen < 4) {
> - DRM_DEBUG("no rel constants on pre-gen4\n");
> - ret = -EINVAL;
> - goto error;
> - }
> -
> - if (INTEL_INFO(dev)->gen > 5 &&
> - instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) {
> - DRM_DEBUG("rel surface constants mode invalid on gen5+\n");
> - ret = -EINVAL;
> - goto error;
> - }
> -
> - /* The HW changed the meaning on this bit on gen6 */
> - if (INTEL_INFO(dev)->gen >= 6)
> - instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE;
> - }
> - break;
> - default:
> - DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode);
> - ret = -EINVAL;
> - goto error;
> - }
> -
> if (ring == &dev_priv->ring[RCS] &&
> instp_mode != dev_priv->relative_constants_mode) {
> ret = intel_ring_begin(ring, 4);
> @@ -1145,12 +1151,6 @@ legacy_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
> dev_priv->relative_constants_mode = instp_mode;
> }
>
> - if (args->flags & I915_EXEC_GEN7_SOL_RESET) {
> - ret = i915_reset_gen7_sol_offsets(dev, ring);
> - if (ret)
> - goto error;
> - }
> -
> exec_len = args->batch_len;
> if (cliprects) {
> for (i = 0; i < args->num_cliprects; i++) {
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
