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

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>Sent: Tuesday, September 5, 2017 6:46 PM
>To: Shankar, Uma <uma.shankar at intel.com>
>Cc: Kahola, Mika <mika.kahola at intel.com>; Srinivas, Vidya
><vidya.srinivas at intel.com>; intel-gfx at 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 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.
>
>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 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
>>
>
>--
>Ville Syrjälä
>Intel OTC