Re: [Intel-gfx] [PATCH resend 12/15] drm/i915/dsi: Document always using v3 SHUTDOWN / MIPI_SEQ_DISPLAY_OFF order
Hi,
On 27-02-17 18:16, Bob Paauwe wrote:
On Sat, 25 Feb 2017 11:42:09 +0100
Hans de Goede wrote:
Hi,
On 24-02-17 18:02, Bob Paauwe wrote:
On Mon, 20 Feb 2017 15:08:42 +0100
Hans de Goede 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
---
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'd be OK with just stating that "field testing has shown that the v3
sequence works with v2 VBT's so just use that."
Ok, done for v2.
Regards,
Hans
+*/
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH resend 12/15] drm/i915/dsi: Document always using v3 SHUTDOWN / MIPI_SEQ_DISPLAY_OFF order
On Mon, 27 Feb 2017, Bob Paauwe wrote:
> On Sat, 25 Feb 2017 11:42:09 +0100
> Hans de Goede wrote:
>
>> Hi,
>>
>> On 24-02-17 18:02, Bob Paauwe wrote:
>> > On Mon, 20 Feb 2017 15:08:42 +0100
>> > Hans de Goede 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
>> >> ---
>> >> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH resend 12/15] drm/i915/dsi: Document always using v3 SHUTDOWN / MIPI_SEQ_DISPLAY_OFF order
On Sat, 25 Feb 2017 11:42:09 +0100
Hans de Goede wrote:
> Hi,
>
> On 24-02-17 18:02, Bob Paauwe wrote:
> > On Mon, 20 Feb 2017 15:08:42 +0100
> > Hans de Goede 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
> >> ---
> >> 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'd be OK with just stating that "field testing has shown that the v3
sequence works with v2 VBT's so just use that."
>
> >> + */
> >>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
--
--
Bob Paauwe
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp. Folsom, CA
(916) 356-6193
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH resend 12/15] drm/i915/dsi: Document always using v3 SHUTDOWN / MIPI_SEQ_DISPLAY_OFF order
Hi,
On 24-02-17 18:02, Bob Paauwe wrote:
On Mon, 20 Feb 2017 15:08:42 +0100
Hans de Goede 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
---
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 (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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH resend 12/15] drm/i915/dsi: Document always using v3 SHUTDOWN / MIPI_SEQ_DISPLAY_OFF order
On Mon, 20 Feb 2017 15:08:42 +0100
Hans de Goede 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
> ---
> 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 '?'
> + */
> 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?
--
--
Bob Paauwe
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp. Folsom, CA
(916) 356-6193
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH resend 12/15] drm/i915/dsi: Document always using v3 SHUTDOWN / MIPI_SEQ_DISPLAY_OFF order
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
---
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?
+*/
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);
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
