On 13/03/2023 12:41, Konrad Dybcio wrote:
> The BWMON hardware has two sets of registers: one for the monitor itself
> and one called "global". It has what seems to be some kind of a head
> switch and an interrupt control register. It's usually 0x200 in size.
>
> On fairly recent SoCs (with the starting point seemingly being moving
> the OSM programming to the firmware) these two register sets are
> contiguous and overlapping, like this (on sm8450):
>
> /* notice how base.start == global_base.start+0x100 */
> reg = <0x90b6400 0x300>, <0x90b6300 0x200>;
> reg-names = "base", "global_base";
>
> Which led to some confusion and the assumption that since the
> "interesting" global registers begin right after global_base+0x100,
> there's no need to map two separate regions and one can simply subtract
> 0x100 from the offsets.
>
> This is however not the case for anything older than SDM845, as the
> global region can appear in seemingly random spots on the register map.
>
> Handle the case where the global registers are mapped separately to allow
> proper functioning of BWMONv4 on MSM8998 and older. Add specific
> compatibles for 845, 8280xp, 7280 and 8550 (all of which use the single
> reg space scheme) to keep backwards compatibility with old DTs.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> ---
> drivers/soc/qcom/icc-bwmon.c | 228 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 208 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
> index d07be3700db6..2fe67a3cd2d7 100644
> --- a/drivers/soc/qcom/icc-bwmon.c
> +++ b/drivers/soc/qcom/icc-bwmon.c
> @@ -34,14 +34,27 @@
> /* Internal sampling clock frequency */
> #define HW_TIMER_HZ 19200000
>
> -#define BWMON_V4_GLOBAL_IRQ_CLEAR 0x008
> -#define BWMON_V4_GLOBAL_IRQ_ENABLE 0x00c
> +#define BWMON_V4_GLOBAL_IRQ_CLEAR 0x108
> +#define BWMON_V4_GLOBAL_IRQ_ENABLE 0x10c
> /*
> * All values here and further are matching regmap fields, so without absolute
> * register offsets.
> */
> #define BWMON_V4_GLOBAL_IRQ_ENABLE_ENABLE BIT(0)
>
> +/*
> + * Starting with SDM845, the BWMON4 register space has changed a bit:
> + * the global registers were jammed into the beginning of the monitor region.
> + * To keep the proper offsets, one would have to map <GLOBAL_BASE 0x200> and
> + * <GLOBAL_BASE+0x100 0x300>, which is straight up wrong.
> + * To facilitate for that, while allowing the older, arguably more proper
> + * implementations to work, offset the global registers by -0x100 to avoid
> + * having to map half of the global registers twice.
> + */
> +#define BWMON_V4_845_OFFSET 0x100
> +#define BWMON_V4_GLOBAL_IRQ_CLEAR_845 (BWMON_V4_GLOBAL_IRQ_CLEAR - BWMON_V4_845_OFFSET)
> +#define BWMON_V4_GLOBAL_IRQ_ENABLE_845 (BWMON_V4_GLOBAL_IRQ_ENABLE - BWMON_V4_845_OFFSET)
> +
> #define BWMON_V4_IRQ_STATUS 0x100
> #define BWMON_V4_IRQ_CLEAR 0x108
>
> @@ -118,8 +131,12 @@
> #define BWMON_NEEDS_FORCE_CLEAR BIT(1)
>
> enum bwmon_fields {
> + /* Global region fields, keep them at the top */
> F_GLOBAL_IRQ_CLEAR,
> F_GLOBAL_IRQ_ENABLE,
> + F_NUM_GLOBAL_FIELDS,
> +
> + /* Monitor region fields */
> F_IRQ_STATUS,
F_IRQ_STATUS = F_NUM_GLOBAL_FIELDS
or = 2, so you won't waste one space in the array.
> F_IRQ_CLEAR,
> F_IRQ_ENABLE,
> @@ -157,6 +174,9 @@ struct icc_bwmon_data {
>
> const struct regmap_config *regmap_cfg;
> const struct reg_field *regmap_fields;
> +
> + const struct regmap_config *global_regmap_cfg;
> + const struct reg_field *global_regmap_fields;
> };
>
> struct icc_bwmon {
> @@ -166,6 +186,7 @@ struct icc_bwmon {
>
> struct regmap *regmap;
> struct regmap_field *regs[F_NUM_FIELDS];
> + struct regmap_field *global_regs[F_NUM_FIELDS];
F_NUM_GLOBAL_FIELDS?
>
(...)
> }
>
> static int bwmon_probe(struct platform_device *pdev)
> @@ -645,6 +816,21 @@ static const struct icc_bwmon_data msm8998_bwmon_data = {
> .quirks = BWMON_HAS_GLOBAL_IRQ,
> .regmap_fields = msm8998_bwmon_reg_fields,
> .regmap_cfg = &msm8998_bwmon_regmap_cfg,
> + .global_regmap_fields = msm8998_bwmon_global_reg_fields,
> + .global_regmap_cfg = &msm8998_bwmon_global_regmap_cfg,
> +};
> +
> +static const struct icc_bwmon_data sdm845_ddr_bwmon_data = {
The name "ddr" is here (and other places) confusing. This is not the DDR
bwmon.
> + .sample_ms = 4,
> + .count_unit_kb = 64,
> + .default_highbw_kbps = 4800 * 1024, /* 4.8 GBps */
> + .default_medbw_kbps = 512 * 1024, /* 512 MBps */
> + .default_lowbw_kbps = 0,
> + .zone1_thres_count = 16,
> + .zone3_thres_count = 1,
> + .quirks = BWMON_HAS_GLOBAL_IRQ,
> + .regmap_fields = sdm845_ddr_bwmon_reg_fields,
> + .regmap_cfg = &sdm845_ddr_bwmon_regmap_cfg,
> };
>
> static const struct icc_bwmon_data sdm845_llcc_bwmon_data = {
> @@ -673,16 +859,18 @@ static const struct icc_bwmon_data sc7280_llcc_bwmon_data = {
> };
>
> static const struct of_device_id bwmon_of_match[] = {
> - {
> - .compatible = "qcom,msm8998-bwmon",
> - .data = &msm8998_bwmon_data
> - }, {
> - .compatible = "qcom,sdm845-llcc-bwmon",
> - .data = &sdm845_llcc_bwmon_data
> - }, {
> - .compatible = "qcom,sc7280-llcc-bwmon",
> - .data = &sc7280_llcc_bwmon_data
> - },
> + /* BWMONv4, separate monitor and global register spaces */
> + { .compatible = "qcom,msm8998-bwmon", .data = &msm8998_bwmon_data },
> + /* BWMONv4, unified register space */
> + { .compatible = "qcom,sdm845-bwmon", .data = &sdm845_ddr_bwmon_data },
> + /* BWMONv5 */
> + { .compatible = "qcom,sdm845-llcc-bwmon", .data = &sdm845_llcc_bwmon_data },
> + { .compatible = "qcom,sc7280-llcc-bwmon", .data = &sc7280_llcc_bwmon_data },
> +
> + /* Compatibles kept for legacy reasons */
> + { .compatible = "qcom,sc7280-cpu-bwmon", .data = &sdm845_ddr_bwmon_data },
> + { .compatible = "qcom,sc8280xp-cpu-bwmon", .data = &sdm845_ddr_bwmon_data },
> + { .compatible = "qcom,sm8550-cpu-bwmon", .data = &sdm845_ddr_bwmon_data },
> {}
> };
> MODULE_DEVICE_TABLE(of, bwmon_of_match);
>
Best regards,
Krzysztof
