[Intel-gfx] [PATCH 1/2] drm/i915: Enable LVDS downclock feature for LVDS

[ykzhao]

On Thu, 2009-11-19 at 07:09 +0800, Jesse Barnes wrote:
> On Tue, 17 Nov 2009 17:12:42 +0800
> yakui.zhao at intel.com wrote:
> 
> > From: Zhao Yakui <yakui.zhao at intel.com>
> > 
> > If more than one mode with the same resolution defined in EDID has
> > different refresh rate, it is thought that the downclock is found for
> > LVDS. We will program the different FPx0/1 register so that we can
> > select dynamically between the low and high frequency.
> 
> Cool, we've been wanting this for awhile.
>  
> > On the g4x platform we will use the CxSR feature to switch the
> > different refresh rate if the LVDS downclock feature is supported.
> 
> Great, may as well use the hardware when available...
> 
> > @@ -539,6 +539,8 @@ typedef struct drm_i915_private {
> >  	/* Reclocking support */
> >  	bool render_reclock_avail;
> >  	bool lvds_downclock_avail;
> > +	/* indicates the reduced downclock for LVDS*/
> > +	int	lvds_downclock;
> 
> Weird indentation, a single space should be sufficient.
OK. I will modify it.
> 
> >  /**
> > + * intel_find_lvds_downclock - find the reduced downclock for LVDS
> > in EDID
> > + * @dev: drm device
> > + * @connector: LVDS connector
> > + *
> > + * Find the reduced downclock for LVDS in EDID.
> > + */
> > +static void intel_find_lvds_downclock(struct drm_device *dev,
> > +				struct drm_connector *connector)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_display_mode *scan, *panel_fixed_mode;
> > +
> > +	panel_fixed_mode = dev_priv->panel_fixed_mode;
> > +	list_for_each_entry(scan, &connector->probed_modes, head) {
> > +		mutex_lock(&dev->mode_config.mutex);
> 
> We need to take the mutex across the whole scan, otherwise the list may
> change under us.
OK.
> 
> > @@ -1023,6 +1065,7 @@ void intel_lvds_init(struct drm_device *dev)
> >  			dev_priv->panel_fixed_mode =
> >  				drm_mode_duplicate(dev, scan);
> >  			mutex_unlock(&dev->mode_config.mutex);
> > +			intel_find_lvds_downclock(dev, connector);
> >  			goto out;
> >  		}
> >  		mutex_unlock(&dev->mode_config.mutex);
> 
> Is there a VBT equivalent?  Most of our panel timings come from VBT, so
> if we only support EDID we won't enable the feature very often (it's
> fine to add that as a separate patch though).
I collect some vbios.dumps from some laptops based on 965gm/gm45
platform. 
Unfortunately I don't find that more than one resolution modes with the
resolution has the different refresh rate. 

Of course it seems harmless to check this feature in VBT. I can add this
feature in a separate patch.

thanks for pointing out this issue.
 
> 
> Assuming you fix the above, you can add my Reviewed-by to the patch.
> 
> Thanks,