mbox series

[v2,00/10] interconnect: osm-l3: SC8280XP L3 and DDR scaling

Message ID 20221111032515.3460-1-quic_bjorande@quicinc.com
Headers show
Series interconnect: osm-l3: SC8280XP L3 and DDR scaling | expand

Message

Bjorn Andersson Nov. 11, 2022, 3:25 a.m. UTC
The SC8280XP currently shows depressing results in memory benchmarks.
Fix this by introducing support for the platform in the OSM (and EPSS)
L3 driver and support for the platform in the bwmon binding.

Then add the necessary nodes and values throughout the sc8280xp and
sa8540p dtsi files to make the various devices on these platforms scale
both L3, memory bus and DDR.

Bjorn Andersson (10):
  interconnect: qcom: osm-l3: Use platform-independent node ids
  interconnect: qcom: osm-l3: Squash common descriptors
  interconnect: qcom: osm-l3: Add per-core EPSS L3 support
  interconnect: qcom: osm-l3: Simplify osm_l3_set()
  dt-bindings: interconnect: Add sm8350, sc8280xp and generic OSM L3
    compatibles
  arm64: dts: qcom: Align with generic osm-l3/epss-l3
  arm64: dts: qcom: sc8280xp: Add epss_l3 node
  arm64: dts: qcom: sc8280xp: Set up L3 scaling
  dt-bindings: interconnect: qcom,msm8998-bwmon: Add sc8280xp bwmon
    instances
  arm64: dts: qcom: sc8280xp: Add bwmon instances

 .../interconnect/qcom,msm8998-bwmon.yaml      |   5 +
 .../bindings/interconnect/qcom,osm-l3.yaml    |  24 ++-
 arch/arm64/boot/dts/qcom/sa8540p.dtsi         |  39 +++++
 arch/arm64/boot/dts/qcom/sc7180.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sc7280.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi        | 152 ++++++++++++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sm8150.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |   2 +-
 drivers/interconnect/qcom/osm-l3.c            | 126 ++++-----------
 10 files changed, 252 insertions(+), 104 deletions(-)

Comments

Sibi Sankar Nov. 11, 2022, 10:23 a.m. UTC | #1
Hey Bjorn
Thanks for the patch.

On 11/11/22 08:55, Bjorn Andersson wrote:
> The identifiers used for nodes needs to be unique in the running system,
> but defining them per platform results in a lot of duplicated
> definitions and prevents us from using generic compatibles.
> 
> As these identifiers are not exposed outside the kernel, change to use
> driver-local numbers, picked completely at random.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> Tested-by: Steev Klimaszewski <steev@kali.org>

With the change in implementation, you can remove now remove the per soc
node id headers included in the driver. With ^^ done.

Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com>

> ---
> 
> Changes since v1:
> - None
> 
>   drivers/interconnect/qcom/osm-l3.c | 87 +++++++++++-------------------
>   1 file changed, 30 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> index ddbdf0943f94..d23769844419 100644
> --- a/drivers/interconnect/qcom/osm-l3.c
> +++ b/drivers/interconnect/qcom/osm-l3.c
> @@ -74,6 +74,11 @@ struct qcom_osm_l3_desc {
>   	unsigned int reg_perf_state;
>   };
>   
> +enum {
> +	OSM_L3_MASTER_NODE = 10000,
> +	OSM_L3_SLAVE_NODE,
> +};
> +
>   #define DEFINE_QNODE(_name, _id, _buswidth, ...)			\
>   	static const struct qcom_osm_l3_node _name = {			\
>   		.name = #_name,						\
> @@ -83,97 +88,65 @@ struct qcom_osm_l3_desc {
>   		.links = { __VA_ARGS__ },				\
>   	}
>   
> -DEFINE_QNODE(sdm845_osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
> -DEFINE_QNODE(sdm845_osm_l3, SDM845_SLAVE_OSM_L3, 16);
> +DEFINE_QNODE(osm_l3_master, OSM_L3_MASTER_NODE, 16, OSM_L3_SLAVE_NODE);
> +DEFINE_QNODE(osm_l3_slave, OSM_L3_SLAVE_NODE, 16);
> +
> +static const struct qcom_osm_l3_node * const osm_l3_nodes[] = {
> +	[MASTER_OSM_L3_APPS] = &osm_l3_master,
> +	[SLAVE_OSM_L3] = &osm_l3_slave,
> +};
> +
> +DEFINE_QNODE(epss_l3_master, OSM_L3_MASTER_NODE, 32, OSM_L3_SLAVE_NODE);
> +DEFINE_QNODE(epss_l3_slave, OSM_L3_SLAVE_NODE, 32);
>   
> -static const struct qcom_osm_l3_node * const sdm845_osm_l3_nodes[] = {
> -	[MASTER_OSM_L3_APPS] = &sdm845_osm_apps_l3,
> -	[SLAVE_OSM_L3] = &sdm845_osm_l3,
> +static const struct qcom_osm_l3_node * const epss_l3_nodes[] = {
> +	[MASTER_EPSS_L3_APPS] = &epss_l3_master,
> +	[SLAVE_EPSS_L3_SHARED] = &epss_l3_slave,
>   };
>   
>   static const struct qcom_osm_l3_desc sdm845_icc_osm_l3 = {
> -	.nodes = sdm845_osm_l3_nodes,
> -	.num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
> +	.nodes = osm_l3_nodes,
> +	.num_nodes = ARRAY_SIZE(osm_l3_nodes),
>   	.lut_row_size = OSM_LUT_ROW_SIZE,
>   	.reg_freq_lut = OSM_REG_FREQ_LUT,
>   	.reg_perf_state = OSM_REG_PERF_STATE,
>   };
>   
> -DEFINE_QNODE(sc7180_osm_apps_l3, SC7180_MASTER_OSM_L3_APPS, 16, SC7180_SLAVE_OSM_L3);
> -DEFINE_QNODE(sc7180_osm_l3, SC7180_SLAVE_OSM_L3, 16);
> -
> -static const struct qcom_osm_l3_node * const sc7180_osm_l3_nodes[] = {
> -	[MASTER_OSM_L3_APPS] = &sc7180_osm_apps_l3,
> -	[SLAVE_OSM_L3] = &sc7180_osm_l3,
> -};
> -
>   static const struct qcom_osm_l3_desc sc7180_icc_osm_l3 = {
> -	.nodes = sc7180_osm_l3_nodes,
> -	.num_nodes = ARRAY_SIZE(sc7180_osm_l3_nodes),
> +	.nodes = osm_l3_nodes,
> +	.num_nodes = ARRAY_SIZE(osm_l3_nodes),
>   	.lut_row_size = OSM_LUT_ROW_SIZE,
>   	.reg_freq_lut = OSM_REG_FREQ_LUT,
>   	.reg_perf_state = OSM_REG_PERF_STATE,
>   };
>   
> -DEFINE_QNODE(sc7280_epss_apps_l3, SC7280_MASTER_EPSS_L3_APPS, 32, SC7280_SLAVE_EPSS_L3);
> -DEFINE_QNODE(sc7280_epss_l3, SC7280_SLAVE_EPSS_L3, 32);
> -
> -static const struct qcom_osm_l3_node * const sc7280_epss_l3_nodes[] = {
> -	[MASTER_EPSS_L3_APPS] = &sc7280_epss_apps_l3,
> -	[SLAVE_EPSS_L3_SHARED] = &sc7280_epss_l3,
> -};
> -
>   static const struct qcom_osm_l3_desc sc7280_icc_epss_l3 = {
> -	.nodes = sc7280_epss_l3_nodes,
> -	.num_nodes = ARRAY_SIZE(sc7280_epss_l3_nodes),
> +	.nodes = epss_l3_nodes,
> +	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
>   	.lut_row_size = EPSS_LUT_ROW_SIZE,
>   	.reg_freq_lut = EPSS_REG_FREQ_LUT,
>   	.reg_perf_state = EPSS_REG_PERF_STATE,
>   };
>   
> -DEFINE_QNODE(sc8180x_osm_apps_l3, SC8180X_MASTER_OSM_L3_APPS, 32, SC8180X_SLAVE_OSM_L3);
> -DEFINE_QNODE(sc8180x_osm_l3, SC8180X_SLAVE_OSM_L3, 32);
> -
> -static const struct qcom_osm_l3_node * const sc8180x_osm_l3_nodes[] = {
> -	[MASTER_OSM_L3_APPS] = &sc8180x_osm_apps_l3,
> -	[SLAVE_OSM_L3] = &sc8180x_osm_l3,
> -};
> -
>   static const struct qcom_osm_l3_desc sc8180x_icc_osm_l3 = {
> -	.nodes = sc8180x_osm_l3_nodes,
> -	.num_nodes = ARRAY_SIZE(sc8180x_osm_l3_nodes),
> +	.nodes = osm_l3_nodes,
> +	.num_nodes = ARRAY_SIZE(osm_l3_nodes),
>   	.lut_row_size = OSM_LUT_ROW_SIZE,
>   	.reg_freq_lut = OSM_REG_FREQ_LUT,
>   	.reg_perf_state = OSM_REG_PERF_STATE,
>   };
>   
> -DEFINE_QNODE(sm8150_osm_apps_l3, SM8150_MASTER_OSM_L3_APPS, 32, SM8150_SLAVE_OSM_L3);
> -DEFINE_QNODE(sm8150_osm_l3, SM8150_SLAVE_OSM_L3, 32);
> -
> -static const struct qcom_osm_l3_node * const sm8150_osm_l3_nodes[] = {
> -	[MASTER_OSM_L3_APPS] = &sm8150_osm_apps_l3,
> -	[SLAVE_OSM_L3] = &sm8150_osm_l3,
> -};
> -
>   static const struct qcom_osm_l3_desc sm8150_icc_osm_l3 = {
> -	.nodes = sm8150_osm_l3_nodes,
> -	.num_nodes = ARRAY_SIZE(sm8150_osm_l3_nodes),
> +	.nodes = osm_l3_nodes,
> +	.num_nodes = ARRAY_SIZE(osm_l3_nodes),
>   	.lut_row_size = OSM_LUT_ROW_SIZE,
>   	.reg_freq_lut = OSM_REG_FREQ_LUT,
>   	.reg_perf_state = OSM_REG_PERF_STATE,
>   };
>   
> -DEFINE_QNODE(sm8250_epss_apps_l3, SM8250_MASTER_EPSS_L3_APPS, 32, SM8250_SLAVE_EPSS_L3);
> -DEFINE_QNODE(sm8250_epss_l3, SM8250_SLAVE_EPSS_L3, 32);
> -
> -static const struct qcom_osm_l3_node * const sm8250_epss_l3_nodes[] = {
> -	[MASTER_EPSS_L3_APPS] = &sm8250_epss_apps_l3,
> -	[SLAVE_EPSS_L3_SHARED] = &sm8250_epss_l3,
> -};
> -
>   static const struct qcom_osm_l3_desc sm8250_icc_epss_l3 = {
> -	.nodes = sm8250_epss_l3_nodes,
> -	.num_nodes = ARRAY_SIZE(sm8250_epss_l3_nodes),
> +	.nodes = epss_l3_nodes,
> +	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
>   	.lut_row_size = EPSS_LUT_ROW_SIZE,
>   	.reg_freq_lut = EPSS_REG_FREQ_LUT,
>   	.reg_perf_state = EPSS_REG_PERF_STATE,
Sibi Sankar Nov. 11, 2022, 10:24 a.m. UTC | #2
On 11/11/22 08:55, Bjorn Andersson wrote:
> The EPSS instance in e.g. SM8350 and SC8280XP has per-core L3 voting
> enabled. In this configuration, the "shared" vote is done using the
> REG_L3_VOTE register instead of PERF_STATE.
> 
> Rename epss_l3 to clarify that it's affecting the PERF_STATE register
> and add a new L3_VOTE description. Given platform lineage it's assumed
> that the L3_VOTE-based case will be the predominant one, so use this for
> a new generic qcom,epss-l3 compatible.
> 
> While adding the EPSS generic, also add qcom,osm-l3.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> Tested-by: Steev Klimaszewski <steev@kali.org>

Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com>

> ---
> 
> Changes since v1:
> - None
> 
>   drivers/interconnect/qcom/osm-l3.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> index 7d6844253241..469be732a00b 100644
> --- a/drivers/interconnect/qcom/osm-l3.c
> +++ b/drivers/interconnect/qcom/osm-l3.c
> @@ -34,6 +34,7 @@
>   
>   /* EPSS Register offsets */
>   #define EPSS_LUT_ROW_SIZE		4
> +#define EPSS_REG_L3_VOTE		0x90
>   #define EPSS_REG_FREQ_LUT		0x100
>   #define EPSS_REG_PERF_STATE		0x320
>   
> @@ -112,7 +113,7 @@ static const struct qcom_osm_l3_desc osm_l3 = {
>   	.reg_perf_state = OSM_REG_PERF_STATE,
>   };
>   
> -static const struct qcom_osm_l3_desc epss_l3 = {
> +static const struct qcom_osm_l3_desc epss_l3_perf_state = {
>   	.nodes = epss_l3_nodes,
>   	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
>   	.lut_row_size = EPSS_LUT_ROW_SIZE,
> @@ -120,6 +121,14 @@ static const struct qcom_osm_l3_desc epss_l3 = {
>   	.reg_perf_state = EPSS_REG_PERF_STATE,
>   };
>   
> +static const struct qcom_osm_l3_desc epss_l3_l3_vote = {
> +	.nodes = epss_l3_nodes,
> +	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
> +	.lut_row_size = EPSS_LUT_ROW_SIZE,
> +	.reg_freq_lut = EPSS_REG_FREQ_LUT,
> +	.reg_perf_state = EPSS_REG_L3_VOTE,
> +};
> +
>   static int qcom_osm_l3_set(struct icc_node *src, struct icc_node *dst)
>   {
>   	struct qcom_osm_l3_icc_provider *qp;
> @@ -285,12 +294,14 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
>   }
>   
>   static const struct of_device_id osm_l3_of_match[] = {
> +	{ .compatible = "qcom,epss-l3", .data = &epss_l3_l3_vote },
> +	{ .compatible = "qcom,osm-l3", .data = &osm_l3 },
>   	{ .compatible = "qcom,sc7180-osm-l3", .data = &osm_l3 },
> -	{ .compatible = "qcom,sc7280-epss-l3", .data = &epss_l3 },
> +	{ .compatible = "qcom,sc7280-epss-l3", .data = &epss_l3_perf_state },
>   	{ .compatible = "qcom,sdm845-osm-l3", .data = &osm_l3 },
>   	{ .compatible = "qcom,sm8150-osm-l3", .data = &osm_l3 },
>   	{ .compatible = "qcom,sc8180x-osm-l3", .data = &osm_l3 },
> -	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3 },
> +	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3_perf_state },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, osm_l3_of_match);
Sibi Sankar Nov. 11, 2022, 10:44 a.m. UTC | #3
On 11/11/22 08:55, Bjorn Andersson wrote:
> Update all references to OSM or EPSS L3 compatibles, to include the
> generic compatible, as defined by the updated binding.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> Tested-by: Steev Klimaszewski <steev@kali.org>
> ---
> 
> Changes since v1:
> - None
> 
>   arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
>   arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
>   arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
>   arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +-
>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 +-
>   5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index ea886cf08b4d..f71cf21a8dd8 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -3558,7 +3558,7 @@
>   		};
>   
>   		osm_l3: interconnect@18321000 {
> -			compatible = "qcom,sc7180-osm-l3";
> +			compatible = "qcom,sc7180-osm-l3", "qcom,osm-l3";
>   			reg = <0 0x18321000 0 0x1400>;
>   
>   			clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 07334e19be99..ad9c61768016 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -5359,7 +5359,7 @@
>   		};
>   
>   		epss_l3: interconnect@18590000 {
> -			compatible = "qcom,sc7280-epss-l3";
> +			compatible = "qcom,sc7280-epss-l3", "qcom,epss-l3";
>   			reg = <0 0x18590000 0 0x1000>;
>   			clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_GPLL0>;
>   			clock-names = "xo", "alternate";
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 1a257f672887..9c7d484ce72f 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -5302,7 +5302,7 @@
>   		};
>   
>   		osm_l3: interconnect@17d41000 {
> -			compatible = "qcom,sdm845-osm-l3";
> +			compatible = "qcom,sdm845-osm-l3", "qcom,osm-l3";
>   			reg = <0 0x17d41000 0 0x1400>;
>   
>   			clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index 18bf51ce8b13..8409fb5ea532 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -3958,7 +3958,7 @@
>   		};
>   
>   		osm_l3: interconnect@18321000 {
> -			compatible = "qcom,sm8150-osm-l3";
> +			compatible = "qcom,sm8150-osm-l3", "qcom,osm-l3";
>   			reg = <0 0x18321000 0 0x1400>;
>   
>   			clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 27b507f3632b..351c232b8dc6 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4884,7 +4884,7 @@
>   		};
>   
>   		epss_l3: interconnect@18590000 {
> -			compatible = "qcom,sm8250-epss-l3";
> +			compatible = "qcom,sm8250-epss-l3", "qcom,epss-l3";

if we do change sc7280, sm8250 epss compatible in the bindings, you'll
no longer need to list the qcom,epss-l3 here. With ^^ done.

Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com>


>   			reg = <0 0x18590000 0 0x1000>;
>   
>   			clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
Georgi Djakov Nov. 17, 2022, 4:25 p.m. UTC | #4
On 11.11.22 5:25, Bjorn Andersson wrote:
> The SC8280XP currently shows depressing results in memory benchmarks.
> Fix this by introducing support for the platform in the OSM (and EPSS)
> L3 driver and support for the platform in the bwmon binding.
> 
> Then add the necessary nodes and values throughout the sc8280xp and
> sa8540p dtsi files to make the various devices on these platforms scale
> both L3, memory bus and DDR.

Good stuff! Thanks Bjorn!

I plan to merge everything except the dts patches, that should go
through the qcom tree.

BR,
Georgi


> Bjorn Andersson (10):
>    interconnect: qcom: osm-l3: Use platform-independent node ids
>    interconnect: qcom: osm-l3: Squash common descriptors
>    interconnect: qcom: osm-l3: Add per-core EPSS L3 support
>    interconnect: qcom: osm-l3: Simplify osm_l3_set()
>    dt-bindings: interconnect: Add sm8350, sc8280xp and generic OSM L3
>      compatibles
>    arm64: dts: qcom: Align with generic osm-l3/epss-l3
>    arm64: dts: qcom: sc8280xp: Add epss_l3 node
>    arm64: dts: qcom: sc8280xp: Set up L3 scaling
>    dt-bindings: interconnect: qcom,msm8998-bwmon: Add sc8280xp bwmon
>      instances
>    arm64: dts: qcom: sc8280xp: Add bwmon instances
> 
>   .../interconnect/qcom,msm8998-bwmon.yaml      |   5 +
>   .../bindings/interconnect/qcom,osm-l3.yaml    |  24 ++-
>   arch/arm64/boot/dts/qcom/sa8540p.dtsi         |  39 +++++
>   arch/arm64/boot/dts/qcom/sc7180.dtsi          |   2 +-
>   arch/arm64/boot/dts/qcom/sc7280.dtsi          |   2 +-
>   arch/arm64/boot/dts/qcom/sc8280xp.dtsi        | 152 ++++++++++++++++++
>   arch/arm64/boot/dts/qcom/sdm845.dtsi          |   2 +-
>   arch/arm64/boot/dts/qcom/sm8150.dtsi          |   2 +-
>   arch/arm64/boot/dts/qcom/sm8250.dtsi          |   2 +-
>   drivers/interconnect/qcom/osm-l3.c            | 126 ++++-----------
>   10 files changed, 252 insertions(+), 104 deletions(-)
>