[Intel-gfx] [PATCH] drm/i915: fastboot support for HSW and BDW
Paulo Zanoni
przanoni at gmail.com
Mon Jan 13 20:55:27 CET 2014
2014/1/8 Jesse Barnes <jbarnes at virtuousgeek.org>:
> No idea if this is correct or not, all the WRPLL programming is new to
> me. Paulo, can you take a look? At least it doesn't complain on my BDW
> here...
As a side note: can't we add some debugfs interface that, when read,
would trigger a HW state readout, and then would compare it against
what we have? It would be the perfect thing to validate patches like
the one you wrote... And it would make QA easier (just write a test
case that sets modes and reads the debugfs interface! Or just modify
kms_setmode/testdisplay to do it everywhere).
Some comments:
- I guess the commit message could be improved :)
- We probably need to do something about the VGA output, which uses
the old intel_crt encoder instead of intel_digital_port. If this patch
doesn't regress anything, we could do it on a separate patch.
- Don't we also need to patch intel_pipe_config_compare() so it checks
the now-used adjusted_mode.crtc_clock?
- Based on that, don't we also need HW readout support for port_clock?
- You should probably compare your patch against "[PATCH 2/2]
drm/i915: add fast boot support for Haswell" sent by Furquan Shaikh in
August, and also check the reviews we gave to it.
- I tried booting it on my HSW machine that only has an eDP output,
and the driver doesn't load:
I get:
[ 62.635700] WARNING: CPU: 0 PID: 1120 at
drivers/gpu/drm/i915/intel_ddi.c:431
intel_ddi_get_crtc_encoder+0x95/0xa0 [i915]()
[ 62.635703] 0 encoders on crtc for pipe A
Then later I see:
[ 62.636200] kernel BUG at drivers/gpu/drm/i915/intel_ddi.c:433!
[ 62.636249] invalid opcode: 0000 [#1] SMP
Which is the "BUG_ON(ret == NULL);" line
Please notice that I'm not using the i915_fastboot command line argument.
More below:
>
> Thanks,
> Jesse
> ---
> drivers/gpu/drm/i915/i915_reg.h | 6 ++++
> drivers/gpu/drm/i915/intel_ddi.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 4 files changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a699efd..644e4f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5318,8 +5318,13 @@
> #define WRPLL_PLL_SELECT_LCPLL_2700 (0x03<<28)
> /* WRPLL divider programming */
> #define WRPLL_DIVIDER_REFERENCE(x) ((x)<<0)
> +#define WRPLL_DIVIDER_REF_MASK (0xff)
> #define WRPLL_DIVIDER_POST(x) ((x)<<8)
> +#define WRPLL_DIVIDER_POST_MASK (0x3f<<8)
> +#define WRPLL_DIVIDER_POST_SHIFT 8
> #define WRPLL_DIVIDER_FEEDBACK(x) ((x)<<16)
> +#define WRPLL_DIVIDER_FB_SHIFT 16
> +#define WRPLL_DIVIDER_FB_MASK (0xff<<16)
>
> /* Port clock selection */
> #define PORT_CLK_SEL_A 0x46100
> @@ -5332,6 +5337,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 1488b28..f3d7b42 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -413,7 +413,7 @@ static void intel_ddi_mode_set(struct intel_encoder *encoder)
> }
> }
>
> -static struct intel_encoder *
> +struct intel_encoder *
> intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 74137d5..f87244d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -48,6 +48,8 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> struct intel_crtc_config *pipe_config);
> static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> struct intel_crtc_config *pipe_config);
> +static void haswell_ddi_clock_get(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config);
>
> static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
> int x, int y, struct drm_framebuffer *old_fb);
> @@ -6994,10 +6996,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> tmp = I915_READ(FDI_RX_CTL(PIPE_A));
> pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
> FDI_DP_PORT_WIDTH_SHIFT) + 1;
> -
> - ironlake_get_fdi_m_n_config(crtc, pipe_config);
> }
>
> + ironlake_get_fdi_m_n_config(crtc, pipe_config);
This is a little confusing, probably wrong, because FDI is only used
when has_pch_encoder is true, but you're calling this code on all
cases.
Function ironlake_get_fdi_m_n_config() calls
intel_cpu_transcoder_get_m_n() for eDP/DP/HDMI.
However, we have encoder->get_config which calls
intel_ddi_get_config() which calls intel_dp_get_m_n(), which will call
intel_cpu_transcoder_get_m_n() again on the same cases. So we call the
same thing twice.
I would imagine you probably don't need to move this line above, but I
may be wrong. If something is wrong, you probably need to fix some
other code that's assuming that "gen5+ DP m/n registers are on the
PCH" or something like that.
> + haswell_ddi_clock_get(crtc, pipe_config);
> +
> intel_get_pipe_timings(crtc, pipe_config);
>
> pfit_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> @@ -8034,6 +8037,60 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> &pipe_config->fdi_m_n);
> }
>
> +#define LC_FREQ 2700
> +
> +static int intel_ddi_calc_wrpll_link(u32 wrpll)
> +{
> + int n, p, r;
> +
> + r = wrpll & WRPLL_DIVIDER_REF_MASK;
> + p = (wrpll & WRPLL_DIVIDER_POST_MASK) >> WRPLL_DIVIDER_POST_SHIFT;
> + n = (wrpll & WRPLL_DIVIDER_FB_MASK) >> WRPLL_DIVIDER_FB_SHIFT;
> +
> + return (LC_FREQ * n) / (p * r);
> +}
> +
> +static void haswell_ddi_clock_get(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config)
> +{
> + struct intel_encoder *intel_encoder =
> + intel_ddi_get_crtc_encoder(&crtc->base);
> + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> + enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + int link_clock;
> + u32 val, pll;
> +
> + val = I915_READ(PORT_CLK_SEL(port));
> + switch (val & PORT_CLK_SEL_MASK) {
> + case PORT_CLK_SEL_LCPLL_810:
> + link_clock = 81000;
> + break;
> + case PORT_CLK_SEL_LCPLL_1350:
> + link_clock = 135000;
> + break;
> + case PORT_CLK_SEL_LCPLL_2700:
> + link_clock = 270000;
> + break;
> + case PORT_CLK_SEL_WRPLL1:
> + pll = I915_READ(WRPLL_CTL1);
> + link_clock = intel_ddi_calc_wrpll_link(pll);
> + break;
> + case PORT_CLK_SEL_WRPLL2:
> + pll = I915_READ(WRPLL_CTL2);
> + link_clock = intel_ddi_calc_wrpll_link(pll);
> + break;
> + case PORT_CLK_SEL_SPLL:
> + link_clock = 135000;
> + break;
> + default:
> + WARN(1, "bad port clock sel\n");
> + return;
> + }
> +
> + pipe_config->adjusted_mode.crtc_clock =
> + intel_dotclock_calculate(link_clock, &pipe_config->fdi_m_n);
> +}
> +
> /** Returns the currently programmed mode of the given pipe. */
> struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 46aea6c..7bfc19a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -604,6 +604,7 @@ void intel_prepare_ddi(struct drm_device *dev);
> void hsw_fdi_link_train(struct drm_crtc *crtc);
> void intel_ddi_init(struct drm_device *dev, enum port port);
> enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
> +struct intel_encoder *intel_ddi_get_crtc_encoder(struct drm_crtc *crtc);
> bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
> int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv);
> void intel_ddi_pll_init(struct drm_device *dev);
> --
> 1.8.3.2
>
--
Paulo Zanoni
More information about the Intel-gfx
mailing list