Hi Ville,
On Fri, Sep 18, 2015 at 08:03:45PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
