[Intel-gfx] [PATCH] drm/i915/BXT: Fixed COS blanking issue

Ramalingam C ramalingam.c at intel.com
Fri Mar 11 13:24:56 UTC 2016


Hi,

This change retrieves the pipe timings from port registers as the pipe 
registers are inactive in BXT DSI.
As the  hfp, hsync and hbp are programmed to port registers interms of 
byteclk hence converted to the pixels.
In this conversions we have multiple approximations due to DIV_ROUND_UP 
hence we will have some delta in the retrieved values.

As per my discussion with Jani, these errors are calculated and handled 
at pipe_config comparison at
https://lists.freedesktop.org/archives/intel-gfx/2016-March/089548.html

On Tuesday 01 March 2016 12:19 AM, Ramalingam C wrote:
> During Charging OS mode, mipi display was blanking.This is
> because during driver load, though encoder, connector were
> active but crtc returned inactive. This caused sanitize
> function to disable the DSI panel. In AOS, this is fine
> since HWC will do a modeset and crtc, connector, encoder
> mapping will be restored. But in COS, no modeset is called,
> it just calls DPMS ON/OFF.
>
> This is fine on BYT/CHT since transcoder is common b/w
> all encoders. But for BXT, there is a separate mipi
> transcoder. Hence this needs special handling for BXT.
>
> V2: pipe_config->has_dsi_encoder is set for BXT [By Jani]
>
> V3: Timing parameters are retrieved from VBT. [By Jani]
>
> V4: Retrieving the pipe timing from the DSI port registers.
>      Added a encoder function for this purpose. [By Jani]
>
> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
> ---
> Review of previous patch set can be found at
> https://lists.freedesktop.org/archives/intel-gfx/2016-February/088060.html
>
>   drivers/gpu/drm/i915/intel_display.c |   88 ++++++++++++++++++++++++++++--
>   drivers/gpu/drm/i915/intel_drv.h     |    9 +++-
>   drivers/gpu/drm/i915/intel_dsi.c     |   98 ++++++++++++++++++++++++++++++++++
>   3 files changed, 189 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index adde6f8..31b9a71 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -34,6 +34,7 @@
>   #include <drm/drm_edid.h>
>   #include <drm/drmP.h>
>   #include "intel_drv.h"
> +#include "intel_dsi.h"
>   #include <drm/i915_drm.h>
>   #include "i915_drv.h"
>   #include "i915_trace.h"
> @@ -9981,11 +9982,66 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>   	}
>   }
>   
> +enum pipe pipe_linked_to_dsi_port(struct drm_device *dev, enum port port)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = INVALID_PIPE;
> +	uint32_t dsi_ctrl;
> +
> +	dsi_ctrl = I915_READ(MIPI_CTRL(port));
> +	switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
> +	case BXT_PIPE_SELECT(PIPE_A):
> +		pipe = PIPE_A;
> +		break;
> +	case BXT_PIPE_SELECT(PIPE_B):
> +		pipe = PIPE_B;
> +		break;
> +	case BXT_PIPE_SELECT(PIPE_C):
> +		pipe = PIPE_C;
> +		break;
> +	default:
> +		WARN(1, "unknown pipe linked to dsi transcoder\n");
> +	}
> +
> +	return pipe;
> +}
> +
> +struct intel_encoder *bxt_get_dsi_encoder_for_crtc(struct intel_crtc *crtc,
> +					struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_encoder *intel_encoder;
> +	struct intel_dsi *intel_dsi;
> +	enum port port;
> +
> +	for_each_intel_encoder(dev, intel_encoder) {
> +		if (intel_encoder->type == INTEL_OUTPUT_DSI) {
> +			intel_dsi = enc_to_intel_dsi(&intel_encoder->base);
> +			if (!intel_dsi)
> +				continue;
> +
> +			for_each_dsi_port(port, intel_dsi->ports) {
> +				if (!(I915_READ(BXT_MIPI_PORT_CTRL(port)) &
> +								DPI_ENABLE))
> +					continue;
> +				if (pipe_linked_to_dsi_port(dev, port) ==
> +								crtc->pipe) {
> +					pipe_config->has_dsi_encoder = true;
> +					return intel_encoder;
> +				}
> +			}
> +		}
> +	}
> +	return NULL;
> +}
> +
>   static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>   				    struct intel_crtc_state *pipe_config)
>   {
>   	struct drm_device *dev = crtc->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_encoder *intel_encoder, *attached_encoder = NULL;
>   	enum intel_display_power_domain power_domain;
>   	unsigned long power_domain_mask;
>   	uint32_t tmp;
> @@ -10023,18 +10079,40 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>   			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>   	}
>   
> +	for_each_encoder_on_crtc(dev, &crtc->base, intel_encoder)
> +		attached_encoder = intel_encoder;
> +
> +	/*
> +	 * attached_encoder will be NULL, if there is no modeset from the
> +	 * kernel bootup.
> +	 */
> +	if (!attached_encoder && dev_priv->vbt.has_mipi && IS_BROXTON(dev))
> +		attached_encoder =
> +				bxt_get_dsi_encoder_for_crtc(crtc, pipe_config);
> +
>   	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
>   	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>   		goto out;
>   	power_domain_mask |= BIT(power_domain);
>   
> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> -	if (!(tmp & PIPECONF_ENABLE))
> -		goto out;
> +	if (attached_encoder && attached_encoder->get_pipe_config)
> +		/*
> +		 * BXT doesn't have the PIPE timing registers.
> +		 * Hence retrieved the pipe detail from PORT register.
> +		 * And timing parameters are obtained from VBT
> +		 */
> +		attached_encoder->get_pipe_config(attached_encoder,
> +								pipe_config);
> +	else {
> +		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> +		if (!(tmp & PIPECONF_ENABLE))
> +			goto out;
>   
> -	haswell_get_ddi_port_state(crtc, pipe_config);
> +		if (!IS_VALLEYVIEW(dev))
> +			haswell_get_ddi_port_state(crtc, pipe_config);
>   
> -	intel_get_pipe_timings(crtc, pipe_config);
> +		intel_get_pipe_timings(crtc, pipe_config);
> +	}
>   
>   	if (INTEL_INFO(dev)->gen >= 9) {
>   		skl_init_scalers(dev, crtc, pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4852049..58979e4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -152,6 +152,13 @@ struct intel_encoder {
>   	void (*get_config)(struct intel_encoder *,
>   			   struct intel_crtc_state *pipe_config);
>   	/*
> +	 * Retrives the PIPE config from the PORT registers where PIPE
> +	 * registers are invalid and only PORT registers will hold the timing
> +	 * parameters.
> +	 */
> +	void (*get_pipe_config)(struct intel_encoder *,
> +			   struct intel_crtc_state *pipe_config);
> +	/*
>   	 * Called during system suspend after all pending requests for the
>   	 * encoder are flushed (for example for DP AUX transactions) and
>   	 * device interrupts are disabled.
> @@ -1290,7 +1297,7 @@ int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int con
>   void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
>   /* intel_dsi.c */
>   void intel_dsi_init(struct drm_device *dev);
> -
> +enum pipe pipe_linked_to_dsi_port(struct drm_device *dev, enum port port);
>   
>   /* intel_dvo.c */
>   void intel_dvo_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 734f06a..7fbeae5 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -745,6 +745,103 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>   	pipe_config->port_clock = pclk;
>   }
>   
> +/* 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));
> +}
> +
> +static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder,
> +				 struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_display_mode *adjusted_mode =
> +					&pipe_config->base.adjusted_mode;
> +	struct intel_dsi *intel_dsi =
> +				enc_to_intel_dsi(&encoder->base);
> +	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
> +	unsigned int lane_count = intel_dsi->lane_count;
> +	enum port port;
> +	enum pipe dsi_pipe;
> +	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
> +	uint32_t tmp;
> +
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		if (!(I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE))
> +			continue;
> +
> +		/* In terms of pixels */
> +		adjusted_mode->crtc_hdisplay =
> +					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +		adjusted_mode->crtc_vdisplay =
> +					I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +		adjusted_mode->crtc_vtotal =
> +					I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +
> +		hactive = adjusted_mode->crtc_hdisplay;
> +		hfp = I915_READ(MIPI_HFP_COUNT(port));
> +
> +		/*
> +		 * 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));
> +
> +		/* horizontal 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_vblank_start = adjusted_mode->crtc_vdisplay;
> +		adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
> +
> +		pipe_config->pipe_bpp =
> +				dsi_pixel_format_bpp(intel_dsi->pixel_format);
> +
> +		dsi_pipe = pipe_linked_to_dsi_port(dev, port);
> +		tmp = I915_READ(PIPESRC(dsi_pipe));
> +		pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> +		pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> +
> +		pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
> +		pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
> +
> +		break;
> +	}
> +}
> +
>   static enum drm_mode_status
>   intel_dsi_mode_valid(struct drm_connector *connector,
>   		     struct drm_display_mode *mode)
> @@ -1173,6 +1270,7 @@ void intel_dsi_init(struct drm_device *dev)
>   	intel_encoder->post_disable = intel_dsi_post_disable;
>   	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
>   	intel_encoder->get_config = intel_dsi_get_config;
> +	intel_encoder->get_pipe_config = bxt_dsi_get_pipe_config;
>   
>   	intel_connector->get_hw_state = intel_connector_get_hw_state;
>   	intel_connector->unregister = intel_connector_unregister;

-- 
Thanks,
--Ram



More information about the Intel-gfx mailing list