Re: [Freedreno] [PATCH RESEND 0/6] drm: simplify support for transparent DRM bridges
On Sun, Dec 03, 2023 at 02:43:27PM +0300, Dmitry Baryshkov wrote: > Greg, could you please ack the last patch to be merged through the > drm-misc tree? You have acked patch 3, but since that time I've added > patches 4-6. That is up to the typec maintainer to ack, not me! thanks, greg k-h
[Freedreno] [PATCH 5.15 026/297] drm/msm/dp: skip validity check for DP CTS EDID checksum
5.15-stable review patch. If anyone has any objections, please let me know.
--
From: Jani Nikula
[ Upstream commit a251c9d8e30833b260101edb9383b176ee2b7cb1 ]
The DP CTS test for EDID last block checksum expects the checksum for
the last block, invalid or not. Skip the validity check.
For the most part (*), the EDIDs returned by drm_get_edid() will be
valid anyway, and there's the CTS workaround to get the checksum for
completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
correct edid checksum after corrupted edid checksum read").
This lets us remove one user of drm_edid_block_valid() with hopes the
function can be removed altogether in the future.
(*) drm_get_edid() ignores checksum errors on CTA extensions.
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Kuogee Hsieh
Cc: Marijn Suijten
Cc: Rob Clark
Cc: Sean Paul
Cc: Stephen Boyd
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Jani Nikula
Reviewed-by: Stephen Boyd
Reviewed-by: Abhinav Kumar
Reviewed-by: Kuogee Hsieh
Patchwork: https://patchwork.freedesktop.org/patch/555361/
Link: https://lore.kernel.org/r/20230901142034.580802-1-jani.nik...@intel.com
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_panel.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 62b742e701d2c..f9d31069f4848 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -263,26 +263,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
static u8 dp_panel_get_edid_checksum(struct edid *edid)
{
- struct edid *last_block;
- u8 *raw_edid;
- bool is_edid_corrupt = false;
+ edid += edid->extensions;
- if (!edid) {
- DRM_ERROR("invalid edid input\n");
- return 0;
- }
-
- raw_edid = (u8 *)edid;
- raw_edid += (edid->extensions * EDID_LENGTH);
- last_block = (struct edid *)raw_edid;
-
- /* block type extension */
- drm_edid_block_valid(raw_edid, 1, false, _edid_corrupt);
- if (!is_edid_corrupt)
- return last_block->checksum;
-
- DRM_ERROR("Invalid block, no checksum\n");
- return 0;
+ return edid->checksum;
}
void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
--
2.42.0
[Freedreno] [PATCH 5.10 018/193] drm/msm/dp: skip validity check for DP CTS EDID checksum
5.10-stable review patch. If anyone has any objections, please let me know.
--
From: Jani Nikula
[ Upstream commit a251c9d8e30833b260101edb9383b176ee2b7cb1 ]
The DP CTS test for EDID last block checksum expects the checksum for
the last block, invalid or not. Skip the validity check.
For the most part (*), the EDIDs returned by drm_get_edid() will be
valid anyway, and there's the CTS workaround to get the checksum for
completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
correct edid checksum after corrupted edid checksum read").
This lets us remove one user of drm_edid_block_valid() with hopes the
function can be removed altogether in the future.
(*) drm_get_edid() ignores checksum errors on CTA extensions.
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Kuogee Hsieh
Cc: Marijn Suijten
Cc: Rob Clark
Cc: Sean Paul
Cc: Stephen Boyd
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Jani Nikula
Reviewed-by: Stephen Boyd
Reviewed-by: Abhinav Kumar
Reviewed-by: Kuogee Hsieh
Patchwork: https://patchwork.freedesktop.org/patch/555361/
Link: https://lore.kernel.org/r/20230901142034.580802-1-jani.nik...@intel.com
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_panel.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 4e8a19114e87d..93a2ee0f772fc 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -264,26 +264,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
static u8 dp_panel_get_edid_checksum(struct edid *edid)
{
- struct edid *last_block;
- u8 *raw_edid;
- bool is_edid_corrupt = false;
+ edid += edid->extensions;
- if (!edid) {
- DRM_ERROR("invalid edid input\n");
- return 0;
- }
-
- raw_edid = (u8 *)edid;
- raw_edid += (edid->extensions * EDID_LENGTH);
- last_block = (struct edid *)raw_edid;
-
- /* block type extension */
- drm_edid_block_valid(raw_edid, 1, false, _edid_corrupt);
- if (!is_edid_corrupt)
- return last_block->checksum;
-
- DRM_ERROR("Invalid block, no checksum\n");
- return 0;
+ return edid->checksum;
}
void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
--
2.42.0
[Freedreno] [PATCH 6.1 042/372] drm/msm/dp: skip validity check for DP CTS EDID checksum
6.1-stable review patch. If anyone has any objections, please let me know.
--
From: Jani Nikula
[ Upstream commit a251c9d8e30833b260101edb9383b176ee2b7cb1 ]
The DP CTS test for EDID last block checksum expects the checksum for
the last block, invalid or not. Skip the validity check.
For the most part (*), the EDIDs returned by drm_get_edid() will be
valid anyway, and there's the CTS workaround to get the checksum for
completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
correct edid checksum after corrupted edid checksum read").
This lets us remove one user of drm_edid_block_valid() with hopes the
function can be removed altogether in the future.
(*) drm_get_edid() ignores checksum errors on CTA extensions.
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Kuogee Hsieh
Cc: Marijn Suijten
Cc: Rob Clark
Cc: Sean Paul
Cc: Stephen Boyd
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Jani Nikula
Reviewed-by: Stephen Boyd
Reviewed-by: Abhinav Kumar
Reviewed-by: Kuogee Hsieh
Patchwork: https://patchwork.freedesktop.org/patch/555361/
Link: https://lore.kernel.org/r/20230901142034.580802-1-jani.nik...@intel.com
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_panel.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 5149cebc93f61..d38086650fcf7 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -266,26 +266,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
static u8 dp_panel_get_edid_checksum(struct edid *edid)
{
- struct edid *last_block;
- u8 *raw_edid;
- bool is_edid_corrupt = false;
+ edid += edid->extensions;
- if (!edid) {
- DRM_ERROR("invalid edid input\n");
- return 0;
- }
-
- raw_edid = (u8 *)edid;
- raw_edid += (edid->extensions * EDID_LENGTH);
- last_block = (struct edid *)raw_edid;
-
- /* block type extension */
- drm_edid_block_valid(raw_edid, 1, false, _edid_corrupt);
- if (!is_edid_corrupt)
- return last_block->checksum;
-
- DRM_ERROR("Invalid block, no checksum\n");
- return 0;
+ return edid->checksum;
}
void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
--
2.42.0
[Freedreno] [PATCH 6.5 062/491] drm/msm/dp: skip validity check for DP CTS EDID checksum
6.5-stable review patch. If anyone has any objections, please let me know.
--
From: Jani Nikula
[ Upstream commit a251c9d8e30833b260101edb9383b176ee2b7cb1 ]
The DP CTS test for EDID last block checksum expects the checksum for
the last block, invalid or not. Skip the validity check.
For the most part (*), the EDIDs returned by drm_get_edid() will be
valid anyway, and there's the CTS workaround to get the checksum for
completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
correct edid checksum after corrupted edid checksum read").
This lets us remove one user of drm_edid_block_valid() with hopes the
function can be removed altogether in the future.
(*) drm_get_edid() ignores checksum errors on CTA extensions.
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Kuogee Hsieh
Cc: Marijn Suijten
Cc: Rob Clark
Cc: Sean Paul
Cc: Stephen Boyd
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Jani Nikula
Reviewed-by: Stephen Boyd
Reviewed-by: Abhinav Kumar
Reviewed-by: Kuogee Hsieh
Patchwork: https://patchwork.freedesktop.org/patch/555361/
Link: https://lore.kernel.org/r/20230901142034.580802-1-jani.nik...@intel.com
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_panel.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 42d52510ffd4a..86a8e06c7a60f 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -289,26 +289,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
static u8 dp_panel_get_edid_checksum(struct edid *edid)
{
- struct edid *last_block;
- u8 *raw_edid;
- bool is_edid_corrupt = false;
+ edid += edid->extensions;
- if (!edid) {
- DRM_ERROR("invalid edid input\n");
- return 0;
- }
-
- raw_edid = (u8 *)edid;
- raw_edid += (edid->extensions * EDID_LENGTH);
- last_block = (struct edid *)raw_edid;
-
- /* block type extension */
- drm_edid_block_valid(raw_edid, 1, false, _edid_corrupt);
- if (!is_edid_corrupt)
- return last_block->checksum;
-
- DRM_ERROR("Invalid block, no checksum\n");
- return 0;
+ return edid->checksum;
}
void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
--
2.42.0
[Freedreno] [PATCH 6.6 064/530] drm/msm/dp: skip validity check for DP CTS EDID checksum
6.6-stable review patch. If anyone has any objections, please let me know.
--
From: Jani Nikula
[ Upstream commit a251c9d8e30833b260101edb9383b176ee2b7cb1 ]
The DP CTS test for EDID last block checksum expects the checksum for
the last block, invalid or not. Skip the validity check.
For the most part (*), the EDIDs returned by drm_get_edid() will be
valid anyway, and there's the CTS workaround to get the checksum for
completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
correct edid checksum after corrupted edid checksum read").
This lets us remove one user of drm_edid_block_valid() with hopes the
function can be removed altogether in the future.
(*) drm_get_edid() ignores checksum errors on CTA extensions.
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Kuogee Hsieh
Cc: Marijn Suijten
Cc: Rob Clark
Cc: Sean Paul
Cc: Stephen Boyd
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Jani Nikula
Reviewed-by: Stephen Boyd
Reviewed-by: Abhinav Kumar
Reviewed-by: Kuogee Hsieh
Patchwork: https://patchwork.freedesktop.org/patch/555361/
Link: https://lore.kernel.org/r/20230901142034.580802-1-jani.nik...@intel.com
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_panel.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 42d52510ffd4a..86a8e06c7a60f 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -289,26 +289,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
static u8 dp_panel_get_edid_checksum(struct edid *edid)
{
- struct edid *last_block;
- u8 *raw_edid;
- bool is_edid_corrupt = false;
+ edid += edid->extensions;
- if (!edid) {
- DRM_ERROR("invalid edid input\n");
- return 0;
- }
-
- raw_edid = (u8 *)edid;
- raw_edid += (edid->extensions * EDID_LENGTH);
- last_block = (struct edid *)raw_edid;
-
- /* block type extension */
- drm_edid_block_valid(raw_edid, 1, false, _edid_corrupt);
- if (!is_edid_corrupt)
- return last_block->checksum;
-
- DRM_ERROR("Invalid block, no checksum\n");
- return 0;
+ return edid->checksum;
}
void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
--
2.42.0
Re: [Freedreno] [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
On Tue, Nov 07, 2023 at 01:18:14PM +0100, Maxime Ripard wrote: > On Tue, Nov 07, 2023 at 12:22:21PM +0100, Greg Kroah-Hartman wrote: > > On Tue, Nov 07, 2023 at 11:57:49AM +0100, Maxime Ripard wrote: > > > +GKH > > > > Why? I don't see a question for me here, sorry. > > I guess the question is: we have a bus with various power states > (powered off, low power, high speed) Great, have fun! And is this per-device or per-bus-instance? > low power is typically used to send commands to a device, high speed to > transmit pixels, but still allows to send commands. > > Depending on the devices, there's different requirements about the state > devices expect the bus to be in to send commands. Some will need to send > all the commands in the low power state, some don't care, etc. See > the mail I was replying too for more details. > > We've tried so far to model that in KMS itself, so the framework the > drivers would register too, but we're kind of reaching the limits of > what we can do there. It also feels to me that "the driver can't access > its device" is more of a problem for the bus to solve rather than the > framework. This is up to the specific bus to resolve, there's nothing special needed in the driver core for it, right? > Do you agree? Are you aware of any other bus in Linux with similar > requirements we could look at? Or any suggestion on how to solve it? There might be others, yes, look at how the dynamic power management works for different devices on most busses, that might help you out here. good luck! greg k-h
Re: [Freedreno] [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
On Tue, Nov 07, 2023 at 11:57:49AM +0100, Maxime Ripard wrote: > +GKH Why? I don't see a question for me here, sorry. greg k-h
[Freedreno] [PATCH 5.4 119/367] drm/msm/mdp5: Dont leak some plane state
5.4-stable review patch. If anyone has any objections, please let me know.
--
From: Daniel Vetter
[ Upstream commit fd0ad3b2365c1c58aa5a761c18efc4817193beb6 ]
Apparently no one noticed that mdp5 plane states leak like a sieve
ever since we introduced plane_state->commit refcount a few years ago
in 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too
early by tracking commits, v3.")
Fix it by using the right helpers.
Fixes: 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too early
by tracking commits, v3.")
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reported-and-tested-by: do...@noisolation.com
Cc: do...@noisolation.com
Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Patchwork: https://patchwork.freedesktop.org/patch/551236/
Link: https://lore.kernel.org/r/20230803204521.928582-1-daniel.vet...@ffwll.ch
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 0dc23c86747e8..e1c1b4ad5ed04 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -221,8 +221,7 @@ static void mdp5_plane_destroy_state(struct drm_plane
*plane,
{
struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
- if (state->fb)
- drm_framebuffer_put(state->fb);
+ __drm_atomic_helper_plane_destroy_state(state);
kfree(pstate);
}
--
2.40.1
[Freedreno] [PATCH 4.19 101/273] drm/msm/mdp5: Dont leak some plane state
4.19-stable review patch. If anyone has any objections, please let me know.
--
From: Daniel Vetter
[ Upstream commit fd0ad3b2365c1c58aa5a761c18efc4817193beb6 ]
Apparently no one noticed that mdp5 plane states leak like a sieve
ever since we introduced plane_state->commit refcount a few years ago
in 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too
early by tracking commits, v3.")
Fix it by using the right helpers.
Fixes: 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too early
by tracking commits, v3.")
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reported-and-tested-by: do...@noisolation.com
Cc: do...@noisolation.com
Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Patchwork: https://patchwork.freedesktop.org/patch/551236/
Link: https://lore.kernel.org/r/20230803204521.928582-1-daniel.vet...@ffwll.ch
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 0cbc43f61d9c7..cd5b9ee22a5bb 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -230,8 +230,7 @@ static void mdp5_plane_destroy_state(struct drm_plane
*plane,
{
struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
- if (state->fb)
- drm_framebuffer_put(state->fb);
+ __drm_atomic_helper_plane_destroy_state(state);
kfree(pstate);
}
--
2.40.1
[Freedreno] [PATCH 5.15 173/511] drm/msm/mdp5: Dont leak some plane state
5.15-stable review patch. If anyone has any objections, please let me know.
--
From: Daniel Vetter
[ Upstream commit fd0ad3b2365c1c58aa5a761c18efc4817193beb6 ]
Apparently no one noticed that mdp5 plane states leak like a sieve
ever since we introduced plane_state->commit refcount a few years ago
in 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too
early by tracking commits, v3.")
Fix it by using the right helpers.
Fixes: 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too early
by tracking commits, v3.")
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reported-and-tested-by: do...@noisolation.com
Cc: do...@noisolation.com
Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Patchwork: https://patchwork.freedesktop.org/patch/551236/
Link: https://lore.kernel.org/r/20230803204521.928582-1-daniel.vet...@ffwll.ch
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 9c42776cb9a8a..f9cae6460c3be 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -128,8 +128,7 @@ static void mdp5_plane_destroy_state(struct drm_plane
*plane,
{
struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
- if (state->fb)
- drm_framebuffer_put(state->fb);
+ __drm_atomic_helper_plane_destroy_state(state);
kfree(pstate);
}
--
2.40.1
[Freedreno] [PATCH 5.10 158/406] drm/msm/mdp5: Dont leak some plane state
5.10-stable review patch. If anyone has any objections, please let me know.
--
From: Daniel Vetter
[ Upstream commit fd0ad3b2365c1c58aa5a761c18efc4817193beb6 ]
Apparently no one noticed that mdp5 plane states leak like a sieve
ever since we introduced plane_state->commit refcount a few years ago
in 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too
early by tracking commits, v3.")
Fix it by using the right helpers.
Fixes: 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too early
by tracking commits, v3.")
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reported-and-tested-by: do...@noisolation.com
Cc: do...@noisolation.com
Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Patchwork: https://patchwork.freedesktop.org/patch/551236/
Link: https://lore.kernel.org/r/20230803204521.928582-1-daniel.vet...@ffwll.ch
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 0dc23c86747e8..e1c1b4ad5ed04 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -221,8 +221,7 @@ static void mdp5_plane_destroy_state(struct drm_plane
*plane,
{
struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
- if (state->fb)
- drm_framebuffer_put(state->fb);
+ __drm_atomic_helper_plane_destroy_state(state);
kfree(pstate);
}
--
2.40.1
[Freedreno] [PATCH 6.1 293/600] drm/msm/mdp5: Dont leak some plane state
6.1-stable review patch. If anyone has any objections, please let me know.
--
From: Daniel Vetter
[ Upstream commit fd0ad3b2365c1c58aa5a761c18efc4817193beb6 ]
Apparently no one noticed that mdp5 plane states leak like a sieve
ever since we introduced plane_state->commit refcount a few years ago
in 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too
early by tracking commits, v3.")
Fix it by using the right helpers.
Fixes: 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too early
by tracking commits, v3.")
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reported-and-tested-by: do...@noisolation.com
Cc: do...@noisolation.com
Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Patchwork: https://patchwork.freedesktop.org/patch/551236/
Link: https://lore.kernel.org/r/20230803204521.928582-1-daniel.vet...@ffwll.ch
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index bd2c4ac456017..0d5ff03cb0910 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -130,8 +130,7 @@ static void mdp5_plane_destroy_state(struct drm_plane
*plane,
{
struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
- if (state->fb)
- drm_framebuffer_put(state->fb);
+ __drm_atomic_helper_plane_destroy_state(state);
kfree(pstate);
}
--
2.40.1
[Freedreno] [PATCH 6.4 358/737] drm/msm/mdp5: Dont leak some plane state
6.4-stable review patch. If anyone has any objections, please let me know.
--
From: Daniel Vetter
[ Upstream commit fd0ad3b2365c1c58aa5a761c18efc4817193beb6 ]
Apparently no one noticed that mdp5 plane states leak like a sieve
ever since we introduced plane_state->commit refcount a few years ago
in 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too
early by tracking commits, v3.")
Fix it by using the right helpers.
Fixes: 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too early
by tracking commits, v3.")
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reported-and-tested-by: do...@noisolation.com
Cc: do...@noisolation.com
Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Patchwork: https://patchwork.freedesktop.org/patch/551236/
Link: https://lore.kernel.org/r/20230803204521.928582-1-daniel.vet...@ffwll.ch
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index bd2c4ac456017..0d5ff03cb0910 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -130,8 +130,7 @@ static void mdp5_plane_destroy_state(struct drm_plane
*plane,
{
struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
- if (state->fb)
- drm_framebuffer_put(state->fb);
+ __drm_atomic_helper_plane_destroy_state(state);
kfree(pstate);
}
--
2.40.1
[Freedreno] [PATCH 6.5 309/739] drm/msm/mdp5: Dont leak some plane state
6.5-stable review patch. If anyone has any objections, please let me know.
--
From: Daniel Vetter
[ Upstream commit fd0ad3b2365c1c58aa5a761c18efc4817193beb6 ]
Apparently no one noticed that mdp5 plane states leak like a sieve
ever since we introduced plane_state->commit refcount a few years ago
in 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too
early by tracking commits, v3.")
Fix it by using the right helpers.
Fixes: 21a01abbe32a ("drm/atomic: Fix freeing connector/plane state too early
by tracking commits, v3.")
Cc: Maarten Lankhorst
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reported-and-tested-by: do...@noisolation.com
Cc: do...@noisolation.com
Signed-off-by: Daniel Vetter
Reviewed-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Patchwork: https://patchwork.freedesktop.org/patch/551236/
Link: https://lore.kernel.org/r/20230803204521.928582-1-daniel.vet...@ffwll.ch
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index bd2c4ac456017..0d5ff03cb0910 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -130,8 +130,7 @@ static void mdp5_plane_destroy_state(struct drm_plane
*plane,
{
struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
- if (state->fb)
- drm_framebuffer_put(state->fb);
+ __drm_atomic_helper_plane_destroy_state(state);
kfree(pstate);
}
--
2.40.1
Re: [Freedreno] [PATCH v4 3/3] usb: typec: nb7vpq904m: switch to DRM_AUX_BRIDGE
On Thu, Aug 17, 2023 at 05:55:16PM +0300, Dmitry Baryshkov wrote: > Switch to using the new DRM_AUX_BRIDGE helper to create the > transparent DRM bridge device instead of handcoding corresponding > functionality. > > Reviewed-by: Heikki Krogerus > Signed-off-by: Dmitry Baryshkov > --- > drivers/usb/typec/mux/Kconfig | 2 +- > drivers/usb/typec/mux/nb7vpq904m.c | 44 ++ > 2 files changed, 3 insertions(+), 43 deletions(-) Just take this through the drm tree: Acked-by: Greg Kroah-Hartman
[Freedreno] [PATCH 6.1 069/223] drm/client: Send hotplug event after registering a client
file in 6.1.y because
these commits haven't happened in 6.1.y.
8ab59da26bc0 drm/fb-helper: Move generic fbdev emulation into separate source
file
b9c93f4ec737 drm/fbdev-generic: Rename symbols ]
Cc: alexandru.gagn...@hp.com
Link:
https://lore.kernel.org/stable/sj0pr84mb20882eea1abb36f60e845e378f...@sj0pr84mb2088.namprd84.prod.outlook.com/
Signed-off-by: Mario Limonciello
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_client.c| 21 +
drivers/gpu/drm/drm_fb_helper.c |4
2 files changed, 21 insertions(+), 4 deletions(-)
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -122,13 +122,34 @@ EXPORT_SYMBOL(drm_client_init);
* drm_client_register() it is no longer permissible to call
drm_client_release()
* directly (outside the unregister callback), instead cleanup will happen
* automatically on driver unload.
+ *
+ * Registering a client generates a hotplug event that allows the client
+ * to set up its display from pre-existing outputs. The client must have
+ * initialized its state to able to handle the hotplug event successfully.
*/
void drm_client_register(struct drm_client_dev *client)
{
struct drm_device *dev = client->dev;
+ int ret;
mutex_lock(>clientlist_mutex);
list_add(>list, >clientlist);
+
+ if (client->funcs && client->funcs->hotplug) {
+ /*
+* Perform an initial hotplug event to pick up the
+* display configuration for the client. This step
+* has to be performed *after* registering the client
+* in the list of clients, or a concurrent hotplug
+* event might be lost; leaving the display off.
+*
+* Hold the clientlist_mutex as for a regular hotplug
+* event.
+*/
+ ret = client->funcs->hotplug(client);
+ if (ret)
+ drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
+ }
mutex_unlock(>clientlist_mutex);
}
EXPORT_SYMBOL(drm_client_register);
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2634,10 +2634,6 @@ void drm_fbdev_generic_setup(struct drm_
preferred_bpp = 32;
fb_helper->preferred_bpp = preferred_bpp;
- ret = drm_fbdev_client_hotplug(_helper->client);
- if (ret)
- drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
-
drm_client_register(_helper->client);
}
EXPORT_SYMBOL(drm_fbdev_generic_setup);
[Freedreno] [PATCH 6.4 111/292] drm/client: Send hotplug event after registering a client
From: Thomas Zimmermann
commit 27655b9bb9f0d9c32b8de8bec649b676898c52d5 upstream.
Generate a hotplug event after registering a client to allow the
client to configure its display. Remove the hotplug calls from the
existing clients for fbdev emulation. This change fixes a concurrency
bug between registering a client and receiving events from the DRM
core. The bug is present in the fbdev emulation of all drivers.
The fbdev emulation currently generates a hotplug event before
registering the client to the device. For each new output, the DRM
core sends an additional hotplug event to each registered client.
If the DRM core detects first output between sending the artificial
hotplug and registering the device, the output's hotplug event gets
lost. If this is the first output, the fbdev console display remains
dark. This has been observed with amdgpu and fbdev-generic.
Fix this by adding hotplug generation directly to the client's
register helper drm_client_register(). Registering the client and
receiving events are serialized by struct drm_device.clientlist_mutex.
So an output is either configured by the initial hotplug event, or
the client has already been registered.
The bug was originally added in commit 6e3f17ee73f7 ("drm/fb-helper:
generic: Call drm_client_add() after setup is done"), in which adding
a client and receiving a hotplug event switched order. It was hidden,
as most hardware and drivers have at least on static output configured.
Other drivers didn't use the internal DRM client or still had struct
drm_mode_config_funcs.output_poll_changed set. That callback handled
hotplug events as well. After not setting the callback in amdgpu in
commit 0e3172bac3f4 ("drm/amdgpu: Don't set struct
drm_driver.output_poll_changed"), amdgpu did not show a framebuffer
console if output events got lost. The bug got copy-pasted from
fbdev-generic into the other fbdev emulation.
Reported-by: Moritz Duge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2649
Fixes: 6e3f17ee73f7 ("drm/fb-helper: generic: Call drm_client_add() after setup
is done")
Fixes: 8ab59da26bc0 ("drm/fb-helper: Move generic fbdev emulation into separate
source file")
Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for GEM DMA
helpers")
Fixes: 63c381552f69 ("drm/armada: Implement fbdev emulation as in-kernel
client")
Fixes: 49953b70e7d3 ("drm/exynos: Implement fbdev emulation as in-kernel
client")
Fixes: 8f1aaccb04b7 ("drm/gma500: Implement client-based fbdev emulation")
Fixes: 940b869c2f2f ("drm/msm: Implement fbdev emulation as in-kernel client")
Fixes: 9e69bcd88e45 ("drm/omapdrm: Implement fbdev emulation as in-kernel
client")
Fixes: e317a69fe891 ("drm/radeon: Implement client-based fbdev emulation")
Fixes: 71ec16f45ef8 ("drm/tegra: Implement fbdev emulation as in-kernel client")
Fixes: 0e3172bac3f4 ("drm/amdgpu: Don't set struct
drm_driver.output_poll_changed")
Signed-off-by: Thomas Zimmermann
Tested-by: Moritz Duge
Tested-by: Torsten Krah
Tested-by: Paul Schyska
Cc: Daniel Vetter
Cc: David Airlie
Cc: Noralf Trønnes
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Javier Martinez Canillas
Cc: Russell King
Cc: Inki Dae
Cc: Seung-Woo Kim
Cc: Kyungmin Park
Cc: Krzysztof Kozlowski
Cc: Patrik Jakobsson
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Tomi Valkeinen
Cc: Alex Deucher
Cc: "Christian König"
Cc: "Pan, Xinhui"
Cc: Thierry Reding
Cc: Mikko Perttunen
Cc: dri-de...@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Cc: linux-te...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: # v5.2+
Reviewed-by: Javier Martinez Canillas
Reviewed-by: Dmitry Baryshkov # msm
Link:
https://patchwork.freedesktop.org/patch/msgid/20230710091029.27503-1-tzimmerm...@suse.de
[ Dropped changes to drivers/gpu/drm/armada/armada_fbdev.c as
174c3c38e3a2 drm/armada: Initialize fbdev DRM client
was introduced in 6.5-rc1 ]
Signed-off-by: Mario Limonciello
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_client.c | 21 +
drivers/gpu/drm/drm_fbdev_dma.c |4
drivers/gpu/drm/drm_fbdev_generic.c |4
drivers/gpu/drm/exynos/exynos_drm_fbdev.c |4
drivers/gpu/drm/gma500/fbdev.c|4
drivers/gpu/drm/msm/msm_fbdev.c |4
drivers/gpu/drm/omapdrm/omap_fbdev.c |4
drivers/gpu/drm/radeon/radeon_fbdev.c |4
drivers/gpu/drm/tegra/fbdev.c |4
9 files changed, 21 insertions(+), 32 deletions(-)
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -122,13 +122,34 @@ EXPORT_SYMBOL(drm_cli
[Freedreno] [PATCH 5.10 514/783] drm/msm: Use drm_mode_copy()
From: Ville Syrjälä
[ Upstream commit b2a1c5ca50db22b3677676dd5bad5f6092429acf ]
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
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Reviewed-by: Daniel Vetter
Signed-off-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20221107192545.9896-5-ville.syrj...@linux.intel.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 5a152d505dfb..1c3dcbc6cce8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -848,7 +848,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);
--
2.35.1
[Freedreno] [PATCH 6.1 1002/1146] drm/msm: Use drm_mode_copy()
From: Ville Syrjälä
[ Upstream commit b2a1c5ca50db22b3677676dd5bad5f6092429acf ]
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
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Reviewed-by: Daniel Vetter
Signed-off-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20221107192545.9896-5-ville.syrj...@linux.intel.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index a49f6dbbe888..c9d9b384ddd0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -857,7 +857,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);
--
2.35.1
[Freedreno] [PATCH 6.0 0937/1073] drm/msm: Use drm_mode_copy()
From: Ville Syrjälä
[ Upstream commit b2a1c5ca50db22b3677676dd5bad5f6092429acf ]
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
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Reviewed-by: Daniel Vetter
Signed-off-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20221107192545.9896-5-ville.syrj...@linux.intel.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index a49f6dbbe888..c9d9b384ddd0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -857,7 +857,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);
--
2.35.1
[Freedreno] [PATCH 5.15 649/731] drm/msm: Use drm_mode_copy()
From: Ville Syrjälä
[ Upstream commit b2a1c5ca50db22b3677676dd5bad5f6092429acf ]
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
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
Reviewed-by: Daniel Vetter
Signed-off-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20221107192545.9896-5-ville.syrj...@linux.intel.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index d13fd39f05de..15e38ad7aefb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -840,7 +840,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);
--
2.35.1
Re: [Freedreno] [PATCH] device property: Fix recent breakage of fwnode_get_next_parent_dev()
On Fri, May 13, 2022 at 08:58:12PM -0700, Saravana Kannan wrote:
> On Thu, May 5, 2022 at 5:21 AM Rafael J. Wysocki wrote:
> >
> > On Sun, May 1, 2022 at 9:50 AM Andy Shevchenko
> > wrote:
> > >
> > > On Sat, Apr 30, 2022 at 3:00 PM Douglas Anderson
> > > wrote:
> > > >
> > > > Due to a subtle typo, instead of commit 87ffea09470d ("device
> > > > property: Introduce fwnode_for_each_parent_node()") being a no-op
> > > > change, it ended up causing the display on my sc7180-trogdor-lazor
> > > > device from coming up unless I added "fw_devlink=off" to my kernel
> > > > command line. Fix the typo.
> > >
> > > Sorry and merci pour la fix!
> > > Reviewed-by: Andy Shevchenko
> >
> > Applied, thanks!
>
> Has this been picked up by one of the driver-core branches yet? I was
> poking around that
> git repo and didn't see any commit with this title. This breaks
> fw_devlink in a severe manner, so I want to make sure it gets into
> 5.18.
Look in linux-next:
117ef574074d ("device property: Fix recent breakage of
fwnode_get_next_parent_dev()")
Rafael's tree is not my tree :)
thanks,
greg k-h
Re: [Freedreno] Adding CI results to the kernel tree was Re: [RFC v2] drm/msm: Add initial ci/ subdirectory
On Wed, May 11, 2022 at 12:26:05PM +0200, Michel Dänzer wrote: > On 2022-05-11 08:22, Greg Kroah-Hartman wrote: > > On Wed, May 11, 2022 at 03:06:47PM +1000, Dave Airlie wrote: > >>> And use it to store expectations about what the drm/msm driver is > >>> supposed to pass in the IGT test suite. > >> > >> I wanted to loop in Linus/Greg to see if there are any issues raised > >> by adding CI results file to the tree in their minds, or if any other > >> subsystem has done this already, and it's all fine. > > > > Why does the results need to be added to the tree? Shouldn't they be > > either "all is good" or "constantly changing and a constant churn"? > > > >> I think this is a good thing after our Mesa experience, but Mesa has a > >> lot tighter integration here, so I want to get some more opinions > >> outside the group. > > > > For systems that have "tight integration" this might make sense as proof > > that all is working for a specific commit, but I can't see how this will > > help the kernel out much. > > > > What are you going to do with these results being checked in all the > > time? > > Having the expected results in the tree keeps them consistent with the driver > code itself, and allows putting in place gating CI to prevent merging driver > changes which make any of the tests deviate from the expected result. Shouldn't "expected result" always be "pass"? If not, then the test should be changed to be "skipped" like we have today in the kselftest tests. And how about tieing this into the kselftest process as well, why would this be somehow separate from the rest of the kernel tests? > Keeping them separate inevitably results in divergence between the driver > code and the expected test results, which would result in spurious failures > of such CI. Again, "pass" should be the expected results :) > I expect the main complication for the kernel will be due to driver changes > merged via different trees, e.g. for cross-subsystem reworks. Since those > will not go through the same CI, they may accidentally introduce > inconsistencies. The ideal solution for this IMO would be centralizing CI > such that the same gating tests have to pass regardless of how the code is > merged. But there's likely quite a long way to go until we get there. :) We have in-kernel tests for the rest of the kernel, why can't you put your testing stuff into there as well? thanks, greg k-h
Re: [Freedreno] Adding CI results to the kernel tree was Re: [RFC v2] drm/msm: Add initial ci/ subdirectory
On Wed, May 11, 2022 at 03:06:47PM +1000, Dave Airlie wrote: > > And use it to store expectations about what the drm/msm driver is > > supposed to pass in the IGT test suite. > > I wanted to loop in Linus/Greg to see if there are any issues raised > by adding CI results file to the tree in their minds, or if any other > subsystem has done this already, and it's all fine. Why does the results need to be added to the tree? Shouldn't they be either "all is good" or "constantly changing and a constant churn"? > I think this is a good thing after our Mesa experience, but Mesa has a > lot tighter integration here, so I want to get some more opinions > outside the group. For systems that have "tight integration" this might make sense as proof that all is working for a specific commit, but I can't see how this will help the kernel out much. What are you going to do with these results being checked in all the time? thanks, greg k-h
Re: [Freedreno] [PATCH 1/2] drm: Add HPD state to drm_connector_oob_hotplug_event()
On Mon, Feb 07, 2022 at 08:43:27PM -0800, Bjorn Andersson wrote:
> In some implementations, such as the Qualcomm platforms, the display
> driver has no way to query the current HPD state and as such it's
> impossible to distinguish between disconnect and attention events.
>
> Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD
> state.
>
> Also push the test for unchanged state in the displayport altmode driver
> into the i915 driver, to allow other drivers to act upon each update.
>
> Signed-off-by: Bjorn Andersson
> ---
>
> Note that the Intel driver has only been compile tested with this patch.
>
> drivers/gpu/drm/drm_connector.c | 6 --
> drivers/gpu/drm/i915/display/intel_dp.c | 14 +++---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/usb/typec/altmodes/displayport.c | 9 ++---
> include/drm/drm_connector.h | 5 +++--
> 5 files changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index a50c82bc2b2f..ad7295597c0f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2825,6 +2825,7 @@ struct drm_connector
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> /**
> * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to
> connector
> * @connector_fwnode: fwnode_handle to report the event on
> + * @hpd_state: number of data lanes available
"number"?
> *
> * On some hardware a hotplug event notification may come from outside the
> display
> * driver / device. An example of this is some USB Type-C setups where the
> hardware
> @@ -2834,7 +2835,8 @@ struct drm_connector
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> * This function can be used to report these out-of-band events after
> obtaining
> * a drm_connector reference through calling drm_connector_find_by_fwnode().
> */
> -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> + bool hpd_state)
This is a boolean, how can it be a number?
And having a "flag" like this is a pain, how do you know what the
parameter really means?
> {
> struct drm_connector *connector;
>
> @@ -2843,7 +2845,7 @@ void drm_connector_oob_hotplug_event(struct
> fwnode_handle *connector_fwnode)
> return;
>
> if (connector->funcs->oob_hotplug_event)
> - connector->funcs->oob_hotplug_event(connector);
> + connector->funcs->oob_hotplug_event(connector, hpd_state);
>
> drm_connector_put(connector);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 146b83916005..00520867d37b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4816,15 +4816,23 @@ static int intel_dp_connector_atomic_check(struct
> drm_connector *conn,
> return intel_modeset_synced_crtcs(state, conn);
> }
>
> -static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
> +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, bool
> hpd_state)
> {
> struct intel_encoder *encoder =
> intel_attached_encoder(to_intel_connector(connector));
> struct drm_i915_private *i915 = to_i915(connector->dev);
> + bool need_work = false;
>
> spin_lock_irq(>irq_lock);
> - i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> + if (hpd_state != i915->hotplug.oob_hotplug_state) {
> + i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> +
> + i915->hotplug.oob_hotplug_state = hpd_state;
> + need_work = true;
> + }
> spin_unlock_irq(>irq_lock);
> - queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
> +
> + if (need_work)
> + queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
> }
>
> static const struct drm_connector_funcs intel_dp_connector_funcs = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8c1706fd81f9..543ebf1cfcf4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -149,6 +149,9 @@ struct i915_hotplug {
> /* Whether or not to count short HPD IRQs in HPD storms */
> u8 hpd_short_storm_enabled;
>
> + /* Last state reported by oob_hotplug_event */
> + bool oob_hotplug_state;
> +
> /*
>* if we get a HPD irq from DP and a HPD irq from non-DP
>* the non-DP HPD could block the workqueue on a mode config
> diff --git a/drivers/usb/typec/altmodes/displayport.c
> b/drivers/usb/typec/altmodes/displayport.c
> index c1d8c23baa39..a4596be4d34a 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -59,7 +59,6 @@ struct dp_altmode {
>
Re: [Freedreno] [PATCH 2/2] drm/msm/dp: Implement oob_hotplug_event()
On Mon, Feb 07, 2022 at 08:43:28PM -0800, Bjorn Andersson wrote:
> The Qualcomm DisplayPort driver contains traces of the necessary
> plumbing to hook up USB HPD, in the form of the dp_hpd module and the
> dp_usbpd_cb struct. Use this as basis for implementing the
> oob_hotplug_event() callback, by amending the dp_hpd module with the
> missing logic.
>
> Overall the solution is similar to what's done downstream, but upstream
> all the code to disect the HPD notification lives on the calling side of
> drm_connector_oob_hotplug_event().
>
> drm_connector_oob_hotplug_event() performs the lookup of the
> drm_connector based on fwnode, hence the need to assign the fwnode in
> dp_drm_connector_init().
>
> Signed-off-by: Bjorn Andersson
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 8
> drivers/gpu/drm/msm/dp/dp_display.h | 2 ++
> drivers/gpu/drm/msm/dp/dp_drm.c | 10 ++
> drivers/gpu/drm/msm/dp/dp_hpd.c | 19 +++
> drivers/gpu/drm/msm/dp/dp_hpd.h | 4
> 5 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 7cc4d21f2091..124a2f794382 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -414,6 +414,13 @@ static int dp_display_usbpd_configure_cb(struct device
> *dev)
> return dp_display_process_hpd_high(dp);
> }
>
> +void dp_display_oob_hotplug_event(struct msm_dp *dp_display, bool hpd_state)
> +{
> + struct dp_display_private *dp = container_of(dp_display, struct
> dp_display_private, dp_display);
> +
> + dp->usbpd->oob_event(dp->usbpd, hpd_state);
> +}
> +
> static int dp_display_usbpd_disconnect_cb(struct device *dev)
> {
> struct dp_display_private *dp = dev_get_dp_display_private(dev);
> @@ -1251,6 +1258,7 @@ static int dp_display_probe(struct platform_device
> *pdev)
> dp->pdev = pdev;
> dp->name = "drm_dp";
> dp->dp_display.connector_type = desc->connector_type;
> + dp->dp_display.dev = >dev;
You did not properly reference count this pointer you just saved. What
is to keep that pointer from going away without you knowing about it?
And you already have a pointer to pdev, why save another one here?
>
> rc = dp_init_sub_modules(dp);
> if (rc) {
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
> b/drivers/gpu/drm/msm/dp/dp_display.h
> index e3adcd578a90..1f856b3bca79 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -11,6 +11,7 @@
> #include "disp/msm_disp_snapshot.h"
>
> struct msm_dp {
> + struct device *dev;
> struct drm_device *drm_dev;
> struct device *codec_dev;
So you now have pointers to 3 different devices here? What does 'dev'
point to that the other ones do not? This needs to be documented really
well here.
thanks,
greg k-h
Re: [Freedreno] [PATCH v6 02/35] component: Introduce the aggregate bus_type
On Mon, Jan 31, 2022 at 04:15:09PM +0100, Daniel Vetter wrote: > On Mon, Jan 31, 2022 at 2:48 PM Greg Kroah-Hartman > wrote: > > > > On Thu, Jan 27, 2022 at 12:01:08PM -0800, Stephen Boyd wrote: > > > The component framework only provides 'bind' and 'unbind' callbacks to > > > tell the host driver that it is time to assemble the aggregate driver > > > now that all the components have probed. The component framework doesn't > > > attempt to resolve runtime PM or suspend/resume ordering, and explicitly > > > mentions this in the code. This lack of support leads to some pretty > > > gnarly usages of the 'prepare' and 'complete' power management hooks in > > > drivers that host the aggregate device, and it fully breaks down when > > > faced with ordering shutdown between the various components, the > > > aggregate driver, and the host driver that registers the whole thing. > > > > > > In a concrete example, the MSM display driver at drivers/gpu/drm/msm is > > > using 'prepare' and 'complete' to call the drm helpers > > > drm_mode_config_helper_suspend() and drm_mode_config_helper_resume() > > > respectively, so that it can move the aggregate driver suspend/resume > > > callbacks to be before and after the components that make up the drm > > > device call any suspend/resume hooks they have. This only works as long > > > as the component devices don't do anything in their own 'prepare' and > > > 'complete' callbacks. If they did, then the ordering would be incorrect > > > and we would be doing something in the component drivers before the > > > aggregate driver could do anything. Yuck! > > > > > > Similarly, when trying to add shutdown support to the MSM driver we run > > > across a problem where we're trying to shutdown the drm device via > > > drm_atomic_helper_shutdown(), but some of the devices in the encoder > > > chain have already been shutdown. This time, the component devices > > > aren't the problem (although they could be if they did anything in their > > > shutdown callbacks), but there's a DSI to eDP bridge in the encoder > > > chain that has already been shutdown before the driver hosting the > > > aggregate device runs shutdown. The ordering of driver probe is like > > > this: > > > > > > 1. msm_pdev_probe() (host driver) > > > 2. DSI bridge > > > 3. aggregate bind > > > > > > When it comes to shutdown we have this order: > > > > > > 1. DSI bridge > > > 2. msm_pdev_shutdown() (host driver) > > > > > > and so the bridge is already off, but we want to communicate to it to > > > turn things off on the display during msm_pdev_shutdown(). Double yuck! > > > Unfortunately, this time we can't split shutdown into multiple phases > > > and swap msm_pdev_shutdown() with the DSI bridge. > > > > > > Let's make the component_master_ops into an actual device driver that has > > > probe/remove/shutdown functions. The driver will only be bound to the > > > aggregate device once all component drivers have called component_add() > > > to indicate they're ready to assemble the aggregate driver. This allows > > > us to attach shutdown logic (and in the future runtime PM logic) to the > > > aggregate driver so that it runs the hooks in the correct order. > > > > I know I asked before, but I can not remember the answer. > > > > This really looks like it is turning into the aux bus code. Why can't > > you just use that instead here for this type of thing? You are creating > > another bus and drivers for that bus that are "fake" which is great, but > > that's what the aux bus code was supposed to help out with, so we > > wouldn't have to write more of these. > > > > So, if this really is different, can you document it here so I remember > > next time you resend this patch series? > > aux takes a device and splits it into a lot of sub-devices, each with > their own driver. > > This takes a pile of devices, and turns it into a single logical > device with a single driver. > > So aux is 1:N, component is N:1. > > And yes you asked this already, I typed this up already :-) Ok, thanks. But then why is a bus needed if there's a single driver? I guess a bus for that driver? So one bus, one driver, and one device? I think we need better documentation here... thanks, greg k-h
Re: [Freedreno] [PATCH v6 01/35] component: Replace most references to 'master' with 'aggregate device'
On Thu, Jan 27, 2022 at 12:01:07PM -0800, Stephen Boyd wrote: > Remove most references to 'master' in the code and replace them with > some form of 'aggregate device'. This better reflects the reality of > what this code does, i.e. an aggregate device that represents a > device like a GPU card once some set of devices that make up the > aggregate device probe and register with the component framework. > > Cc: Daniel Vetter > Cc: Greg Kroah-Hartman > Cc: Laurent Pinchart > Cc: "Rafael J. Wysocki" > Cc: Rob Clark > Cc: Russell King > Cc: Saravana Kannan > Signed-off-by: Stephen Boyd > --- > drivers/base/component.c | 242 +++--- > include/linux/component.h | 18 +-- > 2 files changed, 128 insertions(+), 132 deletions(-) This one change is fine as-is and I will queue it up now, thanks for doing it. greg k-h
Re: [Freedreno] [PATCH v6 02/35] component: Introduce the aggregate bus_type
On Thu, Jan 27, 2022 at 12:01:08PM -0800, Stephen Boyd wrote: > The component framework only provides 'bind' and 'unbind' callbacks to > tell the host driver that it is time to assemble the aggregate driver > now that all the components have probed. The component framework doesn't > attempt to resolve runtime PM or suspend/resume ordering, and explicitly > mentions this in the code. This lack of support leads to some pretty > gnarly usages of the 'prepare' and 'complete' power management hooks in > drivers that host the aggregate device, and it fully breaks down when > faced with ordering shutdown between the various components, the > aggregate driver, and the host driver that registers the whole thing. > > In a concrete example, the MSM display driver at drivers/gpu/drm/msm is > using 'prepare' and 'complete' to call the drm helpers > drm_mode_config_helper_suspend() and drm_mode_config_helper_resume() > respectively, so that it can move the aggregate driver suspend/resume > callbacks to be before and after the components that make up the drm > device call any suspend/resume hooks they have. This only works as long > as the component devices don't do anything in their own 'prepare' and > 'complete' callbacks. If they did, then the ordering would be incorrect > and we would be doing something in the component drivers before the > aggregate driver could do anything. Yuck! > > Similarly, when trying to add shutdown support to the MSM driver we run > across a problem where we're trying to shutdown the drm device via > drm_atomic_helper_shutdown(), but some of the devices in the encoder > chain have already been shutdown. This time, the component devices > aren't the problem (although they could be if they did anything in their > shutdown callbacks), but there's a DSI to eDP bridge in the encoder > chain that has already been shutdown before the driver hosting the > aggregate device runs shutdown. The ordering of driver probe is like > this: > > 1. msm_pdev_probe() (host driver) > 2. DSI bridge > 3. aggregate bind > > When it comes to shutdown we have this order: > > 1. DSI bridge > 2. msm_pdev_shutdown() (host driver) > > and so the bridge is already off, but we want to communicate to it to > turn things off on the display during msm_pdev_shutdown(). Double yuck! > Unfortunately, this time we can't split shutdown into multiple phases > and swap msm_pdev_shutdown() with the DSI bridge. > > Let's make the component_master_ops into an actual device driver that has > probe/remove/shutdown functions. The driver will only be bound to the > aggregate device once all component drivers have called component_add() > to indicate they're ready to assemble the aggregate driver. This allows > us to attach shutdown logic (and in the future runtime PM logic) to the > aggregate driver so that it runs the hooks in the correct order. I know I asked before, but I can not remember the answer. This really looks like it is turning into the aux bus code. Why can't you just use that instead here for this type of thing? You are creating another bus and drivers for that bus that are "fake" which is great, but that's what the aux bus code was supposed to help out with, so we wouldn't have to write more of these. So, if this really is different, can you document it here so I remember next time you resend this patch series? thanks, greg k-h
Re: [Freedreno] [PATCH v4 01/34] component: Introduce struct aggregate_device
On Thu, Dec 02, 2021 at 02:26:59PM -0800, Stephen Boyd wrote:
> Replace 'struct master' with 'struct aggregate_device' and then rename
> 'master' to 'adev' everywhere in the code. While we're here, put a
> struct device inside the aggregate device so that we can register it
> with a bus_type in the next patch.
>
> The diff is large but that's because this is mostly a rename, where
> sometimes 'master' is replaced with 'adev' and other times it is
> replaced with 'parent' to indicate that the struct device that was being
> used is actually the parent of the aggregate device and driver.
>
> Cc: Daniel Vetter
> Cc: Greg Kroah-Hartman
> Cc: Laurent Pinchart
> Cc: "Rafael J. Wysocki"
> Cc: Rob Clark
> Cc: Russell King
> Cc: Saravana Kannan
> Signed-off-by: Stephen Boyd
> ---
> drivers/base/component.c | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 2d25a6416587..d25048d04b70 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -9,6 +9,7 @@
> */
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -63,7 +64,10 @@ struct master {
>
> const struct component_master_ops *ops;
> struct device *parent;
> + struct device dev;
Who initializes this new structure?
> struct component_match *match;
> +
> + int id;
> };
>
> struct component {
> @@ -79,6 +83,7 @@ struct component {
> static DEFINE_MUTEX(component_mutex);
> static LIST_HEAD(component_list);
> static LIST_HEAD(masters);
> +static DEFINE_IDA(aggregate_ida);
>
> #ifdef CONFIG_DEBUG_FS
>
> @@ -440,6 +445,7 @@ static void free_master(struct master *master)
> }
> }
>
> + ida_free(_ida, master->id);
> kfree(master);
> }
>
> @@ -460,7 +466,7 @@ int component_master_add_with_match(struct device *parent,
> struct component_match *match)
> {
> struct master *master;
> - int ret;
> + int ret, id;
>
> /* Reallocate the match array for its true size */
> ret = component_match_realloc(match, match->num);
> @@ -471,9 +477,17 @@ int component_master_add_with_match(struct device
> *parent,
> if (!master)
> return -ENOMEM;
>
> + id = ida_alloc(_ida, GFP_KERNEL);
> + if (id < 0) {
> + kfree(master);
> + return id;
> + }
> +
> + master->id = id;
> master->parent = parent;
> master->ops = ops;
> master->match = match;
> + dev_set_name(>dev, "aggregate%d", id);
You set the name yet the device is not "real"?
I don't understand this patch at all, sorry.
greg k-h
Re: [Freedreno] [PATCH v4 01/34] component: Introduce struct aggregate_device
On Thu, Dec 02, 2021 at 02:26:59PM -0800, Stephen Boyd wrote: > Replace 'struct master' with 'struct aggregate_device' and then rename > 'master' to 'adev' everywhere in the code. While we're here, put a > struct device inside the aggregate device so that we can register it > with a bus_type in the next patch. Do not do a "while we are here" type change please. Do the rename/replace first, and then make the other change. > The diff is large but that's because this is mostly a rename, where > sometimes 'master' is replaced with 'adev' and other times it is > replaced with 'parent' to indicate that the struct device that was being > used is actually the parent of the aggregate device and driver. The diff is 15 lines, how is that "large"? thanks, greg k-h
[Freedreno] [PATCH 5.15 261/917] drm/msm: prevent NULL dereference in msm_gpu_crashstate_capture()
From: Tim Gardner
[ Upstream commit b220c154832c5cd0df34cbcbcc19d7135c16e823 ]
Coverity complains of a possible NULL dereference:
CID 120718 (#1 of 1): Dereference null return value (NULL_RETURNS)
23. dereference: Dereferencing a pointer that might be NULL state->bos when
calling msm_gpu_crashstate_get_bo. [show details]
301msm_gpu_crashstate_get_bo(state, submit->bos[i].obj,
302submit->bos[i].iova, submit->bos[i].flags);
Fix this by employing the same state->bos NULL check as is used in the next
for loop.
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Tim Gardner
Reviewed-by: Dmitry Baryshkov
Link: https://lore.kernel.org/r/20210929162554.14295-1-tim.gard...@canonical.com
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/msm_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 8a3a592da3a4d..2c46cd968ac4c 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -296,7 +296,7 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
state->bos = kcalloc(nr,
sizeof(struct msm_gpu_state_bo), GFP_KERNEL);
- for (i = 0; i < submit->nr_bos; i++) {
+ for (i = 0; state->bos && i < submit->nr_bos; i++) {
if (should_dump(submit, i)) {
msm_gpu_crashstate_get_bo(state,
submit->bos[i].obj,
submit->bos[i].iova,
submit->bos[i].flags);
--
2.33.0
[Freedreno] [PATCH 5.14 280/849] drm/msm: prevent NULL dereference in msm_gpu_crashstate_capture()
From: Tim Gardner
[ Upstream commit b220c154832c5cd0df34cbcbcc19d7135c16e823 ]
Coverity complains of a possible NULL dereference:
CID 120718 (#1 of 1): Dereference null return value (NULL_RETURNS)
23. dereference: Dereferencing a pointer that might be NULL state->bos when
calling msm_gpu_crashstate_get_bo. [show details]
301msm_gpu_crashstate_get_bo(state, submit->bos[i].obj,
302submit->bos[i].iova, submit->bos[i].flags);
Fix this by employing the same state->bos NULL check as is used in the next
for loop.
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Tim Gardner
Reviewed-by: Dmitry Baryshkov
Link: https://lore.kernel.org/r/20210929162554.14295-1-tim.gard...@canonical.com
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/msm_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 0ebf7bc6ad097..8236989828ba3 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -404,7 +404,7 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
state->bos = kcalloc(nr,
sizeof(struct msm_gpu_state_bo), GFP_KERNEL);
- for (i = 0; i < submit->nr_bos; i++) {
+ for (i = 0; state->bos && i < submit->nr_bos; i++) {
if (should_dump(submit, i)) {
msm_gpu_crashstate_get_bo(state,
submit->bos[i].obj,
submit->bos[i].iova,
submit->bos[i].flags);
--
2.33.0
[Freedreno] [PATCH 5.10 218/575] drm/msm: prevent NULL dereference in msm_gpu_crashstate_capture()
From: Tim Gardner
[ Upstream commit b220c154832c5cd0df34cbcbcc19d7135c16e823 ]
Coverity complains of a possible NULL dereference:
CID 120718 (#1 of 1): Dereference null return value (NULL_RETURNS)
23. dereference: Dereferencing a pointer that might be NULL state->bos when
calling msm_gpu_crashstate_get_bo. [show details]
301msm_gpu_crashstate_get_bo(state, submit->bos[i].obj,
302submit->bos[i].iova, submit->bos[i].flags);
Fix this by employing the same state->bos NULL check as is used in the next
for loop.
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Tim Gardner
Reviewed-by: Dmitry Baryshkov
Link: https://lore.kernel.org/r/20210929162554.14295-1-tim.gard...@canonical.com
Signed-off-by: Dmitry Baryshkov
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/msm_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 55d16489d0f3f..90c26da109026 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -376,7 +376,7 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
state->bos = kcalloc(nr,
sizeof(struct msm_gpu_state_bo), GFP_KERNEL);
- for (i = 0; i < submit->nr_bos; i++) {
+ for (i = 0; state->bos && i < submit->nr_bos; i++) {
if (should_dump(submit, i)) {
msm_gpu_crashstate_get_bo(state,
submit->bos[i].obj,
submit->bos[i].iova,
submit->bos[i].flags);
--
2.33.0
Re: [Freedreno] [PATCH v2 02/34] component: Introduce the aggregate bus_type
On Wed, Oct 06, 2021 at 12:37:47PM -0700, Stephen Boyd wrote: > The component driver only provides 'bind' and 'unbind' callbacks to tell > the host driver that it is time to assemble the aggregate driver now > that all the components have probed. The component driver model doesn't > attempt to resolve runtime PM or suspend/resume ordering, and explicitly > mentions this in the code. This lack of support leads to some pretty > gnarly usages of the 'prepare' and 'complete' power management hooks in > drivers that host the aggregate device, and it fully breaks down when > faced with ordering shutdown between the various components, the > aggregate driver, and the host driver that registers the whole thing. > > In a concrete example, the MSM display driver at drivers/gpu/drm/msm is > using 'prepare' and 'complete' to call the drm helpers > drm_mode_config_helper_suspend() and drm_mode_config_helper_resume() > respectively, so that it can move the aggregate driver suspend/resume > callbacks to be before and after the components that make up the drm > device call any suspend/resume hooks they have. This only works as long > as the component devices don't do anything in their own 'prepare' and > 'complete' callbacks. If they did, then the ordering would be incorrect > and we would be doing something in the component drivers before the > aggregate driver could do anything. Yuck! > > Similarly, when trying to add shutdown support to the MSM driver we run > across a problem where we're trying to shutdown the drm device via > drm_atomic_helper_shutdown(), but some of the devices in the encoder > chain have already been shutdown. This time, the component devices > aren't the problem (although they could be if they did anything in their > shutdown callbacks), but there's a DSI to eDP bridge in the encoder > chain that has already been shutdown before the driver hosting the > aggregate device runs shutdown. The ordering of driver probe is like > this: > > 1. msm_pdev_probe() (host driver) > 2. DSI bridge > 3. aggregate bind > > When it comes to shutdown we have this order: > > 1. DSI bridge > 2. msm_pdev_shutdown() (host driver) > > and so the bridge is already off, but we want to communicate to it to > turn things off on the display during msm_pdev_shutdown(). Double yuck! > Unfortunately, this time we can't split shutdown into multiple phases > and swap msm_pdev_shutdown() with the DSI bridge. > > Let's make the component driver into an actual device driver that has > probe/remove/shutdown functions. The driver will only be bound to the > aggregate device once all component drivers have called component_add() > to indicate they're ready to assemble the aggregate driver. This allows > us to attach shutdown logic (and in the future runtime PM logic) to the > aggregate driver so that it runs the hooks in the correct order. Why are you creating a new bus type and not using the auxiliary bus instead? You have seen Documentation/driver-api/auxiliary_bus.rst, right? thanks, greg k-h
[Freedreno] [PATCH 5.11 513/775] drm/msm/dp: Add a missing semi-colon
From: Stephen Boyd
[ Upstream commit 182b4a2d251305201b6f9cae29067f7112f05835 ]
A missing semicolon here causes my external display to stop working.
Indeed, missing the semicolon on the return statement leads to
dp_panel_update_tu_timings() not existing because the compiler thinks
it's part of the return statement of a void function, so it must not be
important.
$ ./scripts/bloat-o-meter before.o after.o
add/remove: 1/1 grow/shrink: 0/1 up/down: 7400/-7540 (-140)
Function old new delta
dp_panel_update_tu_timings -7400 +7400
_dp_ctrl_calc_tu.constprop 18024 17900-124
dp_panel_update_tu_timings.constprop7416 - -7416
Total: Before=54440, After=54300, chg -0.26%
Add a semicolon so this function works like it used to.
Cc: Sean Paul
Cc: Kuogee Hsieh
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: Lee Jones
Fixes: cc9014bf63a4 ("drm/msm/dp/dp_ctrl: Move 'tu' from the stack to the heap")
Signed-off-by: Stephen Boyd
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index e3462f5d96d75..6cbe10af0a7af 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -631,7 +631,7 @@ static void _dp_ctrl_calc_tu(struct dp_tu_calc_input *in,
tu = kzalloc(sizeof(*tu), GFP_KERNEL);
if (!tu)
- return
+ return;
dp_panel_update_tu_timings(in, tu);
--
2.27.0
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 5.4 000/132] 5.4.66-rc1 review
On Tue, Sep 15, 2020 at 08:14:34PM +0530, Naresh Kamboju wrote:
> On Tue, 15 Sep 2020 at 19:50, Greg Kroah-Hartman
> wrote:
> >
> > This is the start of the stable review cycle for the 5.4.66 release.
> > There are 132 patches in this series, all will be posted as a response
> > to this one. If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Thu, 17 Sep 2020 14:06:12 +.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> >
> > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.4.66-rc1.gz
> > or in the git tree and branch at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > linux-5.4.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
> >
>
> arm and arm64 build breaks on stable rc 5.4.
>
> make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=arm
> CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc CC="sccache
> arm-linux-gnueabihf-gcc" O=build zImage
> #
> ../kernel/kprobes.c: In function ‘kill_kprobe’:
> ../kernel/kprobes.c:1081:33: warning: statement with no effect
> [-Wunused-value]
> 1081 | #define disarm_kprobe_ftrace(p) (-ENODEV)
> | ^
> ../kernel/kprobes.c:2113:3: note: in expansion of macro ‘disarm_kprobe_ftrace’
> 2113 | disarm_kprobe_ftrace(p);
> | ^~~~
> #
> # make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=arm
> CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc CC="sccache
> arm-linux-gnueabihf-gcc" O=build modules
> #
> ../drivers/gpu/drm/msm/adreno/a5xx_preempt.c: In function ‘preempt_init_ring’:
> ../drivers/gpu/drm/msm/adreno/a5xx_preempt.c:235:21: error:
> ‘MSM_BO_MAP_PRIV’ undeclared (first use in this function)
> 235 | MSM_BO_UNCACHED | MSM_BO_MAP_PRIV, gpu->aspace, , );
> | ^~~
> ../drivers/gpu/drm/msm/adreno/a5xx_preempt.c:235:21: note: each
> undeclared identifier is reported only once for each function it
> appears in
> make[5]: *** [../scripts/Makefile.build:266:
> drivers/gpu/drm/msm/adreno/a5xx_preempt.o] Error 1
> ../drivers/gpu/drm/msm/adreno/a6xx_gpu.c: In function ‘a6xx_hw_init’:
> ../drivers/gpu/drm/msm/adreno/a6xx_gpu.c:414:6: error: implicit
> declaration of function ‘adreno_is_a640’; did you mean
> ‘adreno_is_a540’? [-Werror=implicit-function-declaration]
> 414 | if (adreno_is_a640(adreno_gpu) || adreno_is_a650(adreno_gpu)) {
> | ^~
> | adreno_is_a540
> ../drivers/gpu/drm/msm/adreno/a6xx_gpu.c:414:36: error: implicit
> declaration of function ‘adreno_is_a650’; did you mean
> ‘adreno_is_a540’? [-Werror=implicit-function-declaration]
> 414 | if (adreno_is_a640(adreno_gpu) || adreno_is_a650(adreno_gpu)) {
> |^~
> |adreno_is_a540
> ../drivers/gpu/drm/msm/adreno/a6xx_gpu.c:415:18: error:
> ‘REG_A6XX_GBIF_QSB_SIDE0’ undeclared (first use in this function)
> 415 | gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE0, 0x00071620);
> | ^~~
> ../drivers/gpu/drm/msm/adreno/a6xx_gpu.c:415:18: note: each undeclared
> identifier is reported only once for each function it appears in
> ../drivers/gpu/drm/msm/adreno/a6xx_gpu.c:416:18: error:
> ‘REG_A6XX_GBIF_QSB_SIDE1’ undeclared (first use in this function)
> 416 | gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE1, 0x00071620);
> | ^~~
> ../drivers/gpu/drm/msm/adreno/a6xx_gpu.c:417:18: error:
> ‘REG_A6XX_GBIF_QSB_SIDE2’ undeclared (first use in this function)
> 417 | gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE2, 0x00071620);
> | ^~~
> ../drivers/gpu/drm/msm/adreno/a6xx_gpu.c:418:18: error:
> ‘REG_A6XX_GBIF_QSB_SIDE3’ undeclared (first use in this function)
> 418 | gpu_write(gpu, REG_A6XX_GBIF_QSB_SIDE3, 0x00071620);
> | ^~~
> cc1: some warnings being treated as errors
> make[5]: *** [../scripts/Makefile.build:265:
> drivers/gpu/drm/msm/adreno/a6xx_gpu.o] Error 1
> In file included from ../drivers/gpu/drm/msm/msm_gpu.c:7:
> ../drivers/gpu/drm/msm/msm_gpu.c: In function ‘msm_gpu_init’:
> ../drivers/gpu/drm/msm/msm_gpu.h:330:22: error: ‘MSM_BO_MAP_PRIV’
> undeclared (first use in this function)
> 330 | (((gpu)->hw_apriv ? MSM_BO_MAP_PRIV : 0) | (flags))
> | ^~~
&
Re: [Freedreno] stable-rc 4.14: arm64: Internal error: Oops: clk_reparent __clk_set_parent_before on db410c
On Thu, Aug 13, 2020 at 07:30:40PM +0530, Naresh Kamboju wrote:
> On Wed, 12 Aug 2020 at 23:14, Stephen Boyd wrote:
> >
> > Quoting Stephen Boyd (2020-07-23 03:26:02)
> > > Quoting Naresh Kamboju (2020-07-23 03:10:37)
> > > > On Thu, 23 Jul 2020 at 13:36, Stephen Boyd wrote:
> > > > >
> > > > > It sounds like maybe you need this patch?
> > > > >
> > > > > bdcf1dc25324 ("clk: Evict unregistered clks from parent caches")
> > > >
> > > > Cherry-pick did not work on stable-rc 4.14
> > > > this patch might need backporting.
> > > > I am not sure.
> > > >
> > >
> > > Ok. That commit fixes a regression in the 3.x series of the kernel so it
> > > should go back to any LTS kernels. It looks like at least on 4.14 it's a
> > > trivial conflict. Here's a backport to 4.14
> >
> > Did this help?
>
> Thanks for your patch.
> Sorry for the late reply.
> The reported issue is not always reproducible However,
> I have tested this for 100 cycles and did not notice the reported problem.
>
> Tested-by: Naresh Kamboju
Great, now queued up to 4.14.y and 4.19.y.
greg k-h
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 9/9] soc/qcom: Add REVID driver
On Sun, Jul 26, 2020 at 01:40:46PM +0200, Konrad Dybcio wrote: > >Horrible global symbol name. Who calls this? > > Welcome to development on qcom platforms :D Yeah, I have seen :( > >This is the last patch in > >the series, so if there is no user for this, please don't export it. > > Other downstream drivers make use of it.. need to get this up first, sorry :V Then use a proper name for it, with a correct prefix, and export it with EXPORT_SYMBOL_GPL() please. > >Why do you need a .h file in the include directory if only a single .c > >file needs it? Just put that info in the .c file itself. > > Again, other downstream drivers which some people and I intend to > bring to upstream standards use that to access the PMIC model/hw revision. But all of those defines are not needed, just the function name, right? > >But again, who uses this module? If it's only good for a single line in > >the kernel log, that feels like a huge waste to me. > > downstream-kernel-dir$ rg -l qpnp-revid.h | wc -l > 25 > > So yeah, quite a bunch of other qcom-specific drivers. > > I'll try to fix these and send a v2. Hold off on this and wait until there is a real user for it, as we can not take api additions without a user, otherwise we will just come along and delete the code. Submit it as part of a series that actually uses the function, so we can at least see if the function makes sense to be used in that way as well, right now we have no clue. thanks, greg k-h ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 9/9] soc/qcom: Add REVID driver
On Sun, Jul 26, 2020 at 01:12:06PM +0200, Konrad Dybcio wrote:
> From: Xiaozhe Shi
>
> Add the REVID device driver. The REVID driver will print out the PMIC
> revision at probe time.
Why do we need this noise in the kernel log?
> --- /dev/null
> +++ b/drivers/soc/qcom/qpnp-revid.c
> @@ -0,0 +1,288 @@
> +/* Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
You can drop the GPL boilerplate text and add a proper SPDX line at the
top.
Didn't checkpatch ask for that?
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define REVID_REVISION1 0x0
> +#define REVID_REVISION2 0x1
> +#define REVID_REVISION3 0x2
> +#define REVID_REVISION4 0x3
> +#define REVID_TYPE 0x4
> +#define REVID_SUBTYPE0x5
> +#define REVID_STATUS10x8
> +#define REVID_SPARE_00x60
> +#define REVID_TP_REV 0xf1
> +#define REVID_FAB_ID 0xf2
> +
> +#define QPNP_REVID_DEV_NAME "qcom,qpnp-revid"
> +
> +static const char *const pmic_names[] = {
> + [0] = "Unknown PMIC",
> + [PM8941_SUBTYPE] = "PM8941",
> + [PM8841_SUBTYPE] = "PM8841",
> + [PM8019_SUBTYPE] = "PM8019",
> + [PM8226_SUBTYPE] = "PM8226",
> + [PM8110_SUBTYPE] = "PM8110",
> + [PMA8084_SUBTYPE] = "PMA8084",
> + [PMI8962_SUBTYPE] = "PMI8962",
> + [PMD9635_SUBTYPE] = "PMD9635",
> + [PM8994_SUBTYPE] = "PM8994",
> + [PMI8994_SUBTYPE] = "PMI8994",
> + [PM8916_SUBTYPE] = "PM8916",
> + [PM8004_SUBTYPE] = "PM8004",
> + [PM8909_SUBTYPE] = "PM8909",
> + [PM2433_SUBTYPE] = "PM2433",
> + [PMD9655_SUBTYPE] = "PMD9655",
> + [PM8950_SUBTYPE] = "PM8950",
> + [PMI8950_SUBTYPE] = "PMI8950",
> + [PMK8001_SUBTYPE] = "PMK8001",
> + [PMI8996_SUBTYPE] = "PMI8996",
> + [PM8998_SUBTYPE] = "PM8998",
> + [PMI8998_SUBTYPE] = "PMI8998",
> + [PM8005_SUBTYPE] = "PM8005",
> + [PM8937_SUBTYPE] = "PM8937",
> + [PM660L_SUBTYPE] = "PM660L",
> + [PM660_SUBTYPE] = "PM660",
> + [PMI632_SUBTYPE] = "PMI632",
> + [PMI8937_SUBTYPE] = "PMI8937",
> + [PM8150_SUBTYPE] = "PM8150",
> + [PM8150B_SUBTYPE] = "PM8150B",
> + [PM8150L_SUBTYPE] = "PM8150L",
> + [PM6150_SUBTYPE] = "PM6150",
> + [PM8150A_SUBTYPE] = "PM8150A",
> + [PME9205_SUBTYPE] = "PME9205",
> + [PM6125_SUBTYPE] = "PM6125",
> + [PM8008_SUBTYPE] = "PM8008",
> + [SMB1355_SUBTYPE] = "SMB1355",
> + [SMB1390_SUBTYPE] = "SMB1390",
> +};
> +
> +struct revid_chip {
> + struct list_headlink;
> + struct device_node *dev_node;
> + struct pmic_revid_data data;
> +};
> +
> +static LIST_HEAD(revid_chips);
> +static DEFINE_MUTEX(revid_chips_lock);
> +
> +static const struct of_device_id qpnp_revid_match_table[] = {
> + { .compatible = QPNP_REVID_DEV_NAME },
> + {}
> +};
> +
> +static u8 qpnp_read_byte(struct regmap *regmap, u16 addr)
> +{
> + int rc;
> + int val;
> +
> + rc = regmap_read(regmap, addr, );
> + if (rc) {
> + pr_err("read failed rc=%d\n", rc);
Drivers should always use dev_err() and friends, as you have access to a
struct device * always. Please fix up the driver here to use that api
instead, no pr_* should be needed at all.
> + return 0;
> + }
> + return (u8)val;
> +}
> +
> +/**
> + * get_revid_data - Return the revision information of PMIC
> + * @dev_node: Pointer to the revid peripheral of the PMIC for which
> + * revision information is seeked
> + *
> + * CONTEXT: Should be called in non atomic context
> + *
> + * RETURNS: pointer to struct pmic_revid_data filled with the information
> + * about the PMIC revision
> + */
> +struct pmic_revid_data *get_revid_data(struct device_node *dev_node)
> +{
> + struct revid_chip *revid_chip;
> +
> + if (!dev_node)
> + return ERR_PTR(-EINVAL);
> +
> + mutex_lock(_chips_lock);
> + list_for_each_entry(revid_chip, _chips, link) {
> + if (dev_node == revid_chip->dev_node) {
> + mutex_unlock(_chips_lock);
> + return _chip->data;
> + }
> + }
> + mutex_unlock(_chips_lock);
> + return ERR_PTR(-EINVAL);
> +}
> +EXPORT_SYMBOL(get_revid_data);
Horrible global symbol name. Who calls this? This is the last patch in
the series, so if there is no user for this, please don't export it.
> +
> +#define PM8941_PERIPHERAL_SUBTYPE0x01
> +#define PM8226_PERIPHERAL_SUBTYPE0x04
>
[Freedreno] [PATCH 4.19 14/86] drm: msm: Fix return type of dsi_mgr_connector_mode_valid for kCFI
From: John Stultz
[ Upstream commit 7fd2dfc3694922eb7ace4801b7208cf9f62ebc7d ]
I was hitting kCFI crashes when building with clang, and after
some digging finally narrowed it down to the
dsi_mgr_connector_mode_valid() function being implemented as
returning an int, instead of an enum drm_mode_status.
This patch fixes it, and appeases the opaque word of the kCFI
gods (seriously, clang inlining everything makes the kCFI
backtraces only really rough estimates of where things went
wrong).
Thanks as always to Sami for his help narrowing this down.
Cc: Rob Clark
Cc: Sean Paul
Cc: Sami Tolvanen
Cc: Todd Kjos
Cc: Alistair Delva
Cc: Amit Pundir
Cc: Sumit Semwal
Cc: freedreno@lists.freedesktop.org
Cc: clang-built-li...@googlegroups.com
Signed-off-by: John Stultz
Reviewed-by: Nick Desaulniers
Tested-by: Amit Pundir
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 5224010d90e4a..bd66d2aac41f7 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -328,7 +328,7 @@ static int dsi_mgr_connector_get_modes(struct drm_connector
*connector)
return num;
}
-static int dsi_mgr_connector_mode_valid(struct drm_connector *connector,
+static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector
*connector,
struct drm_display_mode *mode)
{
int id = dsi_mgr_connector_get_id(connector);
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.14 080/126] drm: msm: Fix return type of dsi_mgr_connector_mode_valid for kCFI
From: John Stultz
[ Upstream commit 7fd2dfc3694922eb7ace4801b7208cf9f62ebc7d ]
I was hitting kCFI crashes when building with clang, and after
some digging finally narrowed it down to the
dsi_mgr_connector_mode_valid() function being implemented as
returning an int, instead of an enum drm_mode_status.
This patch fixes it, and appeases the opaque word of the kCFI
gods (seriously, clang inlining everything makes the kCFI
backtraces only really rough estimates of where things went
wrong).
Thanks as always to Sami for his help narrowing this down.
Cc: Rob Clark
Cc: Sean Paul
Cc: Sami Tolvanen
Cc: Todd Kjos
Cc: Alistair Delva
Cc: Amit Pundir
Cc: Sumit Semwal
Cc: freedreno@lists.freedesktop.org
Cc: clang-built-li...@googlegroups.com
Signed-off-by: John Stultz
Reviewed-by: Nick Desaulniers
Tested-by: Amit Pundir
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 855248132b2bd..7d46399a39b4d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -400,7 +400,7 @@ static int dsi_mgr_connector_get_modes(struct drm_connector
*connector)
return num;
}
-static int dsi_mgr_connector_mode_valid(struct drm_connector *connector,
+static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector
*connector,
struct drm_display_mode *mode)
{
int id = dsi_mgr_connector_get_id(connector);
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 5.5 023/189] drm: msm: Fix return type of dsi_mgr_connector_mode_valid for kCFI
From: John Stultz
[ Upstream commit 7fd2dfc3694922eb7ace4801b7208cf9f62ebc7d ]
I was hitting kCFI crashes when building with clang, and after
some digging finally narrowed it down to the
dsi_mgr_connector_mode_valid() function being implemented as
returning an int, instead of an enum drm_mode_status.
This patch fixes it, and appeases the opaque word of the kCFI
gods (seriously, clang inlining everything makes the kCFI
backtraces only really rough estimates of where things went
wrong).
Thanks as always to Sami for his help narrowing this down.
Cc: Rob Clark
Cc: Sean Paul
Cc: Sami Tolvanen
Cc: Todd Kjos
Cc: Alistair Delva
Cc: Amit Pundir
Cc: Sumit Semwal
Cc: freedreno@lists.freedesktop.org
Cc: clang-built-li...@googlegroups.com
Signed-off-by: John Stultz
Reviewed-by: Nick Desaulniers
Tested-by: Amit Pundir
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 271aa7bbca925..355a60b4a536f 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -336,7 +336,7 @@ static int dsi_mgr_connector_get_modes(struct drm_connector
*connector)
return num;
}
-static int dsi_mgr_connector_mode_valid(struct drm_connector *connector,
+static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector
*connector,
struct drm_display_mode *mode)
{
int id = dsi_mgr_connector_get_id(connector);
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 5.4 023/168] drm: msm: Fix return type of dsi_mgr_connector_mode_valid for kCFI
From: John Stultz
[ Upstream commit 7fd2dfc3694922eb7ace4801b7208cf9f62ebc7d ]
I was hitting kCFI crashes when building with clang, and after
some digging finally narrowed it down to the
dsi_mgr_connector_mode_valid() function being implemented as
returning an int, instead of an enum drm_mode_status.
This patch fixes it, and appeases the opaque word of the kCFI
gods (seriously, clang inlining everything makes the kCFI
backtraces only really rough estimates of where things went
wrong).
Thanks as always to Sami for his help narrowing this down.
Cc: Rob Clark
Cc: Sean Paul
Cc: Sami Tolvanen
Cc: Todd Kjos
Cc: Alistair Delva
Cc: Amit Pundir
Cc: Sumit Semwal
Cc: freedreno@lists.freedesktop.org
Cc: clang-built-li...@googlegroups.com
Signed-off-by: John Stultz
Reviewed-by: Nick Desaulniers
Tested-by: Amit Pundir
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 271aa7bbca925..355a60b4a536f 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -336,7 +336,7 @@ static int dsi_mgr_connector_get_modes(struct drm_connector
*connector)
return num;
}
-static int dsi_mgr_connector_mode_valid(struct drm_connector *connector,
+static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector
*connector,
struct drm_display_mode *mode)
{
int id = dsi_mgr_connector_get_id(connector);
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.9 55/88] drm: msm: Fix return type of dsi_mgr_connector_mode_valid for kCFI
From: John Stultz
[ Upstream commit 7fd2dfc3694922eb7ace4801b7208cf9f62ebc7d ]
I was hitting kCFI crashes when building with clang, and after
some digging finally narrowed it down to the
dsi_mgr_connector_mode_valid() function being implemented as
returning an int, instead of an enum drm_mode_status.
This patch fixes it, and appeases the opaque word of the kCFI
gods (seriously, clang inlining everything makes the kCFI
backtraces only really rough estimates of where things went
wrong).
Thanks as always to Sami for his help narrowing this down.
Cc: Rob Clark
Cc: Sean Paul
Cc: Sami Tolvanen
Cc: Todd Kjos
Cc: Alistair Delva
Cc: Amit Pundir
Cc: Sumit Semwal
Cc: freedreno@lists.freedesktop.org
Cc: clang-built-li...@googlegroups.com
Signed-off-by: John Stultz
Reviewed-by: Nick Desaulniers
Tested-by: Amit Pundir
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index c8d1f19c9a6d9..10d49d43c17eb 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -306,7 +306,7 @@ static int dsi_mgr_connector_get_modes(struct drm_connector
*connector)
return num;
}
-static int dsi_mgr_connector_mode_valid(struct drm_connector *connector,
+static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector
*connector,
struct drm_display_mode *mode)
{
int id = dsi_mgr_connector_get_id(connector);
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.4 43/72] drm: msm: Fix return type of dsi_mgr_connector_mode_valid for kCFI
From: John Stultz
[ Upstream commit 7fd2dfc3694922eb7ace4801b7208cf9f62ebc7d ]
I was hitting kCFI crashes when building with clang, and after
some digging finally narrowed it down to the
dsi_mgr_connector_mode_valid() function being implemented as
returning an int, instead of an enum drm_mode_status.
This patch fixes it, and appeases the opaque word of the kCFI
gods (seriously, clang inlining everything makes the kCFI
backtraces only really rough estimates of where things went
wrong).
Thanks as always to Sami for his help narrowing this down.
Cc: Rob Clark
Cc: Sean Paul
Cc: Sami Tolvanen
Cc: Todd Kjos
Cc: Alistair Delva
Cc: Amit Pundir
Cc: Sumit Semwal
Cc: freedreno@lists.freedesktop.org
Cc: clang-built-li...@googlegroups.com
Signed-off-by: John Stultz
Reviewed-by: Nick Desaulniers
Tested-by: Amit Pundir
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 0455ff75074ad..439dfb69e2ef8 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -302,7 +302,7 @@ static int dsi_mgr_connector_get_modes(struct drm_connector
*connector)
return num;
}
-static int dsi_mgr_connector_mode_valid(struct drm_connector *connector,
+static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector
*connector,
struct drm_display_mode *mode)
{
int id = dsi_mgr_connector_get_id(connector);
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH] msm: dpu1: no need to check return value of debugfs_create functions
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jeykumar Sankaran
Cc: Jordan Crouse
Cc: Abhinav Kumar
Cc: Bruce Wang
Cc: Sravanthi Kollukuduru
Cc: Fritz Koenig
Cc: Chandan Uddaraju
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 2 --
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 --
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 12 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 6 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 3 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 4
7 files changed, 4 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 9f20f397f77d..33cd4ee38f67 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -506,8 +506,6 @@ int dpu_core_perf_debugfs_init(struct dpu_kms *dpu_kms,
struct dentry *parent)
struct dentry *entry;
entry = debugfs_create_dir("core_perf", parent);
- if (IS_ERR_OR_NULL(entry))
- return -EINVAL;
debugfs_create_u64("max_core_clk_rate", 0600, entry,
>max_core_clk_rate);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index dfdfa766da8f..e92c2c2bb23f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1266,10 +1266,7 @@ static int _dpu_crtc_init_debugfs(struct drm_crtc *crtc)
dpu_crtc->debugfs_root = debugfs_create_dir(dpu_crtc->name,
crtc->dev->primary->debugfs_root);
- if (!dpu_crtc->debugfs_root)
- return -ENOMEM;
- /* don't error check these */
debugfs_create_file("status", 0400,
dpu_crtc->debugfs_root,
dpu_crtc, _status_fops);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82bf16d61a45..ea99643a3d6f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1985,8 +1985,6 @@ static int _dpu_encoder_init_debugfs(struct drm_encoder
*drm_enc)
/* create overall sub-directory for the encoder */
dpu_enc->debugfs_root = debugfs_create_dir(name,
drm_enc->dev->primary->debugfs_root);
- if (!dpu_enc->debugfs_root)
- return -ENOMEM;
/* don't error check these */
debugfs_create_file("status", 0600,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 885bf88afa3e..bcd24509403a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -142,8 +142,6 @@ static void dpu_debugfs_danger_init(struct dpu_kms *dpu_kms,
struct dentry *parent)
{
struct dentry *entry = debugfs_create_dir("danger", parent);
- if (IS_ERR_OR_NULL(entry))
- return;
debugfs_create_file("danger_status", 0600, entry,
dpu_kms, _debugfs_danger_stats_fops);
@@ -218,17 +216,16 @@ void dpu_debugfs_setup_regset32(struct
dpu_debugfs_regset32 *regset,
}
}
-void *dpu_debugfs_create_regset32(const char *name, umode_t mode,
+void dpu_debugfs_create_regset32(const char *name, umode_t mode,
void *parent, struct dpu_debugfs_regset32 *regset)
{
if (!name || !regset || !regset->dpu_kms || !regset->blk_len)
- return NULL;
+ return;
/* make sure offset is a multiple of 4 */
regset->offset = round_down(regset->offset, 4);
- return debugfs_create_file(name, mode, parent,
- regset, _fops_regset32);
+ debugfs_create_file(name, mode, parent, regset, _fops_regset32);
}
static int _dpu_debugfs_init(struct dpu_kms *dpu_kms)
@@ -240,10 +237,7 @@ static int _dpu_debugfs_init(struct dpu_kms *dpu_kms)
return -EINVAL;
entry = debugfs_create_dir("debug",
dpu_kms->dev->primary->debugfs_root);
- if (IS_ERR_OR_NULL(entry))
- return -ENODEV;
- /* allow root to be NULL */
debugfs_create_x32(DPU_DEBUGFS_HWMASKNAME, 0600, entry, p);
dpu_debugfs_danger_init(dpu_kms, entry);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
i
[Freedreno] [PATCH] msm: no need to check return value of debugfs_create functions
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/msm/msm_perf.c | 15 ++-
drivers/gpu/drm/msm/msm_rd.c | 15 ++-
2 files changed, 4 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_perf.c b/drivers/gpu/drm/msm/msm_perf.c
index 5ab21bd2decb..95948cfe7a12 100644
--- a/drivers/gpu/drm/msm/msm_perf.c
+++ b/drivers/gpu/drm/msm/msm_perf.c
@@ -205,7 +205,6 @@ int msm_perf_debugfs_init(struct drm_minor *minor)
{
struct msm_drm_private *priv = minor->dev->dev_private;
struct msm_perf_state *perf;
- struct dentry *ent;
/* only create on first minor: */
if (priv->perf)
@@ -220,19 +219,9 @@ int msm_perf_debugfs_init(struct drm_minor *minor)
mutex_init(>read_lock);
priv->perf = perf;
- ent = debugfs_create_file("perf", S_IFREG | S_IRUGO,
- minor->debugfs_root, perf, _debugfs_fops);
- if (!ent) {
- DRM_ERROR("Cannot create /sys/kernel/debug/dri/%pd/perf\n",
- minor->debugfs_root);
- goto fail;
- }
-
+ debugfs_create_file("perf", S_IFREG | S_IRUGO, minor->debugfs_root,
+ perf, _debugfs_fops);
return 0;
-
-fail:
- msm_perf_debugfs_cleanup(priv);
- return -1;
}
void msm_perf_debugfs_cleanup(struct msm_drm_private *priv)
diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c
index d21172933d92..46f7eb6a7eaa 100644
--- a/drivers/gpu/drm/msm/msm_rd.c
+++ b/drivers/gpu/drm/msm/msm_rd.c
@@ -244,7 +244,6 @@ static void rd_cleanup(struct msm_rd_state *rd)
static struct msm_rd_state *rd_init(struct drm_minor *minor, const char *name)
{
struct msm_rd_state *rd;
- struct dentry *ent;
int ret = 0;
rd = kzalloc(sizeof(*rd), GFP_KERNEL);
@@ -258,20 +257,10 @@ static struct msm_rd_state *rd_init(struct drm_minor
*minor, const char *name)
init_waitqueue_head(>fifo_event);
- ent = debugfs_create_file(name, S_IFREG | S_IRUGO,
- minor->debugfs_root, rd, _debugfs_fops);
- if (!ent) {
- DRM_ERROR("Cannot create /sys/kernel/debug/dri/%pd/%s\n",
- minor->debugfs_root, name);
- ret = -ENOMEM;
- goto fail;
- }
+ debugfs_create_file(name, S_IFREG | S_IRUGO, minor->debugfs_root, rd,
+ _debugfs_fops);
return rd;
-
-fail:
- rd_cleanup(rd);
- return ERR_PTR(ret);
}
int msm_rd_debugfs_init(struct drm_minor *minor)
--
2.22.0
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH] msm: adreno: no need to check return value of debugfs_create functions
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jordan Crouse
Cc: Mamta Shukla
Cc: Thomas Zimmermann
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
index d9af3aff690f..cb8dfc970ec3 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
@@ -158,7 +158,6 @@ DEFINE_SIMPLE_ATTRIBUTE(reset_fops, NULL, reset_set,
"%llx\n");
int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor)
{
struct drm_device *dev;
- struct dentry *ent;
int ret;
if (!minor)
@@ -175,11 +174,8 @@ int a5xx_debugfs_init(struct msm_gpu *gpu, struct
drm_minor *minor)
return ret;
}
- ent = debugfs_create_file("reset", S_IWUGO,
- minor->debugfs_root,
- dev, _fops);
- if (!ent)
- return -ENOMEM;
+ debugfs_create_file("reset", S_IWUGO, minor->debugfs_root, dev,
+ _fops);
return 0;
}
--
2.22.0
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.14 103/193] drm/msm: a5xx: fix possible object reference leak
[ Upstream commit 6cd5235c3135ea84b32469ea51b2aae384eda8af ]
The call to of_get_child_by_name returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
Signed-off-by: Wen Yang
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jordan Crouse
Cc: Mamta Shukla
Cc: Thomas Zimmermann
Cc: Sharat Masetty
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org (open list)
Reviewed-by: Jordan Crouse
Signed-off-by: Rob Clark
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 17c59d839e6fa..f1aaa76cc2e4e 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -29,7 +29,7 @@ static void a5xx_dump(struct msm_gpu *gpu);
static int zap_shader_load_mdt(struct device *dev, const char *fwname)
{
const struct firmware *fw;
- struct device_node *np;
+ struct device_node *np, *mem_np;
struct resource r;
phys_addr_t mem_phys;
ssize_t mem_size;
@@ -43,11 +43,13 @@ static int zap_shader_load_mdt(struct device *dev, const
char *fwname)
if (!np)
return -ENODEV;
- np = of_parse_phandle(np, "memory-region", 0);
- if (!np)
+ mem_np = of_parse_phandle(np, "memory-region", 0);
+ of_node_put(np);
+ if (!mem_np)
return -EINVAL;
- ret = of_address_to_resource(np, 0, );
+ ret = of_address_to_resource(mem_np, 0, );
+ of_node_put(mem_np);
if (ret)
return ret;
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.19 140/276] drm/msm: a5xx: fix possible object reference leak
[ Upstream commit 6cd5235c3135ea84b32469ea51b2aae384eda8af ]
The call to of_get_child_by_name returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
Signed-off-by: Wen Yang
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jordan Crouse
Cc: Mamta Shukla
Cc: Thomas Zimmermann
Cc: Sharat Masetty
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org (open list)
Reviewed-by: Jordan Crouse
Signed-off-by: Rob Clark
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index ab1d9308c3114..ba6f3c14495c0 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -35,7 +35,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
char *fwname)
{
struct device *dev = >pdev->dev;
const struct firmware *fw;
- struct device_node *np;
+ struct device_node *np, *mem_np;
struct resource r;
phys_addr_t mem_phys;
ssize_t mem_size;
@@ -49,11 +49,13 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
char *fwname)
if (!np)
return -ENODEV;
- np = of_parse_phandle(np, "memory-region", 0);
- if (!np)
+ mem_np = of_parse_phandle(np, "memory-region", 0);
+ of_node_put(np);
+ if (!mem_np)
return -EINVAL;
- ret = of_address_to_resource(np, 0, );
+ ret = of_address_to_resource(mem_np, 0, );
+ of_node_put(mem_np);
if (ret)
return ret;
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 5.0 158/346] drm/msm: a5xx: fix possible object reference leak
[ Upstream commit 6cd5235c3135ea84b32469ea51b2aae384eda8af ]
The call to of_get_child_by_name returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
Signed-off-by: Wen Yang
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jordan Crouse
Cc: Mamta Shukla
Cc: Thomas Zimmermann
Cc: Sharat Masetty
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org (open list)
Reviewed-by: Jordan Crouse
Signed-off-by: Rob Clark
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index d5f5e56422f57..270da14cba673 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -34,7 +34,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
char *fwname)
{
struct device *dev = >pdev->dev;
const struct firmware *fw;
- struct device_node *np;
+ struct device_node *np, *mem_np;
struct resource r;
phys_addr_t mem_phys;
ssize_t mem_size;
@@ -48,11 +48,13 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
char *fwname)
if (!np)
return -ENODEV;
- np = of_parse_phandle(np, "memory-region", 0);
- if (!np)
+ mem_np = of_parse_phandle(np, "memory-region", 0);
+ of_node_put(np);
+ if (!mem_np)
return -EINVAL;
- ret = of_address_to_resource(np, 0, );
+ ret = of_address_to_resource(mem_np, 0, );
+ of_node_put(mem_np);
if (ret)
return ret;
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 5.1 173/405] drm/msm: a5xx: fix possible object reference leak
[ Upstream commit 6cd5235c3135ea84b32469ea51b2aae384eda8af ]
The call to of_get_child_by_name returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 47, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 51, but without a
corresponding object release within this function.
Signed-off-by: Wen Yang
Cc: Rob Clark
Cc: Sean Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jordan Crouse
Cc: Mamta Shukla
Cc: Thomas Zimmermann
Cc: Sharat Masetty
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org (open list)
Reviewed-by: Jordan Crouse
Signed-off-by: Rob Clark
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index d5f5e56422f57..270da14cba673 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -34,7 +34,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
char *fwname)
{
struct device *dev = >pdev->dev;
const struct firmware *fw;
- struct device_node *np;
+ struct device_node *np, *mem_np;
struct resource r;
phys_addr_t mem_phys;
ssize_t mem_size;
@@ -48,11 +48,13 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
char *fwname)
if (!np)
return -ENODEV;
- np = of_parse_phandle(np, "memory-region", 0);
- if (!np)
+ mem_np = of_parse_phandle(np, "memory-region", 0);
+ of_node_put(np);
+ if (!mem_np)
return -EINVAL;
- ret = of_address_to_resource(np, 0, );
+ ret = of_address_to_resource(mem_np, 0, );
+ of_node_put(mem_np);
if (ret)
return ret;
--
2.20.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.19 36/67] drm/msm: Fix error return checking
4.19-stable review patch. If anyone has any objections, please let me know. -- [ Upstream commit 098336deb946f37a70afc0979af388b615c378bf ] The error checks on ret for a negative error return always fails because the return value of iommu_map_sg() is unsigned and can never be negative. Detected with Coccinelle: drivers/gpu/drm/msm/msm_iommu.c:69:9-12: WARNING: Unsigned expression compared with zero: ret < 0 Signed-off-by: Wen Yang CC: Rob Clark CC: David Airlie CC: Julia Lawall CC: linux-arm-...@vger.kernel.org CC: dri-de...@lists.freedesktop.org CC: freedreno@lists.freedesktop.org CC: linux-ker...@vger.kernel.org Signed-off-by: Sean Paul Signed-off-by: Sasha Levin --- drivers/gpu/drm/msm/msm_iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index b23d33622f37..2a90aa4caec0 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -66,7 +66,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova, // pm_runtime_get_sync(mmu->dev); ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot); // pm_runtime_put_sync(mmu->dev); - WARN_ON(ret < 0); + WARN_ON(!ret); return (ret == len) ? 0 : -EINVAL; } -- 2.19.1 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.14 51/72] drm/msm: Fix error return checking
4.14-stable review patch. If anyone has any objections, please let me know. -- [ Upstream commit 098336deb946f37a70afc0979af388b615c378bf ] The error checks on ret for a negative error return always fails because the return value of iommu_map_sg() is unsigned and can never be negative. Detected with Coccinelle: drivers/gpu/drm/msm/msm_iommu.c:69:9-12: WARNING: Unsigned expression compared with zero: ret < 0 Signed-off-by: Wen Yang CC: Rob Clark CC: David Airlie CC: Julia Lawall CC: linux-arm-...@vger.kernel.org CC: dri-de...@lists.freedesktop.org CC: freedreno@lists.freedesktop.org CC: linux-ker...@vger.kernel.org Signed-off-by: Sean Paul Signed-off-by: Sasha Levin --- drivers/gpu/drm/msm/msm_iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index b23d33622f37..2a90aa4caec0 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -66,7 +66,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova, // pm_runtime_get_sync(mmu->dev); ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot); // pm_runtime_put_sync(mmu->dev); - WARN_ON(ret < 0); + WARN_ON(!ret); return (ret == len) ? 0 : -EINVAL; } -- 2.19.1 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 0/7] v4.19-stable randconfig fixes
On Mon, Dec 17, 2018 at 07:20:28PM -0500, Sasha Levin wrote:
> On Fri, Dec 14, 2018 at 11:10:05PM +0100, Arnd Bergmann wrote:
> > Hi Greg,
> >
> > I did some randconfig testing on linux-4.19 arm/arm64/x86. So far I needed
> > 27 patches, most of which are also still needed in mainline Linux. I
> > had submitted some before, and others were not submitted previously
> > for some reason. I'll try to get those fixed in mainline and then
> > make sure we get them into 4.19 as well.
> >
> > This series for now contains four patches that did make it into mainline:
> >
> > 2e6ae11dd0d1 ("slimbus: ngd: mark PM functions as __maybe_unused")
> > 33f49571d750 ("staging: olpc_dcon: add a missing dependency")
> > 0eeec01488da ("scsi: raid_attrs: fix unused variable warning")
> > 11d4afd4ff66 ("sched/pelt: Fix warning and clean up IRQ PELT config")
> >
> > Feel free to either cherry-pick those from mainline or apply the
> > patch from this series, whichever works best for you.
> >
> > The other three patches are for warnings in code that got removed in
> > mainline kernels:
> >
> > 3e9efc3299dd ("i2c: aspeed: Handle master/slave combined irq events
> > properly")
> > 972910948fb6 ("ARM: dts: qcom: Remove Arrow SD600 eval board")
> > effec874792f ("drm/msm/dpu: Remove dpu_dbg")
> >
> > My feeling was that it's safer to just address the warning by fixing
> > the code correctly in each of these cases, but if you disagree,
> > applying the mainline change should work equally well, so decide
> > for yourself.
>
> Thanks Arnd, I took the series as is.
>
> We really need to discuss how -stable deals with removed code upstream.
> For some cases, we should probably follow suit and remove it from
> -stable as well (I'm mostly thinking dodgy code with potential security
> issues).
It would be nice to do that at times (like lustre and ipx), but it's
good to keep that code around as maybe someone is using it? I don't
know, it's a tough call...
thanks,
greg k-h
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.14 038/189] drm/msm: dont deref error pointer in the msm_fbdev_create error path
4.14-stable review patch. If anyone has any objections, please let me know.
--
From: Emil Velikov
[ Upstream commit 789d4c300e10eb2096ee83c3497118e67ccc951e ]
Currently the error pointer returned by msm_alloc_stolen_fb gets passed
to drm_framebuffer_remove. The latter handles only NULL pointers, thus
a nasty crash will occur.
Drop the unnecessary fail label and the associated checks - both err and
fb will be set at this stage.
Cc: Rob Clark
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Emil Velikov
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/msm/msm_fbdev.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -92,8 +92,7 @@ static int msm_fbdev_create(struct drm_f
if (IS_ERR(fb)) {
dev_err(dev->dev, "failed to allocate fb\n");
- ret = PTR_ERR(fb);
- goto fail;
+ return PTR_ERR(fb);
}
bo = msm_framebuffer_bo(fb, 0);
@@ -151,13 +150,7 @@ static int msm_fbdev_create(struct drm_f
fail_unlock:
mutex_unlock(>struct_mutex);
-fail:
-
- if (ret) {
- if (fb)
- drm_framebuffer_remove(fb);
- }
-
+ drm_framebuffer_remove(fb);
return ret;
}
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.16 059/279] drm/msm: dont deref error pointer in the msm_fbdev_create error path
4.16-stable review patch. If anyone has any objections, please let me know.
--
From: Emil Velikov
[ Upstream commit 789d4c300e10eb2096ee83c3497118e67ccc951e ]
Currently the error pointer returned by msm_alloc_stolen_fb gets passed
to drm_framebuffer_remove. The latter handles only NULL pointers, thus
a nasty crash will occur.
Drop the unnecessary fail label and the associated checks - both err and
fb will be set at this stage.
Cc: Rob Clark
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Emil Velikov
Signed-off-by: Rob Clark
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/msm/msm_fbdev.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -92,8 +92,7 @@ static int msm_fbdev_create(struct drm_f
if (IS_ERR(fb)) {
dev_err(dev->dev, "failed to allocate fb\n");
- ret = PTR_ERR(fb);
- goto fail;
+ return PTR_ERR(fb);
}
bo = msm_framebuffer_bo(fb, 0);
@@ -151,13 +150,7 @@ static int msm_fbdev_create(struct drm_f
fail_unlock:
mutex_unlock(>struct_mutex);
-fail:
-
- if (ret) {
- if (fb)
- drm_framebuffer_remove(fb);
- }
-
+ drm_framebuffer_remove(fb);
return ret;
}
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.4 50/90] drm/msm: Expose our reservation object when exporting a dmabuf.
4.4-stable review patch. If anyone has any objections, please let me know.
--
From: Eric Anholt <e...@anholt.net>
commit 43523eba79bda8f5b4c27f8ffe20ea078d20113a upstream.
Without this, polling on the dma-buf (and presumably other devices
synchronizing against our rendering) would return immediately, even
while the BO was busy.
Signed-off-by: Eric Anholt <e...@anholt.net>
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Cc: Rob Clark <robdcl...@gmail.com>
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reviewed-by: Rob Clark <robdcl...@gmail.com>
Signed-off-by: Rob Clark <robdcl...@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
drivers/gpu/drm/msm/msm_drv.c |1 +
drivers/gpu/drm/msm/msm_drv.h |1 +
drivers/gpu/drm/msm/msm_gem_prime.c |7 +++
3 files changed, 9 insertions(+)
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -986,6 +986,7 @@ static struct drm_driver msm_driver = {
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_export = drm_gem_prime_export,
.gem_prime_import = drm_gem_prime_import,
+ .gem_prime_res_obj = msm_gem_prime_res_obj,
.gem_prime_pin = msm_gem_prime_pin,
.gem_prime_unpin= msm_gem_prime_unpin,
.gem_prime_get_sg_table = msm_gem_prime_get_sg_table,
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -212,6 +212,7 @@ struct sg_table *msm_gem_prime_get_sg_ta
void *msm_gem_prime_vmap(struct drm_gem_object *obj);
void msm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
+struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj);
struct drm_gem_object *msm_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach, struct sg_table *sg);
int msm_gem_prime_pin(struct drm_gem_object *obj);
--- a/drivers/gpu/drm/msm/msm_gem_prime.c
+++ b/drivers/gpu/drm/msm/msm_gem_prime.c
@@ -70,3 +70,10 @@ void msm_gem_prime_unpin(struct drm_gem_
if (!obj->import_attach)
msm_gem_put_pages(obj);
}
+
+struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj)
+{
+ struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+ return msm_obj->resv;
+}
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.9 071/119] drm/msm: Expose our reservation object when exporting a dmabuf.
4.9-stable review patch. If anyone has any objections, please let me know.
--
From: Eric Anholt <e...@anholt.net>
commit 43523eba79bda8f5b4c27f8ffe20ea078d20113a upstream.
Without this, polling on the dma-buf (and presumably other devices
synchronizing against our rendering) would return immediately, even
while the BO was busy.
Signed-off-by: Eric Anholt <e...@anholt.net>
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Cc: Rob Clark <robdcl...@gmail.com>
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reviewed-by: Rob Clark <robdcl...@gmail.com>
Signed-off-by: Rob Clark <robdcl...@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
drivers/gpu/drm/msm/msm_drv.c |1 +
drivers/gpu/drm/msm/msm_drv.h |1 +
drivers/gpu/drm/msm/msm_gem_prime.c |7 +++
3 files changed, 9 insertions(+)
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -801,6 +801,7 @@ static struct drm_driver msm_driver = {
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_export = drm_gem_prime_export,
.gem_prime_import = drm_gem_prime_import,
+ .gem_prime_res_obj = msm_gem_prime_res_obj,
.gem_prime_pin = msm_gem_prime_pin,
.gem_prime_unpin= msm_gem_prime_unpin,
.gem_prime_get_sg_table = msm_gem_prime_get_sg_table,
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -203,6 +203,7 @@ struct sg_table *msm_gem_prime_get_sg_ta
void *msm_gem_prime_vmap(struct drm_gem_object *obj);
void msm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
+struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj);
struct drm_gem_object *msm_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach, struct sg_table *sg);
int msm_gem_prime_pin(struct drm_gem_object *obj);
--- a/drivers/gpu/drm/msm/msm_gem_prime.c
+++ b/drivers/gpu/drm/msm/msm_gem_prime.c
@@ -70,3 +70,10 @@ void msm_gem_prime_unpin(struct drm_gem_
if (!obj->import_attach)
msm_gem_put_pages(obj);
}
+
+struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj)
+{
+ struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+ return msm_obj->resv;
+}
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4.11 092/150] drm/msm: Expose our reservation object when exporting a dmabuf.
4.11-stable review patch. If anyone has any objections, please let me know.
--
From: Eric Anholt <e...@anholt.net>
commit 43523eba79bda8f5b4c27f8ffe20ea078d20113a upstream.
Without this, polling on the dma-buf (and presumably other devices
synchronizing against our rendering) would return immediately, even
while the BO was busy.
Signed-off-by: Eric Anholt <e...@anholt.net>
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Cc: Rob Clark <robdcl...@gmail.com>
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Reviewed-by: Rob Clark <robdcl...@gmail.com>
Signed-off-by: Rob Clark <robdcl...@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
drivers/gpu/drm/msm/msm_drv.c |1 +
drivers/gpu/drm/msm/msm_drv.h |1 +
drivers/gpu/drm/msm/msm_gem_prime.c |7 +++
3 files changed, 9 insertions(+)
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -828,6 +828,7 @@ static struct drm_driver msm_driver = {
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_export = drm_gem_prime_export,
.gem_prime_import = drm_gem_prime_import,
+ .gem_prime_res_obj = msm_gem_prime_res_obj,
.gem_prime_pin = msm_gem_prime_pin,
.gem_prime_unpin= msm_gem_prime_unpin,
.gem_prime_get_sg_table = msm_gem_prime_get_sg_table,
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -223,6 +223,7 @@ struct sg_table *msm_gem_prime_get_sg_ta
void *msm_gem_prime_vmap(struct drm_gem_object *obj);
void msm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
+struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj);
struct drm_gem_object *msm_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach, struct sg_table *sg);
int msm_gem_prime_pin(struct drm_gem_object *obj);
--- a/drivers/gpu/drm/msm/msm_gem_prime.c
+++ b/drivers/gpu/drm/msm/msm_gem_prime.c
@@ -70,3 +70,10 @@ void msm_gem_prime_unpin(struct drm_gem_
if (!obj->import_attach)
msm_gem_put_pages(obj);
}
+
+struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj)
+{
+ struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+ return msm_obj->resv;
+}
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
