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

[Shankar, Uma]

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

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.

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