From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751712AbdJCLB7 (ORCPT ); Tue, 3 Oct 2017 07:01:59 -0400 Received: from mail-dm3nam03on0055.outbound.protection.outlook.com ([104.47.41.55]:5185 "EHLO NAM03-DM3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750720AbdJCLB4 (ORCPT ); Tue, 3 Oct 2017 07:01:56 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=George.Cherian@cavium.com; Subject: Re: [PATCH v4 2/2] ACPI / CPPC: Make cppc acpi driver aware of pcc subspace ids To: "Prakash, Prashanth" , George Cherian , devel@acpica.org, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org Cc: lv.zheng@intel.com, robert.moore@intel.com, jassisinghbrar@gmail.com, lenb@kernel.org, rjw@rjwysocki.net References: <1505885087-5112-1-git-send-email-george.cherian@cavium.com> <1505885087-5112-3-git-send-email-george.cherian@cavium.com> <929b30f2-03d1-269c-8672-72577a4e5a46@codeaurora.org> From: George Cherian Message-ID: Date: Tue, 3 Oct 2017 16:31:35 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <929b30f2-03d1-269c-8672-72577a4e5a46@codeaurora.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [111.93.218.67] X-ClientProxiedBy: BM1PR01CA0094.INDPRD01.PROD.OUTLOOK.COM (10.174.208.162) To BLUPR0701MB1698.namprd07.prod.outlook.com (10.163.85.12) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 233a6ff6-332a-49bd-b685-08d50a4e27e7 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(2017030254152)(2017052603199)(201703131423075)(201703031133081)(201702281549075);SRVR:BLUPR0701MB1698; X-Microsoft-Exchange-Diagnostics: 1;BLUPR0701MB1698;3:4RBxLN/P5rYS/pMqas6bkRxsMF4bsr5SxWzIV+Sd2rBEiMmDcPxg41qdnm/5UCMJLVtgv+5uhsaGQfURFHIhUxUy6XBYvh9Xe3Uq9gWkMOkDenxrlcAXkGsz/QMLQU7qoR/OmvyRFNCxvrr8c3e1FqsuNxTWF3EwZknlB91Ht9ksKnSnSFQeg7bBUzvsG7OFKtjCsyXg2hFCVUzaRtLug+qP37+p52723U4ymAwWD9UlOvaSB3nmL2XiTLkMJWJw;25:Igiq+r9TKG0Gx2oLXhW8JmaHJlB4Oy/Vh3oM+fakcRwc9OoyDctHseGC2CANUHoDPZDMJSSF+WVeY6/LeeUL2sCEcBmIKG4kNiiR3NQRia7+E8QguX3WPiFazdF++5f1O6rhr9XcVbZHw3bIAQ4h1U4tcxLUxou372M+diHg2t5Zk3ozgN/ITolgwgSZnSlREG0KbYMj6z0bnDo50VPNPKqje3ZbwgH2a5TUjGABpqtpasxavgqF03AvXVkqLhvnMEP8tCnXrBC/lNLm7me6fOaQpDwk/Nlt9CofJMx1BzHIeLtLgi7uCmM/GFzCLKWGwrkj85z19OaFY/XJ5UkD2g==;31:ZpHda9/fU5kvWMBd39/G+m2QpxhlRmbTfVt8RzYlOdEn7yzfJq6cvnzlpuxuJP5YkzeQ5uyG+u2v8BFxdluqK0Ufe9xNQouK9DODGIfRjmf5bz2S0mDaihzEaX05fhqrzvGJV++E84rzG3QU+d9zNKvHPCZVNsU0b759HXhdEr1ChHU75M3UjxWzyBY9W0pDCj7ypKNLoMXLFovuQA2h3ebRIs7cWrnU83rmKnrY+q8= X-MS-TrafficTypeDiagnostic: BLUPR0701MB1698: X-Microsoft-Exchange-Diagnostics: 1;BLUPR0701MB1698;20:+z38rLPZUI2c15+8UunslfxxPWRL95n+wiTyvVveqUvx/jllGD0cGNExA1kcEa3zoylfLeKQDY3C3kfiKqIeHF9TX2V0ZhvtSerJh/L8kRu3gZTeI94qpWwnZGJyoLe5AjI0NUrjNsVHFWdwSztoQyrQNrR+VhaCeP98afOr7fP1J79oEwtGj4ABa/DuF8qA7lWVL63VZg3G/slUXcpvb8K3Q4LeVr5/VoqA/H9Pio9uu0jRTRG3MmToyQC+Zyx55hdltJJyUIQcMYSPNnqBwbVb5F09MzPgYcnqcblj7se2PlwYaYkOHFXpdPMGOLPfRb+6uH9lJjzgM0LCArEBsFkHTN/MSAJhe2/AePIVhIYMZEgOtPqLQ5msCjN/kfqTt7Tcvcr6MuYX/hm0aSBcMT9IUBuzhb88nmPUmubFDPpJv0z/f4ubQ91dttMRWrSTffNsuULh/gVqWqfg0bfuJcwSobPramNWpTX7J4BGu98yG8UJsL7g8vRHkvv4glFvo/q2T1S2xGFIKEEKNaz/LdBEcMNk5mu6L9bZKy1WB4jlQznmXKDdmcT9eFzGx5c5aQqp30t1DpJARY6GEO5uPWwfrwMjP+jFFW7JDSvcdK4= X-Exchange-Antispam-Report-Test: UriScan:(60795455431006)(17755550239193); X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(10201501046)(100000703101)(100105400095)(3002001)(93006095)(6041248)(20161123564025)(20161123560025)(20161123558100)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123562025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:BLUPR0701MB1698;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:BLUPR0701MB1698; X-Microsoft-Exchange-Diagnostics: 1;BLUPR0701MB1698;4:EXIdVPrjzaoiimlPBwb7AteCUIy4rNess/KZ1NbSgFwf8P6PkW5EWHEqXalb85rYrHnhT2lXqffsoKupGgBO1q9MrKGXyhwTH9L0pM878ZuJVJJmy2cYz+6ZFLI8hLj4HpJ1mWEUdhUwei11+c3w478fLWo7fbY0VppOSNOQM1XU7OLTfudBs/DtbYOGDNf0Sr2GTNoy2LKdxOY1+TWaaljYHSD12SiGrE9YUwwR5O9YBY07i1sI7LmNPFXkpKDtCQTMtUlDS+/RwC0LyG7cXL8AsCTfvgNUl5Lw2zEBXJzkcyxAaLe96dEs1v2Af+Zmnz23PO86GoFO9rmfqLXg5Q== X-Forefront-PRVS: 044968D9E1 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6049001)(6009001)(376002)(346002)(24454002)(57704003)(189002)(199003)(377454003)(47776003)(90366009)(7736002)(305945005)(53546010)(58126008)(4326008)(110136005)(39060400002)(25786009)(97736004)(6486002)(77096006)(16576012)(66066001)(16526017)(3846002)(6116002)(65956001)(65806001)(316002)(50986999)(76176999)(54356999)(2906002)(53936002)(42882006)(33646002)(31686004)(2870700001)(2950100002)(105586002)(106356001)(101416001)(68736007)(189998001)(8676002)(81156014)(81166006)(6666003)(65826007)(83506001)(5660300001)(31696002)(5009440100003)(8936002)(50466002)(36756003)(229853002)(64126003)(6246003)(23676002)(72206003)(478600001);DIR:OUT;SFP:1101;SCL:1;SRVR:BLUPR0701MB1698;H:[10.167.103.249];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTFVQUjA3MDFNQjE2OTg7MjM6UlNOQ01JeUVMZWJVSUx2bER6dTQzYm43?= =?utf-8?B?aGlNNUZTOFo3NFNZZTdzdVdNUVh1WnlUV2tDeXJXRi9VOEFlN1hTRGREU1Vu?= =?utf-8?B?OWFFZit2T0ptd2Z0cjJxdHJDOVhCdXVtUmxNNkhveDJpRHI3T2NMM3ZaMmNK?= =?utf-8?B?R243NlZxS3VuNkNVS0FlNFdudURRdG1pam53TkRKbG52em1QcU96OElrVkpY?= =?utf-8?B?dUlvaCtseEpzbWpTUDQyVmFITTlrRVRPd0hKZGhtWThKZUVoaUNOYWR0dzll?= =?utf-8?B?c2M3cEVxUmMyaXp1SXcvQWlpd0tITkZXRjduOUt5Q1dQMEZ0dFJyQ3NNSGho?= =?utf-8?B?VGIyWmFZV01NTm4zdEN5MGVuWEV4alpIc1dybU16K01JRkh1ZHRVRWdieTVs?= =?utf-8?B?Y2g4L3hldjZxcDZiYlpQZ0tub3VTYk5razNxNmFmRGNUdjI2bkZTbVIzRTkx?= =?utf-8?B?Q2NnOG43dUN4alBCWHE3Rzg3WCs5V3NTemdnMllNQnJaK3oxVWJxZ2ZOWkFI?= =?utf-8?B?VjhIVWJiZHFlTm5Vc0N2Qmg4TWs2ZnluWEtXM3FkVG84OVJNK0ZPVkZZSUNp?= =?utf-8?B?WVdHNVlmTldYSzE0emFPSjZJRTY2bGFGTUY3NXpTSENrL3RyUjZNb09mQ2c3?= =?utf-8?B?VU01WDhEd2tnRUg3aDFpakN3NGVocFI3aXVudE9BRlFxUXVLc1E2amQwMkxk?= =?utf-8?B?djJCQWRFM1g5eWlNQ3lMd2FCa1dydERaSG5mL2svMEhITmtNSmN5SUY4UVph?= =?utf-8?B?S2NPQktnQW5WR0NVK0NRNE4rV0dqUkcyRlo2aHFzOFBsSWwzRmRlam1QTGV6?= =?utf-8?B?Vjd1dmpjM2ZFM1kwT2dKR1psNmluK2YwWWc4Um1ia1o5RmNpSzZkMWY4Wko4?= =?utf-8?B?dCtUOENWTERvWWppOE1CbERoK2dSb0MrSXh1OXdCc0hleVRQL3VEMDRXcEZ6?= =?utf-8?B?Z3czV01LVi9DN2tYYVZyUXNNdmlWVVJ2TmpGc2lGeTZwMDRGa0d2QnhPYXp5?= =?utf-8?B?cUtqR1pFSXZsSlViZFlrSEpqa3YvNlZGQ2Z1a3d5Y0d3cm9CV3hrNjJYdVB6?= =?utf-8?B?QUs5aXppL2RURmFoRE9nN2pnbHRFTVY1dzhXVkpJaXQyMUt2RDNzZUR0V0ZF?= =?utf-8?B?b0JuOGtYSlkyS3Q4V0FzVjJFTit6azR4WmVnZHhNNmMvV1k3Y1lSM2tyVzlL?= =?utf-8?B?Z2tFdDBkS09tTnpjSktqdXJsK1BiMWwyVWRBREpRQmk5bXRZUWE5MCt0NHhu?= =?utf-8?B?UlhndXlMdjdaOEdPUUpyS211Si9SeXZsU2Z1MERKK0FybVJEWUpLWVN3VCs3?= =?utf-8?B?WFJWbWVneWRpYVBqY2U4NVlXdnd0cFRDdnFqRnRXUFk4T1hkRHlPdnJJUzlU?= =?utf-8?B?Q3RFVGEvbmRVMndYUWpISnJjTEV5cytDQnBSaTROMGpOdzlxMHd0QWN1cmdP?= =?utf-8?B?SjVkOGpuVVpsTytNcFllWk9DUFh5MitEeG5qdW56c015QlExeE4valFpNWVk?= =?utf-8?B?cENyakhYOTNPQTZoZmNETlR4UWlNU3g4dG9YZmJnQ2krbFhKL1VKYTU1NStt?= =?utf-8?B?VlRac056KzZGUUJ3RUlLZnBYMURUYXk1VmVQazZ4UnVvaE1GdzhpTEI3ZTRZ?= =?utf-8?B?UGxSTFBzb0lPN213M3Y0WXR3UFdEaFpBaHRNcnl0SlhHT09hYXQ3L2NqYXo4?= =?utf-8?B?RzA3SGdHOVpBTElEdzlQZ0ZURWtKM2xKNFNvaDlHc3gvcUxMYVVudTBWbVBU?= =?utf-8?B?eGhnNWg0ODZGTHVoS1dhenFrRHNObGQ1c0hMS0w3WGlBankydUhiK0YxeGQw?= =?utf-8?B?aEpLVDZxSkNFNW0wY1FyemhXN2ZJTlY2NW9kb2J5UW4vbHhMd2pUYzFSbGli?= =?utf-8?B?WEcwWlVSYXY2MGhnbG5naVhnVDRXam00dTNoMlFOQWZuVzJzcGlsVHRnWHBt?= =?utf-8?B?MWZDNE5mNCt1NDd6elZZZHZuK0FRTFd0U01MMzZGYnlyQ09TUTlHL2xMYmhq?= =?utf-8?Q?tro1e680?= X-Microsoft-Exchange-Diagnostics: 1;BLUPR0701MB1698;6:L2HtkFLK11th2sDJFGi+/3MHrGgMib8BrY3fc1UNo5igWTH2/6a+Zl+dsamqzsKMA5KeZdxIHfj2BqJ6JbAVKjA3Z25TWWxzymRcHNBywnZ5k6V4BC9/ijeyUs+2zc6kihtlDvhlC7x6nN2qq/to4cxu81RFb7+FEi+y/QiIf2tfvN8vDUoDA9qoIrbUkMB4K/6jdqyTGumF/Qq7EY0+dqjtgNiDZkE8f7Z13xVmf1/o7s03mnD9+cfp4H+c9ODwY/PsRxSzjpcCkM7BbuO9EzvAE24yN7Ub6peYs/iP8a4UVkVuMo3IbwydGMY/Ya/kp3n3RZBSxyP+zKICT7Gjig==;5:KB75W1MAgMFpJjy3kX2efz4T15coJHlDpe4RA5kgS+War1VEZForxszbHHXol8qiy3OSGyIqeldY/KNv3AfQeqN/7FMNFN4C4H353pRRLxNEYZDt6+Vm2HUQ3Pf73r4nhwS/KbKnPXcbZBLsnKk+vYurTCIHaA1esIaOpK6aXaU=;24:ezirUEbN6Qit3uTKdAHD+BC/FurK+yDaoO/oUp569Wyu7WQUOa8fkMd7fxOsm5D0rY86MfVjAVR8+uk4nr7f62urXqXZpUOw6HynoMaqNjc=;7:o/8lEpkJo7ZHmdPlwRc4ltn//ZMb7CGhyIjTufbVLYRt/kUkvCZbkR9Zl19YjyeJDIBmm0vJFaS1+SK+rn0zFaZQVTe2pfTFFDfbmtrPQ36PTHIUJiJk7GhqNuOUMLDeqkzZgoxexyrAI9SDlaBQOjQCuKyn0BKHfIiwbTlfONJlbp4STetW4QomD2UKPWN3i3/HpLNIjdjtmIa+h6RcqXZe1V6zMN5CeO6ge5BUd3Q= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Oct 2017 11:01:50.8203 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR0701MB1698 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Prakash, On 09/29/2017 04:49 AM, Prakash, Prashanth wrote: > Hi George, > > On 9/19/2017 11:24 PM, George Cherian wrote: >> Based on ACPI 6.2 Section 8.4.7.1.9 If the PCC register space is used, >> all PCC registers, for all processors in the same performance >> domain (as defined by _PSD), must be defined to be in the same subspace. >> Based on Section 14.1 of ACPI specification, it is possible to have a >> maximum of 256 PCC subspace ids. Add support of multiple PCC subspace id >> instead of using a single global pcc_data structure. >> >> While at that fix the time_delta check in send_pcc_cmd() so that last_mpar_reset >> and mpar_count is initialized properly. >> >> Signed-off-by: George Cherian >> --- >> drivers/acpi/cppc_acpi.c | 243 +++++++++++++++++++++++++++++------------------ >> 1 file changed, 153 insertions(+), 90 deletions(-) >> >> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c >> index e5b47f0..3ae79ef 100644 >> --- a/drivers/acpi/cppc_acpi.c >> +++ b/drivers/acpi/cppc_acpi.c >> @@ -75,13 +75,16 @@ struct cppc_pcc_data { >> >> /* Wait queue for CPUs whose requests were batched */ >> wait_queue_head_t pcc_write_wait_q; >> + ktime_t last_cmd_cmpl_time; >> + ktime_t last_mpar_reset; >> + int mpar_count; >> + int refcount; >> }; >> >> -/* Structure to represent the single PCC channel */ >> -static struct cppc_pcc_data pcc_data = { >> - .pcc_subspace_idx = -1, >> - .platform_owns_pcc = true, >> -}; >> +/* Array to represent the PCC channel per subspace id */ >> +static struct cppc_pcc_data *pcc_data[MAX_PCC_SUBSPACES]; >> +/* The cpu_pcc_subspace_idx containsper CPU subspace id */ >> +static DEFINE_PER_CPU(int, cpu_pcc_subspace_idx); >> >> /* >> * The cpc_desc structure contains the ACPI register details >> @@ -93,7 +96,8 @@ static struct cppc_pcc_data pcc_data = { >> static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr); >> >> /* pcc mapped address + header size + offset within PCC subspace */ >> -#define GET_PCC_VADDR(offs) (pcc_data.pcc_comm_addr + 0x8 + (offs)) >> +#define GET_PCC_VADDR(offs, pcc_ss_id) (pcc_data[pcc_ss_id]->pcc_comm_addr + \ >> + 0x8 + (offs)) >> >> /* Check if a CPC register is in PCC */ >> #define CPC_IN_PCC(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \ >> @@ -188,13 +192,16 @@ static struct kobj_type cppc_ktype = { >> .default_attrs = cppc_attrs, >> }; >> >> -static int check_pcc_chan(bool chk_err_bit) >> +static int check_pcc_chan(int pcc_ss_id, bool chk_err_bit) >> { >> int ret = -EIO, status = 0; >> - struct acpi_pcct_shared_memory __iomem *generic_comm_base = pcc_data.pcc_comm_addr; >> - ktime_t next_deadline = ktime_add(ktime_get(), pcc_data.deadline); >> + struct cppc_pcc_data *pcc_ss_data = pcc_data[pcc_ss_id]; >> + struct acpi_pcct_shared_memory __iomem *generic_comm_base = >> + pcc_ss_data->pcc_comm_addr; >> + ktime_t next_deadline = ktime_add(ktime_get(), >> + pcc_ss_data->deadline); >> >> - if (!pcc_data.platform_owns_pcc) >> + if (!pcc_ss_data->platform_owns_pcc) >> return 0; >> >> /* Retry in case the remote processor was too slow to catch up. */ >> @@ -219,7 +226,7 @@ static int check_pcc_chan(bool chk_err_bit) >> } >> >> if (likely(!ret)) >> - pcc_data.platform_owns_pcc = false; >> + pcc_ss_data->platform_owns_pcc = false; >> else >> pr_err("PCC check channel failed. Status=%x\n", status); >> >> @@ -230,13 +237,12 @@ static int check_pcc_chan(bool chk_err_bit) >> * This function transfers the ownership of the PCC to the platform >> * So it must be called while holding write_lock(pcc_lock) >> */ >> -static int send_pcc_cmd(u16 cmd) >> +static int send_pcc_cmd(int pcc_ss_id, u16 cmd) >> { >> int ret = -EIO, i; >> + struct cppc_pcc_data *pcc_ss_data = pcc_data[pcc_ss_id]; >> struct acpi_pcct_shared_memory *generic_comm_base = >> - (struct acpi_pcct_shared_memory *) pcc_data.pcc_comm_addr; >> - static ktime_t last_cmd_cmpl_time, last_mpar_reset; >> - static int mpar_count; >> + (struct acpi_pcct_shared_memory *)pcc_ss_data->pcc_comm_addr; >> unsigned int time_delta; >> >> /* >> @@ -249,24 +255,25 @@ static int send_pcc_cmd(u16 cmd) >> * before write completion, so first send a WRITE command to >> * platform >> */ >> - if (pcc_data.pending_pcc_write_cmd) >> - send_pcc_cmd(CMD_WRITE); >> + if (pcc_ss_data->pending_pcc_write_cmd) >> + send_pcc_cmd(pcc_ss_id, CMD_WRITE); >> >> - ret = check_pcc_chan(false); >> + ret = check_pcc_chan(pcc_ss_id, false); >> if (ret) >> goto end; >> } else /* CMD_WRITE */ >> - pcc_data.pending_pcc_write_cmd = FALSE; >> + pcc_ss_data->pending_pcc_write_cmd = FALSE; >> >> /* >> * Handle the Minimum Request Turnaround Time(MRTT) >> * "The minimum amount of time that OSPM must wait after the completion >> * of a command before issuing the next command, in microseconds" >> */ >> - if (pcc_data.pcc_mrtt) { >> - time_delta = ktime_us_delta(ktime_get(), last_cmd_cmpl_time); >> - if (pcc_data.pcc_mrtt > time_delta) >> - udelay(pcc_data.pcc_mrtt - time_delta); >> + if (pcc_ss_data->pcc_mrtt) { >> + time_delta = ktime_us_delta(ktime_get(), >> + pcc_ss_data->last_cmd_cmpl_time); >> + if (pcc_ss_data->pcc_mrtt > time_delta) >> + udelay(pcc_ss_data->pcc_mrtt - time_delta); >> } >> >> /* >> @@ -280,18 +287,19 @@ static int send_pcc_cmd(u16 cmd) >> * not send the request to the platform after hitting the MPAR limit in >> * any 60s window >> */ >> - if (pcc_data.pcc_mpar) { >> - if (mpar_count == 0) { >> - time_delta = ktime_ms_delta(ktime_get(), last_mpar_reset); >> - if (time_delta < 60 * MSEC_PER_SEC) { >> + if (pcc_ss_data->pcc_mpar) { >> + if (pcc_ss_data->mpar_count == 0) { >> + time_delta = ktime_ms_delta(ktime_get(), >> + pcc_ss_data->last_mpar_reset); >> + if ((time_delta < 60 * MSEC_PER_SEC) && pcc_ss_data->last_mpar_reset) { >> pr_debug("PCC cmd not sent due to MPAR limit"); >> ret = -EIO; >> goto end; >> } >> - last_mpar_reset = ktime_get(); >> - mpar_count = pcc_data.pcc_mpar; >> + pcc_ss_data->last_mpar_reset = ktime_get(); >> + pcc_ss_data->mpar_count = pcc_ss_data->pcc_mpar; >> } >> - mpar_count--; >> + pcc_ss_data->mpar_count--; >> } >> >> /* Write to the shared comm region. */ >> @@ -300,10 +308,10 @@ static int send_pcc_cmd(u16 cmd) >> /* Flip CMD COMPLETE bit */ >> writew_relaxed(0, &generic_comm_base->status); >> >> - pcc_data.platform_owns_pcc = true; >> + pcc_ss_data->platform_owns_pcc = true; >> >> /* Ring doorbell */ >> - ret = mbox_send_message(pcc_data.pcc_channel, &cmd); >> + ret = mbox_send_message(pcc_ss_data->pcc_channel, &cmd); >> if (ret < 0) { >> pr_err("Err sending PCC mbox message. cmd:%d, ret:%d\n", >> cmd, ret); >> @@ -311,15 +319,15 @@ static int send_pcc_cmd(u16 cmd) >> } >> >> /* wait for completion and check for PCC errro bit */ >> - ret = check_pcc_chan(true); >> + ret = check_pcc_chan(pcc_ss_id, true); >> >> - if (pcc_data.pcc_mrtt) >> - last_cmd_cmpl_time = ktime_get(); >> + if (pcc_ss_data->pcc_mrtt) >> + pcc_ss_data->last_cmd_cmpl_time = ktime_get(); >> >> - if (pcc_data.pcc_channel->mbox->txdone_irq) >> - mbox_chan_txdone(pcc_data.pcc_channel, ret); >> + if (pcc_ss_data->pcc_channel->mbox->txdone_irq) >> + mbox_chan_txdone(pcc_ss_data->pcc_channel, ret); >> else >> - mbox_client_txdone(pcc_data.pcc_channel, ret); >> + mbox_client_txdone(pcc_ss_data->pcc_channel, ret); >> >> end: >> if (cmd == CMD_WRITE) { >> @@ -329,12 +337,12 @@ static int send_pcc_cmd(u16 cmd) >> if (!desc) >> continue; >> >> - if (desc->write_cmd_id == pcc_data.pcc_write_cnt) >> + if (desc->write_cmd_id == pcc_ss_data->pcc_write_cnt) >> desc->write_cmd_status = ret; >> } >> } >> - pcc_data.pcc_write_cnt++; >> - wake_up_all(&pcc_data.pcc_write_wait_q); >> + pcc_ss_data->pcc_write_cnt++; >> + wake_up_all(&pcc_ss_data->pcc_write_wait_q); >> } >> >> return ret; >> @@ -536,16 +544,16 @@ int acpi_get_psd_map(struct cppc_cpudata **all_cpu_data) >> } >> EXPORT_SYMBOL_GPL(acpi_get_psd_map); >> >> -static int register_pcc_channel(int pcc_subspace_idx) >> +static int register_pcc_channel(int pcc_ss_idx) >> { >> struct acpi_pcct_hw_reduced *cppc_ss; >> u64 usecs_lat; >> >> - if (pcc_subspace_idx >= 0) { >> - pcc_data.pcc_channel = pcc_mbox_request_channel(&cppc_mbox_cl, >> - pcc_subspace_idx); >> + if (pcc_ss_idx >= 0) { >> + pcc_data[pcc_ss_idx]->pcc_channel = >> + pcc_mbox_request_channel(&cppc_mbox_cl, pcc_ss_idx); >> >> - if (IS_ERR(pcc_data.pcc_channel)) { >> + if (IS_ERR(pcc_data[pcc_ss_idx]->pcc_channel)) { >> pr_err("Failed to find PCC communication channel\n"); >> return -ENODEV; >> } >> @@ -556,7 +564,7 @@ static int register_pcc_channel(int pcc_subspace_idx) >> * PCC channels) and stored pointers to the >> * subspace communication region in con_priv. >> */ >> - cppc_ss = (pcc_data.pcc_channel)->con_priv; >> + cppc_ss = (pcc_data[pcc_ss_idx]->pcc_channel)->con_priv; >> >> if (!cppc_ss) { >> pr_err("No PCC subspace found for CPPC\n"); >> @@ -569,19 +577,20 @@ static int register_pcc_channel(int pcc_subspace_idx) >> * So add an arbitrary amount of wait on top of Nominal. >> */ >> usecs_lat = NUM_RETRIES * cppc_ss->latency; >> - pcc_data.deadline = ns_to_ktime(usecs_lat * NSEC_PER_USEC); >> - pcc_data.pcc_mrtt = cppc_ss->min_turnaround_time; >> - pcc_data.pcc_mpar = cppc_ss->max_access_rate; >> - pcc_data.pcc_nominal = cppc_ss->latency; >> - >> - pcc_data.pcc_comm_addr = acpi_os_ioremap(cppc_ss->base_address, cppc_ss->length); >> - if (!pcc_data.pcc_comm_addr) { >> + pcc_data[pcc_ss_idx]->deadline = ns_to_ktime(usecs_lat * NSEC_PER_USEC); >> + pcc_data[pcc_ss_idx]->pcc_mrtt = cppc_ss->min_turnaround_time; >> + pcc_data[pcc_ss_idx]->pcc_mpar = cppc_ss->max_access_rate; >> + pcc_data[pcc_ss_idx]->pcc_nominal = cppc_ss->latency; >> + >> + pcc_data[pcc_ss_idx]->pcc_comm_addr = >> + acpi_os_ioremap(cppc_ss->base_address, cppc_ss->length); >> + if (!pcc_data[pcc_ss_idx]->pcc_comm_addr) { >> pr_err("Failed to ioremap PCC comm region mem\n"); >> return -ENOMEM; >> } >> >> /* Set flag so that we dont come here for each CPU. */ >> - pcc_data.pcc_channel_acquired = true; >> + pcc_data[pcc_ss_idx]->pcc_channel_acquired = true; >> } >> >> return 0; >> @@ -600,6 +609,39 @@ bool __weak cpc_ffh_supported(void) >> return false; >> } >> >> + >> +/** >> + * pcc_data_alloc() - Allocate the pcc_data memory for pcc subspace >> + * >> + * Check and allocate the cppc_pcc_data memory. >> + * In some processor configurations it is possible that same subspace >> + * is shared between multiple CPU's. This is seen especially in CPU's >> + * with hardware multi-threading support. >> + * >> + * Return: 0 for success, errno for failure >> + */ >> +int pcc_data_alloc(int pcc_ss_id) >> +{ >> + int loop; >> + >> + if (pcc_ss_id < 0) > Above should be (pcc_ss_id < 0 || pcc_ss_id >= MAX_PCC_SUBSPACES) Yes we can have this additional check. >> + return -EINVAL; >> + >> + for (loop = 0; pcc_data[loop] != NULL; loop++) { >> + if (pcc_data[loop]->pcc_subspace_idx == pcc_ss_id) { >> + pcc_data[loop]->refcount++; >> + return 0; >> + } >> + } > Why do we need the above for loop? can't it be direct array lookup? > if (pcc_data[pcc_ss_id]) { >     //increment ref_count and return > } Yes, we could get rid of the loop and increment the reference directly if already allocated. > > Also, we should remove the pcc_subspace_idx from cppc_pcc_data structure, > it is no longer useful and probably adds to confusion. Please see below >> + >> + pcc_data[pcc_ss_id] = kzalloc(sizeof(struct cppc_pcc_data), GFP_KERNEL); >> + if (!pcc_data[pcc_ss_id]) >> + return -ENOMEM; >> + pcc_data[pcc_ss_id]->pcc_subspace_idx = pcc_ss_id; >> + pcc_data[pcc_ss_id]->refcount++; >> + >> + return 0; >> +} >> /* >> * An example CPC table looks like the following. >> * >> @@ -661,6 +703,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) >> struct device *cpu_dev; >> acpi_handle handle = pr->handle; >> unsigned int num_ent, i, cpc_rev; >> + int pcc_subspace_id = -1; >> acpi_status status; >> int ret = -EFAULT; >> >> @@ -733,12 +776,9 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) >> * so extract it only once. >> */ >> if (gas_t->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) { >> - if (pcc_data.pcc_subspace_idx < 0) >> - pcc_data.pcc_subspace_idx = gas_t->access_width; >> - else if (pcc_data.pcc_subspace_idx != gas_t->access_width) { >> - pr_debug("Mismatched PCC ids.\n"); > We need to retain the above checks to make sure all PCC registers > within a _CPC package is under same subspace. The Spec still requires: > "If the PCC register space is used, all PCC registers, for all processors in > the same performance domain (as defined by _PSD), must be defined > to be in the same subspace." So that means we need to maintain the pcc_subspace_idx in cppc_pcc_data. If so, then I can move the check inside pcc_data_alloc(). > >> + pcc_subspace_id = gas_t->access_width; >> + if (pcc_data_alloc(pcc_subspace_id)) >> goto out_free; > We need to call pcc_data_alloc(to increment the reference) only once per CPU, > otherwise acpi_cppc_processor_exit( ) will never free the memory allocated in > pcc_data. It is infact called only once per CPU, The refcounting was added in case if multiple CPU's share the same domain for eg: In instances, where CPU have hardware multi-threading support. or in cases where all CPU's are controlled using single subspace (as earlier). Do you see any issue with acpi_cppc_processor_exit() in the earlier cases, where refcount remains > 0 and the memory not getting freed-up? > > > -- > Thanks, > Prashanth >