| Message ID | 20190415075847.16483-1-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] drm: Do not allow to set NOFB for active primary plane | expand |
On Mon, Apr 15, 2019 at 10:58:47AM +0300, Stanislav Lisovskiy wrote: > There was an issue reported from end users, confirmed > also locally that when user executes xrandr --output <some DP> > --rotate left/right, the other eDP screen gets blank after rotation. > > Investigation showed that reason was that primary plane > was that screen was for some reason disabled, while cursor > plane was still enabled. > After some effort it turns out that userspace might wrongly > assign NOFB to active primary plane for some reason. > > Then this gets detected in drm_atomic_helper_check_plane_state, > called from ->plane_check and plane gets deactivated, leaving > the screen blank and cursor visible. This can be cured by reboot > or xrandr off/on sequence for that crtc. > > This patch is proposal to fix the issue by forbiding fb removal > from active primary plane. If one needs to remove fb plane must be > disabled first. > > Not sure if this is correct, however it fixes the issue at least. It is not. Removing the fb (and crtc) is how you disable a plane. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110375 > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > --- > drivers/gpu/drm/drm_atomic_uapi.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index ea797d4c82ee..e2f078b302f3 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -229,6 +229,15 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state, > { > struct drm_plane *plane = plane_state->plane; > > + if (!fb) { > + if (plane->state->visible && plane->type == DRM_PLANE_TYPE_PRIMARY) {a .visible is not even computed yet when this is called. > + DRM_DEBUG_ATOMIC("Not allowed to set [NOFB] for active" > + " primary [PLANE:%d:%s] - disable first", > + plane->base.id, plane->name); > + return; > + } > + } > + > if (fb) > DRM_DEBUG_ATOMIC("Set [FB:%d] for [PLANE:%d:%s] state %p\n", > fb->base.id, plane->base.id, plane->name, > -- > 2.17.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, 2019-04-15 at 15:36 +0300, Ville Syrjälä wrote: > On Mon, Apr 15, 2019 at 10:58:47AM +0300, Stanislav Lisovskiy wrote: > > There was an issue reported from end users, confirmed > > also locally that when user executes xrandr --output <some DP> > > --rotate left/right, the other eDP screen gets blank after > > rotation. > > > > Investigation showed that reason was that primary plane > > was that screen was for some reason disabled, while cursor > > plane was still enabled. > > After some effort it turns out that userspace might wrongly > > assign NOFB to active primary plane for some reason. > > > > Then this gets detected in drm_atomic_helper_check_plane_state, > > called from ->plane_check and plane gets deactivated, leaving > > the screen blank and cursor visible. This can be cured by reboot > > or xrandr off/on sequence for that crtc. > > > > This patch is proposal to fix the issue by forbiding fb removal > > from active primary plane. If one needs to remove fb plane must be > > disabled first. > > > > Not sure if this is correct, however it fixes the issue at least. > > It is not. Removing the fb (and crtc) is how you disable a plane. Ok, most likely this should be fixed in userspace then. As what I see is that it removes FB from the primary plane for eDP, once we rotate the other screen(why?), which is then turned off in drm_atomic_helper_check_plane_state(!fb check) and then we have this nice blank screen which you saw. If I prevent it somehow, theneverything works fine, however the origin of a problem seems to be in userspace. > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110375 > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > --- > > drivers/gpu/drm/drm_atomic_uapi.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > b/drivers/gpu/drm/drm_atomic_uapi.c > > index ea797d4c82ee..e2f078b302f3 100644 > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > @@ -229,6 +229,15 @@ drm_atomic_set_fb_for_plane(struct > > drm_plane_state *plane_state, > > { > > struct drm_plane *plane = plane_state->plane; > > > > + if (!fb) { > > + if (plane->state->visible && plane->type == > > DRM_PLANE_TYPE_PRIMARY) {a > > .visible is not even computed yet when this is called. > > > + DRM_DEBUG_ATOMIC("Not allowed to set [NOFB] for > > active" > > + " primary [PLANE:%d:%s] - > > disable first", > > + plane->base.id, plane->name); > > + return; > > + } > > + } > > + > > if (fb) > > DRM_DEBUG_ATOMIC("Set [FB:%d] for [PLANE:%d:%s] state > > %p\n", > > fb->base.id, plane->base.id, plane- > > >name, > > -- > > 2.17.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > >
On Mon, Apr 15, 2019 at 12:41:33PM +0000, Lisovskiy, Stanislav wrote: > On Mon, 2019-04-15 at 15:36 +0300, Ville Syrjälä wrote: > > On Mon, Apr 15, 2019 at 10:58:47AM +0300, Stanislav Lisovskiy wrote: > > > There was an issue reported from end users, confirmed > > > also locally that when user executes xrandr --output <some DP> > > > --rotate left/right, the other eDP screen gets blank after > > > rotation. > > > > > > Investigation showed that reason was that primary plane > > > was that screen was for some reason disabled, while cursor > > > plane was still enabled. > > > After some effort it turns out that userspace might wrongly > > > assign NOFB to active primary plane for some reason. > > > > > > Then this gets detected in drm_atomic_helper_check_plane_state, > > > called from ->plane_check and plane gets deactivated, leaving > > > the screen blank and cursor visible. This can be cured by reboot > > > or xrandr off/on sequence for that crtc. > > > > > > This patch is proposal to fix the issue by forbiding fb removal > > > from active primary plane. If one needs to remove fb plane must be > > > disabled first. > > > > > > Not sure if this is correct, however it fixes the issue at least. > > > > It is not. Removing the fb (and crtc) is how you disable a plane. > > Ok, most likely this should be fixed in userspace then. As what I see > is that it removes FB from the primary plane for eDP, once we rotate > the other screen(why?), which is then turned off > in drm_atomic_helper_check_plane_state(!fb check) and then we have this > nice blank screen which you saw. If I prevent it somehow, theneverything works fine, however the origin of a problem seems to be in userspace. If it's an RMFB, then the fallback for rejecting this config is to disable the entire CRTC. Which might trigger some hotplug action in userspace somehow, papering over the issue. But yeah, that's not really how it's supposed to be done, and we need to at least first understand what's going on before applying duct-tape. -Daniel > > > > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110375 > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > > --- > > > drivers/gpu/drm/drm_atomic_uapi.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > > b/drivers/gpu/drm/drm_atomic_uapi.c > > > index ea797d4c82ee..e2f078b302f3 100644 > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > > @@ -229,6 +229,15 @@ drm_atomic_set_fb_for_plane(struct > > > drm_plane_state *plane_state, > > > { > > > struct drm_plane *plane = plane_state->plane; > > > > > > + if (!fb) { > > > + if (plane->state->visible && plane->type == > > > DRM_PLANE_TYPE_PRIMARY) {a > > > > .visible is not even computed yet when this is called. > > > > > + DRM_DEBUG_ATOMIC("Not allowed to set [NOFB] for > > > active" > > > + " primary [PLANE:%d:%s] - > > > disable first", > > > + plane->base.id, plane->name); > > > + return; > > > + } > > > + } > > > + > > > if (fb) > > > DRM_DEBUG_ATOMIC("Set [FB:%d] for [PLANE:%d:%s] state > > > %p\n", > > > fb->base.id, plane->base.id, plane- > > > >name, > > > -- > > > 2.17.1 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > >
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index ea797d4c82ee..e2f078b302f3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -229,6 +229,15 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state, { struct drm_plane *plane = plane_state->plane; + if (!fb) { + if (plane->state->visible && plane->type == DRM_PLANE_TYPE_PRIMARY) { + DRM_DEBUG_ATOMIC("Not allowed to set [NOFB] for active" + " primary [PLANE:%d:%s] - disable first", + plane->base.id, plane->name); + return; + } + } + if (fb) DRM_DEBUG_ATOMIC("Set [FB:%d] for [PLANE:%d:%s] state %p\n", fb->base.id, plane->base.id, plane->name,
There was an issue reported from end users, confirmed also locally that when user executes xrandr --output <some DP> --rotate left/right, the other eDP screen gets blank after rotation. Investigation showed that reason was that primary plane was that screen was for some reason disabled, while cursor plane was still enabled. After some effort it turns out that userspace might wrongly assign NOFB to active primary plane for some reason. Then this gets detected in drm_atomic_helper_check_plane_state, called from ->plane_check and plane gets deactivated, leaving the screen blank and cursor visible. This can be cured by reboot or xrandr off/on sequence for that crtc. This patch is proposal to fix the issue by forbiding fb removal from active primary plane. If one needs to remove fb plane must be disabled first. Not sure if this is correct, however it fixes the issue at least. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110375 Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> --- drivers/gpu/drm/drm_atomic_uapi.c | 9 +++++++++ 1 file changed, 9 insertions(+)