Re: [PATCH 1/9] drm/ast: Handle configuration without P2A bridge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2017-02-17 at 16:32 +1100, Benjamin Herrenschmidt wrote:
> The ast driver configures a window to enable access into BMC
> memory space in order to read some configuration registers.

Aspeed suggested a couple of refinements for some chipsets,
so I'll respin either this week-end or monday.

> If this window is disabled, which it can be from the BMC side,
> the ast driver can't function.
> 
> Closing this window is a necessity for security if a machine's
> host side and BMC side are controlled by different parties;
> i.e. a cloud provider offering machines "bare metal".
> 
> A recent patch went in to try to check if that window is open
> but it does so by trying to access the registers in question
> and testing if the result is 0xffffffff.
> 
> This method will trigger a PCIe error when the window is closed
> which on some systems will be fatal (it will trigger an EEH
> for example on POWER which will take out the device).
> 
> This patch improves this in two ways:
> 
>  - First, if the firmware has put properties in the device-tree
> containing the relevant configuration information, we use these.
> 
>  - Otherwise, a bit in one of the SCU scratch registers (which
> are readable via the VGA register space and writeable by the BMC)
> will indicate if the BMC has closed the window. This bit has been
> defined by Y.C Chen from Aspeed.
> 
> If the window is closed and the configuration isn't available from
> the device-tree, some sane defaults are used. Those defaults are
> hopefully sufficient for standard video modes used on a server.
> 
> Signed-off-by: Russell Currey <ruscur@xxxxxxxxxx>
> Signed-off-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> --
> 
> v2. [BenH]
>     - Reworked on top of Aspeed P2A patch
>     - Cleanup overall detection via a "config_mode" and log the
>       selected mode for diagnostics purposes
>     - Add a property for the SCU straps
> 
> v3. [BenH]
>     - Moved the config mode detection to a separate functionn
>     - Add reading of SCU 0x40 D[12] to detect the window is
>       closed as to not trigger a bus error by just "trying".
>       (change provided by Y.C. Chen)
> ---
>  drivers/gpu/drm/ast/ast_drv.h  |   6 +-
>  drivers/gpu/drm/ast/ast_main.c | 223 +++++++++++++++++++++++++----
> ------------
>  drivers/gpu/drm/ast/ast_post.c |   7 +-
>  3 files changed, 141 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h
> b/drivers/gpu/drm/ast/ast_drv.h
> index 7abda94..3bedcf7 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -113,7 +113,11 @@ struct ast_private {
>  	struct ttm_bo_kmap_obj cache_kmap;
>  	int next_cursor;
>  	bool support_wide_screen;
> -	bool DisableP2A;
> +	enum {
> +		ast_use_p2a,
> +		ast_use_dt,
> +		ast_use_defaults
> +	} config_mode;
>  
>  	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 533e762..823c68f 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -62,13 +62,58 @@ uint8_t ast_get_index_reg_mask(struct ast_private
> *ast,
>  	return ret;
>  }
>  
> +static void ast_detect_config_mode(struct drm_device *dev, u32
> *scu_rev)
> +{
> +	struct device_node *np = dev->pdev->dev.of_node;
> +	struct ast_private *ast = dev->dev_private;
> +	uint32_t data, jreg;
> +
> +	/* Check if we have device-tree properties */
> +	if (np && !of_property_read_u32(np, "ast,scu-revision-id",
> scu_rev)) {
> +		/* We do, disable P2A access */
> +		ast->config_mode = ast_use_dt;
> +		DRM_INFO("Using device-tree for configuration\n");
> +		return;
> +	}
> +
> +	/*
> +	 * The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
> +	 * is disabled
> +	 */
> +	jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1,
> 0xff);
> +	if (!(jreg & 0x10)) {
> +		/* Double check it's actually working */
> +		data = ast_read32(ast, 0xf004);
> +		if (data != 0xFFFFFFFF) {
> +			/* P2A works, grab silicon revision */
> +			ast->config_mode = ast_use_p2a;
> +
> +			DRM_INFO("Using P2A bridge for
> configuration\n");
> +
> +			/* Read SCU7c (silicon revision register) */
> +			ast_write32(ast, 0xf004, 0x1e6e0000);
> +			ast_write32(ast, 0xf000, 0x1);
> +			*scu_rev = ast_read32(ast, 0x1207c);
> +			return;
> +		}
> +	}
> +
> +	DRM_INFO("P2A bridge disabled, using default
> configuration\n");
> +	ast->config_mode = ast_use_defaults;
> +	*scu_rev = 0xffffffff;
> +}
>  
>  static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>  {
>  	struct ast_private *ast = dev->dev_private;
> -	uint32_t data, jreg;
> +	uint32_t jreg, scu_rev;
> +
>  	ast_open_key(ast);
>  
> +	/* Find out whether P2A works or whether to use device-tree
> */
> +	ast_detect_config_mode(dev, &scu_rev);
> +
> +	/* Identify chipset */
>  	if (dev->pdev->device == PCI_CHIP_AST1180) {
>  		ast->chip = AST1100;
>  		DRM_INFO("AST 1180 detected\n");
> @@ -80,12 +125,7 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>  			ast->chip = AST2300;
>  			DRM_INFO("AST 2300 detected\n");
>  		} else if (dev->pdev->revision >= 0x10) {
> -			uint32_t data;
> -			ast_write32(ast, 0xf004, 0x1e6e0000);
> -			ast_write32(ast, 0xf000, 0x1);
> -
> -			data = ast_read32(ast, 0x1207c);
> -			switch (data & 0x0300) {
> +			switch (scu_rev & 0x0300) {
>  			case 0x0200:
>  				ast->chip = AST1100;
>  				DRM_INFO("AST 1100 detected\n");
> @@ -124,12 +164,6 @@ 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:
> @@ -146,17 +180,12 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>  			ast->support_wide_screen = true;
>  		else {
>  			ast->support_wide_screen = false;
> -			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;
> -			}
> +			if (ast->chip == AST2300 &&
> +			    (scu_rev & 0x300) == 0x0) /* ast1300 */
> +				ast->support_wide_screen = true;
> +			if (ast->chip == AST2400 &&
> +			    (scu_rev & 0x300) == 0x100) /* ast1400
> */
> +				ast->support_wide_screen = true;
>  		}
>  		break;
>  	}
> @@ -220,85 +249,101 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>  
>  static int ast_get_dram_info(struct drm_device *dev)
>  {
> +	struct device_node *np = dev->pdev->dev.of_node;
>  	struct ast_private *ast = dev->dev_private;
> -	uint32_t data, data2;
> -	uint32_t denum, num, div, ref_pll;
> +	uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
> +	uint32_t denum, num, div, ref_pll, dsel;
>  
> -	if (ast->DisableP2A)
> -	{
> +	switch (ast->config_mode) {
> +	case ast_use_dt:
> +		/*
> +		 * If some properties are missing, use reasonable
> +		 * defaults for AST2400
> +		 */
> +		if (of_property_read_u32(np, "ast,mcr-
> configuration", &mcr_cfg))
> +			mcr_cfg = 0x00000577;
> +		if (of_property_read_u32(np, "ast,ast,mcr-scu-mpll",
> +					 &mcr_scu_mpll))
> +			mcr_scu_mpll = 0x000050C0;
> +		if (of_property_read_u32(np, "ast,ast,mcr-scu-
> strap",
> +					 &mcr_scu_strap))
> +			mcr_scu_strap = 0;
> +		break;
> +	case ast_use_p2a:
> +		ast_write32(ast, 0xf004, 0x1e6e0000);
> +		ast_write32(ast, 0xf000, 0x1);
> +		mcr_cfg = ast_read32(ast, 0x10004);
> +		mcr_scu_mpll = ast_read32(ast, 0x10120);
> +		mcr_scu_strap = ast_read32(ast, 0x10170);
> +		break;
> +	case ast_use_defaults:
> +	default:
>  		ast->dram_bus_width = 16;
>  		ast->dram_type = AST_DRAM_1Gx16;
>  		ast->mclk = 396;
> +		return 0;
>  	}
> -	else
> -	{
> -		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 (mcr_cfg & 0x40)
> +		ast->dram_bus_width = 16;
> +	else
> +		ast->dram_bus_width = 32;
>  
> -		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;
> +	if (ast->chip == AST2300 || ast->chip == AST2400) {
> +		switch (mcr_cfg & 0x03) {
> +		case 0:
> +			ast->dram_type = AST_DRAM_512Mx16;
>  			break;
> -		case 2:
> +		default:
>  		case 1:
> -			div = 0x2;
> +			ast->dram_type = AST_DRAM_1Gx16;
>  			break;
> -		default:
> -			div = 0x1;
> +		case 2:
> +			ast->dram_type = AST_DRAM_2Gx16;
> +			break;
> +		case 3:
> +			ast->dram_type = AST_DRAM_4Gx16;
>  			break;
>  		}
> -		ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div
> * 1000);
> +	} else {
> +		switch (mcr_cfg & 0x0c) {
> +		case 0:
> +		case 4:
> +			ast->dram_type = AST_DRAM_512Mx16;
> +			break;
> +		case 8:
> +			if (mcr_cfg & 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 (mcr_scu_strap & 0x2000)
> +		ref_pll = 14318;
> +	else
> +		ref_pll = 12000;
> +
> +	denum = mcr_scu_mpll & 0x1f;
> +	num = (mcr_scu_mpll & 0x3fe0) >> 5;
> +	dsel = (mcr_scu_mpll & 0xc000) >> 14;
> +	switch (dsel) {
> +	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 5331ee1..7197635 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -379,17 +379,14 @@ void ast_post_gpu(struct drm_device *dev)
>  	ast_open_key(ast);
>  	ast_set_def_ext_reg(dev);
>  
> -	if (ast->DisableP2A == false)
> -	{
> +	if (ast->config_mode == ast_use_p2a) {
>  		if (ast->chip == AST2300 || ast->chip == AST2400)
>  			ast_init_dram_2300(dev);
>  		else
>  			ast_init_dram_reg(dev);
>  
>  		ast_init_3rdtx(dev);
> -	}
> -	else
> -	{
> +	} else {
>  		if (ast->tx_chip_type != AST_TX_NONE)
>  			ast_set_index_reg_mask(ast,
> AST_IO_CRTC_PORT, 0xa3, 0xcf, 0x80);	/* Enable DVO */
>  	}
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux