Skip to content

Commit

Permalink
nouveau/gsp: don't free ctrl messages on errors
Browse files Browse the repository at this point in the history 
It looks like for some messages the upper layers need to get access to the
results of the message so we can interpret it.

Rework the ctrl push interface to not free things and cleanup properly
whereever it errors out.

Requested-by: Lyude
Signed-off-by: Dave Airlie <airlied@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231222043308.3090089-9-airlied@gmail.com
  • Loading branch information
Dave Airlie authored and Dave Airlie committed Jan 5, 2024
1 parent 59f6a3d commit 4ae3a20
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 61 deletions.
17 changes: 10 additions & 7 deletions drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ struct nvkm_gsp {
void (*rpc_done)(struct nvkm_gsp *gsp, void *repv);

void *(*rm_ctrl_get)(struct nvkm_gsp_object *, u32 cmd, u32 argc);
void *(*rm_ctrl_push)(struct nvkm_gsp_object *, void *argv, u32 repc);
int (*rm_ctrl_push)(struct nvkm_gsp_object *, void **argv, u32 repc);
void (*rm_ctrl_done)(struct nvkm_gsp_object *, void *repv);

void *(*rm_alloc_get)(struct nvkm_gsp_object *, u32 oclass, u32 argc);
Expand Down Expand Up @@ -265,7 +265,7 @@ nvkm_gsp_rm_ctrl_get(struct nvkm_gsp_object *object, u32 cmd, u32 argc)
return object->client->gsp->rm->rm_ctrl_get(object, cmd, argc);
}

static inline void *
static inline int
nvkm_gsp_rm_ctrl_push(struct nvkm_gsp_object *object, void *argv, u32 repc)
{
return object->client->gsp->rm->rm_ctrl_push(object, argv, repc);
Expand All @@ -275,21 +275,24 @@ static inline void *
nvkm_gsp_rm_ctrl_rd(struct nvkm_gsp_object *object, u32 cmd, u32 repc)
{
void *argv = nvkm_gsp_rm_ctrl_get(object, cmd, repc);
int ret;

if (IS_ERR(argv))
return argv;

return nvkm_gsp_rm_ctrl_push(object, argv, repc);
ret = nvkm_gsp_rm_ctrl_push(object, &argv, repc);
if (ret)
return ERR_PTR(ret);
return argv;
}

static inline int
nvkm_gsp_rm_ctrl_wr(struct nvkm_gsp_object *object, void *argv)
{
void *repv = nvkm_gsp_rm_ctrl_push(object, argv, 0);

if (IS_ERR(repv))
return PTR_ERR(repv);
int ret = nvkm_gsp_rm_ctrl_push(object, &argv, 0);

if (ret)
return ret;
return 0;
}

Expand Down
108 changes: 69 additions & 39 deletions drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ r535_sor_bl_get(struct nvkm_ior *sor)
{
struct nvkm_disp *disp = sor->disp;
NV0073_CTRL_SPECIFIC_BACKLIGHT_BRIGHTNESS_PARAMS *ctrl;
int lvl;
int ret, lvl;

ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
NV0073_CTRL_CMD_SPECIFIC_GET_BACKLIGHT_BRIGHTNESS,
Expand All @@ -292,9 +292,11 @@ r535_sor_bl_get(struct nvkm_ior *sor)

ctrl->displayId = BIT(sor->asy.outp->index);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

lvl = ctrl->brightness;
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
Expand Down Expand Up @@ -649,9 +651,11 @@ r535_conn_new(struct nvkm_disp *disp, u32 id)
ctrl->subDeviceInstance = 0;
ctrl->displayId = BIT(id);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return (void *)ctrl;
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ERR_PTR(ret);
}

list_for_each_entry(conn, &disp->conns, head) {
if (conn->index == ctrl->data[0].index) {
Expand Down Expand Up @@ -686,7 +690,7 @@ r535_outp_acquire(struct nvkm_outp *outp, bool hda)
struct nvkm_disp *disp = outp->disp;
struct nvkm_ior *ior;
NV0073_CTRL_DFP_ASSIGN_SOR_PARAMS *ctrl;
int or;
int ret, or;

ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
NV0073_CTRL_CMD_DFP_ASSIGN_SOR, sizeof(*ctrl));
Expand All @@ -699,9 +703,11 @@ r535_outp_acquire(struct nvkm_outp *outp, bool hda)
if (hda)
ctrl->flags |= NVDEF(NV0073_CTRL, DFP_ASSIGN_SOR_FLAGS, AUDIO, OPTIMAL);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

for (or = 0; or < ARRAY_SIZE(ctrl->sorAssignListWithTag); or++) {
if (ctrl->sorAssignListWithTag[or].displayMask & BIT(outp->index)) {
Expand All @@ -727,6 +733,7 @@ static int
r535_disp_head_displayid(struct nvkm_disp *disp, int head, u32 *displayid)
{
NV0073_CTRL_SYSTEM_GET_ACTIVE_PARAMS *ctrl;
int ret;

ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
NV0073_CTRL_CMD_SYSTEM_GET_ACTIVE, sizeof(*ctrl));
Expand All @@ -736,9 +743,11 @@ r535_disp_head_displayid(struct nvkm_disp *disp, int head, u32 *displayid)
ctrl->subDeviceInstance = 0;
ctrl->head = head;

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

*displayid = ctrl->displayId;
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
Expand Down Expand Up @@ -772,9 +781,11 @@ r535_outp_inherit(struct nvkm_outp *outp)
ctrl->subDeviceInstance = 0;
ctrl->displayId = displayid;

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return NULL;
}

id = ctrl->index;
proto = ctrl->protocol;
Expand Down Expand Up @@ -825,16 +836,19 @@ r535_outp_dfp_get_info(struct nvkm_outp *outp)
{
NV0073_CTRL_DFP_GET_INFO_PARAMS *ctrl;
struct nvkm_disp *disp = outp->disp;
int ret;

ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom, NV0073_CTRL_CMD_DFP_GET_INFO, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);

ctrl->displayId = BIT(outp->index);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

nvkm_debug(&disp->engine.subdev, "DFP %08x: flags:%08x flags2:%08x\n",
ctrl->displayId, ctrl->flags, ctrl->flags2);
Expand All @@ -858,9 +872,11 @@ r535_outp_detect(struct nvkm_outp *outp)
ctrl->subDeviceInstance = 0;
ctrl->displayMask = BIT(outp->index);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

if (ctrl->displayMask & BIT(outp->index)) {
ret = r535_outp_dfp_get_info(outp);
Expand Down Expand Up @@ -895,6 +911,7 @@ r535_dp_mst_id_get(struct nvkm_outp *outp, u32 *pid)
{
NV0073_CTRL_CMD_DP_TOPOLOGY_ALLOCATE_DISPLAYID_PARAMS *ctrl;
struct nvkm_disp *disp = outp->disp;
int ret;

ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
NV0073_CTRL_CMD_DP_TOPOLOGY_ALLOCATE_DISPLAYID,
Expand All @@ -904,9 +921,11 @@ r535_dp_mst_id_get(struct nvkm_outp *outp, u32 *pid)

ctrl->subDeviceInstance = 0;
ctrl->displayId = BIT(outp->index);
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

*pid = ctrl->displayIdAssigned;
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
Expand Down Expand Up @@ -964,9 +983,11 @@ r535_dp_train_target(struct nvkm_outp *outp, u8 target, bool mst, u8 link_nr, u8
!(outp->dp.dpcd[DPCD_RC03] & DPCD_RC03_TPS4_SUPPORTED))
ctrl->cmd |= NVDEF(NV0073_CTRL, DP_CMD, POST_LT_ADJ_REQ_GRANTED, YES);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

ret = ctrl->err ? -EIO : 0;
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
Expand Down Expand Up @@ -1036,9 +1057,11 @@ r535_dp_aux_xfer(struct nvkm_outp *outp, u8 type, u32 addr, u8 *data, u8 *psize)
ctrl->size = !ctrl->bAddrOnly ? (size - 1) : 0;
memcpy(ctrl->data, data, size);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return PTR_ERR(ctrl);
}

memcpy(data, ctrl->data, size);
*psize = ctrl->size;
Expand Down Expand Up @@ -1111,10 +1134,13 @@ r535_tmds_edid_get(struct nvkm_outp *outp, u8 *data, u16 *psize)
ctrl->subDeviceInstance = 0;
ctrl->displayId = BIT(outp->index);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

ret = -E2BIG;
if (ctrl->bufferSize <= *psize) {
memcpy(data, ctrl->edidBuffer, ctrl->bufferSize);
*psize = ctrl->bufferSize;
Expand Down Expand Up @@ -1153,9 +1179,11 @@ r535_outp_new(struct nvkm_disp *disp, u32 id)
ctrl->subDeviceInstance = 0;
ctrl->displayId = BIT(id);

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

switch (ctrl->type) {
case NV0073_CTRL_SPECIFIC_OR_TYPE_NONE:
Expand Down Expand Up @@ -1229,9 +1257,11 @@ r535_outp_new(struct nvkm_disp *disp, u32 id)

ctrl->sorIndex = ~0;

ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
if (ret) {
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
return ret;
}

switch (NVVAL_GET(ctrl->maxLinkRate, NV0073_CTRL_CMD, DP_GET_CAPS, MAX_LINK_RATE)) {
case NV0073_CTRL_CMD_DP_GET_CAPS_MAX_LINK_RATE_1_62:
Expand Down
36 changes: 21 additions & 15 deletions drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -599,13 +599,13 @@ r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *argv, u32 repc)

if (rpc->status) {
ret = ERR_PTR(r535_rpc_status_to_errno(rpc->status));
if (ret != -EAGAIN)
if (PTR_ERR(ret) != -EAGAIN)
nvkm_error(&gsp->subdev, "RM_ALLOC: 0x%x\n", rpc->status);
} else {
ret = repc ? rpc->params : NULL;
}

if (IS_ERR_OR_NULL(ret))
if (ret)
nvkm_gsp_rpc_done(gsp, rpc);

return ret;
Expand Down Expand Up @@ -639,30 +639,34 @@ r535_gsp_rpc_rm_ctrl_done(struct nvkm_gsp_object *object, void *repv)
{
rpc_gsp_rm_control_v03_00 *rpc = container_of(repv, typeof(*rpc), params);

if (!repv)
return;
nvkm_gsp_rpc_done(object->client->gsp, rpc);
}

static void *
r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void *argv, u32 repc)
static int
r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **argv, u32 repc)
{
rpc_gsp_rm_control_v03_00 *rpc = container_of(argv, typeof(*rpc), params);
rpc_gsp_rm_control_v03_00 *rpc = container_of((*argv), typeof(*rpc), params);
struct nvkm_gsp *gsp = object->client->gsp;
void *ret;
int ret = 0;

rpc = nvkm_gsp_rpc_push(gsp, rpc, true, repc);
if (IS_ERR_OR_NULL(rpc))
return rpc;
if (IS_ERR_OR_NULL(rpc)) {
*argv = NULL;
return PTR_ERR(rpc);
}

if (rpc->status) {
ret = ERR_PTR(r535_rpc_status_to_errno(rpc->status));
ret = r535_rpc_status_to_errno(rpc->status);
if (ret != -EAGAIN)
nvkm_error(&gsp->subdev, "cli:0x%08x obj:0x%08x ctrl cmd:0x%08x failed: 0x%08x\n",
object->client->object.handle, object->handle, rpc->cmd, rpc->status);
} else {
ret = repc ? rpc->params : NULL;
}

if (IS_ERR_OR_NULL(ret))
if (repc)
*argv = rpc->params;
else
nvkm_gsp_rpc_done(gsp, rpc);

return ret;
Expand Down Expand Up @@ -860,9 +864,11 @@ r535_gsp_intr_get_table(struct nvkm_gsp *gsp)
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);

ctrl = nvkm_gsp_rm_ctrl_push(&gsp->internal.device.subdevice, ctrl, sizeof(*ctrl));
if (WARN_ON(IS_ERR(ctrl)))
return PTR_ERR(ctrl);
ret = nvkm_gsp_rm_ctrl_push(&gsp->internal.device.subdevice, &ctrl, sizeof(*ctrl));
if (WARN_ON(ret)) {
nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, ctrl);
return ret;
}

for (unsigned i = 0; i < ctrl->tableLen; i++) {
enum nvkm_subdev_type type;
Expand Down

0 comments on commit 4ae3a20

Please sign in to comment.