[Intel-gfx] [PATCH resend 07/15] drm/i915/dsi: Drop bogus MIPI_SEQ_ASSERT_RESET before POWER_ON

[Bob Paauwe]

On Sat, 25 Feb 2017 11:35:03 +0100
Hans de Goede <hdegoede at redhat.com> wrote:

> Hi,
> 
> On 24-02-17 18:00, Bob Paauwe wrote:
> > On Mon, 20 Feb 2017 15:08:37 +0100
> > Hans de Goede <hdegoede at redhat.com> wrote:
> >  
> >> MIPI_SEQ_ASSERT_RESET before POWER_ON is not necessary for 2 reasons:
> >> 1) The reset should already be asserted before intel_dsi_pre_enable()
> >>    gets called
> >> 2) Most (some?) VBTs will ensure reset was asserted in their
> >>    MIPI_SEQ_DEASSERT_RESET themselves
> >>
> >> Signed-off-by: Hans de Goede <hdegoede at redhat.com>  
> >
> > Using words like "should" and some? don't inspire a lot of confidence
> > that this is the right thing to do. Is there some way to verify that
> > these two conditions are true?  
> 
> Well the i915 code should never call intel_dsi_[pre_]enable without
> first having called intel_dsi_post_disable() which does:
> 
> intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
> 
> Already, also this is undoing a recent (4.9 kernel IIRC) change,
> before that the i915 did not call the MIPI_SEQ_ASSERT_RESET
> sequence from the enable path. Also doing this is against the VBT
> spec.
> 
> Regards,
> 
> Hans
> 

So let's try to make the commit message state that.  Using what you
just wrote maybe something like:

intel_dsi_post_disable(), which does the MIPI_SEQ_ASSERT_RESET,
will always be called at some point before intel_dsi_pre_enable()
making the MIPI_SEQ_ASSERT_RESET in intel_dsi_pre_enable() redundent. 

In addition, calling MIPI_SEQ_ASSERT_RESET in the enable path goes
against the VBT spec.

With the commit message updated the change looks to be correct.


> 
> 
> >  
> >> ---
> >>  drivers/gpu/drm/i915/intel_dsi.c | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> >> index 78d5884..4ebf308 100644
> >> --- a/drivers/gpu/drm/i915/intel_dsi.c
> >> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >> @@ -669,7 +669,6 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
> >>  	/* put device in ready state */
> >>  	intel_dsi_device_ready(encoder);
> >>
> >> -	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
> >>  	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
> >>  	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
> >>  	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);  
> >
> >
> >  



-- 
--
Bob Paauwe                  
Bob.J.Paauwe at intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193