* [PATCH 1/2] drm/i915/BXT: Retrieving the horizontal timing for DSI @ 2016-04-19 8:18 Ramalingam C 2016-04-19 8:18 ` [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval Ramalingam C 2016-04-27 16:20 ` [PATCH 1/2] drm/i915/BXT: Retrieving the horizontal timing for DSI Imre Deak 0 siblings, 2 replies; 7+ messages in thread From: Ramalingam C @ 2016-04-19 8:18 UTC (permalink / raw) To: intel-gfx, daniel, jani.nikula Retriving the horizontal timings from the port registers as part of get_config() Signed-off-by: Ramalingam C <ramalingam.c@intel.com> --- drivers/gpu/drm/i915/intel_dsi.c | 44 ++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 9ff6435..e0259e6 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -46,6 +46,14 @@ static const struct { }, }; +/* return pixels equvalent to txbyteclkhs */ +static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count, + u16 burst_mode_ratio) +{ + return DIV_ROUND_UP((clk_hs * lane_count * 8 * 100), + (bpp * burst_mode_ratio)); +} + enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt) { /* It just so happens the VBT matches register contents. */ @@ -772,9 +780,10 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); + unsigned int lane_count = intel_dsi->lane_count; unsigned int bpp, fmt; enum port port; - u16 vfp, vsync, vbp; + u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; /* * Atleast one port is active as encoder->get_config called only if @@ -799,22 +808,43 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, adjusted_mode->crtc_vtotal = I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); + hactive = adjusted_mode->crtc_hdisplay; + hfp = I915_READ(MIPI_HFP_COUNT(port)); + /* - * TODO: Retrieve hfp, hsync and hbp. Adjust them for dual link and - * calculate hsync_start, hsync_end, htotal and hblank_end + * Meaningful for video mode non-burst sync pulse mode only, + * can be zero for non-burst sync events and burst modes */ + hsync = I915_READ(MIPI_HSYNC_PADDING_COUNT(port)); + hbp = I915_READ(MIPI_HBP_COUNT(port)); + + /* harizontal values are in terms of high speed byte clock */ + hfp = pixels_from_txbyteclkhs(hfp, bpp, lane_count, + intel_dsi->burst_mode_ratio); + hsync = pixels_from_txbyteclkhs(hsync, bpp, lane_count, + intel_dsi->burst_mode_ratio); + hbp = pixels_from_txbyteclkhs(hbp, bpp, lane_count, + intel_dsi->burst_mode_ratio); + + if (intel_dsi->dual_link) { + hfp *= 2; + hsync *= 2; + hbp *= 2; + } /* vertical values are in terms of lines */ vfp = I915_READ(MIPI_VFP_COUNT(port)); vsync = I915_READ(MIPI_VSYNC_PADDING_COUNT(port)); vbp = I915_READ(MIPI_VBP_COUNT(port)); + adjusted_mode->crtc_htotal = hactive + hfp + hsync + hbp; + adjusted_mode->crtc_hsync_start = hfp + adjusted_mode->crtc_hdisplay; + adjusted_mode->crtc_hsync_end = hsync + adjusted_mode->crtc_hsync_start; adjusted_mode->crtc_hblank_start = adjusted_mode->crtc_hdisplay; + adjusted_mode->crtc_hblank_end = adjusted_mode->crtc_htotal; - adjusted_mode->crtc_vsync_start = - vfp + adjusted_mode->crtc_vdisplay; - adjusted_mode->crtc_vsync_end = - vsync + adjusted_mode->crtc_vsync_start; + adjusted_mode->crtc_vsync_start = vfp + adjusted_mode->crtc_vdisplay; + adjusted_mode->crtc_vsync_end = vsync + adjusted_mode->crtc_vsync_start; adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay; adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal; } -- 1.7.9.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval 2016-04-19 8:18 [PATCH 1/2] drm/i915/BXT: Retrieving the horizontal timing for DSI Ramalingam C @ 2016-04-19 8:18 ` Ramalingam C 2016-04-20 13:26 ` Daniel Vetter 2016-04-27 17:11 ` Ville Syrjälä 2016-04-27 16:20 ` [PATCH 1/2] drm/i915/BXT: Retrieving the horizontal timing for DSI Imre Deak 1 sibling, 2 replies; 7+ messages in thread From: Ramalingam C @ 2016-04-19 8:18 UTC (permalink / raw) To: intel-gfx, daniel, jani.nikula In BXT DSI there is no regs programmed with few horizontal timings in Pixels but txbyteclkhs.. So retrieval process adds some ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. Actually here for the given adjusted_mode, we are calculating the value programmed to the port and then back to the horizontal timing param in pixels. This is the expected value at the end of get_config, including roundup errors. And if that is same as retrieved value from port, then retrieved (HW state) adjusted_mode's horizontal timings are corrected to match with SW state to nullify the errors. Signed-off-by: Ramalingam C <ramalingam.c@intel.com> --- drivers/gpu/drm/i915/intel_dsi.c | 97 ++++++++++++++++++++++++++++++++++---- 1 file changed, 88 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index e0259e6..8a1b872 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -46,6 +46,14 @@ static const struct { }, }; +/* return pixels in terms of txbyteclkhs */ +static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, + u16 burst_mode_ratio) +{ + return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, + 8 * 100), lane_count); +} + /* return pixels equvalent to txbyteclkhs */ static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count, u16 burst_mode_ratio) @@ -779,11 +787,19 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; + struct drm_display_mode *adjusted_mode_sw; + struct intel_crtc *intel_crtc; struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); unsigned int lane_count = intel_dsi->lane_count; unsigned int bpp, fmt; enum port port; u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; + u16 hfp_sw, hsync_sw, hbp_sw; + u16 crtc_htotal_sw, crtc_hsync_start_sw, crtc_hsync_end_sw, + crtc_hblank_start_sw, crtc_hblank_end_sw; + + intel_crtc = to_intel_crtc(encoder->base.crtc); + adjusted_mode_sw = &intel_crtc->config->base.adjusted_mode; /* * Atleast one port is active as encoder->get_config called only if @@ -847,8 +863,79 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, adjusted_mode->crtc_vsync_end = vsync + adjusted_mode->crtc_vsync_start; adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay; adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal; -} + /* + * In BXT DSI there is no regs programmed with few horizontal timings + * in Pixels but txbyteclkhs.. So retrieval process adds some + * ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. + * Actually here for the given adjusted_mode, we are calculating the + * value programmed to the port and then back to the horizontal timing + * param in pixels. This is the expected value, including roundup errors + * And if that is same as retrieved value from port, then + * (HW state) adjusted_mode's horizontal timings are corrected to + * match with SW state to nullify the errors. + */ + /* Calculating the value programmed to the Port register */ + hfp_sw = adjusted_mode_sw->crtc_hsync_start - + adjusted_mode_sw->crtc_hdisplay; + hsync_sw = adjusted_mode_sw->crtc_hsync_end - + adjusted_mode_sw->crtc_hsync_start; + hbp_sw = adjusted_mode_sw->crtc_htotal - + adjusted_mode_sw->crtc_hsync_end; + + if (intel_dsi->dual_link) { + hfp_sw /= 2; + hsync_sw /= 2; + hbp_sw /= 2; + } + + hfp_sw = txbyteclkhs(hfp_sw, bpp, lane_count, + intel_dsi->burst_mode_ratio); + hsync_sw = txbyteclkhs(hsync_sw, bpp, lane_count, + intel_dsi->burst_mode_ratio); + hbp_sw = txbyteclkhs(hbp_sw, bpp, lane_count, + intel_dsi->burst_mode_ratio); + + /* Reverse calculating the adjusted mode parameters from port reg vals*/ + hfp_sw = pixels_from_txbyteclkhs(hfp_sw, bpp, lane_count, + intel_dsi->burst_mode_ratio); + hsync_sw = pixels_from_txbyteclkhs(hsync_sw, bpp, lane_count, + intel_dsi->burst_mode_ratio); + hbp_sw = pixels_from_txbyteclkhs(hbp_sw, bpp, lane_count, + intel_dsi->burst_mode_ratio); + + if (intel_dsi->dual_link) { + hfp_sw *= 2; + hsync_sw *= 2; + hbp_sw *= 2; + } + + crtc_htotal_sw = adjusted_mode_sw->crtc_hdisplay + hfp_sw + + hsync_sw + hbp_sw; + crtc_hsync_start_sw = hfp_sw + adjusted_mode_sw->crtc_hdisplay; + crtc_hsync_end_sw = hsync_sw + crtc_hsync_start_sw; + crtc_hblank_start_sw = adjusted_mode_sw->crtc_hdisplay; + crtc_hblank_end_sw = crtc_htotal_sw; + + if (adjusted_mode->crtc_htotal == crtc_htotal_sw) + adjusted_mode->crtc_htotal = adjusted_mode_sw->crtc_htotal; + + if (adjusted_mode->crtc_hsync_start == crtc_hsync_start_sw) + adjusted_mode->crtc_hsync_start = + adjusted_mode_sw->crtc_hsync_start; + + if (adjusted_mode->crtc_hsync_end == crtc_hsync_end_sw) + adjusted_mode->crtc_hsync_end = + adjusted_mode_sw->crtc_hsync_end; + + if (adjusted_mode->crtc_hblank_start == crtc_hblank_start_sw) + adjusted_mode->crtc_hblank_start = + adjusted_mode_sw->crtc_hblank_start; + + if (adjusted_mode->crtc_hblank_end == crtc_hblank_end_sw) + adjusted_mode->crtc_hblank_end = + adjusted_mode_sw->crtc_hblank_end; +} static void intel_dsi_get_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config) @@ -917,14 +1004,6 @@ static u16 txclkesc(u32 divider, unsigned int us) } } -/* return pixels in terms of txbyteclkhs */ -static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, - u16 burst_mode_ratio) -{ - return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, - 8 * 100), lane_count); -} - static void set_dsi_timings(struct drm_encoder *encoder, const struct drm_display_mode *adjusted_mode) { -- 1.7.9.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval 2016-04-19 8:18 ` [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval Ramalingam C @ 2016-04-20 13:26 ` Daniel Vetter 2016-04-21 4:41 ` Ramalingam C 2016-04-27 17:11 ` Ville Syrjälä 1 sibling, 1 reply; 7+ messages in thread From: Daniel Vetter @ 2016-04-20 13:26 UTC (permalink / raw) To: Ramalingam C; +Cc: jani.nikula, intel-gfx On Tue, Apr 19, 2016 at 01:48:14PM +0530, Ramalingam C wrote: > In BXT DSI there is no regs programmed with few horizontal timings > in Pixels but txbyteclkhs.. So retrieval process adds some > ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. > > Actually here for the given adjusted_mode, we are calculating the > value programmed to the port and then back to the horizontal timing > param in pixels. This is the expected value at the end of get_config, > including roundup errors. And if that is same as retrieved value > from port, then retrieved (HW state) adjusted_mode's horizontal > timings are corrected to match with SW state to nullify the errors. > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> I didn't check the details, but I think this is the fix we want. Still needs in-depth review, and some testing on a few different dsi bxt machines just to make sure I think, but on both patches: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cheers, Daniel > --- > drivers/gpu/drm/i915/intel_dsi.c | 97 ++++++++++++++++++++++++++++++++++---- > 1 file changed, 88 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index e0259e6..8a1b872 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -46,6 +46,14 @@ static const struct { > }, > }; > > +/* return pixels in terms of txbyteclkhs */ > +static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, > + u16 burst_mode_ratio) > +{ > + return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, > + 8 * 100), lane_count); > +} > + > /* return pixels equvalent to txbyteclkhs */ > static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count, > u16 burst_mode_ratio) > @@ -779,11 +787,19 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_display_mode *adjusted_mode = > &pipe_config->base.adjusted_mode; > + struct drm_display_mode *adjusted_mode_sw; > + struct intel_crtc *intel_crtc; > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > unsigned int lane_count = intel_dsi->lane_count; > unsigned int bpp, fmt; > enum port port; > u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; > + u16 hfp_sw, hsync_sw, hbp_sw; > + u16 crtc_htotal_sw, crtc_hsync_start_sw, crtc_hsync_end_sw, > + crtc_hblank_start_sw, crtc_hblank_end_sw; > + > + intel_crtc = to_intel_crtc(encoder->base.crtc); > + adjusted_mode_sw = &intel_crtc->config->base.adjusted_mode; > > /* > * Atleast one port is active as encoder->get_config called only if > @@ -847,8 +863,79 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, > adjusted_mode->crtc_vsync_end = vsync + adjusted_mode->crtc_vsync_start; > adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay; > adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal; > -} > > + /* > + * In BXT DSI there is no regs programmed with few horizontal timings > + * in Pixels but txbyteclkhs.. So retrieval process adds some > + * ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. > + * Actually here for the given adjusted_mode, we are calculating the > + * value programmed to the port and then back to the horizontal timing > + * param in pixels. This is the expected value, including roundup errors > + * And if that is same as retrieved value from port, then > + * (HW state) adjusted_mode's horizontal timings are corrected to > + * match with SW state to nullify the errors. > + */ > + /* Calculating the value programmed to the Port register */ > + hfp_sw = adjusted_mode_sw->crtc_hsync_start - > + adjusted_mode_sw->crtc_hdisplay; > + hsync_sw = adjusted_mode_sw->crtc_hsync_end - > + adjusted_mode_sw->crtc_hsync_start; > + hbp_sw = adjusted_mode_sw->crtc_htotal - > + adjusted_mode_sw->crtc_hsync_end; > + > + if (intel_dsi->dual_link) { > + hfp_sw /= 2; > + hsync_sw /= 2; > + hbp_sw /= 2; > + } > + > + hfp_sw = txbyteclkhs(hfp_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hsync_sw = txbyteclkhs(hsync_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hbp_sw = txbyteclkhs(hbp_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + > + /* Reverse calculating the adjusted mode parameters from port reg vals*/ > + hfp_sw = pixels_from_txbyteclkhs(hfp_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hsync_sw = pixels_from_txbyteclkhs(hsync_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hbp_sw = pixels_from_txbyteclkhs(hbp_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + > + if (intel_dsi->dual_link) { > + hfp_sw *= 2; > + hsync_sw *= 2; > + hbp_sw *= 2; > + } > + > + crtc_htotal_sw = adjusted_mode_sw->crtc_hdisplay + hfp_sw + > + hsync_sw + hbp_sw; > + crtc_hsync_start_sw = hfp_sw + adjusted_mode_sw->crtc_hdisplay; > + crtc_hsync_end_sw = hsync_sw + crtc_hsync_start_sw; > + crtc_hblank_start_sw = adjusted_mode_sw->crtc_hdisplay; > + crtc_hblank_end_sw = crtc_htotal_sw; > + > + if (adjusted_mode->crtc_htotal == crtc_htotal_sw) > + adjusted_mode->crtc_htotal = adjusted_mode_sw->crtc_htotal; > + > + if (adjusted_mode->crtc_hsync_start == crtc_hsync_start_sw) > + adjusted_mode->crtc_hsync_start = > + adjusted_mode_sw->crtc_hsync_start; > + > + if (adjusted_mode->crtc_hsync_end == crtc_hsync_end_sw) > + adjusted_mode->crtc_hsync_end = > + adjusted_mode_sw->crtc_hsync_end; > + > + if (adjusted_mode->crtc_hblank_start == crtc_hblank_start_sw) > + adjusted_mode->crtc_hblank_start = > + adjusted_mode_sw->crtc_hblank_start; > + > + if (adjusted_mode->crtc_hblank_end == crtc_hblank_end_sw) > + adjusted_mode->crtc_hblank_end = > + adjusted_mode_sw->crtc_hblank_end; > +} > > static void intel_dsi_get_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config) > @@ -917,14 +1004,6 @@ static u16 txclkesc(u32 divider, unsigned int us) > } > } > > -/* return pixels in terms of txbyteclkhs */ > -static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, > - u16 burst_mode_ratio) > -{ > - return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, > - 8 * 100), lane_count); > -} > - > static void set_dsi_timings(struct drm_encoder *encoder, > const struct drm_display_mode *adjusted_mode) > { > -- > 1.7.9.5 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval 2016-04-20 13:26 ` Daniel Vetter @ 2016-04-21 4:41 ` Ramalingam C 0 siblings, 0 replies; 7+ messages in thread From: Ramalingam C @ 2016-04-21 4:41 UTC (permalink / raw) To: Daniel Vetter; +Cc: jani.nikula, intel-gfx On Wednesday 20 April 2016 06:56 PM, Daniel Vetter wrote: > On Tue, Apr 19, 2016 at 01:48:14PM +0530, Ramalingam C wrote: >> In BXT DSI there is no regs programmed with few horizontal timings >> in Pixels but txbyteclkhs.. So retrieval process adds some >> ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. >> >> Actually here for the given adjusted_mode, we are calculating the >> value programmed to the port and then back to the horizontal timing >> param in pixels. This is the expected value at the end of get_config, >> including roundup errors. And if that is same as retrieved value >> from port, then retrieved (HW state) adjusted_mode's horizontal >> timings are corrected to match with SW state to nullify the errors. >> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > I didn't check the details, but I think this is the fix we want. Still > needs in-depth review, and some testing on a few different dsi bxt > machines just to make sure I think, but on both patches: > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Daniel and Jani, I have tested this on BXT-T B1 board with Tianma panel. This is working fine w.r.t PIPE_CONFIG COMPARE. Not sure whether i can give tested-by for my own patch. :) --Ram > > Cheers, Daniel >> --- >> drivers/gpu/drm/i915/intel_dsi.c | 97 ++++++++++++++++++++++++++++++++++---- >> 1 file changed, 88 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index e0259e6..8a1b872 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -46,6 +46,14 @@ static const struct { >> }, >> }; >> >> +/* return pixels in terms of txbyteclkhs */ >> +static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, >> + u16 burst_mode_ratio) >> +{ >> + return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, >> + 8 * 100), lane_count); >> +} >> + >> /* return pixels equvalent to txbyteclkhs */ >> static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count, >> u16 burst_mode_ratio) >> @@ -779,11 +787,19 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct drm_display_mode *adjusted_mode = >> &pipe_config->base.adjusted_mode; >> + struct drm_display_mode *adjusted_mode_sw; >> + struct intel_crtc *intel_crtc; >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >> unsigned int lane_count = intel_dsi->lane_count; >> unsigned int bpp, fmt; >> enum port port; >> u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; >> + u16 hfp_sw, hsync_sw, hbp_sw; >> + u16 crtc_htotal_sw, crtc_hsync_start_sw, crtc_hsync_end_sw, >> + crtc_hblank_start_sw, crtc_hblank_end_sw; >> + >> + intel_crtc = to_intel_crtc(encoder->base.crtc); >> + adjusted_mode_sw = &intel_crtc->config->base.adjusted_mode; >> >> /* >> * Atleast one port is active as encoder->get_config called only if >> @@ -847,8 +863,79 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, >> adjusted_mode->crtc_vsync_end = vsync + adjusted_mode->crtc_vsync_start; >> adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay; >> adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal; >> -} >> >> + /* >> + * In BXT DSI there is no regs programmed with few horizontal timings >> + * in Pixels but txbyteclkhs.. So retrieval process adds some >> + * ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. >> + * Actually here for the given adjusted_mode, we are calculating the >> + * value programmed to the port and then back to the horizontal timing >> + * param in pixels. This is the expected value, including roundup errors >> + * And if that is same as retrieved value from port, then >> + * (HW state) adjusted_mode's horizontal timings are corrected to >> + * match with SW state to nullify the errors. >> + */ >> + /* Calculating the value programmed to the Port register */ >> + hfp_sw = adjusted_mode_sw->crtc_hsync_start - >> + adjusted_mode_sw->crtc_hdisplay; >> + hsync_sw = adjusted_mode_sw->crtc_hsync_end - >> + adjusted_mode_sw->crtc_hsync_start; >> + hbp_sw = adjusted_mode_sw->crtc_htotal - >> + adjusted_mode_sw->crtc_hsync_end; >> + >> + if (intel_dsi->dual_link) { >> + hfp_sw /= 2; >> + hsync_sw /= 2; >> + hbp_sw /= 2; >> + } >> + >> + hfp_sw = txbyteclkhs(hfp_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + hsync_sw = txbyteclkhs(hsync_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + hbp_sw = txbyteclkhs(hbp_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + >> + /* Reverse calculating the adjusted mode parameters from port reg vals*/ >> + hfp_sw = pixels_from_txbyteclkhs(hfp_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + hsync_sw = pixels_from_txbyteclkhs(hsync_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + hbp_sw = pixels_from_txbyteclkhs(hbp_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + >> + if (intel_dsi->dual_link) { >> + hfp_sw *= 2; >> + hsync_sw *= 2; >> + hbp_sw *= 2; >> + } >> + >> + crtc_htotal_sw = adjusted_mode_sw->crtc_hdisplay + hfp_sw + >> + hsync_sw + hbp_sw; >> + crtc_hsync_start_sw = hfp_sw + adjusted_mode_sw->crtc_hdisplay; >> + crtc_hsync_end_sw = hsync_sw + crtc_hsync_start_sw; >> + crtc_hblank_start_sw = adjusted_mode_sw->crtc_hdisplay; >> + crtc_hblank_end_sw = crtc_htotal_sw; >> + >> + if (adjusted_mode->crtc_htotal == crtc_htotal_sw) >> + adjusted_mode->crtc_htotal = adjusted_mode_sw->crtc_htotal; >> + >> + if (adjusted_mode->crtc_hsync_start == crtc_hsync_start_sw) >> + adjusted_mode->crtc_hsync_start = >> + adjusted_mode_sw->crtc_hsync_start; >> + >> + if (adjusted_mode->crtc_hsync_end == crtc_hsync_end_sw) >> + adjusted_mode->crtc_hsync_end = >> + adjusted_mode_sw->crtc_hsync_end; >> + >> + if (adjusted_mode->crtc_hblank_start == crtc_hblank_start_sw) >> + adjusted_mode->crtc_hblank_start = >> + adjusted_mode_sw->crtc_hblank_start; >> + >> + if (adjusted_mode->crtc_hblank_end == crtc_hblank_end_sw) >> + adjusted_mode->crtc_hblank_end = >> + adjusted_mode_sw->crtc_hblank_end; >> +} >> >> static void intel_dsi_get_config(struct intel_encoder *encoder, >> struct intel_crtc_state *pipe_config) >> @@ -917,14 +1004,6 @@ static u16 txclkesc(u32 divider, unsigned int us) >> } >> } >> >> -/* return pixels in terms of txbyteclkhs */ >> -static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, >> - u16 burst_mode_ratio) >> -{ >> - return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, >> - 8 * 100), lane_count); >> -} >> - >> static void set_dsi_timings(struct drm_encoder *encoder, >> const struct drm_display_mode *adjusted_mode) >> { >> -- >> 1.7.9.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval 2016-04-19 8:18 ` [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval Ramalingam C 2016-04-20 13:26 ` Daniel Vetter @ 2016-04-27 17:11 ` Ville Syrjälä 2016-04-28 14:11 ` Jani Nikula 1 sibling, 1 reply; 7+ messages in thread From: Ville Syrjälä @ 2016-04-27 17:11 UTC (permalink / raw) To: Ramalingam C; +Cc: jani.nikula, intel-gfx On Tue, Apr 19, 2016 at 01:48:14PM +0530, Ramalingam C wrote: > In BXT DSI there is no regs programmed with few horizontal timings > in Pixels but txbyteclkhs.. So retrieval process adds some > ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. > > Actually here for the given adjusted_mode, we are calculating the > value programmed to the port and then back to the horizontal timing > param in pixels. This is the expected value at the end of get_config, > including roundup errors. And if that is same as retrieved value > from port, then retrieved (HW state) adjusted_mode's horizontal > timings are corrected to match with SW state to nullify the errors. > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi.c | 97 ++++++++++++++++++++++++++++++++++---- > 1 file changed, 88 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index e0259e6..8a1b872 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -46,6 +46,14 @@ static const struct { > }, > }; > > +/* return pixels in terms of txbyteclkhs */ > +static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, > + u16 burst_mode_ratio) > +{ > + return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, > + 8 * 100), lane_count); > +} > + > /* return pixels equvalent to txbyteclkhs */ > static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count, > u16 burst_mode_ratio) > @@ -779,11 +787,19 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_display_mode *adjusted_mode = > &pipe_config->base.adjusted_mode; > + struct drm_display_mode *adjusted_mode_sw; > + struct intel_crtc *intel_crtc; > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > unsigned int lane_count = intel_dsi->lane_count; > unsigned int bpp, fmt; > enum port port; > u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; > + u16 hfp_sw, hsync_sw, hbp_sw; > + u16 crtc_htotal_sw, crtc_hsync_start_sw, crtc_hsync_end_sw, > + crtc_hblank_start_sw, crtc_hblank_end_sw; > + > + intel_crtc = to_intel_crtc(encoder->base.crtc); > + adjusted_mode_sw = &intel_crtc->config->base.adjusted_mode; > > /* > * Atleast one port is active as encoder->get_config called only if > @@ -847,8 +863,79 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, > adjusted_mode->crtc_vsync_end = vsync + adjusted_mode->crtc_vsync_start; > adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay; > adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal; > -} > > + /* > + * In BXT DSI there is no regs programmed with few horizontal timings > + * in Pixels but txbyteclkhs.. So retrieval process adds some > + * ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. > + * Actually here for the given adjusted_mode, we are calculating the > + * value programmed to the port and then back to the horizontal timing > + * param in pixels. This is the expected value, including roundup errors > + * And if that is same as retrieved value from port, then > + * (HW state) adjusted_mode's horizontal timings are corrected to > + * match with SW state to nullify the errors. > + */ > + /* Calculating the value programmed to the Port register */ > + hfp_sw = adjusted_mode_sw->crtc_hsync_start - > + adjusted_mode_sw->crtc_hdisplay; > + hsync_sw = adjusted_mode_sw->crtc_hsync_end - > + adjusted_mode_sw->crtc_hsync_start; > + hbp_sw = adjusted_mode_sw->crtc_htotal - > + adjusted_mode_sw->crtc_hsync_end; So during the initial state readout we get passed crtc->config as pipe_config, and so we'll end up comparing the thing with itself. That should be fine. A bit of extra care is needed to make sure we don't use anything from crtc->config before we've filled it out with something, but looks like the code does things in the right order (given the previous patch which fills out all the htimings with something). I think the biggest issue with this patch is seeing the forest from the trees. Some refactoring would be good so that we'd have some kind of htimings_{to,from}_txbyteclkhs() functions instead of duplicating the same code multiple times. So while it does look quite strange to be using crtc->config in the .get_config() I think it should work out. Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + > + if (intel_dsi->dual_link) { > + hfp_sw /= 2; > + hsync_sw /= 2; > + hbp_sw /= 2; > + } > + > + hfp_sw = txbyteclkhs(hfp_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hsync_sw = txbyteclkhs(hsync_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hbp_sw = txbyteclkhs(hbp_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + > + /* Reverse calculating the adjusted mode parameters from port reg vals*/ > + hfp_sw = pixels_from_txbyteclkhs(hfp_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hsync_sw = pixels_from_txbyteclkhs(hsync_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hbp_sw = pixels_from_txbyteclkhs(hbp_sw, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + > + if (intel_dsi->dual_link) { > + hfp_sw *= 2; > + hsync_sw *= 2; > + hbp_sw *= 2; > + } > + > + crtc_htotal_sw = adjusted_mode_sw->crtc_hdisplay + hfp_sw + > + hsync_sw + hbp_sw; > + crtc_hsync_start_sw = hfp_sw + adjusted_mode_sw->crtc_hdisplay; > + crtc_hsync_end_sw = hsync_sw + crtc_hsync_start_sw; > + crtc_hblank_start_sw = adjusted_mode_sw->crtc_hdisplay; > + crtc_hblank_end_sw = crtc_htotal_sw; > + > + if (adjusted_mode->crtc_htotal == crtc_htotal_sw) > + adjusted_mode->crtc_htotal = adjusted_mode_sw->crtc_htotal; > + > + if (adjusted_mode->crtc_hsync_start == crtc_hsync_start_sw) > + adjusted_mode->crtc_hsync_start = > + adjusted_mode_sw->crtc_hsync_start; > + > + if (adjusted_mode->crtc_hsync_end == crtc_hsync_end_sw) > + adjusted_mode->crtc_hsync_end = > + adjusted_mode_sw->crtc_hsync_end; > + > + if (adjusted_mode->crtc_hblank_start == crtc_hblank_start_sw) > + adjusted_mode->crtc_hblank_start = > + adjusted_mode_sw->crtc_hblank_start; > + > + if (adjusted_mode->crtc_hblank_end == crtc_hblank_end_sw) > + adjusted_mode->crtc_hblank_end = > + adjusted_mode_sw->crtc_hblank_end; > +} > > static void intel_dsi_get_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config) > @@ -917,14 +1004,6 @@ static u16 txclkesc(u32 divider, unsigned int us) > } > } > > -/* return pixels in terms of txbyteclkhs */ > -static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, > - u16 burst_mode_ratio) > -{ > - return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, > - 8 * 100), lane_count); > -} > - > static void set_dsi_timings(struct drm_encoder *encoder, > const struct drm_display_mode *adjusted_mode) > { > -- > 1.7.9.5 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval 2016-04-27 17:11 ` Ville Syrjälä @ 2016-04-28 14:11 ` Jani Nikula 0 siblings, 0 replies; 7+ messages in thread From: Jani Nikula @ 2016-04-28 14:11 UTC (permalink / raw) To: Ville Syrjälä, Ramalingam C; +Cc: intel-gfx On Wed, 27 Apr 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Tue, Apr 19, 2016 at 01:48:14PM +0530, Ramalingam C wrote: >> In BXT DSI there is no regs programmed with few horizontal timings >> in Pixels but txbyteclkhs.. So retrieval process adds some >> ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. >> >> Actually here for the given adjusted_mode, we are calculating the >> value programmed to the port and then back to the horizontal timing >> param in pixels. This is the expected value at the end of get_config, >> including roundup errors. And if that is same as retrieved value >> from port, then retrieved (HW state) adjusted_mode's horizontal >> timings are corrected to match with SW state to nullify the errors. >> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dsi.c | 97 ++++++++++++++++++++++++++++++++++---- >> 1 file changed, 88 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index e0259e6..8a1b872 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -46,6 +46,14 @@ static const struct { >> }, >> }; >> >> +/* return pixels in terms of txbyteclkhs */ >> +static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, >> + u16 burst_mode_ratio) >> +{ >> + return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, >> + 8 * 100), lane_count); >> +} >> + >> /* return pixels equvalent to txbyteclkhs */ >> static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count, >> u16 burst_mode_ratio) >> @@ -779,11 +787,19 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct drm_display_mode *adjusted_mode = >> &pipe_config->base.adjusted_mode; >> + struct drm_display_mode *adjusted_mode_sw; >> + struct intel_crtc *intel_crtc; >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >> unsigned int lane_count = intel_dsi->lane_count; >> unsigned int bpp, fmt; >> enum port port; >> u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; >> + u16 hfp_sw, hsync_sw, hbp_sw; >> + u16 crtc_htotal_sw, crtc_hsync_start_sw, crtc_hsync_end_sw, >> + crtc_hblank_start_sw, crtc_hblank_end_sw; >> + >> + intel_crtc = to_intel_crtc(encoder->base.crtc); >> + adjusted_mode_sw = &intel_crtc->config->base.adjusted_mode; >> > >> /* >> * Atleast one port is active as encoder->get_config called only if >> @@ -847,8 +863,79 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, >> adjusted_mode->crtc_vsync_end = vsync + adjusted_mode->crtc_vsync_start; >> adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay; >> adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal; >> -} >> >> + /* >> + * In BXT DSI there is no regs programmed with few horizontal timings >> + * in Pixels but txbyteclkhs.. So retrieval process adds some >> + * ROUND_UP ERRORS in the process of PIXELS<==>txbyteclkhs. >> + * Actually here for the given adjusted_mode, we are calculating the >> + * value programmed to the port and then back to the horizontal timing >> + * param in pixels. This is the expected value, including roundup errors >> + * And if that is same as retrieved value from port, then >> + * (HW state) adjusted_mode's horizontal timings are corrected to >> + * match with SW state to nullify the errors. >> + */ >> + /* Calculating the value programmed to the Port register */ >> + hfp_sw = adjusted_mode_sw->crtc_hsync_start - >> + adjusted_mode_sw->crtc_hdisplay; >> + hsync_sw = adjusted_mode_sw->crtc_hsync_end - >> + adjusted_mode_sw->crtc_hsync_start; >> + hbp_sw = adjusted_mode_sw->crtc_htotal - >> + adjusted_mode_sw->crtc_hsync_end; > > So during the initial state readout we get passed crtc->config as > pipe_config, and so we'll end up comparing the thing with itself. That > should be fine. A bit of extra care is needed to make sure we don't use > anything from crtc->config before we've filled it out with something, > but looks like the code does things in the right order (given the > previous patch which fills out all the htimings with something). > > I think the biggest issue with this patch is seeing the forest from the > trees. Some refactoring would be good so that we'd have some kind of > htimings_{to,from}_txbyteclkhs() functions instead of duplicating the > same code multiple times. Agreed. However, I opted to push them as-is as they fix the boot hang, and leave the refactoring as follow-up. BR, Jani. > > So while it does look quite strange to be using crtc->config in the > .get_config() I think it should work out. > > Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> + >> + if (intel_dsi->dual_link) { >> + hfp_sw /= 2; >> + hsync_sw /= 2; >> + hbp_sw /= 2; >> + } >> + >> + hfp_sw = txbyteclkhs(hfp_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + hsync_sw = txbyteclkhs(hsync_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + hbp_sw = txbyteclkhs(hbp_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + >> + /* Reverse calculating the adjusted mode parameters from port reg vals*/ >> + hfp_sw = pixels_from_txbyteclkhs(hfp_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + hsync_sw = pixels_from_txbyteclkhs(hsync_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + hbp_sw = pixels_from_txbyteclkhs(hbp_sw, bpp, lane_count, >> + intel_dsi->burst_mode_ratio); >> + >> + if (intel_dsi->dual_link) { >> + hfp_sw *= 2; >> + hsync_sw *= 2; >> + hbp_sw *= 2; >> + } >> + >> + crtc_htotal_sw = adjusted_mode_sw->crtc_hdisplay + hfp_sw + >> + hsync_sw + hbp_sw; >> + crtc_hsync_start_sw = hfp_sw + adjusted_mode_sw->crtc_hdisplay; >> + crtc_hsync_end_sw = hsync_sw + crtc_hsync_start_sw; >> + crtc_hblank_start_sw = adjusted_mode_sw->crtc_hdisplay; >> + crtc_hblank_end_sw = crtc_htotal_sw; >> + >> + if (adjusted_mode->crtc_htotal == crtc_htotal_sw) >> + adjusted_mode->crtc_htotal = adjusted_mode_sw->crtc_htotal; >> + >> + if (adjusted_mode->crtc_hsync_start == crtc_hsync_start_sw) >> + adjusted_mode->crtc_hsync_start = >> + adjusted_mode_sw->crtc_hsync_start; >> + >> + if (adjusted_mode->crtc_hsync_end == crtc_hsync_end_sw) >> + adjusted_mode->crtc_hsync_end = >> + adjusted_mode_sw->crtc_hsync_end; >> + >> + if (adjusted_mode->crtc_hblank_start == crtc_hblank_start_sw) >> + adjusted_mode->crtc_hblank_start = >> + adjusted_mode_sw->crtc_hblank_start; >> + >> + if (adjusted_mode->crtc_hblank_end == crtc_hblank_end_sw) >> + adjusted_mode->crtc_hblank_end = >> + adjusted_mode_sw->crtc_hblank_end; >> +} >> >> static void intel_dsi_get_config(struct intel_encoder *encoder, >> struct intel_crtc_state *pipe_config) >> @@ -917,14 +1004,6 @@ static u16 txclkesc(u32 divider, unsigned int us) >> } >> } >> >> -/* return pixels in terms of txbyteclkhs */ >> -static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count, >> - u16 burst_mode_ratio) >> -{ >> - return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio, >> - 8 * 100), lane_count); >> -} >> - >> static void set_dsi_timings(struct drm_encoder *encoder, >> const struct drm_display_mode *adjusted_mode) >> { >> -- >> 1.7.9.5 >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/i915/BXT: Retrieving the horizontal timing for DSI 2016-04-19 8:18 [PATCH 1/2] drm/i915/BXT: Retrieving the horizontal timing for DSI Ramalingam C 2016-04-19 8:18 ` [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval Ramalingam C @ 2016-04-27 16:20 ` Imre Deak 1 sibling, 0 replies; 7+ messages in thread From: Imre Deak @ 2016-04-27 16:20 UTC (permalink / raw) To: Ramalingam C, intel-gfx, daniel, jani.nikula On ti, 2016-04-19 at 13:48 +0530, Ramalingam C wrote: > Retriving the horizontal timings from the port registers as > part of get_config() > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> I saw the crash below with current -nightly on a BXT-M board and Jani pointed me to these two patches which get rid of it, so Acked-by: Imre Deak <imre.deak@intel.com> [ 56.916557] divide error: 0000 [#1] PREEMPT SMP [ 56.921741] Modules linked in: i915(+) drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm intel_gtt agpgart cf g80211 rfkill binfmt_misc ax88179_178a kvm_intel kvm irqbypass crc32c_intel efivars tpm_tis tpm fuse [ 56.944106] CPU: 3 PID: 1097 Comm: modprobe Not tainted 4.6.0-rc4+ #433 [ 56.951501] Hardware name: Intel Corp. Broxton M/RVP, BIOS BXT1RVPA.X64.0131.B30.1604142217 04/14/2016 [ 56.961908] task: ffff88007a854d00 ti: ffff88007aea0000 task.ti: ffff88007aea0000 [ 56.970273] RIP: 0010:[<ffffffffa01235b2>] [<ffffffffa01235b2>] drm_mode_hsync+0x22/0x40 [drm] [ 56.980043] RSP: 0018:ffff88007aea3788 EFLAGS: 00010206 [ 56.985982] RAX: 000000000788b600 RBX: ffff880073c22108 RCX: 0000000000000000 [ 56.993957] RDX: 0000000000000000 RSI: ffff88007ab06800 RDI: ffff880073c22108 [ 57.001935] RBP: ffff88007aea3788 R08: 0000000000000001 R09: ffff880073c221e8 [ 57.009903] R10: ffff880073c22108 R11: 0000000000000001 R12: ffff88007a300000 [ 57.017872] R13: ffff880073c22000 R14: ffff880175f78000 R15: ffff880175f78798 [ 57.025849] FS: 00007f105d3e6700(0000) GS:ffff88017fd80000(0000) knlGS:0000000000000000 [ 57.034894] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 57.041317] CR2: 00007f4d485101d0 CR3: 000000007a820000 CR4: 00000000003406e0 [ 57.049292] Stack: [ 57.051539] ffff88007aea37a0 ffffffffa043b632 ffff880175f787c8 ffff88007aea3810 [ 57.059825] ffffffffa043d59e ffff880175f787b0 ffff88007ab68c00 ffff88007aea37f0 [ 57.068128] ffff880073c221e8 ffff880073c22108 ffff880175f78780 ffff880100000000 [ 57.076430] Call Trace: [ 57.079254] [<ffffffffa043b632>] intel_mode_from_pipe_config+0x82/0xb0 [i915] [ 57.087405] [<ffffffffa043d59e>] intel_modeset_setup_hw_state+0x55e/0xd60 [i915] [ 57.095847] [<ffffffffa043ff94>] intel_modeset_init+0x8e4/0x1630 [i915] [ 57.103415] [<ffffffffa047bcf0>] i915_driver_load+0xbe0/0x1980 [i915] [ 57.110745] [<ffffffffa0116c19>] drm_dev_register+0xa9/0xc0 [drm] [ 57.117681] [<ffffffffa011921d>] drm_get_pci_dev+0x8d/0x1e0 [drm] [ 57.124600] [<ffffffff8195f942>] ? _raw_spin_unlock_irqrestore+0x42/0x70 [ 57.132253] [<ffffffffa03b0384>] i915_pci_probe+0x34/0x50 [i915] [ 57.139070] [<ffffffff8149c375>] local_pci_probe+0x45/0xa0 [ 57.145303] [<ffffffff8149d300>] ? pci_match_device+0xe0/0x110 [ 57.151924] [<ffffffff8149d6cb>] pci_device_probe+0xdb/0x130 [ 57.158355] [<ffffffff81579b93>] driver_probe_device+0x223/0x440 [ 57.165169] [<ffffffff81579e85>] __driver_attach+0xd5/0x100 [ 57.171500] [<ffffffff81579db0>] ? driver_probe_device+0x440/0x440 [ 57.178510] [<ffffffff81577736>] bus_for_each_dev+0x66/0xa0 [ 57.184841] [<ffffffff815793de>] driver_attach+0x1e/0x20 [ 57.190881] [<ffffffff81578d6e>] bus_add_driver+0x1ee/0x280 [ 57.197212] [<ffffffff8157abc0>] driver_register+0x60/0xe0 [ 57.203447] [<ffffffff8149bc50>] __pci_register_driver+0x60/0x70 [ 57.210285] [<ffffffffa0119450>] drm_pci_init+0xe0/0x110 [drm] [ 57.216911] [<ffffffff810dcd8d>] ? trace_hardirqs_on+0xd/0x10 [ 57.223434] [<ffffffffa023a000>] ? 0xffffffffa023a000 [ 57.229237] [<ffffffffa023a092>] i915_init+0x92/0x99 [i915] [ 57.235570] [<ffffffff810003db>] do_one_initcall+0xab/0x1d0 [ 57.241900] [<ffffffff810f9eef>] ? rcu_read_lock_sched_held+0x7f/0x90 [ 57.249205] [<ffffffff81204f18>] ? kmem_cache_alloc_trace+0x248/0x2b0 [ 57.256509] [<ffffffff811a5eee>] ? do_init_module+0x27/0x1d9 [ 57.262934] [<ffffffff811a5f26>] do_init_module+0x5f/0x1d9 [ 57.269167] [<ffffffff8112392f>] load_module+0x20ef/0x27b0 [ 57.275401] [<ffffffff8111f8e0>] ? store_uevent+0x40/0x40 [ 57.281541] [<ffffffff81124243>] SYSC_finit_module+0xc3/0xf0 [ 57.287969] [<ffffffff8112428e>] SyS_finit_module+0xe/0x10 [ 57.294203] [<ffffffff81960069>] entry_SYSCALL_64_fastpath+0x1c/0xac [ 57.301406] Code: ff 5d c3 66 0f 1f 44 00 00 0f 1f 44 00 00 8b 87 d8 00 00 00 55 48 89 e5 85 c0 75 22 8b 4f 68 85 c9 78 1b 69 47 58 e8 03 00 00 99 <f7> f9 b9 d3 4d 62 10 05 f4 01 00 00 f7 e1 89 d0 c1 e8 06 5d c3 [ 57.322964] RIP [<ffffffffa01235b2>] drm_mode_hsync+0x22/0x40 [drm] [ 57.330103] RSP <ffff88007aea3788> [ 57.334276] ---[ end trace d414224cb2e2a4cf ]--- [ 57.339861] modprobe (1097) used greatest stack depth: 12048 bytes left > --- > drivers/gpu/drm/i915/intel_dsi.c | 44 ++++++++++++++++++++++++++++++++------ > 1 file changed, 37 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 9ff6435..e0259e6 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -46,6 +46,14 @@ static const struct { > }, > }; > > +/* return pixels equvalent to txbyteclkhs */ > +static u16 pixels_from_txbyteclkhs(u16 clk_hs, int bpp, int lane_count, > + u16 burst_mode_ratio) > +{ > + return DIV_ROUND_UP((clk_hs * lane_count * 8 * 100), > + (bpp * burst_mode_ratio)); > +} > + > enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt) > { > /* It just so happens the VBT matches register contents. */ > @@ -772,9 +780,10 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, > struct drm_display_mode *adjusted_mode = > &pipe_config->base.adjusted_mode; > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > + unsigned int lane_count = intel_dsi->lane_count; > unsigned int bpp, fmt; > enum port port; > - u16 vfp, vsync, vbp; > + u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; > > /* > * Atleast one port is active as encoder->get_config called only if > @@ -799,22 +808,43 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, > adjusted_mode->crtc_vtotal = > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > > + hactive = adjusted_mode->crtc_hdisplay; > + hfp = I915_READ(MIPI_HFP_COUNT(port)); > + > /* > - * TODO: Retrieve hfp, hsync and hbp. Adjust them for dual link and > - * calculate hsync_start, hsync_end, htotal and hblank_end > + * Meaningful for video mode non-burst sync pulse mode only, > + * can be zero for non-burst sync events and burst modes > */ > + hsync = I915_READ(MIPI_HSYNC_PADDING_COUNT(port)); > + hbp = I915_READ(MIPI_HBP_COUNT(port)); > + > + /* harizontal values are in terms of high speed byte clock */ > + hfp = pixels_from_txbyteclkhs(hfp, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hsync = pixels_from_txbyteclkhs(hsync, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + hbp = pixels_from_txbyteclkhs(hbp, bpp, lane_count, > + intel_dsi->burst_mode_ratio); > + > + if (intel_dsi->dual_link) { > + hfp *= 2; > + hsync *= 2; > + hbp *= 2; > + } > > /* vertical values are in terms of lines */ > vfp = I915_READ(MIPI_VFP_COUNT(port)); > vsync = I915_READ(MIPI_VSYNC_PADDING_COUNT(port)); > vbp = I915_READ(MIPI_VBP_COUNT(port)); > > + adjusted_mode->crtc_htotal = hactive + hfp + hsync + hbp; > + adjusted_mode->crtc_hsync_start = hfp + adjusted_mode->crtc_hdisplay; > + adjusted_mode->crtc_hsync_end = hsync + adjusted_mode->crtc_hsync_start; > adjusted_mode->crtc_hblank_start = adjusted_mode->crtc_hdisplay; > + adjusted_mode->crtc_hblank_end = adjusted_mode->crtc_htotal; > > - adjusted_mode->crtc_vsync_start = > - vfp + adjusted_mode->crtc_vdisplay; > - adjusted_mode->crtc_vsync_end = > - vsync + adjusted_mode->crtc_vsync_start; > + adjusted_mode->crtc_vsync_start = vfp + adjusted_mode->crtc_vdisplay; > + adjusted_mode->crtc_vsync_end = vsync + adjusted_mode->crtc_vsync_start; > adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay; > adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal; > } _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-04-28 14:11 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-19 8:18 [PATCH 1/2] drm/i915/BXT: Retrieving the horizontal timing for DSI Ramalingam C 2016-04-19 8:18 ` [RFC PATCH 2/2] drm/i915/bxt: Adjusting the error in horizontal timings retrieval Ramalingam C 2016-04-20 13:26 ` Daniel Vetter 2016-04-21 4:41 ` Ramalingam C 2016-04-27 17:11 ` Ville Syrjälä 2016-04-28 14:11 ` Jani Nikula 2016-04-27 16:20 ` [PATCH 1/2] drm/i915/BXT: Retrieving the horizontal timing for DSI Imre Deak
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.