Skip to content

cppc: advertise optional fast-channel doorbell (follow-up to autonomous mode) #119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
icosta-sifive wants to merge 1 commit into
base: main
Choose a base branch
from
+386 −18
Open

cppc: advertise optional fast-channel doorbell (follow-up to autonomous mode) #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions include/librpmi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1438,6 +1438,20 @@ enum rpmi_cppc_mode {
/**
* ACPI CPPC Registers
*/
/** CPPC Perf Request fast-channel doorbell */
struct rpmi_cppc_fastchan_doorbell {
/** Whether the Perf Request fast-channel doorbell is supported */
rpmi_bool_t db_supported;
/** Perf Request fast-channel doorbell register width in bits */
rpmi_uint32_t perf_req_fastchan_width;
/** doorbell addr low */
rpmi_uint32_t db_addr_low;
/** doorbell addr high */
rpmi_uint32_t db_addr_high;
/** doorbell write value */
rpmi_uint32_t db_write_value;
};

struct rpmi_cppc_regs {
/* highest performance (r) */
rpmi_uint32_t highest_perf;
Expand Down Expand Up @@ -1578,6 +1592,7 @@ struct rpmi_cppc_platform_ops {
* @param[in] shmem_fastchan pointer to fastchannel shared memory instance
* @param[in] perf_request_shmem_offset perf request fastchannel shmem region offset
* @param[in] perf_feedback_shmem_offset perf feedback fastchannel shmem region offset
* @param[in] doorbell optional Perf Request fastchannel doorbell
* @param[in] ops pointer to platform specific cppc operations
* @param[in] ops_priv pointer to private data of platform operations
* @return rpmi_service_group * pointer to RPMI service group instance upon
Expand All @@ -1590,6 +1605,7 @@ rpmi_service_group_cppc_create(struct rpmi_hsm *hsm,
struct rpmi_shmem *shmem_fastchan,
rpmi_uint64_t perf_request_shmem_offset,
rpmi_uint64_t perf_feedback_shmem_offset,
const struct rpmi_cppc_fastchan_doorbell *doorbell,
const struct rpmi_cppc_platform_ops *ops,
void *ops_priv);

Expand Down
14 changes: 14 additions & 0 deletions lib/librpmi_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@
#define array_size(x) (sizeof(x) / sizeof((x)[0]))
#endif

#define RPMI_BIT(nr) (1UL << (nr))
#define RPMI_GENMASK(h, l) ((~0UL << (l)) & \
(~0UL >> ((sizeof(unsigned long) * 8) - 1 - (h))))

/** CPPC fast-channel flags */
#define RPMI_CPPC_FST_CHN_MODE_NORMAL 0U
#define RPMI_CPPC_FST_CHN_MODE_AUTONOMOUS RPMI_BIT(3)
#define RPMI_CPPC_FST_CHN_DB_REG_WIDTH_MASK RPMI_GENMASK(2, 1)
#define RPMI_CPPC_FST_CHN_DB_REG_08_BITS 0U
#define RPMI_CPPC_FST_CHN_DB_REG_16_BITS RPMI_BIT(1)
#define RPMI_CPPC_FST_CHN_DB_REG_32_BITS RPMI_BIT(2)
#define RPMI_CPPC_FST_CHN_DB_NOT_SUPP 0U
#define RPMI_CPPC_FST_CHN_DB_SUPP RPMI_BIT(0)

#define RPMI_MAX(a, b) \
({ \
__typeof__(a) _a = (a); \
Expand Down
73 changes: 58 additions & 15 deletions lib/rpmi_service_group_cppc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

#include <librpmi.h>
#include "librpmi_internal.h"

#ifdef LIBRPMI_DEBUG
#define DPRINTF(msg...) rpmi_env_printf(msg)
Expand Down Expand Up @@ -41,6 +42,8 @@ _Static_assert( \
struct rpmi_cppc_fastchan {
/** shared memory backing the fast channels */
struct rpmi_shmem *shmem;
/** optional Perf Request fastchannel doorbell */
const struct rpmi_cppc_fastchan_doorbell *doorbell;

rpmi_uint64_t perf_request_shmem_offset;
rpmi_uint64_t perf_feedback_shmem_offset;
Expand Down Expand Up @@ -637,36 +640,60 @@ rpmi_cppc_sg_get_fast_channel_region(struct rpmi_service_group *group,
rpmi_uint32_t resp_dlen, flags;
rpmi_uint64_t fastchan_region_base, fastchan_region_size;
struct rpmi_cppc_group *cppcgrp = group->priv;
struct rpmi_cppc_fastchan *fc = cppcgrp->fastchan_ctx;
const struct rpmi_cppc_fastchan_doorbell *db;
rpmi_uint32_t *resp = (void *)response_data;

if (!cppcgrp->fastchan_ctx) {
if (!fc) {
status = RPMI_ERR_NOTSUPP;
resp_dlen = sizeof(*resp);
goto done;
}

fastchan_region_base = rpmi_shmem_base(cppcgrp->fastchan_ctx->shmem);
fastchan_region_size = rpmi_shmem_size(cppcgrp->fastchan_ctx->shmem);
db = fc->doorbell;
fastchan_region_base = rpmi_shmem_base(fc->shmem);
fastchan_region_size = rpmi_shmem_size(fc->shmem);

/* FLAGS[4:3]: 0b00 = normal/passive, 0b01 = autonomous. No doorbell. */
flags = (cppcgrp->cppc_mode == RPMI_CPPC_AUTO_MODE) ? (0x01U << 3) : 0;
/* FLAGS[4:3] CPPC mode */
flags = (cppcgrp->cppc_mode == RPMI_CPPC_AUTO_MODE) ?
RPMI_CPPC_FST_CHN_MODE_AUTONOMOUS : RPMI_CPPC_FST_CHN_MODE_NORMAL;

/* FLAGS[2:1] doorbell width, FLAGS[0] doorbell support (optional) */
if (db) {
flags |= RPMI_CPPC_FST_CHN_DB_SUPP;
switch (db->perf_req_fastchan_width) {
case 8:
flags |= RPMI_CPPC_FST_CHN_DB_REG_08_BITS;
break;
case 16:
flags |= RPMI_CPPC_FST_CHN_DB_REG_16_BITS;
break;
case 32:
flags |= RPMI_CPPC_FST_CHN_DB_REG_32_BITS;
break;
}
/* doorbell addr low */
resp[6] = rpmi_to_xe32(trans->is_be, db->db_addr_low);
/* doorbell addr high */
resp[7] = rpmi_to_xe32(trans->is_be, db->db_addr_high);
/* doorbell write value */
resp[8] = rpmi_to_xe32(trans->is_be, db->db_write_value);
} else {
resp[6] = 0;
resp[7] = 0;
resp[8] = 0;
}

@saini-ranbirs saini-ranbirs Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments for resp[6/7/8] are visible in close proximity through the if part, so else part even without comments wrt resp[6/7/8] and the resp[6] = resp[7]= resp[8] = 0; assignment should be good enough.

@icosta-sifive icosta-sifive Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saini-ranbirs I initially combined these assignments for conciseness, but reverted to separate assignments after running the full patch through Linux checkpatch.pl --strict, which flags chained assignments with MULTIPLE_ASSIGNMENTS. The behavior is unchanged; this is only to follow the project’s Linux-style formatting and keep the patch checkpatch-clean.

@saini-ranbirs saini-ranbirs Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. However, the else part is for DB not supported. So, see if those comments there really make sense.

@icosta-sifive icosta-sifive Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Since the else path means the doorbell is not supported, the individual doorbell field comments are unnecessary there. I’ve removed them and kept the assignments separate to avoid the checkpatch MULTIPLE_ASSIGNMENTS warning.

@saini-ranbirs saini-ranbirs Sep 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per spec, the doorbell details are unspecified and considered invalid if the Performance Request fast-channel
doorbell (FLAGS[0] = 0) is not supported and must not be used.

Hence, it seems safe even if else part assignments are totally removed. One more option is to set the resp_dlen appropriately based on DB supported or not supported as 9 or 6 times .. respectively.

@icosta-sifive icosta-sifive Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although those fields must not be used when the doorbell is unsupported, librpmi reuses the ACK buffer, so leaving words 6–8 unwritten could expose stale data. Also, the spec defines words 0–8 as the response layout, so I’d prefer to keep the fixed 9-word response and explicitly zero the unused fields.


status = RPMI_SUCCESS;
resp[1] = rpmi_to_xe32(trans->is_be, (rpmi_uint32_t)flags);
/* fast channel region address low */
resp[2] = rpmi_to_xe32(trans->is_be, (rpmi_uint32_t)fastchan_region_base);
/* fast channel region address low */
/* fast channel region address high */
resp[3] = rpmi_to_xe32(trans->is_be, (rpmi_uint32_t)(fastchan_region_base >> 32));
/* fast channel region size low */
resp[4] = rpmi_to_xe32(trans->is_be, (rpmi_uint32_t)fastchan_region_size);;
resp[4] = rpmi_to_xe32(trans->is_be, (rpmi_uint32_t)fastchan_region_size);
/* fast channel region size high */
resp[5] = rpmi_to_xe32(trans->is_be, (rpmi_uint32_t)(fastchan_region_size >> 32));
/* doorbell addr low */
resp[6] = 0;
/* doorbell addr high */
resp[7] = 0;
/* doorbell write value */
resp[8] = 0;

resp_dlen = 9 * sizeof(*resp);

Expand Down Expand Up @@ -869,7 +896,8 @@ static struct rpmi_cppc_fastchan *
rpmi_cppc_fastchan_create(rpmi_uint32_t hart_count,
struct rpmi_shmem *shmem_fastchan,
rpmi_uint64_t perf_request_shmem_offset,
rpmi_uint64_t perf_feedback_shmem_offset)
rpmi_uint64_t perf_feedback_shmem_offset,
const struct rpmi_cppc_fastchan_doorbell *doorbell)
{
struct rpmi_cppc_fastchan *cppc_fastchan_ctx;
rpmi_size_t fc_perf_request_region_size, fc_perf_feedback_region_size;
Expand Down Expand Up @@ -996,6 +1024,7 @@ rpmi_cppc_fastchan_create(rpmi_uint32_t hart_count,

cppc_fastchan_ctx->hart_perf_request = fc_hart_perf_request_array;
cppc_fastchan_ctx->shmem = shmem_fastchan;
cppc_fastchan_ctx->doorbell = doorbell;
cppc_fastchan_ctx->perf_request_shmem_offset = perf_request_shmem_offset;
cppc_fastchan_ctx->perf_feedback_shmem_offset = perf_feedback_shmem_offset;

Expand All @@ -1009,6 +1038,7 @@ rpmi_service_group_cppc_create(struct rpmi_hsm *hsm,
struct rpmi_shmem *shmem_fastchan,
rpmi_uint64_t perf_request_shmem_offset,
rpmi_uint64_t perf_feedback_shmem_offset,
const struct rpmi_cppc_fastchan_doorbell *doorbell,
const struct rpmi_cppc_platform_ops *ops,
void *ops_priv)
{
Expand All @@ -1027,6 +1057,18 @@ rpmi_service_group_cppc_create(struct rpmi_hsm *hsm,
return NULL;
}

if (doorbell && !doorbell->db_supported) {
DPRINTF("%s: cppc fastchannel doorbell support not set\n", __func__);
return NULL;
}

if (doorbell && doorbell->perf_req_fastchan_width != 8 &&
doorbell->perf_req_fastchan_width != 16 &&
doorbell->perf_req_fastchan_width != 32) {
DPRINTF("%s: invalid cppc fastchannel doorbell width\n", __func__);
return NULL;
}

/** allocate cppc group instance memory */
cppcgrp = rpmi_env_zalloc(sizeof(*cppcgrp));
if (!cppcgrp) {
Expand All @@ -1046,7 +1088,8 @@ rpmi_service_group_cppc_create(struct rpmi_hsm *hsm,
cppc_fastchan_ctx = rpmi_cppc_fastchan_create(hart_count,
shmem_fastchan,
perf_request_shmem_offset,
perf_feedback_shmem_offset);
perf_feedback_shmem_offset,
doorbell);
if (!cppc_fastchan_ctx) {
DPRINTF("%s: failed to create cppc fastchannel\n", __func__);
rpmi_env_free(cppcgrp);
Expand Down
Loading