On Tue, Oct 13, 2015 at 01:24:27PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > >Sent: Tuesday, October 13, 2015 4:54 PM > >To: Shankar, Uma > >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Kumar, Shobhit > >Subject: Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe > >timing for BXT DSI > > > >On Tue, Oct 13, 2015 at 11:03:27AM +0000, Shankar, Uma wrote: > >> > >> > >> >-----Original Message----- > >> >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > >> >Sent: Monday, October 12, 2015 10:54 PM > >> >To: Shankar, Uma > >> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Kumar, Shobhit > >> >Subject: Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: > >> >Get pipe timing for BXT DSI > >> > > >> >On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote: > >> >> For BXT DSI, vtotal, vactive, hactive registers are different. > >> >> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to > >> >> read the correct registers for BXT DSI. > >> >> > >> >> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> > >> >> Signed-off-by: Vandana Kannan <vandana.kannan@xxxxxxxxx> > >> >> --- > >> >> drivers/gpu/drm/i915/intel_display.c | 48 > >> >+++++++++++++++++++++++++++++++--- > >> >> 1 file changed, 45 insertions(+), 3 deletions(-) > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c > >> >> b/drivers/gpu/drm/i915/intel_display.c > >> >> index 75c60b8..2717823 100644 > >> >> --- a/drivers/gpu/drm/i915/intel_display.c > >> >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> >> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(struct > >> >> intel_crtc > >> >*crtc, > >> >> struct drm_device *dev = crtc->base.dev; > >> >> struct drm_i915_private *dev_priv = dev->dev_private; > >> >> enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; > >> >> + bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI); > >> >> uint32_t tmp; > >> >> > >> >> tmp = I915_READ(HTOTAL(cpu_transcoder)); > >> >> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct > >> >> intel_crtc > >> >*crtc, > >> >> pipe_config->base.adjusted_mode.crtc_vblank_end += 1; > >> >> } > >> >> > >> >> + if (IS_BROXTON(dev) && is_dsi) { > >> >> + struct intel_encoder *encoder; > >> >> + > >> >> + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { > >> >> + struct intel_dsi *intel_dsi = > >> >> + enc_to_intel_dsi(&encoder->base); > >> >> + enum port port; > >> >> + > >> >> + for_each_dsi_port(port, intel_dsi->ports) { > >> >> + pipe_config->base.adjusted_mode.crtc_hdisplay > >> >= > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > >> >> + pipe_config->base.adjusted_mode.crtc_vdisplay > >> >= > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > >> >> + pipe_config->base.adjusted_mode.crtc_vtotal = > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > >> >> + } > >> >> + } > >> >> + > >> >> + } > >> > > >> >I think I already asked this earlier when this patch was posted; > >> >Don't the normal pipe timing registers contain the same values? And > >> >if so, why would you need to read them out from the DSI speciific registers? > >> > > >> > >> Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI > >> for BXT. Hence getting the data from DSI specific registers for BXT. Separate > >MIPI transcoder is used for BXT. > > > >So calling intel_set_pipe_timings() is pointless (apart from PIPESRC)? > > > >If that's the case then it seems the readback should also read all of it from the > >DSI registers instead of reading some from the pipe timings and some from DSI > >registers. > > > > Yes set_pipe_timings is doing only the pipesrc programming which is useful for DSI, rest is not needed. > > Readback can be rectified to read all other details as well from DSI specific registers apart from the ones > currently being used. Also this kind of is_dsi check is a bit a hack and won't really work for fastboot case (intel_pipe_has_type doesn't work at that point yet). Instead you need to push this into a dsi-specific readout function. -Daniel > > >> > >> Regards, > >> Uma Shankar > >> > >> >> + > >> >> tmp = I915_READ(PIPESRC(crtc->pipe)); > >> >> pipe_config->pipe_src_h = (tmp & 0xffff) + 1; > >> >> pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6 > >> >> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct > >> >drm_device *dev, > >> >> int vtot = I915_READ(VTOTAL(cpu_transcoder)); > >> >> int vsync = I915_READ(VSYNC(cpu_transcoder)); > >> >> enum pipe pipe = intel_crtc->pipe; > >> >> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); > >> >> > >> >> mode = kzalloc(sizeof(*mode), GFP_KERNEL); > >> >> if (!mode) > >> >> @@ -10684,15 +10706,35 @@ struct drm_display_mode > >> >*intel_crtc_mode_get(struct drm_device *dev, > >> >> i9xx_crtc_clock_get(intel_crtc, &pipe_config); > >> >> > >> >> mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier; > >> >> - mode->hdisplay = (htot & 0xffff) + 1; > >> >> mode->htotal = ((htot & 0xffff0000) >> 16) + 1; > >> >> mode->hsync_start = (hsync & 0xffff) + 1; > >> >> mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1; > >> >> - mode->vdisplay = (vtot & 0xffff) + 1; > >> >> - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > >> >> mode->vsync_start = (vsync & 0xffff) + 1; > >> >> mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1; > >> >> > >> >> + if (IS_BROXTON(dev) && is_dsi) { > >> >> + struct intel_encoder *encoder; > >> >> + > >> >> + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) { > >> >> + struct intel_dsi *intel_dsi = > >> >> + enc_to_intel_dsi(&encoder->base); > >> >> + enum port port; > >> >> + > >> >> + for_each_dsi_port(port, intel_dsi->ports) { > >> >> + mode->vtotal = > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); > >> >> + mode->hdisplay = > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); > >> >> + mode->vdisplay = > >> >> + > >> > I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); > >> >> + } > >> >> + } > >> >> + } else { > >> >> + mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1; > >> >> + mode->hdisplay = (htot & 0xffff) + 1; > >> >> + mode->vdisplay = (vtot & 0xffff) + 1; > >> >> + } > >> >> + > >> >> drm_mode_set_name(mode); > >> >> > >> >> return mode; > >> >> -- > >> >> 1.7.9.5 > >> >> > >> >> _______________________________________________ > >> >> Intel-gfx mailing list > >> >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> > > >> >-- > >> >Ville Syrjälä > >> >Intel OTC > > > >-- > >Ville Syrjälä > >Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
