[PATCH v13 1/6] dt-bindings: media: add pclk-sample dual edge property
Some chips's sample mode are rising, falling and dual edge (both falling and rising edge). Extern the pclk-sample property to support dual edge. Acked-by: Rob Herring Reviewed-by: CK Hu Signed-off-by: Jitao Shi --- Documentation/devicetree/bindings/media/video-interfaces.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt index f884ada0bffc..da9ad24935db 100644 --- a/Documentation/devicetree/bindings/media/video-interfaces.txt +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt @@ -118,8 +118,8 @@ Optional endpoint properties - data-enable-active: similar to HSYNC and VSYNC, specifies the data enable signal polarity. - field-even-active: field signal level during the even field data transmission. -- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock - signal. +- pclk-sample: sample data on rising (1), falling (0) or both rising and + falling (2) edge of the pixel clock signal. - sync-on-green-active: active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. - data-lanes: an array of physical data lane indexes. Position of an entry -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v13 4/6] dt-bindings: display: mediatek: convert the document format from txt to yaml
Signed-off-by: Jitao Shi --- .../display/mediatek/mediatek,dpi.txt | 45 .../display/mediatek/mediatek,dpi.yaml| 103 ++ 2 files changed, 103 insertions(+), 45 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt deleted file mode 100644 index 2dfb50a7321e.. --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt +++ /dev/null @@ -1,45 +0,0 @@ -Mediatek DPI Device -=== - -The Mediatek DPI function block is a sink of the display subsystem and -provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel -output bus. - -Required properties: -- compatible: "mediatek,-dpi" - the supported chips are mt2701 , mt8173 and mt8183. -- reg: Physical base address and length of the controller's registers -- interrupts: The interrupt signal from the function block. -- clocks: device clocks - See Documentation/devicetree/bindings/clock/clock-bindings.txt for details. -- clock-names: must contain "pixel", "engine", and "pll" -- port: Output port node with endpoint definitions as described in - Documentation/devicetree/bindings/graph.txt. This port should be connected - to the input port of an attached HDMI or LVDS encoder chip. - -Optional properties: -- pinctrl-names: Contain "default" and "sleep". - pinctrl-names see Documentation/devicetree/bindings/pinctrlpinctrl-bindings.txt -- pclk-sample: refer Documentation/devicetree/bindings/media/video-interfaces.txt. - -Example: - -dpi0: dpi@1401d000 { - compatible = "mediatek,mt8173-dpi"; - reg = <0 0x1401d000 0 0x1000>; - interrupts = ; - clocks = <&mmsys CLK_MM_DPI_PIXEL>, -<&mmsys CLK_MM_DPI_ENGINE>, -<&apmixedsys CLK_APMIXED_TVDPLL>; - clock-names = "pixel", "engine", "pll"; - pinctrl-names = "default", "sleep"; - pinctrl-0 = <&dpi_pin_func>; - pinctrl-1 = <&dpi_pin_idle>; - - port { - dpi0_out: endpoint { - pclk-sample = <0>; - remote-endpoint = <&hdmi0_in>; - }; - }; -}; diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml new file mode 100644 index ..d65543e3bf8c --- /dev/null +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml @@ -0,0 +1,103 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: mediatek DPI Controller Device Tree Bindings + +maintainers: + - CK Hu + - Jitao shi + +description: | + The Mediatek DPI function block is a sink of the display subsystem and + provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel + output bus. + +properties: + compatible: +enum: + - mediatek,mt2701-dpi + - mediatek,mt8173-dpi + - mediatek,mt8183-dpi + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + clocks: +items: + - description: Pixel Clock + - description: Engine Clock + - description: DPI PLL + + clock-names: +items: + - const: pixel + - const: engine + - const: pll + + pinctrl-0: true + pinctrl-1: true + + pinctrl-names: +items: + - const: default + - const: sleep + + port@0: +type: object +description: + Output port node with endpoint definitions as described in + Documentation/devicetree/bindings/graph.txt. This port should be connected + to the input port of an attached HDMI or LVDS encoder chip. + +properties: + endpoint: +type: object + +properties: + pclk-sample: +items: +- description: refer Documentation/devicetree/bindings/media/video-interfaces.txt. + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - port@0 + +additionalProperties: false + +examples: + - | +#include +#include +#include +#include +dpi0: dpi@1401d000 { +compatible = "mediatek,mt8173-dpi"; +reg = <0 0x1401d000 0 0x1000>; +interrupts = ; +clocks = <&mmsys CLK_MM_DPI_PIXEL>, + <&mmsys CLK_MM_DPI_ENGINE>, + <&apmixedsys CLK_APMIXED_TVDPLL>; +clock-names = "pixel", "engine", "pll"; +pinctrl-names = "default", "sleep"; +pinctrl-0 = <&dpi_pin_func>; +pinctrl-1 = <&dpi_pin_idle>; + +port@0 { +dpi0_out: endpoint { +pclk-sample = <0>; +remote-endpoint = <&hdmi0_in>; +}; +
[PATCH v13 6/6] drm/mediatek: set dpi pin mode to gpio low to avoid leakage current
Config dpi pins mode to output and pull low when dpi is disabled. Aovid leakage current from some dpi pins (Hsync Vsync DE ... ). Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dpi.c | 31 ++ 1 file changed, 31 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 2871e68e7767..b6359e979588 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -10,7 +10,9 @@ #include #include #include +#include #include +#include #include #include @@ -74,6 +76,9 @@ struct mtk_dpi { enum mtk_dpi_out_yc_map yc_map; enum mtk_dpi_out_bit_num bit_num; enum mtk_dpi_out_channel_swap channel_swap; + struct pinctrl *pinctrl; + struct pinctrl_state *pins_gpio; + struct pinctrl_state *pins_dpi; int refcount; u32 pclk_sample; }; @@ -387,6 +392,9 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi) if (--dpi->refcount != 0) return; + if (dpi->pinctrl && dpi->pins_gpio) + pinctrl_select_state(dpi->pinctrl, dpi->pins_gpio); + mtk_dpi_disable(dpi); clk_disable_unprepare(dpi->pixel_clk); clk_disable_unprepare(dpi->engine_clk); @@ -411,6 +419,9 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi) goto err_pixel; } + if (dpi->pinctrl && dpi->pins_dpi) + pinctrl_select_state(dpi->pinctrl, dpi->pins_dpi); + mtk_dpi_enable(dpi); return 0; @@ -728,6 +739,26 @@ static int mtk_dpi_probe(struct platform_device *pdev) of_property_read_u32(ep, "pclk-sample", &dpi->pclk_sample); of_node_put(ep); + dpi->pinctrl = devm_pinctrl_get(&pdev->dev); + if (IS_ERR(dpi->pinctrl)) { + dpi->pinctrl = NULL; + dev_dbg(&pdev->dev, "Cannot find pinctrl!\n"); + } + if (dpi->pinctrl) { + dpi->pins_gpio = pinctrl_lookup_state(dpi->pinctrl, "sleep"); + if (IS_ERR(dpi->pins_gpio)) { + dpi->pins_gpio = NULL; + dev_dbg(&pdev->dev, "Cannot find pinctrl idle!\n"); + } + if (dpi->pins_gpio) + pinctrl_select_state(dpi->pinctrl, dpi->pins_gpio); + + dpi->pins_dpi = pinctrl_lookup_state(dpi->pinctrl, "default"); + if (IS_ERR(dpi->pins_dpi)) { + dpi->pins_dpi = NULL; + dev_dbg(&pdev->dev, "Cannot find pinctrl active!\n"); + } + } mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); dpi->regs = devm_ioremap_resource(dev, mem); if (IS_ERR(dpi->regs)) { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v13 2/6] dt-bindings: display: mediatek: control dpi pins mode to avoid leakage
Add property "pinctrl-names" to swap pin mode between gpio and dpi mode. Set the dpi pins to gpio mode and output-low to avoid leakage current when dpi disabled. Signed-off-by: Jitao Shi --- .../devicetree/bindings/display/mediatek/mediatek,dpi.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt index 58914cf681b8..260ae75ac640 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt @@ -17,6 +17,10 @@ Required properties: Documentation/devicetree/bindings/graph.txt. This port should be connected to the input port of an attached HDMI or LVDS encoder chip. +Optional properties: +- pinctrl-names: Contain "default" and "sleep". + pinctrl-names see Documentation/devicetree/bindings/pinctrlpinctrl-bindings.txt + Example: dpi0: dpi@1401d000 { @@ -27,6 +31,9 @@ dpi0: dpi@1401d000 { <&mmsys CLK_MM_DPI_ENGINE>, <&apmixedsys CLK_APMIXED_TVDPLL>; clock-names = "pixel", "engine", "pll"; + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&dpi_pin_func>; + pinctrl-1 = <&dpi_pin_idle>; port { dpi0_out: endpoint { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v13 5/6] drm/mediatek: dpi sample mode support
DPI can sample on falling, rising or both edge. When DPI sample the data both rising and falling edge. It can reduce half data io pins. Reviewed-by: CK Hu Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dpi.c | 27 +-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 087f5ce732e1..2871e68e7767 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -75,6 +75,7 @@ struct mtk_dpi { enum mtk_dpi_out_bit_num bit_num; enum mtk_dpi_out_channel_swap channel_swap; int refcount; + u32 pclk_sample; }; static inline struct mtk_dpi *mtk_dpi_from_encoder(struct drm_encoder *e) @@ -348,6 +349,13 @@ static void mtk_dpi_config_disable_edge(struct mtk_dpi *dpi) mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN); } +static void mtk_dpi_enable_pclk_sample_dual_edge(struct mtk_dpi *dpi) +{ + mtk_dpi_mask(dpi, DPI_DDR_SETTING, DDR_EN | DDR_4PHASE, +DDR_EN | DDR_4PHASE); + mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, EDGE_SEL, EDGE_SEL); +} + static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, enum mtk_dpi_out_color_format format) { @@ -439,7 +447,8 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, pll_rate = clk_get_rate(dpi->tvd_clk); vm.pixelclock = pll_rate / factor; - clk_set_rate(dpi->pixel_clk, vm.pixelclock); + clk_set_rate(dpi->pixel_clk, +vm.pixelclock * (dpi->pclk_sample > 1 ? 2 : 1)); vm.pixelclock = clk_get_rate(dpi->pixel_clk); dev_dbg(dpi->dev, "Got PLL %lu Hz, pixel clock %lu Hz\n", @@ -450,7 +459,8 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, limit.y_bottom = 0x0010; limit.y_top = 0x0FE0; - dpi_pol.ck_pol = MTK_DPI_POLARITY_FALLING; + dpi_pol.ck_pol = dpi->pclk_sample == 1 ? +MTK_DPI_POLARITY_RISING : MTK_DPI_POLARITY_FALLING; dpi_pol.de_pol = MTK_DPI_POLARITY_RISING; dpi_pol.hsync_pol = vm.flags & DISPLAY_FLAGS_HSYNC_HIGH ? MTK_DPI_POLARITY_FALLING : MTK_DPI_POLARITY_RISING; @@ -504,6 +514,8 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, mtk_dpi_config_color_format(dpi, dpi->color_format); mtk_dpi_config_2n_h_fre(dpi); mtk_dpi_config_disable_edge(dpi); + if (dpi->pclk_sample > 1) + mtk_dpi_enable_pclk_sample_dual_edge(dpi); mtk_dpi_sw_reset(dpi, false); return 0; @@ -693,6 +705,7 @@ static const struct mtk_dpi_conf mt8183_conf = { static int mtk_dpi_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; + struct device_node *ep; struct mtk_dpi *dpi; struct resource *mem; int comp_id; @@ -705,6 +718,16 @@ static int mtk_dpi_probe(struct platform_device *pdev) dpi->dev = dev; dpi->conf = (struct mtk_dpi_conf *)of_device_get_match_data(dev); + ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0); + if (!ep) { + dev_err(dev, "Failed get the endpoint port\n"); + return -EINVAL; + } + + /* Get the sampling edge from the endpoint. */ + of_property_read_u32(ep, "pclk-sample", &dpi->pclk_sample); + of_node_put(ep); + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); dpi->regs = devm_ioremap_resource(dev, mem); if (IS_ERR(dpi->regs)) { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v13 3/6] dt-bindings: display: mediatek: dpi sample data in dual edge support
Add property "pclk-sample" to config the dpi sample on falling (0), rising (1), both falling and rising (2). Acked-by: Rob Herring Signed-off-by: Jitao Shi --- .../devicetree/bindings/display/mediatek/mediatek,dpi.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt index 260ae75ac640..2dfb50a7321e 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt @@ -20,6 +20,7 @@ Required properties: Optional properties: - pinctrl-names: Contain "default" and "sleep". pinctrl-names see Documentation/devicetree/bindings/pinctrlpinctrl-bindings.txt +- pclk-sample: refer Documentation/devicetree/bindings/media/video-interfaces.txt. Example: @@ -37,6 +38,7 @@ dpi0: dpi@1401d000 { port { dpi0_out: endpoint { + pclk-sample = <0>; remote-endpoint = <&hdmi0_in>; }; }; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v13 0/6] mt8183 dpi supports dual edge and pin mode swap
Changes since v12: - fix mediatek,dpi.yaml make_dt_binding_check errors. Change since v11: - fine tune mediatek,dpi.yaml. - add Acked-by: Rob Herring . Change since v10: - convert the Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt to yaml format. - read the pclk-sample in endpoint. Changes since v9: - rename pinctrl-names = "gpiomode", "dpimode" to "active", "idle". - fix some typo. Changes since v8: - drop pclk-sample redefine in mediatek,dpi.txt - only get the gpiomode and dpimode when dpi->pinctrl is successful. Changes since v7: - separate dt-bindings to independent patches. - move dpi dual edge to one patch. Changes since v6: - change dual_edge to pclk-sample - remove dpi_pin_mode_swap and Changes since v5: - fine tune the dt-bindings commit message. Changes since v4: - move pin mode control and dual edge control to deveice tree. - update dt-bindings document for pin mode swap and dual edge control. Changes since v3: - add dpi pin mode control when dpi on or off. - update dpi dual edge comment. Changes since v2: - update dt-bindings document for mt8183 dpi. - separate dual edge modfication as independent patch. Jitao Shi (6): dt-bindings: media: add pclk-sample dual edge property dt-bindings: display: mediatek: control dpi pins mode to avoid leakage dt-bindings: display: mediatek: dpi sample data in dual edge support dt-bindings: display: mediatek: convert the document format from txt to yaml drm/mediatek: dpi sample mode support drm/mediatek: set dpi pin mode to gpio low to avoid leakage current .../display/mediatek/mediatek,dpi.txt | 36 -- .../display/mediatek/mediatek,dpi.yaml| 103 ++ .../bindings/media/video-interfaces.txt | 4 +- drivers/gpu/drm/mediatek/mtk_dpi.c| 58 +- 4 files changed, 161 insertions(+), 40 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: Use scnprintf() for avoiding potential buffer overflow
Since snprintf() returns the would-be-output size instead of the actual output size, the succeeding calls may go beyond the given buffer limit. Fix it by replacing with scnprintf(). Signed-off-by: Takashi Iwai --- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index bf876faea592..faefaaef7909 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -604,7 +604,7 @@ static struct dma_pool *ttm_dma_pool_init(struct device *dev, gfp_t flags, p = pool->name; for (i = 0; i < ARRAY_SIZE(t); i++) { if (type & t[i]) { - p += snprintf(p, sizeof(pool->name) - (p - pool->name), + p += scnprintf(p, sizeof(pool->name) - (p - pool->name), "%s", n[i]); } } -- 2.16.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: sysfs: Use scnprintf() for avoiding potential buffer overflow
Since snprintf() returns the would-be-output size instead of the actual output size, the succeeding calls may go beyond the given buffer limit. Fix it by replacing with scnprintf(). Signed-off-by: Takashi Iwai --- drivers/gpu/drm/drm_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index dd2bc85f43cc..9b3180e8c12f 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -230,7 +230,7 @@ static ssize_t modes_show(struct device *device, mutex_lock(&connector->dev->mode_config.mutex); list_for_each_entry(mode, &connector->modes, head) { - written += snprintf(buf + written, PAGE_SIZE - written, "%s\n", + written += scnprintf(buf + written, PAGE_SIZE - written, "%s\n", mode->name); } mutex_unlock(&connector->dev->mode_config.mutex); -- 2.16.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/4] dt-bindings: display: mediatek: get mipitx calibration data from nvmem
Add properties to get get mipitx calibration data. Signed-off-by: Jitao Shi --- .../devicetree/bindings/display/mediatek/mediatek,dsi.txt| 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt index d501f9ff4b1f..b179293c43de 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt @@ -34,6 +34,9 @@ Required properties: - #phy-cells: must be <0>. Optional properties: +- nvmem-cells: A phandle to the calibration data provided by a nvmem device. If + unspecified default values shall be used. +- nvmem-cell-names: Should be "calibration-data" - drive-strength-microamp: adjust driving current, should be 1 ~ 0xF Example: @@ -45,6 +48,8 @@ mipi_tx0: mipi-dphy@10215000 { clock-output-names = "mipi_tx0_pll"; #clock-cells = <0>; #phy-cells = <0>; + nvmem-cells= <&mipi_tx_calibration>; + nvmem-cell-names = "calibration-data"; drive-strength-microamp = <0x8>; }; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/4] drm/mediatek: add the mipitx driving control
Add a property in device tree to control the driving by different board. Reviewed-by: Matthias Brugger Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_mipi_tx.c| 6 ++ drivers/gpu/drm/mediatek/mtk_mipi_tx.h| 1 + drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c | 7 +++ 3 files changed, 14 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c index e4d34484ecc8..2a1ac4e97cbb 100644 --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c @@ -125,6 +125,12 @@ static int mtk_mipi_tx_probe(struct platform_device *pdev) return ret; } + ret = of_property_read_u32(dev->of_node, "drive-strength-microamp", + &mipi_tx->mipitx_drive); + /* If can't get the "mipi_tx->mipitx_drive", set it default 0x8 */ + if (ret < 0) + mipi_tx->mipitx_drive = 0x8; + ref_clk_name = __clk_get_name(ref_clk); ret = of_property_read_string(dev->of_node, "clock-output-names", diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.h b/drivers/gpu/drm/mediatek/mtk_mipi_tx.h index 413f35d86219..eea44327fe9f 100644 --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.h +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.h @@ -27,6 +27,7 @@ struct mtk_mipi_tx { struct device *dev; void __iomem *regs; u32 data_rate; + u32 mipitx_drive; const struct mtk_mipitx_data *driver_data; struct clk_hw pll_hw; struct clk *pll; diff --git a/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c index 91f08a351fd0..124fdf95f1e5 100644 --- a/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c +++ b/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c @@ -17,6 +17,9 @@ #define RG_DSI_BG_CORE_EN BIT(7) #define RG_DSI_PAD_TIEL_SELBIT(8) +#define MIPITX_VOLTAGE_SEL 0x0010 +#define RG_DSI_HSTX_LDO_REF_SEL(0xf << 6) + #define MIPITX_PLL_PWR 0x0028 #define MIPITX_PLL_CON00x002c #define MIPITX_PLL_CON10x0030 @@ -123,6 +126,10 @@ static void mtk_mipi_tx_power_on_signal(struct phy *phy) mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_D3_SW_CTL_EN, DSI_SW_CTL_EN); mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_CK_SW_CTL_EN, DSI_SW_CTL_EN); + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_VOLTAGE_SEL, + RG_DSI_HSTX_LDO_REF_SEL, + mipi_tx->mipitx_drive << 6); + mtk_mipi_tx_set_bits(mipi_tx, MIPITX_CK_CKMODE_EN, DSI_CK_CKMODE_EN); } -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/4] dt-bindings: display: mediatek: add property to control mipi tx drive current
Add a property to control mipi tx drive current: "drive-strength-microamp" Signed-off-by: Jitao Shi --- .../devicetree/bindings/display/mediatek/mediatek,dsi.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt index a19a6cc375ed..d501f9ff4b1f 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt @@ -33,6 +33,9 @@ Required properties: - #clock-cells: must be <0>; - #phy-cells: must be <0>. +Optional properties: +- drive-strength-microamp: adjust driving current, should be 1 ~ 0xF + Example: mipi_tx0: mipi-dphy@10215000 { @@ -42,6 +45,7 @@ mipi_tx0: mipi-dphy@10215000 { clock-output-names = "mipi_tx0_pll"; #clock-cells = <0>; #phy-cells = <0>; + drive-strength-microamp = <0x8>; }; dsi0: dsi@1401b000 { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/4] drm/mediatek: config mipitx impedance with calibration data
Read calibration data from nvmem, and config mipitx impedance with calibration data to make sure their impedance are 100ohm. Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c | 57 +++ 1 file changed, 57 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c index 124fdf95f1e5..878feeb7ac6c 100644 --- a/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c +++ b/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c @@ -5,6 +5,8 @@ */ #include "mtk_mipi_tx.h" +#include +#include #define MIPITX_LANE_CON0x000c #define RG_DSI_CPHY_T1DRV_EN BIT(0) @@ -28,6 +30,7 @@ #define MIPITX_PLL_CON40x003c #define RG_DSI_PLL_IBIAS (3 << 10) +#define MIPITX_D2P_RTCODE 0x0100 #define MIPITX_D2_SW_CTL_EN0x0144 #define MIPITX_D0_SW_CTL_EN0x0244 #define MIPITX_CK_CKMODE_EN0x0328 @@ -108,6 +111,58 @@ static const struct clk_ops mtk_mipi_tx_pll_ops = { .recalc_rate = mtk_mipi_tx_pll_recalc_rate, }; +static void mtk_mipi_tx_config_calibration_data(struct mtk_mipi_tx *mipi_tx) +{ + u32 *buf; + u32 rt_code[5]; + int i, j; + struct nvmem_cell *cell; + struct device *dev = mipi_tx->dev; + size_t len; + + cell = nvmem_cell_get(dev, "calibration-data"); + if (IS_ERR(cell)) { + dev_info(dev, "nvmem_cell_get fail\n"); + return; + } + + buf = (u32 *)nvmem_cell_read(cell, &len); + + nvmem_cell_put(cell); + + if (IS_ERR(buf)) { + dev_info(dev, "can't get data\n"); + return; + } + + if (len < 3 * sizeof(u32)) { + dev_info(dev, "invalid calibration data\n"); + kfree(buf); + return; + } + + rt_code[0] = ((buf[0] >> 6 & 0x1f) << 5) | (buf[0] >> 11 & 0x1f); + rt_code[1] = ((buf[1] >> 27 & 0x1f) << 5) | (buf[0] >> 1 & 0x1f); + rt_code[2] = ((buf[1] >> 17 & 0x1f) << 5) | (buf[1] >> 22 & 0x1f); + rt_code[3] = ((buf[1] >> 7 & 0x1f) << 5) | (buf[1] >> 12 & 0x1f); + rt_code[4] = ((buf[2] >> 27 & 0x1f) << 5) | (buf[1] >> 2 & 0x1f); + + for (i = 0; i < 5; i++) { + if ((rt_code[i] & 0x1f) == 0) + rt_code[i] |= 0x10; + + if ((rt_code[i] >> 5 & 0x1f) == 0) + rt_code[i] |= 0x10 << 5; + + for (j = 0; j < 10; j++) + mtk_mipi_tx_update_bits(mipi_tx, + MIPITX_D2P_RTCODE * (i + 1) + j * 4, + 1, rt_code[i] >> j & 1); + } + + kfree(buf); +} + static void mtk_mipi_tx_power_on_signal(struct phy *phy) { struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy); @@ -130,6 +185,8 @@ static void mtk_mipi_tx_power_on_signal(struct phy *phy) RG_DSI_HSTX_LDO_REF_SEL, mipi_tx->mipitx_drive << 6); + mtk_mipi_tx_config_calibration_data(mipi_tx); + mtk_mipi_tx_set_bits(mipi_tx, MIPITX_CK_CKMODE_EN, DSI_CK_CKMODE_EN); } -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/4] Config mipi tx drive current and impedance
Changes since v2: - fix the title of commit message. - rename mipitx-current-drive to drive-strength-microamp Changes since v1: - fix coding style. - change mtk_mipi_tx_config_calibration_data() to void Jitao Shi (4): dt-bindings: display: mediatek: add property to control mipi tx drive current dt-bindings: display: mediatek: get mipitx calibration data from nvmem drm/mediatek: add the mipitx driving control drm/mediatek: config mipitx impedance with calibration data .../display/mediatek/mediatek,dsi.txt | 9 +++ drivers/gpu/drm/mediatek/mtk_mipi_tx.c| 6 ++ drivers/gpu/drm/mediatek/mtk_mipi_tx.h| 1 + drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c | 64 +++ 4 files changed, 80 insertions(+) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: Use scnprintf() for avoiding potential buffer overflow
On Wed, Mar 11, 2020 at 03:34:52PM +0800, Takashi Iwai wrote: > Since snprintf() returns the would-be-output size instead of the > actual output size, the succeeding calls may go beyond the given > buffer limit. Fix it by replacing with scnprintf(). > > Signed-off-by: Takashi Iwai Reviewed-by: Huang Rui > --- > drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > index bf876faea592..faefaaef7909 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > @@ -604,7 +604,7 @@ static struct dma_pool *ttm_dma_pool_init(struct device > *dev, gfp_t flags, > p = pool->name; > for (i = 0; i < ARRAY_SIZE(t); i++) { > if (type & t[i]) { > - p += snprintf(p, sizeof(pool->name) - (p - pool->name), > + p += scnprintf(p, sizeof(pool->name) - (p - pool->name), > "%s", n[i]); > } > } > -- > 2.16.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: Use scnprintf() for avoiding potential buffer overflow
Am 11.03.20 um 08:52 schrieb Huang Rui: On Wed, Mar 11, 2020 at 03:34:52PM +0800, Takashi Iwai wrote: Since snprintf() returns the would-be-output size instead of the actual output size, the succeeding calls may go beyond the given buffer limit. Fix it by replacing with scnprintf(). Signed-off-by: Takashi Iwai Reviewed-by: Huang Rui Reviewed-by: Christian König Takashi, should I push this to drm-misc-next or do you want to merge that somehow else? Thanks, Christian. --- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index bf876faea592..faefaaef7909 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -604,7 +604,7 @@ static struct dma_pool *ttm_dma_pool_init(struct device *dev, gfp_t flags, p = pool->name; for (i = 0; i < ARRAY_SIZE(t); i++) { if (type & t[i]) { - p += snprintf(p, sizeof(pool->name) - (p - pool->name), + p += scnprintf(p, sizeof(pool->name) - (p - pool->name), "%s", n[i]); } } -- 2.16.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: Use scnprintf() for avoiding potential buffer overflow
On Wed, 11 Mar 2020 08:56:11 +0100, Christian K6nig wrote: > > Am 11.03.20 um 08:52 schrieb Huang Rui: > > On Wed, Mar 11, 2020 at 03:34:52PM +0800, Takashi Iwai wrote: > >> Since snprintf() returns the would-be-output size instead of the > >> actual output size, the succeeding calls may go beyond the given > >> buffer limit. Fix it by replacing with scnprintf(). > >> > >> Signed-off-by: Takashi Iwai > > Reviewed-by: Huang Rui > > Reviewed-by: Christian König > > Takashi, should I push this to drm-misc-next or do you want to merge > that somehow else? Please take through your tree as you like. Thanks! Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: sysfs: Use scnprintf() for avoiding potential buffer overflow
Hi Takashi Am 11.03.20 um 08:35 schrieb Takashi Iwai: > Since snprintf() returns the would-be-output size instead of the > actual output size, the succeeding calls may go beyond the given > buffer limit. Fix it by replacing with scnprintf(). > > Signed-off-by: Takashi Iwai > --- > drivers/gpu/drm/drm_sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index dd2bc85f43cc..9b3180e8c12f 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -230,7 +230,7 @@ static ssize_t modes_show(struct device *device, > > mutex_lock(&connector->dev->mode_config.mutex); > list_for_each_entry(mode, &connector->modes, head) { > - written += snprintf(buf + written, PAGE_SIZE - written, "%s\n", > + written += scnprintf(buf + written, PAGE_SIZE - written, "%s\n", > mode->name); > } > mutex_unlock(&connector->dev->mode_config.mutex); > In drm_sysfs.c, there are more _show functions with calls to snprintf() that could be replaced by scnprintf(). ATM they don't return the correct length for output that exceeds PAGE_SIZE. since you're at it, you may replace them as well. But in any case Reviewed-by: Thomas Zimmermann for this patch. Do you want me to merge the patch into drm-misc-next? Best regards Thomas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/17] drm/i915: have *_debugfs_init() functions return void.
On Tue, 10 Mar 2020, Wambui Karuga wrote: > Since commit 987d65d01356 (drm: debugfs: make > drm_debugfs_create_files() never fail), drm_debugfs_create_files() never > fails and should return void. Therefore, remove its use as the > return value of debugfs_init() functions and have the functions return > void. > > v2: convert intel_display_debugfs_register() stub to return void too. > > Signed-off-by: Wambui Karuga Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/display/intel_display_debugfs.c | 8 > drivers/gpu/drm/i915/display/intel_display_debugfs.h | 4 ++-- > drivers/gpu/drm/i915/i915_debugfs.c | 8 > drivers/gpu/drm/i915/i915_debugfs.h | 4 ++-- > 4 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 1e6eb7f2f72d..424f4e52f783 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -1927,7 +1927,7 @@ static const struct { > {"i915_edp_psr_debug", &i915_edp_psr_debug_fops}, > }; > > -int intel_display_debugfs_register(struct drm_i915_private *i915) > +void intel_display_debugfs_register(struct drm_i915_private *i915) > { > struct drm_minor *minor = i915->drm.primary; > int i; > @@ -1940,9 +1940,9 @@ int intel_display_debugfs_register(struct > drm_i915_private *i915) > intel_display_debugfs_files[i].fops); > } > > - return drm_debugfs_create_files(intel_display_debugfs_list, > - ARRAY_SIZE(intel_display_debugfs_list), > - minor->debugfs_root, minor); > + drm_debugfs_create_files(intel_display_debugfs_list, > + ARRAY_SIZE(intel_display_debugfs_list), > + minor->debugfs_root, minor); > } > > static int i915_panel_show(struct seq_file *m, void *data) > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.h > b/drivers/gpu/drm/i915/display/intel_display_debugfs.h > index a3bea1ce04c2..c922c1745bfe 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.h > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.h > @@ -10,10 +10,10 @@ struct drm_connector; > struct drm_i915_private; > > #ifdef CONFIG_DEBUG_FS > -int intel_display_debugfs_register(struct drm_i915_private *i915); > +void intel_display_debugfs_register(struct drm_i915_private *i915); > int intel_connector_debugfs_add(struct drm_connector *connector); > #else > -static inline int intel_display_debugfs_register(struct drm_i915_private > *i915) { return 0; } > +static inline void intel_display_debugfs_register(struct drm_i915_private > *i915) {} > static inline int intel_connector_debugfs_add(struct drm_connector > *connector) { return 0; } > #endif > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 8f2525e4ce0f..de313199c714 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2392,7 +2392,7 @@ static const struct i915_debugfs_files { > {"i915_guc_log_relay", &i915_guc_log_relay_fops}, > }; > > -int i915_debugfs_register(struct drm_i915_private *dev_priv) > +void i915_debugfs_register(struct drm_i915_private *dev_priv) > { > struct drm_minor *minor = dev_priv->drm.primary; > int i; > @@ -2409,7 +2409,7 @@ int i915_debugfs_register(struct drm_i915_private > *dev_priv) > i915_debugfs_files[i].fops); > } > > - return drm_debugfs_create_files(i915_debugfs_list, > - I915_DEBUGFS_ENTRIES, > - minor->debugfs_root, minor); > + drm_debugfs_create_files(i915_debugfs_list, > + I915_DEBUGFS_ENTRIES, > + minor->debugfs_root, minor); > } > diff --git a/drivers/gpu/drm/i915/i915_debugfs.h > b/drivers/gpu/drm/i915/i915_debugfs.h > index 6da39c76ab5e..1de2736f1248 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.h > +++ b/drivers/gpu/drm/i915/i915_debugfs.h > @@ -12,10 +12,10 @@ struct drm_i915_private; > struct seq_file; > > #ifdef CONFIG_DEBUG_FS > -int i915_debugfs_register(struct drm_i915_private *dev_priv); > +void i915_debugfs_register(struct drm_i915_private *dev_priv); > void i915_debugfs_describe_obj(struct seq_file *m, struct > drm_i915_gem_object *obj); > #else > -static inline int i915_debugfs_register(struct drm_i915_private *dev_priv) { > return 0; } > +static inline void i915_debugfs_register(struct drm_i915_private *dev_priv) > {} > static inline void i915_debugfs_describe_obj(struct seq_file *m, struct > drm_i915_gem_object *obj) {} > #endif -- Jani Nikula, Intel Open Source Graphics Center __
Re: [PATCH] drm: sysfs: Use scnprintf() for avoiding potential buffer overflow
On Wed, 11 Mar 2020 09:10:56 +0100, Thomas Zimmermann wrote: > > Hi Takashi > > Am 11.03.20 um 08:35 schrieb Takashi Iwai: > > Since snprintf() returns the would-be-output size instead of the > > actual output size, the succeeding calls may go beyond the given > > buffer limit. Fix it by replacing with scnprintf(). > > > > Signed-off-by: Takashi Iwai > > --- > > drivers/gpu/drm/drm_sysfs.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > > index dd2bc85f43cc..9b3180e8c12f 100644 > > --- a/drivers/gpu/drm/drm_sysfs.c > > +++ b/drivers/gpu/drm/drm_sysfs.c > > @@ -230,7 +230,7 @@ static ssize_t modes_show(struct device *device, > > > > mutex_lock(&connector->dev->mode_config.mutex); > > list_for_each_entry(mode, &connector->modes, head) { > > - written += snprintf(buf + written, PAGE_SIZE - written, "%s\n", > > + written += scnprintf(buf + written, PAGE_SIZE - written, "%s\n", > > mode->name); > > } > > mutex_unlock(&connector->dev->mode_config.mutex); > > > > In drm_sysfs.c, there are more _show functions with calls to snprintf() > that could be replaced by scnprintf(). ATM they don't return the correct > length for output that exceeds PAGE_SIZE. since you're at it, you may > replace them as well. Well, the rest snprintf() calls are single calls and can't be over PAGE_SIZE obviously. IOW, they could be rather replaced with sprintf() instead, for a sake of simplicity. > But in any case > > Reviewed-by: Thomas Zimmermann > > for this patch. > > Do you want me to merge the patch into drm-misc-next? Yes, please. thanks, Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 02/12] drm/i915: Use 64-bit division macro
Since the PWM framework is switching struct pwm_state.duty_cycle's datatype to u64, prepare for this transition by using DIV_ROUND_UP_ULL to handle a 64-bit dividend. Cc: Jani Nikula Cc: Joonas Lahtinen Cc: David Airlie Cc: Daniel Vetter Cc: Chris Wilson Cc: "Ville Syrjälä" Cc: intel-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Guru Das Srinagesh --- drivers/gpu/drm/i915/display/intel_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index bc14e9c..843cac1 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -1868,7 +1868,7 @@ static int pwm_setup_backlight(struct intel_connector *connector, panel->backlight.min = 0; /* 0% */ panel->backlight.max = 100; /* 100% */ - panel->backlight.level = DIV_ROUND_UP( + panel->backlight.level = DIV_ROUND_UP_ULL( pwm_get_duty_cycle(panel->backlight.pwm) * 100, CRC_PMIC_PWM_PERIOD_NS); panel->backlight.enabled = panel->backlight.level != 0; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format
On 2020-03-10 20:31, Daniel Thompson wrote: On Mon, Mar 09, 2020 at 06:55:59PM +0530, Kiran Gunda wrote: Convert the qcom-wled bindings from .txt to .yaml format. Signed-off-by: Kiran Gunda Acked-by: Daniel Thompson Thanks. --- .../bindings/leds/backlight/qcom-wled.txt | 154 - .../bindings/leds/backlight/qcom-wled.yaml | 184 + 2 files changed, 184 insertions(+), 154 deletions(-) delete mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt create mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt deleted file mode 100644 index c06863b..000 --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt +++ /dev/null @@ -1,154 +0,0 @@ -Binding for Qualcomm Technologies, Inc. WLED driver - -WLED (White Light Emitting Diode) driver is used for controlling display -backlight that is part of PMIC on Qualcomm Technologies, Inc. reference -platforms. The PMIC is connected to the host processor via SPMI bus. - -- compatible - Usage:required - Value type: - Definition: should be one of: - "qcom,pm8941-wled" - "qcom,pmi8998-wled" - "qcom,pm660l-wled" - -- reg - Usage:required - Value type: - Definition: Base address of the WLED modules. - -- default-brightness - Usage:optional - Value type: - Definition: brightness value on boot, value from: 0-4095. - Default: 2048 - -- label - Usage:required - Value type: - Definition: The name of the backlight device - -- qcom,cs-out - Usage:optional - Value type: - Definition: enable current sink output. - This property is supported only for PM8941. - -- qcom,cabc - Usage:optional - Value type: - Definition: enable content adaptive backlight control. - -- qcom,ext-gen - Usage:optional - Value type: - Definition: use externally generated modulator signal to dim. - This property is supported only for PM8941. - -- qcom,current-limit - Usage:optional - Value type: - Definition: mA; per-string current limit; value from 0 to 25 with - 1 mA step. Default 20 mA. - This property is supported only for pm8941. - -- qcom,current-limit-microamp - Usage:optional - Value type: - Definition: uA; per-string current limit; value from 0 to 3 with - 2500 uA step. Default 25 mA. - -- qcom,current-boost-limit - Usage:optional - Value type: - Definition: mA; boost current limit. - For pm8941: one of: 105, 385, 525, 805, 980, 1260, 1400, - 1680. Default: 805 mA. - For pmi8998: one of: 105, 280, 450, 620, 970, 1150, 1300, - 1500. Default: 970 mA. - -- qcom,switching-freq - Usage:optional - Value type: -Definition: kHz; switching frequency; one of: 600, 640, 685, 738, - 800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200, - 4800, 9600. - Default: for pm8941: 1600 kHz - for pmi8998: 800 kHz - -- qcom,ovp - Usage:optional - Value type: - Definition: V; Over-voltage protection limit; one of: - 27, 29, 32, 35. Default: 29V - This property is supported only for PM8941. - -- qcom,ovp-millivolt - Usage:optional - Value type: - Definition: mV; Over-voltage protection limit; - For pmi8998: one of 18100, 19600, 29600, 31100. - Default 29600 mV. - If this property is not specified for PM8941, it - falls back to "qcom,ovp" property. - -- qcom,num-strings - Usage:optional - Value type: - Definition: #; number of led strings attached; - value: For PM8941 from 1 to 3. Default: 2 -For PMI8998 from 1 to 4. - -- interrupts - Usage:optional - Value type: - Definition: Interrupts associated with WLED. This should be - "short" and "ovp" interrupts. Interrupts can be - specified as per the encoding listed under - Documentation/devicetree/bindings/spmi/ - qcom,spmi-pmic-arb.txt. - -- interrupt-names - Usage:optional - Value type: - Definition: Interrupt names associa
[PATCH v8 11/12] backlight: pwm_bl: Use 64-bit division function
Since the PWM framework is switching struct pwm_state.period's datatype to u64, prepare for this transition by using div_u64 to handle a 64-bit dividend instead of a straight division operation. Cc: Lee Jones Cc: Daniel Thompson Cc: Jingoo Han Cc: Bartlomiej Zolnierkiewicz Cc: linux-...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-fb...@vger.kernel.org Signed-off-by: Guru Das Srinagesh --- drivers/video/backlight/pwm_bl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index efb4efc..3e5dbcf 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -625,7 +625,8 @@ static int pwm_backlight_probe(struct platform_device *pdev) pb->scale = data->max_brightness; } - pb->lth_brightness = data->lth_brightness * (state.period / pb->scale); + pb->lth_brightness = data->lth_brightness * (div_u64(state.period, + pb->scale)); props.type = BACKLIGHT_RAW; props.max_brightness = data->max_brightness; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next 013/491] INGENIC JZ47xx SoCs: Use fallthrough;
Hi Joe, Joe Perches wrote on Tue, 10 Mar 2020 21:51:27 -0700: > Convert the various uses of fallthrough comments to fallthrough; > > Done via script > Link: > https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/ > > Signed-off-by: Joe Perches > --- > drivers/gpu/drm/ingenic/ingenic-drm.c | 2 +- > drivers/mmc/host/jz4740_mmc.c | 6 ++ > drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c | 2 +- > drivers/mtd/nand/raw/ingenic/jz4725b_bch.c | 4 ++-- > drivers/mtd/nand/raw/ingenic/jz4780_bch.c | 4 ++-- > sound/soc/codecs/jz4770.c | 2 +- > 6 files changed, 9 insertions(+), 11 deletions(-) I like very much the new way to advertise for fallthrough statements, but I am not willing to take any patch converting a single driver anymore. I had too many from Gustavo when these comments had to be inserted. I would really prefer a MTD-wide or a NAND-wide or at least a raw-NAND-wide single patch (anything inside drivers/mtd/nand/raw/). Hope you'll understand! Thanks, Miquèl ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 000/491] treewide: use fallthrough;
There is a new fallthrough pseudo-keyword macro that can be used to replace the various /* fallthrough */ style comments that are used to indicate a case label code block is intended to fallthrough to the next case label block. See commit 294f69e662d1 ("compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use") These patches are intended to allow clang to detect missing switch/case fallthrough uses. Do a depth-first pass on the MAINTAINERS file and find the various F: pattern files and convert the fallthrough comments to fallthrough; for all files matched by all F: patterns in in each section. Done via the perl script below and the previously posted cvt_fallthrough.pl script. Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/ These patches are based on next-20200310 and are available in git://repo.or.cz/linux-2.6/trivial-mods.git in branch 20200310_fallthrough_2 $ cat commit_fallthrough.pl #!/usr/bin/env perl use sort 'stable'; # # Reorder a sorted array so file entries are before directory entries # depends on a trailing / for directories # so: # foo/ # foo/bar.c # becomes # foo/bar.c # foo/ # sub file_before_directory { my ($array_ref) = (@_); my $count = scalar(@$array_ref); for (my $i = 1; $i < $count; $i++) { if (substr(@$array_ref[$i - 1], -1) eq '/' && substr(@$array_ref[$i], 0, length(@$array_ref[$i - 1])) eq @$array_ref[$i - 1]) { my $string = @$array_ref[$i - 1]; @$array_ref[$i - 1] = @$array_ref[$i]; @$array_ref[$i] = $string; } } } sub uniq { my (@parms) = @_; my %saw; @parms = grep(!$saw{$_}++, @parms); return @parms; } # Get all the F: file patterns in MAINTAINERS that could be a .[ch] file my $maintainer_patterns = `grep -P '^F:\\s+' MAINTAINERS`; my @patterns = split('\n', $maintainer_patterns); s/^F:\s*// for @patterns; @patterns = grep(!/^(?:Documentation|tools|scripts)\//, @patterns); @patterns = grep(!/\.(?:dtsi?|rst|config)$/, @patterns); @patterns = sort @patterns; @patterns = sort { $b =~ tr/\//\// cmp $a =~ tr/\//\// } @patterns; file_before_directory(\@patterns); my %sections_done; foreach my $pattern (@patterns) { # Find the files the pattern matches my $pattern_files = `git ls-files -- $pattern`; my @new_patterns = split('\n', $pattern_files); $pattern_files = join(' ', @new_patterns); next if ($pattern_files =~ /^\s*$/); # Find the section the first file matches my $pattern_file = @new_patterns[0]; my $section_output = `./scripts/get_maintainer.pl --nogit --nogit-fallback --sections --pattern-depth=1 $pattern_file`; my @section = split('\n', $section_output); my $section_header = @section[0]; print("Section: <$section_header>\n"); # Skip the section if it's already done print("Already done '$section_header'\n") if ($sections_done{$section_header}); next if ($sections_done{$section_header}++); # Find all the .[ch] files in all F: lines in that section my @new_section; foreach my $line (@section) { last if ($line =~ /^\s*$/); push(@new_section, $line); } @section = grep(/^F:/, @new_section); s/^F:\s*// for @section; @section = grep(!/^(?:Documentation|tools|scripts)\//, @section); @section = grep(!/\.(?:dtsi?|rst|config)$/, @section); @section = sort @section; @section = uniq(@section); my $section_files = join(' ', @section); print("section_files: <$section_files>\n"); next if ($section_files =~ /^\s*$/); my $cvt_files = `git ls-files -- $section_files`; my @files = split('\n', $cvt_files); @files = grep(!/^(?:Documentation|tools|scripts)\//, @files); @files = grep(!/\.(?:dtsi?|rst|config)$/, @files); @files = grep(/\.[ch]$/, @files); @files = sort @files; @files = uniq(@files); $cvt_files = join(' ', @files); print("files: <$cvt_files>\n"); next if (scalar(@files) < 1); # Convert fallthroughs for all [.ch] files in the section print("doing cvt_fallthrough.pl -- $cvt_files\n"); `cvt_fallthrough.pl -- $cvt_files`; # If nothing changed, nothing to commit `git diff-index --quiet HEAD --`; next if (!$?); # Commit the changes my $fh; open($fh, "+>", "cvt_fallthrough.commit_msg") or die "$0: can't create temporary file: $!\n"; print $fh
Re: [PATCH V3 3/4] backlight: qcom-wled: Add support for WLED5 peripheral in PM8150L
On 2020-03-10 21:15, Daniel Thompson wrote: On Mon, Mar 09, 2020 at 06:56:01PM +0530, Kiran Gunda wrote: Add support for WLED5 peripheral that is present on PM8150L PMICs. PM8150L WLED supports the following: - Two modulators and each sink can use any of the modulator - Multiple CABC selection options - Multiple brightness width selection (12 bits to 15 bits) Signed-off-by: Kiran Gunda Mostly just style comments below... --- .../bindings/leds/backlight/qcom-wled.yaml | 39 +++ drivers/video/backlight/qcom-wled.c| 336 - Shouldn't the bindings and driver be separate? Ok. I will split it out in to a separate patch in next post. 2 files changed, 374 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml index d334f81..e0dadc4 100644 --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml @@ -20,6 +20,7 @@ properties: - qcom,pm8941-wled - qcom,pmi8998-wled - qcom,pm660l-wled + - qcom,pm8150l-wled reg: maxItems: 1 @@ -28,10 +29,23 @@ properties: maxItems: 1 description: brightness value on boot, value from 0-4095. + For pm8150l this value vary from 0-4095 or 0-32767 + depending on the brightness control mode. If CABC is + enabled 0-4095 range is used. Is this a pm8150l restriction or a WLED5 restriction (will other WLED5 have different ranges)? It is a WLED5 restriction which is used in pm8150l PMIC. allOf: - $ref: /schemas/types.yaml#/definitions/uint32 default: 2048 + max-brightness: +maxItems: 1 +description: + Maximum brightness level. Allowed values are, + for pmi8998 it is 0-4095. + For pm8150l, this can be either 4095 or 32767. Ditto. It is a WLED5 restriction which is used in pm8150l PMIC. + If CABC is enabled, this is capped to 4095. +allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + label: maxItems: 1 description: @@ -124,6 +138,31 @@ properties: value for PM8941 from 1 to 3. Default 2 For PMI8998 from 1 to 4. + qcom,modulator-sel: +maxItems: 1 +allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 +description: + Selects the modulator used for brightness modulation. + Allowed values are, + 0 - Modulator A + 1 - Modulator B + If not specified, then modulator A will be used by default. + This property is applicable only to WLED5 peripheral. + + qcom,cabc-sel: +maxItems: 1 +allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 +description: + Selects the CABC pin signal used for brightness modulation. + Allowed values are, + 0 - CABC disabled + 1 - CABC 1 + 2 - CABC 2 + 3 - External signal (e.g. LPG) is used for dimming + This property is applicable only to WLED5 peripheral. + interrupts: maxItems: 2 description: diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index b73f273..edbbcb2 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -19,6 +19,8 @@ #define WLED_DEFAULT_BRIGHTNESS2048 #define WLED_SOFT_START_DLY_US 1 #define WLED3_SINK_REG_BRIGHT_MAX 0xFFF +#define WLED5_SINK_REG_BRIGHT_MAX_12B 0xFFF +#define WLED5_SINK_REG_BRIGHT_MAX_15B 0x7FFF /* WLED3/WLED4 control registers */ #define WLED3_CTRL_REG_FAULT_STATUS0x08 @@ -40,6 +42,7 @@ #define WLED3_CTRL_REG_OVP 0x4d #define WLED3_CTRL_REG_OVP_MASK GENMASK(1, 0) +#define WLED5_CTRL_REG_OVP_MASK GENMASK(3, 0) #define WLED3_CTRL_REG_ILIMIT 0x4e #define WLED3_CTRL_REG_ILIMIT_MASKGENMASK(2, 0) @@ -101,6 +104,40 @@ #define WLED4_SINK_REG_BRIGHT(n) (0x57 + (n * 0x10)) +/* WLED5 specific sink registers */ +#define WLED5_SINK_REG_MOD_A_EN0x50 +#define WLED5_SINK_REG_MOD_B_EN0x60 +#define WLED5_SINK_REG_MOD_EN_MASKBIT(7) + +#define WLED5_SINK_REG_MOD_A_SRC_SEL 0x51 +#define WLED5_SINK_REG_MOD_B_SRC_SEL 0x61 +#define WLED5_SINK_REG_MOD_SRC_SEL_HIGH 0 +#define WLED5_SINK_REG_MOD_SRC_SEL_EXT0x03 +#define WLED5_SINK_REG_MOD_SRC_SEL_MASK GENMASK(1, 0) + +#define WLED5_SINK_REG_MOD_A_BRIGHTNESS_WIDTH_SEL 0x52 +#define WLED5_SINK_REG_MOD_B_BRIGHTNESS_WIDTH_SEL 0x62 +#define WLED5_SINK_REG_BRIGHT
Re: [PATCH V3 1/4] backlight: qcom-wled: convert the wled bindings to .yaml format
On 2020-03-11 00:01, Rob Herring wrote: On Mon, 9 Mar 2020 18:55:59 +0530, Kiran Gunda wrote: Convert the qcom-wled bindings from .txt to .yaml format. Signed-off-by: Kiran Gunda --- .../bindings/leds/backlight/qcom-wled.txt | 154 - .../bindings/leds/backlight/qcom-wled.yaml | 184 + 2 files changed, 184 insertions(+), 154 deletions(-) delete mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt create mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml My bot found errors running 'make dt_binding_check' on your patch: Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml: $id: relative path/filename doesn't match actual path or filename expected: http://devicetree.org/schemas/leds/backlight/qcom-wled.yaml# See https://patchwork.ozlabs.org/patch/1251567 Please check and re-submit. I will fix it in next post. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 00/12] Convert PWM period and duty cycle to u64
Because period and duty cycle are defined in the PWM framework structs as ints with units of nanoseconds, the maximum time duration that can be set is limited to ~2.147 seconds. Consequently, applications desiring to set greater time periods via the PWM framework are not be able to do so - like, for instance, causing an LED to blink at an interval of 5 seconds. Redefining the period and duty cycle struct members in the core PWM framework structs as u64 values will enable larger time durations to be set and solve this problem. Such a change to the framework mandates that drivers using these struct members (and corresponding helper functions) also be modified correctly in order to prevent compilation errors. This patch series introduces the changes to all the drivers first, followed by the framework change at the very end so that when the latter is applied, all the drivers are in good shape and there are no compilation errors. Changes from v7: - Changed commit messages of all patches to be brief and to the point. - Added explanation of change in cover letter. - Dropped change to pwm-sti.c as upon review it was unnecessary as struct pwm_capture is not being modified in the PWM core. Changes from v6: - Split out the driver changes out into separate patches, one patch per file for ease of reviewing. Changes from v5: - Dropped the conversion of struct pwm_capture to u64 for reasons mentioned in https://www.spinics.net/lists/linux-pwm/msg11541.html Changes from v4: - Split the patch into two: one for changes to the drivers, and the actual switch to u64 for ease of reverting should the need arise. - Re-examined the patch and made the following corrections: * intel_panel.c: DIV64_U64_ROUND_UP -> DIV_ROUND_UP_ULL (as only the numerator would be 64-bit in this case). * pwm-sti.c: do_div -> div_u64 (do_div is optimized only for x86 architectures, and div_u64's comment block suggests to use this as much as possible). Changes from v3: - Rebased to current tip of for-next. Changes from v2: - Fixed %u -> %llu in a dev_dbg in pwm-stm32-lp.c, thanks to kbuild test robot - Added a couple of fixes to pwm-imx-tpm.c and pwm-sifive.c Changes from v1: - Fixed compilation errors seen when compiling for different archs. v1: - Reworked the change pushed upstream earlier [1] so as to not add an extension to an obsolete API. With this change, pwm_ops->apply() can be used to set pwm_state parameters as usual. [1] https://lore.kernel.org/lkml/20190916140048.GB7488@ulmo/ Cc: Lee Jones Cc: Daniel Thompson Cc: Jingoo Han Cc: Bartlomiej Zolnierkiewicz Cc: linux-fb...@vger.kernel.org Cc: Maxime Ripard Cc: Chen-Yu Tsai Cc: Philipp Zabel Cc: Fabrice Gasnier Cc: Maxime Coquelin Cc: Alexandre Torgue Cc: Palmer Dabbelt Cc: Paul Walmsley Cc: linux-ri...@lists.infradead.org Cc: Yash Shah Cc: Atish Patra Cc: Shawn Guo Cc: Sascha Hauer Cc: Pengutronix Kernel Team Cc: Fabio Estevam Cc: NXP Linux Team Cc: Sascha Hauer Cc: Pengutronix Kernel Team Cc: Fabio Estevam Cc: NXP Linux Team Cc: Alexander Shiyan Cc: Mauro Carvalho Chehab Cc: Richard Fontana Cc: Thomas Gleixner Cc: Kate Stewart Cc: Allison Randal Cc: linux-me...@vger.kernel.org Cc: Kamil Debski Cc: Bartlomiej Zolnierkiewicz Cc: Jean Delvare Cc: Guenter Roeck Cc: Liam Girdwood Cc: Mark Brown Cc: linux-hw...@vger.kernel.org Cc: Jani Nikula Cc: Joonas Lahtinen Cc: David Airlie Cc: Daniel Vetter Cc: Chris Wilson Cc: "Ville Syrjälä" Cc: intel-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Cc: Michael Turquette Cc: Stephen Boyd Cc: linux-...@vger.kernel.org Guru Das Srinagesh (12): clk: pwm: Use 64-bit division function drm/i915: Use 64-bit division macro hwmon: pwm-fan: Use 64-bit division macro ir-rx51: Use 64-bit division macro pwm: clps711x: Use 64-bit division macro pwm: pwm-imx-tpm: Use 64-bit division macro pwm: imx27: Use 64-bit division macro and function pwm: sifive: Use 64-bit division macro pwm: stm32-lp: Use %llu format specifier for period pwm: sun4i: Use 64-bit division function backlight: pwm_bl: Use 64-bit division function pwm: core: Convert period and duty cycle to u64 drivers/clk/clk-pwm.c | 2 +- drivers/gpu/drm/i915/display/intel_panel.c | 2 +- drivers/hwmon/pwm-fan.c| 2 +- drivers/media/rc/ir-rx51.c | 3 ++- drivers/pwm/core.c | 4 ++-- drivers/pwm/pwm-clps711x.c | 2 +- drivers/pwm/pwm-imx-tpm.c | 2 +- drivers/pwm/pwm-imx27.c| 5 ++--- drivers/pwm/pwm-sifive.c | 2 +- drivers/pwm/pwm-stm32-lp.c | 2 +- drivers/pwm/pwm-sun4i.c| 2 +- drivers/pwm/sysfs.c| 8 drivers/video/backlight/pwm_bl.c | 3 ++- include/linux/pwm.h|
Re: [PATCH v4] drm/i915: Init lspcon after HPD in intel_dp_detect()
> On Feb 15, 2020, at 01:56, Kai-Heng Feng wrote: > > On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port > becomes useless and never responds to cable hotplugging: > [3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon > [3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port > D > > Seems like the lspcon chip on the system in question only gets powered > after the cable is plugged. > > So let's call lspcon_init() dynamically to properly initialize the > lspcon chip and make HDMI port work. > > Signed-off-by: Kai-Heng Feng A gentle ping. > --- > v4: > - Trust VBT in intel_infoframe_init(). > - Init lspcon in intel_dp_detect(). > > v3: > - Make sure it's handled under long HPD case. > > v2: > - Move lspcon_init() inside of intel_dp_hpd_pulse(). > > drivers/gpu/drm/i915/display/intel_ddi.c | 17 + > drivers/gpu/drm/i915/display/intel_dp.c | 13 - > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- > 3 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 33f1dc3d7c1a..ca717434b406 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -4741,7 +4741,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) > &dev_priv->vbt.ddi_port_info[port]; > struct intel_digital_port *intel_dig_port; > struct intel_encoder *encoder; > - bool init_hdmi, init_dp, init_lspcon = false; > + bool init_hdmi, init_dp; > enum phy phy = intel_port_to_phy(dev_priv, port); > > init_hdmi = port_info->supports_dvi || port_info->supports_hdmi; > @@ -4754,7 +4754,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) >* is initialized before lspcon. >*/ > init_dp = true; > - init_lspcon = true; > init_hdmi = false; > DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port)); > } > @@ -4833,20 +4832,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) > goto err; > } > > - if (init_lspcon) { > - if (lspcon_init(intel_dig_port)) > - /* TODO: handle hdmi info frame part */ > - DRM_DEBUG_KMS("LSPCON init success on port %c\n", > - port_name(port)); > - else > - /* > - * LSPCON init faied, but DP init was success, so > - * lets try to drive as DP++ port. > - */ > - DRM_ERROR("LSPCON init failed on port %c\n", > - port_name(port)); > - } > - > intel_infoframe_init(intel_dig_port); > > return; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index c7424e2a04a3..43117aa86292 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5663,8 +5663,19 @@ intel_dp_detect(struct drm_connector *connector, > /* Can't disconnect eDP */ > if (intel_dp_is_edp(intel_dp)) > status = edp_detect(intel_dp); > - else if (intel_digital_port_connected(encoder)) > + else if (intel_digital_port_connected(encoder)) { > + if (intel_bios_is_lspcon_present(dev_priv, dig_port->base.port) > && > + !dig_port->lspcon.active) { > + if (lspcon_init(dig_port)) > + DRM_DEBUG_KMS("LSPCON init success on port > %c\n", > + port_name(dig_port->base.port)); > + else > + DRM_DEBUG_KMS("LSPCON init failed on port %c\n", > + port_name(dig_port->base.port)); > + } > + > status = intel_dp_detect_dpcd(intel_dp); > + } > else > status = connector_status_disconnected; > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c > b/drivers/gpu/drm/i915/display/intel_hdmi.c > index 93ac0f296852..27a5aa8cefc9 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > @@ -3100,7 +3100,7 @@ void intel_infoframe_init(struct intel_digital_port > *intel_dig_port) > intel_dig_port->set_infoframes = g4x_set_infoframes; > intel_dig_port->infoframes_enabled = g4x_infoframes_enabled; > } else if (HAS_DDI(dev_priv)) { > - if (intel_dig_port->lspcon.active) { > + if (intel_bios_is_lspcon_present(dev_priv, > intel_dig_port->base.port)) { > intel_dig_port->write_infoframe = > lspcon_write_infoframe; > intel_dig_port->rea
Re: [PATCH V3 2/4] backlight: qcom-wled: Add callback functions
On 2020-03-10 20:57, Daniel Thompson wrote: On Mon, Mar 09, 2020 at 06:56:00PM +0530, Kiran Gunda wrote: Add cabc_config, sync_toggle, wled_ovp_fault_status and wled_ovp_delay callback functions to prepare the driver for adding WLED5 support. Signed-off-by: Kiran Gunda Overall this code would a lot easier to review if --- drivers/video/backlight/qcom-wled.c | 196 +++- 1 file changed, 126 insertions(+), 70 deletions(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 3d276b3..b73f273 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -128,6 +128,7 @@ struct wled_config { bool cs_out_en; bool ext_gen; bool cabc; + bool en_cabc; Does this ever get set to true? Yes. If user wants use the cabc pin to control the brightness and use the "qcom,cabc" DT property in the device tree. bool external_pfet; bool auto_detection_enabled; }; @@ -147,14 +148,20 @@ struct wled { u32 max_brightness; u32 short_count; u32 auto_detect_count; + u32 version; bool disabled_by_short; bool has_short_detect; + bool cabc_disabled; int short_irq; int ovp_irq; struct wled_config cfg; struct delayed_work ovp_work; int (*wled_set_brightness)(struct wled *wled, u16 brightness); + int (*cabc_config)(struct wled *wled, bool enable); + int (*wled_sync_toggle)(struct wled *wled); + int (*wled_ovp_fault_status)(struct wled *wled, bool *fault_set); + int (*wled_ovp_delay)(struct wled *wled); Let's get some doc comments explaining what these callbacks do (and which versions they apply to). Sure. I will update it in the commit text in next post. cabc_config() in particular appears to have a very odd interface for wled4. It looks like it relies on being initially called with enable set a particular way and prevents itself from acting again. Therefore if the comment you end up writing doesn't sound "right" then please also fix the API! Actually this variable is useful for WLED5, where the default HW state is CABC1 enabled mode. So, if the user doesn't want to use the CABC we are configuring the HW state to "0" based on the DT property and then setting a flag to not enable it again. This is not needed for WLED4. I will remove it for WLED4 in next post. Finally, why is everything except cabc_config() prefixed with wled? It is typo. I will correct it in the next post. Daniel. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge: analogix_dp: Split bind() into probe() and real bind()
On 3/10/20 6:34 PM, Marek Szyprowski wrote: Analogix_dp driver acquires all its resources in the ->bind() callback, what is a bit against the component driver based approach, where the driver initialization is split into a probe(), where all resources are gathered, and a bind(), where all objects are created and a compound driver is initialized. Extract all the resource related operations to analogix_dp_probe() and analogix_dp_remove(), then call them before/after registration of the device components from the main Exynos DP and Rockchip DP drivers. Also move the plat_data initialization to the probe() to make it available for the analogix_dp_probe() function. This fixes the multiple calls to the bind() of the DRM compound driver when the DP PHY driver is not yet loaded/probed: [drm] Exynos DRM: using 1440.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 1440.fimd (ops fimd_component_ops [exynosdrm]) exynos-drm exynos-drm: bound 1445.mixer (ops mixer_component_ops [exynosdrm]) exynos-dp 145b.dp-controller: no DP phy configured exynos-drm exynos-drm: failed to bind 145b.dp-controller (ops exynos_dp_ops [exynosdrm]): -517 exynos-drm exynos-drm: master bind failed: -517 ... [drm] Exynos DRM: using 1440.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 1440.fimd (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 1445.mixer (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 145b.dp-controller (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 1453.hdmi (ops hdmi_enable [exynosdrm]) [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). Console: switching to colour frame buffer device 170x48 exynos-drm exynos-drm: fb0: exynosdrmfb frame buffer device [drm] Initialized exynos 1.1.0 20180330 for exynos-drm on minor 1 ... Signed-off-by: Marek Szyprowski Acked-by: Andy Yan --- v2: - moved plat_data initialization to exynos_dp_probe/rockchip_dp_probe as pointed by Andy Yan --- .../drm/bridge/analogix/analogix_dp_core.c| 33 +++-- drivers/gpu/drm/exynos/exynos_dp.c| 29 --- .../gpu/drm/rockchip/analogix_dp-rockchip.c | 36 ++- include/drm/bridge/analogix_dp.h | 5 +-- 4 files changed, 61 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 9ded2cef57dd..76736fb8ed94 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1652,8 +1652,7 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux, } struct analogix_dp_device * -analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, -struct analogix_dp_plat_data *plat_data) +analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data) { struct platform_device *pdev = to_platform_device(dev); struct analogix_dp_device *dp; @@ -1756,22 +1755,30 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, irq_flags, "analogix-dp", dp); if (ret) { dev_err(&pdev->dev, "failed to request irq\n"); - goto err_disable_pm_runtime; + return ERR_PTR(ret); } disable_irq(dp->irq); + return dp; +} +EXPORT_SYMBOL_GPL(analogix_dp_probe); + +int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev) +{ + int ret; + dp->drm_dev = drm_dev; dp->encoder = dp->plat_data->encoder; dp->aux.name = "DP-AUX"; dp->aux.transfer = analogix_dpaux_transfer; - dp->aux.dev = &pdev->dev; + dp->aux.dev = dp->dev; ret = drm_dp_aux_register(&dp->aux); if (ret) - return ERR_PTR(ret); + return ret; - pm_runtime_enable(dev); + pm_runtime_enable(dp->dev); ret = analogix_dp_create_bridge(drm_dev, dp); if (ret) { @@ -1779,13 +1786,12 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, goto err_disable_pm_runtime; } - return dp; + return 0; err_disable_pm_runtime: + pm_runtime_disable(dp->dev); - pm_runtime_disable(dev); - - return ERR_PTR(ret); + return ret; } EXPORT_SYMBOL_GPL(analogix_dp_bind); @@ -1802,10 +1808,15 @@ void analogix_dp_unbind(struct analogix_dp_device *dp) drm_dp_aux_unregister(&dp->aux); pm_runtime_disable(dp->dev); - clk_disable_unprepare(dp->clock); } EXPORT_SYMBOL_GPL(analogix_dp_unbind); +void analogix_dp_remove(struct analogix_dp_device *dp) +{ + clk_disable_unprepare(dp->clock); +} +EXPORT_SYMBOL_GPL(analogix_dp_remove); + #ifdef CONFIG_PM int analogix_dp_suspend(struct analogix_dp_device *dp) { diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/ex
[PATCH] staging: fbtft: Remove prohibited spaces before ')'
Fix checkpatch errors: "ERROR: space prohibited before that close parenthesis ')'" in fbtft-bus.c:65 and fbtft-bus.c:67. Signed-off-by: Marcio Albano --- drivers/staging/fbtft/fbtft-bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c index 63c65dd67..847cbfbbd 100644 --- a/drivers/staging/fbtft/fbtft-bus.c +++ b/drivers/staging/fbtft/fbtft-bus.c @@ -62,9 +62,9 @@ out: \ } \ EXPORT_SYMBOL(func); -define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, ) +define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8) define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16) -define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, ) +define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16) void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...) { -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: WARNING in idr_destroy
syzbot has bisected this bug to: commit 5b3709793d151e6e12eb6a38a5da3f7fc2923d3a Author: Thomas Zimmermann Date: Wed May 8 08:26:19 2019 + drm/ast: Convert AST driver to |struct drm_gem_vram_object| bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=15a66fb5e0 start commit: 63623fd4 Merge tag 'for-linus' of git://git.kernel.org/pub.. git tree: upstream final crash:https://syzkaller.appspot.com/x/report.txt?x=17a66fb5e0 console output: https://syzkaller.appspot.com/x/log.txt?x=13a66fb5e0 kernel config: https://syzkaller.appspot.com/x/.config?x=5d2e033af114153f dashboard link: https://syzkaller.appspot.com/bug?extid=05835159fe322770fe3d syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14e978e3e0 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10b1a819e0 Reported-by: syzbot+05835159fe322770f...@syzkaller.appspotmail.com Fixes: 5b3709793d15 ("drm/ast: Convert AST driver to |struct drm_gem_vram_object|") For information about bisection process see: https://goo.gl/tpsmEJ#bisection ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: rgb: don't count non-existent devices when determining subdrivers
Am Dienstag, 21. Januar 2020, 23:48:28 CET schrieb Heiko Stuebner: > From: Heiko Stuebner > > rockchip_drm_endpoint_is_subdriver() may also return error codes. > For example if the target-node is in the disabled state, so no > platform-device is getting created for it. > > In that case current code would count that as external rgb device, > which in turn would make probing the rockchip-drm device fail. > > So only count the target as rgb device if the function actually > returns 0. > > Signed-off-by: Heiko Stuebner applied to drm-misc-next ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/51] drm: add managed resources tied to drm_device
Hi Daniel Am 02.03.20 um 23:25 schrieb Daniel Vetter: > We have lots of these. And the cleanup code tends to be of dubious > quality. The biggest wrong pattern is that developers use devm_, which > ties the release action to the underlying struct device, whereas > all the userspace visible stuff attached to a drm_device can long > outlive that one (e.g. after a hotunplug while userspace has open > files and mmap'ed buffers). Give people what they want, but with more > correctness. > > Mostly copied from devres.c, with types adjusted to fit drm_device and > a few simplifications - I didn't (yet) copy over everything. Since > the types don't match code sharing looked like a hopeless endeavour. > > For now it's only super simplified, no groups, you can't remove > actions (but kfree exists, we'll need that soon). Plus all specific to > drm_device ofc, including the logging. Which I didn't bother to make > compile-time optional, since none of the other drm logging is compile > time optional either. > > One tricky bit here is the chicken&egg between allocating your > drm_device structure and initiliazing it with drm_dev_init. For > perfect onion unwinding we'd need to have the action to kfree the > allocation registered before drm_dev_init registers any of its own > release handlers. But drm_dev_init doesn't know where exactly the > drm_device is emebedded into the overall structure, and by the time it > returns it'll all be too late. And forcing drivers to be able clean up > everything except the one kzalloc is silly. > > Work around this by having a very special final_kfree pointer. This > also avoids troubles with the list head possibly disappearing from > underneath us when we release all resources attached to the > drm_device. > > v2: Do all the kerneldoc at the end, to avoid lots of fairly pointless > shuffling while getting everything into shape. > > v3: Add static to add/del_dr (Neil) > Move typo fix to the right patch (Neil) > > v4: Enforce contract for drmm_add_final_kfree: > > Use ksize() to check that the drm_device is indeed contained somewhere > in the final kfree(). Because we need that or the entire managed > release logic blows up in a pile of use-after-frees. Motivated by a > discussion with Laurent. > > v5: Review from Laurent: > - %zu instead of casting size_t > - header guards > - sorting of includes > - guarding of data assignment if we didn't allocate it for a NULL > pointer > - delete spurious newline > - cast void* data parameter correctly in ->release call, no idea how > this even worked before > > v3: Review from Sam > - Add the kerneldoc for the managed sub-struct back in, even if it > doesn't show up in the generated html somehow. > - Explain why __always_inline. > - Fix bisectability around the final kfree() in drm_dev_relase(). This > is just interim code which will disappear again. > - Some whitespace polish. > - Add debug output when drmm_add_action or drmm_kmalloc fail. > > Cc: Sam Ravnborg > Cc: Laurent Pinchart > Cc: Neil Armstrong Cc: Greg Kroah-Hartman > Cc: "Rafael J. Wysocki" > Signed-off-by: Daniel Vetter > --- > Documentation/gpu/drm-internals.rst | 6 + > drivers/gpu/drm/Makefile| 3 +- > drivers/gpu/drm/drm_drv.c | 12 ++ > drivers/gpu/drm/drm_internal.h | 3 + > drivers/gpu/drm/drm_managed.c | 186 > include/drm/drm_device.h| 15 +++ > include/drm/drm_managed.h | 30 + > include/drm/drm_print.h | 6 + > 8 files changed, 260 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/drm_managed.c > create mode 100644 include/drm/drm_managed.h > > diff --git a/Documentation/gpu/drm-internals.rst > b/Documentation/gpu/drm-internals.rst > index a73320576ca9..a6b6145fda78 100644 > --- a/Documentation/gpu/drm-internals.rst > +++ b/Documentation/gpu/drm-internals.rst > @@ -132,6 +132,12 @@ be unmapped; on many devices, the ROM address decoder is > shared with > other BARs, so leaving it mapped could cause undesired behaviour like > hangs or memory corruption. > > +Managed Resources > +- > + > +.. kernel-doc:: drivers/gpu/drm/drm_managed.c > + :doc: managed resources > + > Bus-specific Device Registration and PCI Support > > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 7f72ef5e7811..183c60048307 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -17,7 +17,8 @@ drm-y := drm_auth.o drm_cache.o \ > drm_plane.o drm_color_mgmt.o drm_print.o \ > drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \ > drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \ > - drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o > + drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o \ > + drm_managed.o > > drm-$(CONFIG_DRM_LEGACY) += drm
Re: [PATCH 04/51] drm: Set final_kfree in drm_dev_alloc
Am 02.03.20 um 23:25 schrieb Daniel Vetter: > I also did a full review of all callers, and only the xen driver > forgot to call drm_dev_put in the failure path. Fix that up too. > > v2: I noticed that xen has a drm_driver.release hook, and uses > drm_dev_alloc(). We need to remove the kfree from > xen_drm_drv_release(). > > bochs also has a release hook, but leaked the drm_device ever since > > commit 0a6659bdc5e8221da99eebb176fd9591435e38de > Author: Gerd Hoffmann > Date: Tue Dec 17 18:04:46 2013 +0100 > > drm/bochs: new driver > > This patch here fixes that leak. > > Same for virtio, started leaking with > > commit b1df3a2b24a917f8853d43fe9683c0e360d2c33a > Author: Gerd Hoffmann > Date: Tue Feb 11 14:58:04 2020 +0100 > > drm/virtio: add drm_driver.release callback. > > Cc: Gerd Hoffmann > Cc: Oleksandr Andrushchenko > Cc: xen-de...@lists.xenproject.org > > Reviewed-by: Oleksandr Andrushchenko > Signed-off-by: Daniel Vetter Acked-by: Thomas Zimmermann > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > Cc: Oleksandr Andrushchenko > Cc: xen-de...@lists.xenproject.org > --- > drivers/gpu/drm/drm_drv.c | 3 +++ > drivers/gpu/drm/xen/xen_drm_front.c | 2 +- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 153050fc926c..7b84ee8a5eb5 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -39,6 +39,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -819,6 +820,8 @@ struct drm_device *drm_dev_alloc(struct drm_driver > *driver, > return ERR_PTR(ret); > } > > + drmm_add_final_kfree(dev, dev); > + > return dev; > } > EXPORT_SYMBOL(drm_dev_alloc); > diff --git a/drivers/gpu/drm/xen/xen_drm_front.c > b/drivers/gpu/drm/xen/xen_drm_front.c > index 4be49c1aef51..d22b5da38935 100644 > --- a/drivers/gpu/drm/xen/xen_drm_front.c > +++ b/drivers/gpu/drm/xen/xen_drm_front.c > @@ -461,7 +461,6 @@ static void xen_drm_drv_release(struct drm_device *dev) > drm_mode_config_cleanup(dev); > > drm_dev_fini(dev); > - kfree(dev); > > if (front_info->cfg.be_alloc) > xenbus_switch_state(front_info->xb_dev, > @@ -561,6 +560,7 @@ static int xen_drm_drv_init(struct xen_drm_front_info > *front_info) > fail_modeset: > drm_kms_helper_poll_fini(drm_dev); > drm_mode_config_cleanup(drm_dev); > + drm_dev_put(drm_dev); > fail: > kfree(drm_info); > return ret; > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/51] drm/udl: Use drmm_add_final_kfree
Am 02.03.20 um 23:25 schrieb Daniel Vetter: > With this we can drop the final kfree from the release function. > > v2: We need drm_dev_put to unroll the driver creation (once > drm_dev_init and drmm_add_final_kfree suceeded), otherwise > the drmm_ magic doesn't happen. > > v3: Actually squash in the fixup (Laurent). > > Acked-by: Sam Ravnborg > Signed-off-by: Daniel Vetter Acked-by: Thomas Zimmermann > Cc: Laurent Pinchart > Cc: Dave Airlie > Cc: Sean Paul > Cc: Thomas Zimmermann > Cc: Emil Velikov > Cc: Daniel Vetter > Cc: "Noralf Trønnes" > Cc: Thomas Gleixner > Cc: Sam Ravnborg > --- > drivers/gpu/drm/udl/udl_drv.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index e6c1cd77d4d4..6a5594946096 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -38,7 +39,6 @@ static void udl_driver_release(struct drm_device *dev) > udl_fini(dev); > udl_modeset_cleanup(dev); > drm_dev_fini(dev); > - kfree(dev); > } > > static struct drm_driver driver = { > @@ -77,11 +77,11 @@ static struct udl_device *udl_driver_create(struct > usb_interface *interface) > > udl->udev = udev; > udl->drm.dev_private = udl; > + drmm_add_final_kfree(&udl->drm, udl); > > r = udl_init(udl); > if (r) { > - drm_dev_fini(&udl->drm); > - kfree(udl); > + drm_dev_put(&udl->drm); > return ERR_PTR(r); > } > > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/51] drm: add managed resources tied to drm_device
Am 02.03.20 um 23:25 schrieb Daniel Vetter: <...> > + > +int __drmm_add_action(struct drm_device *dev, > + drmres_release_t action, > + void *data, const char *name) > +{ > + struct drmres *dr; > + void **void_ptr; > + > + dr = alloc_dr(action, data ? sizeof(void*) : 0, > + GFP_KERNEL | __GFP_ZERO, > + dev_to_node(dev->dev)); > + if (!dr) { > + drm_dbg_drmres(dev, "failed to add action %s for %p\n", > +name, data); > + return -ENOMEM; > + } > + > + dr->node.name = name; Maybe do a kstrdup_const() on name and later a kfree_const() during release. Just in case someone decides to allocate 'name' dynamically. > + if (data) { > + void_ptr = (void **)&dr->data; > + *void_ptr = data; > + } > + > + add_dr(dev, dr); > + > + return 0; > +} > +EXPORT_SYMBOL(__drmm_add_action); > + > +void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) > +{ > + struct drmres *dr; > + > + dr = alloc_dr(NULL, size, gfp, dev_to_node(dev->dev)); > + if (!dr) { > + drm_dbg_drmres(dev, "failed to allocate %zu bytes, %u flags\n", > +size, gfp); > + return NULL; > + } > + dr->node.name = "kmalloc"; > + > + add_dr(dev, dr); > + > + return dr->data; > +} > +EXPORT_SYMBOL(drmm_kmalloc); > + > +void drmm_kfree(struct drm_device *dev, void *data) > +{ > + struct drmres *dr_match = NULL, *dr; > + unsigned long flags; > + > + if (!data) > + return; > + > + spin_lock_irqsave(&dev->managed.lock, flags); > + list_for_each_entry(dr, &dev->managed.resources, node.entry) { > + if (dr->data == data) { > + dr_match = dr; > + del_dr(dev, dr_match); > + break; > + } > + } > + spin_unlock_irqrestore(&dev->managed.lock, flags); > + > + if (WARN_ON(!dr_match)) > + return; > + > + kfree(dr_match); > +} > +EXPORT_SYMBOL(drmm_kfree); > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > index bb60a949f416..d39132b477dd 100644 > --- a/include/drm/drm_device.h > +++ b/include/drm/drm_device.h > @@ -67,6 +67,21 @@ struct drm_device { > /** @dev: Device structure of bus-device */ > struct device *dev; > > + /** > + * @managed: > + * > + * Managed resources linked to the lifetime of this &drm_device as > + * tracked by @ref. > + */ > + struct { > + /** @managed.resources: managed resources list */ > + struct list_head resources; > + /** @managed.final_kfree: pointer for final kfree() call */ > + void *final_kfree; > + /** @managed.lock: protects @managed.resources */ > + spinlock_t lock; > + } managed; > + > /** @driver: DRM driver managing the device */ > struct drm_driver *driver; > > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h > new file mode 100644 > index ..7b5df7d09b19 > --- /dev/null > +++ b/include/drm/drm_managed.h > @@ -0,0 +1,30 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#ifndef _DRM_MANAGED_H_ > +#define _DRM_MANAGED_H_ > + > +#include > +#include > + > +struct drm_device; > + > +typedef void (*drmres_release_t)(struct drm_device *dev, void *res); > + > +#define drmm_add_action(dev, action, data) \ > + __drmm_add_action(dev, action, data, #action) > + > +int __must_check __drmm_add_action(struct drm_device *dev, > +drmres_release_t action, > +void *data, const char *name); > + > +void drmm_add_final_kfree(struct drm_device *dev, void *parent); > + > +void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc; > +static inline void *drmm_kzalloc(struct drm_device *dev, size_t size, gfp_t > gfp) > +{ > + return drmm_kmalloc(dev, size, gfp | __GFP_ZERO); > +} > + > +void drmm_kfree(struct drm_device *dev, void *data); > + > +#endif > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index ca7cee8e728a..1c9417430d08 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -313,6 +313,10 @@ enum drm_debug_category { >* @DRM_UT_DP: Used in the DP code. >*/ > DRM_UT_DP = 0x100, > + /** > + * @DRM_UT_DRMRES: Used in the drm managed resources code. > + */ > + DRM_UT_DRMRES = 0x200, > }; > > static inline bool drm_debug_enabled(enum drm_debug_category category) > @@ -442,6 +446,8 @@ void drm_dev_dbg(const struct device *dev, enum > drm_debug_category category, > drm_dev_dbg((drm)->dev, DRM_UT_LEASE, fmt, ##__VA_ARGS__) > #define drm_dbg_dp(drm, fmt, ...)\ > drm_dev_dbg((drm)->dev, DRM_UT_DP, fmt, ##__VA_ARGS__) > +#d
Re: [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
Am 02.03.20 um 23:25 schrieb Daniel Vetter: > A few things: > - Update the example driver in the documentation. > - We can drop the old kfree in drm_dev_release. > - Add a WARN_ON check in drm_dev_register to make sure everyone calls > drmm_add_final_kfree and there's no leaks. > > Signed-off-by: Daniel Vetter Acked-by: Thomas Zimmermann > --- > drivers/gpu/drm/drm_drv.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 7b84ee8a5eb5..1a048325f30e 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor) > * > * drm_mode_config_cleanup(drm); > * drm_dev_fini(drm); > - * kfree(priv->userspace_facing); > - * kfree(priv); > * } > * > * static struct drm_driver driver_drm_driver = { > @@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor) > * kfree(drm); > * return ret; > * } > + * drmm_add_final_kfree(drm, priv); > * > * drm_mode_config_init(drm); > * > - * priv->userspace_facing = kzalloc(..., GFP_KERNEL); > + * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL); > * if (!priv->userspace_facing) > * return -ENOMEM; > * > @@ -961,6 +960,8 @@ int drm_dev_register(struct drm_device *dev, unsigned > long flags) > struct drm_driver *driver = dev->driver; > int ret; > > + WARN_ON(!dev->managed.final_kfree); > + > if (drm_dev_needs_global_mutex(dev)) > mutex_lock(&drm_global_mutex); > > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 20/51] drm: Handle dev->unique with drmm_
Am 02.03.20 um 23:26 schrieb Daniel Vetter: > We need to add a drmm_kstrdup for this, but let's start somewhere. > > This is not exactly perfect onion unwinding, but it's jsut a kfree so > doesn't really matter at all. > > Signed-off-by: Daniel Vetter Acked-by: Thomas Zimmermann > --- > drivers/gpu/drm/drm_drv.c | 5 ++--- > drivers/gpu/drm/drm_managed.c | 16 > include/drm/drm_managed.h | 1 + > 3 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 1a048325f30e..ef79c03e311c 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -777,7 +777,6 @@ void drm_dev_fini(struct drm_device *dev) > mutex_destroy(&dev->filelist_mutex); > mutex_destroy(&dev->struct_mutex); > drm_legacy_destroy_members(dev); > - kfree(dev->unique); > } > EXPORT_SYMBOL(drm_dev_fini); > > @@ -1068,8 +1067,8 @@ EXPORT_SYMBOL(drm_dev_unregister); > */ > int drm_dev_set_unique(struct drm_device *dev, const char *name) > { > - kfree(dev->unique); > - dev->unique = kstrdup(name, GFP_KERNEL); > + drmm_kfree(dev, dev->unique); > + dev->unique = drmm_kstrdup(dev, name, GFP_KERNEL); > > return dev->unique ? 0 : -ENOMEM; > } > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c > index 57dc79fa90af..514d5bd42446 100644 > --- a/drivers/gpu/drm/drm_managed.c > +++ b/drivers/gpu/drm/drm_managed.c > @@ -160,6 +160,22 @@ void *drmm_kmalloc(struct drm_device *dev, size_t size, > gfp_t gfp) > } > EXPORT_SYMBOL(drmm_kmalloc); > > +char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp) > +{ > + size_t size; > + char *buf; > + > + if (!s) > + return NULL; > + > + size = strlen(s) + 1; > + buf = drmm_kmalloc(dev, size, gfp); > + if (buf) > + memcpy(buf, s, size); > + return buf; > +} > +EXPORT_SYMBOL_GPL(drmm_kstrdup); > + > void drmm_kfree(struct drm_device *dev, void *data) > { > struct drmres *dr_match = NULL, *dr; > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h > index 7b5df7d09b19..89e6fce9f689 100644 > --- a/include/drm/drm_managed.h > +++ b/include/drm/drm_managed.h > @@ -24,6 +24,7 @@ static inline void *drmm_kzalloc(struct drm_device *dev, > size_t size, gfp_t gfp) > { > return drmm_kmalloc(dev, size, gfp | __GFP_ZERO); > } > +char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp); > > void drmm_kfree(struct drm_device *dev, void *data); > > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 21/51] drm: Use drmm_ for drm_dev_init cleanup
Hi Am 02.03.20 um 23:26 schrieb Daniel Vetter: > Well for the simple stuff at least, vblank, gem and minor cleanup I > want to further split up as a demonstration. > > v2: We need to clear drm_device->dev otherwise the debug drm printing > after our cleanup hook (e.g. in drm_manged_release) will chase > released memory and result in a use-after-free. Not really pretty, but > oh well. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_drv.c | 48 --- > 1 file changed, 25 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index ef79c03e311c..23e5b0e7e041 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -580,6 +580,23 @@ static void drm_fs_inode_free(struct inode *inode) > *used. > */ > > +static void drm_dev_init_release(struct drm_device *dev, void *res) > +{ > + drm_legacy_ctxbitmap_cleanup(dev); > + drm_legacy_remove_map_hash(dev); > + drm_fs_inode_free(dev->anon_inode); > + > + put_device(dev->dev); > + /* Prevent use-after-free in drm_managed_release when debugging is > + * enabled. Slightly awkward, but can't really be helped. */ > + dev->dev = NULL; > + mutex_destroy(&dev->master_mutex); > + mutex_destroy(&dev->clientlist_mutex); > + mutex_destroy(&dev->filelist_mutex); > + mutex_destroy(&dev->struct_mutex); > + drm_legacy_destroy_members(dev); > +} > + > /** > * drm_dev_init - Initialise new DRM device > * @dev: DRM device > @@ -647,11 +664,15 @@ int drm_dev_init(struct drm_device *dev, > mutex_init(&dev->clientlist_mutex); > mutex_init(&dev->master_mutex); > > + ret = drmm_add_action(dev, drm_dev_init_release, NULL); > + if (ret) > + return ret; > + Is this code supposed to stay for the long term? As devices are allocated dynamically, I can imagine that there will be a call that allocates the memory and, at the same time, sets drm_dev_init_release() as the release callback. The question is also released to patch 3, where I proposed to rename __drm_add_action() to __drmm_kzalloc(). > dev->anon_inode = drm_fs_inode_new(); > if (IS_ERR(dev->anon_inode)) { > ret = PTR_ERR(dev->anon_inode); > DRM_ERROR("Cannot allocate anonymous inode: %d\n", ret); > - goto err_free; > + goto err; > } > > if (drm_core_check_feature(dev, DRIVER_RENDER)) { > @@ -688,19 +709,12 @@ int drm_dev_init(struct drm_device *dev, > if (drm_core_check_feature(dev, DRIVER_GEM)) > drm_gem_destroy(dev); > err_ctxbitmap: > - drm_legacy_ctxbitmap_cleanup(dev); > - drm_legacy_remove_map_hash(dev); > err_minors: > drm_minor_free(dev, DRM_MINOR_PRIMARY); > drm_minor_free(dev, DRM_MINOR_RENDER); > - drm_fs_inode_free(dev->anon_inode); > -err_free: > - put_device(dev->dev); > - mutex_destroy(&dev->master_mutex); > - mutex_destroy(&dev->clientlist_mutex); > - mutex_destroy(&dev->filelist_mutex); > - mutex_destroy(&dev->struct_mutex); > - drm_legacy_destroy_members(dev); > +err: > + drm_managed_release(dev); > + Here's more of a general observation than a comment on the actual patch: One odd thing about the overall interface is that there's no way of updating the release callback afterwards. In an OOP language, such as C++, an error within the constructor would rollback the performed actions and return without calling the destructor. Destructors only run for fully constructed objects. In our case, the equivalent is to run the init function and set drm_dev_init_release() as the final step. The init's rollback-code would have to stay, obviously. Best regards Thomas > return ret; > } > EXPORT_SYMBOL(drm_dev_init); > @@ -763,20 +777,8 @@ void drm_dev_fini(struct drm_device *dev) > if (drm_core_check_feature(dev, DRIVER_GEM)) > drm_gem_destroy(dev); > > - drm_legacy_ctxbitmap_cleanup(dev); > - drm_legacy_remove_map_hash(dev); > - drm_fs_inode_free(dev->anon_inode); > - > drm_minor_free(dev, DRM_MINOR_PRIMARY); > drm_minor_free(dev, DRM_MINOR_RENDER); > - > - put_device(dev->dev); > - > - mutex_destroy(&dev->master_mutex); > - mutex_destroy(&dev->clientlist_mutex); > - mutex_destroy(&dev->filelist_mutex); > - mutex_destroy(&dev->struct_mutex); > - drm_legacy_destroy_members(dev); > } > EXPORT_SYMBOL(drm_dev_fini); > > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/51] drm: add managed resources tied to drm_device
Am 11.03.20 um 10:07 schrieb Thomas Zimmermann: > Hi Daniel > > Am 02.03.20 um 23:25 schrieb Daniel Vetter: >> We have lots of these. And the cleanup code tends to be of dubious >> quality. The biggest wrong pattern is that developers use devm_, which >> ties the release action to the underlying struct device, whereas >> all the userspace visible stuff attached to a drm_device can long >> outlive that one (e.g. after a hotunplug while userspace has open >> files and mmap'ed buffers). Give people what they want, but with more >> correctness. >> >> Mostly copied from devres.c, with types adjusted to fit drm_device and >> a few simplifications - I didn't (yet) copy over everything. Since >> the types don't match code sharing looked like a hopeless endeavour. >> >> For now it's only super simplified, no groups, you can't remove >> actions (but kfree exists, we'll need that soon). Plus all specific to >> drm_device ofc, including the logging. Which I didn't bother to make >> compile-time optional, since none of the other drm logging is compile >> time optional either. >> >> One tricky bit here is the chicken&egg between allocating your >> drm_device structure and initiliazing it with drm_dev_init. For >> perfect onion unwinding we'd need to have the action to kfree the >> allocation registered before drm_dev_init registers any of its own >> release handlers. But drm_dev_init doesn't know where exactly the >> drm_device is emebedded into the overall structure, and by the time it >> returns it'll all be too late. And forcing drivers to be able clean up >> everything except the one kzalloc is silly. >> >> Work around this by having a very special final_kfree pointer. This >> also avoids troubles with the list head possibly disappearing from >> underneath us when we release all resources attached to the >> drm_device. >> >> v2: Do all the kerneldoc at the end, to avoid lots of fairly pointless >> shuffling while getting everything into shape. >> >> v3: Add static to add/del_dr (Neil) >> Move typo fix to the right patch (Neil) >> >> v4: Enforce contract for drmm_add_final_kfree: >> >> Use ksize() to check that the drm_device is indeed contained somewhere >> in the final kfree(). Because we need that or the entire managed >> release logic blows up in a pile of use-after-frees. Motivated by a >> discussion with Laurent. >> >> v5: Review from Laurent: >> - %zu instead of casting size_t >> - header guards >> - sorting of includes >> - guarding of data assignment if we didn't allocate it for a NULL >> pointer >> - delete spurious newline >> - cast void* data parameter correctly in ->release call, no idea how >> this even worked before >> >> v3: Review from Sam >> - Add the kerneldoc for the managed sub-struct back in, even if it >> doesn't show up in the generated html somehow. >> - Explain why __always_inline. >> - Fix bisectability around the final kfree() in drm_dev_relase(). This >> is just interim code which will disappear again. >> - Some whitespace polish. >> - Add debug output when drmm_add_action or drmm_kmalloc fail. >> >> Cc: Sam Ravnborg >> Cc: Laurent Pinchart >> Cc: Neil Armstrong > Cc: Greg Kroah-Hartman >> Cc: "Rafael J. Wysocki" >> Signed-off-by: Daniel Vetter >> --- >> Documentation/gpu/drm-internals.rst | 6 + >> drivers/gpu/drm/Makefile| 3 +- >> drivers/gpu/drm/drm_drv.c | 12 ++ >> drivers/gpu/drm/drm_internal.h | 3 + >> drivers/gpu/drm/drm_managed.c | 186 >> include/drm/drm_device.h| 15 +++ >> include/drm/drm_managed.h | 30 + >> include/drm/drm_print.h | 6 + >> 8 files changed, 260 insertions(+), 1 deletion(-) >> create mode 100644 drivers/gpu/drm/drm_managed.c >> create mode 100644 include/drm/drm_managed.h >> >> diff --git a/Documentation/gpu/drm-internals.rst >> b/Documentation/gpu/drm-internals.rst >> index a73320576ca9..a6b6145fda78 100644 >> --- a/Documentation/gpu/drm-internals.rst >> +++ b/Documentation/gpu/drm-internals.rst >> @@ -132,6 +132,12 @@ be unmapped; on many devices, the ROM address decoder >> is shared with >> other BARs, so leaving it mapped could cause undesired behaviour like >> hangs or memory corruption. >> >> +Managed Resources >> +- >> + >> +.. kernel-doc:: drivers/gpu/drm/drm_managed.c >> + :doc: managed resources >> + >> Bus-specific Device Registration and PCI Support >> >> >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >> index 7f72ef5e7811..183c60048307 100644 >> --- a/drivers/gpu/drm/Makefile >> +++ b/drivers/gpu/drm/Makefile >> @@ -17,7 +17,8 @@ drm-y := drm_auth.o drm_cache.o \ >> drm_plane.o drm_color_mgmt.o drm_print.o \ >> drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \ >> drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \ >> -drm_client_modeset.o drm_atomic_uapi
Re: [PATCH V2] drm/imx: parallel-display: Adjust bus_flags handling
Hi Marek, On Mon, 2020-03-09 at 21:18 +0100, Marek Vasut wrote: > The bus_flags handling logic does not seem to cover all potential > usecases. Specifically, this seems to fail with an "edt,etm0700g0edh6" > display attached to an 24bit display interface, with interface-pix-fmt > = "rgb24" set in DT. > > This patch fixes the problem by overriding the imx_crtc_state->bus_flags > from the imxpd->bus_flags only if the DT property "interface-pix-fmt" is > present or if the DI provides no formats. > > Signed-off-by: Marek Vasut > Cc: Daniel Vetter > Cc: David Airlie > Cc: Fabio Estevam > Cc: NXP Linux Team > Cc: Philipp Zabel > Cc: Sascha Hauer > Cc: Shawn Guo > Cc: linux-arm-ker...@lists.infradead.org > To: dri-devel@lists.freedesktop.org > --- > V2: Rebase on next, update description Thank you for the update, I've applied this patch to imx-drm/next. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 22/51] drm: manage drm_minor cleanup with drmm_
Hi Am 02.03.20 um 23:26 schrieb Daniel Vetter: > The cleanup here is somewhat tricky, since we can't tell apart the > allocated minor index from 0. So register a cleanup action first, and > if the index allocation fails, unregister that cleanup action again to > avoid bad mistakes. > > The kdev for the minor already handles NULL, so no problem there. > > Hence add drmm_remove_action() to the drm_managed library. > > v2: Make pointer math around void ** consistent with what Laurent > suggested. > > Cc: Laurent Pinchart > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_drv.c | 74 +-- > drivers/gpu/drm/drm_managed.c | 28 + > include/drm/drm_managed.h | 4 ++ > 3 files changed, 59 insertions(+), 47 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 23e5b0e7e041..29d106195ab3 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -93,19 +93,35 @@ static struct drm_minor **drm_minor_get_slot(struct > drm_device *dev, > } > } > > +static void drm_minor_alloc_release(struct drm_device *dev, void *data) > +{ > + struct drm_minor *minor = data; > + unsigned long flags; > + > + put_device(minor->kdev); > + > + spin_lock_irqsave(&drm_minor_lock, flags); > + idr_remove(&drm_minors_idr, minor->index); > + spin_unlock_irqrestore(&drm_minor_lock, flags); > +} > + > static int drm_minor_alloc(struct drm_device *dev, unsigned int type) > { > struct drm_minor *minor; > unsigned long flags; > int r; > > - minor = kzalloc(sizeof(*minor), GFP_KERNEL); > + minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL); > if (!minor) > return -ENOMEM; > > minor->type = type; > minor->dev = dev; > > + r = drmm_add_action(dev, drm_minor_alloc_release, minor); > + if (r) > + return r; > + > idr_preload(GFP_KERNEL); > spin_lock_irqsave(&drm_minor_lock, flags); > r = idr_alloc(&drm_minors_idr, > @@ -116,47 +132,18 @@ static int drm_minor_alloc(struct drm_device *dev, > unsigned int type) > spin_unlock_irqrestore(&drm_minor_lock, flags); > idr_preload_end(); > > - if (r < 0) > - goto err_free; > + if (r < 0) { > + drmm_remove_action(dev, drm_minor_alloc_release, minor); > + return r; > + } > > minor->index = r; > - > minor->kdev = drm_sysfs_minor_alloc(minor); > - if (IS_ERR(minor->kdev)) { > - r = PTR_ERR(minor->kdev); > - goto err_index; > - } > + if (IS_ERR(minor->kdev)) > + return PTR_ERR(minor->kdev); > > *drm_minor_get_slot(dev, type) = minor; > return 0; > - > -err_index: > - spin_lock_irqsave(&drm_minor_lock, flags); > - idr_remove(&drm_minors_idr, minor->index); > - spin_unlock_irqrestore(&drm_minor_lock, flags); > -err_free: > - kfree(minor); > - return r; > -} TBH, I think you're reducing code quality by removing the rollback code from init functions, just for the sake of it. Specifically in this case here, you saved a few lines of code, but the overall flow is way more complicated to follow. That's typically a reliably source of bugs. This call to drmm_remove_action() just makes it worse. Rather, see my remark on OOP destruction in patch 21. For now, I'd focus on the device cleanup and leave init functions as they are. Best regards Thomas > - > -static void drm_minor_free(struct drm_device *dev, unsigned int type) > -{ > - struct drm_minor **slot, *minor; > - unsigned long flags; > - > - slot = drm_minor_get_slot(dev, type); > - minor = *slot; > - if (!minor) > - return; > - > - put_device(minor->kdev); > - > - spin_lock_irqsave(&drm_minor_lock, flags); > - idr_remove(&drm_minors_idr, minor->index); > - spin_unlock_irqrestore(&drm_minor_lock, flags); > - > - kfree(minor); > - *slot = NULL; > } > > static int drm_minor_register(struct drm_device *dev, unsigned int type) > @@ -678,16 +665,16 @@ int drm_dev_init(struct drm_device *dev, > if (drm_core_check_feature(dev, DRIVER_RENDER)) { > ret = drm_minor_alloc(dev, DRM_MINOR_RENDER); > if (ret) > - goto err_minors; > + goto err; > } > > ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY); > if (ret) > - goto err_minors; > + goto err; > > ret = drm_legacy_create_map_hash(dev); > if (ret) > - goto err_minors; > + goto err; > > drm_legacy_ctxbitmap_init(dev); > > @@ -695,7 +682,7 @@ int drm_dev_init(struct drm_device *dev, > ret = drm_gem_init(dev); > if (ret) { > DRM_ERROR("Cannot initialize graphics execution manager > (GEM)\n"); > - goto err_ctxbitmap; > +
Re: [PATCH v8 11/12] backlight: pwm_bl: Use 64-bit division function
On Tue, Mar 10, 2020 at 06:41:20PM -0700, Guru Das Srinagesh wrote: > Since the PWM framework is switching struct pwm_state.period's datatype > to u64, prepare for this transition by using div_u64 to handle a 64-bit > dividend instead of a straight division operation. > > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han > Cc: Bartlomiej Zolnierkiewicz > Cc: linux-...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: linux-fb...@vger.kernel.org > > Signed-off-by: Guru Das Srinagesh Reviewed-by: Daniel Thompson > --- > drivers/video/backlight/pwm_bl.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/video/backlight/pwm_bl.c > b/drivers/video/backlight/pwm_bl.c > index efb4efc..3e5dbcf 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -625,7 +625,8 @@ static int pwm_backlight_probe(struct platform_device > *pdev) > pb->scale = data->max_brightness; > } > > - pb->lth_brightness = data->lth_brightness * (state.period / pb->scale); > + pb->lth_brightness = data->lth_brightness * (div_u64(state.period, > + pb->scale)); > > props.type = BACKLIGHT_RAW; > props.max_brightness = data->max_brightness; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] arm64: dts: renesas: Add HiHope RZ/G2M board with idk-1110wr display
Hi Lad, On Tue, Mar 10, 2020 at 9:42 PM Lad Prabhakar wrote: > From: Fabrizio Castro > > The HiHope RZ/G2M is advertised as compatible with panel idk-1110wr > from Advantech, however the panel isn't sold alongside the board. > A new dts, adding everything that's required to get the panel to > work the HiHope RZ/G2M, is the most convenient way to support the > HiHope RZ/G2M when it's connected to the idk-1110wr. > > Signed-off-by: Fabrizio Castro > Acked-by: Laurent Pinchart > Signed-off-by: Lad Prabhakar Thanks for picking up this patch! > --- /dev/null > +++ b/arch/arm64/boot/dts/renesas/r8a774a1-hihope-rzg2m-ex-idk-1110wr.dts > @@ -0,0 +1,86 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the HiHope RZ/G2M sub board connected to an > + * Advantech IDK-1110WR 10.1" LVDS panel > + * > + * Copyright (C) 2020 Renesas Electronics Corp. > + */ > + > +#include "r8a774a1-hihope-rzg2m-ex.dts" > + > +/ { > + backlight { > + compatible = "pwm-backlight"; > + pwms = <&pwm0 0 5>; > + > + brightness-levels = <0 2 8 16 32 64 128 255>; > + default-brightness-level = <6>; > + }; > + > + panel-lvds { > + compatible = "advantech,idk-1110wr", "panel-lvds"; > + > + width-mm = <223>; > + height-mm = <125>; > + > + data-mapping = "jeida-24"; > + > + panel-timing { > + /* 1024x600 @60Hz */ > + clock-frequency = <5120>; > + hactive = <1024>; > + vactive = <600>; > + hsync-len = <240>; > + hfront-porch = <40>; > + hback-porch = <40>; > + vfront-porch = <15>; > + vback-porch = <10>; > + vsync-len = <10>; > + }; > + > + port { > + panel_in: endpoint { > + remote-endpoint = <&lvds0_out>; > + }; > + }; > + }; I believe the plan was to include the existing rzg2-advantech-idk-1110wr-panel.dtsi instead, to provide the panel-lvds node... > +}; > + > +&gpio1 { > + /* > +* When GP1_20 is LOW LVDS0 is connected to the LVDS connector > +* When GP1_20 is HIGH LVDS0 is connected to the LT8918L > +*/ > + lvds-connector-en-gpio { > + gpio-hog; > + gpios = <20 GPIO_ACTIVE_HIGH>; > + output-low; > + line-name = "lvds-connector-en-gpio"; > + }; > +}; > + > +&lvds0 { > + status = "okay"; > + > + ports { > + port@1 { > + lvds0_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > +}; ... and the lvds_connector endpoint configuration. The rest looks good to me. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: sysfs: Use scnprintf() for avoiding potential buffer overflow
Hi Am 11.03.20 um 09:24 schrieb Takashi Iwai: > On Wed, 11 Mar 2020 09:10:56 +0100, > Thomas Zimmermann wrote: >> >> Hi Takashi >> >> Am 11.03.20 um 08:35 schrieb Takashi Iwai: >>> Since snprintf() returns the would-be-output size instead of the >>> actual output size, the succeeding calls may go beyond the given >>> buffer limit. Fix it by replacing with scnprintf(). >>> >>> Signed-off-by: Takashi Iwai >>> --- >>> drivers/gpu/drm/drm_sysfs.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c >>> index dd2bc85f43cc..9b3180e8c12f 100644 >>> --- a/drivers/gpu/drm/drm_sysfs.c >>> +++ b/drivers/gpu/drm/drm_sysfs.c >>> @@ -230,7 +230,7 @@ static ssize_t modes_show(struct device *device, >>> >>> mutex_lock(&connector->dev->mode_config.mutex); >>> list_for_each_entry(mode, &connector->modes, head) { >>> - written += snprintf(buf + written, PAGE_SIZE - written, "%s\n", >>> + written += scnprintf(buf + written, PAGE_SIZE - written, "%s\n", >>> mode->name); >>> } >>> mutex_unlock(&connector->dev->mode_config.mutex); >>> >> >> In drm_sysfs.c, there are more _show functions with calls to snprintf() >> that could be replaced by scnprintf(). ATM they don't return the correct >> length for output that exceeds PAGE_SIZE. since you're at it, you may >> replace them as well. > > Well, the rest snprintf() calls are single calls and can't be over > PAGE_SIZE obviously. IOW, they could be rather replaced with > sprintf() instead, for a sake of simplicity. Admittedly, none of these strings look as if they ever go beyond PAGE_SIZE, but sncprintf() is still a simple way of defensive programming here (and returns the correct value). > >> But in any case >> >> Reviewed-by: Thomas Zimmermann >> >> for this patch. >> >> Do you want me to merge the patch into drm-misc-next? > > Yes, please. OK, will do later today. Best regards Thomas > > > thanks, > > Takashi > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V3 2/4] backlight: qcom-wled: Add callback functions
On Wed, Mar 11, 2020 at 12:11:00PM +0530, kgu...@codeaurora.org wrote: > On 2020-03-10 20:57, Daniel Thompson wrote: > > On Mon, Mar 09, 2020 at 06:56:00PM +0530, Kiran Gunda wrote: > > > Add cabc_config, sync_toggle, wled_ovp_fault_status and wled_ovp_delay > > > callback functions to prepare the driver for adding WLED5 support. > > > > > > Signed-off-by: Kiran Gunda > > > > Overall this code would a lot easier to review if > > > --- > > > drivers/video/backlight/qcom-wled.c | 196 > > > +++- > > > 1 file changed, 126 insertions(+), 70 deletions(-) > > > > > > diff --git a/drivers/video/backlight/qcom-wled.c > > > b/drivers/video/backlight/qcom-wled.c > > > index 3d276b3..b73f273 100644 > > > --- a/drivers/video/backlight/qcom-wled.c > > > +++ b/drivers/video/backlight/qcom-wled.c > > > @@ -128,6 +128,7 @@ struct wled_config { > > > bool cs_out_en; > > > bool ext_gen; > > > bool cabc; > > > + bool en_cabc; > > > > Does this ever get set to true? > > > Yes. If user wants use the cabc pin to control the brightness and > use the "qcom,cabc" DT property in the device tree. That sounds like what you intended the code to do! Is the code that does this present in the patch? I could not find it. Daniel. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 0/8] *** Per context fencing ***
Hi, > I should've been more clear -- this is an internal cleanup/preparation and > the per-context changes are invisible to host userspace. Ok, it wasn't clear that you don't flip the switch yet. In general the commit messages could be a bit more verbose ... I'm wondering though why we need the new fence_id in the first place. Isn't it enough to have per-context (instead of global) last_seq? > Multi-queue sounds very interesting indeed, especially with VK > multi-threaded command submission. That to me is V3 rather than V2.. let's > start easy! Having v2 if we plan to obsolete it with v3 soon doesn't look like a good plan to me. It'll make backward compatibility more complex for no good reason ... Also: Does virglrenderer render different contexts in parallel today? Only in case it does we'll actually get benefits from per-context fences. But I think it doesn't, so there is no need to rush. I think we should better have a rough plan for parallel rendering first, then go start implementing the pieces needed. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next 019/491] Hyper-V CORE AND DRIVERS: Use fallthrough;
On Tue, Mar 10, 2020 at 09:51:33PM -0700, Joe Perches wrote: > Convert the various uses of fallthrough comments to fallthrough; > > Done via script > Link: > https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/ > > Signed-off-by: Joe Perches Reviewed-by: Wei Liu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/i915: Init lspcon after HPD in intel_dp_detect()
On Sat, Feb 15, 2020 at 01:56:27AM +0800, Kai-Heng Feng wrote: > On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port > becomes useless and never responds to cable hotplugging: > [3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon > [3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port > D > > Seems like the lspcon chip on the system in question only gets powered > after the cable is plugged. > > So let's call lspcon_init() dynamically to properly initialize the > lspcon chip and make HDMI port work. > > Signed-off-by: Kai-Heng Feng > --- > v4: > - Trust VBT in intel_infoframe_init(). > - Init lspcon in intel_dp_detect(). > > v3: > - Make sure it's handled under long HPD case. > > v2: > - Move lspcon_init() inside of intel_dp_hpd_pulse(). > > drivers/gpu/drm/i915/display/intel_ddi.c | 17 + > drivers/gpu/drm/i915/display/intel_dp.c | 13 - > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- > 3 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 33f1dc3d7c1a..ca717434b406 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -4741,7 +4741,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) > &dev_priv->vbt.ddi_port_info[port]; > struct intel_digital_port *intel_dig_port; > struct intel_encoder *encoder; > - bool init_hdmi, init_dp, init_lspcon = false; > + bool init_hdmi, init_dp; > enum phy phy = intel_port_to_phy(dev_priv, port); > > init_hdmi = port_info->supports_dvi || port_info->supports_hdmi; > @@ -4754,7 +4754,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) >* is initialized before lspcon. >*/ > init_dp = true; > - init_lspcon = true; > init_hdmi = false; > DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port)); > } > @@ -4833,20 +4832,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) > goto err; > } > > - if (init_lspcon) { > - if (lspcon_init(intel_dig_port)) > - /* TODO: handle hdmi info frame part */ > - DRM_DEBUG_KMS("LSPCON init success on port %c\n", > - port_name(port)); > - else > - /* > - * LSPCON init faied, but DP init was success, so > - * lets try to drive as DP++ port. > - */ > - DRM_ERROR("LSPCON init failed on port %c\n", > - port_name(port)); > - } > - > intel_infoframe_init(intel_dig_port); > > return; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index c7424e2a04a3..43117aa86292 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5663,8 +5663,19 @@ intel_dp_detect(struct drm_connector *connector, > /* Can't disconnect eDP */ > if (intel_dp_is_edp(intel_dp)) > status = edp_detect(intel_dp); > - else if (intel_digital_port_connected(encoder)) > + else if (intel_digital_port_connected(encoder)) { > + if (intel_bios_is_lspcon_present(dev_priv, dig_port->base.port) > && > + !dig_port->lspcon.active) { > + if (lspcon_init(dig_port)) > + DRM_DEBUG_KMS("LSPCON init success on port > %c\n", > + port_name(dig_port->base.port)); > + else > + DRM_DEBUG_KMS("LSPCON init failed on port %c\n", > + port_name(dig_port->base.port)); > + } I was going to ask what happens when you unplug+replug, but looks like we already have lspcon_resume()in intel_dp_detect_dpcd(). This should be there as well. In fact I think we should just move all the logic into the lspcon code and let it decide on its own whether to take the init path or the resume path (assuming there is even any difference between the two). Not sure what we should do with the lspcon_resume() call in intel_dp_encoder_reset()... > + > status = intel_dp_detect_dpcd(intel_dp); > + } > else > status = connector_status_disconnected; > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c > b/drivers/gpu/drm/i915/display/intel_hdmi.c > index 93ac0f296852..27a5aa8cefc9 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > @@ -3100,7 +3100,7 @@ void intel_infoframe_init(struct intel_digital_port > *intel_dig_port) >
Re: [PATCH] drm: rework SET_MASTER and DROP_MASTER perm handling
On Mon, 9 Mar 2020 at 18:36, Emil Velikov wrote: > > On Mon, 9 Mar 2020 at 13:13, Emil Velikov wrote: > > > > OTOH, if applications exist that rely on drop-master failing in this > > > specific case, making drop-master succeed would break them. That might > > > include a buggy set-master path that was written, but does not actually > > > work because it was never tested since drop-master never worked. > > > > > > So I do not fully buy this argument yet, but I also cannot name any > > > explicit examples that would break. > > > > > > > > I've ventured for a while in the X (Xorg + drivers), Weston, > > sway/wlroots and Mesa's codebase. > > There were zero instances of such misuse. If other projects come to > > mind - I'll gladly take a look. > > > Just checked a few other projects with git pickaxe* - zero cases of > mentioned (mis)use. In particular: > - qtbase + qtwayland + gtk > Never used the wrappers or ioctls > > - kwin + plasmashell > Never used the wrappers or ioctls > > - mutter + gnome-shell > Briefly used the wrappers. Sane codepath > > - igt-gpu-tools ... just because I had it open > Sane use both wrappers and ioctls. > > Any other projects I should check? > Coming back from an interesting venture in efl world: - enlightment - correctly usage during 2012-2014, switched to ecore_drm - evas (imlib2 successor) correct usage, few months in 2014, switched to ecore_drm - legacy/ecore - never used either - ecore - ecore_drm backend, correct usage of drmSet/DropMaster Given the above, it does seem that the raised concern is more or less hypothetical. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 0/5] arm/arm64: mediatek: Fix mt8173 mmsys device probing
Hi, Enric: I'm confused this is v11 or v12. For v12, you've lost some 'Acked-by' and 'Reviewed-by' tag. Regards, CK On Wed, 2020-03-11 at 12:56 +0100, Enric Balletbo i Serra wrote: > Dear all, > > These patches are intended to solve an old standing issue on some > Mediatek devices (mt8173, mt2701 and mt2712 are affected by this issue). > > Up to now both drivers, clock and drm are probed with the same device tree > compatible. But only the first driver gets probed, which in effect breaks > graphics on those devices. > > The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to > control clock gates (which is used in the clk driver) and some registers > to set the routing and enable the differnet blocks of the display > and MDP (Media Data Path) subsystem. On this series the clk driver is > not a pure clock controller but a system controller that can provide > access to the shared registers between the different drivers that need > it (mediatek-drm and mediatek-mdp). Hence the MMSYS clk driver was moved > to drivers/soc/mediatek and is the entry point (parent) which will trigger > the probe of the corresponding mediatek-drm driver. > > **IMPORTANT** This series only fixes the issue on mt8173 to make it > simple and as is the only platform I can test. Similar changes should be > applied for mt2701 and mt2712 to have display working. > > These patches apply on top of linux-next. > > For reference, here are the links to the old discussions: > * v10: https://patchwork.kernel.org/project/linux-mediatek/list/?series=248505 > * v9: https://patchwork.kernel.org/project/linux-clk/list/?series=247591 > * v8: https://patchwork.kernel.org/project/linux-mediatek/list/?series=244891 > * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217 > * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219 > * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063 > * v4: > * https://patchwork.kernel.org/patch/10530871/ > * https://patchwork.kernel.org/patch/10530883/ > * https://patchwork.kernel.org/patch/10530885/ > * https://patchwork.kernel.org/patch/10530911/ > * https://patchwork.kernel.org/patch/10530913/ > * v3: > * https://patchwork.kernel.org/patch/10367857/ > * https://patchwork.kernel.org/patch/10367861/ > * https://patchwork.kernel.org/patch/10367877/ > * https://patchwork.kernel.org/patch/10367875/ > * https://patchwork.kernel.org/patch/10367885/ > * https://patchwork.kernel.org/patch/10367883/ > * https://patchwork.kernel.org/patch/10367889/ > * https://patchwork.kernel.org/patch/10367907/ > * https://patchwork.kernel.org/patch/10367909/ > * https://patchwork.kernel.org/patch/10367905/ > * v2: No relevant discussion, see v3 > * v1: > * https://patchwork.kernel.org/patch/10016497/ > * https://patchwork.kernel.org/patch/10016499/ > * https://patchwork.kernel.org/patch/10016505/ > * https://patchwork.kernel.org/patch/10016507/ > > Best regards, > Enric > > Changes in v11: > - Leave the clocks part in drivers/clk (clk-mt8173-mm) > - Instantiate the clock driver from the mtk-mmsys driver. > - Add default config option to not break anything. > - Removed the Reviewed-by CK tag as changed the organization. > > Changes in v10: > - Update the binding documentation for the mmsys system controller. > - Renamed to be generic mtk-mmsys > - Add driver data support to be able to support diferent SoCs > - Select CONFIG_MTK_MMSYS (CK) > - Pass device pointer of mmsys device instead of config regs (CK) > - Match driver data to get display routing. > > Changes in v9: > - Move mmsys to drivers/soc/mediatek (CK) > - Introduced a new patch to move routing control into mmsys driver. > - Removed the patch to use regmap as is not needed anymore. > - Do not move the display routing from the drm driver (CK) > > Changes in v8: > - Be a builtin_platform_driver like other mediatek mmsys drivers. > - New patch introduced in this series. > > Changes in v7: > - Free clk_data->clks as well > - Get rid of private data structure > > Enric Balletbo i Serra (3): > dt-bindings: mediatek: Update mmsys binding to reflect it is a system > controller > soc / drm: mediatek: Move routing control to mmsys device > soc / drm: mediatek: Fix mediatek-drm device probing > > Matthias Brugger (2): > drm/mediatek: Omit warning on probe defers > clk / soc: mediatek: Move mt8173 MMSYS to platform driver > > .../bindings/arm/mediatek/mediatek,mmsys.txt | 7 +- > drivers/clk/mediatek/Kconfig | 7 + > drivers/clk/mediatek/Makefile | 1 + > drivers/clk/mediatek/clk-mt8173-mm.c | 146 > drivers/clk/mediatek/clk-mt8173.c | 104 -- > drivers/gpu/drm/mediatek/Kconfig | 1 + > drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 +- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 +- > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 +-
Re: [PATCH v11 3/5] clk / soc: mediatek: Move mt8173 MMSYS to platform driver
Hi, Enric: On Wed, 2020-03-11 at 12:56 +0100, Enric Balletbo i Serra wrote: > From: Matthias Brugger > > There is no strong reason for this to use CLK_OF_DECLARE instead of > being a platform driver. Plus, MMSYS provides clocks but also a shared > register space for the mediatek-drm and the mediatek-mdp > driver. So move the MMSYS clocks to a new platform driver and also > create a new MMSYS platform driver in drivers/soc/mediatek that > instantiates the clock driver. > Reviewed-by: CK Hu > Signed-off-by: Matthias Brugger > Signed-off-by: Enric Balletbo i Serra > --- > > Changes in v11: > - Leave the clocks part in drivers/clk (clk-mt8173-mm) > - Instantiate the clock driver from the mtk-mmsys driver. > - Add default config option to not break anything. > - Removed the Reviewed-by CK tag as changed the organization. > > Changes in v10: > - Renamed to be generic mtk-mmsys > - Add driver data support to be able to support diferent SoCs > > Changes in v9: > - Move mmsys to drivers/soc/mediatek (CK) > > Changes in v8: > - Be a builtin_platform_driver like other mediatek mmsys drivers. > > Changes in v7: > - Free clk_data->clks as well > - Get rid of private data structure > > drivers/clk/mediatek/Kconfig | 7 ++ > drivers/clk/mediatek/Makefile| 1 + > drivers/clk/mediatek/clk-mt8173-mm.c | 146 +++ > drivers/clk/mediatek/clk-mt8173.c| 104 --- > drivers/soc/mediatek/Kconfig | 8 ++ > drivers/soc/mediatek/Makefile| 1 + > drivers/soc/mediatek/mtk-mmsys.c | 50 + > 7 files changed, 213 insertions(+), 104 deletions(-) > create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c > create mode 100644 drivers/soc/mediatek/mtk-mmsys.c > > obj-$(CONFIG_COMMON_CLK_MT8183_CAMSYS) += clk-mt8183-cam.o ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 4/5] soc / drm: mediatek: Move routing control to mmsys device
Hi, Enric: On Wed, 2020-03-11 at 12:56 +0100, Enric Balletbo i Serra wrote: > Provide a mtk_mmsys_ddp_connect() and mtk_mmsys_disconnect() functions to > replace mtk_ddp_add_comp_to_path() and mtk_ddp_remove_comp_from_path(). > Those functions will allow DRM driver and others to control the data > path routing. > Reviewed-by: CK Hu > Signed-off-by: Enric Balletbo i Serra > Reviewed-by: Matthias Brugger > --- > > Changes in v11: None > Changes in v10: > - Select CONFIG_MTK_MMSYS (CK) > - Pass device pointer of mmsys device instead of config regs (CK) > > Changes in v9: > - Introduced a new patch to move routing control into mmsys driver. > - Removed the patch to use regmap as is not needed anymore. > > Changes in v8: None > Changes in v7: None > > drivers/gpu/drm/mediatek/Kconfig| 1 + > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 19 +- > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 256 -- > drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 7 - > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 14 +- > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +- > drivers/soc/mediatek/mtk-mmsys.c| 279 > include/linux/soc/mediatek/mtk-mmsys.h | 20 ++ > 8 files changed, 316 insertions(+), 282 deletions(-) > create mode 100644 include/linux/soc/mediatek/mtk-mmsys.h > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] RFC: dma-buf: Add an API for importing and exporting sync files (v4)
Am 11.03.20 um 04:43 schrieb Jason Ekstrand: Explicit synchronization is the future. At least, that seems to be what most userspace APIs are agreeing on at this point. However, most of our Linux APIs (both userspace and kernel UAPI) are currently built around implicit synchronization with dma-buf. While work is ongoing to change many of the userspace APIs and protocols to an explicit synchronization model, switching over piecemeal is difficult due to the number of potential components involved. On the kernel side, many drivers use dma-buf including GPU (3D/compute), display, v4l, and others. In userspace, we have X11, several Wayland compositors, 3D drivers, compute drivers (OpenCL etc.), media encode/decode, and the list goes on. This patch provides a path forward by allowing userspace to manually manage the fences attached to a dma-buf. Alternatively, one can think of this as making dma-buf's implicit synchronization simply a carrier for an explicit fence. This is accomplished by adding two IOCTLs to dma-buf for importing and exporting a sync file to/from the dma-buf. This way a userspace component which is uses explicit synchronization, such as a Vulkan driver, can manually set the write fence on a buffer before handing it off to an implicitly synchronized component such as a Wayland compositor or video encoder. In this way, each of the different components can be upgraded to an explicit synchronization model one at a time as long as the userspace pieces connecting them are aware of it and import/export fences at the right times. There is a potential race condition with this API if userspace is not careful. A typical use case for implicit synchronization is to wait for the dma-buf to be ready, use it, and then signal it for some other component. Because a sync_file cannot be created until it is guaranteed to complete in finite time, userspace can only signal the dma-buf after it has already submitted the work which uses it to the kernel and has received a sync_file back. There is no way to atomically submit a wait-use-signal operation. This is not, however, really a problem with this API so much as it is a problem with explicit synchronization itself. The way this is typically handled is to have very explicit ownership transfer points in the API or protocol which ensure that only one component is using it at any given time. Both X11 (via the PRESENT extension) and Wayland provide such ownership transfer points via explicit present and idle messages. The decision was intentionally made in this patch to make the import and export operations IOCTLs on the dma-buf itself rather than as a DRM IOCTL. This makes it the import/export operation universal across all components which use dma-buf including GPU, display, v4l, and others. It also means that a userspace component can do the import/export without access to the DRM fd which may be tricky to get in cases where the client communicates with DRM via a userspace API such as OpenGL or Vulkan. At a future date we may choose to add direct import/export APIs to components such as drm_syncobj to avoid allocating a file descriptor and going through two ioctls. However, that seems to be something of a micro-optimization as import/export operations are likely to happen at a rate of a few per frame of rendered or decoded video. v2 (Jason Ekstrand): - Use a wrapper dma_fence_array of all fences including the new one when importing an exclusive fence. v3 (Jason Ekstrand): - Lock around setting shared fences as well as exclusive - Mark SIGNAL_SYNC_FILE as a read-write ioctl. - Initialize ret to 0 in dma_buf_wait_sync_file v4 (Jason Ekstrand): - Use the new dma_resv_get_singleton helper Signed-off-by: Jason Ekstrand --- drivers/dma-buf/dma-buf.c| 96 include/uapi/linux/dma-buf.h | 13 - 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d4097856c86b..09973c689866 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -348,6 +349,95 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf) return ret; } +static long dma_buf_wait_sync_file(struct dma_buf *dmabuf, + const void __user *user_data) +{ + struct dma_buf_sync_file arg; + struct dma_fence *fence; + int ret = 0; + + if (copy_from_user(&arg, user_data, sizeof(arg))) + return -EFAULT; + + if (arg.flags != 0 && arg.flags != DMA_BUF_SYNC_FILE_SYNC_WRITE) + return -EINVAL; + + fence = sync_file_get_fence(arg.fd); + if (!fence) + return -EINVAL; + + dma_resv_lock(dmabuf->resv, NULL); + + if (arg.flags & DMA_BUF_SYNC_FILE_SYNC_WRITE) { + struct dma_fence *singleton =
[PATCH] DRM: ARC: PGU: interlaced mode not supported
Filter out interlaced modes as they are not supported by ARC PGU hardware. Signed-off-by: Eugeniy Paltsev --- drivers/gpu/drm/arc/arcpgu_crtc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index 8ae1e1f97a73..c854066d4c75 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -67,6 +67,9 @@ static enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, long rate, clk_rate = mode->clock * 1000; long diff = clk_rate / 200; /* +-0.5% allowed by HDMI spec */ + if (mode->flags & DRM_MODE_FLAG_INTERLACE) + return MODE_NO_INTERLACE; + rate = clk_round_rate(arcpgu->clk, clk_rate); if ((max(rate, clk_rate) - min(rate, clk_rate) < diff) && (rate > 0)) return MODE_OK; -- 2.21.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] DRM: ARC: PGU: interlaced mode not supported
On Wed, Mar 11, 2020 at 04:13:10PM +0300, Eugeniy Paltsev wrote: > Filter out interlaced modes as they are not supported by ARC PGU > hardware. > > Signed-off-by: Eugeniy Paltsev > --- > drivers/gpu/drm/arc/arcpgu_crtc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c > b/drivers/gpu/drm/arc/arcpgu_crtc.c > index 8ae1e1f97a73..c854066d4c75 100644 > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c > @@ -67,6 +67,9 @@ static enum drm_mode_status arc_pgu_crtc_mode_valid(struct > drm_crtc *crtc, > long rate, clk_rate = mode->clock * 1000; > long diff = clk_rate / 200; /* +-0.5% allowed by HDMI spec */ > > + if (mode->flags & DRM_MODE_FLAG_INTERLACE) > + return MODE_NO_INTERLACE; > + > rate = clk_round_rate(arcpgu->clk, clk_rate); > if ((max(rate, clk_rate) - min(rate, clk_rate) < diff) && (rate > 0)) > return MODE_OK; > -- > 2.21.1 > This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 4/5] soc / drm: mediatek: Move routing control to mmsys device
On Wed, 2020-03-11 at 14:25 +0100, Matthias Brugger wrote: > > On 11/03/2020 14:07, CK Hu wrote: > > Hi, Enric: > > > > On Wed, 2020-03-11 at 12:56 +0100, Enric Balletbo i Serra wrote: > >> Provide a mtk_mmsys_ddp_connect() and mtk_mmsys_disconnect() functions to > >> replace mtk_ddp_add_comp_to_path() and mtk_ddp_remove_comp_from_path(). > >> Those functions will allow DRM driver and others to control the data > >> path routing. > >> > > > > Reviewed-by: CK Hu > > > > If I remember correctly you are OK me taking the patch through the SoC tree, > right? > > In this case I'd need a Acked-by tag. Not a big deal, just trying to remeber > the > tag policy in the linux kernel :) > > Regards, > Matthias > Acked-by: CK Hu > >> Signed-off-by: Enric Balletbo i Serra > >> Reviewed-by: Matthias Brugger > >> --- > >> > >> Changes in v11: None > >> Changes in v10: > >> - Select CONFIG_MTK_MMSYS (CK) > >> - Pass device pointer of mmsys device instead of config regs (CK) > >> > >> Changes in v9: > >> - Introduced a new patch to move routing control into mmsys driver. > >> - Removed the patch to use regmap as is not needed anymore. > >> > >> Changes in v8: None > >> Changes in v7: None > >> > >> drivers/gpu/drm/mediatek/Kconfig| 1 + > >> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 19 +- > >> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 256 -- > >> drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 7 - > >> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 14 +- > >> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +- > >> drivers/soc/mediatek/mtk-mmsys.c| 279 > >> include/linux/soc/mediatek/mtk-mmsys.h | 20 ++ > >> 8 files changed, 316 insertions(+), 282 deletions(-) > >> create mode 100644 include/linux/soc/mediatek/mtk-mmsys.h > >> > > > > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 5/5] soc / drm: mediatek: Fix mediatek-drm device probing
On Wed, 2020-03-11 at 14:26 +0100, Matthias Brugger wrote: > > On 11/03/2020 12:56, Enric Balletbo i Serra wrote: > > In the actual implementation the same compatible string > > "mediatek,-mmsys" is used to bind the clock drivers > > (drivers/soc/mediatek) as well as to the gpu driver > > (drivers/gpu/drm/mediatek/mtk_drm_drv.c). This ends with the problem > > that the only probed driver is the clock driver and there is no display > > at all. > > > > In any case having the same compatible string for two drivers is not > > correct and should be fixed. To fix this, and maintain backward > > compatibility, we can consider that the mmsys driver is the top-level > > entry point for the multimedia subsystem, so is not a pure clock > > controller but a system controller, and the drm driver is instantiated > > by that MMSYS driver. > > > > Signed-off-by: Enric Balletbo i Serra > > Reviewed-by: CK Hu > > Same here. Acked-by: CK Hu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v13 6/6] drm/mediatek: set dpi pin mode to gpio low to avoid leakage current
Jitao Shi 於 2020年3月11日 週三 下午3:19寫道: > Config dpi pins mode to output and pull low when dpi is disabled. > Aovid leakage current from some dpi pins (Hsync Vsync DE ... ). > > Reviewed-by: Chun-Kuang Hu > Signed-off-by: Jitao Shi > --- > drivers/gpu/drm/mediatek/mtk_dpi.c | 31 ++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > b/drivers/gpu/drm/mediatek/mtk_dpi.c > index 2871e68e7767..b6359e979588 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > @@ -10,7 +10,9 @@ > #include > #include > #include > +#include > #include > +#include > #include > #include > > @@ -74,6 +76,9 @@ struct mtk_dpi { > enum mtk_dpi_out_yc_map yc_map; > enum mtk_dpi_out_bit_num bit_num; > enum mtk_dpi_out_channel_swap channel_swap; > + struct pinctrl *pinctrl; > + struct pinctrl_state *pins_gpio; > + struct pinctrl_state *pins_dpi; > int refcount; > u32 pclk_sample; > }; > @@ -387,6 +392,9 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi) > if (--dpi->refcount != 0) > return; > > + if (dpi->pinctrl && dpi->pins_gpio) > + pinctrl_select_state(dpi->pinctrl, dpi->pins_gpio); > + > mtk_dpi_disable(dpi); > clk_disable_unprepare(dpi->pixel_clk); > clk_disable_unprepare(dpi->engine_clk); > @@ -411,6 +419,9 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi) > goto err_pixel; > } > > + if (dpi->pinctrl && dpi->pins_dpi) > + pinctrl_select_state(dpi->pinctrl, dpi->pins_dpi); > + > mtk_dpi_enable(dpi); > return 0; > > @@ -728,6 +739,26 @@ static int mtk_dpi_probe(struct platform_device *pdev) > of_property_read_u32(ep, "pclk-sample", &dpi->pclk_sample); > of_node_put(ep); > > + dpi->pinctrl = devm_pinctrl_get(&pdev->dev); > + if (IS_ERR(dpi->pinctrl)) { > + dpi->pinctrl = NULL; > + dev_dbg(&pdev->dev, "Cannot find pinctrl!\n"); > + } > + if (dpi->pinctrl) { > + dpi->pins_gpio = pinctrl_lookup_state(dpi->pinctrl, > "sleep"); > + if (IS_ERR(dpi->pins_gpio)) { > + dpi->pins_gpio = NULL; > + dev_dbg(&pdev->dev, "Cannot find pinctrl idle!\n"); > + } > + if (dpi->pins_gpio) > + pinctrl_select_state(dpi->pinctrl, dpi->pins_gpio); > + > + dpi->pins_dpi = pinctrl_lookup_state(dpi->pinctrl, > "default"); > + if (IS_ERR(dpi->pins_dpi)) { > + dpi->pins_dpi = NULL; > + dev_dbg(&pdev->dev, "Cannot find pinctrl > active!\n"); > + } > + } > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > dpi->regs = devm_ioremap_resource(dev, mem); > if (IS_ERR(dpi->regs)) { > -- > 2.21.0 > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v13 2/6] dt-bindings: display: mediatek: control dpi pins mode to avoid leakage
Jitao Shi 於 2020年3月11日 週三 下午3:18寫道: > Add property "pinctrl-names" to swap pin mode between gpio and dpi mode. > Set > the dpi pins to gpio mode and output-low to avoid leakage current when dpi > disabled. > Reviewed-by: Chun-Kuang Hu > > Signed-off-by: Jitao Shi > --- > .../devicetree/bindings/display/mediatek/mediatek,dpi.txt | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt > index 58914cf681b8..260ae75ac640 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt > @@ -17,6 +17,10 @@ Required properties: >Documentation/devicetree/bindings/graph.txt. This port should be > connected >to the input port of an attached HDMI or LVDS encoder chip. > > +Optional properties: > +- pinctrl-names: Contain "default" and "sleep". > + pinctrl-names see > Documentation/devicetree/bindings/pinctrlpinctrl-bindings.txt > + > Example: > > dpi0: dpi@1401d000 { > @@ -27,6 +31,9 @@ dpi0: dpi@1401d000 { > <&mmsys CLK_MM_DPI_ENGINE>, > <&apmixedsys CLK_APMIXED_TVDPLL>; > clock-names = "pixel", "engine", "pll"; > + pinctrl-names = "default", "sleep"; > + pinctrl-0 = <&dpi_pin_func>; > + pinctrl-1 = <&dpi_pin_idle>; > > port { > dpi0_out: endpoint { > -- > 2.21.0 > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
P2P for DMA-buf
This is the third and final part of my series to start supporting P2P with DMA-buf. The implementation is straight forward, apart from a helper to aid constructing scatterlists without having struct pages we only add a new flag indicating that an DMA-buf importer can handle peer2peer. The exporter can then check if P2P is general possible using the pci_p2pdma_distance_many() function and if necessary can also clear the flag. The rest is an example how to implementing the necessary functionality into the amdgpu driver to setup scatterlists pointing to device memory. Please review and comment, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] drm/amdgpu: note that we can handle peer2peer DMA-buf
Importing should work out of the box. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index ffeb20f11c07..aef12ee2f1e3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -514,6 +514,7 @@ amdgpu_dma_buf_move_notify(struct dma_buf_attachment *attach) } static const struct dma_buf_attach_ops amdgpu_dma_buf_attach_ops = { + .allow_peer2peer = true, .move_notify = amdgpu_dma_buf_move_notify }; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] lib/scatterlist: add sg_set_dma_addr() function
This can be used by drivers to setup P2P DMA between device memory which is not backed by struct pages. The drivers of the involved devices are responsible for setting up and tearing down DMA addresses as necessary using dma_map_resource(). The page pointer is set to NULL and only the DMA address, length and offset values are valid. Signed-off-by: Christian König --- include/linux/scatterlist.h | 23 +++ 1 file changed, 23 insertions(+) diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 6eec50fb36c8..28a477bf0bdf 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -145,6 +145,29 @@ static inline void sg_set_buf(struct scatterlist *sg, const void *buf, sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf)); } +/** + * sg_set_dma_addr - Set sg entry to point at specified dma address + * @sg: SG entry + * @address:DMA address to set + * @len:Length of data + * @offset: Offset into page + * + * Description: + * Use this function to set an sg entry to point to device resources mapped + * using dma_map_resource(). The page pointer is set to NULL and only the DMA + * address, length and offset values are valid. + * + **/ +static inline void sg_set_dma_addr(struct scatterlist *sg, dma_addr_t address, + unsigned int len, unsigned int offset) +{ + sg_set_page(sg, NULL, len, offset); + sg->dma_address = address; +#ifdef CONFIG_NEED_SG_DMA_LENGTH + sg->dma_length = len; +#endif +} + /* * Loop over each sg element, following the pointer to a new list if necessary */ -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] dma-buf: add peer2peer flag
Add a peer2peer flag noting that the importer can deal with device resources which are not backed by pages. Signed-off-by: Christian König --- drivers/dma-buf/dma-buf.c | 2 ++ include/linux/dma-buf.h | 10 ++ 2 files changed, 12 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index f4ace9af2191..f9220928ec90 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -689,6 +689,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, attach->dev = dev; attach->dmabuf = dmabuf; + if (importer_ops) + attach->peer2peer = importer_ops->allow_peer2peer; attach->importer_ops = importer_ops; attach->importer_priv = importer_priv; diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 1ade486fc2bb..82e0a4a64601 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -334,6 +334,14 @@ struct dma_buf { * Attachment operations implemented by the importer. */ struct dma_buf_attach_ops { + /** +* @allow_peer2peer: +* +* If this is set to true the importer must be able to handle peer +* resources without struct pages. +*/ + bool allow_peer2peer; + /** * @move_notify * @@ -362,6 +370,7 @@ struct dma_buf_attach_ops { * @node: list of dma_buf_attachment, protected by dma_resv lock of the dmabuf. * @sgt: cached mapping. * @dir: direction of cached mapping. + * @peer2peer: true if the importer can handle peer resources without pages. * @priv: exporter specific attachment data. * @importer_ops: importer operations for this attachment, if provided * dma_buf_map/unmap_attachment() must be called with the dma_resv lock held. @@ -382,6 +391,7 @@ struct dma_buf_attachment { struct list_head node; struct sg_table *sgt; enum dma_data_direction dir; + bool peer2peer; const struct dma_buf_attach_ops *importer_ops; void *importer_priv; void *priv; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] drm/amdgpu: add checks if DMA-buf P2P is supported
Check if we can do peer2peer on the PCIe bus. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index aef12ee2f1e3..bbf67800c8a6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -38,6 +38,7 @@ #include #include #include +#include /** * amdgpu_gem_prime_vmap - &dma_buf_ops.vmap implementation @@ -179,6 +180,9 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf, struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); int r; + if (pci_p2pdma_distance_many(adev->pdev, &attach->dev, 1, true) < 0) + attach->peer2peer = false; + if (attach->dev->driver == adev->dev->driver) return 0; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] drm/amdgpu: add support for exporting VRAM using DMA-buf v2
We should be able to do this now after checking all the prerequisites. v2: fix entrie count in the sgt Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 56 --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 12 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 97 3 files changed, 151 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index bbf67800c8a6..43d8ed7dbd00 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -276,14 +276,21 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach, struct dma_buf *dma_buf = attach->dmabuf; struct drm_gem_object *obj = dma_buf->priv; struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj); + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); struct sg_table *sgt; long r; if (!bo->pin_count) { - /* move buffer into GTT */ + /* move buffer into GTT or VRAM */ struct ttm_operation_ctx ctx = { false, false }; + unsigned domains = AMDGPU_GEM_DOMAIN_GTT; - amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT); + if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM && + attach->peer2peer) { + bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; + domains |= AMDGPU_GEM_DOMAIN_VRAM; + } + amdgpu_bo_placement_from_domain(bo, domains); r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx); if (r) return ERR_PTR(r); @@ -293,20 +300,34 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach, return ERR_PTR(-EBUSY); } - sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, bo->tbo.num_pages); - if (IS_ERR(sgt)) - return sgt; - - if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, - DMA_ATTR_SKIP_CPU_SYNC)) - goto error_free; + switch (bo->tbo.mem.mem_type) { + case TTM_PL_TT: + sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, + bo->tbo.num_pages); + if (IS_ERR(sgt)) + return sgt; + + if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, + DMA_ATTR_SKIP_CPU_SYNC)) + goto error_free; + break; + + case TTM_PL_VRAM: + r = amdgpu_vram_mgr_alloc_sgt(adev, &bo->tbo.mem, attach->dev, + dir, &sgt); + if (r) + return ERR_PTR(r); + break; + default: + return ERR_PTR(-EINVAL); + } return sgt; error_free: sg_free_table(sgt); kfree(sgt); - return ERR_PTR(-ENOMEM); + return ERR_PTR(-EBUSY); } /** @@ -322,9 +343,18 @@ static void amdgpu_dma_buf_unmap(struct dma_buf_attachment *attach, struct sg_table *sgt, enum dma_data_direction dir) { - dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir); - sg_free_table(sgt); - kfree(sgt); + struct dma_buf *dma_buf = attach->dmabuf; + struct drm_gem_object *obj = dma_buf->priv; + struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj); + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); + + if (sgt->sgl->page_link) { + dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir); + sg_free_table(sgt); + kfree(sgt); + } else { + amdgpu_vram_mgr_free_sgt(adev, attach->dev, dir, sgt); + } } /** diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index 7551f3729445..a99d813b23a5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -24,8 +24,9 @@ #ifndef __AMDGPU_TTM_H__ #define __AMDGPU_TTM_H__ -#include "amdgpu.h" +#include #include +#include "amdgpu.h" #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1) @@ -74,6 +75,15 @@ uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager *man); int amdgpu_gtt_mgr_recover(struct ttm_mem_type_manager *man); u64 amdgpu_vram_mgr_bo_visible_size(struct amdgpu_bo *bo); +int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev, + struct ttm_mem_reg *mem, + struct device *dev, + enum dma_data_direction dir, + struct sg_table **sgt); +void amdgpu_vram_mgr_free_sgt(struct amdgp
[PATCH 6/6] drm/amdgpu: improve amdgpu_gem_info debugfs file
Note if a buffer was imported using peer2peer. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 4277125a79ee..e42608115c99 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -854,7 +855,8 @@ static int amdgpu_debugfs_gem_bo_info(int id, void *ptr, void *data) attachment = READ_ONCE(bo->tbo.base.import_attach); if (attachment) - seq_printf(m, " imported from %p", dma_buf); + seq_printf(m, " imported from %p%s", dma_buf, + attachment->peer2peer ? " P2P" : ""); else if (dma_buf) seq_printf(m, " exported as %p", dma_buf); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v13 1/6] dt-bindings: media: add pclk-sample dual edge property
Hi, On 11/03/2020 08:18, Jitao Shi wrote: > Some chips's sample mode are rising, falling and dual edge (both > falling and rising edge). > Extern the pclk-sample property to support dual edge. > > Acked-by: Rob Herring > Reviewed-by: CK Hu > Signed-off-by: Jitao Shi > --- > Documentation/devicetree/bindings/media/video-interfaces.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > b/Documentation/devicetree/bindings/media/video-interfaces.txt > index f884ada0bffc..da9ad24935db 100644 > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > @@ -118,8 +118,8 @@ Optional endpoint properties > - data-enable-active: similar to HSYNC and VSYNC, specifies the data enable >signal polarity. > - field-even-active: field signal level during the even field data > transmission. > -- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel > clock > - signal. > +- pclk-sample: sample data on rising (1), falling (0) or both rising and > + falling (2) edge of the pixel clock signal. > - sync-on-green-active: active state of Sync-on-green (SoG) signal, 0/1 for >LOW/HIGH respectively. > - data-lanes: an array of physical data lane indexes. Position of an entry > This changes the bus format, but we recently introduced a bus format negociation between bridges to avoid adding such properties into DT, and make bus format setup dynamic between an encoder and a bridge. It would be great to use that instead. Neil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v13 5/6] drm/mediatek: dpi sample mode support
On 11/03/2020 08:18, Jitao Shi wrote: > DPI can sample on falling, rising or both edge. > When DPI sample the data both rising and falling edge. > It can reduce half data io pins. > > Reviewed-by: CK Hu > Signed-off-by: Jitao Shi > --- > drivers/gpu/drm/mediatek/mtk_dpi.c | 27 +-- > 1 file changed, 25 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > b/drivers/gpu/drm/mediatek/mtk_dpi.c > index 087f5ce732e1..2871e68e7767 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > @@ -75,6 +75,7 @@ struct mtk_dpi { > enum mtk_dpi_out_bit_num bit_num; > enum mtk_dpi_out_channel_swap channel_swap; > int refcount; > + u32 pclk_sample; > }; > > static inline struct mtk_dpi *mtk_dpi_from_encoder(struct drm_encoder *e) > @@ -348,6 +349,13 @@ static void mtk_dpi_config_disable_edge(struct mtk_dpi > *dpi) > mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN); > } > > +static void mtk_dpi_enable_pclk_sample_dual_edge(struct mtk_dpi *dpi) > +{ > + mtk_dpi_mask(dpi, DPI_DDR_SETTING, DDR_EN | DDR_4PHASE, > + DDR_EN | DDR_4PHASE); > + mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, EDGE_SEL, EDGE_SEL); > +} > + > static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, > enum mtk_dpi_out_color_format format) > { > @@ -439,7 +447,8 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, > pll_rate = clk_get_rate(dpi->tvd_clk); > > vm.pixelclock = pll_rate / factor; > - clk_set_rate(dpi->pixel_clk, vm.pixelclock); > + clk_set_rate(dpi->pixel_clk, > + vm.pixelclock * (dpi->pclk_sample > 1 ? 2 : 1)); > vm.pixelclock = clk_get_rate(dpi->pixel_clk); > > dev_dbg(dpi->dev, "Got PLL %lu Hz, pixel clock %lu Hz\n", > @@ -450,7 +459,8 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, > limit.y_bottom = 0x0010; > limit.y_top = 0x0FE0; > > - dpi_pol.ck_pol = MTK_DPI_POLARITY_FALLING; > + dpi_pol.ck_pol = dpi->pclk_sample == 1 ? > + MTK_DPI_POLARITY_RISING : MTK_DPI_POLARITY_FALLING; > dpi_pol.de_pol = MTK_DPI_POLARITY_RISING; > dpi_pol.hsync_pol = vm.flags & DISPLAY_FLAGS_HSYNC_HIGH ? > MTK_DPI_POLARITY_FALLING : MTK_DPI_POLARITY_RISING; > @@ -504,6 +514,8 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, > mtk_dpi_config_color_format(dpi, dpi->color_format); > mtk_dpi_config_2n_h_fre(dpi); > mtk_dpi_config_disable_edge(dpi); > + if (dpi->pclk_sample > 1) > + mtk_dpi_enable_pclk_sample_dual_edge(dpi); > mtk_dpi_sw_reset(dpi, false); > > return 0; > @@ -693,6 +705,7 @@ static const struct mtk_dpi_conf mt8183_conf = { > static int mtk_dpi_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > + struct device_node *ep; > struct mtk_dpi *dpi; > struct resource *mem; > int comp_id; > @@ -705,6 +718,16 @@ static int mtk_dpi_probe(struct platform_device *pdev) > dpi->dev = dev; > dpi->conf = (struct mtk_dpi_conf *)of_device_get_match_data(dev); > > + ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0); > + if (!ep) { > + dev_err(dev, "Failed get the endpoint port\n"); > + return -EINVAL; > + } > + > + /* Get the sampling edge from the endpoint. */ > + of_property_read_u32(ep, "pclk-sample", &dpi->pclk_sample); > + of_node_put(ep); Instead of having hard-coded value in DT, you should switch to bridge bus format negotiation instead. Boris pushed support a few weeks ago and the dw-hdmi bridge has been updated to work with it and negotiate the format between the encoder and the bridge. Phong is pushing the it66121 bridge driver which is used on a MT8183 board, and should make use of that to handle the dual edge support between the 8183 dpi and the it66121 bridge. Neil > + > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > dpi->regs = devm_ioremap_resource(dev, mem); > if (IS_ERR(dpi->regs)) { > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: sysfs: Use scnprintf() for avoiding potential buffer overflow
Am 11.03.20 um 09:24 schrieb Takashi Iwai: > On Wed, 11 Mar 2020 09:10:56 +0100, > Thomas Zimmermann wrote: >> >> Hi Takashi >> >> Am 11.03.20 um 08:35 schrieb Takashi Iwai: >>> Since snprintf() returns the would-be-output size instead of the >>> actual output size, the succeeding calls may go beyond the given >>> buffer limit. Fix it by replacing with scnprintf(). >>> >>> Signed-off-by: Takashi Iwai >>> --- >>> drivers/gpu/drm/drm_sysfs.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c >>> index dd2bc85f43cc..9b3180e8c12f 100644 >>> --- a/drivers/gpu/drm/drm_sysfs.c >>> +++ b/drivers/gpu/drm/drm_sysfs.c >>> @@ -230,7 +230,7 @@ static ssize_t modes_show(struct device *device, >>> >>> mutex_lock(&connector->dev->mode_config.mutex); >>> list_for_each_entry(mode, &connector->modes, head) { >>> - written += snprintf(buf + written, PAGE_SIZE - written, "%s\n", >>> + written += scnprintf(buf + written, PAGE_SIZE - written, "%s\n", >>> mode->name); >>> } >>> mutex_unlock(&connector->dev->mode_config.mutex); >>> >> >> In drm_sysfs.c, there are more _show functions with calls to snprintf() >> that could be replaced by scnprintf(). ATM they don't return the correct >> length for output that exceeds PAGE_SIZE. since you're at it, you may >> replace them as well. > > Well, the rest snprintf() calls are single calls and can't be over > PAGE_SIZE obviously. IOW, they could be rather replaced with > sprintf() instead, for a sake of simplicity. > >> But in any case >> >> Reviewed-by: Thomas Zimmermann >> >> for this patch. >> >> Do you want me to merge the patch into drm-misc-next? > > Yes, please. https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9b9f2219b2c4fa3d1a41245cdc263d09a4c9ad92 Best regards Thomas > > > thanks, > > Takashi > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/6] drm/amdgpu: add checks if DMA-buf P2P is supported
Am 11.03.20 um 15:04 schrieb Jason Gunthorpe: On Wed, Mar 11, 2020 at 02:51:56PM +0100, Christian König wrote: Check if we can do peer2peer on the PCIe bus. Signed-off-by: Christian König drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index aef12ee2f1e3..bbf67800c8a6 100644 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -38,6 +38,7 @@ #include #include #include +#include /** * amdgpu_gem_prime_vmap - &dma_buf_ops.vmap implementation @@ -179,6 +180,9 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf, struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); int r; + if (pci_p2pdma_distance_many(adev->pdev, &attach->dev, 1, true) < 0) + attach->peer2peer = false; + Are there other related patches than this series? p2p dma mapping needs to be done in common code, in p2pdma.c - ie this open coding is missing the bus_offset stuff, at least. Yeah, I'm aware of this. But I couldn't find a better way for now. I really do not want to see drivers open code this stuff. We already have a p2pdma API for handling the struct page case, so I suggest adding some new p2pdma API to handle this for non-struct page cases. ie some thing like: int 'p2pdma map bar'( struct pci_device *source, unsigned int source_bar_number, struct pci_device *dest, physaddr&len *array_of_offsets & length pairs into source bar, struct scatterlist *output_sgl) Well that's exactly what I have to avoid since I don't have the array of offsets around and want to avoid constructing it. Similar problem for dma_map_resource(). My example does this on demand, but essentially we also have use cases where this is done only once. Ideally we would have some function to create an sgl based on some arbitrary collection of offsets and length inside a BAR. Regards, Christian. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/21] drm/tegra: remove checks for debugfs functions return value
On Thu, Feb 27, 2020 at 03:02:21PM +0300, Wambui Karuga wrote: > Since 987d65d01356 (drm: debugfs: make > drm_debugfs_create_files() never fail) there is no need to check the > return value of drm_debugfs_create_files(). Therefore, remove the > return checks and error handling of the drm_debugfs_create_files() > function from various debugfs init functions in drm/tegra and have > them return 0 directly. > > This change also includes removing the use of drm_debugfs_create_files > as a return value in tegra_debugfs_init() and have the function declared > as void. > > Signed-off-by: Wambui Karuga > --- > drivers/gpu/drm/tegra/dc.c | 11 +-- > drivers/gpu/drm/tegra/drm.c | 8 > drivers/gpu/drm/tegra/dsi.c | 11 +-- > drivers/gpu/drm/tegra/hdmi.c | 11 +-- > drivers/gpu/drm/tegra/sor.c | 11 +-- > 5 files changed, 8 insertions(+), 44 deletions(-) Applied, thanks. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/6] drm/amdgpu: add checks if DMA-buf P2P is supported
Am 11.03.20 um 15:38 schrieb Jason Gunthorpe: On Wed, Mar 11, 2020 at 03:33:01PM +0100, Christian König wrote: Am 11.03.20 um 15:04 schrieb Jason Gunthorpe: On Wed, Mar 11, 2020 at 02:51:56PM +0100, Christian König wrote: Check if we can do peer2peer on the PCIe bus. Signed-off-by: Christian König drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index aef12ee2f1e3..bbf67800c8a6 100644 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -38,6 +38,7 @@ #include #include #include +#include /** * amdgpu_gem_prime_vmap - &dma_buf_ops.vmap implementation @@ -179,6 +180,9 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf, struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); int r; + if (pci_p2pdma_distance_many(adev->pdev, &attach->dev, 1, true) < 0) + attach->peer2peer = false; + Are there other related patches than this series? p2p dma mapping needs to be done in common code, in p2pdma.c - ie this open coding is missing the bus_offset stuff, at least. Yeah, I'm aware of this. But I couldn't find a better way for now. Well, it isn't optional :) I really do not want to see drivers open code this stuff. We already have a p2pdma API for handling the struct page case, so I suggest adding some new p2pdma API to handle this for non-struct page cases. ie some thing like: int 'p2pdma map bar'( struct pci_device *source, unsigned int source_bar_number, struct pci_device *dest, physaddr&len *array_of_offsets & length pairs into source bar, struct scatterlist *output_sgl) Well that's exactly what I have to avoid since I don't have the array of offsets around and want to avoid constructing it. Maybe it doesn't need an array of offsets - just a single offset and callers can iterate the API? Yes, that would of course work as well. But I was assuming that p2pdma_map_bar() needs some state between those calls. Similar problem for dma_map_resource(). My example does this on demand, but essentially we also have use cases where this is done only once. I'm not sure if this is portable. Does any IOMMU HW need to know P2P is happening to setup successfully? We currently support such a narrow scope of HW for P2P.. On the AMD hardware I'm testing this calling dma_map_resource() already seems to work with IOMMU enabled. (Well at least it seemed so 6month ago when I last tested this). Ideally we would have some function to create an sgl based on some arbitrary collection of offsets and length inside a BAR. Isn't that what I just proposed above ? Yes, just didn't thought that this would easily possible. I will double check the p2pdma code again. Thanks, Christian. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/33] drm/panel-sony-acx424akp: Fix dotclocks
On Sat, Mar 07, 2020 at 03:40:11PM +0100, Linus Walleij wrote: > On Mon, Mar 2, 2020 at 9:35 PM Ville Syrjala > wrote: > > > From: Ville Syrjälä > > > > The currently listed dotclocks disagree with the currently > > listed vrefresh rates. Change the dotclocks to match the vrefresh. > > > > Someone tell me which (if either) of the dotclock or vreresh is > > correct? > > > > Cc: Linus Walleij > > Cc: Sam Ravnborg > > Signed-off-by: Ville Syrjälä > > These are better than what is currently in the driver > at least, we don't know the real dotclocks. (No datasheet.) > Reviewed-by: Linus Walleij Thanks. Pushed to drm-misc-next. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/33] drm/panel-lg-lg4573: Fix dotclock
On Tue, Mar 03, 2020 at 06:24:25AM +0100, Heiko Schocher wrote: > Hello Ville Syrjala, > > Am 02.03.2020 um 21:34 schrieb Ville Syrjala: > > From: Ville Syrjälä > > > > The currently listed dotclock disagrees with the currently > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > Someone tell me which (if either) of the dotclock or vreresh is > > correct? > > Your clock fix is correct, thanks! > > > > > Cc: Heiko Schocher > > Cc: Thierry Reding > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/panel/panel-lg-lg4573.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Heiko Schocher Thanks. Pushed to drm-misc-next. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 01/33] drm/panel-novatek-nt35510: Fix dotclock
On Mon, Mar 09, 2020 at 04:33:19PM +0100, Linus Walleij wrote: > On Mon, Mar 9, 2020 at 2:36 PM Ville Syrjala > wrote: > > > From: Ville Syrjälä > > > > The dotclock is three orders of magnitude out. Fix it. > > > > v2: Just set it to 20MHz (Linus) > > > > Cc: Linus Walleij > > Cc: Sam Ravnborg > > Signed-off-by: Ville Syrjälä > > Reviewed-by: Linus Walleij Thanks. Pushed to drm-misc-next. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 21/33] drm/panel-simple: Fix dotclock for Logic PD Type 28
On Tue, Mar 03, 2020 at 07:00:12AM -0600, Adam Ford wrote: > On Mon, Mar 2, 2020 at 2:36 PM Ville Syrjala > wrote: > > > > From: Ville Syrjälä > > > > The currently listed dotclock disagrees with the currently > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > Someone tell me which (if either) of the dotclock or vreresh is > > correct? > > > > Cc: Adam Ford > > Cc: Sam Ravnborg > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > b/drivers/gpu/drm/panel/panel-simple.c > > index 225be4757a85..3a46b82722f5 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -2277,7 +2277,7 @@ static const struct drm_display_mode > > mitsubishi_aa070mc01_mode = { > > }; > > > > static const struct drm_display_mode logicpd_type_28_mode = { > > - .clock = 9000, > > + .clock = 9107, > > This should be OK. The max dotclk frequency of this panel is 12MHz, > so 9.107MHz should be just fine. > > Reviewed-by: Adam Ford Thanks. Pushed to drm-misc-next. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/33] drm/panel-ilitek-ili9322: Fix dotclocks
On Mon, Mar 09, 2020 at 04:33:56PM +0100, Linus Walleij wrote: > On Mon, Mar 9, 2020 at 2:38 PM Ville Syrjala > wrote: > > > From: Ville Syrjälä > > > > The listed dotclocks are two orders of mangnitude out. > > Fix them. > > > > v2: Just divide everything by 100 (Linus) > > > > Cc: Linus Walleij > > Cc: Thierry Reding > > Signed-off-by: Ville Syrjälä > > Reviewed-by: Linus Walleij Thanks. Pushed to drm-misc-next. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper
The new struct contains afbc-specific data. The new function can be used by drivers which support afbc to complete the preparation of struct drm_afbc_framebuffer. It must be called after allocating the said struct and calling drm_gem_fb_init_with_funcs(). Signed-off-by: Andrzej Pietrasiewicz --- Documentation/gpu/todo.rst | 15 +++ drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++ include/drm/drm_framebuffer.h| 45 include/drm/drm_gem_framebuffer_helper.h | 10 ++ 4 files changed, 178 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 439656f55c5d..37a3a023c114 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers Level: Intermediate +Encode cpp properly in malidp +- + +cpp (chars per pixel) is not encoded properly in malidp, zero is +used instead. afbc implementation needs bpp or cpp, but if it is +zero it needs to be provided elsewhere, and so the bpp field exists +in struct drm_afbc_framebuffer. + +Properly encode cpp in malidp and remove the bpp field in struct +drm_afbc_framebuffer. + +Contact: malidp maintainers + +Level: Intermediate + Core refactorings = diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 86c1907c579a..7e3982c36baa 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -21,6 +21,13 @@ #include #include +#define AFBC_HEADER_SIZE 16 +#define AFBC_TH_LAYOUT_ALIGNMENT 8 +#define AFBC_HDR_ALIGN 64 +#define AFBC_SUPERBLOCK_PIXELS 256 +#define AFBC_SUPERBLOCK_ALIGNMENT 128 +#define AFBC_TH_BODY_START_ALIGNMENT 4096 + /** * DOC: overview * @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file, } EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty); +static int drm_gem_afbc_min_size(struct drm_device *dev, +const struct drm_mode_fb_cmd2 *mode_cmd, +struct drm_afbc_framebuffer *afbc_fb) +{ + const struct drm_format_info *info; + __u32 n_blocks, w_alignment, h_alignment, hdr_alignment; + /* remove bpp when all users properly encode cpp in drm_format_info */ + __u32 bpp; + + switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) { + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16: + afbc_fb->block_width = 16; + afbc_fb->block_height = 16; + break; + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8: + afbc_fb->block_width = 32; + afbc_fb->block_height = 8; + break; + /* no user exists yet - fall through */ + case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4: + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4: + default: + DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n", + mode_cmd->modifier[0] + & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK); + return -EINVAL; + } + + /* tiled header afbc */ + w_alignment = afbc_fb->block_width; + h_alignment = afbc_fb->block_height; + hdr_alignment = AFBC_HDR_ALIGN; + if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) { + w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT; + h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT; + hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT; + } + + afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment); + afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment); + afbc_fb->offset = mode_cmd->offsets[0]; + + info = drm_get_format_info(dev, mode_cmd); + /* +* Change to always using info->cpp[0] +* when all users properly encode it +*/ + bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp; + + n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height) + / AFBC_SUPERBLOCK_PIXELS; + afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment); + afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8, + AFBC_SUPERBLOCK_ALIGNMENT); + + return 0; +} + +/** + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to + * fill and validate all the afbc-specific + * struct drm_afbc_framebuffer members + * + * @dev: DRM device + * @afbc_fb: afbc-specific framebuffer + * @mode_cmd: Metadata from the userspace framebuffer creation request + * @afbc_fb: afbc framebuffer + * + * This function can be used by drivers which support afbc to complete + * the preparation of struct drm_afbc_framebuffer. It mu
[PATCHv7 0/6] Add AFBC support for Rockchip
This series adds AFBC support for Rockchip. It is inspired by: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/factory-gru-9017.B-chromeos-4.4/drivers/gpu/drm/rockchip/rockchip_drm_vop.c This is the seventh iteration of the afbc series, which addresses comments received for v6. Compared to v5 it simplifies the way afbc-related helpers are exposed to their users. A new struct drm_afbc_framebuffer is added, which stores afbc-related driver-specific data. Interested drivers need to explicitly allocate an instance of struct drm_afbc_framebuffer, use drm_gem_fb_init_with_funcs() call drm_gem_fb_afbc_init() and do their specific afbc-related checks. There are 3 interested drivers: malidp, komeda and rockchip and I only have rockchip hardware. As Liviu reports, due to the coronavirus outbreak, it is difficult to test komeda now, so, according to his suggestion, I purposedly omit changes to komeda. Malidp is changed accordingly, though, which is a proof that it can be done. Then adding afbc support for rockchip follows the malidp example. I kindly ask for reviewing the series. I need to mention that my ultimate goal is merging afbc for rockchip and I don't have other hardware, so some help from malidp developers/maintainers would be appreciated. Rebased onto drm-misc-next. v6..v7: - used IS_ERR() instead of IS_ERR_OR_NULL() (Emil) - made drm_gem_fb_afbc_init() symmetric in terms of not putting the objects in case of error, it is now caller's responsibility (Emil) - added an entry in Documentation/gpu/todo.rst about drivers not encoding cpp in their format info (Emil) - sticked to the 80 columns per line rule wherever possible, excluding error messages so that they can be grepped (Emil) - removed redundant WARN_ON() in rockchip_mod_supported() (Emil) - made drm_gem_fb_init() and drm_gem_fb_init_with_funcs() return an int (James) - factored in drm_afbc_get_superblock_wh() (James) - eliminated unknown types error for u32/u64 (kbuild) v5..v6: - reworked the way afbc-specific helpers are exposed to drivers (Daniel) - not checking block size mask in drm_is_afbc (James) - fixed the test for afbc format (Boris) - documented unused afbc format modifier values in drm_afbc_get_superblock_wh() (Boris) - changed drm_is_afbc to a macro - renamed drm_gem_fb_lookup() to drm_gem_fb_objs_lookup() (James) - eliminated storing block/tile alignment constraint in struct drm_afbc_framebuffer (James) - eliminated storing afbc header alignment constraint in struct drm_afbc_framebuffer (James) - eliminated storing afbc payload's offset in struct drm_afbc_framebuffer (James) - moved to taking bpp value from drm_format_info except malidp which doesn't set it properly (James) - removed unrelated coding style fixes in rockchip (Boris) - consolidated 2-line error messages into one-liners in rockchip (Boris) - added checking that there is at most one AFBC plane in vop_crtc_atomic_check() (Boris) - added checking that AFBC format is indeed supported in rockchip_mod_supported() (Boris) - removed requirement of exactly one color plane in rockchip_mod_supported() - removed afbc_win hack in rockchip in favor of adding a field to crtc state and ensuring only one AFBC plane in crtc's atomic check (Boris) v4..v5: - used proper way of subclassing drm_framebuffer (Daniel Vetter) - added documentation to exported functions (Liviu Dudau) - reordered new functions in drm_gem_framebuffer_helper.c to make a saner diff (Liviu Dudau) - used "2" suffix instead of "_special" for the special version of size checks (Liviu Dudau) - dropped unnecessarily added condition in drm_get_format_info() (Liviu Dudau) - dropped "block_size = 0;" trick in framebuffer_check() (Daniel Vetter) - relaxed sticking to 80 characters per line rule in some cases v3..v4: - addressed (some) comments from Daniel Stone, Ezequiel Garcia, Daniel Vetter and James Qian Wang - thank you for input - refactored helpers to ease accommodating drivers with afbc needs - moved afbc checks to helpers - converted komeda, malidp and (the newly added) rockchip to use the afbc helpers - eliminated a separate, dedicated source code file v2..v3: - addressed (some) comments from Daniel Stone, Liviu Dudau, Daniel Vetter and Brian Starkey - thank you all In this iteration some rework has been done. The checking logic is now moved to framebuffer_check() so it is common to all drivers. But the common part is not good for komeda, so this series is not good for merging yet. I kindly ask for feedback whether the changes are in the right direction. I also kindly ask for input on how to accommodate komeda. The CONFIG_DRM_AFBC option has been eliminated in favour of adding drm_afbc.c to drm_kms_helper. v1..v2: - addressed comments from Daniel Stone, Ayan Halder, Mihail Atanassov - coding style fixes Andrzej Pietrasiewicz (6): drm/core: Allow drivers allocate a subclass of struct drm_framebuffer drm/core: Add drm_afbc_framebuffer and a corresponding helper drm/arm/ma
[PATCHv7 3/6] drm/arm/malidp: Factor-in framebuffer creation
Consolidating framebuffer creation into one function will make it easier to transition to generic afbc-aware helpers. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/arm/malidp_drv.c | 157 +-- 1 file changed, 66 insertions(+), 91 deletions(-) diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index 37d92a06318e..cafbd81e4c1e 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -269,112 +269,87 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = { .atomic_commit_tail = malidp_atomic_commit_tail, }; -static bool -malidp_verify_afbc_framebuffer_caps(struct drm_device *dev, - const struct drm_mode_fb_cmd2 *mode_cmd) +static struct drm_framebuffer * +malidp_fb_create(struct drm_device *dev, struct drm_file *file, +const struct drm_mode_fb_cmd2 *mode_cmd) { - if (malidp_format_mod_supported(dev, mode_cmd->pixel_format, - mode_cmd->modifier[0]) == false) - return false; + if (mode_cmd->modifier[0]) { + int n_superblocks = 0; + const struct drm_format_info *info; + struct drm_gem_object *objs = NULL; + u32 afbc_superblock_size = 0, afbc_superblock_height = 0; + u32 afbc_superblock_width = 0, afbc_size = 0; + int bpp = 0; + + if (malidp_format_mod_supported(dev, mode_cmd->pixel_format, + mode_cmd->modifier[0]) == false) + return ERR_PTR(-EINVAL); - if (mode_cmd->offsets[0] != 0) { - DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n"); - return false; - } + if (mode_cmd->offsets[0] != 0) { + DRM_DEBUG_KMS("AFBC buffer plane offset should be 0\n"); + return ERR_PTR(-EINVAL); + } - switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { - case AFBC_SIZE_16X16: - if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) { - DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 pixels\n"); - return false; + switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { + case AFBC_SIZE_16X16: + if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) { + DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 pixels\n"); + return ERR_PTR(-EINVAL); + } + break; + default: + DRM_DEBUG_KMS("Unsupported AFBC block size\n"); + return ERR_PTR(-EINVAL); } - break; - default: - DRM_DEBUG_KMS("Unsupported AFBC block size\n"); - return false; - } - return true; -} + switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { + case AFBC_SIZE_16X16: + afbc_superblock_height = 16; + afbc_superblock_width = 16; + break; + default: + DRM_DEBUG_KMS("AFBC superblock size not supported\n"); + return ERR_PTR(-EINVAL); + } -static bool -malidp_verify_afbc_framebuffer_size(struct drm_device *dev, - struct drm_file *file, - const struct drm_mode_fb_cmd2 *mode_cmd) -{ - int n_superblocks = 0; - const struct drm_format_info *info; - struct drm_gem_object *objs = NULL; - u32 afbc_superblock_size = 0, afbc_superblock_height = 0; - u32 afbc_superblock_width = 0, afbc_size = 0; - int bpp = 0; - - switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { - case AFBC_SIZE_16X16: - afbc_superblock_height = 16; - afbc_superblock_width = 16; - break; - default: - DRM_DEBUG_KMS("AFBC superblock size is not supported\n"); - return false; - } + info = drm_get_format_info(dev, mode_cmd); - info = drm_get_format_info(dev, mode_cmd); + n_superblocks = (mode_cmd->width / afbc_superblock_width) * + (mode_cmd->height / afbc_superblock_height); - n_superblocks = (mode_cmd->width / afbc_superblock_width) * - (mode_cmd->height / afbc_superblock_height); + bpp = malidp_format_get_bpp(info->format); - bpp = malidp_format_get_bpp(info->format); + afbc_superblock_size = + (bpp * afbc_superblock_width * afbc_superblock_height) + / BITS_PER_BYTE; - afbc_superblock_size = (bpp * afbc_superblock_width *
[PATCHv7 1/6] drm/core: Allow drivers allocate a subclass of struct drm_framebuffer
Allow allocating a specialized version of struct drm_framebuffer by moving the actual fb allocation out of drm_gem_fb_create_with_funcs(); the respective functions names are adjusted to reflect that fact. Please note, though, that standard size checks are performed on buffers, so the drm_gem_fb_init_with_funcs() is useful for cases where those standard size checks are appropriate or at least don't conflict the checks to be performed in the specialized case. Thanks to this change the drivers can call drm_gem_fb_init_with_funcs() having allocated their special version of struct drm_framebuffer, exactly the way the new version of drm_gem_fb_create_with_funcs() does. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 87 ++-- include/drm/drm_gem_framebuffer_helper.h | 5 ++ 2 files changed, 67 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 3a7ace19a902..86c1907c579a 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -54,19 +54,15 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb, } EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj); -static struct drm_framebuffer * -drm_gem_fb_alloc(struct drm_device *dev, +static int +drm_gem_fb_init(struct drm_device *dev, +struct drm_framebuffer *fb, const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **obj, unsigned int num_planes, const struct drm_framebuffer_funcs *funcs) { - struct drm_framebuffer *fb; int ret, i; - fb = kzalloc(sizeof(*fb), GFP_KERNEL); - if (!fb) - return ERR_PTR(-ENOMEM); - drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd); for (i = 0; i < num_planes; i++) @@ -76,10 +72,9 @@ drm_gem_fb_alloc(struct drm_device *dev, if (ret) { drm_err(dev, "Failed to init framebuffer: %d\n", ret); kfree(fb); - return ERR_PTR(ret); } - return fb; + return ret; } /** @@ -123,10 +118,13 @@ int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file, EXPORT_SYMBOL(drm_gem_fb_create_handle); /** - * drm_gem_fb_create_with_funcs() - Helper function for the - * &drm_mode_config_funcs.fb_create - * callback + * drm_gem_fb_init_with_funcs() - Helper function for implementing + * &drm_mode_config_funcs.fb_create + * callback in cases when the driver + * allocates a subclass of + * struct drm_framebuffer * @dev: DRM device + * @fb: framebuffer object * @file: DRM file that holds the GEM handle(s) backing the framebuffer * @mode_cmd: Metadata from the userspace framebuffer creation request * @funcs: vtable to be used for the new framebuffer object @@ -134,23 +132,26 @@ EXPORT_SYMBOL(drm_gem_fb_create_handle); * This function can be used to set &drm_framebuffer_funcs for drivers that need * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to * change &drm_framebuffer_funcs. The function does buffer size validation. + * The buffer size validation is for a general case, though, so users should + * pay attention to the checks being appropriate for them or, at least, + * non-conflicting. * * Returns: - * Pointer to a &drm_framebuffer on success or an error pointer on failure. + * Zero or a negative error code. */ -struct drm_framebuffer * -drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, -const struct drm_mode_fb_cmd2 *mode_cmd, -const struct drm_framebuffer_funcs *funcs) +int drm_gem_fb_init_with_funcs(struct drm_device *dev, + struct drm_framebuffer *fb, + struct drm_file *file, + const struct drm_mode_fb_cmd2 *mode_cmd, + const struct drm_framebuffer_funcs *funcs) { const struct drm_format_info *info; struct drm_gem_object *objs[4]; - struct drm_framebuffer *fb; int ret, i; info = drm_get_format_info(dev, mode_cmd); if (!info) - return ERR_PTR(-EINVAL); + return -EINVAL; for (i = 0; i < info->num_planes; i++) { unsigned int width = mode_cmd->width / (i ? info->hsub : 1); @@ -175,19 +176,55 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, } } - fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs); - if (IS_ERR(fb)) { - ret = PTR_ERR(fb); + ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); + i
[PATCHv7 5/6] drm/arm/malidp: Switch to afbc helpers
Use available afbc helpers. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/arm/malidp_drv.c | 44 +++- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index b9715b19af94..bae532290c89 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -297,11 +297,7 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file, return ERR_PTR(-ENOMEM); } - if (mode_cmd->modifier[0]) { - int n_superblocks = 0; - struct drm_gem_object *objs = NULL; - u32 afbc_superblock_size = 0, afbc_superblock_height = 0; - u32 afbc_superblock_width = 0, afbc_size = 0; + if (drm_is_afbc(mode_cmd->modifier[0])) { int bpp = 0; if (malidp_format_mod_supported(dev, mode_cmd->pixel_format, @@ -325,31 +321,8 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file, goto free_fb; } - switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { - case AFBC_SIZE_16X16: - afbc_superblock_height = 16; - afbc_superblock_width = 16; - break; - default: - DRM_DEBUG_KMS("AFBC superblock size not supported\n"); - goto free_fb; - } - - - n_superblocks = (mode_cmd->width / afbc_superblock_width) * - (mode_cmd->height / afbc_superblock_height); - bpp = malidp_format_get_bpp(info->format); - afbc_superblock_size = - (bpp * afbc_superblock_width * afbc_superblock_height) - / BITS_PER_BYTE; - - afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE, - AFBC_SUPERBLK_ALIGNMENT); - afbc_size += n_superblocks - * ALIGN(afbc_superblock_size, AFBC_SUPERBLK_ALIGNMENT); - if ((mode_cmd->width * bpp) != (mode_cmd->pitches[0] * BITS_PER_BYTE)) { DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) should be same as width (=%u) * bpp (=%u)\n", @@ -358,20 +331,11 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file, goto free_fb; } - objs = drm_gem_object_lookup(file, mode_cmd->handles[0]); - if (!objs) { - DRM_DEBUG_KMS("Failed to lookup GEM object\n"); - goto free_fb; - } + /* eliminate when cpp is properly encoded in drm_format_info */ + afbc_fb->bpp = bpp; - if (objs->size < afbc_size) { - DRM_DEBUG_KMS("buffer size %zu/%u too small for AFBC\n", - objs->size, afbc_size); - drm_gem_object_put_unlocked(objs); + if (drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb)) goto free_fb; - } - - drm_gem_object_put_unlocked(objs); } return &afbc_fb->base; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv7 4/6] drm/arm/malidp: Allocate an afbc-specific drm_framebuffer
Prepare for using generic afbc helpers. Use an existing helper which allows allocating a struct drm_framebuffer in the driver. afbc-specific checks should go after drm_gem_fb_init_with_funcs(). Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/arm/malidp_drv.c | 50 +--- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index cafbd81e4c1e..b9715b19af94 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -269,13 +269,36 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = { .atomic_commit_tail = malidp_atomic_commit_tail, }; +static const struct drm_framebuffer_funcs malidp_fb_funcs = { + .destroy= drm_gem_fb_destroy, + .create_handle = drm_gem_fb_create_handle, +}; + static struct drm_framebuffer * malidp_fb_create(struct drm_device *dev, struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd) { + struct drm_afbc_framebuffer *afbc_fb; + const struct drm_format_info *info; + int ret, i; + + info = drm_get_format_info(dev, mode_cmd); + if (!info) + return ERR_PTR(-ENOMEM); + + afbc_fb = kzalloc(sizeof(*afbc_fb), GFP_KERNEL); + if (!afbc_fb) + return ERR_PTR(-ENOMEM); + + ret = drm_gem_fb_init_with_funcs(dev, &afbc_fb->base, file, mode_cmd, +&malidp_fb_funcs); + if (ret) { + kfree(afbc_fb); + return ERR_PTR(-ENOMEM); + } + if (mode_cmd->modifier[0]) { int n_superblocks = 0; - const struct drm_format_info *info; struct drm_gem_object *objs = NULL; u32 afbc_superblock_size = 0, afbc_superblock_height = 0; u32 afbc_superblock_width = 0, afbc_size = 0; @@ -283,23 +306,23 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file, if (malidp_format_mod_supported(dev, mode_cmd->pixel_format, mode_cmd->modifier[0]) == false) - return ERR_PTR(-EINVAL); + goto free_fb; if (mode_cmd->offsets[0] != 0) { DRM_DEBUG_KMS("AFBC buffer plane offset should be 0\n"); - return ERR_PTR(-EINVAL); + goto free_fb; } switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { case AFBC_SIZE_16X16: if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) { DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 pixels\n"); - return ERR_PTR(-EINVAL); + goto free_fb; } break; default: DRM_DEBUG_KMS("Unsupported AFBC block size\n"); - return ERR_PTR(-EINVAL); + goto free_fb; } switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { @@ -309,10 +332,9 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file, break; default: DRM_DEBUG_KMS("AFBC superblock size not supported\n"); - return ERR_PTR(-EINVAL); + goto free_fb; } - info = drm_get_format_info(dev, mode_cmd); n_superblocks = (mode_cmd->width / afbc_superblock_width) * (mode_cmd->height / afbc_superblock_height); @@ -333,26 +355,32 @@ malidp_fb_create(struct drm_device *dev, struct drm_file *file, DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) should be same as width (=%u) * bpp (=%u)\n", (mode_cmd->pitches[0] * BITS_PER_BYTE), mode_cmd->width, bpp); - return ERR_PTR(-EINVAL); + goto free_fb; } objs = drm_gem_object_lookup(file, mode_cmd->handles[0]); if (!objs) { DRM_DEBUG_KMS("Failed to lookup GEM object\n"); - return ERR_PTR(-EINVAL); + goto free_fb; } if (objs->size < afbc_size) { DRM_DEBUG_KMS("buffer size %zu/%u too small for AFBC\n", objs->size, afbc_size); drm_gem_object_put_unlocked(objs); - return ERR_PTR(-EINVAL); + goto free_fb; } drm_gem_object_put_unlocked(objs); } - return drm_gem_fb_create(dev, file, mode_cmd); +
[PATCHv7 6/6] drm/rockchip: Add support for afbc
This patch adds support for afbc handling. afbc is a compressed format which reduces the necessary memory bandwidth. Co-developed-by: Mark Yao Signed-off-by: Mark Yao Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 43 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 137 +++- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 17 +++ drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 83 +++- 5 files changed, 276 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index c5b06048124e..e33c2dcd0d4b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -30,6 +30,7 @@ struct rockchip_crtc_state { int output_mode; int output_bpc; int output_flags; + bool enable_afbc; }; #define to_rockchip_crtc_state(s) \ container_of(s, struct rockchip_crtc_state, base) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index 221e72e71432..9b13c784b347 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -57,8 +57,49 @@ static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, }; +static struct drm_framebuffer * +rockchip_fb_create(struct drm_device *dev, struct drm_file *file, + const struct drm_mode_fb_cmd2 *mode_cmd) +{ + struct drm_afbc_framebuffer *afbc_fb; + const struct drm_format_info *info; + int ret; + + info = drm_get_format_info(dev, mode_cmd); + if (!info) + return ERR_PTR(-ENOMEM); + + afbc_fb = kzalloc(sizeof(*afbc_fb), GFP_KERNEL); + if (!afbc_fb) + return ERR_PTR(-ENOMEM); + + ret = drm_gem_fb_init_with_funcs(dev, &afbc_fb->base, file, mode_cmd, +&rockchip_drm_fb_funcs); + if (ret) { + kfree(afbc_fb); + return ERR_PTR(ret); + } + + if (drm_is_afbc(mode_cmd->modifier[0])) { + int ret, i; + + ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb); + if (ret) { + struct drm_gem_object **obj = afbc_fb->base.obj; + + for (i = 0; i < info->num_planes; ++i) + drm_gem_object_put_unlocked(obj[i]); + + kfree(afbc_fb); + return ERR_PTR(ret); + } + } + + return &afbc_fb->base; +} + static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = { - .fb_create = drm_gem_fb_create_with_dirty, + .fb_create = rockchip_fb_create, .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index cecb2cc781f5..b87d22eb6ae1 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -91,9 +91,22 @@ #define VOP_WIN_TO_INDEX(vop_win) \ ((vop_win) - (vop_win)->vop->win) +#define VOP_AFBC_SET(vop, name, v) \ + do { \ + if ((vop)->data->afbc) \ + vop_reg_set((vop), &(vop)->data->afbc->name, \ + 0, ~0, v, #name); \ + } while (0) + #define to_vop(x) container_of(x, struct vop, crtc) #define to_vop_win(x) container_of(x, struct vop_win, base) +#define AFBC_FMT_RGB5650x0 +#define AFBC_FMT_U8U8U8U8 0x5 +#define AFBC_FMT_U8U8U80x4 + +#define AFBC_TILE_16x16BIT(4) + /* * The coefficients of the following matrix are all fixed points. * The format is S2.10 for the 3x3 part of the matrix, and S9.12 for the offsets. @@ -274,6 +287,29 @@ static enum vop_data_format vop_convert_format(uint32_t format) } } +static int vop_convert_afbc_format(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_XRGB: + case DRM_FORMAT_ARGB: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_ABGR: + return AFBC_FMT_U8U8U8U8; + case DRM_FORMAT_RGB888: + case DRM_FORMAT_BGR888: + return AFBC_FMT_U8U8U8; + case DRM_FORMAT_RGB565: + case DRM_FORMAT_BGR565: + return AFBC_FMT_RGB565; + /* either of the below should not be reachable */ + default: + DRM_WARN_ONCE("unsupported AFBC format[%08x]\n", format); + return -EINVAL; + } + + return -EINVAL; +} + static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
Re: [PATCH 31/33] drm/panel-simple: Fix dotclock for XT VL050-8048NT-C01
Hi Ville, On Mon, Mar 2, 2020 at 5:36 PM Ville Syrjala wrote: > > From: Ville Syrjälä > > The currently listed dotclock disagrees with the currently > listed vrefresh rate. Change the dotclock to match the vrefresh. > > Someone tell me which (if either) of the dotclock or vreresh is > correct? > > Cc: Fabio Estevam > Cc: Sam Ravnborg > Cc: Thierry Reding > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/panel/panel-simple.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > b/drivers/gpu/drm/panel/panel-simple.c > index 5ce1328fd7dc..6b48c02af112 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -3368,7 +3368,7 @@ static const struct panel_desc urt_umsh_8596md_parallel > = { > }; > > static const struct drm_display_mode vl050_8048nt_c01_mode = { > - .clock = 3, > + .clock = 34540, I don't have access to hardware to test this change at the moment, but looking at the panel datasheet I see that 34.54MHz is still inside the valid range: Reviewed-by: Fabio Estevam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/6] drm/amdgpu: add support for exporting VRAM using DMA-buf v2
On Wed, Mar 11, 2020 at 9:52 AM Christian König wrote: > > We should be able to do this now after checking all the prerequisites. > > v2: fix entrie count in the sgt > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 56 --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 12 ++- > drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 97 > 3 files changed, 151 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > index bbf67800c8a6..43d8ed7dbd00 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > @@ -276,14 +276,21 @@ static struct sg_table *amdgpu_dma_buf_map(struct > dma_buf_attachment *attach, > struct dma_buf *dma_buf = attach->dmabuf; > struct drm_gem_object *obj = dma_buf->priv; > struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj); > + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); > struct sg_table *sgt; > long r; > > if (!bo->pin_count) { > - /* move buffer into GTT */ > + /* move buffer into GTT or VRAM */ > struct ttm_operation_ctx ctx = { false, false }; > + unsigned domains = AMDGPU_GEM_DOMAIN_GTT; > > - amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT); > + if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM && > + attach->peer2peer) { > + bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; > + domains |= AMDGPU_GEM_DOMAIN_VRAM; > + } > + amdgpu_bo_placement_from_domain(bo, domains); > r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx); > if (r) > return ERR_PTR(r); > @@ -293,20 +300,34 @@ static struct sg_table *amdgpu_dma_buf_map(struct > dma_buf_attachment *attach, > return ERR_PTR(-EBUSY); > } > > - sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, bo->tbo.num_pages); > - if (IS_ERR(sgt)) > - return sgt; > - > - if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, > - DMA_ATTR_SKIP_CPU_SYNC)) > - goto error_free; > + switch (bo->tbo.mem.mem_type) { > + case TTM_PL_TT: > + sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, > + bo->tbo.num_pages); > + if (IS_ERR(sgt)) > + return sgt; > + > + if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, > + DMA_ATTR_SKIP_CPU_SYNC)) > + goto error_free; > + break; > + > + case TTM_PL_VRAM: > + r = amdgpu_vram_mgr_alloc_sgt(adev, &bo->tbo.mem, attach->dev, > + dir, &sgt); > + if (r) > + return ERR_PTR(r); > + break; > + default: > + return ERR_PTR(-EINVAL); > + } > > return sgt; > > error_free: > sg_free_table(sgt); > kfree(sgt); > - return ERR_PTR(-ENOMEM); > + return ERR_PTR(-EBUSY); > } > > /** > @@ -322,9 +343,18 @@ static void amdgpu_dma_buf_unmap(struct > dma_buf_attachment *attach, > struct sg_table *sgt, > enum dma_data_direction dir) > { > - dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir); > - sg_free_table(sgt); > - kfree(sgt); > + struct dma_buf *dma_buf = attach->dmabuf; > + struct drm_gem_object *obj = dma_buf->priv; > + struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj); > + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); > + > + if (sgt->sgl->page_link) { > + dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir); > + sg_free_table(sgt); > + kfree(sgt); > + } else { > + amdgpu_vram_mgr_free_sgt(adev, attach->dev, dir, sgt); > + } > } > > /** > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > index 7551f3729445..a99d813b23a5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > @@ -24,8 +24,9 @@ > #ifndef __AMDGPU_TTM_H__ > #define __AMDGPU_TTM_H__ > > -#include "amdgpu.h" > +#include > #include > +#include "amdgpu.h" > > #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) > #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1) > @@ -74,6 +75,15 @@ uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager > *man); > int amdgpu_gtt_mgr_recover(struct ttm_mem_type_manager *man); > > u64 amdgpu_vram_mgr_bo_visible_size(struct amdgpu_bo *bo); > +int amdgpu_vram_mgr_allo
Re: [PATCH v2 07/17] drm/etnaviv: remove check for return value of drm_debugfs_create_files()
On Di, 2020-03-10 at 16:31 +0300, Wambui Karuga wrote: > Since commit 987d65d01356 (drm: debugfs: make > drm_debugfs_create_files() never fail), drm_debugfs_create_files() never > fails and only returns 0. Therefore, remove the unnecessary check of its > return value and error handling in etnaviv_debugfs_init() and have the > function return 0 directly. > > v2: have etnaviv_debugfs_init() return 0 instead of void to ensure > individual compilation and avoid build breakage. > > References: > https://lists.freedesktop.org/archives/dri-devel/2020-February/257183.html > Signed-off-by: Wambui Karuga Acked-by: Lucas Stach > --- > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 16 > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index 6b43c1c94e8f..a65d30a48a9d 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -233,19 +233,11 @@ static struct drm_info_list etnaviv_debugfs_list[] = { > > static int etnaviv_debugfs_init(struct drm_minor *minor) > { > - struct drm_device *dev = minor->dev; > - int ret; > - > - ret = drm_debugfs_create_files(etnaviv_debugfs_list, > - ARRAY_SIZE(etnaviv_debugfs_list), > - minor->debugfs_root, minor); > + drm_debugfs_create_files(etnaviv_debugfs_list, > + ARRAY_SIZE(etnaviv_debugfs_list), > + minor->debugfs_root, minor); > > - if (ret) { > - dev_err(dev->dev, "could not install etnaviv_debugfs_list\n"); > - return ret; > - } > - > - return ret; > + return 0; > } > #endif > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[tegra-drm:drm/tegra/for-next 1/1] drivers/gpu/drm/tegra/drm.c:858:18: error: initialization of 'int (*)(struct drm_minor *)' from incompatible pointer type 'void (*)(struct drm_minor *)'
tree: git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next head: 4e1b4dc4172af081c1feb211ed48b77a008aa054 commit: 4e1b4dc4172af081c1feb211ed48b77a008aa054 [1/1] drm/tegra: Remove checks for debugfs functions return value config: arm64-allmodconfig (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 4e1b4dc4172af081c1feb211ed48b77a008aa054 # save the attached .config to linux build tree GCC_VERSION=9.2.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): >> drivers/gpu/drm/tegra/drm.c:858:18: error: initialization of 'int (*)(struct >> drm_minor *)' from incompatible pointer type 'void (*)(struct drm_minor *)' >> [-Werror=incompatible-pointer-types] 858 | .debugfs_init = tegra_debugfs_init, | ^~ drivers/gpu/drm/tegra/drm.c:858:18: note: (near initialization for 'tegra_drm_driver.debugfs_init') cc1: some warnings being treated as errors vim +858 drivers/gpu/drm/tegra/drm.c e450fcc6669705 drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-02-13 849 9b57f5f2c53e1f drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-11-08 850 static struct drm_driver tegra_drm_driver = { 0424fdaf883a68 drivers/gpu/drm/tegra/drm.c Daniel Vetter 2019-06-17 851 .driver_features = DRIVER_MODESET | DRIVER_GEM | 6c68b71776e760 drivers/gpu/drm/tegra/drm.c Thierry Reding 2017-08-15 852 DRIVER_ATOMIC | DRIVER_RENDER, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 853 .open = tegra_drm_open, bda0ecc45fe20b drivers/gpu/drm/tegra/drm.c Daniel Vetter 2017-05-08 854 .postclose = tegra_drm_postclose, c94bedabb3dd72 drivers/gpu/drm/tegra/drm.c Noralf Trønnes 2017-12-05 855 .lastclose = drm_fb_helper_lastclose, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 856 e450fcc6669705 drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-02-13 857 #if defined(CONFIG_DEBUG_FS) e450fcc6669705 drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-02-13 @858 .debugfs_init = tegra_debugfs_init, e450fcc6669705 drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-02-13 859 #endif e450fcc6669705 drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-02-13 860 1ddbdbd6e996c7 drivers/gpu/drm/tegra/drm.c Daniel Vetter 2016-04-26 861 .gem_free_object_unlocked = tegra_bo_free_object, de2ba664c30fcd drivers/gpu/host1x/drm/drm.c Arto Merilainen 2013-03-22 862 .gem_vm_ops = &tegra_bo_vm_ops, 3800391db1b22a drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-12-12 863 3800391db1b22a drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-12-12 864 .prime_handle_to_fd = drm_gem_prime_handle_to_fd, 3800391db1b22a drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-12-12 865 .prime_fd_to_handle = drm_gem_prime_fd_to_handle, 3800391db1b22a drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-12-12 866 .gem_prime_export = tegra_gem_prime_export, 3800391db1b22a drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-12-12 867 .gem_prime_import = tegra_gem_prime_import, 3800391db1b22a drivers/gpu/drm/tegra/drm.c Thierry Reding 2013-12-12 868 de2ba664c30fcd drivers/gpu/host1x/drm/drm.c Arto Merilainen 2013-03-22 869 .dumb_create = tegra_bo_dumb_create, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 870 d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 871 .ioctls = tegra_drm_ioctls, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 872 .num_ioctls = ARRAY_SIZE(tegra_drm_ioctls), d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 873 .fops = &tegra_drm_fops, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 874 d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 875 .name = DRIVER_NAME, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 876 .desc = DRIVER_DESC, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 877 .date = DRIVER_DATE, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 878 .major = DRIVER_MAJOR, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 879 .minor = DRIVER_MINOR, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 880 .patchlevel = DRIVER_PATCHLEVEL, d8f4a9eda00678 drivers/gpu/drm/tegra/drm.c Thierry Reding 2012-11-15 881 }; 776dc38403676f drivers/gpu/host1x/drm/drm.c Thierry Reding 2013-10-14 882 :: The code at line 858 was first introduced by commit :: e450fcc6669705ef49784080ac6dd8513df37763 drm/tegra: Add list of framebuffers to debugfs :: TO:
[PATCH 1/2] drm/vmwgfx: Fix the refuse_dma mode when using guest-backed objects
From: Thomas Hellstrom When we refuse DMA from system pages for whatever reason, we don't handle that correctly when guest-backed objects was enabled. Since guest-backed objects by definition require DMA to and from system pages, disable all functionality that relies on them. That basically amounts to 3D acceleration and screen targets. Signed-off-by: Thomas Hellstrom Reviewed-by: Brian Paul --- drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 3 ++- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 6 -- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c index 065015d2a8f6..3b41cf63110a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c @@ -1241,7 +1241,8 @@ int vmw_cmdbuf_set_pool_size(struct vmw_cmdbuf_man *man, * actually call into the already enabled manager, when * binding the MOB. */ - if (!(dev_priv->capabilities & SVGA_CAP_DX)) + if (!(dev_priv->capabilities & SVGA_CAP_DX) || + !dev_priv->has_mob) return -ENOMEM; ret = ttm_bo_create(&dev_priv->bdev, size, ttm_bo_type_device, diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index cf3dc56d7cf4..f2ec3155468d 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -682,8 +682,10 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) ret = vmw_dma_select_mode(dev_priv); if (unlikely(ret != 0)) { - DRM_INFO("Restricting capabilities due to IOMMU setup.\n"); + DRM_INFO("Restricting capabilities since DMA not available.\n"); refuse_dma = true; + if (dev_priv->capabilities & SVGA_CAP_GBOBJECTS) + DRM_INFO("Disabling 3D acceleration.\n"); } dev_priv->vram_size = vmw_read(dev_priv, SVGA_REG_VRAM_SIZE); @@ -866,7 +868,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) dev_priv->has_gmr = false; } - if (dev_priv->capabilities & SVGA_CAP_GBOBJECTS) { + if (dev_priv->capabilities & SVGA_CAP_GBOBJECTS && !refuse_dma) { dev_priv->has_mob = true; if (ttm_bo_init_mm(&dev_priv->bdev, VMW_PL_MOB, VMW_PL_MOB) != 0) { diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index 570687a1a327..68aecb6d9f87 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -1888,7 +1888,7 @@ int vmw_kms_stdu_init_display(struct vmw_private *dev_priv) /* Do nothing if Screen Target support is turned off */ - if (!VMWGFX_ENABLE_SCREEN_TARGET_OTABLE) + if (!VMWGFX_ENABLE_SCREEN_TARGET_OTABLE || !dev_priv->has_mob) return -ENOSYS; if (!(dev_priv->capabilities & SVGA_CAP_GBOBJECTS)) -- 2.21.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/vmwgfx: Refuse DMA operation when SEV encryption is active
From: Thomas Hellstrom TTM doesn't yet fully support mapping of DMA memory when SEV is active, so in that case, refuse DMA operation. For guest-backed object operation this means 3D acceleration will be disabled. For host-backed, VRAM will be used for data transfer between the guest and the device. Signed-off-by: Thomas Hellstrom Reviewed-by: Brian Paul --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index f2ec3155468d..4f58364421ce 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -575,6 +576,10 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv) [vmw_dma_map_populate] = "Caching DMA mappings.", [vmw_dma_map_bind] = "Giving up DMA mappings early."}; + /* TTM currently doesn't fully support SEV encryption. */ + if (mem_encrypt_active()) + return -EINVAL; + if (vmw_force_coherent) dev_priv->map_mode = vmw_dma_alloc_coherent; else if (vmw_restrict_iommu) -- 2.21.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Plumbing explicit synchronization through the Linux ecosystem
All, Sorry for casting such a broad net with this one. I'm sure most people who reply will get at least one mailing list rejection. However, this is an issue that affects a LOT of components and that's why it's thorny to begin with. Please pardon the length of this e-mail as well; I promise there's a concrete point/proposal at the end. Explicit synchronization is the future of graphics and media. At least, that seems to be the consensus among all the graphics people I've talked to. I had a chat with one of the lead Android graphics engineers recently who told me that doing explicit sync from the start was one of the best engineering decisions Android ever made. It's also the direction being taken by more modern APIs such as Vulkan. ## What are implicit and explicit synchronization? For those that aren't familiar with this space, GPUs, media encoders, etc. are massively parallel and synchronization of some form is required to ensure that everything happens in the right order and avoid data races. Implicit synchronization is when bits of work (3D, compute, video encode, etc.) are implicitly based on the absolute CPU-time order in which API calls occur. Explicit synchronization is when the client (whatever that means in any given context) provides the dependency graph explicitly via some sort of synchronization primitives. If you're still confused, consider the following examples: With OpenGL and EGL, almost everything is implicit sync. Say you have two OpenGL contexts sharing an image where one writes to it and the other textures from it. The way the OpenGL spec works, the client has to make the API calls to render to the image before (in CPU time) it makes the API calls which texture from the image. As long as it does this (and maybe inserts a glFlush?), the driver will ensure that the rendering completes before the texturing happens and you get correct contents. Implicit synchronization can also happen across processes. Wayland, for instance, is currently built on implicit sync where the client does their rendering and then does a hand-off (via wl_surface::commit) to tell the compositor it's done at which point the compositor can now texture from the surface. The hand-off ensures that the client's OpenGL API calls happen before the server's OpenGL API calls. A good example of explicit synchronization is the Vulkan API. There, a client (or multiple clients) can simultaneously build command buffers in different threads where one of those command buffers renders to an image and the other textures from it and then submit both of them at the same time with instructions to the driver for which order to execute them in. The execution order is described via the VkSemaphore primitive. With the new VK_KHR_timeline_semaphore extension, you can even submit the work which does the texturing BEFORE the work which does the rendering and the driver will sort it out. The #1 problem with implicit synchronization (which explicit solves) is that it leads to a lot of over-synchronization both in client space and in driver/device space. The client has to synchronize a lot more because it has to ensure that the API calls happen in a particular order. The driver/device have to synchronize a lot more because they never know what is going to end up being a synchronization point as an API call on another thread/process may occur at any time. As we move to more and more multi-threaded programming this synchronization (on the client-side especially) becomes more and more painful. ## Current status in Linux Implicit synchronization in Linux works via a the kernel's internal dma_buf and dma_fence data structures. A dma_fence is a tiny object which represents the "done" status for some bit of work. Typically, dma_fences are created as a by-product of someone submitting some bit of work (say, 3D rendering) to the kernel. The dma_buf object has a set of dma_fences on it representing shared (read) and exclusive (write) access to the object. When work is submitted which, for instance renders to the dma_buf, it's queued waiting on all the fences on the dma_buf and and a dma_fence is created representing the end of said rendering work and it's installed as the dma_buf's exclusive fence. This way, the kernel can manage all its internal queues (3D rendering, display, video encode, etc.) and know which things to submit in what order. For the last few years, we've had sync_file in the kernel and it's plumbed into some drivers. A sync_file is just a wrapper around a single dma_fence. A sync_file is typically created as a by-product of submitting work (3D, compute, etc.) to the kernel and is signaled when that work completes. When a sync_file is created, it is guaranteed by the kernel that it will become signaled in finite time and, once it's signaled, it remains signaled for the rest of time. A sync_file is represented in UAPIs as a file descriptor and can be used with normal file APIs such as dup(). It ca
Re: [RFC PATCH 0/8] *** Per context fencing ***
On Wed, Mar 11, 2020 at 3:36 AM Gerd Hoffmann wrote: > > Hi, > > > I should've been more clear -- this is an internal cleanup/preparation and > > the per-context changes are invisible to host userspace. > > Ok, it wasn't clear that you don't flip the switch yet. In general the > commit messages could be a bit more verbose ... > > I'm wondering though why we need the new fence_id in the first place. > Isn't it enough to have per-context (instead of global) last_seq? > > > Multi-queue sounds very interesting indeed, especially with VK > > multi-threaded command submission. That to me is V3 rather than V2.. let's > > start easy! > > Having v2 if we plan to obsolete it with v3 soon doesn't look like a > good plan to me. It'll make backward compatibility more complex for > no good reason ... I agree we want to study multi-queue a little bit before doing v2. If we do decide that multi-queue will be v3, we should at least design v2 in a forward-compatible way. Every VK context (or GL context if we go multi-process GL) is isolated. I think there will need to be at least one virtqueue for each VK context. Can virtqueues be added dynamically? Or can we have fixed but enough (e.g., 64) virtqueues? Multi-threaded command submission is not helped by multi-queue unless we go with one virtqueue for each VKQueue in a VK context. Otherwise, multi-queue only makes context scheduling easier, which is not a priority yet IMO. > > Also: Does virglrenderer render different contexts in parallel today? > Only in case it does we'll actually get benefits from per-context > fences. But I think it doesn't, so there is no need to rush. > > I think we should better have a rough plan for parallel rendering first, > then go start implementing the pieces needed. It will be soon. Each VK context will be rendered by a different renderer process. Besides, VK contexts and GL contexts are not on the same timeline. We don't want one to delay another by presenting a unified timeline. > > cheers, > Gerd > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] Cleanup drm_dp_mst_topology_cbs hooks
On Mon, 9 Mar 2020 at 20:27, Lyude Paul wrote: > > On Sat, 2020-03-07 at 14:00 +0530, Pankaj Bharadiya wrote: > > drm_dp_mst_topology_mgr_cbs.register_connector callbacks are identical > > amongst every driver and don't do anything other than calling > > drm_connector_register(). > > drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical > > amongst every driver and don't do anything other than cleaning up the > > connector((drm_connector_unregister()/drm_connector_put())) except for > > amdgpu_dm driver where some amdgpu_dm specific code in there. > > Yeah that amdgpu destruction code kinda stinks a little bit :\. I think we can > just drop some of it and move the rest into their connector destruction > callbacks. > > For the whole series: > Reviewed-by: Lyude Paul > > I'm going to go ahead and let the maintainers know I'm going to push this > (since there's some minor changes here outside of drm-misc), and push this to > drm-misc-next. Then I'll go and write some patches to remove the leftover amd > bits and drop the callback for good (I'll cc it to you as well). > Thanks for following on these Pankaj. For the series: Reviewed-by: Emil Velikov -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Plumbing explicit synchronization through the Linux ecosystem
On Wed, Mar 11, 2020 at 12:31 PM Jason Ekstrand wrote: > > All, > > Sorry for casting such a broad net with this one. I'm sure most people > who reply will get at least one mailing list rejection. However, this > is an issue that affects a LOT of components and that's why it's > thorny to begin with. Please pardon the length of this e-mail as > well; I promise there's a concrete point/proposal at the end. > > > Explicit synchronization is the future of graphics and media. At > least, that seems to be the consensus among all the graphics people > I've talked to. I had a chat with one of the lead Android graphics > engineers recently who told me that doing explicit sync from the start > was one of the best engineering decisions Android ever made. It's > also the direction being taken by more modern APIs such as Vulkan. > > > ## What are implicit and explicit synchronization? > > For those that aren't familiar with this space, GPUs, media encoders, > etc. are massively parallel and synchronization of some form is > required to ensure that everything happens in the right order and > avoid data races. Implicit synchronization is when bits of work (3D, > compute, video encode, etc.) are implicitly based on the absolute > CPU-time order in which API calls occur. Explicit synchronization is > when the client (whatever that means in any given context) provides > the dependency graph explicitly via some sort of synchronization > primitives. If you're still confused, consider the following > examples: > > With OpenGL and EGL, almost everything is implicit sync. Say you have > two OpenGL contexts sharing an image where one writes to it and the > other textures from it. The way the OpenGL spec works, the client has > to make the API calls to render to the image before (in CPU time) it > makes the API calls which texture from the image. As long as it does > this (and maybe inserts a glFlush?), the driver will ensure that the > rendering completes before the texturing happens and you get correct > contents. > > Implicit synchronization can also happen across processes. Wayland, > for instance, is currently built on implicit sync where the client > does their rendering and then does a hand-off (via wl_surface::commit) > to tell the compositor it's done at which point the compositor can now > texture from the surface. The hand-off ensures that the client's > OpenGL API calls happen before the server's OpenGL API calls. > > A good example of explicit synchronization is the Vulkan API. There, > a client (or multiple clients) can simultaneously build command > buffers in different threads where one of those command buffers > renders to an image and the other textures from it and then submit > both of them at the same time with instructions to the driver for > which order to execute them in. The execution order is described via > the VkSemaphore primitive. With the new VK_KHR_timeline_semaphore > extension, you can even submit the work which does the texturing > BEFORE the work which does the rendering and the driver will sort it > out. > > The #1 problem with implicit synchronization (which explicit solves) > is that it leads to a lot of over-synchronization both in client space > and in driver/device space. The client has to synchronize a lot more > because it has to ensure that the API calls happen in a particular > order. The driver/device have to synchronize a lot more because they > never know what is going to end up being a synchronization point as an > API call on another thread/process may occur at any time. As we move > to more and more multi-threaded programming this synchronization (on > the client-side especially) becomes more and more painful. > > > ## Current status in Linux > > Implicit synchronization in Linux works via a the kernel's internal > dma_buf and dma_fence data structures. A dma_fence is a tiny object > which represents the "done" status for some bit of work. Typically, > dma_fences are created as a by-product of someone submitting some bit > of work (say, 3D rendering) to the kernel. The dma_buf object has a > set of dma_fences on it representing shared (read) and exclusive > (write) access to the object. When work is submitted which, for > instance renders to the dma_buf, it's queued waiting on all the fences > on the dma_buf and and a dma_fence is created representing the end of > said rendering work and it's installed as the dma_buf's exclusive > fence. This way, the kernel can manage all its internal queues (3D > rendering, display, video encode, etc.) and know which things to > submit in what order. > > For the last few years, we've had sync_file in the kernel and it's > plumbed into some drivers. A sync_file is just a wrapper around a > single dma_fence. A sync_file is typically created as a by-product of > submitting work (3D, compute, etc.) to the kernel and is signaled when > that work completes. When a sync_file is created, it is guaranteed by > the kernel that
Variable Refresh Rate & flickering screens
Hi all, I've been working on adding VRR support to Sway [1] (a Wayland compositor). The compositor just sets the VRR_ENABLED property. This works fine for some screens, but causes flcikering for other screens as expected [2]. Fixing the flickering is something we've talked about last XDC [3]. The flickering is caused by physical limitations of the screen: changing the refresh rate too quickly results in brightness issues. The approach taken by xf86-video-amdgpu is to only enable VRR if an app is fullscreen and not present in a special Mesa blacklist (e.g. Firefox is in the blacklist because it doesn't render at a fixed interval). For Wayland, I'd prefer to avoid having a blacklist. I'd like to be able to use VRR in the general case (not just for fullscreen apps). A way to fix the flickering would be to implement a slew rate and make it so refresh rate variations are capped by the slew rate. The main question is: should this be implemented in the kernel or in user-space? I personally think it'd make more sense to implement this in the kernel. This would de-duplicate the slew rate logic between compositors and would avoid unnecessarily waking up user-space. Moreover, it seems Intel hardware supports programming a slew rate directly. To take advantage of this feature the slew rate needs to be implemented in the kernel. Harry, what do you think? Thanks, Simon [1]: https://github.com/swaywm/sway/pull/5063 [2]: https://github.com/swaywm/sway/issues/5076 [3]: https://xdc2019.x.org/event/5/contributions/331/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 3/3] drm: bridge: cdns-mhdp: add j721e wrapper
Hi Yuti, Thank you for the patch. On Wed, Feb 26, 2020 at 11:22:59AM +0100, Yuti Amonkar wrote: > Add j721e wrapper for mhdp, which sets up the clock and data muxes. > > Signed-off-by: Yuti Amonkar > Signed-off-by: Jyri Sarha > Reviewed-by: Tomi Valkeinen > --- > drivers/gpu/drm/bridge/Kconfig | 12 > drivers/gpu/drm/bridge/Makefile | 4 ++ > drivers/gpu/drm/bridge/cdns-mhdp-core.c | 14 + > drivers/gpu/drm/bridge/cdns-mhdp-core.h | 1 + > drivers/gpu/drm/bridge/cdns-mhdp-j721e.c | 79 > drivers/gpu/drm/bridge/cdns-mhdp-j721e.h | 55 + > 6 files changed, 165 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-j721e.h > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 3bfabb76f2bb..ba945071bb0b 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -38,6 +38,18 @@ config DRM_CDNS_MHDP > It takes a DPI stream as input and output it encoded > in DP format. > > +if DRM_CDNS_MHDP > + > +config DRM_CDNS_MHDP_J721E > + bool "J721E Cadence DPI/DP wrapper support" > + default y Should this be automatically selected when support for the J721E platform is enabled, instead of being user-selectable ? > + help > + Support J721E Cadence DPI/DP wrapper. This is a wrapper > + which adds support for J721E related platform ops. It > + initializes the J721e Display Port and sets up the > + clock and data muxes. > +endif > + > config DRM_DUMB_VGA_DAC > tristate "Dumb VGA DAC Bridge support" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 2e2c5be7c714..fa575ad57b95 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -19,5 +19,9 @@ obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > obj-$(CONFIG_DRM_CDNS_MHDP) += cdns-mhdp.o > cdns-mhdp-objs := cdns-mhdp-core.o > > +ifeq ($(CONFIG_DRM_CDNS_MHDP_J721E),y) > + cdns-mhdp-objs += cdns-mhdp-j721e.o > +endif > + > obj-y += analogix/ > obj-y += synopsys/ > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.c > b/drivers/gpu/drm/bridge/cdns-mhdp-core.c > index cc642893baa8..8d07ffe2d791 100644 > --- a/drivers/gpu/drm/bridge/cdns-mhdp-core.c > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.c > @@ -36,8 +36,22 @@ > > #include "cdns-mhdp-core.h" > You can drop the blank line here. > +#include "cdns-mhdp-j721e.h" > + > +#ifdef CONFIG_DRM_CDNS_MHDP_J721E > +static const struct mhdp_platform_ops mhdp_ti_j721e_ops = { > + .init = cdns_mhdp_j721e_init, > + .exit = cdns_mhdp_j721e_fini, > + .enable = cdns_mhdp_j721e_enable, > + .disable = cdns_mhdp_j721e_disable, > +}; > +#endif > + How about moving this structure to cdns-mhdp-j721e.c instead of exposing the four functions ? > static const struct of_device_id mhdp_ids[] = { > { .compatible = "cdns,mhdp8546", }, > +#ifdef CONFIG_DRM_CDNS_MHDP_J721E > + { .compatible = "ti,j721e-mhdp8546", .data = &mhdp_ti_j721e_ops }, > +#endif > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, mhdp_ids); > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.h > b/drivers/gpu/drm/bridge/cdns-mhdp-core.h > index f8df54917816..0878a6e3fd31 100644 > --- a/drivers/gpu/drm/bridge/cdns-mhdp-core.h > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.h > @@ -335,6 +335,7 @@ struct mhdp_platform_ops { > > struct cdns_mhdp_device { > void __iomem *regs; > + void __iomem *j721e_regs; > > struct device *dev; > struct clk *clk; > diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > new file mode 100644 > index ..a87faf55c065 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/cdns-mhdp-j721e.c > @@ -0,0 +1,79 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * TI j721e Cadence MHDP DP wrapper > + * > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Jyri Sarha + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. You can drop this paragraph, it's implied by the SPDX header. > + */ > + > +#include This should be linux/platform_device.h > +#include > + > +#include "cdns-mhdp-j721e.h" > + > +#define REVISION0x00 > +#define DPTX_IPCFG 0x04 > +#define ECC_MEM_CFG 0x08 > +#define DPTX_DSC_CFG0x0c > +#define DPTX_SRC_CFG0x10 > +#define DPTX_VIF_SECURE_MODE_CFG0x14 > +#define DPTX_VIF_CONN_STATUS0x18 > +#define PHY_CLK_STATUS 0x1c > + > +#define DPTX_SRC_AIF_EN BIT(16) > +#define D
[PATCH] dt-bindings: display: Fix dtc unit-address warnings in examples
Extra dtc warnings (roughly what W=1 enables) are now enabled by default when building the binding examples. These were fixed treewide in 5.6-rc5, but some new display bindings have been added with new warnings: Documentation/devicetree/bindings/display/panel/raydium,rm68200.example.dts:17.7-27.11: Warning (unit_address_vs_reg): /example-0/dsi@0: node has a unit name, but no reg property Documentation/devicetree/bindings/display/panel/panel-simple-dsi.example.dts:17.19-31.11: Warning (unit_address_vs_reg): /example-0/mdss_dsi@fd922800: node has a unit name, but no reg property Documentation/devicetree/bindings/display/panel/orisetech,otm8009a.example.dts:17.7-26.11: Warning (unit_address_vs_reg): /example-0/dsi@0: node has a unit name, but no reg property Documentation/devicetree/bindings/display/ti/ti,am65x-dss.example.dts:21.27-49.11: Warning (unit_address_format): /example-0/dss@04a0: unit name should not have leading 0s Documentation/devicetree/bindings/display/ti/ti,j721e-dss.example.dts:21.27-72.11: Warning (unit_address_format): /example-0/dss@04a0: unit name should not have leading 0s Documentation/devicetree/bindings/display/ti/ti,k2g-dss.example.dts:20.27-42.11: Warning (unit_address_format): /example-0/dss@0254: unit name should not have leading 0s Cc: Thierry Reding Cc: Sam Ravnborg Cc: Jyri Sarha Cc: Tomi Valkeinen Signed-off-by: Rob Herring --- .../devicetree/bindings/display/panel/orisetech,otm8009a.yaml | 3 +-- .../devicetree/bindings/display/panel/panel-simple-dsi.yaml| 2 +- .../devicetree/bindings/display/panel/raydium,rm68200.yaml | 2 +- Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml | 2 +- Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml | 2 +- Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml | 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/orisetech,otm8009a.yaml b/Documentation/devicetree/bindings/display/panel/orisetech,otm8009a.yaml index 6e6ac995c27b..2e7c65b093d7 100644 --- a/Documentation/devicetree/bindings/display/panel/orisetech,otm8009a.yaml +++ b/Documentation/devicetree/bindings/display/panel/orisetech,otm8009a.yaml @@ -39,7 +39,7 @@ required: examples: - | -dsi@0 { +dsi { #address-cells = <1>; #size-cells = <0>; panel@0 { @@ -50,4 +50,3 @@ examples: }; }; ... - diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml index 8b60368a2425..b2e8742fd6af 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml @@ -50,7 +50,7 @@ required: examples: - | -mdss_dsi@fd922800 { +dsi { #address-cells = <1>; #size-cells = <0>; panel@0 { diff --git a/Documentation/devicetree/bindings/display/panel/raydium,rm68200.yaml b/Documentation/devicetree/bindings/display/panel/raydium,rm68200.yaml index 09149f140d5f..a35ba16fc000 100644 --- a/Documentation/devicetree/bindings/display/panel/raydium,rm68200.yaml +++ b/Documentation/devicetree/bindings/display/panel/raydium,rm68200.yaml @@ -42,7 +42,7 @@ required: examples: - | -dsi@0 { +dsi { #address-cells = <1>; #size-cells = <0>; panel@0 { diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml index cac61a998203..aa5543a64526 100644 --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml @@ -121,7 +121,7 @@ examples: #include #include -dss: dss@04a0 { +dss: dss@4a0 { compatible = "ti,am65x-dss"; reg = <0x0 0x04a0 0x0 0x1000>, /* common */ <0x0 0x04a02000 0x0 0x1000>, /* vidl1 */ diff --git a/Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml index ade9b2f513f5..6d47cd7206c2 100644 --- a/Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml +++ b/Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml @@ -154,7 +154,7 @@ examples: #include #include -dss: dss@04a0 { +dss: dss@4a0 { compatible = "ti,j721e-dss"; reg = <0x00 0x04a0 0x00 0x1>, /* common_m */ <0x00 0x04a1 0x00 0x1>, /* common_s0*/ diff --git a/Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml index 385bd060ccf9..7cb37053e95b 100644 --- a/Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml +++ b/Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml @@ -81,7 +81,7 @@ examples: #include