[Intel-gfx] [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 at linux.intel.com>
>Cc: intel-gfx at lists.freedesktop.org; Shankar, Uma <uma.shankar at 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 at 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 at lists.freedesktop.org
>> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> > --
>> > Mika Kahola - Intel OTC
>--
>Mika Kahola - Intel OTC