On Mon, 27 Feb 2017, Bob Paauwe <bob.j.paauwe@xxxxxxxxx> wrote:
> On Sat, 25 Feb 2017 11:42:09 +0100
> Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
>> Hi,
>>
>> On 24-02-17 18:02, Bob Paauwe wrote:
>> > On Mon, 20 Feb 2017 15:08:42 +0100
>> > Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>> >
>> >> According to the spec for v2 VBTs we should call MIPI_SEQ_DISPLAY_OFF
>> >> before sending SHUTDOWN, where as for v3 VBTs we should send SHUTDOWN
>> >> first.
>> >>
>> >> Since the v2 order has known issues, we use the v3 order everywhere,
>> >> add a comment documenting this.
>> >>
>> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>> >> ---
>> >> drivers/gpu/drm/i915/intel_dsi.c | 7 +++++++
>> >> 1 file changed, 7 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> >> index a8d0948..1914311 100644
>> >> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> >> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> >> @@ -732,6 +732,11 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder,
>> >> I915_WRITE(MIPI_DEVICE_READY(port), 0);
>> >> }
>> >>
>> >> + /*
>> >> + * XXX: According to the spec we should send SHUTDOWN before
>> >> + * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but testing in the field
>> >> + * has shown that we should do this for v2 VBTs too?
>> > drop the '?'
>>
>> I added that because I'm not 100% sure this is true, looking through git
>> history (and android x86 kernel patch-sets) I managed to piece together that
>> at one point in time the v2 sequence was used, but that yielded problems
>> during some testing, what the commits do not tell if is that testing was
>> using boards with v3 VBTs, but assuming v2 tables are out there in the
>> wild then it seems that the v3 order works fine for v2 too.
>>
>> TLDR I'm not 100% sure about this hence the '?', my main goal with this
>> patch is to document that we're deviating from the spec for v2 tables here.
>
> If anyone else, Jani?, has more information about this, that would be
> good to know.
I wish. The documentation on this is disgraceful.
> I'd be OK with just stating that "field testing has shown that the v3
> sequence works with v2 VBT's so just use that."
Ack.
>
>>
>> >> + */
>> >> if (is_vid_mode(intel_dsi)) {
>> >> /* Send Shutdown command to the panel in LP mode */
>> >> for_each_dsi_port(port, intel_dsi->ports)
>> >> @@ -764,6 +769,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder,
>> >> /*
>> >> * if disable packets are sent before sending shutdown packet then in
>> >> * some next enable sequence send turn on packet error is observed
>> >> + * XXX spec specifies SHUTDOWN before MIPI_SEQ_DISPLAY_OFF for
>> >> + * v3 VBTs, but not for v2 VBTs?
>> >> */
>> >> intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
>> >>
>> >
>> > Should XXX be replaced with something?
>>
>> XXX is used in many places in intel_dsi.c to indicate code which may need
>> work / which may needs to be investigated further. I followed that and
>> added XXX here since this code is deviating from the spec.
>>
>> Regards,
>>
>> Hans
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
