* [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-10-31 15:46 [PATCH v1 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Mrinmay Sarkar
@ 2023-10-31 15:46 ` Mrinmay Sarkar
2023-10-31 16:50 ` Konrad Dybcio
2023-11-02 15:34 ` Dmitry Baryshkov
2023-10-31 15:46 ` [PATCH v1 2/3] PCI: qcom-ep: Enable cache coherency for SA8775P EP Mrinmay Sarkar
` (2 subsequent siblings)
3 siblings, 2 replies; 14+ messages in thread
From: Mrinmay Sarkar @ 2023-10-31 15:46 UTC (permalink / raw)
To: agross, andersson, krzysztof.kozlowski+dt, conor+dt,
konrad.dybcio, mani, robh+dt
Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
dmitry.baryshkov, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Mrinmay Sarkar, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
This change will enable cache snooping logic to support
cache coherency for SA8755P RC platform.
Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6902e97..6f240fc 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -51,6 +51,7 @@
#define PARF_SID_OFFSET 0x234
#define PARF_BDF_TRANSLATE_CFG 0x24c
#define PARF_SLV_ADDR_SPACE_SIZE 0x358
+#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
#define PARF_DEVICE_TYPE 0x1000
#define PARF_BDF_TO_SID_TABLE_N 0x2000
@@ -117,6 +118,9 @@
/* PARF_LTSSM register fields */
#define LTSSM_EN BIT(8)
+/* PARF_NO_SNOOP_OVERIDE register value */
+#define NO_SNOOP_OVERIDE_EN 0xa
+
/* PARF_DEVICE_TYPE register fields */
#define DEVICE_TYPE_RC 0x4
@@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
{
+ struct dw_pcie *pci = pcie->pci;
+ struct device *dev = pci->dev;
+
+ /* Enable cache snooping for SA8775P */
+ if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p"))
+ writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE);
+
qcom_pcie_clear_hpc(pcie->pci);
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-10-31 15:46 ` [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC Mrinmay Sarkar
@ 2023-10-31 16:50 ` Konrad Dybcio
2023-11-02 10:16 ` Mrinmay Sarkar
2023-11-02 15:34 ` Dmitry Baryshkov
1 sibling, 1 reply; 14+ messages in thread
From: Konrad Dybcio @ 2023-10-31 16:50 UTC (permalink / raw)
To: Mrinmay Sarkar, agross, andersson, krzysztof.kozlowski+dt,
conor+dt, mani, robh+dt
Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
dmitry.baryshkov, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
On 31.10.2023 16:46, Mrinmay Sarkar wrote:
> This change will enable cache snooping logic to support
> cache coherency for SA8755P RC platform.
8775
>
> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6902e97..6f240fc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -51,6 +51,7 @@
> #define PARF_SID_OFFSET 0x234
> #define PARF_BDF_TRANSLATE_CFG 0x24c
> #define PARF_SLV_ADDR_SPACE_SIZE 0x358
> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
> #define PARF_DEVICE_TYPE 0x1000
> #define PARF_BDF_TO_SID_TABLE_N 0x2000
>
> @@ -117,6 +118,9 @@
> /* PARF_LTSSM register fields */
> #define LTSSM_EN BIT(8)
>
> +/* PARF_NO_SNOOP_OVERIDE register value */
override
> +#define NO_SNOOP_OVERIDE_EN 0xa
is this actually some magic value and not BIT(1) | BIT(3)?
> /* PARF_DEVICE_TYPE register fields */
> #define DEVICE_TYPE_RC 0x4
>
> @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>
> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
> {
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> +
> + /* Enable cache snooping for SA8775P */
> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p"))
> + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE);
Why only for 8775 and not for other v2.7, or perhaps all other
revisions?
Konrad
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-10-31 16:50 ` Konrad Dybcio
@ 2023-11-02 10:16 ` Mrinmay Sarkar
2023-11-02 22:27 ` Konrad Dybcio
0 siblings, 1 reply; 14+ messages in thread
From: Mrinmay Sarkar @ 2023-11-02 10:16 UTC (permalink / raw)
To: Konrad Dybcio, agross, andersson, krzysztof.kozlowski+dt,
conor+dt, mani, robh+dt
Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
dmitry.baryshkov, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
On 10/31/2023 10:20 PM, Konrad Dybcio wrote:
> On 31.10.2023 16:46, Mrinmay Sarkar wrote:
>> This change will enable cache snooping logic to support
>> cache coherency for SA8755P RC platform.
> 8775
>
>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
>> ---
>> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 6902e97..6f240fc 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -51,6 +51,7 @@
>> #define PARF_SID_OFFSET 0x234
>> #define PARF_BDF_TRANSLATE_CFG 0x24c
>> #define PARF_SLV_ADDR_SPACE_SIZE 0x358
>> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
>> #define PARF_DEVICE_TYPE 0x1000
>> #define PARF_BDF_TO_SID_TABLE_N 0x2000
>>
>> @@ -117,6 +118,9 @@
>> /* PARF_LTSSM register fields */
>> #define LTSSM_EN BIT(8)
>>
>> +/* PARF_NO_SNOOP_OVERIDE register value */
> override
>> +#define NO_SNOOP_OVERIDE_EN 0xa
> is this actually some magic value and not BIT(1) | BIT(3)?
we need to set 1st and 3rd bit. yes, we can use BIT(1) | BIT(3).
>
>> /* PARF_DEVICE_TYPE register fields */
>> #define DEVICE_TYPE_RC 0x4
>>
>> @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>>
>> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
>> {
>> + struct dw_pcie *pci = pcie->pci;
>> + struct device *dev = pci->dev;
>> +
>> + /* Enable cache snooping for SA8775P */
>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p"))
>> + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE);
> Why only for 8775 and not for other v2.7, or perhaps all other
> revisions?
yes this is only required for 8775 due to hw requirement we need to enable
cache snooping from the register level for 8775.
> Konrad
Thanks,
Mrinmay
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-11-02 10:16 ` Mrinmay Sarkar
@ 2023-11-02 22:27 ` Konrad Dybcio
0 siblings, 0 replies; 14+ messages in thread
From: Konrad Dybcio @ 2023-11-02 22:27 UTC (permalink / raw)
To: Mrinmay Sarkar, agross, andersson, krzysztof.kozlowski+dt,
conor+dt, mani, robh+dt
Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
dmitry.baryshkov, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
On 02/11/2023 11:16, Mrinmay Sarkar wrote:
>
> On 10/31/2023 10:20 PM, Konrad Dybcio wrote:
>> On 31.10.2023 16:46, Mrinmay Sarkar wrote:
>>> This change will enable cache snooping logic to support
>>> cache coherency for SA8755P RC platform.
>> 8775
>>
>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
>>> ---
>>> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
>>> b/drivers/pci/controller/dwc/pcie-qcom.c
>>> index 6902e97..6f240fc 100644
>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>> @@ -51,6 +51,7 @@
>>> #define PARF_SID_OFFSET 0x234
>>> #define PARF_BDF_TRANSLATE_CFG 0x24c
>>> #define PARF_SLV_ADDR_SPACE_SIZE 0x358
>>> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
>>> #define PARF_DEVICE_TYPE 0x1000
>>> #define PARF_BDF_TO_SID_TABLE_N 0x2000
>>> @@ -117,6 +118,9 @@
>>> /* PARF_LTSSM register fields */
>>> #define LTSSM_EN BIT(8)
>>> +/* PARF_NO_SNOOP_OVERIDE register value */
>> override
>>> +#define NO_SNOOP_OVERIDE_EN 0xa
>> is this actually some magic value and not BIT(1) | BIT(3)?
> we need to set 1st and 3rd bit. yes, we can use BIT(1) | BIT(3).
It would be great if you could explain what each of these bits means
separately, #defining them instead and ORing at usage time.
Konrad
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-10-31 15:46 ` [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC Mrinmay Sarkar
2023-10-31 16:50 ` Konrad Dybcio
@ 2023-11-02 15:34 ` Dmitry Baryshkov
2023-11-02 16:36 ` Manivannan Sadhasivam
1 sibling, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2023-11-02 15:34 UTC (permalink / raw)
To: Mrinmay Sarkar
Cc: agross, andersson, krzysztof.kozlowski+dt, conor+dt,
konrad.dybcio, mani, robh+dt, quic_shazhuss, quic_nitegupt,
quic_ramkri, quic_nayiluri, robh, quic_krichai, quic_vbadigan,
quic_parass, quic_schintav, quic_shijjose, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote:
>
> This change will enable cache snooping logic to support
> cache coherency for SA8755P RC platform.
>
> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6902e97..6f240fc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -51,6 +51,7 @@
> #define PARF_SID_OFFSET 0x234
> #define PARF_BDF_TRANSLATE_CFG 0x24c
> #define PARF_SLV_ADDR_SPACE_SIZE 0x358
> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
> #define PARF_DEVICE_TYPE 0x1000
> #define PARF_BDF_TO_SID_TABLE_N 0x2000
>
> @@ -117,6 +118,9 @@
> /* PARF_LTSSM register fields */
> #define LTSSM_EN BIT(8)
>
> +/* PARF_NO_SNOOP_OVERIDE register value */
> +#define NO_SNOOP_OVERIDE_EN 0xa
> +
> /* PARF_DEVICE_TYPE register fields */
> #define DEVICE_TYPE_RC 0x4
>
> @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>
> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
> {
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> +
> + /* Enable cache snooping for SA8775P */
> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p"))
Obviously: please populate a flag in the data structures instead of
doing of_device_is_compatible(). Same applies to the patch 2.
> + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE);
> +
> qcom_pcie_clear_hpc(pcie->pci);
>
> return 0;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-11-02 15:34 ` Dmitry Baryshkov
@ 2023-11-02 16:36 ` Manivannan Sadhasivam
2023-11-02 22:25 ` Konrad Dybcio
0 siblings, 1 reply; 14+ messages in thread
From: Manivannan Sadhasivam @ 2023-11-02 16:36 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Mrinmay Sarkar, agross, andersson, krzysztof.kozlowski+dt,
conor+dt, konrad.dybcio, mani, robh+dt, quic_shazhuss,
quic_nitegupt, quic_ramkri, quic_nayiluri, robh, quic_krichai,
quic_vbadigan, quic_parass, quic_schintav, quic_shijjose,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
linux-arm-msm, devicetree, linux-kernel, linux-pci
On Thu, Nov 02, 2023 at 05:34:24PM +0200, Dmitry Baryshkov wrote:
> On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote:
> >
> > This change will enable cache snooping logic to support
> > cache coherency for SA8755P RC platform.
> >
> > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 6902e97..6f240fc 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -51,6 +51,7 @@
> > #define PARF_SID_OFFSET 0x234
> > #define PARF_BDF_TRANSLATE_CFG 0x24c
> > #define PARF_SLV_ADDR_SPACE_SIZE 0x358
> > +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
> > #define PARF_DEVICE_TYPE 0x1000
> > #define PARF_BDF_TO_SID_TABLE_N 0x2000
> >
> > @@ -117,6 +118,9 @@
> > /* PARF_LTSSM register fields */
> > #define LTSSM_EN BIT(8)
> >
> > +/* PARF_NO_SNOOP_OVERIDE register value */
> > +#define NO_SNOOP_OVERIDE_EN 0xa
> > +
> > /* PARF_DEVICE_TYPE register fields */
> > #define DEVICE_TYPE_RC 0x4
> >
> > @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
> >
> > static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
> > {
> > + struct dw_pcie *pci = pcie->pci;
> > + struct device *dev = pci->dev;
> > +
> > + /* Enable cache snooping for SA8775P */
> > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p"))
>
> Obviously: please populate a flag in the data structures instead of
> doing of_device_is_compatible(). Same applies to the patch 2.
>
Not necessary at this point. For some unknown reasons, the HW team ended up
disabling cache snooping on this specific platform. Whereas on other platforms,
it is enabled by default. So I have low expectations that we would need this
setting on other platforms in the future.
My concern with the usage of flag is that it warrants a new "qcom_pcie_cfg"
instance just for this quirk and it looks overkill to me.
So if we endup seeing this behavior on other platforms as well (unlikely) then
we can switch to the flag approach.
- Mani
> > + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE);
> > +
> > qcom_pcie_clear_hpc(pcie->pci);
> >
> > return 0;
>
>
>
> --
> With best wishes
> Dmitry
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-11-02 16:36 ` Manivannan Sadhasivam
@ 2023-11-02 22:25 ` Konrad Dybcio
2023-11-03 7:58 ` Manivannan Sadhasivam
0 siblings, 1 reply; 14+ messages in thread
From: Konrad Dybcio @ 2023-11-02 22:25 UTC (permalink / raw)
To: Manivannan Sadhasivam, Dmitry Baryshkov
Cc: Mrinmay Sarkar, agross, andersson, krzysztof.kozlowski+dt,
conor+dt, robh+dt, quic_shazhuss, quic_nitegupt, quic_ramkri,
quic_nayiluri, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
On 02/11/2023 17:36, Manivannan Sadhasivam wrote:
> On Thu, Nov 02, 2023 at 05:34:24PM +0200, Dmitry Baryshkov wrote:
>> On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote:
>>>
>>> This change will enable cache snooping logic to support
>>> cache coherency for SA8755P RC platform.
>>>
>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
>>> ---
>>> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>> index 6902e97..6f240fc 100644
>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>> @@ -51,6 +51,7 @@
>>> #define PARF_SID_OFFSET 0x234
>>> #define PARF_BDF_TRANSLATE_CFG 0x24c
>>> #define PARF_SLV_ADDR_SPACE_SIZE 0x358
>>> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
>>> #define PARF_DEVICE_TYPE 0x1000
>>> #define PARF_BDF_TO_SID_TABLE_N 0x2000
>>>
>>> @@ -117,6 +118,9 @@
>>> /* PARF_LTSSM register fields */
>>> #define LTSSM_EN BIT(8)
>>>
>>> +/* PARF_NO_SNOOP_OVERIDE register value */
>>> +#define NO_SNOOP_OVERIDE_EN 0xa
>>> +
>>> /* PARF_DEVICE_TYPE register fields */
>>> #define DEVICE_TYPE_RC 0x4
>>>
>>> @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>>>
>>> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
>>> {
>>> + struct dw_pcie *pci = pcie->pci;
>>> + struct device *dev = pci->dev;
>>> +
>>> + /* Enable cache snooping for SA8775P */
>>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p"))
>>
>> Obviously: please populate a flag in the data structures instead of
>> doing of_device_is_compatible(). Same applies to the patch 2.
>>
>
> Not necessary at this point. For some unknown reasons, the HW team ended up
> disabling cache snooping on this specific platform. Whereas on other platforms,
> it is enabled by default. So I have low expectations that we would need this
> setting on other platforms in the future.
>
> My concern with the usage of flag is that it warrants a new "qcom_pcie_cfg"
> instance just for this quirk and it looks overkill to me.
>
> So if we endup seeing this behavior on other platforms as well (unlikely) then
> we can switch to the flag approach.
This register reads zeroes on 8250, can we confirm it works as
expected there? I guess some benchmarks with and without
'dma-coherent'?
Konrad
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-11-02 22:25 ` Konrad Dybcio
@ 2023-11-03 7:58 ` Manivannan Sadhasivam
2023-11-06 7:19 ` Mrinmay Sarkar
0 siblings, 1 reply; 14+ messages in thread
From: Manivannan Sadhasivam @ 2023-11-03 7:58 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Manivannan Sadhasivam, Dmitry Baryshkov, Mrinmay Sarkar, agross,
andersson, krzysztof.kozlowski+dt, conor+dt, robh+dt,
quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri, robh,
quic_krichai, quic_vbadigan, quic_parass, quic_schintav,
quic_shijjose, Lorenzo Pieralisi, Krzysztof Wilczyński,
Bjorn Helgaas, linux-arm-msm, devicetree, linux-kernel, linux-pci
On Thu, Nov 02, 2023 at 11:25:36PM +0100, Konrad Dybcio wrote:
>
>
> On 02/11/2023 17:36, Manivannan Sadhasivam wrote:
> > On Thu, Nov 02, 2023 at 05:34:24PM +0200, Dmitry Baryshkov wrote:
> > > On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote:
> > > >
> > > > This change will enable cache snooping logic to support
> > > > cache coherency for SA8755P RC platform.
> > > >
> > > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
> > > > ---
> > > > drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
> > > > 1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > index 6902e97..6f240fc 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > @@ -51,6 +51,7 @@
> > > > #define PARF_SID_OFFSET 0x234
> > > > #define PARF_BDF_TRANSLATE_CFG 0x24c
> > > > #define PARF_SLV_ADDR_SPACE_SIZE 0x358
> > > > +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
> > > > #define PARF_DEVICE_TYPE 0x1000
> > > > #define PARF_BDF_TO_SID_TABLE_N 0x2000
> > > >
> > > > @@ -117,6 +118,9 @@
> > > > /* PARF_LTSSM register fields */
> > > > #define LTSSM_EN BIT(8)
> > > >
> > > > +/* PARF_NO_SNOOP_OVERIDE register value */
> > > > +#define NO_SNOOP_OVERIDE_EN 0xa
> > > > +
> > > > /* PARF_DEVICE_TYPE register fields */
> > > > #define DEVICE_TYPE_RC 0x4
> > > >
> > > > @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
> > > >
> > > > static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
> > > > {
> > > > + struct dw_pcie *pci = pcie->pci;
> > > > + struct device *dev = pci->dev;
> > > > +
> > > > + /* Enable cache snooping for SA8775P */
> > > > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p"))
> > >
> > > Obviously: please populate a flag in the data structures instead of
> > > doing of_device_is_compatible(). Same applies to the patch 2.
> > >
> >
> > Not necessary at this point. For some unknown reasons, the HW team ended up
> > disabling cache snooping on this specific platform. Whereas on other platforms,
> > it is enabled by default. So I have low expectations that we would need this
> > setting on other platforms in the future.
> >
> > My concern with the usage of flag is that it warrants a new "qcom_pcie_cfg"
> > instance just for this quirk and it looks overkill to me.
> >
> > So if we endup seeing this behavior on other platforms as well (unlikely) then
> > we can switch to the flag approach.
> This register reads zeroes on 8250, can we confirm it works as
> expected there?
I don't know if this register is even implemented in 8250. Mrinmay, can you
check?
> I guess some benchmarks with and without
> 'dma-coherent'?
>
The performance benefit can be measured by saturating the link. But it is
obvious that snooping the cache will give better performance (plus it avoids
cache flush in kernel).
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC
2023-11-03 7:58 ` Manivannan Sadhasivam
@ 2023-11-06 7:19 ` Mrinmay Sarkar
0 siblings, 0 replies; 14+ messages in thread
From: Mrinmay Sarkar @ 2023-11-06 7:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Konrad Dybcio
Cc: Dmitry Baryshkov, agross, andersson, krzysztof.kozlowski+dt,
conor+dt, robh+dt, quic_shazhuss, quic_nitegupt, quic_ramkri,
quic_nayiluri, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
On 11/3/2023 1:28 PM, Manivannan Sadhasivam wrote:
> On Thu, Nov 02, 2023 at 11:25:36PM +0100, Konrad Dybcio wrote:
>>
>> On 02/11/2023 17:36, Manivannan Sadhasivam wrote:
>>> On Thu, Nov 02, 2023 at 05:34:24PM +0200, Dmitry Baryshkov wrote:
>>>> On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote:
>>>>> This change will enable cache snooping logic to support
>>>>> cache coherency for SA8755P RC platform.
>>>>>
>>>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
>>>>> ---
>>>>> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
>>>>> 1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> index 6902e97..6f240fc 100644
>>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> @@ -51,6 +51,7 @@
>>>>> #define PARF_SID_OFFSET 0x234
>>>>> #define PARF_BDF_TRANSLATE_CFG 0x24c
>>>>> #define PARF_SLV_ADDR_SPACE_SIZE 0x358
>>>>> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
>>>>> #define PARF_DEVICE_TYPE 0x1000
>>>>> #define PARF_BDF_TO_SID_TABLE_N 0x2000
>>>>>
>>>>> @@ -117,6 +118,9 @@
>>>>> /* PARF_LTSSM register fields */
>>>>> #define LTSSM_EN BIT(8)
>>>>>
>>>>> +/* PARF_NO_SNOOP_OVERIDE register value */
>>>>> +#define NO_SNOOP_OVERIDE_EN 0xa
>>>>> +
>>>>> /* PARF_DEVICE_TYPE register fields */
>>>>> #define DEVICE_TYPE_RC 0x4
>>>>>
>>>>> @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>>>>>
>>>>> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
>>>>> {
>>>>> + struct dw_pcie *pci = pcie->pci;
>>>>> + struct device *dev = pci->dev;
>>>>> +
>>>>> + /* Enable cache snooping for SA8775P */
>>>>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p"))
>>>> Obviously: please populate a flag in the data structures instead of
>>>> doing of_device_is_compatible(). Same applies to the patch 2.
>>>>
>>> Not necessary at this point. For some unknown reasons, the HW team ended up
>>> disabling cache snooping on this specific platform. Whereas on other platforms,
>>> it is enabled by default. So I have low expectations that we would need this
>>> setting on other platforms in the future.
>>>
>>> My concern with the usage of flag is that it warrants a new "qcom_pcie_cfg"
>>> instance just for this quirk and it looks overkill to me.
>>>
>>> So if we endup seeing this behavior on other platforms as well (unlikely) then
>>> we can switch to the flag approach.
>> This register reads zeroes on 8250, can we confirm it works as
>> expected there?
> I don't know if this register is even implemented in 8250. Mrinmay, can you
> check?
Yes we have this register in 8250 platform as well
and I can see the default value is 0x0.
--Mrinmay
>> I guess some benchmarks with and without
>> 'dma-coherent'?
>>
> The performance benefit can be measured by saturating the link. But it is
> obvious that snooping the cache will give better performance (plus it avoids
> cache flush in kernel).
>
> - Mani
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 2/3] PCI: qcom-ep: Enable cache coherency for SA8775P EP
2023-10-31 15:46 [PATCH v1 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Mrinmay Sarkar
2023-10-31 15:46 ` [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC Mrinmay Sarkar
@ 2023-10-31 15:46 ` Mrinmay Sarkar
2023-10-31 16:50 ` Konrad Dybcio
2023-10-31 15:46 ` [PATCH v1 3/3] arm64: dts: qcom: sa8775p: Mark PCIe controller as cache coherent Mrinmay Sarkar
2023-10-31 16:57 ` [PATCH v1 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Manivannan Sadhasivam
3 siblings, 1 reply; 14+ messages in thread
From: Mrinmay Sarkar @ 2023-10-31 15:46 UTC (permalink / raw)
To: agross, andersson, krzysztof.kozlowski+dt, conor+dt,
konrad.dybcio, mani, robh+dt
Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
dmitry.baryshkov, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Mrinmay Sarkar, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
This change will enable cache snooping logic to support
cache coherency for SA8755P EP platform.
Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
---
drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 3a53d97..bc958a0 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -47,6 +47,7 @@
#define PARF_DBI_BASE_ADDR_HI 0x354
#define PARF_SLV_ADDR_SPACE_SIZE 0x358
#define PARF_SLV_ADDR_SPACE_SIZE_HI 0x35c
+#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4
#define PARF_ATU_BASE_ADDR 0x634
#define PARF_ATU_BASE_ADDR_HI 0x638
#define PARF_SRIS_MODE 0x644
@@ -86,6 +87,9 @@
#define PARF_DEBUG_INT_CFG_BUS_MASTER_EN BIT(2)
#define PARF_DEBUG_INT_RADM_PM_TURNOFF BIT(3)
+/* PARF_NO_SNOOP_OVERIDE register value */
+#define NO_SNOOP_OVERIDE_EN 0xa
+
/* PARF_DEVICE_TYPE register fields */
#define PARF_DEVICE_TYPE_EP 0x0
@@ -489,6 +493,10 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
val |= BIT(8);
writel_relaxed(val, pcie_ep->parf + PARF_LTSSM);
+ /* Enable cache snooping for SA8775P */
+ if (of_device_is_compatible(dev->of_node, "qcom,sa8775p-pcie-ep"))
+ writel_relaxed(NO_SNOOP_OVERIDE_EN, pcie_ep->parf + PCIE_PARF_NO_SNOOP_OVERIDE);
+
return 0;
err_disable_resources:
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/3] PCI: qcom-ep: Enable cache coherency for SA8775P EP
2023-10-31 15:46 ` [PATCH v1 2/3] PCI: qcom-ep: Enable cache coherency for SA8775P EP Mrinmay Sarkar
@ 2023-10-31 16:50 ` Konrad Dybcio
0 siblings, 0 replies; 14+ messages in thread
From: Konrad Dybcio @ 2023-10-31 16:50 UTC (permalink / raw)
To: Mrinmay Sarkar, agross, andersson, krzysztof.kozlowski+dt,
conor+dt, mani, robh+dt
Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
dmitry.baryshkov, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
On 31.10.2023 16:46, Mrinmay Sarkar wrote:
> This change will enable cache snooping logic to support
> cache coherency for SA8755P EP platform.
>
> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
> ---
same comments as patch 1
Konrad
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 3/3] arm64: dts: qcom: sa8775p: Mark PCIe controller as cache coherent
2023-10-31 15:46 [PATCH v1 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Mrinmay Sarkar
2023-10-31 15:46 ` [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC Mrinmay Sarkar
2023-10-31 15:46 ` [PATCH v1 2/3] PCI: qcom-ep: Enable cache coherency for SA8775P EP Mrinmay Sarkar
@ 2023-10-31 15:46 ` Mrinmay Sarkar
2023-10-31 16:57 ` [PATCH v1 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Manivannan Sadhasivam
3 siblings, 0 replies; 14+ messages in thread
From: Mrinmay Sarkar @ 2023-10-31 15:46 UTC (permalink / raw)
To: agross, andersson, krzysztof.kozlowski+dt, conor+dt,
konrad.dybcio, mani, robh+dt
Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
dmitry.baryshkov, robh, quic_krichai, quic_vbadigan, quic_parass,
quic_schintav, quic_shijjose, Mrinmay Sarkar, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
devicetree, linux-kernel, linux-pci
The PCIe controller on SA8775P supports cache coherency, hence add the
"dma-coherent" property to mark it as such.
Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
---
arch/arm64/boot/dts/qcom/sa8775p.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 7eab458..7df941d 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -3620,6 +3620,8 @@
<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_PCIE_0 0>;
interconnect-names = "pcie-mem", "cpu-pcie";
+ dma-coherent;
+
iommus = <&pcie_smmu 0x0000 0x7f>;
resets = <&gcc GCC_PCIE_0_BCR>;
reset-names = "core";
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P
2023-10-31 15:46 [PATCH v1 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Mrinmay Sarkar
` (2 preceding siblings ...)
2023-10-31 15:46 ` [PATCH v1 3/3] arm64: dts: qcom: sa8775p: Mark PCIe controller as cache coherent Mrinmay Sarkar
@ 2023-10-31 16:57 ` Manivannan Sadhasivam
3 siblings, 0 replies; 14+ messages in thread
From: Manivannan Sadhasivam @ 2023-10-31 16:57 UTC (permalink / raw)
To: Mrinmay Sarkar
Cc: agross, andersson, krzysztof.kozlowski+dt, conor+dt,
konrad.dybcio, mani, robh+dt, quic_shazhuss, quic_nitegupt,
quic_ramkri, quic_nayiluri, dmitry.baryshkov, robh, quic_krichai,
quic_vbadigan, quic_parass, quic_schintav, quic_shijjose,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
linux-arm-msm, devicetree, linux-kernel, linux-pci
On Tue, Oct 31, 2023 at 09:16:23PM +0530, Mrinmay Sarkar wrote:
> This series is to enable cache snooping logic in both RC and EP
> driver and add the "dma-coherent" property in dtsi to support
> cache coherency in SA8775P.
>
You need to explain why cache snooping needs to be enabled _only_ on SA8775P and
not on other platforms, here and also in patch description.
Cover letter is not just meant for describing the changes you are submitting,
you should also put as much info required for the reviewers/maintainers to get
motivation behind the series.
- Mani
> To verify this series we required [1]
>
> [1] https://lore.kernel.org/all/1698729108-27356-1-git-send-email-quic_msarkar@quicinc.com/
>
> Mrinmay Sarkar (3):
> PCI: qcom: Enable cache coherency for SA8775P RC
> PCI: qcom-ep: Enable cache coherency for SA8775P EP
> arm64: dts: qcom: sa8775p: Mark PCIe controller as cache coherent
>
> arch/arm64/boot/dts/qcom/sa8775p.dtsi | 2 ++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 ++++++++
> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
> 3 files changed, 21 insertions(+)
>
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 14+ messages in thread