[PATCH 0/2] iommu/arm-smmu-qcom: Add SDM670 SMMU v2

[PATCH 0/2] iommu/arm-smmu-qcom: Add SDM670 SMMU v2

[Richard Acayan]

This adds the SMMU v2 for the Snapdragon 670, used for the Adreno GPU.

Richard Acayan (2):
  dt-bindings: iommu: arm,smmu: add sdm670 adreno iommu compatible
  iommu/arm-smmu-qcom: add sdm670 adreno iommu compatible

 Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 +
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c            | 1 +
 2 files changed, 2 insertions(+)

-- 
2.45.2

[PATCH 1/2] dt-bindings: iommu: arm,smmu: add sdm670 adreno iommu compatible

[Richard Acayan]

SDM670 has a separate IOMMU for the GPU, like SDM845. Add the compatible
for it.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
 Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 280b4e49f219..c672f7b6a2ae 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -116,6 +116,7 @@ properties:
               - qcom,msm8996-smmu-v2
               - qcom,sc7180-smmu-v2
               - qcom,sdm630-smmu-v2
+              - qcom,sdm670-smmu-v2
               - qcom,sdm845-smmu-v2
               - qcom,sm6350-smmu-v2
               - qcom,sm7150-smmu-v2
-- 
2.45.2

[PATCH 2/2] iommu/arm-smmu-qcom: add sdm670 adreno iommu compatible

[Richard Acayan]

Add the compatible for the separate IOMMU on SDM670 for the Adreno GPU.

Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 36c6b36ad4ff..7f4b15be4a11 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -539,6 +539,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
 	{ .compatible = "qcom,sc8180x-smmu-500", .data = &qcom_smmu_500_impl0_data },
 	{ .compatible = "qcom,sc8280xp-smmu-500", .data = &qcom_smmu_500_impl0_data },
 	{ .compatible = "qcom,sdm630-smmu-v2", .data = &qcom_smmu_v2_data },
+	{ .compatible = "qcom,sdm670-smmu-v2", .data = &qcom_smmu_v2_data },
 	{ .compatible = "qcom,sdm845-smmu-v2", .data = &qcom_smmu_v2_data },
 	{ .compatible = "qcom,sdm845-smmu-500", .data = &sdm845_smmu_500_data },
 	{ .compatible = "qcom,sm6115-smmu-500", .data = &qcom_smmu_500_impl0_data},
-- 
2.45.2

Re: [PATCH 2/2] iommu/arm-smmu-qcom: add sdm670 adreno iommu compatible

[Krzysztof Kozlowski]

On 30/07/2024 03:38, Richard Acayan wrote:
> Add the compatible for the separate IOMMU on SDM670 for the Adreno GPU.
> 
> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 36c6b36ad4ff..7f4b15be4a11 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -539,6 +539,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
>  	{ .compatible = "qcom,sc8180x-smmu-500", .data = &qcom_smmu_500_impl0_data },
>  	{ .compatible = "qcom,sc8280xp-smmu-500", .data = &qcom_smmu_500_impl0_data },
>  	{ .compatible = "qcom,sdm630-smmu-v2", .data = &qcom_smmu_v2_data },
> +	{ .compatible = "qcom,sdm670-smmu-v2", .data = &qcom_smmu_v2_data },

Do the people even read the comments? What the comment is saying? I
admit it might be not precise, but "500" and "v2" are the same from that
point of view.

NAK.

Best regards,
Krzysztof

Re: [PATCH 2/2] iommu/arm-smmu-qcom: add sdm670 adreno iommu compatible

[Dmitry Baryshkov]

On Tue, Jul 30, 2024 at 08:23:44AM GMT, Krzysztof Kozlowski wrote:
> On 30/07/2024 03:38, Richard Acayan wrote:
> > Add the compatible for the separate IOMMU on SDM670 for the Adreno GPU.
> > 
> > Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> > ---
> >  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > index 36c6b36ad4ff..7f4b15be4a11 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > @@ -539,6 +539,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
> >  	{ .compatible = "qcom,sc8180x-smmu-500", .data = &qcom_smmu_500_impl0_data },
> >  	{ .compatible = "qcom,sc8280xp-smmu-500", .data = &qcom_smmu_500_impl0_data },
> >  	{ .compatible = "qcom,sdm630-smmu-v2", .data = &qcom_smmu_v2_data },
> > +	{ .compatible = "qcom,sdm670-smmu-v2", .data = &qcom_smmu_v2_data },
> 
> Do the people even read the comments? What the comment is saying? I
> admit it might be not precise, but "500" and "v2" are the same from that
> point of view.

Well, the comment speaks about -500 explicitly, because -v2 have
differences (it is known that these platforms require more quirks).

> 
> NAK.
> 
> Best regards,
> Krzysztof
> 

-- 
With best wishes
Dmitry

Re: [PATCH 0/2] iommu/arm-smmu-qcom: Add SDM670 SMMU v2

[Richard Acayan]

On Mon, Jul 29, 2024 at 09:38:21PM -0400, Richard Acayan wrote:
> This adds the SMMU v2 for the Snapdragon 670, used for the Adreno GPU.
> 
> Richard Acayan (2):
>   dt-bindings: iommu: arm,smmu: add sdm670 adreno iommu compatible
>   iommu/arm-smmu-qcom: add sdm670 adreno iommu compatible
> 
>  Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 +
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c            | 1 +
>  2 files changed, 2 insertions(+)

Hi, I'm wondering if this is still being considered.

The NAK doesn't really make sense here, as there's a warning/BUG at the
bottom of the qcom SMMU driver that occurs when the compatible isn't
added:

	/*
	 * If you hit this WARN_ON() you are missing an entry in the
	 * qcom_smmu_impl_of_match[] table, and GPU per-process page-
	 * tables will be broken.
	 */
	WARN(of_device_is_compatible(np, "qcom,adreno-smmu"),
	     "Missing qcom_smmu_impl_of_match entry for: %s",
	     dev_name(smmu->dev));

DTS change for context (pending):
https://lore.kernel.org/linux-arm-msm/20240806214452.16406-10-mailingradian@gmail.com

Re: [PATCH 0/2] iommu/arm-smmu-qcom: Add SDM670 SMMU v2

[Dmitry Baryshkov]

On Tue, Oct 29, 2024 at 05:22:46PM -0400, Richard Acayan wrote:
> On Mon, Jul 29, 2024 at 09:38:21PM -0400, Richard Acayan wrote:
> > This adds the SMMU v2 for the Snapdragon 670, used for the Adreno GPU.
> > 
> > Richard Acayan (2):
> >   dt-bindings: iommu: arm,smmu: add sdm670 adreno iommu compatible
> >   iommu/arm-smmu-qcom: add sdm670 adreno iommu compatible
> > 
> >  Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 +
> >  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c            | 1 +
> >  2 files changed, 2 insertions(+)
> 
> Hi, I'm wondering if this is still being considered.
> 
> The NAK doesn't really make sense here, as there's a warning/BUG at the
> bottom of the qcom SMMU driver that occurs when the compatible isn't
> added:

Please resend it, adding an explicit note about having no generic
-v2 fallback and that being a correct way. I don't think Krzysztof
reacts to old patches.

> 
> 	/*
> 	 * If you hit this WARN_ON() you are missing an entry in the
> 	 * qcom_smmu_impl_of_match[] table, and GPU per-process page-
> 	 * tables will be broken.
> 	 */
> 	WARN(of_device_is_compatible(np, "qcom,adreno-smmu"),
> 	     "Missing qcom_smmu_impl_of_match entry for: %s",
> 	     dev_name(smmu->dev));
> 
> DTS change for context (pending):
> https://lore.kernel.org/linux-arm-msm/20240806214452.16406-10-mailingradian@gmail.com

-- 
With best wishes
Dmitry