All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
@ 2018-05-04 14:13 Ville Syrjala
  2018-05-04 14:13 ` [PATCH 2/2] drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt Ville Syrjala
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Ville Syrjala @ 2018-05-04 14:13 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Ondrej Zary

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

VBT seems to have some bits to tell us whether the internal LVDS port
has something hooked up. In theory one might expect the VBT to not have
a child device for the LVDS port if there's no panel hooked up, but
in practice many VBTs still add the child device. The "LVDS config" bits
seem more reliable though, so let's check those.

So far we've used the "LVDS config" bits to check for eDP support on
ILK+, and disable the internal LVDS when the value is 3. That value
is actually documented as "Both internal LVDS and SDVO LVDS", but in
practice it looks to mean "eDP" on all the ilk+ VBTs I've seen. So let's
keep that interpretation, but for pre-ILK we will consider the value
3 to also indicate the presence of the internal LVDS.

Currently we have 25 DMI matches for the "no internal LVDS" quirk. In an
effort to reduce that let's toss in a WARN when the DMI match and VBT
both tell us that the internal LVDS is not present. The hope is that
people will report a bug, and then we can just nuke the corresponding
entry from the DMI quirk list. Credits to Jani for this idea.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ondrej Zary <linux@rainbow-software.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105468
References: https://lists.freedesktop.org/archives/intel-gfx/2018-March/158408.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  1 +
 drivers/gpu/drm/i915/intel_bios.c     | 20 ++++++++++++++++++--
 drivers/gpu/drm/i915/intel_lvds.c     | 14 +++++++++-----
 drivers/gpu/drm/i915/intel_vbt_defs.h |  1 +
 4 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 04e27806e581..ffd0c8d33336 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1052,6 +1052,7 @@ struct intel_vbt_data {
 	unsigned int lvds_vbt:1;
 	unsigned int int_crt_support:1;
 	unsigned int lvds_use_ssc:1;
+	unsigned int int_lvds_support:1;
 	unsigned int display_clock_mode:1;
 	unsigned int fdi_rx_polarity_inverted:1;
 	unsigned int panel_type:4;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 702d3fab97fc..f75541b11f11 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -518,8 +518,21 @@ parse_driver_features(struct drm_i915_private *dev_priv,
 	if (!driver)
 		return;
 
-	if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
-		dev_priv->vbt.edp.support = 1;
+	if (INTEL_GEN(dev_priv) >= 5) {
+		if (driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS)
+			dev_priv->vbt.int_lvds_support = 0;
+
+		/*
+		 * Note that the VBT spec doesn't agree with this
+		 * interpretation, but real world VBTs seem to.
+		 */
+		if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
+			dev_priv->vbt.edp.support = 1;
+	} else {
+		if (driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS &&
+		    driver->lvds_config != BDB_DRIVER_FEATURE_INT_SDVO_LVDS)
+			dev_priv->vbt.int_lvds_support = 0;
+	}
 
 	DRM_DEBUG_KMS("DRRS State Enabled:%d\n", driver->drrs_enabled);
 	/*
@@ -1512,6 +1525,9 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
 	dev_priv->vbt.int_tv_support = 1;
 	dev_priv->vbt.int_crt_support = 1;
 
+	/* driver features */
+	dev_priv->vbt.int_lvds_support = 1;
+
 	/* Default to using SSC */
 	dev_priv->vbt.lvds_use_ssc = 1;
 	/*
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d35d2d50f595..f6596c5ce973 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -962,8 +962,16 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 		return;
 
 	/* Skip init on machines we know falsely report LVDS */
-	if (dmi_check_system(intel_no_lvds))
+	if (dmi_check_system(intel_no_lvds)) {
+		WARN(!dev_priv->vbt.int_lvds_support,
+		     "Useless DMI match. Internal LVDS support disabled by VBT\n");
 		return;
+	}
+
+	if (!dev_priv->vbt.int_lvds_support) {
+		DRM_DEBUG_KMS("Internal LVDS support disabled by VBT.\n");
+		return;
+	}
 
 	if (HAS_PCH_SPLIT(dev_priv))
 		lvds_reg = PCH_LVDS;
@@ -975,10 +983,6 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 	if (HAS_PCH_SPLIT(dev_priv)) {
 		if ((lvds & LVDS_DETECTED) == 0)
 			return;
-		if (dev_priv->vbt.edp.support) {
-			DRM_DEBUG_KMS("disable LVDS for eDP support\n");
-			return;
-		}
 	}
 
 	pin = GMBUS_PIN_PANEL;
diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
index 458468237b5f..a1d5e39f9a00 100644
--- a/drivers/gpu/drm/i915/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
@@ -635,6 +635,7 @@ struct bdb_sdvo_lvds_options {
 #define BDB_DRIVER_FEATURE_NO_LVDS		0
 #define BDB_DRIVER_FEATURE_INT_LVDS		1
 #define BDB_DRIVER_FEATURE_SDVO_LVDS		2
+#define BDB_DRIVER_FEATURE_INT_SDVO_LVDS	3
 #define BDB_DRIVER_FEATURE_EDP			3
 
 struct bdb_driver_features {
-- 
2.16.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt
  2018-05-04 14:13 [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Ville Syrjala
@ 2018-05-04 14:13 ` Ville Syrjala
  2018-05-04 20:14   ` Rodrigo Vivi
  2018-05-04 14:33 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjala @ 2018-05-04 14:13 UTC (permalink / raw)
  To: intel-gfx

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

dev_priv->vbt.lvds_vbt is set but never actually used. Kill it.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ffd0c8d33336..8302bd22ddcc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1049,7 +1049,6 @@ struct intel_vbt_data {
 	/* Feature bits */
 	unsigned int int_tv_support:1;
 	unsigned int lvds_dither:1;
-	unsigned int lvds_vbt:1;
 	unsigned int int_crt_support:1;
 	unsigned int lvds_use_ssc:1;
 	unsigned int int_lvds_support:1;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index f75541b11f11..db429eea2e99 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -267,8 +267,6 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 	if (!lvds_lfp_data_ptrs)
 		return;
 
-	dev_priv->vbt.lvds_vbt = 1;
-
 	panel_dvo_timing = get_lvds_dvo_timing(lvds_lfp_data,
 					       lvds_lfp_data_ptrs,
 					       panel_type);
@@ -1516,7 +1514,6 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
 
 	/* LFP panel data */
 	dev_priv->vbt.lvds_dither = 1;
-	dev_priv->vbt.lvds_vbt = 0;
 
 	/* SDVO panel data */
 	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
-- 
2.16.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
  2018-05-04 14:13 [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Ville Syrjala
  2018-05-04 14:13 ` [PATCH 2/2] drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt Ville Syrjala
@ 2018-05-04 14:33 ` Patchwork
  2018-05-04 14:34 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-05-04 14:33 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
URL   : https://patchwork.freedesktop.org/series/42690/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
55f6fb40253e drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
-:32: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#32: 
References: https://lists.freedesktop.org/archives/intel-gfx/2018-March/158408.html

total: 0 errors, 1 warnings, 0 checks, 73 lines checked
1b01d8a4324f drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
  2018-05-04 14:13 [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Ville Syrjala
  2018-05-04 14:13 ` [PATCH 2/2] drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt Ville Syrjala
  2018-05-04 14:33 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Patchwork
@ 2018-05-04 14:34 ` Patchwork
  2018-05-04 14:49 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-05-04 14:34 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
URL   : https://patchwork.freedesktop.org/series/42690/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3653:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3654:16: warning: expression using sizeof(void)

Commit: drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3654:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3653:16: warning: expression using sizeof(void)

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
  2018-05-04 14:13 [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Ville Syrjala
                   ` (2 preceding siblings ...)
  2018-05-04 14:34 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-05-04 14:49 ` Patchwork
  2018-05-04 16:22 ` ✓ Fi.CI.IGT: " Patchwork
  2018-05-08  9:33 ` [PATCH 1/2] " Jani Nikula
  5 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-05-04 14:49 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
URL   : https://patchwork.freedesktop.org/series/42690/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4146 -> Patchwork_8910 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42690/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8910 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@gem_exec_suspend@basic-s4-devices:
      fi-skl-guc:         FAIL (fdo#105900, fdo#104699) -> PASS +1

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         FAIL (fdo#102575) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       DMESG-FAIL (fdo#102614) -> PASS

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104699 https://bugs.freedesktop.org/show_bug.cgi?id=104699
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900


== Participating hosts (39 -> 37) ==

  Additional (1): fi-skl-gvtdvm 
  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4146 -> Patchwork_8910

  CI_DRM_4146: be068d4cb8a8b4709af83ffa993d8fc8889f0230 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4461: f772d9a910130b3aec8efa4f09ed723618fae656 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8910: 1b01d8a4324fe65b5ec49d4356973215bf8369db @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4461: 55207ea5154dfaa6d2c128124c50e3be4f9b6440 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

1b01d8a4324f drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt
55f6fb40253e drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8910/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
  2018-05-04 14:13 [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Ville Syrjala
                   ` (3 preceding siblings ...)
  2018-05-04 14:49 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-05-04 16:22 ` Patchwork
  2018-05-08  9:33 ` [PATCH 1/2] " Jani Nikula
  5 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-05-04 16:22 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
URL   : https://patchwork.freedesktop.org/series/42690/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4146_full -> Patchwork_8910_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8910_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8910_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42690/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8910_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          PASS -> SKIP

    

  === Piglit changes ===

    ==== Possible regressions ====

    spec@!opengl es 3.0@gles-3.0-transform-feedback-uniform-buffer-object:
      {pig-snb-2600}:     NOTRUN -> FAIL +11

    
== Known issues ==

  Here are the changes found in Patchwork_8910_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#100368) +1

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +1

    
    ==== Possible fixes ====

    igt@kms_flip@2x-dpms-vs-vblank-race:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    igt@kms_flip@basic-flip-vs-wf_vblank:
      shard-hsw:          FAIL (fdo#103928) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105707) -> PASS

    igt@kms_rotation_crc@primary-rotation-90:
      shard-apl:          FAIL (fdo#103925) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-kbl 


== Build changes ==

    * Linux: CI_DRM_4146 -> Patchwork_8910

  CI_DRM_4146: be068d4cb8a8b4709af83ffa993d8fc8889f0230 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4461: f772d9a910130b3aec8efa4f09ed723618fae656 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8910: 1b01d8a4324fe65b5ec49d4356973215bf8369db @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4461: 55207ea5154dfaa6d2c128124c50e3be4f9b6440 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8910/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt
  2018-05-04 14:13 ` [PATCH 2/2] drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt Ville Syrjala
@ 2018-05-04 20:14   ` Rodrigo Vivi
  0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2018-05-04 20:14 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, May 04, 2018 at 05:13:49PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> dev_priv->vbt.lvds_vbt is set but never actually used. Kill it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h   | 1 -
>  drivers/gpu/drm/i915/intel_bios.c | 3 ---
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ffd0c8d33336..8302bd22ddcc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1049,7 +1049,6 @@ struct intel_vbt_data {
>  	/* Feature bits */
>  	unsigned int int_tv_support:1;
>  	unsigned int lvds_dither:1;
> -	unsigned int lvds_vbt:1;
>  	unsigned int int_crt_support:1;
>  	unsigned int lvds_use_ssc:1;
>  	unsigned int int_lvds_support:1;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index f75541b11f11..db429eea2e99 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -267,8 +267,6 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
>  	if (!lvds_lfp_data_ptrs)
>  		return;
>  
> -	dev_priv->vbt.lvds_vbt = 1;
> -
>  	panel_dvo_timing = get_lvds_dvo_timing(lvds_lfp_data,
>  					       lvds_lfp_data_ptrs,
>  					       panel_type);
> @@ -1516,7 +1514,6 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>  
>  	/* LFP panel data */
>  	dev_priv->vbt.lvds_dither = 1;
> -	dev_priv->vbt.lvds_vbt = 0;
>  
>  	/* SDVO panel data */
>  	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
> -- 
> 2.16.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present
  2018-05-04 14:13 [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Ville Syrjala
                   ` (4 preceding siblings ...)
  2018-05-04 16:22 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-05-08  9:33 ` Jani Nikula
  5 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2018-05-08  9:33 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Ondrej Zary

On Fri, 04 May 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VBT seems to have some bits to tell us whether the internal LVDS port
> has something hooked up. In theory one might expect the VBT to not have
> a child device for the LVDS port if there's no panel hooked up, but
> in practice many VBTs still add the child device. The "LVDS config" bits
> seem more reliable though, so let's check those.
>
> So far we've used the "LVDS config" bits to check for eDP support on
> ILK+, and disable the internal LVDS when the value is 3. That value
> is actually documented as "Both internal LVDS and SDVO LVDS", but in
> practice it looks to mean "eDP" on all the ilk+ VBTs I've seen. So let's
> keep that interpretation, but for pre-ILK we will consider the value
> 3 to also indicate the presence of the internal LVDS.
>
> Currently we have 25 DMI matches for the "no internal LVDS" quirk. In an
> effort to reduce that let's toss in a WARN when the DMI match and VBT
> both tell us that the internal LVDS is not present. The hope is that
> people will report a bug, and then we can just nuke the corresponding
> entry from the DMI quirk list. Credits to Jani for this idea.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ondrej Zary <linux@rainbow-software.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105468
> References: https://lists.freedesktop.org/archives/intel-gfx/2018-March/158408.html
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>  drivers/gpu/drm/i915/intel_bios.c     | 20 ++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_lvds.c     | 14 +++++++++-----
>  drivers/gpu/drm/i915/intel_vbt_defs.h |  1 +
>  4 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 04e27806e581..ffd0c8d33336 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1052,6 +1052,7 @@ struct intel_vbt_data {
>  	unsigned int lvds_vbt:1;
>  	unsigned int int_crt_support:1;
>  	unsigned int lvds_use_ssc:1;
> +	unsigned int int_lvds_support:1;
>  	unsigned int display_clock_mode:1;
>  	unsigned int fdi_rx_polarity_inverted:1;
>  	unsigned int panel_type:4;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 702d3fab97fc..f75541b11f11 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -518,8 +518,21 @@ parse_driver_features(struct drm_i915_private *dev_priv,
>  	if (!driver)
>  		return;
>  

I like the changes in intel_lvds_init(), particularly the fact that the
eDP check is abstracted away from there. But the stuff below is really
subtle. I'll walk through it with commentary.

> -	if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
> -		dev_priv->vbt.edp.support = 1;
> +	if (INTEL_GEN(dev_priv) >= 5) {

Okay, so the eDP check in intel_lvds_init() is wrapped in
HAS_PCH_SPLIT(), but VLV/CHV don't have LVDS so switching to a gen 5
check here is fine. We also only have eDP since gen 5. So far so good.

> +		if (driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS)
> +			dev_priv->vbt.int_lvds_support = 0;

Here's the change for IBX/CPT (the only gen 5+ that can have LVDS). It
used to be that value 3 disabled LVDS, but now 0, 2 and 3 disable
LVDS. It stands to reason 0 and 2 disable LVDS, but looking at the spec,
it seems odd that value 3 disables LVDS...

So for IBX/CPT we disable LVDS more per VBT.

> +
> +		/*
> +		 * Note that the VBT spec doesn't agree with this
> +		 * interpretation, but real world VBTs seem to.
> +		 */
> +		if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
> +			dev_priv->vbt.edp.support = 1;

In a surprise twist, this is now useless. We could remove it altogether,
as it was only ever used to disable LVDS. And some likely useless
logging. Really.

Maybe then we should throw BDB_DRIVER_FEATURE_EDP macro out in favor of
BDB_DRIVER_FEATURE_INT_SDVO_LVDS too, to match the spec.

> +	} else {
> +		if (driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS &&
> +		    driver->lvds_config != BDB_DRIVER_FEATURE_INT_SDVO_LVDS)
> +			dev_priv->vbt.int_lvds_support = 0;

For pre-gen5 we start looking at VBT for clues. We enable LVDS for
values 1 and 3, disable otherwise. This matches the spec (unlike for
gen5+). This is likely the useful change for the referenced bug, and
many of the quirks.

Overall I like the direction here, very much, but at the same time I'm
wondering if we should split this up a bit just for caution. First part,
all the changes in intel_lvds_init() you have, as is, and throwing
edp.support out, but keeping the same logic for LVDS vs. no LVDS. Second
part, actually changing the logic here for setting
int_lvds_support. Minimizes and localizes the logic changing patch in
case we need to revert it.

How does that sound? Too wimpy? ;)

BR,
Jani.

> +	}
>  
>  	DRM_DEBUG_KMS("DRRS State Enabled:%d\n", driver->drrs_enabled);
>  	/*
> @@ -1512,6 +1525,9 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>  	dev_priv->vbt.int_tv_support = 1;
>  	dev_priv->vbt.int_crt_support = 1;
>  
> +	/* driver features */
> +	dev_priv->vbt.int_lvds_support = 1;
> +
>  	/* Default to using SSC */
>  	dev_priv->vbt.lvds_use_ssc = 1;
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index d35d2d50f595..f6596c5ce973 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -962,8 +962,16 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>  		return;
>  
>  	/* Skip init on machines we know falsely report LVDS */
> -	if (dmi_check_system(intel_no_lvds))
> +	if (dmi_check_system(intel_no_lvds)) {
> +		WARN(!dev_priv->vbt.int_lvds_support,
> +		     "Useless DMI match. Internal LVDS support disabled by VBT\n");
>  		return;
> +	}
> +
> +	if (!dev_priv->vbt.int_lvds_support) {
> +		DRM_DEBUG_KMS("Internal LVDS support disabled by VBT.\n");
> +		return;
> +	}
>  
>  	if (HAS_PCH_SPLIT(dev_priv))
>  		lvds_reg = PCH_LVDS;
> @@ -975,10 +983,6 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>  	if (HAS_PCH_SPLIT(dev_priv)) {
>  		if ((lvds & LVDS_DETECTED) == 0)
>  			return;
> -		if (dev_priv->vbt.edp.support) {
> -			DRM_DEBUG_KMS("disable LVDS for eDP support\n");
> -			return;
> -		}
>  	}
>  
>  	pin = GMBUS_PIN_PANEL;
> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
> index 458468237b5f..a1d5e39f9a00 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -635,6 +635,7 @@ struct bdb_sdvo_lvds_options {
>  #define BDB_DRIVER_FEATURE_NO_LVDS		0
>  #define BDB_DRIVER_FEATURE_INT_LVDS		1
>  #define BDB_DRIVER_FEATURE_SDVO_LVDS		2
> +#define BDB_DRIVER_FEATURE_INT_SDVO_LVDS	3
>  #define BDB_DRIVER_FEATURE_EDP			3
>  
>  struct bdb_driver_features {

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-05-08  9:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04 14:13 [PATCH 1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Ville Syrjala
2018-05-04 14:13 ` [PATCH 2/2] drm/i915: Eliminate the unused dev_priv->vbt.lvds_vbt Ville Syrjala
2018-05-04 20:14   ` Rodrigo Vivi
2018-05-04 14:33 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present Patchwork
2018-05-04 14:34 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-05-04 14:49 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-04 16:22 ` ✓ Fi.CI.IGT: " Patchwork
2018-05-08  9:33 ` [PATCH 1/2] " Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.