From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shobhit Kumar Subject: Re: [PATCH v3 5/7] drm/i915: Reorganize the DSI enable/disable sequence Date: Wed, 11 Dec 2013 17:48:41 +0530 Message-ID: <52A85821.7050402@intel.com> References: <1386657900-12132-1-git-send-email-shobhit.kumar@intel.com> <1386657900-12132-6-git-send-email-shobhit.kumar@intel.com> <87txef6473.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id EDC69FA8E8 for ; Wed, 11 Dec 2013 04:19:21 -0800 (PST) In-Reply-To: <87txef6473.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jani Nikula , intel-gfx@lists.freedesktop.org Cc: vijayakumar.balakrishnan@intel.com, yogesh.mohan.marimuthu@intel.com List-Id: intel-gfx@lists.freedesktop.org On Wednesday 11 December 2013 04:32 PM, Jani Nikula wrote: > On Tue, 10 Dec 2013, Shobhit Kumar wrote: >> Basically ULPS handling during enable/disable has been moved to >> pre_enable and post_disable phases. PLL and panel power disable >> also has been moved to post_disable phase. The ULPS entry/exit >> sequneces as suggested by HW team is as follows - >> >> During enable time - >> set DEVICE_READY --> Clear DEVICE_READY --> set DEVICE_READY >> >> And during disable time to flush all FIFOs - >> set ENTER_SLEEP --> EXIT_SLEEP --> ENTER_SLEEP >> >> Also during disbale sequnece sub-encoder disable is moved to the end >> after port is disabled. >> >> v2: Based on comments from Ville >> - Detailed epxlaination in the commit messgae >> - Moved parameter changes out into another patch >> - Backlight enabling will be a new patch >> >> v3: Updated as per Jani's comments >> - Removed the I915_WRITE_BITS as it is not needed >> - Moved panel_reset and send_otp_cmds hooks to dsi_pre_enable >> - Moved disable_panel_power hook to dsi_post_disable >> - Replace hardcoding with AFE_LATCHOUT >> >> Signed-off-by: Yogesh Mohan Marimuthu >> Signed-off-by: Shobhit Kumar >> --- >> drivers/gpu/drm/i915/intel_dsi.c | 110 +++++++++++++++++++++++++++------------ >> drivers/gpu/drm/i915/intel_dsi.h | 2 + >> 2 files changed, 79 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index 1016e7b..01b9f3a 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -101,46 +101,57 @@ static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder) >> vlv_enable_dsi_pll(encoder); >> } >> >> -static void intel_dsi_pre_enable(struct intel_encoder *encoder) >> +void intel_dsi_device_ready(struct intel_encoder *encoder) > > Should be static. > >> { >> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private; >> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc); >> + int pipe = intel_crtc->pipe; >> + u32 val; >> + >> DRM_DEBUG_KMS("\n"); >> -} >> >> -static void intel_dsi_enable(struct intel_encoder *encoder) >> + val = I915_READ(MIPI_PORT_CTRL(pipe)); >> + I915_WRITE(MIPI_PORT_CTRL(pipe), val | LP_OUTPUT_HOLD); >> + usleep_range(1000, 1500); >> + I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY | ULPS_STATE_EXIT); >> + usleep_range(2000, 2500); >> + I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY); >> + usleep_range(2000, 2500); >> + I915_WRITE(MIPI_DEVICE_READY(pipe), 0x00); >> + usleep_range(2000, 2500); >> + I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY); >> + usleep_range(2000, 2500); >> +} >> +static void intel_dsi_pre_enable(struct intel_encoder *encoder) >> { >> - struct drm_i915_private *dev_priv = encoder->base.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); >> - int pipe = intel_crtc->pipe; >> - u32 temp; >> >> DRM_DEBUG_KMS("\n"); >> >> if (intel_dsi->dev.dev_ops->panel_reset) >> intel_dsi->dev.dev_ops->panel_reset(&intel_dsi->dev); >> >> - temp = I915_READ(MIPI_DEVICE_READY(pipe)); >> - if ((temp & DEVICE_READY) == 0) { >> - temp &= ~ULPS_STATE_MASK; >> - I915_WRITE(MIPI_DEVICE_READY(pipe), temp | DEVICE_READY); >> - } else if (temp & ULPS_STATE_MASK) { >> - temp &= ~ULPS_STATE_MASK; >> - I915_WRITE(MIPI_DEVICE_READY(pipe), temp | ULPS_STATE_EXIT); >> - /* >> - * We need to ensure that there is a minimum of 1 ms time >> - * available before clearing the UPLS exit state. >> - */ >> - msleep(2); >> - I915_WRITE(MIPI_DEVICE_READY(pipe), temp); >> - } >> + /* put device in ready state */ >> + intel_dsi_device_ready(encoder); >> >> if (intel_dsi->dev.dev_ops->send_otp_cmds) >> intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev); >> +} >> + >> +static void intel_dsi_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); >> + int pipe = intel_crtc->pipe; >> + u32 temp; >> + >> + DRM_DEBUG_KMS("\n"); >> >> if (is_cmd_mode(intel_dsi)) >> I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(pipe), 8 * 4); >> - >> - if (is_vid_mode(intel_dsi)) { >> + else { >> msleep(20); /* XXX */ >> dpi_send_cmd(intel_dsi, TURN_ON); >> msleep(100); >> @@ -157,7 +168,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder) >> >> static void intel_dsi_disable(struct intel_encoder *encoder) >> { >> - struct drm_i915_private *dev_priv = encoder->base.dev->dev_private; >> + 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); >> int pipe = intel_crtc->pipe; >> @@ -165,8 +177,6 @@ static void intel_dsi_disable(struct intel_encoder *encoder) >> >> DRM_DEBUG_KMS("\n"); >> >> - intel_dsi->dev.dev_ops->disable(&intel_dsi->dev); >> - >> if (is_vid_mode(intel_dsi)) { >> dpi_send_cmd(intel_dsi, SHUTDOWN); >> msleep(10); >> @@ -179,20 +189,54 @@ static void intel_dsi_disable(struct intel_encoder *encoder) >> msleep(2); >> } >> >> - temp = I915_READ(MIPI_DEVICE_READY(pipe)); >> - if (temp & DEVICE_READY) { >> - temp &= ~DEVICE_READY; >> - temp &= ~ULPS_STATE_MASK; >> - I915_WRITE(MIPI_DEVICE_READY(pipe), temp); >> - } >> + /* if disable packets are sent before sending shutdown packet then in >> + * some next enable sequence send turn on packet error is observed */ >> + if (intel_dsi->dev.dev_ops->disable) >> + intel_dsi->dev.dev_ops->disable(&intel_dsi->dev); >> } >> >> -static void intel_dsi_post_disable(struct intel_encoder *encoder) >> +void intel_dsi_clear_device_ready(struct intel_encoder *encoder) > > Should be static. > > With these fixed, > Reviewed-by: Jani Nikula Yeah sending the updated patch Regards Shobhit