| Message ID | 1382358067-5578-5-git-send-email-shobhit.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 21, 2013 at 05:51:07PM +0530, Shobhit Kumar wrote: > Has been tested on couple of panels now. While it's nice to get patches, I can't say I'm very happy about the shape of this one. The patch contains several changes in one patch. It should be split up into several patches. Based on a cursory examination I would suggest something like this: - weird ULPS ping-pong - add backlight support - moving the ->disable() call - each of the new intel_dsi->foo/bar/etc. parameter could probably be a separate patch As far as the various timeout related parameters are concerned, to me it would make more sense to specify them in usecs or some other real world unit. Or you could provide/leave in some helper functions to calculate the clock based values from some real world values. And finally justficiation for each of these changes is missing from the current patch. We want to know why the code has to change. > > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> > Signed-off-by: Vijaykumar Balakrishnan <vijayakumar.balakrishnan@intel.com> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 11 ++ > drivers/gpu/drm/i915/intel_dsi.c | 334 +++++++++++++++++++++----------------- > 2 files changed, 199 insertions(+), 146 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 1c42bb4..1c28d02 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2375,6 +2375,17 @@ __i915_write(64) > #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) > #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg) > > +#define I915_WRITE_BITS(reg, val, mask) \ > +do { \ > + u32 tmp, data; \ > + tmp = I915_READ((reg)); \ > + tmp &= ~(mask); \ > + data = (val) & (mask); \ > + data = data | tmp; \ > + I915_WRITE((reg), data); \ > +} while(0) > + > + > /* "Broadcast RGB" property */ > #define INTEL_BROADCAST_RGB_AUTO 0 > #define INTEL_BROADCAST_RGB_FULL 1 > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 5a9dbfd..241bb55 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -21,6 +21,8 @@ > * DEALINGS IN THE SOFTWARE. > * > * Author: Jani Nikula <jani.nikula@intel.com> > + * : Shobhit Kumar <shobhit.kumar@intel.com> > + * : Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> > */ > > #include <drm/drmP.h> > @@ -101,63 +103,80 @@ 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) > -{ > - DRM_DEBUG_KMS("\n"); > -} > - > -static void intel_dsi_enable(struct intel_encoder *encoder) > +void intel_dsi_device_ready(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); > - } > + I915_WRITE_BITS(MIPI_PORT_CTRL(pipe), LP_OUTPUT_HOLD, LP_OUTPUT_HOLD); > + usleep_range(1000, 1500); > + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY | > + ULPS_STATE_EXIT, DEVICE_READY | ULPS_STATE_MASK); > + usleep_range(2000, 2500); > + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY, > + DEVICE_READY | ULPS_STATE_MASK); > + usleep_range(2000, 2500); > + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), 0x00, > + DEVICE_READY | ULPS_STATE_MASK); > + usleep_range(2000, 2500); > + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY, > + DEVICE_READY | ULPS_STATE_MASK); > + usleep_range(2000, 2500); > > if (intel_dsi->dev.dev_ops->send_otp_cmds) > intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev); > > +} > +static void intel_dsi_pre_enable(struct intel_encoder *encoder) > +{ > + DRM_DEBUG_KMS("\n"); > + > + /* put device in ready state */ > + intel_dsi_device_ready(encoder); > +} > + > +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); > > /* assert ip_tg_enable signal */ > temp = I915_READ(MIPI_PORT_CTRL(pipe)); > + temp = temp | intel_dsi->port_bits; > I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE); > POSTING_READ(MIPI_PORT_CTRL(pipe)); > } > > if (intel_dsi->dev.dev_ops->enable) > intel_dsi->dev.dev_ops->enable(&intel_dsi->dev); > + > + intel_panel_enable_backlight(dev, pipe); > } > > 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,11 +184,18 @@ static void intel_dsi_disable(struct intel_encoder *encoder) > > DRM_DEBUG_KMS("\n"); > > - intel_dsi->dev.dev_ops->disable(&intel_dsi->dev); > + intel_panel_disable_backlight(dev); > + if (intel_dsi->backlight_off_delay >= 20) > + msleep(intel_dsi->backlight_off_delay); > + else > + usleep_range(intel_dsi->backlight_off_delay * 1000, > + (intel_dsi->backlight_off_delay * 1000) + 500); > > if (is_vid_mode(intel_dsi)) { > - dpi_send_cmd(intel_dsi, SHUTDOWN); > - msleep(10); > + if (intel_dsi->send_shutdown) { > + dpi_send_cmd(intel_dsi, SHUTDOWN); > + msleep(intel_dsi->shutdown_pkt_delay); > + } > > /* de-assert ip_tg_enable signal */ > temp = I915_READ(MIPI_PORT_CTRL(pipe)); > @@ -179,19 +205,52 @@ 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) > { > + 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; > + > DRM_DEBUG_KMS("\n"); > > + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER, > + ULPS_STATE_MASK); > + usleep_range(2000, 2500); > + > + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_EXIT, > + ULPS_STATE_MASK); > + usleep_range(2000, 2500); > + > + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER, > + ULPS_STATE_MASK); > + usleep_range(2000, 2500); > + > + I915_WRITE_BITS(MIPI_PORT_CTRL(pipe), 0, LP_OUTPUT_HOLD); > + usleep_range(1000, 1500); > + > + if (wait_for(((I915_READ(MIPI_PORT_CTRL(pipe)) & 0x20000) > + == 0x00000), 30)) > + DRM_ERROR("DSI LP not going Low\n"); > + > + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), 0x00, DEVICE_READY); > + usleep_range(2000, 2500); > + > vlv_disable_dsi_pll(encoder); > + > + if (intel_dsi->dev.dev_ops->disable_panel_power) > + intel_dsi->dev.dev_ops->disable_panel_power(&intel_dsi->dev); > +} > +static void intel_dsi_post_disable(struct intel_encoder *encoder) > +{ > + DRM_DEBUG_KMS("\n"); > + intel_dsi_clear_device_ready(encoder); > } > > static bool intel_dsi_get_hw_state(struct intel_encoder *encoder, > @@ -251,20 +310,6 @@ static int intel_dsi_mode_valid(struct drm_connector *connector, > return intel_dsi->dev.dev_ops->mode_valid(&intel_dsi->dev, mode); > } > > -/* return txclkesc cycles in terms of divider and duration in us */ > -static u16 txclkesc(u32 divider, unsigned int us) > -{ > - switch (divider) { > - case ESCAPE_CLOCK_DIVIDER_1: > - default: > - return 20 * us; > - case ESCAPE_CLOCK_DIVIDER_2: > - return 10 * us; > - case ESCAPE_CLOCK_DIVIDER_4: > - return 5 * us; > - } > -} > - > /* return pixels in terms of txbyteclkhs */ > static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count) > { > @@ -313,26 +358,16 @@ static void set_dsi_timings(struct drm_encoder *encoder, > I915_WRITE(MIPI_VBP_COUNT(pipe), vbp); > } > > -static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) > +static void dsi_config(struct drm_encoder *encoder) > { > - struct drm_encoder *encoder = &intel_encoder->base; > struct drm_device *dev = encoder->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); > struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); > - struct drm_display_mode *adjusted_mode = > - &intel_crtc->config.adjusted_mode; > + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); > int pipe = intel_crtc->pipe; > - unsigned int bpp = intel_crtc->config.pipe_bpp; > - u32 val, tmp; > - > - DRM_DEBUG_KMS("pipe %d\n", pipe); > + u32 tmp; > > - /* Update the DSI PLL */ > - vlv_enable_dsi_pll(intel_encoder); > - > - /* XXX: Location of the call */ > - band_gap_reset(dev_priv); > + DRM_DEBUG_KMS("\n"); > > /* escape clock divider, 20MHz, shared for A and C. device ready must be > * off when doing this! txclkesc? */ > @@ -345,102 +380,108 @@ static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) > tmp &= ~READ_REQUEST_PRIORITY_MASK; > I915_WRITE(MIPI_CTRL(pipe), tmp | READ_REQUEST_PRIORITY_HIGH); > > - /* XXX: why here, why like this? handling in irq handler?! */ > - I915_WRITE(MIPI_INTR_STAT(pipe), 0xffffffff); > I915_WRITE(MIPI_INTR_EN(pipe), 0xffffffff); > > - I915_WRITE(MIPI_DPHY_PARAM(pipe), > - 0x3c << EXIT_ZERO_COUNT_SHIFT | > - 0x1f << TRAIL_COUNT_SHIFT | > - 0xc5 << CLK_ZERO_COUNT_SHIFT | > - 0x1f << PREPARE_COUNT_SHIFT); > + I915_WRITE(MIPI_DPHY_PARAM(pipe), intel_dsi->dphy_reg); > +} > > - I915_WRITE(MIPI_DPI_RESOLUTION(pipe), > - adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT | > - adjusted_mode->hdisplay << HORIZONTAL_ADDRESS_SHIFT); > +static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) > +{ > + struct drm_encoder *encoder = &intel_encoder->base; > + struct drm_device *dev = encoder->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); > + struct drm_display_mode *adjusted_mode = > + &intel_crtc->config.adjusted_mode; > + int pipe = intel_crtc->pipe; > + unsigned int bpp = intel_crtc->config.pipe_bpp; > + u32 val; > > - set_dsi_timings(encoder, adjusted_mode); > + band_gap_reset(dev_priv); > > - val = intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT; > - if (is_cmd_mode(intel_dsi)) { > - val |= intel_dsi->channel << CMD_MODE_CHANNEL_NUMBER_SHIFT; > - val |= CMD_MODE_DATA_WIDTH_8_BIT; /* XXX */ > - } else { > - val |= intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT; > + dsi_config(encoder); > > - /* XXX: cross-check bpp vs. pixel format? */ > - val |= intel_dsi->pixel_format; > - } > - I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); > - > - /* timeouts for recovery. one frame IIUC. if counter expires, EOT and > - * stop state. */ > - > - /* > - * In burst mode, value greater than one DPI line Time in byte clock > - * (txbyteclkhs) To timeout this timer 1+ of the above said value is > - * recommended. > - * > - * In non-burst mode, Value greater than one DPI frame time in byte > - * clock(txbyteclkhs) To timeout this timer 1+ of the above said value > - * is recommended. > - * > - * In DBI only mode, value greater than one DBI frame time in byte > - * clock(txbyteclkhs) To timeout this timer 1+ of the above said value > - * is recommended. > - */ > - > - if (is_vid_mode(intel_dsi) && > - intel_dsi->video_mode_format == VIDEO_MODE_BURST) { > - I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), > - txbyteclkhs(adjusted_mode->htotal, bpp, > - intel_dsi->lane_count) + 1); > - } else { > - I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), > - txbyteclkhs(adjusted_mode->vtotal * > - adjusted_mode->htotal, > - bpp, intel_dsi->lane_count) + 1); > - } > - I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), 8309); /* max */ > - I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), 0x14); /* max */ > - I915_WRITE(MIPI_DEVICE_RESET_TIMER(pipe), 0xffff); /* max */ > + I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout); > + I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), > + intel_dsi->turn_arnd_val); > + I915_WRITE(MIPI_DEVICE_RESET_TIMER(pipe), > + intel_dsi->rst_timer_val); > + /* in terms of low power clock */ > + I915_WRITE(MIPI_INIT_COUNT(pipe), intel_dsi->init_count); > > - /* dphy stuff */ > + if (intel_dsi->eot_disable) > + I915_WRITE(MIPI_EOT_DISABLE(pipe), 0); > + else > + I915_WRITE(MIPI_EOT_DISABLE(pipe), 1); > > - /* in terms of low power clock */ > - I915_WRITE(MIPI_INIT_COUNT(pipe), txclkesc(ESCAPE_CLOCK_DIVIDER_1, 100)); > - > - /* recovery disables */ > - I915_WRITE(MIPI_EOT_DISABLE(pipe), intel_dsi->eot_disable); > - > - /* in terms of txbyteclkhs. actual high to low switch + > - * MIPI_STOP_STATE_STALL * MIPI_LP_BYTECLK. > - * > - * XXX: write MIPI_STOP_STATE_STALL? > - */ > - I915_WRITE(MIPI_HIGH_LOW_SWITCH_COUNT(pipe), 0x46); > - > - /* XXX: low power clock equivalence in terms of byte clock. the number > - * of byte clocks occupied in one low power clock. based on txbyteclkhs > - * and txclkesc. txclkesc time / txbyteclk time * (105 + > - * MIPI_STOP_STATE_STALL) / 105.??? > - */ > - I915_WRITE(MIPI_LP_BYTECLK(pipe), 4); > - > - /* the bw essential for transmitting 16 long packets containing 252 > - * bytes meant for dcs write memory command is programmed in this > - * register in terms of byte clocks. based on dsi transfer rate and the > - * number of lanes configured the time taken to transmit 16 long packets > - * in a dsi stream varies. */ > - I915_WRITE(MIPI_DBI_BW_CTRL(pipe), 0x820); > + I915_WRITE(MIPI_HIGH_LOW_SWITCH_COUNT(pipe), \ > + intel_dsi->hs_to_lp_count); > + I915_WRITE(MIPI_LP_BYTECLK(pipe), intel_dsi->lp_byte_clk); > + > + I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(pipe), 0x64); > > I915_WRITE(MIPI_CLK_LANE_SWITCH_TIME_CNT(pipe), > - 0xa << LP_HS_SSW_CNT_SHIFT | > - 0x14 << HS_LP_PWR_SW_CNT_SHIFT); > + ((u32)intel_dsi->clk_lp_to_hs_count > + << LP_HS_SSW_CNT_SHIFT) | > + (intel_dsi->clk_hs_to_lp_count << HS_LP_PWR_SW_CNT_SHIFT)); > + > + if (is_vid_mode(intel_dsi)) { > + I915_WRITE(MIPI_DPI_RESOLUTION(pipe), > + (adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT) | > + (adjusted_mode->hdisplay << HORIZONTAL_ADDRESS_SHIFT)); > + > + set_dsi_timings(encoder, adjusted_mode); > + > + val = intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT | > + intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT | > + intel_dsi->pixel_format; > + I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); > + > + /* timeouts for recovery. one frame IIUC. if counter expires, EOT and > + * stop state. */ > + > + /* > + * In burst mode, value greater than one DPI line Time in byte clock > + * (txbyteclkhs) To timeout this timer 1+ of the above said value is > + * recommended. > + * > + * In non-burst mode, Value greater than one DPI frame time in byte > + * clock(txbyteclkhs) To timeout this timer 1+ of the above said value > + * is recommended. > + * > + * In DBI only mode, value greater than one DBI frame time in byte > + * clock(txbyteclkhs) To timeout this timer 1+ of the above said value > + * is recommended. > + */ > + > + if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) { > + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), > + txbyteclkhs(adjusted_mode->htotal, bpp, > + intel_dsi->lane_count) + 1); > + } else { > + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), > + txbyteclkhs(adjusted_mode->vtotal * > + adjusted_mode->htotal, > + bpp, intel_dsi->lane_count) + 1); > + } > > - if (is_vid_mode(intel_dsi)) > I915_WRITE(MIPI_VIDEO_MODE_FORMAT(pipe), > - intel_dsi->video_mode_format); > + intel_dsi->video_frmt_cfg_bits | > + intel_dsi->video_mode_type); > + } else { > + val = intel_dsi->channel << CMD_MODE_CHANNEL_NUMBER_SHIFT | > + intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT | > + intel_dsi->data_width; > + I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); > + > + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), > + txbyteclkhs(adjusted_mode->vtotal * > + adjusted_mode->htotal, > + bpp, intel_dsi->lane_count) + 1); > + > + I915_WRITE(MIPI_DBI_BW_CTRL(pipe), intel_dsi->bw_timer); > + } > } > > static enum drm_connector_status > @@ -584,6 +625,7 @@ bool intel_dsi_init(struct drm_device *dev) > > fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; > intel_panel_init(&intel_connector->panel, fixed_mode); > + intel_panel_setup_backlight(connector); > > return true; > > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 10/21/2013 6:53 PM, Ville Syrjälä wrote: > On Mon, Oct 21, 2013 at 05:51:07PM +0530, Shobhit Kumar wrote: >> Has been tested on couple of panels now. > > While it's nice to get patches, I can't say I'm very happy about the > shape of this one. > > The patch contains several changes in one patch. It should be split up > into several patches. Based on a cursory examination I would suggest > something like this: > - weird ULPS ping-pong Suggested and approved sequence by HW team for ULPS entry/exit 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 I will push this is new patch > - add backlight support Ok will push in new patch > - moving the ->disable() call Earlier disable was called at the beginning even before pixel stream was stopped. Ideal flow would be to disable pixel stream and then follow panel's required disable sequence > - each of the new intel_dsi->foo/bar/etc. parameter could probably > be a separate patch Ok, I can break all parameter changes into a separate patch > > As far as the various timeout related parameters are concerned, to me > it would make more sense to specify them in usecs or some other real > world unit. Or you could provide/leave in some helper functions to > calculate the clock based values from some real world values. Few timeouts are as per spec. Are you referring to back-light or shutdown packet delays ? If yes we can change them to usecs. > > And finally justficiation for each of these changes is missing from > the current patch. We want to know why the code has to change. I hope I have provided some clarifications above. I will work on splitting this patch into few more patches for more clarity. Regards Shobhit > >> >> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> >> Signed-off-by: Vijaykumar Balakrishnan <vijayakumar.balakrishnan@intel.com> >> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 11 ++ >> drivers/gpu/drm/i915/intel_dsi.c | 334 +++++++++++++++++++++----------------- >> 2 files changed, 199 insertions(+), 146 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 1c42bb4..1c28d02 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2375,6 +2375,17 @@ __i915_write(64) >> #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) >> #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg) >> >> +#define I915_WRITE_BITS(reg, val, mask) \ >> +do { \ >> + u32 tmp, data; \ >> + tmp = I915_READ((reg)); \ >> + tmp &= ~(mask); \ >> + data = (val) & (mask); \ >> + data = data | tmp; \ >> + I915_WRITE((reg), data); \ >> +} while(0) >> + >> + >> /* "Broadcast RGB" property */ >> #define INTEL_BROADCAST_RGB_AUTO 0 >> #define INTEL_BROADCAST_RGB_FULL 1 >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index 5a9dbfd..241bb55 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -21,6 +21,8 @@ >> * DEALINGS IN THE SOFTWARE. >> * >> * Author: Jani Nikula <jani.nikula@intel.com> >> + * : Shobhit Kumar <shobhit.kumar@intel.com> >> + * : Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> >> */ >> >> #include <drm/drmP.h> >> @@ -101,63 +103,80 @@ 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) >> -{ >> - DRM_DEBUG_KMS("\n"); >> -} >> - >> -static void intel_dsi_enable(struct intel_encoder *encoder) >> +void intel_dsi_device_ready(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); >> - } >> + I915_WRITE_BITS(MIPI_PORT_CTRL(pipe), LP_OUTPUT_HOLD, LP_OUTPUT_HOLD); >> + usleep_range(1000, 1500); >> + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY | >> + ULPS_STATE_EXIT, DEVICE_READY | ULPS_STATE_MASK); >> + usleep_range(2000, 2500); >> + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY, >> + DEVICE_READY | ULPS_STATE_MASK); >> + usleep_range(2000, 2500); >> + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), 0x00, >> + DEVICE_READY | ULPS_STATE_MASK); >> + usleep_range(2000, 2500); >> + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY, >> + DEVICE_READY | ULPS_STATE_MASK); >> + usleep_range(2000, 2500); >> >> if (intel_dsi->dev.dev_ops->send_otp_cmds) >> intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev); >> >> +} >> +static void intel_dsi_pre_enable(struct intel_encoder *encoder) >> +{ >> + DRM_DEBUG_KMS("\n"); >> + >> + /* put device in ready state */ >> + intel_dsi_device_ready(encoder); >> +} >> + >> +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); >> >> /* assert ip_tg_enable signal */ >> temp = I915_READ(MIPI_PORT_CTRL(pipe)); >> + temp = temp | intel_dsi->port_bits; >> I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE); >> POSTING_READ(MIPI_PORT_CTRL(pipe)); >> } >> >> if (intel_dsi->dev.dev_ops->enable) >> intel_dsi->dev.dev_ops->enable(&intel_dsi->dev); >> + >> + intel_panel_enable_backlight(dev, pipe); >> } >> >> 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,11 +184,18 @@ static void intel_dsi_disable(struct intel_encoder *encoder) >> >> DRM_DEBUG_KMS("\n"); >> >> - intel_dsi->dev.dev_ops->disable(&intel_dsi->dev); >> + intel_panel_disable_backlight(dev); >> + if (intel_dsi->backlight_off_delay >= 20) >> + msleep(intel_dsi->backlight_off_delay); >> + else >> + usleep_range(intel_dsi->backlight_off_delay * 1000, >> + (intel_dsi->backlight_off_delay * 1000) + 500); >> >> if (is_vid_mode(intel_dsi)) { >> - dpi_send_cmd(intel_dsi, SHUTDOWN); >> - msleep(10); >> + if (intel_dsi->send_shutdown) { >> + dpi_send_cmd(intel_dsi, SHUTDOWN); >> + msleep(intel_dsi->shutdown_pkt_delay); >> + } >> >> /* de-assert ip_tg_enable signal */ >> temp = I915_READ(MIPI_PORT_CTRL(pipe)); >> @@ -179,19 +205,52 @@ 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) >> { >> + 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; >> + >> DRM_DEBUG_KMS("\n"); >> >> + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER, >> + ULPS_STATE_MASK); >> + usleep_range(2000, 2500); >> + >> + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_EXIT, >> + ULPS_STATE_MASK); >> + usleep_range(2000, 2500); >> + >> + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER, >> + ULPS_STATE_MASK); >> + usleep_range(2000, 2500); >> + >> + I915_WRITE_BITS(MIPI_PORT_CTRL(pipe), 0, LP_OUTPUT_HOLD); >> + usleep_range(1000, 1500); >> + >> + if (wait_for(((I915_READ(MIPI_PORT_CTRL(pipe)) & 0x20000) >> + == 0x00000), 30)) >> + DRM_ERROR("DSI LP not going Low\n"); >> + >> + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), 0x00, DEVICE_READY); >> + usleep_range(2000, 2500); >> + >> vlv_disable_dsi_pll(encoder); >> + >> + if (intel_dsi->dev.dev_ops->disable_panel_power) >> + intel_dsi->dev.dev_ops->disable_panel_power(&intel_dsi->dev); >> +} >> +static void intel_dsi_post_disable(struct intel_encoder *encoder) >> +{ >> + DRM_DEBUG_KMS("\n"); >> + intel_dsi_clear_device_ready(encoder); >> } >> >> static bool intel_dsi_get_hw_state(struct intel_encoder *encoder, >> @@ -251,20 +310,6 @@ static int intel_dsi_mode_valid(struct drm_connector *connector, >> return intel_dsi->dev.dev_ops->mode_valid(&intel_dsi->dev, mode); >> } >> >> -/* return txclkesc cycles in terms of divider and duration in us */ >> -static u16 txclkesc(u32 divider, unsigned int us) >> -{ >> - switch (divider) { >> - case ESCAPE_CLOCK_DIVIDER_1: >> - default: >> - return 20 * us; >> - case ESCAPE_CLOCK_DIVIDER_2: >> - return 10 * us; >> - case ESCAPE_CLOCK_DIVIDER_4: >> - return 5 * us; >> - } >> -} >> - >> /* return pixels in terms of txbyteclkhs */ >> static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count) >> { >> @@ -313,26 +358,16 @@ static void set_dsi_timings(struct drm_encoder *encoder, >> I915_WRITE(MIPI_VBP_COUNT(pipe), vbp); >> } >> >> -static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) >> +static void dsi_config(struct drm_encoder *encoder) >> { >> - struct drm_encoder *encoder = &intel_encoder->base; >> struct drm_device *dev = encoder->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); >> - struct drm_display_mode *adjusted_mode = >> - &intel_crtc->config.adjusted_mode; >> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); >> int pipe = intel_crtc->pipe; >> - unsigned int bpp = intel_crtc->config.pipe_bpp; >> - u32 val, tmp; >> - >> - DRM_DEBUG_KMS("pipe %d\n", pipe); >> + u32 tmp; >> >> - /* Update the DSI PLL */ >> - vlv_enable_dsi_pll(intel_encoder); >> - >> - /* XXX: Location of the call */ >> - band_gap_reset(dev_priv); >> + DRM_DEBUG_KMS("\n"); >> >> /* escape clock divider, 20MHz, shared for A and C. device ready must be >> * off when doing this! txclkesc? */ >> @@ -345,102 +380,108 @@ static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) >> tmp &= ~READ_REQUEST_PRIORITY_MASK; >> I915_WRITE(MIPI_CTRL(pipe), tmp | READ_REQUEST_PRIORITY_HIGH); >> >> - /* XXX: why here, why like this? handling in irq handler?! */ >> - I915_WRITE(MIPI_INTR_STAT(pipe), 0xffffffff); >> I915_WRITE(MIPI_INTR_EN(pipe), 0xffffffff); >> >> - I915_WRITE(MIPI_DPHY_PARAM(pipe), >> - 0x3c << EXIT_ZERO_COUNT_SHIFT | >> - 0x1f << TRAIL_COUNT_SHIFT | >> - 0xc5 << CLK_ZERO_COUNT_SHIFT | >> - 0x1f << PREPARE_COUNT_SHIFT); >> + I915_WRITE(MIPI_DPHY_PARAM(pipe), intel_dsi->dphy_reg); >> +} >> >> - I915_WRITE(MIPI_DPI_RESOLUTION(pipe), >> - adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT | >> - adjusted_mode->hdisplay << HORIZONTAL_ADDRESS_SHIFT); >> +static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) >> +{ >> + struct drm_encoder *encoder = &intel_encoder->base; >> + struct drm_device *dev = encoder->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); >> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); >> + struct drm_display_mode *adjusted_mode = >> + &intel_crtc->config.adjusted_mode; >> + int pipe = intel_crtc->pipe; >> + unsigned int bpp = intel_crtc->config.pipe_bpp; >> + u32 val; >> >> - set_dsi_timings(encoder, adjusted_mode); >> + band_gap_reset(dev_priv); >> >> - val = intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT; >> - if (is_cmd_mode(intel_dsi)) { >> - val |= intel_dsi->channel << CMD_MODE_CHANNEL_NUMBER_SHIFT; >> - val |= CMD_MODE_DATA_WIDTH_8_BIT; /* XXX */ >> - } else { >> - val |= intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT; >> + dsi_config(encoder); >> >> - /* XXX: cross-check bpp vs. pixel format? */ >> - val |= intel_dsi->pixel_format; >> - } >> - I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); >> - >> - /* timeouts for recovery. one frame IIUC. if counter expires, EOT and >> - * stop state. */ >> - >> - /* >> - * In burst mode, value greater than one DPI line Time in byte clock >> - * (txbyteclkhs) To timeout this timer 1+ of the above said value is >> - * recommended. >> - * >> - * In non-burst mode, Value greater than one DPI frame time in byte >> - * clock(txbyteclkhs) To timeout this timer 1+ of the above said value >> - * is recommended. >> - * >> - * In DBI only mode, value greater than one DBI frame time in byte >> - * clock(txbyteclkhs) To timeout this timer 1+ of the above said value >> - * is recommended. >> - */ >> - >> - if (is_vid_mode(intel_dsi) && >> - intel_dsi->video_mode_format == VIDEO_MODE_BURST) { >> - I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), >> - txbyteclkhs(adjusted_mode->htotal, bpp, >> - intel_dsi->lane_count) + 1); >> - } else { >> - I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), >> - txbyteclkhs(adjusted_mode->vtotal * >> - adjusted_mode->htotal, >> - bpp, intel_dsi->lane_count) + 1); >> - } >> - I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), 8309); /* max */ >> - I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), 0x14); /* max */ >> - I915_WRITE(MIPI_DEVICE_RESET_TIMER(pipe), 0xffff); /* max */ >> + I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout); >> + I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), >> + intel_dsi->turn_arnd_val); >> + I915_WRITE(MIPI_DEVICE_RESET_TIMER(pipe), >> + intel_dsi->rst_timer_val); >> + /* in terms of low power clock */ >> + I915_WRITE(MIPI_INIT_COUNT(pipe), intel_dsi->init_count); >> >> - /* dphy stuff */ >> + if (intel_dsi->eot_disable) >> + I915_WRITE(MIPI_EOT_DISABLE(pipe), 0); >> + else >> + I915_WRITE(MIPI_EOT_DISABLE(pipe), 1); >> >> - /* in terms of low power clock */ >> - I915_WRITE(MIPI_INIT_COUNT(pipe), txclkesc(ESCAPE_CLOCK_DIVIDER_1, 100)); >> - >> - /* recovery disables */ >> - I915_WRITE(MIPI_EOT_DISABLE(pipe), intel_dsi->eot_disable); >> - >> - /* in terms of txbyteclkhs. actual high to low switch + >> - * MIPI_STOP_STATE_STALL * MIPI_LP_BYTECLK. >> - * >> - * XXX: write MIPI_STOP_STATE_STALL? >> - */ >> - I915_WRITE(MIPI_HIGH_LOW_SWITCH_COUNT(pipe), 0x46); >> - >> - /* XXX: low power clock equivalence in terms of byte clock. the number >> - * of byte clocks occupied in one low power clock. based on txbyteclkhs >> - * and txclkesc. txclkesc time / txbyteclk time * (105 + >> - * MIPI_STOP_STATE_STALL) / 105.??? >> - */ >> - I915_WRITE(MIPI_LP_BYTECLK(pipe), 4); >> - >> - /* the bw essential for transmitting 16 long packets containing 252 >> - * bytes meant for dcs write memory command is programmed in this >> - * register in terms of byte clocks. based on dsi transfer rate and the >> - * number of lanes configured the time taken to transmit 16 long packets >> - * in a dsi stream varies. */ >> - I915_WRITE(MIPI_DBI_BW_CTRL(pipe), 0x820); >> + I915_WRITE(MIPI_HIGH_LOW_SWITCH_COUNT(pipe), \ >> + intel_dsi->hs_to_lp_count); >> + I915_WRITE(MIPI_LP_BYTECLK(pipe), intel_dsi->lp_byte_clk); >> + >> + I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(pipe), 0x64); >> >> I915_WRITE(MIPI_CLK_LANE_SWITCH_TIME_CNT(pipe), >> - 0xa << LP_HS_SSW_CNT_SHIFT | >> - 0x14 << HS_LP_PWR_SW_CNT_SHIFT); >> + ((u32)intel_dsi->clk_lp_to_hs_count >> + << LP_HS_SSW_CNT_SHIFT) | >> + (intel_dsi->clk_hs_to_lp_count << HS_LP_PWR_SW_CNT_SHIFT)); >> + >> + if (is_vid_mode(intel_dsi)) { >> + I915_WRITE(MIPI_DPI_RESOLUTION(pipe), >> + (adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT) | >> + (adjusted_mode->hdisplay << HORIZONTAL_ADDRESS_SHIFT)); >> + >> + set_dsi_timings(encoder, adjusted_mode); >> + >> + val = intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT | >> + intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT | >> + intel_dsi->pixel_format; >> + I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); >> + >> + /* timeouts for recovery. one frame IIUC. if counter expires, EOT and >> + * stop state. */ >> + >> + /* >> + * In burst mode, value greater than one DPI line Time in byte clock >> + * (txbyteclkhs) To timeout this timer 1+ of the above said value is >> + * recommended. >> + * >> + * In non-burst mode, Value greater than one DPI frame time in byte >> + * clock(txbyteclkhs) To timeout this timer 1+ of the above said value >> + * is recommended. >> + * >> + * In DBI only mode, value greater than one DBI frame time in byte >> + * clock(txbyteclkhs) To timeout this timer 1+ of the above said value >> + * is recommended. >> + */ >> + >> + if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) { >> + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), >> + txbyteclkhs(adjusted_mode->htotal, bpp, >> + intel_dsi->lane_count) + 1); >> + } else { >> + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), >> + txbyteclkhs(adjusted_mode->vtotal * >> + adjusted_mode->htotal, >> + bpp, intel_dsi->lane_count) + 1); >> + } >> >> - if (is_vid_mode(intel_dsi)) >> I915_WRITE(MIPI_VIDEO_MODE_FORMAT(pipe), >> - intel_dsi->video_mode_format); >> + intel_dsi->video_frmt_cfg_bits | >> + intel_dsi->video_mode_type); >> + } else { >> + val = intel_dsi->channel << CMD_MODE_CHANNEL_NUMBER_SHIFT | >> + intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT | >> + intel_dsi->data_width; >> + I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); >> + >> + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), >> + txbyteclkhs(adjusted_mode->vtotal * >> + adjusted_mode->htotal, >> + bpp, intel_dsi->lane_count) + 1); >> + >> + I915_WRITE(MIPI_DBI_BW_CTRL(pipe), intel_dsi->bw_timer); >> + } >> } >> >> static enum drm_connector_status >> @@ -584,6 +625,7 @@ bool intel_dsi_init(struct drm_device *dev) >> >> fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; >> intel_panel_init(&intel_connector->panel, fixed_mode); >> + intel_panel_setup_backlight(connector); >> >> return true; >> >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Tue, Oct 22, 2013 at 02:36:18PM +0530, Shobhit Kumar wrote: > On 10/21/2013 6:53 PM, Ville Syrjälä wrote: > > On Mon, Oct 21, 2013 at 05:51:07PM +0530, Shobhit Kumar wrote: > >> Has been tested on couple of panels now. > > > > While it's nice to get patches, I can't say I'm very happy about the > > shape of this one. > > > > The patch contains several changes in one patch. It should be split up > > into several patches. Based on a cursory examination I would suggest > > something like this: > > - weird ULPS ping-pong > > Suggested and approved sequence by HW team for ULPS entry/exit 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 > > I will push this is new patch > > > - add backlight support > > Ok will push in new patch > > > - moving the ->disable() call > > Earlier disable was called at the beginning even before pixel stream was > stopped. Ideal flow would be to disable pixel stream and then follow > panel's required disable sequence > > > - each of the new intel_dsi->foo/bar/etc. parameter could probably > > be a separate patch > > Ok, I can break all parameter changes into a separate patch > > > > > As far as the various timeout related parameters are concerned, to me > > it would make more sense to specify them in usecs or some other real > > world unit. Or you could provide/leave in some helper functions to > > calculate the clock based values from some real world values. > > Few timeouts are as per spec. Are you referring to back-light or > shutdown packet delays ? If yes we can change them to usecs. These at least: MIPI_LP_RX_TIMEOUT MIPI_TURN_AROUND_TIMEOUT MIPI_DEVICE_RESET_TIMER MIPI_INIT_COUNT MIPI_HIGH_LOW_SWITCH_COUNT It's been a while since I read the spec so I don't remember anymore how all those were specified there. If the spec defines them in some clocks, then that would be the best choice, but if they're specified in some time units, then I would possibly follow that. At the very least you should add some documentation about the units in the intel_dsi struct (or whereever we expect these things to live). > > > > > And finally justficiation for each of these changes is missing from > > the current patch. We want to know why the code has to change. > > I hope I have provided some clarifications above. I will work on > splitting this patch into few more patches for more clarity. Yeah, looks like good stuff. Looking forward to seeing the split up patches. Thanks.
On 10/22/2013 04:19 PM, Ville Syrjälä wrote: > On Tue, Oct 22, 2013 at 02:36:18PM +0530, Shobhit Kumar wrote: >> On 10/21/2013 6:53 PM, Ville Syrjälä wrote: >>> On Mon, Oct 21, 2013 at 05:51:07PM +0530, Shobhit Kumar wrote: >>>> Has been tested on couple of panels now. >>> >>> While it's nice to get patches, I can't say I'm very happy about the >>> shape of this one. >>> >>> The patch contains several changes in one patch. It should be split up >>> into several patches. Based on a cursory examination I would suggest >>> something like this: >>> - weird ULPS ping-pong >> >> Suggested and approved sequence by HW team for ULPS entry/exit 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 >> >> I will push this is new patch >> >>> - add backlight support >> >> Ok will push in new patch >> >>> - moving the ->disable() call >> >> Earlier disable was called at the beginning even before pixel stream was >> stopped. Ideal flow would be to disable pixel stream and then follow >> panel's required disable sequence >> >>> - each of the new intel_dsi->foo/bar/etc. parameter could probably >>> be a separate patch >> >> Ok, I can break all parameter changes into a separate patch >> >>> >>> As far as the various timeout related parameters are concerned, to me >>> it would make more sense to specify them in usecs or some other real >>> world unit. Or you could provide/leave in some helper functions to >>> calculate the clock based values from some real world values. >> >> Few timeouts are as per spec. Are you referring to back-light or >> shutdown packet delays ? If yes we can change them to usecs. > > These at least: > MIPI_LP_RX_TIMEOUT > MIPI_TURN_AROUND_TIMEOUT > MIPI_DEVICE_RESET_TIMER > MIPI_INIT_COUNT > MIPI_HIGH_LOW_SWITCH_COUNT > > It's been a while since I read the spec so I don't remember anymore how > all those were specified there. If the spec defines them in some clocks, > then that would be the best choice, but if they're specified in some > time units, then I would possibly follow that. At the very least you > should add some documentation about the units in the intel_dsi struct > (or whereever we expect these things to live). > Ok, got it. All the above are defined as byte clocks. Will take care as per your suggestions. >> >>> >>> And finally justficiation for each of these changes is missing from >>> the current patch. We want to know why the code has to change. >> >> I hope I have provided some clarifications above. I will work on >> splitting this patch into few more patches for more clarity. > > Yeah, looks like good stuff. Looking forward to seeing the split up > patches. Thanks. > WIP Regards Shobhit
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1c42bb4..1c28d02 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2375,6 +2375,17 @@ __i915_write(64) #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg) +#define I915_WRITE_BITS(reg, val, mask) \ +do { \ + u32 tmp, data; \ + tmp = I915_READ((reg)); \ + tmp &= ~(mask); \ + data = (val) & (mask); \ + data = data | tmp; \ + I915_WRITE((reg), data); \ +} while(0) + + /* "Broadcast RGB" property */ #define INTEL_BROADCAST_RGB_AUTO 0 #define INTEL_BROADCAST_RGB_FULL 1 diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 5a9dbfd..241bb55 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -21,6 +21,8 @@ * DEALINGS IN THE SOFTWARE. * * Author: Jani Nikula <jani.nikula@intel.com> + * : Shobhit Kumar <shobhit.kumar@intel.com> + * : Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> */ #include <drm/drmP.h> @@ -101,63 +103,80 @@ 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) -{ - DRM_DEBUG_KMS("\n"); -} - -static void intel_dsi_enable(struct intel_encoder *encoder) +void intel_dsi_device_ready(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); - } + I915_WRITE_BITS(MIPI_PORT_CTRL(pipe), LP_OUTPUT_HOLD, LP_OUTPUT_HOLD); + usleep_range(1000, 1500); + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY | + ULPS_STATE_EXIT, DEVICE_READY | ULPS_STATE_MASK); + usleep_range(2000, 2500); + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY, + DEVICE_READY | ULPS_STATE_MASK); + usleep_range(2000, 2500); + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), 0x00, + DEVICE_READY | ULPS_STATE_MASK); + usleep_range(2000, 2500); + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY, + DEVICE_READY | ULPS_STATE_MASK); + usleep_range(2000, 2500); if (intel_dsi->dev.dev_ops->send_otp_cmds) intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev); +} +static void intel_dsi_pre_enable(struct intel_encoder *encoder) +{ + DRM_DEBUG_KMS("\n"); + + /* put device in ready state */ + intel_dsi_device_ready(encoder); +} + +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); /* assert ip_tg_enable signal */ temp = I915_READ(MIPI_PORT_CTRL(pipe)); + temp = temp | intel_dsi->port_bits; I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE); POSTING_READ(MIPI_PORT_CTRL(pipe)); } if (intel_dsi->dev.dev_ops->enable) intel_dsi->dev.dev_ops->enable(&intel_dsi->dev); + + intel_panel_enable_backlight(dev, pipe); } 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,11 +184,18 @@ static void intel_dsi_disable(struct intel_encoder *encoder) DRM_DEBUG_KMS("\n"); - intel_dsi->dev.dev_ops->disable(&intel_dsi->dev); + intel_panel_disable_backlight(dev); + if (intel_dsi->backlight_off_delay >= 20) + msleep(intel_dsi->backlight_off_delay); + else + usleep_range(intel_dsi->backlight_off_delay * 1000, + (intel_dsi->backlight_off_delay * 1000) + 500); if (is_vid_mode(intel_dsi)) { - dpi_send_cmd(intel_dsi, SHUTDOWN); - msleep(10); + if (intel_dsi->send_shutdown) { + dpi_send_cmd(intel_dsi, SHUTDOWN); + msleep(intel_dsi->shutdown_pkt_delay); + } /* de-assert ip_tg_enable signal */ temp = I915_READ(MIPI_PORT_CTRL(pipe)); @@ -179,19 +205,52 @@ 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) { + 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; + DRM_DEBUG_KMS("\n"); + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER, + ULPS_STATE_MASK); + usleep_range(2000, 2500); + + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_EXIT, + ULPS_STATE_MASK); + usleep_range(2000, 2500); + + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER, + ULPS_STATE_MASK); + usleep_range(2000, 2500); + + I915_WRITE_BITS(MIPI_PORT_CTRL(pipe), 0, LP_OUTPUT_HOLD); + usleep_range(1000, 1500); + + if (wait_for(((I915_READ(MIPI_PORT_CTRL(pipe)) & 0x20000) + == 0x00000), 30)) + DRM_ERROR("DSI LP not going Low\n"); + + I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), 0x00, DEVICE_READY); + usleep_range(2000, 2500); + vlv_disable_dsi_pll(encoder); + + if (intel_dsi->dev.dev_ops->disable_panel_power) + intel_dsi->dev.dev_ops->disable_panel_power(&intel_dsi->dev); +} +static void intel_dsi_post_disable(struct intel_encoder *encoder) +{ + DRM_DEBUG_KMS("\n"); + intel_dsi_clear_device_ready(encoder); } static bool intel_dsi_get_hw_state(struct intel_encoder *encoder, @@ -251,20 +310,6 @@ static int intel_dsi_mode_valid(struct drm_connector *connector, return intel_dsi->dev.dev_ops->mode_valid(&intel_dsi->dev, mode); } -/* return txclkesc cycles in terms of divider and duration in us */ -static u16 txclkesc(u32 divider, unsigned int us) -{ - switch (divider) { - case ESCAPE_CLOCK_DIVIDER_1: - default: - return 20 * us; - case ESCAPE_CLOCK_DIVIDER_2: - return 10 * us; - case ESCAPE_CLOCK_DIVIDER_4: - return 5 * us; - } -} - /* return pixels in terms of txbyteclkhs */ static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count) { @@ -313,26 +358,16 @@ static void set_dsi_timings(struct drm_encoder *encoder, I915_WRITE(MIPI_VBP_COUNT(pipe), vbp); } -static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) +static void dsi_config(struct drm_encoder *encoder) { - struct drm_encoder *encoder = &intel_encoder->base; struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); - struct drm_display_mode *adjusted_mode = - &intel_crtc->config.adjusted_mode; + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); int pipe = intel_crtc->pipe; - unsigned int bpp = intel_crtc->config.pipe_bpp; - u32 val, tmp; - - DRM_DEBUG_KMS("pipe %d\n", pipe); + u32 tmp; - /* Update the DSI PLL */ - vlv_enable_dsi_pll(intel_encoder); - - /* XXX: Location of the call */ - band_gap_reset(dev_priv); + DRM_DEBUG_KMS("\n"); /* escape clock divider, 20MHz, shared for A and C. device ready must be * off when doing this! txclkesc? */ @@ -345,102 +380,108 @@ static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) tmp &= ~READ_REQUEST_PRIORITY_MASK; I915_WRITE(MIPI_CTRL(pipe), tmp | READ_REQUEST_PRIORITY_HIGH); - /* XXX: why here, why like this? handling in irq handler?! */ - I915_WRITE(MIPI_INTR_STAT(pipe), 0xffffffff); I915_WRITE(MIPI_INTR_EN(pipe), 0xffffffff); - I915_WRITE(MIPI_DPHY_PARAM(pipe), - 0x3c << EXIT_ZERO_COUNT_SHIFT | - 0x1f << TRAIL_COUNT_SHIFT | - 0xc5 << CLK_ZERO_COUNT_SHIFT | - 0x1f << PREPARE_COUNT_SHIFT); + I915_WRITE(MIPI_DPHY_PARAM(pipe), intel_dsi->dphy_reg); +} - I915_WRITE(MIPI_DPI_RESOLUTION(pipe), - adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT | - adjusted_mode->hdisplay << HORIZONTAL_ADDRESS_SHIFT); +static void intel_dsi_mode_set(struct intel_encoder *intel_encoder) +{ + struct drm_encoder *encoder = &intel_encoder->base; + struct drm_device *dev = encoder->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); + struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); + struct drm_display_mode *adjusted_mode = + &intel_crtc->config.adjusted_mode; + int pipe = intel_crtc->pipe; + unsigned int bpp = intel_crtc->config.pipe_bpp; + u32 val; - set_dsi_timings(encoder, adjusted_mode); + band_gap_reset(dev_priv); - val = intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT; - if (is_cmd_mode(intel_dsi)) { - val |= intel_dsi->channel << CMD_MODE_CHANNEL_NUMBER_SHIFT; - val |= CMD_MODE_DATA_WIDTH_8_BIT; /* XXX */ - } else { - val |= intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT; + dsi_config(encoder); - /* XXX: cross-check bpp vs. pixel format? */ - val |= intel_dsi->pixel_format; - } - I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); - - /* timeouts for recovery. one frame IIUC. if counter expires, EOT and - * stop state. */ - - /* - * In burst mode, value greater than one DPI line Time in byte clock - * (txbyteclkhs) To timeout this timer 1+ of the above said value is - * recommended. - * - * In non-burst mode, Value greater than one DPI frame time in byte - * clock(txbyteclkhs) To timeout this timer 1+ of the above said value - * is recommended. - * - * In DBI only mode, value greater than one DBI frame time in byte - * clock(txbyteclkhs) To timeout this timer 1+ of the above said value - * is recommended. - */ - - if (is_vid_mode(intel_dsi) && - intel_dsi->video_mode_format == VIDEO_MODE_BURST) { - I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), - txbyteclkhs(adjusted_mode->htotal, bpp, - intel_dsi->lane_count) + 1); - } else { - I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), - txbyteclkhs(adjusted_mode->vtotal * - adjusted_mode->htotal, - bpp, intel_dsi->lane_count) + 1); - } - I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), 8309); /* max */ - I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), 0x14); /* max */ - I915_WRITE(MIPI_DEVICE_RESET_TIMER(pipe), 0xffff); /* max */ + I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout); + I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), + intel_dsi->turn_arnd_val); + I915_WRITE(MIPI_DEVICE_RESET_TIMER(pipe), + intel_dsi->rst_timer_val); + /* in terms of low power clock */ + I915_WRITE(MIPI_INIT_COUNT(pipe), intel_dsi->init_count); - /* dphy stuff */ + if (intel_dsi->eot_disable) + I915_WRITE(MIPI_EOT_DISABLE(pipe), 0); + else + I915_WRITE(MIPI_EOT_DISABLE(pipe), 1); - /* in terms of low power clock */ - I915_WRITE(MIPI_INIT_COUNT(pipe), txclkesc(ESCAPE_CLOCK_DIVIDER_1, 100)); - - /* recovery disables */ - I915_WRITE(MIPI_EOT_DISABLE(pipe), intel_dsi->eot_disable); - - /* in terms of txbyteclkhs. actual high to low switch + - * MIPI_STOP_STATE_STALL * MIPI_LP_BYTECLK. - * - * XXX: write MIPI_STOP_STATE_STALL? - */ - I915_WRITE(MIPI_HIGH_LOW_SWITCH_COUNT(pipe), 0x46); - - /* XXX: low power clock equivalence in terms of byte clock. the number - * of byte clocks occupied in one low power clock. based on txbyteclkhs - * and txclkesc. txclkesc time / txbyteclk time * (105 + - * MIPI_STOP_STATE_STALL) / 105.??? - */ - I915_WRITE(MIPI_LP_BYTECLK(pipe), 4); - - /* the bw essential for transmitting 16 long packets containing 252 - * bytes meant for dcs write memory command is programmed in this - * register in terms of byte clocks. based on dsi transfer rate and the - * number of lanes configured the time taken to transmit 16 long packets - * in a dsi stream varies. */ - I915_WRITE(MIPI_DBI_BW_CTRL(pipe), 0x820); + I915_WRITE(MIPI_HIGH_LOW_SWITCH_COUNT(pipe), \ + intel_dsi->hs_to_lp_count); + I915_WRITE(MIPI_LP_BYTECLK(pipe), intel_dsi->lp_byte_clk); + + I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(pipe), 0x64); I915_WRITE(MIPI_CLK_LANE_SWITCH_TIME_CNT(pipe), - 0xa << LP_HS_SSW_CNT_SHIFT | - 0x14 << HS_LP_PWR_SW_CNT_SHIFT); + ((u32)intel_dsi->clk_lp_to_hs_count + << LP_HS_SSW_CNT_SHIFT) | + (intel_dsi->clk_hs_to_lp_count << HS_LP_PWR_SW_CNT_SHIFT)); + + if (is_vid_mode(intel_dsi)) { + I915_WRITE(MIPI_DPI_RESOLUTION(pipe), + (adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT) | + (adjusted_mode->hdisplay << HORIZONTAL_ADDRESS_SHIFT)); + + set_dsi_timings(encoder, adjusted_mode); + + val = intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT | + intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT | + intel_dsi->pixel_format; + I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); + + /* timeouts for recovery. one frame IIUC. if counter expires, EOT and + * stop state. */ + + /* + * In burst mode, value greater than one DPI line Time in byte clock + * (txbyteclkhs) To timeout this timer 1+ of the above said value is + * recommended. + * + * In non-burst mode, Value greater than one DPI frame time in byte + * clock(txbyteclkhs) To timeout this timer 1+ of the above said value + * is recommended. + * + * In DBI only mode, value greater than one DBI frame time in byte + * clock(txbyteclkhs) To timeout this timer 1+ of the above said value + * is recommended. + */ + + if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) { + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), + txbyteclkhs(adjusted_mode->htotal, bpp, + intel_dsi->lane_count) + 1); + } else { + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), + txbyteclkhs(adjusted_mode->vtotal * + adjusted_mode->htotal, + bpp, intel_dsi->lane_count) + 1); + } - if (is_vid_mode(intel_dsi)) I915_WRITE(MIPI_VIDEO_MODE_FORMAT(pipe), - intel_dsi->video_mode_format); + intel_dsi->video_frmt_cfg_bits | + intel_dsi->video_mode_type); + } else { + val = intel_dsi->channel << CMD_MODE_CHANNEL_NUMBER_SHIFT | + intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT | + intel_dsi->data_width; + I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val); + + I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe), + txbyteclkhs(adjusted_mode->vtotal * + adjusted_mode->htotal, + bpp, intel_dsi->lane_count) + 1); + + I915_WRITE(MIPI_DBI_BW_CTRL(pipe), intel_dsi->bw_timer); + } } static enum drm_connector_status @@ -584,6 +625,7 @@ bool intel_dsi_init(struct drm_device *dev) fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; intel_panel_init(&intel_connector->panel, fixed_mode); + intel_panel_setup_backlight(connector); return true;