[PATCH 00/10] HSW/BDW PCH modeset fixes and stuff

[PATCH 00/10] HSW/BDW PCH modeset fixes and stuff

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I was trying to get to the bottom of the FDI fails on Brix Pro, and
thus eneded up going through the entire PCH modeset stuff for HSW.
While I did find a bunch of stuff missing/wrong, sadly I wasn't able
to make FDI training succeed on that machine. So in the end I simply
had to resort to getting rid of the CRT connector. And to add insult
to injury, the only way I could do that is to look at the VBT :(

Anwyay, here are the fruits of my labor. The whole thing is available
here:
git://github.com/vsyrjala/linux.git hsw_pch_fixes

Ville Syrjälä (10):
  drm/i915: Don't register the CRT connector when it's fused off
  drm/i915: Don't register CRT connectro when DDI E can't be used
  drm/i915: Check VBT for CRT port presence on HSW/BDW
  drm/i915: Add "missing" break to haswell_get_ddi_pll()
  drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed
  drm/i915: Round to closest when computing the VGA dotclock for LPT-H
  drm/i915: Disable FDI after the CRT port on LPT-H
  drm/i915: Refactor LPT-H VGA dotclock disabling
  drm/i915: Disable LPT-H VGA dotclock during crtc disable
  drm/i915: Leave FDI running after failed link training on LPT-H

 drivers/gpu/drm/i915/i915_reg.h      |   4 +-
 drivers/gpu/drm/i915/intel_ddi.c     |  24 ++++---
 drivers/gpu/drm/i915/intel_display.c | 123 ++++++++++++++++++++++++++++-------
 3 files changed, 117 insertions(+), 34 deletions(-)

-- 
2.4.10

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

[PATCH 01/10] drm/i915: Don't register the CRT connector when it's fused off

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

LPT-H has a strap bit for fused off CRT block. Check it to see if
we should register the CRT connector or not. Supposedly this also
forces the ADAP enable bit to 0, so the detection we added in
commit 6c03a6bd0dd8 ("drm/i915: Don't register CRT connector when it's fused off")
should already catch it, but checking the fuse bit should at least
do no harm.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 1 +
 drivers/gpu/drm/i915/intel_display.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 487224572022..6d7ac192982d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7549,6 +7549,7 @@ enum skl_disp_power_wells {
 #define SFUSE_STRAP			_MMIO(0xc2014)
 #define  SFUSE_STRAP_FUSE_LOCK		(1<<13)
 #define  SFUSE_STRAP_DISPLAY_DISABLED	(1<<7)
+#define  SFUSE_STRAP_CRT_DISABLED	(1<<6)
 #define  SFUSE_STRAP_DDIB_DETECTED	(1<<2)
 #define  SFUSE_STRAP_DDIC_DETECTED	(1<<1)
 #define  SFUSE_STRAP_DDID_DETECTED	(1<<0)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4ae490dfe2c4..c3d5a7319ef4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14256,6 +14256,9 @@ static bool intel_crt_present(struct drm_device *dev)
 	if (IS_CHERRYVIEW(dev))
 		return false;
 
+	if (HAS_PCH_LPT(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED)
+		return false;
+
 	if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
 		return false;
 
-- 
2.4.10

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

Re: [PATCH 01/10] drm/i915: Don't register the CRT connector when it's fused off

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> LPT-H has a strap bit for fused off CRT block. Check it to see if
> we should register the CRT connector or not. Supposedly this also
> forces the ADAP enable bit to 0, so the detection we added in
> commit 6c03a6bd0dd8 ("drm/i915: Don't register CRT connector when it's fused off")

The interesting thing is that the commit title of 6c03a6 is almost the
same as this one, the only difference being the word "the" :).

> should already catch it, but checking the fuse bit should at least
> do no harm.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 1 +
>  drivers/gpu/drm/i915/intel_display.c | 3 +++
>  2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 487224572022..6d7ac192982d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7549,6 +7549,7 @@ enum skl_disp_power_wells {
>  #define SFUSE_STRAP                    _MMIO(0xc2014)
>  #define  SFUSE_STRAP_FUSE_LOCK         (1<<13)
>  #define  SFUSE_STRAP_DISPLAY_DISABLED  (1<<7)
> +#define  SFUSE_STRAP_CRT_DISABLED      (1<<6)
>  #define  SFUSE_STRAP_DDIB_DETECTED     (1<<2)
>  #define  SFUSE_STRAP_DDIC_DETECTED     (1<<1)
>  #define  SFUSE_STRAP_DDID_DETECTED     (1<<0)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4ae490dfe2c4..c3d5a7319ef4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14256,6 +14256,9 @@ static bool intel_crt_present(struct drm_device *dev)
>         if (IS_CHERRYVIEW(dev))
>                 return false;
>
> +       if (HAS_PCH_LPT(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED)

Bonus points if you use your new HAS_PCH_LPT_H() here.

With or without that:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> +               return false;
> +
>         if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
>                 return false;
>
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 01/10] drm/i915: Don't register the CRT connector when it's fused off

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 05:05:42PM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > LPT-H has a strap bit for fused off CRT block. Check it to see if
> > we should register the CRT connector or not. Supposedly this also
> > forces the ADAP enable bit to 0, so the detection we added in
> > commit 6c03a6bd0dd8 ("drm/i915: Don't register CRT connector when it's fused off")
> 
> The interesting thing is that the commit title of 6c03a6 is almost the
> same as this one, the only difference being the word "the" :).

Demonstrates a lack of imagination on the author's part?

> 
> > should already catch it, but checking the fuse bit should at least
> > do no harm.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      | 1 +
> >  drivers/gpu/drm/i915/intel_display.c | 3 +++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 487224572022..6d7ac192982d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7549,6 +7549,7 @@ enum skl_disp_power_wells {
> >  #define SFUSE_STRAP                    _MMIO(0xc2014)
> >  #define  SFUSE_STRAP_FUSE_LOCK         (1<<13)
> >  #define  SFUSE_STRAP_DISPLAY_DISABLED  (1<<7)
> > +#define  SFUSE_STRAP_CRT_DISABLED      (1<<6)
> >  #define  SFUSE_STRAP_DDIB_DETECTED     (1<<2)
> >  #define  SFUSE_STRAP_DDIC_DETECTED     (1<<1)
> >  #define  SFUSE_STRAP_DDID_DETECTED     (1<<0)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 4ae490dfe2c4..c3d5a7319ef4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14256,6 +14256,9 @@ static bool intel_crt_present(struct drm_device *dev)
> >         if (IS_CHERRYVIEW(dev))
> >                 return false;
> >
> > +       if (HAS_PCH_LPT(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED)
> 
> Bonus points if you use your new HAS_PCH_LPT_H() here.

I can. Though we already reject ULT/ULX machines earlier, so anything
that reaches this will be -H anyway.

> 
> With or without that:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> > +               return false;
> > +
> >         if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
> >                 return false;
> >
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

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

[PATCH v2 01/10] drm/i915: Don't register the CRT connector when it's fused off on LPT-H

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

LPT-H has a strap bit for fused off CRT block. Check it to see if
we should register the CRT connector or not. Supposedly this also
forces the ADAP enable bit to 0, so the detection we added in
commit 6c03a6bd0dd8 ("drm/i915: Don't register CRT connector when it's fused off")
should already catch it, but checking the fuse bit should at least
do no harm.

v2: Use HAS_PCH_LPT_H() (Paulo)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 1 +
 drivers/gpu/drm/i915/intel_display.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1a12d44b9710..dd02ef587159 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7549,6 +7549,7 @@ enum skl_disp_power_wells {
 #define SFUSE_STRAP			_MMIO(0xc2014)
 #define  SFUSE_STRAP_FUSE_LOCK		(1<<13)
 #define  SFUSE_STRAP_DISPLAY_DISABLED	(1<<7)
+#define  SFUSE_STRAP_CRT_DISABLED	(1<<6)
 #define  SFUSE_STRAP_DDIB_DETECTED	(1<<2)
 #define  SFUSE_STRAP_DDIC_DETECTED	(1<<1)
 #define  SFUSE_STRAP_DDID_DETECTED	(1<<0)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4aa4cadeaf31..bfb00e383587 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14257,6 +14257,9 @@ static bool intel_crt_present(struct drm_device *dev)
 	if (IS_CHERRYVIEW(dev))
 		return false;
 
+	if (HAS_PCH_LPT_H(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED)
+		return false;
+
 	if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
 		return false;
 
-- 
2.4.10

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

[PATCH 02/10] drm/i915: Don't register CRT connectro when DDI E can't be used

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On HSW/BDW DDI A and E share 2 lanes, so when DDI A requires the
shared lanes DDI E can't be used. The lanes are not suppsoed to
be dynamically switched between the two uses, so there's no point
in registering the CRT connector when DDI E has no lanes.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c3d5a7319ef4..e80387dd6582 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14259,6 +14259,10 @@ static bool intel_crt_present(struct drm_device *dev)
 	if (HAS_PCH_LPT(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED)
 		return false;
 
+	/* DDI E can't be used if DDI A requires 4 lanes */
+	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)
 		return false;
 
-- 
2.4.10

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

Re: [PATCH 02/10] drm/i915: Don't register CRT connectro when DDI E can't be used

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Subject: s/connectro/connector/

>
> On HSW/BDW DDI A and E share 2 lanes, so when DDI A requires the
> shared lanes DDI E can't be used. The lanes are not suppsoed to

s/suppsoed/supposed/

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> be dynamically switched between the two uses, so there's no point
> in registering the CRT connector when DDI E has no lanes.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c3d5a7319ef4..e80387dd6582 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14259,6 +14259,10 @@ static bool intel_crt_present(struct drm_device *dev)
>         if (HAS_PCH_LPT(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED)
>                 return false;
>
> +       /* DDI E can't be used if DDI A requires 4 lanes */
> +       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)
>                 return false;
>
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

[PATCH v2 02/10] drm/i915: Don't register CRT connector when DDI E can't be used

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On HSW/BDW DDI A and E share 2 lanes, so when DDI A requires the
shared lanes DDI E can't be used. The lanes are not supposed to
be dynamically switched between the two uses, so there's no point
in registering the CRT connector when DDI E has no lanes.

v2: Fix typos in the commit message (Paulo)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bfb00e383587..0ce988af1300 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14260,6 +14260,10 @@ static bool intel_crt_present(struct drm_device *dev)
 	if (HAS_PCH_LPT_H(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED)
 		return false;
 
+	/* DDI E can't be used if DDI A requires 4 lanes */
+	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)
 		return false;
 
-- 
2.4.10

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

[PATCH 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[ville.syrjala]

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)
 		return false;
 
 	return true;
-- 
2.4.10

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

Re: [PATCH 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[Ville Syrjälä]

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.

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

Re: [PATCH 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[Ville Syrjälä]

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.

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

[PATCH v2 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[ville.syrjala]

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
desktop machines with actual CRT ports and both have
int_crt_support==1 in their VBTs.

v2: Move the platform checks into the VBT parsing code
    Also check that the VBT version is at least 155

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c    | 5 ++++-
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index ce82f9c7df24..070470fe9a91 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -356,7 +356,10 @@ parse_general_features(struct drm_i915_private *dev_priv,
 	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;
+		/* int_crt_support can't be trusted on earlier platforms */
+		if (bdb->version >= 155 &&
+		    (HAS_DDI(dev_priv) || IS_VALLEYVIEW(dev_priv)))
+			dev_priv->vbt.int_crt_support = general->int_crt_support;
 		dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
 		dev_priv->vbt.lvds_ssc_freq =
 			intel_bios_ssc_frequency(dev, general->ssc_freq);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e80387dd6582..68822395914b 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 (!dev_priv->vbt.int_crt_support)
 		return false;
 
 	return true;
-- 
2.4.10

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

Re: [PATCH v2 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 06:07:09PM +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
> desktop machines with actual CRT ports and both have
> int_crt_support==1 in their VBTs.
> 
> v2: Move the platform checks into the VBT parsing code
>     Also check that the VBT version is at least 155
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93190

> ---
>  drivers/gpu/drm/i915/intel_bios.c    | 5 ++++-
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index ce82f9c7df24..070470fe9a91 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -356,7 +356,10 @@ parse_general_features(struct drm_i915_private *dev_priv,
>  	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;
> +		/* int_crt_support can't be trusted on earlier platforms */
> +		if (bdb->version >= 155 &&
> +		    (HAS_DDI(dev_priv) || IS_VALLEYVIEW(dev_priv)))
> +			dev_priv->vbt.int_crt_support = general->int_crt_support;
>  		dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
>  		dev_priv->vbt.lvds_ssc_freq =
>  			intel_bios_ssc_frequency(dev, general->ssc_freq);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e80387dd6582..68822395914b 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 (!dev_priv->vbt.int_crt_support)
>  		return false;
>  
>  	return true;
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v2 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[Paulo Zanoni]

2015-12-01 14:07 GMT-02:00  <ville.syrjala@linux.intel.com>:
> 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?

I know you clarified this on IRC, but just a little nitpick: when
reading this, it was not very clear to me whether these machines had
an actual physical CRT connector or not. If you read this while
assuming the machine does have a CRT connector, the patch would sound
like "we fail to properly train FDI, so let's give up and not register
the CRT connector at all", which is not our case.

>
> 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
> desktop machines with actual CRT ports and both have
> int_crt_support==1 in their VBTs.
>
> v2: Move the platform checks into the VBT parsing code
>     Also check that the VBT version is at least 155

You could also add that HAS_DDI platforms are already believing the
VBT for versions >= 155: see intel_ddi_init(), so we have a precedent.

(by the way, I'm trusting you that VLV VBT versions are always >= 155,
I couldn't check this)

With or without any changes:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c    | 5 ++++-
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index ce82f9c7df24..070470fe9a91 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -356,7 +356,10 @@ parse_general_features(struct drm_i915_private *dev_priv,
>         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;
> +               /* int_crt_support can't be trusted on earlier platforms */
> +               if (bdb->version >= 155 &&
> +                   (HAS_DDI(dev_priv) || IS_VALLEYVIEW(dev_priv)))
> +                       dev_priv->vbt.int_crt_support = general->int_crt_support;
>                 dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
>                 dev_priv->vbt.lvds_ssc_freq =
>                         intel_bios_ssc_frequency(dev, general->ssc_freq);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e80387dd6582..68822395914b 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 (!dev_priv->vbt.int_crt_support)
>                 return false;
>
>         return true;
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH v2 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 05:28:19PM -0200, Paulo Zanoni wrote:
> 2015-12-01 14:07 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > 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?
> 
> I know you clarified this on IRC, but just a little nitpick: when
> reading this, it was not very clear to me whether these machines had
> an actual physical CRT connector or not. If you read this while
> assuming the machine does have a CRT connector, the patch would sound
> like "we fail to properly train FDI, so let's give up and not register
> the CRT connector at all", which is not our case.

I'll try revise a bit.

> 
> >
> > 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
> > desktop machines with actual CRT ports and both have
> > int_crt_support==1 in their VBTs.
> >
> > v2: Move the platform checks into the VBT parsing code
> >     Also check that the VBT version is at least 155
> 
> You could also add that HAS_DDI platforms are already believing the
> VBT for versions >= 155: see intel_ddi_init(), so we have a precedent.

Can do.

> 
> (by the way, I'm trusting you that VLV VBT versions are always >= 155,
> I couldn't check this)

It is at least on the VLV I have on my desk. But even if it wasn't, I
don't think things should break. We live with shadow VGA connectors on
many platforms, and I don't think VLV would be too upset by one. IIRC
it was mainly a speed optimization to get rid of it.

> 
> With or without any changes:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_bios.c    | 5 ++++-
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index ce82f9c7df24..070470fe9a91 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -356,7 +356,10 @@ parse_general_features(struct drm_i915_private *dev_priv,
> >         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;
> > +               /* int_crt_support can't be trusted on earlier platforms */
> > +               if (bdb->version >= 155 &&
> > +                   (HAS_DDI(dev_priv) || IS_VALLEYVIEW(dev_priv)))
> > +                       dev_priv->vbt.int_crt_support = general->int_crt_support;
> >                 dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
> >                 dev_priv->vbt.lvds_ssc_freq =
> >                         intel_bios_ssc_frequency(dev, general->ssc_freq);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index e80387dd6582..68822395914b 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 (!dev_priv->vbt.int_crt_support)
> >                 return false;
> >
> >         return true;
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

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

[PATCH v3 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[ville.syrjala]

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 that have no physical
CRT connector and 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
desktop machines with actual CRT ports and both have
int_crt_support==1 in their VBTs.

Also we already trust VBT >= 155 to tell us various details about
the DDI ports, so trusting it a bit more seems reasonable.

As far as VLV goes, the added VBT version check should be fine. Even
if someone has some weird VLV machine with a very old VBT version,
it just means they'll end up with a shadow CRT connector. IIRC the
reason for eliminating the shadow CRT connector on VLV was to speed
up display probing rather than fixing something more serious.

v2: Move the platform checks into the VBT parsing code
    Also check that the VBT version is at least 155
v3: Improve commit message (Paulo)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c    | 5 ++++-
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index ce82f9c7df24..070470fe9a91 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -356,7 +356,10 @@ parse_general_features(struct drm_i915_private *dev_priv,
 	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;
+		/* int_crt_support can't be trusted on earlier platforms */
+		if (bdb->version >= 155 &&
+		    (HAS_DDI(dev_priv) || IS_VALLEYVIEW(dev_priv)))
+			dev_priv->vbt.int_crt_support = general->int_crt_support;
 		dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
 		dev_priv->vbt.lvds_ssc_freq =
 			intel_bios_ssc_frequency(dev, general->ssc_freq);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0ce988af1300..9f8ab09eb411 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14264,7 +14264,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 (!dev_priv->vbt.int_crt_support)
 		return false;
 
 	return true;
-- 
2.4.10

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

[PATCH 04/10] drm/i915: Add "missing" break to haswell_get_ddi_pll()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While not techically needed on the last case in the switch statement,
the 'break' makes it look better IMO.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 29ea4c458ab3..d049b087e8e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9808,6 +9808,7 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 		break;
 	case PORT_CLK_SEL_SPLL:
 		pipe_config->shared_dpll = DPLL_ID_SPLL;
+		break;
 	}
 }
 
-- 
2.4.10

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

Re: [PATCH 04/10] drm/i915: Add "missing" break to haswell_get_ddi_pll()

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> While not techically needed on the last case in the switch statement,

"techically"

> the 'break' makes it look better IMO.

Just out of curiosity: what's your opinion on the lack of a "break" at
the default case, such as the one we have in bxt_get_ddi_pll()?

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 29ea4c458ab3..d049b087e8e6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9808,6 +9808,7 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
>                 break;
>         case PORT_CLK_SEL_SPLL:
>                 pipe_config->shared_dpll = DPLL_ID_SPLL;
> +               break;
>         }
>  }
>
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 04/10] drm/i915: Add "missing" break to haswell_get_ddi_pll()

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 05:34:40PM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > While not techically needed on the last case in the switch statement,
> 
> "techically"
> 
> > the 'break' makes it look better IMO.
> 
> Just out of curiosity: what's your opinion on the lack of a "break" at
> the default case, such as the one we have in bxt_get_ddi_pll()?

I think I tend to include the break in all cases, except when I want
an explicit fall through, at which point I'd usually put a comment in
its place to tell people that I really meant it.

If on the hand there's a 'return' involved, I leave out the break
since it's dead anyway.

But I suspect I fail to follow even my own rules sometimes.

> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 29ea4c458ab3..d049b087e8e6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9808,6 +9808,7 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
> >                 break;
> >         case PORT_CLK_SEL_SPLL:
> >                 pipe_config->shared_dpll = DPLL_ID_SPLL;
> > +               break;
> >         }
> >  }
> >
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

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

[PATCH v2 04/10] drm/i915: Add "missing" break to haswell_get_ddi_pll()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While not technically needed on the last case in the switch statement,
the 'break' makes it look better IMO.

v2: Fixed a typo in the commit message (Paulo)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9f8ab09eb411..edc034dc38b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9808,6 +9808,7 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 		break;
 	case PORT_CLK_SEL_SPLL:
 		pipe_config->shared_dpll = DPLL_ID_SPLL;
+		break;
 	}
 }
 
-- 
2.4.10

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

Re: [PATCH v2 04/10] drm/i915: Add "missing" break to haswell_get_ddi_pll()

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 11:32:07PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> While not technically needed on the last case in the switch statement,
> the 'break' makes it look better IMO.
> 
> v2: Fixed a typo in the commit message (Paulo)
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9f8ab09eb411..edc034dc38b2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9808,6 +9808,7 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
>  		break;
>  	case PORT_CLK_SEL_SPLL:
>  		pipe_config->shared_dpll = DPLL_ID_SPLL;
> +		break;
>  	}
>  }

Merged up to this patch. Thanks for the reviews.

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

[PATCH 05/10] drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When we want to use SPLL for FDI we want SSC, which means we have to
disable clock bending for the PCH SSC reference (bend and spread are
mtutually exclusive). So let's turn off bending when we want spread.
In case the BIOS enabled clock bending for some reason we'll just turn
it off and enable the spread mode instead.

Not sure what happens if the BIOS is actually using the bend source for
HDMI at this time, but I suppose it should be no worse than what already
happens when we simply turn on the spread.

We don't currently use the bend source for anything, and only use the
PCH SSC reference for the SPLL to drive FDI (always with spread).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 65 ++++++++++++++++++++++++++++++++++--
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6d7ac192982d..fcc819f400a6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7320,6 +7320,7 @@ enum skl_disp_power_wells {
 #define  SBI_READY			(0x0<<0)
 
 /* SBI offsets */
+#define  SBI_SSCDIVINTPHASE			0x0200
 #define  SBI_SSCDIVINTPHASE6			0x0600
 #define   SBI_SSCDIVINTPHASE_DIVSEL_MASK	((0x7f)<<1)
 #define   SBI_SSCDIVINTPHASE_DIVSEL(x)		((x)<<1)
@@ -7327,6 +7328,7 @@ enum skl_disp_power_wells {
 #define   SBI_SSCDIVINTPHASE_INCVAL(x)		((x)<<8)
 #define   SBI_SSCDIVINTPHASE_DIR(x)		((x)<<15)
 #define   SBI_SSCDIVINTPHASE_PROPAGATE		(1<<0)
+#define  SBI_SSCDITHPHASE			0x0204
 #define  SBI_SSCCTL				0x020c
 #define  SBI_SSCCTL6				0x060C
 #define   SBI_SSCCTL_PATHALT			(1<<3)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d049b087e8e6..c429029e3bed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8551,6 +8551,65 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+#define BEND_IDX(steps) ((50 + (steps)) / 5)
+
+static const uint16_t sscdivintphase[] = {
+	[BEND_IDX( 50)] = 0x3B23,
+	[BEND_IDX( 45)] = 0x3B23,
+	[BEND_IDX( 40)] = 0x3C23,
+	[BEND_IDX( 35)] = 0x3C23,
+	[BEND_IDX( 30)] = 0x3D23,
+	[BEND_IDX( 25)] = 0x3D23,
+	[BEND_IDX( 20)] = 0x3E23,
+	[BEND_IDX( 15)] = 0x3E23,
+	[BEND_IDX( 10)] = 0x3F23,
+	[BEND_IDX(  5)] = 0x3F23,
+	[BEND_IDX(  0)] = 0x0025,
+	[BEND_IDX( -5)] = 0x0025,
+	[BEND_IDX(-10)] = 0x0125,
+	[BEND_IDX(-15)] = 0x0125,
+	[BEND_IDX(-20)] = 0x0225,
+	[BEND_IDX(-25)] = 0x0225,
+	[BEND_IDX(-30)] = 0x0325,
+	[BEND_IDX(-35)] = 0x0325,
+	[BEND_IDX(-40)] = 0x0425,
+	[BEND_IDX(-45)] = 0x0425,
+	[BEND_IDX(-50)] = 0x0525,
+};
+
+/*
+ * Bend CLKOUT_DP
+ * steps -50 to 50 inclusive, in steps of 5
+ * < 0 slow down the clock, > 0 speed up the clock, 0 == no bend (135MHz)
+ * change in clock period = -(steps / 10) * 5.787 ps
+ */
+static void lpt_bend_clkout_dp(struct drm_i915_private *dev_priv, int steps)
+{
+	uint32_t tmp;
+
+	int idx = BEND_IDX(steps);
+
+	if (WARN_ON(idx >= ARRAY_SIZE(sscdivintphase)))
+		return;
+
+	mutex_lock(&dev_priv->sb_lock);
+
+	if (steps % 5 != 0)
+		tmp = 0xAAAAAAAB;
+	else
+		tmp = 0x00000000;
+	intel_sbi_write(dev_priv, SBI_SSCDITHPHASE, tmp, SBI_ICLK);
+
+	tmp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE, SBI_ICLK);
+	tmp &= 0xffff0000;
+	tmp |= sscdivintphase[idx];
+	intel_sbi_write(dev_priv, SBI_SSCDIVINTPHASE, tmp, SBI_ICLK);
+
+	mutex_unlock(&dev_priv->sb_lock);
+}
+
+#undef BEND_IDX
+
 static void lpt_init_pch_refclk(struct drm_device *dev)
 {
 	struct intel_encoder *encoder;
@@ -8566,10 +8625,12 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
 		}
 	}
 
-	if (has_vga)
+	if (has_vga) {
+		lpt_bend_clkout_dp(to_i915(dev), 0);
 		lpt_enable_clkout_dp(dev, true, true);
-	else
+	} else {
 		lpt_disable_clkout_dp(dev);
+	}
 }
 
 /*
-- 
2.4.10

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

Re: [PATCH 05/10] drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> When we want to use SPLL for FDI we want SSC, which means we have to
> disable clock bending for the PCH SSC reference (bend and spread are
> mtutually exclusive). So let's turn off bending when we want spread.
> In case the BIOS enabled clock bending for some reason we'll just turn
> it off and enable the spread mode instead.
>
> Not sure what happens if the BIOS is actually using the bend source for
> HDMI at this time, but I suppose it should be no worse than what already
> happens when we simply turn on the spread.
>
> We don't currently use the bend source for anything, and only use the
> PCH SSC reference for the SPLL to drive FDI (always with spread).
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 65 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6d7ac192982d..fcc819f400a6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7320,6 +7320,7 @@ enum skl_disp_power_wells {
>  #define  SBI_READY                     (0x0<<0)
>
>  /* SBI offsets */
> +#define  SBI_SSCDIVINTPHASE                    0x0200
>  #define  SBI_SSCDIVINTPHASE6                   0x0600
>  #define   SBI_SSCDIVINTPHASE_DIVSEL_MASK       ((0x7f)<<1)
>  #define   SBI_SSCDIVINTPHASE_DIVSEL(x)         ((x)<<1)
> @@ -7327,6 +7328,7 @@ enum skl_disp_power_wells {
>  #define   SBI_SSCDIVINTPHASE_INCVAL(x)         ((x)<<8)
>  #define   SBI_SSCDIVINTPHASE_DIR(x)            ((x)<<15)
>  #define   SBI_SSCDIVINTPHASE_PROPAGATE         (1<<0)
> +#define  SBI_SSCDITHPHASE                      0x0204
>  #define  SBI_SSCCTL                            0x020c
>  #define  SBI_SSCCTL6                           0x060C
>  #define   SBI_SSCCTL_PATHALT                   (1<<3)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d049b087e8e6..c429029e3bed 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8551,6 +8551,65 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
>         mutex_unlock(&dev_priv->sb_lock);
>  }
>
> +#define BEND_IDX(steps) ((50 + (steps)) / 5)
> +
> +static const uint16_t sscdivintphase[] = {
> +       [BEND_IDX( 50)] = 0x3B23,
> +       [BEND_IDX( 45)] = 0x3B23,
> +       [BEND_IDX( 40)] = 0x3C23,
> +       [BEND_IDX( 35)] = 0x3C23,
> +       [BEND_IDX( 30)] = 0x3D23,
> +       [BEND_IDX( 25)] = 0x3D23,
> +       [BEND_IDX( 20)] = 0x3E23,
> +       [BEND_IDX( 15)] = 0x3E23,
> +       [BEND_IDX( 10)] = 0x3F23,
> +       [BEND_IDX(  5)] = 0x3F23,
> +       [BEND_IDX(  0)] = 0x0025,
> +       [BEND_IDX( -5)] = 0x0025,
> +       [BEND_IDX(-10)] = 0x0125,
> +       [BEND_IDX(-15)] = 0x0125,
> +       [BEND_IDX(-20)] = 0x0225,
> +       [BEND_IDX(-25)] = 0x0225,
> +       [BEND_IDX(-30)] = 0x0325,
> +       [BEND_IDX(-35)] = 0x0325,
> +       [BEND_IDX(-40)] = 0x0425,
> +       [BEND_IDX(-45)] = 0x0425,
> +       [BEND_IDX(-50)] = 0x0525,
> +};
> +
> +/*
> + * Bend CLKOUT_DP
> + * steps -50 to 50 inclusive, in steps of 5
> + * < 0 slow down the clock, > 0 speed up the clock, 0 == no bend (135MHz)
> + * change in clock period = -(steps / 10) * 5.787 ps
> + */
> +static void lpt_bend_clkout_dp(struct drm_i915_private *dev_priv, int steps)

As far as I understood from your comments and the table, "int steps"
should always be a multiple of 5, right?

> +{
> +       uint32_t tmp;
> +
> +       int idx = BEND_IDX(steps);

So shouldn't we do the following?

if (WARN_ON(steps % 5 != 0))
        return;

> +
> +       if (WARN_ON(idx >= ARRAY_SIZE(sscdivintphase)))
> +               return;
> +
> +       mutex_lock(&dev_priv->sb_lock);
> +
> +       if (steps % 5 != 0)

Shouldn't this be "steps % 10 != 0"? Otherwise, we'll always assign 0x0.

Everything else looks correct.


> +               tmp = 0xAAAAAAAB;
> +       else
> +               tmp = 0x00000000;
> +       intel_sbi_write(dev_priv, SBI_SSCDITHPHASE, tmp, SBI_ICLK);
> +
> +       tmp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE, SBI_ICLK);
> +       tmp &= 0xffff0000;
> +       tmp |= sscdivintphase[idx];
> +       intel_sbi_write(dev_priv, SBI_SSCDIVINTPHASE, tmp, SBI_ICLK);
> +
> +       mutex_unlock(&dev_priv->sb_lock);
> +}
> +
> +#undef BEND_IDX
> +
>  static void lpt_init_pch_refclk(struct drm_device *dev)
>  {
>         struct intel_encoder *encoder;
> @@ -8566,10 +8625,12 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
>                 }
>         }
>
> -       if (has_vga)
> +       if (has_vga) {
> +               lpt_bend_clkout_dp(to_i915(dev), 0);
>                 lpt_enable_clkout_dp(dev, true, true);
> -       else
> +       } else {
>                 lpt_disable_clkout_dp(dev);
> +       }
>  }
>
>  /*
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 05/10] drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed

[Ville Syrjälä]

On Wed, Dec 02, 2015 at 11:35:00AM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > When we want to use SPLL for FDI we want SSC, which means we have to
> > disable clock bending for the PCH SSC reference (bend and spread are
> > mtutually exclusive). So let's turn off bending when we want spread.
    ^^^^^^^^^
Spotted this typo here just now. Will fix that one too.

> > In case the BIOS enabled clock bending for some reason we'll just turn
> > it off and enable the spread mode instead.
> >
> > Not sure what happens if the BIOS is actually using the bend source for
> > HDMI at this time, but I suppose it should be no worse than what already
> > happens when we simply turn on the spread.
> >
> > We don't currently use the bend source for anything, and only use the
> > PCH SSC reference for the SPLL to drive FDI (always with spread).
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  2 ++
> >  drivers/gpu/drm/i915/intel_display.c | 65 ++++++++++++++++++++++++++++++++++--
> >  2 files changed, 65 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 6d7ac192982d..fcc819f400a6 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7320,6 +7320,7 @@ enum skl_disp_power_wells {
> >  #define  SBI_READY                     (0x0<<0)
> >
> >  /* SBI offsets */
> > +#define  SBI_SSCDIVINTPHASE                    0x0200
> >  #define  SBI_SSCDIVINTPHASE6                   0x0600
> >  #define   SBI_SSCDIVINTPHASE_DIVSEL_MASK       ((0x7f)<<1)
> >  #define   SBI_SSCDIVINTPHASE_DIVSEL(x)         ((x)<<1)
> > @@ -7327,6 +7328,7 @@ enum skl_disp_power_wells {
> >  #define   SBI_SSCDIVINTPHASE_INCVAL(x)         ((x)<<8)
> >  #define   SBI_SSCDIVINTPHASE_DIR(x)            ((x)<<15)
> >  #define   SBI_SSCDIVINTPHASE_PROPAGATE         (1<<0)
> > +#define  SBI_SSCDITHPHASE                      0x0204
> >  #define  SBI_SSCCTL                            0x020c
> >  #define  SBI_SSCCTL6                           0x060C
> >  #define   SBI_SSCCTL_PATHALT                   (1<<3)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d049b087e8e6..c429029e3bed 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8551,6 +8551,65 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
> >         mutex_unlock(&dev_priv->sb_lock);
> >  }
> >
> > +#define BEND_IDX(steps) ((50 + (steps)) / 5)
> > +
> > +static const uint16_t sscdivintphase[] = {
> > +       [BEND_IDX( 50)] = 0x3B23,
> > +       [BEND_IDX( 45)] = 0x3B23,
> > +       [BEND_IDX( 40)] = 0x3C23,
> > +       [BEND_IDX( 35)] = 0x3C23,
> > +       [BEND_IDX( 30)] = 0x3D23,
> > +       [BEND_IDX( 25)] = 0x3D23,
> > +       [BEND_IDX( 20)] = 0x3E23,
> > +       [BEND_IDX( 15)] = 0x3E23,
> > +       [BEND_IDX( 10)] = 0x3F23,
> > +       [BEND_IDX(  5)] = 0x3F23,
> > +       [BEND_IDX(  0)] = 0x0025,
> > +       [BEND_IDX( -5)] = 0x0025,
> > +       [BEND_IDX(-10)] = 0x0125,
> > +       [BEND_IDX(-15)] = 0x0125,
> > +       [BEND_IDX(-20)] = 0x0225,
> > +       [BEND_IDX(-25)] = 0x0225,
> > +       [BEND_IDX(-30)] = 0x0325,
> > +       [BEND_IDX(-35)] = 0x0325,
> > +       [BEND_IDX(-40)] = 0x0425,
> > +       [BEND_IDX(-45)] = 0x0425,
> > +       [BEND_IDX(-50)] = 0x0525,
> > +};
> > +
> > +/*
> > + * Bend CLKOUT_DP
> > + * steps -50 to 50 inclusive, in steps of 5
> > + * < 0 slow down the clock, > 0 speed up the clock, 0 == no bend (135MHz)
> > + * change in clock period = -(steps / 10) * 5.787 ps
> > + */
> > +static void lpt_bend_clkout_dp(struct drm_i915_private *dev_priv, int steps)
> 
> As far as I understood from your comments and the table, "int steps"
> should always be a multiple of 5, right?

Yes, it's in decimal .1 fixed point, but only .0 and .5 values are allowed.
I could have made it binary .1 fixed point, but then comparing with the
spec might have been somewhat less obvious.

> 
> > +{
> > +       uint32_t tmp;
> > +
> > +       int idx = BEND_IDX(steps);
> 
> So shouldn't we do the following?
> 
> if (WARN_ON(steps % 5 != 0))
>     return;

Yes, that's a good idea.

> 
> > +
> > +       if (WARN_ON(idx >= ARRAY_SIZE(sscdivintphase)))
> > +               return;
> > +
> > +       mutex_lock(&dev_priv->sb_lock);
> > +
> > +       if (steps % 5 != 0)
> 
> Shouldn't this be "steps % 10 != 0"? Otherwise, we'll always assign 0x0.

Doh! Thanks for catching that. Will fix.

> 
> Everything else looks correct.
> 
> 
> > +               tmp = 0xAAAAAAAB;
> > +       else
> > +               tmp = 0x00000000;
> > +       intel_sbi_write(dev_priv, SBI_SSCDITHPHASE, tmp, SBI_ICLK);
> > +
> > +       tmp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE, SBI_ICLK);
> > +       tmp &= 0xffff0000;
> > +       tmp |= sscdivintphase[idx];
> > +       intel_sbi_write(dev_priv, SBI_SSCDIVINTPHASE, tmp, SBI_ICLK);
> > +
> > +       mutex_unlock(&dev_priv->sb_lock);
> > +}
> > +
> > +#undef BEND_IDX
> > +
> >  static void lpt_init_pch_refclk(struct drm_device *dev)
> >  {
> >         struct intel_encoder *encoder;
> > @@ -8566,10 +8625,12 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
> >                 }
> >         }
> >
> > -       if (has_vga)
> > +       if (has_vga) {
> > +               lpt_bend_clkout_dp(to_i915(dev), 0);
> >                 lpt_enable_clkout_dp(dev, true, true);
> > -       else
> > +       } else {
> >                 lpt_disable_clkout_dp(dev);
> > +       }
> >  }
> >
> >  /*
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

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

Re: [PATCH 05/10] drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed

[Paulo Zanoni]

2015-12-03 8:03 GMT-02:00 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Wed, Dec 02, 2015 at 11:35:00AM -0200, Paulo Zanoni wrote:
>> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > When we want to use SPLL for FDI we want SSC, which means we have to
>> > disable clock bending for the PCH SSC reference (bend and spread are
>> > mtutually exclusive). So let's turn off bending when we want spread.
>     ^^^^^^^^^
> Spotted this typo here just now. Will fix that one too.
>
>> > In case the BIOS enabled clock bending for some reason we'll just turn
>> > it off and enable the spread mode instead.
>> >
>> > Not sure what happens if the BIOS is actually using the bend source for
>> > HDMI at this time, but I suppose it should be no worse than what already
>> > happens when we simply turn on the spread.
>> >
>> > We don't currently use the bend source for anything, and only use the
>> > PCH SSC reference for the SPLL to drive FDI (always with spread).
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h      |  2 ++
>> >  drivers/gpu/drm/i915/intel_display.c | 65 ++++++++++++++++++++++++++++++++++--
>> >  2 files changed, 65 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index 6d7ac192982d..fcc819f400a6 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -7320,6 +7320,7 @@ enum skl_disp_power_wells {
>> >  #define  SBI_READY                     (0x0<<0)
>> >
>> >  /* SBI offsets */
>> > +#define  SBI_SSCDIVINTPHASE                    0x0200
>> >  #define  SBI_SSCDIVINTPHASE6                   0x0600
>> >  #define   SBI_SSCDIVINTPHASE_DIVSEL_MASK       ((0x7f)<<1)
>> >  #define   SBI_SSCDIVINTPHASE_DIVSEL(x)         ((x)<<1)
>> > @@ -7327,6 +7328,7 @@ enum skl_disp_power_wells {
>> >  #define   SBI_SSCDIVINTPHASE_INCVAL(x)         ((x)<<8)
>> >  #define   SBI_SSCDIVINTPHASE_DIR(x)            ((x)<<15)
>> >  #define   SBI_SSCDIVINTPHASE_PROPAGATE         (1<<0)
>> > +#define  SBI_SSCDITHPHASE                      0x0204
>> >  #define  SBI_SSCCTL                            0x020c
>> >  #define  SBI_SSCCTL6                           0x060C
>> >  #define   SBI_SSCCTL_PATHALT                   (1<<3)
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > index d049b087e8e6..c429029e3bed 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -8551,6 +8551,65 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
>> >         mutex_unlock(&dev_priv->sb_lock);
>> >  }
>> >
>> > +#define BEND_IDX(steps) ((50 + (steps)) / 5)
>> > +
>> > +static const uint16_t sscdivintphase[] = {
>> > +       [BEND_IDX( 50)] = 0x3B23,
>> > +       [BEND_IDX( 45)] = 0x3B23,
>> > +       [BEND_IDX( 40)] = 0x3C23,
>> > +       [BEND_IDX( 35)] = 0x3C23,
>> > +       [BEND_IDX( 30)] = 0x3D23,
>> > +       [BEND_IDX( 25)] = 0x3D23,
>> > +       [BEND_IDX( 20)] = 0x3E23,
>> > +       [BEND_IDX( 15)] = 0x3E23,
>> > +       [BEND_IDX( 10)] = 0x3F23,
>> > +       [BEND_IDX(  5)] = 0x3F23,
>> > +       [BEND_IDX(  0)] = 0x0025,
>> > +       [BEND_IDX( -5)] = 0x0025,
>> > +       [BEND_IDX(-10)] = 0x0125,
>> > +       [BEND_IDX(-15)] = 0x0125,
>> > +       [BEND_IDX(-20)] = 0x0225,
>> > +       [BEND_IDX(-25)] = 0x0225,
>> > +       [BEND_IDX(-30)] = 0x0325,
>> > +       [BEND_IDX(-35)] = 0x0325,
>> > +       [BEND_IDX(-40)] = 0x0425,
>> > +       [BEND_IDX(-45)] = 0x0425,
>> > +       [BEND_IDX(-50)] = 0x0525,
>> > +};
>> > +
>> > +/*
>> > + * Bend CLKOUT_DP
>> > + * steps -50 to 50 inclusive, in steps of 5
>> > + * < 0 slow down the clock, > 0 speed up the clock, 0 == no bend (135MHz)
>> > + * change in clock period = -(steps / 10) * 5.787 ps
>> > + */
>> > +static void lpt_bend_clkout_dp(struct drm_i915_private *dev_priv, int steps)
>>
>> As far as I understood from your comments and the table, "int steps"
>> should always be a multiple of 5, right?
>
> Yes, it's in decimal .1 fixed point, but only .0 and .5 values are allowed.
> I could have made it binary .1 fixed point, but then comparing with the
> spec might have been somewhat less obvious.

I actually liked your approach since it kinda matches the spec, so you
can keep it. I was just asking for confirmation due to the "% 5"
problem below.

>
>>
>> > +{
>> > +       uint32_t tmp;
>> > +
>> > +       int idx = BEND_IDX(steps);
>>
>> So shouldn't we do the following?
>>
>> if (WARN_ON(steps % 5 != 0))
>>     return;
>
> Yes, that's a good idea.
>
>>
>> > +
>> > +       if (WARN_ON(idx >= ARRAY_SIZE(sscdivintphase)))
>> > +               return;
>> > +
>> > +       mutex_lock(&dev_priv->sb_lock);
>> > +
>> > +       if (steps % 5 != 0)
>>
>> Shouldn't this be "steps % 10 != 0"? Otherwise, we'll always assign 0x0.
>
> Doh! Thanks for catching that. Will fix.
>
>>
>> Everything else looks correct.
>>
>>
>> > +               tmp = 0xAAAAAAAB;
>> > +       else
>> > +               tmp = 0x00000000;
>> > +       intel_sbi_write(dev_priv, SBI_SSCDITHPHASE, tmp, SBI_ICLK);
>> > +
>> > +       tmp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE, SBI_ICLK);
>> > +       tmp &= 0xffff0000;
>> > +       tmp |= sscdivintphase[idx];
>> > +       intel_sbi_write(dev_priv, SBI_SSCDIVINTPHASE, tmp, SBI_ICLK);
>> > +
>> > +       mutex_unlock(&dev_priv->sb_lock);
>> > +}
>> > +
>> > +#undef BEND_IDX
>> > +
>> >  static void lpt_init_pch_refclk(struct drm_device *dev)
>> >  {
>> >         struct intel_encoder *encoder;
>> > @@ -8566,10 +8625,12 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
>> >                 }
>> >         }
>> >
>> > -       if (has_vga)
>> > +       if (has_vga) {
>> > +               lpt_bend_clkout_dp(to_i915(dev), 0);
>> >                 lpt_enable_clkout_dp(dev, true, true);
>> > -       else
>> > +       } else {
>> >                 lpt_disable_clkout_dp(dev);
>> > +       }
>> >  }
>> >
>> >  /*
>> > --
>> > 2.4.10
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>
> --
> Ville Syrjälä
> Intel OTC



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

[PATCH v2 05/10] drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When we want to use SPLL for FDI we want SSC, which means we have to
disable clock bending for the PCH SSC reference (bend and spread are
mutually exclusive). So let's turn off bending when we want spread.
In case the BIOS enabled clock bending for some reason we'll just turn
it off and enable the spread mode instead.

Not sure what happens if the BIOS is actually using the bend source for
HDMI at this time, but I suppose it should be no worse than what already
happens when we simply turn on the spread.

We don't currently use the bend source for anything, and only use the
PCH SSC reference for the SPLL to drive FDI (always with spread).

v2: Fix the %5 vs %10 fumble for SSCDITHPHASE (Paulo)
    Add 'WARN_ON(steps % 5 != 0)' sanity check (Paulo)
    Fix typos in commit message (Paulo)

Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 67 ++++++++++++++++++++++++++++++++++--
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1dae5ac3e0b1..d0ea877011c1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7327,6 +7327,7 @@ enum skl_disp_power_wells {
 #define  SBI_READY			(0x0<<0)
 
 /* SBI offsets */
+#define  SBI_SSCDIVINTPHASE			0x0200
 #define  SBI_SSCDIVINTPHASE6			0x0600
 #define   SBI_SSCDIVINTPHASE_DIVSEL_MASK	((0x7f)<<1)
 #define   SBI_SSCDIVINTPHASE_DIVSEL(x)		((x)<<1)
@@ -7334,6 +7335,7 @@ enum skl_disp_power_wells {
 #define   SBI_SSCDIVINTPHASE_INCVAL(x)		((x)<<8)
 #define   SBI_SSCDIVINTPHASE_DIR(x)		((x)<<15)
 #define   SBI_SSCDIVINTPHASE_PROPAGATE		(1<<0)
+#define  SBI_SSCDITHPHASE			0x0204
 #define  SBI_SSCCTL				0x020c
 #define  SBI_SSCCTL6				0x060C
 #define   SBI_SSCCTL_PATHALT			(1<<3)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 61000ff524f1..3811bcb73229 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8564,6 +8564,67 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+#define BEND_IDX(steps) ((50 + (steps)) / 5)
+
+static const uint16_t sscdivintphase[] = {
+	[BEND_IDX( 50)] = 0x3B23,
+	[BEND_IDX( 45)] = 0x3B23,
+	[BEND_IDX( 40)] = 0x3C23,
+	[BEND_IDX( 35)] = 0x3C23,
+	[BEND_IDX( 30)] = 0x3D23,
+	[BEND_IDX( 25)] = 0x3D23,
+	[BEND_IDX( 20)] = 0x3E23,
+	[BEND_IDX( 15)] = 0x3E23,
+	[BEND_IDX( 10)] = 0x3F23,
+	[BEND_IDX(  5)] = 0x3F23,
+	[BEND_IDX(  0)] = 0x0025,
+	[BEND_IDX( -5)] = 0x0025,
+	[BEND_IDX(-10)] = 0x0125,
+	[BEND_IDX(-15)] = 0x0125,
+	[BEND_IDX(-20)] = 0x0225,
+	[BEND_IDX(-25)] = 0x0225,
+	[BEND_IDX(-30)] = 0x0325,
+	[BEND_IDX(-35)] = 0x0325,
+	[BEND_IDX(-40)] = 0x0425,
+	[BEND_IDX(-45)] = 0x0425,
+	[BEND_IDX(-50)] = 0x0525,
+};
+
+/*
+ * Bend CLKOUT_DP
+ * steps -50 to 50 inclusive, in steps of 5
+ * < 0 slow down the clock, > 0 speed up the clock, 0 == no bend (135MHz)
+ * change in clock period = -(steps / 10) * 5.787 ps
+ */
+static void lpt_bend_clkout_dp(struct drm_i915_private *dev_priv, int steps)
+{
+	uint32_t tmp;
+	int idx = BEND_IDX(steps);
+
+	if (WARN_ON(steps % 5 != 0))
+		return;
+
+	if (WARN_ON(idx >= ARRAY_SIZE(sscdivintphase)))
+		return;
+
+	mutex_lock(&dev_priv->sb_lock);
+
+	if (steps % 10 != 0)
+		tmp = 0xAAAAAAAB;
+	else
+		tmp = 0x00000000;
+	intel_sbi_write(dev_priv, SBI_SSCDITHPHASE, tmp, SBI_ICLK);
+
+	tmp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE, SBI_ICLK);
+	tmp &= 0xffff0000;
+	tmp |= sscdivintphase[idx];
+	intel_sbi_write(dev_priv, SBI_SSCDIVINTPHASE, tmp, SBI_ICLK);
+
+	mutex_unlock(&dev_priv->sb_lock);
+}
+
+#undef BEND_IDX
+
 static void lpt_init_pch_refclk(struct drm_device *dev)
 {
 	struct intel_encoder *encoder;
@@ -8579,10 +8640,12 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
 		}
 	}
 
-	if (has_vga)
+	if (has_vga) {
+		lpt_bend_clkout_dp(to_i915(dev), 0);
 		lpt_enable_clkout_dp(dev, true, true);
-	else
+	} else {
 		lpt_disable_clkout_dp(dev);
+	}
 }
 
 /*
-- 
2.4.10

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

Re: [PATCH v2 05/10] drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed

[Paulo Zanoni]

2015-12-04 18:19 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> When we want to use SPLL for FDI we want SSC, which means we have to
> disable clock bending for the PCH SSC reference (bend and spread are
> mutually exclusive). So let's turn off bending when we want spread.
> In case the BIOS enabled clock bending for some reason we'll just turn
> it off and enable the spread mode instead.
>
> Not sure what happens if the BIOS is actually using the bend source for
> HDMI at this time, but I suppose it should be no worse than what already
> happens when we simply turn on the spread.
>
> We don't currently use the bend source for anything, and only use the
> PCH SSC reference for the SPLL to drive FDI (always with spread).
>
> v2: Fix the %5 vs %10 fumble for SSCDITHPHASE (Paulo)
>     Add 'WARN_ON(steps % 5 != 0)' sanity check (Paulo)
>     Fix typos in commit message (Paulo)
>
> Cc: Paulo Zanoni <przanoni@gmail.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 67 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 67 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1dae5ac3e0b1..d0ea877011c1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7327,6 +7327,7 @@ enum skl_disp_power_wells {
>  #define  SBI_READY                     (0x0<<0)
>
>  /* SBI offsets */
> +#define  SBI_SSCDIVINTPHASE                    0x0200
>  #define  SBI_SSCDIVINTPHASE6                   0x0600
>  #define   SBI_SSCDIVINTPHASE_DIVSEL_MASK       ((0x7f)<<1)
>  #define   SBI_SSCDIVINTPHASE_DIVSEL(x)         ((x)<<1)
> @@ -7334,6 +7335,7 @@ enum skl_disp_power_wells {
>  #define   SBI_SSCDIVINTPHASE_INCVAL(x)         ((x)<<8)
>  #define   SBI_SSCDIVINTPHASE_DIR(x)            ((x)<<15)
>  #define   SBI_SSCDIVINTPHASE_PROPAGATE         (1<<0)
> +#define  SBI_SSCDITHPHASE                      0x0204
>  #define  SBI_SSCCTL                            0x020c
>  #define  SBI_SSCCTL6                           0x060C
>  #define   SBI_SSCCTL_PATHALT                   (1<<3)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 61000ff524f1..3811bcb73229 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8564,6 +8564,67 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
>         mutex_unlock(&dev_priv->sb_lock);
>  }
>
> +#define BEND_IDX(steps) ((50 + (steps)) / 5)
> +
> +static const uint16_t sscdivintphase[] = {
> +       [BEND_IDX( 50)] = 0x3B23,
> +       [BEND_IDX( 45)] = 0x3B23,
> +       [BEND_IDX( 40)] = 0x3C23,
> +       [BEND_IDX( 35)] = 0x3C23,
> +       [BEND_IDX( 30)] = 0x3D23,
> +       [BEND_IDX( 25)] = 0x3D23,
> +       [BEND_IDX( 20)] = 0x3E23,
> +       [BEND_IDX( 15)] = 0x3E23,
> +       [BEND_IDX( 10)] = 0x3F23,
> +       [BEND_IDX(  5)] = 0x3F23,
> +       [BEND_IDX(  0)] = 0x0025,
> +       [BEND_IDX( -5)] = 0x0025,
> +       [BEND_IDX(-10)] = 0x0125,
> +       [BEND_IDX(-15)] = 0x0125,
> +       [BEND_IDX(-20)] = 0x0225,
> +       [BEND_IDX(-25)] = 0x0225,
> +       [BEND_IDX(-30)] = 0x0325,
> +       [BEND_IDX(-35)] = 0x0325,
> +       [BEND_IDX(-40)] = 0x0425,
> +       [BEND_IDX(-45)] = 0x0425,
> +       [BEND_IDX(-50)] = 0x0525,
> +};
> +
> +/*
> + * Bend CLKOUT_DP
> + * steps -50 to 50 inclusive, in steps of 5
> + * < 0 slow down the clock, > 0 speed up the clock, 0 == no bend (135MHz)
> + * change in clock period = -(steps / 10) * 5.787 ps
> + */
> +static void lpt_bend_clkout_dp(struct drm_i915_private *dev_priv, int steps)
> +{
> +       uint32_t tmp;
> +       int idx = BEND_IDX(steps);
> +
> +       if (WARN_ON(steps % 5 != 0))
> +               return;
> +
> +       if (WARN_ON(idx >= ARRAY_SIZE(sscdivintphase)))
> +               return;
> +
> +       mutex_lock(&dev_priv->sb_lock);
> +
> +       if (steps % 10 != 0)
> +               tmp = 0xAAAAAAAB;
> +       else
> +               tmp = 0x00000000;
> +       intel_sbi_write(dev_priv, SBI_SSCDITHPHASE, tmp, SBI_ICLK);
> +
> +       tmp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE, SBI_ICLK);
> +       tmp &= 0xffff0000;
> +       tmp |= sscdivintphase[idx];
> +       intel_sbi_write(dev_priv, SBI_SSCDIVINTPHASE, tmp, SBI_ICLK);
> +
> +       mutex_unlock(&dev_priv->sb_lock);
> +}
> +
> +#undef BEND_IDX
> +
>  static void lpt_init_pch_refclk(struct drm_device *dev)
>  {
>         struct intel_encoder *encoder;
> @@ -8579,10 +8640,12 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
>                 }
>         }
>
> -       if (has_vga)
> +       if (has_vga) {
> +               lpt_bend_clkout_dp(to_i915(dev), 0);
>                 lpt_enable_clkout_dp(dev, true, true);
> -       else
> +       } else {
>                 lpt_disable_clkout_dp(dev);
> +       }
>  }
>
>  /*
> --
> 2.4.10
>



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

[PATCH 06/10] drm/i915: Round to closest when computing the VGA dotclock for LPT-H

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bspec says we should round to closest when computong the LPT-H VGA
dotclock, so let's do that.

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 c429029e3bed..55419e4d032c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3976,7 +3976,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 		u32 iclk_pi_range = 64;
 		u32 desired_divisor, msb_divisor_value, pi_value;
 
-		desired_divisor = (iclk_virtual_root_freq / clock);
+		desired_divisor = DIV_ROUND_CLOSEST(iclk_virtual_root_freq, clock);
 		msb_divisor_value = desired_divisor / iclk_pi_range;
 		pi_value = desired_divisor % iclk_pi_range;
 
-- 
2.4.10

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

Re: [PATCH 06/10] drm/i915: Round to closest when computing the VGA dotclock for LPT-H

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Bspec says we should round to closest when computong the LPT-H VGA

s/computong/computing/

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> dotclock, so let's do that.
>
> 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 c429029e3bed..55419e4d032c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3976,7 +3976,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
>                 u32 iclk_pi_range = 64;
>                 u32 desired_divisor, msb_divisor_value, pi_value;
>
> -               desired_divisor = (iclk_virtual_root_freq / clock);
> +               desired_divisor = DIV_ROUND_CLOSEST(iclk_virtual_root_freq, clock);
>                 msb_divisor_value = desired_divisor / iclk_pi_range;
>                 pi_value = desired_divisor % iclk_pi_range;
>
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

[PATCH v2 06/10] drm/i915: Round to closest when computing the VGA dotclock for LPT-H

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bspec says we should round to closest when computing the LPT-H VGA
dotclock, so let's do that.

v2: Fix typo in commit message (Paulo)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@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 3811bcb73229..3391ab0ca752 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3978,7 +3978,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 		u32 iclk_pi_range = 64;
 		u32 desired_divisor, msb_divisor_value, pi_value;
 
-		desired_divisor = (iclk_virtual_root_freq / clock);
+		desired_divisor = DIV_ROUND_CLOSEST(iclk_virtual_root_freq, clock);
 		msb_divisor_value = desired_divisor / iclk_pi_range;
 		pi_value = desired_divisor % iclk_pi_range;
 
-- 
2.4.10

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

[PATCH 07/10] drm/i915: Disable FDI after the CRT port on LPT-H

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bspec modeset sequence tells us to disable the PCH transcoder and
FDI after the CRT port on LPT-H, so let's do that.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 55419e4d032c..1dc125b6dcdc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5159,18 +5159,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->config->has_dsi_encoder)
 		intel_ddi_disable_pipe_clock(intel_crtc);
 
-	if (intel_crtc->config->has_pch_encoder) {
-		lpt_disable_pch_transcoder(dev_priv);
-		intel_ddi_fdi_disable(crtc);
-	}
-
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->post_disable)
 			encoder->post_disable(encoder);
 
-	if (intel_crtc->config->has_pch_encoder)
+	if (intel_crtc->config->has_pch_encoder) {
+		lpt_disable_pch_transcoder(dev_priv);
+		intel_ddi_fdi_disable(crtc);
+
 		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
 						      true);
+	}
 }
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
-- 
2.4.10

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

Re: [PATCH 07/10] drm/i915: Disable FDI after the CRT port on LPT-H

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Bspec modeset sequence tells us to disable the PCH transcoder and
> FDI after the CRT port on LPT-H, so let's do that.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 55419e4d032c..1dc125b6dcdc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5159,18 +5159,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>         if (!intel_crtc->config->has_dsi_encoder)
>                 intel_ddi_disable_pipe_clock(intel_crtc);
>
> -       if (intel_crtc->config->has_pch_encoder) {
> -               lpt_disable_pch_transcoder(dev_priv);
> -               intel_ddi_fdi_disable(crtc);
> -       }
> -
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->post_disable)
>                         encoder->post_disable(encoder);

On HAS_DDI platforms, encoder->post_disable() is unset.

Also, this commit is a revert of commit
97b040aa391651793e4d463408c137b81517cc90.

>
> -       if (intel_crtc->config->has_pch_encoder)
> +       if (intel_crtc->config->has_pch_encoder) {
> +               lpt_disable_pch_transcoder(dev_priv);
> +               intel_ddi_fdi_disable(crtc);
> +
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
>                                                       true);
> +       }
>  }
>
>  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 07/10] drm/i915: Disable FDI after the CRT port on LPT-H

[Ville Syrjälä]

On Wed, Dec 02, 2015 at 12:01:43PM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Bspec modeset sequence tells us to disable the PCH transcoder and
> > FDI after the CRT port on LPT-H, so let's do that.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 55419e4d032c..1dc125b6dcdc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5159,18 +5159,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> >         if (!intel_crtc->config->has_dsi_encoder)
> >                 intel_ddi_disable_pipe_clock(intel_crtc);
> >
> > -       if (intel_crtc->config->has_pch_encoder) {
> > -               lpt_disable_pch_transcoder(dev_priv);
> > -               intel_ddi_fdi_disable(crtc);
> > -       }
> > -
> >         for_each_encoder_on_crtc(dev, crtc, encoder)
> >                 if (encoder->post_disable)
> >                         encoder->post_disable(encoder);
> 
> On HAS_DDI platforms, encoder->post_disable() is unset.

Hmm. It should be set for CRT... except it isn't. I guess I chickened
out on LPT when I fixed the other PCH platforms. I'll need to fix this
too then, and then redo some testing.

> 
> Also, this commit is a revert of commit
> 97b040aa391651793e4d463408c137b81517cc90.

Hmm. OK, so we had the SPLL disable in the post_disable hook, which was
correct as far as the modeset sequence goes, but the port disable was
always in the wrong place. SPLL was then converted into a shared pll
which moved the SPLL disable out from post_disable to some higher level
code, leaving the door open to fixing the rest of the modeset sequence.

If we still had the SPLL disable in the crt code, we'd have to move it
to the post_pll_disable hook instead, but since it's a shared pll now we
don't have to do anything.

> 
> >
> > -       if (intel_crtc->config->has_pch_encoder)
> > +       if (intel_crtc->config->has_pch_encoder) {
> > +               lpt_disable_pch_transcoder(dev_priv);
> > +               intel_ddi_fdi_disable(crtc);
> > +
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> >                                                       true);
> > +       }
> >  }
> >
> >  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

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

[PATCH v2 07/10] drm/i915: Disable FDI after the CRT port on LPT-H

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bspec modeset sequence tells us to disable the PCH transcoder and
FDI after the CRT port on LPT-H, so let's do that. And the CRT port
should be disabled after the pipe, as we do on other PCH platforms
too since
commit 1ea56e269e13 ("drm/i915: Disable CRT port after pipe on PCH platforms")

commit 00490c22b1b5 ("drm/i915: Consider SPLL as another shared pll, v2.")
moved the SPLL disaable from the .post_disable() hook to some upper
laeve code, so we can just move the CRT port disabling into the
.post_disable() hook. If we still had the non-shared SPLL, it would have
needed to be moved into the .post_pll_disable() hook.

v2: Actually move the CRT port disable to the .post_disable() hook,
    and amend the commit message with more details (Paulo)

Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 12008af797bd..cef359958c73 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -844,7 +844,7 @@ void intel_crt_init(struct drm_device *dev)
 	crt->adpa_reg = adpa_reg;
 
 	crt->base.compute_config = intel_crt_compute_config;
-	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) {
+	if (HAS_PCH_SPLIT(dev)) {
 		crt->base.disable = pch_disable_crt;
 		crt->base.post_disable = pch_post_disable_crt;
 	} else {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3391ab0ca752..42ed799390e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5166,18 +5166,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->config->has_dsi_encoder)
 		intel_ddi_disable_pipe_clock(intel_crtc);
 
-	if (intel_crtc->config->has_pch_encoder) {
-		lpt_disable_pch_transcoder(dev_priv);
-		intel_ddi_fdi_disable(crtc);
-	}
-
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->post_disable)
 			encoder->post_disable(encoder);
 
-	if (intel_crtc->config->has_pch_encoder)
+	if (intel_crtc->config->has_pch_encoder) {
+		lpt_disable_pch_transcoder(dev_priv);
+		intel_ddi_fdi_disable(crtc);
+
 		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
 						      true);
+	}
 
 	intel_fbc_disable_crtc(intel_crtc);
 }
-- 
2.4.10

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

Re: [PATCH v2 07/10] drm/i915: Disable FDI after the CRT port on LPT-H

[Paulo Zanoni]

2015-12-04 18:20 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Bspec modeset sequence tells us to disable the PCH transcoder and
> FDI after the CRT port on LPT-H, so let's do that. And the CRT port
> should be disabled after the pipe, as we do on other PCH platforms
> too since
> commit 1ea56e269e13 ("drm/i915: Disable CRT port after pipe on PCH platforms")
>
> commit 00490c22b1b5 ("drm/i915: Consider SPLL as another shared pll, v2.")
> moved the SPLL disaable from the .post_disable() hook to some upper

disaable

> laeve code, so we can just move the CRT port disabling into the

laeve

> .post_disable() hook. If we still had the non-shared SPLL, it would have
> needed to be moved into the .post_pll_disable() hook.
>
> v2: Actually move the CRT port disable to the .post_disable() hook,
>     and amend the commit message with more details (Paulo)

Since I seem to recall that the DDI disable sequence for CRT was
correct at some point in a distant past, I suppose your commit is a
regression fix, and maybe we'd like a backportable version. Maybe we
could also provide explicit information on the first bad commit.

Anyway, now it looks correct:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
>  2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 12008af797bd..cef359958c73 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -844,7 +844,7 @@ void intel_crt_init(struct drm_device *dev)
>         crt->adpa_reg = adpa_reg;
>
>         crt->base.compute_config = intel_crt_compute_config;
> -       if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) {
> +       if (HAS_PCH_SPLIT(dev)) {
>                 crt->base.disable = pch_disable_crt;
>                 crt->base.post_disable = pch_post_disable_crt;
>         } else {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3391ab0ca752..42ed799390e5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5166,18 +5166,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>         if (!intel_crtc->config->has_dsi_encoder)
>                 intel_ddi_disable_pipe_clock(intel_crtc);
>
> -       if (intel_crtc->config->has_pch_encoder) {
> -               lpt_disable_pch_transcoder(dev_priv);
> -               intel_ddi_fdi_disable(crtc);
> -       }
> -
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->post_disable)
>                         encoder->post_disable(encoder);
>
> -       if (intel_crtc->config->has_pch_encoder)
> +       if (intel_crtc->config->has_pch_encoder) {
> +               lpt_disable_pch_transcoder(dev_priv);
> +               intel_ddi_fdi_disable(crtc);
> +
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
>                                                       true);
> +       }
>
>         intel_fbc_disable_crtc(intel_crtc);
>  }
> --
> 2.4.10
>



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

Re: [PATCH v2 07/10] drm/i915: Disable FDI after the CRT port on LPT-H

[Ville Syrjälä]

On Mon, Dec 07, 2015 at 03:51:06PM -0200, Paulo Zanoni wrote:
> 2015-12-04 18:20 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Bspec modeset sequence tells us to disable the PCH transcoder and
> > FDI after the CRT port on LPT-H, so let's do that. And the CRT port
> > should be disabled after the pipe, as we do on other PCH platforms
> > too since
> > commit 1ea56e269e13 ("drm/i915: Disable CRT port after pipe on PCH platforms")
> >
> > commit 00490c22b1b5 ("drm/i915: Consider SPLL as another shared pll, v2.")
> > moved the SPLL disaable from the .post_disable() hook to some upper
> 
> disaable
> 
> > laeve code, so we can just move the CRT port disabling into the
> 
> laeve
> 
> > .post_disable() hook. If we still had the non-shared SPLL, it would have
> > needed to be moved into the .post_pll_disable() hook.
> >
> > v2: Actually move the CRT port disable to the .post_disable() hook,
> >     and amend the commit message with more details (Paulo)
> 
> Since I seem to recall that the DDI disable sequence for CRT was
> correct at some point in a distant past, I suppose your commit is a
> regression fix, and maybe we'd like a backportable version. Maybe we
> could also provide explicit information on the first bad commit.

Well, even the spec has changed since the early days, at least w.r.t.
the FDI RX disable, so not sure if something else has changed in there
as well. I didn't go digging through history to see if we accidentally
changed the order of some steps at some point.

> 
> Anyway, now it looks correct:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> >
> > Cc: Paulo Zanoni <przanoni@gmail.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_crt.c     |  2 +-
> >  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
> >  2 files changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> > index 12008af797bd..cef359958c73 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -844,7 +844,7 @@ void intel_crt_init(struct drm_device *dev)
> >         crt->adpa_reg = adpa_reg;
> >
> >         crt->base.compute_config = intel_crt_compute_config;
> > -       if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) {
> > +       if (HAS_PCH_SPLIT(dev)) {
> >                 crt->base.disable = pch_disable_crt;
> >                 crt->base.post_disable = pch_post_disable_crt;
> >         } else {
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3391ab0ca752..42ed799390e5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5166,18 +5166,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> >         if (!intel_crtc->config->has_dsi_encoder)
> >                 intel_ddi_disable_pipe_clock(intel_crtc);
> >
> > -       if (intel_crtc->config->has_pch_encoder) {
> > -               lpt_disable_pch_transcoder(dev_priv);
> > -               intel_ddi_fdi_disable(crtc);
> > -       }
> > -
> >         for_each_encoder_on_crtc(dev, crtc, encoder)
> >                 if (encoder->post_disable)
> >                         encoder->post_disable(encoder);
> >
> > -       if (intel_crtc->config->has_pch_encoder)
> > +       if (intel_crtc->config->has_pch_encoder) {
> > +               lpt_disable_pch_transcoder(dev_priv);
> > +               intel_ddi_fdi_disable(crtc);
> > +
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> >                                                       true);
> > +       }
> >
> >         intel_fbc_disable_crtc(intel_crtc);
> >  }
> > --
> > 2.4.10
> >
> 
> 
> 
> -- 
> Paulo Zanoni

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

Re: [PATCH v2 07/10] drm/i915: Disable FDI after the CRT port on LPT-H

[Ville Syrjälä]

On Mon, Dec 07, 2015 at 08:57:59PM +0200, Ville Syrjälä wrote:
> On Mon, Dec 07, 2015 at 03:51:06PM -0200, Paulo Zanoni wrote:
> > 2015-12-04 18:20 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Bspec modeset sequence tells us to disable the PCH transcoder and
> > > FDI after the CRT port on LPT-H, so let's do that. And the CRT port
> > > should be disabled after the pipe, as we do on other PCH platforms
> > > too since
> > > commit 1ea56e269e13 ("drm/i915: Disable CRT port after pipe on PCH platforms")
> > >
> > > commit 00490c22b1b5 ("drm/i915: Consider SPLL as another shared pll, v2.")
> > > moved the SPLL disaable from the .post_disable() hook to some upper
> > 
> > disaable
> > 
> > > laeve code, so we can just move the CRT port disabling into the
> > 
> > laeve
> > 
> > > .post_disable() hook. If we still had the non-shared SPLL, it would have
> > > needed to be moved into the .post_pll_disable() hook.
> > >
> > > v2: Actually move the CRT port disable to the .post_disable() hook,
> > >     and amend the commit message with more details (Paulo)
> > 
> > Since I seem to recall that the DDI disable sequence for CRT was
> > correct at some point in a distant past, I suppose your commit is a
> > regression fix, and maybe we'd like a backportable version. Maybe we
> > could also provide explicit information on the first bad commit.
> 
> Well, even the spec has changed since the early days, at least w.r.t.
> the FDI RX disable, so not sure if something else has changed in there
> as well. I didn't go digging through history to see if we accidentally
> changed the order of some steps at some point.

I tried to look through the hisrtory a bit, but didn't spot any place
where any significant reordering took place (ie. we always seemed to
disable the port before the pipe). So I'm leaning towards the "spec
got changed" theory here, or perhaps the "copy paste" theory since
we did get this order wrong for all earlier PCH platforms as well.

> 
> > 
> > Anyway, now it looks correct:
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > >
> > > Cc: Paulo Zanoni <przanoni@gmail.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_crt.c     |  2 +-
> > >  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
> > >  2 files changed, 6 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> > > index 12008af797bd..cef359958c73 100644
> > > --- a/drivers/gpu/drm/i915/intel_crt.c
> > > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > > @@ -844,7 +844,7 @@ void intel_crt_init(struct drm_device *dev)
> > >         crt->adpa_reg = adpa_reg;
> > >
> > >         crt->base.compute_config = intel_crt_compute_config;
> > > -       if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) {
> > > +       if (HAS_PCH_SPLIT(dev)) {
> > >                 crt->base.disable = pch_disable_crt;
> > >                 crt->base.post_disable = pch_post_disable_crt;
> > >         } else {
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 3391ab0ca752..42ed799390e5 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5166,18 +5166,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> > >         if (!intel_crtc->config->has_dsi_encoder)
> > >                 intel_ddi_disable_pipe_clock(intel_crtc);
> > >
> > > -       if (intel_crtc->config->has_pch_encoder) {
> > > -               lpt_disable_pch_transcoder(dev_priv);
> > > -               intel_ddi_fdi_disable(crtc);
> > > -       }
> > > -
> > >         for_each_encoder_on_crtc(dev, crtc, encoder)
> > >                 if (encoder->post_disable)
> > >                         encoder->post_disable(encoder);
> > >
> > > -       if (intel_crtc->config->has_pch_encoder)
> > > +       if (intel_crtc->config->has_pch_encoder) {
> > > +               lpt_disable_pch_transcoder(dev_priv);
> > > +               intel_ddi_fdi_disable(crtc);
> > > +
> > >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > >                                                       true);
> > > +       }
> > >
> > >         intel_fbc_disable_crtc(intel_crtc);
> > >  }
> > > --
> > > 2.4.10
> > >
> > 
> > 
> > 
> > -- 
> > Paulo Zanoni
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH] drm/i915: Disable FDI after the CRT port on LPT-H

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bspec modeset sequence tells us to disable the PCH transcoder and
FDI after the CRT port on LPT-H, so let's do that. And the CRT port
should be disabled after the pipe, as we do on other PCH platforms
too since
commit 1ea56e269e13 ("drm/i915: Disable CRT port after pipe on PCH platforms")

commit 00490c22b1b5 ("drm/i915: Consider SPLL as another shared pll, v2.")
moved the SPLL disable from the .post_disable() hook to some upper
level code, so we can just move the CRT port disabling into the
.post_disable() hook. If we still had the non-shared SPLL, it would have
needed to be moved into the .post_pll_disable() hook.

v2: Actually move the CRT port disable to the .post_disable() hook,
    and amend the commit message with more details (Paulo)
v3: Fix typos in commit message (Paulo)

Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 12008af797bd..cef359958c73 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -844,7 +844,7 @@ void intel_crt_init(struct drm_device *dev)
 	crt->adpa_reg = adpa_reg;
 
 	crt->base.compute_config = intel_crt_compute_config;
-	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) {
+	if (HAS_PCH_SPLIT(dev)) {
 		crt->base.disable = pch_disable_crt;
 		crt->base.post_disable = pch_post_disable_crt;
 	} else {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7e3afcc62997..429c3df99e48 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5164,18 +5164,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->config->has_dsi_encoder)
 		intel_ddi_disable_pipe_clock(intel_crtc);
 
-	if (intel_crtc->config->has_pch_encoder) {
-		lpt_disable_pch_transcoder(dev_priv);
-		intel_ddi_fdi_disable(crtc);
-	}
-
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->post_disable)
 			encoder->post_disable(encoder);
 
-	if (intel_crtc->config->has_pch_encoder)
+	if (intel_crtc->config->has_pch_encoder) {
+		lpt_disable_pch_transcoder(dev_priv);
+		intel_ddi_fdi_disable(crtc);
+
 		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
 						      true);
+	}
 
 	intel_fbc_disable_crtc(intel_crtc);
 }
-- 
2.4.10

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

[PATCH 08/10] drm/i915: Refactor LPT-H VGA dotclock disabling

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract the LPT-H VGA dotclock disable to a separate function in
anticipation of further use.

While at it move the sb_lock locking inwards when enabling the VGA
dotclock, as it's only needed to protect the sideband accesses.

Also toss out the PIXCLK_GATE_GATE nop define and just use 0.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  1 -
 drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++--------------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fcc819f400a6..3a9819735833 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7342,7 +7342,6 @@ enum skl_disp_power_wells {
 /* LPT PIXCLK_GATE */
 #define PIXCLK_GATE			_MMIO(0xC6020)
 #define  PIXCLK_GATE_UNGATE		(1<<0)
-#define  PIXCLK_GATE_GATE		(0<<0)
 
 /* SPLL */
 #define SPLL_CTL			_MMIO(0x46020)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1dc125b6dcdc..322a35c67870 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3938,6 +3938,21 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 	return 0;
 }
 
+static void lpt_disable_iclkip(struct drm_i915_private *dev_priv)
+{
+	u32 temp;
+
+	I915_WRITE(PIXCLK_GATE, 0);
+
+	mutex_lock(&dev_priv->sb_lock);
+
+	temp = intel_sbi_read(dev_priv, SBI_SSCCTL6, SBI_ICLK);
+	temp |= SBI_SSCCTL_DISABLE;
+	intel_sbi_write(dev_priv, SBI_SSCCTL6, temp, SBI_ICLK);
+
+	mutex_unlock(&dev_priv->sb_lock);
+}
+
 /* Program iCLKIP clock to the desired frequency */
 static void lpt_program_iclkip(struct drm_crtc *crtc)
 {
@@ -3947,18 +3962,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 	u32 divsel, phaseinc, auxdiv, phasedir = 0;
 	u32 temp;
 
-	mutex_lock(&dev_priv->sb_lock);
-
-	/* It is necessary to ungate the pixclk gate prior to programming
-	 * the divisors, and gate it back when it is done.
-	 */
-	I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_GATE);
-
-	/* Disable SSCCTL */
-	intel_sbi_write(dev_priv, SBI_SSCCTL6,
-			intel_sbi_read(dev_priv, SBI_SSCCTL6, SBI_ICLK) |
-				SBI_SSCCTL_DISABLE,
-			SBI_ICLK);
+	lpt_disable_iclkip(dev_priv);
 
 	/* 20MHz is a corner case which is out of range for the 7-bit divisor */
 	if (clock == 20000) {
@@ -3998,6 +4002,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 			phasedir,
 			phaseinc);
 
+	mutex_lock(&dev_priv->sb_lock);
+
 	/* Program SSCDIVINTPHASE6 */
 	temp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE6, SBI_ICLK);
 	temp &= ~SBI_SSCDIVINTPHASE_DIVSEL_MASK;
@@ -4019,12 +4025,12 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 	temp &= ~SBI_SSCCTL_DISABLE;
 	intel_sbi_write(dev_priv, SBI_SSCCTL6, temp, SBI_ICLK);
 
+	mutex_unlock(&dev_priv->sb_lock);
+
 	/* Wait for initialization time */
 	udelay(24);
 
 	I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_UNGATE);
-
-	mutex_unlock(&dev_priv->sb_lock);
 }
 
 static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
-- 
2.4.10

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

Re: [PATCH 08/10] drm/i915: Refactor LPT-H VGA dotclock disabling

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extract the LPT-H VGA dotclock disable to a separate function in
> anticipation of further use.
>
> While at it move the sb_lock locking inwards when enabling the VGA
> dotclock, as it's only needed to protect the sideband accesses.
>
> Also toss out the PIXCLK_GATE_GATE nop define and just use 0.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  1 -
>  drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++--------------
>  2 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fcc819f400a6..3a9819735833 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7342,7 +7342,6 @@ enum skl_disp_power_wells {
>  /* LPT PIXCLK_GATE */
>  #define PIXCLK_GATE                    _MMIO(0xC6020)
>  #define  PIXCLK_GATE_UNGATE            (1<<0)
> -#define  PIXCLK_GATE_GATE              (0<<0)

I actually liked the #define...

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
>  /* SPLL */
>  #define SPLL_CTL                       _MMIO(0x46020)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1dc125b6dcdc..322a35c67870 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3938,6 +3938,21 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
>         return 0;
>  }
>
> +static void lpt_disable_iclkip(struct drm_i915_private *dev_priv)
> +{
> +       u32 temp;
> +
> +       I915_WRITE(PIXCLK_GATE, 0);
> +
> +       mutex_lock(&dev_priv->sb_lock);
> +
> +       temp = intel_sbi_read(dev_priv, SBI_SSCCTL6, SBI_ICLK);
> +       temp |= SBI_SSCCTL_DISABLE;
> +       intel_sbi_write(dev_priv, SBI_SSCCTL6, temp, SBI_ICLK);
> +
> +       mutex_unlock(&dev_priv->sb_lock);
> +}
> +
>  /* Program iCLKIP clock to the desired frequency */
>  static void lpt_program_iclkip(struct drm_crtc *crtc)
>  {
> @@ -3947,18 +3962,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
>         u32 divsel, phaseinc, auxdiv, phasedir = 0;
>         u32 temp;
>
> -       mutex_lock(&dev_priv->sb_lock);
> -
> -       /* It is necessary to ungate the pixclk gate prior to programming
> -        * the divisors, and gate it back when it is done.
> -        */
> -       I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_GATE);
> -
> -       /* Disable SSCCTL */
> -       intel_sbi_write(dev_priv, SBI_SSCCTL6,
> -                       intel_sbi_read(dev_priv, SBI_SSCCTL6, SBI_ICLK) |
> -                               SBI_SSCCTL_DISABLE,
> -                       SBI_ICLK);
> +       lpt_disable_iclkip(dev_priv);
>
>         /* 20MHz is a corner case which is out of range for the 7-bit divisor */
>         if (clock == 20000) {
> @@ -3998,6 +4002,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
>                         phasedir,
>                         phaseinc);
>
> +       mutex_lock(&dev_priv->sb_lock);
> +
>         /* Program SSCDIVINTPHASE6 */
>         temp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE6, SBI_ICLK);
>         temp &= ~SBI_SSCDIVINTPHASE_DIVSEL_MASK;
> @@ -4019,12 +4025,12 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
>         temp &= ~SBI_SSCCTL_DISABLE;
>         intel_sbi_write(dev_priv, SBI_SSCCTL6, temp, SBI_ICLK);
>
> +       mutex_unlock(&dev_priv->sb_lock);
> +
>         /* Wait for initialization time */
>         udelay(24);
>
>         I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_UNGATE);
> -
> -       mutex_unlock(&dev_priv->sb_lock);
>  }
>
>  static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 08/10] drm/i915: Refactor LPT-H VGA dotclock disabling

[Ville Syrjälä]

On Wed, Dec 02, 2015 at 02:56:01PM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Extract the LPT-H VGA dotclock disable to a separate function in
> > anticipation of further use.
> >
> > While at it move the sb_lock locking inwards when enabling the VGA
> > dotclock, as it's only needed to protect the sideband accesses.
> >
> > Also toss out the PIXCLK_GATE_GATE nop define and just use 0.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  1 -
> >  drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++--------------
> >  2 files changed, 20 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index fcc819f400a6..3a9819735833 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7342,7 +7342,6 @@ enum skl_disp_power_wells {
> >  /* LPT PIXCLK_GATE */
> >  #define PIXCLK_GATE                    _MMIO(0xC6020)
> >  #define  PIXCLK_GATE_UNGATE            (1<<0)
> > -#define  PIXCLK_GATE_GATE              (0<<0)
> 
> I actually liked the #define...

I can keep it if you prefer. Just personally not too fond of defines for
zeroes.

> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> >
> >  /* SPLL */
> >  #define SPLL_CTL                       _MMIO(0x46020)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1dc125b6dcdc..322a35c67870 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3938,6 +3938,21 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
> >         return 0;
> >  }
> >
> > +static void lpt_disable_iclkip(struct drm_i915_private *dev_priv)
> > +{
> > +       u32 temp;
> > +
> > +       I915_WRITE(PIXCLK_GATE, 0);
> > +
> > +       mutex_lock(&dev_priv->sb_lock);
> > +
> > +       temp = intel_sbi_read(dev_priv, SBI_SSCCTL6, SBI_ICLK);
> > +       temp |= SBI_SSCCTL_DISABLE;
> > +       intel_sbi_write(dev_priv, SBI_SSCCTL6, temp, SBI_ICLK);
> > +
> > +       mutex_unlock(&dev_priv->sb_lock);
> > +}
> > +
> >  /* Program iCLKIP clock to the desired frequency */
> >  static void lpt_program_iclkip(struct drm_crtc *crtc)
> >  {
> > @@ -3947,18 +3962,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
> >         u32 divsel, phaseinc, auxdiv, phasedir = 0;
> >         u32 temp;
> >
> > -       mutex_lock(&dev_priv->sb_lock);
> > -
> > -       /* It is necessary to ungate the pixclk gate prior to programming
> > -        * the divisors, and gate it back when it is done.
> > -        */
> > -       I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_GATE);
> > -
> > -       /* Disable SSCCTL */
> > -       intel_sbi_write(dev_priv, SBI_SSCCTL6,
> > -                       intel_sbi_read(dev_priv, SBI_SSCCTL6, SBI_ICLK) |
> > -                               SBI_SSCCTL_DISABLE,
> > -                       SBI_ICLK);
> > +       lpt_disable_iclkip(dev_priv);
> >
> >         /* 20MHz is a corner case which is out of range for the 7-bit divisor */
> >         if (clock == 20000) {
> > @@ -3998,6 +4002,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
> >                         phasedir,
> >                         phaseinc);
> >
> > +       mutex_lock(&dev_priv->sb_lock);
> > +
> >         /* Program SSCDIVINTPHASE6 */
> >         temp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE6, SBI_ICLK);
> >         temp &= ~SBI_SSCDIVINTPHASE_DIVSEL_MASK;
> > @@ -4019,12 +4025,12 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
> >         temp &= ~SBI_SSCCTL_DISABLE;
> >         intel_sbi_write(dev_priv, SBI_SSCCTL6, temp, SBI_ICLK);
> >
> > +       mutex_unlock(&dev_priv->sb_lock);
> > +
> >         /* Wait for initialization time */
> >         udelay(24);
> >
> >         I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_UNGATE);
> > -
> > -       mutex_unlock(&dev_priv->sb_lock);
> >  }
> >
> >  static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

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

[PATCH v2 08/10] drm/i915: Refactor LPT-H VGA dotclock disabling

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract the LPT-H VGA dotclock disable to a separate function in
anticipation of further use.

While at it move the sb_lock locking inwards when enabling the VGA
dotclock, as it's only needed to protect the sideband accesses.

v2: Keep the PIXCLK_GATE_GATE name for 0 (Paulo)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 42ed799390e5..a92753c1ffe3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3940,6 +3940,21 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 	return 0;
 }
 
+static void lpt_disable_iclkip(struct drm_i915_private *dev_priv)
+{
+	u32 temp;
+
+	I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_GATE);
+
+	mutex_lock(&dev_priv->sb_lock);
+
+	temp = intel_sbi_read(dev_priv, SBI_SSCCTL6, SBI_ICLK);
+	temp |= SBI_SSCCTL_DISABLE;
+	intel_sbi_write(dev_priv, SBI_SSCCTL6, temp, SBI_ICLK);
+
+	mutex_unlock(&dev_priv->sb_lock);
+}
+
 /* Program iCLKIP clock to the desired frequency */
 static void lpt_program_iclkip(struct drm_crtc *crtc)
 {
@@ -3949,18 +3964,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 	u32 divsel, phaseinc, auxdiv, phasedir = 0;
 	u32 temp;
 
-	mutex_lock(&dev_priv->sb_lock);
-
-	/* It is necessary to ungate the pixclk gate prior to programming
-	 * the divisors, and gate it back when it is done.
-	 */
-	I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_GATE);
-
-	/* Disable SSCCTL */
-	intel_sbi_write(dev_priv, SBI_SSCCTL6,
-			intel_sbi_read(dev_priv, SBI_SSCCTL6, SBI_ICLK) |
-				SBI_SSCCTL_DISABLE,
-			SBI_ICLK);
+	lpt_disable_iclkip(dev_priv);
 
 	/* 20MHz is a corner case which is out of range for the 7-bit divisor */
 	if (clock == 20000) {
@@ -4000,6 +4004,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 			phasedir,
 			phaseinc);
 
+	mutex_lock(&dev_priv->sb_lock);
+
 	/* Program SSCDIVINTPHASE6 */
 	temp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE6, SBI_ICLK);
 	temp &= ~SBI_SSCDIVINTPHASE_DIVSEL_MASK;
@@ -4021,12 +4027,12 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 	temp &= ~SBI_SSCCTL_DISABLE;
 	intel_sbi_write(dev_priv, SBI_SSCCTL6, temp, SBI_ICLK);
 
+	mutex_unlock(&dev_priv->sb_lock);
+
 	/* Wait for initialization time */
 	udelay(24);
 
 	I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_UNGATE);
-
-	mutex_unlock(&dev_priv->sb_lock);
 }
 
 static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
-- 
2.4.10

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

[PATCH 09/10] drm/i915: Disable LPT-H VGA dotclock during crtc disable

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we leave the LPT-H VGA dotclock running after turning
the pipe/fdi/port/etc. Propoerly disable the VGA dotclock as
specified in the modeset sequence.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 322a35c67870..5e74456a90aa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5171,6 +5171,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	if (intel_crtc->config->has_pch_encoder) {
 		lpt_disable_pch_transcoder(dev_priv);
+		lpt_disable_iclkip(dev_priv);
 		intel_ddi_fdi_disable(crtc);
 
 		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-- 
2.4.10

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

Re: [PATCH 09/10] drm/i915: Disable LPT-H VGA dotclock during crtc disable

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we leave the LPT-H VGA dotclock running after turning
> the pipe/fdi/port/etc. Propoerly disable the VGA dotclock as

s/Propoerly/Properly/

(DId you also notice that steps 13 and 18 are the same?)

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> specified in the modeset sequence.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 322a35c67870..5e74456a90aa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5171,6 +5171,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>
>         if (intel_crtc->config->has_pch_encoder) {
>                 lpt_disable_pch_transcoder(dev_priv);
> +               lpt_disable_iclkip(dev_priv);
>                 intel_ddi_fdi_disable(crtc);
>
>                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 09/10] drm/i915: Disable LPT-H VGA dotclock during crtc disable

[Ville Syrjälä]

On Wed, Dec 02, 2015 at 03:02:20PM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently we leave the LPT-H VGA dotclock running after turning
> > the pipe/fdi/port/etc. Propoerly disable the VGA dotclock as
> 
> s/Propoerly/Properly/
> 
> (DId you also notice that steps 13 and 18 are the same?)

No I didn't. That's a bit weird. I did notice that there seems to be an
off by one when it talks about dealing with FDI training failure. It
says:
"To retry FDI training, follow the Disable Sequence steps to Disable FDI,
but skip the steps related to clocks and PLLs (16, 19, and 20), ..."

But actually it should says "17, 20, and 21"

Which now makes me wonder that maybe they meant to move the FDI RX
disable to step 13 at some point, but simply forgot to remove it
from the old place in the sequence. That could explain the off by
one, and it would be more symmetrical with the enable sequence. We
seem to do the FDI RX disable at step 18 currently.

> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> > specified in the modeset sequence.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 322a35c67870..5e74456a90aa 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5171,6 +5171,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> >
> >         if (intel_crtc->config->has_pch_encoder) {
> >                 lpt_disable_pch_transcoder(dev_priv);
> > +               lpt_disable_iclkip(dev_priv);
> >                 intel_ddi_fdi_disable(crtc);
> >
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

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

[PATCH v2 09/10] drm/i915: Disable LPT-H VGA dotclock during crtc disable

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we leave the LPT-H VGA dotclock running after turning
the pipe/fdi/port/etc. Properly disable the VGA dotclock as
specified in the modeset sequence.

v2: Fix commit message typo (Paulo)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a92753c1ffe3..796378352125 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5178,6 +5178,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	if (intel_crtc->config->has_pch_encoder) {
 		lpt_disable_pch_transcoder(dev_priv);
+		lpt_disable_iclkip(dev_priv);
 		intel_ddi_fdi_disable(crtc);
 
 		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
-- 
2.4.10

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

[PATCH 10/10] drm/i915: Leave FDI running after failed link training on LPT-H

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we disable some parts of FDI setup after a failed link
training. But despite that we continue with the modeset as if everything
is fine. This results in tons of noise from the state checker, and
it means we're not following the proper modeset sequence for the rest of
crtc enabling, nor for crtc disabling.

Ideally we should abort the modeset and follow the proper disable
suquenced to shut off everything we enabled so far, but that would
require a big rework of the modeset code. So instead just leave FDI
up and running in its untrained state, and log an error. This is
what we do on older platforms too.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 76ce7c2960b6..b312a47a0654 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -675,15 +675,16 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 		temp = I915_READ(DP_TP_STATUS(PORT_E));
 		if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
 			DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
+			break;
+		}
 
-			/* Enable normal pixel sending for FDI */
-			I915_WRITE(DP_TP_CTL(PORT_E),
-				   DP_TP_CTL_FDI_AUTOTRAIN |
-				   DP_TP_CTL_LINK_TRAIN_NORMAL |
-				   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
-				   DP_TP_CTL_ENABLE);
-
-			return;
+		/*
+		 * Leave things enabled even if we failed to train FDI.
+		 * Results in less fireworks from the state checker.
+		 */
+		if (i == ARRAY_SIZE(hsw_ddi_translations_fdi) * 2 - 1) {
+			DRM_ERROR("FDI link training failed!\n");
+			break;
 		}
 
 		temp = I915_READ(DDI_BUF_CTL(PORT_E));
@@ -712,7 +713,12 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 		POSTING_READ(FDI_RX_MISC(PIPE_A));
 	}
 
-	DRM_ERROR("FDI link training failed!\n");
+	/* Enable normal pixel sending for FDI */
+	I915_WRITE(DP_TP_CTL(PORT_E),
+		   DP_TP_CTL_FDI_AUTOTRAIN |
+		   DP_TP_CTL_LINK_TRAIN_NORMAL |
+		   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
+		   DP_TP_CTL_ENABLE);
 }
 
 void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
-- 
2.4.10

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

Re: [PATCH 10/10] drm/i915: Leave FDI running after failed link training on LPT-H

[Paulo Zanoni]

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we disable some parts of FDI setup after a failed link
> training. But despite that we continue with the modeset as if everything
> is fine. This results in tons of noise from the state checker, and
> it means we're not following the proper modeset sequence for the rest of
> crtc enabling, nor for crtc disabling.
>
> Ideally we should abort the modeset and follow the proper disable
> suquenced to shut off everything we enabled so far, but that would
> require a big rework of the modeset code. So instead just leave FDI
> up and running in its untrained state, and log an error. This is
> what we do on older platforms too.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 76ce7c2960b6..b312a47a0654 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -675,15 +675,16 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>                 temp = I915_READ(DP_TP_STATUS(PORT_E));
>                 if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
>                         DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
> +                       break;
> +               }
>
> -                       /* Enable normal pixel sending for FDI */
> -                       I915_WRITE(DP_TP_CTL(PORT_E),
> -                                  DP_TP_CTL_FDI_AUTOTRAIN |
> -                                  DP_TP_CTL_LINK_TRAIN_NORMAL |
> -                                  DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> -                                  DP_TP_CTL_ENABLE);
> -
> -                       return;
> +               /*
> +                * Leave things enabled even if we failed to train FDI.
> +                * Results in less fireworks from the state checker.
> +                */
> +               if (i == ARRAY_SIZE(hsw_ddi_translations_fdi) * 2 - 1) {
> +                       DRM_ERROR("FDI link training failed!\n");
> +                       break;
>                 }
>
>                 temp = I915_READ(DDI_BUF_CTL(PORT_E));
> @@ -712,7 +713,12 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>                 POSTING_READ(FDI_RX_MISC(PIPE_A));
>         }
>
> -       DRM_ERROR("FDI link training failed!\n");
> +       /* Enable normal pixel sending for FDI */
> +       I915_WRITE(DP_TP_CTL(PORT_E),
> +                  DP_TP_CTL_FDI_AUTOTRAIN |
> +                  DP_TP_CTL_LINK_TRAIN_NORMAL |
> +                  DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> +                  DP_TP_CTL_ENABLE);
>  }
>
>  void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 10/10] drm/i915: Leave FDI running after failed link training on LPT-H

[Ville Syrjälä]

On Wed, Dec 02, 2015 at 03:16:56PM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently we disable some parts of FDI setup after a failed link
> > training. But despite that we continue with the modeset as if everything
> > is fine. This results in tons of noise from the state checker, and
> > it means we're not following the proper modeset sequence for the rest of
> > crtc enabling, nor for crtc disabling.
> >
> > Ideally we should abort the modeset and follow the proper disable
> > suquenced to shut off everything we enabled so far, but that would
    ^^^^^^^^^
And I spotted another typo, which I'll fix.

> > require a big rework of the modeset code. So instead just leave FDI
> > up and running in its untrained state, and log an error. This is
> > what we do on older platforms too.
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
> >  1 file changed, 15 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 76ce7c2960b6..b312a47a0654 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -675,15 +675,16 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
> >                 temp = I915_READ(DP_TP_STATUS(PORT_E));
> >                 if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
> >                         DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
> > +                       break;
> > +               }
> >
> > -                       /* Enable normal pixel sending for FDI */
> > -                       I915_WRITE(DP_TP_CTL(PORT_E),
> > -                                  DP_TP_CTL_FDI_AUTOTRAIN |
> > -                                  DP_TP_CTL_LINK_TRAIN_NORMAL |
> > -                                  DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> > -                                  DP_TP_CTL_ENABLE);
> > -
> > -                       return;
> > +               /*
> > +                * Leave things enabled even if we failed to train FDI.
> > +                * Results in less fireworks from the state checker.
> > +                */
> > +               if (i == ARRAY_SIZE(hsw_ddi_translations_fdi) * 2 - 1) {
> > +                       DRM_ERROR("FDI link training failed!\n");
> > +                       break;
> >                 }
> >
> >                 temp = I915_READ(DDI_BUF_CTL(PORT_E));
> > @@ -712,7 +713,12 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
> >                 POSTING_READ(FDI_RX_MISC(PIPE_A));
> >         }
> >
> > -       DRM_ERROR("FDI link training failed!\n");
> > +       /* Enable normal pixel sending for FDI */
> > +       I915_WRITE(DP_TP_CTL(PORT_E),
> > +                  DP_TP_CTL_FDI_AUTOTRAIN |
> > +                  DP_TP_CTL_LINK_TRAIN_NORMAL |
> > +                  DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> > +                  DP_TP_CTL_ENABLE);
> >  }
> >
> >  void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

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

Re: [PATCH 10/10] drm/i915: Leave FDI running after failed link training on LPT-H

[Daniel Vetter]

On Tue, Dec 01, 2015 at 03:08:41PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we disable some parts of FDI setup after a failed link
> training. But despite that we continue with the modeset as if everything
> is fine. This results in tons of noise from the state checker, and
> it means we're not following the proper modeset sequence for the rest of
> crtc enabling, nor for crtc disabling.
> 
> Ideally we should abort the modeset and follow the proper disable
> suquenced to shut off everything we enabled so far, but that would
> require a big rework of the modeset code. So instead just leave FDI
> up and running in its untrained state, and log an error. This is
> what we do on older platforms too.

No, ideally we should light the pipe up and tell userspace through some
uevent that something bad has happened. Especially with async commits
failing to light up the pipe looks to userspace like the kernel just
randomly disabled it. And that usually results in random crashes when the
next flip or vblank wait fails and everything comes crashing down. And
even if we wouldn't -EINVAL these userspace would still have a problem
since the flip or vblank it wants to wait for simply never happens and
then it's just stalled.

We've had tons of problems with this in the past, and therefore removed
all the in-kernel sources for shutting down a pipe. E.g. DP re-training
also forces the pipe to on again, even if it fails.

Yup dp mst is an exception and I need to fix it eventually.

Cheers, Daniel
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 76ce7c2960b6..b312a47a0654 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -675,15 +675,16 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  		temp = I915_READ(DP_TP_STATUS(PORT_E));
>  		if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
>  			DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
> +			break;
> +		}
>  
> -			/* Enable normal pixel sending for FDI */
> -			I915_WRITE(DP_TP_CTL(PORT_E),
> -				   DP_TP_CTL_FDI_AUTOTRAIN |
> -				   DP_TP_CTL_LINK_TRAIN_NORMAL |
> -				   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> -				   DP_TP_CTL_ENABLE);
> -
> -			return;
> +		/*
> +		 * Leave things enabled even if we failed to train FDI.
> +		 * Results in less fireworks from the state checker.
> +		 */
> +		if (i == ARRAY_SIZE(hsw_ddi_translations_fdi) * 2 - 1) {
> +			DRM_ERROR("FDI link training failed!\n");
> +			break;
>  		}
>  
>  		temp = I915_READ(DDI_BUF_CTL(PORT_E));
> @@ -712,7 +713,12 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  		POSTING_READ(FDI_RX_MISC(PIPE_A));
>  	}
>  
> -	DRM_ERROR("FDI link training failed!\n");
> +	/* Enable normal pixel sending for FDI */
> +	I915_WRITE(DP_TP_CTL(PORT_E),
> +		   DP_TP_CTL_FDI_AUTOTRAIN |
> +		   DP_TP_CTL_LINK_TRAIN_NORMAL |
> +		   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> +		   DP_TP_CTL_ENABLE);
>  }
>  
>  void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Leave FDI running after failed link training on LPT-H

[Ville Syrjälä]

On Fri, Dec 04, 2015 at 11:09:11AM +0100, Daniel Vetter wrote:
> On Tue, Dec 01, 2015 at 03:08:41PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we disable some parts of FDI setup after a failed link
> > training. But despite that we continue with the modeset as if everything
> > is fine. This results in tons of noise from the state checker, and
> > it means we're not following the proper modeset sequence for the rest of
> > crtc enabling, nor for crtc disabling.
> > 
> > Ideally we should abort the modeset and follow the proper disable
> > suquenced to shut off everything we enabled so far, but that would
> > require a big rework of the modeset code. So instead just leave FDI
> > up and running in its untrained state, and log an error. This is
> > what we do on older platforms too.
> 
> No, ideally we should light the pipe up and tell userspace through some
> uevent that something bad has happened.

That's not ideal from the hardware POV, and it's rather against the spec
even. But this patch does exactly that.

> Especially with async commits
> failing to light up the pipe looks to userspace like the kernel just
> randomly disabled it. And that usually results in random crashes when the
> next flip or vblank wait fails and everything comes crashing down. And
> even if we wouldn't -EINVAL these userspace would still have a problem
> since the flip or vblank it wants to wait for simply never happens and
> then it's just stalled.
> 
> We've had tons of problems with this in the past, and therefore removed
> all the in-kernel sources for shutting down a pipe. E.g. DP re-training
> also forces the pipe to on again, even if it fails.
> 
> Yup dp mst is an exception and I need to fix it eventually.
> 
> Cheers, Daniel
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
> >  1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 76ce7c2960b6..b312a47a0654 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -675,15 +675,16 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
> >  		temp = I915_READ(DP_TP_STATUS(PORT_E));
> >  		if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
> >  			DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
> > +			break;
> > +		}
> >  
> > -			/* Enable normal pixel sending for FDI */
> > -			I915_WRITE(DP_TP_CTL(PORT_E),
> > -				   DP_TP_CTL_FDI_AUTOTRAIN |
> > -				   DP_TP_CTL_LINK_TRAIN_NORMAL |
> > -				   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> > -				   DP_TP_CTL_ENABLE);
> > -
> > -			return;
> > +		/*
> > +		 * Leave things enabled even if we failed to train FDI.
> > +		 * Results in less fireworks from the state checker.
> > +		 */
> > +		if (i == ARRAY_SIZE(hsw_ddi_translations_fdi) * 2 - 1) {
> > +			DRM_ERROR("FDI link training failed!\n");
> > +			break;
> >  		}
> >  
> >  		temp = I915_READ(DDI_BUF_CTL(PORT_E));
> > @@ -712,7 +713,12 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
> >  		POSTING_READ(FDI_RX_MISC(PIPE_A));
> >  	}
> >  
> > -	DRM_ERROR("FDI link training failed!\n");
> > +	/* Enable normal pixel sending for FDI */
> > +	I915_WRITE(DP_TP_CTL(PORT_E),
> > +		   DP_TP_CTL_FDI_AUTOTRAIN |
> > +		   DP_TP_CTL_LINK_TRAIN_NORMAL |
> > +		   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> > +		   DP_TP_CTL_ENABLE);
> >  }
> >  
> >  void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

[PATCH v2 10/10] drm/i915: Leave FDI running after failed link training on LPT-H

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we disable some parts of FDI setup after a failed link
training. But despite that we continue with the modeset as if everything
is fine. This results in tons of noise from the state checker, and
it means we're not following the proper modeset sequence for the rest of
crtc enabling, nor for crtc disabling.

Ideally we should abort the modeset and follow the proper disable
sequence to shut off everything we enabled so far, but that would
require a big rework of the modeset code. So instead just leave FDI
up and running in its untrained state, and log an error. This is
what we do on older platforms too.

v2: Fix a typo in the commit message

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7f618cf5289c..5d20c64d8566 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -675,15 +675,16 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 		temp = I915_READ(DP_TP_STATUS(PORT_E));
 		if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
 			DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
+			break;
+		}
 
-			/* Enable normal pixel sending for FDI */
-			I915_WRITE(DP_TP_CTL(PORT_E),
-				   DP_TP_CTL_FDI_AUTOTRAIN |
-				   DP_TP_CTL_LINK_TRAIN_NORMAL |
-				   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
-				   DP_TP_CTL_ENABLE);
-
-			return;
+		/*
+		 * Leave things enabled even if we failed to train FDI.
+		 * Results in less fireworks from the state checker.
+		 */
+		if (i == ARRAY_SIZE(hsw_ddi_translations_fdi) * 2 - 1) {
+			DRM_ERROR("FDI link training failed!\n");
+			break;
 		}
 
 		temp = I915_READ(DDI_BUF_CTL(PORT_E));
@@ -712,7 +713,12 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 		POSTING_READ(FDI_RX_MISC(PIPE_A));
 	}
 
-	DRM_ERROR("FDI link training failed!\n");
+	/* Enable normal pixel sending for FDI */
+	I915_WRITE(DP_TP_CTL(PORT_E),
+		   DP_TP_CTL_FDI_AUTOTRAIN |
+		   DP_TP_CTL_LINK_TRAIN_NORMAL |
+		   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
+		   DP_TP_CTL_ENABLE);
 }
 
 void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
-- 
2.4.10

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

Re: [PATCH 00/10] HSW/BDW PCH modeset fixes and stuff

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 03:08:31PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I was trying to get to the bottom of the FDI fails on Brix Pro, and
> thus eneded up going through the entire PCH modeset stuff for HSW.
> While I did find a bunch of stuff missing/wrong, sadly I wasn't able
> to make FDI training succeed on that machine. So in the end I simply
> had to resort to getting rid of the CRT connector. And to add insult
> to injury, the only way I could do that is to look at the VBT :(
> 
> Anwyay, here are the fruits of my labor. The whole thing is available
> here:
> git://github.com/vsyrjala/linux.git hsw_pch_fixes
> 
> Ville Syrjälä (10):
>   drm/i915: Don't register the CRT connector when it's fused off
>   drm/i915: Don't register CRT connectro when DDI E can't be used
>   drm/i915: Check VBT for CRT port presence on HSW/BDW
>   drm/i915: Add "missing" break to haswell_get_ddi_pll()
>   drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed
>   drm/i915: Round to closest when computing the VGA dotclock for LPT-H
>   drm/i915: Disable FDI after the CRT port on LPT-H
>   drm/i915: Refactor LPT-H VGA dotclock disabling
>   drm/i915: Disable LPT-H VGA dotclock during crtc disable
>   drm/i915: Leave FDI running after failed link training on LPT-H

Entire series now pushed to dinq. Thanks for the reviews.

> 
>  drivers/gpu/drm/i915/i915_reg.h      |   4 +-
>  drivers/gpu/drm/i915/intel_ddi.c     |  24 ++++---
>  drivers/gpu/drm/i915/intel_display.c | 123 ++++++++++++++++++++++++++++-------
>  3 files changed, 117 insertions(+), 34 deletions(-)
> 
> -- 
> 2.4.10

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