Re: [PATCH] drm/ast: Fixed system hanged if disable P2A

[Benjamin Herrenschmidt <benh@au1.ibm.com>]

On Thu, 2017-01-26 at 09:45 +0800, Y.C. Chen wrote:
> From: "Y.C. Chen" <yc_chen@aspeedtech.com>
> 
> The original ast driver will access some BMC configuration through
> P2A bridge
> that can be disabled since AST2300 and after.
> It will cause system hanged if P2A bridge is disabled.
> Here is the update to fix it.

Will this work for us on POWER ? The problem we observed is that if P2A
is disabled, the read from f00x seems to not respond, ie, we get a PCIe
target abort on the MMIO, not 0xffff_ffff. That trips our HW error
detection mechanism "EEH" which will take out the entire device.

Russell (CC) proposed a patch a while ago that seems to have gone
nowhere that allows the driver to retrieve all the information it
needs from the device-tree on platforms that support it.

That allows us to grab the necessary info from firmware before the
bridge is closed (or via some specific BMC interface we have if we
close the bridge) and pass it to Linux.

Any reason that patch hasn't been merged ? Dave ?

Cheers,
Ben.


> Signed-off-by: Y.C. Chen <yc_chen@aspeedtech.com>
> ---
>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
>  drivers/gpu/drm/ast/ast_main.c | 156 ++++++++++++++++++++++---------
> ----------
>  drivers/gpu/drm/ast/ast_post.c |  18 +++--
>  3 files changed, 96 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h
> b/drivers/gpu/drm/ast/ast_drv.h
> index 908011d..7abda94 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -113,6 +113,7 @@ struct ast_private {
>  	struct ttm_bo_kmap_obj cache_kmap;
>  	int next_cursor;
>  	bool support_wide_screen;
> +	bool DisableP2A;
>  
>  	enum ast_tx_chip tx_chip_type;
>  	u8 dp501_maxclk;
> diff --git a/drivers/gpu/drm/ast/ast_main.c
> b/drivers/gpu/drm/ast/ast_main.c
> index f75c642..c374685 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -124,6 +124,11 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>  	} else
>  		*need_post = false;
>  
> +	/* Check P2A Access */
> +	ast->DisableP2A = true;
> +	data = ast_read32(ast, 0xf004);
> +	if (data != 0xFFFFFFFF) ast->DisableP2A = false;
> +
>  	/* Check if we support wide screen */
>  	switch (ast->chip) {
>  	case AST1180:
> @@ -140,15 +145,17 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>  			ast->support_wide_screen = true;
>  		else {
>  			ast->support_wide_screen = false;
> -			/* Read SCU7c (silicon revision register) */
> -			ast_write32(ast, 0xf004, 0x1e6e0000);
> -			ast_write32(ast, 0xf000, 0x1);
> -			data = ast_read32(ast, 0x1207c);
> -			data &= 0x300;
> -			if (ast->chip == AST2300 && data == 0x0) /*
> ast1300 */
> -				ast->support_wide_screen = true;
> -			if (ast->chip == AST2400 && data == 0x100)
> /* ast1400 */
> -				ast->support_wide_screen = true;
> +			if (ast->DisableP2A == false) {
> +				/* Read SCU7c (silicon revision
> register) */
> +				ast_write32(ast, 0xf004,
> 0x1e6e0000);
> +				ast_write32(ast, 0xf000, 0x1);
> +				data = ast_read32(ast, 0x1207c);
> +				data &= 0x300;
> +				if (ast->chip == AST2300 && data ==
> 0x0) /* ast1300 */
> +					ast->support_wide_screen =
> true;
> +				if (ast->chip == AST2400 && data ==
> 0x100) /* ast1400 */
> +					ast->support_wide_screen =
> true;
> +			}
>  		}
>  		break;
>  	}
> @@ -216,80 +223,81 @@ static int ast_get_dram_info(struct drm_device
> *dev)
>  	uint32_t data, data2;
>  	uint32_t denum, num, div, ref_pll;
>  
> -	ast_write32(ast, 0xf004, 0x1e6e0000);
> -	ast_write32(ast, 0xf000, 0x1);
> -
> -
> -	ast_write32(ast, 0x10000, 0xfc600309);
> -
> -	do {
> -		if (pci_channel_offline(dev->pdev))
> -			return -EIO;
> -	} while (ast_read32(ast, 0x10000) != 0x01);
> -	data = ast_read32(ast, 0x10004);
> -
> -	if (data & 0x40)
> +	if (ast->DisableP2A)
> +	{
>  		ast->dram_bus_width = 16;
> +		ast->dram_type = AST_DRAM_1Gx16;
> +		ast->mclk = 396;
> +	}
>  	else
> -		ast->dram_bus_width = 32;
> +	{
> +		ast_write32(ast, 0xf004, 0x1e6e0000);
> +		ast_write32(ast, 0xf000, 0x1);
> +		data = ast_read32(ast, 0x10004);
> +
> +		if (data & 0x40)
> +			ast->dram_bus_width = 16;
> +		else
> +			ast->dram_bus_width = 32;
> +
> +		if (ast->chip == AST2300 || ast->chip == AST2400) {
> +			switch (data & 0x03) {
> +			case 0:
> +				ast->dram_type = AST_DRAM_512Mx16;
> +				break;
> +			default:
> +			case 1:
> +				ast->dram_type = AST_DRAM_1Gx16;
> +				break;
> +			case 2:
> +				ast->dram_type = AST_DRAM_2Gx16;
> +				break;
> +			case 3:
> +				ast->dram_type = AST_DRAM_4Gx16;
> +				break;
> +			}
> +		} else {
> +			switch (data & 0x0c) {
> +			case 0:
> +			case 4:
> +				ast->dram_type = AST_DRAM_512Mx16;
> +				break;
> +			case 8:
> +				if (data & 0x40)
> +					ast->dram_type =
> AST_DRAM_1Gx16;
> +				else
> +					ast->dram_type =
> AST_DRAM_512Mx32;
> +				break;
> +			case 0xc:
> +				ast->dram_type = AST_DRAM_1Gx32;
> +				break;
> +			}
> +		}
>  
> -	if (ast->chip == AST2300 || ast->chip == AST2400) {
> -		switch (data & 0x03) {
> -		case 0:
> -			ast->dram_type = AST_DRAM_512Mx16;
> -			break;
> -		default:
> -		case 1:
> -			ast->dram_type = AST_DRAM_1Gx16;
> -			break;
> -		case 2:
> -			ast->dram_type = AST_DRAM_2Gx16;
> -			break;
> +		data = ast_read32(ast, 0x10120);
> +		data2 = ast_read32(ast, 0x10170);
> +		if (data2 & 0x2000)
> +			ref_pll = 14318;
> +		else
> +			ref_pll = 12000;
> +
> +		denum = data & 0x1f;
> +		num = (data & 0x3fe0) >> 5;
> +		data = (data & 0xc000) >> 14;
> +		switch (data) {
>  		case 3:
> -			ast->dram_type = AST_DRAM_4Gx16;
> -			break;
> -		}
> -	} else {
> -		switch (data & 0x0c) {
> -		case 0:
> -		case 4:
> -			ast->dram_type = AST_DRAM_512Mx16;
> +			div = 0x4;
>  			break;
> -		case 8:
> -			if (data & 0x40)
> -				ast->dram_type = AST_DRAM_1Gx16;
> -			else
> -				ast->dram_type = AST_DRAM_512Mx32;
> +		case 2:
> +		case 1:
> +			div = 0x2;
>  			break;
> -		case 0xc:
> -			ast->dram_type = AST_DRAM_1Gx32;
> +		default:
> +			div = 0x1;
>  			break;
>  		}
> +		ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div
> * 1000);
>  	}
> -
> -	data = ast_read32(ast, 0x10120);
> -	data2 = ast_read32(ast, 0x10170);
> -	if (data2 & 0x2000)
> -		ref_pll = 14318;
> -	else
> -		ref_pll = 12000;
> -
> -	denum = data & 0x1f;
> -	num = (data & 0x3fe0) >> 5;
> -	data = (data & 0xc000) >> 14;
> -	switch (data) {
> -	case 3:
> -		div = 0x4;
> -		break;
> -	case 2:
> -	case 1:
> -		div = 0x2;
> -		break;
> -	default:
> -		div = 0x1;
> -		break;
> -	}
> -	ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div *
> 1000);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/ast/ast_post.c
> b/drivers/gpu/drm/ast/ast_post.c
> index 810c51d..5331ee1 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -379,12 +379,20 @@ void ast_post_gpu(struct drm_device *dev)
>  	ast_open_key(ast);
>  	ast_set_def_ext_reg(dev);
>  
> -	if (ast->chip == AST2300 || ast->chip == AST2400)
> -		ast_init_dram_2300(dev);
> -	else
> -		ast_init_dram_reg(dev);
> +	if (ast->DisableP2A == false)
> +	{
> +		if (ast->chip == AST2300 || ast->chip == AST2400)
> +			ast_init_dram_2300(dev);
> +		else
> +			ast_init_dram_reg(dev);
>  
> -	ast_init_3rdtx(dev);
> +		ast_init_3rdtx(dev);
> +	}
> +	else
> +	{
> +		if (ast->tx_chip_type != AST_TX_NONE)
> +			ast_set_index_reg_mask(ast,
> AST_IO_CRTC_PORT, 0xa3, 0xcf, 0x80);	/* Enable DVO */
> +	}
>  }
>  
>  /* AST 2300 DRAM settings */

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel