On Tue, Oct 13, 2015 at 01:24:27PM +0000, Shankar, Uma wrote:
>
>
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:[email protected]]
> >Sent: Tuesday, October 13, 2015 4:54 PM
> >To: Shankar, Uma
> >Cc: [email protected]; Kumar, Shobhit
> >Subject: Re: [Intel-gfx] [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:[email protected]]
> >> >Sent: Monday, October 12, 2015 10:54 PM
> >> >To: Shankar, Uma
> >> >Cc: [email protected]; Kumar, Shobhit
> >> >Subject: Re: [Intel-gfx] [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 <[email protected]>
> >> >> Signed-off-by: Vandana Kannan <[email protected]>
> >> >> ---
> >> >> 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
> >> >> [email protected]
> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >
> >> >--
> >> >Ville Syrjälä
> >> >Intel OTC
> >
> >--
> >Ville Syrjälä
> >Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
