From cf3d02a185edf449d30465dae8d22a4979545829 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Wed, 7 Nov 2018 15:55:33 -0500 Subject: [PATCH 01/23] drm: Fix htmldocs warnings in drm_fourcc.c Add a description for dev and remove the excess one for native. Fixes the following warnings: ../drivers/gpu/drm/drm_fourcc.c:112: warning: Function parameter or member 'dev' not described in 'drm_driver_legacy_fb_format' ../drivers/gpu/drm/drm_fourcc.c:112: warning: Excess function parameter 'native' description in 'drm_driver_legacy_fb_format' Fixes: 059b5eb5d955 ("drm: move native byte order quirk to new drm_driver_legacy_fb_format function") Cc: Gerd Hoffmann Cc: Daniel Vetter Reviewed-by: Daniel Vetter Signed-off-by: Sean Paul Link: https://patchwork.freedesktop.org/patch/msgid/20181107205546.216088-1-sean@poorly.run --- drivers/gpu/drm/drm_fourcc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index 90a1c846fc25..8aaa5e86a979 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -97,9 +97,9 @@ EXPORT_SYMBOL(drm_mode_legacy_fb_format); /** * drm_driver_legacy_fb_format - compute drm fourcc code from legacy description + * @dev: DRM device * @bpp: bits per pixels * @depth: bit depth per pixel - * @native: use host native byte order * * Computes a drm fourcc pixel format code for the given @bpp/@depth values. * Unlike drm_mode_legacy_fb_format() this looks at the drivers mode_config, From 23d8003907d094f77cf959228e2248d6db819fa7 Mon Sep 17 00:00:00 2001 From: Stanislav Lisovskiy Date: Fri, 9 Nov 2018 11:00:12 +0200 Subject: [PATCH 02/23] drm/dp_mst: Check if primary mstb is null Unfortunately drm_dp_get_mst_branch_device which is called from both drm_dp_mst_handle_down_rep and drm_dp_mst_handle_up_rep seem to rely on that mgr->mst_primary is not NULL, which seem to be wrong as it can be cleared with simultaneous mode set, if probing fails or in other case. mgr->lock mutex doesn't protect against that as it might just get assigned to NULL right before, not simultaneously. There are currently bugs 107738, 108616 bugs which crash in drm_dp_get_mst_branch_device, caused by this issue. v2: Refactored the code, as it was nicely noticed. Fixed Bugzilla bug numbers(second was 108616, but not 108816) and added links. [changed title and added stable cc] Signed-off-by: Lyude Paul Signed-off-by: Stanislav Lisovskiy Cc: stable@vger.kernel.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108616 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107738 Link: https://patchwork.freedesktop.org/patch/msgid/20181109090012.24438-1-stanislav.lisovskiy@intel.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 5ff1d79b86c4..0e0df398222d 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1275,6 +1275,9 @@ static struct drm_dp_mst_branch *drm_dp_get_mst_branch_device(struct drm_dp_mst_ mutex_lock(&mgr->lock); mstb = mgr->mst_primary; + if (!mstb) + goto out; + for (i = 0; i < lct - 1; i++) { int shift = (i % 2) ? 0 : 4; int port_num = (rad[i / 2] >> shift) & 0xf; From e0c827aca0730b51f38081aa4e8ecf0912aab55f Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sat, 10 Nov 2018 13:16:51 +0200 Subject: [PATCH 03/23] drm/omap: Populate DSS children in omapdss driver The DSS DT node contains children that describe the DSS components (DISPC and internal encoders). Each of those components is handled by a platform driver, and thus needs to be backed by a platform device. The corresponding platform devices are created in mach-omap2 code by a call to of_platform_populate(). While this approach has worked so far, it doesn't model the hardware architecture very well, as it creates child devices before the parent is ready to handle them. This would be akin to creating I2C slaves before the I2C master is available. The task can be easily performed in the omapdss driver code instead, simplifying mach-omap2 code. We however can't remove the mach-omap2 code completely as the omap2fb driver still depends on it, but we can move it to the omap2fb-specific section, where it can stay until the omap2fb driver gets removed. This has the added benefit of not allowing DSS components to probe before the DSS itself, which led to runtime PM issues when the DSS probe is deferred. Fixes: 27d624527d99 ("drm/omap: dss: Acquire next dssdev at probe time") Signed-off-by: Laurent Pinchart Acked-by: Tony Lindgren Reviewed-by: Sebastian Reichel Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/20181110111654.4387-2-laurent.pinchart@ideasonboard.com --- arch/arm/mach-omap2/display.c | 111 ++++++++++++++---------------- drivers/gpu/drm/omapdrm/dss/dss.c | 11 ++- 2 files changed, 63 insertions(+), 59 deletions(-) diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c index 9500b6e27380..f86b72d1d59e 100644 --- a/arch/arm/mach-omap2/display.c +++ b/arch/arm/mach-omap2/display.c @@ -209,11 +209,61 @@ static int __init omapdss_init_fbdev(void) return 0; } -#else -static inline int omapdss_init_fbdev(void) + +static const char * const omapdss_compat_names[] __initconst = { + "ti,omap2-dss", + "ti,omap3-dss", + "ti,omap4-dss", + "ti,omap5-dss", + "ti,dra7-dss", +}; + +static struct device_node * __init omapdss_find_dss_of_node(void) { - return 0; + struct device_node *node; + int i; + + for (i = 0; i < ARRAY_SIZE(omapdss_compat_names); ++i) { + node = of_find_compatible_node(NULL, NULL, + omapdss_compat_names[i]); + if (node) + return node; + } + + return NULL; } + +static int __init omapdss_init_of(void) +{ + int r; + struct device_node *node; + struct platform_device *pdev; + + /* only create dss helper devices if dss is enabled in the .dts */ + + node = omapdss_find_dss_of_node(); + if (!node) + return 0; + + if (!of_device_is_available(node)) + return 0; + + pdev = of_find_device_by_node(node); + + if (!pdev) { + pr_err("Unable to find DSS platform device\n"); + return -ENODEV; + } + + r = of_platform_populate(node, NULL, NULL, &pdev->dev); + if (r) { + pr_err("Unable to populate DSS submodule devices\n"); + return r; + } + + return omapdss_init_fbdev(); +} +omap_device_initcall(omapdss_init_of); #endif /* CONFIG_FB_OMAP2 */ static void dispc_disable_outputs(void) @@ -361,58 +411,3 @@ int omap_dss_reset(struct omap_hwmod *oh) return r; } - -static const char * const omapdss_compat_names[] __initconst = { - "ti,omap2-dss", - "ti,omap3-dss", - "ti,omap4-dss", - "ti,omap5-dss", - "ti,dra7-dss", -}; - -static struct device_node * __init omapdss_find_dss_of_node(void) -{ - struct device_node *node; - int i; - - for (i = 0; i < ARRAY_SIZE(omapdss_compat_names); ++i) { - node = of_find_compatible_node(NULL, NULL, - omapdss_compat_names[i]); - if (node) - return node; - } - - return NULL; -} - -static int __init omapdss_init_of(void) -{ - int r; - struct device_node *node; - struct platform_device *pdev; - - /* only create dss helper devices if dss is enabled in the .dts */ - - node = omapdss_find_dss_of_node(); - if (!node) - return 0; - - if (!of_device_is_available(node)) - return 0; - - pdev = of_find_device_by_node(node); - - if (!pdev) { - pr_err("Unable to find DSS platform device\n"); - return -ENODEV; - } - - r = of_platform_populate(node, NULL, NULL, &pdev->dev); - if (r) { - pr_err("Unable to populate DSS submodule devices\n"); - return r; - } - - return omapdss_init_fbdev(); -} -omap_device_initcall(omapdss_init_of); diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c index 1aaf260aa9b8..7553c7fc1c45 100644 --- a/drivers/gpu/drm/omapdrm/dss/dss.c +++ b/drivers/gpu/drm/omapdrm/dss/dss.c @@ -1484,16 +1484,23 @@ static int dss_probe(struct platform_device *pdev) dss); /* Add all the child devices as components. */ + r = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev); + if (r) + goto err_uninit_debugfs; + omapdss_gather_components(&pdev->dev); device_for_each_child(&pdev->dev, &match, dss_add_child_component); r = component_master_add_with_match(&pdev->dev, &dss_component_ops, match); if (r) - goto err_uninit_debugfs; + goto err_of_depopulate; return 0; +err_of_depopulate: + of_platform_depopulate(&pdev->dev); + err_uninit_debugfs: dss_debugfs_remove_file(dss->debugfs.clk); dss_debugfs_remove_file(dss->debugfs.dss); @@ -1522,6 +1529,8 @@ static int dss_remove(struct platform_device *pdev) { struct dss_device *dss = platform_get_drvdata(pdev); + of_platform_depopulate(&pdev->dev); + component_master_del(&pdev->dev, &dss_component_ops); dss_debugfs_remove_file(dss->debugfs.clk); From f8523b64d2d2f94bb429c15166d7601d39243c4d Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sat, 10 Nov 2018 13:16:52 +0200 Subject: [PATCH 04/23] drm/omap: hdmi4: Ensure the device is active during bind The bind function performs hardware access (in hdmi4_cec_init()) and thus requires the device to be active. Ensure this by surrounding the bind function by hdmi_runtime_get() and hdmi_runtime_put() calls. Fixes: 27d624527d99 ("drm/omap: dss: Acquire next dssdev at probe time") Signed-off-by: Laurent Pinchart Reviewed-by: Sebastian Reichel Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/20181110111654.4387-3-laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index cf6230eac31a..073fa462930a 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c @@ -635,10 +635,14 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data) hdmi->dss = dss; - r = hdmi_pll_init(dss, hdmi->pdev, &hdmi->pll, &hdmi->wp); + r = hdmi_runtime_get(hdmi); if (r) return r; + r = hdmi_pll_init(dss, hdmi->pdev, &hdmi->pll, &hdmi->wp); + if (r) + goto err_runtime_put; + r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp); if (r) goto err_pll_uninit; @@ -652,12 +656,16 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data) hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs, hdmi); + hdmi_runtime_put(hdmi); + return 0; err_cec_uninit: hdmi4_cec_uninit(&hdmi->core); err_pll_uninit: hdmi_pll_uninit(&hdmi->pll); +err_runtime_put: + hdmi_runtime_put(hdmi); return r; } From 350c03e880038bf60184500bab9025d3361c0b0e Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sat, 10 Nov 2018 13:16:53 +0200 Subject: [PATCH 05/23] drm/omap: dsi: Ensure the device is active during probe The probe function performs hardware access to read the number of supported data lanes from a configuration register and thus requires the device to be active. Ensure this by surrounding the access with dsi_runtime_get() and dsi_runtime_put() calls. Fixes: edb715dffdee ("drm/omap: dss: dsi: Move initialization code from bind to probe") Signed-off-by: Laurent Pinchart Acked-by: Tony Lindgren Reviewed-by: Sebastian Reichel Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/20181110111654.4387-4-laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/dsi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index 394c129cfb3b..b9d5ad7e67d8 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -5409,11 +5409,14 @@ static int dsi_probe(struct platform_device *pdev) /* DSI on OMAP3 doesn't have register DSI_GNQ, set number * of data to 3 by default */ - if (dsi->data->quirks & DSI_QUIRK_GNQ) + if (dsi->data->quirks & DSI_QUIRK_GNQ) { + dsi_runtime_get(dsi); /* NB_DATA_LANES */ dsi->num_lanes_supported = 1 + REG_GET(dsi, DSI_GNQ, 11, 9); - else + dsi_runtime_put(dsi); + } else { dsi->num_lanes_supported = 3; + } r = dsi_init_output(dsi); if (r) From 24ec84e854c68ceda59a26027114eb7f260f9411 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sat, 10 Nov 2018 13:16:54 +0200 Subject: [PATCH 06/23] drm/omap: Move DISPC runtime PM handling to omapdrm The internal encoders (DSI, HDMI4, HDMI5 and VENC) runtime PM handlers attempt to manage the runtime PM state of the connected DISPC, based on the rationale that the DISPC providing data to the encoders requires ensuring that the display is active whenever the encoders are active. While the DISPC provides data to the encoders, it doesn't as such constitute a resource that encoders require in order to be taken out of suspend, contrary to for instance a functional clock or a power supply. Encoders registers can be accessed without the DISPC being active, and while the encoders will not output any video stream without being fed by the DISPC, the DISPC PM state doesn't influence the encoders PM state. For this reason the DISPC PM state is better managed from the omapdrm driver, in the CRTC enable and disable operations. This allows the encoders PM state to be handled separately from the DISPC, and in particular at times when the DISPC may not be available (for instance at probe due to the DSS probe being deferred, or at remove time du to the DISPC being already removed). Fixes: edb715dffdee ("drm/omap: dss: dsi: Move initialization code from bind to probe") Signed-off-by: Laurent Pinchart Reviewed-by: Sebastian Reichel Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/20181110111654.4387-5-laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/dsi.c | 7 ------- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 27 --------------------------- drivers/gpu/drm/omapdrm/dss/hdmi5.c | 27 --------------------------- drivers/gpu/drm/omapdrm/dss/venc.c | 7 ------- drivers/gpu/drm/omapdrm/omap_crtc.c | 6 ++++++ 5 files changed, 6 insertions(+), 68 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index b9d5ad7e67d8..36123c086d97 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -5473,19 +5473,12 @@ static int dsi_runtime_suspend(struct device *dev) /* wait for current handler to finish before turning the DSI off */ synchronize_irq(dsi->irq); - dispc_runtime_put(dsi->dss->dispc); - return 0; } static int dsi_runtime_resume(struct device *dev) { struct dsi_data *dsi = dev_get_drvdata(dev); - int r; - - r = dispc_runtime_get(dsi->dss->dispc); - if (r) - return r; dsi->is_enabled = true; /* ensure the irq handler sees the is_enabled value */ diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index 073fa462930a..aabdda394c9c 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c @@ -841,32 +841,6 @@ static int hdmi4_remove(struct platform_device *pdev) return 0; } -static int hdmi_runtime_suspend(struct device *dev) -{ - struct omap_hdmi *hdmi = dev_get_drvdata(dev); - - dispc_runtime_put(hdmi->dss->dispc); - - return 0; -} - -static int hdmi_runtime_resume(struct device *dev) -{ - struct omap_hdmi *hdmi = dev_get_drvdata(dev); - int r; - - r = dispc_runtime_get(hdmi->dss->dispc); - if (r < 0) - return r; - - return 0; -} - -static const struct dev_pm_ops hdmi_pm_ops = { - .runtime_suspend = hdmi_runtime_suspend, - .runtime_resume = hdmi_runtime_resume, -}; - static const struct of_device_id hdmi_of_match[] = { { .compatible = "ti,omap4-hdmi", }, {}, @@ -877,7 +851,6 @@ struct platform_driver omapdss_hdmi4hw_driver = { .remove = hdmi4_remove, .driver = { .name = "omapdss_hdmi", - .pm = &hdmi_pm_ops, .of_match_table = hdmi_of_match, .suppress_bind_attrs = true, }, diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c index b0e4a7463f8c..9e8556f67a29 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c @@ -825,32 +825,6 @@ static int hdmi5_remove(struct platform_device *pdev) return 0; } -static int hdmi_runtime_suspend(struct device *dev) -{ - struct omap_hdmi *hdmi = dev_get_drvdata(dev); - - dispc_runtime_put(hdmi->dss->dispc); - - return 0; -} - -static int hdmi_runtime_resume(struct device *dev) -{ - struct omap_hdmi *hdmi = dev_get_drvdata(dev); - int r; - - r = dispc_runtime_get(hdmi->dss->dispc); - if (r < 0) - return r; - - return 0; -} - -static const struct dev_pm_ops hdmi_pm_ops = { - .runtime_suspend = hdmi_runtime_suspend, - .runtime_resume = hdmi_runtime_resume, -}; - static const struct of_device_id hdmi_of_match[] = { { .compatible = "ti,omap5-hdmi", }, { .compatible = "ti,dra7-hdmi", }, @@ -862,7 +836,6 @@ struct platform_driver omapdss_hdmi5hw_driver = { .remove = hdmi5_remove, .driver = { .name = "omapdss_hdmi5", - .pm = &hdmi_pm_ops, .of_match_table = hdmi_of_match, .suppress_bind_attrs = true, }, diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c b/drivers/gpu/drm/omapdrm/dss/venc.c index ff0b18c8e4ac..b5f52727f8b1 100644 --- a/drivers/gpu/drm/omapdrm/dss/venc.c +++ b/drivers/gpu/drm/omapdrm/dss/venc.c @@ -946,19 +946,12 @@ static int venc_runtime_suspend(struct device *dev) if (venc->tv_dac_clk) clk_disable_unprepare(venc->tv_dac_clk); - dispc_runtime_put(venc->dss->dispc); - return 0; } static int venc_runtime_resume(struct device *dev) { struct venc_device *venc = dev_get_drvdata(dev); - int r; - - r = dispc_runtime_get(venc->dss->dispc); - if (r < 0) - return r; if (venc->tv_dac_clk) clk_prepare_enable(venc->tv_dac_clk); diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 62928ec0e7db..caffc547ef97 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -350,11 +350,14 @@ static void omap_crtc_arm_event(struct drm_crtc *crtc) static void omap_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct omap_drm_private *priv = crtc->dev->dev_private; struct omap_crtc *omap_crtc = to_omap_crtc(crtc); int ret; DBG("%s", omap_crtc->name); + priv->dispc_ops->runtime_get(priv->dispc); + spin_lock_irq(&crtc->dev->event_lock); drm_crtc_vblank_on(crtc); ret = drm_crtc_vblank_get(crtc); @@ -367,6 +370,7 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc, static void omap_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct omap_drm_private *priv = crtc->dev->dev_private; struct omap_crtc *omap_crtc = to_omap_crtc(crtc); DBG("%s", omap_crtc->name); @@ -379,6 +383,8 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc, spin_unlock_irq(&crtc->dev->event_lock); drm_crtc_vblank_off(crtc); + + priv->dispc_ops->runtime_put(priv->dispc); } static enum drm_mode_status omap_crtc_mode_valid(struct drm_crtc *crtc, From cbed7545db7ae5907d7dc9d4002717d46cae29e9 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Tue, 6 Nov 2018 07:28:02 -0800 Subject: [PATCH 07/23] drm/omap: dsi: Fix missing of_platform_depopulate() We're missing a call to of_platform_depopulate() on errors for dsi. Looks like dss is already doing this. Signed-off-by: Tony Lindgren Reviewed-by: Laurent Pinchart Reviewed-by: Sebastian Reichel Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/20181106152802.38599-1-tony@atomide.com --- drivers/gpu/drm/omapdrm/dss/dsi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index 36123c086d97..0a485c5b982e 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -5429,15 +5429,19 @@ static int dsi_probe(struct platform_device *pdev) } r = of_platform_populate(dev->of_node, NULL, NULL, dev); - if (r) + if (r) { DSSERR("Failed to populate DSI child devices: %d\n", r); + goto err_uninit_output; + } r = component_add(&pdev->dev, &dsi_component_ops); if (r) - goto err_uninit_output; + goto err_of_depopulate; return 0; +err_of_depopulate: + of_platform_depopulate(dev); err_uninit_output: dsi_uninit_output(dsi); err_pm_disable: From fb5bbae9b1333d44023713946fdd28db0cd85751 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 5 Nov 2018 09:43:05 +0000 Subject: [PATCH 08/23] drm/i915/ringbuffer: Delay after EMIT_INVALIDATE for gen4/gen5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exercising the gpu reloc path strenuously revealed an issue where the updated relocations (from MI_STORE_DWORD_IMM) were not being observed upon execution. After some experiments with adding pipecontrols (a lot of pipecontrols (32) as gen4/5 do not have a bit to wait on earlier pipe controls or even the current on), it was discovered that we merely needed to delay the EMIT_INVALIDATE by several flushes. It is important to note that it is the EMIT_INVALIDATE as opposed to the EMIT_FLUSH that needs the delay as opposed to what one might first expect -- that the delay is required for the TLB invalidation to take effect (one presumes to purge any CS buffers) as opposed to a delay after flushing to ensure the writes have landed before triggering invalidation. Testcase: igt/gem_tiled_fence_blits Signed-off-by: Chris Wilson Cc: stable@vger.kernel.org Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181105094305.5767-1-chris@chris-wilson.co.uk (cherry picked from commit 55f99bf2a9c331838c981694bc872cd1ec4070b2) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_ringbuffer.c | 38 +++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index d0ef50bf930a..187bb0ceb4ac 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -91,6 +91,7 @@ static int gen4_render_ring_flush(struct i915_request *rq, u32 mode) { u32 cmd, *cs; + int i; /* * read/write caches: @@ -127,12 +128,45 @@ gen4_render_ring_flush(struct i915_request *rq, u32 mode) cmd |= MI_INVALIDATE_ISP; } - cs = intel_ring_begin(rq, 2); + i = 2; + if (mode & EMIT_INVALIDATE) + i += 20; + + cs = intel_ring_begin(rq, i); if (IS_ERR(cs)) return PTR_ERR(cs); *cs++ = cmd; - *cs++ = MI_NOOP; + + /* + * A random delay to let the CS invalidate take effect? Without this + * delay, the GPU relocation path fails as the CS does not see + * the updated contents. Just as important, if we apply the flushes + * to the EMIT_FLUSH branch (i.e. immediately after the relocation + * write and before the invalidate on the next batch), the relocations + * still fail. This implies that is a delay following invalidation + * that is required to reset the caches as opposed to a delay to + * ensure the memory is written. + */ + if (mode & EMIT_INVALIDATE) { + *cs++ = GFX_OP_PIPE_CONTROL(4) | PIPE_CONTROL_QW_WRITE; + *cs++ = i915_ggtt_offset(rq->engine->scratch) | + PIPE_CONTROL_GLOBAL_GTT; + *cs++ = 0; + *cs++ = 0; + + for (i = 0; i < 12; i++) + *cs++ = MI_FLUSH; + + *cs++ = GFX_OP_PIPE_CONTROL(4) | PIPE_CONTROL_QW_WRITE; + *cs++ = i915_ggtt_offset(rq->engine->scratch) | + PIPE_CONTROL_GLOBAL_GTT; + *cs++ = 0; + *cs++ = 0; + } + + *cs++ = cmd; + intel_ring_advance(rq, cs); return 0; From 7c4512300cfa5a4dcc8c1c52ae61e3fa4bd11a39 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 6 Nov 2018 16:30:12 -0500 Subject: [PATCH 09/23] drm/i915: Fix possible race in intel_dp_add_mst_connector() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This hasn't caused any issues yet that I'm aware of, but as Ville Syrjälä pointed out - we need to make sure that intel_connector->mst_port is set before initializing MST connectors, since in theory we could potentially check intel_connector->mst_port in i915_hpd_poll_init_work() after registering the connector but before having written it's value. Signed-off-by: Lyude Paul Reviewed-by: Ville Syrjälä Cc: Rodrigo Vivi Cc: stable@vger.kernel.org Link: https://patchwork.freedesktop.org/patch/msgid/20181106213017.14563-2-lyude@redhat.com (cherry picked from commit 66a5ab1034be801630816d1fa6cfc30db1a2f0b0) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_dp_mst.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 1b00f8ea145b..a911691dbd0f 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -452,6 +452,10 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo if (!intel_connector) return NULL; + intel_connector->get_hw_state = intel_dp_mst_get_hw_state; + intel_connector->mst_port = intel_dp; + intel_connector->port = port; + connector = &intel_connector->base; ret = drm_connector_init(dev, connector, &intel_dp_mst_connector_funcs, DRM_MODE_CONNECTOR_DisplayPort); @@ -462,10 +466,6 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo drm_connector_helper_add(connector, &intel_dp_mst_connector_helper_funcs); - intel_connector->get_hw_state = intel_dp_mst_get_hw_state; - intel_connector->mst_port = intel_dp; - intel_connector->port = port; - for_each_pipe(dev_priv, pipe) { struct drm_encoder *enc = &intel_dp->mst_encoders[pipe]->base.base; From 541ff7e96c13cd5d67f6021d233f8e1c3df49278 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 6 Nov 2018 16:30:13 -0500 Subject: [PATCH 10/23] drm/i915: Fix NULL deref when re-enabling HPD IRQs on systems with MST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns out that if you trigger an HPD storm on a system that has an MST topology connected to it, you'll end up causing the kernel to eventually hit a NULL deref: [ 332.339041] BUG: unable to handle kernel NULL pointer dereference at 00000000000000ec [ 332.340906] PGD 0 P4D 0 [ 332.342750] Oops: 0000 [#1] SMP PTI [ 332.344579] CPU: 2 PID: 25 Comm: kworker/2:0 Kdump: loaded Tainted: G O 4.18.0-rc3short-hpd-storm+ #2 [ 332.346453] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET71WW (1.35 ) 09/14/2018 [ 332.348361] Workqueue: events intel_hpd_irq_storm_reenable_work [i915] [ 332.350301] RIP: 0010:intel_hpd_irq_storm_reenable_work.cold.3+0x2f/0x86 [i915] [ 332.352213] Code: 00 00 ba e8 00 00 00 48 c7 c6 c0 aa 5f a0 48 c7 c7 d0 73 62 a0 4c 89 c1 4c 89 04 24 e8 7f f5 af e0 4c 8b 04 24 44 89 f8 29 e8 <41> 39 80 ec 00 00 00 0f 85 43 13 fc ff 41 0f b6 86 b8 04 00 00 41 [ 332.354286] RSP: 0018:ffffc90000147e48 EFLAGS: 00010006 [ 332.356344] RAX: 0000000000000005 RBX: ffff8802c226c9d4 RCX: 0000000000000006 [ 332.358404] RDX: 0000000000000000 RSI: 0000000000000082 RDI: ffff88032dc95570 [ 332.360466] RBP: 0000000000000005 R08: 0000000000000000 R09: ffff88031b3dc840 [ 332.362528] R10: 0000000000000000 R11: 000000031a069602 R12: ffff8802c226ca20 [ 332.364575] R13: ffff8802c2268000 R14: ffff880310661000 R15: 000000000000000a [ 332.366615] FS: 0000000000000000(0000) GS:ffff88032dc80000(0000) knlGS:0000000000000000 [ 332.368658] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 332.370690] CR2: 00000000000000ec CR3: 000000000200a003 CR4: 00000000003606e0 [ 332.372724] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 332.374773] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 332.376798] Call Trace: [ 332.378809] process_one_work+0x1a1/0x350 [ 332.380806] worker_thread+0x30/0x380 [ 332.382777] ? wq_update_unbound_numa+0x10/0x10 [ 332.384772] kthread+0x112/0x130 [ 332.386740] ? kthread_create_worker_on_cpu+0x70/0x70 [ 332.388706] ret_from_fork+0x35/0x40 [ 332.390651] Modules linked in: i915(O) vfat fat joydev btusb btrtl btbcm btintel bluetooth ecdh_generic iTCO_wdt wmi_bmof i2c_algo_bit drm_kms_helper intel_rapl syscopyarea sysfillrect x86_pkg_temp_thermal sysimgblt coretemp fb_sys_fops crc32_pclmul drm psmouse pcspkr mei_me mei i2c_i801 lpc_ich mfd_core i2c_core tpm_tis tpm_tis_core thinkpad_acpi wmi tpm rfkill video crc32c_intel serio_raw ehci_pci xhci_pci ehci_hcd xhci_hcd [last unloaded: i915] [ 332.394963] CR2: 00000000000000ec This appears to be due to the fact that with an MST topology, not all intel_connector structs will have ->encoder set. So, fix this by skipping connectors without encoders in intel_hpd_irq_storm_reenable_work(). For those wondering, this bug was found on accident while simulating HPD storms using a Chamelium connected to a ThinkPad T450s (Broadwell). Changes since v1: - Check intel_connector->mst_port instead of intel_connector->encoder Signed-off-by: Lyude Paul Reviewed-by: Ville Syrjälä Cc: stable@vger.kernel.org Cc: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20181106213017.14563-3-lyude@redhat.com (cherry picked from commit fee61deecb1d850bf34f682a6a452e5ee51b7572) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_hotplug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index 648a13c6043c..8326900a311e 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -228,7 +228,9 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work) drm_for_each_connector_iter(connector, &conn_iter) { struct intel_connector *intel_connector = to_intel_connector(connector); - if (intel_connector->encoder->hpd_pin == pin) { + /* Don't check MST ports, they don't have pins */ + if (!intel_connector->mst_port && + intel_connector->encoder->hpd_pin == pin) { if (connector->polled != intel_connector->polled) DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", connector->name); From c4f224076d00ccf30c7bd3561cceaed82628c8ce Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Fri, 2 Nov 2018 20:22:00 +0200 Subject: [PATCH 11/23] drm/i915/icl: Fix power well 2 wrt. DC-off toggling order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To enable DC5/6 power well 2 has to be disabled as for previous platforms, so fix things up. Bspec: 4234 Fixes: 67ca07e7ac10 ("drm/i915/icl: Add power well support") Cc: Animesh Manna Cc: Paulo Zanoni Cc: Ville Syrjälä Signed-off-by: Imre Deak Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181102182200.17219-1-imre.deak@intel.com (cherry picked from commit a33e1ece777996ddddb1f23a30f8c66422ed0b68) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 0fdabce647ab..0a4990d8843c 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -2748,6 +2748,12 @@ static const struct i915_power_well_desc icl_power_wells[] = { .hsw.has_fuses = true, }, }, + { + .name = "DC off", + .domains = ICL_DISPLAY_DC_OFF_POWER_DOMAINS, + .ops = &gen9_dc_off_power_well_ops, + .id = DISP_PW_ID_NONE, + }, { .name = "power well 2", .domains = ICL_PW_2_POWER_DOMAINS, @@ -2759,12 +2765,6 @@ static const struct i915_power_well_desc icl_power_wells[] = { .hsw.has_fuses = true, }, }, - { - .name = "DC off", - .domains = ICL_DISPLAY_DC_OFF_POWER_DOMAINS, - .ops = &gen9_dc_off_power_well_ops, - .id = DISP_PW_ID_NONE, - }, { .name = "power well 3", .domains = ICL_PW_3_POWER_DOMAINS, From 0a823e8fd4fd67726697854578f3584ee3a49b1d Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 8 Nov 2018 08:17:38 +0000 Subject: [PATCH 12/23] drm/i915/execlists: Force write serialisation into context image vs execution Ensure that the writes into the context image are completed prior to the register mmio to trigger execution. Although previously we were assured by the SDM that all writes are flushed before an uncached memory transaction (our mmio write to submit the context to HW for execution), we have empirical evidence to believe that this is not actually the case. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108656 References: https://bugs.freedesktop.org/show_bug.cgi?id=108315 References: https://bugs.freedesktop.org/show_bug.cgi?id=106887 Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Tvrtko Ursulin Acked-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20181108081740.25615-1-chris@chris-wilson.co.uk Cc: stable@vger.kernel.org (cherry picked from commit 987abd5c62f92ee4970b45aa077f47949974e615) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_lrc.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 43957bb37a42..37c94a54efcb 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -424,7 +424,8 @@ static u64 execlists_update_context(struct i915_request *rq) reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail); - /* True 32b PPGTT with dynamic page allocation: update PDP + /* + * True 32b PPGTT with dynamic page allocation: update PDP * registers and point the unallocated PDPs to scratch page. * PML4 is allocated during ppgtt init, so this is not needed * in 48-bit mode. @@ -432,6 +433,17 @@ static u64 execlists_update_context(struct i915_request *rq) if (ppgtt && !i915_vm_is_48bit(&ppgtt->vm)) execlists_update_context_pdps(ppgtt, reg_state); + /* + * Make sure the context image is complete before we submit it to HW. + * + * Ostensibly, writes (including the WCB) should be flushed prior to + * an uncached write such as our mmio register access, the empirical + * evidence (esp. on Braswell) suggests that the WC write into memory + * may not be visible to the HW prior to the completion of the UC + * register write and that we may begin execution from the context + * before its image is complete leading to invalid PD chasing. + */ + wmb(); return ce->lrc_desc; } From 44a7276b30c3c15f2b7790a5729640597fb6a1df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 12 Nov 2018 15:36:16 +0200 Subject: [PATCH 13/23] drm/i915: Fix hpd handling for pins with two encoders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In my haste to remove irq_port[] I accidentally changed the way we deal with hpd pins that are shared by multiple encoders (DP and HDMI for pre-DDI platforms). Previously we would only handle such pins via ->hpd_pulse(), but now we queue up the hotplug work for the HDMI encoder directly. Worse yet, we now count each hpd twice and this increment the hpd storm count twice as fast. This can lead to spurious storms being detected. Go back to the old way of doing things, ie. delegate to ->hpd_pulse() for any pin which has an encoder with that hook implemented. I don't really like the idea of adding irq_port[] back so let's loop through the encoders first to check if we have an encoder with ->hpd_pulse() for the pin, and then go through all the pins and decided on the correct course of action based on the earlier findings. I have occasionally toyed with the idea of unifying the pre-DDI HDMI and DP encoders into a single encoder as well. Besides the hotplug processing it would have the other benefit of preventing userspace from trying to enable both encoders at the same time. That is simply illegal as they share the same clock/data pins. We have some testcases that will attempt that and thus fail on many older machines. But for now let's stick to fixing just the hotplug code. Cc: stable@vger.kernel.org # 4.19+ Cc: Lyude Paul Cc: Rodrigo Vivi Fixes: b6ca3eee18ba ("drm/i915: Nuke dev_priv->irq_port[]") Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181108200424.28371-1-ville.syrjala@linux.intel.com Reviewed-by: Lyude Paul (cherry picked from commit 5a3aeca97af1b6b3498d59a7fd4e8bb95814c108) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_hotplug.c | 70 +++++++++++++++++++--------- 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index 8326900a311e..9a8018130237 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -397,37 +397,54 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, struct intel_encoder *encoder; bool storm_detected = false; bool queue_dig = false, queue_hp = false; + u32 long_hpd_pulse_mask = 0; + u32 short_hpd_pulse_mask = 0; + enum hpd_pin pin; if (!pin_mask) return; spin_lock(&dev_priv->irq_lock); - for_each_intel_encoder(&dev_priv->drm, encoder) { - enum hpd_pin pin = encoder->hpd_pin; - bool has_hpd_pulse = intel_encoder_has_hpd_pulse(encoder); + /* + * Determine whether ->hpd_pulse() exists for each pin, and + * whether we have a short or a long pulse. This is needed + * as each pin may have up to two encoders (HDMI and DP) and + * only the one of them (DP) will have ->hpd_pulse(). + */ + for_each_intel_encoder(&dev_priv->drm, encoder) { + bool has_hpd_pulse = intel_encoder_has_hpd_pulse(encoder); + enum port port = encoder->port; + bool long_hpd; + + pin = encoder->hpd_pin; if (!(BIT(pin) & pin_mask)) continue; - if (has_hpd_pulse) { - bool long_hpd = long_mask & BIT(pin); - enum port port = encoder->port; + if (!has_hpd_pulse) + continue; - DRM_DEBUG_DRIVER("digital hpd port %c - %s\n", port_name(port), - long_hpd ? "long" : "short"); - /* - * For long HPD pulses we want to have the digital queue happen, - * but we still want HPD storm detection to function. - */ - queue_dig = true; - if (long_hpd) { - dev_priv->hotplug.long_port_mask |= (1 << port); - } else { - /* for short HPD just trigger the digital queue */ - dev_priv->hotplug.short_port_mask |= (1 << port); - continue; - } + long_hpd = long_mask & BIT(pin); + + DRM_DEBUG_DRIVER("digital hpd port %c - %s\n", port_name(port), + long_hpd ? "long" : "short"); + queue_dig = true; + + if (long_hpd) { + long_hpd_pulse_mask |= BIT(pin); + dev_priv->hotplug.long_port_mask |= BIT(port); + } else { + short_hpd_pulse_mask |= BIT(pin); + dev_priv->hotplug.short_port_mask |= BIT(port); } + } + + /* Now process each pin just once */ + for_each_hpd_pin(pin) { + bool long_hpd; + + if (!(BIT(pin) & pin_mask)) + continue; if (dev_priv->hotplug.stats[pin].state == HPD_DISABLED) { /* @@ -444,11 +461,22 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, if (dev_priv->hotplug.stats[pin].state != HPD_ENABLED) continue; - if (!has_hpd_pulse) { + /* + * Delegate to ->hpd_pulse() if one of the encoders for this + * pin has it, otherwise let the hotplug_work deal with this + * pin directly. + */ + if (((short_hpd_pulse_mask | long_hpd_pulse_mask) & BIT(pin))) { + long_hpd = long_hpd_pulse_mask & BIT(pin); + } else { dev_priv->hotplug.event_bits |= BIT(pin); + long_hpd = true; queue_hp = true; } + if (!long_hpd) + continue; + if (intel_hpd_irq_storm_detect(dev_priv, pin)) { dev_priv->hotplug.event_bits &= ~BIT(pin); storm_detected = true; From 5581c670fb7ec267fc79215c6d5176b07e5f6dad Mon Sep 17 00:00:00 2001 From: shaoyunl Date: Mon, 12 Nov 2018 11:19:24 -0500 Subject: [PATCH 14/23] drm/amdgpu: set system aperture to cover whole FB region MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In XGMI configuration, the FB region covers vram region from peer device, adjust system aperture to cover all of them Reviewed-by: Alex Deucher Reviewed-by: Christian König Signed-off-by: shaoyunl Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 6 +++--- drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c index ceb7847b504f..bfa317ad20a9 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c @@ -72,7 +72,7 @@ static void gfxhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev) /* Program the system aperture low logical page number. */ WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR, - min(adev->gmc.vram_start, adev->gmc.agp_start) >> 18); + min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18); if (adev->asic_type == CHIP_RAVEN && adev->rev_id >= 0x8) /* @@ -82,11 +82,11 @@ static void gfxhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev) * to get rid of the VM fault and hardware hang. */ WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR, - max((adev->gmc.vram_end >> 18) + 0x1, + max((adev->gmc.fb_end >> 18) + 0x1, adev->gmc.agp_end >> 18)); else WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR, - max(adev->gmc.vram_end, adev->gmc.agp_end) >> 18); + max(adev->gmc.fb_end, adev->gmc.agp_end) >> 18); /* Set default page address. */ value = adev->vram_scratch.gpu_addr - adev->gmc.vram_start diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c index fd23ba1226a5..a0db67adc34c 100644 --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c @@ -90,7 +90,7 @@ static void mmhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev) /* Program the system aperture low logical page number. */ WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR, - min(adev->gmc.vram_start, adev->gmc.agp_start) >> 18); + min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18); if (adev->asic_type == CHIP_RAVEN && adev->rev_id >= 0x8) /* @@ -100,11 +100,11 @@ static void mmhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev) * to get rid of the VM fault and hardware hang. */ WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR, - max((adev->gmc.vram_end >> 18) + 0x1, + max((adev->gmc.fb_end >> 18) + 0x1, adev->gmc.agp_end >> 18)); else WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR, - max(adev->gmc.vram_end, adev->gmc.agp_end) >> 18); + max(adev->gmc.fb_end, adev->gmc.agp_end) >> 18); /* Set default page address. */ value = adev->vram_scratch.gpu_addr - adev->gmc.vram_start + From adf59dd2408c4536d490bee649784f0465562444 Mon Sep 17 00:00:00 2001 From: Jorge Ramirez-Ortiz Date: Mon, 12 Nov 2018 19:41:09 +0100 Subject: [PATCH 15/23] drm/meson: venc: dmt mode must use encp The video mode for DMT is only populated to support encp. Signed-off-by: Jorge Ramirez-Ortiz Acked-by: Neil Armstrong Signed-off-by: Neil Armstrong Link: https://patchwork.freedesktop.org/patch/msgid/1542048069-22603-1-git-send-email-jramirez@baylibre.com --- drivers/gpu/drm/meson/meson_venc.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c index 514245e69b38..acbbad3e322c 100644 --- a/drivers/gpu/drm/meson/meson_venc.c +++ b/drivers/gpu/drm/meson/meson_venc.c @@ -854,6 +854,13 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic, unsigned int sof_lines; unsigned int vsync_lines; + /* Use VENCI for 480i and 576i and double HDMI pixels */ + if (mode->flags & DRM_MODE_FLAG_DBLCLK) { + hdmi_repeat = true; + use_enci = true; + venc_hdmi_latency = 1; + } + if (meson_venc_hdmi_supported_vic(vic)) { vmode = meson_venc_hdmi_get_vic_vmode(vic); if (!vmode) { @@ -865,13 +872,7 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic, } else { meson_venc_hdmi_get_dmt_vmode(mode, &vmode_dmt); vmode = &vmode_dmt; - } - - /* Use VENCI for 480i and 576i and double HDMI pixels */ - if (mode->flags & DRM_MODE_FLAG_DBLCLK) { - hdmi_repeat = true; - use_enci = true; - venc_hdmi_latency = 1; + use_enci = false; } /* Repeat VENC pixels for 480/576i/p, 720p50/60 and 1080p50/60 */ From c837243ff4017f493c7d6f4ab57278d812a86859 Mon Sep 17 00:00:00 2001 From: Philip Yang Date: Mon, 12 Nov 2018 14:00:45 -0500 Subject: [PATCH 16/23] drm/amdgpu: fix bug with IH ring setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug limits the IH ring wptr address to 40bit. When the system memory is bigger than 1TB, the bus address is more than 40bit, this causes the interrupt cannot be handled and cleared correctly. Reviewed-by: Christian König Signed-off-by: Philip Yang Reviewed-by: Alex Deucher Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c index a99f71797aa3..a0fda6f9252a 100644 --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c @@ -129,7 +129,7 @@ static int vega10_ih_irq_init(struct amdgpu_device *adev) else wptr_off = adev->wb.gpu_addr + (adev->irq.ih.wptr_offs * 4); WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_ADDR_LO, lower_32_bits(wptr_off)); - WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_ADDR_HI, upper_32_bits(wptr_off) & 0xFF); + WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_ADDR_HI, upper_32_bits(wptr_off) & 0xFFFF); /* set rptr, wptr to 0 */ WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR, 0); From 4d454e9ffdb1ef5a51ebc147b5389c96048db683 Mon Sep 17 00:00:00 2001 From: Rex Zhu Date: Tue, 13 Nov 2018 11:15:56 +0800 Subject: [PATCH 17/23] drm/amd/pp: Fix truncated clock value when set watermark the clk value should be tranferred to MHz first and then transfer to uint16. otherwise, the clock value will be truncated. Reviewed-by: Alex Deucher Reported-by: Hersen Wu Signed-off-by: Rex Zhu Signed-off-by: Alex Deucher Cc: stable@vger.kernel.org --- .../gpu/drm/amd/powerplay/hwmgr/smu_helper.c | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c index 99a33c33a32c..101c09b212ad 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c @@ -713,20 +713,20 @@ int smu_set_watermarks_for_clocks_ranges(void *wt_table, for (i = 0; i < wm_with_clock_ranges->num_wm_dmif_sets; i++) { table->WatermarkRow[1][i].MinClock = cpu_to_le16((uint16_t) - (wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_dcfclk_clk_in_khz) / - 1000); + (wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_dcfclk_clk_in_khz / + 1000)); table->WatermarkRow[1][i].MaxClock = cpu_to_le16((uint16_t) - (wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_dcfclk_clk_in_khz) / - 1000); + (wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_dcfclk_clk_in_khz / + 1000)); table->WatermarkRow[1][i].MinUclk = cpu_to_le16((uint16_t) - (wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_mem_clk_in_khz) / - 1000); + (wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_mem_clk_in_khz / + 1000)); table->WatermarkRow[1][i].MaxUclk = cpu_to_le16((uint16_t) - (wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_mem_clk_in_khz) / - 1000); + (wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_mem_clk_in_khz / + 1000)); table->WatermarkRow[1][i].WmSetting = (uint8_t) wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_set_id; } @@ -734,20 +734,20 @@ int smu_set_watermarks_for_clocks_ranges(void *wt_table, for (i = 0; i < wm_with_clock_ranges->num_wm_mcif_sets; i++) { table->WatermarkRow[0][i].MinClock = cpu_to_le16((uint16_t) - (wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_socclk_clk_in_khz) / - 1000); + (wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_socclk_clk_in_khz / + 1000)); table->WatermarkRow[0][i].MaxClock = cpu_to_le16((uint16_t) - (wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_socclk_clk_in_khz) / - 1000); + (wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_socclk_clk_in_khz / + 1000)); table->WatermarkRow[0][i].MinUclk = cpu_to_le16((uint16_t) - (wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_mem_clk_in_khz) / - 1000); + (wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_mem_clk_in_khz / + 1000)); table->WatermarkRow[0][i].MaxUclk = cpu_to_le16((uint16_t) - (wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_mem_clk_in_khz) / - 1000); + (wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_mem_clk_in_khz / + 1000)); table->WatermarkRow[0][i].WmSetting = (uint8_t) wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_set_id; } From c1a17777eb45d9f3821f35e9869c0a08cd2e664e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Mon, 12 Nov 2018 18:08:31 +0100 Subject: [PATCH 18/23] drm/amdgpu: fix huge page handling on Vega10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We accidentially set the huge flag on the parent instead of the childs. This caused some VM faults under memory pressure. Signed-off-by: Christian König Acked-by: Alex Deucher Tested-by: Samuel Pitoiset Reviewed-by: Felix Kuehling Signed-off-by: Alex Deucher Cc: stable@vger.kernel.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 352b30409060..dad0e2342df9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1632,13 +1632,6 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params, continue; } - /* First check if the entry is already handled */ - if (cursor.pfn < frag_start) { - cursor.entry->huge = true; - amdgpu_vm_pt_next(adev, &cursor); - continue; - } - /* If it isn't already handled it can't be a huge page */ if (cursor.entry->huge) { /* Add the entry to the relocated list to update it. */ @@ -1701,8 +1694,17 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params, } } while (frag_start < entry_end); - if (frag >= shift) + if (amdgpu_vm_pt_descendant(adev, &cursor)) { + /* Mark all child entries as huge */ + while (cursor.pfn < frag_start) { + cursor.entry->huge = true; + amdgpu_vm_pt_next(adev, &cursor); + } + + } else if (frag >= shift) { + /* or just move on to the next on the same level. */ amdgpu_vm_pt_next(adev, &cursor); + } } return 0; From 6a67a20366f894c172734f28c5646bdbe48a46e3 Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Mon, 12 Nov 2018 12:39:31 +0000 Subject: [PATCH 19/23] drm/i915: fix broadwell EU computation subslice_mask is an array indexed by slice, not subslice. Signed-off-by: Lionel Landwerlin Fixes: 8cc7669355136f ("drm/i915: store all subslice masks") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108712 Reviewed-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20181112123931.2815-1-lionel.g.landwerlin@intel.com (cherry picked from commit 63ac3328f0d1d37f286e397b14d9596ed09d7ca5) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_device_info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 0ef0c6448d53..01fa98299bae 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -474,7 +474,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv) u8 eu_disabled_mask; u32 n_disabled; - if (!(sseu->subslice_mask[ss] & BIT(ss))) + if (!(sseu->subslice_mask[s] & BIT(ss))) /* skip disabled subslice */ continue; From a22612301ae61d78a7c0c82dc556931a35db0e91 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Fri, 9 Nov 2018 16:09:23 +0200 Subject: [PATCH 20/23] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update Register DBUF_CTL_S2 is read and it's value is not used. As there is no explanation why we should prime the hardware with read, remove it as spurious. Fixes: aa9664ffe863 ("drm/i915/icl: Enable 2nd DBuf slice only when needed") Cc: Mahesh Kumar Cc: Rodrigo Vivi Signed-off-by: Mika Kuoppala Reviewed-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20181109140924.2663-1-mika.kuoppala@linux.intel.com (cherry picked from commit 8577c319b6511fbc391f3775225fecd8b979bc26) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 0a4990d8843c..44e4491a4918 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3176,8 +3176,7 @@ static u8 intel_dbuf_max_slices(struct drm_i915_private *dev_priv) void icl_dbuf_slices_update(struct drm_i915_private *dev_priv, u8 req_slices) { - u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices; - u32 val; + const u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices; bool ret; if (req_slices > intel_dbuf_max_slices(dev_priv)) { @@ -3188,7 +3187,6 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv, if (req_slices == hw_enabled_slices || req_slices == 0) return; - val = I915_READ(DBUF_CTL_S2); if (req_slices > hw_enabled_slices) ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, true); else From 2a2777990a342b05f611e78822fc4fa2d9789ade Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 14 Nov 2018 13:49:23 +0200 Subject: [PATCH 21/23] drm/i915: Move programming plane scaler to its own function. This cleans the code up slightly, and will make other changes easier. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/20180920102711.4184-8-maarten.lankhorst@linux.intel.com (cherry picked from commit ab5c60bf76755d24ae8de5c1c6ac594934656ace) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_sprite.c | 90 +++++++++++++++++------------ 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 5fd2f7bf3927..873adcc20109 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -302,13 +302,63 @@ skl_plane_max_stride(struct intel_plane *plane, return min(8192 * cpp, 32768); } +static void +skl_program_scaler(struct drm_i915_private *dev_priv, + struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state) +{ + enum plane_id plane_id = plane->id; + enum pipe pipe = plane->pipe; + int scaler_id = plane_state->scaler_id; + const struct intel_scaler *scaler = + &crtc_state->scaler_state.scalers[scaler_id]; + int crtc_x = plane_state->base.dst.x1; + int crtc_y = plane_state->base.dst.y1; + uint32_t crtc_w = drm_rect_width(&plane_state->base.dst); + uint32_t crtc_h = drm_rect_height(&plane_state->base.dst); + u16 y_hphase, uv_rgb_hphase; + u16 y_vphase, uv_rgb_vphase; + + /* Sizes are 0 based */ + crtc_w--; + crtc_h--; + + /* TODO: handle sub-pixel coordinates */ + if (plane_state->base.fb->format->format == DRM_FORMAT_NV12) { + y_hphase = skl_scaler_calc_phase(1, false); + y_vphase = skl_scaler_calc_phase(1, false); + + /* MPEG2 chroma siting convention */ + uv_rgb_hphase = skl_scaler_calc_phase(2, true); + uv_rgb_vphase = skl_scaler_calc_phase(2, false); + } else { + /* not used */ + y_hphase = 0; + y_vphase = 0; + + uv_rgb_hphase = skl_scaler_calc_phase(1, false); + uv_rgb_vphase = skl_scaler_calc_phase(1, false); + } + + I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id), + PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode); + I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0); + I915_WRITE_FW(SKL_PS_VPHASE(pipe, scaler_id), + PS_Y_PHASE(y_vphase) | PS_UV_RGB_PHASE(uv_rgb_vphase)); + I915_WRITE_FW(SKL_PS_HPHASE(pipe, scaler_id), + PS_Y_PHASE(y_hphase) | PS_UV_RGB_PHASE(uv_rgb_hphase)); + I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y); + I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), + ((crtc_w + 1) << 16)|(crtc_h + 1)); +} + void skl_update_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); - const struct drm_framebuffer *fb = plane_state->base.fb; enum plane_id plane_id = plane->id; enum pipe pipe = plane->pipe; u32 plane_ctl = plane_state->ctl; @@ -318,8 +368,6 @@ skl_update_plane(struct intel_plane *plane, u32 aux_stride = skl_plane_stride(plane_state, 1); int crtc_x = plane_state->base.dst.x1; int crtc_y = plane_state->base.dst.y1; - uint32_t crtc_w = drm_rect_width(&plane_state->base.dst); - uint32_t crtc_h = drm_rect_height(&plane_state->base.dst); uint32_t x = plane_state->color_plane[0].x; uint32_t y = plane_state->color_plane[0].y; uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; @@ -329,8 +377,6 @@ skl_update_plane(struct intel_plane *plane, /* Sizes are 0 based */ src_w--; src_h--; - crtc_w--; - crtc_h--; spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); @@ -355,39 +401,7 @@ skl_update_plane(struct intel_plane *plane, /* program plane scaler */ if (plane_state->scaler_id >= 0) { - int scaler_id = plane_state->scaler_id; - const struct intel_scaler *scaler = - &crtc_state->scaler_state.scalers[scaler_id]; - u16 y_hphase, uv_rgb_hphase; - u16 y_vphase, uv_rgb_vphase; - - /* TODO: handle sub-pixel coordinates */ - if (fb->format->format == DRM_FORMAT_NV12) { - y_hphase = skl_scaler_calc_phase(1, false); - y_vphase = skl_scaler_calc_phase(1, false); - - /* MPEG2 chroma siting convention */ - uv_rgb_hphase = skl_scaler_calc_phase(2, true); - uv_rgb_vphase = skl_scaler_calc_phase(2, false); - } else { - /* not used */ - y_hphase = 0; - y_vphase = 0; - - uv_rgb_hphase = skl_scaler_calc_phase(1, false); - uv_rgb_vphase = skl_scaler_calc_phase(1, false); - } - - I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id), - PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode); - I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0); - I915_WRITE_FW(SKL_PS_VPHASE(pipe, scaler_id), - PS_Y_PHASE(y_vphase) | PS_UV_RGB_PHASE(uv_rgb_vphase)); - I915_WRITE_FW(SKL_PS_HPHASE(pipe, scaler_id), - PS_Y_PHASE(y_hphase) | PS_UV_RGB_PHASE(uv_rgb_hphase)); - I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y); - I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), - ((crtc_w + 1) << 16)|(crtc_h + 1)); + skl_program_scaler(dev_priv, plane, crtc_state, plane_state); I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0); } else { From 27971d613fcb5b6ad96320bc4f2c90f4d4fde768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 14 Nov 2018 13:49:24 +0200 Subject: [PATCH 22/23] drm/i915: Clean up skl_program_scaler() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the "sizes are 0 based" stuff that is not even true for the scaler. v2: Rebase Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181101151736.20522-1-ville.syrjala@linux.intel.com Reviewed-by: Rodrigo Vivi (cherry picked from commit d0105af939769393d6447a04cee2d1ae12e3f09a) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_sprite.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 873adcc20109..fa7eaace5f92 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -303,12 +303,11 @@ skl_plane_max_stride(struct intel_plane *plane, } static void -skl_program_scaler(struct drm_i915_private *dev_priv, - struct intel_plane *plane, +skl_program_scaler(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { - enum plane_id plane_id = plane->id; + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; int scaler_id = plane_state->scaler_id; const struct intel_scaler *scaler = @@ -320,10 +319,6 @@ skl_program_scaler(struct drm_i915_private *dev_priv, u16 y_hphase, uv_rgb_hphase; u16 y_vphase, uv_rgb_vphase; - /* Sizes are 0 based */ - crtc_w--; - crtc_h--; - /* TODO: handle sub-pixel coordinates */ if (plane_state->base.fb->format->format == DRM_FORMAT_NV12) { y_hphase = skl_scaler_calc_phase(1, false); @@ -342,15 +337,14 @@ skl_program_scaler(struct drm_i915_private *dev_priv, } I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id), - PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode); + PS_SCALER_EN | PS_PLANE_SEL(plane->id) | scaler->mode); I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0); I915_WRITE_FW(SKL_PS_VPHASE(pipe, scaler_id), PS_Y_PHASE(y_vphase) | PS_UV_RGB_PHASE(uv_rgb_vphase)); I915_WRITE_FW(SKL_PS_HPHASE(pipe, scaler_id), PS_Y_PHASE(y_hphase) | PS_UV_RGB_PHASE(uv_rgb_hphase)); I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y); - I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), - ((crtc_w + 1) << 16)|(crtc_h + 1)); + I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), (crtc_w << 16) | crtc_h); } void @@ -399,9 +393,8 @@ skl_update_plane(struct intel_plane *plane, (plane_state->color_plane[1].y << 16) | plane_state->color_plane[1].x); - /* program plane scaler */ if (plane_state->scaler_id >= 0) { - skl_program_scaler(dev_priv, plane, crtc_state, plane_state); + skl_program_scaler(plane, crtc_state, plane_state); I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0); } else { From 6e8adf6f4a4fa57dd3bef6b70de96e2b7b311204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 14 Nov 2018 15:32:55 +0200 Subject: [PATCH 23/23] drm/i915: Account for scale factor when calculating initial phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To get the initial phase correct we need to account for the scale factor as well. I forgot this initially and was mostly looking at heavily upscaled content where the minor difference between -0.5 and the proper initial phase was not readily apparent. And let's toss in a comment that tries to explain the formula a little bit. v2: The initial phase upper limit is 1.5, not 24.0! Cc: Maarten Lankhorst Fixes: 0a59952b24e2 ("drm/i915: Configure SKL+ scaler initial phase correctly") Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181029181820.21956-1-ville.syrjala@linux.intel.com Tested-by: Juha-Pekka Heikkila Tested-by: Maarten Lankhorst #irc Reviewed-by: Maarten Lankhorst #irc (cherry picked from commit e7a278a329dd8aa2c70c564849f164cb5673689c) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_display.c | 45 ++++++++++++++++++++++++++-- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_sprite.c | 20 +++++++++---- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 23d8008a93bb..a54843fdeb2f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4850,8 +4850,31 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe) * chroma samples for both of the luma samples, and thus we don't * actually get the expected MPEG2 chroma siting convention :( * The same behaviour is observed on pre-SKL platforms as well. + * + * Theory behind the formula (note that we ignore sub-pixel + * source coordinates): + * s = source sample position + * d = destination sample position + * + * Downscaling 4:1: + * -0.5 + * | 0.0 + * | | 1.5 (initial phase) + * | | | + * v v v + * | s | s | s | s | + * | d | + * + * Upscaling 1:4: + * -0.5 + * | -0.375 (initial phase) + * | | 0.0 + * | | | + * v v v + * | s | + * | d | d | d | d | */ -u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) +u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_cosited) { int phase = -0x8000; u16 trip = 0; @@ -4859,6 +4882,15 @@ u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) if (chroma_cosited) phase += (sub - 1) * 0x8000 / sub; + phase += scale / (2 * sub); + + /* + * Hardware initial phase limited to [-0.5:1.5]. + * Since the max hardware scale factor is 3.0, we + * should never actually excdeed 1.0 here. + */ + WARN_ON(phase < -0x8000 || phase > 0x18000); + if (phase < 0) phase = 0x10000 + phase; else @@ -5067,13 +5099,20 @@ static void skylake_pfit_enable(struct intel_crtc *crtc) if (crtc->config->pch_pfit.enabled) { u16 uv_rgb_hphase, uv_rgb_vphase; + int pfit_w, pfit_h, hscale, vscale; int id; if (WARN_ON(crtc->config->scaler_state.scaler_id < 0)) return; - uv_rgb_hphase = skl_scaler_calc_phase(1, false); - uv_rgb_vphase = skl_scaler_calc_phase(1, false); + pfit_w = (crtc->config->pch_pfit.size >> 16) & 0xFFFF; + pfit_h = crtc->config->pch_pfit.size & 0xFFFF; + + hscale = (crtc->config->pipe_src_w << 16) / pfit_w; + vscale = (crtc->config->pipe_src_h << 16) / pfit_h; + + uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false); + uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false); id = scaler_state->scaler_id; I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN | diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f8dc84b2d2d3..8b298e5f012d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1646,7 +1646,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state); -u16 skl_scaler_calc_phase(int sub, bool chroma_center); +u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center); int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state); int skl_max_scale(const struct intel_crtc_state *crtc_state, u32 pixel_format); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index fa7eaace5f92..d3090a7537bb 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -318,22 +318,30 @@ skl_program_scaler(struct intel_plane *plane, uint32_t crtc_h = drm_rect_height(&plane_state->base.dst); u16 y_hphase, uv_rgb_hphase; u16 y_vphase, uv_rgb_vphase; + int hscale, vscale; + + hscale = drm_rect_calc_hscale(&plane_state->base.src, + &plane_state->base.dst, + 0, INT_MAX); + vscale = drm_rect_calc_vscale(&plane_state->base.src, + &plane_state->base.dst, + 0, INT_MAX); /* TODO: handle sub-pixel coordinates */ if (plane_state->base.fb->format->format == DRM_FORMAT_NV12) { - y_hphase = skl_scaler_calc_phase(1, false); - y_vphase = skl_scaler_calc_phase(1, false); + y_hphase = skl_scaler_calc_phase(1, hscale, false); + y_vphase = skl_scaler_calc_phase(1, vscale, false); /* MPEG2 chroma siting convention */ - uv_rgb_hphase = skl_scaler_calc_phase(2, true); - uv_rgb_vphase = skl_scaler_calc_phase(2, false); + uv_rgb_hphase = skl_scaler_calc_phase(2, hscale, true); + uv_rgb_vphase = skl_scaler_calc_phase(2, vscale, false); } else { /* not used */ y_hphase = 0; y_vphase = 0; - uv_rgb_hphase = skl_scaler_calc_phase(1, false); - uv_rgb_vphase = skl_scaler_calc_phase(1, false); + uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false); + uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false); } I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id),