[PATCH v2 9/9] drm/msm: drop msm_kms_funcs::get_format() callback
Now as all subdrivers were converted to use common database of formats,
drop the get_format() callback and use mdp_get_format() directly.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 5 ++---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c| 2 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 1 -
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 1 -
drivers/gpu/drm/msm/msm_fb.c | 2 +-
drivers/gpu/drm/msm/msm_kms.h| 4
8 files changed, 6 insertions(+), 14 deletions(-)
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 b966c44ec835..ef69c2f408c3 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
@@ -274,7 +274,7 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
drm_mode_to_intf_timing_params(phys_enc, , _params);
- fmt =
phys_enc->dpu_kms->base.funcs->get_format(_enc->dpu_kms->base, fmt_fourcc,
0);
+ fmt = mdp_get_format(_enc->dpu_kms->base, fmt_fourcc, 0);
DPU_DEBUG_VIDENC(phys_enc, "fmt_fourcc 0x%X\n", fmt_fourcc);
if (phys_enc->hw_cdm)
@@ -414,7 +414,7 @@ static void dpu_encoder_phys_vid_enable(struct
dpu_encoder_phys *phys_enc)
ctl = phys_enc->hw_ctl;
fmt_fourcc = dpu_encoder_get_drm_fmt(phys_enc);
- fmt =
phys_enc->dpu_kms->base.funcs->get_format(_enc->dpu_kms->base, fmt_fourcc,
0);
+ fmt = mdp_get_format(_enc->dpu_kms->base, fmt_fourcc, 0);
DPU_DEBUG_VIDENC(phys_enc, "\n");
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index de17bcbb8492..d3ea91c1d7d2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -326,8 +326,7 @@ static void dpu_encoder_phys_wb_setup(
wb_job = wb_enc->wb_job;
format = msm_framebuffer_format(wb_enc->wb_job->fb);
- dpu_fmt =
phys_enc->dpu_kms->base.funcs->get_format(_enc->dpu_kms->base,
-
format->pixel_format, wb_job->fb->modifier);
+ dpu_fmt = mdp_get_format(_enc->dpu_kms->base,
format->pixel_format, wb_job->fb->modifier);
DPU_DEBUG("[mode_set:%d, \"%s\",%d,%d]\n",
hw_wb->idx - WB_0, mode.name,
@@ -577,7 +576,7 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
dpu_encoder_phys *phys_enc
format = msm_framebuffer_format(job->fb);
- wb_cfg->dest.format =
phys_enc->dpu_kms->base.funcs->get_format(_enc->dpu_kms->base,
+ wb_cfg->dest.format = mdp_get_format(_enc->dpu_kms->base,
format->pixel_format,
job->fb->modifier);
if (!wb_cfg->dest.format) {
/* this error should be detected during atomic_check */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index cb30137443e8..1955848b1b78 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -982,7 +982,6 @@ static const struct msm_kms_funcs kms_funcs = {
.enable_vblank = dpu_kms_enable_vblank,
.disable_vblank = dpu_kms_disable_vblank,
.check_modified_format = dpu_format_check_modified_format,
- .get_format = mdp_get_format,
.destroy = dpu_kms_destroy,
.snapshot= dpu_kms_mdp_snapshot,
#ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index b92a13cc9b36..1c3a2657450c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -627,7 +627,7 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
* select fill format to match user property expectation,
* h/w only supports RGB variants
*/
- fmt = priv->kms->funcs->get_format(priv->kms, DRM_FORMAT_ABGR, 0);
+ fmt = mdp_get_format(priv->kms, DRM_FORMAT_ABGR, 0);
/* should not happen ever */
if (!fmt)
return;
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 4ba1cb74ad76..6e4e74f9d63d 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -151,7 +151,6 @@ static const struct mdp_kms_funcs kms_funcs = {
.flush_commit= mdp4_flush_commit,
.wait_flush = mdp4_wait_flush,
.complete_commit = mdp4_complete_commit,
- .get_format = mdp_get_format,
.round_pixclk=
[PATCH v2 5/9] 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.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 12 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 4 +-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 4 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 184 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 8 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 4 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 4 +-
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 | 18 +-
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_format.h | 28
drivers/gpu/drm/msm/disp/mdp_kms.h | 13 --
28 files changed, 296 insertions(+), 305 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 88c2e51ab166..9f2164782844 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -320,7 +320,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;
@@ -363,7 +363,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)
@@ -395,7 +395,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,
@@ -412,7 +412,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",
@@ -440,7 +440,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;
@@ -459,7 +459,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)
[PATCH v2 8/9] drm/msm: merge dpu format database to MDP formats
Finally remove duplication between DPU and generic MDP code by merging
DPU format lists to the MDP format database.
Signed-off-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 4 +-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 7 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 602
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 23 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 10 -
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 3 +-
drivers/gpu/drm/msm/disp/mdp_format.c | 614 ++---
drivers/gpu/drm/msm/disp/mdp_format.h | 10 +
drivers/gpu/drm/msm/disp/mdp_kms.h | 2 -
drivers/gpu/drm/msm/msm_drv.h | 2 +
11 files changed, 571 insertions(+), 708 deletions(-)
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 deb2f6b446d3..b966c44ec835 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
@@ -274,7 +274,7 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
drm_mode_to_intf_timing_params(phys_enc, , _params);
- fmt = dpu_get_dpu_format(fmt_fourcc);
+ fmt =
phys_enc->dpu_kms->base.funcs->get_format(_enc->dpu_kms->base, fmt_fourcc,
0);
DPU_DEBUG_VIDENC(phys_enc, "fmt_fourcc 0x%X\n", fmt_fourcc);
if (phys_enc->hw_cdm)
@@ -414,7 +414,7 @@ static void dpu_encoder_phys_vid_enable(struct
dpu_encoder_phys *phys_enc)
ctl = phys_enc->hw_ctl;
fmt_fourcc = dpu_encoder_get_drm_fmt(phys_enc);
- fmt = dpu_get_dpu_format(fmt_fourcc);
+ fmt =
phys_enc->dpu_kms->base.funcs->get_format(_enc->dpu_kms->base, fmt_fourcc,
0);
DPU_DEBUG_VIDENC(phys_enc, "\n");
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 8b5a4a1c239e..de17bcbb8492 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -326,7 +326,8 @@ static void dpu_encoder_phys_wb_setup(
wb_job = wb_enc->wb_job;
format = msm_framebuffer_format(wb_enc->wb_job->fb);
- dpu_fmt = dpu_get_dpu_format_ext(format->pixel_format,
wb_job->fb->modifier);
+ dpu_fmt =
phys_enc->dpu_kms->base.funcs->get_format(_enc->dpu_kms->base,
+
format->pixel_format, wb_job->fb->modifier);
DPU_DEBUG("[mode_set:%d, \"%s\",%d,%d]\n",
hw_wb->idx - WB_0, mode.name,
@@ -576,8 +577,8 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
dpu_encoder_phys *phys_enc
format = msm_framebuffer_format(job->fb);
- wb_cfg->dest.format = dpu_get_dpu_format_ext(
- format->pixel_format, job->fb->modifier);
+ wb_cfg->dest.format =
phys_enc->dpu_kms->base.funcs->get_format(_enc->dpu_kms->base,
+format->pixel_format,
job->fb->modifier);
if (!wb_cfg->dest.format) {
/* this error should be detected during atomic_check */
DPU_ERROR("failed to get format %p4cc\n",
>pixel_format);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 2bb1584920c6..6b1e9a617da3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -11,186 +11,11 @@
#include "dpu_kms.h"
#include "dpu_formats.h"
-#define DPU_UBWC_META_MACRO_W_H16
-#define DPU_UBWC_META_BLOCK_SIZE 256
#define DPU_UBWC_PLANE_SIZE_ALIGNMENT 4096
-#define DPU_TILE_HEIGHT_DEFAULT1
-#define DPU_TILE_HEIGHT_TILED 4
-#define DPU_TILE_HEIGHT_UBWC 4
-#define DPU_TILE_HEIGHT_NV12 8
-
#define DPU_MAX_IMG_WIDTH 0x3FFF
#define DPU_MAX_IMG_HEIGHT 0x3FFF
-/*
- * DPU supported format packing, bpp, and other format
- * information.
- * DPU currently only supports interleaved RGB formats
- * UBWC support for a pixel format is indicated by the flag,
- * there is additional meta data plane for such formats
- */
-
-#define INTERLEAVED_RGB_FMT(fmt, a, r, g, b, e0, e1, e2, e3, uc, alpha, \
-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,
[PATCH v2 7/9] drm/msm: convert msm_format::unpack_align_msb to the flag
Instead of having a u8 or bool field unpack_align_msb, convert it to the flag, this save space in the tables and allows us to handle all booleans in the same way. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 12 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 2 +- drivers/gpu/drm/msm/disp/mdp_format.h | 4 ++-- 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 705b91582b0f..2bb1584920c6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -43,7 +43,6 @@ bp, flg, fm, np) \ .bpc_r_cr = r,\ .bpc_a = a, \ .chroma_sample = CHROMA_FULL, \ - .unpack_align_msb = 0,\ .unpack_count = uc, \ .bpp = bp,\ .fetch_mode = fm, \ @@ -64,7 +63,6 @@ alpha, bp, flg, fm, np, th) \ .bpc_r_cr = r,\ .bpc_a = a, \ .chroma_sample = CHROMA_FULL, \ - .unpack_align_msb = 0,\ .unpack_count = uc, \ .bpp = bp,\ .fetch_mode = fm, \ @@ -86,7 +84,6 @@ alpha, chroma, count, bp, flg, fm, np) \ .bpc_r_cr = r,\ .bpc_a = a, \ .chroma_sample = chroma, \ - .unpack_align_msb = 0,\ .unpack_count = count,\ .bpp = bp,\ .fetch_mode = fm, \ @@ -106,7 +103,6 @@ alpha, chroma, count, bp, flg, fm, np) \ .bpc_r_cr = r,\ .bpc_a = a, \ .chroma_sample = chroma, \ - .unpack_align_msb = 0,\ .unpack_count = 2,\ .bpp = 2, \ .fetch_mode = fm, \ @@ -127,7 +123,6 @@ flg, fm, np, th) \ .bpc_r_cr = r,\ .bpc_a = a, \ .chroma_sample = chroma, \ - .unpack_align_msb = 0,\ .unpack_count = 2,\ .bpp = 2, \ .fetch_mode = fm, \ @@ -147,11 +142,10 @@ flg, fm, np, th) \ .bpc_r_cr = r,\ .bpc_a = a, \ .chroma_sample = chroma, \ - .unpack_align_msb = 1,\ .unpack_count = 2,\ .bpp = 2, \ .fetch_mode = fm, \ - .flags = flg, \ + .flags = MSM_FORMAT_FLAG_UNPACK_ALIGN_MSB | flg, \ .num_planes = np, \ .tile_height = DPU_TILE_HEIGHT_DEFAULT\ } @@ -168,11 +162,10 @@ flg, fm, np, th) \ .bpc_r_cr = r,
[PATCH v2 6/9] drm/msm: convert msm_format::unpack_tight to the flag
Instead of having a u8 or bool field unpack_tight, convert it to the flag, this save space in the tables and allows us to handle all booleans in the same way. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 22 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 2 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 3 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +- drivers/gpu/drm/msm/disp/mdp_format.c | 52 ++--- drivers/gpu/drm/msm/disp/mdp_format.h | 4 +-- 7 files changed, 41 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 855f0d29c387..705b91582b0f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -44,11 +44,10 @@ bp, flg, fm, np) \ .bpc_a = a, \ .chroma_sample = CHROMA_FULL, \ .unpack_align_msb = 0,\ - .unpack_tight = 1,\ .unpack_count = uc, \ .bpp = bp,\ .fetch_mode = fm, \ - .flags = flg, \ + .flags = MSM_FORMAT_FLAG_UNPACK_TIGHT | flg, \ .num_planes = np, \ .tile_height = DPU_TILE_HEIGHT_DEFAULT\ } @@ -66,11 +65,10 @@ alpha, bp, flg, fm, np, th) \ .bpc_a = a, \ .chroma_sample = CHROMA_FULL, \ .unpack_align_msb = 0,\ - .unpack_tight = 1,\ .unpack_count = uc, \ .bpp = bp,\ .fetch_mode = fm, \ - .flags = flg, \ + .flags = MSM_FORMAT_FLAG_UNPACK_TIGHT | flg, \ .num_planes = np, \ .tile_height = th \ } @@ -89,11 +87,10 @@ alpha, chroma, count, bp, flg, fm, np) \ .bpc_a = a, \ .chroma_sample = chroma, \ .unpack_align_msb = 0,\ - .unpack_tight = 1,\ .unpack_count = count,\ .bpp = bp,\ .fetch_mode = fm, \ - .flags = flg, \ + .flags = MSM_FORMAT_FLAG_UNPACK_TIGHT | flg, \ .num_planes = np, \ .tile_height = DPU_TILE_HEIGHT_DEFAULT\ } @@ -110,11 +107,10 @@ alpha, chroma, count, bp, flg, fm, np) \ .bpc_a = a, \ .chroma_sample = chroma, \ .unpack_align_msb = 0,\ - .unpack_tight = 1,\ .unpack_count = 2,\ .bpp = 2, \ .fetch_mode = fm, \ - .flags = flg, \ + .flags = MSM_FORMAT_FLAG_UNPACK_TIGHT | flg, \ .num_planes = np, \ .tile_height = DPU_TILE_HEIGHT_DEFAULT\ } @@ -132,11 +128,10 @@ flg, fm, np, th) \ .bpc_a = a, \ .chroma_sample = chroma, \ .unpack_align_msb = 0,
[PATCH v2 4/9] drm/msm/dpu: pull format flag definitions to mdp_format.h
In preparation to merger of formats databases, pull format flag definitions to mdp_format.h header, so that they are visibile to both dpu and mdp drivers. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 98 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 31 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 8 +-- drivers/gpu/drm/msm/disp/mdp_format.c | 6 +- drivers/gpu/drm/msm/disp/mdp_format.h | 39 drivers/gpu/drm/msm/disp/mdp_kms.h | 4 +- drivers/gpu/drm/msm/msm_drv.h | 4 -- 9 files changed, 109 insertions(+), 89 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index caf536788ece..0c2afded0e56 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -44,8 +44,8 @@ bp, flg, fm, np) \ .unpack_tight = 1,\ .unpack_count = uc, \ .bpp = bp,\ - .fetch_mode = fm, \ - .flags = flg, \ + .base.fetch_mode = fm,\ + .base.flags = flg,\ .num_planes = np, \ .tile_height = DPU_TILE_HEIGHT_DEFAULT\ } @@ -63,8 +63,8 @@ alpha, bp, flg, fm, np, th) \ .unpack_tight = 1,\ .unpack_count = uc, \ .bpp = bp,\ - .fetch_mode = fm, \ - .flags = flg, \ + .base.fetch_mode = fm,\ + .base.flags = flg,\ .num_planes = np, \ .tile_height = th \ } @@ -83,8 +83,8 @@ alpha, chroma, count, bp, flg, fm, np) \ .unpack_tight = 1,\ .unpack_count = count,\ .bpp = bp,\ - .fetch_mode = fm, \ - .flags = flg, \ + .base.fetch_mode = fm,\ + .base.flags = flg,\ .num_planes = np, \ .tile_height = DPU_TILE_HEIGHT_DEFAULT\ } @@ -101,8 +101,8 @@ alpha, chroma, count, bp, flg, fm, np) \ .unpack_tight = 1,\ .unpack_count = 2,\ .bpp = 2, \ - .fetch_mode = fm, \ - .flags = flg, \ + .base.fetch_mode = fm,\ + .base.flags = flg,\ .num_planes = np, \ .tile_height = DPU_TILE_HEIGHT_DEFAULT\ } @@ -120,8 +120,8 @@ flg, fm, np, th) \ .unpack_tight = 1,\ .unpack_count = 2,\ .bpp = 2, \ - .fetch_mode = fm, \ - .flags = flg, \ + .base.fetch_mode = fm,\ + .base.flags = flg,\ .num_planes = np, \ .tile_height = th \ } @@ -138,8 +138,8 @@ flg, fm, np, th)
[PATCH v2 3/9] drm/msm/dpu: in dpu_format replace bitmap with unsigned long field
Using bitmap for the flags results in a clumsy syntax on test_bit,
replace it with unsigned long type and simple binary ops.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 18 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 16 +++-
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 87fa14fc5dd0..caf536788ece 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -45,7 +45,7 @@ bp, flg, fm, np)
\
.unpack_count = uc, \
.bpp = bp,\
.fetch_mode = fm, \
- .flag = {(flg)}, \
+ .flags = flg, \
.num_planes = np, \
.tile_height = DPU_TILE_HEIGHT_DEFAULT\
}
@@ -64,7 +64,7 @@ alpha, bp, flg, fm, np, th)
\
.unpack_count = uc, \
.bpp = bp,\
.fetch_mode = fm, \
- .flag = {(flg)}, \
+ .flags = flg, \
.num_planes = np, \
.tile_height = th \
}
@@ -84,7 +84,7 @@ alpha, chroma, count, bp, flg, fm, np)
\
.unpack_count = count,\
.bpp = bp,\
.fetch_mode = fm, \
- .flag = {(flg)}, \
+ .flags = flg, \
.num_planes = np, \
.tile_height = DPU_TILE_HEIGHT_DEFAULT\
}
@@ -102,7 +102,7 @@ alpha, chroma, count, bp, flg, fm, np)
\
.unpack_count = 2,\
.bpp = 2, \
.fetch_mode = fm, \
- .flag = {(flg)}, \
+ .flags = flg, \
.num_planes = np, \
.tile_height = DPU_TILE_HEIGHT_DEFAULT\
}
@@ -121,7 +121,7 @@ flg, fm, np, th)
\
.unpack_count = 2,\
.bpp = 2, \
.fetch_mode = fm, \
- .flag = {(flg)}, \
+ .flags = flg, \
.num_planes = np, \
.tile_height = th \
}
@@ -139,7 +139,7 @@ flg, fm, np, th)
\
.unpack_count = 2,\
.bpp = 2, \
.fetch_mode = fm, \
- .flag = {(flg)}, \
+ .flags = flg, \
.num_planes = np, \
.tile_height = DPU_TILE_HEIGHT_DEFAULT\
}
@@ -158,7 +158,7 @@ flg, fm, np, th)
\
.unpack_count = 2,\
.bpp = 2, \
.fetch_mode = fm, \
- .flag = {(flg)}, \
+ .flags = flg, \
.num_planes = np, \
.tile_height = th
[PATCH v2 2/9] drm/msm: add arrays listing formats supported by MDP4/MDP5 hardware
MDP4 and MDP5 drivers enumerate supported formats each time the plane is
created. In preparation to merger of MDP DPU format databases, define
precise formats list, so that changes to the database do not cause the
driver to add unsupported format to the list.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 57 +++---
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 36 ++-
drivers/gpu/drm/msm/disp/mdp_format.c | 28 ---
drivers/gpu/drm/msm/disp/mdp_kms.h | 1 -
4 files changed, 80 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
index b689b618da78..cebe20c82a54 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
@@ -371,6 +371,47 @@ static const uint64_t supported_format_modifiers[] = {
DRM_FORMAT_MOD_INVALID
};
+const uint32_t mdp4_rgb_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+};
+
+const uint32_t mdp4_rgb_yuv_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+};
+
/* initialize plane */
struct drm_plane *mdp4_plane_init(struct drm_device *dev,
enum mdp4_pipe pipe_id, bool private_plane)
@@ -379,6 +420,8 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev,
struct mdp4_plane *mdp4_plane;
int ret;
enum drm_plane_type type;
+ const uint32_t *formats;
+ unsigned int nformats;
mdp4_plane = kzalloc(sizeof(*mdp4_plane), GFP_KERNEL);
if (!mdp4_plane) {
@@ -392,13 +435,17 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev,
mdp4_plane->name = pipe_names[pipe_id];
mdp4_plane->caps = mdp4_pipe_caps(pipe_id);
- mdp4_plane->nformats = mdp_get_formats(mdp4_plane->formats,
- ARRAY_SIZE(mdp4_plane->formats),
- !pipe_supports_yuv(mdp4_plane->caps));
-
type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
+
+ if (pipe_supports_yuv(mdp4_plane->caps)) {
+ formats = mdp4_rgb_yuv_formats;
+ nformats = ARRAY_SIZE(mdp4_rgb_yuv_formats);
+ } else {
+ formats = mdp4_rgb_formats;
+ nformats = ARRAY_SIZE(mdp4_rgb_formats);
+ }
ret = drm_universal_plane_init(dev, plane, 0xff, _plane_funcs,
-mdp4_plane->formats, mdp4_plane->nformats,
+formats, nformats,
supported_format_modifiers, type, NULL);
if (ret)
goto fail;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 0d5ff03cb091..aa8342d93393 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -17,9 +17,6 @@
struct mdp5_plane {
struct drm_plane base;
-
- uint32_t nformats;
- uint32_t formats[32];
};
#define to_mdp5_plane(x) container_of(x, struct mdp5_plane, base)
@@ -1007,6 +1004,32 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane)
return mask;
}
+const uint32_t mdp5_plane_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+};
+
/* initialize plane */
struct drm_plane *mdp5_plane_init(struct drm_device *dev,
enum drm_plane_type type)
@@ -1023,12 +1046,9 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
plane =
[PATCH v2 1/9] drm/msm/dpu: use format-related definitions from mdp_common.xml.h
Instead of having DPU-specific defines, switch to the definitions from
the mdp_common.xml.h file. This is the preparation for merged of DPU and
MDP format tables.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 8 +-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 290 ++---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 6 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 64 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c| 12 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 +-
8 files changed, 169 insertions(+), 219 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index aa1e68379d9f..43431cb55421 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2223,19 +2223,19 @@ void dpu_encoder_helper_phys_setup_cdm(struct
dpu_encoder_phys *phys_enc,
/* enable 10 bit logic */
switch (cdm_cfg->output_fmt->chroma_sample) {
- case DPU_CHROMA_RGB:
+ case CHROMA_FULL:
cdm_cfg->h_cdwn_type = CDM_CDWN_DISABLE;
cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
break;
- case DPU_CHROMA_H2V1:
+ case CHROMA_H2V1:
cdm_cfg->h_cdwn_type = CDM_CDWN_COSITE;
cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
break;
- case DPU_CHROMA_420:
+ case CHROMA_420:
cdm_cfg->h_cdwn_type = CDM_CDWN_COSITE;
cdm_cfg->v_cdwn_type = CDM_CDWN_OFFSITE;
break;
- case DPU_CHROMA_H1V2:
+ case CHROMA_H1V2:
default:
DPU_ERROR("[enc:%d] unsupported chroma sampling type\n",
DRMID(phys_enc->parent));
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 9dbb8ddcddec..ff41493147ab 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -594,7 +594,7 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
dpu_encoder_phys *phys_enc
wb_cfg->dest.height = job->fb->height;
wb_cfg->dest.num_planes = wb_cfg->dest.format->num_planes;
- if ((wb_cfg->dest.format->fetch_planes == DPU_PLANE_PLANAR) &&
+ if ((wb_cfg->dest.format->fetch_planes == MDP_PLANE_PLANAR) &&
(wb_cfg->dest.format->element[0] == C1_B_Cb))
swap(wb_cfg->dest.plane_addr[1], wb_cfg->dest.plane_addr[2]);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 95e6e58b1a21..87fa14fc5dd0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -35,11 +35,11 @@
bp, flg, fm, np) \
{ \
.base.pixel_format = DRM_FORMAT_ ## fmt, \
- .fetch_planes = DPU_PLANE_INTERLEAVED,\
+ .fetch_planes = MDP_PLANE_INTERLEAVED,\
.alpha_enable = alpha,\
.element = { (e0), (e1), (e2), (e3) },\
.bits = { g, b, r, a }, \
- .chroma_sample = DPU_CHROMA_RGB, \
+ .chroma_sample = CHROMA_FULL, \
.unpack_align_msb = 0,\
.unpack_tight = 1,\
.unpack_count = uc, \
@@ -54,11 +54,11 @@ bp, flg, fm, np)
\
alpha, bp, flg, fm, np, th) \
{ \
.base.pixel_format = DRM_FORMAT_ ## fmt, \
- .fetch_planes = DPU_PLANE_INTERLEAVED,\
+ .fetch_planes = MDP_PLANE_INTERLEAVED,\
.alpha_enable = alpha,\
.element = { (e0), (e1), (e2), (e3) },\
.bits = { g, b, r, a }, \
- .chroma_sample = DPU_CHROMA_RGB, \
+ .chroma_sample = CHROMA_FULL, \
.unpack_align_msb = 0,\
.unpack_tight =
[PATCH v2 0/9] drm/msm: fold dpu_format into mdp_formats database
During the review of [1] Abhinav pointed out that mdp_rgb_formats and mdp_rgb_yuv_formats arrays from patch 1 are directly based on the struct mdp_format formats array. This was true, because MDP4 / MDP5 drivers used their own (small) list of supported formats. The DPU driver, supporting more formats, had larger database of the formats and their properties. While we don't have plans to expand MDP5 formats support, it make sense to merge these two databases into a common dataset. [1] https://patchwork.freedesktop.org/series/120377/ -- Changes in v2: - Rebased on top of msm-next - Moved all formats data to the new header mdp_formats.h (Abhinav) - Dropped the alpha_enable flag changes (Abhinav) - Link to v1: https://lore.kernel.org/r/20231202214016.1257621-1-dmitry.barysh...@linaro.org --- Dmitry Baryshkov (9): drm/msm/dpu: use format-related definitions from mdp_common.xml.h drm/msm: add arrays listing formats supported by MDP4/MDP5 hardware drm/msm/dpu: in dpu_format replace bitmap with unsigned long field drm/msm/dpu: pull format flag definitions to mdp_format.h drm/msm: merge dpu_format and mdp_format in struct msm_format drm/msm: convert msm_format::unpack_tight to the flag drm/msm: convert msm_format::unpack_align_msb to the flag drm/msm: merge dpu format database to MDP formats drm/msm: drop msm_kms_funcs::get_format() callback drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 20 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 8 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 658 + drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 27 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 16 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 124 +--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c| 40 +- 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 | 22 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 75 +-- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 4 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 1 - drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 86 ++- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 7 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 1 - drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 95 +-- 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 | 630 +--- drivers/gpu/drm/msm/disp/mdp_format.h | 77 +++ drivers/gpu/drm/msm/disp/mdp_kms.h | 18 +- drivers/gpu/drm/msm/msm_drv.h | 4 +- drivers/gpu/drm/msm/msm_fb.c | 2 +- drivers/gpu/drm/msm/msm_kms.h | 4 - 34 files changed, 913 insertions(+), 1075 deletions(-) --- base-commit: a35e92ef04c07bd473404b9b73d489aea19a60a8 change-id: 20240420-dpu-format-d655c60875df Best regards, -- Dmitry Baryshkov
Re: [PATCH 9/9] drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c
On 4/19/2024 8:06 PM, Dmitry Baryshkov wrote:
On Sat, 20 Apr 2024 at 06:05, Abhinav Kumar wrote:
On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote:
Lift mode_config limits set by the DPU driver to the actual FB limits as
handled by the dpu_plane.c.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 7257ac4020d8..e7dda9eca466 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1136,13 +1136,8 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
dev->mode_config.min_width = 0;
dev->mode_config.min_height = 0;
- /*
- * max crtc width is equal to the max mixer width * 2 and max height is
- * is 4K
- */
- dev->mode_config.max_width =
- dpu_kms->catalog->caps->max_mixer_width * 2;
- dev->mode_config.max_height = 4096;
+ dev->mode_config.max_width = DPU_MAX_IMG_WIDTH;
+ dev->mode_config.max_height = DPU_MAX_IMG_HEIGHT;
Can you please explain a little more about why the previous limits did
not work in the multi-monitor case?
We support at the most using 2 LMs per display today. Quad pipe support
is not there yet. So by bounding to 2 * mixer_width should have been
same as rejecting the mixer width in atomic_check.
This is the framebuffer limit, not a CRTC size limit.
As discussed on IRC, the DRM fwk uses this to limit the modes on the
connector, please check
2922if (out_resp->count_modes == 0) {
2923if (is_current_master)
2924connector->funcs->fill_modes(connector,
2925 dev->mode_config.max_width,
2926
dev->mode_config.max_height);
2927else
2928 drm_dbg_kms(dev, "User-space requested a forced probe on
[CONNECTOR:%d:%s] but is not the DRM master, demoting to read-only probe",
2929connector->base.id, connector->name);
2930}
So the documentation of this doesnt really align with the usage.
Unless we alter these pieces, I am hesitant to ack this.
dev->max_vblank_count = 0x;
/* Disable vblank irqs aggressively for power-saving */
Re: [PATCH 9/9] drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c
On Sat, 20 Apr 2024 at 06:05, Abhinav Kumar wrote: > > > > On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote: > > Lift mode_config limits set by the DPU driver to the actual FB limits as > > handled by the dpu_plane.c. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++--- > > 1 file changed, 2 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > index 7257ac4020d8..e7dda9eca466 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > @@ -1136,13 +1136,8 @@ static int dpu_kms_hw_init(struct msm_kms *kms) > > dev->mode_config.min_width = 0; > > dev->mode_config.min_height = 0; > > > > - /* > > - * max crtc width is equal to the max mixer width * 2 and max height > > is > > - * is 4K > > - */ > > - dev->mode_config.max_width = > > - dpu_kms->catalog->caps->max_mixer_width * 2; > > - dev->mode_config.max_height = 4096; > > + dev->mode_config.max_width = DPU_MAX_IMG_WIDTH; > > + dev->mode_config.max_height = DPU_MAX_IMG_HEIGHT; > > > > Can you please explain a little more about why the previous limits did > not work in the multi-monitor case? > > We support at the most using 2 LMs per display today. Quad pipe support > is not there yet. So by bounding to 2 * mixer_width should have been > same as rejecting the mixer width in atomic_check. This is the framebuffer limit, not a CRTC size limit. > > > dev->max_vblank_count = 0x; > > /* Disable vblank irqs aggressively for power-saving */ > > -- With best wishes Dmitry
Re: [PATCH 9/9] drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c
On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote: Lift mode_config limits set by the DPU driver to the actual FB limits as handled by the dpu_plane.c. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 7257ac4020d8..e7dda9eca466 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1136,13 +1136,8 @@ static int dpu_kms_hw_init(struct msm_kms *kms) dev->mode_config.min_width = 0; dev->mode_config.min_height = 0; - /* -* max crtc width is equal to the max mixer width * 2 and max height is -* is 4K -*/ - dev->mode_config.max_width = - dpu_kms->catalog->caps->max_mixer_width * 2; - dev->mode_config.max_height = 4096; + dev->mode_config.max_width = DPU_MAX_IMG_WIDTH; + dev->mode_config.max_height = DPU_MAX_IMG_HEIGHT; Can you please explain a little more about why the previous limits did not work in the multi-monitor case? We support at the most using 2 LMs per display today. Quad pipe support is not there yet. So by bounding to 2 * mixer_width should have been same as rejecting the mixer width in atomic_check. dev->max_vblank_count = 0x; /* Disable vblank irqs aggressively for power-saving */
Re: [PATCH 8/9] drm/msm/dpu: merge MAX_IMG_WIDTH/HEIGHT with DPU_MAX_IMG_WIDTH/HEIGHT
On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote: dpu_formats.c defines DPU_MAX_IMG_WIDTH and _HEIGHT, while dpu_hw_catalog.h defines just MAX_IMG_WIDTH and _HEIGHT. Merge these constants to remove duplication. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 3 --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 ++-- 3 files changed, 4 insertions(+), 7 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH 4/9] drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check
On 4/19/2024 6:34 PM, Dmitry Baryshkov wrote:
On Fri, Apr 19, 2024 at 05:14:01PM -0700, Abhinav Kumar wrote:
On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote:
Move a call to dpu_format_populate_plane_sizes() to the atomic_check
step, so that any issues with the FB layout can be reported as early as
possible.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index d9631fe90228..a9de1fbd0df3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -673,12 +673,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
}
}
- ret = dpu_format_populate_plane_sizes(new_state->fb, >layout);
- if (ret) {
- DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n",
ret);
- return ret;
- }
-
/* validate framebuffer layout before commit */
ret = dpu_format_populate_addrs(pstate->aspace,
new_state->fb,
@@ -864,6 +858,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -E2BIG;
}
+ ret = dpu_format_populate_plane_sizes(new_plane_state->fb,
>layout);
+ if (ret) {
+ DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n",
ret);
+ return ret;
+ }
+
I think we need another function to do the check. It seems incorrect to
populate the layout to the plane state knowing it can potentially fail.
why? The state is interim object, which is subject to checks. In other
parts of the atomic_check we also fill parts of the state, perform
checks and then destroy it if the check fails.
Yes, the same thing you wrote.
I felt we can perform the validation and reject it before populating it
in the state as it seems thats doable here rather than populating it
without knowing whether it can be discarded.
Maybe I'm missing your point here. Could you please explain what is the
problem from your point of view?
Can we move the validation part of dpu_format_populate_plane_sizes() out to
another helper dpu_format_validate_plane_sizes() and use that?
And then make the remaining dpu_format_populate_plane_sizes() just a void
API to fill the layout?
[PATCH 2/3] drm/msm/mdp4: don't destroy mdp4_kms in mdp4_kms_init error path
Since commit 3c74682637e6 ("drm/msm/mdp4: move resource allocation to
the _probe function") the mdp4_kms data is allocated during probe. It is
an error to destroy it during mdp4_kms_init(), as the data is still
referenced by the drivers's data and can be used later in case of probe
deferral.
Fixes: 3c74682637e6 ("drm/msm/mdp4: move resource allocation to the _probe
function")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 28 +---
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 4ba1cb74ad76..4c5f72b7e0e5 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -392,7 +392,7 @@ static int mdp4_kms_init(struct drm_device *dev)
ret = mdp_kms_init(_kms->base, _funcs);
if (ret) {
DRM_DEV_ERROR(dev->dev, "failed to init kms\n");
- goto fail;
+ return ret;
}
kms = priv->kms;
@@ -403,7 +403,7 @@ static int mdp4_kms_init(struct drm_device *dev)
ret = regulator_enable(mdp4_kms->vdd);
if (ret) {
DRM_DEV_ERROR(dev->dev, "failed to enable regulator
vdd: %d\n", ret);
- goto fail;
+ return ret;
}
}
@@ -414,8 +414,7 @@ static int mdp4_kms_init(struct drm_device *dev)
if (major != 4) {
DRM_DEV_ERROR(dev->dev, "unexpected MDP version: v%d.%d\n",
major, minor);
- ret = -ENXIO;
- goto fail;
+ return -ENXIO;
}
mdp4_kms->rev = minor;
@@ -423,8 +422,7 @@ static int mdp4_kms_init(struct drm_device *dev)
if (mdp4_kms->rev >= 2) {
if (!mdp4_kms->lut_clk) {
DRM_DEV_ERROR(dev->dev, "failed to get lut_clk\n");
- ret = -ENODEV;
- goto fail;
+ return -ENODEV;
}
clk_set_rate(mdp4_kms->lut_clk, max_clk);
}
@@ -445,8 +443,7 @@ static int mdp4_kms_init(struct drm_device *dev)
mmu = msm_iommu_new(>dev, 0);
if (IS_ERR(mmu)) {
- ret = PTR_ERR(mmu);
- goto fail;
+ return PTR_ERR(mmu);
} else if (!mmu) {
DRM_DEV_INFO(dev->dev, "no iommu, fallback to phys "
"contig buffers for scanout\n");
@@ -458,8 +455,7 @@ static int mdp4_kms_init(struct drm_device *dev)
if (IS_ERR(aspace)) {
if (!IS_ERR(mmu))
mmu->funcs->destroy(mmu);
- ret = PTR_ERR(aspace);
- goto fail;
+ return PTR_ERR(aspace);
}
kms->aspace = aspace;
@@ -468,7 +464,7 @@ static int mdp4_kms_init(struct drm_device *dev)
ret = modeset_init(mdp4_kms);
if (ret) {
DRM_DEV_ERROR(dev->dev, "modeset_init failed: %d\n", ret);
- goto fail;
+ return ret;
}
mdp4_kms->blank_cursor_bo = msm_gem_new(dev, SZ_16K, MSM_BO_WC |
MSM_BO_SCANOUT);
@@ -476,14 +472,14 @@ static int mdp4_kms_init(struct drm_device *dev)
ret = PTR_ERR(mdp4_kms->blank_cursor_bo);
DRM_DEV_ERROR(dev->dev, "could not allocate blank-cursor bo:
%d\n", ret);
mdp4_kms->blank_cursor_bo = NULL;
- goto fail;
+ return ret;
}
ret = msm_gem_get_and_pin_iova(mdp4_kms->blank_cursor_bo, kms->aspace,
_kms->blank_cursor_iova);
if (ret) {
DRM_DEV_ERROR(dev->dev, "could not pin blank-cursor bo: %d\n",
ret);
- goto fail;
+ return ret;
}
dev->mode_config.min_width = 0;
@@ -492,12 +488,6 @@ static int mdp4_kms_init(struct drm_device *dev)
dev->mode_config.max_height = 2048;
return 0;
-
-fail:
- if (kms)
- mdp4_destroy(kms);
-
- return ret;
}
static const struct dev_pm_ops mdp4_pm_ops = {
--
2.39.2
[PATCH 1/3] drm/msm: don't clean up priv->kms prematurely
MSM display drivers provide kms structure allocated during probe().
Don't clean up priv->kms field in case of an error. Otherwise probe
functions might fail after KMS probe deferral.
Fixes: a2ab5d5bb6b1 ("drm/msm: allow passing struct msm_kms to msm_drv_probe()")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/msm_kms.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
index af6a6fcb1173..6749f0fbca96 100644
--- a/drivers/gpu/drm/msm/msm_kms.c
+++ b/drivers/gpu/drm/msm/msm_kms.c
@@ -244,7 +244,6 @@ int msm_drm_kms_init(struct device *dev, const struct
drm_driver *drv)
ret = priv->kms_init(ddev);
if (ret) {
DRM_DEV_ERROR(dev, "failed to load kms\n");
- priv->kms = NULL;
return ret;
}
--
2.39.2
[PATCH 3/3] drm/msm/mdp4: correct LCDC regulator name
Correct c error from the conversion of LCDC regulators to the bulk
API.
Fixes: 54f1fbcb47d4 ("drm/msm/mdp4: use bulk regulators API for LCDC encoder")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
index 576995ddce37..8bbc7fb881d5 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
@@ -389,7 +389,7 @@ struct drm_encoder *mdp4_lcdc_encoder_init(struct
drm_device *dev,
/* TODO: different regulators in other cases? */
mdp4_lcdc_encoder->regs[0].supply = "lvds-vccs-3p3v";
- mdp4_lcdc_encoder->regs[1].supply = "lvds-vccs-3p3v";
+ mdp4_lcdc_encoder->regs[1].supply = "lvds-pll-vdda";
mdp4_lcdc_encoder->regs[2].supply = "lvds-vdda";
ret = devm_regulator_bulk_get(dev->dev,
--
2.39.2
[PATCH 0/3] drm/msm/mdp4: fix probe deferral issues
While testing MDP4 LVDS support I noticed several issues (two are related to probe deferral case and last one is a c error in LCDC part). Fix those issues. Signed-off-by: Dmitry Baryshkov --- Dmitry Baryshkov (3): drm/msm: don't clean up priv->kms prematurely drm/msm/mdp4: don't destroy mdp4_kms in mdp4_kms_init error path drm/msm/mdp4: correct LCDC regulator name drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 28 --- drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 2 +- drivers/gpu/drm/msm/msm_kms.c | 1 - 3 files changed, 10 insertions(+), 21 deletions(-) --- base-commit: 7b4f2bc91c15fdcf948bb2d9741a9d7d54303f8d change-id: 20240420-mdp4-fixes-f33b5a21308b Best regards, -- Dmitry Baryshkov
Re: [PATCH 1/9] drm/msm/dpu: drop dpu_format_check_modified_format
On 4/19/2024 6:26 PM, Dmitry Baryshkov wrote:
On Fri, Apr 19, 2024 at 04:43:20PM -0700, Abhinav Kumar wrote:
On 3/19/2024 6:21 AM, Dmitry Baryshkov wrote:
The msm_kms_funcs::check_modified_format() callback is not used by the
driver. Drop it completely.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 45
-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 15 --
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 -
drivers/gpu/drm/msm/msm_kms.h | 5
4 files changed, 66 deletions(-)
I think in this case, I am leaning towards completing the implementation
rather than dropping it as usual.
It seems its easier to just add the support to call this like the attached
patch?
Please don't attach patches to the email. It makes it impossible to
respond to them.
I attached it because it was too much to paste over here.
Please review msm_framebuffer_init() in the downstream sources.
The only missing piece I can see is the handling of
DRM_MODE_FB_MODIFIERS flags.
I am unable to trace back why this support was not present.
Anyway, what are we missing with the current codebase? Why wasn't the
callback / function used in the first place?
WDYT?
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index e366ab134249..ff0df478c958 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -960,51 +960,6 @@ int dpu_format_populate_layout(
return ret;
}
-int dpu_format_check_modified_format(
- const struct msm_kms *kms,
- const struct msm_format *msm_fmt,
- const struct drm_mode_fb_cmd2 *cmd,
- struct drm_gem_object **bos)
-{
- const struct drm_format_info *info;
- const struct dpu_format *fmt;
- struct dpu_hw_fmt_layout layout;
- uint32_t bos_total_size = 0;
- int ret, i;
-
- if (!msm_fmt || !cmd || !bos) {
- DRM_ERROR("invalid arguments\n");
- return -EINVAL;
- }
-
- fmt = to_dpu_format(msm_fmt);
- info = drm_format_info(fmt->base.pixel_format);
- if (!info)
- return -EINVAL;
-
- ret = dpu_format_get_plane_sizes(fmt, cmd->width, cmd->height,
- , cmd->pitches);
- if (ret)
- return ret;
-
- for (i = 0; i < info->num_planes; i++) {
- if (!bos[i]) {
- DRM_ERROR("invalid handle for plane %d\n", i);
- return -EINVAL;
- }
- if ((i == 0) || (bos[i] != bos[0]))
- bos_total_size += bos[i]->size;
- }
-
- if (bos_total_size < layout.total_size) {
- DRM_ERROR("buffers total size too small %u expected %u\n",
- bos_total_size, layout.total_size);
- return -EINVAL;
- }
-
- return 0;
-}
-
const struct dpu_format *dpu_get_dpu_format_ext(
const uint32_t format,
const uint64_t modifier)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
index 84b8b3289f18..9442445f1a86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
@@ -54,21 +54,6 @@ const struct msm_format *dpu_get_msm_format(
const uint32_t format,
const uint64_t modifiers);
-/**
- * dpu_format_check_modified_format - validate format and buffers for
- * dpu non-standard, i.e. modified format
- * @kms: kms driver
- * @msm_fmt: pointer to the msm_fmt base pointer of an dpu_format
- * @cmd: fb_cmd2 structure user request
- * @bos: gem buffer object list
- *
- * Return: error code on failure, 0 on success
- */
-int dpu_format_check_modified_format(
- const struct msm_kms *kms,
- const struct msm_format *msm_fmt,
- const struct drm_mode_fb_cmd2 *cmd,
- struct drm_gem_object **bos);
/**
* dpu_format_populate_layout - populate the given format layout based on
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index a1f5d7c4ab91..7257ac4020d8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -969,7 +969,6 @@ static const struct msm_kms_funcs kms_funcs = {
.complete_commit = dpu_kms_complete_commit,
.enable_vblank = dpu_kms_enable_vblank,
.disable_vblank = dpu_kms_disable_vblank,
- .check_modified_format = dpu_format_check_modified_format,
.get_format = dpu_get_msm_format,
.destroy = dpu_kms_destroy,
.snapshot= dpu_kms_mdp_snapshot,
diff --git a/drivers/gpu/drm/msm/msm_kms.h
Re: [PATCH 5/9] drm/msm/dpu: check for the plane pitch overflow
On Fri, Apr 19, 2024 at 05:16:30PM -0700, Abhinav Kumar wrote:
>
>
> On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote:
> > Check that the plane pitch doesn't overflow the maximum pitch size
> > allowed by the hardware.
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 ++
> > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 6 +-
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > index b7dc52312c39..86b1defa5d21 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > @@ -12,6 +12,8 @@
> > struct dpu_hw_sspp;
> > +#define DPU_SSPP_MAX_PITCH_SIZE0x
> > +
>
> You obtained this value from below code right?
Yes. And also from DPU_MAX_IMG_WIDTH / MAX_IMG_WIDTH.
>
> if (pipe->multirect_index == DPU_SSPP_RECT_0) {
> 487 ystride0 = (ystride0 & 0x) |
> 488 (layout->plane_pitch[0] & 0x);
> 489 ystride1 = (ystride1 & 0x)|
> 490 (layout->plane_pitch[2] & 0x);
> 491 } else {
> 492 ystride0 = (ystride0 & 0x) |
> 493 ((layout->plane_pitch[0] << 16) &
> 4940x);
> 495 ystride1 = (ystride1 & 0x) |
> 496 ((layout->plane_pitch[2] << 16) &
> 4970x);
> 498 }
>
> Seems correct, but was just curious
>
> Reviewed-by: Abhinav Kumar
--
With best wishes
Dmitry
Re: [PATCH 4/9] drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check
On Fri, Apr 19, 2024 at 05:14:01PM -0700, Abhinav Kumar wrote:
>
>
> On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote:
> > Move a call to dpu_format_populate_plane_sizes() to the atomic_check
> > step, so that any issues with the FB layout can be reported as early as
> > possible.
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 ++--
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index d9631fe90228..a9de1fbd0df3 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -673,12 +673,6 @@ static int dpu_plane_prepare_fb(struct drm_plane
> > *plane,
> > }
> > }
> > - ret = dpu_format_populate_plane_sizes(new_state->fb, >layout);
> > - if (ret) {
> > - DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n",
> > ret);
> > - return ret;
> > - }
> > -
> > /* validate framebuffer layout before commit */
> > ret = dpu_format_populate_addrs(pstate->aspace,
> > new_state->fb,
> > @@ -864,6 +858,12 @@ static int dpu_plane_atomic_check(struct drm_plane
> > *plane,
> > return -E2BIG;
> > }
> > + ret = dpu_format_populate_plane_sizes(new_plane_state->fb,
> > >layout);
> > + if (ret) {
> > + DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n",
> > ret);
> > + return ret;
> > + }
> > +
>
> I think we need another function to do the check. It seems incorrect to
> populate the layout to the plane state knowing it can potentially fail.
why? The state is interim object, which is subject to checks. In other
parts of the atomic_check we also fill parts of the state, perform
checks and then destroy it if the check fails.
Maybe I'm missing your point here. Could you please explain what is the
problem from your point of view?
>
> Can we move the validation part of dpu_format_populate_plane_sizes() out to
> another helper dpu_format_validate_plane_sizes() and use that?
>
> And then make the remaining dpu_format_populate_plane_sizes() just a void
> API to fill the layout?
--
With best wishes
Dmitry
Re: [PATCH 1/9] drm/msm/dpu: drop dpu_format_check_modified_format
On Fri, Apr 19, 2024 at 04:43:20PM -0700, Abhinav Kumar wrote:
>
>
> On 3/19/2024 6:21 AM, Dmitry Baryshkov wrote:
> > The msm_kms_funcs::check_modified_format() callback is not used by the
> > driver. Drop it completely.
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 45
> > -
> > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 15 --
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 -
> > drivers/gpu/drm/msm/msm_kms.h | 5
> > 4 files changed, 66 deletions(-)
> >
>
> I think in this case, I am leaning towards completing the implementation
> rather than dropping it as usual.
>
> It seems its easier to just add the support to call this like the attached
> patch?
Please don't attach patches to the email. It makes it impossible to
respond to them.
Anyway, what are we missing with the current codebase? Why wasn't the
callback / function used in the first place?
>
> WDYT?
>
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > index e366ab134249..ff0df478c958 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > @@ -960,51 +960,6 @@ int dpu_format_populate_layout(
> > return ret;
> > }
> > -int dpu_format_check_modified_format(
> > - const struct msm_kms *kms,
> > - const struct msm_format *msm_fmt,
> > - const struct drm_mode_fb_cmd2 *cmd,
> > - struct drm_gem_object **bos)
> > -{
> > - const struct drm_format_info *info;
> > - const struct dpu_format *fmt;
> > - struct dpu_hw_fmt_layout layout;
> > - uint32_t bos_total_size = 0;
> > - int ret, i;
> > -
> > - if (!msm_fmt || !cmd || !bos) {
> > - DRM_ERROR("invalid arguments\n");
> > - return -EINVAL;
> > - }
> > -
> > - fmt = to_dpu_format(msm_fmt);
> > - info = drm_format_info(fmt->base.pixel_format);
> > - if (!info)
> > - return -EINVAL;
> > -
> > - ret = dpu_format_get_plane_sizes(fmt, cmd->width, cmd->height,
> > - , cmd->pitches);
> > - if (ret)
> > - return ret;
> > -
> > - for (i = 0; i < info->num_planes; i++) {
> > - if (!bos[i]) {
> > - DRM_ERROR("invalid handle for plane %d\n", i);
> > - return -EINVAL;
> > - }
> > - if ((i == 0) || (bos[i] != bos[0]))
> > - bos_total_size += bos[i]->size;
> > - }
> > -
> > - if (bos_total_size < layout.total_size) {
> > - DRM_ERROR("buffers total size too small %u expected %u\n",
> > - bos_total_size, layout.total_size);
> > - return -EINVAL;
> > - }
> > -
> > - return 0;
> > -}
> > -
> > const struct dpu_format *dpu_get_dpu_format_ext(
> > const uint32_t format,
> > const uint64_t modifier)
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
> > index 84b8b3289f18..9442445f1a86 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
> > @@ -54,21 +54,6 @@ const struct msm_format *dpu_get_msm_format(
> > const uint32_t format,
> > const uint64_t modifiers);
> > -/**
> > - * dpu_format_check_modified_format - validate format and buffers for
> > - * dpu non-standard, i.e. modified format
> > - * @kms: kms driver
> > - * @msm_fmt: pointer to the msm_fmt base pointer of an dpu_format
> > - * @cmd: fb_cmd2 structure user request
> > - * @bos: gem buffer object list
> > - *
> > - * Return: error code on failure, 0 on success
> > - */
> > -int dpu_format_check_modified_format(
> > - const struct msm_kms *kms,
> > - const struct msm_format *msm_fmt,
> > - const struct drm_mode_fb_cmd2 *cmd,
> > - struct drm_gem_object **bos);
> > /**
> >* dpu_format_populate_layout - populate the given format layout based on
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index a1f5d7c4ab91..7257ac4020d8 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -969,7 +969,6 @@ static const struct msm_kms_funcs kms_funcs = {
> > .complete_commit = dpu_kms_complete_commit,
> > .enable_vblank = dpu_kms_enable_vblank,
> > .disable_vblank = dpu_kms_disable_vblank,
> > - .check_modified_format = dpu_format_check_modified_format,
> > .get_format = dpu_get_msm_format,
> > .destroy = dpu_kms_destroy,
> > .snapshot= dpu_kms_mdp_snapshot,
> > diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> > index 0641f6111b93..b794ed918b56 100644
> > --- a/drivers/gpu/drm/msm/msm_kms.h
Re: [PATCH 5/9] drm/msm/dpu: check for the plane pitch overflow
On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote:
Check that the plane pitch doesn't overflow the maximum pitch size
allowed by the hardware.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 6 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index b7dc52312c39..86b1defa5d21 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -12,6 +12,8 @@
struct dpu_hw_sspp;
+#define DPU_SSPP_MAX_PITCH_SIZE 0x
+
You obtained this value from below code right?
if (pipe->multirect_index == DPU_SSPP_RECT_0) {
487 ystride0 = (ystride0 & 0x) |
488 (layout->plane_pitch[0] & 0x);
489 ystride1 = (ystride1 & 0x)|
490 (layout->plane_pitch[2] & 0x);
491 } else {
492 ystride0 = (ystride0 & 0x) |
493 ((layout->plane_pitch[0] << 16) &
494 0x);
495 ystride1 = (ystride1 & 0x) |
496 ((layout->plane_pitch[2] << 16) &
497 0x);
498 }
Seems correct, but was just curious
Reviewed-by: Abhinav Kumar
Re: [PATCH 4/9] drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check
On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote:
Move a call to dpu_format_populate_plane_sizes() to the atomic_check
step, so that any issues with the FB layout can be reported as early as
possible.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index d9631fe90228..a9de1fbd0df3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -673,12 +673,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
}
}
- ret = dpu_format_populate_plane_sizes(new_state->fb, >layout);
- if (ret) {
- DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n",
ret);
- return ret;
- }
-
/* validate framebuffer layout before commit */
ret = dpu_format_populate_addrs(pstate->aspace,
new_state->fb,
@@ -864,6 +858,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -E2BIG;
}
+ ret = dpu_format_populate_plane_sizes(new_plane_state->fb, >layout);
+ if (ret) {
+ DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n",
ret);
+ return ret;
+ }
+
I think we need another function to do the check. It seems incorrect to
populate the layout to the plane state knowing it can potentially fail.
Can we move the validation part of dpu_format_populate_plane_sizes() out
to another helper dpu_format_validate_plane_sizes() and use that?
And then make the remaining dpu_format_populate_plane_sizes() just a
void API to fill the layout?
Re: [PATCH 3/9] drm/msm/dpu: split dpu_format_populate_layout
On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote: Split dpu_format_populate_layout() into addess-related and pitch/format-related parts. Signed-off-by: Dmitry Baryshkov --- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 8 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 44 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 8 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 -- 4 files changed, 45 insertions(+), 27 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH 2/9] drm/msm/dpu: drop dpu_format_populate_layout from dpu_plane_sspp_atomic_update
On 3/19/2024 6:22 AM, Dmitry Baryshkov wrote: The dpu_plane_prepare_fb() already calls dpu_format_populate_layout(). Store the generated layour in the plane state and drop this call from dpu_plane_sspp_update(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 19 --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 3 +++ 2 files changed, 7 insertions(+), 15 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH 1/9] drm/msm/dpu: drop dpu_format_check_modified_format
On 3/19/2024 6:21 AM, Dmitry Baryshkov wrote:
The msm_kms_funcs::check_modified_format() callback is not used by the
driver. Drop it completely.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 45 -
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 15 --
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 -
drivers/gpu/drm/msm/msm_kms.h | 5
4 files changed, 66 deletions(-)
I think in this case, I am leaning towards completing the implementation
rather than dropping it as usual.
It seems its easier to just add the support to call this like the
attached patch?
WDYT?
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index e366ab134249..ff0df478c958 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -960,51 +960,6 @@ int dpu_format_populate_layout(
return ret;
}
-int dpu_format_check_modified_format(
- const struct msm_kms *kms,
- const struct msm_format *msm_fmt,
- const struct drm_mode_fb_cmd2 *cmd,
- struct drm_gem_object **bos)
-{
- const struct drm_format_info *info;
- const struct dpu_format *fmt;
- struct dpu_hw_fmt_layout layout;
- uint32_t bos_total_size = 0;
- int ret, i;
-
- if (!msm_fmt || !cmd || !bos) {
- DRM_ERROR("invalid arguments\n");
- return -EINVAL;
- }
-
- fmt = to_dpu_format(msm_fmt);
- info = drm_format_info(fmt->base.pixel_format);
- if (!info)
- return -EINVAL;
-
- ret = dpu_format_get_plane_sizes(fmt, cmd->width, cmd->height,
- , cmd->pitches);
- if (ret)
- return ret;
-
- for (i = 0; i < info->num_planes; i++) {
- if (!bos[i]) {
- DRM_ERROR("invalid handle for plane %d\n", i);
- return -EINVAL;
- }
- if ((i == 0) || (bos[i] != bos[0]))
- bos_total_size += bos[i]->size;
- }
-
- if (bos_total_size < layout.total_size) {
- DRM_ERROR("buffers total size too small %u expected %u\n",
- bos_total_size, layout.total_size);
- return -EINVAL;
- }
-
- return 0;
-}
-
const struct dpu_format *dpu_get_dpu_format_ext(
const uint32_t format,
const uint64_t modifier)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
index 84b8b3289f18..9442445f1a86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
@@ -54,21 +54,6 @@ const struct msm_format *dpu_get_msm_format(
const uint32_t format,
const uint64_t modifiers);
-/**
- * dpu_format_check_modified_format - validate format and buffers for
- * dpu non-standard, i.e. modified format
- * @kms: kms driver
- * @msm_fmt: pointer to the msm_fmt base pointer of an dpu_format
- * @cmd: fb_cmd2 structure user request
- * @bos: gem buffer object list
- *
- * Return: error code on failure, 0 on success
- */
-int dpu_format_check_modified_format(
- const struct msm_kms *kms,
- const struct msm_format *msm_fmt,
- const struct drm_mode_fb_cmd2 *cmd,
- struct drm_gem_object **bos);
/**
* dpu_format_populate_layout - populate the given format layout based on
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index a1f5d7c4ab91..7257ac4020d8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -969,7 +969,6 @@ static const struct msm_kms_funcs kms_funcs = {
.complete_commit = dpu_kms_complete_commit,
.enable_vblank = dpu_kms_enable_vblank,
.disable_vblank = dpu_kms_disable_vblank,
- .check_modified_format = dpu_format_check_modified_format,
.get_format = dpu_get_msm_format,
.destroy = dpu_kms_destroy,
.snapshot= dpu_kms_mdp_snapshot,
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 0641f6111b93..b794ed918b56 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -96,11 +96,6 @@ struct msm_kms_funcs {
const struct msm_format *(*get_format)(struct msm_kms *kms,
const uint32_t format,
const uint64_t modifiers);
- /* do format checking on format modified through fb_cmd2 modifiers */
- int (*check_modified_format)(const struct msm_kms *kms,
- const struct msm_format *msm_fmt,
-
Re: [PATCH 2/7] drm/msm/dsi: Pass bonded-DSI hdisplay/2 to DSC timing configuration
On Sat, Apr 20, 2024 at 12:18:39AM +0200, Marijn Suijten wrote:
> On 2024-04-17 14:58:25, Dmitry Baryshkov wrote:
> > On Wed, 17 Apr 2024 at 02:57, Marijn Suijten
> > wrote:
> > >
> > > When configuring the timing of DSI hosts (interfaces) in
> > > dsi_timing_setup() all values written to registers are taking bonded
> > > DSI into account by dividing the original mode width by 2 (half the
> > > data is sent over each of the two DSI hosts), but the full width
> > > instead of the interface width is passed as hdisplay parameter to
> > > dsi_update_dsc_timing().
> > >
> > > Currently only msm_dsc_get_slices_per_intf() is called within
> > > dsi_update_dsc_timing() with the `hdisplay` argument which clearly
> > > documents that it wants the width of a single interface (which, again,
> > > in bonded DSI mode is half the total width of the mode). Thus pass the
> > > bonded-mode-adjusted hdisplay parameter into dsi_update_dsc_timing()
> > > otherwise all values written to registers by this function (i.e. the
> > > number of slices per interface or packet, and derived from this the EOL
> > > byte number) are twice too large.
> > >
> > > Inversely the panel driver is expected to only set the slice width and
> > > number of slices for half the panel, i.e. what will be sent by each
> > > host individually, rather than fixing that up like hdisplay here.
> > >
> > > Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> > > Signed-off-by: Marijn Suijten
> > > ---
> > > drivers/gpu/drm/msm/dsi/dsi_host.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Reviewed-by: Dmitry Baryshkov
>
> Thanks, it seems this patch has already been picked up for 6.10 [1] to test at
> least, but I'd advise you to drop it until I resend it in v2, as it no longer
> performs as written in the title.
Ok, dropping.
>
> When I wrote this patch in in June 2023, commit efcbd6f9cdeb ("drm/msm/
> dsi: Enable widebus for DSI") from August 2023 wasn't there yet. That patch
> updates hdisplay (because it is unused after that point) with the number
> of compressed bytes to be sent over each interface, which is effectively
> hdisplay (based on slice_count * slice_width, so as explained in the commit
> message that corresponds to half the panel width), divided by a compression
> ratio of 3 or 6 depending on widebus, thus passing a way too low value into
> dsi_update_dsc_timing().
>
> As a result this patch regresses the DSC panel on my SM8150 Sony Xperia 1, and
> likely also explains why it was quite hard to get the porches "just right" on
> the Xperia 1 III with its dual-DSI dual-DSC 4k@120Hz panel (that these patches
> are specifically for).
>
> I'm still thinking of how to best fix that: probably introducing a new
> separate
> local variable, though dsi_update_dsc_timing() only uses it to calculate
> the number of slices per interface, which again as written in the commit
> description, is currently required to already be for one interface (in other
> words, the Xperia 1 with only a single intf sets slice_count=2, but the
> Xperia 1
> III with 2 bonded DSI interfaces sets slice_count=1). Which means that this
> is
> always equivalent to slice_per_intf = dsc->slice_count.
>
> Let me know which approach is preferred.
>
> - Marijn
>
> [1]: https://gitlab.freedesktop.org/drm/msm/-/merge_requests/110
--
With best wishes
Dmitry
Re: [PATCH 2/7] drm/msm/dsi: Pass bonded-DSI hdisplay/2 to DSC timing configuration
On 2024-04-17 14:58:25, Dmitry Baryshkov wrote:
> On Wed, 17 Apr 2024 at 02:57, Marijn Suijten
> wrote:
> >
> > When configuring the timing of DSI hosts (interfaces) in
> > dsi_timing_setup() all values written to registers are taking bonded
> > DSI into account by dividing the original mode width by 2 (half the
> > data is sent over each of the two DSI hosts), but the full width
> > instead of the interface width is passed as hdisplay parameter to
> > dsi_update_dsc_timing().
> >
> > Currently only msm_dsc_get_slices_per_intf() is called within
> > dsi_update_dsc_timing() with the `hdisplay` argument which clearly
> > documents that it wants the width of a single interface (which, again,
> > in bonded DSI mode is half the total width of the mode). Thus pass the
> > bonded-mode-adjusted hdisplay parameter into dsi_update_dsc_timing()
> > otherwise all values written to registers by this function (i.e. the
> > number of slices per interface or packet, and derived from this the EOL
> > byte number) are twice too large.
> >
> > Inversely the panel driver is expected to only set the slice width and
> > number of slices for half the panel, i.e. what will be sent by each
> > host individually, rather than fixing that up like hdisplay here.
> >
> > Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> > Signed-off-by: Marijn Suijten
> > ---
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Reviewed-by: Dmitry Baryshkov
Thanks, it seems this patch has already been picked up for 6.10 [1] to test at
least, but I'd advise you to drop it until I resend it in v2, as it no longer
performs as written in the title.
When I wrote this patch in in June 2023, commit efcbd6f9cdeb ("drm/msm/
dsi: Enable widebus for DSI") from August 2023 wasn't there yet. That patch
updates hdisplay (because it is unused after that point) with the number
of compressed bytes to be sent over each interface, which is effectively
hdisplay (based on slice_count * slice_width, so as explained in the commit
message that corresponds to half the panel width), divided by a compression
ratio of 3 or 6 depending on widebus, thus passing a way too low value into
dsi_update_dsc_timing().
As a result this patch regresses the DSC panel on my SM8150 Sony Xperia 1, and
likely also explains why it was quite hard to get the porches "just right" on
the Xperia 1 III with its dual-DSI dual-DSC 4k@120Hz panel (that these patches
are specifically for).
I'm still thinking of how to best fix that: probably introducing a new separate
local variable, though dsi_update_dsc_timing() only uses it to calculate
the number of slices per interface, which again as written in the commit
description, is currently required to already be for one interface (in other
words, the Xperia 1 with only a single intf sets slice_count=2, but the Xperia 1
III with 2 bonded DSI interfaces sets slice_count=1). Which means that this is
always equivalent to slice_per_intf = dsc->slice_count.
Let me know which approach is preferred.
- Marijn
[1]: https://gitlab.freedesktop.org/drm/msm/-/merge_requests/110
Re: [PATCH 04/12] drm/msm: add arrays listing formats supported by MDP4/MDP5 hardware
On 4/19/2024 2:21 PM, Dmitry Baryshkov wrote:
On Sat, 20 Apr 2024 at 00:06, Abhinav Kumar wrote:
On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:
MDP4 and MDP5 drivers enumerate supported formats each time the plane is
created. In preparation to merger of MDP DPU format databases, define
precise formats list, so that changes to the database do not cause the
driver to add unsupported format to the list.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 57 --
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 36 +++---
drivers/gpu/drm/msm/disp/mdp_format.c | 28 ---
drivers/gpu/drm/msm/disp/mdp_kms.h | 1 -
4 files changed, 80 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
index b689b618da78..cebe20c82a54 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
@@ -371,6 +371,47 @@ static const uint64_t supported_format_modifiers[] = {
DRM_FORMAT_MOD_INVALID
};
+const uint32_t mdp4_rgb_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+};
+
+const uint32_t mdp4_rgb_yuv_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+};
+
/* initialize plane */
struct drm_plane *mdp4_plane_init(struct drm_device *dev,
enum mdp4_pipe pipe_id, bool private_plane)
@@ -379,6 +420,8 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev,
struct mdp4_plane *mdp4_plane;
int ret;
enum drm_plane_type type;
+ const uint32_t *formats;
+ unsigned int nformats;
mdp4_plane = kzalloc(sizeof(*mdp4_plane), GFP_KERNEL);
if (!mdp4_plane) {
@@ -392,13 +435,17 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev,
mdp4_plane->name = pipe_names[pipe_id];
mdp4_plane->caps = mdp4_pipe_caps(pipe_id);
- mdp4_plane->nformats = mdp_get_formats(mdp4_plane->formats,
- ARRAY_SIZE(mdp4_plane->formats),
- !pipe_supports_yuv(mdp4_plane->caps));
-
type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
+
+ if (pipe_supports_yuv(mdp4_plane->caps)) {
+ formats = mdp4_rgb_yuv_formats;
+ nformats = ARRAY_SIZE(mdp4_rgb_yuv_formats);
+ } else {
+ formats = mdp4_rgb_formats;
+ nformats = ARRAY_SIZE(mdp4_rgb_formats);
+ }
ret = drm_universal_plane_init(dev, plane, 0xff, _plane_funcs,
- mdp4_plane->formats, mdp4_plane->nformats,
+ formats, nformats,
supported_format_modifiers, type, NULL);
if (ret)
goto fail;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 0d5ff03cb091..aa8342d93393 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -17,9 +17,6 @@
struct mdp5_plane {
struct drm_plane base;
-
- uint32_t nformats;
- uint32_t formats[32];
};
#define to_mdp5_plane(x) container_of(x, struct mdp5_plane, base)
@@ -1007,6 +1004,32 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane)
return mask;
}
+const uint32_t mdp5_plane_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+};
+
/* initialize plane */
struct drm_plane *mdp5_plane_init(struct drm_device *dev,
enum drm_plane_type type)
@@ -1023,12 +1046,9 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
plane = _plane->base;
- mdp5_plane->nformats =
Re: [PATCH 04/12] drm/msm: add arrays listing formats supported by MDP4/MDP5 hardware
On Sat, 20 Apr 2024 at 00:06, Abhinav Kumar wrote:
>
>
>
> On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:
> > MDP4 and MDP5 drivers enumerate supported formats each time the plane is
> > created. In preparation to merger of MDP DPU format databases, define
> > precise formats list, so that changes to the database do not cause the
> > driver to add unsupported format to the list.
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 57 --
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 36 +++---
> > drivers/gpu/drm/msm/disp/mdp_format.c | 28 ---
> > drivers/gpu/drm/msm/disp/mdp_kms.h | 1 -
> > 4 files changed, 80 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
> > b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
> > index b689b618da78..cebe20c82a54 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
> > @@ -371,6 +371,47 @@ static const uint64_t supported_format_modifiers[] = {
> > DRM_FORMAT_MOD_INVALID
> > };
> >
> > +const uint32_t mdp4_rgb_formats[] = {
> > + DRM_FORMAT_ARGB,
> > + DRM_FORMAT_ABGR,
> > + DRM_FORMAT_RGBA,
> > + DRM_FORMAT_BGRA,
> > + DRM_FORMAT_XRGB,
> > + DRM_FORMAT_XBGR,
> > + DRM_FORMAT_RGBX,
> > + DRM_FORMAT_BGRX,
> > + DRM_FORMAT_RGB888,
> > + DRM_FORMAT_BGR888,
> > + DRM_FORMAT_RGB565,
> > + DRM_FORMAT_BGR565,
> > +};
> > +
> > +const uint32_t mdp4_rgb_yuv_formats[] = {
> > + DRM_FORMAT_ARGB,
> > + DRM_FORMAT_ABGR,
> > + DRM_FORMAT_RGBA,
> > + DRM_FORMAT_BGRA,
> > + DRM_FORMAT_XRGB,
> > + DRM_FORMAT_XBGR,
> > + DRM_FORMAT_RGBX,
> > + DRM_FORMAT_BGRX,
> > + DRM_FORMAT_RGB888,
> > + DRM_FORMAT_BGR888,
> > + DRM_FORMAT_RGB565,
> > + DRM_FORMAT_BGR565,
> > +
> > + DRM_FORMAT_NV12,
> > + DRM_FORMAT_NV21,
> > + DRM_FORMAT_NV16,
> > + DRM_FORMAT_NV61,
> > + DRM_FORMAT_VYUY,
> > + DRM_FORMAT_UYVY,
> > + DRM_FORMAT_YUYV,
> > + DRM_FORMAT_YVYU,
> > + DRM_FORMAT_YUV420,
> > + DRM_FORMAT_YVU420,
> > +};
> > +
> > /* initialize plane */
> > struct drm_plane *mdp4_plane_init(struct drm_device *dev,
> > enum mdp4_pipe pipe_id, bool private_plane)
> > @@ -379,6 +420,8 @@ struct drm_plane *mdp4_plane_init(struct drm_device
> > *dev,
> > struct mdp4_plane *mdp4_plane;
> > int ret;
> > enum drm_plane_type type;
> > + const uint32_t *formats;
> > + unsigned int nformats;
> >
> > mdp4_plane = kzalloc(sizeof(*mdp4_plane), GFP_KERNEL);
> > if (!mdp4_plane) {
> > @@ -392,13 +435,17 @@ struct drm_plane *mdp4_plane_init(struct drm_device
> > *dev,
> > mdp4_plane->name = pipe_names[pipe_id];
> > mdp4_plane->caps = mdp4_pipe_caps(pipe_id);
> >
> > - mdp4_plane->nformats = mdp_get_formats(mdp4_plane->formats,
> > - ARRAY_SIZE(mdp4_plane->formats),
> > - !pipe_supports_yuv(mdp4_plane->caps));
> > -
> > type = private_plane ? DRM_PLANE_TYPE_PRIMARY :
> > DRM_PLANE_TYPE_OVERLAY;
> > +
> > + if (pipe_supports_yuv(mdp4_plane->caps)) {
> > + formats = mdp4_rgb_yuv_formats;
> > + nformats = ARRAY_SIZE(mdp4_rgb_yuv_formats);
> > + } else {
> > + formats = mdp4_rgb_formats;
> > + nformats = ARRAY_SIZE(mdp4_rgb_formats);
> > + }
> > ret = drm_universal_plane_init(dev, plane, 0xff, _plane_funcs,
> > - mdp4_plane->formats, mdp4_plane->nformats,
> > + formats, nformats,
> >supported_format_modifiers, type, NULL);
> > if (ret)
> > goto fail;
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> > index 0d5ff03cb091..aa8342d93393 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> > @@ -17,9 +17,6 @@
> >
> > struct mdp5_plane {
> > struct drm_plane base;
> > -
> > - uint32_t nformats;
> > - uint32_t formats[32];
> > };
> > #define to_mdp5_plane(x) container_of(x, struct mdp5_plane, base)
> >
> > @@ -1007,6 +1004,32 @@ uint32_t mdp5_plane_get_flush(struct drm_plane
> > *plane)
> > return mask;
> > }
> >
> > +const uint32_t mdp5_plane_formats[] = {
> > + DRM_FORMAT_ARGB,
> > + DRM_FORMAT_ABGR,
> > + DRM_FORMAT_RGBA,
> > + DRM_FORMAT_BGRA,
> > + DRM_FORMAT_XRGB,
> > + DRM_FORMAT_XBGR,
> > + DRM_FORMAT_RGBX,
> > + DRM_FORMAT_BGRX,
> > + DRM_FORMAT_RGB888,
> > + DRM_FORMAT_BGR888,
> > + DRM_FORMAT_RGB565,
> > + DRM_FORMAT_BGR565,
> > +
> > + DRM_FORMAT_NV12,
> > + DRM_FORMAT_NV21,
> > +
Re: [PATCH 04/12] drm/msm: add arrays listing formats supported by MDP4/MDP5 hardware
On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:
MDP4 and MDP5 drivers enumerate supported formats each time the plane is
created. In preparation to merger of MDP DPU format databases, define
precise formats list, so that changes to the database do not cause the
driver to add unsupported format to the list.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 57 --
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 36 +++---
drivers/gpu/drm/msm/disp/mdp_format.c | 28 ---
drivers/gpu/drm/msm/disp/mdp_kms.h | 1 -
4 files changed, 80 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
index b689b618da78..cebe20c82a54 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
@@ -371,6 +371,47 @@ static const uint64_t supported_format_modifiers[] = {
DRM_FORMAT_MOD_INVALID
};
+const uint32_t mdp4_rgb_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+};
+
+const uint32_t mdp4_rgb_yuv_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+};
+
/* initialize plane */
struct drm_plane *mdp4_plane_init(struct drm_device *dev,
enum mdp4_pipe pipe_id, bool private_plane)
@@ -379,6 +420,8 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev,
struct mdp4_plane *mdp4_plane;
int ret;
enum drm_plane_type type;
+ const uint32_t *formats;
+ unsigned int nformats;
mdp4_plane = kzalloc(sizeof(*mdp4_plane), GFP_KERNEL);
if (!mdp4_plane) {
@@ -392,13 +435,17 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev,
mdp4_plane->name = pipe_names[pipe_id];
mdp4_plane->caps = mdp4_pipe_caps(pipe_id);
- mdp4_plane->nformats = mdp_get_formats(mdp4_plane->formats,
- ARRAY_SIZE(mdp4_plane->formats),
- !pipe_supports_yuv(mdp4_plane->caps));
-
type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
+
+ if (pipe_supports_yuv(mdp4_plane->caps)) {
+ formats = mdp4_rgb_yuv_formats;
+ nformats = ARRAY_SIZE(mdp4_rgb_yuv_formats);
+ } else {
+ formats = mdp4_rgb_formats;
+ nformats = ARRAY_SIZE(mdp4_rgb_formats);
+ }
ret = drm_universal_plane_init(dev, plane, 0xff, _plane_funcs,
-mdp4_plane->formats, mdp4_plane->nformats,
+formats, nformats,
supported_format_modifiers, type, NULL);
if (ret)
goto fail;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 0d5ff03cb091..aa8342d93393 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -17,9 +17,6 @@
struct mdp5_plane {
struct drm_plane base;
-
- uint32_t nformats;
- uint32_t formats[32];
};
#define to_mdp5_plane(x) container_of(x, struct mdp5_plane, base)
@@ -1007,6 +1004,32 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane)
return mask;
}
+const uint32_t mdp5_plane_formats[] = {
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+};
+
/* initialize plane */
struct drm_plane *mdp5_plane_init(struct drm_device *dev,
enum drm_plane_type type)
@@ -1023,12 +1046,9 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
Re: [PATCH 03/12] drm/msm/dpu: use format-related definitions from mdp_common.xml.h
On 4/10/2024 7:38 PM, Dmitry Baryshkov wrote: On Thu, 11 Apr 2024 at 02:54, Abhinav Kumar wrote: On 4/10/2024 2:12 PM, Dmitry Baryshkov wrote: On Wed, Apr 10, 2024 at 01:18:42PM -0700, Abhinav Kumar wrote: On 4/10/2024 1:16 PM, Dmitry Baryshkov wrote: On Wed, 10 Apr 2024 at 23:00, Abhinav Kumar wrote: On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote: Instead of having DPU-specific defines, switch to the definitions from the mdp_common.xml.h file. This is the preparation for merged of DPU and MDP format tables. Adding MDP_***__ usages in DPU driver is quite confusing. Can we align to a common naming scheme such as DISP_***? No, it's not something display-generic. It is specific to MDP platforms. In the end DPU is a continuation of the MDP lineup, isn't it? No some aspects of the hw are completely different as you already know between MDP4/MDP5 and DPU. Bringing back MDP usages into DPU does not seem right. MDP4 is different, it's true. But there is a lot of common between MDP5 and DPU. Frakly speaking, I don't see an issue with using the constant that was defined for MDP5 for DPU layer. Especially since we are also going to use mdp_ functions for format handling. All the HW naming etc in the doc has migrated to DPU and in fact it only makes sense to start using DPU for MDP5 as we plan to move mdp5 targets to DPU anyway. Not the other way around. MDP4 remains different. How about MSM_DISP then? I dont get why this is MDP platform specific. Because the term MDP no longer holds true for DPU. I am even looking for future chipsets. We cannot live with MDP5 names. Have to think of generic names for formats. Another point: MDP_ is still frequently used in the DPU driver. See dpu_hwio.h, dpu_hw_catalog.h or dpu_hw_interrupts.c As I wrote in https://patchwork.freedesktop.org/patch/570148/?series=127230=1, lets go ahead with the MDP naming which you have. If we see its not working out later on, please be open to a mass renaming that time. With that expectation set, Reviewed-by: Abhinav Kumar
Re: [PATCH] drm/msm: Fix gen_header.py for older python3 versions
On Fri, Apr 12, 2024 at 05:54:07PM +0100, Jon Hunter wrote: > The gen_header.py script is failing for older versions of python3 such > as python 3.5. Two issues observed with python 3.5 are ... > > 1. Python 3 versions prior to 3.6 do not support the f-string format. > 2. Early python 3 versions do not support the 'required' argument for > the argparse add_subparsers(). > > Fix both of the above so that older versions of python 3 still work. > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
