nouveau/gsp: don't free ctrl messages on errors

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

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index 2fa0445d89280c6677e58ccafa8f91b5962b5f76..d1437c08645f90d9c745ee77405d3fa1d8d51f9d 100644 (file)
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -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);
@@ -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);
@@ -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;
 }
 

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c
index 1c8c4cca0957a1f7d7bf7c8f39c5a7452eff5b96..1b4f988df7ed80bb9a50459cf98c73d8cf4e1278 100644 (file)
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c
@@ -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,
@@ -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);
@@ -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) {
@@ -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));
@@ -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)) {
@@ -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));
@@ -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);
@@ -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;
@@ -825,6 +836,7 @@ 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))
@@ -832,9 +844,11 @@ r535_outp_dfp_get_info(struct nvkm_outp *outp)
 
        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);
@@ -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);
@@ -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,
@@ -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);
@@ -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);
@@ -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;
@@ -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;
@@ -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:
@@ -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:

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 263dbe45bb36d33f2747e231139905c916f7a392..642ce0dd0faf00008b825ff5ddd9f8f4aeb63f77 100644 (file)
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -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;
@@ -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;
@@ -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;