diff mbox series

[RFC,4/7] drm/i915: Add checks specific to async flips

Message ID 20200306113927.16904-5-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 March 6, 2020, 11:39 a.m. UTC
Support added only for async flips on primary plane.
If flip is requested on any other plane, reject it.

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 29 ++++++++++++++++++++
 1 file changed, 29 insertions(+)

Comments

Zanoni, Paulo R March 9, 2020, 11:19 p.m. UTC | #1
Em sex, 2020-03-06 às 17:09 +0530, Karthik B S escreveu:
> Support added only for async flips on primary plane.
> If flip is requested on any other plane, reject it.
> 
> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 29 ++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 25fad5d01e67..a8de08c3773e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14732,6 +14732,31 @@ 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 *crtc_state;
> +	struct intel_crtc *crtc;
> +	int i, j;
> +
> +	/*FIXME: Async flip is only supported for primary plane currently
> +	 * Support for overlays to be added.
> +	 */
> +	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		if (crtc_state->uapi.async_flip) {
> +			for_each_new_plane_in_state(&state->base,
> +						    plane, plane_state, j) {
> +				if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
> +					DRM_ERROR("Async flips is NOT supported for non-primary plane\n");

My understanding is that this is not a case of DRM_ERROR, since it's
just user space doing something it shouldn't.

Have we checked if xf86-video-modesetting or some other current user
space is going to try these calls on non-primary when async_flips are
enabled? Specifically, how does it react when it gets the EINVAL? We
should try to avoid the case where we release a Kernel that current
user space is not prepared for (even if it's not the Kernel's fault).


> +					return -EINVAL;
> +				}
> +			}
> +		}
> +	}
> +	return 0;
> +}
> +
>  /**
>   * intel_atomic_check - validate state object
>   * @dev: drm device
> @@ -14760,6 +14785,10 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	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 March 10, 2020, 12:51 p.m. UTC | #2
> -----Original Message-----
> From: Zanoni, Paulo R <paulo.r.zanoni@intel.com>
> Sent: Tuesday, March 10, 2020 4:49 AM
> To: B S, Karthik <karthik.b.s@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: ville.syrjala@linux.intel.com; Kulkarni, Vandita
> <vandita.kulkarni@intel.com>; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [RFC 4/7] drm/i915: Add checks specific to async flips
> 
> Em sex, 2020-03-06 às 17:09 +0530, Karthik B S escreveu:
> > Support added only for async flips on primary plane.
> > If flip is requested on any other plane, reject it.
> >
> > Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 29
> > ++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 25fad5d01e67..a8de08c3773e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14732,6 +14732,31 @@ 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 *crtc_state;
> > +	struct intel_crtc *crtc;
> > +	int i, j;
> > +
> > +	/*FIXME: Async flip is only supported for primary plane currently
> > +	 * Support for overlays to be added.
> > +	 */
> > +	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> > +		if (crtc_state->uapi.async_flip) {
> > +			for_each_new_plane_in_state(&state->base,
> > +						    plane, plane_state, j) {
> > +				if (plane->type !=
> DRM_PLANE_TYPE_PRIMARY) {
> > +					DRM_ERROR("Async flips is NOT
> supported for non-primary
> > +plane\n");
> 
> My understanding is that this is not a case of DRM_ERROR, since it's just user
> space doing something it shouldn't.

Sure. Will fix that in the next revision.
> 
> Have we checked if xf86-video-modesetting or some other current user
> space is going to try these calls on non-primary when async_flips are
> enabled? Specifically, how does it react when it gets the EINVAL? We should
> try to avoid the case where we release a Kernel that current user space is not
> prepared for (even if it's not the Kernel's fault).

Will check the user space behavior and update accordingly in the next revision. 
> 
> 
> > +					return -EINVAL;
> > +				}
> > +			}
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> >  /**
> >   * intel_atomic_check - validate state object
> >   * @dev: drm device
> > @@ -14760,6 +14785,10 @@ static int intel_atomic_check(struct
> drm_device *dev,
> >  	if (ret)
> >  		goto fail;
> >
> > +	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 25fad5d01e67..a8de08c3773e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14732,6 +14732,31 @@  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 *crtc_state;
+	struct intel_crtc *crtc;
+	int i, j;
+
+	/*FIXME: Async flip is only supported for primary plane currently
+	 * Support for overlays to be added.
+	 */
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		if (crtc_state->uapi.async_flip) {
+			for_each_new_plane_in_state(&state->base,
+						    plane, plane_state, j) {
+				if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
+					DRM_ERROR("Async flips is NOT supported for non-primary plane\n");
+					return -EINVAL;
+				}
+			}
+		}
+	}
+	return 0;
+}
+
 /**
  * intel_atomic_check - validate state object
  * @dev: drm device
@@ -14760,6 +14785,10 @@  static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	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)) {