[PATCH] drm/amd/display: dual cursors are seen if scaling is enabled

2021-03-29 Thread Louis Li 
[Why]
This issue is found when scaling is not equal to one from src to dest.
When issue happens, there are offsets in both axis x and y between
two cursors. Users cannot control APP under such a condition.

[How]
For dual cursors, cursor should be disabled if there is a visible pipe
on top of the current pipe at the current cursor position.
For offsets between two cursors, need translate cursor position from
stream space to plane space with scaling into consideration.

Tested-by: Louis Li 
Signed-off-by: Louis Li 
Change-Id: Ic19e4f3b9225736f037f5ade10b68e8afe5f9ab7
---
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 40 ++-
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index 83212ea40077..1ce5e58e3a9e 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -2999,6 +2999,10 @@ static bool dcn10_can_pipe_disable_cursor(struct 
pipe_ctx *pipe_ctx)
const struct rect *r1 = _ctx->plane_res.scl_data.recout, *r2;
int r1_r = r1->x + r1->width, r1_b = r1->y + r1->height, r2_r, r2_b;
 
+   struct dc_cursor_position pos_cpy = pipe_ctx->stream->cursor_position;
+   int cp_x = pos_cpy.x;
+   int cp_y = pos_cpy.y;
+
/**
 * Disable the cursor if there's another pipe above this with a
 * plane that contains this pipe's viewport to prevent double cursor
@@ -3013,7 +3017,8 @@ static bool dcn10_can_pipe_disable_cursor(struct pipe_ctx 
*pipe_ctx)
r2_r = r2->x + r2->width;
r2_b = r2->y + r2->height;
 
-   if (r1->x >= r2->x && r1->y >= r2->y && r1_r <= r2_r && r1_b <= 
r2_b)
+   if ((cp_x >= r1->x && cp_y >= r1->y && cp_x <= r1_r && cp_y <= 
r1_b)
+  && (cp_x >= r2->x && cp_y >= r2->y && cp_x <= r2_r && cp_y 
<= r2_b))
return true;
}
 
@@ -3034,15 +3039,30 @@ static void dcn10_set_cursor_position(struct pipe_ctx 
*pipe_ctx)
.rotation = pipe_ctx->plane_state->rotation,
.mirror = pipe_ctx->plane_state->horizontal_mirror
};
-   uint32_t x_plane = pipe_ctx->plane_state->dst_rect.x;
-   uint32_t y_plane = pipe_ctx->plane_state->dst_rect.y;
-   uint32_t x_offset = min(x_plane, pos_cpy.x);
-   uint32_t y_offset = min(y_plane, pos_cpy.y);
-
-   pos_cpy.x -= x_offset;
-   pos_cpy.y -= y_offset;
-   pos_cpy.x_hotspot += (x_plane - x_offset);
-   pos_cpy.y_hotspot += (y_plane - y_offset);
+
+   int x_plane = pipe_ctx->plane_state->dst_rect.x;
+   int y_plane = pipe_ctx->plane_state->dst_rect.y;
+   int x_pos = pos_cpy.x;
+   int y_pos = pos_cpy.y;
+
+   // translate cursor from stream space to plane space
+   x_pos = (x_pos - x_plane) * pipe_ctx->plane_state->src_rect.width /
+   pipe_ctx->plane_state->dst_rect.width;
+   y_pos = (y_pos - y_plane) * pipe_ctx->plane_state->src_rect.height /
+   pipe_ctx->plane_state->dst_rect.height;
+
+   if (x_pos < 0) {
+   pos_cpy.x_hotspot -= x_pos;
+   x_pos = 0;
+   }
+
+   if (y_pos < 0) {
+   pos_cpy.y_hotspot -= y_pos;
+   y_pos = 0;
+   }
+
+   pos_cpy.x = (uint32_t)x_pos;
+   pos_cpy.y = (uint32_t)y_pos;
 
if (pipe_ctx->plane_state->address.type
== PLN_ADDR_TYPE_VIDEO_PROGRESSIVE)
-- 
2.27.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3] drm/amd/display: Fix AppleDongle can't be detected

2019-12-23 Thread Louis Li 
On Tue, Dec 17, 2019 at 10:57:11AM -0500, Harry Wentland wrote:
> On 2019-12-11 2:33 a.m., Louis Li wrote:
> > [Why]
> > External monitor cannot be displayed consistently, if connecting
> > via this Apple dongle (A1621, USB Type-C to HDMI).
> > Experiments prove that the dongle needs 200ms at least to be ready
> > for communication, after it drives HPDsignal high, and DPCD cannot
> > be read correctly during the period, even reading it repeatedly.
> > In such a case, driver does not perform link training bcz of no DPCD.
> > 
> > [How]
> > When driver is run to the modified point, EDID is read correctly
> > and dpcd_sink_count of link is not zero. Therefore, link training
> > should be successfully performed. Which implies parameters should
> > be updated, e.g. lane count, link rate, etc. Checking parameters,
> > if values of those parameters are zero, link training is not
> > performed. So, do link-training to have detection completed.
> > 
> > With this patch applied, the problem cannot be reproduced.
> > Testing other dongles, results are PASS.
> > Patch(v3) is verified PASS by both AMD internal lab and customer.
> > 
> > 
> > Signed-off-by: Louis Li 
> > ---
> >  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index 7372dedd2f48..6188edc92d0f 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -725,7 +725,9 @@ bool dc_link_detect(struct dc_link *link, enum 
> > dc_detect_reason reason)
> >  
> > if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT &&
> > sink_caps.transaction_type == 
> > DDC_TRANSACTION_TYPE_I2C_OVER_AUX &&
> > -   reason != DETECT_REASON_HPDRX) {
> 
> Do we need to drop this line? This looks like it'll break the previous
> fix here.
> 
> It looks like Abdoulaye added this here to fix the 400.1.1 DP compliance
> test. If you can check with him that your solution is fine and make sure
> to test that you can get a consistent pass of 400.1.1 over 30 runs I'm
> okay to take the change.
> 
> Harry
> 

Thank Rickey helped to verify this patch. Confirmed that 400.1.1 failed
after applying this patch. Abandon this submission.

> > +   link->verified_link_cap.lane_count == 0 &&
> > +   link->verified_link_cap.link_rate == 0 &&
> > +   link->verified_link_cap.link_spread == 0) {
> > /*
> >  * TODO debug why Dell 2413 doesn't like
> >  *  two link trainings
> > 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3] drm/amd/display: Fix AppleDongle can't be detected

2019-12-18 Thread Louis Li 
On Tue, Dec 17, 2019 at 10:57:11AM -0500, Harry Wentland wrote:
> On 2019-12-11 2:33 a.m., Louis Li wrote:
> > [Why]
> > External monitor cannot be displayed consistently, if connecting
> > via this Apple dongle (A1621, USB Type-C to HDMI).
> > Experiments prove that the dongle needs 200ms at least to be ready
> > for communication, after it drives HPDsignal high, and DPCD cannot
> > be read correctly during the period, even reading it repeatedly.
> > In such a case, driver does not perform link training bcz of no DPCD.
> > 
> > [How]
> > When driver is run to the modified point, EDID is read correctly
> > and dpcd_sink_count of link is not zero. Therefore, link training
> > should be successfully performed. Which implies parameters should
> > be updated, e.g. lane count, link rate, etc. Checking parameters,
> > if values of those parameters are zero, link training is not
> > performed. So, do link-training to have detection completed.
> > 
> > With this patch applied, the problem cannot be reproduced.
> > Testing other dongles, results are PASS.
> > Patch(v3) is verified PASS by both AMD internal lab and customer.
> > 
> > 
> > Signed-off-by: Louis Li 
> > ---
> >  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index 7372dedd2f48..6188edc92d0f 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -725,7 +725,9 @@ bool dc_link_detect(struct dc_link *link, enum 
> > dc_detect_reason reason)
> >  
> > if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT &&
> > sink_caps.transaction_type == 
> > DDC_TRANSACTION_TYPE_I2C_OVER_AUX &&
> > -   reason != DETECT_REASON_HPDRX) {
> 
> Do we need to drop this line? This looks like it'll break the previous
> fix here.
> 
> It looks like Abdoulaye added this here to fix the 400.1.1 DP compliance
> test. If you can check with him that your solution is fine and make sure
> to test that you can get a consistent pass of 400.1.1 over 30 runs I'm
> okay to take the change.
> 
> Harry
> 

Yes, need drop this line for this fix. Good to know it may impact 400.1.1.
I will verify it with this patch. And update test result.

Louis

> > +   link->verified_link_cap.lane_count == 0 &&
> > +   link->verified_link_cap.link_rate == 0 &&
> > +   link->verified_link_cap.link_spread == 0) {
> > /*
> >  * TODO debug why Dell 2413 doesn't like
> >  *  two link trainings
> > 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v3] drm/amd/display: Fix AppleDongle can't be detected

2019-12-10 Thread Louis Li 
[Why]
External monitor cannot be displayed consistently, if connecting
via this Apple dongle (A1621, USB Type-C to HDMI).
Experiments prove that the dongle needs 200ms at least to be ready
for communication, after it drives HPDsignal high, and DPCD cannot
be read correctly during the period, even reading it repeatedly.
In such a case, driver does not perform link training bcz of no DPCD.

[How]
When driver is run to the modified point, EDID is read correctly
and dpcd_sink_count of link is not zero. Therefore, link training
should be successfully performed. Which implies parameters should
be updated, e.g. lane count, link rate, etc. Checking parameters,
if values of those parameters are zero, link training is not
performed. So, do link-training to have detection completed.

With this patch applied, the problem cannot be reproduced.
Testing other dongles, results are PASS.
Patch(v3) is verified PASS by both AMD internal lab and customer.


Signed-off-by: Louis Li 
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 7372dedd2f48..6188edc92d0f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -725,7 +725,9 @@ bool dc_link_detect(struct dc_link *link, enum 
dc_detect_reason reason)
 
if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT &&
sink_caps.transaction_type == 
DDC_TRANSACTION_TYPE_I2C_OVER_AUX &&
-   reason != DETECT_REASON_HPDRX) {
+   link->verified_link_cap.lane_count == 0 &&
+   link->verified_link_cap.link_rate == 0 &&
+   link->verified_link_cap.link_spread == 0) {
/*
 * TODO debug why Dell 2413 doesn't like
 *  two link trainings
-- 
2.21.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected

2019-11-26 Thread Louis Li 
On Mon, Nov 25, 2019 at 09:24:56AM -0500, Harry Wentland wrote:
> 
> 
> On 2019-11-25 4:49 a.m., Louis Li wrote:
> > On Fri, Nov 22, 2019 at 10:31:19AM -0500, Harry Wentland wrote:
> >>
> >>
> >> On 2019-11-22 1:33 a.m., Louis Li wrote:
> >>> On Thu, Nov 21, 2019 at 08:47:50AM -0500, Kazlauskas, Nicholas wrote:
> >>>> On 2019-11-21 8:40 a.m., Kazlauskas, Nicholas wrote:
> >>>>> On 2019-11-21 3:31 a.m., Li, Ching-shih (Louis) wrote:
> >>>>>> Hi reviewers,
> >>>>>>
> >>>>>> What is the review result for this patch? Customer is pushing on this
> >>>>>> change to merge. TKS for your attention.
> >>>>>>
> >>>>>> BR,
> >>>>>> Louis
> >>>>>>
> >>>>>> -Original Message-
> >>>>>> From: Louis Li 
> >>>>>> Sent: Thursday, November 14, 2019 11:42 AM
> >>>>>> To: amd-gfx@lists.freedesktop.org
> >>>>>> Cc: Kazlauskas, Nicholas ; Wentland, Harry
> >>>>>> ; Li, Ching-shih (Louis) 
> >>>>>> 
> >>>>>> Subject: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be
> >>>>>> successfully detected
> >>>>>>
> >>>>>> [Why]
> >>>>>> External monitor cannot be displayed consistently, if connecting via
> >>>>>> this Apple dongle (A1621, USB Type-C to HDMI).
> >>>>>> By experiments, it is confirmed that the dongle needs 200ms at least to
> >>>>>> be ready for communication, after it sets HPD signal high.
> >>>>>>
> >>>>>> [How]
> >>>>>> When receiving HPD IRQ, delay 300ms at the beginning of 
> >>>>>> handle_hpd_irq().
> >>>>>> Then run the original procedure.
> >>>>>> With this patch applied, the problem cannot be reproduced.
> >>>>>> With other dongles, test results are PASS.
> >>>>>> Test result is PASS to plug in HDMI cable while playing video, and the
> >>>>>> video is being playing smoothly.
> >>>>>> Test result is PASS after system resumes from suspend.
> >>>>>>
> >>>>>> Signed-off-by: Louis Li 
> >>>>>
> >>>>> This is still a NAK from me since the logic hasn't changed from the 
> >>>>> first
> >>>>> patch.
> >>>>>
> >>>>> Sleeps don't belong in IRQ handlers.
> >>>>>
> >>>>> Regards,
> >>>>> Nicholas Kazlauskas
> >>>>
> >>>> Actually, this lives in the low IRQ context which means that it's already
> >>>> been queued off to a work thread so it's safe to sleep.
> >>>>
> >>>> I'm not sure we want a general 300ms sleep (even by experiment you said 
> >>>> that
> >>>> it only needed 200ms) for all connectors.
> >>>>
> >>>> Nicholas Kazlauskas
> >>>>
> >>>
> >>> Yes, it is IRQ context. Safe to call sleep(). Moreover, in current driver,
> >>> even udelay() is called in wait_for_training_aux_rd_interval() in the flow
> >>> of handle_hpd_irq().
> >>>
> >>> For 2nd question, of course not all connectors have this behavior.
> >>> Based on real cases we ever dealt, some dongles like this, or some
> >>> monitors driven by TCON, have same behavior. And no chance to read
> >>> out anything to decide if delay is needed. This change does help
> >>> to have our driver gain better compatibility. Truly this should be
> >>> problem of dongles/monitors. We are not the only one to
> >>> workaround such a problem. This change does not hurt other connects,
> >>> and some other dongles are tested ok, e.g. HP/Huwai dongles, etc.
> >>>
> >>
> >> I still don't like this change. It might impact other use cases, such as
> >> SST-to-MST switching on MST displays.
> >>
> >> Have you checked how Windows deals with this dongle and how the Windows
> >> team solved this? Have you checked how other drivers (such as i915) deal
> >> with this dongle?
> >>
> >> Have you checked whether you can pass DP compliance with this change?
> >>
> >> Harry
> >>
> > 
&

Re: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected

2019-11-25 Thread Louis Li 
On Fri, Nov 22, 2019 at 10:31:19AM -0500, Harry Wentland wrote:
> 
> 
> On 2019-11-22 1:33 a.m., Louis Li wrote:
> > On Thu, Nov 21, 2019 at 08:47:50AM -0500, Kazlauskas, Nicholas wrote:
> >> On 2019-11-21 8:40 a.m., Kazlauskas, Nicholas wrote:
> >>> On 2019-11-21 3:31 a.m., Li, Ching-shih (Louis) wrote:
> >>>> Hi reviewers,
> >>>>
> >>>> What is the review result for this patch? Customer is pushing on this
> >>>> change to merge. TKS for your attention.
> >>>>
> >>>> BR,
> >>>> Louis
> >>>>
> >>>> -Original Message-
> >>>> From: Louis Li 
> >>>> Sent: Thursday, November 14, 2019 11:42 AM
> >>>> To: amd-gfx@lists.freedesktop.org
> >>>> Cc: Kazlauskas, Nicholas ; Wentland, Harry
> >>>> ; Li, Ching-shih (Louis) 
> >>>> Subject: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be
> >>>> successfully detected
> >>>>
> >>>> [Why]
> >>>> External monitor cannot be displayed consistently, if connecting via
> >>>> this Apple dongle (A1621, USB Type-C to HDMI).
> >>>> By experiments, it is confirmed that the dongle needs 200ms at least to
> >>>> be ready for communication, after it sets HPD signal high.
> >>>>
> >>>> [How]
> >>>> When receiving HPD IRQ, delay 300ms at the beginning of handle_hpd_irq().
> >>>> Then run the original procedure.
> >>>> With this patch applied, the problem cannot be reproduced.
> >>>> With other dongles, test results are PASS.
> >>>> Test result is PASS to plug in HDMI cable while playing video, and the
> >>>> video is being playing smoothly.
> >>>> Test result is PASS after system resumes from suspend.
> >>>>
> >>>> Signed-off-by: Louis Li 
> >>>
> >>> This is still a NAK from me since the logic hasn't changed from the first
> >>> patch.
> >>>
> >>> Sleeps don't belong in IRQ handlers.
> >>>
> >>> Regards,
> >>> Nicholas Kazlauskas
> >>
> >> Actually, this lives in the low IRQ context which means that it's already
> >> been queued off to a work thread so it's safe to sleep.
> >>
> >> I'm not sure we want a general 300ms sleep (even by experiment you said 
> >> that
> >> it only needed 200ms) for all connectors.
> >>
> >> Nicholas Kazlauskas
> >>
> > 
> > Yes, it is IRQ context. Safe to call sleep(). Moreover, in current driver,
> > even udelay() is called in wait_for_training_aux_rd_interval() in the flow
> > of handle_hpd_irq().
> > 
> > For 2nd question, of course not all connectors have this behavior.
> > Based on real cases we ever dealt, some dongles like this, or some
> > monitors driven by TCON, have same behavior. And no chance to read
> > out anything to decide if delay is needed. This change does help
> > to have our driver gain better compatibility. Truly this should be
> > problem of dongles/monitors. We are not the only one to
> > workaround such a problem. This change does not hurt other connects,
> > and some other dongles are tested ok, e.g. HP/Huwai dongles, etc.
> > 
> 
> I still don't like this change. It might impact other use cases, such as
> SST-to-MST switching on MST displays.
> 
> Have you checked how Windows deals with this dongle and how the Windows
> team solved this? Have you checked how other drivers (such as i915) deal
> with this dongle?
> 
> Have you checked whether you can pass DP compliance with this change?
> 
> Harry
> 

Good points. MST and DP compliance are not verified yet.

For Windows cases, same solution was implemented. As I know, it goes to
point release (PR) instead of main line. Company N. has similar solution
to workaround such a problem. For i915, the solution seems to be different.

Will this change be accepted if it can pass MST and compilance?

Louis

> >>>
> >>>> ---
> >>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
> >>>>   1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>>> index 0aef92b7c037..5b844b6a5a59 100644
> >>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>>> +++ b/drivers/gpu/drm/amd/displ

Re: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected

2019-11-21 Thread Louis Li 
On Thu, Nov 21, 2019 at 08:47:50AM -0500, Kazlauskas, Nicholas wrote:
> On 2019-11-21 8:40 a.m., Kazlauskas, Nicholas wrote:
> >On 2019-11-21 3:31 a.m., Li, Ching-shih (Louis) wrote:
> >>Hi reviewers,
> >>
> >>What is the review result for this patch? Customer is pushing on this
> >>change to merge. TKS for your attention.
> >>
> >>BR,
> >>Louis
> >>
> >>-Original Message-
> >>From: Louis Li 
> >>Sent: Thursday, November 14, 2019 11:42 AM
> >>To: amd-gfx@lists.freedesktop.org
> >>Cc: Kazlauskas, Nicholas ; Wentland, Harry
> >>; Li, Ching-shih (Louis) 
> >>Subject: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be
> >>successfully detected
> >>
> >>[Why]
> >>External monitor cannot be displayed consistently, if connecting via
> >>this Apple dongle (A1621, USB Type-C to HDMI).
> >>By experiments, it is confirmed that the dongle needs 200ms at least to
> >>be ready for communication, after it sets HPD signal high.
> >>
> >>[How]
> >>When receiving HPD IRQ, delay 300ms at the beginning of handle_hpd_irq().
> >>Then run the original procedure.
> >>With this patch applied, the problem cannot be reproduced.
> >>With other dongles, test results are PASS.
> >>Test result is PASS to plug in HDMI cable while playing video, and the
> >>video is being playing smoothly.
> >>Test result is PASS after system resumes from suspend.
> >>
> >>Signed-off-by: Louis Li 
> >
> >This is still a NAK from me since the logic hasn't changed from the first
> >patch.
> >
> >Sleeps don't belong in IRQ handlers.
> >
> >Regards,
> >Nicholas Kazlauskas
> 
> Actually, this lives in the low IRQ context which means that it's already
> been queued off to a work thread so it's safe to sleep.
> 
> I'm not sure we want a general 300ms sleep (even by experiment you said that
> it only needed 200ms) for all connectors.
> 
> Nicholas Kazlauskas
> 

Yes, it is IRQ context. Safe to call sleep(). Moreover, in current driver,
even udelay() is called in wait_for_training_aux_rd_interval() in the flow
of handle_hpd_irq().

For 2nd question, of course not all connectors have this behavior.
Based on real cases we ever dealt, some dongles like this, or some
monitors driven by TCON, have same behavior. And no chance to read
out anything to decide if delay is needed. This change does help
to have our driver gain better compatibility. Truly this should be
problem of dongles/monitors. We are not the only one to
workaround such a problem. This change does not hurt other connects,
and some other dongles are tested ok, e.g. HP/Huwai dongles, etc.

> >
> >>---
> >>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
> >>  1 file changed, 5 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>index 0aef92b7c037..5b844b6a5a59 100644
> >>--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>@@ -1025,6 +1025,11 @@ static void handle_hpd_irq(void *param)
> >>  struct drm_device *dev = connector->dev;
> >>  enum dc_connection_type new_connection_type = dc_connection_none;
> >>+    /* Some monitors/dongles need around 200ms to be ready for
> >>communication
> >>+ * after those devices drive HPD signal high.
> >>+ */
> >>+    msleep(300);
> >>+
> >>  /* In case of failure or MST no need to update connector status or
> >>notify the OS
> >>   * since (for MST case) MST does this in it's own context.
> >>   */
> >>-- 
> >>2.21.0
> >>
> >
> >___
> >amd-gfx mailing list
> >amd-gfx@lists.freedesktop.org
> >https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected

2019-11-13 Thread Louis Li 
[Why]
External monitor cannot be displayed consistently, if connecting
via this Apple dongle (A1621, USB Type-C to HDMI).
By experiments, it is confirmed that the dongle needs 200ms at least
to be ready for communication, after it sets HPD signal high.

[How]
When receiving HPD IRQ, delay 300ms at the beginning of handle_hpd_irq().
Then run the original procedure.
With this patch applied, the problem cannot be reproduced.
With other dongles, test results are PASS.
Test result is PASS to plug in HDMI cable while playing video,
and the video is being playing smoothly.
Test result is PASS after system resumes from suspend.

Signed-off-by: Louis Li 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0aef92b7c037..5b844b6a5a59 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1025,6 +1025,11 @@ static void handle_hpd_irq(void *param)
struct drm_device *dev = connector->dev;
enum dc_connection_type new_connection_type = dc_connection_none;
 
+   /* Some monitors/dongles need around 200ms to be ready for communication
+* after those devices drive HPD signal high.
+*/
+   msleep(300);
+
/* In case of failure or MST no need to update connector status or 
notify the OS
 * since (for MST case) MST does this in it's own context.
 */
-- 
2.21.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] UPSTREAM: drm/amd/display: Fix Apple dongle cannot be successfully detected

2019-10-28 Thread Louis Li 
On Wed, Oct 23, 2019 at 02:19:56PM +0800, S, Shirish wrote:
> The UPSTREAM tag in the commit message needs to be removed.
> 

OK. Will remove it.

> On 10/21/2019 1:24 PM, Louis Li wrote:
> > [Why]
> > External monitor cannot be displayed consistently, if connecting
> > via this Apple dongle (A1621, USB Type-C to HDMI).
> > By experiments, it is confirmed that the dongle needs 200ms at least
> > to be ready for communication, after it sets HPD signal high.
> >
> > [How]
> > When receiving HPD IRQ, delay 500ms at the beginning of handle_hpd_irq().
> 
> Am not sure how this delay shall impact on dongles that don't need it,
> 
> ideally it should be added as quirk, at least restrict it to these 
> specific vendors.
> 
> Instead of delay, can you find any parameter to wait for for the 
> communication to be ready,
> 
> in that way it shall be failsafe.
> 

For such devices (monitors/dongles), there will be no chance to
get information before the defer, because it is not ready to
commmunicate right after HPD interrup. All other available
dongles were verified with the patch applied, and work well
still. As replied earlier, this is caused by dongle/monitor,
and the solution is to have the driver better compatibility.

Louis Li

> > Then run the original procedure.
> > With this patch applied, the problem cannot be reproduced.
> > With other dongles, test results are PASS.
> > Test result is PASS after system resumes from suspend.
> >
> > Signed-off-by: Louis Li 
> > ---
> >   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 0aef92b7c037..043ddac73862 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -1025,6 +1025,11 @@ static void handle_hpd_irq(void *param)
> > struct drm_device *dev = connector->dev;
> > enum dc_connection_type new_connection_type = dc_connection_none;
> >   
> > +/* Some monitors/dongles need around 200ms to be ready for 
> > communication
> > + * after they drive HPD signal high.
> > + */
> > +mdelay(500);
> > +
> > /* In case of failure or MST no need to update connector status or 
> > notify the OS
> >  * since (for MST case) MST does this in it's own context.
> >  */
> 
> -- 
> Regards,
> Shirish S
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] UPSTREAM: drm/amd/display: Fix Apple dongle cannot be successfully detected

2019-10-28 Thread Louis Li 
On Mon, Oct 21, 2019 at 08:45:18PM +0800, Kazlauskas, Nicholas wrote:
> On 2019-10-21 3:54 a.m., Louis Li wrote:
> > [Why]
> > External monitor cannot be displayed consistently, if connecting
> > via this Apple dongle (A1621, USB Type-C to HDMI).
> > By experiments, it is confirmed that the dongle needs 200ms at least
> > to be ready for communication, after it sets HPD signal high.
> > 
> > [How]
> > When receiving HPD IRQ, delay 500ms at the beginning of handle_hpd_irq().
> > Then run the original procedure.
> > With this patch applied, the problem cannot be reproduced.
> > With other dongles, test results are PASS.
> > Test result is PASS after system resumes from suspend.
> > 
> > Signed-off-by: Louis Li 
> > ---
> >   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 0aef92b7c037..043ddac73862 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -1025,6 +1025,11 @@ static void handle_hpd_irq(void *param)
> > struct drm_device *dev = connector->dev;
> > enum dc_connection_type new_connection_type = dc_connection_none;
> >   
> > +/* Some monitors/dongles need around 200ms to be ready for 
> > communication
> > + * after they drive HPD signal high.
> > + */
> > +mdelay(500);
> > +
> This sounds more like a quirk than behavior we want for all monitors, 
> but regardless this sleep isn't the correct place for this - since this 
> is an high priority interrupt handler this is really just blocking 
> everything for half a second.
> 
> I think it'd make more sense to queue off the work to occur half a 
> second later.
> 
> Nicholas Kazlauskas
> 

Yes, I agree with your comments. However, dealing with monitors and
dongles, it is proved that some monitors/dongles don't work as the
way people expected. The solution makes sense to have our driver
better compatibility to work with such devices. Truly, should not
block high priority interrupt. I had V2 patch to use msleep
instead. Customer is verifying V2. Will submit once it pass tests.

Louis Li

> > /* In case of failure or MST no need to update connector status or 
> > notify the OS
> >  * since (for MST case) MST does this in it's own context.
> >  */
> > 
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] UPSTREAM: drm/amd/display: Fix Apple dongle cannot be successfully detected

2019-10-21 Thread Louis Li 
[Why]
External monitor cannot be displayed consistently, if connecting
via this Apple dongle (A1621, USB Type-C to HDMI).
By experiments, it is confirmed that the dongle needs 200ms at least
to be ready for communication, after it sets HPD signal high.

[How]
When receiving HPD IRQ, delay 500ms at the beginning of handle_hpd_irq().
Then run the original procedure.
With this patch applied, the problem cannot be reproduced.
With other dongles, test results are PASS.
Test result is PASS after system resumes from suspend.

Signed-off-by: Louis Li 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0aef92b7c037..043ddac73862 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1025,6 +1025,11 @@ static void handle_hpd_irq(void *param)
struct drm_device *dev = connector->dev;
enum dc_connection_type new_connection_type = dc_connection_none;
 
+/* Some monitors/dongles need around 200ms to be ready for communication
+ * after they drive HPD signal high.
+ */
+mdelay(500);
+
/* In case of failure or MST no need to update connector status or 
notify the OS
 * since (for MST case) MST does this in it's own context.
 */
-- 
2.21.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx