[PATCH 10/12] drm/msm: convert msm_format::alpha_enable to the flag

[Dmitry Baryshkov]

On Fri, 12 Apr 2024 at 00:20, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>
>
>
> On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:
> > Instead of having a bool field alpha_enable, convert it to the
> > flag, this save space in the tables and allows us to handle all booleans
> > in the same way.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 12 ++++++-----
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 24 ++++++++++-----------
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c |  7 +++---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c |  3 ++-
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c   |  4 ++--
> >   drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c   |  2 +-
> >   drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c  |  3 ++-
> >   drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c   |  9 ++++----
> >   drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c  |  3 ++-
> >   drivers/gpu/drm/msm/disp/mdp_format.c       |  2 +-
> >   drivers/gpu/drm/msm/msm_drv.h               |  4 ++--
> >   11 files changed, 40 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 9041b0d71b25..201010038660 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -342,7 +342,7 @@ static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer,
> >
> >       /* default to opaque blending */
> >       if (pstate->base.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE ||
> > -         !format->alpha_enable) {
> > +         !(format->flags & MSM_FORMAT_FLAG_ALPHA_ENABLE)) {
> >               blend_op = DPU_BLEND_FG_ALPHA_FG_CONST |
> >                       DPU_BLEND_BG_ALPHA_BG_CONST;
> >       } else if (pstate->base.pixel_blend_mode == DRM_MODE_BLEND_PREMULTI) {
> > @@ -373,8 +373,8 @@ static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer,
> >       lm->ops.setup_blend_config(lm, pstate->stage,
> >                               fg_alpha, bg_alpha, blend_op);
> >
> > -     DRM_DEBUG_ATOMIC("format:%p4cc, alpha_en:%u blend_op:0x%x\n",
> > -               &format->pixel_format, format->alpha_enable, blend_op);
> > +     DRM_DEBUG_ATOMIC("format:%p4cc, alpha_en:%lu blend_op:0x%x\n",
> > +               &format->pixel_format, format->flags & MSM_FORMAT_FLAG_ALPHA_ENABLE, blend_op);
> >   }
> >
> >   static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
> > @@ -472,7 +472,8 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> >
> >               format = msm_framebuffer_format(pstate->base.fb);
> >
> > -             if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
> > +             if (pstate->stage == DPU_STAGE_BASE &&
> > +                 format->flags & MSM_FORMAT_FLAG_ALPHA_ENABLE)
> >                       bg_alpha_enable = true;
> >
> >               set_bit(pstate->pipe.sspp->idx, fetch_active);
> > @@ -495,7 +496,8 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> >               for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
> >                       _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, format);
> >
> > -                     if (bg_alpha_enable && !format->alpha_enable)
> > +                     if (bg_alpha_enable &&
> > +                         !(format->flags & MSM_FORMAT_FLAG_ALPHA_ENABLE))
> >                               mixer[lm_idx].mixer_op_mode = 0;
> >                       else
> >                               mixer[lm_idx].mixer_op_mode |=
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > index baf0fd67bf42..de9e93cb42c4 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > @@ -36,7 +36,6 @@ bp, flg, fm, np)                                                          \
> >   {                                                                         \
> >       .pixel_format = DRM_FORMAT_ ## fmt,                               \
> >       .fetch_type = MDP_PLANE_INTERLEAVED,                              \
> > -     .alpha_enable = alpha,                                            \
> >       .element = { (e0), (e1), (e2), (e3) },                            \
> >       .bpc_g_y = g,                                                     \
> >       .bpc_b_cb = b,                                                    \
> > @@ -46,7 +45,9 @@ bp, flg, fm, np)                                                          \
> >       .unpack_count = uc,                                               \
> >       .bpp = bp,                                                        \
> >       .fetch_mode = fm,                                                 \
> > -     .flags = MSM_FORMAT_FLAG_UNPACK_TIGHT | flg,                      \
> > +     .flags = MSM_FORMAT_FLAG_UNPACK_TIGHT |                           \
> > +             (alpha ? MSM_FORMAT_FLAG_ALPHA_ENABLE : 0) |              \
> > +             flg,                                                      \
>
> In the previous two patches where the same thing was done for
> unpack_tight and unpack_align_msb, it was different in the sense that
> just on the basis of which macro we were choosing we knew the value of
> those flags so you could just unconditionally OR those flags.
>
> But for alpha, you are performing a conditional before ORing this so I
> think for this leaving it as a bool is cleaner.

Ack

-- 
With best wishes
Dmitry