linux-kernel - Re: drivers/gpu/drm/i915/intel_dp.c problem with commit dc22ee6fc18ce0f15424e753e8473c306ece95c1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <86k46m8z1s.fsf@sumi.keithp.com>
Date:	Sat, 26 Nov 2011 11:27:11 -0800
From:	Keith Packard <keith.packard@...el.com>
To:	Jerker Buud <b.jerker@...il.com>,
	Patrik Kullman <patrik.kullman@...il.com>
cc:	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: drivers/gpu/drm/i915/intel_dp.c problem with commit dc22ee6fc18ce0f15424e753e8473c306ece95c1

On Sat, 26 Nov 2011 09:50:41 -0800, Jerker Buud <b.jerker@...il.com> wrote:

> 245,247c245,246
> <       if (!is_edp(intel_dp) &&
> <           (intel_dp_link_required(intel_dp, mode->clock)
> <            > intel_dp_max_data_rate(max_link_clock, max_lanes)))
> ---
> >       if (intel_dp_link_required(intel_dp, mode->clock)
> >           > intel_dp_max_data_rate(max_link_clock, max_lanes))

Ok, I think I understand why this change is required.

These machines are 'optimized' to provide the bare minimum connection
necessary between the CPU and the eDP panel -- a single lane is hooked
up, which is just enough to run the link at 18bpp, but not enough to run
it at 24bpp.

The problem here (in intel_dp_link_required) is that this is being
called to see which modes can be supported on the panel. Without the
CRTC being configured with the appropriate bpp value, this code assumes
24bpp.

I thought Adam Jackson had some code around that automatically switched
From 24bpp to 18bpp when the link didn't have enough bandwidth for
24bpp, but I can't find that now.

In any case, I think we need to centralize the bpp selection inside
intel_dp.c instead of having it in both places. Then, we can make that
depend on the requested mode as well as the machine configuration.

Here's a patch which uses the VBT configured bpp value in this case. I'd
love to hear whether this suffices to resolve your problem.

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 294f557..3e15479 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -212,10 +212,16 @@ intel_dp_link_required(struct intel_dp *intel_dp, int pixel_clock)
 {
 	struct drm_crtc *crtc = intel_dp->base.base.crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int bpp = 24;
+	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int bpp;

 	if (intel_crtc)
 		bpp = intel_crtc->bpp;
+	else if (intel_dp->base.type == INTEL_OUTPUT_EDP)
+		bpp = dev_priv->edp.bpp;
+	else
+		bpp = 24;

 	return (pixel_clock * bpp + 9) / 10;
 }

-- 
keith.packard@...el.com

Content of type "application/pgp-signature" skipped