Re: [Freedreno] [PATCH 1/3] drm/msm/dp: drop dp_mode argument from dp_panel_get_modes()
On 3/23/2022 3:35 AM, Dmitry Baryshkov wrote:
Since the commit ab205927592b ("drm/msm/dp: remove mode hard-coding in
case of DP CTS") the function dp_panel_get_modes() doesn't use (or fill)
the dp_mode argument. Drop it completely.
Signed-off-by: Dmitry Baryshkov
---
Reviewed-by: Kuogee Hsieh
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
drivers/gpu/drm/msm/dp/dp_panel.c | 2 +-
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 178b774a5fbd..3dd790083cf7 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1013,7 +1013,7 @@ int dp_display_get_modes(struct msm_dp *dp,
dp_display = container_of(dp, struct dp_display_private, dp_display);
ret = dp_panel_get_modes(dp_display->panel,
- dp->connector, dp_mode);
+ dp->connector);
if (dp_mode->drm_mode.clock)
dp->max_pclk_khz = dp_mode->drm_mode.clock;
return ret;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index f1418722c549..8d7662028f30 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -259,7 +259,7 @@ u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel,
}
int dp_panel_get_modes(struct dp_panel *dp_panel,
- struct drm_connector *connector, struct dp_display_mode *mode)
+ struct drm_connector *connector)
{
if (!dp_panel) {
DRM_ERROR("invalid input\n");
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9023e5bb4b8b..9fa6e524832c 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -64,7 +64,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel, u32 mode_max_bpp,
u32 mode_pclk_khz);
int dp_panel_get_modes(struct dp_panel *dp_panel,
- struct drm_connector *connector, struct dp_display_mode *mode);
+ struct drm_connector *connector);
void dp_panel_handle_sink_request(struct dp_panel *dp_panel);
void dp_panel_tpg_config(struct dp_panel *dp_panel, bool enable);
Re: [Freedreno] [PATCH 00/22] drm: Review of mode copies
On 3/23/2022 3:39 AM, Dmitry Baryshkov wrote: On 22/03/2022 01:37, Ville Syrjälä wrote: On Tue, Mar 15, 2022 at 02:52:38PM -0400, Alex Deucher wrote: On Mon, Mar 14, 2022 at 6:12 PM Ville Syrjälä wrote: On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote: drm: Add drm_mode_init() drm/bridge: Use drm_mode_copy() drm/imx: Use drm_mode_duplicate() drm/panel: Use drm_mode_duplicate() drm/vc4: Use drm_mode_copy() These have been pushed to drm-misc-next. drm/amdgpu: Remove pointless on stack mode copies drm/amdgpu: Use drm_mode_init() for on-stack modes drm/amdgpu: Use drm_mode_copy() amdgpu ones are reviewed, but I'll leave them for the AMD folks to push to whichever tree they prefer. I pulled patches 2, 4, 5 into my tree. Thanks. For 3, I'm happy to have it land via drm-misc with the rest of the mode_init changes if you'd prefer. Either way works for me. I don't yet have reviews yet for the other drivers, so I'll proably hold off for a bit more at least. And the i915 patch I'll be merging via drm-intel. drm/radeon: Use drm_mode_copy() drm/gma500: Use drm_mode_copy() drm/tilcdc: Use drm_mode_copy() drm/i915: Use drm_mode_copy() Those are now all in. Which leaves us with these stragglers: drm/hisilicon: Use drm_mode_init() for on-stack modes drm/msm: Nuke weird on stack mode copy drm/msm: Use drm_mode_init() for on-stack modes drm/msm: Use drm_mode_copy() These three patches went beneath my radar for some reason. I have just sent a replacement for the first patch ([1]). Other two patches can be pulled in msm/msm-next (or msm/msm-fixes) during the next development cycle if that works for you. [1] https://patchwork.freedesktop.org/series/101682/ Agree with this approach. We can replace the first patch with https://patchwork.freedesktop.org/series/101682/. Thanks Abhinav drm/mtk: Use drm_mode_init() for on-stack modes drm/rockchip: Use drm_mode_copy() drm/sti: Use drm_mode_copy() drm: Use drm_mode_init() for on-stack modes drm: Use drm_mode_copy()
Re: [Freedreno] [PATCH 0/3] drm/msm/dp: simplify dp_connector_get_modes()
On 3/23/2022 3:35 AM, Dmitry Baryshkov wrote: As noted by Ville Syrjälä [1] the dp_connector_get_modes() has weird on-stack interim copy of the mode. When reviewing the mentioned patch I noticed that the rest of the code in dp_connector_get_modes() is weird, unused since September of 2020 and can be removed. This series removes dead weird code leaving dp_connector_get_modes() simple enough. [1] https://lore.kernel.org/linux-arm-msm/20220218100403.7028-11-ville.syrj...@linux.intel.com/ For the series, Reviewed-by: Abhinav Kumar *** BLURB HERE *** Dmitry Baryshkov (3): drm/msm/dp: drop dp_mode argument from dp_panel_get_modes() drm/msm/dp: simplify dp_connector_get_modes() drm/msm/dp: remove max_pclk_khz field from dp_panel/dp_display drivers/gpu/drm/msm/dp/dp_debug.c | 2 -- drivers/gpu/drm/msm/dp/dp_display.c | 11 +++--- drivers/gpu/drm/msm/dp/dp_display.h | 5 + drivers/gpu/drm/msm/dp/dp_drm.c | 33 ++--- drivers/gpu/drm/msm/dp/dp_panel.c | 2 +- drivers/gpu/drm/msm/dp/dp_panel.h | 3 +-- 6 files changed, 8 insertions(+), 48 deletions(-)
Re: [Freedreno] [PATCH 3/3] drm/msm/dp: remove max_pclk_khz field from dp_panel/dp_display
On 3/23/2022 3:35 AM, Dmitry Baryshkov wrote:
Since the last commit, the max_pclk_khz became constant, it's set to
DP_MAX_PIXEL_CLK_KHZ and never changed afterwards. Remove it completely
and use DP_MAX_PIXEL_CLK_KHZ directly.
Signed-off-by: Dmitry Baryshkov
---
Reviewed-by: Kuogee Hsieh
drivers/gpu/drm/msm/dp/dp_debug.c | 2 --
drivers/gpu/drm/msm/dp/dp_display.c | 1 -
drivers/gpu/drm/msm/dp/dp_display.h | 2 --
drivers/gpu/drm/msm/dp/dp_drm.c | 4 +---
drivers/gpu/drm/msm/dp/dp_panel.h | 1 -
5 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
b/drivers/gpu/drm/msm/dp/dp_debug.c
index 2f9c943f12d5..5e35033ba3e4 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -44,8 +44,6 @@ static int dp_debug_show(struct seq_file *seq, void *p)
drm_mode = >panel->dp_mode.drm_mode;
seq_printf(seq, "\tname = %s\n", DEBUG_NAME);
- seq_printf(seq, "\tdp_panel\n\t\tmax_pclk_khz = %d\n",
- debug->panel->max_pclk_khz);
seq_printf(seq, "\tdrm_dp_link\n\t\trate = %u\n",
debug->panel->link_info.rate);
seq_printf(seq, "\t\tnum_lanes = %u\n",
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index be1e9c558fda..aeb08416b309 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -370,7 +370,6 @@ static int dp_display_process_hpd_high(struct
dp_display_private *dp)
dp->audio_supported = drm_detect_monitor_audio(edid);
dp_panel_handle_sink_request(dp->panel);
- dp->dp_display.max_pclk_khz = DP_MAX_PIXEL_CLK_KHZ;
dp->dp_display.max_dp_lanes = dp->parser->max_dp_lanes;
/*
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 6efb5c853c89..f3a9d7449a1c 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -24,8 +24,6 @@ struct msm_dp {
hdmi_codec_plugged_cb plugged_cb;
- u32 max_pclk_khz;
-
u32 max_dp_lanes;
struct dp_audio *dp_audio;
};
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index af5f1b001192..a94c9b34f397 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -88,9 +88,7 @@ static enum drm_mode_status dp_connector_mode_valid(
dp_disp = to_dp_connector(connector)->dp_display;
- if ((dp_disp->max_pclk_khz <= 0) ||
- (dp_disp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) ||
- (mode->clock > dp_disp->max_pclk_khz))
+ if (mode->clock > DP_MAX_PIXEL_CLK_KHZ)
return MODE_BAD;
return dp_display_validate_mode(dp_disp, mode->clock);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9fa6e524832c..d861197ac1c8 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -49,7 +49,6 @@ struct dp_panel {
bool video_test;
u32 vic;
- u32 max_pclk_khz;
u32 max_dp_lanes;
u32 max_bw_code;
Re: [Freedreno] [PATCH 1/3] drm/msm/dp: drop dp_mode argument from dp_panel_get_modes()
On 23/03/2022 23:46, Kuogee Hsieh wrote:
On 3/23/2022 3:35 AM, Dmitry Baryshkov wrote:
Since the commit ab205927592b ("drm/msm/dp: remove mode hard-coding in
case of DP CTS") the function dp_panel_get_modes() doesn't use (or fill)
the dp_mode argument. Drop it completely.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Kuogee Hsieh
There was an extra >, so this line wasn't picked up by patchwork.
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
drivers/gpu/drm/msm/dp/dp_panel.c | 2 +-
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 178b774a5fbd..3dd790083cf7 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1013,7 +1013,7 @@ int dp_display_get_modes(struct msm_dp *dp,
dp_display = container_of(dp, struct dp_display_private,
dp_display);
ret = dp_panel_get_modes(dp_display->panel,
- dp->connector, dp_mode);
+ dp->connector);
if (dp_mode->drm_mode.clock)
dp->max_pclk_khz = dp_mode->drm_mode.clock;
return ret;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index f1418722c549..8d7662028f30 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -259,7 +259,7 @@ u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel,
}
int dp_panel_get_modes(struct dp_panel *dp_panel,
- struct drm_connector *connector, struct dp_display_mode *mode)
+ struct drm_connector *connector)
{
if (!dp_panel) {
DRM_ERROR("invalid input\n");
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9023e5bb4b8b..9fa6e524832c 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -64,7 +64,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel, u32 mode_max_bpp,
u32 mode_pclk_khz);
int dp_panel_get_modes(struct dp_panel *dp_panel,
- struct drm_connector *connector, struct dp_display_mode *mode);
+ struct drm_connector *connector);
void dp_panel_handle_sink_request(struct dp_panel *dp_panel);
void dp_panel_tpg_config(struct dp_panel *dp_panel, bool enable);
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH 2/3] drm/msm/dp: simplify dp_connector_get_modes()
On 3/23/2022 3:35 AM, Dmitry Baryshkov wrote:
Since dp_panel_get_modes() handling for dp_mode was removed,
dp_display_get_modes also doesn't change the passed dp_mode, drop the
unused dp_mode variable being allocated unused and then freed.
Signed-off-by: Dmitry Baryshkov
---
Reviewed-by: Kuogee Hsieh
drivers/gpu/drm/msm/dp/dp_display.c | 8 ++--
drivers/gpu/drm/msm/dp/dp_display.h | 3 +--
drivers/gpu/drm/msm/dp/dp_drm.c | 29 +
3 files changed, 4 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 3dd790083cf7..be1e9c558fda 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -999,8 +999,7 @@ int dp_display_validate_mode(struct msm_dp *dp, u32
mode_pclk_khz)
return MODE_OK;
}
-int dp_display_get_modes(struct msm_dp *dp,
- struct dp_display_mode *dp_mode)
+int dp_display_get_modes(struct msm_dp *dp)
{
struct dp_display_private *dp_display;
int ret = 0;
@@ -1012,11 +1011,8 @@ int dp_display_get_modes(struct msm_dp *dp,
dp_display = container_of(dp, struct dp_display_private, dp_display);
- ret = dp_panel_get_modes(dp_display->panel,
+ return dp_panel_get_modes(dp_display->panel,
dp->connector);
- if (dp_mode->drm_mode.clock)
- dp->max_pclk_khz = dp_mode->drm_mode.clock;
- return ret;
}
bool dp_display_check_video_test(struct msm_dp *dp)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 7af2b186d2d9..6efb5c853c89 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -33,8 +33,7 @@ struct msm_dp {
int dp_display_set_plugged_cb(struct msm_dp *dp_display,
hdmi_codec_plugged_cb fn, struct device *codec_dev);
int dp_display_validate_mode(struct msm_dp *dp_display, u32 mode_pclk_khz);
-int dp_display_get_modes(struct msm_dp *dp_display,
- struct dp_display_mode *dp_mode);
+int dp_display_get_modes(struct msm_dp *dp_display);
int dp_display_request_irq(struct msm_dp *dp_display);
bool dp_display_check_video_test(struct msm_dp *dp_display);
int dp_display_get_test_bpp(struct msm_dp *dp_display);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 80f59cf99089..af5f1b001192 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -55,49 +55,22 @@ static int dp_connector_get_modes(struct drm_connector
*connector)
{
int rc = 0;
struct msm_dp *dp;
- struct dp_display_mode *dp_mode = NULL;
- struct drm_display_mode *m, drm_mode;
if (!connector)
return 0;
dp = to_dp_connector(connector)->dp_display;
- dp_mode = kzalloc(sizeof(*dp_mode), GFP_KERNEL);
- if (!dp_mode)
- return 0;
-
/* pluggable case assumes EDID is read when HPD */
if (dp->is_connected) {
- /*
-*The get_modes() function might return one mode that is stored
-* in dp_mode when compliance test is in progress. If not, the
-* return value is equal to the total number of modes supported
-* by the sink
-*/
- rc = dp_display_get_modes(dp, dp_mode);
+ rc = dp_display_get_modes(dp);
if (rc <= 0) {
DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
- kfree(dp_mode);
return rc;
}
- if (dp_mode->drm_mode.clock) { /* valid DP mode */
- memset(_mode, 0x0, sizeof(drm_mode));
- drm_mode_copy(_mode, _mode->drm_mode);
- m = drm_mode_duplicate(connector->dev, _mode);
- if (!m) {
- DRM_ERROR("failed to add mode %ux%u\n",
- drm_mode.hdisplay,
- drm_mode.vdisplay);
- kfree(dp_mode);
- return 0;
- }
- drm_mode_probed_add(connector, m);
- }
} else {
DRM_DEBUG_DP("No sink connected\n");
}
- kfree(dp_mode);
return rc;
}
Re: [Freedreno] [PATCH 1/3] drm/msm/dp: drop dp_mode argument from dp_panel_get_modes()
On 3/23/2022 3:35 AM, Dmitry Baryshkov wrote:
Since the commit ab205927592b ("drm/msm/dp: remove mode hard-coding in
case of DP CTS") the function dp_panel_get_modes() doesn't use (or fill)
the dp_mode argument. Drop it completely.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
drivers/gpu/drm/msm/dp/dp_panel.c | 2 +-
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 178b774a5fbd..3dd790083cf7 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1013,7 +1013,7 @@ int dp_display_get_modes(struct msm_dp *dp,
dp_display = container_of(dp, struct dp_display_private, dp_display);
ret = dp_panel_get_modes(dp_display->panel,
- dp->connector, dp_mode);
+ dp->connector);
if (dp_mode->drm_mode.clock)
dp->max_pclk_khz = dp_mode->drm_mode.clock;
return ret;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index f1418722c549..8d7662028f30 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -259,7 +259,7 @@ u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel,
}
int dp_panel_get_modes(struct dp_panel *dp_panel,
- struct drm_connector *connector, struct dp_display_mode *mode)
+ struct drm_connector *connector)
{
if (!dp_panel) {
DRM_ERROR("invalid input\n");
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9023e5bb4b8b..9fa6e524832c 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -64,7 +64,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel, u32 mode_max_bpp,
u32 mode_pclk_khz);
int dp_panel_get_modes(struct dp_panel *dp_panel,
- struct drm_connector *connector, struct dp_display_mode *mode);
+ struct drm_connector *connector);
void dp_panel_handle_sink_request(struct dp_panel *dp_panel);
void dp_panel_tpg_config(struct dp_panel *dp_panel, bool enable);
Re: [Freedreno] [PATCH 12/22] drm/msm: Use drm_mode_copy()
On 2/18/2022 2:03 AM, Ville Syrjala wrote:
From: Ville Syrjälä
struct drm_display_mode embeds a list head, so overwriting
the full struct with another one will corrupt the list
(if the destination mode is on a list). Use drm_mode_copy()
instead which explicitly preserves the list head of
the destination mode.
Even if we know the destination mode is not on any list
using drm_mode_copy() seems decent as it sets a good
example. Bad examples of not using it might eventually
get copied into code where preserving the list head
actually matters.
Obviously one case not covered here is when the mode
itself is embedded in a larger structure and the whole
structure is copied. But if we are careful when copying
into modes embedded in structures I think we can be a
little more reassured that bogus list heads haven't been
propagated in.
@is_mode_copy@
@@
drm_mode_copy(...)
{
...
}
@depends on !is_mode_copy@
struct drm_display_mode *mode;
expression E, S;
@@
(
- *mode = E
+ drm_mode_copy(mode, )
|
- memcpy(mode, E, S)
+ drm_mode_copy(mode, E)
)
@depends on !is_mode_copy@
struct drm_display_mode mode;
expression E;
@@
(
- mode = E
+ drm_mode_copy(, )
|
- memcpy(, E, S)
+ drm_mode_copy(, E)
)
@@
struct drm_display_mode *mode;
@@
- &*mode
+ mode
Cc: Rob Clark
Cc: Sean Paul
Cc: Abhinav Kumar
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Ville Syrjälä
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 34a6940d12c5..57592052af23 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -157,7 +157,7 @@ static void dpu_encoder_phys_cmd_mode_set(
DPU_ERROR("invalid args\n");
return;
}
- phys_enc->cached_mode = *adj_mode;
+ drm_mode_copy(_enc->cached_mode, adj_mode);
DPU_DEBUG_CMDENC(cmd_enc, "caching mode:\n");
drm_mode_debug_printmodeline(adj_mode);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index e7813c6f7bd9..d5deca07b65a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -370,7 +370,7 @@ static void dpu_encoder_phys_vid_mode_set(
struct dpu_encoder_irq *irq;
if (adj_mode) {
- phys_enc->cached_mode = *adj_mode;
+ drm_mode_copy(_enc->cached_mode, adj_mode);
drm_mode_debug_printmodeline(adj_mode);
DPU_DEBUG_VIDENC(phys_enc, "caching mode:\n");
}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 7cc4d21f2091..2ed6028ca8d6 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -825,7 +825,7 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
dp = container_of(dp_display, struct dp_display_private, dp_display);
- dp->panel->dp_mode.drm_mode = mode->drm_mode;
+ drm_mode_copy(>panel->dp_mode.drm_mode, >drm_mode);
dp->panel->dp_mode.bpp = mode->bpp;
dp->panel->dp_mode.capabilities = mode->capabilities;
dp_panel_init_panel_info(dp->panel);
Re: [Freedreno] [PATCH 11/22] drm/msm: Use drm_mode_init() for on-stack modes
On 2/18/2022 2:03 AM, Ville Syrjala wrote:
From: Ville Syrjälä
Initialize on-stack modes with drm_mode_init() to guarantee
no stack garbage in the list head, or that we aren't copying
over another mode's list head.
Based on the following cocci script, with manual fixups:
@decl@
identifier M;
expression E;
@@
- struct drm_display_mode M = E;
+ struct drm_display_mode M;
@@
identifier decl.M;
expression decl.E;
statement S, S1;
@@
struct drm_display_mode M;
... when != S
+ drm_mode_init(, );
+
S1
@@
expression decl.E;
@@
- &*E
+ E
Cc: Rob Clark
Cc: Sean Paul
Cc: Abhinav Kumar
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Ville Syrjälä
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index ddd9d89cd456..e7813c6f7bd9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -248,12 +248,13 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
unsigned long lock_flags;
struct dpu_hw_intf_cfg intf_cfg = { 0 };
+ drm_mode_init(, _enc->cached_mode);
+
if (!phys_enc->hw_ctl->ops.setup_intf_cfg) {
DPU_ERROR("invalid encoder %d\n", phys_enc != NULL);
return;
}
- mode = phys_enc->cached_mode;
if (!phys_enc->hw_intf->ops.setup_timing_gen) {
DPU_ERROR("timing engine setup is not supported\n");
return;
@@ -652,7 +653,9 @@ static int dpu_encoder_phys_vid_get_frame_count(
{
struct intf_status s = {0};
u32 fetch_start = 0;
- struct drm_display_mode mode = phys_enc->cached_mode;
+ struct drm_display_mode mode;
+
+ drm_mode_init(, _enc->cached_mode);
if (!dpu_encoder_phys_vid_is_master(phys_enc))
return -EINVAL;
Re: [Freedreno] [PATCH v5 2/4] drm: introduce drm_writeback_connector_init_with_encoder() API
Hi Liviu
Thanks for the review.
On 3/23/2022 9:46 AM, Liviu Dudau wrote:
On Mon, Mar 21, 2022 at 04:56:43PM -0700, Abhinav Kumar wrote:
For vendors drivers which pass an already allocated and
initialized encoder especially for cases where the encoder
hardware is shared OR the writeback encoder shares the resources
with the rest of the display pipeline introduce a new API,
drm_writeback_connector_init_with_encoder() which expects
an initialized encoder as a parameter and only sets up the
writeback connector.
changes in v5:
- reorder this change to come before in the series
to avoid incorrect functionality in subsequent changes
- continue using struct drm_encoder instead of
struct drm_encoder * and switch it in next change
Signed-off-by: Abhinav Kumar
Hi Abhinav,
---
drivers/gpu/drm/drm_writeback.c | 143
include/drm/drm_writeback.h | 5 ++
2 files changed, 106 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dc2ef12..abe78b9 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -149,37 +149,15 @@ static const struct drm_encoder_funcs
drm_writeback_encoder_funcs = {
.destroy = drm_encoder_cleanup,
};
-/**
- * drm_writeback_connector_init - Initialize a writeback connector and its
properties
- * @dev: DRM device
- * @wb_connector: Writeback connector to initialize
- * @con_funcs: Connector funcs vtable
- * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal
encoder
- * @formats: Array of supported pixel formats for the writeback engine
- * @n_formats: Length of the formats array
- * @possible_crtcs: possible crtcs for the internal writeback encoder
- *
- * This function creates the writeback-connector-specific properties if they
- * have not been already created, initializes the connector as
- * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
- * values. It will also create an internal encoder associated with the
- * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
- * the encoder helper.
- *
- * Drivers should always use this function instead of drm_connector_init() to
- * set up writeback connectors.
- *
- * Returns: 0 on success, or a negative error code
- */
-int drm_writeback_connector_init(struct drm_device *dev,
-struct drm_writeback_connector *wb_connector,
-const struct drm_connector_funcs *con_funcs,
-const struct drm_encoder_helper_funcs
*enc_helper_funcs,
-const u32 *formats, int n_formats, uint32_t
possible_crtcs)
+static int drm_writeback_connector_setup(struct drm_device *dev,
+ struct drm_writeback_connector *wb_connector,
+ const struct drm_connector_funcs *con_funcs, const u32 *formats,
+ int n_formats)
{
struct drm_property_blob *blob;
- struct drm_connector *connector = _connector->base;
struct drm_mode_config *config = >mode_config;
+ struct drm_connector *connector = _connector->base;
+
Point of this reordering the statements is...?
This diff can be avoided. There was no reason to reorder this. I will
remove this re-order.
int ret = create_writeback_properties(dev);
if (ret != 0)
@@ -187,18 +165,10 @@ int drm_writeback_connector_init(struct drm_device *dev,
blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
formats);
- if (IS_ERR(blob))
- return PTR_ERR(blob);
-
- drm_encoder_helper_add(_connector->encoder, enc_helper_funcs);
-
- wb_connector->encoder.possible_crtcs = possible_crtcs;
-
- ret = drm_encoder_init(dev, _connector->encoder,
- _writeback_encoder_funcs,
- DRM_MODE_ENCODER_VIRTUAL, NULL);
- if (ret)
- goto fail;
+ if (IS_ERR(blob)) {
+ ret = PTR_ERR(blob);
+ return ret;
+ }
I don't see why you have changed the earlier code to store the result of
PTR_ERR into
ret other than to help your debugging. I suggest that you keep the existing
code that
returns PTR_ERR(blob) directly and you will have a nicer diff stat as well.
Sure, i can fix this for a smaller diff stat.
connector->interlace_allowed = 0;
@@ -237,13 +207,102 @@ int drm_writeback_connector_init(struct drm_device *dev,
attach_fail:
drm_connector_cleanup(connector);
connector_fail:
- drm_encoder_cleanup(_connector->encoder);
-fail:
drm_property_blob_put(blob);
return ret;
}
+
+/**
+ * drm_writeback_connector_init - Initialize a writeback connector and its
properties
+ * @dev: DRM device
+ * @wb_connector: Writeback connector to initialize
+ *
Re: [Freedreno] [PATCH v5 2/4] drm: introduce drm_writeback_connector_init_with_encoder() API
On Mon, Mar 21, 2022 at 04:56:43PM -0700, Abhinav Kumar wrote:
> For vendors drivers which pass an already allocated and
> initialized encoder especially for cases where the encoder
> hardware is shared OR the writeback encoder shares the resources
> with the rest of the display pipeline introduce a new API,
> drm_writeback_connector_init_with_encoder() which expects
> an initialized encoder as a parameter and only sets up the
> writeback connector.
>
> changes in v5:
> - reorder this change to come before in the series
> to avoid incorrect functionality in subsequent changes
> - continue using struct drm_encoder instead of
> struct drm_encoder * and switch it in next change
>
> Signed-off-by: Abhinav Kumar
Hi Abhinav,
> ---
> drivers/gpu/drm/drm_writeback.c | 143
>
> include/drm/drm_writeback.h | 5 ++
> 2 files changed, 106 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index dc2ef12..abe78b9 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -149,37 +149,15 @@ static const struct drm_encoder_funcs
> drm_writeback_encoder_funcs = {
> .destroy = drm_encoder_cleanup,
> };
>
> -/**
> - * drm_writeback_connector_init - Initialize a writeback connector and its
> properties
> - * @dev: DRM device
> - * @wb_connector: Writeback connector to initialize
> - * @con_funcs: Connector funcs vtable
> - * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal
> encoder
> - * @formats: Array of supported pixel formats for the writeback engine
> - * @n_formats: Length of the formats array
> - * @possible_crtcs: possible crtcs for the internal writeback encoder
> - *
> - * This function creates the writeback-connector-specific properties if they
> - * have not been already created, initializes the connector as
> - * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
> - * values. It will also create an internal encoder associated with the
> - * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
> - * the encoder helper.
> - *
> - * Drivers should always use this function instead of drm_connector_init() to
> - * set up writeback connectors.
> - *
> - * Returns: 0 on success, or a negative error code
> - */
> -int drm_writeback_connector_init(struct drm_device *dev,
> - struct drm_writeback_connector *wb_connector,
> - const struct drm_connector_funcs *con_funcs,
> - const struct drm_encoder_helper_funcs
> *enc_helper_funcs,
> - const u32 *formats, int n_formats, uint32_t
> possible_crtcs)
> +static int drm_writeback_connector_setup(struct drm_device *dev,
> + struct drm_writeback_connector *wb_connector,
> + const struct drm_connector_funcs *con_funcs, const u32 *formats,
> + int n_formats)
> {
> struct drm_property_blob *blob;
> - struct drm_connector *connector = _connector->base;
> struct drm_mode_config *config = >mode_config;
> + struct drm_connector *connector = _connector->base;
> +
Point of this reordering the statements is...?
> int ret = create_writeback_properties(dev);
>
> if (ret != 0)
> @@ -187,18 +165,10 @@ int drm_writeback_connector_init(struct drm_device *dev,
>
> blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
> formats);
> - if (IS_ERR(blob))
> - return PTR_ERR(blob);
> -
> - drm_encoder_helper_add(_connector->encoder, enc_helper_funcs);
> -
> - wb_connector->encoder.possible_crtcs = possible_crtcs;
> -
> - ret = drm_encoder_init(dev, _connector->encoder,
> -_writeback_encoder_funcs,
> -DRM_MODE_ENCODER_VIRTUAL, NULL);
> - if (ret)
> - goto fail;
> + if (IS_ERR(blob)) {
> + ret = PTR_ERR(blob);
> + return ret;
> + }
I don't see why you have changed the earlier code to store the result of
PTR_ERR into
ret other than to help your debugging. I suggest that you keep the existing
code that
returns PTR_ERR(blob) directly and you will have a nicer diff stat as well.
>
> connector->interlace_allowed = 0;
>
> @@ -237,13 +207,102 @@ int drm_writeback_connector_init(struct drm_device
> *dev,
> attach_fail:
> drm_connector_cleanup(connector);
> connector_fail:
> - drm_encoder_cleanup(_connector->encoder);
> -fail:
> drm_property_blob_put(blob);
> return ret;
> }
> +
> +/**
> + * drm_writeback_connector_init - Initialize a writeback connector and its
> properties
> + * @dev: DRM device
> + * @wb_connector: Writeback connector to initialize
> + * @con_funcs: Connector funcs vtable
> + * @enc_helper_funcs: Encoder helper funcs
Re: [Freedreno] [PATCH 00/22] drm: Review of mode copies
On Wed, Mar 23, 2022 at 01:39:44PM +0300, Dmitry Baryshkov wrote: > On 22/03/2022 01:37, Ville Syrjälä wrote: > > On Tue, Mar 15, 2022 at 02:52:38PM -0400, Alex Deucher wrote: > >> On Mon, Mar 14, 2022 at 6:12 PM Ville Syrjälä > >> wrote: > >>> > >>> On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote: > drm: Add drm_mode_init() > drm/bridge: Use drm_mode_copy() > drm/imx: Use drm_mode_duplicate() > drm/panel: Use drm_mode_duplicate() > drm/vc4: Use drm_mode_copy() > >>> These have been pushed to drm-misc-next. > >>> > drm/amdgpu: Remove pointless on stack mode copies > drm/amdgpu: Use drm_mode_init() for on-stack modes > drm/amdgpu: Use drm_mode_copy() > >>> amdgpu ones are reviewed, but I'll leave them for the > >>> AMD folks to push to whichever tree they prefer. > >> > >> I pulled patches 2, 4, 5 into my tree. > > > > Thanks. > > > >> For 3, I'm happy to have it > >> land via drm-misc with the rest of the mode_init changes if you'd > >> prefer. > > > > Either way works for me. I don't yet have reviews yet for > > the other drivers, so I'll proably hold off for a bit more > > at least. And the i915 patch I'll be merging via drm-intel. > > > drm/radeon: Use drm_mode_copy() > drm/gma500: Use drm_mode_copy() > drm/tilcdc: Use drm_mode_copy() > drm/i915: Use drm_mode_copy() > > > > Those are now all in. > > > > Which leaves us with these stragglers: > drm/hisilicon: Use drm_mode_init() for on-stack modes > > drm/msm: Nuke weird on stack mode copy > drm/msm: Use drm_mode_init() for on-stack modes > drm/msm: Use drm_mode_copy() > > These three patches went beneath my radar for some reason. > > I have just sent a replacement for the first patch ([1]). Other two > patches can be pulled in msm/msm-next (or msm/msm-fixes) during the next > development cycle if that works for you. That'll do fine for me. Thanks. > > [1] https://patchwork.freedesktop.org/series/101682/ > > drm/mtk: Use drm_mode_init() for on-stack modes > drm/rockchip: Use drm_mode_copy() > drm/sti: Use drm_mode_copy() > drm: Use drm_mode_init() for on-stack modes > drm: Use drm_mode_copy() > > > > > -- > With best wishes > Dmitry -- Ville Syrjälä Intel
Re: [Freedreno] [REPOST PATCH v4 07/13] drm/msm/disp/dpu1: Add support for DSC in encoder
On 17-02-22, 23:32, Marijn Suijten wrote:
> On 2022-02-10 16:04:17, Vinod Koul wrote:
> > We need to configure the encoder for DSC configuration and calculate DSC
> > parameters for the given timing so this patch adds that support by
> > adding dpu_encoder_prep_dsc() which is invoked when DSC is enabled.
> >
> > Signed-off-by: Vinod Koul
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 164 +-
> > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 8 +
> > 2 files changed, 171 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 1e648db439f9..95a7bf362e81 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -21,6 +21,7 @@
> > #include "dpu_hw_intf.h"
> > #include "dpu_hw_ctl.h"
> > #include "dpu_hw_dspp.h"
> > +#include "dpu_hw_dsc.h"
> > #include "dpu_formats.h"
> > #include "dpu_encoder_phys.h"
> > #include "dpu_crtc.h"
> > @@ -136,6 +137,8 @@ enum dpu_enc_rc_states {
> > * @cur_slave: As above but for the slave encoder.
> > * @hw_pp: Handle to the pingpong blocks used for the display. No.
> > * pingpong blocks can be different than num_phys_encs.
> > + * @hw_dsc:Handle to the DSC blocks used for the display.
> > + * @dsc_mask: The bitmask of used DSC blocks.
>
> No need for "^The" prefix here.
Removed
>
> > * @intfs_swapped: Whether or not the phys_enc interfaces have been swapped
> > * for partial update right-only cases, such as pingpong
> > * split where virtual pingpong does not generate IRQs
> > @@ -169,6 +172,7 @@ enum dpu_enc_rc_states {
> > * @topology: topology of the display
> > * @idle_timeout: idle timeout duration in milliseconds
> > * @dp:msm_dp pointer, for DP encoders
> > + * @dsc: msm_display_dsc_config pointer, for DSC-enabled
> > encoders
> > */
> > struct dpu_encoder_virt {
> > struct drm_encoder base;
> > @@ -182,6 +186,9 @@ struct dpu_encoder_virt {
> > struct dpu_encoder_phys *cur_master;
> > struct dpu_encoder_phys *cur_slave;
> > struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> > + struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> > +
> > + unsigned int dsc_mask;
> >
> > bool intfs_swapped;
> >
> > @@ -209,6 +216,9 @@ struct dpu_encoder_virt {
> > u32 idle_timeout;
> >
> > struct msm_dp *dp;
> > +
> > + /* DSC configuration */
> > + struct msm_display_dsc_config *dsc;
> > };
> >
> > #define to_dpu_encoder_virt(x) container_of(x, struct dpu_encoder_virt,
> > base)
> > @@ -972,7 +982,8 @@ static void dpu_encoder_virt_mode_set(struct
> > drm_encoder *drm_enc,
> > struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
> > struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
> > struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
> > - int num_lm, num_ctl, num_pp;
> > + struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
> > + int num_lm, num_ctl, num_pp, num_dsc;
> > int i, j;
> >
> > if (!drm_enc) {
> > @@ -1027,6 +1038,23 @@ static void dpu_encoder_virt_mode_set(struct
> > drm_encoder *drm_enc,
> > dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
> > : NULL;
> >
> > + dpu_enc->dsc_mask = 0;
> > +
> > + if (dpu_enc->dsc) {
> > + unsigned int dsc_mask = 0;
>
> Perhaps move this definition outside the if, and unconditionally
> perform:
>
> dpu_enc->dsc_mask = dsc_mask;
>
> Below the if, instead of having to write it twice when ->dsc is truthy?
That is a good suggestion, updated now
>
> > +
> > + num_dsc = dpu_rm_get_assigned_resources(_kms->rm,
> > global_state,
> > + drm_enc->base.id,
> > DPU_HW_BLK_DSC,
> > + hw_dsc,
> > ARRAY_SIZE(hw_dsc));
> > + for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> > + dpu_enc->hw_dsc[i] = i < num_dsc ?
> > to_dpu_hw_dsc(hw_dsc[i]) : NULL;
> > +
> > + for (i = 0; i < num_dsc; i++)
> > + dsc_mask |= BIT(dpu_enc->hw_dsc[i]->idx - DSC_0);
>
> Perhaps you can merge these loops? Assuming hw_dsc is already
> NULL-initialized, you only need to iterate to `i extra conditional anyway?
Agreed, this does make it look neat
>
> > +
> > + dpu_enc->dsc_mask = dsc_mask;
> > + }
> > +
> > cstate = to_dpu_crtc_state(drm_crtc->state);
> >
> > for (i = 0; i < num_lm; i++) {
> > @@ -1739,6 +1767,127 @@ static void
> > dpu_encoder_vsync_event_work_handler(struct kthread_work *work)
> > nsecs_to_jiffies(ktime_to_ns(wakeup_time)));
> > }
> >
> > +static void
> >
Re: [Freedreno] [RESEND PATCH] dt-bindings: display/msm: add missing brace in dpu-qcm2290.yaml
On Tue, Mar 8, 2022 at 1:36 PM Rob Herring wrote: > > On Tue, Mar 1, 2022 at 6:14 PM Dmitry Baryshkov > wrote: > > > > Add missing brace in dpu-qcm2290.yaml. While we are at it, also fix > > indentation for another brace, so it matches the corresponding line. > > > > Reported-by: Rob Herring > > Cc: Loic Poulain > > Reviewed-by: Bjorn Andersson > > Signed-off-by: Dmitry Baryshkov > > --- > > Didn't include freedreno@ in the first email, so resending. > > --- > > Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > Now that the example actually builds, we get just schema warnings: > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.example.dt.yaml: > mdss@5e0: compatible: ['qcom,qcm2290-mdss', 'qcom,mdss'] is too > long > From schema: > /builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml > /builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.example.dt.yaml: > mdss@5e0: 'mdp@5e01000' does not match any of the regexes: > '^display-controller@[0-9a-f]+$', 'pinctrl-[0-9]+' > From schema: > /builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml > > > I would have assumed upon reporting errors with 'make > dt_binding_check' that the fixes would be tested with 'make > dt_binding_check'... Still failing. Please send a fix ASAP. Rob
Re: [Freedreno] [PATCH v3 3/6] drm/msm: split the main platform driver
Hi Dmitry,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm/drm-next]
[cannot apply to v5.17 next-20220323]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/drm-msm-rework-MDSS-drivers/20220323-172654
base: git://anongit.freedesktop.org/drm/drm drm-next
config: nios2-randconfig-p002-20220323
(https://download.01.org/0day-ci/archive/20220323/202203232120.4ejf1vfq-...@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/5484d7bfa709bbe2cd2cbb3b9959190d7a025c16
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Dmitry-Baryshkov/drm-msm-rework-MDSS-drivers/20220323-172654
git checkout 5484d7bfa709bbe2cd2cbb3b9959190d7a025c16
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross
O=build_dir ARCH=nios2 SHELL=/bin/bash drivers/gpu/drm/msm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from include/linux/acpi.h:15,
from include/linux/irqchip.h:14,
from drivers/gpu/drm/msm/msm_mdss.c:8:
>> drivers/gpu/drm/msm/msm_mdss.c:403:25: error: 'dt_match' undeclared here
>> (not in a function); did you mean 'dr_match_t'?
403 | MODULE_DEVICE_TABLE(of, dt_match);
| ^~~~
include/linux/module.h:244:15: note: in definition of macro
'MODULE_DEVICE_TABLE'
244 | extern typeof(name) __mod_##type##__##name##_device_table
\
| ^~~~
>> include/linux/module.h:244:21: error: '__mod_of__dt_match_device_table'
>> aliased to undefined symbol 'dt_match'
244 | extern typeof(name) __mod_##type##__##name##_device_table
\
| ^~
drivers/gpu/drm/msm/msm_mdss.c:403:1: note: in expansion of macro
'MODULE_DEVICE_TABLE'
403 | MODULE_DEVICE_TABLE(of, dt_match);
| ^~~
vim +403 drivers/gpu/drm/msm/msm_mdss.c
390
391 static const struct of_device_id mdss_dt_match[] = {
392 { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
393 { .compatible = "qcom,msm8998-mdss", .data = (void *)KMS_DPU },
394 { .compatible = "qcom,qcm2290-mdss", .data = (void *)KMS_DPU },
395 { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU },
396 { .compatible = "qcom,sc7180-mdss", .data = (void *)KMS_DPU },
397 { .compatible = "qcom,sc7280-mdss", .data = (void *)KMS_DPU },
398 { .compatible = "qcom,sc8180x-mdss", .data = (void *)KMS_DPU },
399 { .compatible = "qcom,sm8150-mdss", .data = (void *)KMS_DPU },
400 { .compatible = "qcom,sm8250-mdss", .data = (void *)KMS_DPU },
401 {}
402 };
> 403 MODULE_DEVICE_TABLE(of, dt_match);
404
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Re: [Freedreno] [REPOST PATCH v4 10/13] drm/msm/disp/dpu1: Add support for DSC in topology
On 17-02-22, 23:37, Marijn Suijten wrote:
> On 2022-02-10 16:04:20, Vinod Koul wrote:
> > For DSC to work we typically need a 2,2,1 configuration. This should
> > suffice for resolutions up to 4k. For more resolutions like 8k this won't
> > work.
> >
> > Also, it is better to use 2 LMs and DSC instances as half width results
> > in lesser power consumption as compared to single LM, DSC at full width.
> >
> > The panel has been tested only with 2,2,1 configuration, so for
> > now we blindly create 2,2,1 topology when DSC is enabled
> >
> > Co-developed-by: Abhinav Kumar
> > Signed-off-by: Abhinav Kumar
> > Signed-off-by: Vinod Koul
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 +
> > drivers/gpu/drm/msm/msm_drv.h | 2 ++
> > 2 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 95a7bf362e81..13ccb7b3cce5 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -574,8 +574,21 @@ static struct msm_display_topology
> > dpu_encoder_get_topology(
> > topology.num_enc = 0;
> > topology.num_intf = intf_count;
> >
> > + if (dpu_enc->dsc) {
> > + /* In case of Display Stream Compression DSC, we would use
>
> Just like elsewhere, since you wrote out Display Stream Compression,
> write the "DSC" abbreviation between parentheses ()?
Updated
--
~Vinod
Re: [Freedreno] [REPOST PATCH v4 10/13] drm/msm/disp/dpu1: Add support for DSC in topology
On 17-02-22, 22:44, Marijn Suijten wrote:
> On 2022-02-10 16:04:20, Vinod Koul wrote:
> > For DSC to work we typically need a 2,2,1 configuration. This should
> > suffice for resolutions up to 4k. For more resolutions like 8k this won't
> > work.
> >
> > Also, it is better to use 2 LMs and DSC instances as half width results
> > in lesser power consumption as compared to single LM, DSC at full width.
> >
> > The panel has been tested only with 2,2,1 configuration, so for
> > now we blindly create 2,2,1 topology when DSC is enabled
> >
> > Co-developed-by: Abhinav Kumar
> > Signed-off-by: Abhinav Kumar
> > Signed-off-by: Vinod Koul
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 +
> > drivers/gpu/drm/msm/msm_drv.h | 2 ++
> > 2 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 95a7bf362e81..13ccb7b3cce5 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -574,8 +574,21 @@ static struct msm_display_topology
> > dpu_encoder_get_topology(
> > topology.num_enc = 0;
> > topology.num_intf = intf_count;
> >
> > + if (dpu_enc->dsc) {
> > + /* In case of Display Stream Compression DSC, we would use
> > +* 2 encoders, 2 line mixers and 1 interface
>
> LM is a layer mixer, not a line mixer, right?
Right, thanks for spotting
--
~Vinod
Re: [Freedreno] [PATCH v3 12/13] drm/msm/dsi: Add support for DSC configuration
On 22-03-22, 19:59, Marijn Suijten wrote: > On 2022-03-22 22:46:50, Vinod Koul wrote: > > On 17-02-22, 16:11, Marijn Suijten wrote: > > > Hi Vinod, > > > > > > Thanks for taking time to go through this review, please find some > > > clarifications below. > > > > > > On 2022-02-17 16:44:04, Vinod Koul wrote: > > > > Hi Marijn, > > > > > > > > On 11-12-21, 01:03, Marijn Suijten wrote: > > > > > > > > > > +static int dsi_dsc_update_pic_dim(struct msm_display_dsc_config > > > > > > *dsc, > > > > > > + int pic_width, int pic_height) > > > > > > > > > > This function - adopted from downstream - does not seem to perform a > > > > > whole lot, especially without the modulo checks against the slice > > > > > size. > > > > > Perhaps it can be inlined? > > > > > > > > Most of the code here is :) > > > > > > > > This was split from downstream code to check and update dimension. We > > > > can inline this, or should we leave that to compiler. I am not a very > > > > big fan of inlining... > > > > > > It doesn't seem beneficial to code readability to have this function, > > > which is only called just once and also has the same struct members read > > > in a `DBG()` directly, abstracted away to a function. Not really > > > concerned about generated code/performance FWIW. > > > > > > Also note that the caller isn't checking the `-EINVAL` result... > > > > I have made this void inline. > > Perhaps there is a misunderstanding here: with inlining I am referring > to the process of transplanting the _function body_ to the only > call-site, not adding the `inline` keyword nor changing this to `void`. > > The checks that make this function return `-EINVAL` seem valid, so the > caller should check it instead of removing the return? Okay somehow I misunderstood then, let me see how the code looks in this case then -- ~Vinod
Re: [Freedreno] [PATCH 00/22] drm: Review of mode copies
On 22/03/2022 01:37, Ville Syrjälä wrote: On Tue, Mar 15, 2022 at 02:52:38PM -0400, Alex Deucher wrote: On Mon, Mar 14, 2022 at 6:12 PM Ville Syrjälä wrote: On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote: drm: Add drm_mode_init() drm/bridge: Use drm_mode_copy() drm/imx: Use drm_mode_duplicate() drm/panel: Use drm_mode_duplicate() drm/vc4: Use drm_mode_copy() These have been pushed to drm-misc-next. drm/amdgpu: Remove pointless on stack mode copies drm/amdgpu: Use drm_mode_init() for on-stack modes drm/amdgpu: Use drm_mode_copy() amdgpu ones are reviewed, but I'll leave them for the AMD folks to push to whichever tree they prefer. I pulled patches 2, 4, 5 into my tree. Thanks. For 3, I'm happy to have it land via drm-misc with the rest of the mode_init changes if you'd prefer. Either way works for me. I don't yet have reviews yet for the other drivers, so I'll proably hold off for a bit more at least. And the i915 patch I'll be merging via drm-intel. drm/radeon: Use drm_mode_copy() drm/gma500: Use drm_mode_copy() drm/tilcdc: Use drm_mode_copy() drm/i915: Use drm_mode_copy() Those are now all in. Which leaves us with these stragglers: drm/hisilicon: Use drm_mode_init() for on-stack modes drm/msm: Nuke weird on stack mode copy drm/msm: Use drm_mode_init() for on-stack modes drm/msm: Use drm_mode_copy() These three patches went beneath my radar for some reason. I have just sent a replacement for the first patch ([1]). Other two patches can be pulled in msm/msm-next (or msm/msm-fixes) during the next development cycle if that works for you. [1] https://patchwork.freedesktop.org/series/101682/ drm/mtk: Use drm_mode_init() for on-stack modes drm/rockchip: Use drm_mode_copy() drm/sti: Use drm_mode_copy() drm: Use drm_mode_init() for on-stack modes drm: Use drm_mode_copy() -- With best wishes Dmitry
[Freedreno] [PATCH 2/3] drm/msm/dp: simplify dp_connector_get_modes()
Since dp_panel_get_modes() handling for dp_mode was removed,
dp_display_get_modes also doesn't change the passed dp_mode, drop the
unused dp_mode variable being allocated unused and then freed.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dp/dp_display.c | 8 ++--
drivers/gpu/drm/msm/dp/dp_display.h | 3 +--
drivers/gpu/drm/msm/dp/dp_drm.c | 29 +
3 files changed, 4 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 3dd790083cf7..be1e9c558fda 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -999,8 +999,7 @@ int dp_display_validate_mode(struct msm_dp *dp, u32
mode_pclk_khz)
return MODE_OK;
}
-int dp_display_get_modes(struct msm_dp *dp,
- struct dp_display_mode *dp_mode)
+int dp_display_get_modes(struct msm_dp *dp)
{
struct dp_display_private *dp_display;
int ret = 0;
@@ -1012,11 +1011,8 @@ int dp_display_get_modes(struct msm_dp *dp,
dp_display = container_of(dp, struct dp_display_private, dp_display);
- ret = dp_panel_get_modes(dp_display->panel,
+ return dp_panel_get_modes(dp_display->panel,
dp->connector);
- if (dp_mode->drm_mode.clock)
- dp->max_pclk_khz = dp_mode->drm_mode.clock;
- return ret;
}
bool dp_display_check_video_test(struct msm_dp *dp)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 7af2b186d2d9..6efb5c853c89 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -33,8 +33,7 @@ struct msm_dp {
int dp_display_set_plugged_cb(struct msm_dp *dp_display,
hdmi_codec_plugged_cb fn, struct device *codec_dev);
int dp_display_validate_mode(struct msm_dp *dp_display, u32 mode_pclk_khz);
-int dp_display_get_modes(struct msm_dp *dp_display,
- struct dp_display_mode *dp_mode);
+int dp_display_get_modes(struct msm_dp *dp_display);
int dp_display_request_irq(struct msm_dp *dp_display);
bool dp_display_check_video_test(struct msm_dp *dp_display);
int dp_display_get_test_bpp(struct msm_dp *dp_display);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 80f59cf99089..af5f1b001192 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -55,49 +55,22 @@ static int dp_connector_get_modes(struct drm_connector
*connector)
{
int rc = 0;
struct msm_dp *dp;
- struct dp_display_mode *dp_mode = NULL;
- struct drm_display_mode *m, drm_mode;
if (!connector)
return 0;
dp = to_dp_connector(connector)->dp_display;
- dp_mode = kzalloc(sizeof(*dp_mode), GFP_KERNEL);
- if (!dp_mode)
- return 0;
-
/* pluggable case assumes EDID is read when HPD */
if (dp->is_connected) {
- /*
-*The get_modes() function might return one mode that is stored
-* in dp_mode when compliance test is in progress. If not, the
-* return value is equal to the total number of modes supported
-* by the sink
-*/
- rc = dp_display_get_modes(dp, dp_mode);
+ rc = dp_display_get_modes(dp);
if (rc <= 0) {
DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
- kfree(dp_mode);
return rc;
}
- if (dp_mode->drm_mode.clock) { /* valid DP mode */
- memset(_mode, 0x0, sizeof(drm_mode));
- drm_mode_copy(_mode, _mode->drm_mode);
- m = drm_mode_duplicate(connector->dev, _mode);
- if (!m) {
- DRM_ERROR("failed to add mode %ux%u\n",
- drm_mode.hdisplay,
- drm_mode.vdisplay);
- kfree(dp_mode);
- return 0;
- }
- drm_mode_probed_add(connector, m);
- }
} else {
DRM_DEBUG_DP("No sink connected\n");
}
- kfree(dp_mode);
return rc;
}
--
2.35.1
[Freedreno] [PATCH 3/3] drm/msm/dp: remove max_pclk_khz field from dp_panel/dp_display
Since the last commit, the max_pclk_khz became constant, it's set to
DP_MAX_PIXEL_CLK_KHZ and never changed afterwards. Remove it completely
and use DP_MAX_PIXEL_CLK_KHZ directly.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dp/dp_debug.c | 2 --
drivers/gpu/drm/msm/dp/dp_display.c | 1 -
drivers/gpu/drm/msm/dp/dp_display.h | 2 --
drivers/gpu/drm/msm/dp/dp_drm.c | 4 +---
drivers/gpu/drm/msm/dp/dp_panel.h | 1 -
5 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
b/drivers/gpu/drm/msm/dp/dp_debug.c
index 2f9c943f12d5..5e35033ba3e4 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -44,8 +44,6 @@ static int dp_debug_show(struct seq_file *seq, void *p)
drm_mode = >panel->dp_mode.drm_mode;
seq_printf(seq, "\tname = %s\n", DEBUG_NAME);
- seq_printf(seq, "\tdp_panel\n\t\tmax_pclk_khz = %d\n",
- debug->panel->max_pclk_khz);
seq_printf(seq, "\tdrm_dp_link\n\t\trate = %u\n",
debug->panel->link_info.rate);
seq_printf(seq, "\t\tnum_lanes = %u\n",
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index be1e9c558fda..aeb08416b309 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -370,7 +370,6 @@ static int dp_display_process_hpd_high(struct
dp_display_private *dp)
dp->audio_supported = drm_detect_monitor_audio(edid);
dp_panel_handle_sink_request(dp->panel);
- dp->dp_display.max_pclk_khz = DP_MAX_PIXEL_CLK_KHZ;
dp->dp_display.max_dp_lanes = dp->parser->max_dp_lanes;
/*
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 6efb5c853c89..f3a9d7449a1c 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -24,8 +24,6 @@ struct msm_dp {
hdmi_codec_plugged_cb plugged_cb;
- u32 max_pclk_khz;
-
u32 max_dp_lanes;
struct dp_audio *dp_audio;
};
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index af5f1b001192..a94c9b34f397 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -88,9 +88,7 @@ static enum drm_mode_status dp_connector_mode_valid(
dp_disp = to_dp_connector(connector)->dp_display;
- if ((dp_disp->max_pclk_khz <= 0) ||
- (dp_disp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) ||
- (mode->clock > dp_disp->max_pclk_khz))
+ if (mode->clock > DP_MAX_PIXEL_CLK_KHZ)
return MODE_BAD;
return dp_display_validate_mode(dp_disp, mode->clock);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9fa6e524832c..d861197ac1c8 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -49,7 +49,6 @@ struct dp_panel {
bool video_test;
u32 vic;
- u32 max_pclk_khz;
u32 max_dp_lanes;
u32 max_bw_code;
--
2.35.1
[Freedreno] [PATCH 1/3] drm/msm/dp: drop dp_mode argument from dp_panel_get_modes()
Since the commit ab205927592b ("drm/msm/dp: remove mode hard-coding in
case of DP CTS") the function dp_panel_get_modes() doesn't use (or fill)
the dp_mode argument. Drop it completely.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
drivers/gpu/drm/msm/dp/dp_panel.c | 2 +-
drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 178b774a5fbd..3dd790083cf7 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1013,7 +1013,7 @@ int dp_display_get_modes(struct msm_dp *dp,
dp_display = container_of(dp, struct dp_display_private, dp_display);
ret = dp_panel_get_modes(dp_display->panel,
- dp->connector, dp_mode);
+ dp->connector);
if (dp_mode->drm_mode.clock)
dp->max_pclk_khz = dp_mode->drm_mode.clock;
return ret;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index f1418722c549..8d7662028f30 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -259,7 +259,7 @@ u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel,
}
int dp_panel_get_modes(struct dp_panel *dp_panel,
- struct drm_connector *connector, struct dp_display_mode *mode)
+ struct drm_connector *connector)
{
if (!dp_panel) {
DRM_ERROR("invalid input\n");
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9023e5bb4b8b..9fa6e524832c 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -64,7 +64,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
u32 dp_panel_get_mode_bpp(struct dp_panel *dp_panel, u32 mode_max_bpp,
u32 mode_pclk_khz);
int dp_panel_get_modes(struct dp_panel *dp_panel,
- struct drm_connector *connector, struct dp_display_mode *mode);
+ struct drm_connector *connector);
void dp_panel_handle_sink_request(struct dp_panel *dp_panel);
void dp_panel_tpg_config(struct dp_panel *dp_panel, bool enable);
--
2.35.1
[Freedreno] [PATCH 0/3] drm/msm/dp: simplify dp_connector_get_modes()
As noted by Ville Syrjälä [1] the dp_connector_get_modes() has weird on-stack interim copy of the mode. When reviewing the mentioned patch I noticed that the rest of the code in dp_connector_get_modes() is weird, unused since September of 2020 and can be removed. This series removes dead weird code leaving dp_connector_get_modes() simple enough. [1] https://lore.kernel.org/linux-arm-msm/20220218100403.7028-11-ville.syrj...@linux.intel.com/ *** BLURB HERE *** Dmitry Baryshkov (3): drm/msm/dp: drop dp_mode argument from dp_panel_get_modes() drm/msm/dp: simplify dp_connector_get_modes() drm/msm/dp: remove max_pclk_khz field from dp_panel/dp_display drivers/gpu/drm/msm/dp/dp_debug.c | 2 -- drivers/gpu/drm/msm/dp/dp_display.c | 11 +++--- drivers/gpu/drm/msm/dp/dp_display.h | 5 + drivers/gpu/drm/msm/dp/dp_drm.c | 33 ++--- drivers/gpu/drm/msm/dp/dp_panel.c | 2 +- drivers/gpu/drm/msm/dp/dp_panel.h | 3 +-- 6 files changed, 8 insertions(+), 48 deletions(-) -- 2.35.1
Re: [Freedreno] [PATCH 10/22] drm/msm: Nuke weird on stack mode copy
On 18/02/2022 13:03, Ville Syrjala wrote:
From: Ville Syrjälä
This on stack middle man mode looks entirely pointless.
Just duplicate the original mode directly.
Cc: Rob Clark
Cc: Sean Paul
Cc: Abhinav Kumar
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Ville Syrjälä
I took a glance at the surrounding piece of code.
The dp_connector_get_modes() calls dp_display_get_modes() in attempt to
fill the dp_mode argument. However the dp_display_get_modes() function
just calls dp_panel_get_modes(), which does not touch dp_mode argument
since the commit ab205927592b ("drm/msm/dp: remove mode hard-coding in
case of DP CTS") dating September 2020. I think we can drop this piece
of code completely.
---
drivers/gpu/drm/msm/dp/dp_drm.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index d4d360d19eba..09188d02aa1e 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -56,7 +56,7 @@ static int dp_connector_get_modes(struct drm_connector
*connector)
int rc = 0;
struct msm_dp *dp;
struct dp_display_mode *dp_mode = NULL;
- struct drm_display_mode *m, drm_mode;
+ struct drm_display_mode *m;
if (!connector)
return 0;
@@ -82,13 +82,11 @@ static int dp_connector_get_modes(struct drm_connector
*connector)
return rc;
}
if (dp_mode->drm_mode.clock) { /* valid DP mode */
- memset(_mode, 0x0, sizeof(drm_mode));
- drm_mode_copy(_mode, _mode->drm_mode);
- m = drm_mode_duplicate(connector->dev, _mode);
+ m = drm_mode_duplicate(connector->dev,
_mode->drm_mode);
if (!m) {
DRM_ERROR("failed to add mode %ux%u\n",
- drm_mode.hdisplay,
- drm_mode.vdisplay);
+ dp_mode->drm_mode.hdisplay,
+ dp_mode->drm_mode.vdisplay);
kfree(dp_mode);
return 0;
}
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH 12/22] drm/msm: Use drm_mode_copy()
On 18/02/2022 13:03, Ville Syrjala wrote:
From: Ville Syrjälä
struct drm_display_mode embeds a list head, so overwriting
the full struct with another one will corrupt the list
(if the destination mode is on a list). Use drm_mode_copy()
instead which explicitly preserves the list head of
the destination mode.
Even if we know the destination mode is not on any list
using drm_mode_copy() seems decent as it sets a good
example. Bad examples of not using it might eventually
get copied into code where preserving the list head
actually matters.
Obviously one case not covered here is when the mode
itself is embedded in a larger structure and the whole
structure is copied. But if we are careful when copying
into modes embedded in structures I think we can be a
little more reassured that bogus list heads haven't been
propagated in.
@is_mode_copy@
@@
drm_mode_copy(...)
{
...
}
@depends on !is_mode_copy@
struct drm_display_mode *mode;
expression E, S;
@@
(
- *mode = E
+ drm_mode_copy(mode, )
|
- memcpy(mode, E, S)
+ drm_mode_copy(mode, E)
)
@depends on !is_mode_copy@
struct drm_display_mode mode;
expression E;
@@
(
- mode = E
+ drm_mode_copy(, )
|
- memcpy(, E, S)
+ drm_mode_copy(, E)
)
@@
struct drm_display_mode *mode;
@@
- &*mode
+ mode
Cc: Rob Clark
Cc: Sean Paul
Cc: Abhinav Kumar
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Ville Syrjälä
Reviewed-by: Dmitry Baryshkov
---
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH 11/22] drm/msm: Use drm_mode_init() for on-stack modes
On 18/02/2022 13:03, Ville Syrjala wrote: From: Ville Syrjälä Initialize on-stack modes with drm_mode_init() to guarantee no stack garbage in the list head, or that we aren't copying over another mode's list head. Based on the following cocci script, with manual fixups: @decl@ identifier M; expression E; @@ - struct drm_display_mode M = E; + struct drm_display_mode M; @@ identifier decl.M; expression decl.E; statement S, S1; @@ struct drm_display_mode M; ... when != S + drm_mode_init(, ); + S1 @@ expression decl.E; @@ - &*E + E Cc: Rob Clark Cc: Sean Paul Cc: Abhinav Kumar Cc: linux-arm-...@vger.kernel.org Cc: freedreno@lists.freedesktop.org Signed-off-by: Ville Syrjälä Reviewed-by: Dmitry Baryshkov --- -- With best wishes Dmitry
[Freedreno] [PATCH v3 4/6] drm/msm: stop using device's match data pointer
Let's make the match's data pointer a (sub-)driver's private data. The
only user currently is the msm_drm_init() function, using this data to
select kms_init callback. Pass this callback through the driver's
private data instead.
Reviewed-by: Stephen Boyd
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 ---
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 14 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 11 ---
drivers/gpu/drm/msm/msm_drv.c| 38 ++--
drivers/gpu/drm/msm/msm_drv.h| 5 +---
drivers/gpu/drm/msm/msm_kms.h| 4 ---
drivers/gpu/drm/msm/msm_mdss.c | 29 +++---
7 files changed, 42 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index e29796c4f27b..38627ccf3068 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1172,7 +1172,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
return rc;
}
-struct msm_kms *dpu_kms_init(struct drm_device *dev)
+static int dpu_kms_init(struct drm_device *dev)
{
struct msm_drm_private *priv;
struct dpu_kms *dpu_kms;
@@ -1180,7 +1180,7 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev)
if (!dev) {
DPU_ERROR("drm device node invalid\n");
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
priv = dev->dev_private;
@@ -1189,11 +1189,11 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev)
irq = irq_of_parse_and_map(dpu_kms->pdev->dev.of_node, 0);
if (irq < 0) {
DPU_ERROR("failed to get irq: %d\n", irq);
- return ERR_PTR(irq);
+ return irq;
}
dpu_kms->base.irq = irq;
- return _kms->base;
+ return 0;
}
static int dpu_bind(struct device *dev, struct device *master, void *data)
@@ -1204,6 +1204,8 @@ static int dpu_bind(struct device *dev, struct device
*master, void *data)
struct dpu_kms *dpu_kms;
int ret = 0;
+ priv->kms_init = dpu_kms_init;
+
dpu_kms = devm_kzalloc(>dev, sizeof(*dpu_kms), GFP_KERNEL);
if (!dpu_kms)
return -ENOMEM;
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index c5c0650414c5..2e5f6b6fd3c3 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -389,7 +389,7 @@ static void read_mdp_hw_revision(struct mdp4_kms *mdp4_kms,
DRM_DEV_INFO(dev->dev, "MDP4 version v%d.%d", *major, *minor);
}
-struct msm_kms *mdp4_kms_init(struct drm_device *dev)
+static int mdp4_kms_init(struct drm_device *dev)
{
struct platform_device *pdev = to_platform_device(dev->dev);
struct mdp4_platform_config *config = mdp4_get_config(pdev);
@@ -403,8 +403,7 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev)
mdp4_kms = kzalloc(sizeof(*mdp4_kms), GFP_KERNEL);
if (!mdp4_kms) {
DRM_DEV_ERROR(dev->dev, "failed to allocate kms\n");
- ret = -ENOMEM;
- goto fail;
+ return -ENOMEM;
}
ret = mdp_kms_init(_kms->base, _funcs);
@@ -551,12 +550,13 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev)
dev->mode_config.max_width = 2048;
dev->mode_config.max_height = 2048;
- return kms;
+ return 0;
fail:
if (kms)
mdp4_destroy(kms);
- return ERR_PTR(ret);
+
+ return ret;
}
static struct mdp4_platform_config *mdp4_get_config(struct platform_device
*dev)
@@ -583,6 +583,8 @@ static int mdp4_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
+ priv->kms_init = mdp4_kms_init;
+
platform_set_drvdata(pdev, priv);
/*
@@ -600,7 +602,7 @@ static int mdp4_remove(struct platform_device *pdev)
}
static const struct of_device_id mdp4_dt_match[] = {
- { .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
+ { .compatible = "qcom,mdp4" },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mdp4_dt_match);
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 3b92372e7bdf..0c78608832c3 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -544,7 +544,7 @@ static int get_clk(struct platform_device *pdev, struct clk
**clkp,
return 0;
}
-struct msm_kms *mdp5_kms_init(struct drm_device *dev)
+static int mdp5_kms_init(struct drm_device *dev)
{
struct msm_drm_private *priv = dev->dev_private;
struct platform_device *pdev;
@@ -558,7 +558,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
/* priv->kms would have been populated by the MDP5 driver */
kms = priv->kms;
[Freedreno] [PATCH v3 5/6] drm/msm: allow compile time selection of driver components
MSM DRM driver already allows one to compile out the DP or DSI support. Add support for disabling other features like MDP4/MDP5/DPU drivers or direct HDMI output support. Suggested-by: Stephen Boyd Reviewed-by: Stephen Boyd Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/Kconfig | 50 +-- drivers/gpu/drm/msm/Makefile | 24 ++--- .../gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c | 3 ++ .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 3 ++ drivers/gpu/drm/msm/msm_drv.h | 33 drivers/gpu/drm/msm/msm_mdss.c| 13 - 6 files changed, 115 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 9b019598e042..96b01873ce36 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -46,12 +46,39 @@ config DRM_MSM_GPU_SUDO Only use this if you are a driver developer. This should *not* be enabled for production kernels. If unsure, say N. -config DRM_MSM_HDMI_HDCP - bool "Enable HDMI HDCP support in MSM DRM driver" +config DRM_MSM_MDSS + bool + depends on DRM_MSM + default n + +config DRM_MSM_MDP4 + bool "Enable MDP4 support in MSM DRM driver" depends on DRM_MSM default y help - Choose this option to enable HDCP state machine + Compile in support for the Mobile Display Processor v4 (MDP4) in + the MSM DRM driver. It is the older display controller found in + devices using APQ8064/MSM8960/MSM8x60 platforms. + +config DRM_MSM_MDP5 + bool "Enable MDP5 support in MSM DRM driver" + depends on DRM_MSM + select DRM_MSM_MDSS + default y + help + Compile in support for the Mobile Display Processor v5 (MDP5) in + the MSM DRM driver. It is the display controller found in devices + using e.g. APQ8016/MSM8916/APQ8096/MSM8996/MSM8974/SDM6x0 platforms. + +config DRM_MSM_DPU + bool "Enable DPU support in MSM DRM driver" + depends on DRM_MSM + select DRM_MSM_MDSS + default y + help + Compile in support for the Display Processing Unit in + the MSM DRM driver. It is the display controller found in devices + using e.g. SDM845 and newer platforms. config DRM_MSM_DP bool "Enable DisplayPort support in MSM DRM driver" @@ -116,3 +143,20 @@ config DRM_MSM_DSI_7NM_PHY help Choose this option if DSI PHY on SM8150/SM8250/SC7280 is used on the platform. + +config DRM_MSM_HDMI + bool "Enable HDMI support in MSM DRM driver" + depends on DRM_MSM + default y + help + Compile in support for the HDMI output MSM DRM driver. It can + be a primary or a secondary display on device. Note that this is used + only for the direct HDMI output. If the device outputs HDMI data + throught some kind of DSI-to-HDMI bridge, this option can be disabled. + +config DRM_MSM_HDMI_HDCP + bool "Enable HDMI HDCP support in MSM DRM driver" + depends on DRM_MSM && DRM_MSM_HDMI + default y + help + Choose this option to enable HDCP state machine diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index e76927b42033..3dc576309255 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -16,6 +16,8 @@ msm-y := \ adreno/a6xx_gpu.o \ adreno/a6xx_gmu.o \ adreno/a6xx_hfi.o \ + +msm-$(CONFIG_DRM_MSM_HDMI) += \ hdmi/hdmi.o \ hdmi/hdmi_audio.o \ hdmi/hdmi_bridge.o \ @@ -27,9 +29,10 @@ msm-y := \ hdmi/hdmi_phy_8x60.o \ hdmi/hdmi_phy_8x74.o \ hdmi/hdmi_pll_8960.o \ - disp/mdp_format.o \ - disp/mdp_kms.o \ + +msm-$(CONFIG_DRM_MSM_MDP4) += \ disp/mdp4/mdp4_crtc.o \ + disp/mdp4/mdp4_dsi_encoder.o \ disp/mdp4/mdp4_dtv_encoder.o \ disp/mdp4/mdp4_lcdc_encoder.o \ disp/mdp4/mdp4_lvds_connector.o \ @@ -37,7 +40,10 @@ msm-y := \ disp/mdp4/mdp4_irq.o \ disp/mdp4/mdp4_kms.o \ disp/mdp4/mdp4_plane.o \ + +msm-$(CONFIG_DRM_MSM_MDP5) += \ disp/mdp5/mdp5_cfg.o \ + disp/mdp5/mdp5_cmd_encoder.o \ disp/mdp5/mdp5_ctl.o \ disp/mdp5/mdp5_crtc.o \ disp/mdp5/mdp5_encoder.o \ @@ -47,6 +53,8 @@ msm-y := \ disp/mdp5/mdp5_mixer.o \ disp/mdp5/mdp5_plane.o \ disp/mdp5/mdp5_smp.o \ + +msm-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_core_perf.o \ disp/dpu1/dpu_crtc.o \ disp/dpu1/dpu_encoder.o \ @@ -69,6 +77,13 @@ msm-y := \ disp/dpu1/dpu_plane.o \ disp/dpu1/dpu_rm.o \ disp/dpu1/dpu_vbif.o \ + +msm-$(CONFIG_DRM_MSM_MDSS) += \ + msm_mdss.o \ + +msm-y += \ + disp/mdp_format.o \ + disp/mdp_kms.o \ disp/msm_disp_snapshot.o \ disp/msm_disp_snapshot_util.o \ msm_atomic.o \
[Freedreno] [PATCH v3 6/6] drm/msm: make mdp5/dpu devices master components
The msm_mdss serves several roles at this moment. It provides IRQ domain
used by MDP5 and DPU drivers but it also serves as a component master
for both those usecases. MDP4 (which does not have separate MDSS device)
is the component master on it's own.
Remove this assymmetry and make both MDP5 and DPU component masters too.
This removes a need to care about drm/components from msm_mdss driver,
removes an mdss pointer from struct msm_drm_private and simplifies the
interface between mdp5/dpu and msm_drv.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 80
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 16 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 47 +-
drivers/gpu/drm/msm/msm_drv.c| 32 +-
drivers/gpu/drm/msm/msm_drv.h| 6 +-
drivers/gpu/drm/msm/msm_kms.h| 3 -
drivers/gpu/drm/msm/msm_mdss.c | 61 +++---
7 files changed, 67 insertions(+), 178 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 38627ccf3068..ab8a35e09bc9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -381,8 +381,8 @@ static int dpu_kms_parse_data_bus_icc_path(struct dpu_kms
*dpu_kms)
struct icc_path *path1;
struct drm_device *dev = dpu_kms->dev;
- path0 = of_icc_get(dev->dev, "mdp0-mem");
- path1 = of_icc_get(dev->dev, "mdp1-mem");
+ path0 = of_icc_get(dev->dev->parent, "mdp0-mem");
+ path1 = of_icc_get(dev->dev->parent, "mdp1-mem");
if (IS_ERR_OR_NULL(path0))
return PTR_ERR_OR_ZERO(path0);
@@ -837,6 +837,9 @@ static void dpu_kms_destroy(struct msm_kms *kms)
_dpu_kms_hw_destroy(dpu_kms);
msm_kms_destroy(_kms->base);
+
+ if (dpu_kms->rpm_enabled)
+ pm_runtime_disable(_kms->pdev->dev);
}
static irqreturn_t dpu_irq(struct msm_kms *kms)
@@ -978,7 +981,7 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
if (!domain)
return 0;
- mmu = msm_iommu_new(dpu_kms->dev->dev, domain);
+ mmu = msm_iommu_new(dpu_kms->dev->dev->parent, domain);
if (IS_ERR(mmu)) {
iommu_domain_free(domain);
return PTR_ERR(mmu);
@@ -1172,40 +1175,15 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
return rc;
}
-static int dpu_kms_init(struct drm_device *dev)
-{
- struct msm_drm_private *priv;
- struct dpu_kms *dpu_kms;
- int irq;
-
- if (!dev) {
- DPU_ERROR("drm device node invalid\n");
- return -EINVAL;
- }
-
- priv = dev->dev_private;
- dpu_kms = to_dpu_kms(priv->kms);
-
- irq = irq_of_parse_and_map(dpu_kms->pdev->dev.of_node, 0);
- if (irq < 0) {
- DPU_ERROR("failed to get irq: %d\n", irq);
- return irq;
- }
- dpu_kms->base.irq = irq;
-
- return 0;
-}
-
-static int dpu_bind(struct device *dev, struct device *master, void *data)
+static int dpu_kms_init(struct drm_device *ddev)
{
- struct msm_drm_private *priv = dev_get_drvdata(master);
+ struct msm_drm_private *priv = ddev->dev_private;
+ struct device *dev = ddev->dev;
struct platform_device *pdev = to_platform_device(dev);
- struct drm_device *ddev = priv->dev;
struct dpu_kms *dpu_kms;
+ int irq;
int ret = 0;
- priv->kms_init = dpu_kms_init;
-
dpu_kms = devm_kzalloc(>dev, sizeof(*dpu_kms), GFP_KERNEL);
if (!dpu_kms)
return -ENOMEM;
@@ -1227,8 +1205,6 @@ static int dpu_bind(struct device *dev, struct device
*master, void *data)
}
dpu_kms->num_clocks = ret;
- platform_set_drvdata(pdev, dpu_kms);
-
ret = msm_kms_init(_kms->base, _funcs);
if (ret) {
DPU_ERROR("failed to init kms, ret=%d\n", ret);
@@ -1242,31 +1218,25 @@ static int dpu_bind(struct device *dev, struct device
*master, void *data)
priv->kms = _kms->base;
- return ret;
-}
-
-static void dpu_unbind(struct device *dev, struct device *master, void *data)
-{
- struct platform_device *pdev = to_platform_device(dev);
- struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
+ irq = irq_of_parse_and_map(dpu_kms->pdev->dev.of_node, 0);
+ if (irq < 0) {
+ DPU_ERROR("failed to get irq: %d\n", irq);
+ return irq;
+ }
+ dpu_kms->base.irq = irq;
- if (dpu_kms->rpm_enabled)
- pm_runtime_disable(>dev);
+ return 0;
}
-static const struct component_ops dpu_ops = {
- .bind = dpu_bind,
- .unbind = dpu_unbind,
-};
-
static int dpu_dev_probe(struct platform_device *pdev)
{
- return component_add(>dev, _ops);
+ return msm_drv_probe(>dev, dpu_kms_init);
}
static int dpu_dev_remove(struct platform_device *pdev)
[Freedreno] [PATCH v3 3/6] drm/msm: split the main platform driver
Currently the msm platform driver is a multiplex handling several cases:
- headless GPU-only driver,
- MDP4 with flat device nodes,
- MDP5/DPU MDSS with all the nodes being children of MDSS node.
This results in not-so-perfect code, checking the hardware version
(MDP4/MDP5/DPU) in several places, checking for mdss even when it can
not exist, etc. Split the code into three handling subdrivers (mdp4,
mdss and headless msm).
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 56 ++
drivers/gpu/drm/msm/msm_drv.c| 228 ---
drivers/gpu/drm/msm/msm_drv.h| 27 ++-
drivers/gpu/drm/msm/msm_kms.h| 7 -
drivers/gpu/drm/msm/msm_mdss.c | 175 -
5 files changed, 288 insertions(+), 205 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 3cf476c55158..c5c0650414c5 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -569,3 +569,59 @@ static struct mdp4_platform_config *mdp4_get_config(struct
platform_device *dev)
return
}
+
+static const struct dev_pm_ops mdp4_pm_ops = {
+ .prepare = msm_pm_prepare,
+ .complete = msm_pm_complete,
+};
+
+static int mdp4_probe(struct platform_device *pdev)
+{
+ struct msm_drm_private *priv;
+
+ priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, priv);
+
+ /*
+* on MDP4 based platforms, the MDP platform device is the component
+* master that adds other display interface components to itself.
+*/
+ return msm_drv_probe(>dev, >dev);
+}
+
+static int mdp4_remove(struct platform_device *pdev)
+{
+ component_master_del(>dev, _drm_ops);
+
+ return 0;
+}
+
+static const struct of_device_id mdp4_dt_match[] = {
+ { .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mdp4_dt_match);
+
+static struct platform_driver mdp4_platform_driver = {
+ .probe = mdp4_probe,
+ .remove = mdp4_remove,
+ .shutdown = msm_drv_shutdown,
+ .driver = {
+ .name = "mdp4",
+ .of_match_table = mdp4_dt_match,
+ .pm = _pm_ops,
+ },
+};
+
+void __init msm_mdp4_register(void)
+{
+ platform_driver_register(_platform_driver);
+}
+
+void __exit msm_mdp4_unregister(void)
+{
+ platform_driver_unregister(_platform_driver);
+}
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index d3cce39c13bf..16d8aa225405 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -256,10 +256,6 @@ static int msm_drm_uninit(struct device *dev)
return 0;
}
-#define KMS_MDP4 4
-#define KMS_MDP5 5
-#define KMS_DPU 3
-
static int get_mdp_ver(struct platform_device *pdev)
{
struct device *dev = >dev;
@@ -970,50 +966,7 @@ static const struct drm_driver msm_driver = {
.patchlevel = MSM_VERSION_PATCHLEVEL,
};
-static int __maybe_unused msm_runtime_suspend(struct device *dev)
-{
- struct msm_drm_private *priv = dev_get_drvdata(dev);
- struct msm_mdss *mdss = priv->mdss;
-
- DBG("");
-
- if (mdss)
- return msm_mdss_disable(mdss);
-
- return 0;
-}
-
-static int __maybe_unused msm_runtime_resume(struct device *dev)
-{
- struct msm_drm_private *priv = dev_get_drvdata(dev);
- struct msm_mdss *mdss = priv->mdss;
-
- DBG("");
-
- if (mdss)
- return msm_mdss_enable(mdss);
-
- return 0;
-}
-
-static int __maybe_unused msm_pm_suspend(struct device *dev)
-{
-
- if (pm_runtime_suspended(dev))
- return 0;
-
- return msm_runtime_suspend(dev);
-}
-
-static int __maybe_unused msm_pm_resume(struct device *dev)
-{
- if (pm_runtime_suspended(dev))
- return 0;
-
- return msm_runtime_resume(dev);
-}
-
-static int __maybe_unused msm_pm_prepare(struct device *dev)
+int msm_pm_prepare(struct device *dev)
{
struct msm_drm_private *priv = dev_get_drvdata(dev);
struct drm_device *ddev = priv ? priv->dev : NULL;
@@ -1024,7 +977,7 @@ static int __maybe_unused msm_pm_prepare(struct device
*dev)
return drm_mode_config_helper_suspend(ddev);
}
-static void __maybe_unused msm_pm_complete(struct device *dev)
+void msm_pm_complete(struct device *dev)
{
struct msm_drm_private *priv = dev_get_drvdata(dev);
struct drm_device *ddev = priv ? priv->dev : NULL;
@@ -1036,8 +989,6 @@ static void __maybe_unused msm_pm_complete(struct device
*dev)
}
static const struct dev_pm_ops msm_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(msm_pm_suspend, msm_pm_resume)
- SET_RUNTIME_PM_OPS(msm_runtime_suspend, msm_runtime_resume, NULL)
[Freedreno] [PATCH v3 2/6] drm/msm: remove extra indirection for msm_mdss
Since now there is just one mdss subdriver, drop all the indirection,
make msm_mdss struct completely opaque (and defined inside msm_mdss.c)
and call mdss functions directly.
Reviewed-by: Abhinav Kumar
Reviewed-by: Stephen Boyd
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/msm_drv.c | 29 +++
drivers/gpu/drm/msm/msm_kms.h | 16 ++--
drivers/gpu/drm/msm/msm_mdss.c | 136 +++--
3 files changed, 81 insertions(+), 100 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index a924fa46da9b..d3cce39c13bf 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -977,8 +977,8 @@ static int __maybe_unused msm_runtime_suspend(struct device
*dev)
DBG("");
- if (mdss && mdss->funcs)
- return mdss->funcs->disable(mdss);
+ if (mdss)
+ return msm_mdss_disable(mdss);
return 0;
}
@@ -990,8 +990,8 @@ static int __maybe_unused msm_runtime_resume(struct device
*dev)
DBG("");
- if (mdss && mdss->funcs)
- return mdss->funcs->enable(mdss);
+ if (mdss)
+ return msm_mdss_enable(mdss);
return 0;
}
@@ -1226,6 +1226,7 @@ static const struct component_master_ops msm_drm_ops = {
static int msm_pdev_probe(struct platform_device *pdev)
{
struct component_match *match = NULL;
+ struct msm_mdss *mdss;
struct msm_drm_private *priv;
int ret;
@@ -1237,20 +1238,22 @@ static int msm_pdev_probe(struct platform_device *pdev)
switch (get_mdp_ver(pdev)) {
case KMS_MDP5:
- ret = msm_mdss_init(pdev, true);
+ mdss = msm_mdss_init(pdev, true);
break;
case KMS_DPU:
- ret = msm_mdss_init(pdev, false);
+ mdss = msm_mdss_init(pdev, false);
break;
default:
- ret = 0;
+ mdss = NULL;
break;
}
- if (ret) {
- platform_set_drvdata(pdev, NULL);
+ if (IS_ERR(mdss)) {
+ ret = PTR_ERR(mdss);
return ret;
}
+ priv->mdss = mdss;
+
if (get_mdp_ver(pdev)) {
ret = add_display_components(pdev, );
if (ret)
@@ -1277,8 +1280,8 @@ static int msm_pdev_probe(struct platform_device *pdev)
fail:
of_platform_depopulate(>dev);
- if (priv->mdss && priv->mdss->funcs)
- priv->mdss->funcs->destroy(priv->mdss);
+ if (priv->mdss)
+ msm_mdss_destroy(priv->mdss);
return ret;
}
@@ -1291,8 +1294,8 @@ static int msm_pdev_remove(struct platform_device *pdev)
component_master_del(>dev, _drm_ops);
of_platform_depopulate(>dev);
- if (mdss && mdss->funcs)
- mdss->funcs->destroy(mdss);
+ if (mdss)
+ msm_mdss_destroy(mdss);
return 0;
}
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 10d5ae3e76df..09c21994 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -201,18 +201,12 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev);
extern const struct of_device_id dpu_dt_match[];
extern const struct of_device_id mdp5_dt_match[];
-struct msm_mdss_funcs {
- int (*enable)(struct msm_mdss *mdss);
- int (*disable)(struct msm_mdss *mdss);
- void (*destroy)(struct msm_mdss *mdss);
-};
-
-struct msm_mdss {
- struct device *dev;
- const struct msm_mdss_funcs *funcs;
-};
+struct msm_mdss;
-int msm_mdss_init(struct platform_device *pdev, bool is_mdp5);
+struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5);
+int msm_mdss_enable(struct msm_mdss *mdss);
+int msm_mdss_disable(struct msm_mdss *mdss);
+void msm_mdss_destroy(struct msm_mdss *mdss);
#define for_each_crtc_mask(dev, crtc, crtc_mask) \
drm_for_each_crtc(crtc, dev) \
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 71f3277bde32..62007a4f29a2 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -3,19 +3,16 @@
* Copyright (c) 2018, The Linux Foundation
*/
+#include
#include
#include
#include
#include
-
-#include "msm_drv.h"
-#include "msm_kms.h"
+#include
/* for DPU_HW_* defines */
#include "disp/dpu1/dpu_hw_catalog.h"
-#define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
-
#define HW_REV 0x0
#define HW_INTR_STATUS 0x0010
@@ -23,8 +20,9 @@
#define UBWC_CTRL_20x150
#define UBWC_PREDICTION_MODE 0x154
-struct dpu_mdss {
- struct msm_mdss base;
+struct msm_mdss {
+ struct device *dev;
+
void __iomem *mmio;
struct clk_bulk_data *clocks;
size_t num_clocks;
@@ -36,22 +34,22 @@ struct dpu_mdss {
static void msm_mdss_irq(struct irq_desc *desc)
{
-
[Freedreno] [PATCH v3 0/6] drm/msm: rework MDSS drivers
These patches coninue work started by AngeloGioacchino Del Regno in the previous cycle by further decoupling and dissecting MDSS and MDP drivers probe/binding paths. This removes code duplication between MDP5 and DPU1 MDSS drivers, by merging them and moving to the top level. This patchset depends on the patches 1 and 2 from [1] Changes since v3: - Rebased on top of current msm/msm-next - Fixed issue with enabling/disabling MDP4/MDP5 vs DSI driver (per Stephen's suggestion) - Reworked mdss_probe to remove extra platform_set_drvdata calls (per Stephen's suggestion) - Fixed a typo in the Kconfig (noted by Rob) - Added a patch to move component mastership from mdss to mdp5/dpu1 devices Changes since v2: - Rebased on top of current msm/msm-next(-staging) - Allow disabling MDP4/MDP5/DPU/HDMI components (like we do for DP and DSI) - Made mdp5_mdss_parse_clock() static - Changed mdp5 to is_mdp5 argument in several functions - Dropped boolean device data from the mdss driver - Reworked error handling in msm_pdev_probe() - Removed unused header inclusion - Dropped __init/__exit from function prototypes Changes since v1: - Rebased on top of [2] and [1] [1] https://patchwork.freedesktop.org/series/99066/ [2] https://patchwork.freedesktop.org/series/98521/ Dmitry Baryshkov (6): drm/msm: unify MDSS drivers drm/msm: remove extra indirection for msm_mdss drm/msm: split the main platform driver drm/msm: stop using device's match data pointer drm/msm: allow compile time selection of driver components drm/msm: make mdp5/dpu devices master components drivers/gpu/drm/msm/Kconfig | 50 ++- drivers/gpu/drm/msm/Makefile | 25 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 78 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 260 .../gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c | 3 + drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 54 ++- .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 3 + drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 54 +-- drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c | 252 drivers/gpu/drm/msm/msm_drv.c | 261 +++- drivers/gpu/drm/msm/msm_drv.h | 61 ++- drivers/gpu/drm/msm/msm_kms.h | 21 - drivers/gpu/drm/msm/msm_mdss.c| 379 ++ 13 files changed, 645 insertions(+), 856 deletions(-) delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c delete mode 100644 drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c create mode 100644 drivers/gpu/drm/msm/msm_mdss.c -- 2.35.1
[Freedreno] [PATCH v3 1/6] drm/msm: unify MDSS drivers
MDP5 and DPU1 both provide the driver handling the MDSS region, which
handles the irq domain and (incase of DPU1) adds some init for the UBWC
controller. Unify those two pieces of code into a common driver.
Reviewed-by: Abhinav Kumar
Reviewed-by: Stephen Boyd
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/Makefile | 3 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c | 252 --
drivers/gpu/drm/msm/msm_drv.c | 4 +-
drivers/gpu/drm/msm/msm_kms.h | 3 +-
.../msm/{disp/dpu1/dpu_mdss.c => msm_mdss.c} | 145 +-
5 files changed, 83 insertions(+), 324 deletions(-)
delete mode 100644 drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c
rename drivers/gpu/drm/msm/{disp/dpu1/dpu_mdss.c => msm_mdss.c} (63%)
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index e9cc7d8ac301..e76927b42033 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -42,7 +42,6 @@ msm-y := \
disp/mdp5/mdp5_crtc.o \
disp/mdp5/mdp5_encoder.o \
disp/mdp5/mdp5_irq.o \
- disp/mdp5/mdp5_mdss.o \
disp/mdp5/mdp5_kms.o \
disp/mdp5/mdp5_pipe.o \
disp/mdp5/mdp5_mixer.o \
@@ -67,7 +66,6 @@ msm-y := \
disp/dpu1/dpu_hw_util.o \
disp/dpu1/dpu_hw_vbif.o \
disp/dpu1/dpu_kms.o \
- disp/dpu1/dpu_mdss.o \
disp/dpu1/dpu_plane.o \
disp/dpu1/dpu_rm.o \
disp/dpu1/dpu_vbif.o \
@@ -88,6 +86,7 @@ msm-y := \
msm_gpu_devfreq.o \
msm_io_utils.o \
msm_iommu.o \
+ msm_mdss.o \
msm_perf.o \
msm_rd.o \
msm_ringbuffer.o \
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c
deleted file mode 100644
index 049c6784a531..
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c
+++ /dev/null
@@ -1,252 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2016, The Linux Foundation. All rights reserved.
- */
-
-#include
-#include
-
-#include "msm_drv.h"
-#include "mdp5_kms.h"
-
-#define to_mdp5_mdss(x) container_of(x, struct mdp5_mdss, base)
-
-struct mdp5_mdss {
- struct msm_mdss base;
-
- void __iomem *mmio, *vbif;
-
- struct clk *ahb_clk;
- struct clk *axi_clk;
- struct clk *vsync_clk;
-
- struct {
- volatile unsigned long enabled_mask;
- struct irq_domain *domain;
- } irqcontroller;
-};
-
-static inline void mdss_write(struct mdp5_mdss *mdp5_mdss, u32 reg, u32 data)
-{
- msm_writel(data, mdp5_mdss->mmio + reg);
-}
-
-static inline u32 mdss_read(struct mdp5_mdss *mdp5_mdss, u32 reg)
-{
- return msm_readl(mdp5_mdss->mmio + reg);
-}
-
-static irqreturn_t mdss_irq(int irq, void *arg)
-{
- struct mdp5_mdss *mdp5_mdss = arg;
- u32 intr;
-
- intr = mdss_read(mdp5_mdss, REG_MDSS_HW_INTR_STATUS);
-
- VERB("intr=%08x", intr);
-
- while (intr) {
- irq_hw_number_t hwirq = fls(intr) - 1;
-
- generic_handle_domain_irq(mdp5_mdss->irqcontroller.domain,
hwirq);
- intr &= ~(1 << hwirq);
- }
-
- return IRQ_HANDLED;
-}
-
-/*
- * interrupt-controller implementation, so sub-blocks (MDP/HDMI/eDP/DSI/etc)
- * can register to get their irq's delivered
- */
-
-#define VALID_IRQS (MDSS_HW_INTR_STATUS_INTR_MDP | \
- MDSS_HW_INTR_STATUS_INTR_DSI0 | \
- MDSS_HW_INTR_STATUS_INTR_DSI1 | \
- MDSS_HW_INTR_STATUS_INTR_HDMI | \
- MDSS_HW_INTR_STATUS_INTR_EDP)
-
-static void mdss_hw_mask_irq(struct irq_data *irqd)
-{
- struct mdp5_mdss *mdp5_mdss = irq_data_get_irq_chip_data(irqd);
-
- smp_mb__before_atomic();
- clear_bit(irqd->hwirq, _mdss->irqcontroller.enabled_mask);
- smp_mb__after_atomic();
-}
-
-static void mdss_hw_unmask_irq(struct irq_data *irqd)
-{
- struct mdp5_mdss *mdp5_mdss = irq_data_get_irq_chip_data(irqd);
-
- smp_mb__before_atomic();
- set_bit(irqd->hwirq, _mdss->irqcontroller.enabled_mask);
- smp_mb__after_atomic();
-}
-
-static struct irq_chip mdss_hw_irq_chip = {
- .name = "mdss",
- .irq_mask = mdss_hw_mask_irq,
- .irq_unmask = mdss_hw_unmask_irq,
-};
-
-static int mdss_hw_irqdomain_map(struct irq_domain *d, unsigned int irq,
-irq_hw_number_t hwirq)
-{
- struct mdp5_mdss *mdp5_mdss = d->host_data;
-
- if (!(VALID_IRQS & (1 << hwirq)))
- return -EPERM;
-
- irq_set_chip_and_handler(irq, _hw_irq_chip, handle_level_irq);
- irq_set_chip_data(irq, mdp5_mdss);
-
- return 0;
-}
-
-static const struct irq_domain_ops mdss_hw_irqdomain_ops = {
- .map = mdss_hw_irqdomain_map,
- .xlate = irq_domain_xlate_onecell,
-};
-
-
-static int mdss_irq_domain_init(struct mdp5_mdss *mdp5_mdss)
-{
- struct device *dev = mdp5_mdss->base.dev;
- struct
