Re: [PATCH 2/9] drm/i915: Added port as parameter to the functions which does read/write of DSI Controller

[Jani Nikula <jani.nikula@linux.intel.com>]

On Sat, 29 Nov 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
> This patch is in preparation of DSI dual link panels. For dual link
> panels, few packets needs to be sent to Port A or Port C or both. Based
> on the portno from MIPI Sequence Block#53, these sequences needs to be
> sent accordingly.
>
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_cmd.c       |   75 ++++++++++++----------------
>  drivers/gpu/drm/i915/intel_dsi_cmd.h       |   46 +++++++++--------
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   21 +++++---
>  3 files changed, 72 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c b/drivers/gpu/drm/i915/intel_dsi_cmd.c
> index 004fa91..8e30684 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
> @@ -48,13 +48,11 @@
>   * For memory writes, these should probably be used for performance.
>   */
>  
> -static void print_stat(struct intel_dsi *intel_dsi)
> +static void print_stat(struct intel_dsi *intel_dsi, enum port port)
>  {
>  	struct drm_encoder *encoder = &intel_dsi->base.base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>  	u32 val;
>  
>  	val = I915_READ(MIPI_INTR_STAT(port));
> @@ -104,13 +102,12 @@ enum dsi_type {
>  };
>  
>  /* enable or disable command mode hs transmissions */
> -void dsi_hs_mode_enable(struct intel_dsi *intel_dsi, bool enable)
> +void dsi_hs_mode_enable(struct intel_dsi *intel_dsi, bool enable,
> +						enum port port)
>  {
>  	struct drm_encoder *encoder = &intel_dsi->base.base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>  	u32 temp;
>  	u32 mask = DBI_FIFO_EMPTY;
>  
> @@ -125,13 +122,11 @@ void dsi_hs_mode_enable(struct intel_dsi *intel_dsi, bool enable)
>  }
>  
>  static int dsi_vc_send_short(struct intel_dsi *intel_dsi, int channel,
> -			     u8 data_type, u16 data)
> +			     u8 data_type, u16 data, enum port port)
>  {
>  	struct drm_encoder *encoder = &intel_dsi->base.base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>  	u32 ctrl_reg;
>  	u32 ctrl;
>  	u32 mask;
> @@ -149,7 +144,7 @@ static int dsi_vc_send_short(struct intel_dsi *intel_dsi, int channel,
>  
>  	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & mask) == 0, 50)) {
>  		DRM_ERROR("Timeout waiting for HS/LP CTRL FIFO !full\n");
> -		print_stat(intel_dsi);
> +		print_stat(intel_dsi, port);
>  	}
>  
>  	/*
> @@ -167,13 +162,11 @@ static int dsi_vc_send_short(struct intel_dsi *intel_dsi, int channel,
>  }
>  
>  static int dsi_vc_send_long(struct intel_dsi *intel_dsi, int channel,
> -			    u8 data_type, const u8 *data, int len)
> +		u8 data_type, const u8 *data, int len, enum port port)
>  {
>  	struct drm_encoder *encoder = &intel_dsi->base.base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>  	u32 data_reg;
>  	int i, j, n;
>  	u32 mask;
> @@ -204,12 +197,12 @@ static int dsi_vc_send_long(struct intel_dsi *intel_dsi, int channel,
>  		 * dwords, then wait for not set, then continue. */
>  	}
>  
> -	return dsi_vc_send_short(intel_dsi, channel, data_type, len);
> +	return dsi_vc_send_short(intel_dsi, channel, data_type, len, port);
>  }
>  
>  static int dsi_vc_write_common(struct intel_dsi *intel_dsi,
>  			       int channel, const u8 *data, int len,
> -			       enum dsi_type type)
> +			       enum dsi_type type, enum port port)
>  {
>  	int ret;
>  
> @@ -217,50 +210,54 @@ static int dsi_vc_write_common(struct intel_dsi *intel_dsi,
>  		BUG_ON(type == DSI_GENERIC);
>  		ret = dsi_vc_send_short(intel_dsi, channel,
>  					MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM,
> -					0);
> +					0, port);
>  	} else if (len == 1) {
>  		ret = dsi_vc_send_short(intel_dsi, channel,
>  					type == DSI_GENERIC ?
>  					MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM :
> -					MIPI_DSI_DCS_SHORT_WRITE, data[0]);
> +					MIPI_DSI_DCS_SHORT_WRITE, data[0],
> +					port);
>  	} else if (len == 2) {
>  		ret = dsi_vc_send_short(intel_dsi, channel,
>  					type == DSI_GENERIC ?
>  					MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM :
>  					MIPI_DSI_DCS_SHORT_WRITE_PARAM,
> -					(data[1] << 8) | data[0]);
> +					(data[1] << 8) | data[0], port);
>  	} else {
>  		ret = dsi_vc_send_long(intel_dsi, channel,
> -				       type == DSI_GENERIC ?
> -				       MIPI_DSI_GENERIC_LONG_WRITE :
> -				       MIPI_DSI_DCS_LONG_WRITE, data, len);
> +					type == DSI_GENERIC ?
> +					MIPI_DSI_GENERIC_LONG_WRITE :
> +					MIPI_DSI_DCS_LONG_WRITE, data, len,
> +					port);
>  	}
>  
>  	return ret;
>  }
>  
>  int dsi_vc_dcs_write(struct intel_dsi *intel_dsi, int channel,
> -		     const u8 *data, int len)
> +		     const u8 *data, int len, enum port port)
>  {
> -	return dsi_vc_write_common(intel_dsi, channel, data, len, DSI_DCS);
> +	return dsi_vc_write_common(intel_dsi, channel, data, len, DSI_DCS,
> +									port);
>  }
>  
>  int dsi_vc_generic_write(struct intel_dsi *intel_dsi, int channel,
> -			 const u8 *data, int len)
> +			 const u8 *data, int len, enum port port)
>  {
> -	return dsi_vc_write_common(intel_dsi, channel, data, len, DSI_GENERIC);
> +	return dsi_vc_write_common(intel_dsi, channel, data, len, DSI_GENERIC,
> +									port);
>  }
>  
>  static int dsi_vc_dcs_send_read_request(struct intel_dsi *intel_dsi,
> -					int channel, u8 dcs_cmd)
> +				int channel, u8 dcs_cmd, enum port port)
>  {
>  	return dsi_vc_send_short(intel_dsi, channel, MIPI_DSI_DCS_READ,
> -				 dcs_cmd);
> +				 dcs_cmd, port);
>  }
>  
>  static int dsi_vc_generic_send_read_request(struct intel_dsi *intel_dsi,
>  					    int channel, u8 *reqdata,
> -					    int reqlen)
> +					    int reqlen, enum port port)
>  {
>  	u16 data;
>  	u8 data_type;
> @@ -282,17 +279,15 @@ static int dsi_vc_generic_send_read_request(struct intel_dsi *intel_dsi,
>  		BUG();
>  	}
>  
> -	return dsi_vc_send_short(intel_dsi, channel, data_type, data);
> +	return dsi_vc_send_short(intel_dsi, channel, data_type, data, port);
>  }
>  
>  static int dsi_read_data_return(struct intel_dsi *intel_dsi,
> -				u8 *buf, int buflen)
> +				u8 *buf, int buflen, enum port port)
>  {
>  	struct drm_encoder *encoder = &intel_dsi->base.base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>  	int i, len = 0;
>  	u32 data_reg, val;
>  
> @@ -312,13 +307,11 @@ static int dsi_read_data_return(struct intel_dsi *intel_dsi,
>  }
>  
>  int dsi_vc_dcs_read(struct intel_dsi *intel_dsi, int channel, u8 dcs_cmd,
> -		    u8 *buf, int buflen)
> +		    u8 *buf, int buflen, enum port port)
>  {
>  	struct drm_encoder *encoder = &intel_dsi->base.base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>  	u32 mask;
>  	int ret;
>  
> @@ -329,7 +322,7 @@ int dsi_vc_dcs_read(struct intel_dsi *intel_dsi, int channel, u8 dcs_cmd,
>  
>  	I915_WRITE(MIPI_INTR_STAT(port), GEN_READ_DATA_AVAIL);
>  
> -	ret = dsi_vc_dcs_send_read_request(intel_dsi, channel, dcs_cmd);
> +	ret = dsi_vc_dcs_send_read_request(intel_dsi, channel, dcs_cmd, port);
>  	if (ret)
>  		return ret;
>  
> @@ -337,7 +330,7 @@ int dsi_vc_dcs_read(struct intel_dsi *intel_dsi, int channel, u8 dcs_cmd,
>  	if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & mask) == mask, 50))
>  		DRM_ERROR("Timeout waiting for read data.\n");
>  
> -	ret = dsi_read_data_return(intel_dsi, buf, buflen);
> +	ret = dsi_read_data_return(intel_dsi, buf, buflen, port);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -348,13 +341,11 @@ int dsi_vc_dcs_read(struct intel_dsi *intel_dsi, int channel, u8 dcs_cmd,
>  }
>  
>  int dsi_vc_generic_read(struct intel_dsi *intel_dsi, int channel,
> -			u8 *reqdata, int reqlen, u8 *buf, int buflen)
> +		u8 *reqdata, int reqlen, u8 *buf, int buflen, enum port port)
>  {
>  	struct drm_encoder *encoder = &intel_dsi->base.base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
>  	u32 mask;
>  	int ret;
>  
> @@ -366,7 +357,7 @@ int dsi_vc_generic_read(struct intel_dsi *intel_dsi, int channel,
>  	I915_WRITE(MIPI_INTR_STAT(port), GEN_READ_DATA_AVAIL);
>  
>  	ret = dsi_vc_generic_send_read_request(intel_dsi, channel, reqdata,
> -					       reqlen);
> +					       reqlen, port);
>  	if (ret)
>  		return ret;
>  
> @@ -374,7 +365,7 @@ int dsi_vc_generic_read(struct intel_dsi *intel_dsi, int channel,
>  	if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & mask) == mask, 50))
>  		DRM_ERROR("Timeout waiting for read data.\n");
>  
> -	ret = dsi_read_data_return(intel_dsi, buf, buflen);
> +	ret = dsi_read_data_return(intel_dsi, buf, buflen, port);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.h b/drivers/gpu/drm/i915/intel_dsi_cmd.h
> index 46aa1ac..326a5ac 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_cmd.h
> +++ b/drivers/gpu/drm/i915/intel_dsi_cmd.h
> @@ -36,77 +36,81 @@
>  #define DPI_LP_MODE_EN	false
>  #define DPI_HS_MODE_EN	true
>  
> -void dsi_hs_mode_enable(struct intel_dsi *intel_dsi, bool enable);
> +void dsi_hs_mode_enable(struct intel_dsi *intel_dsi, bool enable,
> +						enum port port);
>  
>  int dsi_vc_dcs_write(struct intel_dsi *intel_dsi, int channel,
> -		     const u8 *data, int len);
> +		     const u8 *data, int len, enum port port);
>  
>  int dsi_vc_generic_write(struct intel_dsi *intel_dsi, int channel,
> -			 const u8 *data, int len);
> +			 const u8 *data, int len, enum port port);
>  
>  int dsi_vc_dcs_read(struct intel_dsi *intel_dsi, int channel, u8 dcs_cmd,
> -		    u8 *buf, int buflen);
> +		    u8 *buf, int buflen, enum port port);
>  
>  int dsi_vc_generic_read(struct intel_dsi *intel_dsi, int channel,
> -			u8 *reqdata, int reqlen, u8 *buf, int buflen);
> +		u8 *reqdata, int reqlen, u8 *buf, int buflen, enum port port);
>  
>  int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs);
>  void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi);
>  
>  /* XXX: questionable write helpers */
>  static inline int dsi_vc_dcs_write_0(struct intel_dsi *intel_dsi,
> -				     int channel, u8 dcs_cmd)
> +				     int channel, u8 dcs_cmd, enum port port)
>  {
> -	return dsi_vc_dcs_write(intel_dsi, channel, &dcs_cmd, 1);
> +	return dsi_vc_dcs_write(intel_dsi, channel, &dcs_cmd, 1, port);
>  }
>  
>  static inline int dsi_vc_dcs_write_1(struct intel_dsi *intel_dsi,
> -				     int channel, u8 dcs_cmd, u8 param)
> +			int channel, u8 dcs_cmd, u8 param, enum port port)
>  {
>  	u8 buf[2] = { dcs_cmd, param };
> -	return dsi_vc_dcs_write(intel_dsi, channel, buf, 2);
> +	return dsi_vc_dcs_write(intel_dsi, channel, buf, 2, port);
>  }
>  
>  static inline int dsi_vc_generic_write_0(struct intel_dsi *intel_dsi,
> -					 int channel)
> +					 int channel, enum port port)
>  {
> -	return dsi_vc_generic_write(intel_dsi, channel, NULL, 0);
> +	return dsi_vc_generic_write(intel_dsi, channel, NULL, 0, port);
>  }
>  
>  static inline int dsi_vc_generic_write_1(struct intel_dsi *intel_dsi,
> -					 int channel, u8 param)
> +					 int channel, u8 param, enum port port)
>  {
> -	return dsi_vc_generic_write(intel_dsi, channel, &param, 1);
> +	return dsi_vc_generic_write(intel_dsi, channel, &param, 1, port);
>  }
>  
>  static inline int dsi_vc_generic_write_2(struct intel_dsi *intel_dsi,
> -					 int channel, u8 param1, u8 param2)
> +			int channel, u8 param1, u8 param2, enum port port)
>  {
>  	u8 buf[2] = { param1, param2 };
> -	return dsi_vc_generic_write(intel_dsi, channel, buf, 2);
> +	return dsi_vc_generic_write(intel_dsi, channel, buf, 2, port);
>  }
>  
>  /* XXX: questionable read helpers */
>  static inline int dsi_vc_generic_read_0(struct intel_dsi *intel_dsi,
> -					int channel, u8 *buf, int buflen)
> +			int channel, u8 *buf, int buflen, enum port port)
>  {
> -	return dsi_vc_generic_read(intel_dsi, channel, NULL, 0, buf, buflen);
> +	return dsi_vc_generic_read(intel_dsi, channel, NULL, 0, buf, buflen,
> +									port);
>  }
>  
>  static inline int dsi_vc_generic_read_1(struct intel_dsi *intel_dsi,
>  					int channel, u8 param, u8 *buf,
> -					int buflen)
> +					int buflen, enum port port)
>  {
> -	return dsi_vc_generic_read(intel_dsi, channel, &param, 1, buf, buflen);
> +	return dsi_vc_generic_read(intel_dsi, channel, &param, 1, buf, buflen,
> +									port);
>  }
>  
>  static inline int dsi_vc_generic_read_2(struct intel_dsi *intel_dsi,
>  					int channel, u8 param1, u8 param2,
> -					u8 *buf, int buflen)
> +					u8 *buf, int buflen, enum port port)
>  {
>  	u8 req[2] = { param1, param2 };
>  
> -	return dsi_vc_generic_read(intel_dsi, channel, req, 2, buf, buflen);
> +	return dsi_vc_generic_read(intel_dsi, channel, req, 2, buf, buflen,
> +									port);
>  }
>  
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index f6bdd44..612592f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -94,16 +94,23 @@ static struct gpio_table gtable[] = {
>  	{ GPIO_NC_11_PCONF0, GPIO_NC_11_PAD, 0}
>  };
>  
> +static inline enum port intel_dsi_seq_port_to_port(u8 port)
> +{
> +	return port ? PORT_C : PORT_A;
> +}
> +
>  static u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, u8 *data)
>  {
>  	u8 type, byte, mode, vc, port;

Please rename this port to seq_port or whatever describes it best.

>  	u16 len;
> +	enum port port_no;

And rename this to port, so "port" really means the hw port all around
the driver.

>  	byte = *data++;
>  	mode = (byte >> MIPI_TRANSFER_MODE_SHIFT) & 0x1;
>  	vc = (byte >> MIPI_VIRTUAL_CHANNEL_SHIFT) & 0x3;
>  	port = (byte >> MIPI_PORT_SHIFT) & 0x3;
>  
> +	port_no = intel_dsi_seq_port_to_port(port);
>  	/* LP or HS mode */
>  	intel_dsi->hs = mode;
>  
> @@ -115,13 +122,13 @@ static u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, u8 *data)
>  
>  	switch (type) {
>  	case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM:
> -		dsi_vc_generic_write_0(intel_dsi, vc);
> +		dsi_vc_generic_write_0(intel_dsi, vc, port);

And once you've made the renames above, this and the calls below will
actually use the correct port!

BR,
Jani.

>  		break;
>  	case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM:
> -		dsi_vc_generic_write_1(intel_dsi, vc, *data);
> +		dsi_vc_generic_write_1(intel_dsi, vc, *data, port);
>  		break;
>  	case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
> -		dsi_vc_generic_write_2(intel_dsi, vc, *data, *(data + 1));
> +		dsi_vc_generic_write_2(intel_dsi, vc, *data, *(data + 1), port);
>  		break;
>  	case MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM:
>  	case MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM:
> @@ -129,19 +136,19 @@ static u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, u8 *data)
>  		DRM_DEBUG_DRIVER("Generic Read not yet implemented or used\n");
>  		break;
>  	case MIPI_DSI_GENERIC_LONG_WRITE:
> -		dsi_vc_generic_write(intel_dsi, vc, data, len);
> +		dsi_vc_generic_write(intel_dsi, vc, data, len, port);
>  		break;
>  	case MIPI_DSI_DCS_SHORT_WRITE:
> -		dsi_vc_dcs_write_0(intel_dsi, vc, *data);
> +		dsi_vc_dcs_write_0(intel_dsi, vc, *data, port);
>  		break;
>  	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
> -		dsi_vc_dcs_write_1(intel_dsi, vc, *data, *(data + 1));
> +		dsi_vc_dcs_write_1(intel_dsi, vc, *data, *(data + 1), port);
>  		break;
>  	case MIPI_DSI_DCS_READ:
>  		DRM_DEBUG_DRIVER("DCS Read not yet implemented or used\n");
>  		break;
>  	case MIPI_DSI_DCS_LONG_WRITE:
> -		dsi_vc_dcs_write(intel_dsi, vc, data, len);
> +		dsi_vc_dcs_write(intel_dsi, vc, data, len, port);
>  		break;
>  	}
>  
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx