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.baryshkov@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-0a9a66a7b3a2@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-988f0662cb7e@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