[Intel-gfx] [PATCH 32/43] drm/i915: Clean up LVDS register handling

[Lukas Wunner]

Hi Ville,

On Fri, Sep 18, 2015 at 08:03:45PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Keep single 'lvds_reg' and 'lvds' variable around in
> intel_lvds_init(), and read it just once at the start.

Hm, is it intentional that you didn't also replace this register readout
at the end of the function?

	lvds_encoder->a3_power = I915_READ(lvds_encoder->reg) &
				 LVDS_A3_POWER_MASK;

(Sorry, I only noticed this today, post-merge, while rebasing my LVDS
reprobing stuff.)

Best regards,

Lukas

> 
> Also intel_lvds_get_config() doesn't need to figure out which reg to use
> since it can just consult lvds_encoder->reg.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lvds.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 2c2d1f0..35bad71 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -98,15 +98,11 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
>  {
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 lvds_reg, tmp, flags = 0;
> +	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> +	u32 tmp, flags = 0;
>  	int dotclock;
>  
> -	if (HAS_PCH_SPLIT(dev))
> -		lvds_reg = PCH_LVDS;
> -	else
> -		lvds_reg = LVDS;
> -
> -	tmp = I915_READ(lvds_reg);
> +	tmp = I915_READ(lvds_encoder->reg);
>  	if (tmp & LVDS_HSYNC_POLARITY)
>  		flags |= DRM_MODE_FLAG_NHSYNC;
>  	else
> @@ -944,6 +940,7 @@ void intel_lvds_init(struct drm_device *dev)
>  	struct drm_display_mode *downclock_mode = NULL;
>  	struct edid *edid;
>  	struct drm_crtc *crtc;
> +	u32 lvds_reg;
>  	u32 lvds;
>  	int pipe;
>  	u8 pin;
> @@ -966,8 +963,15 @@ void intel_lvds_init(struct drm_device *dev)
>  	if (dmi_check_system(intel_no_lvds))
>  		return;
>  
> +	if (HAS_PCH_SPLIT(dev))
> +		lvds_reg = PCH_LVDS;
> +	else
> +		lvds_reg = LVDS;
> +
> +	lvds = I915_READ(lvds_reg);
> +
>  	if (HAS_PCH_SPLIT(dev)) {
> -		if ((I915_READ(PCH_LVDS) & LVDS_DETECTED) == 0)
> +		if ((lvds & LVDS_DETECTED) == 0)
>  			return;
>  		if (dev_priv->vbt.edp_support) {
>  			DRM_DEBUG_KMS("disable LVDS for eDP support\n");
> @@ -977,8 +981,7 @@ void intel_lvds_init(struct drm_device *dev)
>  
>  	pin = GMBUS_PIN_PANEL;
>  	if (!lvds_is_present_in_vbt(dev, &pin)) {
> -		u32 reg = HAS_PCH_SPLIT(dev) ? PCH_LVDS : LVDS;
> -		if ((I915_READ(reg) & LVDS_PORT_EN) == 0) {
> +		if ((lvds & LVDS_PORT_EN) == 0) {
>  			DRM_DEBUG_KMS("LVDS is not present in VBT\n");
>  			return;
>  		}
> @@ -1055,11 +1058,7 @@ void intel_lvds_init(struct drm_device *dev)
>  	connector->interlace_allowed = false;
>  	connector->doublescan_allowed = false;
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> -		lvds_encoder->reg = PCH_LVDS;
> -	} else {
> -		lvds_encoder->reg = LVDS;
> -	}
> +	lvds_encoder->reg = lvds_reg;
>  
>  	/* create the scaling mode property */
>  	drm_mode_create_scaling_mode_property(dev);
> @@ -1140,7 +1139,6 @@ void intel_lvds_init(struct drm_device *dev)
>  	if (HAS_PCH_SPLIT(dev))
>  		goto failed;
>  
> -	lvds = I915_READ(LVDS);
>  	pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
>  	crtc = intel_get_crtc_for_pipe(dev, pipe);
>  
> -- 
> 2.4.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx