[Intel-gfx] [RFC PATCH 4/5] DRM/I915: Use the child device to decide whether the LVDS should be intialized

[ykzhao]

On Wed, 2009-11-18 at 23:14 +0800, Adam Jackson wrote:
> On Wed, 2009-11-18 at 10:39 +0800, ykzhao wrote:
> > On Tue, 2009-11-17 at 23:47 +0800, Adam Jackson wrote:
> > > On Tue, 2009-11-17 at 15:43 +0800, yakui.zhao at intel.com wrote:
> > > > +int lvds_is_present_in_vbt(struct drm_device *dev)
> > > > +{
> > > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > > +	struct child_device_config *p_child;
> > > > +	int i, ret;
> > > > +
> > > > +	if (!dev_priv->child_dev_num)
> > > > +		return 1;
> > > > +
> > > > +	ret = 0;
> > > > +	for (i = 0; i < dev_priv->child_dev_num; i++) {
> > > > +		p_child = dev_priv->child_dev + i;
> > > > +		/*
> > > > +		 * If the device type is not LFP, continue.
> > > > +		 */
> > > > +		if (p_child->device_type != INT_LFP_CLASS)
> > > > +			continue;
> > > 
> > > Not that I have ROMs to prove this, but I see a whole bunch of other LFP
> > > DEVICE_TYPE defines in intel_bios.h.  I suspect we should treat them the
> > > same.
> > Can you send me the vbios.dump on your box so that we can check whether
> > it can work?
> 
> I wasn't being snarky.  I really don't have ROMs like this.  All the
> ROMs I have, have 0x1022 for the device type for LFP.
> 
> However, the source documents more LFP device types.  I have to assume
> those numbers came from some Intel-internal source and weren't just
> invented out of thin air.  Therefore they should probably be treated the
> same as 0x1022.
I check the vbios.dump on a lot of laptops. And the device type for LVDS
is 0x1022.

Of course we can also treat the device type with 0x022 as INT_LFP type.
Is this ok to you?
> 
> > > > @@ -936,6 +974,10 @@ void intel_lvds_init(struct drm_device *dev)
> > > >  	if (dmi_check_system(intel_no_lvds))
> > > >  		return;
> > > >  
> > > > +	if (!lvds_is_present_in_vbt(dev)) {
> > > > +		DRM_DEBUG_KMS("LVDS is not present in VBT\n");
> > > > +		return;
> > > > +	}
> > > >  	/* Assume that any device without an ACPI LID device also doesn't
> > > >  	 * have an integrated LVDS.  We would be better off parsing the BIOS
> > > >  	 * to get a reliable indicator, but that code isn't written yet.
> > > 
> > > The juxtaposition between code and comment here is pretty funny.
> > > Therefore I'm reasonably sure it's wrong.
> >
> > The above comment is not related with this patch. It is the comment
> > about the patch that uses the LID device to check whether the LVDS is
> > present.
> 
> It _is_ related.  The comment says "we should parse the BIOS for LVDS
> presence instead of assuming it's correlated with lid presence".  You
> wrote code to parse the BIOS for LVDS presence, but did _not_ remove the
> code that conflates lid with LVDS.
> 
> Consider - just as a thought exercise - a tablet machine.  Something
> like this, for example:
> 
> http://www.ngtmedical.com/products/hardware/motion/
> 
> No, I do not have one.  Just think about what its firmware is going to
> look like.  It's going to say "I have an LVDS panel attached", because
> it does.  It's also not going to have a lid device in ACPI, because
> there's no lid.
Understand what you said.
But it seems that this issue on Motion C5/LE7000 tablet-pc is not
related with this patch set.
In fact the LVDS detection is a problem that confuses us for a long
time. 
In UMS mode we use the flag in VBT to check whether it exits. But it
seems that this is not reliable on some laptops.
So we turn to LID device to detect whether the LVDS exits. Now it seems
that it is appliable for most laptops. If the LVDS is detected
correctly, we will try to add it to the quirk table.

thanks.
> 
> - ajax