diff mbox

[v3] drm/i915: Add missing MacBook Pro models with dual channel LVDS

Message ID 20150430132811.GA2875@wunner.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lukas Wunner April 30, 2015, 1:28 p.m. UTC
Single channel LVDS maxes out at 112 MHz. All 17" models with i915
graphics had a resolution of 1920x1200 (193 MHz), necessitating dual
channel LVDS. The 15" pre-retina models had either 1440x900 (106 MHz)
or 1680x1050 (119 MHz), both versions used dual channel LVDS even
though the smaller one would have fit into a single channel.

v2: Assume dual channel LVDS if the pixel clock necessitates it,
eliminating the need to hardcode 17" models, suggested by Jani Nikula.

v3: Fix typo, thanks Joonas Lahtinen.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Tested-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_lvds.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

Comments

Ville Syrjälä April 30, 2015, 2:15 p.m. UTC | #1
On Thu, Apr 30, 2015 at 03:28:11PM +0200, Lukas Wunner wrote:
> Single channel LVDS maxes out at 112 MHz. All 17" models with i915
> graphics had a resolution of 1920x1200 (193 MHz), necessitating dual
> channel LVDS. The 15" pre-retina models had either 1440x900 (106 MHz)
> or 1680x1050 (119 MHz), both versions used dual channel LVDS even
> though the smaller one would have fit into a single channel.
> 
> v2: Assume dual channel LVDS if the pixel clock necessitates it,
> eliminating the need to hardcode 17" models, suggested by Jani Nikula.

I would suggest splitting that into a separate patch. Otherwise it's
going to harder to revert either change separately, should the need
arise. So that part is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I have no clue about the Mac stuff so won't pretend to say anything
about that.

> 
> v3: Fix typo, thanks Joonas Lahtinen.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_lvds.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 071b96d..417ccab 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -812,12 +812,28 @@ static int intel_dual_link_lvds_callback(const struct dmi_system_id *id)
>  static const struct dmi_system_id intel_dual_link_lvds[] = {
>  	{
>  		.callback = intel_dual_link_lvds_callback,
> -		.ident = "Apple MacBook Pro (Core i5/i7 Series)",
> +		.ident = "Apple MacBook Pro 15\" (2010)",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro6,2"),
> +		},
> +	},
> +	{
> +		.callback = intel_dual_link_lvds_callback,
> +		.ident = "Apple MacBook Pro 15\" (2011)",
>  		.matches = {
>  			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
>  			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro8,2"),
>  		},
>  	},
> +	{
> +		.callback = intel_dual_link_lvds_callback,
> +		.ident = "Apple MacBook Pro 15\" (2012)",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro9,1"),
> +		},
> +	},
>  	{ }	/* terminating entry */
>  };
>  
> @@ -847,6 +863,11 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder)
>  	if (i915.lvds_channel_mode > 0)
>  		return i915.lvds_channel_mode == 2;
>  
> +	/* single channel LVDS is limited to 112 MHz */
> +	if (lvds_encoder->attached_connector->base.panel.fixed_mode->clock
> +	    > 112999)
> +		return true;
> +
>  	if (dmi_check_system(intel_dual_link_lvds))
>  		return true;
>  
> @@ -1104,6 +1125,8 @@ void intel_lvds_init(struct drm_device *dev)
>  out:
>  	mutex_unlock(&dev->mode_config.mutex);
>  
> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> +
>  	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
>  	DRM_DEBUG_KMS("detected %s-link lvds configuration\n",
>  		      lvds_encoder->is_dual_link ? "dual" : "single");
> @@ -1118,7 +1141,6 @@ out:
>  	}
>  	drm_connector_register(connector);
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
>  	intel_panel_setup_backlight(connector, INVALID_PIPE);
>  
>  	return;
> -- 
> 1.8.5.2 (Apple Git-48)
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lukas Wunner May 4, 2015, 2:52 p.m. UTC | #2
Hi Ville,

> I would suggest splitting that into a separate patch. Otherwise it's
> going to harder to revert either change separately, should the need
> arise. So that part is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Okay, commit split in two as requested.

Question:

Daniel Vetter hardcoded the MacBookPro8,2 with commit
618563e3945b9d0864154bab3c607865b557cecc, noting:
"Add a dual link lvds quirk for MacBook Pro 8,2
When booting with EFI, Apple botched this one up."

Nouveau recognizes just fine that dual channel LVDS is necessary,
it seems to retrieve this information from VBIOS.

On the other hand, when i915 initializes, it logs the following
complaint (on a MacBookPro9,1): "[drm] failed to find VBIOS tables".

Would it be possible to somehow point i915 to the correct location
where it can find the VBIOS? Or can we somehow copy the VBIOS nouveau
is using? I'm wondering, even if we had to hardcode memory locations
to do that, wouldn't that make more sense rather than hardcoding the
DMI IDs?

Thanks,

Lukas
Jani Nikula May 5, 2015, 11:26 a.m. UTC | #3
On Mon, 04 May 2015, Lukas Wunner <lukas@wunner.de> wrote:
> Hi Ville,
>
>> I would suggest splitting that into a separate patch. Otherwise it's
>> going to harder to revert either change separately, should the need
>> arise. So that part is
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Okay, commit split in two as requested.

I'll look into them, thanks.

> Question:
>
> Daniel Vetter hardcoded the MacBookPro8,2 with commit
> 618563e3945b9d0864154bab3c607865b557cecc, noting:
> "Add a dual link lvds quirk for MacBook Pro 8,2
> When booting with EFI, Apple botched this one up."
>
> Nouveau recognizes just fine that dual channel LVDS is necessary,
> it seems to retrieve this information from VBIOS.
>
> On the other hand, when i915 initializes, it logs the following
> complaint (on a MacBookPro9,1): "[drm] failed to find VBIOS tables".
>
> Would it be possible to somehow point i915 to the correct location
> where it can find the VBIOS? Or can we somehow copy the VBIOS nouveau
> is using? I'm wondering, even if we had to hardcode memory locations
> to do that, wouldn't that make more sense rather than hardcoding the
> DMI IDs?

These are vendor specific bios tables we're talking about. We couldn't
readily use it anyway.

I don't really know, but looks like OS X gets the IGD info from
somewhere other than the usual VBT Windows and Linux use, while the
nvidia VBIOS is the same also on OS X. *shrug*

BR,
Jani.


>
> Thanks,
>
> Lukas
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 071b96d..417ccab 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -812,12 +812,28 @@  static int intel_dual_link_lvds_callback(const struct dmi_system_id *id)
 static const struct dmi_system_id intel_dual_link_lvds[] = {
 	{
 		.callback = intel_dual_link_lvds_callback,
-		.ident = "Apple MacBook Pro (Core i5/i7 Series)",
+		.ident = "Apple MacBook Pro 15\" (2010)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro6,2"),
+		},
+	},
+	{
+		.callback = intel_dual_link_lvds_callback,
+		.ident = "Apple MacBook Pro 15\" (2011)",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
 			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro8,2"),
 		},
 	},
+	{
+		.callback = intel_dual_link_lvds_callback,
+		.ident = "Apple MacBook Pro 15\" (2012)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro9,1"),
+		},
+	},
 	{ }	/* terminating entry */
 };
 
@@ -847,6 +863,11 @@  static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder)
 	if (i915.lvds_channel_mode > 0)
 		return i915.lvds_channel_mode == 2;
 
+	/* single channel LVDS is limited to 112 MHz */
+	if (lvds_encoder->attached_connector->base.panel.fixed_mode->clock
+	    > 112999)
+		return true;
+
 	if (dmi_check_system(intel_dual_link_lvds))
 		return true;
 
@@ -1104,6 +1125,8 @@  void intel_lvds_init(struct drm_device *dev)
 out:
 	mutex_unlock(&dev->mode_config.mutex);
 
+	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+
 	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
 	DRM_DEBUG_KMS("detected %s-link lvds configuration\n",
 		      lvds_encoder->is_dual_link ? "dual" : "single");
@@ -1118,7 +1141,6 @@  out:
 	}
 	drm_connector_register(connector);
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	return;