Unify slow/combinatorial test handling

New | Submitter: David Weinehall | Reviewer: None | Updated on Oct. 23, 2015, 11:42 a.m. | Series ID: 1140

[Intel-gfx,i-g-t,1/3] Copy gem_concurrent_all to gem_concurrent_blit

We'll both rename gem_concurrent_all over gem_concurrent_blit
and change gem_concurrent_blit in this changeset. To make
this easier to follow we first do the the rename.

Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
  ---
  tests/gem_concurrent_blit.c | 1116 ++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 1108 insertions(+), 8 deletions(-)
        
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e80387dd6582..29ea4c458ab3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14263,7 +14263,7 @@  static bool intel_crt_present(struct drm_device *dev)
 	if (HAS_DDI(dev) && I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
 		return false;

-	if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
+	if ((HAS_DDI(dev) || IS_VALLEYVIEW(dev)) && !dev_priv->vbt.int_crt_support)
 		return false;

 	return true;

          
David Weinehall Oct. 28, 2015, 11:29 a.m.
We'll both rename gem_concurrent_all over gem_concurrent_blit
and change gem_concurrent_blit in this changeset. To make
this easier to follow we first do the the rename.

Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
  ---
  tests/gem_concurrent_blit.c | 1116 ++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 1108 insertions(+), 8 deletions(-)
        
Chris Wilson Dec. 1, 2015, 1:41 p.m.
On Tue, Dec 01, 2015 at 03:08:34PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Unfortunatey there appear to quite a few HSW/BDW machines (eg.
> NUCs, Brix Pro) in the wild with LPT/WPT-H but non-working FDI.
> FDI training fails every single time on these machines. Dunno,
> maybe they just didn't bother wiring it up or something?
> 
> Unfortunately all the fuse bits and whatnot are telling us that
> the CRT connector is present. And so what we get from this is tons
> of false positives from the CI systems due to VGA connector forcing.
> 
> I've not found any way to detect this purely from hardware, so we
> have to resort to looking at the VBT int_crt_support bit. We used
> to check this bit on all platforms, but that broke all the old
> machines, so the check was then restricted to VLV only in
> commit 84b4e042c470 ("drm/i915: only apply crt_present check on VLV")
> 
> Considering HSW and VLV VBT probably got defined around the same time,
> it should be reasonably safe to assume that the bits is sane for
> HSW/BDW as well. At least I have one copy of some VBT spec here that
> says it's meant for both VLV and HSW, and it knows about the bit
> (lists it being valid from version 155 onwards). Also I have two
> HSW desktop machines with actual CRT ports and both have
> int_crt_support==1 in their VBTs.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e80387dd6582..29ea4c458ab3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14263,7 +14263,7 @@ static bool intel_crt_present(struct drm_device *dev)
>  	if (HAS_DDI(dev) && I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
>  		return false;
>  
> -	if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
> +	if ((HAS_DDI(dev) || IS_VALLEYVIEW(dev)) && !dev_priv->vbt.int_crt_support)

Would it not be better to move this knowledge to vbt if it is version
dependent?

	vbt.int_crtc_support = (UNKNOWN, NOT_PRESENT, PRESENT)

then

	if (dev_priv->vbt.int_crt_support == NOT_PRESENT)
>  		return false;
-Chris
        
ville.syrjala@linux.intel.com Dec. 1, 2015, 1:57 p.m.
On Tue, Dec 01, 2015 at 01:41:49PM +0000, Chris Wilson wrote:
> On Tue, Dec 01, 2015 at 03:08:34PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Unfortunatey there appear to quite a few HSW/BDW machines (eg.
> > NUCs, Brix Pro) in the wild with LPT/WPT-H but non-working FDI.
> > FDI training fails every single time on these machines. Dunno,
> > maybe they just didn't bother wiring it up or something?
> > 
> > Unfortunately all the fuse bits and whatnot are telling us that
> > the CRT connector is present. And so what we get from this is tons
> > of false positives from the CI systems due to VGA connector forcing.
> > 
> > I've not found any way to detect this purely from hardware, so we
> > have to resort to looking at the VBT int_crt_support bit. We used
> > to check this bit on all platforms, but that broke all the old
> > machines, so the check was then restricted to VLV only in
> > commit 84b4e042c470 ("drm/i915: only apply crt_present check on VLV")
> > 
> > Considering HSW and VLV VBT probably got defined around the same time,
> > it should be reasonably safe to assume that the bits is sane for
> > HSW/BDW as well. At least I have one copy of some VBT spec here that
> > says it's meant for both VLV and HSW, and it knows about the bit
> > (lists it being valid from version 155 onwards). Also I have two
> > HSW desktop machines with actual CRT ports and both have
> > int_crt_support==1 in their VBTs.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index e80387dd6582..29ea4c458ab3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14263,7 +14263,7 @@ static bool intel_crt_present(struct drm_device *dev)
> >  	if (HAS_DDI(dev) && I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
> >  		return false;
> >  
> > -	if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
> > +	if ((HAS_DDI(dev) || IS_VALLEYVIEW(dev)) && !dev_priv->vbt.int_crt_support)
> 
> Would it not be better to move this knowledge to vbt if it is version
> dependent?
> 
> 	vbt.int_crtc_support = (UNKNOWN, NOT_PRESENT, PRESENT)
> 
> then
> 
> 	if (dev_priv->vbt.int_crt_support == NOT_PRESENT)
> >  		return false;

I'm not sure the UNKNWON value really buys us anything. But I suppose we
could just do this:

        general = find_section(bdb, BDB_GENERAL_FEATURES);
        if (general) {
                dev_priv->vbt.int_tv_support = general->int_tv_support;
-               dev_priv->vbt.int_crt_support = general->int_crt_support;
+               if (HAS_DDI(dev_priv) || IS_VALLEYIEW(dev_priv))
+                       dev_priv->vbt.int_crt_support = general->int_crt_support;

The other option is to make it check the version. But I'm not sure if
that's entirely safe. For extra paranoia I guess we could do both,
just in case there's some early VLV/HSW out there with <155 VBT
version.
        
Chris Wilson Dec. 1, 2015, 2:06 p.m.
On Tue, Dec 01, 2015 at 03:57:37PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 01, 2015 at 01:41:49PM +0000, Chris Wilson wrote:
> > On Tue, Dec 01, 2015 at 03:08:34PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Unfortunatey there appear to quite a few HSW/BDW machines (eg.
> > > NUCs, Brix Pro) in the wild with LPT/WPT-H but non-working FDI.
> > > FDI training fails every single time on these machines. Dunno,
> > > maybe they just didn't bother wiring it up or something?
> > > 
> > > Unfortunately all the fuse bits and whatnot are telling us that
> > > the CRT connector is present. And so what we get from this is tons
> > > of false positives from the CI systems due to VGA connector forcing.
> > > 
> > > I've not found any way to detect this purely from hardware, so we
> > > have to resort to looking at the VBT int_crt_support bit. We used
> > > to check this bit on all platforms, but that broke all the old
> > > machines, so the check was then restricted to VLV only in
> > > commit 84b4e042c470 ("drm/i915: only apply crt_present check on VLV")
> > > 
> > > Considering HSW and VLV VBT probably got defined around the same time,
> > > it should be reasonably safe to assume that the bits is sane for
> > > HSW/BDW as well. At least I have one copy of some VBT spec here that
> > > says it's meant for both VLV and HSW, and it knows about the bit
> > > (lists it being valid from version 155 onwards). Also I have two
> > > HSW desktop machines with actual CRT ports and both have
> > > int_crt_support==1 in their VBTs.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index e80387dd6582..29ea4c458ab3 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -14263,7 +14263,7 @@ static bool intel_crt_present(struct drm_device *dev)
> > >  	if (HAS_DDI(dev) && I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
> > >  		return false;
> > >  
> > > -	if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
> > > +	if ((HAS_DDI(dev) || IS_VALLEYVIEW(dev)) && !dev_priv->vbt.int_crt_support)
> > 
> > Would it not be better to move this knowledge to vbt if it is version
> > dependent?
> > 
> > 	vbt.int_crtc_support = (UNKNOWN, NOT_PRESENT, PRESENT)
> > 
> > then
> > 
> > 	if (dev_priv->vbt.int_crt_support == NOT_PRESENT)
> > >  		return false;
> 
> I'm not sure the UNKNWON value really buys us anything.

Don't you need a flag to say when the .int_crt_support can be trusted?

> But I suppose we
> could just do this:
> 
>         general = find_section(bdb, BDB_GENERAL_FEATURES);
>         if (general) {
>                 dev_priv->vbt.int_tv_support = general->int_tv_support;
> -               dev_priv->vbt.int_crt_support = general->int_crt_support;
> +               if (HAS_DDI(dev_priv) || IS_VALLEYIEW(dev_priv))
> +                       dev_priv->vbt.int_crt_support = general->int_crt_support;
> 
> The other option is to make it check the version. But I'm not sure if
> that's entirely safe. For extra paranoia I guess we could do both,
> just in case there's some early VLV/HSW out there with <155 VBT
> version.

I would, if only to get better documentation on the changes. I also
wouldn't put it past someone to build a new VBIOS with an old tool...
-Chris
        
ville.syrjala@linux.intel.com Dec. 1, 2015, 2:19 p.m.
On Tue, Dec 01, 2015 at 02:06:26PM +0000, Chris Wilson wrote:
> On Tue, Dec 01, 2015 at 03:57:37PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 01, 2015 at 01:41:49PM +0000, Chris Wilson wrote:
> > > On Tue, Dec 01, 2015 at 03:08:34PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Unfortunatey there appear to quite a few HSW/BDW machines (eg.
> > > > NUCs, Brix Pro) in the wild with LPT/WPT-H but non-working FDI.
> > > > FDI training fails every single time on these machines. Dunno,
> > > > maybe they just didn't bother wiring it up or something?
> > > > 
> > > > Unfortunately all the fuse bits and whatnot are telling us that
> > > > the CRT connector is present. And so what we get from this is tons
> > > > of false positives from the CI systems due to VGA connector forcing.
> > > > 
> > > > I've not found any way to detect this purely from hardware, so we
> > > > have to resort to looking at the VBT int_crt_support bit. We used
> > > > to check this bit on all platforms, but that broke all the old
> > > > machines, so the check was then restricted to VLV only in
> > > > commit 84b4e042c470 ("drm/i915: only apply crt_present check on VLV")
> > > > 
> > > > Considering HSW and VLV VBT probably got defined around the same time,
> > > > it should be reasonably safe to assume that the bits is sane for
> > > > HSW/BDW as well. At least I have one copy of some VBT spec here that
> > > > says it's meant for both VLV and HSW, and it knows about the bit
> > > > (lists it being valid from version 155 onwards). Also I have two
> > > > HSW desktop machines with actual CRT ports and both have
> > > > int_crt_support==1 in their VBTs.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index e80387dd6582..29ea4c458ab3 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -14263,7 +14263,7 @@ static bool intel_crt_present(struct drm_device *dev)
> > > >  	if (HAS_DDI(dev) && I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
> > > >  		return false;
> > > >  
> > > > -	if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
> > > > +	if ((HAS_DDI(dev) || IS_VALLEYVIEW(dev)) && !dev_priv->vbt.int_crt_support)
> > > 
> > > Would it not be better to move this knowledge to vbt if it is version
> > > dependent?
> > > 
> > > 	vbt.int_crtc_support = (UNKNOWN, NOT_PRESENT, PRESENT)
> > > 
> > > then
> > > 
> > > 	if (dev_priv->vbt.int_crt_support == NOT_PRESENT)
> > > >  		return false;
> > 
> > I'm not sure the UNKNWON value really buys us anything.
> 
> Don't you need a flag to say when the .int_crt_support can be trusted?

If we can't trust it we have no choice but to register the connetor
(assuming the hw based probes said it's present). So in effect UNKNOWN
would be the same as PRESENT.

> 
> > But I suppose we
> > could just do this:
> > 
> >         general = find_section(bdb, BDB_GENERAL_FEATURES);
> >         if (general) {
> >                 dev_priv->vbt.int_tv_support = general->int_tv_support;
> > -               dev_priv->vbt.int_crt_support = general->int_crt_support;
> > +               if (HAS_DDI(dev_priv) || IS_VALLEYIEW(dev_priv))
> > +                       dev_priv->vbt.int_crt_support = general->int_crt_support;
> > 
> > The other option is to make it check the version. But I'm not sure if
> > that's entirely safe. For extra paranoia I guess we could do both,
> > just in case there's some early VLV/HSW out there with <155 VBT
> > version.
> 
> I would, if only to get better documentation on the changes. I also
> wouldn't put it past someone to build a new VBIOS with an old tool...

I can respin using this approach.
        
URLs
In bundles
PATCH ID
66386
MESSAGE ID
1448975321-20192-4-git-send-email-ville.syrjala@linux.intel.com
EMAIL HEADERS
Return-Path: <intel-gfx-bounces@lists.freedesktop.org>
X-Original-To: patchwork@annarchy.freedesktop.org
Delivered-To: patchwork@annarchy.freedesktop.org
Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177])
	by annarchy.freedesktop.org (Postfix) with ESMTP id E617E18176
	for <patchwork@annarchy.freedesktop.org>;
	Tue,  1 Dec 2015 05:08:54 -0800 (PST)
Received: from gabe.freedesktop.org (localhost [127.0.0.1])
	by gabe.freedesktop.org (Postfix) with ESMTP id 7E81B72126;
	Tue,  1 Dec 2015 05:08:54 -0800 (PST)
X-Original-To: intel-gfx@lists.freedesktop.org
Delivered-To: intel-gfx@lists.freedesktop.org
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
	by gabe.freedesktop.org (Postfix) with ESMTP id 72D5572126
	for <intel-gfx@lists.freedesktop.org>;
	Tue,  1 Dec 2015 05:08:53 -0800 (PST)
Received: from orsmga001.jf.intel.com ([10.7.209.18])
	by fmsmga101.fm.intel.com with ESMTP; 01 Dec 2015 05:08:53 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.20,369,1444719600"; d="scan'208";a="831802654"
Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174])
	by orsmga001.jf.intel.com with SMTP; 01 Dec 2015 05:08:52 -0800
Received: by stinkbox (sSMTP sendmail emulation);
	Tue, 01 Dec 2015 15:08:50 +0200
From: ville.syrjala@linux.intel.com
To: intel-gfx@lists.freedesktop.org
Date: Tue,  1 Dec 2015 15:08:34 +0200
Message-Id: <1448975321-20192-4-git-send-email-ville.syrjala@linux.intel.com>
X-Mailer: git-send-email 2.4.10
In-Reply-To: <1448975321-20192-1-git-send-email-ville.syrjala@linux.intel.com>
References: <1448975321-20192-1-git-send-email-ville.syrjala@linux.intel.com>
MIME-Version: 1.0
Subject: [Intel-gfx] [PATCH 03/10] drm/i915: Check VBT for CRT port presence
	on HSW/BDW
X-BeenThere: intel-gfx@lists.freedesktop.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: Intel graphics driver community testing & development
	<intel-gfx.lists.freedesktop.org>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>