From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 7/8] drm/i915: create pipe_config->dpll for clock state
Date: Tue, 2 Apr 2013 14:14:23 -0700 [thread overview]
Message-ID: <20130402141423.1035829f@jbarnes-desktop> (raw)
In-Reply-To: <1364463723-30252-8-git-send-email-daniel.vetter@ffwll.ch>
On Thu, 28 Mar 2013 10:42:02 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Clock computations and handling are highly encoder specific, both in
> the optimal clock selection and also in which clocks to use and when
> sharing of clocks is possible.
>
> So the best place to do this is somewhere in the encoders, with a
> generic fallback for those encoders without special needs. To facility
> this, add a pipe_config->clocks_set boolean.
>
> This patch here is only prep work, it simply sets the computed clock
> values in pipe_config->dpll, and uses that data in the hw clock
> setting functions.
>
> Haswell code isn't touched, simply because Haswell clocks work much
> different and need their own infrastructure (with probably a
> Haswell-specific config->ddi_clock substruct).
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 155 +++++++++++++++++++----------------
> drivers/gpu/drm/i915/intel_drv.h | 12 +++
> 2 files changed, 95 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c9e873e..5319133 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4118,37 +4118,38 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
> return refclk;
> }
>
> -static void i9xx_adjust_sdvo_tv_clock(struct drm_display_mode *adjusted_mode,
> - intel_clock_t *clock)
> +static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
> {
> + unsigned dotclock = crtc->config.adjusted_mode.clock;
> + struct dpll *clock = &crtc->config.dpll;
> +
> /* SDVO TV has fixed PLL values depend on its clock range,
> this mirrors vbios setting. */
> - if (adjusted_mode->clock >= 100000
> - && adjusted_mode->clock < 140500) {
> + if (dotclock >= 100000 && dotclock < 140500) {
> clock->p1 = 2;
> clock->p2 = 10;
> clock->n = 3;
> clock->m1 = 16;
> clock->m2 = 8;
> - } else if (adjusted_mode->clock >= 140500
> - && adjusted_mode->clock <= 200000) {
> + } else if (dotclock >= 140500 && dotclock <= 200000) {
> clock->p1 = 1;
> clock->p2 = 10;
> clock->n = 6;
> clock->m1 = 12;
> clock->m2 = 8;
> }
> +
> + crtc->config.clock_set = true;
> }
>
> -static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
> - intel_clock_t *clock,
> +static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
> intel_clock_t *reduced_clock)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 fp, fp2 = 0;
> + struct dpll *clock = &crtc->config.dpll;
>
> if (IS_PINEVIEW(dev)) {
> fp = (1 << clock->n) << 16 | clock->m1 << 8 | clock->m2;
> @@ -4164,11 +4165,11 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
>
> I915_WRITE(FP0(pipe), fp);
>
> - intel_crtc->lowfreq_avail = false;
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> + crtc->lowfreq_avail = false;
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> reduced_clock && i915_powersave) {
> I915_WRITE(FP1(pipe), fp2);
> - intel_crtc->lowfreq_avail = true;
> + crtc->lowfreq_avail = true;
> } else {
> I915_WRITE(FP1(pipe), fp);
> }
> @@ -4182,14 +4183,11 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
> intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
> }
>
> -static void vlv_update_pll(struct drm_crtc *crtc,
> - intel_clock_t *clock, intel_clock_t *reduced_clock,
> - int num_connectors)
> +static void vlv_update_pll(struct intel_crtc *crtc)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 dpll, mdiv, pdiv;
> u32 bestn, bestm1, bestm2, bestp1, bestp2;
> bool is_sdvo;
> @@ -4197,8 +4195,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>
> mutex_lock(&dev_priv->dpio_lock);
>
> - is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> - intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> + is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> + intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>
> dpll = DPLL_VGA_MODE_DIS;
> dpll |= DPLL_EXT_BUFFER_ENABLE_VLV;
> @@ -4208,11 +4206,11 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> I915_WRITE(DPLL(pipe), dpll);
> POSTING_READ(DPLL(pipe));
>
> - bestn = clock->n;
> - bestm1 = clock->m1;
> - bestm2 = clock->m2;
> - bestp1 = clock->p1;
> - bestp2 = clock->p2;
> + bestn = crtc->config.dpll.n;
> + bestm1 = crtc->config.dpll.m1;
> + bestm2 = crtc->config.dpll.m2;
> + bestp1 = crtc->config.dpll.p1;
> + bestp2 = crtc->config.dpll.p2;
>
> /*
> * In Valleyview PLL and program lane counter registers are exposed
> @@ -4244,8 +4242,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>
> intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
>
> - if (intel_crtc->config.has_dp_encoder)
> - intel_dp_set_m_n(intel_crtc);
> + if (crtc->config.has_dp_encoder)
> + intel_dp_set_m_n(crtc);
>
> I915_WRITE(DPLL(pipe), dpll);
>
> @@ -4256,8 +4254,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> temp = 0;
> if (is_sdvo) {
> temp = 0;
> - if (intel_crtc->config.pixel_multiplier > 1) {
> - temp = (intel_crtc->config.pixel_multiplier - 1)
> + if (crtc->config.pixel_multiplier > 1) {
> + temp = (crtc->config.pixel_multiplier - 1)
> << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> }
> }
> @@ -4265,16 +4263,15 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> POSTING_READ(DPLL_MD(pipe));
>
> /* Now program lane control registers */
> - if(intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)
> - || intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> - {
> + if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)
> + || intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) {
> temp = 0x1000C4;
> if(pipe == 1)
> temp |= (1 << 21);
> intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL1, temp);
> }
> - if(intel_pipe_has_type(crtc,INTEL_OUTPUT_EDP))
> - {
> +
> + if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP)) {
> temp = 0x1000C4;
> if(pipe == 1)
> temp |= (1 << 21);
> @@ -4284,39 +4281,39 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> mutex_unlock(&dev_priv->dpio_lock);
> }
>
> -static void i9xx_update_pll(struct drm_crtc *crtc,
> - intel_clock_t *clock, intel_clock_t *reduced_clock,
> +static void i9xx_update_pll(struct intel_crtc *crtc,
> + intel_clock_t *reduced_clock,
> int num_connectors)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 dpll;
> bool is_sdvo;
> + struct dpll *clock = &crtc->config.dpll;
>
> - i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> + i9xx_update_pll_dividers(crtc, reduced_clock);
>
> - is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> - intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> + is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> + intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>
> dpll = DPLL_VGA_MODE_DIS;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))
> dpll |= DPLLB_MODE_LVDS;
> else
> dpll |= DPLLB_MODE_DAC_SERIAL;
>
> if (is_sdvo) {
> - if ((intel_crtc->config.pixel_multiplier > 1) &&
> + if ((crtc->config.pixel_multiplier > 1) &&
> (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) {
> - dpll |= (intel_crtc->config.pixel_multiplier - 1)
> + dpll |= (crtc->config.pixel_multiplier - 1)
> << SDVO_MULTIPLIER_SHIFT_HIRES;
> }
> dpll |= DPLL_DVO_HIGH_SPEED;
> }
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
> dpll |= DPLL_DVO_HIGH_SPEED;
>
> /* compute bitmask from p1 value */
> @@ -4344,13 +4341,13 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> if (INTEL_INFO(dev)->gen >= 4)
> dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
>
> - if (is_sdvo && intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> + if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
> dpll |= PLL_REF_INPUT_TVCLKINBC;
> - else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> + else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
> /* XXX: just matching BIOS for now */
> /* dpll |= PLL_REF_INPUT_TVCLKINBC; */
> dpll |= 3;
> - else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> + else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
> else
> @@ -4361,12 +4358,12 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> POSTING_READ(DPLL(pipe));
> udelay(150);
>
> - for_each_encoder_on_crtc(dev, crtc, encoder)
> + for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> if (encoder->pre_pll_enable)
> encoder->pre_pll_enable(encoder);
>
> - if (intel_crtc->config.has_dp_encoder)
> - intel_dp_set_m_n(intel_crtc);
> + if (crtc->config.has_dp_encoder)
> + intel_dp_set_m_n(crtc);
>
> I915_WRITE(DPLL(pipe), dpll);
>
> @@ -4378,8 +4375,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> u32 temp = 0;
> if (is_sdvo) {
> temp = 0;
> - if (intel_crtc->config.pixel_multiplier > 1) {
> - temp = (intel_crtc->config.pixel_multiplier - 1)
> + if (crtc->config.pixel_multiplier > 1) {
> + temp = (crtc->config.pixel_multiplier - 1)
> << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> }
> }
> @@ -4394,23 +4391,23 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> }
> }
>
> -static void i8xx_update_pll(struct drm_crtc *crtc,
> +static void i8xx_update_pll(struct intel_crtc *crtc,
> struct drm_display_mode *adjusted_mode,
> - intel_clock_t *clock, intel_clock_t *reduced_clock,
> + intel_clock_t *reduced_clock,
> int num_connectors)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 dpll;
> + struct dpll *clock = &crtc->config.dpll;
>
> - i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> + i9xx_update_pll_dividers(crtc, reduced_clock);
>
> dpll = DPLL_VGA_MODE_DIS;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
> dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
> } else {
> if (clock->p1 == 2)
> @@ -4421,7 +4418,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
> dpll |= PLL_P2_DIVIDE_BY_4;
> }
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
> else
> @@ -4432,7 +4429,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
> POSTING_READ(DPLL(pipe));
> udelay(150);
>
> - for_each_encoder_on_crtc(dev, crtc, encoder)
> + for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> if (encoder->pre_pll_enable)
> encoder->pre_pll_enable(encoder);
>
> @@ -4579,20 +4576,26 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> &clock,
> &reduced_clock);
> }
> + /* Compat-code for transition, will disappear. */
> + if (!intel_crtc->config.clock_set) {
> + intel_crtc->config.dpll.n = clock.n;
> + intel_crtc->config.dpll.m1 = clock.m1;
> + intel_crtc->config.dpll.m2 = clock.m2;
> + intel_crtc->config.dpll.p1 = clock.p1;
> + intel_crtc->config.dpll.p2 = clock.p2;
> + }
>
> if (is_sdvo && is_tv)
> - i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
> + i9xx_adjust_sdvo_tv_clock(intel_crtc);
>
> if (IS_GEN2(dev))
> - i8xx_update_pll(crtc, adjusted_mode, &clock,
> + i8xx_update_pll(intel_crtc, adjusted_mode,
> has_reduced_clock ? &reduced_clock : NULL,
> num_connectors);
> else if (IS_VALLEYVIEW(dev))
> - vlv_update_pll(crtc, &clock,
> - has_reduced_clock ? &reduced_clock : NULL,
> - num_connectors);
> + vlv_update_pll(intel_crtc);
> else
> - i9xx_update_pll(crtc, &clock,
> + i9xx_update_pll(intel_crtc,
> has_reduced_clock ? &reduced_clock : NULL,
> num_connectors);
>
> @@ -5221,7 +5224,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
> }
>
> if (is_sdvo && is_tv)
> - i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
> + i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
>
> return true;
> }
> @@ -5525,6 +5528,14 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> DRM_ERROR("Couldn't find PLL settings for mode!\n");
> return -EINVAL;
> }
> + /* Compat-code for transition, will disappear. */
> + if (!intel_crtc->config.clock_set) {
> + intel_crtc->config.dpll.n = clock.n;
> + intel_crtc->config.dpll.m1 = clock.m1;
> + intel_crtc->config.dpll.m2 = clock.m2;
> + intel_crtc->config.dpll.p1 = clock.p1;
> + intel_crtc->config.dpll.p2 = clock.p2;
> + }
>
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2a86a12..d27885a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -196,6 +196,18 @@ struct intel_crtc_config {
> * accordingly. */
> bool has_dp_encoder;
> bool dither;
> +
> + /* Controls for the clock computation, to override various stages. */
> + bool clock_set;
> +
> + /* Settings for the intel dpll used on pretty much everything but
> + * haswell. */
> + struct dpll {
> + unsigned n;
> + unsigned m1, m2;
> + unsigned p1, p2;
> + } dpll;
> +
> int pipe_bpp;
> struct intel_link_m_n dp_m_n;
> /**
This one's hard to review since you mixed in a drm_crtc->intel_crtc
function arg change.
I'd rather have that split out, but it looks ok.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
next prev parent reply other threads:[~2013-04-02 21:14 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-28 9:41 [PATCH 0/8] dp/fdi m/n rework + basic pipe_config readout Daniel Vetter
2013-03-28 9:41 ` [PATCH 1/8] drm/i915: clear up the fdi/dp set_m_n confusion Daniel Vetter
2013-04-02 20:47 ` Jesse Barnes
2013-04-02 20:49 ` Jesse Barnes
2013-04-02 21:38 ` [PATCH] " Daniel Vetter
2013-03-28 9:41 ` [PATCH 2/8] drm/i915: move dp_m_n computation to dp_encoder->compute_config Daniel Vetter
2013-04-02 20:51 ` Jesse Barnes
2013-04-02 21:42 ` [PATCH] " Daniel Vetter
2013-03-28 9:41 ` [PATCH 3/8] drm/i915: track dp target_clock in pipe_config Daniel Vetter
2013-04-02 20:56 ` Jesse Barnes
2013-04-02 21:27 ` [PATCH] drm/i915: remove leaky eDP functions Daniel Vetter
2013-04-02 21:30 ` Jesse Barnes
2013-03-28 9:41 ` [PATCH 4/8] drm/i915: rip out superflous is_dp&is_cpu_edp tracking Daniel Vetter
2013-04-02 21:01 ` Jesse Barnes
2013-03-28 9:42 ` [PATCH 5/8] drm/i915: add hw state readout/checking for pipe_config Daniel Vetter
2013-04-02 21:05 ` Jesse Barnes
2013-03-28 9:42 ` [PATCH 6/8] drm/i915: hw readout support for ->has_pch_encoders Daniel Vetter
2013-04-02 21:08 ` Jesse Barnes
2013-03-28 9:42 ` [PATCH 7/8] drm/i915: create pipe_config->dpll for clock state Daniel Vetter
2013-04-02 21:14 ` Jesse Barnes [this message]
2013-04-03 9:44 ` Daniel Vetter
2013-03-28 9:42 ` [PATCH 8/8] drm/i915: move dp clock computations to encoder->compute_config Daniel Vetter
2013-04-02 21:20 ` Jesse Barnes
2013-04-02 21:26 ` Daniel Vetter
2013-04-05 18:51 ` Daniel Vetter
-- strict thread matches above, loose matches on Subject: below --
2013-02-22 0:04 [PATCH 00/10] bpc handling fixes Daniel Vetter
2013-02-22 0:05 ` [PATCH 7/8] drm/i915: create pipe_config->dpll for clock state Daniel Vetter
2013-02-22 0:00 [PATCH 0/8] fdi/dp m_n reorg and a few other clock changes Daniel Vetter
2013-02-22 0:00 ` [PATCH 7/8] drm/i915: create pipe_config->dpll for clock state Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130402141423.1035829f@jbarnes-desktop \
--to=jbarnes@virtuousgeek.org \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox