[Intel-gfx] [PATCH v2 5/6] drm/i915: Add checks specific to async flips

[Karthik B S]

On 4/20/2020 11:28 PM, Paulo Zanoni wrote:
> Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
>> Support added only for async flips on primary plane.
>> If flip is requested on any other plane, reject it.
>>
>> Make sure there is no change in fbc, offset and framebuffer modifiers
>> when async flip is requested.
>>
>> If any of these are modified, reject async flip.
>>
>> v2: -Replace DRM_ERROR (Paulo)
>>      -Add check for changes in OFFSET, FBC, RC(Paulo)
>>
>> Signed-off-by: Karthik B S <karthik.b.s at intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c | 59 ++++++++++++++++++++
>>   1 file changed, 59 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index a5203de24045..ac7f26a9ac4a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -14669,6 +14669,57 @@ static bool intel_cpu_transcoders_need_modeset(struct intel_atomic_state *state,
>>   	return false;
>>   }
>>   
>> +static int intel_atomic_check_async(struct intel_atomic_state *state)
>> +{
>> +	struct drm_plane *plane;
>> +	struct drm_plane_state *plane_state;
>> +	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
>> +	struct intel_plane_state *new_plane_state, *old_plane_state;
>> +	struct intel_crtc *crtc;
>> +	struct intel_plane *intel_plane;
>> +	int i, j;
>> +
>> +	/*FIXME: Async flip is only supported for primary plane currently
>> +	 * Support for overlays to be added.
>> +	 */
>> +
>> +	/*TODO: Check if the user space can handle the EINVAL return
>> +	 * or if it needs to be handled differently
>> +	 */
> 
> Does this mean we still didn't test the series against real user space?
> I mean, X server with xf86-video-modesetting and some real workload
> instead of just igt. I can't feel confident to give r-b tags if I know
> the patches were not tested yet. The series should probably have been
> marked as an RFC.
> 

I've now run benchmarking tests for this series on Gen9 platform and the 
initial results are positive.

Thanks,
Karthik.B.S
> 
>> +	for_each_new_plane_in_state(&state->base, plane, plane_state, j) {
>> +		if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
>> +			DRM_DEBUG_KMS("Async flips is NOT supported for non-primary plane\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>> +					    new_crtc_state, i) {
>> +		if (old_crtc_state->enable_fbc != new_crtc_state->enable_fbc) {
>> +			DRM_DEBUG_KMS("FBC status cannot be changed in async flip\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	for_each_oldnew_intel_plane_in_state(state, intel_plane, old_plane_state,
>> +					     new_plane_state, i) {
>> +		if ((old_plane_state->color_plane[0].x !=
>> +		     new_plane_state->color_plane[0].x) ||
>> +		    (old_plane_state->color_plane[0].y !=
>> +		     new_plane_state->color_plane[0].y)) {
>> +			DRM_DEBUG_KMS("Offsets cannot be changed in async\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		if (old_plane_state->uapi.fb->modifier !=
>> +		    new_plane_state->uapi.fb->modifier) {
>> +			DRM_DEBUG_KMS("Framebuffer modifiers cannot be changed in async flip\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +	return 0;
>> +}
>> +
>>   /**
>>    * intel_atomic_check - validate state object
>>    * @dev: drm device
>> @@ -14697,6 +14748,14 @@ static int intel_atomic_check(struct drm_device *dev,
>>   	if (ret)
>>   		goto fail;
>>   
>> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>> +		if (new_crtc_state->uapi.async_flip) {
>> +			ret = intel_atomic_check_async(state);
>> +			if  (ret)
>> +				goto fail;
>> +		}
>> +	}
>> +
>>   	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>>   					    new_crtc_state, i) {
>>   		if (!needs_modeset(new_crtc_state)) {
>