RE: [PATCH] drm/bridge/synopsys: dsi: read status error during transfer

[Angelo Ribeiro]

Hi Yannick,

Could you please share why you are adding this retry?
Are you trying to solve an issue with the generic interface buffs/fifos 
overflow?

I think that it could be nice read the CMD_PKT_STATUS register to ensure 
that buffs/fifos are not full or empty before a read or write.

> From: Yannick Fertre <yannick.fertre@xxxxxx>
> Date: Wed, Nov 27, 2019 at 10:23:13
>
> From: Yannick Fertrà <yannick.fertre@xxxxxx>
> 
> Read the DSI_INT_ST1 status register to check if
> errors occur while reading/writing a command to panel.
> In case of error, the transfer is retried 3 times.
> 
> Signed-off-by: Yannick Fertre <yannick.fertre@xxxxxx>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 99 +++++++++++++++++++++++----
>  1 file changed, 85 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index b6e793b..cc806ba 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -212,6 +212,20 @@
>  
>  #define DSI_INT_ST0			0xbc
>  #define DSI_INT_ST1			0xc0
> +#define GPRXE				BIT(12)
> +#define GPRDE				BIT(11)
> +#define GPTXE				BIT(10)
> +#define GPWRE				BIT(9)
> +#define GCWRE				BIT(8)
> +#define DPIPLDWE			BIT(7)
> +#define EOTPE				BIT(6)
> +#define PSE				BIT(5)
> +#define CRCE				BIT(4)
> +#define ECCME				BIT(3)
> +#define ECCSE				BIT(2)
> +#define TOLPRX				BIT(1)
> +#define TOHSTX				BIT(0)
> +
>  #define DSI_INT_MSK0			0xc4
>  #define DSI_INT_MSK1			0xc8
>  
> @@ -397,6 +411,42 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
>  	return 0;
>  }
>  
> +static int dw_mipi_dsi_read_status(struct dw_mipi_dsi *dsi)
> +{
> +	u32 val;
> +
> +	val = dsi_read(dsi, DSI_INT_ST1);
> +
> +	if (val & GPRXE)
> +		DRM_DEBUG_DRIVER("DSI Generic payload receive error\n");
> +	if (val & GPRDE)
> +		DRM_DEBUG_DRIVER("DSI Generic payload read error\n");
> +	if (val & GPTXE)
> +		DRM_DEBUG_DRIVER("DSI Generic payload transmit error\n");
> +	if (val & GPWRE)
> +		DRM_DEBUG_DRIVER("DSI Generic payload write error\n");
> +	if (val & GCWRE)
> +		DRM_DEBUG_DRIVER("DSI Generic command write error\n");
> +	if (val & DPIPLDWE)
> +		DRM_DEBUG_DRIVER("DSI DPI payload write error\n");
> +	if (val & EOTPE)
> +		DRM_DEBUG_DRIVER("DSI EoTp error\n");
> +	if (val & PSE)
> +		DRM_DEBUG_DRIVER("DSI Packet size error\n");
> +	if (val & CRCE)
> +		DRM_DEBUG_DRIVER("DSI CRC error\n");
> +	if (val & ECCME)
> +		DRM_DEBUG_DRIVER("DSI ECC multi-bit error\n");
> +	if (val & ECCSE)
> +		DRM_DEBUG_DRIVER("DSI ECC single-bit error\n");
> +	if (val & TOLPRX)
> +		DRM_DEBUG_DRIVER("DSI Timeout low-power reception\n");
> +	if (val & TOHSTX)
> +		DRM_DEBUG_DRIVER("DSI Timeout high-speed transmission\n");
> +
> +	return val;
> +}
> +
>  static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
>  			     const struct mipi_dsi_packet *packet)
>  {
> @@ -426,6 +476,12 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
>  				"failed to get available write payload FIFO\n");
>  			return ret;
>  		}
> +
> +		val = dw_mipi_dsi_read_status(dsi);
> +		if (val) {
> +			dev_err(dsi->dev, "dsi status error 0x%0x\n", val);
> +			return -EINVAL;
> +		}
>  	}
>  
>  	word = 0;
> @@ -459,6 +515,12 @@ static int dw_mipi_dsi_read(struct dw_mipi_dsi *dsi,
>  			return ret;
>  		}
>  
> +		val = dw_mipi_dsi_read_status(dsi);
> +		if (val) {
> +			dev_err(dsi->dev, "dsi status error 0x%0x\n", val);
> +			return -EINVAL;
> +		}
> +
>  		val = dsi_read(dsi, DSI_GEN_PLD_DATA);
>  		for (j = 0; j < 4 && j + i < len; j++)
>  			buf[i + j] = val >> (8 * j);
> @@ -473,6 +535,7 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>  	struct dw_mipi_dsi *dsi = host_to_dsi(host);
>  	struct mipi_dsi_packet packet;
>  	int ret, nb_bytes;
> +	int retry = 3;
>  
>  	ret = mipi_dsi_create_packet(&packet, msg);
>  	if (ret) {
> @@ -484,24 +547,32 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>  	if (dsi->slave)
>  		dw_mipi_message_config(dsi->slave, msg);
>  
> -	ret = dw_mipi_dsi_write(dsi, &packet);
> -	if (ret)
> -		return ret;
> -	if (dsi->slave) {
> -		ret = dw_mipi_dsi_write(dsi->slave, &packet);
> +	while (retry--) {
> +		ret = dw_mipi_dsi_write(dsi, &packet);
>  		if (ret)
> -			return ret;
> -	}
> +			continue;
>  
> -	if (msg->rx_buf && msg->rx_len) {
> -		ret = dw_mipi_dsi_read(dsi, msg);
> -		if (ret)
> -			return ret;
> -		nb_bytes = msg->rx_len;
> -	} else {
> -		nb_bytes = packet.size;
> +		if (dsi->slave) {
> +			ret = dw_mipi_dsi_write(dsi->slave, &packet);
> +			if (ret)
> +				continue;
> +		}
> +
> +		if (msg->rx_buf && msg->rx_len) {
> +			ret = dw_mipi_dsi_read(dsi, msg);
> +			if (ret)
> +				continue;
> +			nb_bytes = msg->rx_len;
> +
> +		} else {
> +			nb_bytes = packet.size;
> +		}
> +		break;
>  	}
>  
> +	if (ret)
> +		return ret;
> +
>  	return nb_bytes;
>  }
>  
> -- 
> 2.7.4
 
Angelo Ribeiro
Software Engineer, Synopsys, Inc.
angelo.ribeiro@xxxxxxxxxxxx
Work: (+351)220 977 109

 _______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=qo_6hx-85FdiWxFnlP_7O-HosW3LyWP6_23ipK8AVDE&m=6HesTnV8KBATeAUwGjP83TkzKbOxOqUYz3WSV3Q6u9g&s=TSaHlviW61V_jSga9_rAicgdZRnvyIll7JR8Uncj53s&e=