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

[Paulo Zanoni]

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.


> +	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)) {