On Tue, 01 Sep 2015, Uma Shankar <uma.shankar@xxxxxxxxx> wrote:
> From: Shashank Sharma <shashank.sharma@xxxxxxxxx>
>
> 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
>
> v2: Fixed Jani's review comments. Removed the changes in VLV/CHV
> code. Fixed the macros to get proper port offsets.
>
> v3: Rebased on latest drm-nightly branch. Fixed Jani's review comments.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
When you look at the commits before sending, you should pay more
attention to what the diffs end up looking like. In this case, the
review becomes unnecessarily hard just because you've moved code
(intel_dsi_port_enable) around for no apparent reason, and the diff
seems like intel_dsi_port_enable is rewritten into
bxt_dsi_device_ready. It should be a hint to *not* combine code movement
into the same patch.
> ---
> drivers/gpu/drm/i915/i915_reg.h | 7 ++
> drivers/gpu/drm/i915/intel_dsi.c | 165 ++++++++++++++++++++++++++------------
> 2 files changed, 119 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 997a999..57c5dbf 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7403,6 +7403,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) _MIPI_PORT(tc, _BXT_MIPIA_PORT_CTRL, \
> + _BXT_MIPIC_PORT_CTRL)
> +
> #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 6d0c992..5a42f87 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -286,58 +286,46 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
> return true;
> }
>
> -static void intel_dsi_port_enable(struct intel_encoder *encoder)
> +static void bxt_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_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + 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 temp;
> + u32 val;
>
> - if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> - temp = I915_READ(VLV_CHICKEN_3);
> - temp &= ~PIXEL_OVERLAP_CNT_MASK |
> - intel_dsi->pixel_overlap <<
> - PIXEL_OVERLAP_CNT_SHIFT;
> - I915_WRITE(VLV_CHICKEN_3, temp);
> - }
> + DRM_DEBUG_KMS("\n");
>
> + /* Exit Low power state in 4 steps*/
> for_each_dsi_port(port, intel_dsi->ports) {
> - temp = I915_READ(MIPI_PORT_CTRL(port));
> - temp &= ~LANE_CONFIGURATION_MASK;
> - temp &= ~DUAL_LINK_MODE_MASK;
>
> - if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
> - temp |= (intel_dsi->dual_link - 1)
> - << DUAL_LINK_MODE_SHIFT;
> - temp |= intel_crtc->pipe ?
> - LANE_CONFIGURATION_DUAL_LINK_B :
> - 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));
> - }
> -}
> + /* 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);
>
> -static void intel_dsi_port_disable(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);
> - enum port port;
> - u32 temp;
> + /* 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);
>
> - for_each_dsi_port(port, intel_dsi->ports) {
> - /* de-assert ip_tg_enable signal */
> - temp = I915_READ(MIPI_PORT_CTRL(port));
> - I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
> - POSTING_READ(MIPI_PORT_CTRL(port));
> + /* 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 intel_dsi_device_ready(struct intel_encoder *encoder)
> +static void vlv_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);
> @@ -376,6 +364,72 @@ static void intel_dsi_device_ready(struct intel_encoder *encoder)
> }
> }
>
> +static void intel_dsi_device_ready(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> +
> + if (IS_VALLEYVIEW(dev))
> + vlv_dsi_device_ready(encoder);
> + else if (IS_BROXTON(dev))
> + bxt_dsi_device_ready(encoder);
> +}
> +
> +static void intel_dsi_port_enable(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + 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);
> + temp &= ~PIXEL_OVERLAP_CNT_MASK |
> + intel_dsi->pixel_overlap <<
> + PIXEL_OVERLAP_CNT_SHIFT;
> + I915_WRITE(VLV_CHICKEN_3, temp);
> + }
> +
> + for_each_dsi_port(port, intel_dsi->ports) {
> + 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;
> +
> + if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
> + temp |= (intel_dsi->dual_link - 1)
> + << DUAL_LINK_MODE_SHIFT;
> + temp |= intel_crtc->pipe ?
> + LANE_CONFIGURATION_DUAL_LINK_B :
> + LANE_CONFIGURATION_DUAL_LINK_A;
> + }
> + /* assert ip_tg_enable signal */
> + I915_WRITE(port_ctrl, temp | DPI_ENABLE);
> + POSTING_READ(port_ctrl);
> + }
> +}
> +
> +static void intel_dsi_port_disable(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);
> + enum port port;
> + u32 temp;
> +
> + for_each_dsi_port(port, intel_dsi->ports) {
> + /* de-assert ip_tg_enable signal */
> + temp = I915_READ(MIPI_PORT_CTRL(port));
> + I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
> + POSTING_READ(MIPI_PORT_CTRL(port));
> + }
> +}
> +
> static void intel_dsi_enable(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> @@ -415,19 +469,24 @@ 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);
> -
> - /* 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);
> + 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;
> +
> + 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);
> --
> 1.7.9.5
>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
