diff mbox series

[v5,3/3] media: dt-bindings: qcom-venus: Deprecate video-decoder and video-encoder where applicable

Message ID 20241209-media-staging-24-11-25-rb3-hw-compat-string-v5-3-ef7e5f85f302@linaro.org (mailing list archive)
State New
Headers show
Series media: venus: Provide support for selecting encoder/decoder from in-driver | expand

Commit Message

Bryan O'Donoghue Dec. 9, 2024, 11:52 a.m. UTC
For the list of yaml files here the video-decoder and video-encoder nodes
provide nothing more than configuration input for the driver. These entries
do not in fact impart hardware specific data and should be deprecated.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../devicetree/bindings/media/qcom,msm8916-venus.yaml        | 12 ++----------
 .../devicetree/bindings/media/qcom,sc7180-venus.yaml         | 12 ++----------
 .../devicetree/bindings/media/qcom,sc7280-venus.yaml         | 12 ++----------
 .../devicetree/bindings/media/qcom,sdm845-venus-v2.yaml      | 12 ++----------
 .../devicetree/bindings/media/qcom,sm8250-venus.yaml         | 12 ++----------
 5 files changed, 10 insertions(+), 50 deletions(-)

Comments

Renjiang Han Dec. 18, 2024, 10:46 a.m. UTC | #1
On 12/9/2024 7:52 PM, Bryan O'Donoghue wrote:
> For the list of yaml files here the video-decoder and video-encoder nodes
> provide nothing more than configuration input for the driver. These entries
> do not in fact impart hardware specific data and should be deprecated.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   .../devicetree/bindings/media/qcom,msm8916-venus.yaml        | 12 ++----------
>   .../devicetree/bindings/media/qcom,sc7180-venus.yaml         | 12 ++----------
>   .../devicetree/bindings/media/qcom,sc7280-venus.yaml         | 12 ++----------
>   .../devicetree/bindings/media/qcom,sdm845-venus-v2.yaml      | 12 ++----------
>   .../devicetree/bindings/media/qcom,sm8250-venus.yaml         | 12 ++----------
>   5 files changed, 10 insertions(+), 50 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,msm8916-venus.yaml b/Documentation/devicetree/bindings/media/qcom,msm8916-venus.yaml
> index 9410f13ca97c181973c62fe62d0399fc9e82f05d..da140c2e3d3f3c3e886496e3e2303eda1df99bb4 100644
> --- a/Documentation/devicetree/bindings/media/qcom,msm8916-venus.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,msm8916-venus.yaml
> @@ -45,6 +45,7 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>     video-encoder:
> @@ -57,13 +58,12 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>   required:
>     - compatible
>     - iommus
> -  - video-decoder
> -  - video-encoder
>   
>   unevaluatedProperties: false
>   
> @@ -83,12 +83,4 @@ examples:
>           power-domains = <&gcc VENUS_GDSC>;
>           iommus = <&apps_iommu 5>;
>           memory-region = <&venus_mem>;
> -
> -        video-decoder {
> -            compatible = "venus-decoder";
> -        };
> -
> -        video-encoder {
> -            compatible = "venus-encoder";
> -        };
>       };
> diff --git a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
> index 5cec1d077cda77817f6d876109defcb0abbfeb2c..83c4a5d95f020437bd160d6456850bc84a2cf5ff 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
> @@ -70,6 +70,7 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>     video-encoder:
> @@ -82,14 +83,13 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>   required:
>     - compatible
>     - power-domain-names
>     - iommus
> -  - video-decoder
> -  - video-encoder
>   
>   unevaluatedProperties: false
>   
> @@ -114,12 +114,4 @@ examples:
>                         "vcodec0_core", "vcodec0_bus";
>           iommus = <&apps_smmu 0x0c00 0x60>;
>           memory-region = <&venus_mem>;
> -
> -        video-decoder {
> -            compatible = "venus-decoder";
> -        };
> -
> -        video-encoder {
> -            compatible = "venus-encoder";
> -        };
>       };
> diff --git a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
> index 10c334e6b3dcf25967fa438f8e6e5035448af1b9..413c5b4ee6504ba1d5fe9f74d5be04ad8c90c318 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
> @@ -68,6 +68,7 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>     video-encoder:
> @@ -80,14 +81,13 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>   required:
>     - compatible
>     - power-domain-names
>     - iommus
> -  - video-decoder
> -  - video-encoder
>   
>   unevaluatedProperties: false
>   
> @@ -125,14 +125,6 @@ examples:
>   
>           memory-region = <&video_mem>;
>   
> -        video-decoder {
> -            compatible = "venus-decoder";
> -        };
> -
> -        video-encoder {
> -            compatible = "venus-encoder";
> -        };
> -
>           video-firmware {
>               iommus = <&apps_smmu 0x21a2 0x0>;
>           };
> diff --git a/Documentation/devicetree/bindings/media/qcom,sdm845-venus-v2.yaml b/Documentation/devicetree/bindings/media/qcom,sdm845-venus-v2.yaml
> index 6228fd2b324631f3138e128c918266da58f6b544..c839cb1ebc0999e10b865f4bb43ea76ffa2bf46d 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sdm845-venus-v2.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sdm845-venus-v2.yaml
> @@ -70,6 +70,7 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>     video-core1:
> @@ -82,14 +83,13 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>   required:
>     - compatible
>     - power-domain-names
>     - iommus
> -  - video-core0
> -  - video-core1
>   
>   unevaluatedProperties: false
>   
> @@ -119,12 +119,4 @@ examples:
>           iommus = <&apps_smmu 0x10a0 0x8>,
>                    <&apps_smmu 0x10b0 0x0>;
>           memory-region = <&venus_mem>;
> -
> -        video-core0 {
> -            compatible = "venus-decoder";
> -        };
> -
> -        video-core1 {
> -            compatible = "venus-encoder";
> -        };
>       };
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml
> index f66033ae8b590e7b6f1e344c368994744411aca2..da54493220c9dc90e7d9f5fcfce7590acb241c85 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml
> @@ -73,6 +73,7 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>     video-encoder:
> @@ -85,6 +86,7 @@ properties:
>       required:
>         - compatible
>   
> +    deprecated: true
>       additionalProperties: false
>   
>   required:
> @@ -95,8 +97,6 @@ required:
>     - iommus
>     - resets
>     - reset-names
> -  - video-decoder
> -  - video-encoder
>   
>   unevaluatedProperties: false
>   
> @@ -132,12 +132,4 @@ examples:
>           resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>,
>                    <&videocc VIDEO_CC_MVS0C_CLK_ARES>;
>           reset-names = "bus", "core";
> -
> -        video-decoder {
> -            compatible = "venus-decoder";
> -        };
> -
> -        video-encoder {
> -            compatible = "venus-encoder";
> -        };
>       };
It is working fine on QCS615.
Tested-by: Renjiang Han <quic_renjiang@quicinc.com>
Krzysztof Kozlowski Dec. 18, 2024, 12:38 p.m. UTC | #2
On 18/12/2024 11:46, Renjiang Han wrote:
> 
> On 12/9/2024 7:52 PM, Bryan O'Donoghue wrote:
>> For the list of yaml files here the video-decoder and video-encoder nodes
>> provide nothing more than configuration input for the driver. These entries
>> do not in fact impart hardware specific data and should be deprecated.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---


Please kindly trim the replies from unnecessary context. It makes it
much easier to find new content.

>> @@ -132,12 +132,4 @@ examples:
>>           resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>,
>>                    <&videocc VIDEO_CC_MVS0C_CLK_ARES>;
>>           reset-names = "bus", "core";
>> -
>> -        video-decoder {
>> -            compatible = "venus-decoder";
>> -        };
>> -
>> -        video-encoder {
>> -            compatible = "venus-encoder";
>> -        };
>>       };
> It is working fine on QCS615.
> Tested-by: Renjiang Han <quic_renjiang@quicinc.com>


How? How is it possible to test a binding on real hardware? To my
knowledge it is impossible, so I really would like to see explanation of
this tag.

I recently voiced my concerns that I really dislike tested-by tags on
bindings, because they are fake or not accurate. Story continues...



Best regards,
Krzysztof
Renjiang Han Dec. 19, 2024, 2:45 a.m. UTC | #3
On 12/18/2024 8:38 PM, Krzysztof Kozlowski wrote:
>>> @@ -132,12 +132,4 @@ examples:
>>>            resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>,
>>>                     <&videocc VIDEO_CC_MVS0C_CLK_ARES>;
>>>            reset-names = "bus", "core";
>>> -
>>> -        video-decoder {
>>> -            compatible = "venus-decoder";
>>> -        };
>>> -
>>> -        video-encoder {
>>> -            compatible = "venus-encoder";
>>> -        };
>>>        };
>> It is working fine on QCS615.
>> Tested-by: Renjiang Han<quic_renjiang@quicinc.com>
> How? How is it possible to test a binding on real hardware? To my
> knowledge it is impossible, so I really would like to see explanation of
> this tag.
  I used this binding and modified the driver to verify the video
  function, and the result was fine. Also, I used this binding to
  verify the dtb, and there was no related error, so I added
  Tested-by.
Krzysztof Kozlowski Dec. 19, 2024, 8:16 a.m. UTC | #4
On 19/12/2024 03:45, Renjiang Han wrote:
> 
> On 12/18/2024 8:38 PM, Krzysztof Kozlowski wrote:
>>>> @@ -132,12 +132,4 @@ examples:
>>>>            resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>,
>>>>                     <&videocc VIDEO_CC_MVS0C_CLK_ARES>;
>>>>            reset-names = "bus", "core";
>>>> -
>>>> -        video-decoder {
>>>> -            compatible = "venus-decoder";
>>>> -        };
>>>> -
>>>> -        video-encoder {
>>>> -            compatible = "venus-encoder";
>>>> -        };
>>>>        };
>>> It is working fine on QCS615.
>>> Tested-by: Renjiang Han<quic_renjiang@quicinc.com>
>> How? How is it possible to test a binding on real hardware? To my
>> knowledge it is impossible, so I really would like to see explanation of
>> this tag.
>   I used this binding and modified the driver to verify the video
>   function, and the result was fine. Also, I used this binding to
>   verify the dtb, and there was no related error, so I added
>   Tested-by.

This is not a testing of binding. You tested driver. Don't add fake tags.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/qcom,msm8916-venus.yaml b/Documentation/devicetree/bindings/media/qcom,msm8916-venus.yaml
index 9410f13ca97c181973c62fe62d0399fc9e82f05d..da140c2e3d3f3c3e886496e3e2303eda1df99bb4 100644
--- a/Documentation/devicetree/bindings/media/qcom,msm8916-venus.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,msm8916-venus.yaml
@@ -45,6 +45,7 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
   video-encoder:
@@ -57,13 +58,12 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
 required:
   - compatible
   - iommus
-  - video-decoder
-  - video-encoder
 
 unevaluatedProperties: false
 
@@ -83,12 +83,4 @@  examples:
         power-domains = <&gcc VENUS_GDSC>;
         iommus = <&apps_iommu 5>;
         memory-region = <&venus_mem>;
-
-        video-decoder {
-            compatible = "venus-decoder";
-        };
-
-        video-encoder {
-            compatible = "venus-encoder";
-        };
     };
diff --git a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
index 5cec1d077cda77817f6d876109defcb0abbfeb2c..83c4a5d95f020437bd160d6456850bc84a2cf5ff 100644
--- a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
@@ -70,6 +70,7 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
   video-encoder:
@@ -82,14 +83,13 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
 required:
   - compatible
   - power-domain-names
   - iommus
-  - video-decoder
-  - video-encoder
 
 unevaluatedProperties: false
 
@@ -114,12 +114,4 @@  examples:
                       "vcodec0_core", "vcodec0_bus";
         iommus = <&apps_smmu 0x0c00 0x60>;
         memory-region = <&venus_mem>;
-
-        video-decoder {
-            compatible = "venus-decoder";
-        };
-
-        video-encoder {
-            compatible = "venus-encoder";
-        };
     };
diff --git a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
index 10c334e6b3dcf25967fa438f8e6e5035448af1b9..413c5b4ee6504ba1d5fe9f74d5be04ad8c90c318 100644
--- a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
@@ -68,6 +68,7 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
   video-encoder:
@@ -80,14 +81,13 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
 required:
   - compatible
   - power-domain-names
   - iommus
-  - video-decoder
-  - video-encoder
 
 unevaluatedProperties: false
 
@@ -125,14 +125,6 @@  examples:
 
         memory-region = <&video_mem>;
 
-        video-decoder {
-            compatible = "venus-decoder";
-        };
-
-        video-encoder {
-            compatible = "venus-encoder";
-        };
-
         video-firmware {
             iommus = <&apps_smmu 0x21a2 0x0>;
         };
diff --git a/Documentation/devicetree/bindings/media/qcom,sdm845-venus-v2.yaml b/Documentation/devicetree/bindings/media/qcom,sdm845-venus-v2.yaml
index 6228fd2b324631f3138e128c918266da58f6b544..c839cb1ebc0999e10b865f4bb43ea76ffa2bf46d 100644
--- a/Documentation/devicetree/bindings/media/qcom,sdm845-venus-v2.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sdm845-venus-v2.yaml
@@ -70,6 +70,7 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
   video-core1:
@@ -82,14 +83,13 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
 required:
   - compatible
   - power-domain-names
   - iommus
-  - video-core0
-  - video-core1
 
 unevaluatedProperties: false
 
@@ -119,12 +119,4 @@  examples:
         iommus = <&apps_smmu 0x10a0 0x8>,
                  <&apps_smmu 0x10b0 0x0>;
         memory-region = <&venus_mem>;
-
-        video-core0 {
-            compatible = "venus-decoder";
-        };
-
-        video-core1 {
-            compatible = "venus-encoder";
-        };
     };
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml
index f66033ae8b590e7b6f1e344c368994744411aca2..da54493220c9dc90e7d9f5fcfce7590acb241c85 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml
@@ -73,6 +73,7 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
   video-encoder:
@@ -85,6 +86,7 @@  properties:
     required:
       - compatible
 
+    deprecated: true
     additionalProperties: false
 
 required:
@@ -95,8 +97,6 @@  required:
   - iommus
   - resets
   - reset-names
-  - video-decoder
-  - video-encoder
 
 unevaluatedProperties: false
 
@@ -132,12 +132,4 @@  examples:
         resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>,
                  <&videocc VIDEO_CC_MVS0C_CLK_ARES>;
         reset-names = "bus", "core";
-
-        video-decoder {
-            compatible = "venus-decoder";
-        };
-
-        video-encoder {
-            compatible = "venus-encoder";
-        };
     };