diff mbox

Linux 3.7-rc6

Message ID 20121120103905.GA3908@polaris.bitmath.org (mailing list archive)
State New, archived
Headers show

Commit Message

Henrik Rydberg Nov. 20, 2012, 10:39 a.m. UTC
>       drm/i915: do not ignore eDP bpc settings from vbt

As advertised, this patch breaks the Macbook Pro Retina, which seems
unfair. The patch below is certainly not the best remedy, but it does
work. Tested on a MacbookPro10,1.

Thanks,
Henrik

---

From: Henrik Rydberg <rydberg@euromail.se>
Date: Tue, 20 Nov 2012 11:16:05 +0100
Subject: [PATCH] drm/i915: Ignore eDP bpc settings from vbt based on dmi data
Status: O
Content-Length: 1637
Lines: 51

Commit 2f4f649a6 breaks the Retina display on MacBookPro10 laptops.
This patch reintroduces the logic reverted by that patch, but only
for the Retina machines.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Nov. 21, 2012, 11:55 a.m. UTC | #1
On Tue, Nov 20, 2012 at 11:39 AM, Henrik Rydberg <rydberg@bitmath.se> wrote:
>>       drm/i915: do not ignore eDP bpc settings from vbt
>
> As advertised, this patch breaks the Macbook Pro Retina, which seems
> unfair. The patch below is certainly not the best remedy, but it does
> work. Tested on a MacbookPro10,1.

My apologies for the long delay in answering, I've somehow mixed up
different bugreports and thought I've sent you a patch to test
already. Anyway, please test

https://patchwork.kernel.org/patch/1728111/

Thanks, Daniel
Henrik Rydberg Nov. 21, 2012, 5:55 p.m. UTC | #2
Hi Daniel,

> > As advertised, this patch breaks the Macbook Pro Retina, which seems
> > unfair. The patch below is certainly not the best remedy, but it does
> > work. Tested on a MacbookPro10,1.
> 
> My apologies for the long delay in answering, I've somehow mixed up
> different bugreports and thought I've sent you a patch to test
> already. Anyway, please test
> 
> https://patchwork.kernel.org/patch/1728111/

Thanks for the patch! I will try it out tomorrow and let you know.

Henrik
Henrik Rydberg Nov. 22, 2012, 11:18 a.m. UTC | #3
Hi Daniel,

> My apologies for the long delay in answering, I've somehow mixed up
> different bugreports and thought I've sent you a patch to test
> already. Anyway, please test
> 
> https://patchwork.kernel.org/patch/1728111/

    Tested-by: Henrik Rydberg <rydberg@euromail.se>

Thanks,
Henrik
Daniel Vetter Nov. 22, 2012, 1:10 p.m. UTC | #4
On Thu, Nov 22, 2012 at 12:18 PM, Henrik Rydberg <rydberg@bitmath.se> wrote:
>> My apologies for the long delay in answering, I've somehow mixed up
>> different bugreports and thought I've sent you a patch to test
>> already. Anyway, please test
>>
>> https://patchwork.kernel.org/patch/1728111/
>
>     Tested-by: Henrik Rydberg <rydberg@euromail.se>

Can you please boot with drm.debug=0xe added to your kernel cmdline
with that patch applied and attach the full dmesg?
-Daniel
Henrik Rydberg Nov. 22, 2012, 6:23 p.m. UTC | #5
Hi Daniel,

> >> My apologies for the long delay in answering, I've somehow mixed up
> >> different bugreports and thought I've sent you a patch to test
> >> already. Anyway, please test
> >>
> >> https://patchwork.kernel.org/patch/1728111/
> >
> >     Tested-by: Henrik Rydberg <rydberg@euromail.se>
> 
> Can you please boot with drm.debug=0xe added to your kernel cmdline
> with that patch applied and attach the full dmesg?

Are you looking for something in particular? The patch obviously works
because edp_bpp is never set. The reason seems to be

[    1.634759] [drm:intel_parse_bios], VBT signature missing

In the source, a few lines above that message, we have something that
looks suspciciously like an off-by-one error:

	/* Scour memory looking for the VBT signature */
	for (i = 0; i + 4 < size; i++) {

If that matters or not, I do not know. Any more tests would have to
wait until tomorrow.

Thanks,
Henrik
Daniel Vetter Nov. 22, 2012, 8:32 p.m. UTC | #6
On Thu, Nov 22, 2012 at 7:23 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> >> My apologies for the long delay in answering, I've somehow mixed up
>> >> different bugreports and thought I've sent you a patch to test
>> >> already. Anyway, please test
>> >>
>> >> https://patchwork.kernel.org/patch/1728111/
>> >
>> >     Tested-by: Henrik Rydberg <rydberg@euromail.se>
>>
>> Can you please boot with drm.debug=0xe added to your kernel cmdline
>> with that patch applied and attach the full dmesg?
>
> Are you looking for something in particular? The patch obviously works
> because edp_bpp is never set. The reason seems to be
>
> [    1.634759] [drm:intel_parse_bios], VBT signature missing
>
> In the source, a few lines above that message, we have something that
> looks suspciciously like an off-by-one error:
>
>         /* Scour memory looking for the VBT signature */
>         for (i = 0; i + 4 < size; i++) {
>
> If that matters or not, I do not know. Any more tests would have to
> wait until tomorrow.

Yeah, the utter lack of a vbt fits very nicely, thanks for checking,
I've merged the patch into drm-intel-fixes and will forward it for
inclusion into 3.7 rsn.
-Daniel
Henrik Rydberg Nov. 22, 2012, 8:53 p.m. UTC | #7
> Yeah, the utter lack of a vbt fits very nicely, thanks for checking,
> I've merged the patch into drm-intel-fixes and will forward it for
> inclusion into 3.7 rsn.

Great, thanks. One thing about that patch: if we would ever encounter
a non-zero edp.bpp < 3, display_bpc would not be clamped. I suppose
monochrome screens went out of fashion twenty years ago, but who
knows...

Thanks,
Henrik
Jani Nikula Nov. 23, 2012, 9:53 a.m. UTC | #8
On Thu, 22 Nov 2012, Henrik Rydberg <rydberg@euromail.se> wrote:
>> Yeah, the utter lack of a vbt fits very nicely, thanks for checking,
>> I've merged the patch into drm-intel-fixes and will forward it for
>> inclusion into 3.7 rsn.
>
> Great, thanks. One thing about that patch: if we would ever encounter
> a non-zero edp.bpp < 3, display_bpc would not be clamped. I suppose
> monochrome screens went out of fashion twenty years ago, but who
> knows...

ATM, edp.bpp is known to be 18, 24, or 30. It's mapped from a two-bit
value in the VBT.

Glad to hear the patch works for you.

BR,
Jani.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4154bcd..97658d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3776,6 +3776,24 @@  static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
+static int dmi_ignore_bpc_from_vbt_callback(const struct dmi_system_id *id)
+{
+	DRM_INFO("Ignoring bpc from vbt on %s\n", id->ident);
+	return 1;
+}
+
+static const struct dmi_system_id dmi_ignore_bpc_from_vbt[] = {
+	{
+		.callback = dmi_ignore_bpc_from_vbt_callback,
+		.ident = "Apple MacBook Pro Retina",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro10"),
+		},
+	},
+	{ }	/* terminating entry */
+};
+
 /**
  * intel_choose_pipe_bpp_dither - figure out what color depth the pipe should send
  * @crtc: CRTC structure
@@ -3841,7 +3859,8 @@  static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 			}
 		}
 
-		if (intel_encoder->type == INTEL_OUTPUT_EDP) {
+		if (intel_encoder->type == INTEL_OUTPUT_EDP &&
+			!dmi_check_system(dmi_ignore_bpc_from_vbt)) {
 			/* Use VBT settings if we have an eDP panel */
 			unsigned int edp_bpc = dev_priv->edp.bpp / 3;