| Message ID | 1497981426-27203-1-git-send-email-Andrey.Grodzovsky@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2017-06-20 01:57 PM, Andrey Grodzovsky wrote: > Problem : While running IGT kms_atomic_transition test suite i encountered > a hang in drmHandleEvent immediately following an atomic_commit. > After dumping the atomic state I relized that in this case there was > not even one CRTC attached to the state and only disabled > planes. This probably due to a commit which hadn't changed any property > which would require attaching crtc state. This means drmHandleEvent > will never wake up from read since without CRTC in atomic state > the event fd will not be signaled. > > Fix: Protect against this issue by failing atomic_commit early in > drm_mode_atomic_commit where such probelm can be identified. > > v2: > Fix typos and extra newlines. > > Change-Id: I3ee28ffae35fd1e8bfe553146c44da53da02e6f8 > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > --- > drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index a567310..48145bf 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1933,7 +1933,7 @@ static int prepare_crtc_signaling(struct drm_device *dev, > { > struct drm_crtc *crtc; > struct drm_crtc_state *crtc_state; > - int i, ret; > + int i, c = 0, ret; > > if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) > return 0; > @@ -1994,8 +1994,17 @@ static int prepare_crtc_signaling(struct drm_device *dev, > > crtc_state->event->base.fence = fence; > } > + > + c++; > } > > + /* > + * Having this flag means user mode pends on event which will never > + * reach due to lack of at least one CRTC for signaling > + */ > + if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) > + return -EINVAL; > + > return 0; > } > >
On Mon, Jun 26, 2017 at 03:44:07PM -0400, Harry Wentland wrote: > On 2017-06-20 01:57 PM, Andrey Grodzovsky wrote: > > Problem : While running IGT kms_atomic_transition test suite i encountered > > a hang in drmHandleEvent immediately following an atomic_commit. > > After dumping the atomic state I relized that in this case there was > > not even one CRTC attached to the state and only disabled > > planes. This probably due to a commit which hadn't changed any property > > which would require attaching crtc state. This means drmHandleEvent > > will never wake up from read since without CRTC in atomic state > > the event fd will not be signaled. > > > > Fix: Protect against this issue by failing atomic_commit early in > > drm_mode_atomic_commit where such probelm can be identified. > > > > v2: > > Fix typos and extra newlines. > > > > Change-Id: I3ee28ffae35fd1e8bfe553146c44da53da02e6f8 > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > > Reviewed-by: Harry Wentland <harry.wentland@amd.com> Stalling on this hoping for the igt patch. Does it exist already? -Daniel > > Harry > > > --- > > drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index a567310..48145bf 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -1933,7 +1933,7 @@ static int prepare_crtc_signaling(struct drm_device *dev, > > { > > struct drm_crtc *crtc; > > struct drm_crtc_state *crtc_state; > > - int i, ret; > > + int i, c = 0, ret; > > > > if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) > > return 0; > > @@ -1994,8 +1994,17 @@ static int prepare_crtc_signaling(struct drm_device *dev, > > > > crtc_state->event->base.fence = fence; > > } > > + > > + c++; > > } > > > > + /* > > + * Having this flag means user mode pends on event which will never > > + * reach due to lack of at least one CRTC for signaling > > + */ > > + if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) > > + return -EINVAL; > > + > > return 0; > > } > > > >
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index a567310..48145bf 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1933,7 +1933,7 @@ static int prepare_crtc_signaling(struct drm_device *dev, { struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; - int i, ret; + int i, c = 0, ret; if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) return 0; @@ -1994,8 +1994,17 @@ static int prepare_crtc_signaling(struct drm_device *dev, crtc_state->event->base.fence = fence; } + + c++; } + /* + * Having this flag means user mode pends on event which will never + * reach due to lack of at least one CRTC for signaling + */ + if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) + return -EINVAL; + return 0; }
Problem : While running IGT kms_atomic_transition test suite i encountered a hang in drmHandleEvent immediately following an atomic_commit. After dumping the atomic state I relized that in this case there was not even one CRTC attached to the state and only disabled planes. This probably due to a commit which hadn't changed any property which would require attaching crtc state. This means drmHandleEvent will never wake up from read since without CRTC in atomic state the event fd will not be signaled. Fix: Protect against this issue by failing atomic_commit early in drm_mode_atomic_commit where such probelm can be identified. v2: Fix typos and extra newlines. Change-Id: I3ee28ffae35fd1e8bfe553146c44da53da02e6f8 Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> --- drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)