[PATCH] PCI: enable extended tags support for PCIe endpoints

[PATCH] PCI: enable extended tags support for PCIe endpoints

[Sinan Kaya]

Each PCIe device can issue up to 32 transactions at a time by default.
Each transaction is tracked by a tag number on the bus. 32 outstanding
transactions is not enough for some performance critical applications
especially when a lot of small sized frames are transmitted.

Extended tags support increases this number to 256. Devices not
supporting extended tags tie-off this field to 0. According to ECN, it
is safe to enable this feature for all PCIe endpoints.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/pci/probe.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 93f280d..2424f38 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1505,12 +1505,19 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
+static int pci_configure_extended_tags(struct pci_dev *dev)
+{
+	return pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
+					 PCI_EXP_DEVCTL_EXT_TAG);
+}
+
 static void pci_configure_device(struct pci_dev *dev)
 {
 	struct hotplug_params hpp;
 	int ret;
 
 	pci_configure_mps(dev);
+	pci_configure_extended_tags(dev);
 
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
-- 
1.9.1

Re: [PATCH] PCI: enable extended tags support for PCIe endpoints

[Sinan Kaya]

On 9/24/2016 10:10 PM, Sinan Kaya wrote:
> Each PCIe device can issue up to 32 transactions at a time by default.
> Each transaction is tracked by a tag number on the bus. 32 outstanding
> transactions is not enough for some performance critical applications
> especially when a lot of small sized frames are transmitted.
> 
> Extended tags support increases this number to 256. Devices not
> supporting extended tags tie-off this field to 0. According to ECN, it
> is safe to enable this feature for all PCIe endpoints.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/pci/probe.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 93f280d..2424f38 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1505,12 +1505,19 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
>  	 */
>  }
>  
> +static int pci_configure_extended_tags(struct pci_dev *dev)
> +{

I should have checked the capability here before trying to enable it. 
I'll post a follow up patch on this. 

Is there any other feedback?


> +	return pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> +					 PCI_EXP_DEVCTL_EXT_TAG);
> +}
> +
>  static void pci_configure_device(struct pci_dev *dev)
>  {
>  	struct hotplug_params hpp;
>  	int ret;
>  
>  	pci_configure_mps(dev);
> +	pci_configure_extended_tags(dev);
>  
>  	memset(&hpp, 0, sizeof(hpp));
>  	ret = pci_get_hp_params(dev, &hpp);
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] PCI: enable extended tags support for PCIe endpoints

[Bjorn Helgaas]

On Thu, Nov 10, 2016 at 01:35:41PM -0500, Sinan Kaya wrote:
> On 9/24/2016 10:10 PM, Sinan Kaya wrote:
> > Each PCIe device can issue up to 32 transactions at a time by default.
> > Each transaction is tracked by a tag number on the bus. 32 outstanding
> > transactions is not enough for some performance critical applications
> > especially when a lot of small sized frames are transmitted.
> > 
> > Extended tags support increases this number to 256. Devices not
> > supporting extended tags tie-off this field to 0. According to ECN, it
> > is safe to enable this feature for all PCIe endpoints.
> > 
> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> > ---
> >  drivers/pci/probe.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 93f280d..2424f38 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1505,12 +1505,19 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
> >  	 */
> >  }
> >  
> > +static int pci_configure_extended_tags(struct pci_dev *dev)
> > +{
> 
> I should have checked the capability here before trying to enable it. 
> I'll post a follow up patch on this. 
> 
> Is there any other feedback?

If this were completely safe to enable for every device that supported
it, why would there be an enable bit in Device Control?

I don't know anything about extended tags, but it worries me a little
when there's a "go-fast" switch and no explanation about when and why
we might need to go slow.

> > +	return pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> > +					 PCI_EXP_DEVCTL_EXT_TAG);
> > +}
> > +
> >  static void pci_configure_device(struct pci_dev *dev)
> >  {
> >  	struct hotplug_params hpp;
> >  	int ret;
> >  
> >  	pci_configure_mps(dev);
> > +	pci_configure_extended_tags(dev);
> >  
> >  	memset(&hpp, 0, sizeof(hpp));
> >  	ret = pci_get_hp_params(dev, &hpp);
> > 
> 
> 
> -- 
> Sinan Kaya
> Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PCI: enable extended tags support for PCIe endpoints

[Sinan Kaya]

On 11/11/2016 3:58 PM, Bjorn Helgaas wrote:
>> I should have checked the capability here before trying to enable it. 
>> > I'll post a follow up patch on this. 
>> > 
>> > Is there any other feedback?
> If this were completely safe to enable for every device that supported
> it, why would there be an enable bit in Device Control?

reading from the ECN here.

https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf

The initial value is implementation specific and functions are allowed
to set it to 1 by default.

> 
> I don't know anything about extended tags, but it worries me a little
> when there's a "go-fast" switch and no explanation about when and why
> we might need to go slo

Based on my observation, extended tags increase the number of reads that
can be queued up back to back downstream. Otherwise, the requests will not
make progress until 1 tag out of 32 gets available.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] PCI: enable extended tags support for PCIe endpoints

[Sinan Kaya]

Hi Bjorn,

On 11/11/2016 3:58 PM, Bjorn Helgaas wrote:
>> Is there any other feedback?
> If this were completely safe to enable for every device that supported
> it, why would there be an enable bit in Device Control?
> 
> I don't know anything about extended tags, but it worries me a little
> when there's a "go-fast" switch and no explanation about when and why
> we might need to go slow.
> 

I have v2 posted. Do you feel like applying to linux-next to get some testing
exposure or do you want to tie it to some DMI so that we enable it only on
recent HW?

Sinan

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] PCI: enable extended tags support for PCIe endpoints

[Sinan Kaya]

On 1/25/2017 12:49 PM, Sinan Kaya wrote:
> Hi Bjorn,
> 
> On 11/11/2016 3:58 PM, Bjorn Helgaas wrote:
>>> Is there any other feedback?
>> If this were completely safe to enable for every device that supported
>> it, why would there be an enable bit in Device Control?
>>
>> I don't know anything about extended tags, but it worries me a little
>> when there's a "go-fast" switch and no explanation about when and why
>> we might need to go slow.
>>

I tried to answer your question in the new commit message. All PCIe completers
are required to support 8 bit tags. 

Generation of 8 bit tags is optional. That's why, there is a supported and an
enable/disable bit.

> 
> I have v2 posted. Do you feel like applying to linux-next to get some testing
> exposure or do you want to tie it to some DMI so that we enable it only on
> recent HW?
> 
> Sinan
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.