All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-06-28 15:20 ` Kuogee Hsieh
  0 siblings, 0 replies; 16+ messages in thread
From: Kuogee Hsieh @ 2022-06-28 15:20 UTC (permalink / raw)
  To: robdclark, sean, swboyd, dianders, vkoul, daniel, airlied,
	agross, dmitry.baryshkov, bjorn.andersson
  Cc: quic_abhinavk, quic_aravindh, quic_khsieh, quic_sbillaka,
	freedreno, dri-devel, linux-arm-msm, linux-kernel

Some userspace presumes that the first connected connector is the main
display, where it's supposed to display e.g. the login screen. For
laptops, this should be the main panel.

This patch call drm_helper_move_panel_connectors_to_head() after
drm_bridge_connector_init() to make sure eDP stay at head of
connected connector list. This fixes unexpected corruption happen
at eDP panel if eDP is not placed at head of connected connector
list.

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_drm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index ce0ec3a..2d18884 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -136,5 +136,7 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
 
 	drm_connector_attach_encoder(connector, dp_display->encoder);
 
+	drm_helper_move_panel_connectors_to_head(dp_display->drm_dev);
+
 	return connector;
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-06-28 15:20 ` Kuogee Hsieh
  0 siblings, 0 replies; 16+ messages in thread
From: Kuogee Hsieh @ 2022-06-28 15:20 UTC (permalink / raw)
  To: robdclark, sean, swboyd, dianders, vkoul, daniel, airlied,
	agross, dmitry.baryshkov, bjorn.andersson
  Cc: quic_sbillaka, linux-arm-msm, quic_abhinavk, dri-devel,
	quic_khsieh, quic_aravindh, freedreno, linux-kernel

Some userspace presumes that the first connected connector is the main
display, where it's supposed to display e.g. the login screen. For
laptops, this should be the main panel.

This patch call drm_helper_move_panel_connectors_to_head() after
drm_bridge_connector_init() to make sure eDP stay at head of
connected connector list. This fixes unexpected corruption happen
at eDP panel if eDP is not placed at head of connected connector
list.

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_drm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index ce0ec3a..2d18884 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -136,5 +136,7 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
 
 	drm_connector_attach_encoder(connector, dp_display->encoder);
 
+	drm_helper_move_panel_connectors_to_head(dp_display->drm_dev);
+
 	return connector;
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
  2022-06-28 15:20 ` Kuogee Hsieh
@ 2022-06-28 20:13   ` Dmitry Baryshkov
  -1 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2022-06-28 20:13 UTC (permalink / raw)
  To: Kuogee Hsieh, robdclark, sean, swboyd, dianders, vkoul, daniel,
	airlied, agross, bjorn.andersson
  Cc: quic_sbillaka, linux-arm-msm, quic_abhinavk, dri-devel,
	quic_khsieh, quic_aravindh, freedreno, linux-kernel



On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>Some userspace presumes that the first connected connector is the main
>display, where it's supposed to display e.g. the login screen. For
>laptops, this should be the main panel.
>
>This patch call drm_helper_move_panel_connectors_to_head() after
>drm_bridge_connector_init() to make sure eDP stay at head of
>connected connector list. This fixes unexpected corruption happen
>at eDP panel if eDP is not placed at head of connected connector
>list.

The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
Also could you please describe the mind of corruption you are observing?


>
>Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>---
> drivers/gpu/drm/msm/dp/dp_drm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
>index ce0ec3a..2d18884 100644
>--- a/drivers/gpu/drm/msm/dp/dp_drm.c
>+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
>@@ -136,5 +136,7 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
> 
> 	drm_connector_attach_encoder(connector, dp_display->encoder);
> 
>+	drm_helper_move_panel_connectors_to_head(dp_display->drm_dev);
>+
> 	return connector;
> }

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-06-28 20:13   ` Dmitry Baryshkov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2022-06-28 20:13 UTC (permalink / raw)
  To: Kuogee Hsieh, robdclark, sean, swboyd, dianders, vkoul, daniel,
	airlied, agross, bjorn.andersson
  Cc: quic_abhinavk, quic_aravindh, quic_khsieh, quic_sbillaka,
	freedreno, dri-devel, linux-arm-msm, linux-kernel



On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>Some userspace presumes that the first connected connector is the main
>display, where it's supposed to display e.g. the login screen. For
>laptops, this should be the main panel.
>
>This patch call drm_helper_move_panel_connectors_to_head() after
>drm_bridge_connector_init() to make sure eDP stay at head of
>connected connector list. This fixes unexpected corruption happen
>at eDP panel if eDP is not placed at head of connected connector
>list.

The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
Also could you please describe the mind of corruption you are observing?


>
>Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>---
> drivers/gpu/drm/msm/dp/dp_drm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
>index ce0ec3a..2d18884 100644
>--- a/drivers/gpu/drm/msm/dp/dp_drm.c
>+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
>@@ -136,5 +136,7 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
> 
> 	drm_connector_attach_encoder(connector, dp_display->encoder);
> 
>+	drm_helper_move_panel_connectors_to_head(dp_display->drm_dev);
>+
> 	return connector;
> }

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
  2022-06-28 20:13   ` Dmitry Baryshkov
@ 2022-06-30  0:36     ` Doug Anderson
  -1 siblings, 0 replies; 16+ messages in thread
From: Doug Anderson @ 2022-06-30  0:36 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Stephen Boyd, Vinod Koul, Daniel Vetter,
	David Airlie, Andy Gross, Bjorn Andersson, Abhinav Kumar (QUIC),
	Aravind Venkateswaran (QUIC), Kuogee Hsieh (QUIC),
	Sankeerth Billakanti, freedreno, dri-devel, linux-arm-msm, LKML

Hi,

On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
> >Some userspace presumes that the first connected connector is the main
> >display, where it's supposed to display e.g. the login screen. For
> >laptops, this should be the main panel.
> >
> >This patch call drm_helper_move_panel_connectors_to_head() after
> >drm_bridge_connector_init() to make sure eDP stay at head of
> >connected connector list. This fixes unexpected corruption happen
> >at eDP panel if eDP is not placed at head of connected connector
> >list.
>
> The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
> Also could you please describe the mind of corruption you are observing?

I've spent a whole bunch of time poking at this and in the end my
conclusion is this:

1. The glitchyness seems to be a result of the Chrome OS userspace
somehow telling the kernel to do something wrong.

2. I believe (though I have no proof other than Kuogee's patch fixing
things) that the Chrome OS userspace is simply confused by the eDP
connector being second. This would imply that Kuogee's patch is
actually the right one.

3. It would be ideal if the Chrome OS userspace were fixed to handle
this, but it's an area of code that I've never looked at. It also
seems terribly low priority to fix since apparently other OSes have
similar problems (seems like this code was originally added by
RedHat?)


Specifically, I tested with a similar but "persistent" glitch that I
reproduced. The glitch Kuogee was digging into was a transitory glitch
on the eDP (internal) display when you plugged in a DP (external)
display. It would show up for a frame or two and then be fixed. I can
get a similar-looking glitch (vertical black and white bars) that
persists by doing these steps on a Chrome OS device (and Chrome OS
kernel):

a) Observe screen looks good.
b) Observe DP not connected.
c) Plug in DP
d) See transitory glitch on screen, then it all looks fine.
e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
f) Wait for screen to turn off
g) Unplug DP
h) Hit key on keyboard to wake device.
i) See glitchy.
j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
--ac_screen_off_delay=10000

Once I'm in the persistent glitch:

* The "screenshot" command in Chrome OS shows corruption. Not exactly
black and white bars, but the image produced has distinct bands of
garbage.

* I can actually toggle between VT2 and the main screen (VT1). Note
that VT1/VT2 are not quite the normal Linux managed solution--I
believe they're handled by frecon. In any case, when I switch to VT2
it looks normal (I can see the login prompt). Then back to VT1 and the
vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
glitch again. This implies (especially with the extra evidence of
screenshot) that the display controller hardware is all fine and that
it's the underlying data that's somehow messed up.

When I pick Kuogee's patch then this "persistent" glitch goes away
just like the transitory one does.

I'm going to go ahead and do:

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-06-30  0:36     ` Doug Anderson
  0 siblings, 0 replies; 16+ messages in thread
From: Doug Anderson @ 2022-06-30  0:36 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: freedreno, Sankeerth Billakanti, Abhinav Kumar (QUIC),
	David Airlie, Kuogee Hsieh (QUIC),
	Vinod Koul, dri-devel, Stephen Boyd, Andy Gross, linux-arm-msm,
	Aravind Venkateswaran (QUIC),
	Bjorn Andersson, Sean Paul, LKML

Hi,

On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
> >Some userspace presumes that the first connected connector is the main
> >display, where it's supposed to display e.g. the login screen. For
> >laptops, this should be the main panel.
> >
> >This patch call drm_helper_move_panel_connectors_to_head() after
> >drm_bridge_connector_init() to make sure eDP stay at head of
> >connected connector list. This fixes unexpected corruption happen
> >at eDP panel if eDP is not placed at head of connected connector
> >list.
>
> The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
> Also could you please describe the mind of corruption you are observing?

I've spent a whole bunch of time poking at this and in the end my
conclusion is this:

1. The glitchyness seems to be a result of the Chrome OS userspace
somehow telling the kernel to do something wrong.

2. I believe (though I have no proof other than Kuogee's patch fixing
things) that the Chrome OS userspace is simply confused by the eDP
connector being second. This would imply that Kuogee's patch is
actually the right one.

3. It would be ideal if the Chrome OS userspace were fixed to handle
this, but it's an area of code that I've never looked at. It also
seems terribly low priority to fix since apparently other OSes have
similar problems (seems like this code was originally added by
RedHat?)


Specifically, I tested with a similar but "persistent" glitch that I
reproduced. The glitch Kuogee was digging into was a transitory glitch
on the eDP (internal) display when you plugged in a DP (external)
display. It would show up for a frame or two and then be fixed. I can
get a similar-looking glitch (vertical black and white bars) that
persists by doing these steps on a Chrome OS device (and Chrome OS
kernel):

a) Observe screen looks good.
b) Observe DP not connected.
c) Plug in DP
d) See transitory glitch on screen, then it all looks fine.
e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
f) Wait for screen to turn off
g) Unplug DP
h) Hit key on keyboard to wake device.
i) See glitchy.
j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
--ac_screen_off_delay=10000

Once I'm in the persistent glitch:

* The "screenshot" command in Chrome OS shows corruption. Not exactly
black and white bars, but the image produced has distinct bands of
garbage.

* I can actually toggle between VT2 and the main screen (VT1). Note
that VT1/VT2 are not quite the normal Linux managed solution--I
believe they're handled by frecon. In any case, when I switch to VT2
it looks normal (I can see the login prompt). Then back to VT1 and the
vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
glitch again. This implies (especially with the extra evidence of
screenshot) that the display controller hardware is all fine and that
it's the underlying data that's somehow messed up.

When I pick Kuogee's patch then this "persistent" glitch goes away
just like the transitory one does.

I'm going to go ahead and do:

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
  2022-06-30  0:36     ` Doug Anderson
@ 2022-06-30  1:57       ` Rob Clark
  -1 siblings, 0 replies; 16+ messages in thread
From: Rob Clark @ 2022-06-30  1:57 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Dmitry Baryshkov, Sean Paul, Stephen Boyd, Vinod Koul,
	Daniel Vetter, David Airlie, Andy Gross, Bjorn Andersson,
	Abhinav Kumar (QUIC), Aravind Venkateswaran (QUIC),
	Kuogee Hsieh (QUIC),
	Sankeerth Billakanti, freedreno, dri-devel, linux-arm-msm, LKML

On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
> > >Some userspace presumes that the first connected connector is the main
> > >display, where it's supposed to display e.g. the login screen. For
> > >laptops, this should be the main panel.
> > >
> > >This patch call drm_helper_move_panel_connectors_to_head() after
> > >drm_bridge_connector_init() to make sure eDP stay at head of
> > >connected connector list. This fixes unexpected corruption happen
> > >at eDP panel if eDP is not placed at head of connected connector
> > >list.
> >
> > The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
> > Also could you please describe the mind of corruption you are observing?
>
> I've spent a whole bunch of time poking at this and in the end my
> conclusion is this:
>
> 1. The glitchyness seems to be a result of the Chrome OS userspace
> somehow telling the kernel to do something wrong.
>
> 2. I believe (though I have no proof other than Kuogee's patch fixing
> things) that the Chrome OS userspace is simply confused by the eDP
> connector being second. This would imply that Kuogee's patch is
> actually the right one.
>
> 3. It would be ideal if the Chrome OS userspace were fixed to handle
> this, but it's an area of code that I've never looked at. It also
> seems terribly low priority to fix since apparently other OSes have
> similar problems (seems like this code was originally added by
> RedHat?)
>
>
> Specifically, I tested with a similar but "persistent" glitch that I
> reproduced. The glitch Kuogee was digging into was a transitory glitch
> on the eDP (internal) display when you plugged in a DP (external)
> display. It would show up for a frame or two and then be fixed. I can
> get a similar-looking glitch (vertical black and white bars) that
> persists by doing these steps on a Chrome OS device (and Chrome OS
> kernel):
>
> a) Observe screen looks good.
> b) Observe DP not connected.
> c) Plug in DP
> d) See transitory glitch on screen, then it all looks fine.
> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
> f) Wait for screen to turn off
> g) Unplug DP
> h) Hit key on keyboard to wake device.
> i) See glitchy.
> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
> --ac_screen_off_delay=10000
>
> Once I'm in the persistent glitch:
>
> * The "screenshot" command in Chrome OS shows corruption. Not exactly
> black and white bars, but the image produced has distinct bands of
> garbage.
>
> * I can actually toggle between VT2 and the main screen (VT1). Note
> that VT1/VT2 are not quite the normal Linux managed solution--I
> believe they're handled by frecon. In any case, when I switch to VT2
> it looks normal (I can see the login prompt). Then back to VT1 and the
> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
> glitch again. This implies (especially with the extra evidence of
> screenshot) that the display controller hardware is all fine and that
> it's the underlying data that's somehow messed up.

fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
simply just an un-renderered buffer being interpreted by the display
controller as UBWC (because userspace tells it to)

BR,
-R

> When I pick Kuogee's patch then this "persistent" glitch goes away
> just like the transitory one does.
>
> I'm going to go ahead and do:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Tested-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-06-30  1:57       ` Rob Clark
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Clark @ 2022-06-30  1:57 UTC (permalink / raw)
  To: Doug Anderson
  Cc: freedreno, Sankeerth Billakanti, David Airlie,
	Kuogee Hsieh (QUIC), Abhinav Kumar (QUIC),
	dri-devel, Stephen Boyd, Vinod Koul, Andy Gross, linux-arm-msm,
	Dmitry Baryshkov, Aravind Venkateswaran (QUIC),
	Bjorn Andersson, Sean Paul, LKML

On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
> > >Some userspace presumes that the first connected connector is the main
> > >display, where it's supposed to display e.g. the login screen. For
> > >laptops, this should be the main panel.
> > >
> > >This patch call drm_helper_move_panel_connectors_to_head() after
> > >drm_bridge_connector_init() to make sure eDP stay at head of
> > >connected connector list. This fixes unexpected corruption happen
> > >at eDP panel if eDP is not placed at head of connected connector
> > >list.
> >
> > The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
> > Also could you please describe the mind of corruption you are observing?
>
> I've spent a whole bunch of time poking at this and in the end my
> conclusion is this:
>
> 1. The glitchyness seems to be a result of the Chrome OS userspace
> somehow telling the kernel to do something wrong.
>
> 2. I believe (though I have no proof other than Kuogee's patch fixing
> things) that the Chrome OS userspace is simply confused by the eDP
> connector being second. This would imply that Kuogee's patch is
> actually the right one.
>
> 3. It would be ideal if the Chrome OS userspace were fixed to handle
> this, but it's an area of code that I've never looked at. It also
> seems terribly low priority to fix since apparently other OSes have
> similar problems (seems like this code was originally added by
> RedHat?)
>
>
> Specifically, I tested with a similar but "persistent" glitch that I
> reproduced. The glitch Kuogee was digging into was a transitory glitch
> on the eDP (internal) display when you plugged in a DP (external)
> display. It would show up for a frame or two and then be fixed. I can
> get a similar-looking glitch (vertical black and white bars) that
> persists by doing these steps on a Chrome OS device (and Chrome OS
> kernel):
>
> a) Observe screen looks good.
> b) Observe DP not connected.
> c) Plug in DP
> d) See transitory glitch on screen, then it all looks fine.
> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
> f) Wait for screen to turn off
> g) Unplug DP
> h) Hit key on keyboard to wake device.
> i) See glitchy.
> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
> --ac_screen_off_delay=10000
>
> Once I'm in the persistent glitch:
>
> * The "screenshot" command in Chrome OS shows corruption. Not exactly
> black and white bars, but the image produced has distinct bands of
> garbage.
>
> * I can actually toggle between VT2 and the main screen (VT1). Note
> that VT1/VT2 are not quite the normal Linux managed solution--I
> believe they're handled by frecon. In any case, when I switch to VT2
> it looks normal (I can see the login prompt). Then back to VT1 and the
> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
> glitch again. This implies (especially with the extra evidence of
> screenshot) that the display controller hardware is all fine and that
> it's the underlying data that's somehow messed up.

fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
simply just an un-renderered buffer being interpreted by the display
controller as UBWC (because userspace tells it to)

BR,
-R

> When I pick Kuogee's patch then this "persistent" glitch goes away
> just like the transitory one does.
>
> I'm going to go ahead and do:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Tested-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [Freedreno] [PATCH] drm/msm/dp: make eDP panel as the first connected connector
  2022-06-30  1:57       ` Rob Clark
@ 2022-06-30  2:10         ` Abhinav Kumar
  -1 siblings, 0 replies; 16+ messages in thread
From: Abhinav Kumar @ 2022-06-30  2:10 UTC (permalink / raw)
  To: Rob Clark, Doug Anderson
  Cc: freedreno, Sankeerth Billakanti, David Airlie,
	Kuogee Hsieh (QUIC),
	dri-devel, Stephen Boyd, Vinod Koul, Andy Gross, Daniel Vetter,
	linux-arm-msm, Dmitry Baryshkov, Aravind Venkateswaran (QUIC),
	Bjorn Andersson, Sean Paul, LKML



On 6/29/2022 6:57 PM, Rob Clark wrote:
> On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> wrote:
>>
>> Hi,
>>
>> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
>> <dmitry.baryshkov@linaro.org> wrote:
>>>
>>> On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>>>> Some userspace presumes that the first connected connector is the main
>>>> display, where it's supposed to display e.g. the login screen. For
>>>> laptops, this should be the main panel.
>>>>
>>>> This patch call drm_helper_move_panel_connectors_to_head() after
>>>> drm_bridge_connector_init() to make sure eDP stay at head of
>>>> connected connector list. This fixes unexpected corruption happen
>>>> at eDP panel if eDP is not placed at head of connected connector
>>>> list.
>>>
>>> The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
>>> Also could you please describe the mind of corruption you are observing?
>>
>> I've spent a whole bunch of time poking at this and in the end my
>> conclusion is this:
>>
>> 1. The glitchyness seems to be a result of the Chrome OS userspace
>> somehow telling the kernel to do something wrong.
>>
>> 2. I believe (though I have no proof other than Kuogee's patch fixing
>> things) that the Chrome OS userspace is simply confused by the eDP
>> connector being second. This would imply that Kuogee's patch is
>> actually the right one.
>>
>> 3. It would be ideal if the Chrome OS userspace were fixed to handle
>> this, but it's an area of code that I've never looked at. It also
>> seems terribly low priority to fix since apparently other OSes have
>> similar problems (seems like this code was originally added by
>> RedHat?)
>>
>>
>> Specifically, I tested with a similar but "persistent" glitch that I
>> reproduced. The glitch Kuogee was digging into was a transitory glitch
>> on the eDP (internal) display when you plugged in a DP (external)
>> display. It would show up for a frame or two and then be fixed. I can
>> get a similar-looking glitch (vertical black and white bars) that
>> persists by doing these steps on a Chrome OS device (and Chrome OS
>> kernel):
>>
>> a) Observe screen looks good.
>> b) Observe DP not connected.
>> c) Plug in DP
>> d) See transitory glitch on screen, then it all looks fine.
>> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
>> f) Wait for screen to turn off
>> g) Unplug DP
>> h) Hit key on keyboard to wake device.
>> i) See glitchy.
>> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
>> --ac_screen_off_delay=10000
>>
>> Once I'm in the persistent glitch:
>>
>> * The "screenshot" command in Chrome OS shows corruption. Not exactly
>> black and white bars, but the image produced has distinct bands of
>> garbage.
>>
>> * I can actually toggle between VT2 and the main screen (VT1). Note
>> that VT1/VT2 are not quite the normal Linux managed solution--I
>> believe they're handled by frecon. In any case, when I switch to VT2
>> it looks normal (I can see the login prompt). Then back to VT1 and the
>> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
>> glitch again. This implies (especially with the extra evidence of
>> screenshot) that the display controller hardware is all fine and that
>> it's the underlying data that's somehow messed up.
> 
> fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
> simply just an un-renderered buffer being interpreted by the display
> controller as UBWC (because userspace tells it to)
> 
> BR,
> -R

Acked and agree with the comments both of you have stated and looking at 
the corrupted buffers in the snapshot.

Hence,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

> 
>> When I pick Kuogee's patch then this "persistent" glitch goes away
>> just like the transitory one does.
>>
>> I'm going to go ahead and do:
>>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>> Tested-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [Freedreno] [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-06-30  2:10         ` Abhinav Kumar
  0 siblings, 0 replies; 16+ messages in thread
From: Abhinav Kumar @ 2022-06-30  2:10 UTC (permalink / raw)
  To: Rob Clark, Doug Anderson
  Cc: Sean Paul, Sankeerth Billakanti, David Airlie, linux-arm-msm,
	LKML, dri-devel, Stephen Boyd, Vinod Koul, Andy Gross,
	Bjorn Andersson, Dmitry Baryshkov, Aravind Venkateswaran (QUIC),
	Kuogee Hsieh (QUIC),
	freedreno



On 6/29/2022 6:57 PM, Rob Clark wrote:
> On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> wrote:
>>
>> Hi,
>>
>> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
>> <dmitry.baryshkov@linaro.org> wrote:
>>>
>>> On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>>>> Some userspace presumes that the first connected connector is the main
>>>> display, where it's supposed to display e.g. the login screen. For
>>>> laptops, this should be the main panel.
>>>>
>>>> This patch call drm_helper_move_panel_connectors_to_head() after
>>>> drm_bridge_connector_init() to make sure eDP stay at head of
>>>> connected connector list. This fixes unexpected corruption happen
>>>> at eDP panel if eDP is not placed at head of connected connector
>>>> list.
>>>
>>> The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
>>> Also could you please describe the mind of corruption you are observing?
>>
>> I've spent a whole bunch of time poking at this and in the end my
>> conclusion is this:
>>
>> 1. The glitchyness seems to be a result of the Chrome OS userspace
>> somehow telling the kernel to do something wrong.
>>
>> 2. I believe (though I have no proof other than Kuogee's patch fixing
>> things) that the Chrome OS userspace is simply confused by the eDP
>> connector being second. This would imply that Kuogee's patch is
>> actually the right one.
>>
>> 3. It would be ideal if the Chrome OS userspace were fixed to handle
>> this, but it's an area of code that I've never looked at. It also
>> seems terribly low priority to fix since apparently other OSes have
>> similar problems (seems like this code was originally added by
>> RedHat?)
>>
>>
>> Specifically, I tested with a similar but "persistent" glitch that I
>> reproduced. The glitch Kuogee was digging into was a transitory glitch
>> on the eDP (internal) display when you plugged in a DP (external)
>> display. It would show up for a frame or two and then be fixed. I can
>> get a similar-looking glitch (vertical black and white bars) that
>> persists by doing these steps on a Chrome OS device (and Chrome OS
>> kernel):
>>
>> a) Observe screen looks good.
>> b) Observe DP not connected.
>> c) Plug in DP
>> d) See transitory glitch on screen, then it all looks fine.
>> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
>> f) Wait for screen to turn off
>> g) Unplug DP
>> h) Hit key on keyboard to wake device.
>> i) See glitchy.
>> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
>> --ac_screen_off_delay=10000
>>
>> Once I'm in the persistent glitch:
>>
>> * The "screenshot" command in Chrome OS shows corruption. Not exactly
>> black and white bars, but the image produced has distinct bands of
>> garbage.
>>
>> * I can actually toggle between VT2 and the main screen (VT1). Note
>> that VT1/VT2 are not quite the normal Linux managed solution--I
>> believe they're handled by frecon. In any case, when I switch to VT2
>> it looks normal (I can see the login prompt). Then back to VT1 and the
>> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
>> glitch again. This implies (especially with the extra evidence of
>> screenshot) that the display controller hardware is all fine and that
>> it's the underlying data that's somehow messed up.
> 
> fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
> simply just an un-renderered buffer being interpreted by the display
> controller as UBWC (because userspace tells it to)
> 
> BR,
> -R

Acked and agree with the comments both of you have stated and looking at 
the corrupted buffers in the snapshot.

Hence,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

> 
>> When I pick Kuogee's patch then this "persistent" glitch goes away
>> just like the transitory one does.
>>
>> I'm going to go ahead and do:
>>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>> Tested-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
  2022-06-30  1:57       ` Rob Clark
@ 2022-06-30  6:14         ` Dmitry Baryshkov
  -1 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2022-06-30  6:14 UTC (permalink / raw)
  To: Rob Clark, Doug Anderson
  Cc: Sean Paul, Stephen Boyd, Vinod Koul, Daniel Vetter, David Airlie,
	Andy Gross, Bjorn Andersson, Abhinav Kumar (QUIC),
	Aravind Venkateswaran (QUIC), Kuogee Hsieh (QUIC),
	Sankeerth Billakanti, freedreno, dri-devel, linux-arm-msm, LKML



On 30 June 2022 04:57:35 GMT+03:00, Rob Clark <robdclark@gmail.com> wrote:
>On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> wrote:
>>
>> Hi,
>>
>> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
>> <dmitry.baryshkov@linaro.org> wrote:
>> >
>> > On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>> > >Some userspace presumes that the first connected connector is the main
>> > >display, where it's supposed to display e.g. the login screen. For
>> > >laptops, this should be the main panel.
>> > >
>> > >This patch call drm_helper_move_panel_connectors_to_head() after
>> > >drm_bridge_connector_init() to make sure eDP stay at head of
>> > >connected connector list. This fixes unexpected corruption happen
>> > >at eDP panel if eDP is not placed at head of connected connector
>> > >list.
>> >
>> > The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
>> > Also could you please describe the mind of corruption you are observing?
>>
>> I've spent a whole bunch of time poking at this and in the end my
>> conclusion is this:
>>
>> 1. The glitchyness seems to be a result of the Chrome OS userspace
>> somehow telling the kernel to do something wrong.
>>
>> 2. I believe (though I have no proof other than Kuogee's patch fixing
>> things) that the Chrome OS userspace is simply confused by the eDP
>> connector being second. This would imply that Kuogee's patch is
>> actually the right one.
>>
>> 3. It would be ideal if the Chrome OS userspace were fixed to handle
>> this, but it's an area of code that I've never looked at. It also
>> seems terribly low priority to fix since apparently other OSes have
>> similar problems (seems like this code was originally added by
>> RedHat?)
>>
>>
>> Specifically, I tested with a similar but "persistent" glitch that I
>> reproduced. The glitch Kuogee was digging into was a transitory glitch
>> on the eDP (internal) display when you plugged in a DP (external)
>> display. It would show up for a frame or two and then be fixed. I can
>> get a similar-looking glitch (vertical black and white bars) that
>> persists by doing these steps on a Chrome OS device (and Chrome OS
>> kernel):
>>
>> a) Observe screen looks good.
>> b) Observe DP not connected.
>> c) Plug in DP
>> d) See transitory glitch on screen, then it all looks fine.
>> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
>> f) Wait for screen to turn off
>> g) Unplug DP
>> h) Hit key on keyboard to wake device.
>> i) See glitchy.
>> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
>> --ac_screen_off_delay=10000
>>
>> Once I'm in the persistent glitch:
>>
>> * The "screenshot" command in Chrome OS shows corruption. Not exactly
>> black and white bars, but the image produced has distinct bands of
>> garbage.
>>
>> * I can actually toggle between VT2 and the main screen (VT1). Note
>> that VT1/VT2 are not quite the normal Linux managed solution--I
>> believe they're handled by frecon. In any case, when I switch to VT2
>> it looks normal (I can see the login prompt). Then back to VT1 and the
>> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
>> glitch again. This implies (especially with the extra evidence of
>> screenshot) that the display controller hardware is all fine and that
>> it's the underlying data that's somehow messed up.
>
>fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
>simply just an un-renderered buffer being interpreted by the display
>controller as UBWC (because userspace tells it to)

Thanks for the description. I think the userspace code should be fixed too, but this patch can go in on its own.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


>
>BR,
>-R
>
>> When I pick Kuogee's patch then this "persistent" glitch goes away
>> just like the transitory one does.
>>
>> I'm going to go ahead and do:
>>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>> Tested-by: Douglas Anderson <dianders@chromium.org>

-- 
With best wishes
Dmitry

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-06-30  6:14         ` Dmitry Baryshkov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2022-06-30  6:14 UTC (permalink / raw)
  To: Rob Clark, Doug Anderson
  Cc: freedreno, Sankeerth Billakanti, David Airlie,
	Kuogee Hsieh (QUIC), Abhinav Kumar (QUIC),
	dri-devel, Stephen Boyd, Vinod Koul, Andy Gross, linux-arm-msm,
	Aravind Venkateswaran (QUIC),
	Bjorn Andersson, Sean Paul, LKML



On 30 June 2022 04:57:35 GMT+03:00, Rob Clark <robdclark@gmail.com> wrote:
>On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> wrote:
>>
>> Hi,
>>
>> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
>> <dmitry.baryshkov@linaro.org> wrote:
>> >
>> > On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>> > >Some userspace presumes that the first connected connector is the main
>> > >display, where it's supposed to display e.g. the login screen. For
>> > >laptops, this should be the main panel.
>> > >
>> > >This patch call drm_helper_move_panel_connectors_to_head() after
>> > >drm_bridge_connector_init() to make sure eDP stay at head of
>> > >connected connector list. This fixes unexpected corruption happen
>> > >at eDP panel if eDP is not placed at head of connected connector
>> > >list.
>> >
>> > The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
>> > Also could you please describe the mind of corruption you are observing?
>>
>> I've spent a whole bunch of time poking at this and in the end my
>> conclusion is this:
>>
>> 1. The glitchyness seems to be a result of the Chrome OS userspace
>> somehow telling the kernel to do something wrong.
>>
>> 2. I believe (though I have no proof other than Kuogee's patch fixing
>> things) that the Chrome OS userspace is simply confused by the eDP
>> connector being second. This would imply that Kuogee's patch is
>> actually the right one.
>>
>> 3. It would be ideal if the Chrome OS userspace were fixed to handle
>> this, but it's an area of code that I've never looked at. It also
>> seems terribly low priority to fix since apparently other OSes have
>> similar problems (seems like this code was originally added by
>> RedHat?)
>>
>>
>> Specifically, I tested with a similar but "persistent" glitch that I
>> reproduced. The glitch Kuogee was digging into was a transitory glitch
>> on the eDP (internal) display when you plugged in a DP (external)
>> display. It would show up for a frame or two and then be fixed. I can
>> get a similar-looking glitch (vertical black and white bars) that
>> persists by doing these steps on a Chrome OS device (and Chrome OS
>> kernel):
>>
>> a) Observe screen looks good.
>> b) Observe DP not connected.
>> c) Plug in DP
>> d) See transitory glitch on screen, then it all looks fine.
>> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
>> f) Wait for screen to turn off
>> g) Unplug DP
>> h) Hit key on keyboard to wake device.
>> i) See glitchy.
>> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
>> --ac_screen_off_delay=10000
>>
>> Once I'm in the persistent glitch:
>>
>> * The "screenshot" command in Chrome OS shows corruption. Not exactly
>> black and white bars, but the image produced has distinct bands of
>> garbage.
>>
>> * I can actually toggle between VT2 and the main screen (VT1). Note
>> that VT1/VT2 are not quite the normal Linux managed solution--I
>> believe they're handled by frecon. In any case, when I switch to VT2
>> it looks normal (I can see the login prompt). Then back to VT1 and the
>> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
>> glitch again. This implies (especially with the extra evidence of
>> screenshot) that the display controller hardware is all fine and that
>> it's the underlying data that's somehow messed up.
>
>fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
>simply just an un-renderered buffer being interpreted by the display
>controller as UBWC (because userspace tells it to)

Thanks for the description. I think the userspace code should be fixed too, but this patch can go in on its own.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


>
>BR,
>-R
>
>> When I pick Kuogee's patch then this "persistent" glitch goes away
>> just like the transitory one does.
>>
>> I'm going to go ahead and do:
>>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>> Tested-by: Douglas Anderson <dianders@chromium.org>

-- 
With best wishes
Dmitry

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
  2022-06-30  6:14         ` Dmitry Baryshkov
@ 2022-07-04 18:14           ` Dmitry Baryshkov
  -1 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2022-07-04 18:14 UTC (permalink / raw)
  To: Rob Clark, Doug Anderson, Kuogee Hsieh
  Cc: Sean Paul, Stephen Boyd, Vinod Koul, Daniel Vetter, David Airlie,
	Andy Gross, Bjorn Andersson, Abhinav Kumar (QUIC),
	Aravind Venkateswaran (QUIC), Kuogee Hsieh (QUIC),
	Sankeerth Billakanti, freedreno, dri-devel, linux-arm-msm, LKML

On 30/06/2022 09:14, Dmitry Baryshkov wrote:
> 
> 
> On 30 June 2022 04:57:35 GMT+03:00, Rob Clark <robdclark@gmail.com> wrote:
>> On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
>>> <dmitry.baryshkov@linaro.org> wrote:
>>>>
>>>> On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>>>>> Some userspace presumes that the first connected connector is the main
>>>>> display, where it's supposed to display e.g. the login screen. For
>>>>> laptops, this should be the main panel.
>>>>>
>>>>> This patch call drm_helper_move_panel_connectors_to_head() after
>>>>> drm_bridge_connector_init() to make sure eDP stay at head of
>>>>> connected connector list. This fixes unexpected corruption happen
>>>>> at eDP panel if eDP is not placed at head of connected connector
>>>>> list.
>>>>
>>>> The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
>>>> Also could you please describe the mind of corruption you are observing?
>>>
>>> I've spent a whole bunch of time poking at this and in the end my
>>> conclusion is this:
>>>
>>> 1. The glitchyness seems to be a result of the Chrome OS userspace
>>> somehow telling the kernel to do something wrong.
>>>
>>> 2. I believe (though I have no proof other than Kuogee's patch fixing
>>> things) that the Chrome OS userspace is simply confused by the eDP
>>> connector being second. This would imply that Kuogee's patch is
>>> actually the right one.
>>>
>>> 3. It would be ideal if the Chrome OS userspace were fixed to handle
>>> this, but it's an area of code that I've never looked at. It also
>>> seems terribly low priority to fix since apparently other OSes have
>>> similar problems (seems like this code was originally added by
>>> RedHat?)
>>>
>>>
>>> Specifically, I tested with a similar but "persistent" glitch that I
>>> reproduced. The glitch Kuogee was digging into was a transitory glitch
>>> on the eDP (internal) display when you plugged in a DP (external)
>>> display. It would show up for a frame or two and then be fixed. I can
>>> get a similar-looking glitch (vertical black and white bars) that
>>> persists by doing these steps on a Chrome OS device (and Chrome OS
>>> kernel):
>>>
>>> a) Observe screen looks good.
>>> b) Observe DP not connected.
>>> c) Plug in DP
>>> d) See transitory glitch on screen, then it all looks fine.
>>> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
>>> f) Wait for screen to turn off
>>> g) Unplug DP
>>> h) Hit key on keyboard to wake device.
>>> i) See glitchy.
>>> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
>>> --ac_screen_off_delay=10000
>>>
>>> Once I'm in the persistent glitch:
>>>
>>> * The "screenshot" command in Chrome OS shows corruption. Not exactly
>>> black and white bars, but the image produced has distinct bands of
>>> garbage.
>>>
>>> * I can actually toggle between VT2 and the main screen (VT1). Note
>>> that VT1/VT2 are not quite the normal Linux managed solution--I
>>> believe they're handled by frecon. In any case, when I switch to VT2
>>> it looks normal (I can see the login prompt). Then back to VT1 and the
>>> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
>>> glitch again. This implies (especially with the extra evidence of
>>> screenshot) that the display controller hardware is all fine and that
>>> it's the underlying data that's somehow messed up.
>>
>> fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
>> simply just an un-renderered buffer being interpreted by the display
>> controller as UBWC (because userspace tells it to)
> 
> Thanks for the description. I think the userspace code should be fixed too, but this patch can go in on its own.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

After some time (please excuse me), musing with the code and even 
picking up the commit for the merge branch, I understood the fact that I 
did not like about this change. It moves all panel connectors (generic 
code) from the DP-specific driver.

I'd like to retract my R-b. Please move this call to the msm_drm_init(). 
Calling this function somewhere after the ->kms_init() would make sure 
that all panel connectors are close to the top of the list, whichever 
MDP/DPU driver is used and whichever actual interface is bound to this 
panel.

> 
> 
>>
>> BR,
>> -R
>>
>>> When I pick Kuogee's patch then this "persistent" glitch goes away
>>> just like the transitory one does.
>>>
>>> I'm going to go ahead and do:
>>>
>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>> Tested-by: Douglas Anderson <dianders@chromium.org>
> 


-- 
With best wishes
Dmitry

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

* Re: [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-07-04 18:14           ` Dmitry Baryshkov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2022-07-04 18:14 UTC (permalink / raw)
  To: Rob Clark, Doug Anderson, Kuogee Hsieh
  Cc: freedreno, Sankeerth Billakanti, David Airlie,
	Kuogee Hsieh (QUIC), Abhinav Kumar (QUIC),
	dri-devel, Stephen Boyd, Vinod Koul, Andy Gross, linux-arm-msm,
	Aravind Venkateswaran (QUIC),
	Bjorn Andersson, Sean Paul, LKML

On 30/06/2022 09:14, Dmitry Baryshkov wrote:
> 
> 
> On 30 June 2022 04:57:35 GMT+03:00, Rob Clark <robdclark@gmail.com> wrote:
>> On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
>>> <dmitry.baryshkov@linaro.org> wrote:
>>>>
>>>> On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>>>>> Some userspace presumes that the first connected connector is the main
>>>>> display, where it's supposed to display e.g. the login screen. For
>>>>> laptops, this should be the main panel.
>>>>>
>>>>> This patch call drm_helper_move_panel_connectors_to_head() after
>>>>> drm_bridge_connector_init() to make sure eDP stay at head of
>>>>> connected connector list. This fixes unexpected corruption happen
>>>>> at eDP panel if eDP is not placed at head of connected connector
>>>>> list.
>>>>
>>>> The change itself is a good fix anyway. (And I'd ack it.) However I would like to understand why does it fix the corruption issue. What is we have eDP and DSI, with DSI ending up before the eDP? Would we see the issue?
>>>> Also could you please describe the mind of corruption you are observing?
>>>
>>> I've spent a whole bunch of time poking at this and in the end my
>>> conclusion is this:
>>>
>>> 1. The glitchyness seems to be a result of the Chrome OS userspace
>>> somehow telling the kernel to do something wrong.
>>>
>>> 2. I believe (though I have no proof other than Kuogee's patch fixing
>>> things) that the Chrome OS userspace is simply confused by the eDP
>>> connector being second. This would imply that Kuogee's patch is
>>> actually the right one.
>>>
>>> 3. It would be ideal if the Chrome OS userspace were fixed to handle
>>> this, but it's an area of code that I've never looked at. It also
>>> seems terribly low priority to fix since apparently other OSes have
>>> similar problems (seems like this code was originally added by
>>> RedHat?)
>>>
>>>
>>> Specifically, I tested with a similar but "persistent" glitch that I
>>> reproduced. The glitch Kuogee was digging into was a transitory glitch
>>> on the eDP (internal) display when you plugged in a DP (external)
>>> display. It would show up for a frame or two and then be fixed. I can
>>> get a similar-looking glitch (vertical black and white bars) that
>>> persists by doing these steps on a Chrome OS device (and Chrome OS
>>> kernel):
>>>
>>> a) Observe screen looks good.
>>> b) Observe DP not connected.
>>> c) Plug in DP
>>> d) See transitory glitch on screen, then it all looks fine.
>>> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
>>> f) Wait for screen to turn off
>>> g) Unplug DP
>>> h) Hit key on keyboard to wake device.
>>> i) See glitchy.
>>> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
>>> --ac_screen_off_delay=10000
>>>
>>> Once I'm in the persistent glitch:
>>>
>>> * The "screenshot" command in Chrome OS shows corruption. Not exactly
>>> black and white bars, but the image produced has distinct bands of
>>> garbage.
>>>
>>> * I can actually toggle between VT2 and the main screen (VT1). Note
>>> that VT1/VT2 are not quite the normal Linux managed solution--I
>>> believe they're handled by frecon. In any case, when I switch to VT2
>>> it looks normal (I can see the login prompt). Then back to VT1 and the
>>> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
>>> glitch again. This implies (especially with the extra evidence of
>>> screenshot) that the display controller hardware is all fine and that
>>> it's the underlying data that's somehow messed up.
>>
>> fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
>> simply just an un-renderered buffer being interpreted by the display
>> controller as UBWC (because userspace tells it to)
> 
> Thanks for the description. I think the userspace code should be fixed too, but this patch can go in on its own.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

After some time (please excuse me), musing with the code and even 
picking up the commit for the merge branch, I understood the fact that I 
did not like about this change. It moves all panel connectors (generic 
code) from the DP-specific driver.

I'd like to retract my R-b. Please move this call to the msm_drm_init(). 
Calling this function somewhere after the ->kms_init() would make sure 
that all panel connectors are close to the top of the list, whichever 
MDP/DPU driver is used and whichever actual interface is bound to this 
panel.

> 
> 
>>
>> BR,
>> -R
>>
>>> When I pick Kuogee's patch then this "persistent" glitch goes away
>>> just like the transitory one does.
>>>
>>> I'm going to go ahead and do:
>>>
>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>> Tested-by: Douglas Anderson <dianders@chromium.org>
> 


-- 
With best wishes
Dmitry

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

* Re: [Freedreno] [PATCH] drm/msm/dp: make eDP panel as the first connected connector
  2022-07-04 18:14           ` Dmitry Baryshkov
@ 2022-07-05  5:41             ` Abhinav Kumar
  -1 siblings, 0 replies; 16+ messages in thread
From: Abhinav Kumar @ 2022-07-05  5:41 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Doug Anderson, Kuogee Hsieh
  Cc: freedreno, Sankeerth Billakanti, David Airlie, dri-devel,
	Stephen Boyd, Vinod Koul, Andy Gross, Daniel Vetter,
	linux-arm-msm, Aravind Venkateswaran (QUIC),
	Bjorn Andersson, Sean Paul, LKML



On 7/4/2022 11:14 AM, Dmitry Baryshkov wrote:
> On 30/06/2022 09:14, Dmitry Baryshkov wrote:
>>
>>
>> On 30 June 2022 04:57:35 GMT+03:00, Rob Clark <robdclark@gmail.com> 
>> wrote:
>>> On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> 
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
>>>> <dmitry.baryshkov@linaro.org> wrote:
>>>>>
>>>>> On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh 
>>>>> <quic_khsieh@quicinc.com> wrote:
>>>>>> Some userspace presumes that the first connected connector is the 
>>>>>> main
>>>>>> display, where it's supposed to display e.g. the login screen. For
>>>>>> laptops, this should be the main panel.
>>>>>>
>>>>>> This patch call drm_helper_move_panel_connectors_to_head() after
>>>>>> drm_bridge_connector_init() to make sure eDP stay at head of
>>>>>> connected connector list. This fixes unexpected corruption happen
>>>>>> at eDP panel if eDP is not placed at head of connected connector
>>>>>> list.
>>>>>
>>>>> The change itself is a good fix anyway. (And I'd ack it.) However I 
>>>>> would like to understand why does it fix the corruption issue. What 
>>>>> is we have eDP and DSI, with DSI ending up before the eDP? Would we 
>>>>> see the issue?
>>>>> Also could you please describe the mind of corruption you are 
>>>>> observing?
>>>>
>>>> I've spent a whole bunch of time poking at this and in the end my
>>>> conclusion is this:
>>>>
>>>> 1. The glitchyness seems to be a result of the Chrome OS userspace
>>>> somehow telling the kernel to do something wrong.
>>>>
>>>> 2. I believe (though I have no proof other than Kuogee's patch fixing
>>>> things) that the Chrome OS userspace is simply confused by the eDP
>>>> connector being second. This would imply that Kuogee's patch is
>>>> actually the right one.
>>>>
>>>> 3. It would be ideal if the Chrome OS userspace were fixed to handle
>>>> this, but it's an area of code that I've never looked at. It also
>>>> seems terribly low priority to fix since apparently other OSes have
>>>> similar problems (seems like this code was originally added by
>>>> RedHat?)
>>>>
>>>>
>>>> Specifically, I tested with a similar but "persistent" glitch that I
>>>> reproduced. The glitch Kuogee was digging into was a transitory glitch
>>>> on the eDP (internal) display when you plugged in a DP (external)
>>>> display. It would show up for a frame or two and then be fixed. I can
>>>> get a similar-looking glitch (vertical black and white bars) that
>>>> persists by doing these steps on a Chrome OS device (and Chrome OS
>>>> kernel):
>>>>
>>>> a) Observe screen looks good.
>>>> b) Observe DP not connected.
>>>> c) Plug in DP
>>>> d) See transitory glitch on screen, then it all looks fine.
>>>> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
>>>> f) Wait for screen to turn off
>>>> g) Unplug DP
>>>> h) Hit key on keyboard to wake device.
>>>> i) See glitchy.
>>>> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
>>>> --ac_screen_off_delay=10000
>>>>
>>>> Once I'm in the persistent glitch:
>>>>
>>>> * The "screenshot" command in Chrome OS shows corruption. Not exactly
>>>> black and white bars, but the image produced has distinct bands of
>>>> garbage.
>>>>
>>>> * I can actually toggle between VT2 and the main screen (VT1). Note
>>>> that VT1/VT2 are not quite the normal Linux managed solution--I
>>>> believe they're handled by frecon. In any case, when I switch to VT2
>>>> it looks normal (I can see the login prompt). Then back to VT1 and the
>>>> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
>>>> glitch again. This implies (especially with the extra evidence of
>>>> screenshot) that the display controller hardware is all fine and that
>>>> it's the underlying data that's somehow messed up.
>>>
>>> fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
>>> simply just an un-renderered buffer being interpreted by the display
>>> controller as UBWC (because userspace tells it to)
>>
>> Thanks for the description. I think the userspace code should be fixed 
>> too, but this patch can go in on its own.
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> After some time (please excuse me), musing with the code and even 
> picking up the commit for the merge branch, I understood the fact that I 
> did not like about this change. It moves all panel connectors (generic 
> code) from the DP-specific driver.
> 
> I'd like to retract my R-b. Please move this call to the msm_drm_init(). 
> Calling this function somewhere after the ->kms_init() would make sure 
> that all panel connectors are close to the top of the list, whichever 
> MDP/DPU driver is used and whichever actual interface is bound to this 
> panel.
> 
Ah. True, but just to add. It should be after kms_init() but before 
drm_dev_register().

>>
>>
>>>
>>> BR,
>>> -R
>>>
>>>> When I pick Kuogee's patch then this "persistent" glitch goes away
>>>> just like the transitory one does.
>>>>
>>>> I'm going to go ahead and do:
>>>>
>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>> Tested-by: Douglas Anderson <dianders@chromium.org>
>>
> 
> 

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

* Re: [Freedreno] [PATCH] drm/msm/dp: make eDP panel as the first connected connector
@ 2022-07-05  5:41             ` Abhinav Kumar
  0 siblings, 0 replies; 16+ messages in thread
From: Abhinav Kumar @ 2022-07-05  5:41 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Doug Anderson, Kuogee Hsieh
  Cc: Sean Paul, Sankeerth Billakanti, David Airlie, linux-arm-msm,
	LKML, dri-devel, Stephen Boyd, Vinod Koul, Andy Gross,
	Aravind Venkateswaran (QUIC),
	Bjorn Andersson, freedreno



On 7/4/2022 11:14 AM, Dmitry Baryshkov wrote:
> On 30/06/2022 09:14, Dmitry Baryshkov wrote:
>>
>>
>> On 30 June 2022 04:57:35 GMT+03:00, Rob Clark <robdclark@gmail.com> 
>> wrote:
>>> On Wed, Jun 29, 2022 at 5:36 PM Doug Anderson <dianders@chromium.org> 
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Tue, Jun 28, 2022 at 1:14 PM Dmitry Baryshkov
>>>> <dmitry.baryshkov@linaro.org> wrote:
>>>>>
>>>>> On 28 June 2022 18:20:06 GMT+03:00, Kuogee Hsieh 
>>>>> <quic_khsieh@quicinc.com> wrote:
>>>>>> Some userspace presumes that the first connected connector is the 
>>>>>> main
>>>>>> display, where it's supposed to display e.g. the login screen. For
>>>>>> laptops, this should be the main panel.
>>>>>>
>>>>>> This patch call drm_helper_move_panel_connectors_to_head() after
>>>>>> drm_bridge_connector_init() to make sure eDP stay at head of
>>>>>> connected connector list. This fixes unexpected corruption happen
>>>>>> at eDP panel if eDP is not placed at head of connected connector
>>>>>> list.
>>>>>
>>>>> The change itself is a good fix anyway. (And I'd ack it.) However I 
>>>>> would like to understand why does it fix the corruption issue. What 
>>>>> is we have eDP and DSI, with DSI ending up before the eDP? Would we 
>>>>> see the issue?
>>>>> Also could you please describe the mind of corruption you are 
>>>>> observing?
>>>>
>>>> I've spent a whole bunch of time poking at this and in the end my
>>>> conclusion is this:
>>>>
>>>> 1. The glitchyness seems to be a result of the Chrome OS userspace
>>>> somehow telling the kernel to do something wrong.
>>>>
>>>> 2. I believe (though I have no proof other than Kuogee's patch fixing
>>>> things) that the Chrome OS userspace is simply confused by the eDP
>>>> connector being second. This would imply that Kuogee's patch is
>>>> actually the right one.
>>>>
>>>> 3. It would be ideal if the Chrome OS userspace were fixed to handle
>>>> this, but it's an area of code that I've never looked at. It also
>>>> seems terribly low priority to fix since apparently other OSes have
>>>> similar problems (seems like this code was originally added by
>>>> RedHat?)
>>>>
>>>>
>>>> Specifically, I tested with a similar but "persistent" glitch that I
>>>> reproduced. The glitch Kuogee was digging into was a transitory glitch
>>>> on the eDP (internal) display when you plugged in a DP (external)
>>>> display. It would show up for a frame or two and then be fixed. I can
>>>> get a similar-looking glitch (vertical black and white bars) that
>>>> persists by doing these steps on a Chrome OS device (and Chrome OS
>>>> kernel):
>>>>
>>>> a) Observe screen looks good.
>>>> b) Observe DP not connected.
>>>> c) Plug in DP
>>>> d) See transitory glitch on screen, then it all looks fine.
>>>> e) set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10
>>>> f) Wait for screen to turn off
>>>> g) Unplug DP
>>>> h) Hit key on keyboard to wake device.
>>>> i) See glitchy.
>>>> j) Within 5 seconds: set_power_policy --ac_screen_dim_delay=5000
>>>> --ac_screen_off_delay=10000
>>>>
>>>> Once I'm in the persistent glitch:
>>>>
>>>> * The "screenshot" command in Chrome OS shows corruption. Not exactly
>>>> black and white bars, but the image produced has distinct bands of
>>>> garbage.
>>>>
>>>> * I can actually toggle between VT2 and the main screen (VT1). Note
>>>> that VT1/VT2 are not quite the normal Linux managed solution--I
>>>> believe they're handled by frecon. In any case, when I switch to VT2
>>>> it looks normal (I can see the login prompt). Then back to VT1 and the
>>>> vertical bars glitch. Back to VT2 and it's normal. Back to VT1 and the
>>>> glitch again. This implies (especially with the extra evidence of
>>>> screenshot) that the display controller hardware is all fine and that
>>>> it's the underlying data that's somehow messed up.
>>>
>>> fwiw, from looking at this a bit w/ Doug, I think the "glitch" is
>>> simply just an un-renderered buffer being interpreted by the display
>>> controller as UBWC (because userspace tells it to)
>>
>> Thanks for the description. I think the userspace code should be fixed 
>> too, but this patch can go in on its own.
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> After some time (please excuse me), musing with the code and even 
> picking up the commit for the merge branch, I understood the fact that I 
> did not like about this change. It moves all panel connectors (generic 
> code) from the DP-specific driver.
> 
> I'd like to retract my R-b. Please move this call to the msm_drm_init(). 
> Calling this function somewhere after the ->kms_init() would make sure 
> that all panel connectors are close to the top of the list, whichever 
> MDP/DPU driver is used and whichever actual interface is bound to this 
> panel.
> 
Ah. True, but just to add. It should be after kms_init() but before 
drm_dev_register().

>>
>>
>>>
>>> BR,
>>> -R
>>>
>>>> When I pick Kuogee's patch then this "persistent" glitch goes away
>>>> just like the transitory one does.
>>>>
>>>> I'm going to go ahead and do:
>>>>
>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>> Tested-by: Douglas Anderson <dianders@chromium.org>
>>
> 
> 

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

end of thread, other threads:[~2022-07-05 11:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-28 15:20 [PATCH] drm/msm/dp: make eDP panel as the first connected connector Kuogee Hsieh
2022-06-28 15:20 ` Kuogee Hsieh
2022-06-28 20:13 ` Dmitry Baryshkov
2022-06-28 20:13   ` Dmitry Baryshkov
2022-06-30  0:36   ` Doug Anderson
2022-06-30  0:36     ` Doug Anderson
2022-06-30  1:57     ` Rob Clark
2022-06-30  1:57       ` Rob Clark
2022-06-30  2:10       ` [Freedreno] " Abhinav Kumar
2022-06-30  2:10         ` Abhinav Kumar
2022-06-30  6:14       ` Dmitry Baryshkov
2022-06-30  6:14         ` Dmitry Baryshkov
2022-07-04 18:14         ` Dmitry Baryshkov
2022-07-04 18:14           ` Dmitry Baryshkov
2022-07-05  5:41           ` [Freedreno] " Abhinav Kumar
2022-07-05  5:41             ` Abhinav Kumar

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.