[PATCH 0/6] Simplify some mps and mrrs setting related code

[PATCH 0/6] Simplify some mps and mrrs setting related code

[Yijing Wang]

This series simplify mps and mrrs setting related code in drivers.
Export the pcie_set/get_mps() like pcie_set/get_readrq(). So drivers
can use these interfaces to simplify code that set mps and mrrs.
Patch 2 use pci_is_root_bus() instead of if (bus->parent) statement
for better readability.

Yijing Wang (6):
  PCI: Export pcie_set_mps() and pcie_get_mps()
  title/pci: use cached pci_dev->pcie_mpss to simplify code
  IB/qib: Use pci_is_root_bus() to check whether it is a root bus
  IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code
  staging/et131x: Use cached pci_dev->pcie_mpss and pcie_set_readrq()
    to simplif code
  radeon: Use pcie_get_readrq() and pcie_set_readrq() to simplify code

 arch/tile/kernel/pci.c               |    7 +--
 drivers/gpu/drm/radeon/evergreen.c   |   19 ++-----
 drivers/infiniband/hw/qib/qib_pcie.c |   98 +++++++++++-----------------------
 drivers/pci/pci.c                    |    2 +
 drivers/staging/et131x/et131x.c      |   14 +----
 5 files changed, 44 insertions(+), 96 deletions(-)

[PATCH 1/6] PCI: Export pcie_set_mps() and pcie_get_mps()

[Yijing Wang]

Export pcie_get_mps() and pcie_set_mps() functions,
so driver can use them to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pci.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b821a62..e35f7ec 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3972,6 +3972,7 @@ int pcie_get_mps(struct pci_dev *dev)
 
 	return 128 << ((ctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5);
 }
+EXPORT_SYMBOL(pcie_get_mps);
 
 /**
  * pcie_set_mps - set PCI Express maximum payload size
@@ -3996,6 +3997,7 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
 	return pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
 						  PCI_EXP_DEVCTL_PAYLOAD, v);
 }
+EXPORT_SYMBOL(pcie_set_mps);
 
 /**
  * pci_select_bars - Make BAR mask from the type of resource
-- 
1.7.1

[PATCH 2/6] title/pci: use cached pci_dev->pcie_mpss to simplify code

[Yijing Wang]

The PCI core caches the "PCI-E Max Payload Size Supported" in
pci_dev->pcie_mpss, so use that instead of pcie_capability_read_dword().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 arch/tile/kernel/pci.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 67237d3..692a799 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -246,15 +246,12 @@ static void fixup_read_and_payload_sizes(void)
 	/* Scan for the smallest maximum payload size. */
 	for_each_pci_dev(dev) {
 		u32 devcap;
-		int max_payload;
 
 		if (!pci_is_pcie(dev))
 			continue;
 
-		pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &devcap);
-		max_payload = devcap & PCI_EXP_DEVCAP_PAYLOAD;
-		if (max_payload < smallest_max_payload)
-			smallest_max_payload = max_payload;
+		if (dev->pcie_mpss < smallest_max_payload)
+			smallest_max_payload = dev->pcie_mpss;
 	}
 
 	/* Now, set the max_payload_size for all devices to that value. */
-- 
1.7.1

[PATCH 3/6] IB/qib: Use pci_is_root_bus() to check whether it is a root bus

[Yijing Wang]

Use pci_is_root_bus() instead of "if (bus->parent)" statement
for better readability.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/infiniband/hw/qib/qib_pcie.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index c574ec7..3a5b99b 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -590,7 +590,7 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 
 	/* Find out supported and configured values for parent (root) */
 	parent = dd->pcidev->bus->self;
-	if (parent->bus->parent) {
+	if (!pci_is_root_bus(parent->bus)) {
 		qib_devinfo(dd->pcidev, "Parent not root\n");
 		goto bail;
 	}
-- 
1.7.1

[PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

[Yijing Wang]

Refactor qib_tune_pcie_caps() function, use pcie_set_mps()
and pcie_get_mps() to simply code. Because pci core caches
the "PCI-E Max Payload Size Supported" in pci_dev->pcie_mpss,
so use that instead of pcie_capability_read_word(). Remove
the unused val2fld() and fld2val().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/infiniband/hw/qib/qib_pcie.c |   96 +++++++++++-----------------------
 1 files changed, 30 insertions(+), 66 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index 3a5b99b..f52a2fe 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -476,30 +476,6 @@ void qib_pcie_reenable(struct qib_devdata *dd, u16 cmd, u8 iline, u8 cline)
 			"pci_enable_device failed after reset: %d\n", r);
 }
 
-/* code to adjust PCIe capabilities. */
-
-static int fld2val(int wd, int mask)
-{
-	int lsbmask;
-
-	if (!mask)
-		return 0;
-	wd &= mask;
-	lsbmask = mask ^ (mask & (mask - 1));
-	wd /= lsbmask;
-	return wd;
-}
-
-static int val2fld(int wd, int mask)
-{
-	int lsbmask;
-
-	if (!mask)
-		return 0;
-	lsbmask = mask ^ (mask & (mask - 1));
-	wd *= lsbmask;
-	return wd;
-}
 
 static int qib_pcie_coalesce;
 module_param_named(pcie_coalesce, qib_pcie_coalesce, int, S_IRUGO);
@@ -584,9 +560,8 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 {
 	int ret = 1; /* Assume the worst */
 	struct pci_dev *parent;
-	u16 pcaps, pctl, ecaps, ectl;
-	int rc_sup, ep_sup;
-	int rc_cur, ep_cur;
+	u16 rc_mpss, rc_mps, ep_mpss, ep_mps;
+	u16 rc_mrrs, ep_mrrs, max_mrrs;
 
 	/* Find out supported and configured values for parent (root) */
 	parent = dd->pcidev->bus->self;
@@ -597,38 +572,29 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 
 	if (!pci_is_pcie(parent) || !pci_is_pcie(dd->pcidev))
 		goto bail;
-	pcie_capability_read_word(parent, PCI_EXP_DEVCAP, &pcaps);
-	pcie_capability_read_word(parent, PCI_EXP_DEVCTL, &pctl);
+	rc_mpss = parent->pcie_mpss;
+	rc_mps = ffs(pcie_get_mps(parent)) - 8;
 	/* Find out supported and configured values for endpoint (us) */
-	pcie_capability_read_word(dd->pcidev, PCI_EXP_DEVCAP, &ecaps);
-	pcie_capability_read_word(dd->pcidev, PCI_EXP_DEVCTL, &ectl);
+	ep_mpss = dd->pcidev->pcie_mpss;
+	ep_mps = ffs(pcie_get_mps(dd->pcidev)) - 8;
 
 	ret = 0;
 	/* Find max payload supported by root, endpoint */
-	rc_sup = fld2val(pcaps, PCI_EXP_DEVCAP_PAYLOAD);
-	ep_sup = fld2val(ecaps, PCI_EXP_DEVCAP_PAYLOAD);
-	if (rc_sup > ep_sup)
-		rc_sup = ep_sup;
-
-	rc_cur = fld2val(pctl, PCI_EXP_DEVCTL_PAYLOAD);
-	ep_cur = fld2val(ectl, PCI_EXP_DEVCTL_PAYLOAD);
+	if (rc_mpss > ep_mpss)
+		rc_mpss = ep_mpss;
 
 	/* If Supported greater than limit in module param, limit it */
-	if (rc_sup > (qib_pcie_caps & 7))
-		rc_sup = qib_pcie_caps & 7;
+	if (rc_mpss > (qib_pcie_caps & 7))
+		rc_mpss = qib_pcie_caps & 7;
 	/* If less than (allowed, supported), bump root payload */
-	if (rc_sup > rc_cur) {
-		rc_cur = rc_sup;
-		pctl = (pctl & ~PCI_EXP_DEVCTL_PAYLOAD) |
-			val2fld(rc_cur, PCI_EXP_DEVCTL_PAYLOAD);
-		pcie_capability_write_word(parent, PCI_EXP_DEVCTL, pctl);
+	if (rc_mpss > rc_mps) {
+		rc_mps = rc_mpss;
+		pcie_set_mps(parent, 128 << rc_mps);
 	}
 	/* If less than (allowed, supported), bump endpoint payload */
-	if (rc_sup > ep_cur) {
-		ep_cur = rc_sup;
-		ectl = (ectl & ~PCI_EXP_DEVCTL_PAYLOAD) |
-			val2fld(ep_cur, PCI_EXP_DEVCTL_PAYLOAD);
-		pcie_capability_write_word(dd->pcidev, PCI_EXP_DEVCTL, ectl);
+	if (rc_mpss > ep_mps) {
+		ep_mps = rc_mpss;
+		pcie_set_mps(dd->pcidev, 128 << ep_mps);
 	}
 
 	/*
@@ -636,23 +602,21 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 	 * No field for max supported, but PCIe spec limits it to 4096,
 	 * which is code '5' (log2(4096) - 7)
 	 */
-	rc_sup = 5;
-	if (rc_sup > ((qib_pcie_caps >> 4) & 7))
-		rc_sup = (qib_pcie_caps >> 4) & 7;
-	rc_cur = fld2val(pctl, PCI_EXP_DEVCTL_READRQ);
-	ep_cur = fld2val(ectl, PCI_EXP_DEVCTL_READRQ);
-
-	if (rc_sup > rc_cur) {
-		rc_cur = rc_sup;
-		pctl = (pctl & ~PCI_EXP_DEVCTL_READRQ) |
-			val2fld(rc_cur, PCI_EXP_DEVCTL_READRQ);
-		pcie_capability_write_word(parent, PCI_EXP_DEVCTL, pctl);
+	max_mrrs = 5;
+	if (max_mrrs > ((qib_pcie_caps >> 4) & 7))
+		max_mrrs = (qib_pcie_caps >> 4) & 7;
+
+	max_mrrs = 128 << max_mrrs;
+	rc_mrrs = pcie_get_readrq(parent);
+	ep_mrrs = pcie_get_readrq(dd->pcidev);
+
+	if (max_mrrs > rc_mrrs) {
+		rc_mrrs = max_mrrs;
+		pcie_set_readrq(parent, rc_mrrs);
 	}
-	if (rc_sup > ep_cur) {
-		ep_cur = rc_sup;
-		ectl = (ectl & ~PCI_EXP_DEVCTL_READRQ) |
-			val2fld(ep_cur, PCI_EXP_DEVCTL_READRQ);
-		pcie_capability_write_word(dd->pcidev, PCI_EXP_DEVCTL, ectl);
+	if (max_mrrs > ep_mrrs) {
+		ep_mrrs = max_mrrs;
+		pcie_set_readrq(dd->pcidev, ep_mrrs);
 	}
 bail:
 	return ret;
-- 
1.7.1

[PATCH 5/6] staging/et131x: Use cached pci_dev->pcie_mpss and pcie_set_readrq() to simplif code

[Yijing Wang]

The PCI core caches the "PCI-E Max Payload Size Supported" in
pci_dev->pcie_mpss, so use that instead of pcie_capability_read_dword().
Also use pcie_set_readrq() instead of pcie_capability_clear_and_set_word()
to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/staging/et131x/et131x.c |   14 +++-----------
 1 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index f73e58f..876881d 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -3605,17 +3605,10 @@ static int et131x_pci_init(struct et131x_adapter *adapter,
 		goto err_out;
 	}
 
-	/* Let's set up the PORT LOGIC Register.  First we need to know what
-	 * the max_payload_size is
-	 */
-	if (pcie_capability_read_word(pdev, PCI_EXP_DEVCAP, &max_payload)) {
-		dev_err(&pdev->dev,
-		    "Could not read PCI config space for Max Payload Size\n");
-		goto err_out;
-	}
+	/* Let's set up the PORT LOGIC Register. */
 
 	/* Program the Ack/Nak latency and replay timers */
-	max_payload &= 0x07;
+	max_payload = pdev->pcie_mpss;
 
 	if (max_payload < 2) {
 		static const u16 acknak[2] = { 0x76, 0xD0 };
@@ -3645,8 +3638,7 @@ static int et131x_pci_init(struct et131x_adapter *adapter,
 	}
 
 	/* Change the max read size to 2k */
-	if (pcie_capability_clear_and_set_word(pdev, PCI_EXP_DEVCTL,
-				PCI_EXP_DEVCTL_READRQ, 0x4 << 12)) {
+	if (pcie_set_readrq(pdev, 2048)) {
 		dev_err(&pdev->dev,
 			"Couldn't change PCI config space for Max read size\n");
 		goto err_out;
-- 
1.7.1

[PATCH 6/6] radeon: Use pcie_get_readrq() and pcie_set_readrq() to simplify code

[Yijing Wang]

Use pcie_get_readrq() and pcie_set_readrq() functions to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/gpu/drm/radeon/evergreen.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index d5b49e3..b191f92 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -1169,23 +1169,16 @@ int evergreen_set_uvd_clocks(struct radeon_device *rdev, u32 vclk, u32 dclk)
 
 void evergreen_fix_pci_max_read_req_size(struct radeon_device *rdev)
 {
-	u16 ctl, v;
-	int err;
-
-	err = pcie_capability_read_word(rdev->pdev, PCI_EXP_DEVCTL, &ctl);
-	if (err)
-		return;
-
-	v = (ctl & PCI_EXP_DEVCTL_READRQ) >> 12;
+	int readrq;
+	u16 v;
 
+	readrq = pcie_get_readrq(rdev->pdev);
+	v = ffs(readrq) - 8;
 	/* if bios or OS sets MAX_READ_REQUEST_SIZE to an invalid value, fix it
 	 * to avoid hangs or perfomance issues
 	 */
-	if ((v == 0) || (v == 6) || (v == 7)) {
-		ctl &= ~PCI_EXP_DEVCTL_READRQ;
-		ctl |= (2 << 12);
-		pcie_capability_write_word(rdev->pdev, PCI_EXP_DEVCTL, ctl);
-	}
+	if ((v == 0) || (v == 6) || (v == 7))
+		pcie_set_readrq(rdev->pdev, 512);
 }
 
 static bool dce4_is_in_vblank(struct radeon_device *rdev, int crtc)
-- 
1.7.1

RE: [PATCH 3/6] IB/qib: Use pci_is_root_bus() to check whether it is a root bus

[Marciniszyn, Mike]

> Subject: [PATCH 3/6] IB/qib: Use pci_is_root_bus() to check whether it is a root
> bus
> 

Thanks for the patch!

Acked-by: Mike Marciniszyn <mike.marciniszyn@intel.com>

RE: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

[Marciniszyn, Mike]

> Subject: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify
> code
> 
> Refactor qib_tune_pcie_caps() function, use pcie_set_mps() and
> pcie_get_mps() to simply code. Because pci core caches the "PCI-E Max
> Payload Size Supported" in pci_dev->pcie_mpss, so use that instead of
> pcie_capability_read_word(). Remove the unused val2fld() and fld2val().
> 

I tested this patch as is and saw no issues.

The only thing I would suggest is that the new use of pcie_get_readrq/pcie_set_readrq() be reflected in the comments with an appropriate adjustment in the subject.

Mike

 

Re: [PATCH 5/6] staging/et131x: Use cached pci_dev->pcie_mpss and pcie_set_readrq() to simplif code

[Greg Kroah-Hartman]

On Mon, Sep 09, 2013 at 09:13:07PM +0800, Yijing Wang wrote:
> The PCI core caches the "PCI-E Max Payload Size Supported" in
> pci_dev->pcie_mpss, so use that instead of pcie_capability_read_dword().
> Also use pcie_set_readrq() instead of pcie_capability_clear_and_set_word()
> to simplify code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

[Yijing Wang]

On 2013/9/9 22:55, Marciniszyn, Mike wrote:
>> Subject: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify
>> code
>>
>> Refactor qib_tune_pcie_caps() function, use pcie_set_mps() and
>> pcie_get_mps() to simply code. Because pci core caches the "PCI-E Max
>> Payload Size Supported" in pci_dev->pcie_mpss, so use that instead of
>> pcie_capability_read_word(). Remove the unused val2fld() and fld2val().
>>
> 
> I tested this patch as is and saw no issues.
> 
> The only thing I would suggest is that the new use of pcie_get_readrq/pcie_set_readrq() be reflected in the comments with an appropriate adjustment in the subject.

Hi Mike,
   Thanks for your tests very much! Ok, I will update the comments.

Thanks!
Yijing.

> 
> Mike
> 
>  
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 5/6] staging/et131x: Use cached pci_dev->pcie_mpss and pcie_set_readrq() to simplif code

[Mark Einon]

On Mon, Sep 09, 2013 at 09:13:07PM +0800, Yijing Wang wrote:
> The PCI core caches the "PCI-E Max Payload Size Supported" in
> pci_dev->pcie_mpss, so use that instead of pcie_capability_read_dword().
> Also use pcie_set_readrq() instead of pcie_capability_clear_and_set_word()
> to simplify code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>

Acked-by: Mark Einon <mark.einon@gmail.com>

Re: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

[Bjorn Helgaas]

On Mon, Sep 09, 2013 at 02:55:46PM +0000, Marciniszyn, Mike wrote:
> > Subject: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify
> > code
> > 
> > Refactor qib_tune_pcie_caps() function, use pcie_set_mps() and
> > pcie_get_mps() to simply code. Because pci core caches the "PCI-E Max
> > Payload Size Supported" in pci_dev->pcie_mpss, so use that instead of
> > pcie_capability_read_word(). Remove the unused val2fld() and fld2val().
> > 
> 
> I tested this patch as is and saw no issues.
> 
> The only thing I would suggest is that the new use of pcie_get_readrq/pcie_set_readrq() be reflected in the comments with an appropriate adjustment in the subject.

Is something like the following what you had in mind?  If so, I can
just queue it up.  Otherwise, I'll wait for Yijing to post a v2 patch.


IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

From: Yijing Wang <wangyijing@huawei.com>

Refactor qib_tune_pcie_caps().  Use pcie_get_mps(), pcie_set_mps(),
pcie_get_readrq(), and pcie_set_readrq() to simplify the code.  The PCI
core caches the "PCIe Max Payload Size Supported" in pci_dev->pcie_mpss,
so use that instead of pcie_capability_read_word().  Remove the unused
val2fld() and fld2val().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/infiniband/hw/qib/qib_pcie.c |   96 +++++++++++-----------------------
 1 file changed, 30 insertions(+), 66 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index 45e55ff..24973c8 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -476,30 +476,6 @@ void qib_pcie_reenable(struct qib_devdata *dd, u16 cmd, u8 iline, u8 cline)
 			"pci_enable_device failed after reset: %d\n", r);
 }
 
-/* code to adjust PCIe capabilities. */
-
-static int fld2val(int wd, int mask)
-{
-	int lsbmask;
-
-	if (!mask)
-		return 0;
-	wd &= mask;
-	lsbmask = mask ^ (mask & (mask - 1));
-	wd /= lsbmask;
-	return wd;
-}
-
-static int val2fld(int wd, int mask)
-{
-	int lsbmask;
-
-	if (!mask)
-		return 0;
-	lsbmask = mask ^ (mask & (mask - 1));
-	wd *= lsbmask;
-	return wd;
-}
 
 static int qib_pcie_coalesce;
 module_param_named(pcie_coalesce, qib_pcie_coalesce, int, S_IRUGO);
@@ -584,9 +560,8 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 {
 	int ret = 1; /* Assume the worst */
 	struct pci_dev *parent;
-	u16 pcaps, pctl, ecaps, ectl;
-	int rc_sup, ep_sup;
-	int rc_cur, ep_cur;
+	u16 rc_mpss, rc_mps, ep_mpss, ep_mps;
+	u16 rc_mrrs, ep_mrrs, max_mrrs;
 
 	/* Find out supported and configured values for parent (root) */
 	parent = dd->pcidev->bus->self;
@@ -597,38 +572,29 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 
 	if (!pci_is_pcie(parent) || !pci_is_pcie(dd->pcidev))
 		goto bail;
-	pcie_capability_read_word(parent, PCI_EXP_DEVCAP, &pcaps);
-	pcie_capability_read_word(parent, PCI_EXP_DEVCTL, &pctl);
+	rc_mpss = parent->pcie_mpss;
+	rc_mps = ffs(pcie_get_mps(parent)) - 8;
 	/* Find out supported and configured values for endpoint (us) */
-	pcie_capability_read_word(dd->pcidev, PCI_EXP_DEVCAP, &ecaps);
-	pcie_capability_read_word(dd->pcidev, PCI_EXP_DEVCTL, &ectl);
+	ep_mpss = dd->pcidev->pcie_mpss;
+	ep_mps = ffs(pcie_get_mps(dd->pcidev)) - 8;
 
 	ret = 0;
 	/* Find max payload supported by root, endpoint */
-	rc_sup = fld2val(pcaps, PCI_EXP_DEVCAP_PAYLOAD);
-	ep_sup = fld2val(ecaps, PCI_EXP_DEVCAP_PAYLOAD);
-	if (rc_sup > ep_sup)
-		rc_sup = ep_sup;
-
-	rc_cur = fld2val(pctl, PCI_EXP_DEVCTL_PAYLOAD);
-	ep_cur = fld2val(ectl, PCI_EXP_DEVCTL_PAYLOAD);
+	if (rc_mpss > ep_mpss)
+		rc_mpss = ep_mpss;
 
 	/* If Supported greater than limit in module param, limit it */
-	if (rc_sup > (qib_pcie_caps & 7))
-		rc_sup = qib_pcie_caps & 7;
+	if (rc_mpss > (qib_pcie_caps & 7))
+		rc_mpss = qib_pcie_caps & 7;
 	/* If less than (allowed, supported), bump root payload */
-	if (rc_sup > rc_cur) {
-		rc_cur = rc_sup;
-		pctl = (pctl & ~PCI_EXP_DEVCTL_PAYLOAD) |
-			val2fld(rc_cur, PCI_EXP_DEVCTL_PAYLOAD);
-		pcie_capability_write_word(parent, PCI_EXP_DEVCTL, pctl);
+	if (rc_mpss > rc_mps) {
+		rc_mps = rc_mpss;
+		pcie_set_mps(parent, 128 << rc_mps);
 	}
 	/* If less than (allowed, supported), bump endpoint payload */
-	if (rc_sup > ep_cur) {
-		ep_cur = rc_sup;
-		ectl = (ectl & ~PCI_EXP_DEVCTL_PAYLOAD) |
-			val2fld(ep_cur, PCI_EXP_DEVCTL_PAYLOAD);
-		pcie_capability_write_word(dd->pcidev, PCI_EXP_DEVCTL, ectl);
+	if (rc_mpss > ep_mps) {
+		ep_mps = rc_mpss;
+		pcie_set_mps(dd->pcidev, 128 << ep_mps);
 	}
 
 	/*
@@ -636,23 +602,21 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 	 * No field for max supported, but PCIe spec limits it to 4096,
 	 * which is code '5' (log2(4096) - 7)
 	 */
-	rc_sup = 5;
-	if (rc_sup > ((qib_pcie_caps >> 4) & 7))
-		rc_sup = (qib_pcie_caps >> 4) & 7;
-	rc_cur = fld2val(pctl, PCI_EXP_DEVCTL_READRQ);
-	ep_cur = fld2val(ectl, PCI_EXP_DEVCTL_READRQ);
-
-	if (rc_sup > rc_cur) {
-		rc_cur = rc_sup;
-		pctl = (pctl & ~PCI_EXP_DEVCTL_READRQ) |
-			val2fld(rc_cur, PCI_EXP_DEVCTL_READRQ);
-		pcie_capability_write_word(parent, PCI_EXP_DEVCTL, pctl);
+	max_mrrs = 5;
+	if (max_mrrs > ((qib_pcie_caps >> 4) & 7))
+		max_mrrs = (qib_pcie_caps >> 4) & 7;
+
+	max_mrrs = 128 << max_mrrs;
+	rc_mrrs = pcie_get_readrq(parent);
+	ep_mrrs = pcie_get_readrq(dd->pcidev);
+
+	if (max_mrrs > rc_mrrs) {
+		rc_mrrs = max_mrrs;
+		pcie_set_readrq(parent, rc_mrrs);
 	}
-	if (rc_sup > ep_cur) {
-		ep_cur = rc_sup;
-		ectl = (ectl & ~PCI_EXP_DEVCTL_READRQ) |
-			val2fld(ep_cur, PCI_EXP_DEVCTL_READRQ);
-		pcie_capability_write_word(dd->pcidev, PCI_EXP_DEVCTL, ectl);
+	if (max_mrrs > ep_mrrs) {
+		ep_mrrs = max_mrrs;
+		pcie_set_readrq(dd->pcidev, ep_mrrs);
 	}
 bail:
 	return ret;

Re: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

[Bjorn Helgaas]

On Mon, Sep 09, 2013 at 09:13:06PM +0800, Yijing Wang wrote:
> Refactor qib_tune_pcie_caps() function, use pcie_set_mps()
> and pcie_get_mps() to simply code. Because pci core caches
> the "PCI-E Max Payload Size Supported" in pci_dev->pcie_mpss,
> so use that instead of pcie_capability_read_word(). Remove
> the unused val2fld() and fld2val().

I propose the following patch on top of this one:


IB/qib: Drop qib_tune_pcie_caps() and qib_tune_pcie_coalesce() return values

From: Bjorn Helgaas <bhelgaas@google.com>

The callers of qib_tune_pcie_caps() and qib_tune_pcie_coalesce() don't
check the return values, so this patch drops the return values altogether.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/infiniband/hw/qib/qib_pcie.c |   28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index 24973c8..c8d9c4a 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -51,8 +51,8 @@
  * file calls, even though this violates some
  * expectations of harmlessness.
  */
-static int qib_tune_pcie_caps(struct qib_devdata *);
-static int qib_tune_pcie_coalesce(struct qib_devdata *);
+static void qib_tune_pcie_caps(struct qib_devdata *);
+static void qib_tune_pcie_coalesce(struct qib_devdata *);
 
 /*
  * Do all the common PCIe setup and initialization.
@@ -487,7 +487,7 @@ MODULE_PARM_DESC(pcie_coalesce, "tune PCIe colescing on some Intel chipsets");
  * of these chipsets, with some BIOS settings, and enabling it on those
  * systems may result in the system crashing, and/or data corruption.
  */
-static int qib_tune_pcie_coalesce(struct qib_devdata *dd)
+static void qib_tune_pcie_coalesce(struct qib_devdata *dd)
 {
 	int r;
 	struct pci_dev *parent;
@@ -495,18 +495,18 @@ static int qib_tune_pcie_coalesce(struct qib_devdata *dd)
 	u32 mask, bits, val;
 
 	if (!qib_pcie_coalesce)
-		return 0;
+		return;
 
 	/* Find out supported and configured values for parent (root) */
 	parent = dd->pcidev->bus->self;
 	if (parent->bus->parent) {
 		qib_devinfo(dd->pcidev, "Parent not root\n");
-		return 1;
+		return;
 	}
 	if (!pci_is_pcie(parent))
-		return 1;
+		return;
 	if (parent->vendor != 0x8086)
-		return 1;
+		return;
 
 	/*
 	 *  - bit 12: Max_rdcmp_Imt_EN: need to set to 1
@@ -539,13 +539,12 @@ static int qib_tune_pcie_coalesce(struct qib_devdata *dd)
 		mask = (3U << 24) | (7U << 10);
 	} else {
 		/* not one of the chipsets that we know about */
-		return 1;
+		return;
 	}
 	pci_read_config_dword(parent, 0x48, &val);
 	val &= ~mask;
 	val |= bits;
 	r = pci_write_config_dword(parent, 0x48, val);
-	return 0;
 }
 
 /*
@@ -556,9 +555,8 @@ static int qib_pcie_caps;
 module_param_named(pcie_caps, qib_pcie_caps, int, S_IRUGO);
 MODULE_PARM_DESC(pcie_caps, "Max PCIe tuning: Payload (0..3), ReadReq (4..7)");
 
-static int qib_tune_pcie_caps(struct qib_devdata *dd)
+static void qib_tune_pcie_caps(struct qib_devdata *dd)
 {
-	int ret = 1; /* Assume the worst */
 	struct pci_dev *parent;
 	u16 rc_mpss, rc_mps, ep_mpss, ep_mps;
 	u16 rc_mrrs, ep_mrrs, max_mrrs;
@@ -567,18 +565,18 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 	parent = dd->pcidev->bus->self;
 	if (!pci_is_root_bus(parent->bus)) {
 		qib_devinfo(dd->pcidev, "Parent not root\n");
-		goto bail;
+		return;
 	}
 
 	if (!pci_is_pcie(parent) || !pci_is_pcie(dd->pcidev))
-		goto bail;
+		return;
+
 	rc_mpss = parent->pcie_mpss;
 	rc_mps = ffs(pcie_get_mps(parent)) - 8;
 	/* Find out supported and configured values for endpoint (us) */
 	ep_mpss = dd->pcidev->pcie_mpss;
 	ep_mps = ffs(pcie_get_mps(dd->pcidev)) - 8;
 
-	ret = 0;
 	/* Find max payload supported by root, endpoint */
 	if (rc_mpss > ep_mpss)
 		rc_mpss = ep_mpss;
@@ -618,8 +616,6 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
 		ep_mrrs = max_mrrs;
 		pcie_set_readrq(dd->pcidev, ep_mrrs);
 	}
-bail:
-	return ret;
 }
 /* End of PCIe capability tuning */
 

RE: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

[Marciniszyn, Mike]

> 
> Is something like the following what you had in mind?  If so, I can
> just queue it up.  Otherwise, I'll wait for Yijing to post a v2 patch.
> 
> 
> IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code
> 
> From: Yijing Wang <wangyijing@huawei.com>
> 
> Refactor qib_tune_pcie_caps().  Use pcie_get_mps(), pcie_set_mps(),
> pcie_get_readrq(), and pcie_set_readrq() to simplify the code.  The PCI
> core caches the "PCIe Max Payload Size Supported" in pci_dev->pcie_mpss,
> so use that instead of pcie_capability_read_word().  Remove the unused
> val2fld() and fld2val().
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

This works!

Go ahead and queue it up.

Mike

Re: [PATCH 6/6] radeon: Use pcie_get_readrq() and pcie_set_readrq() to simplify code

[Bjorn Helgaas]

[-cc unrelated folks, +cc Alex, Christian]

On Mon, Sep 9, 2013 at 7:13 AM, Yijing Wang <wangyijing@huawei.com> wrote:
> Use pcie_get_readrq() and pcie_set_readrq() functions to simplify code.
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>

I believe the following patch is correct, and I'd be happy to merge it
via the PCI tree along with the rest of this series.

But it'd be better to have an ack from Alex, and he might prefer to
merge it directly.

Bjorn

> ---
>  drivers/gpu/drm/radeon/evergreen.c |   19 ++++++-------------
>  1 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
> index d5b49e3..b191f92 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -1169,23 +1169,16 @@ int evergreen_set_uvd_clocks(struct radeon_device *rdev, u32 vclk, u32 dclk)
>
>  void evergreen_fix_pci_max_read_req_size(struct radeon_device *rdev)
>  {
> -       u16 ctl, v;
> -       int err;
> -
> -       err = pcie_capability_read_word(rdev->pdev, PCI_EXP_DEVCTL, &ctl);
> -       if (err)
> -               return;
> -
> -       v = (ctl & PCI_EXP_DEVCTL_READRQ) >> 12;
> +       int readrq;
> +       u16 v;
>
> +       readrq = pcie_get_readrq(rdev->pdev);
> +       v = ffs(readrq) - 8;
>         /* if bios or OS sets MAX_READ_REQUEST_SIZE to an invalid value, fix it
>          * to avoid hangs or perfomance issues
>          */
> -       if ((v == 0) || (v == 6) || (v == 7)) {
> -               ctl &= ~PCI_EXP_DEVCTL_READRQ;
> -               ctl |= (2 << 12);
> -               pcie_capability_write_word(rdev->pdev, PCI_EXP_DEVCTL, ctl);
> -       }
> +       if ((v == 0) || (v == 6) || (v == 7))
> +               pcie_set_readrq(rdev->pdev, 512);
>  }
>
>  static bool dce4_is_in_vblank(struct radeon_device *rdev, int crtc)
> --
> 1.7.1
>
>

Re: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

[Bjorn Helgaas]

On Mon, Sep 30, 2013 at 8:56 AM, Marciniszyn, Mike
<mike.marciniszyn@intel.com> wrote:
>>
>> Is something like the following what you had in mind?  If so, I can
>> just queue it up.  Otherwise, I'll wait for Yijing to post a v2 patch.
>>
>>
>> IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code
>>
>> From: Yijing Wang <wangyijing@huawei.com>
>>
>> Refactor qib_tune_pcie_caps().  Use pcie_get_mps(), pcie_set_mps(),
>> pcie_get_readrq(), and pcie_set_readrq() to simplify the code.  The PCI
>> core caches the "PCIe Max Payload Size Supported" in pci_dev->pcie_mpss,
>> so use that instead of pcie_capability_read_word().  Remove the unused
>> val2fld() and fld2val().
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> This works!
>
> Go ahead and queue it up.

Applied to my pci/yijing-mps-v1 branch with your ack:

http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/yijing-mps-v1&id=0ce0e62f1f7893f983a8f61bc8f5306e80d520b1

Let me know if that's not what you had in mind.  Thanks!

Bjorn

Re: [PATCH 4/6] IB/qib: Use pcie_set_mps() and pcie_get_mps() to simplify code

[Bjorn Helgaas]

[-cc extra folks]

On Tue, Sep 24, 2013 at 2:39 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Mon, Sep 09, 2013 at 09:13:06PM +0800, Yijing Wang wrote:
>> Refactor qib_tune_pcie_caps() function, use pcie_set_mps()
>> and pcie_get_mps() to simply code. Because pci core caches
>> the "PCI-E Max Payload Size Supported" in pci_dev->pcie_mpss,
>> so use that instead of pcie_capability_read_word(). Remove
>> the unused val2fld() and fld2val().
>
> I propose the following patch on top of this one:

I applied this to my pci/yijing-mps-v1 branch, Mike:

http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/yijing-mps-v1&id=03078633a6eb86fdb6ea2f40e6352de4b1181bbf

It's the same as the patch below, with your ack added.  Let me know if
I need to tweak/change/drop anything.  Thanks!

Bjorn

> IB/qib: Drop qib_tune_pcie_caps() and qib_tune_pcie_coalesce() return values
>
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> The callers of qib_tune_pcie_caps() and qib_tune_pcie_coalesce() don't
> check the return values, so this patch drops the return values altogether.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/infiniband/hw/qib/qib_pcie.c |   28 ++++++++++++----------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
> index 24973c8..c8d9c4a 100644
> --- a/drivers/infiniband/hw/qib/qib_pcie.c
> +++ b/drivers/infiniband/hw/qib/qib_pcie.c
> @@ -51,8 +51,8 @@
>   * file calls, even though this violates some
>   * expectations of harmlessness.
>   */
> -static int qib_tune_pcie_caps(struct qib_devdata *);
> -static int qib_tune_pcie_coalesce(struct qib_devdata *);
> +static void qib_tune_pcie_caps(struct qib_devdata *);
> +static void qib_tune_pcie_coalesce(struct qib_devdata *);
>
>  /*
>   * Do all the common PCIe setup and initialization.
> @@ -487,7 +487,7 @@ MODULE_PARM_DESC(pcie_coalesce, "tune PCIe colescing on some Intel chipsets");
>   * of these chipsets, with some BIOS settings, and enabling it on those
>   * systems may result in the system crashing, and/or data corruption.
>   */
> -static int qib_tune_pcie_coalesce(struct qib_devdata *dd)
> +static void qib_tune_pcie_coalesce(struct qib_devdata *dd)
>  {
>         int r;
>         struct pci_dev *parent;
> @@ -495,18 +495,18 @@ static int qib_tune_pcie_coalesce(struct qib_devdata *dd)
>         u32 mask, bits, val;
>
>         if (!qib_pcie_coalesce)
> -               return 0;
> +               return;
>
>         /* Find out supported and configured values for parent (root) */
>         parent = dd->pcidev->bus->self;
>         if (parent->bus->parent) {
>                 qib_devinfo(dd->pcidev, "Parent not root\n");
> -               return 1;
> +               return;
>         }
>         if (!pci_is_pcie(parent))
> -               return 1;
> +               return;
>         if (parent->vendor != 0x8086)
> -               return 1;
> +               return;
>
>         /*
>          *  - bit 12: Max_rdcmp_Imt_EN: need to set to 1
> @@ -539,13 +539,12 @@ static int qib_tune_pcie_coalesce(struct qib_devdata *dd)
>                 mask = (3U << 24) | (7U << 10);
>         } else {
>                 /* not one of the chipsets that we know about */
> -               return 1;
> +               return;
>         }
>         pci_read_config_dword(parent, 0x48, &val);
>         val &= ~mask;
>         val |= bits;
>         r = pci_write_config_dword(parent, 0x48, val);
> -       return 0;
>  }
>
>  /*
> @@ -556,9 +555,8 @@ static int qib_pcie_caps;
>  module_param_named(pcie_caps, qib_pcie_caps, int, S_IRUGO);
>  MODULE_PARM_DESC(pcie_caps, "Max PCIe tuning: Payload (0..3), ReadReq (4..7)");
>
> -static int qib_tune_pcie_caps(struct qib_devdata *dd)
> +static void qib_tune_pcie_caps(struct qib_devdata *dd)
>  {
> -       int ret = 1; /* Assume the worst */
>         struct pci_dev *parent;
>         u16 rc_mpss, rc_mps, ep_mpss, ep_mps;
>         u16 rc_mrrs, ep_mrrs, max_mrrs;
> @@ -567,18 +565,18 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
>         parent = dd->pcidev->bus->self;
>         if (!pci_is_root_bus(parent->bus)) {
>                 qib_devinfo(dd->pcidev, "Parent not root\n");
> -               goto bail;
> +               return;
>         }
>
>         if (!pci_is_pcie(parent) || !pci_is_pcie(dd->pcidev))
> -               goto bail;
> +               return;
> +
>         rc_mpss = parent->pcie_mpss;
>         rc_mps = ffs(pcie_get_mps(parent)) - 8;
>         /* Find out supported and configured values for endpoint (us) */
>         ep_mpss = dd->pcidev->pcie_mpss;
>         ep_mps = ffs(pcie_get_mps(dd->pcidev)) - 8;
>
> -       ret = 0;
>         /* Find max payload supported by root, endpoint */
>         if (rc_mpss > ep_mpss)
>                 rc_mpss = ep_mpss;
> @@ -618,8 +616,6 @@ static int qib_tune_pcie_caps(struct qib_devdata *dd)
>                 ep_mrrs = max_mrrs;
>                 pcie_set_readrq(dd->pcidev, ep_mrrs);
>         }
> -bail:
> -       return ret;
>  }
>  /* End of PCIe capability tuning */
>

RE: [PATCH 6/6] radeon: Use pcie_get_readrq() and pcie_set_readrq() to simplify code

[Deucher, Alexander]

> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Friday, October 04, 2013 4:45 PM
> To: Yijing Wang
> Cc: David Airlie; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org;
> Hanjun Guo; Deucher, Alexander; Koenig, Christian
> Subject: Re: [PATCH 6/6] radeon: Use pcie_get_readrq() and
> pcie_set_readrq() to simplify code
> 
> [-cc unrelated folks, +cc Alex, Christian]
> 
> On Mon, Sep 9, 2013 at 7:13 AM, Yijing Wang <wangyijing@huawei.com>
> wrote:
> > Use pcie_get_readrq() and pcie_set_readrq() functions to simplify code.
> >
> > Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> 
> I believe the following patch is correct, and I'd be happy to merge it
> via the PCI tree along with the rest of this series.
> 
> But it'd be better to have an ack from Alex, and he might prefer to
> merge it directly.

Patch looks correct to me.  Feel free to merge it via the pci tree.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> 
> Bjorn
> 
> > ---
> >  drivers/gpu/drm/radeon/evergreen.c |   19 ++++++-------------
> >  1 files changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/evergreen.c
> b/drivers/gpu/drm/radeon/evergreen.c
> > index d5b49e3..b191f92 100644
> > --- a/drivers/gpu/drm/radeon/evergreen.c
> > +++ b/drivers/gpu/drm/radeon/evergreen.c
> > @@ -1169,23 +1169,16 @@ int evergreen_set_uvd_clocks(struct
> radeon_device *rdev, u32 vclk, u32 dclk)
> >
> >  void evergreen_fix_pci_max_read_req_size(struct radeon_device *rdev)
> >  {
> > -       u16 ctl, v;
> > -       int err;
> > -
> > -       err = pcie_capability_read_word(rdev->pdev, PCI_EXP_DEVCTL, &ctl);
> > -       if (err)
> > -               return;
> > -
> > -       v = (ctl & PCI_EXP_DEVCTL_READRQ) >> 12;
> > +       int readrq;
> > +       u16 v;
> >
> > +       readrq = pcie_get_readrq(rdev->pdev);
> > +       v = ffs(readrq) - 8;
> >         /* if bios or OS sets MAX_READ_REQUEST_SIZE to an invalid value, fix it
> >          * to avoid hangs or perfomance issues
> >          */
> > -       if ((v == 0) || (v == 6) || (v == 7)) {
> > -               ctl &= ~PCI_EXP_DEVCTL_READRQ;
> > -               ctl |= (2 << 12);
> > -               pcie_capability_write_word(rdev->pdev, PCI_EXP_DEVCTL, ctl);
> > -       }
> > +       if ((v == 0) || (v == 6) || (v == 7))
> > +               pcie_set_readrq(rdev->pdev, 512);
> >  }
> >
> >  static bool dce4_is_in_vblank(struct radeon_device *rdev, int crtc)
> > --
> > 1.7.1
> >
> >

Re: [PATCH 6/6] radeon: Use pcie_get_readrq() and pcie_set_readrq() to simplify code

[Bjorn Helgaas]

On Mon, Oct 7, 2013 at 6:15 AM, Deucher, Alexander
<Alexander.Deucher@amd.com> wrote:
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Friday, October 04, 2013 4:45 PM
>> To: Yijing Wang
>> Cc: David Airlie; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org;
>> Hanjun Guo; Deucher, Alexander; Koenig, Christian
>> Subject: Re: [PATCH 6/6] radeon: Use pcie_get_readrq() and
>> pcie_set_readrq() to simplify code
>>
>> [-cc unrelated folks, +cc Alex, Christian]
>>
>> On Mon, Sep 9, 2013 at 7:13 AM, Yijing Wang <wangyijing@huawei.com>
>> wrote:
>> > Use pcie_get_readrq() and pcie_set_readrq() functions to simplify code.
>> >
>> > Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>>
>> I believe the following patch is correct, and I'd be happy to merge it
>> via the PCI tree along with the rest of this series.
>>
>> But it'd be better to have an ack from Alex, and he might prefer to
>> merge it directly.
>
> Patch looks correct to me.  Feel free to merge it via the pci tree.
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks, I'll add your Reviewed-by and merge it.

Bjorn

>> > ---
>> >  drivers/gpu/drm/radeon/evergreen.c |   19 ++++++-------------
>> >  1 files changed, 6 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/radeon/evergreen.c
>> b/drivers/gpu/drm/radeon/evergreen.c
>> > index d5b49e3..b191f92 100644
>> > --- a/drivers/gpu/drm/radeon/evergreen.c
>> > +++ b/drivers/gpu/drm/radeon/evergreen.c
>> > @@ -1169,23 +1169,16 @@ int evergreen_set_uvd_clocks(struct
>> radeon_device *rdev, u32 vclk, u32 dclk)
>> >
>> >  void evergreen_fix_pci_max_read_req_size(struct radeon_device *rdev)
>> >  {
>> > -       u16 ctl, v;
>> > -       int err;
>> > -
>> > -       err = pcie_capability_read_word(rdev->pdev, PCI_EXP_DEVCTL, &ctl);
>> > -       if (err)
>> > -               return;
>> > -
>> > -       v = (ctl & PCI_EXP_DEVCTL_READRQ) >> 12;
>> > +       int readrq;
>> > +       u16 v;
>> >
>> > +       readrq = pcie_get_readrq(rdev->pdev);
>> > +       v = ffs(readrq) - 8;
>> >         /* if bios or OS sets MAX_READ_REQUEST_SIZE to an invalid value, fix it
>> >          * to avoid hangs or perfomance issues
>> >          */
>> > -       if ((v == 0) || (v == 6) || (v == 7)) {
>> > -               ctl &= ~PCI_EXP_DEVCTL_READRQ;
>> > -               ctl |= (2 << 12);
>> > -               pcie_capability_write_word(rdev->pdev, PCI_EXP_DEVCTL, ctl);
>> > -       }
>> > +       if ((v == 0) || (v == 6) || (v == 7))
>> > +               pcie_set_readrq(rdev->pdev, 512);
>> >  }
>> >
>> >  static bool dce4_is_in_vblank(struct radeon_device *rdev, int crtc)
>> > --
>> > 1.7.1
>> >
>> >
>
>