All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] drm: Do not allow to set NOFB for active primary plane
@ 2019-04-15  8:00 Stanislav Lisovskiy
  2019-04-15 10:40 ` Maarten Lankhorst
  0 siblings, 1 reply; 8+ messages in thread
From: Stanislav Lisovskiy @ 2019-04-15  8:00 UTC (permalink / raw)
  To: dri-devel
  Cc: ville.syrjala, daniel.vetter, lakshminarayana.vudum,
	martin.peres, juha-pekka.heikkila

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

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,
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v1] drm: Do not allow to set NOFB for active primary plane
  2019-04-15  8:00 [PATCH v1] drm: Do not allow to set NOFB for active primary plane Stanislav Lisovskiy
@ 2019-04-15 10:40 ` Maarten Lankhorst
  2019-04-15 11:27   ` Lisovskiy, Stanislav
  0 siblings, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2019-04-15 10:40 UTC (permalink / raw)
  To: Stanislav Lisovskiy, dri-devel
  Cc: daniel.vetter, lakshminarayana.vudum, ville.syrjala,
	martin.peres, juha-pekka.heikkila

Op 15-04-2019 om 10:00 schreef Stanislav Lisovskiy:
> 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 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(+)
>
> 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,

If userspace asks to disable the primary plane, but not the crtc then that is not an atomic bug. The bug is in the userspace requesting this (Xorg).

If you want to include such a check, the correct place is at the end of drm_atomic_check_only(), after ->atomic_check(), return -EINVAL there.

But such a patch will not be accepted. It's perfectly acceptable to disable the primary plane for whatever reason, and some of our internal tests do so.

~Maarten


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1] drm: Do not allow to set NOFB for active primary plane
  2019-04-15 10:40 ` Maarten Lankhorst
@ 2019-04-15 11:27   ` Lisovskiy, Stanislav
  2019-04-16  7:38     ` Daniel Vetter
  0 siblings, 1 reply; 8+ messages in thread
From: Lisovskiy, Stanislav @ 2019-04-15 11:27 UTC (permalink / raw)
  To: dri-devel, maarten.lankhorst
  Cc: daniel.vetter, Peres, Martin, Syrjala, Ville, Vudum,
	Lakshminarayana, Heikkila, Juha-pekka

On Mon, 2019-04-15 at 12:40 +0200, Maarten Lankhorst wrote:
> Op 15-04-2019 om 10:00 schreef Stanislav Lisovskiy:
> > 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 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(+)
> > 
> > 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,
> 
> If userspace asks to disable the primary plane, but not the crtc then
> that is not an atomic bug. The bug is in the userspace requesting
> this (Xorg).
> 
> If you want to include such a check, the correct place is at the end
> of drm_atomic_check_only(), after ->atomic_check(), return -EINVAL
> there.
> 
> But such a patch will not be accepted. It's perfectly acceptable to
> disable the primary plane for whatever reason, and some of our
> internal tests do so.

Well, yeah that's why I was not quite sure, if this is correct.
However to me the problem seems to be that userspace is disabling
primary plane without realizing it. For example on my skl I see
that drm_atomic_helper_check_plane_state complains that primary
plane has no FB and then disables it.

> 
> ~Maarten
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1] drm: Do not allow to set NOFB for active primary plane
  2019-04-15 11:27   ` Lisovskiy, Stanislav
@ 2019-04-16  7:38     ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2019-04-16  7:38 UTC (permalink / raw)
  To: Lisovskiy, Stanislav
  Cc: Syrjala, Ville, daniel.vetter, Vudum, Lakshminarayana, dri-devel,
	Peres, Martin, Heikkila, Juha-pekka

On Mon, Apr 15, 2019 at 11:27:25AM +0000, Lisovskiy, Stanislav wrote:
> On Mon, 2019-04-15 at 12:40 +0200, Maarten Lankhorst wrote:
> > Op 15-04-2019 om 10:00 schreef Stanislav Lisovskiy:
> > > 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 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(+)
> > > 
> > > 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,
> > 
> > If userspace asks to disable the primary plane, but not the crtc then
> > that is not an atomic bug. The bug is in the userspace requesting
> > this (Xorg).
> > 
> > If you want to include such a check, the correct place is at the end
> > of drm_atomic_check_only(), after ->atomic_check(), return -EINVAL
> > there.
> > 
> > But such a patch will not be accepted. It's perfectly acceptable to
> > disable the primary plane for whatever reason, and some of our
> > internal tests do so.
> 
> Well, yeah that's why I was not quite sure, if this is correct.
> However to me the problem seems to be that userspace is disabling
> primary plane without realizing it. For example on my skl I see
> that drm_atomic_helper_check_plane_state complains that primary
> plane has no FB and then disables it.

Yeah we need to figure out where this is coming from. Is this with sna or
-modesetting or something else?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1] drm: Do not allow to set NOFB for active primary plane
  2019-04-15 12:41   ` Lisovskiy, Stanislav
@ 2019-04-15 16:14     ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2019-04-15 16:14 UTC (permalink / raw)
  To: Lisovskiy, Stanislav
  Cc: Syrjala, Ville, daniel.vetter, Vudum, Lakshminarayana, dri-devel,
	Peres, Martin, Heikkila, Juha-pekka

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
> > 
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1] drm: Do not allow to set NOFB for active primary plane
  2019-04-15 12:36 ` Ville Syrjälä
@ 2019-04-15 12:41   ` Lisovskiy, Stanislav
  2019-04-15 16:14     ` Daniel Vetter
  0 siblings, 1 reply; 8+ messages in thread
From: Lisovskiy, Stanislav @ 2019-04-15 12:41 UTC (permalink / raw)
  To: ville.syrjala
  Cc: Syrjala, Ville, daniel.vetter, Peres, Martin, dri-devel, Vudum,
	Lakshminarayana, Heikkila, Juha-pekka

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
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1] drm: Do not allow to set NOFB for active primary plane
  2019-04-15  7:58 Stanislav Lisovskiy
@ 2019-04-15 12:36 ` Ville Syrjälä
  2019-04-15 12:41   ` Lisovskiy, Stanislav
  0 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2019-04-15 12:36 UTC (permalink / raw)
  To: Stanislav Lisovskiy
  Cc: ville.syrjala, daniel.vetter, lakshminarayana.vudum, dri-devel,
	martin.peres, juha-pekka.heikkila

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

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v1] drm: Do not allow to set NOFB for active primary plane
@ 2019-04-15  7:58 Stanislav Lisovskiy
  2019-04-15 12:36 ` Ville Syrjälä
  0 siblings, 1 reply; 8+ messages in thread
From: Stanislav Lisovskiy @ 2019-04-15  7:58 UTC (permalink / raw)
  To: dri-devel
  Cc: ville.syrjala, daniel.vetter, lakshminarayana.vudum,
	martin.peres, juha-pekka.heikkila

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

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,
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-04-16  7:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15  8:00 [PATCH v1] drm: Do not allow to set NOFB for active primary plane Stanislav Lisovskiy
2019-04-15 10:40 ` Maarten Lankhorst
2019-04-15 11:27   ` Lisovskiy, Stanislav
2019-04-16  7:38     ` Daniel Vetter
  -- strict thread matches above, loose matches on Subject: below --
2019-04-15  7:58 Stanislav Lisovskiy
2019-04-15 12:36 ` Ville Syrjälä
2019-04-15 12:41   ` Lisovskiy, Stanislav
2019-04-15 16:14     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.