Hi Bryan, On 12/9/2024 5:22 PM, Bryan O'Donoghue wrote: > v5: > - Fixes venus_remove_dynamic_nodes() on probe err path - Dikshita > - Link to v4: https://lore.kernel.org/r/20241128-media-staging-24-11-25-rb3-hw-compat-string-v4-0-fd062b399374@xxxxxxxxxx > > v4: > > - Adds some unavoidable conditional CONFIG_OF_DYNAMIC to fix media-ci testcase # Test build:OF x86_64 > - Added logic for of_changeset_revert() and of_changeset_destroy() on > error/remove paths - Bryan > - Link to v3: https://lore.kernel.org/r/20241127-media-staging-24-11-25-rb3-hw-compat-string-v3-0-ef6bd25e98db@xxxxxxxxxx > > v3: > - Adds select OF_DYNAMIC to venus/Kconfig to ensure of_changeset_*() is > available. Instead of ifdefing and have the fix not work without > OF_DYNAMIC, select OF_DYANMIC with venus - linux-media-ci > - Link to v2: https://lore.kernel.org/r/20241127-media-staging-24-11-25-rb3-hw-compat-string-v2-0-c010fd45f7ff@xxxxxxxxxx > > v2: > - Removes useless dev_info() leftover from debugging - Bryan > Link: https://lore.kernel.org/r/ce9ac473-2f73-4c7a-97b1-08be39f3adb4@xxxxxxxxxx > - Trivial newline change @ np = of_changeset_create_node(ocs, dev->of_node, node_name); - Bryan > - Fixes a missing goto identified by smatch - Smatch/Bryan > - Adds Krzysztof's RB to deprecated - Krzysztof > - Link to v1: https://lore.kernel.org/r/20241127-media-staging-24-11-25-rb3-hw-compat-string-v1-0-99c16f266b46@xxxxxxxxxx > > v1: > Various venus patches have been held up due to the misuse of DT to provide > a configuration input to venus as to which mode a given transcoder should > be in. > > Link: https://lore.kernel.org/linux-arm-msm/436145fd-d65f-44ec-b950-c434775187ca@xxxxxxxxxx > Link: https://lore.kernel.org/linux-media/ba40de82-b308-67b1-5751-bb2d95f2b8a5@xxxxxxxxxx/ > > This series provides support for static configuration of venus from the resource > structure via: > > 1. Adding two strings to the resource structure. > One string for the decoder one for the encoder. > 2. The string for each SoC has been matched to the existing in the > DT which currently specifies the mode as decoder or encoder. > 3. New logic in the driver parses the DTB looking for the node name > specified for the decoder and encoder . > 4. If the DTB contains the node name, then no new node is added as > we assume to be working with an "old" DTB. > 5. If the DTB does not contain the specified decoder/encoder string > then a new in-memory node is added which contains a compat string > consistent with upstream compat strings used to currently select > between the decoder and encoder respectively. > 6. In this way new venus driver entries may be added which respect > the requirement to move mode selection out of DTB and into driver. > 7. Simple instances of decoder/encoder nodes in the yaml schema have been > marked as deprecated. > 8. Since the proposed scheme here always defers to what the DTB says that > means it would be possible to remove decoder/encoder entries for the > deprecated schema should we choose to do so at a later date but, > that step is not taken in this series. > 9. Some of the upstream encoder/decoder nodes for example sdm630/sdm660 > also contain clock and power-domain information and have not been > updated with the static configuration data or had the schema amended to > deprecate values. Because these nodes impart hardware specific > information and are already upstream this series proposes to leave > those as-is. > > However if this scheme is adopted it should allow for addition of venus for > both qcs615[1] and sc8280xp[2]. > > Other SoCs such as sm8550, sm8650 and beyond are expected to be supported > by Iris. > > The sm8350 and sm8280xp in the second series would then be able to excise > the offending compat = "video-encoder" | "video-decoder" in the schema and > DT. > > I considered making this series an all singing all dancing method to select > between encoder and decoder for all SoCs but, the objective here is not to > add functionality but to provide support for configuration in-driver > consistent with current usage and to do so with a minimal code > intervention. > > So far I've tested on RB3 by removing: > > video-core0 { > compatible = "venus-decoder"; > }; > > video-core1 { > compatible = "venus-encoder"; > }; > > This works - the code adds the nodes into memory and the video > encoder/decoder logic in the plaform code runs. > > Similarly if the nodes are left in-place then no new nodes are added by the > code in this series and still both encoder and decoder probe. > > Thus proving the code works and will provide support for new platforms > while also leaving open the option of dropping nodes from upstream. > > I've left the dropping step out for now, it can be implemented later. > > [1] https://lore.kernel.org/linux-arm-msm/20241125-add-venus-for-qcs615-v3-0-5a376b97a68e@xxxxxxxxxxx > [2] https://lore.kernel.org/linux-media/20230731-topic-8280_venus-v1-0-8c8bbe1983a5@xxxxxxxxxx/ > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> Thank you for the changes, looks good to me. Let me come back with some validation on few SOCs and update here. Regards, Vikash > --- > Bryan O'Donoghue (3): > media: venus: Add support for static video encoder/decoder declarations > media: venus: Populate video encoder/decoder nodename entries > media: dt-bindings: qcom-venus: Deprecate video-decoder and video-encoder where applicable > > .../bindings/media/qcom,msm8916-venus.yaml | 12 +-- > .../bindings/media/qcom,sc7180-venus.yaml | 12 +-- > .../bindings/media/qcom,sc7280-venus.yaml | 12 +-- > .../bindings/media/qcom,sdm845-venus-v2.yaml | 12 +-- > .../bindings/media/qcom,sm8250-venus.yaml | 12 +-- > drivers/media/platform/qcom/venus/Kconfig | 1 + > drivers/media/platform/qcom/venus/core.c | 104 ++++++++++++++++++++- > drivers/media/platform/qcom/venus/core.h | 4 + > 8 files changed, 118 insertions(+), 51 deletions(-) > --- > base-commit: 72ad4ff638047bbbdf3232178fea4bec1f429319 > change-id: 20241127-media-staging-24-11-25-rb3-hw-compat-string-ea3c99938021 > > Best regards,
