Re: [RFC PATCH v3 09/17] drm/i915: Do not support vm_bind mode in execbuf2
On Sat, Aug 27, 2022 at 09:43:55PM +0200, Andi Shyti wrote: From: Niranjana Vishwanathapura Do not support the vm in vm_bind_mode in execbuf2 ioctl. Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Ramalingam C Signed-off-by: Andi Shyti --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index cd75b0ca2555f..f85f10cf9c34b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -781,6 +781,11 @@ static int eb_select_context(struct i915_execbuffer *eb) if (unlikely(IS_ERR(ctx))) return PTR_ERR(ctx); + if (ctx->vm->vm_bind_mode) { + i915_gem_context_put(ctx); + return -EOPNOTSUPP; + } + eb->gem_context = ctx; if (i915_gem_context_has_full_ppgtt(ctx)) eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT; This should probably be merged with patch #2 that introduces vm_bind_mode uapi. Niranjana -- 2.34.1
RE: [PATCH v6 2/2] drm: rcar-du: Add RZ/G2L DSI driver
Hi Laurent and all, Thanks for the feedback. > Subject: Re: [PATCH v6 2/2] drm: rcar-du: Add RZ/G2L DSI driver > > Hi Biju, > > On Tue, Aug 30, 2022 at 08:22:08AM +, Biju Das wrote: > > Subject: Re: [PATCH v6 2/2] drm: rcar-du: Add RZ/G2L DSI driver > > > On Mon, Aug 29, 2022 at 10:19:01AM +0100, Biju Das wrote: > > > > This driver supports the MIPI DSI encoder found in the RZ/G2L SoC. > > > > It currently supports DSI video mode only. > > > > > > > > Signed-off-by: Biju Das > > > > Acked-by: Sam Ravnborg > > > > --- > > > > v5->v6: > > > > * Updated commit description > > > > * Moved handling of arst and prst from rzg2l_mipi_dsi_startup- > > > >runtime > > > >PM suspend/resume handlers. > > > > * Max lane capability read at probe(), and enforced in > > > >rzg2l_mipi_dsi_host_attach() > > > > * Simplified vich1ppsetr setting. > > > > * Renamed hsclk_running_mode,hsclk_mode->is_clk_cont. > > > > * Fixed typo in probe error message(arst->rst). > > > > * Reordered DRM bridge initaization in probe() > > > > * Updated typo in e-mail address. > > > > v4->v5: > > > > * Added Ack from Sam. > > > > * Added a trivial change, replaced rzg2l_mipi_dsi_parse_dt() > > > >with drm_of_get_data_lanes_count_ep() in probe. > > > > v3->v4: > > > > * Updated error handling in rzg2l_mipi_dsi_startup() and > > > > rzg2l_mipi_dsi_atomic_enable() > > > > v2->v3: > > > > * pass rzg2l_mipi_dsi pointer to {Link,Phy} register rd/wr > > > > function > > > instead > > > >of the memory pointer > > > > * Fixed the comment in rzg2l_mipi_dsi_startup() > > > > * Removed unnecessary dbg message from > > > > rzg2l_mipi_dsi_start_video() > > > > * DRM bridge parameter initialization moved to probe > > > > * Replaced dev_dbg->dev_err in rzg2l_mipi_dsi_parse_dt() > > > > * Inserted the missing blank lane after return in probe() > > > > * Added missing MODULE_DEVICE_TABLE > > > > * Added include linux/bits.h in header file > > > > * Fixed various macros in header file. > > > > * Reorder the make file for DSI, so that it is no more dependent > > > >on RZ/G2L DU patch series. > > > > v1->v2: > > > > * Rework based on dt-binding change (DSI + D-PHY) as single block > > > > * Replaced link_mmio and phy_mmio with mmio in struct > > > > rzg2l_mipi_dsi > > > > * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write > > > >and rzg2l_mipi_dsi_link_write > > > > * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read > > > > RFC->v1: > > > > * Added "depends on ARCH_RENESAS || COMPILE_TEST" on KCONFIG > > > >and dropped DRM as it is implied by DRM_BRIDGE > > > > * Used devm_reset_control_get_exclusive() for reset handle > > > > * Removed bool hsclkmode from struct rzg2l_mipi_dsi > > > > * Added error check for pm, using pm_runtime_resume_and_get() > > > > instead > > > of > > > >pm_runtime_get_sync() > > > > * Added check for unsupported formats in > > > > rzg2l_mipi_dsi_host_attach() > > > > * Avoided read-modify-write stopping hsclock > > > > * Used devm_platform_ioremap_resource for resource allocation > > > > * Removed unnecessary assert call from probe and remove. > > > > * wrap the line after the PTR_ERR() in probe() > > > > * Updated reset failure messages in probe > > > > * Fixed the typo arstc->prstc > > > > * Made hex constants to lower case. > > > > RFC: > > > > * > > > > --- > > > > drivers/gpu/drm/rcar-du/Kconfig | 8 + > > > > drivers/gpu/drm/rcar-du/Makefile | 2 + > > > > drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c | 703 > > > ++ > > > > drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h | 151 > > > > 4 files changed, 864 insertions(+) create mode 100644 > > > > drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c > > > > create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h > > > > > > > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig > > > > b/drivers/gpu/drm/rcar-du/Kconfig index c959e8c6be7d..58ffb8c2443b > > > > 100644 > > > > --- a/drivers/gpu/drm/rcar-du/Kconfig > > > > +++ b/drivers/gpu/drm/rcar-du/Kconfig > > > > @@ -51,6 +51,14 @@ config DRM_RCAR_MIPI_DSI > > > > help > > > > Enable support for the R-Car Display Unit embedded MIPI > DSI > > > encoders. > > > > > > > > +config DRM_RZG2L_MIPI_DSI > > > > + tristate "RZ/G2L MIPI DSI Encoder Support" > > > > + depends on DRM_BRIDGE && OF > > > > + depends on ARCH_RENESAS || COMPILE_TEST > > > > + select DRM_MIPI_DSI > > > > + help > > > > + Enable support for the RZ/G2L Display Unit embedded MIPI > DSI > > > encoders. > > > > + > > > > config DRM_RCAR_VSP > > > > bool "R-Car DU VSP Compositor Support" if ARM > > > > default y if ARM64 > > > > diff --git a/drivers/gpu/drm/rcar-du/Makefile > > > > b/drivers/gpu/drm/rcar-du/Makefile > > > > index 6f132325c8b7..b8f2c82651d9 100644 > > > > --- a/drivers/gpu/drm/rcar-du/Makefile > > > > +++ b/drivers/gpu/drm/rcar-du/Makefile > > > >
[PATCH v6 6/6] arm64: dts: qcom: sc7280: Add Reset support for gpu
Add support for Reset using GPUCC driver for GPU. This helps to ensure that GPU state is reset by making sure that CX head switch is collapsed. Signed-off-by: Akhil P Oommen --- (no changes since v1) arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index e66fc67..f5257d6 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -2243,6 +2243,9 @@ nvmem-cells = <_speed_bin>; nvmem-cell-names = "speed_bin"; + resets = < GPU_CX_COLLAPSE>; + reset-names = "cx_collapse"; + gpu_opp_table: opp-table { compatible = "operating-points-v2"; -- 2.7.4
[PATCH v6 5/6] dt-bindings: drm/msm/gpu: Add optional resets
Add an optional reference to GPUCC reset which can be used to ensure cx gdsc collapse during gpu recovery. Signed-off-by: Akhil P Oommen Acked-by: Rob Herring Acked-by: Krzysztof Kozlowski --- (no changes since v5) Changes in v5: - Nit: Remove a duplicate blank line (Krzysztof) Changes in v4: - New patch in v4 Documentation/devicetree/bindings/display/msm/gpu.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml index 3397bc3..408ed97 100644 --- a/Documentation/devicetree/bindings/display/msm/gpu.yaml +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml @@ -109,6 +109,12 @@ properties: For GMU attached devices a phandle to the GMU device that will control the power for the GPU. + resets: +maxItems: 1 + + reset-names: +items: + - const: cx_collapse required: - compatible -- 2.7.4
[PATCH v6 0/6] clk/qcom: Support gdsc collapse polling using 'reset' interface
Some clients like adreno gpu driver would like to ensure that its gdsc is collapsed at hardware during a gpu reset sequence. This is because it has a votable gdsc which could be ON due to a vote from another subsystem like tz, hyp etc or due to an internal hardware signal. To allow this, gpucc driver can expose an interface to the client driver using reset framework. Using this the client driver can trigger a polling within the gdsc driver. This series is rebased on top of linus's master branch. Related discussion: https://patchwork.freedesktop.org/patch/493144/ Changes in v6: - No code changes in this version. Just captured the Acked-by tags Changes in v5: - Nit: Remove a duplicate blank line (Krzysztof) Changes in v4: - Update gpu dt-binding schema - Typo fix in commit text Changes in v3: - Use pointer to const for "struct qcom_reset_ops" in qcom_reset_map (Krzysztof) Changes in v2: - Return error when a particular custom reset op is not implemented. (Dmitry) Akhil P Oommen (6): dt-bindings: clk: qcom: Support gpu cx gdsc reset clk: qcom: Allow custom reset ops clk: qcom: gdsc: Add a reset op to poll gdsc collapse clk: qcom: gpucc-sc7280: Add cx collapse reset support dt-bindings: drm/msm/gpu: Add optional resets arm64: dts: qcom: sc7280: Add Reset support for gpu .../devicetree/bindings/display/msm/gpu.yaml | 6 + arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++ drivers/clk/qcom/gdsc.c| 23 ++ drivers/clk/qcom/gdsc.h| 7 ++ drivers/clk/qcom/gpucc-sc7280.c| 10 drivers/clk/qcom/reset.c | 27 ++ drivers/clk/qcom/reset.h | 8 +++ include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++ 8 files changed, 83 insertions(+), 4 deletions(-) -- 2.7.4
[PATCH v6 3/6] clk: qcom: gdsc: Add a reset op to poll gdsc collapse
Add a reset op compatible function to poll for gdsc collapse. Signed-off-by: Akhil P Oommen Reviewed-by: Dmitry Baryshkov --- (no changes since v2) Changes in v2: - Minor update to function prototype drivers/clk/qcom/gdsc.c | 23 +++ drivers/clk/qcom/gdsc.h | 7 +++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index 44520ef..2d0f1d1 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -17,6 +17,7 @@ #include #include #include "gdsc.h" +#include "reset.h" #define PWR_ON_MASKBIT(31) #define EN_REST_WAIT_MASK GENMASK_ULL(23, 20) @@ -116,7 +117,8 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en) return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val); } -static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status) +static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status, + s64 timeout_us, unsigned int interval_ms) { ktime_t start; @@ -124,7 +126,9 @@ static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status) do { if (gdsc_check_status(sc, status)) return 0; - } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US); + if (interval_ms) + msleep(interval_ms); + } while (ktime_us_delta(ktime_get(), start) < timeout_us); if (gdsc_check_status(sc, status)) return 0; @@ -172,7 +176,7 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status) udelay(1); } - ret = gdsc_poll_status(sc, status); + ret = gdsc_poll_status(sc, status, TIMEOUT_US, 0); WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n"); if (!ret && status == GDSC_OFF && sc->rsupply) { @@ -343,7 +347,7 @@ static int _gdsc_disable(struct gdsc *sc) */ udelay(1); - ret = gdsc_poll_status(sc, GDSC_ON); + ret = gdsc_poll_status(sc, GDSC_ON, TIMEOUT_US, 0); if (ret) return ret; } @@ -565,3 +569,14 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain) return 0; } EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable); + +int gdsc_wait_for_collapse(void *priv) +{ + struct gdsc *sc = priv; + int ret; + + ret = gdsc_poll_status(sc, GDSC_OFF, 50, 5); + WARN(ret, "%s status stuck at 'on'", sc->pd.name); + return ret; +} +EXPORT_SYMBOL_GPL(gdsc_wait_for_collapse); diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index ad313d7..d484bdb 100644 --- a/drivers/clk/qcom/gdsc.h +++ b/drivers/clk/qcom/gdsc.h @@ -12,6 +12,7 @@ struct regmap; struct regulator; struct reset_controller_dev; +struct qcom_reset_map; /** * struct gdsc - Globally Distributed Switch Controller @@ -79,6 +80,7 @@ int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *, struct regmap *); void gdsc_unregister(struct gdsc_desc *desc); int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain); +int gdsc_wait_for_collapse(void *priv); #else static inline int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *rcdev, @@ -88,5 +90,10 @@ static inline int gdsc_register(struct gdsc_desc *desc, } static inline void gdsc_unregister(struct gdsc_desc *desc) {}; + +static int gdsc_wait_for_collapse(void *priv) +{ + return -ENOSYS; +} #endif /* CONFIG_QCOM_GDSC */ #endif /* __QCOM_GDSC_H__ */ -- 2.7.4
[PATCH v6 2/6] clk: qcom: Allow custom reset ops
Allow soc specific clk drivers to specify a custom reset operation. We will use this in an upcoming patch to allow gpucc driver to specify a differet reset operation for cx_gdsc. Signed-off-by: Akhil P Oommen Reviewed-by: Dmitry Baryshkov --- (no changes since v3) Changes in v3: - Use pointer to const for "struct qcom_reset_ops" in qcom_reset_map (Krzysztof) Changes in v2: - Return error when a particular custom reset op is not implemented. (Dmitry) drivers/clk/qcom/reset.c | 27 +++ drivers/clk/qcom/reset.h | 8 2 files changed, 35 insertions(+) diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c index 819d194..b7ae4a3 100644 --- a/drivers/clk/qcom/reset.c +++ b/drivers/clk/qcom/reset.c @@ -13,6 +13,21 @@ static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id) { + struct qcom_reset_controller *rst; + const struct qcom_reset_map *map; + + rst = to_qcom_reset_controller(rcdev); + map = >reset_map[id]; + + if (map->ops && map->ops->reset) + return map->ops->reset(map->priv); + /* +* If custom ops is implemented but just not this callback, return +* error +*/ + else if (map->ops) + return -EOPNOTSUPP; + rcdev->ops->assert(rcdev, id); udelay(1); rcdev->ops->deassert(rcdev, id); @@ -28,6 +43,12 @@ qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id) rst = to_qcom_reset_controller(rcdev); map = >reset_map[id]; + + if (map->ops && map->ops->assert) + return map->ops->assert(map->priv); + else if (map->ops) + return -EOPNOTSUPP; + mask = BIT(map->bit); return regmap_update_bits(rst->regmap, map->reg, mask, mask); @@ -42,6 +63,12 @@ qcom_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id) rst = to_qcom_reset_controller(rcdev); map = >reset_map[id]; + + if (map->ops && map->ops->deassert) + return map->ops->deassert(map->priv); + else if (map->ops) + return -EOPNOTSUPP; + mask = BIT(map->bit); return regmap_update_bits(rst->regmap, map->reg, mask, 0); diff --git a/drivers/clk/qcom/reset.h b/drivers/clk/qcom/reset.h index 2a08b5e..f3147eb 100644 --- a/drivers/clk/qcom/reset.h +++ b/drivers/clk/qcom/reset.h @@ -8,9 +8,17 @@ #include +struct qcom_reset_ops { + int (*reset)(void *priv); + int (*assert)(void *priv); + int (*deassert)(void *priv); +}; + struct qcom_reset_map { unsigned int reg; u8 bit; + const struct qcom_reset_ops *ops; + void *priv; }; struct regmap; -- 2.7.4
[PATCH v6 4/6] clk: qcom: gpucc-sc7280: Add cx collapse reset support
Allow a consumer driver to poll for cx gdsc collapse through Reset framework. Signed-off-by: Akhil P Oommen Reviewed-by: Dmitry Baryshkov --- (no changes since v3) Changes in v3: - Convert 'struct qcom_reset_ops cx_gdsc_reset' to 'static const' (Krzysztof) Changes in v2: - Minor update to use the updated custom reset ops implementation drivers/clk/qcom/gpucc-sc7280.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c index 9a832f2..fece3f4 100644 --- a/drivers/clk/qcom/gpucc-sc7280.c +++ b/drivers/clk/qcom/gpucc-sc7280.c @@ -433,12 +433,22 @@ static const struct regmap_config gpu_cc_sc7280_regmap_config = { .fast_io = true, }; +static const struct qcom_reset_ops cx_gdsc_reset = { + .reset = gdsc_wait_for_collapse, +}; + +static const struct qcom_reset_map gpucc_sc7280_resets[] = { + [GPU_CX_COLLAPSE] = { .ops = _gdsc_reset, .priv = _gdsc }, +}; + static const struct qcom_cc_desc gpu_cc_sc7280_desc = { .config = _cc_sc7280_regmap_config, .clks = gpu_cc_sc7280_clocks, .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks), .gdscs = gpu_cc_sc7180_gdscs, .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs), + .resets = gpucc_sc7280_resets, + .num_resets = ARRAY_SIZE(gpucc_sc7280_resets), }; static const struct of_device_id gpu_cc_sc7280_match_table[] = { -- 2.7.4
[PATCH v6 1/6] dt-bindings: clk: qcom: Support gpu cx gdsc reset
Add necessary definitions in gpucc bindings to ensure gpu cx gdsc collapse through 'reset' framework for SC7280. Signed-off-by: Akhil P Oommen Acked-by: Krzysztof Kozlowski --- (no changes since v1) include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/dt-bindings/clock/qcom,gpucc-sc7280.h b/include/dt-bindings/clock/qcom,gpucc-sc7280.h index 669b23b..843a31b 100644 --- a/include/dt-bindings/clock/qcom,gpucc-sc7280.h +++ b/include/dt-bindings/clock/qcom,gpucc-sc7280.h @@ -32,4 +32,7 @@ #define GPU_CC_CX_GDSC 0 #define GPU_CC_GX_GDSC 1 +/* GPU_CC reset IDs */ +#define GPU_CX_COLLAPSE0 + #endif -- 2.7.4
[PATCH linux-next] drm/gem: Remove the unneeded result variable
From: ye xingchen Return the value drm_gem_handle_delete() directly instead of storing it in another redundant variable. Reported-by: Zeal Robot Signed-off-by: ye xingchen --- drivers/gpu/drm/drm_gem.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index ad068865ba20..3fa0deff3014 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -782,14 +782,11 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_gem_close *args = data; - int ret; if (!drm_core_check_feature(dev, DRIVER_GEM)) return -EOPNOTSUPP; - ret = drm_gem_handle_delete(file_priv, args->handle); - - return ret; + return drm_gem_handle_delete(file_priv, args->handle); } /** -- 2.25.1
Re: [PATCH v2 32/41] drm/vc4: vec: Convert to the new TV mode property
Hi Maxime, I tested your patchset on my Pi and it mostly works. Good work! However, I noticed a couple of issues. > -static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder, > - struct drm_crtc_state *crtc_state, > - struct drm_connector_state *conn_state) > -{ > - const struct vc4_vec_tv_mode *vec_mode; > - > - vec_mode = _vec_tv_modes[conn_state->tv.mode]; > - > - if (conn_state->crtc && > - !drm_mode_equal(vec_mode->mode, _state->adjusted_mode)) > - return -EINVAL; > - > - return 0; > -} I may have said it myself that we should allow custom modelines without too much validation. The VC4 and VEC, however, have some considerable limitations when it comes to the modelines that they can reliably output. In particular, attempting to use "50 Hz" timings in NTSC/PAL-M modes (or "60 Hz" in PAL/SECAM modes) results in a weirdly skewed image. Here's how it may look like: https://user-images.githubusercontent.com/4499762/187575940-736e7262-c82d-42f3-a2d8-f309cbd51139.png This is because if the CRTC does not trigger the sync pulses within an acceptable time window, the VEC apparently generates them itself. This causes the VEC sync pulses (which go onto the wire) not quite line up with the ones from the modeline, which results in what you see on the screenshot. I once wrote a validation function based on extensive testing of what produces a sensible output and what doesn't. You can find it here: https://github.com/raspberrypi/linux/pull/4406/commits/15c0c51. I think it might be a good idea to include something like that - even though I know it's somewhat ugly. (BTW, those %2 checks on vertical timings in that linked commit can be ignored; those values are divided by 2 for interlaced modes anyway. Those checks were intended to ensure proper odd-first or even-first timings; I'm not sure if your code calculates those in the same way) > static int vc4_vec_connector_get_modes(struct drm_connector *connector) > { > - struct drm_connector_state *state = connector->state; > struct drm_display_mode *mode; > + int count = 0; > > - mode = drm_mode_duplicate(connector->dev, > - vc4_vec_tv_modes[state->tv.mode].mode); > + mode = drm_mode_analog_ntsc_480i(connector->dev); > if (!mode) { > DRM_ERROR("Failed to create a new display mode\n"); > return -ENOMEM; > } > > drm_mode_probed_add(connector, mode); > + count += 1; > > - return 1; > + mode = drm_mode_analog_pal_576i(connector->dev); > + if (!mode) { > + DRM_ERROR("Failed to create a new display mode\n"); > + return -ENOMEM; > + } > + > + drm_mode_probed_add(connector, mode); > + count += 1; > + > + return count; > +} Xorg is pretty confused by these modes being reported like that. The 576i mode is *always* preferred, presumably because of the higher resolution. If the NTSC mode is set (via the kernel cmdline or just due to it being the default), this results in a mess on the screen - exactly the same thing as on the screenshot linked above. Note that drm_helper_probe_add_cmdline_mode() *does* add the DRM_MODE_TYPE_USERDEF flag to the 480i mode, having detected it as preferred on the command line - but Xorg does not seem to care about that. I remember Noralf suggesting setting DRM_MODE_TYPE_PREFERRED for the mode that corresponds to the currently chosen tv_mode - that would fix the problem. An alternative would be to _not_ add the "opposite" mode at all, like the current default Raspberry Pi OS kernel behaves. Note that if you decide to add the modeline validation like I suggested in the comment above, then without setting the preferred mode properly, Xorg will just give up and sit on a blank screen until you run xrandr from another terminal if tv_mode incompatible with 576i is selected. Best regards, Mateusz Kwiatkowski
Re: [PATCH v2 10/41] drm/modes: Add a function to generate analog display modes
Hi Maxime, Wow. That's an enormous amount of effort put into this patch. But I'm tempted to say that this is actually overengineered quite a bit :D Considering that there's no way to access all these calculations from user space, and I can't imagine anybody using anything else than those standard 480i/576i (and maybe 240p/288p) modes at 13.5 MHz any time soon... I'm not sure if we actually need all this. But anyway, I'm not the maintainer of this subsystem, so I'm not the one to decide. > +enum drm_mode_analog { > + DRM_MODE_ANALOG_NTSC, > + DRM_MODE_ANALOG_PAL, > +}; Using "NTSC" and "PAL" to describe the 50Hz and 60Hz analog TV modes is common, but strictly speaking a misnomer. Those are color encoding systems, and your patchset fully supports lesser used, but standard encodings for those (e.g. PAL-M for 60Hz and SECAM for 50Hz). I'd propose switching to some more neutral naming scheme. Some ideas: - DRM_MODE_ANALOG_60_HZ / DRM_MODE_ANALOG_50_HZ (after standard refresh rate) - DRM_MODE_ANALOG_525_LINES / DRM_MODE_ANALOG_625_LINES (after standard line count) - DRM_MODE_ANALOG_JM / DRM_MODE_ANALOG_BDGHIKLN (after corresponding ITU System Letter Designations) > +#define NTSC_HFP_DURATION_TYP_NS 1500 > +#define NTSC_HFP_DURATION_MIN_NS 1270 > +#define NTSC_HFP_DURATION_MAX_NS 2220 You've defined those min/typ/max ranges, but you're not using the "typ" field for anything other than hslen. The actual "typical" value is thus always the midpoint, which isn't necessarily the best choice. In particular, for the standard 720px wide modes at 13.5 MHz, hsync_start ends up being 735 for 480i and 734 for 576i, instead of 736 and 732 requested by BT.601. That's all obviously within tolerances, but the image ends up noticeably off-center (at least on modern TVs), especially in the 576i case. > + htotal = params->line_duration_ns * pixel_clock_hz / NSEC_PER_SEC; You're multiplying an unsigned int and an unsigned long - both types are only required to be 32 bit, so this is likely to overflow. You need to use a cast to unsigned long long, and then call do_div() for 64-bit division. This actually overflowed on me on my Pi running ARM32 kernel, resulting in negative horizontal porch lengths, and drm_helper_probe_add_cmdline_mode() taking over the mode generation (badly), and a horrible mess on screen. > + vfp = vfp_min + (porches_rem / 2); > + vbp = porches - vfp; Relative position of the vertical sync within the VBI effectively moves the image up and down. Adding that (porches_rem / 2) moves the image up off center by that many pixels. I'd keep the VFP always at minimum to keep the image centered. Best regards, Mateusz Kwiatkowski
Re: [PATCH 00/15] Tidy up vfio_device life cycle
On Sun, Aug 28, 2022 at 01:10:22AM +0800, Kevin Tian wrote: > Kevin Tian (6): > vfio: Add helpers for unifying vfio_device life cycle > drm/i915/gvt: Use the new device life cycle helpers > vfio/platform: Use the new device life cycle helpers > vfio/amba: Use the new device life cycle helpers > vfio/ccw: Use the new device life cycle helpers > vfio: Rename vfio_device_put() and vfio_device_try_get() > > Yi Liu (9): > vfio/pci: Use the new device life cycle helpers > vfio/mlx5: Use the new device life cycle helpers > vfio/hisi_acc: Use the new device life cycle helpers > vfio/mdpy: Use the new device life cycle helpers > vfio/mtty: Use the new device life cycle helpers > vfio/mbochs: Use the new device life cycle helpers > vfio/ap: Use the new device life cycle helpers > vfio/fsl-mc: Use the new device life cycle helpers > vfio: Add struct device to vfio_device Other than my small remarks this all looked good to me - for every patch: Reviewed-by: Jason Gunthorpe Thanks, Jason
RE: [PATCH 5/8] drm/i915: Rename and expose common GT early init routine
> -Original Message- > From: Roper, Matthew D > Sent: Monday, August 29, 2022 10:03 AM > To: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Sripada, Radhakrishna > ; Roper, Matthew D > > Subject: [PATCH 5/8] drm/i915: Rename and expose common GT early init > routine > > The common early GT init is needed for initialization of all GT types > (root/primary, remote tile, standalone media). Since standalone media > (coming in the next patch) will be implemented in a separate file, > rename and expose the function for use. > Reviewed-by: Radhakrishna Sripada - RK Sripada > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 6 +++--- > drivers/gpu/drm/i915/gt/intel_gt.h | 1 + > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c > b/drivers/gpu/drm/i915/gt/intel_gt.c > index 7c0525e96155..d21ec11346a5 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -35,7 +35,7 @@ > #include "intel_uncore.h" > #include "shmem_utils.h" > > -static void __intel_gt_init_early(struct intel_gt *gt) > +void intel_gt_common_init_early(struct intel_gt *gt) > { > spin_lock_init(>irq_lock); > > @@ -65,7 +65,7 @@ void intel_root_gt_init_early(struct drm_i915_private > *i915) > gt->i915 = i915; > gt->uncore = >uncore; > > - __intel_gt_init_early(gt); > + intel_gt_common_init_early(gt); > } > > static int intel_gt_probe_lmem(struct intel_gt *gt) > @@ -789,7 +789,7 @@ static int intel_gt_tile_setup(struct intel_gt *gt, > phys_addr_t phys_addr) > > gt->uncore = uncore; > > - __intel_gt_init_early(gt); > + intel_gt_common_init_early(gt); > } > > intel_uncore_init_early(gt->uncore, gt); > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h > b/drivers/gpu/drm/i915/gt/intel_gt.h > index 4d8779529cc2..c9a359f35d0f 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt.h > @@ -44,6 +44,7 @@ static inline struct intel_gt *gsc_to_gt(struct intel_gsc > *gsc) > return container_of(gsc, struct intel_gt, gsc); > } > > +void intel_gt_common_init_early(struct intel_gt *gt); > void intel_root_gt_init_early(struct drm_i915_private *i915); > int intel_gt_assign_ggtt(struct intel_gt *gt); > int intel_gt_init_mmio(struct intel_gt *gt); > -- > 2.37.2
Re: [PATCH 15/15] vfio: Add struct device to vfio_device
On Sun, Aug 28, 2022 at 01:10:37AM +0800, Kevin Tian wrote: > From: Yi Liu > > and replace kref. With it a 'vfio-dev/vfioX' node is created under the > sysfs path of the parent, indicating the device is bound to a vfio > driver, e.g.: > > /sys/devices/pci\:6f/\:6f\:01.0/vfio-dev/vfio0 > > It is also a preparatory step toward adding cdev for supporting future > device-oriented uAPI. > > Suggested-by: Jason Gunthorpe > Signed-off-by: Yi Liu > Signed-off-by: Kevin Tian > --- > drivers/vfio/vfio_main.c | 70 +--- > include/linux/vfio.h | 6 ++-- > 2 files changed, 61 insertions(+), 15 deletions(-) > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > index 0c5d120aeced..9ad0cbb83f1c 100644 > --- a/drivers/vfio/vfio_main.c > +++ b/drivers/vfio/vfio_main.c > @@ -46,6 +46,8 @@ static struct vfio { > struct mutexgroup_lock; /* locks group_list */ > struct ida group_ida; > dev_t group_devt; > + struct class*device_class; > + struct ida device_ida; > } vfio; > > struct vfio_iommu_driver { > @@ -524,11 +526,19 @@ EXPORT_SYMBOL_GPL(_vfio_alloc_device); > * > * Only vfio-ccw driver should call this interface. > */ > +static void vfio_device_release(struct device *dev); Since you added this new function in patch 1, it would be nice to place it in a way that avoids this forward reference in this patch > ret = alloc_chrdev_region(_devt, 0, MINORMASK + 1, "vfio"); I think we should change this "vfio" string at this point, it is really the group fd, so "vfio_group" ? It only shows in procfs. Jason
RE: [PATCH 4/8] drm/i915: Prepare more multi-GT initialization
> -Original Message- > From: Roper, Matthew D > Sent: Monday, August 29, 2022 10:03 AM > To: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Sripada, Radhakrishna > ; Roper, Matthew D > ; Iddamsetty, Aravind > > Subject: [PATCH 4/8] drm/i915: Prepare more multi-GT initialization > > We're going to introduce an additional intel_gt for MTL's media unit > soon. Let's provide a bit more multi-GT initialization framework in > preparation for that. The initialization will pull the list of GTs for > a platform from the device info structure. Although necessary for the > immediate MTL media enabling, this same framework will also be used > farther down the road when we enable remote tiles on xehpsdv and pvc. > > Cc: Aravind Iddamsetty LGTM. Reviewed-by: Radhakrishna Sripada - RK Sripada > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- > drivers/gpu/drm/i915/gt/intel_gt.c| 48 +-- > drivers/gpu/drm/i915/gt/intel_gt.h| 1 - > drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 ++ > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/intel_device_info.h | 16 +++ > .../gpu/drm/i915/selftests/mock_gem_device.c | 1 + > 7 files changed, 67 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > index 275ad72940c1..41acc285e8bf 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > @@ -736,7 +736,7 @@ static intel_engine_mask_t init_engine_mask(struct > intel_gt *gt) > u16 vdbox_mask; > u16 vebox_mask; > > - info->engine_mask = RUNTIME_INFO(i915)->platform_engine_mask; > + GEM_BUG_ON(!info->engine_mask); > > if (GRAPHICS_VER(i915) < 11) > return info->engine_mask; > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c > b/drivers/gpu/drm/i915/gt/intel_gt.c > index cf7aab7adb30..7c0525e96155 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -807,17 +807,16 @@ static void > intel_gt_tile_cleanup(struct intel_gt *gt) > { > intel_uncore_cleanup_mmio(gt->uncore); > - > - if (!gt_is_root(gt)) > - kfree(gt); > } > > int intel_gt_probe_all(struct drm_i915_private *i915) > { > struct pci_dev *pdev = to_pci_dev(i915->drm.dev); > struct intel_gt *gt = >gt0; > + const struct intel_gt_definition *gtdef; > phys_addr_t phys_addr; > unsigned int mmio_bar; > + unsigned int i; > int ret; > > mmio_bar = GRAPHICS_VER(i915) == 2 ? GEN2_GTTMMADR_BAR : > GTTMMADR_BAR; > @@ -828,14 +827,55 @@ int intel_gt_probe_all(struct drm_i915_private *i915) >* and it has been already initialized early during probe >* in i915_driver_probe() >*/ > + gt->i915 = i915; > + gt->name = "Primary GT"; > + gt->info.engine_mask = RUNTIME_INFO(i915)->platform_engine_mask; > + > + drm_dbg(>drm, "Setting up %s\n", gt->name); > ret = intel_gt_tile_setup(gt, phys_addr); > if (ret) > return ret; > > i915->gt[0] = gt; > > - /* TODO: add more tiles */ > + for (i = 1, gtdef = _INFO(i915)->extra_gt_list[i - 1]; > + gtdef->setup != NULL; > + i++, gtdef = _INFO(i915)->extra_gt_list[i - 1]) { > + gt = drmm_kzalloc(>drm, sizeof(*gt), GFP_KERNEL); > + if (!gt) { > + ret = -ENOMEM; > + goto err; > + } > + > + gt->i915 = i915; > + gt->name = gtdef->name; > + gt->type = gtdef->type; > + gt->info.engine_mask = gtdef->engine_mask; > + gt->info.id = i; > + > + drm_dbg(>drm, "Setting up %s\n", gt->name); > + if (GEM_WARN_ON(range_overflows_t(resource_size_t, > + gtdef->mapping_base, > + SZ_16M, > + pci_resource_len(pdev, > mmio_bar { > + ret = -ENODEV; > + goto err; > + } > + > + ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base); > + if (ret) > + goto err; > + > + i915->gt[i] = gt; > + } > + > return 0; > + > +err: > + i915_probe_error(i915, "Failed to initialize %s! (%d)\n", gtdef->name, > ret); > + intel_gt_release_all(i915); > + > + return ret; > } > > int intel_gt_tiles_init(struct drm_i915_private *i915) > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h > b/drivers/gpu/drm/i915/gt/intel_gt.h > index 40b06adf509a..4d8779529cc2 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt.h > @@ -54,7 +54,6 @@ void intel_gt_driver_register(struct intel_gt *gt); > void
Re: [PATCH 10/15] vfio/fsl-mc: Use the new device life cycle helpers
On Sun, Aug 28, 2022 at 01:10:32AM +0800, Kevin Tian wrote: > From: Yi Liu > > Export symbol of vfio_release_device_set() so fsl-mc @init can handle > the error path cleanly instead of assuming certain vfio core API can > help release device_set afterwards. I think you should leave it as is, the "device_set" cleanup is just something handled completely internally to vfio If ops->init fails then we expect the core code to clean the device_set, and it does because it calls vfio_init_device() already. Having a single weirdly placed release in the driver is pretty confusing, IMHO. Jason
RE: [PATCH 3/8] drm/i915: Use managed allocations for extra uncore objects
> -Original Message- > From: Roper, Matthew D > Sent: Monday, August 29, 2022 10:03 AM > To: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Sripada, Radhakrishna > ; Roper, Matthew D > > Subject: [PATCH 3/8] drm/i915: Use managed allocations for extra uncore > objects > > We're slowly transitioning the init-time kzalloc's of the driver over to > DRM-managed allocations; let's make sure the uncore objects allocated > for non-root GTs are thus allocated. > Reviewed-by: Radhakrishna Sripada - RK Sripada > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c > b/drivers/gpu/drm/i915/gt/intel_gt.c > index a82b5e2e0d83..cf7aab7adb30 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -783,7 +783,7 @@ static int intel_gt_tile_setup(struct intel_gt *gt, > phys_addr_t phys_addr) > if (!gt_is_root(gt)) { > struct intel_uncore *uncore; > > - uncore = kzalloc(sizeof(*uncore), GFP_KERNEL); > + uncore = drmm_kzalloc(>i915->drm, sizeof(*uncore), > GFP_KERNEL); > if (!uncore) > return -ENOMEM; > > @@ -808,10 +808,8 @@ intel_gt_tile_cleanup(struct intel_gt *gt) > { > intel_uncore_cleanup_mmio(gt->uncore); > > - if (!gt_is_root(gt)) { > - kfree(gt->uncore); > + if (!gt_is_root(gt)) > kfree(gt); > - } > } > > int intel_gt_probe_all(struct drm_i915_private *i915) > -- > 2.37.2
RE: [PATCH 2/8] drm/i915: Only hook up uncore->debug for primary uncore
Hi Matt, > -Original Message- > From: Roper, Matthew D > Sent: Monday, August 29, 2022 10:03 AM > To: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Sripada, Radhakrishna > ; Roper, Matthew D > > Subject: [PATCH 2/8] drm/i915: Only hook up uncore->debug for primary uncore > > The original intent of intel_uncore_mmio_debug as described in commit > 0a9b26306d6a ("drm/i915: split out uncore_mmio_debug") was to be a > singleton structure that could be shared between multiple GTs' uncore > objects in a multi-tile system. Somehow we went off track and > started allocating separate instances of this structure for each GT, > which defeats that original goal. > > But in reality, there isn't even a need to share the mmio_debug between > multiple GTs; on all modern platforms (i.e., everything after gen7) > unclaimed register accesses are something that can only be detected for > display registers. There's no point in grabbing the debug spinlock and > checking for unclaimed accesses on an uncore used by an xehpsdv or pvc > remote tile GT, or the uncore used by a mtl standalone media GT since > all of the display accesses go through the primary intel_uncore. > > The simplest solution is to simply leave uncore->debug NULL on all > intel_uncore instances except for the primary one. This will allow us > to avoid the pointless debug spinlock acquisition we've been doing on > MMIO accesses coming in through these intel_uncores. > Reviewed-by: Radhakrishna Sripada > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 9 - > drivers/gpu/drm/i915/i915_driver.c | 2 +- > drivers/gpu/drm/i915/intel_uncore.c | 23 ++- > drivers/gpu/drm/i915/intel_uncore.h | 3 +-- > 4 files changed, 20 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c > b/drivers/gpu/drm/i915/gt/intel_gt.c > index e4bac2431e41..a82b5e2e0d83 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -781,21 +781,13 @@ static int intel_gt_tile_setup(struct intel_gt *gt, > phys_addr_t phys_addr) > int ret; > > if (!gt_is_root(gt)) { > - struct intel_uncore_mmio_debug *mmio_debug; > struct intel_uncore *uncore; > > uncore = kzalloc(sizeof(*uncore), GFP_KERNEL); > if (!uncore) > return -ENOMEM; > > - mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL); > - if (!mmio_debug) { > - kfree(uncore); > - return -ENOMEM; > - } > - > gt->uncore = uncore; > - gt->uncore->debug = mmio_debug; > > __intel_gt_init_early(gt); > } > @@ -817,7 +809,6 @@ intel_gt_tile_cleanup(struct intel_gt *gt) > intel_uncore_cleanup_mmio(gt->uncore); > > if (!gt_is_root(gt)) { > - kfree(gt->uncore->debug); > kfree(gt->uncore); > kfree(gt); > } > diff --git a/drivers/gpu/drm/i915/i915_driver.c > b/drivers/gpu/drm/i915/i915_driver.c > index 053a7dab5506..de9020771836 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -326,7 +326,7 @@ static int i915_driver_early_probe(struct > drm_i915_private *dev_priv) > intel_device_info_subplatform_init(dev_priv); > intel_step_init(dev_priv); > > - intel_uncore_mmio_debug_init_early(_priv->mmio_debug); > + intel_uncore_mmio_debug_init_early(dev_priv); > > spin_lock_init(_priv->irq_lock); > spin_lock_init(_priv->gpu_error.lock); > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index e717ea55484a..6841f76533f9 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -44,14 +44,19 @@ fw_domains_get(struct intel_uncore *uncore, enum > forcewake_domains fw_domains) > } > > void > -intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug > *mmio_debug) > +intel_uncore_mmio_debug_init_early(struct drm_i915_private *i915) > { > - spin_lock_init(_debug->lock); > - mmio_debug->unclaimed_mmio_check = 1; > + spin_lock_init(>mmio_debug.lock); > + i915->mmio_debug.unclaimed_mmio_check = 1; > + > + i915->uncore.debug = >mmio_debug; > } > > static void mmio_debug_suspend(struct intel_uncore *uncore) > { > + if (!uncore->debug) > + return; > + > spin_lock(>debug->lock); > > /* Save and disable mmio debugging for the user bypass */ > @@ -67,6 +72,9 @@ static bool check_for_unclaimed_mmio(struct > intel_uncore *uncore); > > static void mmio_debug_resume(struct intel_uncore *uncore) > { > + if (!uncore->debug) > + return; > + > spin_lock(>debug->lock); > > if (!--uncore->debug->suspend_count) > @@ -1705,7 +1713,7 @@ unclaimed_reg_debug(struct intel_uncore *uncore, >
Re: [PATCH 15/15] vfio: Add struct device to vfio_device
On Tue, Aug 30, 2022 at 04:18:38PM -0600, Alex Williamson wrote: > On Sun, 28 Aug 2022 01:10:37 +0800 > Kevin Tian wrote: > > > From: Yi Liu > > > > and replace kref. With it a 'vfio-dev/vfioX' node is created under the > > sysfs path of the parent, indicating the device is bound to a vfio > > driver, e.g.: > > > > /sys/devices/pci\:6f/\:6f\:01.0/vfio-dev/vfio0 > > > > It is also a preparatory step toward adding cdev for supporting future > > device-oriented uAPI. > > Shall we start Documentation/ABI/testing/vfio-dev now? Thanks. I always thought that was something to use when adding new custom sysfs attributes? Here we are just creating a standard struct device with its standard sysfs? Jason
RE: [PATCH 1/8] drm/i915: Move locking and unclaimed check into mmio_debug_{suspend,resume}
> -Original Message- > From: Roper, Matthew D > Sent: Monday, August 29, 2022 10:03 AM > To: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Sripada, Radhakrishna > ; Roper, Matthew D > > Subject: [PATCH 1/8] drm/i915: Move locking and unclaimed check into > mmio_debug_{suspend,resume} > > Moving the locking for MMIO debug (and the final check for unclaimed > accesses when resuming debug after a userspace-initiated forcewake) will > make it simpler to completely skip MMIO debug handling on uncores that > don't support it in future patches. > LGTM, Reviewed-by: Radhakrishna Sripada > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/intel_uncore.c | 41 +++-- > 1 file changed, 21 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index 9b81b2543ce2..e717ea55484a 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -50,23 +50,33 @@ intel_uncore_mmio_debug_init_early(struct > intel_uncore_mmio_debug *mmio_debug) > mmio_debug->unclaimed_mmio_check = 1; > } > > -static void mmio_debug_suspend(struct intel_uncore_mmio_debug > *mmio_debug) > +static void mmio_debug_suspend(struct intel_uncore *uncore) > { > - lockdep_assert_held(_debug->lock); > + spin_lock(>debug->lock); > > /* Save and disable mmio debugging for the user bypass */ > - if (!mmio_debug->suspend_count++) { > - mmio_debug->saved_mmio_check = mmio_debug- > >unclaimed_mmio_check; > - mmio_debug->unclaimed_mmio_check = 0; > + if (!uncore->debug->suspend_count++) { > + uncore->debug->saved_mmio_check = uncore->debug- > >unclaimed_mmio_check; > + uncore->debug->unclaimed_mmio_check = 0; > } > + > + spin_unlock(>debug->lock); > } > > -static void mmio_debug_resume(struct intel_uncore_mmio_debug > *mmio_debug) > +static bool check_for_unclaimed_mmio(struct intel_uncore *uncore); > + > +static void mmio_debug_resume(struct intel_uncore *uncore) > { > - lockdep_assert_held(_debug->lock); > + spin_lock(>debug->lock); > + > + if (!--uncore->debug->suspend_count) > + uncore->debug->unclaimed_mmio_check = uncore->debug- > >saved_mmio_check; > > - if (!--mmio_debug->suspend_count) > - mmio_debug->unclaimed_mmio_check = mmio_debug- > >saved_mmio_check; > + if (check_for_unclaimed_mmio(uncore)) > + drm_info(>i915->drm, > + "Invalid mmio detected during user access\n"); > + > + spin_unlock(>debug->lock); > } > > static const char * const forcewake_domain_names[] = { > @@ -677,9 +687,7 @@ void intel_uncore_forcewake_user_get(struct > intel_uncore *uncore) > spin_lock_irq(>lock); > if (!uncore->user_forcewake_count++) { > intel_uncore_forcewake_get__locked(uncore, > FORCEWAKE_ALL); > - spin_lock(>debug->lock); > - mmio_debug_suspend(uncore->debug); > - spin_unlock(>debug->lock); > + mmio_debug_suspend(uncore); > } > spin_unlock_irq(>lock); > } > @@ -695,14 +703,7 @@ void intel_uncore_forcewake_user_put(struct > intel_uncore *uncore) > { > spin_lock_irq(>lock); > if (!--uncore->user_forcewake_count) { > - spin_lock(>debug->lock); > - mmio_debug_resume(uncore->debug); > - > - if (check_for_unclaimed_mmio(uncore)) > - drm_info(>i915->drm, > - "Invalid mmio detected during user access\n"); > - spin_unlock(>debug->lock); > - > + mmio_debug_resume(uncore); > intel_uncore_forcewake_put__locked(uncore, > FORCEWAKE_ALL); > } > spin_unlock_irq(>lock); > -- > 2.37.2
Re: [PATCH 15/15] vfio: Add struct device to vfio_device
On Sun, 28 Aug 2022 01:10:37 +0800 Kevin Tian wrote: > From: Yi Liu > > and replace kref. With it a 'vfio-dev/vfioX' node is created under the > sysfs path of the parent, indicating the device is bound to a vfio > driver, e.g.: > > /sys/devices/pci\:6f/\:6f\:01.0/vfio-dev/vfio0 > > It is also a preparatory step toward adding cdev for supporting future > device-oriented uAPI. Shall we start Documentation/ABI/testing/vfio-dev now? Thanks. Alex
Re: [PATCH 3/5] dt-bindings: clock: drop minItems equal to maxItems
Quoting Krzysztof Kozlowski (2022-08-25 04:33:32) > minItems, if missing, are implicitly equal to maxItems, so drop > redundant piece to reduce size of code. > > Signed-off-by: Krzysztof Kozlowski > --- Applied to clk-next
Re: [PATCH v1 00/35] drm: Analog TV Improvements
W dniu 26.08.2022 o 16:56, Dom Cobley pisze: > On Fri, 26 Aug 2022 at 05:08, Mateusz Kwiatkowski wrote: >> - Commenting out the pm_runtime_put() / pm_runtime_get_sync() calls in >> vc4_vec.c >> - Reverting this PR by Dom Cobley a.k.a. popcornmix: >>https://github.com/raspberrypi/linux/pull/4639 >> >> Either of these approaches makes VEC mode switching work again. Obviously >> neither is appropriate for a permanent solution. > Might be worth trying the latest rpi-update firmware. > There was a change that affects restoring PIXEL/VEC clocks after a > power domain cycle. > There is also a fix for a USB boot breakage. > > If you still have an issue that occurs in downstream pi tree but not > upstream, then create a linux github issue. Hi Dom, I just tested the 868f1cf firmware and its associated kernel, and everything works like a charm for me, both USB boot and VEC power management. Thanks!
[PATCH 2/2] drm/tests: Change "igt_" prefix to "drm_"
With the introduction of KUnit, IGT is no longer the only option to run the DRM unit tests, as the tests can be run through kunit-tool or on real hardware with CONFIG_KUNIT. Therefore, remove the "igt_" prefix from the tests and replace it with the "drm_" prefix, making the tests' names independent from the tool used. Signed-off-by: Maíra Canal --- drivers/gpu/drm/tests/drm_buddy_test.c| 84 +- .../gpu/drm/tests/drm_damage_helper_test.c| 84 +- .../gpu/drm/tests/drm_dp_mst_helper_test.c| 8 +- .../gpu/drm/tests/drm_format_helper_test.c| 8 +- drivers/gpu/drm/tests/drm_format_test.c | 20 +-- drivers/gpu/drm/tests/drm_mm_test.c | 155 +- drivers/gpu/drm/tests/drm_plane_helper_test.c | 4 +- drivers/gpu/drm/tests/drm_rect_test.c | 16 +- 8 files changed, 190 insertions(+), 189 deletions(-) diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c index d76f83833e75..a9393d788390 100644 --- a/drivers/gpu/drm/tests/drm_buddy_test.c +++ b/drivers/gpu/drm/tests/drm_buddy_test.c @@ -13,7 +13,7 @@ #include "../lib/drm_random.h" -#define IGT_TIMEOUT(name__) \ +#define TIMEOUT(name__) \ unsigned long name__ = jiffies + MAX_SCHEDULE_TIMEOUT static unsigned int random_seed; @@ -24,7 +24,7 @@ static inline u64 get_size(int order, u64 chunk_size) } __printf(2, 3) -static bool __igt_timeout(unsigned long timeout, const char *fmt, ...) +static bool __timeout(unsigned long timeout, const char *fmt, ...) { va_list va; @@ -43,8 +43,8 @@ static bool __igt_timeout(unsigned long timeout, const char *fmt, ...) return true; } -static void __igt_dump_block(struct kunit *test, struct drm_buddy *mm, -struct drm_buddy_block *block, bool buddy) +static void __dump_block(struct kunit *test, struct drm_buddy *mm, +struct drm_buddy_block *block, bool buddy) { kunit_err(test, "block info: header=%llx, state=%u, order=%d, offset=%llx size=%llx root=%d buddy=%d\n", block->header, drm_buddy_block_state(block), @@ -52,20 +52,20 @@ static void __igt_dump_block(struct kunit *test, struct drm_buddy *mm, drm_buddy_block_size(mm, block), !block->parent, buddy); } -static void igt_dump_block(struct kunit *test, struct drm_buddy *mm, - struct drm_buddy_block *block) +static void dump_block(struct kunit *test, struct drm_buddy *mm, + struct drm_buddy_block *block) { struct drm_buddy_block *buddy; - __igt_dump_block(test, mm, block, false); + __dump_block(test, mm, block, false); buddy = drm_get_buddy(block); if (buddy) - __igt_dump_block(test, mm, buddy, true); + __dump_block(test, mm, buddy, true); } -static int igt_check_block(struct kunit *test, struct drm_buddy *mm, - struct drm_buddy_block *block) +static int check_block(struct kunit *test, struct drm_buddy *mm, + struct drm_buddy_block *block) { struct drm_buddy_block *buddy; unsigned int block_state; @@ -137,8 +137,8 @@ static int igt_check_block(struct kunit *test, struct drm_buddy *mm, return err; } -static int igt_check_blocks(struct kunit *test, struct drm_buddy *mm, - struct list_head *blocks, u64 expected_size, bool is_contiguous) +static int check_blocks(struct kunit *test, struct drm_buddy *mm, + struct list_head *blocks, u64 expected_size, bool is_contiguous) { struct drm_buddy_block *block; struct drm_buddy_block *prev; @@ -150,7 +150,7 @@ static int igt_check_blocks(struct kunit *test, struct drm_buddy *mm, total = 0; list_for_each_entry(block, blocks, link) { - err = igt_check_block(test, mm, block); + err = check_block(test, mm, block); if (!drm_buddy_block_is_allocated(block)) { kunit_err(test, "block not allocated\n"); @@ -190,16 +190,16 @@ static int igt_check_blocks(struct kunit *test, struct drm_buddy *mm, if (prev) { kunit_err(test, "prev block, dump:\n"); - igt_dump_block(test, mm, prev); + dump_block(test, mm, prev); } kunit_err(test, "bad block, dump:\n"); - igt_dump_block(test, mm, block); + dump_block(test, mm, block); return err; } -static int igt_check_mm(struct kunit *test, struct drm_buddy *mm) +static int check_mm(struct kunit *test, struct drm_buddy *mm) { struct drm_buddy_block *root; struct drm_buddy_block *prev; @@ -233,7 +233,7 @@ static int igt_check_mm(struct kunit *test, struct drm_buddy *mm)
[PATCH 1/2] drm/tests: Split drm_framebuffer_create_test into parameterized tests
The igt_check_drm_framebuffer_create is based on a loop that executes tests for all createbuffer_tests test cases. This could be better represented by parameterized tests, provided by KUnit. So, convert the igt_check_drm_framebuffer_create test into parameterized tests. Signed-off-by: Maíra Canal --- drivers/gpu/drm/tests/drm_framebuffer_test.c | 23 +--- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c index ec7a08ba4056..3e46fd9f6615 100644 --- a/drivers/gpu/drm/tests/drm_framebuffer_test.c +++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c @@ -25,7 +25,7 @@ struct drm_framebuffer_test { const char *name; }; -static struct drm_framebuffer_test createbuffer_tests[] = { +static const struct drm_framebuffer_test drm_framebuffer_create_cases[] = { { .buffer_created = 1, .name = "ABGR normal sizes", .cmd = { .width = 600, .height = 600, .pixel_format = DRM_FORMAT_ABGR, .handles = { 1, 0, 0 }, .pitches = { 4 * 600, 0, 0 }, @@ -340,28 +340,25 @@ static struct drm_device mock_drm_device = { }, }; -static int execute_drm_mode_fb_cmd2(struct drm_mode_fb_cmd2 *r) +static void drm_framebuffer_create_test(struct kunit *test) { + const struct drm_framebuffer_test *params = test->param_value; int buffer_created = 0; mock_drm_device.dev_private = _created; - drm_internal_framebuffer_create(_drm_device, r, NULL); - return buffer_created; + drm_internal_framebuffer_create(_drm_device, >cmd, NULL); + KUNIT_EXPECT_EQ(test, params->buffer_created, buffer_created); } -static void igt_check_drm_framebuffer_create(struct kunit *test) +static void drm_framebuffer_to_desc(const struct drm_framebuffer_test *t, char *desc) { - int i = 0; - - for (i = 0; i < ARRAY_SIZE(createbuffer_tests); i++) { - KUNIT_EXPECT_EQ_MSG(test, createbuffer_tests[i].buffer_created, - execute_drm_mode_fb_cmd2(_tests[i].cmd), -"Test %d: \"%s\" failed\n", i, createbuffer_tests[i].name); - } + strcpy(desc, t->name); } +KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases, drm_framebuffer_to_desc); + static struct kunit_case drm_framebuffer_tests[] = { - KUNIT_CASE(igt_check_drm_framebuffer_create), + KUNIT_CASE_PARAM(drm_framebuffer_create_test, drm_framebuffer_create_gen_params), { } }; -- 2.37.2
Re: build failure of next-20220830 due to 5f8cdece42ff ("drm/msm/dsi: switch to DRM_PANEL_BRIDGE")
Hi, On 30/08/2022 15:40, Nathan Chancellor wrote: Hi Sudip, On Tue, Aug 30, 2022 at 10:18:43AM +0100, Sudip Mukherjee (Codethink) wrote: Hi All, The builds of arm64 allmodconfig with clang have failed to build next-20220830 with the error: drivers/gpu/drm/msm/dsi/dsi_host.c:1903:14: error: variable 'device_node' is uninitialized when used here [-Werror,-Wuninitialized] of_node_put(device_node); ^~~ drivers/gpu/drm/msm/dsi/dsi_host.c:1870:44: note: initialize the variable 'device_node' to silence this warning struct device_node *endpoint, *device_node; ^ = NULL git bisect pointed to 5f8cdece42ff ("drm/msm/dsi: switch to DRM_PANEL_BRIDGE") I will be happy to test any patch or provide any extra log if needed. Thanks for the report. I noticed this yesterday and sent a patch: https://lore.kernel.org/20220829165450.217628-1-nat...@kernel.org/ https://github.com/ClangBuiltLinux/linux/issues/1700 Updated and pushed the branch to unbreak the linux-next. Thank you! Cheers, Nathan -- With best wishes Dmitry
Re: [PATCH] drm/msm/dsi: Remove use of device_node in dsi_host_parse_dt()
On 29/08/2022 19:54, Nathan Chancellor wrote: Clang warns: drivers/gpu/drm/msm/dsi/dsi_host.c:1903:14: error: variable 'device_node' is uninitialized when used here [-Werror,-Wuninitialized] of_node_put(device_node); ^~~ drivers/gpu/drm/msm/dsi/dsi_host.c:1870:44: note: initialize the variable 'device_node' to silence this warning struct device_node *endpoint, *device_node; ^ = NULL 1 error generated. device_node's assignment was removed but not all of its uses. Remove the call to of_node_put() and the variable declaration to clean up the warning. Fixes: 5f8cdece42ff ("drm/msm/dsi: switch to DRM_PANEL_BRIDGE") Link: https://github.com/ClangBuiltLinux/linux/issues/1700 Signed-off-by: Nathan Chancellor Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: mainline build failure for x86_64 allmodconfig with clang
On Fri, Aug 26, 2022 at 10:31:34AM -0400, Alex Deucher wrote: > On Thu, Aug 25, 2022 at 6:34 PM Nathan Chancellor wrote: > > > > Hi AMD folks, > > > > Top posting because it might not have been obvious but I was looking for > > your feedback on this message (which can be viewed on lore.kernel.org if > > you do not have the original [1]) so that we can try to get this fixed > > in some way for 6.0/6.1. If my approach is not welcome, please consider > > suggesting another one or looking to see if this is something you all > > could look into. > > The patch looks good to me. I was hoping Harry or Rodrigo could > comment more since they are more familiar with this code and trying to > keep it in sync with what we get from the hardware teams. Thanks a lot for the input! That patch was broken but I have polished it and a few other patches up and sent them along for review: https://lore.kernel.org/20220830203409.3491379-1-nat...@kernel.org/ I did not CC everyone from this thread but it is on lore if others want to comment on it. Hopefully we can get this all sorted out for 6.0 final. Cheers, Nathan > > [1]: https://lore.kernel.org/Yv5h0rb3AgTZLVJv@dev-arch.thelio-3990X/ > > > > Cheers, > > Nathan > > > > On Thu, Aug 18, 2022 at 08:59:14AM -0700, Nathan Chancellor wrote: > > > Hi Arnd, > > > > > > Doubling back around to this now since I think this is the only thing > > > breaking x86_64 allmodconfig with clang 11 through 15. > > > > > > On Fri, Aug 05, 2022 at 09:32:13PM +0200, Arnd Bergmann wrote: > > > > On Fri, Aug 5, 2022 at 8:02 PM Nathan Chancellor > > > > wrote: > > > > > On Fri, Aug 05, 2022 at 06:16:45PM +0200, Arnd Bergmann wrote: > > > > > > On Fri, Aug 5, 2022 at 5:32 PM Harry Wentland > > > > > > wrote: > > > > > > While splitting out sub-functions can help reduce the maximum stack > > > > > > usage, it seems that in this case it makes the actual problem worse: > > > > > > I see 2168 bytes for the combined > > > > > > dml32_ModeSupportAndSystemConfigurationFull(), but marking > > > > > > mode_support_configuration() as noinline gives me 1992 bytes > > > > > > for the outer function plus 384 bytes for the inner one. So it does > > > > > > avoid the warning (barely), but not the problem that the warning > > > > > > tries > > > > > > to point out. > > > > > > > > > > I haven't had a chance to take a look at splitting things up yet, > > > > > would > > > > > you recommend a different approach? > > > > > > > > Splitting up large functions can help when you have large local > > > > variables > > > > that are used in different parts of the function, and the split gets the > > > > compiler to reuse stack locations. > > > > > > > > I think in this particular function, the problem isn't actually local > > > > variables > > > > but either pushing variables on the stack for argument passing, > > > > or something that causes the compiler to run out of registers so it > > > > has to spill registers to the stack. > > > > > > > > In either case, one has to actually look at the generated output > > > > and then try to rearrange the codes so this does not happen. > > > > > > > > One thing to try would be to condense a function call like > > > > > > > > > > > > dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport( > > > > > > > > >dummy_vars.dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport, > > > > mode_lib->vba.USRRetrainingRequiredFinal, > > > > mode_lib->vba.UsesMALLForPStateChange, > > > > > > > > mode_lib->vba.PrefetchModePerState[mode_lib->vba.VoltageLevel][mode_lib->vba.maxMpcComb], > > > > mode_lib->vba.NumberOfActiveSurfaces, > > > > mode_lib->vba.MaxLineBufferLines, > > > > mode_lib->vba.LineBufferSizeFinal, > > > > mode_lib->vba.WritebackInterfaceBufferSize, > > > > mode_lib->vba.DCFCLK, > > > > mode_lib->vba.ReturnBW, > > > > mode_lib->vba.SynchronizeTimingsFinal, > > > > > > > > mode_lib->vba.SynchronizeDRRDisplaysForUCLKPStateChangeFinal, > > > > mode_lib->vba.DRRDisplay, > > > > v->dpte_group_bytes, > > > > v->meta_row_height, > > > > v->meta_row_height_chroma, > > > > > > > > v->dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.mmSOCParameters, > > > > mode_lib->vba.WritebackChunkSize, > > > > mode_lib->vba.SOCCLK, > > > > v->DCFCLKDeepSleep, > > > > mode_lib->vba.DETBufferSizeY, > > > > mode_lib->vba.DETBufferSizeC, > > > > mode_lib->vba.SwathHeightY, > > > > mode_lib->vba.SwathHeightC, > > > >
[PATCH 4/5] drm/amd/display: Reduce number of arguments of dml31's CalculateFlipSchedule()
Most of the arguments are identical between the two call sites and they can be accessed through the 'struct vba_vars_st' pointer. This reduces the total amount of stack space that dml31_ModeSupportAndSystemConfigurationFull() uses by 112 bytes with LLVM 16 (1976 -> 1864), helping clear up the following clang warning: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3908:6: error: stack frame size (2216) exceeds limit (2048) in 'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml31_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. Link: https://github.com/ClangBuiltLinux/linux/issues/1681 Reported-by: "Sudip Mukherjee (Codethink)" Signed-off-by: Nathan Chancellor --- .../dc/dml/dcn31/display_mode_vba_31.c| 172 +- 1 file changed, 47 insertions(+), 125 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c index 21c74ee1deec..60ee936e6436 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c @@ -251,33 +251,13 @@ static void CalculateRowBandwidth( static void CalculateFlipSchedule( struct display_mode_lib *mode_lib, + unsigned int k, double HostVMInefficiencyFactor, double UrgentExtraLatency, double UrgentLatency, - unsigned int GPUVMMaxPageTableLevels, - bool HostVMEnable, - unsigned int HostVMMaxNonCachedPageTableLevels, - bool GPUVMEnable, - double HostVMMinPageSize, double PDEAndMetaPTEBytesPerFrame, double MetaRowBytes, - double DPTEBytesPerRow, - double BandwidthAvailableForImmediateFlip, - unsigned int TotImmediateFlipBytes, - enum source_format_class SourcePixelFormat, - double LineTime, - double VRatio, - double VRatioChroma, - double Tno_bw, - bool DCCEnable, - unsigned int dpte_row_height, - unsigned int meta_row_height, - unsigned int dpte_row_height_chroma, - unsigned int meta_row_height_chroma, - double *DestinationLinesToRequestVMInImmediateFlip, - double *DestinationLinesToRequestRowInImmediateFlip, - double *final_flip_bw, - bool *ImmediateFlipSupportedForPipe); + double DPTEBytesPerRow); static double CalculateWriteBackDelay( enum source_format_class WritebackPixelFormat, double WritebackHRatio, @@ -2868,33 +2848,13 @@ static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerforman for (k = 0; k < v->NumberOfActivePlanes; ++k) { CalculateFlipSchedule( mode_lib, + k, HostVMInefficiencyFactor, v->UrgentExtraLatency, v->UrgentLatency, - v->GPUVMMaxPageTableLevels, - v->HostVMEnable, - v->HostVMMaxNonCachedPageTableLevels, - v->GPUVMEnable, - v->HostVMMinPageSize, v->PDEAndMetaPTEBytesFrame[k], v->MetaRowByte[k], - v->PixelPTEBytesPerRow[k], - v->BandwidthAvailableForImmediateFlip, - v->TotImmediateFlipBytes, - v->SourcePixelFormat[k], - v->HTotal[k] / v->PixelClock[k], - v->VRatio[k], - v->VRatioChroma[k], - v->Tno_bw[k], - v->DCCEnable[k], - v->dpte_row_height[k], - v->meta_row_height[k], - v->dpte_row_height_chroma[k], - v->meta_row_height_chroma[k], - >DestinationLinesToRequestVMInImmediateFlip[k], -
[PATCH 5/5] drm/amd/display: Mark dml30's UseMinimumDCFCLK() as noinline for stack usage
This function consumes a lot of stack space and it blows up the size of dml30_ModeSupportAndSystemConfigurationFull() with clang: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:3542:6: error: stack frame size (2200) exceeds limit (2048) in 'dml30_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml30_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. Commit a0f7e7f759cf ("drm/amd/display: fix i386 frame size warning") aimed to address this for i386 but it did not help x86_64. To reduce the amount of stack space that dml30_ModeSupportAndSystemConfigurationFull() uses, mark UseMinimumDCFCLK() as noinline, using the _for_stack variant for documentation. While this will increase the total amount of stack usage between the two functions (1632 and 1304 bytes respectively), it will make sure both stay below the limit of 2048 bytes for these files. The aforementioned change does help reduce UseMinimumDCFCLK()'s stack usage so it should not be reverted in favor of this change. Link: https://github.com/ClangBuiltLinux/linux/issues/1681 Reported-by: "Sudip Mukherjee (Codethink)" Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c index b7fa003ffe06..6991a68061ef 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c @@ -6497,7 +6497,7 @@ static double CalculateUrgentLatency( return ret; } -static void UseMinimumDCFCLK( +static noinline_for_stack void UseMinimumDCFCLK( struct display_mode_lib *mode_lib, struct vba_vars_st *v, int MaxPrefetchMode, -- 2.37.2
[PATCH 3/5] drm/amd/display: Reduce number of arguments of dml31's CalculateWatermarksAndDRAMSpeedChangeSupport()
Most of the arguments are identical between the two call sites and they can be accessed through the 'struct vba_vars_st' pointer. This reduces the total amount of stack space that dml31_ModeSupportAndSystemConfigurationFull() uses by 240 bytes with LLVM 16 (2216 -> 1976), helping clear up the following clang warning: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3908:6: error: stack frame size (2216) exceeds limit (2048) in 'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml31_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. Link: https://github.com/ClangBuiltLinux/linux/issues/1681 Reported-by: "Sudip Mukherjee (Codethink)" Signed-off-by: Nathan Chancellor --- .../dc/dml/dcn31/display_mode_vba_31.c| 248 -- 1 file changed, 52 insertions(+), 196 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c index d63b4209b14c..21c74ee1deec 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c @@ -311,64 +311,28 @@ static void CalculateVupdateAndDynamicMetadataParameters( static void CalculateWatermarksAndDRAMSpeedChangeSupport( struct display_mode_lib *mode_lib, unsigned int PrefetchMode, - unsigned int NumberOfActivePlanes, - unsigned int MaxLineBufferLines, - unsigned int LineBufferSize, - unsigned int WritebackInterfaceBufferSize, double DCFCLK, double ReturnBW, - bool SynchronizedVBlank, - unsigned int dpte_group_bytes[], - unsigned int MetaChunkSize, double UrgentLatency, double ExtraLatency, - double WritebackLatency, - double WritebackChunkSize, double SOCCLK, - double DRAMClockChangeLatency, - double SRExitTime, - double SREnterPlusExitTime, - double SRExitZ8Time, - double SREnterPlusExitZ8Time, double DCFCLKDeepSleep, unsigned int DETBufferSizeY[], unsigned int DETBufferSizeC[], unsigned int SwathHeightY[], unsigned int SwathHeightC[], - unsigned int LBBitPerPixel[], double SwathWidthY[], double SwathWidthC[], - double HRatio[], - double HRatioChroma[], - unsigned int vtaps[], - unsigned int VTAPsChroma[], - double VRatio[], - double VRatioChroma[], - unsigned int HTotal[], - double PixelClock[], - unsigned int BlendingAndTiming[], unsigned int DPPPerPlane[], double BytePerPixelDETY[], double BytePerPixelDETC[], - double DSTXAfterScaler[], - double DSTYAfterScaler[], - bool WritebackEnable[], - enum source_format_class WritebackPixelFormat[], - double WritebackDestinationWidth[], - double WritebackDestinationHeight[], - double WritebackSourceHeight[], bool UnboundedRequestEnabled, int unsigned CompressedBufferSizeInkByte, enum clock_change_support *DRAMClockChangeSupport, - double *UrgentWatermark, - double *WritebackUrgentWatermark, - double *DRAMClockChangeWatermark, - double *WritebackDRAMClockChangeWatermark, double *StutterExitWatermark, double *StutterEnterPlusExitWatermark, double *Z8StutterExitWatermark, - double *Z8StutterEnterPlusExitWatermark, - double *MinActiveDRAMClockChangeLatencySupported); + double *Z8StutterEnterPlusExitWatermark); static void CalculateDCFCLKDeepSleep( struct display_mode_lib *mode_lib, @@ -3017,64 +2981,28 @@ static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerforman CalculateWatermarksAndDRAMSpeedChangeSupport( mode_lib, PrefetchMode, - v->NumberOfActivePlanes, - v->MaxLineBufferLines, - v->LineBufferSize, - v->WritebackInterfaceBufferSize, v->DCFCLK, v->ReturnBW, - v->SynchronizedVBlank, - v->dpte_group_bytes, - v->MetaChunkSize,
[PATCH 2/5] drm/amd/display: Reduce number of arguments of dml32_CalculatePrefetchSchedule()
Several of the arguments are identical between the two call sites and they can be accessed through the 'struct vba_vars_st' pointer. This reduces the total amount of stack space that dml32_ModeSupportAndSystemConfigurationFull() uses by 208 bytes with LLVM 16 (1936 -> 1728), helping clear up the following clang warning: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1721:6: error: stack frame size (2152) exceeds limit (2048) in 'dml32_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml32_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. Additionally, while modifying the arguments to dml32_CalculatePrefetchSchedule(), use 'v' consistently, instead of 'v' mixed with 'mode_lib->vba'. Link: https://github.com/ClangBuiltLinux/linux/issues/1681 Reported-by: "Sudip Mukherjee (Codethink)" Signed-off-by: Nathan Chancellor --- .../dc/dml/dcn32/display_mode_vba_32.c| 118 +++--- .../dc/dml/dcn32/display_mode_vba_util_32.c | 75 +-- .../dc/dml/dcn32/display_mode_vba_util_32.h | 18 +-- 3 files changed, 78 insertions(+), 133 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c index 7da90fba95fc..58c6cc58583a 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c @@ -755,30 +755,18 @@ static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerforman v->dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.myPipe.BytePerPixelY = v->BytePerPixelY[k]; v->dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.myPipe.BytePerPixelC = v->BytePerPixelC[k]; v->dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.myPipe.ProgressiveToInterlaceUnitInOPP = mode_lib->vba.ProgressiveToInterlaceUnitInOPP; - v->ErrorResult[k] = dml32_CalculatePrefetchSchedule(v->dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.HostVMInefficiencyFactor, - >dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.myPipe, v->DSCDelay[k], - mode_lib->vba.DPPCLKDelaySubtotal + mode_lib->vba.DPPCLKDelayCNVCFormater, - mode_lib->vba.DPPCLKDelaySCL, - mode_lib->vba.DPPCLKDelaySCLLBOnly, - mode_lib->vba.DPPCLKDelayCNVCCursor, - mode_lib->vba.DISPCLKDelaySubtotal, - (unsigned int) (v->SwathWidthY[k] / mode_lib->vba.HRatio[k]), - mode_lib->vba.OutputFormat[k], - mode_lib->vba.MaxInterDCNTileRepeaters, + v->ErrorResult[k] = dml32_CalculatePrefetchSchedule( + v, + k, + v->dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.HostVMInefficiencyFactor, + >dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.myPipe, + v->DSCDelay[k], + (unsigned int) (v->SwathWidthY[k] / v->HRatio[k]), dml_min(v->VStartupLines, v->MaxVStartupLines[k]), v->MaxVStartupLines[k], - mode_lib->vba.GPUVMMaxPageTableLevels, - mode_lib->vba.GPUVMEnable, - mode_lib->vba.HostVMEnable, - mode_lib->vba.HostVMMaxNonCachedPageTableLevels, - mode_lib->vba.HostVMMinPageSize, - mode_lib->vba.DynamicMetadataEnable[k], - mode_lib->vba.DynamicMetadataVMEnabled, - mode_lib->vba.DynamicMetadataLinesBeforeActiveRequired[k], - mode_lib->vba.DynamicMetadataTransmittedBytes[k], v->UrgentLatency, v->UrgentExtraLatency, - mode_lib->vba.TCalc, + v->TCalc, v->PDEAndMetaPTEBytesFrame[k],
[PATCH 0/5] drm/amd/display: Reduce stack usage for clang
Hi all, This series aims to address the following warnings, which are visible when building x86_64 allmodconfig with clang after commit 3876a8b5e241 ("drm/amd/display: Enable building new display engine with KCOV enabled"). drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:3542:6: error: stack frame size (2200) exceeds limit (2048) in 'dml30_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml30_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3908:6: error: stack frame size (2216) exceeds limit (2048) in 'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml31_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1721:6: error: stack frame size (2152) exceeds limit (2048) in 'dml32_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml32_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. This series is based on commit b3235e8635e1 ("drm/amd/display: clean up some inconsistent indentings"). These warnings are fatal for allmodconfig due to CONFIG_WERROR so ideally, I would like to see these patches cherry-picked to a branch targeting mainline to allow our builds to go back to green. However, since this series is not exactly trivial in size, I can understand not wanting to apply these to mainline during the -rc cycle. If they cannot be cherry-picked to mainline, I can add a patch raising the value of -Wframe-larger-than for these files that can be cherry-picked to 6.0/mainline then add a revert of that change as the last patch in the stack so everything goes back to normal for -next/6.1. I am open to other options though! I have built this series against clang 16.0.0 (ToT) and GCC 12.2.0 for x86_64. It has seen no runtime testing, as my only test system with AMD graphics is a Renoir one, which as far as I understand it uses DCN 2.1. Nathan Chancellor (5): drm/amd/display: Reduce number of arguments of dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport() drm/amd/display: Reduce number of arguments of dml32_CalculatePrefetchSchedule() drm/amd/display: Reduce number of arguments of dml31's CalculateWatermarksAndDRAMSpeedChangeSupport() drm/amd/display: Reduce number of arguments of dml31's CalculateFlipSchedule() drm/amd/display: Mark dml30's UseMinimumDCFCLK() as noinline for stack usage .../dc/dml/dcn30/display_mode_vba_30.c| 2 +- .../dc/dml/dcn31/display_mode_vba_31.c| 420 +- .../dc/dml/dcn32/display_mode_vba_32.c| 236 +++--- .../dc/dml/dcn32/display_mode_vba_util_32.c | 323 ++ .../dc/dml/dcn32/display_mode_vba_util_32.h | 51 +-- 5 files changed, 318 insertions(+), 714 deletions(-) base-commit: b3235e8635e1dd7ac1a27a73330e9880dfe05154 -- 2.37.2
[PATCH 1/5] drm/amd/display: Reduce number of arguments of dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport()
Most of the arguments are identical between the two call sites and they can be accessed through the 'struct vba_vars_st' pointer created at the top of dml32_ModeSupportAndSystemConfigurationFull(). This reduces the total amount of stack space that dml32_ModeSupportAndSystemConfigurationFull() uses by 216 bytes with LLVM 16 (2152 -> 1936), helping clear up the following clang warning: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1721:6: error: stack frame size (2152) exceeds limit (2048) in 'dml32_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml32_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. Additionally, while modifying the arguments to dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport(), use 'v' consistently, instead of 'v' mixed with 'mode_lib->vba'. Link: https://github.com/ClangBuiltLinux/linux/issues/1681 Reported-by: "Sudip Mukherjee (Codethink)" Signed-off-by: Nathan Chancellor --- .../dc/dml/dcn32/display_mode_vba_32.c| 118 ++--- .../dc/dml/dcn32/display_mode_vba_util_32.c | 248 -- .../dc/dml/dcn32/display_mode_vba_util_32.h | 33 +-- 3 files changed, 140 insertions(+), 259 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c index d8014bfbc3fe..7da90fba95fc 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c @@ -1163,58 +1163,28 @@ static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerforman v->dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.mmSOCParameters.SMNLatency = mode_lib->vba.SMNLatency; dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport( - mode_lib->vba.USRRetrainingRequiredFinal, - mode_lib->vba.UsesMALLForPStateChange, - mode_lib->vba.PrefetchModePerState[mode_lib->vba.VoltageLevel][mode_lib->vba.maxMpcComb], - mode_lib->vba.NumberOfActiveSurfaces, - mode_lib->vba.MaxLineBufferLines, - mode_lib->vba.LineBufferSizeFinal, - mode_lib->vba.WritebackInterfaceBufferSize, - mode_lib->vba.DCFCLK, - mode_lib->vba.ReturnBW, - mode_lib->vba.SynchronizeTimingsFinal, - mode_lib->vba.SynchronizeDRRDisplaysForUCLKPStateChangeFinal, - mode_lib->vba.DRRDisplay, - v->dpte_group_bytes, - v->meta_row_height, - v->meta_row_height_chroma, + v, + v->PrefetchModePerState[v->VoltageLevel][v->maxMpcComb], + v->DCFCLK, + v->ReturnBW, v->dummy_vars.DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation.mmSOCParameters, - mode_lib->vba.WritebackChunkSize, - mode_lib->vba.SOCCLK, + v->SOCCLK, v->DCFCLKDeepSleep, - mode_lib->vba.DETBufferSizeY, - mode_lib->vba.DETBufferSizeC, - mode_lib->vba.SwathHeightY, - mode_lib->vba.SwathHeightC, - mode_lib->vba.LBBitPerPixel, + v->DETBufferSizeY, + v->DETBufferSizeC, + v->SwathHeightY, + v->SwathHeightC, v->SwathWidthY, v->SwathWidthC, - mode_lib->vba.HRatio, - mode_lib->vba.HRatioChroma, - mode_lib->vba.vtaps, - mode_lib->vba.VTAPsChroma, - mode_lib->vba.VRatio, - mode_lib->vba.VRatioChroma, - mode_lib->vba.HTotal, - mode_lib->vba.VTotal, - mode_lib->vba.VActive, - mode_lib->vba.PixelClock, - mode_lib->vba.BlendingAndTiming, - mode_lib->vba.DPPPerPlane, + v->DPPPerPlane, v->BytePerPixelDETY, v->BytePerPixelDETC, v->DSTXAfterScaler, v->DSTYAfterScaler, - mode_lib->vba.WritebackEnable, - mode_lib->vba.WritebackPixelFormat, - mode_lib->vba.WritebackDestinationWidth, - mode_lib->vba.WritebackDestinationHeight, -
Re: [PATCH linux-next] drm/amdgpu: Remove the unneeded result variable 'r'
Applied. Thanks! Alex On Tue, Aug 30, 2022 at 4:32 AM wrote: > > From: ye xingchen > > Return the value sdma_v4_0_start() directly instead of storing it in > another redundant variable. > > Reported-by: Zeal Robot > Signed-off-by: ye xingchen > --- > drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > index 65181efba50e..0cf9d3b486b2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > @@ -2002,7 +2002,6 @@ static int sdma_v4_0_sw_fini(void *handle) > > static int sdma_v4_0_hw_init(void *handle) > { > - int r; > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > if (adev->flags & AMD_IS_APU) > @@ -2011,9 +2010,7 @@ static int sdma_v4_0_hw_init(void *handle) > if (!amdgpu_sriov_vf(adev)) > sdma_v4_0_init_golden_registers(adev); > > - r = sdma_v4_0_start(adev); > - > - return r; > + return sdma_v4_0_start(adev); > } > > static int sdma_v4_0_hw_fini(void *handle) > -- > 2.25.1
Re: [PATCH 01/15] vfio: Add helpers for unifying vfio_device life cycle
On 8/30/22 11:10 AM, Jason Gunthorpe wrote: On Tue, Aug 30, 2022 at 09:42:42AM -0400, Anthony Krowiak wrote: +/* + * Alloc and initialize vfio_device so it can be registered to vfio + * core. + * + * Drivers should use the wrapper vfio_alloc_device() for allocation. + * @size is the size of the structure to be allocated, including any + * private data used by the driver. It seems the purpose of the wrapper is to ensure that the object being allocated has as its first field a struct vfio_device object and to return its container. Why not just make that a requirement for this function - which I would rename vfio_alloc_device - and document it in the prologue? The caller can then cast the return pointer or use container_of. There are three fairly common patterns for this kind of thing 1) The caller open codes everything: driver_struct = kzalloc() core_init(_struct->core) 2) Some 'get priv' / 'get data' is used instead of container_of(): core_struct = core_alloc(sizeof(*driver_struct)) driver_struct = core_get_priv(core_struct) 3) The allocations and initialization are consolidated in the core, but we continue to use container_of() driver_struct = core_alloc(typeof(*driver_struct)) #1 has a general drawback that people routinely mess up the lifecycle model and get really confused about when to do kfree() vs put(), creating bugs. #2 has a general drawback of not using container_of() at all, and being a bit confusing in some cases #3 has the general drawback of being a bit magical, but solves 1 and 2's problems. I would not fix the struct layout without the BUILD_BUG_ON because someone will accidently change the order and that becomes a subtle runtime error - so at a minimum the wrapper macro has to exist to check that. If you want to allow a dynamic struct layout and avoid the pitfall of exposing the user to kalloc/kfree, then you still need the macro, and it does some more complicated offset stuff. Having the wrapper macro be entirely type safe is appealing and reduces code in the drivers, IMHO. Tell it what type you are initing and get back init'd memory for that type that you always, always free with a put operation. Sounds reasonable, okay I'm buying. Jason
Re: (subset) [PATCH v2 0/7] Devm helpers for regulator get and enable
Quoting Mark Brown (2022-08-15 15:07:35) > On Mon, Aug 15, 2022 at 01:58:55PM -0700, Stephen Boyd wrote: > > > I think the main issue is that platform drivers are being asked to do > > too much. We've put the burden on platform driver authors to intimately > > understand how their devices are integrated, and as we all know they're > > This is for the regulator API, it's mainly for off SoC devices so it's > not a question of understanding the integration of a device into a piece > of silicon, it's a question of understanding the integration of a chip > into a board which seems reasonably in scope for a chip driver and is > certainly the sort of thing that you'd be talking to your customers > about as a silicon vendor. Right. I'm coming from the devm_clk_get_*() APIs angle when saying that platform drivers don't want to know everything. > > > The basic idea is that drivers should be focused on what they're > > driving, not navigating the (sometimes) complex integration that's > > taking place around them. When a device driver probe function is called > > the device should already be powered on. When the driver is > > removed/unbound, the power should be removed after the driver's remove > > function is called. We're only going to be able to solve the power > > sequencing and ordering problem by taking away power control and > > sequencing from drivers. > > That is a sensible approach for most on SoC things but for something > shipped as a separate driver there's little point in separating the > power and clocking domain driver from the device since there's typically > a 1:1 mapping. Usually either it's extremely simple (eg, turn > everything on and remove reset) but some devices really need to manage > things. There's obviously some edge cases in SoC integration as well > (eg, the need to manage card supplies for SD controllers, or knowing > exact clock rates for things like audio controllers) so you need some > flex. I think we're on the same page. The clk API bridges both on SoC and off SoC devices, but leans more towards on SoC devices so I'm coming from that angle. I agree it doesn't make sense to rip out and move power management logic for off SoC devices (your chip driver), because then you get a driver that is split to two places. The hardware engineer for those types of devices has designed the chip to be more aware of the system integration and how their chip is powered, so that it can be easily integrated into various designs without their involvement. This allows it to be used on numerous boards and that's partly the reason why Linux doesn't have board files or board "drivers" because the combinatorial explosion is unmanageable, hence DTS and driver subsystems. The boundary of the combinations ends at the chip which is 1:1 with the platform driver. For on SoC devices, the hardware engineer typically isn't involved in the system integration at all. Instead they hand that task off to the SoC integrator who has to wire everything up (clks, power, resets) and layout the SoC. The combinatorial explosion isn't possible here, because only so many SoCs are ever created and customers can't rewire the internals of the SoC to change which clks go there (although I guess with FPGAs this may be possible). The boundary where the combinations exist is at the device level, not the SoC level, but we've encoded the SoC details into the compatible strings and the drivers to the point that the boundary is pushed to the SoC level. For these on SoC devices, we should extract and consolidate the power management logic away from the drivers, because we're spreading the SoC integration knowledge all around the drivers/ directory for every device class that exists in that SoC. I continue to see drivers that get another clk in the next SoC generation because there was some change to split a clk domain or they get another regulator because they split a power domain. The driver doesn't care, but it has to match a new compatible string and then get the proper list of clks or regulators even though it just wants to turn the thing on and get it running. This gunk needs to go. Runtime PM is a solution to part of the problem, but I think RPM ops should be about poking device registers for these on SoC devices, not about controlling yet another clk or regulator that got wired up this SoC generation. Probably we need to get away from having platform driver probe for on SoC devices get resources like clks, regulators, and interconnects at all. Instead, those should be managed by some "SoC" driver that knows the integration of the SoC and can make sure the proper power sequencing is followed. Hopefully we can do this largely via genpd and RPM, with a little bit of help from some SoC driver that registers genpds for devices under /soc. Of course there are exact clk frequencies required sometimes (audio rates, display link rates, serial baud rates, etc.) but those sorts of things could use a higher level of abstraction so
Re: [PATCH v2 06/41] drm/connector: Rename legacy TV property
Den 29.08.2022 15.11, skrev Maxime Ripard: > The current tv_mode has driver-specific values that don't allow to > > easily share code using it, either at the userspace or kernel level. > > > > Since we're going to introduce a new, generic, property that fit the > > same purpose, let's rename this one to legacy_tv_mode to make it > > obvious we should move away from it. > > > > Signed-off-by: Maxime Ripard > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > index 1d5e3cccb9e3..5cfad8b6ad83 100644 > > --- a/include/drm/drm_connector.h > > +++ b/include/drm/drm_connector.h > > @@ -695,7 +695,7 @@ struct drm_connector_tv_margins { > > * @select_subconnector: selected subconnector > > * @subconnector: detected subconnector > > * @margins: TV margins > > - * @mode: TV mode > > + * @legacy_mode: Legacy TV mode, driver specific value > > * @brightness: brightness in percent > > * @contrast: contrast in percent > > * @flicker_reduction: flicker reduction in percent > > @@ -707,7 +707,7 @@ struct drm_tv_connector_state { > > enum drm_mode_subconnector select_subconnector; > > enum drm_mode_subconnector subconnector; > > struct drm_connector_tv_margins margins; > > - unsigned int mode; > > + unsigned int legacy_mode; I suggest you do a build of the affected drivers after adding this patch to make sure you have changed all mode -> legacy_mode occurrences _before_ adding back mode in a later patch. A simple grep gave me these: drivers/gpu/drm/vc4/vc4_vec.c: vc4_vec_tv_modes[state->tv.mode].mode); drivers/gpu/drm/vc4/vc4_vec.c: vec->tv_mode = _vec_tv_modes[conn_state->tv.mode]; drivers/gpu/drm/vc4/vc4_vec.c: vec_mode = _vec_tv_modes[conn_state->tv.mode]; drivers/gpu/drm/i915/display/intel_tv.c:int format = conn_state->tv.mode; drivers/gpu/drm/i915/display/intel_tv.c: connector->state->tv.mode = i; drivers/gpu/drm/i915/display/intel_tv.c:if (old_state->tv.mode != new_state->tv.mode || drivers/gpu/drm/i915/display/intel_tv.c:state->tv.mode = initial_mode; drivers/gpu/drm/i915/display/intel_tv.c: state->tv.mode); drivers/gpu/drm/i915/display/intel_sdvo.c: format_map = 1 << conn_state->tv.mode; drivers/gpu/drm/i915/display/intel_sdvo.c: format_map = 1 << conn_state->tv.mode; drivers/gpu/drm/i915/display/intel_sdvo.c: if (state->tv.mode == intel_sdvo_connector->tv_format_supported[i]) { drivers/gpu/drm/i915/display/intel_sdvo.c: state->tv.mode = intel_sdvo_connector->tv_format_supported[val]; drivers/gpu/drm/i915/display/intel_sdvo.c: intel_sdvo_connector->base.base.state->tv.mode = intel_sdvo_connector->tv_format_supported[0]; Not so easy to grep for is this in gud: static unsigned int *gud_connector_tv_state_val(u16 prop, struct drm_tv_connector_state *state) { switch (prop) { ... case GUD_PROPERTY_TV_MODE: return >mode; Noralf. > > unsigned int brightness; > > unsigned int contrast; > > unsigned int flicker_reduction; > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > > index 6b5e01295348..35a827175c24 100644 > > --- a/include/drm/drm_mode_config.h > > +++ b/include/drm/drm_mode_config.h > > @@ -714,11 +714,13 @@ struct drm_mode_config { > >* between different TV connector types. > >*/ > > struct drm_property *tv_select_subconnector_property; > > + > > /** > > - * @tv_mode_property: Optional TV property to select > > + * @legacy_tv_mode_property: Optional TV property to select > >* the output TV mode. > >*/ > > - struct drm_property *tv_mode_property; > > + struct drm_property *legacy_tv_mode_property; > > + > > /** > >* @tv_left_margin_property: Optional TV property to set the left > >* margin (expressed in pixels). > > >
Re: [PATCH v2 32/41] drm/vc4: vec: Convert to the new TV mode property
Den 29.08.2022 15.11, skrev Maxime Ripard: > Now that the core can deal fine with analog TV modes, let's convert the vc4 > > VEC driver to leverage those new features. > > > > We've added some backward compatibility to support the old TV mode property > > and translate it into the new TV norm property. > > > > Signed-off-by: Maxime Ripard > > > > diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c > > index ba6f81908923..58286acf4b9e 100644 > > --- a/drivers/gpu/drm/vc4/vc4_vec.c > > +++ b/drivers/gpu/drm/vc4/vc4_vec.c > @@ -192,7 +200,7 @@ enum vc4_vec_tv_mode_id { > > }; > > > > struct vc4_vec_tv_mode { > > - const struct drm_display_mode *mode; > > + unsigned int mode; > > u32 config0; > > u32 config1; > > u32 custom_freq; > > @@ -226,28 +234,50 @@ static const struct debugfs_reg32 vec_regs[] = { > > }; > > > > static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = { > > - [VC4_VEC_TV_MODE_NTSC] = { > > - .mode = _mode_480i, > > + { > > + .mode = DRM_MODE_TV_MODE_NTSC_M, > > .config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN, > > .config1 = VEC_CONFIG1_C_CVBS_CVBS, > > }, > > - [VC4_VEC_TV_MODE_NTSC_J] = { > > - .mode = _mode_480i, > > + { > > + .mode = DRM_MODE_TV_MODE_NTSC_J, > > .config0 = VEC_CONFIG0_NTSC_STD, > > .config1 = VEC_CONFIG1_C_CVBS_CVBS, > > }, > > - [VC4_VEC_TV_MODE_PAL] = { > > - .mode = _mode_576i, > > + { > > + .mode = DRM_MODE_TV_MODE_PAL_B, > > .config0 = VEC_CONFIG0_PAL_BDGHI_STD, > > .config1 = VEC_CONFIG1_C_CVBS_CVBS, > > }, > > - [VC4_VEC_TV_MODE_PAL_M] = { > > - .mode = _mode_480i, > > + { > > + .mode = DRM_MODE_TV_MODE_PAL_M, > > .config0 = VEC_CONFIG0_PAL_M_STD, > > .config1 = VEC_CONFIG1_C_CVBS_CVBS, > > }, > > }; > > > > +static inline const struct vc4_vec_tv_mode * > > +vc4_vec_tv_mode_lookup(unsigned int mode) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < ARRAY_SIZE(vc4_vec_tv_modes); i++) { > > + const struct vc4_vec_tv_mode *tv_mode = _vec_tv_modes[i]; > > + > > + if (tv_mode->mode == mode) > > + return tv_mode; > > + } > > + > > + return NULL; > > +} > > + > > +static const struct drm_prop_enum_list tv_mode_names[] = { Maybe call it legacy_tv_mode_enums? > > + { VC4_VEC_TV_MODE_NTSC, "NTSC", }, > > + { VC4_VEC_TV_MODE_NTSC_J, "NTSC-J", }, > > + { VC4_VEC_TV_MODE_PAL, "PAL", }, > > + { VC4_VEC_TV_MODE_PAL_M, "PAL-M", }, If you use DRM_MODE_TV_MODE_* here you don't need to translate the value using the switch statement in get/set property, you can use the value directly to get/set tv.mode. Noralf. > > +};
Re: [PATCH v2 31/41] drm/vc4: vec: Use TV Reset implementation
Den 29.08.2022 15.11, skrev Maxime Ripard: > The analog TV properties created by the drm_mode_create_tv_properties() are > > not properly initialised at reset. Let's switch our implementation to call > > drm_atomic_helper_connector_tv_reset(). > > > > Signed-off-by: Maxime Ripard > Reviewed-by: Noralf Trønnes
Re: [PATCH v2 23/41] drm/atomic-helper: Add an analog TV atomic_check implementation
Den 29.08.2022 15.11, skrev Maxime Ripard: > The analog TV connector drivers share some atomic_check logic, and the new > > TV standard property have created a bunch of new constraints that needs to > > be shared across drivers too. > > > > Let's create an atomic_check helper for those use cases. > > > > Signed-off-by: Maxime Ripard > > > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c > b/drivers/gpu/drm/drm_atomic_state_helper.c > > index 0373c3dc824b..d64733c6aae3 100644 > > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > > @@ -556,6 +556,42 @@ void drm_atomic_helper_connector_tv_reset(struct > drm_connector *connector) > > } > > EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset); > > > > +/** > > + * @drm_atomic_helper_connector_tv_check: Validate an analog TV connector > state > > + * @connector: DRM Connector > > + * @state: the DRM State object > > + * > > + * Checks the state object to see if the requested state is valid for an > > + * analog TV connector. > > + * > > + * Returns: > > + * Zero for success, a negative error code on error. > > + */ > > +int drm_atomic_helper_connector_tv_check(struct drm_connector *connector, > > + struct drm_atomic_state *state) > > +{ > > + struct drm_connector_state *old_conn_state = > > + drm_atomic_get_old_connector_state(state, connector); > > + struct drm_connector_state *new_conn_state = > > + drm_atomic_get_new_connector_state(state, connector); > > + struct drm_crtc_state *crtc_state; > > + struct drm_crtc *crtc; > > + > > + crtc = new_conn_state->crtc; > > + if (!crtc) > > + return 0; > > + > > + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > > + if (!crtc_state) > > + return -EINVAL; > > + > > + if (old_conn_state->tv.mode != new_conn_state->tv.mode) > > + crtc_state->mode_changed = true; > If you can expand this check then I can use it in gud: if (old_conn_state->tv.margins.left != new_conn_state->tv.margins.left || old_conn_state->tv.margins.right != new_conn_state->tv.margins.right || old_conn_state->tv.margins.top != new_conn_state->tv.margins.top || old_conn_state->tv.margins.bottom != new_conn_state->tv.margins.bottom || old_conn_state->tv.mode != new_conn_state->tv.mode || old_conn_state->tv.brightness != new_conn_state->tv.brightness || old_conn_state->tv.contrast != new_conn_state->tv.contrast || old_conn_state->tv.flicker_reduction != new_conn_state->tv.flicker_reduction || old_conn_state->tv.overscan != new_conn_state->tv.overscan || old_conn_state->tv.saturation != new_conn_state->tv.saturation || old_conn_state->tv.hue != new_conn_state->tv.hue) crtc_state->connectors_changed = true; With that considered: Reviewed-by: Noralf Trønnes > + > > + return 0; > > +} > > +EXPORT_SYMBOL(drm_atomic_helper_connector_tv_check); > > + > > /** > > * __drm_atomic_helper_connector_duplicate_state - copy atomic connector > state > > * @connector: connector object > > diff --git a/include/drm/drm_atomic_state_helper.h > b/include/drm/drm_atomic_state_helper.h > > index c8fbce795ee7..b9740edb2658 100644 > > --- a/include/drm/drm_atomic_state_helper.h > > +++ b/include/drm/drm_atomic_state_helper.h > > @@ -26,6 +26,7 @@ > > > > #include > > > > +struct drm_atomic_state; > > struct drm_bridge; > > struct drm_bridge_state; > > struct drm_crtc; > > @@ -71,6 +72,8 @@ void __drm_atomic_helper_connector_reset(struct > drm_connector *connector, > >struct drm_connector_state > *conn_state); > > void drm_atomic_helper_connector_reset(struct drm_connector *connector); > > void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector); > > +int drm_atomic_helper_connector_tv_check(struct drm_connector *connector, > > + struct drm_atomic_state *state); > > void drm_atomic_helper_connector_tv_margins_reset(struct drm_connector > *connector); > > void > > __drm_atomic_helper_connector_duplicate_state(struct drm_connector > *connector, > > >
Re: [PATCH v2 22/41] drm/atomic-helper: Add a TV properties reset helper
Den 29.08.2022 15.11, skrev Maxime Ripard: > The drm_tv_create_properties() function will create a bunch of properties, > > but it's up to each and every driver using that function to properly reset > > the state of these properties leading to inconsistent behaviours. > > > > Let's create a helper that will take care of it. > > > > Signed-off-by: Maxime Ripard > Reviewed-by: Noralf Trønnes
Re: [PATCH v2 29/41] drm/vc4: vec: Switch for common modes
Den 29.08.2022 15.11, skrev Maxime Ripard: > Now that the core has a definition for the 525 and 625 lines analog TV > > modes, let's switch to it for vc4. > > > > Signed-off-by: Maxime Ripard > > > > diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c > > index d1d40b69279e..63e4e617e321 100644 > > --- a/drivers/gpu/drm/vc4/vc4_vec.c > > +++ b/drivers/gpu/drm/vc4/vc4_vec.c > > @@ -224,38 +224,24 @@ static const struct debugfs_reg32 vec_regs[] = { > > VC4_REG32(VEC_DAC_MISC), > > }; > > > > -static const struct drm_display_mode ntsc_mode = { > > - DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 13500, > > - 720, 720 + 14, 720 + 14 + 64, 720 + 14 + 64 + 60, 0, > > - 480, 480 + 7, 480 + 7 + 6, 525, 0, > > - DRM_MODE_FLAG_INTERLACE) > > -}; > > - > > -static const struct drm_display_mode pal_mode = { > > - DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 13500, > > - 720, 720 + 20, 720 + 20 + 64, 720 + 20 + 64 + 60, 0, > > - 576, 576 + 4, 576 + 4 + 6, 625, 0, > > - DRM_MODE_FLAG_INTERLACE) > > -}; > > - > > static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = { > > [VC4_VEC_TV_MODE_NTSC] = { > > - .mode = _mode, > > + .mode = _mode_480i, > I can't find drm_mode_480i anywhere, maybe the compiler doesn't complain since you remove the reference in a later patch? Noralf. > .config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN, > > .config1 = VEC_CONFIG1_C_CVBS_CVBS, > > }, > > [VC4_VEC_TV_MODE_NTSC_J] = { > > - .mode = _mode, > > + .mode = _mode_480i, > > .config0 = VEC_CONFIG0_NTSC_STD, > > .config1 = VEC_CONFIG1_C_CVBS_CVBS, > > }, > > [VC4_VEC_TV_MODE_PAL] = { > > - .mode = _mode, > > + .mode = _mode_576i, > > .config0 = VEC_CONFIG0_PAL_BDGHI_STD, > > .config1 = VEC_CONFIG1_C_CVBS_CVBS, > > }, > > [VC4_VEC_TV_MODE_PAL_M] = { > > - .mode = _mode, > > + .mode = _mode_576i, > > .config0 = VEC_CONFIG0_PAL_BDGHI_STD, > > .config1 = VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > > .custom_freq = 0x223b61d1, > > >
Re: [PATCH 3/3] drm: omapdrm: Do no allocate non-scanout GEMs through DMM/TILER
On 30/08/2022 21:08, Ivaylo Dimitrov wrote: flags &= ~OMAP_BO_TILED_MASK; flags |= 0x0008; flags |= OMAP_BO_WC; bo = omap_bo_new(dev, size, flags); As you can see we use 0x0008 (OMAP_BO_MEM_CONTIG) unconditionally. This was a hack added since even non-scanout buffers sometimes need to be contiguous (video decoder surfaces), but we had no way back Hmm, why would video decoder need linear memory? No MMU? Not sure about this case, but many/most IPs don't have MMU. E.g. CSI-2 or parallel capture. If you tell me what the code should look like, I can rebuild the lib and post a copy. Long term, I'd like to start using DMA-BUF Heaps for CMA memory allocations in gralloc and elsewhere, then drop out the DMM/TILER support from OMAPDRM, since it never really belonged there in the first place (being a IOMMU unrelated to the display/GPU). Umm, how will we rotate scanout buffers then? Didn't we discuss this earlier in this thread. Or some other thread. Related to VRFB... I'm not sure =). Anyway, neither VRFB nor DMM/TILER are part of the DSS. They're part of the memory subsystem. They can be used without DSS being in the setup. Thus the code for VRFB and DMM/TILER should not be in the DSS driver. The DSS driver should still, of course, support DMM/TILER (and maybe VRFB some day) in the "use" sense, i.e. so that DSS can use the DMM/TILER provided from another driver. But how exactly that's to be implemented, I don't know. Tomi
Re: [PATCH v2 28/41] drm/vc4: vec: Fix timings for VEC modes
Den 29.08.2022 15.11, skrev Maxime Ripard: > From: Mateusz Kwiatkowski > > > > This commit fixes vertical timings of the VEC (composite output) modes > > to accurately represent the 525-line ("NTSC") and 625-line ("PAL") ITU-R > > standards. > > > > Previous timings were actually defined as 502 and 601 lines, resulting > > in non-standard 62.69 Hz and 52 Hz signals being generated, > > respectively. > > > > Signed-off-by: Mateusz Kwiatkowski > > Signed-off-by: Maxime Ripard > > Acked-by: Noralf Trønnes
Re: [RFC PATCH v3 04/17] drm/i915: Implement bind and unbind of object
On 27/08/2022 20:43, Andi Shyti wrote: From: Niranjana Vishwanathapura Implement the bind and unbind of an object at the specified GPU virtual addresses. Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Prathap Kumar Valsan Signed-off-by: Ramalingam C Signed-off-by: Andi Shyti --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 21 ++ .../drm/i915/gem/i915_gem_vm_bind_object.c| 322 ++ drivers/gpu/drm/i915/gt/intel_gtt.c | 10 + drivers/gpu/drm/i915/gt/intel_gtt.h | 9 + drivers/gpu/drm/i915/i915_driver.c| 1 + drivers/gpu/drm/i915/i915_vma.c | 3 +- drivers/gpu/drm/i915/i915_vma.h | 2 - drivers/gpu/drm/i915/i915_vma_types.h | 14 + include/uapi/drm/i915_drm.h | 163 + 10 files changed, 543 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 522ef9b4aff32..4e1627e96c6e0 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -165,6 +165,7 @@ gem-y += \ gem/i915_gem_ttm_move.o \ gem/i915_gem_ttm_pm.o \ gem/i915_gem_userptr.o \ + gem/i915_gem_vm_bind_object.o \ gem/i915_gem_wait.o \ gem/i915_gemfs.o i915-y += \ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h new file mode 100644 index 0..ebc493b7dafc1 --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2022 Intel Corporation + */ + +#ifndef __I915_GEM_VM_BIND_H +#define __I915_GEM_VM_BIND_H + +#include "i915_drv.h" + +struct i915_vma * +i915_gem_vm_bind_lookup_vma(struct i915_address_space *vm, u64 va); +void i915_gem_vm_bind_remove(struct i915_vma *vma, bool release_obj); + +int i915_gem_vm_bind_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); +int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data, +struct drm_file *file); + +void i915_gem_vm_unbind_vma_all(struct i915_address_space *vm); +#endif /* __I915_GEM_VM_BIND_H */ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c new file mode 100644 index 0..dadd1d4b1761b --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c @@ -0,0 +1,322 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2022 Intel Corporation + */ + +#include + +#include "gem/i915_gem_vm_bind.h" +#include "gem/i915_gem_context.h" +#include "gt/gen8_engine_cs.h" + +#include "i915_drv.h" +#include "i915_gem_gtt.h" + +#define START(node) ((node)->start) +#define LAST(node) ((node)->last) + +INTERVAL_TREE_DEFINE(struct i915_vma, rb, u64, __subtree_last, +START, LAST, static inline, i915_vm_bind_it) + +#undef START +#undef LAST + +/** + * DOC: VM_BIND/UNBIND ioctls + * + * DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer + * objects (BOs) or sections of a BOs at specified GPU virtual addresses on a + * specified address space (VM). Multiple mappings can map to the same physical + * pages of an object (aliasing). These mappings (also referred to as persistent + * mappings) will be persistent across multiple GPU submissions (execbuf calls) + * issued by the UMD, without user having to provide a list of all required + * mappings during each submission (as required by older execbuf mode). + * + * The VM_BIND/UNBIND calls allow UMDs to request a timeline out fence for + * signaling the completion of bind/unbind operation. + * + * VM_BIND feature is advertised to user via I915_PARAM_VM_BIND_VERSION. + * User has to opt-in for VM_BIND mode of binding for an address space (VM) + * during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension. + * + * VM_BIND/UNBIND ioctl calls executed on different CPU threads concurrently + * are not ordered. Furthermore, parts of the VM_BIND/UNBIND operations can be + * done asynchronously, when valid out fence is specified. + * + * VM_BIND locking order is as below. + * + * 1) vm_bind_lock mutex will protect vm_bind lists. This lock is taken in + *vm_bind/vm_unbind ioctl calls, in the execbuf path and while releasing the + *mapping. + * + *In future, when GPU page faults are supported, we can potentially use a + *rwsem instead, so that multiple page fault handlers can take the read + *side lock to lookup the mapping and hence can run in parallel. + *The older execbuf mode of binding do not need this lock. + * + * 2) The object's dma-resv lock will protect i915_vma state and needs + *to be held while binding/unbinding a vma in the async
Re: [PATCH 3/3] drm: omapdrm: Do no allocate non-scanout GEMs through DMM/TILER
Hi, On 30.08.22 г. 18:08 ч., Yongqin Liu wrote: HI, Andrew Thanks a lot for the information! And great to have you here! Hi, Ivaylo With the code provided by Andrew, could you please help give suggestions on how to modify it in the gralloc lib side? to add the OMAP_BO_SCANOUT flag unconditionally as OMAP_BO_MEM_CONTIG? I don't think adding OMAP_BO_SCANOUT unconditionally is a good idea - we already agreed on why. Without having access to the whole source code, I would not make blind suggestions and would leave between you (as user) and Andrew (as a provider) to agree on what is the best way to fix the issue. Still, see the comments bellow. Thanks, Yongqin Liu On Mon, 29 Aug 2022 at 22:36, Andrew Davis wrote: On 8/29/22 8:24 AM, Ivaylo Dimitrov wrote: Hi, On 29.08.22 г. 5:51 ч., Yongqin Liu wrote: Hi, Ivaylo Sorry for the late response, and Thanks very much for the detailed explanations! On Thu, 18 Aug 2022 at 18:23, Ivaylo Dimitrov wrote: Hi, On 17.08.22 г. 7:52 ч., Yongqin Liu wrote: Hi, Ivaylo On Mon, 15 Aug 2022 at 14:23, Ivaylo Dimitrov wrote: Hi Liu, On 14.08.22 г. 17:27 ч., Yongqin Liu wrote: Hi, IvayIo Thanks very much for the reply! On Sat, 13 Aug 2022 at 14:58, Ivaylo Dimitrov wrote: Hi Liu, On 12.08.22 г. 7:35 ч., Yongqin Liu wrote: Hi, Ivaylo, Tomi We have one X15 Android AOSP master build, it could not have the home screen displayed on the hdmi monitor connected with this change, with the following message printed on the serial console [ 607.404205] omapdrm omapdrm.0: Failed to setup plane plane-0 [ 607.410522] omapdrm omapdrm.0: Failed to setup plane plane-1 [ 607.416381] omapdrm omapdrm.0: Failed to setup plane plane-2 [ 607.422088] omapdrm omapdrm.0: Failed to setup plane plane-3 # for details, please check the link here: http://ix.io/47m1 It will work with home screen displayed on the hdmi monitor if this change is reverted. Is this the broken problem you talked about here? And could you please give some suggestions on how to have the x15 Android build work with this change? Make sure scanout (i.e. those to be displayed) buffers are actually allocated as such - OMAP_BO_SCANOUT flag must be set when calling omap_bo_new(). I am not familiar with this area, I am sorry if I asked quite silly questions:( I googled omap_bo_new, and found it's a function of libdrm here[1], is it what you meant here? Yes, calling this function from userspace ends in kernel code the $subject patch is part of. If it's the omap_bo_new that we should pass OMAP_BO_SCANOUT when it is called, then is it the correct way to update omap_bo_new to add the OMAP_BO_SCANOUT flag before it calls omap_bo_new_impl? omap_bo_new() is fine and does not need any updates/fixes, it is the code that uses it (whoever it is, I am not familiar with the userspace you are using) that shall pass correct flags (third parameter) when calling it. Sorry, I do not get the point here. Like you said, the code that calls omap_bo_new needs to pass OMAP_BO_SCANOUT, then IMO omap_bo_new should be the best place to add the OMAP_BO_SCANOUT flag, (like via flags = flags | OMAP_BO_SCANOUT), that could help avoid missing the flag by some code, and also avoids hacks/changes on the possible blob binaries. Do I misunderstand somewhere? Or is there some case that OMAP_BO_SCANOUT shouldn't be passed when omap_bo_new is called? Exactly. You need to pass OMAP_BO_SCANOUT only when you want your buffers to be 'scanout' buffers(i.e. buffers that can be displayed on screen), which is not always the case - there is no need offscreen buffers or pixmaps to be scanout capable, for example. There are more cases like that. The problem is that scanout buffer on OMAP4 allocate additional resources in DMM/TILER (a piece of hardware) and those resources are limited. Not only that, but DMM/TILER memory space eventually gets fragmented over time (if you have lots of allocataoins/deallocations) and you will start getting ENOMEM (or similar) errors. Ofc, in your particular use case you may never hit such issues. Thanks, I understand the cases now. BTW you shall really find who and how uses OMAP BO API, in theory it might use ioctls directly and not call omap_bo_xxx functions. Do you mean the DRM_OMAP_GEM_NEW ioctl api? There is no place in the AOSP tree to call that except the omap_bo_new_impl function, which is called by the omap_bo_new and omap_bo_new_tiled functions. The omap_bo_new should not be called with the OMAP_BO_TILED flag, while the omap_bo_new_tiled should be called with the OMAP_BO_TILED flag Regarding to the omap_bo_new function, there are 2 places call it in the AOSP tree: #1 ./external/libkmsxx/kms++/src/omap/omapframebuffer.cpp #2 ./device/ti/beagle_x15/gpu/gralloc.am57x.so #1 seems not used in AOSP yet, and #2 is one blob binary we do not have the source for. I would bet on gralloc.am57x.so. yeah, that's my guess as well. strace
Re: [RFC PATCH v3 04/17] drm/i915: Implement bind and unbind of object
On 27/08/2022 20:43, Andi Shyti wrote: From: Niranjana Vishwanathapura Implement the bind and unbind of an object at the specified GPU virtual addresses. Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Prathap Kumar Valsan Signed-off-by: Ramalingam C Signed-off-by: Andi Shyti --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 21 ++ .../drm/i915/gem/i915_gem_vm_bind_object.c| 322 ++ drivers/gpu/drm/i915/gt/intel_gtt.c | 10 + drivers/gpu/drm/i915/gt/intel_gtt.h | 9 + drivers/gpu/drm/i915/i915_driver.c| 1 + drivers/gpu/drm/i915/i915_vma.c | 3 +- drivers/gpu/drm/i915/i915_vma.h | 2 - drivers/gpu/drm/i915/i915_vma_types.h | 14 + include/uapi/drm/i915_drm.h | 163 + 10 files changed, 543 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 522ef9b4aff32..4e1627e96c6e0 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -165,6 +165,7 @@ gem-y += \ gem/i915_gem_ttm_move.o \ gem/i915_gem_ttm_pm.o \ gem/i915_gem_userptr.o \ + gem/i915_gem_vm_bind_object.o \ gem/i915_gem_wait.o \ gem/i915_gemfs.o i915-y += \ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h new file mode 100644 index 0..ebc493b7dafc1 --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2022 Intel Corporation + */ + +#ifndef __I915_GEM_VM_BIND_H +#define __I915_GEM_VM_BIND_H + +#include "i915_drv.h" + +struct i915_vma * +i915_gem_vm_bind_lookup_vma(struct i915_address_space *vm, u64 va); +void i915_gem_vm_bind_remove(struct i915_vma *vma, bool release_obj); + +int i915_gem_vm_bind_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); +int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data, +struct drm_file *file); + +void i915_gem_vm_unbind_vma_all(struct i915_address_space *vm); +#endif /* __I915_GEM_VM_BIND_H */ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c new file mode 100644 index 0..dadd1d4b1761b --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c @@ -0,0 +1,322 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2022 Intel Corporation + */ + +#include + +#include "gem/i915_gem_vm_bind.h" +#include "gem/i915_gem_context.h" +#include "gt/gen8_engine_cs.h" + +#include "i915_drv.h" +#include "i915_gem_gtt.h" + +#define START(node) ((node)->start) +#define LAST(node) ((node)->last) + +INTERVAL_TREE_DEFINE(struct i915_vma, rb, u64, __subtree_last, +START, LAST, static inline, i915_vm_bind_it) + +#undef START +#undef LAST + +/** + * DOC: VM_BIND/UNBIND ioctls + * + * DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer + * objects (BOs) or sections of a BOs at specified GPU virtual addresses on a + * specified address space (VM). Multiple mappings can map to the same physical + * pages of an object (aliasing). These mappings (also referred to as persistent + * mappings) will be persistent across multiple GPU submissions (execbuf calls) + * issued by the UMD, without user having to provide a list of all required + * mappings during each submission (as required by older execbuf mode). + * + * The VM_BIND/UNBIND calls allow UMDs to request a timeline out fence for + * signaling the completion of bind/unbind operation. + * + * VM_BIND feature is advertised to user via I915_PARAM_VM_BIND_VERSION. + * User has to opt-in for VM_BIND mode of binding for an address space (VM) + * during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension. + * + * VM_BIND/UNBIND ioctl calls executed on different CPU threads concurrently + * are not ordered. Furthermore, parts of the VM_BIND/UNBIND operations can be + * done asynchronously, when valid out fence is specified. + * + * VM_BIND locking order is as below. + * + * 1) vm_bind_lock mutex will protect vm_bind lists. This lock is taken in + *vm_bind/vm_unbind ioctl calls, in the execbuf path and while releasing the + *mapping. + * + *In future, when GPU page faults are supported, we can potentially use a + *rwsem instead, so that multiple page fault handlers can take the read + *side lock to lookup the mapping and hence can run in parallel. + *The older execbuf mode of binding do not need this lock. + * + * 2) The object's dma-resv lock will protect i915_vma state and needs + *to be held while binding/unbinding a vma in the async
[PATCH v2 4/6] drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits
If the driver supports it, allow user-space to supply the DRM_MODE_PAGE_FLIP_ASYNC flag to request an async page-flip. Set drm_crtc_state.async_flip accordingly. Document that drivers will reject atomic commits if an async flip isn't possible. This allows user-space to fall back to something else. For instance, Xorg falls back to a blit. Another option is to wait as close to the next vblank as possible before performing the page-flip to reduce latency. v2: document new uAPI Signed-off-by: Simon Ser Co-developed-by: André Almeida Signed-off-by: André Almeida Cc: Daniel Vetter Cc: Joshua Ashton Cc: Melissa Wen Cc: Alex Deucher Cc: Harry Wentland Cc: Nicholas Kazlauskas Cc: Ville Syrjälä --- drivers/gpu/drm/drm_atomic_uapi.c | 28 +--- include/uapi/drm/drm_mode.h | 4 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 79730fa1dd8e..ee24ed7e2edb 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1278,6 +1278,18 @@ static void complete_signaling(struct drm_device *dev, kfree(fence_state); } +static void +set_async_flip(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + crtc_state->async_flip = true; + } +} + int drm_mode_atomic_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -1318,9 +1330,16 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, } if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) { - drm_dbg_atomic(dev, - "commit failed: invalid flag DRM_MODE_PAGE_FLIP_ASYNC\n"); - return -EINVAL; + if (!dev->mode_config.async_page_flip) { + drm_dbg_atomic(dev, + "commit failed: DRM_MODE_PAGE_FLIP_ASYNC not supported\n"); + return -EINVAL; + } + if (dev->mode_config.atomic_async_page_flip_not_supported) { + drm_dbg_atomic(dev, + "commit failed: DRM_MODE_PAGE_FLIP_ASYNC not supported with atomic\n"); + return -EINVAL; + } } /* can't test and expect an event at the same time. */ @@ -1418,6 +1437,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, if (ret) goto out; + if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) + set_async_flip(state); + if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) { ret = drm_atomic_check_only(state); } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 86a292c3185a..cce1a1bea645 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -942,6 +942,10 @@ struct hdr_output_metadata { * Request that the page-flip is performed as soon as possible, ie. with no * delay due to waiting for vblank. This may cause tearing to be visible on * the screen. + * + * When used with atomic uAPI, the driver will return an error if the hardware + * doesn't support performing an asynchronous page-flip for this update. + * User-space should handle this, e.g. by falling back to a regular page-flip. */ #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4 -- 2.37.2
[PATCH v2 6/6] amd/display: indicate support for atomic async page-flips on DC
amdgpu_dm_commit_planes() already sets the flip_immediate flag for async page-flips. This flag is used to set the UNP_FLIP_CONTROL register. Thus, no additional change is required to handle async page-flips with the atomic uAPI. v2: make it clear this commit is about DC and not only DCN Signed-off-by: Simon Ser Cc: Joshua Ashton Cc: Melissa Wen Cc: Alex Deucher Cc: Harry Wentland Cc: Nicholas Kazlauskas Cc: André Almeida --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index c9195e7bb649..7f96d81f4e0d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3804,7 +3804,6 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev) adev_to_drm(adev)->mode_config.prefer_shadow = 0; /* indicates support for immediate flip */ adev_to_drm(adev)->mode_config.async_page_flip = true; - adev_to_drm(adev)->mode_config.atomic_async_page_flip_not_supported = true; adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; -- 2.37.2
[PATCH v2 5/6] drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
This new kernel capability indicates whether async page-flips are supported via the atomic uAPI. DRM clients can use it to check for support before feeding DRM_MODE_PAGE_FLIP_ASYNC to the kernel. Make it clear that DRM_CAP_ASYNC_PAGE_FLIP is for legacy uAPI only. Signed-off-by: Simon Ser Cc: Daniel Vetter Cc: Joshua Ashton Cc: Melissa Wen Cc: Alex Deucher Cc: Harry Wentland Cc: Nicholas Kazlauskas Cc: André Almeida Cc: Ville Syrjälä --- drivers/gpu/drm/drm_ioctl.c | 5 + include/uapi/drm/drm.h | 10 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index ca2a6e6101dc..5b1591e2b46c 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -302,6 +302,11 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ case DRM_CAP_CRTC_IN_VBLANK_EVENT: req->value = 1; break; + case DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP: + req->value = drm_core_check_feature(dev, DRIVER_ATOMIC) && +dev->mode_config.async_page_flip && + !dev->mode_config.atomic_async_page_flip_not_supported; + break; default: return -EINVAL; } diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 642808520d92..b1962628ecda 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -706,7 +706,8 @@ struct drm_gem_open { /** * DRM_CAP_ASYNC_PAGE_FLIP * - * If set to 1, the driver supports _MODE_PAGE_FLIP_ASYNC. + * If set to 1, the driver supports _MODE_PAGE_FLIP_ASYNC for legacy + * page-flips. */ #define DRM_CAP_ASYNC_PAGE_FLIP0x7 /** @@ -767,6 +768,13 @@ struct drm_gem_open { * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects". */ #define DRM_CAP_SYNCOBJ_TIMELINE 0x14 +/** + * DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP + * + * If set to 1, the driver supports _MODE_PAGE_FLIP_ASYNC for atomic + * commits. + */ +#define DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP 0x15 /* DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap { -- 2.37.2
[PATCH v2 3/6] drm: introduce drm_mode_config.atomic_async_page_flip_not_supported
This new field indicates whether the driver has the necessary logic to support async page-flips via the atomic uAPI. This is leveraged by the next commit to allow user-space to use this functionality. All atomic drivers setting drm_mode_config.async_page_flip are updated to also set drm_mode_config.atomic_async_page_flip_not_supported. We will gradually check and update these drivers to properly handle drm_crtc_state.async_flip in their atomic logic. The goal of this negative flag is the same as fb_modifiers_not_supported: we want to eventually get rid of all drivers missing atomic support for async flips. New drivers should not set this flag, instead they should support atomic async flips (if they support async flips at all). IOW, we don't want more drivers with async flip support for legacy but not atomic. v2: only set the flag on atomic drivers (remove it on amdgpu DCE and on radeon) Signed-off-by: Simon Ser Cc: Daniel Vetter Cc: Joshua Ashton Cc: Melissa Wen Cc: Alex Deucher Cc: Harry Wentland Cc: Nicholas Kazlauskas Cc: André Almeida Cc: Ville Syrjälä --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 + drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 1 + drivers/gpu/drm/i915/display/intel_display.c | 1 + drivers/gpu/drm/nouveau/nouveau_display.c | 1 + drivers/gpu/drm/vc4/vc4_kms.c | 1 + include/drm/drm_mode_config.h | 11 +++ 6 files changed, 16 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 7f96d81f4e0d..c9195e7bb649 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3804,6 +3804,7 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev) adev_to_drm(adev)->mode_config.prefer_shadow = 0; /* indicates support for immediate flip */ adev_to_drm(adev)->mode_config.async_page_flip = true; + adev_to_drm(adev)->mode_config.atomic_async_page_flip_not_supported = true; adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index f7e7f4e919c7..ffb3a2fa797f 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -639,6 +639,7 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev) dev->mode_config.max_height = dc->desc->max_height; dev->mode_config.funcs = _config_funcs; dev->mode_config.async_page_flip = true; + dev->mode_config.atomic_async_page_flip_not_supported = true; return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 2a45a25e42fb..265492e6c135 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8622,6 +8622,7 @@ static void intel_mode_config_init(struct drm_i915_private *i915) mode_config->helper_private = _mode_config_funcs; mode_config->async_page_flip = HAS_ASYNC_FLIPS(i915); + mode_config->atomic_async_page_flip_not_supported = true; /* * Maximum framebuffer dimensions, chosen to match diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index a2f5df568ca5..2b5c4f24aedd 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -699,6 +699,7 @@ nouveau_display_create(struct drm_device *dev) dev->mode_config.async_page_flip = false; else dev->mode_config.async_page_flip = true; + dev->mode_config.atomic_async_page_flip_not_supported = true; drm_kms_helper_poll_init(dev); drm_kms_helper_poll_disable(dev); diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 4419e810103d..3fe59c6b2cf0 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -1047,6 +1047,7 @@ int vc4_kms_load(struct drm_device *dev) dev->mode_config.helper_private = _mode_config_helpers; dev->mode_config.preferred_depth = 24; dev->mode_config.async_page_flip = true; + dev->mode_config.atomic_async_page_flip_not_supported = true; ret = vc4_ctm_obj_init(vc4); if (ret) diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 6b5e01295348..1b535d94f2f4 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -917,6 +917,17 @@ struct drm_mode_config { */ bool async_page_flip; + /** +* @atomic_async_page_flip_not_supported: +* +* If true, the driver does not support async page-flips with the +* atomic uAPI. This is only used by old drivers which haven't yet +* accomodated for
[PATCH v2 2/6] drm: document DRM_MODE_PAGE_FLIP_ASYNC
This is a subset of [1], included here because a subsequent patch needs to document the behavior of this flag under the atomic uAPI. v2: new patch [1]: https://patchwork.freedesktop.org/patch/500177/ Signed-off-by: Simon Ser --- include/uapi/drm/drm_mode.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index fa953309d9ce..86a292c3185a 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -936,6 +936,13 @@ struct hdr_output_metadata { }; #define DRM_MODE_PAGE_FLIP_EVENT 0x01 +/** + * DRM_MODE_PAGE_FLIP_ASYNC + * + * Request that the page-flip is performed as soon as possible, ie. with no + * delay due to waiting for vblank. This may cause tearing to be visible on + * the screen. + */ #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4 #define DRM_MODE_PAGE_FLIP_TARGET_RELATIVE 0x8 -- 2.37.2
[PATCH v2 1/6] amd/display: only accept async flips for fast updates
Up until now, amdgpu was silently degrading to vsync when user-space requested an async flip but the hardware didn't support it. The hardware doesn't support immediate flips when the update changes the FB pitch, the DCC state, the rotation, enables or disables CRTCs or planes, etc. This is reflected in the dm_crtc_state.update_type field: UPDATE_TYPE_FAST means that immediate flip is supported. Silently degrading async flips to vsync is not the expected behavior from a uAPI point-of-view. Xorg expects async flips to fail if unsupported, to be able to fall back to a blit. i915 already behaves this way. This patch aligns amdgpu with uAPI expectations and returns a failure when an async flip is not possible. v2: new patch Signed-off-by: Simon Ser Cc: Joshua Ashton Cc: Melissa Wen Cc: Alex Deucher Cc: Harry Wentland Cc: Nicholas Kazlauskas Cc: André Almeida --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 10 ++ 2 files changed, 18 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 9ab01c58bedb..7f96d81f4e0d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7613,7 +7613,15 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, /* * Only allow immediate flips for fast updates that don't * change FB pitch, DCC state, rotation or mirroing. +* +* dm_crtc_helper_atomic_check() only accepts async flips with +* fast updates. */ + if (crtc->state->async_flip && + acrtc_state->update_type != UPDATE_TYPE_FAST) + drm_warn_once(state->dev, + "[PLANE:%d:%s] async flip with non-fast update\n", + plane->base.id, plane->name); bundle->flip_addrs[planes_count].flip_immediate = crtc->state->async_flip && acrtc_state->update_type == UPDATE_TYPE_FAST; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c index 594fe8a4d02b..97ead857f507 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c @@ -388,6 +388,16 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, return -EINVAL; } + /* Only allow async flips for fast updates that don't change the FB +* pitch, the DCC state, rotation, etc. */ + if (crtc_state->async_flip && + dm_crtc_state->update_type != UPDATE_TYPE_FAST) { + drm_dbg_atomic(crtc->dev, + "[CRTC:%d:%s] async flips are only supported for fast updates\n", + crtc->base.id, crtc->name); + return -EINVAL; + } + /* In some use cases, like reset, no stream is attached */ if (!dm_crtc_state->stream) return 0; -- 2.37.2
[PATCH v2 0/6] Add support for atomic async page-flips
This series adds support for DRM_MODE_PAGE_FLIP_ASYNC for atomic commits, aka. "immediate flip" (which might result in tearing). The feature was only available via the legacy uAPI, however for gaming use-cases it may be desirable to enable it via the atomic uAPI too. - v1: https://patchwork.freedesktop.org/series/107683/ - User-space patch: https://github.com/Plagman/gamescope/pull/595 - IGT patch: https://patchwork.freedesktop.org/series/107681/ Main changes in v2: add docs, fail atomic commit if async flip isn't possible. Tested on an AMD Picasso iGPU. Simon Ser (6): amd/display: only accept async flips for fast updates drm: document DRM_MODE_PAGE_FLIP_ASYNC drm: introduce drm_mode_config.atomic_async_page_flip_not_supported drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP amd/display: indicate support for atomic async page-flips on DC .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++ .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 10 +++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 1 + drivers/gpu/drm/drm_atomic_uapi.c | 28 +-- drivers/gpu/drm/drm_ioctl.c | 5 drivers/gpu/drm/i915/display/intel_display.c | 1 + drivers/gpu/drm/nouveau/nouveau_display.c | 1 + drivers/gpu/drm/vc4/vc4_kms.c | 1 + include/drm/drm_mode_config.h | 11 include/uapi/drm/drm.h| 10 ++- include/uapi/drm/drm_mode.h | 11 11 files changed, 83 insertions(+), 4 deletions(-) -- 2.37.2
[PATCH AUTOSEL 4.9 6/6] fbdev: chipsfb: Add missing pci_disable_device() in chipsfb_pci_init()
From: Yang Yingliang [ Upstream commit 07c55c9803dea748d17a054000cbf1913ce06399 ] Add missing pci_disable_device() in error path in chipsfb_pci_init(). Signed-off-by: Yang Yingliang Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/chipsfb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c index 84a3778552eba..ec1f8af165e9e 100644 --- a/drivers/video/fbdev/chipsfb.c +++ b/drivers/video/fbdev/chipsfb.c @@ -432,6 +432,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent) err_release_fb: framebuffer_release(p); err_disable: + pci_disable_device(dp); err_out: return rc; } -- 2.35.1
[PATCH AUTOSEL 4.9 5/6] fbdev: fb_pm2fb: Avoid potential divide by zero error
From: Letu Ren [ Upstream commit 19f953e7435644b81332dd632ba1b2d80b1e37af ] In `do_fb_ioctl()` of fbmem.c, if cmd is FBIOPUT_VSCREENINFO, var will be copied from user, then go through `fb_set_var()` and `info->fbops->fb_check_var()` which could may be `pm2fb_check_var()`. Along the path, `var->pixclock` won't be modified. This function checks whether reciprocal of `var->pixclock` is too high. If `var->pixclock` is zero, there will be a divide by zero error. So, it is necessary to check whether denominator is zero to avoid crash. As this bug is found by Syzkaller, logs are listed below. divide error in pm2fb_check_var Call Trace: fb_set_var+0x367/0xeb0 drivers/video/fbdev/core/fbmem.c:1015 do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1110 fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1189 Reported-by: Zheyu Ma Signed-off-by: Letu Ren Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/pm2fb.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c index 1a4070f719c29..9b32b9fc44a5c 100644 --- a/drivers/video/fbdev/pm2fb.c +++ b/drivers/video/fbdev/pm2fb.c @@ -614,6 +614,11 @@ static int pm2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) return -EINVAL; } + if (!var->pixclock) { + DPRINTK("pixclock is zero\n"); + return -EINVAL; + } + if (PICOS2KHZ(var->pixclock) > PM2_MAX_PIXCLOCK) { DPRINTK("pixclock too high (%ldKHz)\n", PICOS2KHZ(var->pixclock)); -- 2.35.1
[PATCH AUTOSEL 4.14 8/8] fbdev: chipsfb: Add missing pci_disable_device() in chipsfb_pci_init()
From: Yang Yingliang [ Upstream commit 07c55c9803dea748d17a054000cbf1913ce06399 ] Add missing pci_disable_device() in error path in chipsfb_pci_init(). Signed-off-by: Yang Yingliang Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/chipsfb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c index 413b465e69d8e..7ca149ab86d20 100644 --- a/drivers/video/fbdev/chipsfb.c +++ b/drivers/video/fbdev/chipsfb.c @@ -432,6 +432,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent) err_release_fb: framebuffer_release(p); err_disable: + pci_disable_device(dp); err_out: return rc; } -- 2.35.1
[PATCH AUTOSEL 4.9 2/6] drm/radeon: add a force flush to delay work when radeon
From: Zhenneng Li [ Upstream commit f461950fdc374a3ada5a63c669d997de4600dffe ] Although radeon card fence and wait for gpu to finish processing current batch rings, there is still a corner case that radeon lockup work queue may not be fully flushed, and meanwhile the radeon_suspend_kms() function has called pci_set_power_state() to put device in D3hot state. Per PCI spec rev 4.0 on 5.3.1.4.1 D3hot State. > Configuration and Message requests are the only TLPs accepted by a Function in > the D3hot state. All other received Requests must be handled as Unsupported > Requests, > and all received Completions may optionally be handled as Unexpected > Completions. This issue will happen in following logs: Unable to handle kernel paging request at virtual address 8800e0008010 CPU 0 kworker/0:3(131): Oops 0 pc = [] ra = [] ps = Tainted: G W pc is at si_gpu_check_soft_reset+0x3c/0x240 ra is at si_dma_is_lockup+0x34/0xd0 v0 = t0 = fff08800e0008010 t1 = 0001 t2 = 8010 t3 = fff7e3c0 t4 = fff7e3c00258 t5 = t6 = 0001 t7 = fff7ef078000 s0 = fff7e3c016e8 s1 = fff7e3c0 s2 = fff7e3c00018 s3 = fff7e3c0 s4 = fff7fff59d80 s5 = s6 = fff7ef07bd98 a0 = fff7e3c0 a1 = fff7e3c016e8 a2 = 0008 a3 = 0001 a4 = 8f5c28f5c28f5c29 a5 = 810f4338 t8 = 0275 t9 = 809b66f8 t10 = ff6769c5d964b800 t11= b886 pv = 811bea20 at = gp = 81d89690 sp = aa814126 Disabling lock debugging due to kernel taint Trace: [] si_dma_is_lockup+0x34/0xd0 [] radeon_fence_check_lockup+0xd0/0x290 [] process_one_work+0x280/0x550 [] worker_thread+0x70/0x7c0 [] worker_thread+0x130/0x7c0 [] kthread+0x200/0x210 [] worker_thread+0x0/0x7c0 [] kthread+0x14c/0x210 [] ret_from_kernel_thread+0x18/0x20 [] kthread+0x0/0x210 Code: ad3e0008 43f0074a ad7e0018 ad9e0020 8c3001e8 40230101 <8821> 4821ed21 So force lockup work queue flush to fix this problem. Acked-by: Christian König Signed-off-by: Zhenneng Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/radeon/radeon_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 82b01123c3868..227c4733de2ea 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1661,6 +1661,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, if (r) { /* delay GPU reset to resume */ radeon_fence_driver_force_completion(rdev, i); + } else { + /* finish executing delayed work */ + flush_delayed_work(>fence_drv[i].lockup_work); } } -- 2.35.1
[PATCH AUTOSEL 4.14 7/8] fbdev: fb_pm2fb: Avoid potential divide by zero error
From: Letu Ren [ Upstream commit 19f953e7435644b81332dd632ba1b2d80b1e37af ] In `do_fb_ioctl()` of fbmem.c, if cmd is FBIOPUT_VSCREENINFO, var will be copied from user, then go through `fb_set_var()` and `info->fbops->fb_check_var()` which could may be `pm2fb_check_var()`. Along the path, `var->pixclock` won't be modified. This function checks whether reciprocal of `var->pixclock` is too high. If `var->pixclock` is zero, there will be a divide by zero error. So, it is necessary to check whether denominator is zero to avoid crash. As this bug is found by Syzkaller, logs are listed below. divide error in pm2fb_check_var Call Trace: fb_set_var+0x367/0xeb0 drivers/video/fbdev/core/fbmem.c:1015 do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1110 fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1189 Reported-by: Zheyu Ma Signed-off-by: Letu Ren Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/pm2fb.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c index bd6c2f5f6095d..a5375b09415a6 100644 --- a/drivers/video/fbdev/pm2fb.c +++ b/drivers/video/fbdev/pm2fb.c @@ -614,6 +614,11 @@ static int pm2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) return -EINVAL; } + if (!var->pixclock) { + DPRINTK("pixclock is zero\n"); + return -EINVAL; + } + if (PICOS2KHZ(var->pixclock) > PM2_MAX_PIXCLOCK) { DPRINTK("pixclock too high (%ldKHz)\n", PICOS2KHZ(var->pixclock)); -- 2.35.1
[PATCH AUTOSEL 4.19 10/10] drm/amdgpu: mmVM_L2_CNTL3 register not initialized correctly
From: Qu Huang [ Upstream commit b8983d42524f10ac6bf35bbce6a7cc8e45f61e04 ] The mmVM_L2_CNTL3 register is not assigned an initial value Signed-off-by: Qu Huang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c index c963eec58c702..923bc097a00b2 100644 --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c @@ -155,6 +155,7 @@ static void mmhub_v1_0_init_cache_regs(struct amdgpu_device *adev) tmp = REG_SET_FIELD(tmp, VM_L2_CNTL2, INVALIDATE_L2_CACHE, 1); WREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL2, tmp); + tmp = mmVM_L2_CNTL3_DEFAULT; if (adev->gmc.translate_further) { tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, BANK_SELECT, 12); tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, -- 2.35.1
[PATCH AUTOSEL 4.14 2/8] drm/amdgpu: Check num_gfx_rings for gfx v9_0 rb setup.
From: Candice Li [ Upstream commit c351938350ab9b5e978dede2c321da43de7eb70c ] No need to set up rb when no gfx rings. Signed-off-by: Candice Li Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 53186c5e1066b..bb0d32b4be74d 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -1514,7 +1514,8 @@ static void gfx_v9_0_gpu_init(struct amdgpu_device *adev) gfx_v9_0_tiling_mode_table_init(adev); - gfx_v9_0_setup_rb(adev); + if (adev->gfx.num_gfx_rings) + gfx_v9_0_setup_rb(adev); gfx_v9_0_get_cu_info(adev, >gfx.cu_info); /* XXX SH_MEM regs */ -- 2.35.1
[PATCH AUTOSEL 4.14 3/8] drm/radeon: add a force flush to delay work when radeon
From: Zhenneng Li [ Upstream commit f461950fdc374a3ada5a63c669d997de4600dffe ] Although radeon card fence and wait for gpu to finish processing current batch rings, there is still a corner case that radeon lockup work queue may not be fully flushed, and meanwhile the radeon_suspend_kms() function has called pci_set_power_state() to put device in D3hot state. Per PCI spec rev 4.0 on 5.3.1.4.1 D3hot State. > Configuration and Message requests are the only TLPs accepted by a Function in > the D3hot state. All other received Requests must be handled as Unsupported > Requests, > and all received Completions may optionally be handled as Unexpected > Completions. This issue will happen in following logs: Unable to handle kernel paging request at virtual address 8800e0008010 CPU 0 kworker/0:3(131): Oops 0 pc = [] ra = [] ps = Tainted: G W pc is at si_gpu_check_soft_reset+0x3c/0x240 ra is at si_dma_is_lockup+0x34/0xd0 v0 = t0 = fff08800e0008010 t1 = 0001 t2 = 8010 t3 = fff7e3c0 t4 = fff7e3c00258 t5 = t6 = 0001 t7 = fff7ef078000 s0 = fff7e3c016e8 s1 = fff7e3c0 s2 = fff7e3c00018 s3 = fff7e3c0 s4 = fff7fff59d80 s5 = s6 = fff7ef07bd98 a0 = fff7e3c0 a1 = fff7e3c016e8 a2 = 0008 a3 = 0001 a4 = 8f5c28f5c28f5c29 a5 = 810f4338 t8 = 0275 t9 = 809b66f8 t10 = ff6769c5d964b800 t11= b886 pv = 811bea20 at = gp = 81d89690 sp = aa814126 Disabling lock debugging due to kernel taint Trace: [] si_dma_is_lockup+0x34/0xd0 [] radeon_fence_check_lockup+0xd0/0x290 [] process_one_work+0x280/0x550 [] worker_thread+0x70/0x7c0 [] worker_thread+0x130/0x7c0 [] kthread+0x200/0x210 [] worker_thread+0x0/0x7c0 [] kthread+0x14c/0x210 [] ret_from_kernel_thread+0x18/0x20 [] kthread+0x0/0x210 Code: ad3e0008 43f0074a ad7e0018 ad9e0020 8c3001e8 40230101 <8821> 4821ed21 So force lockup work queue flush to fix this problem. Acked-by: Christian König Signed-off-by: Zhenneng Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/radeon/radeon_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 58488eac84627..906547b229a9a 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1655,6 +1655,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, if (r) { /* delay GPU reset to resume */ radeon_fence_driver_force_completion(rdev, i); + } else { + /* finish executing delayed work */ + flush_delayed_work(>fence_drv[i].lockup_work); } } -- 2.35.1
[PATCH AUTOSEL 4.19 09/10] fbdev: chipsfb: Add missing pci_disable_device() in chipsfb_pci_init()
From: Yang Yingliang [ Upstream commit 07c55c9803dea748d17a054000cbf1913ce06399 ] Add missing pci_disable_device() in error path in chipsfb_pci_init(). Signed-off-by: Yang Yingliang Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/chipsfb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c index 413b465e69d8e..7ca149ab86d20 100644 --- a/drivers/video/fbdev/chipsfb.c +++ b/drivers/video/fbdev/chipsfb.c @@ -432,6 +432,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent) err_release_fb: framebuffer_release(p); err_disable: + pci_disable_device(dp); err_out: return rc; } -- 2.35.1
[PATCH AUTOSEL 4.19 08/10] fbdev: fb_pm2fb: Avoid potential divide by zero error
From: Letu Ren [ Upstream commit 19f953e7435644b81332dd632ba1b2d80b1e37af ] In `do_fb_ioctl()` of fbmem.c, if cmd is FBIOPUT_VSCREENINFO, var will be copied from user, then go through `fb_set_var()` and `info->fbops->fb_check_var()` which could may be `pm2fb_check_var()`. Along the path, `var->pixclock` won't be modified. This function checks whether reciprocal of `var->pixclock` is too high. If `var->pixclock` is zero, there will be a divide by zero error. So, it is necessary to check whether denominator is zero to avoid crash. As this bug is found by Syzkaller, logs are listed below. divide error in pm2fb_check_var Call Trace: fb_set_var+0x367/0xeb0 drivers/video/fbdev/core/fbmem.c:1015 do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1110 fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1189 Reported-by: Zheyu Ma Signed-off-by: Letu Ren Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/pm2fb.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c index 1dcf02e12af4f..8ae010f07d7da 100644 --- a/drivers/video/fbdev/pm2fb.c +++ b/drivers/video/fbdev/pm2fb.c @@ -616,6 +616,11 @@ static int pm2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) return -EINVAL; } + if (!var->pixclock) { + DPRINTK("pixclock is zero\n"); + return -EINVAL; + } + if (PICOS2KHZ(var->pixclock) > PM2_MAX_PIXCLOCK) { DPRINTK("pixclock too high (%ldKHz)\n", PICOS2KHZ(var->pixclock)); -- 2.35.1
[PATCH AUTOSEL 5.4 11/12] fbdev: chipsfb: Add missing pci_disable_device() in chipsfb_pci_init()
From: Yang Yingliang [ Upstream commit 07c55c9803dea748d17a054000cbf1913ce06399 ] Add missing pci_disable_device() in error path in chipsfb_pci_init(). Signed-off-by: Yang Yingliang Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/chipsfb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c index 80fdd3ee0565f..57b1e011d2d34 100644 --- a/drivers/video/fbdev/chipsfb.c +++ b/drivers/video/fbdev/chipsfb.c @@ -430,6 +430,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent) err_release_fb: framebuffer_release(p); err_disable: + pci_disable_device(dp); err_out: return rc; } -- 2.35.1
[PATCH AUTOSEL 4.19 03/10] drm/radeon: add a force flush to delay work when radeon
From: Zhenneng Li [ Upstream commit f461950fdc374a3ada5a63c669d997de4600dffe ] Although radeon card fence and wait for gpu to finish processing current batch rings, there is still a corner case that radeon lockup work queue may not be fully flushed, and meanwhile the radeon_suspend_kms() function has called pci_set_power_state() to put device in D3hot state. Per PCI spec rev 4.0 on 5.3.1.4.1 D3hot State. > Configuration and Message requests are the only TLPs accepted by a Function in > the D3hot state. All other received Requests must be handled as Unsupported > Requests, > and all received Completions may optionally be handled as Unexpected > Completions. This issue will happen in following logs: Unable to handle kernel paging request at virtual address 8800e0008010 CPU 0 kworker/0:3(131): Oops 0 pc = [] ra = [] ps = Tainted: G W pc is at si_gpu_check_soft_reset+0x3c/0x240 ra is at si_dma_is_lockup+0x34/0xd0 v0 = t0 = fff08800e0008010 t1 = 0001 t2 = 8010 t3 = fff7e3c0 t4 = fff7e3c00258 t5 = t6 = 0001 t7 = fff7ef078000 s0 = fff7e3c016e8 s1 = fff7e3c0 s2 = fff7e3c00018 s3 = fff7e3c0 s4 = fff7fff59d80 s5 = s6 = fff7ef07bd98 a0 = fff7e3c0 a1 = fff7e3c016e8 a2 = 0008 a3 = 0001 a4 = 8f5c28f5c28f5c29 a5 = 810f4338 t8 = 0275 t9 = 809b66f8 t10 = ff6769c5d964b800 t11= b886 pv = 811bea20 at = gp = 81d89690 sp = aa814126 Disabling lock debugging due to kernel taint Trace: [] si_dma_is_lockup+0x34/0xd0 [] radeon_fence_check_lockup+0xd0/0x290 [] process_one_work+0x280/0x550 [] worker_thread+0x70/0x7c0 [] worker_thread+0x130/0x7c0 [] kthread+0x200/0x210 [] worker_thread+0x0/0x7c0 [] kthread+0x14c/0x210 [] ret_from_kernel_thread+0x18/0x20 [] kthread+0x0/0x210 Code: ad3e0008 43f0074a ad7e0018 ad9e0020 8c3001e8 40230101 <8821> 4821ed21 So force lockup work queue flush to fix this problem. Acked-by: Christian König Signed-off-by: Zhenneng Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/radeon/radeon_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 59c8a6647ff21..cc1c07963116c 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1625,6 +1625,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, if (r) { /* delay GPU reset to resume */ radeon_fence_driver_force_completion(rdev, i); + } else { + /* finish executing delayed work */ + flush_delayed_work(>fence_drv[i].lockup_work); } } -- 2.35.1
[PATCH AUTOSEL 5.4 12/12] drm/amdgpu: mmVM_L2_CNTL3 register not initialized correctly
From: Qu Huang [ Upstream commit b8983d42524f10ac6bf35bbce6a7cc8e45f61e04 ] The mmVM_L2_CNTL3 register is not assigned an initial value Signed-off-by: Qu Huang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c index 641f1258f08dc..e60157fe7a7bf 100644 --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c @@ -182,6 +182,7 @@ static void mmhub_v1_0_init_cache_regs(struct amdgpu_device *adev) tmp = REG_SET_FIELD(tmp, VM_L2_CNTL2, INVALIDATE_L2_CACHE, 1); WREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL2, tmp); + tmp = mmVM_L2_CNTL3_DEFAULT; if (adev->gmc.translate_further) { tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, BANK_SELECT, 12); tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, -- 2.35.1
[PATCH AUTOSEL 4.19 02/10] drm/amdgpu: Check num_gfx_rings for gfx v9_0 rb setup.
From: Candice Li [ Upstream commit c351938350ab9b5e978dede2c321da43de7eb70c ] No need to set up rb when no gfx rings. Signed-off-by: Candice Li Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index d8ae6a23e6133..d36bea68a67e1 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -1771,7 +1771,8 @@ static void gfx_v9_0_gpu_init(struct amdgpu_device *adev) gfx_v9_0_tiling_mode_table_init(adev); - gfx_v9_0_setup_rb(adev); + if (adev->gfx.num_gfx_rings) + gfx_v9_0_setup_rb(adev); gfx_v9_0_get_cu_info(adev, >gfx.cu_info); adev->gfx.config.db_debug2 = RREG32_SOC15(GC, 0, mmDB_DEBUG2); -- 2.35.1
[PATCH AUTOSEL 5.4 10/12] fbdev: fb_pm2fb: Avoid potential divide by zero error
From: Letu Ren [ Upstream commit 19f953e7435644b81332dd632ba1b2d80b1e37af ] In `do_fb_ioctl()` of fbmem.c, if cmd is FBIOPUT_VSCREENINFO, var will be copied from user, then go through `fb_set_var()` and `info->fbops->fb_check_var()` which could may be `pm2fb_check_var()`. Along the path, `var->pixclock` won't be modified. This function checks whether reciprocal of `var->pixclock` is too high. If `var->pixclock` is zero, there will be a divide by zero error. So, it is necessary to check whether denominator is zero to avoid crash. As this bug is found by Syzkaller, logs are listed below. divide error in pm2fb_check_var Call Trace: fb_set_var+0x367/0xeb0 drivers/video/fbdev/core/fbmem.c:1015 do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1110 fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1189 Reported-by: Zheyu Ma Signed-off-by: Letu Ren Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/pm2fb.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c index 1dcf02e12af4f..8ae010f07d7da 100644 --- a/drivers/video/fbdev/pm2fb.c +++ b/drivers/video/fbdev/pm2fb.c @@ -616,6 +616,11 @@ static int pm2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) return -EINVAL; } + if (!var->pixclock) { + DPRINTK("pixclock is zero\n"); + return -EINVAL; + } + if (PICOS2KHZ(var->pixclock) > PM2_MAX_PIXCLOCK) { DPRINTK("pixclock too high (%ldKHz)\n", PICOS2KHZ(var->pixclock)); -- 2.35.1
[PATCH AUTOSEL 5.4 03/12] drm/gem: Fix GEM handle release errors
From: Jeffy Chen [ Upstream commit ea2aa97ca37a9044ade001aef71dbc06318e8d44 ] Currently we are assuming a one to one mapping between dmabuf and GEM handle when releasing GEM handles. But that is not always true, since we would create extra handles for the GEM obj in cases like gem_open() and getfb{,2}(). A similar issue was reported at: https://lore.kernel.org/all/20211105083308.392156-1-jay...@rock-chips.com/ Another problem is that the imported dmabuf might not always have gem_obj->dma_buf set, which would cause leaks in drm_gem_remove_prime_handles(). Let's fix these for now by using handle to find the exact map to remove. Signed-off-by: Jeffy Chen Reviewed-by: Christian König Signed-off-by: Christian König Link: https://patchwork.freedesktop.org/patch/msgid/20220819072834.17888-1-jeffy.c...@rock-chips.com Signed-off-by: Sasha Levin --- drivers/gpu/drm/drm_gem.c | 17 + drivers/gpu/drm/drm_internal.h | 4 ++-- drivers/gpu/drm/drm_prime.c| 20 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 25a2d80287d67..d6a72f3cb1fbd 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -167,21 +167,6 @@ void drm_gem_private_object_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_gem_private_object_init); -static void -drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) -{ - /* -* Note: obj->dma_buf can't disappear as long as we still hold a -* handle reference in obj->handle_count. -*/ - mutex_lock(>prime.lock); - if (obj->dma_buf) { - drm_prime_remove_buf_handle_locked(>prime, - obj->dma_buf); - } - mutex_unlock(>prime.lock); -} - /** * drm_gem_object_handle_free - release resources bound to userspace handles * @obj: GEM object to clean up. @@ -255,7 +240,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) else if (dev->driver->gem_close_object) dev->driver->gem_close_object(obj, file_priv); - drm_gem_remove_prime_handles(obj, file_priv); + drm_prime_remove_buf_handle(_priv->prime, id); drm_vma_node_revoke(>vma_node, file_priv); drm_gem_object_handle_put_unlocked(obj); diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 51a2055c8f18a..41a9a9bae5848 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -59,8 +59,8 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); -void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf); +void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, +uint32_t handle); /* drm_drv.c */ struct drm_minor *drm_minor_acquire(unsigned int minor_id); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0a2316e0e8121..6b7cf0170f9d1 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -187,29 +187,33 @@ static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri return -ENOENT; } -void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf) +void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, +uint32_t handle) { struct rb_node *rb; - rb = prime_fpriv->dmabufs.rb_node; + mutex_lock(_fpriv->lock); + + rb = prime_fpriv->handles.rb_node; while (rb) { struct drm_prime_member *member; - member = rb_entry(rb, struct drm_prime_member, dmabuf_rb); - if (member->dma_buf == dma_buf) { + member = rb_entry(rb, struct drm_prime_member, handle_rb); + if (member->handle == handle) { rb_erase(>handle_rb, _fpriv->handles); rb_erase(>dmabuf_rb, _fpriv->dmabufs); - dma_buf_put(dma_buf); + dma_buf_put(member->dma_buf); kfree(member); - return; - } else if (member->dma_buf < dma_buf) { + break; + } else if (member->handle < handle) { rb = rb->rb_right; } else { rb = rb->rb_left; } } + + mutex_unlock(_fpriv->lock); } void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv) -- 2.35.1
[PATCH AUTOSEL 5.10 16/16] drm/amdgpu: mmVM_L2_CNTL3 register not initialized correctly
From: Qu Huang [ Upstream commit b8983d42524f10ac6bf35bbce6a7cc8e45f61e04 ] The mmVM_L2_CNTL3 register is not assigned an initial value Signed-off-by: Qu Huang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c index f84701c562bf2..97441f373531f 100644 --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c @@ -178,6 +178,7 @@ static void mmhub_v1_0_init_cache_regs(struct amdgpu_device *adev) tmp = REG_SET_FIELD(tmp, VM_L2_CNTL2, INVALIDATE_L2_CACHE, 1); WREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL2, tmp); + tmp = mmVM_L2_CNTL3_DEFAULT; if (adev->gmc.translate_further) { tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, BANK_SELECT, 12); tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, -- 2.35.1
[PATCH AUTOSEL 5.4 05/12] drm/radeon: add a force flush to delay work when radeon
From: Zhenneng Li [ Upstream commit f461950fdc374a3ada5a63c669d997de4600dffe ] Although radeon card fence and wait for gpu to finish processing current batch rings, there is still a corner case that radeon lockup work queue may not be fully flushed, and meanwhile the radeon_suspend_kms() function has called pci_set_power_state() to put device in D3hot state. Per PCI spec rev 4.0 on 5.3.1.4.1 D3hot State. > Configuration and Message requests are the only TLPs accepted by a Function in > the D3hot state. All other received Requests must be handled as Unsupported > Requests, > and all received Completions may optionally be handled as Unexpected > Completions. This issue will happen in following logs: Unable to handle kernel paging request at virtual address 8800e0008010 CPU 0 kworker/0:3(131): Oops 0 pc = [] ra = [] ps = Tainted: G W pc is at si_gpu_check_soft_reset+0x3c/0x240 ra is at si_dma_is_lockup+0x34/0xd0 v0 = t0 = fff08800e0008010 t1 = 0001 t2 = 8010 t3 = fff7e3c0 t4 = fff7e3c00258 t5 = t6 = 0001 t7 = fff7ef078000 s0 = fff7e3c016e8 s1 = fff7e3c0 s2 = fff7e3c00018 s3 = fff7e3c0 s4 = fff7fff59d80 s5 = s6 = fff7ef07bd98 a0 = fff7e3c0 a1 = fff7e3c016e8 a2 = 0008 a3 = 0001 a4 = 8f5c28f5c28f5c29 a5 = 810f4338 t8 = 0275 t9 = 809b66f8 t10 = ff6769c5d964b800 t11= b886 pv = 811bea20 at = gp = 81d89690 sp = aa814126 Disabling lock debugging due to kernel taint Trace: [] si_dma_is_lockup+0x34/0xd0 [] radeon_fence_check_lockup+0xd0/0x290 [] process_one_work+0x280/0x550 [] worker_thread+0x70/0x7c0 [] worker_thread+0x130/0x7c0 [] kthread+0x200/0x210 [] worker_thread+0x0/0x7c0 [] kthread+0x14c/0x210 [] ret_from_kernel_thread+0x18/0x20 [] kthread+0x0/0x210 Code: ad3e0008 43f0074a ad7e0018 ad9e0020 8c3001e8 40230101 <8821> 4821ed21 So force lockup work queue flush to fix this problem. Acked-by: Christian König Signed-off-by: Zhenneng Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/radeon/radeon_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 5d017f0aec665..e892582e847b5 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1623,6 +1623,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, if (r) { /* delay GPU reset to resume */ radeon_fence_driver_force_completion(rdev, i); + } else { + /* finish executing delayed work */ + flush_delayed_work(>fence_drv[i].lockup_work); } } -- 2.35.1
[PATCH AUTOSEL 5.4 04/12] drm/amdgpu: Check num_gfx_rings for gfx v9_0 rb setup.
From: Candice Li [ Upstream commit c351938350ab9b5e978dede2c321da43de7eb70c ] No need to set up rb when no gfx rings. Signed-off-by: Candice Li Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 5906a8951a6c6..685a2df01d096 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -2472,7 +2472,8 @@ static void gfx_v9_0_constants_init(struct amdgpu_device *adev) gfx_v9_0_tiling_mode_table_init(adev); - gfx_v9_0_setup_rb(adev); + if (adev->gfx.num_gfx_rings) + gfx_v9_0_setup_rb(adev); gfx_v9_0_get_cu_info(adev, >gfx.cu_info); adev->gfx.config.db_debug2 = RREG32_SOC15(GC, 0, mmDB_DEBUG2); -- 2.35.1
[PATCH AUTOSEL 5.10 15/16] fbdev: chipsfb: Add missing pci_disable_device() in chipsfb_pci_init()
From: Yang Yingliang [ Upstream commit 07c55c9803dea748d17a054000cbf1913ce06399 ] Add missing pci_disable_device() in error path in chipsfb_pci_init(). Signed-off-by: Yang Yingliang Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/chipsfb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c index 393894af26f84..2b00a9d554fc0 100644 --- a/drivers/video/fbdev/chipsfb.c +++ b/drivers/video/fbdev/chipsfb.c @@ -430,6 +430,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent) err_release_fb: framebuffer_release(p); err_disable: + pci_disable_device(dp); err_out: return rc; } -- 2.35.1
[PATCH AUTOSEL 5.10 14/16] fbdev: fb_pm2fb: Avoid potential divide by zero error
From: Letu Ren [ Upstream commit 19f953e7435644b81332dd632ba1b2d80b1e37af ] In `do_fb_ioctl()` of fbmem.c, if cmd is FBIOPUT_VSCREENINFO, var will be copied from user, then go through `fb_set_var()` and `info->fbops->fb_check_var()` which could may be `pm2fb_check_var()`. Along the path, `var->pixclock` won't be modified. This function checks whether reciprocal of `var->pixclock` is too high. If `var->pixclock` is zero, there will be a divide by zero error. So, it is necessary to check whether denominator is zero to avoid crash. As this bug is found by Syzkaller, logs are listed below. divide error in pm2fb_check_var Call Trace: fb_set_var+0x367/0xeb0 drivers/video/fbdev/core/fbmem.c:1015 do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1110 fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1189 Reported-by: Zheyu Ma Signed-off-by: Letu Ren Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/pm2fb.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c index 0642555289e06..c12d46e283598 100644 --- a/drivers/video/fbdev/pm2fb.c +++ b/drivers/video/fbdev/pm2fb.c @@ -616,6 +616,11 @@ static int pm2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) return -EINVAL; } + if (!var->pixclock) { + DPRINTK("pixclock is zero\n"); + return -EINVAL; + } + if (PICOS2KHZ(var->pixclock) > PM2_MAX_PIXCLOCK) { DPRINTK("pixclock too high (%ldKHz)\n", PICOS2KHZ(var->pixclock)); -- 2.35.1
[PATCH AUTOSEL 5.10 05/16] drm/amdgpu: Move psp_xgmi_terminate call from amdgpu_xgmi_remove_device to psp_hw_fini
From: YiPeng Chai [ Upstream commit 9d705d7741ae70764f3d6d87e67fad3b5c30ffd0 ] V1: The amdgpu_xgmi_remove_device function will send unload command to psp through psp ring to terminate xgmi, but psp ring has been destroyed in psp_hw_fini. V2: 1. Change the commit title. 2. Restore amdgpu_xgmi_remove_device to its original calling location. Move psp_xgmi_terminate call from amdgpu_xgmi_remove_device to psp_hw_fini. Signed-off-by: YiPeng Chai Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 2f47f81a74a57..ae84d3b582aa5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -2146,6 +2146,9 @@ static int psp_hw_fini(void *handle) psp_rap_terminate(psp); psp_dtm_terminate(psp); psp_hdcp_terminate(psp); + + if (adev->gmc.xgmi.num_physical_nodes > 1) + psp_xgmi_terminate(psp); } psp_asd_unload(psp); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c index 042c85fc528bb..def0b7092438f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c @@ -622,7 +622,7 @@ int amdgpu_xgmi_remove_device(struct amdgpu_device *adev) amdgpu_put_xgmi_hive(hive); } - return psp_xgmi_terminate(>psp); + return 0; } int amdgpu_xgmi_ras_late_init(struct amdgpu_device *adev) -- 2.35.1
[PATCH AUTOSEL 5.10 06/16] drm/amdgpu: Check num_gfx_rings for gfx v9_0 rb setup.
From: Candice Li [ Upstream commit c351938350ab9b5e978dede2c321da43de7eb70c ] No need to set up rb when no gfx rings. Signed-off-by: Candice Li Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 405bb3efa2a96..38f4c7474487b 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -2570,7 +2570,8 @@ static void gfx_v9_0_constants_init(struct amdgpu_device *adev) gfx_v9_0_tiling_mode_table_init(adev); - gfx_v9_0_setup_rb(adev); + if (adev->gfx.num_gfx_rings) + gfx_v9_0_setup_rb(adev); gfx_v9_0_get_cu_info(adev, >gfx.cu_info); adev->gfx.config.db_debug2 = RREG32_SOC15(GC, 0, mmDB_DEBUG2); -- 2.35.1
[PATCH AUTOSEL 5.10 07/16] drm/radeon: add a force flush to delay work when radeon
From: Zhenneng Li [ Upstream commit f461950fdc374a3ada5a63c669d997de4600dffe ] Although radeon card fence and wait for gpu to finish processing current batch rings, there is still a corner case that radeon lockup work queue may not be fully flushed, and meanwhile the radeon_suspend_kms() function has called pci_set_power_state() to put device in D3hot state. Per PCI spec rev 4.0 on 5.3.1.4.1 D3hot State. > Configuration and Message requests are the only TLPs accepted by a Function in > the D3hot state. All other received Requests must be handled as Unsupported > Requests, > and all received Completions may optionally be handled as Unexpected > Completions. This issue will happen in following logs: Unable to handle kernel paging request at virtual address 8800e0008010 CPU 0 kworker/0:3(131): Oops 0 pc = [] ra = [] ps = Tainted: G W pc is at si_gpu_check_soft_reset+0x3c/0x240 ra is at si_dma_is_lockup+0x34/0xd0 v0 = t0 = fff08800e0008010 t1 = 0001 t2 = 8010 t3 = fff7e3c0 t4 = fff7e3c00258 t5 = t6 = 0001 t7 = fff7ef078000 s0 = fff7e3c016e8 s1 = fff7e3c0 s2 = fff7e3c00018 s3 = fff7e3c0 s4 = fff7fff59d80 s5 = s6 = fff7ef07bd98 a0 = fff7e3c0 a1 = fff7e3c016e8 a2 = 0008 a3 = 0001 a4 = 8f5c28f5c28f5c29 a5 = 810f4338 t8 = 0275 t9 = 809b66f8 t10 = ff6769c5d964b800 t11= b886 pv = 811bea20 at = gp = 81d89690 sp = aa814126 Disabling lock debugging due to kernel taint Trace: [] si_dma_is_lockup+0x34/0xd0 [] radeon_fence_check_lockup+0xd0/0x290 [] process_one_work+0x280/0x550 [] worker_thread+0x70/0x7c0 [] worker_thread+0x130/0x7c0 [] kthread+0x200/0x210 [] worker_thread+0x0/0x7c0 [] kthread+0x14c/0x210 [] ret_from_kernel_thread+0x18/0x20 [] kthread+0x0/0x210 Code: ad3e0008 43f0074a ad7e0018 ad9e0020 8c3001e8 40230101 <8821> 4821ed21 So force lockup work queue flush to fix this problem. Acked-by: Christian König Signed-off-by: Zhenneng Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/radeon/radeon_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 266e3cbbd09bd..8287410f471fb 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1623,6 +1623,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, if (r) { /* delay GPU reset to resume */ radeon_fence_driver_force_completion(rdev, i); + } else { + /* finish executing delayed work */ + flush_delayed_work(>fence_drv[i].lockup_work); } } -- 2.35.1
[PATCH AUTOSEL 5.10 04/16] drm/gem: Fix GEM handle release errors
From: Jeffy Chen [ Upstream commit ea2aa97ca37a9044ade001aef71dbc06318e8d44 ] Currently we are assuming a one to one mapping between dmabuf and GEM handle when releasing GEM handles. But that is not always true, since we would create extra handles for the GEM obj in cases like gem_open() and getfb{,2}(). A similar issue was reported at: https://lore.kernel.org/all/20211105083308.392156-1-jay...@rock-chips.com/ Another problem is that the imported dmabuf might not always have gem_obj->dma_buf set, which would cause leaks in drm_gem_remove_prime_handles(). Let's fix these for now by using handle to find the exact map to remove. Signed-off-by: Jeffy Chen Reviewed-by: Christian König Signed-off-by: Christian König Link: https://patchwork.freedesktop.org/patch/msgid/20220819072834.17888-1-jeffy.c...@rock-chips.com Signed-off-by: Sasha Levin --- drivers/gpu/drm/drm_gem.c | 17 + drivers/gpu/drm/drm_internal.h | 4 ++-- drivers/gpu/drm/drm_prime.c| 20 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 5979af230eda0..8b30e8d83fbcf 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -166,21 +166,6 @@ void drm_gem_private_object_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_gem_private_object_init); -static void -drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) -{ - /* -* Note: obj->dma_buf can't disappear as long as we still hold a -* handle reference in obj->handle_count. -*/ - mutex_lock(>prime.lock); - if (obj->dma_buf) { - drm_prime_remove_buf_handle_locked(>prime, - obj->dma_buf); - } - mutex_unlock(>prime.lock); -} - /** * drm_gem_object_handle_free - release resources bound to userspace handles * @obj: GEM object to clean up. @@ -254,7 +239,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) else if (dev->driver->gem_close_object) dev->driver->gem_close_object(obj, file_priv); - drm_gem_remove_prime_handles(obj, file_priv); + drm_prime_remove_buf_handle(_priv->prime, id); drm_vma_node_revoke(>vma_node, file_priv); drm_gem_object_handle_put_unlocked(obj); diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index b65865c630b0a..f80e0f28087d1 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -86,8 +86,8 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); -void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf); +void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, +uint32_t handle); /* drm_drv.c */ struct drm_minor *drm_minor_acquire(unsigned int minor_id); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 9f955f2010c25..825499ea3ff59 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -187,29 +187,33 @@ static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri return -ENOENT; } -void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf) +void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, +uint32_t handle) { struct rb_node *rb; - rb = prime_fpriv->dmabufs.rb_node; + mutex_lock(_fpriv->lock); + + rb = prime_fpriv->handles.rb_node; while (rb) { struct drm_prime_member *member; - member = rb_entry(rb, struct drm_prime_member, dmabuf_rb); - if (member->dma_buf == dma_buf) { + member = rb_entry(rb, struct drm_prime_member, handle_rb); + if (member->handle == handle) { rb_erase(>handle_rb, _fpriv->handles); rb_erase(>dmabuf_rb, _fpriv->dmabufs); - dma_buf_put(dma_buf); + dma_buf_put(member->dma_buf); kfree(member); - return; - } else if (member->dma_buf < dma_buf) { + break; + } else if (member->handle < handle) { rb = rb->rb_right; } else { rb = rb->rb_left; } } + + mutex_unlock(_fpriv->lock); } void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv) -- 2.35.1
[PATCH AUTOSEL 5.15 23/23] drm/amdgpu: mmVM_L2_CNTL3 register not initialized correctly
From: Qu Huang [ Upstream commit b8983d42524f10ac6bf35bbce6a7cc8e45f61e04 ] The mmVM_L2_CNTL3 register is not assigned an initial value Signed-off-by: Qu Huang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c index b3bede1dc41da..4259f623a9d7a 100644 --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c @@ -176,6 +176,7 @@ static void mmhub_v1_0_init_cache_regs(struct amdgpu_device *adev) tmp = REG_SET_FIELD(tmp, VM_L2_CNTL2, INVALIDATE_L2_CACHE, 1); WREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL2, tmp); + tmp = mmVM_L2_CNTL3_DEFAULT; if (adev->gmc.translate_further) { tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, BANK_SELECT, 12); tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, -- 2.35.1
[PATCH AUTOSEL 5.15 21/23] fbdev: fbcon: Destroy mutex on freeing struct fb_info
From: Shigeru Yoshida [ Upstream commit 58559dfc1ebba2ae0c7627dc8f8991ae1984c6e3 ] It's needed to destroy bl_curve_mutex on freeing struct fb_info since the mutex is embedded in the structure and initialized when it's allocated. Signed-off-by: Shigeru Yoshida Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/core/fbsysfs.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c index ce699396d6bad..09ee27e7fc25f 100644 --- a/drivers/video/fbdev/core/fbsysfs.c +++ b/drivers/video/fbdev/core/fbsysfs.c @@ -84,6 +84,10 @@ void framebuffer_release(struct fb_info *info) if (WARN_ON(refcount_read(>count))) return; +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) + mutex_destroy(>bl_curve_mutex); +#endif + kfree(info->apertures); kfree(info); } -- 2.35.1
[PATCH AUTOSEL 5.15 22/23] fbdev: chipsfb: Add missing pci_disable_device() in chipsfb_pci_init()
From: Yang Yingliang [ Upstream commit 07c55c9803dea748d17a054000cbf1913ce06399 ] Add missing pci_disable_device() in error path in chipsfb_pci_init(). Signed-off-by: Yang Yingliang Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/chipsfb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c index 393894af26f84..2b00a9d554fc0 100644 --- a/drivers/video/fbdev/chipsfb.c +++ b/drivers/video/fbdev/chipsfb.c @@ -430,6 +430,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent) err_release_fb: framebuffer_release(p); err_disable: + pci_disable_device(dp); err_out: return rc; } -- 2.35.1
[PATCH AUTOSEL 5.15 20/23] fbdev: fb_pm2fb: Avoid potential divide by zero error
From: Letu Ren [ Upstream commit 19f953e7435644b81332dd632ba1b2d80b1e37af ] In `do_fb_ioctl()` of fbmem.c, if cmd is FBIOPUT_VSCREENINFO, var will be copied from user, then go through `fb_set_var()` and `info->fbops->fb_check_var()` which could may be `pm2fb_check_var()`. Along the path, `var->pixclock` won't be modified. This function checks whether reciprocal of `var->pixclock` is too high. If `var->pixclock` is zero, there will be a divide by zero error. So, it is necessary to check whether denominator is zero to avoid crash. As this bug is found by Syzkaller, logs are listed below. divide error in pm2fb_check_var Call Trace: fb_set_var+0x367/0xeb0 drivers/video/fbdev/core/fbmem.c:1015 do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1110 fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1189 Reported-by: Zheyu Ma Signed-off-by: Letu Ren Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/pm2fb.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c index c68725eebee3b..cbcf112c88d30 100644 --- a/drivers/video/fbdev/pm2fb.c +++ b/drivers/video/fbdev/pm2fb.c @@ -617,6 +617,11 @@ static int pm2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) return -EINVAL; } + if (!var->pixclock) { + DPRINTK("pixclock is zero\n"); + return -EINVAL; + } + if (PICOS2KHZ(var->pixclock) > PM2_MAX_PIXCLOCK) { DPRINTK("pixclock too high (%ldKHz)\n", PICOS2KHZ(var->pixclock)); -- 2.35.1
[PATCH AUTOSEL 5.15 08/23] drm/radeon: add a force flush to delay work when radeon
From: Zhenneng Li [ Upstream commit f461950fdc374a3ada5a63c669d997de4600dffe ] Although radeon card fence and wait for gpu to finish processing current batch rings, there is still a corner case that radeon lockup work queue may not be fully flushed, and meanwhile the radeon_suspend_kms() function has called pci_set_power_state() to put device in D3hot state. Per PCI spec rev 4.0 on 5.3.1.4.1 D3hot State. > Configuration and Message requests are the only TLPs accepted by a Function in > the D3hot state. All other received Requests must be handled as Unsupported > Requests, > and all received Completions may optionally be handled as Unexpected > Completions. This issue will happen in following logs: Unable to handle kernel paging request at virtual address 8800e0008010 CPU 0 kworker/0:3(131): Oops 0 pc = [] ra = [] ps = Tainted: G W pc is at si_gpu_check_soft_reset+0x3c/0x240 ra is at si_dma_is_lockup+0x34/0xd0 v0 = t0 = fff08800e0008010 t1 = 0001 t2 = 8010 t3 = fff7e3c0 t4 = fff7e3c00258 t5 = t6 = 0001 t7 = fff7ef078000 s0 = fff7e3c016e8 s1 = fff7e3c0 s2 = fff7e3c00018 s3 = fff7e3c0 s4 = fff7fff59d80 s5 = s6 = fff7ef07bd98 a0 = fff7e3c0 a1 = fff7e3c016e8 a2 = 0008 a3 = 0001 a4 = 8f5c28f5c28f5c29 a5 = 810f4338 t8 = 0275 t9 = 809b66f8 t10 = ff6769c5d964b800 t11= b886 pv = 811bea20 at = gp = 81d89690 sp = aa814126 Disabling lock debugging due to kernel taint Trace: [] si_dma_is_lockup+0x34/0xd0 [] radeon_fence_check_lockup+0xd0/0x290 [] process_one_work+0x280/0x550 [] worker_thread+0x70/0x7c0 [] worker_thread+0x130/0x7c0 [] kthread+0x200/0x210 [] worker_thread+0x0/0x7c0 [] kthread+0x14c/0x210 [] ret_from_kernel_thread+0x18/0x20 [] kthread+0x0/0x210 Code: ad3e0008 43f0074a ad7e0018 ad9e0020 8c3001e8 40230101 <8821> 4821ed21 So force lockup work queue flush to fix this problem. Acked-by: Christian König Signed-off-by: Zhenneng Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/radeon/radeon_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 4f0fbf6674316..92905ebb7b459 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1617,6 +1617,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, if (r) { /* delay GPU reset to resume */ radeon_fence_driver_force_completion(rdev, i); + } else { + /* finish executing delayed work */ + flush_delayed_work(>fence_drv[i].lockup_work); } } -- 2.35.1
[PATCH AUTOSEL 5.15 06/23] drm/amdgpu: Move psp_xgmi_terminate call from amdgpu_xgmi_remove_device to psp_hw_fini
From: YiPeng Chai [ Upstream commit 9d705d7741ae70764f3d6d87e67fad3b5c30ffd0 ] V1: The amdgpu_xgmi_remove_device function will send unload command to psp through psp ring to terminate xgmi, but psp ring has been destroyed in psp_hw_fini. V2: 1. Change the commit title. 2. Restore amdgpu_xgmi_remove_device to its original calling location. Move psp_xgmi_terminate call from amdgpu_xgmi_remove_device to psp_hw_fini. Signed-off-by: YiPeng Chai Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 57e9932d8a04e..5b41c29f3ed50 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -2729,6 +2729,9 @@ static int psp_hw_fini(void *handle) psp_rap_terminate(psp); psp_dtm_terminate(psp); psp_hdcp_terminate(psp); + + if (adev->gmc.xgmi.num_physical_nodes > 1) + psp_xgmi_terminate(psp); } psp_asd_unload(psp); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c index a799e0b1ff736..ce0b9cb61f582 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c @@ -723,7 +723,7 @@ int amdgpu_xgmi_remove_device(struct amdgpu_device *adev) amdgpu_put_xgmi_hive(hive); } - return psp_xgmi_terminate(>psp); + return 0; } static int amdgpu_xgmi_ras_late_init(struct amdgpu_device *adev) -- 2.35.1
[PATCH AUTOSEL 5.15 07/23] drm/amdgpu: Check num_gfx_rings for gfx v9_0 rb setup.
From: Candice Li [ Upstream commit c351938350ab9b5e978dede2c321da43de7eb70c ] No need to set up rb when no gfx rings. Signed-off-by: Candice Li Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index db27fcf87cd04..16cbae04078ad 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -2624,7 +2624,8 @@ static void gfx_v9_0_constants_init(struct amdgpu_device *adev) gfx_v9_0_tiling_mode_table_init(adev); - gfx_v9_0_setup_rb(adev); + if (adev->gfx.num_gfx_rings) + gfx_v9_0_setup_rb(adev); gfx_v9_0_get_cu_info(adev, >gfx.cu_info); adev->gfx.config.db_debug2 = RREG32_SOC15(GC, 0, mmDB_DEBUG2); -- 2.35.1
[PATCH AUTOSEL 5.15 04/23] drm/gem: Fix GEM handle release errors
From: Jeffy Chen [ Upstream commit ea2aa97ca37a9044ade001aef71dbc06318e8d44 ] Currently we are assuming a one to one mapping between dmabuf and GEM handle when releasing GEM handles. But that is not always true, since we would create extra handles for the GEM obj in cases like gem_open() and getfb{,2}(). A similar issue was reported at: https://lore.kernel.org/all/20211105083308.392156-1-jay...@rock-chips.com/ Another problem is that the imported dmabuf might not always have gem_obj->dma_buf set, which would cause leaks in drm_gem_remove_prime_handles(). Let's fix these for now by using handle to find the exact map to remove. Signed-off-by: Jeffy Chen Reviewed-by: Christian König Signed-off-by: Christian König Link: https://patchwork.freedesktop.org/patch/msgid/20220819072834.17888-1-jeffy.c...@rock-chips.com Signed-off-by: Sasha Levin --- drivers/gpu/drm/drm_gem.c | 17 + drivers/gpu/drm/drm_internal.h | 4 ++-- drivers/gpu/drm/drm_prime.c| 20 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6410563a9cb6f..dbd19a34b517b 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -167,21 +167,6 @@ void drm_gem_private_object_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_gem_private_object_init); -static void -drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) -{ - /* -* Note: obj->dma_buf can't disappear as long as we still hold a -* handle reference in obj->handle_count. -*/ - mutex_lock(>prime.lock); - if (obj->dma_buf) { - drm_prime_remove_buf_handle_locked(>prime, - obj->dma_buf); - } - mutex_unlock(>prime.lock); -} - /** * drm_gem_object_handle_free - release resources bound to userspace handles * @obj: GEM object to clean up. @@ -252,7 +237,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) if (obj->funcs->close) obj->funcs->close(obj, file_priv); - drm_gem_remove_prime_handles(obj, file_priv); + drm_prime_remove_buf_handle(_priv->prime, id); drm_vma_node_revoke(>vma_node, file_priv); drm_gem_object_handle_put_unlocked(obj); diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 17f3548c8ed25..d05e6a5b66873 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -74,8 +74,8 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); -void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf); +void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, +uint32_t handle); /* drm_drv.c */ struct drm_minor *drm_minor_acquire(unsigned int minor_id); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index d6c7f4f9a7a29..a350310b65d89 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -187,29 +187,33 @@ static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri return -ENOENT; } -void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf) +void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, +uint32_t handle) { struct rb_node *rb; - rb = prime_fpriv->dmabufs.rb_node; + mutex_lock(_fpriv->lock); + + rb = prime_fpriv->handles.rb_node; while (rb) { struct drm_prime_member *member; - member = rb_entry(rb, struct drm_prime_member, dmabuf_rb); - if (member->dma_buf == dma_buf) { + member = rb_entry(rb, struct drm_prime_member, handle_rb); + if (member->handle == handle) { rb_erase(>handle_rb, _fpriv->handles); rb_erase(>dmabuf_rb, _fpriv->dmabufs); - dma_buf_put(dma_buf); + dma_buf_put(member->dma_buf); kfree(member); - return; - } else if (member->dma_buf < dma_buf) { + break; + } else if (member->handle < handle) { rb = rb->rb_right; } else { rb = rb->rb_left; } } + + mutex_unlock(_fpriv->lock); } void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv) -- 2.35.1
[PATCH AUTOSEL 5.19 33/33] drm/amdgpu: mmVM_L2_CNTL3 register not initialized correctly
From: Qu Huang [ Upstream commit b8983d42524f10ac6bf35bbce6a7cc8e45f61e04 ] The mmVM_L2_CNTL3 register is not assigned an initial value Signed-off-by: Qu Huang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c index 3f44a099c52a4..3e51e773f92be 100644 --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c @@ -176,6 +176,7 @@ static void mmhub_v1_0_init_cache_regs(struct amdgpu_device *adev) tmp = REG_SET_FIELD(tmp, VM_L2_CNTL2, INVALIDATE_L2_CACHE, 1); WREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL2, tmp); + tmp = mmVM_L2_CNTL3_DEFAULT; if (adev->gmc.translate_further) { tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, BANK_SELECT, 12); tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, -- 2.35.1
[PATCH AUTOSEL 5.19 32/33] drm/amdgpu: add sdma instance check for gfx11 CGCG
From: Tim Huang [ Upstream commit 00047c3d967d7ef8adf8bac3c3579294a3bc0bb1 ] For some ASICs, like GFX IP v11.0.1, only have one SDMA instance, so not need to configure SDMA1_RLC_CGCG_CTRL for this case. Signed-off-by: Tim Huang Reviewed-by: Yifan Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c index a4a6751b1e449..30998ac47707c 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c @@ -5090,9 +5090,12 @@ static void gfx_v11_0_update_coarse_grain_clock_gating(struct amdgpu_device *ade data = REG_SET_FIELD(data, SDMA0_RLC_CGCG_CTRL, CGCG_INT_ENABLE, 1); WREG32_SOC15(GC, 0, regSDMA0_RLC_CGCG_CTRL, data); - data = RREG32_SOC15(GC, 0, regSDMA1_RLC_CGCG_CTRL); - data = REG_SET_FIELD(data, SDMA1_RLC_CGCG_CTRL, CGCG_INT_ENABLE, 1); - WREG32_SOC15(GC, 0, regSDMA1_RLC_CGCG_CTRL, data); + /* Some ASICs only have one SDMA instance, not need to configure SDMA1 */ + if (adev->sdma.num_instances > 1) { + data = RREG32_SOC15(GC, 0, regSDMA1_RLC_CGCG_CTRL); + data = REG_SET_FIELD(data, SDMA1_RLC_CGCG_CTRL, CGCG_INT_ENABLE, 1); + WREG32_SOC15(GC, 0, regSDMA1_RLC_CGCG_CTRL, data); + } } else { /* Program RLC_CGCG_CGLS_CTRL */ def = data = RREG32_SOC15(GC, 0, regRLC_CGCG_CGLS_CTRL); @@ -5121,9 +5124,12 @@ static void gfx_v11_0_update_coarse_grain_clock_gating(struct amdgpu_device *ade data &= ~SDMA0_RLC_CGCG_CTRL__CGCG_INT_ENABLE_MASK; WREG32_SOC15(GC, 0, regSDMA0_RLC_CGCG_CTRL, data); - data = RREG32_SOC15(GC, 0, regSDMA1_RLC_CGCG_CTRL); - data &= ~SDMA1_RLC_CGCG_CTRL__CGCG_INT_ENABLE_MASK; - WREG32_SOC15(GC, 0, regSDMA1_RLC_CGCG_CTRL, data); + /* Some ASICs only have one SDMA instance, not need to configure SDMA1 */ + if (adev->sdma.num_instances > 1) { + data = RREG32_SOC15(GC, 0, regSDMA1_RLC_CGCG_CTRL); + data &= ~SDMA1_RLC_CGCG_CTRL__CGCG_INT_ENABLE_MASK; + WREG32_SOC15(GC, 0, regSDMA1_RLC_CGCG_CTRL, data); + } } } -- 2.35.1
[PATCH AUTOSEL 5.19 29/33] fbdev: fbcon: Destroy mutex on freeing struct fb_info
From: Shigeru Yoshida [ Upstream commit 58559dfc1ebba2ae0c7627dc8f8991ae1984c6e3 ] It's needed to destroy bl_curve_mutex on freeing struct fb_info since the mutex is embedded in the structure and initialized when it's allocated. Signed-off-by: Shigeru Yoshida Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/core/fbsysfs.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c index c2a60b187467e..4d7f63892dcc4 100644 --- a/drivers/video/fbdev/core/fbsysfs.c +++ b/drivers/video/fbdev/core/fbsysfs.c @@ -84,6 +84,10 @@ void framebuffer_release(struct fb_info *info) if (WARN_ON(refcount_read(>count))) return; +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) + mutex_destroy(>bl_curve_mutex); +#endif + kfree(info->apertures); kfree(info); } -- 2.35.1
[PATCH AUTOSEL 5.19 30/33] fbdev: chipsfb: Add missing pci_disable_device() in chipsfb_pci_init()
From: Yang Yingliang [ Upstream commit 07c55c9803dea748d17a054000cbf1913ce06399 ] Add missing pci_disable_device() in error path in chipsfb_pci_init(). Signed-off-by: Yang Yingliang Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/chipsfb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c index 393894af26f84..2b00a9d554fc0 100644 --- a/drivers/video/fbdev/chipsfb.c +++ b/drivers/video/fbdev/chipsfb.c @@ -430,6 +430,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent) err_release_fb: framebuffer_release(p); err_disable: + pci_disable_device(dp); err_out: return rc; } -- 2.35.1
[PATCH AUTOSEL 5.19 28/33] fbdev: fb_pm2fb: Avoid potential divide by zero error
From: Letu Ren [ Upstream commit 19f953e7435644b81332dd632ba1b2d80b1e37af ] In `do_fb_ioctl()` of fbmem.c, if cmd is FBIOPUT_VSCREENINFO, var will be copied from user, then go through `fb_set_var()` and `info->fbops->fb_check_var()` which could may be `pm2fb_check_var()`. Along the path, `var->pixclock` won't be modified. This function checks whether reciprocal of `var->pixclock` is too high. If `var->pixclock` is zero, there will be a divide by zero error. So, it is necessary to check whether denominator is zero to avoid crash. As this bug is found by Syzkaller, logs are listed below. divide error in pm2fb_check_var Call Trace: fb_set_var+0x367/0xeb0 drivers/video/fbdev/core/fbmem.c:1015 do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1110 fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1189 Reported-by: Zheyu Ma Signed-off-by: Letu Ren Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/pm2fb.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c index d3be2c64f1c08..8fd79deb1e2ae 100644 --- a/drivers/video/fbdev/pm2fb.c +++ b/drivers/video/fbdev/pm2fb.c @@ -617,6 +617,11 @@ static int pm2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) return -EINVAL; } + if (!var->pixclock) { + DPRINTK("pixclock is zero\n"); + return -EINVAL; + } + if (PICOS2KHZ(var->pixclock) > PM2_MAX_PIXCLOCK) { DPRINTK("pixclock too high (%ldKHz)\n", PICOS2KHZ(var->pixclock)); -- 2.35.1
[PATCH AUTOSEL 5.19 27/33] fbdev: omapfb: Fix tests for platform_get_irq() failure
From: Yu Zhe [ Upstream commit acf4c6205e862304681234a6a4375b478af12552 ] The platform_get_irq() returns negative error codes. It can't actually return zero. Signed-off-by: Yu Zhe Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- drivers/video/fbdev/omap/omapfb_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/fbdev/omap/omapfb_main.c b/drivers/video/fbdev/omap/omapfb_main.c index 292fcb0a24fc9..6ff237cee7f87 100644 --- a/drivers/video/fbdev/omap/omapfb_main.c +++ b/drivers/video/fbdev/omap/omapfb_main.c @@ -1643,14 +1643,14 @@ static int omapfb_do_probe(struct platform_device *pdev, goto cleanup; } fbdev->int_irq = platform_get_irq(pdev, 0); - if (!fbdev->int_irq) { + if (fbdev->int_irq < 0) { dev_err(>dev, "unable to get irq\n"); r = ENXIO; goto cleanup; } fbdev->ext_irq = platform_get_irq(pdev, 1); - if (!fbdev->ext_irq) { + if (fbdev->ext_irq < 0) { dev_err(>dev, "unable to get irq\n"); r = ENXIO; goto cleanup; -- 2.35.1
[PATCH AUTOSEL 5.19 13/33] drm/radeon: add a force flush to delay work when radeon
From: Zhenneng Li [ Upstream commit f461950fdc374a3ada5a63c669d997de4600dffe ] Although radeon card fence and wait for gpu to finish processing current batch rings, there is still a corner case that radeon lockup work queue may not be fully flushed, and meanwhile the radeon_suspend_kms() function has called pci_set_power_state() to put device in D3hot state. Per PCI spec rev 4.0 on 5.3.1.4.1 D3hot State. > Configuration and Message requests are the only TLPs accepted by a Function in > the D3hot state. All other received Requests must be handled as Unsupported > Requests, > and all received Completions may optionally be handled as Unexpected > Completions. This issue will happen in following logs: Unable to handle kernel paging request at virtual address 8800e0008010 CPU 0 kworker/0:3(131): Oops 0 pc = [] ra = [] ps = Tainted: G W pc is at si_gpu_check_soft_reset+0x3c/0x240 ra is at si_dma_is_lockup+0x34/0xd0 v0 = t0 = fff08800e0008010 t1 = 0001 t2 = 8010 t3 = fff7e3c0 t4 = fff7e3c00258 t5 = t6 = 0001 t7 = fff7ef078000 s0 = fff7e3c016e8 s1 = fff7e3c0 s2 = fff7e3c00018 s3 = fff7e3c0 s4 = fff7fff59d80 s5 = s6 = fff7ef07bd98 a0 = fff7e3c0 a1 = fff7e3c016e8 a2 = 0008 a3 = 0001 a4 = 8f5c28f5c28f5c29 a5 = 810f4338 t8 = 0275 t9 = 809b66f8 t10 = ff6769c5d964b800 t11= b886 pv = 811bea20 at = gp = 81d89690 sp = aa814126 Disabling lock debugging due to kernel taint Trace: [] si_dma_is_lockup+0x34/0xd0 [] radeon_fence_check_lockup+0xd0/0x290 [] process_one_work+0x280/0x550 [] worker_thread+0x70/0x7c0 [] worker_thread+0x130/0x7c0 [] kthread+0x200/0x210 [] worker_thread+0x0/0x7c0 [] kthread+0x14c/0x210 [] ret_from_kernel_thread+0x18/0x20 [] kthread+0x0/0x210 Code: ad3e0008 43f0074a ad7e0018 ad9e0020 8c3001e8 40230101 <8821> 4821ed21 So force lockup work queue flush to fix this problem. Acked-by: Christian König Signed-off-by: Zhenneng Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/radeon/radeon_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 429644d5ddc69..9fba16cb3f1e7 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1604,6 +1604,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, if (r) { /* delay GPU reset to resume */ radeon_fence_driver_force_completion(rdev, i); + } else { + /* finish executing delayed work */ + flush_delayed_work(>fence_drv[i].lockup_work); } } -- 2.35.1
[PATCH AUTOSEL 5.19 11/33] drm/amdgpu: Check num_gfx_rings for gfx v9_0 rb setup.
From: Candice Li [ Upstream commit c351938350ab9b5e978dede2c321da43de7eb70c ] No need to set up rb when no gfx rings. Signed-off-by: Candice Li Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 5349ca4d19e38..6d8ff3b099422 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -2587,7 +2587,8 @@ static void gfx_v9_0_constants_init(struct amdgpu_device *adev) gfx_v9_0_tiling_mode_table_init(adev); - gfx_v9_0_setup_rb(adev); + if (adev->gfx.num_gfx_rings) + gfx_v9_0_setup_rb(adev); gfx_v9_0_get_cu_info(adev, >gfx.cu_info); adev->gfx.config.db_debug2 = RREG32_SOC15(GC, 0, mmDB_DEBUG2); -- 2.35.1
[PATCH AUTOSEL 5.19 12/33] drm/amdgpu: Remove the additional kfd pre reset call for sriov
From: shaoyunl [ Upstream commit 06671734881af2bcf7f453661b5f8616e32bb3fc ] The additional call is caused by merge conflict Reviewed-by: Felix Kuehling Signed-off-by: shaoyunl Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index ea2b74c0fd229..67d4a3c13ed19 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -4475,8 +4475,6 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device *adev, retry: amdgpu_amdkfd_pre_reset(adev); - amdgpu_amdkfd_pre_reset(adev); - if (from_hypervisor) r = amdgpu_virt_request_full_gpu(adev, true); else -- 2.35.1
[PATCH AUTOSEL 5.19 10/33] drm/amdgpu: fix hive reference leak when adding xgmi device
From: YiPeng Chai [ Upstream commit f5994da72ba124a3d0463672fdfbec073e3bb72f ] Only amdgpu_get_xgmi_hive but no amdgpu_put_xgmi_hive which will leak the hive reference. Signed-off-by: YiPeng Chai Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 3adebb63680e0..ea2b74c0fd229 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2482,12 +2482,14 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) if (!hive->reset_domain || !amdgpu_reset_get_reset_domain(hive->reset_domain)) { r = -ENOENT; + amdgpu_put_xgmi_hive(hive); goto init_failed; } /* Drop the early temporary reset domain we created for device */ amdgpu_reset_put_reset_domain(adev->reset_domain); adev->reset_domain = hive->reset_domain; + amdgpu_put_xgmi_hive(hive); } } -- 2.35.1
[PATCH AUTOSEL 5.19 09/33] drm/amdgpu: Move psp_xgmi_terminate call from amdgpu_xgmi_remove_device to psp_hw_fini
From: YiPeng Chai [ Upstream commit 9d705d7741ae70764f3d6d87e67fad3b5c30ffd0 ] V1: The amdgpu_xgmi_remove_device function will send unload command to psp through psp ring to terminate xgmi, but psp ring has been destroyed in psp_hw_fini. V2: 1. Change the commit title. 2. Restore amdgpu_xgmi_remove_device to its original calling location. Move psp_xgmi_terminate call from amdgpu_xgmi_remove_device to psp_hw_fini. Signed-off-by: YiPeng Chai Reviewed-by: Hawking Zhang Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index e9411c28d88ba..2b00f8fe15a89 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -2612,6 +2612,9 @@ static int psp_hw_fini(void *handle) psp_rap_terminate(psp); psp_dtm_terminate(psp); psp_hdcp_terminate(psp); + + if (adev->gmc.xgmi.num_physical_nodes > 1) + psp_xgmi_terminate(psp); } psp_asd_terminate(psp); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c index 1b108d03e7859..f2aebbf3fbe38 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c @@ -742,7 +742,7 @@ int amdgpu_xgmi_remove_device(struct amdgpu_device *adev) amdgpu_put_xgmi_hive(hive); } - return psp_xgmi_terminate(>psp); + return 0; } static int amdgpu_xgmi_ras_late_init(struct amdgpu_device *adev, struct ras_common_if *ras_block) -- 2.35.1