* [PATCH] drm/i915/vbt: ignore extraneous child devices for a port
@ 2017-08-11 11:39 Jani Nikula
2017-08-11 12:01 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jani Nikula @ 2017-08-11 11:39 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, # v4 . 12+, Paulo Zanoni
Ever since we've parsed VBT child devices, starting from 6acab15a7b0d
("drm/i915: use the HDMI DDI buffer translations from VBT"), we've
ignored the child device information if more than one child device
references the same port. The rationale for this seems lost in time.
Since commit 311a20949f04 ("drm/i915: don't init DP or HDMI when not
supported by DDI port") we started using this information more to skip
HDMI/DP init if the port wasn't there per VBT child devices. However, at
the same time it added port defaults without further explanation.
Thus, if the child device info was skipped due to multiple child devices
referencing the same port, the device info would be retrieved from the
somewhat arbitrary defaults.
Finally, when commit bb1d132935c2 ("drm/i915/vbt: split out defaults
that are set when there is no VBT") stopped initializing the defaults
whenever VBT is present, thus trusting the VBT more, we stopped
initializing ports which were referenced by more than one child device.
Apparently at least Asus UX305UA, UX305U, and UX306U laptops have VBT
child device blocks which cause this behaviour. Arguably they were
shipped with a broken VBT.
Relax the rules for multiple references to the same port, and use the
first child device info to reference a port. Retain the logic to debug
log about this, though.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101745
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196233
Fixes: bb1d132935c2 ("drm/i915/vbt: split out defaults that are set when there is no VBT")
Tested-by: Oliver Weißbarth <mail@oweissbarth.de>
Reported-by: Oliver Weißbarth <mail@oweissbarth.de>
Reported-by: Didier G <didierg-divers@orange.fr>
Reported-by: Giles Anderson <agander@gmail.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: <stable@vger.kernel.org> # v4.12+
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_bios.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 82b144cdfa1d..183e87e8ea31 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1120,8 +1120,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
uint8_t aux_channel, ddc_pin;
/* Each DDI port can have more than one value on the "DVO Port" field,
- * so look for all the possible values for each port and abort if more
- * than one is found. */
+ * so look for all the possible values for each port.
+ */
int dvo_ports[][3] = {
{DVO_PORT_HDMIA, DVO_PORT_DPA, -1},
{DVO_PORT_HDMIB, DVO_PORT_DPB, -1},
@@ -1130,7 +1130,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
{DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
};
- /* Find the child device to use, abort if more than one found. */
+ /*
+ * Find the first child device to reference the port, report if more
+ * than one found.
+ */
for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
it = dev_priv->vbt.child_dev + i;
@@ -1140,11 +1143,11 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
if (it->common.dvo_port == dvo_ports[port][j]) {
if (child) {
- DRM_DEBUG_KMS("More than one child device for port %c in VBT.\n",
+ DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n",
port_name(port));
- return;
+ } else {
+ child = it;
}
- child = it;
}
}
}
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/vbt: ignore extraneous child devices for a port
2017-08-11 11:39 [PATCH] drm/i915/vbt: ignore extraneous child devices for a port Jani Nikula
@ 2017-08-11 12:01 ` Patchwork
2017-08-14 23:02 ` [PATCH] " Manasi Navare
2017-08-16 14:33 ` Ville Syrjälä
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-08-11 12:01 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/vbt: ignore extraneous child devices for a port
URL : https://patchwork.freedesktop.org/series/28681/
State : success
== Summary ==
Series 28681v1 drm/i915/vbt: ignore extraneous child devices for a port
https://patchwork.freedesktop.org/api/1.0/series/28681/revisions/1/mbox/
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip -> PASS (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (fi-byt-n2820) fdo#101705
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:451s
fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:440s
fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:355s
fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:550s
fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:515s
fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:520s
fi-byt-n2820 total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:509s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:604s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:444s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:414s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:420s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:518s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:480s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:477s
fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:586s
fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:588s
fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:525s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:461s
fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:473s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:486s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:441s
fi-skl-x1585l total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:501s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:546s
fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:415s
fbb8288699ef622bbfc6e10bdca6773a16f93fac drm-tip: 2017y-08m-11d-09h-03m-47s UTC integration manifest
848d790e03a8 drm/i915/vbt: ignore extraneous child devices for a port
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5381/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vbt: ignore extraneous child devices for a port
2017-08-11 11:39 [PATCH] drm/i915/vbt: ignore extraneous child devices for a port Jani Nikula
2017-08-11 12:01 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-08-14 23:02 ` Manasi Navare
2017-08-15 7:26 ` Jani Nikula
2017-08-16 14:33 ` Ville Syrjälä
2 siblings, 1 reply; 6+ messages in thread
From: Manasi Navare @ 2017-08-14 23:02 UTC (permalink / raw)
To: Jani Nikula; +Cc: # v4 . 12+, intel-gfx, Paulo Zanoni
On Fri, Aug 11, 2017 at 02:39:07PM +0300, Jani Nikula wrote:
> Ever since we've parsed VBT child devices, starting from 6acab15a7b0d
> ("drm/i915: use the HDMI DDI buffer translations from VBT"), we've
> ignored the child device information if more than one child device
> references the same port. The rationale for this seems lost in time.
>
> Since commit 311a20949f04 ("drm/i915: don't init DP or HDMI when not
> supported by DDI port") we started using this information more to skip
> HDMI/DP init if the port wasn't there per VBT child devices. However, at
> the same time it added port defaults without further explanation.
>
> Thus, if the child device info was skipped due to multiple child devices
> referencing the same port, the device info would be retrieved from the
> somewhat arbitrary defaults.
>
> Finally, when commit bb1d132935c2 ("drm/i915/vbt: split out defaults
> that are set when there is no VBT") stopped initializing the defaults
> whenever VBT is present, thus trusting the VBT more, we stopped
> initializing ports which were referenced by more than one child device.
>
> Apparently at least Asus UX305UA, UX305U, and UX306U laptops have VBT
> child device blocks which cause this behaviour. Arguably they were
> shipped with a broken VBT.
>
> Relax the rules for multiple references to the same port, and use the
> first child device info to reference a port. Retain the logic to debug
> log about this, though.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101745
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196233
> Fixes: bb1d132935c2 ("drm/i915/vbt: split out defaults that are set when there is no VBT")
> Tested-by: Oliver Weißbarth <mail@oweissbarth.de>
> Reported-by: Oliver Weißbarth <mail@oweissbarth.de>
> Reported-by: Didier G <didierg-divers@orange.fr>
> Reported-by: Giles Anderson <agander@gmail.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: <stable@vger.kernel.org> # v4.12+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_bios.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 82b144cdfa1d..183e87e8ea31 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1120,8 +1120,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> uint8_t aux_channel, ddc_pin;
> /* Each DDI port can have more than one value on the "DVO Port" field,
> - * so look for all the possible values for each port and abort if more
> - * than one is found. */
> + * so look for all the possible values for each port.
> + */
> int dvo_ports[][3] = {
> {DVO_PORT_HDMIA, DVO_PORT_DPA, -1},
> {DVO_PORT_HDMIB, DVO_PORT_DPB, -1},
> @@ -1130,7 +1130,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
> };
>
> - /* Find the child device to use, abort if more than one found. */
> + /*
> + * Find the first child device to reference the port, report if more
> + * than one found.
> + */
> for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> it = dev_priv->vbt.child_dev + i;
>
> @@ -1140,11 +1143,11 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>
> if (it->common.dvo_port == dvo_ports[port][j]) {
> if (child) {
> - DRM_DEBUG_KMS("More than one child device for port %c in VBT.\n",
> + DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n",
> port_name(port));
> - return;
So the bug here was that in case of port referenced by multiple child devices because it would return from the function,
it would skip the initialization of flags like is_dp/is_hdmi?
It almost feels like they meant to have a break; here instead of return.
Now that this patch removes return it will still iterate through all the chile devices even
after finding second refernce to the same port, isnt that unnecessary?
Would adding a break there instead optimize it?
Regards
Manasi
> + } else {
> + child = it;
> }
> - child = it;
> }
> }
> }
> --
> 2.11.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vbt: ignore extraneous child devices for a port
2017-08-14 23:02 ` [PATCH] " Manasi Navare
@ 2017-08-15 7:26 ` Jani Nikula
0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2017-08-15 7:26 UTC (permalink / raw)
To: Manasi Navare
Cc: intel-gfx, Ville Syrjälä, Paulo Zanoni, # v4 . 12+
On Mon, 14 Aug 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> On Fri, Aug 11, 2017 at 02:39:07PM +0300, Jani Nikula wrote:
>> Ever since we've parsed VBT child devices, starting from 6acab15a7b0d
>> ("drm/i915: use the HDMI DDI buffer translations from VBT"), we've
>> ignored the child device information if more than one child device
>> references the same port. The rationale for this seems lost in time.
>>
>> Since commit 311a20949f04 ("drm/i915: don't init DP or HDMI when not
>> supported by DDI port") we started using this information more to skip
>> HDMI/DP init if the port wasn't there per VBT child devices. However, at
>> the same time it added port defaults without further explanation.
>>
>> Thus, if the child device info was skipped due to multiple child devices
>> referencing the same port, the device info would be retrieved from the
>> somewhat arbitrary defaults.
>>
>> Finally, when commit bb1d132935c2 ("drm/i915/vbt: split out defaults
>> that are set when there is no VBT") stopped initializing the defaults
>> whenever VBT is present, thus trusting the VBT more, we stopped
>> initializing ports which were referenced by more than one child device.
>>
>> Apparently at least Asus UX305UA, UX305U, and UX306U laptops have VBT
>> child device blocks which cause this behaviour. Arguably they were
>> shipped with a broken VBT.
>>
>> Relax the rules for multiple references to the same port, and use the
>> first child device info to reference a port. Retain the logic to debug
>> log about this, though.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101745
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196233
>> Fixes: bb1d132935c2 ("drm/i915/vbt: split out defaults that are set when there is no VBT")
>> Tested-by: Oliver Weißbarth <mail@oweissbarth.de>
>> Reported-by: Oliver Weißbarth <mail@oweissbarth.de>
>> Reported-by: Didier G <didierg-divers@orange.fr>
>> Reported-by: Giles Anderson <agander@gmail.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: <stable@vger.kernel.org> # v4.12+
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_bios.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 82b144cdfa1d..183e87e8ea31 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1120,8 +1120,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>> bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
>> uint8_t aux_channel, ddc_pin;
>> /* Each DDI port can have more than one value on the "DVO Port" field,
>> - * so look for all the possible values for each port and abort if more
>> - * than one is found. */
>> + * so look for all the possible values for each port.
>> + */
>> int dvo_ports[][3] = {
>> {DVO_PORT_HDMIA, DVO_PORT_DPA, -1},
>> {DVO_PORT_HDMIB, DVO_PORT_DPB, -1},
>> @@ -1130,7 +1130,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>> {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
>> };
>>
>> - /* Find the child device to use, abort if more than one found. */
>> + /*
>> + * Find the first child device to reference the port, report if more
>> + * than one found.
>> + */
>> for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> it = dev_priv->vbt.child_dev + i;
>>
>> @@ -1140,11 +1143,11 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>>
>> if (it->common.dvo_port == dvo_ports[port][j]) {
>> if (child) {
>> - DRM_DEBUG_KMS("More than one child device for port %c in VBT.\n",
>> + DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n",
>> port_name(port));
>> - return;
>
> So the bug here was that in case of port referenced by multiple child devices because it would return from the function,
> it would skip the initialization of flags like is_dp/is_hdmi?
> It almost feels like they meant to have a break; here instead of return.
> Now that this patch removes return it will still iterate through all the chile devices even
> after finding second refernce to the same port, isnt that unnecessary?
> Would adding a break there instead optimize it?
The commit message explicitly says I want to retain the behaviour of
flagging the duplicates. There is nothing particularly slow about the
loop that needs optimization; debugging machines out the there in the
wild is the slow part eating developer time.
BR,
Jani.
>
> Regards
> Manasi
>
>> + } else {
>> + child = it;
>> }
>> - child = it;
>> }
>> }
>> }
>> --
>> 2.11.0
>>
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vbt: ignore extraneous child devices for a port
2017-08-11 11:39 [PATCH] drm/i915/vbt: ignore extraneous child devices for a port Jani Nikula
2017-08-11 12:01 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-14 23:02 ` [PATCH] " Manasi Navare
@ 2017-08-16 14:33 ` Ville Syrjälä
2017-08-16 14:46 ` Jani Nikula
2 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2017-08-16 14:33 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Paulo Zanoni, # v4 . 12+
On Fri, Aug 11, 2017 at 02:39:07PM +0300, Jani Nikula wrote:
> Ever since we've parsed VBT child devices, starting from 6acab15a7b0d
> ("drm/i915: use the HDMI DDI buffer translations from VBT"), we've
> ignored the child device information if more than one child device
> references the same port. The rationale for this seems lost in time.
>
> Since commit 311a20949f04 ("drm/i915: don't init DP or HDMI when not
> supported by DDI port") we started using this information more to skip
> HDMI/DP init if the port wasn't there per VBT child devices. However, at
> the same time it added port defaults without further explanation.
>
> Thus, if the child device info was skipped due to multiple child devices
> referencing the same port, the device info would be retrieved from the
> somewhat arbitrary defaults.
>
> Finally, when commit bb1d132935c2 ("drm/i915/vbt: split out defaults
> that are set when there is no VBT") stopped initializing the defaults
> whenever VBT is present, thus trusting the VBT more, we stopped
> initializing ports which were referenced by more than one child device.
>
> Apparently at least Asus UX305UA, UX305U, and UX306U laptops have VBT
> child device blocks which cause this behaviour. Arguably they were
> shipped with a broken VBT.
>
> Relax the rules for multiple references to the same port, and use the
> first child device info to reference a port. Retain the logic to debug
> log about this, though.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101745
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196233
> Fixes: bb1d132935c2 ("drm/i915/vbt: split out defaults that are set when there is no VBT")
> Tested-by: Oliver Weißbarth <mail@oweissbarth.de>
> Reported-by: Oliver Weißbarth <mail@oweissbarth.de>
> Reported-by: Didier G <didierg-divers@orange.fr>
> Reported-by: Giles Anderson <agander@gmail.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: <stable@vger.kernel.org> # v4.12+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_bios.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 82b144cdfa1d..183e87e8ea31 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1120,8 +1120,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> uint8_t aux_channel, ddc_pin;
> /* Each DDI port can have more than one value on the "DVO Port" field,
> - * so look for all the possible values for each port and abort if more
> - * than one is found. */
> + * so look for all the possible values for each port.
> + */
> int dvo_ports[][3] = {
> {DVO_PORT_HDMIA, DVO_PORT_DPA, -1},
So at least one of the machines has dvo_port==HDMI-A in the VBT
alongside DP-A. HDMI-A is not really legal on any platform IIRC, so we
might want to reject it outright. But on the other hand, we've seen a
lot of VBTs that make a real mess of the dvo_port type vs. the device_type
bits, and thus we often consider both HDMI and DP dvo_ports to be valid
for either port type. So I guess it might be possible that there are VBTs
out there that rely on HDMI-A being picked up.
Either way the patch seems about as sane as anything else relating to VBT so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> {DVO_PORT_HDMIB, DVO_PORT_DPB, -1},
> @@ -1130,7 +1130,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
> };
>
> - /* Find the child device to use, abort if more than one found. */
> + /*
> + * Find the first child device to reference the port, report if more
> + * than one found.
> + */
> for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> it = dev_priv->vbt.child_dev + i;
>
> @@ -1140,11 +1143,11 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>
> if (it->common.dvo_port == dvo_ports[port][j]) {
> if (child) {
> - DRM_DEBUG_KMS("More than one child device for port %c in VBT.\n",
> + DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n",
> port_name(port));
> - return;
> + } else {
> + child = it;
> }
> - child = it;
> }
> }
> }
> --
> 2.11.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vbt: ignore extraneous child devices for a port
2017-08-16 14:33 ` Ville Syrjälä
@ 2017-08-16 14:46 ` Jani Nikula
0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2017-08-16 14:46 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-gfx, Manasi Navare, Paulo Zanoni, # v4 . 12+
On Wed, 16 Aug 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Aug 11, 2017 at 02:39:07PM +0300, Jani Nikula wrote:
>> Ever since we've parsed VBT child devices, starting from 6acab15a7b0d
>> ("drm/i915: use the HDMI DDI buffer translations from VBT"), we've
>> ignored the child device information if more than one child device
>> references the same port. The rationale for this seems lost in time.
>>
>> Since commit 311a20949f04 ("drm/i915: don't init DP or HDMI when not
>> supported by DDI port") we started using this information more to skip
>> HDMI/DP init if the port wasn't there per VBT child devices. However, at
>> the same time it added port defaults without further explanation.
>>
>> Thus, if the child device info was skipped due to multiple child devices
>> referencing the same port, the device info would be retrieved from the
>> somewhat arbitrary defaults.
>>
>> Finally, when commit bb1d132935c2 ("drm/i915/vbt: split out defaults
>> that are set when there is no VBT") stopped initializing the defaults
>> whenever VBT is present, thus trusting the VBT more, we stopped
>> initializing ports which were referenced by more than one child device.
>>
>> Apparently at least Asus UX305UA, UX305U, and UX306U laptops have VBT
>> child device blocks which cause this behaviour. Arguably they were
>> shipped with a broken VBT.
>>
>> Relax the rules for multiple references to the same port, and use the
>> first child device info to reference a port. Retain the logic to debug
>> log about this, though.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101745
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196233
>> Fixes: bb1d132935c2 ("drm/i915/vbt: split out defaults that are set when there is no VBT")
>> Tested-by: Oliver Weißbarth <mail@oweissbarth.de>
>> Reported-by: Oliver Weißbarth <mail@oweissbarth.de>
>> Reported-by: Didier G <didierg-divers@orange.fr>
>> Reported-by: Giles Anderson <agander@gmail.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: <stable@vger.kernel.org> # v4.12+
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_bios.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 82b144cdfa1d..183e87e8ea31 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1120,8 +1120,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>> bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
>> uint8_t aux_channel, ddc_pin;
>> /* Each DDI port can have more than one value on the "DVO Port" field,
>> - * so look for all the possible values for each port and abort if more
>> - * than one is found. */
>> + * so look for all the possible values for each port.
>> + */
>> int dvo_ports[][3] = {
>> {DVO_PORT_HDMIA, DVO_PORT_DPA, -1},
>
> So at least one of the machines has dvo_port==HDMI-A in the VBT
> alongside DP-A. HDMI-A is not really legal on any platform IIRC, so we
> might want to reject it outright. But on the other hand, we've seen a
> lot of VBTs that make a real mess of the dvo_port type vs. the device_type
> bits, and thus we often consider both HDMI and DP dvo_ports to be valid
> for either port type. So I guess it might be possible that there are VBTs
> out there that rely on HDMI-A being picked up.
>
> Either way the patch seems about as sane as anything else relating to VBT so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks, pushed to drm-intel-next-queued.
BR,
Jani.
>
>> {DVO_PORT_HDMIB, DVO_PORT_DPB, -1},
>> @@ -1130,7 +1130,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>> {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
>> };
>>
>> - /* Find the child device to use, abort if more than one found. */
>> + /*
>> + * Find the first child device to reference the port, report if more
>> + * than one found.
>> + */
>> for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> it = dev_priv->vbt.child_dev + i;
>>
>> @@ -1140,11 +1143,11 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>>
>> if (it->common.dvo_port == dvo_ports[port][j]) {
>> if (child) {
>> - DRM_DEBUG_KMS("More than one child device for port %c in VBT.\n",
>> + DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n",
>> port_name(port));
>> - return;
>> + } else {
>> + child = it;
>> }
>> - child = it;
>> }
>> }
>> }
>> --
>> 2.11.0
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-16 14:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-11 11:39 [PATCH] drm/i915/vbt: ignore extraneous child devices for a port Jani Nikula
2017-08-11 12:01 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-14 23:02 ` [PATCH] " Manasi Navare
2017-08-15 7:26 ` Jani Nikula
2017-08-16 14:33 ` Ville Syrjälä
2017-08-16 14:46 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox