On Fri, 03 Mar 2017, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Fri, Mar 03, 2017 at 05:01:42AM -0800, Manasi Navare wrote:
>> If during VBT parsing we find that the port is unused,
>> the driver code just bails out without clearing the
>> defaults for that port. This can cause failures down
>> the path through link training for unused Port.
>> This patch fixes this issue by clearing the defaults
>> before bailing out from the VBT parsing function.
>>
>> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
>> Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/intel_bios.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index e144f03..3ac3d24 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1148,9 +1148,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>> }
>> }
>> }
>> - if (!child)
>> - return;
>> + if (!child) {
>> + /* Clear the DDI VBT Port info values */
>> + info->supports_dvi = 0;
>> + info->supports_hdmi = 0;
>> + info->supports_dp = 0;
>> + info->supports_edp = 0;
>
> I would s/0/false/ here. Although they are apparently of type 'uint8_t:1'.
> As a followup someone might want to s/uint8_t:1/bool:1/ the definitions
> so that the compiler will protect us against values >1 when we assign
> these.
>
> Otherwise lgtm. And based on a cursory glance the bogus port A has now
> disappaered from the ci results on fi-skl-6700k, so looks like my
> analysis of the problem was correct.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Bikeshedding after review...
I think the approach here is a bit magical, first setting defaults on
all platforms, then removing same defaults on platforms that a) are DDI,
b) have VBT, c) have a high enough VBT revision, and d) do not have
child devices.
I think I'd go with simplicity, and split out a function from
init_vbt_defaults() to be called on platforms without VBT. It would
initially just contain the loop to initialize
info->supports_dvi = (port != PORT_A && port != PORT_E);
info->supports_hdmi = info->supports_dvi;
info->supports_dp = (port != PORT_E);
BR,
Jani.
>
>>
>> + return;
>> + }
>> aux_channel = child->common.aux_channel;
>> ddc_pin = child->common.ddc_pin;
>>
>> --
>> 2.1.4
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
