| Message ID | 1397454507-10273-5-git-send-email-shobhit.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 14, 2014 at 11:18:27AM +0530, Shobhit Kumar wrote: > This driver makes use of the generic panel information from the VBT. > Panel information is classified into two - panel configuration and panel > power sequence which is unique to each panel. The generic driver uses the > panel configuration and sequence parsed from VBT block #52 and #53 > > v2: Address review comments by Jani > - Move all of the things in driver c file from header > - Make all functions static > - Make use of video/mipi_display.c instead of redefining > - Null checks during sequence execution > > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> I've done a first past on this. Overall looks reasonable. I'm missing some documentation to double check the various LP->HS, HS->LP count and other magic around the clocks (send you a mail about it) before I can add my r-b tag. I've added a few tiny comments as well along the road.
Thanks Damien for your review On Thursday 15 May 2014 10:18 PM, Damien Lespiau wrote: > On Mon, Apr 14, 2014 at 11:18:27AM +0530, Shobhit Kumar wrote: >> >This driver makes use of the generic panel information from the VBT. >> >Panel information is classified into two - panel configuration and panel >> >power sequence which is unique to each panel. The generic driver uses the >> >panel configuration and sequence parsed from VBT block #52 and #53 >> > >> >v2: Address review comments by Jani >> > - Move all of the things in driver c file from header >> > - Make all functions static >> > - Make use of video/mipi_display.c instead of redefining >> > - Null checks during sequence execution >> > >> >Signed-off-by: Shobhit Kumar<shobhit.kumar@intel.com> > I've done a first past on this. Overall looks reasonable. I'm missing > some documentation to double check the various LP->HS, HS->LP count and > other magic around the clocks (send you a mail about it) before I can > add my r-b tag. > > I've added a few tiny comments as well along the road. All look okay to me and Will push updated patch asap. There is one issue which I am struggling for now. If we have all these patches in, then disable sequence pipe off does not work and wait_for_pipe_off gives a warn dump but everything works. Its not this patch issue but DSI patches that are already merged. I know the fix is to actually disable port after disabling pipe and plane but doing that does not succeed enable in first attempt. Subsequent disable/enable works. Looking into that and should have a fix by next week on that. Regards Shobhit
On Thu, May 15, 2014 at 05:48:57PM +0100, Damien Lespiau wrote: > > +static struct gpio_table gtable[] = { > > const > Please, disregard this comment. It's being written to to track GPIO initialization. -- Damien
On Mon, Apr 14, 2014 at 11:18:27AM +0530, Shobhit Kumar wrote: > +#define NS_MHZ_RATIO 1000000 [...] > +static bool generic_init(struct intel_dsi_device *dsi) > +{ [...] > + /* > + * ui(s) = 1/f [f in hz] > + * ui(ns) = 10^9/f*10^6 [f in Mhz] -> 10^3/f(Mhz) ui(ns) = 10^9/(f*10^6) > + * > + * LP byte clock = TLPX/8ui Mind putting that comment just above the appropriate computation? Also, LP byte clock = Tlpx / (8UI) > + * > + * Since txddrclkhs_i is 2xUI, the count values programmed in > + * DPHY param registers are divided by 2 That looks like a general comment that apply to a bunch of calculations below, probably worth separating it from the UI comment. > + * > + */ > + > + /* in Kbps */ > + ui_num = bitrate; > + ui_den = NS_MHZ_RATIO; I'm a bit confused here, most likely missing something, care to clarify? - IIUC, you want the computations to happen in ns. I'm a bit puzzled by that NS_MHZ_RATIO constant name when we're dealing with Kbps. That constant is 10^6 which seems to be OK for KHz. So maybe just a naming problem? - UI is a period, so is homogeneous to time (s), but ui_num being in s^-1 and ui_den a constant, ui_num/ui_den looks like a frequency. Or could it be that UI = ui_den / ui_num? would be confusing, but the code below would make more sense. In which case could we have UI = ui_num / ui_den? > + > + tclk_prepare_clkzero = mipi_config->tclk_prepare_clkzero; > + ths_prepare_hszero = mipi_config->ths_prepare_hszero; > + > + /* B060 */ > + intel_dsi->lp_byte_clk = CEIL_DIV(tlpx_ns * ui_num, 8 * ui_den); > + > + /* count values in UI = (ns value) * (bitrate / (2 * 10^6)) */ > + /* prepare count */ > + ths_prepare_ns = > + (mipi_config->ths_prepare > mipi_config->tclk_prepare) ? > + mipi_config->ths_prepare : > + mipi_config->tclk_prepare; That looks like max() > + > + prepare_cnt = CEIL_DIV(ths_prepare_ns * ui_num, ui_den * 2); The formula above has a 10^6, why is that OK not to have it there? (in which unit is bitrate in the formula? MHz?) Is this something like: Count in UI = count(ns) / UI(ns) and then as UI = ui_den/ui_num (?!) we end up with: Count in UI = count(ns) * ui_num / ui_den And then the / 2 comment applies. > + > + /* exit zero count */ > + exit_zero_cnt = CEIL_DIV( > + (ths_prepare_hszero - ths_prepare_ns) * ui_num, > + ui_den * 2 > + ); > + > + /* > + * Exit zero is unified val ths_zero and ths_exit > + * minimum value for ths_exit = 110ns > + * min (exit_zero_cnt * 2) = 110/UI > + * exit_zero_cnt = 55/UI > + */ > + if (exit_zero_cnt < (55 * ui_num / ui_den)) > + if ((55 * ui_num) % ui_den) > + exit_zero_cnt += 1; I'm not sure what we're achieving with the +=1 here, mind explaining? > + > + /* clk zero count */ > + clk_zero_cnt = CEIL_DIV( > + (tclk_prepare_clkzero - ths_prepare_ns) > + * ui_num, 2 * ui_den); > + > + /* trail count */ > + tclk_trail_ns = (mipi_config->tclk_trail > mipi_config->ths_trail) ? > + mipi_config->tclk_trail : mipi_config->ths_trail; max() > + trail_cnt = CEIL_DIV(tclk_trail_ns * ui_num, 2 * ui_den); > + > + if (prepare_cnt > PREPARE_CNT_MAX || > + exit_zero_cnt > EXIT_ZERO_CNT_MAX || > + clk_zero_cnt > CLK_ZERO_CNT_MAX || > + trail_cnt > TRAIL_CNT_MAX) > + DRM_DEBUG_DRIVER("Values crossing maximum limits\n"); Is that a situation that may happen in a normal case? or should we go with a DRM_ERROR() here and potentially abort the modeset? > + > + if (prepare_cnt > PREPARE_CNT_MAX) > + prepare_cnt = PREPARE_CNT_MAX; > + > + if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) > + exit_zero_cnt = EXIT_ZERO_CNT_MAX; > + > + if (clk_zero_cnt > CLK_ZERO_CNT_MAX) > + clk_zero_cnt = CLK_ZERO_CNT_MAX; > + > + if (trail_cnt > TRAIL_CNT_MAX) > + trail_cnt = TRAIL_CNT_MAX; > + > + /* B080 */ > + intel_dsi->dphy_reg = exit_zero_cnt << 24 | trail_cnt << 16 | > + clk_zero_cnt << 8 | prepare_cnt; > + > + /* > + * LP to HS switch count = 4TLPX + PREP_COUNT * 2 + EXIT_ZERO_COUNT * 2 > + * + 10UI + Extra Byte Count > + * > + * HS to LP switch count = THS-TRAIL + 2TLPX + Extra Byte Count > + * Extra Byte Count is calculated according to number of lanes. > + * High Low Switch Count is the Max of LP to HS and > + * HS to LP switch count > + * > + */ > + tlpx_ui = CEIL_DIV(tlpx_ns * ui_num, ui_den); > + > + /* B044 */ > + intel_dsi->hs_to_lp_count = > + CEIL_DIV( > + 4 * tlpx_ui + prepare_cnt * 2 + > + exit_zero_cnt * 2 + 10, > + 8); The previous was before I tried to look at the spec too closely. Mind explaining why we don't look at the HS to LP switch count? ie why HS to LP switch cound is always smaller than the LP to HS one? > + > + intel_dsi->hs_to_lp_count += extra_byte_count; > + > + /* B088 */ > + /* LP -> HS for clock lanes > + * LP clk sync + LP11 + LP01 + tclk_prepare + tclk_zero + > + * extra byte count > + * 2TPLX + 1TLPX + 1 TPLX(in ns) + prepare_cnt * 2 + clk_zero_cnt * > + * 2(in UI) + extra byte count > + * In byteclks = (4TLPX + prepare_cnt * 2 + clk_zero_cnt *2 (in UI)) / > + * 8 + extra byte count > + */ > + intel_dsi->clk_lp_to_hs_count = > + CEIL_DIV( > + 4 * tlpx_ui + prepare_cnt * 2 + > + clk_zero_cnt * 2, > + 8); > + > + intel_dsi->clk_lp_to_hs_count += extra_byte_count; > + > + /* HS->LP for Clock Lanes > + * Low Power clock synchronisations + 1Tx byteclk + tclk_trail + > + * Extra byte count > + * 2TLPX + 8UI + (trail_count*2)(in UI) + Extra byte count > + * In byteclks = (2*TLpx(in UI) + trail_count*2 +8)(in UI)/8 + > + * Extra byte count > + */ > + intel_dsi->clk_hs_to_lp_count = > + CEIL_DIV(2 * tlpx_ui + trail_cnt * 2 + 8, > + 8); > + intel_dsi->clk_hs_to_lp_count += extra_byte_count; > + > + DRM_DEBUG_KMS("EOT %s\n", intel_dsi->eotp_pkt ? "ENABLED" : "DISABLED"); > + DRM_DEBUG_KMS("CLOCKSTOP %s\n", intel_dsi->clock_stop ? > + "ENABLED" : "DISABLED"); > + DRM_DEBUG_KMS("Mode %s\n", intel_dsi->operation_mode ? "COMMAND" : "VIDEO"); > + DRM_DEBUG_KMS("Pixel Format %d\n", intel_dsi->pixel_format); > + DRM_DEBUG_KMS("TLPX %d\n", intel_dsi->escape_clk_div); > + DRM_DEBUG_KMS("LP RX Timeout 0x%x\n", intel_dsi->lp_rx_timeout); > + DRM_DEBUG_KMS("Turnaround Timeout 0x%x\n", intel_dsi->turn_arnd_val); > + DRM_DEBUG_KMS("Init Count 0x%x\n", intel_dsi->init_count); > + DRM_DEBUG_KMS("HS to LP Count 0x%x\n", intel_dsi->hs_to_lp_count); > + DRM_DEBUG_KMS("LP Byte Clock %d\n", intel_dsi->lp_byte_clk); > + DRM_DEBUG_KMS("DBI BW Timer 0x%x\n", intel_dsi->bw_timer); > + DRM_DEBUG_KMS("LP to HS Clock Count 0x%x\n", intel_dsi->clk_lp_to_hs_count); > + DRM_DEBUG_KMS("HS to LP Clock Count 0x%x\n", intel_dsi->clk_hs_to_lp_count); > + DRM_DEBUG_KMS("BTA %s\n", > + intel_dsi->video_frmt_cfg_bits & DISABLE_VIDEO_BTA ? > + "DISABLED" : "ENABLED"); > + DRM_DEBUG_KMS("B060 = 0x%Xx, B080 = 0x%x, B044 = 0x%x, B088 = 0x%x\n", > + intel_dsi->lp_byte_clk, intel_dsi->dphy_reg, intel_dsi->hs_to_lp_count, > + (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)); > + > + /* delays in VBT are in unit of 100us, so need to convert > + * here in ms > + * Delay (100us) * 100 /1000 = Delay / 10 (ms) */ > + intel_dsi->backlight_off_delay = pps->bl_disable_delay / 10; > + intel_dsi->backlight_on_delay = pps->bl_enable_delay / 10; > + intel_dsi->panel_on_delay = pps->panel_on_delay / 10; > + intel_dsi->panel_off_delay = pps->panel_off_delay / 10; > + intel_dsi->panel_pwr_cycle_delay = pps->panel_power_cycle_delay / 10; > + > + return true; > +}
On Monday 19 May 2014 07:53 PM, Damien Lespiau wrote: > On Mon, Apr 14, 2014 at 11:18:27AM +0530, Shobhit Kumar wrote: >> +#define NS_MHZ_RATIO 1000000 > > [...] > >> +static bool generic_init(struct intel_dsi_device *dsi) >> +{ > > [...] > >> + /* >> + * ui(s) = 1/f [f in hz] >> + * ui(ns) = 10^9/f*10^6 [f in Mhz] -> 10^3/f(Mhz) > > ui(ns) = 10^9/(f*10^6) > >> + * >> + * LP byte clock = TLPX/8ui > > Mind putting that comment just above the appropriate computation? > Also, LP byte clock = Tlpx / (8UI) > >> + * >> + * Since txddrclkhs_i is 2xUI, the count values programmed in >> + * DPHY param registers are divided by 2 > > That looks like a general comment that apply to a bunch of calculations > below, probably worth separating it from the UI comment. > Will update as suggested. >> + * >> + */ >> + >> + /* in Kbps */ >> + ui_num = bitrate; >> + ui_den = NS_MHZ_RATIO; > > I'm a bit confused here, most likely missing something, care to clarify? > > - IIUC, you want the computations to happen in ns. I'm a bit puzzled by > that NS_MHZ_RATIO constant name when we're dealing with Kbps. > > That constant is 10^6 which seems to be OK for KHz. So maybe just a > naming problem? Yeah, I now realize that it should be something like NS_KHZ_RATIO to avoid confusion > > - UI is a period, so is homogeneous to time (s), but ui_num being in > s^-1 and ui_den a constant, ui_num/ui_den looks like a frequency. Or > could it be that UI = ui_den / ui_num? would be confusing, but the > code below would make more sense. In which case could we have UI = > ui_num / ui_den? I just kept ui_num and ui_den separately to take care of precision loss, but I see how it is adding to confusion. Actually it is ui_den / ui_num and we have all computations as 1/UI so it works. I think I will compute UI directly as UI = (NS_KHZ_RATIO * 1000) /bitrate and divide by 1000 wherever we use to maintain precision. Sounds ok ? > >> + >> + tclk_prepare_clkzero = mipi_config->tclk_prepare_clkzero; >> + ths_prepare_hszero = mipi_config->ths_prepare_hszero; >> + >> + /* B060 */ >> + intel_dsi->lp_byte_clk = CEIL_DIV(tlpx_ns * ui_num, 8 * ui_den); >> + >> + /* count values in UI = (ns value) * (bitrate / (2 * 10^6)) */ >> + /* prepare count */ >> + ths_prepare_ns = >> + (mipi_config->ths_prepare > mipi_config->tclk_prepare) ? >> + mipi_config->ths_prepare : >> + mipi_config->tclk_prepare; > > That looks like max() > >> + >> + prepare_cnt = CEIL_DIV(ths_prepare_ns * ui_num, ui_den * 2); > > The formula above has a 10^6, why is that OK not to have it there? (in > which unit is bitrate in the formula? MHz?) Is this something like: > > Count in UI = count(ns) / UI(ns) > > and then as UI = ui_den/ui_num (?!) we end up with: > > Count in UI = count(ns) * ui_num / ui_den > > And then the / 2 comment applies. Yeah actually its like this. I will correct as suggested above. > >> + >> + /* exit zero count */ >> + exit_zero_cnt = CEIL_DIV( >> + (ths_prepare_hszero - ths_prepare_ns) * ui_num, >> + ui_den * 2 >> + ); >> + >> + /* >> + * Exit zero is unified val ths_zero and ths_exit >> + * minimum value for ths_exit = 110ns >> + * min (exit_zero_cnt * 2) = 110/UI >> + * exit_zero_cnt = 55/UI >> + */ >> + if (exit_zero_cnt < (55 * ui_num / ui_den)) >> + if ((55 * ui_num) % ui_den) >> + exit_zero_cnt += 1; > > I'm not sure what we're achieving with the +=1 here, mind explaining? This is as per MIPI host controller spec to ceil the value > >> + >> + /* clk zero count */ >> + clk_zero_cnt = CEIL_DIV( >> + (tclk_prepare_clkzero - ths_prepare_ns) >> + * ui_num, 2 * ui_den); >> + >> + /* trail count */ >> + tclk_trail_ns = (mipi_config->tclk_trail > mipi_config->ths_trail) ? >> + mipi_config->tclk_trail : mipi_config->ths_trail; > > max() > >> + trail_cnt = CEIL_DIV(tclk_trail_ns * ui_num, 2 * ui_den); >> + >> + if (prepare_cnt > PREPARE_CNT_MAX || >> + exit_zero_cnt > EXIT_ZERO_CNT_MAX || >> + clk_zero_cnt > CLK_ZERO_CNT_MAX || >> + trail_cnt > TRAIL_CNT_MAX) >> + DRM_DEBUG_DRIVER("Values crossing maximum limits\n"); > > Is that a situation that may happen in a normal case? or should we go > with a DRM_ERROR() here and potentially abort the modeset? > Generally it should not happen. We should not abort but clip to max values though there is no guarantee it will work, but there is high chance that it will work. >> + >> + if (prepare_cnt > PREPARE_CNT_MAX) >> + prepare_cnt = PREPARE_CNT_MAX; >> + >> + if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) >> + exit_zero_cnt = EXIT_ZERO_CNT_MAX; >> + >> + if (clk_zero_cnt > CLK_ZERO_CNT_MAX) >> + clk_zero_cnt = CLK_ZERO_CNT_MAX; >> + >> + if (trail_cnt > TRAIL_CNT_MAX) >> + trail_cnt = TRAIL_CNT_MAX; >> + >> + /* B080 */ >> + intel_dsi->dphy_reg = exit_zero_cnt << 24 | trail_cnt << 16 | >> + clk_zero_cnt << 8 | prepare_cnt; >> + >> + /* >> + * LP to HS switch count = 4TLPX + PREP_COUNT * 2 + EXIT_ZERO_COUNT * 2 >> + * + 10UI + Extra Byte Count >> + * >> + * HS to LP switch count = THS-TRAIL + 2TLPX + Extra Byte Count >> + * Extra Byte Count is calculated according to number of lanes. >> + * High Low Switch Count is the Max of LP to HS and >> + * HS to LP switch count >> + * >> + */ >> + tlpx_ui = CEIL_DIV(tlpx_ns * ui_num, ui_den); >> + >> + /* B044 */ >> + intel_dsi->hs_to_lp_count = >> + CEIL_DIV( >> + 4 * tlpx_ui + prepare_cnt * 2 + >> + exit_zero_cnt * 2 + 10, >> + 8); > > The previous was before I tried to look at the spec too closely. Mind > explaining why we don't look at the HS to LP switch count? ie why HS to > LP switch cound is always smaller than the LP to HS one? Because LP to HS uses exit_zero_count which is generally higher than clk_zero_count. So just directly used LP to HS which amounts to saying that switching from HS to LP takes lesser time than switching from LP to HS. I can/should add code to compute max of the two. > >> + >> + intel_dsi->hs_to_lp_count += extra_byte_count; >> + >> + /* B088 */ >> + /* LP -> HS for clock lanes >> + * LP clk sync + LP11 + LP01 + tclk_prepare + tclk_zero + >> + * extra byte count >> + * 2TPLX + 1TLPX + 1 TPLX(in ns) + prepare_cnt * 2 + clk_zero_cnt * >> + * 2(in UI) + extra byte count >> + * In byteclks = (4TLPX + prepare_cnt * 2 + clk_zero_cnt *2 (in UI)) / >> + * 8 + extra byte count >> + */ >> + intel_dsi->clk_lp_to_hs_count = >> + CEIL_DIV( >> + 4 * tlpx_ui + prepare_cnt * 2 + >> + clk_zero_cnt * 2, >> + 8); >> + >> + intel_dsi->clk_lp_to_hs_count += extra_byte_count; >> + >> + /* HS->LP for Clock Lanes >> + * Low Power clock synchronisations + 1Tx byteclk + tclk_trail + >> + * Extra byte count >> + * 2TLPX + 8UI + (trail_count*2)(in UI) + Extra byte count >> + * In byteclks = (2*TLpx(in UI) + trail_count*2 +8)(in UI)/8 + >> + * Extra byte count >> + */ >> + intel_dsi->clk_hs_to_lp_count = >> + CEIL_DIV(2 * tlpx_ui + trail_cnt * 2 + 8, >> + 8); >> + intel_dsi->clk_hs_to_lp_count += extra_byte_count; >> + >> + DRM_DEBUG_KMS("EOT %s\n", intel_dsi->eotp_pkt ? "ENABLED" : "DISABLED"); >> + DRM_DEBUG_KMS("CLOCKSTOP %s\n", intel_dsi->clock_stop ? >> + "ENABLED" : "DISABLED"); >> + DRM_DEBUG_KMS("Mode %s\n", intel_dsi->operation_mode ? "COMMAND" : "VIDEO"); >> + DRM_DEBUG_KMS("Pixel Format %d\n", intel_dsi->pixel_format); >> + DRM_DEBUG_KMS("TLPX %d\n", intel_dsi->escape_clk_div); >> + DRM_DEBUG_KMS("LP RX Timeout 0x%x\n", intel_dsi->lp_rx_timeout); >> + DRM_DEBUG_KMS("Turnaround Timeout 0x%x\n", intel_dsi->turn_arnd_val); >> + DRM_DEBUG_KMS("Init Count 0x%x\n", intel_dsi->init_count); >> + DRM_DEBUG_KMS("HS to LP Count 0x%x\n", intel_dsi->hs_to_lp_count); >> + DRM_DEBUG_KMS("LP Byte Clock %d\n", intel_dsi->lp_byte_clk); >> + DRM_DEBUG_KMS("DBI BW Timer 0x%x\n", intel_dsi->bw_timer); >> + DRM_DEBUG_KMS("LP to HS Clock Count 0x%x\n", intel_dsi->clk_lp_to_hs_count); >> + DRM_DEBUG_KMS("HS to LP Clock Count 0x%x\n", intel_dsi->clk_hs_to_lp_count); >> + DRM_DEBUG_KMS("BTA %s\n", >> + intel_dsi->video_frmt_cfg_bits & DISABLE_VIDEO_BTA ? >> + "DISABLED" : "ENABLED"); >> + DRM_DEBUG_KMS("B060 = 0x%Xx, B080 = 0x%x, B044 = 0x%x, B088 = 0x%x\n", >> + intel_dsi->lp_byte_clk, intel_dsi->dphy_reg, intel_dsi->hs_to_lp_count, >> + (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)); >> + >> + /* delays in VBT are in unit of 100us, so need to convert >> + * here in ms >> + * Delay (100us) * 100 /1000 = Delay / 10 (ms) */ >> + intel_dsi->backlight_off_delay = pps->bl_disable_delay / 10; >> + intel_dsi->backlight_on_delay = pps->bl_enable_delay / 10; >> + intel_dsi->panel_on_delay = pps->panel_on_delay / 10; >> + intel_dsi->panel_off_delay = pps->panel_off_delay / 10; >> + intel_dsi->panel_pwr_cycle_delay = pps->panel_power_cycle_delay / 10; >> + >> + return true; >> +}
On Tue, May 20, 2014 at 09:46:01PM +0530, Shobhit Kumar wrote: > >- UI is a period, so is homogeneous to time (s), but ui_num being in > > s^-1 and ui_den a constant, ui_num/ui_den looks like a frequency. Or > > could it be that UI = ui_den / ui_num? would be confusing, but the > > code below would make more sense. In which case could we have UI = > > ui_num / ui_den? > > I just kept ui_num and ui_den separately to take care of precision > loss, but I see how it is adding to confusion. Actually it is ui_den > / ui_num and we have all computations as 1/UI so it works. I think I > will compute UI directly as UI = (NS_KHZ_RATIO * 1000) /bitrate and > divide by 1000 wherever we use to maintain precision. Sounds ok ? I think just exchanging the two variable names (ui_num and ui_den) should be less work for you and should be enough. It's really just about having ui_num being the UI numerator so the reader is not too surprised > >>+ /* B044 */ > >>+ intel_dsi->hs_to_lp_count = > >>+ CEIL_DIV( > >>+ 4 * tlpx_ui + prepare_cnt * 2 + > >>+ exit_zero_cnt * 2 + 10, > >>+ 8); > > > >The previous was before I tried to look at the spec too closely. Mind > >explaining why we don't look at the HS to LP switch count? ie why HS to > >LP switch cound is always smaller than the LP to HS one? > > Because LP to HS uses exit_zero_count which is generally higher than > clk_zero_count. So just directly used LP to HS which amounts to > saying that switching from HS to LP takes lesser time than switching > from LP to HS. I can/should add code to compute max of the two. This could go to a separate task if you don't have time right now, Thanks for your answers!
On 5/21/2014 2:25 AM, Damien Lespiau wrote: > On Tue, May 20, 2014 at 09:46:01PM +0530, Shobhit Kumar wrote: >>> - UI is a period, so is homogeneous to time (s), but ui_num being in >>> s^-1 and ui_den a constant, ui_num/ui_den looks like a frequency. Or >>> could it be that UI = ui_den / ui_num? would be confusing, but the >>> code below would make more sense. In which case could we have UI = >>> ui_num / ui_den? >> >> I just kept ui_num and ui_den separately to take care of precision >> loss, but I see how it is adding to confusion. Actually it is ui_den >> / ui_num and we have all computations as 1/UI so it works. I think I >> will compute UI directly as UI = (NS_KHZ_RATIO * 1000) /bitrate and >> divide by 1000 wherever we use to maintain precision. Sounds ok ? > > I think just exchanging the two variable names (ui_num and ui_den) > should be less work for you and should be enough. It's really just about > having ui_num being the UI numerator so the reader is not too surprised Yeah. Will fix this > >>>> + /* B044 */ >>>> + intel_dsi->hs_to_lp_count = >>>> + CEIL_DIV( >>>> + 4 * tlpx_ui + prepare_cnt * 2 + >>>> + exit_zero_cnt * 2 + 10, >>>> + 8); >>> >>> The previous was before I tried to look at the spec too closely. Mind >>> explaining why we don't look at the HS to LP switch count? ie why HS to >>> LP switch cound is always smaller than the LP to HS one? >> >> Because LP to HS uses exit_zero_count which is generally higher than >> clk_zero_count. So just directly used LP to HS which amounts to >> saying that switching from HS to LP takes lesser time than switching >> from LP to HS. I can/should add code to compute max of the two. > > This could go to a separate task if you don't have time right now, > Most likely I can do this as well. Will push the updated patch by sometime tomorrow. Regards Shobhit
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index b1445b7..756a7a4 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -55,6 +55,7 @@ i915-y += dvo_ch7017.o \ intel_dsi_cmd.o \ intel_dsi.o \ intel_dsi_pll.o \ + dsi_mod_vbt_generic.o \ intel_dvo.o \ intel_hdmi.o \ intel_i2c.o \ diff --git a/drivers/gpu/drm/i915/dsi_mod_vbt_generic.c b/drivers/gpu/drm/i915/dsi_mod_vbt_generic.c new file mode 100644 index 0000000..0c12ce8 --- /dev/null +++ b/drivers/gpu/drm/i915/dsi_mod_vbt_generic.c @@ -0,0 +1,598 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Author: Shobhit Kumar <shobhit.kumar@intel.com> + * + */ + +#include <drm/drmP.h> +#include <drm/drm_crtc.h> +#include <drm/drm_edid.h> +#include <drm/i915_drm.h> +#include <linux/slab.h> +#include <video/mipi_display.h> +#include <asm/intel-mid.h> +#include <video/mipi_display.h> +#include "i915_drv.h" +#include "intel_drv.h" +#include "intel_dsi.h" +#include "intel_dsi_cmd.h" + +#define MIPI_TRANSFER_MODE_SHIFT 0 +#define MIPI_VIRTUAL_CHANNEL_SHIFT 1 +#define MIPI_PORT_SHIFT 3 + +#define PREPARE_CNT_MAX 0x3F +#define EXIT_ZERO_CNT_MAX 0x3F +#define CLK_ZERO_CNT_MAX 0xFF +#define TRAIL_CNT_MAX 0x1F + +#define NS_MHZ_RATIO 1000000 + +/* This macro divides two integers and rounds fractional values up + * to the nearest integer value. */ +#define CEIL_DIV(X, Y) (((X)%(Y)) ? ((X)/(Y)+1) : ((X)/(Y))) + +#define GPI0_NC_0_HV_DDI0_HPD 0x4130 +#define GPIO_NC_0_HV_DDI0_PAD 0x4138 +#define GPIO_NC_1_HV_DDI0_DDC_SDA 0x4120 +#define GPIO_NC_1_HV_DDI0_DDC_SDA_PAD 0x4128 +#define GPIO_NC_2_HV_DDI0_DDC_SCL 0x4110 +#define GPIO_NC_2_HV_DDI0_DDC_SCL_PAD 0x4118 +#define GPIO_NC_3_PANEL0_VDDEN 0x4140 +#define GPIO_NC_3_PANEL0_VDDEN_PAD 0x4148 +#define GPIO_NC_4_PANEL0_BLKEN 0x4150 +#define GPIO_NC_4_PANEL0_BLKEN_PAD 0x4158 +#define GPIO_NC_5_PANEL0_BLKCTL 0x4160 +#define GPIO_NC_5_PANEL0_BLKCTL_PAD 0x4168 +#define GPIO_NC_6_PCONF0 0x4180 +#define GPIO_NC_6_PAD 0x4188 +#define GPIO_NC_7_PCONF0 0x4190 +#define GPIO_NC_7_PAD 0x4198 +#define GPIO_NC_8_PCONF0 0x4170 +#define GPIO_NC_8_PAD 0x4178 +#define GPIO_NC_9_PCONF0 0x4100 +#define GPIO_NC_9_PAD 0x4108 +#define GPIO_NC_10_PCONF0 0x40E0 +#define GPIO_NC_10_PAD 0x40E8 +#define GPIO_NC_11_PCONF0 0x40F0 +#define GPIO_NC_11_PAD 0x40F8 + +struct gpio_table { + u16 function_reg; + u16 pad_reg; + u8 init; +}; + +static struct gpio_table gtable[] = { + { GPI0_NC_0_HV_DDI0_HPD, GPIO_NC_0_HV_DDI0_PAD, 0 }, + { GPIO_NC_1_HV_DDI0_DDC_SDA, GPIO_NC_1_HV_DDI0_DDC_SDA_PAD, 0 }, + { GPIO_NC_2_HV_DDI0_DDC_SCL, GPIO_NC_2_HV_DDI0_DDC_SCL_PAD, 0 }, + { GPIO_NC_3_PANEL0_VDDEN, GPIO_NC_3_PANEL0_VDDEN_PAD, 0 }, + { GPIO_NC_4_PANEL0_BLKEN, GPIO_NC_4_PANEL0_BLKEN_PAD, 0 }, + { GPIO_NC_5_PANEL0_BLKCTL, GPIO_NC_5_PANEL0_BLKCTL_PAD, 0 }, + { GPIO_NC_6_PCONF0, GPIO_NC_6_PAD, 0 }, + { GPIO_NC_7_PCONF0, GPIO_NC_7_PAD, 0 }, + { GPIO_NC_8_PCONF0, GPIO_NC_8_PAD, 0 }, + { GPIO_NC_9_PCONF0, GPIO_NC_9_PAD, 0 }, + { GPIO_NC_10_PCONF0, GPIO_NC_10_PAD, 0}, + { GPIO_NC_11_PCONF0, GPIO_NC_11_PAD, 0} +}; + +static u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, u8 *data) +{ + u8 type, byte, mode, vc, port; + u16 len; + + byte = *data++; + mode = (byte >> MIPI_TRANSFER_MODE_SHIFT) & 0x1; + vc = (byte >> MIPI_VIRTUAL_CHANNEL_SHIFT) & 0x3; + port = (byte >> MIPI_PORT_SHIFT) & 0x3; + + /* LP or HS mode */ + intel_dsi->hs = mode; + + /* get packet type and increment the pointer */ + type = *data++; + + len = *((u16 *) data); + data += 2; + + switch (type) { + case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM: + dsi_vc_generic_write_0(intel_dsi, vc); + break; + case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM: + dsi_vc_generic_write_1(intel_dsi, vc, *data); + break; + case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM: + dsi_vc_generic_write_2(intel_dsi, vc, *data, *(data + 1)); + break; + case MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM: + case MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM: + case MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM: + DRM_DEBUG_DRIVER("Generic Read not yet implemented or used\n"); + break; + case MIPI_DSI_GENERIC_LONG_WRITE: + dsi_vc_generic_write(intel_dsi, vc, data, len); + break; + case MIPI_DSI_DCS_SHORT_WRITE: + dsi_vc_dcs_write_0(intel_dsi, vc, *data); + break; + case MIPI_DSI_DCS_SHORT_WRITE_PARAM: + dsi_vc_dcs_write_1(intel_dsi, vc, *data, *(data + 1)); + break; + case MIPI_DSI_DCS_READ: + DRM_DEBUG_DRIVER("DCS Read not yet implemented or used\n"); + break; + case MIPI_DSI_DCS_LONG_WRITE: + dsi_vc_dcs_write(intel_dsi, vc, data, len); + break; + }; + + data += len; + + return data; +} + +static u8 *mipi_exec_delay(struct intel_dsi *intel_dsi, u8 *data) +{ + u32 delay = *((u32 *) data); + + DRM_DEBUG_DRIVER("MIPI: executing delay element\n"); + usleep_range(delay, delay + 10); + data += 4; + + return data; +} + +static u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, u8 *data) +{ + u8 gpio, action; + u16 function, pad; + u32 val; + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + DRM_DEBUG_DRIVER("MIPI: executing gpio element\n"); + gpio = *data++; + + /* pull up/down */ + action = *data++; + + function = gtable[gpio].function_reg; + pad = gtable[gpio].pad_reg; + + mutex_lock(&dev_priv->dpio_lock); + if (!gtable[gpio].init) { + /* program the function */ + vlv_gpio_nc_write(dev_priv, function, 0x2000CC00); + gtable[gpio].init = 1; + } + + val = 0x4 | action; + + /* pull up/down */ + vlv_gpio_nc_write(dev_priv, pad, val); + mutex_unlock(&dev_priv->dpio_lock); + + return data; +} + +typedef u8 * (*fn_mipi_elem_exec)(struct intel_dsi *intel_dsi, u8 *data); +const fn_mipi_elem_exec exec_elem[] = { + NULL, /* reserved */ + mipi_exec_send_packet, + mipi_exec_delay, + mipi_exec_gpio, + NULL, /* status read; later */ +}; + +/* + * MIPI Sequence from VBT #53 parsing logic + * We have already separated each seqence during bios parsing + * Following is generic execution function for any sequence + */ + +static const char *seq_name[] = { + "UNDEFINED", + "MIPI_SEQ_ASSERT_RESET", + "MIPI_SEQ_INIT_OTP", + "MIPI_SEQ_DISPLAY_ON", + "MIPI_SEQ_DISPLAY_OFF", + "MIPI_SEQ_DEASSERT_RESET" +}; + +static void generic_exec_sequence(struct intel_dsi *intel_dsi, char *sequence) +{ + u8 *data = sequence; + fn_mipi_elem_exec mipi_elem_exec; + int index; + + if (!sequence) + return; + + DRM_DEBUG_DRIVER("Starting MIPI sequence - %s\n", seq_name[*data]); + + /* go to the first element of the sequence */ + data++; + + /* parse each byte till we reach end of sequence byte - 0x00 */ + while (1) { + index = *data; + mipi_elem_exec = exec_elem[index]; + if (!mipi_elem_exec) { + DRM_ERROR("Unsupported MIPI element, skipping sequence execution\n"); + return; + } + + /* goto element payload */ + data++; + + /* execute the element specifc rotines */ + data = mipi_elem_exec(intel_dsi, data); + + /* + * After processing the element, data should point to + * next element or end of sequence + * check if have we reached end of sequence + */ + if (*data == 0x00) + break; + } +} + +static bool generic_init(struct intel_dsi_device *dsi) +{ + struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev); + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config; + struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps; + struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode; + u32 bits_per_pixel = 24; + u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui; + u32 ui_num, ui_den; + u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt; + u32 ths_prepare_ns, tclk_trail_ns; + u32 tclk_prepare_clkzero, ths_prepare_hszero; + + DRM_DEBUG_KMS("\n"); + + intel_dsi->eotp_pkt = mipi_config->eot_pkt_disabled ? 0 : 1; + intel_dsi->clock_stop = mipi_config->enable_clk_stop ? 1 : 0; + intel_dsi->lane_count = mipi_config->lane_cnt + 1; + intel_dsi->pixel_format = mipi_config->videomode_color_format << 7; + + if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666) + bits_per_pixel = 18; + else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565) + bits_per_pixel = 16; + + bitrate = (mode->clock * bits_per_pixel) / intel_dsi->lane_count; + + intel_dsi->operation_mode = mipi_config->is_cmd_mode; + intel_dsi->video_mode_format = mipi_config->video_transfer_mode; + intel_dsi->escape_clk_div = mipi_config->byte_clk_sel; + intel_dsi->lp_rx_timeout = mipi_config->lp_rx_timeout; + intel_dsi->turn_arnd_val = mipi_config->turn_around_timeout; + intel_dsi->rst_timer_val = mipi_config->device_reset_timer; + intel_dsi->init_count = mipi_config->master_init_timer; + intel_dsi->bw_timer = mipi_config->dbi_bw_timer; + intel_dsi->video_frmt_cfg_bits = mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0; + + switch (intel_dsi->escape_clk_div) { + case 0: + tlpx_ns = 50; + break; + case 1: + tlpx_ns = 100; + break; + + case 2: + tlpx_ns = 200; + break; + default: + tlpx_ns = 50; + break; + } + + switch (intel_dsi->lane_count) { + case 1: + case 2: + extra_byte_count = 2; + break; + case 3: + extra_byte_count = 4; + break; + case 4: + default: + extra_byte_count = 3; + break; + } + + /* + * ui(s) = 1/f [f in hz] + * ui(ns) = 10^9/f*10^6 [f in Mhz] -> 10^3/f(Mhz) + * + * LP byte clock = TLPX/8ui + * + * Since txddrclkhs_i is 2xUI, the count values programmed in + * DPHY param register are divided by 2 + * + */ + + /* in Kbps */ + ui_num = bitrate; + ui_den = NS_MHZ_RATIO; + + tclk_prepare_clkzero = mipi_config->tclk_prepare_clkzero; + ths_prepare_hszero = mipi_config->ths_prepare_hszero; + + /* B060 */ + intel_dsi->lp_byte_clk = CEIL_DIV(tlpx_ns * ui_num, 8 * ui_den); + + /* count values in UI = (ns value) * (bitrate / (2 * 10^6)) */ + /* prepare count */ + ths_prepare_ns = + (mipi_config->ths_prepare > mipi_config->tclk_prepare) ? + mipi_config->ths_prepare : + mipi_config->tclk_prepare; + + prepare_cnt = CEIL_DIV(ths_prepare_ns * ui_num, ui_den * 2); + + /* exit zero count */ + exit_zero_cnt = CEIL_DIV( + (ths_prepare_hszero - ths_prepare_ns) * ui_num, + ui_den * 2 + ); + + /* + * Exit zero is unified val ths_zero and ths_exit + * minimum value for ths_exit = 110ns + * min (exit_zero_cnt * 2) = 110/UI + * exit_zero_cnt = 55/UI + */ + if (exit_zero_cnt < (55 * ui_num / ui_den)) + if ((55 * ui_num) % ui_den) + exit_zero_cnt += 1; + + /* clk zero count */ + clk_zero_cnt = CEIL_DIV( + (tclk_prepare_clkzero - ths_prepare_ns) + * ui_num, 2 * ui_den); + + /* trail count */ + tclk_trail_ns = (mipi_config->tclk_trail > mipi_config->ths_trail) ? + mipi_config->tclk_trail : mipi_config->ths_trail; + trail_cnt = CEIL_DIV(tclk_trail_ns * ui_num, 2 * ui_den); + + if (prepare_cnt > PREPARE_CNT_MAX || + exit_zero_cnt > EXIT_ZERO_CNT_MAX || + clk_zero_cnt > CLK_ZERO_CNT_MAX || + trail_cnt > TRAIL_CNT_MAX) + DRM_DEBUG_DRIVER("Values crossing maximum limits\n"); + + if (prepare_cnt > PREPARE_CNT_MAX) + prepare_cnt = PREPARE_CNT_MAX; + + if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) + exit_zero_cnt = EXIT_ZERO_CNT_MAX; + + if (clk_zero_cnt > CLK_ZERO_CNT_MAX) + clk_zero_cnt = CLK_ZERO_CNT_MAX; + + if (trail_cnt > TRAIL_CNT_MAX) + trail_cnt = TRAIL_CNT_MAX; + + /* B080 */ + intel_dsi->dphy_reg = exit_zero_cnt << 24 | trail_cnt << 16 | + clk_zero_cnt << 8 | prepare_cnt; + + /* + * LP to HS switch count = 4TLPX + PREP_COUNT * 2 + EXIT_ZERO_COUNT * 2 + * + 10UI + Extra Byte Count + * + * HS to LP switch count = THS-TRAIL + 2TLPX + Extra Byte Count + * Extra Byte Count is calculated according to number of lanes. + * High Low Switch Count is the Max of LP to HS and + * HS to LP switch count + * + */ + tlpx_ui = CEIL_DIV(tlpx_ns * ui_num, ui_den); + + /* B044 */ + intel_dsi->hs_to_lp_count = + CEIL_DIV( + 4 * tlpx_ui + prepare_cnt * 2 + + exit_zero_cnt * 2 + 10, + 8); + + intel_dsi->hs_to_lp_count += extra_byte_count; + + /* B088 */ + /* LP -> HS for clock lanes + * LP clk sync + LP11 + LP01 + tclk_prepare + tclk_zero + + * extra byte count + * 2TPLX + 1TLPX + 1 TPLX(in ns) + prepare_cnt * 2 + clk_zero_cnt * + * 2(in UI) + extra byte count + * In byteclks = (4TLPX + prepare_cnt * 2 + clk_zero_cnt *2 (in UI)) / + * 8 + extra byte count + */ + intel_dsi->clk_lp_to_hs_count = + CEIL_DIV( + 4 * tlpx_ui + prepare_cnt * 2 + + clk_zero_cnt * 2, + 8); + + intel_dsi->clk_lp_to_hs_count += extra_byte_count; + + /* HS->LP for Clock Lanes + * Low Power clock synchronisations + 1Tx byteclk + tclk_trail + + * Extra byte count + * 2TLPX + 8UI + (trail_count*2)(in UI) + Extra byte count + * In byteclks = (2*TLpx(in UI) + trail_count*2 +8)(in UI)/8 + + * Extra byte count + */ + intel_dsi->clk_hs_to_lp_count = + CEIL_DIV(2 * tlpx_ui + trail_cnt * 2 + 8, + 8); + intel_dsi->clk_hs_to_lp_count += extra_byte_count; + + DRM_DEBUG_KMS("EOT %s\n", intel_dsi->eotp_pkt ? "ENABLED" : "DISABLED"); + DRM_DEBUG_KMS("CLOCKSTOP %s\n", intel_dsi->clock_stop ? + "ENABLED" : "DISABLED"); + DRM_DEBUG_KMS("Mode %s\n", intel_dsi->operation_mode ? "COMMAND" : "VIDEO"); + DRM_DEBUG_KMS("Pixel Format %d\n", intel_dsi->pixel_format); + DRM_DEBUG_KMS("TLPX %d\n", intel_dsi->escape_clk_div); + DRM_DEBUG_KMS("LP RX Timeout 0x%x\n", intel_dsi->lp_rx_timeout); + DRM_DEBUG_KMS("Turnaround Timeout 0x%x\n", intel_dsi->turn_arnd_val); + DRM_DEBUG_KMS("Init Count 0x%x\n", intel_dsi->init_count); + DRM_DEBUG_KMS("HS to LP Count 0x%x\n", intel_dsi->hs_to_lp_count); + DRM_DEBUG_KMS("LP Byte Clock %d\n", intel_dsi->lp_byte_clk); + DRM_DEBUG_KMS("DBI BW Timer 0x%x\n", intel_dsi->bw_timer); + DRM_DEBUG_KMS("LP to HS Clock Count 0x%x\n", intel_dsi->clk_lp_to_hs_count); + DRM_DEBUG_KMS("HS to LP Clock Count 0x%x\n", intel_dsi->clk_hs_to_lp_count); + DRM_DEBUG_KMS("BTA %s\n", + intel_dsi->video_frmt_cfg_bits & DISABLE_VIDEO_BTA ? + "DISABLED" : "ENABLED"); + DRM_DEBUG_KMS("B060 = 0x%Xx, B080 = 0x%x, B044 = 0x%x, B088 = 0x%x\n", + intel_dsi->lp_byte_clk, intel_dsi->dphy_reg, intel_dsi->hs_to_lp_count, + (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)); + + /* delays in VBT are in unit of 100us, so need to convert + * here in ms + * Delay (100us) * 100 /1000 = Delay / 10 (ms) */ + intel_dsi->backlight_off_delay = pps->bl_disable_delay / 10; + intel_dsi->backlight_on_delay = pps->bl_enable_delay / 10; + intel_dsi->panel_on_delay = pps->panel_on_delay / 10; + intel_dsi->panel_off_delay = pps->panel_off_delay / 10; + intel_dsi->panel_pwr_cycle_delay = pps->panel_power_cycle_delay / 10; + + return true; +} + +static int generic_mode_valid(struct intel_dsi_device *dsi, + struct drm_display_mode *mode) +{ + return MODE_OK; +} + +static bool generic_mode_fixup(struct intel_dsi_device *dsi, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { + return true; +} + +static void generic_panel_reset(struct intel_dsi_device *dsi) +{ + struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev); + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET]; + + generic_exec_sequence(intel_dsi, sequence); +} + +static void generic_disable_panel_power(struct intel_dsi_device *dsi) +{ + struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev); + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET]; + + generic_exec_sequence(intel_dsi, sequence); +} + +static void generic_send_otp_cmds(struct intel_dsi_device *dsi) +{ + struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev); + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP]; + + generic_exec_sequence(intel_dsi, sequence); +} + +static void generic_enable(struct intel_dsi_device *dsi) +{ + struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev); + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_DISPLAY_ON]; + + generic_exec_sequence(intel_dsi, sequence); +} + +static void generic_disable(struct intel_dsi_device *dsi) +{ + struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev); + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + char *sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_DISPLAY_OFF]; + + generic_exec_sequence(intel_dsi, sequence); +} + +static enum drm_connector_status generic_detect(struct intel_dsi_device *dsi) +{ + return connector_status_connected; +} + +static bool generic_get_hw_state(struct intel_dsi_device *dev) +{ + return true; +} + +static struct drm_display_mode *generic_get_modes(struct intel_dsi_device *dsi) +{ + struct intel_dsi *intel_dsi = container_of(dsi, struct intel_dsi, dev); + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + dev_priv->vbt.lfp_lvds_vbt_mode->type |= DRM_MODE_TYPE_PREFERRED; + return dev_priv->vbt.lfp_lvds_vbt_mode; +} + +static void generic_destroy(struct intel_dsi_device *dsi) { } + +/* Callbacks. We might not need them all. */ +struct intel_dsi_dev_ops vbt_generic_dsi_display_ops = { + .init = generic_init, + .mode_valid = generic_mode_valid, + .mode_fixup = generic_mode_fixup, + .panel_reset = generic_panel_reset, + .disable_panel_power = generic_disable_panel_power, + .send_otp_cmds = generic_send_otp_cmds, + .enable = generic_enable, + .disable = generic_disable, + .detect = generic_detect, + .get_hw_state = generic_get_hw_state, + .get_modes = generic_get_modes, + .destroy = generic_destroy, +}; diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 0d4dd54..7f1ddaa 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -35,6 +35,11 @@ /* the sub-encoders aka panel drivers */ static const struct intel_dsi_device intel_dsi_devices[] = { + { + .panel_id = MIPI_DSI_GENERIC_PANEL_ID, + .name = "vbt-generic-dsi-vid-mode-display", + .dev_ops = &vbt_generic_dsi_display_ops, + }, }; static void band_gap_reset(struct drm_i915_private *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h index e3f4e91..31db33d 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -133,4 +133,6 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder) extern void vlv_enable_dsi_pll(struct intel_encoder *encoder); extern void vlv_disable_dsi_pll(struct intel_encoder *encoder); +extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops; + #endif /* _INTEL_DSI_H */
This driver makes use of the generic panel information from the VBT. Panel information is classified into two - panel configuration and panel power sequence which is unique to each panel. The generic driver uses the panel configuration and sequence parsed from VBT block #52 and #53 v2: Address review comments by Jani - Move all of the things in driver c file from header - Make all functions static - Make use of video/mipi_display.c instead of redefining - Null checks during sequence execution Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/dsi_mod_vbt_generic.c | 598 +++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_dsi.c | 5 + drivers/gpu/drm/i915/intel_dsi.h | 2 + 4 files changed, 606 insertions(+) create mode 100644 drivers/gpu/drm/i915/dsi_mod_vbt_generic.c