dri-devel January 2022

dri-devel@lists.freedesktop.org

373 participants
842 discussions

[PATCH 5/5] drm/stm: ltdc: add support of ycbcr pixel formats
by Yannick Fertre 07 Feb '22

07 Feb '22

This patch adds the following YCbCr input pixel formats on the latest LTDC hardware version: 1 plane (co-planar) : YUYV, YVYU, UYVY, VYUY 2 planes (semi-planar): NV12, NV21 3 planes (full-planar): YU12=I420=DRM YUV420, YV12=DRM YVU420 Signed-off-by: Yannick Fertre <yannick.fertre(a)foss.st.com> --- drivers/gpu/drm/stm/ltdc.c | 251 +++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/stm/ltdc.h | 1 + 2 files changed, 245 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 4d249bc99894..7fd173390b9f 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -198,6 +198,21 @@ #define LXCFBLNR_CFBLN GENMASK(10, 0) /* Color Frame Buffer Line Number */ +#define LXCR_C1R_YIA BIT(0) /* Ycbcr 422 Interleaved Ability */ +#define LXCR_C1R_YSPA BIT(1) /* Ycbcr 420 Semi-Planar Ability */ +#define LXCR_C1R_YFPA BIT(2) /* Ycbcr 420 Full-Planar Ability */ +#define LXCR_C1R_SCA BIT(31) /* SCaling Ability*/ + +#define LxPCR_YREN BIT(9) /* Y Rescale Enable for the color dynamic range */ +#define LxPCR_OF BIT(8) /* Odd pixel First */ +#define LxPCR_CBF BIT(7) /* CB component First */ +#define LxPCR_YF BIT(6) /* Y component First */ +#define LxPCR_YCM GENMASK(5, 4) /* Ycbcr Conversion Mode */ +#define YCM_I 0x0 /* Interleaved 422 */ +#define YCM_SP 0x1 /* Semi-Planar 420 */ +#define YCM_FP 0x2 /* Full-Planar 420 */ +#define LxPCR_YCEN BIT(3) /* YCbCr-to-RGB Conversion Enable */ + #define LXRCR_IMR BIT(0) /* IMmediate Reload */ #define LXRCR_VBR BIT(1) /* Vertical Blanking Reload */ #define LXRCR_GRMSK BIT(2) /* Global (centralized) Reload MaSKed */ @@ -311,6 +326,23 @@ static const u32 ltdc_drm_fmt_a2[] = { DRM_FORMAT_C8 }; +static const u32 ltdc_drm_fmt_ycbcr_cp[] = { + DRM_FORMAT_YUYV, + DRM_FORMAT_YVYU, + DRM_FORMAT_UYVY, + DRM_FORMAT_VYUY +}; + +static const u32 ltdc_drm_fmt_ycbcr_sp[] = { + DRM_FORMAT_NV12, + DRM_FORMAT_NV21 +}; + +static const u32 ltdc_drm_fmt_ycbcr_fp[] = { + DRM_FORMAT_YUV420, + DRM_FORMAT_YVU420 +}; + /* Layer register offsets */ static const u32 ltdc_layer_regs_a0[] = { 0x80, /* L1 configuration 0 */ @@ -410,6 +442,26 @@ static const struct regmap_config stm32_ltdc_regmap_cfg = { .cache_type = REGCACHE_NONE, }; +static const u32 ltdc_ycbcr2rgb_coeffs[DRM_COLOR_ENCODING_MAX][DRM_COLOR_RANGE_MAX][2] = { + [DRM_COLOR_YCBCR_BT601][DRM_COLOR_YCBCR_LIMITED_RANGE] = { + 0x02040199, /* (b_cb = 516 / r_cr = 409) */ + 0x006400D0 /* (g_cb = 100 / g_cr = 208) */ + }, + [DRM_COLOR_YCBCR_BT601][DRM_COLOR_YCBCR_FULL_RANGE] = { + 0x01C60167, /* (b_cb = 454 / r_cr = 359) */ + 0x005800B7 /* (g_cb = 88 / g_cr = 183) */ + }, + [DRM_COLOR_YCBCR_BT709][DRM_COLOR_YCBCR_LIMITED_RANGE] = { + 0x021D01CB, /* (b_cb = 541 / r_cr = 459) */ + 0x00370089 /* (g_cb = 55 / g_cr = 137) */ + }, + [DRM_COLOR_YCBCR_BT709][DRM_COLOR_YCBCR_FULL_RANGE] = { + 0x01DB0193, /* (b_cb = 475 / r_cr = 403) */ + 0x00300078 /* (g_cb = 48 / g_cr = 120) */ + } + /* BT2020 not supported */ +}; + static inline struct ltdc_device *crtc_to_ltdc(struct drm_crtc *crtc) { return (struct ltdc_device *)crtc->dev->dev_private; @@ -540,6 +592,78 @@ static inline u32 is_xrgb(u32 drm) return ((drm & 0xFF) == 'X' || ((drm >> 8) & 0xFF) == 'X'); } +static inline void ltdc_set_ycbcr_config(struct drm_plane *plane, u32 drm_pix_fmt) +{ + struct ltdc_device *ldev = plane_to_ltdc(plane); + struct drm_plane_state *state = plane->state; + u32 lofs = plane->index * LAY_OFS; + u32 val; + + switch (drm_pix_fmt) { + case DRM_FORMAT_YUYV: + val = (YCM_I << 4) | LxPCR_YF | LxPCR_CBF; + break; + case DRM_FORMAT_YVYU: + val = (YCM_I << 4) | LxPCR_YF; + break; + case DRM_FORMAT_UYVY: + val = (YCM_I << 4) | LxPCR_CBF; + break; + case DRM_FORMAT_VYUY: + val = (YCM_I << 4); + break; + case DRM_FORMAT_NV12: + val = (YCM_SP << 4) | LxPCR_CBF; + break; + case DRM_FORMAT_NV21: + val = (YCM_SP << 4); + break; + case DRM_FORMAT_YUV420: + case DRM_FORMAT_YVU420: + val = (YCM_FP << 4); + break; + default: + /* RGB or not a YCbCr supported format */ + break; + } + + /* Enable limited range */ + if (state->color_range == DRM_COLOR_YCBCR_LIMITED_RANGE) + val |= LxPCR_YREN; + + /* enable ycbcr conversion */ + val |= LxPCR_YCEN; + + regmap_write(ldev->regmap, LTDC_L1PCR + lofs, val); +} + +static inline void ltdc_set_ycbcr_coeffs(struct drm_plane *plane) +{ + struct ltdc_device *ldev = plane_to_ltdc(plane); + struct drm_plane_state *state = plane->state; + enum drm_color_encoding enc = state->color_encoding; + enum drm_color_range ran = state->color_range; + u32 lofs = plane->index * LAY_OFS; + + if (enc != DRM_COLOR_YCBCR_BT601 && enc != DRM_COLOR_YCBCR_BT709) { + DRM_ERROR("color encoding %d not supported, use bt601 by default\n", enc); + /* set by default color encoding to DRM_COLOR_YCBCR_BT601 */ + enc = DRM_COLOR_YCBCR_BT601; + } + + if (ran != DRM_COLOR_YCBCR_LIMITED_RANGE && ran != DRM_COLOR_YCBCR_FULL_RANGE) { + DRM_ERROR("color range %d not supported, use limited range by default\n", ran); + /* set by default color range to DRM_COLOR_YCBCR_LIMITED_RANGE */ + ran = DRM_COLOR_YCBCR_LIMITED_RANGE; + } + + DRM_DEBUG_DRIVER("Color encoding=%d, range=%d\n", enc, ran); + regmap_write(ldev->regmap, LTDC_L1CYR0R + lofs, + ltdc_ycbcr2rgb_coeffs[enc][ran][0]); + regmap_write(ldev->regmap, LTDC_L1CYR1R + lofs, + ltdc_ycbcr2rgb_coeffs[enc][ran][1]); +} + static irqreturn_t ltdc_irq_thread(int irq, void *arg) { struct drm_device *ddev = arg; @@ -1010,7 +1134,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane, u32 y0 = newstate->crtc_y; u32 y1 = newstate->crtc_y + newstate->crtc_h - 1; u32 src_x, src_y, src_w, src_h; - u32 val, pitch_in_bytes, line_length, paddr, ahbp, avbp, bpcr; + u32 val, pitch_in_bytes, line_length, line_number, paddr, ahbp, avbp, bpcr; enum ltdc_pix_fmt pf; if (!newstate->crtc || !fb) { @@ -1086,8 +1210,8 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane, regmap_write_bits(ldev->regmap, LTDC_L1BFCR + lofs, LXBFCR_BF2 | LXBFCR_BF1, val); /* Configures the frame buffer line number */ - val = y1 - y0 + 1; - regmap_write_bits(ldev->regmap, LTDC_L1CFBLNR + lofs, LXCFBLNR_CFBLN, val); + line_number = y1 - y0 + 1; + regmap_write_bits(ldev->regmap, LTDC_L1CFBLNR + lofs, LXCFBLNR_CFBLN, line_number); /* Sets the FB address */ paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 0); @@ -1095,6 +1219,77 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane, DRM_DEBUG_DRIVER("fb: phys 0x%08x", paddr); regmap_write(ldev->regmap, LTDC_L1CFBAR + lofs, paddr); + if (ldev->caps.ycbcr_input) { + if (fb->format->is_yuv) { + switch (fb->format->format) { + case DRM_FORMAT_NV12: + case DRM_FORMAT_NV21: + /* Configure the auxiliary frame buffer address 0 & 1 */ + paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 1); + regmap_write(ldev->regmap, LTDC_L1AFBA0R + lofs, paddr); + regmap_write(ldev->regmap, LTDC_L1AFBA1R + lofs, paddr + 1); + + /* Configure the buffer length */ + val = ((pitch_in_bytes << 16) | line_length); + regmap_write(ldev->regmap, LTDC_L1AFBLR + lofs, val); + + /* Configure the frame buffer line number */ + val = (line_number >> 1); + regmap_write(ldev->regmap, LTDC_L1AFBLNR + lofs, val); + break; + case DRM_FORMAT_YUV420: + /* Configure the auxiliary frame buffer address 0 */ + paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 1); + regmap_write(ldev->regmap, LTDC_L1AFBA0R + lofs, paddr); + + /* Configure the auxiliary frame buffer address 1 */ + paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 2); + regmap_write(ldev->regmap, LTDC_L1AFBA1R + lofs, paddr); + + line_length = ((fb->format->cpp[0] * (x1 - x0 + 1)) >> 1) + + (ldev->caps.bus_width >> 3) - 1; + + /* Configure the buffer length */ + val = (((pitch_in_bytes >> 1) << 16) | line_length); + regmap_write(ldev->regmap, LTDC_L1AFBLR + lofs, val); + + /* Configure the frame buffer line number */ + val = (line_number >> 1); + regmap_write(ldev->regmap, LTDC_L1AFBLNR + lofs, val); + break; + case DRM_FORMAT_YVU420: + /* Configure the auxiliary frame buffer address 0 */ + paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 2); + regmap_write(ldev->regmap, LTDC_L1AFBA0R + lofs, paddr); + + /* Configure the auxiliary frame buffer address 1 */ + paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 1); + regmap_write(ldev->regmap, LTDC_L1AFBA1R + lofs, paddr); + + line_length = ((fb->format->cpp[0] * (x1 - x0 + 1)) >> 1) + + (ldev->caps.bus_width >> 3) - 1; + + /* Configure the buffer length */ + val = (((pitch_in_bytes >> 1) << 16) | line_length); + regmap_write(ldev->regmap, LTDC_L1AFBLR + lofs, val); + + /* Configure the frame buffer line number */ + val = (line_number >> 1); + regmap_write(ldev->regmap, LTDC_L1AFBLNR + lofs, val); + break; + } + + /* Configure YCbC conversion coefficient */ + ltdc_set_ycbcr_coeffs(plane); + + /* Configure YCbCr format and enable/disable conversion */ + ltdc_set_ycbcr_config(plane, fb->format->format); + } else { + /* disable ycbcr conversion */ + regmap_write(ldev->regmap, LTDC_L1PCR + lofs, 0); + } + } + /* Enable layer and CLUT if needed */ val = fb->format->format == DRM_FORMAT_C8 ? LXCR_CLUTEN : 0; val |= LXCR_LEN; @@ -1186,7 +1381,8 @@ static const struct drm_plane_helper_funcs ltdc_plane_helper_funcs = { }; static struct drm_plane *ltdc_plane_create(struct drm_device *ddev, - enum drm_plane_type type) + enum drm_plane_type type, + int index) { unsigned long possible_crtcs = CRTC_MASK; struct ltdc_device *ldev = ddev->dev_private; @@ -1196,9 +1392,16 @@ static struct drm_plane *ltdc_plane_create(struct drm_device *ddev, u32 *formats; u32 drm_fmt; const u64 *modifiers = ltdc_format_modifiers; + u32 lofs = index * LAY_OFS; + u32 val; int ret; - formats = devm_kzalloc(dev, ldev->caps.pix_fmt_nb * sizeof(*formats), GFP_KERNEL); + /* Allocate the biggest size according to supported color formats */ + formats = devm_kzalloc(dev, (ldev->caps.pix_fmt_nb + + ARRAY_SIZE(ltdc_drm_fmt_ycbcr_cp) + + ARRAY_SIZE(ltdc_drm_fmt_ycbcr_sp) + + ARRAY_SIZE(ltdc_drm_fmt_ycbcr_fp)) * + sizeof(*formats), GFP_KERNEL); for (i = 0; i < ldev->caps.pix_fmt_nb; i++) { drm_fmt = ldev->caps.pix_fmt_drm[i]; @@ -1212,6 +1415,26 @@ static struct drm_plane *ltdc_plane_create(struct drm_device *ddev, formats[nb_fmt++] = drm_fmt; } + /* Add YCbCr supported pixel formats */ + if (ldev->caps.ycbcr_input) { + regmap_read(ldev->regmap, LTDC_L1C1R + lofs, &val); + if (val & LXCR_C1R_YIA) { + memcpy(&formats[nb_fmt], ltdc_drm_fmt_ycbcr_cp, + ARRAY_SIZE(ltdc_drm_fmt_ycbcr_cp) * sizeof(*formats)); + nb_fmt += ARRAY_SIZE(ltdc_drm_fmt_ycbcr_cp); + } + if (val & LXCR_C1R_YSPA) { + memcpy(&formats[nb_fmt], ltdc_drm_fmt_ycbcr_sp, + ARRAY_SIZE(ltdc_drm_fmt_ycbcr_sp) * sizeof(*formats)); + nb_fmt += ARRAY_SIZE(ltdc_drm_fmt_ycbcr_sp); + } + if (val & LXCR_C1R_YFPA) { + memcpy(&formats[nb_fmt], ltdc_drm_fmt_ycbcr_fp, + ARRAY_SIZE(ltdc_drm_fmt_ycbcr_fp) * sizeof(*formats)); + nb_fmt += ARRAY_SIZE(ltdc_drm_fmt_ycbcr_fp); + } + } + plane = devm_kzalloc(dev, sizeof(*plane), GFP_KERNEL); if (!plane) return NULL; @@ -1222,6 +1445,17 @@ static struct drm_plane *ltdc_plane_create(struct drm_device *ddev, if (ret < 0) return NULL; + if (ldev->caps.ycbcr_input) { + if (val & (LXCR_C1R_YIA | LXCR_C1R_YSPA | LXCR_C1R_YFPA)) + drm_plane_create_color_properties(plane, + BIT(DRM_COLOR_YCBCR_BT601) | + BIT(DRM_COLOR_YCBCR_BT709), + BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) | + BIT(DRM_COLOR_YCBCR_FULL_RANGE), + DRM_COLOR_YCBCR_BT601, + DRM_COLOR_YCBCR_LIMITED_RANGE); + } + drm_plane_helper_add(plane, &ltdc_plane_helper_funcs); drm_plane_create_alpha_property(plane); @@ -1247,7 +1481,7 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc) unsigned int i; int ret; - primary = ltdc_plane_create(ddev, DRM_PLANE_TYPE_PRIMARY); + primary = ltdc_plane_create(ddev, DRM_PLANE_TYPE_PRIMARY, 0); if (!primary) { DRM_ERROR("Can not create primary plane\n"); return -EINVAL; @@ -1271,7 +1505,7 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc) /* Add planes. Note : the first layer is used by primary plane */ for (i = 1; i < ldev->caps.nb_layers; i++) { - overlay = ltdc_plane_create(ddev, DRM_PLANE_TYPE_OVERLAY); + overlay = ltdc_plane_create(ddev, DRM_PLANE_TYPE_OVERLAY, i); if (!overlay) { ret = -ENOMEM; DRM_ERROR("Can not create overlay plane %d\n", i); @@ -1403,6 +1637,7 @@ static int ltdc_get_caps(struct drm_device *ddev) if (ldev->caps.hw_version == HWVER_10200) ldev->caps.pad_max_freq_hz = 65000000; ldev->caps.nb_irq = 2; + ldev->caps.ycbcr_input = false; ldev->caps.ycbcr_output = false; ldev->caps.plane_reg_shadow = false; break; @@ -1416,6 +1651,7 @@ static int ltdc_get_caps(struct drm_device *ddev) ldev->caps.non_alpha_only_l1 = false; ldev->caps.pad_max_freq_hz = 150000000; ldev->caps.nb_irq = 4; + ldev->caps.ycbcr_input = false; ldev->caps.ycbcr_output = false; ldev->caps.plane_reg_shadow = false; break; @@ -1429,6 +1665,7 @@ static int ltdc_get_caps(struct drm_device *ddev) ldev->caps.non_alpha_only_l1 = false; ldev->caps.pad_max_freq_hz = 90000000; ldev->caps.nb_irq = 2; + ldev->caps.ycbcr_input = true; ldev->caps.ycbcr_output = true; ldev->caps.plane_reg_shadow = true; break; diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h index adc4f9cf7f95..6968d1ca5149 100644 --- a/drivers/gpu/drm/stm/ltdc.h +++ b/drivers/gpu/drm/stm/ltdc.h @@ -24,6 +24,7 @@ struct ltdc_caps { bool non_alpha_only_l1; /* non-native no-alpha formats on layer 1 */ int pad_max_freq_hz; /* max frequency supported by pad */ int nb_irq; /* number of hardware interrupts */ + bool ycbcr_input; /* ycbcr input converter supported */ bool ycbcr_output; /* ycbcr output converter supported */ bool plane_reg_shadow; /* plane shadow registers ability */ }; -- 2.17.1

4 4

[PATCH 0/8] drm/ast: Untangle connector helpers
by Thomas Zimmermann 07 Feb '22

07 Feb '22

The ast driver supports different transmitter chips to support DVI and HDMI connectors. It's al been packed into the same helpers functons and exported as VGA connector. Introduce a separate set of connector helpers for each transmitter chip, and thus connector type. Also provide the correct encoder for each connector. This change mostly affects the connector's get_modes helper, where VGA-, DVI- and HDMI-related code was lumped into the same function. It's now all separate. While at it, also rework and cleanup the initialization of the related data structures. Tested on AST 2100 and AST 2300 hardware. I don't have hardware with special transmitter chips (SIL164, DP501), so I could only test the VGA code. Thomas Zimmermann (8): drm/ast: Fail if connector initialization fails drm/ast: Move connector mode_valid function to CRTC drm/ast: Remove AST_TX_ITE66121 constant drm/ast: Remove unused value dp501_maxclk drm/ast: Rename struct ast_connector to struct ast_vga_connector drm/ast: Initialize encoder and connector for VGA in helper function drm/ast: Move DP501-based connector code into separate helpers drm/ast: Move SIL164-based connector code into separate helpers drivers/gpu/drm/ast/ast_dp501.c | 58 ----- drivers/gpu/drm/ast/ast_drv.h | 37 ++- drivers/gpu/drm/ast/ast_mode.c | 413 +++++++++++++++++++++++--------- 3 files changed, 331 insertions(+), 177 deletions(-) base-commit: fbce7b8d8df5af8d404b6aeaf63779f91bdbeb5d prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 -- 2.34.1

2 18

[PATCH] drm/privacy-screen: Fix sphinx warning
by Hans de Goede 07 Feb '22

07 Feb '22

Fix the following warning from "make htmldocs": drivers/gpu/drm/drm_privacy_screen.c:270: WARNING: Inline emphasis start-string without end-string. Fixes: 8a12b170558a ("drm/privacy-screen: Add notifier support (v2)") Reported-by: Stephen Rothwell <sfr(a)canb.auug.org.au> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> --- drivers/gpu/drm/drm_privacy_screen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_privacy_screen.c b/drivers/gpu/drm/drm_privacy_screen.c index beaf99e9120a..9f35f8f0cf60 100644 --- a/drivers/gpu/drm/drm_privacy_screen.c +++ b/drivers/gpu/drm/drm_privacy_screen.c @@ -269,7 +269,7 @@ EXPORT_SYMBOL(drm_privacy_screen_get_state); * * The notifier is called with no locks held. The new hw_state and sw_state * can be retrieved using the drm_privacy_screen_get_state() function. - * A pointer to the drm_privacy_screen's struct is passed as the void *data + * A pointer to the drm_privacy_screen's struct is passed as the void \*data * argument of the notifier_block's notifier_call. * * The notifier will NOT be called when changes are made through -- 2.33.1

2 3

[PATCH] drm/dp: Remove common Post Cursor2 register handling
by Kees Cook 07 Feb '22

07 Feb '22

The link_status array was not large enough to read the Adjust Request Post Cursor2 register, so remove the common helper function to avoid an OOB read, found with a -Warray-bounds build: drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_get_adjust_request_post_cursor': drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'} [-Werror=array-bounds] 59 | return link_status[r - DP_LANE0_1_STATUS]; | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing 'link_status' 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE], | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Replace the only user of the helper with an open-coded fetch and decode, similar to drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c. Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments") Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: David Airlie <airlied(a)linux.ie> Cc: Daniel Vetter <daniel(a)ffwll.ch> Cc: dri-devel(a)lists.freedesktop.org Signed-off-by: Kees Cook <keescook(a)chromium.org> --- This is the alternative to: https://lore.kernel.org/lkml/20211203084354.3105253-1-keescook@chromium.org/ --- drivers/gpu/drm/drm_dp_helper.c | 10 ---------- drivers/gpu/drm/tegra/dp.c | 11 ++++++++++- include/drm/drm_dp_helper.h | 2 -- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 23f9073bc473..c9528aa62c9c 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -144,16 +144,6 @@ u8 drm_dp_get_adjust_tx_ffe_preset(const u8 link_status[DP_LINK_STATUS_SIZE], } EXPORT_SYMBOL(drm_dp_get_adjust_tx_ffe_preset); -u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE], - unsigned int lane) -{ - unsigned int offset = DP_ADJUST_REQUEST_POST_CURSOR2; - u8 value = dp_link_status(link_status, offset); - - return (value >> (lane << 1)) & 0x3; -} -EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor); - static int __8b10b_clock_recovery_delay_us(const struct drm_dp_aux *aux, u8 rd_interval) { if (rd_interval > 4) diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c index 70dfb7d1dec5..f5535eb04c6b 100644 --- a/drivers/gpu/drm/tegra/dp.c +++ b/drivers/gpu/drm/tegra/dp.c @@ -549,6 +549,15 @@ static void drm_dp_link_get_adjustments(struct drm_dp_link *link, { struct drm_dp_link_train_set *adjust = &link->train.adjust; unsigned int i; + u8 post_cursor; + int err; + + err = drm_dp_dpcd_read(link->aux, DP_ADJUST_REQUEST_POST_CURSOR2, + &post_cursor, sizeof(post_cursor)); + if (err < 0) { + DRM_ERROR("failed to read post_cursor2: %d\n", err); + post_cursor = 0; + } for (i = 0; i < link->lanes; i++) { adjust->voltage_swing[i] = @@ -560,7 +569,7 @@ static void drm_dp_link_get_adjustments(struct drm_dp_link *link, DP_TRAIN_PRE_EMPHASIS_SHIFT; adjust->post_cursor[i] = - drm_dp_get_adjust_request_post_cursor(status, i); + (post_cursor >> (i << 1)) & 0x3; } } diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 472dac376284..fdf3cf6ccc02 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1528,8 +1528,6 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI int lane); u8 drm_dp_get_adjust_tx_ffe_preset(const u8 link_status[DP_LINK_STATUS_SIZE], int lane); -u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE], - unsigned int lane); #define DP_BRANCH_OUI_HEADER_SIZE 0xc #define DP_RECEIVER_CAP_SIZE 0xf -- 2.30.2

3 7

[PATCH v7 6/8] phy: phy-mtk-dp: Add driver for DP phy
by Guillaume Ranquet 07 Feb '22

07 Feb '22

From: Markus Schneider-Pargmann <msp(a)baylibre.com> This is a new driver that supports the integrated DisplayPort phy for mediatek SoCs, especially the mt8195. The phy is integrated into the DisplayPort controller and will be created by the mtk-dp driver. This driver expects a struct regmap to be able to work on the same registers as the DisplayPort controller. It sets the device data to be the struct phy so that the DisplayPort controller can easily work with it. The driver does not have any devicetree bindings because the datasheet does not list the controller and the phy as distinct units. The interaction with the controller can be covered by the configure callback of the phy framework and its displayport parameters. Signed-off-by: Markus Schneider-Pargmann <msp(a)baylibre.com> Signed-off-by: Guillaume Ranquet <granquet(a)baylibre.com> --- MAINTAINERS | 1 + drivers/phy/mediatek/Kconfig | 8 ++ drivers/phy/mediatek/Makefile | 1 + drivers/phy/mediatek/phy-mtk-dp.c | 219 ++++++++++++++++++++++++++++++ 4 files changed, 229 insertions(+) create mode 100644 drivers/phy/mediatek/phy-mtk-dp.c diff --git a/MAINTAINERS b/MAINTAINERS index 8b7a98daf8e05..c44829d8a74df 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6394,6 +6394,7 @@ L: linux-mediatek(a)lists.infradead.org (moderated for non-subscribers) S: Supported F: Documentation/devicetree/bindings/display/mediatek/ F: drivers/gpu/drm/mediatek/ +F: drivers/phy/mediatek/phy-mtk-dp.c F: drivers/phy/mediatek/phy-mtk-hdmi* F: drivers/phy/mediatek/phy-mtk-mipi* diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig index 55f8e6c048ab3..f7ec860590492 100644 --- a/drivers/phy/mediatek/Kconfig +++ b/drivers/phy/mediatek/Kconfig @@ -55,3 +55,11 @@ config PHY_MTK_MIPI_DSI select GENERIC_PHY help Support MIPI DSI for Mediatek SoCs. + +config PHY_MTK_DP + tristate "MediaTek DP-PHY Driver" + depends on ARCH_MEDIATEK || COMPILE_TEST + depends on OF + select GENERIC_PHY + help + Support DisplayPort PHY for Mediatek SoCs. diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile index ace660fbed3a1..4ba1e06504346 100644 --- a/drivers/phy/mediatek/Makefile +++ b/drivers/phy/mediatek/Makefile @@ -3,6 +3,7 @@ # Makefile for the phy drivers. # +obj-$(CONFIG_PHY_MTK_DP) += phy-mtk-dp.o obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c new file mode 100644 index 0000000000000..e0de2a367e788 --- /dev/null +++ b/drivers/phy/mediatek/phy-mtk-dp.c @@ -0,0 +1,219 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2021 BayLibre + * Author: Markus Schneider-Pargmann <msp(a)baylibre.com> + */ + +#include <linux/delay.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +#define PHY_OFFSET 0x1000 + +#define MTK_DP_PHY_DIG_PLL_CTL_1 (PHY_OFFSET + 0x014) +# define TPLL_SSC_EN BIT(3) + +#define MTK_DP_PHY_DIG_BIT_RATE (PHY_OFFSET + 0x03C) +# define BIT_RATE_RBR 0 +# define BIT_RATE_HBR 1 +# define BIT_RATE_HBR2 2 +# define BIT_RATE_HBR3 3 + +#define MTK_DP_PHY_DIG_SW_RST (PHY_OFFSET + 0x038) +# define DP_GLB_SW_RST_PHYD BIT(0) + +#define MTK_DP_LANE0_DRIVING_PARAM_3 (PHY_OFFSET + 0x138) +#define MTK_DP_LANE1_DRIVING_PARAM_3 (PHY_OFFSET + 0x238) +#define MTK_DP_LANE2_DRIVING_PARAM_3 (PHY_OFFSET + 0x338) +#define MTK_DP_LANE3_DRIVING_PARAM_3 (PHY_OFFSET + 0x438) +# define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT 0x10 +# define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT (0x14 << 8) +# define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT (0x18 << 16) +# define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT (0x20 << 24) +# define DRIVING_PARAM_3_DEFAULT (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_4 (PHY_OFFSET + 0x13C) +#define MTK_DP_LANE1_DRIVING_PARAM_4 (PHY_OFFSET + 0x23C) +#define MTK_DP_LANE2_DRIVING_PARAM_4 (PHY_OFFSET + 0x33C) +#define MTK_DP_LANE3_DRIVING_PARAM_4 (PHY_OFFSET + 0x43C) +# define XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT 0x18 +# define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT (0x1e << 8) +# define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT (0x24 << 16) +# define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT (0x20 << 24) +# define DRIVING_PARAM_4_DEFAULT (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_5 (PHY_OFFSET + 0x140) +#define MTK_DP_LANE1_DRIVING_PARAM_5 (PHY_OFFSET + 0x240) +#define MTK_DP_LANE2_DRIVING_PARAM_5 (PHY_OFFSET + 0x340) +#define MTK_DP_LANE3_DRIVING_PARAM_5 (PHY_OFFSET + 0x440) +# define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT 0x28 +# define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT (0x30 << 8) +# define DRIVING_PARAM_5_DEFAULT (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_6 (PHY_OFFSET + 0x144) +#define MTK_DP_LANE1_DRIVING_PARAM_6 (PHY_OFFSET + 0x244) +#define MTK_DP_LANE2_DRIVING_PARAM_6 (PHY_OFFSET + 0x344) +#define MTK_DP_LANE3_DRIVING_PARAM_6 (PHY_OFFSET + 0x444) +# define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT 0x00 +# define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT (0x04 << 8) +# define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT (0x08 << 16) +# define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT (0x10 << 24) +# define DRIVING_PARAM_6_DEFAULT (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_7 (PHY_OFFSET + 0x148) +#define MTK_DP_LANE1_DRIVING_PARAM_7 (PHY_OFFSET + 0x248) +#define MTK_DP_LANE2_DRIVING_PARAM_7 (PHY_OFFSET + 0x348) +#define MTK_DP_LANE3_DRIVING_PARAM_7 (PHY_OFFSET + 0x448) +# define XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT 0x00 +# define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT (0x06 << 8) +# define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT (0x0c << 16) +# define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT (0x00 << 24) +# define DRIVING_PARAM_7_DEFAULT (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_8 (PHY_OFFSET + 0x14C) +#define MTK_DP_LANE1_DRIVING_PARAM_8 (PHY_OFFSET + 0x24C) +#define MTK_DP_LANE2_DRIVING_PARAM_8 (PHY_OFFSET + 0x34C) +#define MTK_DP_LANE3_DRIVING_PARAM_8 (PHY_OFFSET + 0x44C) +# define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT 0x08 +# define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT (0x00 << 8) +# define DRIVING_PARAM_8_DEFAULT (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT) + +struct mtk_dp_phy { + struct regmap *regs; +}; + +static int mtk_dp_phy_init(struct phy *phy) +{ + struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy); + u32 driving_params[] = { + DRIVING_PARAM_3_DEFAULT, + DRIVING_PARAM_4_DEFAULT, + DRIVING_PARAM_5_DEFAULT, + DRIVING_PARAM_6_DEFAULT, + DRIVING_PARAM_7_DEFAULT, + DRIVING_PARAM_8_DEFAULT + }; + + regmap_bulk_write(dp_phy->regs, MTK_DP_LANE0_DRIVING_PARAM_3, + driving_params, ARRAY_SIZE(driving_params)); + regmap_bulk_write(dp_phy->regs, MTK_DP_LANE1_DRIVING_PARAM_3, + driving_params, ARRAY_SIZE(driving_params)); + regmap_bulk_write(dp_phy->regs, MTK_DP_LANE2_DRIVING_PARAM_3, + driving_params, ARRAY_SIZE(driving_params)); + regmap_bulk_write(dp_phy->regs, MTK_DP_LANE3_DRIVING_PARAM_3, + driving_params, ARRAY_SIZE(driving_params)); + + return 0; +} + +static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts) +{ + struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy); + u32 val; + + if (opts->dp.set_rate) { + switch (opts->dp.link_rate) { + default: + dev_err(&phy->dev, + "Implementation error, unknown linkrate %x\n", + opts->dp.link_rate); + return -EINVAL; + case 1620: + val = BIT_RATE_RBR; + break; + case 2700: + val = BIT_RATE_HBR; + break; + case 5400: + val = BIT_RATE_HBR2; + break; + case 8100: + val = BIT_RATE_HBR3; + break; + } + regmap_write(dp_phy->regs, MTK_DP_PHY_DIG_BIT_RATE, val); + } + + regmap_update_bits(dp_phy->regs, MTK_DP_PHY_DIG_PLL_CTL_1, + TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0); + + return 0; +} + +static int mtk_dp_phy_reset(struct phy *phy) +{ + struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy); + + regmap_update_bits(dp_phy->regs, MTK_DP_PHY_DIG_SW_RST, + DP_GLB_SW_RST_PHYD, 0); + usleep_range(50, 200); + regmap_update_bits(dp_phy->regs, MTK_DP_PHY_DIG_SW_RST, + DP_GLB_SW_RST_PHYD, 1); + + return 0; +} + +static const struct phy_ops mtk_dp_phy_dev_ops = { + .init = mtk_dp_phy_init, + .configure = mtk_dp_phy_configure, + .reset = mtk_dp_phy_reset, + .owner = THIS_MODULE, +}; + +static int mtk_dp_phy_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct mtk_dp_phy *dp_phy; + struct phy *phy; + + dp_phy = devm_kzalloc(dev, sizeof(*dp_phy), GFP_KERNEL); + if (!dp_phy) + return -ENOMEM; + + dp_phy->regs = *(struct regmap **)dev->platform_data; + if (!dp_phy->regs) { + dev_err(dev, "No data passed, requires struct regmap**\n"); + return -EINVAL; + } + + phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops); + if (IS_ERR(phy)) { + dev_err(dev, "Failed to create DP PHY: %ld\n", PTR_ERR(phy)); + return PTR_ERR(phy); + } + phy_set_drvdata(phy, dp_phy); + + if (!dev->of_node) + phy_create_lookup(phy, "dp", dev_name(dev)); + + return 0; +} + +struct platform_driver mtk_dp_phy_driver = { + .probe = mtk_dp_phy_probe, + .driver = { + .name = "mediatek-dp-phy", + }, +}; +module_platform_driver(mtk_dp_phy_driver); + +MODULE_AUTHOR("Markus Schneider-Pargmann <msp(a)baylibre.com>"); +MODULE_DESCRIPTION("MediaTek DP PHY Driver"); +MODULE_LICENSE("GPL v2"); -- 2.32.0

3 3

Re: [PATCH] WIP: drm/dp_mst: Add support for dumping topology ref histories from debugfs
by Lyude Paul 07 Feb '22

07 Feb '22

(CC'ing this to dri-devel, since this is basically patch review) Alright - so first off sorry about the broken debugging patch! I thought I had tested that but I guess I hadn't tested it well enough, I'll get it fixed asap, but feel free to try getting to it before I do. As for this patch… (comments below) On Mon, 2021-12-20 at 02:17 +0000, Lin, Wayne wrote: > [AMD Official Use Only] > > Hi Lyude, > > No rush and thanks for your time! : ) > Just want to help clarify what I mean that "registering a callback function" > for CSN of disconnecting > stream sink device (not mst branch case). Attach below the tentative patch > that I planned to do. Thanks so much! > > Regards, > Wayne > --- > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 53 +++++++++++++++++++ > drivers/gpu/drm/drm_dp_mst_topology.c | 16 +++++- > include/drm/drm_dp_mst_helper.h | 1 + > 3 files changed, 68 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index cc34a35d0bcb..d7343c64908c 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -472,8 +472,61 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr > *mgr, > return connector; > } > > +void dm_dp_notify_csn_disconnection(struct drm_connector *connector) > +{ > + struct amdgpu_dm_connector *aconnector = > + to_amdgpu_dm_connector(connector); > + struct dc_sink *dc_sink = aconnector->dc_sink; > + struct dc_link *dc_link = aconnector->dc_link; > + struct amdgpu_device *adev = drm_to_adev(ddev); > + > + ASSERT(dc_link); > + > + if (dc_sink) { > + mutex_lock(&adev->dm.dc_lock); > + > + /*clear the remote sink of the link*/ > + dc_link_remove_remote_sink(dc_link, dc_sink); > + dc_sink_release(dc_sink); > + aconnector->dc_sink = NULL; > + > + mutex_unlock(&adev->dm.dc_lock); > + } > +} > + > static const struct drm_dp_mst_topology_cbs dm_mst_cbs = { > .add_connector = dm_dp_add_mst_connector, > + .notify_csn_disconnection = dm_dp_notify_csn_disconnection, > }; I still don't really think this is a good idea. This seems like it's just adding another hotplugging path to the code in order to avoid sending hotplugs for non-endpoint devices. In addition to the drm_connector issues I mentioned before, we also really need to stop doing any kind of payload maintence in hotplugging paths. The reality is any kind of payload maintanence we do outside of normal modesetting paths is a relic from legacy modesetting that I'm dropping ASAP (more on this below), and we can't keep adding to it because it dramatically complicates maintanence as well. Sorry for repeating this point so often but - the biggest issue too is I'm still not sure what it is we're even avoiding here. We know resources aren't released consistently, and that we're able to reproduce the behavior with repeated hotplugs. We also know that if we skip sending certain hotplug events, that fixes the issue. And we know we can workaround it by adding a special case for forcing a payload release in DC. But none of those actually tell us exactly what piece of code is leaking and why, which means that any workarounds we're putting in to avoid this mysterious guilty section of code we don't entirely understand either - which means we're just adding more code in that no one actually fully understands. This just ends up making maintence difficult because every change in code nearby workarounds like this has to strugle to try to figure out said workarounds in order to avoid breaking things. I'm actually currently running into these "later" issues right now, as recently I've (-actively-, finally!!!) been working on trying to remove as much non-atomic MST as I can because. As it turns out - a huge amount of the payload maintanence code just isn't needed at all when we stop caring about non-atomic drivers and stick everything in atomic state structs. Step 1 for updating updating the payload table, e.g. drm_dp_update_payload_part1(), is a great example of how messy things have become. Here's a small sample of some of the stuff I've seen from just that one function so far that either just don't make sense here or is totally redundant. I should note that a lot of these things also come from patches I reviwed, but didn't really look at as closely as I should have because I was swamped at work, some are historical artifacts, and others are less-than-ideal patches I got wrote myself when I was first started working on MST and didn't know the code as well as I do now: * We try to avoid some sort of userspace issue by using drm_dp_mst_port_downstream_of_branch() to avoid releasing payloads for a branch if we can't prove it's downstream of the top of the topology. This seems to workaround either a userspace bug. This is a redundant, since that's what topology refs are already supposed to be doing to the extent is reasonably possibly. It's also unfortunately racy solution because we have to be able to handle the situation where a connector is removed from under us. That can happen at any time, including _immediately_ after we call drm_dp_mst_port_downstream_of_branch() - rendering the call not really useful. * If we fail to validate the sink in drm_dp_update_payload_part, we don't update the payload table. I think at best this solution is racy and not useful, at worst it leaves us with a payload table that doesn't match what we attempted to set in the atomic state - which at worst brings us into undefined territory where we're just plain out of sync with the reality in hw. * Actually fun fact - mgr->payloads and mgr->proposed_vcpis both can and definitely should be removed entirely. All of the info for mgr->payloads could just be in the atomic state, because that + the magic of atomic state duplication means we'll also have an accurate view of the previous state's payload allocations: which renders mgr->proposed_vcpis redundant. Apologies for the long explanation again, but I hope that explains my point here a bit. I'm going to be trying to get to moving amdgpu's DSC code out of amdgpu and into DRM helpers as well soon, so I'm really determined to clean stuff up beforehand as every time I've done so it's become substantially easier to make changes to this code. Things used to be even worse before I started cleaning things up 2 or 3 years ago, where simple changes would end up getting me stuck spending hours trying to dig through lockdep or memory manegement issues. As well, I would be entirely unsurprised if bugs like this very payload leak we're working on just disappear once we've gotten rid of all the extraneous workarounds and state tracking here - especially with how many special cases we have for maintaining the payload table right now. That's certainly ended up being the case in the past with a number of other difficult to track down issues I've dealt with in MST. Anyhow. I've been way more productive this year then last because I had over a month off work and am finally not super burnt out from my job, and so far I've been making progress on the payload state cleanup far faster then I was last year :). I think if you can't figure out where the leak is coming from even once I get the debugging patches I mentioned fixed up, it might be a good idea for us to try again after I've got some of this code cleaned up. I've got a currently WIP branch here: https://gitlab.freedesktop.org/lyudess/linux/-/commit/624dd68fa804e64b5b206… > > void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 857c5d15e81d..a70e26c5d084 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -2508,6 +2508,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch > *mstb, > u8 new_pdt; > bool new_mcs; > bool dowork = false, create_connector = false; > + bool notify_unplug_event = false; > > port = drm_dp_get_port(mstb, conn_stat->port_number); > if (!port) > @@ -2520,6 +2521,9 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch > *mstb, > * port, so just throw the port out and make sure we > * reprobe the link address of it's parent MSTB > */ > + /* should also consider notify_unplug_event here. > + * but it's not a normal case for products in the > market > + */ > drm_dp_mst_topology_unlink_port(mgr, port); > mstb->link_address_sent = false; > dowork = true; > @@ -2541,10 +2545,14 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch > *mstb, > port->ddps = conn_stat->displayport_device_plug_status; > > if (old_ddps != port->ddps) { > - if (port->ddps && !port->input) > + if (port->ddps && !port->input) { > drm_dp_send_enum_path_resources(mgr, mstb, port); > - else > + } else { > port->full_pbn = 0; > + if (port->connector && > + drm_dp_mst_is_end_device(port->pdt, port- > >mcs)) > + notify_unplug_event = true; > + } > } > > new_pdt = port->input ? DP_PEER_DEVICE_NONE : conn_stat- > >peer_device_type; > @@ -2557,11 +2565,15 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch > *mstb, > dowork = false; > } > > + if (notify_unplug_event && mgr->cbs->notify_csn_disconnection) > + mgr->cbs->notify_csn_disconnection(port->connector); > + > if (port->connector) > drm_modeset_unlock(&mgr->base.lock); > else if (create_connector) > drm_dp_mst_port_add_connector(mstb, port); > > + > out: > drm_dp_mst_topology_put_port(port); > if (dowork) > diff --git a/include/drm/drm_dp_mst_helper.h > b/include/drm/drm_dp_mst_helper.h > index 78044ac5b59b..ff9e47729841 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -525,6 +525,7 @@ struct drm_dp_mst_topology_cbs { > * IRQ pulse handler. > */ > void (*poll_hpd_irq)(struct drm_dp_mst_topology_mgr *mgr); > + void (*notify_csn_disconnection)(struct drm_connector *connector); > }; > > #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8) > -- > 2.31.0 > > > -----Original Message----- > > From: Lin, Wayne > > Sent: Wednesday, December 8, 2021 11:39 AM > > To: 'Lyude Paul' <lyude(a)redhat.com> > > Subject: RE: [PATCH] WIP: drm/dp_mst: Add support for dumping topology ref > > histories from debugfs > > > > No worries Lyude! > > Thanks for keeping helping on this. Take your time : ) > > > > > -----Original Message----- > > > From: Lyude Paul <lyude(a)redhat.com> > > > Sent: Wednesday, December 8, 2021 7:05 AM > > > To: Lin, Wayne <Wayne.Lin(a)amd.com> > > > Subject: Re: [PATCH] WIP: drm/dp_mst: Add support for dumping topology > > > ref histories from debugfs > > > > > > Sorry! I will try to get to this tomorrow, if not then sometime this > > > week. > > > > > > On Tue, 2021-11-30 at 08:41 +0000, Lin, Wayne wrote: > > > > [Public] > > > > > > > > Hi Lyude, > > > > > > > > Finally have some bandwidth to get back to this problem! > > > > I roughly went through this patch and I'm just aware that we already > > > > have such kind of convenient tool for a while. > > > > I think it's definitely useful for us to track port/mstb reference > > > > count issues and I'll start to embrace this feature for cleaning up > > > > those issues. Thank you Lyude! > > > > > > > > However, I think the issue that I was trying to fix is not related > > > > to what you suggested: > > > > " The idea here is that if stream resources aren't being released, > > > > my guess would be that we're not dropping topology references for > > > > the port which means the connector never goes away." > > > > The issue I was trying to fix is about releasing > > > > dc_link->remote_sinks while receiving a CSN message notifying the > > > > connection status of a sst connector of a port changed from > > > > connected to disconnected. Not the connection status changed of a mst > > > > branch device. > > > > e.g. > > > > src - 1st_mstb - 2nd_mstb - sst_monitor => src - 1st_mstb > > > > (disconnect) 2nd_mstb - sst_monitor > > > > > > > > In above case, after receiving CSN, we will put topology references > > > > of 2nd mstb and its port which is connected with the sst monitor. As > > > > the result of that, we can call > > > > drm_dp_delayed_destroy_port() to unregister and put the drm connector. > > > > > > > > However, in below case: > > > > e.g. > > > > src - 1st_mstb - sst_monitor => src - 1st_mstb (disconnect) > > > > sst_monitor > > > > > > > > In this case, which is the case having problem, it definitely won't > > > > decrease the topology references count of the port which was > > > > connected to the sst monitor to zero since the port is still > > > > existing in the topology. Same as the malloc reference since the > > > > port can't get destroyed. Hence, the port still exists and we won't > > > > call > > > > drm_dp_delayed_destroy_port() to unregister and put the drm connector. > > > > I looked up the code and drm_dp_delayed_destroy_port() seems like > > > > the only place to call drm_connector_put() which means we can't put > > > > reference count of drm connector under this case and can't release > > > > dc_sink resource by destroying drm connector. > > > > > > > > I would also like to point out that this resource (remote_sinks) is > > > > specific to different stream sinks. So if we're trying to release > > > > this dc_sink resource by destroying the drm connector, it conflicts > > > > the idea that you suggested before that we should always keep the > > > > drm connector until it's no longer reachable in the topology. > > > > Releasing dc_sink should be binding with the disconnection event. > > > > > > > > I understand your concern that we should not just easily change the > > > > logic here since it's the result after solving tons of bugs before > > > > and might cause other side effect. So, just my 2 cents, what I'm > > > > thinking is to register a callback function for our driver to notify > > > > us that the remote sink is detached. This just aligns our flow > > > > handling long HPD event of legacy (sst) DP. > > > > For sst case, once we detect long HPD event indicating the monitor > > > > is detached, we will immediately try to release the > > > > dc_link->local_sink and fire hotplug event to upper layer. Same as > > > > here, once receives a CSN message notifying a drm connector is > > > > changed from connected to disconnected, trigger the callback > > > > function and we can try to release the dc_sink resource. > > > > > > > > Would like to know your thought and insight please : ) > > > > > > > > Btw, I got some errors and warnings while building and have some > > > > code adjustments as below : ) Thank you Lyude for your always kindly > > > > help!! > > > > > > > > Regards, > > > > Wayne > > > > > -----Original Message----- > > > > > From: Lyude Paul <lyude(a)redhat.com> > > > > > Sent: Wednesday, November 3, 2021 7:15 AM > > > > > To: Lin, Wayne <Wayne.Lin(a)amd.com> > > > > > Subject: [PATCH] WIP: drm/dp_mst: Add support for dumping topology > > > > > ref histories from debugfs > > > > > > > > > > TODO: > > > > > * Implement support for i915 > > > > > * Finish writing this commit message > > > > > * ??? > > > > > * profit > > > > > > > > > > Signed-off-by: Lyude Paul <lyude(a)redhat.com> > > > > > --- > > > > > > > > > > Hey wayne! SO-hopefully if I did this correctly then this should > > > > > just work on amdgpu. What this patch should do is add a debugfs > > > > > file to amdgpu called "amdgpu_dp_mst_topology_refs", and when you > > > > > read the file it should print out the topology reference history > > > > > of every MSTB and Port in memory, along with how many times we've > > > > > hit the codepath in each backtrace. An example: > > > > > > > > > > Port DP-5 (0000000005c37748) topology ref history: > > > > > 1 gets (last at 58.468973): > > > > > drm_dp_send_link_address+0x6a5/0xa00 [drm_kms_helper] > > > > > drm_dp_check_and_send_link_address+0xad/0xd0 [drm_kms_helper] > > > > > drm_dp_mst_link_probe_work+0x14e/0x1a0 [drm_kms_helper] > > > > > process_one_work+0x1e3/0x390 > > > > > worker_thread+0x50/0x3a0 > > > > > kthread+0x124/0x150 > > > > > ret_from_fork+0x1f/0x30 > > > > > 1 puts (last at 58.469357): > > > > > drm_dp_mst_topology_put_port+0x6a/0x210 [drm_kms_helper] > > > > > drm_dp_send_link_address+0x39e/0xa00 [drm_kms_helper] > > > > > drm_dp_check_and_send_link_address+0xad/0xd0 [drm_kms_helper] > > > > > drm_dp_mst_link_probe_work+0x14e/0x1a0 [drm_kms_helper] > > > > > process_one_work+0x1e3/0x390 > > > > > worker_thread+0x50/0x3a0 > > > > > kthread+0x124/0x150 > > > > > ret_from_fork+0x1f/0x30 > > > > > > > > > > The idea here is that if stream resources aren't being released, > > > > > my guess would be that we're not dropping topology references for > > > > > the port which means the connector never goes away. So, if that's > > > > > really the case then once we unplug the offending connector we > > > > > should be able to find a pair of gets/puts for the offending > > > > > leaked connector where the get/put count doesn't match up. Also, > > > > > if the frame count on the backtrace isn't long enough you can > > > > > increase the value of STACK_DEPTH in > > > > > drivers/gpu/drm/drm_dp_mst_topology.c and recompile to get more > > > > > frames. > > > > > > > > > > To enable this, first enable CONFIG_EXPERT for your kernel which > > > > > will unhide CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS. Then just > > > > > enable CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS, recompile, and it > > > > > should be good to go. > > > > > > > > > > Let me know if this works for you, and hopefully this should tell > > > > > us exactly what the problem actually is here. > > > > > > > > > > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 35 ++++ > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 173 > > > > > ++++++++++++++---- > > > > > drivers/gpu/drm/nouveau/nouveau_debugfs.c | 35 ++++ > > > > > include/drm/drm_dp_mst_helper.h | 18 ++ > > > > > 4 files changed, 228 insertions(+), 33 deletions(-) > > > > > > > > > > > > > > > diff --git > > > > > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > > > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > > > > > index 1a68a674913c..1a14732c52b4 100644 > > > > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > > > > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > > > > > @@ -3063,6 +3063,37 @@ static int mst_topo_show(struct seq_file > > > > > *m, void > > > > > *unused) > > > > > return 0; > > > > > } > > > > > > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > +static int mst_topology_ref_dump_show(struct seq_file *m, void > > > > > +*unused) { > > > > > + struct amdgpu_device *adev = (struct amdgpu_device > > > > > +*)m->private; > > > > > + struct drm_device *dev = adev_to_drm(adev); > > > > > + struct drm_connector *connector; > > > > > + struct drm_connector_list_iter conn_iter; > > > > > + struct amdgpu_dm_connector *aconnector; > > > > > + > > > > > + drm_connector_list_iter_begin(dev, &conn_iter); > > > > > + drm_for_each_connector_iter(connector, &conn_iter) { > > > > > + if (connector->connector_type != > > > > > DRM_MODE_CONNECTOR_DisplayPort) > > > > > + continue; > > > > > + > > > > > + aconnector = to_amdgpu_dm_connector(connector); > > > > > + > > > > > + /* Ensure we're only dumping the topology of a root > > > > > +mst node > > > > > */ > > > > > + if (!aconnector->mst_mgr.max_payloads) > > > > > + continue; > > > > > + > > > > > + seq_printf(m, "\nMST topology for connector %d\n", > > > > > aconnector->connector_id); > > > > > + drm_dp_mst_dump_topology_refs(m, > > > > > +&aconnector->mst_mgr); > > > > > + } > > > > > + drm_connector_list_iter_end(&conn_iter); > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +DEFINE_SHOW_ATTRIBUTE(mst_topology_ref_dump); > > > > > +#endif > > > > > + > > > > > /* > > > > > * Sets trigger hpd for MST topologies. > > > > > * All connected connectors will be rediscovered and re started > > > > > as needed if val of 1 is sent. > > > > > @@ -3299,6 +3330,10 @@ void dtn_debugfs_init(struct amdgpu_device > > > > > *adev) > > > > > > > > > > debugfs_create_file("amdgpu_mst_topology", 0444, root, > > > > > adev, &mst_topo_fops); > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > + debugfs_create_file("amdgpu_dp_mst_topology_refs", 0444, > > > > > +root, adev, > > > > > + &mst_topology_ref_dump_fops); #endif > > > > > debugfs_create_file("amdgpu_dm_dtn_log", 0644, root, adev, > > > > > &dtn_log_fops); > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > index 1aa8702383d4..0159828c494d 100644 > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > @@ -1366,23 +1366,6 @@ static int drm_dp_mst_wait_tx_reply(struct > > > > > drm_dp_mst_branch *mstb, > > > > > return ret; > > > > > } > > > > > > > > > > -static struct drm_dp_mst_branch *drm_dp_add_mst_branch_device(u8 > > > > > lct, u8 > > > > > *rad) -{ > > > > > - struct drm_dp_mst_branch *mstb; > > > > > - > > > > > - mstb = kzalloc(sizeof(*mstb), GFP_KERNEL); > > > > > - if (!mstb) > > > > > - return NULL; > > > > > - > > > > > - mstb->lct = lct; > > > > > - if (lct > 1) > > > > > - memcpy(mstb->rad, rad, lct / 2); > > > > > - INIT_LIST_HEAD(&mstb->ports); > > > > > - kref_init(&mstb->topology_kref); > > > > > - kref_init(&mstb->malloc_kref); > > > > > - return mstb; > > > > > -} > > > > > - > > > > > static void drm_dp_free_mst_branch_device(struct kref *kref) { > > > > > struct drm_dp_mst_branch *mstb = @@ -1642,12 +1625,20 @@ > > > > > topology_ref_type_to_str(enum drm_dp_mst_topology_ref_type type) > > > > > return "put"; > > > > > } > > > > > > > > > > +static const char *topology_ref_history_type_to_str(enum > > > > > +drm_dp_mst_topology_history_type type) { > > > > > + if (type == DRM_DP_MST_TOPOLOGY_HISTORY_PORT) > > > > > + return "Port"; > > > > > + else > > > > > + return "MSTB"; > > > > > +} > > > > > + > > > > > static void > > > > > -__dump_topology_ref_history(struct > > > > > drm_dp_mst_topology_ref_history *history, > > > > > - void *ptr, const char *type_str) > > > > > +dump_topology_ref_history(struct drm_dp_mst_topology_ref_history > > > > > +*history, struct drm_printer p) > > > > > { > > > > > - struct drm_printer p = drm_debug_printer(DBG_PREFIX); > > > > > + char *connector_name = NULL; > > > > > char *buf = kzalloc(PAGE_SIZE, GFP_KERNEL); > > > > > + void *ptr; > > > > > int i; > > > > > > > > > > if (!buf) > > > > > @@ -1656,14 +1647,29 @@ __dump_topology_ref_history(struct > > > > > drm_dp_mst_topology_ref_history *history, > > > > > if (!history->len) > > > > > goto out; > > > > > > > > > > + /* Get a pointer to the actual MSTB/port so we can the > > > > > +memory > > > > > address to the kernel log */ > > > > > + if (history->type == DRM_DP_MST_TOPOLOGY_HISTORY_PORT) > > > > > + ptr = container_of(history, struct drm_dp_mst_port, > > > > > topology_ref_history); > > > > > + else > > > > > + ptr = container_of(history, struct > > > > > +drm_dp_mst_branch, topology_ref_history); > > > > > + > > > > > /* First, sort the list so that it goes from oldest to > > > > > newest > > > > > * reference entry > > > > > */ > > > > > sort(history->entries, history->len, > > > > > sizeof(*history->entries), > > > > > topology_ref_history_cmp, NULL); > > > > > > > > > > - drm_printf(&p, "%s (%p) topology count reached 0, dumping > > > > > history:\n", > > > > > - type_str, ptr); > > > > > + if (history->type == DRM_DP_MST_TOPOLOGY_HISTORY_PORT) { > > > > > + struct drm_dp_mst_port *port = ptr; > > > > > + > > > > > + if (port->connector) > > > > > + connector_name = port->connector->name; > > > > > + } > > > > > + if (connector_name) > > > > > + drm_printf(&p, "Port %s (%p) topology ref > > > > > +history:\n", > > > > > connector_name, ptr); > > > > > + else > > > > > + drm_printf(&p, "%s (%p) topology ref history:\n", > > > > > + > > > > > +topology_ref_history_type_to_str(history->type), > > > > > ptr); > > > > > > > > > > for (i = 0; i < history->len; i++) { > > > > > const struct drm_dp_mst_topology_ref_entry *entry = > > > > > @@ > > > > > - > > > > > 1682,24 +1688,92 @@ __dump_topology_ref_history(struct > > > > > drm_dp_mst_topology_ref_history *history, > > > > > ts_nsec, rem_nsec / 1000, buf); > > > > > } > > > > > > > > > > - /* Now free the history, since this is the only time we > > > > > expose it */ > > > > > - kfree(history->entries); > > > > > out: > > > > > kfree(buf); > > > > > } > > > > > > > > > > +/** > > > > > + * drm_dp_mst_dump_topology_refs - helper function for dumping > > > > > +the topology ref history > > > > > + * @m: File to print to > > > > > + * @mgr: &struct drm_dp_mst_topology_mgr to use > > > > > + * > > > > > + * Prints the topology ref history of all ports and MSTBs on @mgr > > > > > +that are still in memory, > > > > > + * regardless of whether they're actually still reachable through > > > > > +the topology or not. Only enabled > > > > > + * when %CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS is enabled. Can be > > > > > +implemented by drivers to assist > > > > > + * with debugging leaks in the DP MST helpers. > > > > > + */ > > > > > +void drm_dp_mst_dump_topology_refs(struct seq_file *m, struct > > > > > +drm_dp_mst_topology_mgr *mgr) { > > > > > + struct drm_dp_mst_topology_ref_history *history; > > > > > + struct drm_printer p = drm_seq_file_printer(m); > > > > > + > > > > > + mutex_lock(&mgr->topology_ref_history_lock); > > > > > + list_for_each_entry(history, > > > > > +&mgr->topology_ref_history_list, > > > > > +node) > > > > > + dump_topology_ref_history(history, p); > > > > > + mutex_unlock(&mgr->topology_ref_history_lock); > > > > > +} > > > > > +EXPORT_SYMBOL(drm_dp_mst_dump_topology_refs); > > > > > + > > > > > +static void > > > > > +__init_topology_ref_history(struct > > > > > +drm_dp_mst_topology_ref_history > > > > > *history, > > > > > + struct drm_dp_mst_topology_mgr *mgr, > > > > > + enum drm_dp_mst_topology_history_type > > > > > +type) { > > > > > + history->type = type; > > > > > + INIT_LIST_HEAD(&history->node); > > > > > + > > > > > + mutex_lock(&mgr->topology_ref_history_lock); > > > > > + list_add(&history->node, &mgr->topology_ref_history_list); > > > > > + mutex_unlock(&mgr->topology_ref_history_lock); > > > > > +} > > > > > + > > > > > +static void > > > > > +__destroy_topology_ref_history(struct > > > > > +drm_dp_mst_topology_ref_history > > > > > *history, > > > > > + struct drm_dp_mst_topology_mgr *mgr) > > > > > +{ > > > > > + mutex_lock(&mgr->topology_ref_history_lock); > > > > > + list_del(&mgr->topology_ref_history_list); > > > > > + mutex_unlock(&mgr->topology_ref_history_lock); > > > > > + > > > > > + kfree(history->entries); > > > > > +} > > > > > + > > > > > +static __always_inline void > > > > > +init_port_topology_history(struct drm_dp_mst_topology_mgr *mgr, > > > > > +struct drm_dp_mst_port *port) { > > > > > + __init_topology_ref_history(&port->topology_ref_history, > > > > > +mgr, > > > > > + > > > > > +DRM_DP_MST_TOPOLOGY_HISTORY_PORT); > > > > > +} > > > > > + > > > > > +static __always_inline void > > > > > +init_mstb_topology_history(struct drm_dp_mst_topology_mgr *mgr, > > > > > +struct drm_dp_mst_branch *mstb) { > > > > > + __init_topology_ref_history(&mstb->topology_ref_history, > > > > > +mgr, > > > > > + > > > > > +DRM_DP_MST_TOPOLOGY_HISTORY_MSTB); > > > > > +} > > > > > + > > > > > +static __always_inline void > > > > > +destroy_port_topology_history(struct drm_dp_mst_port *port) { > > > > > + __destroy_topology_ref_history(&port->topology_ref_history, > > > > > +port->mgr); } > > > > > + > > > > > +static __always_inline void > > > > > +destroy_mstb_topology_history(struct drm_dp_mst_branch *mstb) { > > > > > + __destroy_topology_ref_history(&mstb->topology_ref_history, > > > > > +mstb->mgr); } > > > > > + > > > > > static __always_inline void > > > > > dump_mstb_topology_history(struct drm_dp_mst_branch *mstb) { > > > > > - __dump_topology_ref_history(&mstb->topology_ref_history, > > > > > mstb, > > > > > - "MSTB"); > > > > > + dump_topology_ref_history(&mstb->topology_ref_history, > > > > > +drm_debug_printer(DBG_PREFIX)); > > > > > } > > > > > > > > > > static __always_inline void > > > > > dump_port_topology_history(struct drm_dp_mst_port *port) { > > > > > - __dump_topology_ref_history(&port->topology_ref_history, > > > > > port, > > > > > - "Port"); > > > > > + dump_topology_ref_history(&port->topology_ref_history, > > > > > +drm_debug_printer(DBG_PREFIX)); > > > > > } > > > > > > > > > > static __always_inline void > > > > > @@ -1729,6 +1803,14 @@ topology_ref_history_unlock(struct > > > > > drm_dp_mst_topology_mgr *mgr) } #else static inline void > > > > > +init_port_topology_history(struct drm_dp_mst_topology_mgr *mgr, > > > > > +struct drm_dp_mst_port *port); static inline void > > > > Should also add the bracket, otherwise will get warnings. > > > > => static inline void init_port_topology_history(struct > > > > drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port) {}; > > > > > > > > > +init_mstb_topology_history(struct drm_dp_mst_topology_mgr *mgr, > > > > > +struct drm_dp_mst_branch *mstb); static inline void > > > > Same as above > > > > => static inline void init_mstb_topology_history(struct > > > > drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_branch *mstb) {}; > > > > > > > > > +destroy_port_topology_history(struct drm_dp_mst_topology_mgr > > > > > +*mgr, struct drm_dp_mst_port *port); static inline void > > > > destroy_port_topology_history() takes one parameter only which is > > > > port. > > > > => destroy_port_topology_history(struct drm_dp_mst_port *port) {}; > > > > > > > > > +destroy_mstb_topology_history(struct drm_dp_mst_topology_mgr > > > > > +*mgr, struct drm_dp_mst_branch *mstb); static inline void > > > > destroy_mstb_topology_history() takes one parameter only which is > > > > mstb => destroy_mstb_topology_history(struct drm_dp_mst_branch > > > > *mstb) {}; > > > > > > > > > topology_ref_history_lock(struct drm_dp_mst_topology_mgr *mgr) {} > > > > > static inline void topology_ref_history_unlock(struct > > > > > drm_dp_mst_topology_mgr *mgr) {} @@ -1740,6 +1822,25 @@ > > > > > dump_port_topology_history(struct drm_dp_mst_port *port) {} > > > > > #define save_port_topology_ref(port, type) #endif > > > > > > > > > > +static struct drm_dp_mst_branch * > > > > > +drm_dp_add_mst_branch_device(struct drm_dp_mst_topology_mgr *mgr, > > > > > +u8 lct, u8 *rad) { > > > > > + struct drm_dp_mst_branch *mstb; > > > > > + > > > > > + mstb = kzalloc(sizeof(*mstb), GFP_KERNEL); > > > > > + if (!mstb) > > > > > + return NULL; > > > > > + > > > > > + mstb->lct = lct; > > > > > + if (lct > 1) > > > > > + memcpy(mstb->rad, rad, lct / 2); > > > > > + INIT_LIST_HEAD(&mstb->ports); > > > > > + kref_init(&mstb->topology_kref); > > > > > + kref_init(&mstb->malloc_kref); > > > > > + init_mstb_topology_history(mgr, mstb); > > > > > + return mstb; > > > > > +} > > > > > + > > > > > static void drm_dp_destroy_mst_branch_device(struct kref *kref) > > > > > { > > > > > struct drm_dp_mst_branch *mstb = @@ -1747,6 +1848,7 @@ > > > > > static void drm_dp_destroy_mst_branch_device(struct > > > > > kref *kref) > > > > > struct drm_dp_mst_topology_mgr *mgr = mstb->mgr; > > > > > > > > > > dump_mstb_topology_history(mstb); > > > > > + destroy_mstb_topology_history(mstb); > > > > > > > > > > INIT_LIST_HEAD(&mstb->destroy_next); > > > > > > > > > > @@ -1856,6 +1958,7 @@ static void drm_dp_destroy_port(struct kref > > > > > *kref) > > > > > struct drm_dp_mst_topology_mgr *mgr = port->mgr; > > > > > > > > > > dump_port_topology_history(port); > > > > > + destroy_port_topology_history(port); > > > > > > > > > > /* There's nothing that needs locking to destroy an input > > > > > port yet */ > > > > > if (port->input) { > > > > > @@ -2135,7 +2238,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port > > > > > *port, u8 new_pdt, > > > > > ret = drm_dp_mst_register_i2c_bus(port); > > > > > } else { > > > > > lct = drm_dp_calculate_rad(port, rad); > > > > > - mstb = drm_dp_add_mst_branch_device(lct, > > > > > rad); > > > > > + mstb = drm_dp_add_mst_branch_device(mgr, > > > > > +lct, rad); > > > > > if (!mstb) { > > > > > ret = -ENOMEM; > > > > > drm_err(mgr->dev, "Failed to create > > > > > MSTB for port %p", port); @@ -2366,6 +2469,8 @@ > > > > > drm_dp_mst_add_port(struct drm_device *dev, > > > > > */ > > > > > drm_dp_mst_get_mstb_malloc(mstb); > > > > > > > > > > + init_port_topology_history(mgr, port); > > > > > + > > > > > return port; > > > > > } > > > > > > > > > > @@ -3745,7 +3850,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct > > > > > drm_dp_mst_topology_mgr *mgr, bool ms > > > > > } > > > > > > > > > > /* add initial branch device at LCT 1 */ > > > > > - mstb = drm_dp_add_mst_branch_device(1, NULL); > > > > > + mstb = drm_dp_add_mst_branch_device(mgr, 1, NULL); > > > > > if (mstb == NULL) { > > > > > ret = -ENOMEM; > > > > > goto out_unlock; @@ -5512,14 +5617,16 @@ int > > > > > drm_dp_mst_topology_mgr_init(struct > > > > > drm_dp_mst_topology_mgr *mgr, > > > > > mutex_init(&mgr->delayed_destroy_lock); > > > > > mutex_init(&mgr->up_req_lock); > > > > > mutex_init(&mgr->probe_lock); -#if > > > > > IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) > > > > > - mutex_init(&mgr->topology_ref_history_lock); > > > > > -#endif > > > > > INIT_LIST_HEAD(&mgr->tx_msg_downq); > > > > > INIT_LIST_HEAD(&mgr->destroy_port_list); > > > > > INIT_LIST_HEAD(&mgr->destroy_branch_device_list); > > > > > INIT_LIST_HEAD(&mgr->up_req_list); > > > > > > > > > > +#if IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) > > > > > + mutex_init(&mgr->topology_ref_history_lock); > > > > > + INIT_LIST_HEAD(&mgr->topology_ref_history_list); > > > > > +#endif > > > > > + > > > > > /* > > > > > * delayed_destroy_work will be queued on a dedicated WQ, so > > > > > that any > > > > > * requeuing will be also flushed when deiniting the > > > > > topology manager. > > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c > > > > > b/drivers/gpu/drm/nouveau/nouveau_debugfs.c > > > > > index 1cbe01048b93..53ec7c1daada 100644 > > > > > --- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c > > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c > > > > > @@ -29,9 +29,13 @@ > > > > > */ > > > > > > > > > > #include <linux/debugfs.h> > > > > > +#include <drm/drm_dp_mst_helper.h> #include <drm/drm_encoder.h> > > > > > #include <nvif/class.h> > > > > > #include <nvif/if0001.h> > > > > > +#include <nouveau_encoder.h> > > > > > #include "nouveau_debugfs.h" > > > > > +#include "nouveau_display.h" > > > > > #include "nouveau_drv.h" > > > > > > > > > > static int > > > > > @@ -68,6 +72,34 @@ nouveau_debugfs_strap_peek(struct seq_file *m, > > > > > void > > > > > *data) > > > > > return 0; > > > > > } > > > > > > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > +static int nouveau_debugfs_mst_topology_history(struct seq_file > > > > > +*m, void *data) { > > > > > + struct drm_info_node *node = m->private; > > > > > + struct drm_device *dev = node->minor->dev; > > > > > + struct drm_encoder *encoder; > > > > > + struct nouveau_display *disp = nouveau_display(dev); > > > > > + > > > > > + if (disp->disp.object.oclass < GF110_DISP) > > > > > + return -EINVAL; > > > > > + > > > > > + drm_for_each_encoder(encoder, dev) { > > > > > + struct nv50_mstm *mstm; > > > > > + > > > > > + /* We need the real encoder for each MST mgr */ > > > > > + if (encoder->encoder_type != DRM_MODE_ENCODER_TMDS) > > > > > + continue; > > > > > + mstm = nouveau_encoder(encoder)->dp.mstm; > > > > > + if (!mstm) > > > > > + continue; > > > > > + > > > > > + seq_printf(m, "MSTM on %s:\n", encoder->name); > > > > > + drm_dp_mst_dump_topology_refs(m, &mstm->mgr); > > > > > + } > > > > > + return 0; > > > > > +} > > > > > +#endif > > > > > + > > > > > static int > > > > > nouveau_debugfs_pstate_get(struct seq_file *m, void *data) { @@ > > > > > -213,6 > > > > > +245,9 @@ static const struct file_operations > > > > > nouveau_pstate_fops = { static struct drm_info_list > > > > > nouveau_debugfs_list[] = { > > > > > { "vbios.rom", nouveau_debugfs_vbios_image, 0, NULL }, > > > > > { "strap_peek", nouveau_debugfs_strap_peek, 0, NULL }, > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > + { "dp_mst_topology_refs", > > > > > +nouveau_debugfs_mst_topology_history, 0, NULL }, #endif > > > > > }; > > > > > #define NOUVEAU_DEBUGFS_ENTRIES ARRAY_SIZE(nouveau_debugfs_list) > > > > > > > > > > diff --git a/include/drm/drm_dp_mst_helper.h > > > > > b/include/drm/drm_dp_mst_helper.h index 78044ac5b59b..e3a73d8d97c0 > > > > > 100644 > > > > > --- a/include/drm/drm_dp_mst_helper.h > > > > > +++ b/include/drm/drm_dp_mst_helper.h > > > > > @@ -35,6 +35,11 @@ enum drm_dp_mst_topology_ref_type { > > > > > DRM_DP_MST_TOPOLOGY_REF_PUT, > > > > > }; > > > > > > > > > > +enum drm_dp_mst_topology_history_type { > > > > > + DRM_DP_MST_TOPOLOGY_HISTORY_PORT, > > > > > + DRM_DP_MST_TOPOLOGY_HISTORY_MSTB, }; > > > > > + > > > > > struct drm_dp_mst_topology_ref_history { > > > > > struct drm_dp_mst_topology_ref_entry { > > > > > enum drm_dp_mst_topology_ref_type type; @@ -43,6 > > > > > +48,9 @@ struct drm_dp_mst_topology_ref_history { > > > > > depot_stack_handle_t backtrace; > > > > > } *entries; > > > > > int len; > > > > > + > > > > > + enum drm_dp_mst_topology_history_type type; > > > > > + struct list_head node; > > > > > }; > > > > > #endif /* IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) */ > > > > > > > > > > @@ -769,6 +777,12 @@ struct drm_dp_mst_topology_mgr { > > > > > * &drm_dp_mst_branch.topology_ref_history. > > > > > */ > > > > > struct mutex topology_ref_history_lock; > > > > > + > > > > > + /** > > > > > + * @topology_ref_history_list: contains a list of topology > > > > > +ref > > > > > histories for any MSTBs or > > > > > + * ports that are still in memory > > > > > + */ > > > > > + struct list_head topology_ref_history_list; > > > > > #endif > > > > > }; > > > > > > > > > > @@ -873,6 +887,10 @@ void drm_dp_mst_put_port_malloc(struct > > > > > drm_dp_mst_port *port); > > > > > > > > > > struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct > > > > > drm_dp_mst_port *port); > > > > > > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > +void drm_dp_mst_dump_topology_refs(struct seq_file *m, struct > > > > > +drm_dp_mst_topology_mgr *mgr); #endif > > > > > + > > > > > extern const struct drm_private_state_funcs > > > > > drm_dp_mst_topology_state_funcs; > > > > > > > > > > /** > > > > > -- > > > > > 2.31.1 > > > > > > > > > > -- > > > Cheers, > > > Lyude Paul (she/her) > > > Software Engineer at Red Hat > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat

2 5

[RFC v2 0/8] Define and use reset domain for GPU recovery in amdgpu
by Andrey Grodzovsky 07 Feb '22

07 Feb '22

This patchset is based on earlier work by Boris[1] that allowed to have an ordered workqueue at the driver level that will be used by the different schedulers to queue their timeout work. On top of that I also serialized any GPU reset we trigger from within amdgpu code to also go through the same ordered wq and in this way simplify somewhat our GPU reset code so we don't need to protect from concurrency by multiple GPU reset triggeres such as TDR on one hand and sysfs trigger or RAS trigger on the other hand. As advised by Christian and Daniel I defined a reset_domain struct such that all the entities that go through reset together will be serialized one against another. TDR triggered by multiple entities within the same domain due to the same reason will not be triggered as the first such reset will cancel all the pending resets. This is relevant only to TDR timers and not to triggered resets coming from RAS or SYSFS, those will still happen after the in flight resets finishes. v2: Add handling on SRIOV configuration, the reset notify coming from host and driver already trigger a work queue to handle the reset so drop this intermidiate wq and send directly to timeout wq. (Shaoyun) [1] https://patchwork.kernel.org/project/dri-devel/patch/20210629073510.2764391… P.S Going through drm-misc-next and not amd-staging-drm-next as Boris work hasn't landed yet there. Andrey Grodzovsky (8): drm/amdgpu: Introduce reset domain drm/amdgpu: Move scheduler init to after XGMI is ready drm/amdgpu: Fix crash on modprobe drm/amdgpu: Serialize non TDR gpu recovery with TDRs drm/amd/virt: For SRIOV send GPU reset directly to TDR queue. drm/amdgpu: Drop hive->in_reset drm/amdgpu: Drop concurrent GPU reset protection for device drm/amd/virt: Drop concurrent GPU reset protection for SRIOV drivers/gpu/drm/amd/amdgpu/amdgpu.h | 9 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 206 +++++++++++---------- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 36 +--- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h | 3 +- drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 18 +- drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c | 18 +- drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c | 7 +- 10 files changed, 147 insertions(+), 164 deletions(-) -- 2.25.1

9 54

[Bug 201539] New: AMDGPU R9 390 automatic fan speed control in Linux 4.19
by bugzilla-daemon＠bugzilla.kernel.org 05 Feb '22

05 Feb '22

https://bugzilla.kernel.org/show_bug.cgi?id=201539 Bug ID: 201539 Summary: AMDGPU R9 390 automatic fan speed control in Linux 4.19 Product: Drivers Version: 2.5 Kernel Version: 4.19 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri(a)kernel-bugs.osdl.org Reporter: 0xe2.0x9a.0x9b(a)gmail.com Regression: No GPU: R9 390 Kernel module: amdgpu Application (example): Rise of the Tomb Raider benchmark In Linux 4.18 the fan speed of the GPU gradually adapts to GPU load. Maximum pwm is 155 during the benchmark. In Linux 4.19 the overall fan speed is lower (which is more silent than 4.18, with maximum at 112) but the fan spikes to full speed (pwm=255) for 1-2 seconds which is extremely loud. The 4.18 and 4.19 kernels are using the same firmware. -- You are receiving this mail because: You are watching the assignee of the bug.

2 78

[PATCH v2] drm/dp: Fix off-by-one in register cache size
by Kees Cook 05 Feb '22

05 Feb '22

The pcon_dsc_dpcd array holds 13 registers (0x92 through 0x9E). Fix the math to calculate the max size. Found from a -Warray-bounds build: drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_pcon_dsc_bpp_incr': drivers/gpu/drm/drm_dp_helper.c:3130:28: error: array subscript 12 is outside array bounds of 'const u8[12]' {aka 'const unsigned char[12]'} [-Werror=array-bounds] 3130 | buf = pcon_dsc_dpcd[DP_PCON_DSC_BPP_INCR - DP_PCON_DSC_ENCODER]; | ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/drm_dp_helper.c:3126:39: note: while referencing 'pcon_dsc_dpcd' 3126 | int drm_dp_pcon_dsc_bpp_incr(const u8 pcon_dsc_dpcd[DP_PCON_DSC_ENCODER_CAP_SIZE]) | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: David Airlie <airlied(a)linux.ie> Cc: Daniel Vetter <daniel(a)ffwll.ch> Cc: dri-devel(a)lists.freedesktop.org Fixes: e2e16da398d9 ("drm/dp_helper: Add support for Configuring DSC for HDMI2.1 Pcon") Cc: stable(a)vger.kernel.org Reviewed-by: Gustavo A. R. Silva <gustavoars(a)kernel.org> Link: https://lore.kernel.org/lkml/20211214001849.GA62559@embeddedor/ Signed-off-by: Kees Cook <keescook(a)chromium.org> --- v1: https://lore.kernel.org/lkml/20211203084333.3105038-1-keescook@chromium.org/ v2: - add reviewed-by - add cc:stable --- include/drm/drm_dp_helper.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 30359e434c3f..472dac376284 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -456,7 +456,7 @@ struct drm_panel; #define DP_FEC_CAPABILITY_1 0x091 /* 2.0 */ /* DP-HDMI2.1 PCON DSC ENCODER SUPPORT */ -#define DP_PCON_DSC_ENCODER_CAP_SIZE 0xC /* 0x9E - 0x92 */ +#define DP_PCON_DSC_ENCODER_CAP_SIZE 0xD /* 0x92 through 0x9E */ #define DP_PCON_DSC_ENCODER 0x092 # define DP_PCON_DSC_ENCODER_SUPPORTED (1 << 0) # define DP_PCON_DSC_PPS_ENC_OVERRIDE (1 << 1) -- 2.30.2

1 1

[v5 0/3] drm/msm/dsi: Add 10nm dsi phy tuning configuration support
by Rajeev Nandan 04 Feb '22

04 Feb '22

This series is to add DSI PHY tuning support in Qualcomm Snapdragon SoCs with 10nm DSI PHY e.g. SC7180 In most cases the default values of DSI PHY tuning registers should be sufficient as they are fully optimized. However, in some cases (for example, where extreme board parasitics cause the eye shape to degrade), the override bits can be used to improve the signal quality. Different DSI PHY versions have different configurations to adjust the drive strength, drive level, de-emphasis, etc. The current series has only those configuration options supported by 10nm PHY, e.g. drive strength and drive level. The number of registers to configure the drive strength are different for 7nm PHY. The design can be extended to other DSI PHY versions if required, as each PHY version can have its callback to get the input from DT and prepare register values. Changes in v2: - Addressed dt-bindings comments (Stephen Boyd, Dmitry Baryshkov) - Split into generic code and 10nm-specific part (Dmitry Baryshkov) - Fix the backward compatibility (Dmitry Baryshkov) Changes in v3: - Addressed dt-bindings comments (Rob Herring, Dmitry Baryshkov) - Address comments for phy tuning data structure (Dmitry Baryshkov) - s/ops.tuning_cfg_init/ops.parse_dt_properties (Dmitry Baryshkov) Changes in v4: - Fixed dt_binding_check error (Rob Herring's bot) - Return error in case of out of range values (Dmitry Baryshkov) - Return error if dt property is present but parsing is failing Changes in v5: - Fixed missing printk arg Rajeev Nandan (3): dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties drm/msm/dsi: Add dsi phy tuning configuration support drm/msm/dsi: Add 10nm dsi phy tuning configuration support .../bindings/display/msm/dsi-phy-10nm.yaml | 36 +++++++ drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 6 ++ drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 4 + drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 118 +++++++++++++++++++-- 4 files changed, 158 insertions(+), 6 deletions(-) -- 2.7.4

3 5

← Newer
1
...
12
13
14
15
16
17
18
...
85
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel January 2022