Re: [PATCH 2/2] drm/i915: add fast boot support for Haswell

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

2013/8/19 Furquan Shaikh <furquan@xxxxxxxxxx>:
> Enables getting correct mode clock when reading pipe config
> This patch has been tested successfully on top of drm-intel-nightly tree
>
> Signed-off-by: Furquan Shaikh <furquan@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  6 ++++
>  drivers/gpu/drm/i915/intel_ddi.c     | 70 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  1 +
>  3 files changed, 77 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2b96d6b..9511129 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4970,6 +4970,7 @@
>  #define  SPLL_PLL_NON_SSC              (2<<28)
>  #define  SPLL_PLL_FREQ_810MHz          (0<<26)
>  #define  SPLL_PLL_FREQ_1350MHz         (1<<26)
> +#define  SPLL_PLL_FREQ_MASK             (3<<26)
>
>  /* WRPLL */
>  #define WRPLL_CTL1                     0x46040
> @@ -4983,6 +4984,10 @@
>  #define  WRPLL_DIVIDER_POST(x)         ((x)<<8)
>  #define  WRPLL_DIVIDER_FEEDBACK(x)     ((x)<<16)
>
> +#define WRPLL_DIVIDER_REFERENCE_BITS(reg)    (reg & 0xff)
> +#define WRPLL_DIVIDER_POST_BITS(reg)         ((reg >> 8) & 0x3f)
> +#define WRPLL_DIVIDER_FEEDBACK_BITS(reg)     ((reg >> 16) & 0xff)
> +
>  /* Port clock selection */
>  #define PORT_CLK_SEL_A                 0x46100
>  #define PORT_CLK_SEL_B                 0x46104
> @@ -4994,6 +4999,7 @@
>  #define  PORT_CLK_SEL_WRPLL1           (4<<29)
>  #define  PORT_CLK_SEL_WRPLL2           (5<<29)
>  #define  PORT_CLK_SEL_NONE             (7<<29)
> +#define  PORT_CLK_SEL_MASK              (7<<29)
>
>  /* Transcoder clock selection */
>  #define TRANS_CLK_SEL_A                        0x46140
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b8c096b..a24a375 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1246,6 +1246,74 @@ static void intel_ddi_hot_plug(struct intel_encoder *intel_encoder)
>                 intel_dp_check_link_status(intel_dp);
>  }
>
> +static void intel_ddi_get_clock(struct intel_encoder *encoder,
> +                               struct intel_crtc_config *pipe_config)
> +{
> +       struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +       int port = intel_ddi_get_encoder_port(encoder);
> +       u32 temp = I915_READ(PORT_CLK_SEL(port)) & PORT_CLK_SEL_MASK;
> +       u8 link_bw;
> +
> +       if ((temp == PORT_CLK_SEL_LCPLL_810) ||
> +           (temp == PORT_CLK_SEL_LCPLL_1350) ||
> +           (temp == PORT_CLK_SEL_LCPLL_2700)) {
> +
> +               switch (temp) {
> +               case PORT_CLK_SEL_LCPLL_810:
> +                       link_bw = DP_LINK_BW_1_62;
> +                       break;
> +               case PORT_CLK_SEL_LCPLL_1350:
> +                       link_bw = DP_LINK_BW_2_7;
> +                       break;
> +               case PORT_CLK_SEL_LCPLL_2700:
> +                       link_bw = DP_LINK_BW_5_4;
> +                       break;
> +               }
> +               pipe_config->port_clock = drm_dp_bw_code_to_link_rate(link_bw);

drivers/gpu/drm/i915/intel_ddi.c:1275:56: aviso: ‘link_bw’ may be used
uninitialized in this function [-Wmaybe-uninitialized]

Looks like gcc 4.7.3 isn't that smart :( You should probably add a
"default" case with a WARN_ON(1) inside it.

> +       } else if ((temp == PORT_CLK_SEL_WRPLL1) ||
> +                  (temp == PORT_CLK_SEL_WRPLL2)) {
> +               uint64_t p, n2, r2;
> +               u32 val, reg;
> +               uint64_t freq2k;
> +
> +               switch (temp) {
> +               case PORT_CLK_SEL_WRPLL1:
> +                       reg = WRPLL_CTL1;
> +                       break;
> +               case PORT_CLK_SEL_WRPLL2:
> +                       reg = WRPLL_CTL2;
> +                       break;
> +               }
> +               val = I915_READ(reg);

drivers/gpu/drm/i915/intel_ddi.c:1290:7: aviso: ‘reg’ may be used
uninitialized in this function [-Wmaybe-uninitialized]

Same thing here.


> +               r2 = WRPLL_DIVIDER_REFERENCE_BITS(val);
> +               p = WRPLL_DIVIDER_POST_BITS(val);
> +               n2 = WRPLL_DIVIDER_FEEDBACK_BITS(val);
> +
> +               freq2k = DIV_ROUND_UP_ULL(LC_FREQ * n2, r2 * p);
> +               pipe_config->port_clock = freq2k / 10;

If you put the "/ 10" on the line above so it becomes
DIV_ROUND_UP_ULL(LC_FREQ * n2, r2 * p * 10) you'll avoid the rounding
errors of 1 division.

By the way, I applied your patch + Daniel's suggestion on -nightly and
now I see "unclaimed register" errors when booting with HDMI + eDP.
The problem is that we try to read disabled LINK_M/LINK_N registers
due to the power well disabled, so we get the error messages. I also
once saw a "pipe state mismatch" error, but I can't find a way to
reproduce it now. I'm also not sure whether VGA will really work since
we don't touch intel_crt.c (I can't test it for now). When testing the
patches, please always check dmesg for errors or warns, we should have
none. It looks to me that enabling fastboot on Haswell won't be so
simple due to these error messages I'm seeing. You should probably
convince Jesse to fix these problems :)

Thanks for the patches,
Paulo


> +       } else if (temp == PORT_CLK_SEL_SPLL) {
> +               u32 val;
> +
> +               val = I915_READ(PORT_CLK_SEL_SPLL);
> +
> +               switch (val & SPLL_PLL_FREQ_MASK) {
> +               case SPLL_PLL_FREQ_810MHz:
> +                       pipe_config->port_clock = 810000;
> +                       break;
> +               case SPLL_PLL_FREQ_1350MHz:
> +                       pipe_config->port_clock = 1350000;
> +                       break;
> +               default:
> +                       WARN(1, "Invalid pll freq %d\n",
> +                            val & SPLL_PLL_FREQ_MASK);
> +                       return;
> +               }
> +       } else {
> +               WARN(1, "Invalid clock selection %d\n", temp);
> +               return;
> +       }
> +}
> +
>  static void intel_ddi_get_config(struct intel_encoder *encoder,
>                                  struct intel_crtc_config *pipe_config)
>  {
> @@ -1265,6 +1333,8 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
>                 flags |= DRM_MODE_FLAG_NVSYNC;
>
>         pipe_config->adjusted_mode.flags |= flags;
> +
> +       intel_ddi_get_clock(encoder, pipe_config);
>  }
>
>  static void intel_ddi_destroy(struct drm_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e0069f4..1c6a3d2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9579,6 +9579,7 @@ static void intel_init_display(struct drm_device *dev)
>
>         if (HAS_DDI(dev)) {
>                 dev_priv->display.get_pipe_config = haswell_get_pipe_config;
> +               dev_priv->display.get_clock = ironlake_crtc_clock_get;
>                 dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
>                 dev_priv->display.crtc_enable = haswell_crtc_enable;
>                 dev_priv->display.crtc_disable = haswell_crtc_disable;
> --
> 1.8.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux