Re: [PATCH 07/12] drm/msm: merge dpu_format and mdp_format in struct msm_format
On Thu, 11 Apr 2024 at 22:15, Abhinav Kumar wrote:
>
>
>
> On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:
> > Structures dpu_format and mdp_format are largely the same structures.
> > In order to remove duplication between format databases, merge these two
> > stucture definitions into the global struct msm_format.
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 +-
> > .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +-
> > .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 184 ++
> > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 10 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 41 +---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 30 +--
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 6 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 14 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 16 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 74 +++
> > drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 4 +-
> > drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c| 26 +--
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 7 +-
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 54 ++---
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 4 +-
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h | 2 +-
> > drivers/gpu/drm/msm/disp/mdp_format.c | 28 ++-
> > drivers/gpu/drm/msm/disp/mdp_kms.h| 13 --
> > drivers/gpu/drm/msm/msm_drv.h | 28 +++
> > 24 files changed, 279 insertions(+), 288 deletions(-)
> >
>
>
>
> > int mdp5_smp_assign(struct mdp5_smp *smp, struct mdp5_smp_state *state,
> > diff --git a/drivers/gpu/drm/msm/disp/mdp_format.c
> > b/drivers/gpu/drm/msm/disp/mdp_format.c
> > index 30919641c813..5fc55f41e74f 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp_format.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp_format.c
> > @@ -63,26 +63,24 @@ static struct csc_cfg csc_convert[CSC_MAX] = {
> > };
> >
> > #define FMT(name, a, r, g, b, e0, e1, e2, e3, alpha, tight, c, cnt, fp,
> > cs, yuv) { \
> > - .base = {\
> > - .pixel_format = DRM_FORMAT_ ## name, \
> > - .flags = yuv ? MSM_FORMAT_FLAG_YUV : 0, \
> > - }, \
> > + .pixel_format = DRM_FORMAT_ ## name, \
> > .bpc_a = BPC ## a ## A, \
> > - .bpc_r = BPC ## r, \
> > - .bpc_g = BPC ## g, \
> > - .bpc_b = BPC ## b, \
> > - .unpack = { e0, e1, e2, e3 },\
> > + .bpc_r_cr = BPC ## r,\
> > + .bpc_g_y = BPC ## g, \
> > + .bpc_b_cb = BPC ## b,\
> > + .element = { e0, e1, e2, e3 }, \
> > + .fetch_type = fp,\
> > + .chroma_sample = cs, \
> > .alpha_enable = alpha, \
> > .unpack_tight = tight, \
> > - .cpp = c,\
> > .unpack_count = cnt, \
> > - .fetch_type = fp,\
> > - .chroma_sample = cs, \
>
> Minor nit:
>
> These two lines are only moving the locations of assignment so
> unnecessary change?
Sure, let's drop that. I think it was just C of some kind.
>
> Rest LGTM,
>
> Reviewed-by: Abhinav Kumar
>
> For validation, are you relying mostly on the CI here OR also other
> internal farms? Even though mostly its just making code common, basic
> display coming up on one target each of MDP4/MDP5/DPU will be great to
> be safe.
It was a visual inspection, but not for each and every platform.
--
With best wishes
Dmitry
Re: [PATCH 07/12] drm/msm: merge dpu_format and mdp_format in struct msm_format
On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:
Structures dpu_format and mdp_format are largely the same structures.
In order to remove duplication between format databases, merge these two
stucture definitions into the global struct msm_format.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 184 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 10 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 41 +---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 30 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 6 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 14 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 16 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 74 +++
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 4 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c| 26 +--
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 7 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 54 ++---
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 4 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h | 2 +-
drivers/gpu/drm/msm/disp/mdp_format.c | 28 ++-
drivers/gpu/drm/msm/disp/mdp_kms.h| 13 --
drivers/gpu/drm/msm/msm_drv.h | 28 +++
24 files changed, 279 insertions(+), 288 deletions(-)
int mdp5_smp_assign(struct mdp5_smp *smp, struct mdp5_smp_state *state,
diff --git a/drivers/gpu/drm/msm/disp/mdp_format.c
b/drivers/gpu/drm/msm/disp/mdp_format.c
index 30919641c813..5fc55f41e74f 100644
--- a/drivers/gpu/drm/msm/disp/mdp_format.c
+++ b/drivers/gpu/drm/msm/disp/mdp_format.c
@@ -63,26 +63,24 @@ static struct csc_cfg csc_convert[CSC_MAX] = {
};
#define FMT(name, a, r, g, b, e0, e1, e2, e3, alpha, tight, c, cnt, fp, cs, yuv) { \
- .base = {\
- .pixel_format = DRM_FORMAT_ ## name, \
- .flags = yuv ? MSM_FORMAT_FLAG_YUV : 0, \
- }, \
+ .pixel_format = DRM_FORMAT_ ## name, \
.bpc_a = BPC ## a ## A, \
- .bpc_r = BPC ## r, \
- .bpc_g = BPC ## g, \
- .bpc_b = BPC ## b, \
- .unpack = { e0, e1, e2, e3 },\
+ .bpc_r_cr = BPC ## r,\
+ .bpc_g_y = BPC ## g, \
+ .bpc_b_cb = BPC ## b,\
+ .element = { e0, e1, e2, e3 }, \
+ .fetch_type = fp,\
+ .chroma_sample = cs, \
.alpha_enable = alpha, \
.unpack_tight = tight, \
- .cpp = c,\
.unpack_count = cnt, \
- .fetch_type = fp,\
- .chroma_sample = cs, \
Minor nit:
These two lines are only moving the locations of assignment so
unnecessary change?
Rest LGTM,
Reviewed-by: Abhinav Kumar
For validation, are you relying mostly on the CI here OR also other
internal farms? Even though mostly its just making code common, basic
display coming up on one target each of MDP4/MDP5/DPU will be great to
be safe.
[Freedreno] [PATCH 07/12] drm/msm: merge dpu_format and mdp_format in struct msm_format
Structures dpu_format and mdp_format are largely the same structures.
In order to remove duplication between format databases, merge these two
stucture definitions into the global struct msm_format.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 184 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 10 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 41 +---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 30 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 6 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 14 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 16 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 74 +++
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 4 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c| 26 +--
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 7 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 54 ++---
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 4 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h | 2 +-
drivers/gpu/drm/msm/disp/mdp_format.c | 28 ++-
drivers/gpu/drm/msm/disp/mdp_kms.h| 13 --
drivers/gpu/drm/msm/msm_drv.h | 28 +++
24 files changed, 279 insertions(+), 288 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 3c475f8042b0..9041b0d71b25 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -331,7 +331,7 @@ static bool dpu_crtc_get_scanout_position(struct drm_crtc
*crtc,
}
static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer,
- struct dpu_plane_state *pstate, struct dpu_format *format)
+ struct dpu_plane_state *pstate, const struct msm_format *format)
{
struct dpu_hw_mixer *lm = mixer->hw_lm;
uint32_t blend_op;
@@ -374,7 +374,7 @@ static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer
*mixer,
fg_alpha, bg_alpha, blend_op);
DRM_DEBUG_ATOMIC("format:%p4cc, alpha_en:%u blend_op:0x%x\n",
- >base.pixel_format, format->alpha_enable, blend_op);
+ >pixel_format, format->alpha_enable, blend_op);
}
static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
@@ -406,7 +406,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc
*crtc,
struct dpu_crtc_mixer *mixer,
u32 num_mixers,
enum dpu_stage stage,
- struct dpu_format *format,
+ const struct msm_format *format,
uint64_t modifier,
struct dpu_sw_pipe *pipe,
unsigned int stage_idx,
@@ -423,7 +423,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc
*crtc,
trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
state, to_dpu_plane_state(state), stage_idx,
- format->base.pixel_format,
+ format->pixel_format,
modifier);
DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d
multirect_idx %d\n",
@@ -451,7 +451,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
struct drm_plane_state *state;
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
struct dpu_plane_state *pstate = NULL;
- struct dpu_format *format;
+ const struct msm_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
uint32_t lm_idx;
@@ -470,7 +470,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
- format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
+ format = msm_framebuffer_format(pstate->base.fb);
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index a01fda711883..35aaead897e1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -232,7 +232,7 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
{
