diff mbox series

[v2,5/6] drm/i915: Add checks specific to async flips

Message ID 20200420094746.20409-6-karthik.b.s@intel.com (mailing list archive)
State New, archived
Headers show
Series Asynchronous flip implementation for i915 | expand

Commit Message

Karthik B S April 20, 2020, 9:47 a.m. UTC
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@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 59 ++++++++++++++++++++
 1 file changed, 59 insertions(+)

Comments

Zanoni, Paulo R April 20, 2020, 5:58 p.m. UTC | #1
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@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)) {
Karthik B S May 29, 2020, 4:50 a.m. UTC | #2
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@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)) {
>
diff mbox series

Patch

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