All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH][next] PCI: imx6: Use fallthrough pseudo-keyword
From: Bjorn Helgaas @ 2020-07-16 22:06 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Richard Zhu, Lucas Stach, Lorenzo Pieralisi, Rob Herring,
	Bjorn Helgaas, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, linux-pci, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20200716211052.GA16893@embeddedor>

On Thu, Jul 16, 2020 at 04:10:52PM -0500, Gustavo A. R. Silva wrote:
> Replace the existing /* fall through */ comments and its variants with
> the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
> fall-through markings when it is the case.
> 
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through

Hi Gustavo,

I'm certainly fine with these patches, and thanks for doing them!

And thanks for providing a link to the rationale.  But the URL
contains "latest", so I think it may break if deprecated.rst or the
section is ever renamed.

I think I would prefer if we could reference the current text, e.g.,
via

  https://www.kernel.org/doc/html/v5.7-rc7/process/deprecated.html#implicit-switch-case-fall-through

(The v5.7 doc would be better but doesn't seem to be generated yet; I
pinged the helpdesk about that.)

Or we could refer to b9918bdcac1f ("Documentation/process: Add
fallthrough pseudo-keyword"), although it's not nearly as pretty as
the HTML.

> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 4e5c379ae418..1119ded593d0 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -439,7 +439,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
>  		break;
> -	case IMX6QP:		/* FALLTHROUGH */
> +	case IMX6QP:
>  	case IMX6Q:
>  		/* power up core phy and enable ref clock */
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> @@ -642,7 +642,7 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_2);
> -		/* FALLTHROUGH */
> +		fallthrough;
>  	default:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> @@ -1107,7 +1107,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  			dev_err(dev, "pcie_aux clock source missing or invalid\n");
>  			return PTR_ERR(imx6_pcie->pcie_aux);
>  		}
> -		/* fall through */
> +		fallthrough;
>  	case IMX7D:
>  		if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
>  			imx6_pcie->controller_id = 1;
> -- 
> 2.27.0
> 

^ permalink raw reply

* Re: [PATCH v3 09/12] ppc64/kexec_file: setup backup region for kdump kernel
From: Thiago Jung Bauermann @ 2020-07-16 22:06 UTC (permalink / raw)
  To: Hari Bathini
  Cc: kernel test robot, Pingfan Liu, Petr Tesarik, Nayna Jain,
	Kexec-ml, Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev,
	Sourabh Jain, Andrew Morton, Dave Young, Vivek Goyal,
	Eric Biederman
In-Reply-To: <bea19627-c6b7-5d59-e194-03038bb4d9f6@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 16/07/20 7:08 am, Thiago Jung Bauermann wrote:
>>
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>>
>>> @@ -968,7 +1040,7 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
>>>
>>>  	/*
>>>  	 * Restrict memory usage for kdump kernel by setting up
>>> -	 * usable memory ranges.
>>> +	 * usable memory ranges and memory reserve map.
>>>  	 */
>>>  	if (image->type == KEXEC_TYPE_CRASH) {
>>>  		ret = get_usable_memory_ranges(&umem);
>>> @@ -980,6 +1052,24 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
>>>  			pr_err("Error setting up usable-memory property for kdump kernel\n");
>>>  			goto out;
>>>  		}
>>> +
>>> +		ret = fdt_add_mem_rsv(fdt, BACKUP_SRC_START + BACKUP_SRC_SIZE,
>>> +				      crashk_res.start - BACKUP_SRC_SIZE);
>>
>> I believe this answers my question from the other email about how the
>> crashkernel is prevented from stomping in the crashed kernel's memory,
>> right? I needed to think for a bit to understand what the above
>> reservation was protecting. I think it's worth adding a comment.
>
> Right. The reason to add it in the first place is, prom presses the panic button if
> it can't find low memory. Marking it reserved seems to keep it quiet though. so..
>
> Will add comment mentioning that..

Ah, makes sense. Thanks for the explanation.

>>> +void purgatory(void)
>>> +{
>>> +	void *dest, *src;
>>> +
>>> +	src = (void *)BACKUP_SRC_START;
>>> +	if (backup_start) {
>>> +		dest = (void *)backup_start;
>>> +		__memcpy(dest, src, BACKUP_SRC_SIZE);
>>> +	}
>>> +}
>>
>> In general I'm in favor of using C code over assembly, but having to
>> bring in that relocation support just for the above makes me wonder if
>> it's worth it in this case.
>
> I am planning to build on purgatory later with "I'm in purgatory" print support
> for pseries at least and also, sha256 digest check.

Ok. In that case, my preference would be to convert both the powerpc and
x86 purgatories to PIE since this greatly reduces the types of
relocations that are emitted, but better ask Dave Young what he thinks
before going down that route.

--
Thiago Jung Bauermann
IBM Linux Technology Center

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply

* Re: db57e98d87 ("mm/sparse.c: fix ALIGN() without power of 2 in .."): BUG: kernel reboot-without-warning in early-boot stage, last printk: early console in setup code
From: Andrew Morton @ 2020-07-16 22:06 UTC (permalink / raw)
  To: kernel test robot
  Cc: Lecopzer Chen, Mark-PK Tsai, Linux Memory Management List,
	linux-kernel, LKP
In-Reply-To: <20200716083159.GH3874@shao2-debian>

On Thu, 16 Jul 2020 16:32:00 +0800 kernel test robot <lkp@intel.com> wrote:

> Greetings,
> 
> 0day kernel testing robot got the below dmesg and the first bad commit is
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> 
> commit db57e98d87908b8837352abe08515e42752270c1
> Author:     Lecopzer Chen <lecopzer.chen@mediatek.com>
> AuthorDate: Mon Sep 23 15:36:24 2019 -0700
> Commit:     Linus Torvalds <torvalds@linux-foundation.org>
> CommitDate: Tue Sep 24 15:54:09 2019 -0700
> 
>     mm/sparse.c: fix ALIGN() without power of 2 in sparse_buffer_alloc()

Are we sure about this?  That patch is a year old - has something
in the test setup changed to make it visible at this late stage?



^ permalink raw reply

* [Intel-gfx] [PATCH v8 5/5] drm/i915/rkl: Add Wa_14011224835 for PHY B initialization
From: Matt Roper @ 2020-07-16 22:05 UTC (permalink / raw)
  To: intel-gfx
In-Reply-To: <20200716220551.2730644-1-matthew.d.roper@intel.com>

After doing normal PHY-B initialization on Rocket Lake, we need to
manually copy some additional PHY-A register values into PHY-B
registers.

Note that the bspec's combo phy page doesn't specify that this
workaround is restricted to specific platform steppings (and doesn't
even do a very good job of specifying that RKL is the only platform this
is needed on), but the RKL workaround page lists this as relevant only
for A and B steppings, so I'm trusting that information for now.

v2:  Make rkl_combo_phy_b_init_wa() static

v3:
 - Minimize variables in WA function. (Jose)
 - Fix timeout duration (usec vs msec). (Jose)
 - Add verification of workaround. (Jose)
 - Fix stepping bounds in comment.

Bspec: 49291
Bspec: 53273
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 .../gpu/drm/i915/display/intel_combo_phy.c    | 50 +++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h               | 13 ++++-
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c
index eccaa79cb4a9..d88f91038428 100644
--- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
@@ -255,6 +255,26 @@ static bool phy_is_master(struct drm_i915_private *dev_priv, enum phy phy)
 	return phy == PHY_A;
 }
 
+static bool verify_wa14011224835(struct drm_i915_private *i915)
+{
+	u32 grccode, val;
+	bool ret = true;
+
+	grccode = REG_FIELD_GET(GRCCODE,
+				intel_de_read(i915, ICL_PORT_COMP_DW6(PHY_A)));
+	val = REG_FIELD_PREP(IREF_RCAL_ORD, grccode);
+	ret &= check_phy_reg(i915, PHY_B, ICL_PORT_COMP_DW2(PHY_B),
+			     IREF_RCAL_ORD, val);
+
+	grccode = REG_FIELD_GET(GRCCODE_LDO,
+				intel_de_read(i915, ICL_PORT_COMP_DW0(PHY_A)));
+	val = REG_FIELD_PREP(RCOMPCODE_LD_CAP_OV, grccode);
+	ret &= check_phy_reg(i915, PHY_B, ICL_PORT_COMP_DW2(PHY_B),
+			     IREF_RCAL_ORD, val);
+
+	return ret;
+}
+
 static bool icl_combo_phy_verify_state(struct drm_i915_private *dev_priv,
 				       enum phy phy)
 {
@@ -295,6 +315,11 @@ static bool icl_combo_phy_verify_state(struct drm_i915_private *dev_priv,
 	ret &= check_phy_reg(dev_priv, phy, ICL_PORT_CL_DW5(phy),
 			     CL_POWER_DOWN_ENABLE, CL_POWER_DOWN_ENABLE);
 
+	/* Wa_14011224835:rkl[a0..b0] */
+	if (IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_B0) &&
+	    phy == PHY_B)
+		ret &= verify_wa14011224835(dev_priv);
+
 	return ret;
 }
 
@@ -350,6 +375,26 @@ void intel_combo_phy_power_up_lanes(struct drm_i915_private *dev_priv,
 	intel_de_write(dev_priv, ICL_PORT_CL_DW10(phy), val);
 }
 
+static void rkl_combo_phy_b_init_wa(struct drm_i915_private *i915)
+{
+	u32 grccode, val;
+
+	wait_for_us(intel_de_read(i915, ICL_PORT_COMP_DW3(PHY_A)) &
+		    FIRST_COMP_DONE, 100);
+
+	grccode = REG_FIELD_GET(GRCCODE,
+				intel_de_read(i915, ICL_PORT_COMP_DW6(PHY_A)));
+	val = REG_FIELD_PREP(IREF_RCAL_ORD, grccode);
+	intel_de_rmw(i915, ICL_PORT_COMP_DW2(PHY_B), IREF_RCAL_ORD,
+		     val | IREF_RCAL_ORD_EN);
+
+	grccode = REG_FIELD_GET(GRCCODE_LDO,
+				intel_de_read(i915, ICL_PORT_COMP_DW0(PHY_A)));
+	val = REG_FIELD_PREP(RCOMPCODE_LD_CAP_OV, grccode);
+	intel_de_rmw(i915, ICL_PORT_COMP_DW6(PHY_B), RCOMPCODE_LD_CAP_OV,
+		     val | RCOMPCODEOVEN_LDO_SYNC);
+}
+
 static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
 {
 	enum phy phy;
@@ -415,6 +460,11 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
 		val = intel_de_read(dev_priv, ICL_PORT_CL_DW5(phy));
 		val |= CL_POWER_DOWN_ENABLE;
 		intel_de_write(dev_priv, ICL_PORT_CL_DW5(phy), val);
+
+		if (IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_B0) &&
+		    phy == PHY_B)
+			/* Wa_14011224835:rkl[a0..b0] */
+			rkl_combo_phy_b_init_wa(dev_priv);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 89a9f2d8110e..a0d31f3bf634 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1911,11 +1911,16 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 
 #define CNL_PORT_COMP_DW0		_MMIO(0x162100)
 #define ICL_PORT_COMP_DW0(phy)		_MMIO(_ICL_PORT_COMP_DW(0, phy))
-#define   COMP_INIT			(1 << 31)
+#define   COMP_INIT			REG_BIT(31)
+#define   GRCCODE_LDO			REG_GENMASK(7, 0)
 
 #define CNL_PORT_COMP_DW1		_MMIO(0x162104)
 #define ICL_PORT_COMP_DW1(phy)		_MMIO(_ICL_PORT_COMP_DW(1, phy))
 
+#define ICL_PORT_COMP_DW2(phy)		_MMIO(_ICL_PORT_COMP_DW(2, phy))
+#define   IREF_RCAL_ORD_EN		REG_BIT(7)
+#define   IREF_RCAL_ORD			REG_GENMASK(6, 0)
+
 #define CNL_PORT_COMP_DW3		_MMIO(0x16210c)
 #define ICL_PORT_COMP_DW3(phy)		_MMIO(_ICL_PORT_COMP_DW(3, phy))
 #define   PROCESS_INFO_DOT_0		(0 << 26)
@@ -1928,6 +1933,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define   VOLTAGE_INFO_1_05V		(2 << 24)
 #define   VOLTAGE_INFO_MASK		(3 << 24)
 #define   VOLTAGE_INFO_SHIFT		24
+#define   FIRST_COMP_DONE		REG_BIT(22)
+
+#define ICL_PORT_COMP_DW6(phy)		_MMIO(_ICL_PORT_COMP_DW(6, phy))
+#define   GRCCODE			REG_GENMASK(30, 24)
+#define   RCOMPCODEOVEN_LDO_SYNC	REG_BIT(23)
+#define   RCOMPCODE_LD_CAP_OV		REG_GENMASK(22, 16)
 
 #define ICL_PORT_COMP_DW8(phy)		_MMIO(_ICL_PORT_COMP_DW(8, phy))
 #define   IREFGEN			(1 << 24)
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [Intel-gfx] [PATCH v8 4/5] drm/i915/rkl: Handle HTI
From: Matt Roper @ 2020-07-16 22:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi
In-Reply-To: <20200716220551.2730644-1-matthew.d.roper@intel.com>

If HTI (also sometimes called HDPORT) is enabled at startup, it may be
using some of the PHYs and DPLLs making them unavailable for general
usage.  Let's read out the HDPORT_STATE register and avoid making use of
resources that HTI is already using.

v2:
 - Fix minor checkpatch warnings

v3:
 - Just readout HDPORT_STATE register once during init and then parse it
   later as needed.
 - Add a 'has_hti' device info flag to track whether we should readout
   HDPORT_STATE or not.  We can skip the platform/flag tests later since
   the hti_state in dev_priv will remain 0 for platforms it does not
   apply to.
 - Move PLL masking into icl_get_combo_phy_dpll() since at the moment
   RKL is the only platform that has HTI.  (Jose)

Bspec: 49189
Bspec: 53707
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      | 19 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.c  |  8 ++++++++
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 11 +++++++++++
 drivers/gpu/drm/i915/i915_drv.h               |  8 ++++++++
 drivers/gpu/drm/i915/i915_pci.c               |  1 +
 drivers/gpu/drm/i915/i915_reg.h               |  6 ++++++
 drivers/gpu/drm/i915/intel_device_info.h      |  1 +
 7 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 1ca70f9abc8d..714b2bc96f23 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4923,6 +4923,13 @@ intel_ddi_max_lanes(struct intel_digital_port *dig_port)
 	return max_lanes;
 }
 
+static bool hti_uses_phy(struct drm_i915_private *i915, enum phy phy)
+{
+	return i915->hti_state & HDPORT_ENABLED &&
+		(i915->hti_state & HDPORT_PHY_USED_DP(phy) ||
+		 i915->hti_state & HDPORT_PHY_USED_HDMI(phy));
+}
+
 void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 {
 	struct intel_digital_port *dig_port;
@@ -4930,6 +4937,18 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	bool init_hdmi, init_dp, init_lspcon = false;
 	enum phy phy = intel_port_to_phy(dev_priv, port);
 
+	/*
+	 * On platforms with HTI (aka HDPORT), if it's enabled at boot it may
+	 * have taken over some of the PHYs and made them unavailable to the
+	 * driver.  In that case we should skip initializing the corresponding
+	 * outputs.
+	 */
+	if (hti_uses_phy(dev_priv, phy)) {
+		drm_dbg_kms(&dev_priv->drm, "PORT %c / PHY %c reserved by HTI\n",
+			    port_name(port), phy_name(phy));
+		return;
+	}
+
 	init_hdmi = intel_bios_port_supports_dvi(dev_priv, port) ||
 		intel_bios_port_supports_hdmi(dev_priv, port);
 	init_dp = intel_bios_port_supports_dp(dev_priv, port);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6cb66580ad2c..db2a5a1a9b35 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -47,6 +47,7 @@
 #include "display/intel_ddi.h"
 #include "display/intel_dp.h"
 #include "display/intel_dp_mst.h"
+#include "display/intel_dpll_mgr.h"
 #include "display/intel_dsi.h"
 #include "display/intel_dvo.h"
 #include "display/intel_gmbus.h"
@@ -17903,6 +17904,13 @@ int intel_modeset_init(struct drm_i915_private *i915)
 	if (i915->max_cdclk_freq == 0)
 		intel_update_max_cdclk(i915);
 
+	/*
+	 * If the platform has HTI, we need to find out whether it has reserved
+	 * any display resources before we create our display outputs.
+	 */
+	if (INTEL_INFO(i915)->display.has_hti)
+		i915->hti_state = intel_de_read(i915, HDPORT_STATE);
+
 	/* Just disable it once at startup */
 	intel_vga_disable(i915);
 	intel_setup_outputs(i915);
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 134c2ecf4c80..81ab975fe4f0 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3475,6 +3475,14 @@ static void icl_update_active_dpll(struct intel_atomic_state *state,
 	icl_set_active_port_dpll(crtc_state, port_dpll_id);
 }
 
+static u32 intel_get_hti_plls(struct drm_i915_private *i915)
+{
+	if (!(i915->hti_state & HDPORT_ENABLED))
+		return 0;
+
+	return REG_FIELD_GET(HDPORT_DPLL_USED_MASK, i915->hti_state);
+}
+
 static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
 				   struct intel_crtc *crtc,
 				   struct intel_encoder *encoder)
@@ -3518,6 +3526,9 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
 		dpll_mask = BIT(DPLL_ID_ICL_DPLL1) | BIT(DPLL_ID_ICL_DPLL0);
 	}
 
+	/* Eliminate DPLLs from consideration if reserved by HTI */
+	dpll_mask &= ~intel_get_hti_plls(dev_priv);
+
 	port_dpll->pll = intel_find_shared_dpll(state, crtc,
 						&port_dpll->hw_state,
 						dpll_mask);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e4f7f6518945..56dfc6d98caa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1044,6 +1044,14 @@ struct drm_i915_private {
 
 	struct intel_l3_parity l3_parity;
 
+	/*
+	 * HTI (aka HDPORT) state read during initial hw readout.  Most
+	 * platforms don't have HTI, so this will just stay 0.  Those that do
+	 * will use this later to figure out which PLLs and PHYs are unavailable
+	 * for driver usage.
+	 */
+	u32 hti_state;
+
 	/*
 	 * edram size in MB.
 	 * Cannot be determined by PCIID. You must always read a register.
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 2338f92ce490..366ddfc8df6b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -890,6 +890,7 @@ static const struct intel_device_info rkl_info = {
 	.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
 		BIT(TRANSCODER_C),
 	.require_force_probe = 1,
+	.display.has_hti = 1,
 	.display.has_psr_hw_tracking = 0,
 	.platform_engine_mask =
 		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0),
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index bfdb6d23b5d8..89a9f2d8110e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2921,6 +2921,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define MBUS_BBOX_CTL_S1		_MMIO(0x45040)
 #define MBUS_BBOX_CTL_S2		_MMIO(0x45044)
 
+#define HDPORT_STATE			_MMIO(0x45050)
+#define   HDPORT_DPLL_USED_MASK		REG_GENMASK(14, 12)
+#define   HDPORT_PHY_USED_DP(phy)	REG_BIT(2 * (phy) + 2)
+#define   HDPORT_PHY_USED_HDMI(phy)	REG_BIT(2 * (phy) + 1)
+#define   HDPORT_ENABLED		REG_BIT(0)
+
 /* Make render/texture TLB fetches lower priorty than associated data
  *   fetches. This is not turned on by default
  */
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index fd2385457ab6..6a3d607218aa 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -146,6 +146,7 @@ enum intel_ppgtt_type {
 	func(has_gmch); \
 	func(has_hdcp); \
 	func(has_hotplug); \
+	func(has_hti); \
 	func(has_ipc); \
 	func(has_modular_fia); \
 	func(has_overlay); \
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* Re: [PATCH 3/4] repair: use fs root ino for dummy parent value instead of zero
From: Dave Chinner @ 2020-07-16 22:06 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs
In-Reply-To: <20200716104103.GB26218@bfoster>

On Thu, Jul 16, 2020 at 06:41:03AM -0400, Brian Foster wrote:
> On Thu, Jul 16, 2020 at 08:22:16AM +1000, Dave Chinner wrote:
> > On Wed, Jul 15, 2020 at 10:08:35AM -0400, Brian Foster wrote:
> > > If a directory inode has an invalid parent ino on disk, repair
> > > replaces the invalid value with a dummy value of zero in the buffer
> > > and NULLFSINO in the in-core parent tracking. The zero value serves
> > > no functional purpose as it is still an invalid value and the parent
> > > must be repaired by phase 6 based on the in-core state before the
> > > buffer can be written out.  Instead, use the root fs inode number as
> > > a catch all for invalid parent values so phase 6 doesn't have to
> > > create custom verifier infrastructure just to work around this
> > > behavior.
> > > 
> > > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > 
> > Reasonale, but wouldn't it be better to use lost+found as the dummy
> > parent inode (i.e. the orphanage inode)? Because if the parent can't
> > be found and the inode reconnected correctly, we're going to put it
> > in lost+found, anyway?
> > 
> 
> That was my first thought when I originally wrote this, but there's
> several reasons I didn't end up doing that. The orphanage isn't created
> until much later in repair and only if we end up with orphaned inodes.
> We'd have to change that in order to use a dummy parent inode number
> that corresponds to a valid orphanage, and TBH I'm not even sure if it's
> always going to be safe to expect an inode allocation to work at this
> point in repair.
> 
> Further, it's still too early to tell whether these directories are
> orphaned because the directory scan in phase 6 can easily repair
> missing/broken parent information. The scenarios I used to test this
> functionality didn't involve the orphanage at all, so now we not only
> need to change when/how the orphanage is created, but need to free it if
> it ends up unused before we exit (which could be via any number of
> do_error() calls before we ever get close to phase 6).

Fair enough - can you please capture all this in the commit message
to preserve the explanation of why the root inode was chosen and
not lost+found?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply

* [Intel-gfx] [PATCH v8 3/5] drm/i915/rkl: Add DPLL4 support
From: Matt Roper @ 2020-07-16 22:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi
In-Reply-To: <20200716220551.2730644-1-matthew.d.roper@intel.com>

Rocket Lake has a third DPLL (called 'DPLL4') that must be used to
enable a third display.  Unlike EHL's variant of DPLL4, the RKL variant
behaves the same as DPLL0/1.  And despite its name, the DPLL4 registers
are offset as if it were DPLL2.

v2:
 - Add new .update_ref_clks() hook.

v3:
 - Renumber TBT PLL to '3' and switch _MMIO_PLL3 to _MMIO_PLL (Lucas)

v4:
 - Don't drop _MMIO_PLL3; although it's now unused, we're going to need
   it very soon again for upcoming DG1 patches.  (Lucas)

v5:
 - Don't re-number TBT PLL and beyond, just use new RKL_DPLL_CFGCR
   macros to lookup the proper registers instead.  Although renumbering
   the PLLs might be something we want to consider down the road, it
   opens a big can of worms right now since a bunch of places in the
   code have an assumption that the PLL table has idx==id and no holes.
   Renumbering creates a hole for TGL, so we'd either need to allow
   holes in the table or break the idx==id invariant, both of which are
   somewhat invasive changes to the design.

Bspec: 49202
Bspec: 49443
Bspec: 50288
Bspec: 50289
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 41 ++++++++++++++++---
 drivers/gpu/drm/i915/i915_reg.h               |  6 ++-
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index aeb6ee395cce..134c2ecf4c80 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3504,13 +3504,19 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
 
 	icl_calc_dpll_state(dev_priv, &pll_params, &port_dpll->hw_state);
 
-	if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
+	if (IS_ROCKETLAKE(dev_priv)) {
 		dpll_mask =
 			BIT(DPLL_ID_EHL_DPLL4) |
 			BIT(DPLL_ID_ICL_DPLL1) |
 			BIT(DPLL_ID_ICL_DPLL0);
-	else
+	} else if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A) {
+		dpll_mask =
+			BIT(DPLL_ID_EHL_DPLL4) |
+			BIT(DPLL_ID_ICL_DPLL1) |
+			BIT(DPLL_ID_ICL_DPLL0);
+	} else {
 		dpll_mask = BIT(DPLL_ID_ICL_DPLL1) | BIT(DPLL_ID_ICL_DPLL0);
+	}
 
 	port_dpll->pll = intel_find_shared_dpll(state, crtc,
 						&port_dpll->hw_state,
@@ -3791,7 +3797,12 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	if (!(val & PLL_ENABLE))
 		goto out;
 
-	if (INTEL_GEN(dev_priv) >= 12) {
+	if (IS_ROCKETLAKE(dev_priv)) {
+		hw_state->cfgcr0 = intel_de_read(dev_priv,
+						 RKL_DPLL_CFGCR0(id));
+		hw_state->cfgcr1 = intel_de_read(dev_priv,
+						 RKL_DPLL_CFGCR1(id));
+	} else if (INTEL_GEN(dev_priv) >= 12) {
 		hw_state->cfgcr0 = intel_de_read(dev_priv,
 						 TGL_DPLL_CFGCR0(id));
 		hw_state->cfgcr1 = intel_de_read(dev_priv,
@@ -3844,7 +3855,10 @@ static void icl_dpll_write(struct drm_i915_private *dev_priv,
 	const enum intel_dpll_id id = pll->info->id;
 	i915_reg_t cfgcr0_reg, cfgcr1_reg;
 
-	if (INTEL_GEN(dev_priv) >= 12) {
+	if (IS_ROCKETLAKE(dev_priv)) {
+		cfgcr0_reg = RKL_DPLL_CFGCR0(id);
+		cfgcr1_reg = RKL_DPLL_CFGCR1(id);
+	} else if (INTEL_GEN(dev_priv) >= 12) {
 		cfgcr0_reg = TGL_DPLL_CFGCR0(id);
 		cfgcr1_reg = TGL_DPLL_CFGCR1(id);
 	} else {
@@ -4276,6 +4290,21 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
 	.dump_hw_state = icl_dump_hw_state,
 };
 
+static const struct dpll_info rkl_plls[] = {
+	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
+	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
+	{ "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
+	{ },
+};
+
+static const struct intel_dpll_mgr rkl_pll_mgr = {
+	.dpll_info = rkl_plls,
+	.get_dplls = icl_get_dplls,
+	.put_dplls = icl_put_dplls,
+	.update_ref_clks = icl_update_dpll_ref_clks,
+	.dump_hw_state = icl_dump_hw_state,
+};
+
 /**
  * intel_shared_dpll_init - Initialize shared DPLLs
  * @dev: drm device
@@ -4289,7 +4318,9 @@ void intel_shared_dpll_init(struct drm_device *dev)
 	const struct dpll_info *dpll_info;
 	int i;
 
-	if (INTEL_GEN(dev_priv) >= 12)
+	if (IS_ROCKETLAKE(dev_priv))
+		dpll_mgr = &rkl_pll_mgr;
+	else if (INTEL_GEN(dev_priv) >= 12)
 		dpll_mgr = &tgl_pll_mgr;
 	else if (IS_ELKHARTLAKE(dev_priv))
 		dpll_mgr = &ehl_pll_mgr;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00414c457941..bfdb6d23b5d8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10511,19 +10511,21 @@ enum skl_power_gate {
 
 #define _TGL_DPLL0_CFGCR0		0x164284
 #define _TGL_DPLL1_CFGCR0		0x16428C
-/* TODO: add DPLL4 */
 #define _TGL_TBTPLL_CFGCR0		0x16429C
 #define TGL_DPLL_CFGCR0(pll)		_MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
 						  _TGL_DPLL1_CFGCR0, \
 						  _TGL_TBTPLL_CFGCR0)
+#define RKL_DPLL_CFGCR0(pll)		_MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, \
+						  _TGL_DPLL1_CFGCR0)
 
 #define _TGL_DPLL0_CFGCR1		0x164288
 #define _TGL_DPLL1_CFGCR1		0x164290
-/* TODO: add DPLL4 */
 #define _TGL_TBTPLL_CFGCR1		0x1642A0
 #define TGL_DPLL_CFGCR1(pll)		_MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
 						   _TGL_DPLL1_CFGCR1, \
 						   _TGL_TBTPLL_CFGCR1)
+#define RKL_DPLL_CFGCR1(pll)		_MMIO_PLL(pll, _TGL_DPLL0_CFGCR1, \
+						  _TGL_DPLL1_CFGCR1)
 
 #define _DKL_PHY1_BASE			0x168000
 #define _DKL_PHY2_BASE			0x169000
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [Intel-gfx] [PATCH v8 2/5] drm/i915/rkl: Add initial workarounds
From: Matt Roper @ 2020-07-16 22:05 UTC (permalink / raw)
  To: intel-gfx
In-Reply-To: <20200716220551.2730644-1-matthew.d.roper@intel.com>

RKL and TGL share some general gen12 workarounds, but each platform also
has its own platform-specific workarounds.

v2:
 - Add Wa_1604555607 for RKL.  This makes RKL's ctx WA list identical to
   TGL's, so we'll have both functions call the tgl_ function for now;
   this workaround isn't listed for DG1 so we don't want to add it to
   the general gen12_ function.

Cc: Matt Atwood <matthew.s.atwood@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_sprite.c |  5 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 89 +++++++++++++--------
 2 files changed, 60 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index d03860fef2d7..c26ca029fc0a 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -2843,8 +2843,9 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
 static bool gen12_plane_supports_mc_ccs(struct drm_i915_private *dev_priv,
 					enum plane_id plane_id)
 {
-	/* Wa_14010477008:tgl[a0..c0] */
-	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
+	/* Wa_14010477008:tgl[a0..c0],rkl[all] */
+	if (IS_ROCKETLAKE(dev_priv) ||
+	    IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
 		return false;
 
 	return plane_id < PLANE_SPRITE4;
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 5726cd0a37e0..cef1c122696f 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -596,8 +596,8 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
 	wa_masked_en(wal, GEN9_ROW_CHICKEN4, GEN11_DIS_PICK_2ND_EU);
 }
 
-static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
-				     struct i915_wa_list *wal)
+static void gen12_ctx_workarounds_init(struct intel_engine_cs *engine,
+				       struct i915_wa_list *wal)
 {
 	/*
 	 * Wa_1409142259:tgl
@@ -607,12 +607,28 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
 	 * Wa_1409207793:tgl
 	 * Wa_1409178076:tgl
 	 * Wa_1408979724:tgl
+	 * Wa_14010443199:rkl
+	 * Wa_14010698770:rkl
 	 */
 	WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,
 			  GEN12_DISABLE_CPS_AWARE_COLOR_PIPE);
 
+	/* WaDisableGPGPUMidThreadPreemption:gen12 */
+	WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1,
+			    GEN9_PREEMPT_GPGPU_LEVEL_MASK,
+			    GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
+}
+
+static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
+				     struct i915_wa_list *wal)
+{
+	gen12_ctx_workarounds_init(engine, wal);
+
 	/*
-	 * Wa_1604555607:gen12 and Wa_1608008084:gen12
+	 * Wa_1604555607:tgl,rkl
+	 *
+	 * Note that the implementation of this workaround is further modified
+	 * according to the FF_MODE2 guidance given by Wa_1608008084:gen12.
 	 * FF_MODE2 register will return the wrong value when read. The default
 	 * value for this register is zero for all fields and there are no bit
 	 * masks. So instead of doing a RMW we should just write the GS Timer
@@ -623,11 +639,6 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
 	       FF_MODE2_GS_TIMER_MASK | FF_MODE2_TDS_TIMER_MASK,
 	       FF_MODE2_GS_TIMER_224  | FF_MODE2_TDS_TIMER_128,
 	       0);
-
-	/* WaDisableGPGPUMidThreadPreemption:tgl */
-	WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1,
-			    GEN9_PREEMPT_GPGPU_LEVEL_MASK,
-			    GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
 }
 
 static void
@@ -642,8 +653,10 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
 
 	wa_init_start(wal, name, engine->name);
 
-	if (IS_GEN(i915, 12))
+	if (IS_ROCKETLAKE(i915) || IS_TIGERLAKE(i915))
 		tgl_ctx_workarounds_init(engine, wal);
+	else if (IS_GEN(i915, 12))
+		gen12_ctx_workarounds_init(engine, wal);
 	else if (IS_GEN(i915, 11))
 		icl_ctx_workarounds_init(engine, wal);
 	else if (IS_CANNONLAKE(i915))
@@ -1176,9 +1189,16 @@ icl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 }
 
 static void
-tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
+gen12_gt_workarounds_init(struct drm_i915_private *i915,
+			  struct i915_wa_list *wal)
 {
 	wa_init_mcr(i915, wal);
+}
+
+static void
+tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
+{
+	gen12_gt_workarounds_init(i915, wal);
 
 	/* Wa_1409420604:tgl */
 	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
@@ -1196,8 +1216,10 @@ tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 static void
 gt_init_workarounds(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
-	if (IS_GEN(i915, 12))
+	if (IS_TIGERLAKE(i915))
 		tgl_gt_workarounds_init(i915, wal);
+	else if (IS_GEN(i915, 12))
+		gen12_gt_workarounds_init(i915, wal);
 	else if (IS_GEN(i915, 11))
 		icl_gt_workarounds_init(i915, wal);
 	else if (IS_CANNONLAKE(i915))
@@ -1629,18 +1651,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			    GEN9_CTX_PREEMPT_REG,
 			    GEN12_DISABLE_POSH_BUSY_FF_DOP_CG);
 
-		/*
-		 * Wa_1607030317:tgl
-		 * Wa_1607186500:tgl
-		 * Wa_1607297627:tgl there is 3 entries for this WA on BSpec, 2
-		 * of then says it is fixed on B0 the other one says it is
-		 * permanent
-		 */
-		wa_masked_en(wal,
-			     GEN6_RC_SLEEP_PSMI_CONTROL,
-			     GEN12_WAIT_FOR_EVENT_POWER_DOWN_DISABLE |
-			     GEN8_RC_SEMA_IDLE_MSG_DISABLE);
-
 		/*
 		 * Wa_1606679103:tgl
 		 * (see also Wa_1606682166:icl)
@@ -1654,22 +1664,17 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			    VSUNIT_CLKGATE_DIS_TGL);
 	}
 
-	if (IS_TIGERLAKE(i915)) {
-		/* Wa_1606931601:tgl */
+	if (IS_ROCKETLAKE(i915) || IS_TIGERLAKE(i915)) {
+		/* Wa_1606931601:tgl,rkl */
 		wa_masked_en(wal, GEN7_ROW_CHICKEN2, GEN12_DISABLE_EARLY_READ);
 
-		/* Wa_1409804808:tgl */
+		/* Wa_1409804808:tgl,rkl */
 		wa_masked_en(wal, GEN7_ROW_CHICKEN2,
 			     GEN12_PUSH_CONST_DEREF_HOLD_DIS);
 
-		/* Wa_1606700617:tgl */
-		wa_masked_en(wal,
-			     GEN9_CS_DEBUG_MODE1,
-			     FF_DOP_CLOCK_GATE_DISABLE);
-
 		/*
 		 * Wa_1409085225:tgl
-		 * Wa_14010229206:tgl
+		 * Wa_14010229206:tgl,rkl
 		 */
 		wa_masked_en(wal, GEN9_ROW_CHICKEN4, GEN12_DISABLE_TDL_PUSH);
 
@@ -1677,9 +1682,29 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 		 * Wa_1407928979:tgl A*
 		 * Wa_18011464164:tgl B0+
 		 * Wa_22010931296:tgl B0+
+		 * Wa_14010919138:rkl
 		 */
 		wa_write_or(wal, GEN7_FF_THREAD_MODE,
 			    GEN12_FF_TESSELATION_DOP_GATE_DISABLE);
+
+		/*
+		 * Wa_1607030317:tgl
+		 * Wa_1607186500:tgl
+		 * Wa_1607297627:tgl,rkl there are multiple entries for this
+		 * WA in the BSpec; some indicate this is an A0-only WA,
+		 * others indicate it applies to all steppings.
+		 */
+		wa_masked_en(wal,
+			     GEN6_RC_SLEEP_PSMI_CONTROL,
+			     GEN12_WAIT_FOR_EVENT_POWER_DOWN_DISABLE |
+			     GEN8_RC_SEMA_IDLE_MSG_DISABLE);
+	}
+
+	if (IS_TIGERLAKE(i915)) {
+		/* Wa_1606700617:tgl */
+		wa_masked_en(wal,
+			     GEN9_CS_DEBUG_MODE1,
+			     FF_DOP_CLOCK_GATE_DISABLE);
 	}
 
 	if (IS_GEN(i915, 11)) {
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [Intel-gfx] [PATCH v8 1/5] drm/i915/rkl: Handle new DPCLKA_CFGCR0 layout
From: Matt Roper @ 2020-07-16 22:05 UTC (permalink / raw)
  To: intel-gfx
In-Reply-To: <20200716220551.2730644-1-matthew.d.roper@intel.com>

RKL uses a slightly different bit layout for the DPCLKA_CFGCR0 register.

v2:
 - Fix inverted mask application when updating ICL_DPCLKA_CFGCR0
 - Checkpatch style fixes

Bspec: 50287
Cc: Aditya Swarup <aditya.swarup@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     | 18 +++++++++++++++---
 drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++++---
 drivers/gpu/drm/i915/i915_reg.h              |  6 ++++++
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index c52ad5ecb645..1ca70f9abc8d 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2845,7 +2845,9 @@ hsw_set_signal_levels(struct intel_dp *intel_dp)
 static u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv,
 				     enum phy phy)
 {
-	if (intel_phy_is_combo(dev_priv, phy)) {
+	if (IS_ROCKETLAKE(dev_priv)) {
+		return RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
+	} else if (intel_phy_is_combo(dev_priv, phy)) {
 		return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
 	} else if (intel_phy_is_tc(dev_priv, phy)) {
 		enum tc_port tc_port = intel_port_to_tc(dev_priv,
@@ -2872,6 +2874,16 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder,
 		    (val & icl_dpclka_cfgcr0_clk_off(dev_priv, phy)) == 0);
 
 	if (intel_phy_is_combo(dev_priv, phy)) {
+		u32 mask, sel;
+
+		if (IS_ROCKETLAKE(dev_priv)) {
+			mask = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
+			sel = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
+		} else {
+			mask = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
+			sel = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
+		}
+
 		/*
 		 * Even though this register references DDIs, note that we
 		 * want to pass the PHY rather than the port (DDI).  For
@@ -2882,8 +2894,8 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder,
 		 *   Clock Select chooses the PLL for both DDIA and DDID and
 		 *   drives port A in all cases."
 		 */
-		val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
-		val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy);
+		val &= ~mask;
+		val |= sel;
 		intel_de_write(dev_priv, ICL_DPCLKA_CFGCR0, val);
 		intel_de_posting_read(dev_priv, ICL_DPCLKA_CFGCR0);
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 729ec6e0d43a..6cb66580ad2c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10802,9 +10802,18 @@ static void icl_get_ddi_pll(struct drm_i915_private *dev_priv, enum port port,
 	u32 temp;
 
 	if (intel_phy_is_combo(dev_priv, phy)) {
-		temp = intel_de_read(dev_priv, ICL_DPCLKA_CFGCR0) &
-			ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
-		id = temp >> ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
+		u32 mask, shift;
+
+		if (IS_ROCKETLAKE(dev_priv)) {
+			mask = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
+			shift = RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
+		} else {
+			mask = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
+			shift = ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
+		}
+
+		temp = intel_de_read(dev_priv, ICL_DPCLKA_CFGCR0) & mask;
+		id = temp >> shift;
 		port_dpll_id = ICL_PORT_DPLL_DEFAULT;
 	} else if (intel_phy_is_tc(dev_priv, phy)) {
 		u32 clk_sel = intel_de_read(dev_priv, DDI_CLK_SEL(port)) & DDI_CLK_SEL_MASK;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b9607ac3620d..00414c457941 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10279,12 +10279,18 @@ enum skl_power_gate {
 
 #define ICL_DPCLKA_CFGCR0			_MMIO(0x164280)
 #define  ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy)	(1 << _PICK(phy, 10, 11, 24))
+#define  RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy)	REG_BIT((phy) + 10)
 #define  ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port)	(1 << ((tc_port) < PORT_TC4 ? \
 						       (tc_port) + 12 : \
 						       (tc_port) - PORT_TC4 + 21))
 #define  ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)	((phy) * 2)
 #define  ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy)	(3 << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
 #define  ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy)	((pll) << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
+#define  RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)	_PICK(phy, 0, 2, 4, 27)
+#define  RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) \
+	(3 << RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
+#define  RKL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) \
+	((pll) << RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
 
 /* CNL PLL */
 #define DPLL0_ENABLE		0x46010
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [Intel-gfx] [PATCH v8 0/5] Remaining RKL patches
From: Matt Roper @ 2020-07-16 22:05 UTC (permalink / raw)
  To: intel-gfx

The first couple patches here already have r-b's from Jose, but since
it's been a while since they were last sent to the list we should get
another CI pass before merging them.

Changes since v7:
 - Undo the renumbering of PLL IDs in the DPLL4 patch; the shared DPLL
   code has deep-rooted assumptions that the PLL table has no holes and
   that id==idx; renumbering works fine for RKL, but breaks one of those
   assumptions for TGL.  Instead introduce RKL-specific RKL_DPLL_CFGCR
   macros to look up the proper register with the existing PLL IDs.
 - Incorporate Jose's review feedback on the HTI patch and combo PHY WA
   patch.

Cc: José Roberto de Souza <jose.souza@intel.com>

Matt Roper (5):
  drm/i915/rkl: Handle new DPCLKA_CFGCR0 layout
  drm/i915/rkl: Add initial workarounds
  drm/i915/rkl: Add DPLL4 support
  drm/i915/rkl: Handle HTI
  drm/i915/rkl: Add Wa_14011224835 for PHY B initialization

 .../gpu/drm/i915/display/intel_combo_phy.c    | 50 +++++++++++
 drivers/gpu/drm/i915/display/intel_ddi.c      | 37 +++++++-
 drivers/gpu/drm/i915/display/intel_display.c  | 23 ++++-
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 52 +++++++++--
 drivers/gpu/drm/i915/display/intel_sprite.c   |  5 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c   | 89 ++++++++++++-------
 drivers/gpu/drm/i915/i915_drv.h               |  8 ++
 drivers/gpu/drm/i915/i915_pci.c               |  1 +
 drivers/gpu/drm/i915/i915_reg.h               | 31 ++++++-
 drivers/gpu/drm/i915/intel_device_info.h      |  1 +
 10 files changed, 249 insertions(+), 48 deletions(-)

-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [dpdk-dev] [PATCH v4 25/27] eal: mark old naming as deprecated
From: Stephen Hemminger @ 2020-07-16 22:05 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev
In-Reply-To: <2e34fe08-6548-e474-a7bc-f10e4bef947a@intel.com>

On Thu, 16 Jul 2020 14:41:41 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> On 15-Jul-20 9:29 PM, Stephen Hemminger wrote:
> > On Wed, 15 Jul 2020 14:28:17 +0100
> > "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
> >   
> >>> -#define SKIP_MASTER	SKIP_INITIAL
> >>> -#define CALL_MASTER	CALL_INITIAL
> >>> +#define SKIP_MASTER _Pragma("GCC warning \"'SKIP_MASTER' is deprecated\"") SKIP_INITIAL
> >>> +#define CALL_MASTER _Pragma("GCC warning \"'CALL_MASTER' is deprecated\"") CALL_INITIAL  
> >>
> >> Presumably this is a generic header, should we introduce GCC-specific
> >> things there?  
> > 
> > It works with Clang as well. Likely ICC but don't have that.
> >   
> 
> What about MSVC?
> 



The compiler issues a warning when it finds a pragma that it doesn't recognize, and continues compilation.

^ permalink raw reply

* [PATCH ethtool] Fix segfault with cable test and ./configure --disable-netlink
From: Andrew Lunn @ 2020-07-16 22:05 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy, Andrew Lunn

When the netlink interface code is disabled, a stub version of
netlink_run_handler() is used. This stub version needs to handle the
case when there is no possibility for a command to fall back to the
IOCTL call. The two cable tests commands have no such fallback, and if
we don't handle this, ethtool tries to jump through a NULL pointer
resulting in a segfault.

Reported-by: Chris Healy <cphealy@gmail.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 netlink/extapi.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/netlink/extapi.h b/netlink/extapi.h
index c5bfde9..a35d5f2 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -46,6 +46,12 @@ void nl_monitor_usage(void);
 static inline void netlink_run_handler(struct cmd_context *ctx,
 				       nl_func_t nlfunc, bool no_fallback)
 {
+	if (no_fallback) {
+		fprintf(stderr,
+			"Command requires kernel netlink support which is not "
+			"enabled in this ethtool binary\n");
+		exit(1);
+	}
 }
 
 static inline int nl_monitor(struct cmd_context *ctx)
-- 
2.27.0


^ permalink raw reply related

* Re: [PATCH v3 05/12] powerpc/drmem: make lmb walk a bit more flexible
From: Thiago Jung Bauermann @ 2020-07-16 22:01 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Petr Tesarik, Nayna Jain, Kexec-ml,
	Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <30e8f02a-f009-70a5-01e9-dec9eff213b1@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 15/07/20 9:20 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>> 
>>> @@ -534,7 +537,7 @@ static int __init early_init_dt_scan_memory_ppc(unsigned long node,
>>>  #ifdef CONFIG_PPC_PSERIES
>>>  	if (depth == 1 &&
>>>  	    strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) {
>>> -		walk_drmem_lmbs_early(node, early_init_drmem_lmb);
>>> +		walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb);
>> 
>> walk_drmem_lmbs_early() can now fail. Should this failure be propagated
>> as a return value of early_init_dt_scan_memory_ppc()?
>   
>> 
>>>  		return 0;
>>>  	}
>>>  #endif
>> <snip>
>> 
>>> @@ -787,7 +790,7 @@ static int __init parse_numa_properties(void)
>>>  	 */
>>>  	memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>>>  	if (memory) {
>>> -		walk_drmem_lmbs(memory, numa_setup_drmem_lmb);
>>> +		walk_drmem_lmbs(memory, NULL, numa_setup_drmem_lmb);
>> 
>> Similarly here. Now that this call can fail, should
>> parse_numa_properties() handle or propagate the failure?
>
> They would still not fail unless the callbacks early_init_drmem_lmb() & numa_setup_drmem_lmb()
> are updated to have failure scenarios. Also, these call sites always ignored failure scenarios
> even before walk_drmem_lmbs() was introduced. So, I prefer to keep them the way they are?

Ok, makes sense. In this case:

Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* Re: [dpdk-dev] [PATCH v4 25/27] eal: mark old naming as deprecated
From: Stephen Hemminger @ 2020-07-16 22:04 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev
In-Reply-To: <2e34fe08-6548-e474-a7bc-f10e4bef947a@intel.com>

On Thu, 16 Jul 2020 14:41:41 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> On 15-Jul-20 9:29 PM, Stephen Hemminger wrote:
> > On Wed, 15 Jul 2020 14:28:17 +0100
> > "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
> >   
> >>> -#define SKIP_MASTER	SKIP_INITIAL
> >>> -#define CALL_MASTER	CALL_INITIAL
> >>> +#define SKIP_MASTER _Pragma("GCC warning \"'SKIP_MASTER' is deprecated\"") SKIP_INITIAL
> >>> +#define CALL_MASTER _Pragma("GCC warning \"'CALL_MASTER' is deprecated\"") CALL_INITIAL  
> >>
> >> Presumably this is a generic header, should we introduce GCC-specific
> >> things there?  
> > 
> > It works with Clang as well. Likely ICC but don't have that.
> >   
> 
> What about MSVC?
> 

_Pragma is C99 standard so MSVC know it.
MSVC should ignore any pragman it doesn't understand.

There is a better pragma for deprecating keywords in MSVC, but GCC and Clang don't
understand it.

^ permalink raw reply

* Re: [PATCH] mm: Warn mmput() from memory reclaim context.
From: Tetsuo Handa @ 2020-07-16 22:04 UTC (permalink / raw)
  To: David Rientjes; +Cc: Andrew Morton, linux-mm
In-Reply-To: <alpine.DEB.2.23.453.2007161242370.3086260@chino.kir.corp.google.com>

On 2020/07/17 4:45, David Rientjes wrote:
> I wasn't familiar with this particular report, but it seems like the fix 
> is simply to do the kzalloc() before taking delayed_uprobe_lock and 
> freeing it if delayed_uprobe_check() already finds one for that uprobe?

The fix will be to use mmput_async()
( https://lkml.kernel.org/r/20200716162931.g3delsp7qmfjup6x@wittgenstein ).
We didn't call mmput() from the OOM reaper context. I think the reason is that
__mmput() might do something more complicated which blocks reclaiming memory.



^ permalink raw reply

* Re: [PATCH v2 4/5] LSM: Define SELinux function to measure security state
From: Lakshmi Ramasubramanian @ 2020-07-16 22:03 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
	SElinux list, LSM List, linux-kernel
In-Reply-To: <CAEjxPJ5p_T+C1NDz3iF7fvQzQAURpAcipvQfQXLZTfLP4Wiqbg@mail.gmail.com>

On 7/16/20 12:45 PM, Stephen Smalley wrote:
> On Thu, Jul 16, 2020 at 3:13 PM Lakshmi Ramasubramanian
> <nramas@linux.microsoft.com> wrote:
>>
>> On 7/16/20 11:54 AM, Stephen Smalley wrote:
>>> Not sure about this error handling approach (silent, proceeding as if
>>> the length was zero and then later failing with ENOMEM on every
>>> attempt?). I'd be more inclined to panic/BUG here but I know Linus
>>> doesn't like that.
>> I am not sure if failing (kernel panic/BUG) to "measure" LSM data under
>> memory pressure conditions is the right thing. But I am open to treating
>> this error as a fatal error. Please let me know.
> 
> Let's at least log an error message since it otherwise silently
> disables all measuring of security state.
Agree - will log error messages as appropriate.

> Also not sure why we bother returning errors from
> selinux_measure_data() since nothing appears to check or use the
> result.
Maybe SELinux can log audit messages on failures, but I guess it may be 
better to do that closer to where the error occurs.

Will change selinux_measure_data() to void function.

> Don't know if integrity/IMA has any equivalent to the audit
> subsystem's concept of audit_failure settings to control whether
> errors that prevent auditing (measuring) are handled silently, with a
> log message, or via a panic.  If not, I guess that can be explored
> separately.
> 

Yes - integrity subsystem logs audit messages for errors\failures.

  -lakshmi



^ permalink raw reply

* Re: [PATCH v3 06/12] ppc64/kexec_file: restrict memory usage of kdump kernel
From: Thiago Jung Bauermann @ 2020-07-16 22:03 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Nayna Jain, Kexec-ml, Mahesh J Salgaonkar,
	Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain, Petr Tesarik,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <baa29ea9-7698-a7e8-e5a4-c9f842e1fcc8@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 16/07/20 4:22 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>> 
>
> <snip>
>
>>> +/**
>>> + * get_node_path - Get the full path of the given node.
>>> + * @dn:            Node.
>>> + * @path:          Updated with the full path of the node.
>>> + *
>>> + * Returns nothing.
>>> + */
>>> +static void get_node_path(struct device_node *dn, char *path)
>>> +{
>>> +	if (!dn)
>>> +		return;
>>> +
>>> +	get_node_path(dn->parent, path);
>> 
>> Is it ok to do recursion in the kernel? In this case I believe it's not
>> problematic since the maximum call depth will be the maximum depth of a
>> device tree node which shouldn't be too much. Also, there are no local
>> variables in this function. But I thought it was worth mentioning.
>
> You are right. We are better off avoiding the recursion here. Will
> change it to an iterative version instead.

Ok.

>>> +	 * each representing a memory range.
>>> +	 */
>>> +	ranges = (len >> 2) / (n_mem_addr_cells + n_mem_size_cells);
>>> +
>>> +	for (i = 0; i < ranges; i++) {
>>> +		base = of_read_number(prop, n_mem_addr_cells);
>>> +		prop += n_mem_addr_cells;
>>> +		end = base + of_read_number(prop, n_mem_size_cells) - 1;
>
> prop is not used after the above.
>
>> You need to `prop += n_mem_size_cells` here.
>
> But yeah, adding it would make it look complete in some sense..

Isn't it used in the next iteration of the loop?

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH v3 06/12] ppc64/kexec_file: restrict memory usage of kdump kernel
From: Thiago Jung Bauermann @ 2020-07-16 22:03 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Petr Tesarik, Nayna Jain, Kexec-ml,
	Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <baa29ea9-7698-a7e8-e5a4-c9f842e1fcc8@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 16/07/20 4:22 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>> 
>
> <snip>
>
>>> +/**
>>> + * get_node_path - Get the full path of the given node.
>>> + * @dn:            Node.
>>> + * @path:          Updated with the full path of the node.
>>> + *
>>> + * Returns nothing.
>>> + */
>>> +static void get_node_path(struct device_node *dn, char *path)
>>> +{
>>> +	if (!dn)
>>> +		return;
>>> +
>>> +	get_node_path(dn->parent, path);
>> 
>> Is it ok to do recursion in the kernel? In this case I believe it's not
>> problematic since the maximum call depth will be the maximum depth of a
>> device tree node which shouldn't be too much. Also, there are no local
>> variables in this function. But I thought it was worth mentioning.
>
> You are right. We are better off avoiding the recursion here. Will
> change it to an iterative version instead.

Ok.

>>> +	 * each representing a memory range.
>>> +	 */
>>> +	ranges = (len >> 2) / (n_mem_addr_cells + n_mem_size_cells);
>>> +
>>> +	for (i = 0; i < ranges; i++) {
>>> +		base = of_read_number(prop, n_mem_addr_cells);
>>> +		prop += n_mem_addr_cells;
>>> +		end = base + of_read_number(prop, n_mem_size_cells) - 1;
>
> prop is not used after the above.
>
>> You need to `prop += n_mem_size_cells` here.
>
> But yeah, adding it would make it look complete in some sense..

Isn't it used in the next iteration of the loop?

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply

* Re: [PATCH v3 04/12] ppc64/kexec_file: avoid stomping memory used by special regions
From: Thiago Jung Bauermann @ 2020-07-16 21:59 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Petr Tesarik, Nayna Jain, Kexec-ml,
	Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <0582476e-415e-3f60-2bb2-6199d0340156@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 15/07/20 8:09 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>> 
>
> <snip>
>  
>>> +/**
>>> + * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
>>> + *                              in the memory regions between buf_min & buf_max
>>> + *                              for the buffer. If found, sets kbuf->mem.
>>> + * @kbuf:                       Buffer contents and memory parameters.
>>> + * @buf_min:                    Minimum address for the buffer.
>>> + * @buf_max:                    Maximum address for the buffer.
>>> + *
>>> + * Returns 0 on success, negative errno on error.
>>> + */
>>> +static int __locate_mem_hole_top_down(struct kexec_buf *kbuf,
>>> +				      u64 buf_min, u64 buf_max)
>>> +{
>>> +	int ret = -EADDRNOTAVAIL;
>>> +	phys_addr_t start, end;
>>> +	u64 i;
>>> +
>>> +	for_each_mem_range_rev(i, &memblock.memory, NULL, NUMA_NO_NODE,
>>> +			       MEMBLOCK_NONE, &start, &end, NULL) {
>>> +		if (start > buf_max)
>>> +			continue;
>>> +
>>> +		/* Memory hole not found */
>>> +		if (end < buf_min)
>>> +			break;
>>> +
>>> +		/* Adjust memory region based on the given range */
>>> +		if (start < buf_min)
>>> +			start = buf_min;
>>> +		if (end > buf_max)
>>> +			end = buf_max;
>>> +
>>> +		start = ALIGN(start, kbuf->buf_align);
>>> +		if (start < end && (end - start + 1) >= kbuf->memsz) {
>> 
>> This is why I dislike using start and end to express address ranges:
>> 
>> While struct resource seems to use the [address, end] convention, my
>
> struct crash_mem also uses [address, end] convention.
> This off-by-one error did not cause any issues as the hole start and size we try to find
> are at least page aligned.
>
> Nonetheless, I think fixing 'end' early in the loop with "end -= 1" would ensure
> correctness while continuing to use the same convention for structs crash_mem & resource.

Sounds good.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH 2/2] xfs: replace ialloc space res macro with inline helper
From: Dave Chinner @ 2020-07-16 22:01 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs
In-Reply-To: <20200716121849.36661-1-bfoster@redhat.com>

On Thu, Jul 16, 2020 at 08:18:49AM -0400, Brian Foster wrote:
> Rewrite the macro as a static inline helper to clean up the logic
> and have one less macro.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_trans_space.h | 24 ++++++++++++++++--------
>  fs/xfs/xfs_inode.c              |  4 ++--
>  fs/xfs/xfs_symlink.c            |  2 +-
>  3 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_trans_space.h b/fs/xfs/libxfs/xfs_trans_space.h
> index c6df01a2a158..d08dfc8795c3 100644
> --- a/fs/xfs/libxfs/xfs_trans_space.h
> +++ b/fs/xfs/libxfs/xfs_trans_space.h
> @@ -55,10 +55,18 @@
>  	 XFS_DIRENTER_MAX_SPLIT(mp,nl))
>  #define	XFS_DIRREMOVE_SPACE_RES(mp)	\
>  	XFS_DAREMOVE_SPACE_RES(mp, XFS_DATA_FORK)
> -#define	XFS_IALLOC_SPACE_RES(mp)	\
> -	(M_IGEO(mp)->ialloc_blks + \
> -	 ((xfs_sb_version_hasfinobt(&mp->m_sb) ? 2 : 1) * \
> -	  (M_IGEO(mp)->inobt_maxlevels - 1)))
> +
> +static inline int
> +xfs_ialloc_space_res(
> +	struct xfs_mount	*mp)
> +{
> +	int			res = M_IGEO(mp)->ialloc_blks;
> +
> +	res += M_IGEO(mp)->inobt_maxlevels - 1;
> +	if (xfs_sb_version_hasfinobt(&mp->m_sb))
> +		res += M_IGEO(mp)->inobt_maxlevels - 1;
> +	return res;
> +}

This misses the point I made. i.e. that the space reservation is
constant and never changes, yet we calculate it -twice- per inode
create. That means we can be calculating it hundreds of thousands of
times a second instead of just reading a variable that is likely hot
in cache.

IOWs, if we are going to improve this code, it should to be moved to
a pre-calculated, read-only, per-mount variable so the repeated
calculation goes away entirely.

Then the macro/function goes away entirely an is replaced simply
by mp->m_ialloc_space_res or M_IGEO(mp)->alloc_space_res....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply

* Re: [PATCH v3 05/12] powerpc/drmem: make lmb walk a bit more flexible
From: Thiago Jung Bauermann @ 2020-07-16 22:01 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Nayna Jain, Kexec-ml, Mahesh J Salgaonkar,
	Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain, Petr Tesarik,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <30e8f02a-f009-70a5-01e9-dec9eff213b1@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 15/07/20 9:20 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>> 
>>> @@ -534,7 +537,7 @@ static int __init early_init_dt_scan_memory_ppc(unsigned long node,
>>>  #ifdef CONFIG_PPC_PSERIES
>>>  	if (depth == 1 &&
>>>  	    strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) {
>>> -		walk_drmem_lmbs_early(node, early_init_drmem_lmb);
>>> +		walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb);
>> 
>> walk_drmem_lmbs_early() can now fail. Should this failure be propagated
>> as a return value of early_init_dt_scan_memory_ppc()?
>   
>> 
>>>  		return 0;
>>>  	}
>>>  #endif
>> <snip>
>> 
>>> @@ -787,7 +790,7 @@ static int __init parse_numa_properties(void)
>>>  	 */
>>>  	memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>>>  	if (memory) {
>>> -		walk_drmem_lmbs(memory, numa_setup_drmem_lmb);
>>> +		walk_drmem_lmbs(memory, NULL, numa_setup_drmem_lmb);
>> 
>> Similarly here. Now that this call can fail, should
>> parse_numa_properties() handle or propagate the failure?
>
> They would still not fail unless the callbacks early_init_drmem_lmb() & numa_setup_drmem_lmb()
> are updated to have failure scenarios. Also, these call sites always ignored failure scenarios
> even before walk_drmem_lmbs() was introduced. So, I prefer to keep them the way they are?

Ok, makes sense. In this case:

Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH v3 05/12] powerpc/drmem: make lmb walk a bit more flexible
From: Thiago Jung Bauermann @ 2020-07-16 22:01 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Petr Tesarik, Nayna Jain, Kexec-ml,
	Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <30e8f02a-f009-70a5-01e9-dec9eff213b1@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 15/07/20 9:20 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>> 
>>> @@ -534,7 +537,7 @@ static int __init early_init_dt_scan_memory_ppc(unsigned long node,
>>>  #ifdef CONFIG_PPC_PSERIES
>>>  	if (depth == 1 &&
>>>  	    strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) {
>>> -		walk_drmem_lmbs_early(node, early_init_drmem_lmb);
>>> +		walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb);
>> 
>> walk_drmem_lmbs_early() can now fail. Should this failure be propagated
>> as a return value of early_init_dt_scan_memory_ppc()?
>   
>> 
>>>  		return 0;
>>>  	}
>>>  #endif
>> <snip>
>> 
>>> @@ -787,7 +790,7 @@ static int __init parse_numa_properties(void)
>>>  	 */
>>>  	memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>>>  	if (memory) {
>>> -		walk_drmem_lmbs(memory, numa_setup_drmem_lmb);
>>> +		walk_drmem_lmbs(memory, NULL, numa_setup_drmem_lmb);
>> 
>> Similarly here. Now that this call can fail, should
>> parse_numa_properties() handle or propagate the failure?
>
> They would still not fail unless the callbacks early_init_drmem_lmb() & numa_setup_drmem_lmb()
> are updated to have failure scenarios. Also, these call sites always ignored failure scenarios
> even before walk_drmem_lmbs() was introduced. So, I prefer to keep them the way they are?

Ok, makes sense. In this case:

Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply

* Re: [PATCH v3 04/12] ppc64/kexec_file: avoid stomping memory used by special regions
From: Thiago Jung Bauermann @ 2020-07-16 21:59 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Nayna Jain, Kexec-ml, Mahesh J Salgaonkar,
	Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain, Petr Tesarik,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <0582476e-415e-3f60-2bb2-6199d0340156@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 15/07/20 8:09 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>> 
>
> <snip>
>  
>>> +/**
>>> + * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
>>> + *                              in the memory regions between buf_min & buf_max
>>> + *                              for the buffer. If found, sets kbuf->mem.
>>> + * @kbuf:                       Buffer contents and memory parameters.
>>> + * @buf_min:                    Minimum address for the buffer.
>>> + * @buf_max:                    Maximum address for the buffer.
>>> + *
>>> + * Returns 0 on success, negative errno on error.
>>> + */
>>> +static int __locate_mem_hole_top_down(struct kexec_buf *kbuf,
>>> +				      u64 buf_min, u64 buf_max)
>>> +{
>>> +	int ret = -EADDRNOTAVAIL;
>>> +	phys_addr_t start, end;
>>> +	u64 i;
>>> +
>>> +	for_each_mem_range_rev(i, &memblock.memory, NULL, NUMA_NO_NODE,
>>> +			       MEMBLOCK_NONE, &start, &end, NULL) {
>>> +		if (start > buf_max)
>>> +			continue;
>>> +
>>> +		/* Memory hole not found */
>>> +		if (end < buf_min)
>>> +			break;
>>> +
>>> +		/* Adjust memory region based on the given range */
>>> +		if (start < buf_min)
>>> +			start = buf_min;
>>> +		if (end > buf_max)
>>> +			end = buf_max;
>>> +
>>> +		start = ALIGN(start, kbuf->buf_align);
>>> +		if (start < end && (end - start + 1) >= kbuf->memsz) {
>> 
>> This is why I dislike using start and end to express address ranges:
>> 
>> While struct resource seems to use the [address, end] convention, my
>
> struct crash_mem also uses [address, end] convention.
> This off-by-one error did not cause any issues as the hole start and size we try to find
> are at least page aligned.
>
> Nonetheless, I think fixing 'end' early in the loop with "end -= 1" would ensure
> correctness while continuing to use the same convention for structs crash_mem & resource.

Sounds good.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* Pull request for UEFI sub-system for efi-2020-10-rc1 (4)
From: Tom Rini @ 2020-07-16 22:00 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <3328ed17-364a-7840-d791-cc020857c711@gmx.de>

On Thu, Jul 16, 2020 at 05:55:55PM +0200, Heinrich Schuchardt wrote:

> The following changes since commit 47b0a493247b38c1557062e108cc4868e211a73e:
> 
>   Merge branch '2020-07-15-ci-updates' (2020-07-15 15:48:05 -0400)
> 
> are available in the Git repository at:
> 
>   https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git
> tags/efi-2020-10-rc1-4
> 
> for you to fetch changes up to c70f44817d466848c421ed7159bc9aba428e69ad:
> 
>   efi_loader: simplify 'printenv -e' (2020-07-16 12:37:03 +0200)
> 
> Gitlab and Travis showed no problems:
> 
> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/pipelines/4071
> https://travis-ci.org/github/xypron2/u-boot/builds/708688078
> 

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200716/37dd5a86/attachment.sig>

^ permalink raw reply

* Re: [PATCH v3 04/12] ppc64/kexec_file: avoid stomping memory used by special regions
From: Thiago Jung Bauermann @ 2020-07-16 21:59 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Petr Tesarik, Nayna Jain, Kexec-ml,
	Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <0582476e-415e-3f60-2bb2-6199d0340156@linux.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> On 15/07/20 8:09 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbathini@linux.ibm.com> writes:
>> 
>
> <snip>
>  
>>> +/**
>>> + * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
>>> + *                              in the memory regions between buf_min & buf_max
>>> + *                              for the buffer. If found, sets kbuf->mem.
>>> + * @kbuf:                       Buffer contents and memory parameters.
>>> + * @buf_min:                    Minimum address for the buffer.
>>> + * @buf_max:                    Maximum address for the buffer.
>>> + *
>>> + * Returns 0 on success, negative errno on error.
>>> + */
>>> +static int __locate_mem_hole_top_down(struct kexec_buf *kbuf,
>>> +				      u64 buf_min, u64 buf_max)
>>> +{
>>> +	int ret = -EADDRNOTAVAIL;
>>> +	phys_addr_t start, end;
>>> +	u64 i;
>>> +
>>> +	for_each_mem_range_rev(i, &memblock.memory, NULL, NUMA_NO_NODE,
>>> +			       MEMBLOCK_NONE, &start, &end, NULL) {
>>> +		if (start > buf_max)
>>> +			continue;
>>> +
>>> +		/* Memory hole not found */
>>> +		if (end < buf_min)
>>> +			break;
>>> +
>>> +		/* Adjust memory region based on the given range */
>>> +		if (start < buf_min)
>>> +			start = buf_min;
>>> +		if (end > buf_max)
>>> +			end = buf_max;
>>> +
>>> +		start = ALIGN(start, kbuf->buf_align);
>>> +		if (start < end && (end - start + 1) >= kbuf->memsz) {
>> 
>> This is why I dislike using start and end to express address ranges:
>> 
>> While struct resource seems to use the [address, end] convention, my
>
> struct crash_mem also uses [address, end] convention.
> This off-by-one error did not cause any issues as the hole start and size we try to find
> are at least page aligned.
>
> Nonetheless, I think fixing 'end' early in the loop with "end -= 1" would ensure
> correctness while continuing to use the same convention for structs crash_mem & resource.

Sounds good.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.