Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650

[Vikash Garodia]

Hi Neil,

On 4/14/2025 1:05 PM, Neil Armstrong wrote:
> Hi Vikash, Dikshita,
> 
> On 10/04/2025 18:29, Neil Armstrong wrote:
>> Re-organize the platform support core into a gen1 catalog C file
>> declaring common platform structure and include platform headers
>> containing platform specific entries and iris_platform_data
>> structure.
>>
>> The goal is to share most of the structure while having
>> clear and separate per-SoC catalog files.
>>
>> The organization is based on the curent drm/msm dpu1 catalog
>> entries.
> 
> Any feedback on this patchset ?
Myself and Dikshita went through the approach you are bringing here, let me
update some context here:
- sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the scaled
down variant i.e have 2 PIPE vs others having 4. Similarly there are other
irisv3 having 1 pipe as well.
- With above variations, firmware and instance caps would change for the variant
SOCs.
- Above these, few(less) bindings/connections specific delta would be there,
like there is reset delta in sm8550 and sm8650.

Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific tables and
SOC platform data, i.e sm8650_data (for sm8650). On top of this, individual SOC
specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
preset register table, etc and export these delta structs in xxx_gen1.c or
xxx_gen2.c.

Going with above approach, sm8650.c would have only one reset table for now.
Later if any delta is identified, the same can be added in it. All other common
structs, can reside in xxx_gen2.c for now.

Regards,
Vikash
> 
> Thanks,
> Neil
> 
>>
>> Add support for the IRIS accelerator for the SM8650
>> platform, which uses the iris33 hardware.
>>
>> The vpu33 requires a different reset & poweroff sequence
>> in order to properly get out of runtime suspend.
>>
>> Follow-up of [1]:
>> https://lore.kernel.org/all/20250409-topic-sm8x50-iris-v10-v4-0-40e411594285@xxxxxxxxxx/
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
>> ---
>> Changes in v4:
>> - Reorganized into catalog, rebased sm8650 support on top
>> - Link to v4:
>> https://lore.kernel.org/all/20250409-topic-sm8x50-iris-v10-v4-0-40e411594285@xxxxxxxxxx
>>
>> Changes in v4:
>> - collected tags
>> - un-split power_off in vpu3x
>> - removed useless function defines
>> - added back vpu3x disappeared rename commit
>> - Link to v3:
>> https://lore.kernel.org/r/20250407-topic-sm8x50-iris-v10-v3-0-63569f6d04aa@xxxxxxxxxx
>>
>> Changes in v3:
>> - Collected review tags
>> - Removed bulky reset_controller ops
>> - Removed iris_vpu_power_off_controller split
>> - Link to v2:
>> https://lore.kernel.org/r/20250305-topic-sm8x50-iris-v10-v2-0-bd65a3fc099e@xxxxxxxxxx
>>
>> Changes in v2:
>> - Collected bindings review
>> - Reworked rest handling by adding a secondary optional table to be used by
>> controller poweroff
>> - Reworked power_off_controller to be reused and extended by vpu33 support
>> - Removed useless and unneeded vpu33 init
>> - Moved vpu33 into vpu3x files to reuse code from vpu3
>> - Moved sm8650 data table into sm8550
>> - Link to v1:
>> https://lore.kernel.org/r/20250225-topic-sm8x50-iris-v10-v1-0-128ef05d9665@xxxxxxxxxx
>>
>> ---
>> Neil Armstrong (8):
>>        media: qcom: iris: move sm8250 to gen1 catalog
>>        media: qcom: iris: move sm8550 to gen2 catalog
>>        dt-bindings: media: qcom,sm8550-iris: document SM8650 IRIS accelerator
>>        media: platform: qcom/iris: add power_off_controller to vpu_ops
>>        media: platform: qcom/iris: introduce optional controller_rst_tbl
>>        media: platform: qcom/iris: rename iris_vpu3 to iris_vpu3x
>>        media: platform: qcom/iris: add support for vpu33
>>        media: platform: qcom/iris: add sm8650 support
>>
>>   .../bindings/media/qcom,sm8550-iris.yaml           |  33 ++-
>>   drivers/media/platform/qcom/iris/Makefile          |   6 +-
>>   .../media/platform/qcom/iris/iris_catalog_gen1.c   |  83 +++++++
>>   ...{iris_platform_sm8550.c => iris_catalog_gen2.c} |  85 +------
>>   ...ris_platform_sm8250.c => iris_catalog_sm8250.h} |  80 +-----
>>   .../media/platform/qcom/iris/iris_catalog_sm8550.h |  91 +++++++
>>   .../media/platform/qcom/iris/iris_catalog_sm8650.h |  68 +++++
>>   drivers/media/platform/qcom/iris/iris_core.h       |   1 +
>>   .../platform/qcom/iris/iris_platform_common.h      |   3 +
>>   drivers/media/platform/qcom/iris/iris_probe.c      |  43 +++-
>>   drivers/media/platform/qcom/iris/iris_vpu2.c       |   1 +
>>   drivers/media/platform/qcom/iris/iris_vpu3.c       | 122 ---------
>>   drivers/media/platform/qcom/iris/iris_vpu3x.c      | 275 +++++++++++++++++++++
>>   drivers/media/platform/qcom/iris/iris_vpu_common.c |   4 +-
>>   drivers/media/platform/qcom/iris/iris_vpu_common.h |   3 +
>>   15 files changed, 598 insertions(+), 300 deletions(-)
>> ---
>> base-commit: 2bdde620f7f2bff2ff1cb7dc166859eaa0c78a7c
>> change-id: 20250410-topic-sm8x50-upstream-iris-catalog-3e2e4a033d6f
>>
>> Best regards,
>