[PATCH] drm/i915: Clear the VBT defaults for unused ports

[PATCH] drm/i915: Clear the VBT defaults for unused ports

[Manasi Navare]

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@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 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;
 
+		return;
+	}
 	aux_channel = child->common.aux_channel;
 	ddc_pin = child->common.ddc_pin;
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915: Clear the VBT defaults for unused ports

[Patchwork]

== Series Details ==

Series: drm/i915: Clear the VBT defaults for unused ports
URL   : https://patchwork.freedesktop.org/series/20619/
State : warning

== Summary ==

Series 20619v1 drm/i915: Clear the VBT defaults for unused ports
https://patchwork.freedesktop.org/api/1.0/series/20619/revisions/1/mbox/

Test gem_exec_fence:
        Subgroup await-hang-default:
                incomplete -> PASS       (fi-hsw-4770) fdo#99726
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-bxt-t5700)

fdo#99726 https://bugs.freedesktop.org/show_bug.cgi?id=99726

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:278  pass:257  dwarn:1   dfail:0   fail:0   skip:20 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 

740d0bc8790801dad0f99ea437183542fc7861be drm-tip: 2017y-03m-03d-13h-27m-37s UTC integration manifest
c6b4ac2 drm/i915: Clear the VBT defaults for unused ports

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4054/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Clear the VBT defaults for unused ports

[Ville Syrjälä]

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@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  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@linux.intel.com>

>  
> +		return;
> +	}
>  	aux_channel = child->common.aux_channel;
>  	ddc_pin = child->common.ddc_pin;
>  
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Clear the VBT defaults for unused ports

[Jani Nikula]

On Fri, 03 Mar 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> 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@linux.intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
>> ---
>>  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@linux.intel.com>

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Clear the VBT defaults for unused ports

[Manasi Navare]

On Tue, Mar 07, 2017 at 07:36:45PM +0200, Jani Nikula wrote:
> On Fri, 03 Mar 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> 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@linux.intel.com>
> >> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> >> ---
> >>  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@linux.intel.com>
> 
> 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.
>

How do we determine if the platforms have VBT or not?
So then in case of platforms with VBT, do not initialize them at all??

Regards
Manasi

 
> 
> >
> >>  
> >> +		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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx