[Intel-gfx] [DRM/I915]: Check the LID device to decide whether the LVDS should be initialized

[yakui]

On Thu, 2009-07-02 at 14:18 +0800, yakui wrote:
> On Thu, 2009-07-02 at 09:46 +0800, Eric Anholt wrote:
> > From 8f5a8ebda01c505117c93b74eca1b5a49e11b430 Mon Sep 17 00:00:00 2001
> > From: Eric Anholt <eric at anholt.net>
> > Date: Wed, 1 Jul 2009 18:37:03 -0700
> > Subject: [PATCH] drm/i915: Clean up the ACPI LID-based detection code.
> > 
> > Signed-off-by: Eric Anholt <eric at anholt.net>
> > ---
> >  drivers/gpu/drm/i915/intel_lvds.c |   69 ++++++++++++++++---------------------
> >  1 files changed, 30 insertions(+), 39 deletions(-)
> > 
> > On Tue, 2009-06-30 at 10:22 +0800, yakui wrote: 
> > > On Fri, 2009-06-26 at 09:46 +0800, Zhao, Yakui wrote:
> > > > From: Zhao Yakui <yakui.zhao at intel.com>
> > > Hi, Eric
> > >     How about this patch?
> > >     Thanks.
> > 
> > This code still looks like it needs work -- bad hungarian notation,
> > missing whitespace, and gratuitous comments explaining exactly the logic
> > of the code you were just reading without enlightening as to why.  How
> > about this patch squashed into yours, or on top of it? 
> Your changes look OK to me. And the code style is better than mine.
> 
> It is enough to squash it into one patch.
Hi, Eric
    Your change looks ok to me.
    How about picking up this patch?
thanks.
> 
> Thanks.
>    Yakui
> 
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 2812524..2d0426d 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -788,24 +788,21 @@ static const struct dmi_system_id intel_no_lvds[] = {
> >  
> >  	{ }	/* terminating entry */
> >  };
> > +
> >  #ifdef CONFIG_ACPI
> >  /*
> > - * check_lid_device -- check whether it is ACPI LID device.
> > + * check_lid_device -- check whether it @handle is an ACPI LID device.
> >   * @handle: ACPI device handle
> >   * @level : depth in the ACPI namespace tree
> >   * @context: the number of LID device when we find the device
> >   * @rv: a return value to fill if desired (Not use)
> > - *
> > - * check whether it is a LID device by comparing the HID. If it is,
> > - * increase the number of LID device.
> >   */
> >  static acpi_status
> >  check_lid_device(acpi_handle handle, u32 level, void *context,
> > -			void **retyurn_value)
> > +			void **return_value)
> >  {
> > -#define		ACPI_HID_LID		"PNP0C0D"
> >  	struct acpi_device *acpi_dev;
> > -	int *p_lid = (int *)context;
> > +	int *lid_present = context;
> >  
> >  	acpi_dev = NULL;
> >  	/* Get the acpi device for device handle */
> > @@ -813,49 +810,44 @@ check_lid_device(acpi_handle handle, u32 level, void *context,
> >  		/* If there is no ACPI device for handle, return */
> >  		return AE_OK;
> >  	}
> > -	if (!strncmp(acpi_device_hid(acpi_dev), ACPI_HID_LID, 7)) {
> > -		/*
> > -		 * compare the device HID with "PNP0C0D". If it is equal, the
> > -		 * LID device is found. Increase the count
> > -		 */
> > -		(*p_lid)++;
> > -	}
> > +
> > +	if (!strncmp(acpi_device_hid(acpi_dev), "PNP0C0D", 7))
> > +		*lid_present = 1;
> > +
> >  	return AE_OK;
> >  }
> > +
> >  /**
> >   * check whether there exists the ACPI LID device by enumerating the ACPI
> >   * device tree.
> > - * If ACPI is disabled, there is no ACPI device tree. one is returned.
> > - * If the LID device is found, it will return one.
> > - * If no LID device is found, it will return  zero.
> >   */
> >  static int intel_lid_present(void)
> >  {
> > -	int lid_count = 0;
> > +	int lid_present = 0;
> >  
> >  	if (acpi_disabled) {
> > -		/*
> > -		 * if ACPI is disabled, there is no ACPI device tree. And
> > -		 * we don't know whether there exists the LID device.
> > -		 * In such case we will return 1.
> > +		/* If ACPI is disabled, there is no ACPI device tree to
> > +		 * check, so assume the LID device would have been present.
> >  		 */
> >  		return 1;
> >  	}
> >  
> >  	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> >  				ACPI_UINT32_MAX,
> > -				check_lid_device, &lid_count, NULL);
> > -
> > -	if (!lid_count) {
> > -		/* No LID device is not found. Return zero */
> > -		return 0;
> > -	}
> > +				check_lid_device, &lid_present, NULL);
> >  
> > -	return 1;
> > +	return lid_present;
> >  }
> >  #else
> > -static inline int intel_lid_present(void) { return 1; }
> > +static int intel_lid_present(void)
> > +{
> > +	/* In the absence of ACPI built in, assume that the LID device would
> > +	 * have been present.
> > +	 */
> > +	return 1;
> > +}
> >  #endif
> > +
> >  /**
> >   * intel_lvds_init - setup LVDS connectors on this device
> >   * @dev: drm device
> > @@ -879,16 +871,15 @@ void intel_lvds_init(struct drm_device *dev)
> >  	if (dmi_check_system(intel_no_lvds))
> >  		return;
> >  
> > -	if (!intel_lid_present()) {
> > -		/* If there is no LID device, we can think that there is
> > -		 * no LVDS output device. In such case it is unnecessary to
> > -		 * create the LVDS output device.
> > -		 * But maybe on some boxes there is no LVDS device while the
> > -		 * LID device is found. If so, it had better be added to
> > -		 * the quirk list.
> > -		 */
> > +	/* 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.
> > +	 *
> > +	 * In the case of all-in-one desktops using LVDS that we've seen,
> > +	 * they're using SDVO LVDS.
> > +	 */
> > +	if (!intel_lid_present())
> >  		return;
> > -	}
> >  
> >  	if (IS_IGDNG(dev)) {
> >  		if ((I915_READ(PCH_LVDS) & LVDS_DETECTED) == 0)
> > -- 
> > 1.6.3.1
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx