On Fri, 12 Apr 2024 at 00:20, Abhinav Kumar <quic_abhin...@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.barysh...@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
