Re: [PATCH] drm/i915: Split alloc from init for lrc

[Nick Hoath <nicholas.hoath@intel.com>]

On 18/08/2015 15:31, Chris Wilson wrote:
> On Tue, Aug 18, 2015 at 03:23:32PM +0100, Nick Hoath wrote:
>> Extend init/init_hw split to context init.
>>     - Move context initialisation in to i915_gem_init_hw
>>     - Move one off initialisation for render ring to
>>          i915_gem_validate_context
>>     - Move default context initialisation to logical_ring_init
>>
>> Rename intel_lr_context_deferred_create to
>> intel_lr_context_deferred_alloc, to reflect reduced functionality &
>> alloc/init split.
>>
>> This patch is intended to split out the allocation of resources & initialisation
>> to allow easier reuse of code for resume/gpu reset.
>>
>> v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
>>      Left ->init_context int intel_lr_context_deferred_alloc (Daniel Vetter)
>>      Remove unnecessary init flag & ring type test. (Daniel Vetter)
>>      Improve commit message (Daniel Vetter)
>>
>> Issue: VIZ-4798
>> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h            |   1 -
>>   drivers/gpu/drm/i915/i915_gem.c            |  12 +--
>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |   3 +-
>>   drivers/gpu/drm/i915/intel_lrc.c           | 147 ++++++++++++++---------------
>>   drivers/gpu/drm/i915/intel_lrc.h           |   4 +-
>>   5 files changed, 80 insertions(+), 87 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index f7fd519..844ccf0 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -880,7 +880,6 @@ struct intel_context {
>>   	} legacy_hw_ctx;
>>
>>   	/* Execlists */
>> -	bool rcs_initialized;
>>   	struct {
>>   		struct drm_i915_gem_object *state;
>>   		struct intel_ringbuffer *ringbuf;
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 73293b4..3ccef2d 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4603,14 +4603,8 @@ int i915_gem_init_rings(struct drm_device *dev)
>>   			goto cleanup_vebox_ring;
>>   	}
>>
>> -	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
>> -	if (ret)
>> -		goto cleanup_bsd2_ring;
>> -
>>   	return 0;
>>
>> -cleanup_bsd2_ring:
>> -	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
>>   cleanup_vebox_ring:
>>   	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
>>   cleanup_blt_ring:
>> @@ -4629,6 +4623,7 @@ i915_gem_init_hw(struct drm_device *dev)
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	struct intel_engine_cs *ring;
>>   	int ret, i, j;
>> +	struct drm_i915_gem_request *req;
>>
>>   	if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
>>   		return -EIO;
>> @@ -4680,9 +4675,12 @@ i915_gem_init_hw(struct drm_device *dev)
>>   			goto out;
>>   	}
>>
>> +	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
>> +	if (ret)
>> +		goto out;
>
> This is the wrong location. Just kill set_seqno, the experiment has run
> its course and we now have a n igt to exercise seqno wraparound.
It has to be here as the seqno has to be initialised before it is used 
to create requests for the initialisation.
According to the commit history, the seqno has to be initialised to 
non-zero for proper functioning. Is this no longer true?
Maybe it should just be set to 1 instead of ~0-0x1000


> -Chris
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx