[Intel-gfx] [RFC 4/7] drm/i915: Add checks specific to async flips

[B S, Karthik]

> -----Original Message-----
> From: Zanoni, Paulo R <paulo.r.zanoni at intel.com>
> Sent: Tuesday, March 10, 2020 4:49 AM
> To: B S, Karthik <karthik.b.s at intel.com>; intel-gfx at lists.freedesktop.org
> Cc: ville.syrjala at linux.intel.com; Kulkarni, Vandita
> <vandita.kulkarni at intel.com>; Shankar, Uma <uma.shankar at 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 at 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)) {