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