[coreboot] Patch set updated for coreboot: 7f2d27b rs780: hide unused gpp ports

[She, Kerry]

Dear Paul

> -----Original Message-----
> From: coreboot-bounces+kerry.she=amd.com at coreboot.org [mailto:coreboot-
> bounces+kerry.she=amd.com at coreboot.org] On Behalf Of Paul Menzel
> Sent: Tuesday, September 13, 2011 6:25 PM
> To: coreboot at coreboot.org
> Cc: Kerry She
> Subject: Re: [coreboot] Patch set updated for coreboot: 7f2d27b rs780:
> hide unused gpp ports
> 
> Dear Kerry,
> 
> 
> Am Dienstag, den 13.09.2011, 12:08 +0200 schrieb Kerry Sheh:
> > Kerry Sheh (shekairui at gmail.com) just uploaded a new patch set to
> > gerrit, which you can find at http://review.coreboot.org/206
> 
> is there a typo in your Gerrit user information: »Sheh«?
I have update the profile :)

> > -gerrit
> >
> > commit 7f2d27b0bddf97948fff2f81fab52633e0f77709
> > Author: Kerry She <shekairui at gmail.com>
> > Date:   Tue Sep 13 18:29:27 2011 +0800
> >
> >     rs780: hide unused gpp ports
> >
> >     hide unused gpp ports, test on avalue/eax-785e
> 
> Copying the commit summary into the commit message body does not have any
> benefits.
> 
> Could you update the commit message using `git commit --amend`?
> 
> 1. Why is hiding these ports needed? Did you get an error message?
the information get by Lspci -vvv may not accurate if no device on the pcie port, such as the link speed etc.

> 2. Looking at the code, does this patch do more than is written in the
> commit message? What does for example `AtiPcieCfg.PortDetect |= 1 << 2;
> /* Port 2 */` do? Is that also hiding these ports? (I am sorry for this
> noob question.) If this is doing something else please add that to the
> commit message or even better split the commit up to make it even smaller.
This because the pcie training code for gfx port 2 is hardcoded,
If the training successful, we make a mark on the PortDect bitmap.
I think we will improve the magic number later.

Thanks for your elaborative review.
Kerry
  
> 
> 
> Thanks,
> 
> Paul
> 
> 
> >     Change-Id: Iaabfd362a0a01f21d0f49aa2bd2d26f9259013fb
> >     Signed-off-by: Kerry She <kerry.she at amd.com>
> >     Signed-off-by: Kerry She <shekairui at gmail.com>
> > ---
> >  src/southbridge/amd/rs780/gfx.c   |    2 ++
> >  src/southbridge/amd/rs780/pcie.c  |   13 +++++++++++++
> >  src/southbridge/amd/rs780/rs780.c |    5 ++++-
> >  src/southbridge/amd/rs780/rs780.h |    1 +
> >  4 files changed, 20 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/southbridge/amd/rs780/gfx.c
> > b/src/southbridge/amd/rs780/gfx.c index 9262bb9..3c06d44 100644
> > --- a/src/southbridge/amd/rs780/gfx.c
> > +++ b/src/southbridge/amd/rs780/gfx.c
> > @@ -1009,6 +1009,7 @@ static void single_port_configuration(device_t
> nb_dev, device_t dev)
> >  			set_nbmisc_enable_bits(nb_dev, 0x7, 1 << 3, 1 << 3);
> >  		}
> >  	} else {		/* step 13.b Link Training was successful */
> > +		AtiPcieCfg.PortDetect |= 1 << 2; /* Port 2 */
> >  		set_pcie_enable_bits(dev, 0xA2, 0xFF, 0x1);
> >  		reg32 = nbpcie_p_read_index(dev, 0x29);
> >  		width = reg32 & 0xFF;
> > @@ -1064,6 +1065,7 @@ static void dual_port_configuration(device_t
> nb_dev, device_t dev)
> >  		set_nbmisc_enable_bits(nb_dev, 0x0c, 1 << dev_ind, 1 <<
> dev_ind);
> >
> >  	} else {		/* step 16.b Link Training was successful */
> > +		AtiPcieCfg.PortDetect |= 1 << dev_ind;
> >  		reg32 = nbpcie_p_read_index(dev, 0xa2);
> >  		width = (reg32 >> 4) & 0x7;
> >  		printk(BIOS_DEBUG, "GFX LC_LINK_WIDTH = 0x%x.\n", width);
> diff
> > --git a/src/southbridge/amd/rs780/pcie.c
> > b/src/southbridge/amd/rs780/pcie.c
> > index 9cbd832..5e2d985 100644
> > --- a/src/southbridge/amd/rs780/pcie.c
> > +++ b/src/southbridge/amd/rs780/pcie.c
> > @@ -390,3 +390,16 @@ void config_gpp_core(device_t nb_dev, device_t
> sb_dev)
> >  		switching_gpp_configurations(nb_dev, sb_dev);
> >  	ValidatePortEn(nb_dev);
> >  }
> > +
> > +/**
> > + * Hide unused Gpp port
> > + */
> > +void pcie_hide_unused_ports(device_t nb_dev) {
> > +	u16 hide = 0x6FC; /* skip port 0, 1, 8 */
> > +
> > +	hide &= ~(AtiPcieCfg.PortDetect | AtiPcieCfg.PortHp);
> > +	printk(BIOS_INFO, "rs780 unused GPP ports bitmap=0x%03x, force
> disabled\n", hide);
> > +	set_nbmisc_enable_bits(nb_dev, 0x0C, 0xFC, (hide & 0xFC)); /*
> bridge 2-7 */
> > +	set_nbmisc_enable_bits(nb_dev, 0x0C, 0x30000, ((hide >> 9) & 0x3)
> <<
> > +16); /* bridge 9-a */ }
> > diff --git a/src/southbridge/amd/rs780/rs780.c
> > b/src/southbridge/amd/rs780/rs780.c
> > index d8f1be3..b8c7d04 100644
> > --- a/src/southbridge/amd/rs780/rs780.c
> > +++ b/src/southbridge/amd/rs780/rs780.c
> > @@ -362,7 +362,10 @@ void rs780_enable(device_t dev)
> >  		if (dev->enabled)
> >  			rs780_gpp_sb_init(nb_dev, dev, dev_ind);
> >
> > -		if (dev_ind == 10) disable_pcie_bar3(nb_dev);
> > +		if (dev_ind == 10) {
> > +			disable_pcie_bar3(nb_dev);
> > +			pcie_hide_unused_ports(nb_dev);
> > +		}
> >  		break;
> >  	default:
> >  		printk(BIOS_DEBUG, "unknown dev: %s\n", dev_path(dev)); diff
> --git
> > a/src/southbridge/amd/rs780/rs780.h
> > b/src/southbridge/amd/rs780/rs780.h
> > index aba3e69..5b8d251 100644
> > --- a/src/southbridge/amd/rs780/rs780.h
> > +++ b/src/southbridge/amd/rs780/rs780.h
> > @@ -213,4 +213,5 @@ u32 extractbits(u32 source, int lsb, int msb);
> > int cpuidFamily(void);  int is_family0Fh(void);  int
> > is_family10h(void);
> > +void pcie_hide_unused_ports(device_t nb_dev);
> >  #endif				/* RS780_H */