intel_opregion_get_panel_type potential bug fix

* intel_opregion_get_panel_type potential bug fix
@ 2016-08-29 17:35 Sean Greenslade
  2016-08-29 18:19 ` ✓ Fi.CI.BAT: success for " Patchwork
  2016-09-14  9:39 ` Jani Nikula
  0 siblings, 2 replies; 3+ messages in thread
From: Sean Greenslade @ 2016-08-29 17:35 UTC (permalink / raw)
  To: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]

Hello, all. I've been attempting to track down a bug with my Panasonic
CF-30 laptop. I have a bug report[0] submitted.

In investigating the issue myself, I discovered that the root of the
problem seemed to be that the new code for getting the panel type from
the opregion seems to believe it is getting good data, but it is not.
The function intel_opregion_get_panel_type (introduced in commit
a05628195a0d, the one my bisect says caused this issue) seems to do some
checks for validity, but apparently not enough.

I see there has already been a fix for one particular laptop[1], however
my machine is not a Skylake so this fix does not apply to me. After
reading over what Ville Syrjälä wrote in the patch notes for the
get_panel_type function, I went and read part of the intel spec[2] that
describes this data, and found a discrepancy that I don't fully
understand. On page 106 of [2], there is a table that describes the
layout of the response that intel_opregion_get_panel_type parses for the
panel type. Bit 17 is listed as reserved, must be zero. On my machine,
that bit is set to one. Additionally, on my two other Intel graphics
laptops that aren't exhibiting this issue, it is also set to one,
however they both fail the existing sanity checks in the function and
revert to the VBT entry.

But now I'm in a bit over my head. I've attached the patch (applies to
drm-intel-nightly) that I used to get the full u32 response of the swsci
lookup, as well as a tentative fix. My fix checks for the reserved bit
being set incorrectly and reverts to using the VBT entry. This fixes the
regression on my CF-30, and doesn't hurt anything on my other two
machines, but I have no newer machines to test it on. Can someone look
this over and see if I'm taking the correct approach? Additionally, I
would love to know if anyone has any more information on why the
reserved bit is set (it was set on all three of my machines).

Thanks for your attention,

--Sean


[0]: https://bugs.freedesktop.org/show_bug.cgi?id=97443
[1]: https://lists.freedesktop.org/archives/intel-gfx/2016-July/100573.html
[2]: https://01.org/sites/default/files/documentation/acpi_igd_opregion_spec_0.pdf

[-- Attachment #2: get_panel_type.patch --]
[-- Type: text/x-diff, Size: 1008 bytes --]

diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index adca262..1c35eab 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -1059,6 +1059,7 @@ intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
 			      ret);
 		return ret;
 	}
+	DRM_DEBUG_KMS("SEAN: panel_details: 0x%08x\n", panel_details);
 
 	ret = (panel_details >> 8) & 0xff;
 	if (ret > 0x10) {
@@ -1072,6 +1073,14 @@ intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
 		return -ENODEV;
 	}
 
+	/* Bit 17 is reserved and should be zero. Observed it set to one on
+	 * a Panasonic CF-30, where it also returns the incorrect panel type.
+	 */
+	if (panel_details & (1<<17)) {
+		DRM_DEBUG_KMS("Invalid panel details, ignoring OpRegion panel type (%d)\n", ret - 1);
+		return -ENODEV;
+	}
+
 	/*
 	 * FIXME On Dell XPS 13 9350 the OpRegion panel type (0) gives us
 	 * low vswing for eDP, whereas the VBT panel type (2) gives us normal

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

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