lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <fqozl3yxbq4lxdhdadsuhdbibj4qudungmgu6za37qfkwq7yti@lhfm4gz6r4cs>
Date: Tue, 6 May 2025 15:46:45 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Jessica Zhang <quic_jesszhan@...cinc.com>
Cc: Rob Clark <robdclark@...il.com>, Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Sean Paul <sean@...rly.run>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Vinod Koul <vkoul@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>,
        linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 00/33] drm/msm/dpu: rework HW block feature handling

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@....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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ