Re: [PATCH 4/4] drm/i915: Ignore LVDS EDID when it is unavailabe or invalid

[ykzhao <yakui.zhao@intel.com>]

On Sat, 2010-04-10 at 05:22 +0800, Eric Anholt wrote:
> On Wed,  7 Apr 2010 17:11:22 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> > This trys to shut up complains about invalid LVDS EDID during
> > mode probe, but uses fixed panel mode directly for panels with
> > broken EDID.
> > 
> > https://bugs.freedesktop.org/show_bug.cgi?id=23099
> > https://bugs.freedesktop.org/show_bug.cgi?id=26395
> > 
> > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> > Tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |    2 ++
> >  drivers/gpu/drm/i915/intel_lvds.c |   13 +++++++++----
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index b7cb4aa..6960849 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -611,6 +611,8 @@ typedef struct drm_i915_private {
> >  	/* Reclocking support */
> >  	bool render_reclock_avail;
> >  	bool lvds_downclock_avail;
> > +	/* indicate whether the LVDS EDID is OK */
> > +	bool lvds_edid_good;
> >  	/* indicates the reduced downclock for LVDS*/
> >  	int lvds_downclock;
> >  	struct work_struct idle_work;
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 3fa088c..8bbcb32 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -638,10 +638,12 @@ static int intel_lvds_get_modes(struct drm_connector *connector)
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int ret = 0;
> >  
> > -	ret = intel_ddc_get_modes(intel_encoder);
> > +	if (dev_priv->lvds_edid_good) {
> > +		ret = intel_ddc_get_modes(intel_encoder);
> >  
> > -	if (ret)
> > -		return ret;
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	/* Didn't get an EDID, so
> >  	 * Set wide sync ranges so we get all modes
> > @@ -1064,7 +1066,10 @@ void intel_lvds_init(struct drm_device *dev)
> >  	 * Attempt to get the fixed panel mode from DDC.  Assume that the
> >  	 * preferred mode is the right one.
> >  	 */
> > -	intel_ddc_get_modes(intel_encoder);
> > +	dev_priv->lvds_edid_good = true;
> > +
> > +	if (!intel_ddc_get_modes(intel_encoder))
> > +		dev_priv->lvds_edid_good = false;
> 
> I've applied this one to for-linus, though I'm not a huge fan.  We
> shouldn't be re-probing DDC on LVDS panels anyway -- there shouldn't be
> any circumstance when it can change.

Yes. The edid won't be changed for the LVDS panel. 

Can we cache the probed EDID for LVDS panel if we can get the EDID for
LVDS panel? Then when we try to probe LVDS EDID next time, we use the
cached EDID and don't probe the EDID really.(And now the LID state is
not used for the LVDS detection any more).It seems that the feature of
cached LVDS EDID is already used in the meego kernel.

Is this idea ok?
    
thanks.