From: Jani Nikula <jani.nikula@linux.intel.com>
To: Uma Shankar <uma.shankar@intel.com>, intel-gfx@lists.freedesktop.org
Cc: shobhit.kumar@intel.com
Subject: Re: [PATCH 06/12] drm/i915/bxt: DSI enable for BXT
Date: Mon, 25 May 2015 19:39:47 +0300 [thread overview]
Message-ID: <87bnh8ipbg.fsf@intel.com> (raw)
In-Reply-To: <1432310765-2218-7-git-send-email-uma.shankar@intel.com>
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> This patch contains following changes:
> 1. MIPI device ready changes to support dsi_pre_enable. Changes
> are specific to BXT device ready sequence. Added check for
> ULPS mode(No effects on VLV).
> 2. Changes in dsi_enable to pick BXT port control register.
> 3. Changes in dsi_pre_enable to restrict DPIO programming for VLV
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 7 ++
> drivers/gpu/drm/i915/intel_dsi.c | 185 ++++++++++++++++++++++++++++----------
> 2 files changed, 144 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 57c3a48..5eeb2b7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7408,6 +7408,13 @@ enum skl_disp_power_wells {
> #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
> #define _MIPIC_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
> #define MIPI_PORT_CTRL(port) _MIPI_PORT(port, _MIPIA_PORT_CTRL, _MIPIC_PORT_CTRL)
> +
> + /* BXT port control */
> +#define _BXT_MIPIA_PORT_CTRL 0x6B0C0
> +#define _BXT_MIPIC_PORT_CTRL 0x6B8C0
> +#define BXT_MIPI_PORT_CTRL(tc) _TRANSCODER(tc, _BXT_MIPIA_PORT_CTRL, \
> + _BXT_MIPIC_PORT_CTRL)
Shouldn't this use _MIPI_PORT, not _TRANSCODER?
> +
> #define DPI_ENABLE (1 << 31) /* A + C */
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 892b518..3a1bb04 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -286,6 +286,101 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
> return true;
> }
>
> +static void bxt_dsi_device_ready(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + enum port port;
> + u32 val;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* Exit Low power state in 4 steps*/
> + for_each_dsi_port(port, intel_dsi->ports) {
> +
> + /* 1. Enable MIPI PHY transparent latch */
> + val = I915_READ(BXT_MIPI_PORT_CTRL(port));
> + I915_WRITE(BXT_MIPI_PORT_CTRL(port), val | LP_OUTPUT_HOLD);
> + usleep_range(2000, 2500);
> +
> + /* 2. Enter ULPS */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= (ULPS_STATE_ENTER | DEVICE_READY);
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> + usleep_range(2, 3);
> +
> + /* 3. Exit ULPS */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= (ULPS_STATE_EXIT | DEVICE_READY);
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> + usleep_range(1000, 1500);
> +
> + /* Clear ULPS and set device ready */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= DEVICE_READY;
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> + }
> +}
> +
> +static void vlv_dsi_device_ready(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + int pipe = intel_crtc->pipe;
enum pipe.
> + u32 val;
> + int count = 1;
> + u32 reg = 0;
> + bool afe_latchout = true;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + pipe = intel_crtc->pipe;
> + reg = MIPI_PORT_CTRL(pipe);
Take care to not use pipe for port. Passing PIPE_B as port will *not*
use PORT_C.
> +
> + /* Ceck LP state */
> + val = I915_READ(reg);
> + afe_latchout = (val & AFE_LATCHOUT);
> +
> + /* Enter low power state, if not already in */
> + if (afe_latchout) {
> + /* Enter ULPS, wait for 2.5 ms */
> + reg = MIPI_DEVICE_READY(pipe);
> + I915_WRITE(reg, val | ULPS_STATE_ENTER);
> + usleep_range(2000, 2500);
> + }
> +
> + /* Enable transparent latch */
> + I915_WRITE(reg, val | LP_OUTPUT_HOLD);
> +
> + if (intel_dsi->dual_link) {
> + count = 2;
> + pipe = PIPE_A;
> + }
> +
> + do {
> + I915_WRITE(reg, val | ULPS_STATE_EXIT);
> + usleep_range(2000, 2500);
> + /* Clear ULPS state */
> + val = I915_READ(MIPI_DEVICE_READY(pipe)) & ~ULPS_STATE_MASK;
> + I915_WRITE(MIPI_DEVICE_READY(pipe), val);
> + usleep_range(2000, 2500);
> +
> + /* Set device ready */
> + val |= DEVICE_READY;
> + I915_WRITE(MIPI_DEVICE_READY(pipe), val);
> + usleep_range(2000, 2500);
> +
> + /* For Port C for dual link */
> + if (intel_dsi->dual_link)
> + pipe = PIPE_B;
> + } while (--count > 0);
You're making functional changes to the BYT/BSW DSI code. These *must*
be separated to independent patches.
Additionally the loop is hard to follow. If you abstract the block into
a separate function, you can just call it once for single link, and
twice for dual link, with appropriate parameters.
> +}
> +
> static void intel_dsi_port_enable(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> @@ -294,6 +389,7 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> enum port port;
> u32 temp;
> + u32 port_ctrl;
>
> if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> temp = I915_READ(VLV_CHICKEN_3);
> @@ -304,7 +400,10 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
> }
>
> for_each_dsi_port(port, intel_dsi->ports) {
> - temp = I915_READ(MIPI_PORT_CTRL(port));
> + port_ctrl = IS_BROXTON(dev) ? BXT_MIPI_PORT_CTRL(port) :
> + MIPI_PORT_CTRL(port);
> + temp = I915_READ(port_ctrl);
> +
> temp &= ~LANE_CONFIGURATION_MASK;
> temp &= ~DUAL_LINK_MODE_MASK;
>
> @@ -316,8 +415,8 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
> LANE_CONFIGURATION_DUAL_LINK_A;
> }
> /* assert ip_tg_enable signal */
> - I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
> - POSTING_READ(MIPI_PORT_CTRL(port));
> + I915_WRITE(port_ctrl, temp | DPI_ENABLE);
> + POSTING_READ(port_ctrl);
> }
> }
>
> @@ -339,41 +438,15 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
>
> static void intel_dsi_device_ready(struct intel_encoder *encoder)
> {
> - struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> - struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> - enum port port;
> - u32 val;
> -
> - DRM_DEBUG_KMS("\n");
> -
> - mutex_lock(&dev_priv->dpio_lock);
> - /* program rcomp for compliance, reduce from 50 ohms to 45 ohms
> - * needed everytime after power gate */
> - vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
> - mutex_unlock(&dev_priv->dpio_lock);
> -
> - /* bandgap reset is needed after everytime we do power gate */
> - band_gap_reset(dev_priv);
> -
> - for_each_dsi_port(port, intel_dsi->ports) {
> -
> - I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_ENTER);
> - usleep_range(2500, 3000);
> -
> - /* Enable MIPI PHY transparent latch
> - * Common bit for both MIPI Port A & MIPI Port C
> - * No similar bit in MIPI Port C reg
> - */
> - val = I915_READ(MIPI_PORT_CTRL(PORT_A));
> - I915_WRITE(MIPI_PORT_CTRL(PORT_A), val | LP_OUTPUT_HOLD);
> - usleep_range(1000, 1500);
> + struct drm_device *dev = encoder->base.dev;
>
> - I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
> - usleep_range(2500, 3000);
> + if (IS_VALLEYVIEW(dev))
> + vlv_dsi_device_ready(encoder);
> + else if (IS_BROXTON(dev))
> + bxt_dsi_device_ready(encoder);
> + else
> + DRM_ERROR("Invalid DSI device to device ready\n");
Please remove all such else branches.
>
> - I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY);
> - usleep_range(2500, 3000);
> - }
> }
>
> static void intel_dsi_enable(struct intel_encoder *encoder)
> @@ -415,19 +488,22 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>
> DRM_DEBUG_KMS("\n");
>
> - /* Disable DPOunit clock gating, can stall pipe
> - * and we need DPLL REFA always enabled */
> - tmp = I915_READ(DPLL(pipe));
> - tmp |= DPLL_REFA_CLK_ENABLE_VLV;
> - I915_WRITE(DPLL(pipe), tmp);
> + if (IS_VALLEYVIEW(dev)) {
> + /* Disable DPOunit clock gating, can stall pipe
> + * and we need DPLL REFA always enabled
> + */
> + tmp = I915_READ(DPLL(pipe));
> + tmp |= DPLL_REFA_CLK_ENABLE_VLV;
> + I915_WRITE(DPLL(pipe), tmp);
>
> - /* update the hw state for DPLL */
> - intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> - DPLL_REFA_CLK_ENABLE_VLV;
> + /* update the hw state for DPLL */
> + intel_crtc->config->dpll_hw_state.dpll =
> + DPLL_INTEGRATED_CLOCK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
>
> - tmp = I915_READ(DSPCLK_GATE_D);
> - tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> - I915_WRITE(DSPCLK_GATE_D, tmp);
> + tmp = I915_READ(DSPCLK_GATE_D);
> + tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> + I915_WRITE(DSPCLK_GATE_D, tmp);
> + }
>
> /* put device in ready state */
> intel_dsi_device_ready(encoder);
> @@ -959,11 +1035,24 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>
> static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
> {
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> DRM_DEBUG_KMS("\n");
>
> + if (IS_VALLEYVIEW(dev)) {
> + mutex_lock(&dev_priv->dpio_lock);
> + /* program rcomp for compliance, reduce from 50 ohms to 45 ohms
> + * needed everytime after power gate */
> + vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
> + mutex_unlock(&dev_priv->dpio_lock);
> +
> + /* bandgap reset is needed after everytime we do power gate */
> + band_gap_reset(dev_priv);
> + }
> +
You're making functional changes to BYT/BSW DSI code. These *must* be
separated to independent patches.
BR,
Jani.
> intel_dsi_prepare(encoder);
> intel_enable_dsi_pll(encoder);
> -
> }
>
> static enum drm_connector_status
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-05-25 16:41 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
2015-05-22 16:05 ` [PATCH 01/12] drm/i915/bxt: Initialize MIPI for BXT Uma Shankar
2015-05-22 16:05 ` [PATCH 02/12] drm/i915/bxt: Enable BXT DSI PLL Uma Shankar
2015-05-25 16:10 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 03/12] drm/i915/bxt: Disable DSI PLL for BXT Uma Shankar
2015-05-25 16:12 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 04/12] drm/i915/bxt: DSI prepare changes " Uma Shankar
2015-05-25 16:25 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset Uma Shankar
2015-05-25 10:13 ` Jani Nikula
2015-05-25 11:24 ` Jani Nikula
2015-05-25 10:25 ` Jani Nikula
2015-05-26 7:11 ` Daniel Vetter
2015-05-26 7:19 ` Jani Nikula
2015-05-26 8:26 ` Daniel Vetter
2015-05-22 16:05 ` [PATCH 06/12] drm/i915/bxt: DSI enable for BXT Uma Shankar
2015-05-25 16:39 ` Jani Nikula [this message]
2015-05-22 16:06 ` [PATCH 07/12] drm/i915/bxt: Program Tx Rx and Dphy clocks Uma Shankar
2015-05-25 16:52 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 08/12] drm/i915/bxt: DSI disable and post-disable Uma Shankar
2015-05-25 16:44 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 09/12] drm/i915/bxt: get_hw_state for BXT Uma Shankar
2015-05-22 16:06 ` [PATCH 10/12] drm/i915/bxt: get DSI pixelclock Uma Shankar
2015-05-25 16:54 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 11/12] drm/i915/bxt: Modify BXT BLC according to VBT changes Uma Shankar
2015-05-25 10:03 ` Jani Nikula
2015-05-25 16:57 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 12/12] drm/i915/bxt: Remove DSP CLK_GATE programming for BXT Uma Shankar
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=87bnh8ipbg.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=shobhit.kumar@intel.com \
--cc=uma.shankar@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.