Re: [PATCH v4 4/4] arm64: dts: qcom: add HDMI nodes for msm8998
On 6/13/24 17:15, Marc Gonzalez wrote:
From: Arnaud Vrac
Port device nodes from vendor code.
Signed-off-by: Arnaud Vrac
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Marc Gonzalez
---
[...]
+
+ hdmi: hdmi-tx@c9a {
+ compatible = "qcom,hdmi-tx-8998";
+ reg = <0x0c9a 0x50c>,
+ <0x0078 0x6220>,
+ <0x0c9e 0x2c>;
+ reg-names = "core_physical",
+ "qfprom_physical",
+ "hdcp_physical";
The way qfprom is accessed (bypassing nvmem APIs) will need to be reworked..
but since we already have it like that on 8996, I'm fine with batch-reworking
these some time in the future..
+
+ interrupt-parent = <>;
+ interrupts = <8>;
+
+ clocks = < MDSS_MDP_CLK>,
Not sure if the MDP core clock is necessary here. Pretty sure it only
powers the display-controller@.. peripheral
+< MDSS_AHB_CLK>,
+< MDSS_HDMI_CLK>,
+< MDSS_HDMI_DP_AHB_CLK>,
+< MDSS_EXTPCLK_CLK>,
+< MDSS_AXI_CLK>,
+< MNOC_AHB_CLK>,
This one is an interconnect clock, drop it
+< MISC_AHB_CLK>;
And please confirm whether this one is necessary
+ clock-names =
+ "mdp_core",
+ "iface",
+ "core",
+ "alt_iface",
+ "extp",
+ "bus",
+ "mnoc",
+ "iface_mmss";
+
+ phys = <_phy>;
+ #sound-dai-cells = <1>;
+
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <_hpd_default
+_ddc_default
+_cec_default>;
+ pinctrl-1 = <_hpd_sleep
+_ddc_default
+_cec_default>;
property
property-names
please
and use <>, <>; for phandle arrays instead of < bar>
(this is a really old dt and we still haven't got around to cleaning
up old junk for style issues..)
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ hdmi_in: endpoint {
+ remote-endpoint =
<_intf3_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ hdmi_out: endpoint {
+ };
+ };
+ };
+ };
+
+ hdmi_phy: hdmi-phy@c9a0600 {
+ compatible = "qcom,hdmi-phy-8998";
+ reg = <0x0c9a0600 0x18b>,
+ <0x0c9a0a00 0x38>,
+ <0x0c9a0c00 0x38>,
+ <0x0c9a0e00 0x38>,
+ <0x0c9a1000 0x38>,
+ <0x0c9a1200 0x0e8>;
+ reg-names = "hdmi_pll",
+ "hdmi_tx_l0",
+ "hdmi_tx_l1",
+ "hdmi_tx_l2",
+ "hdmi_tx_l3",
+ "hdmi_phy";
+
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+
+ clocks = < MDSS_AHB_CLK>,
+< GCC_HDMI_CLKREF_CLK>,
+< RPM_SMD_XO_CLK_SRC>;
+ clock-names = "iface",
+ "ref",
+
Re: [PATCH v3 7/9] drm/msm/dpu: drop _dpu_crtc_check_and_setup_lm_bounds from atomic_begin
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote: The dpu_crtc_atomic_check() already calls the function _dpu_crtc_check_and_setup_lm_bounds(). There is no need to call it again from dpu_crtc_atomic_begin(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 -- 1 file changed, 2 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH v3 5/9] drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check
On 6/13/2024 3:36 PM, 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(-) Did anything change between v2 to v3 that R-b was dropped? Here it is again, Reviewed-by: Abhinav Kumar
Re: [PATCH v3 9/9] drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote: Lift mode_config limits set by the DPU driver to the actual FB limits as handled by the dpu_plane.c. Move 2*max_lm_width check where it belongs, to the drm_crtc_helper_funcs::mode_valid() callback. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++--- 2 files changed, 17 insertions(+), 7 deletions(-) Did anything change in this patch from v2 that the R-b was dropped? Here it is again, Reviewed-by: Abhinav Kumar
Re: [PATCH v3 2/9] drm/msm/dpu: drop dpu_format_check_modified_format
On 6/13/2024 3:36 PM, 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 | 43 - drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 16 --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 - drivers/gpu/drm/msm/msm_kms.h | 6 4 files changed, 66 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH v3 1/9] drm/msm/dpu: check for overflow in _dpu_crtc_setup_lm_bounds()
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote:
Make _dpu_crtc_setup_lm_bounds() check that CRTC width is not
overflowing LM requirements. Rename the function accordingly.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
Reviewed-by: Abhinav Kumar
[PATCH v3 6/9] drm/msm/dpu: check for the plane pitch overflow
Check that the plane pitch doesn't overflow the maximum pitch size
allowed by the hardware.
Reviewed-by: Abhinav Kumar
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 4a910b808687..8998d1862e16 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
+
/**
* Flags
*/
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 927fde2f1391..b5848fae14ce 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -791,7 +791,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
{
struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state,
plane);
- int ret = 0, min_scale;
+ int i, ret = 0, min_scale;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_kms *kms = _dpu_plane_get_kms(>base);
u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate;
@@ -865,6 +865,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return ret;
}
+ for (i = 0; i < pstate->layout.num_planes; i++)
+ if (pstate->layout.plane_pitch[i] > DPU_SSPP_MAX_PITCH_SIZE)
+ return -E2BIG;
+
fmt = msm_framebuffer_format(new_plane_state->fb);
max_linewidth = pdpu->catalog->caps->max_linewidth;
--
2.39.2
[PATCH v3 9/9] drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c
Lift mode_config limits set by the DPU driver to the actual FB limits as
handled by the dpu_plane.c. Move 2*max_lm_width check where it belongs,
to the drm_crtc_helper_funcs::mode_valid() callback.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++---
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 5dbf5254d310..44531666edf2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1236,6 +1236,20 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
return 0;
}
+static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
+ const struct drm_display_mode
*mode)
+{
+ struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
+
+ /*
+* max crtc width is equal to the max mixer width * 2 and max height is
+* is 4K
+*/
+ return drm_mode_validate_size(mode,
+ 2 *
dpu_kms->catalog->caps->max_mixer_width,
+ 4096);
+}
+
int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
{
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
@@ -1451,6 +1465,7 @@ static const struct drm_crtc_helper_funcs
dpu_crtc_helper_funcs = {
.atomic_check = dpu_crtc_atomic_check,
.atomic_begin = dpu_crtc_atomic_begin,
.atomic_flush = dpu_crtc_atomic_flush,
+ .mode_valid = dpu_crtc_mode_valid,
.get_scanout_position = dpu_crtc_get_scanout_position,
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 0d1dcc94455c..d1b937e127b0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1147,13 +1147,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;
dev->max_vblank_count = 0x;
/* Disable vblank irqs aggressively for power-saving */
--
2.39.2
[PATCH v3 7/9] drm/msm/dpu: drop _dpu_crtc_check_and_setup_lm_bounds from atomic_begin
The dpu_crtc_atomic_check() already calls the function
_dpu_crtc_check_and_setup_lm_bounds(). There is no need to call it
again from dpu_crtc_atomic_begin().
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index b0d81e8ea765..5dbf5254d310 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -809,8 +809,6 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
DRM_DEBUG_ATOMIC("crtc%d\n", crtc->base.id);
- _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc->state);
-
/* encoder will trigger pending mask now */
drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
dpu_encoder_trigger_kickoff_pending(encoder);
--
2.39.2
[PATCH v3 8/9] drm/msm/dpu: merge MAX_IMG_WIDTH/HEIGHT with DPU_MAX_IMG_WIDTH/HEIGHT
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.
Reviewed-by: Abhinav Kumar
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(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index c6485cb6f1d2..6d7c4373bf84 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -13,9 +13,6 @@
#define DPU_UBWC_PLANE_SIZE_ALIGNMENT 4096
-#define DPU_MAX_IMG_WIDTH 0x3FFF
-#define DPU_MAX_IMG_HEIGHT 0x3FFF
-
/*
* struct dpu_media_color_map - maps drm format to media format
* @format: DRM base pixel format
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index d1aef778340b..af2ead1c4886 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -21,8 +21,8 @@
#define DPU_HW_BLK_NAME_LEN16
-#define MAX_IMG_WIDTH 0x3fff
-#define MAX_IMG_HEIGHT 0x3fff
+#define DPU_MAX_IMG_WIDTH 0x3fff
+#define DPU_MAX_IMG_HEIGHT 0x3fff
#define CRTC_DUAL_MIXERS 2
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index b5848fae14ce..6000e84598c2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -852,8 +852,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
fb_rect.y2 = new_plane_state->fb->height;
/* Ensure fb size is supported */
- if (drm_rect_width(_rect) > MAX_IMG_WIDTH ||
- drm_rect_height(_rect) > MAX_IMG_HEIGHT) {
+ if (drm_rect_width(_rect) > DPU_MAX_IMG_WIDTH ||
+ drm_rect_height(_rect) > DPU_MAX_IMG_HEIGHT) {
DPU_DEBUG_PLANE(pdpu, "invalid framebuffer " DRM_RECT_FMT "\n",
DRM_RECT_ARG(_rect));
return -E2BIG;
--
2.39.2
[PATCH v3 1/9] drm/msm/dpu: check for overflow in _dpu_crtc_setup_lm_bounds()
Make _dpu_crtc_setup_lm_bounds() check that CRTC width is not
overflowing LM requirements. Rename the function accordingly.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9f2164782844..b0d81e8ea765 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -711,12 +711,13 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc)
_dpu_crtc_complete_flip(crtc);
}
-static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
+static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
struct dpu_crtc_state *cstate = to_dpu_crtc_state(state);
struct drm_display_mode *adj_mode = >adjusted_mode;
u32 crtc_split_width = adj_mode->hdisplay / cstate->num_mixers;
+ struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
int i;
for (i = 0; i < cstate->num_mixers; i++) {
@@ -727,7 +728,12 @@ static void _dpu_crtc_setup_lm_bounds(struct drm_crtc
*crtc,
r->y2 = adj_mode->vdisplay;
trace_dpu_crtc_setup_lm_bounds(DRMID(crtc), i, r);
+
+ if (drm_rect_width(r) > dpu_kms->catalog->caps->max_mixer_width)
+ return -E2BIG;
}
+
+ return 0;
}
static void _dpu_crtc_get_pcc_coeff(struct drm_crtc_state *state,
@@ -803,7 +809,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
DRM_DEBUG_ATOMIC("crtc%d\n", crtc->base.id);
- _dpu_crtc_setup_lm_bounds(crtc, crtc->state);
+ _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc->state);
/* encoder will trigger pending mask now */
drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
@@ -1197,8 +1203,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
if (crtc_state->active_changed)
crtc_state->mode_changed = true;
- if (cstate->num_mixers)
- _dpu_crtc_setup_lm_bounds(crtc, crtc_state);
+ if (cstate->num_mixers) {
+ rc = _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc_state);
+ if (rc)
+ return rc;
+ }
/* FIXME: move this to dpu_plane_atomic_check? */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
--
2.39.2
[PATCH v3 5/9] drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check
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 a57853ac70b1..927fde2f1391 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -674,12 +674,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,
@@ -865,6 +859,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;
+ }
+
fmt = msm_framebuffer_format(new_plane_state->fb);
max_linewidth = pdpu->catalog->caps->max_linewidth;
--
2.39.2
[PATCH v3 2/9] drm/msm/dpu: drop dpu_format_check_modified_format
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 | 43 -
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 16 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 -
drivers/gpu/drm/msm/msm_kms.h | 6
4 files changed, 66 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 6b1e9a617da3..027eb5ecff08 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -423,46 +423,3 @@ int dpu_format_populate_layout(
return ret;
}
-
-int dpu_format_check_modified_format(
- const struct msm_kms *kms,
- const struct msm_format *fmt,
- const struct drm_mode_fb_cmd2 *cmd,
- struct drm_gem_object **bos)
-{
- const struct drm_format_info *info;
- struct dpu_hw_fmt_layout layout;
- uint32_t bos_total_size = 0;
- int ret, i;
-
- if (!fmt || !cmd || !bos) {
- DRM_ERROR("invalid arguments\n");
- return -EINVAL;
- }
-
- info = drm_format_info(fmt->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;
-}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
index 210d0ed5f0af..ef1239c95058 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
@@ -31,22 +31,6 @@ static inline bool dpu_find_format(u32 format, const u32
*supported_formats,
return false;
}
-/**
- * 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 msm_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
* mmu, fb, and format found in the fb
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1955848b1b78..0d1dcc94455c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -981,7 +981,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,
.destroy = dpu_kms_destroy,
.snapshot= dpu_kms_mdp_snapshot,
#ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 1e0c54de3716..e60162744c66 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -92,12 +92,6 @@ struct msm_kms_funcs {
* Format handling:
*/
- /* 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,
- const struct drm_mode_fb_cmd2 *cmd,
- struct drm_gem_object **bos);
-
/* misc: */
long (*round_pixclk)(struct msm_kms *kms, unsigned long rate,
struct drm_encoder *encoder);
--
2.39.2
[PATCH v3 4/9] drm/msm/dpu: split dpu_format_populate_layout
Split dpu_format_populate_layout() into addess-related and
pitch/format-related parts.
Reviewed-by: Abhinav Kumar
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| 45 --
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 8 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 --
4 files changed, 46 insertions(+), 27 deletions(-)
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 d3ea91c1d7d2..ccf2d030cf20 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
@@ -584,7 +584,13 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
dpu_encoder_phys *phys_enc
return;
}
- ret = dpu_format_populate_layout(aspace, job->fb, _cfg->dest);
+ ret = dpu_format_populate_plane_sizes(job->fb, _cfg->dest);
+ if (ret) {
+ DPU_DEBUG("failed to populate plane sizes%d\n", ret);
+ return;
+ }
+
+ ret = dpu_format_populate_addrs(aspace, job->fb, _cfg->dest);
if (ret) {
DPU_DEBUG("failed to populate layout %d\n", ret);
return;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 027eb5ecff08..c6485cb6f1d2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -93,7 +93,7 @@ static int _dpu_format_get_media_color_ubwc(const struct
msm_format *fmt)
return color_fmt;
}
-static int _dpu_format_get_plane_sizes_ubwc(
+static int _dpu_format_populate_plane_sizes_ubwc(
const struct msm_format *fmt,
const uint32_t width,
const uint32_t height,
@@ -172,7 +172,7 @@ static int _dpu_format_get_plane_sizes_ubwc(
return 0;
}
-static int _dpu_format_get_plane_sizes_linear(
+static int _dpu_format_populate_plane_sizes_linear(
const struct msm_format *fmt,
const uint32_t width,
const uint32_t height,
@@ -244,27 +244,38 @@ static int _dpu_format_get_plane_sizes_linear(
return 0;
}
-static int dpu_format_get_plane_sizes(
- const struct msm_format *fmt,
- const uint32_t w,
- const uint32_t h,
- struct dpu_hw_fmt_layout *layout,
- const uint32_t *pitches)
+/*
+ * dpu_format_populate_addrs - populate non-address part of the layout based on
+ * fb, and format found in the fb
+ * @fb:framebuffer pointer
+ * @layout: format layout structure to populate
+ *
+ * Return: error code on failure or 0 if new addresses were populated
+ */
+int dpu_format_populate_plane_sizes(
+ struct drm_framebuffer *fb,
+ struct dpu_hw_fmt_layout *layout)
{
- if (!layout || !fmt) {
+ const struct msm_format *fmt;
+
+ if (!layout || !fb) {
DRM_ERROR("invalid pointer\n");
return -EINVAL;
}
- if ((w > DPU_MAX_IMG_WIDTH) || (h > DPU_MAX_IMG_HEIGHT)) {
+ if (fb->width > DPU_MAX_IMG_WIDTH ||
+ fb->height > DPU_MAX_IMG_HEIGHT) {
DRM_ERROR("image dimensions outside max range\n");
return -ERANGE;
}
+ fmt = msm_framebuffer_format(fb);
+
if (MSM_FORMAT_IS_UBWC(fmt) || MSM_FORMAT_IS_TILE(fmt))
- return _dpu_format_get_plane_sizes_ubwc(fmt, w, h, layout);
+ return _dpu_format_populate_plane_sizes_ubwc(fmt, fb->width,
fb->height, layout);
- return _dpu_format_get_plane_sizes_linear(fmt, w, h, layout, pitches);
+ return _dpu_format_populate_plane_sizes_linear(fmt, fb->width,
fb->height,
+ layout, fb->pitches);
}
static int _dpu_format_populate_addrs_ubwc(
@@ -388,7 +399,7 @@ static int _dpu_format_populate_addrs_linear(
return 0;
}
-int dpu_format_populate_layout(
+int dpu_format_populate_addrs(
struct msm_gem_address_space *aspace,
struct drm_framebuffer *fb,
struct dpu_hw_fmt_layout *layout)
@@ -406,14 +417,6 @@ int dpu_format_populate_layout(
return -ERANGE;
}
- layout->format = msm_framebuffer_format(fb);
-
- /* Populate the plane sizes etc via get_format */
- ret = dpu_format_get_plane_sizes(layout->format, fb->width, fb->height,
- layout, fb->pitches);
- if (ret)
- return ret;
-
/* Populate the addresses given the fb */
if (MSM_FORMAT_IS_UBWC(layout->format) ||
MSM_FORMAT_IS_TILE(layout->format))
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
[PATCH v3 3/9] drm/msm/dpu: drop dpu_format_populate_layout from dpu_plane_sspp_atomic_update
The dpu_plane_prepare_fb() already calls dpu_format_populate_layout().
Store the generated layout in the plane state and drop this call from
dpu_plane_sspp_update().
Reviewed-by: Abhinav Kumar
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(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 1c3a2657450c..9ee178a09a3b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -647,7 +647,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
struct drm_framebuffer *fb = new_state->fb;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_state);
- struct dpu_hw_fmt_layout layout;
struct dpu_kms *kms = _dpu_plane_get_kms(>base);
int ret;
@@ -677,7 +676,8 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
/* validate framebuffer layout before commit */
ret = dpu_format_populate_layout(pstate->aspace,
- new_state->fb, );
+new_state->fb,
+>layout);
if (ret) {
DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
return ret;
@@ -1100,17 +1100,6 @@ static void dpu_plane_sspp_atomic_update(struct
drm_plane *plane)
msm_framebuffer_format(fb);
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = >r_pipe_cfg;
- struct dpu_kms *kms = _dpu_plane_get_kms(>base);
- struct msm_gem_address_space *aspace = kms->base.aspace;
- struct dpu_hw_fmt_layout layout;
- bool layout_valid = false;
- int ret;
-
- ret = dpu_format_populate_layout(aspace, fb, );
- if (ret)
- DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
- else
- layout_valid = true;
pstate->pending = true;
@@ -1125,12 +1114,12 @@ static void dpu_plane_sspp_atomic_update(struct
drm_plane *plane)
dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt,
drm_mode_vrefresh(>mode),
- layout_valid ? : NULL);
+ >layout);
if (r_pipe->sspp) {
dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt,
drm_mode_vrefresh(>mode),
- layout_valid ? : NULL);
+ >layout);
}
if (pstate->needs_qos_remap)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index abd6b21a049b..348b0075d1ce 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -31,6 +31,7 @@
* @plane_clk: calculated clk per plane
* @needs_dirtyfb: whether attached CRTC needs pixel data explicitly flushed
* @rotation: simplified drm rotation hint
+ * @layout: framebuffer memory layout
*/
struct dpu_plane_state {
struct drm_plane_state base;
@@ -48,6 +49,8 @@ struct dpu_plane_state {
bool needs_dirtyfb;
unsigned int rotation;
+
+ struct dpu_hw_fmt_layout layout;
};
#define to_dpu_plane_state(x) \
--
2.39.2
[PATCH v3 0/9] drm/msm/dpu: be more friendly to X.org
Unlike other compositors X.org allocates a single framebuffer covering the whole screen space. This is not an issue with the single screens, but with the multi-monitor setup 5120x4096 becomes a limiting factor. Check the hardware-bound limitations and lift the FB max size to 16383x16383. Signed-off-by: Dmitry Baryshkov --- Changes in v3: - Reoder the functions to pull up a fix to the start of the patchset (Abhinav) - Rename the _dpu_crtc_setup_lm_bounds() to _dpu_crtc_check_and_setup_lm_bounds() (Abhinav) - Make dpu_crtc_mode_valid() static. - Link to v2: https://lore.kernel.org/r/20240603-dpu-mode-config-width-v2-0-16af52057...@linaro.org Changes in v2: - Added dpu_crtc_valid() to verify that 2*lm_width limit is enforced (Abhinav) - Link to v1: https://lore.kernel.org/r/20240319-dpu-mode-config-width-v1-0-d0fe6bf81...@linaro.org --- Dmitry Baryshkov (9): drm/msm/dpu: check for overflow in _dpu_crtc_setup_lm_bounds() drm/msm/dpu: drop dpu_format_check_modified_format drm/msm/dpu: drop dpu_format_populate_layout from dpu_plane_sspp_atomic_update drm/msm/dpu: split dpu_format_populate_layout drm/msm/dpu: move dpu_format_populate_plane_sizes to atomic_check drm/msm/dpu: check for the plane pitch overflow drm/msm/dpu: drop _dpu_crtc_check_and_setup_lm_bounds from atomic_begin drm/msm/dpu: merge MAX_IMG_WIDTH/HEIGHT with DPU_MAX_IMG_WIDTH/HEIGHT drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 32 ++-- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 91 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 24 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h| 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 10 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 37 + drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 3 + drivers/gpu/drm/msm/msm_kms.h | 6 -- 10 files changed, 91 insertions(+), 126 deletions(-) --- base-commit: 03d44168cbd7fc57d5de56a3730427db758fc7f6 change-id: 20240318-dpu-mode-config-width-626d3c7ad52a Best regards, -- Dmitry Baryshkov
Re: [PATCH v2 7/8] drm/msm/dpu: support setting the TE source
On 2024-06-13 20:05:10, Dmitry Baryshkov wrote:
> Make the DPU driver use the TE source specified in the DT. If none is
> specified, the driver defaults to the first GPIO (mdp_vsync0).
mdp_vsync_p?
>
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 44
> -
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index e9991f3756d4..6fcb3cf4a382 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -505,6 +505,44 @@ static void dpu_kms_wait_flush(struct msm_kms *kms,
> unsigned crtc_mask)
> dpu_kms_wait_for_commit_done(kms, crtc);
> }
>
> +static const char *dpu_vsync_sources[] = {
> + [DPU_VSYNC_SOURCE_GPIO_0] = "mdp_vsync_p",
> + [DPU_VSYNC_SOURCE_GPIO_1] = "mdp_vsync_s",
> + [DPU_VSYNC_SOURCE_GPIO_2] = "mdp_vsync_e",
> + [DPU_VSYNC_SOURCE_INTF_0] = "mdp_intf0",
> + [DPU_VSYNC_SOURCE_INTF_1] = "mdp_intf1",
> + [DPU_VSYNC_SOURCE_INTF_2] = "mdp_intf2",
> + [DPU_VSYNC_SOURCE_INTF_3] = "mdp_intf3",
> + [DPU_VSYNC_SOURCE_WD_TIMER_0] = "timer0",
> + [DPU_VSYNC_SOURCE_WD_TIMER_1] = "timer1",
> + [DPU_VSYNC_SOURCE_WD_TIMER_2] = "timer2",
> + [DPU_VSYNC_SOURCE_WD_TIMER_3] = "timer3",
> + [DPU_VSYNC_SOURCE_WD_TIMER_4] = "timer4",
> +};
> +
> +static int dpu_kms_dsi_set_te_source(struct msm_display_info *info,
> + struct msm_dsi *dsi)
> +{
> + const char *te_source = msm_dsi_get_te_source(dsi);
Just checking: if the TE source is different and one has dual-DSI, it must be
set on both controllers?
> + int i;
> +
> + if (!te_source) {
> + info->vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
> + return 0;
> + }
> +
> + /* we can not use match_string since dpu_vsync_sources is a sparse
> array */
Instead of having gaps in the array, you could also store both the vsync_source
and name as the array elements?
> + for (i = 0; i < ARRAY_SIZE(dpu_vsync_sources); i++) {
> + if (dpu_vsync_sources[i] &&
> + !strcmp(dpu_vsync_sources[i], te_source)) {
> + info->vsync_source = i;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> static int _dpu_kms_initialize_dsi(struct drm_device *dev,
> struct msm_drm_private *priv,
> struct dpu_kms *dpu_kms)
> @@ -543,7 +581,11 @@ static int _dpu_kms_initialize_dsi(struct drm_device
> *dev,
>
> info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
>
> - info.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
> + rc = dpu_kms_dsi_set_te_source(, priv->dsi[i]);
> + if (rc) {
> + DPU_ERROR("failed to identify TE source for dsi
> display\n");
> + return rc;
> + }
>
> encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI, );
> if (IS_ERR(encoder)) {
>
> --
> 2.39.2
>
Re: [PATCH] drm/ci: mark kms_addfb_basic@addfb25-bad-modifier as passing on msm
On Thu, 13 Jun 2024 at 20:49, Abhinav Kumar wrote:
>
>
>
> On 6/13/2024 9:33 AM, Dmitry Baryshkov wrote:
> > The commit b228501ff183 ("drm/msm: merge dpu format database to MDP
> > formats") made get_format take modifiers into account. This makes
> > kms_addfb_basic@addfb25-bad-modifier pass on MDP4 and MDP5 platforms.
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt | 1 -
> > drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt | 1 -
> > 2 files changed, 2 deletions(-)
> >
>
> Would be good to also give a link to the CI for the CI maintainers.
>
> But otherwise, LGTM
>
> Reviewed-by: Abhinav Kumar
Yes, good idea: https://gitlab.freedesktop.org/drm/msm/-/merge_requests/119
>
>
> > diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
> > b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
> > index 3dfbabdf905e..6e7fd1ccd1e3 100644
> > --- a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
> > +++ b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
> > @@ -4,7 +4,6 @@ device_reset@unbind-cold-reset-rebind,Fail
> > device_reset@unbind-reset-rebind,Fail
> > dumb_buffer@invalid-bpp,Fail
> > kms_3d,Fail
> > -kms_addfb_basic@addfb25-bad-modifier,Fail
> > kms_cursor_legacy@forked-move,Fail
> > kms_cursor_legacy@single-bo,Fail
> > kms_cursor_legacy@torture-bo,Fail
> > diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
> > b/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
> > index 23a5f6f9097f..46ca69ce2ffe 100644
> > --- a/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
> > +++ b/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
> > @@ -4,6 +4,5 @@ device_reset@unbind-cold-reset-rebind,Fail
> > device_reset@unbind-reset-rebind,Fail
> > dumb_buffer@invalid-bpp,Fail
> > kms_3d,Fail
> > -kms_addfb_basic@addfb25-bad-modifier,Fail
> > kms_lease@lease-uevent,Fail
> > tools_test@tools_test,Fail
> >
> > ---
> > base-commit: 6b4468b0c6ba37a16795da567b58dc80bc7fb439
> > change-id: 20240613-msm-pass-addfb25-bad-modifier-c461fd9c02bb
> >
> > Best regards,
--
With best wishes
Dmitry
Re: [PATCH v2 2/8] drm/msm/dpu: convert vsync source defines to the enum
On Thu, 13 Jun 2024 at 21:17, Marijn Suijten
wrote:
>
> On 2024-06-13 20:05:05, Dmitry Baryshkov wrote:
> > Add enum dpu_vsync_source instead of a series of defines. Use this enum
> > to pass vsync information.
> >
> > Reviewed-by: Abhinav Kumar
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +-
> > 5 files changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 119f3ea50a7c..4988a1029431 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -747,7 +747,7 @@ static void _dpu_encoder_update_vsync_source(struct
> > dpu_encoder_virt *dpu_enc,
> > if (disp_info->is_te_using_watchdog_timer)
> > vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
> > else
> > - vsync_cfg.vsync_source = DPU_VSYNC0_SOURCE_GPIO;
> > + vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
> >
> > hw_mdptop->ops.setup_vsync_source(hw_mdptop, _cfg);
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > index 225c1c7768ff..96f6160cf607 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > @@ -462,7 +462,7 @@ static int dpu_hw_intf_get_vsync_info(struct
> > dpu_hw_intf *intf,
> > }
> >
> > static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
> > - u32 vsync_source)
> > + enum dpu_vsync_source vsync_source)
> > {
> > struct dpu_hw_blk_reg_map *c;
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> > index f9015c67a574..ac244f0b33fb 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> > @@ -107,7 +107,7 @@ struct dpu_hw_intf_ops {
> >
> > int (*connect_external_te)(struct dpu_hw_intf *intf, bool
> > enable_external_te);
> >
> > - void (*vsync_sel)(struct dpu_hw_intf *intf, u32 vsync_source);
> > + void (*vsync_sel)(struct dpu_hw_intf *intf, enum dpu_vsync_source
> > vsync_source);
> >
> > /**
> >* Disable autorefresh if enabled
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > index 66759623fc42..a2eff36a2224 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > @@ -54,18 +54,20 @@
> > #define DPU_BLEND_BG_INV_MOD_ALPHA (1 << 12)
> > #define DPU_BLEND_BG_TRANSP_EN (1 << 13)
> >
> > -#define DPU_VSYNC0_SOURCE_GPIO 0
> > -#define DPU_VSYNC1_SOURCE_GPIO 1
> > -#define DPU_VSYNC2_SOURCE_GPIO 2
> > -#define DPU_VSYNC_SOURCE_INTF_0 3
> > -#define DPU_VSYNC_SOURCE_INTF_1 4
> > -#define DPU_VSYNC_SOURCE_INTF_2 5
> > -#define DPU_VSYNC_SOURCE_INTF_3 6
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_4 11
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_3 12
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_2 13
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_1 14
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_0 15
> > +enum dpu_vsync_source {
> > + DPU_VSYNC_SOURCE_GPIO_0,
> > + DPU_VSYNC_SOURCE_GPIO_1,
> > + DPU_VSYNC_SOURCE_GPIO_2,
>
> While at it, rename this to _P / _S / _E to match the other patches/code?
I thought about it, but Abhinav pointed out that DPU docs use this
kind of naming.
>
> - Marijn
>
> > + DPU_VSYNC_SOURCE_INTF_0 = 3,
> > + DPU_VSYNC_SOURCE_INTF_1,
> > + DPU_VSYNC_SOURCE_INTF_2,
> > + DPU_VSYNC_SOURCE_INTF_3,
> > + DPU_VSYNC_SOURCE_WD_TIMER_4 = 11,
> > + DPU_VSYNC_SOURCE_WD_TIMER_3,
> > + DPU_VSYNC_SOURCE_WD_TIMER_2,
> > + DPU_VSYNC_SOURCE_WD_TIMER_1,
> > + DPU_VSYNC_SOURCE_WD_TIMER_0,
> > +};
> >
> > enum dpu_hw_blk_type {
> > DPU_HW_BLK_TOP = 0,
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> > index 6f3dc98087df..5c9a7ede991e 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> > @@ -64,7 +64,7 @@ struct dpu_vsync_source_cfg {
> > u32 pp_count;
> > u32 frame_rate;
> > u32 ppnumber[PINGPONG_MAX];
> > - u32 vsync_source;
> > + enum dpu_vsync_source vsync_source;
> > };
> >
> > /**
> >
> > --
> > 2.39.2
> >
--
With best wishes
Dmitry
Re: [PATCH v2 1/8] dt-bindings: display/msm/dsi: allow specifying TE source
On 2024-06-13 20:05:04, Dmitry Baryshkov wrote:
> Command mode panels provide TE signal back to the DSI host to signal
> that the frame display has completed and update of the image will not
> cause tearing. Usually it is connected to the first GPIO with the
> mdp_vsync function, which is the default. In such case the property can
> be skipped.
>
> Acked-by: Krzysztof Kozlowski
> Reviewed-by: Rob Herring (Arm)
> Signed-off-by: Dmitry Baryshkov
> ---
> .../bindings/display/msm/dsi-controller-main.yaml | 17
> +
> 1 file changed, 17 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index 1fa28e976559..e1cb3a1fee81 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -162,6 +162,22 @@ properties:
> items:
>enum: [ 0, 1, 2, 3 ]
>
> + qcom,te-source:
> +$ref: /schemas/types.yaml#/definitions/string
> +description:
> + Specifies the source of vsync signal from the panel used
> for
> + tearing elimination.
> +default: mdp_vsync_p
> +enum:
> + - mdp_vsync_p
> + - mdp_vsync_s
> + - mdp_vsync_e
When discussing that these should be renamed, was it also documented what the
suffix means? I can only guess something like primary/secondary/e...?
Are the mdp_intfX variants missing here that you're handling in patch 7/8?
> + - timer0
> + - timer1
> + - timer2
> + - timer3
> + - timer4
> +
> required:
>- port@0
>- port@1
> @@ -452,6 +468,7 @@ examples:
>dsi0_out: endpoint {
> remote-endpoint = <_in>;
> data-lanes = <0 1 2 3>;
> + qcom,te-source = "mdp_vsync_e";
>};
>};
> };
>
> --
> 2.39.2
>
Re: [PATCH v2 8/8] drm/msm/dpu: rename dpu_hw_setup_vsync_source functions
On 2024-06-13 20:05:11, Dmitry Baryshkov wrote:
> Rename dpu_hw_setup_vsync_source functions to make the names match the
> implementation: on DPU 5.x the TOP only contains timer setup, while 3.x
> and 4.x used MDP_VSYNC_SEL register to select TE source.
Yeah that was never really clear anymore after I split this function in two in
commit a2ff096803b3 ("drm/msm/dpu: Disable MDP vsync source selection on DPU
5.0.0 and above").
>
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> index 05e48cf4ec1d..6e2ac50b94a4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> @@ -107,8 +107,8 @@ static void dpu_hw_get_danger_status(struct dpu_hw_mdp
> *mdp,
> status->sspp[SSPP_CURSOR1] = (value >> 26) & 0x3;
> }
>
> -static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp *mdp,
> - struct dpu_vsync_source_cfg *cfg)
> +static void dpu_hw_setup_wd_timer(struct dpu_hw_mdp *mdp,
> + struct dpu_vsync_source_cfg *cfg)
> {
> struct dpu_hw_blk_reg_map *c;
> u32 reg, wd_load_value, wd_ctl, wd_ctl2;
> @@ -163,8 +163,8 @@ static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp
> *mdp,
> }
> }
>
> -static void dpu_hw_setup_vsync_source_and_vsync_sel(struct dpu_hw_mdp *mdp,
> - struct dpu_vsync_source_cfg *cfg)
> +static void dpu_hw_setup_vsync_sel(struct dpu_hw_mdp *mdp,
Maybe keep setup_wd_timer_and_vsync_sel()? OTOH it doesn't always set wd_timer,
only when vsync_source calls for it.
> +struct dpu_vsync_source_cfg *cfg)
> {
> struct dpu_hw_blk_reg_map *c;
> u32 reg, i;
> @@ -187,7 +187,7 @@ static void
> dpu_hw_setup_vsync_source_and_vsync_sel(struct dpu_hw_mdp *mdp,
> }
> DPU_REG_WRITE(c, MDP_VSYNC_SEL, reg);
>
> - dpu_hw_setup_vsync_source(mdp, cfg);
> + dpu_hw_setup_wd_timer(mdp, cfg);
> }
>
> static void dpu_hw_get_safe_status(struct dpu_hw_mdp *mdp,
> @@ -239,9 +239,9 @@ static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops,
> ops->get_danger_status = dpu_hw_get_danger_status;
>
> if (cap & BIT(DPU_MDP_VSYNC_SEL))
> - ops->setup_vsync_source =
> dpu_hw_setup_vsync_source_and_vsync_sel;
> + ops->setup_vsync_source = dpu_hw_setup_vsync_sel;
> else
> - ops->setup_vsync_source = dpu_hw_setup_vsync_source;
> + ops->setup_vsync_source = dpu_hw_setup_wd_timer;
Should the callback also be renamed - and the docs improved? Seems the
vsync_sel register is used to selsect a vsync_source (plus some other bits like
the pingpong block).
- Marijn
>
> ops->get_safe_status = dpu_hw_get_safe_status;
>
>
> --
> 2.39.2
>
Re: [PATCH v4 10/13] drm/msm/dpu: allow sharing SSPP between planes
On 6/13/2024 3:05 AM, Dmitry Baryshkov wrote:
On Wed, Jun 12, 2024 at 06:17:37PM -0700, Abhinav Kumar wrote:
On 6/12/2024 2:08 AM, Dmitry Baryshkov wrote:
On Wed, 12 Jun 2024 at 02:12, Abhinav Kumar wrote:
On 3/13/2024 5:02 PM, Dmitry Baryshkov wrote:
Since SmartDMA planes provide two rectangles, it is possible to use them
to drive two different DRM planes, first plane getting the rect_0,
another one using rect_1 of the same SSPP. The sharing algorithm is
pretty simple, it requires that each of the planes can be driven by the
single rectangle and only consequetive planes are considered.
consequetive - > consecutive
Can you please explain why only consecutive planes are considered for this?
So lets say we have 4 virtual planes : 0, 1, 2, 3
It will try 0-1, 1-2, 2-3
Because all planes are virtual, there are only 3 unique pairs to be
considered? Otherwise technically 6 pairs are possible.
An implementation that tries all 6 pairs taking the zpos and the
overlapping into account is appreciated. I cared for the simplest case
here. Yes, further optimizations can be implemented.
Ok got it. So you would like to build a better one on top of this.
But I see one case where this has an issue or is not optimal. Pls see below.
Yes, it is not optimal. This is the 'best possible effort' or 'best
simple effort' from my POV.
General request:
Patches 1-9 : Add support for using 2 SSPPs in one plane
Patches 10-12 : Add support for using two rectangles of the same SSPP as
two virtual planes
Patch 13 : Can be pushed along with the first set.
Can we break up this series in this way to make it easier to test and
land the bulk of it in this cycle?
Sure.
Thanks.
I have some doubts on patches 10-12 and would like to spend more time
reviewing and testing this. So I am trying to reduce the debt of patches
we have been carrying as this is a tricky feature to simulate and test
the cases.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 128 +++---
1 file changed, 112 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index cde20c1fa90d..2e1c544efc4a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -886,10 +886,9 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane
*plane,
return 0;
}
-static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
-struct dpu_sw_pipe_cfg
*pipe_cfg,
-const struct dpu_format *fmt,
-uint32_t max_linewidth)
+static int dpu_plane_is_multirect_capable(struct dpu_sw_pipe *pipe,
+ struct dpu_sw_pipe_cfg *pipe_cfg,
+ const struct dpu_format *fmt)
{
if (drm_rect_width(_cfg->src_rect) !=
drm_rect_width(_cfg->dst_rect) ||
drm_rect_height(_cfg->src_rect) !=
drm_rect_height(_cfg->dst_rect))
@@ -901,6 +900,13 @@ static int dpu_plane_is_multirect_parallel_capable(struct
dpu_sw_pipe *pipe,
if (DPU_FORMAT_IS_YUV(fmt))
return false;
+ return true;
+}
+
+static int dpu_plane_is_parallel_capable(struct dpu_sw_pipe_cfg *pipe_cfg,
+ const struct dpu_format *fmt,
+ uint32_t max_linewidth)
+{
if (DPU_FORMAT_IS_UBWC(fmt) &&
drm_rect_width(_cfg->src_rect) > max_linewidth / 2)
return false;
@@ -908,6 +914,82 @@ static int dpu_plane_is_multirect_parallel_capable(struct
dpu_sw_pipe *pipe,
return true;
}
+static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
+struct dpu_sw_pipe_cfg
*pipe_cfg,
+const struct dpu_format *fmt,
+uint32_t max_linewidth)
+{
+ return dpu_plane_is_multirect_capable(pipe, pipe_cfg, fmt) &&
+ dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth);
+}
+
+
+static int dpu_plane_try_multirect(struct dpu_plane_state *pstate,
+struct dpu_plane_state *prev_pstate,
+const struct dpu_format *fmt,
+uint32_t max_linewidth)
+{
+ struct dpu_sw_pipe *pipe = >pipe;
+ struct dpu_sw_pipe *r_pipe = >r_pipe;
+ struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
+ struct dpu_sw_pipe *prev_pipe = _pstate->pipe;
+ struct dpu_sw_pipe_cfg *prev_pipe_cfg = _pstate->pipe_cfg;
+ const struct dpu_format *prev_fmt =
+ to_dpu_format(msm_framebuffer_format(prev_pstate->base.fb));
+ u16 max_tile_height = 1;
+
+ if (prev_pstate->r_pipe.sspp != NULL ||
+
[PATCH v2 6/8] drm/msm/dsi: parse vsync source from device tree
Allow board's device tree to specify the vsync source (aka TE source).
If the property is omitted, the display controller driver will use the
default setting.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi.h | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c| 11 +++
drivers/gpu/drm/msm/dsi/dsi_manager.c | 5 +
drivers/gpu/drm/msm/msm_drv.h | 6 ++
4 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index afc290408ba4..87496db203d6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -37,6 +37,7 @@ struct msm_dsi {
struct mipi_dsi_host *host;
struct msm_dsi_phy *phy;
+ const char *te_source;
struct drm_bridge *next_bridge;
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index c4d72562c95a..c26ad0fed54d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1786,9 +1786,11 @@ static int dsi_populate_dsc_params(struct msm_dsi_host
*msm_host, struct drm_dsc
static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
{
+ struct msm_dsi *msm_dsi = platform_get_drvdata(msm_host->pdev);
struct device *dev = _host->pdev->dev;
struct device_node *np = dev->of_node;
struct device_node *endpoint;
+ const char *te_source;
int ret = 0;
/*
@@ -1811,6 +1813,15 @@ static int dsi_host_parse_dt(struct msm_dsi_host
*msm_host)
goto err;
}
+ ret = of_property_read_string(endpoint, "qcom,te-source", _source);
+ if (ret && ret != -EINVAL) {
+ DRM_DEV_ERROR(dev, "%s: invalid TE source configuration %d\n",
+ __func__, ret);
+ goto err;
+ }
+ if (!ret)
+ msm_dsi->te_source = devm_kstrdup(dev, te_source, GFP_KERNEL);
+
if (of_property_read_bool(np, "syscon-sfpb")) {
msm_host->sfpb = syscon_regmap_lookup_by_phandle(np,
"syscon-sfpb");
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 5b3f3068fd92..a210b7c9e5ca 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -603,3 +603,8 @@ bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
{
return IS_MASTER_DSI_LINK(msm_dsi->id);
}
+
+const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi)
+{
+ return msm_dsi->te_source;
+}
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 912ebaa5df84..afd98dffea99 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -330,6 +330,7 @@ bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
bool msm_dsi_wide_bus_enabled(struct msm_dsi *msm_dsi);
struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi);
+const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi);
#else
static inline void __init msm_dsi_register(void)
{
@@ -367,6 +368,11 @@ static inline struct drm_dsc_config
*msm_dsi_get_dsc_config(struct msm_dsi *msm_
{
return NULL;
}
+
+static inline const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi)
+{
+ return NULL;
+}
#endif
#ifdef CONFIG_DRM_MSM_DP
--
2.39.2
[PATCH v2 1/8] dt-bindings: display/msm/dsi: allow specifying TE source
Command mode panels provide TE signal back to the DSI host to signal
that the frame display has completed and update of the image will not
cause tearing. Usually it is connected to the first GPIO with the
mdp_vsync function, which is the default. In such case the property can
be skipped.
Acked-by: Krzysztof Kozlowski
Reviewed-by: Rob Herring (Arm)
Signed-off-by: Dmitry Baryshkov
---
.../bindings/display/msm/dsi-controller-main.yaml | 17 +
1 file changed, 17 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index 1fa28e976559..e1cb3a1fee81 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -162,6 +162,22 @@ properties:
items:
enum: [ 0, 1, 2, 3 ]
+ qcom,te-source:
+$ref: /schemas/types.yaml#/definitions/string
+description:
+ Specifies the source of vsync signal from the panel used for
+ tearing elimination.
+default: mdp_vsync_p
+enum:
+ - mdp_vsync_p
+ - mdp_vsync_s
+ - mdp_vsync_e
+ - timer0
+ - timer1
+ - timer2
+ - timer3
+ - timer4
+
required:
- port@0
- port@1
@@ -452,6 +468,7 @@ examples:
dsi0_out: endpoint {
remote-endpoint = <_in>;
data-lanes = <0 1 2 3>;
+ qcom,te-source = "mdp_vsync_e";
};
};
};
--
2.39.2
Re: [PATCH v2 5/8] drm/msm/dpu: rework vsync_source handling
Maybe retitle this to something that more closely resembles "remove unset
is_te_using_watchdog_timer field"?
On 2024-06-13 20:05:08, Dmitry Baryshkov wrote:
> The struct msm_display_info has is_te_using_watchdog_timer field which
> is neither used anywhere nor is flexible enough to specify different
Well, it's "used", but not "set" (to anything other than the zero-initialized
default). s/used/set?
> sources. Replace it with the field specifying the vsync source using
> enum dpu_vsync_source.
>
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Dmitry Baryshkov
Patch itself is fine, just think the title could be clearer:
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 5 ++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index bd37a56b4d03..b147f8814a18 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -743,10 +743,7 @@ static void _dpu_encoder_update_vsync_source(struct
> dpu_encoder_virt *dpu_enc,
> vsync_cfg.pp_count = dpu_enc->num_phys_encs;
> vsync_cfg.frame_rate =
> drm_mode_vrefresh(_enc->base.crtc->state->adjusted_mode);
>
> - if (disp_info->is_te_using_watchdog_timer)
> - vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
> - else
> - vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
> + vsync_cfg.vsync_source = disp_info->vsync_source;
>
> hw_mdptop->ops.setup_vsync_source(hw_mdptop, _cfg);
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index 76be77e30954..cb59bd4436f4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -26,15 +26,14 @@
> * @h_tile_instance:Controller instance used per tile. Number of
> elements is
> * based on num_of_h_tiles
> * @is_cmd_mode Boolean to indicate if the CMD mode is requested
> - * @is_te_using_watchdog_timer: Boolean to indicate watchdog TE is
> - *used instead of panel TE in cmd mode panels
> + * @vsync_source:Source of the TE signal for DSI CMD devices
> */
> struct msm_display_info {
> enum dpu_intf_type intf_type;
> uint32_t num_of_h_tiles;
> uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
> bool is_cmd_mode;
> - bool is_te_using_watchdog_timer;
> + enum dpu_vsync_source vsync_source;
> };
>
> /**
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 1955848b1b78..e9991f3756d4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -543,6 +543,8 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
>
> info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
>
> + info.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
> +
> encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI, );
> if (IS_ERR(encoder)) {
> DPU_ERROR("encoder init failed for dsi display\n");
>
> --
> 2.39.2
>
Re: [PATCH v2 3/8] drm/msm/dsi: drop unused GPIOs handling
On 2024-06-13 20:05:06, Dmitry Baryshkov wrote:
> Neither disp-enable-gpios nor disp-te-gpios are defined in the schema.
> None of the board DT files use those GPIO pins. Drop them from the
> driver.
What's worse, when people set disp-te-gpios the
devm_gpiod_get_optional("disp-te", GPIOD_IN) below resets the typical mdp_vsync
function via pinctrl to the IN function, causing vsync signals to be lost and
the MDP hardware to fall back to half the requested refresh rate since commit
da9e7b7696d8 ("drm/msm/dpu: Correctly configure vsync tearcheck for command
mode").
>
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Dmitry Baryshkov
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 37 -
> 1 file changed, 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index a50f4dda5941..c4d72562c95a 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -7,7 +7,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -130,9 +129,6 @@ struct msm_dsi_host {
>
> unsigned long src_clk_rate;
>
> - struct gpio_desc *disp_en_gpio;
> - struct gpio_desc *te_gpio;
> -
> const struct msm_dsi_cfg_handler *cfg_hnd;
>
> struct completion dma_comp;
> @@ -1613,28 +1609,6 @@ static irqreturn_t dsi_host_irq(int irq, void *ptr)
> return IRQ_HANDLED;
> }
>
> -static int dsi_host_init_panel_gpios(struct msm_dsi_host *msm_host,
> - struct device *panel_device)
> -{
> - msm_host->disp_en_gpio = devm_gpiod_get_optional(panel_device,
> - "disp-enable",
> - GPIOD_OUT_LOW);
> - if (IS_ERR(msm_host->disp_en_gpio)) {
> - DBG("cannot get disp-enable-gpios %ld",
> - PTR_ERR(msm_host->disp_en_gpio));
> - return PTR_ERR(msm_host->disp_en_gpio);
> - }
> -
> - msm_host->te_gpio = devm_gpiod_get_optional(panel_device, "disp-te",
> - GPIOD_IN);
> - if (IS_ERR(msm_host->te_gpio)) {
> - DBG("cannot get disp-te-gpios %ld", PTR_ERR(msm_host->te_gpio));
> - return PTR_ERR(msm_host->te_gpio);
> - }
> -
> - return 0;
> -}
> -
> static int dsi_host_attach(struct mipi_dsi_host *host,
> struct mipi_dsi_device *dsi)
> {
> @@ -1651,11 +1625,6 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
> if (dsi->dsc)
> msm_host->dsc = dsi->dsc;
>
> - /* Some gpios defined in panel DT need to be controlled by host */
> - ret = dsi_host_init_panel_gpios(msm_host, >dev);
> - if (ret)
> - return ret;
> -
> ret = dsi_dev_attach(msm_host->pdev);
> if (ret)
> return ret;
> @@ -2422,9 +2391,6 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> dsi_sw_reset(msm_host);
> dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
>
> - if (msm_host->disp_en_gpio)
> - gpiod_set_value(msm_host->disp_en_gpio, 1);
> -
> msm_host->power_on = true;
> mutex_unlock(_host->dev_mutex);
>
> @@ -2454,9 +2420,6 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
>
> dsi_ctrl_disable(msm_host);
>
> - if (msm_host->disp_en_gpio)
> - gpiod_set_value(msm_host->disp_en_gpio, 0);
> -
> pinctrl_pm_select_sleep_state(_host->pdev->dev);
>
> cfg_hnd->ops->link_clk_disable(msm_host);
>
> --
> 2.39.2
>
Re: [PATCH v2 4/8] drm/msm/dpu: pull the is_cmd_mode out of _dpu_encoder_update_vsync_source()
On 2024-06-13 20:05:07, Dmitry Baryshkov wrote:
> Setting vsync source makes sense only for DSI CMD panels. Pull the
> is_cmd_mode condition out of the function into the calling code, so that
> it becomes more explicit.
>
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Dmitry Baryshkov
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 4988a1029431..bd37a56b4d03 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -736,8 +736,7 @@ static void _dpu_encoder_update_vsync_source(struct
> dpu_encoder_virt *dpu_enc,
> return;
> }
>
> - if (hw_mdptop->ops.setup_vsync_source &&
> - disp_info->is_cmd_mode) {
> + if (hw_mdptop->ops.setup_vsync_source) {
> for (i = 0; i < dpu_enc->num_phys_encs; i++)
> vsync_cfg.ppnumber[i] = dpu_enc->hw_pp[i]->idx;
>
> @@ -1226,7 +1225,8 @@ static void _dpu_encoder_virt_enable_helper(struct
> drm_encoder *drm_enc)
> dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select(
> dpu_enc->cur_master->hw_mdptop);
>
> - _dpu_encoder_update_vsync_source(dpu_enc, _enc->disp_info);
> + if (dpu_enc->disp_info.is_cmd_mode)
> + _dpu_encoder_update_vsync_source(dpu_enc, _enc->disp_info);
>
> if (dpu_enc->disp_info.intf_type == INTF_DSI &&
> !WARN_ON(dpu_enc->num_phys_encs == 0)) {
>
> --
> 2.39.2
>
Re: [PATCH] drm/ci: mark kms_addfb_basic@addfb25-bad-modifier as passing on msm
On 6/13/2024 9:33 AM, Dmitry Baryshkov wrote:
The commit b228501ff183 ("drm/msm: merge dpu format database to MDP
formats") made get_format take modifiers into account. This makes
kms_addfb_basic@addfb25-bad-modifier pass on MDP4 and MDP5 platforms.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt | 1 -
drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt | 1 -
2 files changed, 2 deletions(-)
Would be good to also give a link to the CI for the CI maintainers.
But otherwise, LGTM
Reviewed-by: Abhinav Kumar
diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
index 3dfbabdf905e..6e7fd1ccd1e3 100644
--- a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
+++ b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
@@ -4,7 +4,6 @@ device_reset@unbind-cold-reset-rebind,Fail
device_reset@unbind-reset-rebind,Fail
dumb_buffer@invalid-bpp,Fail
kms_3d,Fail
-kms_addfb_basic@addfb25-bad-modifier,Fail
kms_cursor_legacy@forked-move,Fail
kms_cursor_legacy@single-bo,Fail
kms_cursor_legacy@torture-bo,Fail
diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
b/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
index 23a5f6f9097f..46ca69ce2ffe 100644
--- a/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
+++ b/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
@@ -4,6 +4,5 @@ device_reset@unbind-cold-reset-rebind,Fail
device_reset@unbind-reset-rebind,Fail
dumb_buffer@invalid-bpp,Fail
kms_3d,Fail
-kms_addfb_basic@addfb25-bad-modifier,Fail
kms_lease@lease-uevent,Fail
tools_test@tools_test,Fail
---
base-commit: 6b4468b0c6ba37a16795da567b58dc80bc7fb439
change-id: 20240613-msm-pass-addfb25-bad-modifier-c461fd9c02bb
Best regards,
Re: [PATCH v2 1/8] dt-bindings: display/msm/dsi: allow specifying TE source
On Thu, 13 Jun 2024 at 21:16, Marijn Suijten
wrote:
>
> On 2024-06-13 20:05:04, Dmitry Baryshkov wrote:
> > Command mode panels provide TE signal back to the DSI host to signal
> > that the frame display has completed and update of the image will not
> > cause tearing. Usually it is connected to the first GPIO with the
> > mdp_vsync function, which is the default. In such case the property can
> > be skipped.
> >
> > Acked-by: Krzysztof Kozlowski
> > Reviewed-by: Rob Herring (Arm)
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > .../bindings/display/msm/dsi-controller-main.yaml | 17
> > +
> > 1 file changed, 17 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > index 1fa28e976559..e1cb3a1fee81 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > @@ -162,6 +162,22 @@ properties:
> > items:
> >enum: [ 0, 1, 2, 3 ]
> >
> > + qcom,te-source:
> > +$ref: /schemas/types.yaml#/definitions/string
> > +description:
> > + Specifies the source of vsync signal from the panel used
> > for
> > + tearing elimination.
> > +default: mdp_vsync_p
> > +enum:
> > + - mdp_vsync_p
> > + - mdp_vsync_s
> > + - mdp_vsync_e
>
> When discussing that these should be renamed, was it also documented what the
> suffix means? I can only guess something like primary/secondary/e...?
external. Note, these names match the name in the datasheets.
>
> Are the mdp_intfX variants missing here that you're handling in patch 7/8?
I didn't test them, so I didn't document them.
>
> > + - timer0
> > + - timer1
> > + - timer2
> > + - timer3
> > + - timer4
> > +
> > required:
> >- port@0
> >- port@1
> > @@ -452,6 +468,7 @@ examples:
> >dsi0_out: endpoint {
> > remote-endpoint = <_in>;
> > data-lanes = <0 1 2 3>;
> > + qcom,te-source = "mdp_vsync_e";
> >};
> >};
> > };
> >
> > --
> > 2.39.2
> >
--
With best wishes
Dmitry
Re: [PATCH v2 6/8] drm/msm/dsi: parse vsync source from device tree
On 2024-06-13 20:05:09, Dmitry Baryshkov wrote:
> Allow board's device tree to specify the vsync source (aka TE source).
> If the property is omitted, the display controller driver will use the
> default setting.
Well, that specific default handling is not really part of this patch, but
how a followup patch is going to respond when msm_dsi_get_te_source() returns
NULL. (Or how that followup patch is expected to deal with that - worth a
doc-comment?)
>
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Dmitry Baryshkov
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi.h | 1 +
> drivers/gpu/drm/msm/dsi/dsi_host.c| 11 +++
> drivers/gpu/drm/msm/dsi/dsi_manager.c | 5 +
> drivers/gpu/drm/msm/msm_drv.h | 6 ++
> 4 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> index afc290408ba4..87496db203d6 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> @@ -37,6 +37,7 @@ struct msm_dsi {
>
> struct mipi_dsi_host *host;
> struct msm_dsi_phy *phy;
> + const char *te_source;
>
> struct drm_bridge *next_bridge;
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index c4d72562c95a..c26ad0fed54d 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1786,9 +1786,11 @@ static int dsi_populate_dsc_params(struct msm_dsi_host
> *msm_host, struct drm_dsc
>
> static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
> {
> + struct msm_dsi *msm_dsi = platform_get_drvdata(msm_host->pdev);
> struct device *dev = _host->pdev->dev;
> struct device_node *np = dev->of_node;
> struct device_node *endpoint;
> + const char *te_source;
> int ret = 0;
>
> /*
> @@ -1811,6 +1813,15 @@ static int dsi_host_parse_dt(struct msm_dsi_host
> *msm_host)
> goto err;
> }
>
> + ret = of_property_read_string(endpoint, "qcom,te-source", _source);
> + if (ret && ret != -EINVAL) {
> + DRM_DEV_ERROR(dev, "%s: invalid TE source configuration %d\n",
> + __func__, ret);
> + goto err;
> + }
> + if (!ret)
> + msm_dsi->te_source = devm_kstrdup(dev, te_source, GFP_KERNEL);
> +
> if (of_property_read_bool(np, "syscon-sfpb")) {
> msm_host->sfpb = syscon_regmap_lookup_by_phandle(np,
> "syscon-sfpb");
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index 5b3f3068fd92..a210b7c9e5ca 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -603,3 +603,8 @@ bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
> {
> return IS_MASTER_DSI_LINK(msm_dsi->id);
> }
> +
> +const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi)
> +{
> + return msm_dsi->te_source;
> +}
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 912ebaa5df84..afd98dffea99 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -330,6 +330,7 @@ bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
> bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
> bool msm_dsi_wide_bus_enabled(struct msm_dsi *msm_dsi);
> struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi);
> +const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi);
> #else
> static inline void __init msm_dsi_register(void)
> {
> @@ -367,6 +368,11 @@ static inline struct drm_dsc_config
> *msm_dsi_get_dsc_config(struct msm_dsi *msm_
> {
> return NULL;
> }
> +
> +static inline const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi)
> +{
> + return NULL;
> +}
> #endif
>
> #ifdef CONFIG_DRM_MSM_DP
>
> --
> 2.39.2
>
Re: [PATCH v2 2/8] drm/msm/dpu: convert vsync source defines to the enum
On 2024-06-13 20:05:05, Dmitry Baryshkov wrote:
> Add enum dpu_vsync_source instead of a series of defines. Use this enum
> to pass vsync information.
>
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +-
> 5 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 119f3ea50a7c..4988a1029431 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -747,7 +747,7 @@ static void _dpu_encoder_update_vsync_source(struct
> dpu_encoder_virt *dpu_enc,
> if (disp_info->is_te_using_watchdog_timer)
> vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
> else
> - vsync_cfg.vsync_source = DPU_VSYNC0_SOURCE_GPIO;
> + vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
>
> hw_mdptop->ops.setup_vsync_source(hw_mdptop, _cfg);
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 225c1c7768ff..96f6160cf607 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -462,7 +462,7 @@ static int dpu_hw_intf_get_vsync_info(struct dpu_hw_intf
> *intf,
> }
>
> static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
> - u32 vsync_source)
> + enum dpu_vsync_source vsync_source)
> {
> struct dpu_hw_blk_reg_map *c;
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> index f9015c67a574..ac244f0b33fb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> @@ -107,7 +107,7 @@ struct dpu_hw_intf_ops {
>
> int (*connect_external_te)(struct dpu_hw_intf *intf, bool
> enable_external_te);
>
> - void (*vsync_sel)(struct dpu_hw_intf *intf, u32 vsync_source);
> + void (*vsync_sel)(struct dpu_hw_intf *intf, enum dpu_vsync_source
> vsync_source);
>
> /**
>* Disable autorefresh if enabled
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> index 66759623fc42..a2eff36a2224 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -54,18 +54,20 @@
> #define DPU_BLEND_BG_INV_MOD_ALPHA (1 << 12)
> #define DPU_BLEND_BG_TRANSP_EN (1 << 13)
>
> -#define DPU_VSYNC0_SOURCE_GPIO 0
> -#define DPU_VSYNC1_SOURCE_GPIO 1
> -#define DPU_VSYNC2_SOURCE_GPIO 2
> -#define DPU_VSYNC_SOURCE_INTF_0 3
> -#define DPU_VSYNC_SOURCE_INTF_1 4
> -#define DPU_VSYNC_SOURCE_INTF_2 5
> -#define DPU_VSYNC_SOURCE_INTF_3 6
> -#define DPU_VSYNC_SOURCE_WD_TIMER_4 11
> -#define DPU_VSYNC_SOURCE_WD_TIMER_3 12
> -#define DPU_VSYNC_SOURCE_WD_TIMER_2 13
> -#define DPU_VSYNC_SOURCE_WD_TIMER_1 14
> -#define DPU_VSYNC_SOURCE_WD_TIMER_0 15
> +enum dpu_vsync_source {
> + DPU_VSYNC_SOURCE_GPIO_0,
> + DPU_VSYNC_SOURCE_GPIO_1,
> + DPU_VSYNC_SOURCE_GPIO_2,
While at it, rename this to _P / _S / _E to match the other patches/code?
- Marijn
> + DPU_VSYNC_SOURCE_INTF_0 = 3,
> + DPU_VSYNC_SOURCE_INTF_1,
> + DPU_VSYNC_SOURCE_INTF_2,
> + DPU_VSYNC_SOURCE_INTF_3,
> + DPU_VSYNC_SOURCE_WD_TIMER_4 = 11,
> + DPU_VSYNC_SOURCE_WD_TIMER_3,
> + DPU_VSYNC_SOURCE_WD_TIMER_2,
> + DPU_VSYNC_SOURCE_WD_TIMER_1,
> + DPU_VSYNC_SOURCE_WD_TIMER_0,
> +};
>
> enum dpu_hw_blk_type {
> DPU_HW_BLK_TOP = 0,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> index 6f3dc98087df..5c9a7ede991e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> @@ -64,7 +64,7 @@ struct dpu_vsync_source_cfg {
> u32 pp_count;
> u32 frame_rate;
> u32 ppnumber[PINGPONG_MAX];
> - u32 vsync_source;
> + enum dpu_vsync_source vsync_source;
> };
>
> /**
>
> --
> 2.39.2
>
[PATCH v4 3/4] arm64: dts: qcom: msm8998: add HDMI GPIOs
MSM8998 GPIO pin controller reference design defines:
- CEC: pin 31
- DDC: pin 32,33
- HPD: pin 34
Downstream vendor code for reference:
https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi#L2324-2400
mdss_hdmi_{cec,ddc,hpd}_{active,suspend}
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Marc Gonzalez
---
arch/arm64/boot/dts/qcom/msm8998.dtsi | 28
1 file changed, 28 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi
b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index e5f051f5a92de..ba5e873f0f35f 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1434,6 +1434,34 @@ blsp2_spi6_default: blsp2-spi6-default-state {
drive-strength = <6>;
bias-disable;
};
+
+ hdmi_cec_default: hdmi-cec-default-state {
+ pins = "gpio31";
+ function = "hdmi_cec";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ hdmi_ddc_default: hdmi-ddc-default-state {
+ pins = "gpio32", "gpio33";
+ function = "hdmi_ddc";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ hdmi_hpd_default: hdmi-hpd-default-state {
+ pins = "gpio34";
+ function = "hdmi_hot";
+ drive-strength = <16>;
+ bias-pull-down;
+ };
+
+ hdmi_hpd_sleep: hdmi-hpd-sleep-state {
+ pins = "gpio34";
+ function = "hdmi_hot";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
};
remoteproc_mss: remoteproc@408 {
--
2.34.1
[PATCH v4 0/4] HDMI TX support in msm8998
DT bits required for HDMI TX support in APQ8098 (msm8998 cousin) Changes in v4: - Collect tags since v3 - Reword patch 1 subject (Vinod) - Link to v3: https://lore.kernel.org/r/20240606-hdmi-tx-v3-0-9d7feb6d3...@freebox.fr Changes in v3 - Address Rob's comments on patch 2: - 'maxItems: 5' for clocks in the 8996 if/then schema - match the order of 8996 for the clock-names in common --- Arnaud Vrac (1): arm64: dts: qcom: add HDMI nodes for msm8998 Marc Gonzalez (3): dt-bindings: phy: add qcom,hdmi-phy-8998 dt-bindings: display/msm: hdmi: add qcom,hdmi-tx-8998 arm64: dts: qcom: msm8998: add HDMI GPIOs .../devicetree/bindings/display/msm/hdmi.yaml | 28 - .../devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml | 1 + arch/arm64/boot/dts/qcom/msm8998.dtsi | 128 - 3 files changed, 154 insertions(+), 3 deletions(-) --- base-commit: 2c4f4d94dcbf6f500b92fff5600989ea23a207e8 change-id: 20240606-hdmi-tx-00ee8e7ddbac Best regards, -- Marc Gonzalez
[PATCH v2 0/8] drm/msm/dpu: handle non-default TE source pins
Command-mode DSI panels need to signal the display controlller when vsync happens, so that the device can start sending the next frame. Some devices (Google Pixel 3) use a non-default pin, so additional configuration is required. Add a way to specify this information in DT and handle it in the DSI and DPU drivers. Signed-off-by: Dmitry Baryshkov --- Changes in v2: - In DT bindings renamed mdp_gpioN to mdp_vsync_p/_s/_e per pins name (Abhinav) - Extended bindings to include default: mdp_vsync_p (Rob) - Renamed dpu_hw_setup_vsync_source() and dpu_hw_setup_vsync_source_and_vsync_sel() to match the implementation (Abhinav) - Link to v1: https://lore.kernel.org/r/20240520-dpu-handle-te-signal-v1-0-f273b42a0...@linaro.org --- Dmitry Baryshkov (8): dt-bindings: display/msm/dsi: allow specifying TE source drm/msm/dpu: convert vsync source defines to the enum drm/msm/dsi: drop unused GPIOs handling drm/msm/dpu: pull the is_cmd_mode out of _dpu_encoder_update_vsync_source() drm/msm/dpu: rework vsync_source handling drm/msm/dsi: parse vsync source from device tree drm/msm/dpu: support setting the TE source drm/msm/dpu: rename dpu_hw_setup_vsync_source functions .../bindings/display/msm/dsi-controller-main.yaml | 17 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 11 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 5 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 26 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 14 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 44 drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 48 +- drivers/gpu/drm/msm/dsi/dsi_manager.c | 5 +++ drivers/gpu/drm/msm/msm_drv.h | 6 +++ 13 files changed, 114 insertions(+), 69 deletions(-) --- base-commit: 03d44168cbd7fc57d5de56a3730427db758fc7f6 change-id: 20240514-dpu-handle-te-signal-82663c0211bd Best regards, -- Dmitry Baryshkov
[PATCH v4 1/4] dt-bindings: phy: add qcom,hdmi-phy-8998
HDMI PHY block embedded in the APQ8098. Acked-by: Rob Herring (Arm) Signed-off-by: Marc Gonzalez --- Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml b/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml index 83fe4b39b56f4..78607ee3e2e84 100644 --- a/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml @@ -14,6 +14,7 @@ properties: compatible: enum: - qcom,hdmi-phy-8996 + - qcom,hdmi-phy-8998 reg: maxItems: 6 -- 2.34.1
[PATCH v2 5/8] drm/msm/dpu: rework vsync_source handling
The struct msm_display_info has is_te_using_watchdog_timer field which
is neither used anywhere nor is flexible enough to specify different
sources. Replace it with the field specifying the vsync source using
enum dpu_vsync_source.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 5 ++---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index bd37a56b4d03..b147f8814a18 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -743,10 +743,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
vsync_cfg.pp_count = dpu_enc->num_phys_encs;
vsync_cfg.frame_rate =
drm_mode_vrefresh(_enc->base.crtc->state->adjusted_mode);
- if (disp_info->is_te_using_watchdog_timer)
- vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
- else
- vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
+ vsync_cfg.vsync_source = disp_info->vsync_source;
hw_mdptop->ops.setup_vsync_source(hw_mdptop, _cfg);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 76be77e30954..cb59bd4436f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -26,15 +26,14 @@
* @h_tile_instance:Controller instance used per tile. Number of elements
is
* based on num_of_h_tiles
* @is_cmd_modeBoolean to indicate if the CMD mode is requested
- * @is_te_using_watchdog_timer: Boolean to indicate watchdog TE is
- * used instead of panel TE in cmd mode panels
+ * @vsync_source: Source of the TE signal for DSI CMD devices
*/
struct msm_display_info {
enum dpu_intf_type intf_type;
uint32_t num_of_h_tiles;
uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
bool is_cmd_mode;
- bool is_te_using_watchdog_timer;
+ enum dpu_vsync_source vsync_source;
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1955848b1b78..e9991f3756d4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -543,6 +543,8 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
+ info.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
+
encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI, );
if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for dsi display\n");
--
2.39.2
[PATCH v4 2/4] dt-bindings: display/msm: hdmi: add qcom,hdmi-tx-8998
HDMI TX block embedded in the APQ8098. Reviewed-by: Rob Herring (Arm) Reviewed-by: Conor Dooley Signed-off-by: Marc Gonzalez --- .../devicetree/bindings/display/msm/hdmi.yaml | 28 -- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.yaml b/Documentation/devicetree/bindings/display/msm/hdmi.yaml index 47e97669821c3..d4a2033afea8d 100644 --- a/Documentation/devicetree/bindings/display/msm/hdmi.yaml +++ b/Documentation/devicetree/bindings/display/msm/hdmi.yaml @@ -19,14 +19,15 @@ properties: - qcom,hdmi-tx-8974 - qcom,hdmi-tx-8994 - qcom,hdmi-tx-8996 + - qcom,hdmi-tx-8998 clocks: minItems: 1 -maxItems: 5 +maxItems: 8 clock-names: minItems: 1 -maxItems: 5 +maxItems: 8 reg: minItems: 1 @@ -142,6 +143,7 @@ allOf: properties: clocks: minItems: 5 + maxItems: 5 clock-names: items: - const: mdp_core @@ -151,6 +153,28 @@ allOf: - const: extp hdmi-mux-supplies: false + - if: + properties: +compatible: + contains: +enum: + - qcom,hdmi-tx-8998 +then: + properties: +clocks: + minItems: 8 + maxItems: 8 +clock-names: + items: +- const: mdp_core +- const: iface +- const: core +- const: alt_iface +- const: extp +- const: bus +- const: mnoc +- const: iface_mmss + additionalProperties: false examples: -- 2.34.1
[PATCH v2 8/8] drm/msm/dpu: rename dpu_hw_setup_vsync_source functions
Rename dpu_hw_setup_vsync_source functions to make the names match the
implementation: on DPU 5.x the TOP only contains timer setup, while 3.x
and 4.x used MDP_VSYNC_SEL register to select TE source.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
index 05e48cf4ec1d..6e2ac50b94a4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
@@ -107,8 +107,8 @@ static void dpu_hw_get_danger_status(struct dpu_hw_mdp *mdp,
status->sspp[SSPP_CURSOR1] = (value >> 26) & 0x3;
}
-static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp *mdp,
- struct dpu_vsync_source_cfg *cfg)
+static void dpu_hw_setup_wd_timer(struct dpu_hw_mdp *mdp,
+ struct dpu_vsync_source_cfg *cfg)
{
struct dpu_hw_blk_reg_map *c;
u32 reg, wd_load_value, wd_ctl, wd_ctl2;
@@ -163,8 +163,8 @@ static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp
*mdp,
}
}
-static void dpu_hw_setup_vsync_source_and_vsync_sel(struct dpu_hw_mdp *mdp,
- struct dpu_vsync_source_cfg *cfg)
+static void dpu_hw_setup_vsync_sel(struct dpu_hw_mdp *mdp,
+ struct dpu_vsync_source_cfg *cfg)
{
struct dpu_hw_blk_reg_map *c;
u32 reg, i;
@@ -187,7 +187,7 @@ static void dpu_hw_setup_vsync_source_and_vsync_sel(struct
dpu_hw_mdp *mdp,
}
DPU_REG_WRITE(c, MDP_VSYNC_SEL, reg);
- dpu_hw_setup_vsync_source(mdp, cfg);
+ dpu_hw_setup_wd_timer(mdp, cfg);
}
static void dpu_hw_get_safe_status(struct dpu_hw_mdp *mdp,
@@ -239,9 +239,9 @@ static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops,
ops->get_danger_status = dpu_hw_get_danger_status;
if (cap & BIT(DPU_MDP_VSYNC_SEL))
- ops->setup_vsync_source =
dpu_hw_setup_vsync_source_and_vsync_sel;
+ ops->setup_vsync_source = dpu_hw_setup_vsync_sel;
else
- ops->setup_vsync_source = dpu_hw_setup_vsync_source;
+ ops->setup_vsync_source = dpu_hw_setup_wd_timer;
ops->get_safe_status = dpu_hw_get_safe_status;
--
2.39.2
[PATCH v2 2/8] drm/msm/dpu: convert vsync source defines to the enum
Add enum dpu_vsync_source instead of a series of defines. Use this enum
to pass vsync information.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +-
5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..4988a1029431 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -747,7 +747,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
if (disp_info->is_te_using_watchdog_timer)
vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
else
- vsync_cfg.vsync_source = DPU_VSYNC0_SOURCE_GPIO;
+ vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
hw_mdptop->ops.setup_vsync_source(hw_mdptop, _cfg);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 225c1c7768ff..96f6160cf607 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -462,7 +462,7 @@ static int dpu_hw_intf_get_vsync_info(struct dpu_hw_intf
*intf,
}
static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
- u32 vsync_source)
+ enum dpu_vsync_source vsync_source)
{
struct dpu_hw_blk_reg_map *c;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index f9015c67a574..ac244f0b33fb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -107,7 +107,7 @@ struct dpu_hw_intf_ops {
int (*connect_external_te)(struct dpu_hw_intf *intf, bool
enable_external_te);
- void (*vsync_sel)(struct dpu_hw_intf *intf, u32 vsync_source);
+ void (*vsync_sel)(struct dpu_hw_intf *intf, enum dpu_vsync_source
vsync_source);
/**
* Disable autorefresh if enabled
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 66759623fc42..a2eff36a2224 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -54,18 +54,20 @@
#define DPU_BLEND_BG_INV_MOD_ALPHA (1 << 12)
#define DPU_BLEND_BG_TRANSP_EN (1 << 13)
-#define DPU_VSYNC0_SOURCE_GPIO 0
-#define DPU_VSYNC1_SOURCE_GPIO 1
-#define DPU_VSYNC2_SOURCE_GPIO 2
-#define DPU_VSYNC_SOURCE_INTF_03
-#define DPU_VSYNC_SOURCE_INTF_14
-#define DPU_VSYNC_SOURCE_INTF_25
-#define DPU_VSYNC_SOURCE_INTF_36
-#define DPU_VSYNC_SOURCE_WD_TIMER_411
-#define DPU_VSYNC_SOURCE_WD_TIMER_312
-#define DPU_VSYNC_SOURCE_WD_TIMER_213
-#define DPU_VSYNC_SOURCE_WD_TIMER_114
-#define DPU_VSYNC_SOURCE_WD_TIMER_015
+enum dpu_vsync_source {
+ DPU_VSYNC_SOURCE_GPIO_0,
+ DPU_VSYNC_SOURCE_GPIO_1,
+ DPU_VSYNC_SOURCE_GPIO_2,
+ DPU_VSYNC_SOURCE_INTF_0 = 3,
+ DPU_VSYNC_SOURCE_INTF_1,
+ DPU_VSYNC_SOURCE_INTF_2,
+ DPU_VSYNC_SOURCE_INTF_3,
+ DPU_VSYNC_SOURCE_WD_TIMER_4 = 11,
+ DPU_VSYNC_SOURCE_WD_TIMER_3,
+ DPU_VSYNC_SOURCE_WD_TIMER_2,
+ DPU_VSYNC_SOURCE_WD_TIMER_1,
+ DPU_VSYNC_SOURCE_WD_TIMER_0,
+};
enum dpu_hw_blk_type {
DPU_HW_BLK_TOP = 0,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
index 6f3dc98087df..5c9a7ede991e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
@@ -64,7 +64,7 @@ struct dpu_vsync_source_cfg {
u32 pp_count;
u32 frame_rate;
u32 ppnumber[PINGPONG_MAX];
- u32 vsync_source;
+ enum dpu_vsync_source vsync_source;
};
/**
--
2.39.2
[PATCH v2 4/8] drm/msm/dpu: pull the is_cmd_mode out of _dpu_encoder_update_vsync_source()
Setting vsync source makes sense only for DSI CMD panels. Pull the
is_cmd_mode condition out of the function into the calling code, so that
it becomes more explicit.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 4988a1029431..bd37a56b4d03 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -736,8 +736,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
return;
}
- if (hw_mdptop->ops.setup_vsync_source &&
- disp_info->is_cmd_mode) {
+ if (hw_mdptop->ops.setup_vsync_source) {
for (i = 0; i < dpu_enc->num_phys_encs; i++)
vsync_cfg.ppnumber[i] = dpu_enc->hw_pp[i]->idx;
@@ -1226,7 +1225,8 @@ static void _dpu_encoder_virt_enable_helper(struct
drm_encoder *drm_enc)
dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select(
dpu_enc->cur_master->hw_mdptop);
- _dpu_encoder_update_vsync_source(dpu_enc, _enc->disp_info);
+ if (dpu_enc->disp_info.is_cmd_mode)
+ _dpu_encoder_update_vsync_source(dpu_enc, _enc->disp_info);
if (dpu_enc->disp_info.intf_type == INTF_DSI &&
!WARN_ON(dpu_enc->num_phys_encs == 0)) {
--
2.39.2
[PATCH v2 3/8] drm/msm/dsi: drop unused GPIOs handling
Neither disp-enable-gpios nor disp-te-gpios are defined in the schema.
None of the board DT files use those GPIO pins. Drop them from the
driver.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 37 -
1 file changed, 37 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a50f4dda5941..c4d72562c95a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -7,7 +7,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -130,9 +129,6 @@ struct msm_dsi_host {
unsigned long src_clk_rate;
- struct gpio_desc *disp_en_gpio;
- struct gpio_desc *te_gpio;
-
const struct msm_dsi_cfg_handler *cfg_hnd;
struct completion dma_comp;
@@ -1613,28 +1609,6 @@ static irqreturn_t dsi_host_irq(int irq, void *ptr)
return IRQ_HANDLED;
}
-static int dsi_host_init_panel_gpios(struct msm_dsi_host *msm_host,
- struct device *panel_device)
-{
- msm_host->disp_en_gpio = devm_gpiod_get_optional(panel_device,
-"disp-enable",
-GPIOD_OUT_LOW);
- if (IS_ERR(msm_host->disp_en_gpio)) {
- DBG("cannot get disp-enable-gpios %ld",
- PTR_ERR(msm_host->disp_en_gpio));
- return PTR_ERR(msm_host->disp_en_gpio);
- }
-
- msm_host->te_gpio = devm_gpiod_get_optional(panel_device, "disp-te",
- GPIOD_IN);
- if (IS_ERR(msm_host->te_gpio)) {
- DBG("cannot get disp-te-gpios %ld", PTR_ERR(msm_host->te_gpio));
- return PTR_ERR(msm_host->te_gpio);
- }
-
- return 0;
-}
-
static int dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *dsi)
{
@@ -1651,11 +1625,6 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
if (dsi->dsc)
msm_host->dsc = dsi->dsc;
- /* Some gpios defined in panel DT need to be controlled by host */
- ret = dsi_host_init_panel_gpios(msm_host, >dev);
- if (ret)
- return ret;
-
ret = dsi_dev_attach(msm_host->pdev);
if (ret)
return ret;
@@ -2422,9 +2391,6 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
dsi_sw_reset(msm_host);
dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
- if (msm_host->disp_en_gpio)
- gpiod_set_value(msm_host->disp_en_gpio, 1);
-
msm_host->power_on = true;
mutex_unlock(_host->dev_mutex);
@@ -2454,9 +2420,6 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
dsi_ctrl_disable(msm_host);
- if (msm_host->disp_en_gpio)
- gpiod_set_value(msm_host->disp_en_gpio, 0);
-
pinctrl_pm_select_sleep_state(_host->pdev->dev);
cfg_hnd->ops->link_clk_disable(msm_host);
--
2.39.2
[PATCH v2 7/8] drm/msm/dpu: support setting the TE source
Make the DPU driver use the TE source specified in the DT. If none is
specified, the driver defaults to the first GPIO (mdp_vsync0).
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 44 -
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index e9991f3756d4..6fcb3cf4a382 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -505,6 +505,44 @@ static void dpu_kms_wait_flush(struct msm_kms *kms,
unsigned crtc_mask)
dpu_kms_wait_for_commit_done(kms, crtc);
}
+static const char *dpu_vsync_sources[] = {
+ [DPU_VSYNC_SOURCE_GPIO_0] = "mdp_vsync_p",
+ [DPU_VSYNC_SOURCE_GPIO_1] = "mdp_vsync_s",
+ [DPU_VSYNC_SOURCE_GPIO_2] = "mdp_vsync_e",
+ [DPU_VSYNC_SOURCE_INTF_0] = "mdp_intf0",
+ [DPU_VSYNC_SOURCE_INTF_1] = "mdp_intf1",
+ [DPU_VSYNC_SOURCE_INTF_2] = "mdp_intf2",
+ [DPU_VSYNC_SOURCE_INTF_3] = "mdp_intf3",
+ [DPU_VSYNC_SOURCE_WD_TIMER_0] = "timer0",
+ [DPU_VSYNC_SOURCE_WD_TIMER_1] = "timer1",
+ [DPU_VSYNC_SOURCE_WD_TIMER_2] = "timer2",
+ [DPU_VSYNC_SOURCE_WD_TIMER_3] = "timer3",
+ [DPU_VSYNC_SOURCE_WD_TIMER_4] = "timer4",
+};
+
+static int dpu_kms_dsi_set_te_source(struct msm_display_info *info,
+struct msm_dsi *dsi)
+{
+ const char *te_source = msm_dsi_get_te_source(dsi);
+ int i;
+
+ if (!te_source) {
+ info->vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
+ return 0;
+ }
+
+ /* we can not use match_string since dpu_vsync_sources is a sparse
array */
+ for (i = 0; i < ARRAY_SIZE(dpu_vsync_sources); i++) {
+ if (dpu_vsync_sources[i] &&
+ !strcmp(dpu_vsync_sources[i], te_source)) {
+ info->vsync_source = i;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
static int _dpu_kms_initialize_dsi(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms)
@@ -543,7 +581,11 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
- info.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
+ rc = dpu_kms_dsi_set_te_source(, priv->dsi[i]);
+ if (rc) {
+ DPU_ERROR("failed to identify TE source for dsi
display\n");
+ return rc;
+ }
encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI, );
if (IS_ERR(encoder)) {
--
2.39.2
[PATCH] drm/ci: mark kms_addfb_basic@addfb25-bad-modifier as passing on msm
The commit b228501ff183 ("drm/msm: merge dpu format database to MDP
formats") made get_format take modifiers into account. This makes
kms_addfb_basic@addfb25-bad-modifier pass on MDP4 and MDP5 platforms.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt | 1 -
drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
index 3dfbabdf905e..6e7fd1ccd1e3 100644
--- a/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
+++ b/drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt
@@ -4,7 +4,6 @@ device_reset@unbind-cold-reset-rebind,Fail
device_reset@unbind-reset-rebind,Fail
dumb_buffer@invalid-bpp,Fail
kms_3d,Fail
-kms_addfb_basic@addfb25-bad-modifier,Fail
kms_cursor_legacy@forked-move,Fail
kms_cursor_legacy@single-bo,Fail
kms_cursor_legacy@torture-bo,Fail
diff --git a/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
b/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
index 23a5f6f9097f..46ca69ce2ffe 100644
--- a/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
+++ b/drivers/gpu/drm/ci/xfails/msm-apq8096-fails.txt
@@ -4,6 +4,5 @@ device_reset@unbind-cold-reset-rebind,Fail
device_reset@unbind-reset-rebind,Fail
dumb_buffer@invalid-bpp,Fail
kms_3d,Fail
-kms_addfb_basic@addfb25-bad-modifier,Fail
kms_lease@lease-uevent,Fail
tools_test@tools_test,Fail
---
base-commit: 6b4468b0c6ba37a16795da567b58dc80bc7fb439
change-id: 20240613-msm-pass-addfb25-bad-modifier-c461fd9c02bb
Best regards,
--
Dmitry Baryshkov
[PATCH v4 4/4] arm64: dts: qcom: add HDMI nodes for msm8998
From: Arnaud Vrac
Port device nodes from vendor code.
Signed-off-by: Arnaud Vrac
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Marc Gonzalez
---
arch/arm64/boot/dts/qcom/msm8998.dtsi | 100 +-
1 file changed, 99 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi
b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index ba5e873f0f35f..5c53957da61c5 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -2785,7 +2785,7 @@ mmcc: clock-controller@c8c {
<_dsi0_phy 0>,
<_dsi1_phy 1>,
<_dsi1_phy 0>,
-<0>,
+<_phy 0>,
<0>,
<0>,
< GCC_MMSS_GPLL0_DIV_CLK>;
@@ -2890,6 +2890,14 @@ dpu_intf2_out: endpoint {
remote-endpoint =
<_dsi1_in>;
};
};
+
+ port@2 {
+ reg = <2>;
+
+ dpu_intf3_out: endpoint {
+ remote-endpoint =
<_in>;
+ };
+ };
};
};
@@ -3045,6 +3053,96 @@ mdss_dsi1_phy: phy@c996400 {
status = "disabled";
};
+
+ hdmi: hdmi-tx@c9a {
+ compatible = "qcom,hdmi-tx-8998";
+ reg = <0x0c9a 0x50c>,
+ <0x0078 0x6220>,
+ <0x0c9e 0x2c>;
+ reg-names = "core_physical",
+ "qfprom_physical",
+ "hdcp_physical";
+
+ interrupt-parent = <>;
+ interrupts = <8>;
+
+ clocks = < MDSS_MDP_CLK>,
+< MDSS_AHB_CLK>,
+< MDSS_HDMI_CLK>,
+< MDSS_HDMI_DP_AHB_CLK>,
+< MDSS_EXTPCLK_CLK>,
+< MDSS_AXI_CLK>,
+< MNOC_AHB_CLK>,
+< MISC_AHB_CLK>;
+ clock-names =
+ "mdp_core",
+ "iface",
+ "core",
+ "alt_iface",
+ "extp",
+ "bus",
+ "mnoc",
+ "iface_mmss";
+
+ phys = <_phy>;
+ #sound-dai-cells = <1>;
+
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <_hpd_default
+_ddc_default
+_cec_default>;
+ pinctrl-1 = <_hpd_sleep
+_ddc_default
+_cec_default>;
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ hdmi_in: endpoint {
+ remote-endpoint =
<_intf3_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ hdmi_out: endpoint {
+ };
+ };
+ };
+ };
+
+ hdmi_phy: hdmi-phy@c9a0600 {
+ compatible = "qcom,hdmi-phy-8998";
+ reg = <0x0c9a0600 0x18b>,
+ <0x0c9a0a00 0x38>,
+ <0x0c9a0c00 0x38>,
+
Re: [PATCH v2 3/4] dt-bindings: display/msm: Add SM7150 DPU
On 13/06/2024 12:13, Dmitry Baryshkov wrote: > On Thu, Jun 13, 2024 at 11:23:50AM +0200, Krzysztof Kozlowski wrote: >> On 12/06/2024 20:43, Danila Tikhonov wrote: >>> Document the DPU hardware found on the Qualcomm SM7150 platform. >> >> In general, this should be before MDSS, because it defines fully the >> compatibles already used in the MDSS schema. For multi-binding devices >> it always starts with children and ends with parent/top schema. >> >>> >>> Signed-off-by: Danila Tikhonov >>> --- >>> .../bindings/display/msm/qcom,sm7150-dpu.yaml | 143 ++ >>> 1 file changed, 143 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml >>> b/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml >>> new file mode 100644 >>> index 0..1a44cad131a72 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml >>> @@ -0,0 +1,143 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/display/msm/qcom,sm7150-dpu.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Qualcomm SM7150 Display DPU >> >> What is DPU? Such acronyms should be explained in description or >> expanded here, if there is space. > > Other bindings here use 'DPU', so probably we need to fix all of them at > the same time. Well, we can also start it for new bindings but that's not a reason for resend itself. Best regards, Krzysztof
Re: [PATCH v3 2/4] dt-bindings: display/msm: hdmi: add qcom,hdmi-tx-8998
On Thu, Jun 13, 2024 at 04:02:24AM +0200, Marc Gonzalez wrote: > On 12/06/2024 19:44, Vinod Koul wrote: > > > On 06-06-24, 18:07, Marc Gonzalez wrote: > > > >> HDMI TX block embedded in the APQ8098. > > > > This one too > > I assume this refers to: > "Why is the patch titled display/msm, this is phy patch and it should be > tagged as such." > > I always copy what others have done before me: > > $ git log --oneline Documentation/devicetree/bindings/display/msm/hdmi.yaml > 27339d689d2f9 dt-bindings: display/msm: hdmi: add qcom,hdmi-tx-8998 > 6c04d89a6138a dt-bindings: display/msm: hdmi: mark hdmi-mux-supply as > deprecated > e3c5ce88e8f93 dt-bindings: display/msm: hdmi: mark old GPIO properties as > deprecated > 2f14bc38d88a4 dt-bindings: display/msm: hdmi: split and convert to yaml > > Are you saying we should diverge from the previous nomenclature? This one is fine. For the phy bindings please use phy: prefix. -- With best wishes Dmitry
Re: [PATCH v4 10/13] drm/msm/dpu: allow sharing SSPP between planes
On Wed, Jun 12, 2024 at 06:17:37PM -0700, Abhinav Kumar wrote:
>
>
> On 6/12/2024 2:08 AM, Dmitry Baryshkov wrote:
> > On Wed, 12 Jun 2024 at 02:12, Abhinav Kumar
> > wrote:
> > >
> > >
> > >
> > > On 3/13/2024 5:02 PM, Dmitry Baryshkov wrote:
> > > > Since SmartDMA planes provide two rectangles, it is possible to use them
> > > > to drive two different DRM planes, first plane getting the rect_0,
> > > > another one using rect_1 of the same SSPP. The sharing algorithm is
> > > > pretty simple, it requires that each of the planes can be driven by the
> > > > single rectangle and only consequetive planes are considered.
> > > >
> > >
> > > consequetive - > consecutive
> > >
> > > Can you please explain why only consecutive planes are considered for
> > > this?
> > >
> > > So lets say we have 4 virtual planes : 0, 1, 2, 3
> > >
> > > It will try 0-1, 1-2, 2-3
> > >
> > > Because all planes are virtual, there are only 3 unique pairs to be
> > > considered? Otherwise technically 6 pairs are possible.
> >
> > An implementation that tries all 6 pairs taking the zpos and the
> > overlapping into account is appreciated. I cared for the simplest case
> > here. Yes, further optimizations can be implemented.
> >
>
> Ok got it. So you would like to build a better one on top of this.
> But I see one case where this has an issue or is not optimal. Pls see below.
Yes, it is not optimal. This is the 'best possible effort' or 'best
simple effort' from my POV.
>
> > >
> > >
> > > General request:
> > >
> > > Patches 1-9 : Add support for using 2 SSPPs in one plane
> > > Patches 10-12 : Add support for using two rectangles of the same SSPP as
> > > two virtual planes
> > > Patch 13 : Can be pushed along with the first set.
> > >
> > > Can we break up this series in this way to make it easier to test and
> > > land the bulk of it in this cycle?
> >
> > Sure.
> >
>
> Thanks.
>
> > >
> > > I have some doubts on patches 10-12 and would like to spend more time
> > > reviewing and testing this. So I am trying to reduce the debt of patches
> > > we have been carrying as this is a tricky feature to simulate and test
> > > the cases.
> > >
> > > > Signed-off-by: Dmitry Baryshkov
> > > > ---
> > > >drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 128
> > > > +++---
> > > >1 file changed, 112 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > index cde20c1fa90d..2e1c544efc4a 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > @@ -886,10 +886,9 @@ static int dpu_plane_atomic_check_nopipe(struct
> > > > drm_plane *plane,
> > > >return 0;
> > > >}
> > > >
> > > > -static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe
> > > > *pipe,
> > > > -struct dpu_sw_pipe_cfg
> > > > *pipe_cfg,
> > > > -const struct
> > > > dpu_format *fmt,
> > > > -uint32_t max_linewidth)
> > > > +static int dpu_plane_is_multirect_capable(struct dpu_sw_pipe *pipe,
> > > > + struct dpu_sw_pipe_cfg
> > > > *pipe_cfg,
> > > > + const struct dpu_format *fmt)
> > > >{
> > > >if (drm_rect_width(_cfg->src_rect) !=
> > > > drm_rect_width(_cfg->dst_rect) ||
> > > >drm_rect_height(_cfg->src_rect) !=
> > > > drm_rect_height(_cfg->dst_rect))
> > > > @@ -901,6 +900,13 @@ static int
> > > > dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
> > > >if (DPU_FORMAT_IS_YUV(fmt))
> > > >return false;
> > > >
> > > > + return true;
> > > > +}
> > > > +
> > > > +static int dpu_plane_is_parallel_capable(struct dpu_sw_pipe_cfg
> > > > *pipe_cfg,
> > > > + const struct dpu_format *fmt,
> > > > + uint32_t max_linewidth)
> > > > +{
> > > >if (DPU_FORMAT_IS_UBWC(fmt) &&
> > > >drm_rect_width(_cfg->src_rect) > max_linewidth / 2)
> > > >return false;
> > > > @@ -908,6 +914,82 @@ static int
> > > > dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
> > > >return true;
> > > >}
> > > >
> > > > +static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe
> > > > *pipe,
> > > > +struct dpu_sw_pipe_cfg
> > > > *pipe_cfg,
> > > > +const struct
> > > > dpu_format *fmt,
> > > > +uint32_t max_linewidth)
> > > > +{
> > > > + return dpu_plane_is_multirect_capable(pipe, pipe_cfg, fmt) &&
> > > > + dpu_plane_is_parallel_capable(pipe_cfg, fmt,
> >
[PATCH RFC v2] drm/msm/dpu: Configure DP INTF/PHY selector
sumption is these functions will be called after clocks are enabled.
@@ -125,6 +132,13 @@ struct dpu_hw_mdp_ops {
void (*get_safe_status)(struct dpu_hw_mdp *mdp,
struct dpu_danger_safe_status *status);
+ /**
+* dp_phy_intf_sel - configure intf to phy mapping
+* @mdp: mdp top context driver
+* @phys: list of phys the DP interfaces should be connected to. 0
disables the INTF.
+*/
+ void (*dp_phy_intf_sel)(struct dpu_hw_mdp *mdp, enum dpu_dp_phy_sel
phys[2]);
+
/**
* intf_audio_select - select the external interface for audio
* @mdp: mdp top context driver
@@ -148,12 +162,12 @@ struct dpu_hw_mdp {
* @dev: Corresponding device for devres management
* @cfg: MDP TOP configuration from catalog
* @addr: Mapped register io address of MDP
- * @m:Pointer to mdss catalog data
+ * @mdss_rev: dpu core's major and minor versions
*/
struct dpu_hw_mdp *dpu_hw_mdptop_init(struct drm_device *dev,
const struct dpu_mdp_cfg *cfg,
void __iomem *addr,
- const struct dpu_mdss_cfg *m);
+ const struct dpu_mdss_version *mdss_rev);
void dpu_hw_mdp_destroy(struct dpu_hw_mdp *mdp);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
index 5acd5683d25a..f1acc04089af 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
@@ -60,6 +60,13 @@
#define MDP_WD_TIMER_4_LOAD_VALUE 0x448
#define DCE_SEL 0x450
+#define MDP_DP_PHY_INTF_SEL 0x460
+#define MDP_DP_PHY_INTF_SEL_INTF0 GENMASK(3, 0)
+#define MDP_DP_PHY_INTF_SEL_INTF1 GENMASK(6, 3)
+#define MDP_DP_PHY_INTF_SEL_PHY0 GENMASK(9, 6)
+#define MDP_DP_PHY_INTF_SEL_PHY1 GENMASK(12, 9)
+#define MDP_DP_PHY_INTF_SEL_PHY2 GENMASK(15, 12)
+
#define MDP_PERIPH_TOP0MDP_WD_TIMER_0_CTL
#define MDP_PERIPH_TOP0_ENDCLK_CTRL3
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1955848b1b78..9db5a784c92f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1102,7 +1102,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
dpu_kms->hw_mdp = dpu_hw_mdptop_init(dev,
dpu_kms->catalog->mdp,
dpu_kms->mmio,
-dpu_kms->catalog);
+dpu_kms->catalog->mdss_ver);
if (IS_ERR(dpu_kms->hw_mdp)) {
rc = PTR_ERR(dpu_kms->hw_mdp);
DPU_ERROR("failed to get hw_mdp: %d\n", rc);
@@ -1137,6 +1137,15 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
goto err_pm_put;
}
+ /*
+* We need to program DP <-> PHY relationship only for SC8180X. If any
+* other platform requires the same kind of programming, or if the INTF
+* <->DP relationship isn't static anymore, this needs to be configured
+* through the DT.
+*/
+ if (of_device_is_compatible(dpu_kms->pdev->dev.of_node,
"qcom,sc8180x-dpu"))
+ dpu_kms->hw_mdp->ops.dp_phy_intf_sel(dpu_kms->hw_mdp, (unsigned
int[]){ 1, 2, });
+
dpu_kms->hw_intr = dpu_hw_intr_init(dev, dpu_kms->mmio,
dpu_kms->catalog);
if (IS_ERR(dpu_kms->hw_intr)) {
rc = PTR_ERR(dpu_kms->hw_intr);
---
base-commit: 03d44168cbd7fc57d5de56a3730427db758fc7f6
change-id: 20240613-dp-phy-sel-1b06dc48ed73
Best regards,
--
Dmitry Baryshkov
Re: [PATCH v2 3/4] dt-bindings: display/msm: Add SM7150 DPU
On Thu, Jun 13, 2024 at 11:23:50AM +0200, Krzysztof Kozlowski wrote: > On 12/06/2024 20:43, Danila Tikhonov wrote: > > Document the DPU hardware found on the Qualcomm SM7150 platform. > > In general, this should be before MDSS, because it defines fully the > compatibles already used in the MDSS schema. For multi-binding devices > it always starts with children and ends with parent/top schema. > > > > > Signed-off-by: Danila Tikhonov > > --- > > .../bindings/display/msm/qcom,sm7150-dpu.yaml | 143 ++ > > 1 file changed, 143 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml > > b/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml > > new file mode 100644 > > index 0..1a44cad131a72 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml > > @@ -0,0 +1,143 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/msm/qcom,sm7150-dpu.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm SM7150 Display DPU > > What is DPU? Such acronyms should be explained in description or > expanded here, if there is space. Other bindings here use 'DPU', so probably we need to fix all of them at the same time. > > Reviewed-by: Krzysztof Kozlowski > > > + > > +maintainers: > > + - Danila Tikhonov > > + > > +$ref: /schemas/display/msm/dpu-common.yaml# > > + > > +properties: > > + compatible: > > +const: qcom,sm7150-dpu > > + > > > > Best regards, > Krzysztof > -- With best wishes Dmitry
Re: [PATCH v2 1/4] dt-bindings: display/msm: Add SM7150 MDSS
On 12/06/2024 20:43, Danila Tikhonov wrote: > Document the MDSS hardware found on the Qualcomm SM7150 platform. > > Signed-off-by: Danila Tikhonov > --- Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 3/4] dt-bindings: display/msm: Add SM7150 DPU
On 12/06/2024 20:43, Danila Tikhonov wrote: > Document the DPU hardware found on the Qualcomm SM7150 platform. In general, this should be before MDSS, because it defines fully the compatibles already used in the MDSS schema. For multi-binding devices it always starts with children and ends with parent/top schema. > > Signed-off-by: Danila Tikhonov > --- > .../bindings/display/msm/qcom,sm7150-dpu.yaml | 143 ++ > 1 file changed, 143 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml > > diff --git > a/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml > new file mode 100644 > index 0..1a44cad131a72 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm7150-dpu.yaml > @@ -0,0 +1,143 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/msm/qcom,sm7150-dpu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm SM7150 Display DPU What is DPU? Such acronyms should be explained in description or expanded here, if there is space. Reviewed-by: Krzysztof Kozlowski > + > +maintainers: > + - Danila Tikhonov > + > +$ref: /schemas/display/msm/dpu-common.yaml# > + > +properties: > + compatible: > +const: qcom,sm7150-dpu > + Best regards, Krzysztof
Re: [PATCH v4 1/2] iommu/io-pgtable-arm: Add way to debug pgtable walk
On Thu, May 23, 2024 at 10:52:21AM -0700, Rob Clark wrote: > From: Rob Clark > > Add an io-pgtable method to walk the pgtable returning the raw PTEs that > would be traversed for a given iova access. > > Signed-off-by: Rob Clark > --- > drivers/iommu/io-pgtable-arm.c | 51 -- > include/linux/io-pgtable.h | 4 +++ > 2 files changed, 46 insertions(+), 9 deletions(-) Acked-by: Joerg Roedel
Re: [PATCH v5 2/9] drm/bridge-connector: switch to using drmm allocations
On Tue, Jun 11, 2024 at 02:26:12PM GMT, Dmitry Baryshkov wrote:
> On Tue, 11 Jun 2024 at 11:54, Maxime Ripard wrote:
> >
> > On Mon, Jun 10, 2024 at 08:54:09PM GMT, Dmitry Baryshkov wrote:
> > > On Mon, Jun 10, 2024 at 02:07:06PM +0200, Maxime Ripard wrote:
> > > > Hi,
> > > >
> > > > +Hans
> > > >
> > > > On Mon, Jun 10, 2024 at 02:46:03PM GMT, Dmitry Baryshkov wrote:
> > > > > On Mon, 10 Jun 2024 at 11:04, Maxime Ripard
> > > > > wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Fri, Jun 07, 2024 at 04:22:59PM GMT, Dmitry Baryshkov wrote:
> > > > > > > Turn drm_bridge_connector to using drmm_kzalloc() and
> > > > > > > drmm_connector_init() and drop the custom destroy function. The
> > > > > > > drm_connector_unregister() and fwnode_handle_put() are already
> > > > > > > handled
> > > > > > > by the drm_connector_cleanup() and so are safe to be dropped.
> > > > > > >
> > > > > > > Acked-by: Maxime Ripard
> > > > > > > Signed-off-by: Dmitry Baryshkov
> > > > > > > ---
> > > > > > > drivers/gpu/drm/drm_bridge_connector.c | 23
> > > > > > > +--
> > > > > > > 1 file changed, 5 insertions(+), 18 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c
> > > > > > > b/drivers/gpu/drm/drm_bridge_connector.c
> > > > > > > index 982552c9f92c..e093fc8928dc 100644
> > > > > > > --- a/drivers/gpu/drm/drm_bridge_connector.c
> > > > > > > +++ b/drivers/gpu/drm/drm_bridge_connector.c
> > > > > > > @@ -15,6 +15,7 @@
> > > > > > > #include
> > > > > > > #include
> > > > > > > #include
> > > > > > > +#include
> > > > > > > #include
> > > > > > > #include
> > > > > > >
> > > > > > > @@ -193,19 +194,6 @@ drm_bridge_connector_detect(struct
> > > > > > > drm_connector *connector, bool force)
> > > > > > > return status;
> > > > > > > }
> > > > > > >
> > > > > > > -static void drm_bridge_connector_destroy(struct drm_connector
> > > > > > > *connector)
> > > > > > > -{
> > > > > > > - struct drm_bridge_connector *bridge_connector =
> > > > > > > - to_drm_bridge_connector(connector);
> > > > > > > -
> > > > > > > - drm_connector_unregister(connector);
> > > > > > > - drm_connector_cleanup(connector);
> > > > > > > -
> > > > > > > - fwnode_handle_put(connector->fwnode);
> > > > > > > -
> > > > > > > - kfree(bridge_connector);
> > > > > > > -}
> > > > > > > -
> > > > > > > static void drm_bridge_connector_debugfs_init(struct
> > > > > > > drm_connector *connector,
> > > > > > > struct dentry *root)
> > > > > > > {
> > > > > > > @@ -224,7 +212,6 @@ static const struct drm_connector_funcs
> > > > > > > drm_bridge_connector_funcs = {
> > > > > > > .reset = drm_atomic_helper_connector_reset,
> > > > > > > .detect = drm_bridge_connector_detect,
> > > > > > > .fill_modes = drm_helper_probe_single_connector_modes,
> > > > > > > - .destroy = drm_bridge_connector_destroy,
> > > > > > > .atomic_duplicate_state =
> > > > > > > drm_atomic_helper_connector_duplicate_state,
> > > > > > > .atomic_destroy_state =
> > > > > > > drm_atomic_helper_connector_destroy_state,
> > > > > > > .debugfs_init = drm_bridge_connector_debugfs_init,
> > > > > > > @@ -328,7 +315,7 @@ struct drm_connector
> > > > > > > *drm_bridge_connector_init(struct drm_device *drm,
> > > > > > > int connector_type;
> > > > > > > int ret;
> > > > > > >
> > > > > > > - bridge_connector = kzalloc(sizeof(*bridge_connector),
> > > > > > > GFP_KERNEL);
> > > > > > > + bridge_connector = drmm_kzalloc(drm,
> > > > > > > sizeof(*bridge_connector), GFP_KERNEL);
> > > > > >
> > > > > > So you make destroy's kfree call unnecessary here ...
> > > > > >
> > > > > > > if (!bridge_connector)
> > > > > > > return ERR_PTR(-ENOMEM);
> > > > > > >
> > > > > > > @@ -383,9 +370,9 @@ struct drm_connector
> > > > > > > *drm_bridge_connector_init(struct drm_device *drm,
> > > > > > > return ERR_PTR(-EINVAL);
> > > > > > > }
> > > > > > >
> > > > > > > - ret = drm_connector_init_with_ddc(drm, connector,
> > > > > > > -
> > > > > > > _bridge_connector_funcs,
> > > > > > > - connector_type, ddc);
> > > > > > > + ret = drmm_connector_init(drm, connector,
> > > > > > > + _bridge_connector_funcs,
> > > > > > > + connector_type, ddc);
> > > > > >
> > > > > > ... and here of drm_connector_cleanup.
> > > > > >
> > > > > > drm_connector_unregister wasn't needed, so can ignore it, but you
> > > > > > leak a reference to
> > > > > > connector->fwnode since you don't call fwnode_handle_put anymore.
> > > > > >
> > > > > > We should register a drmm action right below the call to
> > > > > > fwnode_handle_get too.
> > > > >
> > > > > But drm_connector_cleanup() already contains
> > > > >
