[PATCH 0/2] drm/i915/dsi: Fix error on DSI video mode command

[PATCH 0/2] drm/i915/dsi: Fix error on DSI video mode command

[Mika Kahola]

In CI system, the Broxton with DSI display throws an error message with

*ERROR* Video mode command 0x00000041 send failed.

In order to get rid of the error message, we should send SHUTDOWN before
MIPI_SEQ_DISPLAY_OFF for VBT's higher than v3.

If we end up sending two or more MIPI commands that are the same, an error
message is thrown. The patch suggests to replace this error message with a
debug message as it is not that severe failure. With this change the CI
system is not flip flopping due to error message in dmesg log.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404

Mika Kahola (2):
  drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's
  drm/i915/dsi: Replace MIPI command error message with debug message

 drivers/gpu/drm/i915/intel_dsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Mika Kahola]

According to spec we should send SHUTDOWN before MIPI_SEQ_DISPLAY_OFF for
v3+ VBT's. Testing with VBT v3 the current implementation yields the
following error message

*ERROR* Video mode command 0x00000041 send failed.

To get rid of this error message, let's limit SHUTDOWN only for VBT
versions 3 or higher.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 2a0f5d3..b48b9b7 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -916,7 +916,7 @@ static void intel_dsi_disable(struct intel_encoder *encoder,
 	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
 	 * has shown that the v3 sequence works for v2 VBTs too
 	 */
-	if (is_vid_mode(intel_dsi)) {
+	if (is_vid_mode(intel_dsi) && dev_priv->vbt.dsi.seq_version > 3) {
 		/* Send Shutdown command to the panel in LP mode */
 		for_each_dsi_port(port, intel_dsi->ports)
 			dpi_send_cmd(intel_dsi, SHUTDOWN, false, port);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/dsi: Replace MIPI command error message with debug message

[Mika Kahola]

Error message indicating that the same MIPI command is sent consecutively
is perhaps too strongly said. Let's replace that as a debug message instead.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index b48b9b7..e16055d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -263,7 +263,7 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
 
 	/* XXX: old code skips write if control unchanged */
 	if (cmd == I915_READ(MIPI_DPI_CONTROL(port)))
-		DRM_ERROR("Same special packet %02x twice in a row.\n", cmd);
+		DRM_DEBUG_KMS("Same special packet %02x twice in a row.\n", cmd);
 
 	I915_WRITE(MIPI_DPI_CONTROL(port), cmd);
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dsi: Fix error on DSI video mode command

[Patchwork]

== Series Details ==

Series: drm/i915/dsi: Fix error on DSI video mode command
URL   : https://patchwork.freedesktop.org/series/29658/
State : success

== Summary ==

Series 29658v1 drm/i915/dsi: Fix error on DSI video mode command
https://patchwork.freedesktop.org/api/1.0/series/29658/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_cursor_legacy:
        Subgroup basic-flip-after-cursor-varying-size:
                fail       -> PASS       (fi-hsw-4770) fdo#102402 +1

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#102402 https://bugs.freedesktop.org/show_bug.cgi?id=102402

fi-bdw-5557u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:458s
fi-bdw-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:439s
fi-blb-e6850     total:288  pass:224  dwarn:1   dfail:0   fail:0   skip:63  time:359s
fi-bsw-n3050     total:288  pass:243  dwarn:0   dfail:0   fail:0   skip:45  time:567s
fi-bwr-2160      total:288  pass:184  dwarn:0   dfail:0   fail:0   skip:104 time:254s
fi-bxt-j4205     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:523s
fi-byt-j1900     total:288  pass:254  dwarn:1   dfail:0   fail:0   skip:33  time:521s
fi-byt-n2820     total:288  pass:250  dwarn:1   dfail:0   fail:0   skip:37  time:516s
fi-elk-e7500     total:288  pass:230  dwarn:0   dfail:0   fail:0   skip:58  time:431s
fi-glk-2a        total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:611s
fi-hsw-4770      total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:444s
fi-hsw-4770r     total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:426s
fi-ilk-650       total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:422s
fi-ivb-3520m     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:500s
fi-ivb-3770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:476s
fi-kbl-7500u     total:288  pass:264  dwarn:1   dfail:0   fail:0   skip:23  time:520s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:594s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:598s
fi-pnv-d510      total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:526s
fi-skl-6260u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:469s
fi-skl-6700k     total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:533s
fi-skl-6770hq    total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:488s
fi-skl-gvtdvm    total:288  pass:266  dwarn:0   dfail:0   fail:0   skip:22  time:448s
fi-skl-x1585l    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:490s
fi-snb-2520m     total:288  pass:251  dwarn:0   dfail:0   fail:0   skip:37  time:549s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:2   skip:38  time:413s

ccf4ca2d93383fe1a234aba83df9c21400216433 drm-tip: 2017y-08m-31d-18h-37m-46s UTC integration manifest
e48cf2f7195f drm/i915/dsi: Replace MIPI command error message with debug message
3119370c4963 drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5558/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915/dsi: Fix error on DSI video mode command

[Patchwork]

== Series Details ==

Series: drm/i915/dsi: Fix error on DSI video mode command
URL   : https://patchwork.freedesktop.org/series/29658/
State : warning

== Summary ==

Test kms_universal_plane:
        Subgroup disable-primary-vs-flip-pipe-C:
                pass       -> SKIP       (shard-hsw)

shard-hsw        total:2265 pass:1231 dwarn:0   dfail:0   fail:17  skip:1017 time:9630s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5558/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Ville Syrjälä]

On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
> According to spec we should send SHUTDOWN before MIPI_SEQ_DISPLAY_OFF for
> v3+ VBT's. Testing with VBT v3 the current implementation yields the
> following error message
> 
> *ERROR* Video mode command 0x00000041 send failed.
> 
> To get rid of this error message, let's limit SHUTDOWN only for VBT
> versions 3 or higher.

In the patch you limit it to version 4+, which doesn't make sense since
AFAIK there is no version 4 of the sequence block.

> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 2a0f5d3..b48b9b7 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct intel_encoder *encoder,
>  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
>  	 * has shown that the v3 sequence works for v2 VBTs too
>  	 */
> -	if (is_vid_mode(intel_dsi)) {
> +	if (is_vid_mode(intel_dsi) && dev_priv->vbt.dsi.seq_version > 3) {
>  		/* Send Shutdown command to the panel in LP mode */
>  		for_each_dsi_port(port, intel_dsi->ports)
>  			dpi_send_cmd(intel_dsi, SHUTDOWN, false, port);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Mika Kahola]

On Fri, 2017-09-01 at 16:43 +0300, Ville Syrjälä wrote:
> On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
> > 
> > According to spec we should send SHUTDOWN before
> > MIPI_SEQ_DISPLAY_OFF for
> > v3+ VBT's. Testing with VBT v3 the current implementation yields
> > the
> > following error message
> > 
> > *ERROR* Video mode command 0x00000041 send failed.
> > 
> > To get rid of this error message, let's limit SHUTDOWN only for VBT
> > versions 3 or higher.
> In the patch you limit it to version 4+, which doesn't make sense
> since
> AFAIK there is no version 4 of the sequence block.
It seems that sending SHUTDOWN signal doesn't make any sense either.
Whenever we send that signal it just causes this error message. Do we
really need to signal this? From functionality point of view there's no
difference.

> 
> > 
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index 2a0f5d3..b48b9b7 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct
> > intel_encoder *encoder,
> >  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field
> > testing
> >  	 * has shown that the v3 sequence works for v2 VBTs too
> >  	 */
> > -	if (is_vid_mode(intel_dsi)) {
> > +	if (is_vid_mode(intel_dsi) && dev_priv-
> > >vbt.dsi.seq_version > 3) {
> >  		/* Send Shutdown command to the panel in LP mode
> > */
> >  		for_each_dsi_port(port, intel_dsi->ports)
> >  			dpi_send_cmd(intel_dsi, SHUTDOWN, false,
> > port);
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Ville Syrjälä]

On Mon, Sep 04, 2017 at 10:59:32AM +0300, Mika Kahola wrote:
> On Fri, 2017-09-01 at 16:43 +0300, Ville Syrjälä wrote:
> > On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
> > > 
> > > According to spec we should send SHUTDOWN before
> > > MIPI_SEQ_DISPLAY_OFF for
> > > v3+ VBT's. Testing with VBT v3 the current implementation yields
> > > the
> > > following error message
> > > 
> > > *ERROR* Video mode command 0x00000041 send failed.
> > > 
> > > To get rid of this error message, let's limit SHUTDOWN only for VBT
> > > versions 3 or higher.
> > In the patch you limit it to version 4+, which doesn't make sense
> > since
> > AFAIK there is no version 4 of the sequence block.
> It seems that sending SHUTDOWN signal doesn't make any sense either.
> Whenever we send that signal it just causes this error message. Do we
> really need to signal this? From functionality point of view there's no
> difference.

Well, the spec doesn't even explain what this "shut down" command does.
Is it actually the DCS "display off" command, or something else?

Did you try reverting bbdf0b2ff32a ("drm/i915/bxt: Disable device ready
before shutdown command")? That looks suspicious to me. But so does
about half of the DSI code since it never seems to follow the spec, and
we end up doing totally different things on different platforms without
any explanation why that is).

> 
> > 
> > > 
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > > b/drivers/gpu/drm/i915/intel_dsi.c
> > > index 2a0f5d3..b48b9b7 100644
> > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct
> > > intel_encoder *encoder,
> > >  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field
> > > testing
> > >  	 * has shown that the v3 sequence works for v2 VBTs too
> > >  	 */
> > > -	if (is_vid_mode(intel_dsi)) {
> > > +	if (is_vid_mode(intel_dsi) && dev_priv-
> > > >vbt.dsi.seq_version > 3) {
> > >  		/* Send Shutdown command to the panel in LP mode
> > > */
> > >  		for_each_dsi_port(port, intel_dsi->ports)
> > >  			dpi_send_cmd(intel_dsi, SHUTDOWN, false,
> > > port);
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -- 
> Mika Kahola - Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Mika Kahola]

On Mon, 2017-09-04 at 18:04 +0300, Ville Syrjälä wrote:
> On Mon, Sep 04, 2017 at 10:59:32AM +0300, Mika Kahola wrote:
> > 
> > On Fri, 2017-09-01 at 16:43 +0300, Ville Syrjälä wrote:
> > > 
> > > On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
> > > > 
> > > > 
> > > > According to spec we should send SHUTDOWN before
> > > > MIPI_SEQ_DISPLAY_OFF for
> > > > v3+ VBT's. Testing with VBT v3 the current implementation
> > > > yields
> > > > the
> > > > following error message
> > > > 
> > > > *ERROR* Video mode command 0x00000041 send failed.
> > > > 
> > > > To get rid of this error message, let's limit SHUTDOWN only for
> > > > VBT
> > > > versions 3 or higher.
> > > In the patch you limit it to version 4+, which doesn't make sense
> > > since
> > > AFAIK there is no version 4 of the sequence block.
> > It seems that sending SHUTDOWN signal doesn't make any sense
> > either.
> > Whenever we send that signal it just causes this error message. Do
> > we
> > really need to signal this? From functionality point of view
> > there's no
> > difference.
> Well, the spec doesn't even explain what this "shut down" command
> does.
> Is it actually the DCS "display off" command, or something else?
It seems to do something. At least the intel_wait_for_register() times
out when sending SHUTDOWN signal. Maybe it just shuts down display so
we can't even read the registers after that. 

This brings me to another idea. Maybe we should skip
intel_wait_for_register() if we have sent out SHUTDOWN signal? 

> 
> Did you try reverting bbdf0b2ff32a ("drm/i915/bxt: Disable device
> ready
> before shutdown command")? That looks suspicious to me. But so does
> about half of the DSI code since it never seems to follow the spec,
> and
> we end up doing totally different things on different platforms
> without
> any explanation why that is).
I haven't tried that. I will give it a go and see what happens.

> 
> > 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > > > b/drivers/gpu/drm/i915/intel_dsi.c
> > > > index 2a0f5d3..b48b9b7 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > > @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct
> > > > intel_encoder *encoder,
> > > >  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field
> > > > testing
> > > >  	 * has shown that the v3 sequence works for v2 VBTs
> > > > too
> > > >  	 */
> > > > -	if (is_vid_mode(intel_dsi)) {
> > > > +	if (is_vid_mode(intel_dsi) && dev_priv-
> > > > > 
> > > > > vbt.dsi.seq_version > 3) {
> > > >  		/* Send Shutdown command to the panel in LP
> > > > mode
> > > > */
> > > >  		for_each_dsi_port(port, intel_dsi->ports)
> > > >  			dpi_send_cmd(intel_dsi, SHUTDOWN,
> > > > false,
> > > > port);
> > > > -- 
> > > > 2.7.4
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > -- 
> > Mika Kahola - Intel OTC
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Shankar, Uma]

>-----Original Message-----
>From: Kahola, Mika
>Sent: Tuesday, September 5, 2017 2:03 PM
>To: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>
>Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+
>VBT's
>
>On Mon, 2017-09-04 at 18:04 +0300, Ville Syrjälä wrote:
>> On Mon, Sep 04, 2017 at 10:59:32AM +0300, Mika Kahola wrote:
>> >
>> > On Fri, 2017-09-01 at 16:43 +0300, Ville Syrjälä wrote:
>> > >
>> > > On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
>> > > >
>> > > >
>> > > > According to spec we should send SHUTDOWN before
>> > > > MIPI_SEQ_DISPLAY_OFF for
>> > > > v3+ VBT's. Testing with VBT v3 the current implementation
>> > > > yields
>> > > > the
>> > > > following error message
>> > > >
>> > > > *ERROR* Video mode command 0x00000041 send failed.
>> > > >
>> > > > To get rid of this error message, let's limit SHUTDOWN only for
>> > > > VBT versions 3 or higher.
>> > > In the patch you limit it to version 4+, which doesn't make sense
>> > > since AFAIK there is no version 4 of the sequence block.
>> > It seems that sending SHUTDOWN signal doesn't make any sense either.
>> > Whenever we send that signal it just causes this error message. Do
>> > we really need to signal this? From functionality point of view
>> > there's no difference.
>> Well, the spec doesn't even explain what this "shut down" command
>> does.
>> Is it actually the DCS "display off" command, or something else?
>It seems to do something. At least the intel_wait_for_register() times out when
>sending SHUTDOWN signal. Maybe it just shuts down display so we can't even
>read the registers after that.
>
>This brings me to another idea. Maybe we should skip
>intel_wait_for_register() if we have sent out SHUTDOWN signal?
>
>>
>> Did you try reverting bbdf0b2ff32a ("drm/i915/bxt: Disable device
>> ready before shutdown command")? That looks suspicious to me. But so
>> does about half of the DSI code since it never seems to follow the
>> spec, and we end up doing totally different things on different
>> platforms without any explanation why that is).
>I haven't tried that. I will give it a go and see what happens.
>

The issue is happening due to Device Ready setting. It was added to resolve a DSI split screen
issue in  dual link DSI panels. Since DSI dual link is not enabled and will require a bit of work in
upstream, we should revert this change. Vidya will  send the change and also work on enabling
dual link support in upstream.

FYI - SHUTDOWN is a short packet peripheral command that turns off the display in a Video Mode
display module for power saving. 

For DSI, there have been issues with different panels requiring some unconventional changes, and also bspec
sometimes doesn't cover everything generically.  So, it gets a bit challenging :(

Regards,
Uma Shankar

>> >
>> >
>> > >
>> > >
>> > > >
>> > > >
>> > > >
>> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
>> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> > > > ---
>> > > >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
>> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
>> > > > b/drivers/gpu/drm/i915/intel_dsi.c
>> > > > index 2a0f5d3..b48b9b7 100644
>> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
>> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> > > > @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct
>> > > > intel_encoder *encoder,
>> > > >  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
>> > > >  	 * has shown that the v3 sequence works for v2 VBTs too
>> > > >  	 */
>> > > > -	if (is_vid_mode(intel_dsi)) {
>> > > > +	if (is_vid_mode(intel_dsi) && dev_priv-
>> > > > >
>> > > > > vbt.dsi.seq_version > 3) {
>> > > >  		/* Send Shutdown command to the panel in LP mode */
>> > > >  		for_each_dsi_port(port, intel_dsi->ports)
>> > > >  			dpi_send_cmd(intel_dsi, SHUTDOWN, false, port);
>> > > > --
>> > > > 2.7.4
>> > > >
>> > > > _______________________________________________
>> > > > Intel-gfx mailing list
>> > > > Intel-gfx@lists.freedesktop.org
>> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> > --
>> > Mika Kahola - Intel OTC
>--
>Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] Revert "drm/i915/bxt: Disable device ready before shutdown command"

[Vidya Srinivas]

From: Uma Shankar <uma.shankar@intel.com>

This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282.

Disable device ready before shutdown command was added previously to avoid a
split screen issue seen on dual link DSI panels. As of now, dual link is not
supported and will need some rework in the upstream code. For single link DSI
panels, the change is not required. This will cause failure in sending SHUTDOWN
packet during disable. Hence reverting the change. Will handle the change
as part of dual link enabling in upstream.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 2a0f5d3..fc25d7d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -892,8 +892,6 @@ static void intel_dsi_disable(struct intel_encoder *encoder,
 			      const struct intel_crtc_state *old_crtc_state,
 			      const struct drm_connector_state *old_conn_state)
 {
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	enum port port;
 
@@ -903,15 +901,6 @@ static void intel_dsi_disable(struct intel_encoder *encoder,
 	intel_panel_disable_backlight(old_conn_state);
 
 	/*
-	 * Disable Device ready before the port shutdown in order
-	 * to avoid split screen
-	 */
-	if (IS_BROXTON(dev_priv)) {
-		for_each_dsi_port(port, intel_dsi->ports)
-			I915_WRITE(MIPI_DEVICE_READY(port), 0);
-	}
-
-	/*
 	 * According to the spec we should send SHUTDOWN before
 	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
 	 * has shown that the v3 sequence works for v2 VBTs too
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dsi: Fix error on DSI video mode command (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dsi: Fix error on DSI video mode command (rev2)
URL   : https://patchwork.freedesktop.org/series/29658/
State : success

== Summary ==

Series 29658v2 drm/i915/dsi: Fix error on DSI video mode command
https://patchwork.freedesktop.org/api/1.0/series/29658/revisions/2/mbox/

Test kms_cursor_legacy:
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> FAIL       (fi-hsw-4770) fdo#102402 +1

fdo#102402 https://bugs.freedesktop.org/show_bug.cgi?id=102402

fi-bdw-5557u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:457s
fi-bdw-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:442s
fi-blb-e6850     total:288  pass:224  dwarn:1   dfail:0   fail:0   skip:63  time:368s
fi-bsw-n3050     total:288  pass:243  dwarn:0   dfail:0   fail:0   skip:45  time:564s
fi-bwr-2160      total:288  pass:184  dwarn:0   dfail:0   fail:0   skip:104 time:254s
fi-bxt-j4205     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:519s
fi-byt-j1900     total:288  pass:254  dwarn:1   dfail:0   fail:0   skip:33  time:522s
fi-byt-n2820     total:288  pass:250  dwarn:1   dfail:0   fail:0   skip:37  time:511s
fi-cfl-s         total:288  pass:250  dwarn:4   dfail:0   fail:0   skip:34  time:460s
fi-elk-e7500     total:288  pass:230  dwarn:0   dfail:0   fail:0   skip:58  time:438s
fi-glk-2a        total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:617s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:2   skip:25  time:459s
fi-hsw-4770r     total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:425s
fi-ilk-650       total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:421s
fi-ivb-3520m     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:503s
fi-ivb-3770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:474s
fi-kbl-7500u     total:288  pass:264  dwarn:1   dfail:0   fail:0   skip:23  time:509s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:595s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:598s
fi-pnv-d510      total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:524s
fi-skl-6260u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:468s
fi-skl-6700k     total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:534s
fi-skl-6770hq    total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:516s
fi-skl-gvtdvm    total:288  pass:266  dwarn:0   dfail:0   fail:0   skip:22  time:442s
fi-skl-x1585l    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:489s
fi-snb-2520m     total:288  pass:251  dwarn:0   dfail:0   fail:0   skip:37  time:546s
fi-snb-2600      total:288  pass:249  dwarn:0   dfail:0   fail:1   skip:38  time:402s

9dd459ef87a90495aac4cee73831f4cd694048ca drm-tip: 2017y-09m-04d-19h-10m-24s UTC integration manifest
8c3226fa5a71 drm/i915/dsi: Replace MIPI command error message with debug message
05d0ec5d45a3 Revert "drm/i915/bxt: Disable device ready before shutdown command"

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5579/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] Revert "drm/i915/bxt: Disable device ready before shutdown command"

[Mika Kahola]

The revert patch solved the issue

*ERROR* Video mode command 0x00000041 send failed.

on my setup with APL+MIPI/DSI single link combo.

Tested-by: Mika Kahola <mika.kahola@intel.com>


On Tue, 2017-09-05 at 15:14 +0530, Vidya Srinivas wrote:
> From: Uma Shankar <uma.shankar@intel.com>
> 
> This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282.
> 
> Disable device ready before shutdown command was added previously to
> avoid a
> split screen issue seen on dual link DSI panels. As of now, dual link
> is not
> supported and will need some rework in the upstream code. For single
> link DSI
> panels, the change is not required. This will cause failure in
> sending SHUTDOWN
> packet during disable. Hence reverting the change. Will handle the
> change
> as part of dual link enabling in upstream.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 2a0f5d3..fc25d7d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -892,8 +892,6 @@ static void intel_dsi_disable(struct
> intel_encoder *encoder,
>  			      const struct intel_crtc_state
> *old_crtc_state,
>  			      const struct drm_connector_state
> *old_conn_state)
>  {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder-
> >base);
>  	enum port port;
>  
> @@ -903,15 +901,6 @@ static void intel_dsi_disable(struct
> intel_encoder *encoder,
>  	intel_panel_disable_backlight(old_conn_state);
>  
>  	/*
> -	 * Disable Device ready before the port shutdown in order
> -	 * to avoid split screen
> -	 */
> -	if (IS_BROXTON(dev_priv)) {
> -		for_each_dsi_port(port, intel_dsi->ports)
> -			I915_WRITE(MIPI_DEVICE_READY(port), 0);
> -	}
> -
> -	/*
>  	 * According to the spec we should send SHUTDOWN before
>  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
>  	 * has shown that the v3 sequence works for v2 VBTs too
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915/dsi: Fix error on DSI video mode command (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dsi: Fix error on DSI video mode command (rev2)
URL   : https://patchwork.freedesktop.org/series/29658/
State : failure

== Summary ==

Test tools_test:
        Subgroup tools_test:
                pass       -> DMESG-WARN (shard-hsw) fdo#102543
Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252 +1
Test gem_sync:
        Subgroup basic-many-each:
                pass       -> FAIL       (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912

fdo#102543 https://bugs.freedesktop.org/show_bug.cgi?id=102543
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2265 pass:1232 dwarn:1   dfail:0   fail:16  skip:1016 time:9659s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5579/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Ville Syrjälä]

On Tue, Sep 05, 2017 at 09:27:47AM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Kahola, Mika
> >Sent: Tuesday, September 5, 2017 2:03 PM
> >To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>
> >Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+
> >VBT's
> >
> >On Mon, 2017-09-04 at 18:04 +0300, Ville Syrjälä wrote:
> >> On Mon, Sep 04, 2017 at 10:59:32AM +0300, Mika Kahola wrote:
> >> >
> >> > On Fri, 2017-09-01 at 16:43 +0300, Ville Syrjälä wrote:
> >> > >
> >> > > On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
> >> > > >
> >> > > >
> >> > > > According to spec we should send SHUTDOWN before
> >> > > > MIPI_SEQ_DISPLAY_OFF for
> >> > > > v3+ VBT's. Testing with VBT v3 the current implementation
> >> > > > yields
> >> > > > the
> >> > > > following error message
> >> > > >
> >> > > > *ERROR* Video mode command 0x00000041 send failed.
> >> > > >
> >> > > > To get rid of this error message, let's limit SHUTDOWN only for
> >> > > > VBT versions 3 or higher.
> >> > > In the patch you limit it to version 4+, which doesn't make sense
> >> > > since AFAIK there is no version 4 of the sequence block.
> >> > It seems that sending SHUTDOWN signal doesn't make any sense either.
> >> > Whenever we send that signal it just causes this error message. Do
> >> > we really need to signal this? From functionality point of view
> >> > there's no difference.
> >> Well, the spec doesn't even explain what this "shut down" command
> >> does.
> >> Is it actually the DCS "display off" command, or something else?
> >It seems to do something. At least the intel_wait_for_register() times out when
> >sending SHUTDOWN signal. Maybe it just shuts down display so we can't even
> >read the registers after that.
> >
> >This brings me to another idea. Maybe we should skip
> >intel_wait_for_register() if we have sent out SHUTDOWN signal?
> >
> >>
> >> Did you try reverting bbdf0b2ff32a ("drm/i915/bxt: Disable device
> >> ready before shutdown command")? That looks suspicious to me. But so
> >> does about half of the DSI code since it never seems to follow the
> >> spec, and we end up doing totally different things on different
> >> platforms without any explanation why that is).
> >I haven't tried that. I will give it a go and see what happens.
> >
> 
> The issue is happening due to Device Ready setting. It was added to resolve a DSI split screen
> issue in  dual link DSI panels. Since DSI dual link is not enabled and will require a bit of work in
> upstream, we should revert this change. Vidya will  send the change and also work on enabling
> dual link support in upstream.
> 
> FYI - SHUTDOWN is a short packet peripheral command that turns off the display in a Video Mode
> display module for power saving. 

There is no SHUTDOWN comamnd in the DCS spec that I can find.

> 
> For DSI, there have been issues with different panels requiring some unconventional changes, and also bspec
> sometimes doesn't cover everything generically.  So, it gets a bit challenging :(
> 
> Regards,
> Uma Shankar
> 
> >> >
> >> >
> >> > >
> >> > >
> >> > > >
> >> > > >
> >> > > >
> >> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
> >> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >> > > > ---
> >> > > >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> >> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> > > >
> >> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> >> > > > b/drivers/gpu/drm/i915/intel_dsi.c
> >> > > > index 2a0f5d3..b48b9b7 100644
> >> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> >> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >> > > > @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct
> >> > > > intel_encoder *encoder,
> >> > > >  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
> >> > > >  	 * has shown that the v3 sequence works for v2 VBTs too
> >> > > >  	 */
> >> > > > -	if (is_vid_mode(intel_dsi)) {
> >> > > > +	if (is_vid_mode(intel_dsi) && dev_priv-
> >> > > > >
> >> > > > > vbt.dsi.seq_version > 3) {
> >> > > >  		/* Send Shutdown command to the panel in LP mode */
> >> > > >  		for_each_dsi_port(port, intel_dsi->ports)
> >> > > >  			dpi_send_cmd(intel_dsi, SHUTDOWN, false, port);
> >> > > > --
> >> > > > 2.7.4
> >> > > >
> >> > > > _______________________________________________
> >> > > > Intel-gfx mailing list
> >> > > > Intel-gfx@lists.freedesktop.org
> >> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> > --
> >> > Mika Kahola - Intel OTC
> >--
> >Mika Kahola - Intel OTC
> 

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, September 5, 2017 6:46 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: Kahola, Mika <mika.kahola@intel.com>; Srinivas, Vidya
><vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+
>VBT's
>
>On Tue, Sep 05, 2017 at 09:27:47AM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: Kahola, Mika
>> >Sent: Tuesday, September 5, 2017 2:03 PM
>> >To: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma
>> ><uma.shankar@intel.com>
>> >Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only
>> >for v3+ VBT's
>> >
>> >On Mon, 2017-09-04 at 18:04 +0300, Ville Syrjälä wrote:
>> >> On Mon, Sep 04, 2017 at 10:59:32AM +0300, Mika Kahola wrote:
>> >> >
>> >> > On Fri, 2017-09-01 at 16:43 +0300, Ville Syrjälä wrote:
>> >> > >
>> >> > > On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
>> >> > > >
>> >> > > >
>> >> > > > According to spec we should send SHUTDOWN before
>> >> > > > MIPI_SEQ_DISPLAY_OFF for
>> >> > > > v3+ VBT's. Testing with VBT v3 the current implementation
>> >> > > > yields
>> >> > > > the
>> >> > > > following error message
>> >> > > >
>> >> > > > *ERROR* Video mode command 0x00000041 send failed.
>> >> > > >
>> >> > > > To get rid of this error message, let's limit SHUTDOWN only
>> >> > > > for VBT versions 3 or higher.
>> >> > > In the patch you limit it to version 4+, which doesn't make
>> >> > > sense since AFAIK there is no version 4 of the sequence block.
>> >> > It seems that sending SHUTDOWN signal doesn't make any sense either.
>> >> > Whenever we send that signal it just causes this error message.
>> >> > Do we really need to signal this? From functionality point of
>> >> > view there's no difference.
>> >> Well, the spec doesn't even explain what this "shut down" command
>> >> does.
>> >> Is it actually the DCS "display off" command, or something else?
>> >It seems to do something. At least the intel_wait_for_register()
>> >times out when sending SHUTDOWN signal. Maybe it just shuts down
>> >display so we can't even read the registers after that.
>> >
>> >This brings me to another idea. Maybe we should skip
>> >intel_wait_for_register() if we have sent out SHUTDOWN signal?
>> >
>> >>
>> >> Did you try reverting bbdf0b2ff32a ("drm/i915/bxt: Disable device
>> >> ready before shutdown command")? That looks suspicious to me. But
>> >> so does about half of the DSI code since it never seems to follow
>> >> the spec, and we end up doing totally different things on different
>> >> platforms without any explanation why that is).
>> >I haven't tried that. I will give it a go and see what happens.
>> >
>>
>> The issue is happening due to Device Ready setting. It was added to
>> resolve a DSI split screen issue in  dual link DSI panels. Since DSI
>> dual link is not enabled and will require a bit of work in upstream,
>> we should revert this change. Vidya will  send the change and also work on
>enabling dual link support in upstream.
>>
>> FYI - SHUTDOWN is a short packet peripheral command that turns off the
>> display in a Video Mode display module for power saving.
>
>There is no SHUTDOWN comamnd in the DCS spec that I can find.
>

Hi Ville,
It's part of MIPI_DSI_specification - (v1-3 is the latest)
8.8.5 Shutdown Peripheral Command, Data Type = 10 0010 (0x22)
Shutdown Peripheral command is a Short packet command that turns off the display
in a Video Mode display module for power saving. Note the interface shall remain powered
in order to receive the turn-on,  or wake-up, command.

Regards,
Uma Shankar

>>
>> For DSI, there have been issues with different panels requiring some
>> unconventional changes, and also bspec sometimes doesn't cover
>> everything generically.  So, it gets a bit challenging :(
>>
>> Regards,
>> Uma Shankar
>>
>> >> >
>> >> >
>> >> > >
>> >> > >
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
>> >> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> >> > > > ---
>> >> > > >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
>> >> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> > > >
>> >> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
>> >> > > > b/drivers/gpu/drm/i915/intel_dsi.c
>> >> > > > index 2a0f5d3..b48b9b7 100644
>> >> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
>> >> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> >> > > > @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct
>> >> > > > intel_encoder *encoder,
>> >> > > >  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
>> >> > > >  	 * has shown that the v3 sequence works for v2 VBTs too
>> >> > > >  	 */
>> >> > > > -	if (is_vid_mode(intel_dsi)) {
>> >> > > > +	if (is_vid_mode(intel_dsi) && dev_priv-
>> >> > > > >
>> >> > > > > vbt.dsi.seq_version > 3) {
>> >> > > >  		/* Send Shutdown command to the panel in LP mode */
>> >> > > >  		for_each_dsi_port(port, intel_dsi->ports)
>> >> > > >  			dpi_send_cmd(intel_dsi, SHUTDOWN, false,
>port);
>> >> > > > --
>> >> > > > 2.7.4
>> >> > > >
>> >> > > > _______________________________________________
>> >> > > > Intel-gfx mailing list
>> >> > > > Intel-gfx@lists.freedesktop.org
>> >> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> > --
>> >> > Mika Kahola - Intel OTC
>> >--
>> >Mika Kahola - Intel OTC
>>
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Ville Syrjälä]

On Tue, Sep 05, 2017 at 02:55:31PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >Sent: Tuesday, September 5, 2017 6:46 PM
> >To: Shankar, Uma <uma.shankar@intel.com>
> >Cc: Kahola, Mika <mika.kahola@intel.com>; Srinivas, Vidya
> ><vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org
> >Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+
> >VBT's
> >
> >On Tue, Sep 05, 2017 at 09:27:47AM +0000, Shankar, Uma wrote:
> >>
> >>
> >> >-----Original Message-----
> >> >From: Kahola, Mika
> >> >Sent: Tuesday, September 5, 2017 2:03 PM
> >> >To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma
> >> ><uma.shankar@intel.com>
> >> >Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only
> >> >for v3+ VBT's
> >> >
> >> >On Mon, 2017-09-04 at 18:04 +0300, Ville Syrjälä wrote:
> >> >> On Mon, Sep 04, 2017 at 10:59:32AM +0300, Mika Kahola wrote:
> >> >> >
> >> >> > On Fri, 2017-09-01 at 16:43 +0300, Ville Syrjälä wrote:
> >> >> > >
> >> >> > > On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
> >> >> > > >
> >> >> > > >
> >> >> > > > According to spec we should send SHUTDOWN before
> >> >> > > > MIPI_SEQ_DISPLAY_OFF for
> >> >> > > > v3+ VBT's. Testing with VBT v3 the current implementation
> >> >> > > > yields
> >> >> > > > the
> >> >> > > > following error message
> >> >> > > >
> >> >> > > > *ERROR* Video mode command 0x00000041 send failed.
> >> >> > > >
> >> >> > > > To get rid of this error message, let's limit SHUTDOWN only
> >> >> > > > for VBT versions 3 or higher.
> >> >> > > In the patch you limit it to version 4+, which doesn't make
> >> >> > > sense since AFAIK there is no version 4 of the sequence block.
> >> >> > It seems that sending SHUTDOWN signal doesn't make any sense either.
> >> >> > Whenever we send that signal it just causes this error message.
> >> >> > Do we really need to signal this? From functionality point of
> >> >> > view there's no difference.
> >> >> Well, the spec doesn't even explain what this "shut down" command
> >> >> does.
> >> >> Is it actually the DCS "display off" command, or something else?
> >> >It seems to do something. At least the intel_wait_for_register()
> >> >times out when sending SHUTDOWN signal. Maybe it just shuts down
> >> >display so we can't even read the registers after that.
> >> >
> >> >This brings me to another idea. Maybe we should skip
> >> >intel_wait_for_register() if we have sent out SHUTDOWN signal?
> >> >
> >> >>
> >> >> Did you try reverting bbdf0b2ff32a ("drm/i915/bxt: Disable device
> >> >> ready before shutdown command")? That looks suspicious to me. But
> >> >> so does about half of the DSI code since it never seems to follow
> >> >> the spec, and we end up doing totally different things on different
> >> >> platforms without any explanation why that is).
> >> >I haven't tried that. I will give it a go and see what happens.
> >> >
> >>
> >> The issue is happening due to Device Ready setting. It was added to
> >> resolve a DSI split screen issue in  dual link DSI panels. Since DSI
> >> dual link is not enabled and will require a bit of work in upstream,
> >> we should revert this change. Vidya will  send the change and also work on
> >enabling dual link support in upstream.
> >>
> >> FYI - SHUTDOWN is a short packet peripheral command that turns off the
> >> display in a Video Mode display module for power saving.
> >
> >There is no SHUTDOWN comamnd in the DCS spec that I can find.
> >
> 
> Hi Ville,
> It's part of MIPI_DSI_specification - (v1-3 is the latest)
> 8.8.5 Shutdown Peripheral Command, Data Type = 10 0010 (0x22)
> Shutdown Peripheral command is a Short packet command that turns off the display
> in a Video Mode display module for power saving. Note the interface shall remain powered
> in order to receive the turn-on,  or wake-up, command.

Ah, thanks. I was only looking at the DCS spec.

> 
> Regards,
> Uma Shankar
> 
> >>
> >> For DSI, there have been issues with different panels requiring some
> >> unconventional changes, and also bspec sometimes doesn't cover
> >> everything generically.  So, it gets a bit challenging :(
> >>
> >> Regards,
> >> Uma Shankar
> >>
> >> >> >
> >> >> >
> >> >> > >
> >> >> > >
> >> >> > > >
> >> >> > > >
> >> >> > > >
> >> >> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
> >> >> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >> >> > > > ---
> >> >> > > >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> >> >> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> > > >
> >> >> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> >> >> > > > b/drivers/gpu/drm/i915/intel_dsi.c
> >> >> > > > index 2a0f5d3..b48b9b7 100644
> >> >> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> >> >> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >> >> > > > @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct
> >> >> > > > intel_encoder *encoder,
> >> >> > > >  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
> >> >> > > >  	 * has shown that the v3 sequence works for v2 VBTs too
> >> >> > > >  	 */
> >> >> > > > -	if (is_vid_mode(intel_dsi)) {
> >> >> > > > +	if (is_vid_mode(intel_dsi) && dev_priv-
> >> >> > > > >
> >> >> > > > > vbt.dsi.seq_version > 3) {
> >> >> > > >  		/* Send Shutdown command to the panel in LP mode */
> >> >> > > >  		for_each_dsi_port(port, intel_dsi->ports)
> >> >> > > >  			dpi_send_cmd(intel_dsi, SHUTDOWN, false,
> >port);
> >> >> > > > --
> >> >> > > > 2.7.4
> >> >> > > >
> >> >> > > > _______________________________________________
> >> >> > > > Intel-gfx mailing list
> >> >> > > > Intel-gfx@lists.freedesktop.org
> >> >> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >> > --
> >> >> > Mika Kahola - Intel OTC
> >> >--
> >> >Mika Kahola - Intel OTC
> >>
> >
> >--
> >Ville Syrjälä
> >Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] Revert "drm/i915/bxt: Disable device ready before shutdown command"

[Chauhan, Madhav]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Vidya Srinivas
> Sent: Tuesday, September 5, 2017 3:15 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Srinivas, Vidya <vidya.srinivas@intel.com>
> Subject: [Intel-gfx] [PATCH] Revert "drm/i915/bxt: Disable device ready
> before shutdown command"
> 
> From: Uma Shankar <uma.shankar@intel.com>
> 
> This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282.
> 
> Disable device ready before shutdown command was added previously to
> avoid a split screen issue seen on dual link DSI panels. As of now, dual link is
> not supported and will need some rework in the upstream code. For single
> link DSI panels, the change is not required. This will cause failure in sending
> SHUTDOWN packet during disable. Hence reverting the change. Will handle
> the change as part of dual link enabling in upstream.

Already there are lot of changes  wrt  to DSI dual link inside intel_dsi.c.
Can't we keep this change as well under the condition if (dual_link && Broxton)??
Might help to fetch dual link changes in future instead of losing it.

Regards,
Madhav

> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 2a0f5d3..fc25d7d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -892,8 +892,6 @@ static void intel_dsi_disable(struct intel_encoder
> *encoder,
>  			      const struct intel_crtc_state *old_crtc_state,
>  			      const struct drm_connector_state
> *old_conn_state)  {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	enum port port;
> 
> @@ -903,15 +901,6 @@ static void intel_dsi_disable(struct intel_encoder
> *encoder,
>  	intel_panel_disable_backlight(old_conn_state);
> 
>  	/*
> -	 * Disable Device ready before the port shutdown in order
> -	 * to avoid split screen
> -	 */
> -	if (IS_BROXTON(dev_priv)) {
> -		for_each_dsi_port(port, intel_dsi->ports)
> -			I915_WRITE(MIPI_DEVICE_READY(port), 0);
> -	}
> -
> -	/*
>  	 * According to the spec we should send SHUTDOWN before
>  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
>  	 * has shown that the v3 sequence works for v2 VBTs too
> --
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] Revert "drm/i915/bxt: Disable device ready before shutdown command"

[Jani Nikula]

On Tue, 05 Sep 2017, Mika Kahola <mika.kahola@intel.com> wrote:
> The revert patch solved the issue
>
> *ERROR* Video mode command 0x00000041 send failed.
>
> on my setup with APL+MIPI/DSI single link combo.
>
> Tested-by: Mika Kahola <mika.kahola@intel.com>

Pushed to dinq, thanks for the patch and testing. Alas, I failed to add
your Tested-by tag, apologies.

BR,
Jani.


>
>
> On Tue, 2017-09-05 at 15:14 +0530, Vidya Srinivas wrote:
>> From: Uma Shankar <uma.shankar@intel.com>
>> 
>> This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282.
>> 
>> Disable device ready before shutdown command was added previously to
>> avoid a
>> split screen issue seen on dual link DSI panels. As of now, dual link
>> is not
>> supported and will need some rework in the upstream code. For single
>> link DSI
>> panels, the change is not required. This will cause failure in
>> sending SHUTDOWN
>> packet during disable. Hence reverting the change. Will handle the
>> change
>> as part of dual link enabling in upstream.
>> 
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dsi.c | 11 -----------
>>  1 file changed, 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
>> b/drivers/gpu/drm/i915/intel_dsi.c
>> index 2a0f5d3..fc25d7d 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -892,8 +892,6 @@ static void intel_dsi_disable(struct
>> intel_encoder *encoder,
>>  			      const struct intel_crtc_state
>> *old_crtc_state,
>>  			      const struct drm_connector_state
>> *old_conn_state)
>>  {
>> -	struct drm_device *dev = encoder->base.dev;
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder-
>> >base);
>>  	enum port port;
>>  
>> @@ -903,15 +901,6 @@ static void intel_dsi_disable(struct
>> intel_encoder *encoder,
>>  	intel_panel_disable_backlight(old_conn_state);
>>  
>>  	/*
>> -	 * Disable Device ready before the port shutdown in order
>> -	 * to avoid split screen
>> -	 */
>> -	if (IS_BROXTON(dev_priv)) {
>> -		for_each_dsi_port(port, intel_dsi->ports)
>> -			I915_WRITE(MIPI_DEVICE_READY(port), 0);
>> -	}
>> -
>> -	/*
>>  	 * According to the spec we should send SHUTDOWN before
>>  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing
>>  	 * has shown that the v3 sequence works for v2 VBTs too

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] Revert "drm/i915/bxt: Disable device ready before shutdown command"

[Jani Nikula]

On Tue, 12 Sep 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Vidya Srinivas
>> Sent: Tuesday, September 5, 2017 3:15 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Srinivas, Vidya <vidya.srinivas@intel.com>
>> Subject: [Intel-gfx] [PATCH] Revert "drm/i915/bxt: Disable device ready
>> before shutdown command"
>> 
>> From: Uma Shankar <uma.shankar@intel.com>
>> 
>> This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282.
>> 
>> Disable device ready before shutdown command was added previously to
>> avoid a split screen issue seen on dual link DSI panels. As of now, dual link is
>> not supported and will need some rework in the upstream code. For single
>> link DSI panels, the change is not required. This will cause failure in sending
>> SHUTDOWN packet during disable. Hence reverting the change. Will handle
>> the change as part of dual link enabling in upstream.
>
> Already there are lot of changes  wrt  to DSI dual link inside intel_dsi.c.
> Can't we keep this change as well under the condition if (dual_link && Broxton)??
> Might help to fetch dual link changes in future instead of losing it.

I've pushed the patch as-is. I don't think there are grounds to keep it
even with dual_link. As Uma says, these things may be panel specific
too.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/dsi: Replace MIPI command error message with debug message

[Jani Nikula]

On Fri, 01 Sep 2017, Mika Kahola <mika.kahola@intel.com> wrote:
> Error message indicating that the same MIPI command is sent consecutively
> is perhaps too strongly said. Let's replace that as a debug message instead.
>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>

Pushed to dinq, thanks for the patch.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index b48b9b7..e16055d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -263,7 +263,7 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
>  
>  	/* XXX: old code skips write if control unchanged */
>  	if (cmd == I915_READ(MIPI_DPI_CONTROL(port)))
> -		DRM_ERROR("Same special packet %02x twice in a row.\n", cmd);
> +		DRM_DEBUG_KMS("Same special packet %02x twice in a row.\n", cmd);
>  
>  	I915_WRITE(MIPI_DPI_CONTROL(port), cmd);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] Revert "drm/i915/bxt: Disable device ready before shutdown command"

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Wednesday, September 13, 2017 1:39 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; Srinivas, Vidya
> <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH] Revert "drm/i915/bxt: Disable device ready
> before shutdown command"
> 
> On Tue, 12 Sep 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> >> Behalf Of Vidya Srinivas
> >> Sent: Tuesday, September 5, 2017 3:15 PM
> >> To: intel-gfx@lists.freedesktop.org
> >> Cc: Srinivas, Vidya <vidya.srinivas@intel.com>
> >> Subject: [Intel-gfx] [PATCH] Revert "drm/i915/bxt: Disable device
> >> ready before shutdown command"
> >>
> >> From: Uma Shankar <uma.shankar@intel.com>
> >>
> >> This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282.
> >>
> >> Disable device ready before shutdown command was added previously to
> >> avoid a split screen issue seen on dual link DSI panels. As of now,
> >> dual link is not supported and will need some rework in the upstream
> >> code. For single link DSI panels, the change is not required. This
> >> will cause failure in sending SHUTDOWN packet during disable. Hence
> >> reverting the change. Will handle the change as part of dual link enabling
> in upstream.
> >
> > Already there are lot of changes  wrt  to DSI dual link inside intel_dsi.c.
> > Can't we keep this change as well under the condition if (dual_link &&
> Broxton)??
> > Might help to fetch dual link changes in future instead of losing it.
> 
> I've pushed the patch as-is. I don't think there are grounds to keep it even
> with dual_link. As Uma says, these things may be panel specific too.

Hmm..Ok Thanks for clarification.

Regards,
Madhav

> 
> BR,
> Jani.
> 
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx