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(-)
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Use pci_is_root_bus() instead of "if (bus->parent)" statement
for better readability.
Signed-off-by: Yijing Wang <[email protected]>
---
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
Use pcie_get_readrq() and pcie_set_readrq() functions to simplify code.
Signed-off-by: Yijing Wang <[email protected]>
---
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
Export pcie_get_mps() and pcie_set_mps() functions,
so driver can use them to simplify code.
Signed-off-by: Yijing Wang <[email protected]>
---
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
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 <[email protected]>
---
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
> 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 <[email protected]>
> 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
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 <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
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
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 <[email protected]>
Acked-by: Mark Einon <[email protected]>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
---
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;
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 <[email protected]>
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 <[email protected]>
---
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 */
>
> 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 <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Bjorn Helgaas <[email protected]>
This works!
Go ahead and queue it up.
Mike
[-cc unrelated folks, +cc Alex, Christian]
On Mon, Sep 9, 2013 at 7:13 AM, Yijing Wang <[email protected]> wrote:
> Use pcie_get_readrq() and pcie_set_readrq() functions to simplify code.
>
> Signed-off-by: Yijing Wang <[email protected]>
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
>
>
On Mon, Sep 30, 2013 at 8:56 AM, Marciniszyn, Mike
<[email protected]> 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 <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Bjorn Helgaas <[email protected]>
>
> 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
[-cc extra folks]
On Tue, Sep 24, 2013 at 2:39 PM, Bjorn Helgaas <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 */
>
> -----Original Message-----
> From: Bjorn Helgaas [mailto:[email protected]]
> Sent: Friday, October 04, 2013 4:45 PM
> To: Yijing Wang
> Cc: David Airlie; [email protected]; [email protected];
> 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 <[email protected]>
> wrote:
> > Use pcie_get_readrq() and pcie_set_readrq() functions to simplify code.
> >
> > Signed-off-by: Yijing Wang <[email protected]>
>
> 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 <[email protected]>
>
> 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
> >
> >
On Mon, Oct 7, 2013 at 6:15 AM, Deucher, Alexander
<[email protected]> wrote:
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:[email protected]]
>> Sent: Friday, October 04, 2013 4:45 PM
>> To: Yijing Wang
>> Cc: David Airlie; [email protected]; [email protected];
>> 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 <[email protected]>
>> wrote:
>> > Use pcie_get_readrq() and pcie_set_readrq() functions to simplify code.
>> >
>> > Signed-off-by: Yijing Wang <[email protected]>
>>
>> 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 <[email protected]>
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
>> >
>> >
>
>