[PATCH v3 00/38] drm/msm/dpu: rework HW catalog
Abhinav Kumar
quic_abhinavk at quicinc.com
Mon Apr 3 18:06:17 UTC 2023
On 3/30/2023 2:52 PM, Dmitry Baryshkov wrote:
> This huge series attempts to restructure the DPU HW catalog into a
> manageable and reviewable data set. In order to ease review and testing
> I merged all the necessary fixes into this series. Also I cherry-picked
> & slightly fixed Konrad's patch adding size to the SSPP and INTF macros.
>
I had to first dig up some history about why dpu catalog grew so much in
the first place before starting this review. When the DPU driver first
landed (which pre-dates my work in upstream), it looks like it followed
mdp5 model from mdp5_cfg.c. But looks like as the number of chipsets
which use DPU kept growing, this is becoming a burden.
As everyone knows, downstream follows a devicetree model for the dpu
hardware and that should have always been the case. Perhaps in the last
2-3 years more time could have been spent on standardizing the bindings
used for hw blocks in order to maintain a less hard-coded catalog file
and more in the device tree.
Then the catalog would have just been a place to parse the device tree,
set the feature capability based on chipset (refer
_sde_hardware_pre_caps). That way offsets , number of blocks and the
blocks themselves still come from the device tree but perhaps some
specific features are at SOC level for which the catalog still stays.
That being said, I thought of different strategies even before the
review but two issues prevented me from suggesting those ideas (one of
which I am seeing even here , which I am going to suggest below and also
suggest why it wont work).
1) For the same DPU major/minor version, some features might get dropped
or even get added with different SOCs as overall the system capabilities
might differ like number of SSPPs or memory footprint of the SOC etc.
So there is no good way right now to generalize any dpu catalog or to
tie it with a DPU major/minor version. We will have to stick with a
per-SOC model.
This is what led me to not pursue that route.
2) For the same DPU major/minor version, even if core-DPU is same (in
terms of SSPP, DSPP etc), the number of interfaces can change. So again
no room to generalize same DPU hw version.
3) For the same reason as (1) and (2), I think the de-duplication
strategy used in this series is not correct. The idea of
dpu_hw_version_num_layer_mixer is just not scalable as I dont know how
many variants that will lead to. So it seems like just an attempt to
de-duplicate which perhaps works today for existing dpu chipsets in
upstream but by no means scalable. Lets go ahead with per-SOC catalog
file but lets live with some amount of duplication between them if we
really have to split it across header files.
I also thought of similar strategies to generalize like based on
sub-blocks similar to what you have done but all of these were NAKed
internally by folks who work on more chipsets / have more visibility
into the spread of features across chipsets.
> First 4 patches clean up the catalog a bit in order to make it more
> suitable for refactoring.
>
These are okay. I will address your follow-up questions about patch (1)
and lets land these.
> Then the next batch of 13 + 5 patches split the hw catalog entries into
> per-SoC files.
>
This part is also fine. But perhaps dont have dpu hw version in the
file. So just dpu_hw_sm8250.h or dpu_hw_sm8350.h etc.
> Next 9 patches rework catalog entries, mostly targeting deduplication of
> data used by several platforms. At this moment only three pairs (out of
> 13 devices supported by DPU) are merged. However this part lays out the
> ground to ease adding support for new platforms, some of which use the
> same configuration as the existing platforms
>
This is the part I suggest we drop.
> Last batch of 7 patches renames existing macros to ease using them while
> adding support for new devices.
>
I have to check this part but perhaps after re-basing based on my
earlier comment.
> This pile of patches is submitted in a single batch to allow one to
> observe the final goal of the cleanup which otherwise might be hard to
> assess.
>
>
> Changes since v2:
> - Fixed sc8280xp SSPP size to 0x2ac
> - Rebased on top of msm-next-lumag, dropped merged patches
>
> Changes since v1:
> - Picked up Konrad's patch
> - Picked up dependencies into the main series
> - Moved qseed3lite vs qseed4 patches into the fixes part
> - Fixed sm6115 in a similar manner.
>
> Dmitry Baryshkov (37):
> drm/msm/dpu: constify DSC data structures
> drm/msm/dpu: mark remaining pp data as const
> drm/msm/dpu: move UBWC/memory configuration to separate struct
> drm/msm/dpu: split SM8550 catalog entry to the separate file
> drm/msm/dpu: split SM8450 catalog entry to the separate file
> drm/msm/dpu: split SC8280XP catalog entry to the separate file
> drm/msm/dpu: split SC7280 catalog entry to the separate file
> drm/msm/dpu: split SM8350 catalog entry to the separate file
> drm/msm/dpu: split SM6115 catalog entry to the separate file
> drm/msm/dpu: split QCM2290 catalog entry to the separate file
> drm/msm/dpu: split SC7180 catalog entry to the separate file
> drm/msm/dpu: split SM8250 catalog entry to the separate file
> drm/msm/dpu: split SC8180X catalog entry to the separate file
> drm/msm/dpu: split SM8150 catalog entry to the separate file
> drm/msm/dpu: split MSM8998 catalog entry to the separate file
> drm/msm/dpu: split SDM845 catalog entry to the separate file
> drm/msm/dpu: duplicate sdm845 catalog entries
> drm/msm/dpu: duplicate sc7180 catalog entries
> drm/msm/dpu: duplicate sm8150 catalog entries
> drm/msm/dpu: duplicate sm8250 catalog entries
> drm/msm/dpu: duplicate sm8350 catalog entries
> drm/msm/dpu: use defined symbol for sc8280xp's maxwidth
> drm/msm/dpu: catalog: add comments regarding DPU_CTL_SPLIT_DISPLAY
> drm/msm/dpu: enable DPU_CTL_SPLIT_DISPLAY for sc8280xp
> drm/msm/dpu: enable DSPP_2/3 for LM_2/3 on sm8450
> drm/msm/dpu: drop duplicate vig_sblk instances
> drm/msm/dpu: enable DSPP on sc8180x
> drm/msm/dpu: deduplicate sc8180x with sm8150
> drm/msm/dpu: deduplicate sm6115 with qcm2290
> drm/msm/dpu: deduplicate sc8280xp with sm8450
> drm/msm/dpu: drop unused macros from hw catalog
> drm/msm/dpu: inline IRQ_n_MASK defines
> drm/msm/dpu: rename INTF_foo_MASK to contain major DPU version
> drm/msm/dpu: rename CTL_foo_MASK to contain major DPU version
> drm/msm/dpu: rename VIG and DMA_foo_MASK to contain major DPU version
> drm/msm/dpu: rename MIXER_foo_MASK to contain major DPU version
> drm/msm/dpu: rename MERGE_3D_foo_MASK to contain major DPU version
>
> Konrad Dybcio (1):
> drm/msm/dpu: Allow variable SSPP/INTF_BLK size
>
> .../msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 210 ++
> .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 210 ++
> .../msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 97 +
> .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 91 +
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_lm6.h | 152 ++
> .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 244 ++
> .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h | 151 ++
> .../msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 91 +
> .../msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 83 +
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_lm1.h | 53 +
> .../msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 226 ++
> .../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 158 ++
> .../msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 136 ++
> .../msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 142 ++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_lm6.h | 99 +
> .../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 209 ++
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2175 +----------------
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 37 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 4 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 18 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 4 +-
> 21 files changed, 2443 insertions(+), 2147 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_lm6.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_lm1.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_lm6.h
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
>
More information about the dri-devel
mailing list