Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Oct 13, 2015 at 11:03:27AM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >Sent: Monday, October 12, 2015 10:54 PM
> >To: Shankar, Uma
> >Cc: intel-gfx@lists.freedesktop.org; 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 <uma.shankar@intel.com>
> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> >> ---
> >>  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.

> 
> 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@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >--
> >Ville Syrjälä
> >Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx