On Mon, May 05, 2025 at 04:28:17PM -0700, Jessica Zhang wrote: > > > On 4/24/2025 2:30 AM, Dmitry Baryshkov wrote: > > Some time ago we started the process of converting HW blocks to use > > revision-based checks instead of having feature bits (which are easy to > > miss or to set incorrectly). Then the process of such a conversion was > > postponed. (Mostly) finish the conversion. The only blocks which still > > have feature bits are SSPP, WB and VBIF. In the rare cases where > > behaviour actually differs from platform to platform (or from block to > > block) use unsigned long bitfields, they have simpler syntax to be > > checked and don't involve test_bit() invocation. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com> > > Hi Dmitry, > > I agree that some features like *_HAS_LAYER_EXT4 and INTF_INPUT_CTRL can be > replaced with a general version check.
Great > However, for other features (such as DPU_MIXER_SOURCESPLIT --> > dpu_lm_cfg::sourcesplit), it seems to me, you've just replaced the feature > bit with an equivalent catalog field. Let me drop it too then :-) > So while some features can be dropped from the feature flag list, it seems > that we would still need feature flags (in some form) for others. Do you have any other examples? > Overall though, I think that dropping the feature bits makes it less clear > exactly what features are supported for which chipsets and makes it harder > to relegate features to specific chipsets. > > So while I do see where you're coming from here, I'm a bit hesitant of the > overall move to drop feature flags for the reasons above. The problem is that we currently have two ways to define features - via the MDSS version and via feature bits. I'd like to get rid of that duplication, especially for those features that are really tied to the MDSS / DPU generation. As you can see, after the refactoring we had only three feature bits which require special handling: DPU_MIXER_SOURCESPLIT, DPU_DSC_NATIVE_42x_EN and DPU_CTL_SPLIT_DISPLAY. With the Active CTL being merged, I can easily rework the DPU_CTL_SPLIT_DISPLAY. I think DPU_MIXER_SOURCESPLIT can also easily go away. I think we are left with only one feature bit - DPU_DSC_NATIVE_42x_EN. I think I can live with that. > > Thanks, > > Jessica Zhang > > > --- > > Changes in v3: > > - Repost, fixing email/author issues caused by b4 / mailmap interaction > > - Link to v2: > > https://lore.kernel.org/r/20250424-dpu-drop-features-v2-0-0a9a66a7b...@oss.qualcomm.com > > > > Changes in v2: > > - Rebased on top of the current msm-next > > - Link to v1: > > https://lore.kernel.org/r/20241214-dpu-drop-features-v1-0-988f0662c...@linaro.org > > > > --- > > Dmitry Baryshkov (33): > > drm/msm/dpu: stop passing mdss_ver to setup_timing_gen() > > drm/msm/dpu: drop INTF_SC7280_MASK > > drm/msm/dpu: inline _setup_ctl_ops() > > drm/msm/dpu: inline _setup_dsc_ops() > > drm/msm/dpu: inline _setup_dspp_ops() > > drm/msm/dpu: inline _setup_mixer_ops() > > drm/msm/dpu: remove DSPP_SC7180_MASK > > drm/msm/dpu: get rid of DPU_CTL_HAS_LAYER_EXT4 > > drm/msm/dpu: get rid of DPU_CTL_ACTIVE_CFG > > drm/msm/dpu: get rid of DPU_CTL_FETCH_ACTIVE > > drm/msm/dpu: get rid of DPU_CTL_DSPP_SUB_BLOCK_FLUSH > > drm/msm/dpu: get rid of DPU_CTL_VM_CFG > > drm/msm/dpu: get rid of DPU_DATA_HCTL_EN > > drm/msm/dpu: get rid of DPU_INTF_STATUS_SUPPORTED > > drm/msm/dpu: get rid of DPU_INTF_INPUT_CTRL > > drm/msm/dpu: get rid of DPU_PINGPONG_DSC > > drm/msm/dpu: get rid of DPU_PINGPONG_DITHER > > drm/msm/dpu: get rid of DPU_MDP_VSYNC_SEL > > drm/msm/dpu: get rid of DPU_MDP_PERIPH_0_REMOVED > > drm/msm/dpu: get rid of DPU_MDP_AUDIO_SELECT > > drm/msm/dpu: get rid of DPU_MIXER_COMBINED_ALPHA > > drm/msm/dpu: get rid of DPU_DIM_LAYER > > drm/msm/dpu: get rid of DPU_DSC_HW_REV_1_2 > > drm/msm/dpu: get rid of DPU_DSC_OUTPUT_CTRL > > drm/msm/dpu: get rid of DPU_WB_INPUT_CTRL > > drm/msm/dpu: get rid of DPU_SSPP_QOS_8LVL > > drm/msm/dpu: drop unused MDP TOP features > > drm/msm/dpu: drop ununused PINGPONG features > > drm/msm/dpu: drop ununused MIXER features > > drm/msm/dpu: get rid of DPU_MIXER_SOURCESPLIT > > drm/msm/dpu: get rid of DPU_DSC_NATIVE_42x_EN > > drm/msm/dpu: get rid of DPU_CTL_SPLIT_DISPLAY > > drm/msm/dpu: move features out of the DPU_HW_BLK_INFO > > > > .../drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h | 53 +++----- > > .../drm/msm/disp/dpu1/catalog/dpu_1_14_msm8937.h | 4 - > > .../drm/msm/disp/dpu1/catalog/dpu_1_15_msm8917.h | 3 - > > .../drm/msm/disp/dpu1/catalog/dpu_1_16_msm8953.h | 4 - > > .../drm/msm/disp/dpu1/catalog/dpu_1_7_msm8996.h | 15 +-- > > .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 19 +-- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_3_2_sdm660.h | 19 +-- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_3_3_sdm630.h | 12 +- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 21 +--- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_1_sdm670.h | 11 +- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 43 ++----- > > .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 45 ++----- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_2_sm7150.h | 31 ++--- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h | 19 +-- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 16 +-- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 42 ++----- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h | 14 +-- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 5 - > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h | 16 +-- > > .../drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 5 - > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h | 6 - > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 44 ++----- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 22 +--- > > .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 50 ++------ > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 47 ++------ > > .../drm/msm/disp/dpu1/catalog/dpu_8_4_sa8775p.h | 53 ++------ > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 47 ++------ > > .../drm/msm/disp/dpu1/catalog/dpu_9_2_x1e80100.h | 52 ++------ > > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 +- > > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 3 +- > > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 51 +------- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 134 > > ++------------------- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 108 ++++++++--------- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 4 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 21 ++-- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 3 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 7 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 10 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 14 +-- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 5 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 28 ++--- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 3 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c | 5 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 4 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 5 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 11 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 2 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 17 ++- > > 51 files changed, 304 insertions(+), 864 deletions(-) > > --- > > base-commit: a4efc119e8149503e5fe9e9f7828b79af2fe77c6 > > change-id: 20241213-dpu-drop-features-7603dc3ee189 > > > > Best regards, > -- With best wishes Dmitry
