Reviewed-by: Rodrigo Vivi <rodrigo.v...@gmail.com>

On Thu, Mar 15, 2012 at 11:42 AM, Takashi Iwai <ti...@suse.de> wrote:
> At Thu, 15 Mar 2012 14:30:35 +0100,
> Takashi Iwai wrote:
>>
>> At Thu, 15 Mar 2012 13:25:08 +0000,
>> Chris Wilson wrote:
>> >
>> > On Thu, 15 Mar 2012 14:15:54 +0100, Takashi Iwai <ti...@suse.de> wrote:
>> > > +static bool is_dual_link_lvds(struct drm_i915_private *dev_priv)
>> > > +{
>> > > + /* BIOS should set the proper LVDS register value at boot, but
>> > > +  * in reality, it doesn't set the value when the lid is closed;
>> > > +  * thus when a machine is booted with the lid closed, the LVDS
>> > > +  * reg value can't be trusted.  So we need to check "the value
>> > > +  * to be set" in VBT at first.
>> > > +  */
>> > > + if ((dev_priv->bios_lvds_val & LVDS_CLKB_POWER_MASK) ==
>> > > +     LVDS_CLKB_POWER_UP)
>> > > +         return true;
>> > > + if ((I915_READ(PCH_LVDS) & LVDS_CLKB_POWER_MASK) ==
>> >
>> > This is either PCH_LVDS or LVDS depending on the generation.
>>
>> Oh, right.  The revised patch is below.
>
> One more change.  Now it checks the resolution to be sure.
>
>
> Takashi
>
> ---
> From: Takashi Iwai <ti...@suse.de>
> Subject: [PATCH v3] drm/i915: Check VBIOS value for determining LVDS dual 
> channel mode, too
>
> Currently i915 driver checks [PCH_]LVDS register bits to decide
> whether to set up the dual-link or the single-link mode.  This relies
> implicitly on that BIOS initializes the register properly at boot.
> However, BIOS doesn't initialize it always.  When the machine is
> booted with the closed lid, BIOS skips the LVDS reg initialization.
> This ends up in blank output on a machine with a dual-link LVDS when
> you open the lid after the boot.
>
> This patch adds a workaround for that problem by checking the initial
> LVDS register value in VBT.
>
> Signed-off-by: Takashi Iwai <ti...@suse.de>
> ---
> v1->v2: Fix the register for gen<=4
> v2->v3: Check the resolution of the entry to be sure
>
>  drivers/gpu/drm/i915/i915_drv.h      |    1 +
>  drivers/gpu/drm/i915/intel_bios.c    |   26 ++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |   26 ++++++++++++++++++++------
>  3 files changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9689ca3..8c8e488 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -376,6 +376,7 @@ typedef struct drm_i915_private {
>        unsigned int lvds_use_ssc:1;
>        unsigned int display_clock_mode:1;
>        int lvds_ssc_freq;
> +       unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
>        struct {
>                int rate;
>                int lanes;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 63880e2..6b93f92 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -173,6 +173,27 @@ get_lvds_dvo_timing(const struct bdb_lvds_lfp_data 
> *lvds_lfp_data,
>        return (struct lvds_dvo_timing *)(entry + dvo_timing_offset);
>  }
>
> +/* read the initial LVDS register value for the given panel mode */
> +static unsigned int get_lvds_reg_val(const struct bdb_header *bdb,
> +                                    const struct bdb_lvds_lfp_data_ptrs 
> *ptrs,
> +                                    int index,
> +                                    struct drm_display_mode *mode)
> +{
> +       unsigned int ofs;
> +       const struct lvds_fp_timing *timing;
> +
> +       if (index >= ARRAY_SIZE(ptrs->ptr))
> +               return 0;
> +       ofs = ptrs->ptr[index].fp_timing_offset;
> +       if (ofs + sizeof(*timing) > bdb->bdb_size)
> +               return 0;
> +       timing = (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
> +       /* check the resolution, just to be sure */
> +       if (timing->x_res != mode->hdisplay || timing->y_res != 
> mode->vdisplay)
> +               return 0;
> +       return timing->lvds_reg_val;
> +}
> +
>  /* Try to find integrated panel data */
>  static void
>  parse_lfp_panel_data(struct drm_i915_private *dev_priv,
> @@ -243,6 +264,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
>                              "Normal Clock %dKHz, downclock %dKHz\n",
>                              panel_fixed_mode->clock, 10*downclock);
>        }
> +
> +       dev_priv->bios_lvds_val = get_lvds_reg_val(bdb, lvds_lfp_data_ptrs,
> +                                                  lvds_options->panel_type,
> +                                                  panel_fixed_mode);
> +       DRM_DEBUG_KMS("VBT initial LVDS value %x\n", dev_priv->bios_lvds_val);
>  }
>
>  /* Try to find sdvo panel data */
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index f851db7..314af26 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -356,6 +356,23 @@ static const intel_limit_t 
> intel_limits_ironlake_display_port = {
>        .find_pll = intel_find_pll_ironlake_dp,
>  };
>
> +static bool is_dual_link_lvds(struct drm_i915_private *dev_priv,
> +                             unsigned int reg)
> +{
> +       /* BIOS should set the proper LVDS register value at boot, but
> +        * in reality, it doesn't set the value when the lid is closed;
> +        * thus when a machine is booted with the lid closed, the LVDS
> +        * reg value can't be trusted.  So we need to check "the value
> +        * to be set" in VBT at first.
> +        */
> +       if ((dev_priv->bios_lvds_val & LVDS_CLKB_POWER_MASK) ==
> +           LVDS_CLKB_POWER_UP)
> +               return true;
> +       if ((I915_READ(reg) & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP)
> +               return true;
> +       return false;
> +}
> +
>  static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
>                                                int refclk)
>  {
> @@ -364,8 +381,7 @@ static const intel_limit_t *intel_ironlake_limit(struct 
> drm_crtc *crtc,
>        const intel_limit_t *limit;
>
>        if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> -               if ((I915_READ(PCH_LVDS) & LVDS_CLKB_POWER_MASK) ==
> -                   LVDS_CLKB_POWER_UP) {
> +               if (is_dual_link_lvds(dev_priv, PCH_LVDS)) {
>                        /* LVDS dual channel */
>                        if (refclk == 100000)
>                                limit = &intel_limits_ironlake_dual_lvds_100m;
> @@ -393,8 +409,7 @@ static const intel_limit_t *intel_g4x_limit(struct 
> drm_crtc *crtc)
>        const intel_limit_t *limit;
>
>        if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> -               if ((I915_READ(LVDS) & LVDS_CLKB_POWER_MASK) ==
> -                   LVDS_CLKB_POWER_UP)
> +               if (is_dual_link_lvds(dev_priv, LVDS))
>                        /* LVDS with dual channel */
>                        limit = &intel_limits_g4x_dual_channel_lvds;
>                else
> @@ -531,8 +546,7 @@ intel_find_best_PLL(const intel_limit_t *limit, struct 
> drm_crtc *crtc,
>                 * reliably set up different single/dual channel state, if we
>                 * even can.
>                 */
> -               if ((I915_READ(LVDS) & LVDS_CLKB_POWER_MASK) ==
> -                   LVDS_CLKB_POWER_UP)
> +               if (is_dual_link_lvds(dev_priv, LVDS))
>                        clock.p2 = limit->p2.p2_fast;
>                else
>                        clock.p2 = limit->p2.p2_slow;
> --
> 1.7.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
GPG: 0x905BE242 @ wwwkeys.pgp.net
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to