diff mbox

drm/i915: fix screen blank issue in TV detect

Message ID 1252646372-9881-1-git-send-email-zhenyuw@linux.intel.com (mailing list archive)
State Rejected
Headers show

Commit Message

Zhenyu Wang Sept. 11, 2009, 5:19 a.m. UTC
In load time, this one trys to detect TV by current encoder state
instead of load-detect method. Because load-detect will set mode
which disable VGA mode entirely, but couldn't restore back later.
Destroy VGA mode which is the mode after system boot caused black
screen problem, although load fbcon or X later could setup the mode
on LVDS without problem.

That fixes screen blank issue seen on Macbook after boot and before
fbcon or X kicks in, actually this affects all laptops with TV out,
or without real port but our TV encoder probe fails to detect.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_tv.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

Comments

Eric Anholt Sept. 11, 2009, 5:51 p.m. UTC | #1
On Fri, 2009-09-11 at 13:19 +0800, Zhenyu Wang wrote:
> In load time, this one trys to detect TV by current encoder state
> instead of load-detect method. Because load-detect will set mode
> which disable VGA mode entirely, but couldn't restore back later.
> Destroy VGA mode which is the mode after system boot caused black
> screen problem, although load fbcon or X later could setup the mode
> on LVDS without problem.
> 
> That fixes screen blank issue seen on Macbook after boot and before
> fbcon or X kicks in, actually this affects all laptops with TV out,
> or without real port but our TV encoder probe fails to detect.
> 
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>

I think a black screen after boot and before fbcon loads sounds like the
right behavior.
Zhenyu Wang Sept. 14, 2009, 2:06 a.m. UTC | #2
On 2009.09.11 10:51:06 -0700, Eric Anholt wrote:
> On Fri, 2009-09-11 at 13:19 +0800, Zhenyu Wang wrote:
> > In load time, this one trys to detect TV by current encoder state
> > instead of load-detect method. Because load-detect will set mode
> > which disable VGA mode entirely, but couldn't restore back later.
> > Destroy VGA mode which is the mode after system boot caused black
> > screen problem, although load fbcon or X later could setup the mode
> > on LVDS without problem.
> > 
> > That fixes screen blank issue seen on Macbook after boot and before
> > fbcon or X kicks in, actually this affects all laptops with TV out,
> > or without real port but our TV encoder probe fails to detect.
> > 
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> 
> I think a black screen after boot and before fbcon loads sounds like the
> right behavior.
> 

Really? So this looks like you want to force fbcon loading for kernel,
I also thought that way, but I don't know if we can do that in i915 module?
Jesse Barnes Sept. 18, 2009, 10:32 p.m. UTC | #3
On Mon, 14 Sep 2009 10:06:01 +0800
Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> On 2009.09.11 10:51:06 -0700, Eric Anholt wrote:
> > On Fri, 2009-09-11 at 13:19 +0800, Zhenyu Wang wrote:
> > > In load time, this one trys to detect TV by current encoder state
> > > instead of load-detect method. Because load-detect will set mode
> > > which disable VGA mode entirely, but couldn't restore back later.
> > > Destroy VGA mode which is the mode after system boot caused black
> > > screen problem, although load fbcon or X later could setup the
> > > mode on LVDS without problem.
> > > 
> > > That fixes screen blank issue seen on Macbook after boot and
> > > before fbcon or X kicks in, actually this affects all laptops
> > > with TV out, or without real port but our TV encoder probe fails
> > > to detect.
> > > 
> > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > 
> > I think a black screen after boot and before fbcon loads sounds
> > like the right behavior.
> > 
> 
> Really? So this looks like you want to force fbcon loading for kernel,
> I also thought that way, but I don't know if we can do that in i915
> module?

No, we don't need to force it.  It's up to the distro to load things at
the right time.
Zhenyu Wang Sept. 21, 2009, 3:41 a.m. UTC | #4
On 2009.09.18 15:32:42 -0700, Jesse Barnes wrote:
> On Mon, 14 Sep 2009 10:06:01 +0800
> Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> 
> > On 2009.09.11 10:51:06 -0700, Eric Anholt wrote:
> > > On Fri, 2009-09-11 at 13:19 +0800, Zhenyu Wang wrote:
> > > > In load time, this one trys to detect TV by current encoder state
> > > > instead of load-detect method. Because load-detect will set mode
> > > > which disable VGA mode entirely, but couldn't restore back later.
> > > > Destroy VGA mode which is the mode after system boot caused black
> > > > screen problem, although load fbcon or X later could setup the
> > > > mode on LVDS without problem.
> > > > 
> > > > That fixes screen blank issue seen on Macbook after boot and
> > > > before fbcon or X kicks in, actually this affects all laptops
> > > > with TV out, or without real port but our TV encoder probe fails
> > > > to detect.
> > > > 
> > > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > 
> > > I think a black screen after boot and before fbcon loads sounds
> > > like the right behavior.
> > > 
> > 
> > Really? So this looks like you want to force fbcon loading for kernel,
> > I also thought that way, but I don't know if we can do that in i915
> > module?
> 
> No, we don't need to force it.  It's up to the distro to load things at
> the right time.
> 

Well, yeah, you may prefer that way, but I don't think all distros do that automaticly.
Debian doesn't do that for me after I build my own kernel and modules on MacBook...

We've already known that it'll be blank in the way, and we might be got more
bug reports like http://bugs.freedesktop.org/show_bug.cgi?id=24021, why we can't
just check the current hw status in load time instead of doing load_detect? Just
realized that for older 8xx chips we also do load_detect for CRT, I can refresh
my origin patch to not do any load_detect when loading.
Yves-Alexis Perez Sept. 21, 2009, 6:17 a.m. UTC | #5
On lun, 2009-09-21 at 11:41 +0800, Zhenyu Wang wrote:
> > > Really? So this looks like you want to force fbcon loading for kernel,
> > > I also thought that way, but I don't know if we can do that in i915
> > > module?
> > 
> > No, we don't need to force it.  It's up to the distro to load things at
> > the right time.
> > 
> 
> Well, yeah, you may prefer that way, but I don't think all distros do that automaticly.
> Debian doesn't do that for me after I build my own kernel and modules on MacBook... 

And for good reasons: CONFIG_FRAMEBUFFER_CONSOLE=y so fbcon is builtin.

Cheers,
Jesse Barnes Sept. 21, 2009, 3:49 p.m. UTC | #6
On Mon, 21 Sep 2009 11:41:14 +0800
Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> On 2009.09.18 15:32:42 -0700, Jesse Barnes wrote:
> > On Mon, 14 Sep 2009 10:06:01 +0800
> > Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > 
> > > On 2009.09.11 10:51:06 -0700, Eric Anholt wrote:
> > > > On Fri, 2009-09-11 at 13:19 +0800, Zhenyu Wang wrote:
> > > > > In load time, this one trys to detect TV by current encoder
> > > > > state instead of load-detect method. Because load-detect will
> > > > > set mode which disable VGA mode entirely, but couldn't
> > > > > restore back later. Destroy VGA mode which is the mode after
> > > > > system boot caused black screen problem, although load fbcon
> > > > > or X later could setup the mode on LVDS without problem.
> > > > > 
> > > > > That fixes screen blank issue seen on Macbook after boot and
> > > > > before fbcon or X kicks in, actually this affects all laptops
> > > > > with TV out, or without real port but our TV encoder probe
> > > > > fails to detect.
> > > > > 
> > > > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > > 
> > > > I think a black screen after boot and before fbcon loads sounds
> > > > like the right behavior.
> > > > 
> > > 
> > > Really? So this looks like you want to force fbcon loading for
> > > kernel, I also thought that way, but I don't know if we can do
> > > that in i915 module?
> > 
> > No, we don't need to force it.  It's up to the distro to load
> > things at the right time.
> > 
> 
> Well, yeah, you may prefer that way, but I don't think all distros do
> that automaticly. Debian doesn't do that for me after I build my own
> kernel and modules on MacBook...
> 
> We've already known that it'll be blank in the way, and we might be
> got more bug reports like
> http://bugs.freedesktop.org/show_bug.cgi?id=24021, why we can't just
> check the current hw status in load time instead of doing
> load_detect? Just realized that for older 8xx chips we also do
> load_detect for CRT, I can refresh my origin patch to not do any
> load_detect when loading.

Yeah, avoiding load detect where possible seems like a good thing.  I
just don't think we need to do it to avoid blanking the screen before
fbcon loads.

So I'm not opposed to the patch; would be cool if you can do it for
CRTs as well.

Thanks,
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 5b1c9e9..edb4e6e 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -38,6 +38,11 @@ 
 #include "i915_drm.h"
 #include "i915_drv.h"
 
+/* 
+ * This shortcut TV detect in load time for checking current state only.
+ */
+static bool load_time_detect = true;
+
 enum tv_margin {
 	TV_MARGIN_LEFT, TV_MARGIN_TOP,
 	TV_MARGIN_RIGHT, TV_MARGIN_BOTTOM
@@ -1451,9 +1456,28 @@  intel_tv_detect(struct drm_connector *connector)
 	struct intel_output *intel_output = to_intel_output(connector);
 	struct intel_tv_priv *tv_priv = intel_output->dev_priv;
 	struct drm_encoder *encoder = &intel_output->enc;
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
 	int dpms_mode;
 	int type = tv_priv->type;
 
+	/* 
+	 * In driver load time, TV load detect will try to set mode, 
+	 * during which VGA mode will be disabled and never could be
+	 * restored later. This will blank screen which is in VGA mode
+	 * after boot.
+	 *
+	 * So here we only check current TV encoder state when loading.
+	 * Future TV detect for fbcon or X still work as before.
+	 *
+	 */
+	if (load_time_detect) {
+		load_time_detect = false;
+		if (!(I915_READ(TV_CTL) & TV_ENC_ENABLE))
+			return connector_status_disconnected;
+		else
+			return connector_status_connected;
+	}
+
 	mode = reported_modes[0];
 	drm_mode_set_crtcinfo(&mode, CRTC_INTERLACE_HALVE_V);