| Message ID | 1481792500-30863-4-git-send-email-madhav.chauhan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> wrote: > From: Deepak M <m.deepak@intel.com> > > v2: Addressed Jani's Review comments(renamed bit field macros) > > Signed-off-by: Deepak M <m.deepak@intel.com> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi.c | 134 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 134 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index b78c686..c0697e9 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder, > return true; > } > > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) > +{ > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > + enum port port; > + u32 tmp; > + > + /* Put the IO into reset */ > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); > + > + /* Wait for MIPI PHY status bit to unset */ > + for_each_dsi_port(port, intel_dsi->ports) { > + if (intel_wait_for_register(dev_priv, > + MIPI_CTRL(port), > + GLK_PHY_STATUS_PORT_READY, 0, 20)) > + DRM_ERROR("PHY is not turning OFF\n"); > + } > + > + /* Clear MIPI mode */ > + for_each_dsi_port(port, intel_dsi->ports) { > + tmp = I915_READ(MIPI_CTRL(port)); > + tmp &= ~GLK_MIPIIO_ENABLE; > + I915_WRITE(MIPI_CTRL(port), tmp); > + } > +} > + > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) > +{ > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > + enum port port; > + u32 tmp, val; > + > + /* Put the IO into reset */ > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); > + > + /* Program LP Wake */ > + for_each_dsi_port(port, intel_dsi->ports) { > + tmp = I915_READ(MIPI_CTRL(port)); > + tmp &= ~GLK_LP_WAKE; > + I915_WRITE(MIPI_CTRL(port), tmp); > + } > + > + /* Set the MIPI mode */ > + for_each_dsi_port(port, intel_dsi->ports) { > + tmp = I915_READ(MIPI_CTRL(port)); > + I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE); > + } > + > + /* Wait for Pwr ACK */ > + for_each_dsi_port(port, intel_dsi->ports) { > + if (intel_wait_for_register(dev_priv, > + MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED, > + GLK_MIPIIO_PORT_POWERED, 20)) > + DRM_ERROR("Power ACK not received\n"); > + } > + > + /* Wait for MIPI PHY status bit to set */ > + for_each_dsi_port(port, intel_dsi->ports) { > + if (intel_wait_for_register(dev_priv, > + MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED, > + GLK_MIPIIO_PORT_POWERED, 20)) > + DRM_ERROR("PHY is not ON\n"); > + } > + > + /* Get IO out of reset */ > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > + I915_WRITE(MIPI_CTRL(PORT_A), tmp | GLK_MIPIIO_RESET_RELEASED); > + > + /* Get IO out of Low power state*/ > + for_each_dsi_port(port, intel_dsi->ports) { > + if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY)) { > + val = I915_READ(MIPI_DEVICE_READY(port)); > + val &= ~ULPS_STATE_MASK; > + val |= DEVICE_READY; > + I915_WRITE(MIPI_DEVICE_READY(port), val); > + usleep_range(10, 15); > + } > + > + /* 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); > + > + /* Wait for ULPS Not active */ > + if (intel_wait_for_register(dev_priv, > + MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, > + GLK_ULPS_NOT_ACTIVE, 20)) > + > + /* 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); > + > + /* Enter Normal Mode */ > + val = I915_READ(MIPI_DEVICE_READY(port)); > + val &= ~ULPS_STATE_MASK; > + val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY); > + I915_WRITE(MIPI_DEVICE_READY(port), val); > + > + tmp = I915_READ(MIPI_CTRL(port)); > + tmp &= ~GLK_LP_WAKE; > + I915_WRITE(MIPI_CTRL(port), tmp); > + } > + > + /* Wait for Stop state */ > + for_each_dsi_port(port, intel_dsi->ports) { > + if (intel_wait_for_register(dev_priv, > + MIPI_CTRL(port), GLK_DATA_LANE_STOP_STATE, > + GLK_DATA_LANE_STOP_STATE, 20)) > + DRM_ERROR("Date lane not in STOP state\n"); > + } > + > + /* Wait for AFE LATCH */ > + for_each_dsi_port(port, intel_dsi->ports) { > + if (intel_wait_for_register(dev_priv, > + BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT, > + AFE_LATCHOUT, 20)) > + DRM_ERROR("D-PHY not entering LP-11 state\n"); > + } > +} > + I'm wondering if these should just be GLK versions of intel_dsi_device_ready and intel_dsi_clear_device_ready. It seems totally wrong that you're doing device ready stuff twice on GLK... > static void bxt_dsi_device_ready(struct intel_encoder *encoder) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > @@ -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder, > > intel_dsi_prepare(encoder, pipe_config); > > + if (IS_GEMINILAKE(dev_priv)) > + intel_dsi_enable_mipiio(encoder); > + > /* Panel Enable over CRC PMIC */ > if (intel_dsi->gpio_panel) > gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); > @@ -699,6 +830,9 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder) > usleep_range(2000, 2500); > } > > + if (IS_GEMINILAKE(dev_priv)) > + intel_dsi_disable_mipiio(encoder); > + When you're doing enable/disable of something, they should be called from the corresponding functions in the enable/disable paths. But this is just a general remark, if we conclude that these should be alternative device ready/unready calls instead. BR, Jani. > intel_disable_dsi_pll(encoder); > }
> -----Original Message----- > From: Nikula, Jani > Sent: Friday, December 23, 2016 7:40 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; Chauhan, > Madhav <madhav.chauhan@intel.com> > Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable > sequence > > On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> > wrote: > > From: Deepak M <m.deepak@intel.com> > > > > v2: Addressed Jani's Review comments(renamed bit field macros) > > > > Signed-off-by: Deepak M <m.deepak@intel.com> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dsi.c | 134 > > +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 134 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > > b/drivers/gpu/drm/i915/intel_dsi.c > > index b78c686..c0697e9 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct > intel_encoder *encoder, > > return true; > > } > > > > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) { > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > > + enum port port; > > + u32 tmp; > > + > > + /* Put the IO into reset */ > > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; > > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); > > + > > + /* Wait for MIPI PHY status bit to unset */ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + if (intel_wait_for_register(dev_priv, > > + MIPI_CTRL(port), > > + GLK_PHY_STATUS_PORT_READY, 0, 20)) > > + DRM_ERROR("PHY is not turning OFF\n"); > > + } > > + > > + /* Clear MIPI mode */ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + tmp = I915_READ(MIPI_CTRL(port)); > > + tmp &= ~GLK_MIPIIO_ENABLE; > > + I915_WRITE(MIPI_CTRL(port), tmp); > > + } > > +} > > + > > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) { > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > > + enum port port; > > + u32 tmp, val; > > + > > + /* Put the IO into reset */ > > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; > > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); > > + > > + /* Program LP Wake */ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + tmp = I915_READ(MIPI_CTRL(port)); > > + tmp &= ~GLK_LP_WAKE; > > + I915_WRITE(MIPI_CTRL(port), tmp); > > + } > > + > > + /* Set the MIPI mode */ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + tmp = I915_READ(MIPI_CTRL(port)); > > + I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE); > > + } > > + > > + /* Wait for Pwr ACK */ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + if (intel_wait_for_register(dev_priv, > > + MIPI_CTRL(port), > GLK_MIPIIO_PORT_POWERED, > > + GLK_MIPIIO_PORT_POWERED, 20)) > > + DRM_ERROR("Power ACK not received\n"); > > + } > > + > > + /* Wait for MIPI PHY status bit to set */ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + if (intel_wait_for_register(dev_priv, > > + MIPI_CTRL(port), > GLK_MIPIIO_PORT_POWERED, > > + GLK_MIPIIO_PORT_POWERED, 20)) > > + DRM_ERROR("PHY is not ON\n"); > > + } > > + > > + /* Get IO out of reset */ > > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > > + I915_WRITE(MIPI_CTRL(PORT_A), tmp | > GLK_MIPIIO_RESET_RELEASED); > > + > > + /* Get IO out of Low power state*/ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + if (!(I915_READ(MIPI_DEVICE_READY(port)) & > DEVICE_READY)) { > > + val = I915_READ(MIPI_DEVICE_READY(port)); > > + val &= ~ULPS_STATE_MASK; > > + val |= DEVICE_READY; > > + I915_WRITE(MIPI_DEVICE_READY(port), val); > > + usleep_range(10, 15); > > + } > > + > > + /* 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); > > + > > + /* Wait for ULPS Not active */ > > + if (intel_wait_for_register(dev_priv, > > + MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, > > + GLK_ULPS_NOT_ACTIVE, 20)) > > + > > + /* 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); > > + > > + /* Enter Normal Mode */ > > + val = I915_READ(MIPI_DEVICE_READY(port)); > > + val &= ~ULPS_STATE_MASK; > > + val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY); > > + I915_WRITE(MIPI_DEVICE_READY(port), val); > > + > > + tmp = I915_READ(MIPI_CTRL(port)); > > + tmp &= ~GLK_LP_WAKE; > > + I915_WRITE(MIPI_CTRL(port), tmp); > > + } > > + > > + /* Wait for Stop state */ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + if (intel_wait_for_register(dev_priv, > > + MIPI_CTRL(port), > GLK_DATA_LANE_STOP_STATE, > > + GLK_DATA_LANE_STOP_STATE, 20)) > > + DRM_ERROR("Date lane not in STOP state\n"); > > + } > > + > > + /* Wait for AFE LATCH */ > > + for_each_dsi_port(port, intel_dsi->ports) { > > + if (intel_wait_for_register(dev_priv, > > + BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT, > > + AFE_LATCHOUT, 20)) > > + DRM_ERROR("D-PHY not entering LP-11 state\n"); > > + } > > +} > > + > > I'm wondering if these should just be GLK versions of intel_dsi_device_ready > and intel_dsi_clear_device_ready. It seems totally wrong that you're doing > device ready stuff twice on GLK... Agree. Don't need to call intel_dsi_device_ready for GLK, as device ready is already done inside enable_io. Will do following : If(!IS_GEMINILAKE(dev_priv) intel_dsi_device_ready(encoder); > > > > static void bxt_dsi_device_ready(struct intel_encoder *encoder) { > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > @@ > > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct intel_encoder > > *encoder, > > > > intel_dsi_prepare(encoder, pipe_config); > > > > + if (IS_GEMINILAKE(dev_priv)) > > + intel_dsi_enable_mipiio(encoder); > > + > > /* Panel Enable over CRC PMIC */ > > if (intel_dsi->gpio_panel) > > gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ - > 699,6 > > +830,9 @@ static void intel_dsi_clear_device_ready(struct intel_encoder > *encoder) > > usleep_range(2000, 2500); > > } > > > > + if (IS_GEMINILAKE(dev_priv)) > > + intel_dsi_disable_mipiio(encoder); > > + > > When you're doing enable/disable of something, they should be called from > the corresponding functions in the enable/disable paths. But this is just a > general remark, if we conclude that these should be alternative device > ready/unready calls instead. Yes agree, intel_dsi_disable_mipiio should be called from intel_dsi_post_disable after intel_dsi_clear_device_ready and before intel_disable_dsi_pll Will do these changes in next series. > > BR, > Jani. > > > > intel_disable_dsi_pll(encoder); > > } > > -- > Jani Nikula, Intel Open Source Technology Center
On Mon, 26 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote: >> -----Original Message----- >> From: Nikula, Jani >> Sent: Friday, December 23, 2016 7:40 PM >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- >> gfx@lists.freedesktop.org >> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra >> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; Chauhan, >> Madhav <madhav.chauhan@intel.com> >> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable >> sequence >> >> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> >> wrote: >> > From: Deepak M <m.deepak@intel.com> >> > >> > v2: Addressed Jani's Review comments(renamed bit field macros) >> > >> > Signed-off-by: Deepak M <m.deepak@intel.com> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> >> > --- >> > drivers/gpu/drm/i915/intel_dsi.c | 134 >> > +++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 134 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c >> > b/drivers/gpu/drm/i915/intel_dsi.c >> > index b78c686..c0697e9 100644 >> > --- a/drivers/gpu/drm/i915/intel_dsi.c >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c >> > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct >> intel_encoder *encoder, >> > return true; >> > } >> > >> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) { >> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >> > + enum port port; >> > + u32 tmp; >> > + >> > + /* Put the IO into reset */ >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); >> > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); >> > + >> > + /* Wait for MIPI PHY status bit to unset */ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + if (intel_wait_for_register(dev_priv, >> > + MIPI_CTRL(port), >> > + GLK_PHY_STATUS_PORT_READY, 0, 20)) >> > + DRM_ERROR("PHY is not turning OFF\n"); >> > + } >> > + >> > + /* Clear MIPI mode */ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + tmp = I915_READ(MIPI_CTRL(port)); >> > + tmp &= ~GLK_MIPIIO_ENABLE; >> > + I915_WRITE(MIPI_CTRL(port), tmp); >> > + } >> > +} >> > + >> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) { >> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >> > + enum port port; >> > + u32 tmp, val; >> > + >> > + /* Put the IO into reset */ >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); >> > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); >> > + >> > + /* Program LP Wake */ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + tmp = I915_READ(MIPI_CTRL(port)); >> > + tmp &= ~GLK_LP_WAKE; >> > + I915_WRITE(MIPI_CTRL(port), tmp); >> > + } >> > + >> > + /* Set the MIPI mode */ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + tmp = I915_READ(MIPI_CTRL(port)); >> > + I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE); >> > + } >> > + >> > + /* Wait for Pwr ACK */ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + if (intel_wait_for_register(dev_priv, >> > + MIPI_CTRL(port), >> GLK_MIPIIO_PORT_POWERED, >> > + GLK_MIPIIO_PORT_POWERED, 20)) >> > + DRM_ERROR("Power ACK not received\n"); >> > + } >> > + >> > + /* Wait for MIPI PHY status bit to set */ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + if (intel_wait_for_register(dev_priv, >> > + MIPI_CTRL(port), >> GLK_MIPIIO_PORT_POWERED, >> > + GLK_MIPIIO_PORT_POWERED, 20)) >> > + DRM_ERROR("PHY is not ON\n"); >> > + } >> > + >> > + /* Get IO out of reset */ >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp | >> GLK_MIPIIO_RESET_RELEASED); >> > + >> > + /* Get IO out of Low power state*/ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + if (!(I915_READ(MIPI_DEVICE_READY(port)) & >> DEVICE_READY)) { >> > + val = I915_READ(MIPI_DEVICE_READY(port)); >> > + val &= ~ULPS_STATE_MASK; >> > + val |= DEVICE_READY; >> > + I915_WRITE(MIPI_DEVICE_READY(port), val); >> > + usleep_range(10, 15); >> > + } >> > + >> > + /* 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); >> > + >> > + /* Wait for ULPS Not active */ >> > + if (intel_wait_for_register(dev_priv, >> > + MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, >> > + GLK_ULPS_NOT_ACTIVE, 20)) >> > + >> > + /* 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); >> > + >> > + /* Enter Normal Mode */ >> > + val = I915_READ(MIPI_DEVICE_READY(port)); >> > + val &= ~ULPS_STATE_MASK; >> > + val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY); >> > + I915_WRITE(MIPI_DEVICE_READY(port), val); >> > + >> > + tmp = I915_READ(MIPI_CTRL(port)); >> > + tmp &= ~GLK_LP_WAKE; >> > + I915_WRITE(MIPI_CTRL(port), tmp); >> > + } >> > + >> > + /* Wait for Stop state */ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + if (intel_wait_for_register(dev_priv, >> > + MIPI_CTRL(port), >> GLK_DATA_LANE_STOP_STATE, >> > + GLK_DATA_LANE_STOP_STATE, 20)) >> > + DRM_ERROR("Date lane not in STOP state\n"); >> > + } >> > + >> > + /* Wait for AFE LATCH */ >> > + for_each_dsi_port(port, intel_dsi->ports) { >> > + if (intel_wait_for_register(dev_priv, >> > + BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT, >> > + AFE_LATCHOUT, 20)) >> > + DRM_ERROR("D-PHY not entering LP-11 state\n"); >> > + } >> > +} >> > + >> >> I'm wondering if these should just be GLK versions of intel_dsi_device_ready >> and intel_dsi_clear_device_ready. It seems totally wrong that you're doing >> device ready stuff twice on GLK... > > Agree. Don't need to call intel_dsi_device_ready for GLK, as device ready is already done inside enable_io. > Will do following : > If(!IS_GEMINILAKE(dev_priv) > intel_dsi_device_ready(encoder); > >> >> >> > static void bxt_dsi_device_ready(struct intel_encoder *encoder) { >> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> @@ >> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct intel_encoder >> > *encoder, >> > >> > intel_dsi_prepare(encoder, pipe_config); >> > >> > + if (IS_GEMINILAKE(dev_priv)) >> > + intel_dsi_enable_mipiio(encoder); >> > + >> > /* Panel Enable over CRC PMIC */ >> > if (intel_dsi->gpio_panel) >> > gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ - >> 699,6 >> > +830,9 @@ static void intel_dsi_clear_device_ready(struct intel_encoder >> *encoder) >> > usleep_range(2000, 2500); >> > } >> > >> > + if (IS_GEMINILAKE(dev_priv)) >> > + intel_dsi_disable_mipiio(encoder); >> > + >> >> When you're doing enable/disable of something, they should be called from >> the corresponding functions in the enable/disable paths. But this is just a >> general remark, if we conclude that these should be alternative device >> ready/unready calls instead. > > Yes agree, intel_dsi_disable_mipiio should be called from intel_dsi_post_disable after intel_dsi_clear_device_ready and before intel_disable_dsi_pll > Will do these changes in next series. I meant, how about renaming intel_dsi_enable_mipiio to glk_dsi_device_ready, and making intel_dsi_device_ready call glk_dsi_device_ready for GLK. Then rename intel_dsi_clear_device_ready to vlv_dsi_clear_device_ready and intel_dsi_disable_mipiio to glk_dsi_clear_device_ready, and add a new intel_dsi_clear_device_ready wrapper to call vlv_dsi_clear_device_ready for VLV/CHV/BXT, and glk_dsi_clear_device_ready for GLK. How does that sound? BR, Jani. >> >> BR, >> Jani. >> >> >> > intel_disable_dsi_pll(encoder); >> > } >> >> -- >> Jani Nikula, Intel Open Source Technology Center
> -----Original Message----- > From: Nikula, Jani > Sent: Tuesday, December 27, 2016 6:04 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com> > Subject: RE: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable > sequence > > On Mon, 26 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> > wrote: > >> -----Original Message----- > >> From: Nikula, Jani > >> Sent: Friday, December 23, 2016 7:40 PM > >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > >> gfx@lists.freedesktop.org > >> Cc: Conselvan De Oliveira, Ander > >> <ander.conselvan.de.oliveira@intel.com>; > >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > >> <chandra.konduru@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; > >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; > Chauhan, > >> Madhav <madhav.chauhan@intel.com> > >> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO > >> Enable/disable sequence > >> > >> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> > >> wrote: > >> > From: Deepak M <m.deepak@intel.com> > >> > > >> > v2: Addressed Jani's Review comments(renamed bit field macros) > >> > > >> > Signed-off-by: Deepak M <m.deepak@intel.com> > >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > >> > --- > >> > drivers/gpu/drm/i915/intel_dsi.c | 134 > >> > +++++++++++++++++++++++++++++++++++++++ > >> > 1 file changed, 134 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > >> > b/drivers/gpu/drm/i915/intel_dsi.c > >> > index b78c686..c0697e9 100644 > >> > --- a/drivers/gpu/drm/i915/intel_dsi.c > >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c > >> > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct > >> intel_encoder *encoder, > >> > return true; > >> > } > >> > > >> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) { > >> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > >> > + enum port port; > >> > + u32 tmp; > >> > + > >> > + /* Put the IO into reset */ > >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > >> > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; > >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); > >> > + > >> > + /* Wait for MIPI PHY status bit to unset */ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + if (intel_wait_for_register(dev_priv, > >> > + MIPI_CTRL(port), > >> > + GLK_PHY_STATUS_PORT_READY, 0, 20)) > >> > + DRM_ERROR("PHY is not turning OFF\n"); > >> > + } > >> > + > >> > + /* Clear MIPI mode */ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + tmp = I915_READ(MIPI_CTRL(port)); > >> > + tmp &= ~GLK_MIPIIO_ENABLE; > >> > + I915_WRITE(MIPI_CTRL(port), tmp); > >> > + } > >> > +} > >> > + > >> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) { > >> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > >> > + enum port port; > >> > + u32 tmp, val; > >> > + > >> > + /* Put the IO into reset */ > >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > >> > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; > >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); > >> > + > >> > + /* Program LP Wake */ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + tmp = I915_READ(MIPI_CTRL(port)); > >> > + tmp &= ~GLK_LP_WAKE; > >> > + I915_WRITE(MIPI_CTRL(port), tmp); > >> > + } > >> > + > >> > + /* Set the MIPI mode */ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + tmp = I915_READ(MIPI_CTRL(port)); > >> > + I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE); > >> > + } > >> > + > >> > + /* Wait for Pwr ACK */ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + if (intel_wait_for_register(dev_priv, > >> > + MIPI_CTRL(port), > >> GLK_MIPIIO_PORT_POWERED, > >> > + GLK_MIPIIO_PORT_POWERED, 20)) > >> > + DRM_ERROR("Power ACK not received\n"); > >> > + } > >> > + > >> > + /* Wait for MIPI PHY status bit to set */ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + if (intel_wait_for_register(dev_priv, > >> > + MIPI_CTRL(port), > >> GLK_MIPIIO_PORT_POWERED, > >> > + GLK_MIPIIO_PORT_POWERED, 20)) > >> > + DRM_ERROR("PHY is not ON\n"); > >> > + } > >> > + > >> > + /* Get IO out of reset */ > >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp | > >> GLK_MIPIIO_RESET_RELEASED); > >> > + > >> > + /* Get IO out of Low power state*/ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + if (!(I915_READ(MIPI_DEVICE_READY(port)) & > >> DEVICE_READY)) { > >> > + val = I915_READ(MIPI_DEVICE_READY(port)); > >> > + val &= ~ULPS_STATE_MASK; > >> > + val |= DEVICE_READY; > >> > + I915_WRITE(MIPI_DEVICE_READY(port), val); > >> > + usleep_range(10, 15); > >> > + } > >> > + > >> > + /* 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); > >> > + > >> > + /* Wait for ULPS Not active */ > >> > + if (intel_wait_for_register(dev_priv, > >> > + MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, > >> > + GLK_ULPS_NOT_ACTIVE, 20)) > >> > + > >> > + /* 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); > >> > + > >> > + /* Enter Normal Mode */ > >> > + val = I915_READ(MIPI_DEVICE_READY(port)); > >> > + val &= ~ULPS_STATE_MASK; > >> > + val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY); > >> > + I915_WRITE(MIPI_DEVICE_READY(port), val); > >> > + > >> > + tmp = I915_READ(MIPI_CTRL(port)); > >> > + tmp &= ~GLK_LP_WAKE; > >> > + I915_WRITE(MIPI_CTRL(port), tmp); > >> > + } > >> > + > >> > + /* Wait for Stop state */ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + if (intel_wait_for_register(dev_priv, > >> > + MIPI_CTRL(port), > >> GLK_DATA_LANE_STOP_STATE, > >> > + GLK_DATA_LANE_STOP_STATE, 20)) > >> > + DRM_ERROR("Date lane not in STOP state\n"); > >> > + } > >> > + > >> > + /* Wait for AFE LATCH */ > >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> > + if (intel_wait_for_register(dev_priv, > >> > + BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT, > >> > + AFE_LATCHOUT, 20)) > >> > + DRM_ERROR("D-PHY not entering LP-11 state\n"); > >> > + } > >> > +} > >> > + > >> > >> I'm wondering if these should just be GLK versions of > >> intel_dsi_device_ready and intel_dsi_clear_device_ready. It seems > >> totally wrong that you're doing device ready stuff twice on GLK... > > > > Agree. Don't need to call intel_dsi_device_ready for GLK, as device ready is > already done inside enable_io. > > Will do following : > > If(!IS_GEMINILAKE(dev_priv) > > intel_dsi_device_ready(encoder); > > > >> > >> > >> > static void bxt_dsi_device_ready(struct intel_encoder *encoder) { > >> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > >> @@ > >> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct > >> > intel_encoder *encoder, > >> > > >> > intel_dsi_prepare(encoder, pipe_config); > >> > > >> > + if (IS_GEMINILAKE(dev_priv)) > >> > + intel_dsi_enable_mipiio(encoder); > >> > + > >> > /* Panel Enable over CRC PMIC */ > >> > if (intel_dsi->gpio_panel) > >> > gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ - > >> 699,6 > >> > +830,9 @@ static void intel_dsi_clear_device_ready(struct > >> > +intel_encoder > >> *encoder) > >> > usleep_range(2000, 2500); > >> > } > >> > > >> > + if (IS_GEMINILAKE(dev_priv)) > >> > + intel_dsi_disable_mipiio(encoder); > >> > + > >> > >> When you're doing enable/disable of something, they should be called > >> from the corresponding functions in the enable/disable paths. But > >> this is just a general remark, if we conclude that these should be > >> alternative device ready/unready calls instead. > > > > Yes agree, intel_dsi_disable_mipiio should be called from > > intel_dsi_post_disable after intel_dsi_clear_device_ready and before > intel_disable_dsi_pll Will do these changes in next series. > > I meant, how about renaming intel_dsi_enable_mipiio to > glk_dsi_device_ready, and making intel_dsi_device_ready call > glk_dsi_device_ready for GLK. > > Then rename intel_dsi_clear_device_ready to vlv_dsi_clear_device_ready > and intel_dsi_disable_mipiio to glk_dsi_clear_device_ready, and add a new > intel_dsi_clear_device_ready wrapper to call vlv_dsi_clear_device_ready for > VLV/CHV/BXT, and glk_dsi_clear_device_ready for GLK. > > How does that sound? That's good input. It will align the code to the platforms. For glk_dsi_clear_device_ready, we need to 1. Disable MIPI IO, 2. Enter LP Shouldn't we add 2 separate functions for 1,2 and call them inside glk_dsi_clear_device_ready, it will make code readable/modularize as per BSPEC?? > > > BR, > Jani. > > >> > >> BR, > >> Jani. > >> > >> > >> > intel_disable_dsi_pll(encoder); > >> > } > >> > >> -- > >> Jani Nikula, Intel Open Source Technology Center > > -- > Jani Nikula, Intel Open Source Technology Center
On Tue, 27 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote: >> -----Original Message----- >> From: Nikula, Jani >> Sent: Tuesday, December 27, 2016 6:04 PM >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- >> gfx@lists.freedesktop.org >> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra >> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com> >> Subject: RE: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable >> sequence >> >> On Mon, 26 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> >> wrote: >> >> -----Original Message----- >> >> From: Nikula, Jani >> >> Sent: Friday, December 23, 2016 7:40 PM >> >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- >> >> gfx@lists.freedesktop.org >> >> Cc: Conselvan De Oliveira, Ander >> >> <ander.conselvan.de.oliveira@intel.com>; >> >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra >> >> <chandra.konduru@intel.com>; Shankar, Uma >> <uma.shankar@intel.com>; >> >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit >> >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; >> Chauhan, >> >> Madhav <madhav.chauhan@intel.com> >> >> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO >> >> Enable/disable sequence >> >> >> >> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> >> >> wrote: >> >> > From: Deepak M <m.deepak@intel.com> >> >> > >> >> > v2: Addressed Jani's Review comments(renamed bit field macros) >> >> > >> >> > Signed-off-by: Deepak M <m.deepak@intel.com> >> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> >> >> > --- >> >> > drivers/gpu/drm/i915/intel_dsi.c | 134 >> >> > +++++++++++++++++++++++++++++++++++++++ >> >> > 1 file changed, 134 insertions(+) >> >> > >> >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c >> >> > b/drivers/gpu/drm/i915/intel_dsi.c >> >> > index b78c686..c0697e9 100644 >> >> > --- a/drivers/gpu/drm/i915/intel_dsi.c >> >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c >> >> > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct >> >> intel_encoder *encoder, >> >> > return true; >> >> > } >> >> > >> >> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) { >> >> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >> >> > + enum port port; >> >> > + u32 tmp; >> >> > + >> >> > + /* Put the IO into reset */ >> >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); >> >> > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; >> >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); >> >> > + >> >> > + /* Wait for MIPI PHY status bit to unset */ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + if (intel_wait_for_register(dev_priv, >> >> > + MIPI_CTRL(port), >> >> > + GLK_PHY_STATUS_PORT_READY, 0, 20)) >> >> > + DRM_ERROR("PHY is not turning OFF\n"); >> >> > + } >> >> > + >> >> > + /* Clear MIPI mode */ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + tmp = I915_READ(MIPI_CTRL(port)); >> >> > + tmp &= ~GLK_MIPIIO_ENABLE; >> >> > + I915_WRITE(MIPI_CTRL(port), tmp); >> >> > + } >> >> > +} >> >> > + >> >> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) { >> >> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >> >> > + enum port port; >> >> > + u32 tmp, val; >> >> > + >> >> > + /* Put the IO into reset */ >> >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); >> >> > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; >> >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); >> >> > + >> >> > + /* Program LP Wake */ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + tmp = I915_READ(MIPI_CTRL(port)); >> >> > + tmp &= ~GLK_LP_WAKE; >> >> > + I915_WRITE(MIPI_CTRL(port), tmp); >> >> > + } >> >> > + >> >> > + /* Set the MIPI mode */ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + tmp = I915_READ(MIPI_CTRL(port)); >> >> > + I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE); >> >> > + } >> >> > + >> >> > + /* Wait for Pwr ACK */ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + if (intel_wait_for_register(dev_priv, >> >> > + MIPI_CTRL(port), >> >> GLK_MIPIIO_PORT_POWERED, >> >> > + GLK_MIPIIO_PORT_POWERED, 20)) >> >> > + DRM_ERROR("Power ACK not received\n"); >> >> > + } >> >> > + >> >> > + /* Wait for MIPI PHY status bit to set */ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + if (intel_wait_for_register(dev_priv, >> >> > + MIPI_CTRL(port), >> >> GLK_MIPIIO_PORT_POWERED, >> >> > + GLK_MIPIIO_PORT_POWERED, 20)) >> >> > + DRM_ERROR("PHY is not ON\n"); >> >> > + } >> >> > + >> >> > + /* Get IO out of reset */ >> >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); >> >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp | >> >> GLK_MIPIIO_RESET_RELEASED); >> >> > + >> >> > + /* Get IO out of Low power state*/ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + if (!(I915_READ(MIPI_DEVICE_READY(port)) & >> >> DEVICE_READY)) { >> >> > + val = I915_READ(MIPI_DEVICE_READY(port)); >> >> > + val &= ~ULPS_STATE_MASK; >> >> > + val |= DEVICE_READY; >> >> > + I915_WRITE(MIPI_DEVICE_READY(port), val); >> >> > + usleep_range(10, 15); >> >> > + } >> >> > + >> >> > + /* 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); >> >> > + >> >> > + /* Wait for ULPS Not active */ >> >> > + if (intel_wait_for_register(dev_priv, >> >> > + MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, >> >> > + GLK_ULPS_NOT_ACTIVE, 20)) >> >> > + >> >> > + /* 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); >> >> > + >> >> > + /* Enter Normal Mode */ >> >> > + val = I915_READ(MIPI_DEVICE_READY(port)); >> >> > + val &= ~ULPS_STATE_MASK; >> >> > + val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY); >> >> > + I915_WRITE(MIPI_DEVICE_READY(port), val); >> >> > + >> >> > + tmp = I915_READ(MIPI_CTRL(port)); >> >> > + tmp &= ~GLK_LP_WAKE; >> >> > + I915_WRITE(MIPI_CTRL(port), tmp); >> >> > + } >> >> > + >> >> > + /* Wait for Stop state */ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + if (intel_wait_for_register(dev_priv, >> >> > + MIPI_CTRL(port), >> >> GLK_DATA_LANE_STOP_STATE, >> >> > + GLK_DATA_LANE_STOP_STATE, 20)) >> >> > + DRM_ERROR("Date lane not in STOP state\n"); >> >> > + } >> >> > + >> >> > + /* Wait for AFE LATCH */ >> >> > + for_each_dsi_port(port, intel_dsi->ports) { >> >> > + if (intel_wait_for_register(dev_priv, >> >> > + BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT, >> >> > + AFE_LATCHOUT, 20)) >> >> > + DRM_ERROR("D-PHY not entering LP-11 state\n"); >> >> > + } >> >> > +} >> >> > + >> >> >> >> I'm wondering if these should just be GLK versions of >> >> intel_dsi_device_ready and intel_dsi_clear_device_ready. It seems >> >> totally wrong that you're doing device ready stuff twice on GLK... >> > >> > Agree. Don't need to call intel_dsi_device_ready for GLK, as device ready is >> already done inside enable_io. >> > Will do following : >> > If(!IS_GEMINILAKE(dev_priv) >> > intel_dsi_device_ready(encoder); >> > >> >> >> >> >> >> > static void bxt_dsi_device_ready(struct intel_encoder *encoder) { >> >> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> >> @@ >> >> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct >> >> > intel_encoder *encoder, >> >> > >> >> > intel_dsi_prepare(encoder, pipe_config); >> >> > >> >> > + if (IS_GEMINILAKE(dev_priv)) >> >> > + intel_dsi_enable_mipiio(encoder); >> >> > + >> >> > /* Panel Enable over CRC PMIC */ >> >> > if (intel_dsi->gpio_panel) >> >> > gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ - >> >> 699,6 >> >> > +830,9 @@ static void intel_dsi_clear_device_ready(struct >> >> > +intel_encoder >> >> *encoder) >> >> > usleep_range(2000, 2500); >> >> > } >> >> > >> >> > + if (IS_GEMINILAKE(dev_priv)) >> >> > + intel_dsi_disable_mipiio(encoder); >> >> > + >> >> >> >> When you're doing enable/disable of something, they should be called >> >> from the corresponding functions in the enable/disable paths. But >> >> this is just a general remark, if we conclude that these should be >> >> alternative device ready/unready calls instead. >> > >> > Yes agree, intel_dsi_disable_mipiio should be called from >> > intel_dsi_post_disable after intel_dsi_clear_device_ready and before >> intel_disable_dsi_pll Will do these changes in next series. >> >> I meant, how about renaming intel_dsi_enable_mipiio to >> glk_dsi_device_ready, and making intel_dsi_device_ready call >> glk_dsi_device_ready for GLK. >> >> Then rename intel_dsi_clear_device_ready to vlv_dsi_clear_device_ready >> and intel_dsi_disable_mipiio to glk_dsi_clear_device_ready, and add a new >> intel_dsi_clear_device_ready wrapper to call vlv_dsi_clear_device_ready for >> VLV/CHV/BXT, and glk_dsi_clear_device_ready for GLK. >> >> How does that sound? > > That's good input. It will align the code to the platforms. > For glk_dsi_clear_device_ready, we need to 1. Disable MIPI IO, 2. Enter LP > Shouldn't we add 2 separate functions for 1,2 and call them inside glk_dsi_clear_device_ready, it will make code readable/modularize as per BSPEC?? Sure, abstractions like that are fine. BR, Jani. > >> >> >> BR, >> Jani. >> >> >> >> >> BR, >> >> Jani. >> >> >> >> >> >> > intel_disable_dsi_pll(encoder); >> >> > } >> >> >> >> -- >> >> Jani Nikula, Intel Open Source Technology Center >> >> -- >> Jani Nikula, Intel Open Source Technology Center
> -----Original Message----- > From: Nikula, Jani > Sent: Tuesday, December 27, 2016 8:18 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com> > Subject: RE: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable > sequence > > On Tue, 27 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> > wrote: > >> -----Original Message----- > >> From: Nikula, Jani > >> Sent: Tuesday, December 27, 2016 6:04 PM > >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > >> gfx@lists.freedesktop.org > >> Cc: Conselvan De Oliveira, Ander > >> <ander.conselvan.de.oliveira@intel.com>; > >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > >> <chandra.konduru@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; > >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com> > >> Subject: RE: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO > >> Enable/disable sequence > >> > >> On Mon, 26 Dec 2016, "Chauhan, Madhav" > <madhav.chauhan@intel.com> > >> wrote: > >> >> -----Original Message----- > >> >> From: Nikula, Jani > >> >> Sent: Friday, December 23, 2016 7:40 PM > >> >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > >> >> gfx@lists.freedesktop.org > >> >> Cc: Conselvan De Oliveira, Ander > >> >> <ander.conselvan.de.oliveira@intel.com>; > >> >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > >> >> <chandra.konduru@intel.com>; Shankar, Uma > >> <uma.shankar@intel.com>; > >> >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > >> >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; > >> Chauhan, > >> >> Madhav <madhav.chauhan@intel.com> > >> >> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO > >> >> Enable/disable sequence > >> >> > >> >> On Thu, 15 Dec 2016, Madhav Chauhan > <madhav.chauhan@intel.com> > >> >> wrote: > >> >> > From: Deepak M <m.deepak@intel.com> > >> >> > > >> >> > v2: Addressed Jani's Review comments(renamed bit field macros) > >> >> > > >> >> > Signed-off-by: Deepak M <m.deepak@intel.com> > >> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > >> >> > --- > >> >> > drivers/gpu/drm/i915/intel_dsi.c | 134 > >> >> > +++++++++++++++++++++++++++++++++++++++ > >> >> > 1 file changed, 134 insertions(+) > >> >> > > >> >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > >> >> > b/drivers/gpu/drm/i915/intel_dsi.c > >> >> > index b78c686..c0697e9 100644 > >> >> > --- a/drivers/gpu/drm/i915/intel_dsi.c > >> >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c > >> >> > @@ -357,6 +357,134 @@ static bool > >> >> > intel_dsi_compute_config(struct > >> >> intel_encoder *encoder, > >> >> > return true; > >> >> > } > >> >> > > >> >> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) { > >> >> > + struct drm_i915_private *dev_priv = to_i915(encoder- > >base.dev); > >> >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder- > >base); > >> >> > + enum port port; > >> >> > + u32 tmp; > >> >> > + > >> >> > + /* Put the IO into reset */ > >> >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > >> >> > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; > >> >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); > >> >> > + > >> >> > + /* Wait for MIPI PHY status bit to unset */ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + if (intel_wait_for_register(dev_priv, > >> >> > + MIPI_CTRL(port), > >> >> > + GLK_PHY_STATUS_PORT_READY, 0, > 20)) > >> >> > + DRM_ERROR("PHY is not turning OFF\n"); > >> >> > + } > >> >> > + > >> >> > + /* Clear MIPI mode */ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + tmp = I915_READ(MIPI_CTRL(port)); > >> >> > + tmp &= ~GLK_MIPIIO_ENABLE; > >> >> > + I915_WRITE(MIPI_CTRL(port), tmp); > >> >> > + } > >> >> > +} > >> >> > + > >> >> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) { > >> >> > + struct drm_i915_private *dev_priv = to_i915(encoder- > >base.dev); > >> >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder- > >base); > >> >> > + enum port port; > >> >> > + u32 tmp, val; > >> >> > + > >> >> > + /* Put the IO into reset */ > >> >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > >> >> > + tmp &= ~GLK_MIPIIO_RESET_RELEASED; > >> >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp); > >> >> > + > >> >> > + /* Program LP Wake */ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + tmp = I915_READ(MIPI_CTRL(port)); > >> >> > + tmp &= ~GLK_LP_WAKE; > >> >> > + I915_WRITE(MIPI_CTRL(port), tmp); > >> >> > + } > >> >> > + > >> >> > + /* Set the MIPI mode */ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + tmp = I915_READ(MIPI_CTRL(port)); > >> >> > + I915_WRITE(MIPI_CTRL(port), tmp | > GLK_MIPIIO_ENABLE); > >> >> > + } > >> >> > + > >> >> > + /* Wait for Pwr ACK */ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + if (intel_wait_for_register(dev_priv, > >> >> > + MIPI_CTRL(port), > >> >> GLK_MIPIIO_PORT_POWERED, > >> >> > + GLK_MIPIIO_PORT_POWERED, 20)) > >> >> > + DRM_ERROR("Power ACK not received\n"); > >> >> > + } > >> >> > + > >> >> > + /* Wait for MIPI PHY status bit to set */ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + if (intel_wait_for_register(dev_priv, > >> >> > + MIPI_CTRL(port), > >> >> GLK_MIPIIO_PORT_POWERED, > >> >> > + GLK_MIPIIO_PORT_POWERED, 20)) > >> >> > + DRM_ERROR("PHY is not ON\n"); > >> >> > + } > >> >> > + > >> >> > + /* Get IO out of reset */ > >> >> > + tmp = I915_READ(MIPI_CTRL(PORT_A)); > >> >> > + I915_WRITE(MIPI_CTRL(PORT_A), tmp | > >> >> GLK_MIPIIO_RESET_RELEASED); > >> >> > + > >> >> > + /* Get IO out of Low power state*/ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + if (!(I915_READ(MIPI_DEVICE_READY(port)) & > >> >> DEVICE_READY)) { > >> >> > + val = I915_READ(MIPI_DEVICE_READY(port)); > >> >> > + val &= ~ULPS_STATE_MASK; > >> >> > + val |= DEVICE_READY; > >> >> > + I915_WRITE(MIPI_DEVICE_READY(port), val); > >> >> > + usleep_range(10, 15); > >> >> > + } > >> >> > + > >> >> > + /* 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); > >> >> > + > >> >> > + /* Wait for ULPS Not active */ > >> >> > + if (intel_wait_for_register(dev_priv, > >> >> > + MIPI_CTRL(port), > GLK_ULPS_NOT_ACTIVE, > >> >> > + GLK_ULPS_NOT_ACTIVE, 20)) > >> >> > + > >> >> > + /* 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); > >> >> > + > >> >> > + /* Enter Normal Mode */ > >> >> > + val = I915_READ(MIPI_DEVICE_READY(port)); > >> >> > + val &= ~ULPS_STATE_MASK; > >> >> > + val |= (ULPS_STATE_NORMAL_OPERATION | > DEVICE_READY); > >> >> > + I915_WRITE(MIPI_DEVICE_READY(port), val); > >> >> > + > >> >> > + tmp = I915_READ(MIPI_CTRL(port)); > >> >> > + tmp &= ~GLK_LP_WAKE; > >> >> > + I915_WRITE(MIPI_CTRL(port), tmp); > >> >> > + } > >> >> > + > >> >> > + /* Wait for Stop state */ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + if (intel_wait_for_register(dev_priv, > >> >> > + MIPI_CTRL(port), > >> >> GLK_DATA_LANE_STOP_STATE, > >> >> > + GLK_DATA_LANE_STOP_STATE, 20)) > >> >> > + DRM_ERROR("Date lane not in STOP > state\n"); > >> >> > + } > >> >> > + > >> >> > + /* Wait for AFE LATCH */ > >> >> > + for_each_dsi_port(port, intel_dsi->ports) { > >> >> > + if (intel_wait_for_register(dev_priv, > >> >> > + BXT_MIPI_PORT_CTRL(port), > AFE_LATCHOUT, > >> >> > + AFE_LATCHOUT, 20)) > >> >> > + DRM_ERROR("D-PHY not entering LP-11 > state\n"); > >> >> > + } > >> >> > +} > >> >> > + > >> >> > >> >> I'm wondering if these should just be GLK versions of > >> >> intel_dsi_device_ready and intel_dsi_clear_device_ready. It seems > >> >> totally wrong that you're doing device ready stuff twice on GLK... > >> > > >> > Agree. Don't need to call intel_dsi_device_ready for GLK, as device > >> > ready is > >> already done inside enable_io. > >> > Will do following : > >> > If(!IS_GEMINILAKE(dev_priv) > >> > intel_dsi_device_ready(encoder); > >> > > >> >> > >> >> > >> >> > static void bxt_dsi_device_ready(struct intel_encoder *encoder) { > >> >> > struct drm_i915_private *dev_priv = > >> >> > to_i915(encoder->base.dev); > >> >> @@ > >> >> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct > >> >> > intel_encoder *encoder, > >> >> > > >> >> > intel_dsi_prepare(encoder, pipe_config); > >> >> > > >> >> > + if (IS_GEMINILAKE(dev_priv)) > >> >> > + intel_dsi_enable_mipiio(encoder); > >> >> > + > >> >> > /* Panel Enable over CRC PMIC */ > >> >> > if (intel_dsi->gpio_panel) > >> >> > gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); > @@ - > >> >> 699,6 > >> >> > +830,9 @@ static void intel_dsi_clear_device_ready(struct > >> >> > +intel_encoder > >> >> *encoder) > >> >> > usleep_range(2000, 2500); > >> >> > } > >> >> > > >> >> > + if (IS_GEMINILAKE(dev_priv)) > >> >> > + intel_dsi_disable_mipiio(encoder); > >> >> > + > >> >> > >> >> When you're doing enable/disable of something, they should be > >> >> called from the corresponding functions in the enable/disable > >> >> paths. But this is just a general remark, if we conclude that > >> >> these should be alternative device ready/unready calls instead. > >> > > >> > Yes agree, intel_dsi_disable_mipiio should be called from > >> > intel_dsi_post_disable after intel_dsi_clear_device_ready and > >> > before > >> intel_disable_dsi_pll Will do these changes in next series. > >> > >> I meant, how about renaming intel_dsi_enable_mipiio to > >> glk_dsi_device_ready, and making intel_dsi_device_ready call > >> glk_dsi_device_ready for GLK. > >> > >> Then rename intel_dsi_clear_device_ready to > >> vlv_dsi_clear_device_ready and intel_dsi_disable_mipiio to > >> glk_dsi_clear_device_ready, and add a new > >> intel_dsi_clear_device_ready wrapper to call vlv_dsi_clear_device_ready > for VLV/CHV/BXT, and glk_dsi_clear_device_ready for GLK. > >> > >> How does that sound? > > > > That's good input. It will align the code to the platforms. > > For glk_dsi_clear_device_ready, we need to 1. Disable MIPI IO, 2. > > Enter LP Shouldn't we add 2 separate functions for 1,2 and call them inside > glk_dsi_clear_device_ready, it will make code readable/modularize as per > BSPEC?? > > Sure, abstractions like that are fine. Thanks. Will send the patch addressing all these review comments in next series. > > BR, > Jani. > > > > >> > >> > >> BR, > >> Jani. > >> > >> >> > >> >> BR, > >> >> Jani. > >> >> > >> >> > >> >> > intel_disable_dsi_pll(encoder); } > >> >> > >> >> -- > >> >> Jani Nikula, Intel Open Source Technology Center > >> > >> -- > >> Jani Nikula, Intel Open Source Technology Center > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index b78c686..c0697e9 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder, return true; } +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) +{ + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); + enum port port; + u32 tmp; + + /* Put the IO into reset */ + tmp = I915_READ(MIPI_CTRL(PORT_A)); + tmp &= ~GLK_MIPIIO_RESET_RELEASED; + I915_WRITE(MIPI_CTRL(PORT_A), tmp); + + /* Wait for MIPI PHY status bit to unset */ + for_each_dsi_port(port, intel_dsi->ports) { + if (intel_wait_for_register(dev_priv, + MIPI_CTRL(port), + GLK_PHY_STATUS_PORT_READY, 0, 20)) + DRM_ERROR("PHY is not turning OFF\n"); + } + + /* Clear MIPI mode */ + for_each_dsi_port(port, intel_dsi->ports) { + tmp = I915_READ(MIPI_CTRL(port)); + tmp &= ~GLK_MIPIIO_ENABLE; + I915_WRITE(MIPI_CTRL(port), tmp); + } +} + +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) +{ + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); + enum port port; + u32 tmp, val; + + /* Put the IO into reset */ + tmp = I915_READ(MIPI_CTRL(PORT_A)); + tmp &= ~GLK_MIPIIO_RESET_RELEASED; + I915_WRITE(MIPI_CTRL(PORT_A), tmp); + + /* Program LP Wake */ + for_each_dsi_port(port, intel_dsi->ports) { + tmp = I915_READ(MIPI_CTRL(port)); + tmp &= ~GLK_LP_WAKE; + I915_WRITE(MIPI_CTRL(port), tmp); + } + + /* Set the MIPI mode */ + for_each_dsi_port(port, intel_dsi->ports) { + tmp = I915_READ(MIPI_CTRL(port)); + I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE); + } + + /* Wait for Pwr ACK */ + for_each_dsi_port(port, intel_dsi->ports) { + if (intel_wait_for_register(dev_priv, + MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED, + GLK_MIPIIO_PORT_POWERED, 20)) + DRM_ERROR("Power ACK not received\n"); + } + + /* Wait for MIPI PHY status bit to set */ + for_each_dsi_port(port, intel_dsi->ports) { + if (intel_wait_for_register(dev_priv, + MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED, + GLK_MIPIIO_PORT_POWERED, 20)) + DRM_ERROR("PHY is not ON\n"); + } + + /* Get IO out of reset */ + tmp = I915_READ(MIPI_CTRL(PORT_A)); + I915_WRITE(MIPI_CTRL(PORT_A), tmp | GLK_MIPIIO_RESET_RELEASED); + + /* Get IO out of Low power state*/ + for_each_dsi_port(port, intel_dsi->ports) { + if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY)) { + val = I915_READ(MIPI_DEVICE_READY(port)); + val &= ~ULPS_STATE_MASK; + val |= DEVICE_READY; + I915_WRITE(MIPI_DEVICE_READY(port), val); + usleep_range(10, 15); + } + + /* 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); + + /* Wait for ULPS Not active */ + if (intel_wait_for_register(dev_priv, + MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, + GLK_ULPS_NOT_ACTIVE, 20)) + + /* 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); + + /* Enter Normal Mode */ + val = I915_READ(MIPI_DEVICE_READY(port)); + val &= ~ULPS_STATE_MASK; + val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY); + I915_WRITE(MIPI_DEVICE_READY(port), val); + + tmp = I915_READ(MIPI_CTRL(port)); + tmp &= ~GLK_LP_WAKE; + I915_WRITE(MIPI_CTRL(port), tmp); + } + + /* Wait for Stop state */ + for_each_dsi_port(port, intel_dsi->ports) { + if (intel_wait_for_register(dev_priv, + MIPI_CTRL(port), GLK_DATA_LANE_STOP_STATE, + GLK_DATA_LANE_STOP_STATE, 20)) + DRM_ERROR("Date lane not in STOP state\n"); + } + + /* Wait for AFE LATCH */ + for_each_dsi_port(port, intel_dsi->ports) { + if (intel_wait_for_register(dev_priv, + BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT, + AFE_LATCHOUT, 20)) + DRM_ERROR("D-PHY not entering LP-11 state\n"); + } +} + static void bxt_dsi_device_ready(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); @@ -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder, intel_dsi_prepare(encoder, pipe_config); + if (IS_GEMINILAKE(dev_priv)) + intel_dsi_enable_mipiio(encoder); + /* Panel Enable over CRC PMIC */ if (intel_dsi->gpio_panel) gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ -699,6 +830,9 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder) usleep_range(2000, 2500); } + if (IS_GEMINILAKE(dev_priv)) + intel_dsi_disable_mipiio(encoder); + intel_disable_dsi_pll(encoder); }