Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Simplify processing of the golden render context state
Date: Mon, 16 Jun 2014 14:40:45 +0300	[thread overview]
Message-ID: <87fvj5ozeq.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <1402395813-6568-1-git-send-email-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Rewrite i915_gem_render_state.c for the purposes of clarity and
> compactness, in the process we can eliminate some dodgy math that did
> not handle 64bit addresses correctly.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_render_state.c  | 161 +++++++++++---------------
>  drivers/gpu/drm/i915/intel_renderstate.h      |   2 -
>  drivers/gpu/drm/i915/intel_renderstate_gen6.c |   1 +
>  drivers/gpu/drm/i915/intel_renderstate_gen7.c |   1 +
>  drivers/gpu/drm/i915/intel_renderstate_gen8.c |   1 +
>  5 files changed, 69 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
> index 3521f998a178..e60be3f552a6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -28,64 +28,13 @@
>  #include "i915_drv.h"
>  #include "intel_renderstate.h"
>  
> -struct i915_render_state {
> +struct render_state {
> +	const struct intel_renderstate_rodata *rodata;
>  	struct drm_i915_gem_object *obj;
> -	unsigned long ggtt_offset;
> -	void *batch;
> -	u32 size;
> -	u32 len;
> +	u64 ggtt_offset;
> +	int gen;
>  };
>  
> -static struct i915_render_state *render_state_alloc(struct drm_device *dev)
> -{
> -	struct i915_render_state *so;
> -	struct page *page;
> -	int ret;
> -
> -	so = kzalloc(sizeof(*so), GFP_KERNEL);
> -	if (!so)
> -		return ERR_PTR(-ENOMEM);
> -
> -	so->obj = i915_gem_alloc_object(dev, 4096);
> -	if (so->obj == NULL) {
> -		ret = -ENOMEM;
> -		goto free;
> -	}
> -	so->size = 4096;
> -
> -	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
> -	if (ret)
> -		goto free_gem;
> -
> -	BUG_ON(so->obj->pages->nents != 1);
> -	page = sg_page(so->obj->pages->sgl);
> -
> -	so->batch = kmap(page);
> -	if (!so->batch) {
> -		ret = -ENOMEM;
> -		goto unpin;
> -	}
> -
> -	so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj);
> -
> -	return so;
> -unpin:
> -	i915_gem_object_ggtt_unpin(so->obj);
> -free_gem:
> -	drm_gem_object_unreference(&so->obj->base);
> -free:
> -	kfree(so);
> -	return ERR_PTR(ret);
> -}
> -
> -static void render_state_free(struct i915_render_state *so)
> -{
> -	kunmap(so->batch);
> -	i915_gem_object_ggtt_unpin(so->obj);
> -	drm_gem_object_unreference(&so->obj->base);
> -	kfree(so);
> -}
> -
>  static const struct intel_renderstate_rodata *
>  render_state_get_rodata(struct drm_device *dev, const int gen)
>  {
> @@ -101,98 +50,120 @@ render_state_get_rodata(struct drm_device *dev, const int gen)
>  	return NULL;
>  }
>  
> -static int render_state_setup(const int gen,
> -			      const struct intel_renderstate_rodata *rodata,
> -			      struct i915_render_state *so)
> +static int render_state_init(struct render_state *so, struct drm_device *dev)
>  {
> -	const u64 goffset = i915_gem_obj_ggtt_offset(so->obj);
> -	u32 reloc_index = 0;
> -	u32 * const d = so->batch;
> -	unsigned int i = 0;
>  	int ret;
>  
> -	if (!rodata || rodata->batch_items * 4 > so->size)
> +	so->gen = INTEL_INFO(dev)->gen;
> +	so->rodata = render_state_get_rodata(dev, so->gen);
> +	if (so->rodata == NULL)
> +		return 0;
> +
> +	if (so->rodata->batch_items * 4 > 4096)
>  		return -EINVAL;
>  
> +	so->obj = i915_gem_alloc_object(dev, 4096);
> +	if (so->obj == NULL)
> +		return -ENOMEM;
> +
> +	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
> +	if (ret)
> +		goto free_gem;
> +
> +	so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj);
> +	return 0;
> +
> +free_gem:
> +	drm_gem_object_unreference(&so->obj->base);
> +	return ret;
> +}
> +
> +static int render_state_setup(struct render_state *so)
> +{
> +	const struct intel_renderstate_rodata *rodata = so->rodata;
> +	unsigned int i = 0, reloc_index = 0;
> +	struct page *page;
> +	u32 *d;
> +	int ret;
> +
>  	ret = i915_gem_object_set_to_cpu_domain(so->obj, true);
>  	if (ret)
>  		return ret;
>  
> +	page = sg_page(so->obj->pages->sgl);
> +	d = kmap(page);
> +
>  	while (i < rodata->batch_items) {
>  		u32 s = rodata->batch[i];
>  
> -		if (reloc_index < rodata->reloc_items &&
> -		    i * 4  == rodata->reloc[reloc_index]) {
> -
> -			s += goffset & 0xffffffff;
> -
> -			/* We keep batch offsets max 32bit */
> -			if (gen >= 8) {
> +		if (i * 4  == rodata->reloc[reloc_index]) {
> +			u64 r = s + so->ggtt_offset;
> +			s = lower_32_bits(r);
> +			if (so->gen >= 8) {
>  				if (i + 1 >= rodata->batch_items ||
>  				    rodata->batch[i + 1] != 0)
>  					return -EINVAL;
>  
> -				d[i] = s;
> -				i++;
> -				s = (goffset & 0xffffffff00000000ull) >> 32;
> +				d[i++] = s;
> +				s = upper_32_bits(r);
>  			}
>  
>  			reloc_index++;
>  		}
>  
> -		d[i] = s;
> -		i++;
> +		d[i++] = s;
>  	}
> +	kunmap(page);
>  
>  	ret = i915_gem_object_set_to_gtt_domain(so->obj, false);
>  	if (ret)
>  		return ret;
>  
> -	if (rodata->reloc_items != reloc_index) {
> -		DRM_ERROR("not all relocs resolved, %d out of %d\n",
> -			  reloc_index, rodata->reloc_items);
> +	if (rodata->reloc[reloc_index] != -1) {
> +		DRM_ERROR("only %d relocs resolved\n", reloc_index);
>  		return -EINVAL;
>  	}
>  
> -	so->len = rodata->batch_items * 4;
> -
>  	return 0;
>  }
>  
> +static void render_state_fini(struct render_state *so)
> +{
> +	i915_gem_object_ggtt_unpin(so->obj);
> +	drm_gem_object_unreference(&so->obj->base);
> +}
> +
>  int i915_gem_render_state_init(struct intel_engine_cs *ring)
>  {
> -	const int gen = INTEL_INFO(ring->dev)->gen;
> -	struct i915_render_state *so;
> -	const struct intel_renderstate_rodata *rodata;
> +	struct render_state so;
>  	int ret;
>  
>  	if (WARN_ON(ring->id != RCS))
>  		return -ENOENT;
>  
> -	rodata = render_state_get_rodata(ring->dev, gen);
> -	if (rodata == NULL)
> -		return 0;
> +	ret = render_state_init(&so, ring->dev);
> +	if (ret)
> +		return ret;
>  
> -	so = render_state_alloc(ring->dev);
> -	if (IS_ERR(so))
> -		return PTR_ERR(so);
> +	if (so.rodata == NULL)
> +		return 0;
>  
> -	ret = render_state_setup(gen, rodata, so);
> +	ret = render_state_setup(&so);
>  	if (ret)
>  		goto out;
>  
>  	ret = ring->dispatch_execbuffer(ring,
> -					i915_gem_obj_ggtt_offset(so->obj),
> -					so->len,
> +					so.ggtt_offset,
> +					so.rodata->batch_items * 4,
>  					I915_DISPATCH_SECURE);
>  	if (ret)
>  		goto out;
>  
> -	i915_vma_move_to_active(i915_gem_obj_to_ggtt(so->obj), ring);
> +	i915_vma_move_to_active(i915_gem_obj_to_ggtt(so.obj), ring);
>  
> -	ret = __i915_add_request(ring, NULL, so->obj, NULL);
> +	ret = __i915_add_request(ring, NULL, so.obj, NULL);
>  	/* __i915_add_request moves object to inactive if it fails */
>  out:
> -	render_state_free(so);
> +	render_state_fini(&so);
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_renderstate.h b/drivers/gpu/drm/i915/intel_renderstate.h
> index a5e783a9928a..fd4f66231d30 100644
> --- a/drivers/gpu/drm/i915/intel_renderstate.h
> +++ b/drivers/gpu/drm/i915/intel_renderstate.h
> @@ -28,7 +28,6 @@
>  
>  struct intel_renderstate_rodata {
>  	const u32 *reloc;
> -	const u32 reloc_items;
>  	const u32 *batch;
>  	const u32 batch_items;
>  };
> @@ -40,7 +39,6 @@ extern const struct intel_renderstate_rodata gen8_null_state;
>  #define RO_RENDERSTATE(_g)						\
>  	const struct intel_renderstate_rodata gen ## _g ## _null_state = { \
>  		.reloc = gen ## _g ## _null_state_relocs,		\
> -		.reloc_items = sizeof(gen ## _g ## _null_state_relocs)/4, \
>  		.batch = gen ## _g ## _null_state_batch,		\
>  		.batch_items = sizeof(gen ## _g ## _null_state_batch)/4, \
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.c b/drivers/gpu/drm/i915/intel_renderstate_gen6.c
> index 740538ad0977..56c1429d8a60 100644
> --- a/drivers/gpu/drm/i915/intel_renderstate_gen6.c
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.c
> @@ -6,6 +6,7 @@ static const u32 gen6_null_state_relocs[] = {
>  	0x0000002c,
>  	0x000001e0,
>  	0x000001e4,
> +	-1,
>  };
>  
>  static const u32 gen6_null_state_batch[] = {
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen7.c b/drivers/gpu/drm/i915/intel_renderstate_gen7.c
> index 6fa7ff2a1298..419e35a7b0ff 100644
> --- a/drivers/gpu/drm/i915/intel_renderstate_gen7.c
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen7.c
> @@ -5,6 +5,7 @@ static const u32 gen7_null_state_relocs[] = {
>  	0x00000010,
>  	0x00000018,
>  	0x000001ec,
> +	-1,
>  };
>  
>  static const u32 gen7_null_state_batch[] = {
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.c b/drivers/gpu/drm/i915/intel_renderstate_gen8.c
> index 5c875615d42a..75ef1b5de45c 100644
> --- a/drivers/gpu/drm/i915/intel_renderstate_gen8.c
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.c
> @@ -5,6 +5,7 @@ static const u32 gen8_null_state_relocs[] = {
>  	0x00000050,
>  	0x00000060,
>  	0x000003ec,
> +	-1,
>  };
>  
>  static const u32 gen8_null_state_batch[] = {
> -- 
> 2.0.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2014-06-16 11:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-10 10:23 [PATCH] drm/i915: Simplify processing of the golden render context state Chris Wilson
2014-06-16 11:40 ` Mika Kuoppala [this message]
2014-06-16 17:53   ` Daniel Vetter

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=87fvj5ozeq.fsf@gaia.fi.intel.com \
    --to=mika.kuoppala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --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