Re: [PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 15-04-16 om 09:07 schreef Ander Conselvan De Oliveira:
> On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
>> Re-use unpin_work->pending, but also set vblank count before
>> intel_mark_page_flip_active to be sure.
> Be sure of what?
>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++++++-----
>>  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++-------------------
>>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>>  3 files changed, 18 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 9640738aabf2..df8073a2ffbe 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -582,9 +582,14 @@ static int i915_gem_pageflip_info(struct seq_file *m,
>> void *data)
>>  			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
>>  				   pipe, plane);
>>  		} else {
>> +			u32 pending;
>>  			u32 addr;
>>  
>> -			if (atomic_read(&work->pending) <
>> INTEL_FLIP_COMPLETE) {
>> +			pending = atomic_read(&work->pending);
>> +			if (pending == INTEL_FLIP_INACTIVE) {
>> +				seq_printf(m, "Flip ioctl preparing on pipe
>> %c (plane %c)\n",
>> +					   pipe, plane);
>> +			} else if (pending >= INTEL_FLIP_COMPLETE) {
>>  				seq_printf(m, "Flip queued on pipe %c (plane
>> %c)\n",
>>  					   pipe, plane);
>>  			} else {
>> @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
>> void *data)
>>  				   work->flip_queued_vblank,
>>  				   work->flip_ready_vblank,
>>  				   drm_crtc_vblank_count(&crtc->base));
>> -			if (work->enable_stall_check)
>> -				seq_puts(m, "Stall check enabled, ");
>> -			else
>> -				seq_puts(m, "Stall check waiting for page
>> flip ioctl, ");
>>  			seq_printf(m, "%d prepares\n", atomic_read(&work
>> ->pending));
>>  
>>  			if (INTEL_INFO(dev)->gen >= 4)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index f2be54a48727..618e034a7a5e 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
>> *mmio_flip)
>>  	if (work == NULL)
>>  		return;
>>  
>> -	intel_mark_page_flip_active(work);
>> -
>>  	intel_pipe_update_start(crtc);
>>  
>>  	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
>> @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
>> *mmio_flip)
>>  		ilk_do_mmio_flip(crtc, work);
>>  
>>  	intel_pipe_update_end(crtc);
>> +
>> +	intel_mark_page_flip_active(work);
> Is this to avoid triggering the stall check during the wait from a vblank
> evasion?
It's to ensure that if a vblank happens before pipe_update_end, we don't mark the flip as completed until we actually updated the mmio registers.
>
>>  }
>>  
>>  static void intel_mmio_flip_work_func(struct work_struct *work)
>> @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct
>> drm_device *dev,
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  	struct intel_unpin_work *work = intel_crtc->unpin_work;
>>  	u32 addr;
>> +	u32 pending;
>>  
>> -	if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
>> -		return true;
>> -
>> -	if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
>> -		return false;
>> -
>> -	if (!work->enable_stall_check)
>> -		return false;
>> +	pending = atomic_read(&work->pending);
>> +	if (pending != INTEL_FLIP_PENDING)
>> +		return pending == INTEL_FLIP_COMPLETE;
>>  
>>  	if (work->flip_ready_vblank == 0) {
>>  		if (work->flip_queued_req &&
>> @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc
>> *crtc,
>>  	 */
>>  	if (!mmio_flip) {
>>  		ret = i915_gem_object_sync(obj, engine, &request);
>> +		if (!ret && !request) {
>> +			request = i915_gem_request_alloc(engine, NULL);
>> +			ret = PTR_ERR_OR_ZERO(request);
>> +		}
>> +
>>  		if (ret)
>>  			goto cleanup_pending;
>>  	}
>> @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>>  	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
>>  						  obj, 0);
>>  	work->gtt_offset += intel_crtc->dspaddr_offset;
>> +	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>>  
>>  	if (mmio_flip) {
>>  		ret = intel_queue_mmio_flip(dev, crtc, obj);
>> @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc
>> *crtc,
>>  		i915_gem_request_assign(&work->flip_queued_req,
>>  					obj->last_write_req);
>>  	} else {
>> -		if (!request) {
>> -			request = i915_gem_request_alloc(engine, NULL);
>> -			if (IS_ERR(request)) {
>> -				ret = PTR_ERR(request);
>> -				goto cleanup_unpin;
>> -			}
>> -		}
>> -
>>  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj,
>> request,
>>  						   page_flip_flags);
>>  		if (ret)
>> @@ -11716,7 +11710,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>>  		i915_add_request_no_flush(request);
>>  
>>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
> Do we still need the assigment above?
>
It's used for rps boosting, so likely...
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx