On Sat, 25 Feb 2017 11:35:03 +0100 Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > Hi, > > On 24-02-17 18:00, Bob Paauwe wrote: > > On Mon, 20 Feb 2017 15:08:37 +0100 > > Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx> > > > > 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@xxxxxxxxx IOTG / PED Software Organization Intel Corp. Folsom, CA (916) 356-6193 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx