Add memory bandwidth management to NVIDIA Tegra DRM driver
v18 cover Add memory bandwidth management to NVIDIA Tegra DRM driver
This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, which is done using interconnect framework. It fixes display corruption that happens due to insufficient memory bandwidth.
Changelog:
v18: - Moved total peak bandwidth from CRTC state to plane state and removed
dummy plane bandwidth state initialization from T186+ plane hub. This was suggested by Thierry Reding to v17.
- I haven't done anything about the cursor's plane bandwidth which doesn't contribute to overlapping bandwidths for a small sized window because it works okay as-is.
v17: - No changes, re-sending for v5.14.
v16: - Implemented suggestions that were given by Michał Mirosław to v15.
- Added r-b from Michał Mirosław to the debug-stats patch.
- Rebased on top of a recent linux-next.
- Removed bandwidth scaling based on width difference of src/dst windows since it's not actual anymore. Apparently the recent memory driver changes fixed problems that I witnessed before.
- Average bandwidth calculation now won't overflow for 4k resolutions.
- Average bandwidth calculation now uses the size of the visible area instead of the src area since debug stats of the memory controller clearly show that downscaled window takes less bandwidth, proportionally to the scaled size.
- Bandwidth calculation now uses "adjusted mode" of the CRTC, which is what used for h/w programming, instead of the mode that was requested by userspace, although the two usually match in practice.
v15: - Corrected tegra_plane_icc_names[] NULL-check that was partially lost
by accident in v14 after unsuccessful rebase.
v14: - Made improvements that were suggested by Michał Mirosław to v13:
- Changed 'unsigned int' to 'bool'.
- Renamed functions which calculate bandwidth state.
- Reworked comment in the code that explains why downscaled plane require higher bandwidth.
- Added round-up to bandwidth calculation.
- Added sanity checks of the plane index and fixed out-of-bounds access which happened on T124 due to the cursor plane index.
v13: - No code changes. Patches missed v5.12, re-sending them for v5.13.
Dmitry Osipenko (2):
drm/tegra: dc: Support memory bandwidth management drm/tegra: dc: Extend debug stats with total number of events
drivers/gpu/drm/tegra/Kconfig | 1 + drivers/gpu/drm/tegra/dc.c | 358 +++++++++++++++++++++++++++++++++- drivers/gpu/drm/tegra/dc.h | 17 ++ drivers/gpu/drm/tegra/drm.c | 14 ++ drivers/gpu/drm/tegra/plane.c | 117 +++++++++++ drivers/gpu/drm/tegra/plane.h | 16 ++ 6 files changed, 521 insertions(+), 2 deletions(-)
Signature · 1 line
2.30.2
PATCH v18 1/2 drm/tegra: dc: Support memory bandwidth management
Display controller (DC) performs isochronous memory transfers, and thus, has a requirement for a minimum memory bandwidth that shall be fulfilled, otherwise framebuffer data can't be fetched fast enough and this results in a DC's data-FIFO underflow that follows by a visual corruption.
The Memory Controller drivers provide facility for memory bandwidth management via interconnect API. Let's wire up the interconnect API support to the DC driver in order to fix the distorted display output on T30 Ouya, T124 TK1 and other Tegra devices.
--- drivers/gpu/drm/tegra/Kconfig | 1 + drivers/gpu/drm/tegra/dc.c | 348 +++++++++++++++++++++++++++++++++- drivers/gpu/drm/tegra/dc.h | 12 ++ drivers/gpu/drm/tegra/drm.c | 14 ++ drivers/gpu/drm/tegra/plane.c | 117 ++++++++++++ drivers/gpu/drm/tegra/plane.h | 16 ++ 6 files changed, 506 insertions(+), 2 deletions(-)
drivers/gpu/drm/tegra/Kconfig +1 -0
@@ -9,6 +9,7 @@ config DRM_TEGRA select DRM_MIPI_DSI select DRM_PANEL select TEGRA_HOST1X + select INTERCONNECT select IOMMU_IOVA select CEC_CORE if CEC_NOTIFIER help
drivers/gpu/drm/tegra/dc.c +346 -2
457 lines not on this page · raw message
drivers/gpu/drm/tegra/dc.h +12 -0
@@ -15,6 +15,8 @@struct tegra_output ; +#define TEGRA_DC_LEGACY_PLANES_NUM 7 +struct tegra_dc_state {struct drm_crtc_state base;
@@ -33,6 +35,12 @@ static inline struct tegra_dc_state *to_dc_state(struct drm_crtc_state *state)return NULL ; } +static inline const struct tegra_dc_state * +to_const_dc_state (const struct drm_crtc_state *state) +{ +return to_dc_state ((struct drm_crtc_state *)state); +} +struct tegra_dc_stats {unsigned long frames;unsigned long vblank;
@@ -66,7 +74,9 @@ struct tegra_dc_soc_info {unsigned int num_overlay_formats;const u64 *modifiers;bool has_win_a_without_filters; +bool has_win_b_vfilter_mem_client;bool has_win_c_without_vert_filter; +bool plane_tiled_memory_bandwidth_x2; };struct tegra_dc {
@@ -152,6 +162,8 @@ int tegra_dc_state_setup_clock(struct tegra_dc *dc,struct drm_crtc_state *crtc_state, struct clk *clk, unsigned long pclk, unsigned int div); +void tegra_crtc_atomic_post_commit (struct drm_crtc *crtc, +struct drm_atomic_state *state);/* from rgb.c */ int tegra_dc_rgb_probe (struct tegra_dc *dc);
drivers/gpu/drm/tegra/drm.c +14 -0
@@ -25,6 +25,7 @@#include <asm/dma-iommu.h> #endif +#include "dc.h" #include "drm.h" #include "gem.h"
@@ -64,6 +65,17 @@ static const struct drm_mode_config_funcs tegra_drm_mode_config_funcs = { .atomic_commit = drm_atomic_helper_commit, }; +static void tegra_atomic_post_commit (struct drm_device *drm, +struct drm_atomic_state *old_state) +{ +struct drm_crtc_state *old_crtc_state__maybe_unused ; +struct drm_crtc *crtc; +unsigned int i; + +for_each_old_crtc_in_state (old_state, crtc, old_crtc_state, i) +tegra_crtc_atomic_post_commit (crtc, old_state); +} +static void tegra_atomic_commit_tail (struct drm_atomic_state *old_state) {struct drm_device *drm = old_state->dev;
@@ -83,6 +95,8 @@ static void tegra_atomic_commit_tail(struct drm_atomic_state *old_state) }else {drm_atomic_helper_commit_tail_rpm (old_state); } + +tegra_atomic_post_commit (drm, old_state); } static const struct drm_mode_config_helper_funcs
drivers/gpu/drm/tegra/plane.c +117 -0
144 lines not on this page · raw message
drivers/gpu/drm/tegra/plane.h +16 -0
@@ -8,6 +8,7 @@#include <drm/drm_plane.h> +struct icc_path ;struct tegra_bo ;struct tegra_dc ;
@@ -16,6 +17,9 @@ struct tegra_plane {struct tegra_dc *dc;unsigned int offset;unsigned int index; + +struct icc_path *icc_mem; +struct icc_path *icc_mem_vfilter; };struct tegra_cursor {
@@ -52,6 +56,11 @@ struct tegra_plane_state {/* used for legacy blending support only */ struct tegra_plane_legacy_blending_state blending[2 ];bool opaque; + +/* bandwidths are in ICC units, i.e. kbytes/sec */ +u32 total_peak_memory_bandwidth; +u32 peak_memory_bandwidth; +u32 avg_memory_bandwidth; }; static inline struct tegra_plane_state *
@@ -63,6 +72,12 @@ to_tegra_plane_state(struct drm_plane_state *state)return NULL ; } +static inline const struct tegra_plane_state * +to_const_tegra_plane_state (const struct drm_plane_state *state) +{ +return to_tegra_plane_state ((struct drm_plane_state *)state); +} +extern const struct drm_plane_funcs tegra_plane_funcs; int tegra_plane_prepare_fb(struct drm_plane *plane,
@@ -77,5 +92,6 @@ int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap);bool tegra_plane_format_is_yuv (unsigned int format,bool *planar);int tegra_plane_setup_legacy_state (struct tegra_plane *tegra,struct tegra_plane_state *state); +int tegra_plane_interconnect_init (struct tegra_plane *plane);#endif /* TEGRA_PLANE_H */
Signature · 1 line
2.30.2
PATCH v18 2/2 drm/tegra: dc: Extend debug stats with total number of events
In reply to Dmitry OsipenkoIt's useful to know the total number of underflow events and currently the debug stats are getting reset each time CRTC is being disabled. Let's account the overall number of events that doesn't get a reset.
--- drivers/gpu/drm/tegra/dc.c | 10 ++++++++++ drivers/gpu/drm/tegra/dc.h | 5 +++++ 2 files changed, 15 insertions(+)
drivers/gpu/drm/tegra/dc.c +10 -0
@@ -1602,6 +1602,11 @@ static int tegra_dc_show_stats(struct seq_file *s, void *data)seq_printf (s,"underflow: %lu\n" , dc->stats.underflow);seq_printf (s,"overflow: %lu\n" , dc->stats.overflow); +seq_printf (s,"frames total: %lu\n" , dc->stats.frames_total); +seq_printf (s,"vblank total: %lu\n" , dc->stats.vblank_total); +seq_printf (s,"underflow total: %lu\n" , dc->stats.underflow_total); +seq_printf (s,"overflow total: %lu\n" , dc->stats.overflow_total); +return 0 ; }
@@ -2368,6 +2373,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)/* dev_dbg(dc->dev, "%s(): frame end\n", __func__); */ + dc->stats.frames_total++; dc->stats.frames++; }
@@ -2376,6 +2382,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)dev_dbg(dc->dev, "%s(): vertical blank\n", __func__); */ drm_crtc_handle_vblank (&dc->base); + dc->stats.vblank_total++; dc->stats.vblank++; }
@@ -2383,6 +2390,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)/* dev_dbg(dc->dev, "%s(): underflow\n", __func__); */ + dc->stats.underflow_total++; dc->stats.underflow++; }
@@ -2390,11 +2398,13 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)/* dev_dbg(dc->dev, "%s(): overflow\n", __func__); */ + dc->stats.overflow_total++; dc->stats.overflow++; }if (status &HEAD_UF_INT ) {dev_dbg_ratelimited (dc->dev,"%s(): head underflow\n" , __func__); + dc->stats.underflow_total++; dc->stats.underflow++; }
drivers/gpu/drm/tegra/dc.h +5 -0
@@ -46,6 +46,11 @@ struct tegra_dc_stats {unsigned long vblank;unsigned long underflow;unsigned long overflow; + +unsigned long frames_total; +unsigned long vblank_total; +unsigned long underflow_total; +unsigned long overflow_total; };struct tegra_windowgroup_soc {
Signature · 1 line
2.30.2
01.06.2021 07:21, Dmitry Osipenko пишет:
Quoted 6 lines
This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, which is done using interconnect framework. It fixes display corruption that happens due to insufficient memory bandwidth.
Changelog:
Quoted 7 lines
v18: - Moved total peak bandwidth from CRTC state to plane state and removed
dummy plane bandwidth state initialization from T186+ plane hub. This was suggested by Thierry Reding to v17.- I haven't done anything about the cursor's plane bandwidth which doesn't contribute to overlapping bandwidths for a small sized window because it works okay as-is.
Thierry, will you take these patches for 5.14?
07.06.2021 01:40, Dmitry Osipenko пишет:
Quoted 11 lines
01.06.2021 07:21, Dmitry Osipenko пишет:
Quoted 10 lines
This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, which is done using interconnect framework. It fixes display corruption that happens due to insufficient memory bandwidth.
Changelog:
v18: - Moved total peak bandwidth from CRTC state to plane state and removed
dummy plane bandwidth state initialization from T186+ plane hub. This was suggested by Thierry Reding to v17.
Quoted 6 lines
Quoted 3 lines
- I haven't done anything about the cursor's plane bandwidth which doesn't contribute to overlapping bandwidths for a small sized window because it works okay as-is.
Thierry, will you take these patches for 5.14?
The display controller does _NOT_WORK_ properly without bandwidth management. Can we get this patch into 5.14? What is the problem?
Quoted 16 lines · Dmitry Osipenko
07.06.2021 01:40, Dmitry Osipenko пишет:
Quoted 15 lines
01.06.2021 07:21, Dmitry Osipenko пишет:
Quoted 13 lines
This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, which is done using interconnect framework. It fixes display corruption that happens due to insufficient memory bandwidth.
Changelog:
v18: - Moved total peak bandwidth from CRTC state to plane state and removed
dummy plane bandwidth state initialization from T186+ plane hub. This was suggested by Thierry Reding to v17.- I haven't done anything about the cursor's plane bandwidth which doesn't contribute to overlapping bandwidths for a small sized window because it works okay as-is.
Quoted 5 lines
Quoted 2 lines
Thierry, will you take these patches for 5.14?
The display controller does _NOT_WORK_ properly without bandwidth management.
That's surprising. So either it has never worked before (which I think I'd know) or something has caused this regression recently. In the latter case we need to identify what that was and revert (or fix) it.
Quoted 1 line
Can we get this patch into 5.14? What is the problem?
There was not enough time to review and test this, so I didn't feel comfortable picking it up so close to the -rc6 cut-off. I plan to pick this up early in the v5.14 release cycle and target v5.15.
Thierry
21.06.2021 14:01, Thierry Reding пишет:
Quoted 20 lines · Dmitry Osipenko
Quoted 19 lines
07.06.2021 01:40, Dmitry Osipenko пишет:
Quoted 17 lines
01.06.2021 07:21, Dmitry Osipenko пишет:
Quoted 6 lines
This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, which is done using interconnect framework. It fixes display corruption that happens due to insufficient memory bandwidth.
Changelog:
Quoted 7 lines
v18: - Moved total peak bandwidth from CRTC state to plane state and removed
dummy plane bandwidth state initialization from T186+ plane hub. This was suggested by Thierry Reding to v17.- I haven't done anything about the cursor's plane bandwidth which doesn't contribute to overlapping bandwidths for a small sized window because it works okay as-is.
Thierry, will you take these patches for 5.14?
Quoted 6 lines
Quoted 2 lines
The display controller does _NOT_WORK_ properly without bandwidth management.
That's surprising. So either it has never worked before (which I think I'd know) or something has caused this regression recently. In the latter case we need to identify what that was and revert (or fix) it.
The problem is caused by the support of dynamic memory frequency scaling which does a good job at keeping memory in a low power state during idle time. So display controller may not get enough bandwidth at the start of scanout if it won't request BW beforehand. This problem existed for many years on T124 and now T20/30 are also affected. The DC of T20 is the least tolerant to memory bandwidth troubles.
This problem is not critical, but it hurts user experience since high resolution modes may not work at all and display output may become distorted, requiring a DC reset.
Quoted 5 lines
Quoted 1 line
Can we get this patch into 5.14? What is the problem?
There was not enough time to review and test this, so I didn't feel comfortable picking it up so close to the -rc6 cut-off. I plan to pick this up early in the v5.14 release cycle and target v5.15.
Thank you for the explanation! It's not uncommon to forget about patches, so the silence worries me. I hoped that both the dynamic freq scaling and display BW support would be merged around the same time, but apparently we got a disconnect here.
21.06.2021 14:43, Dmitry Osipenko пишет:
Quoted 41 lines
21.06.2021 14:01, Thierry Reding пишет:
Quoted 20 lines · Dmitry Osipenko
Quoted 19 lines
07.06.2021 01:40, Dmitry Osipenko пишет:
Quoted 17 lines
01.06.2021 07:21, Dmitry Osipenko пишет:
Quoted 6 lines
This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, which is done using interconnect framework. It fixes display corruption that happens due to insufficient memory bandwidth.
Changelog:
Quoted 7 lines
v18: - Moved total peak bandwidth from CRTC state to plane state and removed
dummy plane bandwidth state initialization from T186+ plane hub. This was suggested by Thierry Reding to v17.- I haven't done anything about the cursor's plane bandwidth which doesn't contribute to overlapping bandwidths for a small sized window because it works okay as-is.
Thierry, will you take these patches for 5.14?
Quoted 6 lines
Quoted 2 lines
The display controller does _NOT_WORK_ properly without bandwidth management.
That's surprising. So either it has never worked before (which I think I'd know) or something has caused this regression recently. In the latter case we need to identify what that was and revert (or fix) it.
The problem is caused by the support of dynamic memory frequency scaling which does a good job at keeping memory in a low power state during idle time. So display controller may not get enough bandwidth at the start of scanout if it won't request BW beforehand. This problem existed for many years on T124 and now T20/30 are also affected. The DC of T20 is the least tolerant to memory bandwidth troubles.
This problem is not critical, but it hurts user experience since high resolution modes may not work at all and display output may become distorted, requiring a DC reset.
Quoted 2 lines
Quoted 1 line
Can we get this patch into 5.14? What is the problem?
Quoted 9 lines
Quoted 3 lines
There was not enough time to review and test this, so I didn't feel comfortable picking it up so close to the -rc6 cut-off. I plan to pick this up early in the v5.14 release cycle and target v5.15.
Thank you for the explanation! It's not uncommon to forget about patches, so the silence worries me. I hoped that both the dynamic freq scaling and display BW support would be merged around the same time, but apparently we got a disconnect here.
Reminder ping :)
Quoted 11 lines · Dmitry Osipenko
01.06.2021 07:21, Dmitry Osipenko пишет:
Quoted 10 lines
This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, which is done using interconnect framework. It fixes display corruption that happens due to insufficient memory bandwidth.
Changelog:
v18: - Moved total peak bandwidth from CRTC state to plane state and removed
dummy plane bandwidth state initialization from T186+ plane hub. This was suggested by Thierry Reding to v17.
Quoted 5 lines
Quoted 3 lines
- I haven't done anything about the cursor's plane bandwidth which doesn't contribute to overlapping bandwidths for a small sized window because it works okay as-is.
Thierry, will you take these patches for 5.14?
As discussed offline, I've picked these up for v5.15 with a small patch squashed in to unbreak the Tegra186 and later support.
Thanks,
Thierry
13.08.2021 13:33, Thierry Reding пишет:
Quoted 12 lines · Dmitry Osipenko
Quoted 11 lines
01.06.2021 07:21, Dmitry Osipenko пишет:
Quoted 10 lines
This series adds memory bandwidth management to the NVIDIA Tegra DRM driver, which is done using interconnect framework. It fixes display corruption that happens due to insufficient memory bandwidth.
Changelog:
v18: - Moved total peak bandwidth from CRTC state to plane state and removed
dummy plane bandwidth state initialization from T186+ plane hub. This was suggested by Thierry Reding to v17.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
- I haven't done anything about the cursor's plane bandwidth which doesn't contribute to overlapping bandwidths for a small sized window because it works okay as-is.
Thierry, will you take these patches for 5.14?
As discussed offline, I've picked these up for v5.15 with a small patch squashed in to unbreak the Tegra186 and later support.
Cool, thanks.